Re: [PATCH] Check precision in STRIP_NOPS when possible (take 2)

[Richard Guenther <richard dot guenther at gmail dot com>]

On Wed, Jun 17, 2009 at 7:45 PM, Adam Nemet<anemet@caviumnetworks.com> wrote:
> Richard Guenther writes:
>> what's all this about? ?The mode check will deal with it properly.
>
> Done.
>
>> Just do
>>
>> ? if ((INTEGRAL_TYPE_P (outer_type_p)
>> ? ? ? || POINTER_TYPE_P (outer_type_p)
>> ? ? ? || TREE_CODE (outer_type_p) == OFFSET_TYPE)
>> ? ? ? && (INTEGRAL_TYPE_P (inner_type)
>> ? ? ? ? ? ? ?|| ...))
>> ? ? return TYPE_PRECISION (outer_Type) == TYPE_RECISION (inner_type);
>
> Done.
>
>> > + ?/* Aggregate types have no precision so we fall back on the machine
>> > + ? ? mode. ?The C++ front-end creates NOP_EXPR of aggregate types for
>> > + ? ? example for reinterpret_cast. ?*/
>> > + ?else if (AGGREGATE_TYPE_P (outer_type))
>> > + ? ?return TYPE_MODE (outer_type) == TYPE_MODE (inner_type);
>> > + ?else
>> > + ? ?gcc_unreachable ();
>>
>> no if's necessary, just fall back to mode comparisions.
>
> Done.
>
>> Please fold the follow-up patch and this into one and resent
>> for double-checking.
>
> Done.
>
> Here is the updated patch. ?Boostrapped and regtested on x86_64-linux
> including Ada.
>
> OK for trunk?

Ok.

Thanks,
Richard.

