mbox series

[0/1] target-ppc: fix rlwimi, rlwinm, rlwnm for Clang-9

Message ID 20200501190913.25008-1-dbuono@linux.vnet.ibm.com (mailing list archive)
Headers show
Series target-ppc: fix rlwimi, rlwinm, rlwnm for Clang-9 | expand

Message

Daniele Buono May 1, 2020, 7:09 p.m. UTC
Until Clang v8, -Wtype-limits was kept for GCC compatibility but had no
effect. With Clang v9, the flag is now implemented ( see 
https://tinyurl.com/clang8-manual vs https://tinyurl.com/clang9-manual )

Starting with Clang v9, compiling with -Wtype-limits (default for QEMU)
triggers the following errors on the ppc-softmmu and ppc-linux-user
targets:
   
/root/qemu/target/ppc/translate.c:1894:18: error: result of comparison
'target_ulong' (aka 'unsigned int') <= 4294967295 is always true
[-Werror,-Wtautological-type-limit-compare]
        if (mask <= 0xffffffffu) {
            ~~~~ ^  ~~~~~~~~~~~
/root/qemu/target/ppc/translate.c:1941:18: error: result of comparison
'target_ulong' (aka 'unsigned int') <= 4294967295 is always true
[-Werror,-Wtautological-type-limit-compare]
        if (mask <= 0xffffffffu) {
            ~~~~ ^  ~~~~~~~~~~~
/root/qemu/target/ppc/translate.c:1983:14: error: result of comparison
'target_ulong' (aka 'unsigned int') <= 4294967295 is always true
[-Werror,-Wtautological-type-limit-compare]
    if (mask <= 0xffffffffu) {
        ~~~~ ^  ~~~~~~~~~~~

The same error can be triggered with this small repro:
int main() {
    unsigned int x = 1;
    if ( x <= 0xffffffffu) return 1;
    return 0;
}

$ gcc test.c -Wtype-limits
[nothing]
$ clang-9 test.c -Wtype-limits
test.c:3:12: warning: result of comparison 'unsigned int' <= 4294967295 
is always true [-Wtautological-type-limit-compare]
    if ( x <= 0xffffffffu) 
         ~ ^  ~~~~~~~~~~~
1 warning generated.
$ clang-9 test.c -Wtype-limits -Wno-tautological-type-limit-compare
[nothing]

We could get away with the compilation error by adding the flag
"-Wno-tautological-type-limit-compare", but I think we should avoid that
to make sure the checks are applied to the rest of the code and warn us
for logical errors in the future.

Looking at the code, the comparison is only needed for PPC64, since
the else branch in PPC32 only has a "g_assert_not_reached();"
This patch restructures the code so that PPC32 always runs the "true"
branch.

I tried to keep the changes to a minimum, to make sure to not affect the
semantics of the instructions. However, considering the target
architecture, my testing has been limited. check-acceptance seems to be
able to properly start a few linux kernels succesfully, and make check
didn't complain on both ppc-softmmu and ppc64-softmmu.

Daniele Buono (1):
  target-ppc: fix rlwimi, rlwinm, rlwnm for Clang-9

 target/ppc/translate.c | 34 +++++++++++++++++++---------------
 1 file changed, 19 insertions(+), 15 deletions(-)