[PATCH] dwarf2out: Fix debug info for 2 byte floats [PR99388]

2021-03-05 Thread Jakub Jelinek via Gcc-patches
Hi!

Aarch64, ARM and a couple of other architectures have 16-bit floats, HFmode.
As can be seen e.g. on
void
foo (void)
{
  __fp16 a = 1.0;
  asm ("nop");
  a = 2.0;
  asm ("nop");
  a = 3.0;
  asm ("nop");
}
testcase, GCC mishandles this on the dwarf2out.c side by assuming all
floating point types have sizes in multiples of 4 bytes, so what GCC emits
is it says that e.g. the DW_OP_implicit_value will be 2 bytes but then
doesn't emit anything and so anything emitted after it is treated by
consumers as the value and then they get out of sync.
real_to_target which insert_float uses indeed fills it that way, but putting
into an array of long 32 bits each time, but for the half floats it puts
everything into the least significant 16 bits of the first long no matter
what endianity host or target has.

The following patch fixes it.  With the patch the -g -O2 -dA output changes
(in a cross without .uleb128 support):
.byte   0x9e// DW_OP_implicit_value
.byte   0x2 // uleb128 0x2
+   .2byte  0x3c00  // fp or vector constant word 0
.byte   0x7 // DW_LLE_start_end (*.LLST0)
.8byte  .LVL1   // Location list begin address (*.LLST0)
.8byte  .LVL2   // Location list end address (*.LLST0)
.byte   0x4 // uleb128 0x4; Location expression size
.byte   0x9e// DW_OP_implicit_value
.byte   0x2 // uleb128 0x2
+   .2byte  0x4000  // fp or vector constant word 0
.byte   0x7 // DW_LLE_start_end (*.LLST0)
.8byte  .LVL2   // Location list begin address (*.LLST0)
.8byte  .LFE0   // Location list end address (*.LLST0)
.byte   0x4 // uleb128 0x4; Location expression size
.byte   0x9e// DW_OP_implicit_value
.byte   0x2 // uleb128 0x2
+   .2byte  0x4200  // fp or vector constant word 0
.byte   0   // DW_LLE_end_of_list (*.LLST0)

Bootstrapped/regtested on x86_64-linux, aarch64-linux and
armv7hl-linux-gnueabi, ok for trunk?

I fear the CONST_VECTOR case is still broken, while HFmode elements of vectors
should be fine (it uses eltsize of the element sizes) and likewise SFmode could
be fine, DFmode vectors are emitted as two 32-bit ints regardless of endianity
and I'm afraid it can't be right on big-endian.  But I haven't been able to
create a testcase that emits a CONST_VECTOR, for e.g. unused vector vars
with constant operands we emit CONCATN during expansion and thus ...
DW_OP_*piece for each element of the vector and for
DW_TAG_call_site_parameter we give up (because we handle CONST_VECTOR only
in loc_descriptor, not mem_loc_descriptor).

2021-03-04  Jakub Jelinek  

PR debug/99388
* dwarf2out.c (insert_float): Change return type from void to
unsigned, handle GET_MODE_SIZE (mode) == 2 and return element size.
(mem_loc_descriptor, loc_descriptor, add_const_value_attribute):
Adjust callers.

--- gcc/dwarf2out.c.jj  2021-03-03 09:53:55.391191719 +0100
+++ gcc/dwarf2out.c 2021-03-04 15:51:23.174396552 +0100
@@ -3825,7 +3825,7 @@ static void add_data_member_location_att
 static bool add_const_value_attribute (dw_die_ref, rtx);
 static void insert_int (HOST_WIDE_INT, unsigned, unsigned char *);
 static void insert_wide_int (const wide_int &, unsigned char *, int);
-static void insert_float (const_rtx, unsigned char *);
+static unsigned insert_float (const_rtx, unsigned char *);
 static rtx rtl_for_decl_location (tree);
 static bool add_location_or_const_value_attribute (dw_die_ref, tree, bool);
 static bool tree_add_const_value_attribute (dw_die_ref, tree);
