[PATCH] D130869: [AMDGPU] Add GCNMaxILPSchedStrategy

[Austin Kerbow via Phabricator via llvm-commits]

kerbowa added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/GCNSchedStrategy.h:32
+  PreRARematerialize = 3,
+  ILPInitialSchedule = 4,
+  LastStage = ILPInitialSchedule
----------------
rampitec wrote:
> AFAIR PreRARematerialize shall be a last stage, it was leaving some variables in an inconsistent state. Before the last refactoring there was even static_assert about that.
> 
> I see that you are building stages pipeline within SchedStages, but probably it makes sense to reorder the enum and redefine operator++ to walk SchedStages instead of static casting integers now.
The idea is that different SchedStrategies may have different stages or permutations of stages.

I already defined operator++ in a previous patch. I'm not sure what casting of integers you are referring to.

In this patch, each SchedStrategy has the order of its stages defined in the SchedStages vector. I should make the max occupancy strategy assert that PreRARemat is the last stage in that vector and make all the checks relative to that vector.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130869/new/

https://reviews.llvm.org/D130869