[Patchew-devel] [PATCH 0/5] target/i386: fxtract, fscale fixes
Richard Henderson
rth at twiddle.net
Fri May 8 03:42:57 UTC 2020
On 5/7/20 7:57 AM, Joseph Myers wrote:
> On Thu, 7 May 2020, no-reply at 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~
More information about the Patchew-devel
mailing list