[rs6000] Clean up implementation of built-in functions

Message ID 32d28e0f-894d-b6cc-9d02-b8ca45578196@linux.ibm.com
State New
Headers show
Series
  • [rs6000] Clean up implementation of built-in functions
Related show

Commit Message

Kelvin Nilsen June 1, 2018, 6:22 p.m.
This patch improves maintainability of the rs6000 built-in functions by adding a comment to describe the non-traditional implementation of the __builtin_vec_vsx_ld and __builtin_vec_vsx_st functions, and by removing eight redundant entries from the altivec_overloaded_builtins array.

Note, in the patch file, that the lines immediately preceding each of the deletions from altivec_overloaded_builtins exactly matches the deleted lines.  This redundancy may have been accidentally introduced by manual resolution of merge conflicts.  I did not investigate the origin of the redundancy.  The redundant entries cause trouble to tools that automate consistency checking between implementation and documentation of built-in functions.  Additionally, they are a likely cause of future bugs if any future efforts need to make corrections or changes to the associated functions.

This patch has bootstrapped and tested without regressions on powerpc64le-unknown-linux (P8).

Is this ok for trunk?


gcc/ChangeLog:

2018-06-01  Kelvin Nilsen  <kelvin@gcc.gnu.org>

	* config/rs6000/rs6000-builtin.def (VSX_BUILTIN_VEC_LD,
	VSX_BUILTIN_VEC_ST): Add comment to explain non-traditional uses.
	* config/rs6000/rs6000-c.c (altivec_overloaded_builtins): Remove
	several redundant entries.

Comments

Segher Boessenkool June 5, 2018, 6:52 p.m. | #1
Hi Kelvin,

On Fri, Jun 01, 2018 at 01:22:09PM -0500, Kelvin Nilsen wrote:
> 2018-06-01  Kelvin Nilsen  <kelvin@gcc.gnu.org>

> 

> 	* config/rs6000/rs6000-builtin.def (VSX_BUILTIN_VEC_LD,

> 	VSX_BUILTIN_VEC_ST): Add comment to explain non-traditional uses.

> 	* config/rs6000/rs6000-c.c (altivec_overloaded_builtins): Remove

> 	several redundant entries.


> --- gcc/config/rs6000/rs6000-builtin.def	(revision 261067)

> +++ gcc/config/rs6000/rs6000-builtin.def	(working copy)

> @@ -1811,6 +1811,15 @@ BU_VSX_OVERLOAD_1 (VUNSIGNEDE,  "vunsignede")

>  BU_VSX_OVERLOAD_1 (VUNSIGNEDO,  "vunsignedo")

>  

>  /* VSX builtins that are handled as special cases.  */

> +

> +

> +/* NON-TRADITIONAL BEHAVIOR HERE: Besides introducing the

> +   __builtin_vec_ld and __builtin_vec_st built-in functions,

> +   the VSX_BUILTIN_VEC_LD and VSX_BUILTIN_VEC_ST symbolic constants

> +   introduced below are also affiliated with the __builtin_vec_vsx_ld

> +   and __builtin_vec_vsx_st functions respectively.  This unnatural

> +   binding is formed with explicit calls to the def_builtin function

> +   found in rs6000.c.  */

>  BU_VSX_OVERLOAD_X (LD,	     "ld")

>  BU_VSX_OVERLOAD_X (ST,	     "st")

>  BU_VSX_OVERLOAD_X (XL,	     "xl")


It's not clear to me how this is different from all the other def_builtin
calls, say, __builtin_vec_adde or __builtin_vec_splats.

Either way, the patch is fine, okay for trunk.  Thanks.


Segher

Patch

Index: gcc/config/rs6000/rs6000-builtin.def
===================================================================
--- gcc/config/rs6000/rs6000-builtin.def	(revision 261067)
+++ gcc/config/rs6000/rs6000-builtin.def	(working copy)
@@ -1811,6 +1811,15 @@  BU_VSX_OVERLOAD_1 (VUNSIGNEDE,  "vunsignede")
 BU_VSX_OVERLOAD_1 (VUNSIGNEDO,  "vunsignedo")
 
 /* VSX builtins that are handled as special cases.  */
+
+
+/* NON-TRADITIONAL BEHAVIOR HERE: Besides introducing the
+   __builtin_vec_ld and __builtin_vec_st built-in functions,
+   the VSX_BUILTIN_VEC_LD and VSX_BUILTIN_VEC_ST symbolic constants
+   introduced below are also affiliated with the __builtin_vec_vsx_ld
+   and __builtin_vec_vsx_st functions respectively.  This unnatural
+   binding is formed with explicit calls to the def_builtin function
+   found in rs6000.c.  */
 BU_VSX_OVERLOAD_X (LD,	     "ld")
 BU_VSX_OVERLOAD_X (ST,	     "st")
 BU_VSX_OVERLOAD_X (XL,	     "xl")
Index: gcc/config/rs6000/rs6000-c.c
===================================================================
--- gcc/config/rs6000/rs6000-c.c	(revision 261067)
+++ gcc/config/rs6000/rs6000-c.c	(working copy)
@@ -1375,28 +1375,16 @@  const struct altivec_builtin_types altivec_overloa
     RS6000_BTI_bool_V4SI, RS6000_BTI_V4SF, RS6000_BTI_V4SF, 0 },
   { ALTIVEC_BUILTIN_VEC_VCMPGTSW, ALTIVEC_BUILTIN_VCMPGTSW,
     RS6000_BTI_bool_V4SI, RS6000_BTI_V4SI, RS6000_BTI_V4SI, 0 },
