[PATCH][DAG] Fix non-deterministic code generation

[Hal Finkel]

----- Original Message -----
> From: "Juergen Ributzka" <juergen at apple.com>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: "LLVM Commits" <llvm-commits at cs.uiuc.edu>, "Andrew Trick" <atrick at apple.com>, "Bill Wendling"
> <isanbard at gmail.com>
> Sent: Monday, November 18, 2013 12:59:20 PM
> Subject: Re: [PATCH][DAG] Fix non-deterministic code generation
> 
> 
> SelectionDAGISel::LowerArguments() is called before the first call to
> SelectionDAGBuilder::visit(). This is why I still see SDNodes with
> IR Order 0 in the DAG.

Okay, thanks for looking into this. I'm okay with the change you've proposed, but please also add a comment that we're initializing the value to zero to make sure that nodes generated from SelectionDAGISel::LowerArguments are assigned a source order of zero.

 -Hal

> 
> 
> -Juergen
> 
> 
> On Nov 18, 2013, at 10:37 AM, Hal Finkel < hfinkel at anl.gov > wrote:
> 
> 
> 
> ----- Original Message -----
> 
> 
> From: "Juergen Ributzka" < juergen at apple.com >
> To: "Hal Finkel" < hfinkel at anl.gov >
> Cc: "LLVM Commits" < llvm-commits at cs.uiuc.edu >
> Sent: Monday, November 18, 2013 11:59:17 AM
> Subject: Re: [PATCH][DAG] Fix non-deterministic code generation
> 
> 
> True, the relative order is the same, but the code that sorts the
> availability list (src_ls_rr_sort::operator()) is sensitive to base
> 0.
> 
> 
> // Prefer an ordering where the lower the non-zero order number, the
> higher
> // the preference.
> if ((LOrder || ROrder) && LOrder != ROrder)
> return LOrder != 0 && (LOrder < ROrder || ROrder == 0);
> 
> 
> LOrder and/or ROrder can be 0 if the IR Order is 0 or if there is no
> SDNode associated with the scheduling unit. Considering that 0 seems
> to indicate a special case, reseting IR Order to 0 doesn’t seem to
> be a good idea and maybe it should be reset to 1. Another solution
> would be to change the above code to be insensitive to base 0. What
> do you think?
> 
> I don't understand the rationale for the current behavior. :( Andy?
> Bill?
> 
> FWIW, it looks like SelectionDAGBuilder::visit increments SDNodeOrder
> before it calls SelectionDAGBuilder::visit(unsigned Opcode, ...), so
> setting it to 0 is probably okay.
> 
> -Hal
> 
> 
> 
> 
> 
> In general resting IR base still seems to be a good for the reason
> you mentioned before.
> 
> 
> -Juergen
> 
> 
> 
> 
> On Nov 17, 2013, at 5:09 AM, Hal Finkel < hfinkel at anl.gov > wrote:
> 
> 
> 
> ----- Original Message -----
> 
> 
> From: "Juergen Ributzka" < juergen at apple.com >
> To: "Hal Finkel" < hfinkel at anl.gov >
> Cc: "LLVM Commits" < llvm-commits at cs.uiuc.edu >
> Sent: Sunday, November 17, 2013 1:02:37 AM
> Subject: Re: [PATCH][DAG] Fix non-deterministic code generation
> 
> Hi Hal,
> 
> the availability queue in the list scheduler returns the nodes in IR
> order. If there is more than one node ready to schedule, then the
> difference in the IR order will produce different schedules.
> 
> Maybe I'm missing something, but that does not seem to explain the
> problem. The relative order of the nodes should be the same whether
> the first node starts with 0 or 29348.
> 
> It seems like we should do this anyway (or make the counter 64 bits).
> For one thing, on really large inputs, the current 32-bit counter
> could wrap (which would certainly produce this problem). But you're
> seeing some other issue here.
> 
> -Hal
> 
> 
> 
> 
> -Juergen
> 
> On Nov 16, 2013, at 10:40 PM, Hal Finkel < hfinkel at anl.gov > wrote:
> 
> 
> 
> Juergen,
> 
> Do you happen to know what in CodeGen is sensitive to the absolute
> value of the node order?
> 
> -Hal
> 
> ----- Original Message -----
> 
> 
> From: "Juergen Ributzka" < juergen at apple.com >
> To: "LLVM Commits" < llvm-commits at cs.uiuc.edu >
> Sent: Saturday, November 16, 2013 11:04:29 PM
> Subject: [PATCH][DAG] Fix non-deterministic code generation
> 
> 
> 
> Hi @ll,
> 
> this patch resets SDNodeOrder in the SelectionDAGBuilder before
> processing the IR of a new function. This is required to obtain
> deterministic code generation for a function regardless of its
> location in the source file.
> 
> Cheers,
> Juergen
> 
> 
> 
> 
> 
> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 
> 
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
> 
> 
> 
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
> 
> 
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory