[PATCH] D129095: [AMDGPU][CodeGen] Match complex register SMRD offsets.

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 5 08:17:37 PDT 2022


arsenm added inline comments.


================
Comment at: llvm/test/CodeGen/AMDGPU/amdgcn-load-offset-from-reg.ll:45
+}
+
 declare void @llvm.amdgcn.raw.buffer.store.v4i32(<4 x i32>, <4 x i32>, i32, i32, i32 immarg) #1
----------------
kosarev wrote:
> arsenm wrote:
> > kosarev wrote:
> > > arsenm wrote:
> > > > Doesn't test the 32-bit constant case
> > > Why would we want to check that? Looks like there's nothing specific to 32-bit constants in the change?
> > You have the call to Expand32BitAddress
> Yes, which replicates the existing and already-tested logic. Will it eliminate the need for the test if I move the couples of `SelectSMRDOffset()` and `Expand32BitAddress()` calls into a separate function, something like `SelectSMRDBaseOffset()`? We would need it for matching (register + immediate) offsets anyway.
> 
> Feels like that'd be a test case for what is actually not a special case.
It's another permutation of a somewhat fragile case (which also works differently in globalisel). There's no reason to skimp on tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129095



More information about the llvm-commits mailing list