]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
[RISC-V] Avoid setting output object more than once in IOR/XOR synthesis
authorJeff Law <jlaw@ventanamicro.com>
Sat, 17 May 2025 13:16:50 +0000 (07:16 -0600)
committerJeff Law <jlaw@ventanamicro.com>
Sat, 17 May 2025 13:16:50 +0000 (07:16 -0600)
While evaluating Shreya's logical AND synthesis work on spec2017 I ran into a
code quality regression where combine was failing to eliminate a redundant sign
extension.

I had a hunch the problem would be with the multiple sets of the same pseudo
register in the AND synthesis path.  I was right that the problem was multiple
sets of the same pseudo, but it was actually some of the splitters in the
RISC-V backend that were the culprit.  Those multiple sets caused the sign bit
tracking code to need to make conservative assumptions thus resulting in
failure to eliminate the unnecessary sign extension.

So before we start moving on the logical AND patch we're going to do some
cleanups.

There's multiple moving parts in play.  For example, we have splitters which do
multiple sets of the output register.  Fixing some of those independently would
result in a code quality regression.  Instead they need some adjustments to or
removal of mvconst_internal.  Of course getting rid of mvconst_internal will
trigger all kinds of code quality regressions right now which ultimately lead
back to the need to revamp the logical AND expander.  Point being we've got
some circular dependencies and breaking them may result in short term code
quality regressions.  I'll obviously try to avoid those as much as possible.

So to start the process this patch adjusts the recently added XOR/IOR synthesis
to avoid re-using the destination register.  While the reuse was clearly safe
from a semantic standpoint, various parts of the compiler can do a better job
for pseudos that are only set once.

Given this synthesis path should only be active during initial RTL generation,
we can create new pseudos at will, so we create a new one for each insn.  At
the end of the sequence we copy from the last set into the final destination.

This has various trivial impacts on the code generation, but the resulting code
looks no better or worse to me across spec2017.

This has been tested in my tester and is currently bootstrapping on my BPI.
Waiting on data from the pre-commit tester before moving forward...

gcc/
* config/riscv/riscv.cc (synthesize_ior_xor): Avoid writing
operands[0] more than once, use new pseudos instead.

gcc/config/riscv/riscv.cc

