[PATCH] D118419: [AMDGPU] Allow scalar loads after barrier

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

My gut feeling is this is OK, but I'm also not convinced this is entirely sound. Can you add a test where this does not happen if there is an atomic load?



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUAnnotateUniformValues.cpp:75
 
-bool AMDGPUAnnotateUniformValues::isClobberedInFunction(LoadInst * Load) {
-  const MemoryAccess *MA = MSSA->getWalker()->getClobberingMemoryAccess(Load);
-  return !MSSA->isLiveOnEntryDef(MA);
+bool AMDGPUAnnotateUniformValues::isClobberedInFunction(LoadInst *Load) {
+  MemorySSAWalker *Walker = MSSA->getSkipSelfWalker();
----------------
This needs a comment explaining why this loop is OK


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUAnnotateUniformValues.cpp:80
+
+  LLVM_DEBUG(dbgs() << "Checking clobbering of: " << *Load << '\n');
+
----------------
rampitec wrote:
> arsenm wrote:
> > Don't need extra new line
> It is not extra. This is the actual output on my console:
> 
> ```
> Checking clobbering of:   %i2 = load i32, i32 addrspace(1)* %i1, align 4, !tbaa !15
>   Def:   fence syncscope("workgroup") acquire
>   Def:   tail call void @llvm.amdgcn.s.barrier() #3
>   Def:   fence syncscope("workgroup") release
>       -> no clobber
> ```
Weird that the Instruction and MachineInstr behavior is different


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

https://reviews.llvm.org/D118419