Re: PING [Patch][Middle-end]Add -fzero-call-used-regs=[skip|used-gpr|all-gpr|used|all]

2020-08-10 Thread Kees Cook via Gcc-patches
On Mon, Aug 10, 2020 at 11:39:26PM -0300, Alexandre Oliva wrote:
> Erhm, I don't get why it's important that they be zeroed.  It seems to
> me that restoring their original values, or setting them to random
> values, would be just as good defenses from having them set within the

In the performance analysis I looked at a while ago, doing the
register-self-xor is extremely fast to run (IIRC the cycle counts on x86
were absolutely tiny), and it's smaller for code size which minimized
the overall image footprint.

> [...]
> Code that sets the register to zero in the epilogue would be much harder
> for an attacker to change indeed.

Yes, a fixed value is a significantly better defensive position to take
for ROP. And specifically zero _tends_ to be the safest choice as it's
less "useful" to be used as a size, index, or pointer. And, no, it is
not perfect, but nothing can be if we're dealing with trying to defend
against arbitrary ROP gadget finding (or uninitialized stack contents,
where the same argument for "zero is best" also holds[1]).

-Kees

[1] https://lists.llvm.org/pipermail/cfe-dev/2020-April/065221.html

-- 
Kees Cook


[Bug tree-optimization/96563] Failure to optimize loop with condition to simple arithmetic

2020-08-10 Thread glisse at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96563

Marc Glisse  changed:

   What|Removed |Added

   Last reconfirmed||2020-08-11
 Ever confirmed|0   |1
   Severity|normal  |enhancement
 Status|UNCONFIRMED |NEW
   Keywords||missed-optimization

--- Comment #1 from Marc Glisse  ---
At -O2 (before uncprop)

   [local count: 151290756]:

   [local count: 976138698]:
  # i_9 = PHI <0(2), i_4(6)>
  if (x_3(D) == i_9)
goto ; [5.50%]
  else
goto ; [94.50%]

   [local count: 922451069]:
  i_4 = i_9 + 1;
  if (i_4 != 10)
goto ; [89.42%]
  else
goto ; [10.58%]

   [local count: 97603126]:
  goto ; [100.00%]

   [local count: 824847943]:
  goto ; [100.00%]

   [local count: 53687628]:

   [local count: 151290756]:
  # _2 = PHI <8(7), 4(8)>
  return _2;

We don't really do anything special. At -O3, the loop gets unrolled

   [local count: 151290757]:
  if (x_3(D) == 0)
goto ; [5.50%]
  else
goto ; [94.50%]

   [local count: 8320992]:
  goto ; [100.00%]

   [local count: 142969766]:
  if (x_3(D) == 1)
goto ; [5.50%]
  else
goto ; [94.50%]

   [local count: 7863337]:
  goto ; [100.00%]

   [local count: 135106428]:
  if (x_3(D) == 2)
goto ; [5.50%]
  else
goto ; [94.50%]

   [local count: 7430853]:
  goto ; [100.00%]

   [local count: 127675576]:
  if (x_3(D) == 3)
goto ; [5.50%]
  else
goto ; [94.50%]

   [local count: 7022157]:
  goto ; [100.00%]

   [local count: 120653419]:
  if (x_3(D) == 4)
goto ; [5.50%]
  else
goto ; [94.50%]

   [local count: 6635938]:
  goto ; [100.00%]

   [local count: 114017483]:
  if (x_3(D) == 5)
goto ; [5.50%]
  else
goto ; [94.50%]

   [local count: 6270962]:
  goto ; [100.00%]

   [local count: 107746521]:
  if (x_3(D) == 6)
goto ; [5.50%]
  else
goto ; [94.50%]

   [local count: 5926059]:
  goto ; [100.00%]

   [local count: 101820460]:
  if (x_3(D) == 7)
goto ; [5.50%]
  else
goto ; [94.50%]

   [local count: 5600125]:
  goto ; [100.00%]

   [local count: 96220334]:
  if (x_3(D) == 8)
goto ; [5.50%]
  else
goto ; [94.50%]

   [local count: 5292118]:
  goto ; [100.00%]

   [local count: 90928219]:
  if (x_3(D) == 9)
goto ; [5.50%]
  else
goto ; [94.50%]

   [local count: 5001052]:
  goto ; [100.00%]

   [local count: 97603126]:

   [local count: 151290756]:
  # _2 = PHI <8(14), 4(12), 8(22), 8(21), 8(20), 8(19), 8(18), 8(17), 8(16),
8(15), 8(23)>
  return _2;

We have code in reassoc to handle x==0||x==1||x==2 and turn it into a range
test. I suspect the issue is related to those empty bb between the condition
and the PHI that hides the fact that they are jumping to the same place
eventually. Fixing this for the unrolled case is thus probably easiest,
although of course it would be nice if it also worked for 99 instead of 9,
where we are not going to unroll.

[Bug target/96558] [11 Regression] ICE in extract_constrain_insn, at recog.c:2195 (error: insn does not satisfy its constraints)

2020-08-10 Thread asolokha at gmx dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96558

--- Comment #1 from Arseny Solokha  ---
The internal loop can be simplified a bit:

  do
{
  long int lf = (long int) f1 ? h1 : 0;

  ky += lf;
  vh = lf | f1;
  f1 = 1;
}
  while (vh < (f1 ^ 2));

Problem cropping up in Value Range Propogation

2020-08-10 Thread Gary Oblock via Gcc
I'm trying to debug a problem cropping up in value range propagation.
Ironically I probably own an original copy 1995 copy of the paper it's
based on but that's not going to be much help since I'm lost in the
weeds.  It's running on some optimization (my structure reorg
optimization) generated GIMPLE statements.

Here's the GIMPLE dump:

Function max_of_y (max_of_y, funcdef_no=1, decl_uid=4391, cgraph_uid=2, 
symbol_order=20) (executed once)

max_of_y (unsigned long data, size_t len)
{
  double value;
  double result;
  size_t i;

   [local count: 118111600]:
  field_arry_addr_14 = _reorg_base_var_type_t.y;
  index_15 = (sizetype) data_27(D);
  offset_16 = index_15 * 8;
  field_addr_17 = field_arry_addr_14 + offset_16;
  field_val_temp_13 = MEM  [(void *)field_addr_17];
  result_8 = field_val_temp_13;
  goto ; [100.00%]

   [local count: 955630225]:
  _1 = i_3 * 16;
  PPI_rhs1_cast_18 = (unsigned long) data_27(D);
  PPI_rhs2_cast_19 = (unsigned long) _1;
  PtrPlusInt_Adj_20 = PPI_rhs2_cast_19 / 16;
  PtrPlusInt_21 = PPI_rhs1_cast_18 + PtrPlusInt_Adj_20;
  dedangled_27 = (unsigned long) PtrPlusInt_21;
  field_arry_addr_23 = _reorg_base_var_type_t.y;
  index_24 = (sizetype) dedangled_27;
  offset_25 = index_24 * 8;
  field_addr_26 = field_arry_addr_23 + offset_25;
  field_val_temp_22 = MEM  [(void *)field_addr_26];
  value_11 = field_val_temp_22;
  if (result_5 < value_11)
goto ; [50.00%]
  else
goto ; [50.00%]

   [local count: 477815112]:

   [local count: 955630225]:
  # result_4 = PHI 
  i_12 = i_3 + 1;

   [local count: 1073741824]:
  # i_3 = PHI <1(2), i_12(5)>
  # result_5 = PHI 
  if (i_3 < len_9(D))
goto ; [89.00%]
  else
goto ; [11.00%]

   [local count: 118111600]:
  # result_10 = PHI 
  return result_10;
}

The failure in VRP is occurring on

offset_16 = data_27(D) * 8;

which is the from two adjacent statements above

  index_15 = (sizetype) data_27(D);
  offset_16 = index_15 * 8;

being merged together.

Note, the types of index_15/16 are sizetype and data_27 is unsigned
long.
The error message is:

internal compiler error: tree check: expected class ‘type’, have ‘exceptional’ 
(error_mark) in to_wide,

Things only start to look broken in value_range::lower_bound in
value-range.cc when

return wi::to_wide (t);

is passed error_mark_node in t. It's getting it from m_min just above.
My observation is that m_min is not always error_mark_node. In fact, I
seem to think you need to use set_varying to get this to even happen.

Note, the ssa_propagation_engine processed the statement "offset_16 =
data..."  multiple times before failing on it. What oh what is
happening and how in the heck did I cause it???

Please, somebody throw me a life preserver on this.

Thanks,

Gary


CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, is for 
the sole use of the intended recipient(s) and contains information that is 
confidential and proprietary to Ampere Computing or its subsidiaries. It is to 
be used solely for the purpose of furthering the parties' business 
relationship. Any review, copying, or distribution of this email (or any 
attachments thereto) is strictly prohibited. If you are not the intended 
recipient, please contact the sender immediately and permanently delete the 
original and any copies of this email and any attachments thereto.


[Bug target/96072] ICE: Segmentation fault (in add_reg_note)

2020-08-10 Thread asolokha at gmx dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96072

--- Comment #1 from Arseny Solokha  ---
testsuite/gcc.target/s390/20041109-1.c is another similar case which clobbers
SP on PowerPC. Maybe it's time to turn

  warning: listing the stack pointer register 'sp' in a clobber list is
deprecated

into a hard error and be done with it?

[PATCH] [PR target/96350]Force ENDBR immediate into memory to avoid fake ENDBR opcode.

2020-08-10 Thread Hongtao Liu via Gcc-patches
Hi:
  The issue is described in the bugzilla.
  Bootstrap is ok, regression test for i386/x86-64 backend is ok.
  Ok for trunk?

ChangeLog
gcc/
PR target/96350
* config/i386/i386.c (ix86_legitimate_constant_p): Return
false for ENDBR immediate.
(ix86_legitimate_address_p): Ditto.
* config/i386/predicated.md
(x86_64_immediate_operand): Exclude ENDBR immediate.
(x86_64_zext_immediate_operand): Ditto.
(x86_64_dwzext_immediate_operand): Ditto.
(ix86_not_endbr_immediate_operand): New predicate.

gcc/testsuite
* gcc.target/i386/endbr_immediate.c: New test.

-- 
BR,
Hongtao
From 073517f01e8872e23b2dda5e6e25142ad4cfe274 Mon Sep 17 00:00:00 2001
From: liuhongt 
Date: Tue, 4 Aug 2020 10:00:13 +0800
Subject: [PATCH] Force ENDBR immediate into memory.

gcc/
	PR target/96350
	* config/i386/i386.c (ix86_legitimate_constant_p): Return
	false for ENDBR immediate.
	(ix86_legitimate_address_p): Ditto.
	* config/i386/predicated.md
	(x86_64_immediate_operand): Exclude ENDBR immediate.
	(x86_64_zext_immediate_operand): Ditto.
	(x86_64_dwzext_immediate_operand): Ditto.
	(ix86_not_endbr_immediate_operand): New predicate.

gcc/testsuite
	* gcc.target/i386/endbr_immediate.c: New test.
---
 gcc/config/i386/i386.c|   5 +-
 gcc/config/i386/predicates.md |  33 +++
 .../gcc.target/i386/endbr_immediate.c | 198 ++
 3 files changed, 235 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/endbr_immediate.c

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 8ea6a4d7ea7..228efb60a72 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -10080,7 +10080,7 @@ ix86_legitimate_constant_p (machine_mode mode, rtx x)
 }
 
   /* Otherwise we handle everything else in the move patterns.  */
-  return true;
+  return ix86_not_endbr_immediate_operand (x, VOIDmode);
 }
 
 /* Determine if it's legal to put X into the constant pool.  This
@@ -10568,6 +10568,9 @@ ix86_legitimate_address_p (machine_mode, rtx addr, bool strict)
 	return false;
 }
 
+  if (disp && !ix86_not_endbr_immediate_operand (disp, VOIDmode))
+return false;
+
   /* Everything looks valid.  */
   return true;
 }
diff --git a/gcc/config/i386/predicates.md b/gcc/config/i386/predicates.md
index 07e69d555c0..47e65892d94 100644
--- a/gcc/config/i386/predicates.md
+++ b/gcc/config/i386/predicates.md
@@ -130,10 +130,38 @@
 (define_predicate "symbol_operand"
   (match_code "symbol_ref"))
 
