[llvm] r223401 - Debug info: If the RegisterCoalescer::reMaterializeTrivialDef() is
David Blaikie
dblaikie at gmail.com
Thu Dec 4 15:32:31 PST 2014
On Thu, Dec 4, 2014 at 3:24 PM, Adrian Prantl <aprantl at apple.com> wrote:
>
> On Dec 4, 2014, at 3:17 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Thu, Dec 4, 2014 at 3:08 PM, Adrian Prantl <aprantl at apple.com> wrote:
>
>>
>> > On Dec 4, 2014, at 3:01 PM, David Blaikie <dblaikie at gmail.com> wrote:
>> >
>> >
>> >
>> > On Thu, Dec 4, 2014 at 2:59 PM, Adrian Prantl <aprantl at apple.com>
>> wrote:
>> >
>> > > On Dec 4, 2014, at 2:35 PM, David Blaikie <dblaikie at gmail.com> wrote:
>> > >
>> > >
>> > >
>> > > On Thu, Dec 4, 2014 at 2:29 PM, Adrian Prantl <aprantl at apple.com>
>> wrote:
>> > > Author: adrian
>> > > Date: Thu Dec 4 16:29:04 2014
>> > > New Revision: 223401
>> > >
>> > > URL: http://llvm.org/viewvc/llvm-project?rev=223401&view=rev
>> > > Log:
>> > > Debug info: If the RegisterCoalescer::reMaterializeTrivialDef() is
>> > > eliminating all uses of a vreg, update any DBG_VALUE describing that
>> vreg
>> > > to point to the rematerialized register instead.
>> > >
>> > > Added:
>> > > llvm/trunk/test/DebugInfo/AArch64/coalescing.ll
>> > > Modified:
>> > > llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp
>> > >
>> > > Modified: llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp
>> > > URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp?rev=223401&r1=223400&r2=223401&view=diff
>> > >
>> ==============================================================================
>> > > --- llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp (original)
>> > > +++ llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp Thu Dec 4 16:29:04
>> 2014
>> > > @@ -898,8 +898,20 @@ bool RegisterCoalescer::reMaterializeTri
>> > >
>> > > // The source interval can become smaller because we removed a use.
>> > > LIS->shrinkToUses(&SrcInt, &DeadDefs);
>> > > - if (!DeadDefs.empty())
>> > > + if (!DeadDefs.empty()) {
>> > > + // If the virtual SrcReg is completely eliminated, update all
>> DBG_VALUEs
>> > > + // to describe DstReg instead.
>> > > + for (MachineRegisterInfo::use_iterator UI =
>> MRI->use_begin(SrcReg),
>> > > + UE = MRI->use_end(); UI != UE; ++UI) {
>> > >
>> > > range-for with use_operands?
>> >
>> > Updated in r223405. Looks much nicer, thanks!
>> >
>> > >
>> > > + MachineOperand &UseMO = *UI;
>> > > + MachineInstr *UseMI = UseMO.getParent();
>> > > + if (UseMI->isDebugValue()) {
>> > > + UseMO.setReg(DstReg);
>> > > + DEBUG({dbgs() << "\t\tupdated: " << *UseMI;});
>> > > + }
>> > > + }
>> > > eliminateDeadDefs();
>> > > + }
>> > >
>> > > return true;
>> > > }
>> > >
>> > > Added: llvm/trunk/test/DebugInfo/AArch64/coalescing.ll
>> > > URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/AArch64/coalescing.ll?rev=223401&view=auto
>> > >
>> ==============================================================================
>> > > --- llvm/trunk/test/DebugInfo/AArch64/coalescing.ll (added)
>> > > +++ llvm/trunk/test/DebugInfo/AArch64/coalescing.ll Thu Dec 4
>> 16:29:04 2014
>> > > @@ -0,0 +1,70 @@
>> > > +; RUN: llc -filetype=obj %s -o - | llvm-dwarfdump - | FileCheck %s
>> > > +;
>> > > +; Generated at -Os from:
>> > > +; void *my_memcpy(void *dst, const void * src, long n);
>> > > +; void* getBytesNoCopy();
>> > > +; void start() {
>> > > +; unsigned size;
>> > > +; my_memcpy(&size, getBytesNoCopy(), sizeof(size));
>> > >
>> > > are all of these parameters needed? Any idea what the interactions
>> are?
>> > We can safely omit the last two arguments.
>> >
>> > Could you remove them & anything else that's not needed to reproduce
>> this issue? (optionally, in addition, I usually find it a bit easier to
>> remove the original source identifiers and just use foo/bar/baz/etc - so
>> that there's no confusion about existing/previous semantics of the
>> functions that aren't relevant to the test)
>>
>> Sorry, I forgot to mention, I did remove them in r223405, too :-)
>> >
>> > >
>> > > +; if (size != 0) { }
>> > >
>> > > Sort of surprised this code isn't DCE'd by -Os?
>> >
>> > This is an unrelated, but still interesting bug. If the value isn’t
>> used after the call, we’re dropping the dbg.value entirely. Nevermind that
>> the actual used is still being optimized away.
>> >
>> > It'd be nice to leave it out of an unrelated test, so that it's easier
>> to tell what this test is meant to test when code changes affect its
>> results, etc.
>>
>> Well, I like to put the source code that a testcase was derived from in
>> the comment, and that line *is* necessary to get to the IR in the testcase.
>
>
> Right, sorry, perhaps I was unclear earlier: Is that line necessary to
> reproduce the bug/fix you committed? If so, yeah, keep it there (if not,
> remove it from the source comment and the IR) - I'm still surprised it's
> necessary to reproduce the bug, I would've expected it to be DCE'd long
> before it made a difference to register coalescing.
>
>
> Yeah it’s weird. The comparison is DCE’d. The *only* difference in the
> resulting IR is that if that line is removed, the dbg.value is dropped. I
> have yet to figure out why.
>
OK - maybe a comment about that caveat (something like "working around
other bug to ensure the dbg.value is preserved"?)?
>
> -- adrian
>
>
> - David
>
>
>> I’d rather add a comment than remove the line.
>>
>> -- adrian
>>
>> >
>> >
>> > >
>> > > +; }
>> > > +
>> > > +; ModuleID = 'test1.cpp'
>> > > +target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
>> > > +target triple = "arm64-apple-ios"
>> > > +
>> > > +; Function Attrs: nounwind optsize
>> > > +define void @_Z5startv() #0 {
>> > > +entry:
>> > > + %size = alloca i32, align 4
>> > > + %0 = bitcast i32* %size to i8*, !dbg !15
>> > > + %call = tail call i8* @_Z14getBytesNoCopyv() #3, !dbg !16
>> > > + %call1 = call i8* @_Z9my_memcpyPvPKvl(i8* %0, i8* %call, i64 4)
>> #3, !dbg !15
>> > > + call void @llvm.dbg.value(metadata !{i32* %size}, i64 0, metadata
>> !10, metadata !17), !dbg !18
>> > > + ; CHECK: .debug_info contents:
>> > > + ; CHECK: DW_TAG_variable
>> > > + ; CHECK-NEXT: DW_AT_location
>> > > + ; CHECK-NEXT: DW_AT_name {{.*}}"size"
>> > > + ; CHECK: .debug_loc contents:
>> > > + ; CHECK: Location description: 70 00
>> > > + ret void, !dbg !19
>> > > +}
>> > > +
>> > > +; Function Attrs: optsize
>> > > +declare i8* @_Z9my_memcpyPvPKvl(i8*, i8*, i64) #1
>> > > +
>> > > +; Function Attrs: optsize
>> > > +declare i8* @_Z14getBytesNoCopyv() #1
>> > > +
>> > > +; Function Attrs: nounwind readnone
>> > > +declare void @llvm.dbg.value(metadata, i64, metadata, metadata) #2
>> > > +
>> > > +attributes #0 = { nounwind optsize }
>> > > +attributes #1 = { optsize }
>> > > +attributes #2 = { nounwind readnone }
>> > > +attributes #3 = { nounwind optsize }
>> > > +
>> > > +!llvm.dbg.cu = !{!0}
>> > > +!llvm.module.flags = !{!12, !13}
>> > > +!llvm.ident = !{!14}
>> > > +
>> > > +!0 = metadata !{metadata !"0x11\004\00clang version 3.6.0 (trunk
>> 223149) (llvm/trunk 223153)\001\00\000\00\001", metadata !1, metadata !2,
>> metadata !2, metadata !3, metadata !2, metadata !2} ; [ DW_TAG_compile_unit
>> ] [/<stdin>] [DW_LANG_C_plus_plus]
>> > > +!1 = metadata !{metadata !"<stdin>", metadata !""}
>> > > +!2 = metadata !{}
>> > > +!3 = metadata !{metadata !4}
>> > > +!4 = metadata !{metadata
>> !"0x2e\00start\00start\00_Z5startv\003\000\001\000\000\00256\001\004",
>> metadata !5, metadata !6, metadata !7, null, void ()* @_Z5startv, null,
>> null, metadata !9} ; [ DW_TAG_subprogram ] [line 3] [def] [scope 4] [start]
>> > > +!5 = metadata !{metadata !"test1.cpp", metadata !""}
>> > > +!6 = metadata !{metadata !"0x29", metadata !5} ; [
>> DW_TAG_file_type ] [/test1.cpp]
>> > > +!7 = metadata !{metadata !"0x15\00\000\000\000\000\000\000", null,
>> null, null, metadata !8, null, null, null} ; [ DW_TAG_subroutine_type ]
>> [line 0, size 0, align 0, offset 0] [from ]
>> > > +!8 = metadata !{null}
>> > > +!9 = metadata !{metadata !10}
>> > > +!10 = metadata !{metadata !"0x100\00size\005\000", metadata !4,
>> metadata !6, metadata !11} ; [ DW_TAG_auto_variable ] [size] [line 5]
>> > > +!11 = metadata !{metadata !"0x24\00unsigned
>> int\000\0032\0032\000\000\007", null, null} ; [ DW_TAG_base_type ]
>> [unsigned int] [line 0, size 32, align 32, offset 0, enc DW_ATE_unsigned]
>> > > +!12 = metadata !{i32 2, metadata !"Dwarf Version", i32 2}
>> > > +!13 = metadata !{i32 2, metadata !"Debug Info Version", i32 2}
>> > > +!14 = metadata !{metadata !"clang version 3.6.0 (trunk 223149)
>> (llvm/trunk 223153)"}
>> > > +!15 = metadata !{i32 6, i32 3, metadata !4, null}
>> > > +!16 = metadata !{i32 6, i32 25, metadata !4, null}
>> > > +!17 = metadata !{metadata !"0x102"} ; [
>> DW_TAG_expression ]
>> > > +!18 = metadata !{i32 5, i32 12, metadata !4, null}
>> > > +!19 = metadata !{i32 9, i32 1, metadata !4, null}
>> > >
>> > >
>> > > _______________________________________________
>> > > llvm-commits mailing list
>> > > llvm-commits at cs.uiuc.edu
>> > > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> > >
>> >
>> >
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141204/33647e86/attachment.html>
More information about the llvm-commits
mailing list