[PATCH] D108150: [Remarks] [AMDGPU] Emit optimization remarks for atomics generating hardware instructions

[Anshil Gandhi via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG508b06699a39: [Remarks] [AMDGPU] Emit optimization remarks 
for atomics generating hardware… (authored by gandhi21299).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108150

Files:
  clang/test/CodeGenOpenCL/atomics-cas-remarks-gfx90a.cl
  clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl
  clang/test/CodeGenOpenCL/atomics-unsafe-hw-remarks-gfx90a.cl
  llvm/lib/CodeGen/AtomicExpandPass.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.cpp
  llvm/test/CodeGen/AMDGPU/atomics-cas-remarks-gfx90a.ll
  llvm/test/CodeGen/AMDGPU/atomics-hw-remarks-gfx90a.ll
  llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll

Index: llvm/test/CodeGen/AMDGPU/atomics-hw-remarks-gfx90a.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AMDGPU/atomics-hw-remarks-gfx90a.ll
@@ -0,0 +1,95 @@
+; RUN: llc -march=amdgcn -mcpu=gfx90a -verify-machineinstrs --pass-remarks=si-lower \
+; RUN:  %s -o - 2>&1 | FileCheck %s --check-prefix=GFX90A-HW
+
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope system due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope agent due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope workgroup due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope wavefront due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope singlethread due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope agent-one-as due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope workgroup-one-as due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope wavefront-one-as due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope singlethread-one-as due to an unsafe request.
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw:
+; GFX90A-HW:ds_add_f64 v2, v[0:1]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw(double addrspace(3)* %ptr) #0 {
+main_body:
+  %ret = atomicrmw fadd double addrspace(3)* %ptr, double 4.0 seq_cst
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_agent:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_agent(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("agent") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wg:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_wg(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("workgroup") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wavefront:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_wavefront(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("wavefront") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_single_thread:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_single_thread(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("singlethread") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_aoa:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_aoa(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("agent-one-as") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wgoa:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_wgoa(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("workgroup-one-as") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wfoa:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_wfoa(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = 

[PATCH] D108150: [Remarks] [AMDGPU] Emit optimization remarks for atomics generating hardware instructions

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 marked an inline comment as done.
gandhi21299 added a comment.

Thanks for the review, I will merge this in as soon as the CI passes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108150

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108150: [Remarks] [AMDGPU] Emit optimization remarks for atomics generating hardware instructions

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 updated this revision to Diff 367273.
gandhi21299 marked an inline comment as done.
gandhi21299 added a comment.

- code refactor


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108150

Files:
  clang/test/CodeGenOpenCL/atomics-cas-remarks-gfx90a.cl
  clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl
  clang/test/CodeGenOpenCL/atomics-unsafe-hw-remarks-gfx90a.cl
  llvm/lib/CodeGen/AtomicExpandPass.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.cpp
  llvm/test/CodeGen/AMDGPU/atomics-cas-remarks-gfx90a.ll
  llvm/test/CodeGen/AMDGPU/atomics-hw-remarks-gfx90a.ll
  llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll

Index: llvm/test/CodeGen/AMDGPU/atomics-hw-remarks-gfx90a.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AMDGPU/atomics-hw-remarks-gfx90a.ll
@@ -0,0 +1,95 @@
+; RUN: llc -march=amdgcn -mcpu=gfx90a -verify-machineinstrs --pass-remarks=si-lower \
+; RUN:  %s -o - 2>&1 | FileCheck %s --check-prefix=GFX90A-HW
+
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope system due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope agent due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope workgroup due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope wavefront due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope singlethread due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope agent-one-as due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope workgroup-one-as due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope wavefront-one-as due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope singlethread-one-as due to an unsafe request.
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw:
+; GFX90A-HW:ds_add_f64 v2, v[0:1]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw(double addrspace(3)* %ptr) #0 {
+main_body:
+  %ret = atomicrmw fadd double addrspace(3)* %ptr, double 4.0 seq_cst
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_agent:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_agent(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("agent") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wg:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_wg(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("workgroup") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wavefront:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_wavefront(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("wavefront") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_single_thread:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_single_thread(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("singlethread") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_aoa:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_aoa(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("agent-one-as") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wgoa:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_wgoa(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("workgroup-one-as") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wfoa:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_wfoa(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val 

[PATCH] D108150: [Remarks] [AMDGPU] Emit optimization remarks for atomics generating hardware instructions

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec added inline comments.



Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:12194
 
-  return (fpModeMatchesGlobalFPAtomicMode(RMW) ||
-  RMW->getFunction()
-  ->getFnAttribute("amdgpu-unsafe-fp-atomics")
-  .getValueAsString() == "true")
- ? AtomicExpansionKind::None
- : AtomicExpansionKind::CmpXChg;
+  if (fpModeMatchesGlobalFPAtomicMode(RMW) ||
+  RMW->getFunction()

```
  if (fpModeMatchesGlobalFPAtomicMode(RMW))
return AtomicExpansionKind::None;

  return RMW->getFunction()
 ->getFnAttribute("amdgpu-unsafe-fp-atomics")
 .getValueAsString() == "true"
 ? ReportUnsafeHWInst(AtomicExpansionKind::None)
 : AtomicExpansionKind::CmpXChg;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108150

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108150: [Remarks] [AMDGPU] Emit optimization remarks for atomics generating hardware instructions

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108150

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108150: [Remarks] [AMDGPU] Emit optimization remarks for atomics generating hardware instructions

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 updated this revision to Diff 367042.
gandhi21299 added a comment.

- corrected logic for ORE in SIISelLowering.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108150

Files:
  clang/test/CodeGenOpenCL/atomics-cas-remarks-gfx90a.cl
  clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl
  clang/test/CodeGenOpenCL/atomics-unsafe-hw-remarks-gfx90a.cl
  llvm/lib/CodeGen/AtomicExpandPass.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.cpp
  llvm/test/CodeGen/AMDGPU/atomics-cas-remarks-gfx90a.ll
  llvm/test/CodeGen/AMDGPU/atomics-hw-remarks-gfx90a.ll
  llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll

Index: llvm/test/CodeGen/AMDGPU/atomics-hw-remarks-gfx90a.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AMDGPU/atomics-hw-remarks-gfx90a.ll
@@ -0,0 +1,95 @@
+; RUN: llc -march=amdgcn -mcpu=gfx90a -verify-machineinstrs --pass-remarks=si-lower \
+; RUN:  %s -o - 2>&1 | FileCheck %s --check-prefix=GFX90A-HW
+
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope system due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope agent due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope workgroup due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope wavefront due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope singlethread due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope agent-one-as due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope workgroup-one-as due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope wavefront-one-as due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope singlethread-one-as due to an unsafe request.
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw:
+; GFX90A-HW:ds_add_f64 v2, v[0:1]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw(double addrspace(3)* %ptr) #0 {
+main_body:
+  %ret = atomicrmw fadd double addrspace(3)* %ptr, double 4.0 seq_cst
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_agent:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_agent(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("agent") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wg:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_wg(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("workgroup") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wavefront:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_wavefront(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("wavefront") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_single_thread:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_single_thread(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("singlethread") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_aoa:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_aoa(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("agent-one-as") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wgoa:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_wgoa(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("workgroup-one-as") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wfoa:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_wfoa(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("wavefront-one-as") 

[PATCH] D108150: [Remarks] [AMDGPU] Emit optimization remarks for atomics generating hardware instructions

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec added a comment.

In D108150#2950479 , @gandhi21299 
wrote:

> My understanding is that since we are reporting unsafe expansion into hw 
> instructions, `fpModeMatchesGlobalFPAtomicMode(RMW)` must be false to match 
> the logic.

Please run check-llvm before updating the patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108150

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108150: [Remarks] [AMDGPU] Emit optimization remarks for atomics generating hardware instructions

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 added a comment.

My understanding is that since we are reporting unsafe expansion into hw 
instructions, `fpModeMatchesGlobalFPAtomicMode(RMW)` must be false to match the 
logic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108150

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108150: [Remarks] [AMDGPU] Emit optimization remarks for atomics generating hardware instructions

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec added a comment.

In D108150#2950458 , @gandhi21299 
wrote:

> @rampitec Which part of the logic is wrong?

Still the same around LDS.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108150

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108150: [Remarks] [AMDGPU] Emit optimization remarks for atomics generating hardware instructions

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 added a comment.

@rampitec Which part of the logic is wrong?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108150

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108150: [Remarks] [AMDGPU] Emit optimization remarks for atomics generating hardware instructions

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec requested changes to this revision.
rampitec added a comment.
This revision now requires changes to proceed.

Logic is still wrong.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108150

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108150: [Remarks] [AMDGPU] Emit optimization remarks for atomics generating hardware instructions

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 updated this revision to Diff 367000.
gandhi21299 marked an inline comment as done.
gandhi21299 added a comment.

- corrected AtomicExpansionKind return value in SIISelLowering as pointed out 
by reviewer
- eliminated previous function declaration which is not defined/used anymore


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108150

Files:
  clang/test/CodeGenOpenCL/atomics-cas-remarks-gfx90a.cl
  clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl
  clang/test/CodeGenOpenCL/atomics-unsafe-hw-remarks-gfx90a.cl
  llvm/lib/CodeGen/AtomicExpandPass.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.cpp
  llvm/test/CodeGen/AMDGPU/atomics-cas-remarks-gfx90a.ll
  llvm/test/CodeGen/AMDGPU/atomics-hw-remarks-gfx90a.ll
  llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll

Index: llvm/test/CodeGen/AMDGPU/atomics-hw-remarks-gfx90a.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AMDGPU/atomics-hw-remarks-gfx90a.ll
@@ -0,0 +1,95 @@
+; RUN: llc -march=amdgcn -mcpu=gfx90a -verify-machineinstrs --pass-remarks=si-lower \
+; RUN:  %s -o - 2>&1 | FileCheck %s --check-prefix=GFX90A-HW
+
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope system due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope agent due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope workgroup due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope wavefront due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope singlethread due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope agent-one-as due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope workgroup-one-as due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope wavefront-one-as due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope singlethread-one-as due to an unsafe request.
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw:
+; GFX90A-HW:ds_add_f64 v2, v[0:1]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw(double addrspace(3)* %ptr) #0 {
+main_body:
+  %ret = atomicrmw fadd double addrspace(3)* %ptr, double 4.0 seq_cst
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_agent:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_agent(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("agent") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wg:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_wg(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("workgroup") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wavefront:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_wavefront(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("wavefront") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_single_thread:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_single_thread(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("singlethread") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_aoa:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_aoa(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("agent-one-as") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wgoa:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_wgoa(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("workgroup-one-as") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wfoa:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void 

[PATCH] D108150: [Remarks] [AMDGPU] Emit optimization remarks for atomics generating hardware instructions

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 updated this revision to Diff 366958.
gandhi21299 added a comment.

- requested changes from reviewer


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108150

Files:
  clang/test/CodeGenOpenCL/atomics-cas-remarks-gfx90a.cl
  clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl
  clang/test/CodeGenOpenCL/atomics-unsafe-hw-remarks-gfx90a.cl
  llvm/lib/CodeGen/AtomicExpandPass.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.h
  llvm/test/CodeGen/AMDGPU/atomics-cas-remarks-gfx90a.ll
  llvm/test/CodeGen/AMDGPU/atomics-hw-remarks-gfx90a.ll
  llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll

Index: llvm/test/CodeGen/AMDGPU/atomics-hw-remarks-gfx90a.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AMDGPU/atomics-hw-remarks-gfx90a.ll
@@ -0,0 +1,95 @@
+; RUN: llc -march=amdgcn -mcpu=gfx90a -verify-machineinstrs --pass-remarks=si-lower \
+; RUN:  %s -o - 2>&1 | FileCheck %s --check-prefix=GFX90A-HW
+
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope system due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope agent due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope workgroup due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope wavefront due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope singlethread due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope agent-one-as due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope workgroup-one-as due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope wavefront-one-as due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope singlethread-one-as due to an unsafe request.
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw:
+; GFX90A-HW:ds_add_f64 v2, v[0:1]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw(double addrspace(3)* %ptr) #0 {
+main_body:
+  %ret = atomicrmw fadd double addrspace(3)* %ptr, double 4.0 seq_cst
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_agent:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_agent(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("agent") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wg:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_wg(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("workgroup") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wavefront:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_wavefront(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("wavefront") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_single_thread:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_single_thread(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("singlethread") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_aoa:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_aoa(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("agent-one-as") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wgoa:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_wgoa(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("workgroup-one-as") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wfoa:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_wfoa(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val 

[PATCH] D108150: [Remarks] [AMDGPU] Emit optimization remarks for atomics generating hardware instructions

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec added inline comments.



Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:12195
 
+  if (!fpModeMatchesGlobalFPAtomicMode(RMW))
+return reportUnsafeHWInst(RMW, AtomicExpansionKind::None);

gandhi21299 wrote:
> rampitec wrote:
> > rampitec wrote:
> > > This is wrong. Condition is inverted and essentially tests should fail. 
> > > Make sure you can pass testing before posting a diff.
> > Unresolved.
> Remarks are produced if `fpModeMatchesGlobalFPAtomicMode(RMW) == false`
But you have changed what function was doing. It was returning CmpXChg.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108150

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108150: [Remarks] [AMDGPU] Emit optimization remarks for atomics generating hardware instructions

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 marked 3 inline comments as done.
gandhi21299 added inline comments.



Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:12195
 
+  if (!fpModeMatchesGlobalFPAtomicMode(RMW))
+return reportUnsafeHWInst(RMW, AtomicExpansionKind::None);

rampitec wrote:
> rampitec wrote:
> > This is wrong. Condition is inverted and essentially tests should fail. 
> > Make sure you can pass testing before posting a diff.
> Unresolved.
Remarks are produced if `fpModeMatchesGlobalFPAtomicMode(RMW) == false`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108150

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108150: [Remarks] [AMDGPU] Emit optimization remarks for atomics generating hardware instructions

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec added inline comments.



Comment at: clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl:9
 
+// RUN: %clang_cc1 -cl-std=CL2.0 -O0 -triple=amdgcn-amd-amdhsa -target-cpu 
gfx90a \
+// RUN: -Rpass=si-lower -munsafe-fp-atomics %s -S -o - 2>&1 | \

gandhi21299 wrote:
> rampitec wrote:
> > You are compiling 2 functions with 2 different sets of options. Essentially 
> > it is unclear what are you checking because either half skips half of the 
> > remarks. Either compile a single function differently or make 2 different 
> > tests.
> I will create 2 seperate tests
You do not need to change this file anymore.



Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:12122
 
+TargetLowering::AtomicExpansionKind SITargetLowering::reportUnsafeHWInst(
+AtomicRMWInst *RMW, TargetLowering::AtomicExpansionKind Kind) const {

It does not need to me a method, just a static function or better even lamdba.



Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:12195
 
+  if (!fpModeMatchesGlobalFPAtomicMode(RMW))
+return reportUnsafeHWInst(RMW, AtomicExpansionKind::None);

rampitec wrote:
> This is wrong. Condition is inverted and essentially tests should fail. Make 
> sure you can pass testing before posting a diff.
Unresolved.



Comment at: llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll:108
+
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at 
memory scope system due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at 
memory scope agent due to an unsafe request.

gandhi21299 wrote:
> rampitec wrote:
> > Does it print a function name before the diagnostics? Label checks would be 
> > useful.
> Nope, it does not.
That's pity. Then this file need to be split too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108150

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108150: [Remarks] [AMDGPU] Emit optimization remarks for atomics generating hardware instructions

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108150

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108150: [Remarks] [AMDGPU] Emit optimization remarks for atomics generating hardware instructions

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 updated this revision to Diff 366765.
gandhi21299 marked 2 inline comments as done.
gandhi21299 added a comment.

- split the OpenCL test into two for brevity
- fixed a mistake in SIISelLowering as pointed out by reviewer
- added the missing -munsafe-fp-atomics flag


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108150

Files:
  clang/test/CodeGenOpenCL/atomics-cas-remarks-gfx90a.cl
  clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl
  clang/test/CodeGenOpenCL/atomics-unsafe-hw-remarks-gfx90a.cl
  llvm/lib/CodeGen/AtomicExpandPass.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.h
  llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll

Index: llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll
===
--- llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll
+++ llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll
@@ -1,6 +1,9 @@
 ; RUN: llc -march=amdgcn -mcpu=gfx90a -verify-machineinstrs --pass-remarks=atomic-expand \
 ; RUN:  %s -o - 2>&1 | FileCheck %s --check-prefix=GFX90A-CAS
 
+; RUN: llc -march=amdgcn -mcpu=gfx90a -verify-machineinstrs --pass-remarks=si-lower \
+; RUN:  %s -o - 2>&1 | FileCheck %s --check-prefix=GFX90A-HW
+
 ; GFX90A-CAS: A compare and swap loop was generated for an atomic fadd operation at system memory scope
 ; GFX90A-CAS: A compare and swap loop was generated for an atomic fadd operation at agent memory scope
 ; GFX90A-CAS: A compare and swap loop was generated for an atomic fadd operation at workgroup memory scope
@@ -101,3 +104,96 @@
   %ret = atomicrmw fadd float* %p, float %q syncscope("singlethread-one-as") monotonic, align 4
   ret void
 }
+
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope system due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope agent due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope workgroup due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope wavefront due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope singlethread due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope agent-one-as due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope workgroup-one-as due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope wavefront-one-as due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope singlethread-one-as due to an unsafe request.
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw:
+; GFX90A-HW:ds_add_f64 v2, v[0:1]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw(double addrspace(3)* %ptr) #0 {
+main_body:
+  %ret = atomicrmw fadd double addrspace(3)* %ptr, double 4.0 seq_cst
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_agent:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_agent(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("agent") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wg:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_wg(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("workgroup") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wavefront:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_wavefront(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("wavefront") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_single_thread:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_single_thread(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("singlethread") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_aoa:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_aoa(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("agent-one-as") 

[PATCH] D108150: [Remarks] [AMDGPU] Emit optimization remarks for atomics generating hardware instructions

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 marked 5 inline comments as done.
gandhi21299 added inline comments.



Comment at: clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl:9
 
+// RUN: %clang_cc1 -cl-std=CL2.0 -O0 -triple=amdgcn-amd-amdhsa -target-cpu 
gfx90a \
+// RUN: -Rpass=si-lower -munsafe-fp-atomics %s -S -o - 2>&1 | \

rampitec wrote:
> You are compiling 2 functions with 2 different sets of options. Essentially 
> it is unclear what are you checking because either half skips half of the 
> remarks. Either compile a single function differently or make 2 different 
> tests.
I will create 2 seperate tests



Comment at: llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll:108
+
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at 
memory scope system due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at 
memory scope agent due to an unsafe request.

rampitec wrote:
> Does it print a function name before the diagnostics? Label checks would be 
> useful.
Nope, it does not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108150

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108150: [Remarks] [AMDGPU] Emit optimization remarks for atomics generating hardware instructions

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec added inline comments.



Comment at: clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl:9
 
+// RUN: %clang_cc1 -cl-std=CL2.0 -O0 -triple=amdgcn-amd-amdhsa -target-cpu 
gfx90a \
+// RUN: -Rpass=si-lower -munsafe-fp-atomics %s -S -o - 2>&1 | \

You are compiling 2 functions with 2 different sets of options. Essentially it 
is unclear what are you checking because either half skips half of the remarks. 
Either compile a single function differently or make 2 different tests.



Comment at: clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl:13
+
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -O0 -triple=amdgcn-amd-amdhsa -target-cpu 
gfx90a \
+// RUN: -Rpass=si-lower -S -emit-llvm -o - 2>&1 | \

This line does not have -munsafe-fp-atomics option...



Comment at: clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl:60
+// GFX90A-HW-LABEL: @atomic_unsafe_hw
+// GFX90A-HW:   atomicrmw fadd float addrspace(1)* %{{.*}}, float %{{.*}} 
syncscope("workgroup-one-as") monotonic, align 4
+// GFX90A-HW:   atomicrmw fadd float addrspace(1)* %{{.*}}, float %{{.*}} 
syncscope("agent-one-as") monotonic, align 4

... therefor these checks must fail.



Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:12195
 
+  if (!fpModeMatchesGlobalFPAtomicMode(RMW))
+return reportUnsafeHWInst(RMW, AtomicExpansionKind::None);

This is wrong. Condition is inverted and essentially tests should fail. Make 
sure you can pass testing before posting a diff.



Comment at: llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll:108
+
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at 
memory scope system due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at 
memory scope agent due to an unsafe request.

Does it print a function name before the diagnostics? Label checks would be 
useful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108150

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108150: [Remarks] [AMDGPU] Emit optimization remarks for atomics generating hardware instructions

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 updated this revision to Diff 366752.
gandhi21299 added a comment.

- corrected an argument in AtomicExpand pass


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108150

Files:
  clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl
  llvm/lib/CodeGen/AtomicExpandPass.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.h
  llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll

Index: llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll
===
--- llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll
+++ llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll
@@ -1,6 +1,9 @@
 ; RUN: llc -march=amdgcn -mcpu=gfx90a -verify-machineinstrs --pass-remarks=atomic-expand \
 ; RUN:  %s -o - 2>&1 | FileCheck %s --check-prefix=GFX90A-CAS
 
+; RUN: llc -march=amdgcn -mcpu=gfx90a -verify-machineinstrs --pass-remarks=si-lower \
+; RUN:  %s -o - 2>&1 | FileCheck %s --check-prefix=GFX90A-HW
+
 ; GFX90A-CAS: A compare and swap loop was generated for an atomic fadd operation at system memory scope
 ; GFX90A-CAS: A compare and swap loop was generated for an atomic fadd operation at agent memory scope
 ; GFX90A-CAS: A compare and swap loop was generated for an atomic fadd operation at workgroup memory scope
@@ -100,4 +103,97 @@
 entry:
   %ret = atomicrmw fadd float* %p, float %q syncscope("singlethread-one-as") monotonic, align 4
   ret void
-}
\ No newline at end of file
+}
+
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope system due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope agent due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope workgroup due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope wavefront due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope singlethread due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope agent-one-as due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope workgroup-one-as due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope wavefront-one-as due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope singlethread-one-as due to an unsafe request.
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw:
+; GFX90A-HW:ds_add_f64 v2, v[0:1]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw(double addrspace(3)* %ptr) #0 {
+main_body:
+  %ret = atomicrmw fadd double addrspace(3)* %ptr, double 4.0 seq_cst
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_agent:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_agent(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("agent") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wg:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_wg(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("workgroup") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wavefront:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_wavefront(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("wavefront") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_single_thread:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_single_thread(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("singlethread") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_aoa:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_aoa(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("agent-one-as") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wgoa:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_wgoa(float addrspace(1)* 

[PATCH] D108150: [Remarks] [AMDGPU] Emit optimization remarks for atomics generating hardware instructions

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 updated this revision to Diff 366751.
gandhi21299 added a comment.

- added more tests
- addressed feedback from reviewer


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108150

Files:
  clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl
  llvm/lib/Target/AMDGPU/SIISelLowering.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.h
  llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll

Index: llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll
===
--- llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll
+++ llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll
@@ -1,6 +1,9 @@
 ; RUN: llc -march=amdgcn -mcpu=gfx90a -verify-machineinstrs --pass-remarks=atomic-expand \
 ; RUN:  %s -o - 2>&1 | FileCheck %s --check-prefix=GFX90A-CAS
 
+; RUN: llc -march=amdgcn -mcpu=gfx90a -verify-machineinstrs --pass-remarks=si-lower \
+; RUN:  %s -o - 2>&1 | FileCheck %s --check-prefix=GFX90A-HW
+
 ; GFX90A-CAS: A compare and swap loop was generated for an atomic fadd operation at system memory scope
 ; GFX90A-CAS: A compare and swap loop was generated for an atomic fadd operation at agent memory scope
 ; GFX90A-CAS: A compare and swap loop was generated for an atomic fadd operation at workgroup memory scope
@@ -100,4 +103,97 @@
 entry:
   %ret = atomicrmw fadd float* %p, float %q syncscope("singlethread-one-as") monotonic, align 4
   ret void
-}
\ No newline at end of file
+}
+
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope system due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope agent due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope workgroup due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope wavefront due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope singlethread due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope agent-one-as due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope workgroup-one-as due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope wavefront-one-as due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope singlethread-one-as due to an unsafe request.
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw:
+; GFX90A-HW:ds_add_f64 v2, v[0:1]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw(double addrspace(3)* %ptr) #0 {
+main_body:
+  %ret = atomicrmw fadd double addrspace(3)* %ptr, double 4.0 seq_cst
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_agent:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_agent(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("agent") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wg:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_wg(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("workgroup") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wavefront:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_wavefront(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("wavefront") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_single_thread:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_single_thread(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("singlethread") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_aoa:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_aoa(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("agent-one-as") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wgoa:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_wgoa(float addrspace(1)* %ptr, float %val) #0 {