[PATCH] D137106: [RISCV] Fix crash a vector add has a 4x sext and zext operand.

Mon Oct 31 14:45:56 PDT 2022

qcolombet accepted this revision.
qcolombet added a comment.
This revision is now accepted and ready to land.

Thanks for the quick fix.

================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:8459
+    if (!SExt.has_value())
+      return OrigOperand;
+
----------------
craig.topper wrote:
> qcolombet wrote:
> > That part shouldn't be needed. The `getSource()` should do the right thing.
> > I believe the reason it doesn't is because the `NodeExtensionHelper` got out-of-sync with the actual operand, maybe when we commute the operand of VWADD_W?
> > 
> > Let me check something.
> I don't think it had anything to with commuting. I think we lost track of the fact that we were matching to VWADDU_W and didn't want to look through the VSEXT_VL that was on the LHS. In this function we were acting based only on the opcode of the operand and not what pattern we were matching.
Ok well, got that wrong :).

You're right when we don't require any extension we just want to use the original value.

================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:8738
-      RHSExtOpc = *SExtRHS ? RISCVISD::VSEXT_VL : RISCVISD::VZEXT_VL;
-  }

----------------
That part of the change may not be needed to fix the issue, but I like your refactoring better anyway.
So no need to split it.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137106