[RISCV] Fix wrong register rename for store value during make-compressible optimization

[kito-cheng]

Summary

Current implementation will rename both register in store instructions if
we store base address into memory with same base register, it's OK if
the offset is 0, however that is wrong transform if offset isn't 0, give
a smalle example here:

sd a0, 808(a0)

We should not transform into:

addi a2, a0, 768
sd a2, 40(a2)

That should just rename base address like this:

addi a2, a0, 768
sd a0, 40(a2)

[craig.topper]

llvm/lib/Target/RISCV/RISCVMakeCompressible.cpp

105

Methods name start with lower case.

Is this ever called?

[kito-cheng]

Changes:

• Rename Incompressible -> incompressible
• Use incompressible function to check it's incompressible or not.
• Use const ref if possible for CompressibleInfo

[kito-cheng]

llvm/lib/Target/RISCV/RISCVMakeCompressible.cpp

105

Use CI.incompressible() instead of !RegImm.Reg && RegImm.Imm == 0 now

[kito-cheng]

Changes:

• Less invasive way to fix this issue.

[asb]

LGTM, though see suggestion about slightly more detailed comment.

llvm/lib/Target/RISCV/RISCVMakeCompressible.cpp

298

This is a bit subtle, but easy to understand when spelled out so I'd be tempted to write a slightly more in-depth comment. e.g.:

Perhaps "Skip the first (value) operand to a store instruction (except if the store offset is zero) in order to avoid an incorrect transformation. e.g. sd a0, 808(a0) to addi a2, a0, 768; sd a2, 40(a2)"

llvm/lib/Target/RISCV/RISCVMakeCompressible.cpp

INITIALIZE_PASS(RISCVMakeCompressibleOpt, "riscv-make-compressible",
                 RISCV_COMPRESS_INSTRS_NAME, false, false)
 
 // Return log2(widthInBytes) of load/store done by Opcode.
 static unsigned log2LdstWidth(unsigned Opcode) {
   switch (Opcode) {
   default:
     llvm_unreachable("Unexpected opcode");
   case RISCV::LW:
   case RISCV::SW:
   case RISCV::FLW:
   case RISCV::FSW:
     return 2;
   case RISCV::LD:
   case RISCV::SD:
   case RISCV::FLD:
   case RISCV::FSD:
     return 3;
static void updateOperands(MachineInstr &MI, RegImmPair OldRegImm,
                            Register NewReg) {
   unsigned Opcode = MI.getOpcode();
 
   // If this pass is extended to support more instructions, the check for
   // definedness may need to be strengthened.
   assert((isCompressibleLoad(MI) || isCompressibleStore(MI)) &&
          "Unsupported instruction for this optimization.");
 
+  int SkipN = 0;
+
+  // Skip the first (value) operand to a store instruction (except if the store
+  // offset is zero) in order to avoid an incorrect transformation.
+  // e.g. sd a0, 808(a0) to addi a2, a0, 768; sd a2, 40(a2)
+  if (isCompressibleStore(MI) && OldRegImm.Imm != 0)
+    SkipN = 1;
+
   // Update registers
-  for (MachineOperand &MO : MI.operands())
+  for (MachineOperand &MO : drop_begin(MI.operands(), SkipN))
     if (MO.isReg() && MO.getReg() == OldRegImm.Reg) {
       // Do not update operands that define the old register.
       //
       // The new register was scavenged for the range of instructions that are
       // being updated, therefore it should not be defined within this range
       // except possibly in the final instruction.
       if (MO.isDef()) {
         assert(isCompressibleLoad(MI));

llvm/test/CodeGen/RISCV/make-compressible-for-store-address.mir

bb.0.entry:
     liveins: $x10
 
     ; CHECK-LABEL: name: foo
     ; CHECK: liveins: $x10
     ; CHECK-NEXT: {{  $}}
     ; CHECK-NEXT: renamable $x11 = ADDI $x0, 1
     ; CHECK-NEXT: $x12 = ADDI $x10, 768
     ; CHECK-NEXT: SD killed renamable $x11, $x12, 32 :: (store (s64) into %ir.1)
-    ; CHECK-NEXT: SD $x12, $x12, 40 :: (store (s64) into %ir.2)
+    ; CHECK-NEXT: SD renamable $x10, $x12, 40 :: (store (s64) into %ir.2)
     ; CHECK-NEXT: renamable $x11 = ADDI $x0, 2
     ; CHECK-NEXT: SD killed renamable $x11, killed $x12, 48 :: (store (s64) into %ir.3)
     ; CHECK-NEXT: PseudoRET
     renamable $x11 = ADDI $x0, 1
     SD killed renamable $x11, renamable $x10, 800 :: (store (s64) into %ir.1)
     SD renamable $x10, renamable $x10, 808 :: (store (s64) into %ir.2)
     renamable $x11 = ADDI $x0, 2
     SD killed renamable $x11, killed renamable $x10, 816 :: (store (s64) into %ir.3)
     PseudoRET
 
 ...