Re: [PATCH] [i386] Fix ICE in pass_rpad.
On Sat, Sep 18, 2021 at 11:09:32AM +0800, liuhongt wrote:
> Besides conversion instructions, pass_rpad also handles scalar
> sqrt/rsqrt/rcp/round instructions, while r12-3614 should only want to
> handle conversion instructions, so fix it.
>
> Bootstrapped and regtest on x86_64-linux-gnu{-m32,} w/ configure
> --enable-checking=yes,rtl,extra, failed tests are fixed.
> Ok for trunk?
>
> New tests that PASS (8 tests):
>
> gcc.target/i386/avx512f-vscalefpd-2.c execution test
> gcc.target/i386/avx512f-vscalefps-2.c execution test
> gcc.target/i386/avx512f-vscalefss-2.c execution test
> gcc.target/i386/avx512fp16-vscalefph-1b.c execution test
> gcc.target/i386/avx512fp16-vscalefsh-1b.c execution test
> gcc.target/i386/avx512fp16vl-vscalefph-1b.c execution test
> gcc.target/i386/avx512vl-vscalefpd-2.c execution test
> gcc.target/i386/avx512vl-vscalefps-2.c execution test
>
> Old tests that failed, that have disappeared (8 tests): (Eeek!)
>
> gcc.target/i386/avx512f-vscalefpd-2.c (internal compiler error)
> gcc.target/i386/avx512f-vscalefps-2.c (internal compiler error)
> gcc.target/i386/avx512f-vscalefss-2.c (internal compiler error)
> gcc.target/i386/avx512fp16-vscalefph-1b.c (internal compiler error)
> gcc.target/i386/avx512fp16-vscalefsh-1b.c (internal compiler error)
> gcc.target/i386/avx512fp16vl-vscalefph-1b.c (internal compiler error)
> gcc.target/i386/avx512vl-vscalefpd-2.c (internal compiler error)
> gcc.target/i386/avx512vl-vscalefps-2.c (internal compiler error)
>
> gcc/ChangeLog:
>
> * config/i386/i386-features.c (remove_partial_avx_dependency):
> Restrict TARGET_USE_VECTOR_FP_CONVERTS and
> TARGET_USE_VECTOR_CONVERTS to conversion instructions only.
Ok, with a minor nit:
> @@ -2233,6 +2247,7 @@ remove_partial_avx_dependency (void)
> continue;
> break;
> default:
> + gcc_assert (src_mode == E_VOIDmode);
> break;
> }
Wouldn't it be better to do:
E_VOIDmode:
gcc_assert (convert_p);
break;
default:
gcc_unreachable ();
?
Jakub
Re: [PATCH] [i386] Fix ICE in pass_rpad.
On Sat, Sep 18, 2021 at 3:31 PM Jakub Jelinek wrote:
>
> On Sat, Sep 18, 2021 at 11:09:32AM +0800, liuhongt wrote:
> > Besides conversion instructions, pass_rpad also handles scalar
> > sqrt/rsqrt/rcp/round instructions, while r12-3614 should only want to
> > handle conversion instructions, so fix it.
> >
> > Bootstrapped and regtest on x86_64-linux-gnu{-m32,} w/ configure
> > --enable-checking=yes,rtl,extra, failed tests are fixed.
> > Ok for trunk?
> >
> > New tests that PASS (8 tests):
> >
> > gcc.target/i386/avx512f-vscalefpd-2.c execution test
> > gcc.target/i386/avx512f-vscalefps-2.c execution test
> > gcc.target/i386/avx512f-vscalefss-2.c execution test
> > gcc.target/i386/avx512fp16-vscalefph-1b.c execution test
> > gcc.target/i386/avx512fp16-vscalefsh-1b.c execution test
> > gcc.target/i386/avx512fp16vl-vscalefph-1b.c execution test
> > gcc.target/i386/avx512vl-vscalefpd-2.c execution test
> > gcc.target/i386/avx512vl-vscalefps-2.c execution test
> >
> > Old tests that failed, that have disappeared (8 tests): (Eeek!)
> >
> > gcc.target/i386/avx512f-vscalefpd-2.c (internal compiler error)
> > gcc.target/i386/avx512f-vscalefps-2.c (internal compiler error)
> > gcc.target/i386/avx512f-vscalefss-2.c (internal compiler error)
> > gcc.target/i386/avx512fp16-vscalefph-1b.c (internal compiler error)
> > gcc.target/i386/avx512fp16-vscalefsh-1b.c (internal compiler error)
> > gcc.target/i386/avx512fp16vl-vscalefph-1b.c (internal compiler error)
> > gcc.target/i386/avx512vl-vscalefpd-2.c (internal compiler error)
> > gcc.target/i386/avx512vl-vscalefps-2.c (internal compiler error)
> >
> > gcc/ChangeLog:
> >
> > * config/i386/i386-features.c (remove_partial_avx_dependency):
> > Restrict TARGET_USE_VECTOR_FP_CONVERTS and
> > TARGET_USE_VECTOR_CONVERTS to conversion instructions only.
>
> Ok, with a minor nit:
>
> > @@ -2233,6 +2247,7 @@ remove_partial_avx_dependency (void)
> > continue;
> > break;
> > default:
> > + gcc_assert (src_mode == E_VOIDmode);
> > break;
> > }
>
> Wouldn't it be better to do:
> E_VOIDmode:
> gcc_assert (convert_p);
!convert_p, Must be typo :)
> break;
> default:
> gcc_unreachable ();
> ?
Sure.
>
> Jakub
>
--
BR,
Hongtao
Re: [PATCH] [i386] Fix ICE in pass_rpad.
On Sat, Sep 18, 2021 at 03:56:42PM +0800, Hongtao Liu wrote: > > Wouldn't it be better to do: > > E_VOIDmode: > > gcc_assert (convert_p); > !convert_p, Must be typo :) Yes, sorry. > > > break; > > default: > > gcc_unreachable (); > > ? > Sure. Jakub
openmp: Allow private or firstprivate arguments to default clause even for C/C++
Hi!
OpenMP 5.1 allows default(private) or default(firstprivate) even in C/C++,
but it behaves the same way as in Fortran only for variables not declared at
namespace or file scope. For the namespace/file scope variables it instead
behaves as default(none).
Bootstrapped/regtested on x86_64-linux and i686-linux, committed to trunk.
2021-09-18 Jakub Jelinek
gcc/
* gimplify.c (omp_default_clause): For C/C++ default({,first}private),
if file/namespace scope variable doesn't have predetermined sharing,
treat it as if there was default(none).
gcc/c/
* c-parser.c (c_parser_omp_clause_default): Handle private and
firstprivate arguments, adjust diagnostics on unknown argument.
gcc/cp/
* parser.c (cp_parser_omp_clause_default): Handle private and
firstprivate arguments, adjust diagnostics on unknown argument.
* cp-gimplify.c (cxx_omp_finish_clause): Handle OMP_CLAUSE_PRIVATE.
gcc/testsuite/
* c-c++-common/gomp/default-2.c: New test.
* c-c++-common/gomp/default-3.c: New test.
* g++.dg/gomp/default-1.C: New test.
libgomp/
* testsuite/libgomp.c++/default-1.C: New test.
* testsuite/libgomp.c-c++-common/default-1.c: New test.
* libgomp.texi (OpenMP 5.1): Mark "private and firstprivate argument
to default clause in C and C++" as implemented.
--- gcc/gimplify.c.jj 2021-09-14 10:36:41.351390039 +0200
+++ gcc/gimplify.c 2021-09-17 15:34:34.983711878 +0200
@@ -7369,6 +7369,18 @@ omp_default_clause (struct gimplify_omp_
default_kind = kind;
else if (VAR_P (decl) && TREE_STATIC (decl) && DECL_IN_CONSTANT_POOL (decl))
default_kind = OMP_CLAUSE_DEFAULT_SHARED;
+ /* For C/C++ default({,first}private), variables with static storage duration
+ declared in a namespace or global scope and referenced in construct
+ must be explicitly specified, i.e. acts as default(none). */
+ else if ((default_kind == OMP_CLAUSE_DEFAULT_PRIVATE
+ || default_kind == OMP_CLAUSE_DEFAULT_FIRSTPRIVATE)
+ && VAR_P (decl)
+ && is_global_var (decl)
+ && (DECL_FILE_SCOPE_P (decl)
+ || (DECL_CONTEXT (decl)
+ && TREE_CODE (DECL_CONTEXT (decl)) == NAMESPACE_DECL))
+ && !lang_GNU_Fortran ())
+default_kind = OMP_CLAUSE_DEFAULT_NONE;
switch (default_kind)
{
--- gcc/c/c-parser.c.jj 2021-09-17 11:28:07.601834144 +0200
+++ gcc/c/c-parser.c2021-09-17 14:20:15.942866085 +0200
@@ -13420,6 +13420,9 @@ c_parser_omp_clause_copyprivate (c_parse
/* OpenMP 2.5:
default ( none | shared )
+ OpenMP 5.1:
+ default ( private | firstprivate )
+
OpenACC:
default ( none | present ) */
@@ -13446,9 +13449,24 @@ c_parser_omp_clause_default (c_parser *p
break;
case 'p':
- if (strcmp ("present", p) != 0 || !is_oacc)
+ if (is_oacc)
+ {
+ if (strcmp ("present", p) != 0)
+ goto invalid_kind;
+ kind = OMP_CLAUSE_DEFAULT_PRESENT;
+ }
+ else
+ {
+ if (strcmp ("private", p) != 0)
+ goto invalid_kind;
+ kind = OMP_CLAUSE_DEFAULT_PRIVATE;
+ }
+ break;
+
+ case 'f':
+ if (strcmp ("firstprivate", p) != 0 || is_oacc)
goto invalid_kind;
- kind = OMP_CLAUSE_DEFAULT_PRESENT;
+ kind = OMP_CLAUSE_DEFAULT_FIRSTPRIVATE;
break;
case 's':
@@ -13469,7 +13487,8 @@ c_parser_omp_clause_default (c_parser *p
if (is_oacc)
c_parser_error (parser, "expected % or %");
else
- c_parser_error (parser, "expected % or %");
+ c_parser_error (parser, "expected %, %, "
+ "% or %");
}
parens.skip_until_found_close (parser);
--- gcc/cp/parser.c.jj 2021-09-17 11:28:07.606834074 +0200
+++ gcc/cp/parser.c 2021-09-17 15:44:33.665372845 +0200
@@ -36951,6 +36951,9 @@ cp_parser_omp_clause_collapse (cp_parser
/* OpenMP 2.5:
default ( none | shared )
+ OpenMP 5.1:
+ default ( private | firstprivate )
+
OpenACC:
default ( none | present ) */
@@ -36964,7 +36967,12 @@ cp_parser_omp_clause_default (cp_parser
matching_parens parens;
if (!parens.require_open (parser))
return list;
- if (cp_lexer_next_token_is (parser->lexer, CPP_NAME))
+ if (!is_oacc && cp_lexer_next_token_is_keyword (parser->lexer, RID_PRIVATE))
+{
+ kind = OMP_CLAUSE_DEFAULT_PRIVATE;
+ cp_lexer_consume_token (parser->lexer);
+}
+ else if (cp_lexer_next_token_is (parser->lexer, CPP_NAME))
{
tree id = cp_lexer_peek_token (parser->lexer)->u.value;
const char *p = IDENTIFIER_POINTER (id);
@@ -36983,6 +36991,12 @@ cp_parser_omp_clause_default (cp_parser
kind = OMP_CLAUSE_DEFAULT_PRESENT;
break;
+ case 'f':
+ if (strcmp ("firstprivate", p) != 0 || is_oacc)
+ goto invalid_kind;
+ kind = O
[committed] openmp: Handle unconstrained and reproducible modifiers on order(concurrent)
Hi!
This patch adds handling for unconstrained and reproducible modifiers on
order(concurrent) clause. For all static schedules (including auto and
no schedule or dist_schedule clauses) I believe what we implement is
reproducible, so the patch doesn't do much beyond recognizing those.
Note, there is an OpenMP/spec issue that needs resolution on what
should happen with the dynamic schedules (whether it should be an error
to mix such clauses, or silently make it non-reproducible, and in which
exact cases), so it might need some follow-up.
Besides that, this patch allows order(concurrent) clause on the distribute
construct which is something also added in OpenMP 5.1, and finally
check the newly added restriction that at most one order clause
can appear on a construct.
The allowing of order clause on distribute has a side-effect that
order(concurrent) copyin(thrpriv) is no longer allowed on combined/composite
constructs with distribute parallel for{, simd} in it, previously the
order applied only to for/simd and so a threadprivate var could be seen
in the construct, but now it also applies to distribute and so on the parallel
we shouldn't refer to a threadprivate var.
Bootstrapped/regtested on x86_64-linux and i686-linux, committed to trunk.
2021-09-18 Jakub Jelinek
gcc/
* tree.h (OMP_CLAUSE_ORDER_UNCONSTRAINED): Define.
* tree-pretty-print.c (dump_omp_clause): Print unconstrained:
for OMP_CLAUSE_ORDER_UNCONSTRAINED.
gcc/c-family/
* c-omp.c (c_omp_split_clauses): Split order clause also to
distribute construct. Copy over OMP_CLAUSE_ORDER_UNCONSTRAINED.
gcc/c/
* c-parser.c (c_parser_omp_clause_order): Parse unconstrained
and reproducible modifiers.
(OMP_DISTRIBUTE_CLAUSE_MASK): Add order clause.
gcc/cp/
* parser.c (cp_parser_omp_clause_order): Parse unconstrained
and reproducible modifiers.
(OMP_DISTRIBUTE_CLAUSE_MASK): Add order clause.
gcc/testsuite/
* c-c++-common/gomp/order-1.c (f2): Add tests for distribute
with order clause.
(f3): Remove.
* c-c++-common/gomp/order-2.c: Don't expect error for distribute
with order clause.
* c-c++-common/gomp/order-5.c: New test.
* c-c++-common/gomp/order-6.c: New test.
* c-c++-common/gomp/clause-dups-1.c (f1): Add tests for
duplicated order clause.
(f9): New function.
* c-c++-common/gomp/clauses-1.c (baz, bar): Don't mix copyin and
order(concurrent) clauses on the same composite construct combined
with distribute, instead split it into two tests, one without
copyin and one without order(concurrent). Add order(concurrent)
clauses to {,{,target} teams} distribute.
* g++.dg/gomp/attrs-1.C (baz, bar): Likewise.
* g++.dg/gomp/attrs-2.C (baz, bar): Likewise.
--- gcc/tree.h.jj 2021-09-16 10:51:02.295976216 +0200
+++ gcc/tree.h 2021-09-17 18:21:01.473512901 +0200
@@ -1715,6 +1715,10 @@ class auto_suppress_location_wrappers
#define OMP_CLAUSE_ORDERED_EXPR(NODE) \
OMP_CLAUSE_OPERAND (OMP_CLAUSE_SUBCODE_CHECK (NODE, OMP_CLAUSE_ORDERED), 0)
+/* True for unconstrained modifier on order(concurrent) clause. */
+#define OMP_CLAUSE_ORDER_UNCONSTRAINED(NODE) \
+ (OMP_CLAUSE_SUBCODE_CHECK (NODE, OMP_CLAUSE_ORDER)->base.public_flag)
+
#define OMP_CLAUSE_REDUCTION_CODE(NODE)\
(OMP_CLAUSE_RANGE_CHECK (NODE, OMP_CLAUSE_REDUCTION, \
OMP_CLAUSE_IN_REDUCTION)->omp_clause.subcode.reduction_code)
--- gcc/tree-pretty-print.c.jj 2021-09-11 09:33:37.928331352 +0200
+++ gcc/tree-pretty-print.c 2021-09-17 19:20:35.470795696 +0200
@@ -1149,7 +1149,10 @@ dump_omp_clause (pretty_printer *pp, tre
break;
case OMP_CLAUSE_ORDER:
- pp_string (pp, "order(concurrent)");
+ pp_string (pp, "order(");
+ if (OMP_CLAUSE_ORDER_UNCONSTRAINED (clause))
+ pp_string (pp, "unconstrained:");
+ pp_string (pp, "concurrent)");
break;
case OMP_CLAUSE_BIND:
--- gcc/c-family/c-omp.c.jj 2021-09-17 11:28:07.599834172 +0200
+++ gcc/c-family/c-omp.c2021-09-17 19:19:52.628391341 +0200
@@ -2114,14 +2114,31 @@ c_omp_split_clauses (location_t loc, enu
}
s = C_OMP_CLAUSE_SPLIT_PARALLEL;
break;
- /* order clauses are allowed on for, simd and loop. */
+ /* order clauses are allowed on distribute, for, simd and loop. */
case OMP_CLAUSE_ORDER:
+ if ((mask & (OMP_CLAUSE_MASK_1
+ << PRAGMA_OMP_CLAUSE_DIST_SCHEDULE)) != 0)
+ {
+ if (code == OMP_DISTRIBUTE)
+ {
+ s = C_OMP_CLAUSE_SPLIT_DISTRIBUTE;
+ break;
+ }
+ c = build_omp_clause (OMP_CLAUSE_LOCATION (clauses),
+ OMP_CLAUSE_ORDER);
+ OMP_CLAUSE_ORDER_UNCONSTRAINED (c)
+ = OMP_CLAUSE_ORDER_UNCONSTRAINED (clauses);
+ OMP
[PATCH] PR middle-end/88173: More constant folding of NaN comparisons.
This patch tackles PR middle-end/88173 where the order of operands in
a comparison affects constant folding. As diagnosed by Jason Merrill,
"match.pd handles these comparisons very differently". The history is
that the middle end, typically canonicalizes comparisons to place
constants on the right, but when a comparison contains two constants
we need to check/transform both constants, i.e. on both the left and the
right. Hence the added lines below duplicate for @0 the same transform
applied a few lines above for @1.
Whilst preparing the testcase, I noticed that this transformation is
incorrectly disabled with -fsignaling-nans even when both operands are
known not be be signaling NaNs, so I've corrected that and added a
second test case. Unfortunately, c-c++-common/pr57371-4.c then starts
failing, as it doesn't distinguish QNaNs (which are quiet) from SNaNs
(which signal), so this patch includes a minor tweak to the expected
behaviour for QNaNs in that existing test.
I suspect the middle-end (both at the tree-level and RTL) might need
to respect -ftrapping-math for the non-equality comparisons against
QNaN, but that's a different PR/patch [I need to double check/research
whether and why those checks may have been removed in the past].
This patch has been tested on x86_64-pc-linux-gnu with "make bootstrap"
and "make -k check" with no new failures. Ok for mainline?
2021-09-18 Roger Sayle
gcc/ChangeLog
PR middle-end/88173
* match.pd (cmp @0 REAL_CST@1): When @0 is also REAL_CST, apply
the same transformations as to @1. For comparisons against NaN,
don't check HONOR_SNANS but confirm that neither operand is a
signaling NaN.
gcc/testsuite/ChangeLog
PR middle-end/88173
* c-c++-common/pr57371-4.c: Tweak/correct test case for QNaNs.
* g++.dg/pr88173-1.C: New test case.
* g++.dg/pr88173-2.C: New test case.
Roger
--
diff --git a/gcc/match.pd b/gcc/match.pd
index 008f775..4a2f058 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -4480,9 +4480,20 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
/* a CMP (-0) -> a CMP 0 */
(if (REAL_VALUE_MINUS_ZERO (TREE_REAL_CST (@1)))
(cmp @0 { build_real (TREE_TYPE (@1), dconst0); }))
+ /* (-0) CMP b -> 0 CMP b. */
+ (if (TREE_CODE (@0) == REAL_CST
+ && REAL_VALUE_MINUS_ZERO (TREE_REAL_CST (@0)))
+(cmp { build_real (TREE_TYPE (@0), dconst0); } @1))
/* x != NaN is always true, other ops are always false. */
(if (REAL_VALUE_ISNAN (TREE_REAL_CST (@1))
- && ! HONOR_SNANS (@1))
+ && !tree_expr_signaling_nan_p (@1)
+ && !tree_expr_maybe_signaling_nan_p (@0))
+{ constant_boolean_node (cmp == NE_EXPR, type); })
+ /* NaN != y is always true, other ops are always false. */
+ (if (TREE_CODE (@0) == REAL_CST
+ && REAL_VALUE_ISNAN (TREE_REAL_CST (@0))
+&& !tree_expr_signaling_nan_p (@0)
+&& !tree_expr_signaling_nan_p (@1))
{ constant_boolean_node (cmp == NE_EXPR, type); })
/* Fold comparisons against infinity. */
(if (REAL_VALUE_ISINF (TREE_REAL_CST (@1))
diff --git a/gcc/testsuite/c-c++-common/pr57371-4.c
b/gcc/testsuite/c-c++-common/pr57371-4.c
index f43f7c2..d938ecd 100644
--- a/gcc/testsuite/c-c++-common/pr57371-4.c
+++ b/gcc/testsuite/c-c++-common/pr57371-4.c
@@ -13,25 +13,25 @@ void nonfinite(unsigned short x) {
{
volatile int nonfinite_1;
nonfinite_1 = (float) x > QNAN;
-/* { dg-final { scan-tree-dump "nonfinite_1 = \\(float\\)" "original" } }
*/
+/* { dg-final { scan-tree-dump "nonfinite_1 = 0" "original" } } */
}
{
volatile int nonfinite_2;
nonfinite_2 = (float) x >= QNAN;
-/* { dg-final { scan-tree-dump "nonfinite_2 = \\(float\\)" "original" } }
*/
+/* { dg-final { scan-tree-dump "nonfinite_2 = 0" "original" } } */
}
{
volatile int nonfinite_3;
nonfinite_3 = (float) x < QNAN;
-/* { dg-final { scan-tree-dump "nonfinite_3 = \\(float\\)" "original" } }
*/
+/* { dg-final { scan-tree-dump "nonfinite_3 = 0" "original" } } */
}
{
volatile int nonfinite_4;
nonfinite_4 = (float) x <= QNAN;
-/* { dg-final { scan-tree-dump "nonfinite_4 = \\(float\\)" "original" } }
*/
+/* { dg-final { scan-tree-dump "nonfinite_4 = 0" "original" } } */
}
{
/* { dg-do compile } */
/* { dg-options "-O2 -std=c++11" } */
#define big __builtin_huge_val()
#define nan __builtin_nan("")
constexpr bool b1 = big > nan;
constexpr bool b2 = nan < big;
/* { dg-do compile } */
/* { dg-options "-O2 -fsignaling-nans -std=c++11" } */
#define big __builtin_huge_val()
#define nan __builtin_nan("")
constexpr bool b1 = big > nan;
constexpr bool b2 = nan < big;
Re: [PATCH/RFC 1/2] WPD: Enable whole program devirtualization
>On 9/16/21 22:29, Feng Xue OS wrote: >>> On 9/16/21 05:25, Feng Xue OS via Gcc-patches wrote: This and following patches are composed to enable full devirtualization under whole program assumption (so also called whole-program devirtualization, WPD for short), which is an enhancement to current speculative devirtualization. The base of the optimization is how to identify class type that is local in terms of whole-program scope, at least those class types in libstdc++ must be excluded in some way. Our means is to use typeinfo symbol as identity marker of a class since it is unique and always generated once the class or its derived type is instantiated somewhere, and rely on symbol resolution by lto-linker-plugin to detect whether a typeinfo is referenced by regular object/library, which indirectly tells class types are escaped or not. The RFC at https://gcc.gnu.org/pipermail/gcc/2021-August/237132.html gives more details on that. Bootstrapped/regtested on x86_64-linux and aarch64-linux. Thanks, Feng 2021-09-07 Feng Xue gcc/ * common.opt (-fdevirtualize-fully): New option. * class.c (build_rtti_vtbl_entries): Force generation of typeinfo even -fno-rtti is specificied under full devirtualization. >>> >>> This makes -fno-rtti useless; rather than this, you should warn about >>> the combination of flags and force flag_rtti on. It also sounds like >>> you depend on the library not being built with -fno-rtti. >> >> Although rtti is generated by front-end, we will remove it after lto symtab >> merge, which is meant to keep same behavior as -fno-rtti. > > Ah, the cp/ change is OK, then, with a comment about that. > >> Yes, regular library to be linked with should contain rtti data, otherwise >> WPD could not deduce class type usage safely. By default, we can think >> that it should work for libstdc++, but it probably becomes a problem for >> user library, which might be avoided if we properly document this >> requirement and suggest user doing that when using WPD. > > Yes, I would expect that external libraries would be built with RTTI on > to allow users to use RTTI features even if they aren't used within the > library. But it's good to document it as a requirement. > >> + /* If a class with virtual base is only instantiated as >> + subobjects of derived classes, and has no complete object in >> + compilation unit, merely construction vtables will be >> involved, >> + its primary vtable is really not needed, and subject to being >> + removed. So once a vtable node is encountered, for all >> + polymorphic base classes of the vtable's context class, always >> + force generation of primary vtable nodes when full >> + devirtualization is enabled. */ > > Why do you need the primary vtable if you're relying on RTTI info? > Construction vtables will point to the same RTTI node. At middle end, the easiest way to get vtable of type is via TYPE_BINFO(type), it is the primary one. And WPD relies on existence of varpool_node of the vtable decl to determine if the type has been removed (when it is never instantiated), so we will force generation of vtable node at very early stage. Additionally, construction vtable (C-in-D) belongs to the class (D) of complete object, not the class (C) of subobject actually being constructed for, it is not easy to correlate construction vtable with the subobject class (C) after front end. > >> + /* Public class w/o key member function (or local class in a public >> + inline function) requires COMDAT-like vtable so as to be shared >> + among units. But C++ privatizing via -fno-weak would introduce >> + multiple static vtable copies for one class in merged lto symbol >> + table. This breaks one-to-one correspondence between class and >> + vtable, and makes class liveness check become not that easy. To >> + be simple, we exclude such kind of class from our choice list. > > Same question. Also, why would you use -fno-weak? Forcing multiple > copies of things we're perfectly capable of combining seems like a > strange choice. You can privatize things with the symbol visibility > controls or RTLD_LOCAL. We expect that user does not specify -fno-weak for WPD. But if specified, we should correctly handle that and bypass the type. And indeed there is no need to force generation of vtable under this situation. But if vtable is not keyed to any compilation unit, we might never have any copy of it in ordinary build, while its class type is meaningful to whole-program analysis, such as an abstract root class. Thanks, Feng
Re: [PATCH v3] ipa-inline: Add target info into fn summary [PR102059]
Hi,
On Fri, Sep 17 2021, Segher Boessenkool wrote:
> On Fri, Sep 17, 2021 at 05:42:38PM +0800, Kewen.Lin wrote:
>> Against v2 [2], this v3 addressed Martin's review comments:
>> - Replace HWI auto_vec with unsigned int for target_info
>> to avoid overkill (also Segher's comments), adjust some
>> places need to be updated for this change.
>
> I'd have used a single HWI (always 64 bits), but an int (always at least
> 32 bits in GCC) is fine as well, sure.
Let's have it as unsigned int then (in a separate thread I was
suggesting to go even smaller).
> That easily fits one line. (Many more examples here btw).
>
>> +bool
>> +rs6000_fn_has_any_of_these_mask_bits (enum rs6000_builtins code,
>> + HOST_WIDE_INT mask)
>> +{
>> + gcc_assert (code < RS6000_BUILTIN_COUNT);
>
> We don't have this assert anywhere else, so lose it here as well?
>
> If we want such checking we should make an inline accessor function for
> this, and check it there. But we already do a check in
> rs6000_builtin_decl (and one in def_builtin, but that one has an
> off-by-one error in it).
>
>> +extern bool rs6000_fn_has_any_of_these_mask_bits (enum rs6000_builtins code,
>> + HOST_WIDE_INT mask);
>
> The huge unwieldy name suggests it might not be the best abstraction you
> could use, btw ;-)
>
>> +static bool
>> +rs6000_update_ipa_fn_target_info (unsigned int &info, const gimple *stmt)
>> +{
>> + /* Assume inline asm can use any instruction features. */
>> + if (gimple_code (stmt) == GIMPLE_ASM)
>
> This should be fine for HTM, but it may be a bit *too* pessimistic for
> other features. We'll see when we get there :-)
>
>> +@deftypefn {Target Hook} bool TARGET_NEED_IPA_FN_TARGET_INFO (const_tree
>> @var{decl}, unsigned int& @var{info})
>> +Allow target to check early whether it is necessary to analyze all gimple
>> +statements in the given function to update target specific information for
>> +inlining. See hook @code{update_ipa_fn_target_info} for usage example of
> [ ... ]
>> +The default version of this hook returns false.
>
> And that is really the only reason to have this premature optimisation:
> targets that do not care do not have to pay the price, however trivial
> that price may be, which is a good idea politically ;-)
>
>> +/* { dg-final { scan-tree-dump-times "Inlining foo/\[0-9\]* " 1 "einline"}
>> } */
>
> If you use {} instead of "" you don't need the backslashes.
>
>> +default_update_ipa_fn_target_info (uint16_t &, const gimple *)
>
> I'm surprised the compiler didn't warn about this btw.
>
> The rs6000 parts are okay for trunk (with the trivial cleanups please).
> Thanks!
the IPA bits are OK too (after the type mismatch is fixed).
Martin
Re: [PATCH] introduce predicate analysis class
On 9/17/21 10:08 PM, Jeff Law wrote: On 9/17/2021 4:05 PM, Martin Sebor wrote: On 9/2/21 10:28 AM, Jeff Law via Gcc-patches wrote: On 8/30/2021 2:03 PM, Martin Sebor via Gcc-patches wrote: The predicate analysis subset of the tree-ssa-uninit pass isn't necessarily specific to the detection of uninitialized reads. Suitably parameterized, the same core logic could be used in other warning passes to improve their S/N ratio, or issue more nuanced diagnostics (e.g., when an invalid access cannot be ruled out but also need not in reality be unavoidable, issue a "may be invalid" type of warning rather than "is invalid"). Separating the predicate analysis logic from the uninitialized pass and exposing a narrow API should also make it easier to understand and evolve each part independently of the other, or replace one with a better implementation without modifying the other.(*) As the first step in this direction, the attached patch extracts the predicate analysis logic out of the pass, turns the interface into public class members, and hides the internals in either private members or static functions defined in a new source file. (**) The changes should have no externally observable effect (i.e., should cause no changes in warnings), except on the contents of the uninitialized dump. While making the changes I enhanced the dumps to help me follow the logic. Turning some previously free-standing functions into members involved changing their signatures and adjusting their callers. While making these changes I also renamed some of them as well some variables for improved clarity. Finally, I moved declarations of locals closer to their point of initialization. Tested on x86_64-linux. Besides the usual bootstrap/regtest I also tentatively verified the generality of the new class interfaces by making use of it in -Warray-bounds. Besides there, I'd like to make use of it in the new gimple-ssa-warn-access pass and, longer term, any other flow-sensitive warnings that might benefit from it. Martin [*] A review of open -Wuninitialized bugs I did while working on this project made me aware of a number of opportunities to improve the analyzer to reduce the number of false positives -Wmaybe-uninitiailzed suffers from. [**] The class isn't fully general and, like the uninit pass, only works with PHI nodes. I plan to generalize it to compute the set of predicates between any two basic blocks. gcc-predanal.diff Factor predidacte analysis out of tree-ssa-uninit.c into its own module. gcc/ChangeLog: * Makefile.in (OBJS): Add gimple-predicate-analysis.o. * tree-ssa-uninit.c (max_phi_args): Move to gimple-predicate-analysis. (MASK_SET_BIT, MASK_TEST_BIT, MASK_EMPTY): Same. (check_defs): (can_skip_redundant_opnd): (compute_uninit_opnds_pos): Adjust to namespace change. (find_pdom): Move to gimple-predicate-analysis.cc. (find_dom): Same. (struct uninit_undef_val_t): New. (is_non_loop_exit_postdominating): Move to gimple-predicate-analysis.cc. (find_control_equiv_block): Same. (MAX_NUM_CHAINS, MAX_CHAIN_LEN, MAX_POSTDOM_CHECK): Same. (MAX_SWITCH_CASES): Same. (compute_control_dep_chain): Same. (find_uninit_use): Use predicate analyzer. (struct pred_info): Move to gimple-predicate-analysis. (convert_control_dep_chain_into_preds): Same. (find_predicates): Same. (collect_phi_def_edges): Same. (warn_uninitialized_phi): Use predicate analyzer. (find_def_preds): Move to gimple-predicate-analysis. (dump_pred_info): Same. (dump_pred_chain): Same. (dump_predicates): Same. (destroy_predicate_vecs): Remove. (execute_late_warn_uninitialized): New. (get_cmp_code): Move to gimple-predicate-analysis. (is_value_included_in): Same. (value_sat_pred_p): Same. (find_matching_predicate_in_rest_chains): Same. (is_use_properly_guarded): Same. (prune_uninit_phi_opnds): Same. (find_var_cmp_const): Same. (use_pred_not_overlap_with_undef_path_pred): Same. (pred_equal_p): Same. (is_neq_relop_p): Same. (is_neq_zero_form_p): Same. (pred_expr_equal_p): Same. (is_pred_expr_subset_of): Same. (is_pred_chain_subset_of): Same. (is_included_in): Same. (is_superset_of): Same. (pred_neg_p): Same. (simplify_pred): Same. (simplify_preds_2): Same. (simplify_preds_3): Same. (simplify_preds_4): Same. (simplify_preds): Same. (push_pred): Same. (push_to_worklist): Same. (get_pred_info_from_cmp): Same. (is_degenerated_phi): Same. (normalize_one_pred_1): Same. (normalize_one_pred): Same. (normalize_one_pred_chain): Same. (normalize_preds): Same. (can_one_predicate_be_invalidated_p): Same. (can_chain_union_be_invalidated_p): Same. (uninit_uses_cannot_happen): Same. (pass_late_warn_uninitialized::execute): Define. * gimple-predicate-analysis.cc: New file. * gimple-predicate-analysis.h: New file. Thanks for tack
Re: [PATCH] libgcc, i386: Export *hf* and *hc* from libgcc_s.so.1
Hi Hongtao,
> On 8 Sep 2021, at 10:31, Hongtao Liu wrote:
>
> On Wed, Sep 8, 2021 at 5:09 PM Jakub Jelinek wrote:
>>
>> On Wed, Sep 08, 2021 at 10:37:17AM +0800, Hongtao Liu wrote:
>>> Bootstrapped and regtested on x86_64-linux-gnu{-m32,}.
>>> Ok for trunk?
>>>
>>> libgcc/ChangeLog:
>>>
>>>* config/i386/t-softfp: Compile __{mul,div}hc3 into
>>>libgcc_s.so.1.
this was applied as
https://gcc.gnu.org/pipermail/gcc-cvs/2021-September/353114.html
but it seems that (at least on Linux and Darwin) we now see a lot of:
/src-local/gcc-master-patched/libgcc/shared-object.mk:14: warning: overriding
commands for target `_divhc3.o'
Makefile:501: warning: ignoring old commands for target `_divhc3.o'
/src-local/gcc-master-patched/libgcc/shared-object.mk:17: warning: overriding
commands for target `_divhc3_s.o’
and I think this is because we need to exclude the libgcc2 version of the
functions before adding teh replacements, like the patch below.
tested on x86_64-linux, darwin observing that the __divhc3 and __mulh3 symbols
are present and that the metadata for the source files indicates that they are
the replacement sources.
OK for master?
Iain
——
[PATCH] libgcc, X86: Exclude rules for libgcc2 __{div,mul}hc3.
We want to override the libgcc2 generic version of these functions
for X86. First exclude the original and the add in the replacements.
Signed-off-by: Iain Sandoe
libgcc/ChangeLog:
* config/i386/t-softfp: Exclude libgcc2 versions of __divhc3
and __mulhc3.
---
libgcc/config/i386/t-softfp | 1 +
1 file changed, 1 insertion(+)
diff --git a/libgcc/config/i386/t-softfp b/libgcc/config/i386/t-softfp
index 7620cc0cec5..fe2ad8a3c08 100644
--- a/libgcc/config/i386/t-softfp
+++ b/libgcc/config/i386/t-softfp
@@ -2,6 +2,7 @@ LIB2ADD += $(srcdir)/config/i386/sfp-exceptions.c
# Replace _divhc3 and _mulhc3.
libgcc2-hf-functions = _divhc3 _mulhc3
+LIB2FUNCS_EXCLUDE += $(libgcc2-hf-functions)
libgcc2-hf-extras = $(addsuffix .c, $(libgcc2-hf-functions))
LIB2ADD += $(addprefix $(srcdir)/config/i386/, $(libgcc2-hf-extras))
--
[PATCH] Fix middle-end/102395: reg_class having only NO_REGS and ALL_REGS.
From: Andrew Pinski So this is a simple fix is to just add to the assert that sclass and dclass are both greater than or equal to NO_REGS. NO_REGS is documented as the first register class so it should have the value of 0. gcc/ChangeLog: * lra-constraints.c (check_and_process_move): Assert that dclass and sclass are greater than or equal to NO_REGS. --- gcc/lra-constraints.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gcc/lra-constraints.c b/gcc/lra-constraints.c index a56080bee35..4d734548c38 100644 --- a/gcc/lra-constraints.c +++ b/gcc/lra-constraints.c @@ -1276,7 +1276,7 @@ check_and_process_move (bool *change_p, bool *sec_mem_p ATTRIBUTE_UNUSED) sclass = dclass = NO_REGS; if (REG_P (dreg)) dclass = get_reg_class (REGNO (dreg)); - gcc_assert (dclass < LIM_REG_CLASSES); + gcc_assert (dclass < LIM_REG_CLASSES && dclass >= NO_REGS); if (dclass == ALL_REGS) /* ALL_REGS is used for new pseudos created by transformations like reload of SUBREG_REG (see function @@ -1288,7 +1288,7 @@ check_and_process_move (bool *change_p, bool *sec_mem_p ATTRIBUTE_UNUSED) return false; if (REG_P (sreg)) sclass = get_reg_class (REGNO (sreg)); - gcc_assert (sclass < LIM_REG_CLASSES); + gcc_assert (sclass < LIM_REG_CLASSES && sclass >= NO_REGS); if (sclass == ALL_REGS) /* See comments above. */ return false; -- 2.17.1
Re: [PATCH] introduce predicate analysis class
On 9/18/21 12:46 PM, Martin Sebor wrote: On 9/17/21 10:08 PM, Jeff Law wrote: On 9/17/2021 4:05 PM, Martin Sebor wrote: On 9/2/21 10:28 AM, Jeff Law via Gcc-patches wrote: On 8/30/2021 2:03 PM, Martin Sebor via Gcc-patches wrote: The predicate analysis subset of the tree-ssa-uninit pass isn't necessarily specific to the detection of uninitialized reads. Suitably parameterized, the same core logic could be used in other warning passes to improve their S/N ratio, or issue more nuanced diagnostics (e.g., when an invalid access cannot be ruled out but also need not in reality be unavoidable, issue a "may be invalid" type of warning rather than "is invalid"). Separating the predicate analysis logic from the uninitialized pass and exposing a narrow API should also make it easier to understand and evolve each part independently of the other, or replace one with a better implementation without modifying the other.(*) As the first step in this direction, the attached patch extracts the predicate analysis logic out of the pass, turns the interface into public class members, and hides the internals in either private members or static functions defined in a new source file. (**) The changes should have no externally observable effect (i.e., should cause no changes in warnings), except on the contents of the uninitialized dump. While making the changes I enhanced the dumps to help me follow the logic. Turning some previously free-standing functions into members involved changing their signatures and adjusting their callers. While making these changes I also renamed some of them as well some variables for improved clarity. Finally, I moved declarations of locals closer to their point of initialization. Tested on x86_64-linux. Besides the usual bootstrap/regtest I also tentatively verified the generality of the new class interfaces by making use of it in -Warray-bounds. Besides there, I'd like to make use of it in the new gimple-ssa-warn-access pass and, longer term, any other flow-sensitive warnings that might benefit from it. Martin [*] A review of open -Wuninitialized bugs I did while working on this project made me aware of a number of opportunities to improve the analyzer to reduce the number of false positives -Wmaybe-uninitiailzed suffers from. [**] The class isn't fully general and, like the uninit pass, only works with PHI nodes. I plan to generalize it to compute the set of predicates between any two basic blocks. gcc-predanal.diff Factor predidacte analysis out of tree-ssa-uninit.c into its own module. gcc/ChangeLog: * Makefile.in (OBJS): Add gimple-predicate-analysis.o. * tree-ssa-uninit.c (max_phi_args): Move to gimple-predicate-analysis. (MASK_SET_BIT, MASK_TEST_BIT, MASK_EMPTY): Same. (check_defs): (can_skip_redundant_opnd): (compute_uninit_opnds_pos): Adjust to namespace change. (find_pdom): Move to gimple-predicate-analysis.cc. (find_dom): Same. (struct uninit_undef_val_t): New. (is_non_loop_exit_postdominating): Move to gimple-predicate-analysis.cc. (find_control_equiv_block): Same. (MAX_NUM_CHAINS, MAX_CHAIN_LEN, MAX_POSTDOM_CHECK): Same. (MAX_SWITCH_CASES): Same. (compute_control_dep_chain): Same. (find_uninit_use): Use predicate analyzer. (struct pred_info): Move to gimple-predicate-analysis. (convert_control_dep_chain_into_preds): Same. (find_predicates): Same. (collect_phi_def_edges): Same. (warn_uninitialized_phi): Use predicate analyzer. (find_def_preds): Move to gimple-predicate-analysis. (dump_pred_info): Same. (dump_pred_chain): Same. (dump_predicates): Same. (destroy_predicate_vecs): Remove. (execute_late_warn_uninitialized): New. (get_cmp_code): Move to gimple-predicate-analysis. (is_value_included_in): Same. (value_sat_pred_p): Same. (find_matching_predicate_in_rest_chains): Same. (is_use_properly_guarded): Same. (prune_uninit_phi_opnds): Same. (find_var_cmp_const): Same. (use_pred_not_overlap_with_undef_path_pred): Same. (pred_equal_p): Same. (is_neq_relop_p): Same. (is_neq_zero_form_p): Same. (pred_expr_equal_p): Same. (is_pred_expr_subset_of): Same. (is_pred_chain_subset_of): Same. (is_included_in): Same. (is_superset_of): Same. (pred_neg_p): Same. (simplify_pred): Same. (simplify_preds_2): Same. (simplify_preds_3): Same. (simplify_preds_4): Same. (simplify_preds): Same. (push_pred): Same. (push_to_worklist): Same. (get_pred_info_from_cmp): Same. (is_degenerated_phi): Same. (normalize_one_pred_1): Same. (normalize_one_pred): Same. (normalize_one_pred_chain): Same. (normalize_preds): Same. (can_one_predicate_be_invalidated_p): Same. (can_chain_union_be_invalidated_p): Same. (uninit_uses_cannot_happen): Same. (pass_late_warn_uninitialized::execute): Define. * gimple-predicate-analysis.cc: New file. * gimple-predi
Re: [PATCH] Fix middle-end/102395: reg_class having only NO_REGS and ALL_REGS.
On 9/18/2021 1:16 PM, apinski--- via Gcc-patches wrote: From: Andrew Pinski So this is a simple fix is to just add to the assert that sclass and dclass are both greater than or equal to NO_REGS. NO_REGS is documented as the first register class so it should have the value of 0. gcc/ChangeLog: * lra-constraints.c (check_and_process_move): Assert that dclass and sclass are greater than or equal to NO_REGS. OK jeff
Re: [PATCH] libgcc, i386: Export *hf* and *hc* from libgcc_s.so.1
On Sunday, September 19, 2021, Iain Sandoe
wrote:
> Hi Hongtao,
>
> > On 8 Sep 2021, at 10:31, Hongtao Liu wrote:
> >
> > On Wed, Sep 8, 2021 at 5:09 PM Jakub Jelinek wrote:
> >>
> >> On Wed, Sep 08, 2021 at 10:37:17AM +0800, Hongtao Liu wrote:
> >>> Bootstrapped and regtested on x86_64-linux-gnu{-m32,}.
> >>> Ok for trunk?
> >>>
> >>> libgcc/ChangeLog:
> >>>
> >>>* config/i386/t-softfp: Compile __{mul,div}hc3 into
> >>>libgcc_s.so.1.
>
> this was applied as
> https://gcc.gnu.org/pipermail/gcc-cvs/2021-September/353114.html
>
> but it seems that (at least on Linux and Darwin) we now see a lot of:
>
> /src-local/gcc-master-patched/libgcc/shared-object.mk:14: warning:
> overriding commands for target `_divhc3.o'
> Makefile:501: warning: ignoring old commands for target `_divhc3.o'
> /src-local/gcc-master-patched/libgcc/shared-object.mk:17: warning:
> overriding commands for target `_divhc3_s.o’
>
> and I think this is because we need to exclude the libgcc2 version of the
> functions before adding teh replacements, like the patch below.
>
> tested on x86_64-linux, darwin observing that the __divhc3 and __mulh3
> symbols are present and that the metadata for the source files indicates
> that they are the replacement sources.
>
> OK for master?
Yes, thanks.
> Iain
>
> ——
>
> [PATCH] libgcc, X86: Exclude rules for libgcc2 __{div,mul}hc3.
>
> We want to override the libgcc2 generic version of these functions
> for X86. First exclude the original and the add in the replacements.
>
> Signed-off-by: Iain Sandoe
>
> libgcc/ChangeLog:
>
> * config/i386/t-softfp: Exclude libgcc2 versions of __divhc3
> and __mulhc3.
> ---
> libgcc/config/i386/t-softfp | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/libgcc/config/i386/t-softfp b/libgcc/config/i386/t-softfp
> index 7620cc0cec5..fe2ad8a3c08 100644
> --- a/libgcc/config/i386/t-softfp
> +++ b/libgcc/config/i386/t-softfp
> @@ -2,6 +2,7 @@ LIB2ADD += $(srcdir)/config/i386/sfp-exceptions.c
>
> # Replace _divhc3 and _mulhc3.
> libgcc2-hf-functions = _divhc3 _mulhc3
> +LIB2FUNCS_EXCLUDE += $(libgcc2-hf-functions)
> libgcc2-hf-extras = $(addsuffix .c, $(libgcc2-hf-functions))
> LIB2ADD += $(addprefix $(srcdir)/config/i386/, $(libgcc2-hf-extras))
>
> --
>
>
>
--
BR,
Hongtao
Re: [PATCH] Remove unused function make_unique_name.
On 9/13/2021 6:16 PM, Benjamin Peterson wrote: * attribs.c (make_unique_name): Delete. * attribs.h (make_unique_name): Delete. Thanks. Installed on the trunk. jeff
RE:PIPE GENERATOR
Hi, my friend Do you have any interesting to import from china? home generator parts、ball valve、pipe is our main product, tell me if you need further info. thanks in advance Best regards PAGE ZU NAYAO GROUP TAIZHOU CHINA
Re: [PATCH] Come up with section_flag enum.
On 9/7/2021 3:43 AM, Martin Liška wrote: Hi. I'm planning some refactoring related to 'section *' and I noticed we have quite ugly mask definitions (of form 1UL << N), where SECTION_FORGET is unused and #define SECTION_STYLE_MASK 0x60 /* bits used for SECTION_STYLE */ Is actually OR of 2 other values. What about making that a standard enum value with 1UL << N values? Patch can bootstrap on x86_64-linux-gnu and survives regression tests. Ready to be installed? Thanks, Martin gcc/ChangeLog: * output.h (enum section_flag): New. (SECTION_FORGET): Remove. (SECTION_ENTSIZE): Make it (1UL << 8) - 1. (SECTION_STYLE_MASK): Define it based on other enum values. * varasm.c (switch_to_section): Remove unused handling of SECTION_FORGET. OK jeff
Re: [PATCH] Optimize macro: make it more predictable
On 9/13/2021 7:52 AM, Martin Liška wrote: On 8/27/21 11:05, Richard Biener wrote: So with ignoring darktable which seems completely insane the cases will likely continue to work as intended if we change from the current scheme to appending as proposed. All right, I'm addressing the flag_complex_method in a separate sub-thread. There's slightly updated version of the patch where I modifed the documentation bits. Patch can bootstrap on x86_64-linux-gnu and survives regression tests. Ready to be installed? Thanks, Martin 0001-Append-target-optimize-attr-to-the-current-cmdline.patch From e13e3ec56acfb62543bc1912f1310d00eefba5c3 Mon Sep 17 00:00:00 2001 From: Martin Liska Date: Wed, 2 Jun 2021 08:44:37 +0200 Subject: [PATCH] Append target/optimize attr to the current cmdline. gcc/c-family/ChangeLog: * c-common.c (parse_optimize_options): Combine optimize options with what was provided on the command line. gcc/ChangeLog: * toplev.c (toplev::main): Save decoded optimization options. * toplev.h (save_opt_decoded_options): New. * doc/extend.texi: Be more clear about optimize and target attributes. gcc/testsuite/ChangeLog: * gcc.target/i386/avx512er-vrsqrt28ps-3.c: Disable fast math. * gcc.target/i386/avx512er-vrsqrt28ps-5.c: Likewise. * gcc.target/i386/attr-optimize.c: New test. OK jeff
Re: [PATCH] PR middle-end/88173: More constant folding of NaN comparisons.
On 9/18/2021 2:56 AM, Roger Sayle wrote: This patch tackles PR middle-end/88173 where the order of operands in a comparison affects constant folding. As diagnosed by Jason Merrill, "match.pd handles these comparisons very differently". The history is that the middle end, typically canonicalizes comparisons to place constants on the right, but when a comparison contains two constants we need to check/transform both constants, i.e. on both the left and the right. Hence the added lines below duplicate for @0 the same transform applied a few lines above for @1. Whilst preparing the testcase, I noticed that this transformation is incorrectly disabled with -fsignaling-nans even when both operands are known not be be signaling NaNs, so I've corrected that and added a second test case. Unfortunately, c-c++-common/pr57371-4.c then starts failing, as it doesn't distinguish QNaNs (which are quiet) from SNaNs (which signal), so this patch includes a minor tweak to the expected behaviour for QNaNs in that existing test. I suspect the middle-end (both at the tree-level and RTL) might need to respect -ftrapping-math for the non-equality comparisons against QNaN, but that's a different PR/patch [I need to double check/research whether and why those checks may have been removed in the past]. This patch has been tested on x86_64-pc-linux-gnu with "make bootstrap" and "make -k check" with no new failures. Ok for mainline? 2021-09-18 Roger Sayle gcc/ChangeLog PR middle-end/88173 * match.pd (cmp @0 REAL_CST@1): When @0 is also REAL_CST, apply the same transformations as to @1. For comparisons against NaN, don't check HONOR_SNANS but confirm that neither operand is a signaling NaN. gcc/testsuite/ChangeLog PR middle-end/88173 * c-c++-common/pr57371-4.c: Tweak/correct test case for QNaNs. * g++.dg/pr88173-1.C: New test case. * g++.dg/pr88173-2.C: New test case. OK jeff
Re: [PING] Re: [Committed] [PATCH 2/4] (v4) On-demand locations within string-literals
On 9/10/2021 1:48 AM, Thomas Schwinge wrote:
Hi!
Ping. My patches again attached, for easy reference.
Grüße
Thomas
On 2021-09-03T18:33:37+0200, I wrote:
Hi!
On 2021-09-02T21:09:54+0200, I wrote:
On 2021-09-02T15:59:14+0200, I wrote:
On 2016-08-05T14:16:58-0400, David Malcolm wrote:
Committed to trunk as r239175; I'm attaching the final version of the
patch for reference.
David, you've added here 'gcc/input.h:struct location_hash' (see quoted
below), which will be useful elsewhere, so:
--- a/gcc/input.c
+++ b/gcc/input.c
+/* Internal function. Canonicalize LOC into a form suitable for
+ use as a key within the database, stripping away macro expansion,
+ ad-hoc information, and range information, using the location of
+ the start of LOC within an ordinary linemap. */
+
+location_t
+string_concat_db::get_key_loc (location_t loc)
+{
+ loc = linemap_resolve_location (line_table, loc, LRK_SPELLING_LOCATION,
+ NULL);
+
+ loc = get_range_from_loc (line_table, loc).m_start;
+
+ return loc;
+}
OK to push the attached
"Harden 'gcc/input.c:string_concat_db::get_key_loc'"? (This fell out of
my analysis for development work elsewhere.)
My suggested patch was:
--- a/gcc/input.c
+++ b/gcc/input.c
@@ -1483,6 +1483,9 @@ string_concat_db::get_key_loc (location_t loc)
loc = get_range_from_loc (line_table, loc).m_start;
+ /* Ascertain that 'loc' is valid as a key in 'm_table'. */
+ gcc_checking_assert (!RESERVED_LOCATION_P (loc));
+
return loc;
}
Uh, I should've looked at the correct test logs... This change actually
does regress 'c-c++-common/substring-location-PR-87721.c' and
'gcc.dg/plugin/diagnostic-test-string-literals-1.c': for these, we do see
'BUILTINS_LOCATION' (via 'string_concat_db::record_string_concatenation').
Unless someone tell me that's unexpected (I'm completely lost in this
code...)
I think I convinced myself that the current code doesn't have stable
behavior, so...
I shall change/generalize my changes to provide both a
'location_hash' only using 'UNKNOWN_LOCATION' as a spare value for
'Empty' (as currently used here) and another variant additionally using
'BUILTINS_LOCATION' as spare value for 'Deleted'.
... I didn't do this, but instead would like to push the attached
"Don't record string concatenation data for 'RESERVED_LOCATION_P'"
(replacing "Harden 'gcc/input.c:string_concat_db::get_key_loc'" as
originally proposed). OK?
... and then re:
--- a/gcc/input.h
+++ b/gcc/input.h
+struct location_hash : int_hash { };
+
+class GTY(()) string_concat_db
+{
+[...]
+ hash_map *m_table;
+};
OK to push the attached
"Generalize 'gcc/input.h:struct location_hash'"?
Attached again.
Grüße
Thomas
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
München, HRB 106955
0001-Don-t-record-string-concatenation-data-for-RESERVED_.patch
From 9f1066fcb770397d6e791aa0594f067a755e2ed6 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge
Date: Fri, 3 Sep 2021 18:25:10 +0200
Subject: [PATCH] Don't record string concatenation data for
'RESERVED_LOCATION_P'
'RESERVED_LOCATION_P' means 'UNKNOWN_LOCATION' or 'BUILTINS_LOCATION.
We're using 'UNKNOWN_LOCATION' as a spare value for 'Empty', so should
ascertain that we don't use it as a key additionally. Similarly for
'BUILTINS_LOCATION' that we'd later like to use as a spare value for
'Deleted'.
As discussed in the source code comment added, for these we didn't have
stable behavior anyway.
Follow-up to r239175 (commit 88faa309e5d6c6171b957daaf2f800920869)
"On-demand locations within string-literals".
gcc/
* input.c (string_concat_db::record_string_concatenation)
(string_concat_db::get_string_concatenation): Skip for
'RESERVED_LOCATION_P'.
gcc/testsuite/
* gcc.dg/plugin/diagnostic-test-string-literals-1.c: Adjust
expected error diagnostics.
OK
jeff
Re: [PATCH 1/2] Add gimple_truth_valued_p to match.pd and use it
On 8/13/2021 6:19 PM, apinski--- via Gcc-patches wrote:
From: Andrew Pinski
While working on some more boolean optimizations, I noticed
that there are places which does SSA_NAME@0 and then look
at then either use get_nonzero_bits or ssa_name_has_boolean_range
to see if the ssa name had a boolean range. This cleans this
up slightly by have a simple match pattern call gimple_truth_valued_p
which matches on SSA_NAME and checks ssa_name_has_boolean_range.
This is the first of the few cleanups I am going to do for
match and simplify and boolean related changes.
gcc/ChangeLog:
* match.pd: New match, gimple_truth_valued_p.
Use it for "{ 0 or 1 } * { 0 or 1 }",
"X / bool_range_Y", and "-(type)!A" simplifcations.
OK
jeff
Re: [PATCH 2/2] Fix 101805: Simplify min/max of boolean arguments
On 8/13/2021 6:19 PM, apinski--- via Gcc-patches wrote: From: Andrew Pinski I noticed this while Richard B. fixing PR101756. Basically min of two bools is the same as doing an "and" and max of two bools is doing an "ior". gcc/ChangeLog: * match.pd: Add min/max patterns for bool types. gcc/testsuite/ChangeLog: * gcc.dg/tree-ssa/bool-12.c: New test. OK jeff
