[llvm] [RISCV] Copy AVLs whose LiveIntervals aren't extendable in insertVSETVLI (PR #98342)

Philip Reames via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 11 10:08:58 PDT 2024


================
@@ -1149,15 +1143,26 @@ void RISCVInsertVSETVLI::insertVSETVLI(MachineBasicBlock &MBB,
                 .addImm(Info.encodeVTYPE());
   if (LIS) {
     LIS->InsertMachineInstrInMaps(*MI);
-    // Normally the AVL's live range will already extend past the inserted
-    // vsetvli because the pseudos below will already use the AVL. But this
-    // isn't always the case, e.g. PseudoVMV_X_S doesn't have an AVL operand or
-    // we've taken the AVL from the VL output of another vsetvli.
     LiveInterval &LI = LIS->getInterval(AVLReg);
     SlotIndex SI = LIS->getInstructionIndex(*MI).getRegSlot();
-    assert((LI.liveAt(SI) && LI.getVNInfoAt(SI) == Info.getAVLVNInfo()) ||
-           (!LI.liveAt(SI) && LI.containsOneValue()));
-    LIS->extendToIndices(LI, SI);
+    // If the AVL value isn't live at MI, do a quick check to see if it's easily
+    // extendable. Otherwise, we need to copy it.
+    if (LI.getVNInfoBefore(SI) != Info.getAVLVNInfo()) {
+      if (!LI.liveAt(SI) && LI.containsOneValue())
+        LIS->extendToIndices(LI, SI);
+      else {
+        Register AVLCopyReg =
+            MRI->createVirtualRegister(&RISCV::GPRNoX0RegClass);
+        MachineBasicBlock::iterator AVLDef =
----------------
preames wrote:

> A PHI def's SlotIndex will be at the entry to the block, so it will insert a copy at the entry, which I believe is what we want.

Are we guaranteed that the Slot_Block entry maps to an MachineInstr?  Getting a MachineInstr can return nullptr, and I'm not clear about the exact circumstances.

> > I'd appreciate if you did a local patch which removes the previous extend check and ensures everything works
> 
> If I'm understanding you correctly, the previous extend check is removed in this patch already

You have:
```
      if (!LI.liveAt(SI) && LI.containsOneValue())
        LIS->extendToIndices(LI, SI);
```

I'm suggesting run "ninja check" (and other tests of your choice) with:

```
      if (false && !LI.liveAt(SI) && LI.containsOneValue())
        LIS->extendToIndices(LI, SI);
```

i.e. Force *everything* done the COPY path.  This will hurt code quality, but should never violate invariants.  (This is only a local to you test to make sure it works.)

https://github.com/llvm/llvm-project/pull/98342


More information about the llvm-commits mailing list