Skip to content

[CodeGen] Remove experimental deferred spilling from GreedyRegAlloc #137850

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
merged 2 commits into from
May 1, 2025

Conversation

preames
Copy link
Collaborator

@preames preames commented Apr 29, 2025

This experimental option was introduced in 2015 via commit 1192294, and the target hook was added in 2020 via commit 99e865b. There does not appear to have ever been a use of this target hook in tree.

This code is complicating one of the most complicated and hard to understand parts of our code base, and was an experiment introduced nearly 10 years ago. Let's get rid of it.

Note that the idea described in the original patch is not neccessarily a bad one, and we might return to it someday.

This experimental option was introduced in 2015 via commit 1192294,
and the target hook was added in 2020 via commit 99e865b.  There
does not appear to have ever been a use of this target hook in
tree.

This code is complicating one of the most complicated and hard to
understand parts of our code base, and was an experiment introduced
nearly 10 years ago.  Let's get rid of it.

Note that the idea described in the original patch is not neccessarily
a bad one, and we might return to it someday.
Copy link

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff HEAD~1 HEAD --extensions h,cpp -- llvm/include/llvm/CodeGen/RegAllocEvictionAdvisor.h llvm/include/llvm/CodeGen/TargetRegisterInfo.h llvm/lib/CodeGen/RegAllocGreedy.cpp
View the diff from clang-format here.
diff --git a/llvm/lib/CodeGen/RegAllocGreedy.cpp b/llvm/lib/CodeGen/RegAllocGreedy.cpp
index c93848022..24ed5fcfb 100644
--- a/llvm/lib/CodeGen/RegAllocGreedy.cpp
+++ b/llvm/lib/CodeGen/RegAllocGreedy.cpp
@@ -314,14 +314,8 @@ INITIALIZE_PASS_END(RAGreedyLegacy, "greedy", "Greedy Register Allocator",
                     false, false)
 
 #ifndef NDEBUG
-const char *const RAGreedy::StageName[] = {
-    "RS_New",
-    "RS_Assign",
-    "RS_Split",
-    "RS_Split2",
-    "RS_Spill",
-    "RS_Done"
-};
+const char *const RAGreedy::StageName[] = {"RS_New",    "RS_Assign", "RS_Split",
+                                           "RS_Split2", "RS_Spill",  "RS_Done"};
 #endif
 
 // Hysteresis to use when comparing floats.
@@ -2634,8 +2628,8 @@ MCRegister RAGreedy::selectOrSplitImpl(const LiveInterval &VirtReg,
   }
 
   // Finally spill VirtReg itself.
-  NamedRegionTimer T("spill", "Spiller", TimerGroupName,
-                     TimerGroupDescription, TimePassesIsEnabled);
+  NamedRegionTimer T("spill", "Spiller", TimerGroupName, TimerGroupDescription,
+                     TimePassesIsEnabled);
   LiveRangeEdit LRE(&VirtReg, NewVRegs, *MF, *LIS, VRM, this, &DeadRemats);
   spiller().spill(LRE, &Order);
   ExtraInfo->setStage(NewVRegs.begin(), NewVRegs.end(), RS_Done);

@qcolombet
Copy link
Collaborator

Sure we can get rid of it.

Copy link
Collaborator

@RKSimon RKSimon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - thanks for the cleanup!

@preames preames merged commit 2bb2f8a into llvm:main May 1, 2025
10 of 11 checks passed
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…lvm#137850)

This experimental option was introduced in 2015 via commit 1192294, and
the target hook was added in 2020 via commit 99e865b. There does not
appear to have ever been a use of this target hook in tree.

This code is complicating one of the most complicated and hard to
understand parts of our code base, and was an experiment introduced
nearly 10 years ago. Let's get rid of it.

Note that the idea described in the original patch is not neccessarily a
bad one, and we might return to it someday.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…lvm#137850)

This experimental option was introduced in 2015 via commit 1192294, and
the target hook was added in 2020 via commit 99e865b. There does not
appear to have ever been a use of this target hook in tree.

This code is complicating one of the most complicated and hard to
understand parts of our code base, and was an experiment introduced
nearly 10 years ago. Let's get rid of it.

Note that the idea described in the original patch is not neccessarily a
bad one, and we might return to it someday.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…lvm#137850)

This experimental option was introduced in 2015 via commit 1192294, and
the target hook was added in 2020 via commit 99e865b. There does not
appear to have ever been a use of this target hook in tree.

This code is complicating one of the most complicated and hard to
understand parts of our code base, and was an experiment introduced
nearly 10 years ago. Let's get rid of it.

Note that the idea described in the original patch is not neccessarily a
bad one, and we might return to it someday.
GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this pull request May 7, 2025
…lvm#137850)

This experimental option was introduced in 2015 via commit 1192294, and
the target hook was added in 2020 via commit 99e865b. There does not
appear to have ever been a use of this target hook in tree.

This code is complicating one of the most complicated and hard to
understand parts of our code base, and was an experiment introduced
nearly 10 years ago. Let's get rid of it.

Note that the idea described in the original patch is not neccessarily a
bad one, and we might return to it someday.
Ankur-0429 pushed a commit to Ankur-0429/llvm-project that referenced this pull request May 9, 2025
…lvm#137850)

This experimental option was introduced in 2015 via commit 1192294, and
the target hook was added in 2020 via commit 99e865b. There does not
appear to have ever been a use of this target hook in tree.

This code is complicating one of the most complicated and hard to
understand parts of our code base, and was an experiment introduced
nearly 10 years ago. Let's get rid of it.

Note that the idea described in the original patch is not neccessarily a
bad one, and we might return to it someday.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants