Go patch committed: Improve type handling for constant string concatenation
This patch to the Go frontend by Than McIntosh improves type handling
for string concat ops on constants. This resolves a small problem
with concatenation of string constants: in a string concat X + Y where
X has named type and Y has abstract string type, ensure that the
result has X's type, and disable folding if the both sides have a
concrete type that does not match. This fixes
https://golang.org/issue/31412. Bootstrapped and ran Go testsuite on
x86_64-pc-linux-gnu. Committed to mainline.
Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 270263)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-8822487ed776d55eafed44de7d89ee54bbfbab47
+20010e494f46d8fd58cfd372093b059578d3379a
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
Index: gcc/go/gofrontend/expressions.cc
===
--- gcc/go/gofrontend/expressions.cc(revision 270263)
+++ gcc/go/gofrontend/expressions.cc(working copy)
@@ -1846,12 +1846,20 @@ String_expression::do_dump_expression(As
String_expression::export_string(ast_dump_context, this);
}
-// Make a string expression.
+// Make a string expression with abstract string type (common case).
Expression*
Expression::make_string(const std::string& val, Location location)
{
- return new String_expression(val, location);
+ return new String_expression(val, NULL, location);
+}
+
+// Make a string expression with a specific string type.
+
+Expression*
+Expression::make_string_typed(const std::string& val, Type* type, Location
location)
+{
+ return new String_expression(val, type, location);
}
// An expression that evaluates to some characteristic of a string.
@@ -5485,7 +5493,16 @@ Binary_expression::do_lower(Gogo* gogo,
}
// String constant expressions.
- if (left->type()->is_string_type() && right->type()->is_string_type())
+ //
+ // Avoid constant folding here if the left and right types are incompatible
+ // (leave the operation intact so that the type checker can complain about it
+ // later on). If concatenating an abstract string with a named string type,
+ // result type needs to be of the named type (see issue 31412).
+ if (left->type()->is_string_type()
+ && right->type()->is_string_type()
+ && (left->type()->named_type() == NULL
+ || right->type()->named_type() == NULL
+ || left->type()->named_type() == right->type()->named_type()))
{
std::string left_string;
std::string right_string;
@@ -5493,8 +5510,13 @@ Binary_expression::do_lower(Gogo* gogo,
&& right->string_constant_value(_string))
{
if (op == OPERATOR_PLUS)
- return Expression::make_string(left_string + right_string,
- location);
+{
+ Type* result_type = (left->type()->named_type() != NULL
+ ? left->type()
+ : right->type());
+ return Expression::make_string_typed(left_string + right_string,
+ result_type, location);
+}
else if (is_comparison)
{
int cmp = left_string.compare(right_string);
Index: gcc/go/gofrontend/expressions.h
===
--- gcc/go/gofrontend/expressions.h (revision 270263)
+++ gcc/go/gofrontend/expressions.h (working copy)
@@ -230,6 +230,10 @@ class Expression
static Expression*
make_string(const std::string&, Location);
+ // Make a constant string expression with a specific string subtype.
+ static Expression*
+ make_string_typed(const std::string&, Type*, Location);
+
// Make an expression that evaluates to some characteristic of an string.
// For simplicity, the enum values must match the field indexes in the
// underlying struct.
@@ -1570,9 +1574,9 @@ class Set_and_use_temporary_expression :
class String_expression : public Expression
{
public:
- String_expression(const std::string& val, Location location)
+ String_expression(const std::string& val, Type* type, Location location)
: Expression(EXPRESSION_STRING, location),
- val_(val), type_(NULL)
+ val_(val), type_(type)
{ }
const std::string&
Re: [PATCH] Uglify identifiers missed in previous commit(s)
Tested x86_64-linux, committed to trunk. Jonathan Wakely writes: > On 11/04/19 21:15 -0700, Thomas Rodgers wrote: >> >> * include/pstl/algorithm_impl.h: Uglify identfiers. >> * include/pstl/numeric_impl.h: Uglify identfiers. >> * include/pstl/parallel_backend_tbb.h: Uglify identfiers. > > OK for trunk, thanks.
Re: [PATCH] fix ICEs in c-attribs.c (PR 88383, 89288, 89798, 89797)
On 4/12/19 3:42 PM, Jakub Jelinek wrote: On Fri, Apr 12, 2019 at 10:45:25AM -0600, Jeff Law wrote: gcc/ChangeLog: PR c/89797 * targhooks.c (default_vector_alignment): Avoid assuming argument fits in SHWI. * tree.h (TYPE_VECTOR_SUBPARTS): Avoid sign overflow in a shift expression. gcc/c-family/ChangeLog: PR c/88383 PR c/89288 PR c/89798 PR c/89797 * c-attribs.c (type_valid_for_vector_size): Detect excessively large sizes. ... Has the patch been tested at all? A few times. The c-attribs.c change above didn't make it into the commit. Martin
Re: [PATCH] (RFA tree-tailcall) PR c++/82081 - tail call optimization breaks noexcept
On 4/12/19 3:24 PM, Jason Merrill wrote: > If a noexcept function calls a function that might throw, doing the tail > call optimization means that an exception thrown in the called function > will propagate out, breaking the noexcept specification. So we need to > prevent the optimization in that case. > > Tested x86_64-pc-linux-gnu. OK for trunk or hold for GCC 10? This isn't a > regression, but it is a straightforward fix for a wrong-code bug. > > * tree-tailcall.c (find_tail_calls): Don't turn a call from a > nothrow function to a might-throw function into a tail call. I'd go on the trunk. It's a wrong-code issue, what we're doing is just plain wrong. One could even make a case for backporting to the branches. jeff ps. I'm a bit surprised it hasn't been reported until now.
[PATCH, libphobos] Committed add subdir-objects to configure script.
Hi,
Now that libz_convenience.a is used, it's possible to add
subdir-objects. Running autoreconf regenerates all files cleanly with
autoconf2.69.
Bootstrapped and regression tested on x86_64-linux-gnu.
Committed to trunk as r270330.
--
Iain
---
libphobos/ChangeLog:
2019-04-12 Iain Buclaw
* configure.ac (AM_INIT_AUTOMAKE): Add subdir-objects.
* configure: Regenerate.
* libdruntime/Makefile.in: Regenerate.
---
diff --git a/libphobos/configure b/libphobos/configure
index ead96d11bb4..48b78eb56dd 100755
--- a/libphobos/configure
+++ b/libphobos/configure
@@ -2554,6 +2554,7 @@ target_alias=${target_alias-$target}
# no-dependencies: Don't generate automatic dependencies.
#(because it breaks when using bootstrap-lean, since some of the
#headers are gone at "make install" time).
+# subdir-objects: Build objects in sub-directories.
# -Wall: Issue all automake warnings.
# -Wno-portability: Don't warn about constructs supported by GNU make.
#(because GCC requires GNU make anyhow).
@@ -11494,7 +11495,7 @@ else
lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
lt_status=$lt_dlunknown
cat > conftest.$ac_ext <<_LT_EOF
-#line 11497 "configure"
+#line 11498 "configure"
#include "confdefs.h"
#if HAVE_DLFCN_H
@@ -11600,7 +11601,7 @@ else
lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
lt_status=$lt_dlunknown
cat > conftest.$ac_ext <<_LT_EOF
-#line 11603 "configure"
+#line 11604 "configure"
#include "confdefs.h"
#if HAVE_DLFCN_H
diff --git a/libphobos/configure.ac b/libphobos/configure.ac
index 362f7537ffb..f1ab556bec0 100644
--- a/libphobos/configure.ac
+++ b/libphobos/configure.ac
@@ -42,11 +42,12 @@ AC_SUBST(target_alias)
# no-dependencies: Don't generate automatic dependencies.
#(because it breaks when using bootstrap-lean, since some of the
#headers are gone at "make install" time).
+# subdir-objects: Build objects in sub-directories.
# -Wall: Issue all automake warnings.
# -Wno-portability: Don't warn about constructs supported by GNU make.
#(because GCC requires GNU make anyhow).
# -Wno-override: Overrides used in testsuite.
-AM_INIT_AUTOMAKE([1.11.1 foreign no-dist no-define no-dependencies -Wall -Wno-portability -Wno-override])
+AM_INIT_AUTOMAKE([1.11.1 foreign no-dist no-define no-dependencies subdir-objects -Wall -Wno-portability -Wno-override])
m4_rename([_AC_ARG_VAR_PRECIOUS],[glibd_PRECIOUS])
m4_define([_AC_ARG_VAR_PRECIOUS],[])
diff --git a/libphobos/libdruntime/Makefile.in b/libphobos/libdruntime/Makefile.in
index fe95436c118..4bfd04c3db5 100644
--- a/libphobos/libdruntime/Makefile.in
+++ b/libphobos/libdruntime/Makefile.in
@@ -425,9 +425,9 @@ am__objects_27 = $(am__objects_1) $(am__objects_3) $(am__objects_5) \
$(am__objects_13) $(am__objects_15) $(am__objects_17) \
$(am__objects_19) $(am__objects_21) $(am__objects_23) \
$(am__objects_25) $(am__objects_26)
-am__objects_28 = libgdruntime_la-errno_.lo \
- libgdruntime_la-bss_section.lo
-am__objects_29 = libgdruntime_la-threadasm.lo
+am__objects_28 = core/stdc/libgdruntime_la-errno_.lo \
+ rt/libgdruntime_la-bss_section.lo
+am__objects_29 = core/libgdruntime_la-threadasm.lo
am__objects_30 = $(am__objects_27) $(am__objects_28) $(am__objects_29)
am_libgdruntime_la_OBJECTS = $(am__objects_30)
libgdruntime_la_OBJECTS = $(am_libgdruntime_la_OBJECTS)
@@ -1626,6 +1626,9 @@ core/sys/solaris/sys/types.lo: core/sys/solaris/sys/$(am__dirstamp)
core/sys/solaris/time.lo: core/sys/solaris/$(am__dirstamp)
gcc/config.lo: gcc/$(am__dirstamp)
gcc/libbacktrace.lo: gcc/$(am__dirstamp)
+core/stdc/libgdruntime_la-errno_.lo: core/stdc/$(am__dirstamp)
+rt/libgdruntime_la-bss_section.lo: rt/$(am__dirstamp)
+core/libgdruntime_la-threadasm.lo: core/$(am__dirstamp)
libgdruntime.la: $(libgdruntime_la_OBJECTS) $(libgdruntime_la_DEPENDENCIES) $(EXTRA_libgdruntime_la_DEPENDENCIES)
$(AM_V_GEN)$(libgdruntime_la_LINK) -rpath $(toolexeclibdir) $(libgdruntime_la_OBJECTS) $(libgdruntime_la_LIBADD) $(LIBS)
@@ -1725,8 +1728,8 @@ distclean-compile:
.S.lo:
$(AM_V_CPPAS)$(LTCPPASCOMPILE) -c -o $@ $<
-libgdruntime_la-threadasm.lo: core/threadasm.S
- $(AM_V_CPPAS)$(LIBTOOL) $(AM_V_lt) $(libgdruntime_la_LIBTOOLFLAGS) $(LIBTOOLFLAGS) --mode=compile $(CCAS) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(AM_CCASFLAGS) $(CCASFLAGS) -c -o libgdruntime_la-threadasm.lo `test -f 'core/threadasm.S' || echo '$(srcdir)/'`core/threadasm.S
+core/libgdruntime_la-threadasm.lo: core/threadasm.S
+ $(AM_V_CPPAS)$(LIBTOOL) $(AM_V_lt) $(libgdruntime_la_LIBTOOLFLAGS) $(LIBTOOLFLAGS) --mode=compile $(CCAS) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(AM_CCASFLAGS) $(CCASFLAGS) -c -o core/libgdruntime_la-threadasm.lo `test -f 'core/threadasm.S' || echo '$(srcdir)/'`core/threadasm.S
.c.o:
$(AM_V_CC)$(COMPILE) -c -o $@ $<
@@ -1737,11 +1740,11 @@ libgdruntime_la-threadasm.lo: core/threadasm.S
.c.lo:
$(AM_V_CC)$(LTCOMPILE) -c -o $@ $<
Re: [PATCH] fix ICEs in c-attribs.c (PR 88383, 89288, 89798, 89797)
On Fri, Apr 12, 2019 at 11:42:59PM +0200, Jakub Jelinek wrote:
> > With a suitable doc change of that nature, this is OK.
>
> Has the patch been tested at all?
> I see many new FAILs after the patch:
> make check-gcc check-c++-all RUNTESTFLAGS="--target_board=unix\{-m32,-m64\}
> dg.exp='attributes-1.c builtin-has-attribute-*.c pr71574.c attr-vector_size.c
> pr25559.c'"
> FAIL: gcc.dg/attr-vector_size.c LP64 (test for errors, line 33)
> FAIL: gcc.dg/attr-vector_size.c LP64 (test for errors, line 60)
> FAIL: gcc.dg/attr-vector_size.c (test for excess errors)
> FAIL: gcc.dg/pr25559.c (test for errors, line 5)
> FAIL: gcc.dg/pr25559.c (test for excess errors)
> FAIL: c-c++-common/attributes-1.c -Wc++-compat (test for errors, line 17)
> FAIL: c-c++-common/attributes-1.c -Wc++-compat (test for excess errors)
> FAIL: c-c++-common/builtin-has-attribute-4.c -Wc++-compat (test for excess
> errors)
> FAIL: c-c++-common/builtin-has-attribute-6.c -Wc++-compat (internal
> compiler error)
> FAIL: c-c++-common/builtin-has-attribute-6.c -Wc++-compat (test for excess
> errors)
> FAIL: c-c++-common/builtin-has-attribute-7.c -Wc++-compat (internal
> compiler error)
> FAIL: c-c++-common/builtin-has-attribute-7.c -Wc++-compat (test for excess
> errors)
> FAIL: c-c++-common/pr71574.c -Wc++-compat (test for errors, line 14)
> FAIL: c-c++-common/pr71574.c -Wc++-compat (test for excess errors)
> for -m64,
> FAIL: gcc.dg/attr-vector_size.c (internal compiler error)
> FAIL: gcc.dg/attr-vector_size.c ILP32 (test for errors, line 37)
> FAIL: gcc.dg/attr-vector_size.c ILP32 (test for errors, line 39)
> FAIL: gcc.dg/attr-vector_size.c ILP32 (test for errors, line 64)
> FAIL: gcc.dg/attr-vector_size.c ILP32 (test for errors, line 66)
> FAIL: gcc.dg/attr-vector_size.c (test for excess errors)
> FAIL: gcc.dg/pr25559.c (test for errors, line 5)
> FAIL: gcc.dg/pr25559.c (test for excess errors)
> FAIL: c-c++-common/attributes-1.c -Wc++-compat (test for errors, line 17)
> FAIL: c-c++-common/attributes-1.c -Wc++-compat (test for excess errors)
> FAIL: c-c++-common/builtin-has-attribute-4.c -Wc++-compat (test for excess
> errors)
> FAIL: c-c++-common/builtin-has-attribute-6.c -Wc++-compat (internal
> compiler error)
> FAIL: c-c++-common/builtin-has-attribute-6.c -Wc++-compat (test for excess
> errors)
> FAIL: c-c++-common/builtin-has-attribute-7.c -Wc++-compat (internal
> compiler error)
> FAIL: c-c++-common/builtin-has-attribute-7.c -Wc++-compat (test for excess
> errors)
> FAIL: c-c++-common/pr71574.c -Wc++-compat (test for errors, line 14)
> FAIL: c-c++-common/pr71574.c -Wc++-compat (test for excess errors)
> for -m32, and quite similar set for C++.
And
https://gcc.gnu.org/ml/gcc-testresults/2019-04/msg01416.html
confirms that too.
Jakub
Re: [PATCH] fix ICEs in c-attribs.c (PR 88383, 89288, 89798, 89797)
On Fri, Apr 12, 2019 at 10:45:25AM -0600, Jeff Law wrote:
> > gcc/ChangeLog:
> >
> > PR c/89797
> > * targhooks.c (default_vector_alignment): Avoid assuming
> > argument fits in SHWI.
> > * tree.h (TYPE_VECTOR_SUBPARTS): Avoid sign overflow in
> > a shift expression.
> >
> > gcc/c-family/ChangeLog:
> >
> > PR c/88383
> > PR c/89288
> > PR c/89798
> > PR c/89797
> > * c-attribs.c (type_valid_for_vector_size): Detect excessively
> > large sizes.
> > (validate_attribute): Handle DECLs and expressions.
> > (has_attribute): Handle types referenced by expressions.
> > Avoid considering array attributes in ARRAY_REF expressions .
> >
> > gcc/cp/ChangeLog:
> >
> > PR c/88383
> > PR c/89288
> > * parser.c (cp_parser_has_attribute_expression): Handle assignment
> > expressions.
> >
> > gcc/testsuite/ChangeLog:
> >
> > PR c/88383
> > PR c/89288
> > PR c/89798
> > PR c/89797
> > * c-c++-common/attributes-1.c: Adjust.
> > * c-c++-common/builtin-has-attribute-6.c: New test.
> > * c-c++-common/builtin-has-attribute-7.c: New test.
> > * c-c++-common/builtin-has-attribute-4.c: Adjust expectations.
> > * c-c++-common/builtin-has-attribute-6.c: New test.
New test twice for the same test?
> > * gcc.dg/pr25559.c: Adjust.
> > * gcc.dg/attr-vector_size.c: New test.
> Per our offline discussion, I think this is OK with some documentation
> updates to clarify expression handling.
>
> In particular can you adjust the docs to more clearly indicate that for
> an expression, we look at the attributes of the type of the expression.
>
> With a suitable doc change of that nature, this is OK.
Has the patch been tested at all?
I see many new FAILs after the patch:
make check-gcc check-c++-all RUNTESTFLAGS="--target_board=unix\{-m32,-m64\}
dg.exp='attributes-1.c builtin-has-attribute-*.c pr71574.c attr-vector_size.c
pr25559.c'"
FAIL: gcc.dg/attr-vector_size.c LP64 (test for errors, line 33)
FAIL: gcc.dg/attr-vector_size.c LP64 (test for errors, line 60)
FAIL: gcc.dg/attr-vector_size.c (test for excess errors)
FAIL: gcc.dg/pr25559.c (test for errors, line 5)
FAIL: gcc.dg/pr25559.c (test for excess errors)
FAIL: c-c++-common/attributes-1.c -Wc++-compat (test for errors, line 17)
FAIL: c-c++-common/attributes-1.c -Wc++-compat (test for excess errors)
FAIL: c-c++-common/builtin-has-attribute-4.c -Wc++-compat (test for excess
errors)
FAIL: c-c++-common/builtin-has-attribute-6.c -Wc++-compat (internal compiler
error)
FAIL: c-c++-common/builtin-has-attribute-6.c -Wc++-compat (test for excess
errors)
FAIL: c-c++-common/builtin-has-attribute-7.c -Wc++-compat (internal compiler
error)
FAIL: c-c++-common/builtin-has-attribute-7.c -Wc++-compat (test for excess
errors)
FAIL: c-c++-common/pr71574.c -Wc++-compat (test for errors, line 14)
FAIL: c-c++-common/pr71574.c -Wc++-compat (test for excess errors)
for -m64,
FAIL: gcc.dg/attr-vector_size.c (internal compiler error)
FAIL: gcc.dg/attr-vector_size.c ILP32 (test for errors, line 37)
FAIL: gcc.dg/attr-vector_size.c ILP32 (test for errors, line 39)
FAIL: gcc.dg/attr-vector_size.c ILP32 (test for errors, line 64)
FAIL: gcc.dg/attr-vector_size.c ILP32 (test for errors, line 66)
FAIL: gcc.dg/attr-vector_size.c (test for excess errors)
FAIL: gcc.dg/pr25559.c (test for errors, line 5)
FAIL: gcc.dg/pr25559.c (test for excess errors)
FAIL: c-c++-common/attributes-1.c -Wc++-compat (test for errors, line 17)
FAIL: c-c++-common/attributes-1.c -Wc++-compat (test for excess errors)
FAIL: c-c++-common/builtin-has-attribute-4.c -Wc++-compat (test for excess
errors)
FAIL: c-c++-common/builtin-has-attribute-6.c -Wc++-compat (internal compiler
error)
FAIL: c-c++-common/builtin-has-attribute-6.c -Wc++-compat (test for excess
errors)
FAIL: c-c++-common/builtin-has-attribute-7.c -Wc++-compat (internal compiler
error)
FAIL: c-c++-common/builtin-has-attribute-7.c -Wc++-compat (test for excess
errors)
FAIL: c-c++-common/pr71574.c -Wc++-compat (test for errors, line 14)
FAIL: c-c++-common/pr71574.c -Wc++-compat (test for excess errors)
for -m32, and quite similar set for C++.
Jakub
[PATCH] (RFA tree-tailcall) PR c++/82081 - tail call optimization breaks noexcept
Jason wrote: >If a noexcept function calls a function that might throw, doing the tail call optimization means that an exception thrown in the called function will propagate out, breaking the noexcept specification. So we need to prevent the optimization in that case. >Tested x86_64-pc-linux-gnu. OK for trunk or hold for GCC 10? This isn't a regression, but it is a straightforward fix for a wrong-code bug. It is indeed not a regression, but exceptions escaping through a noexcept are really unfortunate.
[PATCH] (RFA tree-tailcall) PR c++/82081 - tail call optimization breaks noexcept
If a noexcept function calls a function that might throw, doing the tail
call optimization means that an exception thrown in the called function
will propagate out, breaking the noexcept specification. So we need to
prevent the optimization in that case.
Tested x86_64-pc-linux-gnu. OK for trunk or hold for GCC 10? This isn't a
regression, but it is a straightforward fix for a wrong-code bug.
* tree-tailcall.c (find_tail_calls): Don't turn a call from a
nothrow function to a might-throw function into a tail call.
---
gcc/tree-tailcall.c | 7 +++
gcc/testsuite/g++.dg/tree-ssa/tail-call-1.C | 11 +++
gcc/ChangeLog | 6 ++
3 files changed, 24 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/tree-ssa/tail-call-1.C
diff --git a/gcc/tree-tailcall.c b/gcc/tree-tailcall.c
index afe8931b5f0..e0265b22dd5 100644
--- a/gcc/tree-tailcall.c
+++ b/gcc/tree-tailcall.c
@@ -37,6 +37,7 @@ along with GCC; see the file COPYING3. If not see
#include "tree-into-ssa.h"
#include "tree-dfa.h"
#include "except.h"
+#include "tree-eh.h"
#include "dbgcnt.h"
#include "cfgloop.h"
#include "common/common-target.h"
@@ -472,6 +473,12 @@ find_tail_calls (basic_block bb, struct tailcall **ret)
&& !auto_var_in_fn_p (ass_var, cfun->decl))
return;
+ /* If the call might throw an exception that wouldn't propagate out of
+ cfun, we can't transform to a tail or sibling call (82081). */
+ if (stmt_could_throw_p (cfun, stmt)
+ && !stmt_can_throw_external (cfun, stmt))
+return;
+
/* We found the call, check whether it is suitable. */
tail_recursion = false;
func = gimple_call_fndecl (call);
diff --git a/gcc/testsuite/g++.dg/tree-ssa/tail-call-1.C
b/gcc/testsuite/g++.dg/tree-ssa/tail-call-1.C
new file mode 100644
index 000..c67af6e41c7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/tail-call-1.C
@@ -0,0 +1,11 @@
+// PR c++/82081
+// { dg-do compile { target c++11 } }
+// { dg-additional-options "-O2 -fdump-tree-optimized" }
+// { dg-final { scan-tree-dump-not "tail call" "optimized" } }
+
+int g(int) ;
+
+int f() noexcept {
+int i = 42, j = 43;
+return g(i+j);
+}
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 02d3d07d0e4..77ab20929ff 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@
+2019-04-11 Jason Merrill
+
+ PR c++/82081 - tail call optimization breaks noexcept
+ * tree-tailcall.c (find_tail_calls): Don't turn a call from a
+ nothrow function to a might-throw function into a tail call.
+
2019-04-12 Jakub Jelinek
PR rtl-optimization/89965
base-commit: b7a39acf193fd19e41502d5e2cf6b3fa8b44dbac
--
2.20.1
Re: [C/C++ PATCH] Fix ICE with typedef redeclaration with attributes (PR c/89933, take 2)
On 4/12/19 3:56 PM, Jakub Jelinek wrote: On Fri, Apr 12, 2019 at 02:46:19PM -0400, Jason Merrill wrote: On 4/12/19 3:19 AM, Jakub Jelinek wrote: In r234626 Marek has added code to remove TREE_TYPE (newdecl) from its variant list for typedefs, because we'll ggc_free the TYPE_NAME of that type. Unfortunately, that code will ICE if TREE_TYPE (newdecl) is its own TYPE_MAIN_VARIANT. This can happen if due to attributes the newdecl's type has been build_distinct_type_copy created but hasn't been type_hash_canon merged (which we do for a few attributes like aligned, but certainly don't do it for most other attributes). In the likely case there are no variants for that type yet, there is just nothing to remove. If there are some (in theory), the options are do what the following patch does, keep the type in the variant list as the main variant, just change the TYPE_NAME, so that it doesn't refer to ggc_freed TYPE_DECL, or try to pick up some other type as the main variant and adjust the whole variant list (I don't think the C/C++ FEs unlike Ada FE like e.g. qualified types as main variant though). If the typedef is a duplicate, the type ought to be garbage as well, no? I'd hope so. So, would you prefer this instead, where instead of changing the TYPE_NAME of the type we assert it has no variants? I was just worried there might be variants, but duplicate_decls should happen immediately on the typedef, so there shouldn't be time to create the variants. Tried typedef const unsigned int a __attribute__ ((__aligned__(8), __may_alias__)); typedef const unsigned int a __attribute__ ((__aligned__(8), __may_alias__)); and it works too. Yes, let's go with this. Jason
Re: [PATCH 0/3] OpenRISC floating point support + fixes
On Fri, Apr 12, 2019 at 09:28:55AM -0600, Jeff Law wrote: > On 4/10/19 3:27 PM, Stafford Horne wrote: > > Hello, > > > > This is a set of patches to bring FPU support to the OpenRISC backend. The > > backend also add support for 64-bit floating point operations on 32-bit > > cores > > using register pairs, see orfpx64a32 [0]. > > > > This depends on binutils patches which have also been submitted per review. > > [1] > > > > The toolchain has been tested using the gcc and binutils testsuites as well > > as > > floating point test suites running on sim and an fpga soft core > > or1k_marocchino. > > [2] > > > > There is also an unrelated, but trivial patch to fix a code quality issue > > with > > volatile memory loads. > > > > This whole patch series can be found on my github repo [3] as well. > > > > -Stafford > > > > [0] https://openrisc.io/proposals/orfpx64a32 > > [1] g...@github.com:stffrdhrn/binutils-gdb.git orfpx64a32-2 > > [2] https://github.com/openrisc/or1k_marocchino > > [3] g...@github.com:stffrdhrn/gcc.git or1k-fpu-1a > > > > Stafford Horne (3): > > or1k: Initial support for FPU > > or1k: Allow volatile memory for sign/zero extend loads > > or1k: only force reg for immediates > > > > gcc/config.gcc| 1 + > > gcc/config/or1k/or1k.c| 10 ++-- > > gcc/config/or1k/or1k.md | 109 -- > > gcc/config/or1k/or1k.opt | 15 - > > gcc/config/or1k/predicates.md | 16 + > > gcc/doc/invoke.texi | 15 + > > 6 files changed, 156 insertions(+), 10 deletions(-) > > > So the only question is whether or not you're looking to drop this into > gcc-9 or gcc-10. > > gcc-9 is in regression bugfixing stage, so technically this patch should > wait to gcc-10. However, we usually give maintainers a degree of > freedom, particularly if a change doens't "bleed" into generic parts of > the compiler. > > So we'll leave it up to you to decide if you want to add this to gcc-9 > or wait for gcc-10. Hi Jeff, Thanks, I know I sent all the patches together for review. But, I am thinking for gcc-9 I will only be aiming at including 2/3. The others are only needed for FPU support and I can wait for gcc-10. We had a quick discussion about this on irc yesterday too. -Stafford
[C/C++ PATCH] Fix ICE with typedef redeclaration with attributes (PR c/89933, take 2)
On Fri, Apr 12, 2019 at 02:46:19PM -0400, Jason Merrill wrote:
> On 4/12/19 3:19 AM, Jakub Jelinek wrote:
> > In r234626 Marek has added code to remove TREE_TYPE (newdecl) from
> > its variant list for typedefs, because we'll ggc_free the TYPE_NAME of that
> > type. Unfortunately, that code will ICE if TREE_TYPE (newdecl) is its own
> > TYPE_MAIN_VARIANT. This can happen if due to attributes the newdecl's type
> > has been build_distinct_type_copy created but hasn't been type_hash_canon
> > merged (which we do for a few attributes like aligned, but certainly don't
> > do it for most other attributes). In the likely case there are no variants
> > for that type yet, there is just nothing to remove. If there are some (in
> > theory), the options are do what the following patch does, keep the type
> > in the variant list as the main variant, just change the TYPE_NAME, so that
> > it doesn't refer to ggc_freed TYPE_DECL, or try to pick up some other type
> > as the main variant and adjust the whole variant list (I don't think the
> > C/C++ FEs unlike Ada FE like e.g. qualified types as main variant though).
>
> If the typedef is a duplicate, the type ought to be garbage as well, no?
I'd hope so.
So, would you prefer this instead, where instead of changing the TYPE_NAME
of the type we assert it has no variants? I was just worried there might be
variants, but duplicate_decls should happen immediately on the typedef, so
there shouldn't be time to create the variants.
Tried
typedef const unsigned int a __attribute__ ((__aligned__(8), __may_alias__));
typedef const unsigned int a __attribute__ ((__aligned__(8), __may_alias__));
and it works too.
2019-04-12 Jakub Jelinek
PR c/89933
c/
* c-decl.c (merge_decls): When newdecl's type is its main variant,
don't try to remove it from the variant list, but instead assert
it has no variants.
cp/
* decl.c (duplicate_decls): When newdecl's type is its main variant,
don't try to remove it from the variant list, but instead assert
it has no variants.
testsuite/
* c-c++-common/pr89933.c: New test.
--- gcc/c/c-decl.c.jj 2019-04-11 17:25:08.873026245 +0200
+++ gcc/c/c-decl.c 2019-04-12 21:48:15.815779025 +0200
@@ -2512,13 +2512,16 @@ merge_decls (tree newdecl, tree olddecl,
if (TYPE_NAME (TREE_TYPE (newdecl)) == newdecl)
{
tree remove = TREE_TYPE (newdecl);
- for (tree t = TYPE_MAIN_VARIANT (remove); ;
- t = TYPE_NEXT_VARIANT (t))
- if (TYPE_NEXT_VARIANT (t) == remove)
- {
- TYPE_NEXT_VARIANT (t) = TYPE_NEXT_VARIANT (remove);
- break;
- }
+ if (TYPE_MAIN_VARIANT (remove) == remove)
+ gcc_assert (TYPE_NEXT_VARIANT (remove) == NULL_TREE);
+ else
+ for (tree t = TYPE_MAIN_VARIANT (remove); ;
+t = TYPE_NEXT_VARIANT (t))
+ if (TYPE_NEXT_VARIANT (t) == remove)
+ {
+ TYPE_NEXT_VARIANT (t) = TYPE_NEXT_VARIANT (remove);
+ break;
+ }
}
}
--- gcc/cp/decl.c.jj2019-04-11 17:25:08.664029678 +0200
+++ gcc/cp/decl.c 2019-04-12 21:48:28.926563001 +0200
@@ -2132,13 +2132,16 @@ next_arg:;
if (TYPE_NAME (TREE_TYPE (newdecl)) == newdecl)
{
tree remove = TREE_TYPE (newdecl);
- for (tree t = TYPE_MAIN_VARIANT (remove); ;
- t = TYPE_NEXT_VARIANT (t))
- if (TYPE_NEXT_VARIANT (t) == remove)
- {
- TYPE_NEXT_VARIANT (t) = TYPE_NEXT_VARIANT (remove);
- break;
- }
+ if (TYPE_MAIN_VARIANT (remove) == remove)
+ gcc_assert (TYPE_NEXT_VARIANT (remove) == NULL_TREE);
+ else
+ for (tree t = TYPE_MAIN_VARIANT (remove); ;
+t = TYPE_NEXT_VARIANT (t))
+ if (TYPE_NEXT_VARIANT (t) == remove)
+ {
+ TYPE_NEXT_VARIANT (t) = TYPE_NEXT_VARIANT (remove);
+ break;
+ }
}
}
else if (merge_attr)
--- gcc/testsuite/c-c++-common/pr89933.c.jj 2019-04-12 21:47:46.189267166
+0200
+++ gcc/testsuite/c-c++-common/pr89933.c2019-04-12 21:47:46.189267166
+0200
@@ -0,0 +1,5 @@
+/* PR c/89933 */
+/* { dg-do compile } */
+
+typedef unsigned int a __attribute__ ((__aligned__(8), __may_alias__));
+typedef unsigned int a __attribute__ ((__aligned__(8), __may_alias__));
Jakub
Re: [C/C++ PATCH] Fix ICE with typedef redeclaration with attributes (PR c/89933)
On 4/12/19 3:19 AM, Jakub Jelinek wrote: In r234626 Marek has added code to remove TREE_TYPE (newdecl) from its variant list for typedefs, because we'll ggc_free the TYPE_NAME of that type. Unfortunately, that code will ICE if TREE_TYPE (newdecl) is its own TYPE_MAIN_VARIANT. This can happen if due to attributes the newdecl's type has been build_distinct_type_copy created but hasn't been type_hash_canon merged (which we do for a few attributes like aligned, but certainly don't do it for most other attributes). In the likely case there are no variants for that type yet, there is just nothing to remove. If there are some (in theory), the options are do what the following patch does, keep the type in the variant list as the main variant, just change the TYPE_NAME, so that it doesn't refer to ggc_freed TYPE_DECL, or try to pick up some other type as the main variant and adjust the whole variant list (I don't think the C/C++ FEs unlike Ada FE like e.g. qualified types as main variant though). If the typedef is a duplicate, the type ought to be garbage as well, no? Jason
Re: [C++ Patch/RFC] PR 89900 ("[9 Regression] ICE: Segmentation fault (in check_instantiated_arg)")
On 4/11/19 11:20 AM, Paolo Carlini wrote:
Hi,
over the last few days I spent some time on this regression, which at
first seemed just a minor error-recovery issue, but then I noticed that
very slightly tweeking the original testcase uncovered a pretty serious
ICE on valid:
template void
fk (XE..., int/*SW*/);
void
w9 (void)
{
fk (0);
}
The regression has to do with the changes committed by Jason for
c++/86932, in particular with the condition in coerce_template_parms:
if (template_parameter_pack_p (TREE_VALUE (parm))
&& (arg || !(complain & tf_partial))
&& !(arg && ARGUMENT_PACK_P (arg)))
which has the additional (arg || !complain & tf_partial)) false for the
present testcase, thus the null arg is not changed into an empty pack,
thus later instantiate_template calls check_instantiated_args which
finds it still null and crashes. Now, likely some additional analysis is
in order, but for sure there is an important difference between the
testcase which came with c++/86932 and the above: non-type vs type
template parameter pack. It seems to me that the kind of problem fixed
in c++/86932 cannot occur with type packs, because it boils down to a
reference to a previous parm (full disclosure: the comments and logic in
fixed_parameter_pack_p helped me a lot here). Thus I had the idea of
simply restricting the scope of the new condition above by adding an ||
TREE_CODE (TREE_VALUE (parm)) == TYPE_DECL, which definitely leads to a
clean testsuite and a proper behavior on the new testcases, AFAICS. I'm
attaching what I tested on x86_64-linux.
I think the important property here is that it's non-terminal, not that
it's a type pack. We can't deduce anything for a non-terminal pack, so
we should go ahead and make an empty pack.
Jason
Re: [PATCH] Fix PR88936
On Fri, 12 Apr 2019, Richard Biener wrote:
> On Fri, 12 Apr 2019, Michael Matz wrote:
>
> > Hi,
> >
> > On Fri, 12 Apr 2019, Richard Biener wrote:
> >
> > > > You miss PARM_DECLs and RESULT_DECLs, i.e. it's probably better to
> > > > factor
> > > > out tree.c:auto_var_in_fn_p and place the new auto_var_p in tree.c as
> > > > well.
> > >
> > > Hmm, I left the above unchanged from a different variant of the patch
> > > where for some reason I do not remember I explicitely decided
> > > parameters and results are not affected...
> >
> > Even if that were the case the function is sufficiently general (also its
> > name) that it should be generic infrastructure, not hidden away in
> > structalias.
>
> It was not fully equivalent, but yes. So - like the following?
> I think checking DECL_CONTEXT isn't necessary given the
> !DECL_EXTERNAL/STATIC checks.
>
> Bootstrap / regtest running on x86_64-unknown-linux-gnu.
Aww, hits
/space/rguenther/src/svn/trunk/libgomp/testsuite/libgomp.oacc-c/../libgomp.oacc-c-c++-common/zero_length_subarrays.c:33:1:
internal compiler error: in fold_builtin_alloca_with_align, at
tree-ssa-ccp.c:2186^M
0x6d7e45 fold_builtin_alloca_with_align^M
have to look/think about this.
Richard.
> Richard.
>
> 2019-04-12 Richard Biener
>
> PR ipa/88936
> * tree.h (auto_var_p): Declare.
> * tree.c (auto_var_p): New function, split out from ...
> (auto_var_in_fn_p): ... here.
> * tree-ssa-structalias.c (struct variable_info): Add shadow_var_uid
> member.
> (new_var_info): Initialize it.
> (set_uids_in_ptset): Also set the shadow variable uid if required.
> (ipa_pta_execute): Postprocess points-to solutions assigning
> shadow variable uids for locals that may reach their containing
> function recursively.
>
> * gcc.dg/torture/pr88936-1.c: New testcase.
> * gcc.dg/torture/pr88936-2.c: Likewise.
> * gcc.dg/torture/pr88936-3.c: Likewise.
>
> Index: gcc/tree.c
> ===
> --- gcc/tree.c(revision 270306)
> +++ gcc/tree.c(working copy)
> @@ -9268,17 +9268,25 @@ get_type_static_bounds (const_tree type,
> }
> }
>
> +/* Return true if VAR is an automatic variable. */
> +
> +bool
> +auto_var_p (const_tree var)
> +{
> + return VAR_P (var) && ! DECL_EXTERNAL (var))
> + || TREE_CODE (var) == PARM_DECL)
> +&& ! TREE_STATIC (var))
> + || TREE_CODE (var) == RESULT_DECL);
> +}
> +
> /* Return true if VAR is an automatic variable defined in function FN. */
>
> bool
> auto_var_in_fn_p (const_tree var, const_tree fn)
> {
>return (DECL_P (var) && DECL_CONTEXT (var) == fn
> - && VAR_P (var) && ! DECL_EXTERNAL (var))
> - || TREE_CODE (var) == PARM_DECL)
> -&& ! TREE_STATIC (var))
> - || TREE_CODE (var) == LABEL_DECL
> - || TREE_CODE (var) == RESULT_DECL));
> + && (auto_var_p (var)
> + || TREE_CODE (var) == LABEL_DECL));
> }
>
> /* Subprogram of following function. Called by walk_tree.
> Index: gcc/tree.h
> ===
> --- gcc/tree.h(revision 270306)
> +++ gcc/tree.h(working copy)
> @@ -4893,6 +4893,7 @@ extern bool stdarg_p (const_tree);
> extern bool prototype_p (const_tree);
> extern bool is_typedef_decl (const_tree x);
> extern bool typedef_variant_p (const_tree);
> +extern bool auto_var_p (const_tree);
> extern bool auto_var_in_fn_p (const_tree, const_tree);
> extern tree build_low_bits_mask (tree, unsigned);
> extern bool tree_nop_conversion_p (const_tree, const_tree);
> Index: gcc/tree-ssa-structalias.c
> ===
> --- gcc/tree-ssa-structalias.c(revision 270306)
> +++ gcc/tree-ssa-structalias.c(working copy)
> @@ -299,6 +299,11 @@ struct variable_info
>/* Full size of the base variable, in bits. */
>unsigned HOST_WIDE_INT fullsize;
>
> + /* In IPA mode the shadow UID in case the variable needs to be duplicated
> in
> + the final points-to solution because it reaches its containing
> + function recursively. Zero if none is needed. */
> + unsigned int shadow_var_uid;
> +
>/* Name of this variable */
>const char *name;
>
> @@ -397,6 +402,7 @@ new_var_info (tree t, const char *name,
>ret->solution = BITMAP_ALLOC (_obstack);
>ret->oldsolution = NULL;
>ret->next = 0;
> + ret->shadow_var_uid = 0;
>ret->head = ret->id;
>
>stats.total_vars++;
> @@ -6452,6 +6458,16 @@ set_uids_in_ptset (bitmap into, bitmap f
> && (TREE_STATIC (vi->decl) || DECL_EXTERNAL (vi->decl))
> && ! decl_binds_to_current_def_p (vi->decl))
> pt->vars_contains_interposable = true;
> +
> + /* If this is a local variable we can have overlapping lifetime
> + of different function invocations through
Re: [PATCH] fix ICEs in c-attribs.c (PR 88383, 89288, 89798, 89797)
On 4/2/19 3:19 PM, Martin Sebor wrote: > This is yet another follow up on the discussion of the fix for > the ICE in __builtin_has_attribute that started last December > with PR88383. > > After my last post last week I went and added more tests to make > sure the built-in behaves as intended by comparing its result for > non-trivial expressions with that of __alignof__. The test that > does this is the new builtin-has-attribute-7.c. > > The test exposed one problem in the handling of attribute vector_size > by the built-in (I mentioned that in my last post). It also exposed > a couple of bugs in the attribute handler itself. I fixed both of > these in the attached patch. > > This latest revision of the patch resolves the following bugs: > > PR 88383 - ICE calling __builtin_has_attribute on a reference > PR 89288 - ICE in tree_code_size, at tree.c:865 > PR 89798 - excessive vector_size silently accepted and truncated > PR 89797 - ICE on a vector_size (1LU << 33) int variable > > Bootstrapped on x86_64-linux. The tests are still running but > assuming they pass, is this last revision good to commit? > > Martin > > A link to my last comment in the archive: > https://gcc.gnu.org/ml/gcc-patches/2019-03/msg01096.html > > gcc-88383.diff > > PR c/88383 - ICE calling __builtin_has_attribute on a reference > PR c/89288 - ICE in tree_code_size, at tree.c:865 > PR c/89798 - excessive vector_size silently accepted and truncated > PR c/89797 - ICE on a vector_size (1LU << 33) int variable > > gcc/ChangeLog: > > PR c/89797 > * targhooks.c (default_vector_alignment): Avoid assuming > argument fits in SHWI. > * tree.h (TYPE_VECTOR_SUBPARTS): Avoid sign overflow in > a shift expression. > > gcc/c-family/ChangeLog: > > PR c/88383 > PR c/89288 > PR c/89798 > PR c/89797 > * c-attribs.c (type_valid_for_vector_size): Detect excessively > large sizes. > (validate_attribute): Handle DECLs and expressions. > (has_attribute): Handle types referenced by expressions. > Avoid considering array attributes in ARRAY_REF expressions . > > gcc/cp/ChangeLog: > > PR c/88383 > PR c/89288 > * parser.c (cp_parser_has_attribute_expression): Handle assignment > expressions. > > gcc/testsuite/ChangeLog: > > PR c/88383 > PR c/89288 > PR c/89798 > PR c/89797 > * c-c++-common/attributes-1.c: Adjust. > * c-c++-common/builtin-has-attribute-6.c: New test. > * c-c++-common/builtin-has-attribute-7.c: New test. > * c-c++-common/builtin-has-attribute-4.c: Adjust expectations. > * c-c++-common/builtin-has-attribute-6.c: New test. > * gcc.dg/pr25559.c: Adjust. > * gcc.dg/attr-vector_size.c: New test. Per our offline discussion, I think this is OK with some documentation updates to clarify expression handling. In particular can you adjust the docs to more clearly indicate that for an expression, we look at the attributes of the type of the expression. With a suitable doc change of that nature, this is OK. jeff
[C++ PATCH] Avoid ICE on pmf{} in template.
Now that we return the original CONSTRUCTOR from finish_compound_literal,
the call to null_member_pointer_value_p in tsubst_copy_and_build was getting
confused because the CONSTRUCTOR was still empty rather than a valid PMF
value. This is a regression from GCC 8.
Tested x86_64-pc-linux-gnu, applying to trunk.
* call.c (null_member_pointer_value_p): Handle an empty CONSTRUCTOR
of PMF type.
---
gcc/cp/call.c | 1 +
gcc/testsuite/g++.dg/cpp0x/initlist-pmf1.C | 20
gcc/cp/ChangeLog | 5 +
3 files changed, 26 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/initlist-pmf1.C
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 519dad9bf2c..9582345d7e8 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -569,6 +569,7 @@ null_member_pointer_value_p (tree t)
return false;
else if (TYPE_PTRMEMFUNC_P (type))
return (TREE_CODE (t) == CONSTRUCTOR
+ && CONSTRUCTOR_NELTS (t)
&& integer_zerop (CONSTRUCTOR_ELT (t, 0)->value));
else if (TYPE_PTRDATAMEM_P (type))
return integer_all_onesp (t);
diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist-pmf1.C
b/gcc/testsuite/g++.dg/cpp0x/initlist-pmf1.C
new file mode 100644
index 000..3035fefbc5f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/initlist-pmf1.C
@@ -0,0 +1,20 @@
+// { dg-do compile { target c++11 } }
+
+struct A
+{
+ void f();
+};
+
+using ftype = decltype(::f);
+
+template
+bool f()
+{
+ ftype p = ftype{};
+ return p;
+}
+
+int main()
+{
+ f();
+}
diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index 8e63fdaac74..8b2e7574450 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,8 @@
+2019-04-12 Jason Merrill
+
+ * call.c (null_member_pointer_value_p): Handle an empty CONSTRUCTOR
+ of PMF type.
+
2019-04-12 Marek Polacek
* except.c (build_noexcept_spec): Use build_converted_constant_bool_expr
base-commit: 5dcfd55433d543c73c774936a0979e463b9dc9b6
--
2.20.1
Re: [PATCH] Add target-zlib to top-level configure, use zlib from libphobos
On 4/11/19 9:58 AM, Iain Buclaw wrote: > On Sat, 6 Apr 2019 at 19:05, Iain Buclaw wrote: >> >> On Sat, 6 Apr 2019 at 17:27, Matthias Klose wrote: >>> >>> On 29.03.19 23:23, Iain Buclaw wrote: On Mon, 18 Feb 2019 at 14:26, Matthias Klose wrote: > > > sorry, I didn't mean to propose to rename the option, so > --with-target-system-zlib=auto sounds fine. OK, a bit belated, but here it is --with-target-system-zlib=auto. >>> >>> yes, this does the job. >>> >> >> Good. I added documentation to install.texi. >> > > Is this OK for trunk? It's the only prerequisite for applying > subdir-objects to libphobos. > Yes, it's fine for the trunk. jeff
Re: [PR 89693] Reorganize cgraph_node::clone_of_p
Dne 2019-04-11 17:14, Martin Jambor napsal:
Hi,
this reorganization of cgraph_node:clone_of_p() prevents verifier
falsely ICEing because it cannot recognize that a former thunk
(expanded
even before reaching pass pipeline) was cloned by IPA-CP.
It basically traverses the clone_of chain at each step of thunk chain
traversal, rather than just after it. This is only done when checking
is on, so the extra little overhead should be of little concern.
Bootstrapped, LTO-bootstrapped and tested on x86_64, OK for trunk? If
so, I will check if we need it in GCC 8 and if so, backport it there
too.
Thanks,
Martin
2019-04-11 Martin Jambor
PR ipa/89693
* cgraph.c (clone_of_p): Loop over clone chain for each step in
the thunk chain.
testsuite/
* g++.dg/ipa/pr89693.C: New test.
Thanks,
the patch is OK. If this verification turns out to be too much trouble,
i would be for removing it
(it really need to second guess all sane transformations do by all
passes we have)
Honza
---
gcc/cgraph.c | 30 ++---
gcc/testsuite/g++.dg/ipa/pr89693.C | 52 ++
2 files changed, 70 insertions(+), 12 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/ipa/pr89693.C
diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 49d80ad1e28..b1b0b4c42d5 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -2977,17 +2977,25 @@ cgraph_node::collect_callers (void)
static bool
clone_of_p (cgraph_node *node, cgraph_node *node2)
{
- bool skipped_thunk = false;
node = node->ultimate_alias_target ();
node2 = node2->ultimate_alias_target ();
+ if (node2->clone_of == node
+ || node2->former_clone_of == node->decl)
+return true;
+
+ if (!node->thunk.thunk_p && !node->former_thunk_p ())
+{
+ while (node2 && node->decl != node2->decl)
+ node2 = node2->clone_of;
+ return node2 != NULL;
+}
+
/* There are no virtual clones of thunks so check former_clone_of or
if we
might have skipped thunks because this adjustments are no longer
necessary. */
while (node->thunk.thunk_p || node->former_thunk_p ())
{
- if (node2->former_clone_of == node->decl)
- return true;
if (!node->thunk.this_adjusting)
return false;
/* In case of instrumented expanded thunks, which can have
multiple calls
@@ -2996,23 +3004,21 @@ clone_of_p (cgraph_node *node, cgraph_node
*node2)
if (node->callees->next_callee)
return true;
node = node->callees->callee->ultimate_alias_target ();
- skipped_thunk = true;
-}
- if (skipped_thunk)
-{
if (!node2->clone.args_to_skip
|| !bitmap_bit_p (node2->clone.args_to_skip, 0))
return false;
if (node2->former_clone_of == node->decl)
return true;
- else if (!node2->clone_of)
- return false;
+
+ cgraph_node *n2 = node2;
+ while (n2 && node->decl != n2->decl)
+ n2 = n2->clone_of;
+ if (n2)
+ return true;
}
- while (node2 && node->decl != node2->decl)
-node2 = node2->clone_of;
- return node2 != NULL;
+ return false;
}
/* Verify edge count and frequency. */
diff --git a/gcc/testsuite/g++.dg/ipa/pr89693.C
b/gcc/testsuite/g++.dg/ipa/pr89693.C
new file mode 100644
index 000..4ac83eeeb3a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/pr89693.C
@@ -0,0 +1,52 @@
+// Copyright (C) 2005 Free Software Foundation, Inc.
+// Contributed by Nathan Sidwell 4 Apr 2005
+// Re-purposed to check for re-rurgesnce of PR 89693 in 2019.
+
+// { dg-do compile }
+// { dg-options "-O3 -fno-ipa-icf-functions" }
+
+// Origin: yan...@ca.ibm.com
+// nat...@codesourcery.com
+
+struct A {
+ virtual void One ();
+};
+struct B {
+ virtual B *Two ();
+ virtual B ();
+};
+
+struct C : A, B
+{
+ virtual C *Two ();
+ virtual C ();
+};
+void A::One () {}
+B *B::Two(){return this;}
+B ::Three(){return *this;}
+C *C::Two () {return 0;}
+C ::Three () {return *(C *)0;}
+
+B *Foo (B *b)
+{
+ return b->Two ();
+}
+
+B (B *b)
+{
+ return b->Three ();
+}
+
+int main ()
+{
+ C c;
+
+ /* We should not adjust a null pointer. */
+ if (Foo ())
+return 1;
+ /* But we should adjust a (bogus) null reference. */
+ if (! ())
+return 2;
+
+ return 0;
+}
Re: [PATCH v2] Fix __patchable_function_entries section flags
On 4/11/19 11:18 AM, Joao Moreira wrote: > When -fpatchable-relocation-entry is used, gcc places nops on the > prologue of each compiled function and creates a section named > __patchable_function_entries which holds relocation entries for the > positions in which the nops were placed. As is, gcc creates this > section without the proper section flags, causing crashes in the > compiled program during its load. > > Given the above, fix the problem by creating the section with the > SECTION_WRITE and SECTION_RELRO flags. > > The problem was noticed while compiling glibc with > -fpatchable-function-entry compiler flag. After applying the patch, > this issue was solved. > > This was also tested on x86-64 arch without visible problems under > the gcc standard tests. > > 2019-04-10 Joao Moreira > > * targhooks.c (default_print_patchable_function_entry): Emit > __patchable_function_entries section with writable flags to allow > relocation resolution. OK. Do you have write access to the GCC repo? jeff >
Re: [PATCH] Fix up ARM target attribute handling (PR target/89093)
On 12/04/2019 15:12, Jakub Jelinek wrote:
Hi!
Just something I've noticed while looking at Ramana's patch.
As can be seen on the testcase, on arm we accept arbitrary garbage
after arm or thumb prefixes, is that really desirable?
While for fpu= or arch= we reject garbage after it and so do for
target attribute arg starting with +.
Ok if this passes bootstrap/regtest?
Bah, that's not supposed to be there, Yes that's ok .
Note, I don't understand the while (ISSPACE (*q)) ++q; there (aarch64
does the same), do we really want to support
__attribute__((target (" arm"))) ?
Looked at other targets and can't see anything like that being supported
elsewhere.
No, that's not right. we should get rid of this.
Thanks a lot for fixing this up Jakub.
Ramana
2019-04-12 Jakub Jelinek
PR target/89093
* config/arm/arm.c (arm_valid_target_attribute_rec): Use strcmp
instead of strncmp when checking for thumb and arm. Formatting fixes.
* gcc.target/arm/pr89093.c: New test.
--- gcc/config/arm/arm.c.jj 2019-04-09 15:18:37.879816537 +0200
+++ gcc/config/arm/arm.c2019-04-12 15:36:36.993102230 +0200
@@ -30874,16 +30874,16 @@ arm_valid_target_attribute_rec (tree arg
while (ISSPACE (*q)) ++q;
argstr = NULL;
- if (!strncmp (q, "thumb", 5))
- opts->x_target_flags |= MASK_THUMB;
+ if (!strcmp (q, "thumb"))
+ opts->x_target_flags |= MASK_THUMB;
- else if (!strncmp (q, "arm", 3))
- opts->x_target_flags &= ~MASK_THUMB;
+ else if (!strcmp (q, "arm"))
+ opts->x_target_flags &= ~MASK_THUMB;
else if (!strncmp (q, "fpu=", 4))
{
int fpu_index;
- if (! opt_enum_arg_to_value (OPT_mfpu_, q+4,
+ if (! opt_enum_arg_to_value (OPT_mfpu_, q + 4,
_index, CL_TARGET))
{
error ("invalid fpu for target attribute or pragma %qs", q);
@@ -30901,7 +30901,7 @@ arm_valid_target_attribute_rec (tree arg
}
else if (!strncmp (q, "arch=", 5))
{
- char* arch = q+5;
+ char *arch = q + 5;
const arch_option *arm_selected_arch
= arm_parse_arch_option_name (all_architectures, "arch", arch);
--- gcc/testsuite/gcc.target/arm/pr89093.c.jj 2019-04-12 16:05:47.477069147 +0200
+++ gcc/testsuite/gcc.target/arm/pr89093.c 2019-04-12 16:05:15.948591951
+0200
@@ -0,0 +1,7 @@
+/* PR target/89093 */
+/* { dg-do compile } */
+
+__attribute__((target ("arm.foobar"))) void f1 (void) {} /* { dg-error "unknown
target attribute or pragma 'arm.foobar'" } */
+__attribute__((target ("thumbozoo1"))) void f2 (void) {} /* { dg-error "unknown
target attribute or pragma 'thumbozoo1'" } */
+__attribute__((target ("arm,thumbique"))) void f3 (void) {} /* { dg-error "unknown
target attribute or pragma 'thumbique'" } */
+__attribute__((target ("thumb981,arm"))) void f4 (void) {} /* { dg-error "unknown
target attribute or pragma 'thumb981'" } */
Jakub
Re: [PATCH] Fix up RTL DCE find_call_stack_args (PR rtl-optimization/89965, take 2)
On 4/11/19 3:04 AM, Jakub Jelinek wrote: > On Thu, Apr 11, 2019 at 09:54:24AM +0200, Richard Biener wrote: >>> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? >>> >>> During those 2 bootstraps/regtests, data.load_found has been set just >>> on the new testcase on ia32. >> >> Hmm, I wonder whether we really need to DCE calls after reload? >> That said, I'm not familiar enough with the code to check if the >> patch makes sense (can there ever be uses of the argument slots >> _after_ the call?). > > And here is the patch on top of the refactoring patch. > As for the argument slots after the call, hope Jeff and Vlad won't mind if I > copy'n'paste what they said on IRC yesterday: > law: vmakarov: in PR89965, is it valid RTL to store something in stack > argument slot and read it again before the call that uses that stack slot? > law: vmakarov: if yes, I have a rough idea what to do, but if stack argument > slots may be only stored and can be read only by a callee and not by > something else in the caller, then it is a RA issue > jakub: i think it is not defined (or described). But the old > reload used equiv memory for long time to store value in memory. LRA just > uses the same approach. > i think you're allowed to read it up to the call point, who "owns" the > contents of the slot after the call point is subject to debate :-) > not sure if we defined things once a REG_EQUIV is on the MEM, but that > would tend to imply the memory is unchanging across the call > (though one could ask how in the world the caller would know the callee > didn't clobber the slot) > > 2019-04-11 Jakub Jelinek > > PR rtl-optimization/89965 > * dce.c: Include rtl-iter.h. > (struct check_argument_load_data): New type. > (check_argument_load): New function. > (find_call_stack_args): Check for loads from stack slots still tracked > in sp_bytes and punt if any is found. > > * gcc.target/i386/pr89965.c: New test. OK. I'd probably update this comment: > /* Walk backwards, looking for argument stores. The search stops > when seeing another call, sp adjustment or memory store other than > argument store. */ After this patch we'll also stop if we hit a load from an argument slot. jeff
Re: [PATCH] Fix up RTL DCE find_call_stack_args (PR rtl-optimization/89965, take 2)
On 4/11/19 3:26 AM, Richard Biener wrote: > On Thu, 11 Apr 2019, Jakub Jelinek wrote: > >> On Thu, Apr 11, 2019 at 09:54:24AM +0200, Richard Biener wrote: Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? During those 2 bootstraps/regtests, data.load_found has been set just on the new testcase on ia32. >>> >>> Hmm, I wonder whether we really need to DCE calls after reload? >>> That said, I'm not familiar enough with the code to check if the >>> patch makes sense (can there ever be uses of the argument slots >>> _after_ the call?). >> >> And here is the patch on top of the refactoring patch. >> As for the argument slots after the call, hope Jeff and Vlad won't mind if I >> copy'n'paste what they said on IRC yesterday: >> law: vmakarov: in PR89965, is it valid RTL to store something in stack >> argument slot and read it again before the call that uses that stack slot? >> law: vmakarov: if yes, I have a rough idea what to do, but if stack argument >> slots may be only stored and can be read only by a callee and not by >> something else in the caller, then it is a RA issue >> jakub: i think it is not defined (or described). But the old >> reload used equiv memory for long time to store value in memory. LRA just >> uses the same approach. >> i think you're allowed to read it up to the call point, who "owns" the >> contents of the slot after the call point is subject to debate :-) >> not sure if we defined things once a REG_EQUIV is on the MEM, but that >> would tend to imply the memory is unchanging across the call >> (though one could ask how in the world the caller would know the >> callee didn't clobber the slot) > > Ok, so that says "well, nothing prevents it from being used" for example > for a const/pure call (depending on what alias analysis does here...). Agreed. > Who owns the argument slots should be defined by the ABI I guess. Agreed, but I'm not sure it's consistently defined by ABIs. > > So iff alias-analysis doesn't leave us with the chance to pick up > the stack slot contents after the call in any case we're of course fine. > And if we do have the chance but should not then the call instruction > needs some explicit clobbering of the argument area (or the RTL IL > needs to be documented that such clobbering is implicit and the > alias machinery then needs to honor that). I suspect the only way we currently get into this state is around const/pure calls. One could imagine a day where we do strong alias analysis across calls and could make more precise annotations about how the callee uses/modifies specific memory locations. Jeff
Re: [PATCH] Mark MissingArgError, UnknownError and Warn *.opt arguments as gcc-internal-format (PR translation/90041)
On 4/11/19 6:27 AM, Jakub Jelinek wrote: > Hi! > > These 3 *.opt messages are printed using: > opts-common.c:error_at (loc, option->missing_argument_error, opt); > opts-common.c:error_at (loc, e->unknown_error, arg); > opts-common.c:warning_at (loc, 0, decoded->warn_message, opt); > and so they do support the various gcc-internal-format format string > specifiers like %<, %>, %qs etc. > > The following patch tweaks exgettext so that it marks them as such > and also adjusts c.opt so that it passes > contrib/check-internal-format-escaping.py after such a change. > > Tested on x86_64-linux using make gcc.pot, eyeballing the resulting file > and trying ./xgcc -B ./ -S hello.c -fhandle-exceptions > > Ok for trunk? > > 2019-04-11 Jakub Jelinek > > PR translation/90041 > * po/exgettext: Print MissingArgError, UnknownError or Warn > *.opt argument using error or warning instead of _ to mark it > as gcc-internal-format. > > * c.opt (-fhandle-exceptions): Use %< and %> around option names > in the Warn diagnostics. OK jeff
Re: [PATCH] Fix #error in mips loongson-mmintrin.h
On 4/11/19 6:51 AM, Jakub Jelinek wrote:
> Hi!
>
> While doing make gcc.pot, I've noticed warnings about unterminated string
> in loongson-mmintrin.h.
> I don't have any usable mips setup, but just tried:
> /tmp/1a.c:
> # error "You must select -mloongson-mmi or -march=loongson2e/2f/3a to use
>loongson-mmiintrin.h"
> /tmp/1b.c:
> # error You must select -mloongson-mmi or -march=loongson2e/2f/3a to use\
> loongson-mmiintrin.h
> $ gcc -S -o /tmp/1a.{s,c}
> /tmp/1a.c:1:9: warning: missing terminating " character
> # error "You must select -mloongson-mmi or -march=loongson2e/2f/3a to use
> ^
> /tmp/1a.c:1:3: error: #error "You must select -mloongson-mmi or
> -march=loongson2e/2f/3a to use
> # error "You must select -mloongson-mmi or -march=loongson2e/2f/3a to use
>^
> /tmp/1a.c:2:11: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’
> before ‘-’ token
>loongson-mmiintrin.h"
>^
> /tmp/1a.c:2:23: warning: missing terminating " character
>loongson-mmiintrin.h"
>^
> /tmp/1a.c:2:23: error: missing terminating " character
> $ gcc -S -o /tmp/1b.{s,c}
> /tmp/1b.c:1:3: error: #error You must select -mloongson-mmi or
> -march=loongson2e/2f/3a to use loongson-mmiintrin.h
> # error You must select -mloongson-mmi or -march=loongson2e/2f/3a to use\
>^
> and similarly for g++. Ok for trunk?
>
> 2019-04-11 Jakub Jelinek
>
> * config/mips/loongson-mmiintrin.h: Fix up #error message.
OK
jeff
>
Re: [PATCH] Fix up RTL DCE find_call_stack_args (PR rtl-optimization/89965)
On 4/12/19 9:32 AM, Alexander Monakov wrote: > On Fri, 12 Apr 2019, Jeff Law wrote: >> We've been through the "who owns the argument slots, caller or callee" >> discussion a few times and I don't think we've ever reached any kind of >> conclusion in the general case. > > Callee must own the slots for tail calls to be possible. > >> I think this case side-steps the general case. >> >> We've got an argument slot for a const/pure call. Because of the >> const/pure designation the caller can assume the callee doesn't modify >> the argument slot. That may in turn allow the caller to place a >> REG_EQUIV note on the store to the slot. > > I don't think this follows. Imagine a pure foo tailcalling a pure bar. > To make the tailcall, foo may need to change some of its argument slots > to pass new arguments to bar. I'd claim that a pure/const call can't tail call another function as that would potentially modify the argument slots. jeff
Re: [PATCH] Fix up RTL DCE find_call_stack_args (PR rtl-optimization/89965)
On Fri, 12 Apr 2019, Jeff Law wrote: > We've been through the "who owns the argument slots, caller or callee" > discussion a few times and I don't think we've ever reached any kind of > conclusion in the general case. Callee must own the slots for tail calls to be possible. > I think this case side-steps the general case. > > We've got an argument slot for a const/pure call. Because of the > const/pure designation the caller can assume the callee doesn't modify > the argument slot. That may in turn allow the caller to place a > REG_EQUIV note on the store to the slot. I don't think this follows. Imagine a pure foo tailcalling a pure bar. To make the tailcall, foo may need to change some of its argument slots to pass new arguments to bar. Alexander
Re: [PATCH 0/3] OpenRISC floating point support + fixes
On 4/10/19 3:27 PM, Stafford Horne wrote: > Hello, > > This is a set of patches to bring FPU support to the OpenRISC backend. The > backend also add support for 64-bit floating point operations on 32-bit cores > using register pairs, see orfpx64a32 [0]. > > This depends on binutils patches which have also been submitted per review. > [1] > > The toolchain has been tested using the gcc and binutils testsuites as well as > floating point test suites running on sim and an fpga soft core > or1k_marocchino. > [2] > > There is also an unrelated, but trivial patch to fix a code quality issue with > volatile memory loads. > > This whole patch series can be found on my github repo [3] as well. > > -Stafford > > [0] https://openrisc.io/proposals/orfpx64a32 > [1] g...@github.com:stffrdhrn/binutils-gdb.git orfpx64a32-2 > [2] https://github.com/openrisc/or1k_marocchino > [3] g...@github.com:stffrdhrn/gcc.git or1k-fpu-1a > > Stafford Horne (3): > or1k: Initial support for FPU > or1k: Allow volatile memory for sign/zero extend loads > or1k: only force reg for immediates > > gcc/config.gcc| 1 + > gcc/config/or1k/or1k.c| 10 ++-- > gcc/config/or1k/or1k.md | 109 -- > gcc/config/or1k/or1k.opt | 15 - > gcc/config/or1k/predicates.md | 16 + > gcc/doc/invoke.texi | 15 + > 6 files changed, 156 insertions(+), 10 deletions(-) > So the only question is whether or not you're looking to drop this into gcc-9 or gcc-10. gcc-9 is in regression bugfixing stage, so technically this patch should wait to gcc-10. However, we usually give maintainers a degree of freedom, particularly if a change doens't "bleed" into generic parts of the compiler. So we'll leave it up to you to decide if you want to add this to gcc-9 or wait for gcc-10. jeff
Re: [PATCH] Fix up RTL DCE find_call_stack_args (PR rtl-optimization/89965)
On 4/11/19 1:54 AM, Richard Biener wrote: > On Thu, 11 Apr 2019, Jakub Jelinek wrote: > >> Hi! >> >> The following testcase is miscompiled, because the result of >> a DImode (doubleword) right shift is used in a QImode subreg as well as >> later on pushed into a stack slot as an argument to a const function >> whose result isn't used. The RA because of the weirdo target tuning >> reuses the REG_EQUIV argument slot (as it wants the value in a non-Q_REGS >> register), so it first pushes it to the stack slot and then loads from the >> stack slot again (according to Vlad, this can happen with both LRA and old >> reload). Later on during DCE we determine the call is not needed and try to >> find all the argument pushes and don't mark those as needed, so we >> effectively DCE the right shift, push to the argument slot as well as >> other slots and the call, and end up keeping just the load from the >> uninitialized argument slot. >> >> The following patch just punts if we find loads from stack slots in between >> where they are pushed and the const/pure call. In addition to that, I've >> outlined the same largish sequence that had 3 copies in the function already >> and I'd need to add 4th copy, so in the end the dce.c changes are removing >> more than adding: >> 1 file changed, 118 insertions(+), 135 deletions(-) > > The refactoring itself is probably OK (and if done separately > makes reviewing easier). > >> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? >> >> During those 2 bootstraps/regtests, data.load_found has been set just >> on the new testcase on ia32. > > Hmm, I wonder whether we really need to DCE calls after reload? > That said, I'm not familiar enough with the code to check if the > patch makes sense (can there ever be uses of the argument slots > _after_ the call?). We've been through the "who owns the argument slots, caller or callee" discussion a few times and I don't think we've ever reached any kind of conclusion in the general case. I think this case side-steps the general case. We've got an argument slot for a const/pure call. Because of the const/pure designation the caller can assume the callee doesn't modify the argument slot. That may in turn allow the caller to place a REG_EQUIV note on the store to the slot. Once that happens we can replace one form with the other. So we could well end up with a use of the slot after the call. Jeff
Re: Enable BF16 support (Please ignore my former email)
On Fri, Apr 12, 2019 at 3:19 AM Uros Bizjak wrote: > > On Fri, Apr 12, 2019 at 11:03 AM Hongtao Liu wrote: > > > > On Fri, Apr 12, 2019 at 3:30 PM Uros Bizjak wrote: > > > > > > On Fri, Apr 12, 2019 at 9:09 AM Liu, Hongtao > > > wrote: > > > > > > > > Hi : > > > > This patch is about to enable support for bfloat16 which will be in > > > > Future Cooper Lake, Please refer to > > > > https://software.intel.com/en-us/download/intel-architecture-instruction-set-extensions-programming-reference > > > > for more details about BF16. > > > > > > > > There are 3 instructions for AVX512BF16: VCVTNE2PS2BF16, VCVTNEPS2BF16 > > > > and DPBF16PS instructions, which are Vector Neural Network Instructions > > > > supporting: > > > > > > > > - VCVTNE2PS2BF16: Convert Two Packed Single Data to One Packed > > > > BF16 Data. > > > > - VCVTNEPS2BF16: Convert Packed Single Data to Packed BF16 Data. > > > > - VDPBF16PS: Dot Product of BF16 Pairs Accumulated into Packed > > > > Single Precision. > > > > > > > > Since only BF16 intrinsics are supported, we treat it as HI for > > > > simplicity. > > > > > > I think it was a mistake declaring cvtps2ph and cvtph2ps using HImode > > > instead of HFmode. Is there a compelling reason not to introduce > > > corresponding bf16_format supporting infrastructure and declare these > > > intrinsics using half-binary (HBmode ?) mode instead? > > > > > > Uros. > > > > Bfloat16 isn't IEEE standard which we want to reserve HFmode for. > > True. > > > The IEEE 754 standard specifies a binary16 as having the following format: > > Sign bit: 1 bit > > Exponent width: 5 bits > > Significand precision: 11 bits (10 explicitly stored) > > > > Bfloat16 has the following format: > > Sign bit: 1 bit > > Exponent width: 8 bits > > Significand precision: 8 bits (7 explicitly stored), as opposed to 24 > > bits in a classical single-precision floating-point format > > This is why I proposed to introduce HBmode (and corresponding > bfloat16_format) to distingush between ieee HFmode and BFmode. > Unless there is BF16 language level support, HBmode has no advantage over HImode. We can add HBmode when we gain BF16 language support. -- H.J.
Re: [Patch] [testsuite][arm] Update warning prune regex
On Fri, 12 Apr 2019 at 16:23, Matthew Malcomson
wrote:
>
> On 12/04/19 15:16, Christophe Lyon wrote:
> > On Thu, 11 Apr 2019 at 11:26, Matthew Malcomson
> > wrote:
> >>
> >> r269586 changed the format of some warning messages.
> >>
> >> Each switch in the warning message is now surrounded by single quotes.
> >>
> >> This commit updates the regex's in arm.exp dejagnu files to match the
> >> new format and remove the extra 20+ FAILs on excess errors that are
> >> introduced on certain variations (e.g.
> >> arm-eabi-aem/-marm/-march=armv7-a/-mfpu=vfpv3-d16/-mfloat-abi=softfp).
> >>
> >> Regtested arm.exp with cross-compiler arm-none-eabi
> >>
> >> gcc/testsuite/ChangeLog:
> >>
> >> 2019-04-11 Matthew Malcomson
> >>
> >> * g++.target/arm/arm.exp: Change format of default prune regex.
> >> * gcc.target/arm/arm.exp: Change format of default prune regex.
> >>
> >
> > Hi!
> >
> > After you committed this, I'm now seeing errors opposite to what you
> > describe:
> > FAIL: gcc.target/arm/fma.c (test for excess errors)
> > Excess errors:
> > cc1: warning: switch -mcpu=cortex-a15 conflicts with -march=armv5t switch
> >
> > Is there something that can prevent emission of the quotes in the
> > environment for instance?
> >
>
> Hi Christophe,
>
> I don't know of anything in the environment that could cause that, and
> will go look.
>
>
> Just to check for the simple answer:
>
> Originally I mistakenly put this patch onto the gcc-8 branch instead of
> trunk.
>
> Since gcc-8 doesn't have the commit changing the format, you would have
> seen such reverse errors there -- is that where you're seeing them?
>
> If that's the case I just noticed & corrected that this afternoon.
>
Indeed, sorry I didn't have time enough this morning to report the
problem earlier.
Thanks,
Christophe
> Thanks,
> MM
>
> > Thanks,
> >
> > Christophe
> >
> >>
> >>
> >> ### Attachment also inlined for ease of reply
> >> ###
> >>
> >>
> >> diff --git a/gcc/testsuite/g++.target/arm/arm.exp
> >> b/gcc/testsuite/g++.target/arm/arm.exp
> >> index
> >> 247536b8312f8ff7a50c0eaf66e12785b80b6d3b..575f0a11c87583a264761f705997c7efc9d2d368
> >> 100644
> >> --- a/gcc/testsuite/g++.target/arm/arm.exp
> >> +++ b/gcc/testsuite/g++.target/arm/arm.exp
> >> @@ -35,7 +35,7 @@ if ![info exists DEFAULT_CXXFLAGS] then {
> >>
> >> global dg_runtest_extra_prunes
> >> set dg_runtest_extra_prunes ""
> >> -lappend dg_runtest_extra_prunes "warning: switch -m(cpu|arch)=.*
> >> conflicts with -m(cpu|arch)=.* switch"
> >> +lappend dg_runtest_extra_prunes "warning: switch '-m(cpu|arch)=.*'
> >> conflicts with '-m(cpu|arch)=.*' switch"
> >>
> >> # Initialize `dg'.
> >> dg-init
> >> diff --git a/gcc/testsuite/gcc.target/arm/arm.exp
> >> b/gcc/testsuite/gcc.target/arm/arm.exp
> >> index
> >> 0c4981c8316a30944302220b344810e3d44fcab1..829f6836b2918acdbae3160ec48802d9fe7ab5de
> >> 100644
> >> --- a/gcc/testsuite/gcc.target/arm/arm.exp
> >> +++ b/gcc/testsuite/gcc.target/arm/arm.exp
> >> @@ -33,7 +33,7 @@ if ![info exists DEFAULT_CFLAGS] then {
> >> # This variable should only apply to tests called in this exp file.
> >> global dg_runtest_extra_prunes
> >> set dg_runtest_extra_prunes ""
> >> -lappend dg_runtest_extra_prunes "warning: switch -m(cpu|arch)=.*
> >> conflicts with -m(cpu|arch)=.* switch"
> >> +lappend dg_runtest_extra_prunes "warning: switch '-m(cpu|arch)=.*'
> >> conflicts with '-m(cpu|arch)=.*' switch"
> >>
> >> # Initialize `dg'.
> >> dg-init
> >>
>
Re: [Patch] [testsuite][arm] Update warning prune regex
On 12/04/19 15:16, Christophe Lyon wrote:
> On Thu, 11 Apr 2019 at 11:26, Matthew Malcomson
> wrote:
>>
>> r269586 changed the format of some warning messages.
>>
>> Each switch in the warning message is now surrounded by single quotes.
>>
>> This commit updates the regex's in arm.exp dejagnu files to match the
>> new format and remove the extra 20+ FAILs on excess errors that are
>> introduced on certain variations (e.g.
>> arm-eabi-aem/-marm/-march=armv7-a/-mfpu=vfpv3-d16/-mfloat-abi=softfp).
>>
>> Regtested arm.exp with cross-compiler arm-none-eabi
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2019-04-11 Matthew Malcomson
>>
>> * g++.target/arm/arm.exp: Change format of default prune regex.
>> * gcc.target/arm/arm.exp: Change format of default prune regex.
>>
>
> Hi!
>
> After you committed this, I'm now seeing errors opposite to what you describe:
> FAIL: gcc.target/arm/fma.c (test for excess errors)
> Excess errors:
> cc1: warning: switch -mcpu=cortex-a15 conflicts with -march=armv5t switch
>
> Is there something that can prevent emission of the quotes in the
> environment for instance?
>
Hi Christophe,
I don't know of anything in the environment that could cause that, and
will go look.
Just to check for the simple answer:
Originally I mistakenly put this patch onto the gcc-8 branch instead of
trunk.
Since gcc-8 doesn't have the commit changing the format, you would have
seen such reverse errors there -- is that where you're seeing them?
If that's the case I just noticed & corrected that this afternoon.
Thanks,
MM
> Thanks,
>
> Christophe
>
>>
>>
>> ### Attachment also inlined for ease of reply
>> ###
>>
>>
>> diff --git a/gcc/testsuite/g++.target/arm/arm.exp
>> b/gcc/testsuite/g++.target/arm/arm.exp
>> index
>> 247536b8312f8ff7a50c0eaf66e12785b80b6d3b..575f0a11c87583a264761f705997c7efc9d2d368
>> 100644
>> --- a/gcc/testsuite/g++.target/arm/arm.exp
>> +++ b/gcc/testsuite/g++.target/arm/arm.exp
>> @@ -35,7 +35,7 @@ if ![info exists DEFAULT_CXXFLAGS] then {
>>
>> global dg_runtest_extra_prunes
>> set dg_runtest_extra_prunes ""
>> -lappend dg_runtest_extra_prunes "warning: switch -m(cpu|arch)=.* conflicts
>> with -m(cpu|arch)=.* switch"
>> +lappend dg_runtest_extra_prunes "warning: switch '-m(cpu|arch)=.*'
>> conflicts with '-m(cpu|arch)=.*' switch"
>>
>> # Initialize `dg'.
>> dg-init
>> diff --git a/gcc/testsuite/gcc.target/arm/arm.exp
>> b/gcc/testsuite/gcc.target/arm/arm.exp
>> index
>> 0c4981c8316a30944302220b344810e3d44fcab1..829f6836b2918acdbae3160ec48802d9fe7ab5de
>> 100644
>> --- a/gcc/testsuite/gcc.target/arm/arm.exp
>> +++ b/gcc/testsuite/gcc.target/arm/arm.exp
>> @@ -33,7 +33,7 @@ if ![info exists DEFAULT_CFLAGS] then {
>> # This variable should only apply to tests called in this exp file.
>> global dg_runtest_extra_prunes
>> set dg_runtest_extra_prunes ""
>> -lappend dg_runtest_extra_prunes "warning: switch -m(cpu|arch)=.* conflicts
>> with -m(cpu|arch)=.* switch"
>> +lappend dg_runtest_extra_prunes "warning: switch '-m(cpu|arch)=.*'
>> conflicts with '-m(cpu|arch)=.*' switch"
>>
>> # Initialize `dg'.
>> dg-init
>>
Re: [PATCH] Fix PR88936
Hi, On Fri, 12 Apr 2019, Richard Biener wrote: > > > > You miss PARM_DECLs and RESULT_DECLs, i.e. it's probably better to > > > > factor out tree.c:auto_var_in_fn_p and place the new auto_var_p in > > > > tree.c as well. > > > > > > Hmm, I left the above unchanged from a different variant of the > > > patch where for some reason I do not remember I explicitely decided > > > parameters and results are not affected... > > > > Even if that were the case the function is sufficiently general (also > > its name) that it should be generic infrastructure, not hidden away in > > structalias. > > It was not fully equivalent, but yes. So - like the following? Yep. Ciao, Michael.
Re: [Patch] [testsuite][arm] Update warning prune regex
On Fri, 12 Apr 2019 at 16:16, Christophe Lyon
wrote:
>
> On Thu, 11 Apr 2019 at 11:26, Matthew Malcomson
> wrote:
> >
> > r269586 changed the format of some warning messages.
> >
> > Each switch in the warning message is now surrounded by single quotes.
> >
> > This commit updates the regex's in arm.exp dejagnu files to match the
> > new format and remove the extra 20+ FAILs on excess errors that are
> > introduced on certain variations (e.g.
> > arm-eabi-aem/-marm/-march=armv7-a/-mfpu=vfpv3-d16/-mfloat-abi=softfp).
> >
> > Regtested arm.exp with cross-compiler arm-none-eabi
> >
> > gcc/testsuite/ChangeLog:
> >
> > 2019-04-11 Matthew Malcomson
> >
> > * g++.target/arm/arm.exp: Change format of default prune regex.
> > * gcc.target/arm/arm.exp: Change format of default prune regex.
> >
>
> Hi!
>
> After you committed this, I'm now seeing errors opposite to what you describe:
> FAIL: gcc.target/arm/fma.c (test for excess errors)
> Excess errors:
> cc1: warning: switch -mcpu=cortex-a15 conflicts with -march=armv5t switch
>
> Is there something that can prevent emission of the quotes in the
> environment for instance?
>
Sorry for the noise, I noticed too late that you committed this in
gcc-8-branch by mistake, then reverted.
> Thanks,
>
> Christophe
>
> >
> >
> > ### Attachment also inlined for ease of reply
> > ###
> >
> >
> > diff --git a/gcc/testsuite/g++.target/arm/arm.exp
> > b/gcc/testsuite/g++.target/arm/arm.exp
> > index
> > 247536b8312f8ff7a50c0eaf66e12785b80b6d3b..575f0a11c87583a264761f705997c7efc9d2d368
> > 100644
> > --- a/gcc/testsuite/g++.target/arm/arm.exp
> > +++ b/gcc/testsuite/g++.target/arm/arm.exp
> > @@ -35,7 +35,7 @@ if ![info exists DEFAULT_CXXFLAGS] then {
> >
> > global dg_runtest_extra_prunes
> > set dg_runtest_extra_prunes ""
> > -lappend dg_runtest_extra_prunes "warning: switch -m(cpu|arch)=.* conflicts
> > with -m(cpu|arch)=.* switch"
> > +lappend dg_runtest_extra_prunes "warning: switch '-m(cpu|arch)=.*'
> > conflicts with '-m(cpu|arch)=.*' switch"
> >
> > # Initialize `dg'.
> > dg-init
> > diff --git a/gcc/testsuite/gcc.target/arm/arm.exp
> > b/gcc/testsuite/gcc.target/arm/arm.exp
> > index
> > 0c4981c8316a30944302220b344810e3d44fcab1..829f6836b2918acdbae3160ec48802d9fe7ab5de
> > 100644
> > --- a/gcc/testsuite/gcc.target/arm/arm.exp
> > +++ b/gcc/testsuite/gcc.target/arm/arm.exp
> > @@ -33,7 +33,7 @@ if ![info exists DEFAULT_CFLAGS] then {
> > # This variable should only apply to tests called in this exp file.
> > global dg_runtest_extra_prunes
> > set dg_runtest_extra_prunes ""
> > -lappend dg_runtest_extra_prunes "warning: switch -m(cpu|arch)=.* conflicts
> > with -m(cpu|arch)=.* switch"
> > +lappend dg_runtest_extra_prunes "warning: switch '-m(cpu|arch)=.*'
> > conflicts with '-m(cpu|arch)=.*' switch"
> >
> > # Initialize `dg'.
> > dg-init
> >
Re: [Patch] [testsuite][arm] Update warning prune regex
On Thu, 11 Apr 2019 at 11:26, Matthew Malcomson
wrote:
>
> r269586 changed the format of some warning messages.
>
> Each switch in the warning message is now surrounded by single quotes.
>
> This commit updates the regex's in arm.exp dejagnu files to match the
> new format and remove the extra 20+ FAILs on excess errors that are
> introduced on certain variations (e.g.
> arm-eabi-aem/-marm/-march=armv7-a/-mfpu=vfpv3-d16/-mfloat-abi=softfp).
>
> Regtested arm.exp with cross-compiler arm-none-eabi
>
> gcc/testsuite/ChangeLog:
>
> 2019-04-11 Matthew Malcomson
>
> * g++.target/arm/arm.exp: Change format of default prune regex.
> * gcc.target/arm/arm.exp: Change format of default prune regex.
>
Hi!
After you committed this, I'm now seeing errors opposite to what you describe:
FAIL: gcc.target/arm/fma.c (test for excess errors)
Excess errors:
cc1: warning: switch -mcpu=cortex-a15 conflicts with -march=armv5t switch
Is there something that can prevent emission of the quotes in the
environment for instance?
Thanks,
Christophe
>
>
> ### Attachment also inlined for ease of reply
> ###
>
>
> diff --git a/gcc/testsuite/g++.target/arm/arm.exp
> b/gcc/testsuite/g++.target/arm/arm.exp
> index
> 247536b8312f8ff7a50c0eaf66e12785b80b6d3b..575f0a11c87583a264761f705997c7efc9d2d368
> 100644
> --- a/gcc/testsuite/g++.target/arm/arm.exp
> +++ b/gcc/testsuite/g++.target/arm/arm.exp
> @@ -35,7 +35,7 @@ if ![info exists DEFAULT_CXXFLAGS] then {
>
> global dg_runtest_extra_prunes
> set dg_runtest_extra_prunes ""
> -lappend dg_runtest_extra_prunes "warning: switch -m(cpu|arch)=.* conflicts
> with -m(cpu|arch)=.* switch"
> +lappend dg_runtest_extra_prunes "warning: switch '-m(cpu|arch)=.*' conflicts
> with '-m(cpu|arch)=.*' switch"
>
> # Initialize `dg'.
> dg-init
> diff --git a/gcc/testsuite/gcc.target/arm/arm.exp
> b/gcc/testsuite/gcc.target/arm/arm.exp
> index
> 0c4981c8316a30944302220b344810e3d44fcab1..829f6836b2918acdbae3160ec48802d9fe7ab5de
> 100644
> --- a/gcc/testsuite/gcc.target/arm/arm.exp
> +++ b/gcc/testsuite/gcc.target/arm/arm.exp
> @@ -33,7 +33,7 @@ if ![info exists DEFAULT_CFLAGS] then {
> # This variable should only apply to tests called in this exp file.
> global dg_runtest_extra_prunes
> set dg_runtest_extra_prunes ""
> -lappend dg_runtest_extra_prunes "warning: switch -m(cpu|arch)=.* conflicts
> with -m(cpu|arch)=.* switch"
> +lappend dg_runtest_extra_prunes "warning: switch '-m(cpu|arch)=.*' conflicts
> with '-m(cpu|arch)=.*' switch"
>
> # Initialize `dg'.
> dg-init
>
Re: [PATCH] Add support for missing AVX512* ISAs (PR target/89929).
On Fri, Apr 12, 2019 at 4:41 AM Martin Liška wrote:
>
> On 4/11/19 6:30 PM, H.J. Lu wrote:
> > On Thu, Apr 11, 2019 at 1:38 AM Martin Liška wrote:
> >>
> >> Hi.
> >>
> >> The patch is adding missing AVX512 ISAs for target and target_clone
> >> attributes.
> >>
> >> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> >>
> >> Ready to be installed?
> >> Thanks,
> >> Martin
> >>
> >> gcc/ChangeLog:
> >>
> >> 2019-04-10 Martin Liska
> >>
> >> PR target/89929
> >> * config/i386/i386.c (get_builtin_code_for_version): Add
> >> support for missing AVX512 ISAs.
> >>
> >> gcc/testsuite/ChangeLog:
> >>
> >> 2019-04-10 Martin Liska
> >>
> >> PR target/89929
> >> * g++.target/i386/mv28.C: New test.
> >> * gcc.target/i386/mvc14.c: New test.
> >> ---
> >> gcc/config/i386/i386.c| 34 ++-
> >> gcc/testsuite/g++.target/i386/mv28.C | 30 +++
> >> gcc/testsuite/gcc.target/i386/mvc14.c | 16 +
> >> 3 files changed, 79 insertions(+), 1 deletion(-)
> >> create mode 100644 gcc/testsuite/g++.target/i386/mv28.C
> >> create mode 100644 gcc/testsuite/gcc.target/i386/mvc14.c
> >>
> >>
> >
>
> Hi.
>
> > Since any ISAs beyond AVX512F may be enabled individually, we
> > can't simply assign priorities to them. For GFNI, we can have
> >
> > 1. GFNI
> > 2. GFNI + AVX
> > 3. GFNI + AVX512F
> > 4. GFNI + AVX512F + AVX512VL
>
> Makes sense to me! I'm considering syntax extension where one would be
> able to come up with a priority. Eg.
>
> __attribute__((target("gfni,avx512bw", priority((3)
>
> Without that the ISA combinations are probably not comparable in a reasonable
> way.
>
> >
> > For this code, GFNI + AVX512BW is ignored:
> >
> > [hjl@gnu-cfl-1 pr89929]$ cat z.ii
> > __attribute__((target("gfni")))
> > int foo(int i) {
> > return 1;
> > }
> > __attribute__((target("gfni,avx512bw")))
> > int foo(int i) {
> > return 4;
> > }
> > __attribute__((target("default")))
> > int foo(int i) {
> > return 3;
> > }
> > int bar ()
> > {
> > return foo(2);
> > }
>
> For 'target' attribute it works for me:
>
> 1) $ cat z.c && ./xg++ -B. z.c -c
> #include
> volatile __m512i x1, x2;
> volatile __mmask64 m64;
>
> __attribute__((target("gfni")))
> int foo(int i) {
> x1 = _mm512_gf2p8affineinv_epi64_epi8(x1, x2, 3);
> return 1;
> }
> __attribute__((target("gfni,avx512bw")))
> int foo(int i) {
> return 4;
> }
> __attribute__((target("default")))
> int foo(int i) {
> return 3;
> }
> int bar ()
> {
> return foo(2);
> }
> In file included from ./include/immintrin.h:117,
> from ./include/x86intrin.h:32,
> from z.c:1:
> z.c: In function ‘int foo(int)’:
> z.c:7:10: error: ‘__builtin_ia32_vgf2p8affineinvqb_v64qi’ needs isa option
> -m32 -mgfni -mavx512f
> 7 | x1 = _mm512_gf2p8affineinv_epi64_epi8(x1, x2, 3);
> | ^~~~
> z.c:7:10: note: the ABI for passing parameters with 64-byte alignment has
> changed in GCC 4.6
>
> 2) $ cat z.c && ./xg++ -B. z.c -c
> #include
> volatile __m512i x1, x2;
> volatile __mmask64 m64;
>
> __attribute__((target("gfni")))
> int foo(int i) {
> return 1;
> }
> __attribute__((target("gfni,avx512bw")))
> int foo(int i) {
> x1 = _mm512_gf2p8affineinv_epi64_epi8(x1, x2, 3);
> return 4;
> }
> __attribute__((target("default")))
> int foo(int i) {
> return 3;
> }
> int bar ()
> {
> return foo(2);
> }
>
> [OK]
>
> Btw. is it really correct the '-m32' in: 'needs isa option -m32' ?
It does look odd.
> Similar applies to target_clone attribute where we'll have to come up with
> a syntax that will allow multiple ISA to be combined. Something like:
>
> __attribute__((target_clones("gfni+avx512bw")))
>
> ? Priorities can be maybe implemented by order?
>
I am thinking -misa=processor which will enable ISAs for
processor. It differs from -march=. -misa= doesn't set
-mtune.
--
H.J.
[PATCH] Fix up ARM target attribute handling (PR target/89093)
Hi!
Just something I've noticed while looking at Ramana's patch.
As can be seen on the testcase, on arm we accept arbitrary garbage
after arm or thumb prefixes, is that really desirable?
While for fpu= or arch= we reject garbage after it and so do for
target attribute arg starting with +.
Ok if this passes bootstrap/regtest?
Note, I don't understand the while (ISSPACE (*q)) ++q; there (aarch64
does the same), do we really want to support
__attribute__((target (" arm"))) ?
Looked at other targets and can't see anything like that being supported
elsewhere.
2019-04-12 Jakub Jelinek
PR target/89093
* config/arm/arm.c (arm_valid_target_attribute_rec): Use strcmp
instead of strncmp when checking for thumb and arm. Formatting fixes.
* gcc.target/arm/pr89093.c: New test.
--- gcc/config/arm/arm.c.jj 2019-04-09 15:18:37.879816537 +0200
+++ gcc/config/arm/arm.c2019-04-12 15:36:36.993102230 +0200
@@ -30874,16 +30874,16 @@ arm_valid_target_attribute_rec (tree arg
while (ISSPACE (*q)) ++q;
argstr = NULL;
- if (!strncmp (q, "thumb", 5))
- opts->x_target_flags |= MASK_THUMB;
+ if (!strcmp (q, "thumb"))
+ opts->x_target_flags |= MASK_THUMB;
- else if (!strncmp (q, "arm", 3))
- opts->x_target_flags &= ~MASK_THUMB;
+ else if (!strcmp (q, "arm"))
+ opts->x_target_flags &= ~MASK_THUMB;
else if (!strncmp (q, "fpu=", 4))
{
int fpu_index;
- if (! opt_enum_arg_to_value (OPT_mfpu_, q+4,
+ if (! opt_enum_arg_to_value (OPT_mfpu_, q + 4,
_index, CL_TARGET))
{
error ("invalid fpu for target attribute or pragma %qs", q);
@@ -30901,7 +30901,7 @@ arm_valid_target_attribute_rec (tree arg
}
else if (!strncmp (q, "arch=", 5))
{
- char* arch = q+5;
+ char *arch = q + 5;
const arch_option *arm_selected_arch
= arm_parse_arch_option_name (all_architectures, "arch", arch);
--- gcc/testsuite/gcc.target/arm/pr89093.c.jj 2019-04-12 16:05:47.477069147
+0200
+++ gcc/testsuite/gcc.target/arm/pr89093.c 2019-04-12 16:05:15.948591951
+0200
@@ -0,0 +1,7 @@
+/* PR target/89093 */
+/* { dg-do compile } */
+
+__attribute__((target ("arm.foobar"))) void f1 (void) {} /* { dg-error
"unknown target attribute or pragma 'arm.foobar'" } */
+__attribute__((target ("thumbozoo1"))) void f2 (void) {} /* { dg-error
"unknown target attribute or pragma 'thumbozoo1'" } */
+__attribute__((target ("arm,thumbique"))) void f3 (void) {} /* { dg-error
"unknown target attribute or pragma 'thumbique'" } */
+__attribute__((target ("thumb981,arm"))) void f4 (void) {} /* { dg-error
"unknown target attribute or pragma 'thumb981'" } */
Jakub
Re: [PATCH] Fix PR88936
On Fri, 12 Apr 2019, Michael Matz wrote:
> Hi,
>
> On Fri, 12 Apr 2019, Richard Biener wrote:
>
> > > You miss PARM_DECLs and RESULT_DECLs, i.e. it's probably better to factor
> > > out tree.c:auto_var_in_fn_p and place the new auto_var_p in tree.c as
> > > well.
> >
> > Hmm, I left the above unchanged from a different variant of the patch
> > where for some reason I do not remember I explicitely decided
> > parameters and results are not affected...
>
> Even if that were the case the function is sufficiently general (also its
> name) that it should be generic infrastructure, not hidden away in
> structalias.
It was not fully equivalent, but yes. So - like the following?
I think checking DECL_CONTEXT isn't necessary given the
!DECL_EXTERNAL/STATIC checks.
Bootstrap / regtest running on x86_64-unknown-linux-gnu.
Richard.
2019-04-12 Richard Biener
PR ipa/88936
* tree.h (auto_var_p): Declare.
* tree.c (auto_var_p): New function, split out from ...
(auto_var_in_fn_p): ... here.
* tree-ssa-structalias.c (struct variable_info): Add shadow_var_uid
member.
(new_var_info): Initialize it.
(set_uids_in_ptset): Also set the shadow variable uid if required.
(ipa_pta_execute): Postprocess points-to solutions assigning
shadow variable uids for locals that may reach their containing
function recursively.
* gcc.dg/torture/pr88936-1.c: New testcase.
* gcc.dg/torture/pr88936-2.c: Likewise.
* gcc.dg/torture/pr88936-3.c: Likewise.
Index: gcc/tree.c
===
--- gcc/tree.c (revision 270306)
+++ gcc/tree.c (working copy)
@@ -9268,17 +9268,25 @@ get_type_static_bounds (const_tree type,
}
}
+/* Return true if VAR is an automatic variable. */
+
+bool
+auto_var_p (const_tree var)
+{
+ return VAR_P (var) && ! DECL_EXTERNAL (var))
+ || TREE_CODE (var) == PARM_DECL)
+ && ! TREE_STATIC (var))
+ || TREE_CODE (var) == RESULT_DECL);
+}
+
/* Return true if VAR is an automatic variable defined in function FN. */
bool
auto_var_in_fn_p (const_tree var, const_tree fn)
{
return (DECL_P (var) && DECL_CONTEXT (var) == fn
- && VAR_P (var) && ! DECL_EXTERNAL (var))
- || TREE_CODE (var) == PARM_DECL)
- && ! TREE_STATIC (var))
- || TREE_CODE (var) == LABEL_DECL
- || TREE_CODE (var) == RESULT_DECL));
+ && (auto_var_p (var)
+ || TREE_CODE (var) == LABEL_DECL));
}
/* Subprogram of following function. Called by walk_tree.
Index: gcc/tree.h
===
--- gcc/tree.h (revision 270306)
+++ gcc/tree.h (working copy)
@@ -4893,6 +4893,7 @@ extern bool stdarg_p (const_tree);
extern bool prototype_p (const_tree);
extern bool is_typedef_decl (const_tree x);
extern bool typedef_variant_p (const_tree);
+extern bool auto_var_p (const_tree);
extern bool auto_var_in_fn_p (const_tree, const_tree);
extern tree build_low_bits_mask (tree, unsigned);
extern bool tree_nop_conversion_p (const_tree, const_tree);
Index: gcc/tree-ssa-structalias.c
===
--- gcc/tree-ssa-structalias.c (revision 270306)
+++ gcc/tree-ssa-structalias.c (working copy)
@@ -299,6 +299,11 @@ struct variable_info
/* Full size of the base variable, in bits. */
unsigned HOST_WIDE_INT fullsize;
+ /* In IPA mode the shadow UID in case the variable needs to be duplicated in
+ the final points-to solution because it reaches its containing
+ function recursively. Zero if none is needed. */
+ unsigned int shadow_var_uid;
+
/* Name of this variable */
const char *name;
@@ -397,6 +402,7 @@ new_var_info (tree t, const char *name,
ret->solution = BITMAP_ALLOC (_obstack);
ret->oldsolution = NULL;
ret->next = 0;
+ ret->shadow_var_uid = 0;
ret->head = ret->id;
stats.total_vars++;
@@ -6452,6 +6458,16 @@ set_uids_in_ptset (bitmap into, bitmap f
&& (TREE_STATIC (vi->decl) || DECL_EXTERNAL (vi->decl))
&& ! decl_binds_to_current_def_p (vi->decl))
pt->vars_contains_interposable = true;
+
+ /* If this is a local variable we can have overlapping lifetime
+of different function invocations through recursion duplicate
+it with its shadow variable. */
+ if (in_ipa_mode
+ && vi->shadow_var_uid != 0)
+ {
+ bitmap_set_bit (into, vi->shadow_var_uid);
+ pt->vars_contains_nonlocal = true;
+ }
}
else if (TREE_CODE (vi->decl) == FUNCTION_DECL
@@ -8076,6 +8095,62 @@ ipa_pta_execute (void)
/* From the constraints compute the points-to sets. */
solve_constraints ();
+ /* Now post-process solutions to handle locals from different
+ runtime instantiations coming in through
Re: [PATCH] Fix PR88936
Hi, On Fri, 12 Apr 2019, Richard Biener wrote: > > You miss PARM_DECLs and RESULT_DECLs, i.e. it's probably better to factor > > out tree.c:auto_var_in_fn_p and place the new auto_var_p in tree.c as > > well. > > Hmm, I left the above unchanged from a different variant of the patch > where for some reason I do not remember I explicitely decided > parameters and results are not affected... Even if that were the case the function is sufficiently general (also its name) that it should be generic infrastructure, not hidden away in structalias. Ciao, Michael.
Re: [PATCH] Fix PR88936
On Fri, 12 Apr 2019, Michael Matz wrote:
> Hi,
>
> On Fri, 12 Apr 2019, Richard Biener wrote:
>
> > @@ -332,6 +337,24 @@ struct obstack final_solutions_obstack;
> > Indexed directly by variable info id. */
> > static vec varmap;
> >
> > +/* Return whether VAR is an automatic variable. */
> > +
> > +static bool
> > +auto_var_p (const_tree var)
> > +{
> > + if (VAR_P (var))
> > +{
> > + tree context = DECL_CONTEXT (var);
> > + if (context
> > + && TREE_CODE (context) == FUNCTION_DECL
> > + && ! DECL_EXTERNAL (var)
> > + && ! TREE_STATIC (var))
> > + return true;
> > +}
> > + return false;
> > +}
>
> You miss PARM_DECLs and RESULT_DECLs, i.e. it's probably better to factor
> out tree.c:auto_var_in_fn_p and place the new auto_var_p in tree.c as
> well.
Hmm, I left the above unchanged from a different variant of the patch
where for some reason I do not remember I explicitely decided
parameters and results are not affected...
Not so for parameters as the following testcase shows.
Will fix.
Richard.
void bar(int cnt)
{
if (cnt == 0)
{
p =
bar (1);
if (cnt != 1)
__builtin_abort ();
}
else if (cnt == 1)
*p = 1;
}
int main()
{
bar (0);
return 0;
}
Re: C++ PATCH for c++/87603 - constexpr functions are no longer noexcept
On 4/11/19 5:36 PM, Marek Polacek wrote: This patch deals with constexpr functions and whether or not they should be noexcept. CWG 1129 specified that constexpr functions are noexcept: it was a special case in [expr.unary.noexcept]. This was accidentally removed in but the CWG conclusion was to keep it as-is. Clearly we need to change this for C++17. The question is whether we should treat it as a DR and apply it retroactively (which is what clang did). I took the same approach and my reasoning is in the comment in check_noexcept_r. Arguably it might be too late to put this in now, maybe we should defer to GCC 10. But at least there's a patch. Bootstrapped/regtested on x86_64-linux. 2019-04-11 Marek Polacek PR c++/87603 - constexpr functions are no longer noexcept. * constexpr.c (is_sub_constant_expr): Remove unused function. * cp-tree.h (is_sub_constant_expr): Remove declaration. * except.c (check_noexcept_r): Don't consider a call to a constexpr function noexcept. OK. This is actually one of the issues that I ran into with cmcstl2, so let's go ahead and put it in now. Jason
Re: C++ PATCH to use build_converted_constant_bool_expr for noexcept-specifiers
On 4/11/19 4:16 PM, Marek Polacek wrote: [except.spec] says that "In a noexcept-specifier, the constant-expression, if supplied, shall be a contextually converted constant expression of type bool". We now have a dedicated function for creating such an expression, so build_noexcept_spec should make use of it. As a consequence, we now reject pr86397-2.C but that's correct: converting a pointer to bool is a "boolean conversion", which is not allowed under the rules for a converted constant expression ([expr.const]p7). Bootstrapped/regtested on x86_64-linux, ok for trunk? 2019-04-11 Marek Polacek * except.c (build_noexcept_spec): Use build_converted_constant_bool_expr instead of perform_implicit_conversion_flags. OK. Jason
Re: [PATCH] Fix PR88936
Hi,
On Fri, 12 Apr 2019, Richard Biener wrote:
> @@ -332,6 +337,24 @@ struct obstack final_solutions_obstack;
> Indexed directly by variable info id. */
> static vec varmap;
>
> +/* Return whether VAR is an automatic variable. */
> +
> +static bool
> +auto_var_p (const_tree var)
> +{
> + if (VAR_P (var))
> +{
> + tree context = DECL_CONTEXT (var);
> + if (context
> + && TREE_CODE (context) == FUNCTION_DECL
> + && ! DECL_EXTERNAL (var)
> + && ! TREE_STATIC (var))
> + return true;
> +}
> + return false;
> +}
You miss PARM_DECLs and RESULT_DECLs, i.e. it's probably better to factor
out tree.c:auto_var_in_fn_p and place the new auto_var_p in tree.c as
well.
Ciao,
Michael.
Re: [PATCH] Add support for missing AVX512* ISAs (PR target/89929).
On 4/11/19 6:30 PM, H.J. Lu wrote:
> On Thu, Apr 11, 2019 at 1:38 AM Martin Liška wrote:
>>
>> Hi.
>>
>> The patch is adding missing AVX512 ISAs for target and target_clone
>> attributes.
>>
>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>
>> Ready to be installed?
>> Thanks,
>> Martin
>>
>> gcc/ChangeLog:
>>
>> 2019-04-10 Martin Liska
>>
>> PR target/89929
>> * config/i386/i386.c (get_builtin_code_for_version): Add
>> support for missing AVX512 ISAs.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2019-04-10 Martin Liska
>>
>> PR target/89929
>> * g++.target/i386/mv28.C: New test.
>> * gcc.target/i386/mvc14.c: New test.
>> ---
>> gcc/config/i386/i386.c| 34 ++-
>> gcc/testsuite/g++.target/i386/mv28.C | 30 +++
>> gcc/testsuite/gcc.target/i386/mvc14.c | 16 +
>> 3 files changed, 79 insertions(+), 1 deletion(-)
>> create mode 100644 gcc/testsuite/g++.target/i386/mv28.C
>> create mode 100644 gcc/testsuite/gcc.target/i386/mvc14.c
>>
>>
>
Hi.
> Since any ISAs beyond AVX512F may be enabled individually, we
> can't simply assign priorities to them. For GFNI, we can have
>
> 1. GFNI
> 2. GFNI + AVX
> 3. GFNI + AVX512F
> 4. GFNI + AVX512F + AVX512VL
Makes sense to me! I'm considering syntax extension where one would be
able to come up with a priority. Eg.
__attribute__((target("gfni,avx512bw", priority((3)
Without that the ISA combinations are probably not comparable in a reasonable
way.
>
> For this code, GFNI + AVX512BW is ignored:
>
> [hjl@gnu-cfl-1 pr89929]$ cat z.ii
> __attribute__((target("gfni")))
> int foo(int i) {
> return 1;
> }
> __attribute__((target("gfni,avx512bw")))
> int foo(int i) {
> return 4;
> }
> __attribute__((target("default")))
> int foo(int i) {
> return 3;
> }
> int bar ()
> {
> return foo(2);
> }
For 'target' attribute it works for me:
1) $ cat z.c && ./xg++ -B. z.c -c
#include
volatile __m512i x1, x2;
volatile __mmask64 m64;
__attribute__((target("gfni")))
int foo(int i) {
x1 = _mm512_gf2p8affineinv_epi64_epi8(x1, x2, 3);
return 1;
}
__attribute__((target("gfni,avx512bw")))
int foo(int i) {
return 4;
}
__attribute__((target("default")))
int foo(int i) {
return 3;
}
int bar ()
{
return foo(2);
}
In file included from ./include/immintrin.h:117,
from ./include/x86intrin.h:32,
from z.c:1:
z.c: In function ‘int foo(int)’:
z.c:7:10: error: ‘__builtin_ia32_vgf2p8affineinvqb_v64qi’ needs isa option -m32
-mgfni -mavx512f
7 | x1 = _mm512_gf2p8affineinv_epi64_epi8(x1, x2, 3);
| ^~~~
z.c:7:10: note: the ABI for passing parameters with 64-byte alignment has
changed in GCC 4.6
2) $ cat z.c && ./xg++ -B. z.c -c
#include
volatile __m512i x1, x2;
volatile __mmask64 m64;
__attribute__((target("gfni")))
int foo(int i) {
return 1;
}
__attribute__((target("gfni,avx512bw")))
int foo(int i) {
x1 = _mm512_gf2p8affineinv_epi64_epi8(x1, x2, 3);
return 4;
}
__attribute__((target("default")))
int foo(int i) {
return 3;
}
int bar ()
{
return foo(2);
}
[OK]
Btw. is it really correct the '-m32' in: 'needs isa option -m32' ?
Similar applies to target_clone attribute where we'll have to come up with
a syntax that will allow multiple ISA to be combined. Something like:
__attribute__((target_clones("gfni+avx512bw")))
? Priorities can be maybe implemented by order?
Martin
> [hjl@gnu-cfl-1 pr89929]$
> /export/build/gnu/tools-build/gcc-wip-debug/build-x86_64-linux/gcc/xgcc
> -B/export/build/gnu/tools-build/gcc-wip-debug/build-x86_64-linux/gcc/
> -S z.ii -O2
> [hjl@gnu-cfl-1 pr89929]$ cat z.s
> .file "z.ii"
> .text
> .p2align 4
> .globl _Z3fooi.gfni
> .type _Z3fooi.gfni, @function
> _Z3fooi.gfni:
> .LFB0:
> .cfi_startproc
> movl $1, %eax
> ret
> .cfi_endproc
> .LFE0:
> .size _Z3fooi.gfni, .-_Z3fooi.gfni
> .p2align 4
> .globl _Z3fooi
> .type _Z3fooi, @function
> _Z3fooi:
> .LFB2:
> .cfi_startproc
> movl $3, %eax
> ret
> .cfi_endproc
> .LFE2:
> .size _Z3fooi, .-_Z3fooi
> .p2align 4
> .globl _Z3fooi.avx512bw_gfni
> .type _Z3fooi.avx512bw_gfni, @function
> _Z3fooi.avx512bw_gfni:
> .LFB1:
> .cfi_startproc
> movl $4, %eax
> ret
> .cfi_endproc
> .LFE1:
> .size _Z3fooi.avx512bw_gfni, .-_Z3fooi.avx512bw_gfni
> .section
> .text._Z3fooi.resolver,"axG",@progbits,_Z3fooi.resolver,comdat
> .p2align 4
> .weak _Z3fooi.resolver
> .type _Z3fooi.resolver, @function
> _Z3fooi.resolver:
> .LFB5:
> .cfi_startproc
> subq $8, %rsp
> .cfi_def_cfa_offset 16
> call __cpu_indicator_init
> movl $_Z3fooi, %eax
> movl
[C++ PATCH] Add testcases for already fixed C++ duplicate_decls attribute diagnostics (PR c++/89325)
On Fri, Nov 02, 2018 at 07:10:18AM -0400, Nathan Sidwell wrote:
> duplicate_decls is one of the more complex fns in decl.c, and I need to make
> it more complicated. But first some refactoring, so it's a little more
> understandable. Generally moving warning checks later when we know we've
> actually got a duplicate, and splitting up some conflict checking.
>
> Applying to trunk after an x86_64-linux bootstrap.
> 2018-11-01 Nathan Sidwell
>
> gcc/cp/
> * decl.c (duplicate_decls): Refactor checks.
> * name-lookup.c (name_lookup::process_module_binding): Only pubic
> namespaces are shared.
>
> gcc/testsuite/
> * g++.dg/lookup/crash6.C: Adjust error
> * g++.dg/parse/crash38.C: Likewise.
This patch fixed PR c++/89325, previously we were diagnosing mismatched
attributes way too early, before establishing if e.g. different parameter
types don't actually make the decls not duplicate at all but overloaded.
The attrib5{8,9}.C testcases started to FAIL with g++ 7.x, while attrib60.C
testcase used to FAIL even in 3.2.
The following patch adds testcase coverage for that. Tested on x86_64-linux
and i686-linux, ok for trunk?
Could the fix be eventually backported to release branches (or portions
thereof)?
2019-04-12 Jakub Jelinek
PR c++/89325
* g++.dg/ext/attrib58.C: New test.
* g++.dg/ext/attrib59.C: New test.
* g++.dg/ext/attrib60.C: New test.
--- gcc/testsuite/g++.dg/ext/attrib58.C.jj 2019-04-12 11:45:37.934754511
+0200
+++ gcc/testsuite/g++.dg/ext/attrib58.C 2019-04-12 11:45:24.903969682 +0200
@@ -0,0 +1,8 @@
+// PR c++/89325
+// { dg-do compile }
+// { dg-options "-Wattributes" }
+
+struct A { friend int << (int , const A &) { return i; } }; // {
dg-bogus "previous definition" }
+#pragma GCC optimize ("-fno-ipa-cp-clone")
+struct B {};
+int <<(int &, const B &); // { dg-bogus "optimization attribute
on '\[^\n\r]*' follows definition but the attribute doesn.t match" }
--- gcc/testsuite/g++.dg/ext/attrib59.C.jj 2019-04-12 12:02:03.473483696
+0200
+++ gcc/testsuite/g++.dg/ext/attrib59.C 2019-04-12 12:02:38.130910855 +0200
@@ -0,0 +1,11 @@
+// PR c++/89325
+// { dg-do compile }
+// { dg-options "-Wattributes" }
+
+int foo (int) { return 0; }// { dg-bogus "previous definition" }
+int bar (int) { return 0; }// { dg-bogus "previous definition" }
+int baz (int) { return 0; }// { dg-message "previous definition" }
+__attribute__((optimize (0))) int bar (long); // { dg-bogus "optimization
attribute on '\[^\n\r]*' follows definition but the attribute doesn.t match" }
+#pragma GCC optimize ("-fno-ipa-cp-clone")
+int foo (long);// { dg-bogus "optimization attribute
on '\[^\n\r]*' follows definition but the attribute doesn.t match" }
+int baz (int); // { dg-warning "optimization attribute on
'\[^\n\r]*' follows definition but the attribute doesn.t match" }
--- gcc/testsuite/g++.dg/ext/attrib60.C.jj 2019-04-12 12:03:01.002532806
+0200
+++ gcc/testsuite/g++.dg/ext/attrib60.C 2019-04-12 12:07:33.056036076 +0200
@@ -0,0 +1,9 @@
+// PR c++/89325
+// { dg-do compile }
+// { dg-options "-Wattributes" }
+
+__attribute__((noinline)) void foo (int) {}// { dg-bogus "previous
definition" }
+inline void foo (long);// { dg-bogus "inline
declaration of '\[^\n\r]*' follows declaration with attribute 'noinline'" }
+inline void foo (long) {}
+__attribute__((noinline)) void bar (int) {}// { dg-message "previous
definition" }
+inline void bar (int); // { dg-warning "inline
declaration of '\[^\n\r]*' follows declaration with attribute 'noinline'" }
Jakub
Re: Enable BF16 support (Please ignore my former email)
On Fri, Apr 12, 2019 at 11:03 AM Hongtao Liu wrote: > > On Fri, Apr 12, 2019 at 3:30 PM Uros Bizjak wrote: > > > > On Fri, Apr 12, 2019 at 9:09 AM Liu, Hongtao wrote: > > > > > > Hi : > > > This patch is about to enable support for bfloat16 which will be in > > > Future Cooper Lake, Please refer to > > > https://software.intel.com/en-us/download/intel-architecture-instruction-set-extensions-programming-reference > > > for more details about BF16. > > > > > > There are 3 instructions for AVX512BF16: VCVTNE2PS2BF16, VCVTNEPS2BF16 > > > and DPBF16PS instructions, which are Vector Neural Network Instructions > > > supporting: > > > > > > - VCVTNE2PS2BF16: Convert Two Packed Single Data to One Packed BF16 > > > Data. > > > - VCVTNEPS2BF16: Convert Packed Single Data to Packed BF16 Data. > > > - VDPBF16PS: Dot Product of BF16 Pairs Accumulated into Packed > > > Single Precision. > > > > > > Since only BF16 intrinsics are supported, we treat it as HI for > > > simplicity. > > > > I think it was a mistake declaring cvtps2ph and cvtph2ps using HImode > > instead of HFmode. Is there a compelling reason not to introduce > > corresponding bf16_format supporting infrastructure and declare these > > intrinsics using half-binary (HBmode ?) mode instead? > > > > Uros. > > Bfloat16 isn't IEEE standard which we want to reserve HFmode for. True. > The IEEE 754 standard specifies a binary16 as having the following format: > Sign bit: 1 bit > Exponent width: 5 bits > Significand precision: 11 bits (10 explicitly stored) > > Bfloat16 has the following format: > Sign bit: 1 bit > Exponent width: 8 bits > Significand precision: 8 bits (7 explicitly stored), as opposed to 24 > bits in a classical single-precision floating-point format This is why I proposed to introduce HBmode (and corresponding bfloat16_format) to distingush between ieee HFmode and BFmode. Uros.
[PATCH] Fix PR88936
The following fixes a IPA PTA miscompilation involving recursion
which means two instances of the same local variables can become
live. For both testcases we confuse one for the other and cause
DSE to remove a store to the non-local one.
The fix involves adding a "shadow" variable representing all
"other" instances of locals to the IPA points-to solution.
We restrict this treatment to variables reaching their containing
function via parameters and those escaping to globals.
Bootstrap & regtest running on x86_64-unknown-linux-gnu.
Richard.
2019-04-12 Richard Biener
PR ipa/88936
* tree-ssa-structalias.c (struct variable_info): Add shadow_var_uid
member.
(new_var_info): Initialize it.
(auto_var_p): New helper.
(set_uids_in_ptset): Also set the shadow variable uid if required.
(ipa_pta_execute): Postprocess points-to solutions assigning
shadow variable uids for locals that may reach their containing
function recursively.
* gcc.dg/torture/pr88936-1.c: New testcase.
* gcc.dg/torture/pr88936-2.c: New testcase.
Index: gcc/tree-ssa-structalias.c
===
--- gcc/tree-ssa-structalias.c (revision 270306)
+++ gcc/tree-ssa-structalias.c (working copy)
@@ -299,6 +299,11 @@ struct variable_info
/* Full size of the base variable, in bits. */
unsigned HOST_WIDE_INT fullsize;
+ /* In IPA mode the shadow UID in case the variable needs to be duplicated in
+ the final points-to solution because it reaches its containing
+ function recursively. Zero if none is needed. */
+ unsigned int shadow_var_uid;
+
/* Name of this variable */
const char *name;
@@ -332,6 +337,24 @@ struct obstack final_solutions_obstack;
Indexed directly by variable info id. */
static vec varmap;
+/* Return whether VAR is an automatic variable. */
+
+static bool
+auto_var_p (const_tree var)
+{
+ if (VAR_P (var))
+{
+ tree context = DECL_CONTEXT (var);
+ if (context
+ && TREE_CODE (context) == FUNCTION_DECL
+ && ! DECL_EXTERNAL (var)
+ && ! TREE_STATIC (var))
+ return true;
+}
+ return false;
+}
+
+
/* Return the varmap element N */
static inline varinfo_t
@@ -397,6 +420,7 @@ new_var_info (tree t, const char *name,
ret->solution = BITMAP_ALLOC (_obstack);
ret->oldsolution = NULL;
ret->next = 0;
+ ret->shadow_var_uid = 0;
ret->head = ret->id;
stats.total_vars++;
@@ -6452,6 +6476,16 @@ set_uids_in_ptset (bitmap into, bitmap f
&& (TREE_STATIC (vi->decl) || DECL_EXTERNAL (vi->decl))
&& ! decl_binds_to_current_def_p (vi->decl))
pt->vars_contains_interposable = true;
+
+ /* If this is a local variable we can have overlapping lifetime
+of different function invocations through recursion duplicate
+it with its shadow variable. */
+ if (in_ipa_mode
+ && vi->shadow_var_uid != 0)
+ {
+ bitmap_set_bit (into, vi->shadow_var_uid);
+ pt->vars_contains_nonlocal = true;
+ }
}
else if (TREE_CODE (vi->decl) == FUNCTION_DECL
@@ -8076,6 +8113,62 @@ ipa_pta_execute (void)
/* From the constraints compute the points-to sets. */
solve_constraints ();
+ /* Now post-process solutions to handle locals from different
+ runtime instantiations coming in through recursive invocations. */
+ unsigned shadow_var_cnt = 0;
+ for (unsigned i = 1; i < varmap.length (); ++i)
+{
+ varinfo_t fi = get_varinfo (i);
+ if (fi->is_fn_info
+ && fi->decl)
+ /* Automatic variables pointed to by their containing functions
+ parameters need this treatment. */
+ for (varinfo_t ai = first_vi_for_offset (fi, fi_parm_base);
+ai; ai = vi_next (ai))
+ {
+ varinfo_t vi = get_varinfo (find (ai->id));
+ bitmap_iterator bi;
+ unsigned j;
+ EXECUTE_IF_SET_IN_BITMAP (vi->solution, 0, j, bi)
+ {
+ varinfo_t pt = get_varinfo (j);
+ if (pt->shadow_var_uid == 0
+ && pt->decl
+ && auto_var_in_fn_p (pt->decl, fi->decl))
+ {
+ pt->shadow_var_uid = allocate_decl_uid ();
+ shadow_var_cnt++;
+ }
+ }
+ }
+ /* As well as global variables which are another way of passing
+ arguments to recursive invocations. */
+ else if (fi->is_global_var)
+ {
+ for (varinfo_t ai = fi; ai; ai = vi_next (ai))
+ {
+ varinfo_t vi = get_varinfo (find (ai->id));
+ bitmap_iterator bi;
+ unsigned j;
+ EXECUTE_IF_SET_IN_BITMAP (vi->solution, 0, j, bi)
+ {
+ varinfo_t pt = get_varinfo (j);
+ if (pt->shadow_var_uid == 0
+
Re: [C++ Patch/RFC] PR 89900 ("[9 Regression] ICE: Segmentation fault (in check_instantiated_arg)")
.. an additional observation. Over the last couple of days I wondered
if the amended testcase was really valid, given the non-terminal
parameter pack, beyond the evidence that all the compilers I have at
hand accept it. Note anyway, that we - and all the compilers - already
accept a version of the testcase without the explicit argument and
deduce the pack as empty:
template void
fk (XE..., int);
void
w9 (void)
{
fk (0);
}
Thus, it seems to me that at least we have a consistency issue, at some
level. Are we already "inadvertently" implementing P0478R0 or I'm
missing something else?!?
Paolo.
Re: [PATCH, GCC, AARCH64] Add GNU note section with BTI and PAC.
Ping.
On 04/04/2019 17:01, Sudakshina Das wrote:
> Hi Richard
>
> On 03/04/2019 11:28, Richard Henderson wrote:
>> On 4/3/19 5:19 PM, Sudakshina Das wrote:
>>> + /* PT_NOTE header: namesz, descsz, type.
>>> + namesz = 4 ("GNU\0")
>>> + descsz = 16 (Size of the program property array)
>>> + type = 5 (NT_GNU_PROPERTY_TYPE_0). */
>>> + assemble_align (POINTER_SIZE);
>>> + assemble_integer (GEN_INT (4), 4, 32, 1);
>>> + assemble_integer (GEN_INT (16), 4, 32, 1);
>>
>> So, it's 16 only if POINTER_SIZE == 64.
>>
>> I think ROUND_UP (12, POINTER_BYTES) is what you want here.
>>
>
>
> Ah yes. I have made that change now.
>
> Thanks
> Sudi
>
>>
>> r~
>>
>
Re: AW: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop fixes.
Gilles & Reinhold, Following the discussion, I have decided to remove the copy-in for intent(in). Dominique and I have located the problem with -m32 for PR90022 and I am working on PR89846 right now. I will be resubmitting a bit later on. Cheers Paul On Fri, 12 Apr 2019 at 01:25, Gilles Gouaillardet wrote: > > Reinhold, > > > Thanks for the insights ! > > > That means there is currently an other issue since copy-in is performed > even if the argument is declared as ASYNCHRONOUS. > > > I gave the copy-in mechanism some more thoughts, and as a library > developers, I can clearly see a need *not* to do that > > on a case-by-case basis, mainly for performance reasons, but also to be > friendly with legacy apps that are not strictly standard compliant. > > At this stage, I think the best way to move forward is to add an other > directive in the interface definition. > > > for example, we could declare > > > module foo > > interface > > subroutine bar_f08(buf) BIND(C, name="bar_c") > > implicit none > > !GCC$ ATTRIBUTES NO_COPY_IN :: buf > > TYPE(*), DIMENSION(..), INTENT(IN) :: buf > > end subroutine > > end interface > > end module > > > Does this make sense ? > > > Gilles > > On 4/10/2019 4:22 PM, Bader, Reinhold wrote: > > Hi Gilles, > > > >> I also found an other potential issue with copy-in. > >> > >> If in Fortran, we > >> > >> call foo(buf(0,0)) > >> > >> then the C subroutine can only access buf(0,0), and other elements such > >> as buf(1025,1025) cannot be accessed. > >> > >> Such elements are valid in buf, but out of bounds in the copy (that > >> contains a single element). > >> > >> Strictly speaking, I cannot say whether this is a violation of the > >> standard or not, but I can see how this will > >> > >> break a lot of existing apps (once again, those apps might be incorrect > >> in the first place, but most of us got used to them working). > > > > The above call will only be conforming if the dummy argument is declared > > assumed or explicit size. > > Otherwise, the compiler should reject it due to rank mismatch. For assumed > > rank, the call would be > > legitimate, but the rank of the dummy argument is then zero. Even if no > > copy-in is performed, > > accessing data beyond the address range of that scalar is not strictly > > allowed. > > > > Of more interest is the situation where the dummy argument in Fortran is > > declared, e.g., > > > > TYPE(*), ASYNCHRONOUS, INTENT(IN) :: BUF(..) > > > > The standard's semantics *forbids* performing copy-in/out in this case, > > IIRC. > > Otherwise > > ASYNCHRONOUS semantics would not work, and non-blocking MPI calls would fail > > due > > to buffers vanishing into thin air. > > > > Regards > > Reinhold > > > >> To me, this is a second reason why copy-in is not desirable (at least as > >> a default option). > >> > >> > >> > >> Cheers, > >> > >> > >> Gilles > >> > >> On 4/9/2019 7:18 PM, Paul Richard Thomas wrote: > >>> The most part of this patch is concerned with implementing calls from > >>> C of of fortran bind c procedures with assumed shape or assumed rank > >>> dummies to completely fix PR89843. The conversion of the descriptors > >>> from CFI to gfc occur on entry to and reversed on exit from the > >>> procedure. > >>> > >>> This patch is safe for trunk, even at this late stage, because its > >>> effects are barricaded behind the tests for CFI descriptors. I believe > >>> that it appropriately rewards the bug reporters to have this feature > >>> work as well as possible at release. > >>> > >>> Between comments and the ChangeLogs, this patch is self explanatory. > >>> > >>> Bootstrapped and regtested on FC29/x86_64 - OK for trunk? > >>> > >>> Paul > >>> > >>> 2019-04-09 Paul Thomas > >>> > >>> PR fortran/89843 > >>> * trans-decl.c (gfc_get_symbol_decl): Assumed shape and assumed > >>> rank dummies of bind C procs require deferred initialization. > >>> (convert_CFI_desc): New procedure to convert incoming CFI > >>> descriptors to gfc types and back again. > >>> (gfc_trans_deferred_vars): Call it. > >>> * trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): Null the CFI > >>> descriptor pointer. Free the descriptor in all cases. > >>> > >>> PR fortran/90022 > >>> * trans-decl.c (gfc_get_symbol_decl): Make sure that the se > >>> expression is a pointer type before converting it to the symbol > >>> backend_decl type. > >>> > >>> 2019-04-09 Paul Thomas > >>> > >>> PR fortran/89843 > >>> * gfortran.dg/ISO_Fortran_binding_4.f90: Modify the value of x > >>> in ctg. Test the conversion of the descriptor types in the main > >>> program. > >>> * gfortran.dg/ISO_Fortran_binding_10.f90: New test. > >>> * gfortran.dg/ISO_Fortran_binding_10.c: Called by it. > >>> > >>> PR fortran/90022 > >>> * gfortran.dg/ISO_Fortran_binding_1.c: Correct the indexing for > >>> the computation of 'ans'. Also, change the expected results for > >>>
Re: Enable BF16 support (Please ignore my former email)
On Fri, Apr 12, 2019 at 3:30 PM Uros Bizjak wrote: > > On Fri, Apr 12, 2019 at 9:09 AM Liu, Hongtao wrote: > > > > Hi : > > This patch is about to enable support for bfloat16 which will be in > > Future Cooper Lake, Please refer to > > https://software.intel.com/en-us/download/intel-architecture-instruction-set-extensions-programming-reference > > for more details about BF16. > > > > There are 3 instructions for AVX512BF16: VCVTNE2PS2BF16, VCVTNEPS2BF16 and > > DPBF16PS instructions, which are Vector Neural Network Instructions > > supporting: > > > > - VCVTNE2PS2BF16: Convert Two Packed Single Data to One Packed BF16 > > Data. > > - VCVTNEPS2BF16: Convert Packed Single Data to Packed BF16 Data. > > - VDPBF16PS: Dot Product of BF16 Pairs Accumulated into Packed Single > > Precision. > > > > Since only BF16 intrinsics are supported, we treat it as HI for simplicity. > > I think it was a mistake declaring cvtps2ph and cvtph2ps using HImode > instead of HFmode. Is there a compelling reason not to introduce > corresponding bf16_format supporting infrastructure and declare these > intrinsics using half-binary (HBmode ?) mode instead? > > Uros. Bfloat16 isn't IEEE standard which we want to reserve HFmode for. The IEEE 754 standard specifies a binary16 as having the following format: Sign bit: 1 bit Exponent width: 5 bits Significand precision: 11 bits (10 explicitly stored) Bfloat16 has the following format: Sign bit: 1 bit Exponent width: 8 bits Significand precision: 8 bits (7 explicitly stored), as opposed to 24 bits in a classical single-precision floating-point format -- BR, Hongtao
Re: AW: AW: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop fixes.
Dear Reinhold, Thanks again for the insight ! My preference is also to avoid copy-in whenever possible. But since my primary concern is performance (vs enforcing correctness of the user apps), I am not willing to use existing bugs to (fallaciously) conclude copy-in should be simply dropped (unless mandatory). Cheers, Gilles On 4/12/2019 4:53 PM, Bader, Reinhold wrote: Dear Gilles, -Ursprüngliche Nachricht- Von: Gilles Gouaillardet Gesendet: Freitag, 12. April 2019 02:25 An: Bader, Reinhold ; Paul Richard Thomas ; fort...@gcc.gnu.org; gcc-patches Betreff: Re: AW: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop fixes. Reinhold, Thanks for the insights ! That means there is currently an other issue since copy-in is performed even if the argument is declared as ASYNCHRONOUS. I gave the copy-in mechanism some more thoughts, and as a library developers, I can clearly see a need *not* to do that on a case-by-case basis, mainly for performance reasons, but also to be friendly with legacy apps that are not strictly standard compliant. At this stage, I think the best way to move forward is to add an other directive in the interface definition. for example, we could declare module foo interface subroutine bar_f08(buf) BIND(C, name="bar_c") implicit none !GCC$ ATTRIBUTES NO_COPY_IN :: buf maybe so, but some care must be taken to perform proper checking in case copy-in is necessary - these situations exist as well. For example, if the declaration for buf is TYPE(*), DIMENSION(..), INTENT(IN), CONTIGUOUS :: buf and the actual argument is a discontiguous array section. My preference is for the compiler to automatically avoid copy-in whenever possible. Cheers Reinhold TYPE(*), DIMENSION(..), INTENT(IN) :: buf end subroutine end interface end module Does this make sense ? Gilles On 4/10/2019 4:22 PM, Bader, Reinhold wrote: Hi Gilles, I also found an other potential issue with copy-in. If in Fortran, we call foo(buf(0,0)) then the C subroutine can only access buf(0,0), and other elements such as buf(1025,1025) cannot be accessed. Such elements are valid in buf, but out of bounds in the copy (that contains a single element). Strictly speaking, I cannot say whether this is a violation of the standard or not, but I can see how this will break a lot of existing apps (once again, those apps might be incorrect in the first place, but most of us got used to them working). The above call will only be conforming if the dummy argument is declared assumed or explicit size. Otherwise, the compiler should reject it due to rank mismatch. For assumed rank, the call would be legitimate, but the rank of the dummy argument is then zero. Even if no copy-in is performed, accessing data beyond the address range of that scalar is not strictly allowed. Of more interest is the situation where the dummy argument in Fortran is declared, e.g., TYPE(*), ASYNCHRONOUS, INTENT(IN) :: BUF(..) The standard's semantics *forbids* performing copy-in/out in this case, IIRC. Otherwise ASYNCHRONOUS semantics would not work, and non-blocking MPI calls would fail due to buffers vanishing into thin air. Regards Reinhold To me, this is a second reason why copy-in is not desirable (at least as a default option). Cheers, Gilles On 4/9/2019 7:18 PM, Paul Richard Thomas wrote: The most part of this patch is concerned with implementing calls from C of of fortran bind c procedures with assumed shape or assumed rank dummies to completely fix PR89843. The conversion of the descriptors from CFI to gfc occur on entry to and reversed on exit from the procedure. This patch is safe for trunk, even at this late stage, because its effects are barricaded behind the tests for CFI descriptors. I believe that it appropriately rewards the bug reporters to have this feature work as well as possible at release. Between comments and the ChangeLogs, this patch is self explanatory. Bootstrapped and regtested on FC29/x86_64 - OK for trunk? Paul 2019-04-09 Paul Thomas PR fortran/89843 * trans-decl.c (gfc_get_symbol_decl): Assumed shape and assumed rank dummies of bind C procs require deferred initialization. (convert_CFI_desc): New procedure to convert incoming CFI descriptors to gfc types and back again. (gfc_trans_deferred_vars): Call it. * trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): Null the CFI descriptor pointer. Free the descriptor in all cases. PR fortran/90022 * trans-decl.c (gfc_get_symbol_decl): Make sure that the se expression is a pointer type before converting it to the symbol backend_decl type. 2019-04-09 Paul Thomas PR fortran/89843 * gfortran.dg/ISO_Fortran_binding_4.f90: Modify the value of x in ctg. Test the conversion of the descriptor types in the main program. * gfortran.dg/ISO_Fortran_binding_10.f90: New test. *
Re: [PATCH] tilepro.c diagnostics bug fix (PR target/52726)
On Thu, Apr 11, 2019 at 3:02 PM Jakub Jelinek wrote:
>
> On Tue, Mar 12, 2019 at 10:58:14AM +0100, Jakub Jelinek wrote:
> > These are the only remaining cases of gcc-internal-format diagnostics using
> > HOST_WIDE_INT_PRINT*. That is wrong because the string depends on the exact
> > host, and xgettext doesn't handle it anyway, so in gcc.pot the messages are
> > truncated at the spot where HOST_WIDE_INT_PRINT* appears. See also
> > PR79846 for an earlier change of the same kind.
> >
> > Tested on a cross to s390x-linux on the
> > gcc.target/s390/htm-builtins-compile-2.c
> > testcase. Ok for trunk?
> >
> > 2019-03-12 Jakub Jelinek
> >
> > PR target/52726
> > * config/s390/s390.md (tabort): Use %wd instead of
> > HOST_WIDE_INT_PRINT_DEC in error message, reword to avoid two capital
> > letters and periods.
> > * config/tilepro/tilepro.c (tilepro_print_operand): Use %wd in
> > output_operand_lossage instead of HOST_WIDE_INT_PRINT_DEC, replace
> > 's with %< and %>.
>
> Unfortunately, today while building gcc.pot I've noticed:
> config/tilepro/tilepro.c:4774: warning: Although being used in a format
> string position, the msgid is not a valid C format string. Reason: In the
> directive number 2, the token after '<' is not the name of a format specifier
> macro. The valid macro names are listed in ISO C 99 section 7.8.1.
> Indeed, output_operand_lossage argument is not gcc-internal-format, so
> it can't use %wd nor %< nor %>. It is printed using:
> va_start (ap, cmsgid);
>
> pfx_str = this_is_asm_operands ? _("invalid 'asm': ") : "output_operand: ";
> fmt_string = xasprintf ("%s%s", pfx_str, _(cmsgid));
> new_message = xvasprintf (fmt_string, ap);
>
> if (this_is_asm_operands)
> error_for_asm (this_is_asm_operands, "%s", new_message);
> else
> internal_error ("%s", new_message);
> so can use just the fprintf format specifiers. While I perhaps could use
> %ld instead of %<%wd%> and cast the HOST_WIDE_INT argument to long, it
> doesn't seem to be worth it IMHO, if the argument is not a CONST_INT, we
> print also just invalid %%t operand, so I'd suggest doing following.
>
> Ok for trunk?
OK.
Richard.
> 2019-04-11 Jakub Jelinek
>
> PR target/52726
> * config/tilepro/tilepro.c (tilepro_print_operand): Use just
> "invalid %%t operand" in output_operand_lossage message.
>
> --- gcc/config/tilepro/tilepro.c.jj 2019-03-12 10:46:40.995960027 +0100
> +++ gcc/config/tilepro/tilepro.c2019-04-11 14:54:01.708668318 +0200
> @@ -4771,7 +4771,7 @@ tilepro_print_operand (FILE *file, rtx x
> i = exact_log2 (n);
> if (i < 0)
> {
> - output_operand_lossage ("invalid %%t operand %<%wd%>", n);
> + output_operand_lossage ("invalid %%t operand");
> return;
> }
>
>
>
> Jakub
Re: [PATCH] Uglify identifiers missed in previous commit(s)
On 11/04/19 21:15 -0700, Thomas Rodgers wrote: * include/pstl/algorithm_impl.h: Uglify identfiers. * include/pstl/numeric_impl.h: Uglify identfiers. * include/pstl/parallel_backend_tbb.h: Uglify identfiers. OK for trunk, thanks.
AW: AW: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop fixes.
Dear Gilles, > -Ursprüngliche Nachricht- > Von: Gilles Gouaillardet > Gesendet: Freitag, 12. April 2019 02:25 > An: Bader, Reinhold ; Paul Richard Thomas > ; fort...@gcc.gnu.org; gcc-patches patc...@gcc.gnu.org> > Betreff: Re: AW: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop > fixes. > > Reinhold, > > > Thanks for the insights ! > > > That means there is currently an other issue since copy-in is performed even > if the argument is declared as ASYNCHRONOUS. > > > I gave the copy-in mechanism some more thoughts, and as a library > developers, I can clearly see a need *not* to do that > > on a case-by-case basis, mainly for performance reasons, but also to be > friendly with legacy apps that are not strictly standard compliant. > > At this stage, I think the best way to move forward is to add an other > directive in the interface definition. > > > for example, we could declare > > > module foo > > interface > > subroutine bar_f08(buf) BIND(C, name="bar_c") > > implicit none > > !GCC$ ATTRIBUTES NO_COPY_IN :: buf maybe so, but some care must be taken to perform proper checking in case copy-in is necessary - these situations exist as well. For example, if the declaration for buf is TYPE(*), DIMENSION(..), INTENT(IN), CONTIGUOUS :: buf and the actual argument is a discontiguous array section. My preference is for the compiler to automatically avoid copy-in whenever possible. Cheers Reinhold > > TYPE(*), DIMENSION(..), INTENT(IN) :: buf > > end subroutine > > end interface > > end module > > > Does this make sense ? > > > Gilles > > On 4/10/2019 4:22 PM, Bader, Reinhold wrote: > > Hi Gilles, > > > >> I also found an other potential issue with copy-in. > >> > >> If in Fortran, we > >> > >> call foo(buf(0,0)) > >> > >> then the C subroutine can only access buf(0,0), and other elements such > >> as buf(1025,1025) cannot be accessed. > >> > >> Such elements are valid in buf, but out of bounds in the copy (that > >> contains a single element). > >> > >> Strictly speaking, I cannot say whether this is a violation of the > >> standard or not, but I can see how this will > >> > >> break a lot of existing apps (once again, those apps might be incorrect > >> in the first place, but most of us got used to them working). > > > > The above call will only be conforming if the dummy argument is declared > > assumed or explicit size. > > Otherwise, the compiler should reject it due to rank mismatch. For > assumed > > rank, the call would be > > legitimate, but the rank of the dummy argument is then zero. Even if no > > copy-in is performed, > > accessing data beyond the address range of that scalar is not strictly > > allowed. > > > > Of more interest is the situation where the dummy argument in Fortran is > > declared, e.g., > > > > TYPE(*), ASYNCHRONOUS, INTENT(IN) :: BUF(..) > > > > The standard's semantics *forbids* performing copy-in/out in this case, > IIRC. > > Otherwise > > ASYNCHRONOUS semantics would not work, and non-blocking MPI calls > would fail > > due > > to buffers vanishing into thin air. > > > > Regards > > Reinhold > > > >> To me, this is a second reason why copy-in is not desirable (at least as > >> a default option). > >> > >> > >> > >> Cheers, > >> > >> > >> Gilles > >> > >> On 4/9/2019 7:18 PM, Paul Richard Thomas wrote: > >>> The most part of this patch is concerned with implementing calls from > >>> C of of fortran bind c procedures with assumed shape or assumed rank > >>> dummies to completely fix PR89843. The conversion of the descriptors > >>> from CFI to gfc occur on entry to and reversed on exit from the > >>> procedure. > >>> > >>> This patch is safe for trunk, even at this late stage, because its > >>> effects are barricaded behind the tests for CFI descriptors. I believe > >>> that it appropriately rewards the bug reporters to have this feature > >>> work as well as possible at release. > >>> > >>> Between comments and the ChangeLogs, this patch is self explanatory. > >>> > >>> Bootstrapped and regtested on FC29/x86_64 - OK for trunk? > >>> > >>> Paul > >>> > >>> 2019-04-09 Paul Thomas > >>> > >>> PR fortran/89843 > >>> * trans-decl.c (gfc_get_symbol_decl): Assumed shape and assumed > >>> rank dummies of bind C procs require deferred initialization. > >>> (convert_CFI_desc): New procedure to convert incoming CFI > >>> descriptors to gfc types and back again. > >>> (gfc_trans_deferred_vars): Call it. > >>> * trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): Null the CFI > >>> descriptor pointer. Free the descriptor in all cases. > >>> > >>> PR fortran/90022 > >>> * trans-decl.c (gfc_get_symbol_decl): Make sure that the se > >>> expression is a pointer type before converting it to the symbol > >>> backend_decl type. > >>> > >>> 2019-04-09 Paul Thomas > >>> > >>> PR fortran/89843 > >>> * gfortran.dg/ISO_Fortran_binding_4.f90: Modify the value of x > >>>
[Committed] S/390: Fix a problem with the bswap vector pattern
arch13 introduced instructions to perform vector element-wise byte
swaps on the way from or to memory. For a byte swap between vector
registers the vector permute instruction is required which needs a
permute pattern to be loaded into a vector register first.
With the current implementation there is a potential problem when the
decision for the reg-reg variant is made very late. This patch is
supposed to fix that.
With the patch the required permute pattern is generated already in
the expander and attached to the bswap pattern as USE operand. The
predicate in the insn_and_split pattern accepts it although the
permute constant as such is not a valid constant. For the reg-reg
variant only the vector register constraint is used for the permute
constant forcing LRA to a) push the constant into literal pool and b)
load the literal pool constant into a vector register.
Bootstrapped and regression tested on s390x.
Committed to mainline
gcc/ChangeLog:
2019-04-12 Andreas Krebbel
* config/s390/predicates.md (permute_pattern_operand): New
predicate.
* config/s390/vector.md ("*vec_splats_bswap_vec"): Add USE
operand for the permute pattern.
("*vec_perm"): New insn definition.
("bswap"): Generate the permute pattern operand in the
expander and perform the operand reloads for pre arch13 level
already.
("*bswap_emu"): Rename to ...
("*bswap"): ... this. And make the splitter vxe2 only.
* config/s390/vx-builtins.md ("*vec_insert_and_zero_bswap"):
Add the USE operand for the permute pattern.
("*vec_set_bswap_vec"): Likewise.
---
gcc/config/s390/predicates.md | 10
gcc/config/s390/vector.md | 107 ++---
gcc/config/s390/vx-builtins.md | 18 ---
3 files changed, 89 insertions(+), 46 deletions(-)
diff --git a/gcc/config/s390/predicates.md b/gcc/config/s390/predicates.md
index 9a3d99e..92c602e 100644
--- a/gcc/config/s390/predicates.md
+++ b/gcc/config/s390/predicates.md
@@ -46,6 +46,16 @@
(and (match_code "symbol_ref, label_ref, const, const_int, const_wide_int,
const_double, const_vector")
(match_test "CONSTANT_P (op)")))
+; An operand used as vector permutation pattern
+
+; This in particular accepts constants which would otherwise be
+; rejected. These constants require special post reload handling
+
+(define_special_predicate "permute_pattern_operand"
+ (and (match_code "const_vector,mem,reg,subreg")
+ (match_test "GET_MODE (op) == V16QImode")
+ (match_test "!MEM_P (op) || s390_mem_constraint (\"R\", op)")))
+
;; Return true if OP is a valid S-type operand.
(define_predicate "s_operand"
diff --git a/gcc/config/s390/vector.md b/gcc/config/s390/vector.md
index 8f02af4..a2c1012 100644
--- a/gcc/config/s390/vector.md
+++ b/gcc/config/s390/vector.md
@@ -551,9 +551,10 @@
; vlbrreph, vlbrrepf, vlbrrepg
(define_insn "*vec_splats_bswap_vec"
- [(set (match_operand:V_HW_HSD 0 "register_operand"
"=v")
+ [(set (match_operand:V_HW_HSD 0 "register_operand"
"=v")
(bswap:V_HW_HSD
-(vec_duplicate:V_HW_HSD (match_operand: 1 "memory_operand"
"R"]
+(vec_duplicate:V_HW_HSD (match_operand: 1 "memory_operand"
"R"
+ (use (match_operand:V16QI 2
"permute_pattern_operand" "X"))]
"TARGET_VXE2"
"vlbrrep\t%v0,%1"
[(set_attr "op_type" "VRX")])
@@ -655,6 +656,17 @@
"vperm\t%v0,%v1,%v2,%v3"
[(set_attr "op_type" "VRR")])
+(define_insn "*vec_perm"
+ [(set (match_operand:VT_HW0
"register_operand" "=v")
+ (subreg:VT_HW (unspec:V16QI [(subreg:V16QI (match_operand:VT_HW 1
"register_operand" "v") 0)
+(subreg:V16QI (match_operand:VT_HW 2
"register_operand" "v") 0)
+(match_operand:V16QI 3
"register_operand" "v")]
+ UNSPEC_VEC_PERM) 0))]
+ "TARGET_VX"
+ "vperm\t%v0,%v1,%v2,%v3"
+ [(set_attr "op_type" "VRR")])
+
+
; vec_perm_const for V2DI using vpdi?
;;
@@ -2073,35 +2085,11 @@
; FIXME: The bswap rtl standard name currently does not appear to be
; used for vector modes.
(define_expand "bswap"
- [(set (match_operand:VT_HW_HSDT 0 "nonimmediate_operand"
"")
- (bswap:VT_HW_HSDT (match_operand:VT_HW_HSDT 1 "nonimmediate_operand"
"")))]
- "TARGET_VX")
-
-; vlbrh, vlbrf, vlbrg, vlbrq, vstbrh, vstbrf, vstbrg, vstbrq
-(define_insn "*bswap"
- [(set (match_operand:VT_HW_HSDT 0 "nonimmediate_operand"
"=v,v,R")
- (bswap:VT_HW_HSDT (match_operand:VT_HW_HSDT 1 "nonimmediate_operand"
"v,R,v")))]
- "TARGET_VXE2"
- "@
- #
- vlbr\t%v0,%v1
- vstbr\t%v1,%v0"
- [(set_attr "op_type" "*,VRX,VRX")])
-
-(define_insn_and_split "*bswap_emu"
- [(set (match_operand:VT_HW_HSDT
Re: Enable BF16 support (Please ignore my former email)
On Fri, Apr 12, 2019 at 9:09 AM Liu, Hongtao wrote: > > Hi : > This patch is about to enable support for bfloat16 which will be in > Future Cooper Lake, Please refer to > https://software.intel.com/en-us/download/intel-architecture-instruction-set-extensions-programming-reference > for more details about BF16. > > There are 3 instructions for AVX512BF16: VCVTNE2PS2BF16, VCVTNEPS2BF16 and > DPBF16PS instructions, which are Vector Neural Network Instructions > supporting: > > - VCVTNE2PS2BF16: Convert Two Packed Single Data to One Packed BF16 > Data. > - VCVTNEPS2BF16: Convert Packed Single Data to Packed BF16 Data. > - VDPBF16PS: Dot Product of BF16 Pairs Accumulated into Packed Single > Precision. > > Since only BF16 intrinsics are supported, we treat it as HI for simplicity. I think it was a mistake declaring cvtps2ph and cvtph2ps using HImode instead of HFmode. Is there a compelling reason not to introduce corresponding bf16_format supporting infrastructure and declare these intrinsics using half-binary (HBmode ?) mode instead? Uros.
[C/C++ PATCH] Fix ICE with typedef redeclaration with attributes (PR c/89933)
Hi!
In r234626 Marek has added code to remove TREE_TYPE (newdecl) from
its variant list for typedefs, because we'll ggc_free the TYPE_NAME of that
type. Unfortunately, that code will ICE if TREE_TYPE (newdecl) is its own
TYPE_MAIN_VARIANT. This can happen if due to attributes the newdecl's type
has been build_distinct_type_copy created but hasn't been type_hash_canon
merged (which we do for a few attributes like aligned, but certainly don't
do it for most other attributes). In the likely case there are no variants
for that type yet, there is just nothing to remove. If there are some (in
theory), the options are do what the following patch does, keep the type
in the variant list as the main variant, just change the TYPE_NAME, so that
it doesn't refer to ggc_freed TYPE_DECL, or try to pick up some other type
as the main variant and adjust the whole variant list (I don't think the
C/C++ FEs unlike Ada FE like e.g. qualified types as main variant though).
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Though, maybe we could do that TYPE_NAME (remove) = olddecl; unconditionally
and only do the removal from variant list conditionally. Would you prefer
that?
2019-04-12 Jakub Jelinek
PR c/89933
c/
* c-decl.c (merge_decls): When newdecl's type is its main variant,
don't try to remove it from the variant list, but instead change
TYPE_NAME to olddecl.
cp/
* decl.c (duplicate_decls): When newdecl's type is its main variant,
don't try to remove it from the variant list, but instead change
TYPE_NAME to olddecl.
testsuite/
* c-c++-common/pr89933.c: New test.
--- gcc/c/c-decl.c.jj 2019-03-20 12:24:57.896298849 +0100
+++ gcc/c/c-decl.c 2019-04-11 16:43:48.437746631 +0200
@@ -2512,13 +2512,16 @@ merge_decls (tree newdecl, tree olddecl,
if (TYPE_NAME (TREE_TYPE (newdecl)) == newdecl)
{
tree remove = TREE_TYPE (newdecl);
- for (tree t = TYPE_MAIN_VARIANT (remove); ;
- t = TYPE_NEXT_VARIANT (t))
- if (TYPE_NEXT_VARIANT (t) == remove)
- {
- TYPE_NEXT_VARIANT (t) = TYPE_NEXT_VARIANT (remove);
- break;
- }
+ if (TYPE_MAIN_VARIANT (remove) == remove)
+ TYPE_NAME (remove) = olddecl;
+ else
+ for (tree t = TYPE_MAIN_VARIANT (remove); ;
+t = TYPE_NEXT_VARIANT (t))
+ if (TYPE_NEXT_VARIANT (t) == remove)
+ {
+ TYPE_NEXT_VARIANT (t) = TYPE_NEXT_VARIANT (remove);
+ break;
+ }
}
}
--- gcc/cp/decl.c.jj2019-04-09 15:18:36.860833102 +0200
+++ gcc/cp/decl.c 2019-04-11 16:46:40.176923681 +0200
@@ -2132,13 +2132,16 @@ next_arg:;
if (TYPE_NAME (TREE_TYPE (newdecl)) == newdecl)
{
tree remove = TREE_TYPE (newdecl);
- for (tree t = TYPE_MAIN_VARIANT (remove); ;
- t = TYPE_NEXT_VARIANT (t))
- if (TYPE_NEXT_VARIANT (t) == remove)
- {
- TYPE_NEXT_VARIANT (t) = TYPE_NEXT_VARIANT (remove);
- break;
- }
+ if (TYPE_MAIN_VARIANT (remove) == remove)
+ TYPE_NAME (remove) = olddecl;
+ else
+ for (tree t = TYPE_MAIN_VARIANT (remove); ;
+t = TYPE_NEXT_VARIANT (t))
+ if (TYPE_NEXT_VARIANT (t) == remove)
+ {
+ TYPE_NEXT_VARIANT (t) = TYPE_NEXT_VARIANT (remove);
+ break;
+ }
}
}
else if (merge_attr)
--- gcc/testsuite/c-c++-common/pr89933.c.jj 2019-04-11 16:50:13.808415292
+0200
+++ gcc/testsuite/c-c++-common/pr89933.c2019-04-11 16:49:57.090689737
+0200
@@ -0,0 +1,5 @@
+/* PR c/89933 */
+/* { dg-do compile } */
+
+typedef unsigned int a __attribute__ ((__aligned__(8), __may_alias__));
+typedef unsigned int a __attribute__ ((__aligned__(8), __may_alias__));
Jakub
Re: [PATCH] Fix patchable_function_entry attribute handling (PR c/89946)
On Fri, 12 Apr 2019, Jakub Jelinek wrote:
> Hi!
>
> The following patch fixes various issues in patchable_function_entry
> handling:
> 1) most importantly, it adds a warning if the argument(s) aren't integer
>constants or if they are negative and drops the attribute
> 2) if (tree_fits_uhwi_p (x)) y = tree_to_uhwi (x); else gcc_unreachable ();
>makes no sense, as tree_to_uhwi has the same gcc_assert already
> 3) list_length () > 1 might look cool, but we should have guaranteed it
>is only 1 or 2 argument and even if it was longer, computing whole
>length and then comparing it against 1 is inefficient
> 4) warnings shouldn't start with a capital letter
> 5) formatting fixes
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Richard.
> 2019-04-12 Jakub Jelinek
>
> PR c/89946
> * varasm.c (assemble_start_function): Don't use tree_fits_uhwi_p
> and gcc_unreachable if it fails, just call tree_to_uhwi which
> verifies that too. Test TREE_CHAIN instead of list_length > 1.
> Start warning message with a lower-case letter. Formatting fixes.
> c-family/
> * c-attribs.c (handle_patchable_function_entry_attribute): Add
> function comment. Warn if arguments of the attribute are not positive
> integer constants.
> testsuite/
> * c-c++-common/pr89946.c: New test.
>
> --- gcc/varasm.c.jj 2019-02-28 08:14:58.438559862 +0100
> +++ gcc/varasm.c 2019-04-11 15:34:46.933248354 +0200
> @@ -1865,28 +1865,20 @@ assemble_start_function (tree decl, cons
>tree pp_val = TREE_VALUE (patchable_function_entry_attr);
>tree patchable_function_entry_value1 = TREE_VALUE (pp_val);
>
> - if (tree_fits_uhwi_p (patchable_function_entry_value1))
> - patch_area_size = tree_to_uhwi (patchable_function_entry_value1);
> - else
> - gcc_unreachable ();
> -
> + patch_area_size = tree_to_uhwi (patchable_function_entry_value1);
>patch_area_entry = 0;
> - if (list_length (pp_val) > 1)
> + if (TREE_CHAIN (pp_val) != NULL_TREE)
> {
> - tree patchable_function_entry_value2 =
> - TREE_VALUE (TREE_CHAIN (pp_val));
> -
> - if (tree_fits_uhwi_p (patchable_function_entry_value2))
> - patch_area_entry = tree_to_uhwi (patchable_function_entry_value2);
> - else
> - gcc_unreachable ();
> + tree patchable_function_entry_value2
> + = TREE_VALUE (TREE_CHAIN (pp_val));
> + patch_area_entry = tree_to_uhwi (patchable_function_entry_value2);
> }
> }
>
>if (patch_area_entry > patch_area_size)
> {
>if (patch_area_size > 0)
> - warning (OPT_Wattributes, "Patchable function entry > size");
> + warning (OPT_Wattributes, "patchable function entry > size");
>patch_area_entry = 0;
> }
>
> @@ -1906,7 +1898,8 @@ assemble_start_function (tree decl, cons
>/* And the area after the label. Record it if we haven't done so yet. */
>if (patch_area_size > patch_area_entry)
> targetm.asm_out.print_patchable_function_entry (asm_out_file,
> - patch_area_size-patch_area_entry,
> + patch_area_size
> + - patch_area_entry,
> patch_area_entry == 0);
>
>if (lookup_attribute ("no_split_stack", DECL_ATTRIBUTES (decl)))
> --- gcc/c-family/c-attribs.c.jj 2019-04-08 10:11:26.706250584 +0200
> +++ gcc/c-family/c-attribs.c 2019-04-11 15:51:45.851326928 +0200
> @@ -3988,10 +3988,29 @@ handle_fallthrough_attribute (tree *, tr
>return NULL_TREE;
> }
>
> +/* Handle a "patchable_function_entry" attributes; arguments as in
> + struct attribute_spec.handler. */
> +
> static tree
> -handle_patchable_function_entry_attribute (tree *, tree, tree, int, bool *)
> +handle_patchable_function_entry_attribute (tree *, tree name, tree args,
> +int, bool *no_add_attrs)
> {
> - /* Nothing to be done here. */
> + for (; args; args = TREE_CHAIN (args))
> +{
> + tree val = TREE_VALUE (args);
> + if (val && TREE_CODE (val) != IDENTIFIER_NODE
> + && TREE_CODE (val) != FUNCTION_DECL)
> + val = default_conversion (val);
> +
> + if (!tree_fits_uhwi_p (val))
> + {
> + warning (OPT_Wattributes,
> +"%qE attribute argument %qE is not an integer constant",
> +name, val);
> + *no_add_attrs = true;
> + return NULL_TREE;
> + }
> +}
>return NULL_TREE;
> }
>
> --- gcc/testsuite/c-c++-common/pr89946.c.jj 2019-04-11 15:52:33.616532035
> +0200
> +++ gcc/testsuite/c-c++-common/pr89946.c 2019-04-11 15:51:29.303602307
> +0200
> @@ -0,0 +1,7 @@
> +/* PR c/89946 */
> +
> +__attribute__((patchable_function_entry (-1))) void foo (void) {}/* {
> dg-warning "'patchable_function_entry'
Re: [PATCH] Fix up RTL cfg cleanup in cfglayout mode with __builtin_unreachable (PR rtl-optimization/90026)
On Fri, 12 Apr 2019, Jakub Jelinek wrote:
> Hi!
>
> In the following testcase, we have initially during RTL expansion a call
> which can throw, followed by fallthrough into an empty block with two
> similar predecessors (both calling the same function) and with a fallthrough
> into an empty block with no successors (__builtin_unreachable).
> That last block is optimized away soon and in the cfglayout mode we have
> a bb with no successors that has a BARRIER in BB_FOOTER. During cfg cleanup
> during combine we actually optimize away even that other empty bb with no
> successors, but end up not putting a BARRIER into BB_FOOTER of the bb with a
> call. There is in the following hunk in try_optimize_cfg code to handle
> that, but a) it assumes a BARRIER has to be the first in BB_FOOTER, which
> as all other spots I've found don't assume, even for the non-cfglayout
> mode a few lines below we call get_last_bb_insn which walks the following
> insn chain (note, this isn't a problem on this exact testcase though)
> and b) punt if e->src bb already has non-NULL BB_FOOTER (that is what
> breaks this testcase - the testcase has a NOTE_INSN_DELETED_LABEL in
> BB_FOOTER, but of course no BARRIER, we don't add one here and later on
> when outof_cfglayout we ICE because there is no BARRIER after a bb that
> doesn't fall thru). Apparently there is a function which can handle
> adding a barrier, both into an empty BB_FOOTER as well as when there is
> a BB_FOOTER, but doesn't contain a BARRIER in there.
>
> So, this patch fixes both issues.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
LGTM.
Richard.
> 2019-04-12 Jakub Jelinek
>
> PR rtl-optimization/90026
> * cfgcleanup.c (try_optimize_cfg): When removing empty bb with no
> successors, look for BARRIERs inside of the whole BB_FOOTER chain
> rather than just at the start of it. If e->src BB_FOOTER is not NULL
> in cfglayout mode, use emit_barrier_after_bb.
>
> * g++.dg/opt/pr90026.C: New test.
>
> --- gcc/cfgcleanup.c.jj 2019-03-09 09:25:11.743785011 +0100
> +++ gcc/cfgcleanup.c 2019-04-11 11:30:52.967173100 +0200
> @@ -2712,23 +2712,23 @@ try_optimize_cfg (int mode)
>
> if (current_ir_type () == IR_RTL_CFGLAYOUT)
> {
> - if (BB_FOOTER (b)
> - && BARRIER_P (BB_FOOTER (b)))
> + rtx_insn *insn;
> + for (insn = BB_FOOTER (b);
> +insn; insn = NEXT_INSN (insn))
> + if (BARRIER_P (insn))
> + break;
> + if (insn)
> FOR_EACH_EDGE (e, ei, b->preds)
> - if ((e->flags & EDGE_FALLTHRU)
> - && BB_FOOTER (e->src) == NULL)
> + if ((e->flags & EDGE_FALLTHRU))
> {
> - if (BB_FOOTER (b))
> + if (BB_FOOTER (b)
> + && BB_FOOTER (e->src) == NULL)
> {
> BB_FOOTER (e->src) = BB_FOOTER (b);
> BB_FOOTER (b) = NULL;
> }
> else
> - {
> - start_sequence ();
> - BB_FOOTER (e->src) = emit_barrier ();
> - end_sequence ();
> - }
> + emit_barrier_after_bb (e->src);
> }
> }
> else
> --- gcc/testsuite/g++.dg/opt/pr90026.C.jj 2019-04-11 12:40:14.508840308
> +0200
> +++ gcc/testsuite/g++.dg/opt/pr90026.C2019-04-11 12:39:43.431352423
> +0200
> @@ -0,0 +1,24 @@
> +// PR rtl-optimization/90026
> +// { dg-do compile }
> +// { dg-options "-fnon-call-exceptions -ftracer -O2 -w" }
> +
> +typedef __SIZE_TYPE__ size_t;
> +struct S { int *b; ~S () { delete b; } };
> +void bar ();
> +char c[sizeof (int)];
> +
> +void *
> +operator new (size_t, void *)
> +{
> + __builtin_unreachable ();
> +}
> +
> +void
> +foo ()
> +{
> + S a;
> + if (a.b)
> +a.b = new int ();
> + bar ();
> + new (c) int ();
> +}
>
> Jakub
>
--
Richard Biener
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg)
[PATCH] Fix patchable_function_entry attribute handling (PR c/89946)
Hi!
The following patch fixes various issues in patchable_function_entry
handling:
1) most importantly, it adds a warning if the argument(s) aren't integer
constants or if they are negative and drops the attribute
2) if (tree_fits_uhwi_p (x)) y = tree_to_uhwi (x); else gcc_unreachable ();
makes no sense, as tree_to_uhwi has the same gcc_assert already
3) list_length () > 1 might look cool, but we should have guaranteed it
is only 1 or 2 argument and even if it was longer, computing whole
length and then comparing it against 1 is inefficient
4) warnings shouldn't start with a capital letter
5) formatting fixes
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2019-04-12 Jakub Jelinek
PR c/89946
* varasm.c (assemble_start_function): Don't use tree_fits_uhwi_p
and gcc_unreachable if it fails, just call tree_to_uhwi which
verifies that too. Test TREE_CHAIN instead of list_length > 1.
Start warning message with a lower-case letter. Formatting fixes.
c-family/
* c-attribs.c (handle_patchable_function_entry_attribute): Add
function comment. Warn if arguments of the attribute are not positive
integer constants.
testsuite/
* c-c++-common/pr89946.c: New test.
--- gcc/varasm.c.jj 2019-02-28 08:14:58.438559862 +0100
+++ gcc/varasm.c2019-04-11 15:34:46.933248354 +0200
@@ -1865,28 +1865,20 @@ assemble_start_function (tree decl, cons
tree pp_val = TREE_VALUE (patchable_function_entry_attr);
tree patchable_function_entry_value1 = TREE_VALUE (pp_val);
- if (tree_fits_uhwi_p (patchable_function_entry_value1))
- patch_area_size = tree_to_uhwi (patchable_function_entry_value1);
- else
- gcc_unreachable ();
-
+ patch_area_size = tree_to_uhwi (patchable_function_entry_value1);
patch_area_entry = 0;
- if (list_length (pp_val) > 1)
+ if (TREE_CHAIN (pp_val) != NULL_TREE)
{
- tree patchable_function_entry_value2 =
- TREE_VALUE (TREE_CHAIN (pp_val));
-
- if (tree_fits_uhwi_p (patchable_function_entry_value2))
- patch_area_entry = tree_to_uhwi (patchable_function_entry_value2);
- else
- gcc_unreachable ();
+ tree patchable_function_entry_value2
+ = TREE_VALUE (TREE_CHAIN (pp_val));
+ patch_area_entry = tree_to_uhwi (patchable_function_entry_value2);
}
}
if (patch_area_entry > patch_area_size)
{
if (patch_area_size > 0)
- warning (OPT_Wattributes, "Patchable function entry > size");
+ warning (OPT_Wattributes, "patchable function entry > size");
patch_area_entry = 0;
}
@@ -1906,7 +1898,8 @@ assemble_start_function (tree decl, cons
/* And the area after the label. Record it if we haven't done so yet. */
if (patch_area_size > patch_area_entry)
targetm.asm_out.print_patchable_function_entry (asm_out_file,
-patch_area_size-patch_area_entry,
+ patch_area_size
+ - patch_area_entry,
patch_area_entry == 0);
if (lookup_attribute ("no_split_stack", DECL_ATTRIBUTES (decl)))
--- gcc/c-family/c-attribs.c.jj 2019-04-08 10:11:26.706250584 +0200
+++ gcc/c-family/c-attribs.c2019-04-11 15:51:45.851326928 +0200
@@ -3988,10 +3988,29 @@ handle_fallthrough_attribute (tree *, tr
return NULL_TREE;
}
+/* Handle a "patchable_function_entry" attributes; arguments as in
+ struct attribute_spec.handler. */
+
static tree
-handle_patchable_function_entry_attribute (tree *, tree, tree, int, bool *)
+handle_patchable_function_entry_attribute (tree *, tree name, tree args,
+ int, bool *no_add_attrs)
{
- /* Nothing to be done here. */
+ for (; args; args = TREE_CHAIN (args))
+{
+ tree val = TREE_VALUE (args);
+ if (val && TREE_CODE (val) != IDENTIFIER_NODE
+ && TREE_CODE (val) != FUNCTION_DECL)
+ val = default_conversion (val);
+
+ if (!tree_fits_uhwi_p (val))
+ {
+ warning (OPT_Wattributes,
+ "%qE attribute argument %qE is not an integer constant",
+ name, val);
+ *no_add_attrs = true;
+ return NULL_TREE;
+ }
+}
return NULL_TREE;
}
--- gcc/testsuite/c-c++-common/pr89946.c.jj 2019-04-11 15:52:33.616532035
+0200
+++ gcc/testsuite/c-c++-common/pr89946.c2019-04-11 15:51:29.303602307
+0200
@@ -0,0 +1,7 @@
+/* PR c/89946 */
+
+__attribute__((patchable_function_entry (-1))) void foo (void) {} /* {
dg-warning "'patchable_function_entry' attribute argument '-1' is not an
integer constant" } */
+__attribute__((patchable_function_entry (5, -5))) void bar (void) {} /* {
dg-warning "'patchable_function_entry' attribute argument '-5' is not an
integer
Enable BF16 support (Please ignore my former email)
Hi : This patch is about to enable support for bfloat16 which will be in Future Cooper Lake, Please refer to https://software.intel.com/en-us/download/intel-architecture-instruction-set-extensions-programming-reference for more details about BF16. There are 3 instructions for AVX512BF16: VCVTNE2PS2BF16, VCVTNEPS2BF16 and DPBF16PS instructions, which are Vector Neural Network Instructions supporting: - VCVTNE2PS2BF16: Convert Two Packed Single Data to One Packed BF16 Data. - VCVTNEPS2BF16: Convert Packed Single Data to Packed BF16 Data. - VDPBF16PS: Dot Product of BF16 Pairs Accumulated into Packed Single Precision. Since only BF16 intrinsics are supported, we treat it as HI for simplicity. Bootstrap and regression test for x86/i386 backend are ok. Changelog: 2019-04-07Wei Xiao gcc/: * common/config/i386/i386-common.c (OPTION_MASK_ISA_AVX512BF16_SET, OPTION_MASK_ISA_AVX512BF16_UNSET, OPTION_MASK_ISA2_AVX512BW_UNSET ): New. (OPTION_MASK_ISA2_AVX512F_UNSET): Add OPTION_MASK_ISA_AVX512BF16_UNSET. (ix86_handle_option): Handle -mavx512bf16. * config.gcc: Add avx512bf16vlintrin.h and avx512bf16intrin.h to extra_headers. * config/i386/avx512bf16vlintrin.h: New. * config/i386/avx512bf16intrin.h: New. * config/i386/cpuid.h (bit_AVX512BF16): New. * config/i386/driver-i386.c (host_detect_local_cpu): Detect BF16. * config/i386/i386-builtin-types.def: Add new types. * config/i386/i386-builtin.def: Add new builtins. * config/i386/i386-c.c (ix86_target_macros_internal): Define __AVX512BF16__. * config/i386/i386.c (ix86_target_string): Add -mavx512bf16. (ix86_option_override_internal): Handle BF16. (ix86_valid_target_attribute_inner_p): Ditto. (fold_builtin_cpu): Ditto. (ix86_expand_args_builtin): Ditto. * config/i386/i386.h (TARGET_AVX512BF16, TARGET_AVX512BF16_P): New. (PTA_AVX512BF16): Ditto. * config/i386/i386.opt: Add -mavx512bf16. * config/i386/immintrin.h: Include avx512bf16intrin.h and avx512bf16vlintrin.h. * config/i386/sse.md (avx512f_cvtne2ps2bf16_, avx512f_cvtneps2bf16_, avx512f_dpbf16ps_): New define_insn patterns. * config/i386/subst.md (mask_half): Add new subst. * doc/invoke.texi: Document -mavx512bf16. gcc/testsuite/: * gcc.target/i386/avx512bf16-vcvtne2ps2bf16-1.c: New test. * gcc.target/i386/avx512bf16-vcvtneps2bf16-1.c: New test. * gcc.target/i386/avx512bf16-vdpbf16ps-1.c: New test. * gcc.target/i386/avx512bf16vl-vcvtne2ps2bf16-1.c: New test. * gcc.target/i386/avx512bf16vl-vcvtneps2bf16-1.c: New test. * gcc.target/i386/avx512bf16vl-vdpbf16ps-1.c: New test. * gcc.target/i386/sse-12.c: Add -mavx512bf16. * gcc.target/i386/sse-13.c: Ditto. * gcc.target/i386/sse-14.c: Ditto. * gcc.target/i386/sse-22.c: Ditto. * gcc.target/i386/sse-23.c: Ditto. * g++.dg/other/i386-2.C: Ditto. * gcc.target/i386/avx-1.c: Ditto. * gcc.target/i386/avx-2.c: Ditto. * g++.dg/other/i386-3.C: Add avx512bf16. Regards Hongtao Liu 0001-Enable-BF16-support.patch Description: 0001-Enable-BF16-support.patch
[PATCH] Fix up RTL cfg cleanup in cfglayout mode with __builtin_unreachable (PR rtl-optimization/90026)
Hi!
In the following testcase, we have initially during RTL expansion a call
which can throw, followed by fallthrough into an empty block with two
similar predecessors (both calling the same function) and with a fallthrough
into an empty block with no successors (__builtin_unreachable).
That last block is optimized away soon and in the cfglayout mode we have
a bb with no successors that has a BARRIER in BB_FOOTER. During cfg cleanup
during combine we actually optimize away even that other empty bb with no
successors, but end up not putting a BARRIER into BB_FOOTER of the bb with a
call. There is in the following hunk in try_optimize_cfg code to handle
that, but a) it assumes a BARRIER has to be the first in BB_FOOTER, which
as all other spots I've found don't assume, even for the non-cfglayout
mode a few lines below we call get_last_bb_insn which walks the following
insn chain (note, this isn't a problem on this exact testcase though)
and b) punt if e->src bb already has non-NULL BB_FOOTER (that is what
breaks this testcase - the testcase has a NOTE_INSN_DELETED_LABEL in
BB_FOOTER, but of course no BARRIER, we don't add one here and later on
when outof_cfglayout we ICE because there is no BARRIER after a bb that
doesn't fall thru). Apparently there is a function which can handle
adding a barrier, both into an empty BB_FOOTER as well as when there is
a BB_FOOTER, but doesn't contain a BARRIER in there.
So, this patch fixes both issues.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2019-04-12 Jakub Jelinek
PR rtl-optimization/90026
* cfgcleanup.c (try_optimize_cfg): When removing empty bb with no
successors, look for BARRIERs inside of the whole BB_FOOTER chain
rather than just at the start of it. If e->src BB_FOOTER is not NULL
in cfglayout mode, use emit_barrier_after_bb.
* g++.dg/opt/pr90026.C: New test.
--- gcc/cfgcleanup.c.jj 2019-03-09 09:25:11.743785011 +0100
+++ gcc/cfgcleanup.c2019-04-11 11:30:52.967173100 +0200
@@ -2712,23 +2712,23 @@ try_optimize_cfg (int mode)
if (current_ir_type () == IR_RTL_CFGLAYOUT)
{
- if (BB_FOOTER (b)
- && BARRIER_P (BB_FOOTER (b)))
+ rtx_insn *insn;
+ for (insn = BB_FOOTER (b);
+ insn; insn = NEXT_INSN (insn))
+ if (BARRIER_P (insn))
+ break;
+ if (insn)
FOR_EACH_EDGE (e, ei, b->preds)
- if ((e->flags & EDGE_FALLTHRU)
- && BB_FOOTER (e->src) == NULL)
+ if ((e->flags & EDGE_FALLTHRU))
{
- if (BB_FOOTER (b))
+ if (BB_FOOTER (b)
+ && BB_FOOTER (e->src) == NULL)
{
BB_FOOTER (e->src) = BB_FOOTER (b);
BB_FOOTER (b) = NULL;
}
else
- {
- start_sequence ();
- BB_FOOTER (e->src) = emit_barrier ();
- end_sequence ();
- }
+ emit_barrier_after_bb (e->src);
}
}
else
--- gcc/testsuite/g++.dg/opt/pr90026.C.jj 2019-04-11 12:40:14.508840308
+0200
+++ gcc/testsuite/g++.dg/opt/pr90026.C 2019-04-11 12:39:43.431352423 +0200
@@ -0,0 +1,24 @@
+// PR rtl-optimization/90026
+// { dg-do compile }
+// { dg-options "-fnon-call-exceptions -ftracer -O2 -w" }
+
+typedef __SIZE_TYPE__ size_t;
+struct S { int *b; ~S () { delete b; } };
+void bar ();
+char c[sizeof (int)];
+
+void *
+operator new (size_t, void *)
+{
+ __builtin_unreachable ();
+}
+
+void
+foo ()
+{
+ S a;
+ if (a.b)
+a.b = new int ();
+ bar ();
+ new (c) int ();
+}
Jakub
Re: [PATCH, PR d/89255][1/3] Add -fbuilding-libphobos-tests
Patch updated and committed to trunk as r270301.
--
Iain
---
2019-04-12 Iain Buclaw
* d-tree.h (DECL_IN_UNITTEST_CONDITION_P): Define.
* decl.cc (DeclVisitor): Add in_version_unittest_ field.
(DeclVisitor::visit(ConditionalDeclaration)): New override.
(DeclVisitor::visit(FuncDeclaration)): Set
DECL_IN_UNITTEST_CONDITION_P.
* lang.opt (-fbuilding-libphobos-tests): Add option.
* modules.cc (current_testing_module): New static variable.
(build_module_tree): Generate second moduleinfo symbol to hold
reference to unittests if flag_building_libphobos_tests.
(register_module_decl): Check DECL_IN_UNITTEST_CONDITION_P to decide
which moduleinfo the decl should be registered against.
---
diff --git a/gcc/d/d-tree.h b/gcc/d/d-tree.h
index 0b3c5eddedd..cff832cc645 100644
--- a/gcc/d/d-tree.h
+++ b/gcc/d/d-tree.h
@@ -59,7 +59,8 @@ typedef Array Expressions;
Usage of DECL_LANG_FLAG_?:
0: LABEL_VARIABLE_CASE (in LABEL_DECL).
- DECL_BUILT_IN_CTFE (in FUNCTION_DECL). */
+ DECL_BUILT_IN_CTFE (in FUNCTION_DECL).
+ 1: DECL_IN_UNITTEST_CONDITION_P (in FUNCTION_DECL). */
/* The kinds of scopes we recognize. */
@@ -380,6 +381,10 @@ lang_tree_node
#define DECL_BUILT_IN_CTFE(NODE) \
(DECL_LANG_FLAG_0 (FUNCTION_DECL_CHECK (NODE)))
+/* True if the decl is only compiled in when unittests are turned on. */
+#define DECL_IN_UNITTEST_CONDITION_P(NODE) \
+ (DECL_LANG_FLAG_1 (FUNCTION_DECL_CHECK (NODE)))
+
enum d_tree_index
{
DTI_VTABLE_ENTRY_TYPE,
diff --git a/gcc/d/decl.cc b/gcc/d/decl.cc
index fffed97727f..f6c863988e3 100644
--- a/gcc/d/decl.cc
+++ b/gcc/d/decl.cc
@@ -21,6 +21,7 @@ along with GCC; see the file COPYING3. If not see
#include "dmd/aggregate.h"
#include "dmd/attrib.h"
+#include "dmd/cond.h"
#include "dmd/ctfe.h"
#include "dmd/declaration.h"
#include "dmd/enum.h"
@@ -121,9 +122,13 @@ class DeclVisitor : public Visitor
{
using Visitor::visit;
+ /* If we're lowering the body of a version(unittest) condition. */
+ bool in_version_unittest_;
+
public:
DeclVisitor (void)
{
+this->in_version_unittest_ = false;
}
/* This should be overridden by each declaration class. */
@@ -241,6 +246,25 @@ public:
visit ((AttribDeclaration *) d);
}
+ /* Conditional compilation is the process of selecting which code to compile
+ and which code to not compile. Look for version conditions that may */
+
+ void visit (ConditionalDeclaration *d)
+ {
+bool old_condition = this->in_version_unittest_;
+
+if (global.params.useUnitTests)
+ {
+ VersionCondition *vc = d->condition->isVersionCondition ();
+ if (vc && vc->ident == Identifier::idPool ("unittest"))
+ this->in_version_unittest_ = true;
+ }
+
+visit ((AttribDeclaration *) d);
+
+this->in_version_unittest_ = old_condition;
+ }
+
/* Walk over all members in the namespace scope. */
void visit (Nspace *d)
@@ -868,6 +892,7 @@ public:
}
DECL_ARGUMENTS (fndecl) = param_list;
+DECL_IN_UNITTEST_CONDITION_P (fndecl) = this->in_version_unittest_;
rest_of_decl_compilation (fndecl, 1, 0);
/* If this is a member function that nested (possibly indirectly) in another
diff --git a/gcc/d/lang.opt b/gcc/d/lang.opt
index 523f73c90de..f65be444d45 100644
--- a/gcc/d/lang.opt
+++ b/gcc/d/lang.opt
@@ -197,6 +197,10 @@ Enum(bounds_check) String(safeonly) Value(1)
EnumValue
Enum(bounds_check) String(on) Value(2)
+; Generates a secondary ModuleInfo symbol for linking in unittests
+fbuilding-libphobos-tests
+D Undocumented Var(flag_building_libphobos_tests)
+
fbuiltin
D Var(flag_no_builtin, 0)
; Documented in C
diff --git a/gcc/d/modules.cc b/gcc/d/modules.cc
index e9bd44115f9..315f5d82356 100644
--- a/gcc/d/modules.cc
+++ b/gcc/d/modules.cc
@@ -110,6 +110,11 @@ enum module_info_flags
static module_info *current_moduleinfo;
+/* When compiling with -fbuilding-libphobos-tests, this contains information
+ about the module that gets compiled in only when unittests are enabled. */
+
+static module_info *current_testing_module;
+
/* The declaration of the current module being compiled. */
static Module *current_module_decl;
@@ -706,8 +711,10 @@ build_module_tree (Module *decl)
assert (!current_moduleinfo && !current_module_decl);
module_info mi = module_info ();
+ module_info mitest = module_info ();
current_moduleinfo =
+ current_testing_module =
current_module_decl = decl;
/* Layout module members. */
@@ -720,6 +727,53 @@ build_module_tree (Module *decl)
}
}
+ /* For libphobos-internal use only. Generate a separate module info symbol
+ that references all compiled in unittests, this allows compiling library
+ modules and linking to libphobos without having run-time conflicts because
+ of two ModuleInfo records with the same name being present in two DSOs. */
+ if (flag_building_libphobos_tests)
+{
[PATCH, d] Committed merge with upstream dmd
Hi,
This patch merges the D front-end implementation with dmd upstream c185f9df1.
Adds new virtual isVersionCondition, this is so that in the code
generation pass, a ConditionDeclaration's condition can be identified
without requiring a Visitor function.
Bootstrapped and regression tested on x86_64-linux-gnu.
Committed to trunk as r270300.
--
Iain
---
diff --git a/gcc/d/dmd/MERGE b/gcc/d/dmd/MERGE
index 800be95e4e6..be0c5a50da2 100644
--- a/gcc/d/dmd/MERGE
+++ b/gcc/d/dmd/MERGE
@@ -1,4 +1,4 @@
-d7ed327edb0b01ad56e7e73e77b3401cd565675e
+c185f9df1789456c7d88d047f2df23dd784f1182
The first line of this file holds the git revision number of the last
merge done from the dlang/dmd repository.
diff --git a/gcc/d/dmd/cond.h b/gcc/d/dmd/cond.h
index 891969be48d..8e33b16a9da 100644
--- a/gcc/d/dmd/cond.h
+++ b/gcc/d/dmd/cond.h
@@ -39,6 +39,7 @@ public:
virtual Condition *syntaxCopy() = 0;
virtual int include(Scope *sc, ScopeDsymbol *sds) = 0;
virtual DebugCondition *isDebugCondition() { return NULL; }
+virtual VersionCondition *isVersionCondition() { return NULL; }
virtual void accept(Visitor *v) { v->visit(this); }
};
@@ -91,6 +92,7 @@ public:
VersionCondition(Module *mod, unsigned level, Identifier *ident);
int include(Scope *sc, ScopeDsymbol *sds);
+VersionCondition *isVersionCondition() { return this; }
void accept(Visitor *v) { v->visit(this); }
};
Re: [PR89528] reset debug uses of return value when dropping dead RTL call
On April 12, 2019 3:28:20 AM GMT+02:00, Alexandre Oliva
wrote:
>On Apr 5, 2019, Richard Biener wrote:
>
>> Looks OK but can you adjust the testcase to actually test
>> something?
>
>Doh, I *knew* I'd failed to do something I should have ;-)
>
>
>I resorted to the trick you used in pr89892.c, of using a more complex
>expression to get UNSUPPORTED results instead of FAILs when the call
>and
>thus the variable are optimized out. How's this?
Looks good!
Thanks,
Richard.
>
>[PR89528] reset debug uses of return value when dropping dead RTL call
>
>When we remove an RTL call, we wouldn't clean up references to the
>return value of the call in debug insns. Make it so that we do.
>
>
>for gcc/ChangeLog
>
> PR debug/89528
> * valtrack.c (dead_debug_insert_temp): Reset debug references
> to the return value of a call being removed.
>
>for gcc/testsuite/ChangeLog
>
> PR debug/89528
> * gcc.dg/guality/pr89528.c: New.
>---
>gcc/testsuite/gcc.dg/guality/pr89528.c | 25 +
> gcc/valtrack.c | 22 ++
> 2 files changed, 31 insertions(+), 16 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/guality/pr89528.c
>
>diff --git a/gcc/testsuite/gcc.dg/guality/pr89528.c
>b/gcc/testsuite/gcc.dg/guality/pr89528.c
>new file mode 100644
>index ..04a7e84d8755
>--- /dev/null
>+++ b/gcc/testsuite/gcc.dg/guality/pr89528.c
>@@ -0,0 +1,25 @@
>+/* PR debug/89528 */
>+/* { dg-do run } */
>+/* { dg-options "-g" } */
>+
>+#include
>+
>+char b;
>+int d, e;
>+static int i = 1;
>+void a(int l) { printf("", l); }
>+char c(char l) { return l || b && l == 1 ? b : b % l; }
>+short f(int l, int m) { return l * m; }
>+short g(short l, short m) { return m || l == 767 && m == 1; }
>+int h(int l, int m) { return (l ^ m & l ^ (m & 647) - m ^ m) < m; }
>+static int j(int l) { return d == 0 || l == 647 && d == 1 ? l : l % d;
>}
>+short k(int l) { return l >= 2 >> l; }
>+void optimize_me_not() { asm(""); }
>+static short n(void) {
>+ int l_1127 = ~j(9 || 0) ^ 65535;
>+ optimize_me_not(); /* { dg-final { gdb-test . "l_1127+1" "-65534" }
>} */
>+ f(l_1127, i && e ^ 4) && g(0, 0);
>+ e = 0;
>+ return 5;
>+}
>+int main() { n(); }
>diff --git a/gcc/valtrack.c b/gcc/valtrack.c
>index 9b2bb333c0a3..1f67378a867c 100644
>--- a/gcc/valtrack.c
>+++ b/gcc/valtrack.c
>@@ -657,22 +657,12 @@ dead_debug_insert_temp (struct dead_debug_local
>*debug, unsigned int uregno,
> {
> dest = SET_DEST (set);
> src = SET_SRC (set);
>-/* Lose if the REG-setting insn is a CALL. */
>-if (GET_CODE (src) == CALL)
>- {
>-while (uses)
>- {
>-cur = uses->next;
>-XDELETE (uses);
>-uses = cur;
>- }
>-return 0;
>- }
>-/* Asm in DEBUG_INSN is never useful, we can't emit debug info for
>- that. And for volatile_insn_p, it is actually harmful
>- - DEBUG_INSNs shouldn't have any side-effects. */
>-else if (GET_CODE (src) == ASM_OPERANDS
>- || volatile_insn_p (src))
>+/* Reset uses if the REG-setting insn is a CALL. Asm in
>+ DEBUG_INSN is never useful, we can't emit debug info for
>+ that. And for volatile_insn_p, it is actually harmful -
>+ DEBUG_INSNs shouldn't have any side-effects. */
>+if (GET_CODE (src) == CALL || GET_CODE (src) == ASM_OPERANDS
>+|| volatile_insn_p (src))
> set = NULL_RTX;
> }
>
