From 0c0fcea557e4a7cfd51216ad20aa67c82733ab52 Mon Sep 17 00:00:00 2001 From: David Stenberg Date: Tue, 20 Oct 2020 19:30:39 +0200 Subject: [PATCH] Handle value uses wrapped in metadata for the use-list order When generating the use-list order, also consider value uses that are operands which are wrapped in metadata; e.g. llvm.dbg.value operands. This fixes PR36778. The test case is based on the reproducer from that report. Reviewed By: dexonsmith Differential Revision: https://reviews.llvm.org/D53758 --- llvm/lib/Bitcode/Writer/ValueEnumerator.cpp | 29 ++++++++++ llvm/lib/IR/AsmWriter.cpp | 19 ++++-- .../Assembler/metadata-use-uselistorder.ll | 58 +++++++++++++++++++ .../verify-uselistorder.cpp | 15 ++++- 4 files changed, 115 insertions(+), 6 deletions(-) create mode 100644 llvm/test/Assembler/metadata-use-uselistorder.ll diff --git a/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp b/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp index 4ae25f7d77c6..bbee8b324954 100644 --- a/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp +++ b/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp @@ -78,6 +78,16 @@ struct OrderMap { } // end anonymous namespace +/// Look for a value that might be wrapped as metadata, e.g. a value in a +/// metadata operand. Returns nullptr for a non-wrapped input value if +/// OnlyWrapped is true, or it returns the input value as-is if false. +static const Value *skipMetadataWrapper(const Value *V, bool OnlyWrapped) { + if (const auto *MAV = dyn_cast(V)) + if (const auto *VAM = dyn_cast(MAV->getMetadata())) + return VAM->getValue(); + return OnlyWrapped ? nullptr : V; +} + static void orderValue(const Value *V, OrderMap &OM) { if (OM.lookup(V).first) return; @@ -123,6 +133,25 @@ static OrderMap orderModule(const Module &M) { if (!isa(U.get())) orderValue(U.get(), OM); } + + // As constants used in metadata operands are emitted as module-level + // constants, we must order them before other operands. Also, we must order + // these before global values, as these will be read before setting the + // global values' initializers. The latter matters for constants which have + // uses towards other constants that are used as initializers. + for (const Function &F : M) { + if (F.isDeclaration()) + continue; + for (const BasicBlock &BB : F) + for (const Instruction &I : BB) + for (const Value *V : I.operands()) { + if (const Value *Op = skipMetadataWrapper(V, true)) { + if ((isa(*Op) && !isa(*Op)) || + isa(*Op)) + orderValue(Op, OM); + } + } + } OM.LastGlobalConstantID = OM.size(); // Initializers of GlobalValues are processed in diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp index db5a4a727419..6d6a8adab3e8 100644 --- a/llvm/lib/IR/AsmWriter.cpp +++ b/llvm/lib/IR/AsmWriter.cpp @@ -116,6 +116,15 @@ struct OrderMap { } // end anonymous namespace +/// Look for a value that might be wrapped as metadata, e.g. a value in a +/// metadata operand. Returns the input value as-is if it is not wrapped. +static const Value *skipMetadataWrapper(const Value *V) { + if (const auto *MAV = dyn_cast(V)) + if (const auto *VAM = dyn_cast(MAV->getMetadata())) + return VAM->getValue(); + return V; +} + static void orderValue(const Value *V, OrderMap &OM) { if (OM.lookup(V).first) return; @@ -132,8 +141,6 @@ static void orderValue(const Value *V, OrderMap &OM) { } static OrderMap orderModule(const Module *M) { - // This needs to match the order used by ValueEnumerator::ValueEnumerator() - // and ValueEnumerator::incorporateFunction(). OrderMap OM; for (const GlobalVariable &G : M->globals()) { @@ -167,10 +174,12 @@ static OrderMap orderModule(const Module *M) { for (const BasicBlock &BB : F) { orderValue(&BB, OM); for (const Instruction &I : BB) { - for (const Value *Op : I.operands()) + for (const Value *Op : I.operands()) { + Op = skipMetadataWrapper(Op); if ((isa(*Op) && !isa(*Op)) || isa(*Op)) orderValue(Op, OM); + } orderValue(&I, OM); } } @@ -284,9 +293,11 @@ static UseListOrderStack predictUseListOrder(const Module *M) { predictValueUseListOrder(&A, &F, OM, Stack); for (const BasicBlock &BB : F) for (const Instruction &I : BB) - for (const Value *Op : I.operands()) + for (const Value *Op : I.operands()) { + Op = skipMetadataWrapper(Op); if (isa(*Op) || isa(*Op)) // Visit GlobalValues. predictValueUseListOrder(Op, &F, OM, Stack); + } for (const BasicBlock &BB : F) for (const Instruction &I : BB) predictValueUseListOrder(&I, &F, OM, Stack); diff --git a/llvm/test/Assembler/metadata-use-uselistorder.ll b/llvm/test/Assembler/metadata-use-uselistorder.ll new file mode 100644 index 000000000000..6996c2adbc23 --- /dev/null +++ b/llvm/test/Assembler/metadata-use-uselistorder.ll @@ -0,0 +1,58 @@ +; RUN: verify-uselistorder %s + +; Reproducer for PR36778. + +; Verify that uses in metadata operands are considered when generating the +; use-list order. In this case the use-list order for @global_arr was not +; correctly preserved due to the uses in the dbg.value contant expressions not +; being considered, since they are wrapped in metadata. + +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +@global = local_unnamed_addr global i64 0, align 8 +@global_arr = common local_unnamed_addr global [10 x i64] zeroinitializer, align 16 + +define void @foo() local_unnamed_addr !dbg !6 { +entry: + %0 = load i64, i64* getelementptr inbounds ([10 x i64], [10 x i64]* @global_arr, i64 0, i64 4), align 16 + call void @llvm.dbg.value(metadata i64* getelementptr inbounds ([10 x i64], [10 x i64]* @global_arr, i64 0, i64 5), metadata !10, metadata !DIExpression()), !dbg !13 + %1 = load i64, i64* getelementptr inbounds ([10 x i64], [10 x i64]* @global_arr, i64 0, i64 6), align 16 + call void @llvm.dbg.value(metadata i64* getelementptr inbounds ([10 x i64], [10 x i64]* @global_arr, i64 0, i64 6), metadata !10, metadata !DIExpression()), !dbg !14 + ret void +} + +define void @bar() local_unnamed_addr !dbg !15 { +entry: + call void @llvm.dbg.value(metadata i64* getelementptr inbounds ([10 x i64], [10 x i64]* @global_arr, i64 0, i64 7), metadata !17, metadata !DIExpression()), !dbg !18 + ret void +} + +; Function Attrs: nounwind readnone speculatable +declare void @llvm.dbg.value(metadata, metadata, metadata) #0 + +attributes #0 = { nounwind readnone speculatable } + +!llvm.dbg.cu = !{!0} +!llvm.module.flags = !{!3, !4} +!llvm.ident = !{!5} + +!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 8.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, globals: !2, nameTableKind: None) +!1 = !DIFile(filename: "uses.c", directory: "/") +!2 = !{} +!3 = !{i32 2, !"Dwarf Version", i32 4} +!4 = !{i32 2, !"Debug Info Version", i32 3} +!5 = !{!"clang version 8.0.0"} +!6 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 6, type: !7, isLocal: false, isDefinition: true, scopeLine: 6, isOptimized: true, unit: !0, retainedNodes: !9) +!7 = !DISubroutineType(types: !8) +!8 = !{null} +!9 = !{!10} +!10 = !DILocalVariable(name: "local1", scope: !6, file: !1, line: 7, type: !11) +!11 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !12, size: 64) +!12 = !DIBasicType(name: "long int", size: 64, encoding: DW_ATE_signed) +!13 = !DILocation(line: 8, column: 8, scope: !6) +!14 = !DILocation(line: 7, column: 9, scope: !6) +!15 = distinct !DISubprogram(name: "bar", scope: !1, file: !1, line: 12, type: !7, isLocal: false, isDefinition: true, scopeLine: 12, isOptimized: true, unit: !0, retainedNodes: !16) +!16 = !{!17} +!17 = !DILocalVariable(name: "local2", scope: !15, file: !1, line: 13, type: !11) +!18 = !DILocation(line: 14, column: 1, scope: !15) diff --git a/llvm/tools/verify-uselistorder/verify-uselistorder.cpp b/llvm/tools/verify-uselistorder/verify-uselistorder.cpp index f61d23940fb0..cbd2f85ec3e0 100644 --- a/llvm/tools/verify-uselistorder/verify-uselistorder.cpp +++ b/llvm/tools/verify-uselistorder/verify-uselistorder.cpp @@ -224,10 +224,16 @@ ValueMapping::ValueMapping(const Module &M) { // Constants used by instructions. for (const BasicBlock &BB : F) for (const Instruction &I : BB) - for (const Value *Op : I.operands()) + for (const Value *Op : I.operands()) { + // Look through a metadata wrapper. + if (const auto *MAV = dyn_cast(Op)) + if (const auto *VAM = dyn_cast(MAV->getMetadata())) + Op = VAM->getValue(); + if ((isa(Op) && !isa(*Op)) || isa(Op)) map(Op); + } } } @@ -500,10 +506,15 @@ static void changeUseLists(Module &M, Changer changeValueUseList) { // Constants used by instructions. for (BasicBlock &BB : F) for (Instruction &I : BB) - for (Value *Op : I.operands()) + for (Value *Op : I.operands()) { + // Look through a metadata wrapper. + if (auto *MAV = dyn_cast(Op)) + if (auto *VAM = dyn_cast(MAV->getMetadata())) + Op = VAM->getValue(); if ((isa(Op) && !isa(*Op)) || isa(Op)) changeValueUseList(Op); + } } if (verifyModule(M, &errs()))