<div dir="ltr">Thanks Jakob!<div><br></div><div style>Modified patch attached. Anyone adverse to this change?</div><div style><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Mar 15, 2013 at 1:43 PM, Jakob Stoklund Olesen <span dir="ltr"><<a href="mailto:stoklund@2pi.dk" target="_blank">stoklund@2pi.dk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><div class="im"><div>On Mar 15, 2013, at 10:41 AM, Justin Holewinski <<a href="mailto:justin.holewinski@gmail.com" target="_blank">justin.holewinski@gmail.com</a>> wrote:</div>
<br><blockquote type="cite"><div style="letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div dir="ltr">As far as I can tell, this change is just reversing the order of the __ftol2 calls. The end result is the elimination of the fxch at the expense of an additional spill, causing the test to fail. I'm not too familiar with the x87 FP stack, but this still looks correct. Does anyone see any actual issues with the modified output (from original post)? If not, I'll just adjust the test case.</div>
</div></blockquote><div><br></div></div>That change looks safe enough, you can just adjust the test case.</div><span class="HOEnZb"><font color="#888888"><div><br></div><div>/jakob</div></font></span><div><div class="h5">
<div><br><blockquote type="cite"><div style="letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div dir="ltr"><div>Diff of Original -> Patched</div><div><br>
</div><div><div><font face="courier new, monospace"> # After Instruction Selection:</font></div><div><font face="courier new, monospace"> # Machine code for function double_ui64_2: SSA</font></div><div><font face="courier new, monospace"> Frame Objects:</font></div>
<div><font face="courier new, monospace"> fi#-3: size=8, align=4, fixed, at location [SP+20]</font></div><div><font face="courier new, monospace"> fi#-2: size=8, align=4, fixed, at location [SP+12]</font></div><div><font face="courier new, monospace"> fi#-1: size=8, align=4, fixed, at location [SP+4]</font></div>
<div><font face="courier new, monospace"> fi#0: size=8, align=8, at location [SP+4]</font></div><div><font face="courier new, monospace"> fi#1: size=8, align=8, at location [SP+4]</font></div><div><font face="courier new, monospace"><br>
</font></div><div><font face="courier new, monospace"> BB#0: derived from LLVM BB %0</font></div><div><font face="courier new, monospace"> <span style="white-space:pre-wrap"> </span>%vreg2<def> = LD_Fp64m <fi#-3>, 1, %noreg, 0, %noreg, %FPSW<imp-def,dead>; mem:LD8[FixedStack-3] RFP64:%vreg2</font></div>
<div><font face="courier new, monospace"> <span style="white-space:pre-wrap"> </span>%vreg1<def> = LD_Fp64m <fi#-2>, 1, %noreg, 0, %noreg, %FPSW<imp-def,dead>; mem:LD8[FixedStack-2] RFP64:%vreg1</font></div>
<div><font face="courier new, monospace"> <span style="white-space:pre-wrap"> </span>%vreg0<def> = LD_Fp64m <fi#-1>, 1, %noreg, 0, %noreg, %FPSW<imp-def,dead>; mem:LD8[FixedStack-1] RFP64:%vreg0</font></div>
<div><font face="courier new, monospace"> <span style="white-space:pre-wrap"> </span>%vreg3<def> = DIV_Fp64 %vreg0, %vreg1, %FPSW<imp-def,dead>; RFP64:%vreg3,%vreg0,%vreg1</font></div><div><font face="courier new, monospace"> <span style="white-space:pre-wrap"> </span>%vreg4<def> = SUB_Fp64 %vreg0, %vreg2, %FPSW<imp-def,dead>; RFP64:%vreg4,%vreg0,%vreg2</font></div>
<div><font face="courier new, monospace">-<span style="white-space:pre-wrap"> </span>WIN_FTOL_64 %vreg4<kill>, %EAX<imp-def>, %EDX<imp-def>, %EFLAGS<imp-def,dead>; RFP64:%vreg4</font></div><div><font face="courier new, monospace">+<span style="white-space:pre-wrap"> </span>WIN_FTOL_64 %vreg3<kill>, %EAX<imp-def>, %EDX<imp-def>, %EFLAGS<imp-def,dead>; RFP64:%vreg3</font></div>
<div><font face="courier new, monospace"> <span style="white-space:pre-wrap"> </span>%vreg5<def> = COPY %EAX; GR32:%vreg5</font></div><div><font face="courier new, monospace"> <span style="white-space:pre-wrap"> </span>%vreg6<def> = COPY %EDX; GR32:%vreg6</font></div>
<div><font face="courier new, monospace">-<span style="white-space:pre-wrap"> </span>WIN_FTOL_64 %vreg3<kill>, %EAX<imp-def>, %EDX<imp-def>, %EFLAGS<imp-def,dead>; RFP64:%vreg3</font></div><div><font face="courier new, monospace">+<span style="white-space:pre-wrap"> </span>WIN_FTOL_64 %vreg4<kill>, %EAX<imp-def>, %EDX<imp-def>, %EFLAGS<imp-def,dead>; RFP64:%vreg4</font></div>
<div><font face="courier new, monospace"> <span style="white-space:pre-wrap"> </span>%vreg7<def> = COPY %EAX; GR32:%vreg7</font></div><div><font face="courier new, monospace"> <span style="white-space:pre-wrap"> </span>%vreg8<def> = COPY %EDX; GR32:%vreg8</font></div>
<div><font face="courier new, monospace">-<span style="white-space:pre-wrap"> </span>%vreg9<def,tied1> = SUB32rr %vreg7<tied0>, %vreg5, %EFLAGS<imp-def>; GR32:%vreg9,%vreg7,%vreg5</font></div><div><font face="courier new, monospace">-<span style="white-space:pre-wrap"> </span>%vreg10<def,tied1> = SBB32rr %vreg8<tied0>, %vreg6, %EFLAGS<imp-def,dead>, %EFLAGS<imp-use>; GR32:%vreg10,%vreg8,%vreg6</font></div>
<div><font face="courier new, monospace">+<span style="white-space:pre-wrap"> </span>%vreg9<def,tied1> = SUB32rr %vreg5<tied0>, %vreg7, %EFLAGS<imp-def>; GR32:%vreg9,%vreg5,%vreg7</font></div><div><font face="courier new, monospace">+<span style="white-space:pre-wrap"> </span>%vreg10<def,tied1> = SBB32rr %vreg6<tied0>, %vreg8, %EFLAGS<imp-def,dead>, %EFLAGS<imp-use>; GR32:%vreg10,%vreg6,%vreg8</font></div>
<div><font face="courier new, monospace"> <span style="white-space:pre-wrap"> </span>%EAX<def> = COPY %vreg9; GR32:%vreg9</font></div><div><font face="courier new, monospace"> <span style="white-space:pre-wrap"> </span>%EDX<def> = COPY %vreg10; GR32:%vreg10</font></div>
<div><font face="courier new, monospace"> <span style="white-space:pre-wrap"> </span>RET %EAX, %EDX</font></div><div><font face="courier new, monospace"><br></font></div><div><font face="courier new, monospace"> # End machine code for function double_ui64_2.</font></div>
</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Mar 15, 2013 at 12:31 PM, Jakob Stoklund Olesen<span> </span><span dir="ltr"><<a href="mailto:stoklund@2pi.dk" target="_blank">stoklund@2pi.dk</a>></span><span> </span>wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word">Hi Justin,<div><br></div>
<div>The x87 stack is always a bit strange. It's probably easier to compare the outputs of -print-machineinstrs.</div><div><br></div><div>/jakob</div><div><br><div><div><div><div>On Mar 15, 2013, at 7:54 AM, Justin Holewinski <<a href="mailto:justin.holewinski@gmail.com" target="_blank">justin.holewinski@gmail.com</a>> wrote:</div>
<br></div></div><blockquote type="cite"><div style="letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div><div><div dir="ltr"><div>The attached patch modifies DAGTypeLegalizer and SelectionDAGISel to propagate DAG node ordering. We currently (for better or for worse) use source scheduling, and we're being hit by issues in the scheduler due to the ordering being lost.</div>
<div><br></div><div><br></div><div>One issue with this patch is that is causes one X86 test to fail: win_ftol2</div><div><br></div><div>The only meaningful difference I can spot (other than an extra spill) is that the 'fxch' instruction is eliminated. Can anyone more familiar with the X86 back-end comment on why DAG node ordering would affect this test? I've narrowed the issue down to being caused by this patch's change to DAGTypeLegalizer::SetExpandedInteger. (e.g. if this method propagates DAG node ordering in the newly created nodes, this test starts to fail).</div>
<div><br></div><div><br></div><div>Current Output:</div><div><br></div><div><div><font face="courier new, monospace">_double_ui64_2: # @double_ui64_2</font></div><div><font face="courier new, monospace"># BB#0:</font></div>
<div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>pushl<span style="white-space:pre-wrap"> </span>%ebp</font></div><div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>movl<span style="white-space:pre-wrap"> </span>%esp, %ebp</font></div>
<div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>pushl<span style="white-space:pre-wrap"> </span>%esi</font></div><div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>andl<span style="white-space:pre-wrap"> </span>$-8, %esp</font></div>
<div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>subl<span style="white-space:pre-wrap"> </span>$32, %esp</font></div><div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>fldl<span style="white-space:pre-wrap"> </span>24(%ebp)</font></div>
<div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>fldl<span style="white-space:pre-wrap"> </span>16(%ebp)</font></div><div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>fldl<span style="white-space:pre-wrap"> </span>8(%ebp)</font></div>
<div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>fdiv<span style="white-space:pre-wrap"> </span>%st(0), %st(1)</font></div><div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>fsubp<span style="white-space:pre-wrap"> </span>%st(2)</font></div>
<div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>fxch</font></div><div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>calll<span style="white-space:pre-wrap"> </span>__ftol2</font></div>
<div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>movl<span style="white-space:pre-wrap"> </span>%eax, 4(%esp) # 4-byte Spill</font></div><div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>movl<span style="white-space:pre-wrap"> </span>%edx, (%esp) # 4-byte Spill</font></div>
<div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>calll<span style="white-space:pre-wrap"> </span>__ftol2</font></div><div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>movl<span style="white-space:pre-wrap"> </span>4(%esp), %ecx # 4-byte Reload</font></div>
<div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>subl<span style="white-space:pre-wrap"> </span>%ecx, %eax</font></div><div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>movl<span style="white-space:pre-wrap"> </span>(%esp), %esi # 4-byte Reload</font></div>
<div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>sbbl<span style="white-space:pre-wrap"> </span>%esi, %edx</font></div><div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>leal<span style="white-space:pre-wrap"> </span>-4(%ebp), %esp</font></div>
<div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>popl<span style="white-space:pre-wrap"> </span>%esi</font></div><div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>popl<span style="white-space:pre-wrap"> </span>%ebp</font></div>
<div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>ret</font></div><div><br></div><div><br></div><div>Output With Patch:</div><div><br></div><div><div><font face="courier new, monospace">_double_ui64_2: # @double_ui64_2</font></div>
<div><font face="courier new, monospace"># BB#0:</font></div><div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>pushl<span style="white-space:pre-wrap"> </span>%ebp</font></div><div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>movl<span style="white-space:pre-wrap"> </span>%esp, %ebp</font></div>
<div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>andl<span style="white-space:pre-wrap"> </span>$-8, %esp</font></div><div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>subl<span style="white-space:pre-wrap"> </span>$32, %esp</font></div>
<div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>fldl<span style="white-space:pre-wrap"> </span>24(%ebp)</font></div><div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>fldl<span style="white-space:pre-wrap"> </span>16(%ebp)</font></div>
<div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>fldl<span style="white-space:pre-wrap"> </span>8(%ebp)</font></div><div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>fdiv<span style="white-space:pre-wrap"> </span>%st(0), %st(1)</font></div>
<div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>fsubp<span style="white-space:pre-wrap"> </span>%st(2)</font></div><div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>calll<span style="white-space:pre-wrap"> </span>__ftol2</font></div>
<div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>movl<span style="white-space:pre-wrap"> </span>%eax, 12(%esp) # 4-byte Spill</font></div><div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>movl<span style="white-space:pre-wrap"> </span>%edx, 8(%esp) # 4-byte Spill</font></div>
<div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>calll<span style="white-space:pre-wrap"> </span>__ftol2</font></div><div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>movl<span style="white-space:pre-wrap"> </span>12(%esp), %ecx # 4-byte Reload</font></div>
<div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>subl<span style="white-space:pre-wrap"> </span>%eax, %ecx</font></div><div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>movl<span style="white-space:pre-wrap"> </span>8(%esp), %eax # 4-byte Reload</font></div>
<div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>sbbl<span style="white-space:pre-wrap"> </span>%edx, %eax</font></div><div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>movl<span style="white-space:pre-wrap"> </span>%eax, 4(%esp) # 4-byte Spill</font></div>
<div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>movl<span style="white-space:pre-wrap"> </span>%ecx, %eax</font></div><div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>movl<span style="white-space:pre-wrap"> </span>4(%esp), %edx # 4-byte Reload</font></div>
<div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>movl<span style="white-space:pre-wrap"> </span>%ebp, %esp</font></div><div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>popl<span style="white-space:pre-wrap"> </span>%ebp</font></div>
<div><font face="courier new, monospace"><span style="white-space:pre-wrap"> </span>ret</font></div></div></div><br clear="all"><div><br></div>--<span> </span><br><br><div>Thanks,</div><div><br></div><div>Justin Holewinski</div>
</div></div></div><span><0001-Propagate-DAG-node-ordering-during-type-legalization.patch></span>_______________________________________________<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/mailman/listinfo/llvm-commits</a></div></blockquote></div><br></div></div></blockquote></div><br><br clear="all"><div>
<br></div>--<span> </span><br><br><div>Thanks,</div><div><br></div><div>Justin Holewinski</div></div></div></blockquote></div><br></div></div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><br><div>Thanks,</div>
<div><br></div><div>Justin Holewinski</div>
</div>