[PATCH] D47504: [AMDGPU] Simplify memory legalizer

[Tony Tye via Phabricator via llvm-commits]

t-tye marked 6 inline comments as done.
t-tye added inline comments.


================
Comment at: test/CodeGen/AMDGPU/memory-legalizer-invalid-addrspace.mir:3
+
+--- |
+  ; ModuleID = 'memory-legalizer-invalid-addrspace.ll'
----------------
t-tye wrote:
> arsenm wrote:
> > t-tye wrote:
> > > arsenm wrote:
> > > > t-tye wrote:
> > > > > rampitec wrote:
> > > > > > Could you strip mir test from all IR part?
> > > > > The MMO references the IR to get the address space so I believe it is needed.
> > > > The MMO should be able to support address spaces without an IR reference
> > > Since the test is created from LLVM IR it seemed easier to leave the IR so the MMO is constructed with the correct address space. The alternative is to add an explicit address space to each MMO. This test follows the same approach of other existing tests, so if we want to change this we should also change those other tests.
> > We have too many tests as-is that rely on the IR. Most of these are leftovers from before the MMO separately could track the address space, or before MIR supported some feature.
> > 
> > (dereferenceable invariant load 8 from `i64 addrspace(4)* undef`, addrspace 4)
> > This is all you need. The addrspace is a separate field, and I'm pretty sure if you use undef for all of the IR references you can drop the IR segment
> OK I will give it a try. Thanks.
I have reduced the size of the mir tests. However, it was not possible to remove the llvm-ir as the mir parser requires each machine function to have a corresponding llvm-ir function.


https://reviews.llvm.org/D47504