Skip to content

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.
@github-actions
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.
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.
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