llvm / llvm/llvm-project

[AMDGPU][GlobalISel] Large sgpr to vgpr copies are handled poorly

Open
#217,800 2 comments 0 reactions 1 assignee Claimed by @gandhi56 View on GitHub
backend:AMDGPU llvm:globalisel
Dominant language
LLVM
Stars
40.5k
Forks
18.7k
PR merge metrics
PR metrics pending

Description

Split out of the review discussion on https://github.com/llvm/llvm-project/pull/210449 at @petar-avramovic's request, so it has a home outside that review.

## Problem

Quoting @petar-avramovic ([review](https://github.com/llvm/llvm-project/pull/210449#pullrequestreview-4797428671)):

> copies of large sgpr to vgpr are in general terrible. first (sgpr) reg alloc will try to have contiguous sgprs for copy src even that was not really necessary. Final ISA will end up with bunch of moves for pretty much no reason.
> So I was thinking to try and move all large register sgpr to vgpr copies to vgpr by element copy. But then the problem is that there are many regressions in cases where user instr could have used sgpr. So don't know, maybe this kind of combine should happen much later, was thinking near SIFoldOperands

A wide `COPY` from an SGPR tuple to a VGPR tuple forces SGPR allocation into a contiguous tuple whose only purpose is to be the copy source, and the final ISA carries `s_mov` traffic that buys nothing. Expanding such copies into per-element VGPR copies would remove that constraint.

## Why it isn't straightforward

The blocker is knowing whether the *user* could have consumed the SGPR form directly. If it could, moving the value into VGPRs is a pure loss: more VGPRs, and `SIFoldOperands` will generally fold the SGPR straight back in.

#210449 is a narrow instance of the same tension — it rebuilds a merge fed by `G_AMDGPU_READANYLANE` in the VGPR bank.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.