[PATCH] D144070: [llvm][GenericUniformity] Prevent assert while calculating temporal divergence

Tue Feb 21 23:58:59 PST 2023

yassingh marked 2 inline comments as done.
yassingh added a comment.

In D144070#4131694 <https://reviews.llvm.org/D144070#4131694>, @sameerds wrote:

> The commit description uses a number "60638" to refer to some issue, but there is no suitable context. If this is a github issue, just putting the whole URL might be better? And do note that this needs to be in the git commit, and not just the review description on this page.

Will also append issue link to the commit message.

================
Comment at: llvm/test/Analysis/DivergenceAnalysis/AMDGPU/MIR/uses-value-from-cycle.mir:4
+--- |
+  define amdgpu_cs void @f2() #0 {
+  bb:
----------------
arsenm wrote:
> yassingh wrote:
> > arsenm wrote:
> > > Shouldn't need IR section?
> > I took the tests from [[ https://github.com/llvm/llvm-project/issues/60638 | #60638]] (extracted the GMIR input that was fed to uniformity analysis).  I wasn't able to use @llvm.amdgcn.raw.atomic.buffer.load without including the IR section. Is there a workaround? 
> The intrinsic declaration is broken in some way because it's having an address computed. The reference in SI_PC_ADD_REL_OFFSET indicates this wasn't recognized as an intrinsic.
I didn't realize earlier that @llvm.amdgcn.raw.atomic.buffer.load.i32 is not actually an intrinsic just looks like one, hence appears to be broken. 
Considering dropping this test as it's confusing and bloated. 

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144070