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]

Re: [PATCH, PR 46349] One more SRA test for aggregate bit-fields


On Wed, Nov 10, 2010 at 1:22 PM, Martin Jambor <mjambor@suse.cz> wrote:
> Hi,
>
> the following is necessary to fix PR 46349. ?It basically turns off a
> piece of code which is there to try to get rid of VIEW_CONVERT_EXPRs
> when there are aggregate bit-fields involved in the assignment.
>
> I have gone through all other calls to build_ref_for_offset and its
> callers and came to the conclusion that in most cases, tests for
> contains_view_convert_expr_p should be accompanied with a test for a
> bit-field component_ref because there might be assignments in between
> a candidate and a union with aggregate bit-fields which might cause
> the same problems and should therefore be detected. ?So I added that
> to the patch, even though it is not exactly pretty. ?(No, I do not
> have any testcases.)
>
> Bootstrapped and tested on x86_64-linux on top of
> http://gcc.gnu.org/ml/gcc-patches/2010-11/msg00933.html without any
> regressions (the peculiar acats one is gone too). ?OK for trunk?

Ok.

Thanks,
Richard.

> Thanks,
>
> Martin
>
>
> 2010-11-09 ?Martin Jambor ?<mjambor@suse.cz>
>
> ? ? ? ?PR tree-optimization/46349
> ? ? ? ?* tree-sra.c (contains_bitfld_comp_ref_p): New function.
> ? ? ? ?(contains_vce_or_bfcref_p): Likewise.
> ? ? ? ?(sra_modify_assign): Use them.
>
> ? ? ? ?* testsuite/gnat.dg/opt9.adb: New file.
> ? ? ? ?* testsuite/gnat.dg/opt9_pkg.ads: Likewise
>
>
> Index: mine/gcc/tree-sra.c
> ===================================================================
> --- mine.orig/gcc/tree-sra.c
> +++ mine/gcc/tree-sra.c
> @@ -2629,6 +2629,41 @@ get_repl_default_def_ssa_name (struct ac
> ? return repl;
> ?}
>
> +/* Return true if REF has a COMPONENT_REF with a bit-field field declaration
> + ? somewhere in it. ?*/
> +
> +static inline bool
> +contains_bitfld_comp_ref_p (const_tree ref)
> +{
> + ?while (handled_component_p (ref))
> + ? ?{
> + ? ? ?if (TREE_CODE (ref) == COMPONENT_REF
> + ? ? ? ? ?&& DECL_BIT_FIELD (TREE_OPERAND (ref, 1)))
> + ? ? ? ?return true;
> + ? ? ?ref = TREE_OPERAND (ref, 0);
> + ? ?}
> +
> + ?return false;
> +}
> +
> +/* Return true if REF has an VIEW_CONVERT_EXPR or a COMPONENT_REF with a
> + ? bit-field field declaration somewhere in it. ?*/
> +
> +static inline bool
> +contains_vce_or_bfcref_p (const_tree ref)
> +{
> + ?while (handled_component_p (ref))
> + ? ?{
> + ? ? ?if (TREE_CODE (ref) == VIEW_CONVERT_EXPR
> + ? ? ? ? || (TREE_CODE (ref) == COMPONENT_REF
> + ? ? ? ? ? ? && DECL_BIT_FIELD (TREE_OPERAND (ref, 1))))
> + ? ? ? return true;
> + ? ? ?ref = TREE_OPERAND (ref, 0);
> + ? ?}
> +
> + ?return false;
> +}
> +
> ?/* Examine both sides of the assignment statement pointed to by STMT, replace
> ? ?them with a scalare replacement if there is one and generate copying of
> ? ?replacements if scalarized aggregates have been used in the assignment. ?GSI
> @@ -2697,6 +2732,7 @@ sra_modify_assign (gimple *stmt, gimple_
> ? ? ? ? ? ? ??? ?This should move to fold_stmt which we simply should
> ? ? ? ? ? ? call after building a VIEW_CONVERT_EXPR here. ?*/
> ? ? ? ? ?if (AGGREGATE_TYPE_P (TREE_TYPE (lhs))
> + ? ? ? ? ? ? && !contains_bitfld_comp_ref_p (lhs)
> ? ? ? ? ? ? ?&& !access_has_children_p (lacc))
> ? ? ? ? ? ?{
> ? ? ? ? ? ? ?lhs = build_ref_for_offset (loc, lhs, 0, TREE_TYPE (rhs),
> @@ -2704,7 +2740,7 @@ sra_modify_assign (gimple *stmt, gimple_
> ? ? ? ? ? ? ?gimple_assign_set_lhs (*stmt, lhs);
> ? ? ? ? ? ?}
> ? ? ? ? ?else if (AGGREGATE_TYPE_P (TREE_TYPE (rhs))
> - ? ? ? ? ? ? ? ? ?&& !contains_view_convert_expr_p (rhs)
> + ? ? ? ? ? ? ? ? ?&& !contains_vce_or_bfcref_p (rhs)
> ? ? ? ? ? ? ? ? ? && !access_has_children_p (racc))
> ? ? ? ? ? ?rhs = build_ref_for_offset (loc, rhs, 0, TREE_TYPE (lhs),
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?gsi, false);
> @@ -2754,8 +2790,8 @@ sra_modify_assign (gimple *stmt, gimple_
> ? ? ?This is what the first branch does. ?*/
>
> ? if (gimple_has_volatile_ops (*stmt)
> - ? ? ?|| contains_view_convert_expr_p (rhs)
> - ? ? ?|| contains_view_convert_expr_p (lhs))
> + ? ? ?|| contains_vce_or_bfcref_p (rhs)
> + ? ? ?|| contains_vce_or_bfcref_p (lhs))
> ? ? {
> ? ? ? if (access_has_children_p (racc))
> ? ? ? ?generate_subtree_copies (racc->first_child, racc->base, 0, 0, 0,
> Index: mine/gcc/testsuite/gnat.dg/opt9.adb
> ===================================================================
> --- /dev/null
> +++ mine/gcc/testsuite/gnat.dg/opt9.adb
> @@ -0,0 +1,26 @@
> +-- { dg-do compile }
> +-- { dg-options "-gnatws -O" }
> +
> +with Opt9_Pkg; use Opt9_Pkg;
> +
> +procedure Opt9 is
> +
> + ? type Array_T is array (1 .. N) of Integer;
> +
> + ? type Clock_T is record
> + ? ? ?N_Ticks : Integer := 0;
> + ? end record;
> +
> + ? type Rec is record
> + ? ? ?Values : Array_T;
> + ? ? ?Valid ?: Boolean;
> + ? ? ?Tstamp : Clock_T;
> + ? end record;
> +
> + ? pragma Pack (Rec);
> +
> + ? Data : Rec;
> +
> +begin
> + ? null;
> +end;
> Index: mine/gcc/testsuite/gnat.dg/opt9_pkg.ads
> ===================================================================
> --- /dev/null
> +++ mine/gcc/testsuite/gnat.dg/opt9_pkg.ads
> @@ -0,0 +1,5 @@
> +package Opt9_Pkg is
> +
> + ?N : integer := 15;
> +
> +end Opt9_Pkg;
>


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