[PATCH] D125337: [riscv] Prefer to use previous VL for scalar move instructions

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

In D125337#3504964 <https://reviews.llvm.org/D125337#3504964>, @craig.topper wrote:

> I think the "Not simplify due to precision" was supposed to say "simply"?

Yes.

> Can this replace the scalar move handling that's still in VSETVLIInfo::isCompatible?

Not sure, but I suspect not.  Will look more closely in follow up.



================
Comment at: llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp:1233
+      // one if we can prove it equally zero.  This remove a setvli entirely
+      // if the types match or allows use of cheaper avl preserving variant
+      // if VLMAX doesn't change.  If VLMAX might change, we couldn't use
----------------
frasercrmck wrote:
> `or if it allows the use of a cheaper`
The wording is intentional.  As written, it is explicitly an xor for the condition under which we run.  Yours leaves open the possibility of an else.


================
Comment at: llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp:1252
+    if (RISCVII::hasSEWOp(TSFlags)) {
+      CurInfo = computeInfoForInstr(MI, TSFlags, MRI);
+      continue;
----------------
frasercrmck wrote:
> If we don't update the scalar move instruction this is falling through and computing the same as `NewInfo`. I dunno if you think it'd be clearer if we were explicit about that in the previous block (e.g. update `CurInfo` and `continue` in an `else`?)
> 
> You could even early-continue if `MI` has a SEW op and it //isn't// a scalar move.
In the current patch, all three are reasonable.  I think the current form is fairly clear, and it simplifies a patch I'm about to post.  :)


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

https://reviews.llvm.org/D125337