[PATCH] D136916: [LoongArch] Fix codegen for [s/u]itofp instructions

WÁNG Xuěruì via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 28 19:36:46 PDT 2022


xen0n added a comment.

Seems we're using the same methodology recently... namely trying to bootstrap a native LLVM toolchain with LLVM/Clang themselves. Because I've seen the same crash yesterday night. Thanks for catching this.



================
Comment at: llvm/lib/Target/LoongArch/LoongArchFloat32InstrInfo.td:290
+// uint -> f32
+def : Pat<(f32 (uint_to_fp (i64 (sexti32 (i64 GPR:$src))))),
+          (FFINT_S_W (MOVGR2FR_W GPR:$src))>;
----------------
`sexti32` for `uint`? (I might be sleepy so this might not be wrong after all.) The codegen in the touched test cases look good though, this is a bit weird.


================
Comment at: llvm/lib/Target/LoongArch/LoongArchISelDAGToDAG.cpp:216-220
+  if (N.getOpcode() == LoongArchISD::BSTRPICK &&
+      N.getConstantOperandVal(1) < UINT64_C(0X1F) &&
+      N.getConstantOperandVal(2) == UINT64_C(0)) {
+    Val = N;
+    return true;
----------------
This makes me feel a bit nervous, because we conventionally use such `bstrpick` insns for zero extensions. The MSB immediate is tested to be strictly less than 32 so this is actually a guaranteed zero extension. Are we doing the right thing by matching the case in `sexti32` too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136916



More information about the llvm-commits mailing list