This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Scheduler: Don't revert the schedule if the register pressure isn't changed for a region
ClosedPublic

Authored by vpykhtin on Oct 17 2022, 4:33 AM.

Details

Summary

This one-linear fix improves compilation time for about ~40% on ASAN enabled code.

Diff Detail

Event Timeline

vpykhtin created this revision.Oct 17 2022, 4:33 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 17 2022, 4:33 AM
vpykhtin requested review of this revision.Oct 17 2022, 4:33 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 17 2022, 4:33 AM
arsenm added inline comments.Oct 17 2022, 7:31 AM
llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp
995

Maybe adjust the debug statement to say it's unchanged

arsenm added inline comments.Oct 17 2022, 7:40 AM
llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp
995

Plus this is mayCauseSpilling and falsely claiming true, should this check be in the caller?

Interesting. Why exactly does this improve compile time so much? I thought reverting scheduling wasn't exactly expensive and the RP tracking was the problem.

llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp
995

Agreed. Can we put it in the base class shouldRevertScheduling?

Interesting. Why exactly does this improve compile time so much? I thought reverting scheduling wasn't exactly expensive and the RP tracking was the problem.

The number of reverted regions dropped from 34813 to 1289 in my testcase but they're RP measured anyway and this is strange.

I was a bit inaccurate in wording - ~40% improvement is only for scheduling, not the overall compile time.

The check seems reasonable modulo infrastructure comments.

vpykhtin updated this revision to Diff 468482.Oct 18 2022, 4:47 AM
vpykhtin marked 3 inline comments as done.

Rebased, moved the check into shouldRevertScheduling of two stages:

OccInitialScheduleStage
ClusteredLowOccStage

I don't add it to the unclustered stage as this schedule should only be kept if some improvement in RP is achieved.

arsenm accepted this revision.Nov 7 2022, 8:23 AM

LGTM

This revision is now accepted and ready to land.Nov 7 2022, 8:23 AM