Message ID | 20211201020941.3019749-1-jason@redhat.com |
---|---|
State | New |
Headers | show |
Series | [RFA,(fold/symtab)] c++: constexpr, fold, weak redecl, fp/0 [PR103310] | expand |
On Wed, Dec 1, 2021 at 3:10 AM Jason Merrill via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > For PR61825, honza changed tree_single_nonzero_warnv_p to prevent a later > declaration from marking a function as weak after we've determined that it > wasn't weak before. But we shouldn't do that for speculative folding; we > should only do it when we actually need a constant value. In C++, such a > context is called "manifestly constant-evaluated". In fold, this seems to > correspond to the folding_initializer flag, since in C this situation only > occurs in static initializers. > > This change makes nonzero-1.c well-formed; I've added a nonzero-1a.c to > verify that we delete the null check eventually if there is no weak > redeclaration. > > The varasm.c change is so that if we do get the weak redeclaration error, we > get it at the position of the weak declaration rather than the previous > declaration. > > Using the FOLD_INIT paths also affects floating point arithmetic: notably, > this makes floating point division by zero in a manifestly > constant-evaluated context constant, as in a C static initializer. I've had > some success convincing CWG that this is the right direction; C++ should > follow C's floating point semantics more than we have been doing, and Joseph > says that the C policy is that Annex F overrides other parts of the standard > that say that some operations are undefined. But since we're in stage 3, > I'm only making this change with the new flag -fconstexpr-fp-except. It may > turn on by default in a future release. > > I think this distinction is only relevant for binary operations; arithmetic > for the floating point case, comparison for possibly non-zero addresses. > > Tested x86_64-pc-linux-gnu, OK for trunk? OK. Thanks, Richard. > PR c++/103310 > > gcc/ChangeLog: > > * fold-const.c (maybe_nonzero_address): Use get_create or get > depending on folding_initializer. > (fold_binary_initializer_loc): New. > * fold-const.h (fold_binary_initializer_loc): Declare. > * varasm.c (mark_weak): Don't use the decl location. > * doc/invoke.texi: Document -fconstexpr-fp-except. > > gcc/c-family/ChangeLog: > > * c.opt: Add -fconstexpr-fp-except. > > gcc/cp/ChangeLog: > > * constexpr.c (cxx_eval_binary_expression): Use > fold_binary_initializer_loc if manifestly cxeval. > > gcc/testsuite/ChangeLog: > > * g++.dg/cpp0x/constexpr-fp-except1.C: New test. > * g++.dg/cpp1z/constexpr-if36.C: New test. > * gcc.dg/tree-ssa/nonzero-1.c: Now well-formed. > * gcc.dg/tree-ssa/nonzero-1a.c: New test. > --- > gcc/doc/invoke.texi | 14 ++++++++++ > gcc/c-family/c.opt | 4 +++ > gcc/fold-const.h | 1 + > gcc/cp/constexpr.c | 9 ++++++- > gcc/fold-const.c | 26 ++++++++++++++++--- > .../g++.dg/cpp0x/constexpr-fp-except1.C | 4 +++ > gcc/testsuite/g++.dg/cpp1z/constexpr-if36.C | 19 ++++++++++++++ > gcc/testsuite/gcc.dg/tree-ssa/nonzero-1.c | 5 ++-- > gcc/testsuite/gcc.dg/tree-ssa/nonzero-1a.c | 11 ++++++++ > gcc/varasm.c | 2 +- > 10 files changed, 88 insertions(+), 7 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-fp-except1.C > create mode 100644 gcc/testsuite/g++.dg/cpp1z/constexpr-if36.C > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/nonzero-1a.c > > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > index 3bddfbaae6a..d6858d834f9 100644 > --- a/gcc/doc/invoke.texi > +++ b/gcc/doc/invoke.texi > @@ -3035,6 +3035,20 @@ users are likely to want to adjust it, but if your code does heavy > constexpr calculations you might want to experiment to find which > value works best for you. > > +@item -fconstexpr-fp-except > +@opindex fconstexpr-fp-except > +Annex F of the C standard specifies that IEC559 floating point > +exceptions encountered at compile time should not stop compilation. > +C++ compilers have historically not followed this guidance, instead > +treating floating point division by zero as non-constant even though > +it has a well defined value. This flag tells the compiler to give > +Annex F priority over other rules saying that a particular operation > +is undefined. > + > +@smallexample > +constexpr float inf = 1./0.; // OK with -fconstexpr-fp-except > +@end smallexample > + > @item -fconstexpr-loop-limit=@var{n} > @opindex fconstexpr-loop-limit > Set the maximum number of iterations for a loop in C++14 constexpr functions > diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt > index 4b8a094b206..5654f044ae4 100644 > --- a/gcc/c-family/c.opt > +++ b/gcc/c-family/c.opt > @@ -1615,6 +1615,10 @@ fconstexpr-cache-depth= > C++ ObjC++ Joined RejectNegative UInteger Var(constexpr_cache_depth) Init(8) > -fconstexpr-cache-depth=<number> Specify maximum constexpr recursion cache depth. > > +fconstexpr-fp-except > +C++ ObjC++ Var(flag_constexpr_fp_except) Init(0) > +Allow IEC559 floating point exceptions in constant expressions > + > fconstexpr-loop-limit= > C++ ObjC++ Joined RejectNegative UInteger Var(constexpr_loop_limit) Init(262144) > -fconstexpr-loop-limit=<number> Specify maximum constexpr loop iteration count. > diff --git a/gcc/fold-const.h b/gcc/fold-const.h > index 56e9d399c0d..01bfe5df9b5 100644 > --- a/gcc/fold-const.h > +++ b/gcc/fold-const.h > @@ -77,6 +77,7 @@ extern tree fold_build_call_array_loc (location_t, tree, tree, int, tree *); > #define fold_build_call_array_initializer(T1,T2,N,T4)\ > fold_build_call_array_initializer_loc (UNKNOWN_LOCATION, T1, T2, N, T4) > extern tree fold_build_call_array_initializer_loc (location_t, tree, tree, int, tree *); > +extern tree fold_binary_initializer_loc (location_t, tree_code, tree, tree, tree); > extern tree get_array_ctor_element_at_index (tree, offset_int, > unsigned * = NULL); > extern bool fold_convertible_p (const_tree, const_tree); > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c > index d66a56583ae..b4b8a96c6af 100644 > --- a/gcc/cp/constexpr.c > +++ b/gcc/cp/constexpr.c > @@ -3366,7 +3366,14 @@ cxx_eval_binary_expression (const constexpr_ctx *ctx, tree t, > } > > if (r == NULL_TREE) > - r = fold_binary_loc (loc, code, type, lhs, rhs); > + { > + if (ctx->manifestly_const_eval > + && (flag_constexpr_fp_except > + || TREE_CODE (type) != REAL_TYPE)) > + r = fold_binary_initializer_loc (loc, code, type, lhs, rhs); > + else > + r = fold_binary_loc (loc, code, type, lhs, rhs); > + } > > if (r == NULL_TREE > && (code == LSHIFT_EXPR || code == RSHIFT_EXPR) > diff --git a/gcc/fold-const.c b/gcc/fold-const.c > index 90d82257ae7..0b9a42f764a 100644 > --- a/gcc/fold-const.c > +++ b/gcc/fold-const.c > @@ -85,8 +85,8 @@ along with GCC; see the file COPYING3. If not see > #include "asan.h" > #include "gimple-range.h" > > -/* Nonzero if we are folding constants inside an initializer; zero > - otherwise. */ > +/* Nonzero if we are folding constants inside an initializer or a C++ > + manifestly-constant-evaluated context; zero otherwise. */ > int folding_initializer = 0; > > /* The following constants represent a bit based encoding of GCC's > @@ -9924,8 +9924,15 @@ pointer_may_wrap_p (tree base, tree offset, poly_int64 bitpos) > static int > maybe_nonzero_address (tree decl) > { > + /* Normally, don't do anything for variables and functions before symtab is > + built; it is quite possible that DECL will be declared weak later. > + But if folding_initializer, we need a constant answer now, so create > + the symtab entry and prevent later weak declaration. */ > if (DECL_P (decl) && decl_in_symtab_p (decl)) > - if (struct symtab_node *symbol = symtab_node::get_create (decl)) > + if (struct symtab_node *symbol > + = (folding_initializer > + ? symtab_node::get_create (decl) > + : symtab_node::get (decl))) > return symbol->nonzero_address (); > > /* Function local objects are never NULL. */ > @@ -13991,6 +13998,19 @@ fold_build_call_array_initializer_loc (location_t loc, tree type, tree fn, > return result; > } > > +tree > +fold_binary_initializer_loc (location_t loc, tree_code code, tree type, > + tree lhs, tree rhs) > +{ > + tree result; > + START_FOLD_INIT; > + > + result = fold_binary_loc (loc, code, type, lhs, rhs); > + > + END_FOLD_INIT; > + return result; > +} > + > #undef START_FOLD_INIT > #undef END_FOLD_INIT > > diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-fp-except1.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-fp-except1.C > new file mode 100644 > index 00000000000..3887a5ba743 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-fp-except1.C > @@ -0,0 +1,4 @@ > +// { dg-do compile { target c++11 } } > +// { dg-additional-options -fconstexpr-fp-except } > + > +constexpr double inf = 1./0.; > diff --git a/gcc/testsuite/g++.dg/cpp1z/constexpr-if36.C b/gcc/testsuite/g++.dg/cpp1z/constexpr-if36.C > new file mode 100644 > index 00000000000..4a1b134cc19 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp1z/constexpr-if36.C > @@ -0,0 +1,19 @@ > +// PR c++/103310 > +// Test that only manifestly-constant-evaluated comparisons lock a symbol's > +// weakness. > + > +// { dg-do compile { target c++17 } } > + > +extern void weakfn1 (void); > +extern void weakfn2 (void); > + > +void call_weakfn (void) > +{ > + if (weakfn1) > + weakfn1 (); > + if constexpr (weakfn2) > + weakfn2 (); > +} > + > +extern void weakfn1 (void) __attribute__((weak)); > +extern void weakfn2 (void) __attribute__((weak)); // { dg-error "declared weak after being used" } > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/nonzero-1.c b/gcc/testsuite/gcc.dg/tree-ssa/nonzero-1.c > index c9d438e9374..aa21b71c730 100644 > --- a/gcc/testsuite/gcc.dg/tree-ssa/nonzero-1.c > +++ b/gcc/testsuite/gcc.dg/tree-ssa/nonzero-1.c > @@ -1,12 +1,13 @@ > /* { dg-do compile } */ > -/* { dg-options "-O2 -fdelete-null-pointer-checks" } */ > +/* { dg-options "-O2 -fdelete-null-pointer-checks -fdump-tree-optimized" } */ > /* { dg-require-weak "" } */ > > /* { dg-skip-if "" keeps_null_pointer_checks } */ > -extern int a; /* { dg-error "declared weak after being used" } */ > +extern int a; > int > t() > { > + /* { dg-final { scan-tree-dump "&a != 0" "optimized" } } */ > return &a!=0; > } > extern int a __attribute__ ((weak)); > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/nonzero-1a.c b/gcc/testsuite/gcc.dg/tree-ssa/nonzero-1a.c > new file mode 100644 > index 00000000000..3d1eb97f151 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/nonzero-1a.c > @@ -0,0 +1,11 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fdelete-null-pointer-checks -fdump-tree-optimized" } */ > + > +/* { dg-skip-if "" keeps_null_pointer_checks } */ > +extern int a; > +int > +t() > +{ > + /* { dg-final { scan-tree-dump-not "&a != 0" "optimized" } } */ > + return &a!=0; > +} > diff --git a/gcc/varasm.c b/gcc/varasm.c > index d6031d6f1a2..9315e2c6936 100644 > --- a/gcc/varasm.c > +++ b/gcc/varasm.c > @@ -5869,7 +5869,7 @@ mark_weak (tree decl) > > struct symtab_node *n = symtab_node::get (decl); > if (n && n->refuse_visibility_changes) > - error ("%+qD declared weak after being used", decl); > + error ("%qD declared weak after being used", decl); > DECL_WEAK (decl) = 1; > > if (DECL_RTL_SET_P (decl) > > base-commit: a3e75c1491cd2d501081210925a89a65b1c1e5e5 > -- > 2.27.0 >
On Tue, Nov 30, 2021 at 09:09:41PM -0500, Jason Merrill via Gcc-patches wrote: > For PR61825, honza changed tree_single_nonzero_warnv_p to prevent a later > declaration from marking a function as weak after we've determined that it > wasn't weak before. But we shouldn't do that for speculative folding; we > should only do it when we actually need a constant value. In C++, such a > context is called "manifestly constant-evaluated". In fold, this seems to > correspond to the folding_initializer flag, since in C this situation only > occurs in static initializers. > > This change makes nonzero-1.c well-formed; I've added a nonzero-1a.c to > verify that we delete the null check eventually if there is no weak > redeclaration. > > The varasm.c change is so that if we do get the weak redeclaration error, we > get it at the position of the weak declaration rather than the previous > declaration. > > Using the FOLD_INIT paths also affects floating point arithmetic: notably, > this makes floating point division by zero in a manifestly > constant-evaluated context constant, as in a C static initializer. I've had > some success convincing CWG that this is the right direction; C++ should > follow C's floating point semantics more than we have been doing, and Joseph > says that the C policy is that Annex F overrides other parts of the standard > that say that some operations are undefined. But since we're in stage 3, > I'm only making this change with the new flag -fconstexpr-fp-except. It may > turn on by default in a future release. > > I think this distinction is only relevant for binary operations; arithmetic > for the floating point case, comparison for possibly non-zero addresses. > > Tested x86_64-pc-linux-gnu, OK for trunk? [...] > diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt > index 4b8a094b206..5654f044ae4 100644 > --- a/gcc/c-family/c.opt > +++ b/gcc/c-family/c.opt > @@ -1615,6 +1615,10 @@ fconstexpr-cache-depth= > C++ ObjC++ Joined RejectNegative UInteger Var(constexpr_cache_depth) Init(8) > -fconstexpr-cache-depth=<number> Specify maximum constexpr recursion cache depth. > > +fconstexpr-fp-except > +C++ ObjC++ Var(flag_constexpr_fp_except) Init(0) > +Allow IEC559 floating point exceptions in constant expressions Sorry about nitpicking but there's a missing period after 'expressions', which will break a test. Marek
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 3bddfbaae6a..d6858d834f9 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -3035,6 +3035,20 @@ users are likely to want to adjust it, but if your code does heavy constexpr calculations you might want to experiment to find which value works best for you. +@item -fconstexpr-fp-except +@opindex fconstexpr-fp-except +Annex F of the C standard specifies that IEC559 floating point +exceptions encountered at compile time should not stop compilation. +C++ compilers have historically not followed this guidance, instead +treating floating point division by zero as non-constant even though +it has a well defined value. This flag tells the compiler to give +Annex F priority over other rules saying that a particular operation +is undefined. + +@smallexample +constexpr float inf = 1./0.; // OK with -fconstexpr-fp-except +@end smallexample + @item -fconstexpr-loop-limit=@var{n} @opindex fconstexpr-loop-limit Set the maximum number of iterations for a loop in C++14 constexpr functions diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index 4b8a094b206..5654f044ae4 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -1615,6 +1615,10 @@ fconstexpr-cache-depth= C++ ObjC++ Joined RejectNegative UInteger Var(constexpr_cache_depth) Init(8) -fconstexpr-cache-depth=<number> Specify maximum constexpr recursion cache depth. +fconstexpr-fp-except +C++ ObjC++ Var(flag_constexpr_fp_except) Init(0) +Allow IEC559 floating point exceptions in constant expressions + fconstexpr-loop-limit= C++ ObjC++ Joined RejectNegative UInteger Var(constexpr_loop_limit) Init(262144) -fconstexpr-loop-limit=<number> Specify maximum constexpr loop iteration count. diff --git a/gcc/fold-const.h b/gcc/fold-const.h index 56e9d399c0d..01bfe5df9b5 100644 --- a/gcc/fold-const.h +++ b/gcc/fold-const.h @@ -77,6 +77,7 @@ extern tree fold_build_call_array_loc (location_t, tree, tree, int, tree *); #define fold_build_call_array_initializer(T1,T2,N,T4)\ fold_build_call_array_initializer_loc (UNKNOWN_LOCATION, T1, T2, N, T4) extern tree fold_build_call_array_initializer_loc (location_t, tree, tree, int, tree *); +extern tree fold_binary_initializer_loc (location_t, tree_code, tree, tree, tree); extern tree get_array_ctor_element_at_index (tree, offset_int, unsigned * = NULL); extern bool fold_convertible_p (const_tree, const_tree); diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c index d66a56583ae..b4b8a96c6af 100644 --- a/gcc/cp/constexpr.c +++ b/gcc/cp/constexpr.c @@ -3366,7 +3366,14 @@ cxx_eval_binary_expression (const constexpr_ctx *ctx, tree t, } if (r == NULL_TREE) - r = fold_binary_loc (loc, code, type, lhs, rhs); + { + if (ctx->manifestly_const_eval + && (flag_constexpr_fp_except + || TREE_CODE (type) != REAL_TYPE)) + r = fold_binary_initializer_loc (loc, code, type, lhs, rhs); + else + r = fold_binary_loc (loc, code, type, lhs, rhs); + } if (r == NULL_TREE && (code == LSHIFT_EXPR || code == RSHIFT_EXPR) diff --git a/gcc/fold-const.c b/gcc/fold-const.c index 90d82257ae7..0b9a42f764a 100644 --- a/gcc/fold-const.c +++ b/gcc/fold-const.c @@ -85,8 +85,8 @@ along with GCC; see the file COPYING3. If not see #include "asan.h" #include "gimple-range.h" -/* Nonzero if we are folding constants inside an initializer; zero - otherwise. */ +/* Nonzero if we are folding constants inside an initializer or a C++ + manifestly-constant-evaluated context; zero otherwise. */ int folding_initializer = 0; /* The following constants represent a bit based encoding of GCC's @@ -9924,8 +9924,15 @@ pointer_may_wrap_p (tree base, tree offset, poly_int64 bitpos) static int maybe_nonzero_address (tree decl) { + /* Normally, don't do anything for variables and functions before symtab is + built; it is quite possible that DECL will be declared weak later. + But if folding_initializer, we need a constant answer now, so create + the symtab entry and prevent later weak declaration. */ if (DECL_P (decl) && decl_in_symtab_p (decl)) - if (struct symtab_node *symbol = symtab_node::get_create (decl)) + if (struct symtab_node *symbol + = (folding_initializer + ? symtab_node::get_create (decl) + : symtab_node::get (decl))) return symbol->nonzero_address (); /* Function local objects are never NULL. */ @@ -13991,6 +13998,19 @@ fold_build_call_array_initializer_loc (location_t loc, tree type, tree fn, return result; } +tree +fold_binary_initializer_loc (location_t loc, tree_code code, tree type, + tree lhs, tree rhs) +{ + tree result; + START_FOLD_INIT; + + result = fold_binary_loc (loc, code, type, lhs, rhs); + + END_FOLD_INIT; + return result; +} + #undef START_FOLD_INIT #undef END_FOLD_INIT diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-fp-except1.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-fp-except1.C new file mode 100644 index 00000000000..3887a5ba743 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-fp-except1.C @@ -0,0 +1,4 @@ +// { dg-do compile { target c++11 } } +// { dg-additional-options -fconstexpr-fp-except } + +constexpr double inf = 1./0.; diff --git a/gcc/testsuite/g++.dg/cpp1z/constexpr-if36.C b/gcc/testsuite/g++.dg/cpp1z/constexpr-if36.C new file mode 100644 index 00000000000..4a1b134cc19 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1z/constexpr-if36.C @@ -0,0 +1,19 @@ +// PR c++/103310 +// Test that only manifestly-constant-evaluated comparisons lock a symbol's +// weakness. + +// { dg-do compile { target c++17 } } + +extern void weakfn1 (void); +extern void weakfn2 (void); + +void call_weakfn (void) +{ + if (weakfn1) + weakfn1 (); + if constexpr (weakfn2) + weakfn2 (); +} + +extern void weakfn1 (void) __attribute__((weak)); +extern void weakfn2 (void) __attribute__((weak)); // { dg-error "declared weak after being used" } diff --git a/gcc/testsuite/gcc.dg/tree-ssa/nonzero-1.c b/gcc/testsuite/gcc.dg/tree-ssa/nonzero-1.c index c9d438e9374..aa21b71c730 100644 --- a/gcc/testsuite/gcc.dg/tree-ssa/nonzero-1.c +++ b/gcc/testsuite/gcc.dg/tree-ssa/nonzero-1.c @@ -1,12 +1,13 @@ /* { dg-do compile } */ -/* { dg-options "-O2 -fdelete-null-pointer-checks" } */ +/* { dg-options "-O2 -fdelete-null-pointer-checks -fdump-tree-optimized" } */ /* { dg-require-weak "" } */ /* { dg-skip-if "" keeps_null_pointer_checks } */ -extern int a; /* { dg-error "declared weak after being used" } */ +extern int a; int t() { + /* { dg-final { scan-tree-dump "&a != 0" "optimized" } } */ return &a!=0; } extern int a __attribute__ ((weak)); diff --git a/gcc/testsuite/gcc.dg/tree-ssa/nonzero-1a.c b/gcc/testsuite/gcc.dg/tree-ssa/nonzero-1a.c new file mode 100644 index 00000000000..3d1eb97f151 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/nonzero-1a.c @@ -0,0 +1,11 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdelete-null-pointer-checks -fdump-tree-optimized" } */ + +/* { dg-skip-if "" keeps_null_pointer_checks } */ +extern int a; +int +t() +{ + /* { dg-final { scan-tree-dump-not "&a != 0" "optimized" } } */ + return &a!=0; +} diff --git a/gcc/varasm.c b/gcc/varasm.c index d6031d6f1a2..9315e2c6936 100644 --- a/gcc/varasm.c +++ b/gcc/varasm.c @@ -5869,7 +5869,7 @@ mark_weak (tree decl) struct symtab_node *n = symtab_node::get (decl); if (n && n->refuse_visibility_changes) - error ("%+qD declared weak after being used", decl); + error ("%qD declared weak after being used", decl); DECL_WEAK (decl) = 1; if (DECL_RTL_SET_P (decl)