This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Fix dwordx3/southern-islands failures.
ClosedPublic

Authored by sheredom on Jan 8 2019, 4:44 AM.

Details

Summary

This commit fixes the dwordx3/southern-islands failures that were found in bugzilla https://bugs.llvm.org/show_bug.cgi?id=40129, by not generating the dwordx3 variants of load/store instructions that were added to the ISA after southern islands.

Diff Detail

Repository
rL LLVM

Event Timeline

sheredom created this revision.Jan 8 2019, 4:44 AM
arsenm added inline comments.Jan 8 2019, 4:48 AM
lib/Target/AMDGPU/AMDGPUSubtarget.h
821–823 ↗(On Diff #180642)

return CIInsts?

lib/Target/AMDGPU/SILoadStoreOptimizer.cpp
375–379 ↗(On Diff #180642)

I would merge these into just one return

sheredom updated this revision to Diff 180644.Jan 8 2019, 5:32 AM

Fix review comments.

sheredom marked an inline comment as done.Jan 8 2019, 5:35 AM
sheredom added inline comments.
lib/Target/AMDGPU/AMDGPUSubtarget.h
821–823 ↗(On Diff #180642)

Not sure what you mean here?

arsenm added inline comments.Jan 8 2019, 3:13 PM
lib/Target/AMDGPU/AMDGPUSubtarget.h
821–823 ↗(On Diff #180642)

It's better to use the subtarget feature for the instruction set, rather than rely on the generation here. The closest one we already have is ci-insts

sheredom updated this revision to Diff 180819.Jan 9 2019, 5:06 AM

Fix review comment.

sheredom marked 4 inline comments as done.Jan 9 2019, 5:06 AM
sheredom added inline comments.
lib/Target/AMDGPU/AMDGPUSubtarget.h
821–823 ↗(On Diff #180642)

Oh I didn't know about that - good idea!

Thanks for the fix!

arsenm accepted this revision.Jan 10 2019, 12:38 AM

LGTM

This revision is now accepted and ready to land.Jan 10 2019, 12:38 AM
This revision was automatically updated to reflect the committed changes.
sheredom marked an inline comment as done.