This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix PR 54494, removal of volatile store in strlen optimization
- 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>
- Date: Wed, 5 Sep 2012 23:36:20 +0200
- Subject: Re: [PATCH] Fix PR 54494, removal of volatile store in strlen optimization
- References: <CA+=Sn1kDzzsBvndz64W=VTputQivU5QmUW67r-=0p-g2310P6A@mail.gmail.com>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
On Wed, Sep 05, 2012 at 02:10:03PM -0700, Andrew Pinski wrote:
> The problem here is the strlen optimization tries to remove a null
> character store as we already have done it but it does it for a
> volatile store which is not a valid thing to do.
> This patch fixes the problem by ignoring statements which have
> volatile operands.
That should be caught by the !TREE_SIDE_EFFECTS (lhs) check a few lines
later. Isn't the bug instead that remap_gimple_op_r copies over
TREE_THIS_VOLATILE flag, but doesn't copy over TREE_SIDE_EFFECTS?
> * tree-ssa-strlen.c (strlen_optimize_stmt): Don't look at statements
> which have volatile operands.
>
> testsuite/ChangeLog:
> * gcc.dg/tree-ssa/strlen-1.c: New testcase.
> Index: gcc/tree-ssa-strlen.c
> ===================================================================
> --- gcc/tree-ssa-strlen.c (revision 190993)
> +++ gcc/tree-ssa-strlen.c (working copy)
> @@ -1782,7 +1782,8 @@ strlen_optimize_stmt (gimple_stmt_iterat
> break;
> }
> }
> - else if (is_gimple_assign (stmt))
> + else if (is_gimple_assign (stmt)
> + && !gimple_has_volatile_ops (stmt))
> {
> tree lhs = gimple_assign_lhs (stmt);
>
> Index: gcc/testsuite/gcc.dg/tree-ssa/strlen-1.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/strlen-1.c (revision 0)
> +++ gcc/testsuite/gcc.dg/tree-ssa/strlen-1.c (revision 0)
> @@ -0,0 +1,17 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-optimized" } */
> +extern const unsigned long base;
> +static inline void wreg(unsigned char val, unsigned long addr) __attribute__((always_inline));
> +static inline void wreg(unsigned char val, unsigned long addr)
> +{
> + *((volatile unsigned char *) (__SIZE_TYPE__) (base + addr)) = val;
> +}
> +void wreg_twice(void)
> +{
> + wreg(0, 42);
> + wreg(0, 42);
> +}
> +
> +/* We should not remove the second null character store to (base+42) address. */
> +/* { dg-final { scan-tree-dump-times " ={v} 0;" 2 "optimized" } } */
> +/* { dg-final { cleanup-tree-dump "optimized" } } */
Jakub