Re: Rename DECL_IS_BUILTIN to DECL_IS_UNDECLARED_BUILTIN

2020-11-06 Thread Andreas Schwab
../../gcc/ada/gcc-interface/misc.c: In function 'const char* 
gnat_printable_name(tree, int)':
../../gcc/ada/gcc-interface/misc.c:562:47: error: 'DECL_IS_BUILTIN' was not 
declared in this scope
   if (verbosity == 2 && !DECL_IS_BUILTIN (decl))

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."


Re: [PATCH V2] arm: [testcase] Better narrow some bfloat16 testcase

2020-11-06 Thread Christophe Lyon via Gcc-patches
On Fri, 6 Nov 2020 at 15:06, Andrea Corallo  wrote:
>
> Christophe Lyon  writes:
>
> > On Thu, 5 Nov 2020 at 15:30, Andrea Corallo  wrote:
> >>
> >> Christophe Lyon  writes:
> >>
> >> > On Thu, 5 Nov 2020 at 12:11, Andrea Corallo  
> >> > wrote:
> >> >>
> >> >> Christophe Lyon  writes:
> >> >>
> >> >> [...]
> >> >>
> >> >> >> I think you need to add -mfloat-abi=hard to the dg-additional-options
> >> >> >> otherwise vld1_lane_bf16_1.c
> >> >> >> fails on targets with a soft float-abi default (eg 
> >> >> >> arm-linux-gnueabi).
> >> >> >>
> >> >> >> See bf16_vldn_1.c.
> >> >> >
> >> >> > Actually that's not sufficient because in turn we get:
> >> >> > /sysroot-arm-none-linux-gnueabi/usr/include/gnu/stubs.h:10:11: fatal
> >> >> > error: gnu/stubs-hard.h: No such file or directory
> >> >> >
> >> >> > So you should check that -mfloat-abi=hard is supported.
> >> >> >
> >> >> > Ditto for the vst tests.
> >> >>
> >> >> Hi Christophe,
> >> >>
> >> >> this patch should implement your suggestions.
> >> >>
> >> >> On my arm-none-linux-gnueabi setup the tests were already skipped
> >> >> as unsupported so if you could test and confirm this fixes the
> >> >> issue you see would be great.
> >> >
> >> > Do you know why they are unsupported in your setup?
> >>
> >> We probably have a different GCC configuration.  Could you share how
> >> it's configured your?
> >>
> > Sure, for instance:
> > --target=arm-none-linux-gnueabi --with-float=soft --with-mode=arm
> > --with-cpu=cortex-a9
>
> Thanks, I see now what was going on, my gas has no bf16 support so the
> test was marked as unsupported.  Dunno why I assumed
> check_no_compiler_messages_nocache wasn't testing the whole compilation
> process.
>
> >> >> diff --git a/gcc/testsuite/lib/target-supports.exp 
> >> >> b/gcc/testsuite/lib/target-supports.exp
> >> >> index 15f0649f8ae..2ab7e39756d 100644
> >> >> --- a/gcc/testsuite/lib/target-supports.exp
> >> >> +++ b/gcc/testsuite/lib/target-supports.exp
> >> >> @@ -5213,6 +5213,10 @@ proc 
> >> >> check_effective_target_arm_v8_2a_bf16_neon_ok_nocache { } {
> >> >>  return 0;
> >> >>  }
> >> >>
> >> >> +if { ! [check_effective_target_arm_hard_ok] } {
> >> >> + return 0;
> >> >> +}
> >> >> +
> >> >> foreach flags {"" "-mfloat-abi=hard -mfpu=neon-fp-armv8" 
> >> >> "-mfloat-abi=softfp -mfpu=neon-fp-armv8" } {
> >> >> if { [check_no_compiler_messages_nocache arm_v8_2a_bf16_neon_ok 
> >> >> object {
> >> >> #include 
> >> >
> >> > This seems strange since you would now exit early if
> >> > check_effective_target_arm_hard_ok is false, so you'll never need the
> >> > -mfloat-abi=softfp version of the flags.
> >>
> >> So IIUC your suggestion would be to test with higher priority softfp and
> >> in case we decide to go for hardfp make sure
> >> check_effective_target_arm_hard_ok is satisfied.  Am I correct?
> >>
> > ISTM that other tests that need hardfp check if it's supported in the
> > test, not in other effective targets.
> >
> > For instance mve/intrinsics/mve_fpu1.c
> >
> > I can see that quite a few tests that use -mfloat-abi=hard do not
> > check whether it's supported. Those I checked do not include
> > arm_neon.h and thus do not end up with the gnu/stubs-hard.h error
> > above.
>
> I see thanks for the explaination.  The attached should do the job.
>

Yes, it works for me, thanks.

>   Andrea
>


Re: Add fnspec to C++ new and delete

2020-11-06 Thread Christophe Lyon via Gcc-patches
Hi,

On Tue, 27 Oct 2020 at 11:22, Jan Hubicka  wrote:
>
> Hi,
> this patch makes C++ new and delete operators to be handled as
> malloc/free for fnspec.
>
> I still do not understand why free is ".co " and not ".cO ".
> I do not think we need to invalidate memory referenced to by blockbeing
> freed.
>
> Bootstrapped/regtested x86_64-linux, OK?
>
> Honza
>
> gcc/ChangeLog:
>
> 2020-10-27  Jan Hubicka  
>
> * gimple.c (gimple_call_fnspec): Handle C++ new and delete.
> * gimple.h (gimple_call_from_new_or_delete): Constify parameter.
>
> gcc/testsuite/ChangeLog:
>
> 2020-10-27  Jan Hubicka  
>
> * g++.dg/ipa/devirt-24.C: Update template.
>

This patch introduced a regression on arm-eabi (with newlib, as
opposed to arm-linux* with glibc):
FAIL:g++.dg/torture/pr79671.C   -O1  execution test

It seems this also regressed on sparc-solaris according to gcc-testresults.

Christophe

> diff --git a/gcc/gimple.c b/gcc/gimple.c
> index 469e6f369f3..1afed88e1f1 100644
> --- a/gcc/gimple.c
> +++ b/gcc/gimple.c
> @@ -1510,6 +1510,19 @@ gimple_call_fnspec (const gcall *stmt)
>  }
>if (gimple_call_builtin_p (stmt, BUILT_IN_NORMAL))
>  return builtin_fnspec (gimple_call_fndecl (stmt));
> +  tree fndecl = gimple_call_fndecl (stmt);
> +  /* If the call is to a replaceable operator delete and results
> + from a delete expression as opposed to a direct call to
> + such operator, then we can treat it as free.  */
> +  if (fndecl
> +  && DECL_IS_OPERATOR_DELETE_P (fndecl)
> +  && gimple_call_from_new_or_delete (stmt))
> +return ".co ";
> +  /* Similarly operator new can be treated as malloc.  */
> +  if (fndecl
> +  && DECL_IS_OPERATOR_NEW_P (fndecl)
> +  && gimple_call_from_new_or_delete (stmt))
> +return "mC";
>return "";
>  }
>
> diff --git a/gcc/gimple.h b/gcc/gimple.h
> index 3c9b9965f5a..fdb00d57b07 100644
> --- a/gcc/gimple.h
> +++ b/gcc/gimple.h
> @@ -3405,7 +3405,7 @@ gimple_call_set_from_new_or_delete (gcall *s, bool 
> from_new_or_delete_p)
> from a new or delete expression.  */
>
>  static inline bool
> -gimple_call_from_new_or_delete (gcall *s)
> +gimple_call_from_new_or_delete (const gcall *s)
>  {
>return (s->subcode & GF_CALL_FROM_NEW_OR_DELETE) != 0;
>  }
> diff --git a/gcc/testsuite/g++.dg/ipa/devirt-24.C 
> b/gcc/testsuite/g++.dg/ipa/devirt-24.C
> index eaef1f5b3f8..7b5b806dd05 100644
> --- a/gcc/testsuite/g++.dg/ipa/devirt-24.C
> +++ b/gcc/testsuite/g++.dg/ipa/devirt-24.C
> @@ -37,4 +37,4 @@ C *b = new (C);
>}
>  }
>  /* { dg-final { scan-ipa-dump-times "Discovered a virtual call to a known 
> target" 1 "inline" { xfail *-*-* } } } */
> -/* { dg-final { scan-ipa-dump-times "Aggregate passed by reference" 1 "cp"  
> } } */
> +/* { dg-final { scan-ipa-dump-times "Aggregate passed by reference" 2 "cp"  
> } } */


Re: PowerPC: Update long double IEEE 128-bit tests.

2020-11-06 Thread Michael Meissner via Gcc-patches
On Mon, Nov 02, 2020 at 07:00:15PM -0600, Segher Boessenkool wrote:
> Hi!
> 
> On Thu, Oct 22, 2020 at 06:07:14PM -0400, Michael Meissner wrote:
> > This patch fixes 3 tests in the testsuite that fail if long double is set
> > to IEEE 128-bit.
> 
> > * c-c++-common/dfp/convert-bfp-11.c: If long double is IEEE
> > 128-bit, skip the test.
> 
> > --- a/gcc/testsuite/c-c++-common/dfp/convert-bfp-11.c
> > +++ b/gcc/testsuite/c-c++-common/dfp/convert-bfp-11.c
> > @@ -5,6 +5,7 @@
> > Don't force 128-bit long doubles because runtime support depends
> > on glibc.  */
> >  
> > +#include 
> >  #include "convert.h"
> >  
> >  volatile _Decimal32 sd;
> > @@ -39,6 +40,12 @@ main ()
> >if (sizeof (long double) != 16)
> >  return 0;
> >  
> > +  /* This test is written to test IBM extended double, which is a pair of
> > + doubles.  If long double can hold a larger value than a double can, 
> > such
> > + as when long double is IEEE 128-bit, just exit immediately.  */
> 
> A double-double can hold bigger values than a double can, as well
> (if X is the biggest double, then X+Y is a valid double-double whenever
> you take Y small enough).
> 
> > +  if (LDBL_MAX_10_EXP > DBL_MAX_10_EXP)
> > +return 0;

Yes a double-double can hold more mantissa bits than a double, but the exponent
size is the same (which is what I'm testing).

> This is testing something different though: whether the base-10
> logarithm of the maximum finite double is different from that of the
> maximum finite double-double.
> 
> Is there no more direct test you can do?  Just test __FLOAT128__ maybe?
> The test is not even compiled if not powerpc*-linux, so you can test
> such macros just fine.

I will have to look at it.

> > * gcc.dg/nextafter-2.c: On PowerPC, if long double is IEEE
> > 128-bit, include math.h to get the built-in mapped correctly.
> 
> > diff --git a/gcc/testsuite/gcc.dg/nextafter-2.c 
> > b/gcc/testsuite/gcc.dg/nextafter-2.c
> > index e51ae94be0c..64e9e3c485f 100644
> > --- a/gcc/testsuite/gcc.dg/nextafter-2.c
> > +++ b/gcc/testsuite/gcc.dg/nextafter-2.c
> > @@ -13,4 +13,14 @@
> >  #  define NO_LONG_DOUBLE 1
> >  # endif
> >  #endif
> > +
> > +#if defined(_ARCH_PPC) && defined(__LONG_DOUBLE_IEEE128__)
> > +/* On PowerPC systems, long double uses either the IBM long double format, 
> > or
> > +   IEEE 128-bit format.  The compiler switches the long double built-in
> > +   function names and glibc switches the names when math.h is included.
> > +   Because this test is run with -fno-builtin, include math.h so that the
> > +   appropriate nextafter functions are called.  */
> > +#include 
> > +#endif
> > +
> >  #include "nextafter-1.c"
> 
> Please explain *what* mappings are made?  And why is it okay to do this
> in the testsuite, when all "normal" code (that does not do this) will
> just fail?

I can put in a better comment.  However, this test fails because it explicitly
does not include math.h and it uses -fno-builtin.  So the compiler can't
effectively map the nextafter math function.


> > * gcc.target/powerpc/pr70117.c: Add support for long double being
> > IEEE 128-bit.
> 
> That is not what the patch does -- it instead changes the code because
> it does not work correctly with long double ieee128 (which it already
> did claim to support!)
> 
> So what are the actual changes doing, why are they correct, why was the
> original not correct?
> 
> (It is easy to make a test not fail anymore: just delete it!  Something
> here should be better than that :-) )

I will have to look into it.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.ibm.com, phone: +1 (978) 899-4797


Re: [PATCH] c++: Don't purge the satisfaction caches

2020-11-06 Thread Jason Merrill via Gcc-patches

On 11/2/20 9:03 AM, Patrick Palka wrote:

On Tue, 27 Oct 2020, Patrick Palka wrote:


The adoption of P2104 means we can memoize the result of satisfaction
indefinitely and no longer have to clear the satisfaction caches on
various events that would affect satisfaction.  To that end, this patch
removes clear_satisfaction_cache and adjusts its callers appropriately.

This provides a massive reduction in compile time and memory use in some
cases.  For example, on the libstdc++ test std/ranges/adaptor/join.cc,
compile time and memory usage drops nearly 75%, from 7.5s/770MB to
2s/230MB, with a --enable-checking=release compiler.

[ This patch depends on

 c++: Check constraints only for candidate conversion functions.

   Without it, many of the libstdc++ range adaptor tests fail due to
   us now indefinitely memoizing a bogus satisfaction result caused by
   checking the constraints of a conversion function when we weren't
   supposed to, which led to a "use of foo_view::end() before deduction
   of auto" SFINAE error.  This went unnoticed without this patch because
   by the time we needed this satisfaction result again, we had cleared
   the satisfaction cache and finished deducing the return type of
   foo_view::end(), so on subsequent tries we computed the correct
   satisfaction result.  ]


With the library-side workaround r11-4584 for the range adaptor test
failures now committed, the mentioned frontend patch about candidate
conversion functions is no longer a prerequisite (and was not we want
anyway).



Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk (pending approval of the prerequisite patch)?  Also tested on
cmcstl2 and range-v3.


Now successfully bootstrapped and regtested on top of r11-4584, and also
tested on cmcstl2 and range-v3.  Does this look OK for trunk?


For me, this patch broke the view.join test in cmcstl2.  Are you not 
seeing that?


Jason



Re: Fwd: libstdc++: Attempt to resolve PR83562

2020-11-06 Thread Jonathan Yong via Gcc-patches

On 11/6/20 8:34 AM, Martin Storsjö wrote:

On Fri, 6 Nov 2020, Liu Hao via Gcc-patches wrote:


在 2020/10/29 下午3:56, Liu Hao 写道:

I forward it here for comments.

Basing on the behavior of both GCC and Clang, `__cxa_thread_atexit` 
is used to register the
destructor of thread_local objects directly, suggesting the first 
parameter should have `__thiscall`

convention.

libstdc++ used the default `__cdecl` convention and caused crashes on 
1686-w64-mingw32 (see
PR83562). But to my surprise, libcxxabi uses `__cdecl` too [1], but I 
haven't heard any of relevant

reports so far.

Original patch is attached in case you can't find it in gcc-patches.



FWIW, this patch looks good and correct to me, from a mingw perspective.

// Martin



Thanks pushed to gcc master branch as 
7fc0f78c3f43af1967cb7b1ee8f4947f3b890aa2.


OpenPGP_0x713B5FE29C145D45_and_old_rev.asc
Description: application/pgp-keys


OpenPGP_signature
Description: OpenPGP digital signature


Re: [PATCH] "used" attribute saves decl from linker garbage collection

2020-11-06 Thread H.J. Lu via Gcc-patches
On Fri, Nov 6, 2020 at 4:17 PM Jeff Law  wrote:
>
>
> On 11/6/20 5:13 PM, H.J. Lu wrote:
> > On Fri, Nov 6, 2020 at 4:01 PM Jeff Law  wrote:
> >>
> >> On 11/6/20 4:45 PM, H.J. Lu wrote:
> >>> On Fri, Nov 6, 2020 at 3:37 PM Jeff Law  wrote:
>  On 11/6/20 4:29 PM, H.J. Lu wrote:
> > On Fri, Nov 6, 2020 at 3:22 PM Jeff Law  wrote:
> >> On 11/5/20 7:34 AM, H.J. Lu via Gcc-patches wrote:
> >>> On Thu, Nov 5, 2020 at 3:37 AM Jozef Lawrynowicz
> >>>  wrote:
>  On Thu, Nov 05, 2020 at 06:21:21AM -0500, Hans-Peter Nilsson wrote:
> > On Wed, 4 Nov 2020, H.J. Lu wrote:
> >> .retain is ill-defined.   For example,
> >>
> >> [hjl@gnu-cfl-2 gcc]$ cat /tmp/x.c
> >> static int xyzzy __attribute__((__used__));
> >> [hjl@gnu-cfl-2 gcc]$ ./xgcc -B./ -S /tmp/x.c -fcommon
> >> [hjl@gnu-cfl-2 gcc]$ cat x.s
> >> .file "x.c"
> >> .text
> >> .retain xyzzy  < What does it do?
> >> .local xyzzy
> >> .comm xyzzy,4,4
> >> .ident "GCC: (GNU) 11.0.0 20201103 (experimental)"
> >> .section .note.GNU-stack,"",@progbits
> >> [hjl@gnu-cfl-2 gcc]$
> > To answer that question: it's up to the assembler, but for ELF
> > and SHF_GNU_RETAIN, it seems obvious it'd tell the assembler to
> > set SHF_GNU_RETAIN for the section where the symbol ends up.
> > We both know this isn't rocket science with binutils.
>  Indeed, and my patch handles it trivially:
>  https://sourceware.org/pipermail/binutils/2020-November/113993.html
> 
>    +void
>    +obj_elf_retain (int arg ATTRIBUTE_UNUSED)
>     snip 
>    +  sym = get_sym_from_input_line_and_check ();
>    +  symbol_get_obj (sym)->retain = 1;
> 
>    @@ -2624,6 +2704,9 @@ elf_frob_symbol (symbolS *symp, int *puntp)
>  }
> }
> 
>    +  if (symbol_get_obj (symp)->retain)
>    +elf_section_flags (S_GET_SEGMENT (symp)) |= SHF_GNU_RETAIN;
>    +
>   /* Double check weak symbols.  */
>   if (S_IS_WEAK (symp))
> {
> 
>  We could check that the symbol named in the .retain directive has
>  already been defined, however this isn't compatible with GCC
>  mark_decl_preserved handling, since mark_decl_preserved is called
>  emitted before the local symbols are defined in the assembly output
>  file.
> 
>  GAS should at least validate that the symbol named in the .retain
>  directive does end up as a symbol though.
> 
> >>> Don't add .retain.
> >> Why?  I don't see why you find it so objectionable.
> >>
> > An ELF symbol directive should operate on symbol table:
> >
> > http://www.sco.com/developers/gabi/latest/ch4.symtab.html
> >
> > not the section flags where the symbol is defined.
>  I agree in general, but I think this is one of those cases where it's
>  not so clear.  And what you're talking about is an implementation detail.
> >>> There is no need for such a hack.  The proper thing to do in ELF is
> >>> to place such a symbol in a section with SHF_GNU_RETAIN flag.   This
> >>> also avoids the question what to do with SHN_COMMON.
> >> I'm not sure that's a good idea either.  Moving symbols into a section
> >> other than they'd normally live doesn't seem all that wise.
> > In ELF, a symbol must be defined in a section.  If we want to keep a symbol,
> > we should place it in an SHF_GNU_RETAIN section.
>
> Again, that's an implementation detail and it's not clear to me that one
> approach is inherently better than the other.
>
>
> >
> >> Let's face it, there's not a great solution here.  If we mark its
> >> existing section, then everything in that section gets kept.  If we put
> > FWIW, this is what .retain direct does and is one reason why I object
> > it.
>
> We could make .retain work with either approach.I don't see .retain
> as a problem at all.
>
>
>
> >
> >> the object into a different section than it would normally live, then
> >> that opens a whole new can of worms.
> > We should place it in a section which it normally lives in and mark the
> > section with SHF_GNU_RETAIN.
>
> And why not do that with .retain?  We define its semantics as precisely

But the .retain directive implementation being discussed here is different.
One problem with the .retain directive is we can have

.section .data
foo:
...
bar:

.retain bar
...
xxx:
...

What should assembler do with ".retain bar"?

> what you've written above.  The referenced symbol goes into its usual
> section and its section is marked with SHF_GNU_RETAIN.  That seems much
> cleaner than having to track all this in the compiler so that it can
> twiddle the section flags.

When GCC emits a symbol definition, it places the symbol in a section
with proper
attributes 

Re: [PATCH] "used" attribute saves decl from linker garbage collection

2020-11-06 Thread Jeff Law via Gcc-patches


On 11/6/20 5:13 PM, H.J. Lu wrote:
> On Fri, Nov 6, 2020 at 4:01 PM Jeff Law  wrote:
>>
>> On 11/6/20 4:45 PM, H.J. Lu wrote:
>>> On Fri, Nov 6, 2020 at 3:37 PM Jeff Law  wrote:
 On 11/6/20 4:29 PM, H.J. Lu wrote:
> On Fri, Nov 6, 2020 at 3:22 PM Jeff Law  wrote:
>> On 11/5/20 7:34 AM, H.J. Lu via Gcc-patches wrote:
>>> On Thu, Nov 5, 2020 at 3:37 AM Jozef Lawrynowicz
>>>  wrote:
 On Thu, Nov 05, 2020 at 06:21:21AM -0500, Hans-Peter Nilsson wrote:
> On Wed, 4 Nov 2020, H.J. Lu wrote:
>> .retain is ill-defined.   For example,
>>
>> [hjl@gnu-cfl-2 gcc]$ cat /tmp/x.c
>> static int xyzzy __attribute__((__used__));
>> [hjl@gnu-cfl-2 gcc]$ ./xgcc -B./ -S /tmp/x.c -fcommon
>> [hjl@gnu-cfl-2 gcc]$ cat x.s
>> .file "x.c"
>> .text
>> .retain xyzzy  < What does it do?
>> .local xyzzy
>> .comm xyzzy,4,4
>> .ident "GCC: (GNU) 11.0.0 20201103 (experimental)"
>> .section .note.GNU-stack,"",@progbits
>> [hjl@gnu-cfl-2 gcc]$
> To answer that question: it's up to the assembler, but for ELF
> and SHF_GNU_RETAIN, it seems obvious it'd tell the assembler to
> set SHF_GNU_RETAIN for the section where the symbol ends up.
> We both know this isn't rocket science with binutils.
 Indeed, and my patch handles it trivially:
 https://sourceware.org/pipermail/binutils/2020-November/113993.html

   +void
   +obj_elf_retain (int arg ATTRIBUTE_UNUSED)
    snip 
   +  sym = get_sym_from_input_line_and_check ();
   +  symbol_get_obj (sym)->retain = 1;

   @@ -2624,6 +2704,9 @@ elf_frob_symbol (symbolS *symp, int *puntp)
 }
}

   +  if (symbol_get_obj (symp)->retain)
   +elf_section_flags (S_GET_SEGMENT (symp)) |= SHF_GNU_RETAIN;
   +
  /* Double check weak symbols.  */
  if (S_IS_WEAK (symp))
{

 We could check that the symbol named in the .retain directive has
 already been defined, however this isn't compatible with GCC
 mark_decl_preserved handling, since mark_decl_preserved is called
 emitted before the local symbols are defined in the assembly output
 file.

 GAS should at least validate that the symbol named in the .retain
 directive does end up as a symbol though.

>>> Don't add .retain.
>> Why?  I don't see why you find it so objectionable.
>>
> An ELF symbol directive should operate on symbol table:
>
> http://www.sco.com/developers/gabi/latest/ch4.symtab.html
>
> not the section flags where the symbol is defined.
 I agree in general, but I think this is one of those cases where it's
 not so clear.  And what you're talking about is an implementation detail.
>>> There is no need for such a hack.  The proper thing to do in ELF is
>>> to place such a symbol in a section with SHF_GNU_RETAIN flag.   This
>>> also avoids the question what to do with SHN_COMMON.
>> I'm not sure that's a good idea either.  Moving symbols into a section
>> other than they'd normally live doesn't seem all that wise.
> In ELF, a symbol must be defined in a section.  If we want to keep a symbol,
> we should place it in an SHF_GNU_RETAIN section.

Again, that's an implementation detail and it's not clear to me that one
approach is inherently better than the other.


>
>> Let's face it, there's not a great solution here.  If we mark its
>> existing section, then everything in that section gets kept.  If we put
> FWIW, this is what .retain direct does and is one reason why I object
> it.

We could make .retain work with either approach.    I don't see .retain
as a problem at all. 



>
>> the object into a different section than it would normally live, then
>> that opens a whole new can of worms.
> We should place it in a section which it normally lives in and mark the
> section with SHF_GNU_RETAIN.

And why not do that with .retain?  We define its semantics as precisely
what you've written above.  The referenced symbol goes into its usual
section and its section is marked with SHF_GNU_RETAIN.  That seems much
cleaner than having to track all this in the compiler so that it can
twiddle the section flags.


jeff

>



Re: [PATCH] "used" attribute saves decl from linker garbage collection

2020-11-06 Thread H.J. Lu via Gcc-patches
On Fri, Nov 6, 2020 at 4:01 PM Jeff Law  wrote:
>
>
> On 11/6/20 4:45 PM, H.J. Lu wrote:
> > On Fri, Nov 6, 2020 at 3:37 PM Jeff Law  wrote:
> >>
> >> On 11/6/20 4:29 PM, H.J. Lu wrote:
> >>> On Fri, Nov 6, 2020 at 3:22 PM Jeff Law  wrote:
>  On 11/5/20 7:34 AM, H.J. Lu via Gcc-patches wrote:
> > On Thu, Nov 5, 2020 at 3:37 AM Jozef Lawrynowicz
> >  wrote:
> >> On Thu, Nov 05, 2020 at 06:21:21AM -0500, Hans-Peter Nilsson wrote:
> >>> On Wed, 4 Nov 2020, H.J. Lu wrote:
>  .retain is ill-defined.   For example,
> 
>  [hjl@gnu-cfl-2 gcc]$ cat /tmp/x.c
>  static int xyzzy __attribute__((__used__));
>  [hjl@gnu-cfl-2 gcc]$ ./xgcc -B./ -S /tmp/x.c -fcommon
>  [hjl@gnu-cfl-2 gcc]$ cat x.s
>  .file "x.c"
>  .text
>  .retain xyzzy  < What does it do?
>  .local xyzzy
>  .comm xyzzy,4,4
>  .ident "GCC: (GNU) 11.0.0 20201103 (experimental)"
>  .section .note.GNU-stack,"",@progbits
>  [hjl@gnu-cfl-2 gcc]$
> >>> To answer that question: it's up to the assembler, but for ELF
> >>> and SHF_GNU_RETAIN, it seems obvious it'd tell the assembler to
> >>> set SHF_GNU_RETAIN for the section where the symbol ends up.
> >>> We both know this isn't rocket science with binutils.
> >> Indeed, and my patch handles it trivially:
> >> https://sourceware.org/pipermail/binutils/2020-November/113993.html
> >>
> >>   +void
> >>   +obj_elf_retain (int arg ATTRIBUTE_UNUSED)
> >>    snip 
> >>   +  sym = get_sym_from_input_line_and_check ();
> >>   +  symbol_get_obj (sym)->retain = 1;
> >>
> >>   @@ -2624,6 +2704,9 @@ elf_frob_symbol (symbolS *symp, int *puntp)
> >> }
> >>}
> >>
> >>   +  if (symbol_get_obj (symp)->retain)
> >>   +elf_section_flags (S_GET_SEGMENT (symp)) |= SHF_GNU_RETAIN;
> >>   +
> >>  /* Double check weak symbols.  */
> >>  if (S_IS_WEAK (symp))
> >>{
> >>
> >> We could check that the symbol named in the .retain directive has
> >> already been defined, however this isn't compatible with GCC
> >> mark_decl_preserved handling, since mark_decl_preserved is called
> >> emitted before the local symbols are defined in the assembly output
> >> file.
> >>
> >> GAS should at least validate that the symbol named in the .retain
> >> directive does end up as a symbol though.
> >>
> > Don't add .retain.
>  Why?  I don't see why you find it so objectionable.
> 
> >>> An ELF symbol directive should operate on symbol table:
> >>>
> >>> http://www.sco.com/developers/gabi/latest/ch4.symtab.html
> >>>
> >>> not the section flags where the symbol is defined.
> >> I agree in general, but I think this is one of those cases where it's
> >> not so clear.  And what you're talking about is an implementation detail.
> > There is no need for such a hack.  The proper thing to do in ELF is
> > to place such a symbol in a section with SHF_GNU_RETAIN flag.   This
> > also avoids the question what to do with SHN_COMMON.
>
> I'm not sure that's a good idea either.  Moving symbols into a section
> other than they'd normally live doesn't seem all that wise.

In ELF, a symbol must be defined in a section.  If we want to keep a symbol,
we should place it in an SHF_GNU_RETAIN section.

>
> Let's face it, there's not a great solution here.  If we mark its
> existing section, then everything in that section gets kept.  If we put

FWIW, this is what .retain direct does and is one reason why I object
it.

> the object into a different section than it would normally live, then
> that opens a whole new can of worms.

We should place it in a section which it normally lives in and mark the
section with SHF_GNU_RETAIN.

-- 
H.J.


Re: [PATCH] "used" attribute saves decl from linker garbage collection

2020-11-06 Thread Jeff Law via Gcc-patches


On 11/6/20 4:45 PM, H.J. Lu wrote:
> On Fri, Nov 6, 2020 at 3:37 PM Jeff Law  wrote:
>>
>> On 11/6/20 4:29 PM, H.J. Lu wrote:
>>> On Fri, Nov 6, 2020 at 3:22 PM Jeff Law  wrote:
 On 11/5/20 7:34 AM, H.J. Lu via Gcc-patches wrote:
> On Thu, Nov 5, 2020 at 3:37 AM Jozef Lawrynowicz
>  wrote:
>> On Thu, Nov 05, 2020 at 06:21:21AM -0500, Hans-Peter Nilsson wrote:
>>> On Wed, 4 Nov 2020, H.J. Lu wrote:
 .retain is ill-defined.   For example,

 [hjl@gnu-cfl-2 gcc]$ cat /tmp/x.c
 static int xyzzy __attribute__((__used__));
 [hjl@gnu-cfl-2 gcc]$ ./xgcc -B./ -S /tmp/x.c -fcommon
 [hjl@gnu-cfl-2 gcc]$ cat x.s
 .file "x.c"
 .text
 .retain xyzzy  < What does it do?
 .local xyzzy
 .comm xyzzy,4,4
 .ident "GCC: (GNU) 11.0.0 20201103 (experimental)"
 .section .note.GNU-stack,"",@progbits
 [hjl@gnu-cfl-2 gcc]$
>>> To answer that question: it's up to the assembler, but for ELF
>>> and SHF_GNU_RETAIN, it seems obvious it'd tell the assembler to
>>> set SHF_GNU_RETAIN for the section where the symbol ends up.
>>> We both know this isn't rocket science with binutils.
>> Indeed, and my patch handles it trivially:
>> https://sourceware.org/pipermail/binutils/2020-November/113993.html
>>
>>   +void
>>   +obj_elf_retain (int arg ATTRIBUTE_UNUSED)
>>    snip 
>>   +  sym = get_sym_from_input_line_and_check ();
>>   +  symbol_get_obj (sym)->retain = 1;
>>
>>   @@ -2624,6 +2704,9 @@ elf_frob_symbol (symbolS *symp, int *puntp)
>> }
>>}
>>
>>   +  if (symbol_get_obj (symp)->retain)
>>   +elf_section_flags (S_GET_SEGMENT (symp)) |= SHF_GNU_RETAIN;
>>   +
>>  /* Double check weak symbols.  */
>>  if (S_IS_WEAK (symp))
>>{
>>
>> We could check that the symbol named in the .retain directive has
>> already been defined, however this isn't compatible with GCC
>> mark_decl_preserved handling, since mark_decl_preserved is called
>> emitted before the local symbols are defined in the assembly output
>> file.
>>
>> GAS should at least validate that the symbol named in the .retain
>> directive does end up as a symbol though.
>>
> Don't add .retain.
 Why?  I don't see why you find it so objectionable.

>>> An ELF symbol directive should operate on symbol table:
>>>
>>> http://www.sco.com/developers/gabi/latest/ch4.symtab.html
>>>
>>> not the section flags where the symbol is defined.
>> I agree in general, but I think this is one of those cases where it's
>> not so clear.  And what you're talking about is an implementation detail.
> There is no need for such a hack.  The proper thing to do in ELF is
> to place such a symbol in a section with SHF_GNU_RETAIN flag.   This
> also avoids the question what to do with SHN_COMMON.

I'm not sure that's a good idea either.  Moving symbols into a section
other than they'd normally live doesn't seem all that wise.


Let's face it, there's not a great solution here.  If we mark its
existing section, then everything in that section gets kept.  If we put
the object into a different section than it would normally live, then
that opens a whole new can of worms.


jeff



[PATCH] rs6000: Don't use operands[] for temporaries in define_expand

2020-11-06 Thread Segher Boessenkool
In ac001f5ce604 Alan fixed my thinko using operands that do not refer
to anything mentioned in the RTL pattern.  Instead, it just uses fresh
new local rtxes for those.

This patch takes that a tiny bit further: it uses local rtx for all
temporaries used in the expanders.  As a bonus that simplifies the code
a tiny bit as well.


2020-11-06  Segher Boessenkool  

* config/rs6000/rs6000.md (@tablejump_normal): Don't abuse
operands[].
(@tablejump_nospec): Ditto.

---
 gcc/config/rs6000/rs6000.md | 18 ++
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index acbf130..5e5ad9f 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -12732,14 +12732,11 @@ (define_expand "@tablejump_normal"
(use (match_operand:P 1))]
   "rs6000_speculate_indirect_jumps"
 {
-  rtx off;
-  operands[0] = force_reg (SImode, operands[0]);
-  if (mode == SImode)
-off = operands[0];
-  else
+  rtx off = force_reg (SImode, operands[0]);
+  if (mode != SImode)
 {
+  rtx src = gen_rtx_fmt_e (SIGN_EXTEND, Pmode, off);
   off = gen_reg_rtx (Pmode);
-  rtx src = gen_rtx_fmt_e (SIGN_EXTEND, Pmode, operands[0]);
   emit_move_insn (off, src);
 }
 
@@ -12757,14 +12754,11 @@ (define_expand "@tablejump_nospec"
(use (match_operand:CC 2))]
   "!rs6000_speculate_indirect_jumps"
 {
-  rtx off;
-  operands[0] = force_reg (SImode, operands[0]);
-  if (mode == SImode)
-off = operands[0];
-  else
+  rtx off = force_reg (SImode, operands[0]);
+  if (mode != SImode)
 {
+  rtx src = gen_rtx_fmt_e (SIGN_EXTEND, Pmode, off);
   off = gen_reg_rtx (Pmode);
-  rtx src = gen_rtx_fmt_e (SIGN_EXTEND, Pmode, operands[0]);
   emit_move_insn (off, src);
 }
 
-- 
1.8.3.1



Re: [PATCH] libiberty/pex-win32.c: Initialize orig_err

2020-11-06 Thread Jeff Law via Gcc-patches


On 9/18/20 6:33 AM, Christophe Lyon wrote:
> On Thu, 17 Sep 2020 at 23:33, Jeff Law  wrote:
>>
>> On 9/14/20 3:29 AM, Christophe Lyon via Gcc-patches wrote:
>>> Initializing orig_err avoids a warning: "may be used uninitialized".
>>>
>>> 2020-09-14  Torbjörn SVENSSON 
>>>   Christophe Lyon  
>>>
>>>   libiberty/
>>>   * pex-win32 (pex_win32_exec_child): Initialize orig_err.
>> Rather than just blindly initializing orig_err, we'd actually prefer to
>> have a compilable testcase and analyze why we're getting the warning.  A
>> false positive -Wuninitialized warning is often a missed optimization
>> under the hood.
>>
> OK, I filed https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97108
> after checking the problem is still present with trunk and not only
> with the old gcc version we have in our builders.

Thanks.  I've put an initial analysis in there.  I think it's pointing
at a failing in both the traditional threader as well as the backwards
threader.  The backwards threader is probably the place to fix it in the
long term.


In the immediate term, go ahead with the trivial initialization so that
it doesn't show up in your builds.  We'll track the threader issue via
the BZ.


Thanks,

jeff



Re: [PATCH] "used" attribute saves decl from linker garbage collection

2020-11-06 Thread H.J. Lu via Gcc-patches
On Fri, Nov 6, 2020 at 3:37 PM Jeff Law  wrote:
>
>
> On 11/6/20 4:29 PM, H.J. Lu wrote:
> > On Fri, Nov 6, 2020 at 3:22 PM Jeff Law  wrote:
> >>
> >> On 11/5/20 7:34 AM, H.J. Lu via Gcc-patches wrote:
> >>> On Thu, Nov 5, 2020 at 3:37 AM Jozef Lawrynowicz
> >>>  wrote:
>  On Thu, Nov 05, 2020 at 06:21:21AM -0500, Hans-Peter Nilsson wrote:
> > On Wed, 4 Nov 2020, H.J. Lu wrote:
> >> .retain is ill-defined.   For example,
> >>
> >> [hjl@gnu-cfl-2 gcc]$ cat /tmp/x.c
> >> static int xyzzy __attribute__((__used__));
> >> [hjl@gnu-cfl-2 gcc]$ ./xgcc -B./ -S /tmp/x.c -fcommon
> >> [hjl@gnu-cfl-2 gcc]$ cat x.s
> >> .file "x.c"
> >> .text
> >> .retain xyzzy  < What does it do?
> >> .local xyzzy
> >> .comm xyzzy,4,4
> >> .ident "GCC: (GNU) 11.0.0 20201103 (experimental)"
> >> .section .note.GNU-stack,"",@progbits
> >> [hjl@gnu-cfl-2 gcc]$
> > To answer that question: it's up to the assembler, but for ELF
> > and SHF_GNU_RETAIN, it seems obvious it'd tell the assembler to
> > set SHF_GNU_RETAIN for the section where the symbol ends up.
> > We both know this isn't rocket science with binutils.
>  Indeed, and my patch handles it trivially:
>  https://sourceware.org/pipermail/binutils/2020-November/113993.html
> 
>    +void
>    +obj_elf_retain (int arg ATTRIBUTE_UNUSED)
>     snip 
>    +  sym = get_sym_from_input_line_and_check ();
>    +  symbol_get_obj (sym)->retain = 1;
> 
>    @@ -2624,6 +2704,9 @@ elf_frob_symbol (symbolS *symp, int *puntp)
>  }
> }
> 
>    +  if (symbol_get_obj (symp)->retain)
>    +elf_section_flags (S_GET_SEGMENT (symp)) |= SHF_GNU_RETAIN;
>    +
>   /* Double check weak symbols.  */
>   if (S_IS_WEAK (symp))
> {
> 
>  We could check that the symbol named in the .retain directive has
>  already been defined, however this isn't compatible with GCC
>  mark_decl_preserved handling, since mark_decl_preserved is called
>  emitted before the local symbols are defined in the assembly output
>  file.
> 
>  GAS should at least validate that the symbol named in the .retain
>  directive does end up as a symbol though.
> 
> >>> Don't add .retain.
> >> Why?  I don't see why you find it so objectionable.
> >>
> > An ELF symbol directive should operate on symbol table:
> >
> > http://www.sco.com/developers/gabi/latest/ch4.symtab.html
> >
> > not the section flags where the symbol is defined.
>
> I agree in general, but I think this is one of those cases where it's
> not so clear.  And what you're talking about is an implementation detail.

There is no need for such a hack.  The proper thing to do in ELF is
to place such a symbol in a section with SHF_GNU_RETAIN flag.   This
also avoids the question what to do with SHN_COMMON.

-- 
H.J.


Re: [PATCH] "used" attribute saves decl from linker garbage collection

2020-11-06 Thread Jeff Law via Gcc-patches


On 11/6/20 4:29 PM, H.J. Lu wrote:
> On Fri, Nov 6, 2020 at 3:22 PM Jeff Law  wrote:
>>
>> On 11/5/20 7:34 AM, H.J. Lu via Gcc-patches wrote:
>>> On Thu, Nov 5, 2020 at 3:37 AM Jozef Lawrynowicz
>>>  wrote:
 On Thu, Nov 05, 2020 at 06:21:21AM -0500, Hans-Peter Nilsson wrote:
> On Wed, 4 Nov 2020, H.J. Lu wrote:
>> .retain is ill-defined.   For example,
>>
>> [hjl@gnu-cfl-2 gcc]$ cat /tmp/x.c
>> static int xyzzy __attribute__((__used__));
>> [hjl@gnu-cfl-2 gcc]$ ./xgcc -B./ -S /tmp/x.c -fcommon
>> [hjl@gnu-cfl-2 gcc]$ cat x.s
>> .file "x.c"
>> .text
>> .retain xyzzy  < What does it do?
>> .local xyzzy
>> .comm xyzzy,4,4
>> .ident "GCC: (GNU) 11.0.0 20201103 (experimental)"
>> .section .note.GNU-stack,"",@progbits
>> [hjl@gnu-cfl-2 gcc]$
> To answer that question: it's up to the assembler, but for ELF
> and SHF_GNU_RETAIN, it seems obvious it'd tell the assembler to
> set SHF_GNU_RETAIN for the section where the symbol ends up.
> We both know this isn't rocket science with binutils.
 Indeed, and my patch handles it trivially:
 https://sourceware.org/pipermail/binutils/2020-November/113993.html

   +void
   +obj_elf_retain (int arg ATTRIBUTE_UNUSED)
    snip 
   +  sym = get_sym_from_input_line_and_check ();
   +  symbol_get_obj (sym)->retain = 1;

   @@ -2624,6 +2704,9 @@ elf_frob_symbol (symbolS *symp, int *puntp)
 }
}

   +  if (symbol_get_obj (symp)->retain)
   +elf_section_flags (S_GET_SEGMENT (symp)) |= SHF_GNU_RETAIN;
   +
  /* Double check weak symbols.  */
  if (S_IS_WEAK (symp))
{

 We could check that the symbol named in the .retain directive has
 already been defined, however this isn't compatible with GCC
 mark_decl_preserved handling, since mark_decl_preserved is called
 emitted before the local symbols are defined in the assembly output
 file.

 GAS should at least validate that the symbol named in the .retain
 directive does end up as a symbol though.

>>> Don't add .retain.
>> Why?  I don't see why you find it so objectionable.
>>
> An ELF symbol directive should operate on symbol table:
>
> http://www.sco.com/developers/gabi/latest/ch4.symtab.html
>
> not the section flags where the symbol is defined.

I agree in general, but I think this is one of those cases where it's
not so clear.  And what you're talking about is an implementation detail.


jeff



Re: [PING] [PATCH] S/390: Do not turn maybe-uninitialized warnings into errors

2020-11-06 Thread Jeff Law via Gcc-patches


On 10/30/20 4:08 AM, Stefan Schulze Frielinghaus wrote:
> On Wed, Oct 28, 2020 at 11:34:53AM -0600, Jeff Law wrote:
>> On 10/28/20 11:29 AM, Stefan Schulze Frielinghaus wrote:
>>> On Wed, Oct 28, 2020 at 08:39:41AM -0600, Jeff Law wrote:
 On 10/28/20 3:38 AM, Stefan Schulze Frielinghaus via Gcc-patches wrote:
> On Mon, Oct 05, 2020 at 02:02:57PM +0200, Stefan Schulze Frielinghaus via 
> Gcc-patches wrote:
>> On Tue, Sep 22, 2020 at 02:59:30PM +0200, Andreas Krebbel wrote:
>>> On 15.09.20 17:02, Stefan Schulze Frielinghaus wrote:
 Over the last couple of months quite a few warnings about uninitialized
 variables were raised while building GCC.  A reason why these warnings
 show up on S/390 only is due to the aggressive inlining settings here.
 Some of these warnings (2c832ffedf0, b776bdca932, 2786c0221b6,
 1657178f59b) could be fixed or in case of a false positive silenced by
 initializing the corresponding variable.  Since the latter reoccurs and
 while bootstrapping such warnings are turned into errors bootstrapping
 fails on S/390 consistently.  Therefore, for the moment do not turn
 those warnings into errors.

 config/ChangeLog:

* warnings.m4: Do not turn maybe-uninitialized warnings into 
 errors
on S/390.

 fixincludes/ChangeLog:

* configure: Regenerate.

 gcc/ChangeLog:

* configure: Regenerate.

 libcc1/ChangeLog:

* configure: Regenerate.

 libcpp/ChangeLog:

* configure: Regenerate.

 libdecnumber/ChangeLog:

* configure: Regenerate.
>>> That change looks good to me. Could a global reviewer please comment!
>> Ping
> Ping
 I think this would be a huge mistake to install.
>>> The root cause why those false positives show up on S/390 only seems to
>>> be of more aggressive inlining w.r.t. other architectures.  Because of
>>> bigger caches and a rather huge function call overhead we greatly
>>> benefit from those inlining parameters. Thus:
>>>
>>> 1) Reverting those parameters would have a negative performance impact.
>>>
>>> 2) Fixing the maybe-uninitialized warnings analysis itself seems not to
>>>happen in the near future (assuming that it is fixable at all).
>>>
>>> 3) Silencing the warning by initialising the variable itself also seems
>>>to be undesired and feels like a fight against windmills ;-)
>>>
>>> 4) Not lifting maybe-uninitialized warnings to errors on S/390 only.
>>>
>>> Option (4) has the least intrusive effect to me.  At least then it is
>>> not necessary to bootstrap with --disable-werror and we would still
>>> treat all other warnings as errors.  All maybe-uninitialized warnings
>>> which are triggered in common code with non-aggressive inlining are
>>> still caught by other architectures.  Therefore, I'm wondering why this
>>> should be a huge mistake?  What would you propose instead?
>> I'm aware of all that.  What I think it all argues is that y'all need to
>> address the issues because of how you've changed the tuning on the s390
>> port.  Simply disabling things like you've suggested is, IMHO, horribly
>> wrong.
>>
>>
>> Improve the analysis, dummy initializers, pragmas all seem viable.  But
>> again, it feels like it's something the s390 maintainers will have to
>> take the lead on because of how you've retuned the port.
> Fixing the analysis is of course the best option.  However, this sounds
> like a non-trivial task to me and I'm missing a lot of context here,
> i.e., I'm not sure what the initial goals were and if it is possible to
> meet those with the requirements which are necessary to solve those
> false positives (currently having PR96564 in mind where it was mentioned
> that alias info is not enough but also flow-based info is required; does
> this imply that we would have to reschedule the analysis at later time
> which was not desired in the first place etc.).

There are going to be cases we can't solve with just improvements in the
analysis.  My point is that we have several tools in our toolbox and we
should be looking at those to solve the problem rather than just
disabling the warning. 


>
> In the past I tried to come up with some dummy initializers which were
> tough to get accepted (which I can understand up to some degree).  For
> example, this one is still open (I would be happy if you could have a
> look at it and accept/reject):
> https://gcc.gnu.org/pipermail/gcc-patches/2020-June/547063.html
>
> Then there is at least one unreported case (similar to PR96564) where we
> are not talking about a variable of scalar type but of an aggregate
> where only one struct member must be initialized in order to silence the
> warning.  Not sure whether a patch would be accepted where I 

Re: [PATCH] "used" attribute saves decl from linker garbage collection

2020-11-06 Thread H.J. Lu via Gcc-patches
On Fri, Nov 6, 2020 at 3:22 PM Jeff Law  wrote:
>
>
> On 11/5/20 7:34 AM, H.J. Lu via Gcc-patches wrote:
> > On Thu, Nov 5, 2020 at 3:37 AM Jozef Lawrynowicz
> >  wrote:
> >> On Thu, Nov 05, 2020 at 06:21:21AM -0500, Hans-Peter Nilsson wrote:
> >>> On Wed, 4 Nov 2020, H.J. Lu wrote:
>  .retain is ill-defined.   For example,
> 
>  [hjl@gnu-cfl-2 gcc]$ cat /tmp/x.c
>  static int xyzzy __attribute__((__used__));
>  [hjl@gnu-cfl-2 gcc]$ ./xgcc -B./ -S /tmp/x.c -fcommon
>  [hjl@gnu-cfl-2 gcc]$ cat x.s
>  .file "x.c"
>  .text
>  .retain xyzzy  < What does it do?
>  .local xyzzy
>  .comm xyzzy,4,4
>  .ident "GCC: (GNU) 11.0.0 20201103 (experimental)"
>  .section .note.GNU-stack,"",@progbits
>  [hjl@gnu-cfl-2 gcc]$
> >>> To answer that question: it's up to the assembler, but for ELF
> >>> and SHF_GNU_RETAIN, it seems obvious it'd tell the assembler to
> >>> set SHF_GNU_RETAIN for the section where the symbol ends up.
> >>> We both know this isn't rocket science with binutils.
> >> Indeed, and my patch handles it trivially:
> >> https://sourceware.org/pipermail/binutils/2020-November/113993.html
> >>
> >>   +void
> >>   +obj_elf_retain (int arg ATTRIBUTE_UNUSED)
> >>    snip 
> >>   +  sym = get_sym_from_input_line_and_check ();
> >>   +  symbol_get_obj (sym)->retain = 1;
> >>
> >>   @@ -2624,6 +2704,9 @@ elf_frob_symbol (symbolS *symp, int *puntp)
> >> }
> >>}
> >>
> >>   +  if (symbol_get_obj (symp)->retain)
> >>   +elf_section_flags (S_GET_SEGMENT (symp)) |= SHF_GNU_RETAIN;
> >>   +
> >>  /* Double check weak symbols.  */
> >>  if (S_IS_WEAK (symp))
> >>{
> >>
> >> We could check that the symbol named in the .retain directive has
> >> already been defined, however this isn't compatible with GCC
> >> mark_decl_preserved handling, since mark_decl_preserved is called
> >> emitted before the local symbols are defined in the assembly output
> >> file.
> >>
> >> GAS should at least validate that the symbol named in the .retain
> >> directive does end up as a symbol though.
> >>
> > Don't add .retain.
>
> Why?  I don't see why you find it so objectionable.
>

An ELF symbol directive should operate on symbol table:

http://www.sco.com/developers/gabi/latest/ch4.symtab.html

not the section flags where the symbol is defined.

-- 
H.J.


Re: [PATCH] "used" attribute saves decl from linker garbage collection

2020-11-06 Thread Jeff Law via Gcc-patches


On 11/5/20 7:34 AM, H.J. Lu via Gcc-patches wrote:
> On Thu, Nov 5, 2020 at 3:37 AM Jozef Lawrynowicz
>  wrote:
>> On Thu, Nov 05, 2020 at 06:21:21AM -0500, Hans-Peter Nilsson wrote:
>>> On Wed, 4 Nov 2020, H.J. Lu wrote:
 .retain is ill-defined.   For example,

 [hjl@gnu-cfl-2 gcc]$ cat /tmp/x.c
 static int xyzzy __attribute__((__used__));
 [hjl@gnu-cfl-2 gcc]$ ./xgcc -B./ -S /tmp/x.c -fcommon
 [hjl@gnu-cfl-2 gcc]$ cat x.s
 .file "x.c"
 .text
 .retain xyzzy  < What does it do?
 .local xyzzy
 .comm xyzzy,4,4
 .ident "GCC: (GNU) 11.0.0 20201103 (experimental)"
 .section .note.GNU-stack,"",@progbits
 [hjl@gnu-cfl-2 gcc]$
>>> To answer that question: it's up to the assembler, but for ELF
>>> and SHF_GNU_RETAIN, it seems obvious it'd tell the assembler to
>>> set SHF_GNU_RETAIN for the section where the symbol ends up.
>>> We both know this isn't rocket science with binutils.
>> Indeed, and my patch handles it trivially:
>> https://sourceware.org/pipermail/binutils/2020-November/113993.html
>>
>>   +void
>>   +obj_elf_retain (int arg ATTRIBUTE_UNUSED)
>>    snip 
>>   +  sym = get_sym_from_input_line_and_check ();
>>   +  symbol_get_obj (sym)->retain = 1;
>>
>>   @@ -2624,6 +2704,9 @@ elf_frob_symbol (symbolS *symp, int *puntp)
>> }
>>}
>>
>>   +  if (symbol_get_obj (symp)->retain)
>>   +elf_section_flags (S_GET_SEGMENT (symp)) |= SHF_GNU_RETAIN;
>>   +
>>  /* Double check weak symbols.  */
>>  if (S_IS_WEAK (symp))
>>{
>>
>> We could check that the symbol named in the .retain directive has
>> already been defined, however this isn't compatible with GCC
>> mark_decl_preserved handling, since mark_decl_preserved is called
>> emitted before the local symbols are defined in the assembly output
>> file.
>>
>> GAS should at least validate that the symbol named in the .retain
>> directive does end up as a symbol though.
>>
> Don't add .retain.

Why?  I don't see why you find it so objectionable.


jeff



Re: [PATCH] "used" attribute saves decl from linker garbage collection

2020-11-06 Thread Jeff Law via Gcc-patches


On 11/5/20 4:00 AM, Jozef Lawrynowicz wrote:
> On Wed, Nov 04, 2020 at 03:58:56PM -0800, H.J. Lu wrote:
>> On Wed, Nov 4, 2020 at 3:00 PM Hans-Peter Nilsson  wrote:
>>> On Wed, 4 Nov 2020, H.J. Lu wrote:
 On Wed, Nov 4, 2020 at 1:56 PM Hans-Peter Nilsson  
 wrote:
> On Wed, 4 Nov 2020, H.J. Lu wrote:
>
>> On Wed, Nov 4, 2020 at 1:03 PM Hans-Peter Nilsson  
>> wrote:
>>> On Wed, 4 Nov 2020, H.J. Lu wrote:
 On Wed, Nov 4, 2020 at 10:09 AM Hans-Peter Nilsson  
 wrote:
> I'm not much more than a random voice, but an assembly directive
> that specifies the symbol (IIUC your .retain directive) to
 But .retain directive DOES NOT adjust symbol attribute.
> I see I missed to point out that I was speaking about the *gcc
> symbol* attribute "used".
 There is no such corresponding symbol attribute in ELF.
>>> I have not missed that, nor that SHF_GNU_RETAIN is so new that
>>> it's not in binutils master.  I have also not missed that gcc
>>> caters to other object formats too.  A common symbol-specific
>>> directive such as .retain, would be better than messing with
>>> section attributes, for gcc.
>> This is totally irrelevant to SHF_GNU_RETAIN.
>>
> It's cleaner to the compiler if it can pass on to the assembler
> the specific symbol that needs to be kept.
>
 SHF_GNU_RETAIN is for section and GCC should place the symbol,
 which should be kept, in the SHF_GNU_RETAIN section directly, not
 through .retain directive.
>>> This is where opinions differ.  Anyway, this is now repetition;
>>> I'm done.
>> .retain is ill-defined.   For example,
>>
>> [hjl@gnu-cfl-2 gcc]$ cat /tmp/x.c
>> static int xyzzy __attribute__((__used__));
>> [hjl@gnu-cfl-2 gcc]$ ./xgcc -B./ -S /tmp/x.c -fcommon
>> [hjl@gnu-cfl-2 gcc]$ cat x.s
>> .file "x.c"
>> .text
>> .retain xyzzy  < What does it do?
>> .local xyzzy
>> .comm xyzzy,4,4
>> .ident "GCC: (GNU) 11.0.0 20201103 (experimental)"
>> .section .note.GNU-stack,"",@progbits
>> [hjl@gnu-cfl-2 gcc]$
>>
>> A symbol directive should operate on the symbol table.
>> With 'R' flag, we got
>>
>> .file "x.c"
>> .text
>> .section .bss.xyzzy,"awR",@nobits
>> .align 4
>> .type xyzzy, @object
>> .size xyzzy, 4
>> xyzzy:
>> .zero 4
>> .ident "GCC: (GNU) 11.0.0 20201104 (experimental)"
>> .section .note.GNU-stack,"",@progbits
> I still think it is very wrong for the "used" attribute to place the
> symbol in a unique section. The structure of the sections in the object
> file should be no different whether the "used" attribute was applied to
> a symbol or not.

I tend to agree here.  Also note that someone could have a section
attribute in addition to the used attribute and that section attribute
might reference any arbitrary section.


> I will therefore have to make changes to GCC so that we can get the name
> of "unnamed" sections, and emit a .section directive with the "R" flag
> set on that section name, in order to avoid using a .retain directive.

ISTM that we could have the .retain  set the R flag in the bfd
section associated with 's definition.  That has other impacts
(namely that anything else in the same section is retained as well).


THe other alternative is to carry the attribute on the symbol into the
linker and teach the linker about the new symbol flag.


I don't have any fundamental objections to .retain.  I'm not sure why HJ
is so dead set against it.


jeff



Re: [RFC] Add support for the "retain" attribute utilizing SHF_GNU_RETAIN

2020-11-06 Thread Jeff Law via Gcc-patches


On 10/26/20 2:43 PM, Jozef Lawrynowicz wrote:
> On Mon, Oct 26, 2020 at 07:08:06PM +, Pedro Alves via Gcc-patches wrote:
>> On 10/6/20 12:10 PM, Jozef Lawrynowicz wrote:
>>
>>> Should "used" apply SHF_GNU_RETAIN?
>>> ===
>>> Another talking point is whether the existing "used" attribute should
>>> apply the SHF_GNU_RETAIN flag to the containing section.
>>>
>>> It seems unlikely that a user applies the "used" attribute to a
>>> declaration, and means for it to be saved from only compiler
>>> optimization, but *not* linker optimization. So perhaps it would be
>>> beneficial for "used" to apply SHF_GNU_RETAIN in some way.
>>>
>>> If "used" did apply SHF_GNU_RETAIN, we would also have to
>>> consider the above options for how to apply SHF_GNU_RETAIN to the
>>> section. Since the "used" attribute has been around for a while 
>>> it might not be appropriate for its behavior to be changed to place the
>>> associated declaration in its own, unique section, as in option (2).
>>>
>> To me, if I use attribute((used)), and the linker still garbage
>> collects the symbol, then the toolchain has a bug.  Is there any
>> use case that would suggest otherwise?
>>
>> Thanks,
>> Pedro Alves
>>
> I agree that "used" should imply SHF_GNU_RETAIN on whatever section
> contains the declaration that the attribute is applied to. However, I
> think that apart from the section flag being applied to the section, the
> behaviour of "used" shouldn't be modified i.e. the declaration shouldn't
> be put in a unique section.
>
> I originally justified the addition of a "retain" attribute, alongside
> "used" implying SHF_GNU_RETAIN, as indicating that the declaration
> should be placed in it's own section. In hindsight, this is unnecessary;
> if the user wants to increase the granularity of the portions of their
> program being retained, they should build with
> -f{function,data}-sections, or manually put the declaration in its own
> section with the "section" attribute.
>
> So we could shelve the "retain" attribute, and just modify the "used"
> attribute to imply SHF_GNU_RETAIN. If we get consensus on that, I could
> go ahead an implement it, but I never got any specific feedback on the
> GCC behavior from anyone apart from you. I don't know whether to
> interpret that lack of feedback, whilst the other aspects of the
> implementation were commented on, as support for the "retain" attribute.
>
> (I appreciate you giving that feedback in the Binutils discussions, and
> should have engaged in those discussions more at the time. There was
> just a lot of opinions flying about on many aspects of it, which is
> attention for this proposal I now miss...)
>
> Since I'm not proposing to modify the behavior of "used" apart from
> applying SHF_GNU_RETAIN to its section, I'm hoping the GCC side of
> things won't be too controversial.
>
> However, the assembler will have to support mis-matched section
> declarations, i.e.:
>   .section .text,"ax",%progbits
>   ...
>   .section .text,"axR",%progbits
>   ...
>
> The Binutils patch that supported this would create two separate .text
> sections in the assembled object file, one with SHF_GNU_RETAIN and one
> without.
> Perhaps they should be merged into a single .text section, with
> SHF_GNU_RETAIN applied to that merged section, so as to truly not
> interfere with "used" attribute behavior.
>
> There was an opinion that allowing these separate .section directives
> with the same name but different flags was undesirable.
>
> Personally, I don't see it as a problem, this exception is beneficial
> and makes sense, if the assembler merges the sections it is as if they
> all had the flag applied anyway.

So at a high level I agree with Pedro.  As a developer, if I marked
something as used and the linker removed it, I'd be awful annoyed.


What I think it still debatable is the implementation.  I think we
probably want a directive we pass to the assembler that says to keep the
symbol (ie, the .retain directive from the other thread).


The question then becomes how is .retain implemented.  The easy way is
to have it change the section flags under the hood to ensure the section
is never garbage collected away.  The other way would be to do deeper
surgery by making .retain mark the symbol and teach the linker about the
sematics of that new symbol flag.


Jeff




Re: [PATCH] rs6000: Fix default alignment ABI break caused by MMA base support

2020-11-06 Thread Peter Bergner via Gcc-patches
On 11/6/20 4:51 PM, Segher Boessenkool wrote:
> The ABI break is all GCC's faultt, but it is exposed by that glibc code,
> sure :-)

Right, and I'm thankful it was caught (fairly) early enough!


>> This passes bootstrap and regtesting on powerpc64le-linux with no 
>> regressions.
>> Ok for mainline and the GCC 10 release branch after some burn in?
> 
> Yes, okay for both.  Thanks!

Thanks, pushed to trunk.  I'll push to GCC 10 in a day or two.



>> +   Since the test below doesn't need any other MMA support, we can enable
>> +   this test case on basically any cpu that has hard floating point
>> +   registers.  */
> 
> Good comment, thanks :-)  That "basically" does not sound super
> convincing, but this is just a testcase, we can fix it later if need be.

The types and built-ins are enabled with TARGET_EXTRA_BUILTINS which
is a conglomeration of a lot of different target flags.  The hard float
test seemed like the test that would allow it to be run on the most
targets, but being disabled when it should be.  Yeah, we can fix it
if we trip over it somehow.

Peter



Re: [PATCH] rs6000: Fix default alignment ABI break caused by MMA base support

2020-11-06 Thread Segher Boessenkool
Hi!

On Fri, Nov 06, 2020 at 04:18:00PM -0600, Peter Bergner wrote:
> As part of the MMA base support, we incremented BIGGEST_ALIGNMENT in
> order to align the __vector_pair and __vector_quad types to 256 and 512
> bits respectively.  This had the unintended effect of changing the
> default alignment used by __attribute__ ((__aligned__)) which causes
> an ABI break because of some dodgy code in GLIBC's struct pthread
> (GLIBC is going to fix that too).  The fix in GCC is to revert the
> BIGGEST_ALIGNMENT change and to force the alignment on the type itself
> rather than the mode used by the type.

The ABI break is all GCC's faultt, but it is exposed by that glibc code,
sure :-)

> This passes bootstrap and regtesting on powerpc64le-linux with no regressions.
> Ok for mainline and the GCC 10 release branch after some burn in?

Yes, okay for both.  Thanks!

(See note below though.)

> gcc/
>   * config/rs6000/rs6000.h (BIGGEST_ALIGNMENT): Revert previous commit
>   so as not to break the ABI.
>   * config/rs6000/rs6000-call.c (rs6000_init_builtins): Set the ABI
>   mandated alignment for __vector_pair and __vector_quad types.
> 
> gcc/testsuite/
>   * gcc.target/powerpc/mma-alignment.c: New test.

> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/mma-alignment.c
> @@ -0,0 +1,41 @@
> +/* { dg-do run } */
> +/* { dg-require-effective-target hard_float } */
> +/* { dg-options "-O2 -mhard-float" } */
> +
> +#include 
> +
> +/* The MMA types below are enabled for pre-power10 compiles, because the
> +   built-ins that use them must always be initialized in case the user has
> +   a target attribute or pragma on a function that uses the MMA built-ins.
> +   Since the test below doesn't need any other MMA support, we can enable
> +   this test case on basically any cpu that has hard floating point
> +   registers.  */

Good comment, thanks :-)  That "basically" does not sound super
convincing, but this is just a testcase, we can fix it later if need be.


Segher


Re: [PATCH] mixing of labels and code in C2X

2020-11-06 Thread Joseph Myers
On Fri, 6 Nov 2020, Uecker, Martin wrote:

> Am Freitag, den 06.11.2020, 22:07 + schrieb Joseph Myers:
> > On Fri, 6 Nov 2020, Uecker, Martin wrote:
> > 
> > > Hi Joseph,
> > > 
> > > here is the revised patch. I remove the 'fallthrough'
> > > code as suggested, so everything becomes even simpler.
> > > Some tests had to be changed then, but it seems Ok to
> > > me.
> > 
> > This patch is missing the new tests.
> > 
> > > + * gcc.dg/c11-labels-1.c: New test.
> > > + * gcc.dg/c11-labels-2.c: New test.
> > > + * gcc.dg/c11-labels-3.c: New test.
> > > + * gcc.dg/c2x-labels-1.c: New test.
> > > + * gcc.dg/c2x-labels-2.c: New test.
> > > + * gcc.dg/c2x-labels-3.c: New test.
> 
> My bad. Here there are.

Thanks.  This version is OK (note that ChangeLog entries now need to be 
included in the commit message, not as changes to the ChangeLog files, and 
the nightly cron job will update the ChangeLog files).

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


Re: [PATCH] Fix stack pointer handling in ms_hook_prologue functions for i386 target.

2020-11-06 Thread Jeff Law via Gcc-patches


On 2/10/20 9:22 AM, Paul Gofman wrote:
> ChangeLog:
> PR target/91489
> * config/i386/i386.md (simple_return): Also check
> for ms_hook_prologue function attribute.
> * config/i386/i386.c (ix86_can_use_return_insn_p):
> Also check for ms_hook_prologue function attribute.
>
> testsuite/ChangeLog:
> PR target/91489
> * gcc.target/i386/ms_hook_prologue.c: Expand testcase
> to reproduce PR target/91489 issue.

Thanks.  I'm sorry it took so long, but this has been committed to the
trunk.


jeff




Re: [PATCH] mixing of labels and code in C2X

2020-11-06 Thread Uecker, Martin
Am Freitag, den 06.11.2020, 22:07 + schrieb Joseph Myers:
> On Fri, 6 Nov 2020, Uecker, Martin wrote:
> 
> > Hi Joseph,
> > 
> > here is the revised patch. I remove the 'fallthrough'
> > code as suggested, so everything becomes even simpler.
> > Some tests had to be changed then, but it seems Ok to
> > me.
> 
> This patch is missing the new tests.
> 
> > +   * gcc.dg/c11-labels-1.c: New test.
> > +   * gcc.dg/c11-labels-2.c: New test.
> > +   * gcc.dg/c11-labels-3.c: New test.
> > +   * gcc.dg/c2x-labels-1.c: New test.
> > +   * gcc.dg/c2x-labels-2.c: New test.
> > +   * gcc.dg/c2x-labels-3.c: New test.
> 
> 

My bad. Here there are.

diff --git a/gcc/c/ChangeLog b/gcc/c/ChangeLog
index ca844ca775a..c656b5f4cc5 100644
--- a/gcc/c/ChangeLog
+++ b/gcc/c/ChangeLog
@@ -1,3 +1,8 @@
+2020-11-06  Martin Uecker  
+
+   * c-parser.c (c_parser_label): Implement mixing of labels and code.
+   (c_parser_all_labels): Likewise.
+
 2020-10-29  Asher Gordon  
 
* c-typeck.c (free_all_tagged_tu_seen_up_to): Replace free
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index fc97aa3f95f..53179f10985 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -1521,7 +1521,7 @@ static void c_parser_initval (c_parser *, struct c_expr *,
  struct obstack *);
 static tree c_parser_compound_statement (c_parser *, location_t * = NULL);
 static location_t c_parser_compound_statement_nostart (c_parser *);
-static void c_parser_label (c_parser *);
+static void c_parser_label (c_parser *, tree);
 static void c_parser_statement (c_parser *, bool *, location_t * = NULL);
 static void c_parser_statement_after_labels (c_parser *, bool *,
 vec * = NULL);
@@ -5523,7 +5523,7 @@ c_parser_initval (c_parser *parser, struct c_expr *after,
 }
 
 /* Parse a compound statement (possibly a function body) (C90 6.6.2,
-   C99 6.8.2, C11 6.8.2).
+   C99 6.8.2, C11 6.8.2, C2X 6.8.2).
 
compound-statement:
  { block-item-list[opt] }
@@ -5534,6 +5534,7 @@ c_parser_initval (c_parser *parser, struct c_expr *after,
  block-item-list block-item
 
block-item:
+ label
  nested-declaration
  statement
 
@@ -5674,7 +5675,7 @@ c_parser_compound_statement_nostart (c_parser *parser)
 {
   location_t loc = c_parser_peek_token (parser)->location;
   loc = expansion_point_location_if_in_system_header (loc);
-  /* Standard attributes may start a statement or a declaration.  */
+  /* Standard attributes may start a label, statement or declaration.  */
   bool have_std_attrs
= c_parser_nth_token_starts_std_attributes (parser, 1);
   tree std_attrs = NULL_TREE;
