[RISCV] Add an ANDI to shift amount of FSL/FSR instructions

[craig.topper]

Summary

The fshl and fshr intrinsics are defined to modulo their shift amount by the bitwidth of one of their inputs. The FSR/FSL instructions read one extra bit from the shift amount. If that bit is set the inputs are swapped. In order to preserve the semantics of the llvm intrinsics we need to make sure that the extra bit isn't set. DAG combine or instcombine may have removed any mask that was originally present.

We could be smarter here and try to use computeKnownBits to check if the bit is known zero, but wanted to start with correctness.

[frasercrmck]

LGTM.

Out of curiosity, can you use anything like computeKnownBits in TableGen patterns or would we have to do something in C++?

[craig.topper]

In D90905#2390924, @frasercrmck wrote:

LGTM.

Out of curiosity, can you use anything like computeKnownBits in TableGen patterns or would we have to do something in C++?

It can be called from the code part of a PatFrag. I think X86 does in a few places.

llvm/lib/Target/RISCV/RISCVInstrInfoB.td

 def : Pat<(bitreverse GPR:$rs1), (GREVI GPR:$rs1, (i64 63))>;
 } // Predicates = [HasStdExtZbp, IsRV64]
 
 let Predicates = [HasStdExtZbt] in {
 def : Pat<(or (and (not GPR:$rs2), GPR:$rs3), (and GPR:$rs2, GPR:$rs1)),
           (CMIX GPR:$rs1, GPR:$rs2, GPR:$rs3)>;
 def : Pat<(riscv_selectcc GPR:$rs2, (XLenVT 0), (XLenVT 17), GPR:$rs3, GPR:$rs1),
           (CMOV GPR:$rs1, GPR:$rs2, GPR:$rs3)>;
+} // Predicates = [HasStdExtZbt]
 
 // fshl and fshr concatenate their operands in the same order. fsr and fsl
 // instruction use different orders. fshl will return its first operand for
 // shift of zero, fshr will return its second operand. fsl and fsr both return
 // $rs1 so the patterns need to have different operand orders.
