[PATCH] D119475: [AMDGPU] Add scheduler pass to rematerialize trivial defs
Vang Thao via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 22 13:46:40 PST 2022
vangthao marked 10 inline comments as done.
vangthao added inline comments.
================
Comment at: llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp:640
+ if (Stage == PreRARematerialize) {
+ if (RegionsWithMinOcc.count() != 1 || Regions.size() == 1)
+ break;
----------------
kerbowa wrote:
> If we only try this with a single region can RegionsWithMinOcc be an Optional<int> or something? Seems like it would not be necessary to have a vector tracking all the regions that fit the criteria.
Originally I wanted to handle multiple regions, but changed it to a single region to reduce complexity. In a later patch, hopefully we can handle multiple regions.
================
Comment at: llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp:652
+ // Pressure for regions inbetween the defs and region we sinked the def
+ // to. Will need to be fixed if there is another pass after this pass.
+ static_assert(LastStage == PreRARematerialize,
----------------
kerbowa wrote:
> Why is the PreRARematerialize pass different and doesn't need this info? Do we only reschedule the single region?
Yes, we only re-schedule the single high RP region. If we sink to a lower RP region, it should not have any impact on RP for that region since we are only sinking trivially rematerializable defs with no vreg uses.
================
Comment at: llvm/lib/Target/AMDGPU/GCNSchedStrategy.h:138
+
+ // Attempt to reduce RP of VGPR by sinking trivially rematerializable
+ // instructions. Returns true if we were able to sink instruction(s).
----------------
kerbowa wrote:
> Why only VGPRs? Could this be extended to handle SGPRs and AGPRs.
Yes, this could also be extended to handle SGPRs and AGPRs. I can put a TODO here.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119475/new/
https://reviews.llvm.org/D119475
More information about the llvm-commits
mailing list