Hi Segher,
> On Jun 29, 2016, at 4:43 PM, Segher Boessenkool <seg...@kernel.crashing.org>
> wrote:
>
> 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?
This actually has some leftover cruft in it from the original patch that
supported DF and SF modes. I'll simplify it considerably.
>
>> + 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.
Good points, will fix.
>
>> --- 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?
Mike, can you please respond to this? I was curious about this also
but neglected to ask you.
Thanks,
Bill
>
>
> Segher
>
