From 178c369456a5ad187d6a08e3b269a81300825775 Mon Sep 17 00:00:00 2001 From: Andrew Ng Date: Tue, 25 Apr 2017 13:39:49 +0000 Subject: [PATCH] [DebugInfo][X86] Fix handling of DBG_VALUE's in post-RA scheduler. This patch fixes a bug with the updating of DBG_VALUE's in BreakAntiDependencies. Previously, it would only attempt to update the first DBG_VALUE following the instruction whose register is being changed, potentially leaving DBG_VALUE's referring to the wrong register. Now the code will update all DBG_VALUE's that immediately follow the instruction. This issue was detected as a result of an optimized codegen difference with "-g" where an X86 byte/word fixup was not performed due to a DBG_VALUE referencing the wrong register. Differential Revision: https://reviews.llvm.org/D31755 llvm-svn: 301309 --- llvm/lib/CodeGen/AggressiveAntiDepBreaker.cpp | 6 +- llvm/lib/CodeGen/AntiDepBreaker.h | 19 ++ llvm/lib/CodeGen/CriticalAntiDepBreaker.cpp | 6 +- llvm/test/CodeGen/X86/post-ra-sched-with-debug.mir | 318 +++++++++++++++++++++ 4 files changed, 341 insertions(+), 8 deletions(-) create mode 100644 llvm/test/CodeGen/X86/post-ra-sched-with-debug.mir diff --git a/llvm/lib/CodeGen/AggressiveAntiDepBreaker.cpp b/llvm/lib/CodeGen/AggressiveAntiDepBreaker.cpp index 955524c..3a57772 100644 --- a/llvm/lib/CodeGen/AggressiveAntiDepBreaker.cpp +++ b/llvm/lib/CodeGen/AggressiveAntiDepBreaker.cpp @@ -964,10 +964,8 @@ unsigned AggressiveAntiDepBreaker::BreakAntiDependencies( // sure to update that as well. const SUnit *SU = MISUnitMap[Q.second.Operand->getParent()]; if (!SU) continue; - for (DbgValueVector::iterator DVI = DbgValues.begin(), - DVE = DbgValues.end(); DVI != DVE; ++DVI) - if (DVI->second == Q.second.Operand->getParent()) - UpdateDbgValue(*DVI->first, AntiDepReg, NewReg); + UpdateDbgValues(DbgValues, Q.second.Operand->getParent(), + AntiDepReg, NewReg); } // We just went back in time and modified history; the diff --git a/llvm/lib/CodeGen/AntiDepBreaker.h b/llvm/lib/CodeGen/AntiDepBreaker.h index 04f7f41..d14d931 100644 --- a/llvm/lib/CodeGen/AntiDepBreaker.h +++ b/llvm/lib/CodeGen/AntiDepBreaker.h @@ -60,6 +60,25 @@ public: if (MI.getOperand(0).isReg() && MI.getOperand(0).getReg() == OldReg) MI.getOperand(0).setReg(NewReg); } + + /// Update all DBG_VALUE instructions that may be affected by the dependency + /// breaker's update of ParentMI to use NewReg. + void UpdateDbgValues(const DbgValueVector &DbgValues, MachineInstr *ParentMI, + unsigned OldReg, unsigned NewReg) { + // The following code is dependent on the order in which the DbgValues are + // constructed in ScheduleDAGInstrs::buildSchedGraph. + MachineInstr *PrevDbgMI = nullptr; + for (const auto &DV : make_range(DbgValues.crbegin(), DbgValues.crend())) { + MachineInstr *PrevMI = DV.second; + if ((PrevMI == ParentMI) || (PrevMI == PrevDbgMI)) { + MachineInstr *DbgMI = DV.first; + UpdateDbgValue(*DbgMI, OldReg, NewReg); + PrevDbgMI = DbgMI; + } else if (PrevDbgMI) { + break; // If no match and already found a DBG_VALUE, we're done. + } + } + } }; } diff --git a/llvm/lib/CodeGen/CriticalAntiDepBreaker.cpp b/llvm/lib/CodeGen/CriticalAntiDepBreaker.cpp index e1eeddf..b2d6652 100644 --- a/llvm/lib/CodeGen/CriticalAntiDepBreaker.cpp +++ b/llvm/lib/CodeGen/CriticalAntiDepBreaker.cpp @@ -648,10 +648,8 @@ BreakAntiDependencies(const std::vector& SUnits, // as well. const SUnit *SU = MISUnitMap[Q->second->getParent()]; if (!SU) continue; - for (DbgValueVector::iterator DVI = DbgValues.begin(), - DVE = DbgValues.end(); DVI != DVE; ++DVI) - if (DVI->second == Q->second->getParent()) - UpdateDbgValue(*DVI->first, AntiDepReg, NewReg); + UpdateDbgValues(DbgValues, Q->second->getParent(), + AntiDepReg, NewReg); } // We just went back in time and modified history; the diff --git a/llvm/test/CodeGen/X86/post-ra-sched-with-debug.mir b/llvm/test/CodeGen/X86/post-ra-sched-with-debug.mir new file mode 100644 index 0000000..76fb35b --- /dev/null +++ b/llvm/test/CodeGen/X86/post-ra-sched-with-debug.mir @@ -0,0 +1,318 @@ +# RUN: llc -mtriple=x86_64-unknown-unknown -mcpu=btver2 -run-pass=post-RA-sched -o - %s | FileCheck %s + +# Test that multiple DBG_VALUE's following an instruction whose register needs +# to be changed during the post-RA scheduler pass are updated correctly. + +# Test case was derived from the output from the following command and +# the source code below: +# +# clang -S -emit-llvm -target x86_64 -march=btver2 -O2 -g -o - | +# llc -stop-before=post-RA-sched -o - +# +# Source code reduced from the original 8MB source file: +# +# struct a; +# class b { +# public: +# a *c = ap; +# unsigned *d() { return (unsigned *)c; } +# a *ap; +# }; +# enum { e = 2 }; +# template f *g(f *h, f *i) { +# long j = long(i), k = -!h; +# return reinterpret_cast(long(h) | k & j); +# } +# class l { +# public: +# l(int); +# int m; +# }; +# unsigned *n; +# unsigned o; +# class p { +# public: +# int aa(); +# unsigned *q() { +# n = r.d(); +# return g(n, &o); +# } +# b r; +# }; +# class s : l { +# public: +# p t; +# s(int h) : l(h), ab(t), ac(~0 << h) { ae(); } +# p &ab; +# int ac; +# void ae() { +# const unsigned *v; +# const unsigned u = 0; +# v = ab.q(); +# const unsigned *x = g(v, &u); +# int w = x[m] & ac; +# while (w) { +# int z = (ab.aa() - 1) / e; +# if (m <= z) +# return; +# } +# } +# }; +# class ad { +# public: +# ~ad() { +# for (y();;) +# ; +# } +# class y { +# public: +# y() : af(0) {} +# s af; +# }; +# }; +# class ag { +# ad ah; +# }; +# enum ai {}; +# class aj { +# public: +# aj(unsigned(ai)); +# ag ak; +# }; +# struct al { +# static unsigned am(ai); +# }; +# template struct an : al { static aj ao; }; +# template <> aj an<0>::ao(am); + +--- | + + %class.s = type <{ %class.l, [4 x i8], %class.p, %class.p*, i32, [4 x i8] }> + %class.l = type { i32 } + %class.p = type { %class.b } + %class.b = type { %struct.a*, %struct.a* } + %struct.a = type opaque + + @n = local_unnamed_addr global i32* null, align 8 + @o = global i32 0, align 4 + + define linkonce_odr void @_ZN1sC2Ei(%class.s*, i32) unnamed_addr #0 align 2 !dbg !4 { + %3 = alloca i32, align 4 + %4 = bitcast %class.s* %0 to %class.l* + tail call void @_ZN1lC2Ei(%class.l* %4, i32 %1) + %5 = getelementptr inbounds %class.s, %class.s* %0, i64 0, i32 2 + tail call void @llvm.dbg.value(metadata %class.p* %5, i64 0, metadata !10, metadata !17), !dbg !18 + tail call void @llvm.dbg.value(metadata %class.p* %5, i64 0, metadata !20, metadata !17), !dbg !27 + %6 = getelementptr inbounds %class.s, %class.s* %0, i64 0, i32 2, i32 0, i32 1 + %7 = bitcast %struct.a** %6 to i64* + %8 = load i64, i64* %7, align 8 + %9 = bitcast %class.p* %5 to i64* + store i64 %8, i64* %9, align 8 + %10 = getelementptr inbounds %class.s, %class.s* %0, i64 0, i32 3 + store %class.p* %5, %class.p** %10, align 8 + %11 = getelementptr inbounds %class.s, %class.s* %0, i64 0, i32 4 + %12 = shl i32 -1, %1 + store i32 %12, i32* %11, align 8 + store i32 0, i32* %3, align 4 + %13 = bitcast %class.p* %5 to i32** + %14 = load i32*, i32** %13, align 8 + store i32* %14, i32** @n, align 8 + %15 = icmp eq i32* %14, null + %16 = ptrtoint i32* %14 to i64 + %17 = select i1 %15, i64 ptrtoint (i32* @o to i64), i64 0 + %18 = or i64 %17, %16 + tail call void @llvm.dbg.value(metadata i32* %3, i64 0, metadata !29, metadata !35), !dbg !36 + tail call void @llvm.dbg.value(metadata i32* %3, i64 0, metadata !39, metadata !17), !dbg !44 + %19 = ptrtoint i32* %3 to i64 + call void @llvm.dbg.value(metadata i64 %19, i64 0, metadata !46, metadata !17), !dbg !48 + %20 = icmp eq i64 %18, 0 + %21 = select i1 %20, i64 %19, i64 0 + %22 = or i64 %21, %18 + %23 = inttoptr i64 %22 to i32* + %24 = bitcast %class.s* %0 to i32* + %25 = load i32, i32* %24, align 8 + %26 = sext i32 %25 to i64 + %27 = getelementptr inbounds i32, i32* %23, i64 %26 + %28 = load i32, i32* %27, align 4 + %29 = and i32 %12, %28 + %30 = icmp eq i32 %29, 0 + br i1 %30, label %47, label %31 + + ;