[PATCH] D158161: [RISCV] Don't relax policy to ta when vmerge's VL shrinks during folding

[Luke Lau via Phabricator via llvm-commits]

luke added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp:3512
+  bool MergeVLShrunk = VL != OrigVL;
+  uint64_t Policy = (isImplicitDef(Merge) && !MergeVLShrunk) ? RISCVII::TAIL_AGNOSTIC : /*TUMU*/ 0;
   SDValue PolicyOp =
----------------
reames wrote:
> reames wrote:
> > If instead, we were to change this line to:
> > ```
> > uint64_t Policy = isImplicitDef(False)  ? RISCVII::TAIL_AGNOSTIC : /*TUMU*/ 0;
> > 
> > ```
> > What would happen?  I'm worried there's some other case we haven't caught where Merge was implicit_def, but the resulting operation has a non-implicit_def result.  I think *all* such cases needs to be TU.  
> Answer: A lot of regressions.  I tried this, and it definitely doesn't seem to work.
> 
> I'm still a bit worried there's another case here.  I don't have anything specific though.
> 
> 
The new tail of the Result node is every element from min(vmerge.vl, true.vl) onwards. 

Therefore if we want to set a tail agnostic policy on it, we need to know that every element of vmerge from min(vmerge.vl, true.vl) onwards would have been implicit-def anyway.

The vmerge pseudo gets its tail from Merge, so if the Merge at that element was implicit-def, then that tail element will be implicit-def.

implicit-def(Merge[i])
->
implicit-def(vmerge[i]),  vmerge.vl <= i < vlmax

We can rewrite this to

implicit-def(Merge[i]) ∧ vmerge.vl == min(vmerge.vl, true.vl)
->
implicit-def(vmerge[i]),  min(vmerge.vl, true.vl) <= i < vlmax

So as long as we check `isImplicitDef(Merge)` and `VL == OldVL`, then we can infer the result

implicit-def(vmerge[i]),  min(vmerge.vl, true.vl) <= i < vlmax

i.e. every element past min(vmerge.vl, true.vl) in vmerge would have been implicit-def.

Btw I tried substituting Merge for False as well before posting the patch and got the same result.

implicit(False) -> implicit(Merge) so as long as we also check the VL stays the same, it's a correct transformation, just not optimal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158161