AMDGPU/R600: Add PatFrags for selecting the correct vtx id for loads

[• tstellarAMD]

Summary

This moves of the r600 logic out of isGlobalLoad() and into the
TableGen files.

[arsenm]

LGTM, but I still thinking checking GetUnderlyingObject for selection is broken. It should at least check for it returning null which is common

[jvesely]

In D21710#467057, @arsenm wrote:

LGTM, but I still thinking checking GetUnderlyingObject for selection is broken. It should at least check for it returning null which is common

What are the problems with GetUnderLyingObject? Don't CLC restrictions shield us from potentially failing cases (the only objects are initialized in constant AS)?

with or without renaming the instructions LGTM.

lib/Target/AMDGPU/EvergreenInstructions.td

239 ↗

(On Diff #61867)

I think it'd also make sense to rename the instructions:
VTX_READ_GLOBAL -> VTX_READ_ID1. so the vtx to AS assignment is reduced to one place (the pattern).

[arsenm]

In D21710#467269, @jvesely wrote:

In D21710#467057, @arsenm wrote:

LGTM, but I still thinking checking GetUnderlyingObject for selection is broken. It should at least check for it returning null which is common

What are the problems with GetUnderLyingObject? Don't CLC restrictions shield us from potentially failing cases (the only objects are initialized in constant AS)?

with or without renaming the instructions LGTM.

You could loop over them and have an unanalyzed select or phi on the pointer which will fail to find the object. It also has a search depth lint, and it's a pretty heavyweight check for a select pattern

[arsenm]

Checking IR values in the DAG also doesn't seem like the best idea although AA does

[jvesely]

In D21710#467400, @arsenm wrote:

Checking IR values in the DAG also doesn't seem like the best idea although AA does

OK, thanks. I guess the solution for r600 is:
move rodata out of code segment.
introduce constant AS pool in mesa, make sure that rodata is at the beginning of that pool (also takes care of NULL address for user buffers)
allocate __constant user buffers from the new pool.

llvm/trunk/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp

if (CbId == -1)
     return checkType(MemVal, AMDGPUAS::CONSTANT_ADDRESS);
 
   return checkType(MemVal, AMDGPUAS::CONSTANT_BUFFER_0 + CbId);
 }
 
 bool AMDGPUDAGToDAGISel::isGlobalLoad(const MemSDNode *N) const {
   if (!N->readMem())
     return false;
-  if (N->getAddressSpace() == AMDGPUAS::CONSTANT_ADDRESS) {
-    if (Subtarget->getGeneration() < AMDGPUSubtarget::SOUTHERN_ISLANDS)
-      return !isa<GlobalValue>(GetUnderlyingObject(
-          N->getMemOperand()->getValue(), CurDAG->getDataLayout()));
-  }
 
   return checkType(N->getMemOperand()->getValue(), AMDGPUAS::GLOBAL_ADDRESS);
 }
 
 bool AMDGPUDAGToDAGISel::isLocalLoad(const  LoadSDNode *N) const {
   return checkType(N->getMemOperand()->getValue(), AMDGPUAS::LOCAL_ADDRESS);
 }
 

llvm/trunk/lib/Target/AMDGPU/CaymanInstructions.td

def VTX_READ_PARAM_128_cm : VTX_READ_128_cm <0,
   [(set v4i32:$dst_gpr, (load_param ADDRVTX_READ:$src_gpr))]
 >;
 
 //===----------------------------------------------------------------------===//
 // VTX Read from global memory space
 //===----------------------------------------------------------------------===//
 
 // 8-bit reads
-def VTX_READ_GLOBAL_8_cm : VTX_READ_8_cm <1,
-  [(set i32:$dst_gpr, (az_extloadi8_global ADDRVTX_READ:$src_gpr))]
+def VTX_READ_ID1_8_cm : VTX_READ_8_cm <1,
+  [(set i32:$dst_gpr, (vtx_id1_az_extloadi8 ADDRVTX_READ:$src_gpr))]
 >;
 
 // 16-bit reads
-def VTX_READ_GLOBAL_16_cm : VTX_READ_16_cm <1,
-  [(set i32:$dst_gpr, (az_extloadi16_global ADDRVTX_READ:$src_gpr))]
+def VTX_READ_ID1_16_cm : VTX_READ_16_cm <1,
+  [(set i32:$dst_gpr, (vtx_id1_az_extloadi16 ADDRVTX_READ:$src_gpr))]
 >;
 
 // 32-bit reads
-def VTX_READ_GLOBAL_32_cm : VTX_READ_32_cm <1,
-  [(set i32:$dst_gpr, (global_load ADDRVTX_READ:$src_gpr))]
+def VTX_READ_ID1_32_cm : VTX_READ_32_cm <1,
+  [(set i32:$dst_gpr, (vtx_id1_load ADDRVTX_READ:$src_gpr))]
 >;
 
 // 64-bit reads
-def VTX_READ_GLOBAL_64_cm : VTX_READ_64_cm <1,
-  [(set v2i32:$dst_gpr, (global_load ADDRVTX_READ:$src_gpr))]
+def VTX_READ_ID1_64_cm : VTX_READ_64_cm <1,
+  [(set v2i32:$dst_gpr, (vtx_id1_load ADDRVTX_READ:$src_gpr))]
 >;
 
 // 128-bit reads
