[PATCH] D121881: [RISCV][RVV] Select unmasked TU RVV pseudos in a DAG post-process

[ShihPo Hung via Phabricator via llvm-commits]

arcbbb added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp:2259
+        RISCVII::TAIL_UNDISTURBED) {
+      if (N->getOperand(0).isUndef())
+        return false;
----------------
rogfer01 wrote:
> Sorry, I fail to follow the logic here. I'm sure I'm missing something very obvious.
> 
> My understanding is that the original code refrained itself from doing this optimization because it picks a pseudo that does not have a merge operand, so a tail policy that is not agnostic (i.e. tail undisturbed) is not eligible.
> 
> Your change allows picking a tail undisturbed pseudo when the tail policy is undisturbed (i.e. when the tail policy is not agnostic), why is then having an `undef` as a merge operand a problem?
> 
> In fact I think this is what @frasercrmck mentioned here https://reviews.llvm.org/D118810#3293768 
> 
> One option, also mentioned by @frasercrmck in https://reviews.llvm.org/D118810#3296415, is to convert this specific case into a tail agnostic case, rather than giving up as your change does.
> 
> Does this make sense?
Yes you are right, I think I am hesitant to make a decision whether to replace it with unmasked TA or unmasked TU.
Both seems reasonable to me, and besides I don't have a test case for that yet, maybe I should leave a FIXME here before we know how to handle it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121881