This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Add more test cases of D59608.
ClosedPublic

Authored by hliao on Apr 1 2019, 9:10 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

hliao created this revision.Apr 1 2019, 9:10 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 1 2019, 9:10 AM
arsenm added a comment.Apr 1 2019, 9:19 AM

Missing the DS addressing mode ones

hliao updated this revision to Diff 193153.Apr 1 2019, 12:40 PM

Add tests for DS addressing mode.

arsenm added inline comments.Apr 1 2019, 1:11 PM
llvm/test/CodeGen/AMDGPU/ds-sub-offset.ll
3 ↗(On Diff #193153)

You can just use the same run line. +unsafe-ds-offset-folding is only needed for SI

hliao updated this revision to Diff 193166.Apr 1 2019, 1:21 PM

revise more

hliao updated this revision to Diff 193167.Apr 1 2019, 1:23 PM

more revising

arsenm added inline comments.Apr 1 2019, 5:03 PM
llvm/test/CodeGen/AMDGPU/ds-sub-offset.ll
24–26 ↗(On Diff #193167)

You don't need the new, special label for this

hliao marked an inline comment as done.Apr 1 2019, 5:18 PM
hliao added inline comments.
llvm/test/CodeGen/AMDGPU/ds-sub-offset.ll
24–26 ↗(On Diff #193167)

Won't be better to give more clear hint on why that test is added?

arsenm added inline comments.Apr 1 2019, 5:24 PM
llvm/test/CodeGen/AMDGPU/ds-sub-offset.ll
24–26 ↗(On Diff #193167)

That's what a comment is for. Adding more labels just complicates the test

hliao updated this revision to Diff 193207.Apr 1 2019, 5:29 PM

revise again

arsenm accepted this revision.Apr 1 2019, 5:33 PM

LGTM

This revision is now accepted and ready to land.Apr 1 2019, 5:33 PM
This revision was automatically updated to reflect the committed changes.
hliao added a comment.Apr 1 2019, 5:35 PM

LGTM

don't forget accept the original review, :)