[RFC] Experimental __attribute__((saturating)) on integer types.

Message ID 008b01d7b316$63e54060$2bafc120$@nextmovesoftware.com
State New
Headers
Series [RFC] Experimental __attribute__((saturating)) on integer types. |

Commit Message

Roger Sayle Sept. 26, 2021, 8:37 p.m. UTC
  This patch is prototype proof-of-concept (and request for feedback)
that touches the front-end, middle-end and backend.  My recent patch to
perform RTL constant folding of saturating arithmetic revealed how
difficult it is to generate a (portable) test case for that functionality.
This patch experiments with adding an "saturating" attribute to the
C-family front-ends to set the TYPE_SATURATING flag on integer types,
initially as a debugging/testing tool for the middle-end.  GCC already
contains logic during RTL expansion to emit [us]s_plus and [us]s_minus
instructions via the standard named [us]ss{add,sub}<mode>3 optabs.

Disappointingly, although the documentation for ssplus<mode>3 patterns
implies this should work for arbitrary (i.e. integer) modes, the
optab querying infrastructure (based on optabs.def) is currently
limited to fixed-point modes.  Hence the patch below contains a
tweak to optabs.def.

With both of the above pieces in place, GCC can now generate an
ssaddsi3 instruction (such as the example provided for the nvptx
backend), or ICE if the required saturating operation doesn't exist,
as libgcc doesn't (yet) provide fall-back implementations for
saturating signed and unsigned arithmetic.

Sticking with the positive, the following code:

typedef int sat_int32 __attribute__ ((saturating));
int ssadd32(int x, int y) {
  sat_int32 t = (sat_int32)x + (sat_int32)y;
  return (int)t;
}

with this patch, now generates the following on nvptx-none:

mov.u32 %r23, %ar0;
mov.u32 %r24, %ar1;
add.sat.s32     %value, %r23, %r24;


