[llvm] r215962 - [X86, X87 stackifier] Do not mark an operand of a debug instruction as kill.

Duncan P. N. Exon Smith dexonsmith at apple.com
Tue Sep 2 13:28:21 PDT 2014


> On 2014 Aug 19, at 13:55, Akira Hatanaka <ahatanak at gmail.com> wrote:
> 
> On Mon, Aug 18, 2014 at 10:08 PM, David Blaikie <dblaikie at gmail.com> wrote:
> On Mon, Aug 18, 2014 at 7:09 PM, Akira Hatanaka <ahatanaka at apple.com> wrote:
> > Author: ahatanak
> > Date: Mon Aug 18 21:09:57 2014
> > New Revision: 215962
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=215962&view=rev
> > Log:
> > [X86, X87 stackifier] Do not mark an operand of a debug instruction as kill.
> >
> > <rdar://problem/16952634>
> >
> >
> > Added:
> >     llvm/trunk/test/CodeGen/X86/fpstack-debuginstr-kill.ll
> > Modified:
> >     llvm/trunk/lib/Target/X86/X86FloatingPoint.cpp
> >
> > Modified: llvm/trunk/lib/Target/X86/X86FloatingPoint.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86FloatingPoint.cpp?rev=215962&r1=215961&r2=215962&view=diff
> > ==============================================================================
> > --- llvm/trunk/lib/Target/X86/X86FloatingPoint.cpp (original)
> > +++ llvm/trunk/lib/Target/X86/X86FloatingPoint.cpp Mon Aug 18 21:09:57 2014
> > @@ -1654,6 +1654,9 @@ void FPS::setKillFlags(MachineBasicBlock
> >
> >    for (MachineBasicBlock::reverse_iterator I = MBB.rbegin(), E = MBB.rend();
> >         I != E; ++I) {
> > +    if (I->isDebugValue())
> > +      continue;
> > +
> >      BitVector Defs(8);
> >      SmallVector<MachineOperand *, 2> Uses;
> >      MachineInstr &MI = *I;
> >
> > Added: llvm/trunk/test/CodeGen/X86/fpstack-debuginstr-kill.ll
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/fpstack-debuginstr-kill.ll?rev=215962&view=auto
> > ==============================================================================
> > --- llvm/trunk/test/CodeGen/X86/fpstack-debuginstr-kill.ll (added)
> > +++ llvm/trunk/test/CodeGen/X86/fpstack-debuginstr-kill.ll Mon Aug 18 21:09:57 2014
> > @@ -0,0 +1,71 @@
> > +; RUN: llc < %s -mcpu=generic -mtriple=i386-apple-darwin -no-integrated-as
> 
>> Generally we've made a habit of checking in the user-level source
>> along with the IR for debug info tests, to make them easier to
>> maintain in the future, should the schema change greatly, etc. But I
>> realize this is swift code (guessing by the 'sw' prefix in the label
>> names) & with the swift compiler not being part of the LLVM project (&
>> still changing a lot) adding swift code here's probably not much help.
>> 
>> At least a bit more detailed documentation about which part of all of
>> this code is relevant might be helpful?
>> 
>> Could the code be simplified (generally we try to simplify the
>> input/source code, rather than hand-simplify the IR, in these debug
>> info tests - but since there's no source, the IR  could be
>> hand-simplified (but I'd suggest trying to simplify the input source
>> as much as possible first - since that'll also simplify the metadata,
>> too)). Perhaps there's a C++ reproduction you could use, in which case
>> we wouldn't have to worry about the swiftyness of the code and it
>> might be more obvious?
> 
> 
> The source code is actually a c++ program, not a swift code, so I can check it in if that's necessary.
> 
> I tried simplifying the source code first and created a new test case as you've suggested. Do the attached tests look fine to you? Also, where do the source c++ programs live in the test directory?

I think you can include the C++ source (commented out) in the .ll file
directly.

>  
>> Is this test just checking we don't crash? (I don't see any FileCheck,
>> etc) "does not crash" isn't a great thing to test for. There must've
>> been some behavior that was expected of this code other than "does not
>> crash" - perhaps we could/should test for that behavior?
>> 
> 
> The patch fixes a bug where a function in X86FloatingPoint.cpp was trying to add a kill flag to a debug instruction, which was causing an assert to fire. The test is just checking whether the pass runs to completion without crashing. I can certainly add FileCheck check strings to the test, but to be honest, I don't know what difference that would make. Do you have any suggestions on what it should check? Would something like "; CHECK: function_label" be sufficient?
> 
> 
>  
> > +
> > + at g1 = global double 0.000000e+00, align 8
> > + at g2 = global i32 0, align 4
> > +
> > +define void @_Z16fpuop_arithmeticjj(i32, i32) {
> > +entry:
> > +  switch i32 undef, label %sw.bb.i1921 [
> > +  ]
> > +
> > +sw.bb261:                                         ; preds = %entry, %entry
> > +  unreachable
> > +
> > +sw.bb.i1921:                                      ; preds = %if.end504
> > +  switch i32 undef, label %if.end511 [
> > +    i32 1, label %sw.bb27.i
> > +  ]
> > +
> > +sw.bb27.i:                                        ; preds = %sw.bb.i1921
> > +  %conv.i.i1923 = fpext float undef to x86_fp80
> > +  br label %if.end511
> > +
> > +if.end511:                                        ; preds = %sw.bb27.i, %sw.bb13.i
> > +  %src.sroa.0.0.src.sroa.0.0.2280 = phi x86_fp80 [ %conv.i.i1923, %sw.bb27.i ], [ undef, %sw.bb.i1921 ]
> > +  switch i32 undef, label %sw.bb992 [
> > +    i32 3, label %sw.bb735
> > +    i32 18, label %if.end41.i2210
> > +  ]
> > +
> > +sw.bb735:                                         ; preds = %if.end511
> > +  %2 = call x86_fp80 asm sideeffect "frndint", "={st},0,~{dirflag},~{fpsr},~{flags}"(x86_fp80 %src.sroa.0.0.src.sroa.0.0.2280)
> > +  unreachable
> > +
> > +if.end41.i2210:                                   ; preds = %if.end511
> > +  call void @llvm.dbg.value(metadata !{x86_fp80 %src.sroa.0.0.src.sroa.0.0.2280}, i64 0, metadata !20)
> > +  unreachable
> > +
> > +sw.bb992:                                         ; preds = %if.end511
> > +  ret void
> > +}
> > +
> > +declare void @llvm.dbg.value(metadata, i64, metadata)
> > +
> > +!llvm.dbg.cu = !{!0}
> > +!llvm.module.flags = !{!24, !25}
> > +!0 = metadata !{i32 786449, metadata !1, i32 4, metadata !"clang version 3.6.0 (http://llvm.org/git/clang 8444ae7cfeaefae031f8fedf0d1435ca3b14d90b) (http://llvm.org/git/llvm 886f0101a7d176543b831f5efb74c03427244a55)", i1 true, metadata !"", i32 0, metadata !2, metadata !2, metadata !3, metadata !21, metadata !2, metadata !"", i32 1} ; [ DW_TAG_compile_unit ] [x87stackifier/fpu_ieee.cpp] [DW_LANG_C_plus_plus]
> > +!1 = metadata !{metadata !"fpu_ieee.cpp", metadata !"x87stackifier"}
> > +!2 = metadata !{}
> > +!3 = metadata !{metadata !4}
> > +!4 = metadata !{i32 786478, metadata !5, metadata !6, metadata !"fpuop_arithmetic", metadata !"fpuop_arithmetic", metadata !"_Z16fpuop_arithmeticjj", i32 11, metadata !7, i1 false, i1 true, i32 0, i32 0, null, i32 256, i1 true, void (i32, i32)* @_Z16fpuop_arithmeticjj, null, null, metadata !10, i32 13} ; [ DW_TAG_subprogram ] [line 11] [def] [scope 13] [fpuop_arithmetic]
> > +!5 = metadata !{metadata !"f1.cpp", metadata !"x87stackifier"}
> > +!6 = metadata !{i32 786473, metadata !5}          ; [ DW_TAG_file_type ] [x87stackifier/f1.cpp]
> > +!7 = metadata !{i32 786453, i32 0, null, metadata !"", i32 0, i64 0, i64 0, i64 0, i32 0, null, metadata !8, i32 0, null, null, null} ; [ DW_TAG_subroutine_type ] [line 0, size 0, align 0, offset 0] [from ]
> > +!8 = metadata !{null, metadata !9, metadata !9}
> > +!9 = metadata !{i32 786468, null, null, metadata !"unsigned int", i32 0, i64 32, i64 32, i64 0, i32 0, i32 7} ; [ DW_TAG_base_type ] [unsigned int] [line 0, size 32, align 32, offset 0, enc DW_ATE_unsigned]
> > +!10 = metadata !{metadata !11, metadata !12, metadata !13, metadata !18, metadata !20}
> > +!11 = metadata !{i32 786689, metadata !4, metadata !"", metadata !6, i32 16777227, metadata !9, i32 0, i32 0} ; [ DW_TAG_arg_variable ] [line 11]
> > +!12 = metadata !{i32 786689, metadata !4, metadata !"", metadata !6, i32 33554443, metadata !9, i32 0, i32 0} ; [ DW_TAG_arg_variable ] [line 11]
> > +!13 = metadata !{i32 786688, metadata !4, metadata !"x", metadata !6, i32 14, metadata !14, i32 0, i32 0} ; [ DW_TAG_auto_variable ] [x] [line 14]
> > +!14 = metadata !{i32 786454, metadata !5, null, metadata !"fpu_extended", i32 3, i64 0, i64 0, i64 0, i32 0, metadata !15} ; [ DW_TAG_typedef ] [fpu_extended] [line 3, size 0, align 0, offset 0] [from fpu_register]
> > +!15 = metadata !{i32 786454, metadata !5, null, metadata !"fpu_register", i32 2, i64 0, i64 0, i64 0, i32 0, metadata !16} ; [ DW_TAG_typedef ] [fpu_register] [line 2, size 0, align 0, offset 0] [from uae_f64]
> > +!16 = metadata !{i32 786454, metadata !5, null, metadata !"uae_f64", i32 1, i64 0, i64 0, i64 0, i32 0, metadata !17} ; [ DW_TAG_typedef ] [uae_f64] [line 1, size 0, align 0, offset 0] [from double]
> > +!17 = metadata !{i32 786468, null, null, metadata !"double", i32 0, i64 64, i64 64, i64 0, i32 0, i32 4} ; [ DW_TAG_base_type ] [double] [line 0, size 64, align 64, offset 0, enc DW_ATE_float]
> > +!18 = metadata !{i32 786688, metadata !4, metadata !"a", metadata !6, i32 15, metadata !19, i32 0, i32 0} ; [ DW_TAG_auto_variable ] [a] [line 15]
> > +!19 = metadata !{i32 786468, null, null, metadata !"int", i32 0, i64 32, i64 32, i64 0, i32 0, i32 5} ; [ DW_TAG_base_type ] [int] [line 0, size 32, align 32, offset 0, enc DW_ATE_signed]
> > +!20 = metadata !{i32 786688, metadata !4, metadata !"value", metadata !6, i32 16, metadata !14, i32 0, i32 0} ; [ DW_TAG_auto_variable ] [value] [line 16]
> > +!21 = metadata !{metadata !22, metadata !23}
> > +!22 = metadata !{i32 786484, i32 0, null, metadata !"g1", metadata !"g1", metadata !"", metadata !6, i32 5, metadata !14, i32 0, i32 1, double* @g1, null} ; [ DW_TAG_variable ] [g1] [line 5] [def]
> > +!23 = metadata !{i32 786484, i32 0, null, metadata !"g2", metadata !"g2", metadata !"", metadata !6, i32 6, metadata !19, i32 0, i32 1, i32* @g2, null} ; [ DW_TAG_variable ] [g2] [line 6] [def]
> > +!24 = metadata !{i32 2, metadata !"Dwarf Version", i32 2}
> > +!25 = metadata !{i32 2, metadata !"Debug Info Version", i32 1}
> >
> >
> > _______________________________________________
> > 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
> 
> <fpstack-debuginstr-kill.cpp><fpstack-debuginstr-kill.ll>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits




More information about the llvm-commits mailing list