PHI-def LIveInterval's LaneMask can be calculated by subranges LaneMask when Reg has SubRanges.

[Michael-Zhang-21]

Summary

When doing register coalescing for copys from reg to subreg like this:

%72.ssub_1:vregs = COPY %239:svregs

the %239 LiveRange is conflicting with %72.ssub_0 not %72.ssub_1, meaning the two regs are conflicting in different Lanes, which should return CR_Replace.

But the conflicting Segment of %72 was calcuated as 0xFFFFFFFF because %239 doesnot have a subreg Thus, the SubIdx of %72 is 0 and LaneMask is set to 0xFFFFFFFF, which will lead the conflict LaneMask with %239.

I think the LaneMask of PHI-def LiveRange should caculated by its subrange's LaneMask when the Reg has SubRanges. Because the Reg is Live in some subrange, It should have the LaneMask of this SubRange. When the VNI is Live all the SubRanges, Reg's LaneMask should be the Union of LaneMask of Subranges.

In New method, the LaneMask of %72's PHI-def Segment is the same as the subrange's LaneMask. The %72 and %239 will be conflicting in different Lanes and replace CR_Replace.

original question:
When doing register coalescing for copys from reg to subreg like this:
%72.ssub_1:vregs = COPY %239:svregs
If the liverange of %239 conflicts with the liverange of %72, but not for %72.ssub_1.

I have tried set SubRangeJoin to true, this COPY can be coalesced.

So can this attribute be modified by subtarget? Is there any potential problems when set this to true?
Hopeful to get some suggestion. Thanks.

[qcolombet]

I don't think this is correct.
Here we are merging the main live-ranges, so if we set the join subrange flag we may not check the right thing. In particular, the LaneBitmask in this call is incorrect, unless I am mistaken, for a subrange check.

[Michael-Zhang-21]

1、SubRangeJoin cannot be set to true in JoinVirtRegs function. Becuase it will check Live Ranges of a virtual register. When set to true, virtual reg with subranges will all be set to CR_Replace without LaneMask conflicting check. Thus, SubRangeJoin can not be set by Target.

2、I met a IR like this:
3504B bb.16.for.body62:
; predecessors: %bb.15, %bb.16

successors: %bb.17(0x04000000), %bb.16(0x7c000000); %bb.17(3.12%), %bb.16(96.88%)

...
3872B %72.ssub_0:v64_regs = Vector_Calc_Op %72.ssub_0:v64_regs
3904B %239:v32_regs = Vector_Calc_Op %239:v32_regs
4256B BNE %263:scalar_regs, $r0, %bb.16

4272B bb.17.for.cond71.preheader:
; predecessors: %bb.16

	  successors: %bb.19; %bb.19(100.00%)

4304B %72.ssub_1:v64_regs = COPY %239:v32_regs
4320B %237:v64_regs = IMPLICIT_DEF
4336B %238:v64_regs = COPY %72:v64_regs

Here, we are doing JoinVirtRegs function for this MI: 4304B %72.ssub_1:v64_regs = COPY %239:v32_regs.
Although %72 and %239 has conflicting LR, But they are conflicting in different Lanes. Only ssub0 of %72 is conflicting in 3872B, but not the ssub1 of %72 which is what we care about.
The reason why it fails is that PHI-def LR's LaneMask is set to 0xFFFFFFFF. But in this case, the VNI is not Live in all SubRanges. When considering about the LaneMask of this PHI-def LR, we can caculate the LaneMask which VNI is just living in. Other Subranges should have nothing with this VNI.

After modification, the IR looks like this :
3504B bb.16.for.body62:
; predecessors: %bb.15, %bb.16

	  successors: %bb.17(0x04000000), %bb.16(0x7c000000); %bb.17(3.12%), %bb.16(96.88%)

...
3872B %72.ssub_0:v64_regs = Vector_Calc_Op %72.ssub_0:v64_regs
3904B %72.ssub_1:v64_regs = Vector_Calc_Op %72.ssub_1:v64_regs
4256B BNE %263:scalar_regs, $r0, %bb.16

...
4272B bb.17.for.cond71.preheader:
; predecessors: %bb.16

	  successors: %bb.19; %bb.19(100.00%)

4320B %237:v64_regs = IMPLICIT_DEF
4336B %238:v64_regs = COPY %72:v64_regs

Please review

3、Met a case in AMDGPU: llvm/test/CodeGen/AMDGPU/coalescer-subranges-prune-kill-copy.mir
IR:

bb.0:
  undef %0.sub0:vreg_128 = IMPLICIT_DEF
  %0.sub1:vreg_128 = IMPLICIT_DEF
  %1:vreg_128 = COPY %0
  %2:vreg_128 = COPY killed %0
  S_BRANCH %bb.2