Are any of the independent chunks below suitable for the compiler?
Tested on nvptx-none and x86_64-pc-linux-gnu, but nothing changes
unless __attribute__ ((saturating)) is explicitly added to the source
code [and I'd recommend against that except for testing purposes].

Eventually saturating arithmetic such as this might be useful for
kernel security (a hot topic of last week's Linux Plumbers' Conference)
but it would require a lot of polishing to clean-up the rough edges
(and ideally better hardware support).

Thoughts?  Even if a new C-family attribute is unsuitable, is my
logic/implementation in handle_saturating_attribute correct?


2021-09-26  Roger Sayle  <roger@nextmovesoftware.com>

gcc/c-family/ChangeLog
	* c-attribs (handle_saturating_attribute): New callback function
	for a "saturating" attribute to set the TYPE_SATURATING flag on
	an integer type.
	(c_common_attribute_table): New entry for "saturating".

gcc/ChangeLog
	* config/nvptx/nvptx.md (ssaddsi3, sssubsi3): New define_insn
	patterns for SImode saturating addition/subtraction respectively.

	* optabs.def (ssadd_optab, usadd_optab, ssub_optab, usub_optab):
	Allow querying of integer modes in addition to fixed-point modes.

	* print-tree.c (print_node): Output "saturating" when the
	TYPE_SATURATING flag is set on integer types.

Roger
--
  

Comments

Richard Biener Sept. 27, 2021, 1:45 p.m. UTC | #1
On Sun, Sep 26, 2021 at 10:38 PM Roger Sayle <roger@nextmovesoftware.com> wrote:
>
>
> This patch is prototype proof-of-concept (and request for feedback)
> that touches the front-end, middle-end and backend.  My recent patch to
> perform RTL constant folding of saturating arithmetic revealed how
> difficult it is to generate a (portable) test case for that functionality.
> This patch experiments with adding an "saturating" attribute to the
> C-family front-ends to set the TYPE_SATURATING flag on integer types,
> initially as a debugging/testing tool for the middle-end.  GCC already
> contains logic during RTL expansion to emit [us]s_plus and [us]s_minus
> instructions via the standard named [us]ss{add,sub}<mode>3 optabs.
>
> Disappointingly, although the documentation for ssplus<mode>3 patterns
> implies this should work for arbitrary (i.e. integer) modes, the
> optab querying infrastructure (based on optabs.def) is currently
> limited to fixed-point modes.  Hence the patch below contains a
> tweak to optabs.def.
>
> With both of the above pieces in place, GCC can now generate an
> ssaddsi3 instruction (such as the example provided for the nvptx
> backend), or ICE if the required saturating operation doesn't exist,
> as libgcc doesn't (yet) provide fall-back implementations for
> saturating signed and unsigned arithmetic.
>
> Sticking with the positive, the following code:
>
> typedef int sat_int32 __attribute__ ((saturating));
> int ssadd32(int x, int y) {
>   sat_int32 t = (sat_int32)x + (sat_int32)y;
>   return (int)t;
> }
>
> with this patch, now generates the following on nvptx-none:
>
> mov.u32 %r23, %ar0;
> mov.u32 %r24, %ar1;
> add.sat.s32     %value, %r23, %r24;
>
>
> Are any of the independent chunks below suitable for the compiler?
> Tested on nvptx-none and x86_64-pc-linux-gnu, but nothing changes
> unless __attribute__ ((saturating)) is explicitly added to the source
> code [and I'd recommend against that except for testing purposes].
>
> Eventually saturating arithmetic such as this might be useful for
> kernel security (a hot topic of last week's Linux Plumbers' Conference)
> but it would require a lot of polishing to clean-up the rough edges
> (and ideally better hardware support).
>
> Thoughts?  Even if a new C-family attribute is unsuitable, is my
> logic/implementation in handle_saturating_attribute correct?

I wonder if you need to use tricks like those in handle_vector_size_attribute
to handle say

 __attribute__((saturating)) int foo(void);

Now - ISTR that elsewhere Joseph suggested that taking on
saturating operations by type was eventually misguided and we should
have instead added saturating arithmetic tree codes that we could
expose via some builtin functions like the overflow ones.

Btw, I do welcome patches like this to eventually make the
types accessible to the GIMPLE frontend though we might need
something like 'stopat' to stop us from trying to expand things to
RTL when not all targets support saturating arithmetic and we
have no fallback libgcc implementation.

I think the print-tree bits are OK.

Joseph may want to chime in as to whether it's good to expose
saturating "types" more or whether that works against any intent
to retire that detail.

Richard.

>
> 2021-09-26  Roger Sayle  <roger@nextmovesoftware.com>
>
> gcc/c-family/ChangeLog
>         * c-attribs (handle_saturating_attribute): New callback function
>         for a "saturating" attribute to set the TYPE_SATURATING flag on
>         an integer type.
>         (c_common_attribute_table): New entry for "saturating".
>
> gcc/ChangeLog
>         * config/nvptx/nvptx.md (ssaddsi3, sssubsi3): New define_insn
>         patterns for SImode saturating addition/subtraction respectively.
>
>         * optabs.def (ssadd_optab, usadd_optab, ssub_optab, usub_optab):
>         Allow querying of integer modes in addition to fixed-point modes.
>
>         * print-tree.c (print_node): Output "saturating" when the
>         TYPE_SATURATING flag is set on integer types.
>
> Roger
> --
>
  
Joseph Myers Sept. 27, 2021, 8:33 p.m. UTC | #2
On Mon, 27 Sep 2021, Richard Biener via Gcc-patches wrote:

> Now - ISTR that elsewhere Joseph suggested that taking on
> saturating operations by type was eventually misguided and we should
> have instead added saturating arithmetic tree codes that we could
> expose via some builtin functions like the overflow ones.

There are several issues there:

* saturating (and other fixed-point) types at the C API level;

* saturating (and other fixed-point) types in GIMPLE;

* saturating (and other fixed-point) modes in RTL.

As I said in 
<https://gcc.gnu.org/legacy-ml/gcc-patches/2011-05/msg00846.html>, I think 
having special modes for these kinds of types is a bad idea, because 
operations should be lowered to ordinary integer arithmetic at some point 
in GIMPLE, or at the latest in expand.  (Maybe a few cases would sensibly 
use libgcc functions rather than inline arithmetic, but those would be the 
exception.  We handle inline expansion of the overflow-checking built-in 
functions in general, much of that code could be shared to expand 
saturating arithmetic in general on hardware lacking the operations.)  At 
present, there are loads of fixed-point machine modes, and very many 
libgcc functions on the targets supporting fixed-point, and very little 
optimization done on these operations, when if the operations were lowered 
to normal arithmetic earlier, generic code in the compiler could optimize 
them.  (Back ends would still need to know enough about the types in 
question to be able to implement any desired ABI differences from the 
underlying ordinary integer types.)

My inclination is that GIMPLE should also use saturating operations rather 
than saturating types.

At the C API level it's less clear.  When you have saturating types in the 
front end - as in those we currently have implemented, from the Embedded C 
TR, for example - at some point they need lowering to saturating 
operations on normal types, if you follow my suggested model above.  That 
could be at gimplification, or you could allow saturating types in GIMPLE 
but then have some early pass that replaces them by normal types using 
saturating operations.

For some kinds of algorithm, saturating types may well be a convenient 
abstraction for the user.  For others, saturating operations on normal 
types may make more sense (e.g. using saturating arithmetic on size_t to 
compute an allocation size, knowing that SIZE_MAX will result in 
allocation failure if passed to an allocation function).

As for the specific patch: it looks like you create a new type every time 
the user uses the attribute.  If you allow users to create such saturating 
types (distinct from the fixed-point ones) at all, I think that every time 
someone requests int __attribute__ ((saturating)) it should produce the 
same type (and likewise for each other underlying non-saturating integer 
type, and watch out for any interactions with types created for 
bit-fields).  Then there would be API design questions to address such as 
the results of converting out-of-range integer or floating-point values - 
or, for that matter, wider pointers - to a saturating type.
  

Patch

diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index 007b928..cd58605 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -169,6 +169,7 @@  static tree handle_objc_nullability_attribute (tree *, tree, tree, int, bool *);
 static tree handle_signed_bool_precision_attribute (tree *, tree, tree, int,
 						    bool *);
 static tree handle_retain_attribute (tree *, tree, tree, int, bool *);
+static tree handle_saturating_attribute (tree *, tree, tree, int, bool *);
 
 /* Helper to define attribute exclusions.  */
 #define ATTR_EXCL(name, function, type, variable)	\
@@ -536,6 +537,8 @@  const struct attribute_spec c_common_attribute_table[] =
 			      handle_special_var_sec_attribute, attr_section_exclusions },
   { "access",		      1, 3, false, true, true, false,
 			      handle_access_attribute, NULL },
+  { "saturating",	      0, 0, false, true, false, false,
+			      handle_saturating_attribute, NULL },
   /* Attributes used by Objective-C.  */
   { "NSObject",		      0, 0, true, false, false, false,
 			      handle_nsobject_attribute, NULL },
@@ -5761,6 +5764,27 @@  handle_objc_nullability_attribute (tree *node, tree name, tree args,
   return NULL_TREE;
 }
 
+/* Handle a "saturating" attribute.  */
+
+static tree
+handle_saturating_attribute (tree *node, tree name, tree ARG_UNUSED (args),
+			     int flags, bool *no_add_attrs)
+{
+  tree type = *node;
+
+  *no_add_attrs = true;
+
+  if (TREE_CODE (type) == INTEGER_TYPE)
+    {
+      if (!(flags & (int) ATTR_FLAG_TYPE_IN_PLACE))
+	*node = type = build_duplicate_type (type);
+      TYPE_SATURATING (type) = 1;
+    }
+  else
+    warning (OPT_Wattributes,"saturating attribute on non-integer type");
+  return NULL_TREE;
+}
+
 /* Attempt to partially validate a single attribute ATTR as if
    it were to be applied to an entity OPER.  */
 
diff --git a/gcc/config/nvptx/nvptx.md b/gcc/config/nvptx/nvptx.md
index 108de1c..79fa300 100644
--- a/gcc/config/nvptx/nvptx.md
+++ b/gcc/config/nvptx/nvptx.md
@@ -1217,6 +1217,22 @@ 
   return asms[INTVAL (operands[2])];
 })
 