> Adam
>
>
> ? ? ? ?* tree.h (STRIP_NOPS, STRIP_SIGN_NOPS,
> ? ? ? ?STRIP_USELESS_TYPE_CONVERSION): Use tree_strip_nop_conversions,
> ? ? ? ?tree_strip_sign_nop_conversions and
> ? ? ? ?tree_ssa_strip_useless_type_conversions rather than stripping
> ? ? ? ?the operations here.
> ? ? ? ?(tree_strip_nop_conversions, tree_strip_sign_nop_conversions):
> ? ? ? ?Declare them.
> ? ? ? ?* gimple.h (tree_ssa_strip_useless_type_conversions): Declare it.
> ? ? ? ?* tree-ssa.c (tree_ssa_strip_useless_type_conversions): New function.
> ? ? ? ?* tree.c (tree_nop_conversion, tree_sign_nop_conversion,
> ? ? ? ?tree_strip_nop_conversions, tree_strip_sign_nop_conversions): New
> ? ? ? ?functions.
>
> testsuite/
> ? ? ? ?* gcc.c-torture/execute/bitfld-5.c: New test.
>
> Index: tree.h
> ===================================================================
> --- tree.h ? ? ?(revision 148549)
> +++ tree.h ? ? ?(working copy)
> @@ -966,30 +966,17 @@ extern void omp_clause_range_check_faile
> ? case NOP_EXPR: ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> ? case CONVERT_EXPR
>
> -/* Given an expression as a tree, strip any NON_LVALUE_EXPRs and NOP_EXPRs
> - ? that don't change the machine mode. ?*/
> +/* Given an expression as a tree, strip any conversion that generates
> + ? no instruction. ?Accepts both tree and const_tree arguments since
> + ? we are not modifying the tree itself. ?*/
>
> -#define STRIP_NOPS(EXP) ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> - ?while ((CONVERT_EXPR_P (EXP) ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> - ? ? ? ? || TREE_CODE (EXP) == NON_LVALUE_EXPR) ? ? ? ? ? ? ? ?\
> - ? ? ? ?&& TREE_OPERAND (EXP, 0) != error_mark_node ? ? ? ? ? ?\
> - ? ? ? ?&& (TYPE_MODE (TREE_TYPE (EXP)) ? ? ? ? ? ? ? ? ? ? ? ?\
> - ? ? ? ? ? ?== TYPE_MODE (TREE_TYPE (TREE_OPERAND (EXP, 0))))) \
> - ? ?(EXP) = TREE_OPERAND (EXP, 0)
> +#define STRIP_NOPS(EXP) \
> + ?(EXP) = tree_strip_nop_conversions (CONST_CAST_TREE (EXP))
>
> ?/* Like STRIP_NOPS, but don't let the signedness change either. ?*/
>
> ?#define STRIP_SIGN_NOPS(EXP) \
> - ?while ((CONVERT_EXPR_P (EXP) ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> - ? ? ? ? || TREE_CODE (EXP) == NON_LVALUE_EXPR) ? ? ? ? ? ? ? ?\
> - ? ? ? ?&& TREE_OPERAND (EXP, 0) != error_mark_node ? ? ? ? ? ?\
> - ? ? ? ?&& (TYPE_MODE (TREE_TYPE (EXP)) ? ? ? ? ? ? ? ? ? ? ? ?\
> - ? ? ? ? ? ?== TYPE_MODE (TREE_TYPE (TREE_OPERAND (EXP, 0)))) ?\
> - ? ? ? ?&& (TYPE_UNSIGNED (TREE_TYPE (EXP)) ? ? ? ? ? ? ? ? ? ?\
> - ? ? ? ? ? ?== TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (EXP, 0)))) \
> - ? ? ? ?&& (POINTER_TYPE_P (TREE_TYPE (EXP)) ? ? ? ? ? ? ? ? ? \
> - ? ? ? ? ? ?== POINTER_TYPE_P (TREE_TYPE (TREE_OPERAND (EXP, 0))))) \
> - ? ?(EXP) = TREE_OPERAND (EXP, 0)
> + ?(EXP) = tree_strip_sign_nop_conversions (CONST_CAST_TREE (EXP))
>
> ?/* Like STRIP_NOPS, but don't alter the TREE_TYPE either. ?*/
>
> @@ -1004,9 +991,8 @@ extern void omp_clause_range_check_faile
> ?/* Remove unnecessary type conversions according to
> ? ?tree_ssa_useless_type_conversion. ?*/
>
> -#define STRIP_USELESS_TYPE_CONVERSION(EXP) ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> - ? ? ?while (tree_ssa_useless_type_conversion (EXP)) ? ? ? ? ? ? ? ? ? \
> - ? ? ? EXP = TREE_OPERAND (EXP, 0)
> +#define STRIP_USELESS_TYPE_CONVERSION(EXP) \
> + ?(EXP) = tree_ssa_strip_useless_type_conversions (EXP)
>
> ?/* Nonzero if TYPE represents an integral type. ?Note that we do not
> ? ?include COMPLEX types here. ?Keep these checks in ascending code
> @@ -4632,6 +4618,8 @@ extern bool stdarg_p (tree);
> ?extern bool prototype_p (tree);
> ?extern bool auto_var_in_fn_p (const_tree, const_tree);
> ?extern tree build_low_bits_mask (tree, unsigned);
> +extern tree tree_strip_nop_conversions (tree);
> +extern tree tree_strip_sign_nop_conversions (tree);
>
> ?/* In cgraph.c */
> ?extern void change_decl_assembler_name (tree, tree);
> Index: gimple.h
> ===================================================================
> --- gimple.h ? ?(revision 148549)
> +++ gimple.h ? ?(working copy)
> @@ -997,6 +997,7 @@ extern bool validate_gimple_arglist (con
>
> ?/* In tree-ssa.c ?*/
> ?extern bool tree_ssa_useless_type_conversion (tree);
> +extern tree tree_ssa_strip_useless_type_conversions (tree);
> ?extern bool useless_type_conversion_p (tree, tree);
> ?extern bool types_compatible_p (tree, tree);
>
> Index: tree-ssa.c
> ===================================================================
> --- tree-ssa.c ?(revision 148549)
> +++ tree-ssa.c ?(working copy)
> @@ -1079,6 +1079,18 @@ tree_ssa_useless_type_conversion (tree e
> ? return false;
> ?}
>
> +/* Strip conversions from EXP according to
> + ? tree_ssa_useless_type_conversion and return the resulting
> + ? expression. ?*/
> +
> +tree
> +tree_ssa_strip_useless_type_conversions (tree exp)
> +{
> + ?while (tree_ssa_useless_type_conversion (exp))
> + ? ?exp = TREE_OPERAND (exp, 0);
> + ?return exp;
> +}
> +
>
> ?/* Internal helper for walk_use_def_chains. ?VAR, FN and DATA are as
> ? ?described in walk_use_def_chains.
> Index: tree.c
> ===================================================================
> --- tree.c ? ? ?(revision 148549)
> +++ tree.c ? ? ?(working copy)
> @@ -9460,5 +9460,78 @@ list_equal_p (const_tree t1, const_tree
> ? return !t1 && !t2;
> ?}
>
> +/* Return true iff conversion in EXP generates no instruction. ?Mark
> + ? it inline so that we fully inline into the stripping functions even
> + ? though we have two uses of this function. ?*/
> +
> +static inline bool
> +tree_nop_conversion (const_tree exp)
> +{
> + ?tree outer_type, inner_type;
> +
> + ?if (!CONVERT_EXPR_P (exp)
> + ? ? ?&& TREE_CODE (exp) != NON_LVALUE_EXPR)
> + ? ?return false;
> + ?if (TREE_OPERAND (exp, 0) == error_mark_node)
> + ? ?return false;
> +
> + ?outer_type = TREE_TYPE (exp);
> + ?inner_type = TREE_TYPE (TREE_OPERAND (exp, 0));
> +
> + ?/* Use precision rather then machine mode when we can, which gives
> + ? ? the correct answer even for submode (bit-field) types. ?*/
> + ?if ((INTEGRAL_TYPE_P (outer_type)
> + ? ? ? || POINTER_TYPE_P (outer_type)
> + ? ? ? || TREE_CODE (outer_type) == OFFSET_TYPE)
> + ? ? ?&& (INTEGRAL_TYPE_P (inner_type)
> + ? ? ? ? || POINTER_TYPE_P (inner_type)
> + ? ? ? ? || TREE_CODE (inner_type) == OFFSET_TYPE))
> + ? ?return TYPE_PRECISION (outer_type) == TYPE_PRECISION (inner_type);
> +
> + ?/* Otherwise fall back on comparing machine modes (e.g. for
> + ? ? aggregate types, floats). ?*/
> + ?return TYPE_MODE (outer_type) == TYPE_MODE (inner_type);
> +}
> +
> +/* Return true iff conversion in EXP generates no instruction. ?Don't
> + ? consider conversions changing the signedness. ?*/
> +
> +static bool
> +tree_sign_nop_conversion (const_tree exp)
> +{
> + ?tree outer_type, inner_type;
> +
> + ?if (!tree_nop_conversion (exp))
> + ? ?return false;
> +
> + ?outer_type = TREE_TYPE (exp);
> + ?inner_type = TREE_TYPE (TREE_OPERAND (exp, 0));
> +
> + ?return (TYPE_UNSIGNED (outer_type) == TYPE_UNSIGNED (inner_type)
> + ? ? ? ? && POINTER_TYPE_P (outer_type) == POINTER_TYPE_P (inner_type));
> +}
> +
> +/* Strip conversions from EXP according to tree_nop_conversion and
> + ? return the resulting expression. ?*/
> +
> +tree
> +tree_strip_nop_conversions (tree exp)
> +{
> + ?while (tree_nop_conversion (exp))
> + ? ?exp = TREE_OPERAND (exp, 0);
> + ?return exp;
> +}
> +
> +/* Strip conversions from EXP according to tree_sign_nop_conversion
> + ? and return the resulting expression. ?*/
> +
> +tree
> +tree_strip_sign_nop_conversions (tree exp)
> +{
> + ?while (tree_sign_nop_conversion (exp))
> + ? ?exp = TREE_OPERAND (exp, 0);
> + ?return exp;
> +}
> +
>
> ?#include "gt-tree.h"
> Index: testsuite/gcc.c-torture/execute/bitfld-5.c
> ===================================================================
> --- testsuite/gcc.c-torture/execute/bitfld-5.c ?(revision 0)
> +++ testsuite/gcc.c-torture/execute/bitfld-5.c ?(revision 0)
> @@ -0,0 +1,35 @@
> +/* See http://gcc.gnu.org/ml/gcc/2009-06/msg00072.html. ?*/
> +
> +extern void abort (void);
> +
> +struct s
> +{
> + ?unsigned long long a:2;
> + ?unsigned long long b:40;
> + ?unsigned long long c:22;
> +};
> +
> +__attribute__ ((noinline)) void
> +g (unsigned long long a, unsigned long long b)
> +{
> + ?asm ("");
> + ?if (a != b)
> + ? ?abort ();
> +}
> +
> +__attribute__ ((noinline)) void
> +f (struct s s, unsigned long long b)
> +{
> + ?asm ("");
> + ?g (((unsigned long long) (s.b-8)) + 8, b);
> +}
> +
> +int
> +main ()
> +{
> + ?struct s s = {1, 10, 3};
> + ?struct s t = {1, 2, 3};
> + ?f (s, 10);
> + ?f (t, 0x10000000002);
> + ?return 0;
> +}
>