[PATCH] D52977: [RISCV] Introduce codegen patterns for instructions introduced in RV64I

[Alex Bradbury via Phabricator via llvm-commits]

asb marked an inline comment as done.
asb added inline comments.


================
Comment at: llvm/trunk/lib/Target/RISCV/RISCVInstrInfo.td:916
+def : Pat<(sra (sext_inreg GPR:$rs1, i32), GPR:$rs2),
+          (SRAW GPR:$rs1, GPR:$rs2)>;
+def : Pat<(sra (sext_inreg GPR:$rs1, i32), (and GPR:$rs2, immshifti32)),
----------------
jrtc27 wrote:
> efriedma wrote:
> > These shift-right patterns are sort of suspicious.  In particular, what happens if the shift amount is >= 32?  For example, consider the following function:
> > 
> >   long long x(long long y, int s) {
> >     return ((long long)(int)y)>>s;
> >   }
> > 
> > `x(1, 32)` should return 0.
> Agreed, this currently produces a single `sraw a0, a0, a1` for the body of the function, but SRAW only looks at `rs2[4:0]` ie the shift is modulo 32. The older versions of this patch set actually got this right, producing the correct `sext.w a0, a0 ; sra a0, a0, a1` (which is what GCC also does).
Thank you Eli. The pattern incorrectly assumes that (sext_inreg foo, i32) is only produced when the original IR type was i32. There are similar issues with the SLLW and SRLW. I've committed rL348067 which removes these for now and adds new test cases which will pick up these issues.

I think this is a case where we suffer for having i64 as the only legal type. I want to be able to exploit the fact that  `ashr i32 %a, %b` is UB where b >= 32, selecting srlw for this case. But that information seems to have been lost by the time we get to the SelectionDAG. I'll investigate how to handle this properly.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D52977