This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU: Fix missing read/writelane cases to skip with exec=0
ClosedPublic

Authored by arsenm on Oct 16 2020, 5:56 PM.

Details

Summary

Writelane was missing. Additionally, since we emit the final encoded
instructions, this wasn't really matching readlane either.

Diff Detail

Event Timeline

arsenm created this revision.Oct 16 2020, 5:56 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 16 2020, 5:56 PM
arsenm requested review of this revision.Oct 16 2020, 5:56 PM

Adding writelane absolutely makes sense to me, but:

Additionally, since we emit the final encoded instructions, this wasn't really matching readlane either.

:(

We have a bunch of places where we _always_ use real instructions, and presumably those are fine. But if we let real instructions creep into places where we usually use pseudos, we end up making the backend less efficient because over time, everybody will have to check for pseudos and reals (like in this particular case here). Can we instead add some sort of check that those real instructions aren't used during CodeGen? (The MI verifier should be able to check for this, for example.)

Adding writelane absolutely makes sense to me, but:

Additionally, since we emit the final encoded instructions, this wasn't really matching readlane either.

:(

We have a bunch of places where we _always_ use real instructions, and presumably those are fine. But if we let real instructions creep into places where we usually use pseudos, we end up making the backend less efficient because over time, everybody will have to check for pseudos and reals (like in this particular case here). Can we instead add some sort of check that those real instructions aren't used during CodeGen? (The MI verifier should be able to check for this, for example.)

There is some reason we use the encoded versions for readlane/writelane that I never remember what it is

Adding writelane absolutely makes sense to me, but:

Additionally, since we emit the final encoded instructions, this wasn't really matching readlane either.

:(

We have a bunch of places where we _always_ use real instructions, and presumably those are fine. But if we let real instructions creep into places where we usually use pseudos, we end up making the backend less efficient because over time, everybody will have to check for pseudos and reals (like in this particular case here). Can we instead add some sort of check that those real instructions aren't used during CodeGen? (The MI verifier should be able to check for this, for example.)

There is some reason we use the encoded versions for readlane/writelane that I never remember what it is

Would be good to understand that? A quick hack to replace the use of TII->getMCOpcodeFromPseudo from SIRegisterInfo.cpp shows a whole bunch of lit test changes, but so far the ones I've looked at are just scheduling changes that ought to be benign.

foad added a comment.Oct 20 2020, 1:25 AM

Adding writelane absolutely makes sense to me, but:

Additionally, since we emit the final encoded instructions, this wasn't really matching readlane either.

:(

We have a bunch of places where we _always_ use real instructions, and presumably those are fine. But if we let real instructions creep into places where we usually use pseudos, we end up making the backend less efficient because over time, everybody will have to check for pseudos and reals (like in this particular case here). Can we instead add some sort of check that those real instructions aren't used during CodeGen? (The MI verifier should be able to check for this, for example.)

There is some reason we use the encoded versions for readlane/writelane that I never remember what it is

Would be good to understand that? A quick hack to replace the use of TII->getMCOpcodeFromPseudo from SIRegisterInfo.cpp shows a whole bunch of lit test changes, but so far the ones I've looked at are just scheduling changes that ought to be benign.

rG3db6ba8cfae254ecd3b5a8166d857cadb19d04e4.

foad added a comment.Oct 29 2020, 8:19 AM

Adding writelane absolutely makes sense to me, but:

Additionally, since we emit the final encoded instructions, this wasn't really matching readlane either.

:(

We have a bunch of places where we _always_ use real instructions, and presumably those are fine. But if we let real instructions creep into places where we usually use pseudos, we end up making the backend less efficient because over time, everybody will have to check for pseudos and reals (like in this particular case here). Can we instead add some sort of check that those real instructions aren't used during CodeGen? (The MI verifier should be able to check for this, for example.)

There is some reason we use the encoded versions for readlane/writelane that I never remember what it is

Would be good to understand that? A quick hack to replace the use of TII->getMCOpcodeFromPseudo from SIRegisterInfo.cpp shows a whole bunch of lit test changes, but so far the ones I've looked at are just scheduling changes that ought to be benign.

I've cooked this up into D90401.

arsenm updated this revision to Diff 301651.Oct 29 2020, 9:18 AM

Don't bother with encoded versions

foad accepted this revision.Oct 29 2020, 9:34 AM

LGTM.

This revision is now accepted and ready to land.Oct 29 2020, 9:34 AM
llvm/lib/Target/AMDGPU/SIInstrInfo.cpp