-def VTX_READ_GLOBAL_128_cm : VTX_READ_128_cm <1,
-  [(set v4i32:$dst_gpr, (global_load ADDRVTX_READ:$src_gpr))]
+def VTX_READ_ID1_128_cm : VTX_READ_128_cm <1,
+  [(set v4i32:$dst_gpr, (vtx_id1_load ADDRVTX_READ:$src_gpr))]
 >;
 
 // 8-bit reads
-def VTX_READ_CONSTANT_8_cm : VTX_READ_8_cm <2,
-  [(set i32:$dst_gpr, (az_extloadi8_constant ADDRVTX_READ:$src_gpr))]
+def VTX_READ_ID2_8_cm : VTX_READ_8_cm <2,
+  [(set i32:$dst_gpr, (vtx_id2_az_extloadi8 ADDRVTX_READ:$src_gpr))]
 >;
 
 // 16-bit reads
-def VTX_READ_CONSTANT_16_cm : VTX_READ_16_cm <2,
-  [(set i32:$dst_gpr, (az_extloadi16_constant ADDRVTX_READ:$src_gpr))]
+def VTX_READ_ID2_16_cm : VTX_READ_16_cm <2,
+  [(set i32:$dst_gpr, (vtx_id2_az_extloadi16 ADDRVTX_READ:$src_gpr))]
 >;
 
 // 32-bit reads
-def VTX_READ_CONSTANT_32_cm : VTX_READ_32_cm <2,
-  [(set i32:$dst_gpr, (constant_load ADDRVTX_READ:$src_gpr))]
+def VTX_READ_ID2_32_cm : VTX_READ_32_cm <2,
+  [(set i32:$dst_gpr, (vtx_id2_load ADDRVTX_READ:$src_gpr))]
 >;
 
 // 64-bit reads
-def VTX_READ_CONSTANT_64_cm : VTX_READ_64_cm <2,
-  [(set v2i32:$dst_gpr, (constant_load ADDRVTX_READ:$src_gpr))]
+def VTX_READ_ID2_64_cm : VTX_READ_64_cm <2,
+  [(set v2i32:$dst_gpr, (vtx_id2_load ADDRVTX_READ:$src_gpr))]
 >;
 
 // 128-bit reads
-def VTX_READ_CONSTANT_128_cm : VTX_READ_128_cm <2,
-  [(set v4i32:$dst_gpr, (constant_load ADDRVTX_READ:$src_gpr))]
+def VTX_READ_ID2_128_cm : VTX_READ_128_cm <2,
+  [(set v4i32:$dst_gpr, (vtx_id2_load ADDRVTX_READ:$src_gpr))]
 >;
 
 } // End isCayman
 

llvm/trunk/lib/Target/AMDGPU/EvergreenInstructions.td

def VTX_READ_PARAM_128_eg : VTX_READ_128_eg <0,
   [(set v4i32:$dst_gpr, (load_param ADDRVTX_READ:$src_gpr))]
 >;
 
 //===----------------------------------------------------------------------===//
 // VTX Read from global memory space
 //===----------------------------------------------------------------------===//
 
 // 8-bit reads
-def VTX_READ_GLOBAL_8_eg : VTX_READ_8_eg <1,
-  [(set i32:$dst_gpr, (az_extloadi8_global ADDRVTX_READ:$src_gpr))]
+def VTX_READ_ID1_8_eg : VTX_READ_8_eg <1,
+  [(set i32:$dst_gpr, (vtx_id1_az_extloadi8 ADDRVTX_READ:$src_gpr))]
 >;
 
 // 16-bit reads
-def VTX_READ_GLOBAL_16_eg : VTX_READ_16_eg <1,
-  [(set i32:$dst_gpr, (az_extloadi16_global ADDRVTX_READ:$src_gpr))]
+def VTX_READ_ID1_16_eg : VTX_READ_16_eg <1,
+  [(set i32:$dst_gpr, (vtx_id1_az_extloadi16 ADDRVTX_READ:$src_gpr))]
 >;
 
 // 32-bit reads
-def VTX_READ_GLOBAL_32_eg : VTX_READ_32_eg <1,
-  [(set i32:$dst_gpr, (global_load ADDRVTX_READ:$src_gpr))]
+def VTX_READ_ID1_32_eg : VTX_READ_32_eg <1,
+  [(set i32:$dst_gpr, (vtx_id1_load ADDRVTX_READ:$src_gpr))]
 >;
 
 // 64-bit reads
-def VTX_READ_GLOBAL_64_eg : VTX_READ_64_eg <1,
-  [(set v2i32:$dst_gpr, (global_load ADDRVTX_READ:$src_gpr))]
+def VTX_READ_ID1_64_eg : VTX_READ_64_eg <1,
+  [(set v2i32:$dst_gpr, (vtx_id1_load ADDRVTX_READ:$src_gpr))]
 >;
 
 // 128-bit reads