index b2794252291e6e7d551d72dae2cf01e20bc5700c..8d84bee4688241875aa322d44a345d2130da4561 100644 (file)
@@ -14267,17 +14267,21 @@ synthesize_ior_xor (rtx_code code, rtx operands[3])
     {
       /* Pre-flipping bits we want to preserve.  */
       rtx input = operands[1];
+      rtx output = NULL_RTX;
       ival = ~INTVAL (operands[2]);
       while (ival)
        {
          HOST_WIDE_INT tmpval = HOST_WIDE_INT_UC (1) << ctz_hwi (ival);
          rtx x = GEN_INT (tmpval);
          x = gen_rtx_XOR (word_mode, input, x);
-         emit_insn (gen_rtx_SET (operands[0], x));
-         input = operands[0];
+         output = gen_reg_rtx (word_mode);
+         emit_insn (gen_rtx_SET (output, x));
+         input = output;
          ival &= ~tmpval;
        }
 
+      gcc_assert (output);
+
       /* Now flip all the bits, which restores the bits we were
         preserving.  */
       rtx x = gen_rtx_NOT (word_mode, input);
@@ -14300,23 +14304,29 @@ synthesize_ior_xor (rtx_code code, rtx operands[3])
       int msb = BITS_PER_WORD - 1 - clz_hwi (ival);
       if (msb - lsb + 1 <= 11)
        {
+         rtx output = gen_reg_rtx (word_mode);
+         rtx input = operands[1];
+
          /* Rotate the source right by LSB bits.  */
          rtx x = GEN_INT (lsb);
-         x = gen_rtx_ROTATERT (word_mode, operands[1], x);
-         emit_insn (gen_rtx_SET (operands[0], x));
+         x = gen_rtx_ROTATERT (word_mode, input, x);
+         emit_insn (gen_rtx_SET (output, x));
+         input = output;
 
          /* Shift the constant right by LSB bits.  */
          x = GEN_INT (ival >> lsb);
 
          /* Perform the IOR/XOR operation.  */
-         x = gen_rtx_fmt_ee (code, word_mode, operands[0], x);
-         emit_insn (gen_rtx_SET (operands[0], x));
+         x = gen_rtx_fmt_ee (code, word_mode, input, x);
+         output = gen_reg_rtx (word_mode);
+         emit_insn (gen_rtx_SET (output, x));
+         input = output;
 
          /* And rotate left to put everything back in place, we don't
             have rotate left by a constant, so use rotate right by
             an adjusted constant.  */
          x = GEN_INT (BITS_PER_WORD - lsb);
-         x = gen_rtx_ROTATERT (word_mode, operands[1], x);
+         x = gen_rtx_ROTATERT (word_mode, input, x);
          emit_insn (gen_rtx_SET (operands[0], x));
          return true;
        }
@@ -14337,22 +14347,28 @@ synthesize_ior_xor (rtx_code code, rtx operands[3])
       if ((INTVAL (operands[2]) & HOST_WIDE_INT_UC (0x7ff)) != 0
          && msb - lsb + 1 <= 11)
        {
+         rtx output = gen_reg_rtx (word_mode);
+         rtx input = operands[1];
+
          /* Rotate the source left by ROTCOUNT bits, we don't have
             rotate left by a constant, so use rotate right by an
             adjusted constant.  */
          rtx x = GEN_INT (BITS_PER_WORD - rotcount);
-         x = gen_rtx_ROTATERT (word_mode, operands[1], x);
-         emit_insn (gen_rtx_SET (operands[0], x));
+         x = gen_rtx_ROTATERT (word_mode, input, x);
+         emit_insn (gen_rtx_SET (output, x));
+         input = output;
 
          /* We've already rotated the constant.  So perform the IOR/XOR
             operation.  */
          x = GEN_INT (ival);
-         x = gen_rtx_fmt_ee (code, word_mode, operands[0], x);
-         emit_insn (gen_rtx_SET (operands[0], x));
+         x = gen_rtx_fmt_ee (code, word_mode, input, x);
+         output = gen_reg_rtx (word_mode);
+         emit_insn (gen_rtx_SET (output, x));
+         input = output;
 
          /* And rotate right to put everything into its proper place.  */
          x = GEN_INT (rotcount);
-         x = gen_rtx_ROTATERT (word_mode, operands[0], x);
+         x = gen_rtx_ROTATERT (word_mode, input, x);
          emit_insn (gen_rtx_SET (operands[0], x));
          return true;
        }
@@ -14374,6 +14390,7 @@ synthesize_ior_xor (rtx_code code, rtx operands[3])
   /* Synthesis is better than loading the constant.  */
   ival = INTVAL (operands[2]);
   rtx input = operands[1];
+  rtx output;
 
   /* Emit the [x]ori insn that sets the low 11 bits into
      the proper state.  */
@@ -14381,8 +14398,9 @@ synthesize_ior_xor (rtx_code code, rtx operands[3])
     {
       rtx x = GEN_INT (ival & HOST_WIDE_INT_UC (0x7ff));
       x = gen_rtx_fmt_ee (code, word_mode, input, x);
-      emit_insn (gen_rtx_SET (operands[0], x));
-      input = operands[0];
+      output = gen_reg_rtx (word_mode);
+      emit_insn (gen_rtx_SET (output, x));
+      input = output;
       ival &= ~HOST_WIDE_INT_UC (0x7ff);
     }
 
@@ -14396,10 +14414,12 @@ synthesize_ior_xor (rtx_code code, rtx operands[3])
       HOST_WIDE_INT tmpval = HOST_WIDE_INT_UC (1) << ctz_hwi (ival);
       rtx x = GEN_INT (tmpval);
       x = gen_rtx_fmt_ee (code, word_mode, input, x);
-      emit_insn (gen_rtx_SET (operands[0], x));
-      input = operands[0];
+      output = gen_reg_rtx (word_mode);
+      emit_insn (gen_rtx_SET (output, x));
+      input = output;
       ival &= ~tmpval;
     }
+  emit_move_insn (operands[0], output);
   return true;
 }