[PATCH] D63619: [AMDGPU] hazard recognizer for fp atomic to s_denorm_mode

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
Comment at: lib/Target/AMDGPU/GCNHazardRecognizer.cpp:1157-1163
+      return false;
+    const MachineMemOperand *MemOp = *I->memoperands_begin();
+    if (!MemOp->isAtomic())
+      return false;
+    const Value *V = MemOp->getValue();
+    return !V || !V->getType()->isPointerTy() || // assume worst.
+           V->getType()->getPointerElementType()->isFloatTy();
----------------
arsenm wrote:
> This whole approach to identifying the atomic opcodes doesn't really work. You can't rely on any of the information in the memoperand, especially the IR pointee type. We do have an SIInstrFlags::maybeAtomic, but I think you need to check the opcode for if it's an FP one
maybeAtomic will catch too much, including atomic loads and stores. I can use "AMDGPU::getAtomicRetOp(I->getOpcode()) != -1" to catch it.
However listing all possible opcodes is error prone and not future proof, especially given that many of them are just commented out currently. If you believe this value is unreliable I will have to use another bit in the MI flags. Does it sound good?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63619/new/

https://reviews.llvm.org/D63619