> It might make sense to move:
> 
>    /* Don't even try if the comparison operands are weird
>       except that the target supports cbranchcc4.  */
>    if (! general_operand (cmp_a, GET_MODE (cmp_a))
>        || ! general_operand (cmp_b, GET_MODE (cmp_b)))
>      {
>        if (!have_cbranchcc4
> 	  || GET_MODE_CLASS (GET_MODE (cmp_a)) != MODE_CC
> 	  || cmp_b != const0_rtx)
> 	return NULL_RTX;
>      }
> 
> into the “else” arm, since it seems odd to be checking cmp_a and cmp_b
> when we're not going to use them.  Looks like the later call to
> emit_conditional_move should get the same treatment.

Moved it.

> Why's that the case though?  The swapped form is the canonical one,
> so it's the one that the target ought to accept.

My wording was a bit misleading in the comment and I tried to improve it 
in the attached v2.

Before, we had a loop with two iterations that tried "emit_cmov (cond, 
op2, op3)".  op2 and op3 can (or even were) If this did not succeed we 
would revert the condition as well as op3 and op3 in-place and try 
again.  I found that a bit cumbersome and intended to make this explicit 
but it's still kind of involved, particularly since cond may come in 
reversed, we additionally swap op2 and op3 and all the way back again.

I remember from the first iteration of these patches that this (double 
revert) was needed for exactly one test case in the test suite on x86. 
When re-running it today I could not reproduce it anymore, though.

Regards
  Robin