[PATCH] D47504: [AMDGPU] Simplify memory legalizer

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu May 31 02:02:55 PDT 2018


arsenm 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:
> > > 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


https://reviews.llvm.org/D47504





More information about the llvm-commits mailing list