Message ID | ba1a2a6a-fe6a-4ca7-43a3-403fe9326d12@linux.ibm.com |
---|---|
State | New |
Headers | show |
Series | [rs6000] Implement mffscrni pattern | expand |
On Thu, Dec 16, 2021 at 9:43 PM HAO CHEN GUI <guihaoc@linux.ibm.com> wrote: > > Hi, > This patch defines a pattern for mffscrni. If the RN is a constant, it can call > gen_rs6000_mffscrni directly. The "rs6000-builtin-new.def" defines prototype for builtin arguments. > The pattern "rs6000_set_fpscr_rn" is then broken as the mode of its argument is DI while its > corresponding builtin has a const int argument. The patch also fixed it. > > Bootstrapped and tested on powerpc64-linux BE and LE with no regressions. Is this okay for trunk? > Any recommendations? Thanks a lot. Hi, Haochen I have a question about the mode of the input operand in the new pattern below. > > ChangeLog > 2021-12-17 Haochen Gui <guihaoc@linux.ibm.com> > > gcc/ > * config/rs6000/predicates.md (u2bit_cint_operand): Defined. > * config/rs6000/rs6000-call.c > (rs6000_expand_set_fpscr_rn_builtin): Not copy argument to a reg if > it's a constant. The pattern for constant can be recognized now. > * config/rs6000/rs6000.md (UNSPECV_MFFSCRNI): Defined. > (rs6000_mffscrni): Defined. > (rs6000_set_fpscr_rn): Change the type of operand[0] form DI to SI. > Call gen_rs6000_mffscrni when operand[0] is a const int[0,3]. > > gcc/testsuite/ > * gcc.target/powerpc/mffscrni_p9.c: New testcase for mffscrni. > * gcc.target/powerpc/test_fpscr_rn_builtin.c: Modify the test cases to > test mffscrn and mffscrni separately. > > patch.diff > diff --git a/gcc/config/rs6000/predicates.md b/gcc/config/rs6000/predicates.md > index f216ffd..b10b4ce 100644 > --- a/gcc/config/rs6000/predicates.md > +++ b/gcc/config/rs6000/predicates.md > @@ -219,6 +219,11 @@ (define_predicate "u1bit_cint_operand" > (and (match_code "const_int") > (match_test "INTVAL (op) >= 0 && INTVAL (op) <= 1"))) > > +;; Return 1 if op is an unsigned 2-bit constant integer. > +(define_predicate "u2bit_cint_operand" > + (and (match_code "const_int") > + (match_test "INTVAL (op) >= 0 && INTVAL (op) <= 3"))) > + > ;; Return 1 if op is a unsigned 3-bit constant integer. > (define_predicate "u3bit_cint_operand" > (and (match_code "const_int") > diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c > index d9736ea..81261a0 100644 > --- a/gcc/config/rs6000/rs6000-call.c > +++ b/gcc/config/rs6000/rs6000-call.c > @@ -9610,13 +9610,15 @@ rs6000_expand_set_fpscr_rn_builtin (enum insn_code icode, tree exp) > 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 (CONST_INT_P (op0) && !const_0_to_3_operand(op0, VOIDmode)) > + if (CONST_INT_P (op0)) > { > - error ("Argument must be a value between 0 and 3."); > - return const0_rtx; > + if (!const_0_to_3_operand (op0, VOIDmode)) > + { > + error ("Argument must be a value between 0 and 3."); > + return const0_rtx; > + } > } > - > - if (! (*insn_data[icode].operand[0].predicate) (op0, mode0)) > + else if (! (*insn_data[icode].operand[0].predicate) (op0, mode0)) > op0 = copy_to_mode_reg (mode0, op0); > > pat = GEN_FCN (icode) (op0); > diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md > index 6bec2bd..291396c 100644 > --- a/gcc/config/rs6000/rs6000.md > +++ b/gcc/config/rs6000/rs6000.md > @@ -177,6 +177,7 @@ (define_c_enum "unspecv" > UNSPECV_MFFS ; Move from FPSCR > UNSPECV_MFFSL ; Move from FPSCR light instruction version > UNSPECV_MFFSCRN ; Move from FPSCR float rounding mode > + UNSPECV_MFFSCRNI ; Move from FPSCR float rounding mode with imm > UNSPECV_MFFSCDRN ; Move from FPSCR decimal float rounding mode > UNSPECV_MTFSF ; Move to FPSCR Fields 8 to 15 > UNSPECV_MTFSF_HI ; Move to FPSCR Fields 0 to 7 > @@ -6315,6 +6316,14 @@ (define_insn "rs6000_mffscrn" > "mffscrn %0,%1" > [(set_attr "type" "fp")]) > > +(define_insn "rs6000_mffscrni" > + [(set (match_operand:DF 0 "gpc_reg_operand" "=d") > + (unspec_volatile:DF [(match_operand:DF 1 "u2bit_cint_operand" "n")] Why is this input operand 1 DFmode? This is a 2 bit integer value. This pattern is called from rs6000_set_fpscr_rn with an SImode operand, and it seems that this should be SImode as well. Thanks, David > + UNSPECV_MFFSCRNI))] > + "TARGET_P9_MISC" > + "mffscrni %0,%1" > + [(set_attr "type" "fp")]) > + > (define_insn "rs6000_mffscdrn" > [(set (match_operand:DF 0 "gpc_reg_operand" "=d") > (unspec_volatile:DF [(const_int 0)] UNSPECV_MFFSCDRN)) > @@ -6324,7 +6333,7 @@ (define_insn "rs6000_mffscdrn" > [(set_attr "type" "fp")]) > > (define_expand "rs6000_set_fpscr_rn" > - [(match_operand:DI 0 "reg_or_cint_operand")] > + [(match_operand:SI 0 "reg_or_cint_operand")] > "TARGET_HARD_FLOAT" > { > rtx tmp_df = gen_reg_rtx (DFmode); > @@ -6333,9 +6342,15 @@ (define_expand "rs6000_set_fpscr_rn" > new rounding mode bits from operands[0][62:63] into FPSCR[62:63]. */ > if (TARGET_P9_MISC) > { > - rtx src_df = force_reg (DImode, operands[0]); > - src_df = simplify_gen_subreg (DFmode, src_df, DImode, 0); > - emit_insn (gen_rs6000_mffscrn (tmp_df, src_df)); > + if (CONST_INT_P (operands[0]) > + && const_0_to_3_operand (operands[0], VOIDmode)) > + emit_insn (gen_rs6000_mffscrni (tmp_df, operands[0])); > + else > + { > + rtx op0 = convert_to_mode (DImode, operands[0], false); > + rtx src_df = simplify_gen_subreg (DFmode, op0, DImode, 0); > + emit_insn (gen_rs6000_mffscrn (tmp_df, src_df)); > + } > DONE; > } > > @@ -6357,7 +6372,8 @@ (define_expand "rs6000_set_fpscr_rn" > rtx tmp_di = gen_reg_rtx (DImode); > > /* Extract new RN mode from operand. */ > - emit_insn (gen_anddi3 (tmp_rn, operands[0], GEN_INT (0x3))); > + rtx op0 = convert_to_mode (DImode, operands[0], false); > + emit_insn (gen_anddi3 (tmp_rn, op0, GEN_INT (0x3))); > > /* Insert new RN mode into FSCPR. */ > emit_insn (gen_rs6000_mffs (tmp_df)); > diff --git a/gcc/testsuite/gcc.target/powerpc/mffscrni_p9.c b/gcc/testsuite/gcc.target/powerpc/mffscrni_p9.c > new file mode 100644 > index 0000000..989a5bc > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/mffscrni_p9.c > @@ -0,0 +1,9 @@ > +/* { dg-do compile { target { has_arch_pwr9 } } } */ > +/* { dg-options "-O2" } */ > +/* { dg-final { scan-assembler-times {\mmffscrni\M} 1 } } */ > + > +void foo () > +{ > + int val = 2; > + __builtin_set_fpscr_rn (val); > +} > diff --git a/gcc/testsuite/gcc.target/powerpc/test_fpscr_rn_builtin.c b/gcc/testsuite/gcc.target/powerpc/test_fpscr_rn_builtin.c > index 0d0d3f0..594b458 100644 > --- a/gcc/testsuite/gcc.target/powerpc/test_fpscr_rn_builtin.c > +++ b/gcc/testsuite/gcc.target/powerpc/test_fpscr_rn_builtin.c > @@ -8,6 +8,10 @@ > #define RN_MASK 0x3LL /* RN field mask */ > > void abort (void); > +void __attribute__ ((noinline)) wrap_set_fpscr_rn (int val) > +{ > + __builtin_set_fpscr_rn (val); > +} > > int main () > { > @@ -43,7 +47,8 @@ int main () > } > > /* Test float rounding mode builtin with const value argument. */ > - __builtin_set_fpscr_rn(3); > + val = 3; > + __builtin_set_fpscr_rn (val); > conv_val.d = __builtin_mffs(); > ll_value = conv_val.ll & RN_MASK; > > @@ -58,7 +63,7 @@ int main () > } > > val = 2; > - __builtin_set_fpscr_rn(val); > + __builtin_set_fpscr_rn (val); > conv_val.d = __builtin_mffs(); > ll_value = conv_val.ll & RN_MASK; > > @@ -74,7 +79,7 @@ int main () > > /* Reset to 0 for testing */ > val = 0; > - __builtin_set_fpscr_rn(val); > + __builtin_set_fpscr_rn (val); > > __builtin_mtfsb1(31); > conv_val.d = __builtin_mffs(); > @@ -157,7 +162,7 @@ int main () > > /* Test builtin float rounding mode with variable as argument. */ > val = 0; > - __builtin_set_fpscr_rn(val); > + wrap_set_fpscr_rn (val); > conv_val.d = __builtin_mffs(); > ll_value = conv_val.ll & RN_MASK; > > @@ -172,7 +177,7 @@ int main () > } > > val = 3; > - __builtin_set_fpscr_rn(val); > + wrap_set_fpscr_rn (val); > conv_val.d = __builtin_mffs(); > ll_value = conv_val.ll & RN_MASK; > >
On Fri, Dec 17, 2021 at 10:33:12PM -0500, David Edelsohn wrote: > On Thu, Dec 16, 2021 at 9:43 PM HAO CHEN GUI <guihaoc@linux.ibm.com> wrote: > > +(define_insn "rs6000_mffscrni" > > + [(set (match_operand:DF 0 "gpc_reg_operand" "=d") > > + (unspec_volatile:DF [(match_operand:DF 1 "u2bit_cint_operand" "n")] > > Why is this input operand 1 DFmode? This is a 2 bit integer value. > This pattern is called from rs6000_set_fpscr_rn with an SImode > operand, and it seems that this should be SImode as well. Yup. This predicate will only allow const_int, and matching a const_int ignores the mode, so most (or all?) things will work whatever mode you give here. But only MODE_INT modes make sense (and possibly VOIDmode, but VOIDmode in machine descriptions means anything is allowed, also not what you want here). Segher
diff --git a/gcc/config/rs6000/predicates.md b/gcc/config/rs6000/predicates.md index f216ffd..b10b4ce 100644 --- a/gcc/config/rs6000/predicates.md +++ b/gcc/config/rs6000/predicates.md @@ -219,6 +219,11 @@ (define_predicate "u1bit_cint_operand" (and (match_code "const_int") (match_test "INTVAL (op) >= 0 && INTVAL (op) <= 1"))) +;; Return 1 if op is an unsigned 2-bit constant integer. +(define_predicate "u2bit_cint_operand" + (and (match_code "const_int") + (match_test "INTVAL (op) >= 0 && INTVAL (op) <= 3"))) + ;; Return 1 if op is a unsigned 3-bit constant integer. (define_predicate "u3bit_cint_operand" (and (match_code "const_int") diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c index d9736ea..81261a0 100644 --- a/gcc/config/rs6000/rs6000-call.c +++ b/gcc/config/rs6000/rs6000-call.c @@ -9610,13 +9610,15 @@ rs6000_expand_set_fpscr_rn_builtin (enum insn_code icode, tree exp) 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 (CONST_INT_P (op0) && !const_0_to_3_operand(op0, VOIDmode)) + if (CONST_INT_P (op0)) { - error ("Argument must be a value between 0 and 3."); - return const0_rtx; + if (!const_0_to_3_operand (op0, VOIDmode)) + { + error ("Argument must be a value between 0 and 3."); + return const0_rtx; + } } - - if (! (*insn_data[icode].operand[0].predicate) (op0, mode0)) + else if (! (*insn_data[icode].operand[0].predicate) (op0, mode0)) op0 = copy_to_mode_reg (mode0, op0); pat = GEN_FCN (icode) (op0); diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md index 6bec2bd..291396c 100644 --- a/gcc/config/rs6000/rs6000.md +++ b/gcc/config/rs6000/rs6000.md @@ -177,6 +177,7 @@ (define_c_enum "unspecv" UNSPECV_MFFS ; Move from FPSCR UNSPECV_MFFSL ; Move from FPSCR light instruction version UNSPECV_MFFSCRN ; Move from FPSCR float rounding mode + UNSPECV_MFFSCRNI ; Move from FPSCR float rounding mode with imm UNSPECV_MFFSCDRN ; Move from FPSCR decimal float rounding mode UNSPECV_MTFSF ; Move to FPSCR Fields 8 to 15 UNSPECV_MTFSF_HI ; Move to FPSCR Fields 0 to 7 @@ -6315,6 +6316,14 @@ (define_insn "rs6000_mffscrn" "mffscrn %0,%1" [(set_attr "type" "fp")]) +(define_insn "rs6000_mffscrni" + [(set (match_operand:DF 0 "gpc_reg_operand" "=d") + (unspec_volatile:DF [(match_operand:DF 1 "u2bit_cint_operand" "n")] + UNSPECV_MFFSCRNI))] + "TARGET_P9_MISC" + "mffscrni %0,%1" + [(set_attr "type" "fp")]) + (define_insn "rs6000_mffscdrn" [(set (match_operand:DF 0 "gpc_reg_operand" "=d") (unspec_volatile:DF [(const_int 0)] UNSPECV_MFFSCDRN)) @@ -6324,7 +6333,7 @@ (define_insn "rs6000_mffscdrn" [(set_attr "type" "fp")]) (define_expand "rs6000_set_fpscr_rn" - [(match_operand:DI 0 "reg_or_cint_operand")] + [(match_operand:SI 0 "reg_or_cint_operand")] "TARGET_HARD_FLOAT" { rtx tmp_df = gen_reg_rtx (DFmode); @@ -6333,9 +6342,15 @@ (define_expand "rs6000_set_fpscr_rn" new rounding mode bits from operands[0][62:63] into FPSCR[62:63]. */ if (TARGET_P9_MISC) { - rtx src_df = force_reg (DImode, operands[0]); - src_df = simplify_gen_subreg (DFmode, src_df, DImode, 0); - emit_insn (gen_rs6000_mffscrn (tmp_df, src_df)); + if (CONST_INT_P (operands[0]) + && const_0_to_3_operand (operands[0], VOIDmode)) + emit_insn (gen_rs6000_mffscrni (tmp_df, operands[0])); + else + { + rtx op0 = convert_to_mode (DImode, operands[0], false); + rtx src_df = simplify_gen_subreg (DFmode, op0, DImode, 0); + emit_insn (gen_rs6000_mffscrn (tmp_df, src_df)); + } DONE; } @@ -6357,7 +6372,8 @@ (define_expand "rs6000_set_fpscr_rn" rtx tmp_di = gen_reg_rtx (DImode); /* Extract new RN mode from operand. */ - emit_insn (gen_anddi3 (tmp_rn, operands[0], GEN_INT (0x3))); + rtx op0 = convert_to_mode (DImode, operands[0], false); + emit_insn (gen_anddi3 (tmp_rn, op0, GEN_INT (0x3))); /* Insert new RN mode into FSCPR. */ emit_insn (gen_rs6000_mffs (tmp_df)); diff --git a/gcc/testsuite/gcc.target/powerpc/mffscrni_p9.c b/gcc/testsuite/gcc.target/powerpc/mffscrni_p9.c new file mode 100644 index 0000000..989a5bc --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/mffscrni_p9.c @@ -0,0 +1,9 @@ +/* { dg-do compile { target { has_arch_pwr9 } } } */ +/* { dg-options "-O2" } */ +/* { dg-final { scan-assembler-times {\mmffscrni\M} 1 } } */ + +void foo () +{ + int val = 2; + __builtin_set_fpscr_rn (val); +} diff --git a/gcc/testsuite/gcc.target/powerpc/test_fpscr_rn_builtin.c b/gcc/testsuite/gcc.target/powerpc/test_fpscr_rn_builtin.c index 0d0d3f0..594b458 100644 --- a/gcc/testsuite/gcc.target/powerpc/test_fpscr_rn_builtin.c +++ b/gcc/testsuite/gcc.target/powerpc/test_fpscr_rn_builtin.c @@ -8,6 +8,10 @@ #define RN_MASK 0x3LL /* RN field mask */ void abort (void); +void __attribute__ ((noinline)) wrap_set_fpscr_rn (int val) +{ + __builtin_set_fpscr_rn (val); +} int main () { @@ -43,7 +47,8 @@ int main () } /* Test float rounding mode builtin with const value argument. */ - __builtin_set_fpscr_rn(3); + val = 3; + __builtin_set_fpscr_rn (val); conv_val.d = __builtin_mffs(); ll_value = conv_val.ll & RN_MASK; @@ -58,7 +63,7 @@ int main () } val = 2; - __builtin_set_fpscr_rn(val); + __builtin_set_fpscr_rn (val); conv_val.d = __builtin_mffs(); ll_value = conv_val.ll & RN_MASK; @@ -74,7 +79,7 @@ int main () /* Reset to 0 for testing */ val = 0; - __builtin_set_fpscr_rn(val); + __builtin_set_fpscr_rn (val); __builtin_mtfsb1(31); conv_val.d = __builtin_mffs(); @@ -157,7 +162,7 @@ int main () /* Test builtin float rounding mode with variable as argument. */ val = 0; - __builtin_set_fpscr_rn(val); + wrap_set_fpscr_rn (val); conv_val.d = __builtin_mffs(); ll_value = conv_val.ll & RN_MASK; @@ -172,7 +177,7 @@ int main () } val = 3; - __builtin_set_fpscr_rn(val); + wrap_set_fpscr_rn (val); conv_val.d = __builtin_mffs(); ll_value = conv_val.ll & RN_MASK;