Re: [PATCH 1/5] x86: Add -mindirect-branch=

[Markus Trippelsdorf]

On 2018.01.14 at 11:46 +0100, Jan Hubicka wrote:
> > gcc/
> >
> > * config/i386/i386-opts.h (indirect_branch): New.
> > * config/i386/i386-protos.h (ix86_output_indirect_jmp): Likewise.
> > * config/i386/i386.c (ix86_using_red_zone): Disallow red-zone
> > with local indirect jump when converting indirect call and jump.
> > (ix86_set_indirect_branch_type): New.
> > (ix86_set_current_function): Call ix86_set_indirect_branch_type.
> > (indirectlabelno): New.
> > (indirect_thunk_needed): Likewise.
> > (indirect_thunk_bnd_needed): Likewise.
> > (indirect_thunks_used): Likewise.
> > (indirect_thunks_bnd_used): Likewise.
> > (INDIRECT_LABEL): Likewise.
> > (indirect_thunk_name): Likewise.
> > (output_indirect_thunk): Likewise.
> > (output_indirect_thunk_function): Likewise.
> > (ix86_output_indirect_branch): Likewise.
> > (ix86_output_indirect_jmp): Likewise.
> > (ix86_code_end): Call output_indirect_thunk_function if needed.
> > (ix86_output_call_insn): Call ix86_output_indirect_branch if
> > needed.
> > (ix86_handle_fndecl_attribute): Handle indirect_branch.
> > (ix86_attribute_table): Add indirect_branch.
> > * config/i386/i386.h (machine_function): Add indirect_branch_type
> > and has_local_indirect_jump.
> > * config/i386/i386.md (indirect_jump): Set has_local_indirect_jump
> > to true.
> > (tablejump): Likewise.
> > (*indirect_jump): Use ix86_output_indirect_jmp.
> > (*tablejump_1): Likewise.
> > (simple_return_indirect_internal): Likewise.
> > * config/i386/i386.opt (mindirect-branch=): New option.
> > (indirect_branch): New.
> > (keep): Likewise.
> > (thunk): Likewise.
> > (thunk-inline): Likewise.
> > (thunk-extern): Likewise.
> > * doc/extend.texi: Document indirect_branch function attribute.
> > * doc/invoke.texi: Document -mindirect-branch= option.
> > +
> > +  /* Pause .  */
> > +  fprintf (asm_out_file, "\tpause\n");
>
> OK, but please prepare incremental patches to choose between pause and lefence
> as needed for AMD CPUs and check for large code model.

Why not use both? That would make everybody happy, no?

 pause
 lfence
 jmp

-- 
Markus

Re: [PATCH] Fix failure building LLVM with location wrapper nodes (PR c++/83799)

[Markus Trippelsdorf]

On 2018.01.12 at 09:07 +0100, Markus Trippelsdorf wrote:
> On 2018.01.11 at 18:21 -0500, David Malcolm wrote:
> > diff --git a/gcc/testsuite/g++.dg/wrappers/pr83799.C 
> > b/gcc/testsuite/g++.dg/wrappers/pr83799.C
> > new file mode 100644
> > index 000..f93c0ae
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/wrappers/pr83799.C
> > @@ -0,0 +1,18 @@
> > +class DataLayout;
> > +class TargetLoweringBase {
> > +  void getTypeLegalizationCost(const DataLayout ) const;
> > +};
> > +class TargetTransformInfoImplBase {
> > +  const DataLayout  // this line isn't actually needed to reproduce 
> > the issue
> > +};
> > +template 
> > +class TargetTransformInfoImplCRTPBase : public TargetTransformInfoImplBase 
> > {};
> > +template 
> > +class BasicTTIImplBase : public TargetTransformInfoImplCRTPBase {
> > +  const TargetLoweringBase *getTLI() const;
> > +  using TargetTransformInfoImplBase::DL;
> > +  void getArithmeticInstrCost() {
> > +const TargetLoweringBase *TLI = getTLI();
> > +TLI->getTypeLegalizationCost(DL);
> > +  }
> > +};
> 
> Thanks for the fix. Minor nit:
> Please make TargetLoweringBase and TargetLoweringBase a struct instead
 TargetTransformInfoImplBase
-- 
Markus

Re: [PATCH] Fix failure building LLVM with location wrapper nodes (PR c++/83799)

[Markus Trippelsdorf]

On 2018.01.11 at 18:21 -0500, David Malcolm wrote:
> diff --git a/gcc/testsuite/g++.dg/wrappers/pr83799.C 
> b/gcc/testsuite/g++.dg/wrappers/pr83799.C
> new file mode 100644
> index 000..f93c0ae
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/wrappers/pr83799.C
> @@ -0,0 +1,18 @@
> +class DataLayout;
> +class TargetLoweringBase {
> +  void getTypeLegalizationCost(const DataLayout ) const;
> +};
> +class TargetTransformInfoImplBase {
> +  const DataLayout  // this line isn't actually needed to reproduce the 
> issue
> +};
> +template 
> +class TargetTransformInfoImplCRTPBase : public TargetTransformInfoImplBase 
> {};
> +template 
> +class BasicTTIImplBase : public TargetTransformInfoImplCRTPBase {
> +  const TargetLoweringBase *getTLI() const;
> +  using TargetTransformInfoImplBase::DL;
> +  void getArithmeticInstrCost() {
> +const TargetLoweringBase *TLI = getTLI();
> +TLI->getTypeLegalizationCost(DL);
> +  }
> +};

Thanks for the fix. Minor nit:
Please make TargetLoweringBase and TargetLoweringBase a struct instead
of a class, to prevent illegal access of private members.

-- 
Markus

Re: [PATCH 0/5] x86: CVE-2017-5715, aka Spectre

[Markus Trippelsdorf]

On 2018.01.07 at 21:07 -0700, Sandra Loosemore wrote:
> On 01/07/2018 03:58 PM, H.J. Lu wrote:
> > This set of patches for GCC 8 mitigates variant #2 of the speculative 
> > execution
> > vulnerabilities on x86 processors identified by CVE-2017-5715, aka Spectre. 
> >  They
> > convert indirect branches to call and return thunks to avoid speculative 
> > execution
> > via indirect call and jmp.
> 
> I have a general documentation issue with all the new command-line 
> options and attributes added by this patch set:  the documentation is 
> very implementor-speaky and doesn't explain what user-level problem 
> they're trying to solve.
> 
> E.g. to take just one example
> 
> > +@item function_return("@var{choice}")
> > +@cindex @code{function_return} function attribute, x86
> > +On x86 targets, the @code{function_return} attribute causes the compiler
> > +to convert function return with @var{choice}.  @samp{keep} keeps function
> > +return unmodified.  @samp{thunk} converts function return to call and
> > +return thunk.  @samp{thunk-inline} converts function return to inlined
> > +call and return thunk.  @samp{thunk-extern} converts function return to
> > +external call and return thunk provided in a separate object file.
> 
> Why would you want to mess with call and return code generation in this 
> way?  The documentation doesn't say.
> 
> For thunk-extern, is the programmer supposed to provide the thunk?  How 
> would you go about implementing the missing bit of code?  What should it 
> do?  I'm compiler implementor and I wouldn't even know how to use this 
> feature by reading the manual; how would an ordinary application 
> programmer who isn't familiar with GCC internals know how to use it?
> 
> If the goal here is to tell GCC to produce code that is protected 
> against the Spectre vulnerability, perhaps simplify this to adding just 
> one option that controls all the things you've given separate options 
> and attributes for?

Also it would be good to coordinate with the LLVM guys: They currently
use -mretpoline and -mretpoline_external_thunk. 
I agree with Sandra that a single master option like -mretpoline would
be better.

-- 
Markus

Re: [PATCH][i386] Correct imul (r64) latency for modern Intel CPUs

[Markus Trippelsdorf]

On 2017.12.17 at 12:26 +0100, Jan Hubicka wrote:
> > Since Nehalem the 64bit multiplication latency is three cycles, not
> > four. So update the costs to reflect reality.
> 
> I looked into the imul latencies and was a bit confused, decided to look
> into it later and forgot.
> 
> Agner Fog's table http://www.agner.org/optimize/instruction_tables.pdf
> lists for sandybridge-skylake costs of 3 cycles for everything except for
> 16bit (which is 4 cycles).
> 
> For earlier chips this is more varying.
>  Merom (core duo) and Wolfdale is 3 or everything except for 5 for 64bit.
>  Nehalem is 3 for everything however 64bit has smaller throughput.
> 
> We do not have spearate table for sandybridge, but I guess we do not care
> that much about older cores (Vladimir is still running a tester on one,
> so we do benchmark them).
> 
> So I guess the change is OK.  I do not see this as very good reason to split
> the sandybridge table out, but perhaps drop a comment in case we will want
> to do it in future.

Thanks. Here is what I have committed (r255760):

