Hi Carl,
On Wed, Sep 13, 2017 at 04:29:01PM -0700, Carl Love wrote:
> -- add "TARGET_SF_FPR && TARGET_FPRND" to the define_insn "lrintsfsi2"
> as mentioned it was missing on the original define_insn for fctiw.
I don't think TARGET_FPRND is correct: this instruction is in the original
PowerPC specification already. As noted, this is wrong in more patterns,
we can deal with all of them at once later.
> I looked and really couldn't find a macro that really fit for these
> builtins. I added a new macro for miscellaneous pre ISA 2.04 builtins.
> I called it BU_P1_MISC_1 for lack of a better name. Open to suggestions
> on a better name. I really don't know what the ISA numbers are prior to
> 2.04 so called it P1 to catch all builtins prior to isa 2.04.
P7 stands for "POWER7" here, not some ISA version (and POWER1 would be
incorrect, fctiw was introduced on POWER2, and fctid on PowerPC).
BU_FP_1 maybe?
> +#define BU_P1_MISC_1(ENUM, NAME, ATTR, ICODE)
> \
> + RS6000_BUILTIN_1 (MISC_BUILTIN_ ## ENUM, /* ENUM */ \
> + "__builtin_" NAME, /* NAME */ \
> + RS6000_BTM_ALWAYS, /* MASK */ \
> + (RS6000_BTC_ ## ATTR /* ATTR */ \
> + | RS6000_BTC_UNARY), \
> + CODE_FOR_ ## ICODE) /* ICODE */
I wonder if this needs some test for hard float (try to build a testcase
with -msoft-float, what happens? Pretty much anything that isn't an ICE
is fine).
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/builtin-fctid-fctiw-runnable.c
> @@ -0,0 +1,138 @@
> +/* { dg-do run { target { powerpc*-*-* && { lp64 && p8vector_hw } } } } */
> +/* { dg-require-effective-target powerpc_p8vector_ok } */
> +/* { dg-options "-mcpu=power8" } */
Do you still need the p8vector_ok test if you already have p8vector_hw?
Would this test work on 32-bit if you used "long long" instead of "long"?
The patch looks good with those last few details taking care of :-)
Segher
