[PATCH] D22838: AMDGPU/SI: Implement amdgcn image intrinsics

[Tom Stellard via llvm-commits]

tstellarAMD added a comment.

Maybe we should define these intrinsics


================
Comment at: include/llvm/IR/IntrinsicsAMDGPU.td:193-198
@@ -192,1 +192,8 @@
 
+//=======================================================================
+// flags is a 32-bit immediate to encode the flags for MIMG instructions.
+// UNORM = flags[0]
+// GLC   = flags[1]
+// SLC   = flags[2]
+// R128  = flags[3]
+// TFE   = flags[4]
----------------
> Can we just not do this kind of change, please? I don't see how it improves anything, it's inconsistent with the ISA description which has the flags separate, and it'll require an annoying flag day synchronization with Mesa.

I really prefer using a mask over having to create an entire new set of intrinsics each time we have to add a new bit.  I think we have two solutions here:

1. As Matt has suggested, keep mesa the same, and have the auto-upgrader in LLVM change from the old intrinsics to  the new mask style intrinsics.  Although, with this solution, I think we'd still eventually want/need Mesa to start using the mask version.

2. Define the intrinsics as var_arg.  This would allow us to add to i1 operands without breaking the existing operands.  I'm just not sure how well var_arg intrinsics are supported.


https://reviews.llvm.org/D22838