This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU][GlobalISel] Select G_ICMP with G_SELECT to avoid extra copies
Needs RevisionPublic

Authored by mbrkusanin on Jul 26 2023, 1:46 AM.

Details

Reviewers
foad
arsenm

Diff Detail

Event Timeline

mbrkusanin created this revision.Jul 26 2023, 1:46 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 26 2023, 1:46 AM
mbrkusanin requested review of this revision.Jul 26 2023, 1:46 AM
mbrkusanin added inline comments.Jul 26 2023, 1:56 AM
llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp
2076

Not sure if having a check for 'one use' is better or not. More lit tests will have improvements if it is removed but I was able to generate some cases where it was not beneficial.

arsenm added inline comments.Jul 26 2023, 7:21 AM
llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp
2077

you don't know that there isn't an intervening def of SCC between the def and use point, which is the point of the copy. You could check computeRegisterLiveness for SCC, but I don't know if this is a scalable strategy. I think we'd be better off with some kind of general physreg def scheduler such that PeepholeOpt can deal with the copies

arsenm added inline comments.Jul 26 2023, 7:23 AM
llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp
2077

You'd also have to rely on computeRegisterLiveness only using live-out / backwards liveness. It won't see parent defs if they haven't been selected yet

arsenm requested changes to this revision.Jul 26 2023, 10:16 AM
This revision now requires changes to proceed.Jul 26 2023, 10:16 AM