[PATCH] D35073: [RegisterCoalescer] Fix for subrange join unreachable

[Tim Renouf via Phabricator via llvm-commits]

tpr added a comment.

Krzysztof's fix for another occurrence of what looks like the same problem was as follows. This cropped up in the middle of https://reviews.llvm.org/D48102.

I don't know if this fixes all our occurrences of the problem, but it's worth checking.

  --- a/lib/CodeGen/RegisterCoalescer.cpp
  +++ b/lib/CodeGen/RegisterCoalescer.cpp
  @@ -2121,7 +2117,8 @@ class JoinVals {
     LaneBitmask computeWriteLanes(const MachineInstr *DefMI, bool &Redef) const;
   
     /// Find the ultimate value that VNI was copied from.
  -  std::pair<const VNInfo*,unsigned> followCopyChain(const VNInfo *VNI) const;
  +  std::pair<const VNInfo*,unsigned> followCopyChain(const VNInfo *VNI,
  +                                                    unsigned OtherReg) const;
   
     bool valuesIdentical(VNInfo *Val0, VNInfo *Val1, const JoinVals &Other) const;
   
  @@ -2240,18 +2237,18 @@ LaneBitmask JoinVals::computeWriteLanes(const MachineInstr *DefMI, bool &Redef)
   }
   
   std::pair<const VNInfo*, unsigned> JoinVals::followCopyChain(
  -    const VNInfo *VNI) const {
  -  unsigned Reg = this->Reg;
  +    const VNInfo *VNI, unsigned OtherReg) const {
  +  unsigned TrackReg = Reg;
   
     while (!VNI->isPHIDef()) {
       SlotIndex Def = VNI->def;
       MachineInstr *MI = Indexes->getInstructionFromIndex(Def);
       assert(MI && "No defining instruction");
       if (!MI->isFullCopy())
  -      return std::make_pair(VNI, Reg);
  +      return std::make_pair(VNI, TrackReg);
       unsigned SrcReg = MI->getOperand(1).getReg();
       if (!TargetRegisterInfo::isVirtualRegister(SrcReg))
  -      return std::make_pair(VNI, Reg);
  +      return std::make_pair(VNI, TrackReg);
   
       const LiveInterval &LI = LIS->getInterval(SrcReg);
       const VNInfo *ValueIn;
  @@ -2272,25 +2269,41 @@ std::pair<const VNInfo*, unsigned> JoinVals::followCopyChain(
           break;
         }
       }
  -    if (ValueIn == nullptr)
  +    if (ValueIn == nullptr) {
  +      // Reaching undefined value is legitimate in cases where it comes from
  +      // one of the considered registers (Reg or OtherReg).
  +      //
  +      // 1   undef %0.sub1 = ...  ;; %0.sub0 == undef
  +      // 2   %1 = COPY %0         ;; %1 is defined here.
  +      // 3   %0 = COPY %1         ;; Now %0.sub0 has a definition!
  +      // 4   ...                  ;; Here we should indicate that %0 at 3 is
  +      //                          ;; the same as %0 at 2 (%0 at 3 == %1 at 2).
  +      if (SrcReg == Reg || SrcReg == OtherReg)
  +        return std::make_pair(nullptr, SrcReg);
         break;
  +    }
       VNI = ValueIn;
  -    Reg = SrcReg;
  +    TrackReg = SrcReg;
     }
  -  return std::make_pair(VNI, Reg);
  +  return std::make_pair(VNI, TrackReg);
   }
   
   bool JoinVals::valuesIdentical(VNInfo *Value0, VNInfo *Value1,
                                  const JoinVals &Other) const {
     const VNInfo *Orig0;
     unsigned Reg0;
  -  std::tie(Orig0, Reg0) = followCopyChain(Value0);
  +  std::tie(Orig0, Reg0) = followCopyChain(Value0, Other.Reg);
     if (Orig0 == Value1 && Reg0 == Other.Reg)
       return true;
   
     const VNInfo *Orig1;
     unsigned Reg1;
  -  std::tie(Orig1, Reg1) = Other.followCopyChain(Value1);
  +  std::tie(Orig1, Reg1) = Other.followCopyChain(Value1, Reg);
  +  // If both values are undefined, and the source registers are the same
  +  // register, the values are identical. Filter out cases where only one
  +  // value is defined.
  +  if (Orig0 == nullptr || Orig1 == nullptr)
  +    return Orig0 == Orig1 && Reg0 == Reg1;
   
     // The values are equal if they are defined at the same place and use the
     // same register. Note that we cannot compare VNInfos directly as some of


https://reviews.llvm.org/D35073