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 different 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 getSectionPrefix() const; diff --git a/llvm/lib/CodeGen/CodeGenPrepare.cpp b/llvm/lib/CodeGen/CodeGenPrepare.cpp index 9db4c9e5e2807..a190f0dac1379 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"); + (void)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"); + (void)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"); + (void)F.setSectionPrefix("unlikely"); else if (ProfileUnknownInSpecialSection && PSI->hasPartialSampleProfile() && PSI->isFunctionHotnessUnknown(F)) - F.setSectionPrefix("unknown"); + (void)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 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 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 parseIR(LLVMContext &C, const char *IR) { + SMDiagnostic Err; + std::unique_ptr Mod = parseAssemblyString(IR, Err, C); + if (!Mod) + Err.print("GlobalObjectTests", errs()); + return Mod; +} + +static LLVMContext C; +static std::unique_ptr M; + +class GlobalObjectTest : public testing::Test { +public: + static void SetUpTestSuite() { + M = parseIR(C, R"( +@foo = global i32 3, !section_prefix !0 +@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