[llvm] 027bccc - [NFCI][Globals] In GlobalObjects::setSectionPrefix, do conditional update if existing prefix is not equivalent to the new one. Returns whether prefix changed. (#158460)

via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 16 12:01:26 PDT 2025


Author: Mingming Liu
Date: 2025-09-16T12:01:21-07:00
New Revision: 027bccc4692923d0f1ba3d4d970071f747c2255c

URL: https://github.com/llvm/llvm-project/commit/027bccc4692923d0f1ba3d4d970071f747c2255c
DIFF: https://github.com/llvm/llvm-project/commit/027bccc4692923d0f1ba3d4d970071f747c2255c.diff

LOG: [NFCI][Globals] In GlobalObjects::setSectionPrefix, do conditional update if existing prefix is not equivalent to the new one. Returns whether prefix changed. (#158460)

Before this change, `setSectionPrefix` overwrites existing section
prefix with new one unconditionally.

After this change, `setSectionPrefix` checks for equivalences, updates
conditionally and returns whether an update happens.

Update the existing callers to make use of the return value. [PR
155337](https://github.com/llvm/llvm-project/pull/155337/files#diff-cc0c67ac89807f4453f0cfea9164944a4650cd6873a468a0f907e7158818eae9)
is a motivating use case whether the 'update' semantic is needed.

Added: 
    llvm/unittests/IR/GlobalObjectTest.cpp

Modified: 
    llvm/include/llvm/IR/GlobalObject.h
    llvm/lib/CodeGen/CodeGenPrepare.cpp
    llvm/lib/CodeGen/StaticDataAnnotator.cpp
    llvm/lib/IR/Globals.cpp
    llvm/lib/Transforms/Instrumentation/MemProfUse.cpp
    llvm/unittests/IR/CMakeLists.txt

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/IR/GlobalObject.h b/llvm/include/llvm/IR/GlobalObject.h
index 08a02b42bdc14..e273387807cf6 100644
--- a/llvm/include/llvm/IR/GlobalObject.h
+++ b/llvm/include/llvm/IR/GlobalObject.h
@@ -121,8 +121,10 @@ class GlobalObject : public GlobalValue {
   /// appropriate default object file section.
   LLVM_ABI void setSection(StringRef S);
 
-  /// Set the section prefix for this global object.
-  LLVM_ABI void setSectionPrefix(StringRef Prefix);
+  /// If existing prefix is 
diff erent from \p Prefix, set it to \p Prefix. If \p
+  /// Prefix is empty, the set clears the existing metadata. Returns true if
+  /// section prefix changed and false otherwise.
+  LLVM_ABI bool setSectionPrefix(StringRef Prefix);
 
   /// Get the section prefix for this global object.
   LLVM_ABI std::optional<StringRef> getSectionPrefix() const;

diff  --git a/llvm/lib/CodeGen/CodeGenPrepare.cpp b/llvm/lib/CodeGen/CodeGenPrepare.cpp
index 9db4c9e5e2807..92d87681c9adc 100644
--- a/llvm/lib/CodeGen/CodeGenPrepare.cpp
+++ b/llvm/lib/CodeGen/CodeGenPrepare.cpp
@@ -583,23 +583,23 @@ bool CodeGenPrepare::_run(Function &F) {
   // if requested.
   if (BBSectionsGuidedSectionPrefix && BBSectionsProfileReader &&
       BBSectionsProfileReader->isFunctionHot(F.getName())) {
-    F.setSectionPrefix("hot");
+    EverMadeChange |= F.setSectionPrefix("hot");
   } else if (ProfileGuidedSectionPrefix) {
     // The hot attribute overwrites profile count based hotness while profile
     // counts based hotness overwrite the cold attribute.
     // This is a conservative behabvior.
     if (F.hasFnAttribute(Attribute::Hot) ||
         PSI->isFunctionHotInCallGraph(&F, *BFI))
-      F.setSectionPrefix("hot");
+      EverMadeChange |= F.setSectionPrefix("hot");
     // If PSI shows this function is not hot, we will placed the function
     // into unlikely section if (1) PSI shows this is a cold function, or
     // (2) the function has a attribute of cold.
     else if (PSI->isFunctionColdInCallGraph(&F, *BFI) ||
              F.hasFnAttribute(Attribute::Cold))
-      F.setSectionPrefix("unlikely");
+      EverMadeChange |= F.setSectionPrefix("unlikely");
     else if (ProfileUnknownInSpecialSection && PSI->hasPartialSampleProfile() &&
              PSI->isFunctionHotnessUnknown(F))
-      F.setSectionPrefix("unknown");
+      EverMadeChange |= F.setSectionPrefix("unknown");
   }
 
   /// This optimization identifies DIV instructions that can be

diff  --git a/llvm/lib/CodeGen/StaticDataAnnotator.cpp b/llvm/lib/CodeGen/StaticDataAnnotator.cpp
index 2d9b489a80acb..53a9ab4dbda02 100644
--- a/llvm/lib/CodeGen/StaticDataAnnotator.cpp
+++ b/llvm/lib/CodeGen/StaticDataAnnotator.cpp
@@ -91,8 +91,7 @@ bool StaticDataAnnotator::runOnModule(Module &M) {
     if (SectionPrefix.empty())
       continue;
 
-    GV.setSectionPrefix(SectionPrefix);
-    Changed = true;
+    Changed |= GV.setSectionPrefix(SectionPrefix);
   }
 
   return Changed;

diff  --git a/llvm/lib/IR/Globals.cpp b/llvm/lib/IR/Globals.cpp
index 11d33e262fecb..1a7a5c5fbad6b 100644
--- a/llvm/lib/IR/Globals.cpp
+++ b/llvm/lib/IR/Globals.cpp
@@ -288,10 +288,22 @@ void GlobalObject::setSection(StringRef S) {
   setGlobalObjectFlag(HasSectionHashEntryBit, !S.empty());
 }
 
-void GlobalObject::setSectionPrefix(StringRef Prefix) {
+bool GlobalObject::setSectionPrefix(StringRef Prefix) {
+  StringRef ExistingPrefix;
+  if (std::optional<StringRef> MaybePrefix = getSectionPrefix())
+    ExistingPrefix = *MaybePrefix;
+
+  if (ExistingPrefix == Prefix)
+    return false;
+
+  if (Prefix.empty()) {
+    setMetadata(LLVMContext::MD_section_prefix, nullptr);
+    return true;
+  }
   MDBuilder MDB(getContext());
   setMetadata(LLVMContext::MD_section_prefix,
               MDB.createGlobalObjectSectionPrefix(Prefix));
+  return true;
 }
 
 std::optional<StringRef> GlobalObject::getSectionPrefix() const {

diff  --git a/llvm/lib/Transforms/Instrumentation/MemProfUse.cpp b/llvm/lib/Transforms/Instrumentation/MemProfUse.cpp
index ecb2f2dbc552b..c86092bd51eda 100644
--- a/llvm/lib/Transforms/Instrumentation/MemProfUse.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemProfUse.cpp
@@ -848,13 +848,12 @@ bool MemProfUsePass::annotateGlobalVariables(
     // So we just print out the static data section prefix in LLVM_DEBUG.
     if (Record && Record->AccessCount > 0) {
       ++NumOfMemProfHotGlobalVars;
-      GVar.setSectionPrefix("hot");
-      Changed = true;
+      Changed |= GVar.setSectionPrefix("hot");
       LLVM_DEBUG(dbgs() << "Global variable " << Name
                         << " is annotated as hot\n");
     } else if (DataAccessProf->isKnownColdSymbol(Name)) {
       ++NumOfMemProfColdGlobalVars;
-      GVar.setSectionPrefix("unlikely");
+      Changed |= GVar.setSectionPrefix("unlikely");
       Changed = true;
       LLVM_DEBUG(dbgs() << "Global variable " << Name
                         << " is annotated as unlikely\n");

diff  --git a/llvm/unittests/IR/CMakeLists.txt b/llvm/unittests/IR/CMakeLists.txt
index 8b7bd3997ea27..d62ce66ef9d34 100644
--- a/llvm/unittests/IR/CMakeLists.txt
+++ b/llvm/unittests/IR/CMakeLists.txt
@@ -28,6 +28,7 @@ add_llvm_unittest(IRTests
   DominatorTreeBatchUpdatesTest.cpp
   DroppedVariableStatsIRTest.cpp
   FunctionTest.cpp
+  GlobalObjectTest.cpp
   PassBuilderCallbacksTest.cpp
   IRBuilderTest.cpp
   InstructionsTest.cpp

diff  --git a/llvm/unittests/IR/GlobalObjectTest.cpp b/llvm/unittests/IR/GlobalObjectTest.cpp
new file mode 100644
index 0000000000000..0e16d01e759de
--- /dev/null
+++ b/llvm/unittests/IR/GlobalObjectTest.cpp
@@ -0,0 +1,80 @@
+//===- GlobalObjectTest.cpp - Global object unit tests --------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/IR/GlobalObject.h"
+#include "llvm/AsmParser/Parser.h"
+#include "llvm/IR/Module.h"
+#include "llvm/Support/SourceMgr.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+using namespace llvm;
+namespace {
+using testing::Eq;
+using testing::Optional;
+using testing::StrEq;
+
+static std::unique_ptr<Module> parseIR(LLVMContext &C, const char *IR) {
+  SMDiagnostic Err;
+  std::unique_ptr<Module> Mod = parseAssemblyString(IR, Err, C);
+  if (!Mod)
+    Err.print("GlobalObjectTests", errs());
+  return Mod;
+}
+
+static LLVMContext C;
+static std::unique_ptr<Module> M;
+
+class GlobalObjectTest : public testing::Test {
+public:
+  static void SetUpTestSuite() {
+    M = parseIR(C, R"(
+ at foo = global i32 3, !section_prefix !0
+ at bar = global i32 0
+
+!0 = !{!"section_prefix", !"hot"}
+)");
+  }
+};
+
+TEST_F(GlobalObjectTest, SectionPrefix) {
+  GlobalVariable *Foo = M->getGlobalVariable("foo");
+
+  // Initial section prefix is hot.
+  ASSERT_NE(Foo, nullptr);
+  ASSERT_THAT(Foo->getSectionPrefix(), Optional(StrEq("hot")));
+
+  // Test that set method returns false since existing section prefix is hot.
+  EXPECT_FALSE(Foo->setSectionPrefix("hot"));
+
+  // Set prefix from hot to unlikely.
+  Foo->setSectionPrefix("unlikely");
+  EXPECT_THAT(Foo->getSectionPrefix(), Optional(StrEq("unlikely")));
+
+  // Set prefix to empty is the same as clear.
+  Foo->setSectionPrefix("");
+  // Test that section prefix is cleared.
+  EXPECT_THAT(Foo->getSectionPrefix(), Eq(std::nullopt));
+
+  GlobalVariable *Bar = M->getGlobalVariable("bar");
+
+  // Initial section prefix is empty.
+  ASSERT_NE(Bar, nullptr);
+  ASSERT_THAT(Bar->getSectionPrefix(), Eq(std::nullopt));
+
+  // Test that set method returns false since Bar doesn't have prefix metadata.
+  EXPECT_FALSE(Bar->setSectionPrefix(""));
+
+  // Set from empty to hot.
+  EXPECT_TRUE(Bar->setSectionPrefix("hot"));
+  EXPECT_THAT(Bar->getSectionPrefix(), Optional(StrEq("hot")));
+
+  // Test that set method returns true and section prefix is cleared.
+  EXPECT_TRUE(Bar->setSectionPrefix(""));
+  EXPECT_THAT(Bar->getSectionPrefix(), Eq(std::nullopt));
+}
+} // namespace


        


More information about the llvm-commits mailing list