diff mbox series

[07/18] rs6000: Builtin expansion, part 2

Message ID 2d37e1858ac2d6e10a8f167d92fad83c31077af2.1630511335.git.wschmidt@linux.ibm.com
State New
Headers show
Series Replace the Power target-specific builtin machinery | expand

Commit Message

Bill Schmidt Sept. 1, 2021, 4:13 p.m. UTC 
Implement rs6000_invalid_new_builtin, which issues the appropriate error
message when a builtin is used when it is not enabled.  Also implement
rs6000_expand_ldst_mask, which just factors out the code that handles
ALTIVEC_BUILTIN_MASK_FOR_LOAD in the old rs6000_expand_builtin.  Finally,
ensure the variable altivec_builtin_mask_for_load is initialized.

2021-09-01  Bill Schmidt  <wschmidt@linux.ibm.com>

gcc/
	* config/rs6000/rs6000-call.c (rs6000_invalid_new_builtin):
	Implement.
	(rs6000_expand_ldst_mask): Likewise.
	(rs6000_init_builtins): Initialize altivec_builtin_mask_for_load.
---
 gcc/config/rs6000/rs6000-call.c | 101 +++++++++++++++++++++++++++++++-
 1 file changed, 100 insertions(+), 1 deletion(-)

Comments

Segher Boessenkool Nov. 1, 2021, 12:18 p.m. UTC | #1 
Hi!

On Wed, Sep 01, 2021 at 11:13:43AM -0500, Bill Schmidt wrote:
> 	* config/rs6000/rs6000-call.c (rs6000_invalid_new_builtin):
> 	Implement.

That fits on one line.  Don't wrap early, esp. not if that leaves a
colon without anything following it on that line: it looks like
something is missing.

> 	(rs6000_expand_ldst_mask): Likewise.
> 	(rs6000_init_builtins): Initialize altivec_builtin_mask_for_load.


>  static void
>  rs6000_invalid_new_builtin (enum rs6000_gen_builtins fncode)
>  {
> +  size_t uns_fncode = (size_t) fncode;

Like in the previous patch, the "uns_*" name made me think "you do not
need an explicit cast, the assignment will do that automatically".  But
of course it does not matter this is unsigned at all: the cast is
casting an enum to a number, which in C++ does require a cast.

So maybe you can think of some better name?  Something like "j" is fine
with me as well btw, it's nice and short, and it is clear you do not
want more meaning ;-)

> +  switch (rs6000_builtin_info_x[uns_fncode].enable)

> +    case ENB_P6:
> +      error ("%qs requires the %qs option", name, "-mcpu=power6");
> +      break;

> +    case ENB_CELL:
> +      error ("%qs is only valid for the cell processor", name);
> +      break;

Maybe  "%qs requires the %qs option", name, "-mcpu=cell"  ?  Boring is
good ;-)

> +    };

(This is  switch (...) { ... };  )
Stray semi.  Was there no warning?

