[PATCH] D132079: [AMDGPU] Add iglp_opt builtin and MFMA GEMM Opt strategy

[Jeffrey Byrnes via Phabricator via cfe-commits]

jrbyrnes added a comment.

Just a couple nitpicks



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp:1071
 
   PipelineSolver PS(SyncedSchedGroups, SyncedInstrs, DAG);
   // PipelineSolver performs the mutation by adding the edges it
----------------
Have a fully unguarded entry point into PS construction / PS.solve() makes me a bit uneasy -- and it is at best inefficient. Can you guard this with foundSGB || foundIGLP?


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp:427
     DAG->addMutation(createStoreClusterDAGMutation(DAG->TII, DAG->TRI));
   DAG->addMutation(createIGroupLPDAGMutation());
   DAG->addMutation(createAMDGPUMacroFusionDAGMutation());
----------------
I think you can remove this as well since you're doing it from within the scheduler.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132079