diff --git a/gcc/config/i386/x86-tune-costs.h b/gcc/config/i386/x86-tune-costs.h
index 648219338308..477e478f1f77 100644
--- a/gcc/config/i386/x86-tune-costs.h
+++ b/gcc/config/i386/x86-tune-costs.h
@@ -1538,8 +1538,8 @@ struct processor_costs skylake_cost = {
   {COSTS_N_INSNS (3),  /* cost of starting multiply for QI */
COSTS_N_INSNS (4),  /*   HI */
COSTS_N_INSNS (3),  /*   SI */
-   COSTS_N_INSNS (4),  /*   DI */
-   COSTS_N_INSNS (4)}, /*other */
+   COSTS_N_INSNS (3),  /*   DI */
+   COSTS_N_INSNS (3)}, /*other */
   0,   /* cost of multiply per each bit set */
   /* Expanding div/mod currently doesn't consider parallelism. So the cost
  model is not realistic. We compensate by increasing the latencies a bit.  
*/
@@ -2341,8 +2341,9 @@ struct processor_costs core_cost = {
   {COSTS_N_INSNS (3),  /* cost of starting multiply for QI */
COSTS_N_INSNS (4),  /*   HI */
COSTS_N_INSNS (3),  /*   SI */
-   COSTS_N_INSNS (4),  /*   DI */
-   COSTS_N_INSNS (4)}, /*other */
+   /* Here we tune for Sandybridge or newer.  */
+   COSTS_N_INSNS (3),  /*   DI */
+   COSTS_N_INSNS (3)}, /*other */
   0,   /* cost of multiply per each bit set */
   /* Expanding div/mod currently doesn't consider parallelism. So the cost
  model is not realistic. We compensate by increasing the latencies a bit.  
*/
diff --git a/gcc/testsuite/gcc.target/i386/wmul-3.c 
b/gcc/testsuite/gcc.target/i386/wmul-3.c
new file mode 100644
index ..5f1619076386
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/wmul-3.c
@@ -0,0 +1,66 @@
+/* { dg-do compile { target { ! ia32 } } } */
+/* { dg-options "-O2 -march=sandybridge" } */
+
+#include 
+#include 
+
+static const char b100_tab[200] = {
+'0', '0', '0', '1', '0', '2', '0', '3', '0', '4',
+'0', '5', '0', '6', '0', '7', '0', '8', '0', '9',
+'1', '0', '1', '1', '1', '2', '1', '3', '1', '4',
+'1', '5', '1', '6', '1', '7', '1', '8', '1', '9',
+'2', '0', '2', '1', '2', '2', '2', '3', '2', '4',
+'2', '5', '2', '6', '2', '7', '2', '8', '2', '9',
+'3', '0', '3', '1', '3', '2', '3', '3', '3', '4',
+'3', '5', '3', '6', '3', '7', '3', '8', '3', '9',
+'4', '0', '4', '1', '4', '2', '4', '3', '4', '4',
+'4', '5', '4', '6', '4', '7', '4', '8', '4', '9',
+'5', '0', '5', '1', '5', '2', '5', '3', '5', '4',
+'5', '5', '5', '6', '5', '7', '5', '8', '5', '9',
+'6', '0', '6', '1', '6', '2', '6', '3', '6', '4',
+'6', '5', '6', '6', '6', '7', '6', '8', '6', '9',
+'7', '0', '7', '1', '7', '2', '7', '3', '7', '4',
+'7', '5', '7', '6', '7', '7', '7', '8', '7', '9',
+'8', '0', '8', '1', '8', '2', '8', '3', '8', '4',
+'8', '5', '8', '6', '8', '7', '8', '8', '8', '9',
+'9', '0', '9', '1', '9', '2', '9', '3', '9', '4',
+'9', '5', '9', '6', '9', '7', '9', '8', '9', '9',
+};
+
+void uint64_to_ascii_ta7_32_base100(uint64_t val, char *dst) {
+  const int64_t POW10_10 = ((int64_t)10) * 1000 * 1000 * 1000;
+  const uint64_t POW2_57_DIV_POW100_4 =
+  ((int64_t)(1) << 57) / 100 / 100 / 100 / 100 + 1;
+  const uint64_t MASK32 = ((int64_t)(1) << 32) - 1;
+  int64_t hix = val / POW10_10;
+  int64_t lox = val % POW10_10;
+  int64_t lor = lox & (uint64_t)(-2);
+  uint64_t hi = hix * POW2_57_DIV_POW100_4;
+  uint64_t lo = lor * POW2_57_DIV_POW100_4;
+  memcpy(dst + 0 * 

[PATCH][i386] Correct imul (r64) latency for modern Intel CPUs

[Markus Trippelsdorf]

Since Nehalem the 64bit multiplication latency is three cycles, not
four. So update the costs to reflect reality.

Tested on X86_64.
OK for trunk?

Thanks.

* x86-tune-costs.h (skylake_cost, core_cost): Decrease r64 multiply
latencies.

* gcc.target/i386/wmul-3.c: New test.

diff --git a/gcc/config/i386/x86-tune-costs.h b/gcc/config/i386/x86-tune-costs.h
index 648219338308..ddb47ba44056 100644
--- a/gcc/config/i386/x86-tune-costs.h
+++ b/gcc/config/i386/x86-tune-costs.h
@@ -1538,8 +1538,8 @@ struct processor_costs skylake_cost = {
   {COSTS_N_INSNS (3),  /* cost of starting multiply for QI */
COSTS_N_INSNS (4),  /*   HI */
COSTS_N_INSNS (3),  /*   SI */
-   COSTS_N_INSNS (4),  /*   DI */
-   COSTS_N_INSNS (4)}, /*other */
+   COSTS_N_INSNS (3),  /*   DI */
+   COSTS_N_INSNS (3)}, /*other */
   0,   /* cost of multiply per each bit set */
   /* Expanding div/mod currently doesn't consider parallelism. So the cost
  model is not realistic. We compensate by increasing the latencies a bit.  
*/
@@ -2341,8 +2341,8 @@ struct processor_costs core_cost = {
   {COSTS_N_INSNS (3),  /* cost of starting multiply for QI */
COSTS_N_INSNS (4),  /*   HI */
COSTS_N_INSNS (3),  /*   SI */
-   COSTS_N_INSNS (4),  /*   DI */
-   COSTS_N_INSNS (4)}, /*other */
+   COSTS_N_INSNS (3),  /*   DI */
+   COSTS_N_INSNS (3)}, /*other */
   0,   /* cost of multiply per each bit set */
   /* Expanding div/mod currently doesn't consider parallelism. So the cost
  model is not realistic. We compensate by increasing the latencies a bit.  
*/
diff --git a/gcc/testsuite/gcc.target/i386/wmul-3.c 
b/gcc/testsuite/gcc.target/i386/wmul-3.c
new file mode 100644
index ..66c077c2cc0d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/wmul-3.c
@@ -0,0 +1,66 @@
+/* { dg-do compile { target { ! ia32 } } } */
+/* { dg-options "-O2 -march=haswell" } */
+
+#include 
+#include 
+
+static const char b100_tab[200] = {
+'0', '0', '0', '1', '0', '2', '0', '3', '0', '4',
+'0', '5', '0', '6', '0', '7', '0', '8', '0', '9',
+'1', '0', '1', '1', '1', '2', '1', '3', '1', '4',
+'1', '5', '1', '6', '1', '7', '1', '8', '1', '9',
+'2', '0', '2', '1', '2', '2', '2', '3', '2', '4',
+'2', '5', '2', '6', '2', '7', '2', '8', '2', '9',
+'3', '0', '3', '1', '3', '2', '3', '3', '3', '4',
+'3', '5', '3', '6', '3', '7', '3', '8', '3', '9',
+'4', '0', '4', '1', '4', '2', '4', '3', '4', '4',
+'4', '5', '4', '6', '4', '7', '4', '8', '4', '9',
+'5', '0', '5', '1', '5', '2', '5', '3', '5', '4',
+'5', '5', '5', '6', '5', '7', '5', '8', '5', '9',
+'6', '0', '6', '1', '6', '2', '6', '3', '6', '4',
+'6', '5', '6', '6', '6', '7', '6', '8', '6', '9',
+'7', '0', '7', '1', '7', '2', '7', '3', '7', '4',
+'7', '5', '7', '6', '7', '7', '7', '8', '7', '9',
+'8', '0', '8', '1', '8', '2', '8', '3', '8', '4',
+'8', '5', '8', '6', '8', '7', '8', '8', '8', '9',
+'9', '0', '9', '1', '9', '2', '9', '3', '9', '4',
+'9', '5', '9', '6', '9', '7', '9', '8', '9', '9',
+};
+
+void uint64_to_ascii_ta7_32_base100(uint64_t val, char *dst) {
+  const int64_t POW10_10 = ((int64_t)10) * 1000 * 1000 * 1000;
+  const uint64_t POW2_57_DIV_POW100_4 =
+  ((int64_t)(1) << 57) / 100 / 100 / 100 / 100 + 1;
+  const uint64_t MASK32 = ((int64_t)(1) << 32) - 1;
+  int64_t hix = val / POW10_10;
+  int64_t lox = val % POW10_10;
+  int64_t lor = lox & (uint64_t)(-2);
+  uint64_t hi = hix * POW2_57_DIV_POW100_4;
+  uint64_t lo = lor * POW2_57_DIV_POW100_4;
+  memcpy(dst + 0 * 10 + 0, _tab[(hi >> 57) * 2], 2);
+  memcpy(dst + 1 * 10 + 0, _tab[(lo >> 57) * 2], 2);
+  hi = (hi >> 25) + 1;
+  lo = (lo >> 25) + 1;
+  hi = (hi & MASK32) * 100;
+  lo = (lo & MASK32) * 100;
+  memcpy(dst + 0 * 10 + 2, _tab[(hi >> 32) * 2], 2);
+  hi = (hi & MASK32) * 100;
+  memcpy(dst + 1 * 10 + 2, _tab[(lo >> 32) * 2], 2);
+  lo = (lo & MASK32) * 100;
+  memcpy(dst + 0 * 10 + 4, _tab[(hi >> 32) * 2], 2);
+  hi = (hi & MASK32) * 100;
+  memcpy(dst + 1 * 10 + 4, _tab[(lo >> 32) * 2], 2);
+  lo = (lo & MASK32) * 100;
+  memcpy(dst + 0 * 10 + 6, _tab[(hi >> 32) * 2], 2);
+  hi = (hi & MASK32) * 100;
+  memcpy(dst + 1 * 10 + 6, _tab[(lo >> 32) * 2], 2);
+  lo = (lo & MASK32) * 100;
+  hi >>= 32;
+  lo >>= 32;
+  lo = (lo & (-2)) | (lox & 1);
+  memcpy(dst + 0 * 10 + 8, _tab[hi * 2], 2);
+  memcpy(dst + 

[PATCH][i386] Fix PR83358 - increase divide/mod latencies a bit

[Markus Trippelsdorf]

As the testcase shows, trunk currently generates horrible code for
divisions used in tight loops. This happens because the algorithm
expanding div/mod doesn't take parallelism into account and this makes
the cost model unrealistic.
Fix the issue by increasing the estimated latencies a bit.

Tested on X86_64.
OK for trunk?
(I will keep an eye on the periodic SPEC testers.)

PR target/83358
* x86-tune-costs.h (skylake_cost, core_cost): Increase div/mod
latencies a bit.

diff --git a/gcc/config/i386/x86-tune-costs.h b/gcc/config/i386/x86-tune-costs.h
index 312467d9788f..648219338308 100644
--- a/gcc/config/i386/x86-tune-costs.h
+++ b/gcc/config/i386/x86-tune-costs.h
@@ -1541,9 +1541,11 @@ struct processor_costs skylake_cost = {
COSTS_N_INSNS (4),  /*   DI */
COSTS_N_INSNS (4)}, /*other */
   0,   /* cost of multiply per each bit set */
-  {COSTS_N_INSNS (8),  /* cost of a divide/mod for QI */
-   COSTS_N_INSNS (8),  /*  HI */
-   COSTS_N_INSNS (11), /*  SI */
+  /* Expanding div/mod currently doesn't consider parallelism. So the cost
+ model is not realistic. We compensate by increasing the latencies a bit.  
*/
+  {COSTS_N_INSNS (11), /* cost of a divide/mod for QI */
+   COSTS_N_INSNS (11), /*  HI */
+   COSTS_N_INSNS (14), /*  SI */
COSTS_N_INSNS (76), /*  DI */
COSTS_N_INSNS (76)},/*  
other */
   COSTS_N_INSNS (1),   /* cost of movsx */
@@ -2342,11 +2344,11 @@ struct processor_costs core_cost = {
COSTS_N_INSNS (4),  /*   DI */
COSTS_N_INSNS (4)}, /*other */
   0,   /* cost of multiply per each bit set */
-  {COSTS_N_INSNS (8),  /* cost of a divide/mod for QI */
-   COSTS_N_INSNS (8),  /*  HI */
-   /* 8-11 */
-   COSTS_N_INSNS (11), /*  SI */
-   /* 24-81 */
+  /* Expanding div/mod currently doesn't consider parallelism. So the cost
+ model is not realistic. We compensate by increasing the latencies a bit.  
*/
+  {COSTS_N_INSNS (11), /* cost of a divide/mod for QI */
+   COSTS_N_INSNS (11), /*  HI */
+   COSTS_N_INSNS (14), /*  SI */
COSTS_N_INSNS (81), /*  DI */
COSTS_N_INSNS (81)},/*  
other */
   COSTS_N_INSNS (1),   /* cost of movsx */
diff --git a/gcc/testsuite/gcc.target/i386/pr83358-1.c 
b/gcc/testsuite/gcc.target/i386/pr83358-1.c
new file mode 100644
index ..96427b2f56dd
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr83358-1.c
@@ -0,0 +1,31 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -mtune=core2" } */
+
+#include 
+
+void bin2ascii(uint64_t val, char *dst) {
+  const int64_t POW10_10 = ((int64_t)10) * 1000 * 1000 * 1000;
+  int64_t hix = val / POW10_10;
+  int64_t lox = val % POW10_10;
+  int32_t v0 = hix / 10;
+  int32_t v1 = hix % 10;
+  int32_t v2 = lox / 10;
+  int32_t v3 = lox % 10;
+  for (int i = 4; i != 0; --i) {
+dst[i + 0 * 5] = v0 % 10 + '0';
+v0 /= 10;
+dst[i + 1 * 5] = v1 % 10 + '0';
+v1 /= 10;
+dst[i + 2 * 5] = v2 % 10 + '0';
+v2 /= 10;
+dst[i + 3 * 5] = v3 % 10 + '0';
+v3 /= 10;
+  }
+  dst[0 * 5] = v0 + '0';
+  dst[1 * 5] = v1 + '0';
+  dst[2 * 5] = v2 + '0';
+  dst[3 * 5] = v3 + '0';
+  dst[4 * 5] = 0;
+}
+
+/* { dg-final { scan-assembler-not "idiv" } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr83358-2.c 
b/gcc/testsuite/gcc.target/i386/pr83358-2.c
new file mode 100644
index ..f6039bf72feb
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr83358-2.c
@@ -0,0 +1,31 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -mtune=skylake-avx512" } */
+
+#include 
+
+void bin2ascii(uint64_t val, char *dst) {
+  const int64_t POW10_10 = ((int64_t)10) * 1000 * 1000 * 1000;
+  int64_t hix = val / POW10_10;
+  int64_t lox = val % POW10_10;
+  int32_t v0 = hix / 10;
+  int32_t v1 = hix % 10;
+  int32_t v2 = lox / 10;
+  int32_t v3 = lox % 10;
+  for (int i = 4; i != 0; --i) {
+dst[i + 0 * 5] = v0 % 10 + '0';
+v0 /= 10;
+dst[i + 1 * 5] = v1 % 10 + '0';
+v1 /= 10;
+dst[i + 2 * 5] = v2 % 10 + '0';
+v2 /= 10;
+dst[i + 3 * 5] = v3 % 10 + '0';
+v3 /= 10;
+  }
+  dst[0 * 5] = v0 + '0';
+  dst[1 * 5] = v1 + '0';
+  dst[2 * 5] = v2 + '0';
+  dst[3 * 5] = v3 + '0';
+  dst[4 * 5] = 0;
+}
+

[PATCH] Fix PR82488 - signed integer overflow in expr.c

[Markus Trippelsdorf]

bootstrap-ubsan shows:
 gcc/expr.c:4103:17: runtime error: signed integer overflow: 0 - 
-9223372036854775808 cannot be represented in type 'long int'

Fix by handling the saw_unknown case earlier.

bootstrap-ubsan on X86_64 and ppc64le. Tested on ppc64le.

OK for trunk?

PR rtl-optimization/82488
* expr.c (fixup_args_size_notes): Avoid signed integer overflow.


diff --git a/gcc/expr.c b/gcc/expr.c
index ee07de5aaa44..e9d8555c9452 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -4100,10 +4100,13 @@ fixup_args_size_notes (rtx_insn *prev, rtx_insn *last, 
int end_args_size)
   if (STACK_GROWS_DOWNWARD)
this_delta = -(unsigned HOST_WIDE_INT) this_delta;
 
-  args_size -= this_delta;
+  if (saw_unknown)
+   args_size = INT_MIN;
+  else
+   args_size -= this_delta;
 }
 
-  return saw_unknown ? INT_MIN : args_size;
+  return args_size;
 }
 
 #ifdef PUSH_ROUNDING
-- 
Markus

[PATCH] Fix UB in hash-map.h

[Markus Trippelsdorf]

bootstrap-ubsan shows:
  gcc/hash-map.h:277:19: runtime error: member access within null pointer of 
type 'struct hash_map'

Fix the issue by returning early.
bootstrap-ubsan on X86_64 and ppc64le. Tested on ppc64le.

OK for trunk?

gcc/
* hash-map.h (gt_cleare_cache): Avoid UB.

diff --git a/gcc/hash-map.h b/gcc/hash-map.h
index 6b8365a9d0a6..e5630338491a 100644
--- a/gcc/hash-map.h
+++ b/gcc/hash-map.h
@@ -274,6 +274,8 @@ template
 static inline void
 gt_cleare_cache (hash_map *h)
 {
+  if (!h)
+return;
   gt_cleare_cache (>m_table);
 }
 
-- 
Markus

Re: RFA (hash-map): PATCH to support GTY((cache)) with hash_map

[Markus Trippelsdorf]

On 2017.11.14 at 13:32 +0100, Richard Biener wrote:
> On Fri, Sep 15, 2017 at 11:45 PM, Jason Merrill  wrote:
> > The hash_map interface is a lot more convenient than that of
> > hash_table for cases where it makes sense, but there hasn't been a way
> > to get the ggc_cache_remove behavior with a hash_map.  In other words,
> > not marking elements during the initial ggc marking phase, but maybe
> > marking them during the clear_caches phase based on keep_cache_entry.
> >
> > This patch implements that by:
> >
> > Adding a ggc_maybe_mx member function to ggc_remove, and overriding
> > that instead of ggc_mx in ggc_cache_remove.
> > Calling H::ggc_maybe_mx instead of H::ggc_mx in gt_ggc_mx (hash_table *).
> > Calling H::ggc_mx in gt_cleare_caches (hash_table *) rather than
> > relying on an extern declaration of a plain function that cannot be
> > declared for hash_map::hash_entry.
> > Adding ggc_maybe_mx and keep_cache_entry to hash_map::hash_entry.
> > Adding gt_cleare_cache for hash_map.
> > Adding a boolean constant to the hash-map traits indicating whether we
> > want the cache behavior above.
> >
> > I then define a typedef tree_cache_map to use this functionality, and
> > use it in a few places in the C++ front end.
> >
> > Tested x86_64-pc-linux-gnu, OK for trunk?
> 
> Ok.

The patch causes UB in some cases:
 
 gcc/hash-map.h:277:19: runtime error: member access within null pointer of 
type 'struct hash_map'

-- 
Markus

Re: Fix IPA profile updates in inlining and ipa-split

[Markus Trippelsdorf]

On 2017.11.18 at 00:39 +0100, Jan Hubicka wrote:
> Hi,
> this patch fixes remaining IPA profile updating issues I am aware of.  With
> this change there is 1 profile mismatch after inlining for tramp3d -Ofast
> and 112 of them in .optimized dump which is about 10 times less than before.
> I did not inspect them all but they seems mostly justified and not very
> important.
> 
> First patch adds new profile quality stage when global profile is 0 but not
> known precisely.  This is useful for bb partitioning where we do not want to
> move adjusted 0 to cold sections.
> 
> Second the patch adds little infrastructure for turning IPA info to local
> info (profile_count::combine_with_ipa_count) and commonizes all places that
> do this kind of operation.
> 
> Finally it fixes profile updating bug in ipa-split where entry and return
> blocks got wrong profile.
> 
> Bootstrapped/regtested x86_64-linux. I am profilebootstrapping overnight
> and plan to commit tomorrow.

This fixes the tramp3d compile time regression with LTO/PGO bootstrapped
gcc, that I have reported earlier. 
In fact gcc-8 now compiles tramp3d-v4 ~8.5% faster than gcc-7.

The patch also fixes PR83037 and PR83039.

-- 
Markus

Re: Drop frequencies from basic blocks

[Markus Trippelsdorf]

On 2017.11.07 at 00:12 +0100, Jan Hubicka wrote:
> > On 2017.11.05 at 11:55 +0100, Jan Hubicka wrote:
> > > > On 2017.11.03 at 16:48 +0100, Jan Hubicka wrote:
> > > > > this is updated patch which I have comitted after 
> > > > > profiledbootstrapping x86-64
> > > >
> > > > Unfortunately, compiling tramp3d-v4.cpp is 6-7% slower after this patch.
> > > > This happens with an LTO/PGO bootstrapped gcc using 
> > > > --enable-checking=release.
> > >
> > > our periodic testers has also picked up the change and there is no 
> > > compile time
> > > regression reported for tramp3d.
> > > https://gcc.opensuse.org/gcc-old/c++bench-czerny/tramp3d/
> > > so I would conclude that it is regression in LTO+PGO bootstrap.  I am 
> > > fixing one checking
> > > bug that may cause it (where we mix local and global profiles) so perhaps 
> > > it will go away
> > > afterwards.
> >
> > Just to confirm: pure PGO bootstrap is fine, e.g. on Ryzen:
> > (LTO/PGO) 17.65 sec ( +- 0.68% )
> > (PGO) 15.74 sec ( +- 0.27% )
>
> Thanks.  I have comitted the patch for inlining profile update bug, so with 
> some
> luck LTO/PGO may be fine again.

It got worse, unfortunately:

Pure PGO:
 Performance counter stats for '/home/trippels/gcc_8/usr/local/bin/g++ -w 
-Ofast tramp3d-v4.cpp' (4 runs):

  16213.529306  task-clock (msec) #0.999 CPUs utilized  
  ( +-  0.25% )
 1,387  context-switches  #0.086 K/sec  
  ( +-  0.17% )
 4  cpu-migrations#0.000 K/sec  
  ( +- 14.80% )
   261,764  page-faults   #0.016 M/sec  
  ( +-  0.03% )
62,633,457,222  cycles#3.863 GHz
  ( +-  0.20% )  (83.32%)
13,990,050,204  stalled-cycles-frontend   #   22.34% frontend cycles 
idle ( +-  0.51% )  (83.33%)
13,189,755,888  stalled-cycles-backend#   21.06% backend cycles 
idle  ( +-  0.04% )  (83.31%)
75,194,592,630  instructions  #1.20  insn per cycle
  #0.19  stalled cycles per 
insn  ( +-  0.03% )  (83.35%)
17,113,639,942  branches  # 1055.516 M/sec  
  ( +-  0.02% )  (83.38%)
   634,471,544  branch-misses #3.71% of all branches
  ( +-  0.07% )  (83.34%)

  16.226375499 seconds time elapsed 
 ( +-  0.24% )

LTO/PGO:
 Performance counter stats for '/home/trippels/gcc_8/usr/local/bin/g++ -w 
-Ofast tramp3d-v4.cpp' (4 runs):

  18622.496264  task-clock (msec) #0.999 CPUs utilized  
  ( +-  0.35% )
 1,592  context-switches  #0.086 K/sec  
  ( +-  0.32% )
 4  cpu-migrations#0.000 K/sec  
  ( +- 14.43% )
   261,370  page-faults   #0.014 M/sec  
  ( +-  0.12% )
71,849,030,564  cycles#3.858 GHz
  ( +-  0.08% )  (83.34%)
15,987,209,604  stalled-cycles-frontend   #   22.25% frontend cycles 
idle ( +-  0.47% )  (83.32%)
14,336,345,458  stalled-cycles-backend#   19.95% backend cycles 
idle  ( +-  0.05% )  (83.33%)
87,674,608,740  instructions  #1.22  insn per cycle
  #0.18  stalled cycles per 
insn  ( +-  0.01% )  (83.36%)
20,610,950,144  branches  # 1106.777 M/sec  
  ( +-  0.01% )  (83.35%)
   638,454,497  branch-misses #3.10% of all branches
  ( +-  0.08% )  (83.35%)

  18.644370559 seconds time elapsed 
 ( +-  0.38% )

--
Markus

Re: Drop frequencies from basic blocks

[Markus Trippelsdorf]

On 2017.11.05 at 11:55 +0100, Jan Hubicka wrote:
> > On 2017.11.03 at 16:48 +0100, Jan Hubicka wrote:
> > > this is updated patch which I have comitted after profiledbootstrapping 
> > > x86-64
> > 
> > Unfortunately, compiling tramp3d-v4.cpp is 6-7% slower after this patch.
> > This happens with an LTO/PGO bootstrapped gcc using 
> > --enable-checking=release.
> 
> our periodic testers has also picked up the change and there is no compile 
> time
> regression reported for tramp3d.
> https://gcc.opensuse.org/gcc-old/c++bench-czerny/tramp3d/
> so I would conclude that it is regression in LTO+PGO bootstrap.  I am fixing 
> one checking
> bug that may cause it (where we mix local and global profiles) so perhaps it 
> will go away
> afterwards.

Just to confirm: pure PGO bootstrap is fine, e.g. on Ryzen:
(LTO/PGO) 17.65 sec ( +- 0.68% )
(PGO) 15.74 sec ( +- 0.27% )

-- 
Markus

Re: Drop frequencies from basic blocks

[Markus Trippelsdorf]

On 2017.11.03 at 16:48 +0100, Jan Hubicka wrote:
> this is updated patch which I have comitted after profiledbootstrapping x86-64

Unfortunately, compiling tramp3d-v4.cpp is 6-7% slower after this patch.
This happens with an LTO/PGO bootstrapped gcc using --enable-checking=release.

On X86_64:

Before:
 Performance counter stats for 'g++ -w -Ofast tramp3d-v4.cpp' (4 runs):

  25040.360183  task-clock (msec) #1.000 CPUs utilized  
  ( +-  0.25% )
   650  context-switches  #0.026 K/sec  
  ( +- 76.87% )
 2  cpu-migrations#0.000 K/sec  
  ( +- 28.87% )
   268,141  page-faults   #0.011 M/sec  
  ( +-  0.01% )
80,210,085,167  cycles#3.203 GHz
  ( +-  0.26% )  (66.67%)
21,061,765,388  stalled-cycles-frontend   #   26.26% frontend cycles 
idle ( +-  0.37% )  (66.67%)
24,699,976,439  stalled-cycles-backend#   30.79% backend cycles 
idle  ( +-  0.57% )  (66.68%)
69,167,169,243  instructions  #0.86  insn per cycle
  #0.36  stalled cycles per 
insn  ( +-  0.05% )  (66.68%)
15,230,229,662  branches  #  608.227 M/sec  
  ( +-  0.06% )  (66.68%)
   986,612,296  branch-misses #6.48% of all branches
  ( +-  0.07% )  (66.68%)

  25.046439011 seconds time elapsed 
 ( +-  0.25% )

After:
 Performance counter stats for 'g++ -w -Ofast tramp3d-v4.cpp' (4 runs):

  26710.577065  task-clock (msec) #1.000 CPUs utilized  
  ( +-  0.27% )
   199  context-switches  #0.007 K/sec  
  ( +- 21.12% )
 2  cpu-migrations#0.000 K/sec  
  ( +- 14.29% )
   267,676  page-faults   #0.010 M/sec  
  ( +-  0.01% )
85,561,962,974  cycles#3.203 GHz
  ( +-  0.26% )  (66.66%)
19,581,827,643  stalled-cycles-frontend   #   22.89% frontend cycles 
idle ( +-  0.30% )  (66.66%)
26,056,535,726  stalled-cycles-backend#   30.45% backend cycles 
idle  ( +-  0.65% )  (66.68%)
77,222,167,966  instructions  #0.90  insn per cycle
  #0.34  stalled cycles per 
insn  ( +-  0.04% )  (66.68%)
17,471,652,187  branches  #  654.110 M/sec  
  ( +-  0.05% )  (66.69%)
 1,082,141,013  branch-misses #6.19% of all branches
  ( +-  0.04% )  (66.69%)

  26.713823720 seconds time elapsed 
 ( +-  0.27% )

==

On PPC64le:

Before:
 Performance counter stats for 'g++ -w -Ofast tramp3d-v4.cpp' (4 runs):

  24281.894597  task-clock (msec) #0.989 CPUs utilized  
  ( +-  1.85% )
   166  context-switches  #0.007 K/sec  
  ( +-  2.46% )
 5  cpu-migrations#0.000 K/sec  
  ( +- 18.03% )
52,908  page-faults   #0.002 M/sec  
  ( +- 11.61% )
84,939,354,171  cycles#3.498 GHz
  ( +-  1.82% )  (66.71%)
 4,680,693,343  stalled-cycles-frontend   #5.51% frontend cycles 
idle ( +-  8.75% )  (49.98%)
46,697,372,688  stalled-cycles-backend#   54.98% backend cycles 
idle  ( +-  2.06% )  (50.05%)
94,990,460,746  instructions  #1.12  insn per cycle
  #0.49  stalled cycles per 
insn  ( +-  0.10% )  (66.72%)
19,562,344,992  branches  #  805.635 M/sec  
  ( +-  0.07% )  (50.06%)
   807,701,262  branch-misses #4.13% of all branches
  ( +-  0.45% )  (50.05%)

  24.550558669 seconds time elapsed 
 ( +-  1.83% )

After:
 Performance counter stats for 'g++ -w -Ofast tramp3d-v4.cpp' (4 runs):

  26383.472582  task-clock (msec) #0.995 CPUs utilized  
  ( +-  1.83% )
   202  context-switches  #0.008 K/sec  
  ( +-  1.68% )
 5  cpu-migrations#0.000 K/sec  
  ( +- 14.29% )
53,114  page-faults   #0.002 M/sec  
  ( +- 17.86% )
92,099,443,793  cycles#3.491 GHz
  ( +-  0.96% )  (66.68%)
 

Re: [committed][PATCH] Trivial cleanups to new classes

[Markus Trippelsdorf]

On 2017.11.02 at 08:55 -0600, Jeff Law wrote:
> 
> As has been discussed on-list.  This patch adds a virtual destructor to 
> the new classes in tree-ssa-propagate.h per our coding conventions and 
> what are considered best practices.  It doesn't matter for any code I'm 
> aware of today -- it's a defensive measure.
> 
> This also drops the "virtual" keyword on the FINAL OVERRIDE member 
> functions in gimple-ssa-sprintf's sprintf_dom_walker class.  Opinions 
> here are more mixed.  It's agreed that the keyword is redundant in this 
> context.  The question is whether or not it adds confusion or reduces 
> confusion.
> 
> The virtual keyword intuitively implies to me the member can be 
> overridden by a derived class, but that's in direct conflict with the 
> FINAL keyword.
> 
> Others focus more on the fact that the virtual keyword implies that the 
> calls are typically indirect.   But in the case of a FINAL, one of the 
> hopes is that devirt can use the information to change the indirect call 
> into a direct call.
> 
> In the end the arguments for dropping the "virtual" seemed stronger to me.
> 
> Bootstrapped and regression tested on x86.  Installing on the trunk.

Even specifying both override and final is normally frowned upon, see:
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c128-virtual-functions-should-specify-exactly-one-of-virtual-override-or-final

-- 
Markus

Re: [RFC] Make 4-stage PGO bootstrap really working

[Markus Trippelsdorf]

On 2017.10.27 at 15:03 +0200, Martin Liška wrote:
> > And BTW would it make sense to add -gtoggle to stage2 in bootstrap-lto?
> 
> Why do you want to have it there? Am I right that we do not do a stage
> comparison with LTO bootstrap?

The idea was to trigger -g -flto at least during one stage, just as a
sanity check. 
Comparison wouldn't make sense, because we would compare LTO object
files.

-- 
Markus

Re: [RFC] Make 4-stage PGO bootstrap really working

[Markus Trippelsdorf]

On 2017.08.30 at 11:45 +0200, Martin Liška wrote:
> diff --git a/Makefile.in b/Makefile.in
> index 78db0982ba2..16b76906ad0 100644
> --- a/Makefile.in
> +++ b/Makefile.in
> @@ -529,13 +529,14 @@ STAGE1_CONFIGURE_FLAGS = --disable-intermodule 
> $(STAGE1_CHECKING) \
> --disable-coverage --enable-languages="$(STAGE1_LANGUAGES)" \
> --disable-build-format-warnings
>  
> -STAGEprofile_CFLAGS = $(STAGE2_CFLAGS) -fprofile-generate
> +profile_folder=`${PWD_COMMAND}`/gcov-profiles/
> +STAGEprofile_CFLAGS = $(STAGE2_CFLAGS) -fprofile-generate=$(profile_folder)
>  STAGEprofile_TFLAGS = $(STAGE2_TFLAGS)
>  
>  STAGEtrain_CFLAGS = $(STAGE3_CFLAGS)
>  STAGEtrain_TFLAGS = $(STAGE3_TFLAGS)
>  
> -STAGEfeedback_CFLAGS = $(STAGE4_CFLAGS) -fprofile-use
> +STAGEfeedback_CFLAGS = $(STAGE4_CFLAGS) -fprofile-use=$(profile_folder) 
> -fdump-ipa-profile

-fdump-ipa-profile looks like a debugging leftover and should be
dropped.

>  STAGEfeedback_TFLAGS = $(STAGE4_TFLAGS)
>  
>  STAGEautoprofile_CFLAGS = $(STAGE2_CFLAGS) -g
> diff --git a/Makefile.tpl b/Makefile.tpl
> index 5fcd7e358d9..129175a579c 100644
> --- a/Makefile.tpl
> +++ b/Makefile.tpl
> @@ -452,13 +452,14 @@ STAGE1_CONFIGURE_FLAGS = --disable-intermodule 
> $(STAGE1_CHECKING) \
> --disable-coverage --enable-languages="$(STAGE1_LANGUAGES)" \
> --disable-build-format-warnings
>  
> -STAGEprofile_CFLAGS = $(STAGE2_CFLAGS) -fprofile-generate
> +profile_folder=`${PWD_COMMAND}`/gcov-profiles/
> +STAGEprofile_CFLAGS = $(STAGE2_CFLAGS) -fprofile-generate=$(profile_folder)
>  STAGEprofile_TFLAGS = $(STAGE2_TFLAGS)
>  
>  STAGEtrain_CFLAGS = $(STAGE3_CFLAGS)
>  STAGEtrain_TFLAGS = $(STAGE3_TFLAGS)
>  
> -STAGEfeedback_CFLAGS = $(STAGE4_CFLAGS) -fprofile-use
> +STAGEfeedback_CFLAGS = $(STAGE4_CFLAGS) -fprofile-use=$(profile_folder) 
> -fdump-ipa-profile

ditto.


And BTW would it make sense to add -gtoggle to stage2 in bootstrap-lto?

diff --git a/config/bootstrap-lto.mk b/config/bootstrap-lto.mk
index 50b86ef1c81..c0cdee69288 100644
--- a/config/bootstrap-lto.mk
+++ b/config/bootstrap-lto.mk
@@ -1,8 +1,8 @@
 # This option enables LTO for stage2 and stage3 in slim mode
 
-STAGE2_CFLAGS += -flto=jobserver -frandom-seed=1
+STAGE2_CFLAGS += -flto=jobserver -frandom-seed=1 -gtoggle
 STAGE3_CFLAGS += -flto=jobserver -frandom-seed=1
-STAGEprofile_CFLAGS += -flto=jobserver -frandom-seed=1
+STAGEprofile_CFLAGS += -flto=jobserver -frandom-seed=1 -gtoggle
 STAGEtrain_CFLAGS += -flto=jobserver -frandom-seed=1
 STAGEfeedback_CFLAGS += -flto=jobserver -frandom-seed=1
 
-- 
Markus

Re: [RFC] Make 4-stage PGO bootstrap really working

[Markus Trippelsdorf]

On 2017.10.19 at 14:56 +0200, Martin Liška wrote:
> PING^2

> So far so good with a small exception: conftest.gcda files that
> trigger -Wcoverage-mismatch. Can we remove these before a stage? Do we
> do a similar thing somewhere?

I think you should simply remove all these conftest.gcda files before
STAGEfeedback.

-- 
Markus

Re: [PATCH C++] Fix PR82357 - bogus "cannot bind bitfield" error

[Markus Trippelsdorf]

On 2017.10.13 at 12:02 -0400, Jason Merrill wrote:
> On Fri, Oct 13, 2017 at 5:40 AM, Markus Trippelsdorf
> <mar...@trippelsdorf.de> wrote:
> > r253266 introduced a bogus "cannot bind bitfield" error that breaks
> > building Chromium and Node.js.
> > Fix by removing the ugly goto.
> >
> > Tested on ppc64le.
> > Ok for trunk?
> 
> No, this just undoes my change, so we go back to not doing type
> checking for non-dependent static casts.  Better I think to avoid the
> call to build_static_cast in the first place, by teaching
> stabilize_reference that it can return a NON_DEPENDENT_EXPR unchanged.
> How does this (untested) patch work for you?

It works fine. Thanks.
It would be good to have a testcase that checks the type checking for
non-dependent static casts.
I'll let you handle the current issue.

-- 
Markus

[PATCH C++] Fix PR82357 - bogus "cannot bind bitfield" error

[Markus Trippelsdorf]

r253266 introduced a bogus "cannot bind bitfield" error that breaks
building Chromium and Node.js.
Fix by removing the ugly goto.

Tested on ppc64le.
Ok for trunk?
Thanks.

PR c++/82357
* typeck.c (build_static_cast): Handle processing_template_decl
without using goto.

diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 08b2ae555e63..00688a21421c 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -7059,9 +7059,8 @@ build_static_cast (tree type, tree oexpr, tsubst_flags_t 
complain)
 
   bool dependent = (dependent_type_p (type)
|| type_dependent_expression_p (expr));
-  if (dependent)
+  if (dependent || processing_template_decl)
 {
-tmpl:
   expr = build_min (STATIC_CAST_EXPR, type, oexpr);
   /* We don't know if it will or will not have side effects.  */
   TREE_SIDE_EFFECTS (expr) = 1;
@@ -7084,8 +7083,6 @@ build_static_cast (tree type, tree oexpr, tsubst_flags_t 
complain)
  maybe_warn_about_useless_cast (type, expr, complain);
  maybe_warn_about_cast_ignoring_quals (type, complain);
}
-  if (processing_template_decl)
-   goto tmpl;
   return result;
 }
 
diff --git a/gcc/testsuite/g++.dg/template/bitfield4.C 
b/gcc/testsuite/g++.dg/template/bitfield4.C
new file mode 100644
index ..d53b0d406275
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/bitfield4.C
@@ -0,0 +1,6 @@
+// PR c++/82357
+
+template  struct A {
+  A() { x |= 0; } // { dg-bogus "cannot bind bitfield" }
+  int x : 8;
+};
-- 
Markus

Re: [PATCH] Do not error for no_sanitize attributes (PR sanitizer/82490).

[Markus Trippelsdorf]

On 2017.10.11 at 09:39 +0200, Jakub Jelinek wrote:
> On Wed, Oct 11, 2017 at 08:24:28AM +0200, Martin Liška wrote:
> > Hi.
> > 
> > This changes error to a warning:
> > warning: ‘foobar’ attribute directive ignored [-Wattributes]
> > 
> > Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
> 
> What is the rationale for not warning?  LLVM compatibility?
> I think in some cases it would be nice to find out that I wrote a typo...

The patch does warn. But yes, not erroring out improves LLVM
compatibility. Chromium uses __attribute__(no_sanitize("function")) for
example.

-- 
Markus

Re: [PATCH] Bump downloaded ISL version to 0.18

[Markus Trippelsdorf]

On 2017.09.18 at 09:41 +0200, Richard Biener wrote:
> 
> Committed.
> 
> Richard.
> 
> 2017-09-18  Richard Biener  
> 
>   * download_prerequisites (isl): Bump version to 0.18.
> 
> Index: contrib/download_prerequisites
> ===
> --- contrib/download_prerequisites(revision 252906)
> +++ contrib/download_prerequisites(working copy)
> @@ -30,7 +30,7 @@ version='(unversioned)'
>  gmp='gmp-6.1.0.tar.bz2'
>  mpfr='mpfr-3.1.4.tar.bz2'
>  mpc='mpc-1.0.3.tar.gz'
> -isl='isl-0.16.1.tar.bz2'
> +isl='isl-0.18.tar.bz2'

As an obvious follow-up, I've updated the checksums:

commit 076d07cde56c0da62036f3bdd440fa5a160d5f6b
Author: trippels 
Date:   Mon Sep 18 11:25:13 2017 +

Update checksums for isl-0.18.tar.bz2

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@252921 
138bc75d-0d04-0410-961f-82ee72b054a4

diff --git a/contrib/prerequisites.md5 b/contrib/prerequisites.md5
index b8e89d43c8a8..cc71e0f4de68 100644
--- a/contrib/prerequisites.md5
+++ b/contrib/prerequisites.md5
@@ -1,4 +1,4 @@
 86ee6e54ebfc4a90b643a65e402c4048  gmp-6.1.0.tar.bz2
 b8a2f6b0e68bef46e53da2ac439e1cf4  mpfr-3.1.4.tar.bz2
 d6a1d5f8ddea3abd2cc3e98f58352d26  mpc-1.0.3.tar.gz
-ac1f25a0677912952718a51f5bc20f32  isl-0.16.1.tar.bz2
+11436d6b205e516635b666090b94ab32  isl-0.18.tar.bz2
diff --git a/contrib/prerequisites.sha512 b/contrib/prerequisites.sha512
index 808970778c74..cf6b93b8d6b8 100644
--- a/contrib/prerequisites.sha512
+++ b/contrib/prerequisites.sha512
@@ -1,4 +1,4 @@
 
3c82aeab9c1596d4da8afac2eec38e429e84f3211e1a572cf8fd2b546493c44c039b922a1133eaaa48bd7f3e11dbe795a384e21ed95cbe3ecc58d7ac02246117
  gmp-6.1.0.tar.bz2
 
51066066ff2c12ed2198605ecf68846b0c96b548adafa5b80e0c786d0df488411a5e8973358fce7192dc977ad4e68414cf14500e3c39746de62465eb145bb819
  mpfr-3.1.4.tar.bz2
 
0028b76df130720c1fad7de937a0d041224806ce5ef76589f19c7b49d956071a683e2f20d154c192a231e69756b19e48208f2889b0c13950ceb7b3cfaf059a43
  mpc-1.0.3.tar.gz
-c188667a84dc5bdddb4ab7c35f89c91bf15a8171f4fcaf41301cf285fb7328846d9a367c096012fec4cc69d244f0bc9e95d84c09ec097394cd4093076f2a041b
  isl-0.16.1.tar.bz2
+85d0b40f4dbf14cb99d17aa07048cdcab2dc3eb527d2fbb1e84c41b2de5f351025370e57448b63b2b8a8cf8a0843a089c3263f9baee1542d5c2e1cb37ed39d94
  isl-0.18.tar.bz2

-- 
Markus

Re: [PATCH, i386] Enable option -mprefer-avx256 added for Intel AVX512 configuration

[Markus Trippelsdorf]

On 2017.09.14 at 14:36 +0200, Jakub Jelinek wrote:
> On Thu, Sep 14, 2017 at 12:10:50PM +, Shalnov, Sergey wrote:
> > GCC has the option "mprefer-avx128" to use 128-bit AVX registers instead
> > of 256-bit AVX registers in the auto-vectorizer.
> 
> > This patch enables the command line option "mprefer-avx256" that reduces
> > 512-bit registers usage in "march=skylake-avx512" mode.  This is the
> > initial implementation of the option.  Currently, 512-bit registers might
> > appears in some cases.  I have a plan to continue fix the cases where
> > 512-bit registers are appear.  Sergey
> 
> What is the rationale for this?  -mprefer-avx128 has been added because some
> (older) AMD CPUs implement AVX by performing 256-bit ops as two 128-bit uops
> and thus it is faster to emit 128-bit only code.
> Is that the case for any AVX512 implementations too?

You get a huge frequency drop when you run AVX512 code. There are
situations where this more than offsets the potential gains.
Glibc had to disable AVX512 memcpy because of this issue.

-- 
Markus

Re: [PATCH] Add -static-pie to GCC driver to create static PIE

[Markus Trippelsdorf]

On 2017.09.12 at 13:48 -0500, Aaron Sawdey wrote:
> On Tue, 2017-09-12 at 16:20 +, Joseph Myers wrote:
> > On Mon, 28 Aug 2017, H.J. Lu wrote:
> > 
> > > Here is the updated patch.   OK for trunk?
> > 
> > OK.
> 
> This seems to be causing an issue for me on powerpc:
> 
> ../../trunk/gcc/config/rs6000/sysv4.h:819:0: error: "LINK_EH_SPEC" redefined 
> [-Werror]
>  # define LINK_EH_SPEC "%{!static:--eh-frame-hdr} "

It will cause problems on other platforms as well:

gcc/config/alpha/elf.h:171:#define LINK_EH_SPEC "%{!static:--eh-frame-hdr} "
gcc/config/alpha/vms.h:209:#define LINK_EH_SPEC "vms-dwarf2eh.o%s "
gcc/config/dragonfly.h:64:#define LINK_EH_SPEC "--eh-frame-hdr"
gcc/config/freebsd.h:48:#define LINK_EH_SPEC "%{!static:--eh-frame-hdr} "
gcc/config/gnu-user.h:135:#define LINK_EH_SPEC 
"%{!static|static-pie:--eh-frame-hdr} "
gcc/config/netbsd.h:128:#define LINK_EH_SPEC "%{!static:--eh-frame-hdr} "
gcc/config/openbsd.h:139:#define LINK_EH_SPEC "%{!static:--eh-frame-hdr} "
gcc/config/powerpcspe/sysv4.h:808:# define LINK_EH_SPEC 
"%{!static:--eh-frame-hdr} "
gcc/config/rs6000/sysv4.h:819:# define LINK_EH_SPEC "%{!static:--eh-frame-hdr} "
gcc/config/sol2.h:375:#define LINK_EH_SPEC "%{!static:--eh-frame-hdr} "


-- 
Markus

Re: C++ PATCH for c++/81359, Unparsed NSDMI error in SFINAE context

[Markus Trippelsdorf]

On 2017.08.09 at 14:30 -0400, Jason Merrill wrote:
> The issue here is that we try to determine the EH specification of
> B::C::C() from within SFINAE context, and we can't determine it yet
> because the NSDMI for B::C::i hasn't been parsed yet.  This patch
> allows that determination to fail quietly in SFINAE context; we'll try
> again the next time it is needed.

Thanks.

Unfortunately it breaks the following testcase:

 ~ % cat asm-js.ii
struct A {
  void operator delete(void *, unsigned long);
};
struct B : A {
  virtual ~B();
};
struct C : B {};

 ~ % g++ -c asm-js.ii
asm-js.ii:7:8: error: no matching function for call to ‘C::operator 
delete(void*)’
 struct C : B {};
^
asm-js.ii:2:8: note: candidate: ‘static void A::operator delete(void*, long 
unsigned int)’
   void operator delete(void *, unsigned long);
^~~~
asm-js.ii:2:8: note:   candidate expects 2 arguments, 1 provided


-- 
Markus

Re: [PATCH] Add -std=c++2a

[Markus Trippelsdorf]

On 2017.07.20 at 19:04 +0200, Markus Trippelsdorf wrote:
> On 2017.07.20 at 09:33 -0400, Andrew Sutton wrote:
> > This adds a new C++ dialect, enabled by -std=c++2a.
> > 
> > libcpp/
> > Add support for C++2a.
> > * include/cpplib.h (c_lang): Add CXX2A and GNUCXX2A.
> > * init.c (lang_defaults): Add rows for CXX2A and GNUCXX2A.
> > (cpp_init_builtins): Set __cplusplus to 201707L for C++2x.
> > 
> > gcc/c-family/
> > Add support for -std=c++2a.
> > * c-common.h (cxx_dialect): Add cxx2a as a dialect.
> > * opt.c: Add options for -std=c++2a and -std=gnu++2a.
> > * c-opts.c (set_std_cxx2a): New.
> > (c_common_handle_option): Set options when -std=c++2a is enabled.
> > 
> > gcc/testsuite/
> > New test for -std=c++2a.
> > * g++.dg/cpp2a/cplusplus.C: New.
> 
> Perhaps you should enable -fconcepts by default?

On top of your patch:

diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c
index eed9e72b17f0..a923c3e9eedf 100644
--- a/gcc/c-family/c-opts.c
+++ b/gcc/c-family/c-opts.c
@@ -1626,6 +1626,7 @@ set_std_cxx2a (int iso)
   flag_isoc94 = 1;
   flag_isoc99 = 1;
   flag_isoc11 = 1;
+  flag_concepts = 1;
   cxx_dialect = cxx2a;
   lang_hooks.name = "GNU C++17"; /* Pretend C++17 until standardization.  */
 }

-- 
Markus

Re: [PATCH] Add -std=c++2a

[Markus Trippelsdorf]

On 2017.07.20 at 09:33 -0400, Andrew Sutton wrote:
> This adds a new C++ dialect, enabled by -std=c++2a.
> 
> libcpp/
> Add support for C++2a.
> * include/cpplib.h (c_lang): Add CXX2A and GNUCXX2A.
> * init.c (lang_defaults): Add rows for CXX2A and GNUCXX2A.
> (cpp_init_builtins): Set __cplusplus to 201707L for C++2x.
> 
> gcc/c-family/
> Add support for -std=c++2a.
> * c-common.h (cxx_dialect): Add cxx2a as a dialect.
> * opt.c: Add options for -std=c++2a and -std=gnu++2a.
> * c-opts.c (set_std_cxx2a): New.
> (c_common_handle_option): Set options when -std=c++2a is enabled.
> 
> gcc/testsuite/
> New test for -std=c++2a.
> * g++.dg/cpp2a/cplusplus.C: New.

Perhaps you should enable -fconcepts by default?

-- 
Markus

Re: Fix ICE in estimate_bb_frequencies

[Markus Trippelsdorf]

On 2017.07.18 at 09:54 +0200, Jan Hubicka wrote:
> Hi,
> this patch fixes ICE in estimate_bb_frequencies which triggers because we 
> forget
> to compute probability for blocks whose count is earlier statically 
> determined to be
> 0.
> 
> Bootstrapped/regtested x86_64-linux, will commit it shortly.

It also fixes both testcases from PR81318.

However the following testcase still ICEs:

trippels@gcc2-power8 linux % cat main.i
int a;
extern void fn4();
__attribute__((__cold__)) void fn1();
void fn2() { fn1(); }
void fn3() {
  fn2();
  if (a)
fn4();
}

trippels@gcc2-power8 linux % gcc -O2 -c main.i
during GIMPLE pass: profile_estimate
main.i: In function ‘fn3’:
main.i:9:1: internal compiler error: in to_reg_br_prob_base, at 
profile-count.h:189

-- 
Markus

Re: [PATCH] Speed-up indirect-call instrumentation

[Markus Trippelsdorf]

On 2017.06.09 at 14:17 +0200, Martin Liška wrote:
> Hello.
> 
> I discussed with Honza possibility to speed-up instrumentation that we do for
> indirect call target tracking. By direct emission of:
> 
>  if (__gcov_indirect_call_callee != NULL)
>__gcov_indirect_call_profiler_v2 (profile_id, _function_decl);
> 
> we can save reduce # of execution of __gcov_indirect_call_profiler_v2 
> function.
> I measured that tramp3d (-O2 -fprofile-generate) goes from 7.1s to 6.3s.
> 
> Apart from that I slightly change probability for direct instrumentation of
> time profiler.
> 
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

But it crashes on gcc.dg/pr78582.c with -O0.

-- 
Markus

Re: [PATCH][GCC][PATCHv3] Improve fpclassify w.r.t IEEE like numbers in GIMPLE.

[Markus Trippelsdorf]

On 2017.01.19 at 18:20 +, Joseph Myers wrote:
> On Thu, 19 Jan 2017, Tamar Christina wrote:
> 
> > Hi Joseph,
> > 
> > I made the requested changes and did a quick pass over the rest
> > of the fp cases.
> 
> I've no further comments, but watch out for any related test failures 
> being reported.

g++.dg/opt/pr60849.C started ICEing on both X86_64 and ppc64le.

-- 
Markus

Re: [Patch, fortran] PR35339 Optimize implied do loops in io statements

[Markus Trippelsdorf]

On 2017.06.05 at 22:39 +0200, Nicolas Koenig wrote:
> With all the style fixes committed as r248877.

171_swim fails now. I didn't bisect, but I suspect your revision.

-- 
Markus

Re: [PATCH] Introduce 4-stages profiledbootstrap to get a better profile.

[Markus Trippelsdorf]

On 2017.05.25 at 11:55 +0200, Martin Liška wrote:
> Hi.
> 
> As I spoke about the PGO with Honza and Richi, current 3-stage is not ideal 
> for following
> 2 reasons:
> 
> 1) stageprofile compiler is train just on libraries that are built during 
> stage2
> 2) apart from that, as the compiler is also used to build the final compiler, 
> profile
> is being updated during the build. So the stage2 compiler is making different 
> decisions.
> 
> Both problems can be resolved by adding another step in between current 
> stage2 and stage3
> where we train stage2 compiler by building compiler with default options.

