Messages in this thread Patch in this message |  | | Date | Sat, 18 Jan 2025 13:59:36 -0800 | | Subject | Re: Buiild error in i915/xe | | From | Guenter Roeck <> |
| |
On 1/18/25 13:21, Linus Torvalds wrote: > On Sat, 18 Jan 2025 at 09:49, Guenter Roeck <linux@roeck-us.net> wrote: >> >> No idea why the compiler would know that the values are invalid. > > It's not that the compiler knows tat they are invalid, but I bet what > happens is in scale() (and possibly other places that do similar > checks), which does this: > > WARN_ON(source_min > source_max); > ... > source_val = clamp(source_val, source_min, source_max); > > and the compiler notices that the ordering comparison in the first > WARN_ON() is the same as the one in clamp(), so it basically converts > the logic to > > if (source_min > source_max) { > WARN(..); > /* Do the clamp() knowing that source_min > source_max */ > source_val = clamp(source_val, source_min, source_max); > } else { > /* Do the clamp knowing that source_min <= source_max */ > source_val = clamp(source_val, source_min, source_max); > } > > (obviously I dropped the other WARN_ON in the conversion, it wasn't > relevant for this case). > > And now that first clamp() case is done with source_min > source_max, > and it triggers that build error because that's invalid. > > So the condition is not statically true in the *source* code, but in > the "I have moved code around to combine tests" case it now *is* > statically true as far as the compiler is concerned. >
Yes, turns out I can reproduce the problem by adding WARN_ON() ahead of similar clamp() calls (see below). However, I can only reproduce it with gcc 13.3 for parisc. I don't see the problem with other cross compilers (I tried arm, powerpc, and loongarch64). Compilers are weird :-(.
I am not sure what to do here. That kind of problem seems difficult to avoid, and I am sure we will hit it again elsewhere. Should I declare gcc 13.x off limits for parisc builds ?
Guenter
--- diff --git a/drivers/input/mousedev.c b/drivers/input/mousedev.c index 505c562a5daa..71c0da31a9d2 100644 --- a/drivers/input/mousedev.c +++ b/drivers/input/mousedev.c @@ -179,6 +179,7 @@ static void mousedev_abs_event(struct input_dev *dev, struct mousedev *mousedev, if (size == 0) size = xres ? : 1;
+ WARN_ON(min > max); value = clamp(value, min, max);
mousedev->packet.x = ((value - min) * xres) / size;
|  |