Fix Sub-register Rewriting in Aggressive Anti-Dependence Breaker

[tjablin]

Summary

Previously, HandleLastUse would delete RegRef information for sub-registers if they were dead even if their corresponding super-register were still live. If the super-register were later renamed, then the definitions of the sub-register would not be updated appropriately. This patch alters the behavior so that RegInfo information for sub-registers is only deleted when the sub-register and super-register are both dead. This resolves PR26775. This is the mirror image of Hal's r227311 commit.

[hfinkel]

LGTM, thanks! -- Nice efficient patch :-) [I realize that I'm ruining that somewhat by asking for a comment ;) ]

../llvm/lib/CodeGen/AggressiveAntiDepBreaker.cpp

315 ↗

(On Diff #51557)

Please add a comment here explaining why we do this only if the superregister was not live. Maybe something like this:

// Note that we only do this if the superregister was not live because otherwise, regardless whether we have an explicit use of the subregister, the subregister's contents are needed for the uses of the superregister.

[tjablin]

Add comments per Hal's suggestion.

[cycheng]

Committed r265097 (On behalf of Tom)

[tjablin]

Can this patch please be considered for inclusion in LLVM 3.8.1?

[tjablin]

This is also a fix for 25503 which is important for mesa/llvmpipe on
ppc64le. I know some IBMers would like this to be fixed for 3.8.1.

lib/CodeGen/AggressiveAntiDepBreaker.cpp

void AggressiveAntiDepBreaker::HandleLastUse(unsigned Reg, unsigned KillIdx,
   if (!State->IsLive(Reg)) {
     KillIndices[Reg] = KillIdx;
     DefIndices[Reg] = ~0u;
     RegRefs.erase(Reg);
     State->LeaveGroup(Reg);
     DEBUG(if (header) {
         dbgs() << header << TRI->getName(Reg); header = nullptr; });
     DEBUG(dbgs() << "->g" << State->GetGroup(Reg) << tag);
-  }
-  // Repeat for subregisters.
+    // Repeat for subregisters. Note that we only do this if the superregister
+    // was not live because otherwise, regardless whether we have an explicit
+    // use of the subregister, the subregister's contents are needed for the
+    // uses of the superregister.
     for (MCSubRegIterator SubRegs(Reg, TRI); SubRegs.isValid(); ++SubRegs) {
       unsigned SubregReg = *SubRegs;
       if (!State->IsLive(SubregReg)) {
         KillIndices[SubregReg] = KillIdx;
         DefIndices[SubregReg] = ~0u;
         RegRefs.erase(SubregReg);
         State->LeaveGroup(SubregReg);
         DEBUG(if (header) {
             dbgs() << header << TRI->getName(Reg); header = nullptr; });
         DEBUG(dbgs() << " " << TRI->getName(SubregReg) << "->g" <<
               State->GetGroup(SubregReg) << tag);
       }
     }
+  }
 
   DEBUG(if (!header && footer) dbgs() << footer);
 }
 
 void AggressiveAntiDepBreaker::PrescanInstruction(
     MachineInstr &MI, unsigned Count, std::set<unsigned> &PassthruRegs) {
   std::vector<unsigned> &DefIndices = State->GetDefIndices();
   std::multimap<unsigned, AggressiveAntiDepState::RegisterReference>&

test/CodeGen/PowerPC/aggressive-anti-dep-breaker-subreg.ll

+; RUN: llc %s -mtriple=powerpc64-unknown-linux-gnu -O2 -o - -optimize-regalloc=false -regalloc=fast | FileCheck %s
+
+declare void @func(i8*, i64, i64)
+
+define void @test(i8* %context, i32** %elementArrayPtr, i32 %value) {
+entry:
+  %cmp = icmp eq i32 %value, 0
+  br i1 %cmp, label %lreturn, label %lnext
+
+lnext:
+  %elementArray = load i32*, i32** %elementArrayPtr, align 8
+; CHECK: lwz [[LDREG:[0-9]+]], 124(1)                   # 4-byte Folded Reload
+; CHECK: # implicit-def: %X[[TEMPREG:[0-9]+]]
+  %element = load i32, i32* %elementArray, align 4
+; CHECK: mr [[TEMPREG]], [[LDREG]]
+; CHECK: clrldi   4, [[TEMPREG]], 32
+  %element.ext = zext i32 %element to i64
+  %value.ext = zext i32 %value to i64
+  call void @func(i8* %context, i64 %value.ext, i64 %element.ext)
+  br label %lreturn
+
+lreturn:
+  ret void
+}