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

Reply via email to