Re: [PATCH] xfail and improve some failing libgomp tests

[Harwath, Frederik]

Hi Jakub,

On 07.02.20 16:29, Jakub Jelinek wrote:
> On Fri, Feb 07, 2020 at 09:56:38AM +0100, Harwath, Frederik wrote:
>> * {target-32.c, thread-limit-2.c}:
>> no "usleep" implemented for nvptx. Cf. https://gcc.gnu.org/PR81690
> 
> Please don't, I want to deal with that using declare variant, just didn't
> get yet around to finishing the last patch needed for that.  Will try next 
> week.

Ok, great! looking forward to see a better solution.

>> * target-{33,34}.c:
>> no "GOMP_OFFLOAD_async_run" implemented in plugin-nvptx.c. Cf. 
>> https://gcc.gnu.org/PR81688
>>
>> * target-link-1.c:
>> omp "target link" not implemented for nvptx. Cf. https://gcc.gnu.org/PR81689
> 
> I guess this is ok, though of course the right thing would be to implement
> both
Ok, this means that I can commit the attached patch which contains only the 
changes to
target-{33,43}.c and target-link-1.c? Of course, I agree that those features 
should be
implemented.

> There has been even in some PR a suggestion that instead of failing
> in nvptx async_run we should just ignore the nowait clause if the plugin
> doesn't implement it properly.

This must be https://gcc.gnu.org/PR93481.

Best regards,
Frederik


From e5165ccb143022614920dbd208f6f368b84b4382 Mon Sep 17 00:00:00 2001
From: Frederik Harwath 
Date: Mon, 10 Feb 2020 08:08:00 +0100
Subject: [PATCH] Add xfails to libgomp tests target-{33,34}.c, target-link-1.c

Add xfails for nvptx offloading because
"no GOMP_OFFLOAD_async_run implemented in plugin-nvptx.c"
(https://gcc.gnu.org/PR81688) and because
"omp target link not implemented for nvptx"
(https://gcc.gnu.org/PR81689).

libgomp/
	* testsuite/libgomp.c/target-33.c: Add xfail for execution on
	offload_target_nvptx, cf. https://gcc.gnu.org/PR81688.
	* testsuite/libgomp.c/target-34.c: Likewise.
	* testsuite/libgomp.c/target-link-1.c: Add xfail for
	offload_target_nvptx, cf. https://gcc.gnu.org/PR81689.
---
 libgomp/testsuite/libgomp.c/target-33.c | 3 +++
 libgomp/testsuite/libgomp.c/target-34.c | 3 +++
 libgomp/testsuite/libgomp.c/target-link-1.c | 3 +++
 3 files changed, 9 insertions(+)

diff --git a/libgomp/testsuite/libgomp.c/target-33.c b/libgomp/testsuite/libgomp.c/target-33.c
index 1bed4b6bc67..15d2d7e38ab 100644
--- a/libgomp/testsuite/libgomp.c/target-33.c
+++ b/libgomp/testsuite/libgomp.c/target-33.c
@@ -1,3 +1,6 @@
+/* { dg-xfail-run-if "GOMP_OFFLOAD_async_run not implemented" { offload_target_nvptx } }
+   Cf. https://gcc.gnu.org/PR81688.  */
+
 extern void abort (void);
 
 int
diff --git a/libgomp/testsuite/libgomp.c/target-34.c b/libgomp/testsuite/libgomp.c/target-34.c
index 66d9f54202b..5a3596424d8 100644
--- a/libgomp/testsuite/libgomp.c/target-34.c
+++ b/libgomp/testsuite/libgomp.c/target-34.c
@@ -1,3 +1,6 @@
+/* { dg-xfail-run-if "GOMP_OFFLOAD_async_run not implemented" { offload_target_nvptx } }
+   Cf. https://gcc.gnu.org/PR81688.  */
+
 extern void abort (void);
 
 int
diff --git a/libgomp/testsuite/libgomp.c/target-link-1.c b/libgomp/testsuite/libgomp.c/target-link-1.c
index 681677cc2aa..99ce33bc9b4 100644
--- a/libgomp/testsuite/libgomp.c/target-link-1.c
+++ b/libgomp/testsuite/libgomp.c/target-link-1.c
@@ -1,3 +1,6 @@
+/* { dg-xfail-if "#pragma omp target link not implemented" { offload_target_nvptx } }
+   Cf. https://gcc.gnu.org/PR81689.  */
+
 struct S { int s, t; };
 
 int a = 1, b = 1;
-- 
2.17.1

[PATCH 4/4 v2 GCC11] rs6000: P9 D-form test cases

[Kewen.Lin]

Hi Segher,

Updated as below according to your suggestion.

BR,
Kewen



gcc/testsuite/ChangeLog

2020-02-10  Kelvin Nilsen  
Kewen Lin  

* gcc.target/powerpc/p9-dform-0.c: New test.
* gcc.target/powerpc/p9-dform-1.c: New test.
* gcc.target/powerpc/p9-dform-2.c: New test.
* gcc.target/powerpc/p9-dform-3.c: New test.
* gcc.target/powerpc/p9-dform-4.c: New test.
* gcc.target/powerpc/p9-dform-generic.h: New test.

on 2020/1/20 下午9:19, Segher Boessenkool wrote:
> Hi!
> 
> On Thu, Jan 16, 2020 at 05:42:41PM +0800, Kewen.Lin wrote:
>> +/* At time the dform optimization pass was merged with trunk, 12
>> +   lxv instructions were emitted in place of the same number of lxvx
>> +   instructions.  No need to require exactly this number, as it may
>> +   change when other optimization passes evolve.  */
>> +
>> +/* { dg-final { scan-assembler {\mlxv\M} } } */
> 
> Maybe you can also test there ar no lxvx insns generated?
> 

Done, thanks!
---
 gcc/testsuite/gcc.target/powerpc/p9-dform-0.c  | 44 +
 gcc/testsuite/gcc.target/powerpc/p9-dform-1.c  | 57 ++
 gcc/testsuite/gcc.target/powerpc/p9-dform-2.c  | 14 ++
 gcc/testsuite/gcc.target/powerpc/p9-dform-3.c  | 17 +++
 gcc/testsuite/gcc.target/powerpc/p9-dform-4.c  | 14 ++
 .../gcc.target/powerpc/p9-dform-generic.h  | 34 +
 6 files changed, 180 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/powerpc/p9-dform-0.c
 create mode 100644 gcc/testsuite/gcc.target/powerpc/p9-dform-1.c
 create mode 100644 gcc/testsuite/gcc.target/powerpc/p9-dform-2.c
 create mode 100644 gcc/testsuite/gcc.target/powerpc/p9-dform-3.c
 create mode 100644 gcc/testsuite/gcc.target/powerpc/p9-dform-4.c
 create mode 100644 gcc/testsuite/gcc.target/powerpc/p9-dform-generic.h

diff --git a/gcc/testsuite/gcc.target/powerpc/p9-dform-0.c 
b/gcc/testsuite/gcc.target/powerpc/p9-dform-0.c
new file mode 100644
index 000..68b0434
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/p9-dform-0.c
@@ -0,0 +1,44 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target powerpc_p9vector_ok } */
+/* { dg-options "-O3 -mdejagnu-cpu=power9 -funroll-loops" } */
+
+/* This test confirms that the dform instructions are selected in the
+   translation of this main program.  */
+
+extern void first_dummy ();
+extern void dummy (double sacc, int n);
+extern void other_dummy ();
+
+extern float opt_value;
+extern char *opt_desc;
+
+#define M 128
+#define N 512
+
+double x [N];
+double y [N];
+
+int main (int argc, char *argv []) {
+  double sacc;
+
+  first_dummy ();
+  for (int j = 0; j < M; j++) {
+
+sacc = 0.00;
+for (unsigned long long int i = 0; i < N; i++) {
+  sacc += x[i] * y[i];
+}
+dummy (sacc, N);
+  }
+  opt_value = ((float) N) * 2 * ((float) M);
+  opt_desc = "flops";
+  other_dummy ();
+}
+
+/* At time the dform optimization pass was merged with trunk, 12
+   lxv instructions were emitted in place of the same number of lxvx
+   instructions.  No need to require exactly this number, as it may
+   change when other optimization passes evolve.  */
+
+/* { dg-final { scan-assembler {\mlxv\M} } } */
+/* { dg-final { scan-assembler-not {\mlxvx\M} } } */
diff --git a/gcc/testsuite/gcc.target/powerpc/p9-dform-1.c 
b/gcc/testsuite/gcc.target/powerpc/p9-dform-1.c
new file mode 100644
index 000..b80ffbc
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/p9-dform-1.c
@@ -0,0 +1,57 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target powerpc_p9vector_ok } */
+/* { dg-options "-O3 -mdejagnu-cpu=power9 -funroll-loops" } */
+
+/* This test confirms that the dform instructions are selected in the
+   translation of this main program.  */
+
+extern void first_dummy ();
+extern void dummy (double sacc, int n);
+extern void other_dummy ();
+
+extern float opt_value;
+extern char *opt_desc;
+
+#define M 128
+#define N 512
+
+double x [N];
+double y [N];
+double z [N];
+
+int main (int argc, char *argv []) {
+  double sacc;
+
+  first_dummy ();
+  for (int j = 0; j < M; j++) {
+
+sacc = 0.00;
+for (unsigned long long int i = 0; i < N; i++) {
+  z[i] = x[i] * y[i];
+  sacc += z[i];
+}
+dummy (sacc, N);
+  }
+  opt_value = ((float) N) * 2 * ((float) M);
+  opt_desc = "flops";
+  other_dummy ();
+}
+
+
+
+/* At time the dform optimization pass was merged with trunk, 12
+   lxv instructions were emitted in place of the same number of lxvx
+   instructions.  No need to require exactly this number, as it may
+   change when other optimization passes evolve.  */
+
+/* { dg-final { scan-assembler {\mlxv\M} } } */
+/* { dg-final { scan-assembler-not {\mlxvx\M} } } */
+
+/* At time the dform optimization pass was merged with trunk, 6
+   stxv instructions were emitted in place of the same number of stxvx
+   instructions.  No need to require exactly this number, as it may
+   change when other optimization passes evol

[PATCH 1/4 v2 GCC11] Add middle-end unroll factor estimation

[Kewen.Lin]

Hi Segher,

Thanks for your comments!  Updated to v2 as below:

  1) Removed unnecessary hook loop_unroll_adjust_tree.
  2) Updated estimated_uf to estimated_unroll and some comments.

gcc/ChangeLog

2020-02-10  Kewen Lin  

* cfgloop.h (struct loop): New field estimated_unroll.
* tree-ssa-loop-manip.c (decide_uf_const_iter): New function.
(decide_uf_runtime_iter): Likewise.
(decide_uf_stupid): Likewise.
(estimate_unroll_factor): Likewise.
* tree-ssa-loop-manip.h (estimate_unroll_factor): New declare.
* tree-ssa-loop.c (tree_average_num_loop_insns): New function.
* tree-ssa-loop.h (tree_average_num_loop_insns): New declare.

BR,
Kewen

on 2020/1/20 下午9:02, Segher Boessenkool wrote:
> Hi!
> 
> On Thu, Jan 16, 2020 at 05:39:40PM +0800, Kewen.Lin wrote:
>> --- a/gcc/cfgloop.h
>> +++ b/gcc/cfgloop.h
>> @@ -232,6 +232,9 @@ public:
>>   Other values means unroll with the given unrolling factor.  */
>>unsigned short unroll;
>>  
>> +  /* Like unroll field above, but it's estimated in middle-end.  */
>> +  unsigned short estimated_uf;
> 
> Please use full words?  "estimated_unroll" perhaps?  (Similar for other
> new names).
> 

