Buiild error in i915/xe

[Guenter Roeck]

On 1/18/25 13:18, David Laight wrote:
> On Sat, 18 Jan 2025 10:36:11 -0800
> Guenter Roeck <linux at roeck-us.net> wrote:
> 
>> On 1/18/25 10:09, David Laight wrote:
>>> On Sat, 18 Jan 2025 09:49:21 -0800
>>> Guenter Roeck <linux at roeck-us.net> wrote:
>>>    
>>>> On Sat, Jan 18, 2025 at 05:09:59PM +0000, David Laight wrote:
>>>>> On Sat, 18 Jan 2025 08:13:06 -0800
>>>>> Guenter Roeck <linux at roeck-us.net> wrote:
>>>>>       
>>>>>> Hi,
>>>>>>
>>>>>> On Mon, Nov 18, 2024 at 07:13:31PM +0000, David Laight wrote:
>>>>>>> Use BUILD_BUG_ON_MSG(statically_true(ulo > uhi), ...) for the sanity
>>>>>>> check of the bounds in clamp().
>>>>>>> Gives better error coverage and one less expansion of the arguments.
>>>>>>>
>>>>>>> Signed-off-by: David Laight <david.laight at aculab.com>
>>>>>>
>>>>>> This patch triggers a build error when trying to build parisc:allmodconfig.
>>>>>> See error message and bisect log below.
>>>>>>
>>>>>> I don't think there is anything wrong with the patch. The underlying
>>>>>> problem seems to be that parisc:allmodconfig enables CONFIG_DRM_XE which
>>>>>> tries to build the affected file even though CONFIG_DRM_I915 is not
>>>>>> enabled/supported on parisc.
>>>>>
>>>>> This has appeared before.
>>>>> Any idea which inlined copy of scale() is causing the problem.
>>>>> On the face of it they all look ok.
>>>>>
>>>>> If you can reproduce it maybe try commenting out some of the calls.
>>>>>       
>>>>
>>>> See diff below. All three changes are needed.
>>>> No idea why the compiler would know that the values are invalid.
>>>
>>> Maybe it isn't even an inlining issue.
>>> Perhaps that compiler just doesn't like the function ?
>>> What happens without the 'static' (and an extra prototype)?
>>>    
>>
>>
>> You mean like that ?
>>
>> -static u32 scale(u32 source_val,
>> +
>> +u32 scale(u32 source_val,
>> +                u32 source_min, u32 source_max,
>> +                u32 target_min, u32 target_max);
>> +
>> +u32 scale(u32 source_val,
>>                    u32 source_min, u32 source_max,
>>                    u32 target_min, u32 target_max)
>>
>> It doesn't help. Worse, after that change the error is still reported
>> even with the #if 0 elsewhere.
> 
> Yes - that means the compiler is 'objecting' to the scale() function itself.
> (Without any regard for its callers.)
> Which should make it easy to reproduce outside the kernel build.
> 
> I think Mat had a successful build with a different (older?) version of gcc for
> parisc.
> 
> There must be something odd causing the problem - there will be other clamp()
> calls in the build that don't generate the error.
> 
> Remember that lack of the error messages requires the compiler optimise away
> some code - so if the optimisation is skipped the call could be generated
> and the warning output (even if the call is optimised away later).
> 
> Perhaps there is some obscure interaction with the WARN() statements?
> 
> I don't have the required compiler (neither does godbolt).
> 

Oh man - that was a good hint. Turns out I can only reproduce the problem with
gcc 13.2 and 13.3. gcc 10.3, 11.4, and 12.4 do not generate the error.

And, yes, I can "fix" the problem with

-       WARN_ON(source_min > source_max);
+       // WARN_ON(source_min > source_max);

Any idea what to do ? Should I just scrap builds with gcc 13.x ?

Thanks,
Guenter