[PATCH] D106891: [AMDGPU] [Remarks] Emit optimization remarks when an FP atomic instruction is converted into a CAS loop or unsafe hardware instruction for GFX90A

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: clang/test/CodeGenOpenCL/fp-atomics-optremarks-gfx90a.cl:25
+// GFX90A-HW-LABEL: test_atomic_add
+// GFX90A-HW:   global_atomic_add_f64
+float test_atomic_add(global atomic_double *d, double a) {
----------------
Should check the operands etc, this isn't enough to see which version was selected


================
Comment at: llvm/lib/CodeGen/AtomicExpandPass.cpp:585
+  bool UnsafeFPAtomicFlag = RMW->getFunction()
+                                ->getFnAttribute("amdgpu-unsafe-fp-atomics")
+                                .getValueAsBool();
----------------
Cannot check the AMDGPU attribute here


================
Comment at: llvm/lib/CodeGen/AtomicExpandPass.cpp:590
+      OptimizationRemark Remark(DEBUG_TYPE, "Passed", RMW->getFunction());
+      Remark << "An atomic instruction was expanded into a CAS "
+                "loop.";
----------------
Maybe CAS should be spelled out


================
Comment at: llvm/lib/CodeGen/AtomicExpandPass.cpp:591
+      Remark << "An atomic instruction was expanded into a CAS "
+                "loop.";
+      return Remark;
----------------
I think remark messages aren't supposed to end in punctuation


================
Comment at: llvm/lib/CodeGen/AtomicExpandPass.cpp:598
+      OptimizationRemark Remark(DEBUG_TYPE, "Passed", RMW->getFunction());
+      Remark << "An unsafe hardware instruction was generated.";
+      return Remark;
----------------
Unsafe is misleading, plus this is being too specific to AMDGPU


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891