+;; Return true if VALUE isn't an ENDBR opcode in immediate field.
+(define_predicate "ix86_not_endbr_immediate_operand"
+  (match_test "1")
+{
+  if ((flag_cf_protection & CF_BRANCH)
+  && CONST_INT_P (op))
+ {
+   unsigned HOST_WIDE_INT imm = INTVAL (op);
+   if (!TARGET_64BIT || imm <= 0x)
+	 return imm != (TARGET_64BIT ? 0xfa1e0ff3 : 0xfb1e0ff3);
+
+   /* NB: Encoding is byte based.  */
+   do
+	 {
+	  if ((0x & imm) == 0xfa1e0ff3)
+	return false;
+	  imm >>= 8;
+	 }
+   while (imm > 0x);
+
+   return true;
+  }
+  return true;
+})
+
 ;; Return true if VALUE can be stored in a sign extended immediate field.
 (define_predicate "x86_64_immediate_operand"
   (match_code "const_int,symbol_ref,label_ref,const")
 {
+  if (!ix86_not_endbr_immediate_operand (op, VOIDmode))
+return false;
+
   if (!TARGET_64BIT)
 return immediate_operand (op, mode);
 
@@ -260,6 +288,9 @@
 (define_predicate "x86_64_zext_immediate_operand"
   (match_code "const_int,symbol_ref,label_ref,const")
 {
+  if (!ix86_not_endbr_immediate_operand (op, VOIDmode))
+return false;
+
   switch (GET_CODE (op))
 {
 case CONST_INT:
@@ -374,6 +405,8 @@
 (define_predicate "x86_64_dwzext_immediate_operand"
   (match_code "const_int,const_wide_int")
 {
+  if (!ix86_not_endbr_immediate_operand (op, VOIDmode))
+return false;
   switch (GET_CODE (op))
 {
 case CONST_INT:
diff --git a/gcc/testsuite/gcc.target/i386/endbr_immediate.c b/gcc/testsuite/gcc.target/i386/endbr_immediate.c
new file mode 100644
index 000..3015512aa0e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/endbr_immediate.c
@@ -0,0 +1,198 @@
+/* PR target/96350 */
+/* { dg-do compile } */
+/* { dg-options "-fcf-protection -O2" } */
+/* { dg-final { scan-assembler-not "$-81915917" { target { ia32 } } } } */
+/* { dg-final { scan-assembler-not "$-98693133" { target { ! ia32 } } } } *
+/* { dg-final { scan-assembler-not "$-423883778574778368" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler "\[ \t\]*-81915917" { target { ia32 } } } } */
+/* { dg-final { scan-assembler "\[ \t\]*-98693133" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler "\[ \t\]*-423883778574778368" { target { ! ia32 } } } } */
+
+
+#ifdef __x86_64__
+#define ENDBR_IMMEDIATE 0xfa1e0ff3
+#define EXTEND_ENDBR_IMMEDIATE 0xfa1e0ff3
+#else
+#define ENDBR_IMMEDIATE 0xfb1e0ff3
+#define 

[Bug target/96551] [10/11 Regression] FAIL: gcc.target/i386/vectorize8.c (internal compiler error)

2020-08-10 Thread crazylht at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96551

--- Comment #1 from Hongtao.liu  ---
For `vec_unpacku_float_hi_v16si` `vec_unpacku_float_lo_v16si`

---
diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md
index cf083ca28aa..2e60f596bc1 100644
--- a/gcc/config/i386/sse.md
+++ b/gcc/config/i386/sse.md
@@ -6973,7 +6973,7 @@

   emit_insn (gen_vec_extract_hi_v16si (tmp[3], operands[1]));
   emit_insn (gen_floatv8siv8df2 (tmp[2], tmp[3]));
-  emit_insn (gen_rtx_SET (k, gen_rtx_LT (QImode, tmp[2], tmp[0])));
+  ix86_expand_mask_vec_cmp (k, LT, tmp[2], tmp[0]);
   emit_insn (gen_addv8df3_mask (tmp[2], tmp[2], tmp[1], tmp[2], k));
   emit_move_insn (operands[0], tmp[2]);
   DONE;
@@ -7020,7 +7020,7 @@
   k = gen_reg_rtx (QImode);

   emit_insn (gen_avx512f_cvtdq2pd512_2 (tmp[2], operands[1]));
-  emit_insn (gen_rtx_SET (k, gen_rtx_LT (QImode, tmp[2], tmp[0])));
+  ix86_expand_mask_vec_cmp (k, LT, tmp[2], tmp[0]);
   emit_insn (gen_addv8df3_mask (tmp[2], tmp[2], tmp[1], tmp[2], k));
   emit_move_insn (operands[0], tmp[2]);
   DONE;
---

Re: PING [Patch][Middle-end]Add -fzero-call-used-regs=[skip|used-gpr|all-gpr|used|all]

2020-08-10 Thread Alexandre Oliva
On Aug  7, 2020, Qing Zhao  wrote:

> So, I believe that the call-used registers (especially those registers
> that pass parameters) need to be zeroed
> In order to mitigate the ROP attack. 

Erhm, I don't get why it's important that they be zeroed.  It seems to
me that restoring their original values, or setting them to random
values, would be just as good defenses from having them set within the
function to perform a ROP attack than zeroing them.  The point is to get
rid of whatever value the attacker chose within the function.  One could
even argue that restoring the caller value is better than setting to
zero, because the result is not predictable from within the function.

OTOH, there's the flip side, that the function *could* be changed so as
to modify the stack slot in which the register is saved, if there's
hostile code running.  (it wouldn't be modified by "normal" code)

Code that sets the register to zero in the epilogue would be much harder
for an attacker to change indeed.


> I think that moving how to zeroing the registers part to each target
> will be a better solution since each target has
> Better idea on how to use the most efficient insns to do the work.

It's certainly good to allow machine-specific optimized code sequences,
but it would certainly be desirable to have a machine-independent
fallback.  It doesn't seem exceedingly hard to loop over the registers
and emit a (set (reg:M N) (const_int 0)) for each one that is to be
zeroed out.


-- 
Alexandre Oliva, happy hacker
https://FSFLA.org/blogs/lxo/
Free Software Activist
GNU Toolchain Engineer


[Bug c++/96164] Constraints and explicit template instantiation

2020-08-10 Thread ppalka at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96164

Patrick Palka  changed:

   What|Removed |Added

 Resolution|--- |FIXED
   Target Milestone|--- |10.3
 Status|ASSIGNED|RESOLVED

--- Comment #6 from Patrick Palka  ---
This should be fixed for GCC 10.3+, thanks for the report.

[Bug c++/96164] Constraints and explicit template instantiation

2020-08-10 Thread cvs-commit at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96164

--- Comment #5 from CVS Commits  ---
The releases/gcc-10 branch has been updated by Patrick Palka
:

https://gcc.gnu.org/g:215927a736d21d8cff8baeb50f687911a00149b9

commit r10-8602-g215927a736d21d8cff8baeb50f687911a00149b9
Author: Patrick Palka 
Date:   Wed Jul 29 22:06:33 2020 -0400

c++: constraints and explicit instantiation [PR96164]

When considering to instantiate a member of a class template as part of
an explicit instantiation of the class template, we need to first check
the member's constraints before proceeding with the instantiation of the
member.

gcc/cp/ChangeLog:

PR c++/96164
* constraint.cc (constraints_satisfied_p): Return true if
!flags_concepts.
* pt.c (do_type_instantiation): Update a paragraph taken from
[temp.explicit] to reflect the latest specification.  Don't
instantiate a member with unsatisfied constraints.

gcc/testsuite/ChangeLog:

PR c++/96164
* g++.dg/cpp2a/concepts-explicit-inst5.C: New test.

(cherry picked from commit dc3d1e181445fafbbd146eb355a750c41c338794)

Re: PING [PATCH] RX new builtin function

2020-08-10 Thread Oleg Endo
On Mon, 2020-08-10 at 13:51 +0300, Darius Galis wrote:
> 
> I've found the following patch 
> https://gcc.gnu.org/legacy-ml/gcc-patches/2018-11/msg00983.html, but it 
> is not in the latest sources.
> Could please let me know why it was not added? I'm willing to do any 
> rework necessary in order for it to be accepted to the latest sources.

I think it'd be better to fix and/or improve the backend code so that
the compiler generates the bset instruction automatically.  Otherwise
this built-in is not very useful for user code.  Except for the use
case of atomic-or on byte memory location, as a "side effect".  But
that should be implemented as such -- as atomics.

There are a couple of PRs that might be relevant/interesting

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93587
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83832
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81823

Cheers,
Oleg



[Bug target/96562] Rather poor assembly generated for copy-list-initialization in return statement.

2020-08-10 Thread hjl.tools at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96562

H.J. Lu  changed:

   What|Removed |Added

 Ever confirmed|0   |1
 Status|UNCONFIRMED |NEW
   Last reconfirmed||2020-08-11

--- Comment #2 from H.J. Lu  ---
Add -mavx to -O2 triggers this.

[Bug gcov-profile/96534] keep gcov intermediate format

2020-08-10 Thread xlwu at synopsys dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96534

--- Comment #3 from xlwu at synopsys dot com ---
(In reply to xlwu from comment #2)
> (In reply to Martin Liška from comment #1)
> > (In reply to xlwu from comment #0)
> > > since gcc9, the gcov did not support intermediate format and replace with
> > > json format , our application deeply depend on intermediate format , is it
> > > possible to restore the intermediate format ?
> > 
> > No, the support was removed from the GCC.
> > 
> > > or could you let me know any
> > > workaround ? 
> > 
> > Sure, it's very easy. One can get the very same information from the new
> > JSON format (which is fairly simple):
> > 
> > https://gcc.gnu.org/onlinedocs/gcc/Invoking-Gcov.html
> > 
> > > 
> > > background :
> > > I am a software engineer from SNPS , we have an application that extract 
> > > all
> > > file+functions and it's related lines per test case, and we save this data
> > > into DB which help us to predict the test to verify when RnD updated any
> > > lines in source code before their check in. we call this application as
> > > "smart regression". 
> > 
> > That's a nice usage of the GCOV and the new format should simplify your 
> > life.
> > 
> > > 
> > > now , when gcov move to json file, it increase the size a lot which affect
> > > the efficiency to parse the data
> > 
> > I'm aware of the limitation. Note that the JSON format is compressed with
> > gzip.
> > Can you please share more statistics?
> 
> I did not have the real statistics yet as our company is not yet moved to
> gcc9 (maybe end of this year ).
> 
> and even the size of the compressed file is smaller , but we have to unzip
> and parse it , right? it just increase the workload . 
> 
> however, the stdout option ( -t ) could be very helpful , we don't need to
> write to disk and just parse the stdout content.
> 
> well, I also notice that the "function_name" in "lines" object is mangled
> even I use "--demangled-names" option, can you print the demangled_name when
> "-m" option given ? ( yes I know there is demangled name in "functions" part
> ) . the reason is: we need to know the exact function name and it's lines,
> in current format , we have to find all demangled function name and it's
> start and end line number, then check the lines part to assign each lines
> into functions .

OK, maybe this is not a good idea , I guess this will increase the size much
larger 

> > 
> > > , what's worse, we had to revise our code
> > > to support the new json format while we need to support the old format in
> > > the same time , as our company have many products and each product have 
> > > many
> > > live branches , some of them still using gcc6 version.
> > 
> > I see. As mentioned, porting to the new format should be fairly simple.
> > 
> > > 
> > > I tried to use older gcov version on the new gcc instructed gcda and gcno
> > > file , it did not work.
> > 
> > Please don't do it.

[Bug tree-optimization/96563] New: Failure to optimize loop with condition to simple arithmetic

2020-08-10 Thread gabravier at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96563

Bug ID: 96563
   Summary: Failure to optimize loop with condition to simple
arithmetic
   Product: gcc
   Version: 11.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: tree-optimization
  Assignee: unassigned at gcc dot gnu.org
  Reporter: gabravier at gmail dot com
  Target Milestone: ---

int f(int x)
{
int i = 0;
while (i <= 9)
{
if (i == x)
return 8;
++i;
}
return 4;
}

This can be optimized to `return 4 + ((x <= 9) * 4);`. This transformation is
done by LLVM, but not by GCC.

[Bug gcov-profile/96534] keep gcov intermediate format

2020-08-10 Thread xlwu at synopsys dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96534

--- Comment #2 from xlwu at synopsys dot com ---
(In reply to Martin Liška from comment #1)
> (In reply to xlwu from comment #0)
> > since gcc9, the gcov did not support intermediate format and replace with
> > json format , our application deeply depend on intermediate format , is it
> > possible to restore the intermediate format ?
> 
> No, the support was removed from the GCC.
> 
> > or could you let me know any
> > workaround ? 
> 
> Sure, it's very easy. One can get the very same information from the new
> JSON format (which is fairly simple):
> 
> https://gcc.gnu.org/onlinedocs/gcc/Invoking-Gcov.html
> 
> > 
> > background :
> > I am a software engineer from SNPS , we have an application that extract all
> > file+functions and it's related lines per test case, and we save this data
> > into DB which help us to predict the test to verify when RnD updated any
> > lines in source code before their check in. we call this application as
> > "smart regression". 
> 
> That's a nice usage of the GCOV and the new format should simplify your life.
> 
> > 
> > now , when gcov move to json file, it increase the size a lot which affect
> > the efficiency to parse the data
> 
> I'm aware of the limitation. Note that the JSON format is compressed with
> gzip.
> Can you please share more statistics?

I did not have the real statistics yet as our company is not yet moved to gcc9
(maybe end of this year ).

and even the size of the compressed file is smaller , but we have to unzip and
parse it , right? it just increase the workload . 

however, the stdout option ( -t ) could be very helpful , we don't need to
write to disk and just parse the stdout content.

well, I also notice that the "function_name" in "lines" object is mangled even
I use "--demangled-names" option, can you print the demangled_name when "-m"
option given ? ( yes I know there is demangled name in "functions" part ) . the
reason is: we need to know the exact function name and it's lines, in current
format , we have to find all demangled function name and it's start and end
line number, then check the lines part to assign each lines into functions .

> 
> > , what's worse, we had to revise our code
> > to support the new json format while we need to support the old format in
> > the same time , as our company have many products and each product have many
> > live branches , some of them still using gcc6 version.
> 
> I see. As mentioned, porting to the new format should be fairly simple.
> 
> > 
> > I tried to use older gcov version on the new gcc instructed gcda and gcno
> > file , it did not work.
> 
> Please don't do it.

[pushed] c++: Add unfixed test [PR88003]

2020-08-10 Thread Marek Polacek via Gcc-patches
Now that dg-ice is available, let's try it out.

Tested x86_64-pc-linux-gnu, applying to trunk.

gcc/testsuite/ChangeLog:

PR c++/88003
* g++.dg/cpp1y/auto-fn61.C: New test.
---
 gcc/testsuite/g++.dg/cpp1y/auto-fn61.C | 13 +
 1 file changed, 13 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/auto-fn61.C

diff --git a/gcc/testsuite/g++.dg/cpp1y/auto-fn61.C 
b/gcc/testsuite/g++.dg/cpp1y/auto-fn61.C
new file mode 100644
index 000..c24c3b85d78
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/auto-fn61.C
@@ -0,0 +1,13 @@
+// PR c++/88003
+// { dg-do compile { target c++14 } }
+// { dg-ice "poplevel_class" }
+
+auto test() {
+  struct O {
+struct N;
+  };
+  return O();
+}
+
+typedef decltype(test()) TN;
+struct TN::N {};

base-commit: f4b9b136808c31118c52c0addafb3fd323484d1b
-- 
2.26.2



[Bug c++/88003] ICE on outside definition of inner function-local class in poplevel_class, at cp/name-lookup.c:4325

2020-08-10 Thread cvs-commit at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88003

--- Comment #3 from CVS Commits  ---
The master branch has been updated by Marek Polacek :

https://gcc.gnu.org/g:c01b22f12291691d1ce89f82211f00eae4398e18

commit r11-2642-gc01b22f12291691d1ce89f82211f00eae4398e18
Author: Marek Polacek 
Date:   Mon Aug 10 19:57:27 2020 -0400

c++: Add unfixed test [PR88003]

Now that dg-ice is available, let's try it out.

gcc/testsuite/ChangeLog:

PR c++/88003
* g++.dg/cpp1y/auto-fn61.C: New test.

Go patch committed: Use eqtype on AIX

2020-08-10 Thread Ian Lance Taylor via Gcc-patches
This patch by Clément Chigot changes the Go frontend and library to
use an eqtype function on AIX.  The AIX linker is not able to merge
identical type descriptors in a single symbol if they are coming from
different object or shared object files.  This results into several
pointers referencing the same type descriptors.  Thus, an eqtype
function is needed to ensure that these different symbols will be
considered as the same type descriptor.  This fixes
https://golang.org/issue/39276.  Bootstrapped and ran Go testsuite on
x86_64-pc-linux-gnu.  Committed to mainline.

Ian

gcc/go/ChangeLog:

* go-c.h (struct go_create_gogo_args): Add need_eqtype field.
* go-lang.c (go_langhook_init): Set need_eqtype.
742d84d0ea7fe6117b92a93e01b4c843f8038b4d
diff --git a/gcc/go/gofrontend/MERGE b/gcc/go/gofrontend/MERGE
index b6089f3f01d..93aa18cec06 100644
--- a/gcc/go/gofrontend/MERGE
+++ b/gcc/go/gofrontend/MERGE
@@ -1,4 +1,4 @@
-307665073fce992ea8112f74b91954e770afcc70
+c512af85eb8c75a759b5e4fc6b72041fe09b75f1
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
diff --git a/gcc/go/gofrontend/expressions.cc b/gcc/go/gofrontend/expressions.cc
index 7e7fb8c7313..d295fd10136 100644
--- a/gcc/go/gofrontend/expressions.cc
+++ b/gcc/go/gofrontend/expressions.cc
@@ -208,7 +208,7 @@ Expression::is_same_variable(Expression* a, Expression* b)
 // assignment.
 
 Expression*
-Expression::convert_for_assignment(Gogo*, Type* lhs_type,
+Expression::convert_for_assignment(Gogo* gogo, Type* lhs_type,
   Expression* rhs, Location location)
 {
   Type* rhs_type = rhs->type();
@@ -229,7 +229,7 @@ Expression::convert_for_assignment(Gogo*, Type* lhs_type,
 location);
 }
   else if (!are_identical && rhs_type->interface_type() != NULL)
-return Expression::convert_interface_to_type(lhs_type, rhs, location);
+return Expression::convert_interface_to_type(gogo, lhs_type, rhs, 
location);
   else if (lhs_type->is_slice_type() && rhs_type->is_nil_type())
 {
   // Assigning nil to a slice.
@@ -498,7 +498,7 @@ Expression::convert_interface_to_interface(Type *lhs_type, 
Expression* rhs,
 // non-interface type.
 
 Expression*
-Expression::convert_interface_to_type(Type *lhs_type, Expression* rhs,
+Expression::convert_interface_to_type(Gogo* gogo, Type *lhs_type, Expression* 
rhs,
   Location location)
 {
   // We are going to evaluate RHS multiple times.
@@ -507,8 +507,11 @@ Expression::convert_interface_to_type(Type *lhs_type, 
Expression* rhs,
   // Build an expression to check that the type is valid.  It will
   // panic with an appropriate runtime type error if the type is not
   // valid.
-  // (lhs_type != rhs_type ? panicdottype(lhs_type, rhs_type, inter_type) :
-  //nil /*dummy*/)
+  // (lhs_type == rhs_type ? nil /*dummy*/ :
+  //panicdottype(lhs_type, rhs_type, inter_type))
+  // For some Oses, we need to call runtime.eqtype instead of
+  // lhs_type == rhs_type, as we may have unmerged type descriptors
+  // from shared libraries.
   Expression* lhs_type_expr = Expression::make_type_descriptor(lhs_type,
 location);
   Expression* rhs_descriptor =
@@ -518,15 +521,23 @@ Expression::convert_interface_to_type(Type *lhs_type, 
Expression* rhs,
   Expression* rhs_inter_expr = Expression::make_type_descriptor(rhs_type,
 location);
 
-  Expression* cond = Expression::make_binary(OPERATOR_NOTEQ, lhs_type_expr,
- rhs_descriptor, location);
+  Expression* cond;
+  if (gogo->need_eqtype()) {
+cond = Runtime::make_call(Runtime::EQTYPE, location,
+  2, lhs_type_expr,
+  rhs_descriptor);
+  } else {
+cond = Expression::make_binary(OPERATOR_EQEQ, lhs_type_expr,
+   rhs_descriptor, location);
+  }
+
   rhs_descriptor = Expression::get_interface_type_descriptor(rhs);
   Expression* panic = Runtime::make_call(Runtime::PANICDOTTYPE, location,
  3, lhs_type_expr->copy(),
  rhs_descriptor,
  rhs_inter_expr);
   Expression* nil = Expression::make_nil(location);
-  Expression* check = Expression::make_conditional(cond, panic, nil,
+  Expression* check = Expression::make_conditional(cond, nil, panic,
location);
 
   // If the conversion succeeds, pull out the value.
diff --git a/gcc/go/gofrontend/expressions.h b/gcc/go/gofrontend/expressions.h
index a4f892acaf7..acb2732bdde 100644
--- a/gcc/go/gofrontend/expressions.h
+++ b/gcc/go/gofrontend/expressions.h
@@ -1273,7 +1273,7 @@ class Expression
   }
 

Re: [PATCH v2] CSKY: Add -mfloat-abi= option.

2020-08-10 Thread Xianmiao Qu

Hi JOJO,


The patch looks good but the changlog lacks a leading text with commit 
description.




On 8/10/20 4:17 PM, Jojo R wrote:

From: jojo 

gcc/ChangeLog:

* config/csky/csky_opts.h (float_abi_type): New.
* config/csky/csky.h (TARGET_SOFT_FLOAT): New.
(TARGET_HARD_FLOAT): New.
(TARGET_HARD_FLOAT_ABI): New.
(OPTION_DEFAULT_SPECS): Use mfloat-abi.
* config/csky/csky.opt (mfloat-abi): New.
* doc/invoke.texi (C-SKY Options): Document -mfloat-abi=.
* gcc/config/csky/csky-elf.h (ASM_SPEC): Use mfloat-abi.
* gcc/config/csky/csky-linux-elf.h (ASM_SPEC): Use mfloat-abi.
---
  gcc/config/csky/csky-elf.h   |  2 ++
  gcc/config/csky/csky-linux-elf.h |  2 ++
  gcc/config/csky/csky.h   |  9 -
  gcc/config/csky/csky.opt | 29 +
  gcc/config/csky/csky_opts.h  |  7 +++
  gcc/doc/invoke.texi  | 18 ++
  6 files changed, 62 insertions(+), 5 deletions(-)



Re: RFC: Monitoring old PRs, new dg directives [v2]

2020-08-10 Thread Mike Stump via Gcc-patches
On Aug 10, 2020, at 2:30 PM, Marek Polacek via Gcc-patches 
 wrote:
> 
> Thanks a lot.  Here's the latest version of my patch, which only adds dg-ice
> at this point.
> 
> So, um, OK?

Ok.

Re: [PATCH] emit-rtl.c: Allow splitting of RTX_FRAME_RELATED_P insns?

2020-08-10 Thread Segher Boessenkool
On Mon, Aug 10, 2020 at 05:16:15PM +0100, Richard Sandiford wrote:
> Senthil Kumar via Gcc-patches  writes:
> >   The wiki suggests using post-reload splitters, so that's the
> >   direction I took, but I ran into an issue where split_insn
> >   bails out early if RTX_FRAME_RELATED_P is true - this means
> >   that splits for REG_CC clobbering insns with
> >   RTX_FRAME_RELATED_P will never execute, resulting in a
> >   could-not-split insn ICE in the final stage.
> >
> >   I see that the recog.c:peep2_attempt allows splitting of a
> >   RTX_FRAME_RELATED_P insn, provided the result of the split is a
> >   single insn. Would it be ok to modify try_split also to
> >   allow those kinds of insns (tentative patch attached, code
> >   copied over from peep2_attempt, only setting old and new_insn)? Or is 
> > there
> >   a different approach to fix this?
> 
> I agree there's no obvious reason why splitting to a single insn
> should be rejected but a peephole2 to a single instruction should be OK.
> And reusing the existing, tried-and-tested code is the way to go.

The only obvious difference is that the splitters run many times, while
peep2 runs only once, very late.  If you make this only do stuff for
reload_completed splitters, that difference is gone as well.

> But could you split the code out of peep2_attempt into a subroutine
> (probably still in recog.c) and reuse it in try_split?

Yes please :-)


Segher


Re: RFC: Monitoring old PRs, new dg directives

2020-08-10 Thread Mike Stump via Gcc-patches
On Aug 7, 2020, at 6:16 AM, Nathan Sidwell  wrote:
> 
> On 8/6/20 8:01 PM, Mike Stump wrote:
>> On Aug 6, 2020, at 7:01 AM, Nathan Sidwell  wrote:
>>> 
 XFAIL: g++.dg/foo.C  -std=c++17 (internal compiler error)
 PASS: g++.dg/foo.C  -std=c++17 (test for excess errors)
 Which one of these would you not like to see?
>>> 
>>> Neither of these is solving the issue.  How do I find the ICES that are 
>>> unexpected, without tripping over the ICEs that are expected?
>> Don't you already have this issue for current xfailed ICEs?  ^FAIL.*internal 
>> compiler error I think finds them, doesn't it?
> 
> Are there xfailed ICEs?  I've not run into them?

I certainly have seen them in various forms and in various places over the 
years; though, you do raise the interesting point, it may only be theory if 
things work well on all the platforms of interest.  So, yeah, adding new ICEs 
can make the search regex longer in practice.


[Bug target/96562] Rather poor assembly generated for copy-list-initialization in return statement.

2020-08-10 Thread maxim.yegorushkin at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96562

--- Comment #1 from Maxim Egorushkin  ---
Correction:

Span f(unsigned char* p, unsigned char* q) {
return {p, static_cast(q - p)};
}

[Bug target/96562] New: Rather poor assembly generated for copy-list-initialization in return statement.

2020-08-10 Thread maxim.yegorushkin at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96562

Bug ID: 96562
   Summary: Rather poor assembly generated for
copy-list-initialization in return statement.
   Product: gcc
   Version: 10.2.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: target
  Assignee: unassigned at gcc dot gnu.org
  Reporter: maxim.yegorushkin at gmail dot com
  Target Milestone: ---

Rather poor assembly generated for trivial code.

The following code:

template
struct Span {
P begin_;
SizeT size_;
};

Span f(char* p, char* q) {
return {p, static_cast(q - p)};
}

When compiled with gcc-6.1 to gcc-10.2 with options "-O3 -march=skylake
-mtune=skylake" produces unexpectedly long and sub-optimal assembly code:

f(unsigned char*, unsigned char*):
mov QWORD PTR [rsp-16], 0
mov QWORD PTR [rsp-24], rdi
sub rsi, rdi
vmovdqa xmm1, XMMWORD PTR [rsp-24]
vpinsrd xmm0, xmm1, esi, 2
vmovdqa XMMWORD PTR [rsp-24], xmm0
mov rax, QWORD PTR [rsp-24]
mov rdx, QWORD PTR [rsp-16]
ret

clang with the same options produces the expected assembly:

f(unsigned char*, unsigned char*):
mov rdx, rsi
mov rax, rdi
sub edx, eax
ret

Is there a way to make gcc produce the expected assembly, please?

https://gcc.godbolt.org/z/bacGW8

Re: RFC: Monitoring old PRs, new dg directives

2020-08-10 Thread Marek Polacek via Gcc-patches
On Mon, Aug 10, 2020 at 08:58:54AM -0400, Nathan Sidwell wrote:
> On 8/10/20 4:48 AM, Richard Sandiford wrote:
> > Marek Polacek via Gcc-patches  writes:
> > > > > > sure.
> > > > > > * develop patch
> > > > > > * run testsuite
> > > > > > * observe unexpected ICEs
> > > > > > * load g++.log into editor
> > > > > > * ^sinternal comp
> > > > > > * gets to first unexpected ICE
> > > > > > * debug it.
> > > > > > 
> > > > > > What does '^sinternal comp' become?  As there could be many 
> > > > > > expected ICEs
> > > > > > it'll be painful to determine whether any particular utterance of 
> > > > > > 'internal
> > > > > > compiler' is expected or not.
> > > > > 
> > > > > That is a problem I don't know how to deal with.  I know how to pass
> > > > > additional options to the compiler from dejagnu.  I thought maybe I 
> > > > > could use
> > > > > -pass-exit-codes, redirect stderr to /dev/null, and check if the exit 
> > > > > code is
> > > > > ICE_EXIT_CODE, but there seems to be no way to do that redirection.  
> > > > > So I'm
> > > > > stuck.
> > > > > 
> > > > > Though, you could just grep for '^FAIL.*internal comp', which will 
> > > > > find the
> > > > > first unexpected ICE.  Contrary to the expected ICEs, the unexpected 
> > > > > ICEs will
> > > > > be shown in 'Excess errors:'.  Won't that work?
> > > > 
> > > > Read what I wrote:
> > > > > > * load g++.log into editor
> > > > > > * ^sinternal comp
> > > > 
> > > > are you telling me not to use an editor for this task?  the search is 
> > > > so one
> > > > can get the command line.  Grepping the log file will not do that.
> > > 
> > > No, I'm saying that looking for '^FAIL.*internal comp' (in an editor) 
> > > instead
> > > of just 'internal comp' will get you to the ICEs you care about.  In vim,
> > > '/^FAIL.*in' gets me there.
> > 
> > Yeah, this works.  I'm not sure whether Nathan's ^s is isearch
> > or regexp-isearch :-)
> 
> It was incremental :) it;s kind of nice just to stop typing when it hits the
> first ICE message!  But, I'm not going to object any more, I guess I'll live
> with it. 

Sorry.

> [I'm sure I can figure out some emacs macro binding to DTRT]

I'm sure you could.  If it becomes painful, I could also add
GCC_TESTSUITE_SKIP_ICE_TESTS (disabled by default) or similar to skip dg-ice
tests.

Marek



Re: [PATCH] c++: premature analysis of requires-expression [PR96410]

2020-08-10 Thread Jason Merrill via Gcc-patches

On 8/10/20 2:18 PM, Patrick Palka wrote:

On Mon, 10 Aug 2020, Patrick Palka wrote:


In the below testcase, semantic analysis of the requires-expressions in
the generic lambda must be delayed until instantiation of the lambda
because the requirements depend on the lambda's template arguments.  But
tsubst_requires_expr does semantic analysis even during regeneration of
the lambda, which leads to various bogus errors and ICEs since some
subroutines aren't prepared to handle dependent/template trees.

This patch adjusts subroutines of tsubst_requires_expr to avoid doing
some problematic semantic analyses when processing_template_decl.
In particular, expr_noexcept_p generally can't be checked on a dependent
expression.  Next, tsubst_nested_requirement should avoid checking
satisfaction when processing_template_decl.  And similarly for
convert_to_void (called from tsubst_valid_expression_requirement).


I wonder if, instead of trying to do a partial substitution into a 
requires-expression at all, we want to use the 
PACK_EXPANSION_EXTRA_ARGS/IF_STMT_EXTRA_ARGS mechanism to remember the 
arguments for later satisfaction?


Jason


Bootstrapped and regtested on x86_64-pc-linux-gnu, and also tested
against the cmcstl2 project.  Does this look OK to commit?

gcc/cp/ChangeLog:

PR c++/96409
PR c++/96410
* constraint.cc (tsubst_compound_requirement): When
processing_template_decl, don't check noexcept of the
substituted expression.
(tsubst_nested_requirement): Just substitute into the constraint
when processing_template_decl.
* cvt.c (convert_to_void): Don't resolve concept checks when
processing_template_decl.

gcc/testsuite/ChangeLog:

PR c++/96409
PR c++/96410
* g++.dg/cpp2a/concepts-lambda13.C: New test.
---
  gcc/cp/constraint.cc  |  9 ++-
  gcc/cp/cvt.c  |  2 +-
  .../g++.dg/cpp2a/concepts-lambda13.C  | 25 +++
  3 files changed, 34 insertions(+), 2 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index e4aace596e7..db2036502a7 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -1993,7 +1993,8 @@ tsubst_compound_requirement (tree t, tree args, 
subst_info info)
  
/* Check the noexcept condition.  */

bool noexcept_p = COMPOUND_REQ_NOEXCEPT_P (t);
-  if (noexcept_p && !expr_noexcept_p (expr, tf_none))
+  if (!processing_template_decl
+  && noexcept_p && !expr_noexcept_p (expr, tf_none))
  return error_mark_node;
  
/* Substitute through the type expression, if any.  */

@@ -2023,6 +2024,12 @@ static tree
  tsubst_nested_requirement (tree t, tree args, subst_info info)
  {
/* Ensure that we're in an evaluation context prior to satisfaction.  */
+  if (processing_template_decl)
+{
+  tree r = tsubst_constraint (TREE_OPERAND (t, 0), args,
+ info.complain, info.in_decl);


Oops, the patch is missing a check for error_mark_node here, so that
upon substitution failure we immediately resolve the requires-expression
to false.  Here's an updated patch with the check and a regression test
added:

-- >8 --

gcc/cp/ChangeLog:

PR c++/96409
PR c++/96410
* constraint.cc (tsubst_compound_requirement): When
processing_template_decl, don't check noexcept of the
substituted expression.
(tsubst_nested_requirement): Just substitute into the constraint
when processing_template_decl.
* cvt.c (convert_to_void): Don't resolve concept checks when
processing_template_decl.

gcc/testsuite/ChangeLog:

PR c++/96409
PR c++/96410
* g++.dg/cpp2a/concepts-lambda13.C: New test.
* g++.dg/cpp2a/concepts-lambda14.C: New test.
---
  gcc/cp/constraint.cc  | 11 +++-
  gcc/cp/cvt.c  |  2 +-
  .../g++.dg/cpp2a/concepts-lambda13.C  | 25 +++
  .../g++.dg/cpp2a/concepts-lambda14.C  | 10 
  4 files changed, 46 insertions(+), 2 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C
  create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index e4aace596e7..5b4964dd36e 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -1993,7 +1993,8 @@ tsubst_compound_requirement (tree t, tree args, 
subst_info info)
  
/* Check the noexcept condition.  */

bool noexcept_p = COMPOUND_REQ_NOEXCEPT_P (t);
-  if (noexcept_p && !expr_noexcept_p (expr, tf_none))
+  if (!processing_template_decl
+  && noexcept_p && !expr_noexcept_p (expr, tf_none))
  return error_mark_node;
  
/* Substitute through the type expression, if any.  */

@@ -2023,6 +2024,14 @@ static tree
  tsubst_nested_requirement (tree t, tree 

Re: RFC: Monitoring old PRs, new dg directives [v2]

2020-08-10 Thread Marek Polacek via Gcc-patches
On Mon, Aug 10, 2020 at 09:48:06AM +0100, Richard Sandiford wrote:
> Marek Polacek via Gcc-patches  writes:
> >> > > sure.
> >> > > * develop patch
> >> > > * run testsuite
> >> > > * observe unexpected ICEs
> >> > > * load g++.log into editor
> >> > > * ^sinternal comp
> >> > > * gets to first unexpected ICE
> >> > > * debug it.
> >> > > 
> >> > > What does '^sinternal comp' become?  As there could be many expected 
> >> > > ICEs
> >> > > it'll be painful to determine whether any particular utterance of 
> >> > > 'internal
> >> > > compiler' is expected or not.
> >> > 
> >> > That is a problem I don't know how to deal with.  I know how to pass
> >> > additional options to the compiler from dejagnu.  I thought maybe I 
> >> > could use
> >> > -pass-exit-codes, redirect stderr to /dev/null, and check if the exit 
> >> > code is
> >> > ICE_EXIT_CODE, but there seems to be no way to do that redirection.  So 
> >> > I'm
> >> > stuck.
> >> > 
> >> > Though, you could just grep for '^FAIL.*internal comp', which will find 
> >> > the
> >> > first unexpected ICE.  Contrary to the expected ICEs, the unexpected 
> >> > ICEs will
> >> > be shown in 'Excess errors:'.  Won't that work?
> >> 
> >> Read what I wrote:
> >> >> * load g++.log into editor
> >> >> * ^sinternal comp
> >> 
> >> are you telling me not to use an editor for this task?  the search is so 
> >> one
> >> can get the command line.  Grepping the log file will not do that.
> >
> > No, I'm saying that looking for '^FAIL.*internal comp' (in an editor) 
> > instead
> > of just 'internal comp' will get you to the ICEs you care about.  In vim,
> > '/^FAIL.*in' gets me there.
> 
> Yeah, this works.  I'm not sure whether Nathan's ^s is isearch
> or regexp-isearch :-)
> 
> > If that is still too much, we could maybe add a new diagnostic kind, like
> > DK_EXPECTED_ICE that has different wording than DK_ICE, and have dg-ice
> > use it via some option/envvar.
> 
> Seems unnecessary given the above.

Yeah, I would like to avoid adding hacks like that.

> >> while solveable, this seems to be the equivalent of putting stones in ones
> >> shoe.  an annoyance one could do without.  I'm sadly not convinced the goal
> >> is worth it.
> >
> > If this is an annoyance to people, I'll just go back to my own repo.
> 
> Please don't!  I'm sure we can find something for Nathan's use case,
> if your regexp search isn't enough.
> 
> How do things stand with the new directives?  Seems like there are no
> objections to dg-ice, but I lost track of the others.  Personally I'm
> not interested in any solution that replaces one of your new directives
> with two existing directives: one (sub)test should be one directive as
> far as possible.  So IMO we should add every directive that doesn't
> already have a one-for-one analogue.

Thanks a lot.  Here's the latest version of my patch, which only adds dg-ice
at this point.

I do agree with adding useful directives that don't have an existing
variant, or whose effect could only be achieved by a cumbersome combination
of existing directives.  Perhaps we'll realize we need another one down the
line.

Here's the patch.  Tested by adding two same foo.C and foo2.C tests that
ICE, only foo.C has { dg-ice "" }, and running

$ make check-c++ RUNTESTFLAGS=dg.exp=foo*.C

The result is

FAIL: g++.dg/foo2.C  -std=c++14 (internal compiler error)
FAIL: g++.dg/foo2.C  -std=c++14 (test for excess errors)

so only the unexpected ICE is displayed.

So, um, OK?  Mike, would you be able to take a look at this patch?

I guess it wouldn't hurt to mention this in
https://gcc.gnu.org/codingconventions.html#Testsuite

Thanks,

-- >8 --
This patch adds a new DejaGNU directive, dg-ice, as outlined in the
proposal here:
https://gcc.gnu.org/pipermail/gcc-patches/2020-July/550913.html

It means that it's expected that the compiler crashes with an internal
compiler error when compiling test with such a directive.

A minor optimization could be to use -pass-exit-codes and then check for
ICE_EXIT_CODE return code instead of using string match.

gcc/ChangeLog:

* doc/sourcebuild.texi: Document dg-ice.

gcc/testsuite/ChangeLog:

* lib/gcc-dg.exp (gcc-dg-test-1): Handle dg-ice.
(cleanup-after-saved-dg-test): Reset expect_ice.
* lib/prune.exp (prune_ices): New.
* lib/target-supports-dg.exp (dg-ice): New.
---
 gcc/doc/sourcebuild.texi | 10 +
 gcc/testsuite/lib/gcc-dg.exp | 20 +++--
 gcc/testsuite/lib/prune.exp  |  9 
 gcc/testsuite/lib/target-supports-dg.exp | 28 
 4 files changed, 65 insertions(+), 2 deletions(-)

diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi
index 63216a0daba..967cb135cb4 100644
--- a/gcc/doc/sourcebuild.texi
+++ b/gcc/doc/sourcebuild.texi
@@ -1172,6 +1172,16 @@ Expect the execute step of a test to fail if the 
conditions (which are
 the same as for @code{dg-skip-if}) are met.
 @end table
 
+@subsubsection 

Re: RFC: make combine do as advertised (cheaper-than)?

2020-08-10 Thread Segher Boessenkool
Hi!

On Mon, Aug 10, 2020 at 05:47:53PM +0200, Hans-Peter Nilsson wrote:
> > All of this was added in https://gcc.gnu.org/g:64b8935d4809 , which also
> > adds the
> > 
> > +  /* Disallow this recombination if both new_cost and old_cost are
> > + greater than zero, and new_cost is greater than old cost.  */
> > 
> > comment (which is what it actually does).  Before that change, combine
> > didn't look at costs at all.
> > 
> > Combine never has used a "strictly smaller than" cost thing.
> 
> (I suggest we use terms of generally greater/lower cost, instead
> of smaller/greater cost, to avoid confusion whether "smaller"
> refers to the general cost or just code size.

It refers to neither.  It refers to the concept of "cost" in combine,
which is a number (and some extra conditions).  Combine is a local
optimisation ("peephole", if you want), so it uses a local cost
function.

And yes, that cost is calculated differently if you use -Os.  The is
because it uses insn_cost to base on.

As it is a number, I (and most other people) use "smaller than".

> > > Right, you can affect this with your target TARGET_RTX_COSTS and
> > > TARGET_INSN_COST hooks, but only for trivial cases, and you have
> > > increasingly more complex combinations (many-to-many
> > > combinations) where you have to twist simple logic to appease
> > > combine (stop it from combining) or give up.
> > 
> > There are 2-1, 3-1, 4-1, 3-2, 4-2, which all reduce the number of insns,
> > and then there is 2-2, which gets rid of one log_link.  If the isnn_cost
> > stays the same, it always wins something else.
> 
> That "something else" is presumptious.

I wrote this code, I am maintainer of combine, I really do know.  This
is not presumptuous.  This is important to make sure combine never can
get into an infinite loop!

> A longer
> dependency-chain may sum up to faster execution considering
> parallel or OoO execution.  Someone adding another N-M
> combination will notice, after two more years of work. :)

Combine knows nothing about how any specific CPU will actually execute
the code.  It sees just a tiny fraction of code at any one time, anyway.

All it does is optimise for the lowest cost.  Whether that actually
performs best, is something the backend writer has to take care of.

(insn_cost is only the secondary "score", anyway: the number of RTL
insns is primary).

> > 5) Improve your target so that its insn_cost reflects ithe costs of
> > the insns better.
> 
> I see cheap gnawing, I hope I didn't add any of that myself.

I guess we both did.  Sorry.

> I already covered target costs before the 1..4 list and you
> actually quoted that (the quoted paragraph above your log_links
> comment).

Let me write this differently, then:

5) Write your target cost callbacks so that you work *with* combine, not
*against* it.  This gives better results.

> To recap, it all began with the observation of comments in
> combine.c saying "cheaper than", with the code implementing
> "same or cheaper", together with the flaw fixed by 84c5396d4bdbf
> which I belive is a sufficient conclusion for that specific
> instance.  A lower cost also seems more consistent with other
> decisions.  (BTW, the commit is a bit misleading; I believe all
> "case #2" cc0 convertees will benefit from an accurate
> is_just_move, not just CRIS.)

It makes a difference on many other targets, but it never results in
different code there.  I wonder why not (or, why it does for cris).

> The fixed 2-2 case is all the "typical examples" I had so far.
> Remaining suggestions are just fixing perceived inconsistencies.

All the other cases reduce the number of RTL insns, which is a clear
cost metric (and originally the only one!)  If you have RTL insns that
correspond to more than just single machine insns, this can hurt you;
otherwise, there are a few cases where you want to trick combine, but
not many (doing that tends to backfire anyway, better keep it to a
minimum).


Segher


Re: [PATCH] Rewrite get_size_range for irange API.

2020-08-10 Thread Martin Sebor via Gcc-patches

On 8/10/20 2:08 PM, Andrew MacLeod wrote:

On 8/10/20 2:46 PM, Martin Sebor wrote:

On 8/10/20 11:50 AM, Andrew MacLeod wrote:

On 8/10/20 12:35 PM, Martin Sebor via Gcc-patches wrote:

On 8/10/20 5:47 AM, Aldy Hernandez wrote:




int_range is the type which allows for up to X subranges. 
calculations will be merged to fit within X subranges
widest_irange is the type which allows for "unlimited" subranges... 
which currently happens to be capped at 255.. . (its typedef'd as 
int_range<255>).


widest_irange is the type used within the range-ops machinery and 
such, and then whatever result is calculated is "toned down" to 
whatever to user provides.


so if union results in [5,10] and [20, MAX]   and you provide a 
value_range for the result (, or int_range<1>), the result you get 
back will be [5, MAX].. so won't look like there are any multi-ranges 
going on.


This is one part of the puzzle (for me).  I don't get [5, MAX] but
[0, MAX], on trunk as well as in GCC 10:

  void f (unsigned n)
  {
    if (!((n >= 5 && n <= 10)
  || (n >= 20)))    // n2 = [5, 10] U [20, UINT_MAX]
  return;

    if (n == 3) // not folded
  __builtin_abort ();
  }

I'd expect this to get optimized regardless of Ranger (Clang folds
the whole function body into a return statement).


You mean like this? (from our branch.optimized output)  :-)

f (unsigned int n)
{
    [local count: 1073741824]:
   return;
}


Sweet!  I want!  ;-) https://www.youtube.com/watch?v=IrCEhRNgGHY

Martin



Re: [PATCH] Rewrite get_size_range for irange API.

2020-08-10 Thread Andrew MacLeod via Gcc-patches

On 8/10/20 2:46 PM, Martin Sebor wrote:

On 8/10/20 11:50 AM, Andrew MacLeod wrote:

On 8/10/20 12:35 PM, Martin Sebor via Gcc-patches wrote:

On 8/10/20 5:47 AM, Aldy Hernandez wrote:




int_range is the type which allows for up to X subranges. 
calculations will be merged to fit within X subranges
widest_irange is the type which allows for "unlimited" subranges... 
which currently happens to be capped at 255.. . (its typedef'd as 
int_range<255>).


widest_irange is the type used within the range-ops machinery and 
such, and then whatever result is calculated is "toned down" to 
whatever to user provides.


so if union results in [5,10] and [20, MAX]   and you provide a 
value_range for the result (, or int_range<1>), the result you get 
back will be [5, MAX].. so won't look like there are any multi-ranges 
going on.


This is one part of the puzzle (for me).  I don't get [5, MAX] but
[0, MAX], on trunk as well as in GCC 10:

  void f (unsigned n)
  {
    if (!((n >= 5 && n <= 10)
  || (n >= 20)))    // n2 = [5, 10] U [20, UINT_MAX]
  return;

    if (n == 3) // not folded
  __builtin_abort ();
  }

I'd expect this to get optimized regardless of Ranger (Clang folds
the whole function body into a return statement).


You mean like this? (from our branch.optimized output)  :-)

f (unsigned int n)
{
   [local count: 1073741824]:
  return;
}



[Bug ipa/96482] [10/11 Regression] Combination of -finline-small-functions and ipa-cp optimisations causes incorrect values being passed to a function since r279523

2020-08-10 Thread marxin at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96482

Martin Liška  changed:

   What|Removed |Added

 Status|WAITING |ASSIGNED

--- Comment #7 from Martin Liška  ---
Fine, I can reproduce that locally, working on that!
Thanks for the instructions.

Re: [PATCH] Rewrite get_size_range for irange API.

2020-08-10 Thread Andrew MacLeod via Gcc-patches

On 8/10/20 2:46 PM, Martin Sebor wrote:

On 8/10/20 11:50 AM, Andrew MacLeod wrote:

On 8/10/20 12:35 PM, Martin Sebor via Gcc-patches wrote:

On 8/10/20 5:47 AM, Aldy Hernandez wrote:



On 8/6/20 9:30 PM, Martin Sebor wrote:

On 8/6/20 8:53 AM, Aldy Hernandez via Gcc-patches wrote:



+  // Remove the unknown parts of a multi-range.
+  // This will transform [5,10][20,MAX] into [5,10].


Is this comment correct?  Wouldn't this result in returning smaller
sizes than the actual value allows?  If so, I'd expect that to cause
false positives (and in that case, if none of our tests fail we need
to add some that would).

By my reading of the code below it seems to return the upper range
(i.e., [20, MAX]) but I'm not fully acquainted with the new ranger
APIs yet.


I believe the comment is correct.  What I'm trying to do is remove 
[X,TYPE_MAX_VALUE] from the range



+  int pairs = positives.num_pairs ();
+  if (pairs > 1
+  && positives.upper_bound () == wi::to_wide (TYPE_MAX_VALUE 
(exptype)))

  {
-  if (integral)
-    {
-  /* Use the full range of the type of the expression when
- no value range information is available.  */
-  range[0] = TYPE_MIN_VALUE (exptype);
-  range[1] = TYPE_MAX_VALUE (exptype);
-  return true;
-    }
-
-  range[0] = NULL_TREE;
-  range[1] = NULL_TREE;
-  return false;
+  value_range last_range (exptype,
+  positives.lower_bound (pairs - 1),
+  positives.upper_bound (pairs - 1), 
VR_ANTI_RANGE);

+  positives.intersect (last_range);
  }


Notice that we start with positives == vr (where vr is the range 
returned by determine_value_range).


Then we build last_range which is the *opposite* of 
[X,TYPE_MAX_VALUE]. Notice the VR_ANTI_RANGE in the constructor.


(Note that the nomenclature VR_ANTI_RANGE is being used in the 
constructor to keep with the original API.  It means "create the 
inverse of this range".  Ideally, when the m_kind field disappears 
along with VR_ANTI_RANGEs, the enum could become RANGE_INVERSE or 
something less confusing.)


Finally, last_range is intersected with positives, which will 
become whatever VR had, minus the last sub-range.


Those that make sense, or did I miss something?

If you have a testcase that yields a false positive in my proposed 
patch, but not in the original code, please let me know so I can 
adjust the patch.


Sorry, I misremembered what get_size_range was used for.  It's used
to constrain the maximum size of memory accesses by functions like
memcpy (the third argument).  The use case I was thinking of was
to determine the bounds of the size of variably modified objects
(like VLAs).  It doesn't look like it's used for that.

With the current use taking the lower bound is the conservative
thing to do and using the upper bound would cause false positives.
With the latter it would be the other way around.  So the comment
makes sense to me now.

As an aside, here's a test case I played with.  I'd expect it to
trigger a warning because the upper bound of the range of array's
size is less than the lower bound of the range of the number of
bytes being written to it.

void f (void*);

void g (unsigned n1, unsigned n2)
{
  if (!(n1 >= 2 && n1 <= 3))   // n1 = [2, 3]
    return;

  char a[n1];

  if (!((n2 >= 5 && n2 <= 10)
    || (n2 >= 20)))    // n2 = [5, 10] U [20, UINT_MAX]
    return;

  __builtin_memset (a, 0, n2);
  f (a);
}

But I couldn't get it to either produce a multipart range, or
a union of the two parts (i.e., [5, UINT_MAX]).  It makes me
wonder what I'm missing about how such ranges are supposed to
come into existence.

Martin


There mare a couple of things..

My guess is you are using a value_range?   value_ranges are an 
int_range<1> under the covers, so will never have more than a single 
range, or anti range.


I see.  Yes, get_size_range calls determine_value_range which uses
a value_range.



int_range is the type which allows for up to X subranges. 
calculations will be merged to fit within X subranges
widest_irange is the type which allows for "unlimited" subranges... 
which currently happens to be capped at 255.. . (its typedef'd as 
int_range<255>).


widest_irange is the type used within the range-ops machinery and 
such, and then whatever result is calculated is "toned down" to 
whatever to user provides.


so if union results in [5,10] and [20, MAX]   and you provide a 
value_range for the result (, or int_range<1>), the result you get 
back will be [5, MAX].. so won't look like there are any multi-ranges 
going on.


This is one part of the puzzle (for me).  I don't get [5, MAX] but
[0, MAX], on trunk as well as in GCC 10:

  void f (unsigned n)
  {
    if (!((n >= 5 && n <= 10)
  || (n >= 20)))    // n2 = [5, 10] U [20, UINT_MAX]
  return;

    if (n == 3) // not folded
  __builtin_abort ();
  }

I'd expect this to get optimized regardless of Ranger (Clang folds
the whole 

Re: PING [Patch][Middle-end]Add -fzero-call-used-regs=[skip|used-gpr|all-gpr|used|all]

2020-08-10 Thread Qing Zhao via Gcc-patches


>> 
>>> If so, I am okay with name “call-clobbered” if this name sounds better. 
>> 
>> It's more obvious, at least to me.

In the current option list of GCC:  
https://gcc.gnu.org/onlinedocs/gcc/Code-Gen-Options.html#Code-Gen-Options 


There is one available option whose name is: -fcall-used-reg


-fcall-used-reg

Treat the register named reg as an allocable register that is clobbered by 
function calls. It may be allocated for temporaries or variables that do not 
live across a call. Functions compiled this way do not save and restore the 
register reg.

It is an error to use this flag with the frame pointer or stack pointer. Use of 
this flag for other registers that have fixed pervasive roles in the machine’s 
execution model produces disastrous results.

This flag does not have a negative form, because it specifies a three-way 
choice.

So, the name of this option adopted “call-used” instead of “call-clobbered”.  I 
think that for consistency, it might be still better to use 
“-fzero-call-used-regs” instead of “-fzero-call-clobbered-regs”?

Qing





> Okay. 
> 



[Bug middle-end/96561] New: missing warning for buffer overflow with negative offset

2020-08-10 Thread msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96561

Bug ID: 96561
   Summary: missing warning for buffer overflow with negative
offset
   Product: gcc
   Version: 11.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: middle-end
  Assignee: unassigned at gcc dot gnu.org
  Reporter: msebor at gcc dot gnu.org
  Target Milestone: ---

Only two of the four equivalent instances of buffer overflow in the test
program  below are detected.  All four should be.

Even the member into which the store writes isn't mentioned, the store in
'MEM[(int *)p_1(D) + 3B] = 0' in the last two instances can be diagnosed based
on the analysis that starting offset of the access, p_1(D) + 3B, points to one
member of the struct while the ending offset, p_1(D) + 3B + sizeof (int),
points to another.

$ cat t.c && gcc -O2 -S -Wall -Wextra -fdump-tree-vrp=/dev/stdout -xc++ t.c
void* operator new (__SIZE_TYPE__, void *p) { return p; }

struct S { int n; char a[0]; };

void f0 (struct S *p)
{
  new (>a[-1]) int ();// -Wplacement-new (good)
}

void f1 (struct S *p)
{
  p->a[-1] = 0;  // -Warray-bounds (also good)
}

void f2 (struct S *p)
{
  char *q = p->a;
  q[-1] = 0;   // no warning
}

void f3 (struct S *p)
{
  char *q = p->a;
  __builtin_memset (q - 1, 0, sizeof (int));   // no warning
}

t.c: In function ‘void f0(S*)’:
t.c:7:8: warning: placement new constructing an object of type ‘int’ and size
‘4’ in a region of type ‘char [0]’ and size ‘0’ [-Wplacement-new=]
7 |   new (>a[-1]) int ();// -Wplacement-new (good)
  |^

;; Function operator new (_ZnwmPv, funcdef_no=0, decl_uid=2337, cgraph_uid=1,
symbol_order=0)

;; 1 loops found
;;
;; Loop 0
;;  header 0, latch 1
;;  depth 0, outer -1
;;  nodes: 0 1 2
;; 2 succs { 1 }

Value ranges after VRP:



operator new (long unsigned int D.2335, void * p)
{
   [local count: 1073741824]:
  return p_1(D);

}



;; Function operator new (_ZnwmPv, funcdef_no=0, decl_uid=2337, cgraph_uid=1,
symbol_order=0)

;; 1 loops found
;;
;; Loop 0
;;  header 0, latch 1
;;  depth 0, outer -1
;;  nodes: 0 1 2
;; 2 succs { 1 }

Value ranges after VRP:



operator new (long unsigned int D.2335, void * p)
{
   [local count: 1073741824]:
  return p_1(D);

}



;; Function f0 (_Z2f0P1S, funcdef_no=1, decl_uid=2344, cgraph_uid=2,
symbol_order=1)

;; 1 loops found
;;
;; Loop 0
;;  header 0, latch 1
;;  depth 0, outer -1
;;  nodes: 0 1 2
;; 2 succs { 1 }

Value ranges after VRP:



f0 (struct S * p)
{
   [local count: 1073741824]:
  MEM[(int *)p_1(D) + 3B] = 0;
  return;

}



;; Function f0 (_Z2f0P1S, funcdef_no=1, decl_uid=2344, cgraph_uid=2,
symbol_order=1)

;; 1 loops found
;;
;; Loop 0
;;  header 0, latch 1
;;  depth 0, outer -1
;;  nodes: 0 1 2
;; 2 succs { 1 }

Value ranges after VRP:



f0 (struct S * p)
{
   [local count: 1073741824]:
  MEM[(int *)p_1(D) + 3B] = 0;
  return;

}



;; Function f1 (_Z2f1P1S, funcdef_no=2, decl_uid=2350, cgraph_uid=3,
symbol_order=2)

;; 1 loops found
;;
;; Loop 0
;;  header 0, latch 1
;;  depth 0, outer -1
;;  nodes: 0 1 2
;; 2 succs { 1 }

Value ranges after VRP:



t.c: In function ‘void f1(S*)’:
t.c:12:10: warning: array subscript -1 is below array bounds of ‘char [0]’
[-Warray-bounds]
   12 |   p->a[-1] = 0;  // -Warray-bounds (also good)
  |   ~~~^
t.c:3:24: note: while referencing ‘S::a’
3 | struct S { int n; char a[0]; };
  |^
f1 (struct S * p)
{
   [local count: 1073741824]:
  p_2(D)->a[-1] = 0;
  return;

}



;; Function f1 (_Z2f1P1S, funcdef_no=2, decl_uid=2350, cgraph_uid=3,
symbol_order=2)

;; 1 loops found
;;
;; Loop 0
;;  header 0, latch 1
;;  depth 0, outer -1
;;  nodes: 0 1 2
;; 2 succs { 1 }

Value ranges after VRP:



f1 (struct S * p)
{
   [local count: 1073741824]:
  p_2(D)->a[-1] = 0;
  return;

}



;; Function f2 (_Z2f2P1S, funcdef_no=3, decl_uid=2353, cgraph_uid=4,
symbol_order=3)

;; 1 loops found
;;
;; Loop 0
;;  header 0, latch 1
;;  depth 0, outer -1
;;  nodes: 0 1 2
;; 2 succs { 1 }

Value ranges after VRP:



f2 (struct S * p)
{
   [local count: 1073741824]:
  MEM[(char *)p_1(D) + 3B] = 0;
  return;

}



;; Function f2 (_Z2f2P1S, funcdef_no=3, decl_uid=2353, cgraph_uid=4,
symbol_order=3)

;; 1 loops found
;;
;; Loop 0
;;  header 0, latch 1
;;  depth 0, outer -1
;;  nodes: 0 1 2
;; 2 succs { 1 }

Value ranges after VRP:



f2 (struct S * p)
{
   [local count: 1073741824]:
  MEM[(char *)p_1(D) + 3B] = 0;
  return;

}



;; Function f3 (_Z2f3P1S, funcdef_no=4, decl_uid=2357, cgraph_uid=5,
symbol_order=4)

;; 1 loops found
;;
;; Loop 0
;;  header 0, latch 1
;;  depth 0, outer -1
;;  nodes: 0 1 2
;; 2 succs { 1 }

Value ranges after VRP:

_1: char * [1B, +INF]
p_2(D): struct S * VARYING


f3 (struct S * p)
{
  char * _1;

   [local count: 1073741824]:
  _1 =   [(void *)p_2(D) + 3B];
  __builtin_memset (_1, 0, 4);
  

Re: [PATCH] PR libstdc++/91620 Implement DR 526 for std::[forward_]list::remove_if/unique

2020-08-10 Thread François Dumont via Gcc-patches

Gentle reminder, this time with tests.

I've added one for list::remove cause I think there was none, for 
forward_list we had remove_freed.cc.


I added
// { dg-options "-g -O0" }

in the new tests otherwise it doesn't fail, that's life with UB. I know 
that it can pass also with those options.


If you prefer we can go without it and let Valgrind detect the issue.

Whatever, once the patch is in place it doesn't fail anymore.

François


On 27/12/19 11:57 am, François Dumont wrote:
Here is the patch to extend DR 526 to forward_list and list remove_if 
and unique.


As the adopted pattern is simpler I also applied it to the remove 
methods.


    PR libstdc++/91620
    * include/bits/forward_list.tcc (forward_list<>::remove): Collect 
nodes

    to destroy in an intermediate forward_list.
    (forward_list<>::remove_if, forward_list<>::unique): Likewise.
    * include/bits/list.tcc (list<>::remove, list<>::unique): Likewise.
    (list<>::remove_if): Likewise.
    * include/debug/forward_list (forward_list<>::_M_erase_after): 
Remove.

    (forward_list<>::erase_after): Adapt.
    (forward_list<>::remove, forward_list<>::remove_if): Collect nodes to
    destroy in an intermediate forward_list.
    (forward_list<>::unique): Likewise.
    * include/debug/list (list<>::remove, list<>::unique): Likewise.
    (list<>::remove_if): Likewise.

Tested under Linux x86_64 normal and debug modes.

Ok to commit ?

François



diff --git a/libstdc++-v3/include/bits/forward_list.tcc b/libstdc++-v3/include/bits/forward_list.tcc
index c42bdc0fd13..3f94066bd55 100644
--- a/libstdc++-v3/include/bits/forward_list.tcc
+++ b/libstdc++-v3/include/bits/forward_list.tcc
@@ -290,30 +290,19 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 remove(const _Tp& __val) -> __remove_return_type
 {
   size_type __removed __attribute__((__unused__)) = 0;
-  _Node_base* __curr = >_M_impl._M_head;
-  _Node_base* __extra = nullptr;
+  forward_list __to_destroy(get_allocator());
 
-  while (_Node* __tmp = static_cast<_Node*>(__curr->_M_next))
-	{
-	  if (*__tmp->_M_valptr() == __val)
-	{
-	  if (__tmp->_M_valptr() != std::__addressof(__val))
-		{
-		  this->_M_erase_after(__curr);
-		  _GLIBCXX20_ONLY( __removed++ );
-		  continue;
-		}
-	  else
-		__extra = __curr;
-	}
-	  __curr = __curr->_M_next;
-	}
+  auto __prev_it = cbefore_begin();
+  while (_Node* __tmp = static_cast<_Node*>(__prev_it._M_node->_M_next))
+	if (*__tmp->_M_valptr() == __val)
+	  {
+	__to_destroy.splice_after(__to_destroy.cbefore_begin(),
+  *this, __prev_it);
+	_GLIBCXX20_ONLY( __removed++ );
+	  }
+	else
+	  ++__prev_it;
 
-  if (__extra)
-	{
-	  this->_M_erase_after(__extra);
-	  _GLIBCXX20_ONLY( __removed++ );
-	}
   return _GLIBCXX20_ONLY( __removed );
 }
 
@@ -324,17 +313,19 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
   remove_if(_Pred __pred) -> __remove_return_type
   {
 	size_type __removed __attribute__((__unused__)) = 0;
-	_Node_base* __curr = >_M_impl._M_head;
-	while (_Node* __tmp = static_cast<_Node*>(__curr->_M_next))
-	  {
-	if (__pred(*__tmp->_M_valptr()))
-	  {
-		this->_M_erase_after(__curr);
-		_GLIBCXX20_ONLY( __removed++ );
-	  }
-	else
-	  __curr = __curr->_M_next;
-	  }
+	forward_list __to_destroy(get_allocator());
+
+	auto __prev_it = cbefore_begin();
+	while (_Node* __tmp = static_cast<_Node*>(__prev_it._M_node->_M_next))
+	  if (__pred(*__tmp->_M_valptr()))
+	{
+	  __to_destroy.splice_after(__to_destroy.cbefore_begin(),
+	*this, __prev_it);
+	  _GLIBCXX20_ONLY( __removed++ );
+	}
+	  else
+	++__prev_it;
+
 	return _GLIBCXX20_ONLY( __removed );
   }
 
@@ -348,20 +339,24 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	iterator __last = end();
 	if (__first == __last)
 	  return _GLIBCXX20_ONLY(0);
+
+	forward_list __to_destroy(get_allocator());
 	size_type __removed __attribute__((__unused__)) = 0;
 	iterator __next = __first;
 	while (++__next != __last)
 	{
 	  if (__binary_pred(*__first, *__next))
 	{
-	  erase_after(__first);
+	  __to_destroy.splice_after(__to_destroy.cbefore_begin(),
+	*this, __first);
 	  _GLIBCXX20_ONLY( __removed++ );
 	}
 	  else
 	__first = __next;
 	  __next = __first;
 	}
-return _GLIBCXX20_ONLY( __removed );
+
+	return _GLIBCXX20_ONLY( __removed );
   }
 
 #undef _GLIBCXX20_ONLY
diff --git a/libstdc++-v3/include/bits/list.tcc b/libstdc++-v3/include/bits/list.tcc
index ce9e983c539..9b664f11454 100644
--- a/libstdc++-v3/include/bits/list.tcc
+++ b/libstdc++-v3/include/bits/list.tcc
@@ -331,10 +331,12 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 list<_Tp, _Alloc>::
 remove(const value_type& __value)
 {
+#if !_GLIBCXX_USE_CXX11_ABI
   size_type __removed __attribute__((__unused__)) = 0;
+#endif
+  list __to_destroy(get_allocator());
   iterator __first = begin();
   iterator __last = end();
-  iterator __extra = __last;
   while (__first != 

Re: [PATCH] emit-rtl.c: Allow vector subreg of float vectors

2020-08-10 Thread Richard Sandiford
"Stubbs, Andrew"  writes:
> Is GET_INNER_MODE valid for scalers though?

Yeah, GET_MODE_INNER (x) == x for scalars.  That makes GET_MODE_INNER
useful for stripping vectorness or complexness without having to check
for them first.  It's also useful when working out what the valid shift
ranges are for immediate shifts, and other things like that.

Thanks,
Richard


[Bug tree-optimization/96554] -Wall does not include -Wnull-dereference

2020-08-10 Thread rsandifo at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96554

rsandifo at gcc dot gnu.org  changed:

   What|Removed |Added

 CC||rsandifo at gcc dot gnu.org

--- Comment #6 from rsandifo at gcc dot gnu.org  
---
(In reply to Martin Sebor from comment #5)
> The latest trunk of GCC 11 has over 700 distinct instances of the
> -Wnull-dereference warning so some work is still needed before it can be
> enabled in either -Wextra or -Wall.  Here's the breakdown:

It'd be interesting to know how many would still be caught by a
“must” version of the option, if it was split into a may/must pair
(as per earlier suggestions in Wnull-dereference).  Maybe we could
emulate that by sticking in a postdominannce test and rerunning
the numbers…

Re: [PATCH] c++: dependent constraint on placeholder return type [PR96443]

2020-08-10 Thread Patrick Palka via Gcc-patches
On Thu, 6 Aug 2020, Patrick Palka wrote:

> On Wed, 5 Aug 2020, Jason Merrill wrote:
> 
> > On 8/4/20 8:00 PM, Patrick Palka wrote:
> > > On Tue, 4 Aug 2020, Patrick Palka wrote:
> > > 
> > > > In the testcase below, we never substitute function-template arguments
> > > > into f15's placeholder-return-type constraint, which leads to us
> > > > incorrectly rejecting this instantiation in do_auto_deduction due to
> > > > satisfaction failure (of the constraint SameAs).
> > > > 
> > > > The fact that we incorrectly reject this testcase is masked by the
> > > > other instantiation f15, which we correctly reject and diagnose
> > > > (by accident).
> > > > 
> > > > A good place to do this missing substitution seems to be during
> > > > TEMPLATE_TYPE_PARM level lowering.  So this patch adds a call to
> > > > tsubst_constraint there, and also adds dg-bogus directives to this
> > > > testcase wherever we expect instantiation to succeed. (So without the
> > > > substitution fix, this last dg-bogus would FAIL).
> > 
> > > > Successfully tested on x86_64-pc-linux-gnu, and also on the cmcstl2 and
> > > > range-v3 projects.  Does this look OK to commit?
> > > > 
> > > > gcc/cp/ChangeLog:
> > > > 
> > > > PR c++/96443
> > > > * pt.c (tsubst) : Substitute into
> > > > the constraints on a placeholder type when its level.
> > > > 
> > > > gcc/testsuite/ChangeLog:
> > > > 
> > > > PR c++/96443
> > > > * g++.dg/cpp2a/concepts-ts1.C: Add dg-bogus wherever we expect
> > > > instantiation to succeed.
> > > 
> > > Looking back at this patch with fresh eyes, I realized that the commit
> > > message is not the best.  I rewrote the commit message to hopefully be
> > > more coherent below:
> > > 
> > > -- >8 --
> > > 
> > > Subject: [PATCH] c++: dependent constraint on placeholder return type
> > >   [PR96443]
> > > 
> > > In the testcase concepts-ts1.C, we're incorrectly rejecting the call to
> > > 'f15(0)' due to satisfaction failure of the function's
> > > placeholder-return-type constraint.
> > > 
> > > The testcase doesn't spot this rejection because the error we emit for
> > > the constraint failure points to f15's return statement instead of the
> > > call site, and we already have a dg-error at the return statement to
> > > verify the (correct) rejection of the call f15('a').  So in order to
> > > verify that we indeed accept the call 'f15(0)', we need to add a
> > > dg-bogus directive at the call site to look for the "required from here"
> > > diagnostic line that generally accompanies an instantiation failure.
> > > 
> > > As for why satisfaction failure occurs, it turns out that we never
> > > substitute the template arguments of a function template specialization
> > > in to its placeholder-return-type constraint.  So in this case during
> > > do_auto_deduction, we end up checking satisfaction of the still-dependent
> > > constraint SameAs from do_auto_deduction, which fails
> > > because it's dependent.
> > > 
> > > A good place to do this missing substitution seems to be during
> > > TEMPLATE_TYPE_PARM level lowering; so this patch adds a call to
> > > tsubst_constraint there.
> > 
> > Doing substitution seems like the wrong approach here; requirements should
> > never be substituted except as part of satisfaction calculation (or, rarely,
> > for declaration matching).  If we aren't communicating all the necessary
> > template arguments to the later satisfaction, that's what we need to fix.
> 
> Ah okay, that makes sense.
> 
> It also looks like the question of perform a single full substitution
> (during auto deduction) vs two substitutions may also be a correctness
> issue in light of SFINAE.  Consider the following testcase:
> 
> template
> concept C = true;
> 
> auto f(auto x) -> C auto { return 0; }
> auto f(auto x, ...) { return 1; }
> 
> int a = f(0);
> 
> If we do a single substitution, then the substitution failure is a hard
> error and we'll reject this testcase.  If we do two substitutions, then
> it's a SFINAE error and we select the second overload.  Would we be
> correct to issue a hard error here?

Please ignore the previous message :)  I had missed that the
substitution failure should result in the constraint evaluating to false
as part of satisfaction.  Sorry for the noise.

> 
> > 
> > > Successfully tested on x86_64-pc-linux-gnu, and also on the cmcstl2 and
> > > range-v3 projects.  Does this look OK to commit?
> > > 
> > > gcc/cp/ChangeLog:
> > > 
> > >   PR c++/96443
> > >   * pt.c (tsubst) : Substitute into
> > >   the constraints on a placeholder type when reducing its level.
> > > 
> > > gcc/testsuite/ChangeLog:
> > > 
> > >   PR c++/96443
> > >   * g++.dg/cpp2a/concepts-ts1.C: Add dg-bogus to the call to f15
> > >   that we expect to accept.
> > > ---
> > >   gcc/cp/pt.c   | 7 ---
> > >   gcc/testsuite/g++.dg/cpp2a/concepts-ts1.C | 2 +-
> > >   2 files changed, 5 insertions(+), 4 deletions(-)
> 

Re: [PATCH] Rewrite get_size_range for irange API.

2020-08-10 Thread Martin Sebor via Gcc-patches

On 8/10/20 11:50 AM, Andrew MacLeod wrote:

On 8/10/20 12:35 PM, Martin Sebor via Gcc-patches wrote:

On 8/10/20 5:47 AM, Aldy Hernandez wrote:



On 8/6/20 9:30 PM, Martin Sebor wrote:

On 8/6/20 8:53 AM, Aldy Hernandez via Gcc-patches wrote:



+  // Remove the unknown parts of a multi-range.
+  // This will transform [5,10][20,MAX] into [5,10].


Is this comment correct?  Wouldn't this result in returning smaller
sizes than the actual value allows?  If so, I'd expect that to cause
false positives (and in that case, if none of our tests fail we need
to add some that would).

By my reading of the code below it seems to return the upper range
(i.e., [20, MAX]) but I'm not fully acquainted with the new ranger
APIs yet.


I believe the comment is correct.  What I'm trying to do is remove 
[X,TYPE_MAX_VALUE] from the range



+  int pairs = positives.num_pairs ();
+  if (pairs > 1
+  && positives.upper_bound () == wi::to_wide (TYPE_MAX_VALUE 
(exptype)))

  {
-  if (integral)
-    {
-  /* Use the full range of the type of the expression when
- no value range information is available.  */
-  range[0] = TYPE_MIN_VALUE (exptype);
-  range[1] = TYPE_MAX_VALUE (exptype);
-  return true;
-    }
-
-  range[0] = NULL_TREE;
-  range[1] = NULL_TREE;
-  return false;
+  value_range last_range (exptype,
+  positives.lower_bound (pairs - 1),
+  positives.upper_bound (pairs - 1), VR_ANTI_RANGE);
+  positives.intersect (last_range);
  }


Notice that we start with positives == vr (where vr is the range 
returned by determine_value_range).


Then we build last_range which is the *opposite* of 
[X,TYPE_MAX_VALUE]. Notice the VR_ANTI_RANGE in the constructor.


(Note that the nomenclature VR_ANTI_RANGE is being used in the 
constructor to keep with the original API.  It means "create the 
inverse of this range".  Ideally, when the m_kind field disappears 
along with VR_ANTI_RANGEs, the enum could become RANGE_INVERSE or 
something less confusing.)


Finally, last_range is intersected with positives, which will become 
whatever VR had, minus the last sub-range.


Those that make sense, or did I miss something?

If you have a testcase that yields a false positive in my proposed 
patch, but not in the original code, please let me know so I can 
adjust the patch.


Sorry, I misremembered what get_size_range was used for.  It's used
to constrain the maximum size of memory accesses by functions like
memcpy (the third argument).  The use case I was thinking of was
to determine the bounds of the size of variably modified objects
(like VLAs).  It doesn't look like it's used for that.

With the current use taking the lower bound is the conservative
thing to do and using the upper bound would cause false positives.
With the latter it would be the other way around.  So the comment
makes sense to me now.

As an aside, here's a test case I played with.  I'd expect it to
trigger a warning because the upper bound of the range of array's
size is less than the lower bound of the range of the number of
bytes being written to it.

void f (void*);

void g (unsigned n1, unsigned n2)
{
  if (!(n1 >= 2 && n1 <= 3))   // n1 = [2, 3]
    return;

  char a[n1];

  if (!((n2 >= 5 && n2 <= 10)
    || (n2 >= 20)))    // n2 = [5, 10] U [20, UINT_MAX]
    return;

  __builtin_memset (a, 0, n2);
  f (a);
}

But I couldn't get it to either produce a multipart range, or
a union of the two parts (i.e., [5, UINT_MAX]).  It makes me
wonder what I'm missing about how such ranges are supposed to
come into existence.

Martin


There mare a couple of things..

My guess is you are using a value_range?   value_ranges are an 
int_range<1> under the covers, so will never have more than a single 
range, or anti range.


I see.  Yes, get_size_range calls determine_value_range which uses
a value_range.



int_range is the type which allows for up to X subranges. 
calculations will be merged to fit within X subranges
widest_irange is the type which allows for "unlimited" subranges... 
which currently happens to be capped at 255.. .  (its typedef'd as 
int_range<255>).


widest_irange is the type used within the range-ops machinery and such, 
and then whatever result is calculated is "toned down" to whatever to 
user provides.


so if union results in [5,10] and [20, MAX]   and you provide a 
value_range for the result (, or int_range<1>), the result you get back 
will be [5, MAX].. so won't look like there are any multi-ranges going on.


This is one part of the puzzle (for me).  I don't get [5, MAX] but
[0, MAX], on trunk as well as in GCC 10:

  void f (unsigned n)
  {
if (!((n >= 5 && n <= 10)
  || (n >= 20)))// n2 = [5, 10] U [20, UINT_MAX]
  return;

if (n == 3) // not folded
  __builtin_abort ();
  }

I'd expect this to get optimized regardless of Ranger (Clang folds
the whole function body into a return statement).

and 

Re: [PATCH] rs6000: ICE when using an MMA type as a function param

2020-08-10 Thread Peter Bergner via Gcc-patches
On 8/9/20 10:03 PM, Peter Bergner wrote:
> PR96506 shows a problem where we ICE on illegal usage, namely using MMA
> types for function arguments and return values.  The solution is to flag
> these illegal usages as errors early, before we ICE.
> 
> The patch below is currently bootstrapping and regtesting.  Ok for trunk
> once that comes back clean?  Ok for GCC 10 after some bake in?
> 
> Peter
> 
> 
> gcc/
>   PR target/96506
>   * config/rs6000/rs6000-call.c (rs6000_promote_function_mode): Disallow
>   MMA types as return values.
>   (rs6000_function_arg): Disallow MMA types as function arguments.
> 
> gcc/testsuite/
>   PR target/96506
>   * gcc.target/powerpc/pr96506.c: New test.

FYI, testing came back clean with no regressions.

Peter




Re: [PATCH] emit-rtl.c: Allow vector subreg of float vectors

2020-08-10 Thread Stubbs, Andrew


On 10 Aug 2020 17:23, Richard Sandiford  wrote:
Andrew Stubbs  writes:
> On 06/08/2020 04:54, Richard Sandiford wrote:
>>> diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
>>> index f9b0e9714d9..d7067989ad7 100644
>>> --- a/gcc/emit-rtl.c
>>> +++ b/gcc/emit-rtl.c
>>> @@ -947,6 +947,11 @@ validate_subreg (machine_mode omode, machine_mode 
>>> imode,
>>> else if (VECTOR_MODE_P (omode)
>>> && GET_MODE_INNER (omode) == GET_MODE_INNER (imode))
>>>   ;
>>> +  /* Allow sections of vectors, both smaller and paradoxical.  (This just
>>> + works for integers, but needs to be explicitly allowed for floats.)  
>>> */
>>> +  else if (VECTOR_MODE_P (omode) && VECTOR_MODE_P (imode)
>>> +  && GET_MODE_INNER (omode) == GET_MODE_INNER (imode))
>>> +;
>>
>> Might be missing something, but isn't this a subcondition of the
>> previous “else if”?  It looks like that ought to catch every case
>> that the new one does.
>
> Apparently, Hongtao and Uroš fixed my problem while I was working on this.
>
> Yes, my patch does the same (although I would question whether it's safe
> to use "GET_MODE_INNER (imode)" without having first confirmed "imode"
> is a vector mode).

FWIW, skipping the VECTOR_MODE_P test means that we also allow vector
paradoxical subregs of scalars, which can be useful if you're trying
to make a vector in which only element 0 is initialised.  (Only works
for element 0 though.)  IIRC this is what some of the x86 patterns do.

Guess it means we also allow vector subregs of complex values,
but that kind-of seems OK/useful too.

Is GET_INNER_MODE valid for scalers though?

Andrew


Re: [RFC] libstdc++: Fix pretty-printing old implementations of std::unique_ptr

2020-08-10 Thread Andres Rodriguez via Gcc-patches
On Mon, Aug 10, 2020 at 1:49 PM Jonathan Wakely  wrote:
>
> On 10/08/20 09:45 -0400, Andres Rodriguez wrote:
> >*ping*
>
> As it says at https://gcc.gnu.org/lists.html all patches for libstdc++
> need to be sent to the libstdc++ mailing as well as the gcc-patches
> list. Otherwise I won't see them, and everybody else will ignore them.
>

Thanks for the heads up on the libstdc++ mailing list and for the
unique_ptr fix.

-Andres


> >On Tue, Aug 4, 2020 at 10:51 AM Andres Rodriguez  wrote:
> >>
> >> On binaries compiled against gcc5 the impl_type parameter is None,
> >> which results in an exception being raised by is_specialization_of()
> >>
> >> These versions of std::unique_ptr have the tuple as a root element.
>
> The unique_ptr implementation in GCC 5 should already be handled by
> the second branch which handles a tuple member:
>
>  if is_specialization_of(impl_type, '__uniq_ptr_data') \
>  or is_specialization_of(impl_type, '__uniq_ptr_impl'):
>  tuple_member = val['_M_t']['_M_t']
>  elif is_specialization_of(impl_type, 'tuple'):
>  tuple_member = val['_M_t']
>  else:
>  raise ValueError("Unsupported implementation for unique_ptr: %s" 
> % impl_type)
>
> We have a test that's supposed to verify that printing the old
> unique_ptr implementation still works:
> https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libstdc%2B%2B-v3/testsuite/libstdc%2B%2B-prettyprinters/compat.cc;h=f1c3b599634af2eb0a5a0889c528d9de2d21aacd;hb=HEAD
>
> So the right fix is to work out why that branch isn't taken, and fix
> it, not add a kluge.
>
> If impl_type is None that means the .tag field is missing, which seems
> to be because the _M_t member is declared using the __tuple_type
> typedef, rather than std::tuple itself.
>
> That's fixed by the attached patch.
>
> Tested x86_64-linux, pushed to master. I'll backport it to the
> branches too.
>
>


Re: [PATCH] c++: premature analysis of requires-expression [PR96410]

2020-08-10 Thread Patrick Palka via Gcc-patches
On Mon, 10 Aug 2020, Patrick Palka wrote:

> In the below testcase, semantic analysis of the requires-expressions in
> the generic lambda must be delayed until instantiation of the lambda
> because the requirements depend on the lambda's template arguments.  But
> tsubst_requires_expr does semantic analysis even during regeneration of
> the lambda, which leads to various bogus errors and ICEs since some
> subroutines aren't prepared to handle dependent/template trees.
> 
> This patch adjusts subroutines of tsubst_requires_expr to avoid doing
> some problematic semantic analyses when processing_template_decl.
> In particular, expr_noexcept_p generally can't be checked on a dependent
> expression.  Next, tsubst_nested_requirement should avoid checking
> satisfaction when processing_template_decl.  And similarly for
> convert_to_void (called from tsubst_valid_expression_requirement).
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, and also tested
> against the cmcstl2 project.  Does this look OK to commit?
> 
> gcc/cp/ChangeLog:
> 
>   PR c++/96409
>   PR c++/96410
>   * constraint.cc (tsubst_compound_requirement): When
>   processing_template_decl, don't check noexcept of the
>   substituted expression.
>   (tsubst_nested_requirement): Just substitute into the constraint
>   when processing_template_decl.
>   * cvt.c (convert_to_void): Don't resolve concept checks when
>   processing_template_decl.
> 
> gcc/testsuite/ChangeLog:
> 
>   PR c++/96409
>   PR c++/96410
>   * g++.dg/cpp2a/concepts-lambda13.C: New test.
> ---
>  gcc/cp/constraint.cc  |  9 ++-
>  gcc/cp/cvt.c  |  2 +-
>  .../g++.dg/cpp2a/concepts-lambda13.C  | 25 +++
>  3 files changed, 34 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C
> 
> diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
> index e4aace596e7..db2036502a7 100644
> --- a/gcc/cp/constraint.cc
> +++ b/gcc/cp/constraint.cc
> @@ -1993,7 +1993,8 @@ tsubst_compound_requirement (tree t, tree args, 
> subst_info info)
>  
>/* Check the noexcept condition.  */
>bool noexcept_p = COMPOUND_REQ_NOEXCEPT_P (t);
> -  if (noexcept_p && !expr_noexcept_p (expr, tf_none))
> +  if (!processing_template_decl
> +  && noexcept_p && !expr_noexcept_p (expr, tf_none))
>  return error_mark_node;
>  
>/* Substitute through the type expression, if any.  */
> @@ -2023,6 +2024,12 @@ static tree
>  tsubst_nested_requirement (tree t, tree args, subst_info info)
>  {
>/* Ensure that we're in an evaluation context prior to satisfaction.  */
> +  if (processing_template_decl)
> +{
> +  tree r = tsubst_constraint (TREE_OPERAND (t, 0), args,
> +   info.complain, info.in_decl);

Oops, the patch is missing a check for error_mark_node here, so that
upon substitution failure we immediately resolve the requires-expression
to false.  Here's an updated patch with the check and a regression test
added:

-- >8 --

gcc/cp/ChangeLog:

PR c++/96409
PR c++/96410
* constraint.cc (tsubst_compound_requirement): When
processing_template_decl, don't check noexcept of the
substituted expression.
(tsubst_nested_requirement): Just substitute into the constraint
when processing_template_decl.
* cvt.c (convert_to_void): Don't resolve concept checks when
processing_template_decl.

gcc/testsuite/ChangeLog:

PR c++/96409
PR c++/96410
* g++.dg/cpp2a/concepts-lambda13.C: New test.
* g++.dg/cpp2a/concepts-lambda14.C: New test.
---
 gcc/cp/constraint.cc  | 11 +++-
 gcc/cp/cvt.c  |  2 +-
 .../g++.dg/cpp2a/concepts-lambda13.C  | 25 +++
 .../g++.dg/cpp2a/concepts-lambda14.C  | 10 
 4 files changed, 46 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index e4aace596e7..5b4964dd36e 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -1993,7 +1993,8 @@ tsubst_compound_requirement (tree t, tree args, 
subst_info info)
 
   /* Check the noexcept condition.  */
   bool noexcept_p = COMPOUND_REQ_NOEXCEPT_P (t);
-  if (noexcept_p && !expr_noexcept_p (expr, tf_none))
+  if (!processing_template_decl
+  && noexcept_p && !expr_noexcept_p (expr, tf_none))
 return error_mark_node;
 
   /* Substitute through the type expression, if any.  */
@@ -2023,6 +2024,14 @@ static tree
 tsubst_nested_requirement (tree t, tree args, subst_info info)
 {
   /* Ensure that we're in an evaluation context prior to satisfaction.  */
+  if (processing_template_decl)
+{
+  tree r = tsubst_constraint (TREE_OPERAND (t, 0), args,
+  

Re: [committed] libstdc++: Make C++17 ignore --disable-libstdcxx-filesystem-ts [PR 94681]

2020-08-10 Thread Jonathan Wakely via Gcc-patches

On 10/08/20 13:24 +0100, Jonathan Wakely wrote:

The configure switch should only affect the optional Filesystem TS, not
the std::filesystem features of C++17.

libstdc++-v3/ChangeLog:

PR libstdc++/94681
* acinclude.m4 (GLIBCXX_CHECK_FILESYSTEM_DEPS): Do not depend on
$enable_libstdcxx_filesystem_ts.
* configure: Regenerate.


This fixes a build failure reported in the PR.

Tested x86_64-linux. Committed to trunk.



commit 5b065f0563262a0d6cd1fea8426913bfdd841301
Author: Jonathan Wakely 
Date:   Mon Aug 10 18:58:14 2020

libstdc++: Fix build for targets without lstat [PR 94681]

libstdc++-v3/ChangeLog:

PR libstdc++/94681
* src/c++17/fs_ops.cc (read_symlink): Use posix::lstat instead
of calling ::lstat directly.
* src/filesystem/ops.cc (read_symlink): Likewise.

diff --git a/libstdc++-v3/src/c++17/fs_ops.cc b/libstdc++-v3/src/c++17/fs_ops.cc
index c685b1824f9..2cb26e4605b 100644
--- a/libstdc++-v3/src/c++17/fs_ops.cc
+++ b/libstdc++-v3/src/c++17/fs_ops.cc
@@ -1175,7 +1175,7 @@ fs::path fs::read_symlink(const path& p, error_code& ec)
   path result;
 #if defined(_GLIBCXX_HAVE_READLINK) && defined(_GLIBCXX_HAVE_SYS_STAT_H)
   stat_type st;
-  if (::lstat(p.c_str(), ))
+  if (posix::lstat(p.c_str(), ))
 {
   ec.assign(errno, std::generic_category());
   return result;
diff --git a/libstdc++-v3/src/filesystem/ops.cc b/libstdc++-v3/src/filesystem/ops.cc
index 8c8854bf28e..a1138490b3e 100644
--- a/libstdc++-v3/src/filesystem/ops.cc
+++ b/libstdc++-v3/src/filesystem/ops.cc
@@ -993,7 +993,7 @@ fs::path fs::read_symlink(const path& p [[gnu::unused]], error_code& ec)
   path result;
 #if defined(_GLIBCXX_HAVE_READLINK) && defined(_GLIBCXX_HAVE_SYS_STAT_H)
   stat_type st;
-  if (::lstat(p.c_str(), ))
+  if (posix::lstat(p.c_str(), ))
 {
   ec.assign(errno, std::generic_category());
   return result;


[Bug libstdc++/94681] filesystem::sysmlink_status using stat instead of lstat when --disable-libstdcxx-filesystem-ts

2020-08-10 Thread cvs-commit at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94681

--- Comment #7 from CVS Commits  ---
The master branch has been updated by Jonathan Wakely :

https://gcc.gnu.org/g:5b065f0563262a0d6cd1fea8426913bfdd841301

commit r11-2638-g5b065f0563262a0d6cd1fea8426913bfdd841301
Author: Jonathan Wakely 
Date:   Mon Aug 10 18:58:14 2020 +0100

libstdc++: Fix build for targets without lstat [PR 94681]

libstdc++-v3/ChangeLog:

PR libstdc++/94681
* src/c++17/fs_ops.cc (read_symlink): Use posix::lstat instead
of calling ::lstat directly.
* src/filesystem/ops.cc (read_symlink): Likewise.

Re: [RFC PATCH v1 1/1] PPC64: Implement POWER Architecture Vector Function ABI.

2020-08-10 Thread Jakub Jelinek via Gcc-patches
On Mon, Aug 10, 2020 at 05:29:49PM +, GT wrote:
> > For PowerPC, if all you want to support is b which requires VSX, then the
> > right thing is for !TREE_PUBLIC functions return 0 if !TARGET_VSX and
> > otherwise set vecsize_mangle to 'b' and in the end return 1, for exported
> > functions always set it to 'b' (and in the end return 1).
> > Then ensure that the 'b' variants of function definitions get target ("vsx")
> > attribute added if !TARGET_VSX.
> >
> 
> So setting attribute "vsx" for 'b' variants of function definitions is what
> should go in function rs6000_simd_clone_usable?

No.  That function should say if the particular clone ('b' in this case) is
usable from some caller, and the answer for your 'b' is TARGET_VSX is
required to be non-zero.

The adjustment should go into the simd_clone_adjust target hook, see
what ix86_simd_clone_adjust does (though, that one has more variants to
handle).

Jakub



[Bug testsuite/96525] new test gcc.target/powerpc/pr96493.c fails

2020-08-10 Thread bergner at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96525

--- Comment #7 from Peter Bergner  ---
(In reply to seurer from comment #6)
> That changed the test to unsupported on the p8 where it had failed
> previously.
[snip]
> # of unsupported tests1

That is what I would expect on a system with an old binutils.  Do you have a
testsuite run that has a newer power10 enabled binutils?  Then hopefully, you
should see a PASS of the dg-do link command.

Re: std:vec for classes with constructor?

2020-08-10 Thread Jakub Jelinek via Gcc-patches
On Mon, Aug 10, 2020 at 03:05:26PM +0200, Aldy Hernandez wrote:
> For the record.  The final patch (below) passes tests on ppc64le for both
> trunk, and the ranger-staging branch with ranger enabled.
> 
> Aldy
> 
> gcc/ChangeLog:
> 
>   * ipa-fnsummary.c (evaluate_conditions_for_known_args):
>   (evaluate_properties_for_edge):
>   (ipa_fn_summary_t::duplicate):
>   (estimate_ipcp_clone_size_and_time):
>   * vec.h (vl_embed>::embedded_size):

You didn't say what you've changed, supposedly for the first one
Revert 2020-08-?? change., then 3 times Likewise. and finally
something describing the actual change?

Anyway, this is ok for trunk, let's do the proper ctor/dtor/move handling
incrementally.

Jakub



Re: [PATCH] Rewrite get_size_range for irange API.

2020-08-10 Thread Aldy Hernandez via Gcc-patches
Yes, the goal is that anything that may take multi ranges be rewritten to
use an irange * and use the API exclusively.  Then when multi ranges are
passed down eventually, things will work transparently.

Aldy

On Mon, Aug 10, 2020, 19:50 Andrew MacLeod  wrote:

> On 8/10/20 12:35 PM, Martin Sebor via Gcc-patches wrote:
> > On 8/10/20 5:47 AM, Aldy Hernandez wrote:
> >>
> >>
> >> On 8/6/20 9:30 PM, Martin Sebor wrote:
> >>> On 8/6/20 8:53 AM, Aldy Hernandez via Gcc-patches wrote:
> >>
>  +  // Remove the unknown parts of a multi-range.
>  +  // This will transform [5,10][20,MAX] into [5,10].
> >>>
> >>> Is this comment correct?  Wouldn't this result in returning smaller
> >>> sizes than the actual value allows?  If so, I'd expect that to cause
> >>> false positives (and in that case, if none of our tests fail we need
> >>> to add some that would).
> >>>
> >>> By my reading of the code below it seems to return the upper range
> >>> (i.e., [20, MAX]) but I'm not fully acquainted with the new ranger
> >>> APIs yet.
> >>
> >> I believe the comment is correct.  What I'm trying to do is remove
> >> [X,TYPE_MAX_VALUE] from the range
> >>
>  +  int pairs = positives.num_pairs ();
>  +  if (pairs > 1
>  +  && positives.upper_bound () == wi::to_wide (TYPE_MAX_VALUE
>  (exptype)))
>    {
>  -  if (integral)
>  -{
>  -  /* Use the full range of the type of the expression when
>  - no value range information is available.  */
>  -  range[0] = TYPE_MIN_VALUE (exptype);
>  -  range[1] = TYPE_MAX_VALUE (exptype);
>  -  return true;
>  -}
>  -
>  -  range[0] = NULL_TREE;
>  -  range[1] = NULL_TREE;
>  -  return false;
>  +  value_range last_range (exptype,
>  +  positives.lower_bound (pairs - 1),
>  +  positives.upper_bound (pairs - 1), VR_ANTI_RANGE);
>  +  positives.intersect (last_range);
>    }
> >>
> >> Notice that we start with positives == vr (where vr is the range
> >> returned by determine_value_range).
> >>
> >> Then we build last_range which is the *opposite* of
> >> [X,TYPE_MAX_VALUE]. Notice the VR_ANTI_RANGE in the constructor.
> >>
> >> (Note that the nomenclature VR_ANTI_RANGE is being used in the
> >> constructor to keep with the original API.  It means "create the
> >> inverse of this range".  Ideally, when the m_kind field disappears
> >> along with VR_ANTI_RANGEs, the enum could become RANGE_INVERSE or
> >> something less confusing.)
> >>
> >> Finally, last_range is intersected with positives, which will become
> >> whatever VR had, minus the last sub-range.
> >>
> >> Those that make sense, or did I miss something?
> >>
> >> If you have a testcase that yields a false positive in my proposed
> >> patch, but not in the original code, please let me know so I can
> >> adjust the patch.
> >
> > Sorry, I misremembered what get_size_range was used for.  It's used
> > to constrain the maximum size of memory accesses by functions like
> > memcpy (the third argument).  The use case I was thinking of was
> > to determine the bounds of the size of variably modified objects
> > (like VLAs).  It doesn't look like it's used for that.
> >
> > With the current use taking the lower bound is the conservative
> > thing to do and using the upper bound would cause false positives.
> > With the latter it would be the other way around.  So the comment
> > makes sense to me now.
> >
> > As an aside, here's a test case I played with.  I'd expect it to
> > trigger a warning because the upper bound of the range of array's
> > size is less than the lower bound of the range of the number of
> > bytes being written to it.
> >
> > void f (void*);
> >
> > void g (unsigned n1, unsigned n2)
> > {
> >   if (!(n1 >= 2 && n1 <= 3))   // n1 = [2, 3]
> > return;
> >
> >   char a[n1];
> >
> >   if (!((n2 >= 5 && n2 <= 10)
> > || (n2 >= 20)))// n2 = [5, 10] U [20, UINT_MAX]
> > return;
> >
> >   __builtin_memset (a, 0, n2);
> >   f (a);
> > }
> >
> > But I couldn't get it to either produce a multipart range, or
> > a union of the two parts (i.e., [5, UINT_MAX]).  It makes me
> > wonder what I'm missing about how such ranges are supposed to
> > come into existence.
> >
> > Martin
> >
> There mare a couple of things..
>
> My guess is you are using a value_range?   value_ranges are an
> int_range<1> under the covers, so will never have more than a single
> range, or anti range.
>
> int_range is the type which allows for up to X subranges.
> calculations will be merged to fit within X subranges
> widest_irange is the type which allows for "unlimited" subranges...
> which currently happens to be capped at 255.. .  (its typedef'd as
> int_range<255>).
>
> widest_irange is the type used within the range-ops machinery and such,
> and then whatever result is calculated is "toned down" to whatever to
> user provides.
>
> so if union 

[Bug c++/96560] New: Substitution triggers compile-time error when it shouldn't

2020-08-10 Thread mpolacek at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96560

Bug ID: 96560
   Summary: Substitution triggers compile-time error when it
shouldn't
   Product: gcc
   Version: unknown
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c++
  Assignee: unassigned at gcc dot gnu.org
  Reporter: mpolacek at gcc dot gnu.org
  Target Milestone: ---

This triggers the static_assert while it shouldn't, because DR 1227 says "The
substitution proceeds in lexical order and stops when a condition that causes
deduction to fail is encountered."  Compiles with clang++/EDG.

template
struct enable_if { };

template
struct enable_if {
  using type = T;
};

template
struct hard_error {
  static_assert(sizeof(T) == 0);
  static inline constexpr bool value = true;
};

template
struct always_false {
  static inline constexpr bool value = false;
};

template
int foo (int, typename enable_if::value, int>::type = 0,
  typename enable_if::value, int>::type = 0 )
{
  return 0;
}

template
char const *foo (long)
{
  return "";
}

int
main ()
{
  char const *sz = foo(0);
}

[Bug libstdc++/90704] [LWG 3430] filesystem::path overloads for file streams are not conforming

2020-08-10 Thread manx-bugzilla at problemloesungsmaschine dot de
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90704

--- Comment #8 from Jörn Heusipp  ---
(In reply to Jonathan Wakely from comment #6)
> (In reply to Jörn Heusipp from comment #5)

> > and just pray and hope that libstdc++ will not change its non-conforming
> > SFINAE-using implementation details to some other non-conforming
> > implementation details in some future version.
> 
> Or just convert to filesystem::path explicitly.

I'd rather not use std::filesystem whatsoever at all than polluting the whole
code base with redundant explicit conversions. This is exactly what I am trying
to avoid here in the first place by implementing implicit conversion (which the
standard allows me to do). Your implementation breaks this, because you decided
to disregard the standard in order to break string_view conversions. If you
want to break string_view, break it. But please do so selectively and do not
break other implicit conversions at the same time. There is no reason to. This
is just sad.

> > Can you guarantee, that you wont ever change your non-conforming SFINAE
> > implementation whatsoever at all? If not, please just fix it. The standard
> > is absolutely clear here.
> 
> The standard is flawed and no longer follows its own design. I want to get
> some clarity about the intended design.

The intended design of std::basic_fstream::basic_fstream(const
filesystem::path& s, ios_base::openmode mode = ios_base::in | ios_base::out);
(literally from the standard) is unambiguously clear to me. I am allowed to
provide types which are implicitly convertible (as long as no other implicit
conversion causes ambiguity with other overloads). Again, this is IMO unrelated
to whatever the intention for std::string_view might or might not have been.
The fact that they rely on the same overload is coincidence. Are you arguing
that no user-defined type should ever be implicitly convertible to
std::filesystem::path? If so, can you provide reasoning?

Re: [PATCH] Rewrite get_size_range for irange API.

2020-08-10 Thread Andrew MacLeod via Gcc-patches

On 8/10/20 12:35 PM, Martin Sebor via Gcc-patches wrote:

On 8/10/20 5:47 AM, Aldy Hernandez wrote:



On 8/6/20 9:30 PM, Martin Sebor wrote:

On 8/6/20 8:53 AM, Aldy Hernandez via Gcc-patches wrote:



+  // Remove the unknown parts of a multi-range.
+  // This will transform [5,10][20,MAX] into [5,10].


Is this comment correct?  Wouldn't this result in returning smaller
sizes than the actual value allows?  If so, I'd expect that to cause
false positives (and in that case, if none of our tests fail we need
to add some that would).

By my reading of the code below it seems to return the upper range
(i.e., [20, MAX]) but I'm not fully acquainted with the new ranger
APIs yet.


I believe the comment is correct.  What I'm trying to do is remove 
[X,TYPE_MAX_VALUE] from the range



+  int pairs = positives.num_pairs ();
+  if (pairs > 1
+  && positives.upper_bound () == wi::to_wide (TYPE_MAX_VALUE 
(exptype)))

  {
-  if (integral)
-    {
-  /* Use the full range of the type of the expression when
- no value range information is available.  */
-  range[0] = TYPE_MIN_VALUE (exptype);
-  range[1] = TYPE_MAX_VALUE (exptype);
-  return true;
-    }
-
-  range[0] = NULL_TREE;
-  range[1] = NULL_TREE;
-  return false;
+  value_range last_range (exptype,
+  positives.lower_bound (pairs - 1),
+  positives.upper_bound (pairs - 1), VR_ANTI_RANGE);
+  positives.intersect (last_range);
  }


Notice that we start with positives == vr (where vr is the range 
returned by determine_value_range).


Then we build last_range which is the *opposite* of 
[X,TYPE_MAX_VALUE]. Notice the VR_ANTI_RANGE in the constructor.


(Note that the nomenclature VR_ANTI_RANGE is being used in the 
constructor to keep with the original API.  It means "create the 
inverse of this range".  Ideally, when the m_kind field disappears 
along with VR_ANTI_RANGEs, the enum could become RANGE_INVERSE or 
something less confusing.)


Finally, last_range is intersected with positives, which will become 
whatever VR had, minus the last sub-range.


Those that make sense, or did I miss something?

If you have a testcase that yields a false positive in my proposed 
patch, but not in the original code, please let me know so I can 
adjust the patch.


Sorry, I misremembered what get_size_range was used for.  It's used
to constrain the maximum size of memory accesses by functions like
memcpy (the third argument).  The use case I was thinking of was
to determine the bounds of the size of variably modified objects
(like VLAs).  It doesn't look like it's used for that.

With the current use taking the lower bound is the conservative
thing to do and using the upper bound would cause false positives.
With the latter it would be the other way around.  So the comment
makes sense to me now.

As an aside, here's a test case I played with.  I'd expect it to
trigger a warning because the upper bound of the range of array's
size is less than the lower bound of the range of the number of
bytes being written to it.

void f (void*);

void g (unsigned n1, unsigned n2)
{
  if (!(n1 >= 2 && n1 <= 3))   // n1 = [2, 3]
    return;

  char a[n1];

  if (!((n2 >= 5 && n2 <= 10)
    || (n2 >= 20)))    // n2 = [5, 10] U [20, UINT_MAX]
    return;

  __builtin_memset (a, 0, n2);
  f (a);
}

But I couldn't get it to either produce a multipart range, or
a union of the two parts (i.e., [5, UINT_MAX]).  It makes me
wonder what I'm missing about how such ranges are supposed to
come into existence.

Martin


There mare a couple of things..

My guess is you are using a value_range?   value_ranges are an 
int_range<1> under the covers, so will never have more than a single 
range, or anti range.


int_range is the type which allows for up to X subranges. 
calculations will be merged to fit within X subranges
widest_irange is the type which allows for "unlimited" subranges... 
which currently happens to be capped at 255.. .  (its typedef'd as 
int_range<255>).


widest_irange is the type used within the range-ops machinery and such, 
and then whatever result is calculated is "toned down" to whatever to 
user provides.


so if union results in [5,10] and [20, MAX]   and you provide a 
value_range for the result (, or int_range<1>), the result you get back 
will be [5, MAX].. so won't look like there are any multi-ranges going on.


and don't forget, EVRP only works with value_range, or int_range<1>.    
You cant currently get these ranges without the Ranger either... which 
is still on our branch.  We're hoping to get moving into trunk in the 
next couple of weeks.
I think Aldy is just trying to get the code functioning the same with 
the new API, even if we cant get the more precise ranges yet. Then when 
the ranger is added in, we switch you over and voila... multi-ranges.


Andrew



Re: [RFC] libstdc++: Fix pretty-printing old implementations of std::unique_ptr

2020-08-10 Thread Jonathan Wakely via Gcc-patches

On 10/08/20 09:45 -0400, Andres Rodriguez wrote:

*ping*


As it says at https://gcc.gnu.org/lists.html all patches for libstdc++
need to be sent to the libstdc++ mailing as well as the gcc-patches
list. Otherwise I won't see them, and everybody else will ignore them.


On Tue, Aug 4, 2020 at 10:51 AM Andres Rodriguez  wrote:


On binaries compiled against gcc5 the impl_type parameter is None,
which results in an exception being raised by is_specialization_of()

These versions of std::unique_ptr have the tuple as a root element.


The unique_ptr implementation in GCC 5 should already be handled by
the second branch which handles a tuple member:

if is_specialization_of(impl_type, '__uniq_ptr_data') \
or is_specialization_of(impl_type, '__uniq_ptr_impl'):
tuple_member = val['_M_t']['_M_t']
elif is_specialization_of(impl_type, 'tuple'):
tuple_member = val['_M_t']
else:
raise ValueError("Unsupported implementation for unique_ptr: %s" % 
impl_type)

We have a test that's supposed to verify that printing the old
unique_ptr implementation still works:
https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libstdc%2B%2B-v3/testsuite/libstdc%2B%2B-prettyprinters/compat.cc;h=f1c3b599634af2eb0a5a0889c528d9de2d21aacd;hb=HEAD

So the right fix is to work out why that branch isn't taken, and fix
it, not add a kluge.

If impl_type is None that means the .tag field is missing, which seems
to be because the _M_t member is declared using the __tuple_type
typedef, rather than std::tuple itself.

That's fixed by the attached patch.

Tested x86_64-linux, pushed to master. I'll backport it to the
branches too.


commit ed11f7e84bcae89f486f5023e566726a7faa7dd4
Author: Jonathan Wakely 
Date:   Mon Aug 10 18:44:06 2020

libstdc++: Fix compatibility support in unique_ptr pretty printer

The support for the old std::unique_ptr implementation was failing,
because it tried to work on a typedef instead of the underlying type.
The test supposed to verify the support worked wasn't using a typedef,
so didn't notice the problem.

libstdc++-v3/ChangeLog:

* python/libstdcxx/v6/printers.py (UniquePointerPrinter.__init__):
Use gdb.Type.strip_typedefs().
* testsuite/libstdc++-prettyprinters/compat.cc: Use a typedef in
the emulated old type.

diff --git a/libstdc++-v3/python/libstdcxx/v6/printers.py b/libstdc++-v3/python/libstdcxx/v6/printers.py
index 0bf307b8e5f..c0f061f79c1 100644
--- a/libstdc++-v3/python/libstdcxx/v6/printers.py
+++ b/libstdc++-v3/python/libstdcxx/v6/printers.py
@@ -245,7 +245,7 @@ class UniquePointerPrinter:
 
 def __init__ (self, typename, val):
 self.val = val
-impl_type = val.type.fields()[0].type.tag
+impl_type = val.type.fields()[0].type.strip_typedefs()
 # Check for new implementations first:
 if is_specialization_of(impl_type, '__uniq_ptr_data') \
 or is_specialization_of(impl_type, '__uniq_ptr_impl'):
@@ -253,7 +253,7 @@ class UniquePointerPrinter:
 elif is_specialization_of(impl_type, 'tuple'):
 tuple_member = val['_M_t']
 else:
-raise ValueError("Unsupported implementation for unique_ptr: %s" % impl_type)
+raise ValueError("Unsupported implementation for unique_ptr: %s" % str(impl_type))
 tuple_impl_type = tuple_member.type.fields()[0].type # _Tuple_impl
 tuple_head_type = tuple_impl_type.fields()[1].type   # _Head_base
 head_field = tuple_head_type.fields()[0]
@@ -262,7 +262,7 @@ class UniquePointerPrinter:
 elif head_field.is_base_class:
 self.pointer = tuple_member.cast(head_field.type)
 else:
-raise ValueError("Unsupported implementation for tuple in unique_ptr: %s" % impl_type)
+raise ValueError("Unsupported implementation for tuple in unique_ptr: %s" % str(impl_type))
 
 def children (self):
 return SmartPtrIterator(self.pointer)
diff --git a/libstdc++-v3/testsuite/libstdc++-prettyprinters/compat.cc b/libstdc++-v3/testsuite/libstdc++-prettyprinters/compat.cc
index f1c3b599634..c681becf8b9 100644
--- a/libstdc++-v3/testsuite/libstdc++-prettyprinters/compat.cc
+++ b/libstdc++-v3/testsuite/libstdc++-prettyprinters/compat.cc
@@ -53,7 +53,9 @@ namespace std
 {
   unique_ptr(T* p) { _M_t._M_head_impl = p; }
 
-  tuple _M_t;
+  using __tuple_type = tuple;
+
+  __tuple_type _M_t;
 };
 
   // Old representation of std::optional, before GCC 9


[Bug rtl-optimization/48037] Missed optimization: unnecessary register moves

2020-08-10 Thread glisse at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=48037

--- Comment #10 from Marc Glisse  ---
We now generate just
sqrtpd  %xmm0, %xmm0
for 2 and 4,
sqrtpd  (%rdi), %xmm0
for 3, and
movupd  (%rdi), %xmm0
sqrtpd  %xmm0, %xmm0
for 1 (for alignment reasons I guess, the movu disappears with -mavx).

Should we close it as fixed? Or update the testcase to perform a different
operation that isn't recognized?

[Bug target/96559] New: Wrong code with -march=z900 -mtune=z9-109

2020-08-10 Thread marxin at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96559

Bug ID: 96559
   Summary: Wrong code with -march=z900 -mtune=z9-109
   Product: gcc
   Version: 11.0
Status: UNCONFIRMED
  Keywords: wrong-code
  Severity: normal
  Priority: P3
 Component: target
  Assignee: unassigned at gcc dot gnu.org
  Reporter: marxin at gcc dot gnu.org
CC: krebbel at gcc dot gnu.org
  Target Milestone: ---
  Host: x86_64-linux-gnu
Target: s390x-linux-gnu

For the following test-case (reduced from Firefox):

$ cat ff.ii
char compute___trans_tmp_3;
long CountLeadingZeroes64_aValue;
void CeilingLog2() {
  compute___trans_tmp_3 = __builtin_clzll(CountLeadingZeroes64_aValue);
}

we end with:

$ ./xgcc -B. ff.ii -march=z900 -mtune=z9-109 -S -o/dev/stdout -O1
...
.globl __clzdi2
.align  8
.globl _Z11CeilingLog2v
.type   _Z11CeilingLog2v, @function
_Z11CeilingLog2v:
.LFB0:
stmg%r12,%r15,96(%r15)
.LCFI0:
aghi%r15,-160
.LCFI1:
larl%r12,.LANCHOR0
lg  %r2,0(%r12)
brasl   %r14,__clzdi2
stc %r2,8(%r12)
lg  %r4,272(%r15)
lmg %r12,%r15,256(%r15)

which is bogus as __clzdi2 points to the very same place as _Z11CeilingLog2v.
So 
 brasl  %r14,__clzdi2 is an infinite loop.

The .globl for __clzdi2 is emitted in the following back-trace:

Breakpoint 5, default_globalize_label (stream=0x2712300, name=0x77700d50
"__clzdi2") at /home/marxin/Programming/gcc/gcc/varasm.c:7321
7321  fputs (GLOBAL_ASM_OP, stream);
Missing separate debuginfos, use: zypper install
libmpfr6-debuginfo-4.1.0-1.1.x86_64 libzstd1-debuginfo-1.4.5-2.2.x86_64
(gdb) bt
#0  default_globalize_label (stream=0x2712300, name=0x77700d50 "__clzdi2")
at /home/marxin/Programming/gcc/gcc/varasm.c:7321
#1  0x0125d536 in default_external_libcall (fun=0x77764648) at
/home/marxin/Programming/gcc/gcc/targhooks.c:107
#2  0x016c5d15 in assemble_external_libcall (fun=0x77764648) at
/home/marxin/Programming/gcc/gcc/varasm.c:2506
#3  0x00cf28f6 in emit_library_call_value_1 (retval=1,
orgfun=0x77764648, value=0x0, fn_type=, outmode=E_SImode,
nargs=1, args=) at /home/marxin/Programming/gcc/gcc/calls.c:5272
#4  0x010dd18b in emit_library_call_value (fun=0x77764648,
value=0x0, fn_type=LCT_CONST, outmode=E_SImode, arg1=0x77764630,
arg1_mode=E_DImode) at /home/marxin/Programming/gcc/gcc/rtl.h:4245
#5  0x010d06d2 in expand_unop (mode=E_DImode, unoptab=clz_optab,
op0=0x77764630, target=0x777644f8, unsignedp=1) at
/home/marxin/Programming/gcc/gcc/optabs.c:3059
#6  0x00cd49d5 in expand_builtin_unop (target_mode=E_SImode,
exp=, target=0x777644f8, subtarget=, op_optab=clz_optab) at /home/marxin/Programming/gcc/gcc/tree.h:3296
#7  0x00cea061 in expand_builtin (exp=,
target=, subtarget=0x0, mode=E_SImode, ignore=0) at
/home/marxin/Programming/gcc/gcc/builtins.c:8236
#8  0x00e38f5d in expand_expr_real_1 (exp=,
target=, tmode=E_SImode, modifier=EXPAND_NORMAL,
alt_rtl=0x7fffdb48, inner_reference_p=) at
/home/marxin/Programming/gcc/gcc/expr.c:11237
#9  0x00e42281 in store_expr (exp=,
target=0x777644f8, call_param_p=, nontemporal=, reverse=) at /home/marxin/Programming/gcc/gcc/expr.c:5852
#10 0x00e444cf in expand_assignment (to=,
from=, nontemporal=) at
/home/marxin/Programming/gcc/gcc/expr.c:5588
#11 0x00d09e34 in expand_call_stmt (stmt=0x77fc4510) at
/home/marxin/Programming/gcc/gcc/cfgexpand.c:2701
#12 expand_gimple_stmt_1 (stmt=) at
/home/marxin/Programming/gcc/gcc/cfgexpand.c:3682
#13 expand_gimple_stmt (stmt=) at
/home/marxin/Programming/gcc/gcc/cfgexpand.c:3847
#14 0x00d0f60b in expand_gimple_basic_block (bb=,
disable_tail_calls=) at
/home/marxin/Programming/gcc/gcc/cfgexpand.c:5888
#15 0x00d110b7 in (anonymous namespace)::pass_expand::execute
(this=, fun=0x77759000) at
/home/marxin/Programming/gcc/gcc/cfgexpand.c:6572
#16 0x0112fd75 in execute_one_pass (pass=) at /home/marxin/Programming/gcc/gcc/passes.c:2509
#17 0x0113009a in execute_pass_list_1 (pass=) at /home/marxin/Programming/gcc/gcc/passes.c:2597
#18 0x01130123 in execute_pass_list (fn=0x77759000, pass=) at /home/marxin/Programming/gcc/gcc/passes.c:2608
#19 0x00d44cec in cgraph_node::expand (this=) at
/home/marxin/Programming/gcc/gcc/context.h:48
#20 0x00d462ff in expand_all_functions () at
/home/marxin/Programming/gcc/gcc/cgraphunit.c:2472
#21 symbol_table::compile (this=0x775d9000) at
/home/marxin/Programming/gcc/gcc/cgraphunit.c:2835
#22 0x00d48713 in symbol_table::compile (this=0x775d9000) at
/home/marxin/Programming/gcc/gcc/cgraphunit.c:3013
#23 symbol_table::finalize_compilation_unit (this=0x775d9000) at
/home/marxin/Programming/gcc/gcc/cgraphunit.c:3013
#24 0x01265fae in 

[Bug target/50829] avx extra copy for _mm256_insertf128_pd

2020-08-10 Thread glisse at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=50829

Marc Glisse  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
  Known to work||10.1.0
 Resolution|--- |FIXED
  Known to fail||9.3.0

--- Comment #14 from Marc Glisse  ---
This was fixed (by Jakub I think).

Re: [RFC PATCH v1 1/1] PPC64: Implement POWER Architecture Vector Function ABI.

2020-08-10 Thread GT via Gcc-patches
‐‐‐ Original Message ‐‐‐
On Friday, August 7, 2020 4:59 PM, Jakub Jelinek  wrote:

> On Fri, Aug 07, 2020 at 08:35:52PM +, Bert Tenjy via Gcc-patches wrote:
>
> > The document describing POWER Architecture Vector Function interface is
> > tentatively at: 
> > https://sourceware.org/glibc/wiki/Homepage?action=AttachFile=view=powerarchvectfuncabi.html
>
> Doesn't exist.

I can't tell what the issue is with the link as given above. But the one below 
does actually open
the page for me.

https://sourceware.org/glibc/wiki/HomePage?action=AttachFile=view=powerarchvectfuncabi.html


>
> > -   if (TARGET_VSX)
> > -   {
> > -clonei->vecsize_mangle = 'b';
> >
> >
> > -ret = 1;
> >
> >
> > -   }
> > -   clonei->mask_mode = VOIDmode;
> > -   switch (clonei->vecsize_mangle)
> > -   {
> > -   case 'b':
> > -clonei->vecsize_int = 128;
> >
> >
> > -clonei->vecsize_float = 128;
> >
> >
> > -break;
> >
> >
> > -   case 'c':
> > -clonei->vecsize_int = 128;
> >
> >
> > -clonei->vecsize_float = 128;
> >
> >
>
> So what is this 'c' case here for?
>

I should have removed it. It will be deleted in the next version.

> > -break;
> >
> >
> > -   default:
> > -gcc_unreachable ();
> >
> >
>
> If (!TARGET_VSX), this will abort (as you only set vecsize_mangle to 'b'
> if TARGET_VSX, otherwise nothing sets it (so it remains 0).
>
> The way this works is that the caller calls the target hook with 0
> as last argument, and the hook either returns 0 (means not supported at
> all), or returns number of supported variants and provides the first one.
> If more than one are supported, the caller will keep iterating on those.
> E.g. on x86, we support 4 modes (b, c, d, e) for exported functions and
> for non-exported ones just choose a single one based on the ISA, because in
> that case it is not a matter of ABI.
>
> For PowerPC, if all you want to support is b which requires VSX, then the
> right thing is for !TREE_PUBLIC functions return 0 if !TARGET_VSX and
> otherwise set vecsize_mangle to 'b' and in the end return 1, for exported
> functions always set it to 'b' (and in the end return 1).
> Then ensure that the 'b' variants of function definitions get target ("vsx")
> attribute added if !TARGET_VSX.
>

So setting attribute "vsx" for 'b' variants of function definitions is what
should go in function rs6000_simd_clone_usable?

Bert.


Re: std:vec for classes with constructor?

2020-08-10 Thread Jakub Jelinek via Gcc-patches
On Mon, Aug 10, 2020 at 06:58:52PM +0200, Richard Biener wrote:
> We want to construct / destruct on push/pop, not on allocation.
> We also want to use std::move upon reallocation (but then we can't use 
> realloc, can we?) 

I guess so, but we could stop using xrealloc or ggc_realloc only for types
which aren't trivially moveable using some template magic.

Jakub



[patch, fortran, committed] Fix PR 96556, segfault on NULL pointer dereference

2020-08-10 Thread Thomas König

Hi,

in the code about DO loop warning I recently introduced, there was
a hidden NULL pointer dereference found by Jürgen Reuter and fixed
as obvious and simple in r11-2636.

Fix NULL pointer dereference in doloop_contained_function_call.

Thanks to Jürgen for the bug report and to Dominique for spotting
what commit this was caused by.

Unfortunately, not all bugs are quite as simple and obvious as this
one :-)

gcc/fortran/ChangeLog:

PR fortran/96556
* frontend-passes.c (doloop_contained_function_call):
Do not dereference a NULL pointer for value.function.esym.

gcc/testsuite/ChangeLog:

PR fortran/96556
* gfortran.dg/do_check_15.f90: New test.
diff --git a/gcc/fortran/frontend-passes.c b/gcc/fortran/frontend-passes.c
index 6bcb1f06b1c..83f6fd804b1 100644
--- a/gcc/fortran/frontend-passes.c
+++ b/gcc/fortran/frontend-passes.c
@@ -2329,7 +2329,8 @@ doloop_contained_function_call (gfc_expr **e,
   gfc_symbol *sym, *do_var;
   contained_info *info;
 
-  if (expr->expr_type != EXPR_FUNCTION || expr->value.function.isym)
+  if (expr->expr_type != EXPR_FUNCTION || expr->value.function.isym
+  || expr->value.function.esym == NULL)
 return 0;
 
   sym = expr->value.function.esym;
diff --git a/gcc/testsuite/gfortran.dg/do_check_15.f90 b/gcc/testsuite/gfortran.dg/do_check_15.f90
new file mode 100644
index 000..f67452b4660
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/do_check_15.f90
@@ -0,0 +1,58 @@
+! { dg-do compile }
+! PR fortran/96556 - this used to cause an ICE.
+! Test case by Juergen Reuter.
+module polarizations
+
+  implicit none
+  private
+
+  type :: smatrix_t
+ private
+ integer :: dim = 0
+ integer :: n_entry = 0
+ integer, dimension(:,:), allocatable :: index
+   contains
+ procedure :: write => smatrix_write
+  end type smatrix_t
+
+  type, extends (smatrix_t) :: pmatrix_t
+ private
+   contains
+ procedure :: write => pmatrix_write
+ procedure :: normalize => pmatrix_normalize
+  end type pmatrix_t
+
+contains
+
+  subroutine msg_error (string)
+character(len=*), intent(in), optional :: string
+  end subroutine msg_error
+  
+  subroutine smatrix_write (object)
+class(smatrix_t), intent(in) :: object
+  end subroutine smatrix_write
+
+  subroutine pmatrix_write (object)
+class(pmatrix_t), intent(in) :: object
+call object%smatrix_t%write ()
+  end subroutine pmatrix_write
+
+  subroutine pmatrix_normalize (pmatrix)
+class(pmatrix_t), intent(inout) :: pmatrix
+integer :: i, hmax
+logical :: fermion, ok
+do i = 1, pmatrix%n_entry
+   associate (index => pmatrix%index(:,i))
+ if (index(1) == index(2)) then
+call error ("diagonal must be real")
+ end if
+   end associate
+end do
+  contains
+subroutine error (msg)
+  character(*), intent(in) :: msg
+  call pmatrix%write ()
+end subroutine error
+  end subroutine pmatrix_normalize
+
+end module polarizations


[Bug fortran/96556] [11.0 regression] ICE via segmentation violation

2020-08-10 Thread tkoenig at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96556

Thomas Koenig  changed:

   What|Removed |Added

 Status|ASSIGNED|RESOLVED
 Resolution|--- |FIXED

--- Comment #7 from Thomas Koenig  ---
Fixed, closing.

Thanks for the bug report and the quick identification of the culprit!

[Bug libstdc++/90704] [LWG 3430] filesystem::path overloads for file streams are not conforming

2020-08-10 Thread redi at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90704

--- Comment #7 from Jonathan Wakely  ---
btw in practice the fact this is SUSPENDED today is absolutely no different to
the status yesterday. It didn't work in GCC yesterday and it doesn't work
today. The only change is I've publicly stated my intention to discuss this
with the committee before doing anything here.

I could revert the status, and still do nothing here, but that serves no
purpose.

[Bug fortran/96556] [11.0 regression] ICE via segmentation violation

2020-08-10 Thread cvs-commit at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96556

--- Comment #6 from CVS Commits  ---
The master branch has been updated by Thomas Kथà¤nig :

https://gcc.gnu.org/g:a5da50ed65a835dc1ed6179e3f2b6164fd6e4969

commit r11-2636-ga5da50ed65a835dc1ed6179e3f2b6164fd6e4969
Author: Thomas Koenig 
Date:   Mon Aug 10 19:10:26 2020 +0200

Fix NULL pointer dereference in doloop_contained_function_call.

gcc/fortran/ChangeLog:

PR fortran/96556
* frontend-passes.c (doloop_contained_function_call):
Do not dereference a NULL pointer for value.function.esym.

gcc/testsuite/ChangeLog:

PR fortran/96556
* gfortran.dg/do_check_15.f90: New test.

[Bug libstdc++/90704] [LWG 3430] filesystem::path overloads for file streams are not conforming

2020-08-10 Thread redi at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90704

--- Comment #6 from Jonathan Wakely  ---
(In reply to Jörn Heusipp from comment #5)
> > Status: SUSPENDED
> 
> Well, coming from bug 95642, which has been marked as a duplicate of this
> bug, this is *very* disappointing.
> 
> 
> The C++17 standard absolutely clearly specifies that the constructor is
> overloaded for std::filesystem::path. libstdc++'s SFINAE implementation is
> incompatible because it fails for user-defined types that are implicitly
> convertible to std::filesystem::path.

And also for string_view, because that's convertible to filesystem::path, and
that is the topic of the issue reported against the standard.

> In order to at all be able to sensibly use libstdc++ std::filesystem::path
> in my codebase, I would have to put this awful work-around (making tons of
> assumptions about libstdc++'s enable_if expression) into my own type
> mpt::PathString:

Or you can just convert it to filesystem::path explicitly before passing it to
fstream members.


> and just pray and hope that libstdc++ will not change its non-conforming
> SFINAE-using implementation details to some other non-conforming
> implementation details in some future version.

Or just convert to filesystem::path explicitly.

> Can you guarantee, that you wont ever change your non-conforming SFINAE
> implementation whatsoever at all? If not, please just fix it. The standard
> is absolutely clear here.

The standard is flawed and no longer follows its own design. I want to get some
clarity about the intended design.

> After having said all this, I am not even sure anymore that this bug report
> here actually is a duplicate of my bug 95642. I am concerned about implicit
> conversion to std::filesystem::path not working, while this bug report is
> concerned about std::string_view not working. Please consider re-opening bug
> 95642.

They're exactly the same.

string_view is expected to work by converting to filesystem::path, which is the
same as your type.

[Bug target/96558] New: [11 Regression] ICE in extract_constrain_insn, at recog.c:2195 (error: insn does not satisfy its constraints)

2020-08-10 Thread asolokha at gmx dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96558

Bug ID: 96558
   Summary: [11 Regression] ICE in extract_constrain_insn, at
recog.c:2195 (error: insn does not satisfy its
constraints)
   Product: gcc
   Version: 11.0
Status: UNCONFIRMED
  Keywords: ice-on-valid-code
  Severity: normal
  Priority: P3
 Component: target
  Assignee: unassigned at gcc dot gnu.org
  Reporter: asolokha at gmx dot com
  Target Milestone: ---
Target: x86_64-unknown-linux-gnu-gcc

gcc-11.0.0-alpha20200809 snapshot (g:71197a5d13d0b540a9b5efe7ae2512d76386e9d1)
ICEs when compiling the following testcase w/ -O2 -fno-expensive-optimizations
-fno-gcse:

int ky;
long int h1;
__int128 f1;

int
sd (void);

int __attribute__ ((simd))
i8 (void)
{
  __int128 vh;

  if (sd () == 0)
h1 = 0;

  do
{
  long int lf = h1;
  long int w1 = f1;

  if (w1 == 0)
lf = 0;
  else
lf = lf && w1;

  ky += lf;

  vh = lf | f1;
  f1 = 1;
}
  while (vh < (f1 ^ 2));

  return 0;
}

% x86_64-unknown-linux-gnu-gcc-11.0.0 -O2 -fno-expensive-optimizations
-fno-gcse -c yocbyzz3.c
yocbyzz3.c: In function 'i8.simdclone.6':
yocbyzz3.c:34:1: error: insn does not satisfy its constraints:
   34 | }
  | ^
(insn 157 14 158 3 (parallel [
(set (reg:DI 20 xmm0 [orig:121 _53 ] [121])
(const_int 0 [0]))
(clobber (reg:CC 17 flags))
]) "yocbyzz3.c":13:6 68 {*movdi_xor}
 (expr_list:REG_UNUSED (reg:CC 17 flags)
(nil)))
during RTL pass: cprop_hardreg
yocbyzz3.c:34:1: internal compiler error: in extract_constrain_insn, at
recog.c:2195
0x68e957 _fatal_insn(char const*, rtx_def const*, char const*, int, char
const*)
   
/var/tmp/portage/sys-devel/gcc-11.0.0_alpha20200809/work/gcc-11-20200809/gcc/rtl-error.c:108
0x68e97d _fatal_insn_not_found(rtx_def const*, char const*, int, char const*)
   
/var/tmp/portage/sys-devel/gcc-11.0.0_alpha20200809/work/gcc-11-20200809/gcc/rtl-error.c:118
0x68cfb5 extract_constrain_insn(rtx_insn*)
   
/var/tmp/portage/sys-devel/gcc-11.0.0_alpha20200809/work/gcc-11-20200809/gcc/recog.c:2195
0xcf76fb copyprop_hardreg_forward_1
   
/var/tmp/portage/sys-devel/gcc-11.0.0_alpha20200809/work/gcc-11-20200809/gcc/regcprop.c:802
0xcf864e execute
   
/var/tmp/portage/sys-devel/gcc-11.0.0_alpha20200809/work/gcc-11-20200809/gcc/regcprop.c:1367

[Bug fortran/96556] [11.0 regression] ICE via segmentation violation

2020-08-10 Thread tkoenig at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96556

Thomas Koenig  changed:

   What|Removed |Added

   Assignee|unassigned at gcc dot gnu.org  |tkoenig at gcc dot 
gnu.org
 Status|NEW |ASSIGNED

--- Comment #5 from Thomas Koenig  ---
One-line fix.

Re: std:vec for classes with constructor?

2020-08-10 Thread Richard Biener via Gcc-patches
On August 10, 2020 3:51:28 PM GMT+02:00, Jakub Jelinek via Gcc-patches 
 wrote:
>On Mon, Aug 10, 2020 at 02:57:31PM +0200, Aldy Hernandez wrote:
>> > I think it would be nice to see e.g. in -fdump-tree-gimple dump
>> > on ipa-fnsummary.c what value_range ctors does it call for the
>auto_vec and
>> > if that is what we want, other than that LGTM.
>> 
>> I see the following in the dump:
>> 
>> evaluate_properties_for_edge()
>> {
>> ...
>>   struct auto_vec known_value_ranges;
>> ...
>>   try
>> {
>> ...
>>   auto_vec, 32>::auto_vec (_value_ranges);
>> ...
>> 
>> It looks like the auto_vec constructor is calling the int_range<1>
>> constructor correctly:
>> 
>> auto_vec, 32>::auto_vec (struct auto_vec * const this)
>> {
>> ...
>> int_range<1>::int_range (D.130911);
>> 
>> 
>> The int_range<1> constructor also looks correct:
>> 
>> int_range<1>::int_range (struct int_range * const this, const struct
>> int_range & other)
>> {
>>   *this = {CLOBBER};
>>   {
>> _1 = >D.92184;
>> _2 = >m_ranges;
>> irange::irange (_1, _2, 1);
>> _3 = >D.92184;
>> _4 = >D.92184;
>> irange::operator= (_3, _4);
>>   }
>> }
>> 
>> If y'all agree, I will push the patch with Jakub's suggested changes.
>
>I believe even with that change vec doesn't work properly with
>(arbitrary)
>non-PODs that need non-trivial construction or destruction.
>I know you haven't introduced that yourself, but just would like to
>discuss
>the semantics we want.
>
>To test, I've tried:
>--- vec.c.xx   2020-01-12 11:54:38.533381424 +0100
>+++ vec.c  2020-08-10 15:33:40.600680476 +0200
>@@ -544,9 +544,34 @@ test_auto_delete_vec ()
> 
> /* Run all of the selftests within this file.  */
> 
>+struct SSXS
>+{
>+  SSXS () { __builtin_printf ("SSXS ()\n"); }
>+  SSXS (const SSXS &) { __builtin_printf ("SSXS (const SSXS &)\n"); }
>+  SSXS = (const SSXS &) { __builtin_printf ("operator= (const
>SSXS &)\n"); }
>+  ~SSXS () { __builtin_printf ("~SSXS ()\n"); }
>+  int s;
>+};
>+
> void
> vec_c_tests ()
> {
>+  {
>+auto_vec ssxs;
>+__builtin_printf ("constructed\n");
>+SSXS ssxso;
>+__builtin_printf ("ssxso\n");
>+for (int i = 0; i < 16; i++)
>+  {
>+__builtin_printf ("safe_push start\n");
>+{
>+  ssxs.safe_push (ssxso);
>+}
>+__builtin_printf ("safe_push end\n");
>+  }
>+__builtin_printf ("about to destruct\n");
>+  }
>+  __builtin_printf ("destructed\n");
>   test_quick_push ();
>   test_safe_push ();
>   test_truncate ();
>
>and I see following.  Let me add comments to that.
>
>#So auto_vec constructor will default initialize all m_data
>#elements (i.e. all the embedded objects):
>SSXS ()
>SSXS ()
>SSXS ()
>SSXS ()
>SSXS ()
>SSXS ()
>SSXS ()
>SSXS ()
>constructed
>#And now the extra object I've created.
>SSXS ()
>ssxso
># Now, 8 times we invoke an assignment operator, the objects
># are already constructed, so all is good.
>safe_push start
>operator= (const SSXS &)
>safe_push end
>safe_push start
>operator= (const SSXS &)
>safe_push end
>safe_push start
>operator= (const SSXS &)
>safe_push end
>safe_push start
>operator= (const SSXS &)
>safe_push end
>safe_push start
>operator= (const SSXS &)
>safe_push end
>safe_push start
>operator= (const SSXS &)
>safe_push end
>safe_push start
>operator= (const SSXS &)
>safe_push end
>safe_push start
>operator= (const SSXS &)
>safe_push end
># Now, 9th safe_push, we ran out of room in the embedded buffer.
># vec_copy_construct will now copy construct the 8 elements
># in the new array which will be used from now on.
>safe_push start
>SSXS (const SSXS &)
>SSXS (const SSXS &)
>SSXS (const SSXS &)
>SSXS (const SSXS &)
>SSXS (const SSXS &)
>SSXS (const SSXS &)
>SSXS (const SSXS &)
>SSXS (const SSXS &)
># But important note, we do not construct the object at new
>m_vecdata[8],
># only call assignment operator on it.  That is invalid C++, isn't it?
>operator= (const SSXS &)
>safe_push end
>safe_push start
># And again, and again ...
>operator= (const SSXS &)
>safe_push end
>safe_push start
>operator= (const SSXS &)
>safe_push end
>safe_push start
>operator= (const SSXS &)
>safe_push end
>safe_push start
>operator= (const SSXS &)
>safe_push end
>safe_push start
>operator= (const SSXS &)
>safe_push end
>safe_push start
>operator= (const SSXS &)
>safe_push end
>safe_push start
>operator= (const SSXS &)
>safe_push end
>about to destruct
># Now we destruct the 8 embedded elts in auto_vec plus the variable
># I had there, but the 16 elts of the m_vecdata array, 8 of those
># were copy constructed and then operator= modified, while
># the other 8 were only operator= set, are not destructed ever.
>~SSXS ()
>~SSXS ()
>~SSXS ()
>~SSXS ()
>~SSXS ()
>~SSXS ()
>~SSXS ()
>~SSXS ()
>~SSXS ()
>destructed
>
>So, for vec on types with non-trivial construction and/or destruction,
>the
>question is, do we want to construct the elements whenever underlying
>storage for those is allocated (i.e. as now let the embedded buffer
>have
>constructed 

[Bug ipa/96482] [10/11 Regression] Combination of -finline-small-functions and ipa-cp optimisations causes incorrect values being passed to a function since r279523

2020-08-10 Thread yevh.kolesnikov at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96482

--- Comment #6 from Yevhenii Kolesnikov  ---
(In reply to Jan Hubicka from comment #4)
> that patch makes ccp to actually use the bit info ipa-cp determines. Before
> we used it only to detect pointer alignments if I remember correctly. So it
> looks like propagation bug uncovered by the change.  Smaller testcase or
> reproduction steps would be indeed welcome.

Unfortunately, I wasn't able to create a standalone reproducer yet. Even in
mesa it's sometimes finicky to catch this bug. For example, simply adding an
fprintf to aforementioned function addr_to_index, prevents it from inlining.  

As for the reproduction steps with mesa:

1. Build mesa as described in comment #5
2. LD_LIBRARY_PATH=$LD_LIBRARY_PATH:~/builds/mesa-bug/lib/ mpv any_video.mp4
3. mesa crashes. Backtrace will slightly differ depending on the driver in use.
So far, I've been describing behavior on "iris" driver (default driver for
Intel GPUs). I assume, similar optimisation patterns occur in several places,
when building mesa.

Re: Changes to allow PowerPC to change the long double type to use the IEEE 128-bit floating point format

2020-08-10 Thread Michael Meissner via Gcc
On Sat, Aug 08, 2020 at 03:33:51PM +0200, Thomas König wrote:
> Hi Michael,
> 
> I have shortened the distribution list somewhat for the Fortran-relevant
> parts.
> 
> >I want to discuss changes that I think we need to make across the open source
> >toochain to allow us to change the long double type on PowerPC hardware from
> >using the IBM extended double (i.e. a pair of doubles) to the IEEE 128-bit
> >format defined in IEEE 754.
> >
> >I wasn't sure whom to address this to, so I took a scatter shot approach.  I
> >likely missed a few people, and some people were added that may not need to
> >participate in the discussion.  Sorry for either not including you initially 
> >or
> >for including you by mistake.
> >
> >I added people from the following areas:
> >
> > PowerPC folk
> >
> > Langugage maintainers: At the moment, only the C/C++ front ends have
> > code to support both 128-bit floating point types.  The other languages
> > use just the defaults provided by the machine maintainers.  However, it
> > may be we will need to think about rules for code being compiled and
> > linked with a different long double format.
> 
> Currently, we support the IBM format with gfortran.  So, we have to
> look at a) library code called from user routines, and b) user code
> compiled with one version calling another.

I have patches I'm working through that allows the whole toolchain to be built
with the new default, and I have run through the C, C++, and Fortran test
suites with a new version of glibc.  In fact there were 2-3 tests that
traditionally fail with IBM extended double that now pass.

IMHO, changing the default is only appropriate for times like a distribution
major number changes, where backwards and forwards compatibility is carefully
controlled.  But before we can contemplate doing this, we need the ability to
change the default and have it all work together.

In this case, there were no modifications to the gfortran sources.  It is all
controlled from the rs6000 backend gcc and libgcc machine specific functions.
But there is no backwards compatibility if the user used explicit long double
(in C/C++) or real*16 (gfortran).

One of the keys is changing the names of the built-in functions.  Glibc in
math.h changes all of the long double functions it declares to be either the
IBM extended double or IEEE 128-bit versions of the functions.  Similarly
Libstdc++ is in the middle of doing the changes for tht as well.

In addition, the GNU compiler will change the external names of the built-in
functions to be the IEEE 128-bit names.  This allows for C/C++ users to not use
math.h and still get the right function called (there were a few tests in the
test suite that had to be fixed).  It also allows Gfortran to use these
functions by default.

> 
> As for a), this is something that can be done using the right m4
> macros. We might even, with some hackery, be able to provide two
> versions of the functions with the new library.

Glibc and Libstdc++ are doing this right now.  Tulio, Carlos can speak of
glibc, and Johnathon can speak of libstdc++'s efforts.  Generally in a mixed
library approach, you provide both names, and you do not use long double,
instead you use the explicit types (__ibm128 and __float128) for the two
formats.  Typically they use the -mno-gnu-attributes option which says to
disable the gnu attributes that we use to mark which type of long double is
used, so that it can be linked with either ABI.

 
> For b), I do not have a clear solution for Fortran. But I also have
> no idea how this is supposed to work in other languages,
> when a user uses code compiles something with "long double"
> with gcc 10 and links it against "long double" with gcc 11 -
> what are your plans for that?
> 
> It would be possible to annotate every function with calls long double
> with the new format somehow (ugh), or you could make a clear ABI break.
> This would mean a new, incompatible version of libgfortran, but we would
> have to restrict that to POWER (would that be possible?).
> We cannot impose an ABI change on everybody else to this.

We do annotate each function that has long double arguments or returns long
double arguments already with gnu attributes.  There are some issues with it,
and I want to delve into it deeper.

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


[PING 3][PATCH] improve validation of attribute arguments (PR c/78666)

2020-08-10 Thread Martin Sebor via Gcc-patches

Ping: https://gcc.gnu.org/pipermail/gcc-patches/2020-July/549686.html

On 7/26/20 11:41 AM, Martin Sebor wrote:

Ping: https://gcc.gnu.org/pipermail/gcc-patches/2020-July/549686.html

On 7/16/20 4:53 PM, Martin Sebor wrote:

Ping: https://gcc.gnu.org/pipermail/gcc-patches/2020-July/549686.html

(Jeff, I forgot to mention this patch when we spoke earlier today.)

On 7/8/20 6:01 PM, Martin Sebor wrote:

GCC has gotten better at detecting conflicts between various
attributes but it still doesn't do a perfect job of detecting
similar problems due to mismatches between contradictory
arguments to the same attribute.  For example,

   __attribute ((alloc_size (1))) void* allocate (size_t, size_t);

followed by

   __attribute ((alloc_size (2))) void* allocate (size_t, size_t);

is accepted with the former overriding the latter in calls to
the function.  Similar problem exists with a few other attributes
that take arguments.

The attached change adds a new utility function that checks for
such mismatches and issues warnings.  It also adds calls to it
to detect the problem in attributes alloc_align, alloc_size, and
section.  This isn't meant to be a comprehensive fix but rather
a starting point for one.

Tested on x86_64-linux.

Martin

PS I ran into this again while debugging some unrelated changes
and wondering about the behavior in similar situations to mine.
Since the behavior seemed clearly suboptimal I figured I might
as well fix it.

PPS The improved checking triggers warnings in a few calls to
__builtin_has_attribute due to apparent conflicts.  I've xfailed
those in the test since it's a known issue with some existing
attributes that should be fixed at some point.  Valid uses of
the built-in shouldn't trigger diagnostics except for completely
nonsensical arguments.  Unfortunately, the line between valid
and completely nonsensical is a blurry one (GCC either issues
errors, or -Wattributes, or silently ignores some cases
altogether, such as those that are the subject of this patch)
and there is no internal mechanism to control the response.








[Bug c++/96497] Compare std::variant with int using C++20 <=> is not a constant expression

2020-08-10 Thread jakub at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96497

--- Comment #4 from Jakub Jelinek  ---
Should be fixed for 11, I think we should backport to 10.3 too eventually.

[PING #4][PATCH] separate reading past the end from -Wstringop-overflow

2020-08-10 Thread Martin Sebor via Gcc-patches

Ping: https://gcc.gnu.org/pipermail/gcc-patches/2020-June/548786.html

On 7/26/20 11:42 AM, Martin Sebor wrote:

Ping: https://gcc.gnu.org/pipermail/gcc-patches/2020-June/548786.html

On 7/13/20 6:05 PM, Martin Sebor wrote:

Ping: https://gcc.gnu.org/pipermail/gcc-patches/2020-June/548786.html

On 7/7/20 12:56 PM, Martin Sebor wrote:

Ping.  Despite its size, there isn't much new in the patch, it
pretty much just splits an existing warning into two, one for
buffer overflow and another for "overread."

https://gcc.gnu.org/pipermail/gcc-patches/2020-June/548786.html

On 6/23/20 8:05 PM, Martin Sebor wrote:

-Wstringop-overflow is issued for both writing and reading past
the end, even though the latter problem is often viewed as being
lower severity than the former, or at least sufficiently different
to triage separately.  In CWE, for example, each of the two kinds
of problems has its own classification (CWE-787: Out-of-bounds
Write, and CWE-125: Out-of-bounds Read).

To make this easier with GCC, the attached patch introduces
a new option called -Wstringop-overread and splits the current
indiscriminate warning into the respective two categories.  Other
than the new option the improvements also exposed a few instances
of reading past the end that GCC doesn't detect that the new code
does thanks to more consistent checking.

As usual, I tested the patch by building Binutils/GDB, Glibc, and
the Linux kernel with no unexpected (or any, in fact) instances
of the two warnings.

The work involved more churn that I expected, and maintaining
the expected precedence of warnings (missing terminating nul,
excessive bounds, etc.) turned out to be more delicate and time
consuming than I would have liked.  I think the result is cleaner
code, but I'm quite sure it can still stand to be made better.
That's also my plan for the next step when I'd like to move this
checking out of builtins.c and calls.c and into a file of its own.
Ultimately, Jeff and have would like to integrate it into the path
isolation pass.

Martin

PS There's a FIXME comment in expand_builtin_memory_chk as
a reminder of a future change.  It currently has no bearing
on anything.










[PING][PATCH] improve memcmp and memchr constant folding (PR 78257)

2020-08-10 Thread Martin Sebor via Gcc-patches

Ping:
https://gcc.gnu.org/pipermail/gcc-patches/2020-July/551152.html

On 7/31/20 5:55 PM, Martin Sebor wrote:

The folders for these functions (and some others) call c_getsr
which relies on string_constant to return the representation of
constant strings.  Because the function doesn't handle constants
of other types, including aggregates, memcmp or memchr calls
involving those are not folded when they could be.

The attached patch extends the algorithm used by string_constant
to also handle constant aggregates involving elements or members
of the same types as native_encode_expr.  (The change restores
the empty initializer optimization inadvertently disabled in
the fix for pr96058.)

To avoid accidentally misusing either string_constant or c_getstr
with non-strings I have introduced a pair of new functions to get
the representation of those: byte_representation and getbyterep.

Tested on x86_64-linux.

Martin




[Bug fortran/96556] [11.0 regression] ICE via segmentation violation

2020-08-10 Thread dominiq at lps dot ens.fr
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96556

Dominique d'Humieres  changed:

   What|Removed |Added

 Ever confirmed|0   |1
 Status|UNCONFIRMED |NEW
   Last reconfirmed||2020-08-10
   Keywords||ice-on-valid-code
   Priority|P3  |P4
 CC||tkoenig at gcc dot gnu.org

--- Comment #4 from Dominique d'Humieres  ---
Likely r11-2578.

[Bug libstdc++/90704] [LWG 3430] filesystem::path overloads for file streams are not conforming

2020-08-10 Thread manx-bugzilla at problemloesungsmaschine dot de
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90704

--- Comment #5 from Jörn Heusipp  ---

> Status: SUSPENDED

Well, coming from bug 95642, which has been marked as a duplicate of this bug,
this is *very* disappointing.


The C++17 standard absolutely clearly specifies that the constructor is
overloaded for std::filesystem::path. libstdc++'s SFINAE implementation is
incompatible because it fails for user-defined types that are implicitly
convertible to std::filesystem::path.
In order to at all be able to sensibly use libstdc++ std::filesystem::path in
my codebase, I would have to put this awful work-around (making tons of
assumptions about libstdc++'s enable_if expression) into my own type
mpt::PathString:
+   // work-around for GCC bug
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95642 /
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90704
+   mpt::PathString & make_preferred()
+   {
+   *this =
mpt::PathString::FromFsPath(std::filesystem::path{path}.make_preferred());
+   return *this;
+   }
+   mpt::PathString filename() const
+   {
+   return
mpt::PathString::FromFsPath(std::filesystem::path{path}.filename());
+   }
+   std::filesystem::path c_str() const
+   {
+   return std::filesystem::path{path}; 
+   }
and just pray and hope that libstdc++ will not change its non-conforming
SFINAE-using implementation details to some other non-conforming implementation
details in some future version.
Can you guarantee, that you wont ever change your non-conforming SFINAE
implementation whatsoever at all? If not, please just fix it. The standard is
absolutely clear here.
If you insist on doing something non-standard-conforming because of
std::string_view, at the very least please do it for string_view only, without
breaking behaviour for std::filesystem::path. Compatibility with
std::experimental::filesystem::path can also be implemented without breaking
the standard-specified std::filesystem::path overloads.


The issue risen by https://cplusplus.github.io/LWG/issue3430 about the fear of
implicit allocation is not convincing to me. std::basic_fstream accesses the
filesystem, inducing all kinds of memory allocations, in-kernel blocking, and
disk I/O. Some additional user space allocations just do not matter here. None
of the call chain is specified noexcept either.
While the issue of desiring std::string_view overloads itself appears
reasonable to me, I fail to see why this issue should wait on that particular
standard change. Even if std::string_view overloads existed, the current
libstdc++ implementation would still be non-conforming for the
std::filesystem::path overload and would need to be changed in exactly the same
way as according to the current standard.


The suggestion of forcing std::filesystem::path arguments to be exactly
std::filesystem::path (i.e. making the current libstdc++ conforming by forcing
the bug on everyone else) breaks backwards compatibility for other (currently
conforming) implementations and IMO is also wrong on a higher level. There
exist legitimate and semantically sound reasons to have user-defined types that
are implicitly convertible to std::filesystem::path (std::filesystem::path's
greedy behaviour of silently doing enconding conversions from char (which may
be in locale encoding or utf8 in C++17, no way to tell) and thereby potentially
losing data, being one of them, making it impossible to use it safely by
itself). In the end, IMO std::filesystem::path should never have been
implicitly convertible from a string in the first place. *This* is where the
problems began. A string is not necessarily a path, which is why implicit
conversion was/is wrong here. I doubt that will ever change by now, though,
making it even more important to be able to wrap std::filesystem::path in an
encoding-type-safe user-defined type.


So, there are a couple of things wrong related to std::filesystem::path. The
only way to sanely use it, is by strictly adhering to the agreed-upon standard.
Delaying conformance with the standard until after some unrelated (to the
std::filesystem::path overload) problems are resolved in the standard makes
little sense to me, as it makes std::filesystem::path mostly unusable (without
extremely ugly work-arounds) in the meantime (which could easily be multiple
years) in code portable between different implementations.

libstdc++ needs to get fixed as soon as possible. It is currently blocking
progress in portable downstream users.


After having said all this, I am not even sure anymore that this bug report
here actually is a duplicate of my bug 95642. I am concerned about implicit
conversion to std::filesystem::path not working, while this bug report is
concerned about std::string_view not working. Please consider re-opening bug
95642.

[Bug fortran/96556] [11.0 regression] ICE via segmentation violation

2020-08-10 Thread juergen.reuter at desy dot de
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96556

--- Comment #3 from Jürgen Reuter  ---
Created attachment 49038
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49038=edit
Final reproducer, some 70 lines

[Bug c++/96557] Diagnostics: Can you tell me why it's not a constant expression?

2020-08-10 Thread mpolacek at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96557

Marek Polacek  changed:

   What|Removed |Added

 Status|UNCONFIRMED |NEW
 Ever confirmed|0   |1
   Keywords||diagnostic
 CC||mpolacek at gcc dot gnu.org
   Last reconfirmed||2020-08-10

--- Comment #1 from Marek Polacek  ---
That would be great to have.

[Bug testsuite/96525] new test gcc.target/powerpc/pr96493.c fails

2020-08-10 Thread seurer at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96525

--- Comment #6 from seurer at gcc dot gnu.org ---
That changed the test to unsupported on the p8 where it had failed previously.

Executing on host: /home/seurer/gcc/git/build/gcc-test/gcc/xgcc
-B/home/seurer/gcc/git/build/gcc-test/gcc/ power10_hw_available2538527.c   
-fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers
-fdiagnostics-color=never  -fdiagnostics-urls=never  -mcpu=power10  -lm  -o
power10_hw_available2538527.exe(timeout = 300)
spawn -ignore SIGHUP /home/seurer/gcc/git/build/gcc-test/gcc/xgcc
-B/home/seurer/gcc/git/build/gcc-test/gcc/ power10_hw_available2538527.c
-fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers
-fdiagnostics-color=never -fdiagnostics-urls=never -mcpu=power10 -lm -o
power10_hw_available2538527.exe
Assembler messages:
Error: invalid switch -mpower10
Error: unrecognized option -mpower10
compiler exited with status 1
Executing on host: /home/seurer/gcc/git/build/gcc-test/gcc/xgcc
-B/home/seurer/gcc/git/build/gcc-test/gcc/-fno-diagnostics-show-caret
-fno-diagnostics-show-line-numbers -fdiagnostics-color=never 
-fdiagnostics-urls=never  -c -o powerpc_elfv22538527.o powerpc_elfv22538527.c  
 (timeout = 300)
spawn -ignore SIGHUP /home/seurer/gcc/git/build/gcc-test/gcc/xgcc
-B/home/seurer/gcc/git/build/gcc-test/gcc/ -fno-diagnostics-show-caret
-fno-diagnostics-show-line-numbers -fdiagnostics-color=never
-fdiagnostics-urls=never -c -o powerpc_elfv22538527.o powerpc_elfv22538527.c
Executing on host: /home/seurer/gcc/git/build/gcc-test/gcc/xgcc
-B/home/seurer/gcc/git/build/gcc-test/gcc/-fno-diagnostics-show-caret
-fno-diagnostics-show-line-numbers -fdiagnostics-color=never 
-fdiagnostics-urls=never  -mcpu=power10 -c -o power10_ok2538527.o
power10_ok2538527.c(timeout = 300)
spawn -ignore SIGHUP /home/seurer/gcc/git/build/gcc-test/gcc/xgcc
-B/home/seurer/gcc/git/build/gcc-test/gcc/ -fno-diagnostics-show-caret
-fno-diagnostics-show-line-numbers -fdiagnostics-color=never
-fdiagnostics-urls=never -mcpu=power10 -c -o power10_ok2538527.o
power10_ok2538527.c
Assembler messages:
Error: invalid switch -mpower10
Error: unrecognized option -mpower10
compiler exited with status 1
UNSUPPORTED: gcc.target/powerpc/pr96493.c

. . .

=== gcc Summary ===

# of unsupported tests  1

Re: PING [Patch][Middle-end]Add -fzero-call-used-regs=[skip|used-gpr|all-gpr|used|all]

2020-08-10 Thread Qing Zhao via Gcc-patches
Hi, 

> On Aug 7, 2020, at 5:59 PM, Segher Boessenkool  
> wrote:
> 
>> From my understanding (I am not a security expert though), this patch should 
>> serve two purpose:
>> 
>> 1. Erase the registers upon return to avoid information leak;
> 
> But only some of the registers.

All the call-used registers could be erased upon return with 
-fzero-call-used-regs=all.
> 
>> 2. ROP mitigation, for details on this, please refer to paper:
>> 
>> "Clean the Scratch Registers: A Way to Mitigate Return-Oriented Programming 
>> Attacks"
>> 
>> https://ieeexplore.ieee.org/document/8445132 
>> 
> 
> Do you have a link to this that people can actually read?

Sorry, I cannot find a free copy online. Looks like that I can only read the 
whole paper through ieee. ( I read the PDF file
through our company’s account).

> 
>> From the above paper, The call-used registers are used by the ROP hackers as 
>> following:
>> 
>> "Based on the practical experience of reading and writing ROP code. we find 
>> the features of ROP attacks as follows.
>> 
>> First, the destination of using gadget chains in usual is performing system 
>> call or system function to perform 
>> malicious behaviour such as file access, network access and W ⊕ X disable. 
>> In most cases, the adversary
>> would like to disable W ⊕ X.
> 
> That makes things easier, for sure, but is just a nicety really.
> 
>> Because once W ⊕ X has been disabled, shellcode can be executed directly
>> instead of rewritting shellcode to ROP chains which may cause some troubles 
>> for the adversary. In upper 
>> example, the system call is number 59 which is “execve” system call.
>> 
>> Second, if the adversary performs ROP attacks using system call instruction, 
>> no matter on x86 or x64 
>> architecture, the register would be used to pass parameter. Or if the 
>> adversary performs ROP attacks 
>> using system function such as “read” or “mprotect”, on x64 system, the 
>> register would still be used to 
>> pass parameters, as mentioned in subsection B and C.”
>> 
>> We can see that call-used registers might be used by the ROP hackers to pass 
>> parameters to the system call.
>> If compiler can clean these registers before routine “return", then ROP 
>> attack will be invalid. 
> 
> So the idea is that clearing (or otherwise interfering with) the registers
> used for parameter passing makes making useful ROP chains harder?

Yes, that’s my understanding.

> 
>> Yes, there will be performance overhead from adding these register wiping 
>> insns. However, it’s necessary to
>> add overhead for security purpose.
> 
> The point is the balance between how expensive it is, vs. how much it
> makes it harder to exploit the code.
> 
> But of course any user can make that judgment themselves.  For us it
> mostly matters what the cost is to targets that use it, to targets that
> do not use it, and to the generic code, vs. what value we give to our
> users :-)

We need to minimize the performance overhead during the implementation. 
At the same time, provide users options to minimize the overhead at the same 
time (for example the function level 
attribute, and the different level of zeros).

> 
>>> "call-used" is such a bad name.  "call-clobbered" is better already, but
>>> "volatile" (over calls) is most obvious I think.
>> 
>> In our GCC compiler source code, we used the name “call-used” a lot, of 
>> course, “call-clobbered” is
>> also used frequently.  Do these names refer to the same set of registers, 
>> i.e, the register set that  
>> will be corrupted by function call?
> 
> Anything that isn't "call-saved" or "fixed" is called "call-used",
> essentially.  (And the relation with "fixed" isn't always clear).
> 
>> If so, I am okay with name “call-clobbered” if this name sounds better. 
> 
> It's more obvious, at least to me.

Okay. 

> 
>>> There are at least four different kinds of volatile registers:
>>> 
>>> 1) Argument registers are volatile, on most ABIs.
>> These are the registers that  need to be cleaned up upon function return for 
>> the ROP mitigation described in the paper
>> mentioned above.
>> 
>>> 2) The *linker* (or dynamic linker!) may insert code that needs some
>>>  registers for itself;
>>> 3) Registers only used for scratch space;
>>> 4) Registers used for returning the function value.
>> 
>> I think that the above 1,3,4 should be all covered by “call_used_regs”. 
> 
> 1 and 4 are the *same* (or overlap) on most ABIs.  3 can be as well, it
> depends what the compiler is allowed to do; normally, if the compiler
> wants a register, the parameter passing regs are among the cheapest it
> can use.
So, are theyall covered by “call_used_reg” in GCC? 

> 2 you cannot touch usefully at all, for your purposes.
Okay.
> 
>> Not sure about 2, could you explain a little bit more on 2 (The linker may 
>> insert code that needs some register for itself)? 
> 
> Sure.  The linker can decide it needs to insert some code to restore a

Re: [PATCH] Rewrite get_size_range for irange API.

2020-08-10 Thread Martin Sebor via Gcc-patches

On 8/10/20 5:47 AM, Aldy Hernandez wrote:



On 8/6/20 9:30 PM, Martin Sebor wrote:

On 8/6/20 8:53 AM, Aldy Hernandez via Gcc-patches wrote:



+  // Remove the unknown parts of a multi-range.
+  // This will transform [5,10][20,MAX] into [5,10].


Is this comment correct?  Wouldn't this result in returning smaller
sizes than the actual value allows?  If so, I'd expect that to cause
false positives (and in that case, if none of our tests fail we need
to add some that would).

By my reading of the code below it seems to return the upper range
(i.e., [20, MAX]) but I'm not fully acquainted with the new ranger
APIs yet.


I believe the comment is correct.  What I'm trying to do is remove 
[X,TYPE_MAX_VALUE] from the range



+  int pairs = positives.num_pairs ();
+  if (pairs > 1
+  && positives.upper_bound () == wi::to_wide (TYPE_MAX_VALUE 
(exptype)))

  {
-  if (integral)
-    {
-  /* Use the full range of the type of the expression when
- no value range information is available.  */
-  range[0] = TYPE_MIN_VALUE (exptype);
-  range[1] = TYPE_MAX_VALUE (exptype);
-  return true;
-    }
-
-  range[0] = NULL_TREE;
-  range[1] = NULL_TREE;
-  return false;
+  value_range last_range (exptype,
+  positives.lower_bound (pairs - 1),
+  positives.upper_bound (pairs - 1), VR_ANTI_RANGE);
+  positives.intersect (last_range);
  }


Notice that we start with positives == vr (where vr is the range 
returned by determine_value_range).


Then we build last_range which is the *opposite* of [X,TYPE_MAX_VALUE]. 
Notice the VR_ANTI_RANGE in the constructor.


(Note that the nomenclature VR_ANTI_RANGE is being used in the 
constructor to keep with the original API.  It means "create the inverse 
of this range".  Ideally, when the m_kind field disappears along with 
VR_ANTI_RANGEs, the enum could become RANGE_INVERSE or something less 
confusing.)


Finally, last_range is intersected with positives, which will become 
whatever VR had, minus the last sub-range.


Those that make sense, or did I miss something?

If you have a testcase that yields a false positive in my proposed 
patch, but not in the original code, please let me know so I can adjust 
the patch.


Sorry, I misremembered what get_size_range was used for.  It's used
to constrain the maximum size of memory accesses by functions like
memcpy (the third argument).  The use case I was thinking of was
to determine the bounds of the size of variably modified objects
(like VLAs).  It doesn't look like it's used for that.

With the current use taking the lower bound is the conservative
thing to do and using the upper bound would cause false positives.
With the latter it would be the other way around.  So the comment
makes sense to me now.

As an aside, here's a test case I played with.  I'd expect it to
trigger a warning because the upper bound of the range of array's
size is less than the lower bound of the range of the number of
bytes being written to it.

void f (void*);

void g (unsigned n1, unsigned n2)
{
  if (!(n1 >= 2 && n1 <= 3))   // n1 = [2, 3]
return;

  char a[n1];

  if (!((n2 >= 5 && n2 <= 10)
|| (n2 >= 20)))// n2 = [5, 10] U [20, UINT_MAX]
return;

  __builtin_memset (a, 0, n2);
  f (a);
}

But I couldn't get it to either produce a multipart range, or
a union of the two parts (i.e., [5, UINT_MAX]).  It makes me
wonder what I'm missing about how such ranges are supposed to
come into existence.

Martin


[Bug fortran/96101] [9/10/11 Regression] ICE in fold_convert_loc, at fold-const.c:2398

2020-08-10 Thread dominiq at lps dot ens.fr
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96101

Dominique d'Humieres  changed:

   What|Removed |Added

 Status|NEW |ASSIGNED

--- Comment #4 from Dominique d'Humieres  ---
The patch works as expected and even fixes pr91862.

[Bug c++/96557] New: Diagnostics: Can you tell me why it's not a constant expression?

2020-08-10 Thread barry.revzin at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96557

Bug ID: 96557
   Summary: Diagnostics: Can you tell me why it's not a constant
expression?
   Product: gcc
   Version: 10.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c++
  Assignee: unassigned at gcc dot gnu.org
  Reporter: barry.revzin at gmail dot com
  Target Milestone: ---

Consider:

struct X {
char storage[100] = {};
char const* head = storage;
};

void f() {
constexpr X x = {};
}

gcc correctly rejects this with:

error: 'X{"", ((const char*)(& x.X::storage))}' is not a constant expression
7 | constexpr X x = {};
  |  ^

It'd be great if the error here actually indicated _why_ it's not a constant
expression (or how to fix it). clang does a little bit better:

:7:17: error: constexpr variable 'x' must be initialized by a constant
expression
constexpr X x = {};
^   ~~
:7:17: note: pointer to subobject of 'x' is not a constant expression
:7:17: note: declared here

But ideally we get some message about specifically 'head' pointing to 'storage'
and a fixup suggesting making the variable x static.

Re: [PATCH] emit-rtl.c: Allow vector subreg of float vectors

2020-08-10 Thread Richard Sandiford
Andrew Stubbs  writes:
> On 06/08/2020 04:54, Richard Sandiford wrote:
>>> diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
>>> index f9b0e9714d9..d7067989ad7 100644
>>> --- a/gcc/emit-rtl.c
>>> +++ b/gcc/emit-rtl.c
>>> @@ -947,6 +947,11 @@ validate_subreg (machine_mode omode, machine_mode 
>>> imode,
>>> else if (VECTOR_MODE_P (omode)
>>>&& GET_MODE_INNER (omode) == GET_MODE_INNER (imode))
>>>   ;
>>> +  /* Allow sections of vectors, both smaller and paradoxical.  (This just
>>> + works for integers, but needs to be explicitly allowed for floats.)  
>>> */
>>> +  else if (VECTOR_MODE_P (omode) && VECTOR_MODE_P (imode)
>>> +  && GET_MODE_INNER (omode) == GET_MODE_INNER (imode))
>>> +;
>> 
>> Might be missing something, but isn't this a subcondition of the
>> previous “else if”?  It looks like that ought to catch every case
>> that the new one does.
>
> Apparently, Hongtao and Uroš fixed my problem while I was working on this.
>
> Yes, my patch does the same (although I would question whether it's safe 
> to use "GET_MODE_INNER (imode)" without having first confirmed "imode" 
> is a vector mode).

FWIW, skipping the VECTOR_MODE_P test means that we also allow vector
paradoxical subregs of scalars, which can be useful if you're trying
to make a vector in which only element 0 is initialised.  (Only works
for element 0 though.)  IIRC this is what some of the x86 patterns do.

Guess it means we also allow vector subregs of complex values,
but that kind-of seems OK/useful too.

Thanks,
Richard


Re: [PATCH] emit-rtl.c: Allow splitting of RTX_FRAME_RELATED_P insns?

2020-08-10 Thread Richard Sandiford
Senthil Kumar via Gcc-patches  writes:
> Hi,
>
>   I'm working on converting the AVR backend to MODE_CC, following
>   the steps described for case #2 in the CC0 transition wiki page,
>   and I've implemented the first three bullet
>   points (https://github.com/saaadhu/gcc-avr-cc0/tree/avr-cc0-squashed). With
>   the below patch, there are zero regressions (for mega and xmega
>   subarchs) compared to the current mainline, as of yesterday.
>
>   The wiki suggests using post-reload splitters, so that's the
>   direction I took, but I ran into an issue where split_insn
>   bails out early if RTX_FRAME_RELATED_P is true - this means
>   that splits for REG_CC clobbering insns with
>   RTX_FRAME_RELATED_P will never execute, resulting in a
>   could-not-split insn ICE in the final stage.
>
>   I see that the recog.c:peep2_attempt allows splitting of a
>   RTX_FRAME_RELATED_P insn, provided the result of the split is a
>   single insn. Would it be ok to modify try_split also to
>   allow those kinds of insns (tentative patch attached, code
>   copied over from peep2_attempt, only setting old and new_insn)? Or is there
>   a different approach to fix this?

I agree there's no obvious reason why splitting to a single insn
should be rejected but a peephole2 to a single instruction should be OK.
And reusing the existing, tried-and-tested code is the way to go.

But could you split the code out of peep2_attempt into a subroutine
(probably still in recog.c) and reuse it in try_split?

BTW, just to check: is your email address in MAINTAINERS still correct?

Thanks,
Richard


[PATCH] c++: premature analysis of requires-expression [PR96410]

2020-08-10 Thread Patrick Palka via Gcc-patches
In the below testcase, semantic analysis of the requires-expressions in
the generic lambda must be delayed until instantiation of the lambda
because the requirements depend on the lambda's template arguments.  But
tsubst_requires_expr does semantic analysis even during regeneration of
the lambda, which leads to various bogus errors and ICEs since some
subroutines aren't prepared to handle dependent/template trees.

This patch adjusts subroutines of tsubst_requires_expr to avoid doing
some problematic semantic analyses when processing_template_decl.
In particular, expr_noexcept_p generally can't be checked on a dependent
expression.  Next, tsubst_nested_requirement should avoid checking
satisfaction when processing_template_decl.  And similarly for
convert_to_void (called from tsubst_valid_expression_requirement).

Bootstrapped and regtested on x86_64-pc-linux-gnu, and also tested
against the cmcstl2 project.  Does this look OK to commit?

gcc/cp/ChangeLog:

PR c++/96409
PR c++/96410
* constraint.cc (tsubst_compound_requirement): When
processing_template_decl, don't check noexcept of the
substituted expression.
(tsubst_nested_requirement): Just substitute into the constraint
when processing_template_decl.
* cvt.c (convert_to_void): Don't resolve concept checks when
processing_template_decl.

gcc/testsuite/ChangeLog:

PR c++/96409
PR c++/96410
* g++.dg/cpp2a/concepts-lambda13.C: New test.
---
 gcc/cp/constraint.cc  |  9 ++-
 gcc/cp/cvt.c  |  2 +-
 .../g++.dg/cpp2a/concepts-lambda13.C  | 25 +++
 3 files changed, 34 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index e4aace596e7..db2036502a7 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -1993,7 +1993,8 @@ tsubst_compound_requirement (tree t, tree args, 
subst_info info)
 
   /* Check the noexcept condition.  */
   bool noexcept_p = COMPOUND_REQ_NOEXCEPT_P (t);
-  if (noexcept_p && !expr_noexcept_p (expr, tf_none))
+  if (!processing_template_decl
+  && noexcept_p && !expr_noexcept_p (expr, tf_none))
 return error_mark_node;
 
   /* Substitute through the type expression, if any.  */
@@ -2023,6 +2024,12 @@ static tree
 tsubst_nested_requirement (tree t, tree args, subst_info info)
 {
   /* Ensure that we're in an evaluation context prior to satisfaction.  */
+  if (processing_template_decl)
+{
+  tree r = tsubst_constraint (TREE_OPERAND (t, 0), args,
+ info.complain, info.in_decl);
+  return finish_nested_requirement (EXPR_LOCATION (t), r);
+}
   tree norm = TREE_TYPE (t);
   tree result = satisfy_constraint (norm, args, info);
   if (result == error_mark_node && info.quiet ())
diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c
index c9e7b1ff044..1d2c2d3351c 100644
--- a/gcc/cp/cvt.c
+++ b/gcc/cp/cvt.c
@@ -1159,7 +1159,7 @@ convert_to_void (tree expr, impl_conv_void implicit, 
tsubst_flags_t complain)
 
   /* Explicitly evaluate void-converted concept checks since their
  satisfaction may produce ill-formed programs.  */
-   if (concept_check_p (expr))
+   if (!processing_template_decl && concept_check_p (expr))
  expr = evaluate_concept_check (expr, tf_warning_or_error);
 
   if (VOID_TYPE_P (TREE_TYPE (expr)))
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C 
b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C
new file mode 100644
index 000..9757ce49d67
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C
@@ -0,0 +1,25 @@
+// PR c++/96410
+// { dg-do compile { target c++20 } }
+
+struct S { using blah = void; };
+
+template  constexpr bool trait = !__is_same(T, S);
+template  concept C = trait;
+
+template
+void foo() noexcept(!__is_same(T, void)) { }
+
+template
+auto f() {
+  return [](T){
+static_assert(requires { requires C && (C || C); }); // { 
dg-error "assert" }
+static_assert(requires { C; });
+static_assert(requires { { foo() } noexcept -> C; });
+static_assert(!requires { typename T::blah; }); // { dg-error "assert" }
+return 0;
+  };
+}
+
+auto g = f(); // { dg-bogus "" }
+int n = g(0); // { dg-bogus "" }
+int m = g(S{}); // { dg-message "required from here" }
-- 
2.28.0.97.gdc04167d37



Re: [PATCH] rs6000: Disable -fcaller-saves by default

2020-08-10 Thread Peter Bergner via Gcc-patches
On 7/23/20 3:29 PM, Jeff Law wrote:
>>> What's driving this change?
>>
>> Peter noticed IRA allocates stuff to volatile registers while it is life
>> through a call, and then LRA has to correct that, not optimal.
> I can't recall if IRA or LRA handles the need to save them to the stack, but 
> the
> whole point is you can do much better sometimes by saving into the stack with 
> the
> caller-saves algorithm vs just giving up and spilling.
> 
>>
>>> IRA will do a cost/benefit analysis to see using call clobbered registers 
>>> like
>>> this is profitable or not.  You're just turning the whole thing off.


Sorry for taking so long to reply.  I'm the guilty party for asking Pat
to submit the patch. :-)  I was not aware IRA did that and just assumed
it was a bug.  For now, consider the patch withdrawn.  That said, I
will have to look at that cost computation, especially since when I
last looked, IRA does not count potential prologue/epilogue save/restore
insns if it were to assign a non-volatile register when computing reg
costs.  That would seem to be an important consideration here.

I'll note this all came about when I was looking at PR96017, which is
due to not shrink-wrapping a pseudo.  That was due to it being live
across a call.  I first I thought (for the 2nd test case, not the original
one) split_live_ranges_for_shrink_wrap() was nicely splitting the pseudo
for us, but it must have been the caller-saves algorithm you mention above.
However, that code doesn't handle the original test case, which I think
it should.

My thought for that bug was to introduce some extra splitting before RA
(maybe as part of split_live_ranges_for_shrink_wrap?) where we split
pseudos that are live across a call, but that have at least one path
to the exit that doesn't cross a call.  However, if we can beef up
the caller-saves cost computation, maybe that would work too?


Peter


[Bug c/96550] gcc is smart in figuring out a non-returning function.

2020-08-10 Thread redi at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96550

--- Comment #16 from Jonathan Wakely  ---
When you choose RESOLVED you can pick various types of resolution, FIXED,
INVALID, DUPLICATE, MOVED, WORKSFORME etc.

Re: RFA: Fix combine.c combining a move and a non-move into two non-moves, PR93372

2020-08-10 Thread Hans-Peter Nilsson via Gcc-patches
> From: Segher Boessenkool 
> Date: Fri, 17 Jul 2020 02:05:19 +0200

> On Tue, Jul 14, 2020 at 04:33:42PM -0500, Segher Boessenkool wrote:
> > > If combine only did lower-cost combinations (perhaps with
> > > Richard Sandifords lower-size-when-tied suggestion), I guess
> > > this wouldn't happen. 0:-)
> > 
> > And we would regress (a LOT).
>
> Like this.  C0 is an unmodified compiler.  C1 is with the single_set
> modification to is_just_move I committed a minute ago (84c5396d4bdb).
> C2 is with this patch:

Thanks for doing the numbers.

Most targets are of course keyed on the cheaper=same_cost
behaviour of combine.c.  Just try fixing any obvious general
cost-related bug!  I mentioned running the test-suites for each
of x86_64-linux, aarch64-linux and ppc64le-linux (your numbers
confirm my observation) but didn't mention that I *did* inspect
one case, IIRC the first regression for x86_64, but decided to
break off less than half-way down the rabbit-hole with no
obvious target-cost tweak.

I'm considering updating my suggested patch with something
implementing "INSN_COST(log_link) = 1", but don't hold your
breath.

brgds, H-P


[Bug c++/96543] null check on template pointer parameter fails

2020-08-10 Thread redi at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96543

--- Comment #3 from Jonathan Wakely  ---
No, it's fine. I've categorized it as a diagnostic bug, i.e. a bug in a
warning.

[Bug libstdc++/94681] filesystem::sysmlink_status using stat instead of lstat when --disable-libstdcxx-filesystem-ts

2020-08-10 Thread redi at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94681

--- Comment #6 from Jonathan Wakely  ---
Thanks, I thought this might reveal some new issues :-)

I'll fix it asap.

[Bug fortran/96556] [11.0 regression] ICE via segmentation violation

2020-08-10 Thread juergen.reuter at desy dot de
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96556

--- Comment #2 from Jürgen Reuter  ---
Created attachment 49037
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49037=edit
2nd reproducer, single file, shortening further

[Bug c++/96497] Compare std::variant with int using C++20 <=> is not a constant expression

2020-08-10 Thread cvs-commit at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96497

--- Comment #3 from CVS Commits  ---
The master branch has been updated by Jakub Jelinek :

https://gcc.gnu.org/g:5c64df80df274c753bfc8415bd902e1180e76f6a

commit r11-2635-g5c64df80df274c753bfc8415bd902e1180e76f6a
Author: Jakub Jelinek 
Date:   Mon Aug 10 17:53:46 2020 +0200

c++: Fix constexpr evaluation of SPACESHIP_EXPR [PR96497]

The following valid testcase is rejected, because
cxx_eval_binary_expression
is called on the SPACESHIP_EXPR with lval = true, as the address of the
spaceship needs to be passed to a method call.
After recursing on the operands and calling genericize_spaceship which
turns
it into a TARGET_EXPR with initialization, we call
cxx_eval_constant_expression
on it which succeeds, but then we fall through into code that will
VERIFY_CONSTANT (r) which FAILs because it is an address of a variable. 
Rather
than avoiding that for lval = true and SPACESHIP_EXPR, the patch just tail
calls cxx_eval_constant_expression - I believe that call should perform all
the needed verifications.

2020-08-10  Jakub Jelinek  

PR c++/96497
* constexpr.c (cxx_eval_binary_expression): For SPACESHIP_EXPR,
tail
call cxx_eval_constant_expression after genericize_spaceship to
avoid
undesirable further VERIFY_CONSTANT.

* g++.dg/cpp2a/spaceship-constexpr3.C: New test.

[Bug tree-optimization/96554] -Wall does not include -Wnull-dereference

2020-08-10 Thread msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96554

Martin Sebor  changed:

   What|Removed |Added

 CC||msebor at gcc dot gnu.org
   Last reconfirmed||2020-08-10
  Component|c   |tree-optimization
 Blocks||86172
 Status|UNCONFIRMED |NEW
 Ever confirmed|0   |1

--- Comment #5 from Martin Sebor  ---
The latest trunk of GCC 11 has over 700 distinct instances of the
-Wnull-dereference warning so some work is still needed before it can be
enabled in either -Wextra or -Wall.  Here's the breakdown:

DiagnosticCount   UniqueFiles
-Wnull-dereference 3862  713  168
-Wimplicit-fallthrough=   411
-Wstringop-truncation 311
-Wreturn-local-addr   211
-Wmaybe-uninitialized 222

-Wnull-dereference Instances:
  cc1plus: warning: potential null pointer dereference [-Wnull-dereference]
  /src/gcc/trunk/gcc/analyzer/checker-path.cc:737
  /src/gcc/trunk/gcc/analyzer/checker-path.cc:745
  /src/gcc/trunk/gcc/analyzer/checker-path.cc:958
  /src/gcc/trunk/gcc/analyzer/diagnostic-manager.cc:339
  /src/gcc/trunk/gcc/analyzer/diagnostic-manager.cc:456
  /src/gcc/trunk/gcc/analyzer/diagnostic-manager.cc:777
  /src/gcc/trunk/gcc/analyzer/diagnostic-manager.cc:778
  /src/gcc/trunk/gcc/analyzer/engine.cc:1844
  /src/gcc/trunk/gcc/analyzer/engine.cc:2683
  /src/gcc/trunk/gcc/analyzer/engine.cc:3570
  /src/gcc/trunk/gcc/analyzer/engine.cc:3656
  /src/gcc/trunk/gcc/analyzer/engine.cc:834
  /src/gcc/trunk/gcc/analyzer/exploded-graph.h:263
  /src/gcc/trunk/gcc/analyzer/program-point.h:225
  /src/gcc/trunk/gcc/analyzer/program-state.cc:134
  /src/gcc/trunk/gcc/analyzer/program-state.cc:241
  /src/gcc/trunk/gcc/analyzer/program-state.cc:266
  /src/gcc/trunk/gcc/analyzer/program-state.cc:267
  /src/gcc/trunk/gcc/analyzer/program-state.cc:472
  /src/gcc/trunk/gcc/analyzer/program-state.cc:473
  /src/gcc/trunk/gcc/analyzer/program-state.cc:488
  /src/gcc/trunk/gcc/analyzer/program-state.cc:489
  /src/gcc/trunk/gcc/analyzer/program-state.cc:514
  /src/gcc/trunk/gcc/analyzer/program-state.cc:584
  /src/gcc/trunk/gcc/analyzer/program-state.cc:585
  /src/gcc/trunk/gcc/analyzer/region-model.cc:1486
  /src/gcc/trunk/gcc/analyzer/region-model.cc:1571
  /src/gcc/trunk/gcc/analyzer/region-model.cc:2126
  /src/gcc/trunk/gcc/analyzer/region-model.cc:2606
  /src/gcc/trunk/gcc/analyzer/region-model.cc:424
  /src/gcc/trunk/gcc/analyzer/region-model.h:1195
  /src/gcc/trunk/gcc/analyzer/region-model.h:121
  /src/gcc/trunk/gcc/analyzer/region-model.h:129
  /src/gcc/trunk/gcc/analyzer/region-model.h:1583
  /src/gcc/trunk/gcc/analyzer/region-model.h:1587
  /src/gcc/trunk/gcc/analyzer/region-model.h:484
  /src/gcc/trunk/gcc/analyzer/region-model.h:869
  /src/gcc/trunk/gcc/analyzer/region-model.h:882
  /src/gcc/trunk/gcc/analyzer/region-model.h:912
  /src/gcc/trunk/gcc/analyzer/state-purge.h:95
  /src/gcc/trunk/gcc/analyzer/supergraph.cc:177
  /src/gcc/trunk/gcc/analyzer/supergraph.cc:198
  /src/gcc/trunk/gcc/analyzer/supergraph.cc:221
  /src/gcc/trunk/gcc/analyzer/supergraph.cc:240
  /src/gcc/trunk/gcc/analyzer/supergraph.h:106
  /src/gcc/trunk/gcc/analyzer/supergraph.h:126
  /src/gcc/trunk/gcc/analyzer/supergraph.h:132
  /src/gcc/trunk/gcc/analyzer/supergraph.h:138
  /src/gcc/trunk/gcc/analyzer/supergraph.h:144
  /src/gcc/trunk/gcc/analyzer/supergraph.h:163
  /src/gcc/trunk/gcc/auto-profile.c:1517
  /src/gcc/trunk/gcc/bb-reorder.c:467
  /src/gcc/trunk/gcc/bitmap.h:393
  /src/gcc/trunk/gcc/bitmap.h:532
  /src/gcc/trunk/gcc/bitmap.h:576
  /src/gcc/trunk/gcc/bitmap.h:579
  /src/gcc/trunk/gcc/caller-save.c:906
  /src/gcc/trunk/gcc/calls.c:1960
  /src/gcc/trunk/gcc/c/c-parser.c:4442
  /src/gcc/trunk/gcc/c-family/c-ada-spec.c:1905
  /src/gcc/trunk/gcc/c-family/c-attribs.c:3906
  /src/gcc/trunk/gcc/c-family/c-common.c:721
  /src/gcc/trunk/gcc/cfg.c:338
  /src/gcc/trunk/gcc/cfgcleanup.c:2143
  /src/gcc/trunk/gcc/cfgcleanup.c:2409
  /src/gcc/trunk/gcc/cfgloop.c:332
  /src/gcc/trunk/gcc/cfgloop.h:532
  /src/gcc/trunk/gcc/cfgloop.h:541
  /src/gcc/trunk/gcc/cfgrtl.c:1170
  /src/gcc/trunk/gcc/cfgrtl.c:4358
  /src/gcc/trunk/gcc/cfgrtl.c:443
  /src/gcc/trunk/gcc/cfgrtl.c:530
  /src/gcc/trunk/gcc/cgraphbuild.c:403
  /src/gcc/trunk/gcc/cgraph.c:1971
  /src/gcc/trunk/gcc/cgraph.c:2003
  /src/gcc/trunk/gcc/cgraph.c:3089
  /src/gcc/trunk/gcc/cgraph.c:3298
  /src/gcc/trunk/gcc/cgraph.c:3848
  /src/gcc/trunk/gcc/cgraph.c:3875
  /src/gcc/trunk/gcc/cgraphclones.c:186
  /src/gcc/trunk/gcc/cgraphclones.c:259
  /src/gcc/trunk/gcc/cgraph.h:1350
  /src/gcc/trunk/gcc/cgraph.h:1356
  /src/gcc/trunk/gcc/cgraph.h:2189
  

Re: RFC: make combine do as advertised (cheaper-than)?

2020-08-10 Thread Hans-Peter Nilsson via Gcc-patches
(Back from vacation, found that this had an unanswered question,
quoted last.)

> From: Segher Boessenkool 
> Date: Tue, 14 Jul 2020 23:58:05 +0200

> Hi!
> 
> On Mon, Jul 06, 2020 at 04:01:54AM +0200, Hans-Peter Nilsson via Gcc-patches 
> wrote:
> > Most comments, including the second sentence in the head comment
> > of combine_validate_cost, the main decision-maker of the combine
> > pass, refer to the function as returning true if the new
> > insns(s) *cheaper* than the old insns, when in fact the function
> > returned true also if the cost was the same.  Returning true for
> > cheaper also seems more sane than as-cheap-as considering the
> > need to avoid oscillation between same-cost combinations.  Also,
> > it makes the job of later passes harder, having combine make
> > more complex combinations of the same cost.
> 
> All of this was added in https://gcc.gnu.org/g:64b8935d4809 , which also
> adds the
> 
> +  /* Disallow this recombination if both new_cost and old_cost are
> + greater than zero, and new_cost is greater than old cost.  */
> 
> comment (which is what it actually does).  Before that change, combine
> didn't look at costs at all.
> 
> Combine never has used a "strictly smaller than" cost thing.

(I suggest we use terms of generally greater/lower cost, instead
of smaller/greater cost, to avoid confusion whether "smaller"
refers to the general cost or just code size.  JFTR: yes, I know
which one is considered depends on use of -Os and equivalents -
currently. :)

Right, we both looked at the history of combine.c.  The idea of
*cost* "cheaper than" (I read it as "lower than") was introduced
with the comments and documentation misleadingly saying "are
cheaper than", and it was in that same commit.  There's at least
two of them, not counting various comments mentioning various
sub-costs:

+/* Subroutine of try_combine.  Determine whether the combine replacement
+   patterns NEWPAT and NEWI2PAT are cheaper according to combine_insn_cost
+   that the original instruction sequence I1, I2 and I3.  Note that I1

+  /* Only allow this combination if combine_insn_costs reports that the
+ replacement instructions are cheaper than the originals.  */

> > Right, you can affect this with your target TARGET_RTX_COSTS and
> > TARGET_INSN_COST hooks, but only for trivial cases, and you have
> > increasingly more complex combinations (many-to-many
> > combinations) where you have to twist simple logic to appease
> > combine (stop it from combining) or give up.
> 
> There are 2-1, 3-1, 4-1, 3-2, 4-2, which all reduce the number of insns,
> and then there is 2-2, which gets rid of one log_link.  If the isnn_cost
> stays the same, it always wins something else.

That "something else" is presumptious.  A longer
dependency-chain may sum up to faster execution considering
parallel or OoO execution.  Someone adding another N-M
combination will notice, after two more years of work. :)

Perhaps a log_links dependency can be turned into
INSN_COST-based metric, by using a target hook defaulting to 1?

> > Alternatives from the top of my head, one of:
> 
> ...
> 
> 5) Improve your target so that its insn_cost reflects ithe costs of
> the insns better.

I see cheap gnawing, I hope I didn't add any of that myself.
I already covered target costs before the 1..4 list and you
actually quoted that (the quoted paragraph above your log_links
comment).

> Can you share some typical examples where things are worse with the
> current behaviour?

To recap, it all began with the observation of comments in
combine.c saying "cheaper than", with the code implementing
"same or cheaper", together with the flaw fixed by 84c5396d4bdbf
which I belive is a sufficient conclusion for that specific
instance.  A lower cost also seems more consistent with other
decisions.  (BTW, the commit is a bit misleading; I believe all
"case #2" cc0 convertees will benefit from an accurate
is_just_move, not just CRIS.)

The fixed 2-2 case is all the "typical examples" I had so far.
Remaining suggestions are just fixing perceived inconsistencies.

brgds, H-P


Re: [PATCH] c++; Fix constexpr evaluation of SPACESHIP_EXPR [PR96497]

2020-08-10 Thread Jason Merrill via Gcc-patches

On 8/8/20 5:23 AM, Jakub Jelinek wrote:

Hi!

The following valid testcase is rejected, because cxx_eval_binary_expression
is called on the SPACESHIP_EXPR with lval = true, as the address of the
spaceship needs to be passed to a method call.
After recursing on the operands and calling genericize_spaceship which turns
it into a TARGET_EXPR with initialization, we call cxx_eval_constant_expression
on it which succeeds, but then we fall through into code that will
VERIFY_CONSTANT (r) which FAILs because it is an address of a variable.  Rather
than avoiding that for lval = true and SPACESHIP_EXPR, the patch just tail
calls cxx_eval_constant_expression - I believe that call should perform all
the needed verifications.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk (and
later for 10.3)?


OK.


2020-08-08  Jakub Jelinek  

PR c++/96497
* constexpr.c (cxx_eval_binary_expression): For SPACESHIP_EXPR, tail
call cxx_eval_constant_expression after genericize_spaceship to avoid
undesirable further VERIFY_CONSTANT.

* g++.dg/cpp2a/spaceship-constexpr3.C: New test.

--- gcc/cp/constexpr.c.jj   2020-08-06 11:35:12.997122440 +0200
+++ gcc/cp/constexpr.c  2020-08-07 13:30:28.732376843 +0200
@@ -3085,8 +3085,8 @@ cxx_eval_binary_expression (const conste
else if (code == SPACESHIP_EXPR)
  {
r = genericize_spaceship (type, lhs, rhs);
-  r = cxx_eval_constant_expression (ctx, r, lval, non_constant_p,
-   overflow_p);
+  return cxx_eval_constant_expression (ctx, r, lval, non_constant_p,
+  overflow_p);
  }
  
if (r == NULL_TREE)

--- gcc/testsuite/g++.dg/cpp2a/spaceship-constexpr3.C.jj2020-08-07 
13:37:34.883410112 +0200
+++ gcc/testsuite/g++.dg/cpp2a/spaceship-constexpr3.C   2020-08-07 
13:38:09.988918586 +0200
@@ -0,0 +1,7 @@
+// PR c++/96497
+// { dg-do compile { target c++20 } }
+
+#include 
+
+static_assert(std::partial_ordering(std::strong_ordering::less) < 0);
+static_assert(std::partial_ordering(1 <=> 2) < 0);

Jakub





Re: [PATCH] arm: Clear canary value after stack_protect_test [PR96191]

2020-08-10 Thread Richard Sandiford
Christophe Lyon  writes:
> On Wed, 5 Aug 2020 at 16:33, Richard Sandiford
>  wrote:
>>
>> The stack_protect_test patterns were leaving the canary value in the
>> temporary register, meaning that it was often still in registers on
>> return from the function.  An attacker might therefore have been
>> able to use it to defeat stack-smash protection for a later function.
>>
>> Tested on arm-linux-gnueabi, arm-linux-gnueabihf and armeb-eabi.
>> I tested the thumb1.md part using arm-linux-gnueabi with the
>> test flags -march=armv5t -mthumb.  OK for trunk and branches?
>>
>> As I mentioned in the corresponding aarch64 patch, this is needed
>> to make arm conform to GCC's current -fstack-protector implementation.
>> However, I think we should reconsider whether the zeroing is actually
>> necessary and what it's actually protecting against.  I'll send a
>> separate message about that to gcc@.  But since the port isn't even
>> self-consistent (the *set patterns do clear the registers), I think
>> we should do this first rather than wait for any outcome of that
>> discussion.
>>
>> Richard
>>
>>
>> gcc/
>> PR target/96191
>> * config/arm/arm.md (arm_stack_protect_test_insn): Zero out
>> operand 2 after use.
>> * config/arm/thumb1.md (thumb1_stack_protect_test_insn): Likewise.
>>
>> gcc/testsuite/
>> * gcc.target/arm/stack-protector-1.c: New test.
>> * gcc.target/arm/stack-protector-2.c: Likewise.
>
> Hi Richard,
>
> The new tests fail when compiled with -mcpu=cortex-mXX because gas complains:
> use of r13 is deprecated
> It has a comment saying: "In the Thumb-2 ISA, use of R13 as Rm is
> deprecated, but valid."
>
> It's a minor nuisance, I'm not sure what the best way of getting rid of it?
> Add #ifndef __thumb2__ around CHECK(r13) ?

Hmm, maybe we should just drop that line altogether.  It wasn't exactly
likely that r13 would be the register to leak the value :-)

Should I post a patch or do you already have one ready?

Thanks,
Richard


[Bug fortran/96556] [11.0 regression] ICE via segmentation violation

2020-08-10 Thread juergen.reuter at desy dot de
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96556

--- Comment #1 from Jürgen Reuter  ---
Created attachment 49036
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49036=edit
First reproducer

[Bug fortran/96556] New: [11.0 regression] ICE via segmentation violation

2020-08-10 Thread juergen.reuter at desy dot de
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96556

Bug ID: 96556
   Summary: [11.0 regression] ICE via segmentation violation
   Product: gcc
   Version: 11.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: fortran
  Assignee: unassigned at gcc dot gnu.org
  Reporter: juergen.reuter at desy dot de
  Target Milestone: ---

The attached code (I will shorten the reproducer soon) gives the following ICE
below. The offending commit should have taken place between Sunday Aug 9 and
now:
gfortran  -c polarizations.f90
f951: internal compiler error: Segmentation fault
0xe01a4f crash_signal
../../gcc/toplev.c:327
0x8bc210 gfc_sym_get_dummy_args(gfc_symbol*)
../../gcc/fortran/symbol.c:5239
0x960d46 doloop_contained_function_call
../../gcc/fortran/frontend-passes.c:2336
0x965b75 gfc_expr_walker(gfc_expr**, int (*)(gfc_expr**, int*, void*), void*)
../../gcc/fortran/frontend-passes.c:5188
0x968045 gfc_code_walker(gfc_code**, int (*)(gfc_code**, int*, void*), int
(*)(gfc_expr**, int*, void*), void*)
../../gcc/fortran/frontend-passes.c:5613
0x9699d0 doloop_code
../../gcc/fortran/frontend-passes.c:2619
0x967f79 gfc_code_walker(gfc_code**, int (*)(gfc_code**, int*, void*), int
(*)(gfc_expr**, int*, void*), void*)
../../gcc/fortran/frontend-passes.c:5298
0x9680df gfc_code_walker(gfc_code**, int (*)(gfc_code**, int*, void*), int
(*)(gfc_expr**, int*, void*), void*)
../../gcc/fortran/frontend-passes.c:5621
0x9680df gfc_code_walker(gfc_code**, int (*)(gfc_code**, int*, void*), int
(*)(gfc_expr**, int*, void*), void*)
../../gcc/fortran/frontend-passes.c:5621
0x9687f8 gfc_code_walker(gfc_code**, int (*)(gfc_code**, int*, void*), int
(*)(gfc_expr**, int*, void*), void*)
../../gcc/fortran/frontend-passes.c:5324
0x9680df gfc_code_walker(gfc_code**, int (*)(gfc_code**, int*, void*), int
(*)(gfc_expr**, int*, void*), void*)
../../gcc/fortran/frontend-passes.c:5621
0x9690fb doloop_warn
../../gcc/fortran/frontend-passes.c:3051
0x969120 doloop_warn
../../gcc/fortran/frontend-passes.c:3056
0x9695fa gfc_run_passes(gfc_namespace*)
../../gcc/fortran/frontend-passes.c:156
0x884d07 gfc_resolve(gfc_namespace*)
../../gcc/fortran/resolve.c:17344
0x884d07 gfc_resolve(gfc_namespace*)
../../gcc/fortran/resolve.c:17317
0x8777e1 gfc_parse_file()
../../gcc/fortran/parse.c:6488
0x8c90ff gfc_be_parse_file
../../gcc/fortran/f95-lang.c:212
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See  for instructions.
Makefile:71: recipe for target 'polarizations.o' failed

[Bug c++/96555] New: "template argument involves template parameter(s)" with dot or arrow operator in partial specialization

2020-08-10 Thread arthur.j.odwyer at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96555

Bug ID: 96555
   Summary: "template argument involves template parameter(s)"
with dot or arrow operator in partial specialization
   Product: gcc
   Version: 11.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c++
  Assignee: unassigned at gcc dot gnu.org
  Reporter: arthur.j.odwyer at gmail dot com
  Target Milestone: ---

// https://godbolt.org/z/Kc98ea
struct A { int x; };
extern A a;
template struct B {};
template
struct B {};


:5:8: error: template argument '(int)sizeof (a.x)' involves template
parameter(s)
5 | struct B {};
  |^


GCC seems to have a problem only with expressions involving the dot operator
(e.g. `sizeof(a.x)`) and the arrow operator (e.g. `sizeof(p->x)`), but not any
other operators, which leads me to believe that the root cause here is related
to the root cause of bug 96215.

Grepbait: template argument involves template parameters

Same error message in different situations, all involving partial
specializations but none involving dot or arrow specifically: bug 67593, bug
83426, bug 90099

[Bug fortran/93671] gfortran 8-10 ICE on intrinsic assignment to allocatable derived-type component of coarray

2020-08-10 Thread vehre at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93671

vehre at gcc dot gnu.org changed:

   What|Removed |Added

 Status|ASSIGNED|WAITING

--- Comment #3 from vehre at gcc dot gnu.org ---
Patch at https://gcc.gnu.org/pipermail/fortran/2020-August/054865.html

Waiting for review.

[Patch, fortran] PR93671 - gfortran 8-10 ICE on intrinsic assignment to allocatable derived-type component of coarray

2020-08-10 Thread Andre Vehreschild
Hi folks,

long time, no see.  I was asked by Damian to do some Coarray stuff in gfortran
so here is the first step on fixing a bug. The issue at hand is, that the
coarray handling is not propagated correctly and later on the coarray-token
not generated/retrieved from the correct position leading to coarray programs to
crash/hang. This patch fixes at least the misbehavior reported in the PR. More
to come.

Regtests ok on FC31.x86_64. Ok for trunk?

Regards,
Andre
--
Andre Vehreschild * Email: vehre ad gmx dot de
gcc/fortran/ChangeLog:

2020-08-10  Andre Vehreschild  

PR fortran/93671
* trans-array.c (structure_alloc_comps): Keep caf-mode when applying to
components; get the caf_token correctly for allocated scalar components.

gcc/testsuite/ChangeLog:

2020-08-10  Andre Vehreschild  

PR fortran/93671
* gfortran.dg/coarray/pr93671.f90: New test.

diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c
index 8f93b43bafb..7a1b2fc74c9 100644
--- a/gcc/fortran/trans-array.c
+++ b/gcc/fortran/trans-array.c
@@ -8627,14 +8627,13 @@ structure_alloc_comps (gfc_symbol * der_type, tree decl,

   vref = gfc_build_array_ref (var, index, NULL);

-  if ((purpose == COPY_ALLOC_COMP || purpose == COPY_ONLY_ALLOC_COMP)
-	  && !caf_enabled (caf_mode))
+  if (purpose == COPY_ALLOC_COMP || purpose == COPY_ONLY_ALLOC_COMP)
 	{
 	  tmp = build_fold_indirect_ref_loc (input_location,
 	 gfc_conv_array_data (dest));
 	  dref = gfc_build_array_ref (tmp, index, NULL);
 	  tmp = structure_alloc_comps (der_type, vref, dref, rank,
-   COPY_ALLOC_COMP, 0, args);
+   COPY_ALLOC_COMP, caf_mode, args);
 	}
   else
 	tmp = structure_alloc_comps (der_type, vref, NULL_TREE, rank, purpose,
@@ -9375,12 +9374,21 @@ structure_alloc_comps (gfc_symbol * der_type, tree decl,
 	  else if (flag_coarray == GFC_FCOARRAY_LIB
 		   && caf_in_coarray (caf_mode))
 		{
-		  tree dst_tok = c->as ? gfc_conv_descriptor_token (dcmp)
-   : fold_build3_loc (input_location,
-			  COMPONENT_REF,
-			  pvoid_type_node, dest,
-			  c->caf_token,
-			  NULL_TREE);
+		  tree dst_tok;
+		  if (c->as)
+		dst_tok = gfc_conv_descriptor_token (dcmp);
+		  else
+		{
+		  /* For a scalar allocatable component the caf_token is
+			 the next component.  */
+		  if (!c->caf_token)
+			  c->caf_token = c->next->backend_decl;
+		  dst_tok = fold_build3_loc (input_location,
+		 COMPONENT_REF,
+		 pvoid_type_node, dest,
+		 c->caf_token,
+		 NULL_TREE);
+		}
 		  tmp = duplicate_allocatable_coarray (dcmp, dst_tok, comp,
 		   ctype, rank);
 		}
diff --git a/gcc/testsuite/gfortran.dg/coarray/pr93671.f90 b/gcc/testsuite/gfortran.dg/coarray/pr93671.f90
new file mode 100644
index 000..8d26ff88753
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/coarray/pr93671.f90
@@ -0,0 +1,24 @@
+! { dg-do run }
+
+! PR/fortran 93671 - ICE on intrinsic assignment to allocatable derived-type
+!component of coarray
+
+  type flux_planes
+integer, allocatable :: normals
+  end type
+
+  type package
+type(flux_planes) surface_fluxes(1)
+  end type
+
+  type(package) mail[*], halo_data
+
+  halo_data%surface_fluxes(1)%normals = 1
+  mail = halo_data
+
+  if (any(size(mail%surface_fluxes) /= [1]) .OR. &
+  mail%surface_fluxes(1)%normals /= 1) then
+stop 1
+  end if
+end
+


  1   2   3   >