On Fri, Dec 6, 2024 at 3:20 AM David Laight David.Laight@aculab.com wrote:
From: Naresh Kamboju
Sent: 05 December 2024 18:42
On Thu, 5 Dec 2024 at 20:46, Dan Carpenter dan.carpenter@linaro.org wrote:
Add David to the CC list.
Anders bisected this reported issue and found the first bad commit as,
# first bad commit: [ef32b92ac605ba1b7692827330b9c60259f0af49] minmax.h: use BUILD_BUG_ON_MSG() for the lo < hi test in clamp()
That 'just' changed the test to use __builtin_constant_p() and thus gets checked after the optimiser has run.
I can paraphrase the code as: unsigned int fn(unsigned int x) { return clamp(10, 5, x == 0 ? 0 : x - 1); } which is never actually called with x <= 5. The compiler converts it to: return x < 0 ? clamp(10, 5, 0) : clamp(10, 5, x); (Probably because it can see that clamp(10, 5, 0) is constant.) And then the compile-time sanity check in clamp() fires.
The order of the arguments to clamp is just wrong!
David
The build is still failing with today's next, should the offending commit be reverted?
Bartosz
On Wed, Dec 11, 2024 at 01:46:11PM +0100, Bartosz Golaszewski wrote:
On Fri, Dec 6, 2024 at 3:20 AM David Laight David.Laight@aculab.com wrote:
From: Naresh Kamboju
Sent: 05 December 2024 18:42
On Thu, 5 Dec 2024 at 20:46, Dan Carpenter dan.carpenter@linaro.org wrote:
Add David to the CC list.
Anders bisected this reported issue and found the first bad commit as,
# first bad commit: [ef32b92ac605ba1b7692827330b9c60259f0af49] minmax.h: use BUILD_BUG_ON_MSG() for the lo < hi test in clamp()
That 'just' changed the test to use __builtin_constant_p() and thus gets checked after the optimiser has run.
I can paraphrase the code as: unsigned int fn(unsigned int x) { return clamp(10, 5, x == 0 ? 0 : x - 1); } which is never actually called with x <= 5. The compiler converts it to: return x < 0 ? clamp(10, 5, 0) : clamp(10, 5, x); (Probably because it can see that clamp(10, 5, 0) is constant.) And then the compile-time sanity check in clamp() fires.
The order of the arguments to clamp is just wrong!
David
The build is still failing with today's next, should the offending commit be reverted?
It's a simple fix. I've sent a patch.
regards, dan carpenter