[llvm] 2ec1267 - GlobalISel: Fix some artifact combiner worklist inconsistencies

Matt Arsenault via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 18 14:17:48 PDT 2020


Author: Matt Arsenault
Date: 2020-06-18T17:17:39-04:00
New Revision: 2ec1267ecec372e55aed0995917b601f7cf61c5e

URL: https://github.com/llvm/llvm-project/commit/2ec1267ecec372e55aed0995917b601f7cf61c5e
DIFF: https://github.com/llvm/llvm-project/commit/2ec1267ecec372e55aed0995917b601f7cf61c5e.diff

LOG: GlobalISel: Fix some artifact combiner worklist inconsistencies

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.

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h b/llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h
index ba7034557a1a..016b0bacab85 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h
@@ -263,7 +263,7 @@ class LegalizationArtifactCombiner {
         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 @@ class LegalizationArtifactCombiner {
         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 @@ class LegalizationArtifactCombiner {
         // 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 @@ class LegalizationArtifactCombiner {
         // 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 @@ class LegalizationArtifactCombiner {
 
       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 @@ class LegalizationArtifactCombiner {
 
       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());


        


More information about the llvm-commits mailing list