<br><br><div class="gmail_quote">On Fri Dec 12 2014 at 10:49:12 AM Robinson, Paul <<a href="mailto:Paul_Robinson@playstation.sony.com">Paul_Robinson@playstation.sony.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">> -----Original Message-----<br>
> From: <a href="mailto:llvm-commits-bounces@cs.uiuc.edu" target="_blank">llvm-commits-bounces@cs.uiuc.<u></u>edu</a> [mailto:<a href="mailto:llvm-commits-" target="_blank">llvm-commits-</a><br>
> <a href="mailto:bounces@cs.uiuc.edu" target="_blank">bounces@cs.uiuc.edu</a>] On Behalf Of Andrea Di Biagio<br>
> Sent: Friday, December 12, 2014 10:31 AM<br>
> To: David Blaikie<br>
> Cc: <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
> Subject: Re: [llvm] r224126 - Reapply "[MachineScheduler] Fix for PR21807:<br>
> minor code difference building with/without -g."<br>
><br>
> On Fri, Dec 12, 2014 at 5:52 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br>
> ><br>
> ><br>
> > On Fri, Dec 12, 2014 at 7:09 AM, Andrea Di Biagio<br>
> > <<a href="mailto:Andrea_DiBiagio@sn.scee.net" target="_blank">Andrea_DiBiagio@sn.scee.net</a>> wrote:<br>
> >><br>
> >> Author: adibiagio<br>
> >> Date: Fri Dec 12 09:09:58 2014<br>
> >> New Revision: 224126<br>
> >><br>
> >> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=224126&view=rev" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project?rev=224126&view=rev</a><br>
> >> Log:<br>
> >> Reapply "[MachineScheduler] Fix for PR21807: minor code difference<br>
> >> building with/without -g."<br>
> >><br>
> >> This reapplies r224118 with a fix for test<br>
> >> 'misched-code-difference-with-<u></u>debug.ll'.<br>
> >> That test was failing on some buildbots because it was x86 specific but<br>
> it<br>
> >> was<br>
> >> missing a target triple.<br>
> >> Added an explicit triple to test misched-code-difference-with-<u></u>debug.ll.<br>
> >><br>
> >> Added:<br>
> >>     llvm/trunk/test/CodeGen/X86/<u></u>misched-code-difference-with-<u></u>debug.ll<br>
> >> Modified:<br>
> >>     llvm/trunk/lib/CodeGen/<u></u>MachineScheduler.cpp<br>
> >><br>
> >> Modified: llvm/trunk/lib/CodeGen/<u></u>MachineScheduler.cpp<br>
> >> URL:<br>
> >> <a href="http://llvm.org/viewvc/llvm-" target="_blank">http://llvm.org/viewvc/llvm-</a><br>
> project/llvm/trunk/lib/<u></u>CodeGen/MachineScheduler.cpp?<u></u>rev=224126&r1=224125&r<br>
> 2=224126&view=diff<br>
> >><br>
> >><br>
> ==============================<u></u>==============================<u></u>==============<br>
> ====<br>
> >> --- llvm/trunk/lib/CodeGen/<u></u>MachineScheduler.cpp (original)<br>
> >> +++ llvm/trunk/lib/CodeGen/<u></u>MachineScheduler.cpp Fri Dec 12 09:09:58<br>
> 2014<br>
> >> @@ -430,9 +430,11 @@ void MachineSchedulerBase::<u></u>scheduleRegio<br>
> >>        // instruction stream until we find the nearest boundary.<br>
> >>        unsigned NumRegionInstrs = 0;<br>
> >>        MachineBasicBlock::iterator I = RegionEnd;<br>
> >> -      for(;I != MBB->begin(); --I, --RemainingInstrs,<br>
> ++NumRegionInstrs)<br>
> >> {<br>
> >> +      for(;I != MBB->begin(); --I, --RemainingInstrs) {<br>
> >>          if (isSchedBoundary(std::prev(I), MBB, MF, TII, IsPostRA))<br>
> >>            break;<br>
> >> +        if (!I->isDebugValue())<br>
> >> +          ++NumRegionInstrs;<br>
> >>        }<br>
> >>        // Notify the scheduler of the region, even if we may skip<br>
> >> scheduling<br>
> >>        // it. Perhaps it still needs to be bundled.<br>
> >><br>
> >> Added: llvm/trunk/test/CodeGen/X86/<u></u>misched-code-difference-with-<br>
> debug.ll<br>
> >> URL:<br>
> >> <a href="http://llvm.org/viewvc/llvm-" target="_blank">http://llvm.org/viewvc/llvm-</a><br>
> project/llvm/trunk/test/<u></u>CodeGen/X86/misched-code-<u></u>difference-with-<br>
> debug.ll?rev=224126&view=auto<br>
> >><br>
> >><br>
> ==============================<u></u>==============================<u></u>==============<br>
> ====<br>
> >> --- llvm/trunk/test/CodeGen/X86/<u></u>misched-code-difference-with-<u></u>debug.ll<br>
> >> (added)<br>
> >> +++ llvm/trunk/test/CodeGen/X86/<u></u>misched-code-difference-with-<u></u>debug.ll<br>
> Fri<br>
> >> Dec 12 09:09:58 2014<br>
> >> @@ -0,0 +1,90 @@<br>
> >> +; RUN: llc < %s -march=x86-64 -mtriple=x86_64-unknown-<u></u>unknown<br>
> >> -mcpu=generic | FileCheck %s<br>
> >> +; Both functions should produce the same code. The presence of debug<br>
> >> values<br>
> >> +; should not affect the scheduling strategy.<br>
> >> +; Generated from:<br>
> >> +; char argc;<br>
> >> +; class C {<br>
> >> +; public:<br>
> >> +;   int test(char ,char ,char ,...);<br>
> >> +; };<br>
> >> +; void foo() {<br>
> >> +;   C c;<br>
> >> +;   char lc = argc;<br>
> >> +;   c.test(0,argc,0,lc);<br>
> >> +;   c.test(0,argc,0,lc);<br>
> >> +; }<br>
> ><br>
> ><br>
> > Is this a minimal reproduction? I'm slightly surprised at some of the<br>
> quirks<br>
> > here, and it might be best to at least highlight the bits that make this<br>
> > interesting (I assume it's not important that this is a non-static<br>
> member<br>
> > function, for example - it might just need an extra argument (add<br>
> another<br>
> > char argument and make it a free function, removing the class C)). Does<br>
> argc<br>
> > need to be copied like that, or could 'lc' just be another (possibly<br>
> local)<br>
> > variable (maybe it needs to not be optimzed away, in which case a global<br>
> > might be the simplest)? Does 'argc' have to appear between two other<br>
> zeros<br>
> > like that? etc...<br>
> ><br>
> > Anyawy, would be nice to ensure the test case is fully reduced - just<br>
> > checking.<br>
><br>
> Hi David,<br>
><br>
> As far as I understand, the only constraint is that the number of<br>
> instructions in a scheduling region (including dbg.values) has to be<br>
> above a certain threshold to trigger the change in behavior in the<br>
> scheduler.<br>
><br>
> I remember that Russ (the author of this test) tried to obtain a<br>
> minimal reproducible. However, I think you are right and this test can<br>
> probably be further simplified. I will ask him to have a look at see<br>
> if it can be further reduced.<br>
><br>
> Thanks,<br>
> Andrea<br>
<br>
IIRC what's required is that the extra dbg.value instructions have to make<br>
it *cross* the threshold.  By crossing the threshold, the behavior changes.<br>
--paulr<br>
<br></blockquote><div><br></div><div>Yes. That's correct.</div><div><br></div><div>-eric</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
><br>
> ><br>
> >><br>
> >> +;<br>
> >> +; with<br>
> >> +; clang -O2 -c test.cpp -emit-llvm -S<br>
> >> +; clang -O2 -c test.cpp -emit-llvm -S -g<br>
> >> +;<br>
> >> +<br>
> >> +<br>
> >> +%class.C = type { i8 }<br>
> >> +<br>
> >> +@argc = global i8 0, align 1<br>
> >> +<br>
> >> +declare i32 @test_function(%class.C*, i8 signext, i8 signext, i8<br>
> signext,<br>
> >> ...)<br>
> >> +<br>
> >> +; CHECK-LABEL: test_without_debug<br>
> >> +; CHECK: movl [[A:%[a-z]+]], [[B:%[a-z]+]]<br>
> >> +; CHECK-NEXT: movl [[A]], [[C:%[a-z]+]]<br>
> >> +define void @test_without_debug() {<br>
> >> +entry:<br>
> >> +  %c = alloca %class.C, align 1<br>
> >> +  %0 = load i8* @argc, align 1<br>
> >> +  %conv = sext i8 %0 to i32<br>
> >> +  %call = call i32 (%class.C*, i8, i8, i8, ...)*<br>
> @test_function(%class.C*<br>
> >> %c, i8 signext 0, i8 signext %0, i8 signext 0, i32 %conv)<br>
> >> +  %1 = load i8* @argc, align 1<br>
> >> +  %call2 = call i32 (%class.C*, i8, i8, i8, ...)*<br>
> >> @test_function(%class.C* %c, i8 signext 0, i8 signext %1, i8 signext 0,<br>
> i32<br>
> >> %conv)<br>
> >> +  ret void<br>
> >> +}<br>
> >> +<br>
> >> +; CHECK-LABEL: test_with_debug<br>
> >> +; CHECK: movl [[A]], [[B]]<br>
> >> +; CHECK-NEXT: movl [[A]], [[C]]<br>
> >> +define void @test_with_debug() {<br>
> >> +entry:<br>
> >> +  %c = alloca %class.C, align 1<br>
> >> +  %0 = load i8* @argc, align 1<br>
> >> +  tail call void @llvm.dbg.value(metadata !{i8 %0}, i64 0, metadata<br>
> !19,<br>
> >> metadata !29)<br>
> >> +  %conv = sext i8 %0 to i32<br>
> >> +  tail call void @llvm.dbg.value(metadata !{%class.C* %c}, i64 0,<br>
> >> metadata !18, metadata !29)<br>
> >> +  %call = call i32 (%class.C*, i8, i8, i8, ...)*<br>
> @test_function(%class.C*<br>
> >> %c, i8 signext 0, i8 signext %0, i8 signext 0, i32 %conv)<br>
> >> +  %1 = load i8* @argc, align 1<br>
> >> +  call void @llvm.dbg.value(metadata !{%class.C* %c}, i64 0, metadata<br>
> >> !18, metadata !29)<br>
> >> +  %call2 = call i32 (%class.C*, i8, i8, i8, ...)*<br>
> >> @test_function(%class.C* %c, i8 signext 0, i8 signext %1, i8 signext 0,<br>
> i32<br>
> >> %conv)<br>
> >> +  ret void<br>
> >> +}<br>
> >> +<br>
> >> +declare void @llvm.dbg.value(metadata, i64, metadata, metadata)<br>
> >> +<br>
> >> +!<a href="http://llvm.dbg.cu" target="_blank">llvm.dbg.cu</a> = !{!0}<br>
> >> +!llvm.module.flags = !{!22, !23}<br>
> >> +<br>
> >> +!0 = metadata !{metadata !"", metadata !1, metadata !2, metadata !3,<br>
> >> metadata !12, metadata !20, metadata !2} ; [ DW_TAG_compile_unit ]<br>
> >> [test.cpp] [DW_LANG_C_plus_plus]<br>
> >> +!1 = metadata !{metadata !"test.cpp", metadata !""}<br>
> >> +!2 = metadata !{}<br>
> >> +!3 = metadata !{metadata !4}<br>
> >> +!4 = metadata !{metadata !"0x2\00C\002\008\008\000\000\<u></u>000", metadata<br>
> !1,<br>
> >> null, null, metadata !5, null, null, metadata !"_ZTS1C"} ; [<br>
> >> DW_TAG_class_type ] [C] [line 2, size 8, align 8, offset 0] [def] [from<br>
> ]<br>
> >> +!5 = metadata !{metadata !6}<br>
> >> +!6 = metadata !{metadata !"", metadata !1, metadata !"_ZTS1C",<br>
> metadata<br>
> >> !7, null, null, null, null, null} ; [ DW_TAG_subprogram ] [line 4]<br>
> [public]<br>
> >> [test]<br>
> >> +!7 = metadata !{metadata !"", null, null, null, metadata !8, null,<br>
> null,<br>
> >> null} ; [ DW_TAG_subroutine_type ] [line 0, size 0, align 0, offset 0]<br>
> [from<br>
> >> ]<br>
> >> +!8 = metadata !{metadata !9, metadata !10, metadata !11, metadata !11,<br>
> >> metadata !11, null}<br>
> >> +!9 = metadata !{metadata !"", null, null} ; [ DW_TAG_base_type ] [int]<br>
> >> [line 0, size 32, align 32, offset 0, enc DW_ATE_signed]<br>
> >> +!10 = metadata !{metadata !"", null, null, metadata !"_ZTS1C"} ; [<br>
> >> DW_TAG_pointer_type ] [line 0, size 64, align 64, offset 0]<br>
> [artificial]<br>
> >> [from _ZTS1C]<br>
> >> +!11 = metadata !{metadata !"0x24\00char\000\008\008\000\<u></u>000\006",<br>
> null,<br>
> >> null} ; [ DW_TAG_base_type ] [char] [line 0, size 8, align 8, offset 0,<br>
> enc<br>
> >> DW_ATE_signed_char]<br>
> >> +!12 = metadata !{metadata !13}<br>
> >> +!13 = metadata !{metadata<br>
> >><br>
> !"0x2e\00test_with_debug\<u></u>00test_with_debug\00test_with_<u></u>debug\006\000\001\0<br>
> 00\000\00256\001\006",<br>
> >> metadata !1, metadata !14, metadata !15, null, void ()*<br>
> @test_with_debug,<br>
> >> null, null, metadata !17} ; [ DW_TAG_subprogram ] [line 6] [def]<br>
> >> [test_with_debug]<br>
> >> +!14 = metadata !{metadata !"0x29", metadata !1}<br>
> >> +!15 = metadata !{metadata !"0x15\00\000\000\000\000\000\<u></u>000", null,<br>
> null,<br>
> >> null, metadata !16, null, null, null} ; [ DW_TAG_subroutine_type ]<br>
> [line 0,<br>
> >> size 0, align 0, offset 0] [from ]<br>
> >> +!16 = metadata !{null}<br>
> >> +!17 = metadata !{metadata !18, metadata !19}<br>
> >> +!18 = metadata !{metadata !"0x100\00c\007\000", metadata !13, metadata<br>
> >> !14, metadata !"_ZTS1C"} ; [ DW_TAG_auto_variable ] [c] [line 7]<br>
> >> +!19 = metadata !{metadata !"0x100\00lc\008\000", metadata !13,<br>
> metadata<br>
> >> !14, metadata !11} ; [ DW_TAG_auto_variable ] [lc] [line 8]<br>
> >> +!20 = metadata !{metadata !21}<br>
> >> +!21 = metadata !{metadata !"0x34\00argc\00argc\00\001\<u></u>000\001", null,<br>
> >> metadata !14, metadata !11, i8* @argc, null} ; [ DW_TAG_variable ]<br>
> [argc]<br>
> >> [line 1] [def]<br>
> >> +!22 = metadata !{i32 2, metadata !"Dwarf Version", i32 4}<br>
> >> +!23 = metadata !{i32 2, metadata !"Debug Info Version", i32 2}<br>
> >> +!25 = metadata !{i32 8, i32 3, metadata !13, null}<br>
> >> +!29 = metadata !{metadata !"0x102"}               ; [<br>
> DW_TAG_expression ]<br>
> >><br>
> >><br>
> >> ______________________________<u></u>_________________<br>
> >> llvm-commits mailing list<br>
> >> <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
> >> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a><br>
> ><br>
> ><br>
> ><br>
> > ______________________________<u></u>_________________<br>
> > llvm-commits mailing list<br>
> > <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
> > <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a><br>
> ><br>
> ______________________________<u></u>_________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a><br>
<br>
______________________________<u></u>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a><br>
</blockquote></div>