Done.

>> +/* Implement targetm.loop_unroll_adjust_tree, strictly refers to
>> +   targetm.loop_unroll_adjust.  */
>> +
>> +static unsigned
>> +rs6000_loop_unroll_adjust_tree (unsigned nunroll, struct loop *loop)
>> +{
>> +  /* For now loop_unroll_adjust is simple, just invoke directly.  */
>> +  return rs6000_loop_unroll_adjust (nunroll, loop);
>> +}
> 
> Since the two hooks have the same arguments as well, it should really
> just be one hook, and an implementation can check whether
>   current_pass->type == RTL_PASS
> if it needs to do something special for RTL, etc.?  Or it can use some
> more appropriate condition -- the point is you need no extra hook.
> 

Good point, removed it.

>> +  /* Check number of iterations is constant.  */
>> +  if ((niter_desc->may_be_zero && !integer_zerop (niter_desc->may_be_zero))
>> +  || !tree_fits_uhwi_p (niter_desc->niter))
>> +return false;
> 
> Check, and do what?  It's easier to read if you say.
> 

Done.

> 
> "If loop->unroll is set, use that as loop->estimated_unroll"?
> 

Done.

---
 gcc/cfgloop.h |   3 +
 gcc/tree-ssa-loop-manip.c | 253 ++
 gcc/tree-ssa-loop-manip.h |   3 +-
 gcc/tree-ssa-loop.c   |  33 ++
 gcc/tree-ssa-loop.h   |   2 +
 5 files changed, 292 insertions(+), 2 deletions(-)

diff --git a/gcc/cfgloop.h b/gcc/cfgloop.h
index 11378ca..c5bcca7 100644
--- a/gcc/cfgloop.h
+++ b/gcc/cfgloop.h
@@ -232,6 +232,9 @@ public:
  Other values means unroll with the given unrolling factor.  */
   unsigned short unroll;
 
+  /* Like unroll field above, but it's estimated in middle-end.  */
+  unsigned short estimated_unroll;
+
   /* If this loop was inlined the main clique of the callee which does
  not need remapping when copying the loop body.  */
   unsigned short owned_clique;
diff --git a/gcc/tree-ssa-loop-manip.c b/gcc/tree-ssa-loop-manip.c
index 120b35b..72ac335 100644
--- a/gcc/tree-ssa-loop-manip.c
+++ b/gcc/tree-ssa-loop-manip.c
@@ -21,6 +21,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "system.h"
 #include "coretypes.h"
 #include "backend.h"
+#include "target.h"
 #include "tree.h"
 #include "gimple.h"
 #include "cfghooks.h"
@@ -42,6 +43,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "cfgloop.h"
 #include "tree-scalar-evolution.h"
 #include "tree-inline.h"
+#include "wide-int.h"
 
 /* All bitmaps for rewriting into loop-closed SSA go on this obstack,
so that we can free them all at once.  */
@@ -1592,3 +1594,254 @@ canonicalize_loop_ivs (class loop *loop, tree *nit, 
bool bump_in_latch)
 
   return var_before;
 }
