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

[Tom Stellard via llvm-commits]

tstellarAMD added inline comments.

================
Comment at: lib/Target/AMDGPU/AMDGPUSubtarget.cpp:102-105
@@ -101,3 +101,6 @@
   }
+
+  if (getGeneration() == AMDGPUSubtarget::SEA_ISLANDS && isAmdHsaOS())
+    FlatForGlobal = true;
 }
 
----------------
You need to set this value before you call initializeSubtargetDependencies() otherwise users won't be able to toggle this using -mattr=flat-for-global.

================
Comment at: test/CodeGen/AMDGPU/ci-use-flat-for-global.ll:1-2
@@ +1,3 @@
+; RUN: llc < %s -mtriple=amdgcn--amdhsa -mcpu=kaveri | FileCheck -check-prefix=CI-HSA %s
+; RUN: llc < %s -mtriple=amdgcn -mcpu=kaveri | FileCheck -check-prefix=CI-NOHSA %s
+
----------------
I would add two more run line here to test setting the non-default value for each target using -mattr=

================
Comment at: test/CodeGen/AMDGPU/ci-use-flat-for-global.ll:6
@@ +5,3 @@
+; CI-HSA: flat_store_dword
+define spir_kernel void @test_store(i32 %arg, i32 addrspace(1)* nocapture %arg1, i32 %arg2) #0 {
+bb:
----------------
You can drop the spir_kernel calling convention here.

================
Comment at: test/CodeGen/AMDGPU/ci-use-flat-for-global.ll:29-36
@@ +28,10 @@
+
+bb25:                                             ; preds = %bb
+  store i32 3, i32 addrspace(1)* %tmp24, align 4
+  br label %bb26
+
+bb26:                                             ; preds = %bb25, %bb
+  store i32 1, i32 addrspace(1)* %tmp24, align 4
+  ret void
+}
+
----------------
Since we are disabling buffer instructions globally, I think you can use a much more simple test case:

  void @test(i32 addrspace(1)* %out) {
  entry:
    store i32 0, i32 addrspace(1)* %out
    ret void
  }


http://reviews.llvm.org/D15543