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

2022-08-19 Thread Jeffrey Byrnes via Phabricator via cfe-commits
jrbyrnes accepted this revision. jrbyrnes added a comment. This revision is now accepted and ready to land. LGTM Comment at: llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp:427 DAG->addMutation(createStoreClusterDAGMutation(DAG->TII, DAG->TRI));

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

2022-08-19 Thread Austin Kerbow via Phabricator via cfe-commits
kerbowa added inline comments. Comment at: llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp:427 DAG->addMutation(createStoreClusterDAGMutation(DAG->TII, DAG->TRI)); DAG->addMutation(createIGroupLPDAGMutation()); DAG->addMutation(createAMDGPUMacroFusionDAGMutation());

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

2022-08-19 Thread 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

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

2022-08-19 Thread Jeffrey Byrnes via Phabricator via cfe-commits
jrbyrnes added a comment. LGTM again Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132079/new/ https://reviews.llvm.org/D132079 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

2022-08-19 Thread Jeffrey Byrnes via Phabricator via cfe-commits
jrbyrnes added inline comments. Comment at: llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp:1063 +} else if (Opc == AMDGPU::IGLP_OPT) { + if (!foundSB && !foundIGLP) +initIGLPOpt(*R); I think this makes more sense if you parse the entire dag first,

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

2022-08-17 Thread Jeffrey Byrnes via Phabricator via cfe-commits
jrbyrnes added a comment. Hey Austin -- Just have a small question about the purpose of shouldApplyStrategy -- other than that, LGTM. Comment at: llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp:758 + + bool shouldApplyStrategy(ScheduleDAGInstrs *DAG) override { return true; } +