Bug 98853 - [9 Regression] wrong use of bfxil at -O1
Summary: [9 Regression] wrong use of bfxil at -O1
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 11.0
: P3 normal
Target Milestone: 9.4
Assignee: Jakub Jelinek
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2021-01-27 12:29 UTC by Zdenek Sojka
Modified: 2021-04-22 13:32 UTC (History)
2 users (show)

See Also:
Host: x86_64-pc-linux-gnu
Target: aarch64-unknown-linux-gnu
Build:
Known to work:
Known to fail: 11.0
Last reconfirmed: 2021-01-27 00:00:00


Attachments
reduced testcase (211 bytes, text/plain)
2021-01-27 12:29 UTC, Zdenek Sojka
Details
gcc11-pr98853.patch (696 bytes, patch)
2021-01-27 14:31 UTC, Jakub Jelinek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zdenek Sojka 2021-01-27 12:29:23 UTC
Created attachment 50068 [details]
reduced testcase

Output:
$ aarch64-unknown-linux-gnu-gcc -O testcase.c -static
$ ./a.out 
qemu: uncaught target signal 6 (Aborted) - core dumped
Aborted

$ aarch64-unknown-linux-gnu-gcc -v
Using built-in specs.
COLLECT_GCC=/repo/gcc-trunk/binary-latest-aarch64/bin/aarch64-unknown-linux-gnu-gcc
COLLECT_LTO_WRAPPER=/repo/gcc-trunk/binary-trunk-r11-6925-20210127102218-g6cf43433750-checking-yes-rtl-df-extra-aarch64/bin/../libexec/gcc/aarch64-unknown-linux-gnu/11.0.0/lto-wrapper
Target: aarch64-unknown-linux-gnu
Configured with: /repo/gcc-trunk//configure --enable-languages=c,c++ --enable-valgrind-annotations --disable-nls --enable-checking=yes,rtl,df,extra --with-cloog --with-ppl --with-isl --with-sysroot=/usr/aarch64-unknown-linux-gnu --build=x86_64-pc-linux-gnu --host=x86_64-pc-linux-gnu --target=aarch64-unknown-linux-gnu --with-ld=/usr/bin/aarch64-unknown-linux-gnu-ld --with-as=/usr/bin/aarch64-unknown-linux-gnu-as --disable-libstdcxx-pch --prefix=/repo/gcc-trunk//binary-trunk-r11-6925-20210127102218-g6cf43433750-checking-yes-rtl-df-extra-aarch64
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 11.0.0 20210127 (experimental) (GCC)
Comment 1 Jakub Jelinek 2021-01-27 14:16:52 UTC
I admit I know next to nothing about AArch64, but the
https://gcc.gnu.org/legacy-ml/gcc-patches/2018-07/msg01895.html
patch certainly doesn't emit what it claims to, and from a brief look at the assembler guide it appears that emitting what the patch claims to shall fix it.