>  rtx
>  rs6000_expand_ldst_mask (rtx target, tree arg0)
>   {
> +  int icode2 = BYTES_BIG_ENDIAN

You do not need a line break here.

> +    ? (int) CODE_FOR_altivec_lvsr_direct
> +    : (int) CODE_FOR_altivec_lvsl_direct;

You can align the ? and : just fine without it.

> +  rtx op, addr, pat;

Don't declare such things early.

Okay for trunk with those things fixed.  Thanks!


Segher
diff mbox series

Patch

diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c
index 583efc9e98e..3e0ab42317b 100644
--- a/gcc/config/rs6000/rs6000-call.c
+++ b/gcc/config/rs6000/rs6000-call.c
@@ -11671,6 +11671,75 @@  rs6000_invalid_builtin (enum rs6000_builtins fncode)
 static void
 rs6000_invalid_new_builtin (enum rs6000_gen_builtins fncode)
 {
+  size_t uns_fncode = (size_t) fncode;
+  const char *name = rs6000_builtin_info_x[uns_fncode].bifname;
+
+  switch (rs6000_builtin_info_x[uns_fncode].enable)
+    {
+    case ENB_P5:
+      error ("%qs requires the %qs option", name, "-mcpu=power5");
+      break;
+    case ENB_P6:
+      error ("%qs requires the %qs option", name, "-mcpu=power6");
+      break;
+    case ENB_ALTIVEC:
+      error ("%qs requires the %qs option", name, "-maltivec");
+      break;
+    case ENB_CELL:
+      error ("%qs is only valid for the cell processor", name);
+      break;
+    case ENB_VSX:
+      error ("%qs requires the %qs option", name, "-mvsx");
+      break;
+    case ENB_P7:
+      error ("%qs requires the %qs option", name, "-mcpu=power7");
+      break;
+    case ENB_P7_64:
+      error ("%qs requires the %qs option and either the %qs or %qs option",
+	     name, "-mcpu=power7", "-m64", "-mpowerpc64");
+      break;
+    case ENB_P8:
+      error ("%qs requires the %qs option", name, "-mcpu=power8");
+      break;
+    case ENB_P8V:
+      error ("%qs requires the %qs option", name, "-mpower8-vector");
+      break;
+    case ENB_P9:
+      error ("%qs requires the %qs option", name, "-mcpu=power9");
+      break;
+    case ENB_P9_64:
+      error ("%qs requires the %qs option and either the %qs or %qs option",
+	     name, "-mcpu=power9", "-m64", "-mpowerpc64");
+      break;
+    case ENB_P9V:
+      error ("%qs requires the %qs option", name, "-mpower9-vector");
+      break;
+    case ENB_IEEE128_HW:
+      error ("%qs requires ISA 3.0 IEEE 128-bit floating point", name);
+      break;
+    case ENB_DFP:
+      error ("%qs requires the %qs option", name, "-mhard-dfp");
+      break;
+    case ENB_CRYPTO:
+      error ("%qs requires the %qs option", name, "-mcrypto");
+      break;
+    case ENB_HTM:
+      error ("%qs requires the %qs option", name, "-mhtm");
+      break;
+    case ENB_P10:
+      error ("%qs requires the %qs option", name, "-mcpu=power10");
+      break;
+    case ENB_P10_64:
+      error ("%qs requires the %qs option and either the %qs or %qs option",
+	     name, "-mcpu=power10", "-m64", "-mpowerpc64");
+      break;
+    case ENB_MMA:
+      error ("%qs requires the %qs option", name, "-mmma");
+      break;
+    default:
+    case ENB_ALWAYS:
+      gcc_unreachable ();
+    };
 }
 
 /* Target hook for early folding of built-ins, shamelessly stolen
@@ -14542,7 +14611,34 @@  rs6000_expand_builtin (tree exp, rtx target, rtx subtarget ATTRIBUTE_UNUSED,
 rtx
 rs6000_expand_ldst_mask (rtx target, tree arg0)
  {
-  return target;
+  int icode2 = BYTES_BIG_ENDIAN
+    ? (int) CODE_FOR_altivec_lvsr_direct
+    : (int) CODE_FOR_altivec_lvsl_direct;
+  machine_mode tmode = insn_data[icode2].operand[0].mode;
+  machine_mode mode = insn_data[icode2].operand[1].mode;
+  rtx op, addr, pat;
+
+  gcc_assert (TARGET_ALTIVEC);
+
+  gcc_assert (POINTER_TYPE_P (TREE_TYPE (arg0)));
+  op = expand_expr (arg0, NULL_RTX, Pmode, EXPAND_NORMAL);
+  addr = memory_address (mode, op);
+  /* We need to negate the address.  */
+  op = gen_reg_rtx (GET_MODE (addr));
+  emit_insn (gen_rtx_SET (op, gen_rtx_NEG (GET_MODE (addr), addr)));
+  op = gen_rtx_MEM (mode, op);
+
+  if (target == 0
+      || GET_MODE (target) != tmode
+      || !insn_data[icode2].operand[0].predicate (target, tmode))
+    target = gen_reg_rtx (tmode);
+
+  pat = GEN_FCN (icode2) (target, op);
+  if (!pat)
+    return 0;
+  emit_insn (pat);
+
+   return target;
  }
 
 /* Expand the CPU builtin in FCODE and store the result in TARGET.  */
@@ -15351,6 +15447,9 @@  rs6000_init_builtins (void)
 
   if (new_builtins_are_live)
     {
+      altivec_builtin_mask_for_load
+	= rs6000_builtin_decls_x[RS6000_BIF_MASK_FOR_LOAD];
+
 #ifdef SUBTARGET_INIT_BUILTINS
       SUBTARGET_INIT_BUILTINS;
 #endif