PATCH: R600: Enable IR structurizer by default for all Subtargets

Tom Stellard tom at stellard.net
Wed Nov 13 16:51:33 PST 2013


Hi,

The attached patches enable the IR structurizer by default for all
targets.  This fixes a number of failures in the OpenCV test suite.

Please Review.

-Tom
-------------- next part --------------
>From 0eca91c94dd153aeb7647c8d758f4ffbea666589 Mon Sep 17 00:00:00 2001
From: Tom Stellard <thomas.stellard at amd.com>
Date: Wed, 13 Nov 2013 15:02:54 -0800
Subject: [PATCH 1/4] R600: Use lower-case for EnableIRStructurizer feature

llc converts all values passed to -mattr= to lowercase, so this
enables us to toggle this feature when using llc.
---
 lib/Target/R600/AMDGPU.td | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/Target/R600/AMDGPU.td b/lib/Target/R600/AMDGPU.td
index f63617f..e9304c2 100644
--- a/lib/Target/R600/AMDGPU.td
+++ b/lib/Target/R600/AMDGPU.td
@@ -21,7 +21,7 @@ def FeatureDumpCode : SubtargetFeature <"DumpCode",
         "true",
         "Dump MachineInstrs in the CodeEmitter">;
 
-def FeatureIRStructurizer : SubtargetFeature <"EnableIRStructurizer",
+def FeatureIRStructurizer : SubtargetFeature <"enable-irstructurizer",
         "EnableIRStructurizer",
         "true",
         "Enable IR Structurizer">;
-- 
1.8.1.4

-------------- next part --------------
>From fb7c2e477bfcd6fad214238481bdcf0d313bbf22 Mon Sep 17 00:00:00 2001
From: Tom Stellard <thomas.stellard at amd.com>
Date: Wed, 13 Nov 2013 14:04:54 -0800
Subject: [PATCH 2/4] R600: Add a SubtargetFeatture for disabling the ifcvt
 pass.

This is useful when writing test cases for the AMDIL structurizer.
---
 lib/Target/R600/AMDGPU.td               | 5 +++++
 lib/Target/R600/AMDGPUSubtarget.cpp     | 5 +++++
 lib/Target/R600/AMDGPUSubtarget.h       | 2 ++
 lib/Target/R600/AMDGPUTargetMachine.cpp | 3 ++-
 4 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/lib/Target/R600/AMDGPU.td b/lib/Target/R600/AMDGPU.td
index e9304c2..37ff6b1 100644
--- a/lib/Target/R600/AMDGPU.td
+++ b/lib/Target/R600/AMDGPU.td
@@ -28,6 +28,11 @@ def FeatureIRStructurizer : SubtargetFeature <"enable-irstructurizer",
 
 // Target features
 
+def FeatureIfCvt : SubtargetFeature <"disable-ifcvt",
+        "EnableIfCvt",
+        "false",
+        "Disable the if conversion pass">;
+
 def FeatureFP64     : SubtargetFeature<"fp64",
         "FP64",
         "true",
diff --git a/lib/Target/R600/AMDGPUSubtarget.cpp b/lib/Target/R600/AMDGPUSubtarget.cpp
index 1e21c8e..4e97e6e 100644
--- a/lib/Target/R600/AMDGPUSubtarget.cpp
+++ b/lib/Target/R600/AMDGPUSubtarget.cpp
@@ -37,6 +37,7 @@ AMDGPUSubtarget::AMDGPUSubtarget(StringRef TT, StringRef CPU, StringRef FS) :
   FP64 = false;
   CaymanISA = false;
   EnableIRStructurizer = false;
+  EnableIfCvt = true;
   ParseSubtargetFeatures(GPU, FS);
   DevName = GPU;
 }
@@ -70,6 +71,10 @@ AMDGPUSubtarget::IsIRStructurizerEnabled() const {
   return EnableIRStructurizer;
 }
 bool
+AMDGPUSubtarget::isIfCvtEnabled() const {
+  return EnableIfCvt;
+}
+bool
 AMDGPUSubtarget::isTargetELF() const {
   return false;
 }
diff --git a/lib/Target/R600/AMDGPUSubtarget.h b/lib/Target/R600/AMDGPUSubtarget.h
index c08cd6a..4288d27 100644
--- a/lib/Target/R600/AMDGPUSubtarget.h
+++ b/lib/Target/R600/AMDGPUSubtarget.h
@@ -50,6 +50,7 @@ private:
   bool FP64;
   bool CaymanISA;
   bool EnableIRStructurizer;
+  bool EnableIfCvt;
 
   InstrItineraryData InstrItins;
 
@@ -66,6 +67,7 @@ public:
   bool hasHWFP64() const;
   bool hasCaymanISA() const;
   bool IsIRStructurizerEnabled() const;
