[llvm] r350595 - RegBankSelect: Fix copy insertion point for terminators

Matt Arsenault via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 7 17:22:48 PST 2019


Author: arsenm
Date: Mon Jan  7 17:22:47 2019
New Revision: 350595

URL: http://llvm.org/viewvc/llvm-project?rev=350595&view=rev
Log:
RegBankSelect: Fix copy insertion point for terminators

If a copy was needed to handle the condition of brcond, it was being
inserted before the defining instruction. Add tests for iterator edge
cases.

I find the existing code here suspect for the case where it's looking
for terminators that modify the register. It's going to insert a copy
in the middle of the terminators, which isn't allowed (it might be
necessary to have a COPY_terminator if anybody actually needs this).

Also legalize brcond for AMDGPU.

Added:
    llvm/trunk/test/CodeGen/AMDGPU/GlobalISel/legalize-brcond.mir
    llvm/trunk/test/CodeGen/AMDGPU/GlobalISel/regbankselect-brcond.mir
Modified:
    llvm/trunk/lib/CodeGen/GlobalISel/RegBankSelect.cpp
    llvm/trunk/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
    llvm/trunk/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp

Modified: llvm/trunk/lib/CodeGen/GlobalISel/RegBankSelect.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/GlobalISel/RegBankSelect.cpp?rev=350595&r1=350594&r2=350595&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/GlobalISel/RegBankSelect.cpp (original)
+++ llvm/trunk/lib/CodeGen/GlobalISel/RegBankSelect.cpp Mon Jan  7 17:22:47 2019
@@ -717,15 +717,21 @@ RegBankSelect::RepairingPlacement::Repai
     unsigned Reg = MO.getReg();
     if (Before) {
       // Check whether Reg is defined by any terminator.
-      MachineBasicBlock::iterator It = MI;
-      for (auto Begin = MI.getParent()->begin();
-           --It != Begin && It->isTerminator();)
-        if (It->modifiesRegister(Reg, &TRI)) {
-          // Insert the repairing code right after the definition.
-          addInsertPoint(*It, /*Before*/ false);
-          return;
-        }
-      addInsertPoint(*It, /*Before*/ true);
+      MachineBasicBlock::reverse_iterator It = MI;
+      auto REnd = MI.getParent()->rend();
+
+      for (; It != REnd && It->isTerminator(); ++It) {
+        assert(!It->modifiesRegister(Reg, &TRI) &&
+               "copy insertion in middle of terminators not handled");
+      }
+
+      if (It == REnd) {
+        addInsertPoint(*MI.getParent()->begin(), true);
+        return;
+      }
+
+      // We are sure to be right before the first terminator.
+      addInsertPoint(*It, /*Before*/ false);
       return;
     }
     // Make sure Reg is not redefined by other terminators, otherwise

Modified: llvm/trunk/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp?rev=350595&r1=350594&r2=350595&view=diff
==============================================================================
--- llvm/trunk/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp (original)
+++ llvm/trunk/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp Mon Jan  7 17:22:47 2019
@@ -85,6 +85,8 @@ AMDGPULegalizerInfo::AMDGPULegalizerInfo
     PrivatePtr
   };
 
+  setAction({G_BRCOND, S1}, Legal);
+
   setAction({G_ADD, S32}, Legal);
   setAction({G_ASHR, S32}, Legal);
   setAction({G_SUB, S32}, Legal);

Modified: llvm/trunk/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp?rev=350595&r1=350594&r2=350595&view=diff
==============================================================================
--- llvm/trunk/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp (original)
+++ llvm/trunk/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp Mon Jan  7 17:22:47 2019
@@ -221,6 +221,22 @@ AMDGPURegisterBankInfo::getInstrAlternat
     AltMappings.push_back(&VVMapping);
     return AltMappings;
   }
