[llvm] 32417b3 - [DebugInfo] ValueMapper impl for DIArgList respects IgnoreMissingLocals
Stephen Tozer via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 17 09:17:54 PST 2022
Author: Stephen Tozer
Date: 2022-01-17T17:17:32Z
New Revision: 32417b32033925b26b3695b753b38fbc6fdcd93d
URL: https://github.com/llvm/llvm-project/commit/32417b32033925b26b3695b753b38fbc6fdcd93d
DIFF: https://github.com/llvm/llvm-project/commit/32417b32033925b26b3695b753b38fbc6fdcd93d.diff
LOG: [DebugInfo] ValueMapper impl for DIArgList respects IgnoreMissingLocals
This patch fixes an issue in which SSA value reference within a
DIArgList would be unnecessarily dropped by llvm-link, even when
invoking on a single file (which should be a no-op). The reason for the
difference is that the ValueMapper does not refer to the
RF_IgnoreMissingLocals flag for LocalAsMetadata contained within a
DIArgList; this flag is used for direct LocalAsMetadata uses to preserve
SSA references even when the ValueMapper does not have an explicit
mapping for the referenced SSA value, which appears to always be the
case when using llvm-link in this manner.
Differential Revision: https://reviews.llvm.org/D114355
Added:
llvm/test/Linker/debug-info-use-before-def.ll
Modified:
llvm/lib/Transforms/Utils/ValueMapper.cpp
llvm/unittests/Transforms/Utils/ValueMapperTest.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Utils/ValueMapper.cpp b/llvm/lib/Transforms/Utils/ValueMapper.cpp
index b822db938af85..8947303674ee4 100644
--- a/llvm/lib/Transforms/Utils/ValueMapper.cpp
+++ b/llvm/lib/Transforms/Utils/ValueMapper.cpp
@@ -398,13 +398,17 @@ Value *Mapper::mapValue(const Value *V) {
SmallVector<ValueAsMetadata *, 4> MappedArgs;
for (auto *VAM : AL->getArgs()) {
// Map both Local and Constant VAMs here; they will both ultimately
- // be mapped via mapValue (apart from constants when we have no
- // module level changes, which have an identity mapping).
+ // be mapped via mapValue. The exceptions are constants when we have no
+ // module level changes and locals when they have no existing mapped
+ // value and RF_IgnoreMissingLocals is set; these have identity
+ // mappings.
if ((Flags & RF_NoModuleLevelChanges) && isa<ConstantAsMetadata>(VAM)) {
MappedArgs.push_back(VAM);
} else if (Value *LV = mapValue(VAM->getValue())) {
MappedArgs.push_back(
LV == VAM->getValue() ? VAM : ValueAsMetadata::get(LV));
+ } else if ((Flags & RF_IgnoreMissingLocals) && isa<LocalAsMetadata>(VAM)) {
+ MappedArgs.push_back(VAM);
} else {
// If we cannot map the value, set the argument as undef.
MappedArgs.push_back(ValueAsMetadata::get(
diff --git a/llvm/test/Linker/debug-info-use-before-def.ll b/llvm/test/Linker/debug-info-use-before-def.ll
new file mode 100644
index 0000000000000..ac5f3148f83c7
--- /dev/null
+++ b/llvm/test/Linker/debug-info-use-before-def.ll
@@ -0,0 +1,38 @@
+; RUN: llvm-link -S %s | FileCheck %s
+
+; Test that when a debug metadata use-before-def is run through llvm-link, the
+; value reference is preserved. Tests both singular uses and DIArgList uses of
+; the value.
+
+; CHECK-LABEL: @test
+; CHECK: call void @llvm.dbg.value(metadata i32 %A,
+; CHECK-NEXT: call void @llvm.dbg.value(metadata !DIArgList(i32 0, i32 %A),
+; CHECK-NEXT: %A =
+
+target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024"
+target triple = "spir64-unknown-unknown-intelfpga"
+
+define void @test() {
+entry:
+ call void @llvm.dbg.value(metadata i32 %A, metadata !5, metadata !DIExpression()), !dbg !10
+ call void @llvm.dbg.value(metadata !DIArgList(i32 0, i32 %A), metadata !5, metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_minus, DW_OP_stack_value)), !dbg !10
+ %A = add i32 0, 1
+ ret void
+}
+
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "bogus", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "test.c", directory: "test")
+!2 = !{i32 7, !"Dwarf Version", i32 4}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{i32 1, !"wchar_size", i32 4}
+!5 = !DILocalVariable(name: "A", arg: 2, scope: !6, file: !1, line: 60, type: !9)
+!6 = distinct !DISubprogram(name: "test", linkageName: "_test", scope: !1, file: !1, line: 60, type: !7, scopeLine: 61, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0)
+!7 = !DISubroutineType(types: !8)
+!8 = !{null}
+!9 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!10 = !DILocation(line: 0, scope: !6)
diff --git a/llvm/unittests/Transforms/Utils/ValueMapperTest.cpp b/llvm/unittests/Transforms/Utils/ValueMapperTest.cpp
index abfa67143463b..4417cc4e52e53 100644
--- a/llvm/unittests/Transforms/Utils/ValueMapperTest.cpp
+++ b/llvm/unittests/Transforms/Utils/ValueMapperTest.cpp
@@ -8,6 +8,7 @@
#include "llvm/Transforms/Utils/ValueMapper.h"
#include "llvm/IR/Constants.h"
+#include "llvm/IR/DebugInfoMetadata.h"
#include "llvm/IR/Function.h"
#include "llvm/IR/GlobalVariable.h"
#include "llvm/IR/LLVMContext.h"
@@ -324,6 +325,56 @@ TEST(ValueMapperTest, mapValueLocalAsMetadata) {
EXPECT_FALSE(VM.count(&A));
}
+TEST(ValueMapperTest, mapValueLocalInArgList) {
+ LLVMContext C;
+ FunctionType *FTy =
+ FunctionType::get(Type::getVoidTy(C), Type::getInt8Ty(C), false);
+ std::unique_ptr<Function> F(
+ Function::Create(FTy, GlobalValue::ExternalLinkage, "F"));
+ Argument &A = *F->arg_begin();
+
+ auto *LAM = LocalAsMetadata::get(&A);
+ std::vector<ValueAsMetadata*> Elts;
+ Elts.push_back(LAM);
+ auto *ArgList = DIArgList::get(C, Elts);
+ auto *MAV = MetadataAsValue::get(C, ArgList);
+
+ // The principled answer to a LocalAsMetadata of an unmapped SSA value would
+ // be to return nullptr (regardless of RF_IgnoreMissingLocals).
+ //
+ // However, algorithms that use RemapInstruction assume that each instruction
+ // only references SSA values from previous instructions. Arguments of
+ // such as "metadata i32 %x" don't currently successfully maintain that
+ // property. To keep RemapInstruction from crashing we need a non-null
+ // return here, but we also shouldn't reference the unmapped local. Use
+ // undef for uses in a DIArgList.
+ auto *N0 = UndefValue::get(Type::getInt8Ty(C));
+ auto *N0AM = ValueAsMetadata::get(N0);
+ std::vector<ValueAsMetadata*> N0Elts;
+ N0Elts.push_back(N0AM);
+ auto *N0ArgList = DIArgList::get(C, N0Elts);
+ auto *N0AV = MetadataAsValue::get(C, N0ArgList);
+ ValueToValueMapTy VM;
+ EXPECT_EQ(N0AV, ValueMapper(VM).mapValue(*MAV));
+ EXPECT_EQ(MAV, ValueMapper(VM, RF_IgnoreMissingLocals).mapValue(*MAV));
+ EXPECT_FALSE(VM.count(MAV));
+ EXPECT_FALSE(VM.count(&A));
+ EXPECT_EQ(None, VM.getMappedMD(LAM));
+ EXPECT_EQ(None, VM.getMappedMD(ArgList));
+
+ VM[MAV] = MAV;
+ EXPECT_EQ(MAV, ValueMapper(VM).mapValue(*MAV));
+ EXPECT_EQ(MAV, ValueMapper(VM, RF_IgnoreMissingLocals).mapValue(*MAV));
+ EXPECT_TRUE(VM.count(MAV));
+ EXPECT_FALSE(VM.count(&A));
+
+ VM[MAV] = &A;
+ EXPECT_EQ(&A, ValueMapper(VM).mapValue(*MAV));
+ EXPECT_EQ(&A, ValueMapper(VM, RF_IgnoreMissingLocals).mapValue(*MAV));
+ EXPECT_TRUE(VM.count(MAV));
+ EXPECT_FALSE(VM.count(&A));
+}
+
TEST(ValueMapperTest, mapValueLocalAsMetadataToConstant) {
LLVMContext Context;
auto *Int8 = Type::getInt8Ty(Context);
More information about the llvm-commits
mailing list