+  bool isIfCvtEnabled() const;
 
   virtual bool enableMachineScheduler() const {
     return getGeneration() <= NORTHERN_ISLANDS;
diff --git a/lib/Target/R600/AMDGPUTargetMachine.cpp b/lib/Target/R600/AMDGPUTargetMachine.cpp
index b19277d..9186c9d 100644
--- a/lib/Target/R600/AMDGPUTargetMachine.cpp
+++ b/lib/Target/R600/AMDGPUTargetMachine.cpp
@@ -169,7 +169,8 @@ bool AMDGPUPassConfig::addPreSched2() {
 
   if (ST.getGeneration() <= AMDGPUSubtarget::NORTHERN_ISLANDS)
     addPass(createR600EmitClauseMarkers(*TM));
-  addPass(&IfConverterID);
+  if (ST.isIfCvtEnabled())
+    addPass(&IfConverterID);
   if (ST.getGeneration() <= AMDGPUSubtarget::NORTHERN_ISLANDS)
     addPass(createR600ClauseMergePass(*TM));
   return false;
-- 
1.8.1.4

-------------- next part --------------
>From 4b67218c65abcc76ab33e26a804884c335ab936d Mon Sep 17 00:00:00 2001
From: Tom Stellard <thomas.stellard at amd.com>
Date: Wed, 13 Nov 2013 14:06:13 -0800
Subject: [PATCH 3/4] R600: Fix a crash in the AMDILCFGStrucurizer

The ifPatternMatch() function was not correctly reporting the number
of matches in some cases.
---
 lib/Target/R600/AMDILCFGStructurizer.cpp | 13 +++----
 test/CodeGen/R600/structurize1.ll        | 62 ++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+), 6 deletions(-)
 create mode 100644 test/CodeGen/R600/structurize1.ll

diff --git a/lib/Target/R600/AMDILCFGStructurizer.cpp b/lib/Target/R600/AMDILCFGStructurizer.cpp
index beaaba8..507570f 100644
--- a/lib/Target/R600/AMDILCFGStructurizer.cpp
+++ b/lib/Target/R600/AMDILCFGStructurizer.cpp
@@ -1005,13 +1005,14 @@ int AMDGPUCFGStructurizer::ifPatternMatch(MachineBasicBlock *MBB) {
     return 0;
 
   assert(isCondBranch(BranchMI));
+  int NumMatch = 0;
 
   MachineBasicBlock *TrueMBB = getTrueBranch(BranchMI);
-  serialPatternMatch(TrueMBB);
-  ifPatternMatch(TrueMBB);
+  NumMatch += serialPatternMatch(TrueMBB);
+  NumMatch += ifPatternMatch(TrueMBB);
   MachineBasicBlock *FalseMBB = getFalseBranch(MBB, BranchMI);
-  serialPatternMatch(FalseMBB);
-  ifPatternMatch(FalseMBB);
+  NumMatch += serialPatternMatch(FalseMBB);
+  NumMatch += ifPatternMatch(FalseMBB);
   MachineBasicBlock *LandBlk;
   int Cloned = 0;
 
@@ -1040,7 +1041,7 @@ int AMDGPUCFGStructurizer::ifPatternMatch(MachineBasicBlock *MBB) {
     && isSameloopDetachedContbreak(FalseMBB, TrueMBB)) {
     LandBlk = *TrueMBB->succ_begin();
   } else {
-    return handleJumpintoIf(MBB, TrueMBB, FalseMBB);
+    return NumMatch + handleJumpintoIf(MBB, TrueMBB, FalseMBB);
   }
 
   // improveSimpleJumpinfoIf can handle the case where landBlk == NULL but the
@@ -1068,7 +1069,7 @@ int AMDGPUCFGStructurizer::ifPatternMatch(MachineBasicBlock *MBB) {
 
   numClonedBlock += Cloned;
 
-  return 1 + Cloned;
+  return 1 + Cloned + NumMatch;
 }
 
 int AMDGPUCFGStructurizer::loopendPatternMatch() {
diff --git a/test/CodeGen/R600/structurize1.ll b/test/CodeGen/R600/structurize1.ll
new file mode 100644
index 0000000..8c10301
--- /dev/null
+++ b/test/CodeGen/R600/structurize1.ll
@@ -0,0 +1,62 @@
+; RUN: llc < %s -march=r600 -mattr=disable-ifcvt -mcpu=redwood | FileCheck %s
+
+; This tests for abug where the AMDILCFGStructurizer was crashing on loops
+; like this:
+;
+; for (i = 0; i < x; i++) {
+;   if (cond0) {
+;     if (cond1) {
+;
+;     } else {
+;
+;     }
+;     if (cond2) {
+;
+;     }
+;   }
+; }
+
+; CHECK-LABEL: @if_inside_loop
+; CHECK: LOOP_START_DX10
+; CHECK: END_LOOP
+define void @if_inside_loop(i32 addrspace(1)* %out, i32 %a, i32 %b, i32 %c, i32 %d) {
+entry:
+  br label %for.body
+
+for.body:
+  %0 = phi i32 [0, %entry], [%inc, %for.inc]
+  %val = phi i32 [0, %entry], [%val.for.inc, %for.inc]
+  %inc = add i32 %0, 1
+  %1 = icmp ult i32 10, %a
+  br i1 %1, label %for.inc, label %if.then
+
+if.then:
+  %2 = icmp ne i32 0, %b
+  br i1 %2, label %if.then.true, label %if.then.false
+
+if.then.true:
+  %3 = add i32 %a, %val
+  br label %if
+
+if.then.false:
+  %4 = mul i32 %a, %val
+  br label %if
+
+if:
+  %val.if = phi i32 [%3, %if.then.true], [%4, %if.then.false]
+  %5 = icmp ne i32 0, %c
+  br i1 %5, label %if.true, label %for.inc
+
+if.true:
+  %6 = add i32 %a, %val.if
+  br label %for.inc
+
+for.inc:
+  %val.for.inc = phi i32 [%val, %for.body], [%val.if, %if], [%6, %if.true]
+  %7 = icmp ne i32 0, %d
+  br i1 %7, label %for.body, label %exit
+
+exit:
+  store i32 %val.for.inc, i32 addrspace(1)* %out
+  ret void
+}
-- 
1.8.1.4

-------------- next part --------------
>From 8f1e940007331d82c1c3bac91a94106fcccbc860 Mon Sep 17 00:00:00 2001
From: Tom Stellard <thomas.stellard at amd.com>
Date: Wed, 13 Nov 2013 12:00:08 -0800
Subject: [PATCH 4/4] R600: Enable the IR structurizer by default

---
 lib/Target/R600/AMDGPU.td               | 6 +++---
 lib/Target/R600/AMDGPUSubtarget.cpp     | 2 +-
 lib/Target/R600/AMDGPUTargetMachine.cpp | 3 +--
 test/CodeGen/R600/structurize.ll        | 2 +-
 4 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/lib/Target/R600/AMDGPU.td b/lib/Target/R600/AMDGPU.td
index 37ff6b1..182235b 100644
--- a/lib/Target/R600/AMDGPU.td
+++ b/lib/Target/R600/AMDGPU.td
@@ -21,10 +21,10 @@ def FeatureDumpCode : SubtargetFeature <"DumpCode",
         "true",
         "Dump MachineInstrs in the CodeEmitter">;
 
-def FeatureIRStructurizer : SubtargetFeature <"enable-irstructurizer",
+def FeatureIRStructurizer : SubtargetFeature <"disable-irstructurizer",
         "EnableIRStructurizer",
-        "true",
-        "Enable IR Structurizer">;
+        "false",
+        "Disable IR Structurizer">;
 
 // Target features
 
diff --git a/lib/Target/R600/AMDGPUSubtarget.cpp b/lib/Target/R600/AMDGPUSubtarget.cpp
index 4e97e6e..061793a 100644
--- a/lib/Target/R600/AMDGPUSubtarget.cpp
+++ b/lib/Target/R600/AMDGPUSubtarget.cpp
@@ -36,7 +36,7 @@ AMDGPUSubtarget::AMDGPUSubtarget(StringRef TT, StringRef CPU, StringRef FS) :
   Gen = AMDGPUSubtarget::R600;
   FP64 = false;
   CaymanISA = false;
-  EnableIRStructurizer = false;
+  EnableIRStructurizer = true;
   EnableIfCvt = true;
   ParseSubtargetFeatures(GPU, FS);
   DevName = GPU;
diff --git a/lib/Target/R600/AMDGPUTargetMachine.cpp b/lib/Target/R600/AMDGPUTargetMachine.cpp
index 9186c9d..bc4f5d7 100644
--- a/lib/Target/R600/AMDGPUTargetMachine.cpp
+++ b/lib/Target/R600/AMDGPUTargetMachine.cpp
@@ -125,8 +125,7 @@ bool
 AMDGPUPassConfig::addPreISel() {
   const AMDGPUSubtarget &ST = TM->getSubtarget<AMDGPUSubtarget>();
   addPass(createFlattenCFGPass());
-  if (ST.IsIRStructurizerEnabled() ||
-      ST.getGeneration() > AMDGPUSubtarget::NORTHERN_ISLANDS)
+  if (ST.IsIRStructurizerEnabled())
     addPass(createStructurizeCFGPass());
   if (ST.getGeneration() > AMDGPUSubtarget::NORTHERN_ISLANDS) {
     addPass(createSinkingPass());
diff --git a/test/CodeGen/R600/structurize.ll b/test/CodeGen/R600/structurize.ll
index b955619..c2acd93 100644
--- a/test/CodeGen/R600/structurize.ll
+++ b/test/CodeGen/R600/structurize.ll
@@ -1,4 +1,4 @@
-; RUN: llc < %s -march=r600 -mcpu=redwood | FileCheck %s
+; RUN: llc < %s -march=r600 -mcpu=redwood -mattr=disable-irstructurizer | FileCheck %s
 ; Test case for a crash in the AMDILCFGStructurizer from a CFG like this:
 ;
 ;                            entry
-- 
1.8.1.4



More information about the llvm-commits mailing list