Skip to content

compile: restore alignment checks for some cases of ALIGN() #3804

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
lyakh wants to merge 1 commit into from
Closed

compile: restore alignment checks for some cases of ALIGN() #3804

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions src/include/sof/common.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@
#ifndef __SOF_COMMON_H__
#define __SOF_COMMON_H__

/*
* The most basic alignment macro with no checks, should only be used where no
* other checking macros are allowed, e.g. for values, used in assembly.
*/
#define ALIGN_UP_INTERNAL(val, align) (((val) + (align) - 1) & ~((align) - 1))
Copy link
Collaborator

@marc-hb marc-hb Feb 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The most basic alignment macro with no checks, suitable for C, assembly and for linker scripts

This comment is still misleading:

  • maybe you meant "suitable for C static initializers"?
  • it seems to be contradicting this other comment below:

memory.h values are also copied unprocessed to the .x[.in] linker script

that's because ALIGN is unprocessed while ALIGN_UP_INTERNAL is. Too much of a mess.

Copy link
Collaborator Author

@lyakh lyakh Feb 24, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The most basic alignment macro with no checks, suitable for C, assembly and for linker scripts

This comment is still misleading:

* maybe you meant "suitable for C _static initializers_"?
* it seems to be contradicting this other comment below:

@marc-hb I tried to improve it, please, check.

memory.h values are also copied unprocessed to the .x[.in] linker script

that's because ALIGN is unprocessed while ALIGN_UP_INTERNAL is. Too much of a mess.

I'd suggest, that ALIGN() is unprocessed, because common.h isn't included in memory.h as it really should. But interestingly, adding it there doesn't change the resulting .x, strange.

Anyway, I'd like to just commit this one instead of going your proposed route of reverting and fixing. We have introduced an albeit harmless, but still a regression and it would be good to fix it, if possible for 1.7. After that you can propose any further improvements.


#if !defined(LINKER)

/* Align the number to the nearest alignment value */
Expand All @@ -20,8 +26,6 @@
#define compile_check(x) (sizeof(struct{int _f : 1 - !(x); }))
#define compile_fail_zero_or_false_fn(x) ({int _f[(x) - 1]; 0 & _f[0]; })

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

#define VERIFY_ALIGN
#ifdef VERIFY_ALIGN

Expand Down Expand Up @@ -83,12 +87,12 @@

#endif

/* This most basic ALIGN() must be used in header files that are
* included in both C and assembly code. memory.h files require this
* exact spelling matching the linker function because memory.h values
* are _also_ copied unprocessed to the .x[.in] linker script
/*
* memory.h files require this exact spelling of the ALIGN() macro, matching the
* linker function name because memory.h values are _also_ copied unprocessed to
* the .x[.in] linker script
*/
#define ALIGN(val, align) ALIGN_UP_INTERNAL(val, align)
#define ALIGN ALIGN_UP
#define DIV_ROUND_UP(val, div) (((val) + (div) - 1) / (div))

#if !defined(__ASSEMBLER__)
Expand Down Expand Up @@ -157,9 +161,6 @@
#endif /* __XCC__ */

#endif /* __ASSEMBLER__ */
#else /* LINKER */

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

#endif /* LINKER */
#endif /* __SOF_COMMON_H__ */