+  case AMDGPU::G_BRCOND: {
+    assert(MRI.getType(MI.getOperand(0).getReg()).getSizeInBits() == 1);
+
+    const InstructionMapping &SMapping = getInstructionMapping(
+      1, 1, getOperandsMapping(
+        {AMDGPU::getValueMapping(AMDGPU::SCCRegBankID, 1), nullptr}),
+      2); // Num Operands
+    AltMappings.push_back(&SMapping);
+
+    const InstructionMapping &VMapping = getInstructionMapping(
+      1, 1, getOperandsMapping(
+        {AMDGPU::getValueMapping(AMDGPU::SGPRRegBankID, 1), nullptr }),
+      2); // Num Operands
+    AltMappings.push_back(&VMapping);
+    return AltMappings;
+  }
   default:
     break;
   }
@@ -712,6 +728,16 @@ AMDGPURegisterBankInfo::getInstrMapping(
   case AMDGPU::G_ATOMIC_CMPXCHG: {
     return getDefaultMappingAllVGPR(MI);
   }
+  case AMDGPU::G_BRCOND: {
+    unsigned Bank = getRegBankID(MI.getOperand(0).getReg(), MRI, *TRI,
+                                 AMDGPU::SGPRRegBankID);
+    assert(MRI.getType(MI.getOperand(0).getReg()).getSizeInBits() == 1);
+    if (Bank != AMDGPU::SCCRegBankID)
+      Bank = AMDGPU::SGPRRegBankID;
+
+    OpdsMapping[0] = AMDGPU::getValueMapping(Bank, 1);
+    break;
+  }
   }
 
   return getInstructionMapping(1, 1, getOperandsMapping(OpdsMapping),

Added: llvm/trunk/test/CodeGen/AMDGPU/GlobalISel/legalize-brcond.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AMDGPU/GlobalISel/legalize-brcond.mir?rev=350595&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/AMDGPU/GlobalISel/legalize-brcond.mir (added)
+++ llvm/trunk/test/CodeGen/AMDGPU/GlobalISel/legalize-brcond.mir Mon Jan  7 17:22:47 2019
@@ -0,0 +1,24 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple=amdgcn-mesa-mesa3d -mcpu=fiji -O0 -run-pass=legalizer %s -o - | FileCheck %s
+
+---
+name: legal_brcond
+body:             |
+  ; CHECK-LABEL: name: legal_brcond
+  ; CHECK: bb.0.entry:
+  ; CHECK:   successors: %bb.1(0x80000000)
+  ; CHECK:   [[COPY:%[0-9]+]]:_(s32) = COPY $vgpr0
+  ; CHECK:   [[COPY1:%[0-9]+]]:_(s32) = COPY $vgpr1
+  ; CHECK:   [[ICMP:%[0-9]+]]:_(s1) = G_ICMP intpred(ne), [[COPY]](s32), [[COPY1]]
+  ; CHECK:   G_BRCOND [[ICMP]](s1), %bb.1
+  ; CHECK: bb.1:
+  bb.0.entry:
+    successors: %bb.1
+    liveins: $vgpr0, $vgpr1
+    %0:_(s32) = COPY $vgpr0
+    %1:_(s32) = COPY $vgpr1
+    %2:_(s1) = G_ICMP intpred(ne), %0, %1
+    G_BRCOND %2, %bb.1
+
+  bb.1:
+...

Added: llvm/trunk/test/CodeGen/AMDGPU/GlobalISel/regbankselect-brcond.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AMDGPU/GlobalISel/regbankselect-brcond.mir?rev=350595&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/AMDGPU/GlobalISel/regbankselect-brcond.mir (added)
+++ llvm/trunk/test/CodeGen/AMDGPU/GlobalISel/regbankselect-brcond.mir Mon Jan  7 17:22:47 2019
@@ -0,0 +1,179 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -march=amdgcn -mcpu=fiji -run-pass=regbankselect %s -verify-machineinstrs -o - -regbankselect-fast | FileCheck %s
+# RUN: llc -march=amdgcn -mcpu=fiji -run-pass=regbankselect %s -verify-machineinstrs -o - -regbankselect-greedy | FileCheck %s
+
+---
+name: brcond_vcc_cond
+legalized: true
+body:             |
+  ; CHECK-LABEL: name: brcond_vcc_cond
+  ; CHECK: bb.0.entry:
+  ; CHECK:   successors: %bb.1(0x80000000)
+  ; CHECK:   [[COPY:%[0-9]+]]:vgpr(s32) = COPY $vgpr0
+  ; CHECK:   [[COPY1:%[0-9]+]]:vgpr(s32) = COPY $vgpr1
+  ; CHECK:   [[ICMP:%[0-9]+]]:sgpr(s1) = G_ICMP intpred(ne), [[COPY]](s32), [[COPY1]]
+  ; CHECK:   G_BRCOND [[ICMP]](s1), %bb.1
+  ; CHECK: bb.1:
+  bb.0.entry:
+    successors: %bb.1
+    liveins: $vgpr0, $vgpr1
+    %0:_(s32) = COPY $vgpr0
+    %1:_(s32) = COPY $vgpr1
+    %2:_(s1) = G_ICMP intpred(ne), %0, %1
+    G_BRCOND %2, %bb.1
+
+  bb.1:
+...
+
+---
+name: brcond_scc_cond
+legalized: true
+body:             |
+  ; CHECK-LABEL: name: brcond_scc_cond
+  ; CHECK: bb.0.entry:
+  ; CHECK:   successors: %bb.1(0x80000000)
+  ; CHECK:   [[COPY:%[0-9]+]]:sgpr(s32) = COPY $sgpr0
+  ; CHECK:   [[COPY1:%[0-9]+]]:sgpr(s32) = COPY $sgpr1
+  ; CHECK:   [[ICMP:%[0-9]+]]:scc(s1) = G_ICMP intpred(ne), [[COPY]](s32), [[COPY1]]
+  ; CHECK:   G_BRCOND [[ICMP]](s1), %bb.1
+  ; CHECK: bb.1:
+  bb.0.entry:
+    successors: %bb.1
+    liveins: $sgpr0, $sgpr1
+    %0:_(s32) = COPY $sgpr0
+    %1:_(s32) = COPY $sgpr1
+    %2:_(s1) = G_ICMP intpred(ne), %0, %1
+    G_BRCOND %2, %bb.1
+
+  bb.1:
+...
+
+---
+name: brcond_sgpr_cond
+legalized: true
+body:             |
+  ; CHECK-LABEL: name: brcond_sgpr_cond
+  ; CHECK: bb.0.entry:
+  ; CHECK:   successors: %bb.1(0x80000000)
+  ; CHECK:   [[COPY:%[0-9]+]]:sgpr(s32) = COPY $sgpr0
+  ; CHECK:   [[TRUNC:%[0-9]+]]:sgpr(s1) = G_TRUNC [[COPY]](s32)
+  ; CHECK:   G_BRCOND [[TRUNC]](s1), %bb.1
+  ; CHECK: bb.1:
+  bb.0.entry:
+    successors: %bb.1
+    liveins: $sgpr0
+    %0:_(s32) = COPY $sgpr0
+    %1:_(s1) = G_TRUNC %0
+    G_BRCOND %1, %bb.1
+
+  bb.1:
+...
+
+---
+name: brcond_vgpr_cond
+legalized: true
+body:             |
+  ; CHECK-LABEL: name: brcond_vgpr_cond
+  ; CHECK: bb.0.entry:
+  ; CHECK:   successors: %bb.1(0x80000000)
+  ; CHECK:   [[COPY:%[0-9]+]]:vgpr(s32) = COPY $vgpr0
+  ; CHECK:   [[TRUNC:%[0-9]+]]:vgpr(s1) = G_TRUNC [[COPY]](s32)
+  ; CHECK:   [[COPY1:%[0-9]+]]:sgpr(s1) = COPY [[TRUNC]](s1)
+  ; CHECK:   G_BRCOND [[COPY1]](s1), %bb.1
+  ; CHECK: bb.1:
+  bb.0.entry:
+    successors: %bb.1
+    liveins: $vgpr0
+    %0:_(s32) = COPY $vgpr0
+    %1:_(s1) = G_TRUNC %0
+    G_BRCOND %1, %bb.1
+
+  bb.1:
+...
+
+
+# The terminator that needs handling is the only instruction in the
+# block.
+
+---
+name: empty_block_vgpr_brcond
+legalized: true
+body:             |
+  ; CHECK-LABEL: name: empty_block_vgpr_brcond
+  ; CHECK: bb.0.entry:
+  ; CHECK:   successors: %bb.1(0x80000000)
+  ; CHECK:   [[COPY:%[0-9]+]]:vgpr(s32) = COPY $vgpr0
+  ; CHECK:   [[TRUNC:%[0-9]+]]:vgpr(s1) = G_TRUNC [[COPY]](s32)
+  ; CHECK: bb.1:
+  ; CHECK:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
+  ; CHECK:   [[COPY1:%[0-9]+]]:sgpr(s1) = COPY [[TRUNC]](s1)
+  ; CHECK:   G_BRCOND [[COPY1]](s1), %bb.1
+  ; CHECK: bb.2:
+  bb.0.entry:
+    successors: %bb.1
+    liveins: $vgpr0
+    %0:_(s32) = COPY $vgpr0
+    %1:_(s1) = G_TRUNC %0
+
+  bb.1:
+    G_BRCOND %1, %bb.1
+
+  bb.2:
+...
+
+
+# Make sure the first instruction in the block isn't skipped.
+---
+name: copy_first_inst_brcond
+legalized: true
+body:             |
+  ; CHECK-LABEL: name: copy_first_inst_brcond
+  ; CHECK: bb.0.entry:
+  ; CHECK:   successors: %bb.1(0x80000000)
+  ; CHECK:   [[COPY:%[0-9]+]]:vgpr(s32) = COPY $vgpr0
+  ; CHECK: bb.1:
+  ; CHECK:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
+  ; CHECK:   [[TRUNC:%[0-9]+]]:vgpr(s1) = G_TRUNC [[COPY]](s32)
+  ; CHECK:   [[COPY1:%[0-9]+]]:sgpr(s1) = COPY [[TRUNC]](s1)
+  ; CHECK:   G_BRCOND [[COPY1]](s1), %bb.1
+  ; CHECK: bb.2:
+  bb.0.entry:
+    successors: %bb.1
+    liveins: $vgpr0
+    %0:_(s32) = COPY $vgpr0
+
+  bb.1:
+    %1:_(s1) = G_TRUNC %0
+    G_BRCOND %1, %bb.1
+
+  bb.2:
+...
+
+# Extra instruction separates brcond from the condition def
+---
+name: copy_middle_inst_brcond
+legalized: true
+body:             |
+  ; CHECK-LABEL: name: copy_middle_inst_brcond
+  ; CHECK: bb.0.entry:
+  ; CHECK:   successors: %bb.1(0x80000000)
+  ; CHECK:   [[COPY:%[0-9]+]]:vgpr(s32) = COPY $vgpr0
+  ; CHECK: bb.1:
+  ; CHECK:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
+  ; CHECK:   [[TRUNC:%[0-9]+]]:vgpr(s1) = G_TRUNC [[COPY]](s32)
+  ; CHECK:   S_NOP 0
+  ; CHECK:   [[COPY1:%[0-9]+]]:sgpr(s1) = COPY [[TRUNC]](s1)
+  ; CHECK:   G_BRCOND [[COPY1]](s1), %bb.1
+  ; CHECK: bb.2:
+  bb.0.entry:
+    successors: %bb.1
+    liveins: $vgpr0
+    %0:_(s32) = COPY $vgpr0
+
+  bb.1:
+    %1:_(s1) = G_TRUNC %0
+    S_NOP 0
+    G_BRCOND %1, %bb.1
+
+  bb.2:
+...




More information about the llvm-commits mailing list