[PATCH] D100769: [RISCV] Optimize addition with immediate

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.td:1298
+let Predicates = [IsRV64] in
+def : Pat<(sext_inreg (add GPR:$rs1, (AddiPair GPR:$rs2)), i32),
+          (ADDIW (ADDIW GPR:$rs1, (AddiPairImmB GPR:$rs2)),
----------------
benshi001 wrote:
> benshi001 wrote:
> > craig.topper wrote:
> > > craig.topper wrote:
> > > > Do we need to make sure sext_inreg is the only user of the add? Otherwise we’ll emit two ADDIWs and two ADDIs.
> > > I think if we emitted ADDI followed by ADDIW for sign_ext case, the first ADDI would CSE with the first ADDI from a non-sext case if there were multiple uses. Then we wouldn't need to check for multiple uses.
> > I do not quite understand your concern, why do you think "Otherwise we’ll emit two ADDIWs and two ADDIs." ? That is impossible. 
> > 
> > 1. for the IR "%a = add i32 %b, 3001", two ADDIs are emitted on both rv32 and rv64.
> > 
> > 2. for the IR "%a = add i64 %b, 3001", two ADDIs are emitted on rv64.
> > 
> > 3. for the IR "%a = add i64 %b, 3001", two ADDIs are emitted on rv32 for the lower 32-bit, (along with other instrs for the upper 32-bit).
> > 
> > 4. for the IR pattern "%a = add i32 %b, 3001; %c = sext_inreg %a, i32", two ADDIWs are emitted without any ADDI.
> > 
> > I did not find any other cases/IR patterns for this optimization.
> In current patch, there is no possibility that ADDI/ADDIW are mixedly emmitted. I can remove the one-use check, but I do concern the immediate composed by a lui/addi pair has further use, and my transform leads to less efficient code.
Example test

```
define signext i32 @add32_sext_accept(i32 signext %a, i32* %b) nounwind {                                                                                                                      
  %1 = add i32 %a, 2999                                                                                                                                                                        
  store i32 %1, i32* %b                                                                                                                                                                        
  ret i32 %1                                                                                                                                                                                   
}    
``` 

Produces 
```
        addi    a2, a0, 1500
        addi    a2, a2, 1499
        addiw   a0, a0, 1500
        addiw   a0, a0, 1499
        sw      a2, 0(a1)
        ret
```

Though for that example we could use the addiw result for the sw, but that's a bit hard to fix at the moment.

Here's another example

```
define i64 @add32_sext_accept(i64 %a, i64* %b) nounwind {                                                                                                                                      
  %1 = add i64 %a, 2999                                                                                                                                                                        
  store i64 %1, i64* %b                                                                                                                                                                        
  %2 = shl i64 %1, 32                                                                                                                                                                          
  %3 = ashr i64 %2, 32                                                                                                                                                                         
  ret i64 %3                                                                                                                                                                                   
}    
```

produces
```
        addi    a2, a0, 1500
        addi    a2, a2, 1499
        addiw   a0, a0, 1500
        addiw   a0, a0, 1499
        sd      a2, 0(a1)
        ret
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100769/new/

https://reviews.llvm.org/D100769