Re: PATCH to implement `restrict' in C

[Jeffrey A Law <law at cygnus dot com>]

  In message <199810092015.NAA13687@smtp.earthlink.net>you write:
  > Fri Oct  9 12:10:32 1998  Mark Mitchell  <mark@markmitchell.com>
  > 
  > 	* invoke.texi: Document -frestrict.
  > 	
  > 	* rtl.h (record_alias_subset): New function.
  > 	* alias.c: Include splay-tree.h.
  > 	(alias_set_entry_t): New type.
  > 	(CHECK_ALIAS_SETS_FOR_CONSISTENCY): Remove.
  > 	(DIFFERENT_ALIAS_SETS_P): Use mem_in_disjoint_alias_sets_p.
  > 	(mems_in_disjoin_alias_sets_p): New function.
  > 	(alias_set_compare): Likewise.
  > 	(insert_subset_children): Likewise.
  > 	(get_alias_set_entry): Likewise.
  > 	* splay-tree.h: New file.
  > 	* splay-tree.c: Likewise.
  > 	* Makefile.in (OBJS): Add splay-tree.o.
  > 	(c-common.o): Depend on rtl.h.
  > 	(splay-tree.o): List dependencies.
  > 
  > 	* tree.h (TYPE_RESTRICT): New macro.
  > 	(TYPE_UNQUALIFIED): New manifest constant.
  > 	(TYPE_QUAL_CONST): Likewise
  > 	(TYPE_QUAL_VOLATILE): Likewise.
  > 	(TYPE_QUAL_RESTRICT): Likewise.
  > 	(tree_type): Add restrict_flag.  Reduce count of free bits.
  > 	(DECL_POINTER_ALIAS_SET): New macro.
  > 	(DECL_POINTER_ALIAS_SET_KNOWN_P): Likewise.
  > 	(tree_decl): Add pointer_alias_set.
  > 	(build_qualified_type): New function.
  > 	(build_type_variant): Define in terms of build_qualified_type.
  > 	* tree.c (set_type_quals): New function.
  > 	(make_node): Initializae DECL_POINTER_ALIAS_SET.
  > 	(build_type_attribute_variant): Use build_qualified_type and
  > 	set_type_quals.
  > 	(build_type_variant): Rename, and modify, to become...
  > 	(build_qualified_type): New function.
  > 	(build_complex_type): Use set_type_quals.
  > 
  > 	* c-tree.h (C_TYPE_OBJECT_P): New macro.
  > 	(C_TYPE_FUNCTION_P): Likewise.
  > 	(C_TYPE_INCOMPLETE_P): Likewise.
  > 	(C_TYPE_OBJECT_OR_INCOMPLETE_P): Likewise.
  > 	(c_apply_type_quals_to_decl): New function.
  > 	(c_build_qualified_type): New function.
  > 	(c_build_type_variant): Define in terms of c_build_qualified_type.
  > 	(c_comp_type_quals): New function.
  > 	(flag_restrict): Declare.
  > 	* c-typeck.c (qualify_type): Use c_build_qualified_type.
  > 	(common_type): Change to use TYPE_QUALS.
  > 	(comptypes): Likewise. Use c_comp_type_quals.
  > 	(convert_for_assignment): Likewise.
  > 	* c-aux-info.c (gen_type): Likewise.  Deal with `restrict'.
  > 	* c-decl.c (flag_restrict): Define.
  > 	(c_decode_option): Handle -frestrict.
  > 	(grokdeclarator): Update to handle restrict.  Use TYPE_QUALS,
  > 	c_build_qualified_type, etc.  Use c_apply_type_quals_to_decl.
  > 	* c-lex.c (init_lex): Deal with restrict.
  > 	(init_lex): Don't treat restrict as a reserved word in
  > 	-traditional mode, or with -fno-restrict.
  > 	* c-lex.h (rid): Add RID_RESTRICT.
  > 	* c-parse.gperf (restrict, __restrict, __restrict__): Make
  > 	equivalent to RID_RESTRICT.
  > 	* c-parse.in (TYPE_QUAL): Update comment.
  > 	* c-common.c: Include rtl.h.
  > 	(c_find_base_decl): New function.
  > 	(c_build_type_variant): Rename, and modify, to become ...
  > 	(c_build_qualified_type): New function.
  > 	(c_comp_type_quals): Likewise.
  > 	(c_apply_type_quals_to_decl): Likewise.
  > 	(c_get_alias_set): For INDIRECT_REFs, check to see if we can find
  > 	a particular alias set for the reference.
