[PATCH] D127329: [RISCV] A vector instruction without a tail is always tail agnostic

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h:192
+static inline unsigned getVecPolicyOpNum(const MCInstrDesc &Desc) {
+  const uint64_t TSFlags = Desc.TSFlags;
+  assert(hasVecPolicyOp(TSFlags));
----------------
TSFlags would be unused on a release build suggest

```
assert(hasVecPolicyOp(Desc.TSFlags));
```


================
Comment at: llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp:2491
+
+  const RISCVInstrInfo *TII = static_cast<const RISCVInstrInfo *>(
+      Subtarget->getInstrInfo());
----------------
Assuming Subtarget is RISCVSubtarget*, doesn't RISCVSubtarget::getInstrInfo() return RISCVInstrInfo * without a cast?


================
Comment at: llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp:2520
+
+  // A tail undisturbed (tu) op with no tail is tail agnostic (ta)
+  if (!(Policy & RISCVII::TAIL_AGNOSTIC) && isa<ConstantSDNode>(VL) &&
----------------
Just to confirm.

vcompress doesn't have a policy operand so this wouldn't apply? I ask because vcompress writes less than VL elements and the tail policy applies to the other elements.

reductions and vmv.s.x also don't have policy operand.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp:2534
+                                        VecPolicy->getValueType(0));
+  SmallVector<SDValue, 8> Ops;
+  for (unsigned I = 0, E = N->getNumOperands(); I != E; I++) {
----------------
Why not copy all values using the SmallVector range constructor and overwrite Ops[VecPolicyOpNum] afterwards.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127329