@@ -5685,7 +5686,6 @@ c_parser_compound_statement_nostart (c_parser *parser)
  || (c_parser_next_token_is (parser, CPP_NAME)
  && c_parser_peek_2nd_token (parser)->type == CPP_COLON))
{
- c_warn_unused_attributes (std_attrs);
  if (c_parser_next_token_is_keyword (parser, RID_CASE))
label_loc = c_parser_peek_2nd_token (parser)->location;
  else
@@ -5693,27 +5693,31 @@ c_parser_compound_statement_nostart (c_parser *parser)
  last_label = true;
  last_stmt = false;
  mark_valid_location_for_stdc_pragma (false);
- c_parser_label (parser);
+ c_parser_label (parser, std_attrs);
}
-  else if (!last_label
-  && (c_parser_next_tokens_start_declaration (parser)
-  || (have_std_attrs
-  && c_parser_next_token_is (parser, CPP_SEMICOLON
+  else if (c_parser_next_tokens_start_declaration (parser)
+  || (have_std_attrs
+  && c_parser_next_token_is (parser, CPP_SEMICOLON)))
{
- last_label = false;
+ if (last_label)
+   pedwarn_c11 (c_parser_peek_token (parser)->location, OPT_Wpedantic,
+"a label can only be part of a statement and "
+"a declaration is not a statement");
+
  mark_valid_location_for_stdc_pragma (false);
  bool fallthru_attr_p = false;
  c_parser_declaration_or_fndef (parser, true, !have_std_attrs,
 true, true, true, NULL,
 vNULL, have_std_attrs, std_attrs,
 NULL, _attr_p);
+
  if (last_stmt && !fallthru_attr_p)
pedwarn_c90 (loc, OPT_Wdeclaration_after_statement,
 "ISO C90 forbids mixed declarations and code");
  last_stmt = fallthru_attr_p;
+ last_label = false;
}
-  else if (!last_label
-  && c_parser_next_token_is_keyword (parser, RID_EXTENSION))
+  else if (c_parser_next_token_is_keyword (parser, RID_EXTENSION))
{
  /* __extension__ can start a declaration, but is also an
 unary operator that can start an expression.  Consume all
@@ 

[PATCH] rs6000: Fix default alignment ABI break caused by MMA base support

2020-11-06 Thread Peter Bergner via Gcc-patches
As part of the MMA base support, we incremented BIGGEST_ALIGNMENT in
order to align the __vector_pair and __vector_quad types to 256 and 512
bits respectively.  This had the unintended effect of changing the
default alignment used by __attribute__ ((__aligned__)) which causes
an ABI break because of some dodgy code in GLIBC's struct pthread
(GLIBC is going to fix that too).  The fix in GCC is to revert the
BIGGEST_ALIGNMENT change and to force the alignment on the type itself
rather than the mode used by the type.

This passes bootstrap and regtesting on powerpc64le-linux with no regressions.
Ok for mainline and the GCC 10 release branch after some burn in?

Peter

gcc/
* config/rs6000/rs6000.h (BIGGEST_ALIGNMENT): Revert previous commit
so as not to break the ABI.
* config/rs6000/rs6000-call.c (rs6000_init_builtins): Set the ABI
mandated alignment for __vector_pair and __vector_quad types.

gcc/testsuite/
* gcc.target/powerpc/mma-alignment.c: New test.


diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h
index bbd8060e143..5a47aa14722 100644
--- a/gcc/config/rs6000/rs6000.h
+++ b/gcc/config/rs6000/rs6000.h
@@ -776,8 +776,10 @@ extern unsigned rs6000_pointer_size;
 /* Allocation boundary (in *bits*) for the code of a function.  */
 #define FUNCTION_BOUNDARY 32
 
-/* No data type wants to be aligned rounder than this.  */
-#define BIGGEST_ALIGNMENT (TARGET_MMA ? 512 : 128)
+/* No data type is required to be aligned rounder than this.  Warning, if
+   BIGGEST_ALIGNMENT is changed, then this may be an ABI break.  An example
+   of where this can break an ABI is in GLIBC's struct _Unwind_Exception.  */
+#define BIGGEST_ALIGNMENT 128
 
 /* Alignment of field after `int : 0' in a structure.  */
 #define EMPTY_FIELD_BOUNDARY 32
diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c
index 92378e958a9..3bd89a79bad 100644
--- a/gcc/config/rs6000/rs6000-call.c
+++ b/gcc/config/rs6000/rs6000-call.c
@@ -13191,12 +13191,14 @@ rs6000_init_builtins (void)
   if (TARGET_EXTRA_BUILTINS)
 {
   vector_pair_type_node = make_unsigned_type (256);
+  SET_TYPE_ALIGN (vector_pair_type_node, 256);
   SET_TYPE_MODE (vector_pair_type_node, POImode);
   layout_type (vector_pair_type_node);
   lang_hooks.types.register_builtin_type (vector_pair_type_node,
  "__vector_pair");
 
   vector_quad_type_node = make_unsigned_type (512);
+  SET_TYPE_ALIGN (vector_quad_type_node, 512);
   SET_TYPE_MODE (vector_quad_type_node, PXImode);
   layout_type (vector_quad_type_node);
   lang_hooks.types.register_builtin_type (vector_quad_type_node,
diff --git a/gcc/testsuite/gcc.target/powerpc/mma-alignment.c 
b/gcc/testsuite/gcc.target/powerpc/mma-alignment.c
new file mode 100644
index 000..09818931b48
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/mma-alignment.c
@@ -0,0 +1,41 @@
+/* { dg-do run } */
+/* { dg-require-effective-target hard_float } */
+/* { dg-options "-O2 -mhard-float" } */
+
+#include 
+
+/* The MMA types below are enabled for pre-power10 compiles, because the
+   built-ins that use them must always be initialized in case the user has
+   a target attribute or pragma on a function that uses the MMA built-ins.
+   Since the test below doesn't need any other MMA support, we can enable
+   this test case on basically any cpu that has hard floating point
+   registers.  */
+
+struct
+{
+  int __attribute__ ((__aligned__)) ivar;
+  __vector_pair pair;
+  __vector_quad quad;
+} s;
+
+int
+main (void)
+{
+  /* Verify default alignment is 16-byte aligned (BIGGEST_ALIGNMENT).
+ This may change in the future, but that is an ABI break, so this
+ hardcoded test case is here to be a noisy FAIL as a warning, in
+ case the ABI change was unintended and unwanted.  An example of where
+ this can break an ABI is in glibc's struct _Unwind_Exception.  */
+  if (__alignof__ (s.ivar) != 16)
+abort ();
+
+  /* Verify __vector_pair types are 32-byte aligned.  */
+  if (__alignof__ (s.pair) != 32)
+abort ();
+
+  /* Verify __vector_quad types are 64-byte aligned.  */
+  if (__alignof__ (s.quad) != 64)
+abort ();
+
+  return 0;
+}


[PATCH] rs6000: Fix TARGET_POWERPC64 vs. TARGET_64BIT confusion

2020-11-06 Thread Segher Boessenkool
I gave Ke Wen bad advice, luckily David corrected me: it is true that we
cannot use TARGET_POWERPC64 on many 32-bit OSes, since either the kernel
or userland does not save the top half of the 64-bit integer registers,
but we do not have to care about that in separate patterns or related
code.  The flag is automatically not enabled by default on targets that
do not handle this correctly.

This patch fixes it.


Segher


2020-11-06  Segher Boessenkool  

PR target/96933
* config/rs6000/rs6000.c (rs6000_expand_vector_init): Use
TARGET_POWERPC64 instead of TARGET_64BIT.

---
 gcc/config/rs6000/rs6000.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index e613353..63f1c06 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -6799,7 +6799,7 @@ rs6000_expand_vector_init (rtx target, rtx vals)
   for (i = 0; i < n_elts; i++)
{
  rtx tmp = force_reg (GET_MODE_INNER (mode), XVECEXP (vals, 0, i));
- if (TARGET_64BIT)
+ if (TARGET_POWERPC64)
{
  op[i] = gen_reg_rtx (DImode);
  emit_insn (gen_zero_extendqidi2 (op[i], tmp));
@@ -6909,7 +6909,7 @@ rs6000_expand_vector_init (rtx target, rtx vals)
  for (i = 0; i < n_elts; i++)
{
  vr_qi[i] = gen_reg_rtx (V16QImode);
- if (TARGET_64BIT)
+ if (TARGET_POWERPC64)
emit_insn (gen_p8_mtvsrd_v16qidi2 (vr_qi[i], op[i]));
  else
emit_insn (gen_p8_mtvsrwz_v16qisi2 (vr_qi[i], op[i]));
-- 
1.8.3.1



Re: [PATCH] mixing of labels and code in C2X

2020-11-06 Thread Joseph Myers
On Fri, 6 Nov 2020, Uecker, Martin wrote:

> Hi Joseph,
> 
> here is the revised patch. I remove the 'fallthrough'
> code as suggested, so everything becomes even simpler.
> Some tests had to be changed then, but it seems Ok to
> me.

This patch is missing the new tests.

> + * gcc.dg/c11-labels-1.c: New test.
> + * gcc.dg/c11-labels-2.c: New test.
> + * gcc.dg/c11-labels-3.c: New test.
> + * gcc.dg/c2x-labels-1.c: New test.
> + * gcc.dg/c2x-labels-2.c: New test.
> + * gcc.dg/c2x-labels-3.c: New test.

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


Re: [PATCH] Objective-C/C++ (C-family) : Add missing 'atomic' property attribute.

2020-11-06 Thread Joseph Myers
On Fri, 6 Nov 2020, Iain Sandoe wrote:

> Hi
> 
> Arguably, this is actually a bug fix since the ‘atomic’ attribute is
> paired with the ‘nonatomic’ one.  However it is the default and was
> omitted when the @property implementation was added.
> 
> ‘atomic’ in Objective-C terms is not specified in relation to _Atomic
> or std::atomic (the _Atomic keyword is not accepted in that context).
> 
> tested across several Darwin versions and on x86_64-linux-gnu
> 
> OK for the C-family parts?

OK.

(_Atomic isn't accepted for Objective-C at all at present; see the comment 
on the relevant sorry in c-parser.c.)

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


Re: [PATCH] Objective-C : Implement NSObject attribute.

2020-11-06 Thread Joseph Myers
On Fri, 6 Nov 2020, Iain Sandoe wrote:

> Hi
> 
> Originally, I had this as a Darwin-only patch, however GNUStep
> also makes use of NSObject and similar constructs, so this really
> needs to be available to linux-gnu as well.
> 
> tested across several Darwin versions and on x86_64-linux-gnu.
> 
> OK for the c-family changes?

OK.

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


Re: [PATCH] generalized range_query class for multiple contexts

2020-11-06 Thread Andrew MacLeod via Gcc-patches

On 11/6/20 11:13 AM, Martin Sebor wrote:

On 11/5/20 8:08 PM, Andrew MacLeod wrote:

On 11/5/20 7:50 PM, Martin Sebor wrote:

On 11/5/20 5:02 PM, Andrew MacLeod wrote:

On 11/5/20 4:02 PM, Martin Sebor wrote:

On 11/5/20 12:29 PM, Martin Sebor wrote:

On 10/1/20 11:25 AM, Martin Sebor wrote:

On 10/1/20 9:34 AM, Aldy Hernandez wrote:



On 10/1/20 3:22 PM, Andrew MacLeod wrote:
 > On 10/1/20 5:05 AM, Aldy Hernandez via Gcc-patches wrote:
 >>> Thanks for doing all this!  There isn't anything I don't 
understand
 >>> in the sprintf changes so no questions from me (well, 
almost none).

 >>> Just some comments:
 >> Thanks for your comments on the sprintf/strlen API conversion.
 >>
 >>> The current call statement is available in all functions 
that take
 >>> a directive argument, as dir->info.callstmt.  There should 
be no need
 >>> to also add it as a new argument to the functions that now 
need it.

 >> Fixed.
 >>
 >>> The change adds code along these lines in a bunch of places:
 >>>
 >>> + value_range vr;
 >>> + if (!query->range_of_expr (vr, arg, stmt))
 >>> +   vr.set_varying (TREE_TYPE (arg));
 >>>
 >>> I thought under the new Ranger APIs when a range couldn't be
 >>> determined it would be automatically set to the maximum for
 >>> the type.  I like that and have been moving in that direction
 >>> with my code myself (rather than having an API fail, have it
 >>> set the max range and succeed).
 >> I went through all the above idioms and noticed all are 
being used on
 >> supported types (integers or pointers). So range_of_expr 
will always

 >> return true.  I've removed the if() and the set_varying.
 >>
 >>> Since that isn't so in this case, I think it would still 
be nice
 >>> if the added code could be written as if the range were 
set to
 >>> varying in this case and (ideally) reduced to just 
initialization:

 >>>
 >>> value_range vr = some-function (query, stmt, arg);
 >>>
 >>> some-function could be an inline helper defined just for 
the sprintf
 >>> pass (and maybe also strlen which also seems to use the 
same pattern),
 >>> or it could be a value_range AKA irange ctor, or it could 
be a member

 >>> of range_query, whatever is the most appropriate.
 >>>
 >>> (If assigning/copying a value_range is thought to be too 
expensive,
 >>> declaring it first and then passing it to that helper to 
set it

 >>> would work too).
 >>>
 >>> In strlen, is the removed comment no longer relevant? 
(I.e., does

 >>> the ranger solve the problem?)
 >>>
 >>> -  /* The range below may be "inaccurate" if a 
constant has been
 >>> -    substituted earlier for VAL by this pass that 
hasn't been
 >>> -    propagated through the CFG. This shoud be fixed 
by the new
 >>> -    on-demand VRP if/when it becomes available 
(hopefully in

 >>> -    GCC 11).  */
 >> It should.
 >>
 >>> I'm wondering about the comment added to 
get_range_strlen_dynamic

 >>> and other places:
 >>>
 >>> + // FIXME: Use range_query instead of global ranges.
 >>>
 >>> Is that something you're planning to do in a followup or 
should

 >>> I remember to do it at some point?
 >> I'm not planning on doing it.  It's just a reminder that it 
would be

 >> beneficial to do so.
 >>
 >>> Otherwise I have no concern with the changes.
 >> It's not cleared whether Andrew approved all 3 parts of the 
patchset
 >> or just the valuation part.  I'll wait for his nod before 
committing

 >> this chunk.
 >>
 >> Aldy
 >>
 > I have no issue with it, so OK.

Pushed all 3 patches.

 >
 > Just an observation that should be pointed out, I believe 
Aldy has all
 > the code for converting to a ranger, but we have not pursued 
that any
 > further yet since there is a regression due to our lack of 
equivalence
 > processing I think?  That should be resolved in the coming 
month, but at
 > the moment is a holdback/concern for converting these 
passes... iirc.


Yes.  Martin, the take away here is that the strlen/sprintf 
pass has been converted to the new API, but ranger is still not 
up and running on it (even on the branch).


With the new API, all you have to do is remove all instances of 
evrp_range_analyzer and replace them with a ranger. That's it.
Below is an untested patch that would convert you to a ranger 
once it's contributed.


IIRC when I enabled the ranger for your pass a while back, 
there was one or two regressions due to missing equivalences, 
and the rest were because the tests were expecting an actual 
specific range, and the ranger returned a slightly 
different/better one. You'll need to adjust your tests.


Ack.  I'll be on the lookout for the ranger commit (if you hppen
to remember and CC me on it just in case I might miss it that would
be great).


I have applied the patch and ran some tests.  There are quite
a few failures (see the list below).  I have only looked at
a couple.  The one in in gcc.dg/tree-ssa/builtin-sprintf-warn-3.c
boils down to the following test case.  There should be no 

Re: ping x2 [PATCH 0/5] MSP430: Implement macros to describe relative costs of operations

2020-11-06 Thread Jeff Law via Gcc-patches


On 11/6/20 2:23 PM, Jozef Lawrynowicz wrote:
> On Fri, Nov 06, 2020 at 01:53:19PM -0700, Jeff Law via Gcc-patches wrote:
>> On 9/15/20 2:30 PM, Jozef Lawrynowicz wrote:
>>> Ping x2 for below.
>>>
>>> On Fri, Aug 07, 2020 at 12:02:59PM +0100, Jozef Lawrynowicz wrote:
 Pinging for this series of patches.
 Attached all patches to this mail with the ammended patch 4 thanks to
 Segher's review.

 Thanks,
 Jozef

 On Thu, Jul 23, 2020 at 04:43:56PM +0100, Jozef Lawrynowicz wrote:
> The following series of patches for MSP430 implement some of the target
> macros used to determine the relative costs of operations.
>
> To give an indication of the overall effect of these changes on
> codesize, below are some size statistics collected from all the
> executable files from execute.exp that are built at -Os.
> There are around 1470 such tests (depending on the configuration).
>
> The percentage change (((new - old)/old) * 100) in text size is calculated
> for each test and the given metric is applied to that overall set of data.
>
> Configuration | Mean (%) | Median (%) | Delta < 0 (count) | Delta > 0 
> (count)
> -
> -mcpu=msp430  |  -2.4|   -2.7 |  1454 |  17
> -mcpu=msp430x |  -2.3|   -2.4 |  1460 |  10
> -mlarge   |  -1.7|   -1.9 |  1412 |  37
>
> Successfully regtested on trunk for msp430-elf, ok to apply?
>
> Jozef Lawrynowicz (5):
>   MSP430: Implement TARGET_MEMORY_MOVE_COST
>   MSP430: Implement TARGET_RTX_COSTS
>   MSP430: Add defaulting to the insn length attribute
>   MSP430: Implement TARGET_INSN_COST
>   MSP430: Skip index-1.c test
>
>  gcc/config/msp430/msp430-protos.h |   5 +-
>  gcc/config/msp430/msp430.c| 867 --
>  gcc/config/msp430/msp430.h|  13 +
>  gcc/config/msp430/msp430.md   | 439 +++--
>  gcc/config/msp430/msp430.opt  |   4 +
>  gcc/config/msp430/predicates.md   |  13 +
>  gcc/testsuite/gcc.c-torture/execute/index-1.c |   2 +
>  7 files changed, 1206 insertions(+), 137 deletions(-)
>> [ ... ]
>>
>> So it's a series of 5 patches.  They LGTM.    And if  there's 
>> minor
>> updates needed to address issues found once they're on the trunk, the
>> consider those updates pre-approved.
> Spooky, I pinged these patches the minute before you replied to this one.

Yea, I saw your ping when I went to my inbox after reviewing the change ;-)


>
>> Note that defining LOGICAL_OP_NON_SHORT_CIRCUIT and BRANCH_COST impact
>> gimple code generation.  I'm a bit surprised there wasn't more fallout
>> in the existing tests.
> IIRC, when I tried messing with LOGICAL_OP_NON_SHORT_CIRCUIT and
> BRANCH_COST before the full costs were implemented, they never had any
> effect. I'm going to hand wave here and say that whatever behavior these
> macros were affecting before the costs were implemented, was actually
> behaving as it should. Then with the full costs implemented, I had to
> tweak them to get back to that optimal state.
>
> Also, the only conditional instructions available for MSP430 are
> cbranch, i.e. there is never a choice between a cbranch and a cstore.
> I was surprised BRANCH_COST had any effect, but after looking at output
> assembly, it appeared that it did affect the number of cbranch insns
> emitted, vs just other insns to move data about.

It tends to twiddle some of the jump threading tests which are sensitive
to the jumping structure (surprise).  We try to handle both cases, but
it's proven pretty fragile thorugh the years.


>
> The changes did fix these optimization tests:
> gcc.dg/tree-ssa/reassoc-33.c scan-tree-dump-times reassoc1 "Optimizing range 
> tests" 3
> gcc.dg/tree-ssa/reassoc-34.c scan-tree-dump-times reassoc1 "Optimizing range 
> tests" 1
> gcc.dg/tree-ssa/reassoc-35.c scan-tree-dump-times reassoc1 "Optimizing range 
> tests" 1
Yea, those would be well within the realm of expected for changing those
macros since they'd change the decisons  for expanding a range test
either as a series of conditionals or a series of logical ops and a
single conditional.  THat in turn heavily influences optimize_range_test
in tree-ssa-reassoc
> gcc.dg/tree-ssa/reassoc-36.c scan-tree-dump-times reassoc1 "Optimizing range 
> tests" 1
>
> Thanks for the review, I'll watch for fallout after installing on trunk.

Likewise.  I think msp430 was running in my tester while I was reviewing
the changes, so tomorrow's run should be interesting to review.


jeff



Re: ping x2 [PATCH 0/5] MSP430: Implement macros to describe relative costs of operations

2020-11-06 Thread Jozef Lawrynowicz
On Fri, Nov 06, 2020 at 01:53:19PM -0700, Jeff Law via Gcc-patches wrote:
> 
> On 9/15/20 2:30 PM, Jozef Lawrynowicz wrote:
> > Ping x2 for below.
> >
> > On Fri, Aug 07, 2020 at 12:02:59PM +0100, Jozef Lawrynowicz wrote:
> >> Pinging for this series of patches.
> >> Attached all patches to this mail with the ammended patch 4 thanks to
> >> Segher's review.
> >>
> >> Thanks,
> >> Jozef
> >>
> >> On Thu, Jul 23, 2020 at 04:43:56PM +0100, Jozef Lawrynowicz wrote:
> >>> The following series of patches for MSP430 implement some of the target
> >>> macros used to determine the relative costs of operations.
> >>>
> >>> To give an indication of the overall effect of these changes on
> >>> codesize, below are some size statistics collected from all the
> >>> executable files from execute.exp that are built at -Os.
> >>> There are around 1470 such tests (depending on the configuration).
> >>>
> >>> The percentage change (((new - old)/old) * 100) in text size is calculated
> >>> for each test and the given metric is applied to that overall set of data.
> >>>
> >>> Configuration | Mean (%) | Median (%) | Delta < 0 (count) | Delta > 0 
> >>> (count)
> >>> -
> >>> -mcpu=msp430  |  -2.4|   -2.7 |  1454 |  17
> >>> -mcpu=msp430x |  -2.3|   -2.4 |  1460 |  10
> >>> -mlarge   |  -1.7|   -1.9 |  1412 |  37
> >>>
> >>> Successfully regtested on trunk for msp430-elf, ok to apply?
> >>>
> >>> Jozef Lawrynowicz (5):
> >>>   MSP430: Implement TARGET_MEMORY_MOVE_COST
> >>>   MSP430: Implement TARGET_RTX_COSTS
> >>>   MSP430: Add defaulting to the insn length attribute
> >>>   MSP430: Implement TARGET_INSN_COST
> >>>   MSP430: Skip index-1.c test
> >>>
> >>>  gcc/config/msp430/msp430-protos.h |   5 +-
> >>>  gcc/config/msp430/msp430.c| 867 --
> >>>  gcc/config/msp430/msp430.h|  13 +
> >>>  gcc/config/msp430/msp430.md   | 439 +++--
> >>>  gcc/config/msp430/msp430.opt  |   4 +
> >>>  gcc/config/msp430/predicates.md   |  13 +
> >>>  gcc/testsuite/gcc.c-torture/execute/index-1.c |   2 +
> >>>  7 files changed, 1206 insertions(+), 137 deletions(-)
> 
> [ ... ]
> 
> So it's a series of 5 patches.  They LGTM.    And if  there's minor
> updates needed to address issues found once they're on the trunk, the
> consider those updates pre-approved.

Spooky, I pinged these patches the minute before you replied to this one.

> 
> Note that defining LOGICAL_OP_NON_SHORT_CIRCUIT and BRANCH_COST impact
> gimple code generation.  I'm a bit surprised there wasn't more fallout
> in the existing tests.

IIRC, when I tried messing with LOGICAL_OP_NON_SHORT_CIRCUIT and
BRANCH_COST before the full costs were implemented, they never had any
effect. I'm going to hand wave here and say that whatever behavior these
macros were affecting before the costs were implemented, was actually
behaving as it should. Then with the full costs implemented, I had to
tweak them to get back to that optimal state.

Also, the only conditional instructions available for MSP430 are
cbranch, i.e. there is never a choice between a cbranch and a cstore.
I was surprised BRANCH_COST had any effect, but after looking at output
assembly, it appeared that it did affect the number of cbranch insns
emitted, vs just other insns to move data about.

The changes did fix these optimization tests:
gcc.dg/tree-ssa/reassoc-33.c scan-tree-dump-times reassoc1 "Optimizing range 
tests" 3
gcc.dg/tree-ssa/reassoc-34.c scan-tree-dump-times reassoc1 "Optimizing range 
tests" 1
gcc.dg/tree-ssa/reassoc-35.c scan-tree-dump-times reassoc1 "Optimizing range 
tests" 1
gcc.dg/tree-ssa/reassoc-36.c scan-tree-dump-times reassoc1 "Optimizing range 
tests" 1

Thanks for the review, I'll watch for fallout after installing on trunk.
Jozef
> 
> jeff
> 
> 


Re: [PATCH GCC11]Improve uninitialized warning with value range info

2020-11-06 Thread Jeff Law via Gcc-patches


On 1/14/20 1:23 AM, Richard Biener wrote:
> On Mon, Jan 13, 2020 at 3:15 AM bin.cheng  wrote:
>> --
>> Sender:Richard Biener 
>> Sent At:2020 Jan. 9 (Thu.) 20:01
>> Recipient:Bin.Cheng 
>> Cc:bin.cheng ; GCC Patches 
>> 
>> Subject:Re: [PATCH GCC11]Improve uninitialized warning with value range info
>>
>>
>> On Thu, Jan 9, 2020 at 11:17 AM Bin.Cheng  wrote:
 I am not quite follow here. Do you mean we collect three cases "i <
 j", "i < min(j)", "max(i) < j" then
 call prune_uninit_phi_opnds for all three conditions?
>>> No, I've meant to somehow arrange that the 'preds' passed to
>>> use_pred_not_overlap_with_undef_path_pred contain all three predicates
>>> rather than just i < j, thus "expand" fully symbolic predicates.
>> Seems this would require non-trivial refactoring of the original code.
>>
 This is another question? because now we simply break out of for loop
 for finding such condition:

 -  if ((gimple_code (flag_def) == GIMPLE_PHI)
 - && (gimple_bb (flag_def) == gimple_bb (phi))
 - && find_matching_predicate_in_rest_chains (the_pred, preds,
 -num_preds))
 -   break;

 It's always possible that this flag_def can't prune use predicates
 against undefined path predicates, while a later one can prune but is
 skipped?
>>> I don't follow but I also don't want to understand the code too much ;)
>>>
>>> I'm fine with the idea and if the patch cannot introudce extra bogus 
>>> warnings
>>> let's go with it.  Can you amed the comment before the two 
>>> find_var_cmp_const
>>> calls?  I wonder whether eliding the second sweep when the first didn't find
>>> any predicate it skipped is worth the trouble.
>> Thanks for the comments, I updated the patch as attached.
> OK.

I've (finally) re-tested and installed this patch on the trunk.


Jeff




Re: [04/32] cpp lexer

2020-11-06 Thread Nathan Sidwell

On 11/6/20 3:23 PM, Jeff Law wrote:



04-cpp-lexer.diff

diff --git c/libcpp/include/cpplib.h w/libcpp/include/cpplib.h
index 8e398863cf6..81be6457951 100644
--- c/libcpp/include/cpplib.h
+++ w/libcpp/include/cpplib.h

@@ -888,9 +915,9 @@ struct GTY(()) cpp_hashnode {
unsigned int directive_index : 7;   /* If is_directive,
   then index into directive table.
   Otherwise, a NODE_OPERATOR.  */
-  unsigned char rid_code;  /* Rid code - for front ends.  */
+  unsigned int rid_code : 8;   /* Rid code - for front ends.  */
+  unsigned int flags : 9;  /* CPP flags.  */
ENUM_BITFIELD(node_type) type : 2;  /* CPP node type.  */
-  unsigned int flags : 8;  /* CPP flags.  */
  
/* 6 bits spare (plus another 32 on 64-bit hosts).  */


Someone already mentioned it, but the # of spare bits needs updating.


yeah, andit was me manually editing a patch wot flubbed it.


+  /* Enter directives mode for the peeking.  */
+  pfile->state.in_deferred_pragma = true;
+  pfile->state.pragma_allow_expansion = true;
+  pfile->state.save_comments = 0;
+  pfile->directive_line = result->src_loc;
It looks like you slam in known values when you drop out of directive 
mode rather than restoring the original values.  That may be OK, I'm not 
all that familiar with this code to know if that's corerct or not.


Looks like comments would be useful, and maybe asserts.  You only get to 
call the peeking when you were in a known (non-directives) state.  I 
wanted the peeking to be as cheap as possible, so saving and restoring 
did not seem needed.




+
+  if (__builtin_expect (node == n_modules[spec_nodes::M__IMPORT][0], false))
+/* __import  */
+header_count = backup + 2 + 16;
+  else if (__builtin_expect (node == n_modules[spec_nodes::M_IMPORT][0], 
false))
+/* import  */
+header_count = backup + 2 + (CPP_OPTION (pfile, preprocessed) ? 16 : 0);
+  else if (__builtin_expect (node == n_modules[spec_nodes::M_MODULE][0], 
false))
+; /* module  */
+  else
+goto not_module;


Are the __builtin_expects really useful here?  If not I'd prefer to 
avoid them and just let the predictors do their job. Similarly  
elsewhere in this patch.


I'll investigate.


+not_module:
+  /* Drop out of directive mode.  */
+  pfile->state.save_comments
+   = !CPP_OPTION (pfile, discard_comments);
+  pfile->state.in_deferred_pragma = false;
+  pfile->state.angled_headers = false;


This doesn't seem to match the code to go into directives mode all that 
well.  You don't reset pragma_allow_expansion or directive_line.


Hm, yeah.  I think neither of those fields have any bearing in 
non-directives mode, and always get set when entering it?  Again, 
comment at the very least.


nathan

--
Nathan Sidwell


Re: Patch for 96948

2020-11-06 Thread Martin Storsjö

On Fri, 6 Nov 2020, Jeff Law wrote:



On 9/8/20 9:34 AM, Martin Storsjö wrote:

Hi,

On Tue, 8 Sep 2020, Kirill Müller wrote:


Thanks for the heads up. The coincidence is funny -- a file that
hasn't been touched for years.


I think we both may originally be triggered from the same guy asking
around in different places about implementations of _Unwind_Backtrace
for windows, actually.


I do believe that we need the logic around the `first` flag for
consistency with the other unwind-*.c implementations.


Yes, if you store ms_context.Rip/Rsp before the RtlVirtualUnwind step
- but my patch stores them afterwards; after RtlVirtualUnwind, before
calling the callback.

The result should be the same, except if using the first flag
approach, I believe you're missing the last frame that is printed if
using my patch.


Presumably with your patch installed, the patch from Kirill is
unnecessary, right?


Indeed, as far as I know, this issue should be fixed now (but I'd 
appreciate if Kirill can retest things as well). Thanks for your time!


// Martin


Re: ping x2 [PATCH 0/5] MSP430: Implement macros to describe relative costs of operations

2020-11-06 Thread Jeff Law via Gcc-patches


On 9/15/20 2:30 PM, Jozef Lawrynowicz wrote:
> Ping x2 for below.
>
> On Fri, Aug 07, 2020 at 12:02:59PM +0100, Jozef Lawrynowicz wrote:
>> Pinging for this series of patches.
>> Attached all patches to this mail with the ammended patch 4 thanks to
>> Segher's review.
>>
>> Thanks,
>> Jozef
>>
>> On Thu, Jul 23, 2020 at 04:43:56PM +0100, Jozef Lawrynowicz wrote:
>>> The following series of patches for MSP430 implement some of the target
>>> macros used to determine the relative costs of operations.
>>>
>>> To give an indication of the overall effect of these changes on
>>> codesize, below are some size statistics collected from all the
>>> executable files from execute.exp that are built at -Os.
>>> There are around 1470 such tests (depending on the configuration).
>>>
>>> The percentage change (((new - old)/old) * 100) in text size is calculated
>>> for each test and the given metric is applied to that overall set of data.
>>>
>>> Configuration | Mean (%) | Median (%) | Delta < 0 (count) | Delta > 0 
>>> (count)
>>> -
>>> -mcpu=msp430  |  -2.4|   -2.7 |  1454 |  17
>>> -mcpu=msp430x |  -2.3|   -2.4 |  1460 |  10
>>> -mlarge   |  -1.7|   -1.9 |  1412 |  37
>>>
>>> Successfully regtested on trunk for msp430-elf, ok to apply?
>>>
>>> Jozef Lawrynowicz (5):
>>>   MSP430: Implement TARGET_MEMORY_MOVE_COST
>>>   MSP430: Implement TARGET_RTX_COSTS
>>>   MSP430: Add defaulting to the insn length attribute
>>>   MSP430: Implement TARGET_INSN_COST
>>>   MSP430: Skip index-1.c test
>>>
>>>  gcc/config/msp430/msp430-protos.h |   5 +-
>>>  gcc/config/msp430/msp430.c| 867 --
>>>  gcc/config/msp430/msp430.h|  13 +
>>>  gcc/config/msp430/msp430.md   | 439 +++--
>>>  gcc/config/msp430/msp430.opt  |   4 +
>>>  gcc/config/msp430/predicates.md   |  13 +
>>>  gcc/testsuite/gcc.c-torture/execute/index-1.c |   2 +
>>>  7 files changed, 1206 insertions(+), 137 deletions(-)

[ ... ]

So it's a series of 5 patches.  They LGTM.    And if  there's minor
updates needed to address issues found once they're on the trunk, the
consider those updates pre-approved.

Note that defining LOGICAL_OP_NON_SHORT_CIRCUIT and BRANCH_COST impact
gimple code generation.  I'm a bit surprised there wasn't more fallout
in the existing tests.

jeff




ping x4 [PATCH 0/5] MSP430: Implement macros to describe relative costs of operations

2020-11-06 Thread Jozef Lawrynowicz
4th ping for below. Patches are attached, OP linked here:
https://gcc.gnu.org/pipermail/gcc-patches/2020-July/550542.html

Thanks,
Jozef

On Wed, Oct 14, 2020 at 04:31:30PM +0100, Jozef Lawrynowicz wrote:
> 3rd ping for below.
> 
> On Tue, Sep 15, 2020 at 09:30:22PM +0100, Jozef Lawrynowicz wrote:
> > Ping x2 for below.
> > 
> > On Fri, Aug 07, 2020 at 12:02:59PM +0100, Jozef Lawrynowicz wrote:
> > > Pinging for this series of patches.
> > > Attached all patches to this mail with the ammended patch 4 thanks to
> > > Segher's review.
> > > 
> > > Thanks,
> > > Jozef
> > > 
> > > On Thu, Jul 23, 2020 at 04:43:56PM +0100, Jozef Lawrynowicz wrote:
> > > > The following series of patches for MSP430 implement some of the target
> > > > macros used to determine the relative costs of operations.
> > > > 
> > > > To give an indication of the overall effect of these changes on
> > > > codesize, below are some size statistics collected from all the
> > > > executable files from execute.exp that are built at -Os.
> > > > There are around 1470 such tests (depending on the configuration).
> > > > 
> > > > The percentage change (((new - old)/old) * 100) in text size is 
> > > > calculated
> > > > for each test and the given metric is applied to that overall set of 
> > > > data.
> > > > 
> > > > Configuration | Mean (%) | Median (%) | Delta < 0 (count) | Delta > 0 
> > > > (count)
> > > > -
> > > > -mcpu=msp430  |  -2.4|   -2.7 |  1454 |  17
> > > > -mcpu=msp430x |  -2.3|   -2.4 |  1460 |  10
> > > > -mlarge   |  -1.7|   -1.9 |  1412 |  37
> > > > 
> > > > Successfully regtested on trunk for msp430-elf, ok to apply?
> > > > 
> > > > Jozef Lawrynowicz (5):
> > > >   MSP430: Implement TARGET_MEMORY_MOVE_COST
> > > >   MSP430: Implement TARGET_RTX_COSTS
> > > >   MSP430: Add defaulting to the insn length attribute
> > > >   MSP430: Implement TARGET_INSN_COST
> > > >   MSP430: Skip index-1.c test
> > > > 
> > > >  gcc/config/msp430/msp430-protos.h |   5 +-
> > > >  gcc/config/msp430/msp430.c| 867 --
> > > >  gcc/config/msp430/msp430.h|  13 +
> > > >  gcc/config/msp430/msp430.md   | 439 +++--
> > > >  gcc/config/msp430/msp430.opt  |   4 +
> > > >  gcc/config/msp430/predicates.md   |  13 +
> > > >  gcc/testsuite/gcc.c-torture/execute/index-1.c |   2 +
> > > >  7 files changed, 1206 insertions(+), 137 deletions(-)
> > > > 
> > > > -- 
> > > > 2.27.0
> > > > 
> > 
>From e260de5a31e661afdfaaf2c8053b574a292d6826 Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz 
Date: Thu, 16 Jul 2020 11:28:11 +0100
Subject: [PATCH 1/5] MSP430: Implement TARGET_MEMORY_MOVE_COST

The cycle and size cost of a MOV instruction in different addressing
modes can be used to calculate the TARGET_MEMORY_MOVE_COST relative to
TARGET_REGISTER_MOVE_COST.

gcc/ChangeLog:

* config/msp430/msp430.c (struct single_op_cost): New struct.
(struct double_op_cost): Likewise.
(TARGET_REGISTER_MOVE_COST): Don't define but add comment.
(TARGET_MEMORY_MOVE_COST): Define to...
(msp430_memory_move_cost): New function.
(BRANCH_COST): Don't define but add comment.
---
 gcc/config/msp430/msp430.c | 131 +
 1 file changed, 131 insertions(+)

diff --git a/gcc/config/msp430/msp430.c b/gcc/config/msp430/msp430.c
index c2b24974364..9e739233fa0 100644
--- a/gcc/config/msp430/msp430.c
+++ b/gcc/config/msp430/msp430.c
@@ -1043,6 +1043,137 @@ msp430_legitimate_constant (machine_mode mode, rtx x)
 }
 
 
+/* Describing Relative Costs of Operations
+   To model the cost of an instruction, use the number of cycles when
+   optimizing for speed, and the number of words when optimizing for size.
+   The cheapest instruction will execute in one cycle and cost one word.
+   The cycle and size costs correspond to 430 ISA instructions, not 430X
+   instructions or 430X "address" instructions.  The relative costs of 430X
+   instructions is accurately modeled with the 430 costs.  The relative costs
+   of some "address" instructions can differ, but these are not yet handled.
+   Adding support for this could improve performance/code size.  */
+
+const int debug_rtx_costs = 0;
+
+struct single_op_cost
+{
+  const int reg;
+  /* Indirect register (@Rn) or indirect autoincrement (@Rn+).  */
+  const int ind;
+  const int mem;
+};
+
+static const struct single_op_cost cycle_cost_single_op =
+{
+  1, 3, 4
+};
+
+static const struct single_op_cost size_cost_single_op =
+{
+  1, 1, 2
+};
+
+/* When the destination of an insn is memory, the cost is always the same
+   regardless of whether that memory is accessed using indirect register,
+   indexed or absolute addressing.
+   When the source operand is memory, indirect register and post-increment have
+   

Re: [PATCH 4/4 v2] c++: Consider only relevant template arguments in sat_hasher

2020-11-06 Thread Jason Merrill via Gcc-patches

On 11/5/20 8:40 PM, Patrick Palka wrote:

A large source of cache misses in satisfy_atom is caused by the identity
of an (atom,args) pair within the satisfaction cache being determined by
the entire set of supplied template arguments rather than by the subset
of template arguments that the atom actually depends on.  For instance,
consider

   template  concept range = range_v;
   template  void foo () requires range;
   template  void bar () requires range;

The associated constraints of foo and bar are equivalent: they both
consist of the atom range_v (with mapping T -> U).  But the sat_cache
currently will never reuse a satisfaction value between the two atoms
because foo has one template parameter and bar has two, and the
satisfaction cache conservatively assumes that all template parameters
of the constrained decl are relevant to a satisfaction value of one of
its atoms.

This patch eliminates this assumption and makes the sat_cache instead
care about just the subset of args of an (atom,args) pair that is
relevant to satisfaction.

This patch additionally fixes a seemingly latent bug that was found when
testing against range-v3.  In the testcase concepts-decltype2.C below,
during normalization of f's constraints we end up with a TARGET_EXPR
whose _SLOT has a DECL_CONTEXT that points to g instead of f because
current_function_decl is not updated before we start normalizing.
This patch fixes this accordingly, and also adds a sanity check to
keep_template_parm to verify each found parameter has a valid index.

With this patch, compile time and memory usage for the cmcstl2 test
test/algorithm/set_symmetric_difference4.cpp drops from 8.5s/1.2GB to
3.5s/0.4GB.

gcc/cp/ChangeLog:

* constraint.cc (norm_info::norm_info): Initialize orig_decl.
(norm_info::orig_decl): New data member.
(normalize_atom): When caching an atom for the first time,
compute a list of template parameters used in the targets of the
parameter mapping and store it in the TREE_TYPE of the mapping.
(get_normalized_constraints_from_decl): Set current_function_decl
appropriately when normalizing.  As an optimization, don't set
up a push_nested_class_guard when decl has no constraints.
(sat_hasher::hash): Use this list to hash only the template
arguments that are relevant to the atom.
(satisfy_atom): Use this list to compare only the template
arguments that are relevant to the atom.
---
  gcc/cp/constraint.cc  | 78 +--
  gcc/cp/pt.c   | 10 +++
  .../g++.dg/cpp2a/concepts-decltype2.C | 12 +++
  3 files changed, 94 insertions(+), 6 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-decltype2.C

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index 9dd5d892ce9..90901bf7277 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -616,7 +616,8 @@ struct norm_info : subst_info
  
norm_info (tree in_decl, tsubst_flags_t complain)

  : subst_info (tf_warning_or_error | complain, in_decl),
-  context (make_context (in_decl))
+  context (make_context (in_decl)),
+  orig_decl (in_decl)
{}
  
bool generate_diagnostics() const

@@ -647,6 +648,12 @@ struct norm_info : subst_info
   for that check.  */
  
tree context;

+
+  /* The declaration whose constraints we're normalizing.  The targets
+ of the parameter mapping of each atom will be in terms of the
+ template parameters of ORIG_DECL.  */
+
+  tree orig_decl = NULL_TREE;
  };
  
  static tree normalize_expression (tree, tree, norm_info);

@@ -743,6 +750,28 @@ normalize_atom (tree t, tree args, norm_info info)
tree *slot = atom_cache->find_slot (atom, INSERT);
if (*slot)
return *slot;
+
+  /* Find all template parameters used in the targets of the parameter
+mapping, and store a list of them in the TREE_TYPE of the mapping.
+This list will be used by sat_hasher to determine the subset of
+supplied template arguments that the satisfaction value of the atom
+depends on.  */
+  if (map)
+   {
+ tree targets = make_tree_vec (list_length (map));
+ int i = 0;
+ for (tree node = map; node; node = TREE_CHAIN (node))
+   {
+ tree target = TREE_PURPOSE (node);
+ TREE_VEC_ELT (targets, i++) = target;
+   }
+ tree ctx_parms = (info.orig_decl
+   ? DECL_TEMPLATE_PARMS (info.orig_decl)
+   : current_template_parms);
+ tree target_parms = find_template_parameters (targets, ctx_parms);
+ TREE_TYPE (map) = target_parms;
+   }
+
*slot = atom;
  }
return atom;
@@ -854,10 +883,17 @@ get_normalized_constraints_from_decl (tree d, bool diag = 
false)
  if (tree *p = hash_map_safe_get (normalized_map, tmpl))
return *p;
  
-  

Re: [PATCH] pch: Specify reason of -Winvalid-pch warning [PR86674]

2020-11-06 Thread Nicholas Guriev
Hello!

On Wed, 2020-03-25 at 16:46 -0600, Jeff Law wrote:
> On Mon, 2020-03-09 at 11:55 +0300, Nicholas Guriev wrote:
> > gcc/c-family/ChangeLog:
> > 
> > PR pch/86674
> > * c-pch.c (c_common_valid_pch): Use cpp_warning with CPP_W_INVALID_PCH
> > reason to fix -Werror=invalid-pch and -Wno-error=invalid-pch switches.
> THanks.  This looks pretty reasonable, but I'm going to queue it for gcc-11.

What's the status of the patch? Is it good enough for accepting? I don't
see it in the master branch of the GCC repository. Is it needed anything
more from my side? I'm new for GCC development yet and perhaps missed
something...



signature.asc
Description: This is a digitally signed message part


Re: [PATCH] openmp: Retire nest-var ICV

2020-11-06 Thread Tobias Burnus

Hello Kwok, hi Jakub,

On 06.11.20 21:13, Kwok Cheung Yeung wrote:


In addition to deprecating the omp_(get|set)_nested() functions and
OMP_NESTED environment variable, OpenMP 5.0 also removes the nest-var
ICV altogether, defining it in terms of the max-active-levels-var ICV
instead. [...]


Shouldn't libgomp/libgomp.texi be also updated?

Tobias
-
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander 
Walter


Re: [30/32] test harness

2020-11-06 Thread Jeff Law via Gcc-patches


On 11/3/20 2:18 PM, Nathan Sidwell wrote:
> Here's the test harness change, adding a few more prune cases.
>
>
> 30-test-harness.diff
>
OK

jeff




Re: [19/32] global trees

2020-11-06 Thread Jeff Law via Gcc-patches


On 11/3/20 2:16 PM, Nathan Sidwell wrote:
> We directly reference global trees, and expect them to be immutable.
> This reorders the global tree arrays, moving the mutable ones after a
> High Water Mark.  Those after the HWM are not directly accessed in the
> module machinery (we'll reference by name or equivalent).
>
>
>
> 19-global-trees.diff
>
OK

jeff




Re: [14/32] new keywords

2020-11-06 Thread Jeff Law via Gcc-patches


On 11/3/20 2:15 PM, Nathan Sidwell wrote:
> We have 3 new keywords.  As I mentioned in the preprocessor lexing,
> the keywords are context-sensitive, and we create internal ones. 
> These internal ones have an 'invisible' space at the end of them. 
> This has the advantage of making them return to normal identifiers in
> preprocessor output.
>
>
> 14-family-keywords.diff
>
OK

jeff




Re: [PATCH 3/4 v2] c++: Use two levels of caching in satisfy_atom

2020-11-06 Thread Jason Merrill via Gcc-patches

On 11/5/20 8:40 PM, Patrick Palka wrote:

This improves the effectiveness of caching in satisfy_atom by querying
the cache again after we've instantiated the atom's parameter mapping.

Before instantiating its mapping, the identity of an (atom,args) pair
within the satisfaction cache is determined by idiosyncratic things like
the level and index of each template parameter used in targets of the
parameter mapping.  For example, the associated constraints of foo in

   template  concept range = range_v;
   template  void foo () requires range && range;

are range_v (with mapping T -> U) /\ range_v (with mapping T -> V).
If during satisfaction the template arguments supplied for U and V are
the same, then the satisfaction value of these two atoms will be the
same (despite their uninstantiated parameter mappings being different).

But sat_cache doesn't see this because it compares the uninstantiated
parameter mapping and the supplied template arguments of sat_entry's
independently.  So satisy_atom on this latter atom will end up fully
evaluating it instead of reusing the satisfaction value of the former.

But there is a point when the two atoms do look the same to sat_cache,
and that's after instantiating their parameter mappings.  By querying
the cache again at this point, we avoid substituting the instantiated
mapping into the second atom's expression.  This in general results in
a higher cache hit rate in satisfy_atom.

With this patch, compile time and memory usage for the cmcstl2 test
test/algorithm/set_symmetric_diference4.cpp drops from 11s/1.4GB to
8.5s/1.2GB with an --enable-checking=release compiler.


OK.


gcc/cp/ChangeLog:

* cp-tree.h (ATOMIC_CONSTR_MAP_INSTANTIATED_P): Define this flag
for ATOMIC_CONSTRs.
* constraint.cc (sat_hasher::hash): Use hash_atomic_constraint
if the flag is set, otherwise keep using a pointer hash.
(sat_hasher::equal): Return false if the flag's setting differs
on two atoms.  Call atomic_constraints_identical_p if the flag
is set, otherwise keep using a pointer equality test.
(satisfy_atom): After instantiating the parameter mapping, form
another ATOMIC_CONSTR using the instantiated mapping and query
the cache again.  Cache the satisfaction value of both atoms.
(diagnose_atomic_constraint): Simplify now that the supplied
atom has an instantiated mapping.
---
  gcc/cp/constraint.cc | 57 +---
  gcc/cp/cp-tree.h |  7 ++
  2 files changed, 50 insertions(+), 14 deletions(-)

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index 613ced26e2b..9dd5d892ce9 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -2310,17 +2310,37 @@ struct sat_hasher : ggc_ptr_hash
  {
static hashval_t hash (sat_entry *e)
{
-/* Since normalize_atom caches the ATOMIC_CONSTRs it returns,
-   we can assume pointer-based identity for fast hashing and
-   comparison.  Even if this assumption is violated, that's
-   okay, we'll just get a cache miss.  */
+if (ATOMIC_CONSTR_MAP_INSTANTIATED_P (e->constr))
+  {
+   /* Atoms with instantiated mappings are built during satisfaction.
+  They live only inside the sat_cache, and we build one to query
+  the cache with each time we instantiate a mapping.  */
+   gcc_assert (!e->args);
+   return hash_atomic_constraint (e->constr);
+  }
+
+/* Atoms with uninstantiated mappings are built during normalization.
+   Since normalize_atom caches the atoms it returns, we can assume
+   pointer-based identity for fast hashing and comparison.  Even if this
+   assumption is violated, that's okay, we'll just get a cache miss.  */
  hashval_t value = htab_hash_pointer (e->constr);
+
  return iterative_hash_template_arg (e->args, value);
}
  
static bool equal (sat_entry *e1, sat_entry *e2)

{
-/* As in sat_hasher::hash.  */
+if (ATOMIC_CONSTR_MAP_INSTANTIATED_P (e1->constr)
+   != ATOMIC_CONSTR_MAP_INSTANTIATED_P (e2->constr))
+  return false;
+
+/* See sat_hasher::hash.  */
+if (ATOMIC_CONSTR_MAP_INSTANTIATED_P (e1->constr))
+  {
+   gcc_assert (!e1->args && !e2->args);
+   return atomic_constraints_identical_p (e1->constr, e2->constr);
+  }
+
  if (e1->constr != e2->constr)
return false;
  return template_args_equal (e1->args, e2->args);
@@ -2614,6 +2634,18 @@ satisfy_atom (tree t, tree args, subst_info info)
return cache.save (boolean_false_node);
  }
  
+  /* Now build a new atom using the instantiated mapping.  We use

+ this atom as a second key to the satisfaction cache, and we
+ also pass it to diagnose_atomic_constraint so that diagnostics
+ which refer to the atom display the instantiated mapping.  */
+  t = copy_node (t);
+  ATOMIC_CONSTR_MAP (t) = map;
+  gcc_assert (!ATOMIC_CONSTR_MAP_INSTANTIATED_P (t));
+  

Re: [11/32] instrumentation

2020-11-06 Thread Jeff Law via Gcc-patches


On 11/3/20 2:14 PM, Nathan Sidwell wrote:
> I add one new parameter -- the number of concurrently open module
> files, and 3 instrumentation timers.
>
>
>
> 11-core-parmtime.diff
>

OK

jeff




Re: [PATCH] c++: Reuse identical ATOMIC_CONSTRs during normalization

2020-11-06 Thread Jason Merrill via Gcc-patches

On 11/5/20 8:31 PM, Patrick Palka wrote:

On Thu, 5 Nov 2020, Patrick Palka wrote:


On Thu, 5 Nov 2020, Jason Merrill wrote:


On 11/3/20 3:43 PM, Patrick Palka wrote:

Profiling revealed that sat_hasher::equal accounts for nearly 40% of
compile time in some cmcstl2 tests.

This patch eliminates this bottleneck by caching the ATOMIC_CONSTRs
returned by normalize_atom.  This in turn allows us to replace the
expensive atomic_constraints_identical_p check in sat_hasher::equal
with cheap pointer equality, with no loss in cache hit rate.

With this patch, compile time for the cmcstl2 test
test/algorithm/set_symmetric_difference4.cpp drops from 19s to 11s with
an --enable-checking=release compiler.

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?

gcc/cp/ChangeLog:

* constraint.cc (struct atom_hasher): New descriptor class for a
hash_table.  Use it to define ...
(atom_cache): ... this.
(normalize_atom): Use it to cache ATOMIC_CONSTRs when not
generating diagnostics.
(sat_hasher::hash): Use htab_hash_pointer instead of
hash_atomic_constraint.
(sat_hasher::equal): Test for pointer equality instead of
atomic_constraints_identical_p.
---
   gcc/cp/constraint.cc | 37 ++---
   1 file changed, 34 insertions(+), 3 deletions(-)

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index b6f6f0d02a5..ce720c641e8 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -710,6 +710,25 @@ normalize_concept_check (tree check, tree args,
norm_info info)
 return normalize_expression (def, subst, info);
   }
   +/* Hash functions for ATOMIC_CONSTRs.  */
+
+struct atom_hasher : default_hash_traits
+{
+  static hashval_t hash (tree atom)
+  {
+return hash_atomic_constraint (atom);
+  }
+
+  static bool equal (tree atom1, tree atom2)
+  {
+return atomic_constraints_identical_p (atom1, atom2);
+  }
+};


This is the same as constraint_hash in logic.cc; either they should be
combined, or (probably) the hash table in logic.cc should be changed to also
take advantage of pointer equivalence.


Ah, I forgot about the existence of this hasher.  Consider this hasher
changed to take advantage of the pointer equivalence (I'll post a
revised and tested patch later today).


On second thought, if we make the hasher in logic.cc and other places
(e.g. add_constraint and constraints_equivalent_p) take advantage of
pointer-based identity for ATOMIC_CONSTR, then we'd be relying on the
uniqueness assumption in a crucial way rather than just relying on it in
the satisfaction cache in a benign way that doesn't affect the semantics
of the program if the assumption is somehow violated (we just get a
cache miss if it is violated).


True.  But if doing this is a big speedup for satisfaction, I imagine it 
would also be a big speedup for subsumption.


Maybe use the assumption and if CHECKING_P, make sure that the 
structural comparison matches the pointer comparison?


The v2 patch is OK for now, this question doesn't need to block it.


So it seems to me it'd be better to not infect other parts of the code
with this assumption, and to keep it local to the satisfaction cache,
so as to minimize complexity.  So I suppose we should go with combining
these two "structural" hashers.






+/* Used by normalize_atom to cache ATOMIC_CONSTRs.  */
+
+static GTY((deletable)) hash_table *atom_cache;


If we're relying on pointer identity, this can't be deletable; if GC discards
it, later normalization will generate a new equivalent ATOMIC_CONSTR, breaking
the uniqueness assumption.


But because no ATOMIC_CONSTR is ever reachable from a GC root (since
they live only inside GC-deletable structures), there will never be two
equivalent ATOMIC_CONSTR trees live at once, which is a sufficient
enough notion of uniqueness for us, I think.


Makes sense.


   /* The normal form of an atom depends on the expression. The normal
  form of a function call to a function concept is a check constraint
  for that concept. The normal form of a reference to a variable
@@ -729,7 +748,19 @@ normalize_atom (tree t, tree args, norm_info info)
 /* Build a new info object for the atom.  */
 tree ci = build_tree_list (t, info.context);
   -  return build1 (ATOMIC_CONSTR, ci, map);
+  tree atom = build1 (ATOMIC_CONSTR, ci, map);
+  if (!info.generate_diagnostics ())
+{
+  /* Cache the ATOMIC_CONSTRs that we return, so that sat_hasher::equal
+later can quickly compare two atoms using just pointer equality.  */
+  if (!atom_cache)
+   atom_cache = hash_table::create_ggc (31);
+  tree *slot = atom_cache->find_slot (atom, INSERT);
+  if (*slot)
+   return *slot;
+  *slot = atom;
+}
+  return atom;
   }
 /* Returns the normal form of an expression. */
@@ -2284,13 +2315,13 @@ struct sat_hasher : ggc_ptr_hash
   {
 static hashval_t hash (sat_entry *e)
 {


We could 

[PATCH] Objective-C/C++ (C-family) : Add missing 'atomic' property attribute.

2020-11-06 Thread Iain Sandoe
Hi

Arguably, this is actually a bug fix since the ‘atomic’ attribute is
paired with the ‘nonatomic’ one.  However it is the default and was
omitted when the @property implementation was added.

‘atomic’ in Objective-C terms is not specified in relation to _Atomic
or std::atomic (the _Atomic keyword is not accepted in that context).

tested across several Darwin versions and on x86_64-linux-gnu

OK for the C-family parts?
thanks
Iain



This is the default, but it is still legal in user code and therefore
we should handle it in parsing.  Fix whitespace issues in the lines
affected.

gcc/c-family/ChangeLog:

* c-common.c (c_common_reswords): Add 'atomic' property
attribute.
* c-common.h (enum rid): Add RID_PROPATOMIC for atomic
property attributes.

gcc/testsuite/ChangeLog:

* obj-c++.dg/property/at-property-4.mm: Test atomic property
attribute.
* objc.dg/property/at-property-4.m: Likewise.
---
 gcc/c-family/c-common.c | 17 +
 gcc/c-family/c-common.h |  2 +-
 gcc/objc/objc-act.c |  1 +
 .../obj-c++.dg/property/at-property-4.mm|  3 +++
 gcc/testsuite/objc.dg/property/at-property-4.m  |  3 +++
 5 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 9302a2461d4..d4d3228b8f6 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -571,14 +571,15 @@ const struct c_common_resword c_common_reswords[] =
   { "oneway",  RID_ONEWAY, D_OBJC },
   { "out", RID_OUT,D_OBJC },
   /* These are recognized inside a property attribute list */
-  { "assign",  RID_ASSIGN, D_OBJC }, 
-  { "copy",RID_COPY,   D_OBJC }, 
-  { "getter",  RID_GETTER, D_OBJC }, 
-  { "nonatomic",   RID_NONATOMIC,  D_OBJC }, 
-  { "readonly",RID_READONLY,   D_OBJC }, 
-  { "readwrite",   RID_READWRITE,  D_OBJC }, 
-  { "retain",  RID_RETAIN, D_OBJC }, 
-  { "setter",  RID_SETTER, D_OBJC }, 
+  { "assign",  RID_ASSIGN, D_OBJC },
+  { "atomic",  RID_PROPATOMIC, D_OBJC },
+  { "copy",RID_COPY,   D_OBJC },
+  { "getter",  RID_GETTER, D_OBJC },
+  { "nonatomic",   RID_NONATOMIC,  D_OBJC },
+  { "readonly",RID_READONLY,   D_OBJC },
+  { "readwrite",   RID_READWRITE,  D_OBJC },
+  { "retain",  RID_RETAIN, D_OBJC },
+  { "setter",  RID_SETTER, D_OBJC },
 };
 
 const unsigned int num_c_common_reswords =
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 18b489d55a3..7e2cd5342aa 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -85,7 +85,7 @@ enum rid
   RID_GETTER, RID_SETTER,
   RID_READONLY, RID_READWRITE,
   RID_ASSIGN, RID_RETAIN, RID_COPY,
-  RID_NONATOMIC,
+  RID_PROPATOMIC, RID_NONATOMIC,
 
   /* C (reserved and imaginary types not implemented, so any use is a
  syntax error) */
diff --git a/gcc/objc/objc-act.c b/gcc/objc/objc-act.c
index 8be4beadf3b..2dad46aa77e 100644
--- a/gcc/objc/objc-act.c
+++ b/gcc/objc/objc-act.c
@@ -822,6 +822,7 @@ objc_prop_attr_kind_for_rid (enum rid prop_rid)
   case RID_RETAIN: return OBJC_PROPERTY_ATTR_RETAIN;
   case RID_COPY:   return OBJC_PROPERTY_ATTR_COPY;
 
+  case RID_PROPATOMIC: return OBJC_PROPERTY_ATTR_ATOMIC;
   case RID_NONATOMIC:  return OBJC_PROPERTY_ATTR_NONATOMIC;
 
 }
diff --git a/gcc/testsuite/obj-c++.dg/property/at-property-4.mm 
b/gcc/testsuite/obj-c++.dg/property/at-property-4.mm
index 4083947de71..31f2eb4336a 100644
--- a/gcc/testsuite/obj-c++.dg/property/at-property-4.mm
+++ b/gcc/testsuite/obj-c++.dg/property/at-property-4.mm
@@ -16,6 +16,7 @@
 /* Test that all the new property attributes can be parsed.  */
 @property (assign)id property_a;
 @property (copy)  id property_b;
+@property (atomic)int property_ca;
 @property (nonatomic) int property_c;
 @property (readonly)  int property_d;
 @property (readwrite) int property_e;
@@ -34,6 +35,8 @@
 @property (assign, copy) id d;/* { dg-error ".copy. attribute 
conflicts with .assign. attribute" } */
 @property (copy, retain) id e;/* { dg-error ".retain. attribute 
conflicts with .copy. attribute" } */
 
+@property (atomic, nonatomic) int property_j; /* { dg-error {'nonatomic' 
attribute conflicts with 'atomic' attribute} } */
+
 @property (setter=mySetter:,setter=mySetter2:)  int f; /* { dg-warning 
{multiple property 'setter' methods specified, the latest one will be used} } */
 @property (getter=myGetter, getter=myGetter2 )  int g; /* { dg-warning 
{multiple property 'getter' methods specified, the latest one will be used} } */
 
diff 

Re: [04/32] cpp lexer

2020-11-06 Thread Jeff Law via Gcc-patches


On 11/3/20 2:13 PM, Nathan Sidwell wrote:
> c++ modules creates 2 new kinds of preprocessor lines
> [export] module ...
> [export] import ...
>
> To all intents and purposes these are cppdirectives spelt without a
> leading '#'.  module and import are context-sensitive keywords.  Thus
> preprocessor tokenizing needs a bit of token peeking.  This is that
> peeking.
>
> We have a new node flag 'NODE_MODULE', which marks whether an
> identifier is significant to this peeking.  When we see such an
> identifier at the beginning of a logical line, we need to peek further
> and figure out whether these are those keywords.
>
> When successfully peeked, we replace the identifiers with
> internal-only tokens that the c++ parser recognizes.
>
>
> 04-cpp-lexer.diff
>
> diff --git c/libcpp/include/cpplib.h w/libcpp/include/cpplib.h
> index 8e398863cf6..81be6457951 100644
> --- c/libcpp/include/cpplib.h
> +++ w/libcpp/include/cpplib.h
>
> @@ -888,9 +915,9 @@ struct GTY(()) cpp_hashnode {
>unsigned int directive_index : 7;  /* If is_directive,
>  then index into directive table.
>  Otherwise, a NODE_OPERATOR.  */
> -  unsigned char rid_code;/* Rid code - for front ends.  */
> +  unsigned int rid_code : 8; /* Rid code - for front ends.  */
> +  unsigned int flags : 9;/* CPP flags.  */
>ENUM_BITFIELD(node_type) type : 2; /* CPP node type.  */
> -  unsigned int flags : 8;/* CPP flags.  */
>  
>/* 6 bits spare (plus another 32 on 64-bit hosts).  */

Someone already mentioned it, but the # of spare bits needs updating.


>  
> diff --git c/libcpp/lex.c w/libcpp/lex.c
> index fb222924c8c..b3498f195bf 100644
> --- c/libcpp/lex.c
> +++ w/libcpp/lex.c
> @@ -2606,6 +2622,131 @@ _cpp_temp_token (cpp_reader *pfile)
>return result;
>  }
>  
> +/* RESULT is a CPP_NAME with NODE_MODULE set.  See if we should enter
> +   deferred_pragma mode to tokenize the rest of the line.  */
> +
> +static void
> +cpp_maybe_module_directive (cpp_reader *pfile, cpp_token *result)
> +{
> +  unsigned backup = 0; /* Tokens we peeked.  */
> +  cpp_hashnode *node = result->val.node.node;
> +  cpp_token *peek = result;
> +  cpp_token *keyword = peek;
> +  cpp_hashnode *(_modules)[spec_nodes::M_HWM][2] = 
> pfile->spec_nodes.n_modules;
> +  int header_count = 0;
> +
> +  /* Enter directives mode for the peeking.  */
> +  pfile->state.in_deferred_pragma = true;
> +  pfile->state.pragma_allow_expansion = true;
> +  pfile->state.save_comments = 0;
> +  pfile->directive_line = result->src_loc;
It looks like you slam in known values when you drop out of directive
mode rather than restoring the original values.  That may be OK, I'm not
all that familiar with this code to know if that's corerct or not.
> +
> +  if (__builtin_expect (node == n_modules[spec_nodes::M__IMPORT][0], false))
> +/* __import  */
> +header_count = backup + 2 + 16;
> +  else if (__builtin_expect (node == n_modules[spec_nodes::M_IMPORT][0], 
> false))
> +/* import  */
> +header_count = backup + 2 + (CPP_OPTION (pfile, preprocessed) ? 16 : 0);
> +  else if (__builtin_expect (node == n_modules[spec_nodes::M_MODULE][0], 
> false))
> +; /* module  */
> +  else
> +goto not_module;

Are the __builtin_expects really useful here?  If not I'd prefer to
avoid them and just let the predictors do their job.  Similarly 
elsewhere in this patch.


+ {
> +not_module:
> +  /* Drop out of directive mode.  */
> +  pfile->state.save_comments
> + = !CPP_OPTION (pfile, discard_comments);
> +  pfile->state.in_deferred_pragma = false;
> +  pfile->state.angled_headers = false;

This doesn't seem to match the code to go into directives mode all that
well.  You don't reset pragma_allow_expansion or directive_line.


Jeff


[PATCH] Objective-C : Implement NSObject attribute.

2020-11-06 Thread Iain Sandoe
Hi

Originally, I had this as a Darwin-only patch, however GNUStep
also makes use of NSObject and similar constructs, so this really
needs to be available to linux-gnu as well.

tested across several Darwin versions and on x86_64-linux-gnu.

OK for the c-family changes?
thanks
Iain

=

This attribute allows pointers to be marked as pointers to
an NSObject-compatible object.  This allows for additional
checking of assignment etc. when referring to pointers to
opaque types (or type-erased via a void * cast).

gcc/c-family/ChangeLog:

* c-attribs.c (handle_nsobject_attribute): New.
* c.opt: Add WNSObject-attribute.

gcc/objc/ChangeLog:

* objc-act.c (objc_compare_types): Handle NSObject type
attributes.
(objc_type_valid_for_messaging): Likewise.

gcc/testsuite/ChangeLog:

* obj-c++.dg/attributes/nsobject-01.mm: New test.
* objc.dg/attributes/nsobject-01.m: New test.
---
 gcc/c-family/c-attribs.c  | 39 +++
 gcc/c-family/c.opt|  4 ++
 gcc/objc/objc-act.c   | 39 +--
 .../obj-c++.dg/attributes/nsobject-01.mm  | 66 +++
 .../objc.dg/attributes/nsobject-01.m  | 66 +++
 5 files changed, 207 insertions(+), 7 deletions(-)
 create mode 100644 gcc/testsuite/obj-c++.dg/attributes/nsobject-01.mm
 create mode 100644 gcc/testsuite/objc.dg/attributes/nsobject-01.m

diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index 65cba5074e5..f1680820ecd 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -157,6 +157,7 @@ static tree handle_designated_init_attribute (tree *, tree, 
tree, int, bool *);
 static tree handle_patchable_function_entry_attribute (tree *, tree, tree,
   int, bool *);
 static tree handle_copy_attribute (tree *, tree, tree, int, bool *);
+static tree handle_nsobject_attribute (tree *, tree, tree, int, bool *);
 
 /* Helper to define attribute exclusions.  */
 #define ATTR_EXCL(name, function, type, variable)  \
@@ -509,6 +510,9 @@ const struct attribute_spec c_common_attribute_table[] =
  handle_noinit_attribute, attr_noinit_exclusions },
   { "access",1, 3, false, true, true, false,
  handle_access_attribute, NULL },
+  /* Attributes used by Objective-C.  */
+  { "NSObject",  0, 0, true, false, false, false,
+ handle_nsobject_attribute, NULL },
   { NULL, 0, 0, false, false, false, false, NULL, NULL }
 };
 
@@ -5124,6 +5128,41 @@ handle_patchable_function_entry_attribute (tree *, tree 
name, tree args,
   return NULL_TREE;
 }
 
+/* Handle a "NSObject" attributes; arguments as in
+   struct attribute_spec.handler.  */
+
+static tree
+handle_nsobject_attribute (tree *node, tree name, tree args,
+  int /*flags*/, bool *no_add_attrs)
+{
+  *no_add_attrs = true;
+
+  /* This attribute only applies to typedefs (or field decls for properties),
+ we drop it otherwise - but warn about this if enabled.  */
+  if (TREE_CODE (*node) != TYPE_DECL && TREE_CODE (*node) != FIELD_DECL)
+{
+  warning (OPT_WNSObject_attribute, "%qE attribute may be put on a"
+  " typedef only; attribute is ignored", name);
+  return NULL_TREE;
+}
+
+  /* The original implementation only allowed pointers to records, however
+ recent implementations also allow void *.  */
+  tree type = TREE_TYPE (*node);
+  if (!type || !POINTER_TYPE_P (type)
+  || (TREE_CODE (TREE_TYPE (type)) != RECORD_TYPE
+  && !VOID_TYPE_P (TREE_TYPE (type
+{
+  error ("%qE attribute is for pointer types only", name);
+  return NULL_TREE;
+}
+
+  tree t = tree_cons (name, args, TYPE_ATTRIBUTES (type));
+  TREE_TYPE (*node) = build_type_attribute_variant (type, t);
+
+  return NULL_TREE;
+}
+
 /* Attempt to partially validate a single attribute ATTR as if
it were to be applied to an entity OPER.  */
 
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index ebd07cc8059..fe16357db85 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -256,6 +256,10 @@ U
 C ObjC C++ ObjC++ Joined Separate MissingArgError(macro name missing after %qs)
 -U  Undefine .
 
+WNSObject-attribute
+C ObjC C++ ObjC++ LTO Var(warn_nsobject_attribute) Warning Init(1)
+Warn if the NSObject attribute is applied to a non-typedef.
+
 Wabi
 C ObjC C++ ObjC++ LTO Var(warn_abi) Warning
 Warn about things that will change when compiling with an ABI-compliant 
compiler.
diff --git a/gcc/objc/objc-act.c b/gcc/objc/objc-act.c
index 68d829fd773..8be4beadf3b 100644
--- a/gcc/objc/objc-act.c
+++ b/gcc/objc/objc-act.c
@@ -2476,9 +2476,14 @@ objc_compare_types (tree ltyp, tree rtyp, int argno, 
tree callee)
   if (!POINTER_TYPE_P (ltyp) || !POINTER_TYPE_P (rtyp))
 return false;
 
+  tree 

c++: Consistently expose singleton overloads

2020-11-06 Thread Nathan Sidwell

This is a patch from my name-lookup overhaul.  I noticed the parser
and one path in name-lookup looked through an overload of a single
known decl.  It seems more consistent to do that in both paths through
name-lookup, and not in the parser itself.

gcc/cp/
* name-lookup.c (lookup_qualified_name): Expose an overload of a
singleton with known type.
(lookup_name_1): Just check the overload's type to expose it.
* parser.c (cp_parser_lookup_name): Do not do that check here.

pushing to trunk

nathan

--
Nathan Sidwell
diff --git i/gcc/cp/name-lookup.c w/gcc/cp/name-lookup.c
index 5bda4c2eea9..16efd161301 100644
--- i/gcc/cp/name-lookup.c
+++ w/gcc/cp/name-lookup.c
@@ -5885,7 +5885,14 @@ lookup_qualified_name (tree scope, tree name, LOOK_want want, bool complain)
   name_lookup lookup (name, want);
 
   if (qualified_namespace_lookup (scope, ))
-	t = lookup.value;
+	{
+	  t = lookup.value;
+
+	  /* If we have a known type overload, pull it out.  This can happen
+	 for using decls.  */
+	  if (TREE_CODE (t) == OVERLOAD && TREE_TYPE (t) != unknown_type_node)
+	t = OVL_FUNCTION (t);
+	}
 }
   else if (cxx_dialect != cxx98 && TREE_CODE (scope) == ENUMERAL_TYPE)
 t = lookup_enumerator (scope, name);
@@ -6515,8 +6522,9 @@ lookup_name_1 (tree name, LOOK_where where, LOOK_want want)
 
  found:;
 
-  /* If we have a single function from a using decl, pull it out.  */
-  if (val && TREE_CODE (val) == OVERLOAD && !really_overloaded_fn (val))
+  /* If we have a known type overload, pull it out.  This can happen
+ for both using decls and unhidden functions.  */
+  if (val && TREE_CODE (val) == OVERLOAD && TREE_TYPE (val) != unknown_type_node)
 val = OVL_FUNCTION (val);
 
   return val;
diff --git i/gcc/cp/parser.c w/gcc/cp/parser.c
index 6e7b982f073..9e3c67c6d15 100644
--- i/gcc/cp/parser.c
+++ w/gcc/cp/parser.c
@@ -28633,11 +28633,6 @@ cp_parser_lookup_name (cp_parser *parser, tree name,
 	  prefer_type_arg (tag_type),
 	  /*complain=*/true);
 
-	  /* If we have a single function from a using decl, pull it out.  */
-	  if (TREE_CODE (decl) == OVERLOAD
-	  && !really_overloaded_fn (decl))
-	decl = OVL_FUNCTION (decl);
-
 	  if (pushed_scope)
 	pop_scope (pushed_scope);
 	}


Re: [committed 1/2] libstdc++: Export basic_stringbuf constructor [PR 97729]

2020-11-06 Thread Jonathan Wakely via Gcc-patches

On 06/11/20 11:56 +0100, Rainer Orth wrote:

Hi Jonathan,


libstdc++-v3/ChangeLog:

PR libstdc++/97729
* config/abi/pre/gnu.ver (GLIBCXX_3.4.29): Add exports.
* src/c++20/sstream-inst.cc (basic_stringbuf): Instantiate
private constructor taking __xfer_bufptrs.

Tested powerpc64le-linux. Committed to trunk.


unfortunately, this broke Solaris bootstrap again:

ld: fatal: libstdc++-symbols.ver-sun: 7314: symbol 
'_ZNSt7__cxx1115basic_stringbufIcSt11char_traitsIcESaIcEEC1EOS4_RKS3_ONS4_14__xfer_bufptrsE':
 symbol version conflict
ld: fatal: libstdc++-symbols.ver-sun: 7315: symbol 
'_ZNSt7__cxx1115basic_stringbufIcSt11char_traitsIcESaIcEEC2EOS4_RKS3_ONS4_14__xfer_bufptrsE':
 symbol version conflict
ld: fatal: libstdc++-symbols.ver-sun: 7316: symbol 
'_ZNSt7__cxx1115basic_stringbufIwSt11char_traitsIwESaIwEEC1EOS4_RKS3_ONS4_14__xfer_bufptrsE':
 symbol version conflict
ld: fatal: libstdc++-symbols.ver-sun: 7317: symbol 
'_ZNSt7__cxx1115basic_stringbufIwSt11char_traitsIwESaIwEEC2EOS4_RKS3_ONS4_14__xfer_bufptrsE':
 symbol version conflict

Those are matched by both

   
##_ZNSt7__cxx1115basic_stringbufI[cw]St11char_traitsI[cw]ESaI[cw]EEC[12]EOS4_RKS3_ONS4_14__xfer_bufptrsE
 (glob)

but also by the previous

   ##_ZNSt7__cxx1115basic_stringbufI[cw]St11char_traitsI[cw]*__xfer_bufptrs* 
(glob)

I do have a hacky patch to avoid this, but I guess I best leave it to
you how to best tighten the previous pattern.


It should be fixed at 887515acd27e49c176395ab76d5826959d89cb9b which
is the attached patch. Only tested on x86_64-linux, but my script no
longer shows the conflicts.

I'll try to incorporate that script into the testsuite for gcc-11, or
rewrite it as aprt of testsuite/util/testsuite_abi.cc


commit 887515acd27e49c176395ab76d5826959d89cb9b
Author: Jonathan Wakely 
Date:   Fri Nov 6 19:53:36 2020

libstdc++: Fix symbol version conflict in linker script

The change in r11-4748-50b840ac5e1d6534e345c3fee9a97ae45ced6bc7 causes
a build error on Solaris, due to the new explicit instantiation matching
patterns for two different symbol versions.

libstdc++-v3/ChangeLog:

* config/abi/pre/gnu.ver (GLIBCXX_3.4.21): Tighten up patterns
for basic_stringbuf that refer to __xfer_bufptrs.

diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver
index ed68ffa28723..2d0f87aa7cc7 100644
--- a/libstdc++-v3/config/abi/pre/gnu.ver
+++ b/libstdc++-v3/config/abi/pre/gnu.ver
@@ -1774,7 +1774,8 @@ GLIBCXX_3.4.21 {
 _ZNSt7__cxx1115basic_stringbufI[cw]St11char_traitsI[cw]ESaI[cw]EEC[12]ERKNS_12basic_stringI[cw]S2_S3_EESt13_Ios_Openmode;
 _ZNSt7__cxx1115basic_stringbufI[cw]St11char_traitsI[cw]ESaI[cw]EEC[12]ESt13_Ios_Openmode;
 _ZNSt7__cxx1115basic_stringbufI[cw]St11char_traitsI[cw]ESaI[cw]EED[012]Ev;
-_ZNSt7__cxx1115basic_stringbufI[cw]St11char_traitsI[cw]*__xfer_bufptrs*;
+_ZNSt7__cxx1115basic_stringbufI[cw]St11char_traitsI[cw]ESaI[cw]EEC[12]EOS4_ONS4_14__xfer_bufptrsE;
+_ZNSt7__cxx1115basic_stringbufI[cw]St11char_traitsI[cw]14__xfer_bufptrs[CD][12]*;
 _ZNSt7__cxx1115basic_stringbufI[cw]St11char_traitsI[cw]ESaI[cw]EE[a1346789]*;
 #   _ZNSt7__cxx1118basic_stringstreamI[cw]St11char_traitsI[cw]*;
 _ZNSt7__cxx1118basic_stringstreamI[cw]St11char_traitsI[cw]ESaI[cw]EEC[12]EOS4_;


[PATCH] openmp: Retire nest-var ICV

2020-11-06 Thread Kwok Cheung Yeung

Hello

In addition to deprecating the omp_(get|set)_nested() functions and OMP_NESTED 
environment variable, OpenMP 5.0 also removes the nest-var ICV altogether, 
defining it in terms of the max-active-levels-var ICV instead.


This patch removes the ICV, and implements the handling of 
omp_(get|set)_nested() in terms of max-active-levels-var as defined in the spec.


The initial value of max-active-levels-var now depends on the number of items in 
OMP_NUM_THREADS and OMP_PROC_BIND as defined in section 2.5.2 of the spec. 
OMP_NESTED now changes the value of max-active-levels-var. If OMP_NESTED is 
false and OMP_MAX_ACTIVE_LEVELS is > 1, I have opted to use the value specified 
by OMP_MAX_ACTIVE_LEVELS, as OMP_NESTED is deprecated in OpenMP 5.0 (the spec 
says this is implementation defined in section 6.9).


The default value of max-active-levels-var is implementation defined (section 
2.5.2). It was previously set to the maximum supported number, but I think it 
should be 1 now, since OMP_NESTED defaults to false on OpenMP 4.5, and this 
replicates that behaviour.


This change regresses the testcase libgomp.c/target-5.c because nested-var is 
per data environment, while max-active-levels-var is per-device. The change in 
semantics causes the test for the nesting setting to fail, because now any 
changes to the nesting setting apply to the whole device, and not just to the 
current data environment. I just deleted this part of the testing as the test 
looks like it is testing per data environment ICVs.


Bootstrapped on x86_64 with no offloading, and libgomp testing carried out with 
nvptx offloading with no regressions.


Okay for trunk?

Thanks

Kwok
commit aad8afea37b33b4d5836b2b64be8f4dab6d74509
Author: Kwok Cheung Yeung 
Date:   Wed Nov 4 15:34:12 2020 -0800

openmp: Retire nest-var ICV for OpenMP 5.0

This removes the nest-var ICV, expressing nesting in terms of the
max-active-levels-var ICV instead.

2020-11-06  Kwok Cheung Yeung  

libgomp/
* env.c (gomp_global_icv): Remove nest_var field.
(gomp_max_active_levels_var): Initialize to 1.
(parse_boolean): Return true on success.
(handle_omp_display_env): Express OMP_NESTED in terms of
gomp_max_active_levels_var.
(initialize_env): Set gomp_max_active_levels_var from
OMP_MAX_ACTIVE_LEVELS, OMP_NESTED, OMP_NUM_THREADS and
OMP_PROC_BIND.
* icv.c (omp_set_nested): Express in terms of
gomp_max_active_levels_var.
(omp_get_nested): Likewise.
* libgomp.h (struct gomp_task_icv): Remove nest_var field.
* parallel.c (gomp_resolve_num_threads): Replace reference
to nest_var with gomp_max_active_levels_var.
* testsuite/libgomp.c/target-5.c: Remove additional options.
(main): Remove references to omp_get_nested and omp_set_nested.

diff --git a/libgomp/env.c b/libgomp/env.c
index ab22525..75d0fe2 100644
--- a/libgomp/env.c
+++ b/libgomp/env.c
@@ -68,12 +68,11 @@ struct gomp_task_icv gomp_global_icv = {
   .run_sched_chunk_size = 1,
   .default_device_var = 0,
   .dyn_var = false,
-  .nest_var = false,
   .bind_var = omp_proc_bind_false,
   .target_data = NULL
 };
 
-unsigned long gomp_max_active_levels_var = gomp_supported_active_levels;
+unsigned long gomp_max_active_levels_var = 1;
 bool gomp_cancel_var = false;
 enum gomp_target_offload_t gomp_target_offload_var
   = GOMP_TARGET_OFFLOAD_DEFAULT;
@@ -959,16 +958,17 @@ parse_spincount (const char *name, unsigned long long 
*pvalue)
 }
 
 /* Parse a boolean value for environment variable NAME and store the
-   result in VALUE.  */
+   result in VALUE.  Return true if one was present and it was
+   successfully parsed.  */
 
-static void
+static bool
 parse_boolean (const char *name, bool *value)
 {
   const char *env;
 
   env = getenv (name);
   if (env == NULL)
-return;
+return false;
 
   while (isspace ((unsigned char) *env))
 ++env;
@@ -987,7 +987,11 @@ parse_boolean (const char *name, bool *value)
   while (isspace ((unsigned char) *env))
 ++env;
   if (*env != '\0')
-gomp_error ("Invalid value for environment variable %s", name);
+{
+  gomp_error ("Invalid value for environment variable %s", name);
+  return false;
+}
+  return true;
 }
 
 /* Parse the OMP_WAIT_POLICY environment variable and return the value.  */
@@ -1252,7 +1256,7 @@ handle_omp_display_env (unsigned long stacksize, int 
wait_policy)
   fprintf (stderr, "  OMP_DYNAMIC = '%s'\n",
   gomp_global_icv.dyn_var ? "TRUE" : "FALSE");
   fprintf (stderr, "  OMP_NESTED = '%s'\n",
-  gomp_global_icv.nest_var ? "TRUE" : "FALSE");
+  gomp_max_active_levels_var > 1 ? "TRUE" : "FALSE");
 
   fprintf (stderr, "  OMP_NUM_THREADS = '%lu", gomp_global_icv.nthreads_var);
   for (i = 1; i < gomp_nthreads_var_list_len; i++)
@@ -1417,16 +1421,11 @@ initialize_env (void)
 
   parse_schedule ();
   parse_boolean ("OMP_DYNAMIC", 

Re: [03/32] cpp-callbacks

2020-11-06 Thread Jeff Law via Gcc-patches


On 11/3/20 2:13 PM, Nathan Sidwell wrote:
> Here are the callbacks in the preprocessor itself.
>
> a) one to handle deferred macros
>
> b) one to handle include translation.  For every '#include ',
> there is the possibility of replacing that with 'import '. 
> This hook determines if that happens.

OK

jeff




Re: [02/32] linemaps

2020-11-06 Thread Jeff Law via Gcc-patches


On 11/3/20 2:13 PM, Nathan Sidwell wrote:
> Location handling needs to add 2 things
>
> 1) a new kind of inclusion -- namely a module.  We add LC_MODULE as a
> map kind,
>
> 2) the ability to allocate blocks of line-maps for both ordinary
> locations and macro locations, that are then filled in by the module
> loader.

LGTM, but give David M a little more time to chime in (he's been on PTO
for at least part of this week).


Say perhaps until Tuesday?


jeff




Re: [PATCH] c++: DR 1914 - Allow duplicate standard attributes.

2020-11-06 Thread Jason Merrill via Gcc-patches

On 11/6/20 2:34 PM, Marek Polacek wrote:

On Fri, Nov 06, 2020 at 02:23:10PM -0500, Jason Merrill via Gcc-patches wrote:

On 11/6/20 2:06 PM, Marek Polacek wrote:

Following Joseph's change for C to allow duplicate C2x standard attributes
,
this patch does a similar thing for C++.  This is DR 1914, to be resolved by
, which is not part of the standard yet, but has a wide
support so look like a shoo-in.  Some duplications still produce warnings;
I didn't change that because a warning might be desirable.


What's the rationale for warning about some and not others?


I don't have any.  Joseph's patch removed the error for a duplicated
'fallthrough' attribute, but the warning remained so I left it as-is
too.

So either we just downgrade the error to a warning, or remove the
remaining warnings too.  I think I slightly prefer the former; with perhaps
a small tweak not to warn when the duplicated attribute comes from a macro
expansion.


Sounds good.

Jason



Re: [PATCH 1/4] c++: Fix ICE with variadic concepts and aliases [PR93907]

2020-11-06 Thread Jason Merrill via Gcc-patches

On 11/5/20 8:40 PM, Patrick Palka wrote:

This patch (naively) extends the PR93907 fix to also apply to variadic
concepts invoked with a type argument pack.  Without this, we ICE on
the below testcase (a variadic version of concepts-using2.C) in the same
manner as we used to on concepts-using2.C before r10-7133.

Patch series bootstrapped and regtested on x86_64-pc-linux-gnu,
and also tested against cmcstl2 and range-v3.

gcc/cp/ChangeLog:

PR c++/93907
* constraint.cc (tsubst_parameter_mapping): Also canonicalize
the type arguments of a TYPE_ARGUMENT_PACk.

gcc/testsuite/ChangeLog:

PR c++/93907
* g++.dg/cpp2a/concepts-using3.C: New test, based off of
concepts-using2.C.
---
  gcc/cp/constraint.cc | 10 
  gcc/testsuite/g++.dg/cpp2a/concepts-using3.C | 52 
  2 files changed, 62 insertions(+)
  create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-using3.C

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index b6f6f0d02a5..c871a8ab86a 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -2252,6 +2252,16 @@ tsubst_parameter_mapping (tree map, tree args, 
subst_info info)


Hmm, the

>   else if (ARGUMENT_PACK_P (arg))
> new_arg = tsubst_argument_pack (arg, args, complain, in_decl);

just above this seems redundant, since tsubst_template_arg handles packs 
just fine.  In fact, I wonder why tsubst_argument_pack is used 
specifically anywhere?  It seems to get some edge cases better than the 
code in tsubst, but the solution to that would seem to be replacing the 
code in tsubst with a call to tsubst_argument_pack; then we can remove 
all the other calls to the function.



  new_arg = tsubst_template_arg (arg, args, complain, in_decl);
  if (TYPE_P (new_arg))
new_arg = canonicalize_type_argument (new_arg, complain);
+ if (TREE_CODE (new_arg) == TYPE_ARGUMENT_PACK)
+   {
+ tree pack_args = ARGUMENT_PACK_ARGS (new_arg);
+ for (int i = 0; i < TREE_VEC_LENGTH (pack_args); i++)
+   {
+ tree& pack_arg = TREE_VEC_ELT (pack_args, i);
+ if (TYPE_P (pack_arg))
+   pack_arg = canonicalize_type_argument (pack_arg, complain);


Do we need the TYPE_P here, since we already know we're in a 
TYPE_ARGUMENT_PACK?  That is, can an element of a TYPE_ARGUMENT_PACK be 
an invalid argument to canonicalize_type_argument?


OTOH, I wonder if we need to canonicalize non-type arguments here as well?

I wonder if tsubst_template_arg should canonicalize rather than leave 
that up to the caller?  I suppose that could do a bit more work when the 
result is going to end up in convert_template_argument and get 
canonicalized again; I don't know if that would be significant.



}
if (new_arg == error_mark_node)
return error_mark_node;
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-using3.C 
b/gcc/testsuite/g++.dg/cpp2a/concepts-using3.C
new file mode 100644
index 000..2c8ad40d104
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-using3.C
@@ -0,0 +1,52 @@
+// PR c++/93907
+// { dg-options -std=gnu++20 }
+
+// This testcase is a variadic version of concepts-using2.C; the only
+// difference is that 'cd' and 'ce' are now variadic concepts.
+
+template  struct c {
+  static constexpr int d = a;
+  typedef c e;
+};
+template  struct f;
+template  using g = typename f::e;
+struct b;
+template  struct f { using e = b; };
+template  struct m { typedef g aj; };
+template  struct n { typedef typename m::aj e; };
+template  using an = typename n::e;
+template  constexpr bool ao = c::d;
+template  constexpr bool i = c<1>::d;
+template  concept bb = i;
+#ifdef __SIZEOF_INT128__
+using cc = __int128;
+#else
+using cc = long long;
+#endif
+template  concept cd = bb;
+template  concept ce = requires { requires cd; };
+template  concept h = ce;
+template  concept l = h;
+template  concept cl = ao;
+template  concept cp = requires(b j) {
+  requires h>;
+};
+struct o {
+  template  requires cp auto operator()(b) {}
+};
+template  using cm = decltype(o{}(b()));
+template  concept ct = l;
+template  concept dd = ct>;
+template  concept de = dd;
+struct {
+  template  void operator()(da, b);
+} di;
+struct p {
+  void begin();
+};
+template  using df = p;
+template  void q() {
+  df k;
+  int d;
+  di(k, d);
+}





[pushed] Objective-C/C++ : Allow visibility prefix attributes on interfaces.

2020-11-06 Thread Iain Sandoe
Hi,

Some system headers apply visibility attributes to Objective-C
@interface declarations.  Those are “default”, but still need to be
accepted.

tested across the Darwin patch and on x86_64-linux-gnu,
pushed to master,
thanks
Iain

-

This passes visibility through without warning (so that, for example,
__attribute__((__visibility("default"))) does not result in any
diagnostic).

gcc/objc/ChangeLog:

* objc-act.c (start_class): Accept visibility attributes
without warning.
---
 gcc/objc/objc-act.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/gcc/objc/objc-act.c b/gcc/objc/objc-act.c
index 26cdeddfc5a..68d829fd773 100644
--- a/gcc/objc/objc-act.c
+++ b/gcc/objc/objc-act.c
@@ -7013,12 +7013,14 @@ start_class (enum tree_code code, tree class_name, tree 
super_name,
  tree name = TREE_PURPOSE (attribute);
 
  /* TODO: Document what the objc_exception attribute is/does.  */
- /* We handle the 'deprecated' and (undocumented) 'objc_exception'
-attributes.  */
+ /* We handle the 'deprecated', 'visibility' and (undocumented)
+'objc_exception' attributes.  */
  if (is_attribute_p  ("deprecated", name))
TREE_DEPRECATED (klass) = 1;
  else if (is_attribute_p  ("objc_exception", name))
CLASS_HAS_EXCEPTION_ATTR (klass) = 1;
+ else if (is_attribute_p  ("visibility", name))
+   ;
  else
/* Warn about and ignore all others for now, but store them.  */
warning (OPT_Wattributes, "%qE attribute directive ignored", 
name);
-- 
2.24.1




Re: [01/32] langhooks

2020-11-06 Thread Jeff Law via Gcc-patches


On 11/3/20 2:13 PM, Nathan Sidwell wrote:
> I needed a set of hook interfacing the preprocessor to the language.
> they get called from pieces in c-family.
>
> preprocess_main_file:  we need to know when any forced headers have
> been parsed in order to deal with linemaps and macro visibility
>
> preprocess_options: A way for the language to adjust any preprocessor
> options and alter direct callbacks
>
> preprocess_undef: We need visibility of #undefs
>
> preprocess_deferred_macro: macros from header-units are instantiated
> lazily.  This is the hook for the preprocessor to get that done.
>
> preprocess_token: Even in -E processing, we need to observe the token
> stream in order to load up the macro tables of header units.
>
> c-family's c-lex.c, c-opts.c & c-ppoutput.c get to call these hooks in
> various cases

LGTM.

jeff




[pushed] Objective-C/C++ (parsers) : Update @property attribute parsing.

2020-11-06 Thread Iain Sandoe
Hi

This is preparatory work for bringing at least one aspect of the
Objective-C implementation up to date.

Tested on a number of Darwin revisions, and x86_64-linux-gnu
pushed to master,
thanks
Iain

---

At present, we are missing parsing and checking for around
half of the property attributes in use.  The existing ad hoc scheme
for the parser's communication with the Objective C validation
is not suitable for extending to cover all the missing cases.

Additionally:

1/ We were declaring errors in two cases that the reference
   implementation warns (or is silent).

   I've elected to warn for both those cases, (Wattributes) it
   could be that we should implement Wobjc-xxx-property warning
   masks (TODO).

2/ We were emitting spurious complaints about missing property
   attributes when these were not being parsed because we gave
   up on the first syntax error.

3/ The quality of the diagnostic locations was poor (that's
   true for much of Objective-C, we will have to improve it as
   we modernise areas).

We continue to attempt to keep the code, warning and error output
similar (preferably identical output) between the C and C++ front
ends.

The interface to the Objective-C-specific parts of the parsing is
simplified to a vector of parsed (but not fully-checked) property
attributes, this will simplify the addition of new attributes.

gcc/c-family/ChangeLog:

* c-objc.h (enum objc_property_attribute_group): New
(enum objc_property_attribute_kind): New.
(OBJC_PROPATTR_GROUP_MASK): New.
(struct property_attribute_info): Small class encapsulating
parser output from property attributes.
(objc_prop_attr_kind_for_rid): New
(objc_add_property_declaration): Simplify interface.
* stub-objc.c (enum rid): Dummy type.
(objc_add_property_declaration): Simplify interface.
(objc_prop_attr_kind_for_rid): New.

gcc/c/ChangeLog:

* c-parser.c (c_parser_objc_at_property_declaration):
Improve parsing fidelity. Associate better location info
with @property attributes.  Clean up the interface to
objc_add_property_declaration ().

gcc/cp/ChangeLog:

* parser.c (cp_parser_objc_at_property_declaration):
Improve parsing fidelity. Associate better location info
with @property attributes.  Clean up the interface to
objc_add_property_declaration ().

gcc/objc/ChangeLog:

* objc-act.c (objc_prop_attr_kind_for_rid): New.
(objc_add_property_declaration): Adjust to consume the
parser output using a vector of parsed attributes.

gcc/testsuite/ChangeLog:

* obj-c++.dg/property/at-property-1.mm: Adjust expected
diagnostics.
* obj-c++.dg/property/at-property-29.mm: Likewise.
* obj-c++.dg/property/at-property-4.mm: Likewise.
* obj-c++.dg/property/property-neg-2.mm: Likewise.
* objc.dg/property/at-property-1.m: Likewise.
* objc.dg/property/at-property-29.m: Likewise.
* objc.dg/property/at-property-4.m: Likewise.
* objc.dg/property/at-property-5.m: Likewise.
* objc.dg/property/property-neg-2.m: Likewise.
---
 gcc/c-family/c-objc.h |  65 +++-
 gcc/c-family/stub-objc.c  |  21 +-
 gcc/c/c-parser.c  | 280 ++---
 gcc/cp/parser.c   | 266 +---
 gcc/objc/objc-act.c   | 295 +++---
 .../obj-c++.dg/property/at-property-1.mm  |  12 +-
 .../obj-c++.dg/property/at-property-29.mm |   8 +-
 .../obj-c++.dg/property/at-property-4.mm  |  10 +-
 .../obj-c++.dg/property/property-neg-2.mm |   2 +-
 .../objc.dg/property/at-property-1.m  |  12 +-
 .../objc.dg/property/at-property-29.m |   7 +-
 .../objc.dg/property/at-property-4.m  |  10 +-
 .../objc.dg/property/at-property-5.m  |   2 +-
 .../objc.dg/property/property-neg-2.m |   2 +-
 14 files changed, 598 insertions(+), 394 deletions(-)

diff --git a/gcc/c-family/c-objc.h b/gcc/c-family/c-objc.h
index 4577e4f1c7f..a2ca112dcc6 100644
--- a/gcc/c-family/c-objc.h
+++ b/gcc/c-family/c-objc.h
@@ -28,6 +28,67 @@ enum GTY(()) objc_ivar_visibility_kind {
   OBJC_IVAR_VIS_PACKAGE   = 3
 };
 
+/* ObjC property attribute kinds.
+   These have two fields; a unique value (that identifies which attribute)
+   and a group key that indicates membership of an exclusion group.
+   Only one member may be present from an exclusion group in a given attribute
+   list.
+   getters and setters have additional rules, since they are excluded from
+   non-overlapping group sets.  */
+
+enum objc_property_attribute_group
+{
+  OBJC_PROPATTR_GROUP_UNKNOWN = 0,
+  OBJC_PROPATTR_GROUP_GETTER,
+  OBJC_PROPATTR_GROUP_SETTER,
+  OBJC_PROPATTR_GROUP_READWRITE,
+  OBJC_PROPATTR_GROUP_ASSIGN,
+  OBJC_PROPATTR_GROUP_ATOMIC,
+  OBJC_PROPATTR_GROUP_MAX
+};
+
+enum 

[pushed] Darwin: Darwin 20 is to be macOS 11 (Big Sur).

2020-11-06 Thread Iain Sandoe
Hi,

It looks like the next macOS release is imminent.

Tested across the Darwin patch, and on x86_64-linux-gnu,
pushed to master
thanks
Iain

--

As per Nigel Tufnel's assertion "... this one goes to 11".

The various parts of the code that deal with mapping Darwin versions
to macOS (X) versions need updating to deal with  a major version of
11.

So now we have, for example:

Darwin  4 => macOS (X) 10.0
…
Darwin 14 => macOS (X) 10.10
...
Darwin 19 => macOS (X) 10.15

Darwin 20 => macOS  11.0

Because of the historical duplication of the "10" in macOSX 10.xx and
the number of tools that expect this, it is likely that system tools will
allow macos11.0 and/or macosx11.0 (despite that the latter makes little
sense).

Update the link test to cover Catalina (Darwin19/10.15) and
Big Sur (Darwin20/11.0).

gcc/ChangeLog:

* config/darwin-c.c: Allow for Darwin20 to correspond to macOS 11.
* config/darwin-driver.c: Likewise.

gcc/testsuite/ChangeLog:

* gcc.dg/darwin-minversion-link.c: Allow for Darwin19 (macOS 10.15)
and Darwin20 (macOS 11.0).
---
 gcc/config/darwin-c.c |  4 ++--
 gcc/config/darwin-driver.c| 21 ---
 gcc/testsuite/gcc.dg/darwin-minversion-link.c |  5 +++--
 3 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/gcc/config/darwin-c.c b/gcc/config/darwin-c.c
index e3b999e166b..9034f49908e 100644
--- a/gcc/config/darwin-c.c
+++ b/gcc/config/darwin-c.c
@@ -692,10 +692,10 @@ macosx_version_as_macro (void)
   if (!version_array)
 goto fail;
 
-  if (version_array[MAJOR] != 10)
+  if (version_array[MAJOR] < 10 || version_array[MAJOR] > 11)
 goto fail;
 
-  if (version_array[MINOR] < 10)
+  if (version_array[MAJOR] == 10 && version_array[MINOR] < 10)
 version_macro = version_as_legacy_macro (version_array);
   else
 version_macro = version_as_modern_macro (version_array);
diff --git a/gcc/config/darwin-driver.c b/gcc/config/darwin-driver.c
index 8fdd32e2858..8ae300057fd 100644
--- a/gcc/config/darwin-driver.c
+++ b/gcc/config/darwin-driver.c
@@ -65,7 +65,7 @@ validate_macosx_version_min (const char *version_str)
   major = strtoul (version_str, , 10);
   version_str = end + ((*end == '.') ? 1 : 0);
 
-  if (major != 10) /* So far .. all MacOS 10 ... */
+  if (major < 10 || major > 11 ) /* MacOS 10 and 11 are known. */
 return NULL;
 
   /* Version string components must be present and numeric.  */
@@ -104,7 +104,7 @@ validate_macosx_version_min (const char *version_str)
   if (need_rewrite)
 {
   char *new_version;
-  asprintf (_version, "10.%lu.%lu", minor, tiny);
+  asprintf (_version, "%2lu.%lu.%lu", major, minor, tiny);
   return new_version;
 }
 
@@ -115,6 +115,12 @@ validate_macosx_version_min (const char *version_str)
 #include 
 #include "xregex.h"
 
+/* Determine the version of the running OS.
+   We only look at the first two components (ignoring the patch one) and
+   report NN.MM.0 where NN is currently either 10 or 11 and MM is the OS
+   minor release number.
+   If we can't parse what the kernel gives us, warn the user, and do nothing.  
*/
+
 static char *
 darwin_find_version_from_kernel (void)
 {
@@ -125,8 +131,6 @@ darwin_find_version_from_kernel (void)
   char * version_p;
   char * new_flag;
 
-  /* Determine the version of the running OS.  If we can't, warn user,
- and do nothing.  */
   if (sysctl (osversion_name, ARRAY_SIZE (osversion_name), osversion,
  _len, NULL, 0) == -1)
 {
@@ -144,10 +148,11 @@ darwin_find_version_from_kernel (void)
 major_vers = major_vers * 10 + (*version_p++ - '0');
   if (*version_p++ != '.')
 goto parse_failed;
-  
-  /* The major kernel version number is 4 plus the second OS version
- component.  */
-  if (major_vers - 4 <= 4)
+
+  /* Darwin20 sees a transition to macOS 11.  */
+  if (major_vers >= 20)
+asprintf (_flag, "11.%02d.00", major_vers - 20);
+  else if (major_vers - 4 <= 4)
 /* On 10.4 and earlier, the old linker is used which does not
support three-component system versions.
FIXME: we should not assume this - a newer linker could be used.  */
diff --git a/gcc/testsuite/gcc.dg/darwin-minversion-link.c 
b/gcc/testsuite/gcc.dg/darwin-minversion-link.c
index 0a80048ba35..765fb799a91 100644
--- a/gcc/testsuite/gcc.dg/darwin-minversion-link.c
+++ b/gcc/testsuite/gcc.dg/darwin-minversion-link.c
@@ -13,8 +13,9 @@
 /* { dg-additional-options "-mmacosx-version-min=010.011.06 -DCHECK=101106" { 
target *-*-darwin15* } } */
 /* { dg-additional-options "-mmacosx-version-min=010.012.06 -DCHECK=101206" { 
target *-*-darwin16* } } */
 /* { dg-additional-options "-mmacosx-version-min=010.013.06 -DCHECK=101306" { 
target *-*-darwin17* } } */
-/* This next test covers 10.18 and (currently unreleased) 10.19 for now. */  
-/* { dg-additional-options "-mmacosx-version-min=010.014.05 -DCHECK=101405" { 
target *-*-darwin1[89]* } } */
+/* { dg-additional-options 

Re: [PATCH] c++: DR 1914 - Allow duplicate standard attributes.

2020-11-06 Thread Marek Polacek via Gcc-patches
On Fri, Nov 06, 2020 at 02:23:10PM -0500, Jason Merrill via Gcc-patches wrote:
> On 11/6/20 2:06 PM, Marek Polacek wrote:
> > Following Joseph's change for C to allow duplicate C2x standard attributes
> > ,
> > this patch does a similar thing for C++.  This is DR 1914, to be resolved by
> > , which is not part of the standard yet, but has a wide
> > support so look like a shoo-in.  Some duplications still produce warnings;
> > I didn't change that because a warning might be desirable.
> 
> What's the rationale for warning about some and not others?

I don't have any.  Joseph's patch removed the error for a duplicated
'fallthrough' attribute, but the warning remained so I left it as-is
too.

So either we just downgrade the error to a warning, or remove the
remaining warnings too.  I think I slightly prefer the former; with perhaps
a small tweak not to warn when the duplicated attribute comes from a macro
expansion.

Marek



[PATCH] Combine new calculated ranges with existing range.

2020-11-06 Thread Andrew MacLeod via Gcc-patches

This patch fixed PR 97737 and 97741.

basically, when we re-calculate a range, intersect it with whatever we 
knew before so we dont lose any accumulated information.  PR97741 
details the specific case, but I was considering doing this anyway as it 
will allo us to retain any externally accumulated information that is 
known about an SSA_NAME which we would lose right now.


It wasnt really an issue before because the ranger never recalculated a 
range, but with the temporal cache allowing for follow on calculations 
it become more important.


bootstrapped on x86_64-pc-linux-gnu, no regressions. pushed.

Andrew

commit 129e1a8a96d140150705fab30d25afb464eb1d99
Author: Andrew MacLeod 
Date:   Fri Nov 6 14:14:46 2020 -0500

Combine new calculated ranges with existing range.

When a range is recalculated, retain what was previously known as IL changes
can produce different results from un-executed code.   This also paves
the way for external injection of ranges.

gcc/
PR tree-optimization/97737
PR tree-optimization/97741
* gimple-range.cc: (gimple_ranger::range_of_stmt): Intersect newly
calculated ranges with the existing known global range.
gcc/testsuite/
* gcc.dg/pr97737.c: New.
* gcc.dg/pr97741.c: New.

diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
index 0c8ec40448f..92a6335bec5 100644
--- a/gcc/gimple-range.cc
+++ b/gcc/gimple-range.cc
@@ -1026,8 +1026,14 @@ gimple_ranger::range_of_stmt (irange , gimple *s, tree name)
   if (m_cache.get_non_stale_global_range (r, name))
 return true;
 
-  // Otherwise calculate a new value and save it.
-  calc_stmt (r, s, name);
+  // Otherwise calculate a new value.
+  int_range_max tmp;
+  calc_stmt (tmp, s, name);
+
+  // Combine the new value with the old value.  This is required because
+  // the way value propagation works, when the IL changes on the fly we
+  // can sometimes get different results.  See PR 97741.
+  r.intersect (tmp);
   m_cache.set_global_range (name, r);
   return true;
 }
diff --git a/gcc/testsuite/gcc.dg/pr97737.c b/gcc/testsuite/gcc.dg/pr97737.c
new file mode 100644
index 000..eef1c353191
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr97737.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+int a = 1, b, c;
+
+void d() {
+  int e = 1;
+L1:
+  b = e;
+L2:
+  e = e / a;
+  if (!(e || c || e - 1))
+goto L1;
+  if (!b)
+goto L2;
+}
diff --git a/gcc/testsuite/gcc.dg/pr97741.c b/gcc/testsuite/gcc.dg/pr97741.c
new file mode 100644
index 000..47115d31d4a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr97741.c
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-Wall -Wextra -fno-strict-aliasing -fwrapv -Os -fno-toplevel-reorder -fno-tree-ccp -fno-tree-fre" } */
+
+short a = 0;
+long b = 0;
+char c = 0;
+void d() {
+  int e = 0;
+f:
+  for (a = 6; a;)
+c = e;
+  e = 0;
+  for (; e == 20; ++e)
+for (; b;)
+  goto f;
+}
+int main() { return 0; }


Re: [PATCH] c++: Small tweak to can_convert_eh [PR81660]

2020-11-06 Thread Jason Merrill via Gcc-patches

On 11/6/20 1:15 AM, Marek Polacek wrote:

While messing with check_handlers_1, I spotted this bug report which
complains that we don't warn about the case when we have two duplicated
handlers of type int.  can_convert_eh implements [except.handle] and
that says: A handler is a match for an exception object of type E if
  - The handler is of type cv T or cv T& and E and T are the same type
(ignoring the top-level cv-qualifiers), or [...]

but we don't implement this bullet properly for non-class types.  The
fix therefore seems pretty obvious.  Also change the return type to
bool when we're only returning yes/no.


OK.


gcc/cp/ChangeLog:

PR c++/81660
* except.c (can_convert_eh): Change the return type to bool.  If
the type TO and FROM are the same, return true.

gcc/testsuite/ChangeLog:

PR c++/81660
* g++.dg/warn/Wexceptions3.C: New test.
* g++.dg/eh/pr42859.C: Add dg-warning.
* g++.dg/torture/pr81659.C: Likewise.
---
  gcc/cp/except.c  | 14 +++-
  gcc/testsuite/g++.dg/eh/pr42859.C|  2 +-
  gcc/testsuite/g++.dg/torture/pr81659.C   |  2 +-
  gcc/testsuite/g++.dg/warn/Wexceptions3.C | 29 
  4 files changed, 39 insertions(+), 8 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/warn/Wexceptions3.C

diff --git a/gcc/cp/except.c b/gcc/cp/except.c
index b72a28c1aa9..0f6c76b9892 100644
--- a/gcc/cp/except.c
+++ b/gcc/cp/except.c
@@ -41,7 +41,6 @@ static tree do_allocate_exception (tree);
  static tree wrap_cleanups_r (tree *, int *, void *);
  static int complete_ptr_ref_or_void_ptr_p (tree, tree);
  static bool is_admissible_throw_operand_or_catch_parameter (tree, bool);
-static int can_convert_eh (tree, tree);
  
  /* Sets up all the global eh stuff that needs to be initialized at the

 start of compilation.  */
@@ -932,31 +931,34 @@ nothrow_libfn_p (const_tree fn)
  /* Returns nonzero if an exception of type FROM will be caught by a
 handler for type TO, as per [except.handle].  */
  
-static int

+static bool
  can_convert_eh (tree to, tree from)
  {
to = non_reference (to);
from = non_reference (from);
  
+  if (same_type_ignoring_top_level_qualifiers_p (to, from))

+return true;
+
if (TYPE_PTR_P (to) && TYPE_PTR_P (from))
  {
to = TREE_TYPE (to);
from = TREE_TYPE (from);
  
if (! at_least_as_qualified_p (to, from))

-   return 0;
+   return false;
  
if (VOID_TYPE_P (to))

-   return 1;
+   return true;
  
/* Else fall through.  */

  }
  
if (CLASS_TYPE_P (to) && CLASS_TYPE_P (from)

&& publicly_uniquely_derived_p (to, from))
-return 1;
+return true;
  
-  return 0;

+  return false;
  }
  
  /* Check whether any of the handlers in I are shadowed by another handler

diff --git a/gcc/testsuite/g++.dg/eh/pr42859.C 
b/gcc/testsuite/g++.dg/eh/pr42859.C
index a9f1473bc85..0de91409c83 100644
--- a/gcc/testsuite/g++.dg/eh/pr42859.C
+++ b/gcc/testsuite/g++.dg/eh/pr42859.C
@@ -13,7 +13,7 @@ ptw32_terminate (void)
  catch (int)
  {
  }
-catch (int)
+catch (int) // { dg-warning "will be caught by earlier handler" }
  {
  }
}
diff --git a/gcc/testsuite/g++.dg/torture/pr81659.C 
b/gcc/testsuite/g++.dg/torture/pr81659.C
index 3696957532e..074099be6fc 100644
--- a/gcc/testsuite/g++.dg/torture/pr81659.C
+++ b/gcc/testsuite/g++.dg/torture/pr81659.C
@@ -12,7 +12,7 @@ a (int b)
catch (int)
  {
  }
-  catch (int)
+  catch (int) // { dg-warning "will be caught by earlier handler" }
  {
  }
  }
diff --git a/gcc/testsuite/g++.dg/warn/Wexceptions3.C 
b/gcc/testsuite/g++.dg/warn/Wexceptions3.C
new file mode 100644
index 000..97fda9dbd91
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wexceptions3.C
@@ -0,0 +1,29 @@
+// PR c++/81660
+
+void bar (int);
+
+void
+fn (int b)
+{
+  if (b)
+throw;
+  try
+{
+  bar (3);
+}
+  catch (int)
+{
+}
+  catch (int) // { dg-warning "will be caught by earlier handler" }
+{
+}
+  catch (const int) // { dg-warning "will be caught by earlier handler" }
+{
+}
+  catch (int &) // { dg-warning "will be caught by earlier handler" }
+{
+}
+  catch (const int &) // { dg-warning "will be caught by earlier handler" }
+{
+}
+}

base-commit: f72af3af8d526793e4927daf44ae0611c3d0cc85





Re: [PATCH] c++: Propagate attributes to clones in duplicate_decls [PR67453]

2020-11-06 Thread Jason Merrill via Gcc-patches

On 11/6/20 4:07 AM, Jakub Jelinek wrote:

Hi!

On the following testcase where the cdtor attributes aren't on the
in-class declaration but on an out-of-class definition, the cdtors
have their clones created from the in-class declaration, and later on
duplicate_decls updates attributes on the abstract cdtors, but nothing
propagates them to the clones.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?


OK.


2020-11-06  Jakub Jelinek  

PR c++/67453
* decl.c (duplicate_decls): Propagate DECL_ATTRIBUTES and
DECL_PRESERVE_P from olddecl to its clones if any.

* g++.dg/ext/attr-used-2.C: New test.

--- gcc/cp/decl.c.jj2020-11-03 21:42:00.536043737 +0100
+++ gcc/cp/decl.c   2020-11-05 17:33:40.064072970 +0100
@@ -2921,6 +2921,16 @@ duplicate_decls (tree newdecl, tree oldd
snode->remove ();
  }
  
+  if (TREE_CODE (olddecl) == FUNCTION_DECL)

+{
+  tree clone;
+  FOR_EACH_CLONE (clone, olddecl)
+   {
+ DECL_ATTRIBUTES (clone) = DECL_ATTRIBUTES (olddecl);
+ DECL_PRESERVE_P (clone) |= DECL_PRESERVE_P (olddecl);
+   }
+}
+
/* Remove the associated constraints for newdecl, if any, before
   reclaiming memory. */
if (flag_concepts)
--- gcc/testsuite/g++.dg/ext/attr-used-2.C.jj   2020-11-05 17:42:49.895949119 
+0100
+++ gcc/testsuite/g++.dg/ext/attr-used-2.C  2020-11-05 17:42:07.934416482 
+0100
@@ -0,0 +1,15 @@
+// PR c++/67453
+// { dg-do compile }
+// { dg-final { scan-assembler "_ZN1SC\[12]Ev" } }
+// { dg-final { scan-assembler "_ZN1SD\[12]Ev" } }
+// { dg-final { scan-assembler "_ZN1SC\[12]ERKS_" } }
+
+struct S {
+S();
+~S();
+S(const S&);
+};
+
+__attribute__((used)) inline S::S()  { }
+__attribute__((used)) inline S::~S() { }
+__attribute__((used)) inline S::S(const S&) { }

Jakub





Re: [PATCH] c++: DR 1914 - Allow duplicate standard attributes.

2020-11-06 Thread Jason Merrill via Gcc-patches

On 11/6/20 2:06 PM, Marek Polacek wrote:

Following Joseph's change for C to allow duplicate C2x standard attributes
,
this patch does a similar thing for C++.  This is DR 1914, to be resolved by
, which is not part of the standard yet, but has a wide
support so look like a shoo-in.  Some duplications still produce warnings;
I didn't change that because a warning might be desirable.


What's the rationale for warning about some and not others?


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

gcc/c-family/ChangeLog:

DR 1914
* c-common.c (attribute_fallthrough_p): Update comment.

gcc/cp/ChangeLog:

DR 1914
* parser.c (cp_parser_check_std_attribute): Remove.
(cp_parser_std_attribute_list): Don't call it.

gcc/testsuite/ChangeLog:

DR 1914
* g++.dg/cpp0x/gen-attrs-60.C: Remove dg-error.
* g++.dg/cpp2a/nodiscard-once.C: Likewise.
* g++.dg/cpp1y/attr-deprecated-2.C: Likewise.
* g++.dg/cpp0x/gen-attrs-72.C: New test.
---
  gcc/c-family/c-common.c   |  3 +-
  gcc/cp/parser.c   | 27 
  gcc/testsuite/g++.dg/cpp0x/gen-attrs-60.C |  2 +-
  gcc/testsuite/g++.dg/cpp0x/gen-attrs-72.C | 42 +++
  .../g++.dg/cpp1y/attr-deprecated-2.C  |  2 +-
  gcc/testsuite/g++.dg/cpp2a/nodiscard-once.C   |  2 +-
  6 files changed, 47 insertions(+), 31 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp0x/gen-attrs-72.C

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 232a4797c09..03d8fec9936 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -5751,7 +5751,8 @@ attribute_fallthrough_p (tree attr)
tree t = lookup_attribute ("fallthrough", attr);
if (t == NULL_TREE)
  return false;
-  /* This attribute shall appear at most once in each attribute-list.  */
+  /* It is no longer true that "this attribute shall appear at most once in
+ each attribute-list", but we still give a warning.  */
if (lookup_attribute ("fallthrough", TREE_CHAIN (t)))
  warning (OPT_Wattributes, "% attribute specified multiple "
 "times");
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 6e7b982f073..0567646fbe2 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -27269,32 +27269,6 @@ cp_parser_std_attribute (cp_parser *parser, tree 
attr_ns)
return attribute;
  }
  
-/* Check that the attribute ATTRIBUTE appears at most once in the

-   attribute-list ATTRIBUTES.  This is enforced for noreturn (7.6.3),
-   nodiscard, and deprecated (7.6.5).  Note that
-   carries_dependency (7.6.4) isn't implemented yet in GCC.  */
-
-static void
-cp_parser_check_std_attribute (tree attributes, tree attribute)
-{
-  if (attributes)
-{
-  tree name = get_attribute_name (attribute);
-  if (is_attribute_p ("noreturn", name)
- && lookup_attribute ("noreturn", attributes))
-   error ("attribute % can appear at most once "
-  "in an attribute-list");
-  else if (is_attribute_p ("deprecated", name)
-  && lookup_attribute ("deprecated", attributes))
-   error ("attribute % can appear at most once "
-  "in an attribute-list");
-  else if (is_attribute_p ("nodiscard", name)
-  && lookup_attribute ("nodiscard", attributes))
-   error ("attribute % can appear at most once "
-  "in an attribute-list");
-}
-}
-
  /* Parse a list of standard C++-11 attributes.
  
 attribute-list:

@@ -27317,7 +27291,6 @@ cp_parser_std_attribute_list (cp_parser *parser, tree 
attr_ns)
break;
if (attribute != NULL_TREE)
{
- cp_parser_check_std_attribute (attributes, attribute);
  TREE_CHAIN (attribute) = attributes;
  attributes = attribute;
}
diff --git a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-60.C 
b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-60.C
index cb0c31ec63f..4e905c394ca 100644
--- a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-60.C
+++ b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-60.C
@@ -1,4 +1,4 @@
  // PR c++/60365
  // { dg-do compile { target c++11 } }
  
-void func [[noreturn, noreturn]] (); // { dg-error "at most once" }

+void func [[noreturn, noreturn]] ();
diff --git a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-72.C 
b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-72.C
new file mode 100644
index 000..0cb874b6191
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-72.C
@@ -0,0 +1,42 @@
+// DR 1914 - Duplicate standard attributes
+// { dg-do compile { target c++11 } }
+
+[[noreturn, noreturn]] void fn0();
+[[noreturn]] [[noreturn]] void fn1();
+[[deprecated, deprecated]] void fn2();
+[[deprecated]] [[deprecated]] void fn3();
+[[maybe_unused]] [[maybe_unused]] int fn4();
+[[maybe_unused, maybe_unused]] int fn5();
+[[nodiscard]] [[nodiscard]] int fn6();
+[[nodiscard, nodiscard]] int fn7();
+
+struct E { };
+struct A 

[PATCH] c++: DR 1914 - Allow duplicate standard attributes.

2020-11-06 Thread Marek Polacek via Gcc-patches
Following Joseph's change for C to allow duplicate C2x standard attributes
,
this patch does a similar thing for C++.  This is DR 1914, to be resolved by
, which is not part of the standard yet, but has a wide
support so look like a shoo-in.  Some duplications still produce warnings;
I didn't change that because a warning might be desirable.

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

gcc/c-family/ChangeLog:

DR 1914
* c-common.c (attribute_fallthrough_p): Update comment.

gcc/cp/ChangeLog:

DR 1914
* parser.c (cp_parser_check_std_attribute): Remove.
(cp_parser_std_attribute_list): Don't call it.

gcc/testsuite/ChangeLog:

DR 1914
* g++.dg/cpp0x/gen-attrs-60.C: Remove dg-error.
* g++.dg/cpp2a/nodiscard-once.C: Likewise.
* g++.dg/cpp1y/attr-deprecated-2.C: Likewise.
* g++.dg/cpp0x/gen-attrs-72.C: New test.
---
 gcc/c-family/c-common.c   |  3 +-
 gcc/cp/parser.c   | 27 
 gcc/testsuite/g++.dg/cpp0x/gen-attrs-60.C |  2 +-
 gcc/testsuite/g++.dg/cpp0x/gen-attrs-72.C | 42 +++
 .../g++.dg/cpp1y/attr-deprecated-2.C  |  2 +-
 gcc/testsuite/g++.dg/cpp2a/nodiscard-once.C   |  2 +-
 6 files changed, 47 insertions(+), 31 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/gen-attrs-72.C

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 232a4797c09..03d8fec9936 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -5751,7 +5751,8 @@ attribute_fallthrough_p (tree attr)
   tree t = lookup_attribute ("fallthrough", attr);
   if (t == NULL_TREE)
 return false;
-  /* This attribute shall appear at most once in each attribute-list.  */
+  /* It is no longer true that "this attribute shall appear at most once in
+ each attribute-list", but we still give a warning.  */
   if (lookup_attribute ("fallthrough", TREE_CHAIN (t)))
 warning (OPT_Wattributes, "% attribute specified multiple "
 "times");
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 6e7b982f073..0567646fbe2 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -27269,32 +27269,6 @@ cp_parser_std_attribute (cp_parser *parser, tree 
attr_ns)
   return attribute;
 }
 
-/* Check that the attribute ATTRIBUTE appears at most once in the
-   attribute-list ATTRIBUTES.  This is enforced for noreturn (7.6.3),
-   nodiscard, and deprecated (7.6.5).  Note that
-   carries_dependency (7.6.4) isn't implemented yet in GCC.  */
-
-static void
-cp_parser_check_std_attribute (tree attributes, tree attribute)
-{
-  if (attributes)
-{
-  tree name = get_attribute_name (attribute);
-  if (is_attribute_p ("noreturn", name)
- && lookup_attribute ("noreturn", attributes))
-   error ("attribute % can appear at most once "
-  "in an attribute-list");
-  else if (is_attribute_p ("deprecated", name)
-  && lookup_attribute ("deprecated", attributes))
-   error ("attribute % can appear at most once "
-  "in an attribute-list");
-  else if (is_attribute_p ("nodiscard", name)
-  && lookup_attribute ("nodiscard", attributes))
-   error ("attribute % can appear at most once "
-  "in an attribute-list");
-}
-}
-
 /* Parse a list of standard C++-11 attributes.
 
attribute-list:
@@ -27317,7 +27291,6 @@ cp_parser_std_attribute_list (cp_parser *parser, tree 
attr_ns)
break;
   if (attribute != NULL_TREE)
{
- cp_parser_check_std_attribute (attributes, attribute);
  TREE_CHAIN (attribute) = attributes;
  attributes = attribute;
}
diff --git a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-60.C 
b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-60.C
index cb0c31ec63f..4e905c394ca 100644
--- a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-60.C
+++ b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-60.C
@@ -1,4 +1,4 @@
 // PR c++/60365
 // { dg-do compile { target c++11 } }
 
-void func [[noreturn, noreturn]] (); // { dg-error "at most once" }
+void func [[noreturn, noreturn]] ();
diff --git a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-72.C 
b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-72.C
new file mode 100644
index 000..0cb874b6191
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-72.C
@@ -0,0 +1,42 @@
+// DR 1914 - Duplicate standard attributes 
+// { dg-do compile { target c++11 } }
+
+[[noreturn, noreturn]] void fn0();
+[[noreturn]] [[noreturn]] void fn1();
+[[deprecated, deprecated]] void fn2();
+[[deprecated]] [[deprecated]] void fn3();
+[[maybe_unused]] [[maybe_unused]] int fn4();
+[[maybe_unused, maybe_unused]] int fn5();
+[[nodiscard]] [[nodiscard]] int fn6();
+[[nodiscard, nodiscard]] int fn7();
+
+struct E { };
+struct A {
+  [[no_unique_address]] [[no_unique_address]] E e;
+};
+struct B {
+  [[no_unique_address, no_unique_address]] E e;
+};
+
+int

Re: [PATCH] RX add control register PC

2020-11-06 Thread Jeff Law via Gcc-patches


On 8/26/20 4:24 AM, Darius Galis wrote:
> Hello,
> Thank you for adjusting the patch.
> I don't have commit privileges, so if you could please commit it, that would 
> be great.

Sorry it took so long.  I got pulled away for most of the last few months.


It's committed to the trunk.


jeff




[r11-4770 Regression] FAIL: gcc.dg/ipa/modref-2.c scan-ipa-dump modref "Parm 1 param offset:0 offset:0 size:-1 max_size:64" on Linux/x86_64

2020-11-06 Thread sunil.k.pandey via Gcc-patches
On Linux/x86_64,

6cef01c32817b3d08af2cadcdb0e23c72ceed426 is the first bad commit
commit 6cef01c32817b3d08af2cadcdb0e23c72ceed426
Author: Jan Hubicka 
Date:   Fri Nov 6 10:23:58 2020 +0100

Add fnspec handling to ipa mode of ipa-modef.

caused

FAIL: gcc.dg/ipa/modref-2.c scan-ipa-dump modref "Parm 1 param offset:0 
offset:0 size:-1 max_size:64"

with GCC configured with

../../gcc/configure 
--prefix=/local/skpandey/gccwork/toolwork/gcc-bisect-master/master/r11-4770/usr 
--enable-clocale=gnu --with-system-zlib --with-demangler-in-ld 
--with-fpmath=sse --enable-languages=c,c++,fortran --enable-cet --without-isl 
--enable-libmpx x86_64-linux --disable-bootstrap

To reproduce:

$ cd {build_dir}/gcc && make check RUNTESTFLAGS="ipa.exp=gcc.dg/ipa/modref-2.c 
--target_board='unix{-m32}'"
$ cd {build_dir}/gcc && make check RUNTESTFLAGS="ipa.exp=gcc.dg/ipa/modref-2.c 
--target_board='unix{-m32\ -march=cascadelake}'"

(Please do not reply to this email, for question about this report, contact me 
at skpgkp2 at gmail dot com)


Re: [10/32] config

2020-11-06 Thread Nathan Sidwell

On 11/6/20 12:56 PM, Jeff Law wrote:


On 11/4/20 12:24 PM, Nathan Sidwell wrote:

I managed to flub sending this yesterday.

This is the gcc/configure.ac changes (rebuild configure and
config.h.in after applying).  Generally just checking for
network-related functionality.  If it's not available, those features
of the module mapper will be unavailable.


OK with a ChangeLog entry.


thanks.  I should have mentioned the lack of changelogs on the patches. 
I will of course write them when committing, along with rationale that 
may have already been written in the emails


nathan

--
Nathan Sidwell


Re: [patch] Add dg-require-effective-target fpic to an aarch64 specific test in gcc.dg

2020-11-06 Thread Olivier Hainque



> On 6 Nov 2020, at 19:03, Richard Sandiford  wrote:

>>> OK, thanks.  Also OK for any other current or future aarch64 test that
>>> has -fpic or -fPIC in the options and forgets to do this.

>> For the avoidance of doubt, that would hold for arm
>> tests as well, right ?
> 
> Yeah, it does.
> 
> Thanks,

Sure, thanks for confirming.

Cheers,

Olivier



Re: [patch] Add dg-require-effective-target fpic to an aarch64 specific test in gcc.dg

2020-11-06 Thread Richard Sandiford via Gcc-patches
Olivier Hainque  writes:
>> On 4 Nov 2020, at 20:16, Richard Sandiford  wrote:
>> 
>> Olivier Hainque  writes:
>>> Hello,
>>> 
>>> This patch adds dg-require-effective-target fpic
>>> to an aarch64 specific gcc.dg test using -fPIC,
>>> which helps circumvent a failure we observed while
>>> testing the aarch64 port for VxWorks.
>>> 
>>> ok to commit ?
>> 
>> OK, thanks.  Also OK for any other current or future aarch64 test that
>> has -fpic or -fPIC in the options and forgets to do this.
>
> Great, thanks Richard!
>
> This echoes what you had actually already told me some
> months ago at
>
>   https://gcc.gnu.org/pipermail/gcc-patches/2019-December/536909.html
>
> which I didn't remember before sending this patch.
>
> For the avoidance of doubt, that would hold for arm
> tests as well, right ?

Yeah, it does.

Thanks,
Richard

> (I'll be careful not to introduce obvious redundancy with e.g.
> os=linux, as pointed out by Jakub earlier this week for another
> set on i386).
>
> Cheers,
>
> Olivier


Re: [10/32] config

2020-11-06 Thread Jeff Law via Gcc-patches


On 11/4/20 12:24 PM, Nathan Sidwell wrote:
> I managed to flub sending this yesterday.
>
> This is the gcc/configure.ac changes (rebuild configure and
> config.h.in after applying).  Generally just checking for
> network-related functionality.  If it's not available, those features
> of the module mapper will be unavailable.

OK with a ChangeLog entry.


jeff

ps.  Yes, I know this is out of order WRT the others in the series. 
It's not being threaded with the main patch series, so it stood out in
my queue :-)





Re: [PATCH, rs6000] Update instruction attributes for Power10

2020-11-06 Thread will schmidt via Gcc-patches
On Fri, 2020-11-06 at 10:46 -0600, Pat Haugen wrote:
> On 11/5/20 4:32 PM, will schmidt wrote:
> > On Wed, 2020-11-04 at 14:42 -0600, Pat Haugen via Gcc-patches
> > wrote:
> > >   * config/rs6000/rs6000.c (rs6000_final_prescan_insn): Only add
> > > 'p' for
> > >   PREFIXED_YES.
> > 
> > The code change reads as roughly 
> > - next_insn_prefixed_p != PREFIXED_NO
> > 
> > + next_insn_prefixed_p == PREFIXED_YES"
> > 
> > So just an inversion of the logic? I don't obviously see the 'p'
> > impact
> > there.
> > 
> 
> It's no longer an inversion of the logic since I added a
> PREFIXED_ALWAYS value. 'next_insn_prefixed' is used by
> rs6000_final_prescan_insn() to determine whether an insn mnemonic
> needs a 'p' prefix. We want it set for PREFIXED_YES, but not for
> PREFIXED_NO or PREFIXED_ALWAYS.

Ok.  So the next_insn_prefixed_p indicates whether the instruction
has/gets/needs a p prefix.  gotcha.  thanks for clarifying.  :-)

thanks
-will


> 
> > 
> > >   * config/rs6000/rs6000.md (define_attr "size"): Add 256.
> > >   (define_attr "prefixed"): Add 'always'.
> > >   (define_mode_attr bits): Add DD/TD modes.
> > >   (cfuged, cntlzdm, cnttzdm, pdepd, pextd, bswaphi2_reg,
> > > bswapsi2_reg,
> > >   bswapdi2_brd, setbc_signed_,
> > >   *setbcr_signed_, *setnbc_signed_,
> > >   *setnbcr_signed_): Update instruction attributes
> > > for
> > >   Power10.
> > 
> > ok.  (assuming the assorted 'integer' -> 'crypto' changes are
> > correct,
> > of course).  
> > 
> 
> Yes, crypto represents the correct pipe the insns are executed on.
> 
> Thanks for the review,
> Pat
> 



Re: [PATCH][AArch64] Use intrinsics for upper saturating shift right

2020-11-06 Thread Richard Sandiford via Gcc-patches
David Candler  writes:
> Hi Richard,
>
> Thanks for the feedback.
>
> Richard Sandiford  writes:
>> > diff --git a/gcc/config/aarch64/aarch64-builtins.c 
>> > b/gcc/config/aarch64/aarch64-builtins.c
>> > index 4f33dd936c7..f93f4e29c89 100644
>> > --- a/gcc/config/aarch64/aarch64-builtins.c
>> > +++ b/gcc/config/aarch64/aarch64-builtins.c
>> > @@ -254,6 +254,10 @@ 
>> > aarch64_types_binop_imm_qualifiers[SIMD_MAX_BUILTIN_ARGS]
>> >  #define TYPES_GETREG (aarch64_types_binop_imm_qualifiers)
>> >  #define TYPES_SHIFTIMM (aarch64_types_binop_imm_qualifiers)
>> >  static enum aarch64_type_qualifiers
>> > +aarch64_types_ternop_s_imm_qualifiers[SIMD_MAX_BUILTIN_ARGS]
>> > +  = { qualifier_none, qualifier_none, qualifier_none, 
>> > qualifier_immediate};
>> > +#define TYPES_SHIFT2IMM (aarch64_types_ternop_s_imm_qualifiers)
>> > +static enum aarch64_type_qualifiers
>> >  aarch64_types_shift_to_unsigned_qualifiers[SIMD_MAX_BUILTIN_ARGS]
>> >= { qualifier_unsigned, qualifier_none, qualifier_immediate };
>> >  #define TYPES_SHIFTIMM_USS (aarch64_types_shift_to_unsigned_qualifiers)
>> > @@ -265,14 +269,16 @@ static enum aarch64_type_qualifiers
>> >  aarch64_types_unsigned_shift_qualifiers[SIMD_MAX_BUILTIN_ARGS]
>> >= { qualifier_unsigned, qualifier_unsigned, qualifier_immediate };
>> >  #define TYPES_USHIFTIMM (aarch64_types_unsigned_shift_qualifiers)
>> > +#define TYPES_USHIFT2IMM (aarch64_types_ternopu_imm_qualifiers)
>> > +static enum aarch64_type_qualifiers
>> > +aarch64_types_shift2_to_unsigned_qualifiers[SIMD_MAX_BUILTIN_ARGS]
>> > +  = { qualifier_unsigned, qualifier_unsigned, qualifier_none, 
>> > qualifier_immediate };
>> > +#define TYPES_SHIFT2IMM_UUSS (aarch64_types_shift2_to_unsigned_qualifiers)
>> >
>> >  static enum aarch64_type_qualifiers
>> >  aarch64_types_ternop_s_imm_p_qualifiers[SIMD_MAX_BUILTIN_ARGS]
>> >= { qualifier_none, qualifier_none, qualifier_poly, 
>> > qualifier_immediate};
>> >  #define TYPES_SETREGP (aarch64_types_ternop_s_imm_p_qualifiers)
>> > -static enum aarch64_type_qualifiers
>> > -aarch64_types_ternop_s_imm_qualifiers[SIMD_MAX_BUILTIN_ARGS]
>> > -  = { qualifier_none, qualifier_none, qualifier_none, 
>> > qualifier_immediate};
>> >  #define TYPES_SETREG (aarch64_types_ternop_s_imm_qualifiers)
>> >  #define TYPES_SHIFTINSERT (aarch64_types_ternop_s_imm_qualifiers)
>> >  #define TYPES_SHIFTACC (aarch64_types_ternop_s_imm_qualifiers)
>>
>> Very minor, but I think it would be better to keep
>> aarch64_types_ternop_s_imm_qualifiers where it is and define
>> TYPES_SHIFT2IMM here rather than above.  For better or worse,
>> the current style seems to be to keep the defines next to the
>> associated arrays, rather than group them based on the TYPES_* name.
>>
>> > diff --git a/gcc/config/aarch64/aarch64-simd-builtins.def 
>> > b/gcc/config/aarch64/aarch64-simd-builtins.def
>> > index d1b21102b2f..0b82b9c072b 100644
>> > --- a/gcc/config/aarch64/aarch64-simd-builtins.def
>> > +++ b/gcc/config/aarch64/aarch64-simd-builtins.def
>> > @@ -285,6 +285,13 @@
>> >BUILTIN_VSQN_HSDI (USHIFTIMM, uqshrn_n, 0, ALL)
>> >BUILTIN_VSQN_HSDI (SHIFTIMM, sqrshrn_n, 0, ALL)
>> >BUILTIN_VSQN_HSDI (USHIFTIMM, uqrshrn_n, 0, ALL)
>> > +  /* Implemented by aarch64_qshrn2_n.  */
>> > +  BUILTIN_VQN (SHIFT2IMM_UUSS, sqshrun2_n, 0, ALL)
>> > +  BUILTIN_VQN (SHIFT2IMM_UUSS, sqrshrun2_n, 0, ALL)
>> > +  BUILTIN_VQN (SHIFT2IMM, sqshrn2_n, 0, ALL)
>> > +  BUILTIN_VQN (USHIFT2IMM, uqshrn2_n, 0, ALL)
>> > +  BUILTIN_VQN (SHIFT2IMM, sqrshrn2_n, 0, ALL)
>> > +  BUILTIN_VQN (USHIFT2IMM, uqrshrn2_n, 0, ALL)
>>
>> Using ALL is a holdover from the time (until a few weeks ago) when we
>> didn't record function attributes.  New intrinsics should therefore
>> have something more specific than ALL.
>>
>> We discussed offline whether the Q flag side effect of the intrinsics
>> should be observable or not, and the conclusion was that it shouldn't.
>> I think we can therefore treat these functions as pure functions,
>> meaning that they should have flags NONE rather than ALL.
>>
>> For that reason, I think we should also remove the Set_Neon_Cumulative_Sat
>> and CHECK_CUMULATIVE_SAT parts of the test (sorry).
>>
>> Other than that, the patch looks good to go.
>>
>> Thanks,
>> Richard
>
> I've updated the patch with TYPES_SHIFT2IMM moved, the builtins changed
> to NONE, and the Q flag portion of the tests removed.

Looks good to me, thanks.  Pushed to trunk.

Richard


Re: float.h: C2x decimal signaling NaN macros

2020-11-06 Thread Jeff Law via Gcc-patches


On 11/5/20 4:21 PM, Joseph Myers wrote:
> C2x adds macros for decimal floating-point signaling NaNs to
> .  Add these macros to GCC's  implementation.
>
> Note that the current C2x draft has these under incorrect names
> D32_SNAN, D64_SNAN, D128_SNAN.  The intent was to change the naming
> convention to be consistent with other  macros when they were
> moved to , so DEC32_SNAN, DEC64_SNAN, DEC128_NAN, which this
> patch uses (as does the current draft integration of TS 18661-3 as an
> Annex to C2x, for its _Decimal* and _Decimal*x types).
>
> This patch is relative to a tree with
> 
> and
> 
> (both pending review) applied.
>
> Bootstrapped with no regressions for x86_64-pc-linux-gnu.  OK to commit?
>
> gcc/
> 2020-11-05  Joseph Myers  
>
>   * ginclude/float.h (DEC32_SNAN, DEC64_SNAN, DEC128_SNAN): New C2x
>   macros.
>
> gcc/testsuite/
> 2020-11-05  Joseph Myers  
>
>   * gcc.dg/dfp/c2x-float-dfp-7.c, gcc.dg/dfp/c2x-float-dfp-8.c: New
>   tests.
>   * gcc.dg/c2x-float-no-dfp-3.c: Also check that DEC32_SNAN,
>   DEC64_SNAN and DEC128_SNAN are not defined.

OK

jeff




Re: builtins: Add DFP signaling NaN built-in functions

2020-11-06 Thread Jeff Law via Gcc-patches


On 11/4/20 5:35 PM, Joseph Myers wrote:
> Add built-in functions __builtin_nansd32, __builtin_nansd64 and
> __builtin_nansd128 to return signaling NaNs of decimal floating-point
> types, analogous to the functions already present for binary
> floating-point types.
>
> This patch, independent of
> 
> (pending review), is in preparation for adding the  macros
> for such signaling NaNs that are in C2x, analogous to the macros for
> other types that are in that patch.
>
> Bootstrapped with no regressions for x86_64-pc-linux-gnu.  Also ran
> the new tests for powerpc64le-linux-gnu to confirm they do work in the
> case (hardware DFP) where floating-point exceptions are supported for
> DFP.  OK to commit?
>
> gcc/
> 2020-11-05  Joseph Myers  
>
>   * builtins.def (BUILT_IN_NANSD32, BUILT_IN_NANSD64)
>   (BUILT_IN_NANSD128): New built-in functions.
>   * fold-const-call.c (fold_const_call): Handle the new built-in
>   functions.
>   * doc/extend.texi (__builtin_nansd32, __builtin_nansd64)
>   (__builtin_nansd128): Document.
>   * doc/sourcebuild.texi (Effective-Target Keywords): Document
>   fenv_exceptions_dfp.
>
> gcc/testsuite/
> 2020-11-05  Joseph Myers  
>
>   * lib/target-supports.exp
>   (check_effective_target_fenv_exceptions_dfp): New.
>   * gcc.dg/dfp/builtin-snan-1.c, gcc.dg/dfp/builtin-snan-2.c: New
>   tests.

OK

jeff




Re: [Patch] testsuite: Avoid TCL errors when rootme or ASAN/TSAN/UBSAN is not available (was: Re: [Patch] testsuite: Avoid TCL errors when ASAN/TSAN/UBSAN is not available)

2020-11-06 Thread Jeff Law via Gcc-patches


On 10/19/20 10:03 AM, Tobias Burnus wrote:
> Thomas Schwinge and Joseph convinced me that 'rootme' only makes sense
> for in-tree testing and, hence, does not need (or: should not) be set in
> site.exp.
>
> Thus, if it is not set, we have to check its existence before using it –
> to avoid similar TCL errors.
> Hence, I updated the patch to check also for 'rootme'.
>
> OK?
>
> Tobias
>
> On 10/19/20 11:46 AM, Tobias Burnus wrote:
>> In a --disable-libsanitizer build, I see errors such as:
>>   g++.sum:ERROR: can't read "asan_saved_library_path": no such variable
>>
>> I believe the following patch is the right way to solve this.
>> OK?
>>
>> Tobias
>>
> -
> Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München /
> Germany
> Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung,
> Alexander Walter
>
> san-test-v2.diff
>
> testsuite: Avoid TCL errors when rootme or ASAN/TSAN/UBSAN is not avail
>
>   * g++.dg/guality/guality.exp:
>   * gcc.dg/guality/guality.exp:
>   * gfortran.dg/guality/guality.exp:
>   * lib/asan-dg.exp:
>   * lib/tsan-dg.exp:
>   * lib/ubsan-dg.exp:
>
>  gcc/testsuite/g++.dg/guality/guality.exp  | 2 +-
>  gcc/testsuite/gcc.dg/guality/guality.exp  | 2 +-
>  gcc/testsuite/gfortran.dg/guality/guality.exp | 2 +-
>  gcc/testsuite/lib/asan-dg.exp | 6 --
>  gcc/testsuite/lib/tsan-dg.exp | 6 --
>  gcc/testsuite/lib/ubsan-dg.exp| 6 --
>  6 files changed, 15 insertions(+), 9 deletions(-)

OK with ChangeLog entry completed.


jeff




Re: [Patch, fortran] PR83118 - [8/9/10/11 Regression] Bad intrinsic assignment of class(*) array component of derived type

2020-11-06 Thread Tobias Burnus

Hi Paul,

sorry for the belated attempt to review your patch.
Attempt as both via @gcc.gnu.org as in the direct email,
I did not see the attached patch.
(Matches what Andre mentioned to you today at IRC #gfortran.)

I only see:
* Content-Type: multipart/alternative
  Content-Type: text/plain; charset="UTF-8"
* Content-Type: text/html; charset="UTF-8"
(Side remark: I did not know that GCC now accepts
 text/html multipart emails. Still, it is better to
 avoid this.)

* Content-Type: application/octet-stream; name="Change2.Logs"
* Content-Type: application/octet-stream; name="unlimited_polymorphic_32.f03

Regarding the changelog: There first line got he git commit is missing.
That should be a single (not too long) line, which "git log --oneline"
shows, followed by an empty line.
If possible, it is a substring of the email subject (if that makes sense).
For this thread, it helps if "PR83118" is present in that line, which is
also in the thread name.

Additionally spotted:

"Reallocation of lhs only to happen if siz changes"
Typo "siz"? Or is this a badly named variable name?

Tobias

-
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander 
Walter


Re: [PATCH] c-family: Fix regression in location-overflow-test-1.c [PR97117]

2020-11-06 Thread Jeff Law via Gcc-patches


On 10/14/20 9:56 AM, Patrick Palka via Gcc-patches wrote:
> The r11-3266 patch that added macro support to -Wmisleading-indentation
> accidentally suppressed the column-tracking diagnostic in
> get_visual_column in some cases, e.g. in the location-overflow-test-1.c
> testcase.
>
> More generally, when all three tokens are on the same line and we've run
> out of locations with column info, then their location_t values will be
> equal, and we exit early from should_warn_for_misleading_indentation due
> to the new check
>
>   /* Give up if the loci are not all distinct.  */
>   if (guard_loc == body_loc || body_loc == next_stmt_loc)
> return false;
>
> before we ever call get_visual_column.
>
> [ This new check is needed to detect and give up on analyzing code
>   fragments where exactly two out of the three tokens come from the same
>   macro expansion, e.g.
>
> #define MACRO \
>   if (a)  \
> foo ();
>
> MACRO; bar ();
>
>   Here, guard_loc and body_loc will be equal and point to the macro
>   expansion point.  The heuristics the warning uses are not really valid
>   in scenarios like these.  ]
>
> In order to restore the column-tracking diagnostic, this patch moves the
> the diagnostic code out from get_visual_column to earlier in
> should_warn_for_misleading_indentation.  Moreover, it tests the three
> location_t values for a zero column all at once, which I suppose should
> make us issue the diagnostic more consistently.
>
> Tested on x86_64-pc-linux-gnu, does this look OK to commit?
>
> gcc/c-family/ChangeLog:
>
>   PR testsuite/97117
>   * c-indentation.c (get_visual_column): Remove location_t
>   parameter.  Move the column-tracking diagnostic code from here
>   to ...
>   (should_warn_for_misleading_indentation): ... here, before the
>   early exit for when the loci are not all distinct.  Don't pass a
>   location_t argument to get_visual_column.
>   (assert_get_visual_column_succeeds): Don't pass a location_t
>   argument to get_visual_column.
>   (assert_get_visual_column_fails): Likewise.

OK.


jeff




Re: [PATCH] libcpp: Update cpp_wcwidth() to Unicode 13.0.0

2020-11-06 Thread Jeff Law via Gcc-patches


On 10/23/20 9:01 AM, Lewis Hyatt via Gcc-patches wrote:
> Hello-
>
> The attached patch updates cpp_wcwidth() (for computation of display
> widths needed to calculate column numbers in diagnostics) from Unicode 12
> to Unicode 13. The patch was purely mechanical, following the directions
> in contrib/unicode/README without any unexpected hiccups. A couple
> questions please:
>
> -Is it OK for master?

Yes, it is OK for the trunk.  Please go ahead and commit it.


>
> -Unicode 13 actually came out just immediately before GCC 10 was
>  released. Would it make sense to put this on GCC 10 branch as well?

I wouldn't.  The general guidance is that we fix regressions on the
release branches and this wouldn't qualify. 


Jeff




Re: Rename DECL_IS_BUILTIN to DECL_IS_UNDECLARED_BUILTIN

2020-11-06 Thread Jeff Law via Gcc-patches


On 10/23/20 6:48 AM, Nathan Sidwell wrote:
> Patch affects C++, C, GO, common-core
>
> In cleaning up C++'s handling of hidden decls, I renamed its
> DECL_BUILTIN_P, which checks for loc == BUILTINS_LOCATION to
> DECL_UNDECLARED_BUILTIN_P, because the location gets updated, if user
> source declares the builtin, and the predicate no longer holds.  The
> original name was confusing me.  (The builtin may still retain builtin
> properties in the redeclaration, and other predicates can still detect
> that.)
>
> I discovered that tree.h had its own variant 'DECL_IS_BUILTIN', which
> behaves in (almost) the same manner.  And therefore has the same
> mutating behaviour.
>
> This patch deletes the C++ one, and renames tree.h's to
> DECL_IS_UNDECLARED_BUILTIN, to emphasize its non-constantness.  I
> guess _IS_ wins over _P :)
>
> The indirection via SOURCE_LOCUS was introduced by Richard in 2012:
>     2012-09-26  Richard Guenther  
>
>     * tree.h (DECL_IS_BUILTIN): Compare LOCATION_LOCUS.
>
>     From-SVN: r191759
>
> I couldn't find the email on gcc-patches, but I don't see why this is
> necessary -- no undeclared builtin has an adhoc location, they're all
> BUILTINS_LOCATION, or UNKNOWN_LOCATION.
>
> That some builtins have UNKNOWN_LOCATION is why the test is <= rather
> than ==.  This seems wrong, and we should be using BUILTINS_LOCATION
> everywhere.  But that's a different bug.
>
> bootstrapped on x86-64-linux and test results look the same across all
> the languages I can build. ok?
>
> gcc/
> * tree.h (DECL_IS_BUILTIN): Rename to ...
> (DECL_IS_UNDECLARED_BUILTIN): ... here.  No need to use
>     SOURCE_LOCUS.
> * calls.c (maybe_warn_alloc_args_overflow): Adjust for rename.
> * cfgexpand.c (pass_expand::execute): Likewise.
> * dwarf2out.c (base_type_die, is_naming_typedef_decl): Likewise.
> * godump.c (go_decl, go_type_decl): Likewise.
> * print-tree.c (print_decl_identifier): Likewise.
> * tree-pretty-print.c (dump_generic_node): Likewise.
> * tree-ssa-ccp.c (pass_post_ipa_warn::execute): Likewise.
> * xcoffout.c (xcoff_assign_fundamental_type_number): Likewise.
> gcc/c-family/
> * c-ada-spec.c (collect_ada_nodes): Rename
> DECL_IS_BUILTIN->DECL_IS_UNDECLARED_BUILTIN.
> (collect_ada_node): Likewise.
> (dump_forward_type): Likewise.
> * c-common.c (set_underlying_type): Rename
> DECL_IS_BUILTIN->DECL_IS_UNDECLARED_BUILTIN.
> (user_facing_original_type): Likewise.
> (c_common_finalize_early_debug): Likewise.
> gcc/c/
> * c-decl.c (diagnose_mismatched_decls): Rename
> DECL_IS_BUILTIN->DECL_IS_UNDECLARED_BUILTIN.
> (warn_if_shadowing, implicitly_declare, names_builtin_p)
> (collect_source_refs): Likewise.
> * c-typeck.c (inform_declaration, inform_for_arg)
> (convert_for_assignment): Likewise.
> gcc/cp/
> * cp-tree.h (DECL_UNDECLARED_BUILTIN_P): Delete.
> * cp-objcp-common.c (names_bultin_p): Rename
> DECL_IS_BUILTIN->DECL_IS_UNDECLARED_BUILTIN.
> * decl.c (decls_match): Likewise.  Replace
> DECL_UNDECLARED_BUILTIN_P with DECL_IS_UNDECLARED_BUILTIN.
> (duplicate_decls): Likewise.
> * decl2.c (collect_source_refs): Likewise.
> * name-lookup.c (anticipated_builtin_p, print_binding_level)
> (do_nonmember_using_decl): Likewise.
> * pt.c (builtin_pack_fn_p): Likewise.
> * typeck.c (error_args_num): Likewise.
> gcc/lto/
> * lto-symtab.c (lto_symtab_merge_decls_1): Rename
> DECL_IS_BUILTIN->DECL_IS_UNDECLARED_BUILTIN.
> gcc/go/
> * go-gcc.cc (Gcc_backend::call_expression): Rename
> DECL_IS_BUILTIN->DECL_IS_UNDECLARED_BUILTIN.
> libcc1/
> * libcc1plugin.cc (address_rewriter): Rename
> DECL_IS_BUILTIN->DECL_IS_UNDECLARED_BUILTIN.
> * libcp1plugin.cc (supplement_binding): Likewise.

OK

jeff




Re: [PATCH] Optimize macro: make it more predictable

2020-11-06 Thread Jeff Law via Gcc-patches


On 10/23/20 5:47 AM, Martin Liška wrote:
> Hey.
>
> This is a follow-up of the discussion that happened in thread about
> no_stack_protector
> attribute: https://gcc.gnu.org/pipermail/gcc-patches/2020-May/545916.html
>
> The current optimize attribute works in the following way:
> - 1) we take current global_options as base
> - 2) maybe_default_options is called for the currently selected
> optimization level, which
>      means all rules in default_options_table are executed
> - 3) attribute values are applied (via decode_options)
>
> So the step 2) is problematic: in case of -O2 -fno-omit-frame-pointer
> and __attribute__((optimize("-fno-stack-protector")))
> ends basically with -O2 -fno-stack-protector because
> -fno-omit-frame-pointer is default:
>     /* -O1 and -Og optimizations.  */
>     { OPT_LEVELS_1_PLUS, OPT_fomit_frame_pointer, NULL, 1 },
>
> My patch handled and the current optimize attribute really behaves
> that same as appending attribute value
> to the command line. So far so good. We should also reflect that in
> documentation entry which is quite
> vague right now:
>
> """
> The optimize attribute is used to specify that a function is to be
> compiled with different optimization options than specified on the
> command line.
> """
>
> and we may want to handle -Ox in the attribute in a special way. I
> guess many macro/pragma users expect that
>
> -O2 -ftree-vectorize and __attribute__((optimize(1))) will end with
> -O1 and not
> with -ftree-vectorize -O1 ?
>
> I'm also planning to take a look at the target macro/attribute, I
> expect similar problems:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97469
>
> Thoughts?
> Thanks,
> Martin
>
> gcc/c-family/ChangeLog:
>
>     * c-common.c (parse_optimize_options): Decoded attribute options
>     with the ones that were already set on the command line.
>
> gcc/ChangeLog:
>
>     * toplev.c (toplev::main): Save decoded Optimization options.
>     * toplev.h (save_opt_decoded_options): New.
>
> gcc/testsuite/ChangeLog:
>
>     * gcc.target/i386/avx512er-vrsqrt28ps-3.c: Disable -ffast-math.
>     * gcc.target/i386/avx512er-vrsqrt28ps-5.c: Likewise.
So you XNEWVEC and store the result into "merge_decoded_options".  But
you free "decoded_options".  Was that intentional?

This seems to bring a bit more predictability, but I suspect there's
more to do here.


jeff




Re: [PATCH] arc: Improve/add instruction patterns to better use MAC instructions.

2020-11-06 Thread Jeff Law via Gcc-patches


On 10/9/20 8:24 AM, Claudiu Zissulescu wrote:
> From: Claudiu Zissulescu 
>
> ARC MYP7+ instructions add MAC instructions for vector and scalar data
> types. This patch adds a madd pattern for 16it datum that is using the
> 32bit MAC instruction, and dot_prod patterns for v4hi vector
> types. The 64bit moves are also upgraded by using vadd2 instuction.
>
> gcc/
> -xx-xx  Claudiu Zissulescu  
>
>   * config/arc/arc.c (arc_split_move): Recognize vadd2 instructions.
>   * config/arc/arc.md (movdi_insn): Update pattern to use vadd2
>   instructions.
>   (movdf_insn): Likewise.
>   (maddhisi4): New pattern.
>   (umaddhisi4): Likewise.
>   * config/arc/simdext.md (mov_int): Update pattern to use
>   vadd2.
>   (sdot_prodv4hi): New pattern.
>   (udot_prodv4hi): Likewise.
>   (arc_vec_mac_hi_v4hi): Update/renamed to
>   arc_vec_mac_v2hiv2si.
>   (arc_vec_mac_v2hiv2si_zero): New pattern.

OK for the trunk.  Sorry for the delay.

jeff




Re: [PATCH] Support the new ("v0") mangling scheme in rust-demangle.

2020-11-06 Thread Jeff Law via Gcc-patches


On 11/1/20 10:26 AM, Nikhil Benesch via Gcc-patches wrote:
>
>
> On 11/1/20 6:57 AM, Eduard-Mihai Burtescu wrote:
>> Reading the diff patch, the v0 changes look great. I wouldn't be too
>> worried
>> about the "printable character" aspect, there are similar
>> Unicode-related
>> issues elsewhere, e.g. the "non-control ASCII" comment in
>> decode_legacy_escape
>> (I suppose we could make it also go through the "print a non-control
>> ASCII
>> character or some escape sequence" logic you added, if you think that
>> helps).
>
> No, it's entirely fine with me! I just wasn't sure if the small
> deviations in output were acceptable. It sounds like they are.

So I think the best path forward is to let you and Eduard-Mihai make the
technical decisions about what bits are ready for the trunk.  When y'all
think something is ready, let's go ahead and get it installed and
iterate on things that aren't quite ready yet.


For bits y'all think are ready, ISTM that Eduard-Mihai should commit the
changes.



>> I can test the patch and upload the dataset tomorrow, but if you want
>> to get
>> something committed sooner (is there a deadline for the next
>> release?), feel
>> free to land the v0 changes (snprintf + const values) without the
>> legacy ones.
>
> My understanding is that the GCC tree closes to new features on
> November 16 (for "GCC 11 Stage 3"), but I'm not sure whether that
> applies to libiberty or whether this patch would be classified as a
> feature or a bugfix.
>
> I don't have commit rights (nor am I even a GCC developer). Just
> wanted to tee things up for you and Ian this week. I'm very much
> looking forward to the new demangling scheme and didn't want to be
> just another +1 on the GitHub issue.
>
> So certainly no time pressure from me. But perhaps someone from the
> GCC side can confirm whether we are under a bit of time pressure here
> given the GCC 11 release.

It's better to get it in sooner, but there is some degree of freedom
depending on the impact of the changes.  Changes in the rust demangler
aren't likely to trigger codegen or ABI breakages in the compiler itself
-- so with that in mind I think we should give this code a higher degree
of freedom to land after the stage1 close deadline.


jeff




Re: [21/32] miscelaneous

2020-11-06 Thread Nathan Sidwell

On 11/5/20 8:30 AM, Richard Biener wrote:

On Tue, Nov 3, 2020 at 10:16 PM Nathan Sidwell  wrote:


These are changes to gcc/tree.h adding some raw accessors to nodes,
which seemed preferable to direct field access.  I also needed access to
the integral constant cache


can you please document the adjusted interface to cache_integer_cst in
its (non-existing) function level comment?  It looks like 'replace'== true
turns it into get_or_insert from now put with an assertion it wasn't in the
cache.


Sure.  It's a little weird in that the current behaviour is to allow 
duplicates in the hash table, but not in the type's small-value vector.


I renamed the new parameter and documented what happens.  I'll apply 
this as a distinct patch during the merge (with changelog).  For now it 
lives on the modules branch


nathan

--
Nathan Sidwell
diff --git c/gcc/tree.c w/gcc/tree.c
index 9260772b846..9e10df0d7d0 100644
--- c/gcc/tree.c
+++ w/gcc/tree.c
@@ -1727,8 +1727,15 @@ wide_int_to_tree (tree type, const poly_wide_int_ref )
   return build_poly_int_cst (type, value);
 }
 
-void
-cache_integer_cst (tree t)
+/* Insert INTEGER_CST T into a cache of integer constants.  And return
+   the cached constant (which may or may not be T).  If MAY_DUPLICATE
+   is false, and T falls into the type's 'smaller values' range, there
+   cannot be an existing entry.  Otherwise, if MAY_DUPLICATE is true,
+   or the value is large, should an existing entry exist, it is
+   returned (rather than inserting T).  */
+
+tree
+cache_integer_cst (tree t, bool may_duplicate ATTRIBUTE_UNUSED)
 {
   tree type = TREE_TYPE (t);
   int ix = -1;
@@ -1742,7 +1749,7 @@ cache_integer_cst (tree t)
   switch (TREE_CODE (type))
 {
 case NULLPTR_TYPE:
-  gcc_assert (integer_zerop (t));
+  gcc_checking_assert (integer_zerop (t));
   /* Fallthru.  */
 
 case POINTER_TYPE:
@@ -1822,21 +1829,32 @@ cache_integer_cst (tree t)
 	  TYPE_CACHED_VALUES (type) = make_tree_vec (limit);
 	}
 
-  gcc_assert (TREE_VEC_ELT (TYPE_CACHED_VALUES (type), ix) == NULL_TREE);
-  TREE_VEC_ELT (TYPE_CACHED_VALUES (type), ix) = t;
+  if (tree r = TREE_VEC_ELT (TYPE_CACHED_VALUES (type), ix))
+	{
+	  gcc_checking_assert (may_duplicate);
+	  t = r;
+	}
+  else
+	TREE_VEC_ELT (TYPE_CACHED_VALUES (type), ix) = t;
 }
   else
 {
   /* Use the cache of larger shared ints.  */
   tree *slot = int_cst_hash_table->find_slot (t, INSERT);
-  /* If there is already an entry for the number verify it's the
- same.  */
-  if (*slot)
-	gcc_assert (wi::to_wide (tree (*slot)) == wi::to_wide (t));
+  if (tree r = *slot)
+	{
+	  /* If there is already an entry for the number verify it's the
+	 same value.  */
+	  gcc_checking_assert (wi::to_wide (tree (r)) == wi::to_wide (t));
+	  /* And return the cached value.  */
+	  t = r;
+	}
   else
 	/* Otherwise insert this one into the hash table.  */
 	*slot = t;
 }
+
+  return t;
 }
 
 


[PATCH] rework PRE PHI translation cache

2020-11-06 Thread Richard Biener
Turns out its size and time requirements can be stripped down
dramatically.

Bootstrap & regtest running on x86_64-unknown-linux-gnu.

2020-11-06  Richard Biener  

* tree-ssa-pre.c (expr_pred_trans_d): Modify so elements
are embedded rather than allocated.  Remove hashval member,
make all members integers.
(phi_trans_add): Adjust accordingly.
(phi_translate): Likewise.  Deal with re-allocation
of the table.
---
 gcc/tree-ssa-pre.c | 104 +
 1 file changed, 68 insertions(+), 36 deletions(-)

diff --git a/gcc/tree-ssa-pre.c b/gcc/tree-ssa-pre.c
index 65e8aaaca02..3496891f8b5 100644
--- a/gcc/tree-ssa-pre.c
+++ b/gcc/tree-ssa-pre.c
@@ -545,45 +545,74 @@ static bitmap_obstack grand_bitmap_obstack;
 /* A three tuple {e, pred, v} used to cache phi translations in the
phi_translate_table.  */
 
-typedef struct expr_pred_trans_d : free_ptr_hash
+typedef struct expr_pred_trans_d : public typed_noop_remove 
 {
-  /* The expression.  */
-  pre_expr e;
+  typedef expr_pred_trans_d value_type;
+  typedef expr_pred_trans_d compare_type;
 
-  /* The predecessor block along which we translated the expression.  */
-  basic_block pred;
+  /* The expression ID.  */
+  unsigned e;
 
-  /* The value that resulted from the translation.  */
-  pre_expr v;
+  /* The predecessor block index along which we translated the expression.  */
+  int pred;
 
-  /* The hashcode for the expression, pred pair. This is cached for
- speed reasons.  */
-  hashval_t hashcode;
+  /* The value expression ID that resulted from the translation.  */
+  unsigned v;
 
   /* hash_table support.  */
-  static inline hashval_t hash (const expr_pred_trans_d *);
-  static inline int equal (const expr_pred_trans_d *, const expr_pred_trans_d 
*);
+  static inline void mark_empty (expr_pred_trans_d &);
+  static inline bool is_empty (const expr_pred_trans_d &);
+  static inline void mark_deleted (expr_pred_trans_d &);
+  static inline bool is_deleted (const expr_pred_trans_d &);
+  static const bool empty_zero_p = true;
+  static inline hashval_t hash (const expr_pred_trans_d &);
+  static inline int equal (const expr_pred_trans_d &, const expr_pred_trans_d 
&);
 } *expr_pred_trans_t;
 typedef const struct expr_pred_trans_d *const_expr_pred_trans_t;
 
+inline bool
+expr_pred_trans_d::is_empty (const expr_pred_trans_d )
+{
+  return e.e == 0;
+}
+
+inline bool
+expr_pred_trans_d::is_deleted (const expr_pred_trans_d )
+{
+  return e.e == -1u;
+}
+
+inline void
+expr_pred_trans_d::mark_empty (expr_pred_trans_d )
+{
+  e.e = 0;
+}
+
+inline void
+expr_pred_trans_d::mark_deleted (expr_pred_trans_d )
+{
+  e.e = -1u;
+}
+
 inline hashval_t
-expr_pred_trans_d::hash (const expr_pred_trans_d *e)
+expr_pred_trans_d::hash (const expr_pred_trans_d )
 {
-  return e->hashcode;
+  return iterative_hash_hashval_t (e.e, e.pred);
 }
 
 inline int
-expr_pred_trans_d::equal (const expr_pred_trans_d *ve1,
- const expr_pred_trans_d *ve2)
+expr_pred_trans_d::equal (const expr_pred_trans_d ,
+ const expr_pred_trans_d )
 {
-  basic_block b1 = ve1->pred;
-  basic_block b2 = ve2->pred;
+  int b1 = ve1.pred;
+  int b2 = ve2.pred;
 
   /* If they are not translations for the same basic block, they can't
  be equal.  */
   if (b1 != b2)
 return false;
-  return pre_expr_d::equal (ve1->e, ve2->e);
+
+  return ve1.e == ve2.e;
 }
 
 /* The phi_translate_table caches phi translations for a given
@@ -596,24 +625,22 @@ static hash_table *phi_translate_table;
 static inline bool
 phi_trans_add (expr_pred_trans_t *entry, pre_expr e, basic_block pred)
 {
-  expr_pred_trans_t *slot;
+  expr_pred_trans_t slot;
   expr_pred_trans_d tem;
-  hashval_t hash = iterative_hash_hashval_t (pre_expr_d::hash (e),
-pred->index);
-  tem.e = e;
-  tem.pred = pred;
-  tem.hashcode = hash;
-  slot = phi_translate_table->find_slot_with_hash (, hash, INSERT);
-  if (*slot)
+  unsigned id = get_expression_id (e);
+  hashval_t hash = iterative_hash_hashval_t (id, pred->index);
+  tem.e = id;
+  tem.pred = pred->index;
+  slot = phi_translate_table->find_slot_with_hash (tem, hash, INSERT);
+  if (slot->e)
 {
-  *entry = *slot;
+  *entry = slot;
   return true;
 }
 
-  *entry = *slot = XNEW (struct expr_pred_trans_d);
-  (*entry)->e = e;
-  (*entry)->pred = pred;
-  (*entry)->hashcode = hash;
+  *entry = slot;
+  slot->e = id;
+  slot->pred = pred->index;
   return false;
 }
 
@@ -1675,6 +1702,7 @@ phi_translate (bitmap_set_t dest, pre_expr expr,
   bitmap_set_t set1, bitmap_set_t set2, edge e)
 {
   expr_pred_trans_t slot = NULL;
+  size_t slot_size = 0;
   pre_expr phitrans;
 
   if (!expr)
@@ -1691,10 +1719,11 @@ phi_translate (bitmap_set_t dest, pre_expr expr,
   if (expr->kind != NAME)
 {
   if (phi_trans_add (, expr, e->src))
-   return slot->v;
+   return slot->v == 0 ? 

libcpp: Provide date routine

2020-11-06 Thread Nathan Sidwell

Joseph pointed me at cb_get_source_date_epoch, which allows repeatable
builds and solves a FIXME I had on the modules branch.  Unfortunately
it's used exclusively to generate __DATE__ and __TIME__ values, which
fallback to using a time(2) call.  It'd be nicer if the preprocessor
made whatever time value it determined available to the rest of the
compiler.  So this patch adds a new cpp_get_date function, which
abstracts the call to the get_source_date_epoch hook, or uses time
directly.  The value is cached.  Thus the timestamp I end up putting
on CMI files matches __DATE__ and __TIME__ expansions.  That seems
worthwhile.

libcpp/
* include/libcpp.h (enum class CPP_time_kind): New.
(cpp_get_date): Declare.
* internal.h (struct cpp_reader): Replace source_date_epoch with
time_stamp and time_stamp_kind.
* init.c (cpp_create_reader): Initialize them.
* macro.c (_cpp_builtin_macro_text): Use cpp_get_date.
(cpp_get_date): Broken out from _cpp_builtin_macro_text and
genericized.

pushing to trunk

nathan

--
Nathan Sidwell
diff --git i/libcpp/include/cpplib.h w/libcpp/include/cpplib.h
index 8e398863cf6..c4d7cc520d1 100644
--- i/libcpp/include/cpplib.h
+++ w/libcpp/include/cpplib.h
@@ -1040,6 +1040,15 @@ inline location_t cpp_macro_definition_location (cpp_hashnode *node)
 {
   return node->value.macro->line;
 }
+/* Return an idempotent time stamp (possibly from SOURCE_DATE_EPOCH).  */
+enum class CPP_time_kind 
+{
+  FIXED = -1,	/* Fixed time via source epoch.  */
+  DYNAMIC = -2,	/* Dynamic via time(2).  */
+  UNKNOWN = -3	/* Wibbly wobbly, timey wimey.  */
+};
+extern CPP_time_kind cpp_get_date (cpp_reader *, time_t *);
+
 extern void _cpp_backup_tokens (cpp_reader *, unsigned int);
 extern const cpp_token *cpp_peek_token (cpp_reader *, int);
 