+
+/* Try to determine estimated unroll factor for given LOOP with constant number
+   of iterations, mainly refer to decide_unroll_constant_iterations.
+- NITER_DESC holds number of iteration description if it isn't NULL.
+- NUNROLL holds a unroll factor value computed with instruction numbers.
+- ITER holds estimated or likely max loop iterations.
+   Return true if it succeeds, also update estimated_unroll.  */
+
+static bool
+decide_uf_const_iter (class loop *loop, const tree_niter_desc *niter_desc,
+ unsigned nunroll, const widest_int *iter)
+{
+  /* Skip big loops.  */
+  if (nunroll <= 1)
+return false;
+
+  gcc_assert (niter_desc && niter_desc->assumptions);
+
+  /* Check number of iterations is constant, return false if no.  */
+  if ((niter_desc->may_be_zero && !integer_zerop (niter_desc->may_be_zero))
+  || !tree_fits_uhwi_p (niter_desc->niter))
+return false;
+
+  unsigned HOST_WIDE_INT const_niter = tree_to_uhwi (niter_desc->niter);
+
+  /* If unroll factor is set explicitly, use it as estimated_unroll.  */
+  if (loop->unroll > 0 && loop->unroll < USHRT_MAX)
+{
+  /* I

Re: [PATCH 0/4 GCC11] IVOPTs consider step cost for different forms when unrolling

[Kewen.Lin]

Hi Segher,

on 2020/1/20 下午8:33, Segher Boessenkool wrote:
> Hi!
> 
> On Thu, Jan 16, 2020 at 05:36:52PM +0800, Kewen.Lin wrote:
>> As we discussed in the thread
>> https://gcc.gnu.org/ml/gcc-patches/2020-01/msg00196.html
>> Original: https://gcc.gnu.org/ml/gcc-patches/2020-01/msg00104.html,
>> I'm working to teach IVOPTs to consider D-form group access during unrolling.
>> The difference on D-form and other forms during unrolling is we can put the
>> stride into displacement field to avoid additional step increment. eg:
> 
> 
> 
>> Imagining that if the loop get unrolled by 8 times, then 3 step updates with
>> D-form vs. 8 step updates with X-form. Here we only need to check stride
>> meet D-form field requirement, since if OFF doesn't meet, we can construct
>> baseA' with baseA + OFF.
> 
> So why doesn't the existing code do this already?  Why does it make all
> the extra induction variables?  Is the existing cost model bad, are our
> target costs bad, or something like that?
> 

I think the main cause is IVOPTs runs before RTL unroll, when it's determining
the IV sets, it can only take the normal step cost into account, since its
input isn't unrolled yet.  After unrolling, the x-form indexing register has to
play with more UF-1 times update, but we can probably hide them in d-form
displacement field.  The way I proposed here is to adjust IV cost with
additional cost_step according to estimated unroll.  It doesn't introduce new
IV cand but can affect the final optimal set.

BR,
Kewen

Re: [PATCH V2] Generalized value pass-through for self-recursive function (ipa/pr93203)

[Feng Xue OS]

>> -   gcc_checking_assert (item->value);

> I've been staring at this for quite a while, trying to figure out how
> your patch can put NULL here before I realized it was just a clean-up
> :-)  Sending such changes independently or pointing them out in the
> email/ChangeLog makes review easier.

Ok. I'll add some description on this cleanup on ChangeLog.

>> @@ -5564,7 +5610,6 @@ decide_whether_version_node (struct cgraph_node *node)
>>   }
>>clone = create_specialized_node (node, known_csts, known_contexts,
>>  aggvals, callers);
>> -  info = IPA_NODE_REF (node);

> please either drop this change or change it to:
>
>   gcc_checking_assert (info == IPA_NODE_REF (node));

> this line of code was actually necessary when adding nodes possibly
> invalidated addresses of all summaries - like fast_function_summary
> classes still do.  So if we ever decide to use fast summaries we need a
> test to remind us that info address must be obtained again.
Ok. I'm not aware that, will keep the line as original.

Thanks,
Feng

Ping^1: [PATCH v3] ipa-cp: Fix PGO regression caused by r278808

[luoxhu]

Ping,

attachment of 
https://gcc.gnu.org/ml/gcc-patches/2020-01/msg00764/exchange2.tar.gz
shows the profile count difference on cloned nodes digits_2.constprop.[0...8]
without/with this patch.  Thanks!

Xiong Hu


On 2020/1/14 14:45, luoxhu wrote:
> Hi,
> 
> On 2020/1/3 00:58, Jan Hubicka wrote:
>>> diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
>>> index 14064ae0034..947bf7c7199 100644
>>> --- a/gcc/ipa-cp.c
>>> +++ b/gcc/ipa-cp.c
>>> @@ -4272,6 +4272,31 @@ update_profiling_info (struct cgraph_node 
>>> *orig_node,
>>>     false);
>>>     new_sum = stats.count_sum;
>>> +  class ipa_node_params *info = IPA_NODE_REF (orig_node);
>>> +  if (info && info->node_is_self_scc)
>>> +    {
>>> +  profile_count self_recursive_count;
>>> +
>>> +  /* The self recursive edge is on the orig_node.  */
>>> +  for (cs = orig_node->callees; cs; cs = cs->next_callee)
>>> +    if (ipa_edge_within_scc (cs))
>>> +  {
>>> +    cgraph_node *callee = cs->callee->function_symbol ();
>>> +    if (callee && cs->caller == cs->callee)
>>> +  {
>>> +    self_recursive_count = cs->count;
>>> +    break;
>>> +  }
>>> +  }
>>
>> What happens here when there are multiple self recursive calls or when
>> the SCC has two mutually recursive functions?
>>
>> I am still confused by the logic of this function.  I will take a deeper
>> look at your previous email.
>>> +
>>> +  /* Proportion count for self recursive node from all callers.  */
>>> +  new_sum
>>> +    = (orig_sum + new_sum).apply_scale (self_recursive_count, orig_sum);
>>> +
>>> +  /* Proportion count for specialized cloned node.  */
>>> +  new_sum = new_sum.apply_scale (1, param_ipa_cp_max_recursive_depth);
>>> +    }
>>> +
>>>     if (orig_node_count < orig_sum + new_sum)
>>>   {
>>>     if (dump_file)
>>> diff --git a/gcc/params.opt b/gcc/params.opt
>>> index d88ae0c468b..40a03b04580 100644
>>> --- a/gcc/params.opt
>>> +++ b/gcc/params.opt
>>> @@ -199,7 +199,7 @@ Common Joined UInteger 
>>> Var(param_ipa_cp_loop_hint_bonus) Init(64) Param
>>>   Compile-time bonus IPA-CP assigns to candidates which make loop bounds 
>>> or strides known.
>>>   -param=ipa-cp-max-recursive-depth=
>>> -Common Joined UInteger Var(param_ipa_cp_max_recursive_depth) Init(8) Param
>>> +Common Joined UInteger Var(param_ipa_cp_max_recursive_depth) Init(8) 
>>> IntegerRange(1, 10) Param
>>>   Maximum depth of recursive cloning for self-recursive function.
>>
>> The values stats from 0 but I also wonder why 10 is a good upper bound?
>> If the function calls itself with one type of value (like depth-1) then
>> we may clone well over 10 times, if it calls itself with two different
>> sets then 8 is already quite high I would say...
>>
>> I suppose the call probabilities will eventually drop to be very low,
>> but I am not quite sure about that because we do not handle any IP
>> frequency propagation.  Do we have some way to treat wide trees? Or do
>> we clone only all self recursive calls are the same?
>>
>> Honza
> 
> Update v3 patch.  This regression could only be reproduced when built with
> "-fprofile-generate/-fprofile-use --param ipa-cp-eval-threshold=0 --param
> ipa-cp-unit-growth=80" on exchange_2, on some platforms -fno-inline may be
> also needed, I attached 3 files (compressed to exchange.tar.gz)
> exchange2_gcc.use.orig.wpa.071i.cp.old, exchange2_gcc.use.orig.wpa.071i.cp.new
> and cp.diff to show the profile count difference of specialized node
> digits_2.constprop/152 to digits_2.constprop/159 without/with this patch.
> 
> Profile count is decreasing slowly with this patch instead of keeping very
> small from the first clone (very low count as cold will cause complete unroll
> not working), it still differs from really execution (exchange2.png), but this
> average model takes the recursive edge as feedback.  Thanks.
> 
> 
> v3:
> 1. Enable proportion orig_sum to the new nodes for self recursive node (k 
> means
>    multiple self recursive calls):
>    new_sum = (orig_sum + new_sum) * 1 / k \
>    * self_recursive_probability * (1 / param_ipa_cp_max_recursive_depth).
> 2. Two mutually recursive functions are not supported in self recursive
>    clone yet so also not supported in update_profiling_info here.
> 3. Improve value range for param_ipa_cp_max_recursive_depth to (0, 8).
>    If it calls itself two different sets, usually recursive boudary limit
>    will stop the specialize first, otherwise it is slow even without
>    recursive specialize.
> 
> The performance of exchange2 built with PGO will decrease ~28% by r278808
> due to profile count set incorrectly.  The cloned nodes are updated to a
> very small count caused later pass cunroll fail to unroll the recursive
> function in exchange2.
> 
> digits_2 ->
> digits_2.constprop.0, digits_2.constprop.1, etc.
> 
> gcc/ChangeLog:
> 
>  2020-01-14  Luo Xiong Hu  
> 
>  * ipa-cp.c (update_profiling_info): Check self_scc node.
>  * params.opt (i

Re: patch, fortan] PR83113 Bogus "duplicate allocatable attribute" error for submodule character function

[Andrew Benson]

Hi Steve,

Thanks for the review. Adding a macro as you suggest seems like a very good
idea. I'll make that change tomorrow before committing this.

Thanks,
Andrew


--

* Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html

* Galacticus: https://github.com/galacticusorg/galacticus

On Sun, Feb 9, 2020, 4:32 PM Steve Kargl 
wrote:

> Patch looks to me.  OK to commit.
>
> I'm wondering if we need a macro or helper function to
> simplify in conditional from
>
> +  if (gfc_state_stack->previous && gfc_state_stack->previous->previous
> +  && gfc_state_stack->previous->previous->state == COMP_SUBMODULE
> +  && sym->attr.module_procedure)
>
> to for example
>
> +  if (IN_SUBMODULE (sym))
>
> where is
>
> #define IN_SUBMODULE (x) \
>(gfc_state_stack->previous && gfc_state_stack->previous->previous \
> && gfc_state_stack->previous->previous->state == COMP_SUBMODULE \
> && sym->attr.module_procedure)
>
> --
> steve
>
> On Sun, Feb 09, 2020 at 02:59:52PM -0800, Andrew Benson wrote:
> > *ping*
> >
> > On Friday, January 31, 2020 5:06:49 PM PST Andrew Benson wrote:
> > > I've attached on updated patch for PR83113. The now removes the ICE
> when a
> > > duplicate DIMENSION attribute is specified in a submodule function.
> The
> > > patch reg tests cleanly.
> > >
> > > OK to commit?
> > >
> > > Note that this patch doesn't check that the attributes of duplicate
> > > declarations in a submodule function are valid or consistent with those
> > > declared in the module. For example both:
> > >
> > > module mm
> > >   implicit none
> > >   interface
> > >  module function c()
> > >integer, dimension(2)  :: c !! Dimension 2 here
> > >  end function c
> > >   end interface
> > > end module mm
> > >
> > > submodule (mm) oo
> > >   implicit none
> > > contains
> > >   module function c()
> > > integer, dimension(3)  :: c  !! Inconsistent dimension 3 here
> > >   end function c
> > > end submodule oo
> > >
> > >
> > > and
> > >
> > >
> > > module mm
> > >   implicit none
> > >   interface
> > >  module function c()
> > >integer, dimension(2)  :: c !! Dimension 2 here
> > >  end function c
> > >   end interface
> > > end module mm
> > >
> > > submodule (mm) oo
> > >   implicit none
> > > contains
> > >   module function c()
> > > integer, dimension(:)  :: c !! Invalid cannot have a deferred shape
> > >   end function c
> > > end submodule oo
> > >
> > >
> > > compile successfully, but should be rejected. Presumably we should add
> some
> > > check that the attributes of the declaration in the submodule match
> those in
> > > the module?
> > >
> > > If so, I can go ahead and open a PR for this problem.
> > >
> > > -Andrew
> > >
> > > On Friday, January 24, 2020 10:54:24 AM PST Andrew Benson wrote:
> > > > Below is a partial patch for PR 83113
> > > >
> > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83113
> > > >
> > > > It simply circumvents the check for duplicate attributes when
> looking at
> > > > declarations in a module procedure during compilation of a submodule.
> > > >
> > > > As it stands, the patch handles the cases of "allocatable",
> "dimension",
> > > > and "pointer" attributes (corresponding to the two test cases in the
> PR
> > > > plus a case that showed up in my own code). There are other
> attributes
> > > > which should be handled similarly but before I make those changes I
> > > > wanted to seek some opinions on whether this is the correct/best way
> to
> > > > fix this issue.
> > > >
> > > > The patch regression tests cleanly - however,  while the patch
> solves the
> > > > "duplicate attribute" problem for the test case in comment #4:
> > > >
> > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83113#c4
> > > >
> > > > that test case then ICEs with:
> > > >
> > > > $ gfortran -c pr83113_2.F90 -o pr83113_2.o
> > > > f951: internal compiler error: in gfc_set_array_spec, at
> > > > fortran/array.c:879 0x5fd22f gfc_set_array_spec(gfc_symbol*,
> > > > gfc_array_spec*, locus*)
> > > >
> > > > ../../gcc-git/gcc/fortran/array.c:879
> > > >
> > > > 0x7e868e build_sym
> > > >
> > > > ../../gcc-git/gcc/fortran/decl.c:1670
> > > >
> > > > 0x7f0ada variable_decl
> > > >
> > > > ../../gcc-git/gcc/fortran/decl.c:2760
> > > >
> > > > 0x7f0ada gfc_match_data_decl()
> > > >
> > > > ../../gcc-git/gcc/fortran/decl.c:6120
> > > >
> > > > 0x85b66d match_word
> > > >
> > > > ../../gcc-git/gcc/fortran/parse.c:65
> > > >
> > > > 0x85b66d decode_statement
> > > >
> > > > ../../gcc-git/gcc/fortran/parse.c:376
> > > >
> > > > 0x861094 next_free
> > > >
> > > > ../../gcc-git/gcc/fortran/parse.c:1279
> > > >
> > > > 0x861094 next_statement
> > > >
> > > > ../../gcc-git/gcc/fortran/parse.c:1511
> > > >
> > > > 0x8638ac parse_spec
> > > >
> > > > ../../gcc-git/gcc/fortran/parse.c:3738
> > > >
> > > > 0x86591c parse_progunit
> > > >
> > > > ../../gcc-git/gcc/fortran/parse.c:5851

Re: [PATCH 1/2] analyzer: gfortran testsuite support

[Steve Kargl]

On Sun, Feb 09, 2020 at 04:19:13PM -0500, David Malcolm wrote:
> On Sun, 2020-02-09 at 12:55 -0800, Steve Kargl wrote:
> > On Sun, Feb 09, 2020 at 09:15:46PM +0100, Toon Moene wrote:
> > > On 2/6/20 9:01 PM, David Malcolm wrote:
> > > 
> > > > PR analyzer/93405 reports an ICE when attempting to use
> > > > -fanalyzer on
> > > > certain gfortran code.  The second patch in this kit fixes that,
> > > > but
> > > > in the meantime I need somewhere to put regression tests for
> > > > -fanalyzer
> > > > with gfortran.
> > > > 
> > > > This patch adds a gfortran.dg/analyzer subdirectory with an
> > > > analyzer.exp,
> > > > setting DEFAULT_FFLAGS on the tests run within it.
> > > 
> > > I have seen no objections against this proposal, so please go
> > > ahead.
> > > 
> > 
> > Perhaps, there are no objections because the people who contribute
> > patches and provide reviews for gfortran have twindled to 1 or 2
> > people
> > with sporadic available time.  Did you actually review the proposed
> > changes?  If not, how can you rubber stamp this commit?  You have a
> > total of 12 ChangeLog entries over 18 years with the last occurring
> > in
> > 2011, and I do not recall you ever reviewing a patch. 
> 
> FWIW Toon reported in BZ that patch 2 in the kit fixed the ICE he had
> reported, and I asked there if he was able to review this patch, which
> is what led to his email.
> 
> I'm sorry if I overstepped the mark here.

You didn't overstep the mark.  I was questioning the manner in
which approval seem to be rubber stamped.

> >  Finally, trunk
> > is in stage 4 (regression fixes & docs only).  This does not look
> > like
> > either.
> 
> Indeed.  The analyzer is a new feature in GCC 10.  I'm hoping some
> latitude can be granted here given it's new (and hence all of its ICEs
> are, strictly speaking, not regressions), and this is about adding test
> coverage for fixing them.

Having now looked at the patch, I think it's okay to commit.
As you note, it is new functionality in 10 so technically 
cannot be a regression.  But, it does fix an issue before 
10 is even released.

OK to commit.

-- 
Steve

Re: patch, fortan] PR83113 Bogus "duplicate allocatable attribute" error for submodule character function

[Steve Kargl]

Patch looks to me.  OK to commit.

I'm wondering if we need a macro or helper function to
simplify in conditional from

+  if (gfc_state_stack->previous && gfc_state_stack->previous->previous
+  && gfc_state_stack->previous->previous->state == COMP_SUBMODULE
+  && sym->attr.module_procedure)

to for example

+  if (IN_SUBMODULE (sym))

where is 

#define IN_SUBMODULE (x) \
   (gfc_state_stack->previous && gfc_state_stack->previous->previous \
&& gfc_state_stack->previous->previous->state == COMP_SUBMODULE \
&& sym->attr.module_procedure)

-- 
steve

On Sun, Feb 09, 2020 at 02:59:52PM -0800, Andrew Benson wrote:
> *ping*
> 
> On Friday, January 31, 2020 5:06:49 PM PST Andrew Benson wrote:
> > I've attached on updated patch for PR83113. The now removes the ICE when a
> > duplicate DIMENSION attribute is specified in a submodule function.  The
> > patch reg tests cleanly.
> > 
> > OK to commit?
> > 
> > Note that this patch doesn't check that the attributes of duplicate
> > declarations in a submodule function are valid or consistent with those
> > declared in the module. For example both:
> > 
> > module mm
> >   implicit none
> >   interface
> >  module function c()
> >integer, dimension(2)  :: c !! Dimension 2 here
> >  end function c
> >   end interface
> > end module mm
> > 
> > submodule (mm) oo
> >   implicit none
> > contains
> >   module function c()
> > integer, dimension(3)  :: c  !! Inconsistent dimension 3 here
> >   end function c
> > end submodule oo
> > 
> > 
> > and
> > 
> > 
> > module mm
> >   implicit none
> >   interface
> >  module function c()
> >integer, dimension(2)  :: c !! Dimension 2 here
> >  end function c
> >   end interface
> > end module mm
> > 
> > submodule (mm) oo
> >   implicit none
> > contains
> >   module function c()
> > integer, dimension(:)  :: c !! Invalid cannot have a deferred shape
> >   end function c
> > end submodule oo
> > 
> > 
> > compile successfully, but should be rejected. Presumably we should add some
> > check that the attributes of the declaration in the submodule match those in
> > the module?
> > 
> > If so, I can go ahead and open a PR for this problem.
> > 
> > -Andrew
> > 
> > On Friday, January 24, 2020 10:54:24 AM PST Andrew Benson wrote:
> > > Below is a partial patch for PR 83113
> > > 
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83113
> > > 
> > > It simply circumvents the check for duplicate attributes when looking at
> > > declarations in a module procedure during compilation of a submodule.
> > > 
> > > As it stands, the patch handles the cases of "allocatable", "dimension",
> > > and "pointer" attributes (corresponding to the two test cases in the PR
> > > plus a case that showed up in my own code). There are other attributes
> > > which should be handled similarly but before I make those changes I
> > > wanted to seek some opinions on whether this is the correct/best way to
> > > fix this issue.
> > > 
> > > The patch regression tests cleanly - however,  while the patch solves the
> > > "duplicate attribute" problem for the test case in comment #4:
> > > 
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83113#c4
> > > 
> > > that test case then ICEs with:
> > > 
> > > $ gfortran -c pr83113_2.F90 -o pr83113_2.o
> > > f951: internal compiler error: in gfc_set_array_spec, at
> > > fortran/array.c:879 0x5fd22f gfc_set_array_spec(gfc_symbol*,
> > > gfc_array_spec*, locus*)
> > > 
> > > ../../gcc-git/gcc/fortran/array.c:879
> > > 
> > > 0x7e868e build_sym
> > > 
> > > ../../gcc-git/gcc/fortran/decl.c:1670
> > > 
> > > 0x7f0ada variable_decl
> > > 
> > > ../../gcc-git/gcc/fortran/decl.c:2760
> > > 
> > > 0x7f0ada gfc_match_data_decl()
> > > 
> > > ../../gcc-git/gcc/fortran/decl.c:6120
> > > 
> > > 0x85b66d match_word
> > > 
> > > ../../gcc-git/gcc/fortran/parse.c:65
> > > 
> > > 0x85b66d decode_statement
> > > 
> > > ../../gcc-git/gcc/fortran/parse.c:376
> > > 
> > > 0x861094 next_free
> > > 
> > > ../../gcc-git/gcc/fortran/parse.c:1279
> > > 
> > > 0x861094 next_statement
> > > 
> > > ../../gcc-git/gcc/fortran/parse.c:1511
> > > 
> > > 0x8638ac parse_spec
> > > 
> > > ../../gcc-git/gcc/fortran/parse.c:3738
> > > 
> > > 0x86591c parse_progunit
> > > 
> > > ../../gcc-git/gcc/fortran/parse.c:5851
> > > 
> > > 0x865df9 parse_contained
> > > 
> > > ../../gcc-git/gcc/fortran/parse.c:5752
> > > 
> > > 0x866b5e parse_module
> > > 
> > > ../../gcc-git/gcc/fortran/parse.c:6124
> > > 
> > > 0x86709c gfc_parse_file()
> > > 
> > > ../../gcc-git/gcc/fortran/parse.c:6427
> > > 
> > > 0x8b756f gfc_be_parse_file
> > > 
> > > ../../gcc-git/gcc/fortran/f95-lang.c:210
> > > 
> > > Please submit a full bug report,
> > > with preprocessed source if appropriate.
> > > Please include the complete backtrace with any bug report.
> > > See  for instructions.
> > > 
> >

[PATCH] Generalize -fuse-ld= to support absolute path or arbitrary ld.linker

[Fangrui Song via gcc-patches]

PR driver/93645
* common.opt (-fuse-ld=): Delete -fuse-ld=[bfd|gold|lld]. Add -fuse-ld=.
* opts.c (common_handle_option): Handle OPT_fuse_ld_.
* gcc.c (driver_handle_option): Likewise.
* collect2.c (main): Likewise.
---
 gcc/ChangeLog   |  8 ++
 gcc/collect2.c  | 67 -
 gcc/common.opt  | 14 ++
 gcc/doc/invoke.texi | 15 +++---
 gcc/gcc.c   | 14 --
 gcc/opts.c  |  4 +--
 6 files changed, 57 insertions(+), 65 deletions(-)

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index feb2d066d0b..6bcec12d841 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,11 @@
+2020-02-09  Fangrui Song  
+
+   PR driver/93645
+   * common.opt (-fuse-ld=): Delete -fuse-ld=[bfd|gold|lld]. Add -fuse-ld=.
+   * opts.c (common_handle_option): Handle OPT_fuse_ld_.
+   * gcc.c (driver_handle_option): Likewise.
+   * collect2.c (main): Likewise.
+
 2020-02-09  Uroš Bizjak  
 
* recog.c: Move pass_split_before_sched2 code in front of
diff --git a/gcc/collect2.c b/gcc/collect2.c
index 502d629141c..a3ef525a93b 100644
--- a/gcc/collect2.c
+++ b/gcc/collect2.c
@@ -859,18 +859,12 @@ main (int argc, char **argv)
 {
   USE_DEFAULT_LD,
   USE_PLUGIN_LD,
-  USE_GOLD_LD,
-  USE_BFD_LD,
-  USE_LLD_LD,
-  USE_LD_MAX
+  USE_LD
 } selected_linker = USE_DEFAULT_LD;
-  static const char *const ld_suffixes[USE_LD_MAX] =
+  static const char *const ld_suffixes[USE_LD] =
 {
   "ld",
-  PLUGIN_LD_SUFFIX,
-  "ld.gold",
-  "ld.bfd",
-  "ld.lld"
+  PLUGIN_LD_SUFFIX
 };
   static const char *const real_ld_suffix = "real-ld";
   static const char *const collect_ld_suffix = "collect-ld";
@@ -882,7 +876,7 @@ main (int argc, char **argv)
   static const char *const strip_suffix = "strip";
   static const char *const gstrip_suffix = "gstrip";
 
