merge update inside set, and update callsites to make use of return value

Author: mingmingl-llvm

llvm/include/llvm/IR/GlobalObject.h

@@ -121,14 +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. If \p Prefix is empty,
-  /// the section prefix metadata will be cleared if it exists.
-  LLVM_ABI void setSectionPrefix(StringRef Prefix);
-
-  /// If \p Prefix is different from existing prefix, update section prefix;
-  /// if \p Prefix is empty, an update clears the existing metadata.
-  /// Returns true if an update happens and false otherwise.
-  LLVM_ABI bool updateSectionPrefix(StringRef Prefix);
+  /// If existing prefix is different from \p Prefix is different, 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;

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

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;

llvm/lib/IR/Globals.cpp

@@ -288,26 +288,22 @@ void GlobalObject::setSection(StringRef S) {
   setGlobalObjectFlag(HasSectionHashEntryBit, !S.empty());
 }
 
-void GlobalObject::setSectionPrefix(StringRef Prefix) {
-  if (Prefix.empty()) {
-    setMetadata(LLVMContext::MD_section_prefix, nullptr);
-    return;
-  }
-  MDBuilder MDB(getContext());
-  setMetadata(LLVMContext::MD_section_prefix,
-              MDB.createGlobalObjectSectionPrefix(Prefix));
-}
-
-bool GlobalObject::updateSectionPrefix(StringRef Prefix) {
+bool GlobalObject::setSectionPrefix(StringRef Prefix) {
   StringRef ExistingPrefix;
   if (std::optional<StringRef> MaybePrefix = getSectionPrefix())
     ExistingPrefix = *MaybePrefix;
 
-  if (ExistingPrefix != Prefix) {
-    setSectionPrefix(Prefix);
+  if (ExistingPrefix == Prefix)
+    return false;
+
+  if (Prefix.empty()) {
+    setMetadata(LLVMContext::MD_section_prefix, nullptr);
     return true;
   }
-  return false;
+  MDBuilder MDB(getContext());
+  setMetadata(LLVMContext::MD_section_prefix,
+              MDB.createGlobalObjectSectionPrefix(Prefix));
+  return true;
 }
 
 std::optional<StringRef> GlobalObject::getSectionPrefix() const {

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");

llvm/unittests/IR/GlobalObjectTest.cpp

@@ -48,10 +48,10 @@ TEST_F(GlobalObjectTest, SectionPrefix) {
   ASSERT_NE(Foo, nullptr);
   ASSERT_THAT(Foo->getSectionPrefix(), Optional(StrEq("hot")));
 
-  // No actual update.
-  EXPECT_FALSE(Foo->updateSectionPrefix("hot"));
+  // Test that set method returns false since existing section prefix is hot.
+  EXPECT_FALSE(Foo->setSectionPrefix("hot"));
 
-  // Update prefix from hot to unlikely.
+  // Set prefix from hot to unlikely.
   Foo->setSectionPrefix("unlikely");
   EXPECT_THAT(Foo->getSectionPrefix(), Optional(StrEq("unlikely")));
 
@@ -66,16 +66,15 @@ TEST_F(GlobalObjectTest, SectionPrefix) {
   ASSERT_NE(Bar, nullptr);
   ASSERT_THAT(Bar->getSectionPrefix(), Eq(std::nullopt));
 
-  // Test that update method returns false since Bar doesn't have prefix
-  // metadata.
-  EXPECT_FALSE(Bar->updateSectionPrefix(""));
+  // Test that set method returns false since Bar doesn't have prefix metadata.
+  EXPECT_FALSE(Bar->setSectionPrefix(""));
 
-  // Update from empty to hot.
-  EXPECT_TRUE(Bar->updateSectionPrefix("hot"));
+  // Set from empty to hot.
+  EXPECT_TRUE(Bar->setSectionPrefix("hot"));
   EXPECT_THAT(Bar->getSectionPrefix(), Optional(StrEq("hot")));
 
-  // Teset that update method returns true and section prefix is cleared.
-  EXPECT_TRUE(Bar->updateSectionPrefix(""));
+  // Test that set method returns true and section prefix is cleared.
+  EXPECT_TRUE(Bar->setSectionPrefix(""));
   EXPECT_THAT(Bar->getSectionPrefix(), Eq(std::nullopt));
 }
 } // namespace