@@ -16292,11 +16292,12 @@ mem_loc_descriptor (rtx rtl, machine_mod
  scalar_float_mode float_mode = as_a  (mode);
  unsigned int length = GET_MODE_SIZE (float_mode);
  unsigned char *array = ggc_vec_alloc (length);
+ unsigned int elt_size = insert_float (rtl, array);
 
- insert_float (rtl, array);
  mem_loc_result->dw_loc_oprnd2.val_class = dw_val_class_vec;
- mem_loc_result->dw_loc_oprnd2.v.val_vec.length = length / 4;
- mem_loc_result->dw_loc_oprnd2.v.val_vec.elt_size = 4;
+ mem_loc_result->dw_loc_oprnd2.v.val_vec.length
+   = length / elt_size;
+ mem_loc_result->dw_loc_oprnd2.v.val_vec.elt_size = elt_size;
  mem_loc_result->dw_loc_oprnd2.v.val_vec.array = array;
}
}
@@ -16866,11 +16867,11 @@ loc_descriptor (rtx rtl, machine_mode mo
{
  unsigned int length = GET_MODE_SIZE (smode);
  unsigned char *array = ggc_vec_alloc (length);
+ unsigned int elt_size = insert_float (rtl, array);
 
- insert_float (rtl, array);
  loc_result->dw_loc_oprnd2.val_class = dw_val_class_vec;
- loc_result->dw_loc_oprnd2.v.val_vec.length = length / 4;
- loc_result->dw_loc_oprnd2.v.val_vec.elt_size = 4;
+ loc_result->dw_loc_oprnd2.

Re: [patch] Fix PR rtl-optimization/99376

2021-03-05 Thread Richard Biener via Gcc-patches
On Thu, Mar 4, 2021 at 7:28 PM Eric Botcazou  wrote:
>
> Hi,
>
> this is an undefined behavior spotted by the sanitizer that has managed to go
> unnoticed until now.  Tested on x86-64/Linux, OK for the mainline?

OK.

>
> 2021-03-04  Eric Botcazou  
>
> PR rtl-optimization/99376
> * rtlanal.c (nonzero_bits1) : If the number
> of low-order zero bits is too large, set the result to 0 directly.
>
> --
> Eric Botcazou


Re: [AArch64] Fix vector multiplication costs

2021-03-05 Thread Christophe Lyon via Gcc-patches
On Mon, 8 Feb 2021 at 18:10, Kyrylo Tkachov via Gcc-patches
 wrote:
>
>
>
> > -Original Message-
> > From: Andre Vieira (lists) 
> > Sent: 03 February 2021 17:59
> > To: gcc-patches@gcc.gnu.org
> > Cc: Kyrylo Tkachov 
> > Subject: [AArch64] Fix vector multiplication costs
> >
> > This patch introduces a vect.mul RTX cost and decouples the vector
> > multiplication costing from the scalar one.
> >
> > After Wilco's "AArch64: Add cost table for Cortex-A76" patch we saw a
> > regression in vector codegen. Reproduceable with the small test added in
> > this patch.
> > Upon further investigation we noticed 'aarch64_rtx_mult_cost' was using
> > scalar costs to calculate the cost of vector multiplication, which was
> > now lower and preventing 'choose_mult_variant' from making the right
> > choice to expand such vector multiplications with constants as shift and
> > sub's. I also added a special case for SSRA to use the default vector
> > cost rather than mult, SSRA seems to be cost using
> > 'aarch64_rtx_mult_cost', which to be fair is quite curious. I believe we
> > should have a better look at 'aarch64_rtx_costs' altogether and
> > completely decouple vector and scalar costs. Though that is something
> > that requires more rewriting than I believe should be done in Stage 4.
> >
> > I gave all targets a vect.mult cost of 4x the vect.alu cost, with the
> > exception of targets with cost 0 for vect.alu, those I gave the cost 4.
> >
> > Bootstrapped on aarch64.
> >
> > Is this OK for trunk?
>
> Ok.
> Thanks,
> Kyrill
>
> >
> > gcc/ChangeLog:
> >
> >  * config/aarch64/aarch64-cost-tables.h: Add entries for vect.mul.
> >  * config/aarch64/aarch64.c (aarch64_rtx_mult_cost): Use
> > vect.mul for
> >  vector multiplies and vect.alu for SSRA.
> >  * config/arm/aarch-common-protos.h (struct vector_cost_table):
> > Define
> >  vect.mul cost field.
> >  * config/arm/aarch-cost-tables.h: Add entries for vect.mul.
> >  * config/arm/arm.c: Likewise.
> >
> > gcc/testsuite/ChangeLog:
> >
> >  * gcc.target/aarch64/asimd-mul-to-shl-sub.c: New test.
>


Hi Andre,

It seems you forgot to update a test, because I've noticed these
failures since you committed this patch:
FAIL: gcc.target/aarch64/sve/mul_2.c -march=armv8.2-a+sve
scan-assembler-times \\tmul\\tz[0-9]+\\.b, z[0-9]+\\.b, #-120\\n 1
FAIL: gcc.target/aarch64/sve/mul_2.c -march=armv8.2-a+sve
scan-assembler-times \\tmul\\tz[0-9]+\\.h, z[0-9]+\\.h, #-128\\n 2
FAIL: gcc.target/aarch64/sve/mul_2.c -march=armv8.2-a+sve
scan-assembler-times \\tmul\\tz[0-9]+\\.s, z[0-9]+\\.s, #-128\\n 1

Am I missing something?

Thanks,

Christophe


Re: [PATCH] Extract a common logger from jit and analyzer frameworks

2021-03-05 Thread Philip Herron
On 04/03/2021 16:45, David Malcolm wrote:
> On Thu, 2021-03-04 at 16:17 +, Philip Herron wrote:
>> In development of the Rust FE logging is useful in debugging. Instead
>> of
>> rolling our own logger it became clear the loggers part of analyzer
>> and jit
>> projects could be extracted and made generic so they could be reused
>> in
>> other projects.
> Thanks for putting together this patch.
>
>> To test this patch make check-jit was invoked, for analyzer the
>> following
>> flags were used -fanalyzer -fdump-analyzer -fanalyzer-verbosity=4.
> "-fanalyzer-verbosity=" only affects what events appear in diagnostic
> paths from the analyzer; it doesn't directly affect the logging (it
> does indirectly, I suppose, since there are logging messages per-event
> as they are processed)
>
>> gcc/jit/ChangeLog:
>>
>>     * jit-logging.h: has been extracted out to gcc/logging.h
>>
>> gcc/analyzer/ChangeLog:
>>
>>     * analyzer-logging.h: has been extract out to gcc/logging.h
>>
>> gcc/ChangeLog:
>>
>>     * logging.h: added new generic logger based off analyzer's
>> logger
> The ChangeLog entries are incomplete, and so the git hooks will
> probably complain when you try to push this.
>
> Various notes inline below...
>
> [...snip...]
>  
>> diff --git a/gcc/analyzer/analyzer.h b/gcc/analyzer/analyzer.h
>> index f50ac662516..87193268b10 100644
>> --- a/gcc/analyzer/analyzer.h
>> +++ b/gcc/analyzer/analyzer.h
>> @@ -23,6 +23,16 @@ along with GCC; see the file COPYING3.  If not see
>>  
>>  class graphviz_out;
>>  
>> +namespace gcc {
>> +  class logger;
>> +  class log_scope;
>> +  class log_user;
>> +}
>> +
>> +using gcc::logger;
>> +using gcc::log_scope;
>> +using gcc::log_user;
> Are the "using" decls for log_scope and log_user needed?  I know that
> "logger" is used in a bunch of places, so the "using" decl for that is
> probably useful, but my guess is that the other two are barely used in
> the analyzer code, if at all.
>
> [...]
>
>> diff --git a/gcc/analyzer/analyzer-logging.cc b/gcc/logging.c
>> similarity index 76%
>> rename from gcc/analyzer/analyzer-logging.cc
>> rename to gcc/logging.c
>> index 297319069f8..630a16d19dd 100644
>> --- a/gcc/analyzer/analyzer-logging.cc
>> +++ b/gcc/logging.c
> [...]
>>  /* Implementation of class logger.  */
>>  
>>  /* ctor for logger.  */
>>  
>> -logger::logger (FILE *f_out,
>> -   int, /* flags */
>> -   int /* verbosity */,
>> -   const pretty_printer &reference_pp) :
>> -  m_refcount (0),
>> -  m_f_out (f_out),
>> -  m_indent_level (0),
>> -  m_log_refcount_changes (false),
>> -  m_pp (reference_pp.clone ())
>> +logger::logger (FILE *f_out, int, /* flags */
>> +   int /* verbosity */, const pretty_printer
>> *reference_pp,
>> +   const char *module)
>> +  : m_refcount (0), m_f_out (f_out), m_indent_level (0),
>> +    m_log_refcount_changes (false), m_pp (reference_pp->clone ()),
>> +    m_mod (module)
>>  {
>>    pp_show_color (m_pp) = 0;
>>    pp_buffer (m_pp)->stream = f_out;
>> @@ -61,6 +57,15 @@ logger::logger (FILE *f_out,
>>    print_version (f_out, "", false);
>>  }
>>  
>> +logger::logger (FILE *f_out, int, /* flags */
>> +   int /* verbosity */, const char *module)
>> +  : m_refcount (0), m_f_out (f_out), m_indent_level (0),
>> +    m_log_refcount_changes (false), m_pp (nullptr), m_mod (module)
>> +{
>> +  /* Begin the log by writing the GCC version.  */
>> +  print_version (f_out, "", false);
>> +}
> Both of these pass the empty string to print_version, but the to-be-
> deleted implementation in jit-logging.c passed "JIT: ".  So I think
> this one needs to read something like:
>
>  print_version (f_out, m_mod ? m_mod : "", false);
>
> or somesuch.
>
> I'm probably bikeshedding, but could module/m_mod be renamed to
> prefix/m_prefix?
>
> I think it would be good to have a leading comment for this new ctor.
> In particular, summarizing our discussion from github, something like:
>
> /* logger ctor for use by libgccjit.
>
>The module param, if non-NULL, is printed as a prefix to all log
>messages.  This is particularly useful for libgccjit, where the
>log lines are often intermingled with messages from the program
>that libgccjit is linked into.  */
>
> or somesuch.
>
>
> [...]
>
>> @@ -144,19 +152,27 @@ logger::log_partial (const char *fmt, ...)
>>  void
>>  logger::log_va_partial (const char *fmt, va_list *ap)
>>  {
>> -  text_info text;
>> -  text.format_spec = fmt;
>> -  text.args_ptr = ap;
>> -  text.err_no = 0;
>> -  pp_format (m_pp, &text);
>> -  pp_output_formatted_text (m_pp);
> I think there's an issue here: what format codes are accepted by this
> function?
>
>> +  if (!has_pretty_printer ())
>> +    vfprintf (m_f_out, fmt, *ap);
> ...here we're formatting using vfprintf...
>
>> +  else
>> +    {
>> +  text_info text;
>> +  text.format_spec = fmt;
>> +  text.args_ptr = ap;
>> +  text.err_no = 0;
>> +  pp_format (m_pp, &text);

[Ada] Fix PR ada/99264

2021-03-05 Thread Eric Botcazou
This fixes the build breakage introduced by the latest glibc release.

Tested on x86-64/Linux, applied on mainline, 10 and 9 branches.


2021-03-05  Eric Botcazou  

PR ada/99264
* init.c (__gnat_alternate_sta) [Linux]: Remove preprocessor test on
MINSIGSTKSZ and bump size to 32KB.
* libgnarl/s-osinte__linux.ads (Alternate_Stack_Size): Bump to 32KB.

-- 
Eric Botcazoudiff --git a/gcc/ada/init.c b/gcc/ada/init.c
index e76aa79c5a8..3ceb1a31b02 100644
--- a/gcc/ada/init.c
+++ b/gcc/ada/init.c
@@ -579,12 +579,8 @@ __gnat_error_handler (int sig, siginfo_t *si ATTRIBUTE_UNUSED, void *ucontext)
 
 #ifndef __ia64__
 #define HAVE_GNAT_ALTERNATE_STACK 1
-/* This must be in keeping with System.OS_Interface.Alternate_Stack_Size.
-   It must be larger than MINSIGSTKSZ and hopefully near 2 * SIGSTKSZ.  */
-# if 16 * 1024 < MINSIGSTKSZ
-#  error "__gnat_alternate_stack too small"
-# endif
-char __gnat_alternate_stack[16 * 1024];
+/* This must be in keeping with System.OS_Interface.Alternate_Stack_Size.  */
+char __gnat_alternate_stack[32 * 1024];
 #endif
 
 #ifdef __XENO__
diff --git a/gcc/ada/libgnarl/s-osinte__linux.ads b/gcc/ada/libgnarl/s-osinte__linux.ads
index f7af00bf5e2..2272f83d68d 100644
--- a/gcc/ada/libgnarl/s-osinte__linux.ads
+++ b/gcc/ada/libgnarl/s-osinte__linux.ads
@@ -328,7 +328,7 @@ package System.OS_Interface is
   oss : access stack_t) return int;
pragma Import (C, sigaltstack, "sigaltstack");
 
-   Alternate_Stack_Size : constant := 16 * 1024;
+   Alternate_Stack_Size : constant := 32 * 1024;
--  This must be in keeping with init.c:__gnat_alternate_stack
 
Alternate_Stack : aliased char_array (1 .. Alternate_Stack_Size);


c++: instantiating imported specializations [PR 99389]

2021-03-05 Thread Nathan Sidwell
When an	incomplete class specialization is imported, and is completed by 
instantiation, we were failing to mark the instantiation, and thus 
didn't stream it out.  Leading to errors in importing as we had members 
of an incomplete type.


PR c++/99389
gcc/cp/
* pt.c (instantiate_class_template_1): Set instantiating module
here.
gcc/testsuite/
* g++.dg/modules/pr99389_a.H: New.
* g++.dg/modules/pr99389_b.C: New.
* g++.dg/modules/pr99389_c.C: New.

--
Nathan Sidwell
diff --git c/gcc/cp/pt.c w/gcc/cp/pt.c
index 83589101c0d..8ca3dc8ec2b 100644
--- c/gcc/cp/pt.c
+++ w/gcc/cp/pt.c
@@ -11816,6 +11816,8 @@ instantiate_class_template_1 (tree type)
   input_location = DECL_SOURCE_LOCATION (TYPE_NAME (type)) =
 DECL_SOURCE_LOCATION (typedecl);
 
+  set_instantiating_module (TYPE_NAME (type));
+
   TYPE_PACKED (type) = TYPE_PACKED (pattern);
   SET_TYPE_ALIGN (type, TYPE_ALIGN (pattern));
   TYPE_USER_ALIGN (type) = TYPE_USER_ALIGN (pattern);
diff --git c/gcc/testsuite/g++.dg/modules/pr99389_a.H w/gcc/testsuite/g++.dg/modules/pr99389_a.H
new file mode 100644
index 000..cbb64df7562
--- /dev/null
+++ w/gcc/testsuite/g++.dg/modules/pr99389_a.H
@@ -0,0 +1,20 @@
+// PR 99389 failed to stream class specialization
+// { dg-module-do link }
+// { dg-additional-options "-fmodule-header" }
+// { dg-module-cmi {} }
+template
+class basic_string_view
+{
+public:
+  basic_string_view(const _CharT* __str) noexcept
+  {}
+  bool
+empty() const noexcept
+  { return !_M_len; }
+  
+private:
+  unsigned _M_len;
+};
+
+using string_view = basic_string_view;
+
diff --git c/gcc/testsuite/g++.dg/modules/pr99389_b.C w/gcc/testsuite/g++.dg/modules/pr99389_b.C
new file mode 100644
index 000..f8d010e453d
--- /dev/null
+++ w/gcc/testsuite/g++.dg/modules/pr99389_b.C
@@ -0,0 +1,12 @@
+// { dg-additional-options {-fmodules-ts -fdump-lang-module } }
+export module hello;
+// { dg-module-cmi hello }
+
+import "pr99389_a.H";
+
+export inline bool Check (const string_view& n)
+{
+  return !n.empty ();
+}
+
+// { dg-final { scan-lang-dump {Pending specialization '::basic_string_view' entity:. section:. keyed to '::basic_string_view'} module } }
diff --git c/gcc/testsuite/g++.dg/modules/pr99389_c.C w/gcc/testsuite/g++.dg/modules/pr99389_c.C
new file mode 100644
index 000..f31847dd928
--- /dev/null
+++ w/gcc/testsuite/g++.dg/modules/pr99389_c.C
@@ -0,0 +1,7 @@
+// { dg-additional-options -fmodules-ts }
+import hello;
+
+int main ()
+{
+  return Check ("World") ? 0 : 1;
+}


[PATCH] libgcov: Fix build on Darwin [PR99406]

2021-03-05 Thread Jakub Jelinek via Gcc-patches
Hi!

As reported, bootstrap currently fails on older Darwin because MAP_ANONYMOUS
is not defined.

The following is what gcc/system.h does, so I think it should work for
libgcov.
Build tested on x86_64-linux, ok for trunk?

2021-03-05  Jakub Jelinek  

PR gcov-profile/99406
* libgcov.h (MAP_FAILED, MAP_ANONYMOUS): If HAVE_SYS_MMAN_H is
defined, define these macros if not defined already.

--- libgcc/libgcov.h.jj 2021-03-04 19:36:52.931789790 +0100
+++ libgcc/libgcov.h2021-03-05 14:41:18.912291100 +0100
@@ -172,6 +172,16 @@ extern struct gcov_info *gcov_list;
 #define ATTRIBUTE_HIDDEN
 #endif
 
+#if HAVE_SYS_MMAN_H
+#ifndef MAP_FAILED
+#define MAP_FAILED ((void *)-1)
+#endif
+
+#if !defined (MAP_ANONYMOUS) && defined (MAP_ANON)
+#define MAP_ANONYMOUS MAP_ANON
+#endif
+#endif
+
 #include "gcov-io.h"
 
 /* Structures embedded in coveraged program.  The structures generated

Jakub



Re: [PATCH] Extract a common logger from jit and analyzer frameworks

2021-03-05 Thread David Malcolm via Gcc-patches
On Fri, 2021-03-05 at 10:58 +, Philip Herron wrote:
> On 04/03/2021 16:45, David Malcolm wrote:
> > On Thu, 2021-03-04 at 16:17 +, Philip Herron wrote:
> > > In development of the Rust FE logging is useful in debugging.
> > > Instead
> > > of
> > > rolling our own logger it became clear the loggers part of
> > > analyzer
> > > and jit
> > > projects could be extracted and made generic so they could be
> > > reused
> > > in
> > > other projects.

[...]

> > [...]
> > 
> > > @@ -144,19 +152,27 @@ logger::log_partial (const char *fmt, ...)
> > >  void
> > >  logger::log_va_partial (const char *fmt, va_list *ap)
> > >  {
> > > -  text_info text;
> > > -  text.format_spec = fmt;
> > > -  text.args_ptr = ap;
> > > -  text.err_no = 0;
> > > -  pp_format (m_pp, &text);
> > > -  pp_output_formatted_text (m_pp);
> > I think there's an issue here: what format codes are accepted by
> > this
> > function?
> > 
> > > +  if (!has_pretty_printer ())
> > > +    vfprintf (m_f_out, fmt, *ap);
> > ...here we're formatting using vfprintf...
> > 
> > > +  else
> > > +    {
> > > +  text_info text;
> > > +  text.format_spec = fmt;
> > > +  text.args_ptr = ap;
> > > +  text.err_no = 0;
> > > +  pp_format (m_pp, &text);
> > ...whereas here we're formatting using pp_format, which has
> > different
> > conventions.
> > 
> > The jit implementation decls have e.g.:
> >    GNU_PRINTF(2, 3);
> > whereas the analyzer decls have e.g.:
> >    ATTRIBUTE_GCC_DIAG(2, 3);
> > 
> > I'm not quite sure how to square this circle - templates?  Gah.
> > 
> > I guess the analyzer implementation drifted away from the jit
> > implementation (the analyzer code was originally copied from the
> > jit
> > code).  Sorry about that.
> > 
> > [...]
> > 
> > Hope this is constructive
> > Dave
> 
> Hi David,
> 
> Thanks for the great feedback and for reviewing my earlier attempts.
> It
> seems as though there are really two distinct loggers one with a
> pretty
> printer and one without. Maybe there is a case for creating
> BaseLogger
> which is using GNU_PRINTF and vfprintf etc. Then a
> PrettyPrinterLogger
> based on the BaseLogger but allows for the extensions and the pretty
> printer I think could solve this.

Nit: we don't use CamelCase in GCC [1]

>  Though at that point maybe it is
> getting too complex. 

Agreed, that would be too complicated.

> For gccrs I am interested in the logger in the
> analyzer since it can give such detailed output on the trees

FWIW, I'm not quite sure what you mean by the above.

>  which could
> be useful so maybe just extracting that is the right thing to do for
> GCC
> projects.
> 
> What do you think?

I don't think the jit logger exposes the formatted printing API to end-
users, so at some point it ought to be possible to port the jit logger
from vfprintf to pretty_printer.

How's this for a plan? - for the Rust FE you extract just the analyzer
logging, and at some later point I can port the jit logging over to the
shared gccrs/analyzer logging (so that the Rust FE project doesn't need
to depend on the jit porting being done).  Obviously it would be better
to have just one hierarchical logging framework rather than two, but at
least this way we don't have three, and it gives us a path to getting
down to a single one, and hopefully is sufficiently non-controversial
to not make it harder to (eventually) get the Rust FE merged into
mainline gcc.

Thoughts?

Hope this sounds sane
Dave

[1] except in template declarations, for template arguments



[PATCH] Ada: hashed container Cursor type predefined equality non-conformance

2021-03-05 Thread Richard Wai
Hi,

 

We discovered an issue with the GNAT implementation of the hashed container
types.

 

The RM states (A.18-4-18/2, A.18.7-17/2, et al) that "the predefined "="
operator for type Cursor returns True if both cursors are No_Element, or
designate the same element in the same container."

 

In some cases, GNAT's implementation violates this requirement. This was due
to the component "Position" of the Cursor type in Hashed_Sets, Hashed_Maps,
and Indefinite_Hashed_Maps (though interestingly not in
Indefinite_Hashed_Sets). The Position component is used to store the
position of a node in a bucket, and is used internally as an optimization.
Since it was viewed as an optimization, it was only updated
opportunistically. However, this effects the predefined equality for the
type. The result was that various Cursor objects could be returned which
designated the same element in the same container, but yet evaluated as
inequal.

 

The attached patch ensures that the Position value is always updated when a
Cursor object is returned or modified. It also synchronizes comments for the
Cursor type definition across the various packages. Additionally, a new
regression test case is added that checks for this issue among all four of
the hashed container packages.

 

This was successfully bootstrapped and tested on trunk,
x86_64-unknown-freebsd12.2.

 

Cheers,

 

Richard Wai

ANNEXI-STRAYLINE



container_cursor_equality_20210304.patch
Description: Binary data


[PATCH] ipa: Check that scalar types that IPA-CP comes up with are sane (PR99122)

2021-03-05 Thread Martin Jambor
Hi,

this patch fixes the last bit of PR 99122 where various bits of IPA
infrastructure are presented with a program with type mismatches that
make it have undefined behavior, and when inlining or performing
IPA-CP, and encountering such mismatch, we basically try to
VIEW_CONVERT_EXPR whatever the caller has into whatever the callee has
or simply use an empty constructor if that cannot be done.  This
however does not work when the callee has VLA parameters because we
ICE in the process.

Richi has already disabled inlining for such cases, this patch avoids
the issue in IPA-CP.  It adds checks that whatever constant the
propagation arrived at is actually compatible or fold_convertible to
the callees formal parameer type.  Unlike in the past, we now have
types of all parameters of functions that we have analyzed, even with
LTO, and so can do it.

This should prevent only bogus propagations.  I have looked at the
effect of the patch on WPA of Firefox and did not have any.

I have bootstrapped and LTO bootstrapped and tested the patch on
x86_64-linux.  OK for trunk?  And perhaps later for GCC 10 too?

Thanks

gcc/ChangeLog:

2021-02-26  Martin Jambor  

PR ipa/99122
* ipa-cp.c (initialize_node_lattices): Mark as bottom all
parameters with unknown type.
(ipacp_value_safe_for_type): New function.
(propagate_vals_across_arith_jfunc): Verify that the constant type
can be used for a type of the formal parameter.
(propagate_vals_across_ancestor): Likewise.
(propagate_scalar_across_jump_function): Likewise.  Pass the type
also to propagate_vals_across_ancestor.

gcc/testsuite/ChangeLog:

2021-02-26  Martin Jambor  

PR ipa/99122
* gcc.dg/pr99122-3.c: Remove -fno-ipa-cp from options.
---
 gcc/ipa-cp.c | 37 ++--
 gcc/testsuite/gcc.dg/pr99122-3.c |  2 +-
 2 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index 167913cb927..6041f75d824 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -1276,6 +1276,7 @@ initialize_node_lattices (struct cgraph_node *node)
 {
   ipcp_param_lattices *plats = ipa_get_parm_lattices (info, i);
   if (disable
+ || !ipa_get_type (info, i)
  || (pre_modified && (surviving_params.length () <= (unsigned) i
   || !surviving_params[i])))
{
@@ -1304,6 +1305,21 @@ initialize_node_lattices (struct cgraph_node *node)
   }
 }
 
+/* Return true if VALUE can be safely IPA-CP propagated to a parameter of type
+   PARAM_TYPE.  */
+
+static bool
+ipacp_value_safe_for_type (tree param_type, tree value)
+{
+  tree val_type = TREE_TYPE (value);
+  if (param_type == val_type
+  || useless_type_conversion_p (param_type, val_type)
+  || fold_convertible_p (param_type, value))
+return true;
+  else
+return false;
+}
+
 /* Return true iff X and Y should be considered equal values by IPA-CP.  */
 
 static bool
@@ -2072,7 +2088,8 @@ propagate_vals_across_arith_jfunc (cgraph_edge *cs,
{
  tree cstval = get_val_across_arith_op (opcode, opnd1_type, opnd2,
 src_val, res_type);
- if (!cstval)
+ if (!cstval
+ || !ipacp_value_safe_for_type (res_type, cstval))
break;
 
  ret |= dest_lat->add_value (cstval, cs, src_val, src_idx,
@@ -2096,7 +2113,8 @@ propagate_vals_across_arith_jfunc (cgraph_edge *cs,
 
tree cstval = get_val_across_arith_op (opcode, opnd1_type, opnd2,
   src_val, res_type);
-   if (cstval)
+   if (cstval
+   && ipacp_value_safe_for_type (res_type, cstval))
  ret |= dest_lat->add_value (cstval, cs, src_val, src_idx,
  src_offset);
else
@@ -2132,7 +2150,8 @@ static bool
 propagate_vals_across_ancestor (struct cgraph_edge *cs,
struct ipa_jump_func *jfunc,
ipcp_lattice *src_lat,
-   ipcp_lattice *dest_lat, int src_idx)
+   ipcp_lattice *dest_lat, int src_idx,
+   tree param_type)
 {
   ipcp_value *src_val;
   bool ret = false;
@@ -2144,7 +2163,7 @@ propagate_vals_across_ancestor (struct cgraph_edge *cs,
 {
   tree t = ipa_get_jf_ancestor_result (jfunc, src_val->value);
 
-  if (t)
+  if (t && ipacp_value_safe_for_type (param_type, t))
ret |= dest_lat->add_value (t, cs, src_val, src_idx);
   else
ret |= dest_lat->set_contains_variable ();
@@ -2169,7 +2188,10 @@ propagate_scalar_across_jump_function (struct 
cgraph_edge *cs,
   if (jfunc->type == IPA_JF_CONST)
 {
   tree val = ipa_get_jf_constant (jfunc);
-  return dest_lat->add_value (val, cs, NULL, 0);
+  if (ipacp_value_safe_for_type (param_type, val))
+ 

Re: [PATCH] Extract a common logger from jit and analyzer frameworks

2021-03-05 Thread Philip Herron

On 05/03/2021 14:18, David Malcolm wrote:
> On Fri, 2021-03-05 at 10:58 +, Philip Herron wrote:
>> On 04/03/2021 16:45, David Malcolm wrote:
>>> On Thu, 2021-03-04 at 16:17 +, Philip Herron wrote:
 In development of the Rust FE logging is useful in debugging.
 Instead
 of
 rolling our own logger it became clear the loggers part of
 analyzer
 and jit
 projects could be extracted and made generic so they could be
 reused
 in
 other projects.
> [...]
>
>>> [...]
>>>
 @@ -144,19 +152,27 @@ logger::log_partial (const char *fmt, ...)
  void
  logger::log_va_partial (const char *fmt, va_list *ap)
  {
 -  text_info text;
 -  text.format_spec = fmt;
 -  text.args_ptr = ap;
 -  text.err_no = 0;
 -  pp_format (m_pp, &text);
 -  pp_output_formatted_text (m_pp);
>>> I think there's an issue here: what format codes are accepted by
>>> this
>>> function?
>>>
 +  if (!has_pretty_printer ())
 +    vfprintf (m_f_out, fmt, *ap);
>>> ...here we're formatting using vfprintf...
>>>
 +  else
 +    {
 +  text_info text;
 +  text.format_spec = fmt;
 +  text.args_ptr = ap;
 +  text.err_no = 0;
 +  pp_format (m_pp, &text);
>>> ...whereas here we're formatting using pp_format, which has
>>> different
>>> conventions.
>>>
>>> The jit implementation decls have e.g.:
>>>    GNU_PRINTF(2, 3);
>>> whereas the analyzer decls have e.g.:
>>>    ATTRIBUTE_GCC_DIAG(2, 3);
>>>
>>> I'm not quite sure how to square this circle - templates?  Gah.
>>>
>>> I guess the analyzer implementation drifted away from the jit
>>> implementation (the analyzer code was originally copied from the
>>> jit
>>> code).  Sorry about that.
>>>
>>> [...]
>>>
>>> Hope this is constructive
>>> Dave
>> Hi David,
>>
>> Thanks for the great feedback and for reviewing my earlier attempts.
>> It
>> seems as though there are really two distinct loggers one with a
>> pretty
>> printer and one without. Maybe there is a case for creating
>> BaseLogger
>> which is using GNU_PRINTF and vfprintf etc. Then a
>> PrettyPrinterLogger
>> based on the BaseLogger but allows for the extensions and the pretty
>> printer I think could solve this.
> Nit: we don't use CamelCase in GCC [1]
>
>>  Though at that point maybe it is
>> getting too complex. 
> Agreed, that would be too complicated.
>
>> For gccrs I am interested in the logger in the
>> analyzer since it can give such detailed output on the trees
> FWIW, I'm not quite sure what you mean by the above.
>
>>  which could
>> be useful so maybe just extracting that is the right thing to do for
>> GCC
>> projects.
>>
>> What do you think?
> I don't think the jit logger exposes the formatted printing API to end-
> users, so at some point it ought to be possible to port the jit logger
> from vfprintf to pretty_printer.
>
> How's this for a plan? - for the Rust FE you extract just the analyzer
> logging, and at some later point I can port the jit logging over to the
> shared gccrs/analyzer logging (so that the Rust FE project doesn't need
> to depend on the jit porting being done).  Obviously it would be better
> to have just one hierarchical logging framework rather than two, but at
> least this way we don't have three, and it gives us a path to getting
> down to a single one, and hopefully is sufficiently non-controversial
> to not make it harder to (eventually) get the Rust FE merged into
> mainline gcc.
>
> Thoughts?
>
> Hope this sounds sane
> Dave
>
> [1] except in template declarations, for template arguments
>
Hi David,

Extracting the analyzer logger sounds good to me. I was getting anxious
that this patch was going to affect both the analyzer and jit projects,
so its likely safer to make this an incremental change instead of one
big one, when some care might need to be taken.

Thanks for helping out so much.

--Phil




OpenPGP_signature
Description: OpenPGP digital signature


ping^1 Re: [PATCH] Objective-C++ : Fix handling of unnamed message parms [PR49070].

2021-03-05 Thread Iain Sandoe
Although this is an Objective-C++ patch, I need to touch stuff outside  
“objc local”

contexts, so think it needs review,

(this is a regression fix for all open branches).

Iain Sandoe  wrote:

We were discussing proposed reflection splicing syntax - [:reflection:]  
in SG7

which has some similarities with objective-c++ message syntax with unnamed
params.

This reminded me that we have a *very* long-standing regression against
objective-c++ where it is not able to handle unnamed message parameters (an
idiom used in practice).

Fixed as below.
tested on x86_64-darwin, x86_64-linux-gnu,
OK for master / open branches?

thanks
Iain

--

When we are parsing an Objective-C++ message, a colon is a valid
terminator for a assignment-expression.  That is:

[receiver meth:x:x:x:x];

Is a valid, if somewhat unreadable, construction; corresponding
to a method declaration like:

- (id) meth:(id)arg0 :(id)arg1 :(id)arg2 :(id)arg3;

Where three of the message params have no name.

If fact, although it might be unintentional, Objective-C/C++ can
accept message selectors with all the parms unnamed (this applies
to the clang implementation too, which is taken as the reference
for the language).

For regular C++, the pattern x:x is not valid in that position an
an error is emitted with a fixit for the expected scope token.

If we simply made that error conditional on !c_c_dialect_objc()
that would regress Objective-C++ diagnostics for cases outside a
message selector, so we add a state flag for this.

gcc/cp/ChangeLog:

PR objc++/49070
* parser.c (cp_debug_parser): Add Objective-C++ message
state flag.
(cp_parser_nested_name_specifier_opt): Allow colon to
terminate an assignment-expression when parsing Objective-
C++ messages.
(cp_parser_objc_message_expression): Set and clear message
parsing state on entry and exit.
* parser.h (struct cp_parser): Add a context flag for
Objective-C++ message state.

gcc/testsuite/ChangeLog:

PR objc++/49070
* obj-c++.dg/pr49070.mm: New test.
* objc.dg/unnamed-parms.m: New test.
---
gcc/cp/parser.c   |  8 -
gcc/cp/parser.h   |  4 +++
gcc/testsuite/obj-c++.dg/pr49070.mm   | 52 +++
gcc/testsuite/objc.dg/unnamed-parms.m | 28 +++
4 files changed, 91 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/obj-c++.dg/pr49070.mm
create mode 100644 gcc/testsuite/objc.dg/unnamed-parms.m

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 70775792161..e5ba1dcffaa 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -572,6 +572,8 @@ cp_debug_parser (FILE *file, cp_parser *parser)
  parser->colon_corrects_to_scope_p);
  cp_debug_print_flag (file, "Colon doesn't start a class definition",
  parser->colon_doesnt_start_class_def_p);
+  cp_debug_print_flag (file, "Parsing an Objective-C++ message context",
+ parser->objective_c_message_context_p);
  if (parser->type_definition_forbidden_message)
fprintf (file, "Error message for forbidden type definitions: %s %s\n",
 parser->type_definition_forbidden_message,
@@ -6622,7 +6624,9 @@ cp_parser_nested_name_specifier_opt (cp_parser  
*parser,


  if (token->type == CPP_COLON
  && parser->colon_corrects_to_scope_p
- && cp_lexer_peek_nth_token (parser->lexer, 3)->type == CPP_NAME)
+ && cp_lexer_peek_nth_token (parser->lexer, 3)->type == CPP_NAME
+ /* name:name is a valid sequence in an Objective C message.  */
+ && !parser->objective_c_message_context_p)
{
  gcc_rich_location richloc (token->location);
  richloc.add_fixit_replace ("::");
@@ -32965,6 +32969,7 @@ cp_parser_objc_message_expression (cp_parser*  
parser)

{
  tree receiver, messageargs;

+  parser->objective_c_message_context_p = true;
  location_t start_loc = cp_lexer_peek_token (parser->lexer)->location;
  cp_lexer_consume_token (parser->lexer);  /* Eat '['.  */
  receiver = cp_parser_objc_message_receiver (parser);
@@ -32981,6 +32986,7 @@ cp_parser_objc_message_expression (cp_parser*  
parser)

  location_t combined_loc = make_location (start_loc, start_loc, end_loc);
  protected_set_expr_location (result, combined_loc);

+  parser->objective_c_message_context_p = false;
  return result;
}

diff --git a/gcc/cp/parser.h b/gcc/cp/parser.h
index d587cf2404b..a468b6992ad 100644
--- a/gcc/cp/parser.h
+++ b/gcc/cp/parser.h
@@ -350,6 +350,10 @@ struct GTY(()) cp_parser {
 is terminated by colon.  */
  bool colon_doesnt_start_class_def_p;

+  /* TRUE if we are parsing an objective c message, and ':' is permitted
+ to terminate an assignment-expression.  */
+  bool objective_c_message_context_p;
+
  /* If non-NULL, then we are parsing a construct where new type
 definitions are not permitted.  The stri

Re: [PATCH] libgcov: Fix build on Darwin [PR99406]

2021-03-05 Thread Iain Sandoe

Jakub Jelinek via Gcc-patches  wrote:

As reported, bootstrap currently fails on older Darwin because  
MAP_ANONYMOUS

is not defined.

The following is what gcc/system.h does, so I think it should work for
libgcov.
Build tested on x86_64-linux, ok for trunk?


bootstrap suceeded r11-7524 + this patch on Darwin11.
thanks,
Iain



2021-03-05  Jakub Jelinek  

PR gcov-profile/99406
* libgcov.h (MAP_FAILED, MAP_ANONYMOUS): If HAVE_SYS_MMAN_H is
defined, define these macros if not defined already.

--- libgcc/libgcov.h.jj 2021-03-04 19:36:52.931789790 +0100
+++ libgcc/libgcov.h2021-03-05 14:41:18.912291100 +0100
@@ -172,6 +172,16 @@ extern struct gcov_info *gcov_list;
#define ATTRIBUTE_HIDDEN
#endif

+#if HAVE_SYS_MMAN_H
+#ifndef MAP_FAILED
+#define MAP_FAILED ((void *)-1)
+#endif
+
+#if !defined (MAP_ANONYMOUS) && defined (MAP_ANON)
+#define MAP_ANONYMOUS MAP_ANON
+#endif
+#endif
+
#include "gcov-io.h"

/* Structures embedded in coveraged program.  The structures generated

Jakub





[PATCH] c++: ICE with -Wshadow and enumerator in template [PR99120]

2021-03-05 Thread Marek Polacek via Gcc-patches
We crash here, because in a template, an enumerator doesn't have
a type until we've called finish_enum_value_list.  But our -Wshadow
implementation, check_local_shadow, is called when we pushdecl in
build_enumerator, which takes place before finish_enum_value_list.

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

gcc/cp/ChangeLog:

PR c++/99120
* name-lookup.c (check_local_shadow): Check if the type of decl
is non-null before checking TYPE_PTR*.

gcc/testsuite/ChangeLog:

PR c++/99120
* g++.dg/warn/Wshadow-17.C: New test.
---
 gcc/cp/name-lookup.c   |  7 ---
 gcc/testsuite/g++.dg/warn/Wshadow-17.C | 11 +++
 2 files changed, 15 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/warn/Wshadow-17.C

diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index f57708700c2..092fa6b8768 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -3309,7 +3309,7 @@ check_local_shadow (tree decl)
   /* Don't warn for artificial things that are not implicit typedefs.  */
   if (DECL_ARTIFICIAL (decl) && !DECL_IMPLICIT_TYPEDEF_P (decl))
 return;
-  
+
   if (nonlambda_method_basetype ())
 if (tree member = lookup_member (current_nonlambda_class_type (),
 DECL_NAME (decl), /*protect=*/0,
@@ -3321,8 +3321,9 @@ check_local_shadow (tree decl)
   is a function or a pointer-to-function.  */
if (!OVL_P (member)
|| TREE_CODE (decl) == FUNCTION_DECL
-   || TYPE_PTRFN_P (TREE_TYPE (decl))
-   || TYPE_PTRMEMFUNC_P (TREE_TYPE (decl)))
+   || (TREE_TYPE (decl)
+   && (TYPE_PTRFN_P (TREE_TYPE (decl))
+   || TYPE_PTRMEMFUNC_P (TREE_TYPE (decl)
  {
auto_diagnostic_group d;
if (warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wshadow,
diff --git a/gcc/testsuite/g++.dg/warn/Wshadow-17.C 
b/gcc/testsuite/g++.dg/warn/Wshadow-17.C
new file mode 100644
index 000..0dee397796f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wshadow-17.C
@@ -0,0 +1,11 @@
+// PR c++/99120
+// { dg-options "-Wshadow" }
+
+struct S {
+  void X();
+
+  template
+  void fn () {
+enum { X };
+  }
+};

base-commit: 4d66685e49d20e0c7a87c5fa0757c7eb63ffcdaa
-- 
2.29.2



[commited] [PR99378] LRA: Skip decomposing address for asm insn operand with unknown constraint

2021-03-05 Thread Vladimir Makarov via Gcc-patches

  The following patch fixes

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99378

  The patch was successfully bootstrapped and tested on x86-64.


commit e786c7547eda4edd90797f6cae0f5e6405d64773 (HEAD -> master)
Author: Vladimir N. Makarov 
Date:   Fri Mar 5 11:41:25 2021 -0500

[PR99378] LRA: Skip decomposing address for asm insn operand with unknown constraint.

  Function get_constraint_type returns CT__UNKNOWN for empty constraint
and CT_FIXED_FORM for "X".  So process_address_1 skipped
decompose_mem_address only for "X" constraint.  To do the same for empty
constraint, skip decompose_mem_address for CT__UNKNOWN.

gcc/ChangeLog:

PR target/99378
* lra-constraints.c (process_address_1): Skip decomposing address
for asm insn operand with unknown constraint.

gcc/testsuite/ChangeLog:

PR target/99378
* gcc.target/i386/pr99123-2.c: New.

diff --git a/gcc/lra-constraints.c b/gcc/lra-constraints.c
index 51acf7f0701..9253690561a 100644
--- a/gcc/lra-constraints.c
+++ b/gcc/lra-constraints.c
@@ -3450,8 +3450,9 @@ process_address_1 (int nop, bool check_only_p,
  i.e. bcst_mem_operand in i386 backend.  */
   else if (MEM_P (mem)
 	   && !(INSN_CODE (curr_insn) < 0
-		&& get_constraint_type (cn) == CT_FIXED_FORM
-	&& constraint_satisfied_p (op, cn)))
+		&& (cn == CONSTRAINT__UNKNOWN
+		|| (get_constraint_type (cn) == CT_FIXED_FORM
+			&& constraint_satisfied_p (op, cn)
 decompose_mem_address (&ad, mem);
   else if (GET_CODE (op) == SUBREG
 	   && MEM_P (SUBREG_REG (op)))
diff --git a/gcc/testsuite/gcc.target/i386/pr99123-2.c b/gcc/testsuite/gcc.target/i386/pr99123-2.c
new file mode 100644
index 000..def4eae3c9d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr99123-2.c
@@ -0,0 +1,51 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -funroll-loops" } */
+
+static inline void *
+baz (void *s, unsigned long c, unsigned int count)
+{
+  int d0, d1;
+  __asm__ __volatile__ (""
+: "=&c" (d0), "=&D" (d1)
+:"a" (c), "q" (count), "0" (count / 4), "" ((long) s)   /// "1"
+:"memory");
+  return s;
+}
+
+struct A
+{
+  unsigned long *a;
+};
+
+inline static void *
+bar (struct A *x, int y)
+{
+  char *ptr;
+
+  ptr = (void *) x->a[y >> 12];
+  ptr += y % (1UL << 12);
+  return (void *) ptr;
+}
+
+int
+foo (struct A *x, unsigned int *y, int z, int u)
+{
+  int a, b, c, d, e;
+
+  z += *y;
+  c = z + u;
+  a = (z >> 12) + 1;
+  do
+{
+  b = (a << 12);
+  d = b - z;
+  e = c - z;
+  if (e < d)
+d = e;
+  baz (bar (x, z), 0, d);
+  z = b;
+  a++;
+}
+  while (z < c);
+  return 0;
+}


Re: [pushed] c++: Fix class NTTP constness handling [PR98810]

2021-03-05 Thread Eric Botcazou
> Here, when substituting still-dependent args into an alias template, we see
> a non-const type because the default argument is non-const, and is not a
> template parm object because it's still dependent.
> 
> Tested x86_64-pc-linux-gnu, applying to trunk.
> 
> gcc/cp/ChangeLog:
> 
>   PR c++/98810
>   * pt.c (tsubst_copy) [VIEW_CONVERT_EXPR]: Add const
>   to a class non-type template argument that needs it.
> 
> gcc/testsuite/ChangeLog:
> 
>   PR c++/98810
>   * g++.dg/cpp2a/nontype-class-defarg1.C: New test.

This apparently went down to the 9 branch as well and introduced:

Running /home/eric/cvs/gcc-9/gcc/testsuite/g++.dg/dg.exp ...
ERROR: g++.dg/cpp2a/nontype-class-defarg1.C  -std=c++98: syntax error in 
target selector "target c++20" for " dg-do 2 compile { target c++20 } "

ERROR: g++.dg/cpp2a/nontype-class-defarg1.C  -std=c++98: syntax error in 
target selector "target c++20" for " dg-do 2 compile { target c++20 } "
ERROR: g++.dg/cpp2a/nontype-class-defarg1.C  -std=c++14: syntax error in 
target selector "target c++20" for " dg-do 2 compile { target c++20 } "

ERROR: g++.dg/cpp2a/nontype-class-defarg1.C  -std=c++14: syntax error in 
target selector "target c++20" for " dg-do 2 compile { target c++20 } "
ERROR: g++.dg/cpp2a/nontype-class-defarg1.C  -std=c++17: syntax error in 
target selector "target c++20" for " dg-do 2 compile { target c++20 } "

ERROR: g++.dg/cpp2a/nontype-class-defarg1.C  -std=c++17: syntax error in 
target selector "target c++20" for " dg-do 2 compile { target c++20 } "

On the contrary, the 10 branch is a in good shape.

-- 
Eric Botcazou




Re: [PATCH] coroutines : Do not accept throwing final await expressions [PR95616].

2021-03-05 Thread Iain Sandoe

Nathan Sidwell  wrote:


On 3/4/21 2:54 PM, Iain Sandoe wrote:

Hi,
From the PR:
The wording of [dcl.fct.def.coroutine]/15 states:
 * The expression co_await promise.final_suspend() shall not be
   potentially-throwing ([except.spec]).
See http://eel.is/c++draft/dcl.fct.def.coroutine#15
and http://eel.is/c++draft/except.spec#6
ie. all of the following must be declared noexcept (if they form part of  
the await-expression):

- promise_type::final_suspend()
- finalSuspendObj.operator co_await()
- finalSuspendAwaiter.await_ready()
- finalSuspendAwaiter.await_suspend()
- finalSuspendAwaiter.await_resume()
- finalSuspedObj destructor
- finalSuspendAwaiter destructor
This implements the checks for these cases and rejects such code with
a diagnostic.
[ accepts invalid ]
tested on x86_64-darwin, x86_64-linux-gnu,
OK for master / 10.x?
thanks
Iain
gcc/cp/ChangeLog:
PR c++/95616
* coroutines.cc (coro_diagnose_throwing_fn): New helper.
(coro_diagnose_throwing_final_aw_expr): New helper.
(build_co_await): Diagnose throwing final await expression
components.
(build_init_or_final_await): Diagnose a throwing promise
final_suspend() call.


ok.  Does this DTRT in the presence of -fno-exceptions?


thanks for catching this…


 (i.e. use of that flag means you don't have to decorate everything with 
noexcept)


As discussed on IRC, I updated this to gate the diagnostics on  
flag_exceptions (and added three more tests to cover that circumstance).


pushed to master, (for the record, updated version attached),
thanks,
Iain




0001-coroutines-Do-not-accept-throwing-final-await-expres.patch
Description: Binary data





Re: [pushed] c++: Fix class NTTP constness handling [PR98810]

2021-03-05 Thread Marek Polacek via Gcc-patches
On Fri, Mar 05, 2021 at 06:00:04PM +0100, Eric Botcazou wrote:
> > Here, when substituting still-dependent args into an alias template, we see
> > a non-const type because the default argument is non-const, and is not a
> > template parm object because it's still dependent.
> > 
> > Tested x86_64-pc-linux-gnu, applying to trunk.
> > 
> > gcc/cp/ChangeLog:
> > 
> > PR c++/98810
> > * pt.c (tsubst_copy) [VIEW_CONVERT_EXPR]: Add const
> > to a class non-type template argument that needs it.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > PR c++/98810
> > * g++.dg/cpp2a/nontype-class-defarg1.C: New test.
> 
> This apparently went down to the 9 branch as well and introduced:
> 
> Running /home/eric/cvs/gcc-9/gcc/testsuite/g++.dg/dg.exp ...
> ERROR: g++.dg/cpp2a/nontype-class-defarg1.C  -std=c++98: syntax error in 
> target selector "target c++20" for " dg-do 2 compile { target c++20 } "
> 
> ERROR: g++.dg/cpp2a/nontype-class-defarg1.C  -std=c++98: syntax error in 
> target selector "target c++20" for " dg-do 2 compile { target c++20 } "
> ERROR: g++.dg/cpp2a/nontype-class-defarg1.C  -std=c++14: syntax error in 
> target selector "target c++20" for " dg-do 2 compile { target c++20 } "
> 
> ERROR: g++.dg/cpp2a/nontype-class-defarg1.C  -std=c++14: syntax error in 
> target selector "target c++20" for " dg-do 2 compile { target c++20 } "
> ERROR: g++.dg/cpp2a/nontype-class-defarg1.C  -std=c++17: syntax error in 
> target selector "target c++20" for " dg-do 2 compile { target c++20 } "
> 
> ERROR: g++.dg/cpp2a/nontype-class-defarg1.C  -std=c++17: syntax error in 
> target selector "target c++20" for " dg-do 2 compile { target c++20 } "
> 
> On the contrary, the 10 branch is a in good shape.

Fixed:

GCC 9 doesn't have the "c++20" target yet.

* g++.dg/cpp2a/nontype-class-defarg1.C: Use target c++2a.
---
 gcc/testsuite/g++.dg/cpp2a/nontype-class-defarg1.C | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/g++.dg/cpp2a/nontype-class-defarg1.C 
b/gcc/testsuite/g++.dg/cpp2a/nontype-class-defarg1.C
index ddd64d6165b..4e7261a203c 100644
--- a/gcc/testsuite/g++.dg/cpp2a/nontype-class-defarg1.C
+++ b/gcc/testsuite/g++.dg/cpp2a/nontype-class-defarg1.C
@@ -1,5 +1,5 @@
 // PR c++/98810
-// { dg-do compile { target c++20 } }
+// { dg-do compile { target c++2a } }
 
 template  struct a {};
 template  s = a  {}> using b = a ;

base-commit: a5a7cdcaa0c29ee547c41d24f495e9694a6fe7f1
-- 
2.29.2



[PATCH] c++: Fix tsubsting member variable template-id [PR96330]

2021-03-05 Thread Patrick Palka via Gcc-patches
This makes tsubst_copy appropriately handle a variable template-id, which
in turn fixes tsubsting a COMPONENT_REF whose member operand is known at
parse time to be a variable template-id, as in the initialization of 'x'
in the first testcase.  Previously, we rejected this testcase with the
error "foo_t::bar is not a function template", issued from
lookup_template_fuction.

We were already properly handling the analagous case where the object
operand of the COMPONENT_REF is dependent (and so the member operand is
a dependent template name), but there doesn't seems to be existing test
coverage for this, hence the second testcase below.

Bootstrapped and regtested on x86_64-pc-linux-gnu.  Does this look OK
for trunk or perhaps GCC 12?

gcc/cp/ChangeLog:

PR c++/96330
* pt.c (tsubst_copy) : Rename local
variable 'fn' to 'tmpl'.  Handle a variable template-id by
calling lookup_template_variable.

gcc/testsuite/ChangeLog:

PR c++/96330
* g++.dg/cpp1y/var-templ68.C: New test.
* g++.dg/cpp1y/var-templ68a.C: New test.

Co-authored-by: Jakub Jelinek 
---
 gcc/cp/pt.c   |  9 ++---
 gcc/testsuite/g++.dg/cpp1y/var-templ68.C  | 15 +++
 gcc/testsuite/g++.dg/cpp1y/var-templ68a.C | 16 
 3 files changed, 37 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/var-templ68.C
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/var-templ68a.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index ec869451cd2..c956815ce85 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -17108,14 +17108,17 @@ tsubst_copy (tree t, tree args, tsubst_flags_t 
complain, tree in_decl)
 case TEMPLATE_ID_EXPR:
   {
/* Substituted template arguments */
-   tree fn = TREE_OPERAND (t, 0);
+   tree tmpl = TREE_OPERAND (t, 0);
tree targs = TREE_OPERAND (t, 1);
 
-   fn = tsubst_copy (fn, args, complain, in_decl);
+   tmpl = tsubst_copy (tmpl, args, complain, in_decl);
if (targs)
  targs = tsubst_template_args (targs, args, complain, in_decl);
 
-   return lookup_template_function (fn, targs);
+   if (variable_template_p (tmpl))
+ return lookup_template_variable (tmpl, targs);
+   else
+ return lookup_template_function (tmpl, targs);
   }
 
 case TREE_LIST:
diff --git a/gcc/testsuite/g++.dg/cpp1y/var-templ68.C 
b/gcc/testsuite/g++.dg/cpp1y/var-templ68.C
new file mode 100644
index 000..4c560d4bd35
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/var-templ68.C
@@ -0,0 +1,15 @@
+// PR c++/96330
+// { dg-do compile { target c++14 } }
+
+struct foo_t {
+  template  static constexpr bool bar = true;
+};
+constexpr foo_t foo{};
+
+template 
+void f() {
+  int x = foo.bar;
+  int y = foo_t::bar;
+}
+
+template void f();
diff --git a/gcc/testsuite/g++.dg/cpp1y/var-templ68a.C 
b/gcc/testsuite/g++.dg/cpp1y/var-templ68a.C
new file mode 100644
index 000..6091a03a004
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/var-templ68a.C
@@ -0,0 +1,16 @@
+// PR c++/96330
+// { dg-do compile { target c++14 } }
+
+template 
+struct foo_t {
+  template  static constexpr bool bar = true;
+};
+template  constexpr foo_t foo{};
+
+template 
+void f() {
+  int x = foo.template bar;
+  int y = foo_t::template bar;
+}
+
+template void f();
-- 
2.31.0.rc0.75.gec125d1bc1



[PATCH] c++: Fix constexpr evaluation of pre-increment when !lval [PR99287]

2021-03-05 Thread Patrick Palka via Gcc-patches
Here, during cxx_eval_increment_expression (with lval=false) of
++__first where __first is &"mystr"[0], we correctly update __first
to &"mystr"[1] but we end up returning &"mystr"[0] + 1 instead of
&"mystr"[1].  This unreduced return value inhibits other pointer
arithmetic folding during later constexpr evaluation, which ultimately
causes the constexpr evaluation to fail.

It turns out the simplification of &"mystr"[0] + 1 to &"mystr"[1]
is performed by cxx_fold_pointer_plus_expression, not by fold_build2.
So we perform this simplification during constexpr evaluation of
the temporary MODIFY_EXPR (assigning to __first the simplified value),
but then we return 'mod' which has only been folded via fold_build2 and
hasn't gone through cxx_fold_pointer_plus_expression.

This patch fixes this by updating 'mod' to the (rvalue) result of the
MODIFY_EXPR evaluation, so that we capture any additional folding of
'mod'.  We now need to be wary of the evaluation failing and returning
e.g. the MODIFY_EXPR or NULL_TREE; it seems checking *non_constant_p
should cover our bases here and is generally prudent.

(Finally, since returning 'mod' instead of 'op' when !lval seems to be
more than just an optimization, i.e. callers seems to expect this
behavior, this patch additionally clarifies the nearby comment to that
effect.)

Boostrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk or perhaps GCC 12?

gcc/cp/ChangeLog:

PR c++/99287
* constexpr.c (cxx_eval_increment_expression): Pass lval=false
when evaluating the MODIFY_EXPR, and update 'mod' with the
result of this evaluation.  Check *non_constant_p afterwards.
Clarify nearby comment.

gcc/testsuite/ChangeLog:

PR c++/99287
* g++.dg/cpp2a/constexpr-99287.C: New test.

Co-authored-by: Jakub Jelinek 
---
 gcc/cp/constexpr.c   | 16 ++---
 gcc/testsuite/g++.dg/cpp2a/constexpr-99287.C | 61 
 2 files changed, 67 insertions(+), 10 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-99287.C

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index cd0a68e9fd6..49df79837ca 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -5582,20 +5582,16 @@ cxx_eval_increment_expression (const constexpr_ctx 
*ctx, tree t,
   /* Storing the modified value.  */
   tree store = build2_loc (cp_expr_loc_or_loc (t, input_location),
   MODIFY_EXPR, type, op, mod);
-  cxx_eval_constant_expression (ctx, store,
-   true, non_constant_p, overflow_p);
+  mod = cxx_eval_constant_expression (ctx, store, false,
+ non_constant_p, overflow_p);
   ggc_free (store);
+  if (*non_constant_p)
+return t;
 
   /* And the value of the expression.  */
   if (code == PREINCREMENT_EXPR || code == PREDECREMENT_EXPR)
-{
-  /* Prefix ops are lvalues.  */
-  if (lval)
-   return op;
-  else
-   /* But we optimize when the caller wants an rvalue.  */
-   return mod;
-}
+/* Prefix ops are lvalues, but the caller might want an rvalue.  */
+return lval ? op : mod;
   else
 /* Postfix ops are rvalues.  */
 return val;
diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-99287.C 
b/gcc/testsuite/g++.dg/cpp2a/constexpr-99287.C
new file mode 100644
index 000..c9c2ac2f7c2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-99287.C
@@ -0,0 +1,61 @@
+// PR c++/99287
+// { dg-do compile { target c++20 } }
+
+namespace std {
+struct source_location {
+  static consteval source_location
+  current(const void *__p = __builtin_source_location()) {
+source_location __ret;
+__ret._M_impl = static_cast(__p);
+return __ret;
+  }
+  constexpr const char *function_name() {
+return _M_impl ? _M_impl->_M_function_name : "";
+  }
+  struct __impl {
+const char *_M_file_name;
+const char *_M_function_name;
+unsigned _M_line;
+unsigned _M_column;
+  } const *_M_impl;
+};
+struct char_traits {
+  static constexpr long length(const char *__s) {
+return __builtin_strlen(__s);
+  }
+};
+template 
+class basic_string_view {
+public:
+  using traits_type = _Traits;
+  using size_type = unsigned long;
+  constexpr basic_string_view(const _CharT *__str)
+  : _M_len{traits_type::length(__str)}, _M_str{__str} {}
+  constexpr size_type find(const _CharT *, size_type, size_type) const 
noexcept;
+  constexpr size_type find(_CharT *__str) {
+long __trans_tmp_1 = traits_type::length(__str);
+return find(__str, 0, __trans_tmp_1);
+  }
+  long _M_len;
+  const _CharT *_M_str;
+};
+using string_view = basic_string_view;
+template 
+constexpr unsigned long
+basic_string_view<_CharT, _Traits>::find(const _CharT *__str, size_type,
+ size_type __n) const noexcept {
+  int __trans_tmp_2;
+  const _CharT *__first = _M_str;
+  size_type __len = _M_len;
+  while (__len >= __n) {
+__trans_tmp_2 =

[PATCH 1/3] Atomic __platform_wait: accept any 32-bit type, not just int

2021-03-05 Thread Thiago Macieira via Gcc-patches
The kernel doesn't care what we store in those 32 bits, only that they are
comparable. This commit adds:
 * pointers and long on 32-bit architectures
 * unsigned
 * untyped enums and typed enums on int & unsigned int
 * float

We're not using FUTEX_OP anywhere today. The kernel reserves 4 bits for
this field but has only used 6 values so far, so it can be extended to
unsigned compares in the future, if needed.

libstdc++-v3/ChangeLog:

* include/bits/atomic_wait.h: Update __platform_wait_uses_type
---
 libstdc++-v3/include/bits/atomic_wait.h | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/libstdc++-v3/include/bits/atomic_wait.h 
b/libstdc++-v3/include/bits/atomic_wait.h
index 424fccbe4c5..4b4573df691 100644
--- a/libstdc++-v3/include/bits/atomic_wait.h
+++ b/libstdc++-v3/include/bits/atomic_wait.h
@@ -65,7 +65,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 template
   inline constexpr bool __platform_wait_uses_type
 #ifdef _GLIBCXX_HAVE_LINUX_FUTEX
-   = is_same_v, __platform_wait_t>;
+   = sizeof(_Tp) == sizeof(__platform_wait_t) &&
+ alignof(_Tp) >= alignof(__platform_wait_t);
 #else
= false;
 #endif
@@ -91,13 +92,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
 template
   void
-  __platform_wait(const _Tp* __addr, __platform_wait_t __val) noexcept
+  __platform_wait(const _Tp* __addr, _Tp __val) noexcept
   {
for(;;)
  {
auto __e = syscall (SYS_futex, static_cast(__addr),
  
static_cast(__futex_wait_flags::__wait_private),
-   __val, nullptr);
+ static_cast<__platform_wait_t>(__val), 
nullptr);
if (!__e || errno == EAGAIN)
  break;
else if (errno != EINTR)
-- 
2.30.1



[PATCH 0/3] Uncontroversial improvements to C++20 wait-related implementation

2021-03-05 Thread Thiago Macieira via Gcc-patches
Discussion at:
https://gcc.gnu.org/pipermail/libstdc++/2021-February/052043.html

This patch set includes the uncontroversial parts that improve
performance but don't otherwise change ABI.

Please note we still need to decide on how to deal with the future ABI
break.

Thiago Macieira (3):
  Atomic __platform_wait: accept any 32-bit type, not just int
  std::latch: reduce internal implementation from ptrdiff_t to int
  barrier: optimise by not having the hasher in a loop

 libstdc++-v3/include/bits/atomic_wait.h |  7 ---
 libstdc++-v3/include/std/barrier| 10 +-
 libstdc++-v3/include/std/latch  |  4 ++--
 3 files changed, 11 insertions(+), 10 deletions(-)

-- 
2.30.1



[PATCH 3/3] barrier: optimise by not having the hasher in a loop

2021-03-05 Thread Thiago Macieira via Gcc-patches
Our thread's ID does not change so we don't have to get it every time
and hash it every time.

libstdc++-v3/ChangeLog:

* include/std/barrier(arrive): move hasher one level up in the
  stack.
---
 libstdc++-v3/include/std/barrier | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/libstdc++-v3/include/std/barrier b/libstdc++-v3/include/std/barrier
index e09212dfcb9..4bb5642c164 100644
--- a/libstdc++-v3/include/std/barrier
+++ b/libstdc++-v3/include/std/barrier
@@ -94,7 +94,7 @@ It looks different from literature pseudocode for two main 
reasons:
   alignas(__phase_alignment) __barrier_phase_t  _M_phase;
 
   bool
-  _M_arrive(__barrier_phase_t __old_phase)
+  _M_arrive(__barrier_phase_t __old_phase, size_t __current)
   {
const auto __old_phase_val = static_cast(__old_phase);
const auto __half_step =
@@ -103,9 +103,7 @@ It looks different from literature pseudocode for two main 
reasons:
   static_cast<__barrier_phase_t>(__old_phase_val + 2);
 
size_t __current_expected = _M_expected;
-   std::hash __hasher;
-   size_t __current = __hasher(std::this_thread::get_id())
- % ((_M_expected + 1) >> 1);
+__current %= ((__current_expected + 1) >> 1);
 
for (int __round = 0; ; ++__round)
  {
@@ -163,12 +161,14 @@ It looks different from literature pseudocode for two 
main reasons:
   [[nodiscard]] arrival_token
   arrive(ptrdiff_t __update)
   {
+   std::hash __hasher;
+   size_t __current = __hasher(std::this_thread::get_id());
__atomic_phase_ref_t __phase(_M_phase);
const auto __old_phase = __phase.load(memory_order_relaxed);
const auto __cur = static_cast(__old_phase);
for(; __update; --__update)
  {
-   if(_M_arrive(__old_phase))
+   if(_M_arrive(__old_phase, __current))
  {
_M_completion();
_M_expected += 
_M_expected_adjustment.load(memory_order_relaxed);
-- 
2.30.1



[PATCH 2/3] std::latch: reduce internal implementation from ptrdiff_t to int

2021-03-05 Thread Thiago Macieira via Gcc-patches
ints can be used as futex on Linux. ptrdiff_t on 64-bit Linux can't.

libstdc++-v3/ChangeLog:

* include/std/latch: Use int instead of ptrdiff_t
---
 libstdc++-v3/include/std/latch | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libstdc++-v3/include/std/latch b/libstdc++-v3/include/std/latch
index ef8c301e5e9..f3f73360618 100644
--- a/libstdc++-v3/include/std/latch
+++ b/libstdc++-v3/include/std/latch
@@ -48,7 +48,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   public:
 static constexpr ptrdiff_t
 max() noexcept
-{ return __gnu_cxx::__int_traits::__max; }
+{ return __gnu_cxx::__int_traits::__max; }
 
 constexpr explicit latch(ptrdiff_t __expected) noexcept
   : _M_a(__expected) { }
@@ -85,7 +85,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 }
 
   private:
-alignas(__alignof__(ptrdiff_t)) ptrdiff_t _M_a;
+alignas(4) int _M_a;   // kernel requires 4-byte alignment
   };
 _GLIBCXX_END_NAMESPACE_VERSION
 } // namespace
-- 
2.30.1



Re: [PATCH] c++: adc_unify deduction with constrained auto [PR99365]

2021-03-05 Thread Jason Merrill via Gcc-patches

On 3/4/21 9:55 PM, Patrick Palka wrote:

On Thu, 4 Mar 2021, Patrick Palka wrote:


On Thu, 4 Mar 2021, Patrick Palka wrote:


On Thu, 4 Mar 2021, Jason Merrill wrote:


On 3/4/21 11:32 AM, Patrick Palka wrote:

On Thu, 4 Mar 2021, Patrick Palka wrote:


My recent r11-7454 changed the way do_auto_deduction handles constrained
placeholders during template argument deduction (context == adc_unify)
when processing_template_decl != 0.

Before the patch, when processing_template_decl != 0 we would just
ignore the constraints on the placeholder in this situation, and proceed
with deduction:

/* Check any placeholder constraints against the deduced type. */
if (flag_concepts && !processing_template_decl)
  if (tree check = NON_ERROR (PLACEHOLDER_TYPE_CONSTRAINTS
(auto_node)))
{
  ...

After the patch, we now punt and return the original placeholder type:

/* Check any placeholder constraints against the deduced type. */
if (flag_concepts)
  if (NON_ERROR (PLACEHOLDER_TYPE_CONSTRAINTS (auto_node)))
{
  if (processing_template_decl)
/* In general we can't check satisfaction until we know all
   template arguments.  */
return type;
  ...

While this change fixed instances where we'd prematurely resolve a
constrained placeholder return or variable type with non-dependent
initializer at template parse time (such as PR96444), it broke the
adc_unify callers that rely on this previous behavior.

So this patch restores the previous behavior during adc_unify deduction
while retaining the new behavior only during adc_variable_type or
adc_return_type deduction.


Sure, it makes sense for adc_unify to behave differently, since in deduction
context constraints are checked after all template args have been deduced.


I see.




We additionally now need to pass outer template arguments to
do_auto_deduction during unify, for sake of constraint checking.
But we don't want do_auto_deduction to substitute these outer arguments
into type if it's already been done, hence the added TEMPLATE_TYPE_LEVEL
check.

This fixes partial specializations of non-nested templates with
constrained 'auto' template parameters, but nested templates are still
broken, ultimately because most_specialized_partial_spec passes only the
innermost template arguments to get_partial_spec_bindings, and so
outer_targs during do_auto_deduction (called from unify) contains only
the innermost template arguments which makes satisfaction unhappy.
Fixing this might be too invasive at this stage, perhaps..  (Seems we
need to make most_specialized_partial_spec pass all template arguments
to get_partial_spec_bindings.)


How did this work before?


Before, it would work, but only if the constraint didn't also depend on
any outer template arguments.  do_auto_deduction would just "surgically"
replace the auto in the concept-id with the type we deduced and leave
the other template arguments of the concept-id alone.  So if the
constraint was non-dependent, satisfaction would work regardless of the
template nesting level.

Now that we try to do perform satisfaction properly, we're sensitive to
the template nesting level even if the constraint is otherwise
non-dependent, because the template nesting level determines the level
of the auto that appears inside the constraint.  So we rely on
outer_targs to contain all levels of outer template arguments, because
we tack on another level to the end of outer_targs which needs to
map to the level of the auto for satisfaction.

(As a hacky workaround, when outer_targs is incomplete, can probably
just augment it with empty levels until it's TEMPLATE_TYPE_LEVEL(auto_node)-1
levels deep, which would fix the nested template case as long as the
constraint was depended only on the innermost level of template
arguments.)




Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?  Also tested on range-v3 and cmcstl2.


Here's the same patch generated with -w which hides the noisy indentation
changes:

-- >8 --

PR c++/99365
* pt.c (do_auto_deduction): When processing_template_decl != 0
and context is adc_unify and we have constraints, pretend the
constraints are satisfied instead of punting.  Add some
clarifying sanity checks.  Don't substitute outer_targs into
type if not needed.

gcc/testsuite/ChangeLog:

PR c++/99365
* g++.dg/cpp2a/concepts-partial-spec9.C: New test.
---
   gcc/cp/pt.c   | 24 ++-
   .../g++.dg/cpp2a/concepts-partial-spec9.C | 24 +++
   2 files changed, 42 insertions(+), 6 deletions(-)
   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec9.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index a4686e0affb..ce537e4529a 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -23693,7 +23693,8 @@ unify (tree tparms, tree targs, tree parm, tree arg,
int strict,
  if (tr

c++: Local instantiations of imported specializations [PR 99377]

2021-03-05 Thread Nathan Sidwell


This turned out to be the function version of the previous fix.  We can 
import an implicit specialization declaration that we need to 
instantiate.  We must mark the instantiation so we remember to stream it.


PR c++/99377
gcc/cp/
* pt.c (instantiate_decl): Call set_instantiating_module.
gcc/testsuite/
* g++.dg/modules/pr99377_a.H: New.
* g++.dg/modules/pr99377_b.C: New.
* g++.dg/modules/pr99377_c.C: New.

--
Nathan Sidwell
diff --git c/gcc/cp/pt.c w/gcc/cp/pt.c
index 8ca3dc8ec2b..1f3cd9c45f1 100644
--- c/gcc/cp/pt.c
+++ w/gcc/cp/pt.c
@@ -26154,6 +26154,7 @@ instantiate_decl (tree d, bool defer_ok, bool expl_inst_class_mem_p)
 }
   else
 {
+  set_instantiating_module (d);
   if (variable_template_p (gen_tmpl))
 	note_variable_template_instantiation (d);
   instantiate_body (td, args, d, false);
diff --git c/gcc/testsuite/g++.dg/modules/pr99377_a.H w/gcc/testsuite/g++.dg/modules/pr99377_a.H
new file mode 100644
index 000..b5e5a3fea54
--- /dev/null
+++ w/gcc/testsuite/g++.dg/modules/pr99377_a.H
@@ -0,0 +1,21 @@
+// PR 99377 failed to stream locally instantiated member
+// link failure in function `Check(Widget const&)':
+// bug_c.ii:(.text._Z5CheckRK6WidgetIiE[_Z5CheckRK6WidgetIiE]+0x14): undefined reference to `Widget::Second() const'
+// { dg-module-do link }
+// { dg-additional-options -fmodule-header }
+// { dg-module-cmi {} }
+template
+struct Widget
+{
+  Widget (int) { }
+
+  bool First() const { return true; }
+
+  bool Second () const { return true;}
+};
+
+inline void Frob (const Widget& w) noexcept
+{
+  w.First ();
+}
+
diff --git c/gcc/testsuite/g++.dg/modules/pr99377_b.C w/gcc/testsuite/g++.dg/modules/pr99377_b.C
new file mode 100644
index 000..77826379fc7
--- /dev/null
+++ w/gcc/testsuite/g++.dg/modules/pr99377_b.C
@@ -0,0 +1,10 @@
+// { dg-additional-options -fmodules-ts }
+export module Foo;
+// { dg-module-cmi Foo }
+import "pr99377_a.H";
+
+export inline bool Check (const Widget& w)
+{
+  return w.Second ();
+}
+
diff --git c/gcc/testsuite/g++.dg/modules/pr99377_c.C w/gcc/testsuite/g++.dg/modules/pr99377_c.C
new file mode 100644
index 000..287388fa6dd
--- /dev/null
+++ w/gcc/testsuite/g++.dg/modules/pr99377_c.C
@@ -0,0 +1,8 @@
+// { dg-additional-options -fmodules-ts }
+
+import Foo;
+
+int main ()
+{
+  return Check (0) ? 0 : 1;
+}


Re: [PATCH] PR libfortran/99218 - [8/9/10/11 Regression] matmul on temporary array accesses invalid memory

2021-03-05 Thread Harald Anlauf via Fortran
Dear all,

I finally figured out that the array dimensions simply need to be
large enough to get invalid memory accesses that actual lead to a
crash.

I will commit the following testcase along with the fix to libfortran:


! { dg-do run }
! PR libfortran/99218 - matmul on temporary array accesses invalid memory

program p
  implicit none
  integer, parameter :: nState = 30
  integer, parameter :: nCon = 1
  real,parameter :: ZERO = 0.0
  real :: G(nCon,nState) = ZERO
  real :: H(nState,nCon) = ZERO
  real :: lambda(nCon)   = ZERO
  real :: f(nState)  = ZERO
  f = matmul (transpose (G), lambda)
  if (f(1) /= ZERO) stop 1
end program


Cheers,
Harald



[PATCH] libstdc++: Improve std::rot[lr] [PR99396]

2021-03-05 Thread Jakub Jelinek via Gcc-patches
Hi!

As can be seen on:
#include 

unsigned char f1 (unsigned char x, int y) { return std::rotl (x, y); }
unsigned char f2 (unsigned char x, int y) { return std::rotr (x, y); }
unsigned short f3 (unsigned short x, int y) { return std::rotl (x, y); }
unsigned short f4 (unsigned short x, int y) { return std::rotr (x, y); }
unsigned int f5 (unsigned int x, int y) { return std::rotl (x, y); }
unsigned int f6 (unsigned int x, int y) { return std::rotr (x, y); }
unsigned long int f7 (unsigned long int x, int y) { return std::rotl (x, y); }
unsigned long int f8 (unsigned long int x, int y) { return std::rotr (x, y); }
unsigned long long int f9 (unsigned long long int x, int y) { return std::rotl 
(x, y); }
unsigned long long int f10 (unsigned long long int x, int y) { return std::rotr 
(x, y); }
//unsigned __int128 f11 (unsigned __int128 x, int y) { return std::rotl (x, y); 
}
//unsigned __int128 f12 (unsigned __int128 x, int y) { return std::rotr (x, y); 
}

constexpr auto a = std::rotl (1234U, 0);
constexpr auto b = std::rotl (1234U, 5);
constexpr auto c = std::rotl (1234U, -5);
constexpr auto d = std::rotl (1234U, -__INT_MAX__ - 1);
the current  definitions of std::__rot[lr] aren't pattern recognized
as rotates, they are too long/complex for that, starting with signed modulo,
special case for 0 and different cases for positive and negative.

For types with power of two bits the following patch adds definitions that
the compiler can pattern recognize and turn e.g. on x86_64 into ro[lr][bwlq]
instructions.  For weirdo types like unsigned __int20 etc. it keeps the
current definitions.

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

2021-03-05  Jakub Jelinek  

PR libstdc++/99396
* include/std/bit (__rotl, __rotr): Add optimized variants for power of
two _Nd which the compiler can pattern match the rotates.

--- libstdc++-v3/include/std/bit.jj 2021-03-05 10:37:36.108378753 +0100
+++ libstdc++-v3/include/std/bit2021-03-05 12:01:57.926310110 +0100
@@ -68,6 +68,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 __rotl(_Tp __x, int __s) noexcept
 {
   constexpr auto _Nd = __gnu_cxx::__int_traits<_Tp>::__digits;
+  if _GLIBCXX17_CONSTEXPR ((_Nd & (_Nd - 1)) == 0)
+   {
+ // Variant for power of two _Nd which the compiler can
+ // easily pattern match.
+ constexpr unsigned __uNd = _Nd;
+ const unsigned __r = __s;
+ return (__x << (__r % __uNd)) | (__x >> ((-__r) % __uNd));
+   }
   const int __r = __s % _Nd;
   if (__r == 0)
return __x;
@@ -82,6 +90,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 __rotr(_Tp __x, int __s) noexcept
 {
   constexpr auto _Nd = __gnu_cxx::__int_traits<_Tp>::__digits;
+  if _GLIBCXX17_CONSTEXPR ((_Nd & (_Nd - 1)) == 0)
+   {
+ // Variant for power of two _Nd which the compiler can
+ // easily pattern match.
+ constexpr unsigned __uNd = _Nd;
+ const unsigned __r = __s;
+ return (__x >> (__r % __uNd)) | (__x << ((-__r) % __uNd));
+   }
   const int __r = __s % _Nd;
   if (__r == 0)
return __x;

Jakub



[PATCH] i386: Improve [QH]Imode rotates with masked shift count [PR99405]

2021-03-05 Thread Jakub Jelinek via Gcc-patches
Hi!

The following testcase shows that while we nicely optimize away the
useless and? of shift count before rotation for [SD]Imode rotates,
we don't do that for [QH]Imode.

The following patch optimizes that by using the right iterator on those
4 patterns.

Bootstrapped/regtested on x86_64-linux and i686-linux.  Ok for trunk?
Or just GCC12?

2021-03-05  Jakub Jelinek  

PR target/99405
* config/i386/i386.md (*3_mask, *3_mask_1):
For any_rotate define_insn_split and following splitters, use
SWI iterator instead of SWI48.

* gcc.target/i386/pr99405.c: New test.

--- gcc/config/i386/i386.md.jj  2021-03-03 10:02:27.871589603 +0100
+++ gcc/config/i386/i386.md 2021-03-05 14:00:35.768378973 +0100
@@ -11951,9 +11951,9 @@ (define_expand "3"
 
 ;; Avoid useless masking of count operand.
 (define_insn_and_split "*3_mask"
-  [(set (match_operand:SWI48 0 "nonimmediate_operand")
-   (any_rotate:SWI48
- (match_operand:SWI48 1 "nonimmediate_operand")
+  [(set (match_operand:SWI 0 "nonimmediate_operand")
+   (any_rotate:SWI
+ (match_operand:SWI 1 "nonimmediate_operand")
  (subreg:QI
(and:SI
  (match_operand:SI 2 "register_operand" "c")
@@ -11967,15 +11967,15 @@ (define_insn_and_split "*3_m
   "&& 1"
   [(parallel
  [(set (match_dup 0)
-  (any_rotate:SWI48 (match_dup 1)
-(match_dup 2)))
+  (any_rotate:SWI (match_dup 1)
+  (match_dup 2)))
   (clobber (reg:CC FLAGS_REG))])]
   "operands[2] = gen_lowpart (QImode, operands[2]);")
 
 (define_split
-  [(set (match_operand:SWI48 0 "register_operand")
-   (any_rotate:SWI48
- (match_operand:SWI48 1 "const_int_operand")
+  [(set (match_operand:SWI 0 "register_operand")
+   (any_rotate:SWI
+ (match_operand:SWI 1 "const_int_operand")
  (subreg:QI
(and:SI
  (match_operand:SI 2 "register_operand")
@@ -11984,14 +11984,14 @@ (define_split
== GET_MODE_BITSIZE (mode) - 1"
  [(set (match_dup 4) (match_dup 1))
   (set (match_dup 0)
-   (any_rotate:SWI48 (match_dup 4)
-(subreg:QI (match_dup 2) 0)))]
+   (any_rotate:SWI (match_dup 4)
+  (subreg:QI (match_dup 2) 0)))]
  "operands[4] = gen_reg_rtx (mode);")
 
 (define_insn_and_split "*3_mask_1"
-  [(set (match_operand:SWI48 0 "nonimmediate_operand")
-   (any_rotate:SWI48
- (match_operand:SWI48 1 "nonimmediate_operand")
+  [(set (match_operand:SWI 0 "nonimmediate_operand")
+   (any_rotate:SWI
+ (match_operand:SWI 1 "nonimmediate_operand")
  (and:QI
(match_operand:QI 2 "register_operand" "c")
(match_operand:QI 3 "const_int_operand"
@@ -12004,14 +12004,14 @@ (define_insn_and_split "*3_m
   "&& 1"
   [(parallel
  [(set (match_dup 0)
-  (any_rotate:SWI48 (match_dup 1)
-(match_dup 2)))
+  (any_rotate:SWI (match_dup 1)
+  (match_dup 2)))
   (clobber (reg:CC FLAGS_REG))])])
 
 (define_split
-  [(set (match_operand:SWI48 0 "register_operand")
-   (any_rotate:SWI48
- (match_operand:SWI48 1 "const_int_operand")
+  [(set (match_operand:SWI 0 "register_operand")
+   (any_rotate:SWI
+ (match_operand:SWI 1 "const_int_operand")
  (and:QI
(match_operand:QI 2 "register_operand")
(match_operand:QI 3 "const_int_operand"]
@@ -12019,7 +12019,7 @@ (define_split
   == GET_MODE_BITSIZE (mode) - 1"
  [(set (match_dup 4) (match_dup 1))
   (set (match_dup 0)
-   (any_rotate:SWI48 (match_dup 4) (match_dup 2)))]
+   (any_rotate:SWI (match_dup 4) (match_dup 2)))]
  "operands[4] = gen_reg_rtx (mode);")
 
 ;; Implement rotation using two double-precision
--- gcc/testsuite/gcc.target/i386/pr99405.c.jj  2021-03-05 13:40:20.334860937 
+0100
+++ gcc/testsuite/gcc.target/i386/pr99405.c 2021-03-05 13:39:51.131185009 
+0100
@@ -0,0 +1,23 @@
+/* PR target/99405 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -mtune=generic -fomit-frame-pointer" } */
+/* { dg-final { scan-assembler-not "\tand\[bl]\t\\\$" } } */
+
+unsigned char f1 (unsigned char x, unsigned y) { return (x << (y & 7)) | (x >> 
(-y & 7)); }
+unsigned short f2 (unsigned short x, unsigned y) { return (x << (y & 15)) | (x 
>> (-y & 15)); }
+unsigned int f3 (unsigned int x, unsigned y) { return (x << (y & 31)) | (x >> 
(-y & 31)); }
+unsigned char f4 (unsigned char x, unsigned y) { return (x >> (y & 7)) | (x << 
(-y & 7)); }
+unsigned short f5 (unsigned short x, unsigned y) { return (x >> (y & 15)) | (x 
<< (-y & 15)); }
+unsigned int f6 (unsigned int x, unsigned y) { return (x >> (y & 31)) | (x << 
(-y & 31)); }
+unsigned char f7 (unsigned char x, unsigned char y) { unsigned char v = y & 7; 
unsigned char w = -y & 7; return (x << v) | (x >> w); }
+unsigned short f8 (unsigned short x, unsigned char y) { unsigned char v = y & 
15; unsigned 

Re: [PATCH] libgcov: Fix build on Darwin [PR99406]

2021-03-05 Thread Jakub Jelinek via Gcc-patches
On Fri, Mar 05, 2021 at 04:19:47PM +, Iain Sandoe wrote:
> Jakub Jelinek via Gcc-patches  wrote:
> 
> > As reported, bootstrap currently fails on older Darwin because
> > MAP_ANONYMOUS
> > is not defined.
> > 
> > The following is what gcc/system.h does, so I think it should work for
> > libgcov.
> > Build tested on x86_64-linux, ok for trunk?
> 
> bootstrap suceeded r11-7524 + this patch on Darwin11.

And bootstrap/regtest succeeded on x86_64-linux and i686-linux too.

> > 2021-03-05  Jakub Jelinek  
> > 
> > PR gcov-profile/99406
> > * libgcov.h (MAP_FAILED, MAP_ANONYMOUS): If HAVE_SYS_MMAN_H is
> > defined, define these macros if not defined already.

Jakub



Re: [PATCH] c++: adc_unify deduction with constrained auto [PR99365]

2021-03-05 Thread Patrick Palka via Gcc-patches
On Fri, 5 Mar 2021, Jason Merrill wrote:

> On 3/4/21 9:55 PM, Patrick Palka wrote:
> > On Thu, 4 Mar 2021, Patrick Palka wrote:
> > 
> > > On Thu, 4 Mar 2021, Patrick Palka wrote:
> > > 
> > > > On Thu, 4 Mar 2021, Jason Merrill wrote:
> > > > 
> > > > > On 3/4/21 11:32 AM, Patrick Palka wrote:
> > > > > > On Thu, 4 Mar 2021, Patrick Palka wrote:
> > > > > > 
> > > > > > > My recent r11-7454 changed the way do_auto_deduction handles
> > > > > > > constrained
> > > > > > > placeholders during template argument deduction (context ==
> > > > > > > adc_unify)
> > > > > > > when processing_template_decl != 0.
> > > > > > > 
> > > > > > > Before the patch, when processing_template_decl != 0 we would just
> > > > > > > ignore the constraints on the placeholder in this situation, and
> > > > > > > proceed
> > > > > > > with deduction:
> > > > > > > 
> > > > > > > /* Check any placeholder constraints against the deduced type.
> > > > > > > */
> > > > > > > if (flag_concepts && !processing_template_decl)
> > > > > > >   if (tree check = NON_ERROR (PLACEHOLDER_TYPE_CONSTRAINTS
> > > > > > > (auto_node)))
> > > > > > > {
> > > > > > >   ...
> > > > > > > 
> > > > > > > After the patch, we now punt and return the original placeholder
> > > > > > > type:
> > > > > > > 
> > > > > > > /* Check any placeholder constraints against the deduced type.
> > > > > > > */
> > > > > > > if (flag_concepts)
> > > > > > >   if (NON_ERROR (PLACEHOLDER_TYPE_CONSTRAINTS (auto_node)))
> > > > > > > {
> > > > > > >   if (processing_template_decl)
> > > > > > > /* In general we can't check satisfaction until we
> > > > > > > know all
> > > > > > >template arguments.  */
> > > > > > > return type;
> > > > > > >   ...
> > > > > > > 
> > > > > > > While this change fixed instances where we'd prematurely resolve a
> > > > > > > constrained placeholder return or variable type with non-dependent
> > > > > > > initializer at template parse time (such as PR96444), it broke the
> > > > > > > adc_unify callers that rely on this previous behavior.
> > > > > > > 
> > > > > > > So this patch restores the previous behavior during adc_unify
> > > > > > > deduction
> > > > > > > while retaining the new behavior only during adc_variable_type or
> > > > > > > adc_return_type deduction.
> > > > > 
> > > > > Sure, it makes sense for adc_unify to behave differently, since in
> > > > > deduction
> > > > > context constraints are checked after all template args have been
> > > > > deduced.
> > > > 
> > > > I see.
> > > > 
> > > > > 
> > > > > > > We additionally now need to pass outer template arguments to
> > > > > > > do_auto_deduction during unify, for sake of constraint checking.
> > > > > > > But we don't want do_auto_deduction to substitute these outer
> > > > > > > arguments
> > > > > > > into type if it's already been done, hence the added
> > > > > > > TEMPLATE_TYPE_LEVEL
> > > > > > > check.
> > > > > > > 
> > > > > > > This fixes partial specializations of non-nested templates with
> > > > > > > constrained 'auto' template parameters, but nested templates are
> > > > > > > still
> > > > > > > broken, ultimately because most_specialized_partial_spec passes
> > > > > > > only the
> > > > > > > innermost template arguments to get_partial_spec_bindings, and so
> > > > > > > outer_targs during do_auto_deduction (called from unify) contains
> > > > > > > only
> > > > > > > the innermost template arguments which makes satisfaction unhappy.
> > > > > > > Fixing this might be too invasive at this stage, perhaps..  (Seems
> > > > > > > we
> > > > > > > need to make most_specialized_partial_spec pass all template
> > > > > > > arguments
> > > > > > > to get_partial_spec_bindings.)
> > > > > 
> > > > > How did this work before?
> > > > 
> > > > Before, it would work, but only if the constraint didn't also depend on
> > > > any outer template arguments.  do_auto_deduction would just "surgically"
> > > > replace the auto in the concept-id with the type we deduced and leave
> > > > the other template arguments of the concept-id alone.  So if the
> > > > constraint was non-dependent, satisfaction would work regardless of the
> > > > template nesting level.
> > > > 
> > > > Now that we try to do perform satisfaction properly, we're sensitive to
> > > > the template nesting level even if the constraint is otherwise
> > > > non-dependent, because the template nesting level determines the level
> > > > of the auto that appears inside the constraint.  So we rely on
> > > > outer_targs to contain all levels of outer template arguments, because
> > > > we tack on another level to the end of outer_targs which needs to
> > > > map to the level of the auto for satisfaction.
> > > > 
> > > > (As a hacky workaround, when outer_targs is incomplete, can probably
> > > > just augment it with empty levels until it's
> > > > TEMPLATE_TYPE_LEVEL(auto_node)-1
> > > > levels deep, whi

[PATCH] testsuite: Fix up attr-flatten-1.c failure [PR99363]

2021-03-05 Thread Jakub Jelinek via Gcc-patches
On Wed, Feb 24, 2021 at 07:08:34PM -0500, Jason Merrill via Gcc-patches wrote:
> gcc/ChangeLog:
> 
>   PR c++/96078
>   * cgraphunit.c (process_function_and_variable_attributes): Don't
>   warn about flatten on an alias if the target also has it.
>   * cgraph.h (symtab_node::get_alias_target_tree): New.
> 
> gcc/testsuite/ChangeLog:
> 
>   PR c++/96078
>   * g++.dg/ext/attr-flatten1.C: New test.

This broke the gcc.dg/attr-flatten-1.c test, where we no longer
warn because attribute flatten is both on the alias target and on the alias.

> -   warning_at (DECL_SOURCE_LOCATION (node->decl), OPT_Wattributes,
> -   "% attribute is ignored on aliases");
> +   tree tdecl = node->get_alias_target_tree ();
> +   if (!tdecl || !DECL_P (tdecl)
> +   || !lookup_attribute ("flatten", DECL_ATTRIBUTES (tdecl)))
> + warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wattributes,
> + "% attribute is ignored on aliases");

Seems that was the intentional change though, so can we apply
following change to fix that?

Bootstrapped/regtested on x86_64-linux and i686-linux:

2021-03-05  Jakub Jelinek  

PR c/99363
* gcc.dg/attr-flatten-1.c: Remove dg-warning directive.

--- gcc/testsuite/gcc.dg/attr-flatten-1.c.jj2020-03-19 18:13:21.776787973 
+0100
+++ gcc/testsuite/gcc.dg/attr-flatten-1.c   2021-03-05 15:18:27.176637165 
+0100
@@ -10,7 +10,7 @@ int fn1(int p1)
 }
 __attribute__((flatten))
 __attribute__((alias("fn1")))
-int fn4(int p1); /* { dg-warning "ignored" } */
+int fn4(int p1);
 int
 test ()
 {


Jakub



[PATCH] i386: Fix some -mavx512vl -mno-avx512bw bugs [PR99321]

2021-03-05 Thread Jakub Jelinek via Gcc-patches
Hi!

As I wrote in the mail with the previous PR99321 fix, we have various
bugs where we emit instructions that need avx512bw and avx512vl
ISAs when compiling with -mavx512vl -mno-avx512bw.

Without the following patch,
/* PR target/99321 */
/* Would need some effective target for GNU as that supports -march=+noavx512bw 
etc. */
/* { dg-do assemble } */
/* { dg-options "-O2 -mavx512vl -mno-avx512bw -Wa,-march=+noavx512bw" } */

#include 

typedef unsigned char V1 __attribute__((vector_size (16)));
typedef unsigned char V2 __attribute__((vector_size (32)));
typedef unsigned short V3 __attribute__((vector_size (16)));
typedef unsigned short V4 __attribute__((vector_size (32)));

void f1 (void) { register V1 a __asm ("%xmm16"), b __asm ("%xmm17"); __asm ("" 
: "=v" (a), "=v" (b)); a += b; __asm ("" : : "v" (a)); }
void f2 (void) { register V2 a __asm ("%xmm16"), b __asm ("%xmm17"); __asm ("" 
: "=v" (a), "=v" (b)); a += b; __asm ("" : : "v" (a)); }
void f3 (void) { register V3 a __asm ("%xmm16"), b __asm ("%xmm17"); __asm ("" 
: "=v" (a), "=v" (b)); a += b; __asm ("" : : "v" (a)); }
void f4 (void) { register V4 a __asm ("%xmm16"), b __asm ("%xmm17"); __asm ("" 
: "=v" (a), "=v" (b)); a += b; __asm ("" : : "v" (a)); }
void f5 (void) { register V1 a __asm ("%xmm16"), b __asm ("%xmm17"); __asm ("" 
: "=v" (a), "=v" (b)); a -= b; __asm ("" : : "v" (a)); }
void f6 (void) { register V2 a __asm ("%xmm16"), b __asm ("%xmm17"); __asm ("" 
: "=v" (a), "=v" (b)); a -= b; __asm ("" : : "v" (a)); }
void f7 (void) { register V3 a __asm ("%xmm16"), b __asm ("%xmm17"); __asm ("" 
: "=v" (a), "=v" (b)); a -= b; __asm ("" : : "v" (a)); }
void f8 (void) { register V4 a __asm ("%xmm16"), b __asm ("%xmm17"); __asm ("" 
: "=v" (a), "=v" (b)); a -= b; __asm ("" : : "v" (a)); }
void f9 (void) { register V3 a __asm ("%xmm16"), b __asm ("%xmm17"); __asm ("" 
: "=v" (a), "=v" (b)); a *= b; __asm ("" : : "v" (a)); }
void f10 (void) { register V4 a __asm ("%xmm16"), b __asm ("%xmm17"); __asm ("" 
: "=v" (a), "=v" (b)); a *= b; __asm ("" : : "v" (a)); }
void f11 (void) { register V1 a __asm ("%xmm16"), b __asm ("%xmm17"); __asm ("" 
: "=v" (a), "=v" (b)); a = (V1) _mm_min_epu8 ((__m128i) a, (__m128i) b); __asm 
("" : : "v" (a)); }
void f12 (void) { register V2 a __asm ("%xmm16"), b __asm ("%xmm17"); __asm ("" 
: "=v" (a), "=v" (b)); a = (V2) _mm256_min_epu8 ((__m256i) a, (__m256i) b); 
__asm ("" : : "v" (a)); }
void f13 (void) { register V3 a __asm ("%xmm16"), b __asm ("%xmm17"); __asm ("" 
: "=v" (a), "=v" (b)); a = (V3) _mm_min_epu16 ((__m128i) a, (__m128i) b); __asm 
("" : : "v" (a)); }
void f14 (void) { register V4 a __asm ("%xmm16"), b __asm ("%xmm17"); __asm ("" 
: "=v" (a), "=v" (b)); a = (V4) _mm256_min_epu16 ((__m256i) a, (__m256i) b); 
__asm ("" : : "v" (a)); }
void f15 (void) { register V1 a __asm ("%xmm16"), b __asm ("%xmm17"); __asm ("" 
: "=v" (a), "=v" (b)); a = (V1) _mm_min_epi8 ((__m128i) a, (__m128i) b); __asm 
("" : : "v" (a)); }
void f16 (void) { register V2 a __asm ("%xmm16"), b __asm ("%xmm17"); __asm ("" 
: "=v" (a), "=v" (b)); a = (V2) _mm256_min_epi8 ((__m256i) a, (__m256i) b); 
__asm ("" : : "v" (a)); }
void f17 (void) { register V3 a __asm ("%xmm16"), b __asm ("%xmm17"); __asm ("" 
: "=v" (a), "=v" (b)); a = (V3) _mm_min_epi16 ((__m128i) a, (__m128i) b); __asm 
("" : : "v" (a)); }
void f18 (void) { register V4 a __asm ("%xmm16"), b __asm ("%xmm17"); __asm ("" 
: "=v" (a), "=v" (b)); a = (V4) _mm256_min_epi16 ((__m256i) a, (__m256i) b); 
__asm ("" : : "v" (a)); }
void f19 (void) { register V1 a __asm ("%xmm16"), b __asm ("%xmm17"); __asm ("" 
: "=v" (a), "=v" (b)); a = (V1) _mm_max_epu8 ((__m128i) a, (__m128i) b); __asm 
("" : : "v" (a)); }
void f20 (void) { register V2 a __asm ("%xmm16"), b __asm ("%xmm17"); __asm ("" 
: "=v" (a), "=v" (b)); a = (V2) _mm256_max_epu8 ((__m256i) a, (__m256i) b); 
__asm ("" : : "v" (a)); }
void f21 (void) { register V3 a __asm ("%xmm16"), b __asm ("%xmm17"); __asm ("" 
: "=v" (a), "=v" (b)); a = (V3) _mm_max_epu16 ((__m128i) a, (__m128i) b); __asm 
("" : : "v" (a)); }
void f22 (void) { register V4 a __asm ("%xmm16"), b __asm ("%xmm17"); __asm ("" 
: "=v" (a), "=v" (b)); a = (V4) _mm256_max_epu16 ((__m256i) a, (__m256i) b); 
__asm ("" : : "v" (a)); }
void f23 (void) { register V1 a __asm ("%xmm16"), b __asm ("%xmm17"); __asm ("" 
: "=v" (a), "=v" (b)); a = (V1) _mm_max_epi8 ((__m128i) a, (__m128i) b); __asm 
("" : : "v" (a)); }
void f24 (void) { register V2 a __asm ("%xmm16"), b __asm ("%xmm17"); __asm ("" 
: "=v" (a), "=v" (b)); a = (V2) _mm256_max_epi8 ((__m256i) a, (__m256i) b); 
__asm ("" : : "v" (a)); }
void f25 (void) { register V3 a __asm ("%xmm16"), b __asm ("%xmm17"); __asm ("" 
: "=v" (a), "=v" (b)); a = (V3) _mm_max_epi16 ((__m128i) a, (__m128i) b); __asm 
("" : : "v" (a)); }
void f26 (void) { register V4 a __asm ("%xmm16"), b __asm ("%xmm17"); __asm ("" 
: "=v" (a), "=v" (b)); a = (V4) _mm256_max_epi16 ((__m256i) a, (__m256i) b); 
__asm ("" : : "v" (a)); }
test fails wit

c++: Duplicate namespace bindings [PR 99245]

2021-03-05 Thread Nathan Sidwell


Header units can declare the same entity, and this can lead to one of 
them containing a (non-using) binding to an import.  If one gets the 
cluster ordering just right, an assert will trigger.  Relax that assert.


PR c++/99245
gcc/cp/
* module.cc (module_state::write_cluster): Relax binding assert.
gcc/testsuite/
* g++.dg/modules/pr99245_a.H: New.
* g++.dg/modules/pr99245_b.H: New.

--
Nathan Sidwell
diff --git c/gcc/cp/module.cc w/gcc/cp/module.cc
index 31bbf9776dd..48862dd9bbc 100644
--- c/gcc/cp/module.cc
+++ w/gcc/cp/module.cc
@@ -14496,20 +14496,25 @@ module_state::write_cluster (elf_out *to, depset *scc[], unsigned size,
 	  gcc_unreachable ();
 
 	case depset::EK_BINDING:
-	  dump (dumper::CLUSTER)
-	&& dump ("[%u]=%s %P", ix, b->entity_kind_name (),
-		 b->get_entity (), b->get_name ());
-	  for (unsigned jx = b->deps.length (); jx--;)
-	{
-	  depset *dep = b->deps[jx];
-	  if (jx)
-		gcc_checking_assert (dep->get_entity_kind () == depset::EK_USING
- || TREE_VISITED (dep->get_entity ()));
-	  else
-		gcc_checking_assert (dep->get_entity_kind ()
- == depset::EK_NAMESPACE
- && dep->get_entity () == b->get_entity ());
-	}
+	  {
+	dump (dumper::CLUSTER)
+	  && dump ("[%u]=%s %P", ix, b->entity_kind_name (),
+		   b->get_entity (), b->get_name ());
+	depset *ns_dep = b->deps[0];
+	gcc_checking_assert (ns_dep->get_entity_kind ()
+ == depset::EK_NAMESPACE
+ && ns_dep->get_entity () == b->get_entity ());
+	for (unsigned jx = b->deps.length (); --jx;)
+	  {
+		depset *dep = b->deps[jx];
+		// We could be declaring something that is also a
+		// (merged) import
+		gcc_checking_assert (dep->is_import ()
+ || TREE_VISITED (dep->get_entity ())
+ || (dep->get_entity_kind ()
+	 == depset::EK_USING));
+	  }
+	  }
 	  break;
 
 	case depset::EK_DECL:
diff --git c/gcc/testsuite/g++.dg/modules/pr99245_a.H w/gcc/testsuite/g++.dg/modules/pr99245_a.H
new file mode 100644
index 000..94c6bf11995
--- /dev/null
+++ w/gcc/testsuite/g++.dg/modules/pr99245_a.H
@@ -0,0 +1,5 @@
+// PR 99245 ICE writing out user of type_info
+// { dg-additional-options -fmodule-header }
+// { dg-module-cmi {} }
+
+namespace std { class type_info {}; }
diff --git c/gcc/testsuite/g++.dg/modules/pr99245_b.H w/gcc/testsuite/g++.dg/modules/pr99245_b.H
new file mode 100644
index 000..548c2720ef5
--- /dev/null
+++ w/gcc/testsuite/g++.dg/modules/pr99245_b.H
@@ -0,0 +1,9 @@
+// { dg-additional-options -fmodule-header }
+// { dg-module-cmi {} }
+namespace std { class type_info; }
+
+import "pr99245_a.H";
+
+namespace std {
+  const type_info* __cxa_exception_type () noexcept;
+}


[committed] openmp: Avoid ICEs due to orphaned labels in OpenMP regions [PR99322]

2021-03-05 Thread Jakub Jelinek via Gcc-patches
Hi!

When performing cfg cleanup at the end of cfg pass, if there are any OpenMP
regions and some basic blocks are unreachable and contain forced labels,
remove_bb moves the labels to previous bb, but if the two bb belong to different
OpenMP regions, that means it will end up in a different function from where
it was assumed to be and checked e.g. during gimplification or OpenMP region
SESE checking.

The following patch will place the labels to some bb from the right OpenMP
region if the previous bb is not that.  I think it should happen very rarely,
normally the bbs from each OpenMP region should be from the before-cfg pass
adjacent and the problems will usually be only if the OpenMP regions are
no-return, so I hope it isn't fatal that it searches through all bbs on the 
miss.
If it turns out to be a problem, it can always lazily create some better data
structure and maintain it through bb removals when it reaches that case the
first time.

Bootstrapped/regtested on x86_64-linux and i686-linux, committed to trunk.

2021-03-05  Jakub Jelinek  

PR middle-end/99322
* tree-cfg.c (bb_to_omp_idx): New variable.
(execute_build_cfg): Release the bb_to_omp_idx vector after
cleanup_tree_cfg returns.
(handle_abnormal_edges): Remove bb_to_omp_idx argument, adjust
for bb_to_omp_idx being a vec instead of pointer to array
of ints.
(make_edges): Remove bb_to_omp_idx local variable, don't pass
it to handle_abnormal_edges, adjust for bb_to_omp_idx being a
vec instead of pointer to array of ints and don't free/release
it at the end.
(remove_bb): When removing a bb and placing forced label somewhere
else, ensure it is put into the same OpenMP region during cfg
pass if possible or to entry successor as fallback.  Unregister
bb from bb_to_omp_idx.

* c-c++-common/gomp/pr99322.c: New test.

--- gcc/tree-cfg.c.jj   2021-02-19 12:14:31.616880014 +0100
+++ gcc/tree-cfg.c  2021-03-05 17:38:23.639195846 +0100
@@ -93,6 +93,9 @@ static hash_map *edge_to_cas
 
 static bitmap touched_switch_bbs;
 
+/* OpenMP region idxs for blocks during cfg pass.  */
+static vec bb_to_omp_idx;
+
 /* CFG statistics.  */
 struct cfg_stats_d
 {
@@ -372,6 +375,9 @@ execute_build_cfg (void)
   dump_scope_blocks (dump_file, dump_flags);
 }
   cleanup_tree_cfg ();
+
+  bb_to_omp_idx.release ();
+
   loop_optimizer_init (AVOID_CFG_MODIFICATIONS);
   replace_loop_annotate ();
   return 0;
@@ -724,8 +730,7 @@ get_abnormal_succ_dispatcher (basic_bloc
if COMPUTED_GOTO is false, otherwise factor the computed gotos.  */
 
 static void
-handle_abnormal_edges (basic_block *dispatcher_bbs,
-  basic_block for_bb, int *bb_to_omp_idx,
+handle_abnormal_edges (basic_block *dispatcher_bbs, basic_block for_bb,
   auto_vec *bbs, bool computed_goto)
 {
   basic_block *dispatcher = dispatcher_bbs + (computed_goto ? 1 : 0);
@@ -733,7 +738,7 @@ handle_abnormal_edges (basic_block *disp
   basic_block bb;
   bool inner = false;
 
-  if (bb_to_omp_idx)
+  if (!bb_to_omp_idx.is_empty ())
 {
   dispatcher = dispatcher_bbs + 2 * bb_to_omp_idx[for_bb->index];
   if (bb_to_omp_idx[for_bb->index] != 0)
@@ -748,7 +753,7 @@ handle_abnormal_edges (basic_block *disp
   /* Check if there are any basic blocks that need to have
 abnormal edges to this dispatcher.  If there are none, return
 early.  */
-  if (bb_to_omp_idx == NULL)
+  if (bb_to_omp_idx.is_empty ())
{
  if (bbs->is_empty ())
return;
@@ -790,7 +795,7 @@ handle_abnormal_edges (basic_block *disp
 
  FOR_EACH_VEC_ELT (*bbs, idx, bb)
{
- if (bb_to_omp_idx
+ if (!bb_to_omp_idx.is_empty ()
  && bb_to_omp_idx[bb->index] != bb_to_omp_idx[for_bb->index])
continue;
 
@@ -820,7 +825,7 @@ handle_abnormal_edges (basic_block *disp
  /* Create predecessor edges of the dispatcher.  */
  FOR_EACH_VEC_ELT (*bbs, idx, bb)
{
- if (bb_to_omp_idx
+ if (!bb_to_omp_idx.is_empty ()
  && bb_to_omp_idx[bb->index] != bb_to_omp_idx[for_bb->index])
continue;
  make_edge (bb, *dispatcher, EDGE_ABNORMAL);
@@ -957,7 +962,6 @@ make_edges (void)
   struct omp_region *cur_region = NULL;
   auto_vec ab_edge_goto;
   auto_vec ab_edge_call;
-  int *bb_to_omp_idx = NULL;
   int cur_omp_region_idx = 0;
 
   /* Create an edge from entry to the first block with executable
@@ -971,7 +975,7 @@ make_edges (void)
 {
   int mer;
 
-  if (bb_to_omp_idx)
+  if (!bb_to_omp_idx.is_empty ())
bb_to_omp_idx[bb->index] = cur_omp_region_idx;
 
   mer = make_edges_bb (bb, &cur_region, &cur_omp_region_idx);
@@ -980,8 +984,8 @@ make_edges (void)
   else if (mer == 2)
ab_edge_call.safe_push (bb);
 
-  if (cur_region && bb_to_omp_i

Re: [PATCH] c++: adc_unify deduction with constrained auto [PR99365]

2021-03-05 Thread Jason Merrill via Gcc-patches

On 3/5/21 3:42 PM, Patrick Palka wrote:

On Fri, 5 Mar 2021, Jason Merrill wrote:


On 3/4/21 9:55 PM, Patrick Palka wrote:

On Thu, 4 Mar 2021, Patrick Palka wrote:


On Thu, 4 Mar 2021, Patrick Palka wrote:


On Thu, 4 Mar 2021, Jason Merrill wrote:


On 3/4/21 11:32 AM, Patrick Palka wrote:

On Thu, 4 Mar 2021, Patrick Palka wrote:


My recent r11-7454 changed the way do_auto_deduction handles
constrained
placeholders during template argument deduction (context ==
adc_unify)
when processing_template_decl != 0.

Before the patch, when processing_template_decl != 0 we would just
ignore the constraints on the placeholder in this situation, and
proceed
with deduction:

 /* Check any placeholder constraints against the deduced type.
*/
 if (flag_concepts && !processing_template_decl)
   if (tree check = NON_ERROR (PLACEHOLDER_TYPE_CONSTRAINTS
(auto_node)))
 {
   ...

After the patch, we now punt and return the original placeholder
type:

 /* Check any placeholder constraints against the deduced type.
*/
 if (flag_concepts)
   if (NON_ERROR (PLACEHOLDER_TYPE_CONSTRAINTS (auto_node)))
 {
   if (processing_template_decl)
 /* In general we can't check satisfaction until we
know all
template arguments.  */
 return type;
   ...

While this change fixed instances where we'd prematurely resolve a
constrained placeholder return or variable type with non-dependent
initializer at template parse time (such as PR96444), it broke the
adc_unify callers that rely on this previous behavior.

So this patch restores the previous behavior during adc_unify
deduction
while retaining the new behavior only during adc_variable_type or
adc_return_type deduction.


Sure, it makes sense for adc_unify to behave differently, since in
deduction
context constraints are checked after all template args have been
deduced.


I see.




We additionally now need to pass outer template arguments to
do_auto_deduction during unify, for sake of constraint checking.
But we don't want do_auto_deduction to substitute these outer
arguments
into type if it's already been done, hence the added
TEMPLATE_TYPE_LEVEL
check.

This fixes partial specializations of non-nested templates with
constrained 'auto' template parameters, but nested templates are
still
broken, ultimately because most_specialized_partial_spec passes
only the
innermost template arguments to get_partial_spec_bindings, and so
outer_targs during do_auto_deduction (called from unify) contains
only
the innermost template arguments which makes satisfaction unhappy.
Fixing this might be too invasive at this stage, perhaps..  (Seems
we
need to make most_specialized_partial_spec pass all template
arguments
to get_partial_spec_bindings.)


How did this work before?


Before, it would work, but only if the constraint didn't also depend on
any outer template arguments.  do_auto_deduction would just "surgically"
replace the auto in the concept-id with the type we deduced and leave
the other template arguments of the concept-id alone.  So if the
constraint was non-dependent, satisfaction would work regardless of the
template nesting level.

Now that we try to do perform satisfaction properly, we're sensitive to
the template nesting level even if the constraint is otherwise
non-dependent, because the template nesting level determines the level
of the auto that appears inside the constraint.  So we rely on
outer_targs to contain all levels of outer template arguments, because
we tack on another level to the end of outer_targs which needs to
map to the level of the auto for satisfaction.

(As a hacky workaround, when outer_targs is incomplete, can probably
just augment it with empty levels until it's
TEMPLATE_TYPE_LEVEL(auto_node)-1
levels deep, which would fix the nested template case as long as the
constraint was depended only on the innermost level of template
arguments.)




Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
OK for
trunk?  Also tested on range-v3 and cmcstl2.


Here's the same patch generated with -w which hides the noisy
indentation
changes:

-- >8 --

PR c++/99365
* pt.c (do_auto_deduction): When processing_template_decl != 0
and context is adc_unify and we have constraints, pretend the
constraints are satisfied instead of punting.  Add some
clarifying sanity checks.  Don't substitute outer_targs into
type if not needed.

gcc/testsuite/ChangeLog:

PR c++/99365
* g++.dg/cpp2a/concepts-partial-spec9.C: New test.
---
gcc/cp/pt.c   | 24
++-
.../g++.dg/cpp2a/concepts-partial-spec9.C | 24
+++
2 files changed, 42 insertions(+), 6 deletions(-)
create mode 100644
gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec9.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index a4686e0affb..ce537e4529a 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ 

Re: [PATCH] c++: Fix constexpr evaluation of pre-increment when !lval [PR99287]

2021-03-05 Thread Jason Merrill via Gcc-patches

On 3/5/21 1:05 PM, Patrick Palka wrote:

Here, during cxx_eval_increment_expression (with lval=false) of
++__first where __first is &"mystr"[0], we correctly update __first
to &"mystr"[1] but we end up returning &"mystr"[0] + 1 instead of
&"mystr"[1].  This unreduced return value inhibits other pointer
arithmetic folding during later constexpr evaluation, which ultimately
causes the constexpr evaluation to fail.

It turns out the simplification of &"mystr"[0] + 1 to &"mystr"[1]
is performed by cxx_fold_pointer_plus_expression, not by fold_build2.
So we perform this simplification during constexpr evaluation of
the temporary MODIFY_EXPR (assigning to __first the simplified value),
but then we return 'mod' which has only been folded via fold_build2 and
hasn't gone through cxx_fold_pointer_plus_expression.

This patch fixes this by updating 'mod' to the (rvalue) result of the
MODIFY_EXPR evaluation, so that we capture any additional folding of
'mod'.  We now need to be wary of the evaluation failing and returning
e.g. the MODIFY_EXPR or NULL_TREE; it seems checking *non_constant_p
should cover our bases here and is generally prudent.

(Finally, since returning 'mod' instead of 'op' when !lval seems to be
more than just an optimization, i.e. callers seems to expect this
behavior, this patch additionally clarifies the nearby comment to that
effect.)

Boostrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk or perhaps GCC 12?

gcc/cp/ChangeLog:

PR c++/99287
* constexpr.c (cxx_eval_increment_expression): Pass lval=false
when evaluating the MODIFY_EXPR, and update 'mod' with the
result of this evaluation.  Check *non_constant_p afterwards.
Clarify nearby comment.

gcc/testsuite/ChangeLog:

PR c++/99287
* g++.dg/cpp2a/constexpr-99287.C: New test.

Co-authored-by: Jakub Jelinek 
---
  gcc/cp/constexpr.c   | 16 ++---
  gcc/testsuite/g++.dg/cpp2a/constexpr-99287.C | 61 
  2 files changed, 67 insertions(+), 10 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-99287.C

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index cd0a68e9fd6..49df79837ca 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -5582,20 +5582,16 @@ cxx_eval_increment_expression (const constexpr_ctx 
*ctx, tree t,
/* Storing the modified value.  */
tree store = build2_loc (cp_expr_loc_or_loc (t, input_location),
   MODIFY_EXPR, type, op, mod);
-  cxx_eval_constant_expression (ctx, store,
-   true, non_constant_p, overflow_p);
+  mod = cxx_eval_constant_expression (ctx, store, false,


How about passing lval down here and returning mod either way?


+ non_constant_p, overflow_p);
ggc_free (store);
+  if (*non_constant_p)
+return t;
  
/* And the value of the expression.  */

if (code == PREINCREMENT_EXPR || code == PREDECREMENT_EXPR)
-{
-  /* Prefix ops are lvalues.  */
-  if (lval)
-   return op;
-  else
-   /* But we optimize when the caller wants an rvalue.  */
-   return mod;
-}
+/* Prefix ops are lvalues, but the caller might want an rvalue.  */
+return lval ? op : mod;
else
  /* Postfix ops are rvalues.  */
  return val;
diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-99287.C 
b/gcc/testsuite/g++.dg/cpp2a/constexpr-99287.C
new file mode 100644
index 000..c9c2ac2f7c2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-99287.C
@@ -0,0 +1,61 @@
+// PR c++/99287
+// { dg-do compile { target c++20 } }
+
+namespace std {
+struct source_location {
+  static consteval source_location
+  current(const void *__p = __builtin_source_location()) {
+source_location __ret;
+__ret._M_impl = static_cast(__p);
+return __ret;
+  }
+  constexpr const char *function_name() {
+return _M_impl ? _M_impl->_M_function_name : "";
+  }
+  struct __impl {
+const char *_M_file_name;
+const char *_M_function_name;
+unsigned _M_line;
+unsigned _M_column;
+  } const *_M_impl;
+};
+struct char_traits {
+  static constexpr long length(const char *__s) {
+return __builtin_strlen(__s);
+  }
+};
+template 
+class basic_string_view {
+public:
+  using traits_type = _Traits;
+  using size_type = unsigned long;
+  constexpr basic_string_view(const _CharT *__str)
+  : _M_len{traits_type::length(__str)}, _M_str{__str} {}
+  constexpr size_type find(const _CharT *, size_type, size_type) const 
noexcept;
+  constexpr size_type find(_CharT *__str) {
+long __trans_tmp_1 = traits_type::length(__str);
+return find(__str, 0, __trans_tmp_1);
+  }
+  long _M_len;
+  const _CharT *_M_str;
+};
+using string_view = basic_string_view;
+template 
+constexpr unsigned long
+basic_string_view<_CharT, _Traits>::find(const _CharT *__str, size_type,
+ size_type __n) const noexcept {
+  int __

Re: [PATCH] c++: Fix tsubsting member variable template-id [PR96330]

2021-03-05 Thread Jason Merrill via Gcc-patches

On 3/5/21 1:04 PM, Patrick Palka wrote:

This makes tsubst_copy appropriately handle a variable template-id, which
in turn fixes tsubsting a COMPONENT_REF whose member operand is known at
parse time to be a variable template-id, as in the initialization of 'x'
in the first testcase.  Previously, we rejected this testcase with the
error "foo_t::bar is not a function template", issued from
lookup_template_fuction.

We were already properly handling the analagous case where the object
operand of the COMPONENT_REF is dependent (and so the member operand is
a dependent template name), but there doesn't seems to be existing test
coverage for this, hence the second testcase below.

Bootstrapped and regtested on x86_64-pc-linux-gnu.  Does this look OK
for trunk or perhaps GCC 12?


OK for trunk.


gcc/cp/ChangeLog:

PR c++/96330
* pt.c (tsubst_copy) : Rename local
variable 'fn' to 'tmpl'.  Handle a variable template-id by
calling lookup_template_variable.

gcc/testsuite/ChangeLog:

PR c++/96330
* g++.dg/cpp1y/var-templ68.C: New test.
* g++.dg/cpp1y/var-templ68a.C: New test.

Co-authored-by: Jakub Jelinek 
---
  gcc/cp/pt.c   |  9 ++---
  gcc/testsuite/g++.dg/cpp1y/var-templ68.C  | 15 +++
  gcc/testsuite/g++.dg/cpp1y/var-templ68a.C | 16 
  3 files changed, 37 insertions(+), 3 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp1y/var-templ68.C
  create mode 100644 gcc/testsuite/g++.dg/cpp1y/var-templ68a.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index ec869451cd2..c956815ce85 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -17108,14 +17108,17 @@ tsubst_copy (tree t, tree args, tsubst_flags_t 
complain, tree in_decl)
  case TEMPLATE_ID_EXPR:
{
/* Substituted template arguments */
-   tree fn = TREE_OPERAND (t, 0);
+   tree tmpl = TREE_OPERAND (t, 0);
tree targs = TREE_OPERAND (t, 1);
  
-	fn = tsubst_copy (fn, args, complain, in_decl);

+   tmpl = tsubst_copy (tmpl, args, complain, in_decl);
if (targs)
  targs = tsubst_template_args (targs, args, complain, in_decl);
  
-	return lookup_template_function (fn, targs);

+   if (variable_template_p (tmpl))
+ return lookup_template_variable (tmpl, targs);
+   else
+ return lookup_template_function (tmpl, targs);
}
  
  case TREE_LIST:

diff --git a/gcc/testsuite/g++.dg/cpp1y/var-templ68.C 
b/gcc/testsuite/g++.dg/cpp1y/var-templ68.C
new file mode 100644
index 000..4c560d4bd35
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/var-templ68.C
@@ -0,0 +1,15 @@
+// PR c++/96330
+// { dg-do compile { target c++14 } }
+
+struct foo_t {
+  template  static constexpr bool bar = true;
+};
+constexpr foo_t foo{};
+
+template 
+void f() {
+  int x = foo.bar;
+  int y = foo_t::bar;
+}
+
+template void f();
diff --git a/gcc/testsuite/g++.dg/cpp1y/var-templ68a.C 
b/gcc/testsuite/g++.dg/cpp1y/var-templ68a.C
new file mode 100644
index 000..6091a03a004
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/var-templ68a.C
@@ -0,0 +1,16 @@
+// PR c++/96330
+// { dg-do compile { target c++14 } }
+
+template 
+struct foo_t {
+  template  static constexpr bool bar = true;
+};
+template  constexpr foo_t foo{};
+
+template 
+void f() {
+  int x = foo.template bar;
+  int y = foo_t::template bar;
+}
+
+template void f();





Re: [PATCH] c++: ICE with -Wshadow and enumerator in template [PR99120]

2021-03-05 Thread Jason Merrill via Gcc-patches

On 3/5/21 11:44 AM, Marek Polacek wrote:

We crash here, because in a template, an enumerator doesn't have
a type until we've called finish_enum_value_list.  But our -Wshadow
implementation, check_local_shadow, is called when we pushdecl in
build_enumerator, which takes place before finish_enum_value_list.

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


OK.


gcc/cp/ChangeLog:

PR c++/99120
* name-lookup.c (check_local_shadow): Check if the type of decl
is non-null before checking TYPE_PTR*.

gcc/testsuite/ChangeLog:

PR c++/99120
* g++.dg/warn/Wshadow-17.C: New test.
---
  gcc/cp/name-lookup.c   |  7 ---
  gcc/testsuite/g++.dg/warn/Wshadow-17.C | 11 +++
  2 files changed, 15 insertions(+), 3 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/warn/Wshadow-17.C

diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index f57708700c2..092fa6b8768 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -3309,7 +3309,7 @@ check_local_shadow (tree decl)
/* Don't warn for artificial things that are not implicit typedefs.  */
if (DECL_ARTIFICIAL (decl) && !DECL_IMPLICIT_TYPEDEF_P (decl))
  return;
-
+
if (nonlambda_method_basetype ())
  if (tree member = lookup_member (current_nonlambda_class_type (),
 DECL_NAME (decl), /*protect=*/0,
@@ -3321,8 +3321,9 @@ check_local_shadow (tree decl)
   is a function or a pointer-to-function.  */
if (!OVL_P (member)
|| TREE_CODE (decl) == FUNCTION_DECL
-   || TYPE_PTRFN_P (TREE_TYPE (decl))
-   || TYPE_PTRMEMFUNC_P (TREE_TYPE (decl)))
+   || (TREE_TYPE (decl)
+   && (TYPE_PTRFN_P (TREE_TYPE (decl))
+   || TYPE_PTRMEMFUNC_P (TREE_TYPE (decl)
  {
auto_diagnostic_group d;
if (warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wshadow,
diff --git a/gcc/testsuite/g++.dg/warn/Wshadow-17.C 
b/gcc/testsuite/g++.dg/warn/Wshadow-17.C
new file mode 100644
index 000..0dee397796f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wshadow-17.C
@@ -0,0 +1,11 @@
+// PR c++/99120
+// { dg-options "-Wshadow" }
+
+struct S {
+  void X();
+
+  template
+  void fn () {
+enum { X };
+  }
+};

base-commit: 4d66685e49d20e0c7a87c5fa0757c7eb63ffcdaa





Re: [PATCH] c++: Pointer-to-member fn conversion with noexcept [PR99374]

2021-03-05 Thread Jason Merrill via Gcc-patches

On 3/4/21 6:19 PM, Marek Polacek wrote:

The issue in this PR is that we wrongly reject converting pointers to
member function of incomplete types, one of which has noexcept.  Recall
that pointers (including pointers to member functions) to non-throwing
functions can be implicitly converted to potentially-throwing functions
(but not vice versa).

We reject the conversion when called from can_convert_arg_bad because
standard_conversion can't create such a conversion.  It comes down to
the DERIVED_FROM_P check in the TYPE_PTRMEMFUNC_P block.  It considers
every class derived from itself, but not when the class is incomplete.
But surely we want to reach fnptr_conv_p when tbase is fbase (one of
them could be an alias to the other so use same_type_p instead of ==).

Another approach would be to not perform DERIVED_FROM_P at all when
either tbase or fbase are incomplete (so perhaps something like at the
end of ptr_reasonably_similar).

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

gcc/cp/ChangeLog:

PR c++/99374
* call.c (standard_conversion): When converting pointers to
member, don't return NULL when the bases are equivalent but
incomplete.

gcc/testsuite/ChangeLog:

PR c++/99374
* g++.dg/cpp1z/noexcept-type23.C: New test.
---
  gcc/cp/call.c|  4 +++-
  gcc/testsuite/g++.dg/cpp1z/noexcept-type23.C | 14 ++
  2 files changed, 17 insertions(+), 1 deletion(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp1z/noexcept-type23.C

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 123f06b1f2b..605cbe15f4e 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -1449,7 +1449,9 @@ standard_conversion (tree to, tree from, tree expr, bool 
c_cast_p,
tree fbase = class_of_this_parm (fromfn);
tree tbase = class_of_this_parm (tofn);
  
-  if (!DERIVED_FROM_P (fbase, tbase))

+  /* If FBASE and TBASE are equivalent but incomplete, DERIVED_FROM_P
+yields false.  But a pointer to member of incomplete class is OK.  */
+  if (!DERIVED_FROM_P (fbase, tbase) && !same_type_p (fbase, tbase))


Let's check same_type_p first.  OK with that change.


return NULL;
  
tree fstat = static_fn_type (fromfn);

diff --git a/gcc/testsuite/g++.dg/cpp1z/noexcept-type23.C 
b/gcc/testsuite/g++.dg/cpp1z/noexcept-type23.C
new file mode 100644
index 000..612dd6ceb5e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/noexcept-type23.C
@@ -0,0 +1,14 @@
+// PR c++/99374
+// { dg-do compile { target c++17 } }
+
+struct S;
+struct R;
+using F1 = int (S::*)();
+using F2 = int (S::*)() noexcept;
+using F3 = int (R::*)() noexcept;
+using T = S;
+using F4 = int (T::*)() noexcept;
+F1 f21 = F2();
+F1 f41 = F4();
+F2 f12 = F1(); // { dg-error "cannot convert" }
+F1 f31 = F3(); // { dg-error "cannot convert" }

base-commit: 0d737ed2171165ba39ab5647f8a94c588fc9a898





Re: [PATCH] c++: -Wconversion vs value-dependent expressions [PR99331]

2021-03-05 Thread Jason Merrill via Gcc-patches

On 3/4/21 9:37 PM, Marek Polacek wrote:

This PR complains that we issue a -Wconversion warning in

   template  struct X {};
   template  X foo();

saying "conversion from 'long unsigned int' to 'int' may change value".
While it's not technically wrong, I suspect -Wconversion warnings aren't
all that useful for value-dependent expressions.  So this patch disables
them, though I'm open to other ideas.


How about suppressing -Wconversion in 
build_converted_constant_expr_internal?  If the size_t value ended up 
being too large for the int parameter, we would give an error about 
overflow in a constant expression, not just a warning.


Jason



Re: [PATCH] testsuite: Fix up attr-flatten-1.c failure [PR99363]

2021-03-05 Thread Jason Merrill via Gcc-patches

On 3/5/21 3:44 PM, Jakub Jelinek wrote:

On Wed, Feb 24, 2021 at 07:08:34PM -0500, Jason Merrill via Gcc-patches wrote:

gcc/ChangeLog:

PR c++/96078
* cgraphunit.c (process_function_and_variable_attributes): Don't
warn about flatten on an alias if the target also has it.
* cgraph.h (symtab_node::get_alias_target_tree): New.

gcc/testsuite/ChangeLog:

PR c++/96078
* g++.dg/ext/attr-flatten1.C: New test.


This broke the gcc.dg/attr-flatten-1.c test, where we no longer
warn because attribute flatten is both on the alias target and on the alias.

- warning_at (DECL_SOURCE_LOCATION (node->decl), OPT_Wattributes,
- "% attribute is ignored on aliases");
+ tree tdecl = node->get_alias_target_tree ();
+ if (!tdecl || !DECL_P (tdecl)
+ || !lookup_attribute ("flatten", DECL_ATTRIBUTES (tdecl)))
+   warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wattributes,
+   "% attribute is ignored on aliases");


Seems that was the intentional change though, so can we apply
following change to fix that?


We should still test that we get the warning if the target doesn't have 
the attribute.  I'll fix the test, thanks.



Bootstrapped/regtested on x86_64-linux and i686-linux:

2021-03-05  Jakub Jelinek  

PR c/99363
* gcc.dg/attr-flatten-1.c: Remove dg-warning directive.

--- gcc/testsuite/gcc.dg/attr-flatten-1.c.jj2020-03-19 18:13:21.776787973 
+0100
+++ gcc/testsuite/gcc.dg/attr-flatten-1.c   2021-03-05 15:18:27.176637165 
+0100
@@ -10,7 +10,7 @@ int fn1(int p1)
  }
  __attribute__((flatten))
  __attribute__((alias("fn1")))
-int fn4(int p1); /* { dg-warning "ignored" } */
+int fn4(int p1);
  int
  test ()
  {


Jakub





[Patch] tree-nested: Update assert for Fortran module vars [PR97927]

2021-03-05 Thread Tobias Burnus

Nested functions are permitted for C but not C++ as extension.
They are also permitted for Fortran, which generates DECL_CONTEXT
== NAMESPACE_DECL for module variables.

That causes the gcc_assert (decl_function_context (decl) == info->context)
to fail in tree-nested.c's lookup_field_for_decl.

The call happens in convert_local_reference_stmt for:
2524case GIMPLE_ASSIGN:
2525  if (gimple_clobber_p (stmt))
2528  if (DECL_P (lhs)
2529  && !use_pointer_in_frame (lhs)
2530  && lookup_field_for_decl (info, lhs, NO_INSERT))

The latter runs into the assert mentioned above. And the
'= {CLOBBER}' occurs in gfortran due to the intent(out).

As additional ingredient, a nested (internal) procedure involved,
obviously as otherwise tree-nested.c wouldn't be involved.

The patch fixes the assert and in terms of the assert it makes
sense, but I am not sure whether there are assumptions
elsewhere which are wrong for NAMESPACE_DECL.

OK for mainline? GCC 10?

Tobias

-
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München 
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank 
Thürauf
tree-nested: Update assert for Fortran module vars [PR97927]

gcc/ChangeLog:

	PR fortran/97927
	* tree-nested.c (lookup_field_for_decl): Also permit
	that the var is a Fortran module var (= namespace context).

gcc/testsuite/ChangeLog:

	PR fortran/97927
	* gfortran.dg/module_variable_3.f90: New test.

 gcc/testsuite/gfortran.dg/module_variable_3.f90 | 37 +
 gcc/tree-nested.c   |  3 +-
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/gcc/testsuite/gfortran.dg/module_variable_3.f90 b/gcc/testsuite/gfortran.dg/module_variable_3.f90
new file mode 100644
index 000..0dae6d5bdd5
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/module_variable_3.f90
@@ -0,0 +1,37 @@
+! { dg-do compile }
+! { dg-additional-options "-fdump-tree-original" }
+!
+! PR fortran/97927
+!
+! Did ICE due to the in tree-nested.c due to {clobber}
+!
+
+module mpi2
+ interface
+   subroutine MPI_Allreduce(i)
+ implicit none
+ INTEGER, OPTIONAL, INTENT(OUT) :: i
+   end subroutine MPI_Allreduce
+ end interface
+end module
+
+module modmpi
+  implicit none
+  integer ierror  ! module variable = context NAMESPACE_DECL
+end module
+
+subroutine exxengy
+  use modmpi
+  use mpi2, only: mpi_allreduce
+  implicit none
+
+  ! intent(out) implies: ierror = {clobber}
+  call mpi_allreduce(ierror)
+
+contains
+  subroutine zrho2
+return
+  end subroutine
+end subroutine
+
+! { dg-final { scan-tree-dump "ierror = {CLOBBER};" "original" } }
diff --git a/gcc/tree-nested.c b/gcc/tree-nested.c
index addd6eef9ab..cedeccac40b 100644
--- a/gcc/tree-nested.c
+++ b/gcc/tree-nested.c
@@ -382,7 +382,8 @@ static tree
 lookup_field_for_decl (struct nesting_info *info, tree decl,
 		   enum insert_option insert)
 {
-  gcc_checking_assert (decl_function_context (decl) == info->context);
+  gcc_checking_assert (decl_function_context (decl) == info->context
+		   || TREE_CODE (DECL_CONTEXT (decl)) == NAMESPACE_DECL);
 
   if (insert == NO_INSERT)
 {


Re: [PATCH] c++: ICE with real-to-int conversion in template [PR97973]

2021-03-05 Thread Jason Merrill via Gcc-patches

On 3/3/21 7:55 PM, Marek Polacek wrote:

In this test we are building a call in a template, but since neither
the function nor any of its arguments are dependent, we go down the
normal path in finish_call_expr.  convert_arguments sees that we're
binding a reference to int to double and therein convert_to_integer
creates a FIX_TRUNC_EXPR.  Later, we call check_function_arguments
which folds the arguments, and, in a template, fold_for_warn calls
fold_non_dependent_expr.  But tsubst_copy_and_build should not see
a FIX_TRUNC_EXPR (see the patch discussed in
)
or we crash.


This again, sigh.  Let me take a step back.

So basically, the output of convert_like_real in a template is a mix of 
template and non-template trees, and thus unsuitable for consumption by 
anything other than grabbing its type and throwing it away, as most 
callers do.


The problem here is that cp_build_function_call_vec calls 
check_function_arguments with these trees.  build_over_call, however, 
does not call check_function_arguments in a template.  Preventing that 
call in a template also fixes the testcase, though it regresses 
diagnostic location in Wnonnull5.C (which it shouldn't, that's a 
separate bug).


IMPLICIT_CONV_EXPR is a way to represent the conversion so that the 
result is still a template tree, and therefore suitable for 
fold_for_warn, which allows us to warn when parsing the template, which 
is generally desirable.


I think the approach of expanding IMPLICIT_CONV_EXPR is probably the 
right choice, but perhaps we should expand it to all non-trivial 
conversions, not just those that would use problematic tree codes.



So let's not create a FIX_TRUNC_EXPR in a template in the first place
and instead use IMPLICIT_CONV_EXPR.

(I'm not sure why tsubst* also doesn't crash on a FLOAT_EXPR; it'd
make sense to me to also create an IMPLICIT_CONV_EXPR for integer
to real convs and add "case FLOAT_EXPR" just under FIX_TRUNC_EXPR.
But perhaps that's too risky to do now.)

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

gcc/cp/ChangeLog:

PR c++/97973
* call.c (convert_like): When converting real to integer in
a template, use IMPLICIT_CONV_EXPR.

gcc/testsuite/ChangeLog:

PR c++/97973
* g++.dg/conversion/real-to-int1.C: New test.
---
  gcc/cp/call.c  |  7 ++-
  gcc/testsuite/g++.dg/conversion/real-to-int1.C | 17 +
  2 files changed, 23 insertions(+), 1 deletion(-)
  create mode 100644 gcc/testsuite/g++.dg/conversion/real-to-int1.C

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 123f06b1f2b..8074d8fdba8 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -8062,7 +8062,12 @@ convert_like (conversion *convs, tree expr, tree fn, int 
argnum,
tree conv_expr = NULL_TREE;
if (processing_template_decl
&& convs->kind != ck_identity
-  && (CLASS_TYPE_P (convs->type) || CLASS_TYPE_P (TREE_TYPE (expr
+  && (CLASS_TYPE_P (convs->type)
+ || CLASS_TYPE_P (TREE_TYPE (expr))
+ /* Converting real to integer produces FIX_TRUNC_EXPR which
+tsubst also doesn't grok.  */
+ || (SCALAR_FLOAT_TYPE_P (TREE_TYPE (expr))
+ && INTEGRAL_OR_ENUMERATION_TYPE_P (convs->type
  {
conv_expr = build1 (IMPLICIT_CONV_EXPR, convs->type, expr);
if (convs->kind != ck_ref_bind)
diff --git a/gcc/testsuite/g++.dg/conversion/real-to-int1.C 
b/gcc/testsuite/g++.dg/conversion/real-to-int1.C
new file mode 100644
index 000..f7b990b3f4b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/conversion/real-to-int1.C
@@ -0,0 +1,17 @@
+// PR c++/97973
+
+void (*foo[1])(const int &);
+void (*foo2[1])(const double &);
+
+template
+void f ()
+{
+  (foo[0])(1.1);
+  (foo2[0])(1);
+}
+
+void
+g ()
+{
+  f ();
+}

base-commit: 8d57bdadd2d9c2e5c95515ca7a583d7b407b55c4





Re: [PATCH] c++: Fix constexpr evaluation of pre-increment when !lval [PR99287]

2021-03-05 Thread Patrick Palka via Gcc-patches
On Fri, 5 Mar 2021, Jason Merrill wrote:

> On 3/5/21 1:05 PM, Patrick Palka wrote:
> > Here, during cxx_eval_increment_expression (with lval=false) of
> > ++__first where __first is &"mystr"[0], we correctly update __first
> > to &"mystr"[1] but we end up returning &"mystr"[0] + 1 instead of
> > &"mystr"[1].  This unreduced return value inhibits other pointer
> > arithmetic folding during later constexpr evaluation, which ultimately
> > causes the constexpr evaluation to fail.
> > 
> > It turns out the simplification of &"mystr"[0] + 1 to &"mystr"[1]
> > is performed by cxx_fold_pointer_plus_expression, not by fold_build2.
> > So we perform this simplification during constexpr evaluation of
> > the temporary MODIFY_EXPR (assigning to __first the simplified value),
> > but then we return 'mod' which has only been folded via fold_build2 and
> > hasn't gone through cxx_fold_pointer_plus_expression.
> > 
> > This patch fixes this by updating 'mod' to the (rvalue) result of the
> > MODIFY_EXPR evaluation, so that we capture any additional folding of
> > 'mod'.  We now need to be wary of the evaluation failing and returning
> > e.g. the MODIFY_EXPR or NULL_TREE; it seems checking *non_constant_p
> > should cover our bases here and is generally prudent.
> > 
> > (Finally, since returning 'mod' instead of 'op' when !lval seems to be
> > more than just an optimization, i.e. callers seems to expect this
> > behavior, this patch additionally clarifies the nearby comment to that
> > effect.)
> > 
> > Boostrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> > trunk or perhaps GCC 12?
> > 
> > gcc/cp/ChangeLog:
> > 
> > PR c++/99287
> > * constexpr.c (cxx_eval_increment_expression): Pass lval=false
> > when evaluating the MODIFY_EXPR, and update 'mod' with the
> > result of this evaluation.  Check *non_constant_p afterwards.
> > Clarify nearby comment.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > PR c++/99287
> > * g++.dg/cpp2a/constexpr-99287.C: New test.
> > 
> > Co-authored-by: Jakub Jelinek 
> > ---
> >   gcc/cp/constexpr.c   | 16 ++---
> >   gcc/testsuite/g++.dg/cpp2a/constexpr-99287.C | 61 
> >   2 files changed, 67 insertions(+), 10 deletions(-)
> >   create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-99287.C
> > 
> > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
> > index cd0a68e9fd6..49df79837ca 100644
> > --- a/gcc/cp/constexpr.c
> > +++ b/gcc/cp/constexpr.c
> > @@ -5582,20 +5582,16 @@ cxx_eval_increment_expression (const constexpr_ctx
> > *ctx, tree t,
> > /* Storing the modified value.  */
> > tree store = build2_loc (cp_expr_loc_or_loc (t, input_location),
> >MODIFY_EXPR, type, op, mod);
> > -  cxx_eval_constant_expression (ctx, store,
> > -   true, non_constant_p, overflow_p);
> > +  mod = cxx_eval_constant_expression (ctx, store, false,
> 
> How about passing lval down here and returning mod either way?

Sounds good, like this?  Testing in progress

-- >8 --

Subject: [PATCH] c++: Fix constexpr evaluation of pre-increment when !lval
 [PR99287]

Here, during cxx_eval_increment_expression (with lval=false) of
++__first where __first is &"mystr"[0], we correctly update __first
to &"mystr"[1] but we end up returning &"mystr"[0] + 1 instead of
&"mystr"[1].  This unreduced return value inhibits other pointer
arithmetic folding during later constexpr evaluation, which ultimately
causes the constexpr evaluation to fail.

It turns out the simplification of &"mystr"[0] + 1 to &"mystr"[1]
is performed by cxx_fold_pointer_plus_expression, not by fold_build2.
So we perform this simplification during constexpr evaluation of
the temporary MODIFY_EXPR (during which we assign to __first the
simplified value), but then we return 'mod' which has only been folded
via fold_build2 and hasn't gone through cxx_fold_pointer_plus_expression.

This patch fixes this by also updating 'mod' with the result of the
MODIFY_EXPR evaluation appropriately, so that we capture any additional
folding of the expression when !lval.  We now need to be wary of this
evaluation failing and returning e.g. the MODIFY_EXPR or NULL_TREE; it
seems checking *non_constant_p should cover our bases here and is
generally prudent.

gcc/cp/ChangeLog:

PR c++/99287
* constexpr.c (cxx_eval_increment_expression): Pass lval when
evaluating the MODIFY_EXPR, and update 'mod' with the result of
this evaluation.  Check *non_constant_p afterwards.  For prefix
ops, just return 'mod'.

gcc/testsuite/ChangeLog:

PR c++/99287
* g++.dg/cpp2a/constexpr-99287.C: New test.

Co-authored-by: Jakub Jelinek 
---
 gcc/cp/constexpr.c   | 17 +++---
 gcc/testsuite/g++.dg/cpp2a/constexpr-99287.C | 61 
 2 files changed, 68 insertions(+), 10 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-99287.C

diff --git a/gcc/c

[PATCH] c++: ICE on invalid with inheriting constructors [PR94751]

2021-03-05 Thread Marek Polacek via Gcc-patches
This is an ICE on invalid where we crash because since r269032 we
keep error_mark_node around instead of using noexcept_false_spec
when things go wrong; see the walk_field_subobs hunk.

We crash in deduce_inheriting_ctor which calls synthesized_method_walk
to deduce the exception-specification, but fails to do so in this case,
because the testcase is invalid so get_nsdmi returns error_mark_node for
the member 'c', and per r269032 the error_mark_node propagates back to
deduce_inheriting_ctor which subsequently calls build_exception_variant
whereon we crash.  I think we should return early if the deduction fails
and I decided to call mark_used to get an error right away instead of
hoping that it would get called later.  My worry is that we could forget
that there was an error and think that we just deduced noexcept(false).

And then I noticed that the test still crashes in C++98.  Here again we
failed to deduce the exception-specification in implicitly_declare_fn,
but nothing reported an error between synthesized_method_walk and the
assert.  Well, not much we can do except calling synthesized_method_walk
again, this time in the verbose mode and making sure that we did get an
error.

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

gcc/cp/ChangeLog:

PR c++/94751
* call.c (build_over_call): Maybe call mark_used in case
deduce_inheriting_ctor fails and return error_mark_node.
* cp-tree.h (deduce_inheriting_ctor): Adjust declaration.
* method.c (deduce_inheriting_ctor): Return bool if the deduction
fails.
(implicitly_declare_fn): If raises is error_mark_node, call
synthesized_method_walk with diag being true.

gcc/testsuite/ChangeLog:

PR c++/94751
* g++.dg/cpp0x/inh-ctor37.C: New test.
---
 gcc/cp/call.c   |  9 +++--
 gcc/cp/cp-tree.h|  2 +-
 gcc/cp/method.c | 22 -
 gcc/testsuite/g++.dg/cpp0x/inh-ctor37.C | 26 +
 4 files changed, 51 insertions(+), 8 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/inh-ctor37.C

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 123f06b1f2b..fa42165c209 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -8917,8 +8917,13 @@ build_over_call (struct z_candidate *cand, int flags, 
tsubst_flags_t complain)
   /* OK, we're actually calling this inherited constructor; set its deletedness
  appropriately.  We can get away with doing this here because calling is
  the only way to refer to a constructor.  */
-  if (DECL_INHERITED_CTOR (fn))
-deduce_inheriting_ctor (fn);
+  if (DECL_INHERITED_CTOR (fn)
+  && !deduce_inheriting_ctor (fn))
+{
+  if (complain & tf_error)
+   mark_used (fn);
+  return error_mark_node;
+}
 
   /* Make =delete work with SFINAE.  */
   if (DECL_DELETED_FN (fn))
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 39e2ad83abd..587cf35a271 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6919,7 +6919,7 @@ extern bool is_xible  (enum 
tree_code, tree, tree);
 extern tree get_defaulted_eh_spec  (tree, tsubst_flags_t = 
tf_warning_or_error);
 extern bool maybe_explain_implicit_delete  (tree);
 extern void explain_implicit_non_constexpr (tree);
-extern void deduce_inheriting_ctor (tree);
+extern bool deduce_inheriting_ctor (tree);
 extern bool decl_remember_implicit_trigger_p   (tree);
 extern void synthesize_method  (tree);
 extern tree lazily_declare_fn  (special_function_kind,
diff --git a/gcc/cp/method.c b/gcc/cp/method.c
index 3fe3bd82a7d..25c1e681b99 100644
--- a/gcc/cp/method.c
+++ b/gcc/cp/method.c
@@ -2789,9 +2789,9 @@ explain_implicit_non_constexpr (tree decl)
 
 /* DECL is an instantiation of an inheriting constructor template.  Deduce
the correct exception-specification and deletedness for this particular
-   specialization.  */
+   specialization.  Return true if the deduction succeeds; false otherwise.  */
 
-void
+bool
 deduce_inheriting_ctor (tree decl)
 {
   decl = DECL_ORIGIN (decl);
@@ -2804,6 +2804,8 @@ deduce_inheriting_ctor (tree decl)
   /*diag*/false,
   &inh,
   FUNCTION_FIRST_USER_PARMTYPE (decl));
+  if (spec == error_mark_node)
+return false;
   if (TREE_CODE (inherited_ctor_binfo (decl)) != TREE_BINFO)
 /* Inherited the same constructor from different base subobjects.  */
 deleted = true;
@@ -2818,6 +2820,8 @@ deduce_inheriting_ctor (tree decl)
   TREE_TYPE (clone) = build_exception_variant (TREE_TYPE (clone), spec);
   SET_DECL_INHERITED_CTOR (clone, inh);
 }
+
+  return true;
 }
 
 /* Implicitly declare the special function indicated by KIND, as a
@@ -2993,9 +2997,17 @@ implicitly_declare_fn (special_function_kind kind, tree 
type,
   if (raises != error_mark_node)
  

Re: [commited] [PR99378] LRA: Skip decomposing address for asm insn operand with unknown constraint

2021-03-05 Thread Joseph Myers
This introduced an ICE building glibc for i686.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99422
https://sourceware.org/pipermail/libc-testresults/2021q1/007615.html

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


Committed: gcc.target/cris/pr93372-1.c: Adjust expectations for eliminated stack-frame

2021-03-05 Thread Hans-Peter Nilsson via Gcc-patches
See comment.

* gcc.target/cris/pr93372-1.c: Adjust expected assembler result
to allow an eliminated stack-frame.
---
 gcc/testsuite/gcc.target/cris/pr93372-1.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.target/cris/pr93372-1.c 
b/gcc/testsuite/gcc.target/cris/pr93372-1.c
index 20aa65e8d59d..bc637302dfae 100644
--- a/gcc/testsuite/gcc.target/cris/pr93372-1.c
+++ b/gcc/testsuite/gcc.target/cris/pr93372-1.c
@@ -2,7 +2,16 @@
are filled. */
 /* { dg-do compile } */
 /* { dg-options "-O2" } */
-/* { dg-final { scan-assembler-not "\tnop" } } */
+/* { dg-final { scan-assembler-times "\tnop|addq 8,|subq 8," 2 } } */
+
+/* The reason for the weird variant of scan-assembler-not "\tnop" is that we
+   used to have an unused DWunion temp on stack in xlshrdi3 and delay-slots for
+   the conditional jumps filled by the addq 8/subq setting up that, but with a
+   MAX_FIXED_MODE_SIZE no longer 32, but the default 64, that stack-frame is
+   eliminated, but we no longer have eligible insns to fill the delay-slots.
+   Not wanting to tweak the code in the test-case, this is second best: 
allowing
+   two nops -or- an addq 8 + subq 8 assuming code generation is otherwise
+   reasonably sane.  */
 
 void *f(void **p)
 {
-- 
2.11.0



Committed: cris: don't define MAX_FIXED_MODE_SIZE

2021-03-05 Thread Hans-Peter Nilsson via Gcc-patches
It's been 32 ever since the CRIS port was committed.
A TODO-item of mine has been to check whether the
non-default setting of MAX_FIXED_MODE_SIZE makes sense
wrt. performance and/or code-size with a modern gcc.  It
doesn't, so it goes.  The setting is now the default,
GET_MODE_BITSIZE (DImode) (defaults.h) i.e. 64.

Measurements at r11-7500 (f3641ac70eb0) on coremark with
"-O2 -march=v10 -mno-mul-bug-workaround" shows 0.04%
performance improvement with this change, and by inspection
the effect is that unused and/or unneeded stack-frames are
eliminated more often in the floating-point library (not in
the coremark main loop, thus the marginal improvement).  The
floating-point library is full of 64-bit unions used to pick
apart floating point numbers, so this kind of makes sense.

Inspection of a simulator trace shows that this is indeed
the only effect in coremark.  Other local micro-benchmarks
agree as to the net effect (no traces were inspected
though), and the most floating-point-heavy test shows an 8%
improvement.  These effects are of course subject to gcc
core tweaks and may make sense to be adjusted again in a
future release.

While MAX_FIXED_MODE_SIZE is IMO supposed to be an optional
macro for performance, setting it to anything smaller than
twice the size of an address exposes bad decisions in gcc
middle end, sometimes leading to internal compiler errors.
(It being set to 32 should *not* affect use of DImode as an
integer mode; it's for "integer machine modes of this size
or smaller can be used for structures and unions with the
appropriate sizes".)  Thus, with the default 64 instead of
32, there are two tests that now pass for the first time:
gcc.dg/attr-vector_size.c and gcc.dg/tree-ssa/pr93121-1.c.

gcc:
* config/cris/cris.h (MAX_FIXED_MODE_SIZE): Don't define.
---
 gcc/config/cris/cris.h | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/gcc/config/cris/cris.h b/gcc/config/cris/cris.h
index d691da9723c8..1f8ccc5dec98 100644
--- a/gcc/config/cris/cris.h
+++ b/gcc/config/cris/cris.h
@@ -352,13 +352,6 @@ extern int cris_cpu_version;
with other GNU/Linux ports (i.e. elfos.h users).  */
 #undef PCC_BITFIELD_TYPE_MATTERS
 
-/* This is only used for non-scalars.  Strange stuff happens to structs
-   (FIXME: What?) if we use anything larger than largest actually used
-   datum size, so lets make it 32.  The type "long long" will still work
-   as usual.  We can still have DImode insns, but they will only be used
-   for scalar data (i.e. long long).  */
-#define MAX_FIXED_MODE_SIZE 32
-
 
 /* Node: Type Layout */
 
-- 
2.11.0