-  const char *full_ld_suffixes[USE_LD_MAX];
+  const char *full_ld_suffixes[USE_LD];
 #ifdef CROSS_DIRECTORY_STRUCTURE
   /* If we look for a program in the compiler directories, we just use
  the short name, since these directories are already system-specific.
@@ -924,6 +918,7 @@ main (int argc, char **argv)
   const char **ld1;
   bool use_plugin = false;
   bool use_collect_ld = false;
+  const char *use_ld = NULL;
 
   /* The kinds of symbols we will have to consider when scanning the
  outcome of a first pass link.  This is ALL to start with, then might
@@ -948,7 +943,7 @@ main (int argc, char **argv)
 #endif
   int i;
 
-  for (i = 0; i < USE_LD_MAX; i++)
+  for (i = 0; i < USE_LD; i++)
 full_ld_suffixes[i]
 #ifdef CROSS_DIRECTORY_STRUCTURE
   = concat (target_machine, "-", ld_suffixes[i], NULL);
@@ -1041,12 +1036,11 @@ main (int argc, char **argv)
if (selected_linker == USE_DEFAULT_LD)
  selected_linker = USE_PLUGIN_LD;
  }
-   else if (strcmp (argv[i], "-fuse-ld=bfd") == 0)
- selected_linker = USE_BFD_LD;
-   else if (strcmp (argv[i], "-fuse-ld=gold") == 0)
- selected_linker = USE_GOLD_LD;
-   else if (strcmp (argv[i], "-fuse-ld=lld") == 0)
- selected_linker = USE_LLD_LD;
+   else if (!strncmp (argv[i], "-fuse-ld=", 9))
+ {
+   use_ld = argv[i] + 9;
+   selected_linker = USE_LD;
+ }
else if (strncmp (argv[i], "-o", 2) == 0)
  {
/* Parse the output filename if it's given so that we can make
@@ -1152,8 +1146,7 @@ main (int argc, char **argv)
   /* Maybe we know the right file to use (if not cross).  */
   ld_file_name = 0;
 #ifdef DEFAULT_LINKER
-  if (selected_linker == USE_BFD_LD || selected_linker == USE_GOLD_LD ||
-  selected_linker == USE_LLD_LD)
+  if (!ld_file_name && selected_linker == USE_LD)
 {
   char *linker_name;
 # ifdef HOST_EXECUTABLE_SUFFIX
@@ -1168,15 +1161,13 @@ main (int argc, char **argv)
  if (! strcmp (&default_linker[len], HOST_EXECUTABLE_SUFFIX))
{
  default_linker[len] = '\0';
- linker_name = concat (default_linker,
-   &ld_suffixes[selected_linker][2],
+ linker_name = concat (default_linker, ".", use_ld,
HOST_EXECUTABLE_SUFFIX, NULL);
}
}
   if (linker_name == NULL)
 # endif
-  linker_name = concat (DEFAULT_LINKER,
-   &ld_suffixes[selected_linker][2],
+  linker_name = concat (DEFAULT_LINKER, ".", use_ld,
NULL);
   if (access (linker_name, X_OK) == 0)
ld_file_name = linker_name;
@@ -1197,14 +1188,28 @@ main (int argc, char **argv)
   ld_file_name = find_a_file (&cpath, collect_ld_suffix, X_OK);
   use_collect_ld = ld_file_name != 0;
 }
-  /* Search the compiler directories for `ld'.  We have protection against
- recursive calls in find_a_file.  */
-  if (ld_file_name == 0)
-  

Re: patch, fortan] PR83113 Bogus "duplicate allocatable attribute" error for submodule character function

[Andrew Benson]

*ping*

On Friday, January 31, 2020 5:06:49 PM PST Andrew Benson wrote:
> I've attached on updated patch for PR83113. The now removes the ICE when a
> duplicate DIMENSION attribute is specified in a submodule function.  The
> patch reg tests cleanly.
> 
> OK to commit?
> 
> Note that this patch doesn't check that the attributes of duplicate
> declarations in a submodule function are valid or consistent with those
> declared in the module. For example both:
> 
> module mm
>   implicit none
>   interface
>  module function c()
>integer, dimension(2)  :: c !! Dimension 2 here
>  end function c
>   end interface
> end module mm
> 
> submodule (mm) oo
>   implicit none
> contains
>   module function c()
> integer, dimension(3)  :: c  !! Inconsistent dimension 3 here
>   end function c
> end submodule oo
> 
> 
> and
> 
> 
> module mm
>   implicit none
>   interface
>  module function c()
>integer, dimension(2)  :: c !! Dimension 2 here
>  end function c
>   end interface
> end module mm
> 
> submodule (mm) oo
>   implicit none
> contains
>   module function c()
> integer, dimension(:)  :: c !! Invalid cannot have a deferred shape
>   end function c
> end submodule oo
> 
> 
> compile successfully, but should be rejected. Presumably we should add some
> check that the attributes of the declaration in the submodule match those in
> the module?
> 
> If so, I can go ahead and open a PR for this problem.
> 
> -Andrew
> 
> On Friday, January 24, 2020 10:54:24 AM PST Andrew Benson wrote:
> > Below is a partial patch for PR 83113
> > 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83113
> > 
> > It simply circumvents the check for duplicate attributes when looking at
> > declarations in a module procedure during compilation of a submodule.
> > 
> > As it stands, the patch handles the cases of "allocatable", "dimension",
> > and "pointer" attributes (corresponding to the two test cases in the PR
> > plus a case that showed up in my own code). There are other attributes
> > which should be handled similarly but before I make those changes I
> > wanted to seek some opinions on whether this is the correct/best way to
> > fix this issue.
> > 
> > The patch regression tests cleanly - however,  while the patch solves the
> > "duplicate attribute" problem for the test case in comment #4:
> > 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83113#c4
> > 
> > that test case then ICEs with:
> > 
> > $ gfortran -c pr83113_2.F90 -o pr83113_2.o
> > f951: internal compiler error: in gfc_set_array_spec, at
> > fortran/array.c:879 0x5fd22f gfc_set_array_spec(gfc_symbol*,
> > gfc_array_spec*, locus*)
> > 
> > ../../gcc-git/gcc/fortran/array.c:879
> > 
> > 0x7e868e build_sym
> > 
> > ../../gcc-git/gcc/fortran/decl.c:1670
> > 
> > 0x7f0ada variable_decl
> > 
> > ../../gcc-git/gcc/fortran/decl.c:2760
> > 
> > 0x7f0ada gfc_match_data_decl()
> > 
> > ../../gcc-git/gcc/fortran/decl.c:6120
> > 
> > 0x85b66d match_word
> > 
> > ../../gcc-git/gcc/fortran/parse.c:65
> > 
> > 0x85b66d decode_statement
> > 
> > ../../gcc-git/gcc/fortran/parse.c:376
> > 
> > 0x861094 next_free
> > 
> > ../../gcc-git/gcc/fortran/parse.c:1279
> > 
> > 0x861094 next_statement
> > 
> > ../../gcc-git/gcc/fortran/parse.c:1511
> > 
> > 0x8638ac parse_spec
> > 
> > ../../gcc-git/gcc/fortran/parse.c:3738
> > 
> > 0x86591c parse_progunit
> > 
> > ../../gcc-git/gcc/fortran/parse.c:5851
> > 
> > 0x865df9 parse_contained
> > 
> > ../../gcc-git/gcc/fortran/parse.c:5752
> > 
> > 0x866b5e parse_module
> > 
> > ../../gcc-git/gcc/fortran/parse.c:6124
> > 
> > 0x86709c gfc_parse_file()
> > 
> > ../../gcc-git/gcc/fortran/parse.c:6427
> > 
> > 0x8b756f gfc_be_parse_file
> > 
> > ../../gcc-git/gcc/fortran/f95-lang.c:210
> > 
> > Please submit a full bug report,
> > with preprocessed source if appropriate.
> > Please include the complete backtrace with any bug report.
> > See  for instructions.
> > 
> > 
> > This looks like a separate problem to me, but looking quickly at where the
> > ICE occurs it seems to be related to coarrays of which my understanding is
> > very limited - so any insights on this would be welcome.
> > 
> > Patch is appended below.
> > 
> > -Andrew


-- 

* Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html

* Galacticus: https://github.com/galacticusorg/galacticus

[PING] [PATCH] Add --with-diagnostics-urls configuration option and GCC_URLS/TERM_URLS env var

[Bernd Edlinger]

Ping...

the latest version of this patch is here:
https://gcc.gnu.org/ml/gcc-patches/2020-02/msg00121.html


Thanks
Bernd.

On 2/3/20 11:29 PM, Bernd Edlinger wrote:
> On 2/3/20 10:05 PM, Segher Boessenkool wrote:
>> On Mon, Feb 03, 2020 at 08:16:52PM +, Bernd Edlinger wrote:
>>> So gnome terminal is a problem, since it depend heavily on the software
>>> version, VTE library, and gnome-terminal.
>>> Sometimes URLs are functional, sometimes competely buggy.
>>>
>>> But, wait a moment, here is the deal:
>>>
>>> I can detect old gnome terminals,
>>> they have COLORTERM=gnome-terminal (and produce garbage)
>>>
>>> but new gnome terminal with true URL-support have
>>>
>>> COLORTERM=truecolor
>>>
>>> So how about adding that to the detection logic ?
>>
>> It works on at least one of my older setups, too (will have to check
>> the rest when I have time, unfortunately the weekend is just past).
>>
> 
> Cool.
> 
> so here is the next version, which removes tmux, and adds
> detection of old gnome-terminal, and linux console sessions,
> while also attempting to work with ssh sessions, where we
> do we have a bit less reliable information, but I would
> think that is still an improvement.  I'd let TERM_URLS and
> GCC_URLS override the last two exceptions, as TERM=xterm
> can also mean, really xterm, but while that one does not
> print garbage, it does not handle the URLs either.
> 
> 
> How do you like it this way?
> 
> Is it OK for trunk?
> 
> 
> Thanks
> Bernd.
> 

Re: [PATCH 1/2] analyzer: gfortran testsuite support

[Toon Moene]

On 2/9/20 9:55 PM, Steve Kargl wrote:


On Sun, Feb 09, 2020 at 09:15:46PM +0100, Toon Moene wrote:



On 2/6/20 9:01 PM, David Malcolm wrote:


PR analyzer/93405 reports an ICE when attempting to use -fanalyzer on
certain gfortran code.  The second patch in this kit fixes that, but
in the meantime I need somewhere to put regression tests for -fanalyzer
with gfortran.

This patch adds a gfortran.dg/analyzer subdirectory with an analyzer.exp,
setting DEFAULT_FFLAGS on the tests run within it.


I have seen no objections against this proposal, so please go ahead.



Perhaps, there are no objections because the people who contribute
patches and provide reviews for gfortran have twindled to 1 or 2 people
with sporadic available time.  Did you actually review the proposed
changes?