Another issue that I've noticed is that LTO doesn't get used in the
final stage (stagefeedback) with "bootstrap-O3 bootstrap-lto".
It only is used during training. So either move -flto to stagefeedback,
or use -flto both during training and during the final stage.

-- 
Markus

Re: [PATCH] Introduce 4-stages profiledbootstrap to get a better profile.

[Markus Trippelsdorf]

On 2017.05.25 at 11:55 +0200, Martin Liška wrote:
> Hi.
>
> As I spoke about the PGO with Honza and Richi, current 3-stage is not ideal 
> for following
> 2 reasons:
>
> 1) stageprofile compiler is train just on libraries that are built during 
> stage2
> 2) apart from that, as the compiler is also used to build the final compiler, 
> profile
> is being updated during the build. So the stage2 compiler is making different 
> decisions.
>
> Both problems can be resolved by adding another step in between current 
> stage2 and stage3
> where we train stage2 compiler by building compiler with default options.
>
> I'm going to do some measurements.

I did some measurements on gcc67 (trunk with --enable-checking=release).
The apparent speedup is in the noise.

Without your patch:

 Performance counter stats for 'g++ -w -Ofast tramp3d-v4.cpp' (10 runs):

  15749.058451  task-clock (msec) #0.997 CPUs utilized  
  ( +-  0.13% )
 1,352  context-switches  #0.086 K/sec  
  ( +-  0.16% )
 7  cpu-migrations#0.000 K/sec  
  ( +-  5.73% )
   269,142  page-faults   #0.017 M/sec  
  ( +-  0.01% )
60,676,581,181  cycles#3.853 GHz
  ( +-  0.09% )  (83.35%)
13,401,784,189  stalled-cycles-frontend   #   22.09% frontend cycles 
idle ( +-  0.20% )  (83.33%)
12,926,843,370  stalled-cycles-backend#   21.30% backend cycles 
idle  ( +-  0.04% )  (83.31%)
73,074,099,356  instructions  #1.20  insn per cycle
  #0.18  stalled cycles per 
insn  ( +-  0.02% )  (83.34%)
16,607,220,814  branches  # 1054.490 M/sec  
  ( +-  0.03% )  (83.36%)
   616,673,310  branch-misses #3.71% of all branches
  ( +-  0.08% )  (83.36%)

  15.803602619 seconds time elapsed 
 ( +-  0.14% )

With your patch:

 Performance counter stats for 'g++ -w -Ofast tramp3d-v4.cpp' (10 runs):

  15735.220610  task-clock (msec) #0.997 CPUs utilized  
  ( +-  0.11% )
 1,354  context-switches  #0.086 K/sec  
  ( +-  0.22% )
 6  cpu-migrations#0.000 K/sec  
  ( +-  6.67% )
   269,164  page-faults   #0.017 M/sec  
  ( +-  0.01% )
60,723,862,242  cycles#3.859 GHz
  ( +-  0.08% )  (83.35%)
13,382,554,421  stalled-cycles-frontend   #   22.04% frontend cycles 
idle ( +-  0.14% )  (83.31%)
12,912,171,664  stalled-cycles-backend#   21.26% backend cycles 
idle  ( +-  0.03% )  (83.34%)
73,109,081,227  instructions  #1.20  insn per cycle
  #0.18  stalled cycles per 
insn  ( +-  0.03% )  (83.34%)
16,590,421,798  branches  # 1054.349 M/sec  
  ( +-  0.02% )  (83.35%)
   616,669,135  branch-misses #3.72% of all branches
  ( +-  0.08% )  (83.36%)

  15.788772466 seconds time elapsed 
 ( +-  0.12% )



--
Markus

Re: [PATCH] Add -dB option to disable backtraces

[Markus Trippelsdorf]

On 2017.05.16 at 19:16 -0700, Andi Kleen wrote:
> From: Andi Kleen 
> 
> When running creduce on an ICE substantial amounts of the total
> CPU time go to backtrace_qsort() (sorting dwarf of the compiler) for
> printing the backtrace of the ICE. When running a reduction we don't need the
> backtrace. So add a -dB option to turn it off, and make reduction
> a bit faster.

It is useful when reducing compiler segfaults, because here one should
grep for a symbol in the backtrace to not end up with an unrelated
crash.

-- 
Markus

Re: [patch] build xz (instead of bz2) compressed tarballs and diffs

[Markus Trippelsdorf]

On 2017.05.15 at 16:24 +0200, Jakub Jelinek wrote:
> On Mon, May 15, 2017 at 04:13:44PM +0200, Markus Trippelsdorf wrote:
> > On 2017.05.15 at 14:02 +, Joseph Myers wrote:
> > > The xz manpage warns against blindly using -9 (for which --best is a 
> > > deprecated alias) because of the implications for memory requirements for 
> > > decompressing.  If there's a reason it's considered appropriate here, I 
> > > think it needs an explanatory comment.
> > 
> > I think it is unacceptable, because it would increase memory usage when
> > decompressing over 20x compared to bz2 (and over 100x while compressing).
> 
> The memory using during compressing isn't that interesting as long as it
> isn't prohibitive for sourceware or the machines RMs use.
> For the decompression, I guess it matters what is actually the memory needed
> for decompression the -9 gcc tarball, and compare that to minimal memory
> requirements to compile (not bootstrap) the compiler using typical system
> compilers.  If compilation of gcc takes more memory than the decompression,
> then it should be fine, why would anyone try to decompress gcc not to build
> it afterwards?

Ok, it doesn't really matter. With gcc-7.1 tarball:

size: 533084160 (uncompressed)

-9:
 xz -d gcc.tar.xz
4.71user 0.26system 0:04.97elapsed 100%CPU (0avgtext+0avgdata 67804maxresident)k
size: 60806928

-6 (default):
 xz -d gcc.tar.xz
4.88user 0.28system 0:05.17elapsed 99%CPU (0avgtext+0avgdata 10324maxresident)k
size: 65059664

So -9 is actually just fine.

-- 
Markus

Re: [patch] build xz (instead of bz2) compressed tarballs and diffs

[Markus Trippelsdorf]

On 2017.05.15 at 14:02 +, Joseph Myers wrote:
> The xz manpage warns against blindly using -9 (for which --best is a 
> deprecated alias) because of the implications for memory requirements for 
> decompressing.  If there's a reason it's considered appropriate here, I 
> think it needs an explanatory comment.

I think it is unacceptable, because it would increase memory usage when
decompressing over 20x compared to bz2 (and over 100x while compressing).

The default -6 should be good enough (3x more memory when decompressing).

-- 
Markus

Re: [RFC PATCH, i386]: Enable post-reload compare elimination pass

[Markus Trippelsdorf]

On 2017.05.12 at 21:09 +0200, Uros Bizjak wrote:
> On Fri, May 12, 2017 at 8:34 PM, Jeff Law  wrote:
> > On 05/10/2017 01:05 PM, Uros Bizjak wrote:
> >>
> >> On Wed, May 10, 2017 at 5:18 PM, Uros Bizjak  wrote:
> >>>
> >>> On Wed, May 10, 2017 at 4:27 PM, Jakub Jelinek  wrote:
> 
>  On Tue, May 09, 2017 at 06:06:47PM +0200, Uros Bizjak wrote:
> >
> > Attached patch enables post-reload compare elimination pass by
> > providing expected patterns (duplicates of existing patterns with
> > setters of reg and flags switched in the parallel) for flag setting
> > arithmetic instructions.
> >
> > The merge triggers more than 3000 times during the gcc bootstrap,
> > mostly in cases where intervening memory load or store prevents
> > combine from merging the arithmetic insn and the following compare.
> >
> > Also, some recent linux x86_64 defconfig build results in ~200 merges,
> > removing ~200 test/cmp insns. Not much, but I think the results still
> > warrant the pass to be enabled.
> 
> 
>  Isn't the right fix instead to change the compare-elim.c pass to either
>  accept both reg vs. flags orderings in parallel, or both depending
>  on some target hook, or change it to the order i386.md and most other
>  major targets use and just fix up mn10300/rx (and aarch64?) to use the
>  same
>  order?
> >>
> >>
> >> Attached patch changes compare-elim.c order to what i386.md expects.
> >>
> >> Thoughts?
> >
> > I think with an appropriate change to the canonicalization rules in the
> > manual this is fine.
> >
> > I've got the visium, rx and mn103 patches handy to ensure they don't
> > regress.  aarch64 doesn't seem to be affected either way but I didn't
> > investigate why -- I expected it to improve with your change.
> >
> > I'll write up a ChangeLog and commit the mn103, rx and visium changes after
> > you commit the compare-elim & documentation bits.
>
> Thanks!
>
> I went ahead and commit the patch without documentation change (which
> I plan to submit in a separate patch ASAP), with the following
> ChangeLog:
>
> 2017-05-12  Uros Bizjak  
>
> * compare-elim.c (try_eliminate_compare): Canonicalize
> operation with embedded compare to
> [(set (reg:CCM) (compare:CCM (operation) (immediate)))
>  (set (reg) (operation)].
>
> * config/i386/i386.c (TARGET_FLAGS_REGNUM): New define.
>
> Re-bootstrapped and re-tested on x86_64-linux-gnu {,-m32}.
>
> Committed to mainline SVN.

This causes gcc.dg/atomic/c11-atomic-exec-2.c to ICE with e.g.
-march=nehalem:

markus@x4 gcc % gcc -std=c11 -pedantic-errors -march=nehalem -O2 
./gcc/testsuite/gcc.dg/atomic/c11-atomic-exec-2.c
./gcc/testsuite/gcc.dg/atomic/c11-atomic-exec-2.c: In function ‘test_minus’:
./gcc/testsuite/gcc.dg/atomic/c11-atomic-exec-2.c:120:1: internal compiler 
error: in ix86_cc_mode, at config/i386/i386.c:22485
 }
 ^
0xea7b37 ix86_cc_mode(rtx_code, rtx_def*, rtx_def*)
/home/markus/gcc/gcc/config/i386/i386.c:22485
0x1246e11 maybe_select_cc_mode
/home/markus/gcc/gcc/compare-elim.c:500
0x1246e11 try_eliminate_compare
/home/markus/gcc/gcc/compare-elim.c:665
0x1246e11 execute_compare_elim_after_reload
/home/markus/gcc/gcc/compare-elim.c:727
0x1246e11 execute
/home/markus/gcc/gcc/compare-elim.c:770


--
Markus

Re: [PATCH, i386]: Fix PR79804, ICE in print_reg

[Markus Trippelsdorf]

On 2017.04.20 at 22:29 +0200, Uros Bizjak wrote:
> 
> PR target/79804
> * gcc.target/i386/pr79804.c: New test.
> 
> Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
> 
> Committed to mainline SVN.
> --- testsuite/gcc.target/i386/pr79804.c   (nonexistent)
> +++ testsuite/gcc.target/i386/pr79804.c   (working copy)
> @@ -0,0 +1,10 @@
> +/* PR target/79804 */
> +/* { dg-do compile } */
> +/* { dg-options "" } */
> +
> +void foo (void)
> +{
> +  register int r20 asm ("20");
> +
> +  asm volatile ("# %0" : "=r"(r20));  /* { dg-error "invalid use of 
> register" } */
> +}

The test fails without optimizations:

gcc/testsuite/gcc.target/i386/pr79804.c: In function ‘foo’:
gcc/testsuite/gcc.target/i386/pr79804.c:10:1: error: frame cannot be used in 
asm here
 }
 ^
gcc/testsuite/gcc.target/i386/pr79804.c:9:3: error: invalid 'asm': invalid use 
of register 'frame'
   asm volatile ("# %0" : "=r"(r20));  /* { dg-error "invalid use of register" 
} */
   ^~~


-- 
Markus

Re: [P1] [PATCH] [PR tree-optimization/80374] Do not try to convert integer_zero_node to undesirable types

[Markus Trippelsdorf]

On 2017.04.10 at 13:20 -0600, Jeff Law wrote:
> 
> fold_convert can fail for certain types.  It can fail either by returning a
> error_mark_node or triggering a gcc_assert depending on the exact situation.
> 
> Both are problematical.  This patch checks that we can convert
> integer_zero_node to the proper type before calling fold_convert.
> 
> Bootstrapped and regression tested on x86_64.  Installing on the trunk.

I've fixed up the testcase:

FAIL: g++.dg/pr80374.C  -std=c++11 (test for excess errors)
FAIL: g++.dg/pr80374.C  -std=c++14 (test for excess errors)
FAIL: g++.dg/pr80374.C  -std=c++98 (test for excess errors)

Committed as obvious.

diff --git a/gcc/testsuite/g++.dg/pr80374.C b/gcc/testsuite/g++.dg/pr80374.C
index b02b65629aef..83f778be638a 100644
--- a/gcc/testsuite/g++.dg/pr80374.C
+++ b/gcc/testsuite/g++.dg/pr80374.C
@@ -1,3 +1,5 @@
+// { dg-do compile }
+// { dg-options "-O1 -std=c++11" }
 void a (const char *, const char *, int, const char *)
   __attribute__ ((__noreturn__));
 template 
@@ -11,7 +13,8 @@ catch (b d)
 if (d)
   a ("", "", 2, __PRETTY_FUNCTION__);
   }
-main ()
+void
+foo ()
 {
   using e = decltype (nullptr);
   c ();
-- 
Markus

Re: [PATCH] Fix PR80281

[Markus Trippelsdorf]

On 2017.04.03 at 15:20 +0200, Richard Biener wrote:
> I'm re-testing the following variant.
> 
> Richard.
> 
> 2017-04-03  Richard Biener  
> 
>   PR middle-end/80281
>   * match.pd (A + (-B) -> A - B): Make sure to preserve unsigned
>   arithmetic done for the negate or the plus.  Simplify.
>   (A - (-B) -> A + B): Likewise.
>   * fold-const.c (split_tree): Make sure to not negate pointers.
> 
>   * gcc.dg/torture/pr80281.c: New testcase.

gcc.dg/tree-ssa/pr40921.c started to fail with -march=skylake:

 % gcc -march=skylake -c -O2 -fdump-tree-optimized -ffast-math -c 
gcc.dg/tree-ssa/pr40921.c
 % cat pr40921.i.227t.optimized | grep "\-y"
   _3 = -y_4(D);

-- 
Markus

[C++ PATCH] Fix PR80294 -- Segfault in constexpr.c (reduced_constant_expression_p)

[Markus Trippelsdorf]

As the testcase shows, elt can become a NULL tree during
FOR_EACH_CONSTRUCTOR_VALUE. So guard against this possibility before
calling reduced_constant_expression_p() recursively.

Tested on ppc64le.
OK for trunk?

PR c++/80294
* constexpr.c (reduced_constant_expression_p): Guard against NULL
tree before recursing.

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 3ca356071810..9ee794d5bf37 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -1716,7 +1716,7 @@ reduced_constant_expression_p (tree t)
   /* And we need to handle PTRMEM_CST wrapped in a CONSTRUCTOR.  */
   tree elt; unsigned HOST_WIDE_INT idx;
   FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (t), idx, elt)
-   if (!reduced_constant_expression_p (elt))
+   if (elt && !reduced_constant_expression_p (elt))
  return false;
   return true;
 
