<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Hi Hal and Andy,<div><br></div><div>thanks for looking at this. I started to rewrite the patch to incorporate the changes you suggested, but the changed schedule has uncovered some other issues I need to resolve first to fix the existing unit test.</div><div><br></div><div>Cheers,</div><div>Juergen</div><div><br></div><div><br></div><div><div><div>On Nov 19, 2013, at 10:17 AM, Andrew Trick <<a href="mailto:atrick@apple.com">atrick@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br class="Apple-interchange-newline">On Nov 19, 2013, at 10:01 AM, Hal Finkel <<a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>> wrote:</div><br class="Apple-interchange-newline" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">----- Original Message -----<br><blockquote type="cite">From: "Andrew Trick" <<a href="mailto:atrick@apple.com">atrick@apple.com</a>><br>To: "Hal Finkel" <<a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>><br>Cc: "Juergen Ributzka" <<a href="mailto:juergen@apple.com">juergen@apple.com</a>>, "llvm commits" <<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a>>, "Bill Wendling"<br><<a href="mailto:isanbard@gmail.com">isanbard@gmail.com</a>><br>Sent: Tuesday, November 19, 2013 11:54:57 AM<br>Subject: Re: [PATCH][DAG] Fix non-deterministic code generation<br><br><br><br><br>On Nov 18, 2013, at 11:16 AM, Hal Finkel <<span class="Apple-converted-space"> </span><a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a><span class="Apple-converted-space"> </span>> wrote:<br><br><br><br>----- Original Message -----<br><br><br>From: "Juergen Ributzka" <<span class="Apple-converted-space"> </span><a href="mailto:juergen@apple.com">juergen@apple.com</a><span class="Apple-converted-space"> </span>><br>To: "Hal Finkel" <<span class="Apple-converted-space"> </span><a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a><span class="Apple-converted-space"> </span>><br>Cc: "LLVM Commits" <<span class="Apple-converted-space"> </span><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><span class="Apple-converted-space"> </span>>, "Andrew Trick" <<br><a href="mailto:atrick@apple.com">atrick@apple.com</a><span class="Apple-converted-space"> </span>>, "Bill Wendling"<br><<span class="Apple-converted-space"> </span><a href="mailto:isanbard@gmail.com">isanbard@gmail.com</a><span class="Apple-converted-space"> </span>><br>Sent: Monday, November 18, 2013 12:59:20 PM<br>Subject: Re: [PATCH][DAG] Fix non-deterministic code generation<br><br><br>SelectionDAGISel::LowerArguments() is called before the first call to<br>SelectionDAGBuilder::visit(). This is why I still see SDNodes with<br>IR Order 0 in the DAG.<br><br>Okay, thanks for looking into this. I'm okay with the change you've<br>proposed, but please also add a comment that we're initializing the<br>value to zero to make sure that nodes generated from<br>SelectionDAGISel::LowerArguments are assigned a source order of<br>zero.<br><br><br>Thanks for reviewing this Hal.<br><br><br>Failing to reset IROrder was definitely an oversight. I talked with<br>Juergen and we agreed that resetting to one makes a bit more sense<br>since we use IROrder==0 for scheduling nodes that don’t have a<br>corresponding SD node (it’s a special case).<br></blockquote><br>SGTM. Just to clarify, we'll have:<br><br>NodeOrder == 0 : A special case for nodes without a SDNode<br></div></blockquote><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br></div><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">I would say “A special case for nodes with no valid IR Order” because I think it’s possible to create SD nodes without either providing an initialized IR order (SDLoc) or mapping them back to an existing SD node.</div><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">NodeOrder == 1 : Nodes from SelectionDAGISel::LowerArguments<br>NodeOrder > 1 : Everything else.<br></div></blockquote><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br></div><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">That is my understanding. Yes, we need a comment.</div><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br></div><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">Juergen, I think you can checkin if you confirm this behavior and add comments.</div><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br></div><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">Thanks!</div><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">-Andy</div><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">(I just want there to be a comment about this somewhere)<br><br>-Hal<br><br><blockquote type="cite"><br><br>-Andy<br><br><br><br><br>-Hal<br><br><br><br><br><br>-Juergen<br><br><br>On Nov 18, 2013, at 10:37 AM, Hal Finkel <<span class="Apple-converted-space"> </span><a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a><span class="Apple-converted-space"> </span>> wrote:<br><br><br><br>----- Original Message -----<br><br><br>From: "Juergen Ributzka" <<span class="Apple-converted-space"> </span><a href="mailto:juergen@apple.com">juergen@apple.com</a><span class="Apple-converted-space"> </span>><br>To: "Hal Finkel" <<span class="Apple-converted-space"> </span><a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a><span class="Apple-converted-space"> </span>><br>Cc: "LLVM Commits" <<span class="Apple-converted-space"> </span><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><span class="Apple-converted-space"> </span>><br>Sent: Monday, November 18, 2013 11:59:17 AM<br>Subject: Re: [PATCH][DAG] Fix non-deterministic code generation<br><br><br>True, the relative order is the same, but the code that sorts the<br>availability list (src_ls_rr_sort::operator()) is sensitive to base<br>0.<br><br><br>// Prefer an ordering where the lower the non-zero order number, the<br>higher<br>// the preference.<br>if ((LOrder || ROrder) && LOrder != ROrder)<br>return LOrder != 0 && (LOrder < ROrder || ROrder == 0);<br><br><br>LOrder and/or ROrder can be 0 if the IR Order is 0 or if there is no<br>SDNode associated with the scheduling unit. Considering that 0 seems<br>to indicate a special case, reseting IR Order to 0 doesn’t seem to<br>be a good idea and maybe it should be reset to 1. Another solution<br>would be to change the above code to be insensitive to base 0. What<br>do you think?<br><br>I don't understand the rationale for the current behavior. :( Andy?<br>Bill?<br><br>FWIW, it looks like SelectionDAGBuilder::visit increments SDNodeOrder<br>before it calls SelectionDAGBuilder::visit(unsigned Opcode, ...), so<br>setting it to 0 is probably okay.<br><br>-Hal<br><br><br><br><br><br>In general resting IR base still seems to be a good for the reason<br>you mentioned before.<br><br><br>-Juergen<br><br><br><br><br>On Nov 17, 2013, at 5:09 AM, Hal Finkel <<span class="Apple-converted-space"> </span><a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a><span class="Apple-converted-space"> </span>> wrote:<br><br><br><br>----- Original Message -----<br><br><br>From: "Juergen Ributzka" <<span class="Apple-converted-space"> </span><a href="mailto:juergen@apple.com">juergen@apple.com</a><span class="Apple-converted-space"> </span>><br>To: "Hal Finkel" <<span class="Apple-converted-space"> </span><a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a><span class="Apple-converted-space"> </span>><br>Cc: "LLVM Commits" <<span class="Apple-converted-space"> </span><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><span class="Apple-converted-space"> </span>><br>Sent: Sunday, November 17, 2013 1:02:37 AM<br>Subject: Re: [PATCH][DAG] Fix non-deterministic code generation<br><br>Hi Hal,<br><br>the availability queue in the list scheduler returns the nodes in IR<br>order. If there is more than one node ready to schedule, then the<br>difference in the IR order will produce different schedules.<br><br>Maybe I'm missing something, but that does not seem to explain the<br>problem. The relative order of the nodes should be the same whether<br>the first node starts with 0 or 29348.<br><br>It seems like we should do this anyway (or make the counter 64 bits).<br>For one thing, on really large inputs, the current 32-bit counter<br>could wrap (which would certainly produce this problem). But you're<br>seeing some other issue here.<br><br>-Hal<br><br><br><br><br>-Juergen<br><br>On Nov 16, 2013, at 10:40 PM, Hal Finkel <<span class="Apple-converted-space"> </span><a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a><span class="Apple-converted-space"> </span>> wrote:<br><br><br><br>Juergen,<br><br>Do you happen to know what in CodeGen is sensitive to the absolute<br>value of the node order?<br><br>-Hal<br><br>----- Original Message -----<br><br><br>From: "Juergen Ributzka" <<span class="Apple-converted-space"> </span><a href="mailto:juergen@apple.com">juergen@apple.com</a><span class="Apple-converted-space"> </span>><br>To: "LLVM Commits" <<span class="Apple-converted-space"> </span><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><span class="Apple-converted-space"> </span>><br>Sent: Saturday, November 16, 2013 11:04:29 PM<br>Subject: [PATCH][DAG] Fix non-deterministic code generation<br><br><br><br>Hi @ll,<br><br>this patch resets SDNodeOrder in the SelectionDAGBuilder before<br>processing the IR of a new function. This is required to obtain<br>deterministic code generation for a function regardless of its<br>location in the source file.<br><br>Cheers,<br>Juergen<br><br><br><br><br><br><br><br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br><br><br>--<br>Hal Finkel<br>Assistant Computational Scientist<br>Leadership Computing Facility<br>Argonne National Laboratory<br><br><br><br>--<br>Hal Finkel<br>Assistant Computational Scientist<br>Leadership Computing Facility<br>Argonne National Laboratory<br><br><br>--<br>Hal Finkel<br>Assistant Computational Scientist<br>Leadership Computing Facility<br>Argonne National Laboratory<br><br><br>--<br>Hal Finkel<br>Assistant Computational Scientist<br>Leadership Computing Facility<br>Argonne National Laboratory<br><br></blockquote><br>--<span class="Apple-converted-space"> </span><br>Hal Finkel<br>Assistant Computational Scientist<br>Leadership Computing Facility<br>Argonne National Laboratory</div></blockquote></blockquote></div><br></div></body></html>