Page MenuHomePhabricator

AMDGPU: Don't peel of the offset if the resulting base could possibly be negative in Indirect addressing.
ClosedPublic

Authored by cfang on Dec 11 2018, 12:25 PM.

Details

Summary

Don't peel of the offset if the resulting base could possibly be negative in Indirect addressing.
This is because the M0 field is of unsigned.

This patch achieves the similar goal as https://reviews.llvm.org/D55241, but keeps the optimization
if the base is known unsigned.

Diff Detail

Repository
rL LLVM

Event Timeline

cfang created this revision.Dec 11 2018, 12:25 PM
arsenm added inline comments.Dec 12 2018, 7:16 PM
lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp
1459 ↗(On Diff #177755)

Why does the constant itself matter here? Needs a test with a negative offset

arsenm added inline comments.Dec 12 2018, 7:19 PM
lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp
1459 ↗(On Diff #177755)

You should also make the cheaper check of the constant first. Why isn't this offset > 0 && SignBitIsZero?

cfang marked 4 inline comments as done.Dec 13 2018, 8:28 AM
cfang added inline comments.
lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp
1459 ↗(On Diff #177755)

If the offset <=0, base >= (base + offset). It won't make base negative to peel of the offset if (base + offset) is non-negative.
There are already a few negative offset test cases in indirect-addressing-si.ll, and the output won't change with this patch.

1459 ↗(On Diff #177755)

will change the order of checking as offset <=0 || SignBitIsZero

cfang updated this revision to Diff 178487.Dec 17 2018, 10:00 AM
cfang marked an inline comment as done.

Cheap check first.

arsenm accepted this revision.Dec 20 2018, 9:49 PM

LGTM

This revision is now accepted and ready to land.Dec 20 2018, 9:49 PM
This revision was automatically updated to reflect the committed changes.