Hmm, OK. Doesn't expanding both versions up-front create the same kind of problem that the patch is fixing, in that we expand (and therefore cost) both the reversed and unreversed comparison? Also…
[..]
…for min/max, I would have expected this swap to create the canonical operand order for the min and max too. What causes it to be rejected?
We should not be expanding two comparisons but only emit (and cost) the reversed comparison if expanding the non-reversed one failed.
Regarding the reversal, I checked again - the commit introducing the op2/op3 swap is g:deed3da9af697ecf073aea855ecce2d22d85ef71, the corresponding test case is gcc.target/i386/pr70465-2.c. It inlines one long double ternary operation into another, probably causing not for multiple sets, mind you. The situation doesn't occur with double.
+ + rtx rev_comparison = NULL_RTX; bool swapped = false; - if (swap_commutative_operands_p (op2, op3) - && ((reversed = reversed_comparison_code_parts (code, op0, op1, NULL)) - != UNKNOWN)) + + code = unsignedp ? unsigned_condition (code) : code; + comparison = simplify_gen_relational (code, VOIDmode, cmode, op0, op1); + + if ((reversed = reversed_comparison_code_parts (code, op0, op1, NULL)) + != UNKNOWN) { - std::swap (op2, op3); - code = reversed; - swapped = true; + reversed = unsignedp ? unsigned_condition (reversed) : reversed;When is this needed? I'd have expected the reversed from of an unsigned code to be naturally unsigned.
This was also introduced by the commit above, probably just repeating what was done for the non-reversed comparison.
Regards Robin
