[AMDGPU][Scheduler] Fix compile failure due to const/sort interaction #175755
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
On some configurations sorting `ScoredRemat` objects which contains const members fails due to impossibility of swapping elements. This removes const from those fields to address the issue. The design will soon change anyway to not rely on sorting objects of this type, and consts were only here for semantic clarity.
Collaborator
|
Seeing this with the above fix: |
Contributor
Author
|
Thanks for the catch (my local compilation flow had not finished the re-compile), 7a74ba0 should fix this. |
🐧 Linux x64 Test Results
All executed tests passed, but another part of the build failed. Click on a failure below to see the details. lib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/GCNSchedStrategy.cpp.oIf these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the |
🪟 Windows x64 Test Results
All executed tests passed, but another part of the build failed. Click on a failure below to see the details. [code=1] lib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/GCNSchedStrategy.cpp.objIf these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the |
Contributor
Author
|
Thanks for the help and my apologies for the mess. |
Contributor
|
We still see the HIP tester broken (https://lab.llvm.org/buildbot/#/builders/123/builds/33483) |
Member
|
@llvm/pr-subscribers-backend-amdgpu Author: Lucas Ramirez (lucas-rami) ChangesOn some configurations sorting This removes const from those fields to address the issue. The design will soon change anyway to not rely on sorting objects of this type, and consts were only here for semantic clarity. Full diff: https://github.com/llvm/llvm-project/pull/175755.diff 2 Files Affected:
diff --git a/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp b/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp
index cb0cb6510ecd4..16644a03f089a 100644
--- a/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp
+++ b/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp
@@ -1396,7 +1396,7 @@ bool PreRARematStage::initGCNSchedStage() {
});
SmallVector<ScoredRemat> ScoredRemats;
- for (const RematReg &Remat : RematRegs)
+ for (RematReg &Remat : RematRegs)
ScoredRemats.emplace_back(&Remat, FreqInfo, DAG);
// Rematerialize registers in successive rounds until all RP targets are
@@ -2215,8 +2215,7 @@ PreRARematStage::ScoredRemat::FreqInfo::FreqInfo(
}
}
-PreRARematStage::ScoredRemat::ScoredRemat(const RematReg *Remat,
- const FreqInfo &Freq,
+PreRARematStage::ScoredRemat::ScoredRemat(RematReg *Remat, const FreqInfo &Freq,
const GCNScheduleDAGMILive &DAG)
: Remat(Remat), NumRegs(getNumRegs(DAG)), FreqDiff(getFreqDiff(Freq)) {}
diff --git a/llvm/lib/Target/AMDGPU/GCNSchedStrategy.h b/llvm/lib/Target/AMDGPU/GCNSchedStrategy.h
index a5c4c960b1f31..00876601cbc77 100644
--- a/llvm/lib/Target/AMDGPU/GCNSchedStrategy.h
+++ b/llvm/lib/Target/AMDGPU/GCNSchedStrategy.h
@@ -512,7 +512,7 @@ class PreRARematStage : public GCNSchedStage {
/// is not helpful to reduce RP in target regions.
struct ScoredRemat {
/// The rematerializable register under consideration.
- const RematReg *Remat;
+ RematReg *Remat;
/// Execution frequency information required by scoring heuristics.
/// Frequencies are scaled down if they are high to avoid overflow/underflow
@@ -531,7 +531,7 @@ class PreRARematStage : public GCNSchedStage {
/// This only initializes state-independent characteristics of \p Remat, not
/// the actual score.
- ScoredRemat(const RematReg *Remat, const FreqInfo &Freq,
+ ScoredRemat(RematReg *Remat, const FreqInfo &Freq,
const GCNScheduleDAGMILive &DAG);
/// Updates the rematerialization's score w.r.t. the current \p RPTargets.
@@ -570,7 +570,7 @@ class PreRARematStage : public GCNSchedStage {
private:
/// Number of 32-bit registers this rematerialization covers.
- const unsigned NumRegs;
+ unsigned NumRegs;
// The three members below are the scoring components, top to bottom from
// most important to least important when comparing candidates.
@@ -582,7 +582,7 @@ class PreRARematStage : public GCNSchedStage {
/// Frequency difference between defining and using regions. Negative values
/// indicate we are rematerializing to higher frequency regions; positive
/// values indicate the contrary.
- const int64_t FreqDiff;
+ int64_t FreqDiff;
/// Expected number of target regions impacted by the rematerialization,
/// scaled by the size of the register being rematerialized.
unsigned RegionImpact;
|
lucas-rami
added a commit
to lucas-rami/llvm-project
that referenced
this pull request
Jan 13, 2026
…eraction (llvm#175755)" This reverts commit 125d24a.
lucas-rami
added a commit
to lucas-rami/llvm-project
that referenced
this pull request
Jan 13, 2026
…eraction (llvm#175755)" This reverts commit 125d24a.
Priyanshu3820
pushed a commit
to Priyanshu3820/llvm-project
that referenced
this pull request
Jan 18, 2026
…llvm#175755) On some configurations sorting `ScoredRemat` objects which contains const members causes a compile failure due to impossibility of swapping/moving objects. The problem was introduced in llvm#175050. This removes const from those fields to address the issue. The design will soon change anyway to not rely on sorting objects of this type, and consts were only here for semantic clarity.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
On some configurations sorting
ScoredRematobjects which contains const members causes a compile failure due to impossibility of swapping/moving objects. The problem was introduced in #175050.This removes const from those fields to address the issue. The design will soon change anyway to not rely on sorting objects of this type, and consts were only here for semantic clarity.