[MPX,2/X] Pointers Checker [5/25] Tree and gimple ifaces

On 29 Oct 13:39, Jeff Law wrote:
> On 10/24/13 08:43, Ilya Enkovich wrote:
> >
> >+/* Return the number of arguments used by call statement GS.  */
> >+
> >+static inline unsigned
> >+gimple_call_num_nobnd_args (const_gimple gs)
> >+{
> >+  unsigned num_args = gimple_call_num_args (gs);
> >+  unsigned res = num_args;
> >+  for (unsigned n = 0; n < num_args; n++)
> >+    if (POINTER_BOUNDS_P (gimple_call_arg (gs, n)))
> >+      res--;
> >+  return res;
> >+}
> "number of arguments" seems wrong.  Aren't you counting the number
> of arguments without bounds?

Damned copy-paste.
> 
> 
> >+
> >+/* Nonzero if this type supposes bounds existence.  */
> >+#define BOUNDED_TYPE_P(type) \
> >+  (TREE_CODE (type) == POINTER_TYPE \
> >+    || TREE_CODE (type) == REFERENCE_TYPE)
> So how is this different than POINTER_TYPE_P?
> 
> 
> If you really want BOUNDED_TYPE_P, perhaps define it in terms of
> POINTER_TYPE_P?
> 
> With that and the comment fix, this is fine.
> 
> jeff

I'd like to keep this macro.  Currently it is equal to POINTER_TYPE_P but semantics is a little different.

Below is a fixed version to be committed.

Thanks!
Ilya
--