diff --git a/gcc/testsuite/g++.dg/torture/pr80294.C 
b/gcc/testsuite/g++.dg/torture/pr80294.C
new file mode 100644
index ..859a10ba3e31
--- /dev/null
+++ b/gcc/testsuite/g++.dg/torture/pr80294.C
@@ -0,0 +1,167 @@
+// { dg-do compile }
+
+namespace std {
+typedef long unsigned size_t;
+template  struct integral_constant {
+  static constexpr _Tp value = 0;
+};
+template  struct tuple_element;
+template  struct integer_sequence;
+} // namespace std
+namespace meta {
+using std::integer_sequence;
+template  struct list;
+template  class, typename...> struct defer;
+template  using _t = typename T::type;
+template  using size_t = std::integral_constant;
+namespace detail {
+enum indices_strategy_ { recurse };
+constexpr indices_strategy_ strategy_(std::size_t, std::size_t) {
+  return recurse;
+}
+template  struct make_indices_ 
{
+  using type = State;
+};
+} // namespace detail
+template 
+using index_sequence = integer_sequence;
+template 
+using make_index_sequence =
+_t, detail::strategy_(0, 0)>>;
+template 
+using invoke = typename F::template invoke;
+namespace lazy {
+template 
+using invoke = defer;
+}
+template  struct id { using type = int; };
+namespace detail {
+struct defer_if_ {
+  template  class C, typename... Ts> struct result {
+using type = C;
+  };
+  template  class C, typename... Ts>
+  result try_();
+};
+template  class C, typename... Ts>
+using defer_ = decltype(defer_if_{}.try_());
+} // namespace detail
+template  class C, typename... Ts>
+struct defer : detail::defer_ {};
+template  class C> struct quote {
+  template  using invoke = _t>;
+};
+template  class> using quote_trait = int;
+template  struct bind_front {
+  template  using invoke = invoke;
+};
+namespace extension {
+template  struct apply;
+template 
+struct apply : lazy::invoke {};
+} // namespace extension
+template 
+using apply = _t>;
+template  using curry = Q;
+template  using uncurry = bind_front;
+namespace detail {
+template  using first_ = T;
+template  struct repeat_n_c_;
+template 
+struct repeat_n_c_ {
+  using type = list...>;
+};
+} // namespace detail
+template 
+using repeat_n_c = _t>>;
+template  using repeat_n = repeat_n_c;
+namespace detail {
+template  struct at_impl_;
+template  struct at_impl_ {
+  template  static T eval(VoidPtrs..., T *);
+};
+template  struct at_;
+template 
+struct at_ : decltype(at_impl_>::eval(
+ static_cast(0)...)) {};
+} // namespace detail
+template  using at = _t>;
+template  using at_c = at>;
+namespace detail {
+template  struct front_;
+template  struct front_> {
+  using type = Head;
+};
+} // namespace detail
+template  using front = _t;
+namespace detail {
+template  struct back_;
+template  struct back_> {
+  using type = at_c, sizeof...(List)>;
+};
+} // namespace detail
+template  using back = _t;
+namespace detail {
+template 
+struct as_list_ : lazy::invoke>, Sequence> {};
+} // namespace detail
+template  using as_list = _t;
+} // namespace meta
+namespace detail {
+template  using tag_spec = meta::front;
+template  using tag_elem = meta::back;
+template  constexpr auto adl_get(T) {}
+} // namespace detail
+template  struct chain {
+  using type = int;
+};
+template 
+struct chain {
+  using type =
+  typename First::template getter

Re: [PATCH] Fix PR80275

[Markus Trippelsdorf]

On 2017.04.03 at 11:16 +0200, Richard Biener wrote:
> 
> The following extends split_address_to_core_and_offset to handle
> POINTER_PLUS_EXPR to be able to simplify
> (unsigned long) [(void *) + 12B] - (unsigned long) ((const int 
> *)  + 4) which appears during niter analysis.
> 
> We seem to have various copies of similar code but refactoring didn't
> seem appropriate at this stage so I went for the minimal fix.
> 
> Bootstrap and regtest running on x86_64-unknown-linux-gnu.
> 
> Richard.
> 
> 2017-04-03  Richard Biener  
> 
>   PR tree-optimization/80275
>   * fold-const.c (split_address_to_core_and_offset): Handle
>   POINTER_PLUS_EXPR.
> 
>   * g++.dg/opt/pr80275.C: New testcase.
> 
> Index: gcc/fold-const.c
> ===
> --- gcc/fold-const.c  (revision 246642)
> +++ gcc/fold-const.c  (working copy)
> @@ -14341,6 +14341,24 @@ split_address_to_core_and_offset (tree e
> );
>core = build_fold_addr_expr_loc (loc, core);
>  }
> +  else if (TREE_CODE (exp) == POINTER_PLUS_EXPR)
> +{
> +  core = TREE_OPERAND (exp, 0);
> +  STRIP_NOPS (core);
> +  *pbitpos = 0;
> +  *poffset = TREE_OPERAND (exp, 1);
> +  if (TREE_CODE (*poffset) == INTEGER_CST)
> + {
> +   offset_int tem = wi::sext (wi::to_offset (*poffset),
> +  TYPE_PRECISION (TREE_TYPE (*poffset)));
> +   tem <<= LOG2_BITS_PER_UNIT;
> +   if (wi::fits_shwi_p (tem))
> + {
> +   *pbitpos = tem.to_shwi ();
> +   *poffset = NULL_TREE;
> + }
> + }
> +}
>else
>  {
>core = exp;
> Index: gcc/testsuite/g++.dg/opt/pr80275.C
> ===
> --- gcc/testsuite/g++.dg/opt/pr80275.C(nonexistent)
> +++ gcc/testsuite/g++.dg/opt/pr80275.C(working copy)
> @@ -0,0 +1,16 @@
> +// { dg-do compile { target c++14 } }
> +// { dg-options "-O2 -fdump-tree-optimized" }
> +
> +#include 
> +
> +int g()
> +{
> +  return 1234;
> +}
> +
> +int f2()
> +{
> +  return std::min({1, g(), 4});
> +}
> +
> +// { dg-final { scan-tree-dump "return 1;" "optimized" } }

The testcase fails with -fpic, e.g.:
https://gcc.gnu.org/ml/gcc-regression/2017-04/msg2.html

So perhaps:

diff --git a/gcc/testsuite/g++.dg/opt/pr80275.C 
b/gcc/testsuite/g++.dg/opt/pr80275.C
index 7296a07fb2dd..7d625f2c7757 100644
--- a/gcc/testsuite/g++.dg/opt/pr80275.C
+++ b/gcc/testsuite/g++.dg/opt/pr80275.C
@@ -1,4 +1,4 @@
-// { dg-do compile { target c++14 } }
+// { dg-do compile { target c++14 && nonpic } }
 // { dg-options "-O2 -fdump-tree-optimized" }
 
 #include 

-- 
Markus

Re: [PATCH][RFC] Fix P1 PR77498

[Markus Trippelsdorf]

On 2017.03.31 at 11:16 +0200, Richard Biener wrote:
> On Fri, 31 Mar 2017, Richard Biener wrote:
> 
> > On Fri, 31 Mar 2017, Rainer Orth wrote:
> > 
> > > Hi Christophe,
> > > 
> > > > With this patch, the following testcase now fails on arm* targets:
> > > > gcc.dg/tree-ssa/pr71347.c scan-tree-dump-not optimized ".* = MEM.*;"
> > > 
> > > same on Solaris/SPARC.
> > 
> > I've reverted r241968 with (patch reverted).  It doesn't include
> > sparc, so please amend as you see fit.
> 
> Ah, r241441.  I'll fixup myself then.

It also fails on some X86 configurations:
https://gcc.gnu.org/ml/gcc-regression/2017-03/msg00237.html
https://gcc.gnu.org/ml/gcc-regression/2017-03/msg00238.html

-- 
Markus

[PATCH committed] avoid name lookup warning in tree.c

[Markus Trippelsdorf]

In stage1 with -std=gnu++98 I see:

/home/markus/gcc/gcc/tree.c: In function ‘void inchash::add_expr(const_tree, 
inchash::hash&, unsigned int)’:
/home/markus/gcc/gcc/tree.c:8013:11: warning: name lookup of ‘i’ changed
  for (i = TREE_OPERAND_LENGTH (t) - 1; i >= 0; --i)
   ^
/home/markus/gcc/gcc/tree.c:7773:7: warning:   matches this ‘i’ under ISO 
standard rules
   int i;
   ^
/home/markus/gcc/gcc/tree.c:7869:16: warning:   matches this ‘i’ under old rules
   for (int i = 0; i < TREE_VEC_LENGTH (t); ++i)
^

Fix committed as obvious.
 
PR tree-optimization/880216
* tree.c (add_expr): Avoid name lookup warning.

diff --git a/gcc/tree.c b/gcc/tree.c
index 8f87e7cfacb2..c87b7695c82a 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -7866,7 +7866,7 @@ add_expr (const_tree t, inchash::hash , unsigned 
int flags)
return;
   }
 case TREE_VEC:
-  for (int i = 0; i < TREE_VEC_LENGTH (t); ++i)
+  for (i = 0; i < TREE_VEC_LENGTH (t); ++i)
inchash::add_expr (TREE_VEC_ELT (t, i), hstate, flags);
   return;
 case FUNCTION_DECL:
-- 
Markus

[PATCH] Fix PR80183 : _M_color not moved

[Markus Trippelsdorf]

clang-format stopped working when compiled with gcc-7. 
It turned out that an uninitialized _M_color is responsible.

The fix is easy, just copy _M_color in the move case, too.

Tested on ppc64le.
OK for trunk?
Thanks.

PR libstdc++/80183
* include/bits/stl_tree.h:
(_Rb_tree_header::_M_move_data(_Rb_tree_header&)): Also save _M_color.

diff --git a/libstdc++-v3/include/bits/stl_tree.h 
b/libstdc++-v3/include/bits/stl_tree.h
index cbcf7f2606f..ce7ecdaa87a 100644
--- a/libstdc++-v3/include/bits/stl_tree.h
+++ b/libstdc++-v3/include/bits/stl_tree.h
@@ -192,6 +192,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 void
 _M_move_data(_Rb_tree_header& __from)
 {
+  _M_header._M_color = __from._M_header._M_color;
   _M_header._M_parent = __from._M_header._M_parent;
   _M_header._M_left = __from._M_header._M_left;
   _M_header._M_right = __from._M_header._M_right;
-- 
Markus

[PATCH] Fix memory leak in identify_jump_threads()

[Markus Trippelsdorf]

Valgrind shows:

==553== 391,488 bytes in 24 blocks are possibly lost in loss record 4,339 of 
4,342
==553==at 0x4030C15: calloc (vg_replace_malloc.c:711)
==553==by 0x1607CA0: xcalloc (xmalloc.c:162)
==553==by 0x1004A81: data_alloc (hash-table.h:263)
==553==by 0x1004A81: alloc_entries (hash-table.h:650)
==553==by 0x1004A81: hash_table (hash-table.h:586)
==553==by 0x1004A81: identify_jump_threads (tree-vrp.c:11009)
==553==by 0x1004A81: execute_vrp (tree-vrp.c:11684)
==553==by 0x1004A81: (anonymous namespace)::pass_vrp::execute(function*) 
(tree-vrp.c:11777)
==553==by 0xC80AA2: execute_one_pass(opt_pass*) (passes.c:2465)
==553==by 0xC81370: execute_pass_list_1(opt_pass*) (passes.c:2554)
==553==by 0xC81382: execute_pass_list_1(opt_pass*) (passes.c:2555)
==553==by 0xC813CC: execute_pass_list(function*, opt_pass*) (passes.c:2565)
==553==by 0x9409DC: cgraph_node::expand() (cgraphunit.c:2038)
==553==by 0x94217B: expand_all_functions (cgraphunit.c:2174)
==553==by 0x94217B: symbol_table::compile() [clone .part.51] 
(cgraphunit.c:2531)
==553==by 0x9448C4: compile (cgraphunit.c:2624)
==553==by 0x9448C4: symbol_table::finalize_compilation_unit() 
(cgraphunit.c:2621)
==553==by 0xD57C82: compile_file() (toplev.c:492)
==553==by 0x5B8DFF: do_compile (toplev.c:2000)
==553==by 0x5B8DFF: toplev::main(int, char**) (toplev.c:2134)
==553==
==553== 606,520 (2,976 direct, 603,544 indirect) bytes in 62 blocks are 
definitely lost in loss record 4,342 of 4,342
==553==at 0x402F26F: operator new(unsigned long) (vg_replace_malloc.c:334)
==553==by 0x1004A37: identify_jump_threads (tree-vrp.c:11009)
==553==by 0x1004A37: execute_vrp (tree-vrp.c:11684)
==553==by 0x1004A37: (anonymous namespace)::pass_vrp::execute(function*) 
(tree-vrp.c:11777)
==553==by 0xC80AA2: execute_one_pass(opt_pass*) (passes.c:2465)
==553==by 0xC81370: execute_pass_list_1(opt_pass*) (passes.c:2554)
==553==by 0xC81382: execute_pass_list_1(opt_pass*) (passes.c:2555)
==553==by 0xC813CC: execute_pass_list(function*, opt_pass*) (passes.c:2565)
==553==by 0x9409DC: cgraph_node::expand() (cgraphunit.c:2038)
==553==by 0x94217B: expand_all_functions (cgraphunit.c:2174)
==553==by 0x94217B: symbol_table::compile() [clone .part.51] 
(cgraphunit.c:2531)
==553==by 0x9448C4: compile (cgraphunit.c:2624)
==553==by 0x9448C4: symbol_table::finalize_compilation_unit() 
(cgraphunit.c:2621)
==553==by 0xD57C82: compile_file() (toplev.c:492)
==553==by 0x5B8DFF: do_compile (toplev.c:2000)
==553==by 0x5B8DFF: toplev::main(int, char**) (toplev.c:2134)
==553==by 0x5BB0DD: main (main.c:39)

The fix is trivial. Tested on ppc64le,
OK for trunk?

Longer term it would be nice to have a smart pointer available in gcc to
avoid those ugly and error-prone naked news.
Pedro committed a solution for binutils-gdb last year:
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commitdiff;h=da804164742b83965b487bbff5b6334f2e63fe91

* tree-vrp.c (identify_jump_threads): Delete avail_exprs.

diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 26652e3b048a..28d9c175dcd4 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -11021,6 +11021,7 @@ identify_jump_threads (void)
  ASSERT_EXPRs are still in the IL and cfg cleanup code does not yet
  handle ASSERT_EXPRs gracefully.  */
   delete equiv_stack;
+  delete avail_exprs;
   delete avail_exprs_stack;
 }
 
-- 
Markus

Re: C++ backports to 6

[Markus Trippelsdorf]

On 2017.03.14 at 17:08 +0100, Marek Polacek wrote:
> I've backported the attached patches gcc-6.

The PR79264 backport caused:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80091

-- 
Markus

Re: patch, libfortran] [patch, fortran] Fix PR 79956, part two, attempt 3

[Markus Trippelsdorf]

On 2017.03.15 at 00:01 +0100, Thomas Koenig wrote:
> Hello world,
> 
> well, here is the third attempt at fixing the second part of the PR.
> Glancing over the source, I think there are quite a few places where
> we currently issue a runtime error which we could replace by an
> assert, but that's something for 8.0.
> 
> Regression-tested on x86_64-pc-linux-gnu.

> Index: generated/reshape_c10.c
> ===
> --- generated/reshape_c10.c   (Revision 245760)
> +++ generated/reshape_c10.c   (Arbeitskopie)
> @@ -232,6 +232,11 @@ reshape_c10 (gfc_array_c10 * const restrict ret,
>  }
>  
>sdim = GFC_DESCRIPTOR_RANK (source);
> +
> +  /* sdim is always > 0; this lets the compiler optimize more and
> +   avoids a warning.  */
> +  GFC_ASSERT(sdim>0);
> +

You have committed a different patch, which is obviously wrong:

diff --git a/libgfortran/generated/reshape_c10.c 
b/libgfortran/generated/reshape_c10.c
index 00c64aeb746f..af45e960ee7f 100644
--- a/libgfortran/generated/reshape_c10.c
+++ b/libgfortran/generated/reshape_c10.c
@@ -78,6 +78,10 @@ reshape_c10 (gfc_array_c10 * const restrict ret,
   index_type shape_data[GFC_MAX_DIMENSIONS];
 
   rdim = GFC_DESCRIPTOR_EXTENT(shape,0);
+  /* rdim is always > 0; this lets the compiler optimize more and
+   avoids a potential warning.  */
+  GFC_ASSERT(sdim>0);

You should use rdim not sdim in the GFC_ASSERT.

-- 
Markus

Re: [PATCH] Remove dead stores and initializations

[Markus Trippelsdorf]

On 2017.03.16 at 14:40 +0100, Bernd Schmidt wrote:
> On 03/16/2017 01:31 PM, Markus Trippelsdorf wrote:
> > clang --analyze pointed out a number of dead stores and initializations.
> > 
> > Tested on ppc64le. Ok for trunk?
> 
> I'd say - not now.
> 
> Ideally someone would delve into the commit history to figure out what
> happened with each of these, and whether any of these indicate bugs.

Thanks. I've opened https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80066
to track this issue.

-- 
Markus

[PATCH] Remove dead stores and initializations

[Markus Trippelsdorf]

clang --analyze pointed out a number of dead stores and initializations.

Tested on ppc64le. Ok for trunk?

Thanks.

gcc/c-family/ChangeLog:

* c-ada-spec.c (to_ada_name): Remove dead store.

gcc/c/ChangeLog:

* c-array-notation.c (build_array_notation_expr): Remove dead stores.
* c-parser.c (c_parser_objc_try_catch_finally_statement): Remove dead
initialization.
(c_parser_oacc_wait_list): Remove dead store.
(c_parser_oacc_clause_tile): Remove dead initialization.

gcc/cp/ChangeLog:

* class.c (adjust_clone_args): Remove dead store.
* constexpr.c (potential_constant_expression_1): Likewise.
* cp-array-notation.c (expand_an_in_modify_expr): Likewise.
(expand_unary_array_notation_exprs): Likewise.
* decl.c (check_goto): Likewise.
(check_initializer): Remove dead initialization.
(cp_finish_decomp): Remove dead store.
(compute_array_index_type): Likewise.
(grokdeclarator): Likewise.
* decl2.c (import_export_decl): Likewise.
* optimize.c (maybe_clone_body): Likewise.
* search.c (lookup_field_1): Likewise.
* typeck.c (cxx_sizeof_or_alignof_type): Likewise.
(convert_for_initialization): Likewise.

gcc/ChangeLog:

* asan.c (asan_emit_stack_protection): Remove dead store. 
* bt-load.c (move_btr_def): Likewise.
* builtins.c (expand_builtin_apply_args_1): Likewise.
(expand_builtin_apply): Likewise.
* calls.c (expand_call): Likewise.
(emit_library_call_value_1): Likewise.
* cfgexpand.c (expand_asm_stmt): Likewise.
* cfghooks.c (verify_flow_info): Likewise.
* cfgloopmanip.c (remove_path): Likewise.
* cfgrtl.c (rtl_verify_bb_layout): Remove dead initialization.
* cilk-common.c (get_frame_arg): Remove dead store.
* combine.c (simplify_if_then_else): Likewise.
(simplify_comparison): Likewise.
* config/i386/i386.c (choose_baseaddr): Likewise.
(ix86_expand_int_movcc): Likewise.
(ix86_expand_vec_perm): Likewise. 
(ix86_expand_set_or_movmem): Likewise.
(ix86_preferred_output_reload_class): Likewise.
(expand_vec_perm_palignr): Likewise.
* cselib.c (cselib_expand_value_rtx_1): Likewise.
(cselib_record_sets): Likewise.
* dojump.c (do_jump_by_parts_greater_rtx): Likewise.
* dwarf2out.c (loc_list_from_tree_1): Likewise.
* final.c (shorten_branches): Likewise.
* gengtype.c (read_input_list): Remove dead initialization.
* gimplify.c (gimplify_function_tree): Remove dead store.
* ipa-chkp.c (chkp_maybe_create_clone): Likewise.
* ipa-inline-analysis.c (account_size_time): Likewise.
* ipa-prop.c (ipa_make_edge_direct_to_target): Remove dead
initialization.
* ira-color.c (setup_profitable_hard_regs): Remove dead store
(color_pass): Likewise.
* ira.c (rtx_moveable_p): Likewise.
* lra-eliminations.c (eliminate_regs_in_insn): Likewise.
* lra.c (collect_non_operand_hard_regs): Remove dead initialization.
(lra_set_insn_recog_data): Likewise.
* reg-stack.c (check_asm_stack_operands): Remove dead store.
* regrename.c (regrename_analyze): Move initialization out of loop.
(scan_rtx): Remove dead store.
* reorg.c (fill_slots_from_thread): Remove dead initialization.
* sel-sched.c (moveup_expr): Remove dead store.
* tree-inline.c (copy_bb): Likewise.
(tree_function_versioning): Likewise.
* tree-parloops.c (ref_conflicts_with_region): Remove dead
initialization.
* tree-ssa-loop-ivcanon.c (tree_unroll_loops_completely): Remove 
dead store.
* tree-ssa-loop-niter.c (number_of_iterations_lt_to_ne): Remove dead
initialization.
* tree-ssa-phionlycprop.c (pass_phi_only_cprop::execute): Remove 
dead store.
* tree-ssa-structalias.c (solve_graph): Likewise.
* tree-ssa-threadedge.c (thread_across_edge): Likewise.
* tree-vect-loop-manip.c (vect_do_peeling): Remove dead
initialization.
* tree-vect-loop.c (get_initial_def_for_induction): Likewise.
(vect_create_epilog_for_reduction): Likewise.
* tree-vect-slp.c (vect_analyze_slp_instance): Likewise.
* varasm.c (output_constructor_regular_field): Remove dead store.

diff --git a/gcc/asan.c b/gcc/asan.c
index edcc6ea5a913..207e29341614 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -1342,7 +1342,6 @@ asan_emit_stack_protection (rtx base, rtx pbase, unsigned 
int alignb,
= shadow_mem_size (current_offset - last_offset + rounded_size);
  shadow_mem = adjust_address (shadow_mem, VOIDmode, shift);
  last_offset = var_offset + rounded_size;
- current_offset = last_offset;
}
 
}
diff --git a/gcc/bt-load.c b/gcc/bt-load.c
index 

Re: [patch, libfortran] [patch, fortran] Fix PR 79956, part two, attempt 2

[Markus Trippelsdorf]

On 2017.03.13 at 23:24 +0100, Thomas Koenig wrote:
> Hello world,
> 
> Following Richard's suggestion, I have implemented the GFC_ASSERT macro
> and used it to (hopefully) silence the ominous warning and allow
> further optimization.

Unfortunately the single GFC_ASSERT isn't enough:

libgfortran/intrinsics/reshape_generic.c: In function ‘reshape_internal’:
libgfortran/intrinsics/reshape_generic.c:257:21: warning: ‘sstride[0]’ may be 
used uninitialized in this function [-Wmaybe-uninitialized]
   sstride0 = sstride[0] * size;
  ~~~^~~
The following patch is needed to silence them all.

