Hi!
On Thu, Sep 27, 2018 at 04:17:57PM -0700, Carl Love wrote:
> + if (icode == CODE_FOR_rs6000_mffsl
> + && rs6000_isa_flags_explicit & OPTION_MASK_SOFT_FLOAT)
> + fatal_error (input_location,
> + "__builtin_mffsl() not supported with -msoft-float");
Please use plain "error ()" instead. To keep whatever else here from
wreaking havoc, also immediately after the error() do "return const0_rtx"?
(Same for all other fatal_error, of course. fatal_error is for when the
compiler needs to go down ungracefully, _now_. It is nicer to still try
to continue for a little while).
> + /* If the argument is a constant, check the range. Argument can only be a
> + 2-bit value. Unfortunately, can't check the range of the value at
> + compile time if the argument is a variable. The least significant two
> + bits of the argument, regardless of type, are used to set the rounding
> + mode. All other bits are ignored. */
> + if (GET_CODE (op0) == CONST_INT && !const_0_to_3_operand(op0, VOIDmode))
> + {
> + error ("Argument must be a value between 0 and 3.");
> + return const0_rtx;
> + }
These are indented a char too many.
> + if (TARGET_P9_MISC)
> + {
> + rtx src_df = gen_reg_rtx (DImode);
> +
> + src_df = simplify_gen_subreg (DFmode, operands[0], DImode, 0);
> + emit_insn (gen_rs6000_mffscrn (tmp_df, src_df));
> + }
> + else
This is easier if you write it like:
if (...)
{
emit this;
emit that;
DONE;
}
if (...)
{
emit this;
emit that;
DONE;
}
etc.
With that style, code that is semantically at the same level has the same
indent, instead of wandering further and further to the right.
> + {
> + rtx tmp_rn = gen_reg_rtx (DImode);
> + rtx tmp_di = gen_reg_rtx (DImode);
> +
> + /* Extract new RN mode from operand. */
> + emit_insn (gen_anddi3 (tmp_rn, operands[0], GEN_INT (0x3)));
> +
> + /* Insert new RN mode into FSCPR. */
> + emit_insn (gen_rs6000_mffs (tmp_df));
> + tmp_di = simplify_gen_subreg (DImode, tmp_df, DFmode, 0);
> + emit_insn (gen_anddi3 (tmp_di, tmp_di, GEN_INT (0xFFFFFFFC)));
This loses bits 0..31 (the top half of the register). Maybe use
GEN_INT (-4) ?
> + emit_insn (gen_iordi3 (tmp_di, tmp_di, tmp_rn));
> +
> + /* Need to write to field k=15. The fields are [0:15]. Hence with
> + L=0, W=0, FLM_i must be equal to 8, 16 = i + 8*(1-W). FLM is an
> + 8-bit field[0:7]. Need to set the bit that corresponds to the
> + value of i that you want [0:7]. */
> + tmp_df = simplify_gen_subreg (DFmode, tmp_di, DImode, 0);
> + emit_insn (gen_rs6000_mtfsf (GEN_INT (0x01), tmp_df));
> + }
:-)
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/test_fpscr_drn_builtin.c
> @@ -0,0 +1,116 @@
> +/* { dg-do run { target { powerpc*-*-* && lp64 } } } */
> +/* { dg-options "-std=c99" } */
You need to require a system that implements the DRN bits... I think
you'll need the "dfp_hw" selector. (That's power6 and later, may not
be so easy to test this ;-) )
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/test_mffsl.c
> @@ -0,0 +1,34 @@
> +/* { dg-do run { target { powerpc*-*-* } } } */
> +/* { dg-options "-std=c99" } */
Maybe you should do the run tests with -O2? Maybe compile tests, too,
come to think of it.
With those details fixed, okay for trunk. Thanks!
Segher
