[llvm] r215962 - [X86, X87 stackifier] Do not mark an operand of a debug instruction as kill.
Akira Hatanaka
ahatanak at gmail.com
Wed Sep 3 12:46:13 PDT 2014
Thanks Duncan.
On Tue, Sep 2, 2014 at 1:28 PM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:
>
> > 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140903/b30ca8e4/attachment.html>
More information about the llvm-commits
mailing list