[PATCH] D15543: Use Flat For 64-bit Global Buffer

[Tom Stellard via llvm-commits]

tstellarAMD added a comment.

The commit message should be prefixed with: AMDGPU/SI:


================
Comment at: lib/Target/AMDGPU/AMDGPU.td:240
@@ -234,3 +239,3 @@
          FeatureWavefrontSize64, FeatureGCN, FeatureFlatAddressSpace,
-         FeatureGCN1Encoding, FeatureCIInsts]>;
+         FeatureFlatForGlobal, FeatureGCN1Encoding, FeatureCIInsts]>;
 
----------------
We don't want to enable FeatureFlatForGlobal globally.  It should only be used for HSA.

================
Comment at: lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp:417-418
@@ -416,4 +416,4 @@
 
-  if (VCCUsed)
+  if (VCCUsed || FlatUsed)
     MaxSGPR += 2;
 
----------------
I think this fix should be in a separate patch.

================
Comment at: lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp:995-998
@@ -994,4 +994,6 @@
 
   // addr64 bit was removed for volcanic islands.
-  if (Subtarget->getGeneration() >= AMDGPUSubtarget::VOLCANIC_ISLANDS)
+  // Or Subtarget prefers to use flat instruction.
+  if (Subtarget->getGeneration() >= AMDGPUSubtarget::VOLCANIC_ISLANDS ||
+      Subtarget->useFlatForGlobal())
     return false;
----------------
You should move the check for useFlatForGlobal() into SelectMUBUF() that way you only need to add it in one place.

================
Comment at: lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp:1064-1068
@@ -1061,2 +1063,7 @@
                                            SDValue &TFE) const {
+
+  // Subtarget prefers to use flat instruction.
+  if (Subtarget->useFlatForGlobal())
+    return false;
+
   SDValue Ptr, VAddr, Offen, Idxen, Addr64;
----------------
You won't need this once you have the check in SelectMUBUF().

================
Comment at: lib/Target/AMDGPU/AMDGPUSubtarget.h:73
@@ -72,2 +72,3 @@
   bool FlatAddressSpace;
+  bool FlatForGlobal;
   bool EnableIRStructurizer;
----------------
You need to initialize this value to something in the AMDGPUSubtarget constructor.  'true' when targeting HSA, 'false' otherwise.


================
Comment at: lib/Target/AMDGPU/VIInstructions.td:107-112
@@ -106,4 +106,8 @@
 //===----------------------------------------------------------------------===//
 
-let Predicates = [isVI] in {
+def useFlatForGlobal : Predicate <
+  "Subtarget->useFlatForGlobal() || "
+  "Subtarget->getGeneration() >= AMDGPUSubtarget::VOLCANIC_ISLANDS">;
+
+let Predicates = [useFlatForGlobal] in {
 
----------------
SInce the patterns below this will be used for both CI and VI now, they should be moved into CIInstructions.td

================
Comment at: test/CodeGen/AMDGPU/ci-use-flat-for-global.ll:1-2
@@ +1,3 @@
+; RUN: llc < %s -mtriple=amdgcn--amdhsa -mcpu=kaveri | FileCheck %s
+
+
----------------
Should add a RUN line for non-hsa: -march=amdgcn instead of -mtriple=amdgcn--amdhsa to make sure we are still using the buffer instructions.


http://reviews.llvm.org/D15543