public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r13-1716] PR target/106273: Add earlyclobber to *andn<dwi>3_doubleword_bmi on x86_64.
@ 2022-07-15 21:50 Roger Sayle
  0 siblings, 0 replies; only message in thread
From: Roger Sayle @ 2022-07-15 21:50 UTC (permalink / raw)
  To: gcc-cvs

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

commit r13-1716-gfd3d25d6df1cbd385d2834ff3059dfb6905dd75c
Author: Roger Sayle <roger@nextmovesoftware.com>
Date:   Fri Jul 15 22:48:56 2022 +0100

    PR target/106273: Add earlyclobber to *andn<dwi>3_doubleword_bmi on x86_64.
    
    This patch resolves PR target/106273 which is a wrong code regression
    caused by the recent reorganization to split doubleword operations after
    reload on x86.  For the failing test case, the constraints on the
    andnti3_doubleword_bmi pattern allow reload to allocate the output and
    operand in overlapping but non-identical registers, i.e.
    
    (insn 45 44 66 2 (parallel [
                (set (reg/v:TI 5 di [orig:96 i ] [96])
                    (and:TI (not:TI (reg:TI 39 r11 [orig:83 _2 ] [83]))
                        (reg/v:TI 4 si [orig:100 i ] [100])))
                (clobber (reg:CC 17 flags))
            ]) "pr106273.c":13:5 562 {*andnti3_doubleword_bmi}
    
    where the output is in registers 5 and 6, and the second operand is
    registers 4 and 5, which then leads to the incorrect split:
    
    (insn 113 44 114 2 (parallel [
                (set (reg:DI 5 di [orig:96 i ] [96])
                    (and:DI (not:DI (reg:DI 39 r11 [orig:83 _2 ] [83]))
                        (reg:DI 4 si [orig:100 i ] [100])))
                (clobber (reg:CC 17 flags))
            ]) "pr106273.c":13:5 566 {*andndi_1}
    
    (insn 114 113 66 2 (parallel [
                (set (reg:DI 6 bp [ i+8 ])
                    (and:DI (not:DI (reg:DI 40 r12 [ _2+8 ]))
                        (reg:DI 5 di [ i+8 ])))
                (clobber (reg:CC 17 flags))
            ]) "pr106273.c":13:5 566 {*andndi_1}
    
    [Notice that reg:DI 5 is set in the first instruction, but assumed
    to have its original value in the second].  My first thought was
    that this could be fixed by swapping the order of the split instructions
    (which works in this case), but in the general case, it's impossible
    to handle (set (reg:TI x) (op (reg:TI x+1) (reg:TI x-1)).  Hence for
    correctness this pattern needs an earlyclobber "=&r", but we can also
    allow cases where the output is the same as one of the operands (using
    constraint "0").  The other binary logic operations (AND, IOR, XOR)
    are unaffected as they constrain the output to match the first
    operand, but BMI's andn is a three-operand instruction which can
    lead to the overlapping cases described above.
    
    2022-07-15  Roger Sayle  <roger@nextmovesoftware.com>
    
    gcc/ChangeLog
            PR target/106273
            * config/i386/i386.md (*andn<dwi>3_doubleword_bmi): Update the
            constraints to reflect the output is earlyclobber, unless it is
            the same register (pair) as one of the operands.
    
    gcc/testsuite/ChangeLog
            PR target/106273
            * gcc.target/i386/pr106273.c: New test case.

Diff:
---
 gcc/config/i386/i386.md                  |  6 +++---
 gcc/testsuite/gcc.target/i386/pr106273.c | 27 +++++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index bf29f444382..31637bd755f 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -10423,10 +10423,10 @@
 })
 
 (define_insn_and_split "*andn<dwi>3_doubleword_bmi"
-  [(set (match_operand:<DWI> 0 "register_operand" "=r")
+  [(set (match_operand:<DWI> 0 "register_operand" "=&r,r,r")
 	(and:<DWI>
-	  (not:<DWI> (match_operand:<DWI> 1 "register_operand" "r"))
-	  (match_operand:<DWI> 2 "nonimmediate_operand" "ro")))
+	  (not:<DWI> (match_operand:<DWI> 1 "register_operand" "r,0,r"))
+	  (match_operand:<DWI> 2 "nonimmediate_operand" "ro,ro,0")))
    (clobber (reg:CC FLAGS_REG))]
   "TARGET_BMI"
   "#"
diff --git a/gcc/testsuite/gcc.target/i386/pr106273.c b/gcc/testsuite/gcc.target/i386/pr106273.c
new file mode 100644
index 00000000000..0f4221340c7
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr106273.c
@@ -0,0 +1,27 @@
+/* { dg-do compile { target int128 } } */
+/* { dg-options "-Og -march=cascadelake" } */
+typedef unsigned char u8;
+typedef unsigned short u16;
+typedef unsigned long long u64;
+
+u8 g;
+
+void
+foo (__int128 i, u8 *r)
+{
+  u16 a = __builtin_sub_overflow_p (0, i * g, 0);
+  i ^= g & i;
+  u64 s = (i >> 64) + i;
+  *r = ((union { u16 a; u8 b[2]; }) a).b[1] + s;
+}
+
+int
+bar (void)
+{
+  u8 x;
+  foo (5, &x);
+  if (x != 5)
+    __builtin_abort ();
+  return 0;
+}
+/* { dg-final { scan-assembler-not "andn\[ \\t\]+%rdi, %r11, %rdi" } } */


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2022-07-15 21:50 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-15 21:50 [gcc r13-1716] PR target/106273: Add earlyclobber to *andn<dwi>3_doubleword_bmi on x86_64 Roger Sayle

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).