[Patchew-devel] [PATCH 0/5] target/i386: fxtract, fscale fixes
Joseph Myers
joseph at codesourcery.com
Thu May 7 14:57:51 UTC 2020
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.
--
Joseph S. Myers
joseph at codesourcery.com
More information about the Patchew-devel
mailing list