+//
+// fshl and fshr only read the lower log2(xlen) bits of the shift amount, but
+// fsl/fsr instructions read log2(xlen)+1 bits. DAG combine may have removed
+// an AND mask on the shift amount that we need to add back to avoid a one in
+// the extra bit.
+// FIXME: If we can prove that the extra bit in the shift amount is zero, we
+// don't need this mask.
+let Predicates = [HasStdExtZbt, IsRV32] in {
+def : Pat<(fshl GPR:$rs1, GPR:$rs3, GPR:$rs2),
+          (FSL GPR:$rs1, (ANDI GPR:$rs2, 31), GPR:$rs3)>;
+def : Pat<(fshr GPR:$rs3, GPR:$rs1, GPR:$rs2),
+          (FSR GPR:$rs1, (ANDI GPR:$rs2, 31), GPR:$rs3)>;
+}
+let Predicates = [HasStdExtZbt, IsRV64] in {
 def : Pat<(fshl GPR:$rs1, GPR:$rs3, GPR:$rs2),
-          (FSL GPR:$rs1, GPR:$rs2, GPR:$rs3)>;
+          (FSL GPR:$rs1, (ANDI GPR:$rs2, 63), GPR:$rs3)>;
 def : Pat<(fshr GPR:$rs3, GPR:$rs1, GPR:$rs2),
-          (FSR GPR:$rs1, GPR:$rs2, GPR:$rs3)>;
+          (FSR GPR:$rs1, (ANDI GPR:$rs2, 63), GPR:$rs3)>;
+}
+let Predicates = [HasStdExtZbt] in {
 def : Pat<(fshr GPR:$rs3, GPR:$rs1, uimmlog2xlen:$shamt),
           (FSRI GPR:$rs1, GPR:$rs3, uimmlog2xlen:$shamt)>;
 // We can use FSRI for fshl by immediate if we subtract the immediate from
 // XLen and swap the operands.
 def : Pat<(fshl GPR:$rs3, GPR:$rs1, uimmlog2xlen:$shamt),
           (FSRI GPR:$rs1, GPR:$rs3, (ImmROTL2R uimmlog2xlen:$shamt))>;
 } // Predicates = [HasStdExtZbt]
 

 def : Pat<(sra (bswap GPR:$rs1), (i64 32)), (GREVIW GPR:$rs1, (i64 24))>;
 def : Pat<(sra (bitreverse GPR:$rs1), (i64 32)), (GREVIW GPR:$rs1, (i64 31))>;
 } // Predicates = [HasStdExtZbp, IsRV64]
 
 let Predicates = [HasStdExtZbt, IsRV64] in {
 def : Pat<(sext_inreg (fshl GPR:$rs1, (shl GPR:$rs3, (i64 32)),
                             (and GPR:$rs2, (i64 31))),
                       i32),
-          (FSLW GPR:$rs1, GPR:$rs2, GPR:$rs3)>;
+          (FSLW GPR:$rs1, (ANDI GPR:$rs2, 31), GPR:$rs3)>;
 def : Pat<(sext_inreg (fshr GPR:$rs3, (shl GPR:$rs1, (i64 32)),
                             (or GPR:$rs2, (i64 32))),
                       i32),
-          (FSRW GPR:$rs1, GPR:$rs2, GPR:$rs3)>;
+          (FSRW GPR:$rs1, (ANDI GPR:$rs2, 31), GPR:$rs3)>;
 def : Pat<(sext_inreg (fshr GPR:$rs3, (shl GPR:$rs1, (i64 32)),
                             uimm6gt32:$shamt),
                       i32),
           (FSRIW GPR:$rs1, GPR:$rs3, (ImmSub32 uimm6gt32:$shamt))>;
 def : Pat<(sext_inreg (fshl GPR:$rs3, (shl GPR:$rs1, (i64 32)),
                             uimm5:$shamt),
                       i32),
           (FSRIW GPR:$rs1, GPR:$rs3, (ImmROTL2RW uimm5:$shamt))>;

llvm/test/CodeGen/RISCV/rv32Zbt.ll

 ; RV32I-NEXT:    not a2, a2
 ; RV32I-NEXT:    srli a1, a1, 1
 ; RV32I-NEXT:    srl a1, a1, a2
 ; RV32I-NEXT:    or a0, a0, a1
 ; RV32I-NEXT:    ret
 ;
 ; RV32IB-LABEL: fshl_i32:
 ; RV32IB:       # %bb.0:
+; RV32IB-NEXT:    andi a2, a2, 31
 ; RV32IB-NEXT:    fsl a0, a0, a1, a2
 ; RV32IB-NEXT:    ret
 ;
 ; RV32IBT-LABEL: fshl_i32:
 ; RV32IBT:       # %bb.0:
+; RV32IBT-NEXT:    andi a2, a2, 31
 ; RV32IBT-NEXT:    fsl a0, a0, a1, a2
 ; RV32IBT-NEXT:    ret
   %1 = tail call i32 @llvm.fshl.i32(i32 %a, i32 %b, i32 %c)
   ret i32 %1
 }
 
 ; As we are not matching directly i64 code patterns on RV32 some i64 patterns
 ; don't have yet an efficient pattern-matching with bit manipulation

 ; RV32I-NEXT:    not a2, a2
 ; RV32I-NEXT:    slli a0, a0, 1
 ; RV32I-NEXT:    sll a0, a0, a2
 ; RV32I-NEXT:    or a0, a0, a1
 ; RV32I-NEXT:    ret
 ;
 ; RV32IB-LABEL: fshr_i32:
 ; RV32IB:       # %bb.0:
+; RV32IB-NEXT:    andi a2, a2, 31
 ; RV32IB-NEXT:    fsr a0, a1, a0, a2
 ; RV32IB-NEXT:    ret
 ;
 ; RV32IBT-LABEL: fshr_i32:
 ; RV32IBT:       # %bb.0:
+; RV32IBT-NEXT:    andi a2, a2, 31
 ; RV32IBT-NEXT:    fsr a0, a1, a0, a2
 ; RV32IBT-NEXT:    ret
   %1 = tail call i32 @llvm.fshr.i32(i32 %a, i32 %b, i32 %c)
   ret i32 %1
 }
 
 ; As we are not matching directly i64 code patterns on RV32 some i64 patterns
 ; don't have yet an efficient pattern-matching with bit manipulation

llvm/test/CodeGen/RISCV/rv64Zbt.ll

 ; RV64I-NEXT:    or a0, a0, a1
 ; RV64I-NEXT:    andi a1, a2, 31
 ; RV64I-NEXT:    sll a0, a0, a1
 ; RV64I-NEXT:    srai a0, a0, 32
 ; RV64I-NEXT:    ret
 ;
 ; RV64IB-LABEL: fshl_i32:
 ; RV64IB:       # %bb.0:
+; RV64IB-NEXT:    andi a2, a2, 31
 ; RV64IB-NEXT:    fslw a0, a0, a1, a2
 ; RV64IB-NEXT:    ret
 ;
 ; RV64IBT-LABEL: fshl_i32:
 ; RV64IBT:       # %bb.0:
