https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104335

--- Comment #5 from Segher Boessenkool <segher at gcc dot gnu.org> ---
(In reply to rdapp from comment #4)
> originally ifcvt would only pass e.g.
> 
> (unle (reg:SF 129 [ _29 ])
>     (reg/v:SF 118 [ highScore ]))
> 
> as condition to rs6000_emit_cmove via emit_conditional_move ().  (This is
> the example from the ICE).

And that works fine, right?

>   dest = (reg/f:DI 122 [ bestFuzz$__object ])
> 
> The check 
> 
>   if (FLOAT_MODE_P (compare_mode) && !FLOAT_MODE_P (result_mode))

With compare_mode SFmode, and result_mode, what, SImode?  Or VOIDmode?
In either case this would be true.

> fails and we return false

Which does not match my argument assumptions then, so they must be wrong.

> i.e. the expander fails and returns NULL_RTX. This
> is fine as we will just not do anything with it in ifcvt.

But it was fine by accident, then :-(

> Now with the patch rs6000_emit_cmove is passed
> 
>   (unle (reg:CCFP 153)
>       (const_int 0 [0])).
>
> This CC comparison has already been computed before so the backend actually
> needs to do less work and could just use it without preparing a comparison.
> This helps costing on the ifcvt side.

But this is done during expand only, costing is pretty irrelevant there.
expand should emit correct code, and later passes optimise that.  Many of our
oldest problems are expand trying to "optimise" things :-(

Or is this function called later as well?

> dest is the same as before

Which is?

> but now we have
> 
>   FLOAT_MODE_P (compare_mode) == false

That is wrong then.

> and
> 
>   if (FLOAT_MODE_P (compare_mode) && !FLOAT_MODE_P (result_mode))
> 
> not causing a return false.

So you meant that FLOAT_MODE (compare_mode) is true?

> We then call rs6000_emit_int_cmove which in turn calls
> rs6000_generate_compare.
> There we try to
> 
>   emit_insn (gen_rtx_SET (compare_result,                                   
> 
>                           gen_rtx_COMPARE (comp_mode, op0, op1)));
> 
> but ICE since the modes don't match or make sense.

Yes, rs6000_generate_compare does not expect the funny RTL for a condition
code as input: it wants a real comparison, instead, so not a representation
of the result of such a comparison.

> From an ifcvt point of view we would just expect a NULL_RTX if something
> cannot be handled.  We did not pass CC compares to backends before, so I
> don't know how reasonable this assumption is :)

Hard to say, I don't find documentation for this.  But, such a change is not
in scope for stage 4, no matter how you look at it :-(

Reply via email to