Skip to content

Commit

Permalink
Revert "Repply#2 "[RemoveDIs] Load into new debug info format by defa…
Browse files Browse the repository at this point in the history
…ult in LLVM (#89799)""

This reverts commit 91446e2 and
a unittest followup 1530f31 (#90476).

In a stage-2 -flto=thin -gsplit-dwarf -g -fdebug-info-for-profiling
-fprofile-sample-use= build of clang, a ThinLTO backend compile has
assertion failures:

    Global is external, but doesn't have external or weak linkage!
    ptr @_ZN5clang12ast_matchers8internal18makeAllOfCompositeINS_8QualTypeEEENS1_15BindableMatcherIT_EEN4llvm8ArrayRefIPKNS1_7MatcherIS5_EEEE
    function declaration may only have a unique !dbg attachment
    ptr @_ZN5clang12ast_matchers8internal18makeAllOfCompositeINS_8QualTypeEEENS1_15BindableMatcherIT_EEN4llvm8ArrayRefIPKNS1_7MatcherIS5_EEEE

The failures somehow go away if -fprofile-sample-use= is removed.
  • Loading branch information
MaskRay committed May 13, 2024
1 parent c99d115 commit 23f8fac
Show file tree
Hide file tree
Showing 19 changed files with 248 additions and 233 deletions.
7 changes: 0 additions & 7 deletions llvm/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -189,13 +189,6 @@ Changes to the Metadata Info
Changes to the Debug Info
---------------------------------

* LLVM has switched from using debug intrinsics internally to using debug
records by default. This should happen transparently when using the DIBuilder
to construct debug variable information, but will require changes for any code
that interacts with debug intrinsics directly. Debug intrinsics will only be
supported on a best-effort basis from here onwards; for more information, see
the `migration docs <https://llvm.org/docs/RemoveDIsDebugInfo.html>`_.

Changes to the LLVM tools
---------------------------------
* llvm-nm and llvm-objdump can now print symbol information from linked
Expand Down
1 change: 1 addition & 0 deletions llvm/include/llvm/AsmParser/LLParser.h
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,7 @@ namespace llvm {

// Top-Level Entities
bool parseTopLevelEntities();
bool finalizeDebugInfoFormat(Module *M);
void dropUnknownMetadataReferences();
bool validateEndOfModule(bool UpgradeDebugInfo);
bool validateEndOfIndex();
Expand Down
34 changes: 18 additions & 16 deletions llvm/lib/AsmParser/LLParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,23 @@ static std::string getTypeString(Type *T) {
return Tmp.str();
}

// Whatever debug info format we parsed, we should convert to the expected debug
// info format immediately afterwards.
bool LLParser::finalizeDebugInfoFormat(Module *M) {
// We should have already returned an error if we observed both intrinsics and
// records in this IR.
assert(!(SeenNewDbgInfoFormat && SeenOldDbgInfoFormat) &&
"Mixed debug intrinsics/records seen without a parsing error?");
if (PreserveInputDbgFormat == cl::boolOrDefault::BOU_TRUE) {
UseNewDbgInfoFormat = SeenNewDbgInfoFormat;
WriteNewDbgInfoFormatToBitcode = SeenNewDbgInfoFormat;
WriteNewDbgInfoFormat = SeenNewDbgInfoFormat;
} else if (M) {
M->setIsNewDbgInfoFormat(false);
}
return false;
}

/// Run: module ::= toplevelentity*
bool LLParser::Run(bool UpgradeDebugInfo,
DataLayoutCallbackTy DataLayoutCallback) {
Expand All @@ -91,7 +108,7 @@ bool LLParser::Run(bool UpgradeDebugInfo,
}

return parseTopLevelEntities() || validateEndOfModule(UpgradeDebugInfo) ||
validateEndOfIndex();
validateEndOfIndex() || finalizeDebugInfoFormat(M);
}

bool LLParser::parseStandaloneConstantValue(Constant *&C,
Expand Down Expand Up @@ -190,18 +207,6 @@ void LLParser::dropUnknownMetadataReferences() {
bool LLParser::validateEndOfModule(bool UpgradeDebugInfo) {
if (!M)
return false;

// We should have already returned an error if we observed both intrinsics and
// records in this IR.
assert(!(SeenNewDbgInfoFormat && SeenOldDbgInfoFormat) &&
"Mixed debug intrinsics/records seen without a parsing error?");
if (PreserveInputDbgFormat == cl::boolOrDefault::BOU_TRUE) {
UseNewDbgInfoFormat = SeenNewDbgInfoFormat;
WriteNewDbgInfoFormatToBitcode = SeenNewDbgInfoFormat;
WriteNewDbgInfoFormat = SeenNewDbgInfoFormat;
M->setNewDbgInfoFormatFlag(SeenNewDbgInfoFormat);
}

// Handle any function attribute group forward references.
for (const auto &RAG : ForwardRefAttrGroups) {
Value *V = RAG.first;
Expand Down Expand Up @@ -434,9 +439,6 @@ bool LLParser::validateEndOfModule(bool UpgradeDebugInfo) {
UpgradeModuleFlags(*M);
UpgradeSectionAttributes(*M);

if (PreserveInputDbgFormat != cl::boolOrDefault::BOU_TRUE)
M->setIsNewDbgInfoFormat(UseNewDbgInfoFormat);

if (!Slots)
return false;
// Initialize the slot mapping.
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Bitcode/Reader/BitcodeReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4319,7 +4319,7 @@ Error BitcodeReader::parseModule(uint64_t ResumeBit,
if (PreserveInputDbgFormat != cl::boolOrDefault::BOU_TRUE) {
TheModule->IsNewDbgInfoFormat =
UseNewDbgInfoFormat &&
LoadBitcodeIntoNewDbgInfoFormat != cl::boolOrDefault::BOU_FALSE;
LoadBitcodeIntoNewDbgInfoFormat == cl::boolOrDefault::BOU_TRUE;
}

this->ValueTypeCallback = std::move(Callbacks.ValueType);
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/IR/BasicBlock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ template class llvm::SymbolTableListTraits<Instruction,
BasicBlock::BasicBlock(LLVMContext &C, const Twine &Name, Function *NewParent,
BasicBlock *InsertBefore)
: Value(Type::getLabelTy(C), Value::BasicBlockVal),
IsNewDbgInfoFormat(UseNewDbgInfoFormat), Parent(nullptr) {
IsNewDbgInfoFormat(false), Parent(nullptr) {

if (NewParent)
insertInto(NewParent, InsertBefore);
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/IR/DebugProgramInstruction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -366,8 +366,8 @@ void DbgVariableRecord::setKillLocation() {
}

bool DbgVariableRecord::isKillLocation() const {
return (!hasArgList() && isa<MDNode>(getRawLocation())) ||
(getNumVariableLocationOps() == 0 && !getExpression()->isComplex()) ||
return (getNumVariableLocationOps() == 0 &&
!getExpression()->isComplex()) ||
any_of(location_ops(), [](Value *V) { return isa<UndefValue>(V); });
}

Expand Down
4 changes: 1 addition & 3 deletions llvm/lib/IR/Function.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,6 @@ static cl::opt<unsigned> NonGlobalValueMaxNameSize(
"non-global-value-max-name-size", cl::Hidden, cl::init(1024),
cl::desc("Maximum size for the name of non-global values."));

extern cl::opt<bool> UseNewDbgInfoFormat;

void Function::convertToNewDbgValues() {
IsNewDbgInfoFormat = true;
for (auto &BB : *this) {
Expand Down Expand Up @@ -440,7 +438,7 @@ Function::Function(FunctionType *Ty, LinkageTypes Linkage, unsigned AddrSpace,
: GlobalObject(Ty, Value::FunctionVal,
OperandTraits<Function>::op_begin(this), 0, Linkage, name,
computeAddrSpace(AddrSpace, ParentModule)),
NumArgs(Ty->getNumParams()), IsNewDbgInfoFormat(UseNewDbgInfoFormat) {
NumArgs(Ty->getNumParams()), IsNewDbgInfoFormat(false) {
assert(FunctionType::isValidReturnType(getReturnType()) &&
"invalid return type");
setGlobalObjectSubClassData(0);
Expand Down
4 changes: 1 addition & 3 deletions llvm/lib/IR/Module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,6 @@

using namespace llvm;

extern cl::opt<bool> UseNewDbgInfoFormat;

//===----------------------------------------------------------------------===//
// Methods to implement the globals and functions lists.
//
Expand All @@ -74,7 +72,7 @@ template class llvm::SymbolTableListTraits<GlobalIFunc>;
Module::Module(StringRef MID, LLVMContext &C)
: Context(C), ValSymTab(std::make_unique<ValueSymbolTable>(-1)),
ModuleID(std::string(MID)), SourceFileName(std::string(MID)), DL(""),
IsNewDbgInfoFormat(UseNewDbgInfoFormat) {
IsNewDbgInfoFormat(false) {
Context.addModule(this);
}

Expand Down
7 changes: 4 additions & 3 deletions llvm/tools/llvm-as/llvm-as.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,11 @@ int main(int argc, char **argv) {
}

// Convert to new debug format if requested.
M->setIsNewDbgInfoFormat(UseNewDbgInfoFormat &&
WriteNewDbgInfoFormatToBitcode);
if (M->IsNewDbgInfoFormat)
assert(!M->IsNewDbgInfoFormat && "Unexpectedly in new debug mode");
if (UseNewDbgInfoFormat && WriteNewDbgInfoFormatToBitcode) {
M->convertToNewDbgValues();
M->removeDebugIntrinsicDeclarations();
}

std::unique_ptr<ModuleSummaryIndex> Index = std::move(ModuleAndIndex.Index);

Expand Down
2 changes: 1 addition & 1 deletion llvm/tools/llvm-dis/llvm-dis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ int main(int argc, char **argv) {
// All that llvm-dis does is write the assembly to a file.
if (!DontPrint) {
if (M) {
M->setIsNewDbgInfoFormat(WriteNewDbgInfoFormat);
ScopedDbgInfoFormatSetter FormatSetter(*M, WriteNewDbgInfoFormat);
if (WriteNewDbgInfoFormat)
M->removeDebugIntrinsicDeclarations();
M->print(Out->os(), Annotator.get(), PreserveAssemblyUseListOrder);
Expand Down
8 changes: 7 additions & 1 deletion llvm/tools/llvm-link/llvm-link.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,12 @@ int main(int argc, char **argv) {
if (LoadBitcodeIntoNewDbgInfoFormat == cl::boolOrDefault::BOU_UNSET)
LoadBitcodeIntoNewDbgInfoFormat = cl::boolOrDefault::BOU_TRUE;

// RemoveDIs debug-info transition: tests may request that we /try/ to use the
// new debug-info format.
if (TryUseNewDbgInfoFormat) {
// Turn the new debug-info format on.
UseNewDbgInfoFormat = true;
}
// Since llvm-link collects multiple IR modules together, for simplicity's
// sake we disable the "PreserveInputDbgFormat" flag to enforce a single
// debug info format.
Expand Down Expand Up @@ -550,7 +556,7 @@ int main(int argc, char **argv) {
SetFormat(WriteNewDbgInfoFormat);
Composite->print(Out.os(), nullptr, PreserveAssemblyUseListOrder);
} else if (Force || !CheckBitcodeOutputToConsole(Out.os())) {
SetFormat(UseNewDbgInfoFormat && WriteNewDbgInfoFormatToBitcode);
SetFormat(WriteNewDbgInfoFormatToBitcode);
WriteBitcodeToFile(*Composite, Out.os(), PreserveBitcodeUseListOrder);
}

Expand Down
22 changes: 9 additions & 13 deletions llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
//===----------------------------------------------------------------------===//

#include "llvm/Analysis/IRSimilarityIdentifier.h"
#include "llvm/ADT/ScopeExit.h"
#include "llvm/AsmParser/Parser.h"
#include "llvm/IR/LLVMContext.h"
#include "llvm/IR/Module.h"
Expand All @@ -23,11 +22,6 @@
using namespace llvm;
using namespace IRSimilarity;

extern llvm::cl::opt<bool> UseNewDbgInfoFormat;
extern cl::opt<cl::boolOrDefault> PreserveInputDbgFormat;
extern bool WriteNewDbgInfoFormatToBitcode;
extern cl::opt<bool> WriteNewDbgInfoFormat;

static std::unique_ptr<Module> makeLLVMModule(LLVMContext &Context,
StringRef ModuleStr) {
SMDiagnostic Err;
Expand Down Expand Up @@ -1312,18 +1306,19 @@ TEST(IRInstructionMapper, CallBrInstIllegal) {
ASSERT_GT(UnsignedVec[0], Mapper.IllegalInstrNumber);
}

// Checks that an debuginfo records are mapped to be invisible. Since they
// Checks that an debuginfo intrinsics are mapped to be invisible. Since they
// do not semantically change the program, they can be recognized as similar.
TEST(IRInstructionMapper, DebugInfoInvisible) {
StringRef ModuleString = R"(
define i32 @f(i32 %a, i32 %b) {
then:
%0 = add i32 %a, %b
#dbg_value(i32 0, !0, !0, !0)
%1 = add i32 %a, %b
%0 = add i32 %a, %b
call void @llvm.dbg.value(metadata !0)
%1 = add i32 %a, %b
ret i32 0
}
declare void @llvm.dbg.value(metadata)
!0 = distinct !{!"test\00", i32 10})";
LLVMContext Context;
std::unique_ptr<Module> M = makeLLVMModule(Context, ModuleString);
Expand Down Expand Up @@ -1919,19 +1914,19 @@ TEST(IRSimilarityCandidate, CheckRegionsDifferentTypes) {
ASSERT_FALSE(longSimCandCompare(InstrList));
}

// Check that debug records do not impact similarity. They are marked as
// Check that debug instructions do not impact similarity. They are marked as
// invisible.
TEST(IRSimilarityCandidate, IdenticalWithDebug) {
StringRef ModuleString = R"(
define i32 @f(i32 %a, i32 %b) {
bb0:
%0 = add i32 %a, %b
#dbg_value(i32 0, !0, !0, !0)
call void @llvm.dbg.value(metadata !0)
%1 = add i32 %b, %a
ret i32 0
bb1:
%2 = add i32 %a, %b
#dbg_value(i32 1, !1, !1, !1)
call void @llvm.dbg.value(metadata !1)
%3 = add i32 %b, %a
ret i32 0
bb2:
Expand All @@ -1940,6 +1935,7 @@ TEST(IRSimilarityCandidate, IdenticalWithDebug) {
ret i32 0
}
declare void @llvm.dbg.value(metadata)
!0 = distinct !{!"test\00", i32 10}
!1 = distinct !{!"test\00", i32 11})";
LLVMContext Context;
Expand Down
Loading

0 comments on commit 23f8fac

Please sign in to comment.