[llvm] r224126 - Reapply "[MachineScheduler] Fix for PR21807: minor code difference building with/without -g."

Eric Christopher echristo at gmail.com
Fri Dec 12 10:55:04 PST 2014 

On Fri Dec 12 2014 at 10:49:12 AM Robinson, Paul <
Paul_Robinson at playstation.sony.com> wrote:

> > -----Original Message-----
> > From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
> > bounces at cs.uiuc.edu] On Behalf Of Andrea Di Biagio
> > Sent: Friday, December 12, 2014 10:31 AM
> > To: David Blaikie
> > Cc: llvm-commits at cs.uiuc.edu
> > Subject: Re: [llvm] r224126 - Reapply "[MachineScheduler] Fix for
> PR21807:
> > minor code difference building with/without -g."
> >
> > On Fri, Dec 12, 2014 at 5:52 PM, David Blaikie <dblaikie at gmail.com>
> wrote:
> > >
> > >
> > > On Fri, Dec 12, 2014 at 7:09 AM, Andrea Di Biagio
> > > <Andrea_DiBiagio at sn.scee.net> wrote:
> > >>
> > >> Author: adibiagio
> > >> Date: Fri Dec 12 09:09:58 2014
> > >> New Revision: 224126
> > >>
> > >> URL: http://llvm.org/viewvc/llvm-project?rev=224126&view=rev
> > >> Log:
> > >> Reapply "[MachineScheduler] Fix for PR21807: minor code difference
> > >> building with/without -g."
> > >>
> > >> This reapplies r224118 with a fix for test
> > >> 'misched-code-difference-with-debug.ll'.
> > >> That test was failing on some buildbots because it was x86 specific
> but
> > it
> > >> was
> > >> missing a target triple.
> > >> Added an explicit triple to test misched-code-difference-with-
> debug.ll.
> > >>
> > >> Added:
> > >>     llvm/trunk/test/CodeGen/X86/misched-code-difference-with-debug.ll
> > >> Modified:
> > >>     llvm/trunk/lib/CodeGen/MachineScheduler.cpp
> > >>
> > >> Modified: llvm/trunk/lib/CodeGen/MachineScheduler.cpp
> > >> URL:
> > >> http://llvm.org/viewvc/llvm-
> > project/llvm/trunk/lib/CodeGen/MachineScheduler.cpp?
> rev=224126&r1=224125&r
> > 2=224126&view=diff
> > >>
> > >>
> > ============================================================
> ==============
> > ====
> > >> --- llvm/trunk/lib/CodeGen/MachineScheduler.cpp (original)
> > >> +++ llvm/trunk/lib/CodeGen/MachineScheduler.cpp Fri Dec 12 09:09:58
> > 2014
> > >> @@ -430,9 +430,11 @@ void MachineSchedulerBase::scheduleRegio
> > >>        // instruction stream until we find the nearest boundary.
> > >>        unsigned NumRegionInstrs = 0;
> > >>        MachineBasicBlock::iterator I = RegionEnd;
> > >> -      for(;I != MBB->begin(); --I, --RemainingInstrs,
> > ++NumRegionInstrs)
> > >> {
> > >> +      for(;I != MBB->begin(); --I, --RemainingInstrs) {
> > >>          if (isSchedBoundary(std::prev(I), MBB, MF, TII, IsPostRA))
> > >>            break;
> > >> +        if (!I->isDebugValue())
> > >> +          ++NumRegionInstrs;
> > >>        }
> > >>        // Notify the scheduler of the region, even if we may skip
> > >> scheduling
> > >>        // it. Perhaps it still needs to be bundled.
> > >>
> > >> Added: llvm/trunk/test/CodeGen/X86/misched-code-difference-with-
> > debug.ll
> > >> URL:
> > >> http://llvm.org/viewvc/llvm-
> > project/llvm/trunk/test/CodeGen/X86/misched-code-difference-with-
> > debug.ll?rev=224126&view=auto
> > >>
> > >>
> > ============================================================
> ==============
> > ====
> > >> --- llvm/trunk/test/CodeGen/X86/misched-code-difference-with-debug.ll
> > >> (added)
> > >> +++ llvm/trunk/test/CodeGen/X86/misched-code-difference-with-debug.ll
> > Fri
> > >> Dec 12 09:09:58 2014
> > >> @@ -0,0 +1,90 @@
> > >> +; RUN: llc < %s -march=x86-64 -mtriple=x86_64-unknown-unknown
> > >> -mcpu=generic | FileCheck %s
> > >> +; Both functions should produce the same code. The presence of debug
> > >> values
> > >> +; should not affect the scheduling strategy.
> > >> +; Generated from:
> > >> +; char argc;
> > >> +; class C {
> > >> +; public:
> > >> +;   int test(char ,char ,char ,...);
> > >> +; };
> > >> +; void foo() {
> > >> +;   C c;
> > >> +;   char lc = argc;
> > >> +;   c.test(0,argc,0,lc);
> > >> +;   c.test(0,argc,0,lc);
> > >> +; }
> > >
> > >
> > > Is this a minimal reproduction? I'm slightly surprised at some of the
> > quirks
> > > here, and it might be best to at least highlight the bits that make
> this
> > > interesting (I assume it's not important that this is a non-static
> > member
> > > function, for example - it might just need an extra argument (add
> > another
> > > char argument and make it a free function, removing the class C)). Does
> > argc
> > > need to be copied like that, or could 'lc' just be another (possibly
> > local)
> > > variable (maybe it needs to not be optimzed away, in which case a
> global
> > > might be the simplest)? Does 'argc' have to appear between two other
> > zeros
> > > like that? etc...
> > >
> > > Anyawy, would be nice to ensure the test case is fully reduced - just
> > > checking.
> >
> > Hi David,
> >
> > As far as I understand, the only constraint is that the number of
> > instructions in a scheduling region (including dbg.values) has to be
> > above a certain threshold to trigger the change in behavior in the
> > scheduler.
> >
> > I remember that Russ (the author of this test) tried to obtain a
> > minimal reproducible. However, I think you are right and this test can
> > probably be further simplified. I will ask him to have a look at see
> > if it can be further reduced.
> >
> > Thanks,
> > Andrea
>
> IIRC what's required is that the extra dbg.value instructions have to make
> it *cross* the threshold.  By crossing the threshold, the behavior changes.
> --paulr
>
>
Yes. That's correct.

-eric


> >
> > >
> > >>
> > >> +;
> > >> +; with
> > >> +; clang -O2 -c test.cpp -emit-llvm -S
> > >> +; clang -O2 -c test.cpp -emit-llvm -S -g
> > >> +;
> > >> +
> > >> +
> > >> +%class.C = type { i8 }
> > >> +
> > >> + at argc = global i8 0, align 1
> > >> +
> > >> +declare i32 @test_function(%class.C*, i8 signext, i8 signext, i8
> > signext,
> > >> ...)
> > >> +
> > >> +; CHECK-LABEL: test_without_debug
> > >> +; CHECK: movl [[A:%[a-z]+]], [[B:%[a-z]+]]
> > >> +; CHECK-NEXT: movl [[A]], [[C:%[a-z]+]]
> > >> +define void @test_without_debug() {
> > >> +entry:
> > >> +  %c = alloca %class.C, align 1
> > >> +  %0 = load i8* @argc, align 1
> > >> +  %conv = sext i8 %0 to i32
> > >> +  %call = call i32 (%class.C*, i8, i8, i8, ...)*
> > @test_function(%class.C*
> > >> %c, i8 signext 0, i8 signext %0, i8 signext 0, i32 %conv)
> > >> +  %1 = load i8* @argc, align 1
> > >> +  %call2 = call i32 (%class.C*, i8, i8, i8, ...)*
> > >> @test_function(%class.C* %c, i8 signext 0, i8 signext %1, i8 signext
> 0,
> > i32
> > >> %conv)
> > >> +  ret void
> > >> +}
> > >> +
> > >> +; CHECK-LABEL: test_with_debug
> > >> +; CHECK: movl [[A]], [[B]]
> > >> +; CHECK-NEXT: movl [[A]], [[C]]
> > >> +define void @test_with_debug() {
> > >> +entry:
> > >> +  %c = alloca %class.C, align 1
> > >> +  %0 = load i8* @argc, align 1
> > >> +  tail call void @llvm.dbg.value(metadata !{i8 %0}, i64 0, metadata
> > !19,
> > >> metadata !29)
> > >> +  %conv = sext i8 %0 to i32
> > >> +  tail call void @llvm.dbg.value(metadata !{%class.C* %c}, i64 0,
> > >> metadata !18, metadata !29)
> > >> +  %call = call i32 (%class.C*, i8, i8, i8, ...)*
> > @test_function(%class.C*
> > >> %c, i8 signext 0, i8 signext %0, i8 signext 0, i32 %conv)
> > >> +  %1 = load i8* @argc, align 1
> > >> +  call void @llvm.dbg.value(metadata !{%class.C* %c}, i64 0, metadata
> > >> !18, metadata !29)
> > >> +  %call2 = call i32 (%class.C*, i8, i8, i8, ...)*
> > >> @test_function(%class.C* %c, i8 signext 0, i8 signext %1, i8 signext
> 0,
> > i32
> > >> %conv)
> > >> +  ret void
> > >> +}
> > >> +
> > >> +declare void @llvm.dbg.value(metadata, i64, metadata, metadata)
> > >> +
> > >> +!llvm.dbg.cu = !{!0}
> > >> +!llvm.module.flags = !{!22, !23}
> > >> +
> > >> +!0 = metadata !{metadata !"", metadata !1, metadata !2, metadata !3,
> > >> metadata !12, metadata !20, metadata !2} ; [ DW_TAG_compile_unit ]
> > >> [test.cpp] [DW_LANG_C_plus_plus]
> > >> +!1 = metadata !{metadata !"test.cpp", metadata !""}
> > >> +!2 = metadata !{}
> > >> +!3 = metadata !{metadata !4}
> > >> +!4 = metadata !{metadata !"0x2\00C\002\008\008\000\000\000",
> metadata
> > !1,
> > >> null, null, metadata !5, null, null, metadata !"_ZTS1C"} ; [
> > >> DW_TAG_class_type ] [C] [line 2, size 8, align 8, offset 0] [def]
> [from
> > ]
> > >> +!5 = metadata !{metadata !6}
> > >> +!6 = metadata !{metadata !"", metadata !1, metadata !"_ZTS1C",
> > metadata
> > >> !7, null, null, null, null, null} ; [ DW_TAG_subprogram ] [line 4]
> > [public]
> > >> [test]
> > >> +!7 = metadata !{metadata !"", null, null, null, metadata !8, null,
> > null,
> > >> null} ; [ DW_TAG_subroutine_type ] [line 0, size 0, align 0, offset 0]
> > [from
> > >> ]
> > >> +!8 = metadata !{metadata !9, metadata !10, metadata !11, metadata
> !11,
> > >> metadata !11, null}
> > >> +!9 = metadata !{metadata !"", null, null} ; [ DW_TAG_base_type ]
> [int]
> > >> [line 0, size 32, align 32, offset 0, enc DW_ATE_signed]
> > >> +!10 = metadata !{metadata !"", null, null, metadata !"_ZTS1C"} ; [
> > >> DW_TAG_pointer_type ] [line 0, size 64, align 64, offset 0]
> > [artificial]
> > >> [from _ZTS1C]
> > >> +!11 = metadata !{metadata !"0x24\00char\000\008\008\000\000\006",
> > null,
> > >> null} ; [ DW_TAG_base_type ] [char] [line 0, size 8, align 8, offset
> 0,
> > enc
> > >> DW_ATE_signed_char]
> > >> +!12 = metadata !{metadata !13}
> > >> +!13 = metadata !{metadata
> > >>
> > !"0x2e\00test_with_debug\00test_with_debug\00test_with_
> debug\006\000\001\0
> > 00\000\00256\001\006",
> > >> metadata !1, metadata !14, metadata !15, null, void ()*
> > @test_with_debug,
> > >> null, null, metadata !17} ; [ DW_TAG_subprogram ] [line 6] [def]
> > >> [test_with_debug]
> > >> +!14 = metadata !{metadata !"0x29", metadata !1}
> > >> +!15 = metadata !{metadata !"0x15\00\000\000\000\000\000\000", null,
> > null,
> > >> null, metadata !16, null, null, null} ; [ DW_TAG_subroutine_type ]
> > [line 0,
> > >> size 0, align 0, offset 0] [from ]
> > >> +!16 = metadata !{null}
> > >> +!17 = metadata !{metadata !18, metadata !19}
> > >> +!18 = metadata !{metadata !"0x100\00c\007\000", metadata !13,
> metadata
> > >> !14, metadata !"_ZTS1C"} ; [ DW_TAG_auto_variable ] [c] [line 7]
> > >> +!19 = metadata !{metadata !"0x100\00lc\008\000", metadata !13,
> > metadata
> > >> !14, metadata !11} ; [ DW_TAG_auto_variable ] [lc] [line 8]
> > >> +!20 = metadata !{metadata !21}
> > >> +!21 = metadata !{metadata !"0x34\00argc\00argc\00\001\000\001",
> null,
> > >> metadata !14, metadata !11, i8* @argc, null} ; [ DW_TAG_variable ]
> > [argc]
> > >> [line 1] [def]
> > >> +!22 = metadata !{i32 2, metadata !"Dwarf Version", i32 4}
> > >> +!23 = metadata !{i32 2, metadata !"Debug Info Version", i32 2}
> > >> +!25 = metadata !{i32 8, i32 3, metadata !13, null}
> > >> +!29 = metadata !{metadata !"0x102"}               ; [
> > DW_TAG_expression ]
> > >>
> > >>
> > >> _______________________________________________
> > >> llvm-commits mailing list
> > >> llvm-commits at cs.uiuc.edu
> > >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> > >
> > >
> > >
> > > _______________________________________________
> > > llvm-commits mailing list
> > > llvm-commits at cs.uiuc.edu
> > > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> > >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
> _______________________________________________
> 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/20141212/ebd25148/attachment.html>

More information about the llvm-commits mailing list