You are right. I did test the fix for PR93405, and thought this update 
was included in that test, but it was not - my fault.


I will be more careful in the future about what I test, and show the 
results (otherwise, why test ...).


Suggestion withdrawn.

--
Toon Moene - e-mail: t...@moene.org - phone: +31 346 214290
Saturnushof 14, 3738 XG  Maartensdijk, The Netherlands
At home: http://moene.org/~toon/; weather: http://moene.org/~hirlam/
Progress of GNU Fortran: http://gcc.gnu.org/wiki/GFortran#news

Re: [PATCH 1/2] analyzer: gfortran testsuite support

[David Malcolm]

On Sun, 2020-02-09 at 12:55 -0800, Steve Kargl wrote:
> On Sun, Feb 09, 2020 at 09:15:46PM +0100, Toon Moene wrote:
> > On 2/6/20 9:01 PM, David Malcolm wrote:
> > 
> > > PR analyzer/93405 reports an ICE when attempting to use
> > > -fanalyzer on
> > > certain gfortran code.  The second patch in this kit fixes that,
> > > but
> > > in the meantime I need somewhere to put regression tests for
> > > -fanalyzer
> > > with gfortran.
> > > 
> > > This patch adds a gfortran.dg/analyzer subdirectory with an
> > > analyzer.exp,
> > > setting DEFAULT_FFLAGS on the tests run within it.
> > 
> > I have seen no objections against this proposal, so please go
> > ahead.
> > 
> 
> Perhaps, there are no objections because the people who contribute
> patches and provide reviews for gfortran have twindled to 1 or 2
> people
> with sporadic available time.  Did you actually review the proposed
> changes?  If not, how can you rubber stamp this commit?  You have a
> total of 12 ChangeLog entries over 18 years with the last occurring
> in
> 2011, and I do not recall you ever reviewing a patch. 

FWIW Toon reported in BZ that patch 2 in the kit fixed the ICE he had
reported, and I asked there if he was able to review this patch, which
is what led to his email.

I'm sorry if I overstepped the mark here.

>  Finally, trunk
> is in stage 4 (regression fixes & docs only).  This does not look
> like
> either.

Indeed.  The analyzer is a new feature in GCC 10.  I'm hoping some
latitude can be granted here given it's new (and hence all of its ICEs
are, strictly speaking, not regressions), and this is about adding test
coverage for fixing them.

> If I bootstrap gcc with fortran
> 
> % mkdir obj
> % ../gcc/configure --prefix=$HOME/work --enable-languages=c,fortran \
>   --enable-bootstrap --enable-checking=yes
> % gmake bootstrap
> 
> and then do
> 
> % cd gcc
> % gmake check-fortran
> 
> are these analyzer testcases built/executed? 

That's the intent of the patch, yes (the cases are marked with dg-do
compile, so "built", at least, if not "executed").

Note that it's possible to disable the analyzer at configure-time via
-fdisable-analyzer; the analyzer.exp checks for this via
check_effective_target_analyzer.

> Do all architectures
> that support gfortran also support the analyzer infrastructure?

I believe so, yes; I've fixed bugs on e.g. powerpc-ibm-aix7.2.0.0 since
merging (and if not, the check_effective_target_analyzer ought to
immediately reject running the tests).

That said, I've only verified these testcases on x86_64-pc-linux-gnu so
far.

An alternative would be to split patch 2, committing the ICE fix to the
analyzer, and leaving the test coverage to next stage 1.

Thoughts?

Thanks
Dave

Re: [PATCH 1/2] analyzer: gfortran testsuite support

[Steve Kargl]

On Sun, Feb 09, 2020 at 09:15:46PM +0100, Toon Moene wrote:
> On 2/6/20 9:01 PM, David Malcolm wrote:
> 
> > PR analyzer/93405 reports an ICE when attempting to use -fanalyzer on
> > certain gfortran code.  The second patch in this kit fixes that, but
> > in the meantime I need somewhere to put regression tests for -fanalyzer
> > with gfortran.
> > 
> > This patch adds a gfortran.dg/analyzer subdirectory with an analyzer.exp,
> > setting DEFAULT_FFLAGS on the tests run within it.
> 
> I have seen no objections against this proposal, so please go ahead.
> 

Perhaps, there are no objections because the people who contribute
patches and provide reviews for gfortran have twindled to 1 or 2 people
with sporadic available time.  Did you actually review the proposed
changes?  If not, how can you rubber stamp this commit?  You have a
total of 12 ChangeLog entries over 18 years with the last occurring in
2011, and I do not recall you ever reviewing a patch.  Finally, trunk
is in stage 4 (regression fixes & docs only).  This does not look like
either.

If I bootstrap gcc with fortran

% mkdir obj
% ../gcc/configure --prefix=$HOME/work --enable-languages=c,fortran \
  --enable-bootstrap --enable-checking=yes
% gmake bootstrap

and then do

% cd gcc
% gmake check-fortran

are these analyzer testcases built/executed?  Do all architectures
that support gfortran also support the analyzer infrastructure?

-- 
Steve

Re: [PATCH 1/2] analyzer: gfortran testsuite support

[Toon Moene]

On 2/6/20 9:01 PM, David Malcolm wrote:


PR analyzer/93405 reports an ICE when attempting to use -fanalyzer on
certain gfortran code.  The second patch in this kit fixes that, but
in the meantime I need somewhere to put regression tests for -fanalyzer
with gfortran.

This patch adds a gfortran.dg/analyzer subdirectory with an analyzer.exp,
setting DEFAULT_FFLAGS on the tests run within it.


I have seen no objections against this proposal, so please go ahead.

Kind regards,

--
Toon Moene - e-mail: t...@moene.org - phone: +31 346 214290
Saturnushof 14, 3738 XG  Maartensdijk, The Netherlands
At home: http://moene.org/~toon/; weather: http://moene.org/~hirlam/
Progress of GNU Fortran: http://gcc.gnu.org/wiki/GFortran#news

Re: [PATCH] i386: Properly pop restore token in signal frame

[Uros Bizjak]

On Sat, Feb 8, 2020 at 2:43 PM H.J. Lu  wrote:
>
> Linux CET kernel places a restore token on shadow stack for signal
> handler to enhance security.  The restore token is 8 byte and aligned
> to 8 bytes.  It is usually transparent to user programs since kernel
> will pop the restore token when signal handler returns.  But when an
> exception is thrown from a signal handler, now we need to pop the
> restore token from shadow stack.  For x86-64, we just need to treat
> the signal frame as normal frame.  For i386, we need to search for
> the restore token to check if the original shadow stack is 8 byte
> aligned.  If the original shadow stack is 8 byte aligned, we just
> need to pop 2 slots, one restore token, from shadow stack.  Otherwise,
> we need to pop 3 slots, one restore token + 4 byte pading, from
> shadow stack.
>
> This patch also includes 2 tests, one has a restore token with 4 byte
> padding and one without.
>
> Tested on Linux/x86-64 CET machine with and without -m32.  OK for master
> and backport to GCC 8/9 branches?
>
> Thanks.
>
> H.J.
> ---
> libgcc/
>
> PR libgcc/85334
> * config/i386/shadow-stack-unwind.h (_Unwind_Frames_Increment):
> New.
>
> gcc/testsuite/
>
> PR libgcc/85334
> * g++.target/i386/pr85334-1.C: New test.
> * g++.target/i386/pr85334-2.C: Likewise.

LGTM, with a couple of nits.

Disclaimer: I'm not expert with CET, so no thorough review here.

Thanks,
Uros.

