This is an archive of the discontinued LLVM Phabricator instance.

[NVPTX] Move NVPTXPeephole after NVPTXPrologEpilogPass
ClosedPublic

Authored by wengxt on Jun 30 2015, 4:09 PM.

Details

Summary

Offset of frame index is calculated by NVPTXPrologEpilogPass. Before
that the correct offset of stack objects cannot be obtained, which
leads to wrong offset if there are more than 2 frame objects. This patch
move NVPTXPeephole after NVPTXPrologEpilogPass. Because the frame index
is already replaced by %VRFrame in NVPTXPrologEpilogPass, we check
VRFrame register instead, and try to remove the VRFrame if there
is no usage after NVPTXPeephole pass.

Patched by Xuetian Weng.

Diff Detail

Event Timeline

wengxt updated this revision to Diff 28825.Jun 30 2015, 4:09 PM
wengxt retitled this revision from to [NVPTX] Move NVPTXPeephole after NVPTXPrologEpilogPass.
wengxt updated this object.
wengxt edited the test plan for this revision. (Show Details)
wengxt added reviewers: jingyue, jholewinski.
wengxt added a subscriber: Unknown Object (MLST).
jingyue requested changes to this revision.Jun 30 2015, 10:23 PM
jingyue edited edge metadata.

LGTM with the comments addressed.

The commit message could be clearer about what the bug was.

NVPTXPrologEpilogPass relies on frame indices to compute frame offsets. NVPTXPeephole, if run before NVPTXPrologEpilogPass, would lose frame indices too early. Therefore, we move ...

lib/Target/NVPTX/NVPTXPeephole.cpp
25

The comment here needs to be updated.

113

isCVTAToLocalCombination should check Prev.getOperand(2) is an immediate; otherwise, things may break.

146–151

Add a sentence explaining what this do.

lib/Target/NVPTX/NVPTXTargetMachine.cpp
213

Although you mentioned why they are in this order in your commit message, you also need to comment here because people won't usually dig into commit messages.

This revision now requires changes to proceed.Jun 30 2015, 10:23 PM
wengxt updated this revision to Diff 28886.Jul 1 2015, 11:20 AM
wengxt edited edge metadata.

update according to comments

wengxt added inline comments.Jul 1 2015, 11:22 AM
lib/Target/NVPTX/NVPTXPeephole.cpp
25

done

113

since there's no need to use frame offset here (already replaced by NVPTXPrologEpilogPass), this is changed to directly reuse the operand instead of create a new imm.

146–151

done

jingyue accepted this revision.Jul 1 2015, 11:22 AM
jingyue edited edge metadata.

LGTM

This revision is now accepted and ready to land.Jul 1 2015, 11:22 AM
jingyue updated this object.Jul 1 2015, 12:50 PM
jingyue edited edge metadata.
jingyue closed this revision.Jul 1 2015, 1:08 PM