This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH] Fix PR 54494, removal of volatile store in strlen optimization because of the inlininer
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Andrew Pinski <pinskia at gmail dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, Jan Hubicka <jh at suse dot cz>
- Date: Thu, 6 Sep 2012 09:07:57 +0200
- Subject: Re: [PATCH] Fix PR 54494, removal of volatile store in strlen optimization because of the inlininer
- References: <CA+=Sn1=Tw68CwdNrNSwUo57hme7M6wEVf9fZq0B-4Egunmu-Ww@mail.gmail.com>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
On Wed, Sep 05, 2012 at 09:10:53PM -0700, Andrew Pinski wrote:
> The inlininer likes to recreate some MEM_REF, it copies most of the
> bits (TREE_THIS_NOTRAP, TREE_THIS_VOLATILE, etc.) but forgets about
> TREE_SIDE_EFFECTS. This causes the strlen optimization to think the
> memory store does not have a side effects.
> OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
> Andrew Pinski
> * tree-inline.c (remap_gimple_op_r): Copy TREE_SIDE_EFFECTS also.
> * gcc.dg/tree-ssa/strlen-1.c: New testcase.
Patch preapproved, but you've attached a different patch.
I'd say copy_tree_body_r's MEM_REF handling should also copy
TREE_SIDE_EFFECTS/TREE_THIS_NOTRAP (what about TREE_READONLY?),
maybe copy_decl_to_var/copy_result_decl_to_var should also
copy TREE_SIDE_EFFECTS, perhaps omp-low.c copy_var_decl, install_var_field,
tree-nested.c lookup_field_for_decl, tree-sra.c sra_ipa_reset_debug_stmts
(grep TREE_THIS_VOLATILE.*TREE_THIS_VOLATILE, looking for missing
TREE_SIDE_EFFECTS copy nearby). That said, perhaps the tree-ssa-strlen.c
change is desirable too, unless we add some checking that TREE_THIS_VOLATILE
references/decls have TREE_SIDE_EFFECTS bit set in the IL.
> --- testsuite/gcc.c-torture/compile/pr49474.c (revision 0)
> +++ testsuite/gcc.c-torture/compile/pr49474.c (revision 0)
> --- cprop.c (revision 176187)
> +++ cprop.c (working copy)