Message ID | ZcdF270qXqRGFzCz@tucnak |
---|---|
State | New |
Headers | show |
Series | gimple-low: Fix up handling of volatile automatic vars in assume attribute [PR110754] | expand |
> Am 10.02.2024 um 10:46 schrieb Jakub Jelinek <jakub@redhat.com>: > > Hi! > > As the following testcases show, the gimple-low outlining of assume > magic functions handled volatile automatic vars (including > parameters/results) like non-volatile ones except it copied volatile > to the new PARM_DECL, which has the undesirable effect that a load > from the volatile var is passed to IFN_ASSUME and so there is a > side-effect there even when side-effects of the assume attribute > shouldn't be evaluated. > > The following patch fixes that by passing address of the volatile > variables/parameters/results instead and doing loads or stores from it > or to it where it was originally accessed in the assume attribute > expression. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > I guess we want to backport it to 13 eventually too. Ok for trunk and branch after a while. Richard > 2024-02-10 Jakub Jelinek <jakub@redhat.com> > > PR middle-end/110754 > * gimple-low.cc (assumption_copy_decl): For TREE_THIS_VOLATILE > decls create PARM_DECL with pointer to original type, set > TREE_READONLY and keep TREE_THIS_VOLATILE, TREE_ADDRESSABLE, > DECL_NOT_GIMPLE_REG_P and DECL_BY_REFERENCE cleared. > (adjust_assumption_stmt_op): For remapped TREE_THIS_VOLATILE decls > wrap PARM_DECL into a simple TREE_THIS_NO_TRAP MEM_REF. > (lower_assumption): For TREE_THIS_VOLATILE vars pass ADDR_EXPR > of the var as argument. > > * gcc.dg/attr-assume-6.c: New test. > * g++.dg/cpp23/attr-assume12.C: New test. > > --- gcc/gimple-low.cc.jj 2024-02-01 09:16:51.265388345 +0100 > +++ gcc/gimple-low.cc 2024-02-09 15:29:16.003289765 +0100 > @@ -374,15 +374,22 @@ assumption_copy_decl (tree decl, copy_bo > gcc_assert (VAR_P (decl) > || TREE_CODE (decl) == PARM_DECL > || TREE_CODE (decl) == RESULT_DECL); > + if (TREE_THIS_VOLATILE (decl)) > + type = build_pointer_type (type); > tree copy = build_decl (DECL_SOURCE_LOCATION (decl), > PARM_DECL, DECL_NAME (decl), type); > if (DECL_PT_UID_SET_P (decl)) > SET_DECL_PT_UID (copy, DECL_PT_UID (decl)); > - TREE_ADDRESSABLE (copy) = TREE_ADDRESSABLE (decl); > - TREE_READONLY (copy) = TREE_READONLY (decl); > - TREE_THIS_VOLATILE (copy) = TREE_THIS_VOLATILE (decl); > - DECL_NOT_GIMPLE_REG_P (copy) = DECL_NOT_GIMPLE_REG_P (decl); > - DECL_BY_REFERENCE (copy) = DECL_BY_REFERENCE (decl); > + TREE_THIS_VOLATILE (copy) = 0; > + if (TREE_THIS_VOLATILE (decl)) > + TREE_READONLY (copy) = 1; > + else > + { > + TREE_ADDRESSABLE (copy) = TREE_ADDRESSABLE (decl); > + TREE_READONLY (copy) = TREE_READONLY (decl); > + DECL_NOT_GIMPLE_REG_P (copy) = DECL_NOT_GIMPLE_REG_P (decl); > + DECL_BY_REFERENCE (copy) = DECL_BY_REFERENCE (decl); > + } > DECL_ARG_TYPE (copy) = type; > ((lower_assumption_data *) id)->decls.safe_push (decl); > return copy_decl_for_dup_finish (id, decl, copy); > @@ -466,6 +473,11 @@ adjust_assumption_stmt_op (tree *tp, int > case PARM_DECL: > case RESULT_DECL: > *tp = remap_decl (t, &data->id); > + if (TREE_THIS_VOLATILE (t) && *tp != t) > + { > + *tp = build_simple_mem_ref (*tp); > + TREE_THIS_NOTRAP (*tp) = 1; > + } > break; > default: > break; > @@ -600,6 +612,11 @@ lower_assumption (gimple_stmt_iterator * > /* Remaining arguments will be the variables/parameters > mentioned in the condition. */ > vargs[i - sz] = lad.decls[i - 1]; > + if (TREE_THIS_VOLATILE (lad.decls[i - 1])) > + { > + TREE_ADDRESSABLE (lad.decls[i - 1]) = 1; > + vargs[i - sz] = build_fold_addr_expr (lad.decls[i - 1]); > + } > /* If they have gimple types, we might need to regimplify > them to make the IFN_ASSUME call valid. */ > if (is_gimple_reg_type (TREE_TYPE (vargs[i - sz])) > --- gcc/testsuite/gcc.dg/attr-assume-6.c.jj 2024-02-09 15:32:49.384325442 +0100 > +++ gcc/testsuite/gcc.dg/attr-assume-6.c 2024-02-09 15:48:27.612288853 +0100 > @@ -0,0 +1,14 @@ > +/* PR middle-end/110754 */ > +/* { dg-do compile } */ > +/* { dg-options "-std=c23 -O2 -fdump-tree-optimized" } */ > +/* { dg-final { scan-tree-dump-times "a ={v} x" 1 "optimized" } } */ > +/* { dg-final { scan-tree-dump-not "={v} a" "optimized" } } */ > +/* { dg-final { scan-tree-dump-times "return 0;" 1 "optimized" } } */ > + > +int > +foo (int x) > +{ > + volatile int a = x; > + [[gnu::assume (x == (a & 0))]]; > + return x; > +} > --- gcc/testsuite/g++.dg/cpp23/attr-assume12.C.jj 2024-02-09 15:51:21.100878070 +0100 > +++ gcc/testsuite/g++.dg/cpp23/attr-assume12.C 2024-02-09 15:51:51.863450599 +0100 > @@ -0,0 +1,14 @@ > +// PR middle-end/110754 > +// { dg-do compile { target c++11 } } > +// { dg-options "-O2 -fdump-tree-optimized" } > +// { dg-final { scan-tree-dump-times "a ={v} x" 1 "optimized" } } > +// { dg-final { scan-tree-dump-not "={v} a" "optimized" } } > +// { dg-final { scan-tree-dump-times "return 0;" 1 "optimized" } } > + > +int > +foo (int x) > +{ > + volatile int a = x; > + [[gnu::assume (x == (a & 0))]]; > + return x; > +} > > Jakub >
--- gcc/gimple-low.cc.jj 2024-02-01 09:16:51.265388345 +0100 +++ gcc/gimple-low.cc 2024-02-09 15:29:16.003289765 +0100 @@ -374,15 +374,22 @@ assumption_copy_decl (tree decl, copy_bo gcc_assert (VAR_P (decl) || TREE_CODE (decl) == PARM_DECL || TREE_CODE (decl) == RESULT_DECL); + if (TREE_THIS_VOLATILE (decl)) + type = build_pointer_type (type); tree copy = build_decl (DECL_SOURCE_LOCATION (decl), PARM_DECL, DECL_NAME (decl), type); if (DECL_PT_UID_SET_P (decl)) SET_DECL_PT_UID (copy, DECL_PT_UID (decl)); - TREE_ADDRESSABLE (copy) = TREE_ADDRESSABLE (decl); - TREE_READONLY (copy) = TREE_READONLY (decl); - TREE_THIS_VOLATILE (copy) = TREE_THIS_VOLATILE (decl); - DECL_NOT_GIMPLE_REG_P (copy) = DECL_NOT_GIMPLE_REG_P (decl); - DECL_BY_REFERENCE (copy) = DECL_BY_REFERENCE (decl); + TREE_THIS_VOLATILE (copy) = 0; + if (TREE_THIS_VOLATILE (decl)) + TREE_READONLY (copy) = 1; + else + { + TREE_ADDRESSABLE (copy) = TREE_ADDRESSABLE (decl); + TREE_READONLY (copy) = TREE_READONLY (decl); + DECL_NOT_GIMPLE_REG_P (copy) = DECL_NOT_GIMPLE_REG_P (decl); + DECL_BY_REFERENCE (copy) = DECL_BY_REFERENCE (decl); + } DECL_ARG_TYPE (copy) = type; ((lower_assumption_data *) id)->decls.safe_push (decl); return copy_decl_for_dup_finish (id, decl, copy); @@ -466,6 +473,11 @@ adjust_assumption_stmt_op (tree *tp, int case PARM_DECL: case RESULT_DECL: *tp = remap_decl (t, &data->id); + if (TREE_THIS_VOLATILE (t) && *tp != t) + { + *tp = build_simple_mem_ref (*tp); + TREE_THIS_NOTRAP (*tp) = 1; + } break; default: break; @@ -600,6 +612,11 @@ lower_assumption (gimple_stmt_iterator * /* Remaining arguments will be the variables/parameters mentioned in the condition. */ vargs[i - sz] = lad.decls[i - 1]; + if (TREE_THIS_VOLATILE (lad.decls[i - 1])) + { + TREE_ADDRESSABLE (lad.decls[i - 1]) = 1; + vargs[i - sz] = build_fold_addr_expr (lad.decls[i - 1]); + } /* If they have gimple types, we might need to regimplify them to make the IFN_ASSUME call valid. */ if (is_gimple_reg_type (TREE_TYPE (vargs[i - sz])) --- gcc/testsuite/gcc.dg/attr-assume-6.c.jj 2024-02-09 15:32:49.384325442 +0100 +++ gcc/testsuite/gcc.dg/attr-assume-6.c 2024-02-09 15:48:27.612288853 +0100 @@ -0,0 +1,14 @@ +/* PR middle-end/110754 */ +/* { dg-do compile } */ +/* { dg-options "-std=c23 -O2 -fdump-tree-optimized" } */ +/* { dg-final { scan-tree-dump-times "a ={v} x" 1 "optimized" } } */ +/* { dg-final { scan-tree-dump-not "={v} a" "optimized" } } */ +/* { dg-final { scan-tree-dump-times "return 0;" 1 "optimized" } } */ + +int +foo (int x) +{ + volatile int a = x; + [[gnu::assume (x == (a & 0))]]; + return x; +} --- gcc/testsuite/g++.dg/cpp23/attr-assume12.C.jj 2024-02-09 15:51:21.100878070 +0100 +++ gcc/testsuite/g++.dg/cpp23/attr-assume12.C 2024-02-09 15:51:51.863450599 +0100 @@ -0,0 +1,14 @@ +// PR middle-end/110754 +// { dg-do compile { target c++11 } } +// { dg-options "-O2 -fdump-tree-optimized" } +// { dg-final { scan-tree-dump-times "a ={v} x" 1 "optimized" } } +// { dg-final { scan-tree-dump-not "={v} a" "optimized" } } +// { dg-final { scan-tree-dump-times "return 0;" 1 "optimized" } } + +int +foo (int x) +{ + volatile int a = x; + [[gnu::assume (x == (a & 0))]]; + return x; +}