Raul Rangel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/40765 )
Change subject: Makefile: Add is-power-of-two ......................................................................
Makefile: Add is-power-of-two
It is sometimes necessary to verify if a CONFIG_ option is a power of two at build time. This adds a `make` function `is-power-of-two`.
I chose to define all the values because it's the most straightforward way to do this with `make`.
BUG=b:147042464
Signed-off-by: Raul E Rangel rrangel@chromium.org Change-Id: I189a4c722996279e2d8940c566cb362f53ef92d8 --- M Makefile.inc 1 file changed, 12 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/40765/1
diff --git a/Makefile.inc b/Makefile.inc index e315732..cbec2dd 100644 --- a/Makefile.inc +++ b/Makefile.inc @@ -133,6 +133,7 @@ # int-gt: 1 if the first values is greater than the second. 0 otherwise # int-eq: 1 if the two values are equal. 0 otherwise # int-align: align $1 to $2 units +# is-power-of-two:1 if value is a power of two # file-size: returns the filesize of the given file # tolower: returns the value in all lowercase # toupper: returns the value in all uppercase @@ -151,6 +152,15 @@ int-eq=$(if $(filter 1,$(words $1)),$(strip $1),$(shell expr $(call _toint,$(word 1,$1)) = $(call _toint,$(word 2,$1)))) int-align=$(shell A=$(call _toint,$1) B=$(call _toint,$2); expr $$A + ( ( $$B - ( $$A % $$B ) ) % $$B ) ) int-align-down=$(shell A=$(call _toint,$1) B=$(call _toint,$2); expr $$A - ( $$A % $$B ) ) +power-of-twos := 0x00000001 0x00000002 0x00000004 0x00000008 \ +0x00000010 0x00000020 0x00000040 0x00000080 \ +0x00000100 0x00000200 0x00000400 0x00000800 \ +0x00001000 0x00002000 0x00004000 0x00008000 \ +0x00010000 0x00020000 0x00040000 0x00080000 \ +0x00100000 0x00200000 0x00400000 0x00800000 \ +0x01000000 0x02000000 0x04000000 0x08000000 \ +0x10000000 0x20000000 0x40000000 0x80000000 +is-power-of-two=$(if $(filter $(power-of-twos), $(shell printf "0x%08x" $1)),1) file-size=$(strip $(shell cat $1 | wc -c)) tolower=$(shell echo '$1' | tr '[:upper:]' '[:lower:]') toupper=$(shell echo '$1' | tr '[:lower:]' '[:upper:]') @@ -575,6 +585,8 @@ @printf " HOSTCC $(subst $(obj)/,,$(@))\n" $(HOSTCC) $(HOSTCFLAGS) -DCONFIG_ROM_SIZE=$(CONFIG_ROM_SIZE) -o $@ $<
+APCB_EDIT_TOOL:=$(top)/util/apcb/apcb_edit.py + CBOOTIMAGE:=$(objutil)/cbootimage/cbootimage
FUTILITY?=$(objutil)/futility/futility
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40765 )
Change subject: Makefile: Add is-power-of-two ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40765/1/Makefile.inc File Makefile.inc:
https://review.coreboot.org/c/coreboot/+/40765/1/Makefile.inc@162 PS1, Line 162: 0x10000000 0x20000000 0x40000000 0x80000000 Can't you just put a _Static_assert() in a C file instead of doing this? (The IS_POWER_OF_2() we have probably doesn't work in static context, but if you need that you can expand it with a __builtin_choose_expr(__builtin_constant_p()) check, similar to how we have it for MIN()/MAX().)
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40765 )
Change subject: Makefile: Add is-power-of-two ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40765/1/Makefile.inc File Makefile.inc:
https://review.coreboot.org/c/coreboot/+/40765/1/Makefile.inc@162 PS1, Line 162: 0x10000000 0x20000000 0x40000000 0x80000000
Can't you just put a _Static_assert() in a C file instead of doing this? (The IS_POWER_OF_2() we hav […]
(Actually I think __builtin_choose_expr() might not help there either, but we could still define an IS_POWER_OF_2_UNSAFE() instead for those uses.)
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40765 )
Change subject: Makefile: Add is-power-of-two ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40765/1/Makefile.inc File Makefile.inc:
https://review.coreboot.org/c/coreboot/+/40765/1/Makefile.inc@162 PS1, Line 162: 0x10000000 0x20000000 0x40000000 0x80000000
(Actually I think __builtin_choose_expr() might not help there either, but we could still define an […]
It seems like a _Static_assert would work. Power of 2 check is just ((x) & ((x) - 1)) == 0. Raul, can't we do that ?
https://review.coreboot.org/c/coreboot/+/40765/4/Makefile.inc File Makefile.inc:
https://review.coreboot.org/c/coreboot/+/40765/4/Makefile.inc@588 PS4, Line 588: APCB_EDIT_TOOL:=$(top)/util/apcb/apcb_edit.py This is unrelated?
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40765 )
Change subject: Makefile: Add is-power-of-two ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40765/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40765/4//COMMIT_MSG@10 PS4, Line 10: two at build time. This adds a `make` function `is-power-of-two`. I left a comment on CB:40808. The use-case is (indirectly) on C_ENV_BOOTBLOCK_SIZE and it can be handled by runtime alignment. And there is some motivation (on other than picasso, at least) to remove static C_ENV_BOOTBLOCK_SIZE Kconfig altogether.
Raul Rangel has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/40765 )
Change subject: Makefile: Add is-power-of-two ......................................................................
Abandoned
Not working on this
-
Aaron Durbin (Code Review) -
Julius Werner (Code Review)
-
Kyösti Mälkki (Code Review)
-
Raul Rangel (Code Review)