Skip to content

[NFC] Make AMDGPUCombinerHelper methods const #121903

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

paulhuggett
Copy link
Contributor

(This replaces #121740. Sorry for wasting your time.)

This is a follow-up to a previous commit (ee7ca0d) which eliminated several "TODO: make CombinerHelper methods const" remarks. As promised in that ealier commit, this change completes the set by also making the methods of AMDGPUCombinerHelper const so that the Helper member of AMDGPUPreLegalizerCombinerImpl can be const rather than explicitly mutable.

This is a follow-up to a previous commit (ee7ca0d) which eliminated
several "TODO: make CombinerHelper methods const" remarks. As promised
in that ealier commit, this change completes the set by also making
the methods of AMDGPUCombinerHelper const so that the Helper member of
AMDGPUPreLegalizerCombinerImpl can be const rather than explicitly
mutable.
@llvmbot
Copy link
Member

llvmbot commented Jan 7, 2025

@llvm/pr-subscribers-backend-amdgpu

Author: Paul Bowen-Huggett (paulhuggett)

Changes

(This replaces #121740. Sorry for wasting your time.)

This is a follow-up to a previous commit (ee7ca0d) which eliminated several "TODO: make CombinerHelper methods const" remarks. As promised in that ealier commit, this change completes the set by also making the methods of AMDGPUCombinerHelper const so that the Helper member of AMDGPUPreLegalizerCombinerImpl can be const rather than explicitly mutable.


Full diff: https://github.com/llvm/llvm-project/pull/121903.diff

2 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/AMDGPUCombinerHelper.cpp (+5-5)
  • (modified) llvm/lib/Target/AMDGPU/AMDGPUCombinerHelper.h (+5-5)
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUCombinerHelper.cpp b/llvm/lib/Target/AMDGPU/AMDGPUCombinerHelper.cpp
index f6f9f4bc0fb1bb..46194ab46ff6a7 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUCombinerHelper.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUCombinerHelper.cpp
@@ -197,7 +197,7 @@ static unsigned inverseMinMax(unsigned Opc) {
 }
 
 bool AMDGPUCombinerHelper::matchFoldableFneg(MachineInstr &MI,
-                                             MachineInstr *&MatchInfo) {
+                                             MachineInstr *&MatchInfo) const {
   Register Src = MI.getOperand(1).getReg();
   MatchInfo = MRI.getVRegDef(Src);
 
@@ -266,7 +266,7 @@ bool AMDGPUCombinerHelper::matchFoldableFneg(MachineInstr &MI,
 }
 
 void AMDGPUCombinerHelper::applyFoldableFneg(MachineInstr &MI,
-                                             MachineInstr *&MatchInfo) {
+                                             MachineInstr *&MatchInfo) const {
   // Transform:
   // %A = inst %Op1, ...
   // %B = fneg %A
@@ -425,7 +425,7 @@ static bool isFPExtFromF16OrConst(const MachineRegisterInfo &MRI,
 bool AMDGPUCombinerHelper::matchExpandPromotedF16FMed3(MachineInstr &MI,
                                                        Register Src0,
                                                        Register Src1,
-                                                       Register Src2) {
+                                                       Register Src2) const {
   assert(MI.getOpcode() == TargetOpcode::G_FPTRUNC);
   Register SrcReg = MI.getOperand(1).getReg();
   if (!MRI.hasOneNonDBGUse(SrcReg) || MRI.getType(SrcReg) != LLT::scalar(32))
@@ -438,7 +438,7 @@ bool AMDGPUCombinerHelper::matchExpandPromotedF16FMed3(MachineInstr &MI,
 void AMDGPUCombinerHelper::applyExpandPromotedF16FMed3(MachineInstr &MI,
                                                        Register Src0,
                                                        Register Src1,
-                                                       Register Src2) {
+                                                       Register Src2) const {
   // We expect fptrunc (fpext x) to fold out, and to constant fold any constant
   // sources.
   Src0 = Builder.buildFPTrunc(LLT::scalar(16), Src0).getReg(0);
@@ -455,7 +455,7 @@ void AMDGPUCombinerHelper::applyExpandPromotedF16FMed3(MachineInstr &MI,
 
 bool AMDGPUCombinerHelper::matchCombineFmulWithSelectToFldexp(
     MachineInstr &MI, MachineInstr &Sel,
-    std::function<void(MachineIRBuilder &)> &MatchInfo) {
+    std::function<void(MachineIRBuilder &)> &MatchInfo) const {
   assert(MI.getOpcode() == TargetOpcode::G_FMUL);
   assert(Sel.getOpcode() == TargetOpcode::G_SELECT);
   assert(MI.getOperand(2).getReg() == Sel.getOperand(0).getReg());
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUCombinerHelper.h b/llvm/lib/Target/AMDGPU/AMDGPUCombinerHelper.h
index 893b3f5415f8c7..bc3d9daef87c5f 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUCombinerHelper.h
+++ b/llvm/lib/Target/AMDGPU/AMDGPUCombinerHelper.h
@@ -32,17 +32,17 @@ class AMDGPUCombinerHelper : public CombinerHelper {
                        MachineDominatorTree *MDT, const LegalizerInfo *LI,
                        const GCNSubtarget &STI);
 
-  bool matchFoldableFneg(MachineInstr &MI, MachineInstr *&MatchInfo);
-  void applyFoldableFneg(MachineInstr &MI, MachineInstr *&MatchInfo);
+  bool matchFoldableFneg(MachineInstr &MI, MachineInstr *&MatchInfo) const;
+  void applyFoldableFneg(MachineInstr &MI, MachineInstr *&MatchInfo) const;
 
   bool matchExpandPromotedF16FMed3(MachineInstr &MI, Register Src0,
-                                   Register Src1, Register Src2);
+                                   Register Src1, Register Src2) const;
   void applyExpandPromotedF16FMed3(MachineInstr &MI, Register Src0,
-                                   Register Src1, Register Src2);
+                                   Register Src1, Register Src2) const;
 
   bool matchCombineFmulWithSelectToFldexp(
       MachineInstr &MI, MachineInstr &Sel,
-      std::function<void(MachineIRBuilder &)> &MatchInfo);
+      std::function<void(MachineIRBuilder &)> &MatchInfo) const;
 };
 
 } // namespace llvm

This removes a TODO remark and the need to make Helper explicitly mutable.
@paulhuggett
Copy link
Contributor Author

paulhuggett commented Jan 10, 2025

Thanks @arsenm. I pushed an extra commit after you approved merge to actually eliminate the "TODO" that was my goal! Hope that's okay…

I don’t have commit access; if you're happy, can you merge this PR for me (or invite any extra reviewers)?

@arsenm arsenm merged commit bbb53d1 into llvm:main Jan 10, 2025
5 of 8 checks passed
BaiXilin pushed a commit to BaiXilin/llvm-fix-vnni-instr-types that referenced this pull request Jan 12, 2025
(This replaces llvm#121740. Sorry for wasting your time.)

This is a follow-up to a previous commit (ee7ca0d) which eliminated
several "TODO: make CombinerHelper methods const" remarks. As promised
in that ealier commit, this change completes the set by also making the
methods of AMDGPUCombinerHelper const so that the Helper member of
AMDGPUPreLegalizerCombinerImpl can be const rather than explicitly
mutable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants