Change in ...coreboot[master]: libpayload: make log2 and clz work on signed values internally
Patrick Georgi has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32184
Change subject: libpayload: make log2 and clz work on signed values internally ......................................................................
libpayload: make log2 and clz work on signed values internally
Needed to make libpayload build clean with -Wconversion.
BUG=b:111443775 BRANCH=none TEST=make junit.xml shows fewer warnings with -Wconversion enabled
Change-Id: Ie193e39854d2231b6d09a2b0deeeef2873e900ab Signed-off-by: Patrick Georgi pgeorgi@google.com --- M payloads/libpayload/include/libpayload.h 1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/32184/1
diff --git a/payloads/libpayload/include/libpayload.h b/payloads/libpayload/include/libpayload.h index 0b9ab0d..3cf6aee 100644 --- a/payloads/libpayload/include/libpayload.h +++ b/payloads/libpayload/include/libpayload.h @@ -409,9 +409,9 @@ void fatal(const char *msg) __attribute__((noreturn));
/* Count Leading Zeroes: clz(0) == 32, clz(0xf) == 28, clz(1 << 31) == 0 */ -static inline int clz(u32 x) { return x ? __builtin_clz(x) : sizeof(x) * 8; } +static inline int clz(u32 x) { return x ? __builtin_clz(x) : 32; } /* Integer binary logarithm (rounding down): log2(0) == -1, log2(5) == 2 */ -static inline int log2(u32 x) { return sizeof(x) * 8 - clz(x) - 1; } +static inline int log2(u32 x) { return (int)sizeof(x) * 8 - clz(x) - 1; } /* Find First Set: __ffs(0xf) == 0, __ffs(0) == -1, __ffs(1 << 31) == 31 */ static inline int __ffs(u32 x) { return log2(x & (u32)(-(s32)x)); } /** @} */
Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32184 )
Change subject: libpayload: make log2 and clz work on signed values internally ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/32184/1/payloads/libpayload/include/libpaylo... File payloads/libpayload/include/libpayload.h:
https://review.coreboot.org/#/c/32184/1/payloads/libpayload/include/libpaylo... PS1, Line 414: static inline int log2(u32 x) { return (int)sizeof(x) * 8 - clz(x) - 1; } This could also be simplified to 32, no? (or even 31)
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32184 )
Change subject: libpayload: make log2 and clz work on signed values internally ......................................................................
Patch Set 1: Code-Review+2
Hello Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32184
to look at the new patch set (#2).
Change subject: libpayload: make log2 and clz work on signed values internally ......................................................................
libpayload: make log2 and clz work on signed values internally
Needed to make libpayload build clean with -Wconversion.
BUG=b:111443775 BRANCH=none TEST=make junit.xml shows fewer warnings with -Wconversion enabled
Change-Id: Ie193e39854d2231b6d09a2b0deeeef2873e900ab Signed-off-by: Patrick Georgi pgeorgi@google.com --- M payloads/libpayload/include/libpayload.h 1 file changed, 4 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/32184/2
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32184 )
Change subject: libpayload: make log2 and clz work on signed values internally ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/32184/2/payloads/libpayload/include/libpaylo... File payloads/libpayload/include/libpayload.h:
https://review.coreboot.org/#/c/32184/2/payloads/libpayload/include/libpaylo... PS2, Line 437: static inline int clz(u32 x) { open brace '{' following function definitions go on the next line
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32184 )
Change subject: libpayload: make log2 and clz work on signed values internally ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/32184/1/payloads/libpayload/include/libpaylo... File payloads/libpayload/include/libpayload.h:
https://review.coreboot.org/#/c/32184/1/payloads/libpayload/include/libpaylo... PS1, Line 414: static inline int log2(u32 x) { return (int)sizeof(x) * 8 - clz(x) - 1; }
This could also be simplified to 32, no? (or even 31)
I went for the type cast in both instances instead
Hello Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32184
to look at the new patch set (#3).
Change subject: libpayload: make log2 and clz work on signed values internally ......................................................................
libpayload: make log2 and clz work on signed values internally
Needed to make libpayload build clean with -Wconversion.
BUG=b:111443775 BRANCH=none TEST=make junit.xml shows fewer warnings with -Wconversion enabled
Change-Id: Ie193e39854d2231b6d09a2b0deeeef2873e900ab Signed-off-by: Patrick Georgi pgeorgi@google.com --- M payloads/libpayload/include/libpayload.h 1 file changed, 5 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/32184/3
Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32184 )
Change subject: libpayload: make log2 and clz work on signed values internally ......................................................................
Patch Set 3: Code-Review+1
Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32184 )
Change subject: libpayload: make log2 and clz work on signed values internally ......................................................................
Patch Set 3: Code-Review+2
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32184 )
Change subject: libpayload: make log2 and clz work on signed values internally ......................................................................
libpayload: make log2 and clz work on signed values internally
Needed to make libpayload build clean with -Wconversion.
BUG=b:111443775 BRANCH=none TEST=make junit.xml shows fewer warnings with -Wconversion enabled
Change-Id: Ie193e39854d2231b6d09a2b0deeeef2873e900ab Signed-off-by: Patrick Georgi pgeorgi@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/32184 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Jacob Garber jgarber1@ualberta.ca Reviewed-by: Stefan Reinauer stefan.reinauer@coreboot.org --- M payloads/libpayload/include/libpayload.h 1 file changed, 5 insertions(+), 2 deletions(-)
Approvals: build bot (Jenkins): Verified Stefan Reinauer: Looks good to me, approved Jacob Garber: Looks good to me, but someone else must approve
diff --git a/payloads/libpayload/include/libpayload.h b/payloads/libpayload/include/libpayload.h index a578d41..57a3afc 100644 --- a/payloads/libpayload/include/libpayload.h +++ b/payloads/libpayload/include/libpayload.h @@ -434,9 +434,12 @@ void fatal(const char *msg) __attribute__((noreturn));
/* Count Leading Zeroes: clz(0) == 32, clz(0xf) == 28, clz(1 << 31) == 0 */ -static inline int clz(u32 x) { return x ? __builtin_clz(x) : sizeof(x) * 8; } +static inline int clz(u32 x) +{ + return x ? __builtin_clz(x) : (int)sizeof(x) * 8; +} /* Integer binary logarithm (rounding down): log2(0) == -1, log2(5) == 2 */ -static inline int log2(u32 x) { return sizeof(x) * 8 - clz(x) - 1; } +static inline int log2(u32 x) { return (int)sizeof(x) * 8 - clz(x) - 1; } /* Find First Set: __ffs(0xf) == 0, __ffs(0) == -1, __ffs(1 << 31) == 31 */ static inline int __ffs(u32 x) { return log2(x & (u32)(-(s32)x)); } /** @} */
-
build bot (Jenkins) (Code Review) -
Jacob Garber (Code Review)
-
Patrick Georgi (Code Review)
-
Patrick Rudolph (Code Review)
-
Stefan Reinauer (Code Review)