On Tue, Oct 29, 2013 at 10:53 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> On 29 Oct 13:39, Jeff Law wrote:
>> On 10/24/13 08:43, Ilya Enkovich wrote:
>> >
>> >+/* Return the number of arguments used by call statement GS.  */
>> >+
>> >+static inline unsigned
>> >+gimple_call_num_nobnd_args (const_gimple gs)
>> >+{
>> >+  unsigned num_args = gimple_call_num_args (gs);
>> >+  unsigned res = num_args;
>> >+  for (unsigned n = 0; n < num_args; n++)
>> >+    if (POINTER_BOUNDS_P (gimple_call_arg (gs, n)))
>> >+      res--;
>> >+  return res;
>> >+}
>> "number of arguments" seems wrong.  Aren't you counting the number
>> of arguments without bounds?
>
> Damned copy-paste.
>>
>>
>> >+
>> >+/* Nonzero if this type supposes bounds existence.  */
>> >+#define BOUNDED_TYPE_P(type) \
>> >+  (TREE_CODE (type) == POINTER_TYPE \
>> >+    || TREE_CODE (type) == REFERENCE_TYPE)
>> So how is this different than POINTER_TYPE_P?
>>
>>
>> If you really want BOUNDED_TYPE_P, perhaps define it in terms of
>> POINTER_TYPE_P?
>>
>> With that and the comment fix, this is fine.
>>
>> jeff
>
> I'd like to keep this macro.  Currently it is equal to POINTER_TYPE_P but semantics is a little different.
>
> Below is a fixed version to be committed.
>
> Thanks!
> Ilya
> --
>
> diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c
> index e4b0f81..248dfea 100644
> --- a/gcc/gimple-pretty-print.c
> +++ b/gcc/gimple-pretty-print.c
> @@ -539,11 +539,12 @@ dump_gimple_assign (pretty_printer *buffer, gimple gs, int spc, int flags)
>  static void
>  dump_gimple_return (pretty_printer *buffer, gimple gs, int spc, int flags)
>  {
> -  tree t;
> +  tree t, t2;
>
>    t = gimple_return_retval (gs);
> +  t2 = gimple_return_retbnd (gs);
>    if (flags & TDF_RAW)
> -    dump_gimple_fmt (buffer, spc, flags, "%G <%T>", gs, t);
> +    dump_gimple_fmt (buffer, spc, flags, "%G <%T %T>", gs, t, t2);
>    else
>      {
>        pp_string (buffer, "return");
> @@ -552,6 +553,11 @@ dump_gimple_return (pretty_printer *buffer, gimple gs, int spc, int flags)
>           pp_space (buffer);
>           dump_generic_node (buffer, t, spc, flags, false);
>         }
> +      if (t2)
> +       {
> +         pp_string (buffer, ", ");
> +         dump_generic_node (buffer, t2, spc, flags, false);
> +       }
>        pp_semicolon (buffer);
>      }
>  }
> diff --git a/gcc/gimple.c b/gcc/gimple.c
> index 3ddceb9..20f6010 100644
> --- a/gcc/gimple.c
> +++ b/gcc/gimple.c
> @@ -174,7 +174,7 @@ gimple_build_with_ops_stat (enum gimple_code code, unsigned subcode,
>  gimple
>  gimple_build_return (tree retval)
>  {
> -  gimple s = gimple_build_with_ops (GIMPLE_RETURN, ERROR_MARK, 1);
> +  gimple s = gimple_build_with_ops (GIMPLE_RETURN, ERROR_MARK, 2);

Ick - you enlarge all return statements?  But you don't set the actual value?
So why allocate it with 2 ops in the first place??

[Seems I completely missed that MPX changes "gimple" and the design
document that was posted somewhere??]

Bah.

Where is the update to gimple.texi and tree.texi?

Richard.

>    if (retval)
>      gimple_return_set_retval (s, retval);
>    return s;
> @@ -366,6 +366,26 @@ gimple_build_call_from_tree (tree t)
>  }
>
>
> +/* Return index of INDEX's non bound argument of the call.  */
> +
> +unsigned
> +gimple_call_get_nobnd_arg_index (const_gimple gs, unsigned index)
> +{
> +  unsigned num_args = gimple_call_num_args (gs);
> +  for (unsigned n = 0; n < num_args; n++)
> +    {
> +      if (POINTER_BOUNDS_P (gimple_call_arg (gs, n)))
> +       continue;
> +      else if (index)
> +       index--;
> +      else
> +       return n;
> +    }
> +
> +  gcc_unreachable ();
> +}
> +
> +
>  /* Extract the operands and code for expression EXPR into *SUBCODE_P,
>     *OP1_P, *OP2_P and *OP3_P respectively.  */
>
> diff --git a/gcc/gimple.h b/gcc/gimple.h
> index fef64cd..c7ce394 100644
> --- a/gcc/gimple.h
> +++ b/gcc/gimple.h
> @@ -919,6 +919,7 @@ extern tree get_initialized_tmp_var (tree, gimple_seq *, gimple_seq *);
>  extern tree get_formal_tmp_var (tree, gimple_seq *);
>  extern void declare_vars (tree, gimple, bool);
>  extern void annotate_all_with_location (gimple_seq, location_t);
> +extern unsigned gimple_call_get_nobnd_arg_index (const_gimple, unsigned);
>
>  /* Validation of GIMPLE expressions.  Note that these predicates only check
>     the basic form of the expression, they don't recurse to make sure that
> @@ -2414,6 +2415,32 @@ gimple_call_arg (const_gimple gs, unsigned index)
>  }
>
>
> +/* Return the number of arguments used by call statement GS
> +   ignoring bound ones.  */
> +
> +static inline unsigned
> +gimple_call_num_nobnd_args (const_gimple gs)
> +{
> +  unsigned num_args = gimple_call_num_args (gs);
> +  unsigned res = num_args;
> +  for (unsigned n = 0; n < num_args; n++)
> +    if (POINTER_BOUNDS_P (gimple_call_arg (gs, n)))
> +      res--;
> +  return res;
> +}
> +
> +
> +/* Return INDEX's call argument ignoring bound ones.  */
> +static inline tree
> +gimple_call_nobnd_arg (const_gimple gs, unsigned index)
> +{
> +  /* No bound args may exist if pointers checker is off.  */
> +  if (!flag_check_pointer_bounds)
> +    return gimple_call_arg (gs, index);
> +  return gimple_call_arg (gs, gimple_call_get_nobnd_arg_index (gs, index));
> +}
> +
> +
>  /* Return a pointer to the argument at position INDEX for call
>     statement GS.  */
>
> @@ -5220,6 +5247,26 @@ gimple_return_set_retval (gimple gs, tree retval)
>  }
>
>
> +/* Return the return bounds for GIMPLE_RETURN GS.  */
> +
> +static inline tree
> +gimple_return_retbnd (const_gimple gs)
> +{
> +  GIMPLE_CHECK (gs, GIMPLE_RETURN);
> +  return gimple_op (gs, 1);
> +}
> +
> +
> +/* Set RETVAL to be the return bounds for GIMPLE_RETURN GS.  */
> +
> +static inline void
> +gimple_return_set_retbnd (gimple gs, tree retval)
> +{
> +  GIMPLE_CHECK (gs, GIMPLE_RETURN);
> +  gimple_set_op (gs, 1, retval);
> +}
> +
> +
>  /* Returns true when the gimple statement STMT is any of the OpenMP types.  */
>
>  #define CASE_GIMPLE_OMP                                \
> diff --git a/gcc/tree-core.h b/gcc/tree-core.h
> index ea110bc..638b3ab 100644
> --- a/gcc/tree-core.h
> +++ b/gcc/tree-core.h
> @@ -448,6 +448,8 @@ enum tree_index {
>    TI_FILEPTR_TYPE,
>    TI_POINTER_SIZED_TYPE,
>
> +  TI_POINTER_BOUNDS_TYPE,
> +
>    TI_DFLOAT32_TYPE,
>    TI_DFLOAT64_TYPE,
>    TI_DFLOAT128_TYPE,
> diff --git a/gcc/tree.c b/gcc/tree.c
> index 0a42109..de08aea 100644
> --- a/gcc/tree.c
> +++ b/gcc/tree.c
> @@ -9765,6 +9765,8 @@ build_common_tree_nodes (bool signed_char, bool short_double)
>    void_type_node = make_node (VOID_TYPE);
>    layout_type (void_type_node);
>
> +  pointer_bounds_type_node = targetm.chkp_bound_type ();
> +
>    /* We are not going to have real types in C with less than byte alignment,
>       so we might as well not have any types that claim to have it.  */
>    TYPE_ALIGN (void_type_node) = BITS_PER_UNIT;
> diff --git a/gcc/tree.h b/gcc/tree.h
> index ab1d0ab..3fe751e 100644
> --- a/gcc/tree.h
> +++ b/gcc/tree.h
> @@ -546,6 +546,17 @@ extern void omp_clause_range_check_failed (const_tree, const char *, int,
>  #define POINTER_BOUNDS_TYPE_P(NODE) \
>    (TREE_CODE (NODE) == POINTER_BOUNDS_TYPE)
>
> +/* Nonzero if this node has a pointer bounds type.  */
> +#define POINTER_BOUNDS_P(NODE) \
> +  (POINTER_BOUNDS_TYPE_P (TREE_TYPE (NODE)))
> +
> +/* Nonzero if this type supposes bounds existence.  */
> +#define BOUNDED_TYPE_P(type) (POINTER_TYPE_P (type))
> +
> +/* Nonzero for objects with bounded type.  */
> +#define BOUNDED_P(node) \
> +  BOUNDED_TYPE_P (TREE_TYPE (node))
> +
>  /* Nonzero if this type is the (possibly qualified) void type.  */
>  #define VOID_TYPE_P(NODE) (TREE_CODE (NODE) == VOID_TYPE)
>
> @@ -3197,6 +3208,8 @@ tree_operand_check_code (const_tree __t, enum tree_code __code, int __i,
>  #define complex_double_type_node       global_trees[TI_COMPLEX_DOUBLE_TYPE]
>  #define complex_long_double_type_node  global_trees[TI_COMPLEX_LONG_DOUBLE_TYPE]
>
> +#define pointer_bounds_type_node        global_trees[TI_POINTER_BOUNDS_TYPE]
> +
>  #define void_type_node                 global_trees[TI_VOID_TYPE]
>  /* The C type `void *'.  */
>  #define ptr_type_node                  global_trees[TI_PTR_TYPE]

On 10/30/13 03:26, Richard Biener wrote:
>> diff --git a/gcc/gimple.c b/gcc/gimple.c
>> index 3ddceb9..20f6010 100644
>> --- a/gcc/gimple.c
>> +++ b/gcc/gimple.c
>> @@ -174,7 +174,7 @@ gimple_build_with_ops_stat (enum gimple_code code, unsigned subcode,
>>   gimple
>>   gimple_build_return (tree retval)
>>   {
>> -  gimple s = gimple_build_with_ops (GIMPLE_RETURN, ERROR_MARK, 1);
>> +  gimple s = gimple_build_with_ops (GIMPLE_RETURN, ERROR_MARK, 2);
>
> Ick - you enlarge all return statements?  But you don't set the actual value?
> So why allocate it with 2 ops in the first place??
>
> [Seems I completely missed that MPX changes "gimple" and the design
> document that was posted somewhere??]
I'd assumed that'd be in a follow-up patch and probably would be bounds 
on the return value or NULL if there were no bounds.

I'm terribly concerned about the enlarging of the return statements.  On 
the other hand, if something enlarged a GIMPLE_ASSIGN, then I would have 
called it out for further discussion.


>
> Bah.
>
> Where is the update to gimple.texi and tree.texi?
Good catch.  Ilya, please send a patch to update the docs.