Skip to content
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.

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

macro/align: Use ALIGN_UP and ALIGN_DOWN uniformly #15455

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

W-M-R
Copy link
Contributor

@W-M-R W-M-R commented Jan 7, 2025

Note: Please adhere to Contributing Guidelines.

Summary

macro/align: Use ALIGN_UP and ALIGN_DOWN uniformly

Impact

Update this section, where applicable, on how change affects users,
build process, hardware, documentation, security, compatibility, etc.

Testing

Update this section with details on how did you verify the change,
what Host was used for build (OS, CPU, compiler, ..), what Target was
used for verification (arch, board:config, ..), etc. Providing build
and runtime logs from before and after change is highly appreciated.

@github-actions github-actions bot added Arch: arm Issues related to ARM (32-bit) architecture Arch: arm64 Issues related to ARM64 (64-bit) architecture Area: Memory Management Memory Management issues Area: OS Components OS Components issues Area: BINFMT Size: S The size of the change in this PR is small labels Jan 7, 2025
arch/arm/src/imx9/imx9_lpi2c.c Show resolved Hide resolved
arch/arm/src/imxrt/imxrt_lpi2c.c Show resolved Hide resolved
arch/arm/src/s32k1xx/s32k1xx_lpi2c.c Show resolved Hide resolved
arch/arm/src/s32k3xx/s32k3xx_lpi2c.c Show resolved Hide resolved
arch/arm64/src/imx9/imx9_lpi2c.c Show resolved Hide resolved
@xiaoxiang781216
Copy link
Contributor

please fix ci error:

D:\a\nuttx\nuttx\sources\nuttx\mm\iob\iob_initialize.c(63,29): error C2057: expected constant expression [D:\a\nuttx\nuttx\sources\nuttx\vs2022\mm\mm.vcxproj]
  iob_trimhead.c
D:\a\nuttx\nuttx\sources\nuttx\mm\iob\iob_initialize.c(63,29): error C2466: cannot allocate an array of constant size 0 [D:\a\nuttx\nuttx\sources\nuttx\vs2022\mm\mm.vcxproj]
D:\a\nuttx\nuttx\sources\nuttx\mm\iob\iob_initialize.c(63,16): error C2133: 'g_iob_buffer': unknown size [D:\a\nuttx\nuttx\sources\nuttx\vs2022\mm\mm.vcxproj]

binfmt/binfmt_copyactions.c Show resolved Hide resolved
uintptr_t highpc = ROUNDUP((uintptr_t)&_etext,
HISTFRACTION * sizeof(HISTCOUNTER));
uintptr_t highpc = ALIGN_UP((uintptr_t)&_etext,
HISTFRACTION * sizeof(HISTCOUNTER));
Copy link
Contributor

Choose a reason for hiding this comment

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

it isn't obvious HISTFRACTION * sizeof(HISTCOUNTER) is a power of two.

is it possible to have a power-of-two assertion in ALIGN_UP itself?
this PR as it is seems a bit fragile to me.

does ALIGN_UP actually produce a better code than ROUNDUP with a power of two y with modern compilers? otherwise, maybe it's safer to drop the power-of-two assumption from ALIGN_UP.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think what you said makes sense. Let me open a new PR to fix this problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix it in the patch https://github.com/apache/nuttx/pull/15493/files. Sorry, I didn't notice this problem before.

Signed-off-by: wangmingrong1 <wangmingrong1@xiaomi.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Arch: arm Issues related to ARM (32-bit) architecture Arch: arm64 Issues related to ARM64 (64-bit) architecture Area: BINFMT Area: Memory Management Memory Management issues Area: OS Components OS Components issues Size: S The size of the change in this PR is small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants