Re: [PATCH v2] c++: Member template function lookup failure [PR94799]

[Marek Polacek via Gcc-patches]

On Wed, Apr 29, 2020 at 05:10:44PM -0400, Jason Merrill via Gcc-patches wrote:
> On 4/28/20 11:55 PM, Marek Polacek wrote:
> > Whew, this took a while.  We fail to parse "p->template A::a()"
> > (where p is of type A *) because since r249752 we treat the RHS of the ->
> > as dependent and avoid a lookup in the enclosing context: since that rev
> > cp_parser_template_name checks parser->context->object_type too, which
> > here is unknown_type_node, signalling a type-dependent object:
> > 
> >   7756   if (dependent_p)
> >   7757 /* Tell cp_parser_lookup_name that there was an object, even 
> > though it's
> >   7758type-dependent.  */
> >   7759 parser->context->object_type = unknown_type_node;
> > 
> > with which cp_parser_template_name returns identifier 'A', 
> > cp_parser_class_name
> > then creates a TEMPLATE_ID_EXPR A, but then
> > 
> > 23735   decl = make_typename_type (scope, decl, tag_type, tf_error);
> > 
> > in cp_parser_class_name fails because scope is NULL.  Then we return
> > error_mark_node and parse errors ensue.
> > 
> > I've tried various approaches, e.g. keeping TEMPLATE_ID_EXPR around
> > instead of calling make_typename_type, which didn't work, whereupon I
> > realized that since we don't want to perform name lookup if we've seen
> > the template keyword and the scope is dependent, we can adjust
> > parser->context->object_type and use the type of the object expression
> > as the scope, even if it's type-dependent.  This should be in line with
> > [basic.lookup.classref]p4.
> 
> > The "&& scope != unknown_type_node" line in cp_parser_class_name is there
> > for diagnostic purposes only (to avoid issuing a confusing error).
> > 
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > (Happy to defer to GCC 11 if this doesn't seem very safe.)
> > 
> > PR c++/94799
> > * parser.c (cp_parser_postfix_dot_deref_expression): If we have
> > a type-dependent object of class type, stash it to
> > parser->context->object_type.
> > (cp_parser_class_name): Consider object scope too.  Don't call
> > make_typename_type when the scope is unknown_type_node.
> > 
> > * g++.dg/lookup/this1.C: Adjust dg-error.
> > * g++.dg/template/lookup12.C: New test.
> > * g++.dg/template/lookup13.C: New test.
> > * g++.dg/template/lookup14.C: New test.
> > ---
> >   gcc/cp/parser.c  | 28 ++--
> >   gcc/testsuite/g++.dg/lookup/this1.C  |  2 +-
> >   gcc/testsuite/g++.dg/template/lookup12.C | 26 ++
> >   gcc/testsuite/g++.dg/template/lookup13.C | 28 
> >   gcc/testsuite/g++.dg/template/lookup14.C | 11 ++
> >   5 files changed, 87 insertions(+), 8 deletions(-)
> >   create mode 100644 gcc/testsuite/g++.dg/template/lookup12.C
> >   create mode 100644 gcc/testsuite/g++.dg/template/lookup13.C
> >   create mode 100644 gcc/testsuite/g++.dg/template/lookup14.C
> > 
> > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> > index e1f9786893a..b344721fb60 100644
> > --- a/gcc/cp/parser.c
> > +++ b/gcc/cp/parser.c
> > @@ -7694,11 +7694,16 @@ cp_parser_postfix_dot_deref_expression (cp_parser 
> > *parser,
> > bool pseudo_destructor_p;
> > tree scope = NULL_TREE;
> > location_t start_loc = postfix_expression.get_start ();
> > +  tree type = TREE_TYPE (postfix_expression);
> > /* If this is a `->' operator, dereference the pointer.  */
> > if (token_type == CPP_DEREF)
> > -postfix_expression = build_x_arrow (location, postfix_expression,
> > -   tf_warning_or_error);
> > +{
> > +  if (type && POINTER_TYPE_P (type))
> > +   type = TREE_TYPE (type);
> > +  postfix_expression = build_x_arrow (location, postfix_expression,
> > + tf_warning_or_error);
> > +}
> > /* Check to see whether or not the expression is type-dependent and
> >not the current instantiation.  */
> > dependent_p = type_dependent_object_expression_p (postfix_expression);
> > @@ -7754,9 +7759,10 @@ cp_parser_postfix_dot_deref_expression (cp_parser 
> > *parser,
> >   }
> > if (dependent_p)
> > -/* Tell cp_parser_lookup_name that there was an object, even though 
> > it's
> > -   type-dependent.  */
> > -parser->context->object_type = unknown_type_node;
> > +/* If we don't have a (type-dependent) object of class type, use
> > +   unknown_type_node to signal that there was an object.  */
> > +parser->context->object_type = (type && CLASS_TYPE_P (type)
> > +   ? type : unknown_type_node);
> 
> Anything that depends on CLASS_TYPE_P won't work if 'p' isn't clearly a
> class, i.e. if it has type T*, T, or NULL_TREE.  Why not use any non-null
> type here?
> 
> For null type, I wonder if using decltype would make sense.

I've reworked this part to use decltype; curiously it seems to work just fine.
If we use decltype, 

Re: [PATCH] rtl cse: Fix PR94740, ICE on testsuite/gcc.dg/sso/t5.c with -mcpu=future -mpcrel -O1

[Peter Bergner via Gcc-patches]

On 4/30/20 2:02 PM, Richard Sandiford wrote:
> Jeff Law  writes:
>> On Thu, 2020-04-30 at 08:54 +0100, Richard Sandiford wrote:
>>> I guess at this point it needs a review from someone else though.
>>> Jeff, WDYT?  Attached again below, this time without the shonky mime type.
>> It looks reasonable reasonable to me. Re-using simplify_replace_fn_rtx seems
>> like a major simplification, which is definitely good.
> 
> Great, thanks!  Now pushed to master.

I pushed the test case to master too.  Thank you everyone!

Peter

RE: [PATCH][AARCH64] Fix for PR86901

[Modi Mo via Gcc-patches]

Hey all-apologies for the long delay. Haven't had time until recently to look 
into this further.
>>> The zero extract now matching against other modes would generate a
>>> test + branch rather than the combined instruction which led to the
>>> code size regression. I've updated the patch so that tbnz etc. matches GPI 
>>> and
>>that brings code size down to <0.2% in spec2017 and <0.4% in spec2006.
>>
>>That's looking better indeed. I notice there are still differences, eg. 
>>tbz/tbnz
>>counts are significantly different in perlbench, with ~350 missed cases 
>>overall
>>(mostly tbz reg, #7).
>>
>>There are also more uses of uxtw, ubfiz, sbfiz - for example I see cases like 
>>this
>>in namd:
>>
>>  42c7dc:       13007400        sbfx    w0, w0, #0, #30
>>  42c7e0:       937c7c00        sbfiz   x0, x0, #4, #32
>>
>>So it would be a good idea to check any benchmarks where there is still a non-
>>trivial codesize difference. You can get a quick idea what is happening by
>>grepping for instructions like this:
>>
>>grep -c sbfiz out1.txt out2.txt
>>out1.txt:872
>>out2.txt:934
>>
>>grep -c tbnz out1.txt out2.txt
>>out1.txt:5189
>>out2.txt:4989

That's really good insight Wilco! I took a look at the tbnz/tbz case in perl 
and we lose matching against this because allowing SI mode on extv/extzv causes 
subst in combine.c to generate:

(lshiftrt:SI (reg:SI 107 [ _16 ])
(const_int 7 [0x7]))
(nil)

Instead of:

(and:DI (lshiftrt:DI (subreg:DI (reg:SI 107 [ _16 ]) 0)
(const_int 7 [0x7]))
(const_int 1 [0x1]))

The latter case is picked up in make_compound_operation_int to transform into a 
zero_extract while the new case is left alone. A lshiftrt generally can't be 
reduced down to a bit-test but in this case it can because we have zero_bit 
information on it. Given that, looking around try_combine it seems like the 
best place to detect this pattern is in the 2nd chance code after the first 
failure of recog_for_combine which I've done in this patch. I think this is the 
place to put this fix given changing subst/make_compound_operation_int leads to 
significantly more diffs.

After this change the total number of tbnz/tbz lines up near identical to the 
baseline which is good and overall size within .1% on spec 2017 and spec 2006. 
However, looking further at ubfiz there's a pretty large increase in certain 
benchmarks. I looked into spec 2017/blender and we fail to combine this pattern:

Trying 653 -> 654:
  653: r512:SI=r94:SI 0>>0x8
  REG_DEAD r94:SI
  654: r513:DI=zero_extend(r512:SI)
  REG_DEAD r512:SI
Failed to match this instruction:
(set (reg:DI 513)
(zero_extract:DI (reg:SI 94 [ bswapdst_4 ])
(const_int 8 [0x8])
(const_int 8 [0x8])))

Where previously we combined it like this:

Trying 653 -> 654:
  653: r512:SI=r94:SI 0>>0x8
  REG_DEAD r94:SI
  654: r513:DI=zero_extend(r512:SI)
  REG_DEAD r512:SI
Successfully matched this instruction:
(set (reg:DI 513)
(zero_extract:DI (subreg:DI (reg:SI 94 [ bswapdst_4 ]) 0) // subreg used
(const_int 8 [0x8])
(const_int 8 [0x8])))

Here's where I'm at an impasse. The code that generates the modes in 
get_best_reg_extraction_insn looks at the inner mode of SI now that extzvsi is 
valid and generates a non-subreg use. However, the MD pattern is looking for 
all modes being DI or SI not a mix. I think a fix could be done to canonicalize 
these extracts to the same mode but am unsure if in general a mode mismatched 
extract RTX is valid which would make this a fairly large change. 

Latest patch with fix for tbnz/tbz is attached alongside the numbers for SPEC 
and instruction count for SPEC 2017 are attached for reference.

>>> Can you send me the necessary documents to make that happen? Thanks!
>>
>>That's something you need to sort out with the fsf. There is a mailing list 
>>for this:
>>mailto:ass...@gnu.org.

I haven't had any response from my previous mail there. Should I add one of you 
to the CC or mail someone specifically to get traction?

Best,
Modi


pr86901.patch
Description: pr86901.patch
basediff
% increase
textdatabss total   filenametextdata
bss total   filename
1038264 243802  12472   1294538 
base/benchspec/CPU2006/400.perlbench/exe/perlbench_base.gcc9-base   
1038344 243714  12472   1294530 
diff/benchspec/CPU2006/400.perlbench/exe/perlbench_base.gcc9-diff   
0.01%
72024   16030   432892382   
base/benchspec/CPU2006/401.bzip2/exe/bzip2_base.gcc9-base   
72016   16030   432892374   
diff/benchspec/CPU2006/401.bzip2/exe/bzip2_base.gcc9-diff   
-0.01%
2651976 816398  749792  4218166 
base/benchspec/CPU2006/403.gcc/exe/gcc_base.gcc9-base   2652096 
816558  749792  4218446 

Re: [PATCH] diagnostics: get_option_html_page fixes

[David Malcolm via Gcc-patches]

On Thu, 2020-04-30 at 23:38 +0200, Jakub Jelinek wrote:
> On Thu, Apr 30, 2020 at 05:31:22PM -0400, David Malcolm wrote:
> > On Thu, 2020-04-30 at 23:26 +0200, Jakub Jelinek wrote:
> > > On Thu, Apr 30, 2020 at 05:18:13PM -0400, David Malcolm wrote:
> > > > Thanks for working on this; sorry for getting these wrong.
> > > > 
> > > > Is is possible to build gfortran without C and C++?  If so,
> > > > then if
> > > > I'm
> > > 
> > > It is possible without C++, but not without C.
> > > 
> > > E.g. --enable-languages=fortran,go will actually enable
> > > c,fortran,go,lto.
> > > *,c,*)
> > > ;;  
> > > *)  
> > > enable_languages=c,${enable_languages}
> > > ;;
> > > So, strictly speaking the #ifdef CL_C isn't needed there, so if
> > > you
> > > want,
> > > I can drop that #ifdef and only use CL_CXX ifdef.
> > 
> > Thanks.
> > 
> > Perhaps a silly question, but does the patch give the right answers
> > if
> > someone does that?  (which I think is equivalent to "Are there any
> > options labeled with both Fortran and C++ but not C that are
> > documented
> > in gcc/Warning-Options.html"?)
> 
> ATM all the -W* options with CL_Fortran in the mask
> are either CL_C | CL_CXX | CL_Fortran ... | CL_WARNING,
> or CL_Fortran ... | CL_WARNING (not enabled for anything but Fortran)
> 

Thanks.

Let's go with your second patch then (CL_C & CL_CXX).

Dave

Re: [PATCH] diagnostics: get_option_html_page fixes

[Jakub Jelinek via Gcc-patches]

On Thu, Apr 30, 2020 at 05:31:22PM -0400, David Malcolm wrote:
> On Thu, 2020-04-30 at 23:26 +0200, Jakub Jelinek wrote:
> > On Thu, Apr 30, 2020 at 05:18:13PM -0400, David Malcolm wrote:
> > > Thanks for working on this; sorry for getting these wrong.
> > > 
> > > Is is possible to build gfortran without C and C++?  If so, then if
> > > I'm
> > 
> > It is possible without C++, but not without C.
> > 
> > E.g. --enable-languages=fortran,go will actually enable
> > c,fortran,go,lto.
> > *,c,*)
> > ;;  
> > *)  
> > enable_languages=c,${enable_languages}
> > ;;
> > So, strictly speaking the #ifdef CL_C isn't needed there, so if you
> > want,
> > I can drop that #ifdef and only use CL_CXX ifdef.
> 
> Thanks.
> 
> Perhaps a silly question, but does the patch give the right answers if
> someone does that?  (which I think is equivalent to "Are there any
> options labeled with both Fortran and C++ but not C that are documented
> in gcc/Warning-Options.html"?)

ATM all the -W* options with CL_Fortran in the mask
are either CL_C | CL_CXX | CL_Fortran ... | CL_WARNING,
or CL_Fortran ... | CL_WARNING (not enabled for anything but Fortran)

Jakub

Re: [PATCH] diagnostics: get_option_html_page fixes

[David Malcolm via Gcc-patches]

On Thu, 2020-04-30 at 23:26 +0200, Jakub Jelinek wrote:
> On Thu, Apr 30, 2020 at 05:18:13PM -0400, David Malcolm wrote:
> > Thanks for working on this; sorry for getting these wrong.
> > 
> > Is is possible to build gfortran without C and C++?  If so, then if
> > I'm
> 
> It is possible without C++, but not without C.
> 
> E.g. --enable-languages=fortran,go will actually enable
> c,fortran,go,lto.
> *,c,*)
> ;;  
> *)  
> enable_languages=c,${enable_languages}
> ;;
> So, strictly speaking the #ifdef CL_C isn't needed there, so if you
> want,
> I can drop that #ifdef and only use CL_CXX ifdef.

Thanks.

Perhaps a silly question, but does the patch give the right answers if
someone does that?  (which I think is equivalent to "Are there any
options labeled with both Fortran and C++ but not C that are documented
in gcc/Warning-Options.html"?)

Dave

Re: [PATCH] diagnostics: get_option_html_page fixes

[Jakub Jelinek via Gcc-patches]

On Thu, Apr 30, 2020 at 05:18:13PM -0400, David Malcolm wrote:
> Thanks for working on this; sorry for getting these wrong.
> 
> Is is possible to build gfortran without C and C++?  If so, then if I'm

It is possible without C++, but not without C.

E.g. --enable-languages=fortran,go will actually enable c,fortran,go,lto.
*,c,*)
;;  
*)  
enable_languages=c,${enable_languages}
;;
So, strictly speaking the #ifdef CL_C isn't needed there, so if you want,
I can drop that #ifdef and only use CL_CXX ifdef.

Jakub

Re: [PATCH] diagnostics: get_option_html_page fixes

[David Malcolm via Gcc-patches]

On Thu, 2020-04-30 at 15:02 +0200, Jakub Jelinek wrote:
> Hi!
> 
> (Sorry to David, apparently I've posted it only privately, not to
> gcc-patches, so reposting).
> 
> While testing the --with-documentation-root-url= changes, I run into
> [Wreturn-type] URL pointing to gfortran documentation where it
> obviously
> isn't documented.  The following patch updates the list of options to
> match
> reality (on the other side -Wconversion-extra is gfortran only option
> documented in gfortran.texi).
> 
> Or, perhaps better use the attached patch instead, which doesn't have
> a
> hardcoded list and instead uses the flags?  I went through options.c
> and the updated list of options matches exactly the cases where
> CL_Fortran
> is set for "-W*" options together with CL_C and/or CL_CXX (ok, there
> is also
> -Wall and -Wextra, but hopefully we don't emit [Wall] or [Wextra] for
> anything).
> 
> I have successfully bootstrapped/regtested the second patch (CL_C &
> CL_CXX)
> on x86_64-linux and i686-linux, ok for trunk, or do you prefer the
> first
> one?

Thanks for working on this; sorry for getting these wrong.

Is is possible to build gfortran without C and C++?  If so, then if I'm
reading things right the second patch (CL_C & CL_CXX) would get it
wrong for various Fortran warnings for such a build, as the CL_C/CL_CXX
wouldn't be defined and it will erroneously use the Fortran docs page,
rather than the common one for them.

The second patch seems so much cleaner that having a hardcoded list,
but getting the correct result is more important.

Dave

Re: [PATCH] PowerPC -mcpu=future Patch 1 of 7, add target supports for -mpcrel and -mprefixed

[Segher Boessenkool]

Hi!

As Will says, the subject is much too long.  50 chars max is the ideal.
"target supports" isn't clear; maybe

rs6000: Add effective targets powerpc_{pcrel,prefixed_addr}

or something like it (prefix, colon, capital, no dot).


On Mon, Apr 27, 2020 at 03:46:06PM -0400, Michael Meissner wrote:
> 2020-04-27  Michael Meissner  
> 
>   * lib/target-supports.exp (check_effective_target_powerpc_pcrel):
>   New target for PowerPC -mcpu=future support.

This isn't a "new target", it's not a "target" after all?  Just say
"New.", you can't go wrong with that (if it is new, heh).

>   (check_effective_target_powerpc_prefixed_addr): New target for
>   PowerPC -mcpu=future support.

You also say the same thing for both new functions, which cannot be
right.

> +# Return 1 if the target generates PC-relative instructions automatically
> +proc check_effective_target_powerpc_pcrel { } {
> +return [check_no_messages_and_pattern powerpc_pcrel \
> + {\mpld\M.*[@]pcrel} assembly {
> + static long s;
> + long *p = 
> + long foo (void) { return s; }
> + } {-O2 -mcpu=future}]
> +}

A comment on the "long *p" line wouldn't hurt ;-)

Does the "@" need quoting like that?

Do you want to test for the exact word "pcrel", or any word starting
with it?

"If the target generates pcrel automatically"...  well, your test passes
an -mcpu= that makes pcrel possible.  This needs a different description,
maybe a different name, or maybe the test should change?  How is this
effective target used?

> +# Return 1 if the target generates prefixed instructions automatically
> +proc check_effective_target_powerpc_prefixed_addr { } {
> +return [check_no_messages_and_pattern powerpc_prefixed_addr \
> + {\mpld\M} assembly {
> + long foo (long *p) { return p[0x12345]; }
> + } {-O2 -mcpu=future}]
> +}

Similar here.


Segher

Re: [PATCH] coroutines: Fix handling of artificial vars [PR94886]

[Iain Sandoe]

Nathan Sidwell  wrote:


On 4/30/20 9:24 AM, Iain Sandoe wrote:



The testcase ICEs because the range-based for generates three
artificial variables that need to be allocated to the coroutine
frame but, when walking the BIND_EXR that contains these, the
DECL_INITIAL for one of them refers to an entry appearing later,
which means that the frame entry hasn't been allocated when that
INITIAL is walked.


Hm, the error seems to be the range for's construction, did you look at  
altering that so that such forward references do not occur? (range for is  
described as a src->src xform, so I'm sure it's possible to xform it into  
something a user could have written, which a forward-reference is not)


It looks like the range-for lowering doesn’t quite do what its header  
comment says.

Tracked as PR 94897.


your patch is ok, if that for change's not simple.


not something that could have been done in the last hours before branching  
10 ;)


thanks
Iain

[wwwdocs] Improve ugly formatting for std::atomic

[Jonathan Wakely via Gcc-patches]

This looks much nicer for me, because the font for the  part is
much smaller than the other text.

Committed to wwwdocs.

commit 3f573b5fe7df858a27b0275edc5fd4386804ae83
Author: Jonathan Wakely 
Date:   Thu Apr 30 20:54:09 2020 +0100

Improve ugly formatting for std::atomic

diff --git a/htdocs/gcc-10/changes.html b/htdocs/gcc-10/changes.html
index 6a273142..a3cf19cc 100644
--- a/htdocs/gcc-10/changes.html
+++ b/htdocs/gcc-10/changes.html
@@ -412,7 +412,7 @@ a work-in-progress.
   
   
 std::atomic_ref and
-std::atomicfloating point.
+std::atomicfloating point.
   
   
 Integer comparison functions

Re: [PATCH] rtl cse: Fix PR94740, ICE on testsuite/gcc.dg/sso/t5.c with -mcpu=future -mpcrel -O1

[Jeff Law via Gcc-patches]

On Thu, 2020-04-30 at 20:02 +0100, Richard Sandiford wrote:
> Jeff Law  writes:
> > On Thu, 2020-04-30 at 08:54 +0100, Richard Sandiford wrote:
> > > Peter Bergner  writes:
> > > > On 4/29/20 4:15 PM, Peter Bergner wrote:
> > > > > On 4/29/20 3:28 PM, Richard Sandiford wrote:
> > > > > > (Sorry for going ahead and writing an alternative patch, since if we
> > > > > > do
> > > > > > go for this, I guess the earlier misdirections will have wasted two
> > > > > > days
> > > > > > of your time.  But it seemed like I was just never going to think
> > > > > > about
> > > > > > this PR properly unless I actually tried to write something. :-()
> > > > > 
> > > > > No worries from me!  I'm just glad to see this fixed before the
> > > > > release.
> > > > > I'll kill off a bootstrap and regtest on powerpc64le-linux too, in
> > > > > addition
> > > > > to your tests (arm & x86_64?).  Thanks for your help with this!
> > > > 
> > > > My bootstrap and regtesting of your patch on powerpc64le-linux was 
> > > > clean.
> > > 
> > > Thanks.  aarch64-linux-gnu and x86_64-linux-gnu bootstrap & regtests
> > > also came back clean.  I'll kick off an arm-linux-gnueabihf one too
> > > just to be safe.
> > > 
> > > I guess at this point it needs a review from someone else though.
> > > Jeff, WDYT?  Attached again below, this time without the shonky mime type.
> > It looks reasonable reasonable to me. Re-using simplify_replace_fn_rtx seems
> > like a major simplification, which is definitely good.
> 
> Great, thanks!  Now pushed to master.
> 
> > Presumably one of the major goals here is to get the CONST wrapping
> > from simplify_plus_minus?
> 
> Yeah, that's right (via very indirect means :-))
It was certainly indirect :-)  Thankfully there's only one place that does CONST
wrapping in simplify-rtx, so it was just a matter of walking up the most
interesting call sites.

Jeff

Re: [PATCH] rtl cse: Fix PR94740, ICE on testsuite/gcc.dg/sso/t5.c with -mcpu=future -mpcrel -O1

[Richard Sandiford]

Jeff Law  writes:
> On Thu, 2020-04-30 at 08:54 +0100, Richard Sandiford wrote:
>> Peter Bergner  writes:
>> > On 4/29/20 4:15 PM, Peter Bergner wrote:
>> > > On 4/29/20 3:28 PM, Richard Sandiford wrote:
>> > > > (Sorry for going ahead and writing an alternative patch, since if we do
>> > > > go for this, I guess the earlier misdirections will have wasted two 
>> > > > days
>> > > > of your time.  But it seemed like I was just never going to think about
>> > > > this PR properly unless I actually tried to write something. :-()
>> > > 
>> > > No worries from me!  I'm just glad to see this fixed before the release.
>> > > I'll kill off a bootstrap and regtest on powerpc64le-linux too, in 
>> > > addition
>> > > to your tests (arm & x86_64?).  Thanks for your help with this!
>> > 
>> > My bootstrap and regtesting of your patch on powerpc64le-linux was clean.
>> 
>> Thanks.  aarch64-linux-gnu and x86_64-linux-gnu bootstrap & regtests
>> also came back clean.  I'll kick off an arm-linux-gnueabihf one too
>> just to be safe.
>> 
>> I guess at this point it needs a review from someone else though.
>> Jeff, WDYT?  Attached again below, this time without the shonky mime type.
> It looks reasonable reasonable to me. Re-using simplify_replace_fn_rtx seems
> like a major simplification, which is definitely good.

Great, thanks!  Now pushed to master.

> Presumably one of the major goals here is to get the CONST wrapping
> from simplify_plus_minus?

Yeah, that's right (via very indirect means :-))

> ps.  Both forms looked the same in my inbox.  Not sure how they showed up in 
> the
> archives.

The last one came out OK.  I'd missed specifying a charset for
the second one.

Richard

Re: [PATCH] rtl cse: Fix PR94740, ICE on testsuite/gcc.dg/sso/t5.c with -mcpu=future -mpcrel -O1

[Jeff Law via Gcc-patches]

On Thu, 2020-04-30 at 08:54 +0100, Richard Sandiford wrote:
> Peter Bergner  writes:
> > On 4/29/20 4:15 PM, Peter Bergner wrote:
> > > On 4/29/20 3:28 PM, Richard Sandiford wrote:
> > > > (Sorry for going ahead and writing an alternative patch, since if we do
> > > > go for this, I guess the earlier misdirections will have wasted two days
> > > > of your time.  But it seemed like I was just never going to think about
> > > > this PR properly unless I actually tried to write something. :-()
> > > 
> > > No worries from me!  I'm just glad to see this fixed before the release.
> > > I'll kill off a bootstrap and regtest on powerpc64le-linux too, in 
> > > addition
> > > to your tests (arm & x86_64?).  Thanks for your help with this!
> > 
> > My bootstrap and regtesting of your patch on powerpc64le-linux was clean.
> 
> Thanks.  aarch64-linux-gnu and x86_64-linux-gnu bootstrap & regtests
> also came back clean.  I'll kick off an arm-linux-gnueabihf one too
> just to be safe.
> 
> I guess at this point it needs a review from someone else though.
> Jeff, WDYT?  Attached again below, this time without the shonky mime type.
It looks reasonable reasonable to me.  Re-using simplify_replace_fn_rtx seems
like a major simplification, which is definitely good.  Presumably one of the
major goals here is to get the CONST wrapping from simplify_plus_minus?

Jeff

ps.  Both forms looked the same in my inbox.  Not sure how they showed up in the
archives.

Re: [PATCH] c: Fix ICE with _Atomic side-effect in nested fn param decls [PR94842]

[Joseph Myers]

On Thu, 30 Apr 2020, Jakub Jelinek via Gcc-patches wrote:

> Hi!
> 
> If there are _Atomic side-effects in the parameter declarations
> of non-nested function, when they are parsed, current_function_decl is
> NULL, the create_artificial_label created labels during build_atomic* are
> then adjusted by store_parm_decls through set_labels_context_r callback.
> Unfortunately, if such thing happens in nested function parameter
> declarations, while those decls are parsed current_function_decl is the
> parent function (and am not sure it is a good idea to temporarily clear it,
> some code perhaps should be aware it is in a nested function, or it can
> refer to variables from the parent function etc.) and that means
> store_param_decls through set_labels_context_r doesn't adjust anything.
> As those labels are emitted in the nested function body rather than in the
> parent, I think it is ok to override the context in those cases.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: [PATCH] rtl cse: Fix PR94740, ICE on testsuite/gcc.dg/sso/t5.c with -mcpu=future -mpcrel -O1

[Richard Sandiford]

Richard Sandiford  writes:
> Peter Bergner  writes:
>> On 4/29/20 4:15 PM, Peter Bergner wrote:
>>> On 4/29/20 3:28 PM, Richard Sandiford wrote:
 (Sorry for going ahead and writing an alternative patch, since if we do
 go for this, I guess the earlier misdirections will have wasted two days
 of your time.  But it seemed like I was just never going to think about
 this PR properly unless I actually tried to write something. :-()
>>> 
>>> No worries from me!  I'm just glad to see this fixed before the release.
>>> I'll kill off a bootstrap and regtest on powerpc64le-linux too, in addition
>>> to your tests (arm & x86_64?).  Thanks for your help with this!
>>
>> My bootstrap and regtesting of your patch on powerpc64le-linux was clean.
>
> Thanks.  aarch64-linux-gnu and x86_64-linux-gnu bootstrap & regtests
> also came back clean.  I'll kick off an arm-linux-gnueabihf one too
> just to be safe.

FTR, that came back clean too.

> I guess at this point it needs a review from someone else though.
> Jeff, WDYT?  Attached again below, this time without the shonky mime type.

Here's a different attempt to attach the patch (sorry -- experimenting
with different ways of attaching the patch inline, to see how the ML
archive reacts.)

Richard

>From 39e20b51af8f977a1786ef5c15af80e47415d352 Mon Sep 17 00:00:00 2001
From: Richard Sandiford 
Date: Wed, 29 Apr 2020 20:38:13 +0100
Subject: [PATCH] cse: Use simplify_replace_fn_rtx to process notes [PR94740]

cse_process_notes did a very simple substitution, which in the wrong
circumstances could create non-canonical RTL and invalid MEMs.
Various sticking plasters have been applied to cse_process_notes_1
to handle cases like ZERO_EXTEND, SIGN_EXTEND and UNSIGNED_FLOAT,
but I think this PR is a plaster too far.

The code is trying hard to avoid creating unnecessary rtl, which of
course is a good thing.  If we continue to do that, then we can end
up changing subexpressions while keeping the containing rtx.
This in turn means that validate_change will be a no-op on the
containing rtx, even if its contents have changed.  So in these
cases we have to apply validate_change to the individual subexpressions.

On the other hand, if we always apply validate_change to the
individual subexpressions, we'll end up calling validate_change
on something before it has been simplified and canonicalised.
And that's one of the situations we're trying to avoid.

There might be a middle ground in which we queue the validate_changes
as part of a group, and so can cancel the pending validate_changes
for subexpressions if there's a change in the outer expression.
But that seems even more ad-hoc than the current code.
It would also be quite an invasive change.

I think the best thing is just to hook into the existing
simplify_replace_fn_rtx function, keeping the REG and MEM handling
from cse_process_notes_1 essentially unchanged.  It can generate
more redundant rtl when a simplification takes place, but it has
the advantage of being relative well-used code (both directly
and via simplify_replace_rtx).

2020-04-29  Richard Sandiford  

gcc/
PR rtl-optimization/94740
* cse.c (cse_process_notes_1): Replace with...
(cse_process_note_1): ...this new function, acting as a
simplify_replace_fn_rtx callback to process_note.  Handle only
REGs and MEMs directly.  Validate the MEM if cse_process_note
changes its address.
(cse_process_notes): Replace with...
(cse_process_note): ...this new function.
(cse_extended_basic_block): Update accordingly, iterating over
the register notes and passing individual notes to cse_process_note.
---
 gcc/cse.c | 118 --
 1 file changed, 34 insertions(+), 84 deletions(-)

diff --git a/gcc/cse.c b/gcc/cse.c
index 5aaba8d80e0..36bcfc354d8 100644
--- a/gcc/cse.c
+++ b/gcc/cse.c
@@ -585,7 +585,6 @@ static void cse_insn (rtx_insn *);
 static void cse_prescan_path (struct cse_basic_block_data *);
 static void invalidate_from_clobbers (rtx_insn *);
 static void invalidate_from_sets_and_clobbers (rtx_insn *);
-static rtx cse_process_notes (rtx, rtx, bool *);
 static void cse_extended_basic_block (struct cse_basic_block_data *);
 extern void dump_class (struct table_elt*);
 static void get_cse_reg_info_1 (unsigned int regno);
@@ -6222,75 +6221,28 @@ invalidate_from_sets_and_clobbers (rtx_insn *insn)
 }
 }
 
-/* Process X, part of the REG_NOTES of an insn.  Look at any REG_EQUAL notes
-   and replace any registers in them with either an equivalent constant
-   or the canonical form of the register.  If we are inside an address,
-   only do this if the address remains valid.
+static rtx cse_process_note (rtx);
 
-   OBJECT is 0 except when within a MEM in which case it is the MEM.
+/* A simplify_replace_fn_rtx callback for cse_process_note.  Process X,
+   part of the REG_NOTES of an insn.  Replace any registers with either
+   an equivalent 

Re: [PATCH] var-tracking.c: Fix possible use of uninitialized variable pre

[Richard Biener via Gcc-patches]

On Thu, Apr 30, 2020 at 5:14 PM Andreas Krebbel  wrote:
>
> On 30.04.20 08:25, Richard Biener via Gcc-patches wrote:
> > On Wed, Apr 29, 2020 at 5:56 PM Jeff Law  wrote:
> >>
> >> On Tue, 2020-04-28 at 11:44 +0200, Richard Biener via Gcc-patches wrote:
> >>>
> >>> Btw, does s390 have different inlining parameters somehow?
> >> I think so.  We saw a ton of backend warnings that were specific to the 
> >> s390
> >> builds in Fedora that appeared to be triggered by different inlining 
> >> decisions.
> >>
> >> It happened enough that one of the ideas we're going to explore is seeing 
> >> if we
> >> can tune the x86_64 port to use the same heuristics.  That way we can try 
> >> to find
> >> more of these issues in our tester without having to dramatically increase 
> >> s390x
> >> resourcing.  This hasn't started, but I expect we'll be looking at it in 
> >> the fall
> >> (it's unclear if we can just use flags or will need hackery to the x86 
> >> port, but
> >> we're happy to share whatever we find with the wider community).
> >
> > Hmm, OK.  Guess if it becomes an issue for openSUSE I'll simply revert those
> > s390 backend changes.
>
> Too bad. I would prefer to fix the root cause of the warnings instead. Isn't 
> it a good thing that
> thanks to the more aggressive inlining actual bugs in the code get revealed? 
> Or do you see other
> issues with that?

I'm obviously more concerned about latent GCC issues only popping up
on s390.  Basically
the s390 limits throw much of the QA done on x86_64 targets away when
comparing to s390.
For example we do burn-in testing of a new GCC release (10.1 here) in
openSUSE Factory
by using it as system compiler.  Then with GCC 10.2 we ship it as
compiler for their own use
to SLES users and ISVs.  But obviously all the burn-in testing on
openSUSE Factory
is very x86_64 centric these days.

> >
> > As a general advice to backend developers I _strongly_ discourage to adjust
> > --param defaults that affect GIMPLE.
>
> We did tons of measurements to find these values recently and got a nice 
> speedup after increasing
> them. We also had a look at the size increase of binaries. It was significant 
> but we decided that
> the performance benefit is worth it. On IBM Z the function call overhead is 
> bigger than on other
> archs so inlining in general helps us.
>
> I'm not sure I understand your recommendation to leave these values as is. I 
> assumed that such
> parameters are supposed to be used to deal with the characteristics of a 
> platform. Are there better
> ways to achieve the same effect?

Get more inlining than other archs?  No, I don't think so.  But if the
call overhead is bigger then
you should adjust the cost of calls instead?  Also the default limits
are a delicate compromise
between speed and size - of course if you disregard size you get more
speed.  You also only
adjusted two out of very many params which are not really as
independent as one would think...
(and you forgot to re-tune after last years big changes/parameter space splits).

As said you should at least think of adjusting the adjustments to be
relative to the default
value rather than absolute.

Richard.

> Andreas
>
> >
> > Richard.
> >
> >> jeff
> >>
> >>
> >>
>

[PATCH] c++: Parenthesized-init of aggregates accepts invalid code [PR94885]

[Marek Polacek via Gcc-patches]

Here we have (conceptually *) something like

  struct B { };
  struct D : B { };
  D(0); // invalid

and in C++20 the ()-initialization has created a { 0 } constructor that
it tries to initialize an object of type D with.  We should reject
initializing an object of type B from 0, but we wrongly accept it because
process_init_constructor_record skips initializers for empty bases/fields:
   if (DECL_SIZE (field) && integer_zerop (DECL_SIZE (field))
   && !TREE_SIDE_EFFECTS (next))
 /* Don't add trivial initialization of an empty base/field to the
constructor, as they might not be ordered the way the back-end
expects.  */
 continue;
but here 'next' was error_mark_node, returned by massage_elt_init, so we
wound up with { } which would validly value-initialize the object.

[*] Usually digest_init in build_new_method_call_1 would detect this,
but in this case the instance is is_dummy_object and we don't call
digest just yet.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10?

PR c++/94885
* typeck2.c (process_init_constructor_record): Return PICFLAG_ERRONEOUS
if an initializer element was erroneous.

* g++.dg/cpp2a/paren-init26.C: New test.
---
 gcc/cp/typeck2.c  |  6 +-
 gcc/testsuite/g++.dg/cpp2a/paren-init26.C | 14 ++
 2 files changed, 19 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/paren-init26.C

diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index 56fd9bafa7e..9e5d145a6cd 100644
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -1661,7 +1661,11 @@ process_init_constructor_record (tree type, tree init, 
int nested, int flags,
  ++idx;
}
}
-  if (next)
+  if (next == error_mark_node)
+   /* We skip initializers for empty bases/fields, so skipping an invalid
+  one could make us accept invalid code.  */
+   return PICFLAG_ERRONEOUS;
+  else if (next)
/* Already handled above.  */;
   else if (DECL_INITIAL (field))
{
diff --git a/gcc/testsuite/g++.dg/cpp2a/paren-init26.C 
b/gcc/testsuite/g++.dg/cpp2a/paren-init26.C
new file mode 100644
index 000..0b98ebf43bb
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/paren-init26.C
@@ -0,0 +1,14 @@
+// PR c++/94885 - paren-init of aggregates accepts invalid code.
+// { dg-do compile { target c++2a } }
+
+template  // { dg-error "could not 
convert" }
+void foo();
+
+struct base {};
+struct derived : base {};
+
+void
+bar()
+{
+  foo(); // { dg-error "no matching function" }
+}

base-commit: 448c89d590455ed4ab7abc40309b5cf8ec52d13d
-- 
Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA

Re: [PR c+ 94827]: template parm with default requires

[Nathan Sidwell]

On 4/30/20 10:35 AM, Nathan Sidwell wrote:

On 4/30/20 10:18 AM, Jason Merrill wrote:

On 4/29/20 2:50 PM, Nathan Sidwell wrote:

Jason,
this is the patch you suggested, as I understood it.  I kept 
finish_nested_require's saving of the (converted) 
current_template_parms, becase of the comment about use in diagnostics.


Is this what you meant?


Yes, this looks fine.

But I don't think that we need to keep saving the converted 
current_template_parms; diagnostics could also normalize using 
NULL_TREE args.  And it looks like diagnose_nested_requirement isn't 
currently doing re-normalization anyway.  This doesn't need to block 
the release, though.


Ok, I'll deal with that post-release.

Here's a modified version.  The only change is to remove an assert in 
tsubst_nested_requirement.  We have an arg_vec of 2, but only the first 
is filled in.  Thus we think the NULL one is a dependent type.


The testcase checks we evaluate the default arg as expected.

I don't expect the bootstrap to fail, because this is removing an assert.


indeed it didn't.  The patch is pushed to trunk

nathan


--
Nathan Sidwell

[committed] libstdc++: Avoid errors in allocator's noexcept-specifier (PR 89510)

[Jonathan Wakely via Gcc-patches]

This fixes a regression due to the conditional noexcept-specifier on
std::allocator::construct and std::allocator::destroy, as well as the
corresponding members of new_allocator, malloc_allocator, and
allocator_traits. Those noexcept-specifiers were using expressions which
might be ill-formed, which caused errors outside the immediate context
when checking for the presence of construct and destroy in SFINAE
contexts.

The fix is to use the is_nothrow_constructible and
is_nothrow_destructible type traits instead, because those traits are
safe to use even when the construction/destruction itself is not valid.

The is_nothrow_constructible trait will be false for a type that is not
also nothrow-destructible, even if the new-expression used in the
construct function body is actually noexcept. That's not the correct
answer, but isn't a problem because providing a noexcept-specifier on
these functions is not required by the standard anyway. If the answer is
false when it should be true, that's suboptimal but OK (unlike giving
errors for valid code, or giving a true answer when it should be false).

PR libstdc++/89510
* include/bits/alloc_traits.h (allocator_traits::_S_construct)
(allocator_traits::_S_destroy)
(allocator_traits>::construct): Use traits in
noexcept-specifiers.
* include/bits/allocator.h (allocator::construct)
(allocator::destroy): Likewise.
* include/ext/malloc_allocator.h (malloc_allocator::construct)
(malloc_allocator::destroy): Likewise.
* include/ext/new_allocator.h (new_allocator::construct)
(new_allocator::destroy): Likewise.
* testsuite/20_util/allocator/89510.cc: New test.
* testsuite/ext/malloc_allocator/89510.cc: New test.
* testsuite/ext/new_allocator/89510.cc: New test.

Tested powerpc64le-linux, committed to master.

Backport to gcc-9 will follow soon too.

commit b1983f4582bbe060b7da83578acb9ed653681fc8
Author: Jonathan Wakely 
Date:   Thu Apr 30 15:47:52 2020 +0100

libstdc++: Avoid errors in allocator's noexcept-specifier (PR 89510)

This fixes a regression due to the conditional noexcept-specifier on
std::allocator::construct and std::allocator::destroy, as well as the
corresponding members of new_allocator, malloc_allocator, and
allocator_traits. Those noexcept-specifiers were using expressions which
might be ill-formed, which caused errors outside the immediate context
when checking for the presence of construct and destroy in SFINAE
contexts.

The fix is to use the is_nothrow_constructible and
is_nothrow_destructible type traits instead, because those traits are
safe to use even when the construction/destruction itself is not valid.

The is_nothrow_constructible trait will be false for a type that is not
also nothrow-destructible, even if the new-expression used in the
construct function body is actually noexcept. That's not the correct
answer, but isn't a problem because providing a noexcept-specifier on
these functions is not required by the standard anyway. If the answer is
false when it should be true, that's suboptimal but OK (unlike giving
errors for valid code, or giving a true answer when it should be false).

PR libstdc++/89510
* include/bits/alloc_traits.h (allocator_traits::_S_construct)
(allocator_traits::_S_destroy)
(allocator_traits>::construct): Use traits in
noexcept-specifiers.
* include/bits/allocator.h (allocator::construct)
(allocator::destroy): Likewise.
* include/ext/malloc_allocator.h (malloc_allocator::construct)
(malloc_allocator::destroy): Likewise.
* include/ext/new_allocator.h (new_allocator::construct)
(new_allocator::destroy): Likewise.
* testsuite/20_util/allocator/89510.cc: New test.
* testsuite/ext/malloc_allocator/89510.cc: New test.
* testsuite/ext/new_allocator/89510.cc: New test.

diff --git a/libstdc++-v3/include/bits/alloc_traits.h 
b/libstdc++-v3/include/bits/alloc_traits.h
index 6066f48d24c..86d8ed221ff 100644
--- a/libstdc++-v3/include/bits/alloc_traits.h
+++ b/libstdc++-v3/include/bits/alloc_traits.h
@@ -251,8 +251,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_Require<__and_<__not_<__has_construct<_Tp, _Args...>>,
   is_constructible<_Tp, _Args...>>>
_S_construct(_Alloc&, _Tp* __p, _Args&&... __args)
-   noexcept(noexcept(::new((void*)__p)
- _Tp(std::forward<_Args>(__args)...)))
+   noexcept(std::is_nothrow_constructible<_Tp, _Args...>::value)
{
 #if __cplusplus <= 201703L
  ::new((void*)__p) _Tp(std::forward<_Args>(__args)...);
@@ -271,7 +270,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template
static _GLIBCXX14_CONSTEXPR void
_S_destroy(_Alloc2&, _Tp* __p, ...)
-   

Re: [PATCH] var-tracking.c: Fix possible use of uninitialized variable pre

[Andreas Krebbel via Gcc-patches]

On 30.04.20 08:25, Richard Biener via Gcc-patches wrote:
> On Wed, Apr 29, 2020 at 5:56 PM Jeff Law  wrote:
>>
>> On Tue, 2020-04-28 at 11:44 +0200, Richard Biener via Gcc-patches wrote:
>>>
>>> Btw, does s390 have different inlining parameters somehow?
>> I think so.  We saw a ton of backend warnings that were specific to the s390
>> builds in Fedora that appeared to be triggered by different inlining 
>> decisions.
>>
>> It happened enough that one of the ideas we're going to explore is seeing if 
>> we
>> can tune the x86_64 port to use the same heuristics.  That way we can try to 
>> find
>> more of these issues in our tester without having to dramatically increase 
>> s390x
>> resourcing.  This hasn't started, but I expect we'll be looking at it in the 
>> fall
>> (it's unclear if we can just use flags or will need hackery to the x86 port, 
>> but
>> we're happy to share whatever we find with the wider community).
> 
> Hmm, OK.  Guess if it becomes an issue for openSUSE I'll simply revert those
> s390 backend changes.

Too bad. I would prefer to fix the root cause of the warnings instead. Isn't it 
a good thing that
thanks to the more aggressive inlining actual bugs in the code get revealed? Or 
do you see other
issues with that?

> 
> As a general advice to backend developers I _strongly_ discourage to adjust
> --param defaults that affect GIMPLE.

We did tons of measurements to find these values recently and got a nice 
speedup after increasing
them. We also had a look at the size increase of binaries. It was significant 
but we decided that
the performance benefit is worth it. On IBM Z the function call overhead is 
bigger than on other
archs so inlining in general helps us.

I'm not sure I understand your recommendation to leave these values as is. I 
assumed that such
parameters are supposed to be used to deal with the characteristics of a 
platform. Are there better
ways to achieve the same effect?

Andreas

> 
> Richard.
> 
>> jeff
>>
>>
>>

Re: [PATCH] ipa: Cgraph verification fix (PR 94856)

[Jan Hubicka]

> Hi,
> 
> PR 94856 is a call graph verifier error.  We have a method which (in
> the course of IPA-CP) loses its this pointer because it is unused and
> the pass then does not clone all the this adjusting thunks and just
> makes the calls go straight to the new clone - and then the verifier
> complains that the edge does not seem to point to a clone of what it
> used to.  This looked weird because the verifier actually has logic
> detecting this case but it turns out that it is confused by inliner
> body-saving mechanism which invents a new decl for the base function.
> 
> Making the inlining body-saving mechanism to correctly set
> former_clone_of allows us to detect this case too.  Then we pass this
> particular round of verification but the subsequent one fails because
> we have inlined the function into its former thunk - which
> subsequently does not have any callees, but the verifier still access
> them and segfaults.  Therefore the patch also adds a test whether the
> a former hunk even has any call.
> 
> Passed bootstrap and testsuite on x86-64-linux, LTO bootstrap underway.
> OK for trunk (in time for GCC 10) if it passes it too?
> 
> Thanks,
> 
> Martin
> 
> 
> 2020-04-30  Martin Jambor  
> 
>   PR ipa/94856
>   * cgraph.c (clone_of_p): Also consider thunks whih had their bodies
>   saved by the inliner and thunks which had their call inlined.
>   * ipa-inline-transform.c (save_inline_function_body): Fill in
>   former_clone_of of new body holders.
> 
>   PR ipa/94856
>   * g++.dg/ipa/pr94856.C: New test.
OK,
thanks!

Honza

RE: [PATCH] aarch64: prefer using csinv, csneg in zero extend contexts

[Alex Coplan]

> -Original Message-
> From: Richard Sandiford 
> Sent: 30 April 2020 15:13
> To: Alex Coplan 
> Cc: gcc-patches@gcc.gnu.org; Richard Earnshaw ;
> Marcus Shawcroft ; Kyrylo Tkachov
> ; nd 
> Subject: Re: [PATCH] aarch64: prefer using csinv, csneg in zero extend 
> contexts
> 
> Yeah, I was hoping for something like...
> 
> > Indeed, clang generates a MVN + CSEL sequence where the CSEL operates on the
> > 64-bit registers:
> >
> > f:
> > mvn w8, w2
> > cmp w0, #0
> > cselx0, x8, x1, eq
> > ret
> 
> ...this rather than the 4-insn (+ret) sequence that we currently
> generate.  So it would have been a define_insn_and_split that handles
> the zero case directly but splits into the "optimal" two-instruction
> sequence for registers.
> 
> But I guess the underlying problem is instead that we don't have
> a pattern for (zero_extend:DI (not:SI ...)).  Adding that would
> definitely be a better fix.

Yes. I sent a patch for this very fix which Kyrill is going to commit once stage
1 opens: https://gcc.gnu.org/pipermail/gcc-patches/2020-April/544365.html

I tried compiling this function with that patch applied and we get:

f:
cmp w0, 0
mvn w2, w2
cselx0, x2, x1, eq
ret

which is good.

> ChangeLog trivia, but these last two lines should only be indented by a tab.

Ah, thanks. Noted for next time.

> 
> Hopefully one day we'll finally ditch this format and stop having to
> quibble over such minor formatting stuff...

That would be good!

> 
> > diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> > index c7c4d1dd519..37d651724ad 100644
> > --- a/gcc/config/aarch64/aarch64.md
> > +++ b/gcc/config/aarch64/aarch64.md
> > @@ -4390,6 +4390,45 @@
> >[(set_attr "type" "csel")]
> >  )
> >
> > +(define_insn "*csinv3_uxtw_insn1"
> > +  [(set (match_operand:DI 0 "register_operand" "=r")
> > +(if_then_else:DI
> > +  (match_operand 1 "aarch64_comparison_operation" "")
> > +  (zero_extend:DI
> > +(match_operand:SI 2 "register_operand" "r"))
> > +  (zero_extend:DI
> > +(NEG_NOT:SI (match_operand:SI 3 "register_operand" "r")]
> > +  ""
> > +  "cs\\t%w0, %w2, %w3, %m1"
> > +  [(set_attr "type" "csel")]
> > +)
> > +
> > +(define_insn "*csinv3_uxtw_insn2"
> > +  [(set (match_operand:DI 0 "register_operand" "=r")
> > +(if_then_else:DI
> > +  (match_operand 1 "aarch64_comparison_operation" "")
> > +  (zero_extend:DI
> > +(NEG_NOT:SI (match_operand:SI 2 "register_operand" "r")))
> > +  (zero_extend:DI
> > +(match_operand:SI 3 "register_operand" "r"]
> > +  ""
> > +  "cs\\t%w0, %w3, %w2, %M1"
> > +  [(set_attr "type" "csel")]
> > +)
> > +
> > +(define_insn "*csinv3_uxtw_insn3"
> > +  [(set (match_operand:DI 0 "register_operand" "=r")
> > +(if_then_else:DI
> > +  (match_operand 1 "aarch64_comparison_operation" "")
> > +  (zero_extend:DI
> > +(NEG_NOT:SI (match_operand:SI 2 "register_operand" "r")))
> > +  (const_int 0)))]
> > +  ""
> > +  "cs\\t%w0, wzr, %w2, %M1"
> > +  [(set_attr "type" "csel")]
> > +)
> > +
> > +
> 
> Usually there's just one blank line between patterns, even if the
> patterns aren't naturally grouped.

Ok, good to know.

> 
> No need to repost just for that.  I'll push with those changes once
> stage 1 opens.

Great!

Thanks,
Alex

Re: [PR c+ 94827]: template parm with default requires

[Nathan Sidwell]

On 4/30/20 10:18 AM, Jason Merrill wrote:

On 4/29/20 2:50 PM, Nathan Sidwell wrote:

Jason,
this is the patch you suggested, as I understood it.  I kept 
finish_nested_require's saving of the (converted) 
current_template_parms, becase of the comment about use in diagnostics.


Is this what you meant?


Yes, this looks fine.

But I don't think that we need to keep saving the converted 
current_template_parms; diagnostics could also normalize using NULL_TREE 
args.  And it looks like diagnose_nested_requirement isn't currently 
doing re-normalization anyway.  This doesn't need to block the release, 
though.


Ok, I'll deal with that post-release.

Here's a modified version.  The only change is to remove an assert in 
tsubst_nested_requirement.  We have an arg_vec of 2, but only the first 
is filled in.  Thus we think the NULL one is a dependent type.


The testcase checks we evaluate the default arg as expected.

I don't expect the bootstrap to fail, because this is removing an assert.

nathan

--
Nathan Sidwell
2020-04-30  Jason Merrill  
	Nathan Sidwell  

	PR c++/94827
	* constraint.cc (map_arguments): If ARGS is null, it's a
	self-mapping of parms.
	(finish_nested_requirement): Do not pass argified
	current_template_parms to normalization.
	(tsubst_nested_requirement): Don't assert no template parms.

diff --git c/gcc/cp/constraint.cc w/gcc/cp/constraint.cc
index 06161b8c8c4..5e732bb9b81 100644
--- c/gcc/cp/constraint.cc
+++ w/gcc/cp/constraint.cc
@@ -546,12 +546,16 @@ static tree
 map_arguments (tree parms, tree args)
 {
   for (tree p = parms; p; p = TREE_CHAIN (p))
-{
-  int level;
-  int index;
-  template_parm_level_and_index (TREE_VALUE (p), , );
-  TREE_PURPOSE (p) = TMPL_ARG (args, level, index);
-}
+if (args)
+  {
+	int level;
+	int index;
+	template_parm_level_and_index (TREE_VALUE (p), , );
+	TREE_PURPOSE (p) = TMPL_ARG (args, level, index);
+  }
+else
+  TREE_PURPOSE (p) = TREE_VALUE (p);
+
   return parms;
 }
 
@@ -2005,8 +2009,6 @@ tsubst_compound_requirement (tree t, tree args, subst_info info)
 static tree
 tsubst_nested_requirement (tree t, tree args, subst_info info)
 {
-  gcc_assert (!uses_template_parms (args));
-
   /* Ensure that we're in an evaluation context prior to satisfaction.  */
   tree norm = TREE_VALUE (TREE_TYPE (t));
   tree result = satisfy_constraint (norm, args, info);
@@ -2953,12 +2955,15 @@ finish_compound_requirement (location_t loc, tree expr, tree type, bool noexcept
 tree
 finish_nested_requirement (location_t loc, tree expr)
 {
+  /* Currently open template headers have dummy arg vectors, so don't
+ pass into normalization.  */
+  tree norm = normalize_constraint_expression (expr, NULL_TREE, false);
+  tree args = current_template_parms
+? template_parms_to_args (current_template_parms) : NULL_TREE;
+
   /* Save the normalized constraint and complete set of normalization
  arguments with the requirement.  We keep the complete set of arguments
  around for re-normalization during diagnostics.  */
-  tree args = current_template_parms
-? template_parms_to_args (current_template_parms) : NULL_TREE;
-  tree norm = normalize_constraint_expression (expr, args, false);
   tree info = build_tree_list (args, norm);
 
   /* Build the constraint, saving its normalization as its type.  */
diff --git c/gcc/testsuite/g++.dg/concepts/pr94827.C w/gcc/testsuite/g++.dg/concepts/pr94827.C
new file mode 100644
index 000..f14ec2551a1
--- /dev/null
+++ w/gcc/testsuite/g++.dg/concepts/pr94827.C
@@ -0,0 +1,15 @@
+// PR 94287 ICE looking inside open template-parm level
+// { dg-do run { target c++17 } }
+// { dg-options -fconcepts }
+
+template 
+  int foo(T) { return X; }
+
+int main() {
+  if (!foo('4'))
+return 1;
+  if (foo (4))
+return 2;
+  return 0;
+}

[PATCH] ipa: Cgraph verification fix (PR 94856)

[Martin Jambor]

Hi,

PR 94856 is a call graph verifier error.  We have a method which (in
the course of IPA-CP) loses its this pointer because it is unused and
the pass then does not clone all the this adjusting thunks and just
makes the calls go straight to the new clone - and then the verifier
complains that the edge does not seem to point to a clone of what it
used to.  This looked weird because the verifier actually has logic
detecting this case but it turns out that it is confused by inliner
body-saving mechanism which invents a new decl for the base function.

Making the inlining body-saving mechanism to correctly set
former_clone_of allows us to detect this case too.  Then we pass this
particular round of verification but the subsequent one fails because
we have inlined the function into its former thunk - which
subsequently does not have any callees, but the verifier still access
them and segfaults.  Therefore the patch also adds a test whether the
a former hunk even has any call.

Passed bootstrap and testsuite on x86-64-linux, LTO bootstrap underway.
OK for trunk (in time for GCC 10) if it passes it too?

Thanks,

Martin


2020-04-30  Martin Jambor  

PR ipa/94856
* cgraph.c (clone_of_p): Also consider thunks whih had their bodies
saved by the inliner and thunks which had their call inlined.
* ipa-inline-transform.c (save_inline_function_body): Fill in
former_clone_of of new body holders.

PR ipa/94856
* g++.dg/ipa/pr94856.C: New test.
---
 gcc/ChangeLog  |  8 
 gcc/cgraph.c   |  8 +---
 gcc/ipa-inline-transform.c |  2 ++
 gcc/testsuite/ChangeLog|  5 +
 gcc/testsuite/g++.dg/ipa/pr94856.C | 18 ++
 5 files changed, 38 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/ipa/pr94856.C

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index ade7418401a..87f1f1af8f7 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,11 @@
+2020-04-30  Martin Jambor  
+
+   PR ipa/94856
+   * cgraph.c (clone_of_p): Also consider thunks whih had their bodies
+   saved by the inliner and thunks which had their call inlined.
+   * ipa-inline-transform.c (save_inline_function_body): Fill in
+   former_clone_of of new body holders.
+
 2020-04-27  Jakub Jelinek  
 
PR target/94704
diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 72d7cb54301..2a9813df2d9 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -3104,15 +3104,17 @@ clone_of_p (cgraph_node *node, cgraph_node *node2)
return false;
   /* In case of instrumented expanded thunks, which can have multiple calls
 in them, we do not know how to continue and just have to be
-optimistic.  */
-  if (node->callees->next_callee)
+optimistic.  The same applies if all calls have already been inlined
+into the thunk.  */
+  if (!node->callees || node->callees->next_callee)
return true;
   node = node->callees->callee->ultimate_alias_target ();
 
   if (!node2->clone.param_adjustments
  || node2->clone.param_adjustments->first_param_intact_p ())
return false;
-  if (node2->former_clone_of == node->decl)
+  if (node2->former_clone_of == node->decl
+ || node2->former_clone_of == node->former_clone_of)
return true;
 
   cgraph_node *n2 = node2;
diff --git a/gcc/ipa-inline-transform.c b/gcc/ipa-inline-transform.c
index be60bbccb5c..e9e21cc0296 100644
--- a/gcc/ipa-inline-transform.c
+++ b/gcc/ipa-inline-transform.c
@@ -607,6 +607,8 @@ save_inline_function_body (struct cgraph_node *node)
}
 }
   *ipa_saved_clone_sources->get_create (first_clone) = prev_body_holder;
+  first_clone->former_clone_of
+= node->former_clone_of ? node->former_clone_of : node->decl;
   first_clone->clone_of = NULL;
 
   /* Now node in question has no clones.  */
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index abe97b9c0d5..4e12a77184a 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2020-04-30  Martin Jambor  
+
+   PR ipa/94856
+   * g++.dg/ipa/pr94856.C: New test.
+
 2020-04-26  Marek Polacek  
 
PR c++/90320
diff --git a/gcc/testsuite/g++.dg/ipa/pr94856.C 
b/gcc/testsuite/g++.dg/ipa/pr94856.C
new file mode 100644
index 000..5315c52d80e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/pr94856.C
@@ -0,0 +1,18 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fno-tree-dse --param uninlined-function-insns=0 --param 
early-inlining-insns=3 -fgnu-tm " } */
+
+class a {
+public:
+  virtual ~a() {}
+};
+class b {
+public:
+  virtual void c();
+};
+class C : a, public b {};
+class d : C {
+  ~d();
+  void c();
+};
+d::~d() { ((b *)this)->c(); }
+void d::c() {}
-- 
2.26.0

Re: [PR c+ 94827]: template parm with default requires

[Jason Merrill via Gcc-patches]

On 4/29/20 2:50 PM, Nathan Sidwell wrote:

Jason,
this is the patch you suggested, as I understood it.  I kept 
finish_nested_require's saving of the (converted) 
current_template_parms, becase of the comment about use in diagnostics.


Is this what you meant?


Yes, this looks fine.

But I don't think that we need to keep saving the converted 
current_template_parms; diagnostics could also normalize using NULL_TREE 
args.  And it looks like diagnose_nested_requirement isn't currently 
doing re-normalization anyway.  This doesn't need to block the release, 
though.


Jason

Re: [PATCH] aarch64: prefer using csinv, csneg in zero extend contexts

[Richard Sandiford]

Alex Coplan  writes:
>> At the risk of feature creep :-) a useful third pattern could be
>> to combine a zero-extended operator result with an existing DImode value.
>> In that case, the existing DImode value really can be "rZ" and should
>> always be in the "else" arm of the if_then_else.  E.g.:
>> 
>> unsigned long long
>> f(int a, unsigned long b, unsigned c)
>> {
>>   return a ? b : ~c;
>> }
>> 
>> unsigned long long
>> g(int a, unsigned c)
>> {
>>   return a ? 0 : ~c;
>> }
>> 
>> But that's definitely something that can be left for later.
>
> Hm. What sequence would you like to see for the function f here with the
> argument already in DImode? I don't think we can do it with just a CMP + CSINV
> like in the other test cases because you end up wanting to access b as x1 and 
> c
> as w2 which is not allowed.

Yeah, I was hoping for something like...

> Indeed, clang generates a MVN + CSEL sequence where the CSEL operates on the
> 64-bit registers:
>
> f:
> mvn w8, w2
> cmp w0, #0
> cselx0, x8, x1, eq
> ret

...this rather than the 4-insn (+ret) sequence that we currently
generate.  So it would have been a define_insn_and_split that handles
the zero case directly but splits into the "optimal" two-instruction
sequence for registers.

But I guess the underlying problem is instead that we don't have
a pattern for (zero_extend:DI (not:SI ...)).  Adding that would
definitely be a better fix.

> However, I agree that the restricted case where the else arm is (const_int 0) 
> is
> a worthwhile optimization (that wasn't caught by the previous patterns due to
> const_ints never being zero_extended as you mentioned). I've added a pattern 
> and
> tests for this case in the updated patch.

Looks good, just a couple of minor things:

> 2020-04-30  Alex Coplan  
>
>   * config/aarch64/aarch64.md (*csinv3_utxw_insn1): New.
> (*csinv3_uxtw_insn2): New.
> (*csinv3_uxtw_insn3): New.

ChangeLog trivia, but these last two lines should only be indented by a tab.

Hopefully one day we'll finally ditch this format and stop having to
quibble over such minor formatting stuff...

> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index c7c4d1dd519..37d651724ad 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -4390,6 +4390,45 @@
>[(set_attr "type" "csel")]
>  )
>  
> +(define_insn "*csinv3_uxtw_insn1"
> +  [(set (match_operand:DI 0 "register_operand" "=r")
> +(if_then_else:DI
> +  (match_operand 1 "aarch64_comparison_operation" "")
> +  (zero_extend:DI
> +(match_operand:SI 2 "register_operand" "r"))
> +  (zero_extend:DI
> +(NEG_NOT:SI (match_operand:SI 3 "register_operand" "r")]
> +  ""
> +  "cs\\t%w0, %w2, %w3, %m1"
> +  [(set_attr "type" "csel")]
> +)
> +
> +(define_insn "*csinv3_uxtw_insn2"
> +  [(set (match_operand:DI 0 "register_operand" "=r")
> +(if_then_else:DI
> +  (match_operand 1 "aarch64_comparison_operation" "")
> +  (zero_extend:DI
> +(NEG_NOT:SI (match_operand:SI 2 "register_operand" "r")))
> +  (zero_extend:DI
> +(match_operand:SI 3 "register_operand" "r"]
> +  ""
> +  "cs\\t%w0, %w3, %w2, %M1"
> +  [(set_attr "type" "csel")]
> +)
> +
> +(define_insn "*csinv3_uxtw_insn3"
> +  [(set (match_operand:DI 0 "register_operand" "=r")
> +(if_then_else:DI
> +  (match_operand 1 "aarch64_comparison_operation" "")
> +  (zero_extend:DI
> +(NEG_NOT:SI (match_operand:SI 2 "register_operand" "r")))
> +  (const_int 0)))]
> +  ""
> +  "cs\\t%w0, wzr, %w2, %M1"
> +  [(set_attr "type" "csel")]
> +)
> +
> +

Usually there's just one blank line between patterns, even if the
patterns aren't naturally grouped.

No need to repost just for that.  I'll push with those changes once
stage 1 opens.

Thanks,
Richard

[committed] diagnostics: Fix spelling in comment

[Jonathan Wakely via Gcc-patches]

gcc/ChangeLog:

* pretty-print.c (pp_take_prefix): Fix spelling in comment.

Committed to master as obvious.


commit 04e88369a7d95492efccf8f527d27cca74664ea7
Author: Jonathan Wakely 
Date:   Thu Apr 30 14:42:24 2020 +0100

diagnostics: Fix spelling in comment

gcc/ChangeLog:

* pretty-print.c (pp_take_prefix): Fix spelling in comment.

diff --git a/gcc/pretty-print.c b/gcc/pretty-print.c
index d0dd9cbd416..407f7300dfb 100644
--- a/gcc/pretty-print.c
+++ b/gcc/pretty-print.c
@@ -1595,7 +1595,7 @@ pp_set_prefix (pretty_printer *pp, char *prefix)
 }
 
 /* Take ownership of PP's prefix, setting it to NULL.
-   This allows clients to save, overide, and then restore an existing
+   This allows clients to save, override, and then restore an existing
prefix, without it being free-ed.  */
 
 char *

Re: [PR c+ 94827]: template parm with default requires

[Nathan Sidwell]

On 4/29/20 2:50 PM, Nathan Sidwell wrote:

Jason,
this is the patch you suggested, as I understood it.  I kept 
finish_nested_require's saving of the (converted) 
current_template_parms, becase of the comment about use in diagnostics.




this extended test that tries to call the function ices with the patch. 
So not there yet.


nathan

--
Nathan Sidwell
template 
void foo(T) {}

int main() {
  foo('4');
}

Re: [PATCH] coroutines: Fix handling of artificial vars [PR94886]

[Nathan Sidwell]

On 4/30/20 9:24 AM, Iain Sandoe wrote:

Hi

Another case found when building the folly experimental coros stuff.
tested on x86_64-darwin so far,
OK for master after testing on x86_64-linux?
thanks
Iain

The testcase ICEs because the range-based for generates three
artificial variables that need to be allocated to the coroutine
frame but, when walking the BIND_EXR that contains these, the
DECL_INITIAL for one of them refers to an entry appearing later,
which means that the frame entry hasn't been allocated when that
INITIAL is walked.


Hm, the error seems to be the range for's construction, did you look at 
altering that so that such forward references do not occur? (range for 
is described as a src->src xform, so I'm sure it's possible to xform it 
into something a user could have written, which a forward-reference is not)


your patch is ok, if that for change's not simple.



The solution is to defer walking the DECL_INITIAL/SIZE etc. until
all the BIND_EXPR vars have been processed.

gcc/cp/ChangeLog:

2020-04-30  Iain Sandoe  

PR c++/94886
* coroutines.cc (transform_local_var_uses): Defer walking
the DECL_INITIALs of BIND_EXPR vars until all the frame
allocations have been made.

gcc/testsuite/ChangeLog:

2020-04-30  Iain Sandoe  

PR c++/94886
* g++.dg/coroutines/pr94886-folly-3.C: New test.
---
  gcc/cp/coroutines.cc  | 21 ---
  .../g++.dg/coroutines/pr94886-folly-3.C   | 15 +
  2 files changed, 28 insertions(+), 8 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/coroutines/pr94886-folly-3.C

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 7bb3e98fe6c..1bc4bf2e45b 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -1813,14 +1813,6 @@ transform_local_var_uses (tree *stmt, int *do_subtree, 
void *d)
  /* Re-write the variable's context to be in the actor func.  */
  DECL_CONTEXT (lvar) = lvd->context;
  
-	  /* we need to walk some of the decl trees, which might contain

-references to vars replaced at a higher level.  */
- cp_walk_tree (_INITIAL (lvar), transform_local_var_uses, d,
-   NULL);
- cp_walk_tree (_SIZE (lvar), transform_local_var_uses, d, NULL);
- cp_walk_tree (_SIZE_UNIT (lvar), transform_local_var_uses, d,
-   NULL);
-
/* For capture proxies, this could include the decl value expr.  */
if (local_var.is_lambda_capture || local_var.has_value_expr_p)
  {
@@ -1842,6 +1834,19 @@ transform_local_var_uses (tree *stmt, int *do_subtree, 
void *d)
 lvd->actor_frame, fld_ref, NULL_TREE);
  local_var.field_idx = fld_idx;
}
+  /* Now we've built the revised var in the frame, substitute uses of
+it in initializers and the bind expr body.  */
+  for (lvar = BIND_EXPR_VARS (*stmt); lvar != NULL;
+  lvar = DECL_CHAIN (lvar))
+   {
+ /* we need to walk some of the decl trees, which might contain
+references to vars replaced at a higher level.  */
+ cp_walk_tree (_INITIAL (lvar), transform_local_var_uses, d,
+   NULL);
+ cp_walk_tree (_SIZE (lvar), transform_local_var_uses, d, NULL);
+ cp_walk_tree (_SIZE_UNIT (lvar), transform_local_var_uses, d,
+   NULL);
+   }
cp_walk_tree (_EXPR_BODY (*stmt), transform_local_var_uses, d, 
NULL);
  
/* Now we have processed and removed references to the original vars,

diff --git a/gcc/testsuite/g++.dg/coroutines/pr94886-folly-3.C 
b/gcc/testsuite/g++.dg/coroutines/pr94886-folly-3.C
new file mode 100644
index 000..d7bd2c17d4a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr94886-folly-3.C
@@ -0,0 +1,15 @@
+
+#include "coro.h"
+#include "coro1-ret-int-yield-int.h"
+
+#include 
+
+coro1
+my_coro ()
+{
+  const std::array expectedValues = {{0, 3, 1, 4, 2}};
+
+  for (int expectedValue : expectedValues) {
+co_yield expectedValue;
+  }
+}




--
Nathan Sidwell

Re: [PATCH] coroutines: Fix handling of target cleanup exprs [PR94883]

[Nathan Sidwell]

On 4/30/20 8:53 AM, Iain Sandoe wrote:


The problem here is that target cleanup expressions have been
added to the initialisers for the awaitable (and returns of
non-trivial values from await_suspend() calls).  This is because
the expansion of the co_await into its control flow is not
apparent to the machinery adding the target cleanup expressions.
The solution being tested is simply to recreate target expressions
as the co_awaits are lowered.  Teaching the machinery to handle
walking co_await expressions in different ways at different points
(outside the coroutine transformation) seems overly complex.


ok


--
Nathan Sidwell

Re: [PATCH] coroutines: Fix cases where proxy variables are used [PR94879]

[Nathan Sidwell]

On 4/30/20 8:33 AM, Iain Sandoe wrote:


There are several places where the handling of a variable
declaration depends on whether it corresponds to a compiler
temporary, or to some other entity.  We were testing that var
decls were artificial in determining this.  However, proxy vars
are also artificial so that this is not sufficient.  The solution
is to exclude variables with a DECL_VALUE_EXPR as well, since
the value variable will not be a temporary.


Ok.  This approach seems rather brittle, but it is what we have.


gcc/cp/ChangeLog:

2020-04-30  Iain Sandoe  

PR c++/94879
* coroutines.cc (build_co_await): Account for variables
with DECL_VALUE_EXPRs.
(captures_temporary): Likewise.
(register_awaits): Likewise.

gcc/testsuite/ChangeLog:

2020-04-30  Iain Sandoe  

PR c++/94879
* g++.dg/coroutines/pr94879-folly-1.C: New test.

---
  gcc/cp/coroutines.cc  |  9 ++--
  .../g++.dg/coroutines/pr94879-folly-1.C   | 49 +++
  2 files changed, 55 insertions(+), 3 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/coroutines/pr94879-folly-1.C

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 7bb3e98fe6c..e2dbeabf48b 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -748,7 +748,8 @@ build_co_await (location_t loc, tree a, suspend_point_kind 
suspend_kind)
if (INDIRECT_REF_P (e_proxy))
  e_proxy = TREE_OPERAND (e_proxy, 0);
if (TREE_CODE (e_proxy) == PARM_DECL
-  || (TREE_CODE (e_proxy) == VAR_DECL && !DECL_ARTIFICIAL (e_proxy)))
+  || (VAR_P (e_proxy) && (!DECL_ARTIFICIAL (e_proxy)
+ || DECL_HAS_VALUE_EXPR_P (e_proxy
  e_proxy = o;
else
  {
@@ -2659,7 +2660,8 @@ captures_temporary (tree *stmt, int *do_subtree, void *d)
}
  
/* This isn't a temporary.  */

-  if ((TREE_CODE (parm) == VAR_DECL && !DECL_ARTIFICIAL (parm))
+  if ((VAR_P (parm)
+  && (!DECL_ARTIFICIAL (parm) || DECL_HAS_VALUE_EXPR_P (parm)))
  || TREE_CODE (parm) == PARM_DECL
  || TREE_CODE (parm) == NON_LVALUE_EXPR)
continue;
@@ -2742,7 +2744,8 @@ register_awaits (tree *stmt, int *do_subtree 
ATTRIBUTE_UNUSED, void *d)
if (INDIRECT_REF_P (aw))
  aw = TREE_OPERAND (aw, 0);
if (TREE_CODE (aw) == PARM_DECL
-  || (TREE_CODE (aw) == VAR_DECL && !DECL_ARTIFICIAL (aw)))
+  || (VAR_P (aw) && (!DECL_ARTIFICIAL (aw)
+|| DECL_HAS_VALUE_EXPR_P (aw
  ; /* Don't make an additional copy.  */
else
  {
diff --git a/gcc/testsuite/g++.dg/coroutines/pr94879-folly-1.C 
b/gcc/testsuite/g++.dg/coroutines/pr94879-folly-1.C
new file mode 100644
index 000..7d66ce004f7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr94879-folly-1.C
@@ -0,0 +1,49 @@
+//  { dg-additional-options  "-fpreprocessed -w" }
+
+namespace std {
+template  a b(a &&);
+template  struct d { c e; };
+template  struct coroutine_traits : f {};
+template  struct coroutine_handle;
+template <> struct coroutine_handle<> {};
+template  struct coroutine_handle : coroutine_handle<> {};
+struct g {};
+} // namespace std
+
+class h {};
+class i {
+  i(i &&);
+};
+
+namespace ac {
+template  class ad {
+public:
+  bool await_ready();
+  void await_resume();
+  void await_suspend(std::coroutine_handle<>);
+  i ae;
+};
+} // namespace ac
+
+template  ac::ad operator co_await(ab);
+class j {
+  class l {};
+
+public:
+  std::g initial_suspend();
+  l final_suspend();
+};
+class m : public j {
+public:
+  void get_return_object();
+  void unhandled_exception();
+};
+class n {
+public:
+  using promise_type = m;
+};
+std::d k;
+void a() {
+  auto am = k;
+  [&]() -> n { co_await std::b(am.e); };
+}




--
Nathan Sidwell

RE: [PATCH] aarch64: prefer using csinv, csneg in zero extend contexts

[Alex Coplan]

Hi Richard,

Many thanks for the detailed review. I've attached an updated patch based on
your comments (bootstrapped and regtested on aarch64-linux).

> -Original Message-
> From: Richard Sandiford 
> Sent: 29 April 2020 17:16
> To: Alex Coplan 
> Cc: gcc-patches@gcc.gnu.org; Richard Earnshaw ;
> Marcus Shawcroft ; Kyrylo Tkachov
> ; nd 
> Subject: Re: [PATCH] aarch64: prefer using csinv, csneg in zero extend
> contexts
> 
> Yeah.  The thing that surprised me was that the non-extending form
> has the operator in the "then" arm of the if_then_else:
> 
> (define_insn "*csinv3_insn"
>   [(set (match_operand:GPI 0 "register_operand" "=r")
> (if_then_else:GPI
> (match_operand 1 "aarch64_comparison_operation" "")
> (not:GPI (match_operand:GPI 2 "register_operand" "r"))
> (match_operand:GPI 3 "aarch64_reg_or_zero" "rZ")))]
>   ""
>   "csinv\\t%0, %3, %2, %M1"
>   [(set_attr "type" "csel")]
> )
> 
> whereas the new pattern has it in the "else" arm.  I think for the
> non-extending form, having the operator in the "then" arm really is
> canonical and close to guaranteed, since that's how ifcvt processes
> half diamonds.
> 
> But when both values are zero-extended, ifcvt has to convert a full
> diamond, and I'm not sure we can rely on the two sides coming out in
> a particular order.  I think the two ?: orders above work with one
> pattern
> thanks to simplifications that happen before entering gimple.  If instead
> the operator is split out into a separate statement:
> 
> unsigned long long
> inv1(int a, unsigned b, unsigned c)
> {
>   unsigned d = ~c;
>   return a ? b : d;
> }
> 
> then we go back to using the less optimal CSEL sequence.
> 
> So I think it would be worth having a second pattern for the opposite
> order.

Agreed. It did seem a bit magical that we would get the other case for free. I
added this function to the tests and observed that we were getting the less
optimal CSEL sequence until I added the rule with the NEG_NOT on the other arm.

> > diff --git a/gcc/config/aarch64/aarch64.md
> b/gcc/config/aarch64/aarch64.md
> > index c7c4d1dd519..2f7367c0b1a 100644
> > --- a/gcc/config/aarch64/aarch64.md
> > +++ b/gcc/config/aarch64/aarch64.md
> > @@ -4390,6 +4390,19 @@
> >[(set_attr "type" "csel")]
> >  )
> >
> > +(define_insn "*csinv3_uxtw_insn"
> > +  [(set (match_operand:DI 0 "register_operand" "=r")
> > +(if_then_else:DI
> > +  (match_operand 1 "aarch64_comparison_operation" "")
> > +  (zero_extend:DI
> > +(match_operand:SI 2 "aarch64_reg_or_zero" "rZ"))
> > +  (zero_extend:DI
> > +(NEG_NOT:SI (match_operand:SI 3 "register_operand"
> "r")]
> > +  ""
> > +  "cs\\t%w0, %w2, %w3, %m1"
> > +  [(set_attr "type" "csel")]
> > +)
> > +
> 
> The operand to a zero_extend can never be a const_int, so operand 2
> should just be a register_operand with an "r" constraint.

OK, makes sense. I've fixed this in the updated patch.

> At the risk of feature creep :-) a useful third pattern could be
> to combine a zero-extended operator result with an existing DImode value.
> In that case, the existing DImode value really can be "rZ" and should
> always be in the "else" arm of the if_then_else.  E.g.:
> 
> unsigned long long
> f(int a, unsigned long b, unsigned c)
> {
>   return a ? b : ~c;
> }
> 
> unsigned long long
> g(int a, unsigned c)
> {
>   return a ? 0 : ~c;
> }
> 
> But that's definitely something that can be left for later.

Hm. What sequence would you like to see for the function f here with the
argument already in DImode? I don't think we can do it with just a CMP + CSINV
like in the other test cases because you end up wanting to access b as x1 and c
as w2 which is not allowed.

Indeed, clang generates a MVN + CSEL sequence where the CSEL operates on the
64-bit registers:

f:
mvn w8, w2
cmp w0, #0
cselx0, x8, x1, eq
ret

However, I agree that the restricted case where the else arm is (const_int 0) is
a worthwhile optimization (that wasn't caught by the previous patterns due to
const_ints never being zero_extended as you mentioned). I've added a pattern and
tests for this case in the updated patch.

Many thanks,
Alex

---

gcc/ChangeLog:

2020-04-30  Alex Coplan  

* config/aarch64/aarch64.md (*csinv3_utxw_insn1): New.
  (*csinv3_uxtw_insn2): New.
  (*csinv3_uxtw_insn3): New.
* config/aarch64/iterators.md (neg_not_cs): New.

gcc/testsuite/ChangeLog:

2020-04-30  Alex Coplan  

* gcc.target/aarch64/csinv-neg.c: New test.
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index c7c4d1dd519..37d651724ad 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -4390,6 +4390,45 @@
   [(set_attr "type" "csel")]
 )
 
+(define_insn "*csinv3_uxtw_insn1"
+  [(set (match_operand:DI 0 "register_operand" "=r")
+(if_then_else:DI
+  (match_operand 1 

[PATCH] coroutines: Fix handling of artificial vars [PR94886]

[Iain Sandoe]

Hi

Another case found when building the folly experimental coros stuff.
tested on x86_64-darwin so far, 
OK for master after testing on x86_64-linux?
thanks
Iain

The testcase ICEs because the range-based for generates three
artificial variables that need to be allocated to the coroutine
frame but, when walking the BIND_EXR that contains these, the
DECL_INITIAL for one of them refers to an entry appearing later,
which means that the frame entry hasn't been allocated when that
INITIAL is walked.

The solution is to defer walking the DECL_INITIAL/SIZE etc. until
all the BIND_EXPR vars have been processed.

gcc/cp/ChangeLog:

2020-04-30  Iain Sandoe  

PR c++/94886
* coroutines.cc (transform_local_var_uses): Defer walking
the DECL_INITIALs of BIND_EXPR vars until all the frame
allocations have been made.

gcc/testsuite/ChangeLog:

2020-04-30  Iain Sandoe  

PR c++/94886
* g++.dg/coroutines/pr94886-folly-3.C: New test.
---
 gcc/cp/coroutines.cc  | 21 ---
 .../g++.dg/coroutines/pr94886-folly-3.C   | 15 +
 2 files changed, 28 insertions(+), 8 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/coroutines/pr94886-folly-3.C

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 7bb3e98fe6c..1bc4bf2e45b 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -1813,14 +1813,6 @@ transform_local_var_uses (tree *stmt, int *do_subtree, 
void *d)
  /* Re-write the variable's context to be in the actor func.  */
  DECL_CONTEXT (lvar) = lvd->context;
 
- /* we need to walk some of the decl trees, which might contain
-references to vars replaced at a higher level.  */
- cp_walk_tree (_INITIAL (lvar), transform_local_var_uses, d,
-   NULL);
- cp_walk_tree (_SIZE (lvar), transform_local_var_uses, d, NULL);
- cp_walk_tree (_SIZE_UNIT (lvar), transform_local_var_uses, d,
-   NULL);
-
/* For capture proxies, this could include the decl value expr.  */
if (local_var.is_lambda_capture || local_var.has_value_expr_p)
  {
@@ -1842,6 +1834,19 @@ transform_local_var_uses (tree *stmt, int *do_subtree, 
void *d)
 lvd->actor_frame, fld_ref, NULL_TREE);
  local_var.field_idx = fld_idx;
}
+  /* Now we've built the revised var in the frame, substitute uses of
+it in initializers and the bind expr body.  */
+  for (lvar = BIND_EXPR_VARS (*stmt); lvar != NULL;
+  lvar = DECL_CHAIN (lvar))
+   {
+ /* we need to walk some of the decl trees, which might contain
+references to vars replaced at a higher level.  */
+ cp_walk_tree (_INITIAL (lvar), transform_local_var_uses, d,
+   NULL);
+ cp_walk_tree (_SIZE (lvar), transform_local_var_uses, d, NULL);
+ cp_walk_tree (_SIZE_UNIT (lvar), transform_local_var_uses, d,
+   NULL);
+   }
   cp_walk_tree (_EXPR_BODY (*stmt), transform_local_var_uses, d, 
NULL);
 
   /* Now we have processed and removed references to the original vars,
diff --git a/gcc/testsuite/g++.dg/coroutines/pr94886-folly-3.C 
b/gcc/testsuite/g++.dg/coroutines/pr94886-folly-3.C
new file mode 100644
index 000..d7bd2c17d4a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr94886-folly-3.C
@@ -0,0 +1,15 @@
+
+#include "coro.h"
+#include "coro1-ret-int-yield-int.h"
+
+#include 
+
+coro1
+my_coro ()
+{
+  const std::array expectedValues = {{0, 3, 1, 4, 2}};
+
+  for (int expectedValue : expectedValues) {
+co_yield expectedValue;
+  }
+}
-- 
2.24.1

Re: [PATCH] wwwdocs: Added mentioning of TX3 chip to the list of the processors

[Anton Youdkevitch]

On 30.4.2020 13:05 , Kyrylo Tkachov wrote:



-Original Message-
From: Gerald Pfeifer 
Sent: 30 April 2020 10:53
To: Kyrylo Tkachov 
Cc: Anton Youdkevitch ; gcc-
patc...@gcc.gnu.org; Joseph S. Myers 
Subject: RE: [PATCH] wwwdocs: Added mentioning of TX3 chip to the list of
the processors

On Thu, 30 Apr 2020, Kyrylo Tkachov wrote:

Ok.

Ah, and you also pushed it in Git.  (I was just going to do it
since Anton does not appear to have an account, and noticed it
was a no-op. ;-)

Ah yes, sorry. I was initially going to leave it to somebody else, but then 
found the time after all.
Kyrill

Thanks a lot, Kyrill!

Re: [PATCH] match.pd: Optimize (x < 0) != (y < 0) into (x ^ y) < 0 [PR94718]

[Richard Biener]

On Thu, 30 Apr 2020, Jakub Jelinek wrote:

> Hi!
> 
> The following patch (on top of the two other PR94718 patches) performs the
> actual optimization requested in the PR.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for stage1?
> 
> 2020-04-30  Jakub Jelinek  
> 
>   PR tree-optimization/94718
>   * match.pd ((X < 0) != (Y < 0) into (X ^ Y) < 0): New simplification.
> 
>   * gcc.dg/tree-ssa/pr94718-4.c: New test.
>   * gcc.dg/tree-ssa/pr94718-5.c: New test.
> 
> --- gcc/match.pd.jj   2020-04-30 12:43:10.473982154 +0200
> +++ gcc/match.pd  2020-04-30 13:10:11.305134082 +0200
> @@ -4358,6 +4358,28 @@ (define_operator_list COND_TERNARY
>(cmp (bit_and:cs @0 @2) (bit_and:cs @1 @2))
>(cmp (bit_and (bit_xor @0 @1) @2) { build_zero_cst (TREE_TYPE (@2)); })))
>  
> +/* (X < 0) != (Y < 0) into (X ^ Y) < 0.
> +   (X >= 0) != (Y >= 0) into (X ^ Y) < 0.
> +   (X < 0) == (Y < 0) into (X ^ Y) >= 0.
> +   (X >= 0) == (Y >= 0) into (X ^ Y) >= 0.  */
> +(for cmp (eq ne)
> + ncmp (ge lt)
> + (for sgncmp (ge lt)
> +  (simplify
> +   (cmp (sgncmp @0 integer_zerop@2) (sgncmp @1 integer_zerop))
> +   (if (ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0))
> + && !TYPE_UNSIGNED (TREE_TYPE (@0))
> + && types_match (@0, @1))
> +(ncmp (bit_xor @0 @1) @2)

Can you also add a comment for the ones below?  OK with that change.

> +(for cmp (eq ne)
> + ncmp (lt ge)
> + (simplify
> +  (cmp:c (lt @0 integer_zerop@2) (ge @1 integer_zerop))
> +   (if (ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0))
> + && !TYPE_UNSIGNED (TREE_TYPE (@0))
> + && types_match (@0, @1))
> +(ncmp (bit_xor @0 @1) @2
> +
>  /* If we have (A & C) == C where C is a power of 2, convert this into
> (A & C) != 0.  Similarly for NE_EXPR.  */
>  (for cmp (eq ne)
> --- gcc/testsuite/gcc.dg/tree-ssa/pr94718-4.c.jj  2020-04-30 
> 12:56:00.552606387 +0200
> +++ gcc/testsuite/gcc.dg/tree-ssa/pr94718-4.c 2020-04-30 13:12:57.320699373 
> +0200
> @@ -0,0 +1,61 @@
> +/* PR tree-optimization/94718 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fno-ipa-icf -fdump-tree-optimized" } */
> +/* { dg-final { scan-tree-dump-times "= \[xy]_\[0-9]+\\\(D\\\) \\^ 
> \[xy]_\[0-9]+\\\(D\\\);" 8 "optimized" } } */
> +/* { dg-final { scan-tree-dump-times "\[0-9]+ < 0;" 8 "optimized" } } */
> +
> +int
> +f1 (int x, int y)
> +{
> +  return (x < 0) != (y < 0);
> +}
> +
> +int
> +f2 (int x, int y)
> +{
> +  return (x >= 0) != (y >= 0);
> +}
> +
> +int
> +f3 (int x, int y)
> +{
> +  return (x < 0) == (y >= 0);
> +}
> +
> +int
> +f4 (int x, int y)
> +{
> +  return (x >= 0) == (y < 0);
> +}
> +
> +int
> +f5 (int x, int y)
> +{
> +  int s = (x < 0);
> +  int t = (y < 0);
> +  return s != t;
> +}
> +
> +int
> +f6 (int x, int y)
> +{
> +  int s = (x >= 0);
> +  int t = (y >= 0);
> +  return s != t;
> +}
> +
> +int
> +f7 (int x, int y)
> +{
> +  int s = (x < 0);
> +  int t = (y >= 0);
> +  return s == t;
> +}
> +
> +int
> +f8 (int x, int y)
> +{
> +  int s = (x >= 0);
> +  int t = (y < 0);
> +  return s == t;
> +}
> --- gcc/testsuite/gcc.dg/tree-ssa/pr94718-5.c.jj  2020-04-30 
> 13:01:44.893558240 +0200
> +++ gcc/testsuite/gcc.dg/tree-ssa/pr94718-5.c 2020-04-30 13:14:00.434768470 
> +0200
> @@ -0,0 +1,61 @@
> +/* PR tree-optimization/94718 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fno-ipa-icf -fdump-tree-optimized" } */
> +/* { dg-final { scan-tree-dump-times "= \[xy]_\[0-9]+\\\(D\\\) \\^ 
> \[xy]_\[0-9]+\\\(D\\\);" 8 "optimized" } } */
> +/* { dg-final { scan-tree-dump-times "\[0-9]+ >= 0;" 8 "optimized" } } */
> +
> +int
> +f1 (int x, int y)
> +{
> +  return (x < 0) == (y < 0);
> +}
> +
> +int
> +f2 (int x, int y)
> +{
> +  return (x >= 0) == (y >= 0);
> +}
> +
> +int
> +f3 (int x, int y)
> +{
> +  return (x < 0) != (y >= 0);
> +}
> +
> +int
> +f4 (int x, int y)
> +{
> +  return (x >= 0) != (y < 0);
> +}
> +
> +int
> +f5 (int x, int y)
> +{
> +  int s = (x < 0);
> +  int t = (y < 0);
> +  return s == t;
> +}
> +
> +int
> +f6 (int x, int y)
> +{
> +  int s = (x >= 0);
> +  int t = (y >= 0);
> +  return s == t;
> +}
> +
> +int
> +f7 (int x, int y)
> +{
> +  int s = (x < 0);
> +  int t = (y >= 0);
> +  return s != t;
> +}
> +
> +int
> +f8 (int x, int y)
> +{
> +  int s = (x >= 0);
> +  int t = (y < 0);
> +  return s != t;
> +}
> 
>   Jakub
> 
> 

-- 
Richard Biener 
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)

[PATCH] match.pd: Optimize (x < 0) != (y < 0) into (x ^ y) < 0 [PR94718]

[Jakub Jelinek via Gcc-patches]

Hi!

The following patch (on top of the two other PR94718 patches) performs the
actual optimization requested in the PR.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for stage1?

2020-04-30  Jakub Jelinek  

PR tree-optimization/94718
* match.pd ((X < 0) != (Y < 0) into (X ^ Y) < 0): New simplification.

* gcc.dg/tree-ssa/pr94718-4.c: New test.
* gcc.dg/tree-ssa/pr94718-5.c: New test.

--- gcc/match.pd.jj 2020-04-30 12:43:10.473982154 +0200
+++ gcc/match.pd2020-04-30 13:10:11.305134082 +0200
@@ -4358,6 +4358,28 @@ (define_operator_list COND_TERNARY
   (cmp (bit_and:cs @0 @2) (bit_and:cs @1 @2))
   (cmp (bit_and (bit_xor @0 @1) @2) { build_zero_cst (TREE_TYPE (@2)); })))
 
+/* (X < 0) != (Y < 0) into (X ^ Y) < 0.
+   (X >= 0) != (Y >= 0) into (X ^ Y) < 0.
+   (X < 0) == (Y < 0) into (X ^ Y) >= 0.
+   (X >= 0) == (Y >= 0) into (X ^ Y) >= 0.  */
+(for cmp (eq ne)
+ ncmp (ge lt)
+ (for sgncmp (ge lt)
+  (simplify
+   (cmp (sgncmp @0 integer_zerop@2) (sgncmp @1 integer_zerop))
+   (if (ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0))
+   && !TYPE_UNSIGNED (TREE_TYPE (@0))
+   && types_match (@0, @1))
+(ncmp (bit_xor @0 @1) @2)
+(for cmp (eq ne)
+ ncmp (lt ge)
+ (simplify
+  (cmp:c (lt @0 integer_zerop@2) (ge @1 integer_zerop))
+   (if (ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0))
+   && !TYPE_UNSIGNED (TREE_TYPE (@0))
+   && types_match (@0, @1))
+(ncmp (bit_xor @0 @1) @2
+
 /* If we have (A & C) == C where C is a power of 2, convert this into
(A & C) != 0.  Similarly for NE_EXPR.  */
 (for cmp (eq ne)
--- gcc/testsuite/gcc.dg/tree-ssa/pr94718-4.c.jj2020-04-30 
12:56:00.552606387 +0200
+++ gcc/testsuite/gcc.dg/tree-ssa/pr94718-4.c   2020-04-30 13:12:57.320699373 
+0200
@@ -0,0 +1,61 @@
+/* PR tree-optimization/94718 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fno-ipa-icf -fdump-tree-optimized" } */
+/* { dg-final { scan-tree-dump-times "= \[xy]_\[0-9]+\\\(D\\\) \\^ 
\[xy]_\[0-9]+\\\(D\\\);" 8 "optimized" } } */
+/* { dg-final { scan-tree-dump-times "\[0-9]+ < 0;" 8 "optimized" } } */
+
+int
+f1 (int x, int y)
+{
+  return (x < 0) != (y < 0);
+}
+
+int
+f2 (int x, int y)
+{
+  return (x >= 0) != (y >= 0);
+}
+
+int
+f3 (int x, int y)
+{
+  return (x < 0) == (y >= 0);
+}
+
+int
+f4 (int x, int y)
+{
+  return (x >= 0) == (y < 0);
+}
+
+int
+f5 (int x, int y)
+{
+  int s = (x < 0);
+  int t = (y < 0);
+  return s != t;
+}
+
+int
+f6 (int x, int y)
+{
+  int s = (x >= 0);
+  int t = (y >= 0);
+  return s != t;
+}
+
+int
+f7 (int x, int y)
+{
+  int s = (x < 0);
+  int t = (y >= 0);
+  return s == t;
+}
+
+int
+f8 (int x, int y)
+{
+  int s = (x >= 0);
+  int t = (y < 0);
+  return s == t;
+}
--- gcc/testsuite/gcc.dg/tree-ssa/pr94718-5.c.jj2020-04-30 
13:01:44.893558240 +0200
+++ gcc/testsuite/gcc.dg/tree-ssa/pr94718-5.c   2020-04-30 13:14:00.434768470 
+0200
@@ -0,0 +1,61 @@
+/* PR tree-optimization/94718 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fno-ipa-icf -fdump-tree-optimized" } */
+/* { dg-final { scan-tree-dump-times "= \[xy]_\[0-9]+\\\(D\\\) \\^ 
\[xy]_\[0-9]+\\\(D\\\);" 8 "optimized" } } */
+/* { dg-final { scan-tree-dump-times "\[0-9]+ >= 0;" 8 "optimized" } } */
+
+int
+f1 (int x, int y)
+{
+  return (x < 0) == (y < 0);
+}
+
+int
+f2 (int x, int y)
+{
+  return (x >= 0) == (y >= 0);
+}
+
+int
+f3 (int x, int y)
+{
+  return (x < 0) != (y >= 0);
+}
+
+int
+f4 (int x, int y)
+{
+  return (x >= 0) != (y < 0);
+}
+
+int
+f5 (int x, int y)
+{
+  int s = (x < 0);
+  int t = (y < 0);
+  return s == t;
+}
+
+int
+f6 (int x, int y)
+{
+  int s = (x >= 0);
+  int t = (y >= 0);
+  return s == t;
+}
+
+int
+f7 (int x, int y)
+{
+  int s = (x < 0);
+  int t = (y >= 0);
+  return s != t;
+}
+
+int
+f8 (int x, int y)
+{
+  int s = (x >= 0);
+  int t = (y < 0);
+  return s != t;
+}

Jakub

[PATCH] diagnostics: get_option_html_page fixes

[Jakub Jelinek via Gcc-patches]

Hi!

(Sorry to David, apparently I've posted it only privately, not to
gcc-patches, so reposting).

While testing the --with-documentation-root-url= changes, I run into
[Wreturn-type] URL pointing to gfortran documentation where it obviously
isn't documented.  The following patch updates the list of options to match
reality (on the other side -Wconversion-extra is gfortran only option
documented in gfortran.texi).

Or, perhaps better use the attached patch instead, which doesn't have a
hardcoded list and instead uses the flags?  I went through options.c
and the updated list of options matches exactly the cases where CL_Fortran
is set for "-W*" options together with CL_C and/or CL_CXX (ok, there is also
-Wall and -Wextra, but hopefully we don't emit [Wall] or [Wextra] for
anything).

I have successfully bootstrapped/regtested the second patch (CL_C & CL_CXX)
on x86_64-linux and i686-linux, ok for trunk, or do you prefer the first
one?

2020-04-30  Jakub Jelinek  

* opts.c (get_option_html_page): For OPT_Wconversion_extra look in
gfortran documentation rather than in generic, while for
OPT_Wmaybe_uninitialized, OPT_Wpedantic, OPT_Wreturn_type,
OPT_Wuninitialized and OPT_Wunused look in generic documentation
rather than gfortran.

--- gcc/opts.c.jj   2020-04-30 09:37:17.039303011 +0200
+++ gcc/opts.c  2020-04-30 10:27:30.871585618 +0200
@@ -3151,9 +3151,13 @@ get_option_html_page (int option_index)
 
case OPT_Wdate_time:
case OPT_Wconversion:
-   case OPT_Wconversion_extra:
+   case OPT_Wmaybe_uninitialized:
case OPT_Wmissing_include_dirs:
case OPT_Wopenmp_simd:
+   case OPT_Wpedantic:
+   case OPT_Wreturn_type:
+   case OPT_Wuninitialized:
+   case OPT_Wunused:
  /* These warnings are marked in fortran/lang.opt as
 "Documented in C" and thus use the common
 Warning-Options page below.  */


Jakub
2020-04-30  Jakub Jelinek  

* opts.c (get_option_html_page): Instead of hardcoding a list of
options common between C/C++ and Fortran only use gfortran/
documentation for warnings that have CL_Fortran set but not
CL_C or CL_CXX.

--- gcc/opts.c.jj   2020-04-30 09:37:17.039303011 +0200
+++ gcc/opts.c  2020-04-30 10:54:20.753713744 +0200
@@ -3141,25 +3141,17 @@ get_option_html_page (int option_index)
 return "gcc/Static-Analyzer-Options.html";
 
 #ifdef CL_Fortran
-  if (cl_opt->flags & CL_Fortran)
-{
-  switch (option_index)
-   {
-   default:
- /* Most Fortran warnings are documented on this page.  */
- return "gfortran/Error-and-Warning-Options.html";
-
-   case OPT_Wdate_time:
-   case OPT_Wconversion:
-   case OPT_Wconversion_extra:
-   case OPT_Wmissing_include_dirs:
-   case OPT_Wopenmp_simd:
- /* These warnings are marked in fortran/lang.opt as
-"Documented in C" and thus use the common
-Warning-Options page below.  */
- break;
-   }
-}
+  if ((cl_opt->flags & CL_Fortran) != 0
+  /* If it is option common to both C/C++ and Fortran, it is documented
+in gcc/ rather than gfortran/ docs.  */
+#ifdef CL_C
+  && (cl_opt->flags & CL_C) == 0
+#endif
+#ifdef CL_CXX
+  && (cl_opt->flags & CL_CXX) == 0
+#endif
+ )
+return "gfortran/Error-and-Warning-Options.html";
 #endif
 
   return "gcc/Warning-Options.html";

Re: [PATCH] tree: Don't reuse types if TYPE_USER_ALIGN differ [PR94775]

[Marek Polacek via Gcc-patches]

On Thu, Apr 30, 2020 at 08:36:32AM +0200, Richard Biener via Gcc-patches wrote:
> On Wed, Apr 29, 2020 at 11:43 PM Marek Polacek via Gcc-patches
>  wrote:
> >
> > Here we trip on the TYPE_USER_ALIGN (t) assert in strip_typedefs: it
> > gets "const d[0]" with TYPE_USER_ALIGN=0 but the result built by
> > build_cplus_array_type is "const char[0]" with TYPE_USER_ALIGN=1.
> >
> > When we strip_typedefs the element of the array "const d", we see it's
> > a typedef_variant_p, so we look at its DECL_ORIGINAL_TYPE, which is
> > char, but we need to add the const qualifier, so we call
> > cp_build_qualified_type -> build_qualified_type
> > where get_qualified_type checks to see if we already have such a type
> > by walking the variants list, which in this case is:
> >
> >   char -> c -> const char -> const char -> d -> const d
> >
> > Because check_base_type only checks TYPE_ALIGN and not TYPE_USER_ALIGN,
> > we choose the first const char, which has TYPE_USER_ALIGN set.  If the
> > element type of an array has TYPE_USER_ALIGN, the array type gets it too.
> >
> > So we can make check_base_type stricter.  I was afraid that it might make
> > us reuse types less often, but measuring showed that we build the same
> > amount of types with and without the patch, while bootstrapping.
> >
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/9/8?
> 
> Hmm, so while I wonder whether a mismatch in TYPE_USER_ALIGN would
> affect program semantics we should make sure to handle this flag consistently.
> 
> Thus the patch is OK for trunk and branches after a while.

Thanks.

> Note I question that we build new types during diagnostic printing.

This is in the context of printing an 'aka' for a typedef, so we need
strip_typedefs.

> Thanks,
> Richard.
> 
> > PR c++/94775
> > * tree.c (check_base_type): Return true only if TYPE_USER_ALIGN 
> > match.
> > (check_aligned_type): Check if TYPE_USER_ALIGN match.
> >
> > * g++.dg/warn/Warray-bounds-10.C: New test.
> > ---
> >  gcc/testsuite/g++.dg/warn/Warray-bounds-10.C | 40 
> >  gcc/tree.c   |  4 +-
> >  2 files changed, 43 insertions(+), 1 deletion(-)
> >  create mode 100644 gcc/testsuite/g++.dg/warn/Warray-bounds-10.C
> >
> > diff --git a/gcc/testsuite/g++.dg/warn/Warray-bounds-10.C 
> > b/gcc/testsuite/g++.dg/warn/Warray-bounds-10.C
> > new file mode 100644
> > index 000..0a18f637e0e
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/warn/Warray-bounds-10.C
> > @@ -0,0 +1,40 @@
> > +// PR c++/94775
> > +// { dg-do compile { target c++14 } }
> > +// { dg-options "-O2 -Warray-bounds" }
> > +
> > +template  using a = int;
> > +template  using b = int;
> > +typedef char d;
> > +template  using e = int;
> > +template  struct h { using i = b>, e>; };
> > +template  using j = typename h::i;
> > +long ab, k, aj;
> > +const d l[]{};
> > +class m {
> > +public:
> > +  m(int);
> > +};
> > +class n {
> > +  void ad() const;
> > +  template  void o(long) const {
> > +using c __attribute__((aligned(1))) = const ae;
> > +  }
> > +  long p;
> > +  template 
> > +  auto s(unsigned long, unsigned long, unsigned long, unsigned long) const;
> > +  template  auto q(unsigned long, unsigned long) const;
> > +};
> > +template 
> > +auto n::s(unsigned long, unsigned long, unsigned long, unsigned long t) 
> > const {
> > +  o(p);
> > +  return t;
> > +}
> > +template  auto n::q(unsigned long p1, unsigned long p2) const {
> > +  using r = j<4, false>;
> > +  using ai = j<4, g>;
> > +  return s(ab, k, p1, p2);
> > +}
> > +void n::ad() const {
> > +  long f(l[aj]); // { dg-warning "outside array bounds" }
> > +  m(q(8, f));
> > +}
> > diff --git a/gcc/tree.c b/gcc/tree.c
> > index e451401822c..341766c51e5 100644
> > --- a/gcc/tree.c
> > +++ b/gcc/tree.c
> > @@ -6493,7 +6493,8 @@ check_base_type (const_tree cand, const_tree base)
> > TYPE_ATTRIBUTES (base)))
> >  return false;
> >/* Check alignment.  */
> > -  if (TYPE_ALIGN (cand) == TYPE_ALIGN (base))
> > +  if (TYPE_ALIGN (cand) == TYPE_ALIGN (base)
> > +  && TYPE_USER_ALIGN (cand) == TYPE_USER_ALIGN (base))
> >  return true;
> >/* Atomic types increase minimal alignment.  We must to do so as well
> >   or we get duplicated canonical types. See PR88686.  */
> > @@ -6528,6 +6529,7 @@ check_aligned_type (const_tree cand, const_tree base, 
> > unsigned int align)
> >   && TYPE_CONTEXT (cand) == TYPE_CONTEXT (base)
> >   /* Check alignment.  */
> >   && TYPE_ALIGN (cand) == align
> > + && TYPE_USER_ALIGN (cand) == TYPE_USER_ALIGN (base)
> >   && attribute_list_equal (TYPE_ATTRIBUTES (cand),
> >TYPE_ATTRIBUTES (base))
> >   && check_lang_type (cand, base));
> >
> > base-commit: 8f1591763fd50b143af0dc1770741f326a97583a
> > --
> > Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA
> >
> 

[PATCH] c: Fix ICE with _Atomic side-effect in nested fn param decls [PR94842]

[Jakub Jelinek via Gcc-patches]

Hi!

If there are _Atomic side-effects in the parameter declarations
of non-nested function, when they are parsed, current_function_decl is
NULL, the create_artificial_label created labels during build_atomic* are
then adjusted by store_parm_decls through set_labels_context_r callback.
Unfortunately, if such thing happens in nested function parameter
declarations, while those decls are parsed current_function_decl is the
parent function (and am not sure it is a good idea to temporarily clear it,
some code perhaps should be aware it is in a nested function, or it can
refer to variables from the parent function etc.) and that means
store_param_decls through set_labels_context_r doesn't adjust anything.
As those labels are emitted in the nested function body rather than in the
parent, I think it is ok to override the context in those cases.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2020-04-30  Jakub Jelinek  

PR c/94842
* c-decl.c (set_labels_context_r): In addition to context-less
LABEL_DECLs adjust also LABEL_DECLs with context equal to
parent function if any.
(store_parm_decls): Adjust comment.

* gcc.dg/pr94842.c: New test.

--- gcc/c/c-decl.c.jj   2020-04-08 11:59:23.726461457 +0200
+++ gcc/c/c-decl.c  2020-04-30 12:19:35.918946543 +0200
@@ -9722,15 +9722,18 @@ store_parm_decls_from (struct c_arg_info
   store_parm_decls ();
 }
 
-/* Called by walk_tree to look for and update context-less labels.  */
+/* Called by walk_tree to look for and update context-less labels
+   or labels with context in the parent function.  */
 
 static tree
 set_labels_context_r (tree *tp, int *walk_subtrees, void *data)
 {
+  tree ctx = static_cast(data);
   if (TREE_CODE (*tp) == LABEL_EXPR
-  && DECL_CONTEXT (LABEL_EXPR_LABEL (*tp)) == NULL_TREE)
+  && (DECL_CONTEXT (LABEL_EXPR_LABEL (*tp)) == NULL_TREE
+ || DECL_CONTEXT (LABEL_EXPR_LABEL (*tp)) == DECL_CONTEXT (ctx)))
 {
-  DECL_CONTEXT (LABEL_EXPR_LABEL (*tp)) = static_cast(data);
+  DECL_CONTEXT (LABEL_EXPR_LABEL (*tp)) = ctx;
   *walk_subtrees = 0;
 }
 
@@ -9800,7 +9803,11 @@ store_parm_decls (void)
 gotos, labels, etc.  Because at that time the function decl
 for F has not been created yet, those labels do not have any
 function context.  But we have the fndecl now, so update the
-labels accordingly.  gimplify_expr would crash otherwise.  */
+labels accordingly.  gimplify_expr would crash otherwise.
+Or with nested functions the labels could be created with parent
+function's context, while when the statement is emitted at the
+start of the nested function, it needs the nested function's
+context.  */
   walk_tree_without_duplicates (_info->pending_sizes,
set_labels_context_r, fndecl);
   add_stmt (arg_info->pending_sizes);
--- gcc/testsuite/gcc.dg/pr94842.c.jj   2020-04-30 12:25:52.431360645 +0200
+++ gcc/testsuite/gcc.dg/pr94842.c  2020-04-30 12:24:47.522323298 +0200
@@ -0,0 +1,11 @@
+/* PR c/94842 */
+/* { dg-do compile } */
+/* { dg-options "" } */
+
+_Atomic float x = 5;
+
+void
+foo (void)
+{
+  void bar (float y[(int) (x += 2)]) {}
+}

Jakub

[PATCH] coroutines: Fix handling of target cleanup exprs [PR94883]

[Iain Sandoe]

Hi

Another case found building folly’s experimental coros tests.
tested on x86_64-darwin, linux, powerpc64-linux
OK for master?
thanks
Iain



The problem here is that target cleanup expressions have been
added to the initialisers for the awaitable (and returns of
non-trivial values from await_suspend() calls).  This is because
the expansion of the co_await into its control flow is not
apparent to the machinery adding the target cleanup expressions.
The solution being tested is simply to recreate target expressions
as the co_awaits are lowered.  Teaching the machinery to handle
walking co_await expressions in different ways at different points
(outside the coroutine transformation) seems overly complex.

gcc/cp/ChangeLog:

2020-04-30  Iain Sandoe  

PR c++/94883
* coroutines.cc (register_awaits): Update target
expressions for awaitable and suspend handle
initializers.

gcc/testsuite/ChangeLog:

2020-04-30  Iain Sandoe  

PR c++/94883
* g++.dg/coroutines/pr94883-folly-2.C: New test.
---
 gcc/cp/coroutines.cc  | 11 
 .../g++.dg/coroutines/pr948xx-folly-2.C   | 64 +++
 2 files changed, 75 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/coroutines/pr948xx-folly-2.C

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 7bb3e98fe6c..3bff2c7dbdc 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -2754,6 +2754,17 @@ register_awaits (tree *stmt, int *do_subtree 
ATTRIBUTE_UNUSED, void *d)
   free (nam);
 }
 
+  tree o = TREE_OPERAND (aw_expr, 2); /* Initialiser for the frame var.  */
+  /* If this is a target expression, then we need to remake it to strip off
+ any extra cleanups added.  */
+  if (TREE_CODE (o) == TARGET_EXPR)
+TREE_OPERAND (aw_expr, 2) = get_target_expr (TREE_OPERAND (o, 1));
+
+  tree v = TREE_OPERAND (aw_expr, 3);
+  o = TREE_VEC_ELT (v, 1);
+  if (TREE_CODE (o) == TARGET_EXPR)
+TREE_VEC_ELT (v, 1) = get_target_expr (TREE_OPERAND (o, 1));
+
   register_await_info (aw_expr, aw_field_type, aw_field_nam);
 
   /* Count how many awaits the current expression contains.  */
diff --git a/gcc/testsuite/g++.dg/coroutines/pr948xx-folly-2.C 
b/gcc/testsuite/g++.dg/coroutines/pr948xx-folly-2.C
new file mode 100644
index 000..088f1335493
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr948xx-folly-2.C
@@ -0,0 +1,64 @@
+
+namespace std {
+template  struct coroutine_traits : a {};
+template  struct coroutine_handle;
+template <> struct coroutine_handle<> {};
+template  struct coroutine_handle : coroutine_handle<> {};
+struct b {
+  bool await_ready();
+  void await_suspend(coroutine_handle<>);
+  void await_resume();
+};
+} // namespace std
+
+template  auto ab(int ac, d ad) -> decltype(ad.e(ac));
+int f;
+class h {
+  class j {
+  public:
+bool await_ready();
+void await_suspend(std::coroutine_handle<>);
+void await_resume();
+  };
+
+public:
+  void get_return_object();
+  std::b initial_suspend();
+  j final_suspend();
+  void unhandled_exception();
+  template  
+auto await_transform (g c) { return ab(f, c); }
+};
+template  class k {
+public:
+  using promise_type = h;
+  using i = std::coroutine_handle<>;
+  class l {
+  public:
+~l();
+operator bool();
+  };
+  class m {
+  public:
+bool await_ready();
+i await_suspend(std::coroutine_handle<>);
+l await_resume();
+  };
+  class n {
+  public:
+m e(int);
+  };
+  n ah();
+};
+
+template 
+k 
+my_coro (k am, ai) {
+  if (auto an = co_await am.ah())
+;
+}
+
+void foo () {
+  k a;
+  my_coro (a, [] {});
+}
-- 
2.24.1

[PATCH] coroutines: Fix cases where proxy variables are used [PR94879]

[Iain Sandoe]

Hi,

This was found when attempting to build folly’s experimental coros
tests.

tested on x86_64-darwin, linux, powerpc64-linux
ok for master?
thanks
Iain



There are several places where the handling of a variable
declaration depends on whether it corresponds to a compiler
temporary, or to some other entity.  We were testing that var
decls were artificial in determining this.  However, proxy vars
are also artificial so that this is not sufficient.  The solution
is to exclude variables with a DECL_VALUE_EXPR as well, since
the value variable will not be a temporary.

gcc/cp/ChangeLog:

2020-04-30  Iain Sandoe  

PR c++/94879
* coroutines.cc (build_co_await): Account for variables
with DECL_VALUE_EXPRs.
(captures_temporary): Likewise.
(register_awaits): Likewise.

gcc/testsuite/ChangeLog:

2020-04-30  Iain Sandoe  

PR c++/94879
* g++.dg/coroutines/pr94879-folly-1.C: New test.

---
 gcc/cp/coroutines.cc  |  9 ++--
 .../g++.dg/coroutines/pr94879-folly-1.C   | 49 +++
 2 files changed, 55 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/coroutines/pr94879-folly-1.C

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 7bb3e98fe6c..e2dbeabf48b 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -748,7 +748,8 @@ build_co_await (location_t loc, tree a, suspend_point_kind 
suspend_kind)
   if (INDIRECT_REF_P (e_proxy))
 e_proxy = TREE_OPERAND (e_proxy, 0);
   if (TREE_CODE (e_proxy) == PARM_DECL
-  || (TREE_CODE (e_proxy) == VAR_DECL && !DECL_ARTIFICIAL (e_proxy)))
+  || (VAR_P (e_proxy) && (!DECL_ARTIFICIAL (e_proxy)
+ || DECL_HAS_VALUE_EXPR_P (e_proxy
 e_proxy = o;
   else
 {
@@ -2659,7 +2660,8 @@ captures_temporary (tree *stmt, int *do_subtree, void *d)
}
 
   /* This isn't a temporary.  */
-  if ((TREE_CODE (parm) == VAR_DECL && !DECL_ARTIFICIAL (parm))
+  if ((VAR_P (parm)
+  && (!DECL_ARTIFICIAL (parm) || DECL_HAS_VALUE_EXPR_P (parm)))
  || TREE_CODE (parm) == PARM_DECL
  || TREE_CODE (parm) == NON_LVALUE_EXPR)
continue;
@@ -2742,7 +2744,8 @@ register_awaits (tree *stmt, int *do_subtree 
ATTRIBUTE_UNUSED, void *d)
   if (INDIRECT_REF_P (aw))
 aw = TREE_OPERAND (aw, 0);
   if (TREE_CODE (aw) == PARM_DECL
-  || (TREE_CODE (aw) == VAR_DECL && !DECL_ARTIFICIAL (aw)))
+  || (VAR_P (aw) && (!DECL_ARTIFICIAL (aw)
+|| DECL_HAS_VALUE_EXPR_P (aw
 ; /* Don't make an additional copy.  */
   else
 {
diff --git a/gcc/testsuite/g++.dg/coroutines/pr94879-folly-1.C 
b/gcc/testsuite/g++.dg/coroutines/pr94879-folly-1.C
new file mode 100644
index 000..7d66ce004f7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr94879-folly-1.C
@@ -0,0 +1,49 @@
+//  { dg-additional-options  "-fpreprocessed -w" }
+
+namespace std {
+template  a b(a &&);
+template  struct d { c e; };
+template  struct coroutine_traits : f {};
+template  struct coroutine_handle;
+template <> struct coroutine_handle<> {};
+template  struct coroutine_handle : coroutine_handle<> {};
+struct g {};
+} // namespace std
+
+class h {};
+class i {
+  i(i &&);
+};
+
+namespace ac {
+template  class ad {
+public:
+  bool await_ready();
+  void await_resume();
+  void await_suspend(std::coroutine_handle<>);
+  i ae;
+};
+} // namespace ac
+
+template  ac::ad operator co_await(ab);
+class j {
+  class l {};
+
+public:
+  std::g initial_suspend();
+  l final_suspend();
+};
+class m : public j {
+public:
+  void get_return_object();
+  void unhandled_exception();
+};
+class n {
+public:
+  using promise_type = m;
+};
+std::d k;
+void a() {
+  auto am = k;
+  [&]() -> n { co_await std::b(am.e); };
+}
-- 
2.24.1

RE: Should ARMv8-A generic tuning default to -moutline-atomics

[Kyrylo Tkachov]

> -Original Message-
> From: Gcc-patches  On Behalf Of Kyrylo
> Tkachov
> Sent: 30 April 2020 11:57
> To: Andrew Pinski ; Florian Weimer
> 
> Cc: gcc-patches@gcc.gnu.org; nmeye...@amzn.com
> Subject: RE: Should ARMv8-A generic tuning default to -moutline-atomics
> 
> [Moving to gcc-patches]
> 
> > -Original Message-
> > From: Gcc  On Behalf Of Andrew Pinski via Gcc
> > Sent: 30 April 2020 07:21
> > To: Florian Weimer 
> > Cc: GCC Mailing List ; nmeye...@amzn.com
> > Subject: Re: Should ARMv8-A generic tuning default to -moutline-atomics
> >
> > On Wed, Apr 29, 2020 at 6:25 AM Florian Weimer via Gcc 
> > wrote:
> > >
> > > Distributions are receiving requests to build things with
> > > -moutline-atomics:
> > >
> > >   
> > >
> > > Should this be reflected in the GCC upstream defaults for ARMv8-A
> > > generic tuning?  It does not make much sense to me if every distribution
> > > has to overide these flags, either in their build system or by patching
> > > GCC.
> >
> > At least we should make it a configure option.
> > I do want the ability to default it for our (Marvell) toolchain for
> > Linux (our bare metal toolchain will be defaulting to ARMv8.2-a
> > anyways).
> 
> After some internal discussions, I am open to having it on as a default.
> Here are two versions. One has it as a tuning setting that CPUs can override,
> the other just switches it on by default always unless overridden by -mno-
> outline-atomics.
> I slightly prefer the second one as it's cleaner and simpler, but happy to 
> take
> either.
> Any preferences?
> Thanks,
> Kyrill
> 
> ChangeLogs:
> 
> 2020-04-30  Kyrylo Tkachov  
> 
>   * config/aarch64/aarch64-tuning-flags.def (no_outline_atomics):
> Declare.
>   * config/aarch64/aarch64.h (TARGET_OUTLINE_ATOMICS): Define.
>   * config/aarch64/aarch64.opt (moutline-atomics): Change to Int
> variable.
> 
> 2020-04-30  Kyrylo Tkachov  
> 
>   * config/aarch64/aarch64.h (TARGET_OUTLINE_ATOMICS): Define.
>   * config/aarch64/aarch64.opt (moutline-atomics): Change to Int
> variable.
>   * doc/invoke.texi (moutline-atomics): Document as on by default.
> 

I've pushed this second variant after bootstrap and testing on 
aarch64-none-linux-gnu.
Compiled a simple atomic-using testcase with all relevant combinations of 
-moutline-atomics and LSE and no-LSE -march options.
Before the results were (as expected):
 |-moutline-atomics | -mno-outline-atomics | 

LSE  |  inline LSE  | inline LSE   | inline LSE
no-LSE   |  outline | inline LDXR/STXR | outline

Thanks,
Kyrill

Re: Should ARMv8-A generic tuning default to -moutline-atomics

[Richard Earnshaw]

On 30/04/2020 11:56, Kyrylo Tkachov wrote:
> [Moving to gcc-patches]
> 
>> -Original Message-
>> From: Gcc  On Behalf Of Andrew Pinski via Gcc
>> Sent: 30 April 2020 07:21
>> To: Florian Weimer 
>> Cc: GCC Mailing List ; nmeye...@amzn.com
>> Subject: Re: Should ARMv8-A generic tuning default to -moutline-atomics
>>
>> On Wed, Apr 29, 2020 at 6:25 AM Florian Weimer via Gcc 
>> wrote:
>>>
>>> Distributions are receiving requests to build things with
>>> -moutline-atomics:
>>>
>>>   
>>>
>>> Should this be reflected in the GCC upstream defaults for ARMv8-A
>>> generic tuning?  It does not make much sense to me if every distribution
>>> has to overide these flags, either in their build system or by patching
>>> GCC.
>>
>> At least we should make it a configure option.
>> I do want the ability to default it for our (Marvell) toolchain for
>> Linux (our bare metal toolchain will be defaulting to ARMv8.2-a
>> anyways).
> 
> After some internal discussions, I am open to having it on as a default.
> Here are two versions. One has it as a tuning setting that CPUs can override, 
> the other just switches it on by default always unless overridden by 
> -mno-outline-atomics.
> I slightly prefer the second one as it's cleaner and simpler, but happy to 
> take either.
> Any preferences?
> Thanks,
> Kyrill
> 
> ChangeLogs:
> 
> 2020-04-30  Kyrylo Tkachov  
> 
>   * config/aarch64/aarch64-tuning-flags.def (no_outline_atomics): Declare.
>   * config/aarch64/aarch64.h (TARGET_OUTLINE_ATOMICS): Define.
>   * config/aarch64/aarch64.opt (moutline-atomics): Change to Int variable.
> 
> 2020-04-30  Kyrylo Tkachov  
> 
>   * config/aarch64/aarch64.h (TARGET_OUTLINE_ATOMICS): Define.
>   * config/aarch64/aarch64.opt (moutline-atomics): Change to Int variable.
>   * doc/invoke.texi (moutline-atomics): Document as on by default.
> 
> 

I think I prefer the second option.

The whole point of LSE (and hence the name "Large System Extension") was
due to the fact that when you have many cores the v8.0 atomics have
known scaling issues.  It's not a property of the core though, it's
primarily a property of the number of cores in the system.  The problem
really is that we don't have a tuning param for that, nor can we really
tell at compile time how many threads might really be in use.

So I don't think this is really a tuning param that can/should be
selected via the existing -mtune tables.

R.

RE: Should ARMv8-A generic tuning default to -moutline-atomics

[Kyrylo Tkachov]

[Moving to gcc-patches]

> -Original Message-
> From: Gcc  On Behalf Of Andrew Pinski via Gcc
> Sent: 30 April 2020 07:21
> To: Florian Weimer 
> Cc: GCC Mailing List ; nmeye...@amzn.com
> Subject: Re: Should ARMv8-A generic tuning default to -moutline-atomics
> 
> On Wed, Apr 29, 2020 at 6:25 AM Florian Weimer via Gcc 
> wrote:
> >
> > Distributions are receiving requests to build things with
> > -moutline-atomics:
> >
> >   
> >
> > Should this be reflected in the GCC upstream defaults for ARMv8-A
> > generic tuning?  It does not make much sense to me if every distribution
> > has to overide these flags, either in their build system or by patching
> > GCC.
> 
> At least we should make it a configure option.
> I do want the ability to default it for our (Marvell) toolchain for
> Linux (our bare metal toolchain will be defaulting to ARMv8.2-a
> anyways).

After some internal discussions, I am open to having it on as a default.
Here are two versions. One has it as a tuning setting that CPUs can override, 
the other just switches it on by default always unless overridden by 
-mno-outline-atomics.
I slightly prefer the second one as it's cleaner and simpler, but happy to take 
either.
Any preferences?
Thanks,
Kyrill

ChangeLogs:

2020-04-30  Kyrylo Tkachov  

* config/aarch64/aarch64-tuning-flags.def (no_outline_atomics): Declare.
* config/aarch64/aarch64.h (TARGET_OUTLINE_ATOMICS): Define.
* config/aarch64/aarch64.opt (moutline-atomics): Change to Int variable.

2020-04-30  Kyrylo Tkachov  

* config/aarch64/aarch64.h (TARGET_OUTLINE_ATOMICS): Define.
* config/aarch64/aarch64.opt (moutline-atomics): Change to Int variable.
* doc/invoke.texi (moutline-atomics): Document as on by default.




ool-default-tune.patch
Description: ool-default-tune.patch


ool-default-no-tune.patch
Description: ool-default-no-tune.patch

[committed] d: Fix documentation of -defaultlib= and -debuglib=

[Iain Buclaw via Gcc-patches]

Hi,

This patch corrects the documentation of -defaultlib= and -debuglib=.
>From the generated manpages, it was not clear that its usage is
'-debuglib='.

Verified the contents of the generated manpage, committed to mainline.

Regards
Iain.

---
gcc/d/ChangeLog:

* gdc.texi (Options for Linking): Clarify usage of -defaultlib= and
-debuglib= options.
---
 gcc/d/gdc.texi | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/gcc/d/gdc.texi b/gcc/d/gdc.texi
index 496066cb59a..2ce560f3cae 100644
--- a/gcc/d/gdc.texi
+++ b/gcc/d/gdc.texi
@@ -665,14 +665,14 @@ a link step.
 
 @table @gcctabopt
 
-@item -defaultlib @var{libname}
-@cindex @option{-defaultlib}
+@item -defaultlib=@var{libname}
+@cindex @option{-defaultlib=}
 Specify the library to use instead of libphobos when linking.  Options
 specifying the linkage of libphobos, such as @option{-static-libphobos}
 or @option{-shared-libphobos}, are ignored.
 
-@item -debuglib
-@cindex @option{-debuglib}
+@item -debuglib=@var{libname}
+@cindex @option{-debuglib=}
 Specify the debug library to use instead of libphobos when linking.
 This option has no effect unless the @option{-g} option was also given
 on the command line.  Options specifying the linkage of libphobos, such
-- 
2.20.1

[committed] d: Merge upstream dmd 934df6f8c, druntime 7bdd83d7 (PR90719, PR94825)

[Iain Buclaw via Gcc-patches]

Hi,

This patch corrects a previous change made to the SPARC stdc bindings,
and backports PPC-related fixes.  The library and language testsuite now
passes fully on powerpc64le-linux-gnu.

Backported from upstream dmd 934df6f8c, and druntime 7bdd83d7.

Bootstrapped and regression tested on powerpc64le-linux-gnu and
x86_64-linux-gnu.  Committed to mainline.

Regards,
Iain.

libphobos/ChangeLog:

PR d/94825
* libdruntime/Makefile.am (DRUNTIME_SOURCES_CONFIGURED): Remove
config/powerpc/switchcontext.S
* libdruntime/Makefile.in: Regenerate.
* libdruntime/config/powerpc/callwithstack.S: Remove.
* libdruntime/config/powerpc/switchcontext.S: Fix symbol name of
fiber_switchContext.
* libdruntime/core/thread.d: Disable fiber migration tests on PPC.
* testsuite/libphobos.thread/fiber_guard_page.d: Set guardPageSize
same as stackSize.
---
 gcc/d/dmd/MERGE   |   2 +-
 gcc/testsuite/gdc.test/runnable/arrayop.d |   1 +
 .../gdc.test/runnable/ctorpowtests.d  |   1 +
 gcc/testsuite/gdc.test/runnable/template2.d   |   1 +
 gcc/testsuite/gdc.test/runnable/testaa2.d |   1 +
 libphobos/libdruntime/MERGE   |   2 +-
 libphobos/libdruntime/Makefile.am |   3 +-
 libphobos/libdruntime/Makefile.in |  20 +-
 .../config/powerpc/callwithstack.S| 172 --
 .../config/powerpc/switchcontext.S|   8 +-
 libphobos/libdruntime/core/internal/convert.d |  10 +-
 .../libdruntime/core/sys/posix/sys/stat.d |   2 +-
 libphobos/libdruntime/core/thread.d   | 146 ---
 .../libphobos.thread/fiber_guard_page.d   |   2 +-
 14 files changed, 101 insertions(+), 270 deletions(-)
 delete mode 100644 libphobos/libdruntime/config/powerpc/callwithstack.S

diff --git a/gcc/d/dmd/MERGE b/gcc/d/dmd/MERGE
index a2699d39842..82cb6128770 100644
--- a/gcc/d/dmd/MERGE
+++ b/gcc/d/dmd/MERGE
@@ -1,4 +1,4 @@
-06160ccaed7af7955d169024f417c43beb7a8f9f
+934df6f8cf848071dd0312098975f0c13873e01c
 
 The first line of this file holds the git revision number of the last
 merge done from the dlang/dmd repository.
diff --git a/gcc/testsuite/gdc.test/runnable/arrayop.d 
b/gcc/testsuite/gdc.test/runnable/arrayop.d
index e3749bee72a..8c6b083fd30 100644
--- a/gcc/testsuite/gdc.test/runnable/arrayop.d
+++ b/gcc/testsuite/gdc.test/runnable/arrayop.d
@@ -1,3 +1,4 @@
+// RUNNABLE_PHOBOS_TEST
 import std.math;
 
 extern(C) int printf(const char*, ...);
diff --git a/gcc/testsuite/gdc.test/runnable/ctorpowtests.d 
b/gcc/testsuite/gdc.test/runnable/ctorpowtests.d
index b193d3b1e02..1b81a9eb7b4 100644
--- a/gcc/testsuite/gdc.test/runnable/ctorpowtests.d
+++ b/gcc/testsuite/gdc.test/runnable/ctorpowtests.d
@@ -1,3 +1,4 @@
+// RUNNABLE_PHOBOS_TEST
 // PERMUTE_ARGS:
 
 int magicVariable()
diff --git a/gcc/testsuite/gdc.test/runnable/template2.d 
b/gcc/testsuite/gdc.test/runnable/template2.d
index ba5ad13ebd2..3adaeae61ae 100644
--- a/gcc/testsuite/gdc.test/runnable/template2.d
+++ b/gcc/testsuite/gdc.test/runnable/template2.d
@@ -1,3 +1,4 @@
+// RUNNABLE_PHOBOS_TEST
 // original post to the D newsgroup:
 //
http://www.digitalmars.com/pnews/read.php?server=news.digitalmars.com=D=10554
 // Test to manipulate 3D vectors, in D!
diff --git a/gcc/testsuite/gdc.test/runnable/testaa2.d 
b/gcc/testsuite/gdc.test/runnable/testaa2.d
index a8d98c49e9f..d9256039788 100644
--- a/gcc/testsuite/gdc.test/runnable/testaa2.d
+++ b/gcc/testsuite/gdc.test/runnable/testaa2.d
@@ -1,3 +1,4 @@
+// RUNNABLE_PHOBOS_TEST
 // PERMUTE_ARGS:
 
 extern(C) int printf(const char*, ...);
diff --git a/libphobos/libdruntime/MERGE b/libphobos/libdruntime/MERGE
index c6357317ddc..c61ad7ca7ed 100644
--- a/libphobos/libdruntime/MERGE
+++ b/libphobos/libdruntime/MERGE
@@ -1,4 +1,4 @@
-476882795473a884f837bea6da850ac5181868d1
+7bdd83d7b4bd9fd4cb9ffca0d50babc90b31bfd6
 
 The first line of this file holds the git revision number of the last
 merge done from the dlang/druntime repository.
diff --git a/libphobos/libdruntime/Makefile.am 
b/libphobos/libdruntime/Makefile.am
index e1dc24c660b..1d340a0041c 100644
--- a/libphobos/libdruntime/Makefile.am
+++ b/libphobos/libdruntime/Makefile.am
@@ -81,8 +81,7 @@ if DRUNTIME_CPU_MIPS
 DRUNTIME_SOURCES_CONFIGURED += config/mips/switchcontext.S
 endif
 if DRUNTIME_CPU_POWERPC
-DRUNTIME_SOURCES_CONFIGURED += config/powerpc/callwithstack.S \
-  config/powerpc/switchcontext.S
+DRUNTIME_SOURCES_CONFIGURED += config/powerpc/switchcontext.S
 endif
 if DRUNTIME_CPU_X86
 if DRUNTIME_OS_MINGW
diff --git a/libphobos/libdruntime/Makefile.in 
b/libphobos/libdruntime/Makefile.in
index 694de3e0b43..3fddbc340de 100644
--- a/libphobos/libdruntime/Makefile.in
+++ b/libphobos/libdruntime/Makefile.in
@@ -123,9 +123,7 @@ target_triplet = @target@
 @DRUNTIME_CPU_AARCH64_TRUE@am__append_11 = config/aarch64/switchcontext.S
 

Re: [OpenMP, gimplifier] 'inform' after 'error' diagnostic

[Jakub Jelinek via Gcc-patches]

On Thu, Apr 30, 2020 at 12:07:32PM +0200, Thomas Schwinge wrote:
> Hi!
> 
> Is the attached OK for master branch, in next stage 1?  It may even be
> obvious, per our diagnostic conventions?  If approving this patch, please
> respond with "Reviewed-by: NAME " so that your effort will be
> recorded in the commit log, see .

Ok for stage1, thanks.

Jakub

[OpenMP, gimplifier] 'inform' after 'error' diagnostic

[Thomas Schwinge]

Hi!

Is the attached OK for master branch, in next stage 1?  It may even be
obvious, per our diagnostic conventions?  If approving this patch, please
respond with "Reviewed-by: NAME " so that your effort will be
recorded in the commit log, see .


Grüße
 Thomas


-
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander 
Walter
>From 28565fb32202a3b4d166ef86363d0737d077855a Mon Sep 17 00:00:00 2001
From: Thomas Schwinge 
Date: Thu, 30 Apr 2020 09:07:06 +0200
Subject: [PATCH] [OpenMP, gimplifier] 'inform' after 'error' diagnostic

This is not a thorough review of the code, just a few cases I noticed while
scanning 'gcc/gimplify.c' for "enclosing".

	gcc/
	* gimplify.c (omp_notice_threadprivate_variable)
	(omp_default_clause, omp_notice_variable): 'inform' after 'error'
	diagnostic.  Adjust all users.
---
 gcc/gimplify.c | 10 +-
 gcc/testsuite/c-c++-common/gomp/default-1.c|  8 
 gcc/testsuite/c-c++-common/gomp/defaultmap-3.c |  6 +++---
 gcc/testsuite/c-c++-common/gomp/order-4.c  |  8 
 gcc/testsuite/g++.dg/gomp/parallel-2.C |  4 ++--
 gcc/testsuite/g++.dg/gomp/predetermined-1.C| 14 +++---
 gcc/testsuite/g++.dg/gomp/sharing-1.C  |  2 +-
 gcc/testsuite/gcc.dg/gomp/appendix-a/a.24.1.c  |  2 +-
 gcc/testsuite/gcc.dg/gomp/parallel-2.c |  4 ++--
 gcc/testsuite/gcc.dg/gomp/pr44085.c|  2 +-
 gcc/testsuite/gcc.dg/gomp/sharing-1.c  |  2 +-
 gcc/testsuite/gcc.dg/gomp/vla-1.c  |  2 +-
 .../gfortran.dg/gomp/appendix-a/a.24.1.f90 |  2 +-
 gcc/testsuite/gfortran.dg/gomp/crayptr3.f90|  2 +-
 gcc/testsuite/gfortran.dg/gomp/pr33439.f90 |  7 ---
 gcc/testsuite/gfortran.dg/gomp/pr44036-1.f90   |  4 ++--
 gcc/testsuite/gfortran.dg/gomp/pr44085.f90 |  2 +-
 gcc/testsuite/gfortran.dg/gomp/pr44536.f90 |  2 +-
 gcc/testsuite/gfortran.dg/gomp/sharing-1.f90   |  2 +-
 gcc/testsuite/gfortran.dg/gomp/sharing-2.f90   | 10 +-
 gcc/testsuite/gfortran.dg/gomp/sharing-3.f90   |  2 +-
 21 files changed, 49 insertions(+), 48 deletions(-)

diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 2f2c51b2d894..eaff5ae9e7eb 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -7100,13 +7100,13 @@ omp_notice_threadprivate_variable (struct gimplify_omp_ctx *ctx, tree decl,
 	  {
 		error ("threadprivate variable %qE used in a region with"
 		   " % clause", DECL_NAME (decl));
-		error_at (octx->location, "enclosing region");
+		inform (octx->location, "enclosing region");
 	  }
 	else
 	  {
 		error ("threadprivate variable %qE used in target region",
 		   DECL_NAME (decl));
-		error_at (octx->location, "enclosing target region");
+		inform (octx->location, "enclosing target region");
 	  }
 	splay_tree_insert (octx->variables, (splay_tree_key)decl, 0);
 	  }
@@ -7121,7 +7121,7 @@ omp_notice_threadprivate_variable (struct gimplify_omp_ctx *ctx, tree decl,
 {
   error ("threadprivate variable %qE used in untied task",
 	 DECL_NAME (decl));
-  error_at (ctx->location, "enclosing task");
+  inform (ctx->location, "enclosing task");
   splay_tree_insert (ctx->variables, (splay_tree_key)decl, 0);
 }
   if (decl2)
@@ -7199,7 +7199,7 @@ omp_default_clause (struct gimplify_omp_ctx *ctx, tree decl,
 	
 	error ("%qE not specified in enclosing %qs",
 	   DECL_NAME (lang_hooks.decls.omp_report_decl (decl)), rtype);
-	error_at (ctx->location, "enclosing %qs", rtype);
+	inform (ctx->location, "enclosing %qs", rtype);
   }
   /* FALLTHRU */
 case OMP_CLAUSE_DEFAULT_SHARED:
@@ -7469,7 +7469,7 @@ omp_notice_variable (struct gimplify_omp_ctx *ctx, tree decl, bool in_code)
 		  tree d = lang_hooks.decls.omp_report_decl (decl);
 		  error ("%qE not specified in enclosing %",
 			 DECL_NAME (d));
-		  error_at (ctx->location, "enclosing %");
+		  inform (ctx->location, "enclosing %");
 		}
 		  else if (ctx->defaultmap[gdmk]
 			   & (GOVD_MAP_0LEN_ARRAY | GOVD_FIRSTPRIVATE))
diff --git a/gcc/testsuite/c-c++-common/gomp/default-1.c b/gcc/testsuite/c-c++-common/gomp/default-1.c
index 6525483c44e7..eab46892dada 100644
--- a/gcc/testsuite/c-c++-common/gomp/default-1.c
+++ b/gcc/testsuite/c-c++-common/gomp/default-1.c
@@ -2,20 +2,20 @@ void
 foo (void)
 {
   int x = 0, i;
-  #pragma omp task default(none)	/* { dg-error "enclosing 'task'" } */
+  #pragma omp task default(none)	/* { dg-message "note: enclosing 'task'" } */
   {
 x++;	/* { dg-error "'x' not specified in enclosing 'task'" } */
   }
-  #pragma omp taskloop default(none)	/* { dg-error "enclosing 'taskloop'" } */
+  #pragma omp taskloop default(none)	/* { dg-message "note: enclosing 'taskloop'" } */
   for (i = 0; i < 

RE: [PATCH] wwwdocs: Added mentioning of TX3 chip to the list of the processors

[Kyrylo Tkachov]

> -Original Message-
> From: Gerald Pfeifer 
> Sent: 30 April 2020 10:53
> To: Kyrylo Tkachov 
> Cc: Anton Youdkevitch ; gcc-
> patc...@gcc.gnu.org; Joseph S. Myers 
> Subject: RE: [PATCH] wwwdocs: Added mentioning of TX3 chip to the list of
> the processors
> 
> On Thu, 30 Apr 2020, Kyrylo Tkachov wrote:
> > Ok.
> 
> Ah, and you also pushed it in Git.  (I was just going to do it
> since Anton does not appear to have an account, and noticed it
> was a no-op. ;-)

Ah yes, sorry. I was initially going to leave it to somebody else, but then 
found the time after all.
Kyrill

> 
> Thanks,
> Gerald

RE: [PATCH] wwwdocs: Added mentioning of TX3 chip to the list of the processors

[Gerald Pfeifer]

On Thu, 30 Apr 2020, Kyrylo Tkachov wrote:
> Ok.

Ah, and you also pushed it in Git.  (I was just going to do it
since Anton does not appear to have an account, and noticed it
was a no-op. ;-)

Thanks,
Gerald

Re: [PATCH] --with-{documentation,changes}-root-url tweaks

[Richard Biener via Gcc-patches]

On Thu, Apr 30, 2020 at 10:44 AM Jakub Jelinek  wrote:
>
> Hi!
> On Thu, Apr 30, 2020 at 08:42:39AM +0200, Richard Biener wrote:
> >   , CHANGES_URL ("gcc-10/changes.html#empty_base");
> >
> > where the macro would just use preprocessor string concatenation?
>
> Ok, the following patch implements it (doesn't introduce a separate
> macro and just uses CHANGES_ROOT_URL "gcc-10/changes.html#empty_base"),
> in addition adds the documentation Joseph requested.
>
> Tested on x86_64-linux and with crosses to s390x-linux and
> powerpc64le-linux, ok for trunk?

OK.

Richard.

> 2020-04-30  Jakub Jelinek  
>
> * configure.ac (--with-documentation-root-url,
> --with-changes-root-url): Diagnose URL not ending with /,
> use AC_DEFINE_UNQUOTED instead of AC_SUBST.
> * opts.h (get_changes_url): Remove.
> * opts.c (get_changes_url): Remove.
> * Makefile.in (CFLAGS-opts.o): Don't add -DDOCUMENTATION_ROOT_URL
> or -DCHANGES_ROOT_URL.
> * doc/install.texi (--with-documentation-root-url,
> --with-changes-root-url): Document.
> * config/arm/arm.c (aapcs_vfp_is_call_or_return_candidate): Don't call
> get_changes_url and free, change url variable type to const char * and
> set it to CHANGES_ROOT_URL "gcc-10/changes.html#empty_base".
> * config/s390/s390.c (s390_function_arg_vector,
> s390_function_arg_float): Likewise.
> * config/aarch64/aarch64.c (aarch64_vfp_is_call_or_return_candidate):
> Likewise.
> * config/rs6000/rs6000-call.c (rs6000_discover_homogeneous_aggregate):
> Likewise.
> * config.in: Regenerate.
> * configure: Regenerate.
>
> --- gcc/configure.ac.jj 2020-04-29 22:41:05.086585150 +0200
> +++ gcc/configure.ac2020-04-30 09:35:29.800894085 +0200
> @@ -979,12 +979,13 @@ AC_ARG_WITH(documentation-root-url,
>  [case "$withval" in
>yes) AC_MSG_ERROR([documentation root URL not specified]) ;;
>no)  AC_MSG_ERROR([documentation root URL not specified]) ;;
> -  *)   DOCUMENTATION_ROOT_URL="$withval"
> -  ;;
> +  */)  DOCUMENTATION_ROOT_URL="$withval" ;;
> +  *)   AC_MSG_ERROR([documentation root URL does not end with /]) ;;
>   esac],
>   DOCUMENTATION_ROOT_URL="https://gcc.gnu.org/onlinedocs/;
>  )
> -AC_SUBST(DOCUMENTATION_ROOT_URL)
> +AC_DEFINE_UNQUOTED(DOCUMENTATION_ROOT_URL,"$DOCUMENTATION_ROOT_URL",
> +   [Define to the root for documentation URLs.])
>
>  # Allow overriding the default URL for GCC changes
>  AC_ARG_WITH(changes-root-url,
> @@ -993,12 +994,13 @@ AC_ARG_WITH(changes-root-url,
>  [case "$withval" in
>yes) AC_MSG_ERROR([changes root URL not specified]) ;;
>no)  AC_MSG_ERROR([changes root URL not specified]) ;;
> -  *)   CHANGES_ROOT_URL="$withval"
> -  ;;
> +  */)  CHANGES_ROOT_URL="$withval" ;;
> +  *)   AC_MSG_ERROR([changes root URL does not end with /]) ;;
>   esac],
>   CHANGES_ROOT_URL="https://gcc.gnu.org/;
>  )
> -AC_SUBST(CHANGES_ROOT_URL)
> +AC_DEFINE_UNQUOTED(CHANGES_ROOT_URL,"$CHANGES_ROOT_URL",
> +   [Define to the root for URLs about GCC changes.])
>
>  # Sanity check enable_languages in case someone does not run the toplevel
>  # configure # script.
> --- gcc/opts.h.jj   2020-04-29 22:41:05.089585106 +0200
> +++ gcc/opts.h  2020-04-30 09:38:20.110367236 +0200
> @@ -464,7 +464,6 @@ extern void parse_options_from_collect_g
> int *);
>
>  extern void prepend_xassembler_to_collect_as_options (const char *, obstack 
> *);
> -extern char *get_changes_url (const char *);
>
>  /* Set OPTION in OPTS to VALUE if the option is not set in OPTS_SET.  */
>
> --- gcc/opts.c.jj   2020-04-29 22:41:05.090585091 +0200
> +++ gcc/opts.c  2020-04-30 09:37:17.039303011 +0200
> @@ -3190,16 +3190,6 @@ get_option_url (diagnostic_context *, in
>  return NULL;
>  }
>
> -/* Given "gcc-10/changes.html#foobar", return that URL under
> -   CHANGES_ROOT_URL (see --with-changes-root-url).
> -   The caller is responsible for freeing the returned string.  */
> -
> -char *
> -get_changes_url (const char *str)
> -{
> -  return concat (CHANGES_ROOT_URL, str, NULL);
> -}
> -
>  #if CHECKING_P
>
>  namespace selftest {
> --- gcc/Makefile.in.jj  2020-04-29 22:41:05.088585120 +0200
> +++ gcc/Makefile.in 2020-04-30 09:36:14.201235329 +0200
> @@ -2186,9 +2186,6 @@ lto-wrapper$(exeext): $(LTO_WRAPPER_OBJS
>$(LTO_WRAPPER_OBJS) libcommon-target.a $(LIBS)
> mv -f T$@ $@
>
> -CFLAGS-opts.o += -DDOCUMENTATION_ROOT_URL=\"@DOCUMENTATION_ROOT_URL@\"
> -CFLAGS-opts.o += -DCHANGES_ROOT_URL=\"@CHANGES_ROOT_URL@\"
> -
>  # Files used by all variants of C or by the stand-alone pre-processor.
>
>  CFLAGS-c-family/c-opts.o += @TARGET_SYSTEM_ROOT_DEFINE@
> --- gcc/doc/install.texi.jj 2020-04-23 14:42:07.614123663 +0200
> +++ gcc/doc/install.texi2020-04-30 10:13:09.500365247 +0200
> @@ 

Re: arm: Fix vfp_operand_register for VFP HI regs

[Christophe Lyon via Gcc-patches]

On Wed, 29 Apr 2020 at 18:40, Kyrylo Tkachov  wrote:
>
> Hi Christophe,
>
> > -Original Message-
> > From: Gcc-patches  On Behalf Of
> > Christophe Lyon via Gcc-patches
> > Sent: 29 April 2020 16:53
> > To: gcc Patches 
> > Subject: arm: Fix vfp_operand_register for VFP HI regs
> >
> > Hi,
> >
> > While looking at PR target/94743 I noticed an ICE when I tried to save
> > all the FP registers: this was because all HI registers wouldn't match
> > vfp_register_operand.
>
> Hmm, I see that arm_regno_class indeed never returns VFP_REGS and would 
> return VFP_HI_REGS here.
> So the patch looks correct to me.
> Do you have a testcase for the ICE to add to the testsuite?
>

No C source code: I found that while extending the list of registers
pushed in the prologue of an IRQ handler, more-or-less modifying
arm_save_coproc_regs so that more registers are handled by
vfp_emit_fstmd.
The problem occurs when trying to push d16-d31.


> Thanks,
> Kyrill
>
> >
> > Regression-tested and bootstrapped OK.
> >
> > 2020-04-29  Christophe Lyon  
> >
> > gcc/
> > * config/arm/predicates.md (vfp_register_operand): Use VFP_HI_REGS
> > instead of VFP_REGS.
> >
> > OK?
> >
> > Thanks,
> >
> > Christophe

[PATCH] --with-{documentation,changes}-root-url tweaks

[Jakub Jelinek via Gcc-patches]

Hi!
On Thu, Apr 30, 2020 at 08:42:39AM +0200, Richard Biener wrote:
>   , CHANGES_URL ("gcc-10/changes.html#empty_base");
> 
> where the macro would just use preprocessor string concatenation?

Ok, the following patch implements it (doesn't introduce a separate
macro and just uses CHANGES_ROOT_URL "gcc-10/changes.html#empty_base"),
in addition adds the documentation Joseph requested.

Tested on x86_64-linux and with crosses to s390x-linux and
powerpc64le-linux, ok for trunk?

2020-04-30  Jakub Jelinek  

* configure.ac (--with-documentation-root-url,
--with-changes-root-url): Diagnose URL not ending with /,
use AC_DEFINE_UNQUOTED instead of AC_SUBST.
* opts.h (get_changes_url): Remove.
* opts.c (get_changes_url): Remove.
* Makefile.in (CFLAGS-opts.o): Don't add -DDOCUMENTATION_ROOT_URL
or -DCHANGES_ROOT_URL.
* doc/install.texi (--with-documentation-root-url,
--with-changes-root-url): Document.
* config/arm/arm.c (aapcs_vfp_is_call_or_return_candidate): Don't call
get_changes_url and free, change url variable type to const char * and
set it to CHANGES_ROOT_URL "gcc-10/changes.html#empty_base".
* config/s390/s390.c (s390_function_arg_vector,
s390_function_arg_float): Likewise.
* config/aarch64/aarch64.c (aarch64_vfp_is_call_or_return_candidate):
Likewise.
* config/rs6000/rs6000-call.c (rs6000_discover_homogeneous_aggregate):
Likewise.
* config.in: Regenerate.
* configure: Regenerate.

--- gcc/configure.ac.jj 2020-04-29 22:41:05.086585150 +0200
+++ gcc/configure.ac2020-04-30 09:35:29.800894085 +0200
@@ -979,12 +979,13 @@ AC_ARG_WITH(documentation-root-url,
 [case "$withval" in
   yes) AC_MSG_ERROR([documentation root URL not specified]) ;;
   no)  AC_MSG_ERROR([documentation root URL not specified]) ;;
-  *)   DOCUMENTATION_ROOT_URL="$withval"
-  ;;
+  */)  DOCUMENTATION_ROOT_URL="$withval" ;;
+  *)   AC_MSG_ERROR([documentation root URL does not end with /]) ;;
  esac],
  DOCUMENTATION_ROOT_URL="https://gcc.gnu.org/onlinedocs/;
 )
-AC_SUBST(DOCUMENTATION_ROOT_URL)
+AC_DEFINE_UNQUOTED(DOCUMENTATION_ROOT_URL,"$DOCUMENTATION_ROOT_URL",
+   [Define to the root for documentation URLs.])
 
 # Allow overriding the default URL for GCC changes
 AC_ARG_WITH(changes-root-url,
@@ -993,12 +994,13 @@ AC_ARG_WITH(changes-root-url,
 [case "$withval" in
   yes) AC_MSG_ERROR([changes root URL not specified]) ;;
   no)  AC_MSG_ERROR([changes root URL not specified]) ;;
-  *)   CHANGES_ROOT_URL="$withval"
-  ;;
+  */)  CHANGES_ROOT_URL="$withval" ;;
+  *)   AC_MSG_ERROR([changes root URL does not end with /]) ;;
  esac],
  CHANGES_ROOT_URL="https://gcc.gnu.org/;
 )
-AC_SUBST(CHANGES_ROOT_URL)
+AC_DEFINE_UNQUOTED(CHANGES_ROOT_URL,"$CHANGES_ROOT_URL",
+   [Define to the root for URLs about GCC changes.])
 
 # Sanity check enable_languages in case someone does not run the toplevel
 # configure # script.
--- gcc/opts.h.jj   2020-04-29 22:41:05.089585106 +0200
+++ gcc/opts.h  2020-04-30 09:38:20.110367236 +0200
@@ -464,7 +464,6 @@ extern void parse_options_from_collect_g
int *);
 
 extern void prepend_xassembler_to_collect_as_options (const char *, obstack *);
-extern char *get_changes_url (const char *);
 
 /* Set OPTION in OPTS to VALUE if the option is not set in OPTS_SET.  */
 
--- gcc/opts.c.jj   2020-04-29 22:41:05.090585091 +0200
+++ gcc/opts.c  2020-04-30 09:37:17.039303011 +0200
@@ -3190,16 +3190,6 @@ get_option_url (diagnostic_context *, in
 return NULL;
 }
 
-/* Given "gcc-10/changes.html#foobar", return that URL under
-   CHANGES_ROOT_URL (see --with-changes-root-url).
-   The caller is responsible for freeing the returned string.  */
-
-char *
-get_changes_url (const char *str)
-{
-  return concat (CHANGES_ROOT_URL, str, NULL);
-}
-
 #if CHECKING_P
 
 namespace selftest {
--- gcc/Makefile.in.jj  2020-04-29 22:41:05.088585120 +0200
+++ gcc/Makefile.in 2020-04-30 09:36:14.201235329 +0200
@@ -2186,9 +2186,6 @@ lto-wrapper$(exeext): $(LTO_WRAPPER_OBJS
   $(LTO_WRAPPER_OBJS) libcommon-target.a $(LIBS)
mv -f T$@ $@
 
-CFLAGS-opts.o += -DDOCUMENTATION_ROOT_URL=\"@DOCUMENTATION_ROOT_URL@\"
-CFLAGS-opts.o += -DCHANGES_ROOT_URL=\"@CHANGES_ROOT_URL@\"
-
 # Files used by all variants of C or by the stand-alone pre-processor.
 
 CFLAGS-c-family/c-opts.o += @TARGET_SYSTEM_ROOT_DEFINE@
--- gcc/doc/install.texi.jj 2020-04-23 14:42:07.614123663 +0200
+++ gcc/doc/install.texi2020-04-30 10:13:09.500365247 +0200
@@ -684,6 +684,19 @@ if you determine that they are not bugs
 
 The default value refers to the FSF's GCC bug tracker.
 
+@item --with-documentation-root-url=@var{url}
+Specify the URL root that contains GCC option documentation.  The @var{url}
+should end with a @code{/} character.
+
+The 

Re: [PATCH] rtl cse: Fix PR94740, ICE on testsuite/gcc.dg/sso/t5.c with -mcpu=future -mpcrel -O1

[Richard Sandiford]

Peter Bergner  writes:
> On 4/29/20 4:15 PM, Peter Bergner wrote:
>> On 4/29/20 3:28 PM, Richard Sandiford wrote:
>>> (Sorry for going ahead and writing an alternative patch, since if we do
>>> go for this, I guess the earlier misdirections will have wasted two days
>>> of your time.  But it seemed like I was just never going to think about
>>> this PR properly unless I actually tried to write something. :-()
>> 
>> No worries from me!  I'm just glad to see this fixed before the release.
>> I'll kill off a bootstrap and regtest on powerpc64le-linux too, in addition
>> to your tests (arm & x86_64?).  Thanks for your help with this!
>
> My bootstrap and regtesting of your patch on powerpc64le-linux was clean.

Thanks.  aarch64-linux-gnu and x86_64-linux-gnu bootstrap & regtests
also came back clean.  I'll kick off an arm-linux-gnueabihf one too
just to be safe.

I guess at this point it needs a review from someone else though.
Jeff, WDYT?  Attached again below, this time without the shonky mime type.

Richard

>From 39e20b51af8f977a1786ef5c15af80e47415d352 Mon Sep 17 00:00:00 2001
From: Richard Sandiford 
Date: Wed, 29 Apr 2020 20:38:13 +0100
Subject: [PATCH] cse: Use simplify_replace_fn_rtx to process notes [PR94740]

cse_process_notes did a very simple substitution, which in the wrong
circumstances could create non-canonical RTL and invalid MEMs.
Various sticking plasters have been applied to cse_process_notes_1
to handle cases like ZERO_EXTEND, SIGN_EXTEND and UNSIGNED_FLOAT,
but I think this PR is a plaster too far.

The code is trying hard to avoid creating unnecessary rtl, which of
course is a good thing.  If we continue to do that, then we can end
up changing subexpressions while keeping the containing rtx.
This in turn means that validate_change will be a no-op on the
containing rtx, even if its contents have changed.  So in these
cases we have to apply validate_change to the individual subexpressions.

On the other hand, if we always apply validate_change to the
individual subexpressions, we'll end up calling validate_change
on something before it has been simplified and canonicalised.
And that's one of the situations we're trying to avoid.

There might be a middle ground in which we queue the validate_changes
as part of a group, and so can cancel the pending validate_changes
for subexpressions if there's a change in the outer expression.
But that seems even more ad-hoc than the current code.
It would also be quite an invasive change.

I think the best thing is just to hook into the existing
simplify_replace_fn_rtx function, keeping the REG and MEM handling
from cse_process_notes_1 essentially unchanged.  It can generate
more redundant rtl when a simplification takes place, but it has
the advantage of being relative well-used code (both directly
and via simplify_replace_rtx).

2020-04-29  Richard Sandiford  

gcc/
PR rtl-optimization/94740
* cse.c (cse_process_notes_1): Replace with...
(cse_process_note_1): ...this new function, acting as a
simplify_replace_fn_rtx callback to process_note.  Handle only
REGs and MEMs directly.  Validate the MEM if cse_process_note
changes its address.
(cse_process_notes): Replace with...
(cse_process_note): ...this new function.
(cse_extended_basic_block): Update accordingly, iterating over
the register notes and passing individual notes to cse_process_note.
---
 gcc/cse.c | 118 --
 1 file changed, 34 insertions(+), 84 deletions(-)

diff --git a/gcc/cse.c b/gcc/cse.c
index 5aaba8d80e0..36bcfc354d8 100644
--- a/gcc/cse.c
+++ b/gcc/cse.c
@@ -585,7 +585,6 @@ static void cse_insn (rtx_insn *);
 static void cse_prescan_path (struct cse_basic_block_data *);
 static void invalidate_from_clobbers (rtx_insn *);
 static void invalidate_from_sets_and_clobbers (rtx_insn *);
-static rtx cse_process_notes (rtx, rtx, bool *);
 static void cse_extended_basic_block (struct cse_basic_block_data *);
 extern void dump_class (struct table_elt*);
 static void get_cse_reg_info_1 (unsigned int regno);
@@ -6222,75 +6221,28 @@ invalidate_from_sets_and_clobbers (rtx_insn *insn)
 }
 }
 
-/* Process X, part of the REG_NOTES of an insn.  Look at any REG_EQUAL notes
-   and replace any registers in them with either an equivalent constant
-   or the canonical form of the register.  If we are inside an address,
-   only do this if the address remains valid.
+static rtx cse_process_note (rtx);
 
-   OBJECT is 0 except when within a MEM in which case it is the MEM.
+/* A simplify_replace_fn_rtx callback for cse_process_note.  Process X,
+   part of the REG_NOTES of an insn.  Replace any registers with either
+   an equivalent constant or the canonical form of the register.
+   Only replace addresses if the containing MEM remains valid.
 
-   Return the replacement for X.  */
+   Return the replacement for X, or null if it should be simplified
+   recursively.  */
 
 

[PATCH v2] Add handling of MULT_EXPR/PLUS_EXPR for wrapping overflow in affine combination(PR83403)

[luoxhu via Gcc-patches]

Update the patch with overflow check.  Bootstrap and regression tested PASS on 
Power8-LE.


Use determine_value_range to get value range info for fold convert expressions
with internal operation PLUS_EXPR/MINUS_EXPR/MULT_EXPR when not overflow on
wrapping overflow inner type.  i.e.:

(long unsigned int)((unsigned  int)n * 10 + 1)
=>
(long unsigned int)n * (long unsigned int)10 + (long unsigned int)1

With this patch for affine combination, load/store motion could detect
more address refs independency and promote some memory expressions to
registers within loop.

PS: Replace the previous "(T1)(X + CST) as (T1)X - (T1)(-CST))"
to "(T1)(X + CST) as (T1)X + (T1)(CST))" for wrapping overflow.

gcc/ChangeLog

2020-04-30  Xiong Hu Luo  

PR tree-optimization/83403
* tree-affine.c (expr_to_aff_combination): Replace SSA_NAME with
determine_value_range, Add fold conversion of MULT_EXPR, fix the
previous PLUS_EXPR.

gcc/testsuite/ChangeLog

2020-04-30  Xiong Hu Luo  

PR tree-optimization/83403
* gcc.dg/tree-ssa/pr83403-1.c: New test.
* gcc.dg/tree-ssa/pr83403-2.c: New test.
* gcc.dg/tree-ssa/pr83403.h: New header.
---
 gcc/testsuite/gcc.dg/tree-ssa/pr83403-1.c |  8 ++
 gcc/testsuite/gcc.dg/tree-ssa/pr83403-2.c |  8 ++
 gcc/testsuite/gcc.dg/tree-ssa/pr83403.h   | 30 +++
 gcc/tree-affine.c | 24 ++
 4 files changed, 60 insertions(+), 10 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr83403-1.c
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr83403-2.c
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr83403.h

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr83403-1.c 
b/gcc/testsuite/gcc.dg/tree-ssa/pr83403-1.c
new file mode 100644
index 000..748375b03af
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr83403-1.c
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -funroll-loops -fdump-tree-lim2-details" } */
+
+#define TYPE unsigned int
+
+#include "pr83403.h"
+
+/* { dg-final { scan-tree-dump-times "Executing store motion of" 10 "lim2" } } 
*/
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr83403-2.c 
b/gcc/testsuite/gcc.dg/tree-ssa/pr83403-2.c
new file mode 100644
index 000..ca2e6bbd61c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr83403-2.c
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -funroll-loops -fdump-tree-lim2-details" } */
+
+#define TYPE int
+
+#include "pr83403.h"
+
+/* { dg-final { scan-tree-dump-times "Executing store motion of" 10 "lim2" } } 
*/
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr83403.h 
b/gcc/testsuite/gcc.dg/tree-ssa/pr83403.h
new file mode 100644
index 000..0da8a835b5f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr83403.h
@@ -0,0 +1,30 @@
+__attribute__ ((noinline)) void
+calculate (const double *__restrict__ A, const double *__restrict__ B,
+  double *__restrict__ C)
+{
+  TYPE m = 0;
+  TYPE n = 0;
+  TYPE k = 0;
+
+  A = (const double *) __builtin_assume_aligned (A, 16);
+  B = (const double *) __builtin_assume_aligned (B, 16);
+  C = (double *) __builtin_assume_aligned (C, 16);
+
+  for (n = 0; n < 9; n++)
+{
+  for (m = 0; m < 10; m++)
+   {
+ C[(n * 10) + m] = 0.0;
+   }
+
+  for (k = 0; k < 17; k++)
+   {
+#pragma simd
+ for (m = 0; m < 10; m++)
+   {
+ C[(n * 10) + m] += A[(k * 20) + m] * B[(n * 20) + k];
+   }
+   }
+}
+}
+
diff --git a/gcc/tree-affine.c b/gcc/tree-affine.c
index 0eb8db1b086..5620e6bf28f 100644
--- a/gcc/tree-affine.c
+++ b/gcc/tree-affine.c
@@ -343,24 +343,28 @@ expr_to_aff_combination (aff_tree *comb, tree_code code, 
tree type,
wide_int minv, maxv;
/* If inner type has wrapping overflow behavior, fold conversion
   for below case:
-(T1)(X - CST) -> (T1)X - (T1)CST
-  if X - CST doesn't overflow by range information.  Also handle
-  (T1)(X + CST) as (T1)(X - (-CST)).  */
+(T1)(X *+- CST) -> (T1)X *+- (T1)CST
+  if X *+- CST doesn't overflow by range information.  */
if (TYPE_UNSIGNED (itype)
&& TYPE_OVERFLOW_WRAPS (itype)
-   && TREE_CODE (op0) == SSA_NAME
&& TREE_CODE (op1) == INTEGER_CST
-   && icode != MULT_EXPR
-   && get_range_info (op0, , ) == VR_RANGE)
+   && determine_value_range (op0, , ) == VR_RANGE)
  {
+   wi::overflow_type overflow = wi::OVF_NONE;
+   signop sign = UNSIGNED;
if (icode == PLUS_EXPR)
- op1 = wide_int_to_tree (itype, -wi::to_wide (op1));
-   if (wi::geu_p (minv, wi::to_wide (op1)))
+ wi::add (maxv, wi::to_wide (op1), sign, );
+   else if (icode == MULT_EXPR)
+ wi::mul (maxv, wi::to_wide (op1), sign, );
+ 

Re: [Patch, Fortran] OpenMP/OpenACC – fix more issues with OPTIONAL

[Jakub Jelinek via Gcc-patches]

On Thu, Apr 30, 2020 at 08:24:02AM +0200, Richard Biener wrote:
> On Wed, Apr 29, 2020 at 5:05 PM Tobias Burnus  wrote:
> > was a bit on the backburner but I now digged again.
> > See: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94848
> >
> > The problem is a generated static array variable in the
> > device function:
> >static integer(kind=4) A.12[3] = {1, 2, 3};
> > used as
> >_26 = A.12[S.13_67];
> >
> > With -ftree-pre, the expressions using _26 now directly use
> > the value (1, 2 and 3). It seems as if the variable A.12 is
> > eliminated before writing to LTO but the usage (A.12[S.…])
> > is not – at least it still appears in the device dumps.
> >
> > Maybe it is also related to something else, but crucial is
> > the -ftree-pre on the host side; the optimization level on
> > the device lto1 side is irrelevant.
> 
> IIRC there was a bugreport similar to this where offload
> variables were computed "early" instead of at the point
> of streaming.
> 
> It's obvious that the offload "part" use is not reflected in
> the non-offloat "part" IL (like via a function call parameter
> or so), so I assume this is a local static in a function
> we simply compile twice (but did not actually clone),
> once for each offloat target and once for the host.  So it's
> likely the above issue.

I'll try to do something about that early in stage1 and
eventually backport, we need to follow OpenMP rules anyway and
those require auto-discovery of referenced functions (if they are
definitions rather than just declarations), and similarly in
some cases for variables.  And the same code then could, if something
shouldn't be marked for offloading by the standard, complain loudly rather
than leaving it up for later (lto1 of the offloading compiler).

Jakub

RE: [PATCH] wwwdocs: Added mentioning of TX3 chip to the list of the processors

[Kyrylo Tkachov]

> -Original Message-
> From: Anton Youdkevitch 
> Sent: 28 April 2020 19:31
> To: gcc-patches@gcc.gnu.org
> Cc: Kyrylo Tkachov ; ger...@pfeifer.com;
> jos...@codesourcery.com
> Subject: [PATCH] wwwdocs: Added mentioning of TX3 chip to the list of the
> processors
> 
> This adds mentioning of Marvell ThunderX3 chip to
> the list of supported processors.

Ok.
Thanks,
Kyrill

> 
> 
> ---
>  htdocs/gcc-10/changes.html | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/htdocs/gcc-10/changes.html b/htdocs/gcc-10/changes.html
> index 41c2dc0..b37b74d 100644
> --- a/htdocs/gcc-10/changes.html
> +++ b/htdocs/gcc-10/changes.html
> @@ -655,6 +655,7 @@ typedef svbool_t pred512
> __attribute__((arm_sve_vector_bits(512)));
>Arm Cortex-A65 (cortex-a65).
>Arm Cortex-A65AE (cortex-a65ae).
>Arm Cortex-A34 (cortex-a34).
> +  Marvell ThunderX3 (thunderx3t110).
> 
> The GCC identifiers can be used
> as arguments to the -mcpu or -mtune
> options,
> --
> 2.7.4

Re: [PATCH] diagnostics: Add %{...%} pretty-format support for URLs and use it in -Wpsabi diagnostcs

[Jakub Jelinek via Gcc-patches]

On Thu, Apr 30, 2020 at 08:42:39AM +0200, Richard Biener wrote:
> > --- gcc/config/arm/arm.c.jj 2020-04-29 13:12:46.736007298 +0200
> > +++ gcc/config/arm/arm.c2020-04-29 14:34:04.878864236 +0200
> > @@ -6416,6 +6416,8 @@ aapcs_vfp_is_call_or_return_candidate (e
> >   && ((alt = aapcs_vfp_sub_candidate (type, _mode, NULL))
> >   != ag_count))
> > {
> > + const char *url
> > +   = get_changes_url ("gcc-10/changes.html#empty_base");
> 
> This is called even when !warn_psabi_flags and I wonder if we could
> instead use a macro at uses, like

It is not, because the 3 lines above the snippet are:
  if (warn_psabi
  && warn_psabi_flags
  && uid != last_reported_type_uid

> 
> >   gcc_assert (alt == -1);
> >   last_reported_type_uid = uid;
> >   /* Use TYPE_MAIN_VARIANT to strip any redundant const
> > @@ -6423,11 +6425,14 @@ aapcs_vfp_is_call_or_return_candidate (e
> >   if (warn_psabi_flags & WARN_PSABI_NO_UNIQUE_ADDRESS)
> > inform (input_location, "parameter passing for argument of "
> > "type %qT with %<[[no_unique_address]]%> members "
> > -   "changed in GCC 10.1", TYPE_MAIN_VARIANT (type));
> > +   "changed in %{GCC 10.1%}",
> > +   TYPE_MAIN_VARIANT (type), url);
> 
>   , CHANGES_URL ("gcc-10/changes.html#empty_base");
> 
> where the macro would just use preprocessor string concatenation?
> 
> Alternatively 'url' could be static I guess (and never freed)

The reason was that DOCUMENTATION_ROOT_URL is a macro added through
CFLAGS-opts.o += -DDOCUMENTATION_ROOT_URL=\"@DOCUMENTATION_ROOT_URL@\"
and so it is available in opts.o only and I've followed that.

I didn't want to add a long -D option to every *.o file out there.

But, thinking about it now, there is no reason why DOCUMENTATION_ROOT_URL
and CHANGES_ROOT_URL couldn't be in config.h, i.e. AC_DEFINE rather than
AC_SUBST, and then we can indeed use string concatenation.
I've already checked the patch in, so I'll post an incremental patch
(together with the doc/ changes requested by Joseph).

Jakub

Re: [PATCH] diagnostics: Add %{...%} pretty-format support for URLs and use it in -Wpsabi diagnostcs

[Richard Biener via Gcc-patches]

On Wed, Apr 29, 2020 at 3:44 PM Jakub Jelinek via Gcc-patches
 wrote:
>
> Hi!
>
> The following patch attempts to use the diagnostics URL support if available
> to provide more information about the C++17 empty base and C++20
> [[no_unique_address]] empty class ABI changes in -Wpsabi diagnostics.
>
> GCC 10.1 at the end of the diagnostics is then in some terminals
> underlined with a dotted line and points to a (to be written) anchor in
> gcc-10/changes.html which we need to write anyway.
>
> Ok for trunk if this passes bootstrap/regtest?
>
> 2020-04-29  Jakub Jelinek  
>
> * configure.ac (-with-changes-root-url): New configure option,
> defaulting to https://gcc.gnu.org/.
> * Makefile.in (CFLAGS-opts.o): Define CHANGES_ROOT_URL for
> opts.c.
> * pretty-print.c (end_url_string): New function.
> (pp_format): Handle %{ and %} for URLs.
> (pp_begin_url): Use pp_string instead of pp_printf.
> (pp_end_url): Use end_url_string.
> * opts.h (get_changes_url): Declare.
> * opts.c (get_changes_url): New function.
> * config/rs6000/rs6000-call.c: Include opts.h.
> (rs6000_discover_homogeneous_aggregate): Use %{GCC 10.1%} instead of
> just GCC 10.1 in diagnostics and add URL.
> * config/arm/arm.c (aapcs_vfp_is_call_or_return_candidate): Likewise.
> * config/aarch64/aarch64.c (aarch64_vfp_is_call_or_return_candidate):
> Likewise.
> * configure: Regenerated.
>
> * c-format.c (PP_FORMAT_CHAR_TABLE): Add %{ and %}.
>
> --- gcc/configure.ac.jj 2020-04-29 10:21:25.061999873 +0200
> +++ gcc/configure.ac2020-04-29 13:26:51.515516959 +0200
> @@ -986,6 +986,20 @@ AC_ARG_WITH(documentation-root-url,
>  )
>  AC_SUBST(DOCUMENTATION_ROOT_URL)
>
> +# Allow overriding the default URL for GCC changes
> +AC_ARG_WITH(changes-root-url,
> +AS_HELP_STRING([--with-changes-root-url=URL],
> +   [Root for GCC changes URLs]),
> +[case "$withval" in
> +  yes) AC_MSG_ERROR([changes root URL not specified]) ;;
> +  no)  AC_MSG_ERROR([changes root URL not specified]) ;;
> +  *)   CHANGES_ROOT_URL="$withval"
> +  ;;
> + esac],
> + CHANGES_ROOT_URL="https://gcc.gnu.org/;
> +)
> +AC_SUBST(CHANGES_ROOT_URL)
> +
>  # Sanity check enable_languages in case someone does not run the toplevel
>  # configure # script.
>  AC_ARG_ENABLE(languages,
> --- gcc/Makefile.in.jj  2020-02-27 09:28:46.129960135 +0100
> +++ gcc/Makefile.in 2020-04-29 13:38:42.455008718 +0200
> @@ -2187,6 +2187,7 @@ lto-wrapper$(exeext): $(LTO_WRAPPER_OBJS
> mv -f T$@ $@
>
>  CFLAGS-opts.o += -DDOCUMENTATION_ROOT_URL=\"@DOCUMENTATION_ROOT_URL@\"
> +CFLAGS-opts.o += -DCHANGES_ROOT_URL=\"@CHANGES_ROOT_URL@\"
>
>  # Files used by all variants of C or by the stand-alone pre-processor.
>
> --- gcc/pretty-print.c.jj   2020-04-25 00:08:33.359759328 +0200
> +++ gcc/pretty-print.c  2020-04-29 14:30:46.791792554 +0200
> @@ -1020,6 +1020,8 @@ pp_indent (pretty_printer *pp)
>  pp_space (pp);
>  }
>
> +static const char *end_url_string (pretty_printer *);
> +
>  /* The following format specifiers are recognized as being client 
> independent:
> %d, %i: (signed) integer in base ten.
> %u: unsigned integer in base ten.
> @@ -1038,6 +1040,8 @@ pp_indent (pretty_printer *pp)
> %%: '%'.
> %<: opening quote.
> %>: closing quote.
> +   %{: URL start.
> +   %}: URL end.
> %': apostrophe (should only be used in untranslated messages;
> translations should use appropriate punctuation directly).
> %@: diagnostic_event_id_ptr, for which event_id->known_p () must be true.
> @@ -1051,7 +1055,7 @@ pp_indent (pretty_printer *pp)
> Arguments can be used sequentially, or through %N$ resp. *N$
> notation Nth argument after the format string.  If %N$ / *N$
> notation is used, it must be used for all arguments, except %m, %%,
> -   %<, %> and %', which may not have a number, as they do not consume
> +   %<, %>, %} and %', which may not have a number, as they do not consume
> an argument.  When %M$.*N$s is used, M must be N + 1.  (This may
> also be written %M$.*s, provided N is not otherwise used.)  The
> format string must have conversion specifiers with argument numbers
> @@ -1084,7 +1088,7 @@ pp_format (pretty_printer *pp, text_info
>/* Formatting phase 1: split up TEXT->format_spec into chunks in
>   pp_buffer (PP)->args[].  Even-numbered chunks are to be output
>   verbatim, odd-numbered chunks are format specifiers.
> - %m, %%, %<, %>, and %' are replaced with the appropriate text at
> + %m, %%, %<, %>, %} and %' are replaced with the appropriate text at
>   this point.  */
>
>memset (formatters, 0, sizeof formatters);
> @@ -1133,6 +1137,15 @@ pp_format (pretty_printer *pp, text_info
>   p++;
>   continue;
>
> +   case '}':
> + {
> +   const char *endurlstr = end_url_string 

Re: [PATCH] tree: Don't reuse types if TYPE_USER_ALIGN differ [PR94775]

[Richard Biener via Gcc-patches]

On Wed, Apr 29, 2020 at 11:43 PM Marek Polacek via Gcc-patches
 wrote:
>
> Here we trip on the TYPE_USER_ALIGN (t) assert in strip_typedefs: it
> gets "const d[0]" with TYPE_USER_ALIGN=0 but the result built by
> build_cplus_array_type is "const char[0]" with TYPE_USER_ALIGN=1.
>
> When we strip_typedefs the element of the array "const d", we see it's
> a typedef_variant_p, so we look at its DECL_ORIGINAL_TYPE, which is
> char, but we need to add the const qualifier, so we call
> cp_build_qualified_type -> build_qualified_type
> where get_qualified_type checks to see if we already have such a type
> by walking the variants list, which in this case is:
>
>   char -> c -> const char -> const char -> d -> const d
>
> Because check_base_type only checks TYPE_ALIGN and not TYPE_USER_ALIGN,
> we choose the first const char, which has TYPE_USER_ALIGN set.  If the
> element type of an array has TYPE_USER_ALIGN, the array type gets it too.
>
> So we can make check_base_type stricter.  I was afraid that it might make
> us reuse types less often, but measuring showed that we build the same
> amount of types with and without the patch, while bootstrapping.
>
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/9/8?

Hmm, so while I wonder whether a mismatch in TYPE_USER_ALIGN would
affect program semantics we should make sure to handle this flag consistently.

Thus the patch is OK for trunk and branches after a while.

Note I question that we build new types during diagnostic printing.

Thanks,
Richard.

> PR c++/94775
> * tree.c (check_base_type): Return true only if TYPE_USER_ALIGN match.
> (check_aligned_type): Check if TYPE_USER_ALIGN match.
>
> * g++.dg/warn/Warray-bounds-10.C: New test.
> ---
>  gcc/testsuite/g++.dg/warn/Warray-bounds-10.C | 40 
>  gcc/tree.c   |  4 +-
>  2 files changed, 43 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/g++.dg/warn/Warray-bounds-10.C
>
> diff --git a/gcc/testsuite/g++.dg/warn/Warray-bounds-10.C 
> b/gcc/testsuite/g++.dg/warn/Warray-bounds-10.C
> new file mode 100644
> index 000..0a18f637e0e
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/Warray-bounds-10.C
> @@ -0,0 +1,40 @@
> +// PR c++/94775
> +// { dg-do compile { target c++14 } }
> +// { dg-options "-O2 -Warray-bounds" }
> +
> +template  using a = int;
> +template  using b = int;
> +typedef char d;
> +template  using e = int;
> +template  struct h { using i = b>, e>; };
> +template  using j = typename h::i;
> +long ab, k, aj;
> +const d l[]{};
> +class m {
> +public:
> +  m(int);
> +};
> +class n {
> +  void ad() const;
> +  template  void o(long) const {
> +using c __attribute__((aligned(1))) = const ae;
> +  }
> +  long p;
> +  template 
> +  auto s(unsigned long, unsigned long, unsigned long, unsigned long) const;
> +  template  auto q(unsigned long, unsigned long) const;
> +};
> +template 
> +auto n::s(unsigned long, unsigned long, unsigned long, unsigned long t) 
> const {
> +  o(p);
> +  return t;
> +}
> +template  auto n::q(unsigned long p1, unsigned long p2) const {
> +  using r = j<4, false>;
> +  using ai = j<4, g>;
> +  return s(ab, k, p1, p2);
> +}
> +void n::ad() const {
> +  long f(l[aj]); // { dg-warning "outside array bounds" }
> +  m(q(8, f));
> +}
> diff --git a/gcc/tree.c b/gcc/tree.c
> index e451401822c..341766c51e5 100644
> --- a/gcc/tree.c
> +++ b/gcc/tree.c
> @@ -6493,7 +6493,8 @@ check_base_type (const_tree cand, const_tree base)
> TYPE_ATTRIBUTES (base)))
>  return false;
>/* Check alignment.  */
> -  if (TYPE_ALIGN (cand) == TYPE_ALIGN (base))
> +  if (TYPE_ALIGN (cand) == TYPE_ALIGN (base)
> +  && TYPE_USER_ALIGN (cand) == TYPE_USER_ALIGN (base))
>  return true;
>/* Atomic types increase minimal alignment.  We must to do so as well
>   or we get duplicated canonical types. See PR88686.  */
> @@ -6528,6 +6529,7 @@ check_aligned_type (const_tree cand, const_tree base, 
> unsigned int align)
>   && TYPE_CONTEXT (cand) == TYPE_CONTEXT (base)
>   /* Check alignment.  */
>   && TYPE_ALIGN (cand) == align
> + && TYPE_USER_ALIGN (cand) == TYPE_USER_ALIGN (base)
>   && attribute_list_equal (TYPE_ATTRIBUTES (cand),
>TYPE_ATTRIBUTES (base))
>   && check_lang_type (cand, base));
>
> base-commit: 8f1591763fd50b143af0dc1770741f326a97583a
> --
> Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA
>

Re: [PATCH] var-tracking.c: Fix possible use of uninitialized variable pre

[Richard Biener via Gcc-patches]

On Wed, Apr 29, 2020 at 5:56 PM Jeff Law  wrote:
>
> On Tue, 2020-04-28 at 11:44 +0200, Richard Biener via Gcc-patches wrote:
> >
> > Btw, does s390 have different inlining parameters somehow?
> I think so.  We saw a ton of backend warnings that were specific to the s390
> builds in Fedora that appeared to be triggered by different inlining 
> decisions.
>
> It happened enough that one of the ideas we're going to explore is seeing if 
> we
> can tune the x86_64 port to use the same heuristics.  That way we can try to 
> find
> more of these issues in our tester without having to dramatically increase 
> s390x
> resourcing.  This hasn't started, but I expect we'll be looking at it in the 
> fall
> (it's unclear if we can just use flags or will need hackery to the x86 port, 
> but
> we're happy to share whatever we find with the wider community).

Hmm, OK.  Guess if it becomes an issue for openSUSE I'll simply revert those
s390 backend changes.

As a general advice to backend developers I _strongly_ discourage to adjust
--param defaults that affect GIMPLE.

Richard.

> jeff
>
>
>

Re: [Patch, Fortran] OpenMP/OpenACC – fix more issues with OPTIONAL

[Richard Biener via Gcc-patches]

On Wed, Apr 29, 2020 at 5:05 PM Tobias Burnus  wrote:
>
> Hi Thomas,
>
> was a bit on the backburner but I now digged again.
> See: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94848
>
> The problem is a generated static array variable in the
> device function:
>static integer(kind=4) A.12[3] = {1, 2, 3};
> used as
>_26 = A.12[S.13_67];
>
> With -ftree-pre, the expressions using _26 now directly use
> the value (1, 2 and 3). It seems as if the variable A.12 is
> eliminated before writing to LTO but the usage (A.12[S.…])
> is not – at least it still appears in the device dumps.
>
> Maybe it is also related to something else, but crucial is
> the -ftree-pre on the host side; the optimization level on
> the device lto1 side is irrelevant.

IIRC there was a bugreport similar to this where offload
variables were computed "early" instead of at the point
of streaming.

It's obvious that the offload "part" use is not reflected in
the non-offloat "part" IL (like via a function call parameter
or so), so I assume this is a local static in a function
we simply compile twice (but did not actually clone),
once for each offloat target and once for the host.  So it's
likely the above issue.

Richard.

> Cheers,
>
> Tobias
>
> On 4/29/20 12:00 PM, Thomas Schwinge wrote:
>
> > Hi Tobias!
> >
> > Do you happen to have any update regarding this one:
> >
> > On 2020-01-08T09:55:06+0100, Tobias Burnus  wrote:
> >> On 1/8/20 9:33 AM, Thomas Schwinge wrote:
> >>> With 'dg-do run' added, on [...] x86_64-pc-linux-gnu with offloading I'm 
> >>> seeing:
> > | PASS: libgomp.fortran/use_device_ptr-optional-3.f90   -O0  (test for 
> > excess errors)
> > | PASS: libgomp.fortran/use_device_ptr-optional-3.f90   -O0  execution 
> > test
> > | PASS: libgomp.fortran/use_device_ptr-optional-3.f90   -O1  (test for 
> > excess errors)
> > | PASS: libgomp.fortran/use_device_ptr-optional-3.f90   -O1  execution 
> > test
> > | FAIL: libgomp.fortran/use_device_ptr-optional-3.f90   -O2  (test for 
> > excess errors)
> > | UNRESOLVED: libgomp.fortran/use_device_ptr-optional-3.f90   -O2  
> > compilation failed to produce executable
> > | FAIL: libgomp.fortran/use_device_ptr-optional-3.f90   -O3 
> > -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer 
> > -finline-functions  (test for excess errors)
> > | UNRESOLVED: libgomp.fortran/use_device_ptr-optional-3.f90   -O3 
> > -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer 
> > -finline-functions  compilation failed to produce executable
> > | FAIL: libgomp.fortran/use_device_ptr-optional-3.f90   -O3 -g  (test 
> > for excess errors)
> > | UNRESOLVED: libgomp.fortran/use_device_ptr-optional-3.f90   -O3 -g  
> > compilation failed to produce executable
> > | FAIL: libgomp.fortran/use_device_ptr-optional-3.f90   -Os  (test for 
> > excess errors)
> > | UNRESOLVED: libgomp.fortran/use_device_ptr-optional-3.f90   -Os  
> > compilation failed to produce executable
> >
> >>> ... due to:
> >>>   /tmp/cciVc43I.o:(.gnu.offload_vars+0x10): undefined reference to 
> >>> `A.12.4064'
> >>> which may be something like PR90779, PR85063, PR84592, PR90779,
> >>> PR80411, PR71536 -- or something else.
> >> Hmm. It is surely among the listed items, if all fails in the last item.
> >> Note that PR85063 is fixed and PR84592 a duplicate of PR90779 (which is
> >> listed twice). To through in another number it could also be a variant
> >> of PR 92029 to though in yet another number …
> >
> > Grüße
> >   Thomas
> > -
> > Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / 
> > Germany
> > Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, 
> > Alexander Walter
> -
> Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
> Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, 
> Alexander Walter