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

Philip Reames via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 13 08:34:09 PDT 2022


reames added inline comments.


================
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) &&
----------------
reames wrote:
> craig.topper wrote:
> > 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.
> Just noting this question is still open.  I'm going to get dig into each of these cases, but I have not done it yet.
Looking at vcompress, it looks like it does not currently have a policy operand, and thus the patch as written is correct on that case.  However, thinking about it, I suspect this *should* have a policy operand.  Right now, we're taking the tail policy from the tied register logic in computeInfoForInstr, but we can a) frankly do better, and b) that code should probably live in ISEL.  Either way, I think we need to future proof a bit here.

Any suggestions for a reasonable name for "can write less than VL elements" or do we already have that somewhere?   I'll add a helper function with some asserts for now. 

(I did not look at reduction or vmv.s.x in detail, but given the findings on vcompress, guarding one should guard all.)


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

https://reviews.llvm.org/D127329



More information about the llvm-commits mailing list