This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Relax sizetype constraints in size_binop?


I'd like to solicit opinions on the following innocent looking middle-end
patch as an alternate solution for PR middle-end/22439.  This issue
concerns the defensive nature of fold-const.c's size_binop which calls
abort if ever passed arguments with types other than sizetype and
bitsizetype, and their signed variants, i.e. it checks for
TYPE_IS_SIZETYPE.

As witnessed by PR middle-end/22439 and Richard Henderson's analysis in
http://gcc.gnu.org/ml/gcc-patches/2005-08/msg00482.html this doesn't
play well with the philosophy used by the rest of the middle-end,
especially the gimplifiers, where tree_ssa_useless_type_conversion
and friends are more lenient provided that the types are in a weaker
sense equivalent.  This is one of the few places in the compiler where
we use pointer equality to test for identical types.

RTH's very reasonable solution to 22439 was to add an explicit check
for TYPE_IS_SIZETYPE to gimplify_one_sizepos, so that we force the
preservation of these "special" types.  This follows similar idioms
used in the front-ends.  For example in C's c-typeck.c:comptypes_internal
we find the fragment:

  /* If either type is the internal version of sizetype, return the
     language version.  */
  if (TREE_CODE (t1) == INTEGER_TYPE && TYPE_IS_SIZETYPE (t1)
      && TYPE_ORIG_SIZE_TYPE (t1))
    t1 = TYPE_ORIG_SIZE_TYPE (t1);

and likewise in the C++ code the ominous comment:

  /* If one is a sizetype, use it so size_binop doesn't blow up.  */


I'd like to propose an alternate simplifying strategy to this issue.
It turns out that apart from the strict validity checks, size_binop
is in fact a shallow wrapper around fold_build2, with some special
cases for performance.  These underlying functions routinely handle
expressions with the looser type-system used elsewhere in gimple, and
the assertion is not for correctness, but to preserve an ancient
invariant.  Indeed, back prior to tree-ssa, the middle-end had a very
forgiving (non-existant) type system, so it may have made sense to
take extra care over the non-front end types the compiler cared about.


As strange as it may seem, these assertions in size_binop and size_diffop
are (I believe) the only remaining reason for having special sizetype
types.  Indeed, the entire tree can be bootstrapped and regression tested
without failures(*) after completely removing the TYPE_IS_SIZETYPE macro,
once we allow regular types to be potentially be used to internally
calculate the sizes of objects.  I believe that LLVM, ORC, LCC and other
compilers don't internally require separate special types to track object
size, the way that GCC does.


Thoughts?

The following patch has been boostrapped on i686-pc-linux-gnu, with a
full "make bootstrap", all default languages including Ada, and regression
tested with a top-level "make -k check" with no new failures.  Although
not required by the C++ patch I proposed yesterday, relaxing this
gcc_assert from size_binop should help ease concerns that front-ends
still have to be cautious with sizetype types.



2006-11-03  Roger Sayle  <roger@eyesopen.com>

	* fold-const.c (size_binop): Relax constraint that both arguments
	must both exactly the same sizetype type.  Instead allow "equivalent"
	integer types, with matching signedness, precision and machine mode.
	(size_diffop): Likewise.


Index: fold-const.c
===================================================================
*** fold-const.c	(revision 118200)
--- fold-const.c	(working copy)
*************** size_int_kind (HOST_WIDE_INT number, enu
*** 1732,1738 ****

  /* Combine operands OP1 and OP2 with arithmetic operation CODE.  CODE
     is a tree code.  The type of the result is taken from the operands.
!    Both must be the same type integer type and it must be a size type.
     If the operands are constant, so is the result.  */

  tree
--- 1732,1738 ----

  /* Combine operands OP1 and OP2 with arithmetic operation CODE.  CODE
     is a tree code.  The type of the result is taken from the operands.
!    Both must be equivalent integer types.
     If the operands are constant, so is the result.  */

  tree
*************** size_binop (enum tree_code code, tree ar
*** 1743,1750 ****
    if (arg0 == error_mark_node || arg1 == error_mark_node)
      return error_mark_node;

!   gcc_assert (TREE_CODE (type) == INTEGER_TYPE && TYPE_IS_SIZETYPE (type)
! 	      && type == TREE_TYPE (arg1));

    /* Handle the special case of two integer constants faster.  */
    if (TREE_CODE (arg0) == INTEGER_CST && TREE_CODE (arg1) == INTEGER_CST)
--- 1743,1753 ----
    if (arg0 == error_mark_node || arg1 == error_mark_node)
      return error_mark_node;

!   gcc_assert (TREE_CODE (type) == INTEGER_TYPE
! 	      && TREE_CODE (TREE_TYPE (arg1)) == INTEGER_TYPE
! 	      && TYPE_UNSIGNED (type) == TYPE_UNSIGNED (TREE_TYPE (arg1))
! 	      && TYPE_PRECISION (type) == TYPE_PRECISION (TREE_TYPE (arg1))
! 	      && TYPE_MODE (type) == TYPE_MODE (TREE_TYPE (arg1)));

    /* Handle the special case of two integer constants faster.  */
    if (TREE_CODE (arg0) == INTEGER_CST && TREE_CODE (arg1) == INTEGER_CST)
*************** size_diffop (tree arg0, tree arg1)
*** 1775,1788 ****
    tree type = TREE_TYPE (arg0);
    tree ctype;

!   gcc_assert (TREE_CODE (type) == INTEGER_TYPE && TYPE_IS_SIZETYPE (type)
! 	      && type == TREE_TYPE (arg1));

    /* If the type is already signed, just do the simple thing.  */
    if (!TYPE_UNSIGNED (type))
      return size_binop (MINUS_EXPR, arg0, arg1);

!   ctype = type == bitsizetype ? sbitsizetype : ssizetype;

    /* If either operand is not a constant, do the conversions to the signed
       type and subtract.  The hardware will do the right thing with any
--- 1778,1799 ----
    tree type = TREE_TYPE (arg0);
    tree ctype;

!   gcc_assert (TREE_CODE (type) == INTEGER_TYPE
! 	      && TREE_CODE (TREE_TYPE (arg1)) == INTEGER_TYPE
! 	      && TYPE_UNSIGNED (type) == TYPE_UNSIGNED (TREE_TYPE (arg1))
! 	      && TYPE_PRECISION (type) == TYPE_PRECISION (TREE_TYPE (arg1))
! 	      && TYPE_MODE (type) == TYPE_MODE (TREE_TYPE (arg1)));

    /* If the type is already signed, just do the simple thing.  */
    if (!TYPE_UNSIGNED (type))
      return size_binop (MINUS_EXPR, arg0, arg1);

!   if (type == sizetype)
!     ctype = ssizetype;
!   else if (type == bitsizetype)
!     ctype = sbitsizetype;
!   else
!     ctype = lang_hooks.types.signed_type (type);

    /* If either operand is not a constant, do the conversions to the signed
       type and subtract.  The hardware will do the right thing with any


Roger
--


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]