[PATCH] D81956: GlobalISel: Fix some artifact combiner worklist inconsistencies

[Matt Arsenault via Phabricator via llvm-commits]

arsenm created this revision.
arsenm added reviewers: aemerson, paquette, aditya_nandakumar, dsanders.
Herald added subscribers: rovka, wdng.
Herald added a project: LLVM.

In one case, UpdateDefs was not getting set and a dead SmallVector
constructed In another, it was adding new vreg defs to the updated set
which should be unnecessary. This also wasn't considering the multiple
defs of G_UNMERGE_VALUES.

      

Also increase the small vector sizes for merge/unmerge operands to the
usual semi-arbitrary 8. While debugging these, I'm usually seeing
merges and unmerges with at least 4 uses/defs.

      

I haven't run into an actual problem from any of these though.


https://reviews.llvm.org/D81956

Files:
  llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h


Index: llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h
===================================================================
--- llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h
+++ llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h
@@ -263,7 +263,7 @@
         const unsigned NumSrcs = DstSize / MergeSrcSize;
         assert(NumSrcs < SrcMI->getNumOperands() - 1 &&
                "trunc(merge) should require less inputs than merge");
-        SmallVector<Register, 2> SrcRegs(NumSrcs);
+        SmallVector<Register, 8> SrcRegs(NumSrcs);
         for (unsigned i = 0; i < NumSrcs; ++i)
           SrcRegs[i] = SrcMI->getOperand(i + 1).getReg();
 
@@ -375,9 +375,11 @@
         Builder.setInstr(MI);
         auto NewUnmerge = Builder.buildUnmerge(UnmergeTy, CastSrcReg);
 
-        SmallVector<Register, 8> Regs(NumDefs);
-        for (unsigned I = 0; I != NumDefs; ++I)
-          Builder.buildTrunc(MI.getOperand(I), NewUnmerge.getReg(I));
+        for (unsigned I = 0; I != NumDefs; ++I) {
+          Register DefReg = MI.getOperand(I).getReg();
+          UpdatedDefs.push_back(DefReg);
+          Builder.buildTrunc(DefReg, NewUnmerge.getReg(I));
+        }
 
         markInstAndDefDead(MI, CastMI, DeadInsts);
         return true;
@@ -402,7 +404,7 @@
         // Gather the original destination registers and create new ones for the
         // unused bits
         const unsigned NewNumDefs = CastSrcSize / DestSize;
-        SmallVector<Register, 2> DstRegs(NewNumDefs);
+        SmallVector<Register, 8> DstRegs(NewNumDefs);
         for (unsigned Idx = 0; Idx < NewNumDefs; ++Idx) {
           if (Idx < NumDefs)
             DstRegs[Idx] = MI.getOperand(Idx).getReg();
@@ -413,7 +415,7 @@
         // Build new unmerge
         Builder.setInstr(MI);
         Builder.buildUnmerge(DstRegs, CastSrcReg);
-        UpdatedDefs.append(DstRegs.begin(), DstRegs.begin() + NumDefs);
+        UpdatedDefs.append(DstRegs.begin(), DstRegs.begin() + NewNumDefs);
         markInstAndDefDead(MI, CastMI, DeadInsts);
         return true;
       }
@@ -547,7 +549,7 @@
 
       const unsigned NewNumDefs = NumDefs / NumMergeRegs;
       for (unsigned Idx = 0; Idx < NumMergeRegs; ++Idx) {
-        SmallVector<Register, 2> DstRegs;
+        SmallVector<Register, 8> DstRegs;
         for (unsigned j = 0, DefIdx = Idx * NewNumDefs; j < NewNumDefs;
              ++j, ++DefIdx)
           DstRegs.push_back(MI.getOperand(DefIdx).getReg());
@@ -602,7 +604,7 @@
 
       const unsigned NumRegs = NumMergeRegs / NumDefs;
       for (unsigned DefIdx = 0; DefIdx < NumDefs; ++DefIdx) {
-        SmallVector<Register, 2> Regs;
+        SmallVector<Register, 8> Regs;
         for (unsigned j = 0, Idx = NumRegs * DefIdx + 1; j < NumRegs;
              ++j, ++Idx)
           Regs.push_back(MergeI->getOperand(Idx).getReg());
@@ -749,7 +751,8 @@
       for (MachineInstr &U : MRI.use_instructions(MI.getOperand(0).getReg())) {
         if (U.getOpcode() == TargetOpcode::G_UNMERGE_VALUES ||
             U.getOpcode() == TargetOpcode::G_TRUNC) {
-          UpdatedDefs.push_back(MI.getOperand(0).getReg());
+          for (const MachineOperand &Def : MI.defs())
+            UpdatedDefs.push_back(Def.getReg());
           break;
         }
       }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D81956.271163.patch
Type: text/x-patch
Size: 3299 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200616/b6b0260b/attachment.bin>