[TailDuplicator] Fix old bugs in TailDuplicator::duplicateInstruction
authorBjorn Pettersson <bjorn.a.pettersson@ericsson.com>
Wed, 21 Dec 2022 21:10:52 +0000 (22:10 +0100)
committerBjorn Pettersson <bjorn.a.pettersson@ericsson.com>
Mon, 6 Feb 2023 18:21:23 +0000 (19:21 +0100)
This patch is updating TailDuplicator::duplicateInstruction to fix
some old bugs that has been found with an out-of-tree target. There
are three different things being addressed:

1) In one situation two subregister indices are combined using the
   composeSubRegIndices helper. But the order in which those indices
   are combined has been incorrect. For this problem I managed to
   create some kind of reproducer using AArch64 (see the test case
   touched in this patch).

2) Another fault was found in the else branch for the above situation.
   Here we do not compose the two subregisters, instead we insert a
   COPY to replace the PHI, and then the subreg index in the using
   MO remains. Thus, the virtual register created for the COPY should
   always match with the size of the original register. Therefore the
   optimization that "constrain" (or rather relax) the register
   class by looking at the instruction desc must be limited to the
   situation when there is no subregister access. Otherwise we create
   a vreg with the wrong class.

3) Last problem addressed in this patch is that when a new register
   class is picked by looking at the instruction desc, then it
   isn't guaranteed that the isAllocatable property is set for that
   class. So one need to use the getAllocatableClass helper to find
   a subclass that is allocatable before using createVirualRegister,
   or alternatively (as in this patch) just use the OrigRC instead
   of relaxing the register class for the COPY destination.

Haven't been able to find any in-tree reproducers for problem 2 and 3.
The tricky part is to find a target that has register hierarchies that
match with the problem to trigger those code paths (and with subreg
accesses involved).

Differential Revision: https://reviews.llvm.org/D140496

llvm/lib/CodeGen/TailDuplicator.cpp
llvm/test/CodeGen/AArch64/taildup-subreg-compose.mir

index 865add2..93c2252 100644 (file)
@@ -436,16 +436,13 @@ void TailDuplicator::duplicateInstruction(
             MO.setReg(VI->second.Reg);
             // We have Reg -> VI.Reg:VI.SubReg, so if Reg is used with a
             // sub-register, we need to compose the sub-register indices.
-            MO.setSubReg(TRI->composeSubRegIndices(MO.getSubReg(),
-                                                   VI->second.SubReg));
+            MO.setSubReg(
+                TRI->composeSubRegIndices(VI->second.SubReg, MO.getSubReg()));
           } else {
             // The direct replacement is not possible, due to failing register
             // class constraints. An explicit COPY is necessary. Create one
-            // that can be reused
-            auto *NewRC = MI->getRegClassConstraint(i, TII, TRI);
-            if (NewRC == nullptr)
-              NewRC = OrigRC;
-            Register NewReg = MRI->createVirtualRegister(NewRC);
+            // that can be reused.
+            Register NewReg = MRI->createVirtualRegister(OrigRC);
             BuildMI(*PredBB, NewMI, NewMI.getDebugLoc(),
                     TII->get(TargetOpcode::COPY), NewReg)
                 .addReg(VI->second.Reg, 0, VI->second.SubReg);
index 19f7bd3..32aa00a 100644 (file)
@@ -23,7 +23,7 @@ body:             |
   ; CHECK-NEXT:   liveins: $q1
   ; CHECK-NEXT: {{  $}}
   ; CHECK-NEXT:   [[COPY:%[0-9]+]]:fpr128 = COPY $q1
-  ; CHECK-NEXT:   $s1 = COPY [[COPY]].dsub
+  ; CHECK-NEXT:   $s1 = COPY [[COPY]].ssub
   ; CHECK-NEXT:   [[COPY1:%[0-9]+]]:fpr64 = COPY [[COPY]].dsub
   ; CHECK-NEXT:   B %bb.4
   ; CHECK-NEXT: {{  $}}
@@ -31,7 +31,7 @@ body:             |
   ; CHECK-NEXT:   successors: %bb.4(0x80000000)
   ; CHECK-NEXT: {{  $}}
   ; CHECK-NEXT:   [[DEF:%[0-9]+]]:fpr128 = IMPLICIT_DEF
-  ; CHECK-NEXT:   $s1 = COPY [[DEF]].dsub
+  ; CHECK-NEXT:   $s1 = COPY [[DEF]].ssub
   ; CHECK-NEXT:   [[COPY2:%[0-9]+]]:fpr64 = COPY [[DEF]].dsub
   ; CHECK-NEXT:   B %bb.4
   ; CHECK-NEXT: {{  $}}