> ---
>  gcc/testsuite/g++.target/i386/pr85334-1.C | 55 +++
>  gcc/testsuite/g++.target/i386/pr85334-2.C | 48 
>  libgcc/config/i386/shadow-stack-unwind.h  | 43 ++
>  3 files changed, 146 insertions(+)
>  create mode 100644 gcc/testsuite/g++.target/i386/pr85334-1.C
>  create mode 100644 gcc/testsuite/g++.target/i386/pr85334-2.C
>
> diff --git a/gcc/testsuite/g++.target/i386/pr85334-1.C 
> b/gcc/testsuite/g++.target/i386/pr85334-1.C
> new file mode 100644
> index 000..3c5ccad1714
> --- /dev/null
> +++ b/gcc/testsuite/g++.target/i386/pr85334-1.C
> @@ -0,0 +1,55 @@
> +// { dg-do run }
> +// { dg-require-effective-target cet }
> +// { dg-additional-options "-fexceptions -fnon-call-exceptions 
> -fcf-protection" }
> +
> +// Delta between numbers of call stacks of pr85334-1.C and pr85334-2.C is 1.
> +
> +#include 
> +#include 
> +
> +void sighandler (int signo, siginfo_t * si, void * uc)
> +{
> +  throw (5);
> +}
> +
> +char *
> +__attribute ((noinline, noclone))
> +dosegv ()
> +{
> +  * ((volatile int *)0) = 12;
> +  return 0;
> +}
> +
> +int
> +__attribute ((noinline, noclone))
> +func2 ()
> +{
> +  try {
> +dosegv ();
> +  }
> +  catch (int x) {
> +return (x != 5);
> +  }
> +  return 1;
> +}
> +
> +int
> +__attribute ((noinline, noclone))
> +func1 ()
> +{
> +  return func2 ();
> +}
> +
> +int main ()
> +{
> +  struct sigaction sa;
> +  int status;
> +
> +  sa.sa_sigaction = sighandler;
> +  sa.sa_flags = SA_SIGINFO;
> +
> +  status = sigaction (SIGSEGV, & sa, NULL);
> +  status = sigaction (SIGBUS, & sa, NULL);
> +
> +  return func1 ();
> +}
> diff --git a/gcc/testsuite/g++.target/i386/pr85334-2.C 
> b/gcc/testsuite/g++.target/i386/pr85334-2.C
> new file mode 100644
> index 000..e2b5afe78cb
> --- /dev/null
> +++ b/gcc/testsuite/g++.target/i386/pr85334-2.C
> @@ -0,0 +1,48 @@
> +// { dg-do run }
> +// { dg-require-effective-target cet }
> +// { dg-additional-options "-fexceptions -fnon-call-exceptions 
> -fcf-protection" }
> +
> +// Delta between numbers of call stacks of pr85334-1.C and pr85334-2.C is 1.
> +
> +#include 
> +#include 
> +
> +void sighandler (int signo, siginfo_t * si, void * uc)
> +{
> +  throw (5);
> +}
> +
> +char *
> +__attribute ((noinline, noclone))
> +dosegv ()
> +{
> +  * ((volatile int *)0) = 12;
> +  return 0;
> +}
> +
> +int
> +__attribute ((noinline, noclone))
> +func1 ()
> +{
> +  try {
> +dosegv ();
> +  }
> +  catch (int x) {
> +return (x != 5);
> +  }
> +  return 1;
> +}
> +
> +int main ()
> +{
> +  struct sigaction sa;
> +  int status;
> +
> +  sa.sa_sigaction = sighandler;
> +  sa.sa_flags = SA_SIGINFO;
> +
> +  status = sigaction (SIGSEGV, & sa, NULL);
> +  status = sigaction (SIGBUS, & sa, NULL);
> +
> +  return func1 ();
> +}
> diff --git a/libgcc/config/i386/shadow-stack-unwind.h 
> b/libgcc/config/i386/shadow-stack-unwind.h
> index a0244d2ee70..a5f9235b146 100644
> --- a/libgcc/config/i386/shadow-stack-unwind.h
> +++ b/libgcc/config/i386/shadow-stack-unwind.h
> @@ -49,3 +49,46 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  
> If not, see
> }   \
>  }  \
>  while (0)
> +
> +/* Linux CET kernel places a restore token on shadow stack for signal
> +   handler to enhance security.  The restore token is 8 byte and aligned
> +   to 8 bytes.  It is usually transparent to user programs since kernel
> +   will pop the restore token when signal handler returns.  

[committed] testsuite: Fix target selector for pr91333.c

[Uros Bizjak]

Fix target selector for pr91333.c

* gcc.target/i386/pr91333.c (dg-do): Fix target selector.

Tested on x86_64-linux-gnu {,-m32}.

Uros.
diff --git a/gcc/testsuite/gcc.target/i386/pr91333.c 
b/gcc/testsuite/gcc.target/i386/pr91333.c
index 269491202ae..2bdff871024 100644
--- a/gcc/testsuite/gcc.target/i386/pr91333.c
+++ b/gcc/testsuite/gcc.target/i386/pr91333.c
@@ -1,4 +1,4 @@
-/* { dg-do compile { target x86_64-*-* } } */
+/* { dg-do compile { target { ! ia32 } } } */
 /* { dg-options "-O2 -mavx" } */
 /* { dg-final { scan-assembler-times "vmovapd|vmovsd" 3 } } */
 

Re: [committed] libstdc++: Fix BUILTIN-PTR-CMP helpers

[Jonathan Wakely]

On 09/02/20 13:56 +, Jonathan Wakely wrote:

The helpers that implement BUILTIN-PTR-CMP do not currently check if the
arguments are actually comparable, so the concept is true when it
shouldn't be.

Since we're trying to test for an unambiguous conversion to pointers, we
can also require that it returns bool, because the built-in comparisons
for pointers do return bool.

* include/bits/range_cmp.h (__detail::__eq_builtin_ptr_cmp): Require
equality comparison to be valid and return bool.
(__detail::__less_builtin_ptr_cmp): Likewise for less-than comparison.
* testsuite/20_util/function_objects/range.cmp/equal_to.cc: Check
type with ambiguous conversion to fundamental types.
* testsuite/20_util/function_objects/range.cmp/less.cc: Likewise.


This fixes the new comments I added to those tests.

Tested x86_64-linux, committed to master.


commit 38660e87f01dbe15082e83cc4842d66f8d292ba0
Author: Jonathan Wakely 
Date:   Sun Feb 9 13:58:16 2020 +

libstdc++: Fix names of types in comment

* testsuite/20_util/function_objects/range.cmp/equal_to.cc: Fix
comment.
* testsuite/20_util/function_objects/range.cmp/less.ccL Likewise.

diff --git a/libstdc++-v3/testsuite/20_util/function_objects/range.cmp/equal_to.cc b/libstdc++-v3/testsuite/20_util/function_objects/range.cmp/equal_to.cc
index 34f8ee5aca4..2d87b60a8ea 100644
--- a/libstdc++-v3/testsuite/20_util/function_objects/range.cmp/equal_to.cc
+++ b/libstdc++-v3/testsuite/20_util/function_objects/range.cmp/equal_to.cc
@@ -75,7 +75,7 @@ struct Y
   operator int() const;
 };
 
-// X{} == X{} is ambiguous so ranges::equal_to{}(X{}, X{}) should be invalid
+// Y{} == Y{} is ambiguous so ranges::equal_to{}(Y{}, Y{}) should be invalid
 static_assert( !std::is_invocable_v );
 
 int
diff --git a/libstdc++-v3/testsuite/20_util/function_objects/range.cmp/less.cc b/libstdc++-v3/testsuite/20_util/function_objects/range.cmp/less.cc
index bf7d600e7fe..8c04ad4ea6b 100644
--- a/libstdc++-v3/testsuite/20_util/function_objects/range.cmp/less.cc
+++ b/libstdc++-v3/testsuite/20_util/function_objects/range.cmp/less.cc
@@ -80,7 +80,7 @@ struct Y
   operator int() const;
 };
 
-// X{} == X{} is ambiguous so ranges::less{}(X{}, X{}) should be invalid
+// Y{} == Y{} is ambiguous so ranges::less{}(Y{}, Y{}) should be invalid
 static_assert( !std::is_invocable_v );
 
 int

[committed] libstdc++: Fix non-ASCII characters in comment

[Jonathan Wakely]

* include/std/ranges: Fix non-ASCII characters in comment.

Tested x86_64-linux, committed to master.

commit 97a7c22955435c0466ea96c6d77d2a71b2ae1277
Author: Jonathan Wakely 
Date:   Sun Feb 9 13:54:32 2020 +

libstdc++: Fix non-ASCII characters in comment

* include/std/ranges: Fix non-ASCII characters in comment.

diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
index 891ecf75eff..970e904bddd 100644
--- a/libstdc++-v3/include/std/ranges
+++ b/libstdc++-v3/include/std/ranges
@@ -2590,8 +2590,8 @@ namespace views
 
  // XXX: [24.7.11.3.1]
  //  Many of the following specifications refer to the notional member
