[PATCH] D120370: [AMDGPU] Fix combined MMO in load-store merge

[Jay Foad via Phabricator via llvm-commits]

foad added a comment.

MIRParser is a bit strange, because it parses 'align 8' as 'basealign 8'. So your test case:

  %1:vgpr_32 = GLOBAL_LOAD_DWORD %0, 4, 0, implicit $exec :: (load (s32) from `i32 addrspace(1)* undef` + 4, align 8, addrspace 1)
  %2:vgpr_32 = GLOBAL_LOAD_DWORD %0, 0, 0, implicit $exec :: (load (s32) from `float addrspace(1)* undef`, align 4, addrspace 1)

really means:

  %1:vgpr_32 = GLOBAL_LOAD_DWORD %0, 4, 0, implicit $exec :: (load (s32) from `i32 addrspace(1)* undef` + 4, basealign 8, addrspace 1)
  %2:vgpr_32 = GLOBAL_LOAD_DWORD %0, 0, 0, implicit $exec :: (load (s32) from `float addrspace(1)* undef`, basealign 4, addrspace 1)

so the two MMOs have the same base pointer, but different information about how aligned it is, which probably would not happen in real codegen.

If you really want to handle that correctly, I suggest taking the larger of the two basealign values, instead of taking the one with the smaller offset.

Separately I think it would be less confusing if the test case said "basealign" explicitly, at least when specifying an alignment that is "more aligned" than the offset.

Separately maybe we should fix the MIRParser?


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

https://reviews.llvm.org/D120370