[PATCH] D129381: [AMDGPU][CodeGen] Support (register + immediate) SMRD offsets.

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/test/CodeGen/AMDGPU/amdgcn-load-offset-from-reg.ll:1-2
-; RUN: llc -march=amdgcn -global-isel=0 -verify-machineinstrs -stop-after=amdgpu-isel -o - %s | FileCheck -check-prefixes=GCN,SDAG %s
-; RUN: llc -march=amdgcn -global-isel=1 -verify-machineinstrs -stop-after=amdgpu-isel -o - %s | FileCheck -check-prefixes=GCN,GISEL %s
+; RUN: llc -march=amdgcn -mcpu=gfx900 -global-isel=0 -verify-machineinstrs -stop-after=amdgpu-isel -o - %s | FileCheck -check-prefixes=GCN,SDAG %s
+; RUN: llc -march=amdgcn -mcpu=gfx900 -global-isel=1 -verify-machineinstrs -stop-after=amdgpu-isel -o - %s | FileCheck -check-prefixes=GCN,GISEL %s
 
----------------
kosarev wrote:
> arsenm wrote:
> > kosarev wrote:
> > > kosarev wrote:
> > > > arsenm wrote:
> > > > > You shouldn't use -stop-after=amdgpu-isel. Use finalize-isel for MIR tests (although I'm not sure why you're testing MIR here)
> > > > This seems to be where we already have checks for a similar case? Is there a better place for that?
> > > > You shouldn't use -stop-after=amdgpu-isel.
> > > 
> > > Can you expand a bit more on why this is preferable, please? I see literally 4 tests utilising finalize-isel, none of them look like purely isel tests,  and the other 35 all seem to use amdgpu-isel. The description of finalize-isel as it comes from rG9cac4e6d14035 again doesn't suggest anything obviously useful for this test?
> > > 
> > amdgpu-isel is specifically the SDAG pass. Your GISEL check didn't stop anywhere near the same place, and -stop-pass ran off the end and completed register allocation
> What if just remove GISEL checks from this test completely then?
I'd rather just use -stop-after finalize-isel


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129381