On 5/7/20 7:57 AM, Joseph Myers wrote: > On Thu, 7 May 2020, no-re...@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~