-def VTX_READ_GLOBAL_128_eg : VTX_READ_128_eg <1,
-  [(set v4i32:$dst_gpr, (global_load ADDRVTX_READ:$src_gpr))]
+def VTX_READ_ID1_128_eg : VTX_READ_128_eg <1,
+  [(set v4i32:$dst_gpr, (vtx_id1_load ADDRVTX_READ:$src_gpr))]
 >;
 
 // 8-bit reads
-def VTX_READ_CONSTANT_8_eg : VTX_READ_8_eg <2,
-  [(set i32:$dst_gpr, (az_extloadi8_constant ADDRVTX_READ:$src_gpr))]
+def VTX_READ_ID2_8_eg : VTX_READ_8_eg <2,
+  [(set i32:$dst_gpr, (vtx_id2_az_extloadi8 ADDRVTX_READ:$src_gpr))]
 >;
 
 // 16-bit reads
-def VTX_READ_CONSTANT_16_eg : VTX_READ_16_eg <2,
-  [(set i32:$dst_gpr, (az_extloadi16_constant ADDRVTX_READ:$src_gpr))]
+def VTX_READ_ID2_16_eg : VTX_READ_16_eg <2,
+  [(set i32:$dst_gpr, (vtx_id2_az_extloadi16 ADDRVTX_READ:$src_gpr))]
 >;
 
 // 32-bit reads
-def VTX_READ_CONSTANT_32_eg : VTX_READ_32_eg <2,
-  [(set i32:$dst_gpr, (constant_load ADDRVTX_READ:$src_gpr))]
+def VTX_READ_ID2_32_eg : VTX_READ_32_eg <2,
+  [(set i32:$dst_gpr, (vtx_id2_load ADDRVTX_READ:$src_gpr))]
 >;
 
 // 64-bit reads
-def VTX_READ_CONSTANT_64_eg : VTX_READ_64_eg <2,
-  [(set v2i32:$dst_gpr, (constant_load ADDRVTX_READ:$src_gpr))]
+def VTX_READ_ID2_64_eg : VTX_READ_64_eg <2,
+  [(set v2i32:$dst_gpr, (vtx_id2_load ADDRVTX_READ:$src_gpr))]
 >;
 
 // 128-bit reads
-def VTX_READ_CONSTANT_128_eg : VTX_READ_128_eg <2,
-  [(set v4i32:$dst_gpr, (constant_load ADDRVTX_READ:$src_gpr))]
+def VTX_READ_ID2_128_eg : VTX_READ_128_eg <2,
+  [(set v4i32:$dst_gpr, (vtx_id2_load ADDRVTX_READ:$src_gpr))]
 >;
 
 } // End Predicates = [isEG]
 
 //===----------------------------------------------------------------------===//
 // Evergreen / Cayman Instructions
 //===----------------------------------------------------------------------===//
 

llvm/trunk/lib/Target/AMDGPU/R600Instructions.td

class LoadParamFrag <PatFrag load_type> : PatFrag <
   (ops node:$ptr), (load_type node:$ptr),
   [{ return isConstantLoad(dyn_cast<LoadSDNode>(N), 0); }]
 >;
 
 def load_param : LoadParamFrag<load>;
 def load_param_exti8 : LoadParamFrag<az_extloadi8>;
 def load_param_exti16 : LoadParamFrag<az_extloadi16>;
 
+class LoadVtxId1 <PatFrag load> : PatFrag <
+  (ops node:$ptr), (load node:$ptr), [{
+  const MemSDNode *LD = cast<MemSDNode>(N);
+  return LD->getAddressSpace() == AMDGPUAS::GLOBAL_ADDRESS ||
+         (LD->getAddressSpace() == AMDGPUAS::CONSTANT_ADDRESS &&
+           !isa<GlobalValue>(GetUnderlyingObject(
+           LD->getMemOperand()->getValue(), CurDAG->getDataLayout())));
+}]>;
+
+def vtx_id1_az_extloadi8 : LoadVtxId1 <az_extloadi8>;
+def vtx_id1_az_extloadi16 : LoadVtxId1 <az_extloadi16>;
+def vtx_id1_load : LoadVtxId1 <load>;
+
+class LoadVtxId2 <PatFrag load> : PatFrag <
+  (ops node:$ptr), (load node:$ptr), [{
+  const MemSDNode *LD = cast<MemSDNode>(N);
+  return LD->getAddressSpace() == AMDGPUAS::CONSTANT_ADDRESS &&
+         isa<GlobalValue>(GetUnderlyingObject(
+         LD->getMemOperand()->getValue(), CurDAG->getDataLayout()));
+}]>;
+
+def vtx_id2_az_extloadi8 : LoadVtxId2 <az_extloadi8>;
+def vtx_id2_az_extloadi16 : LoadVtxId2 <az_extloadi16>;
+def vtx_id2_load : LoadVtxId2 <load>;
+
 def isR600 : Predicate<"Subtarget->getGeneration() <= R600Subtarget::R700">;
 
 def isR600toCayman
     : Predicate<
           "Subtarget->getGeneration() <= R600Subtarget::NORTHERN_ISLANDS">;
 
 //===----------------------------------------------------------------------===//
 // R600 SDNodes