Analysis:
%1 and %2 should be identical values.
But in JoinVals::followCopyChain, %0 has subranges. The program will return in if (LRQ.valueIn() && ValueIn != LRQ.valueIn()) condition.
But the function returns TrackReg which is %1 and %2 respectively.
When %0 doesnot have subranges, func will return SrcReg, %0.
I think in subranges branch, it should return the final def values, which is SrcReg and VNI should be updated with LRQ.valueIn().

[Michael-Zhang-21]

In D145218#4242579, @qcolombet wrote:

I don't think this is correct.
Here we are merging the main live-ranges, so if we set the join subrange flag we may not check the right thing. In particular, the LaneBitmask in this call is incorrect, unless I am mistaken, for a subrange check.

Thanks for your comments. I figured out that the SubRangeJoin can not be set true when Joining MainRanges.
In order to handle the problem, conflicting subrange with mainrange, I have modified the method of LaneMask calculation for PHI-def LiveRange. Details have been illustrated.

Hopeful to get some remarks of this change. Thx

[qcolombet]

Hey!

Could you re-upload the patch with the full context please?

Thanks,
-Quentin

[Michael-Zhang-21]

In D145218#4387134, @qcolombet wrote:

Hey!

Could you re-upload the patch with the full context please?

Thanks,
-Quentin

Please see the patch:
https://reviews.llvm.org/D152066

[qcolombet]

Please see the patch:
https://reviews.llvm.org/D152066

It's still missing the context.

• Re-create your diff with -U99999
• Click Update Diff in the top right corner

[arsenm]

Needs testcase

[Michael-Zhang-21]

This comment was removed by Michael-Zhang-21.

[Michael-Zhang-21]

This comment was removed by Michael-Zhang-21.

[Michael-Zhang-21]

update patch diff again.

[Michael-Zhang-21]

In D145218#4399256, @qcolombet wrote:

Please see the patch:
https://reviews.llvm.org/D152066

It's still missing the context.

• Re-create your diff with -U99999
• Click Update Diff in the top right corner

thanks for your guidance.
I need to fix the failed cases first.

[Michael-Zhang-21]

In D145218#4399271, @arsenm wrote:

Needs testcase

ok. As the testcase is found on our own target, I need time to prepare some testcases on open targets.

llvm/lib/CodeGen/RegisterCoalescer.cpp

if (!SubRangeJoin || !LI.hasSubRanges()) {
         LaneBitmask SMask = TRI->composeSubRegIndexLaneMask(SubIdx, S.LaneMask);
         if ((SMask & LaneMask).none())
           continue;
         LiveQueryResult LRQ = S.Query(Def);
         if (!ValueIn) {
           ValueIn = LRQ.valueIn();
           continue;
         }
-        if (LRQ.valueIn() && ValueIn != LRQ.valueIn())
+        if (LRQ.valueIn() && ValueIn != LRQ.valueIn()) {
+          VNI = LRQ.valueIn();
+          TrackReg = SrcReg;
           return std::make_pair(VNI, TrackReg);
         }
       }
+    }
     if (ValueIn == nullptr) {
       // Reaching an undefined value is legitimate, for example:
       //
       // 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,
       //                          ;; but it's equivalent to "undef".
       return std::make_pair(nullptr, SrcReg);
JoinVals::analyzeValue(unsigned ValNo, JoinVals &Other) {
   }
 
   // Get the instruction defining this value, compute the lanes written.
   const MachineInstr *DefMI = nullptr;
   if (VNI->isPHIDef()) {
     // Conservatively assume that all lanes in a PHI are valid.
     LaneBitmask Lanes = SubRangeJoin ? LaneBitmask::getLane(0)
                                      : TRI->getSubRegIndexLaneMask(SubIdx);
+    if (!SubRangeJoin && LIS->getInterval(Reg).hasSubRanges()) {
+      Lanes = LaneBitmask::getNone();
+      for(auto subrangeIt = LIS->getInterval(Reg).subrange_begin();
+          subrangeIt != LIS->getInterval(Reg).subrange_end();
+          subrangeIt++){
+        if (subrangeIt->liveAt(VNI->def)) {
+          Lanes |= subrangeIt->LaneMask;
+        }
+      }
+    }
     V.ValidLanes = V.WriteLanes = Lanes;
   } else {
     DefMI = Indexes->getInstructionFromIndex(VNI->def);
     assert(DefMI != nullptr);
     if (SubRangeJoin) {
       // We don't care about the lanes when joining subregister ranges.
       V.WriteLanes = V.ValidLanes = LaneBitmask::getLane(0);
       if (DefMI->isImplicitDef()) {