diff --git i/libcpp/init.c w/libcpp/init.c
index 6c52f50de39..dcf1d4be587 100644
--- i/libcpp/init.c
+++ w/libcpp/init.c
@@ -273,8 +273,9 @@ cpp_create_reader (enum c_lang lang, cpp_hash_table *table,
   /* Do not force token locations by default.  */
   pfile->forced_token_location = 0;
 
-  /* Initialize source_date_epoch to -2 (not yet set).  */
-  pfile->source_date_epoch = (time_t) -2;
+  /* Note the timestamp is unset.  */
+  pfile->time_stamp = time_t (-1);
+  pfile->time_stamp_kind = 0;
 
   /* The expression parser stack.  */
   _cpp_expand_op_stack (pfile);
diff --git i/libcpp/internal.h w/libcpp/internal.h
index 4759961a33a..d7780e49d27 100644
--- i/libcpp/internal.h
+++ w/libcpp/internal.h
@@ -512,10 +512,9 @@ struct cpp_reader
   const unsigned char *date;
   const unsigned char *time;
 
-  /* Externally set timestamp to replace current date and time useful for
- reproducibility.  It should be initialized to -2 (not yet set) and
- set to -1 to disable it or to a non-negative value to enable it.  */
-  time_t source_date_epoch;
+  /* Time stamp, set idempotently lazily.  */
+  time_t time_stamp;
+  int time_stamp_kind; /* Or errno.  */
 
   /* A token forcing paste avoidance, and one demarking macro arguments.  */
   cpp_token avoid_paste;
diff --git i/libcpp/macro.c w/libcpp/macro.c
index e304f67c2e0..e2cb89e4c43 100644
--- i/libcpp/macro.c
+++ w/libcpp/macro.c
@@ -606,29 +606,21 @@ _cpp_builtin_macro_text (cpp_reader *pfile, cpp_hashnode *node,
 	 at init time, because time() and localtime() are very
 	 slow on some systems.  */
 	  time_t tt;
-	  struct tm *tb = NULL;
+	  auto kind = cpp_get_date (pfile, );
 
-	  /* Set a reproducible timestamp for __DATE__ and __TIME__ macro
-	 if SOURCE_DATE_EPOCH is defined.  */
-	  if (pfile->source_date_epoch == (time_t) -2
-	  && pfile->cb.get_source_date_epoch != NULL)
-	pfile->source_date_epoch = pfile->cb.get_source_date_epoch (pfile);
-
-	  if (pfile->source_date_epoch >= (time_t) 0)
-	tb = gmtime (>source_date_epoch);
-	  else
+	  if (kind == CPP_time_kind::UNKNOWN)
 	{
-	  /* (time_t) -1 is a legitimate value for "number of seconds
-		 since the Epoch", so we have to do a little dance to
-		 distinguish that from a genuine error.  */
-	  errno = 0;
-	  tt = time (NULL);
-	  if (tt != (time_t)-1 || errno == 0)
-		tb = localtime ();
+	  cpp_errno (pfile, CPP_DL_WARNING,
+			 "could not determine date and time");
+		
+	  pfile->date = UC"\"??? ?? \"";
+	  pfile->time = UC"\"??:??:??\"";
 	}
-
-	  if (tb)
+	  else
 	{
+	  struct tm *tb = (kind == CPP_time_kind::FIXED
+			   ? gmtime : localtime) ();
+
 	  pfile->date = _cpp_unaligned_alloc (pfile,
 		  sizeof ("\"Oct 11 1347\""));
 	  sprintf ((char *) pfile->date, "\"%s %2d %4d\"",
@@ -640,14 +632,6 @@ _cpp_builtin_macro_text (cpp_reader *pfile, cpp_hashnode *node,
 	  sprintf ((char *) pfile->time, "\"%02d:%02d:%02d\"",
 		   tb->tm_hour, tb->tm_min, tb->tm_sec);
 	}
-	  else
-	{
-	  cpp_errno (pfile, CPP_DL_WARNING,
-			 "could not determine date and time");
-		
-	  

[PATCH] aarch64: Support permutes on unpacked SVE vectors

2020-11-06 Thread Richard Sandiford via Gcc-patches
This patch adds support for permuting unpacked SVE vectors using:

- DUP
- EXT
- REV[BHW]
- REV
- TRN[12]
- UZP[12]
- ZIP[12]

This involves rewriting the REV[BHW] permute code so that the inputs
and outputs of the insn pattern have the same mode as the vectors
being permuted.  This is different from the ACLE form, where the
reversal happens within individual elements rather than within
groups of multiple elements.

The patch does not add a conditional version of REV[BHW].  I'll come
back to that once we have partial-vector comparisons and selects.

The patch is really just enablement, adding an extra tool to the
toolbox.  It doesn't bring any significant vectorisation opportunities
on its own.  However, the patch does have one artificial example that
is now vectorised in a better way than before.

Tested on aarch64-linux-gnu (with and without SVE), applied.

Richard


gcc/
* config/aarch64/aarch64-modes.def (VNx2BF, VNx4BF): Adjust nunits
and alignment based on the current VG.
* config/aarch64/iterators.md (SVE_ALL, SVE_24, SVE_2, SVE_4): Add
partial SVE BF modes.
(UNSPEC_REVBHW): New unspec.
(Vetype, Vesize, Vctype, VEL, Vel, vwcore, V_INT_CONTAINER)
(v_int_container, VPRED, vpred): Handle partial SVE BF modes.
(container_bits, Vcwtype): New mode attributes.
* config/aarch64/aarch64-sve.md
(@aarch64_sve_revbhw_): New pattern.
(@aarch64_sve_dup_lane): Extended from SVE_FULL to SVE_ALL.
(@aarch64_sve_rev, @aarch64_sve_): Likewise.
(@aarch64_sve_ext): Likewise.
* config/aarch64/aarch64.c (aarch64_classify_vector_mode): Handle
E_VNx2BFmode and E_VNx4BFmode.
(aarch64_evpc_rev_local): Base the analysis on the container size
instead of the element size.  Use the new aarch64_sve_revbhw
patterns for SVE.
(aarch64_evpc_dup): Handle partial SVE data modes.  Use the
container size instead of the element size when applying the
SVE immediate limit.  Fix a previously incorrect bounds check.
(aarch64_expand_vec_perm_const_1): Handle partial SVE data modes.

gcc/testsuite/
* gcc.target/aarch64/sve/dup_lane_2.c: New test.
* gcc.target/aarch64/sve/dup_lane_3.c: Likewise.
* gcc.target/aarch64/sve/ext_4.c: Likewise.
* gcc.target/aarch64/sve/rev_2.c: Likewise.
* gcc.target/aarch64/sve/revhw_1.c: Likewise.
* gcc.target/aarch64/sve/revhw_2.c: Likewise.
* gcc.target/aarch64/sve/slp_perm_8.c: Likewise.
* gcc.target/aarch64/sve/trn1_2.c: Likewise.
* gcc.target/aarch64/sve/trn2_2.c: Likewise.
* gcc.target/aarch64/sve/uzp1_2.c: Likewise.
* gcc.target/aarch64/sve/uzp2_2.c: Likewise.
* gcc.target/aarch64/sve/zip1_2.c: Likewise.
* gcc.target/aarch64/sve/zip2_2.c: Likewise.
---
 gcc/config/aarch64/aarch64-modes.def  |   4 +
 gcc/config/aarch64/aarch64-sve.md |  57 ++-
 gcc/config/aarch64/aarch64.c  |  45 +-
 gcc/config/aarch64/iterators.md   |  54 ++-
 .../gcc.target/aarch64/sve/dup_lane_2.c   | 331 ++
 .../gcc.target/aarch64/sve/dup_lane_3.c   |  90 
 gcc/testsuite/gcc.target/aarch64/sve/ext_4.c  | 353 +++
 gcc/testsuite/gcc.target/aarch64/sve/rev_2.c  | 177 
 .../gcc.target/aarch64/sve/revhw_1.c  | 127 ++
 .../gcc.target/aarch64/sve/revhw_2.c  | 127 ++
 .../gcc.target/aarch64/sve/slp_perm_8.c   |  18 +
 gcc/testsuite/gcc.target/aarch64/sve/trn1_2.c | 403 ++
 gcc/testsuite/gcc.target/aarch64/sve/trn2_2.c | 403 ++
 gcc/testsuite/gcc.target/aarch64/sve/uzp1_2.c | 375 
 gcc/testsuite/gcc.target/aarch64/sve/uzp2_2.c | 375 
 gcc/testsuite/gcc.target/aarch64/sve/zip1_2.c | 403 ++
 gcc/testsuite/gcc.target/aarch64/sve/zip2_2.c | 403 ++
 17 files changed, 3685 insertions(+), 60 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/dup_lane_2.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/dup_lane_3.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/ext_4.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/rev_2.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/revhw_1.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/revhw_2.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/slp_perm_8.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/trn1_2.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/trn2_2.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/uzp1_2.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/uzp2_2.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/zip1_2.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/zip2_2.c

diff --git a/gcc/config/aarch64/aarch64-modes.def 
b/gcc/config/aarch64/aarch64-modes.def
index 

Re: [PATCH, rs6000] Update instruction attributes for Power10

2020-11-06 Thread Pat Haugen via Gcc-patches
On 11/5/20 4:32 PM, will schmidt wrote:
> On Wed, 2020-11-04 at 14:42 -0600, Pat Haugen via Gcc-patches wrote:
>>  * config/rs6000/rs6000.c (rs6000_final_prescan_insn): Only add 'p' for
>>  PREFIXED_YES.
> 
> The code change reads as roughly 
> - next_insn_prefixed_p != PREFIXED_NO
> 
> + next_insn_prefixed_p == PREFIXED_YES"
> 
> So just an inversion of the logic? I don't obviously see the 'p' impact
> there.
> 
It's no longer an inversion of the logic since I added a PREFIXED_ALWAYS value. 
'next_insn_prefixed' is used by rs6000_final_prescan_insn() to determine 
whether an insn mnemonic needs a 'p' prefix. We want it set for PREFIXED_YES, 
but not for PREFIXED_NO or PREFIXED_ALWAYS.

> 
>>  * config/rs6000/rs6000.md (define_attr "size"): Add 256.
>>  (define_attr "prefixed"): Add 'always'.
>>  (define_mode_attr bits): Add DD/TD modes.
>>  (cfuged, cntlzdm, cnttzdm, pdepd, pextd, bswaphi2_reg, bswapsi2_reg,
>>  bswapdi2_brd, setbc_signed_,
>>  *setbcr_signed_, *setnbc_signed_,
>>  *setnbcr_signed_): Update instruction attributes for
>>  Power10.
> 
> ok.  (assuming the assorted 'integer' -> 'crypto' changes are correct,
> of course).  
> 
Yes, crypto represents the correct pipe the insns are executed on.

Thanks for the review,
Pat



Re: Patch for 96948

2020-11-06 Thread Jeff Law via Gcc-patches


On 9/8/20 9:34 AM, Martin Storsjö wrote:
> Hi,
>
> On Tue, 8 Sep 2020, Kirill Müller wrote:
>
>> Thanks for the heads up. The coincidence is funny -- a file that
>> hasn't been touched for years.
>
> I think we both may originally be triggered from the same guy asking
> around in different places about implementations of _Unwind_Backtrace
> for windows, actually.
>
>> I do believe that we need the logic around the `first` flag for
>> consistency with the other unwind-*.c implementations.
>
> Yes, if you store ms_context.Rip/Rsp before the RtlVirtualUnwind step
> - but my patch stores them afterwards; after RtlVirtualUnwind, before
> calling the callback.
>
> The result should be the same, except if using the first flag
> approach, I believe you're missing the last frame that is printed if
> using my patch.

Presumably with your patch installed, the patch from Kirill is
unnecessary, right?


jeff




Re: [PATCH] issue -Wstring-compare in more case (PR 95673)

2020-11-06 Thread Jeff Law via Gcc-patches


On 9/30/20 6:14 PM, Martin Sebor via Gcc-patches wrote:
> -Wstring-compare triggers under the same strict conditions as
> the strcmp/strncmp call is folded into a constant: only when
> all the uses of the result are [in]equality expressions with
> zero.  However, even when the call cannot be folded into
> a constant because the result is in addition used in other
> expressions besides equality to zero, GCC still sets the range
> of the result to nonzero.  So in more complex functions where
> some of the uses of the same result are in tests for equality
> to zero and others in other expressions, the warning fails to
> point out the very mistake it's designed to detect.
>
> The attached change enhances the function that determines how
> the strcmp/strncmp is used to also make it possible to detect
> the mistakes in the multi-use situations.
>
> Tested on x86_64-linux & by building Glibc and Binutils/GDB
> and confirming it triggers no new warnings.
>
> Martin
>
> gcc-95673.diff
>
> PR middle-end/95673 - missing -Wstring-compare for an impossible strncmp test
>
> gcc/ChangeLog:
>
>   PR middle-end/95673
>   * tree-ssa-strlen.c (used_only_for_zero_equality): Rename...
>   (use_in_zero_equality): ...to this.  Add a default argument.
>   (handle_builtin_memcmp): Adjust to the name change above.
>   (handle_builtin_string_cmp): Same.
>   (maybe_warn_pointless_strcmp): Same.  Pass in an explicit argument.
>
> gcc/testsuite/ChangeLog:
>
>   PR middle-end/95673
>   * gcc.dg/Wstring-compare-3.c: New test.

Please retest on the trunk and if testing is OK this is fine for the trunk.

jeff




Re: [PATCH] use get_size_range to get allocated size (PR 92942)

2020-11-06 Thread Jeff Law via Gcc-patches


On 8/28/20 11:12 AM, Martin Sebor via Gcc-patches wrote:
> The gimple_call_alloc_size() function that determines the range
> of sizes of allocated objects and constrains the bounds in calls
> to functions like memcpy calls get_range() instead of
> get_size_range() to obtain its result.  The latter is the right
> function to call because it has the necessary logic to constrain
> the range to just the values that are valid for object sizes.
> This is especially useful when the range is the result of
> a conversion from a signed to a wider unsigned integer where
> the upper subrange is excessive and can be eliminated such as in:
>
>   char* f (int n)
>   {
>     if (n > 8)
>   n = 8;
>     char *p = malloc (n);
>     strcpy (p, "0123456789");   // buffer overflow
>     ...
>   }
>
> Attached is a fix that lets -Wstringop-overflow diagnose the buffer
> overflow above.  Besides with GCC I have also tested the change by
> building Binutils/GDB and Glibc and verifying that it doesn't
> introduce any false positives.
>
> Martin
>
> gcc-92942.diff
>
> PR middle-end/92942 - missing -Wstringop-overflow for allocations with a 
> negative lower bound size
>
> gcc/ChangeLog:
>
>   PR middle-end/92942
>   * builtins.c (gimple_call_alloc_size): Call get_size_range instead
>   of get_range.
>   * calls.c (get_size_range): Define new overload.  Handle anti-ranges
> whose upper part is with the valid size range.
>   * calls.h (get_size_range): Declare new overload.
>
> gcc/testsuite/ChangeLog:
>
>   PR middle-end/92942
>   * gcc.dg/Wstringop-overflow-40.c: New test.
>   * gcc.dg/Wstringop-overflow-41.c: New test.
>   * gcc.dg/attr-alloc_size-10.c: Disable macro tracking.

Please re-rest and once re-validated, this is fine for the trunk.


jeff




Re: [PATCH] generalized range_query class for multiple contexts

2020-11-06 Thread Martin Sebor via Gcc-patches

On 11/5/20 8:08 PM, Andrew MacLeod wrote:

On 11/5/20 7:50 PM, Martin Sebor wrote:

On 11/5/20 5:02 PM, Andrew MacLeod wrote:

On 11/5/20 4:02 PM, Martin Sebor wrote:

On 11/5/20 12:29 PM, Martin Sebor wrote:

On 10/1/20 11:25 AM, Martin Sebor wrote:

On 10/1/20 9:34 AM, Aldy Hernandez wrote:



On 10/1/20 3:22 PM, Andrew MacLeod wrote:
 > On 10/1/20 5:05 AM, Aldy Hernandez via Gcc-patches wrote:
 >>> Thanks for doing all this!  There isn't anything I don't 
understand
 >>> in the sprintf changes so no questions from me (well, almost 
none).

 >>> Just some comments:
 >> Thanks for your comments on the sprintf/strlen API conversion.
 >>
 >>> The current call statement is available in all functions 
that take
 >>> a directive argument, as dir->info.callstmt.  There should 
be no need
 >>> to also add it as a new argument to the functions that now 
need it.

 >> Fixed.
 >>
 >>> The change adds code along these lines in a bunch of places:
 >>>
 >>> + value_range vr;
 >>> + if (!query->range_of_expr (vr, arg, stmt))
 >>> +   vr.set_varying (TREE_TYPE (arg));
 >>>
 >>> I thought under the new Ranger APIs when a range couldn't be
 >>> determined it would be automatically set to the maximum for
 >>> the type.  I like that and have been moving in that direction
 >>> with my code myself (rather than having an API fail, have it
 >>> set the max range and succeed).
 >> I went through all the above idioms and noticed all are being 
used on
 >> supported types (integers or pointers).  So range_of_expr 
will always

 >> return true.  I've removed the if() and the set_varying.
 >>
 >>> Since that isn't so in this case, I think it would still be 
nice

 >>> if the added code could be written as if the range were set to
 >>> varying in this case and (ideally) reduced to just 
initialization:

 >>>
 >>> value_range vr = some-function (query, stmt, arg);
 >>>
 >>> some-function could be an inline helper defined just for the 
sprintf
 >>> pass (and maybe also strlen which also seems to use the same 
pattern),
 >>> or it could be a value_range AKA irange ctor, or it could be 
a member

 >>> of range_query, whatever is the most appropriate.
 >>>
 >>> (If assigning/copying a value_range is thought to be too 
expensive,

 >>> declaring it first and then passing it to that helper to set it
 >>> would work too).
 >>>
 >>> In strlen, is the removed comment no longer relevant? (I.e., 
does

 >>> the ranger solve the problem?)
 >>>
 >>> -  /* The range below may be "inaccurate" if a constant 
has been
 >>> -    substituted earlier for VAL by this pass that 
hasn't been
 >>> -    propagated through the CFG.  This shoud be fixed by 
the new
 >>> -    on-demand VRP if/when it becomes available 
(hopefully in

 >>> -    GCC 11).  */
 >> It should.
 >>
 >>> I'm wondering about the comment added to 
get_range_strlen_dynamic

 >>> and other places:
 >>>
 >>> + // FIXME: Use range_query instead of global ranges.
 >>>
 >>> Is that something you're planning to do in a followup or should
 >>> I remember to do it at some point?
 >> I'm not planning on doing it.  It's just a reminder that it 
would be

 >> beneficial to do so.
 >>
 >>> Otherwise I have no concern with the changes.
 >> It's not cleared whether Andrew approved all 3 parts of the 
patchset
 >> or just the valuation part.  I'll wait for his nod before 
committing

 >> this chunk.
 >>
 >> Aldy
 >>
 > I have no issue with it, so OK.

Pushed all 3 patches.

 >
 > Just an observation that should be pointed out, I believe Aldy 
has all
 > the code for converting to a ranger, but we have not pursued 
that any
 > further yet since there is a regression due to our lack of 
equivalence
 > processing I think?  That should be resolved in the coming 
month, but at
 > the moment is a holdback/concern for converting these 
passes... iirc.


Yes.  Martin, the take away here is that the strlen/sprintf pass 
has been converted to the new API, but ranger is still not up and 
running on it (even on the branch).


With the new API, all you have to do is remove all instances of 
evrp_range_analyzer and replace them with a ranger. That's it.
Below is an untested patch that would convert you to a ranger 
once it's contributed.


IIRC when I enabled the ranger for your pass a while back, there 
was one or two regressions due to missing equivalences, and the 
rest were because the tests were expecting an actual specific 
range, and the ranger returned a slightly different/better one. 
You'll need to adjust your tests.


Ack.  I'll be on the lookout for the ranger commit (if you hppen
to remember and CC me on it just in case I might miss it that would
be great).


I have applied the patch and ran some tests.  There are quite
a few failures (see the list below).  I have only looked at
a couple.  The one in in gcc.dg/tree-ssa/builtin-sprintf-warn-3.c
boils down to the following test case.  There should be no warning
for either sprintf call.  The one in 

Re: Fix uninitialized memory use in ipa-modref

2020-11-06 Thread Martin Liška

On 11/5/20 6:37 PM, Jan Hubicka wrote:

We can't because our vec does not accept non-pods and this needs to be
GGC safe since it points to trees.


Ah, that's new to me!

Thanks,
Martin


Re: Fix uninitialized memory use in ipa-modref

2020-11-06 Thread Martin Liška

On 11/5/20 6:54 PM, Jan Hubicka wrote:

On 11/5/20 3:27 PM, Jan Hubicka wrote:

 poly_int64 offset;
 struct modref_parm_map parm_map;
+  parm_map.parm_offset_known = false;
+  parm_map.parm_offset = 0;
+


I'm curious, can't we use a proper C++ class construction.
The IPA pass is new and so we can make it more C++-ish? Similarly
for all newly introduced structs in mod ref.


We can't because our vec does not accept non-pods and this needs to be
GGC safe since it points to trees.


We could probably add construction of writes_errno even though in corret
run it should be never used (in analysis we need to be able to
reinitialize and during stream in we will always stream it in).


It may be error prone approach to initialize it to NULL.


What else do you think can be more ++-ish? The pass even has two
templates :).


Heh, all right :)



diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
index b40f3da3ba2..e80f6de09f2 100644
--- a/gcc/ipa-modref.c
+++ b/gcc/ipa-modref.c
@@ -124,7 +124,7 @@ static GTY(()) fast_function_summary 
  /* Summary for a single function which this pass produces.  */
  
  modref_summary::modref_summary ()

-  : loads (NULL), stores (NULL)
+  : loads (NULL), stores (NULL), writes_errno (NULL)
  {
  }
  





Re: [PATCH] arm: Implement vceqq_p64, vceqz_p64 and vceqzq_p64 intrinsics

2020-11-06 Thread Christophe Lyon via Gcc-patches
On Thu, 5 Nov 2020 at 12:55, Christophe Lyon  wrote:
>
> On Thu, 5 Nov 2020 at 10:36, Kyrylo Tkachov  wrote:
> >
> > H, Christophe,
> >
> > > -Original Message-
> > > From: Gcc-patches  On Behalf Of
> > > Christophe Lyon via Gcc-patches
> > > Sent: 15 October 2020 18:23
> > > To: gcc-patches@gcc.gnu.org
> > > Subject: [PATCH] arm: Implement vceqq_p64, vceqz_p64 and vceqzq_p64
> > > intrinsics
> > >
> > > This patch adds implementations for vceqq_p64, vceqz_p64 and
> > > vceqzq_p64 intrinsics.
> > >
> > > vceqq_p64 uses the existing vceq_p64 after splitting the input vectors
> > > into their high and low halves.
> > >
> > > vceqz[q] simply call the vceq and vceqq with a second argument equal
> > > to zero.
> > >
> > > The added (executable) testcases make sure that the poly64x2_t
> > > variants have results with one element of all zeroes (false) and the
> > > other element with all bits set to one (true).
> > >
> > > 2020-10-15  Christophe Lyon  
> > >
> > >   gcc/
> > >   * config/arm/arm_neon.h (vceqz_p64, vceqq_p64, vceqzq_p64):
> > > New.
> > >
> > >   gcc/testsuite/
> > >   * gcc.target/aarch64/advsimd-intrinsics/p64_p128.c: Add tests for
> > >   vceqz_p64, vceqq_p64 and vceqzq_p64.
> > > ---
> > >  gcc/config/arm/arm_neon.h  | 31 +++
> > >  .../aarch64/advsimd-intrinsics/p64_p128.c  | 46
> > > +-
> > >  2 files changed, 76 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/gcc/config/arm/arm_neon.h b/gcc/config/arm/arm_neon.h
> > > index aa21730..f7eff37 100644
> > > --- a/gcc/config/arm/arm_neon.h
> > > +++ b/gcc/config/arm/arm_neon.h
> > > @@ -16912,6 +16912,37 @@ vceq_p64 (poly64x1_t __a, poly64x1_t __b)
> > >return vreinterpret_u64_u32 (__m);
> > >  }
> > >
> > > +__extension__ extern __inline uint64x1_t
> > > +__attribute__  ((__always_inline__, __gnu_inline__, __artificial__))
> > > +vceqz_p64 (poly64x1_t __a)
> > > +{
> > > +  poly64x1_t __b = vreinterpret_p64_u32 (vdup_n_u32 (0));
> > > +  return vceq_p64 (__a, __b);
> > > +}
> >
> > This approach is okay, but can we have some kind of test to confirm it 
> > generates the VCEQ instruction with immediate zero rather than having a 
> > separate DUP...
>
> I had checked that manually, but I'll add a test.
> However, I have noticed that although vceqz_p64 uses vceq.i32 dX, dY, #0,
> the vceqzq_64 version below first sets
> vmov dZ, #0
> and then emits two
> vmoz dX, dY, dZ
>
> I'm looking at why this happens.
>

Hi,

Here is an updated version, which adds two tests (arm/simd/vceqz_p64.c
and arm/simd/vceqzq_p64.c).

The vceqzq_64 test does not currently expect instructions with
immediate zero, because we generate:
vmov.i32q9, #0  @ v4si
[...]
vceq.i32d16, d16, d19
vceq.i32d17, d17, d19

Looking at the traces, I can see this in reload:
(insn 19 8 15 2 (set (reg:V2SI 48 d16 [orig:128 _18 ] [128])
(neg:V2SI (eq:V2SI (reg:V2SI 48 d16 [orig:139 v1 ] [139])
(reg:V2SI 54 d19 [ _5+8 ]
"/home/christophe.lyon/src/GCC/builds/gcc-fsf-git-neon-intrinsics/tools/lib/gcc/arm-none-linux-gnueabihf/11.0.0/include/arm_neon.h":2404:22
1650 {neon_vceqv2si_insn}
 (expr_list:REG_EQUAL (neg:V2SI (eq:V2SI (subreg:V2SI (reg:DI 48
d16 [orig:139 v1 ] [139]) 0)
(const_vector:V2SI [
(const_int 0 [0]) repeated x2
])))
(nil)))
(insn 15 19 20 2 (set (reg:V2SI 50 d17 [orig:121 _11 ] [121])
(neg:V2SI (eq:V2SI (reg:V2SI 50 d17 [orig:141 v2 ] [141])
(reg:V2SI 54 d19 [ _5+8 ]
"/home/christophe.lyon/src/GCC/builds/gcc-fsf-git-neon-intrinsics/tools/lib/gcc/arm-none-linux-gnueabihf/11.0.0/include/arm_neon.h":2404:22
1650 {neon_vceqv2si_insn}
 (expr_list:REG_EQUAL (neg:V2SI (eq:V2SI (subreg:V2SI (reg:DI 50
d17 [orig:141 v2 ] [141]) 0)
(const_vector:V2SI [
(const_int 0 [0]) repeated x2
])))
(nil)))

but it says:
 Choosing alt 0 in insn 19:  (0) =w  (1) w  (2) w {neon_vceqv2si_insn}
  alt=0,overall=0,losers=0,rld_nregs=0
 Choosing alt 0 in insn 15:  (0) =w  (1) w  (2) w {neon_vceqv2si_insn}
  alt=0,overall=0,losers=0,rld_nregs=0

Why isn't it picking alternative 1 with the Dz constraint?

Christophe


> Thanks,
>
> Christophe
>
>
> > Thanks,
> > Kyrill
> >
> > > +
> > > +/* For vceqq_p64, we rely on vceq_p64 for each of the two elements.  */
> > > +__extension__ extern __inline uint64x2_t
> > > +__attribute__  ((__always_inline__, __gnu_inline__, __artificial__))
> > > +vceqq_p64 (poly64x2_t __a, poly64x2_t __b)
> > > +{
> > > +  poly64_t __high_a = vget_high_p64 (__a);
> > > +  poly64_t __high_b = vget_high_p64 (__b);
> > > +  uint64x1_t __high = vceq_p64(__high_a, __high_b);
> > > +
> > > +  poly64_t __low_a = vget_low_p64 (__a);
> > > +  poly64_t __low_b = vget_low_p64 (__b);
> > > +  uint64x1_t __low = vceq_p64(__low_a, __low_b);
> > > +  

Re: [PATCH]ira: recompute regstat as max_regno changes [PR97705]

2020-11-06 Thread Vladimir Makarov via Gcc-patches



On 2020-11-06 1:15 a.m., Kewen.Lin wrote:

Hi,

As PR97705 shows, my commit r11-4637 caused some dumping
comparison difference error on pass ira.  It exposed one
issue about the newly introduced function remove_scratches,
which can increase the largest pseudo reg number if it
succeeds, later some function will use the max_reg_num()
to get the latest max_regno, when iterating the numbers
we can access some data structures which are allocated as
the previous max_regno, some out of array bound accesses
can occur, the failure can be random since the values
beyond the array could be random.

This patch is to free/reinit/recompute the relevant data
structures that is regstat_n_sets_and_refs and reg_info_p
to ensure we won't access beyond some array bounds.

Bootstrapped/regtested on powerpc64le-linux-gnu P9 and
powerpc64-linux-gnu P8.

Any thoughts?  Is it a reasonable fix?

Sure, Kewen.  A bit unexpected to see lambda to use for this but I 
checked and found couple places in GCC where lambdas are already used.


The patch is ok.  Please, commit it to the mainline.

Thank you for the patch.



[PATCH] make PRE constant value IDs negative

2020-11-06 Thread Richard Biener
This separates constant and non-constant value-ids to allow for
a more efficient constant_value_id_p and for more efficient bit-packing
inside the bitmap sets which never contain any constant values.

There's further optimization opportunities but at this stage
I'll do small refactorings.

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

2020-11-06  Richard Biener  

* tree-ssa-sccvn.h (get_max_constant_value_id): Declare.
(get_next_constant_value_id): Likewise.
(value_id_constant_p): Inline and simplify.
* tree-ssa-sccvn.c (constant_value_ids): Remove.
(next_constant_value_id): Add.
(get_or_alloc_constant_value_id): Adjust.
(value_id_constant_p): Remove definition.
(get_max_constant_value_id): Define.
(get_next_value_id): Add assert for overflow.
(get_next_constant_value_id): Define.
(run_rpo_vn): Adjust.
(free_rpo_vn): Likewise.
(do_rpo_vn): Initialize next_constant_value_id.
* tree-ssa-pre.c (constant_value_expressions): New.
(add_to_value): Split into constant/non-constant value
handling.  Avoid exact re-allocation.
(vn_valnum_from_value_id): Adjust.
(phi_translate_1): Remove spurious exact re-allocation.
(bitmap_find_leader): Adjust.  Make sure we return
a CONSTANT value for a constant value id.
(do_pre_regular_insertion): Use 2 auto-elements for avail.
(do_pre_partial_partial_insertion): Likewise.
(init_pre): Allocate constant_value_expressions.
(fini_pre): Release constant_value_expressions.
---
 gcc/tree-ssa-pre.c   | 57 
 gcc/tree-ssa-sccvn.c | 34 --
 gcc/tree-ssa-sccvn.h | 12 +-
 3 files changed, 69 insertions(+), 34 deletions(-)

diff --git a/gcc/tree-ssa-pre.c b/gcc/tree-ssa-pre.c
index 39c52c9b0f0..65e8aaaca02 100644
--- a/gcc/tree-ssa-pre.c
+++ b/gcc/tree-ssa-pre.c
@@ -444,6 +444,9 @@ public:
 
 /* Mapping from value id to expressions with that value_id.  */
 static vec value_expressions;
+/* ???  We want to just record a single expression for each constant
+   value, one of kind CONSTANT.  */
+static vec constant_value_expressions;
 
 /* Sets that we need to keep track of.  */
 typedef struct bb_bitmap_sets
@@ -624,18 +627,30 @@ add_to_value (unsigned int v, pre_expr e)
 
   gcc_checking_assert (get_expr_value_id (e) == v);
 
-  if (v >= value_expressions.length ())
+  if (value_id_constant_p (v))
 {
-  value_expressions.safe_grow_cleared (v + 1, true);
-}
+  if (-v >= constant_value_expressions.length ())
+   constant_value_expressions.safe_grow_cleared (-v + 1);
 
-  set = value_expressions[v];
-  if (!set)
-{
-  set = BITMAP_ALLOC (_bitmap_obstack);
-  value_expressions[v] = set;
+  set = constant_value_expressions[-v];
+  if (!set)
+   {
+ set = BITMAP_ALLOC (_bitmap_obstack);
+ constant_value_expressions[-v] = set;
+   }
 }
+  else
+{
+  if (v >= value_expressions.length ())
+   value_expressions.safe_grow_cleared (v + 1);
 
+  set = value_expressions[v];
+  if (!set)
+   {
+ set = BITMAP_ALLOC (_bitmap_obstack);
+ value_expressions[v] = set;
+   }
+}
   bitmap_set_bit (set, get_or_alloc_expression_id (e));
 }
 
@@ -687,7 +702,11 @@ vn_valnum_from_value_id (unsigned int val)
 {
   bitmap_iterator bi;
   unsigned int i;
-  bitmap exprset = value_expressions[val];
+  bitmap exprset;
+  if (value_id_constant_p (val))
+exprset = constant_value_expressions[-val];
+  else
+exprset = value_expressions[val];
   EXECUTE_IF_SET_IN_BITMAP (exprset, 0, i, bi)
 {
   pre_expr vexpr = expression_for_id (i);
@@ -1451,8 +1470,6 @@ phi_translate_1 (bitmap_set_t dest,
else
  {
new_val_id = get_next_value_id ();
-   value_expressions.safe_grow_cleared (get_max_value_id () + 1,
-true);
nary = vn_nary_op_insert_pieces (newnary->length,
 newnary->opcode,
 newnary->type,
@@ -1603,11 +1620,7 @@ phi_translate_1 (bitmap_set_t dest,
else
  {
if (changed || !same_valid)
- {
-   new_val_id = get_next_value_id ();
-   value_expressions.safe_grow_cleared
- (get_max_value_id () + 1, true);
- }
+ new_val_id = get_next_value_id ();
else
  new_val_id = ref->value_id;
if (!newoperands.exists ())
@@ -1745,7 +1758,7 @@ bitmap_find_leader (bitmap_set_t set, unsigned int val)
 {
   unsigned int i;
   bitmap_iterator bi;
-  bitmap exprset = value_expressions[val];
+  bitmap exprset = 

[PATCH 3/6] Add Field Reordering

2020-11-06 Thread Erick Ochoa

From 72e6ea57b04ca2bf223faef262b478dc407cdca7 Mon Sep 17 00:00:00 2001
From: Erick Ochoa 
Date: Sun, 9 Aug 2020 10:22:49 +0200
Subject: [PATCH 3/6] Add Field Reordering

Field reordering of structs at link-time

2020-11-04  Erick Ochoa  

* gcc/Makefile.in: add new file to list of sources
* gcc/common.opt: add new flag for field reordering
* gcc/passes.def: add new pass
* gcc/tree-pass.h: same
* gcc/ipa-field-reorder.c: New file
* gcc/ipa-type-escape-analysis.c: Export common functions
* gcc/ipa-type-escape-analysis.h: Same
---
 gcc/Makefile.in|   1 +
 gcc/common.opt |   4 +
 gcc/ipa-dfe.c  |  86 -
 gcc/ipa-dfe.h  |  26 +-
 gcc/ipa-field-reorder.c| 622 +
 gcc/ipa-type-escape-analysis.c |  44 ++-
 gcc/ipa-type-escape-analysis.h |  12 +-
 gcc/passes.def |   1 +
 gcc/tree-pass.h|   2 +
 9 files changed, 749 insertions(+), 49 deletions(-)
 create mode 100644 gcc/ipa-field-reorder.c

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 8ef6047870b..2184bd0fc3d 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1417,6 +1417,7 @@ OBJS = \
internal-fn.o \
ipa-type-escape-analysis.o \
ipa-dfe.o \
+   ipa-field-reorder.o \
ipa-cp.o \
ipa-sra.o \
ipa-devirt.o \
diff --git a/gcc/common.opt b/gcc/common.opt
index 85351738a29..7885d0f5c0c 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -3468,4 +3468,8 @@ Wdfa
 Common Var(warn_dfa) Init(1) Warning
 Warn about dead fields at link time.

+fipa-field-reorder
+Common Report Var(flag_ipa_field_reorder) Optimization
+Reorder fields.
+
 ; This comment is to ensure we retain the blank line above.
diff --git a/gcc/ipa-dfe.c b/gcc/ipa-dfe.c
index 31a5066f1b5..5602ee667d4 100644
--- a/gcc/ipa-dfe.c
+++ b/gcc/ipa-dfe.c
@@ -185,7 +185,7 @@ get_types_replacement (record_field_offset_map_t 
record_field_offset_map,

 {
   TypeStringifier stringifier;

-  TypeReconstructor reconstructor (record_field_offset_map);
+  TypeReconstructor reconstructor (record_field_offset_map, "reorg");
   for (std::set::const_iterator i = to_modify.begin (),
e = to_modify.end ();
i != e; ++i)
@@ -245,9 +245,9 @@ get_types_replacement (record_field_offset_map_t 
record_field_offset_map,

  */
 void
 substitute_types_in_program (reorg_record_map_t map,
-reorg_field_map_t field_map)
+reorg_field_map_t field_map, bool _delete)
 {
-  GimpleTypeRewriter rewriter (map, field_map);
+  GimpleTypeRewriter rewriter (map, field_map, _delete);
   rewriter.walk ();
   rewriter._rewrite_function_decl ();
 }
@@ -361,8 +361,11 @@ TypeReconstructor::set_is_not_modified_yet (tree t)
 return;

   tree type = _reorg_map[tt];
-  const bool is_modified
+  bool is_modified
 = strstr (TypeStringifier::get_type_identifier (type).c_str (), 
".reorg");

+  is_modified
+|= (bool) strstr (TypeStringifier::get_type_identifier (type).c_str (),
+ ".reorder");
   if (!is_modified)
 return;

@@ -408,14 +411,20 @@ TypeReconstructor::is_memoized (tree t)
   return already_changed;
 }

-static tree
-get_new_identifier (tree type)
+const char *
+TypeReconstructor::get_new_suffix ()
+{
+  return _suffix;
+}
+
+tree
+get_new_identifier (tree type, const char *suffix)
 {
   const char *identifier = TypeStringifier::get_type_identifier 
(type).c_str ();

-  const bool is_new_type = strstr (identifier, "reorg");
+  const bool is_new_type = strstr (identifier, suffix);
   gcc_assert (!is_new_type);
   char *new_name;
-  asprintf (_name, "%s.reorg", identifier);
+  asprintf (_name, "%s.%s", identifier, suffix);
   return get_identifier (new_name);
 }

@@ -471,7 +480,9 @@ TypeReconstructor::_walk_ARRAY_TYPE_post (tree t)
   TREE_TYPE (copy) = build_variant_type_copy (TREE_TYPE (copy));
   copy = is_modified ? build_distinct_type_copy (copy) : copy;
   TREE_TYPE (copy) = is_modified ? _reorg_map[TREE_TYPE (t)] : 
TREE_TYPE (copy);
-  TYPE_NAME (copy) = is_modified ? get_new_identifier (copy) : 
TYPE_NAME (copy);

+  TYPE_NAME (copy) = is_modified
+  ? get_new_identifier (copy, this->get_new_suffix ())
+  : TYPE_NAME (copy);
   // This is useful so that we go again through type layout
   TYPE_SIZE (copy) = is_modified ? NULL : TYPE_SIZE (copy);
   tree domain = TYPE_DOMAIN (t);
@@ -524,7 +535,9 @@ TypeReconstructor::_walk_POINTER_TYPE_post (tree t)

   copy = is_modified ? build_variant_type_copy (copy) : copy;
   TREE_TYPE (copy) = is_modified ? _reorg_map[TREE_TYPE (t)] : 
TREE_TYPE (copy);
-  TYPE_NAME (copy) = is_modified ? get_new_identifier (copy) : 
TYPE_NAME (copy);

+  TYPE_NAME (copy) = is_modified
+  ? get_new_identifier (copy, this->get_new_suffix ())
+  : TYPE_NAME (copy);
   TYPE_CACHED_VALUES_P 

[PATCH 6/6] Add heuristic to take into account void* pattern.

2020-11-06 Thread Erick Ochoa

From ccd82a7e484d9e4562c23f1b9cbebf3f47e2a822 Mon Sep 17 00:00:00 2001
From: Erick Ochoa 
Date: Fri, 16 Oct 2020 08:49:08 +0200
Subject: [PATCH 6/6] Add heuristic to take into account void* pattern.

We add a heuristic in order to be able to transform functions which
receive void* arguments as a way to generalize over arguments. An
example of this is qsort. The heuristic works by first inspecting
leaves in the call graph. If the leaves only contain a reference
to a single RECORD_TYPE then we color the nodes in the call graph
as "casts are safe in this function and does not call external
visible functions". We propagate this property up the callgraph
until a fixed point is reached. This will later be changed to
use ipa-modref.

2020-11-04  Erick Ochoa  

* ipa-type-escape-analysis.c : Add new heuristic
* ipa-field-reorder.c : Use heuristic
* ipa-type-escape-analysis.h : Change signatures
---
 gcc/ipa-field-reorder.c|   3 +-
 gcc/ipa-type-escape-analysis.c | 182 +++--
 gcc/ipa-type-escape-analysis.h |  72 -
 3 files changed, 243 insertions(+), 14 deletions(-)

diff --git a/gcc/ipa-field-reorder.c b/gcc/ipa-field-reorder.c
index 9a28097b473..2f694cff7ea 100644
--- a/gcc/ipa-field-reorder.c
+++ b/gcc/ipa-field-reorder.c
@@ -588,8 +588,9 @@ lto_fr_execute ()
   log ("here in field reordering \n");
   // Analysis.
   detected_incompatible_syntax = false;
+  std::map whitelisted = get_whitelisted_nodes();
   tpartitions_t escaping_nonescaping_sets
-= partition_types_into_escaping_nonescaping ();
+= partition_types_into_escaping_nonescaping (whitelisted);
   record_field_map_t record_field_map = find_fields_accessed ();
   record_field_offset_map_t record_field_offset_map
 = obtain_nonescaping_unaccessed_fields (escaping_nonescaping_sets,
diff --git a/gcc/ipa-type-escape-analysis.c b/gcc/ipa-type-escape-analysis.c
index 40dc89c51a2..2fc504ce6f5 100644
--- a/gcc/ipa-type-escape-analysis.c
+++ b/gcc/ipa-type-escape-analysis.c
@@ -104,6 +104,7 @@ along with GCC; see the file COPYING3.  If not see
 #include 
 #include 
 #include 
+#include 

 #include "config.h"
 #include "system.h"
@@ -249,6 +250,99 @@ lto_dfe_execute ()
   return 0;
 }

+/* Heuristic to determine if casting is allowed in a function.
+ * This heuristic attempts to allow casting in functions which follow the
+ * pattern where a struct pointer or array pointer is casted to void* or
+ * char*.  The heuristic works as follows:
+ *
+ * There is a simple per-function analysis that determines whether there
+ * is more than 1 type of struct referenced in the body of the method.
+ * If there is more than 1 type of struct referenced in the body,
+ * then the layout of the structures referenced within the body
+ * cannot be casted.  However, if there's only one type of struct 
referenced

+ * in the body of the function, casting is allowed in the function itself.
+ * The logic behind this is that the if the code follows good programming
+ * practices, the only way the memory should be accessed is via a singular
+ * type. There is also another requisite to this per-function analysis, and
+ * that is that the function can only call colored functions or functions
+ * which are available in the linking unit.
+ *
+ * Using this per-function analysis, we then start coloring leaf nodes 
in the

+ * call graph as ``safe'' or ``unsafe''.  The color is propagated to the
+ * callers of the functions until a fixed point is reached.
+ */
+std::map
+get_whitelisted_nodes ()
+{
+  cgraph_node *node = NULL;
+  std::set nodes;
+  std::set leaf_nodes;
+  std::set leaf_nodes_decl;
+  FOR_EACH_FUNCTION_WITH_GIMPLE_BODY (node)
+  {
+node->get_untransformed_body ();
+nodes.insert(node);
+if (node->callees) continue;
+
+leaf_nodes.insert (node);
+leaf_nodes_decl.insert (node->decl);
+  }
+
+  std::queue worklist;
+  for (std::set::iterator i = leaf_nodes.begin (),
+e = leaf_nodes.end (); i != e; ++i)
+  {
+if (dump_file) fprintf (dump_file, "is a leaf node %s\n", 
(*i)->name ());

+worklist.push (*i);
+  }
+
+  for (std::set::iterator i = nodes.begin (),
+e = nodes.end (); i != e; ++i)
+  {
+worklist.push (*i);
+  }
+
+  std::map map;
+  while (!worklist.empty ())
+  {
+
+if (detected_incompatible_syntax) return map;
+cgraph_node *i = worklist.front ();
+worklist.pop ();
+if (dump_file) fprintf (dump_file, "analyzing %s %p\n", i->name (), 
(void*)i);

+GimpleWhiteLister whitelister;
+whitelister._walk_cnode (i);
+bool no_external = whitelister.does_not_call_external_functions (i, 
map);

+bool before_in_map = map.find (i->decl) != map.end ();
+bool place_callers_in_worklist = !before_in_map;
+if (!before_in_map)
+{
+  map.insert(std::pair(i->decl, no_external));
+} else
+{
+  map[i->decl] = no_external;
+}
+bool previous_value = map[i->decl];
+place_callers_in_worklist |= previous_value != 

  1   2   >