[PATCH] D24623: [AMDGPU] Implement memory model

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: lib/Target/AMDGPU/SIInstructions.td:515
 
+def ATOMIC_FENCE : InstSI<
+  (outs), (ins i32imm:$ordering, i32imm:$scope),
----------------
There's an SPseduoInst or something like that which will avoid needing to set any of these bits


================
Comment at: lib/Target/AMDGPU/SIInstructions.td:519
+  [(atomic_fence (i32 imm:$ordering), (i32 imm:$scope))]> {
+  let hasSideEffects = 1;
+  let isCodeGenOnly = 1;
----------------
I don't think this is necessary, only mayLoad and mayStore


================
Comment at: lib/Target/AMDGPU/SIMemoryLegalizer.cpp:194-195
+    const MachineBasicBlock::iterator &MI) const {
+  if (!MI->hasOneMemOperand())
+    return false;
+
----------------
If there are no memory operands, this should still work and be handled as conservatively as possible


https://reviews.llvm.org/D24623