-  { ALTIVEC_BUILTIN_VEC_VCMPGTSW, ALTIVEC_BUILTIN_VCMPGTSW,
-    RS6000_BTI_bool_V4SI, RS6000_BTI_V4SI, RS6000_BTI_V4SI, 0 },
   { ALTIVEC_BUILTIN_VEC_VCMPGTUW, ALTIVEC_BUILTIN_VCMPGTUW,
     RS6000_BTI_bool_V4SI, RS6000_BTI_unsigned_V4SI, RS6000_BTI_unsigned_V4SI, 0 },
-  { ALTIVEC_BUILTIN_VEC_VCMPGTUW, ALTIVEC_BUILTIN_VCMPGTUW,
-    RS6000_BTI_bool_V4SI, RS6000_BTI_unsigned_V4SI, RS6000_BTI_unsigned_V4SI, 0 },
   { ALTIVEC_BUILTIN_VEC_VCMPGTSH, ALTIVEC_BUILTIN_VCMPGTSH,
     RS6000_BTI_bool_V8HI, RS6000_BTI_V8HI, RS6000_BTI_V8HI, 0 },
-  { ALTIVEC_BUILTIN_VEC_VCMPGTSH, ALTIVEC_BUILTIN_VCMPGTSH,
-    RS6000_BTI_bool_V8HI, RS6000_BTI_V8HI, RS6000_BTI_V8HI, 0 },
   { ALTIVEC_BUILTIN_VEC_VCMPGTUH, ALTIVEC_BUILTIN_VCMPGTUH,
     RS6000_BTI_bool_V8HI, RS6000_BTI_unsigned_V8HI, RS6000_BTI_unsigned_V8HI, 0 },
-  { ALTIVEC_BUILTIN_VEC_VCMPGTUH, ALTIVEC_BUILTIN_VCMPGTUH,
-    RS6000_BTI_bool_V8HI, RS6000_BTI_unsigned_V8HI, RS6000_BTI_unsigned_V8HI, 0 },
   { ALTIVEC_BUILTIN_VEC_VCMPGTSB, ALTIVEC_BUILTIN_VCMPGTSB,
     RS6000_BTI_bool_V16QI, RS6000_BTI_V16QI, RS6000_BTI_V16QI, 0 },
-  { ALTIVEC_BUILTIN_VEC_VCMPGTSB, ALTIVEC_BUILTIN_VCMPGTSB,
-    RS6000_BTI_bool_V16QI, RS6000_BTI_V16QI, RS6000_BTI_V16QI, 0 },
   { ALTIVEC_BUILTIN_VEC_VCMPGTUB, ALTIVEC_BUILTIN_VCMPGTUB,
     RS6000_BTI_bool_V16QI, RS6000_BTI_unsigned_V16QI, RS6000_BTI_unsigned_V16QI, 0 },
-  { ALTIVEC_BUILTIN_VEC_VCMPGTUB, ALTIVEC_BUILTIN_VCMPGTUB,
-    RS6000_BTI_bool_V16QI, RS6000_BTI_unsigned_V16QI, RS6000_BTI_unsigned_V16QI, 0 },
   { ALTIVEC_BUILTIN_VEC_CMPLE, ALTIVEC_BUILTIN_VCMPGEFP,
     RS6000_BTI_bool_V4SI, RS6000_BTI_V4SF, RS6000_BTI_V4SF, 0 },
   { ALTIVEC_BUILTIN_VEC_CMPLE, VSX_BUILTIN_XVCMPGEDP,
@@ -4249,8 +4237,6 @@  const struct altivec_builtin_types altivec_overloa
   { VSX_BUILTIN_VEC_LD, VSX_BUILTIN_LXVD2X_V2DI,
     RS6000_BTI_V2DI, RS6000_BTI_INTSI, ~RS6000_BTI_long_long, 0 },
   { VSX_BUILTIN_VEC_LD, VSX_BUILTIN_LXVD2X_V2DI,
-    RS6000_BTI_V2DI, RS6000_BTI_INTSI, ~RS6000_BTI_long_long, 0 },
-  { VSX_BUILTIN_VEC_LD, VSX_BUILTIN_LXVD2X_V2DI,
     RS6000_BTI_unsigned_V1TI, RS6000_BTI_INTSI, ~RS6000_BTI_UINTTI, 0 },
   { VSX_BUILTIN_VEC_LD, VSX_BUILTIN_LXVD2X_V2DI,
     RS6000_BTI_unsigned_V2DI, RS6000_BTI_INTSI,
@@ -5620,8 +5606,6 @@  const struct altivec_builtin_types altivec_overloa
   { P8V_BUILTIN_VEC_VMRGOW, P8V_BUILTIN_VMRGOW_V2DI,
     RS6000_BTI_V2DI, RS6000_BTI_V2DI, RS6000_BTI_V2DI, 0 },
   { P8V_BUILTIN_VEC_VMRGOW, P8V_BUILTIN_VMRGOW_V2DI,
-    RS6000_BTI_V2DI, RS6000_BTI_V2DI, RS6000_BTI_V2DI, 0 },
-  { P8V_BUILTIN_VEC_VMRGOW, P8V_BUILTIN_VMRGOW_V2DI,
     RS6000_BTI_unsigned_V2DI, RS6000_BTI_unsigned_V2DI,
     RS6000_BTI_unsigned_V2DI, 0 },
   { P8V_BUILTIN_VEC_VMRGOW, P8V_BUILTIN_VMRGOW_V2DI,