Re: [PR c++/84789] do not resolve typename into template-independent
On Mar 20, 2018, Jason Merrill wrote:
> On Tue, Mar 20, 2018 at 6:07 PM, Alexandre Oliva wrote:
>> On Mar 20, 2018, Jason Merrill wrote:
>>> that doesn't mean it's wrong to peek.
>> Huh? We're referencing members of an unrelated template that AFAIK
>> needs not even be defined at that point, and even if it is, it could
>> still be specialized afterwards. How can it possibly be right to
>> short-circuit such nested names?
>> template struct B : A {}; // would /*: A {}*/ make any diff?
>> template struct C : B // would /* : B*/ make any diff?
>> {
>> B::A::I::I i; // { dg-error "typename" }
>> };
> No, we look inside when we're trying to parse the qualified-id as the
> name of a declaration
Yeah, but that's not the case at hand. I guess we're miscommunicating.
I understood you were saying it was ok to peek in this case. Would you
please state, for clarity, what your stance is on peeking in this case,
specifically, in the B::A::I::I within the definition of C above?
> in a declarator-id we can look into uninstantiated classes, otherwise
> there would be no way to define members of class templates.
> void X::N::f() { } // looks inside X
Of course, but then, we wouldn't get to a template-independent member.
A member of a template-dependent name is still template-dependent. It's
only when a qualified name maps to an external name that it might become
template-independent, and when this happens, I believe the qualified-id
is not one that can be used to define a member. Specifically:
struct K { struct L { static double j; }; };
template struct M { struct N { static int i; }; };
template struct O { typedef M P; typedef K Q; };
template int O::P::N::i = 42;
template double O::Q::L::j = 42.0;
if we remap O::P to M and O::Q to K, how will we realize the
given type-ids are not appropriate? Where will the template parmlist
belong when the qualified-id is taken as equivalent to K::L::j?
FWIW, we silently swallow the definition of i above (I think we
shouldn't), and we fail the same assert in resolve_typename_type when
parsing the definition of j.
So, you see, we do have problems with declarators, and we have problems
with types. AFAICT, we shouldn't simplify qualified-ids in declarators,
otherwise we'd fail to issue errors for the definitions above, and we
shouldn't simplify qualified-ids in types involving templates other than
the one we'd in.
So it's not clear to me what purpose the resolver is supposed to serve,
considering that simplifying the types seem to almost never be wanted.
But one issue is whether we can peek into dependent types, another is
whether we can and should simplify them. The answers to these questions
don't seem to be very clear to me. What is clear to me is that
refraining from simplifying from dependent to non-dependent enables us
to report errors that we were missing in both declarators and types, and
that simplifying from dependent to unrelated dependent prevents us from
reporting the error we should in the definition of i above. But is
there any case in which *not* simplifying causes problems?
>>> I disagree; it seems to me that the assert should allow the case where
>>> the scope was originally dependent, but got resolved earlier in the
>>> function.
>>
>> Doesn't the function always take dependent scopes? I for some reason
>> thought that was the case.
> Yes, as the comment says, a TYPENAME_TYPE should always have a
> dependent scope. In this case, the dependent scope was "typename
> B::A", but just above we resolved it to just A, making it no longer
> dependent.
Then you got me thoroughly confused: what did you mean by 'the assert
should allow' a case that's a given?
--
Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/ FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
Re: [PR c++/71251] out-of-range parms in tmpl arg substitution
On Mar 20, 2018, Jason Merrill wrote: >> + if (id == error_mark_node) >> +return error_mark_node; > Why wait until here to return? There are error returns immediately > above and below your first hunk. QOI. Returning immediately, we then get other errors. We could consume tokens till the end of the declaration, but I figured we might as well try to parse them and see whether there were any other legitimate errors to report. -- Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/ You must be the change you wish to see in the world. -- Gandhi Be Free! -- http://FSFLA.org/ FSF Latin America board member Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
Re: [PATCH 2/2] C++: show private field accessor hints with -g and optimization (PR c++/84994)
On Tue, Mar 20, 2018 at 7:37 PM, David Malcolm wrote: > PR c++/84894 reports that the fix-it hints suggesting accessor calls for > private fields doesn't work with -g for -O1 and above. > > The issue is that field_accessor_p requires DECL_SAVED_TREE (fn) to be > a RETURN_EXPR, but the former is a STATEMENT_LIST, created in > start_preparsed_function here: Would constexpr_fn_retval be useful here? Jason
Re: [PATCH 1/2] C++: show private field accessor hints for const accesses (PR c++/84892)
On Tue, Mar 20, 2018 at 7:37 PM, David Malcolm wrote:
> PR c++/84892 reports that we fail to offer fix-it hints about accessor
> member functions when complaining about accesses to private fields
> through const pointers and references.
>
> It turns out that field_accessor_p was incorrectly checking for "this"
> being a "const *T", rather than being a "const T*".
> We were also missing test coverage for this ("offer it for const") case
> (we were only checking for the "don't offer it for non-const" case).
>
> Successfully bootstrapped and regression-tested on x86_64-pc-linux-gnu.
>
> OK for trunk? (not technically a regression, but affects a new feature)
>
> gcc/cp/ChangeLog:
> PR c++/84892
> * search.c (field_accessor_p): Use class_of_this_parm rather than
> type_of_this_parm, to check that "this" is a "const T *", rather
> than a "T const *".
^ T *const
(const T* and T const* are the same type).
The patch is OK.
Jason
Re: [PR c++/71251] out-of-range parms in tmpl arg substitution
On Tue, Mar 20, 2018 at 6:51 PM, Alexandre Oliva wrote: > On Mar 20, 2018, Jason Merrill wrote: > >> On Tue, Mar 20, 2018 at 4:15 PM, Alexandre Oliva wrote: >>> Should we aim at rejecting the declaration of U? > >> Yes. > > Like this? > > [PR c++/71251] check tmpl parms in template using decl > > Check that template using decls have the correct number of parm lists. > > Will regstrap shortly, ok to install if it passes? > > for gcc/cp/ChangeLog > > PR c++/71251 > * parser.c (cp_parser_alias_declaration): Call > parser_check_template_parameters. > > for gcc/testsuite/ChangeLog > > PR c++/71251 > * g++.dg/cpp0x/pr71251.C: New. > --- > gcc/cp/parser.c | 10 ++ > gcc/testsuite/g++.dg/cpp0x/pr71251.C | 15 +++ > 2 files changed, 25 insertions(+) > create mode 100644 gcc/testsuite/g++.dg/cpp0x/pr71251.C > > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c > index ce05615adfba..8fa6a37c82f0 100644 > --- a/gcc/cp/parser.c > +++ b/gcc/cp/parser.c > @@ -18910,6 +18910,13 @@ cp_parser_alias_declaration (cp_parser* parser) >if (id == error_mark_node) > return error_mark_node; > > + if (parser->num_template_parameter_lists > + && !cp_parser_check_template_parameters (parser, > + /*num_templates=*/0, > + id_location, > + /*declarator=*/NULL)) > +id = error_mark_node; >cp_token *attrs_token = cp_lexer_peek_token (parser->lexer); >attributes = cp_parser_attributes_opt (parser); >if (attributes == error_mark_node) > @@ -18980,6 +18987,9 @@ cp_parser_alias_declaration (cp_parser* parser) >ds_alias, >using_token); > > + if (id == error_mark_node) > +return error_mark_node; Why wait until here to return? There are error returns immediately above and below your first hunk. Jason
Re: [PR c++/84789] do not resolve typename into template-independent
On Tue, Mar 20, 2018 at 6:07 PM, Alexandre Oliva wrote:
> On Mar 20, 2018, Jason Merrill wrote:
>
>> On Fri, Mar 16, 2018 at 5:38 PM, Alexandre Oliva wrote:
>>> resolve_typename_type may peek into template types that might still be
>>> specialized. In some cases, e.g. g++.dg/template/friend48.C or
>>> g++.dg/template/decl2.C, that is exactly the right thing to do. In
>>> others, like the newly-added testcase g++.dg/template/pr84789.C, it
>>> isn't, and if the qualifying scope happens to resolve to a non-template
>>> type, we resolve to that and then fail the assert that checks we still
>>> have a template-dependent scope.
>
>> We're looking inside them because we're trying to parse a declarator;
>> the tentative parse will fail in this case, because we aren't in a
>> declarator, but that doesn't mean it's wrong to peek.
>
> Huh? We're referencing members of an unrelated template that AFAIK
> needs not even be defined at that point, and even if it is, it could
> still be specialized afterwards. How can it possibly be right to
> short-circuit such nested names? How would template
> instantiation/specialization get a chance to do the proper mapping?
>
> template struct B : A {}; // would /*: A {}*/ make any diff?
> template struct C : B // would /* : B*/ make any diff?
> {
> B::A::I::I i; // { dg-error "typename" }
> };
> Is it by any chance the fact that B is a base class for C that
> makes it correct to peek into it? I don't recall any exception of this
> sort.
No, we look inside when we're trying to parse the qualified-id as the
name of a declaration; in a declarator-id we can look into
uninstantiated classes, otherwise there would be no way to define
members of class templates.
void X::N::f() { } // looks inside X
>> I disagree; it seems to me that the assert should allow the case where
>> the scope was originally dependent, but got resolved earlier in the
>> function.
>
> Doesn't the function always take dependent scopes? I for some reason
> thought that was the case.
Yes, as the comment says, a TYPENAME_TYPE should always have a
dependent scope. In this case, the dependent scope was "typename
B::A", but just above we resolved it to just A, making it no longer
dependent.
Jason
[Committed] PR fortran/85001 -- Fix a non non-null pointer issue
The following patch fixes an assign of a valid pointer to
to pointer, but what should have been assigned is NULL;
2018-03-20 Steven G. Kargl
PR fortran/85001
* interface.c (symbol_rank): Remove bogus null pointer check that
crept in when translating a ternary operator into an if-else
constructor.
2018-03-20 Steven G. Kargl
PR fortran/85001
* gfortran.dg/interface_41.f90: New test.
Index: gcc/fortran/interface.c
===
--- gcc/fortran/interface.c (revision 258646)
+++ gcc/fortran/interface.c (working copy)
@@ -1268,7 +1268,7 @@ symbol_rank (gfc_symbol *sym)
{
gfc_array_spec *as = NULL;
- if (sym->ts.type == BT_CLASS && CLASS_DATA (sym) && CLASS_DATA (sym)->as)
+ if (sym->ts.type == BT_CLASS && CLASS_DATA (sym))
as = CLASS_DATA (sym)->as;
else
as = sym->as;
Index: gcc/testsuite/gfortran.dg/interface_41.f90
===
--- gcc/testsuite/gfortran.dg/interface_41.f90 (nonexistent)
+++ gcc/testsuite/gfortran.dg/interface_41.f90 (working copy)
@@ -0,0 +1,19 @@
+! { dg-do compile }
+! PR fortran/85001
+! Contributed by Gerhard Steinmetz.
+program p
+ type t
+ end type
+ call s
+contains
+ real function f(x)
+ class(t) :: x
+ dimension :: x(:)
+ f = 1.0
+ end
+ subroutine s
+ type(t) :: x(2)
+ real :: z
+ z = f(x) ! { dg-error "Rank mismatch in argument" }
+ end
+end
--
Steve
Re: Seeking Release Manager approval for: [PATCH] jit: fix link on OS X and Solaris (PR jit/64089 and PR jit/84288)
On Wed, 2018-03-21 at 00:39 +0100, Rainer Orth wrote: > Hi Malcolm, > > > > I've now tested the patch (together with the one from PR > > > jit/84288 > > > for > > > several remaining issues). I've needed another snippet for > > > Solaris/SPARC which links libkstat into xgcc and needs it in > > > libgccjit.so, too. Bootstrapped without regressions on > > > i386-pc-solaris2.11 and sparc-sun-solaris2.11. > > > > FWIW I've successfully tested this on x86_64-pc-linux-gnu > > (regenerating > > the gcc/configure), and, as jit maintainer, it looks good to me > > [1], > > though it may still need RM approval given stage 4. > > thanks for trying this. > > > [1] ...though I have a slight preference for listing > > $(EXTRA_GCC_LIBS) on the same line as $(EXTRA_GCC_OBJS) in > > jit/Make- > > lang.in, so that these two items needed to embed the driver code > > into > > the libgccjit shared library are visually grouped together. > > I've selected the location of $(EXTRA_GCC_LIBS) in the link line to > match what gcc/Makefile.in does for xgcc etc. Indeed, I don't want to bikeshed it - I care much more about whether it works ;)
Re: Seeking Release Manager approval for: [PATCH] jit: fix link on OS X and Solaris (PR jit/64089 and PR jit/84288)
Hi Malcolm, >> I've now tested the patch (together with the one from PR jit/84288 >> for >> several remaining issues). I've needed another snippet for >> Solaris/SPARC which links libkstat into xgcc and needs it in >> libgccjit.so, too. Bootstrapped without regressions on >> i386-pc-solaris2.11 and sparc-sun-solaris2.11. > > FWIW I've successfully tested this on x86_64-pc-linux-gnu (regenerating > the gcc/configure), and, as jit maintainer, it looks good to me [1], > though it may still need RM approval given stage 4. thanks for trying this. > [1] ...though I have a slight preference for listing > $(EXTRA_GCC_LIBS) on the same line as $(EXTRA_GCC_OBJS) in jit/Make- > lang.in, so that these two items needed to embed the driver code into > the libgccjit shared library are visually grouped together. I've selected the location of $(EXTRA_GCC_LIBS) in the link line to match what gcc/Makefile.in does for xgcc etc. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
[PATCH 2/2] C++: show private field accessor hints with -g and optimization (PR c++/84994)
PR c++/84894 reports that the fix-it hints suggesting accessor calls for private fields doesn't work with -g for -O1 and above. The issue is that field_accessor_p requires DECL_SAVED_TREE (fn) to be a RETURN_EXPR, but the former is a STATEMENT_LIST, created in start_preparsed_function here: 15060 /* Start the statement-tree, start the tree now. */ 15061 DECL_SAVED_TREE (decl1) = push_stmt_list (); This works for -O0 and without -g since cp/decl.c: finish_function modifies it here: 15636 /* If we're saving up tree structure, tie off the function now. */ 15637 DECL_SAVED_TREE (fndecl) = pop_stmt_list (DECL_SAVED_TREE (fndecl)); which strips away the stmt list, giving the RETURN_EXPR, but for -g with -O1 and above, the STATEMENT_LIST is preserved. This patch fixes the issue by introducing a new function: get_any_single_nondebug_stmt_in_list to extract the single non-debug stmt from a STATEMENT_LIST, if there is one, using it within field_accessor_p. It moves the existing testcases to the g++.dg/torture subdirectory, so that they are run for a variety of debug and optimization combinations. Successfully bootstrapped and regression-tested on x86_64-pc-linux-gnu. OK for trunk? (not technically a regression, but affects a new feature) gcc/c-family/ChangeLog: PR c++/84994 * c-common.h (get_any_single_nondebug_stmt_in_list): New decl. * c-semantics.c (get_any_single_nondebug_stmt_in_list): New function. gcc/cp/ChangeLog: PR c++/84994 * search.c (field_accessor_p): Call get_any_single_nondebug_stmt_in_list on DECL_SAVED_TREE (fn). gcc/testsuite/ChangeLog: * g++.dg/other/accessor-fixits-1.C: Move to... * g++.dg/torture/accessor-fixits-1.C: ...here. * g++.dg/other/accessor-fixits-2.C: Move to... * g++.dg/torture/accessor-fixits-2.C: ...here. * g++.dg/other/accessor-fixits-3.C: Move to... * g++.dg/torture/accessor-fixits-3.C: ...here. * g++.dg/other/accessor-fixits-4.C: Move to... * g++.dg/torture/accessor-fixits-4.C: ...here. * g++.dg/other/accessor-fixits-5.C: Move to... * g++.dg/torture/accessor-fixits-5.C: ...here. * g++.dg/torture/accessor-fixits-6.C: New testcase. * g++.dg/torture/accessor-fixits-7.C: New testcase. * g++.dg/torture/accessor-fixits-8.C: New testcase. --- gcc/c-family/c-common.h | 1 + gcc/c-family/c-semantics.c | 34 gcc/cp/search.c | 5 + gcc/testsuite/g++.dg/other/accessor-fixits-1.C | 222 --- gcc/testsuite/g++.dg/other/accessor-fixits-2.C | 104 --- gcc/testsuite/g++.dg/other/accessor-fixits-3.C | 15 -- gcc/testsuite/g++.dg/other/accessor-fixits-4.C | 48 - gcc/testsuite/g++.dg/other/accessor-fixits-5.C | 33 gcc/testsuite/g++.dg/torture/accessor-fixits-1.C | 222 +++ gcc/testsuite/g++.dg/torture/accessor-fixits-2.C | 104 +++ gcc/testsuite/g++.dg/torture/accessor-fixits-3.C | 15 ++ gcc/testsuite/g++.dg/torture/accessor-fixits-4.C | 48 + gcc/testsuite/g++.dg/torture/accessor-fixits-5.C | 33 gcc/testsuite/g++.dg/torture/accessor-fixits-6.C | 22 +++ gcc/testsuite/g++.dg/torture/accessor-fixits-7.C | 22 +++ gcc/testsuite/g++.dg/torture/accessor-fixits-8.C | 29 +++ 16 files changed, 535 insertions(+), 422 deletions(-) delete mode 100644 gcc/testsuite/g++.dg/other/accessor-fixits-1.C delete mode 100644 gcc/testsuite/g++.dg/other/accessor-fixits-2.C delete mode 100644 gcc/testsuite/g++.dg/other/accessor-fixits-3.C delete mode 100644 gcc/testsuite/g++.dg/other/accessor-fixits-4.C delete mode 100644 gcc/testsuite/g++.dg/other/accessor-fixits-5.C create mode 100644 gcc/testsuite/g++.dg/torture/accessor-fixits-1.C create mode 100644 gcc/testsuite/g++.dg/torture/accessor-fixits-2.C create mode 100644 gcc/testsuite/g++.dg/torture/accessor-fixits-3.C create mode 100644 gcc/testsuite/g++.dg/torture/accessor-fixits-4.C create mode 100644 gcc/testsuite/g++.dg/torture/accessor-fixits-5.C create mode 100644 gcc/testsuite/g++.dg/torture/accessor-fixits-6.C create mode 100644 gcc/testsuite/g++.dg/torture/accessor-fixits-7.C create mode 100644 gcc/testsuite/g++.dg/torture/accessor-fixits-8.C diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h index 95bb0fd..67d0ed2 100644 --- a/gcc/c-family/c-common.h +++ b/gcc/c-family/c-common.h @@ -581,6 +581,7 @@ extern tree push_stmt_list (void); extern tree pop_stmt_list (tree); extern tree add_stmt (tree); extern void push_cleanup (tree, tree, bool); +extern tree get_any_single_nondebug_stmt_in_list (tree); extern tree build_modify_expr (location_t, tree, tree, enum tree_code, location_t, tree, tree); diff --git a/gcc/c-family/c-semantics.c b/gcc/c-family/c-semantics.c index 322b26b..fbe204a 100644 --- a/gcc/c-family/c-semantics.c +++ b/gcc/c-f
[PATCH 1/2] C++: show private field accessor hints for const accesses (PR c++/84892)
PR c++/84892 reports that we fail to offer fix-it hints about accessor
member functions when complaining about accesses to private fields
through const pointers and references.
It turns out that field_accessor_p was incorrectly checking for "this"
being a "const *T", rather than being a "const T*".
We were also missing test coverage for this ("offer it for const") case
(we were only checking for the "don't offer it for non-const" case).
Successfully bootstrapped and regression-tested on x86_64-pc-linux-gnu.
OK for trunk? (not technically a regression, but affects a new feature)
gcc/cp/ChangeLog:
PR c++/84892
* search.c (field_accessor_p): Use class_of_this_parm rather than
type_of_this_parm, to check that "this" is a "const T *", rather
than a "T const *".
gcc/testsuite/ChangeLog:
PR c++/84892
* g++.dg/other/accessor-fixits-1.C
(test_access_const_t1_color): New.
(test_deref_const_t1_color): New.
* g++.dg/other/accessor-fixits-5.C: New testcase.
---
gcc/cp/search.c| 4 +--
gcc/testsuite/g++.dg/other/accessor-fixits-1.C | 44 ++
gcc/testsuite/g++.dg/other/accessor-fixits-5.C | 33 +++
3 files changed, 79 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/other/accessor-fixits-5.C
diff --git a/gcc/cp/search.c b/gcc/cp/search.c
index 796209f..ddcff69 100644
--- a/gcc/cp/search.c
+++ b/gcc/cp/search.c
@@ -1747,8 +1747,8 @@ field_accessor_p (tree fn, tree field_decl, bool const_p)
that the "this" parameter is const. */
if (const_p)
{
- tree this_type = type_of_this_parm (fntype);
- if (!TYPE_READONLY (this_type))
+ tree this_class = class_of_this_parm (fntype);
+ if (!TYPE_READONLY (this_class))
return false;
}
diff --git a/gcc/testsuite/g++.dg/other/accessor-fixits-1.C
b/gcc/testsuite/g++.dg/other/accessor-fixits-1.C
index cc96b87..fd46a52 100644
--- a/gcc/testsuite/g++.dg/other/accessor-fixits-1.C
+++ b/gcc/testsuite/g++.dg/other/accessor-fixits-1.C
@@ -35,6 +35,28 @@ int test_access_t1_color (t1 &ref)
{ dg-end-multiline-output "" } */
}
+int test_access_const_t1_color (const t1 &ref)
+{
+ return ref.m_color; // { dg-error ".int t1::m_color. is private within this
context" }
+ /* { dg-begin-multiline-output "" }
+ return ref.m_color;
+ ^~~
+ { dg-end-multiline-output "" } */
+
+
+ /* { dg-begin-multiline-output "" }
+ int m_color;
+ ^~~
+ { dg-end-multiline-output "" } */
+
+ // { dg-message "field .int t1::m_color. can be accessed via .int
t1::get_color\\(\\) const." "" { target *-*-* } .-12 }
+ /* { dg-begin-multiline-output "" }
+ return ref.m_color;
+ ^~~
+ get_color()
+ { dg-end-multiline-output "" } */
+}
+
int test_access_t1_shape (t1 &ref)
{
return ref.m_shape; // { dg-error ".int t1::m_shape. is protected within
this context" }
@@ -79,6 +101,28 @@ int test_deref_t1_color (t1 *ptr)
{ dg-end-multiline-output "" } */
}
+int test_deref_const_t1_color (const t1 *ptr)
+{
+ return ptr->m_color; // { dg-error ".int t1::m_color. is private within this
context" }
+ /* { dg-begin-multiline-output "" }
+ return ptr->m_color;
+ ^~~
+ { dg-end-multiline-output "" } */
+
+
+ /* { dg-begin-multiline-output "" }
+ int m_color;
+ ^~~
+ { dg-end-multiline-output "" } */
+
+ // { dg-message "field .int t1::m_color. can be accessed via .int
t1::get_color\\(\\) const." "" { target *-*-* } .-12 }
+ /* { dg-begin-multiline-output "" }
+ return ptr->m_color;
+ ^~~
+ get_color()
+ { dg-end-multiline-output "" } */
+}
+
int test_deref_t1_shape (t1 *ptr)
{
return ptr->m_shape; // { dg-error ".int t1::m_shape. is protected within
this context" }
diff --git a/gcc/testsuite/g++.dg/other/accessor-fixits-5.C
b/gcc/testsuite/g++.dg/other/accessor-fixits-5.C
new file mode 100644
index 000..cf72d78
--- /dev/null
+++ b/gcc/testsuite/g++.dg/other/accessor-fixits-5.C
@@ -0,0 +1,33 @@
+// PR c++/84892
+// { dg-options "-fdiagnostics-show-caret" }
+
+class S {
+private:
+ bool field;
+
+public:
+ bool get_field() const {
+return field;
+ }
+};
+
+bool thingy(const S & s) {
+ return s.field; // { dg-error "'bool S::field' is private within this
context" }
+ /* { dg-begin-multiline-output "" }
+ return s.field;
+^
+ { dg-end-multiline-output "" } */
+
+ // { dg-message "declared private here" "" { target *-*-* } 6 }
+ /* { dg-begin-multiline-output "" }
+ bool field;
+^
+ { dg-end-multiline-output "" } */
+
+ // { dg-message "field 'bool S::field' can be accessed via 'bool
S::get_field\\(\\) const'" "" { target *-*-* } .-12 }
+ /* { dg-begin-multiline-output "" }
+ return s.field;
+^
+get_field()
+ { dg-end-multiline-output "" } */
+}
--
Re: [PR c++/71251] out-of-range parms in tmpl arg substitution
On Mar 20, 2018, Jason Merrill wrote:
> On Tue, Mar 20, 2018 at 4:15 PM, Alexandre Oliva wrote:
>> Should we aim at rejecting the declaration of U?
> Yes.
Like this?
[PR c++/71251] check tmpl parms in template using decl
Check that template using decls have the correct number of parm lists.
Will regstrap shortly, ok to install if it passes?
for gcc/cp/ChangeLog
PR c++/71251
* parser.c (cp_parser_alias_declaration): Call
parser_check_template_parameters.
for gcc/testsuite/ChangeLog
PR c++/71251
* g++.dg/cpp0x/pr71251.C: New.
---
gcc/cp/parser.c | 10 ++
gcc/testsuite/g++.dg/cpp0x/pr71251.C | 15 +++
2 files changed, 25 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/pr71251.C
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index ce05615adfba..8fa6a37c82f0 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -18910,6 +18910,13 @@ cp_parser_alias_declaration (cp_parser* parser)
if (id == error_mark_node)
return error_mark_node;
+ if (parser->num_template_parameter_lists
+ && !cp_parser_check_template_parameters (parser,
+ /*num_templates=*/0,
+ id_location,
+ /*declarator=*/NULL))
+id = error_mark_node;
+
cp_token *attrs_token = cp_lexer_peek_token (parser->lexer);
attributes = cp_parser_attributes_opt (parser);
if (attributes == error_mark_node)
@@ -18980,6 +18987,9 @@ cp_parser_alias_declaration (cp_parser* parser)
ds_alias,
using_token);
+ if (id == error_mark_node)
+return error_mark_node;
+
declarator = make_id_declarator (NULL_TREE, id, sfk_none);
declarator->id_loc = id_location;
diff --git a/gcc/testsuite/g++.dg/cpp0x/pr71251.C
b/gcc/testsuite/g++.dg/cpp0x/pr71251.C
new file mode 100644
index ..3df831bb581d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/pr71251.C
@@ -0,0 +1,15 @@
+// { dg-do compile { target c++11 } }
+
+template template
+using U = void; // { dg-error "too many" }
+
+template
+using V = void;
+
+template struct X {
+ template template
+ using U = void; // { dg-error "too many" }
+
+ template
+ using V = void;
+};
--
Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/ FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
Re: [PATCH] Fix VEC_DUPLICATE simplification (PR rtl-optimization/84989)
On 03/20/2018 02:46 PM, Jakub Jelinek wrote: > Hi! > > On the following testcase, simplify_unary_operation is called on > VEC_DUPLICATE from (vec_duplicate:V4SF something:SF) to V8SFmode, > and simplify_unary_operation_1 tries an optimization usable for most > unary operations, in particular it attempts to do > (vec_duplicate:V8SF (unary:SF something:SF)) > which is reasonable for all unary ops other than when unary is > vec_duplicate, because that needs a vector outer mode and scalar or vector > inner mode, not scalar outer and inner mode. > > Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for > trunk? > > 2018-03-20 Jakub Jelinek > > PR rtl-optimization/84989 > * simplify-rtx.c (simplify_unary_operation_1): Don't try to simplify > VEC_DUPLICATE with scalar result mode. > > * gcc.target/i386/pr84989.c: New test. OK. jeff
Re: [PATCH][PR target/84838] Minor grammar fixes for x86 options
On 03/12/2018 08:56 PM, David H. Gutteridge wrote: > 2018-03-12 David H. Gutteridge > > PR target/84838 > * Minor grammar fixes for x86 options. > THanks. Installed. jeff
Re: [Patch] test_summary: handle single quotes
On 02/28/2018 02:30 AM, Christophe Lyon wrote: > Hi, > > We happen to build GCC with '~' in the src and build dirs, leading to > single quotes around the configure path in config.status. For example: > [...] > S["TOPLEVEL_CONFIGURE_ARGUMENTS"]="'/home/christophe.lyon/src/Linaro/abe/abe-contrib-summary/mybuild/snapshots/gcc.git~linaro~gcc-7-branch/configure' > SHELL=/bin/bash --with-mpc=/home/"\ > [...] > > This confuses contrib/test_summary which matches "/configure .* in its > awk script, which fails to match when we have "/configure' XXX" > instead. > > This patch fixes that by adding matches for the optional single > quotes. I use \047 to match the ascii "'", because I couldn't find how > to quote this properly in the awk script. > > OK for trunk? OK. jeff
[PATCH, PR84660] Fix combine bug with SHIFT_COUNT_TRUNCATED.
This fixes a wrong-code issue on RISC-V, but in theory could be a problem for
any SHIFT_COUNT_TRUNCATED target.
The testcase computes 46 or 47 (0x2e or 0x2f), then ANDs the value with 0xf,
and then SHIFTs the value. On a SHIFT_COUNT_TRUNCATED target, the AND can get
optimized away because for a 32-bit shift we can use the 46/47 directly and
still get the correct result. Combine then tries to convert the 32-bit shift
into a 64-bit shift, and then we have a problem, as the AND 0xf is no longer
redundant. So we must prevent combine from converting a 32-bit shift to a
64-bit shift on a SHIFT_COUNT_TRUNCATED target when there are non-zero bits
in the shift count that matter for the larger shift mode.
Combine already has code to handle the case where shifts are being narrowed
and this accidentally changes the shift amount. This patch adds additional
code to handle the case where shifts are being widened.
This was tested with a cross riscv64-linux toolchain build and make check.
The new testcase fails without the patch, and works with the patch. This was
also tested with an x86_64-linux build and make check. There were no
regressions.
OK?
Jim
gcc/
PR rtl-optimization/84660
* combine.c (force_int_to_mode) : If SHIFT_COUNT_TRUNCATED,
call nonzero_bits and compare against xmode precision.
gcc/testsuite/
PR rtl-optimization/84660
* gcc.dg/pr84660.c: New.
---
gcc/combine.c | 10 --
gcc/testsuite/gcc.dg/pr84660.c | 17 +
2 files changed, 25 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/pr84660.c
diff --git a/gcc/combine.c b/gcc/combine.c
index ff672ad2adb..4ed59eb88c8 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -8897,14 +8897,20 @@ force_int_to_mode (rtx x, scalar_int_mode mode,
scalar_int_mode xmode,
However, we cannot do anything with shifts where we cannot
guarantee that the counts are smaller than the size of the mode
because such a count will have a different meaning in a
-wider mode. */
+different mode. If we are narrowing the mode, the shift count must
+be compared against mode. If we are widening the mode, and shift
+counts are truncated, then the shift count must be compared against
+xmode. */
if (! (CONST_INT_P (XEXP (x, 1))
&& INTVAL (XEXP (x, 1)) >= 0
&& INTVAL (XEXP (x, 1)) < GET_MODE_PRECISION (mode))
&& ! (GET_MODE (XEXP (x, 1)) != VOIDmode
&& (nonzero_bits (XEXP (x, 1), GET_MODE (XEXP (x, 1)))
- < (unsigned HOST_WIDE_INT) GET_MODE_PRECISION (mode
+ < (unsigned HOST_WIDE_INT) GET_MODE_PRECISION (mode))
+ && (! SHIFT_COUNT_TRUNCATED
+ || (nonzero_bits (XEXP (x, 1), GET_MODE (XEXP (x, 1)))
+ < (unsigned HOST_WIDE_INT) GET_MODE_PRECISION
(xmode)
break;
/* If the shift count is a constant and we can do arithmetic in
diff --git a/gcc/testsuite/gcc.dg/pr84660.c b/gcc/testsuite/gcc.dg/pr84660.c
new file mode 100644
index 000..a87fa0a914d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr84660.c
@@ -0,0 +1,17 @@
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+extern void abort (void);
+extern void exit (int);
+
+unsigned int __attribute__ ((noinline, noclone))
+foo(unsigned int i) {
+
+ return 0x & (0xd066 << (((i & 0x1) ^ 0x2f) & 0xf));
+}
+
+int main() {
+ if (foo (1) != 0x8000)
+abort ();
+ exit (0);
+}
--
2.14.1
Re: [PR c++/84789] do not resolve typename into template-independent
On Mar 20, 2018, Jason Merrill wrote:
> On Fri, Mar 16, 2018 at 5:38 PM, Alexandre Oliva wrote:
>> resolve_typename_type may peek into template types that might still be
>> specialized. In some cases, e.g. g++.dg/template/friend48.C or
>> g++.dg/template/decl2.C, that is exactly the right thing to do. In
>> others, like the newly-added testcase g++.dg/template/pr84789.C, it
>> isn't, and if the qualifying scope happens to resolve to a non-template
>> type, we resolve to that and then fail the assert that checks we still
>> have a template-dependent scope.
> We're looking inside them because we're trying to parse a declarator;
> the tentative parse will fail in this case, because we aren't in a
> declarator, but that doesn't mean it's wrong to peek.
Huh? We're referencing members of an unrelated template that AFAIK
needs not even be defined at that point, and even if it is, it could
still be specialized afterwards. How can it possibly be right to
short-circuit such nested names? How would template
instantiation/specialization get a chance to do the proper mapping?
template struct B : A {}; // would /*: A {}*/ make any diff?
template struct C : B // would /* : B*/ make any diff?
{
B::A::I::I i; // { dg-error "typename" }
};
Is it by any chance the fact that B is a base class for C that
makes it correct to peek into it? I don't recall any exception of this
sort.
> I disagree; it seems to me that the assert should allow the case where
> the scope was originally dependent, but got resolved earlier in the
> function.
Doesn't the function always take dependent scopes? I for some reason
thought that was the case.
--
Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/ FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
Re: [PATCH] [PR c++/84610,84642] recover from implicit template parms gracefully
On Tue, Mar 20, 2018 at 5:57 PM, Alexandre Oliva wrote:
> On Mar 20, 2018, Jason Merrill wrote:
>
>>> While debugging this, I first tried another patch, that avoids the same
>>> ICEs. I thought this one was a more complete solution, and it renders
>>> the other unnecessary, but I still though it might be useful to disable
>>> auto->implicit_parm while parsing declarators, so as to avoid useless
>>> processing.
>
>> Better, I think, to disable it while parsing attributes; we could
>> still encounter auto as a template argument in a declarator.
>
> How about this? Regstrapping, ok to install if it passes?
>
> Disable auto_is_implicit_function_template_parm_p while parsing attributes
>
> for gcc/cp/ChangeLog
>
> PR c++/84610
> PR c++/84642
> * parser.c (temp_override): New template class,
> generalizing...
> (cp_parser_parameter_declaration_clause): ... this cleanup.
> Use it.
> (cp_parser_gnu_attributes_opt): Use it.
> (cp_parser_std_attribute): Use it.
> ---
> gcc/cp/parser.c | 36 ++--
> 1 file changed, 26 insertions(+), 10 deletions(-)
>
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 5cc201604166..ce05615adfba 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -255,6 +255,21 @@ static bool cp_parser_omp_declare_reduction_exprs
> static void cp_finalize_oacc_routine
>(cp_parser *, tree, bool);
>
> +template
> +class temp_override
> +{
> + T& overridden_variable;
> + T saved_value;
> +public:
> + temp_override(T& var) : overridden_variable (var), saved_value (var) {}
> + temp_override(T& var, T overrider)
> +: overridden_variable (var), saved_value (var)
> + {
> +overridden_variable = overrider;
> + }
> + ~temp_override() { overridden_variable = saved_value; }
> +};
Let's put this in cp-tree.h, with warning_sentinel.
> + (void)cleanup;
There are lots of RAII variables without this explicit cast to void,
and they don't seem to have been a problem; let's drop it here as
well.
Jason
Re: [PR c++/84729] convert new init to array elt type
On Tue, Mar 20, 2018 at 5:56 PM, Alexandre Oliva wrote:
> On Mar 20, 2018, Jason Merrill wrote:
>
>> On Sat, Mar 10, 2018 at 6:57 AM, Alexandre Oliva wrote:
>>> A parenthesized initializer is only accepted when new()ing an array in
>>> permissive mode. We were not careful, however, to convert the
>>> TREE_LIST initializer to the array element type in this extension.
>>> This patch fixes it: after turning the TREE_LIST initializer to a
>>> compound_expr, we convert it to the base type.
>
>> I think I'd rather turn the permerror into a hard error than improve
>> support for a deprecated extension.
>
> Like this?
>
> [PR c++/84729] convert new init to array elt type
>
> A parenthesized initializer was only accepted when new()ing an array in
> permissive mode. We were not careful, however, to convert the
> TREE_LIST initializer to the array element type in this extension.
>
> Instead of fixing it, converting the initializer to the base type
> after turning the TREE_LIST initializer to a compound_expr, we disable
> this deprecated extension.
>
> Regstrapping. Ok to install if it passes?
>
> for gcc/cp/ChangeLog
>
> PR c++/84729
> * init.c (build_vec_init): Convert tree list to base type.
>
> for gcc/testsuite/ChangeLog
>
> PR c++/84729
> * g++.dg/pr84729.C: New.
> ---
> gcc/cp/init.c |4 ++--
> gcc/testsuite/g++.dg/pr84729.C |7 +++
> 2 files changed, 9 insertions(+), 2 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/pr84729.C
>
> diff --git a/gcc/cp/init.c b/gcc/cp/init.c
> index 15cee17c780c..9091eaa90267 100644
> --- a/gcc/cp/init.c
> +++ b/gcc/cp/init.c
> @@ -3370,8 +3370,8 @@ build_new_1 (vec **placement, tree type,
> tree nelts,
> else if (*init)
> {
>if (complain & tf_error)
> -permerror (input_location,
> - "parenthesized initializer in array new");
> +error_at (input_location,
> + "parenthesized initializer in array new");
>else
> return error_mark_node;
I suspect you'll need to make the return unconditional to avoid the
ICE; OK either way.
Jason
[PATCH] Fix trunk failing to build for msp430-elf
Trunk doesn't build for msp430 due to an ICE whilst building libstdc++. The following patch proposed a while ago fixes the issue, but hasn't been applied: https://gcc.gnu.org/ml/gcc-patches/2017-08/msg01433.html I don't have write access, so if the patch is acceptable I would appreciate if someone could commit it for me. Thanks.
Re: [PATCH] [PR c++/71965] silence multi-dim array init sorry without tf_error
On Tue, Mar 20, 2018 at 5:55 PM, Alexandre Oliva wrote:
> On Mar 20, 2018, Jason Merrill wrote:
>
>> On Sat, Mar 17, 2018 at 8:11 AM, Alexandre Oliva wrote:
>>> - sorry
>>> - ("cannot initialize multi-dimensional array with
>>> initializer");
>
>> This shouldn't even be a sorry anymore
>
>> Let's make it a hard error here.
>
> Like this?
>
>
> [PR c++/71965] silence multi-dim array init sorry without tf_error
>
> We shouldn't substitute templates into short-circuited-out concepts
> constraints, but we do, and to add insult to injury, we issue a
> sorry() error when a concept that shouldn't even have been substituted
> attempts to perform a multi-dimensional array initialization with a
> new{} expression.
>
> Although fixing the requirements short-circuiting is probably too
> risky at this point, we can get closer to the intended effect by
> silencing that sorry just as we silence other errors.
>
> Regstrapping... Ok to install if it passes?
>
> for gcc/cp/ChangeLog
>
> PR c++/71965
> * init.c (build_vec_init): Silence error, former sorry,
> without tf_error.
>
> for gcc/testsuite/ChangeLog
>
> PR c++/71965
> * g++.dg/concepts/pr71965.C: New.
> ---
> gcc/cp/init.c | 19 ---
> gcc/testsuite/g++.dg/concepts/pr71965.C | 27 +++
> 2 files changed, 39 insertions(+), 7 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/concepts/pr71965.C
>
> diff --git a/gcc/cp/init.c b/gcc/cp/init.c
> index 9091eaa90267..5dd4b407d73f 100644
> --- a/gcc/cp/init.c
> +++ b/gcc/cp/init.c
> @@ -4384,12 +4384,17 @@ build_vec_init (tree base, tree maxindex, tree init,
>else if (TREE_CODE (type) == ARRAY_TYPE)
> {
> if (init && !BRACE_ENCLOSED_INITIALIZER_P (init))
> - sorry
> - ("cannot initialize multi-dimensional array with initializer");
> - elt_init = build_vec_init (build1 (INDIRECT_REF, type, base),
> -0, init,
> -explicit_value_init_p,
> -0, complain);
> + {
> + if ((complain & tf_error))
> + error ("cannot initialize multi-dimensional"
> + " array with initializer");
Let's also use the other diagnostic message: "array must be
initialized with a brace-enclosed initializer".
OK with that change.
Jason
Re: [C++ PATCH] Fix ICE with inline asm and MODIFY_EXPR/preinc/predec in output operand (PR c++/84961)
On Tue, Mar 20, 2018 at 5:04 PM, Jakub Jelinek wrote:
> Hi!
>
> While in C, x = 10 or ++x or --x aren't lvalues and so we reject such
> expressions in inline asm output operands (and inputs that only allow
> memory, not registers), in C++ they apparently are lvalues; for output
> operands we ICE in the gimplifier on this, because in the generic code
> MODIFY_EXPR or PREINCREMENT_EXPR or PREDECREMENT_EXPR aren't considered
> to be lvalues, and for "m" inputs we just reject them, but when those
> expressions are allowed on lhs of a store, they should be IMHO allowed
> as "m" inputs too.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?
>
> 2018-03-20 Jakub Jelinek
>
> PR c++/84961
> * semantics.c (finish_asm_stmt): Replace MODIFY_EXPR,
> PREINCREMENT_EXPR
> and PREDECREMENT_EXPR in output and "m" constrained input operands
> with
> COMPOUND_EXPR. Call cxx_mark_addressable on the rightmost
> COMPOUND_EXPR operand.
>
> * c-c++-common/pr43690.c: Don't expect errors on "m" (--x) and
> "m" (++x) in C++.
> * g++.dg/torture/pr84961-1.C: New test.
> * g++.dg/torture/pr84961-2.C: New test.
>
> --- gcc/cp/semantics.c.jj 2018-03-20 11:58:17.069356145 +0100
> +++ gcc/cp/semantics.c 2018-03-20 21:56:43.745292245 +0100
> @@ -1512,6 +1512,26 @@ finish_asm_stmt (int volatile_p, tree st
> && C_TYPE_FIELDS_READONLY (TREE_TYPE (operand)
> cxx_readonly_error (operand, lv_asm);
>
> + tree *op = &operand;
> + while (TREE_CODE (*op) == COMPOUND_EXPR)
> + op = &TREE_OPERAND (*op, 1);
> + switch (TREE_CODE (*op))
> + {
> + case PREINCREMENT_EXPR:
> + case PREDECREMENT_EXPR:
> + case MODIFY_EXPR:
> + if (TREE_SIDE_EFFECTS (TREE_OPERAND (*op, 0)))
> + *op = build2 (TREE_CODE (*op), TREE_TYPE (*op),
> + cp_stabilize_reference (TREE_OPERAND (*op, 0)),
> + TREE_OPERAND (*op, 1));
> + *op = build2 (COMPOUND_EXPR, TREE_TYPE (*op), *op,
> + TREE_OPERAND (*op, 0));
> + op = &TREE_OPERAND (*op, 1);
> + break;
> + default:
> + break;
> + }
Hmm, it would be nice to share this with the similar patterns in
unary_complex_lvalue and cp_build_modify_expr.
Does COND_EXPR work without adjustment?
Jason
Re: [PATCH] [PR c++/84610,84642] recover from implicit template parms gracefully
On Mar 20, 2018, Jason Merrill wrote:
>> While debugging this, I first tried another patch, that avoids the same
>> ICEs. I thought this one was a more complete solution, and it renders
>> the other unnecessary, but I still though it might be useful to disable
>> auto->implicit_parm while parsing declarators, so as to avoid useless
>> processing.
> Better, I think, to disable it while parsing attributes; we could
> still encounter auto as a template argument in a declarator.
How about this? Regstrapping, ok to install if it passes?
Disable auto_is_implicit_function_template_parm_p while parsing attributes
for gcc/cp/ChangeLog
PR c++/84610
PR c++/84642
* parser.c (temp_override): New template class,
generalizing...
(cp_parser_parameter_declaration_clause): ... this cleanup.
Use it.
(cp_parser_gnu_attributes_opt): Use it.
(cp_parser_std_attribute): Use it.
---
gcc/cp/parser.c | 36 ++--
1 file changed, 26 insertions(+), 10 deletions(-)
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 5cc201604166..ce05615adfba 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -255,6 +255,21 @@ static bool cp_parser_omp_declare_reduction_exprs
static void cp_finalize_oacc_routine
(cp_parser *, tree, bool);
+template
+class temp_override
+{
+ T& overridden_variable;
+ T saved_value;
+public:
+ temp_override(T& var) : overridden_variable (var), saved_value (var) {}
+ temp_override(T& var, T overrider)
+: overridden_variable (var), saved_value (var)
+ {
+overridden_variable = overrider;
+ }
+ ~temp_override() { overridden_variable = saved_value; }
+};
+
/* Manifest constants. */
#define CP_LEXER_BUFFER_SIZE ((256 * 1024) / sizeof (cp_token))
#define CP_SAVED_TOKEN_STACK 5
@@ -21187,16 +21202,9 @@ cp_parser_parameter_declaration_clause (cp_parser*
parser)
bool ellipsis_p;
bool is_error;
- struct cleanup {
-cp_parser* parser;
-int auto_is_implicit_function_template_parm_p;
-~cleanup() {
- parser->auto_is_implicit_function_template_parm_p
- = auto_is_implicit_function_template_parm_p;
-}
- } cleanup = { parser, parser->auto_is_implicit_function_template_parm_p };
-
- (void) cleanup;
+ temp_override cleanup
+(parser->auto_is_implicit_function_template_parm_p);
+ (void)cleanup;
if (!processing_specialization
&& !processing_template_parmlist
@@ -24959,6 +24967,10 @@ cp_parser_gnu_attributes_opt (cp_parser* parser)
{
tree attributes = NULL_TREE;
+ temp_override cleanup
+(parser->auto_is_implicit_function_template_parm_p, false);
+ (void)cleanup;
+
while (true)
{
cp_token *token;
@@ -25150,6 +25162,10 @@ cp_parser_std_attribute (cp_parser *parser, tree
attr_ns)
tree attribute, attr_id = NULL_TREE, arguments;
cp_token *token;
+ temp_override cleanup
+(parser->auto_is_implicit_function_template_parm_p, false);
+ (void)cleanup;
+
/* First, parse name of the attribute, a.k.a attribute-token. */
token = cp_lexer_peek_token (parser->lexer);
--
Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/ FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
Re: [PR c++/84729] convert new init to array elt type
On Mar 20, 2018, Jason Merrill wrote:
> On Sat, Mar 10, 2018 at 6:57 AM, Alexandre Oliva wrote:
>> A parenthesized initializer is only accepted when new()ing an array in
>> permissive mode. We were not careful, however, to convert the
>> TREE_LIST initializer to the array element type in this extension.
>> This patch fixes it: after turning the TREE_LIST initializer to a
>> compound_expr, we convert it to the base type.
> I think I'd rather turn the permerror into a hard error than improve
> support for a deprecated extension.
Like this?
[PR c++/84729] convert new init to array elt type
A parenthesized initializer was only accepted when new()ing an array in
permissive mode. We were not careful, however, to convert the
TREE_LIST initializer to the array element type in this extension.
Instead of fixing it, converting the initializer to the base type
after turning the TREE_LIST initializer to a compound_expr, we disable
this deprecated extension.
Regstrapping. Ok to install if it passes?
for gcc/cp/ChangeLog
PR c++/84729
* init.c (build_vec_init): Convert tree list to base type.
for gcc/testsuite/ChangeLog
PR c++/84729
* g++.dg/pr84729.C: New.
---
gcc/cp/init.c |4 ++--
gcc/testsuite/g++.dg/pr84729.C |7 +++
2 files changed, 9 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/pr84729.C
diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 15cee17c780c..9091eaa90267 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -3370,8 +3370,8 @@ build_new_1 (vec **placement, tree type,
tree nelts,
else if (*init)
{
if (complain & tf_error)
-permerror (input_location,
- "parenthesized initializer in array new");
+error_at (input_location,
+ "parenthesized initializer in array new");
else
return error_mark_node;
vecinit = build_tree_list_vec (*init);
diff --git a/gcc/testsuite/g++.dg/pr84729.C b/gcc/testsuite/g++.dg/pr84729.C
new file mode 100644
index ..e5d689e0460c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr84729.C
@@ -0,0 +1,7 @@
+// { dg-do compile }
+// { dg-options "-fpermissive" }
+
+typedef int b[2];
+void a() {
+ new b(a); // { dg-error "parenthesized initializer in array new" }
+}
--
Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/ FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
Re: [PATCH] [PR c++/71965] silence multi-dim array init sorry without tf_error
On Mar 20, 2018, Jason Merrill wrote:
> On Sat, Mar 17, 2018 at 8:11 AM, Alexandre Oliva wrote:
>> - sorry
>> - ("cannot initialize multi-dimensional array with initializer");
> This shouldn't even be a sorry anymore
> Let's make it a hard error here.
Like this?
[PR c++/71965] silence multi-dim array init sorry without tf_error
We shouldn't substitute templates into short-circuited-out concepts
constraints, but we do, and to add insult to injury, we issue a
sorry() error when a concept that shouldn't even have been substituted
attempts to perform a multi-dimensional array initialization with a
new{} expression.
Although fixing the requirements short-circuiting is probably too
risky at this point, we can get closer to the intended effect by
silencing that sorry just as we silence other errors.
Regstrapping... Ok to install if it passes?
for gcc/cp/ChangeLog
PR c++/71965
* init.c (build_vec_init): Silence error, former sorry,
without tf_error.
for gcc/testsuite/ChangeLog
PR c++/71965
* g++.dg/concepts/pr71965.C: New.
---
gcc/cp/init.c | 19 ---
gcc/testsuite/g++.dg/concepts/pr71965.C | 27 +++
2 files changed, 39 insertions(+), 7 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/concepts/pr71965.C
diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 9091eaa90267..5dd4b407d73f 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -4384,12 +4384,17 @@ build_vec_init (tree base, tree maxindex, tree init,
else if (TREE_CODE (type) == ARRAY_TYPE)
{
if (init && !BRACE_ENCLOSED_INITIALIZER_P (init))
- sorry
- ("cannot initialize multi-dimensional array with initializer");
- elt_init = build_vec_init (build1 (INDIRECT_REF, type, base),
-0, init,
-explicit_value_init_p,
-0, complain);
+ {
+ if ((complain & tf_error))
+ error ("cannot initialize multi-dimensional"
+ " array with initializer");
+ elt_init = error_mark_node;
+ }
+ else
+ elt_init = build_vec_init (build1 (INDIRECT_REF, type, base),
+ 0, init,
+ explicit_value_init_p,
+ 0, complain);
}
else if (explicit_value_init_p)
{
@@ -4449,7 +4454,7 @@ build_vec_init (tree base, tree maxindex, tree init,
}
current_stmt_tree ()->stmts_are_full_exprs_p = 1;
- if (elt_init)
+ if (elt_init && !errors)
finish_expr_stmt (elt_init);
current_stmt_tree ()->stmts_are_full_exprs_p = 0;
diff --git a/gcc/testsuite/g++.dg/concepts/pr71965.C
b/gcc/testsuite/g++.dg/concepts/pr71965.C
new file mode 100644
index ..6bfaef19211f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr71965.C
@@ -0,0 +1,27 @@
+// { dg-do compile { target c++14 } }
+// { dg-options "-fconcepts" }
+
+template
+concept bool Destructible() {
+return false;
+}
+
+template
+concept bool ConstructibleObject =
+// Concept evaluation should short-circuit even the template
+// substitution, so we shouldn't even substitute into the requires
+// constraint and the unimplemented multi-dimensional new T{...}
+// initialization. ATM we do, but as long as we don't output the
+// sorry() message we used to for such constructs when asked not
+// to issue errors, this shouldn't be a problem for this and
+// similar cases.
+Destructible() && requires (Args&&...args) {
+new T{ (Args&&)args... };
+};
+
+int main() {
+using T = int[2][2];
+// GCC has not implemented initialization of multi-dimensional
+// arrays with new{} expressions.
+static_assert(!ConstructibleObject);
+}
--
Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/ FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
Re: [PATCH] relax -Wclass-memaccess for class members (PR 84850)
OK. On Mon, Mar 19, 2018 at 10:06 PM, Martin Sebor wrote: > The -Wclass-memaccess warning makes an exception for ctors and > dtors of non-trivial classes with no bases to avoid triggering > for uses of raw memory functions with this as the destination. > All other members as well as non-members trigger the warning. > > In bug 84850 the reporter shows that some code (squid in this > case) calls memset(this, 0. ...) in both the copy assignment > operator and the copy ctor as a short-hand to clear all trivial > members without having to explicitly enumerate them. A similar > idiom has been seen in Firefox (some of the bugs linked from > https://bugzilla.mozilla.org/show_bug.cgi?id=1411029). > > Since calling memset(this, 0, sizeof *this) from any non-static > member of a non-trivial class containing no non-trivial members > is safe, the attached patch relaxes the warning to allow this > idiom. > > I also noticed that the exemption for ctors and dtors is overly > permissive and causes the warning not to trigger for classes with > no bases or virtual functions but containing non-trivial members. > This is bug 84851. Jakub suggested to fix just 84850 for GCC 8 > and defer 84851 to GCC 9, so the patch follows that suggestion. > Fixing the latter is a matter of removing the block in > warn_for_restrict() with the FIXME comment above it. > > Martin
Re: [PATCH] Fix compile-time hog in MPX boundary checking (PR target/84988).
Jeff Law writes: > On 03/20/2018 01:36 PM, Martin Liška wrote: >> Hi. >> >> This is a work-around to not iterate all members of array that can be huge. >> As MPX will be removed in GCC 9.x, I hope it's acceptable. I don't want >> to come >> up with a new param for it. >> >> Survives tests&bootstrap on x86_64-linux-gnu. >> >> Martin >> >> gcc/ChangeLog: >> >> 2018-03-20 Martin Liska >> >> PR target/84988 >> * tree-chkp.c (CHKP_ARRAY_MAX_CHECK_STEPS): Define a new macro. >> (chkp_find_bound_slots_1): Limit number of iterations. > Or just CLOSE/WONTFIX :-) > > I've got no objections here -- we want to minimize the effort put into > CHKP given its going to be deprecated. The problem is that this affects normal configs, not just ones with MPX enabled. Thanks, Richard
Re: Results for 8.0.1 20180316 (experimental) [trunk revision 258610] (GCC) libstdc++ testsuite on x86_64-pc-linux-gnu
On 20 March 2018 at 21:02, François Dumont wrote: > On 20/03/2018 19:20, Jonathan Wakely wrote: >> >> On 17 March 2018 at 09:01, Jonathan Wakely wrote: >>> >>> Native configuration is x86_64-pc-linux-gnu >>> >>> === libstdc++ tests === >>> >>> >>> Running target unix/-std=gnu++11/-D_GLIBCXX_ASSERTIONS >>> >>> === libstdc++ Summary for >>> unix/-std=gnu++11/-D_GLIBCXX_ASSERTIONS === >>> >>> # of expected passes12330 >>> # of expected failures 71 >>> # of unsupported tests 579 >>> >>> Running target unix/-std=gnu++11/-D_GLIBCXX_DEBUG >>> XPASS: 21_strings/basic_string_view/element_access/char/2.cc execution >>> test >>> XPASS: 21_strings/basic_string_view/element_access/wchar_t/2.cc execution >>> test >>> FAIL: 23_containers/bitset/hash/1.cc (test for excess errors) >>> UNRESOLVED: 23_containers/bitset/hash/1.cc compilation failed to produce >>> executable >> >> These debug mode failures are regressions, reported as >> https://gcc.gnu.org/PR84998 >> >> I think they all have the same root cause (Nathan fixed a bug in G++). >> > I had already prepared this patch. Thanks for explaining why we have this > problem now. Ah I was just about to commit my own fix. > Note that I chose to use full specialization with std::hash<> for > std::bitset and std::vector. Great, that's an improvement over my patch. > It also fix a versioned namespace issue when partially specializing > std::hash for debug vector. Even better. > Tested under Linux x86_64 normal, debug and versioned namespace modes. > > Ok to commit ? Yes please! Please be sure to add "PR libstdc++/84998" to the changelog entry so it updates bugzilla. Thanks very much.
[C++ PATCH] Fix ICE with inline asm and MODIFY_EXPR/preinc/predec in output operand (PR c++/84961)
Hi!
While in C, x = 10 or ++x or --x aren't lvalues and so we reject such
expressions in inline asm output operands (and inputs that only allow
memory, not registers), in C++ they apparently are lvalues; for output
operands we ICE in the gimplifier on this, because in the generic code
MODIFY_EXPR or PREINCREMENT_EXPR or PREDECREMENT_EXPR aren't considered
to be lvalues, and for "m" inputs we just reject them, but when those
expressions are allowed on lhs of a store, they should be IMHO allowed
as "m" inputs too.
Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?
2018-03-20 Jakub Jelinek
PR c++/84961
* semantics.c (finish_asm_stmt): Replace MODIFY_EXPR, PREINCREMENT_EXPR
and PREDECREMENT_EXPR in output and "m" constrained input operands with
COMPOUND_EXPR. Call cxx_mark_addressable on the rightmost
COMPOUND_EXPR operand.
* c-c++-common/pr43690.c: Don't expect errors on "m" (--x) and
"m" (++x) in C++.
* g++.dg/torture/pr84961-1.C: New test.
* g++.dg/torture/pr84961-2.C: New test.
--- gcc/cp/semantics.c.jj 2018-03-20 11:58:17.069356145 +0100
+++ gcc/cp/semantics.c 2018-03-20 21:56:43.745292245 +0100
@@ -1512,6 +1512,26 @@ finish_asm_stmt (int volatile_p, tree st
&& C_TYPE_FIELDS_READONLY (TREE_TYPE (operand)
cxx_readonly_error (operand, lv_asm);
+ tree *op = &operand;
+ while (TREE_CODE (*op) == COMPOUND_EXPR)
+ op = &TREE_OPERAND (*op, 1);
+ switch (TREE_CODE (*op))
+ {
+ case PREINCREMENT_EXPR:
+ case PREDECREMENT_EXPR:
+ case MODIFY_EXPR:
+ if (TREE_SIDE_EFFECTS (TREE_OPERAND (*op, 0)))
+ *op = build2 (TREE_CODE (*op), TREE_TYPE (*op),
+ cp_stabilize_reference (TREE_OPERAND (*op, 0)),
+ TREE_OPERAND (*op, 1));
+ *op = build2 (COMPOUND_EXPR, TREE_TYPE (*op), *op,
+ TREE_OPERAND (*op, 0));
+ op = &TREE_OPERAND (*op, 1);
+ break;
+ default:
+ break;
+ }
+
constraint = TREE_STRING_POINTER (TREE_VALUE (TREE_PURPOSE (t)));
oconstraints[i] = constraint;
@@ -1520,7 +1540,7 @@ finish_asm_stmt (int volatile_p, tree st
{
/* If the operand is going to end up in memory,
mark it addressable. */
- if (!allows_reg && !cxx_mark_addressable (operand))
+ if (!allows_reg && !cxx_mark_addressable (*op))
operand = error_mark_node;
}
else
@@ -1562,7 +1582,30 @@ finish_asm_stmt (int volatile_p, tree st
/* Strip the nops as we allow this case. FIXME, this really
should be rejected or made deprecated. */
STRIP_NOPS (operand);
- if (!cxx_mark_addressable (operand))
+
+ tree *op = &operand;
+ while (TREE_CODE (*op) == COMPOUND_EXPR)
+ op = &TREE_OPERAND (*op, 1);
+ switch (TREE_CODE (*op))
+ {
+ case PREINCREMENT_EXPR:
+ case PREDECREMENT_EXPR:
+ case MODIFY_EXPR:
+ if (TREE_SIDE_EFFECTS (TREE_OPERAND (*op, 0)))
+ *op
+ = build2 (TREE_CODE (*op), TREE_TYPE (*op),
+ cp_stabilize_reference (TREE_OPERAND (*op,
+ 0)),
+ TREE_OPERAND (*op, 1));
+ *op = build2 (COMPOUND_EXPR, TREE_TYPE (*op), *op,
+ TREE_OPERAND (*op, 0));
+ op = &TREE_OPERAND (*op, 1);
+ break;
+ default:
+ break;
+ }
+
+ if (!cxx_mark_addressable (*op))
operand = error_mark_node;
}
else if (!allows_reg && !allows_mem)
--- gcc/testsuite/c-c++-common/pr43690.c.jj 2010-11-09 13:58:21.0
+0100
+++ gcc/testsuite/c-c++-common/pr43690.c2018-03-20 21:58:43.077317034
+0100
@@ -6,8 +6,8 @@ void
foo (char *x)
{
asm ("" : : "m" (x++)); /* { dg-error "is not directly addressable" } */
- asm ("" : : "m" (++x)); /* { dg-error "is not directly addressable" } */
+ asm ("" : : "m" (++x)); /* { dg-error "is not directly addressable" ""
{ target c } } */
asm ("" : : "m" (x--)); /* { dg-error "is not directly addressable" } */
- asm ("" : : "m" (--x)); /* { dg-error "is not directly addressable" } */
+ asm ("" : : "m" (--x)); /* { dg-error "is not directly addressable" ""
{ target c } } */
asm ("" : : "m" (x + 1));/* { dg-error "is not directly addressable" } */
}
--- gcc/testsuite/
Re: [PATCH/testsuite] Skip vect-strided-shift-1.c on MIPS with -mpaired-single option.
On 03/14/2018 12:29 AM, Paul Hua wrote: > Hi: > > The vect-strided-shift-1.c test fails on MIPS target. > > FAIL: gcc.dg/vect/vect-strided-shift-1.c -mpaired-single > scan-tree-dump-times vect "vectorized 1 loops in function" 1 (found 0 > times) > FAIL: gcc.dg/vect/vect-strided-shift-1.c -flto -ffat-lto-objects > -mpaired-single scan-tree-dump-times vect "vectorized 1 loops in > function" 1 (found 0 times) > > Because the MIPS paired single insns only support for float > operations, not suite for this test. > added dg-skip-if directives on it for skip -mpaired-single option. > > Ok for commit ? > > Paul Hua > > ChangeLog entries: > > gcc/testsuite/ChangeLog > > 2018-03-14 Chenghua Xu > > * gcc.dg/vect/vect-strided-shift-1.c: Add dg-skip-if for > MIPS with -mpaired-single directives. > OK. Though this may point to a need to have a new target-supports routine. jeff
Re: Results for 8.0.1 20180316 (experimental) [trunk revision 258610] (GCC) libstdc++ testsuite on x86_64-pc-linux-gnu
On 20/03/2018 19:20, Jonathan Wakely wrote:
On 17 March 2018 at 09:01, Jonathan Wakely wrote:
Native configuration is x86_64-pc-linux-gnu
=== libstdc++ tests ===
Running target unix/-std=gnu++11/-D_GLIBCXX_ASSERTIONS
=== libstdc++ Summary for
unix/-std=gnu++11/-D_GLIBCXX_ASSERTIONS ===
# of expected passes12330
# of expected failures 71
# of unsupported tests 579
Running target unix/-std=gnu++11/-D_GLIBCXX_DEBUG
XPASS: 21_strings/basic_string_view/element_access/char/2.cc execution test
XPASS: 21_strings/basic_string_view/element_access/wchar_t/2.cc execution test
FAIL: 23_containers/bitset/hash/1.cc (test for excess errors)
UNRESOLVED: 23_containers/bitset/hash/1.cc compilation failed to produce
executable
These debug mode failures are regressions, reported as
https://gcc.gnu.org/PR84998
I think they all have the same root cause (Nathan fixed a bug in G++).
I had already prepared this patch. Thanks for explaining why we have
this problem now.
Note that I chose to use full specialization with std::hash<> for std::bitset and
std::vector.
It also fix a versioned namespace issue when partially specializing std::hash for
debug vector.
Tested under Linux x86_64 normal, debug and versioned namespace modes.
Ok to commit ?
François
diff --git a/libstdc++-v3/include/bits/stl_bvector.h b/libstdc++-v3/include/bits/stl_bvector.h
index fbf982f..05e9ff2 100644
--- a/libstdc++-v3/include/bits/stl_bvector.h
+++ b/libstdc++-v3/include/bits/stl_bvector.h
@@ -58,6 +58,7 @@
#if __cplusplus >= 201103L
#include
+#include
#endif
namespace std _GLIBCXX_VISIBILITY(default)
@@ -593,7 +594,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
typedef typename _Base::_Bit_alloc_traits _Bit_alloc_traits;
#if __cplusplus >= 201103L
- template friend struct hash;
+ friend struct std::hash;
#endif
public:
@@ -1321,8 +1322,6 @@ _GLIBCXX_END_NAMESPACE_VERSION
#if __cplusplus >= 201103L
-#include
-
namespace std _GLIBCXX_VISIBILITY(default)
{
_GLIBCXX_BEGIN_NAMESPACE_VERSION
diff --git a/libstdc++-v3/include/bits/stl_map.h b/libstdc++-v3/include/bits/stl_map.h
index 2632775..78ac087 100644
--- a/libstdc++-v3/include/bits/stl_map.h
+++ b/libstdc++-v3/include/bits/stl_map.h
@@ -633,7 +633,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
{ return _M_t._M_reinsert_node_hint_unique(__hint, std::move(__nh)); }
template
- friend class _Rb_tree_merge_helper;
+ friend class std::_Rb_tree_merge_helper;
template
void
diff --git a/libstdc++-v3/include/bits/stl_multimap.h b/libstdc++-v3/include/bits/stl_multimap.h
index 31beb84..371cd43 100644
--- a/libstdc++-v3/include/bits/stl_multimap.h
+++ b/libstdc++-v3/include/bits/stl_multimap.h
@@ -651,7 +651,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
{ return _M_t._M_reinsert_node_hint_equal(__hint, std::move(__nh)); }
template
- friend class _Rb_tree_merge_helper;
+ friend class std::_Rb_tree_merge_helper;
template
void
diff --git a/libstdc++-v3/include/bits/stl_multiset.h b/libstdc++-v3/include/bits/stl_multiset.h
index 1bba400..7286df6 100644
--- a/libstdc++-v3/include/bits/stl_multiset.h
+++ b/libstdc++-v3/include/bits/stl_multiset.h
@@ -589,7 +589,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
{ return _M_t._M_reinsert_node_hint_equal(__hint, std::move(__nh)); }
template
- friend class _Rb_tree_merge_helper;
+ friend class std::_Rb_tree_merge_helper;
template
void
diff --git a/libstdc++-v3/include/bits/stl_set.h b/libstdc++-v3/include/bits/stl_set.h
index f79ab1c..b6b2fc4f 100644
--- a/libstdc++-v3/include/bits/stl_set.h
+++ b/libstdc++-v3/include/bits/stl_set.h
@@ -604,7 +604,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
{ return _M_t._M_reinsert_node_hint_unique(__hint, std::move(__nh)); }
template
- friend class _Rb_tree_merge_helper;
+ friend class std::_Rb_tree_merge_helper;
template
void
diff --git a/libstdc++-v3/include/bits/unordered_map.h b/libstdc++-v3/include/bits/unordered_map.h
index b757ff2..07aad9e 100644
--- a/libstdc++-v3/include/bits/unordered_map.h
+++ b/libstdc++-v3/include/bits/unordered_map.h
@@ -862,7 +862,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
#if __cplusplus > 201402L
template
- friend class _Hash_merge_helper;
+ friend class std::_Hash_merge_helper;
template
void
@@ -1742,7 +1742,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
#if __cplusplus > 201402L
template
- friend class _Hash_merge_helper;
+ friend class std::_Hash_merge_helper;
template
void
diff --git a/libstdc++-v3/include/bits/unordered_set.h b/libstdc++-v3/include/bits/unordered_set.h
index 77942a3..c9ac4ad 100644
--- a/libstdc++-v3/include/bits/unordered_set.h
+++ b/libstdc++-v3/include/bits/unordered_set.h
@@ -588,7 +588,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
#if __cplusplus > 201402L
template
- friend class _Hash_merge_helper;
+ friend class std::_Hash_merge_helper;
Re: [PATCH] Add test-case (PR ipa/84805).
On 03/14/2018 11:23 AM, Martin Liška wrote: > Hi. > > This is a new test-case isolated from libreoffice. > > Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. > I'm going to install it. > > Martin > > gcc/testsuite/ChangeLog: > > 2018-03-14 Martin Liska > > PR ipa/8480 > * g++.dg/lto/pr84805_0.C: New test. > * g++.dg/lto/pr84805_1.C: New test. > * g++.dg/lto/pr84805_2.C: New test. OK if you test on other targets and they're passing -- per Eric B's warning. jeff
Re: [C++ PATCH] Fix FIX_TRUNC_EXPR instantiation (PR c++/84942)
On Mon, Mar 19, 2018 at 3:50 PM, Jakub Jelinek wrote: > +int a(__attribute__((b((int)__builtin_inf() * 1ULL / auto; This seems like another situation like 84610 and 84642 that Alex sent a patch for, of 'auto' in an attribute wrongly being treated as an implicit template parameter. As I suggested in response to his patch, we ought to disable auto_is_implicit_function_parm_p within an attribute-specifier. Jason
Re: [Patch] Document __builtin_extend_pointer
On 03/20/2018 06:08 AM, Tom de Vries wrote:
> On 02/20/2018 06:33 PM, Steve Ellcey wrote:
>> +@deftypefn {Built-in Function} Pmode __builtin_extend_pointer (void * x)
>> +On targets where the user visible pointer size is different than the
>> size
>> +of an actual hardware address this function returns the extended user
>> +pointer. Targets where this is true included ILP32 mode on x86_64 or
>> +Aarch64. This function is mainly useful when writing inline assembly
>> +code.
>> +@var{addr}
>> +@end deftypefn
>
> Hi,
>
> I think the "@var{addr}" bit is a pasto.
Agreed. And a patch to remove the pasto is pre-approved.
jeff
Re: [PATCH] Remove superfluous return statement (PR ipa/84963).
On 03/20/2018 01:40 PM, Martin Liška wrote: > Hi. > > I'm sending removal of a stupid mistake where I installed a patch that > contained > a debugging 'return' statement. Fixed that and added condition to > inspect only > functions with SSA. > > Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. > > Thanks, > Martin > > gcc/ChangeLog: > > 2018-03-20 Martin Liska > > PR ipa/84963 > * ipa-icf.c (sem_item_optimizer::fixup_points_to_sets): Remove > not intended return statement. > > gcc/testsuite/ChangeLog: > > 2018-03-20 Martin Liska > > PR ipa/84963 > * gfortran.dg/goacc/pr84963.f90: New test. OK. jeff
[PATCH] Fix VEC_DUPLICATE simplification (PR rtl-optimization/84989)
Hi!
On the following testcase, simplify_unary_operation is called on
VEC_DUPLICATE from (vec_duplicate:V4SF something:SF) to V8SFmode,
and simplify_unary_operation_1 tries an optimization usable for most
unary operations, in particular it attempts to do
(vec_duplicate:V8SF (unary:SF something:SF))
which is reasonable for all unary ops other than when unary is
vec_duplicate, because that needs a vector outer mode and scalar or vector
inner mode, not scalar outer and inner mode.
Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?
2018-03-20 Jakub Jelinek
PR rtl-optimization/84989
* simplify-rtx.c (simplify_unary_operation_1): Don't try to simplify
VEC_DUPLICATE with scalar result mode.
* gcc.target/i386/pr84989.c: New test.
--- gcc/simplify-rtx.c.jj 2018-01-20 10:52:47.0 +0100
+++ gcc/simplify-rtx.c 2018-03-20 17:13:11.906809795 +0100
@@ -1692,7 +1692,9 @@ simplify_unary_operation_1 (enum rtx_cod
break;
}
- if (VECTOR_MODE_P (mode) && vec_duplicate_p (op, &elt))
+ if (VECTOR_MODE_P (mode)
+ && vec_duplicate_p (op, &elt)
+ && code != VEC_DUPLICATE)
{
/* Try applying the operator to ELT and see if that simplifies.
We can duplicate the result if so.
--- gcc/testsuite/gcc.target/i386/pr84989.c.jj 2018-03-20 17:20:46.840921141
+0100
+++ gcc/testsuite/gcc.target/i386/pr84989.c 2018-03-20 17:19:57.257903317
+0100
@@ -0,0 +1,12 @@
+/* PR rtl-optimization/84989 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -mavx512f" } */
+
+#include
+
+__m512
+foo (float a, float *b)
+{
+ return _mm512_sub_ps (_mm512_broadcast_f32x4 (_mm_load_ps (b)),
+ _mm512_broadcast_f32x4 (_mm_set1_ps (a)));
+}
Jakub
Re: [PATCH] Don't RTL DCE REG_CFA_RESTORE noop moves (PR debug/84875)
On 03/15/2018 01:20 PM, Jakub Jelinek wrote: > Hi! > > The following testcase ICEs on s390x-linux, because we have 2 registers > saved in prologue, but only one of them modified in one path and then > both restored there; cprop_hardreg propagates the saving register into the > REG_CFA_RESTORE insn, making it a noop move (we don't really need to restore > it), and then RTL DCE removes the noop move and we ICE during dwarf2 pass, > because of a CFI mismatch. > > The following patch makes sure that such insns are not actually removed, but > turned into a USE still holding the note, which is all we need for CFI. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2018-03-15 Jakub Jelinek > > PR debug/84875 > * dce.c (delete_unmarked_insns): Don't remove frame related noop moves > holding REG_CFA_RESTORE notes, instead turn them into a USE. > > * gcc.dg/pr84875.c: New test. OK. jeff
[PATCH] Fix up handling of bool BIT_NOT_EXPRs in store-merging (PR tree-optimization/84982)
Hi!
Boolean !x is often expanded as ^ 1, but store merging it actually merges
as ^ 255 (for 8-bit bool), which is incorrect.
The following patch fixes it to do that ^ 1 instead.
Bootstrapped/regtested on x86_64-linux and i686-linux and checked on the
testcase with -> powerpc64-linux cross, ok for trunk?
2018-03-20 Jakub Jelinek
PR tree-optimization/84982
* gimple-ssa-store-merging.c (invert_op): Handle boolean inversion
by flipping the least significant bit rather than all bits from
bitpos to bitpos + bitsize - 1.
* c-c++-common/pr84982.c: New test.
--- gcc/gimple-ssa-store-merging.c.jj 2018-03-20 13:53:31.701938584 +0100
+++ gcc/gimple-ssa-store-merging.c 2018-03-20 14:39:50.925657339 +0100
@@ -3248,16 +3248,22 @@ invert_op (split_store *split_store, int
unsigned int i;
store_immediate_info *info;
unsigned int cnt = 0;
+ bool any_bools = false;
FOR_EACH_VEC_ELT (split_store->orig_stores, i, info)
{
bool bit_not_p = idx < 2 ? info->ops[idx].bit_not_p : info->bit_not_p;
if (bit_not_p)
- ++cnt;
+ {
+ ++cnt;
+ tree lhs = gimple_assign_lhs (info->stmt);
+ if (TREE_CODE (TREE_TYPE (lhs)) == BOOLEAN_TYPE && info->bitsize > 1)
+ any_bools = true;
+ }
}
mask = NULL_TREE;
if (cnt == 0)
return NOP_EXPR;
- if (cnt == split_store->orig_stores.length ())
+ if (cnt == split_store->orig_stores.length () && !any_bools)
return BIT_NOT_EXPR;
unsigned HOST_WIDE_INT try_bitpos = split_store->bytepos * BITS_PER_UNIT;
@@ -3275,13 +3281,34 @@ invert_op (split_store *split_store, int
set in the mask. */
unsigned HOST_WIDE_INT bitsize = info->bitsize;
unsigned int pos_in_buffer = 0;
+ bool is_bool = false;
+ if (any_bools && bitsize > 1)
+ {
+ tree lhs = gimple_assign_lhs (info->stmt);
+ if (TREE_CODE (TREE_TYPE (lhs)) == BOOLEAN_TYPE)
+ is_bool = true;
+ }
if (info->bitpos < try_bitpos)
{
gcc_assert (info->bitpos + bitsize > try_bitpos);
bitsize -= (try_bitpos - info->bitpos);
+ if (is_bool && !BYTES_BIG_ENDIAN)
+ continue;
}
else
pos_in_buffer = info->bitpos - try_bitpos;
+ if (is_bool && bitsize)
+ {
+ /* If this is a bool inversion, invert just the LSB
+rather than all bits of it. */
+ if (BYTES_BIG_ENDIAN)
+ {
+ pos_in_buffer += bitsize - 1;
+ if (pos_in_buffer >= split_store->size)
+ continue;
+ }
+ bitsize = 1;
+ }
if (pos_in_buffer + bitsize > split_store->size)
bitsize = split_store->size - pos_in_buffer;
unsigned char *p = buf + (pos_in_buffer / BITS_PER_UNIT);
--- gcc/testsuite/c-c++-common/pr84982.c.jj 2018-03-20 14:49:00.259744750
+0100
+++ gcc/testsuite/c-c++-common/pr84982.c2018-03-20 12:27:34.111363552
+0100
@@ -0,0 +1,38 @@
+/* PR tree-optimization/84982 */
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+#ifndef __cplusplus
+#define bool _Bool
+#define true 1
+#define false 0
+#endif
+
+struct S { bool a, b, c, d; };
+
+__attribute__((noipa)) void
+bar (bool *x)
+{
+ if (x[0] || !x[1] || !x[2] || x[3])
+__builtin_abort ();
+}
+
+__attribute__((noipa)) void
+foo (struct S *x)
+{
+ bool a[4];
+ a[0] = !x->a;
+ a[1] = !x->b;
+ a[2] = x->c;
+ a[3] = !x->d;
+ bar (a);
+}
+
+int
+main ()
+{
+ struct S s;
+ s.a = true; s.b = false; s.c = true; s.d = true;
+ foo (&s);
+ return 0;
+}
Jakub
Re: [PR c++/71251] out-of-range parms in tmpl arg substitution
On Tue, Mar 20, 2018 at 4:15 PM, Alexandre Oliva wrote: > On Mar 20, 2018, Jason Merrill wrote: > >> On Sat, Mar 17, 2018 at 8:13 AM, Alexandre Oliva wrote: >>> As we go through each of the template parameters, substituting it with >>> corresponding template arguments, an incorrect argument list might >>> cause us to index argument vectors past their length (or fail in the >>> preceding tree checks). Avoid such dereferences and instead issue an >>> error (if requested) if we find the argument index to be past the >>> parameter vector length. > >> Any time we hit this abort, it indicates a bug in earlier processing, >> so that we're looking up a template parameter in an argument list for >> a different template. > > That doesn't seem to be the case here. The argument list given for U is > , as in the testcase, the problem is that U is misdeclared as taking > (two template levels for a single template). > > Should we aim at rejecting the declaration of U? Yes. Jason
Re: C++ PATCH for c++/71638, ICE with NSDMI and reference
OK.
On Tue, Mar 20, 2018 at 3:50 PM, Marek Polacek wrote:
> This extends my previous fix for c++/84927 in that it also updates
> the ctor's flags when we replace an element.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2018-03-20 Marek Polacek
>
> PR c++/71638, ICE with NSDMI and reference.
> * constexpr.c (cxx_eval_bare_aggregate): Update constructor's flags
> even when we replace an element.
>
> * g++.dg/cpp0x/nsdmi14.C: New test.
> * g++.dg/cpp1y/nsdmi-aggr10.C: New test.
>
> diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
> index 894bcd0bb3e..e2d66498a66 100644
> --- gcc/cp/constexpr.c
> +++ gcc/cp/constexpr.c
> @@ -2873,16 +2873,17 @@ cxx_eval_bare_aggregate (const constexpr_ctx *ctx,
> tree t,
> gcc_assert (is_empty_class (TREE_TYPE (TREE_TYPE (index;
> changed = true;
> }
> - else if (new_ctx.ctor != ctx->ctor)
> - {
> - /* We appended this element above; update the value. */
> - gcc_assert ((*p)->last().index == index);
> - (*p)->last().value = elt;
> - }
>else
> {
> - CONSTRUCTOR_APPEND_ELT (*p, index, elt);
> - /* Adding an element might change the ctor's flags. */
> + if (new_ctx.ctor != ctx->ctor)
> + {
> + /* We appended this element above; update the value. */
> + gcc_assert ((*p)->last().index == index);
> + (*p)->last().value = elt;
> + }
> + else
> + CONSTRUCTOR_APPEND_ELT (*p, index, elt);
> + /* Adding or replacing an element might change the ctor's flags. */
> TREE_CONSTANT (ctx->ctor) = constant_p;
> TREE_SIDE_EFFECTS (ctx->ctor) = side_effects_p;
> }
> diff --git gcc/testsuite/g++.dg/cpp0x/nsdmi14.C
> gcc/testsuite/g++.dg/cpp0x/nsdmi14.C
> index e69de29bb2d..aac6fa1c81b 100644
> --- gcc/testsuite/g++.dg/cpp0x/nsdmi14.C
> +++ gcc/testsuite/g++.dg/cpp0x/nsdmi14.C
> @@ -0,0 +1,19 @@
> +// PR c++/71638
> +// { dg-do compile { target c++11 } }
> +// { dg-options "-Wall" }
> +
> +struct A {
> + struct {
> +int i;
> +int &j = i;
> + } b;
> + int a = b.j;
> +};
> +
> +void bar (A);
> +
> +void
> +foo ()
> +{
> + bar (A{});
> +}
> diff --git gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr10.C
> gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr10.C
> index e69de29bb2d..1dc396d9ca5 100644
> --- gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr10.C
> +++ gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr10.C
> @@ -0,0 +1,7 @@
> +// PR c++/71638
> +// { dg-do compile { target c++14 } }
> +
> +struct {
> + int &&a;
> + int b{a};
> +} c[] { { 2 } };
>
> Marek
Re: [RFC Patch], PowerPC memory support pre-gcc9, patch #1
On Tue, Mar 20, 2018 at 08:01:57AM -0500, Segher Boessenkool wrote: > Hi! Some comments... > > On Wed, Mar 14, 2018 at 06:54:08PM -0400, Michael Meissner wrote: > > The first patch in the series moves most of the reg_addr structure from > > rs6000.c to rs6000-protos.h, so that in the next patch, we can start > > splitting > > some of the address code to other files. > > Is that the correct header? It currently contains only function > prototypes, and the name indicates that is what it should be. > > > 1) I was playing with making r12 be fixed with a new option (not > > in this > > set of patches), and I noticed it wasn't reflected in the -mdebug=reg > > debug dump, due to the debug dump being done before the conditional > > registers are setup. I made the debug dump set conditional registers. > > Various ABIs use r12 for various things. It's also used for split stack. > Besides that it is available for programs to do with as they please. > > > I likely will remove the undocumented toc-fusion all together, and > > eventually > > rework the p8/p9 fusion support. > > Did it ever give any performance improvement? > > > 2018-03-14 Michael Meissner > > * config/rs6000/rs6000-protos.h (regno_or_subregno): Add > > declaration. > > There is a generic reg_or_subregno, how does this differ? If we need > it please change the name so the difference is clear. > > It is very hard to review these patches. Please do patches that only > move or rename things, not changing functionality, as separate patches > (usually before everything else). Ok, but if you want me to shove everything back into rs6000.c that simplifies things. Some of the artiface is to support the reg_addr stuff in multiple locations. -- Michael Meissner, IBM IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA email: meiss...@linux.vnet.ibm.com, phone: +1 (978) 899-4797
Re: [PR c++/84789] do not resolve typename into template-independent
On Fri, Mar 16, 2018 at 5:38 PM, Alexandre Oliva wrote: > resolve_typename_type may peek into template types that might still be > specialized. In some cases, e.g. g++.dg/template/friend48.C or > g++.dg/template/decl2.C, that is exactly the right thing to do. In > others, like the newly-added testcase g++.dg/template/pr84789.C, it > isn't, and if the qualifying scope happens to resolve to a non-template > type, we resolve to that and then fail the assert that checks we still > have a template-dependent scope. We're looking inside them because we're trying to parse a declarator; the tentative parse will fail in this case, because we aren't in a declarator, but that doesn't mean it's wrong to peek. Though I'm not sure why cp_parser_parse_and_diagnose_invalid_type_name is passing true for declarator_p to cp_parser_id_expression. I'm going to try changing that to false and see if anything breaks. > It appears to me that, in cases in which the assert would fail, we are > missing the typename keyword, and we ought to report an error; if we > just return the incoming type unchanged, that's exactly what we get. > So, I'm turning such failed asserts into early returns, so that the > parser can recover and report an error. I disagree; it seems to me that the assert should allow the case where the scope was originally dependent, but got resolved earlier in the function. Jason
Re: [RFC Patch], PowerPC memory support pre-gcc9, patch #2
On Tue, Mar 20, 2018 at 08:30:57AM -0500, Segher Boessenkool wrote:
> On Thu, Mar 15, 2018 at 01:04:30PM -0400, Michael Meissner wrote:
> > This is patch #2 of my series for improving the PowerPC internal memory
> > support. It assumes patch #1 has been applied.
> >
> > This patch moves the rs6000_move_128bit function from rs6000.c to a new
> > file,
> > rs6000-output.c.
> >
> > The third patch will create a rs6000_move_64bit function and change both
> > 32-bit
> > and 64-bit movdi to call it, instead of having all of the instructions be
> > literals. I will also likely add improvements to setting the reg_addr
> > address
> > masks for offsetable addresses.
> >
> > The fourth patch will like move movdd and movdf to call rs6000_move_64bit as
> > well.
> >
> > I tested this on a little endian power8 system and there were no
> > regressions.
> >
> > 2018-03-14 Michael Meissner
> >
> > * config.gcc (powerpc*-*-*): Add rs6000-output.o to extra_objs.
> > * config/rs6000/t-rs6000 (rs6000-output.o): Add build rule.
> > * config/rs6000/rs6000.c (rs6000_output_move_128bit): Move to
> > rs6000-output.c.
>
> I am not happy at all with this new file, and it won't even work as far
> as I see (for multi-alternative define_insn's; splitting the strings to
> a different file than the constraints and attributes is asking for
> trouble, better keep it all together).
>
> Files should bundle together code that conceptually belongs together,
> not some arbitrary split ("these routine return strings that are
> eventually output from the compiler as instructions").
I was eventually planning to move the other functions that split insns and
output the strings there. But I can keep it in rs6000.c if desired. I was
just trying to keep the mechanical changes down, rather than move everything
all at once.
--
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.vnet.ibm.com, phone: +1 (978) 899-4797
Re: [PATCH] Fix PR84512
Hi Tom,
> On 03/19/2018 10:11 AM, Richard Biener wrote:
>> On Fri, 16 Mar 2018, Tom de Vries wrote:
>>
>>> On 03/16/2018 12:55 PM, Richard Biener wrote:
On Fri, 16 Mar 2018, Tom de Vries wrote:
> On 02/27/2018 01:42 PM, Richard Biener wrote:
>> Index: gcc/testsuite/gcc.dg/tree-ssa/pr84512.c
>> ===
>> --- gcc/testsuite/gcc.dg/tree-ssa/pr84512.c (nonexistent)
>> +++ gcc/testsuite/gcc.dg/tree-ssa/pr84512.c (working copy)
>> @@ -0,0 +1,15 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-O3 -fdump-tree-optimized" } */
>> +
>> +int foo()
>> +{
>> + int a[10];
>> + for(int i = 0; i < 10; ++i)
>> +a[i] = i*i;
>> + int res = 0;
>> + for(int i = 0; i < 10; ++i)
>> +res += a[i];
>> + return res;
>> +}
>> +
>> +/* { dg-final { scan-tree-dump "return 285;" "optimized" } } */
>
> This fails for nvptx, because it doesn't have the required vector
> operations.
> To fix the fail, I've added requiring effective target vect_int_mult.
On targets that do not vectorize you should see the scalar loops unrolled
instead. Or do you have only one loop vectorized?
>>>
>>> Sort of. Loop vectorization has no effect, and the scalar loops are
>>> completely
>>> unrolled. But then slp vectorization vectorizes the stores.
>>>
>>> So at optimized we have:
>>> ...
>>>MEM[(int *)&a] = { 0, 1 };
>>>MEM[(int *)&a + 8B] = { 4, 9 };
>>>MEM[(int *)&a + 16B] = { 16, 25 };
>>>MEM[(int *)&a + 24B] = { 36, 49 };
>>>MEM[(int *)&a + 32B] = { 64, 81 };
>>>_6 = a[0];
>>>_28 = a[1];
>>>res_29 = _6 + _28;
>>>_35 = a[2];
>>>res_36 = res_29 + _35;
>>>_42 = a[3];
>>>res_43 = res_36 + _42;
>>>_49 = a[4];
>>>res_50 = res_43 + _49;
>>>_56 = a[5];
>>>res_57 = res_50 + _56;
>>>_63 = a[6];
>>>res_64 = res_57 + _63;
>>>_70 = a[7];
>>>res_71 = res_64 + _70;
>>>_77 = a[8];
>>>res_78 = res_71 + _77;
>>>_2 = a[9];
>>>res_11 = _2 + res_78;
>>>a ={v} {CLOBBER};
>>>return res_11;
>>> ...
>>>
>>> The stores and loads are eliminated by dse1 in the rtl phase, and in the end
>>> we have:
>>> ...
>>> .visible .func (.param.u32 %value_out) foo
>>> {
>>> .reg.u32 %value;
>>> .local .align 16 .b8 %frame_ar[48];
>>> .reg.u64 %frame;
>>> cvta.local.u64 %frame, %frame_ar;
>>> mov.u32 %value, 285;
>>> st.param.u32[%value_out], %value;
>>> ret;
>>> }
>>> ...
>>>
That's precisely
what the PR was about... which means it isn't fixed for nvptx :/
>>>
>>> Indeed the assembly is not optimal, and would be optimal if we'd have
>>> optimal
>>> code at optimized.
>>>
>>> FWIW, using this patch we generate optimal code at optimized:
>>> ...
>>> diff --git a/gcc/passes.def b/gcc/passes.def
>>> index 3ebcfc30349..6b64f600c4a 100644
>>> --- a/gcc/passes.def
>>> +++ b/gcc/passes.def
>>> @@ -325,6 +325,7 @@ along with GCC; see the file COPYING3. If not see
>>> NEXT_PASS (pass_tracer);
>>> NEXT_PASS (pass_thread_jumps);
>>> NEXT_PASS (pass_dominator, false /* may_peel_loop_headers_p */);
>>> + NEXT_PASS (pass_fre);
>>> NEXT_PASS (pass_strlen);
>>> NEXT_PASS (pass_thread_jumps);
>>> NEXT_PASS (pass_vrp, false /* warn_array_bounds_p */);
>>> ...
>>>
>>> and we get:
>>> ...
>>> .visible .func (.param.u32 %value_out) foo
>>> {
>>> .reg.u32 %value;
>>> mov.u32 %value, 285;
>>> st.param.u32[%value_out], %value;
>>> ret;
>>> }
>>> ...
>>>
>>> I could file a missing optimization PR for nvptx, but I'm not sure where
>>> this
>>> should be fixed.
>>
>> Ah, yeah... the usual issue then.
>>
>> Can you please XFAIL the test on nvptx instead of requiring vect_int_mult?
>>
>
> Done.
>
> Committed at attached.
this caused the test to FAIL on 64-bit (only) sparc-sun-solaris2.11:
FAIL: gcc.dg/tree-ssa/pr84512.c scan-tree-dump optimized "return 285;"
where it was UNSUPPORTED before.
The dump has
;; Function foo (foo, funcdef_no=0, decl_uid=1557, cgraph_uid=0, symbol_order=0)
foo ()
{
int res;
int a[10];
int _2;
int _6;
int _28;
int _35;
int _42;
int _49;
int _56;
int _63;
int _70;
int _77;
[local count: 97603132]:
MEM[(int *)&a] = { 0, 1 };
MEM[(int *)&a + 8B] = { 4, 9 };
MEM[(int *)&a + 16B] = { 16, 25 };
MEM[(int *)&a + 24B] = { 36, 49 };
MEM[(int *)&a + 32B] = { 64, 81 };
_6 = a[0];
_28 = a[1];
res_29 = _6 + _28;
_35 = a[2];
res_36 = res_29 + _35;
_42 = a[3];
res_43 = res_36 + _42;
_49 = a[4];
res_50 = res_43 + _49;
_56 = a[5];
res_57 = res_50 + _56;
_63 = a[6];
res_64 = res_57 + _63;
_70 = a[7];
res_71 = res_64 + _70;
_77 = a[8];
res_78 = res_71 + _77;
_2 = a[9];
res_11 = _2 + res_78;
a ={v} {CLOBBER};
return res_11;
}
Rainer
--
-
Re: [PR c++/71251] out-of-range parms in tmpl arg substitution
On Mar 20, 2018, Jason Merrill wrote:
> On Sat, Mar 17, 2018 at 8:13 AM, Alexandre Oliva wrote:
>> As we go through each of the template parameters, substituting it with
>> corresponding template arguments, an incorrect argument list might
>> cause us to index argument vectors past their length (or fail in the
>> preceding tree checks). Avoid such dereferences and instead issue an
>> error (if requested) if we find the argument index to be past the
>> parameter vector length.
> Any time we hit this abort, it indicates a bug in earlier processing,
> so that we're looking up a template parameter in an argument list for
> a different template.
That doesn't seem to be the case here. The argument list given for U is
, as in the testcase, the problem is that U is misdeclared as taking
(two template levels for a single template).
Should we aim at rejecting the declaration of U?
template template using U=void;
template struct S1;
template struct S1>{ template struct S2:S2{}; };
--
Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/ FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
Re: [PATCH PR84969]Don't reorder builtin memsets if they set different rhs values
On 03/20/2018 07:18 PM, Bin Cheng wrote: Hi, As noted in PR84969, fuse_memset_builtins breaks dependence between different memsets. Specifically, it reorders different builtin memset partitions though it doesn't merge them in the end. This simple patch fixes this wrong code issue by checking if any two builtin memsets set the same rhs value or not. Note we don't need to bother if two memsets intersect with each other or not. Of course, this would miss opportunity merging S1/S3 in below case: memset(p+12, 0, 12); //<-S1 memset(p+17, 1, 10); memset(p, 0, 12); //<-S3 In my opinion, this should be resolved in a more general way maximizing parallelism as well as merging opportunities when sorting partitions into topological order from dependence graph, which isn't GCC8 task. Bootstrap and test on x86_64 and AArch64 ongoing. Okay if no failures? Thanks, bin 2018-03-20 Bin Cheng PR tree-optimization/84969 * tree-loop-distribution.c (fuse_memset_builtins): Don't reorder builtin memset partitions if they set differnt rhs values. gcc/testsuite 2018-03-20 Bin Cheng PR tree-optimization/84969 * gcc.dg/tree-ssa/pr84969.c: New test. Thanks Bin. I can confirm it fixes regression tests of postgres w/ -O3. Martin
C++ PATCH for c++/71638, ICE with NSDMI and reference
This extends my previous fix for c++/84927 in that it also updates
the ctor's flags when we replace an element.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2018-03-20 Marek Polacek
PR c++/71638, ICE with NSDMI and reference.
* constexpr.c (cxx_eval_bare_aggregate): Update constructor's flags
even when we replace an element.
* g++.dg/cpp0x/nsdmi14.C: New test.
* g++.dg/cpp1y/nsdmi-aggr10.C: New test.
diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
index 894bcd0bb3e..e2d66498a66 100644
--- gcc/cp/constexpr.c
+++ gcc/cp/constexpr.c
@@ -2873,16 +2873,17 @@ cxx_eval_bare_aggregate (const constexpr_ctx *ctx, tree
t,
gcc_assert (is_empty_class (TREE_TYPE (TREE_TYPE (index;
changed = true;
}
- else if (new_ctx.ctor != ctx->ctor)
- {
- /* We appended this element above; update the value. */
- gcc_assert ((*p)->last().index == index);
- (*p)->last().value = elt;
- }
else
{
- CONSTRUCTOR_APPEND_ELT (*p, index, elt);
- /* Adding an element might change the ctor's flags. */
+ if (new_ctx.ctor != ctx->ctor)
+ {
+ /* We appended this element above; update the value. */
+ gcc_assert ((*p)->last().index == index);
+ (*p)->last().value = elt;
+ }
+ else
+ CONSTRUCTOR_APPEND_ELT (*p, index, elt);
+ /* Adding or replacing an element might change the ctor's flags. */
TREE_CONSTANT (ctx->ctor) = constant_p;
TREE_SIDE_EFFECTS (ctx->ctor) = side_effects_p;
}
diff --git gcc/testsuite/g++.dg/cpp0x/nsdmi14.C
gcc/testsuite/g++.dg/cpp0x/nsdmi14.C
index e69de29bb2d..aac6fa1c81b 100644
--- gcc/testsuite/g++.dg/cpp0x/nsdmi14.C
+++ gcc/testsuite/g++.dg/cpp0x/nsdmi14.C
@@ -0,0 +1,19 @@
+// PR c++/71638
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wall" }
+
+struct A {
+ struct {
+int i;
+int &j = i;
+ } b;
+ int a = b.j;
+};
+
+void bar (A);
+
+void
+foo ()
+{
+ bar (A{});
+}
diff --git gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr10.C
gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr10.C
index e69de29bb2d..1dc396d9ca5 100644
--- gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr10.C
+++ gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr10.C
@@ -0,0 +1,7 @@
+// PR c++/71638
+// { dg-do compile { target c++14 } }
+
+struct {
+ int &&a;
+ int b{a};
+} c[] { { 2 } };
Marek
Re: [PATCH ] PR 844422 Fix FCTID, FCTIW with -mcpu=power7
On 3/16/18 5:51 PM, Segher Boessenkool wrote: > But fctiw is an ISA 1.xx instruction already (and fctid is as well, > but explicitly undefined for 32-bit implementations until some 2.xx, > I forgot which, 2.02 I think?) > > Requiring power8 for it is weird and surprising. power8-vector doubly so. Completely agree. > (The issue is that pre-power8 we do not allow SImode in FPR registers. > Which makes the current fctiw implementation fail). A similar issue was true of SDmode, in that we couldn't save/restore SDmode values out of FP regs without corrupting them. I can say I didn't like the solution we were forced to use. :-( > I think we have two good options: > > 1) Remove these builtins; > or > 2) Make them work. Agreed. If we make them work, then the pattern that was added in revision 253238 should be fixed. The type of the source operand is defined as DFmode, but the pattern is named as a SFmode to SImode conversion. (define_insn "lrintsfsi2" [(set (match_operand:SI 0 "gpc_reg_operand" "=d") (unspec:SI [(match_operand:DF 1 "gpc_reg_operand" "d")] UNSPEC_FCTIW))] "TARGET_SF_FPR && TARGET_FPRND" "fctiw %0,%1" [(set_attr "type" "fp")]) Peter
Re: [PR c++/84610,84642] recover from implicit template parms gracefully
On Sat, Mar 10, 2018 at 6:57 AM, Alexandre Oliva wrote: > If we get a parse error during an attempted fully implicit function > template parse, and need to skip to the end of the statement or block, > we may discard the function parms scope rather than the enclosing > injected implicit template parms scope. If we rollback a tentative > parse and try something else, we'll no longer be in a function parms > scope, but rather in a template parms scope, but we may still attempt > to synthesize implicit template parms and then fail the assert that > checks we're in a function parms scope. > > This patch introduces an alternative to > finish_fully_implicit_template_p, to be used during error recovery, > that floats the implicit template parm scope to the top so that it > gets discarded as we finish and discard the failed implicit template > data, while other scopes are retained as expected. It also clears the > implicit template parser data as we finish the template, so that it > doesn't linger on referencing discarded or used scopes and parms. > > Regstrapped on i686- and x86_64-linux-gnu. Ok to install? OK. > While debugging this, I first tried another patch, that avoids the same > ICEs. I thought this one was a more complete solution, and it renders > the other unnecessary, but I still though it might be useful to disable > auto->implicit_parm while parsing declarators, so as to avoid useless > processing. Better, I think, to disable it while parsing attributes; we could still encounter auto as a template argument in a declarator. Jason
Re: Seeking Release Manager approval for: [PATCH] jit: fix link on OS X and Solaris (PR jit/64089 and PR jit/84288)
On Tue, 2018-03-20 at 09:38 +0100, Rainer Orth wrote: > Hi David, > > > Release managers: > > > > I'd like to apply FX's patch here: > > https://gcc.gnu.org/ml/gcc-patches/2018-02/msg00881/patch > > to trunk, to fix the build of jit on OS X, and to make it easier to > > fix > > it on Solaris. > > > > This involves touching gcc/configure.ac (and configure). > > > > I've successfully bootstrapped and regression-tested it on x86_64- > > pc- > > linux-gnu. FX reports above that it fixes the build on macOS, and > > Rainer has an (untested) patch on top of it that ought to fix the > > build > > on Solaris: > > https://gcc.gnu.org/ml/gcc-patches/2018-02/msg00835.html > > I've now tested the patch (together with the one from PR jit/84288 > for > several remaining issues). I've needed another snippet for > Solaris/SPARC which links libkstat into xgcc and needs it in > libgccjit.so, too. Bootstrapped without regressions on > i386-pc-solaris2.11 and sparc-sun-solaris2.11. FWIW I've successfully tested this on x86_64-pc-linux-gnu (regenerating the gcc/configure), and, as jit maintainer, it looks good to me [1], though it may still need RM approval given stage 4. Dave [1] ...though I have a slight preference for listing $(EXTRA_GCC_LIBS) on the same line as $(EXTRA_GCC_OBJS) in jit/Make- lang.in, so that these two items needed to embed the driver code into the libgccjit shared library are visually grouped together. > Ok for mainline? > > Rainer >
[PATCH] Remove superfluous return statement (PR ipa/84963).
Hi.
I'm sending removal of a stupid mistake where I installed a patch that contained
a debugging 'return' statement. Fixed that and added condition to inspect only
functions with SSA.
Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
Thanks,
Martin
gcc/ChangeLog:
2018-03-20 Martin Liska
PR ipa/84963
* ipa-icf.c (sem_item_optimizer::fixup_points_to_sets): Remove
not intended return statement.
gcc/testsuite/ChangeLog:
2018-03-20 Martin Liska
PR ipa/84963
* gfortran.dg/goacc/pr84963.f90: New test.
---
gcc/ipa-icf.c | 5 +++--
gcc/testsuite/gfortran.dg/goacc/pr84963.f90 | 7 +++
2 files changed, 10 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/goacc/pr84963.f90
diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
index 1376a54e95e..f974d9f769f 100644
--- a/gcc/ipa-icf.c
+++ b/gcc/ipa-icf.c
@@ -3612,15 +3612,16 @@ void
sem_item_optimizer::fixup_points_to_sets (void)
{
/* TODO: remove in GCC 9 and trigger PTA re-creation after IPA passes. */
-
cgraph_node *cnode;
- return;
FOR_EACH_DEFINED_FUNCTION (cnode)
{
tree name;
unsigned i;
function *fn = DECL_STRUCT_FUNCTION (cnode->decl);
+ if (!gimple_in_ssa_p (fn))
+ continue;
+
FOR_EACH_SSA_NAME (i, name, fn)
if (POINTER_TYPE_P (TREE_TYPE (name))
&& SSA_NAME_PTR_INFO (name))
diff --git a/gcc/testsuite/gfortran.dg/goacc/pr84963.f90 b/gcc/testsuite/gfortran.dg/goacc/pr84963.f90
new file mode 100644
index 000..4548082bee3
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/goacc/pr84963.f90
@@ -0,0 +1,7 @@
+! PR ipa/84963
+! { dg-options "-O2" }
+
+program p
+ print *, sin([1.0, 2.0])
+ print *, cos([1.0, 2.0])
+end
Re: [PATCH] Fix compile-time hog in MPX boundary checking (PR target/84988).
On 03/20/2018 01:36 PM, Martin Liška wrote: > Hi. > > This is a work-around to not iterate all members of array that can be huge. > As MPX will be removed in GCC 9.x, I hope it's acceptable. I don't want > to come > up with a new param for it. > > Survives tests&bootstrap on x86_64-linux-gnu. > > Martin > > gcc/ChangeLog: > > 2018-03-20 Martin Liska > > PR target/84988 > * tree-chkp.c (CHKP_ARRAY_MAX_CHECK_STEPS): Define a new macro. > (chkp_find_bound_slots_1): Limit number of iterations. Or just CLOSE/WONTFIX :-) I've got no objections here -- we want to minimize the effort put into CHKP given its going to be deprecated. jeff
[PATCH] Fix compile-time hog in MPX boundary checking (PR target/84988).
Hi. This is a work-around to not iterate all members of array that can be huge. As MPX will be removed in GCC 9.x, I hope it's acceptable. I don't want to come up with a new param for it. Survives tests&bootstrap on x86_64-linux-gnu. Martin gcc/ChangeLog: 2018-03-20 Martin Liska PR target/84988 * tree-chkp.c (CHKP_ARRAY_MAX_CHECK_STEPS): Define a new macro. (chkp_find_bound_slots_1): Limit number of iterations. --- gcc/tree-chkp.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c index 40497ce94e7..d10e6c40423 100644 --- a/gcc/tree-chkp.c +++ b/gcc/tree-chkp.c @@ -1688,6 +1688,10 @@ chkp_find_bounds_for_elem (tree elem, tree *all_bounds, } } +/* Maximum number of elements to check in an array. */ + +#define CHKP_ARRAY_MAX_CHECK_STEPS4096 + /* Fill HAVE_BOUND output bitmap with information about bounds requred for object of type TYPE. @@ -1733,7 +1737,9 @@ chkp_find_bound_slots_1 (const_tree type, bitmap have_bound, || integer_minus_onep (maxval)) return; - for (cur = 0; cur <= TREE_INT_CST_LOW (maxval); cur++) + for (cur = 0; + cur <= MIN (CHKP_ARRAY_MAX_CHECK_STEPS, TREE_INT_CST_LOW (maxval)); + cur++) chkp_find_bound_slots_1 (etype, have_bound, offs + cur * esize); } }
Re: [PATCH][AARCH64] PR target/84521 Fix frame pointer corruption with -fomit-frame-pointer with __builtin_setjmp
On 19/03/18 12:11, James Greenhalgh wrote:
On Wed, Mar 14, 2018 at 05:40:49PM +, Sudi Das wrote:
Hi
This patch is another partial fix for PR 84521. This is adding a
definition to one of the target hooks used in the SJLJ implemetation so
that AArch64 defines the hard_frame_pointer_rtx as the
TARGET_BUILTIN_SETJMP_FRAME_VALUE. As pointed out by Wilco there is
still a lot more work to be done for these builtins in the future.
Testing: Bootstrapped and regtested on aarch64-none-linux-gnu and added
new test.
Is this ok for trunk?
OK.
Thanks James but I realized I marked this wrong as only AArch64 patch.
This also has a mid change so cc'ing more people for approval.
Sudi
Thanks,
James
*** gcc/ChangeLog ***
2018-03-14 Sudakshina Das
* builtins.c (expand_builtin_setjmp_receiver): Update condition
to restore frame pointer.
* config/aarch64/aarch64.h (DONT_USE_BUILTIN_SETJMP): Update
comment.
* config/aarch64/aarch64.c (aarch64_builtin_setjmp_frame_value):
New.
(TARGET_BUILTIN_SETJMP_FRAME_VALUE): Define.
*** gcc/testsuite/ChangeLog ***
2018-03-14 Sudakshina Das
* gcc.c-torture/execute/pr84521.c: New test.
diff --git a/gcc/builtins.c b/gcc/builtins.c
index 85affa7..640f1a9 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -898,7 +898,8 @@ expand_builtin_setjmp_receiver (rtx receiver_label)
/* Now put in the code to restore the frame pointer, and argument
pointer, if needed. */
- if (! targetm.have_nonlocal_goto ())
+ if (! targetm.have_nonlocal_goto ()
+ && targetm.builtin_setjmp_frame_value () != hard_frame_pointer_rtx)
{
/* First adjust our frame pointer to its actual value. It was
previously set to the start of the virtual area corresponding to
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index e3c52f6..7a21c14 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -474,7 +474,9 @@ extern unsigned aarch64_architecture_version;
#define EH_RETURN_STACKADJ_RTXgen_rtx_REG (Pmode, R4_REGNUM)
#define EH_RETURN_HANDLER_RTX aarch64_eh_return_handler_rtx ()
-/* Don't use __builtin_setjmp until we've defined it. */
+/* Don't use __builtin_setjmp until we've defined it.
+ CAUTION: This macro is only used during exception unwinding.
+ Don't fall for its name. */
#undef DONT_USE_BUILTIN_SETJMP
#define DONT_USE_BUILTIN_SETJMP 1
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index e1fb87f..e7ac0fe 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -12128,6 +12128,13 @@ aarch64_expand_builtin_va_start (tree valist, rtx
nextarg ATTRIBUTE_UNUSED)
expand_expr (t, const0_rtx, VOIDmode, EXPAND_NORMAL);
}
+/* Implement TARGET_BUILTIN_SETJMP_FRAME_VALUE. */
+static rtx
+aarch64_builtin_setjmp_frame_value (void)
+{
+ return hard_frame_pointer_rtx;
+}
+
/* Implement TARGET_GIMPLIFY_VA_ARG_EXPR. */
static tree
@@ -17505,6 +17512,9 @@ aarch64_run_selftests (void)
#undef TARGET_FOLD_BUILTIN
#define TARGET_FOLD_BUILTIN aarch64_fold_builtin
+#undef TARGET_BUILTIN_SETJMP_FRAME_VALUE
+#define TARGET_BUILTIN_SETJMP_FRAME_VALUE aarch64_builtin_setjmp_frame_value
+
#undef TARGET_FUNCTION_ARG
#define TARGET_FUNCTION_ARG aarch64_function_arg
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr84521.c b/gcc/testsuite/gcc.c-torture/execute/pr84521.c
new file mode 100644
index 000..76b10d2
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr84521.c
@@ -0,0 +1,49 @@
+/* { dg-require-effective-target indirect_jumps } */
+
+#include
+
+jmp_buf buf;
+
+int uses_longjmp (void)
+{
+ __builtin_longjmp (buf, 1);
+}
+
+int gl;
+void after_longjmp (void)
+{
+ gl = 5;
+}
+
+int
+test_1 (int n)
+{
+ volatile int *p = alloca (n);
+ if (__builtin_setjmp (buf))
+{
+ after_longjmp ();
+}
+ else
+{
+ uses_longjmp ();
+}
+
+ return 0;
+}
+
+int __attribute__ ((optimize ("no-omit-frame-pointer")))
+test_2 (int n)
+{
+ int i;
+ int *ptr = (int *)__builtin_alloca (sizeof (int) * n);
+ for (i = 0; i < n; i++)
+ptr[i] = i;
+ test_1 (n);
+ return 0;
+}
+
+int main (int argc, const char **argv)
+{
+ __builtin_memset (&buf, 0xaf, sizeof (buf));
+ test_2 (100);
+}
Re: Deprecate some C++ extensions
On 03/20/2018 12:37 PM, Nathan Sidwell wrote: This patch deprecates 2 sets of extensions 1) 'T v (init) __attribute__ ((ignored))' That attribute placement has had no effect since the new parser (2002, I think gcc 3.3). Now we're more noisy about it. 2) anonymous struct or union members could be things other than public non-static data members. Now we're noisier about that too. I updated the 'Deprecated Features' piece of the documentation, it was mentioning some now-removed items as merely deprecated. It occurs to me we should probably just merge the 'Backwards Compatibility' section into 'Deprecated Features'. Perhaps add a 'Removed Features' section too? Sandra, WDYT? In general, I don't think the current GCC manual should document features that are no longer present in current GCC. I've previously done a bunch of cleanups to other parts of the manual removing such bits for features that were documented as having gone away 10-20 years ago. :-P So I think it is better to remove the documentation at the same time you remove the features, and mention the removal of the obsolete feature in the release notes instead of in the manual proper. -Sandra
Desire to allocate bit in DT_PARM bitmask for DEC FORMAT compatibility purposes
Codethink has several more changes that improve gfortran's ability to
handle legacy codebases, particularly those which rely on DEC
extensions. Most are strictly compiler side issues. However, one
touches on the runtime.
Specifically, as an extension, DEC Fortran allows omitting the width in
format statement field descriptors. The runtime then selects a default
width based on the data type.
This is documented in the old manuals from DEC and I've found
essentially the same documentation in Oracle/Sun's current documentation
as well as old MIPS documentation. I have a high degree of confidence
it exists in IBM's Fortran compilers as well. In contrast Intel & PCG's
Fortran compilers do not seem to support this extension.
Oracle's docs can be found here (Defaults for w, d, and e):
https://docs.oracle.com/cd/E19957-01/805-4939/z40007437a2e/index.html
Another example:
http://wwwteor.mi.infn.it/~vicini/decfortman.html#77
Because this is a case where where a compile-time flag needs to affect
the runtime, we need to communicate to the runtime that the magic
compile-time flag is on.
We have two general approaches for this kind of communication. One is
to set a mask within the DT_PARM which gets passed into the runtime at
the call site. The other is to marshall the flags in
gfortran_set{args,options} on a global basis.
Jakub has indicated the former approach is generally preferred and it
matches what was recently done for the default exponent handling. So
that's what this patch does under the control of -fdec-format-defaults
I am _not_ proposing this patch for inclusion into gcc-8. I'll propose
it for gcc-9. However, I would like to get the bit within the DT_PARM
bitmask reserved at this time for this purpose. I'd like to use bit
#28. That leaves two free bits remaining. I'm not aware of any pending
need to allocate either of those two free bits.
If we can agree to allocate bit #28 for this purpose I'll propose a
gcc-8 patch which notes the bit's reservation as a comment.
Thoughts?
Jeff
* gfortran.h (struct gfc_dt): Add DEFAULT_WIDTH field.
* io.c (check_format): For -fdec-format-defaults, allow
empty width field descriptor.
(match_io): Set dt->default_width as necessary.
* ioparm.h (IOPARM_dt_default_width): Define.
* lang.opt: Add -fdec-format-defaults.
* trans-io.c (build_dt): Set IOPARM_dt_default_width as
necessary.
* gfortran.dg/fmt_f_default_field_width.f90: New test.
* gfortran.dg/fmt_g_default_field_width.f90: New test.
* gfortran.dg/fmt_i_default_field_width.f90: New test.
* io/format.c (parse_format_list): Conditionally handle
defaulted widths.
* io/io.h (IOPARM_DT_DEFAULT_WIDTH): Define.
(default_width_for_integer): New function.
(default_width_for_float): New function.
(default_precision_for_float): New function.
* io/read.c (read_decimal): Handle case where width is
the defaulted.
* io/write.c (write_boz): Accept new LEN paramter. Use it
to determine the default width as needed.
(write_b, write_o, write_z): Pass LEN argument to write_boz.
(write_decimal): Use LEN to determine default width as needed.
(size_from_kind): Handle defaulted widths as well.
* write_float.def (build_float_string): Accept new DEFAULT_WIDTH
parameter. Use it as needed.
(FORMAT_FLOAT): Pass new argument to build_float_string.
Handle defaulted widths as needed.
(get_float_string): Similarly.
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 2b9eb23..922558a 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -2440,6 +2440,7 @@ typedef struct
*id, *pos, *asynchronous, *blank, *decimal, *delim, *pad, *round,
*sign, *extra_comma, *dt_io_kind, *udtio;
char default_exp;
+ char default_width;
gfc_symbol *namelist;
/* A format_label of `format_asterisk' indicates the "*" format */
diff --git a/gcc/fortran/io.c b/gcc/fortran/io.c
index ce4eef3..68da85d 100644
--- a/gcc/fortran/io.c
+++ b/gcc/fortran/io.c
@@ -912,6 +912,13 @@ data_desc:
if (u != FMT_POSINT)
{
+ if (flag_dec_format_defaults)
+ {
+ /* Assume a default width based on the variable size. */
+ saved_token = u;
+ break;
+ }
+
format_locus.nextc += format_string_pos;
gfc_error ("Positive width required in format "
"specifier %s at %L", token_to_string (t),
@@ -1036,6 +1043,13 @@ data_desc:
goto fail;
if (t != FMT_ZERO && t != FMT_POSINT)
{
+ if (flag_dec_format_defaults)
+ {
+ /* Assume the default width is expected here and continue lexing.
*/
+ value = 0; /* It doesn't matter what we set the value to here. */
+ saved_token = t;
+
Deprecate some C++ extensions
This patch deprecates 2 sets of extensions
1) 'T v (init) __attribute__ ((ignored))'
That attribute placement has had no effect since the new parser (2002, I
think gcc 3.3). Now we're more noisy about it.
2) anonymous struct or union members could be things other than public
non-static data members. Now we're noisier about that too.
I updated the 'Deprecated Features' piece of the documentation, it was
mentioning some now-removed items as merely deprecated. It occurs to me
we should probably just merge the 'Backwards Compatibility' section into
'Deprecated Features'. Perhaps add a 'Removed Features' section too?
Sandra, WDYT?
nathan
--
Nathan Sidwell
2018-03-20 Nathan Sidwell
* doc/extend.texi (Deprecated Features): Update deprecared flags,
mention anon-struct/union members and trailing attributes.
cp/
* class.c (finish_struct_anon_r): Refactor, deprecate anything
other than public non-static data members.
* parser.c (cp_parser_init_declarator): Deprecate attributes after
parenthesized initializer.
testsuite/
* g++.dg/ext/anon-struct6.C: Adjust.
* g++.dg/ext/deprecate-1.C: New.
* g++.dg/ext/deprecate-2.C: New.
* g++.dg/lookup/pr84602.C: Adjust.
* g++.dg/lookup/pr84962.C: Adjust.
* g++.old-deja/g++.other/anon4.C
Index: cp/class.c
===
--- cp/class.c (revision 258686)
+++ cp/class.c (working copy)
@@ -2869,9 +2869,7 @@ warn_hidden (tree t)
static void
finish_struct_anon_r (tree field, bool complain)
{
- bool is_union = TREE_CODE (TREE_TYPE (field)) == UNION_TYPE;
- tree elt = TYPE_FIELDS (TREE_TYPE (field));
- for (; elt; elt = DECL_CHAIN (elt))
+ for (tree elt = TYPE_FIELDS (TREE_TYPE (field)); elt; elt = DECL_CHAIN (elt))
{
/* We're generally only interested in entities the user
declared, but we also find nested classes by noticing
@@ -2885,50 +2883,34 @@ finish_struct_anon_r (tree field, bool c
|| TYPE_UNNAMED_P (TREE_TYPE (elt
continue;
- if (TREE_CODE (elt) != FIELD_DECL)
+ if (complain
+ && (TREE_CODE (elt) != FIELD_DECL
+ || (TREE_PRIVATE (elt) || TREE_PROTECTED (elt
{
/* We already complained about static data members in
finish_static_data_member_decl. */
- if (complain && !VAR_P (elt))
+ if (!VAR_P (elt)
+ && permerror (DECL_SOURCE_LOCATION (elt),
+ TREE_CODE (TREE_TYPE (field)) == UNION_TYPE
+ ? "%q#D invalid; an anonymous union may "
+ "only have public non-static data members"
+ : "%q#D invalid; an anonymous struct may "
+ "only have public non-static data members", elt))
{
- if (is_union)
- permerror (DECL_SOURCE_LOCATION (elt),
- "%q#D invalid; an anonymous union can "
- "only have non-static data members", elt);
- else
- permerror (DECL_SOURCE_LOCATION (elt),
- "%q#D invalid; an anonymous struct can "
- "only have non-static data members", elt);
- }
- continue;
- }
-
- if (complain)
- {
- if (TREE_PRIVATE (elt))
- {
- if (is_union)
- permerror (DECL_SOURCE_LOCATION (elt),
- "private member %q#D in anonymous union", elt);
- else
- permerror (DECL_SOURCE_LOCATION (elt),
- "private member %q#D in anonymous struct", elt);
- }
- else if (TREE_PROTECTED (elt))
- {
- if (is_union)
- permerror (DECL_SOURCE_LOCATION (elt),
- "protected member %q#D in anonymous union", elt);
- else
- permerror (DECL_SOURCE_LOCATION (elt),
- "protected member %q#D in anonymous struct", elt);
+ static bool hint;
+ if (flag_permissive && !hint)
+ {
+ hint = true;
+ inform (DECL_SOURCE_LOCATION (elt),
+ "this flexibility is deprecated and will be removed");
+ }
}
}
TREE_PRIVATE (elt) = TREE_PRIVATE (field);
TREE_PROTECTED (elt) = TREE_PROTECTED (field);
- /* Recurse into the anonymous aggregates to handle correctly
+ /* Recurse into the anonymous aggregates to correctly handle
access control (c++/24926):
class A {
Index: cp/parser.c
===
--- cp/parser.c (revision 258686)
+++ cp/parser.c (working copy)
@@ -19685,12 +19685,21 @@ cp_parser_init_declarator (cp_parser* pa
/* The old parser allows attributes to appear after a parenthesized
initializer. Mark Mitchell proposed removing this functionality
on the GCC mailing lists on 2002-08-13. This parser accepts the
- attributes -- but ignores them. */
+ attributes -- but ignores them. Made a permerror in GCC 8. */
if (cp_parser_allow_gnu_extensions_p (parser)
- && initialization_kind == CPP_OPEN_PAREN)
-if (cp_parser_attributes_opt (parser))
- warning (OPT_Wattributes,
- "attributes after parenthesized initializer ignored");
+ && initialization_kind == CPP_OPEN_PAREN
+ && cp_parser_attributes_opt (parser)
+ && permerror (input_location,
+ "attributes afte
[og7] backport fix for PR84952
I've applied this patch to openacc-gcc-7-branch which backports Tom's
fix for the nvptx bar.sync placement bug in PR84952. This patch also
reverts some changes I introduced in git revision 7445a4d40.
Tom's patch didn't apply cleanly because of the recent I renamed
nvptx_wsync to nvptx_cta_sync so that function can be used for both
large vector_lengths along with workers. Other than that, I didn't have
to make any changes to his patch.
Cesar
2018-03-20 Cesar Philippidis
gcc/
* config/nvptx/nvptx.c (nvptx_single): Revert changes from
7445a4d40.
Backport from trunk:
2018-03-20 Tom de Vries
PR target/84952
* config/nvptx/nvptx.c (nvptx_single): Don't neuter bar.sync.
(nvptx_process_pars): Emit bar.sync asap and alap.
diff --git a/gcc/config/nvptx/nvptx.c b/gcc/config/nvptx/nvptx.c
index 070d236fa87..b7e3f59fed7 100644
--- a/gcc/config/nvptx/nvptx.c
+++ b/gcc/config/nvptx/nvptx.c
@@ -3988,7 +3988,9 @@ nvptx_single (unsigned mask, basic_block from, basic_block to)
while (true)
{
/* Find first insn of from block. */
- while (head != BB_END (from) && !INSN_P (head))
+ while (head != BB_END (from)
+ && (!INSN_P (head)
+ || recog_memoized (head) == CODE_FOR_nvptx_barsync))
head = NEXT_INSN (head);
if (from == to)
@@ -4037,6 +4039,7 @@ nvptx_single (unsigned mask, basic_block from, basic_block to)
{
default:
break;
+ case CODE_FOR_nvptx_barsync:
case CODE_FOR_nvptx_fork:
case CODE_FOR_nvptx_forked:
case CODE_FOR_nvptx_joining:
@@ -4056,15 +4059,6 @@ nvptx_single (unsigned mask, basic_block from, basic_block to)
return;
}
- /* NVPTX_BARSYNC barriers are placed immediately before NVPTX_JOIN
- in order to ensure that all of the threads in a CTA reach the
- barrier. Don't nueter BLOCK if head is NVPTX_BARSYNC and tail is
- NVPTX_JOIN. */
- if (from == to
- && recog_memoized (head) == CODE_FOR_nvptx_barsync
- && recog_memoized (tail) == CODE_FOR_nvptx_join)
-return;
-
/* Insert the vector test inside the worker test. */
unsigned mode;
rtx_insn *before = tail;
@@ -4112,17 +4106,7 @@ nvptx_single (unsigned mask, basic_block from, basic_block to)
br = gen_br_true (pred, label);
else
br = gen_br_true_uni (pred, label);
-
- if (recog_memoized (head) == CODE_FOR_nvptx_forked
- && recog_memoized (NEXT_INSN (head)) == CODE_FOR_nvptx_barsync)
- {
- head = NEXT_INSN (head);
- emit_insn_after (br, head);
- }
- else if (recog_memoized (head) == CODE_FOR_nvptx_barsync)
- emit_insn_after (br, head);
- else
- emit_insn_before (br, head);
+ emit_insn_before (br, head);
LABEL_NUSES (label)++;
if (tail_branch)
@@ -4348,7 +4332,7 @@ nvptx_process_pars (parallel *par)
if (!empty || !is_call)
{
/* Insert begin and end synchronizations. */
- emit_insn_after (nvptx_cta_sync (false), par->forked_insn);
+ emit_insn_before (nvptx_cta_sync (false), par->forked_insn);
emit_insn_before (nvptx_cta_sync (true), par->join_insn);
}
}
Re: [PR c++/84647] undeclared fn called in auto default arg in ptr decl
On Sat, Mar 10, 2018 at 6:57 AM, Alexandre Oliva wrote: > We ICEd when attempting to convert a default arg for an auto parm, the > default arg amounting to a call to an undeclared function, in a > declaration of a pointer to function variable. This seems to have been fixed by my patch for 84798, so now we don't try to parse an implicit template. Jason
[PATCH PR84969]Don't reorder builtin memsets if they set different rhs values
Hi,
As noted in PR84969, fuse_memset_builtins breaks dependence between different
memsets.
Specifically, it reorders different builtin memset partitions though it doesn't
merge
them in the end. This simple patch fixes this wrong code issue by checking if
any two
builtin memsets set the same rhs value or not. Note we don't need to bother if
two
memsets intersect with each other or not.
Of course, this would miss opportunity merging S1/S3 in below case:
memset(p+12, 0, 12); //<-S1
memset(p+17, 1, 10);
memset(p, 0, 12); //<-S3
In my opinion, this should be resolved in a more general way maximizing
parallelism
as well as merging opportunities when sorting partitions into topological order
from
dependence graph, which isn't GCC8 task.
Bootstrap and test on x86_64 and AArch64 ongoing. Okay if no failures?
Thanks,
bin
2018-03-20 Bin Cheng
PR tree-optimization/84969
* tree-loop-distribution.c (fuse_memset_builtins): Don't reorder
builtin memset partitions if they set differnt rhs values.
gcc/testsuite
2018-03-20 Bin Cheng
PR tree-optimization/84969
* gcc.dg/tree-ssa/pr84969.c: New test.
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr84969.c
b/gcc/testsuite/gcc.dg/tree-ssa/pr84969.c
new file mode 100644
index 000..e15c3d9
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr84969.c
@@ -0,0 +1,57 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -ftree-loop-distribute-patterns" } */
+
+static void
+__attribute__((noipa, noinline))
+foo (char **values, int ndim, char *needquotes, int *dims)
+{
+ int i;
+ int j = 0;
+ int k = 0;
+ char *retval = (char *)__builtin_malloc(1000);
+ char *p = retval;
+ char *tmp;
+
+ int indx[111];
+
+#define APPENDSTR(str) (__builtin_strcpy(p, (str)), p += __builtin_strlen(p))
+#define APPENDCHAR(ch) (*p++ = (ch), *p = '\0')
+
+ APPENDCHAR('{');
+ for (i = 0; i < ndim; i++)
+ indx[i] = 0;
+ do
+ {
+ for (i = j; i < ndim - 1; i++)
+ APPENDCHAR('{');
+
+ APPENDSTR(values[k]);
+ k++;
+
+ for (i = ndim - 1; i >= 0; i--)
+ {
+ indx[i] = (indx[i] + 1) % dims[i];
+ if (indx[i])
+ {
+ APPENDCHAR(',');
+ break;
+ }
+ else
+ APPENDCHAR('}');
+ }
+ j = i;
+ } while (j != -1);
+
+ if (__builtin_strcmp (retval, "{{{0,1},{2,3}}}") != 0)
+ __builtin_abort ();
+}
+
+int main()
+{
+ char* array[4] = {"0", "1", "2", "3"};
+ char f[] = {0, 0, 0, 0, 0, 0, 0, 0};
+ int dims[] = {1, 2, 2};
+ foo (array, 3, f, dims);
+
+ return 0;
+}
diff --git a/gcc/tree-loop-distribution.c b/gcc/tree-loop-distribution.c
index 67f27ba..5e327f4 100644
--- a/gcc/tree-loop-distribution.c
+++ b/gcc/tree-loop-distribution.c
@@ -2569,6 +2569,7 @@ fuse_memset_builtins (vec *partitions)
{
unsigned i, j;
struct partition *part1, *part2;
+ tree rhs1, rhs2;
for (i = 0; partitions->iterate (i, &part1);)
{
@@ -2586,6 +2587,12 @@ fuse_memset_builtins (vec
*partitions)
|| !operand_equal_p (part1->builtin->dst_base_base,
part2->builtin->dst_base_base, 0))
break;
+
+ /* Memset calls setting different values can't be merged. */
+ rhs1 = gimple_assign_rhs1 (DR_STMT (part1->builtin->dst_dr));
+ rhs2 = gimple_assign_rhs1 (DR_STMT (part2->builtin->dst_dr));
+ if (!operand_equal_p (rhs1, rhs2, 0))
+ break;
}
/* Stable sort is required in order to avoid breaking dependence. */
@@ -2617,8 +2624,8 @@ fuse_memset_builtins (vec *partitions)
i++;
continue;
}
- tree rhs1 = gimple_assign_rhs1 (DR_STMT (part1->builtin->dst_dr));
- tree rhs2 = gimple_assign_rhs1 (DR_STMT (part2->builtin->dst_dr));
+ rhs1 = gimple_assign_rhs1 (DR_STMT (part1->builtin->dst_dr));
+ rhs2 = gimple_assign_rhs1 (DR_STMT (part2->builtin->dst_dr));
int bytev1 = const_with_all_bytes_same (rhs1);
int bytev2 = const_with_all_bytes_same (rhs2);
/* Only merge memset partitions of the same value. */
Re: [PR middle-end/70359] uncoalesce IVs outside of loops
On Tue, Mar 20, 2018 at 5:56 PM, Richard Biener wrote: > On March 20, 2018 6:11:53 PM GMT+01:00, "Bin.Cheng" > wrote: >>On Mon, Mar 19, 2018 at 5:08 PM, Aldy Hernandez >>wrote: >>> Hi Richard. >>> >>> As discussed in the PR, the problem here is that we have two >>different >>> iterations of an IV live outside of a loop. This inhibits us from >>using >>> autoinc/dec addressing on ARM, and causes extra lea's on x86. >>> >>> An abbreviated example is this: >>> >>> loop: >>> # p_9 = PHI >>> p_20 = p_9 + 18446744073709551615; >>> goto loop >>> p_24 = p_9 + 18446744073709551614; >>> MEM[(char *)p_20 + -1B] = 45; >>> >>> Here we have both the previous IV (p_9) and the current IV (p_20) >>used >>> outside of the loop. On Arm this keeps us from using auto-dec >>addressing, >>> because one use is -2 and the other one is -1. >>> >>> With the attached patch we attempt to rewrite out-of-loop uses of the >>IV in >>> terms of the current/last IV (p_20 in the case above). With it, we >>end up >>> with: >>> >>> p_24 = p_20 + 18446744073709551615; >>> *p_24 = 45; >>> >>> ...which helps both x86 and Arm. >>> >>> As you have suggested in comment 38 on the PR, I handle specially >>> out-of-loop IV uses of the form IV+CST and propagate those >>accordingly >>> (along with the MEM_REF above). Otherwise, in less specific cases, >>we un-do >>> the IV increment, and use that value in all out-of-loop uses. For >>instance, >>> in the attached testcase, we rewrite: >>> >>> george (p_9); >>> >>> into >>> >>> _26 = p_20 + 1; >>> ... >>> george (_26); >>> >>> The attached testcase tests the IV+CST specific case, as well as the >>more >>> generic case with george(). >>> >>> Although the original PR was for ARM, this behavior can be noticed on >>x86, >>> so I tested on x86 with a full bootstrap + tests. I also ran the >>specific >>> test on an x86 cross ARM build and made sure we had 2 auto-dec with >>the >>> test. For the original test (slightly different than the testcase in >>this >>> patch), with this patch we are at 104 bytes versus 116 without it. >>There is >>> still the issue of a division optimization which would further reduce >>the >>> code size. I will discuss this separately as it is independent from >>this >>> patch. >>> >>> Oh yeah, we could make this more generic, and maybe handle any >>multiple of >>> the constant, or perhaps *= and /=. Perhaps something for next >>stage1... >>> >>> OK for trunk? >>Just FYI, this looks similar to what I did in >>https://gcc.gnu.org/ml/gcc-patches/2013-11/msg00535.html >>That change was non-trivial and didn't give obvious improvement back >>in time. But I still wonder if this >>can be done at rewriting iv_use in a light-overhead way. > > Certainly, but the issue is we wreck it again at forwprop time as ivopts runs > too early. So both values of p_9/p_20 are used after loop. loop: # p_9 = PHI p_20 = p_9 + 18446744073709551615; goto loop p_24 = p_20 + 18446744073709551615; MEM[(char *)p_20 + -1B] = 45; It looks like a fwprop issue that propagating p_20 with p_9 which results in below code: loop: # p_9 = PHI p_20 = p_9 + 18446744073709551615; goto loop p_24 = p_9 + 18446744073709551614; MEM[(char *)p_20 + -1B] = 45; It creates intersecting/longer live ranges while doesn't eliminate copy or definition for p_9. Ah, IIRC, RTL address forward propagation also has this issue. Thanks, bin > > Richard. >> >>Thanks, >>bin >>> Aldy >
Re: [PR c++/84729] convert new init to array elt type
On Sat, Mar 10, 2018 at 6:57 AM, Alexandre Oliva wrote: > A parenthesized initializer is only accepted when new()ing an array in > permissive mode. We were not careful, however, to convert the > TREE_LIST initializer to the array element type in this extension. > This patch fixes it: after turning the TREE_LIST initializer to a > compound_expr, we convert it to the base type. I think I'd rather turn the permerror into a hard error than improve support for a deprecated extension. Jason
Re: C++ PATCH for c++/84978, ICE with NRVO
On Tue, Mar 20, 2018 at 11:55 AM, Jason Merrill wrote:
> On Tue, Mar 20, 2018 at 9:32 AM, Marek Polacek wrote:
>> We started crashing on this test with r258592 which added
>> cp_get_callee_fndecl
>> in in cp_genericize_r.
>>
>> This ICE apparently depends on whether we perform NRVO or not. If the size
>> of
>> S is <=16B we pass it in registers and it compiles fine. But if the size of
>> S
>> is >16B, then we pass in memory, and we NRV-optimize. That means that
>> s.fn ();
>> is turned by finalize_nrv into
>> .fn ();
>>
>> Then the newly added call to cp_get_callee_fndecl calls maybe_constant_init,
>
> Oops, I forgot that cp_get_callee_fndecl would call
> maybe_constant_init, I was just using it to handle both CALL_EXPR and
> AGGR_INIT_EXPR. And in fact it looks like we don't really want that
> for the other users, either. I think I'll remove it.
Thus.
commit 06d961cdae77d73332845eaa2ed8d1ea1867dc73
Author: Jason Merrill
Date: Tue Mar 20 08:51:18 2018 -0400
PR c++/84978, ICE with NRVO.
* cvt.c (cp_get_fndecl_from_callee): Add fold parameter.
(cp_get_callee_fndecl_nofold): New.
* cp-gimplify.c (cp_genericize_r): Use it instead.
* call.c (check_self_delegation): Likewise.
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 23d4f82a1a0..dbdb8d5812d 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -8739,7 +8739,7 @@ check_self_delegation (tree ret)
{
if (TREE_CODE (ret) == TARGET_EXPR)
ret = TARGET_EXPR_INITIAL (ret);
- tree fn = cp_get_callee_fndecl (ret);
+ tree fn = cp_get_callee_fndecl_nofold (ret);
if (fn && DECL_ABSTRACT_ORIGIN (fn) == current_function_decl)
error ("constructor delegates to itself");
}
diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index 332ff2bbb05..3edecf24c92 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -1521,7 +1521,7 @@ cp_genericize_r (tree *stmt_p, int *walk_subtrees, void *data)
version is inlinable, a direct call to this version can be made
otherwise the call should go through the dispatcher. */
{
- tree fn = cp_get_callee_fndecl (stmt);
+ tree fn = cp_get_callee_fndecl_nofold (stmt);
if (fn && DECL_FUNCTION_VERSIONED (fn)
&& (current_function_decl == NULL
|| !targetm.target_option.can_inline_p (current_function_decl,
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 727822e36da..9befde3329d 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6110,7 +6110,8 @@ extern tree cp_convert_and_check(tree, tree, tsubst_flags_t);
extern tree cp_fold_convert (tree, tree);
extern tree cp_get_callee (tree);
extern tree cp_get_callee_fndecl (tree);
-extern tree cp_get_fndecl_from_callee (tree);
+extern tree cp_get_callee_fndecl_nofold (tree);
+extern tree cp_get_fndecl_from_callee (tree, bool fold = true);
extern tree convert_to_void (tree, impl_conv_void,
tsubst_flags_t);
extern tree convert_force (tree, tree, int,
diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c
index 40e7576f23c..9b53fa3067d 100644
--- a/gcc/cp/cvt.c
+++ b/gcc/cp/cvt.c
@@ -941,7 +941,7 @@ cp_get_callee (tree call)
if we can. */
tree
-cp_get_fndecl_from_callee (tree fn)
+cp_get_fndecl_from_callee (tree fn, bool fold /* = true */)
{
if (fn == NULL_TREE)
return fn;
@@ -951,7 +951,8 @@ cp_get_fndecl_from_callee (tree fn)
if (type == unknown_type_node)
return NULL_TREE;
gcc_assert (POINTER_TYPE_P (type));
- fn = maybe_constant_init (fn);
+ if (fold)
+fn = maybe_constant_init (fn);
STRIP_NOPS (fn);
if (TREE_CODE (fn) == ADDR_EXPR)
{
@@ -971,6 +972,14 @@ cp_get_callee_fndecl (tree call)
return cp_get_fndecl_from_callee (cp_get_callee (call));
}
+/* As above, but not using the constexpr machinery. */
+
+tree
+cp_get_callee_fndecl_nofold (tree call)
+{
+ return cp_get_fndecl_from_callee (cp_get_callee (call), false);
+}
+
/* Subroutine of convert_to_void. Warn if we're discarding something with
attribute [[nodiscard]]. */
Re: [PR middle-end/70359] uncoalesce IVs outside of loops
On March 20, 2018 6:11:53 PM GMT+01:00, "Bin.Cheng" wrote: >On Mon, Mar 19, 2018 at 5:08 PM, Aldy Hernandez >wrote: >> Hi Richard. >> >> As discussed in the PR, the problem here is that we have two >different >> iterations of an IV live outside of a loop. This inhibits us from >using >> autoinc/dec addressing on ARM, and causes extra lea's on x86. >> >> An abbreviated example is this: >> >> loop: >> # p_9 = PHI >> p_20 = p_9 + 18446744073709551615; >> goto loop >> p_24 = p_9 + 18446744073709551614; >> MEM[(char *)p_20 + -1B] = 45; >> >> Here we have both the previous IV (p_9) and the current IV (p_20) >used >> outside of the loop. On Arm this keeps us from using auto-dec >addressing, >> because one use is -2 and the other one is -1. >> >> With the attached patch we attempt to rewrite out-of-loop uses of the >IV in >> terms of the current/last IV (p_20 in the case above). With it, we >end up >> with: >> >> p_24 = p_20 + 18446744073709551615; >> *p_24 = 45; >> >> ...which helps both x86 and Arm. >> >> As you have suggested in comment 38 on the PR, I handle specially >> out-of-loop IV uses of the form IV+CST and propagate those >accordingly >> (along with the MEM_REF above). Otherwise, in less specific cases, >we un-do >> the IV increment, and use that value in all out-of-loop uses. For >instance, >> in the attached testcase, we rewrite: >> >> george (p_9); >> >> into >> >> _26 = p_20 + 1; >> ... >> george (_26); >> >> The attached testcase tests the IV+CST specific case, as well as the >more >> generic case with george(). >> >> Although the original PR was for ARM, this behavior can be noticed on >x86, >> so I tested on x86 with a full bootstrap + tests. I also ran the >specific >> test on an x86 cross ARM build and made sure we had 2 auto-dec with >the >> test. For the original test (slightly different than the testcase in >this >> patch), with this patch we are at 104 bytes versus 116 without it. >There is >> still the issue of a division optimization which would further reduce >the >> code size. I will discuss this separately as it is independent from >this >> patch. >> >> Oh yeah, we could make this more generic, and maybe handle any >multiple of >> the constant, or perhaps *= and /=. Perhaps something for next >stage1... >> >> OK for trunk? >Just FYI, this looks similar to what I did in >https://gcc.gnu.org/ml/gcc-patches/2013-11/msg00535.html >That change was non-trivial and didn't give obvious improvement back >in time. But I still wonder if this >can be done at rewriting iv_use in a light-overhead way. Certainly, but the issue is we wreck it again at forwprop time as ivopts runs too early. Richard. > >Thanks, >bin >> Aldy
Re: [PATCH, rs6000] Finish implementation of __builtin_atlivec_lvx_v1ti
On 3/14/18 4:27 PM, Kelvin Nilsen wrote: > @@ -14452,6 +14452,7 @@ altivec_expand_lv_builtin (enum insn_code icode, t > LVXL and LVE*X expand to use UNSPECs to hide their special behavior, > so the raw address is fine. */ >if (icode == CODE_FOR_altivec_lvx_v2df_2op > + || icode == CODE_FOR_altivec_lvx_v1ti_2op >|| icode == CODE_FOR_altivec_lvx_v2di_2op >|| icode == CODE_FOR_altivec_lvx_v4sf_2op >|| icode == CODE_FOR_altivec_lvx_v4si_2op > @@ -15811,6 +15812,9 @@ altivec_expand_builtin (tree exp, rtx target, bool > case ALTIVEC_BUILTIN_LVX_V2DI: >return altivec_expand_lv_builtin (CODE_FOR_altivec_lvx_v2di_2op, > exp, target, false); > +case ALTIVEC_BUILTIN_LVX_V1TI: > + return altivec_expand_lv_builtin (CODE_FOR_altivec_lvx_v1ti_2op, > + exp, target, false); > case ALTIVEC_BUILTIN_LVX_V4SF: >return altivec_expand_lv_builtin (CODE_FOR_altivec_lvx_v4sf_2op, > exp, target, false); FYI, these hunks will need updating due to the fix for PR83789 which I just committed to trunk. Peter
Re: [PATCH, rs6000] Fix PR83789: __builtin_altivec_lvx fails for powerpc for altivec-4.c
On 3/20/18 11:42 AM, Segher Boessenkool wrote: > On Mon, Mar 19, 2018 at 09:12:08PM -0500, Peter Bergner wrote: >> Looking at mu build dirs insn-modes.h, I don't see HAVE_V8HFmode being >> defined on either my LE or BE builds. What am I missing? > > Nothing, I just was confused (we always define the mode in rs6000-modes.def, > but that is not the same thing). Sorry. Ok then, patch committed with the expander change you requested and leaving the HAVE_V8HFmode code as is in the patch. Thanks! Peter
Re: [PATCH] S/390: Set ABI default based on uname
Hi, On Tue, 20 Mar 2018, Andreas Krebbel wrote: > On 03/13/2018 04:53 PM, Michael Matz wrote: > > Hi, > > > > On Tue, 13 Mar 2018, Andreas Krebbel wrote: > > > >> Leaving history aside don't you agree that it would have been more > >> sensible to require a -m option only if you want to build for an ABI > >> different from what is currently mandated by the personality setting? > > > > I agree with you. But we can't leave history aside, so I'm having the > > same sentiment as Jakub. The current situation is not good, but changing > > behaviour would be even worse. > > Ok. I'm not convinced that this change will actually hurt somewhere but > having downvotes from you guys is enough to drop that patch. > > If I still have to build 32 bit stuff after your distros dropped 32 bit > support entirely I'll probably come back with it ;) Heh, maybe in 15 years ;) Ciao, Michael.
[PATCH][arm] PR target/82518: Return false in ARRAY_MODE_SUPPORTED_P for BYTES_BIG_ENDIAN
Hi all,
This PR shows that we get the load/store_lanes logic wrong for arm big-endian.
It is tricky to get right. Aarch64 does it by adding the appropriate
lane-swapping
operations during expansion.
I'd like to do the same on arm eventually, but we'd need to port and validate
the VTBL-generating
code and add it to all the right places and I'm not comfortable enough doing it
for GCC 8, but I am keen
in getting the wrong-code fixed.
As I say in the PR, vectorisation on armeb is already severely restricted (we
disable many patterns on BYTES_BIG_ENDIAN)
and the load/store_lanes patterns really were not working properly at all, so
disabling them is not
a radical approach.
The way to do that is to return false in ARRAY_MODE_SUPPORTED_P for
BYTES_BIG_ENDIAN.
Bootstrapped and tested on arm-none-linux-gnueabihf.
Also tested on armeb-none-eabi.
Committing to trunk.
Thanks,
Kyrill
2018-03-20 Kyrylo Tkachov
PR target/82518
* config/arm/arm.c (arm_array_mode_supported_p): Return false for
BYTES_BIG_ENDIAN.
2018-03-20 Kyrylo Tkachov
PR target/82518
* lib/target-supports.exp (check_effective_target_vect_load_lanes):
Disable for armeb targets.
* gcc.target/arm/pr82518.c: New test.
commit 861f9881d3af7e6f94e9b008e88aa7ba7c44a4df
Author: Kyrylo Tkachov
Date: Tue Feb 20 15:14:01 2018 +
[arm] PR target/82518: Return false in ARRAY_MODE_SUPPORTED_P for BYTES_BIG_ENDIAN
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 73e9fc1..09deb60 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -27151,7 +27151,10 @@ static bool
arm_array_mode_supported_p (machine_mode mode,
unsigned HOST_WIDE_INT nelems)
{
- if (TARGET_NEON
+ /* We don't want to enable interleaved loads and stores for BYTES_BIG_ENDIAN
+ for now, as the lane-swapping logic needs to be extended in the expanders.
+ See PR target/82518. */
+ if (TARGET_NEON && !BYTES_BIG_ENDIAN
&& (VALID_NEON_DREG_MODE (mode) || VALID_NEON_QREG_MODE (mode))
&& (nelems >= 2 && nelems <= 4))
return true;
diff --git a/gcc/testsuite/gcc.target/arm/pr82518.c b/gcc/testsuite/gcc.target/arm/pr82518.c
new file mode 100644
index 000..c3e45b8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr82518.c
@@ -0,0 +1,29 @@
+/* { dg-do run } */
+/* { dg-require-effective-target arm_neon_ok } */
+/* { dg-additional-options "-O3 -fno-inline -std=gnu99" } */
+/* { dg-add-options arm_neon } */
+
+typedef struct { int x, y; } X;
+
+void f4(X *p, int n)
+{
+ for (int i = 0; i < n; i++)
+ { p[i].x = i;
+p[i].y = i + 1;
+ }
+}
+
+__attribute ((aligned (16))) X arr[100];
+
+int main(void)
+{
+ volatile int fail = 0;
+ f4 (arr, 100);
+ for (int i = 0; i < 100; i++)
+if (arr[i].y != i+1 || arr[i].x != i)
+ fail = 1;
+ if (fail)
+ __builtin_abort ();
+
+ return 0;
+}
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 640f76e..5ff5ec6 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -6609,7 +6609,8 @@ proc check_effective_target_vect_load_lanes { } {
verbose "check_effective_target_vect_load_lanes: using cached result" 2
} else {
set et_vect_load_lanes 0
- if { ([istarget arm*-*-*] && [check_effective_target_arm_neon_ok])
+ # We don't support load_lanes correctly on big-endian arm.
+ if { ([istarget arm-*-*] && [check_effective_target_arm_neon_ok])
|| [istarget aarch64*-*-*] } {
set et_vect_load_lanes 1
}
Re: [PR middle-end/70359] uncoalesce IVs outside of loops
On Mon, Mar 19, 2018 at 5:08 PM, Aldy Hernandez wrote: > Hi Richard. > > As discussed in the PR, the problem here is that we have two different > iterations of an IV live outside of a loop. This inhibits us from using > autoinc/dec addressing on ARM, and causes extra lea's on x86. > > An abbreviated example is this: > > loop: > # p_9 = PHI > p_20 = p_9 + 18446744073709551615; > goto loop > p_24 = p_9 + 18446744073709551614; > MEM[(char *)p_20 + -1B] = 45; > > Here we have both the previous IV (p_9) and the current IV (p_20) used > outside of the loop. On Arm this keeps us from using auto-dec addressing, > because one use is -2 and the other one is -1. > > With the attached patch we attempt to rewrite out-of-loop uses of the IV in > terms of the current/last IV (p_20 in the case above). With it, we end up > with: > > p_24 = p_20 + 18446744073709551615; > *p_24 = 45; > > ...which helps both x86 and Arm. > > As you have suggested in comment 38 on the PR, I handle specially > out-of-loop IV uses of the form IV+CST and propagate those accordingly > (along with the MEM_REF above). Otherwise, in less specific cases, we un-do > the IV increment, and use that value in all out-of-loop uses. For instance, > in the attached testcase, we rewrite: > > george (p_9); > > into > > _26 = p_20 + 1; > ... > george (_26); > > The attached testcase tests the IV+CST specific case, as well as the more > generic case with george(). > > Although the original PR was for ARM, this behavior can be noticed on x86, > so I tested on x86 with a full bootstrap + tests. I also ran the specific > test on an x86 cross ARM build and made sure we had 2 auto-dec with the > test. For the original test (slightly different than the testcase in this > patch), with this patch we are at 104 bytes versus 116 without it. There is > still the issue of a division optimization which would further reduce the > code size. I will discuss this separately as it is independent from this > patch. > > Oh yeah, we could make this more generic, and maybe handle any multiple of > the constant, or perhaps *= and /=. Perhaps something for next stage1... > > OK for trunk? Just FYI, this looks similar to what I did in https://gcc.gnu.org/ml/gcc-patches/2013-11/msg00535.html That change was non-trivial and didn't give obvious improvement back in time. But I still wonder if this can be done at rewriting iv_use in a light-overhead way. Thanks, bin > Aldy
Re: [PR c++/71251] out-of-range parms in tmpl arg substitution
On Sat, Mar 17, 2018 at 8:13 AM, Alexandre Oliva wrote: > As we go through each of the template parameters, substituting it with > corresponding template arguments, an incorrect argument list might > cause us to index argument vectors past their length (or fail in the > preceding tree checks). Avoid such dereferences and instead issue an > error (if requested) if we find the argument index to be past the > parameter vector length. Any time we hit this abort, it indicates a bug in earlier processing, so that we're looking up a template parameter in an argument list for a different template. Aborting in that situation is appropriate; it has revealed many bugs. Jason
Re: [PR c++/71965] silence multi-dim array init sorry without tf_error
On Sat, Mar 17, 2018 at 8:11 AM, Alexandre Oliva wrote:
> We shouldn't substitute templates into short-circuited-out concepts
> constraints, but we do, and to add insult to injury, we issue a
> sorry() error when a concept that shouldn't even have been substituted
> attempts to perform a multi-dimensional array initialization with a
> new{} expression.
>
> Although fixing the requirements short-circuiting is probably too
> risky at this point, we can get closer to the intended effect by
> silencing that sorry just as we silence other errors.
>
> for gcc/cp/ChangeLog
>
> PR c++/71965
> * init.c (build_vec_init): Silence sorry without tf_error.
>
> for gcc/testsuite/ChangeLog
>
> PR c++/71965
> * g++.dg/concepts/pr71965.C: New.
> ---
> gcc/cp/init.c | 19 ---
> gcc/testsuite/g++.dg/concepts/pr71965.C | 27 +++
> 2 files changed, 39 insertions(+), 7 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/concepts/pr71965.C
>
> diff --git a/gcc/cp/init.c b/gcc/cp/init.c
> index cb62f4886e6d..dcaad730dc86 100644
> --- a/gcc/cp/init.c
> +++ b/gcc/cp/init.c
> @@ -4384,12 +4384,17 @@ build_vec_init (tree base, tree maxindex, tree init,
>else if (TREE_CODE (type) == ARRAY_TYPE)
> {
> if (init && !BRACE_ENCLOSED_INITIALIZER_P (init))
> - sorry
> - ("cannot initialize multi-dimensional array with initializer");
> - elt_init = build_vec_init (build1 (INDIRECT_REF, type, base),
> -0, init,
> -explicit_value_init_p,
> -0, complain);
> + {
> + if ((complain & tf_error))
> + sorry ("cannot initialize multi-dimensional"
> + " array with initializer");
> + elt_init = error_mark_node;
This shouldn't even be a sorry anymore; in build_aggr_init, we have
permerror (init_loc, "array must be initialized "
"with a brace-enclosed initializer");
Let's make it a hard error here.
BTW, please include the word "PATCH" in the subject line of patch
submissions. I will also see them faster if you CC me.
Jason
Re: deprecate ignored attributes?
On Tue, Mar 20, 2018 at 10:37 AM, Nathan Sidwell wrote: > Jason, in parser.c > /* The old parser allows attributes to appear after a parenthesized > initializer. Mark Mitchell proposed removing this functionality > on the GCC mailing lists on 2002-08-13. This parser accepts the > attributes -- but ignores them. */ > if (cp_parser_allow_gnu_extensions_p (parser) > && initialization_kind == CPP_OPEN_PAREN) > if (cp_parser_attributes_opt (parser)) > warning (OPT_Wattributes, >"attributes after parenthesized initializer ignored"); > > Noisy deprecation warning time? Sure. Jason
Re: [PATCH, rs6000] Fix PR83789: __builtin_altivec_lvx fails for powerpc for altivec-4.c
On Mon, Mar 19, 2018 at 09:12:08PM -0500, Peter Bergner wrote: > On 3/12/18 1:55 PM, Segher Boessenkool wrote: > >> #ifdef HAVE_V8HFmode > >> - else if (mode == V8HFmode) > >> - stvx = TARGET_64BIT > >> -? gen_altivec_stvx_v8hf_1op (src_exp, memory_address) > >> -: gen_altivec_stvx_v8hf_1op_si (src_exp, memory_address); > >> + else if (mode == V8HFmode) > >> +stvx = gen_altivec_stvx_v8hf (src_exp, dest_exp); > >> #endif > > > > Btw, don't we always have V8HFmode these days? > > I have no idea, I was just working around code that was already there. > Looking at the history, Kelvin seems to have added the tests. > Kelvin, what is the above trying to protect? > > Looking at mu build dirs insn-modes.h, I don't see HAVE_V8HFmode being > defined on either my LE or BE builds. What am I missing? Nothing, I just was confused (we always define the mode in rs6000-modes.def, but that is not the same thing). Sorry. Segher
Re: [PATCH] S/390: Set ABI default based on uname
On 03/13/2018 04:53 PM, Michael Matz wrote: > Hi, > > On Tue, 13 Mar 2018, Andreas Krebbel wrote: > >> Leaving history aside don't you agree that it would have been more >> sensible to require a -m option only if you want to build for an ABI >> different from what is currently mandated by the personality setting? > > I agree with you. But we can't leave history aside, so I'm having the > same sentiment as Jakub. The current situation is not good, but changing > behaviour would be even worse. Ok. I'm not convinced that this change will actually hurt somewhere but having downvotes from you guys is enough to drop that patch. If I still have to build 32 bit stuff after your distros dropped 32 bit support entirely I'll probably come back with it ;) -Andreas-
Re: [PR c++/84962] ICE with anon-struct member
On Tue, Mar 20, 2018 at 9:55 AM, Nathan Sidwell wrote:
> This fixes the 84962 ICE, which is where we name a member function of an
> anon-struct in the width specifier of a bitfield. The problem is that when
> permissive we allow some of this strangeness. Prior to my lookup changes, we
> only added TYPE_FIELDS and we still do that. Trouble is that now contains
> member functions, and badness happens. The fix is to add the member_vec
> members, if that exists, which takes care of overloaded.
>
> Due to history, we don't check some of the anon-struct restrictions until
> the outer struct is complete. That's partly historical and partly because
> we don't tell the struct parser the difference between
>
> struct X {
>struct { ... }; // anon-struct
>typedef struct { ... } name; // not an anon-struct
Right, because we can't tell until after the struct:
struct { ... } member; // also not an anon-struct
> However, all this stuff is an extension:
> 1) we permit anon-struct, not just anon-union
> 2) we permit things other than non-static-data members
> Maybe we could deprecate #2 at least? I'm not sure what its use case might
> be.
Fine with me. I know #1 was deliberate, but I haven't been aware of #2 before.
Jason
Re: [RFC Patch], PowerPC memory support pre-gcc9, patch #1
On Fri, Mar 16, 2018 at 07:01:18PM -0400, Michael Meissner wrote: > In patch #4, I mentioned that the spec 2006 benchmark 'tonto' generated > different with the patches applied. I tracked it down, and it was due to the > call I inserted in rs6000_debug_reg_print to update the conditional register > usage seemed to set the Altivec registers VS0..VS19 to call_used instead of > call_saved. Since I no longer need to set the conditional register usage with > -mdebug=reg, it is simpler to just delete it. > > 2018-03-16 Michael Meissner > > * config/rs6000/rs6000.c (rs6000_debug_reg_print): Eliminate call > to rs6000_conditional_register_usage. Yes, debug output should *never* change *any* state. Could you fold this patch into the patch that created the problem please? Segher
Re: [RFC Patch], PowerPC memory support pre-gcc9, patch #4
Hi Mike,
On Fri, Mar 16, 2018 at 12:50:45PM -0400, Michael Meissner wrote:
> --- gcc/config/rs6000/rs6000-output.c (revision 258576)
> +++ gcc/config/rs6000/rs6000-output.c (working copy)
> @@ -162,7 +162,13 @@ rs6000_output_move_64bit (rtx operands[]
>
>/* Moves to SPRs. */
>else if (reg_is_spr_p (dest))
> - return "mt%0 %1";
> + {
> + if (src_gpr_p)
> + return "mt%0 %1";
> +
> + else if (dest_regno == src_regno)
> + return "nop";
> + }
> }
Is this correct? Many SPRs are not simple registers, they do something
when you write to them. But I guess this is only for lr,ctr,vrsave (i.e.
regclass "h", "SPECIAL_REGS"). So maybe we want a better name?
> +;; STD LD MR MT MF G-const
> +;; H-const F-const Special
> +
> (define_insn "*mov_softfloat64"
> - [(set (match_operand:FMOVE64 0 "nonimmediate_operand"
> "=Y,r,r,cl,r,r,r,r,*h")
> - (match_operand:FMOVE64 1 "input_operand" "r,Y,r,r,h,G,H,F,0"))]
> + [(set (match_operand:FMOVE64 0 "nonimmediate_operand"
> + "=Y, r, r, cl, r, r,
> + r, r, *h")
> +
> + (match_operand:FMOVE64 1 "input_operand"
> +"r, Y, r, r, h, G,
> + H, F, 0"))]
> +
>"TARGET_POWERPC64 && TARGET_SOFT_FLOAT
> - && (gpc_reg_operand (operands[0], mode)
> - || gpc_reg_operand (operands[1], mode))"
> - "@
> - std%U0%X0 %1,%0
> - ld%U1%X1 %0,%1
> - mr %0,%1
> - mt%0 %1
> - mf%1 %0
> - #
> - #
> - #
> - nop"
> - [(set_attr "type" "store,load,*,mtjmpr,mfjmpr,*,*,*,*")
> - (set_attr "length" "4,4,4,4,4,8,12,16,4")])
> + && rs6000_valid_move_p (operands[0], operands[1])"
> + "* return rs6000_output_move_64bit (operands);"
> + [(set_attr "type"
> +"store, load, *, mtjmpr, mfjmpr, *,
> + *, *, *")
> +
> + (set_attr "length"
> +"4, 4, 4, 4, 4, 8,
> + 12, 16, 4")])
Let's take this one as example. The attributes depend on which alternative
is selected, but with your change the actual output insn does not. That is
no good.
Maybe you can reduce the number of alternatives? Make it just store,
load, and moves for example, and then select the attributes based on what
machine insns you actually output? The ones that are split are the
problematic ones in that case, the rest is easy to handle.
Segher
Re: C++ PATCH for c++/84978, ICE with NRVO
On Tue, Mar 20, 2018 at 9:32 AM, Marek Polacek wrote: > We started crashing on this test with r258592 which added cp_get_callee_fndecl > in in cp_genericize_r. > > This ICE apparently depends on whether we perform NRVO or not. If the size of > S is <=16B we pass it in registers and it compiles fine. But if the size of S > is >16B, then we pass in memory, and we NRV-optimize. That means that > s.fn (); > is turned by finalize_nrv into > .fn (); > > Then the newly added call to cp_get_callee_fndecl calls maybe_constant_init, Oops, I forgot that cp_get_callee_fndecl would call maybe_constant_init, I was just using it to handle both CALL_EXPR and AGGR_INIT_EXPR. And in fact it looks like we don't really want that for the other users, either. I think I'll remove it. > 2018-03-20 Marek Polacek > > PR c++/84978 > * constexpr.c (cxx_eval_constant_expression): Handle the case when > a RESULT_DECL isn't in the hash map. But your patch is also good. OK. Jason
[PR c++/84970] lookup marking
This is another case of lookups escaping into template bodies, without
being marked. In this case an initializer list.
in the non-template case, we try and recycle these lookup nodes
immediately. But for templates we have to mark lookups, so we don't try
and mutate them via the binding. When the lookup involved via a using
directive we can detect the marking failure.
nathan
--
Nathan Sidwell
2018-03-20 Nathan Sidwell
PR c++/84970
* cp-tree.h (lookup_list_keep): Declare.
* tree.c (lookup_list_keep): New, broken out of ...
(build_min): ... here. Call it.
* decl.c (cp_finish_decl): Call lookup_list_keep.
PR c++/84970
* g++.dg/lookup/pr84970.C: New.
Index: cp/cp-tree.h
===
--- cp/cp-tree.h (revision 258676)
+++ cp/cp-tree.h (working copy)
@@ -7012,6 +7012,7 @@ extern tree lookup_add(tree fns, tre
extern tree lookup_maybe_add (tree fns, tree lookup,
bool deduping);
extern void lookup_keep(tree lookup, bool keep);
+extern void lookup_list_keep (tree list, bool keep);
extern int is_overloaded_fn (tree) ATTRIBUTE_PURE;
extern bool really_overloaded_fn (tree) ATTRIBUTE_PURE;
extern tree dependent_name (tree);
Index: cp/decl.c
===
--- cp/decl.c (revision 258676)
+++ cp/decl.c (working copy)
@@ -7034,7 +7034,11 @@ cp_finish_decl (tree decl, tree init, bo
}
if (init)
- DECL_INITIAL (decl) = init;
+ {
+ if (TREE_CODE (init) == TREE_LIST)
+ lookup_list_keep (init, true);
+ DECL_INITIAL (decl) = init;
+ }
if (dep_init)
{
retrofit_lang_decl (decl);
Index: cp/tree.c
===
--- cp/tree.c (revision 258676)
+++ cp/tree.c (working copy)
@@ -2436,6 +2436,20 @@ lookup_keep (tree lookup, bool keep)
ovl_used (lookup);
}
+/* LIST is a TREE_LIST whose TREE_VALUEs may be OVERLOADS that need
+ keeping, or may be ignored. */
+
+void
+lookup_list_keep (tree list, bool keep)
+{
+ for (; list; list = TREE_CHAIN (list))
+{
+ tree v = TREE_VALUE (list);
+ if (TREE_CODE (v) == OVERLOAD)
+ lookup_keep (v, keep);
+}
+}
+
/* Returns nonzero if X is an expression for a (possibly overloaded)
function. If "f" is a function or function template, "f", "c->f",
"c.f", "C::f", and "f" will all be considered possibly
@@ -3315,9 +3329,7 @@ build_min (enum tree_code code, tree tt,
if (code == CAST_EXPR)
/* The single operand is a TREE_LIST, which we have to check. */
-for (tree v = TREE_OPERAND (t, 0); v; v = TREE_CHAIN (v))
- if (TREE_CODE (TREE_VALUE (v)) == OVERLOAD)
- lookup_keep (TREE_VALUE (v), true);
+lookup_list_keep (TREE_OPERAND (t, 0), true);
return t;
}
Index: testsuite/g++.dg/lookup/pr84970.C
===
--- testsuite/g++.dg/lookup/pr84970.C (revision 0)
+++ testsuite/g++.dg/lookup/pr84970.C (working copy)
@@ -0,0 +1,21 @@
+// PR c++/84970 ICE with deferred initializer
+
+namespace bob {
+ void a();
+}
+using namespace bob;
+
+void a (int);
+
+template
+void *x (b)
+{
+ void (*c)(b) (a);
+
+ return (void *)c;
+}
+
+void d () {
+ x (1);
+}
+
Re: [PATCH] Fix typos (PR other/84819).
On 03/20/2018 04:28 PM, Gerald Pfeifer wrote: > On Tue, 20 Mar 2018, Martin Liška wrote: >> There's patch for couple of documentation issues reported in the PR. > > Looks good to me, thanks! > > Just, should there be an "a" in "use jump table"? Looks good. And 'an indirect jump.' ? Martin > > Gerald >
Re: [PATCH] [Microblaze]: PIC Data Text Relative
Hi Andrew --
I just do a visual check. The script can help.
On 03/20/2018 07:13 AM, Andrew Sadek wrote:
Many Thanks Michael for the updates .. I was actually looking for something
similar for my test case .. I m currently revising the Gnu code conventions
on the patches then will send them again.
I m actually running the 'check_GNU_Style' in contrib folder,, is this the
correct way ? Is it enough ?
Andrew
On Tue, Mar 20, 2018, 03:30 Michael Eager wrote:
Also check the { dg-skip-if } directive.
https://gcc.gnu.org/onlinedocs/gccint/Directives.html
On 03/19/2018 06:14 PM, Michael Eager wrote:
Hi Andrew --
Please take a look at the test case description:
https://gcc.gnu.org/wiki/HowToPrepareATestcase
and see if you can do one of the following:
- Modify the regex expression in the scan-assembler to accept
either format of generated output
or
- Add { dg-option } directives to turn off your new options
if specified. (You should be able to specify -mno-pic)
or
- Duplicate the test cases and add { dg-option } directives
to specify the correct options, with and without your
new options.
or
- Add test cases with a { dg-option } directive with your
new options.
This is not required -- your patch appears to work OK. Normally,
the new PIC Data options would not be used when running the test
suite, so the tests would not fail. It's just nice to have the
test suite updated when new options are added.
On 03/19/2018 01:07 PM, Michael Eager wrote:
Hi Andrew --
Good work.
Please submit your updated patch. Check that you follow
GNU coding standards. Also make sure that the new options
are documented in gcc/doc/invoke.texi.
On 03/18/18 03:27, Andrew Sadek wrote:
Hello Michael,
I have run the test using the new PIC options.
Actually, I have discovered 2 unhandled cases in
'microblaze_expand_move' + missing conditions in linker relax
leading some test cases execution to fail.
After fixing them, I made a re-run for the whole regression, and the
results analogy below:
Original, without my patch:
=== gcc Summary ===
# of expected passes90776
# of unexpected failures1317
# of unexpected successes3
# of expected failures207
# of unresolved testcases115
# of unsupported tests2828
With my patch, calling '-fPIE - mpic-data-text-rel'
=== gcc Summary ===
# of expected passes90843
# of unexpected failures1256
# of unexpected successes3
# of expected failures207
# of unresolved testcases115
# of unsupported tests2853
After running the 'dg-cmp-results.sh' in contrib folder, the
PASS->FAIL are below:
PASS->FAIL: gcc.dg/uninit-19.c (test for excess errors)
PASS->FAIL: gcc.target/microblaze/others/data_var1.c -O0
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var1.c -O1
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var1.c -O2 -flto
-fno-use-linker-plugin -flto-partition=none scan-assembler
lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var1.c -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects scan-assembler
lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var1.c -O2
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var1.c -O3 -g
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var1.c -Os
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var2.c -O0
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var2.c -O1
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var2.c -O2 -flto
-fno-use-linker-plugin -flto-partition=none scan-assembler
lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var2.c -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects scan-assembler
lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var2.c -O2
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var2.c -O3 -g
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var2.c -Os
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/sdata_var1.c -O0
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13
PASS->FAIL: gcc.target/microblaze/others/sdata_var1.c -O1
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13
PASS->FAIL: gcc.target/microblaze/others/sdata_var1.c -O2 -flto
-fno-use-linker-plugin -flto-partition=none scan-assembler
lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13
PASS->FAIL: gcc.target/microblaze/others/sdata_var1.c -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects scan-assembler
lwi\tr([0-9]|[1-2
Re: [PATCH] Fix typos (PR other/84819).
On Tue, 20 Mar 2018, Martin Liška wrote: > There's patch for couple of documentation issues reported in the PR. Looks good to me, thanks! Just, should there be an "a" in "use jump table"? Geralddiff --git a/gcc/calls.c b/gcc/calls.c index 4dcfef77a5a..9eb0467311b 100644 --- a/gcc/calls.c +++ b/gcc/calls.c @@ -1999,7 +1999,7 @@ initialize_argument_information (int num_actuals ATTRIBUTE_UNUSED, *may_tailcall = false; maybe_complain_about_tail_call (exp, "a callee-copied argument is" - " stored in the current " + " stored in the current" " function's frame"); } diff --git a/gcc/common.opt b/gcc/common.opt index e0bc4d1bb18..d6ef85928f3 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -1002,7 +1002,7 @@ Select what to sanitize. fsanitize-coverage= Common Report Joined -Select what to coverage sanitize. +Select type of coverage sanitization. fasan-shadow-offset= Common Joined RejectNegative Var(common_deferred_options) Defer @@ -1637,7 +1637,7 @@ Common Report Joined RejectNegative Enum(cf_protection_level) Var(flag_cf_protec instructions have valid targets. Enum -Name(cf_protection_level) Type(enum cf_protection_level) UnknownError(unknown Cotrol-Flow Protection Level %qs) +Name(cf_protection_level) Type(enum cf_protection_level) UnknownError(unknown Control-Flow Protection Level %qs) EnumValue Enum(cf_protection_level) String(full) Value(CF_FULL) diff --git a/gcc/config/i386/i386.opt b/gcc/config/i386/i386.opt index 311b99d9739..9482b7b52db 100644 --- a/gcc/config/i386/i386.opt +++ b/gcc/config/i386/i386.opt @@ -1023,7 +1023,7 @@ Enforcement Technology (CET). mcet-switch Target Report Undocumented Var(flag_cet_switch) Init(0) -Turn on CET instrumentation for switch statements, which use jump table and +Turn on CET instrumentation for switch statements that use jump table and indirect jump. mforce-indirect-call
Re: Use SCEV information when aligning for vectorisation (PR 84005)
Richard Biener writes:
> On Mon, Mar 19, 2018 at 10:27 PM, Richard Sandiford
> wrote:
>> Richard Biener writes:
>>> On Sat, Mar 17, 2018 at 11:45 AM, Richard Sandiford
>>> wrote:
Index: gcc/tree-data-ref.c
===
--- gcc/tree-data-ref.c 2018-02-14 13:14:36.268006810 +
+++ gcc/tree-data-ref.c 2018-03-17 10:43:42.544669549 +
@@ -5200,6 +5200,75 @@ dr_alignment (innermost_loop_behavior *d
return alignment;
}
+/* If BASE is a pointer-typed SSA name, try to find the object that it
+ is based on. Return this object X on success and store the alignment
+ in bytes of BASE - &X in *ALIGNMENT_OUT. */
+
+static tree
+get_base_for_alignment_1 (tree base, unsigned int *alignment_out)
+{
+ if (TREE_CODE (base) != SSA_NAME || !POINTER_TYPE_P (TREE_TYPE (base)))
+return NULL_TREE;
+
+ gimple *def = SSA_NAME_DEF_STMT (base);
+ base = analyze_scalar_evolution (loop_containing_stmt (def), base);
>>>
>>> I think it is an error to use the def stmt of base here. Instead you
>>> need to pass down the point/loop of the use. For the same reason you
>>> also want to instantiate parameters after analyzing the evolution.
>>>
>>> In the end, if the BB to be vectorized is contained in a loop nest we want
>>> to
>>> instantiate up to the point where (eventually) a DECL we can re-align
>>> appears,
>>> but using the containing loop for now looks OK.
>>
>> Why's that necessary/better though? We're not interested in the
>> evolution of the value at the point it*s used by the potential vector
>> code, but in how we get from the ultimate base (if there is one) to the
>> definition of the SSA name.
>
> Hmm, ok.
>
>> I don't think instantiating the SCEV would give any new information,
>> but it could lose some. E.g. if we have:
>>
>> x = &foo;
>> do
>> x += 8;
>> while (*y++ < 10)
>> ...potential vector use of *x...
>>
>> we wouldn't be able to express the address of x after the do-while
>> loop, because there's nothing that counts the number of iterations.
>> But the uninstantiated SCEV could tell us that x = &foo + N * 8 for
>> some (unknown) N.
>
> Not sure that it works that way. I'm not 100% sure what kind of
> parameters are left symbolic, and if analysis loop and instantiation
> "loop" are the same I guess it doesn't make any difference...
>
>> (OK, so that doesn't actually work unless we take the effort
>> to look through loop-closed SSA form, but still...)
>>
+ /* Peel chrecs and record the minimum alignment preserved by
+ all steps. */
+ unsigned int alignment = MAX_OFILE_ALIGNMENT / BITS_PER_UNIT;
+ while (TREE_CODE (base) == POLYNOMIAL_CHREC)
+{
+ unsigned int step_alignment = highest_pow2_factor (CHREC_RIGHT
(base));
+ alignment = MIN (alignment, step_alignment);
+ base = CHREC_LEFT (base);
+}
+
+ /* Punt if the expression is too complicated to handle. */
+ if (tree_contains_chrecs (base, NULL) || !POINTER_TYPE_P (TREE_TYPE
(base)))
+return NULL_TREE;
+
+ /* Analyze the base to which the steps we peeled were applied. */
+ poly_int64 bitsize, bitpos, bytepos;
+ machine_mode mode;
+ int unsignedp, reversep, volatilep;
+ tree offset;
+ base = get_inner_reference (build_fold_indirect_ref (base),
>>>
>>> ick :/
>>>
>>> what happens if you simply use get_pointer_alignment here and
>>> strip down POINTER_PLUS_EXPRs to the ultimate LHS? (basically
>>> what get_pointer_alignment_1 does) After all get_base_for_alignment_1
>>> itself only deals with plain SSA_NAMEs, not, say, &a_1->b.c or &MEM[a_1 +
>>> 4].
>>
>> Yeah, but those have (hopefully) been handled by dr_analyze_innermost
>> already, which should have stripped off both the constant and variable
>> parts of the offset. So I think SSA names are the only interesting
>> input. The problem is that once we follow the definitions of an SSA
>> name through CHREC_LEFTs, we get a general address again.
>>
>> get_pointer_alignment and get_pointer_alignment_1 don't do what we want
>> because they take the alignment of the existing object into account,
>> whereas here we explicitly want to ignore that and only calculate the
>> alignment of the offset.
>
> Ah, indeed - I missed that fact.
>
>> I guess we could pass a flag down to ignore the current alignment though?
>
> But we need the base anyway... so, given we can only ever re-align decls
> and never plain pointers instead of using build_fold_indirect_ref do
>
> if (TREE_CODE (base) != ADDR_EXPR)
>return NULL_TREE;
>
> else use TREE_OPERAND (base, 0)?
The build_fold_indirect_ref also helps for POINTER_PLUS_EXPR,
which is what we get for things like the do-while above (e.g.
{ &a + 64, +, 64 }_n if x points to a double *.)
I guess everything we care about would be handled by fold_i
Re: PING: [PATCH] x86: Force __x86_indirect_thunk_reg for function call via GOT
On Tue, Mar 20, 2018 at 7:48 AM, Jan Hubicka wrote: >> On Tue, Feb 27, 2018 at 11:39 AM, H.J. Lu wrote: >> > For x86 targets, when -fno-plt is used, external functions are called >> > via GOT slot, in 64-bit mode: >> > >> > [bnd] call/jmp *foo@GOTPCREL(%rip) >> > >> > and in 32-bit mode: >> > >> > [bnd] call/jmp *foo@GOT[(%reg)] >> > >> > With -mindirect-branch=, they are converted to, in 64-bit mode: >> > >> > pushq foo@GOTPCREL(%rip) >> > [bnd] jmp __x86_indirect_thunk[_bnd] >> > >> > and in 32-bit mode: >> > >> > pushl foo@GOT[(%reg)] >> > [bnd] jmp __x86_indirect_thunk[_bnd] >> > >> > which were incompatible with CFI. In 64-bit mode, since R11 is a scratch >> > register, we generate: >> > >> > movq foo@GOTPCREL(%rip), %r11 >> > [bnd] call/jmp __x86_indirect_thunk_[bnd_]r11 >> > >> > instead. We do it in ix86_output_indirect_branch so that we can use >> > the newly proposed R_X86_64_THUNK_GOTPCRELX relocation: >> > >> > https://groups.google.com/forum/#!topic/x86-64-abi/eED5lzn3_Mg >> > >> > movq foo@OTPCREL_THUNK(%rip), %r11 >> > [bnd] call/jmp __x86_indirect_thunk_[bnd_]r11 >> > >> > to load GOT slot into R11. If foo is defined locally, linker can can >> > convert >> > >> > movq foo@GOTPCREL_THUNK(%rip), %reg >> > call/jmp __x86_indirect_thunk_reg >> > >> > to >> > >> > call/jmp foo >> > nop0L(%rax) >> > >> > In 32-bit mode, since all caller-saved registers, EAX, EDX and ECX, may >> > used to function parameters, there is no scratch register available. For >> > -fno-plt -fno-pic -mindirect-branch=, we expand external function call >> > to: >> > >> > movl foo@GOT, %reg >> > [bnd] call/jmp *%reg >> > >> > so that it can be converted to >> > >> > movl foo@GOT, %reg >> > [bnd] call/jmp __x86_indirect_thunk_[bnd_]reg >> > >> > in ix86_output_indirect_branch. Since this is performed during RTL >> > expansion, other instructions may be inserted between movl and call/jmp. >> > Linker optimization isn't always possible. >> > >> > Tested on i686 and x86-64. OK for trunk? >> > >> > >> > H.J. >> > --- >> > gcc/ >> > >> > PR target/83970 >> > * config/i386/constraints.md (Bs): Allow GOT_memory_operand >> > for TARGET_LP64 with indirect branch conversion. >> > (Bw): Likewise. >> > * config/i386/i386.c (ix86_expand_call): Handle -fno-plt with >> > -mindirect-branch=. >> > (ix86_nopic_noplt_attribute_p): Likewise. >> > (ix86_output_indirect_branch): In 64-bit mode, convert function >> > call via GOT with R11 as a scratch register using >> > __x86_indirect_thunk_r11. >> > (ix86_output_call_insn): In 64-bit mode, set xasm to NULL when >> > calling ix86_output_indirect_branch with function call via GOT. >> > * config/i386/i386.md (*call_got_thunk): New call pattern for >> > TARGET_LP64 with indirect branch conversion. >> > (*call_value_got_thunk): Likewise. >> > >> > gcc/testsuite/ >> > >> > PR target/83970 >> > * gcc.target/i386/indirect-thunk-5.c: Updated. >> > * gcc.target/i386/indirect-thunk-6.c: Likewise. >> > * gcc.target/i386/indirect-thunk-bnd-3.c: Likewise. >> > * gcc.target/i386/indirect-thunk-bnd-4.c: Likewise. >> > * gcc.target/i386/indirect-thunk-extern-5.c: Likewise. >> > * gcc.target/i386/indirect-thunk-extern-6.c: Likewise. >> > * gcc.target/i386/indirect-thunk-inline-5.c: Likewise. >> > * gcc.target/i386/indirect-thunk-inline-6.c: Likewise. >> > * gcc.target/i386/indirect-thunk-13.c: New test. >> > * gcc.target/i386/indirect-thunk-14.c: Likewise. >> > * gcc.target/i386/indirect-thunk-bnd-5.c: Likewise. >> > * gcc.target/i386/indirect-thunk-bnd-6.c: Likewise. >> > * gcc.target/i386/indirect-thunk-extern-11.c: Likewise. >> > * gcc.target/i386/indirect-thunk-extern-12.c: Likewise. >> > * gcc.target/i386/indirect-thunk-inline-8.c: Likewise. >> > * gcc.target/i386/indirect-thunk-inline-9.c: Likewise. > > Patch is OK. I am just bit worried how many additional features we will need > relatively late in stage4. My understanding is that at the moment there are no I will punt it for GCC 9 and add new call patterns with a scratch register based on your previous feedback: https://gcc.gnu.org/ml/gcc-patches/2018-03/msg00766.html > direct plans to retpoline userland, but I see that it may change in future. > Can you give us bit of review if there are still some missing parts? This is the only one missing. I do have a proposal to use retpoline with CET in user space such that a single binary will support both retpoline and CET. It requires linker, GCC and glibc changes. But I don't know if there is a demand fo
Re: PING: [PATCH] x86: Force __x86_indirect_thunk_reg for function call via GOT
> On Tue, Feb 27, 2018 at 11:39 AM, H.J. Lu wrote: > > For x86 targets, when -fno-plt is used, external functions are called > > via GOT slot, in 64-bit mode: > > > > [bnd] call/jmp *foo@GOTPCREL(%rip) > > > > and in 32-bit mode: > > > > [bnd] call/jmp *foo@GOT[(%reg)] > > > > With -mindirect-branch=, they are converted to, in 64-bit mode: > > > > pushq foo@GOTPCREL(%rip) > > [bnd] jmp __x86_indirect_thunk[_bnd] > > > > and in 32-bit mode: > > > > pushl foo@GOT[(%reg)] > > [bnd] jmp __x86_indirect_thunk[_bnd] > > > > which were incompatible with CFI. In 64-bit mode, since R11 is a scratch > > register, we generate: > > > > movq foo@GOTPCREL(%rip), %r11 > > [bnd] call/jmp __x86_indirect_thunk_[bnd_]r11 > > > > instead. We do it in ix86_output_indirect_branch so that we can use > > the newly proposed R_X86_64_THUNK_GOTPCRELX relocation: > > > > https://groups.google.com/forum/#!topic/x86-64-abi/eED5lzn3_Mg > > > > movq foo@OTPCREL_THUNK(%rip), %r11 > > [bnd] call/jmp __x86_indirect_thunk_[bnd_]r11 > > > > to load GOT slot into R11. If foo is defined locally, linker can can > > convert > > > > movq foo@GOTPCREL_THUNK(%rip), %reg > > call/jmp __x86_indirect_thunk_reg > > > > to > > > > call/jmp foo > > nop0L(%rax) > > > > In 32-bit mode, since all caller-saved registers, EAX, EDX and ECX, may > > used to function parameters, there is no scratch register available. For > > -fno-plt -fno-pic -mindirect-branch=, we expand external function call > > to: > > > > movl foo@GOT, %reg > > [bnd] call/jmp *%reg > > > > so that it can be converted to > > > > movl foo@GOT, %reg > > [bnd] call/jmp __x86_indirect_thunk_[bnd_]reg > > > > in ix86_output_indirect_branch. Since this is performed during RTL > > expansion, other instructions may be inserted between movl and call/jmp. > > Linker optimization isn't always possible. > > > > Tested on i686 and x86-64. OK for trunk? > > > > > > H.J. > > --- > > gcc/ > > > > PR target/83970 > > * config/i386/constraints.md (Bs): Allow GOT_memory_operand > > for TARGET_LP64 with indirect branch conversion. > > (Bw): Likewise. > > * config/i386/i386.c (ix86_expand_call): Handle -fno-plt with > > -mindirect-branch=. > > (ix86_nopic_noplt_attribute_p): Likewise. > > (ix86_output_indirect_branch): In 64-bit mode, convert function > > call via GOT with R11 as a scratch register using > > __x86_indirect_thunk_r11. > > (ix86_output_call_insn): In 64-bit mode, set xasm to NULL when > > calling ix86_output_indirect_branch with function call via GOT. > > * config/i386/i386.md (*call_got_thunk): New call pattern for > > TARGET_LP64 with indirect branch conversion. > > (*call_value_got_thunk): Likewise. > > > > gcc/testsuite/ > > > > PR target/83970 > > * gcc.target/i386/indirect-thunk-5.c: Updated. > > * gcc.target/i386/indirect-thunk-6.c: Likewise. > > * gcc.target/i386/indirect-thunk-bnd-3.c: Likewise. > > * gcc.target/i386/indirect-thunk-bnd-4.c: Likewise. > > * gcc.target/i386/indirect-thunk-extern-5.c: Likewise. > > * gcc.target/i386/indirect-thunk-extern-6.c: Likewise. > > * gcc.target/i386/indirect-thunk-inline-5.c: Likewise. > > * gcc.target/i386/indirect-thunk-inline-6.c: Likewise. > > * gcc.target/i386/indirect-thunk-13.c: New test. > > * gcc.target/i386/indirect-thunk-14.c: Likewise. > > * gcc.target/i386/indirect-thunk-bnd-5.c: Likewise. > > * gcc.target/i386/indirect-thunk-bnd-6.c: Likewise. > > * gcc.target/i386/indirect-thunk-extern-11.c: Likewise. > > * gcc.target/i386/indirect-thunk-extern-12.c: Likewise. > > * gcc.target/i386/indirect-thunk-inline-8.c: Likewise. > > * gcc.target/i386/indirect-thunk-inline-9.c: Likewise. Patch is OK. I am just bit worried how many additional features we will need relatively late in stage4. My understanding is that at the moment there are no direct plans to retpoline userland, but I see that it may change in future. Can you give us bit of review if there are still some missing parts? Thanks, Honza
deprecate ignored attributes?
Jason, in parser.c /* The old parser allows attributes to appear after a parenthesized initializer. Mark Mitchell proposed removing this functionality on the GCC mailing lists on 2002-08-13. This parser accepts the attributes -- but ignores them. */ if (cp_parser_allow_gnu_extensions_p (parser) && initialization_kind == CPP_OPEN_PAREN) if (cp_parser_attributes_opt (parser)) warning (OPT_Wattributes, "attributes after parenthesized initializer ignored"); Noisy deprecation warning time? nathan -- Nathan Sidwell
Re: [PATCH] Fix PR84859
On Tue, Mar 20, 2018 at 01:42:04PM +0100, Richard Biener wrote:
> 2018-03-20 Richard Biener
>
> * testsuite/libgomp.graphite/force-parallel-4.c: XFAIL one
> parallelizable loop.
>
> Index: libgomp/testsuite/libgomp.graphite/force-parallel-4.c
> ===
> --- libgomp/testsuite/libgomp.graphite/force-parallel-4.c (revision
> 258678)
> +++ libgomp/testsuite/libgomp.graphite/force-parallel-4.c (working copy)
> @@ -46,7 +46,10 @@ int main(void)
>return 0;
> }
>
> -/* Check that parallel code generation part make the right answer. */
> -/* { dg-final { scan-tree-dump-times "2 loops carried no dependency" 1
> "graphite" } } */
> +/* Check that parallel code generation part make the right answer.
> + ??? XFAILed for i1 because conditional store elimination wrecks
> + our dependence representation. */
> +/* { dg-final { scan-tree-dump-times "2 loops carried no dependency" 1
> "graphite" { xfail *-*-* } } } */
> +/* { dg-final { scan-tree-dump-times "1 loops carried no dependency" 1
> "graphite" } } */
Shouldn't this line be then "\[12] loops carried no dependency" 1 "graphite" }
} */
so that when the previous starts XPASSing, we don't actually get a new FAIL?
> /* { dg-final { scan-tree-dump-times "loopfn.0" 4 "optimized" } } */
> /* { dg-final { scan-tree-dump-times "loopfn.1" 4 "optimized" } } */
Jakub
[PATCH] Fix typos (PR other/84819).
Hi. There's patch for couple of documentation issues reported in the PR. Ready for trunk? Thanks, Martin gcc/ChangeLog: 2018-03-20 Martin Liska PR other/84819 * calls.c (initialize_argument_information): Fix trailing space. * common.opt: Fix typo and provide better explanation for -fsanitize-coverage option. * config/i386/i386.opt: Fix typo. --- gcc/calls.c | 2 +- gcc/common.opt | 4 ++-- gcc/config/i386/i386.opt | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/gcc/calls.c b/gcc/calls.c index 4dcfef77a5a..9eb0467311b 100644 --- a/gcc/calls.c +++ b/gcc/calls.c @@ -1999,7 +1999,7 @@ initialize_argument_information (int num_actuals ATTRIBUTE_UNUSED, *may_tailcall = false; maybe_complain_about_tail_call (exp, "a callee-copied argument is" - " stored in the current " + " stored in the current" " function's frame"); } diff --git a/gcc/common.opt b/gcc/common.opt index e0bc4d1bb18..d6ef85928f3 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -1002,7 +1002,7 @@ Select what to sanitize. fsanitize-coverage= Common Report Joined -Select what to coverage sanitize. +Select type of coverage sanitization. fasan-shadow-offset= Common Joined RejectNegative Var(common_deferred_options) Defer @@ -1637,7 +1637,7 @@ Common Report Joined RejectNegative Enum(cf_protection_level) Var(flag_cf_protec instructions have valid targets. Enum -Name(cf_protection_level) Type(enum cf_protection_level) UnknownError(unknown Cotrol-Flow Protection Level %qs) +Name(cf_protection_level) Type(enum cf_protection_level) UnknownError(unknown Control-Flow Protection Level %qs) EnumValue Enum(cf_protection_level) String(full) Value(CF_FULL) diff --git a/gcc/config/i386/i386.opt b/gcc/config/i386/i386.opt index 311b99d9739..9482b7b52db 100644 --- a/gcc/config/i386/i386.opt +++ b/gcc/config/i386/i386.opt @@ -1023,7 +1023,7 @@ Enforcement Technology (CET). mcet-switch Target Report Undocumented Var(flag_cet_switch) Init(0) -Turn on CET instrumentation for switch statements, which use jump table and +Turn on CET instrumentation for switch statements that use jump table and indirect jump. mforce-indirect-call
Re: [PATCH] [Microblaze]: PIC Data Text Relative
Many Thanks Michael for the updates .. I was actually looking for something
similar for my test case .. I m currently revising the Gnu code conventions
on the patches then will send them again.
I m actually running the 'check_GNU_Style' in contrib folder,, is this the
correct way ? Is it enough ?
Andrew
On Tue, Mar 20, 2018, 03:30 Michael Eager wrote:
> Also check the { dg-skip-if } directive.
> https://gcc.gnu.org/onlinedocs/gccint/Directives.html
>
> On 03/19/2018 06:14 PM, Michael Eager wrote:
> > Hi Andrew --
> >
> > Please take a look at the test case description:
> > https://gcc.gnu.org/wiki/HowToPrepareATestcase
> > and see if you can do one of the following:
> >- Modify the regex expression in the scan-assembler to accept
> > either format of generated output
> > or
> >- Add { dg-option } directives to turn off your new options
> > if specified. (You should be able to specify -mno-pic)
> > or
> >- Duplicate the test cases and add { dg-option } directives
> > to specify the correct options, with and without your
> > new options.
> > or
> >- Add test cases with a { dg-option } directive with your
> > new options.
> >
> > This is not required -- your patch appears to work OK. Normally,
> > the new PIC Data options would not be used when running the test
> > suite, so the tests would not fail. It's just nice to have the
> > test suite updated when new options are added.
> >
> > On 03/19/2018 01:07 PM, Michael Eager wrote:
> >> Hi Andrew --
> >>
> >> Good work.
> >>
> >> Please submit your updated patch. Check that you follow
> >> GNU coding standards. Also make sure that the new options
> >> are documented in gcc/doc/invoke.texi.
> >>
> >> On 03/18/18 03:27, Andrew Sadek wrote:
> >>> Hello Michael,
> >>>
> >>> I have run the test using the new PIC options.
> >>> Actually, I have discovered 2 unhandled cases in
> >>> 'microblaze_expand_move' + missing conditions in linker relax
> >>> leading some test cases execution to fail.
> >>> After fixing them, I made a re-run for the whole regression, and the
> >>> results analogy below:
> >>>
> >>> Original, without my patch:
> >>> === gcc Summary ===
> >>>
> >>> # of expected passes90776
> >>> # of unexpected failures1317
> >>> # of unexpected successes3
> >>> # of expected failures207
> >>> # of unresolved testcases115
> >>> # of unsupported tests2828
> >>>
> >>> With my patch, calling '-fPIE - mpic-data-text-rel'
> >>> === gcc Summary ===
> >>>
> >>> # of expected passes90843
> >>> # of unexpected failures1256
> >>> # of unexpected successes3
> >>> # of expected failures207
> >>> # of unresolved testcases115
> >>> # of unsupported tests2853
> >>>
> >>> After running the 'dg-cmp-results.sh' in contrib folder, the
> >>> PASS->FAIL are below:
> >>>
> >>> PASS->FAIL: gcc.dg/uninit-19.c (test for excess errors)
> >>> PASS->FAIL: gcc.target/microblaze/others/data_var1.c -O0
> >>> scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
> >>> PASS->FAIL: gcc.target/microblaze/others/data_var1.c -O1
> >>> scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
> >>> PASS->FAIL: gcc.target/microblaze/others/data_var1.c -O2 -flto
> >>> -fno-use-linker-plugin -flto-partition=none scan-assembler
> >>> lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
> >>> PASS->FAIL: gcc.target/microblaze/others/data_var1.c -O2 -flto
> >>> -fuse-linker-plugin -fno-fat-lto-objects scan-assembler
> >>> lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
> >>> PASS->FAIL: gcc.target/microblaze/others/data_var1.c -O2
> >>> scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
> >>> PASS->FAIL: gcc.target/microblaze/others/data_var1.c -O3 -g
> >>> scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
> >>> PASS->FAIL: gcc.target/microblaze/others/data_var1.c -Os
> >>> scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
> >>> PASS->FAIL: gcc.target/microblaze/others/data_var2.c -O0
> >>> scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
> >>> PASS->FAIL: gcc.target/microblaze/others/data_var2.c -O1
> >>> scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
> >>> PASS->FAIL: gcc.target/microblaze/others/data_var2.c -O2 -flto
> >>> -fno-use-linker-plugin -flto-partition=none scan-assembler
> >>> lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
> >>> PASS->FAIL: gcc.target/microblaze/others/data_var2.c -O2 -flto
> >>> -fuse-linker-plugin -fno-fat-lto-objects scan-assembler
> >>> lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
> >>> PASS->FAIL: gcc.target/microblaze/others/data_var2.c -O2
> >>> scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
> >>> PASS->FAIL: gcc.target/microblaze/others/data_var2.c -O3 -g
> >>> scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
> >>> PASS->FAIL: gcc.target/microblaze/others/data_var2.c -Os
> >>> scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
> >>> PASS->FAIL: gcc.target/microblaze/others/sdata_var1.c -O0
> >>> scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13
>
Re: [PATCH] Handle -fno-guess-branch-probability properly in predict.c (PR ipa/84825).
> On 03/13/2018 01:13 PM, Martin Liška wrote: > > Hi. > > > > This is a fix for situation where we use -fno-guess-branch-probability and > > fnsplit happens. > > > > Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. > > > > Ready for trunk? > > Thanks, > > Martin > > > > gcc/ChangeLog: > > > > 2018-03-13 Martin Liska > > > > PR ipa/84825 > > * predict.c (rebuild_frequencies): Handle case when we have > > PROFILE_ABSENT, but flag_guess_branch_prob is false. > > > > gcc/testsuite/ChangeLog: > > > > 2018-03-13 Martin Liska > > > > * g++.dg/ipa/pr84825.C: New test. > > --- > > gcc/predict.c | 3 +++ > > gcc/testsuite/g++.dg/ipa/pr84825.C | 18 ++ > > 2 files changed, 21 insertions(+) > > create mode 100644 gcc/testsuite/g++.dg/ipa/pr84825.C > > > > > > Honza asked about a place where a BB count is set up. It's here: > > (gdb) bt > #0 init_lowered_empty_function (decl=0x7692a700, in_ssa=true, count=...) > at ../../gcc/cgraphunit.c:1606 > #1 0x00cddde1 in cgraph_node::expand_thunk (this=0x7692f2e0, > output_asm_thunks=false, force_gimple_thunk=false) at > ../../gcc/cgraphunit.c:1856 > #2 0x00cd9e01 in cgraph_node::analyze (this=0x7692f2e0) at > ../../gcc/cgraphunit.c:635 > #3 0x00cdb85c in analyze_functions (first_time=true) at > ../../gcc/cgraphunit.c:1131 > #4 0x00ce0510 in symbol_table::finalize_compilation_unit > (this=0x767b0100) at ../../gcc/cgraphunit.c:2691 > #5 0x012908d8 in compile_file () at ../../gcc/toplev.c:480 > #6 0x012931fd in do_compile () at ../../gcc/toplev.c:2132 > #7 0x012934ea in toplev::main (this=0x7fffd7de, argc=24, > argv=0x7fffd8d8) at ../../gcc/toplev.c:2267 > #8 0x01f69194 in main (argc=24, argv=0x7fffd8d8) at > ../../gcc/main.c:39 > > (gdb) p count.debug() > 1 (estimated locally) > $7 = void I see, expand_thunk guesses even when guessing is disabled and we later inline it and that enables ipa-split. Hmm, I guess patch is OK and I will look into cleaning this up next stage1. Honza > > Martin
[PR c++/84962] ICE with anon-struct member
This fixes the 84962 ICE, which is where we name a member function of an
anon-struct in the width specifier of a bitfield. The problem is that
when permissive we allow some of this strangeness. Prior to my lookup
changes, we only added TYPE_FIELDS and we still do that. Trouble is
that now contains member functions, and badness happens. The fix is to
add the member_vec members, if that exists, which takes care of overloaded.
Due to history, we don't check some of the anon-struct restrictions
until the outer struct is complete. That's partly historical and partly
because we don't tell the struct parser the difference between
struct X {
struct { ... }; // anon-struct
typedef struct { ... } name; // not an anon-struct
};
It'd be nice to fix that, but not right now.
However, all this stuff is an extension:
1) we permit anon-struct, not just anon-union
2) we permit things other than non-static-data members
Maybe we could deprecate #2 at least? I'm not sure what its use case
might be.
nathan
--
Nathan Sidwell
2018-03-20 Nathan Sidwell
PR c++/84962
* name-lookup.c (pushdecl_class_level): Push anon-struct's
member_vec, if there is one.
PR c++/84962
* g++.dg/lookup/pr84962.C: New.
Index: cp/name-lookup.c
===
--- cp/name-lookup.c (revision 258676)
+++ cp/name-lookup.c (working copy)
@@ -4490,16 +4490,30 @@ pushdecl_class_level (tree x)
/* If X is an anonymous aggregate, all of its members are
treated as if they were members of the class containing the
aggregate, for naming purposes. */
- tree f;
-
- for (f = TYPE_FIELDS (TREE_TYPE (x)); f; f = DECL_CHAIN (f))
- {
- location_t save_location = input_location;
- input_location = DECL_SOURCE_LOCATION (f);
- if (!pushdecl_class_level (f))
- is_valid = false;
- input_location = save_location;
+ location_t save_location = input_location;
+ tree anon = TREE_TYPE (x);
+ if (vec *member_vec = CLASSTYPE_MEMBER_VEC (anon))
+ for (unsigned ix = member_vec->length (); ix--;)
+ {
+ tree binding = (*member_vec)[ix];
+ if (STAT_HACK_P (binding))
+ {
+ if (!pushdecl_class_level (STAT_TYPE (binding)))
+ is_valid = false;
+ binding = STAT_DECL (binding);
+ }
+ if (!pushdecl_class_level (binding))
+ is_valid = false;
}
+ else
+ for (tree f = TYPE_FIELDS (anon); f; f = DECL_CHAIN (f))
+ if (TREE_CODE (f) == FIELD_DECL)
+ {
+ input_location = DECL_SOURCE_LOCATION (f);
+ if (!pushdecl_class_level (f))
+ is_valid = false;
+ }
+ input_location = save_location;
}
timevar_cond_stop (TV_NAME_LOOKUP, subtime);
return is_valid;
Index: testsuite/g++.dg/lookup/pr84962.C
===
--- testsuite/g++.dg/lookup/pr84962.C (revision 0)
+++ testsuite/g++.dg/lookup/pr84962.C (working copy)
@@ -0,0 +1,14 @@
+// PR c++/84952 ICE with anon-struct having member fns
+// { dg-do compile { target c++11 } }
+// { dg-additional-options -Wno-pedantic }
+
+struct X {
+ struct
+ {
+template int a ();
+// { dg-error "can only have" "" { target *-*-* } .-1 }
+ };
+
+ int : a; // { dg-error "non-integral" }
+};
+
Re: Use SCEV information when aligning for vectorisation (PR 84005)
On Mon, Mar 19, 2018 at 10:27 PM, Richard Sandiford
wrote:
> Richard Biener writes:
>> On Sat, Mar 17, 2018 at 11:45 AM, Richard Sandiford
>> wrote:
>>> Index: gcc/tree-data-ref.c
>>> ===
>>> --- gcc/tree-data-ref.c 2018-02-14 13:14:36.268006810 +
>>> +++ gcc/tree-data-ref.c 2018-03-17 10:43:42.544669549 +
>>> @@ -5200,6 +5200,75 @@ dr_alignment (innermost_loop_behavior *d
>>>return alignment;
>>> }
>>>
>>> +/* If BASE is a pointer-typed SSA name, try to find the object that it
>>> + is based on. Return this object X on success and store the alignment
>>> + in bytes of BASE - &X in *ALIGNMENT_OUT. */
>>> +
>>> +static tree
>>> +get_base_for_alignment_1 (tree base, unsigned int *alignment_out)
>>> +{
>>> + if (TREE_CODE (base) != SSA_NAME || !POINTER_TYPE_P (TREE_TYPE (base)))
>>> +return NULL_TREE;
>>> +
>>> + gimple *def = SSA_NAME_DEF_STMT (base);
>>> + base = analyze_scalar_evolution (loop_containing_stmt (def), base);
>>
>> I think it is an error to use the def stmt of base here. Instead you
>> need to pass down the point/loop of the use. For the same reason you
>> also want to instantiate parameters after analyzing the evolution.
>>
>> In the end, if the BB to be vectorized is contained in a loop nest we want to
>> instantiate up to the point where (eventually) a DECL we can re-align
>> appears,
>> but using the containing loop for now looks OK.
>
> Why's that necessary/better though? We're not interested in the
> evolution of the value at the point it*s used by the potential vector
> code, but in how we get from the ultimate base (if there is one) to the
> definition of the SSA name.
Hmm, ok.
> I don't think instantiating the SCEV would give any new information,
> but it could lose some. E.g. if we have:
>
> x = &foo;
> do
> x += 8;
> while (*y++ < 10)
> ...potential vector use of *x...
>
> we wouldn't be able to express the address of x after the do-while
> loop, because there's nothing that counts the number of iterations.
> But the uninstantiated SCEV could tell us that x = &foo + N * 8 for
> some (unknown) N.
Not sure that it works that way. I'm not 100% sure what kind of
parameters are left symbolic, and if analysis loop and instantiation
"loop" are the same I guess it doesn't make any difference...
> (OK, so that doesn't actually work unless we take the effort
> to look through loop-closed SSA form, but still...)
>
>>> + /* Peel chrecs and record the minimum alignment preserved by
>>> + all steps. */
>>> + unsigned int alignment = MAX_OFILE_ALIGNMENT / BITS_PER_UNIT;
>>> + while (TREE_CODE (base) == POLYNOMIAL_CHREC)
>>> +{
>>> + unsigned int step_alignment = highest_pow2_factor (CHREC_RIGHT
>>> (base));
>>> + alignment = MIN (alignment, step_alignment);
>>> + base = CHREC_LEFT (base);
>>> +}
>>> +
>>> + /* Punt if the expression is too complicated to handle. */
>>> + if (tree_contains_chrecs (base, NULL) || !POINTER_TYPE_P (TREE_TYPE
>>> (base)))
>>> +return NULL_TREE;
>>> +
>>> + /* Analyze the base to which the steps we peeled were applied. */
>>> + poly_int64 bitsize, bitpos, bytepos;
>>> + machine_mode mode;
>>> + int unsignedp, reversep, volatilep;
>>> + tree offset;
>>> + base = get_inner_reference (build_fold_indirect_ref (base),
>>
>> ick :/
>>
>> what happens if you simply use get_pointer_alignment here and
>> strip down POINTER_PLUS_EXPRs to the ultimate LHS? (basically
>> what get_pointer_alignment_1 does) After all get_base_for_alignment_1
>> itself only deals with plain SSA_NAMEs, not, say, &a_1->b.c or &MEM[a_1 + 4].
>
> Yeah, but those have (hopefully) been handled by dr_analyze_innermost
> already, which should have stripped off both the constant and variable
> parts of the offset. So I think SSA names are the only interesting
> input. The problem is that once we follow the definitions of an SSA
> name through CHREC_LEFTs, we get a general address again.
>
> get_pointer_alignment and get_pointer_alignment_1 don't do what we want
> because they take the alignment of the existing object into account,
> whereas here we explicitly want to ignore that and only calculate the
> alignment of the offset.
Ah, indeed - I missed that fact.
> I guess we could pass a flag down to ignore the current alignment though?
But we need the base anyway... so, given we can only ever re-align decls
and never plain pointers instead of using build_fold_indirect_ref do
if (TREE_CODE (base) != ADDR_EXPR)
return NULL_TREE;
else use TREE_OPERAND (base, 0)?
Richard.
>
> Thanks,
> Richard
Re: [PATCH] Fix PR84986
The way i originally made the code was going through ix86_rtx_costs and factoring out/simplifying logic that translates directly to trees. With NOP_EXPR I probably only tought of subgregs and other free stuff which truly_noop_* probably simulates quite well. ix86_rtx_costs understand how to zero extend, sign extend, and fp conversions, but for scalar code only. I do remember looking into some testcases with noop conversions that needs to be vectorized, so costing them zero did improve code, but hopefully noop test will pass for those. We may get fancier next stage1 or if regressions surfaces. Patch is OK. Thanks for looking into this! Honza Dne 2018-03-20 05:41, Richard Biener napsal: On Tue, 20 Mar 2018, Jakub Jelinek wrote: On Tue, Mar 20, 2018 at 11:08:12AM +0100, Richard Biener wrote: > With the x86 vectorizer cost-model rewrite we ended up costing > scalar conversions as nothing. After my patch using the proper > target cost estimates for the scalar version this now exposes > underestimating scalar cost and thus no longer vectorizing > the testcase in this PR. This fix is to restrict zero-costing > to sign-conversions, all other conversions are possibly > value-changing. I guess some zero-extensions are free as well > but I didn't want to get too fancy as I'm not sure about > QImode -> SImode conversions for example since whether > that's free (can just use %eax instead of %ax) likely depends on > context. Aren't casts from integral to integral with smaller precision also always zero cost, at least for scalar code? It just expands to using a SUBREG of whatever holds the larger value. If the precision matches the mode maybe. But I thought we try to avoid using %al (HImode) or %ax (QImode) operands in the end? So it doesn't matter what we expand to but the code-generation in the end is what matters? Btw, the patched behavior is now that of GCC 7 apart from the sign-conversion case costing nothing. If x86 maintainers want to get fancy they can do that but I'm not too familiar with the code to tell. The only case I'm reasonably sure is zero-extension from SImode to DImode which should be always free since moves to %eax are zero-extending to 64bits. Btw, float <-> int conversions will run into the generic costing for vector_stmt and all other conversions into generic vec_promote_demote costing. An alternative patch would be to just cost sign-extensions (and maybe non-mode-size ones of any kind). Richard.
Re: [PATCH] Fix ICE in match_asm_constraints_1 (PR inline-asm/84941)
Hi,
On Tue, 20 Mar 2018, Jakub Jelinek wrote:
> It is very common that input is one of the above cases, during x86_64-linux
> and i686-linux bootstraps+regtests I got:
> 13201x CONST_INT, 1959x MEM, 114x SUBREG, 110x SYMBOL_REF,
> 2x PLUS (the new testcase only)
> and most of those were actually from input-output constraints, like:
> var = 1;
> asm ("" : "+g" (var));
> var2 = &static_var3;
> asm ("" : "+g" (var2));
> etc. I believe the mini-pass does a useful transformation for these that
> ought to make it easier for reload to actually handle the matching
> constraints.
Well, then, so your handling of them makes somewhat sense.
> > - if (end == constraint)
> > + if (end == constraint || *end)
> > continue;
>
> That wouldn't handle e.g.
> asm volatile ("" : "=m,m" (b), "=r,r" (b) : "1,1" (b));
> case.
No, it wouldn't, which was my intention. I'm fine with either way, but
wouldn't have bothered with these cases. Human-written asms don't use
alternatives very often (the asm template can't easily make use of them),
nor do they use funny matching-or-something-else constraints. But if you
want to handle them: more power to you :)
Ciao,
Michael.
Re: [PR middle-end/70359] uncoalesce IVs outside of loops
On Mon, Mar 19, 2018 at 6:08 PM, Aldy Hernandez wrote: > Hi Richard. > > As discussed in the PR, the problem here is that we have two different > iterations of an IV live outside of a loop. This inhibits us from using > autoinc/dec addressing on ARM, and causes extra lea's on x86. > > An abbreviated example is this: > > loop: > # p_9 = PHI > p_20 = p_9 + 18446744073709551615; > goto loop > p_24 = p_9 + 18446744073709551614; > MEM[(char *)p_20 + -1B] = 45; > > Here we have both the previous IV (p_9) and the current IV (p_20) used > outside of the loop. On Arm this keeps us from using auto-dec addressing, > because one use is -2 and the other one is -1. > > With the attached patch we attempt to rewrite out-of-loop uses of the IV in > terms of the current/last IV (p_20 in the case above). With it, we end up > with: > > p_24 = p_20 + 18446744073709551615; > *p_24 = 45; > > ...which helps both x86 and Arm. > > As you have suggested in comment 38 on the PR, I handle specially > out-of-loop IV uses of the form IV+CST and propagate those accordingly > (along with the MEM_REF above). Otherwise, in less specific cases, we un-do > the IV increment, and use that value in all out-of-loop uses. For instance, > in the attached testcase, we rewrite: > > george (p_9); > > into > > _26 = p_20 + 1; > ... > george (_26); > > The attached testcase tests the IV+CST specific case, as well as the more > generic case with george(). > > Although the original PR was for ARM, this behavior can be noticed on x86, > so I tested on x86 with a full bootstrap + tests. I also ran the specific > test on an x86 cross ARM build and made sure we had 2 auto-dec with the > test. For the original test (slightly different than the testcase in this > patch), with this patch we are at 104 bytes versus 116 without it. There is > still the issue of a division optimization which would further reduce the > code size. I will discuss this separately as it is independent from this > patch. > > Oh yeah, we could make this more generic, and maybe handle any multiple of > the constant, or perhaps *= and /=. Perhaps something for next stage1... > > OK for trunk? Sorry for noticing so late but you use loop properties like ->latch and ->loop_father (via flow_bb_inside_loop_p) that may not be valid at this point given expand doesn't do loop_optimizer_init/finalize. Generally you may face loops with multiple latches (->latch == NULL) here and loops may have multiple exits. You probably are not hitting any of those problems because is_iv_plus_constant is quite restrictive (doesn't handle PLUS_EXPR or MINUS_EXPR). Also the latch doesn't need to be the block with the exit conditional. So first of all you'd need to wrap insert_backedge_copies () with loop_optimizer_init (AVOID_CFG_MODIFICATIONS); ... loop_optimizer_finalize (); that is required to fixup an eventually broken state. Then you probably should restrict handling to PHIs in BBs with bb->loop_father->header == bb (aka loop header PHIs), use get_loop_exit_edges when the IV increment is of OK form and then use dominator info to query in which exit block to insert compensation (or simply refuse to do anything for loops with multiple exits). So if you have more cycles to work on this that would be nice, otherwise I can certainly take over from here... Thanks, Richard. > Aldy
Re: [PATCH] Fix PR84986
On Tue, Mar 20, 2018 at 11:41:01AM +0100, Richard Biener wrote: > If the precision matches the mode maybe. But I thought we try to > avoid using %al (HImode) or %ax (QImode) operands in the end? Yes, but we try to do that only on the level of the emitted assembly, sometimes we simply emit a QImode or HImode instruction that does movl %eax, %ebx and similar. That is also zero cost compared to what it would cost to do it in SImode. Jakub
C++ PATCH for c++/84978, ICE with NRVO
We started crashing on this test with r258592 which added cp_get_callee_fndecl
in in cp_genericize_r.
This ICE apparently depends on whether we perform NRVO or not. If the size of
S is <=16B we pass it in registers and it compiles fine. But if the size of S
is >16B, then we pass in memory, and we NRV-optimize. That means that
s.fn ();
is turned by finalize_nrv into
.fn ();
Then the newly added call to cp_get_callee_fndecl calls maybe_constant_init,
which ends up evaluating , but it's not in the hash map, so we crash
here:
4111 /* We ask for an rvalue for the RESULT_DECL when indirecting
4112 through an invisible reference, or in named return value
4113 optimization. */
4114 return (*ctx->values->get (t));
I thought we could be more careful and not blindly dereference the result
of get(). After all, it's not a problem here if the cannot be
evaluated to a constant.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2018-03-20 Marek Polacek
PR c++/84978
* constexpr.c (cxx_eval_constant_expression): Handle the case when
a RESULT_DECL isn't in the hash map.
* g++.dg/opt/nrv19.C: New test.
diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
index 894bcd0bb3e..1f8ece89730 100644
--- gcc/cp/constexpr.c
+++ gcc/cp/constexpr.c
@@ -4111,7 +4111,15 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx,
tree t,
/* We ask for an rvalue for the RESULT_DECL when indirecting
through an invisible reference, or in named return value
optimization. */
- return (*ctx->values->get (t));
+ if (tree *p = ctx->values->get (t))
+ return *p;
+ else
+ {
+ if (!ctx->quiet)
+ error ("%qE is not a constant expression", t);
+ *non_constant_p = true;
+ }
+ break;
case VAR_DECL:
if (DECL_HAS_VALUE_EXPR_P (t))
diff --git gcc/testsuite/g++.dg/opt/nrv19.C gcc/testsuite/g++.dg/opt/nrv19.C
index e69de29bb2d..385593cc90c 100644
--- gcc/testsuite/g++.dg/opt/nrv19.C
+++ gcc/testsuite/g++.dg/opt/nrv19.C
@@ -0,0 +1,15 @@
+// PR c++/84978
+// { dg-do compile }
+
+struct S {
+ void (*fn)();
+ int a[10];
+};
+
+S
+foo ()
+{
+ S s;
+ s.fn ();
+ return s;
+}
Marek
Re: [RFC Patch], PowerPC memory support pre-gcc9, patch #2
On Thu, Mar 15, 2018 at 01:04:30PM -0400, Michael Meissner wrote:
> This is patch #2 of my series for improving the PowerPC internal memory
> support. It assumes patch #1 has been applied.
>
> This patch moves the rs6000_move_128bit function from rs6000.c to a new file,
> rs6000-output.c.
>
> The third patch will create a rs6000_move_64bit function and change both
> 32-bit
> and 64-bit movdi to call it, instead of having all of the instructions be
> literals. I will also likely add improvements to setting the reg_addr address
> masks for offsetable addresses.
>
> The fourth patch will like move movdd and movdf to call rs6000_move_64bit as
> well.
>
> I tested this on a little endian power8 system and there were no regressions.
>
> 2018-03-14 Michael Meissner
>
> * config.gcc (powerpc*-*-*): Add rs6000-output.o to extra_objs.
> * config/rs6000/t-rs6000 (rs6000-output.o): Add build rule.
> * config/rs6000/rs6000.c (rs6000_output_move_128bit): Move to
> rs6000-output.c.
I am not happy at all with this new file, and it won't even work as far
as I see (for multi-alternative define_insn's; splitting the strings to
a different file than the constraints and attributes is asking for
trouble, better keep it all together).
Files should bundle together code that conceptually belongs together,
not some arbitrary split ("these routine return strings that are
eventually output from the compiler as instructions").
Segher
Re: [PATCH] Remove ICEing test-case.
On Tue, Mar 20, 2018 at 9:08 AM, Martin Liška wrote: > Hi. > > The CHKP test-case ICEs for me and as CHKP will be removed in next stage1, > I would like to remove the test-case. Will it be possible? OK. > Martin > > gcc/testsuite/ChangeLog: > > 2018-03-19 Martin Liska > > * gcc.dg/lto/chkp-ctor-merge_0.c: Remove. > --- > gcc/testsuite/gcc.dg/lto/chkp-ctor-merge_0.c | 23 --- > 1 file changed, 23 deletions(-) > delete mode 100644 gcc/testsuite/gcc.dg/lto/chkp-ctor-merge_0.c > >
Re: [wwwdocs] Update gcc-8/changes.html for some IPA and x86 canges
> On Mon, 19 Mar 2018, Jan Hubicka wrote: > > > Hi, > > this patch adds some documentation for what is new in IPA and x86. > > For lto we should mention early-debug. Richard, perhaps you can suggest > > wording? > > I have committed the following (will happily edit if necessary). Great thanks a lot! I wonder if Caveats section should mention that early-debug may not work with your favorite DWARF consumer and that libunwind, lldb and others needs fixing (perhaps mentioning gdb revision that works) Honza
