[RFC] core: assure alignment is only done on power of 2 values

[lyakh]

Alignment should always be done on powers of 2. Enforcing this rule also allows the use of binary AND for alignment instead of much slower division. Also add compile-time checks where possible.

[paulstelian97]

Are the failures related to the PR or is the CI crazy?

[lgirdwood]

@paulstelian97 CI looks wrong - this could be due to maintenance so I will rerun.

[lgirdwood]

SOFCI TEST

[lgirdwood]

@lyakh I've merged some PRs so henec the conflicts - best to rebase and check on your return.

[lgirdwood]

Can you put more context in the commit message telling us what this is fixing today. i.e. where are non powers of two being used today.

[lyakh]

Can you put more context in the commit message telling us what this is fixing today. i.e. where are non powers of two being used today.

@lgirdwood that's exactly the purpose of this PR - to identify any such occurrences. So far I'm not aware of any but since until now SOF alignment supported non power of 2 values, I propose to put compilation guards for any such calls. So far I'm not aware of any. I'll extend the commit message.

[lgirdwood]

More comments on why GCC and XCC are different here.

[lgirdwood]

why is this different for XCC, need to state this delta in the comments.

[lyakh]

@lgirdwood comment added, conflict resolved. However I don't like all qemu targets failing (at least before). Let's wait for the new test to finish and if failures are still there, who can help get some debugging information from those failures?

[lgirdwood]

@lyakh all passing now.

[marc-hb]

The use of GCC statement-expressions in ALIGN means these macros cannot be used to initialize statics anymore. This broke ./scripts/xtensa-build-all.sh sue (and ../scripts/xtensa-build-all.sh -a) While we may not care about Sue Creek (CI does not build it), maybe we care about a less usable ALIGN macro?

See quick and dirty compilation fix with more details in marc-hb@unalign-sue

[marc-hb]

Would C11 _Static_assert() make this simpler and as flexible as before?

Is C11 too high of a requirement?

[marc-hb]

Would C11 _Static_assert() make this simpler and as flexible as before?

Simpler yes, as flexible as before: no. _Static_assert() seems to be a statement syntactically or something close enough to it.

Naive question: why not just change the "ALIGN API" to have a 2-exponent argument instead? I mean like this:

-#define  ALIGN_UP_INTERNAL(val, align) (((val) + (     align) - 1) & ~((     align) - 1))
+#define ALIGN_UP2_INTERNAL(val, align) (((val) + (1 << align) - 1) & ~((1 << align) - 1))

Since you performed a rename and search/replace anyway. No need for any assert anymore, this is correct by construction.
EXT_MAN_ALIGN becomes EXT_MAN_EXPONENT_ALIGN or something. If there are some inputs that really cannot be converted to an exponent then leave the old macro only in those places and add an explicit assert "manually" next to them instead of sneaking one in a statement-expression.

Just for fun PS, in 2007: https://lkml.org/lkml/2007/1/10/165

I noticed that the ALIGN() macro in kernel.h didn't verify that the
alignment value was a power of 2, so i thought...

[marc-hb]

I found the ALIGN_UP_COMPILE macro without any non standard GCC statement-expression and meant for static initializers. However memory.h files are also included in assembly and in linker files, so it was not enough.

Tentative fix in common.h: restore basic ALIGN() macro compatible with assembly #3747