[PATCH, rs6000] Use direct moves for __builtin_signbit for 128-bit floating-point

[Segher Boessenkool]

Hi,

On Tue, Jun 28, 2016 at 04:44:08PM -0500, Bill Schmidt wrote:
> +void
> +rs6000_split_signbit (rtx dest, rtx src)
> +{
> +  machine_mode d_mode = GET_MODE (dest);
> +  machine_mode s_mode = GET_MODE (src);
> +  rtx dest_di = (d_mode == DImode) ? dest : gen_lowpart (DImode, dest);
> +  rtx shift_reg = dest_di;
> +
> +  gcc_assert (REG_P (dest));
> +  gcc_assert (REG_P (src) || MEM_P (src));
> +
> +  if (MEM_P (src))
> +    {
> +      rtx mem;
> +
> +      if (s_mode == SFmode)
> +	mem = gen_rtx_SIGN_EXTEND (DImode, adjust_address (src, SImode, 0));
> +
> +      else if (GET_MODE_SIZE (s_mode) == 16 && !WORDS_BIG_ENDIAN)
> +	mem = adjust_address (src, DImode, 8);
> +
> +      else
> +	mem = adjust_address (src, DImode, 0);

else if (s_mode == DFmode)
  ...
else
  gcc_unreachable ();

Or does this case catch some other modes, too?  Make those explicit, then?
Or make everything based on size (not mode).

And put it in order of size if you make that change?

> +  if (FLOAT128_IEEE_P (<MODE>mode))
> +    {
> +      if (<MODE>mode == KFmode)
> +	{
> +	  emit_insn (gen_signbitkf2_dm (operands[0], operands[1]));
> +	  DONE;
> +	}
> +      else if (<MODE>mode == TFmode)
> +	{
> +	  emit_insn (gen_signbittf2_dm (operands[0], operands[1]));
> +	  DONE;
> +	}
> +      else
> +	gcc_unreachable ();
> +    }

If you put a single DONE at the end here things will look simpler.

Why does the similar thing in rs6000.c construct the RTL by hand?  This
way is safer.

> --- gcc/testsuite/gcc.target/powerpc/signbit-1.c	(revision 0)
> +++ gcc/testsuite/gcc.target/powerpc/signbit-1.c	(working copy)
> @@ -0,0 +1,16 @@
> +/* { dg-do compile { target { powerpc*-*-* && lp64 } } } */
> +/* { dg-skip-if "" { powerpc*-*-darwin* } { "*" } { "" } } */

Default args aren't necessary.

> --- gcc/testsuite/gcc.target/powerpc/signbit-3.c	(revision 0)
> +++ gcc/testsuite/gcc.target/powerpc/signbit-3.c	(working copy)
> @@ -0,0 +1,172 @@
> +/* { dg-do run { target { powerpc*-*-linux* } } } */
> +/* { dg-require-effective-target ppc_float128_sw } */
> +/* { dg-options "-mcpu=power7 -O2 -mfloat128 -static-libgcc -lm" } */

Why does this need -static-libgcc?


Segher