diff --git a/libgfortran/intrinsics/reshape_generic.c 
b/libgfortran/intrinsics/reshape_generic.c
index 43a822f87ae..d7c9e74853a 100644
--- a/libgfortran/intrinsics/reshape_generic.c
+++ b/libgfortran/intrinsics/reshape_generic.c
@@ -66,6 +66,7 @@ reshape_internal (parray *ret, parray *source, shape_type 
*shape,
   index_type shape_data[GFC_MAX_DIMENSIONS];
 
   rdim = GFC_DESCRIPTOR_EXTENT(shape,0);
+  GFC_ASSERT(rdim>0);
   if (rdim != GFC_DESCRIPTOR_RANK(ret))
 runtime_error("rank of return array incorrect in RESHAPE intrinsic");
 
@@ -158,6 +159,10 @@ reshape_internal (parray *ret, parray *source, shape_type 
*shape,
 
   source_extent = 1;
   sdim = GFC_DESCRIPTOR_RANK (source);
+  /* sdim is always > 0; this lets the compiler optimize more and
+ avoids a warning.  */
+  GFC_ASSERT(sdim>0);
+
   for (n = 0; n < sdim; n++)
{
  index_type se;
@@ -219,6 +224,7 @@ reshape_internal (parray *ret, parray *source, shape_type 
*shape,
 }
 
   sdim = GFC_DESCRIPTOR_RANK (source);
+  GFC_ASSERT(sdim>0);
   ssize = 1;
   sempty = 0;
   for (n = 0; n < sdim; n++)

-- 
Markus

Re: [PATCH] libiberty: Initialize d_printing in all cplus_demangle_* functions.

[Markus Trippelsdorf]

On 2017.03.12 at 23:05 +0100, Mark Wielaard wrote:
> While integrating the d_printing recursion guard change into gdb I
> noticed we forgot to initialize the demangle_component d_printing
> field in cplus_demangle_fill_{name,extended_operator,ctor,dtor}.
> As is done in cplus_demangle_fill_{component,builtin_type,operator}.
> It happened to work because in gcc all demangle_components were
> allocated through d_make_empty. But gdb has its own allocation
> mechanism (as might other users).

Nick has synced the binutils-gdb repro with gcc's. 
I think you should commit your fix as obvious.

And it would be nice if Nick could sync again, after your patch went in.

-- 
Markus

[RFC PATCH libiberty] Fix infinite recursion in demangler

[Markus Trippelsdorf]

The following patch from Mark Wielaard fixes many (all?) open demangler
bugs that happen because we overflow the stack due to infinite
recursion.

I'm not sure why Mark didn't post the patch himself.
Anyway here it is.
Any comments?
Thanks.

diff --git a/include/demangle.h b/include/demangle.h
index 7cc955dc28d5..996203b2d786 100644
--- a/include/demangle.h
+++ b/include/demangle.h
@@ -494,6 +494,11 @@ struct demangle_component
   /* The type of this component.  */
   enum demangle_component_type type;
 
+  /* Guard against recursive component printing.
+ Initialize to zero.  Private to d_print_comp.
+ All other fields are final after initialization.  */
+  int d_printing;
+
   union
   {
 /* For DEMANGLE_COMPONENT_NAME.  */
@@ -688,7 +693,7 @@ cplus_demangle_v3_components (const char *mangled, int 
options, void **mem);
 
 extern char *
 cplus_demangle_print (int options,
-  const struct demangle_component *tree,
+  struct demangle_component *tree,
   int estimated_length,
   size_t *p_allocated_size);
 
@@ -708,7 +713,7 @@ cplus_demangle_print (int options,
 
 extern int
 cplus_demangle_print_callback (int options,
-   const struct demangle_component *tree,
+   struct demangle_component *tree,
demangle_callbackref callback, void *opaque);
 
 #ifdef __cplusplus
diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
index f0dbf9381c6b..341a4182c0b6 100644
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -172,10 +172,10 @@ static struct demangle_component *d_mangled_name (struct 
d_info *, int);
 static struct demangle_component *d_type (struct d_info *);
 
 #define cplus_demangle_print d_print
-static char *d_print (int, const struct demangle_component *, int, size_t *);
+static char *d_print (int, struct demangle_component *, int, size_t *);
 
 #define cplus_demangle_print_callback d_print_callback
-static int d_print_callback (int, const struct demangle_component *,
+static int d_print_callback (int, struct demangle_component *,
  demangle_callbackref, void *);
 
 #define cplus_demangle_init_info d_init_info
@@ -264,7 +264,7 @@ struct d_print_mod
  in which they appeared in the mangled string.  */
   struct d_print_mod *next;
   /* The modifier.  */
-  const struct demangle_component *mod;
+  struct demangle_component *mod;
   /* Whether this modifier was printed.  */
   int printed;
   /* The list of templates which applies to this modifier.  */
@@ -530,7 +530,7 @@ static inline void d_append_string (struct d_print_info *, 
const char *);
 static inline char d_last_char (struct d_print_info *);
 
 static void
-d_print_comp (struct d_print_info *, int, const struct demangle_component *);
+d_print_comp (struct d_print_info *, int, struct demangle_component *);
 
 static void
 d_print_java_identifier (struct d_print_info *, const char *, int);
@@ -539,25 +539,25 @@ static void
 d_print_mod_list (struct d_print_info *, int, struct d_print_mod *, int);
 
 static void
-d_print_mod (struct d_print_info *, int, const struct demangle_component *);
+d_print_mod (struct d_print_info *, int, struct demangle_component *);
 
 static void
 d_print_function_type (struct d_print_info *, int,
-   const struct demangle_component *,
+   struct demangle_component *,
struct d_print_mod *);
 
 static void
 d_print_array_type (struct d_print_info *, int,
-const struct demangle_component *,
+struct demangle_component *,
 struct d_print_mod *);
 
 static void
-d_print_expr_op (struct d_print_info *, int, const struct demangle_component 
*);
+d_print_expr_op (struct d_print_info *, int, struct demangle_component *);
 
 static void d_print_cast (struct d_print_info *, int,
- const struct demangle_component *);
+ struct demangle_component *);
 static void d_print_conversion (struct d_print_info *, int,
-   const struct demangle_component *);
+   struct demangle_component *);
 
 static int d_demangle_callback (const char *, int,
 demangle_callbackref, void *);
@@ -923,6 +923,7 @@ d_make_empty (struct d_info *di)
   if (di->next_comp >= di->num_comps)
 return NULL;
   p = >comps[di->next_comp];
+  p->d_printing = 0;
   ++di->next_comp;
   return p;
 }
@@ -4249,7 +4250,7 @@ d_last_char (struct d_print_info *dpi)
 CP_STATIC_IF_GLIBCPP_V3
 int
 cplus_demangle_print_callback (int options,
-   const struct demangle_component *dc,
+   struct demangle_component *dc,
demangle_callbackref callback, void *opaque)
 {
   struct d_print_info dpi;
@@ -4292,7 +4293,7 @@ 

Re: [wwwdocs] Add a case to porting_to + a question wrt validity of another one

[Markus Trippelsdorf]

On 2017.02.08 at 13:56 +0100, Marek Polacek wrote:
> On Wed, Feb 08, 2017 at 12:54:44PM +0100, Markus Trippelsdorf wrote:
> > On 2017.02.08 at 12:05 +0100, Marek Polacek wrote:
> > > On Tue, Feb 07, 2017 at 04:17:48PM -0500, Jason Merrill wrote:
> > > > On Tue, Feb 7, 2017 at 9:13 AM, Jonathan Wakely <jwak...@redhat.com> 
> > > > wrote:
> > > > > On 07/02/17 15:04 +0100, Marek Polacek wrote:
> > > > >>
> > > > >> Thanks much for the review.  Looks ok now?
> > > > 
> > > > I'd suggest adding something to say that the reason these are now
> > > > being diagnosed is that G++ used to treat e.g. this->member, where
> > > > member has a non-dependent type, as type-dependent, and now it
> > > > doesn't.
> > > 
> > > Like this?
> > > 
> > > Index: porting_to.html
> > > ===
> > > RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-7/porting_to.html,v
> > > retrieving revision 1.5
> > > diff -u -r1.5 porting_to.html
> > > --- porting_to.html   7 Feb 2017 14:22:39 -   1.5
> > > +++ porting_to.html   8 Feb 2017 11:05:22 -
> > > @@ -52,7 +52,9 @@
> > >  
> > >  
> > >  As a consequence, the following examples are invalid and G++ will no 
> > > longer
> > > -compile them:
> > > +compile them, because, in the following examples, G++ used to treat
> > 
> > Please drop the redundant ", in the following examples".
> 
> Why?  I don't mean in generally, I only mean in in the context of those
> examples.

I'm not suggesting to drop both. But:

»As a consequence, the following examples are invalid and G++ will no
longer compile them, because, in the following examples, G++ used to...«

The second occurrence of "the following examples" doesn't add any new
meaning and is therefore redundant, because you are already referring to
"the following examples".

-- 
Markus

Re: [wwwdocs] Add a case to porting_to + a question wrt validity of another one

[Markus Trippelsdorf]

On 2017.02.08 at 12:05 +0100, Marek Polacek wrote:
> On Tue, Feb 07, 2017 at 04:17:48PM -0500, Jason Merrill wrote:
> > On Tue, Feb 7, 2017 at 9:13 AM, Jonathan Wakely  wrote:
> > > On 07/02/17 15:04 +0100, Marek Polacek wrote:
> > >>
> > >> Thanks much for the review.  Looks ok now?
> > 
> > I'd suggest adding something to say that the reason these are now
> > being diagnosed is that G++ used to treat e.g. this->member, where
> > member has a non-dependent type, as type-dependent, and now it
> > doesn't.
> 
> Like this?
> 
> Index: porting_to.html
> ===
> RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-7/porting_to.html,v
> retrieving revision 1.5
> diff -u -r1.5 porting_to.html
> --- porting_to.html   7 Feb 2017 14:22:39 -   1.5
> +++ porting_to.html   8 Feb 2017 11:05:22 -
> @@ -52,7 +52,9 @@
>  
>  
>  As a consequence, the following examples are invalid and G++ will no longer
> -compile them:
> +compile them, because, in the following examples, G++ used to treat

Please drop the redundant ", in the following examples".

> +this->member where member has a non-dependent type, as
> +type-dependent, and now it doesn't.
>  
>  
>  
> 
>   Marek
> 

-- 
Markus

Re: [wwwdocs] Add a case to porting_to + a question wrt validity of another one

[Markus Trippelsdorf]

On 2017.02.06 at 18:13 +0100, Marek Polacek wrote:
> This patch adds a description of something I noticed while doing the 
> Fedora mass rebuild.  Do we want to say more about the invalidity of
> the incomplete type case?
> +
> +GCC 7 no longer accepts ill-formed code involving use of an incomplete type:
> +
> +namespace N {
> +class C;
> +class A {
> +  C fn1();
> +};
> +template  class B : A {
> +  void fn2() { fn1().x; }
> +};
> +}
> +
> +
> +

You could drop the namespace. Also "struct A" would be better, because
otherwise fn1 is a private and thus unaccessible in fn2.

You could also mention Chromium and Node.js as prominent examples.

-- 
Markus

Re: gcc.css colors

[Markus Trippelsdorf]

On 2017.02.01 at 12:41 +0100, Jakub Jelinek wrote:
> Hi!
> 
> On Wed, Feb 01, 2017 at 12:27:05PM +0100, Markus Trippelsdorf wrote:
> > > I've tried various color settings of gnome-terminal (both white on black 
> > > and
> > > black on white plus the different color sets) and all of them except 
> > > for Solarized (which is shades of grey) look much brighter than your 
> > > colors.
> > 
> > See attached screenshot. (I use konsole, but even gnome-terminal
> > supports truecolor now. So one has complete freedom in choosing the
> > default colors.)
> 
> Sure, one can customize anything.  The point is, what colors are configured
> by default and thus used by most of the users?
> 
> Just tried konsole (both Linux colors and Black on white) look like this
> here:

This points to the core of the issue. I guess most users use a dark
(black) background in the terminal. And on a black background the
gcc.css colors are perfectly readable. But because we use a white
background on the website, the colors have way too little contrast and
become hard to read.

-- 
Markus

Re: gcc.css colors

[Markus Trippelsdorf]

On 2017.02.01 at 12:14 +0100, Jakub Jelinek wrote:
> On Wed, Feb 01, 2017 at 11:55:53AM +0100, Markus Trippelsdorf wrote:
> > On 2017.02.01 at 11:48 +0100, Jakub Jelinek wrote:
> > > On Wed, Feb 01, 2017 at 11:45:14AM +0100, Markus Trippelsdorf wrote:
> > > > Some colors on e.g. https://gcc.gnu.org/gcc-7/changes.html are nearly
> > > > unreadable. So what about the following patch?
> > > > 
> > > > --- gcc_orig.css2017-02-01 11:39:17.634017498 +0100
> > > > +++ gcc.css 2017-02-01 11:40:23.979244263 +0100
> > > > @@ -58,8 +58,8 @@
> > > >  }
> > > >  div.copyright p:nth-child(3) { margin-bottom: 0; }
> > > >  
> > > > -.boldcyan{ font-weight:bold; color:cyan; }
> > > > -.boldlime{ font-weight:bold; color:lime; }
> > > > +.boldcyan{ font-weight:bold; color:#25a9a9; }
> > > > +.boldlime{ font-weight:bold; color:green;}
> > > >  .boldmagenta { font-weight:bold; color:magenta; }
> > > >  .boldred { font-weight:bold; color:red; }
> > > >  .boldblue{ font-weight:bold; color:blue; }
> > > 
> > > I think the intent is that they actually match closely what gcc/libasan 
> > > emits
> > > (that of course depends on the exact terminal setting).
> > > So are your colors closer to what gcc/libasan print or not?
> > 
> > As you said, the exact terminal colors are user definable.
> > But yes, the change above bring them closer to what I see in my
> > terminal. 
> 
> Exactly the opposite here, the current colors match very closely what I get
> (gnome-terminal, White on black, Linux console color set),
> your colors are completely different.
> 
> E.g. in changes.html, the Asan spans with boldlime are using
> \033[1m\033[32m
> in libsanitizer, and the note color in gcc by default is
> \033[1m\033[36m
> 
> I've tried various color settings of gnome-terminal (both white on black and
> black on white plus the different color sets) and all of them except 
> for Solarized (which is shades of grey) look much brighter than your colors.

See: http://i.imgur.com/rAlEdVy.png. (I use konsole, but even gnome-
terminal supports truecolor now. So one has complete freedom in choosing
the default colors.)


-- 
Markus

Re: gcc.css colors

[Markus Trippelsdorf]

On 2017.02.01 at 11:48 +0100, Jakub Jelinek wrote:
> On Wed, Feb 01, 2017 at 11:45:14AM +0100, Markus Trippelsdorf wrote:
> > Some colors on e.g. https://gcc.gnu.org/gcc-7/changes.html are nearly
> > unreadable. So what about the following patch?
> > 
> > --- gcc_orig.css2017-02-01 11:39:17.634017498 +0100
> > +++ gcc.css 2017-02-01 11:40:23.979244263 +0100
> > @@ -58,8 +58,8 @@
> >  }
> >  div.copyright p:nth-child(3) { margin-bottom: 0; }
> >  
> > -.boldcyan{ font-weight:bold; color:cyan; }
> > -.boldlime{ font-weight:bold; color:lime; }
> > +.boldcyan{ font-weight:bold; color:#25a9a9; }
> > +.boldlime{ font-weight:bold; color:green;}
> >  .boldmagenta { font-weight:bold; color:magenta; }
> >  .boldred { font-weight:bold; color:red; }
> >  .boldblue{ font-weight:bold; color:blue; }
> 
> I think the intent is that they actually match closely what gcc/libasan emits
> (that of course depends on the exact terminal setting).
> So are your colors closer to what gcc/libasan print or not?

As you said, the exact terminal colors are user definable.
But yes, the change above bring them closer to what I see in my
terminal. 
And readability is much improved by the patch IMHO.

-- 
Markus

gcc.css colors

[Markus Trippelsdorf]

Some colors on e.g. https://gcc.gnu.org/gcc-7/changes.html are nearly
unreadable. So what about the following patch?

--- gcc_orig.css2017-02-01 11:39:17.634017498 +0100
+++ gcc.css 2017-02-01 11:40:23.979244263 +0100
@@ -58,8 +58,8 @@
 }
 div.copyright p:nth-child(3) { margin-bottom: 0; }
 
-.boldcyan{ font-weight:bold; color:cyan; }
-.boldlime{ font-weight:bold; color:lime; }
+.boldcyan{ font-weight:bold; color:#25a9a9; }
+.boldlime{ font-weight:bold; color:green;}
 .boldmagenta { font-weight:bold; color:magenta; }
 .boldred { font-weight:bold; color:red; }
 .boldblue{ font-weight:bold; color:blue; }

-- 
Markus

Re: [PATCH 5/5] add support for width and precision ranges (PR 78703)

[Markus Trippelsdorf]

On 2017.01.27 at 08:45 -0700, Martin Sebor wrote:
> On 01/27/2017 12:44 AM, Markus Trippelsdorf wrote:
> > On 2017.01.22 at 16:53 -0700, Martin Sebor wrote:
> > > This is the last patch in the series.  It adds logic to handle
> > > non-constant width and precision with range information to help
> > > reduce both false positives false negatives.  The patch replaces
> > > the scalar width and precision with two element arrays throughout
> > > the pass and makes adjustments to reflect their bounds in the byte
> > > counters.  Since the basic infrastructure for this is present in
> > > the code the changes are fairly mechanical.
> > 
> > > commit c0a1f67fedb531abaf4760e8cd5b9b037ef5d4c4
> > > Author: Martin Sebor <mse...@redhat.com>
> > > Date:   Sun Jan 22 12:37:33 2017 -0700
> > > 
> > > 2017-01-22  Martin Sebor  <mse...@redhat.com>
> > > 
> > >   * gimple-ssa-sprintf.c (adjust_for_width_or_precision): Change
> > >   to accept adjustment as an array.
> > >   (get_int_range): New function.
> > >   (struct directive): Make width and prec arrays.
> > >   (directive::set_width, directive::set_precision): Call 
> > > get_int_range.
> > >   (format_integer, format_floating): Handle width and precision 
> > > ranges.
> > >   (format_string, parse_directive): Same.
> > 
> > This is the third time that you broke bootstrap with MPFR 2.x.x:
> 
> 
> 
> > 
> >  gcc/gimple-ssa-sprintf.c:1501: error: 'MPFR_RNDN' was not declared in this 
> > scope
> > 
> > Please be more careful in the future.
> 
> I'm sorry, but I'm not sure how to be more careful here.  I can
> realistically only build and test with one version of MPFR.  I
> proposed a solution to this problem in the past: to download
> the oldest supported version of it and the other dependencies:
> 
>   https://gcc.gnu.org/ml/gcc-patches/2016-09/msg01596.html
> 
> That was rejected by those who think GCC should target the latest,
> which is what I have been doing.  So I suggested an alternative
> solution in response to your last message about this problem:
> 
>   https://gcc.gnu.org/ml/gcc-patches/2017-01/msg01886.html
> 
> I haven't had a chance to implement it yet.  You are welcome to
> contribute a patch in the meantime to help forestall any further
> accidental breakage.

Fortunately this was already done by Jakub's r244966.
"MPFR_RNDN MPFR_RNDZ MPFR_RNDU MPFR_RNDD" are now "poisoned", so the issue
cannot happen again.

-- 
Markus

Re: [PATCH committed] Fix build failure with MPFR 2.4.x (gimple-ssa-sprintf.c)

[Markus Trippelsdorf]

On 2017.01.27 at 08:41 +0100, Rainer Orth wrote:
> Hi Martin,
> 
> > On 01/24/2017 02:37 AM, Markus Trippelsdorf wrote:
> >> MPFR_RNDx was introduced in MPFR 3.0.0. Since the minimal version that
> >> gcc checks for is 2.4.0, this leads to a build failure.
> >>
> >> The fix is straightforward.
> >>
> >> Tested on x86_64-pc-linux-gnu. Committed to trunk as obvious.
> >>
> >>* gimple-ssa-sprintf.c (format_floating): Change MPFR_RNDx to
> >>GMP_RNDx for compatibility.
> >
> > This was changed once before for this reason (in r240350).  I forgot
> > all about it and put it back in my latest patch for some reason.  I
> > don't remember why exactly but I suspect I might have been trying to
> > overcome some MPFR oddity.  It might help to #undef the MPFR_RNDx
> > macros after including  to avoid this in the future.  In any
> > event, thanks for the fix.
> 
> we certainly should do something like this: your latest patch introduced
> the same failure again:

Well, you cannot #undef enum members like MPFR_RNDU.

-- 
Markus

Re: [PATCH 5/5] add support for width and precision ranges (PR 78703)

[Markus Trippelsdorf]

On 2017.01.22 at 16:53 -0700, Martin Sebor wrote:
> This is the last patch in the series.  It adds logic to handle
> non-constant width and precision with range information to help
> reduce both false positives false negatives.  The patch replaces
> the scalar width and precision with two element arrays throughout
> the pass and makes adjustments to reflect their bounds in the byte
> counters.  Since the basic infrastructure for this is present in
> the code the changes are fairly mechanical.

> commit c0a1f67fedb531abaf4760e8cd5b9b037ef5d4c4
> Author: Martin Sebor 
> Date:   Sun Jan 22 12:37:33 2017 -0700
> 
> 2017-01-22  Martin Sebor  
> 
>   * gimple-ssa-sprintf.c (adjust_for_width_or_precision): Change
>   to accept adjustment as an array.
>   (get_int_range): New function.
>   (struct directive): Make width and prec arrays.
>   (directive::set_width, directive::set_precision): Call get_int_range.
>   (format_integer, format_floating): Handle width and precision ranges.
>   (format_string, parse_directive): Same.

This is the third time that you broke bootstrap with MPFR 2.x.x:

 gcc/gimple-ssa-sprintf.c:1501: error: 'MPFR_RNDN' was not declared in this 
scope 

Please be more careful in the future.

-- 
Markus

[PATCH committed] Fix build failure with MPFR 2.4.x (gimple-ssa-sprintf.c)

[Markus Trippelsdorf]

MPFR_RNDx was introduced in MPFR 3.0.0. Since the minimal version that
gcc checks for is 2.4.0, this leads to a build failure.

The fix is straightforward. 

Tested on x86_64-pc-linux-gnu. Committed to trunk as obvious.

* gimple-ssa-sprintf.c (format_floating): Change MPFR_RNDx to
GMP_RNDx for compatibility.
 
diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index 66edc3ec1cc9..283b6251d743 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -1560,7 +1560,7 @@ format_floating (const directive , tree arg)
   rounding in either direction can result in longer output.  */
mpfr_t mpfrval;
mpfr_init2 (mpfrval, rfmt->p);
-   mpfr_from_real (mpfrval, rvp, i ? MPFR_RNDU : MPFR_RNDD);
+   mpfr_from_real (mpfrval, rvp, i ? GMP_RNDU : GMP_RNDD);
 
/* Use the MPFR rounding specifier to round down in the first
   iteration and then up.  In most but not all cases this will
-- 
Markus

Re: [libstdc++,doc] Use canonical address for C++ ABI

[Markus Trippelsdorf]

On 2017.01.22 at 17:20 +0100, Gerald Pfeifer wrote:
> On Sun, 1 Jan 2017, Gerald Pfeifer wrote:
> > When I updated those URLs in June 2015, I must have missed these
> > references in libstdc++ land.
> > 
> > Fixed thusly (via revision 244001), and I plan on backporting to 
> > GCC 6 and possibly GCC 5 later.
> > 
> > Gerald
> > 
> > 2017-01-01  Gerald Pfeifer  
> > 
> > * doc/xml/faq.xml: Update address of C++ ABI link.
> > * doc/xml/manual/abi.xml: Ditto.
> 
> Pushed to the GCC 6 and GCC 5 branches now.
> 
> Jonathan, this is the last one I had in my list of backports touching 
> libstdc++ docs; there likely will be more I'll be doing for mainline
> in the coming days/weeks (and some of those may be suitable for back-
> ports then).

Please note that they will move again soon, see:
http://sourcerytools.com/pipermail/cxx-abi-dev/2017-January/003021.html

-- 
Markus

Re: [PATCH] Speed-up use-after-scope (re-writing to SSA) (version 2)

[Markus Trippelsdorf]

On 2017.01.20 at 15:27 +0100, Jakub Jelinek wrote:
> On Fri, Jan 20, 2017 at 03:08:21PM +0100, Martin Liška wrote:
> > Unfortunately this way would not work as clobber marks content of the 
> > memory as uninitialize
> > is different behavior that just marking a memory can be used (and maybe 
> > already contains a value).
> > 
> > This shows the problem:
> > 
> > #include 
> > 
> > char cc;
> > char ptr[] = "sparta2";
> > 
> > void get(char **x)
> > {
> >   *x = ptr;
> > }
> >   
> > int main()
> > {
> >   char *here = 
> > 
> >   for (;;)
> > {
> > next_line:
> > if (here == NULL)
> >   __builtin_abort();
> > get ();
> > if (strcmp (here, "sparta") == 0)
> > goto next_line;
> > else if (strcmp (here, "sparta2") == 0)
> >   break;
> > }
> > }
> > 
> > With the patch, DSE would optimize out '*here = ' and thus aborts. The 
> > problem is definitely
> > related to goto magic, where we are more defensive in placement of 
> > ASAN_MARK(UNPOISON,...).
> > Hope your optimization is still valid for situations w/o artificial 
> > ASAN_MARK(UNPOISON,...) placed due
> > to goto magic.
> > 
> > Do we still want to do it now, or postponing to GCC 8 would be better 
> > option?
> 
> I'd still like to resolve it for GCC 7 if at all possible, I think otherwise
> -fsanitize=address is by default unnecessarily slower (so it is a regression
> anyway).

Another possibility would be to disable use-after-scope for gcc-7 (like
LLVM) and re-enable it for gcc-8.

diff --git a/gcc/opts.c b/gcc/opts.c
index 5f573a16ff15..2664b54133e4 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -993,7 +993,7 @@ finish_options (struct gcc_options *opts, struct 
gcc_options *opts_set,
  enabled.  */
   if ((opts->x_flag_sanitize & SANITIZE_USER_ADDRESS)
   && !opts_set->x_flag_sanitize_address_use_after_scope)
-opts->x_flag_sanitize_address_use_after_scope = true;
+opts->x_flag_sanitize_address_use_after_scope = false;

   /* Force -fstack-reuse=none in case -fsanitize-address-use-after-scope
  is enabled.  */

-- 
Markus

Re: [PATCH C++] Fix PR77489 -- mangling of discriminator >= 10

[Markus Trippelsdorf]

On 2017.01.18 at 16:25 +0100, Jakub Jelinek wrote:
> On Wed, Jan 18, 2017 at 04:16:44PM +0100, Markus Trippelsdorf wrote:
> > No. It appears to work even without the additional condition:
> > 
> >  % g++ -fabi-version=10 -Wabi=11 -Wall -c gcc/testsuite/g++.dg/abi/pr77489.C
> > gcc/testsuite/g++.dg/abi/pr77489.C:56:16: warning: the mangled name of 
> > ‘localVar’ changes between -fabi-version=10 (_ZZ3foovE8localVar_11) and 
> > -fabi-version=11 (_ZZ3foovE8localVar__11_) [-Wabi]
> >  static int localVar = 12;
> > ^~~~
> > gcc/testsuite/g++.dg/abi/pr77489.C:52:16: warning: the mangled name of 
> > ‘localVar’ changes between -fabi-version=10 (_ZZ3foovE8localVar_10) and 
> > -fabi-version=11 (_ZZ3foovE8localVar__10_) [-Wabi]
> >  static int localVar = 11;
> > ^~~~
> 
> But it is less efficient then for the the one digit discriminators.
> When we set G.need_abi_warning unnecessarily, then mangle_decl has to
> mangle it again with different ABI flags and compare the two mangled
> identifiers.  If G.need_abi_warning is not set, we avoid that.

Ok, fair enough.

PR c++/77489
* mangle.c (write_discriminator): Reorganize abi warning check.

diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c
index d1b107cbb1d..31b0f543f31 100644
--- a/gcc/cp/mangle.c
+++ b/gcc/cp/mangle.c
@@ -1966,11 +1966,12 @@ write_discriminator (const int discriminator)
   if (discriminator > 0)
 {
   write_char ('_');
-  if (abi_version_at_least (11) && discriminator - 1 >= 10)
+  if (discriminator - 1 >= 10)
{
- write_char ('_');
  if (abi_warn_or_compat_version_crosses (11))
G.need_abi_warning = 1;
+ if (abi_version_at_least (11))
+   write_char ('_');
}
   write_unsigned_number (discriminator - 1);
   if (abi_version_at_least (11) && discriminator - 1 >= 10)

-- 
Markus

Re: [PATCH C++] Fix PR77489 -- mangling of discriminator >= 10

[Markus Trippelsdorf]

On 2017.01.18 at 10:03 -0500, Jason Merrill wrote:
> On Wed, Jan 18, 2017 at 9:23 AM, Markus Trippelsdorf
> <mar...@trippelsdorf.de> wrote:
> > On 2017.01.18 at 09:11 -0500, Jason Merrill wrote:
> >> On Wed, Jan 18, 2017 at 3:55 AM, Markus Trippelsdorf
> >> <mar...@trippelsdorf.de> wrote:
> >> > On 2017.01.17 at 13:26 -0500, Jason Merrill wrote:
> >> >> On Thu, Jan 12, 2017 at 2:36 AM, Markus Trippelsdorf
> >> >> <mar...@trippelsdorf.de> wrote:
> >> > +  if (abi_version_at_least (11) && discriminator - 1 >= 10)
> >> > +   {
> >> > + write_char ('_');
> >> > + if (abi_warn_or_compat_version_crosses (11))
> >> > +   G.need_abi_warning = 1;
> >>
> >> This check should be outside the abi_version_at_least block; we want
> >> to warn if -fabi-version=10 and -Wabi=11.
> >
> > +  if (abi_warn_or_compat_version_crosses (11))
> > +   G.need_abi_warning = 1;
> >write_char ('_');
> >if (abi_version_at_least (11) && discriminator - 1 >= 10)
> 
> Ah, but it does need to be controlled by the second part of this test;
> we only want the warning if the discriminator will be two digits.

No. It appears to work even without the additional condition:

 % g++ -fabi-version=10 -Wabi=11 -Wall -c gcc/testsuite/g++.dg/abi/pr77489.C
gcc/testsuite/g++.dg/abi/pr77489.C:56:16: warning: the mangled name of 
‘localVar’ changes between -fabi-version=10 (_ZZ3foovE8localVar_11) and 
-fabi-version=11 (_ZZ3foovE8localVar__11_) [-Wabi]
 static int localVar = 12;
^~~~
gcc/testsuite/g++.dg/abi/pr77489.C:52:16: warning: the mangled name of 
‘localVar’ changes between -fabi-version=10 (_ZZ3foovE8localVar_10) and 
-fabi-version=11 (_ZZ3foovE8localVar__10_) [-Wabi]
 static int localVar = 11;
^~~~

-- 
Markus

Re: [PATCH C++] Fix PR77489 -- mangling of discriminator >= 10

[Markus Trippelsdorf]

On 2017.01.18 at 09:11 -0500, Jason Merrill wrote:
> On Wed, Jan 18, 2017 at 3:55 AM, Markus Trippelsdorf
> <mar...@trippelsdorf.de> wrote:
> > On 2017.01.17 at 13:26 -0500, Jason Merrill wrote:
> >> On Thu, Jan 12, 2017 at 2:36 AM, Markus Trippelsdorf
> >> <mar...@trippelsdorf.de> wrote:
> > +  if (abi_version_at_least (11) && discriminator - 1 >= 10)
> > +   {
> > + write_char ('_');
> > + if (abi_warn_or_compat_version_crosses (11))
> > +   G.need_abi_warning = 1;
> 
> This check should be outside the abi_version_at_least block; we want
> to warn if -fabi-version=10 and -Wabi=11.

Ok, thanks.
Is the following OK?

PR c++/77489
* mangle.c (write_discriminator): Move abi warning check out of
if block.

diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c
index d1b107cbb1d..6f2e86d43f3 100644
--- a/gcc/cp/mangle.c
+++ b/gcc/cp/mangle.c
@@ -1965,13 +1965,11 @@ write_discriminator (const int discriminator)
   /* If discriminator is zero, don't write anything.  Otherwise...  */
   if (discriminator > 0)
 {
+  if (abi_warn_or_compat_version_crosses (11))
+   G.need_abi_warning = 1;
   write_char ('_');
   if (abi_version_at_least (11) && discriminator - 1 >= 10)
-   {
- write_char ('_');
- if (abi_warn_or_compat_version_crosses (11))
-   G.need_abi_warning = 1;
-   }
+   write_char ('_');
   write_unsigned_number (discriminator - 1);
   if (abi_version_at_least (11) && discriminator - 1 >= 10)
write_char ('_');
-- 
Markus

Re: [PATCH C++] Fix PR77489 -- mangling of discriminator >= 10

[Markus Trippelsdorf]

On 2017.01.18 at 09:55 +0100, Markus Trippelsdorf wrote:
> index cac3d8bc65e9..767c8f42fee9 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -2252,7 +2252,9 @@ attributes that affect type identity, such as ia32 
> calling convention
>  attributes (e.g. @samp{stdcall}).
>  
>  Version 11, which first appeared in G++ 7, corrects the mangling of
> -sizeof... expressions.  It also implies
> +sizeof... expressions.  For multiple entities with the same name within
> +a function, that are declared in different scopes, the mangling now
> +changes starting with the eighth occurrence.  It also implies
>  @option{-fnew-inheriting-ctors}.
>  
>  See also @option{-Wabi}.

Sorry, this should of course read "twelfth occurrence". I have fixed it.

-- 
Markus

Re: [PATCH C++] Fix PR77489 -- mangling of discriminator >= 10

[Markus Trippelsdorf]

On 2017.01.17 at 13:26 -0500, Jason Merrill wrote:
> On Thu, Jan 12, 2017 at 2:36 AM, Markus Trippelsdorf
> <mar...@trippelsdorf.de> wrote:
> > On 2017.01.11 at 13:03 +0100, Jakub Jelinek wrote:
> >> On Wed, Jan 11, 2017 at 12:48:29PM +0100, Markus Trippelsdorf wrote:
> >> > @@ -1965,7 +1966,11 @@ write_discriminator (const int discriminator)
> >> >if (discriminator > 0)
> >> >  {
> >> >write_char ('_');
> >> > +  if (abi_version_at_least(11) && discriminator - 1 >= 10)
> >> > +   write_char ('_');
> >> >write_unsigned_number (discriminator - 1);
> >> > +  if (abi_version_at_least(11) && discriminator - 1 >= 10)
> >> > +   write_char ('_');
> >>
> >> Formatting nits, there should be space before (11).
> >>
> >> > +// { dg-final { scan-assembler "_ZZ3foovE8localVar__10_" } }
> >> > +// { dg-final { scan-assembler "_ZZ3foovE8localVar__11_" } }
> >>
> >> Would be nice to also
> >> // { dg-final { scan-assembler "_ZZ3foovE8localVar_9" } }
> >>
> >> Otherwise, I defer to Jason (primarily whether this doesn't need
> >> ABI version 12).
> >
> > Thanks for review. I will fix these issues.
> > Jason said on IRC that he is fine with ABI version 11.
> >
> > Ok for trunk?
> 
> This also needs the same (invoke.texi and warning) changes that Nathan
> pointed out on your other mangling patch.

Thanks. For the record, here is what I have committed:

libiberty:

PR c++/77489
* cp-demangle.c (d_discriminator): Handle discriminator >= 10.
* testsuite/demangle-expected: Add tests for discriminator.

gcc:
PR c++/77489
* doc/invoke.texi (fabi-version): Document discriminator mangling.

gcc/cp:

PR c++/77489
* mangle.c (write_discriminator): Handle discriminator >= 10.

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@244566 
138bc75d-0d04-0410-961f-82ee72b054a4

diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c
index 5f2fa35d29e8..67ec5c31f170 100644
--- a/gcc/cp/mangle.c
+++ b/gcc/cp/mangle.c
@@ -1952,7 +1952,8 @@ discriminator_for_string_literal (tree /*function*/,
   return 0;
 }
 
-/*:= _ 
+/*:= _ # when number < 10
+ := __  _ # when number >= 10
 
The discriminator is used only for the second and later occurrences
of the same name within a single function. In this case  is
@@ -1965,7 +1966,15 @@ write_discriminator (const int discriminator)
   if (discriminator > 0)
 {
   write_char ('_');
+  if (abi_version_at_least (11) && discriminator - 1 >= 10)
+   {
+ write_char ('_');
+ if (abi_warn_or_compat_version_crosses (11))
+   G.need_abi_warning = 1;
+   }
   write_unsigned_number (discriminator - 1);
+  if (abi_version_at_least (11) && discriminator - 1 >= 10)
+   write_char ('_');
 }
 }
 
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index cac3d8bc65e9..767c8f42fee9 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -2252,7 +2252,9 @@ attributes that affect type identity, such as ia32 
calling convention
 attributes (e.g. @samp{stdcall}).
 
 Version 11, which first appeared in G++ 7, corrects the mangling of
-sizeof... expressions.  It also implies
+sizeof... expressions.  For multiple entities with the same name within
+a function, that are declared in different scopes, the mangling now
+changes starting with the eighth occurrence.  It also implies
 @option{-fnew-inheriting-ctors}.
 
 See also @option{-Wabi}.
diff --git a/gcc/testsuite/g++.dg/abi/pr77489.C 
b/gcc/testsuite/g++.dg/abi/pr77489.C
new file mode 100644
index ..13c41cc0acfa
--- /dev/null
+++ b/gcc/testsuite/g++.dg/abi/pr77489.C
@@ -0,0 +1,63 @@
+// { dg-options -fabi-version=11 }
+
+extern void bar(int*);
+
+void foo()
+{
+  {
+static int localVar = 0;
+bar();
+  }
+  {
+static int localVar = 1;
+bar();
+  }
+  {
+static int localVar = 2;
+bar();
+  }
+  {
+static int localVar = 3;
+bar();
+  }
+  {
+static int localVar = 4;
+bar();
+  }
+  {
+static int localVar = 5;
+bar();
+  }
+  {
+static int localVar = 6;
+bar();
+  }
+  {
+static int localVar = 7;
+bar();
+  }
+  {
+static int localVar = 8;
+bar();
+  }
+  {
+static int localVar = 9;
+bar();
+  }
+  {
+static int localVar = 10;
+bar();
+  }
+  {
+static int localVar = 11;
+bar();
+  }
+  {
+static int localVar = 12;
+bar();
+  }
+}
+
+// { dg-final { scan-assembler "_ZZ3foovE8localVar_9" } }
+// { dg-final { scan-assembler "_ZZ3

[PATCH v2 C++] Fix PR70182 -- missing "on" in mangling of unresolved operators

[Markus Trippelsdorf]

On 2017.01.11 at 08:21 -0500, Nathan Sidwell wrote:
> On 01/11/2017 08:16 AM, Markus Trippelsdorf wrote:
> 
> > --- a/gcc/cp/mangle.c
> > +++ b/gcc/cp/mangle.c
> > @@ -2813,6 +2813,8 @@ write_template_args (tree args)
> >  static void
> >  write_member_name (tree member)
> >  {
> > +  if (abi_version_at_least (11) && IDENTIFIER_OPNAME_P (member))
> > +write_string ("on");
> 
> It looks like you need to:
> 1) add documentation to doc/invoke.texi (-fabi-version)
> 2) add something like:
>   if (abi_warn_or_compat_version_crosses (11))
>   G.need_abi_warning = 1;
> into that if clause.

Thanks for the review. Here is a new patch:

OK for trunk?


libiberty:

PR c++/70182
* cp-demangle.c (d_unqualified_name): Handle "on" for
operator names.
* testsuite/demangle-expected: Add tests.

gcc/cp:

PR c++/70182
* mangle.c (write_template_args): Add "on" for operator names.

gcc:

PR c++/70182
* doc/invoke.texi (fabi-version): Mention mangling fix for
operator names.


diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c
index e831deb31405..ef9e8fa71221 100644
--- a/gcc/cp/mangle.c
+++ b/gcc/cp/mangle.c
@@ -2813,6 +2813,12 @@ write_template_args (tree args)
 static void
 write_member_name (tree member)
 {
+  if (abi_version_at_least (11) && IDENTIFIER_OPNAME_P (member))
+{
+  write_string ("on");
+  if (abi_warn_or_compat_version_crosses (11))
+   G.need_abi_warning = 1;
+}
   if (identifier_p (member))
 write_unqualified_id (member);
   else if (DECL_P (member))
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 9c77db25e776..75ef5875c0cb 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -2250,7 +2250,7 @@ attributes that affect type identity, such as ia32 
calling convention
 attributes (e.g. @samp{stdcall}).
 
 Version 11, which first appeared in G++ 7, corrects the mangling of
-sizeof... expressions.  It also implies
+sizeof... expressions and operator names.  It also implies
 @option{-fnew-inheriting-ctors}.
 
 See also @option{-Wabi}.
diff --git a/gcc/testsuite/g++.dg/abi/mangle13.C 
b/gcc/testsuite/g++.dg/abi/mangle13.C
index 716c4c36f410..c8822a34039c 100644
--- a/gcc/testsuite/g++.dg/abi/mangle13.C
+++ b/gcc/testsuite/g++.dg/abi/mangle13.C
@@ -1,4 +1,4 @@
-// { dg-options "-fabi-version=0" }
+// { dg-options "-fabi-version=10" }
 
 struct A {
   template  int f ();
diff --git a/gcc/testsuite/g++.dg/abi/mangle37.C 
b/gcc/testsuite/g++.dg/abi/mangle37.C
index 691566b384ba..4dd87e84c108 100644
--- a/gcc/testsuite/g++.dg/abi/mangle37.C
+++ b/gcc/testsuite/g++.dg/abi/mangle37.C
@@ -1,5 +1,6 @@
 // Testcase for mangling of expressions involving operator names.
 // { dg-do compile { target c++11 } }
+// { dg-options "-fabi-version=10" }
 // { dg-final { scan-assembler "_Z1fI1AEDTclonplfp_fp_EET_" } }
 // { dg-final { scan-assembler "_Z1gI1AEDTclonplIT_Efp_fp_EES1_" } }
 // { dg-final { scan-assembler "_Z1hI1AEDTcldtfp_miEET_" } }
diff --git a/gcc/testsuite/g++.dg/abi/pr70182.C 
b/gcc/testsuite/g++.dg/abi/pr70182.C
new file mode 100644
index ..d299362910c1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/abi/pr70182.C
@@ -0,0 +1,28 @@
+// { dg-options "-fabi-version=0" }
+
+struct A {
+  template  int f ();
+  int operator+();
+  operator int ();
+  template  
+  int operator-();
+};
+
+typedef int (A::*P)();
+
+template  struct S {};
+
+template  void g (S<::template f >) {}
+template  void g (S<::operator+ >) {}
+template  void g (S<::operator int>) {}
+template  void g (S<::template operator-  >) {}
+
+template void g (S<::f >);
+template void g (S<::operator+>);
+template void g (S<::operator int>);
+template void g (S<::operator- >);
+
+// { dg-final { scan-assembler _Z1gI1AEv1SIXadsrT_1fIiEEE } }
+// { dg-final { scan-assembler _Z1gI1AEv1SIXadsrT_onplEE } }
+// { dg-final { scan-assembler _Z1gI1AEv1SIXadsrT_oncviEE } }
+// { dg-final { scan-assembler _Z1gI1AEv1SIXadsrT_onmiIdEEE } }
diff --git a/gcc/testsuite/g++.dg/dfp/mangle-1.C 
b/gcc/testsuite/g++.dg/dfp/mangle-1.C
index 455d3e4c0ef6..ee9644b27a53 100644
--- a/gcc/testsuite/g++.dg/dfp/mangle-1.C
+++ b/gcc/testsuite/g++.dg/dfp/mangle-1.C
@@ -1,4 +1,5 @@
 // { dg-do compile }
+// { dg-options "-fabi-version=10" }
 
 // Mangling of classes from std::decimal are special-cased.
 // Derived from g++.dg/abi/mangle13.C.
diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
index d84929eca20d..f0dbf9381c6b 100644
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -1594,6 +1594,8 @@ d_unqualified_name (struct d_info *di)
 ret = d_source_name (di);
   else if (IS_LOWER (peek))
 {
+  if (peek == 'o' && d_peek_next_char (di) == 'n')
+   

Re: [PATCH C++] Fix PR77489 -- mangling of discriminator >= 10

[Markus Trippelsdorf]

On 2017.01.11 at 13:03 +0100, Jakub Jelinek wrote:
> On Wed, Jan 11, 2017 at 12:48:29PM +0100, Markus Trippelsdorf wrote:
> > @@ -1965,7 +1966,11 @@ write_discriminator (const int discriminator)
> >if (discriminator > 0)
> >  {
> >write_char ('_');
> > +  if (abi_version_at_least(11) && discriminator - 1 >= 10)
> > +   write_char ('_');
> >write_unsigned_number (discriminator - 1);
> > +  if (abi_version_at_least(11) && discriminator - 1 >= 10)
> > +   write_char ('_');
> 
> Formatting nits, there should be space before (11).
> 
> > +// { dg-final { scan-assembler "_ZZ3foovE8localVar__10_" } }
> > +// { dg-final { scan-assembler "_ZZ3foovE8localVar__11_" } }
> 
> Would be nice to also
> // { dg-final { scan-assembler "_ZZ3foovE8localVar_9" } }
> 
> Otherwise, I defer to Jason (primarily whether this doesn't need
> ABI version 12).

Thanks for review. I will fix these issues. 
Jason said on IRC that he is fine with ABI version 11.

Ok for trunk?

-- 
Markus

[PATCH C++] Fix PR70182 -- missing "on" in mangling of unresolved operators

[Markus Trippelsdorf]

The ABI says:


   ::= [gs] 
   ::= sr  
   ::= srN  + E 

   ::= [gs] sr + E 


   ::= 
   ::= on 
   ::= on  
   ::= dn  int f ();
diff --git a/gcc/testsuite/g++.dg/abi/mangle37.C 
b/gcc/testsuite/g++.dg/abi/mangle37.C
index 691566b384ba..4dd87e84c108 100644
--- a/gcc/testsuite/g++.dg/abi/mangle37.C
+++ b/gcc/testsuite/g++.dg/abi/mangle37.C
@@ -1,5 +1,6 @@
 // Testcase for mangling of expressions involving operator names.
 // { dg-do compile { target c++11 } }
+// { dg-options "-fabi-version=10" }
 // { dg-final { scan-assembler "_Z1fI1AEDTclonplfp_fp_EET_" } }
 // { dg-final { scan-assembler "_Z1gI1AEDTclonplIT_Efp_fp_EES1_" } }
 // { dg-final { scan-assembler "_Z1hI1AEDTcldtfp_miEET_" } }
diff --git a/gcc/testsuite/g++.dg/abi/pr70182.C 
b/gcc/testsuite/g++.dg/abi/pr70182.C
new file mode 100644
index ..d299362910c1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/abi/pr70182.C
@@ -0,0 +1,28 @@
+// { dg-options "-fabi-version=0" }
+
+struct A {
+  template  int f ();
+  int operator+();
+  operator int ();
+  template  
+  int operator-();
+};
+
+typedef int (A::*P)();
+
+template  struct S {};
+
+template  void g (S<::template f >) {}
+template  void g (S<::operator+ >) {}
+template  void g (S<::operator int>) {}
+template  void g (S<::template operator-  >) {}
+
+template void g (S<::f >);
+template void g (S<::operator+>);
+template void g (S<::operator int>);
+template void g (S<::operator- >);
+
+// { dg-final { scan-assembler _Z1gI1AEv1SIXadsrT_1fIiEEE } }
+// { dg-final { scan-assembler _Z1gI1AEv1SIXadsrT_onplEE } }
+// { dg-final { scan-assembler _Z1gI1AEv1SIXadsrT_oncviEE } }
+// { dg-final { scan-assembler _Z1gI1AEv1SIXadsrT_onmiIdEEE } }
diff --git a/gcc/testsuite/g++.dg/dfp/mangle-1.C 
b/gcc/testsuite/g++.dg/dfp/mangle-1.C
index 455d3e4c0ef6..ee9644b27a53 100644
--- a/gcc/testsuite/g++.dg/dfp/mangle-1.C
+++ b/gcc/testsuite/g++.dg/dfp/mangle-1.C
@@ -1,4 +1,5 @@
 // { dg-do compile }
+// { dg-options "-fabi-version=10" }
 
 // Mangling of classes from std::decimal are special-cased.
 // Derived from g++.dg/abi/mangle13.C.
diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
index d84929eca20d..f0dbf9381c6b 100644
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -1594,6 +1594,8 @@ d_unqualified_name (struct d_info *di)
 ret = d_source_name (di);
   else if (IS_LOWER (peek))
 {
+  if (peek == 'o' && d_peek_next_char (di) == 'n')
+   d_advance (di, 2);
   ret = d_operator_name (di);
   if (ret != NULL && ret->type == DEMANGLE_COMPONENT_OPERATOR)
{
diff --git a/libiberty/testsuite/demangle-expected 
b/libiberty/testsuite/demangle-expected
index 07e258fe58b3..c1cfa1545eca 100644
--- a/libiberty/testsuite/demangle-expected
+++ b/libiberty/testsuite/demangle-expected
@@ -4682,3 +4682,10 @@ _ZZ3foovE8localVar__9_
 
 _ZZ3foovE8localVar__12
 _ZZ3foovE8localVar__12
+
+# PR 70182
+_Z1gI1AEv1SIXadsrT_onplEE
+void g(S<::operator+>)
+
+_Z1gI1AEv1SIXadsrT_plEE
+void g(S<::operator+>)
-- 
Markus

[PATCH C++] Fix PR77489 -- mangling of discriminator >= 10

[Markus Trippelsdorf]

Currently gcc mangles symbols wrongly when the discriminator is greater
than ten. The fix is straightforward. The demangler now handles both the
old and the new correct mangling.

Tested on ppc64le. OK for trunk?

Thanks.

libiberty:

PR c++/77489
* cp-demangle.c (d_discriminator): Handle discriminator >= 10.
* testsuite/demangle-expected: Add tests for discriminator.

gcc/cp:

PR c++/77489
* mangle.c (write_discriminator): Handle discriminator >= 10.

diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c
index 5f2fa35d29e8..ee75f4a25621 100644
--- a/gcc/cp/mangle.c
+++ b/gcc/cp/mangle.c
@@ -1952,7 +1952,8 @@ discriminator_for_string_literal (tree /*function*/,
   return 0;
 }
 
-/*:= _ 
+/*:= _ # when number < 10
+ := __  _ # when number >= 10
 
The discriminator is used only for the second and later occurrences
of the same name within a single function. In this case  is
@@ -1965,7 +1966,11 @@ write_discriminator (const int discriminator)
   if (discriminator > 0)
 {
   write_char ('_');
+  if (abi_version_at_least(11) && discriminator - 1 >= 10)
+   write_char ('_');
   write_unsigned_number (discriminator - 1);
+  if (abi_version_at_least(11) && discriminator - 1 >= 10)
+   write_char ('_');
 }
 }
 
diff --git a/gcc/testsuite/g++.dg/abi/pr77489.C 
b/gcc/testsuite/g++.dg/abi/pr77489.C
new file mode 100644
index ..f640bb6f0676
--- /dev/null
+++ b/gcc/testsuite/g++.dg/abi/pr77489.C
@@ -0,0 +1,62 @@
+// { dg-options -fabi-version=11 }
+
+extern void bar(int*);
+
+void foo()
+{
+  {
+static int localVar = 0;
+bar();
+  }
+  {
+static int localVar = 1;
+bar();
+  }
+  {
+static int localVar = 2;
+bar();
+  }
+  {
+static int localVar = 3;
+bar();
+  }
+  {
+static int localVar = 4;
+bar();
+  }
+  {
+static int localVar = 5;
+bar();
+  }
+  {
+static int localVar = 6;
+bar();
+  }
+  {
+static int localVar = 7;
+bar();
+  }
+  {
+static int localVar = 8;
+bar();
+  }
+  {
+static int localVar = 9;
+bar();
+  }
+  {
+static int localVar = 10;
+bar();
+  }
+  {
+static int localVar = 11;
+bar();
+  }
+  {
+static int localVar = 12;
+bar();
+  }
+}
+
+// { dg-final { scan-assembler "_ZZ3foovE8localVar__10_" } }
+// { dg-final { scan-assembler "_ZZ3foovE8localVar__11_" } }
diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
index 15ef3b48785f..d84929eca20d 100644
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -3609,7 +3609,11 @@ d_local_name (struct d_info *di)
 }
 }
 
-/*  ::= _ <(non-negative) number>
+/*  ::= _ # when number < 10
+   ::= __  _ # when number >= 10
+
+::= _ # when number >=10
+   is also accepted to support gcc versions that wrongly mangled that way.
 
We demangle the discriminator, but we don't print it out.  FIXME:
We should print it out in verbose mode.  */
@@ -3617,14 +3621,28 @@ d_local_name (struct d_info *di)
 static int
 d_discriminator (struct d_info *di)
 {
-  int discrim;
+  int discrim, num_underscores = 1;
 
   if (d_peek_char (di) != '_')
 return 1;
   d_advance (di, 1);
+  if (d_peek_char (di) == '_')
+{
+  ++num_underscores;
+  d_advance (di, 1);
+}
+
   discrim = d_number (di);
   if (discrim < 0)
 return 0;
+  if (num_underscores > 1 && discrim >= 10)
+{
+  if (d_peek_char (di) == '_')
+   d_advance (di, 1);
+  else
+   return 0;
+}
+
   return 1;
 }
 
diff --git a/libiberty/testsuite/demangle-expected 
b/libiberty/testsuite/demangle-expected
index b65dcd3450e9..07e258fe58b3 100644
--- a/libiberty/testsuite/demangle-expected
+++ b/libiberty/testsuite/demangle-expected
@@ -4666,3 +4666,19 @@ void eat(int*&, Foo()::{lambda(auto:1
 
 _Z3eatIPiZ3BarIsEvvEUlPsPT_PT0_E0_EvRS3_RS5_
 void eat(int*&, 
void Bar()::{lambda(short*, auto:1*, auto:2*)#2}&)
+
+# PR 77489
+_ZZ3foovE8localVar_9
+foo()::localVar
+
+_ZZ3foovE8localVar_10
+foo()::localVar
+

+_ZZ3foovE8localVar__10_
+foo()::localVar
+
+_ZZ3foovE8localVar__9_
+_ZZ3foovE8localVar__9_
+
+_ZZ3foovE8localVar__12
+_ZZ3foovE8localVar__12

-- 
Markus

Re: [PATCH] better handling of ranges (PR 78703)

[Markus Trippelsdorf]

On 2016.12.23 at 14:25 -0700, Martin Sebor wrote:
> Bug 78703 points out that the decimal point character in floating
> directives can be longer than just one byte (in locales where the
> decimal point is a multibyte character).  The decimal point can
> result in anywhere between 1 and MB_LEN_MAX bytes.  This is unlikely
> but locales with two-byte decimal point are known to exist, and
> the gimple-ssa-sprintf pass must handle them correctly.
> 
> In a comment on the bug Jakub suggests that while printf return
> value optimization must correctly deal with the worst case (i.e.,
> MB_LEN_MAX of 6 for UTF-8), reflecting the worst case in the text
> of warnings could be confusing to users most of whom expect
> a single byte decimal point.
> 
> Finally, a limitation of the gimple-ssa-sprintf pass has been that
> it only understands constant width and precision and treats others
> as essentially unlimited even if they are constrained to a limited
> range of values.  This results in false positives and negatives
> that can be avoided.
> 
> The attached patch enhances the pass to overcome both of these
> limitations.  It does that by first replacing the exact byte counter
> with two other counters: 1) a likely counter that tracks the number
> of bytes a directive is likely to result in, and 2) an "unlikely"
> byte for lack of a better name, that tracks the unlikely maximum
> byte count in cases like multibyte decimal point, and second by
> adding range handling for width and precision specified by the
> asterisk (such as in sprintf("%*.*i", w, p, i)).
> 
> The patch resulted in more extensive changes than I initially
> intended but the result is a simplified implementation.  A good
> amount of the changes is factoring code out into more general
> functions that can be shared throughout the pass.
> 
> With these enhancements, although the support for ranges in the
> pass is complete, it's not as robust as it could be.  I think
> having the pass run later could improve things.
> 
> The pass does produce a fair number of warnings for calls to
> snprintf in the linux kernel.  Some of these I suspect will be
> considered false positives.  I think it might be worth splitting
> up the snprintf warning from -Wformat-length and adding a separate
> option to control it.

I must confess that I find these -Wformat-length warnings nearly
unparseable. They need much better documentation that explains where all
these seemingly random looking numbers are coming from.

And I also think that -Wformat-length should not be enabled by -Wall and
not even by -Wextra. The user should specifically ask for it.

-- 
Markus

Re: [PATCH] fix powerpc64le bootstrap failure caused by r243661 (PR 78817)

[Markus Trippelsdorf]

On 2016.12.19 at 09:34 -0700, Martin Sebor wrote:
> On 12/19/2016 09:17 AM, Jakub Jelinek wrote:
> > Or apply the patch I've posted which doesn't suffer from this problem,
> > or revert the -Wnonnull changes and resolve somehow in GCC 8.
> 
> I would prefer your patch if it solves the problem.  In fact,
> as I said before, I'm fine with your patch in any case. 

Then please lets go with Jakub's patch. And also please revert r243736
as Richi suggested, because it isn't necessary anymore.

-- 
Markus

Re: [PATCH] fix powerpc64le bootstrap failure caused by r243661 (PR 78817)

[Markus Trippelsdorf]

On 2016.12.16 at 18:27 +0100, Jakub Jelinek wrote:
> On Fri, Dec 16, 2016 at 10:10:00AM -0700, Martin Sebor wrote:
> > > No.  The first call to sm_read_sector just doesn't exit.  So it is warning
> > > about dead code.
> > 
> > If the code is dead then GCC should eliminate it.  With it eliminated
> 
> There is (especially with jump threading, but not limited to that, other
> optimizations may result in that too), code that even very smart optimizing
> compiler isn't able to prove that is dead.
> 
> > the warning would be gone.  The warning isn't smart and it doesn't
> > try to be.  It only works with what GCC gives it.  In this case the
> > dump shows that GCC thinks the code is reachable.  If it isn't that
> > would seem to highlight a missed optimization opportunity, not a need
> > to make the warning smarter than the optimizer.
> 
> No, it highlights that the warning is done in a wrong place where it suffers
> from too many false positives.

Another issue with Martin's patch is that it adds many false positives
when one uses -fsanitize=undefined, e.g.:

 % cat test.ii
struct A {
  char *msg;
  A(const char *);
};
A::A(const char *p1) {
  msg = new char[__builtin_strlen(p1) + 1];
  __builtin_strcpy(msg, p1);
}

 % g++ -Wall  -O2 -c test.ii
 % g++ -Wall -fsanitize=undefined -O2 -c test.ii
test.ii: In constructor ‘A::A(const char*)’:
test.ii:6:34: warning: argument 1 null where non-null expected [-Wnonnull]
   msg = new char[__builtin_strlen(p1) + 1];
  ^~~~
test.ii:6:34: note: in a call to built-in function ‘long unsigned int 
__builtin_strlen(const char*)’

-- 
Markus

Re: [PATCH] fix powerpc64le bootstrap failure caused by r243661 (PR 78817)

[Markus Trippelsdorf]

On 2016.12.16 at 11:07 -0700, Martin Sebor wrote:
> On 12/16/2016 10:27 AM, Jakub Jelinek wrote:
> > On Fri, Dec 16, 2016 at 10:10:00AM -0700, Martin Sebor wrote:
> > > > No.  The first call to sm_read_sector just doesn't exit.  So it is 
> > > > warning
> > > > about dead code.
> > >
> > > If the code is dead then GCC should eliminate it.  With it eliminated
> >
> > There is (especially with jump threading, but not limited to that, other
> > optimizations may result in that too), code that even very smart optimizing
> > compiler isn't able to prove that is dead.
> >
> > > the warning would be gone.  The warning isn't smart and it doesn't
> > > try to be.  It only works with what GCC gives it.  In this case the
> > > dump shows that GCC thinks the code is reachable.  If it isn't that
> > > would seem to highlight a missed optimization opportunity, not a need
> > > to make the warning smarter than the optimizer.
> >
> > No, it highlights that the warning is done in a wrong place where it suffers
> > from too many false positives.
>
> Asserting a claim without providing any data or evidence doesn't make
> it true.  We have seen fewer than 10 instances of it in just one out
> of four sizable projects.  At least three of these instances are
> debatable (as evidenced by the ongoing debate).  That can hardly be
> interpreted as "too many false positives."

So, to be fair a gave Jakub's patch a try and it has exactly the same
issues for the Linux kernel: sometimes the warning only triggers with
-O3, e.g.:

 % cat sm_ftl.i
int a;
void mtd_read_oob(int);
void sm_read_sector(int *buffer) {
  __builtin_memset(buffer, 0, 1);
  mtd_read_oob(a);
}
void sm_get_zone() { sm_read_sector(0); }

 % gcc -c -Wnonnull -O2 sm_ftl.i
 % gcc -c -Wnonnull -O3 sm_ftl.i
sm_ftl.i: In function ‘sm_get_zone’:
sm_ftl.i:4:3: warning: argument 1 null where non-null expected [-Wnonnull]
   __builtin_memset(buffer, 0, 1);
   ^~
sm_ftl.i:4:3: note: in a call to built-in function ‘__builtin_memset’

Also, Jakub's patch doesn't catch the following case:

(tools/perf/util/trace-event-info.c from Linux)
//...
dir = opendir(path);
if (!dir) {
err = -errno;
pr_debug("can't read directory '%s'", path);
goto out;
}
//... other stuff
out:
closedir(dir);

Whereas Martin's does.

--
Markus

Re: [PATCH] fix powerpc64le bootstrap failure caused by r243661 (PR 78817)

[Markus Trippelsdorf]

On 2016.12.15 at 19:27 -0700, Martin Sebor wrote:
> On 12/14/2016 09:19 PM, Jeff Law wrote:
> > On 12/14/2016 03:56 PM, Martin Sebor wrote:
> > > The -Wnonnull warning improvement (PR c/17308 - nonnull attribute
> > > not as useful as it could be) causes a couple of false positives
> > > in a powerpc64le bootstrap.  The attached fix suppresses them.
> > > It passes bootstrap on powerpc64le but tests are still running.
> > > 
> > > I couldn't reproduce the bootstrap comparison failure that Segher
> > > mentioned in his comment on the bug.  I've gone over the patch
> > > again to see if I could spot what could possibly be behind it
> > > but couldn't really see anything.  Jeff, you mentioned attribute
> > > nonnull is exploited by the null pointer optimization.  Do you
> > > think it could possibly have this effect in GCC?
> > It's certainly possible.  It would be an indicator that something in GCC
> > is passing a NULL pointer into a routine where it shouldn't at runtime
> > and that GCC is using its new knowledge to optimize the code assuming
> > that can't happen.
> > 
> > ie, it's an indicator of a latent bug in GCC.  Depending on the
> > difficulty in tracking down, you may need to revert the change.  This is
> > exactly the kind of scenario I was concerned about in that approval
> > message.
> 
> I couldn't reproduce the comparison error either on powerpc64-linux
> or on powerpc64le-linux.
> 
> > The change to the vec is OK.  Can you please verify
> > that if you install just that change that ppc bootstraps are restored
> > and if so, please install.
> 
> Done.
> 
> A profiledbootstrap exposed a few more instances of the enhanced
> -Wnonnull warning.  I spent some time analyzing them and decided
> that three of them are justified (those in gengtype-parse.c and
> gengtype.c) and the other three false positives.
> 
> The attached patch silences them.
> 
> The gengtype code alternates between checking for null pointers
> and assuming that the same pointers are not null (and passing nulls
> to nonnull functions like libiberty's lbasename).  It could probably
> benefit from some more cleanup but I'm out of cycles for that.
> 
> There are two outstanding instances of -Wnonnull in the profiled-
> bootstrap log that both point to the same function that I haven't
> figured out yet:
> 
> In function ‘void check_function_format(tree, int, tree_node**)’:
> cc1plus: warning: argument 1 null where non-null expected [-Wnonnull]
> /usr/include/string.h:398:15: note: in a call to function ‘size_t
> strlen(const char*)’ declared here
> 
> I'll deal with them next but I want to get this patch reviewed
> and approved so bootstrap can be restored on the targets affected
> by the vec.h warning.
> 
> While waiting for builds to finish I also built Binutils, Busybox,
> and the Linux kernel to see if the warning shows up there.  It does
> not.

It does for me with an allmodconf. At -O2 I get three warnings, and at
-O3 I get two additional warnings. Now these additional ones happen way
too deep into the pipeline to be reliable. (For a reduced testcase see:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78817#c8)

When you as the author of the warning have difficulties in figuring out
what causes these warnings, what about the average user?

Therefore -fsanitize=undefined should be preferred. It gives you better
info and a backtrace that makes diagnosis easy.

So in my opinion -Wnonnull should warn only for trivial cases.

-- 
Markus

[PATCH] Fix PR c++/71182 - UB in parser.c

[Markus Trippelsdorf]

An -fsanitize=undefined instrumented compiler shows:

 cp/parser.c:768:7: runtime error: member call on null pointer of type 'struct 
vec'

  760 static inline cp_token *
  761 cp_lexer_previous_token (cp_lexer *lexer)
  762 {
  763   cp_token_position tp = cp_lexer_previous_token_position (lexer);
  764
  765   /* Skip past purged tokens.  */
  766   while (tp->purged_p)
  767 {
  768   gcc_assert (tp != lexer->buffer->address ());
  769   tp--;
  770 }
  771
  772   return cp_lexer_token_at (lexer, tp);
  773 }

(gdb) p *lexer
$1 = {
  buffer = 0x0,
  last_token = 0x77e0c2f8,
  next_token = 0x77e0c298,
  saved_tokens = {
m_vec = 0x92c6b20
  },
  next = 0x75f96480,
  debugging_p = false,
  in_pragma = false
}

Fixed by guarding against the invalid member call on a null pointer.

Tested on ppc64le. Ok for trunk?

Thanks.

PR c++/71182
* parser.c (cp_lexer_previous_token): Guard against
member call on null pointer.

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 08f5f9e52ef2..f430af97fdf1 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -765,7 +765,7 @@ cp_lexer_previous_token (cp_lexer *lexer)
   /* Skip past purged tokens.  */
   while (tp->purged_p)
 {
-  gcc_assert (tp != lexer->buffer->address ());
+  gcc_assert (!lexer->buffer || tp != lexer->buffer->address ());
   tp--;
 }
 
--
Markus

Re: [patch part, libgcc] Add AVX-specific matmul

[Markus Trippelsdorf]

On 2016.11.30 at 08:17 +0100, Thomas Koenig wrote:
> Hello world,
> 
> the patch at https://gcc.gnu.org/ml/fortran/2016-11/msg00246.html
> (the one going to gcc-patches was rejected due to size of
> regernerated files) contains one libgcc change, which exposes
> the __cpu_model interface fox i386 to libgfortran.
> 
> The Fortran bits are OKd, but I need an approval from a libgcc
> maintainer (or some hint how to do this better :-).
> 
> I have attached the libgcc-specific part of the patch.

FYI this gives nice additional speedups for 178.galgel:
http://gcc.opensuse.org/SPEC/CFP/sb-czerny-head-64/178_galgel_recent_big.png

-- 
Markus

[PATCH] Fix PR tree-optimization/78598 - tree-ssa-loop-prefetch.c:835:16: runtime error: signed integer overflow

[Markus Trippelsdorf]

Using bootstrap-ubsan gcc to build mplayer shows:

tree-ssa-loop-prefetch.c:835:16: runtime error: signed integer overflow:
288230376151711743 * 64 cannot be represented in type 'long int'

Here signed und unsigned integers are mixed in a division resulting in
bogus results: (-83 + 64ULL -1) / 64ULL) == 288230376151711743

Fixed by casting the unsigned parameter to signed.

Tested on ppc64le.
OK for trunk?

Thanks.

PR tree-optimization/78598
* tree-ssa-loop-prefetch.c (ddown): Cast to signed to avoid
overflows.


diff --git a/gcc/tree-ssa-loop-prefetch.c b/gcc/tree-ssa-loop-prefetch.c
index 0a2ee5ea25fd..ead2543ada46 100644
--- a/gcc/tree-ssa-loop-prefetch.c
+++ b/gcc/tree-ssa-loop-prefetch.c
@@ -700,9 +700,9 @@ ddown (HOST_WIDE_INT x, unsigned HOST_WIDE_INT by)
   gcc_assert (by > 0);
 
   if (x >= 0)
-return x / by;
+return x / (HOST_WIDE_INT) by;
   else
-return (x + by - 1) / by;
+return (x + (HOST_WIDE_INT) by - 1) / (HOST_WIDE_INT) by;
 }
 
 /* Given a CACHE_LINE_SIZE and two inductive memory references
-- 
Markus

[PATCH] PR rtl-optimization/78596 - combine.c:12561:14: runtime error: left shift of negative value

[Markus Trippelsdorf]

Hopefully one last patch for UB in combine.c:

 combine.c:12561:14: runtime error: left shift of negative value -9

Fixed by casting to unsigned, as usual.

Tested on ppc64le.
OK for trunk?

Thanks.

PR rtl-optimization/78596
* combine.c (simplify_comparison): Cast to unsigned to avoid
left shifting of negative value.

diff --git a/gcc/combine.c b/gcc/combine.c
index faafcb741f41..e32c02b06810 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -12561,7 +12561,8 @@ simplify_comparison (enum rtx_code code, rtx *pop0, rtx 
*pop1)
  if (GET_CODE (op0) == LSHIFTRT)
code = unsigned_condition (code);

- const_op <<= INTVAL (XEXP (op0, 1));
+ const_op = (unsigned HOST_WIDE_INT) const_op
+ << INTVAL (XEXP (op0, 1));
  if (low_bits != 0
  && (code == GT || code == GTU
  || code == LE || code == LEU))

--
Markus

Re: [C++/78252] libiberty demangler crash with lambda (auto)

[Markus Trippelsdorf]

On 2016.11.30 at 14:06 -0500, Nathan Sidwell wrote:
> This patch fixes a problem in libiberty's symbol demangler.  With a
> templated forwarding function such as std::forward, we can end up emitting
> mangled function names that encode lambda information.  Lambdas with auto
> argument types have a synthesized templated operator(), and g++ uses that
> when mangling the lambda.
> 
> Unfortunately g++ doesn't notice the template parameters there mean 'auto'
> and emits regular template parameter references. (This is a bug, see below.)
> 
> But, as the forwarding function itself is a template, and the lambda is part
> of a template parameter substitution, we can end up with the demangler
> recursing unboundedly.  In other cases we can fail to demangle (returning
> null), or demangle to an unexpected type (substituting the current template
> parameter type into the place of the 'auto').
> 
> This patch fixes the demangler by noting when it's printing the argument
> types of a lambda.  In that case whenever we encounter a template parameter
> reference we emit 'auto', and also inhibit some &/&& smushing that needs
> checking.  AFAICT, once inside a lambda argument list we cannot encounter
> template parameter references that actually refer to an enclosing template
> argument list. That means we don't have the problem of disabling this
> additional check within the argument list printing.  I don't think we can
> meet a nested lambda type either, but the ++...-- idiom seemed safer to me.
> 
> We cannot do this substitution when parsing the mangled name, because g++
> applies the usual squangling back references as-if there really was a
> template parameter reference.  Later squangling references to the type
> containing the lambda argument may or may not require the reference to be to
> an enclosing template argument, or be auto, depending on the context of the
> squangle reference.
> 
> I've also included a c++ testcase to check the mangling of the lambdas that
> cause this.  While this is a g++ bug, it's an ABI-affecting one, and we
> shouldn't change the behaviour unintentionally.  I've not investigated why
> the mangler's failing to check is_auto, and will look at that later.  I
> imagine a fix will be -fabi-version dependent. I have filed 78621 to track
> it.

Thanks. This patch also fixes:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70909

-- 
Markus

Re: [v3 PATCH] Fix testsuite failures caused by the patch implementing LWG 2534.

[Markus Trippelsdorf]

On 2016.12.01 at 08:11 +0200, Ville Voutilainen wrote:
> On 1 December 2016 at 07:38, Markus Trippelsdorf <mar...@trippelsdorf.de> 
> wrote:
> > It breaks building Firefox:
> 
> Sigh, when writing a trait, write a proper trait. Does this patch fix
> the problem?

Yes it does. Thank you.

-- 
Markus

Re: [v3 PATCH] Fix testsuite failures caused by the patch implementing LWG 2534.

[Markus Trippelsdorf]

On 2016.11.30 at 16:25 +, Jonathan Wakely wrote:
> On 30/11/16 17:58 +0200, Ville Voutilainen wrote:
> >Fix testsuite failures caused by the patch implementing LWG 2534.
> >* include/std/istream (__is_convertible_to_basic_istream):
> >Change the return types of __check, introduce stream_type.
> >(operator>>(_Istream&&, _Tp&&)):
> >Use __is_convertible_to_basic_istream::stream_type as the return type.
> >* include/std/ostream (__is_convertible_to_basic_ostream):
> >Change the return types of __check, introduce stream_type.
> >(operator>>(_Ostream&&, _Tp&&)):
> >Use __is_convertible_to_basic_ostream::stream_type as the return type.
> 
> As discussed on IRC, please change "stream_type" to istream_type and
> ostream_type, as appropriate, because those names are already used by
> stream iterators, so users can't define them as macros.
> 
> And you could make the remove_reference happen inside the
> __is_convertible_to_basic_[io]stream trait, since it's only ever used
> with references, but that seems stylistic.
> 
> OK with the stream_type renaming.

It breaks building Firefox:

In file included from ../../dist/system_wrappers/ostream:4:0,
 from ../../dist/stl_wrappers/ostream:55,
 from 
/home/trippels/gcc_test/usr/local/include/c++/7.0.0/iterator:64,
 from ../../dist/system_wrappers/iterator:4,
 from ../../dist/stl_wrappers/iterator:55,
 from 
/home/trippels/gcc_test/usr/local/include/c++/7.0.0/backward/hashtable.h:63,
 from 
/home/trippels/gcc_test/usr/local/include/c++/7.0.0/ext/hash_map:64,
 from 
/home/trippels/gecko-dev/ipc/chromium/src/base/hash_tables.h:43,
 from 
/home/trippels/gecko-dev/ipc/chromium/src/base/file_path.h:72,
 from 
/home/trippels/gecko-dev/ipc/chromium/src/chrome/common/ipc_message_utils.h:12,
 from ../../dist/include/ipc/IPCMessageUtils.h:11,
 from ../../dist/include/mozilla/ipc/Transport_posix.h:11,
 from ../../dist/include/mozilla/ipc/Transport.h:15,
 from /home/trippels/gecko-dev/ipc/glue/BackgroundChild.h:10,
 from /home/trippels/gecko-dev/ipc/glue/BackgroundImpl.cpp:5,
 from 
/home/trippels/moz-build-dir/ipc/glue/Unified_cpp_ipc_glue0.cpp:2:
/home/trippels/gcc_test/usr/local/include/c++/7.0.0/ostream: In instantiation 
of ‘struct std::__is_convertible_to_basic_ostream’:
/home/trippels/gcc_test/usr/local/include/c++/7.0.0/ostream:656:5:   required 
by substitution of ‘template typename 
std::enable_if >, 
std::__is_convertible_to_basic_ostream<_Ostream>, 
std::__is_insertable<_Ostream&, const _Tp&, void> >::value, typename 
std::__is_convertible_to_basic_ostream<_Tp>::ostream_type>::type 
std::operator<<(_Ostream&&, const _Tp&) [with _Ostream = const 
mozilla::unused_t&; _Tp = {anonymous}::ParentImpl*]’
../../dist/include/nsCOMPtr.h:185:13:   required from ‘void operator<<(const 
mozilla::unused_t&, const already_AddRefed<{anonymous}::ParentImpl>&)’
/home/trippels/gecko-dev/ipc/glue/BackgroundImpl.cpp:1981:32:   required from 
here
/home/trippels/gcc_test/usr/local/include/c++/7.0.0/ostream:625:23: error: no 
matching function for call to ‘std::__is_convertible_to_basic_ostream::__check(const mozilla::unused_t*)’
   decltype(__check(declval::type*>()));
~~~^~
/home/trippels/gcc_test/usr/local/include/c++/7.0.0/ostream:620:37: note: 
candidate: template static std::basic_ostream<_Ch, _Up>& 
std::__is_convertible_to_basic_ostream<_Tp>::__check(std::basic_ostream<_Ch, 
_Up>*) [with _Ch = _Ch; _Up = _Up; _Tp = const mozilla::unused_t&]
 static basic_ostream<_Ch, _Up>& __check(basic_ostream<_Ch, _Up>*);
 ^~~
/home/trippels/gcc_test/usr/local/include/c++/7.0.0/ostream:620:37: note:   
template argument deduction/substitution failed:
/home/trippels/gcc_test/usr/local/include/c++/7.0.0/ostream:625:23: note:   
types ‘std::basic_ostream<_CharT, _Traits>’ and ‘const mozilla::unused_t’ have 
incompatible cv-qualifiers
   decltype(__check(declval::type*>()));
~~~^~
/home/trippels/gcc_test/usr/local/include/c++/7.0.0/ostream:622:17: note: 
candidate: static void 
std::__is_convertible_to_basic_ostream<_Tp>::__check(void*) [with _Tp = const 
mozilla::unused_t&] 
 static void __check(void*);
 ^~~
/home/trippels/gcc_test/usr/local/include/c++/7.0.0/ostream:622:17: note:   
conversion of argument 1 would be ill-formed:
/home/trippels/gcc_test/usr/local/include/c++/7.0.0/ostream:625:23: error: 
invalid conversion from ‘const void*’ to ‘void*’ [-fpermissive]
   decltype(__check(declval::type*>()));

[PATCH committed] Fix part of PR78555 - gcc/real.c:2890:25: runtime error: left shift of negative value -125

[Markus Trippelsdorf]

bootstrap-ubsan gcc shows:
 gcc/real.c:2890:25: runtime error: left shift of negative value -125

Fixed by casting to unsigned. 
Tested on ppc64le. Committed as obvious.

   PR ipa/78555
   * real.c (real_hash): Add cast to avoid left
   shifting of negative values.

diff --git a/gcc/real.c b/gcc/real.c
index 66e88e2ad366..eabe22de8510 100644
--- a/gcc/real.c
+++ b/gcc/real.c
@@ -2887,7 +2887,7 @@ real_hash (const REAL_VALUE_TYPE *r)
   return h;

 case rvc_normal:
-  h |= REAL_EXP (r) << 3;
+  h |= (unsigned int)REAL_EXP (r) << 3;
   break;

 case rvc_nan:

-- 
Markus

Re: [PATCH v2] Fix PR78588 - rtlanal.c:5210:38: runtime error: shift exponent 4294967295 is too large for 64-bit type

[Markus Trippelsdorf]

On 2016.11.29 at 15:25 -0600, Segher Boessenkool wrote:
> On Tue, Nov 29, 2016 at 05:00:05PM +0100, Markus Trippelsdorf wrote:
> > Building gcc with -fsanitize=undefined shows:
> >  rtlanal.c:5210:38: runtime error: shift exponent 4294967295 is too large 
> > for 64-bit type 'long unsigned int'
> > 
> > This happens because if_then_else_cond() in combine.c calls
> > num_sign_bit_copies() in rtlanal.c with mode==BLKmode.
> > 
> > 5205   bitwidth = GET_MODE_PRECISION (mode);
> > 5206   if (bitwidth > HOST_BITS_PER_WIDE_INT)
> > 5207 return 1;
> > 5208
> > 5209   nonzero = nonzero_bits (x, mode);
> > 5210   return nonzero & (HOST_WIDE_INT_1U << (bitwidth - 1))
> > 5211  ? 1 : bitwidth - floor_log2 (nonzero) - 1;
> > 
> > This causes (bitwidth - 1) to wrap around.
> 
> Could you also add a gcc_assert here?
> 
> > PR rtl-optimization/78588 
> > * combine.c (if_then_else_cond): Also guard against BLKmode.
> 
> Approved, please apply.  Thanks,

Because it can only happen when mode==BLKmode, this is what I checked
in:

diff --git a/gcc/combine.c b/gcc/combine.c
index 22fb7a9..a32a0ec 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -9176,7 +9176,7 @@ if_then_else_cond (rtx x, rtx *ptrue, rtx *pfalse)
   /* If X is known to be either 0 or -1, those are the true and
  false values when testing X.  */
   else if (x == constm1_rtx || x == const0_rtx
-  || (mode != VOIDmode
+  || (mode != VOIDmode && mode != BLKmode
   && num_sign_bit_copies (x, mode) == GET_MODE_PRECISION (mode)))
 {
   *ptrue = constm1_rtx, *pfalse = const0_rtx;
diff --git a/gcc/rtlanal.c b/gcc/rtlanal.c
index 4e4eb2e..60550ad 100644
--- a/gcc/rtlanal.c
+++ b/gcc/rtlanal.c
@@ -4840,6 +4840,8 @@ num_sign_bit_copies1 (const_rtx x, machine_mode mode, 
const_rtx known_x,
   if (mode == VOIDmode)
 mode = GET_MODE (x);
 
+  gcc_checking_assert (mode != BLKmode);
+
   if (mode == VOIDmode || FLOAT_MODE_P (mode) || FLOAT_MODE_P (GET_MODE (x))
   || VECTOR_MODE_P (GET_MODE (x)) || VECTOR_MODE_P (mode))
 return 1;

-- 
Markus

Re: [PATCH v2] Fix PR78588 - rtlanal.c:5210:38: runtime error: shift exponent 4294967295 is too large for 64-bit type

[Markus Trippelsdorf]

Here is v2 of the fix.

Building gcc with -fsanitize=undefined shows:
 rtlanal.c:5210:38: runtime error: shift exponent 4294967295 is too large for 
64-bit type 'long unsigned int'

This happens because if_then_else_cond() in combine.c calls
num_sign_bit_copies() in rtlanal.c with mode==BLKmode.

5205   bitwidth = GET_MODE_PRECISION (mode);
5206   if (bitwidth > HOST_BITS_PER_WIDE_INT)
5207 return 1;
5208
5209   nonzero = nonzero_bits (x, mode);
5210   return nonzero & (HOST_WIDE_INT_1U << (bitwidth - 1))
5211  ? 1 : bitwidth - floor_log2 (nonzero) - 1;

This causes (bitwidth - 1) to wrap around.

Fix by also guarding against BLKmode.

Tested on pcc64le.
OK for trunk?

Thanks.

PR rtl-optimization/78588 
* combine.c (if_then_else_cond): Also guard against BLKmode.

diff --git a/gcc/combine.c b/gcc/combine.c
index 22fb7a976538..a32a0ecc72fb 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -9176,7 +9176,7 @@ if_then_else_cond (rtx x, rtx *ptrue, rtx *pfalse)
   /* If X is known to be either 0 or -1, those are the true and
  false values when testing X.  */
   else if (x == constm1_rtx || x == const0_rtx
-  || (mode != VOIDmode
+  || (mode != VOIDmode && mode != BLKmode
   && num_sign_bit_copies (x, mode) == GET_MODE_PRECISION (mode)))
 {
   *ptrue = constm1_rtx, *pfalse = const0_rtx;

--
Markus

Re: [PATCH] Fix PR78588 - rtlanal.c:5210:38: runtime error: shift exponent 4294967295 is too large for 64-bit type

[Markus Trippelsdorf]

On 2016.11.29 at 16:01 +0100, Markus Trippelsdorf wrote:
> On 2016.11.29 at 15:21 +0100, Markus Trippelsdorf wrote:
> > On 2016.11.29 at 15:14 +0100, Jakub Jelinek wrote:
> > > On Tue, Nov 29, 2016 at 03:08:15PM +0100, Markus Trippelsdorf wrote:
> > > > Building gcc with -fsanitize=undefined shows:
> > > >  rtlanal.c:5210:38: runtime error: shift exponent 4294967295 is too
> > > >  large for 64-bit type 'long unsigned int'
> > > >
> > > > 5210   return nonzero & (HOST_WIDE_INT_1U << (bitwidth - 1))
> > > > 5211  ? 1 : bitwidth - floor_log2 (nonzero) - 1;
> > > >
> > > > Here (bitwidth - 1) wraps around because bitwidth is zero and unsigned.
> > >
> > > Which modes have precision of 0?  I'd expect just VOIDmode and BLKmode, 
> > > any
> > > others?  And for those I'd say it is a bug to call num_sign_bit_copies*.
> > 
> > Yes, only VOIDmode and BLKmode:
> > 
> >  233 const unsigned short mode_precision[NUM_MACHINE_MODES] =
> >  234 {
> >  235   0,   /* VOID */
> >  236   0,   /* BLK */
> 
> markus@x4 libsupc++ % cat cp-demangle.i
> d_demangle_callback_mangled() {
>   if (strncmp(d_demangle_callback_mangled, "", 1))
> d_type();
> }
> 
> markus@x4 libsupc++ % UBSAN_OPTIONS=print_stacktrace=1:halt_on_error=1 
> /var/tmp/gcc_build_dir_/./gcc/cc1 -w -fpreprocessed cp-demangle.i -quiet 
> -dumpbase cp-demangle.i -mtune=generic -march=x86-64 -auxbase cp-demangle -O2 
> -version -o /dev/null
> GNU C11 (GCC) version 7.0.0 20161129 (experimental) (x86_64-pc-linux-gnu)
> compiled by GNU C version 7.0.0 20161129 (experimental), GMP version 
> 6.1.1, MPFR version 3.1.5, MPC version 1.0.3, isl version none
> GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096
> GNU C11 (GCC) version 7.0.0 20161129 (experimental) (x86_64-pc-linux-gnu)
> compiled by GNU C version 7.0.0 20161129 (experimental), GMP version 
> 6.1.1, MPFR version 3.1.5, MPC version 1.0.3, isl version none
> GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096
> Compiler executable checksum: 7cca725773f8a0693a2905f8af7b733c
> ../../gcc/gcc/rtlanal.c:5210:38: runtime error: shift exponent 4294967295 is 
> too large for 64-bit type 'long unsigned int'
> #0 0x1b40fe1 in num_sign_bit_copies1 ../../gcc/gcc/rtlanal.c:5210
> #1 0x35ef5f1 in if_then_else_cond ../../gcc/gcc/combine.c:9180
> #2 0x35ef199 in if_then_else_cond ../../gcc/gcc/combine.c:9034
> #3 0x35ef199 in if_then_else_cond ../../gcc/gcc/combine.c:9034
> #4 0x3625f98 in combine_simplify_rtx ../../gcc/gcc/combine.c:5604
> #5 0x3632525 in subst ../../gcc/gcc/combine.c:5487
> #6 0x36327d6 in subst ../../gcc/gcc/combine.c:5425
> #7 0x3632bd7 in subst ../../gcc/gcc/combine.c:5354
> #8 0x3641a74 in try_combine ../../gcc/gcc/combine.c:3347
> #9 0x365727b in combine_instructions ../../gcc/gcc/combine.c:1421
> #10 0x365727b in rest_of_handle_combine ../../gcc/gcc/combine.c:14581
> #11 0x365727b in execute ../../gcc/gcc/combine.c:14626
> #12 0x195ad18 in execute_one_pass(opt_pass*) ../../gcc/gcc/passes.c:2370
> #13 0x195cbab in execute_pass_list_1 ../../gcc/gcc/passes.c:2459
> #14 0x195cbd4 in execute_pass_list_1 ../../gcc/gcc/passes.c:2460
> #15 0x195cc64 in execute_pass_list(function*, opt_pass*) 
> ../../gcc/gcc/passes.c:2470
> #16 0xc75deb in cgraph_node::expand() ../../gcc/gcc/cgraphunit.c:2001
> #17 0xc7b2fa in expand_all_functions ../../gcc/gcc/cgraphunit.c:2137
> #18 0xc7b2fa in symbol_table::compile() ../../gcc/gcc/cgraphunit.c:2494
> #19 0xc854b7 in symbol_table::compile() ../../gcc/gcc/cgraphunit.c:2587
> #20 0xc854b7 in symbol_table::finalize_compilation_unit() 
> ../../gcc/gcc/cgraphunit.c:2584
> #21 0x1d3ea10 in compile_file ../../gcc/gcc/toplev.c:488
> #22 0x629a14 in do_compile ../../gcc/gcc/toplev.c:1983
> #23 0x629a14 in toplev::main(int, char**) ../../gcc/gcc/toplev.c:2117
> #24 0x62c046 in main ../../gcc/gcc/main.c:39
> #25 0x7f4b6600f310 in __libc_start_main ../csu/libc-start.c:286
> #26 0x62c469 in _start (/var/tmp/gcc_build_dir_/gcc/cc1+0x62c469)

(gdb) p mode
$1 = BLKmode

#6  0x035ef5f2 in if_then_else_cond (x=0x760d3888,
ptrue=ptrue@entry=0x7fffd940, pfalse=pfalse@entry=0x7fffd950) at 
../../gcc/gcc/combine.c:9180
9180   && num_sign_bit_copies (x, mode) == GET_MODE_PRECISION 
(mode)))
(gdb) l
9175
9176  /* If X is known to be either 0 or -1, those are the true and
9177 false values when testing X.  */
9178  else if (x == constm1_rtx || x == const0_rtx
9179   || (mode != VOIDmode
9180   && num_sign_bit_copies (x, mode) == GET_MODE_PRECISION 
(mode)))
9181{
9182  *ptrue = constm1_rtx, *pfalse = const0_rtx;
9183  return x;
9184}


-- 
Markus

Re: [PATCH] Fix PR78588 - rtlanal.c:5210:38: runtime error: shift exponent 4294967295 is too large for 64-bit type

[Markus Trippelsdorf]

On 2016.11.29 at 15:21 +0100, Markus Trippelsdorf wrote:
> On 2016.11.29 at 15:14 +0100, Jakub Jelinek wrote:
> > On Tue, Nov 29, 2016 at 03:08:15PM +0100, Markus Trippelsdorf wrote:
> > > Building gcc with -fsanitize=undefined shows:
> > >  rtlanal.c:5210:38: runtime error: shift exponent 4294967295 is too
> > >  large for 64-bit type 'long unsigned int'
> > >
> > > 5210   return nonzero & (HOST_WIDE_INT_1U << (bitwidth - 1))
> > > 5211  ? 1 : bitwidth - floor_log2 (nonzero) - 1;
> > >
> > > Here (bitwidth - 1) wraps around because bitwidth is zero and unsigned.
> >
> > Which modes have precision of 0?  I'd expect just VOIDmode and BLKmode, any
> > others?  And for those I'd say it is a bug to call num_sign_bit_copies*.
> 
> Yes, only VOIDmode and BLKmode:
> 
>  233 const unsigned short mode_precision[NUM_MACHINE_MODES] =
>  234 {
>  235   0,   /* VOID */
>  236   0,   /* BLK */

markus@x4 libsupc++ % cat cp-demangle.i
d_demangle_callback_mangled() {
  if (strncmp(d_demangle_callback_mangled, "", 1))
d_type();
}

markus@x4 libsupc++ % UBSAN_OPTIONS=print_stacktrace=1:halt_on_error=1 
/var/tmp/gcc_build_dir_/./gcc/cc1 -w -fpreprocessed cp-demangle.i -quiet 
-dumpbase cp-demangle.i -mtune=generic -march=x86-64 -auxbase cp-demangle -O2 
-version -o /dev/null
GNU C11 (GCC) version 7.0.0 20161129 (experimental) (x86_64-pc-linux-gnu)
compiled by GNU C version 7.0.0 20161129 (experimental), GMP version 
6.1.1, MPFR version 3.1.5, MPC version 1.0.3, isl version none
GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096
GNU C11 (GCC) version 7.0.0 20161129 (experimental) (x86_64-pc-linux-gnu)
compiled by GNU C version 7.0.0 20161129 (experimental), GMP version 
6.1.1, MPFR version 3.1.5, MPC version 1.0.3, isl version none
GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096
Compiler executable checksum: 7cca725773f8a0693a2905f8af7b733c
../../gcc/gcc/rtlanal.c:5210:38: runtime error: shift exponent 4294967295 is 
too large for 64-bit type 'long unsigned int'
#0 0x1b40fe1 in num_sign_bit_copies1 ../../gcc/gcc/rtlanal.c:5210
#1 0x35ef5f1 in if_then_else_cond ../../gcc/gcc/combine.c:9180
#2 0x35ef199 in if_then_else_cond ../../gcc/gcc/combine.c:9034
#3 0x35ef199 in if_then_else_cond ../../gcc/gcc/combine.c:9034
#4 0x3625f98 in combine_simplify_rtx ../../gcc/gcc/combine.c:5604
#5 0x3632525 in subst ../../gcc/gcc/combine.c:5487
#6 0x36327d6 in subst ../../gcc/gcc/combine.c:5425
#7 0x3632bd7 in subst ../../gcc/gcc/combine.c:5354
#8 0x3641a74 in try_combine ../../gcc/gcc/combine.c:3347
#9 0x365727b in combine_instructions ../../gcc/gcc/combine.c:1421
#10 0x365727b in rest_of_handle_combine ../../gcc/gcc/combine.c:14581
#11 0x365727b in execute ../../gcc/gcc/combine.c:14626
#12 0x195ad18 in execute_one_pass(opt_pass*) ../../gcc/gcc/passes.c:2370
#13 0x195cbab in execute_pass_list_1 ../../gcc/gcc/passes.c:2459
#14 0x195cbd4 in execute_pass_list_1 ../../gcc/gcc/passes.c:2460
#15 0x195cc64 in execute_pass_list(function*, opt_pass*) 
../../gcc/gcc/passes.c:2470
#16 0xc75deb in cgraph_node::expand() ../../gcc/gcc/cgraphunit.c:2001
#17 0xc7b2fa in expand_all_functions ../../gcc/gcc/cgraphunit.c:2137
#18 0xc7b2fa in symbol_table::compile() ../../gcc/gcc/cgraphunit.c:2494
#19 0xc854b7 in symbol_table::compile() ../../gcc/gcc/cgraphunit.c:2587
#20 0xc854b7 in symbol_table::finalize_compilation_unit() 
../../gcc/gcc/cgraphunit.c:2584
#21 0x1d3ea10 in compile_file ../../gcc/gcc/toplev.c:488
#22 0x629a14 in do_compile ../../gcc/gcc/toplev.c:1983
#23 0x629a14 in toplev::main(int, char**) ../../gcc/gcc/toplev.c:2117
#24 0x62c046 in main ../../gcc/gcc/main.c:39
#25 0x7f4b6600f310 in __libc_start_main ../csu/libc-start.c:286
#26 0x62c469 in _start (/var/tmp/gcc_build_dir_/gcc/cc1+0x62c469)

-- 
Markus

Re: [PATCH] Fix PR78588 - rtlanal.c:5210:38: runtime error: shift exponent 4294967295 is too large for 64-bit type

[Markus Trippelsdorf]

On 2016.11.29 at 15:14 +0100, Jakub Jelinek wrote:
> On Tue, Nov 29, 2016 at 03:08:15PM +0100, Markus Trippelsdorf wrote:
> > Building gcc with -fsanitize=undefined shows:
> >  rtlanal.c:5210:38: runtime error: shift exponent 4294967295 is too
> >  large for 64-bit type 'long unsigned int'
> >
> > 5210   return nonzero & (HOST_WIDE_INT_1U << (bitwidth - 1))
> > 5211  ? 1 : bitwidth - floor_log2 (nonzero) - 1;
> >
> > Here (bitwidth - 1) wraps around because bitwidth is zero and unsigned.
>
> Which modes have precision of 0?  I'd expect just VOIDmode and BLKmode, any
> others?  And for those I'd say it is a bug to call num_sign_bit_copies*.

Yes, only VOIDmode and BLKmode:

 233 const unsigned short mode_precision[NUM_MACHINE_MODES] =
 234 {
 235   0,   /* VOID */
 236   0,   /* BLK */


--
Markus

[PATCH] Fix PR78588 - rtlanal.c:5210:38: runtime error: shift exponent 4294967295 is too large for 64-bit type

[Markus Trippelsdorf]

Building gcc with -fsanitize=undefined shows:
 rtlanal.c:5210:38: runtime error: shift exponent 4294967295 is too
 large for 64-bit type 'long unsigned int'

5210   return nonzero & (HOST_WIDE_INT_1U << (bitwidth - 1))
5211  ? 1 : bitwidth - floor_log2 (nonzero) - 1;

Here (bitwidth - 1) wraps around because bitwidth is zero and unsigned. 

Fix by returning earlier if bitwidth is zero.

Tested on ppc64le.
OK for trunk?

Thanks.

  * rtlanal.c (num_sign_bit_copies1): Check for zero bitwidth.

diff --git a/gcc/rtlanal.c b/gcc/rtlanal.c
index 4e4eb2ef3458..918088a0db8e 100644
--- a/gcc/rtlanal.c
+++ b/gcc/rtlanal.c
@@ -5203,7 +5203,7 @@ num_sign_bit_copies1 (const_rtx x, machine_mode mode, 
const_rtx known_x,
  safely compute the mask for this mode, always return BITWIDTH.  */

   bitwidth = GET_MODE_PRECISION (mode);
-  if (bitwidth > HOST_BITS_PER_WIDE_INT)
+  if (bitwidth == 0 || bitwidth > HOST_BITS_PER_WIDE_INT)
 return 1;

   nonzero = nonzero_bits (x, mode);

--
Markus

Re: [PATCH, rs6000] Fix PR78556 - left shift of negative values

[Markus Trippelsdorf]

On 2016.11.28 at 12:02 -0600, Segher Boessenkool wrote:
> Hi Markus,
> 
> On Mon, Nov 28, 2016 at 02:58:19PM +0100, Markus Trippelsdorf wrote:
> > Running bootstrap-ubsan on ppc64le shows many instances of e.g.:
> >  config/rs6000/rs6000.c:6217:36: runtime error: left shift of negative 
> > value -12301
> > 
> > The attached patch fixes the issue and was tested on ppc64le.
> 
> Thanks for the patch.
> 
> 
>   for (i = 2; i <= copies; i *= 2)
> {
>   HOST_WIDE_INT small_val;
>   bitsize /= 2;
>   small_val = splat_val >> bitsize;
> 
> >bitsize /= 2;
> >small_val = splat_val >> bitsize;
> >mask >>= bitsize;
> > -  if (splat_val != ((small_val << bitsize) | (small_val & mask)))
> > +  if (splat_val != ((HOST_WIDE_INT)
> > +  ((unsigned HOST_WIDE_INT) small_val << bitsize)
> > +  | (small_val & mask)))
> > return false;
> >splat_val = small_val;
> >  }
> 
> Can't you just make small_val an unsigned WIDE_INT, instead?

No, it doesn't work. First of all you'll get -Wsign-compare warnings.
And gcc.target/powerpc/altivec-11.c also starts failing.

An alternative might be:

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 6c28e6a..295bcb0 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -6214,7 +6214,7 @@ vspltis_constant (rtx op, unsigned step, unsigned copies)
   bitsize /= 2;
   small_val = splat_val >> bitsize;
   mask >>= bitsize;
-  if (splat_val != ((small_val << bitsize) | (small_val & mask)))
+  if (splat_val != (small_val * (1 << bitsize) | (small_val & mask)))
return false;
   splat_val = small_val;
 }

-- 
Markus

[PATCH, rs6000] Fix PR78556 - left shift of negative values

[Markus Trippelsdorf]

Running bootstrap-ubsan on ppc64le shows many instances of e.g.:
 config/rs6000/rs6000.c:6217:36: runtime error: left shift of negative value 
-12301

The attached patch fixes the issue and was tested on ppc64le.

OK for trunk?

Thanks.

PR target/78556
* config/rs6000/rs6000.h (vspltis_constant): Add casts to avoid
left shifting of negative values.

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 6c28e6a..dfb5dc8 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -6214,7 +6214,9 @@ vspltis_constant (rtx op, unsigned step, unsigned copies)
   bitsize /= 2;
   small_val = splat_val >> bitsize;
   mask >>= bitsize;
-  if (splat_val != ((small_val << bitsize) | (small_val & mask)))
+  if (splat_val != ((HOST_WIDE_INT)
+  ((unsigned HOST_WIDE_INT) small_val << bitsize)
+  | (small_val & mask)))
return false;
   splat_val = small_val;
 }

--
Markus

Re: [PATCH] eliminate calls to snprintf(0, 0, ...) with known return value (pr78476)

[Markus Trippelsdorf]

On 2016.11.22 at 20:02 -0700, Martin Sebor wrote:
> 
> gcc/ChangeLog:
> 
>   PR tree-optimization/78476
>   * gimple-ssa-sprintf.c (struct pass_sprintf_length::call_info):
>   Add a member.
>   (handle_gimple_call): Adjust signature.
>   (try_substitute_return_value): Remove calls to bounded functions
>   with zero buffer size whose result is known.
>   (pass_sprintf_length::execute): Adjust call to handle_gimple_call.

You left conflict markers in gcc/ChangeLog in your r242854 commit.

-- 
Markus

Re: [PATCH] Fix PR78413

[Markus Trippelsdorf]

On 2016.11.18 at 10:27 -0600, Bill Schmidt wrote:
> ===
> --- gcc/testsuite/gcc.dg/tree-ssa/pr78413.c   (revision 0)
> +++ gcc/testsuite/gcc.dg/tree-ssa/pr78413.c   (working copy)
> @@ -0,0 +1,35 @@
> +/* PR78413.  These previously failed in tree if-conversion due to a loop
> +   latch with multiple predecessors that the code did not anticipate.  */
> +/* { dg-do compile } */
> +/* { dg-options "-O3 -ffast-math" } */

Please add -fno-strict-aliasing, otherwise fn1() wouldn't ICE.

> +extern long long int llrint(double x);
> +int a;
> +double b;
> +__attribute__((cold)) void decode_init() {
> +  int c, d = 0;
> +  for (; d < 12; d++) {
> +if (d)
> +  b = 0;
> +c = 0;
> +for (; c < 6; c++)
> +  a = b ? llrint(b) : 0;
> +  }
> +}
> +
> +struct S {
> +  _Bool bo;
> +};
> +int a, bb, c, d;
> +void fn1() {
> +  do
> +do
> +  do {
> + struct S *e = (struct S *)1;
> + do
> +   bb = a / (e->bo ? 2 : 1);
> + while (bb);
> +  } while (0);
> +while (d);
> +  while (c);
> +}

-- 
Markus

[PATCH] Fix PR78294 - thread sanitizer broken when using ld.gold

[Markus Trippelsdorf]

When one uses ld.gold to build gcc, the thread sanitizer doesn't work,
because gold is more conservative when applying TLS relaxations than
ld.bfd. In this case a missing initial-exec attribute on a declaration
causes gcc to assume the general dynamic model. With ld.bfd this gets
relaxed to initial exec when linking the shared library, so the missing
attribute doesn't matter. But ld.gold doesn't perform this optimization
and this leads to crashes on tsan instrumented binaries.

See: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78294
and: https://sourceware.org/bugzilla/show_bug.cgi?id=20805

The fix is easy, just add the missing attribute.

(I don't think upstream needs this fix. They don't use shared tsan lib
and clang doesn't need the fix anyway.)

Tested on X86_64 using ld.gold.

Ok for trunk and branches?

Thanks.

  PR sanitizer/78294
  * tsan/tsan_rtl.cc: Add missing attribute.

diff --git a/libsanitizer/tsan/tsan_rtl.cc b/libsanitizer/tsan/tsan_rtl.cc
index 07fa165e939c..5be28ce5502e 100644
--- a/libsanitizer/tsan/tsan_rtl.cc
+++ b/libsanitizer/tsan/tsan_rtl.cc
@@ -43,6 +43,7 @@ extern "C" void __tsan_resume() {
 namespace __tsan {

 #if !SANITIZER_GO && !SANITIZER_MAC
+  __attribute__((tls_model("initial-exec")))
 THREADLOCAL char cur_thread_placeholder[sizeof(ThreadState)] ALIGNED(64);
 #endif
 static char ctx_placeholder[sizeof(Context)] ALIGNED(64);
--
Markus

Re: Change default level for -Wimplicit-fallthrough

[Markus Trippelsdorf]

On 2016.11.03 at 14:57 +0100, Jakub Jelinek wrote:
> On Thu, Nov 03, 2016 at 02:55:03PM +0100, Markus Trippelsdorf wrote:
> > On 2016.11.03 at 14:47 +0100, Jakub Jelinek wrote:
> > > On Thu, Nov 03, 2016 at 02:35:57PM +0100, Markus Trippelsdorf wrote:
> > > > I don't have gathered detailed statistics. But for example a simple
> > > > /* drop through */ in a package header file will of course cause many
> > > > bogus warnings during the build on level 2.
> > > > For the Linux kernel false positives decrease ~20% when switching from
> > > > level 3 to 1.
> > > 
> > > One would have to count only warnings with unique locus (i.e. sort -u them
> > > after grepping them from logs).
> > > But even with 20%, if one spends the energy to analyze the 80%, where
> > > one actually has to analyze the code, just mechanically changing a couple 
> > > of
> > > common comment kinds into more standardized one isn't going to be
> > > significant.
> > 
> > I should have written: For the Linux kernel the number of warnings
> > dropped by 20% (going from level 3 to 1) and all of them turned out to
> > be false positives. And yes, I have used "sort -u".
> > I'm not sure if I would call 20% insignificant.
> 
> But we are talking about 2 vs. 1 now, so that is likely smaller than 20%.
> Plus what those comments in that 2 vs. 1 set are where the warnings differ,
> if they are related to fall through or not.

It is still a 12% reduction (2 vs. 1). I've posted a list of them in the
older thread.

-- 
Markus