On 1/20/20 6:51 PM, Segher Boessenkool wrote:
On Tue, Jan 21, 2020 at 12:23:02AM +0100, Jakub Jelinek wrote:
On Mon, Jan 20, 2020 at 05:10:55PM -0600, Segher Boessenkool wrote:
On Mon, Jan 20, 2020 at 11:52:55PM +0100, Jakub Jelinek wrote:
PR target/93073
* config/rs6000/rs6000.c (rs6000_emit_cmove): Punt for compare_mode
other than SFmode or DFmode.
"If using fsel, punt for..." etc.
Ack.
+ /* Don't allow compare_mode other than SFmode or DFmode, for others there
+ is no fsel instruction. */
+ if (compare_mode != SFmode && compare_mode != DFmode)
+ return 0;
And move this a bit later, to right after
/* At this point we know we can use fsel. */
please?
I can move it if you want (not sure what is the gain, because the mode
check is cheaper than some of the other checks in there), but don't
understand why to after that comment rather than say immediately before that
comment. Because with TFmode or KFmode we know we can't use fsel, and
the comment seems to mark the point at which we have decided we can use the
instruction and just prepare arguments for it.
We can (and should) use other instructions than just fsel here as well
(say, xscmpgedp followed by xxsel). This can also work for QP float,
which also can be TFmode, just to complicate matters more.
This function is a bit big, and the logic is all over the place, but
let's try to make it better, little by little :-)
Segher
Segher,
If your goal is make it smaller I just looked at it but wasn't sure due
to this line:
enum rtx_code code = GET_CODE (op);
in rs6000_emit_cmove and how to move it out but without duplicating it.
However the switch case for GE lowering seems to have two main cases
that can
be removed into helper functions. The cases for ORDERED,EQ should
be removed to another function and for UNGE,GT another function.
Not sure if that was your goal but it would make the function smaller and
perhaps less of a mess as at least a third of the function is probably
that switch statement :).
Nick