So I think this should be:
--- gcc/config/aarch64/aarch64.md.jj	2021-01-04 10:25:46.435147744 +0100
+++ gcc/config/aarch64/aarch64.md	2021-01-27 15:13:13.993275204 +0100
@@ -5724,10 +5724,10 @@ (define_insn "*aarch64_bfxilsi_uxtw"
     {
       case 0:
 	operands[3] = GEN_INT (ctz_hwi (~INTVAL (operands[3])));
-	return "bfxil\\t%0, %1, 0, %3";
+	return "bfxil\\t%w0, %w1, 0, %3";
       case 1:
 	operands[3] = GEN_INT (ctz_hwi (~INTVAL (operands[4])));
-	return "bfxil\\t%0, %2, 0, %3";
+	return "bfxil\\t%w0, %w2, 0, %3";
       default:
 	gcc_unreachable ();
     }
Comment 2 Jakub Jelinek 2021-01-27 14:25:09 UTC
That change has been introduced in r9-2905-g2dc09f66b3b49d821e4bd68d3c97ff51d5e080d4 , so I think we have at least latent wrong-code in 9 and 10 too.
Comment 3 Jakub Jelinek 2021-01-27 14:31:17 UTC
Created attachment 50069 [details]
gcc11-pr98853.patch

Untested fix.
Comment 4 GCC Commits 2021-01-27 19:36:32 UTC
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:55163419211c6f17e3e22c68304384eba35782a3

commit r11-6941-g55163419211c6f17e3e22c68304384eba35782a3
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Jan 27 20:35:21 2021 +0100

    aarch64: Fix up *aarch64_bfxilsi_uxtw [PR98853]
    
    The https://gcc.gnu.org/legacy-ml/gcc-patches/2018-07/msg01895.html
    patch that introduced this pattern claimed:
    Would generate:
    
    combine_balanced_int:
            bfxil   w0, w1, 0, 16
            uxtw    x0, w0
            ret
    
    But with this patch generates:
    
    combine_balanced_int:
            bfxil   w0, w1, 0, 16
            ret
    and it is indeed what it should generate, but it doesn't do that,
    it emits bfxil  x0, x1, 0, 16
    instead which doesn't zero extend from 32 to 64 bits, but preserves
    the bits from the destination register.
    
    2021-01-27  Jakub Jelinek  <jakub@redhat.com>
    
            PR target/98853
            * config/aarch64/aarch64.md (*aarch64_bfxilsi_uxtw): Use
            %w0, %w1 and %2 instead of %0, %1 and %2.
    
            * gcc.c-torture/execute/pr98853-1.c: New test.
            * gcc.c-torture/execute/pr98853-2.c: New test.
Comment 5 Jakub Jelinek 2021-01-27 19:39:03 UTC
Fixed on the trunk so far.
Comment 6 GCC Commits 2021-01-29 19:20:06 UTC
The releases/gcc-10 branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:2a2c1e22c2501457608f12d5ab560caaca59c425

commit r10-9322-g2a2c1e22c2501457608f12d5ab560caaca59c425
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Jan 27 20:35:21 2021 +0100

    aarch64: Fix up *aarch64_bfxilsi_uxtw [PR98853]
    
    The https://gcc.gnu.org/legacy-ml/gcc-patches/2018-07/msg01895.html
    patch that introduced this pattern claimed:
    Would generate:
    
    combine_balanced_int:
            bfxil   w0, w1, 0, 16
            uxtw    x0, w0
            ret
    
    But with this patch generates:
    
    combine_balanced_int:
            bfxil   w0, w1, 0, 16
            ret
    and it is indeed what it should generate, but it doesn't do that,
    it emits bfxil  x0, x1, 0, 16
    instead which doesn't zero extend from 32 to 64 bits, but preserves
    the bits from the destination register.
    
    2021-01-27  Jakub Jelinek  <jakub@redhat.com>
    
            PR target/98853
            * config/aarch64/aarch64.md (*aarch64_bfxilsi_uxtw): Use
            %w0, %w1 and %2 instead of %0, %1 and %2.
    
            * gcc.c-torture/execute/pr98853-1.c: New test.
            * gcc.c-torture/execute/pr98853-2.c: New test.
Comment 7 Jakub Jelinek 2021-01-29 19:24:45 UTC
Fixed for 10.3+ too.
Comment 8 GCC Commits 2021-04-20 23:31:40 UTC
The releases/gcc-9 branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:bde3846fb90f51b75685c9b6d677015daaec5f69

commit r9-9411-gbde3846fb90f51b75685c9b6d677015daaec5f69
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Jan 27 20:35:21 2021 +0100

    aarch64: Fix up *aarch64_bfxilsi_uxtw [PR98853]
    
    The https://gcc.gnu.org/legacy-ml/gcc-patches/2018-07/msg01895.html
    patch that introduced this pattern claimed:
    Would generate:
    
    combine_balanced_int:
            bfxil   w0, w1, 0, 16
            uxtw    x0, w0
            ret
    
    But with this patch generates:
    
    combine_balanced_int:
            bfxil   w0, w1, 0, 16
            ret
    and it is indeed what it should generate, but it doesn't do that,
    it emits bfxil  x0, x1, 0, 16
    instead which doesn't zero extend from 32 to 64 bits, but preserves
    the bits from the destination register.
    
    2021-01-27  Jakub Jelinek  <jakub@redhat.com>
    
            PR target/98853
            * config/aarch64/aarch64.md (*aarch64_bfxilsi_uxtw): Use
            %w0, %w1 and %2 instead of %0, %1 and %2.
    
            * gcc.c-torture/execute/pr98853-1.c: New test.
            * gcc.c-torture/execute/pr98853-2.c: New test.
    
    (cherry picked from commit 2a2c1e22c2501457608f12d5ab560caaca59c425)
Comment 9 Jakub Jelinek 2021-04-22 13:32:53 UTC
Fixed.