This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU: Fix SMRD test in trivially disjoint mem access code
ClosedPublic

Authored by dstuttard on Mar 5 2020, 8:38 AM.

Details

Summary

This seems like an obvious error - cut and paste issue?
The change does make a change to one of the lit tests - it stops s_buffer_load
re-ordering past an MUBUF instruction (which is not surprising).

Does it make sense to implement the check for different address spaces as
mentioned in the TODO - that would undo the required change in the test (and
enable more code movement). Is it valid to assume different address spaces can't
alias?

Change-Id: I80be99de5b62af4f42e91af2591b76a52ac9efa6

Diff Detail

Unit TestsFailed

Event Timeline

dstuttard created this revision.Mar 5 2020, 8:38 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 5 2020, 8:38 AM
arsenm accepted this revision.Mar 5 2020, 8:47 AM

This code should not be checking the address space. That will still fall back on the alias analysis pass on the IR in the MMO when available

This revision is now accepted and ready to land.Mar 5 2020, 8:47 AM
foad added inline comments.Mar 5 2020, 8:52 AM
llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
2676

Not strictly related to your change, but I wonder why this line doesn't return !isSMRD(MIb) && !isMUBUF(MIb) && !isMTBUF(MIb).

This revision was automatically updated to reflect the committed changes.