[PATCH] D134703: [RISCV][ISel] Refactor the formation of VW operations
Craig Topper via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 30 14:35:23 PDT 2022
craig.topper added inline comments.
================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:8327
+ // Some splats don't come with a VL.
+ if (OrigOperand.getNumOperands() > 2)
+ VL = OrigOperand.getOperand(2);
----------------
I'm little confused by this check. The number of operands should be a property of the RISCVISD opcode. All RISCVISD::VMV_V_X_VL are supposed to have the same number of operands.
================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:8434
+ bool areVLAndMaskCompatible(const SDNode *Root) const {
+ std::pair<SDValue, SDValue> MaskAndVL = getMaskAndVL(Root);
+ return isMaskCompatible(MaskAndVL.first) &&
----------------
This can also use structured binding
================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:8453
+ default:
+ assert(0 && "Unexpected opcode");
+ return false;
----------------
Can this be llvm_unreachable?
================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:8483
+ /// LHS of the TargetOpcode.
+ NodeExtensionHelper LHS;
+ /// RHS of the TargetOpcode.
----------------
I think these can be references to NodeExtensionHelper? That would avoid needing to copy them. But I don't know if that would be fragile.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134703/new/
https://reviews.llvm.org/D134703
More information about the llvm-commits
mailing list