+;; Saturating integer arithmetic
+
+(define_insn "ssaddsi3"
+  [(set (match_operand:SI 0 "nvptx_register_operand" "=R")
+	(ss_plus:SI (match_operand:SI 1 "nvptx_register_operand" "R")
+		    (match_operand:SI 2 "nvptx_nonmemory_operand" "Ri")))]
+  ""
+  "%.\\tadd.sat.s32\\t%0, %1, %2;")
+
+(define_insn "sssubsi3"
+  [(set (match_operand:SI 0 "nvptx_register_operand" "=R")
+	(ss_minus:SI (match_operand:SI 1 "nvptx_register_operand" "R")
+		     (match_operand:SI 2 "nvptx_register_operand" "R")))]
+  ""
+  "%.\\tsub.sat.s32\\t%0, %1, %2;")
+
 ;; Miscellaneous
 
 (define_insn "nop"
diff --git a/gcc/optabs.def b/gcc/optabs.def
index 201b8aa..7d157ad 100644
--- a/gcc/optabs.def
+++ b/gcc/optabs.def
@@ -106,14 +106,18 @@  OPTAB_NX(add_optab, "add$Q$a3")
 OPTAB_VL(addv_optab, "addv$I$a3", PLUS, "add", '3', gen_intv_fp_libfunc)
 OPTAB_VX(addv_optab, "add$F$a3")
 OPTAB_NL(ssadd_optab, "ssadd$Q$a3", SS_PLUS, "ssadd", '3', gen_signed_fixed_libfunc)
+OPTAB_NX(ssadd_optab, "ssadd$I$a3")
 OPTAB_NL(usadd_optab, "usadd$Q$a3", US_PLUS, "usadd", '3', gen_unsigned_fixed_libfunc)
+OPTAB_NX(usadd_optab, "usadd$I$a3")
 OPTAB_NL(sub_optab, "sub$P$a3", MINUS, "sub", '3', gen_int_fp_fixed_libfunc)
 OPTAB_NX(sub_optab, "sub$F$a3")
 OPTAB_NX(sub_optab, "sub$Q$a3")
 OPTAB_VL(subv_optab, "subv$I$a3", MINUS, "sub", '3', gen_intv_fp_libfunc)
 OPTAB_VX(subv_optab, "sub$F$a3")
 OPTAB_NL(sssub_optab, "sssub$Q$a3", SS_MINUS, "sssub", '3', gen_signed_fixed_libfunc)
+OPTAB_NX(sssub_optab, "sssub$I$a3")
 OPTAB_NL(ussub_optab, "ussub$Q$a3", US_MINUS, "ussub", '3', gen_unsigned_fixed_libfunc)
+OPTAB_NX(sssub_optab, "ussub$I$a3")
 OPTAB_NL(smul_optab, "mul$Q$a3", MULT, "mul", '3', gen_int_fp_fixed_libfunc)
 OPTAB_NX(smul_optab, "mul$P$a3")
 OPTAB_NX(smul_optab, "mul$F$a3")
diff --git a/gcc/print-tree.c b/gcc/print-tree.c
index d1fbd04..d0e135e 100644
--- a/gcc/print-tree.c
+++ b/gcc/print-tree.c
@@ -620,6 +620,10 @@  print_node (FILE *file, const char *prefix, tree node, int indent,
 	  && TYPE_REVERSE_STORAGE_ORDER (node))
 	fputs (" reverse-storage-order", file);
 
+      if (code == INTEGER_TYPE
+	  && TYPE_SATURATING (node))
+	fputs(" saturating", file);
+
       if ((code == RECORD_TYPE
 	   || code == UNION_TYPE)
 	  && TYPE_CXX_ODR_P (node))