Re: [PATCH 0/5] target/i386: fxtract, fscale fixes

[Richard Henderson <rth@twiddle.net>]

On 5/7/20 7:57 AM, Joseph Myers wrote:
> On Thu, 7 May 2020, no-reply@patchew.org wrote:
> 
>> === OUTPUT BEGIN ===
>> 1/5 Checking commit 69eed0bcaaaf (target/i386: implement special cases for fxtract)
>> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> 
> I don't think any MAINTAINERS update is needed for a new testcase in an 
> existing directory.
> 
>> ERROR: Use of volatile is usually wrong, please add a comment
> 
> I think the justification for volatile in such testcase code is obvious 
> without comments in individual cases - to avoid any code movement or 
> optimization that might break what the tests are intending to test (these 
> tests are making heavy use of mixed C and inline asm to test how emulated 
> instructions behave, including on input representations that are not valid 
> long double values in the ABI and with the rounding precision changed 
> behind the compiler's back).  I think making everything possibly relevant 
> volatile in these tests is better than trying to produce a fragile 
> argument that in fact certain data does not need to be volatile to avoid 
> problematic code movement.
> 
>> ERROR: spaces required around that '-' (ctx:VxV)
>> #139: FILE: tests/tcg/i386/test-i386-fxtract.c:80:
>> +                      "0" (0x1p-16445L));
>>                                 ^
> 
> No, this is a C99 hex float contstant, not a subtraction.  There are 
> already such constants in tests/tcg/multiarch/float_helpers.c and 
> tests/tcg/multiarch/float_madds.c at least, so I assume they are OK in 
> QEMU floating-point tests and this style checker should not be objecting 
> to them.
> 

Correct, these are all false positives.


r~