+; RV64IBT-NEXT:    andi a2, a2, 31
 ; RV64IBT-NEXT:    fslw a0, a0, a1, a2
 ; RV64IBT-NEXT:    ret
   %1 = tail call i32 @llvm.fshl.i32(i32 %a, i32 %b, i32 %c)
   ret i32 %1
 }
 
 declare i64 @llvm.fshl.i64(i64, i64, i64)
 
 define i64 @fshl_i64(i64 %a, i64 %b, i64 %c) nounwind {
 ; RV64I-LABEL: fshl_i64:
 ; RV64I:       # %bb.0:
 ; RV64I-NEXT:    sll a0, a0, a2
 ; RV64I-NEXT:    not a2, a2
 ; RV64I-NEXT:    srli a1, a1, 1
 ; RV64I-NEXT:    srl a1, a1, a2
 ; RV64I-NEXT:    or a0, a0, a1
 ; RV64I-NEXT:    ret
 ;
 ; RV64IB-LABEL: fshl_i64:
 ; RV64IB:       # %bb.0:
+; RV64IB-NEXT:    andi a2, a2, 63
 ; RV64IB-NEXT:    fsl a0, a0, a1, a2
 ; RV64IB-NEXT:    ret
 ;
 ; RV64IBT-LABEL: fshl_i64:
 ; RV64IBT:       # %bb.0:
+; RV64IBT-NEXT:    andi a2, a2, 63
 ; RV64IBT-NEXT:    fsl a0, a0, a1, a2
 ; RV64IBT-NEXT:    ret
   %1 = tail call i64 @llvm.fshl.i64(i64 %a, i64 %b, i64 %c)
   ret i64 %1
 }
 
 declare i32 @llvm.fshr.i32(i32, i32, i32)
 
 define signext i32 @fshr_i32(i32 signext %a, i32 signext %b, i32 signext %c) nounwind {
 ; RV64I-LABEL: fshr_i32:
 ; RV64I:       # %bb.0:
 ; RV64I-NEXT:    slli a0, a0, 32
 ; RV64I-NEXT:    slli a1, a1, 32
 ; RV64I-NEXT:    srli a1, a1, 32
 ; RV64I-NEXT:    or a0, a0, a1
 ; RV64I-NEXT:    andi a1, a2, 31
 ; RV64I-NEXT:    srl a0, a0, a1
 ; RV64I-NEXT:    sext.w a0, a0
 ; RV64I-NEXT:    ret
 ;
 ; RV64IB-LABEL: fshr_i32:
 ; RV64IB:       # %bb.0:
+; RV64IB-NEXT:    andi a2, a2, 31
 ; RV64IB-NEXT:    fsrw a0, a1, a0, a2
 ; RV64IB-NEXT:    ret
 ;
 ; RV64IBT-LABEL: fshr_i32:
 ; RV64IBT:       # %bb.0:
+; RV64IBT-NEXT:    andi a2, a2, 31
 ; RV64IBT-NEXT:    fsrw a0, a1, a0, a2
 ; RV64IBT-NEXT:    ret
   %1 = tail call i32 @llvm.fshr.i32(i32 %a, i32 %b, i32 %c)
   ret i32 %1
 }
 
 declare i64 @llvm.fshr.i64(i64, i64, i64)
 
 define i64 @fshr_i64(i64 %a, i64 %b, i64 %c) nounwind {
 ; RV64I-LABEL: fshr_i64:
 ; RV64I:       # %bb.0:
 ; RV64I-NEXT:    srl a1, a1, a2
 ; RV64I-NEXT:    not a2, a2
 ; RV64I-NEXT:    slli a0, a0, 1
 ; RV64I-NEXT:    sll a0, a0, a2
 ; RV64I-NEXT:    or a0, a0, a1
 ; RV64I-NEXT:    ret
 ;
 ; RV64IB-LABEL: fshr_i64:
 ; RV64IB:       # %bb.0:
+; RV64IB-NEXT:    andi a2, a2, 63
 ; RV64IB-NEXT:    fsr a0, a1, a0, a2
 ; RV64IB-NEXT:    ret
 ;
 ; RV64IBT-LABEL: fshr_i64:
 ; RV64IBT:       # %bb.0:
+; RV64IBT-NEXT:    andi a2, a2, 63
 ; RV64IBT-NEXT:    fsr a0, a1, a0, a2
 ; RV64IBT-NEXT:    ret
   %1 = tail call i64 @llvm.fshr.i64(i64 %a, i64 %b, i64 %c)
   ret i64 %1
 }
 
 define signext i32 @fshri_i32(i32 signext %a, i32 signext %b) nounwind {
 ; RV64I-LABEL: fshri_i32: