This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Eliminate SGPR to VGPR copy when possible
ClosedPublic

Authored by rampitec on Jun 12 2017, 6:45 PM.

Details

Diff Detail

Repository
rL LLVM

Event Timeline

rampitec created this revision.Jun 12 2017, 6:45 PM
arsenm edited edge metadata.Jun 13 2017, 4:13 PM

Typo in test file name

lib/Target/AMDGPU/SIFixSGPRCopies.cpp
182–187

This probably breaks if you have a copy that looks like a sub register extract

192

Why do you check the parent?

193

Why do you need to check this? Isn't this filtering the global isel pseudos which should never exist here?

194

Avoid doing the weird pointer arithmetic?

198

I forsee this breaking cases with multiple uses

test/CodeGen/AMDGPU/opt-sgpr-to-vgrp-copy.ll
25

Probably need a MIR test to stress multiple uses

rampitec added inline comments.Jun 13 2017, 4:21 PM
lib/Target/AMDGPU/SIFixSGPRCopies.cpp
182–187

The should not be a problem. I'm changing VGPR class to equivalent SGPR class, they support the same set of subregs.

192

A copy can be in a divergent control flow. If it is the same block we can be sure that is actually the value we are going to use.

193

isOperandLegal does not work with generic opcodes because they have no RC classes assigned to operands.

194

How do I get operand number otherwise?

198

Why? I'm checking all uses for operand legality. It also does not happen if there is a second def of the same register.

rampitec updated this revision to Diff 102449.Jun 13 2017, 4:28 PM

Fixed typo in test name.

rampitec added inline comments.Jun 13 2017, 5:22 PM
test/CodeGen/AMDGPU/opt-sgpr-to-vgrp-copy.ll
25

Can you be more specific, multiple uses of what? Do you mean multiple uses of the same COPY?

rampitec updated this revision to Diff 102618.Jun 14 2017, 4:00 PM

Replaced test with mir to check multiple use case.

rampitec marked an inline comment as done.Jun 14 2017, 4:00 PM
rampitec updated this revision to Diff 102692.Jun 15 2017, 11:12 AM
rampitec marked 2 inline comments as done.

Removed pointer arithmetic on operands.

rampitec updated this revision to Diff 102698.Jun 15 2017, 11:29 AM

Added subreg-extract like copy test.
Cleaned mir test.

lib/Target/AMDGPU/SIFixSGPRCopies.cpp
182–187

I have added test for that.

rampitec added inline comments.Jun 15 2017, 1:18 PM
lib/Target/AMDGPU/SIFixSGPRCopies.cpp
198

I have added test for multiple uses.

arsenm accepted this revision.Jun 20 2017, 11:04 AM

LGTM. It's possible to reduce the mir testcase further though

test/CodeGen/AMDGPU/opt-sgpr-to-vgpr-copy.mir
102–117 ↗(On Diff #102698)

You can strip out most of this stuff

This revision is now accepted and ready to land.Jun 20 2017, 11:04 AM
rampitec updated this revision to Diff 103241.Jun 20 2017, 11:21 AM
rampitec marked an inline comment as done.

Reduced test.

This revision was automatically updated to reflect the committed changes.