[PATCH] D12067: AMDGPU: Refactor exp instructions

[Nicolai Hähnle via llvm-commits]

nhaehnle added a comment.

The TODOs need to be addressed, and for the FIXMEs in the intrinsic definition, I'd say we're better off just adding a real llvm.amdgcn.export (+ .done?) intrinsic in the future. Some more inline comments, but overall I think this patch is fine.



================
Comment at: lib/Target/AMDGPU/AMDGPUInstrInfo.td:288-290
+def AMDGPUexport_done: SDNode<"AMDGPUISD::EXPORT_DONE", AMDGPUExportOp,
+  [SDNPHasChain, SDNPMayStore, SDNPSideEffect]>;
+
----------------
SDNPMayLoad instead of SDNPSideEffect for consistency.


================
Comment at: lib/Target/AMDGPU/SIInstrInfo.td:530-531
 
-  def _vi : EXPCommon, SIMCInstr <"exp", SIEncodingFamily.VI>, EXPe_vi {
-    let DecoderNamespace="VI";
-    let DisableDecoder = DisableVIDecoder;
+// Split EXP instruction into EXP and EXP_DONE so we can set
+// hasSideEffects for done=1.
+multiclass EXP_m<bit done, SDPatternOperator node> {
----------------
s/hasSideEffects/mayLoad/


https://reviews.llvm.org/D12067