- //  current of outer-iterator.  current is equivalent to current_­ if
- //  V models forward_range, and parent_->current_­ otherwise.
+ //  current of outer-iterator.  current is equivalent to current_ if
+ //  V models forward_range, and parent_->current_ otherwise.
  constexpr auto&
  __current()
  {

[committed] libstdc++: Fix BUILTIN-PTR-CMP helpers

[Jonathan Wakely]

The helpers that implement BUILTIN-PTR-CMP do not currently check if the
arguments are actually comparable, so the concept is true when it
shouldn't be.

Since we're trying to test for an unambiguous conversion to pointers, we
can also require that it returns bool, because the built-in comparisons
for pointers do return bool.

* include/bits/range_cmp.h (__detail::__eq_builtin_ptr_cmp): Require
equality comparison to be valid and return bool.
(__detail::__less_builtin_ptr_cmp): Likewise for less-than comparison.
* testsuite/20_util/function_objects/range.cmp/equal_to.cc: Check
type with ambiguous conversion to fundamental types.
* testsuite/20_util/function_objects/range.cmp/less.cc: Likewise.

Tested powerpc64le-linux, committed to master.


commit dcda050e6c3c3726cb14109674053f83cae330be
Author: Jonathan Wakely 
Date:   Sun Feb 9 13:37:43 2020 +

libstdc++: Fix BUILTIN-PTR-CMP helpers

The helpers that implement BUILTIN-PTR-CMP do not currently check if the
arguments are actually comparable, so the concept is true when it
shouldn't be.

Since we're trying to test for an unambiguous conversion to pointers, we
can also require that it returns bool, because the built-in comparisons
for pointers do return bool.

* include/bits/range_cmp.h (__detail::__eq_builtin_ptr_cmp): Require
equality comparison to be valid and return bool.
(__detail::__less_builtin_ptr_cmp): Likewise for less-than 
comparison.
* testsuite/20_util/function_objects/range.cmp/equal_to.cc: Check
type with ambiguous conversion to fundamental types.
* testsuite/20_util/function_objects/range.cmp/less.cc: Likewise.

diff --git a/libstdc++-v3/include/bits/range_cmp.h 
b/libstdc++-v3/include/bits/range_cmp.h
index e0385035543..571ba7f9555 100644
--- a/libstdc++-v3/include/bits/range_cmp.h
+++ b/libstdc++-v3/include/bits/range_cmp.h
@@ -62,7 +62,8 @@ namespace ranges
 // BUILTIN-PTR-CMP(T, ==, U)
 template
   concept __eq_builtin_ptr_cmp
-   = convertible_to<_Tp, const volatile void*>
+   = requires (_Tp&& __t, _Up&& __u) { { __t == __u } -> same_as; }
+ && convertible_to<_Tp, const volatile void*>
  && convertible_to<_Up, const volatile void*>
  && (! requires(_Tp&& __t, _Up&& __u)
  { operator==(std::forward<_Tp>(__t), std::forward<_Up>(__u)); }
@@ -73,7 +74,8 @@ namespace ranges
 // BUILTIN-PTR-CMP(T, <, U)
 template
   concept __less_builtin_ptr_cmp
-   = convertible_to<_Tp, const volatile void*>
+   = requires (_Tp&& __t, _Up&& __u) { { __t < __u } -> same_as; }
+ && convertible_to<_Tp, const volatile void*>
  && convertible_to<_Up, const volatile void*>
  && (! requires(_Tp&& __t, _Up&& __u)
  { operator<(std::forward<_Tp>(__t), std::forward<_Up>(__u)); }
diff --git 
a/libstdc++-v3/testsuite/20_util/function_objects/range.cmp/equal_to.cc 
b/libstdc++-v3/testsuite/20_util/function_objects/range.cmp/equal_to.cc
index 39bc984c508..34f8ee5aca4 100644
--- a/libstdc++-v3/testsuite/20_util/function_objects/range.cmp/equal_to.cc
+++ b/libstdc++-v3/testsuite/20_util/function_objects/range.cmp/equal_to.cc
@@ -69,6 +69,15 @@ test02()
   VERIFY( f(x, x) );
 }
 
+struct Y
+{
+  operator void*() const;
+  operator int() const;
+};
+
+// X{} == X{} is ambiguous so ranges::equal_to{}(X{}, X{}) should be invalid
+static_assert( !std::is_invocable_v );
+
 int
 main()
 {
diff --git a/libstdc++-v3/testsuite/20_util/function_objects/range.cmp/less.cc 
b/libstdc++-v3/testsuite/20_util/function_objects/range.cmp/less.cc
index 978894d5fc0..bf7d600e7fe 100644
--- a/libstdc++-v3/testsuite/20_util/function_objects/range.cmp/less.cc
+++ b/libstdc++-v3/testsuite/20_util/function_objects/range.cmp/less.cc
@@ -74,6 +74,15 @@ test02()
   VERIFY( ! f(x, x) );
 }
 
+struct Y
+{
+  operator void*() const;
+  operator int() const;
+};
+
+// X{} == X{} is ambiguous so ranges::less{}(X{}, X{}) should be invalid
+static_assert( !std::is_invocable_v );
+
 int
 main()
 {

Re: [PATCH] c++: Fix ICE with template codes in check_narrowing [PR91465]

[Jason Merrill]

On 2/6/20 7:30 PM, Marek Polacek wrote:

In ed4f2c001a883b2456fc607a33f1c59f9c4ee65d I changed the call to
fold_non_dependent_expr in check_narrowing to maybe_constant_value.
That was the wrong thing to do as these tests show: check_narrowing
bails out for dependent expressions but we can still have template
codes like CAST_EXPR that don't have anything dependent in it so are
considered non-dependent.  But cxx_eval_* don't grok template codes,
so we need to call fold_non_dependent_expr instead which knows what
to do with template codes.  (I fully accept a "told you so".)

I'm passing tf_none to it, otherwise we'd emit a bogus error for
constexpr-ex4.C: there INIT is "A::operator int(&a)" and while
instantiating this CALL_EXPR (in a template) we call finish_call_expr
and that sees a BASELINK and so emits a new dummy object for 'this',
and then we complain about the wrong number of arguments, because now
we basically have two 'this's.  Which is exactly the problem I saw
recently in c++/92948.


Yeah, the problem continues to be that build_converted_constant_expr is 
breaking the boundary between template and non-template codes: 
convert_like_real produces trees that aren't suitable for later 
substitution, so substituting them breaks.  Perhaps if we're looking at 
a non-dependent constant expression in a template, 
build_converted_constant_expr should instantiate_non_dependent_expr, 
pass the result to convert_like, and then if successful throw away the 
result in favor of an IMPLICIT_CONV_EXPR.



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




PR c++/91465 - ICE with template codes in check_narrowing.
* typeck2.c (check_narrowing): Call fold_non_dependent_expr
instead of maybe_constant_value.

* g++.dg/cpp0x/pr91465.C: New test.
* g++.dg/cpp1z/pr91465.C: New test.
---
  gcc/cp/typeck2.c |  4 +++-
  gcc/testsuite/g++.dg/cpp0x/pr91465.C | 16 
  gcc/testsuite/g++.dg/cpp1z/pr91465.C | 10 ++
  3 files changed, 29 insertions(+), 1 deletion(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp0x/pr91465.C
  create mode 100644 gcc/testsuite/g++.dg/cpp1z/pr91465.C

diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index 371b203c29b..8f8e9703ac8 100644
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -981,7 +981,9 @@ check_narrowing (tree type, tree init, tsubst_flags_t 
complain,
return ok;
  }
  
-  init = maybe_constant_value (init);

+  init = fold_non_dependent_expr (init, tf_none);
+  if (init == error_mark_node)
+return ok;
  
/* If we were asked to only check constants, return early.  */

if (const_only && !TREE_CONSTANT (init))
diff --git a/gcc/testsuite/g++.dg/cpp0x/pr91465.C 
b/gcc/testsuite/g++.dg/cpp0x/pr91465.C
new file mode 100644
index 000..e2021aa13e1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/pr91465.C
@@ -0,0 +1,16 @@
+// PR c++/91465 - ICE with template codes in check_narrowing.
+// { dg-do compile { target c++11 } }
+
+enum class D { X };
+enum class S { Z };
+
+D foo(S) { return D{}; }
+D foo(double) { return D{}; }
+
+template 
+struct Bar {
+  D baz(S s)
+  {
+return D{foo(s)};
+  }
+};
diff --git a/gcc/testsuite/g++.dg/cpp1z/pr91465.C 
b/gcc/testsuite/g++.dg/cpp1z/pr91465.C
new file mode 100644
index 000..5b1205349d0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/pr91465.C
@@ -0,0 +1,10 @@
+// PR c++/91465 - ICE with template codes in check_narrowing.
+// { dg-do compile { target c++17 } }
+
+enum class E { Z };
+
+template 
+void foo(F)
+{
+  E{char(0)};
+}

base-commit: cb273d81a45092ceee793f0357526e291f03c7b7

Re: [PATCH] c++: Fix ICE during constexpr virtual call evaluation [PR93633]

[Jason Merrill]

On 2/9/20 2:27 AM, Jakub Jelinek wrote:

Hi!

The first (valid) testcase ICEs because for
   A *a = new B ();
   a->foo (); // virtual method call
we actually see &heap  and the "heap " objects don't have the class or
whatever else type was used in new expression, but an array type containing
one (or more of those for array new) and so when using TYPE_BINFO (objtype)
on it we ICE.
This patch handles this special case, and otherwise punts (as shown e.g. in
the second testcase, where because the heap object is already deleted,
we don't really want to allow it to be used.

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


OK.


2020-02-08  Jakub Jelinek  

PR c++/93633
* constexpr.c (cxx_eval_constant_expression): If obj is heap var with
ARRAY_TYPE, use the element type.  Punt if objtype after that is not
a class type.

* g++.dg/cpp2a/constexpr-new11.C: New test.
* g++.dg/cpp2a/constexpr-new12.C: New test.
* g++.dg/cpp2a/constexpr-new13.C: New test.

--- gcc/cp/constexpr.c.jj   2020-02-08 10:58:15.434064005 +0100
+++ gcc/cp/constexpr.c  2020-02-08 14:15:55.356768622 +0100
@@ -6027,6 +6027,17 @@ cxx_eval_constant_expression (const cons
   && DECL_FIELD_IS_BASE (TREE_OPERAND (obj, 1)))
  obj = TREE_OPERAND (obj, 0);
tree objtype = TREE_TYPE (obj);
+   if (VAR_P (obj)
+   && DECL_NAME (obj) == heap_identifier
+   && TREE_CODE (objtype) == ARRAY_TYPE)
+ objtype = TREE_TYPE (objtype);
+   if (!CLASS_TYPE_P (objtype))
+ {
+   if (!ctx->quiet)
+ error_at (loc, "expression %qE is not a constant expression", t);
+   *non_constant_p = true;
+   return t;
+ }
/* Find the function decl in the virtual functions list.  TOKEN is
   the DECL_VINDEX that says which function we're looking for.  */
tree virtuals = BINFO_VIRTUALS (TYPE_BINFO (objtype));
--- gcc/testsuite/g++.dg/cpp2a/constexpr-new11.C.jj 2020-02-08 
14:22:34.740789172 +0100
+++ gcc/testsuite/g++.dg/cpp2a/constexpr-new11.C2020-02-08 
14:21:50.689448695 +0100
@@ -0,0 +1,31 @@
+// PR c++/93633
+// { dg-do compile { target c++2a } }
+
+struct A {
+  constexpr A () : a (0) {}
+  constexpr virtual int foo () { return 1 + a * 4; }
+  int a;
+};
+
+struct B : A {
+  constexpr B () : b (0) {}
+  constexpr virtual int foo () { return 0 + b * 4; }
+  int b;
+};
+
+constexpr int
+foo ()
+{
+  A *a = new B ();
+  a->a = 4;
+  int r = a->foo ();
+  delete a;
+  return r;
+}
+
+int
+main ()
+{
+  constexpr auto a = foo ();
+  static_assert (a == 0);
+}
--- gcc/testsuite/g++.dg/cpp2a/constexpr-new12.C.jj 2020-02-08 
14:22:38.043739717 +0100
+++ gcc/testsuite/g++.dg/cpp2a/constexpr-new12.C2020-02-08 
14:24:04.119451025 +0100
@@ -0,0 +1,26 @@
+// PR c++/93633
+// { dg-do compile { target c++2a } }
+
+struct A {
+  constexpr A () : a (0) {}
+  constexpr virtual int foo () { return 1 + a * 4; }
+  int a;
+};
+
+struct B : A {
+  constexpr B () : b (0) {}
+  constexpr virtual int foo () { return 0 + b * 4; }
+  int b;
+};
+
+constexpr int
+foo ()
+{
+  A *a = new B ();
+  a->a = 4;
+  delete a;
+  int r = a->foo ();
+  return r;
+}
+
+constexpr auto a = foo (); // { dg-error "is not a constant expression" }
--- gcc/testsuite/g++.dg/cpp2a/constexpr-new13.C.jj 2020-02-08 
14:22:41.060694553 +0100
+++ gcc/testsuite/g++.dg/cpp2a/constexpr-new13.C2020-02-08 
14:25:38.783033750 +0100
@@ -0,0 +1,26 @@
+// PR c++/93633
+// { dg-do compile { target c++2a } }
+
+struct A {
+  constexpr A () : a (0) {}
+  virtual int foo () { return 1 + a * 4; }
+  int a;
+};
+
+struct B : A {
+  constexpr B () : b (0) {}
+  virtual int foo () { return 0 + b * 4; } // { dg-message "declared here" 
}
+  int b;
+};
+
+constexpr int
+foo ()
+{
+  A *a = new B ();
+  a->a = 4;
+  int r = a->foo ();// { dg-error "call to non-.constexpr. function" }
+  delete a;
+  return r;
+}
+
+constexpr auto a = foo ();

Jakub