Mostly OK.  I'm not a big fan of the "_t" convention for types, and you will
not generally find it used in gcc itself (with the exception of dyn-string).
Unless we are literally importing this code from elsewhere, we should avoid
changing the convention for typedefs.

You code declares xmalloc and xrealloc, it should not do that unless absolutely
necessary (and it may be, I didn't check this closely).

  > +   else if  (n == (*parent)->right && *parent == (*grandparent)->right)
  > +     {
  > +       splay_tree_node_t p = *parent;
A nit -- too many spaces in the if.  

Your patchkit contained alias.c changes twice.  I looked at the first and
assumed the second was the same :-)

I'm not sure we want a -f option to control restrict.  I think we're better off
with an option that enables isoc9x features, instead of controlling each one
separately.

Of course we can us __restrict__ for non-iso code.

Why do we need casts in the call to splay_tree_insert in record_alias_subset?

+      splay_tree_node_t* node;
:-)  You know the drill here :-)


Here's some questions which came up during jfc's original restrict work.  Can
you comment on any of these questions?

1. My first implementation, which I wrote based on documentation for Sun's
compiler, only supports declaring function arguments "restrict".  Would it
still be useful with this limitation?  Extending restrict to apply to all
variable declarations would not be too hard, but getting the expected
semantics might be.  That is, the compiler could be made to accept but
ignore restrict applied to non-parameter variables (this is legal; I am
asking if is it good).

2. C9X restrict is a type qualifier like const or volatile.  My
implementation made restrict apply to a variable.  So, for example, one can
not declare
	typedef int * restrict restrict_pointer_to_int;
or
	int * restrict * foo;
but only
	int * restrict bar;
Is this a serious limitation?  It might be hard to change and the changes
could cause more divergence of the gcc and egcs back end interfaces.

3. Are there any C++ constructs with useful aliasing properties?  The
latest snapshot should know that global non-placement operator new and
new[] return pointers to unique storage.  Is anything useful known about
"this" in constructors, destructors, or other member functions?

4. Should C++ support restrict too?  If so, what language specific concerns
are there?  For example, can member functions be declared "restrict",
causing the this pointer to be restrict-qualified?


And in one of the replies:

> 2. C9X restrict is a type qualifier like const or volatile.  My
> implementation made restrict apply to a variable.  So, for example,
> one can not declare typedef int * restrict restrict_pointer_to_int;
> or int * restrict * foo; but only int * restrict bar; Is this a
> serious limitation?  It might be hard to change and the changes
> could cause more divergence of the gcc and egcs back end interfaces.

I think this would be a serious limitation, in the light of C9X
compliance. When I read the draft right

struct foo{
  struct foo * restrict next;
  int value;
};

Again, the implementation is free to ignore the declaration - it
should not reject it.

> 4. Should C++ support restrict too?  If so, what language specific concerns
> are there?  For example, can member functions be declared "restrict",
> causing the this pointer to be restrict-qualified?

I think the formal definition of restrict (6.5.3.1) gives a clue, as
well as the syntax. restrict can appear anywhere a type qualifier is
allowed (assuming the C++ grammar is similar to C :-). So allowing
restrict declarations for member functions might be reasonable.


Basically I think the patch is mostly OK.  I'd just like some more information
about the issues that were raised regarding the original restrict
implementation to see if any of them have been dealt with.

jeff