[llvm] df947fe - [TailDuplicator] Fix old bugs in TailDuplicator::duplicateInstruction

[Bjorn Pettersson via llvm-commits]

Author: Bjorn Pettersson
Date: 2023-02-06T19:21:23+01:00
New Revision: df947febe2eebce4a1b244947dd8a02068edb306

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

LOG: [TailDuplicator] Fix old bugs in TailDuplicator::duplicateInstruction

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

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/TailDuplicator.cpp b/llvm/lib/CodeGen/TailDuplicator.cpp
index 865add28f781..93c225297839 100644
--- a/llvm/lib/CodeGen/TailDuplicator.cpp
+++ b/llvm/lib/CodeGen/TailDuplicator.cpp
@@ -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);

diff  --git a/llvm/test/CodeGen/AArch64/taildup-subreg-compose.mir b/llvm/test/CodeGen/AArch64/taildup-subreg-compose.mir
index 19f7bd3dec78..32aa00a62151 100644
--- a/llvm/test/CodeGen/AArch64/taildup-subreg-compose.mir
+++ b/llvm/test/CodeGen/AArch64/taildup-subreg-compose.mir
@@ -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: {{  $}}