[PATCH] Fix transitivity of sort in DAGISelEmitter.cpp

[Rafael Espíndola]

The fix LGTM.



On 30 September 2013 09:53, Richard Sandiford
<rsandifo at linux.vnet.ibm.com> wrote:
> DAGISelEmitter.cpp sorts patterns using:
>
>   bool operator()(const PatternToMatch *LHS, const PatternToMatch *RHS) {
>     const TreePatternNode *LHSSrc = LHS->getSrcPattern();
>     const TreePatternNode *RHSSrc = RHS->getSrcPattern();
>
>     if (LHSSrc->getNumTypes() != 0 && RHSSrc->getNumTypes() != 0 &&
>         LHSSrc->getType(0) != RHSSrc->getType(0)) {
>       MVT::SimpleValueType V1 = LHSSrc->getType(0), V2 = RHSSrc->getType(0);
>       if (MVT(V1).isVector() != MVT(V2).isVector())
>         return MVT(V2).isVector();
>
>       if (MVT(V1).isFloatingPoint() != MVT(V2).isFloatingPoint())
>         return MVT(V2).isFloatingPoint();
>     }
>
>     ...
>   }
>
> But ignoring the types of LHS when RHS has no types (or vice versa)
> can break transitivity.  E.g. if X and RHS have types, but LHS doesn't,
> we might have X < LHS, LHS < RHS (both skipping the type test above)
> and RHS < X (using the type test above).
>
> One fix would be to sort on "has types" first.  Another would be to use
> something like MVT::Other for patterns that have no types.  The latter seems
> a little simpler, so that's what the first patch below does.
>
> I checked this using the second patch, which I wasn't intending to commit,
> but can if it seems useful.  The abort triggers for several ports on my box
> as things stand, but doesn't trigger with the first patch applied.
>
> I'm hoping this is the cause of the host-dependent failures seen with r191611,
> since the outcome of the current sort will depend on the qsort implementation.
>
> OK to apply?
>
> Thanks,
> Richard
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>