Re: [ARM] Fix PR85434: spill of stack protector's guard address

[Segher Boessenkool]

Hi!

On Sat, Apr 28, 2018 at 12:32:26AM +0100, Thomas Preudhomme wrote:
> On Arm (Aarch32 and Aarch64) the stack protector's guard is accessed by
> loading its address first before loading its value from it as part of
> the stack_protect_set or stack_protect_check insn pattern. This creates
> the risk of spilling between the two.

> diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
> index deab929..c7ced8f 100644
> --- a/gcc/cfgexpand.c
> +++ b/gcc/cfgexpand.c
> @@ -6156,6 +6156,10 @@ stack_protect_prologue (void)
>tree guard_decl = targetm.stack_protect_guard ();
>rtx x, y;
>  
> +  /* Prevent scheduling of instruction(s) between computation of the guard's
> + address and setting of the canari to avoid any spill of the guard's
> + address if computed outside the setting of the canari.  */
> +  emit_insn (gen_blockage ());
>x = expand_normal (crtl->stack_protect_guard);
>if (guard_decl)
>  y = expand_normal (guard_decl);

[ etc. ]

Why pessimise code for all targets (quite a lot), when it does not even
fix the problem on Arm completely (or not obviously, anyway)?

Instead, implement stack_protect_* and hide the memory accesses to the
stored canary value (and make sure its address is not left in a register
either!)

I doubt this can be done completely target-independent, it will always
be best effort that way, aka it won't really work.


Segher

Re: [PATCH] Fix loop-header copying do-while loop detection (PR85116)

[David Edelsohn]

Hi, Richi

I had been using two source trees to speed the bisection and didn't realize
that one defaulted to DWARF debugging and the other defaulted to XCOFF
debugging, which confused the bisection result.  The -f[no-]checking patch
is the culprit.

Thanks, David

On Sat, Apr 28, 2018 at 4:08 AM Richard Biener  wrote:

> On Sat, 28 Apr 2018, Richard Biener wrote:

> > On Fri, 27 Apr 2018, Richard Biener wrote:
> >
> > > On Fri, 27 Apr 2018, David Edelsohn wrote:
> > >
> > > > Hi, Richi
> > > >
> > > > This patches causes a boostrap failure on AIX.  Everything
miscompares.
> > > > The code itself is the same, but the DWARF debug information
contains many
> > > > differences.
> > >
> > > Does AIX use bootstrap-debug by default?  I don't see how the patch
> > > can cause this kind of issue directly but of course it will change
> > > CH decisions which may expose latent bugs somewhere.
> > >
> > > Can you provide more details please, like actual differences?
> > > I would have expected the dwarf2out.c change to be a more likely
> > > candidate for such symtoms but I trust that you did properly
> > > bisect to my patch?
> >
> > OK, so the x86-64 -O3 bootstrap failure is not caused by this patch,
> > reverting it doesn't fix the issue.
> >
> > The difference w/ the patch reverted is in debug info, all (indirect)
> > strings reside at different offsets.  If I strip the objects they
> > compare identical.
> >
> > I'm trying reversal of that dwarf2out patch now.

> That didn't help.  Reverting the bootstrap -f[no-]checking patch did.

> Richard.

Re: [PATCH] Warn for ignored ASM labels on typdef declarations PR 85444 (v.2)

[Will Hawkins]

Thanks to Mr. Meyers for his comments. Here is an updated version of
the patch. Test cases are included this time.

Tested with 'make bootstrap' on x86_64-pc-linux-gnu. Results from make
-k check available upon request.

I hope that this one is better! Thanks again for everyone's patience!

Will


2018-04-18  Will Hawkins  

  PR c,c++/85444
  * gcc/c/c-decl.c: Warn about ignored asm label for
  typedef declaration
  * gcc/cp/decl.c: Warn about ignored asm label for
  typedef declaration
  * gcc/testsuite/gcc.dg/asm-pr85444.c: c testcase.
  * gcc/testsuite/g++.dg/asm-pr85444.C: c++ testcase.

diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index f0198ec..f18bb7d 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -5166,7 +5166,11 @@ finish_decl (tree decl, location_t init_loc, tree init,
   if (!DECL_FILE_SCOPE_P (decl)
   && variably_modified_type_p (TREE_TYPE (decl), NULL_TREE))
 add_stmt (build_stmt (DECL_SOURCE_LOCATION (decl), DECL_EXPR, decl));
-
+  if (asmspec_tree != NULL_TREE)
+{
+  warning (OPT_Wignored_qualifiers, "asm-specifier is ignored in "
+   "typedef declaration");
+}
   rest_of_decl_compilation (decl, DECL_FILE_SCOPE_P (decl), 0);
 }

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 44a152b..32c2dfa 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -7069,6 +7069,11 @@ cp_finish_decl (tree decl, tree init, bool
init_const_expr_p,
   /* Take care of TYPE_DECLs up front.  */
   if (TREE_CODE (decl) == TYPE_DECL)
 {
+  if (asmspec_tree != NULL_TREE)
+{
+  warning (OPT_Wignored_qualifiers, "asm-specifier is ignored for "
+   "typedef declarations");
+}
   if (type != error_mark_node
   && MAYBE_CLASS_TYPE_P (type) && DECL_NAME (decl))
 {
diff --git a/gcc/testsuite/g++.dg/asm-pr85444.C
b/gcc/testsuite/g++.dg/asm-pr85444.C
new file mode 100644
index 000..9e4b6c6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/asm-pr85444.C
@@ -0,0 +1,13 @@
+/* Fix Bugzilla 8544 -- asm specifier on typedef silently ignored.
+   { dg-do compile }
+   { dg-options "-Wignored-qualifiers" } */
+
+typedef struct
+{
+  int a;
+} x asm ("X"); /* { dg-warning "asm-specifier is ignored" } */
+
+int main()
+{
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/asm-pr85444.c
b/gcc/testsuite/gcc.dg/asm-pr85444.c
new file mode 100644
index 000..9e4b6c6
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/asm-pr85444.c
@@ -0,0 +1,13 @@
+/* Fix Bugzilla 8544 -- asm specifier on typedef silently ignored.
+   { dg-do compile }
+   { dg-options "-Wignored-qualifiers" } */
+
+typedef struct
+{
+  int a;
+} x asm ("X"); /* { dg-warning "asm-specifier is ignored" } */
+
+int main()
+{
+  return 0;
+}

Re: [PATCH] DWARF: Add .debug_addr table header for dwarf_version >= 5.

[Jakub Jelinek]

On Sat, Apr 28, 2018 at 08:54:00PM +0200, Eric Botcazou wrote:
> > I don't know what LLVMish does, or whether it implements the GNU
> > DebugFission variant of split DWARF, the DWARF5 variant or something
> > else. For GCC -gsplit-dwarf generates GNU DebugFission output
> > (including .debug_addr) for older DWARF versions.
> > It is described here: https://gcc.gnu.org/wiki/DebugFission
> 
> Yes, that's what I was referring to.  DebugFission is the name of the Wiki 
> page so let's use "GNU split DWARF" or -gsplit-dwarf in the comment please.

-gsplit-dwarf is not appropriate, because that includes the DWARF5 variant
thereof.  GNU split DWARF extension is ok.

Jakub

Re: [PATCH] DWARF: Add .debug_addr table header for dwarf_version >= 5.

[Mark Wielaard]

On Sat, Apr 28, 2018 at 08:54:00PM +0200, Eric Botcazou wrote:
> > For GCC -gsplit-dwarf generates GNU DebugFission output
> > (including .debug_addr) for older DWARF versions.
> > It is described here: https://gcc.gnu.org/wiki/DebugFission
> 
> Yes, that's what I was referring to.  DebugFission is the name of the Wiki 
> page so let's use "GNU split DWARF" or -gsplit-dwarf in the comment please.

I expanded the comment into a little paragraph explaining the terminology
used and included the URL for background information. Hope that makes
things more clear.

Thanks,

Mark

Re: [PATCH] DWARF: Add .debug_addr table header for dwarf_version >= 5.

[Jakub Jelinek]

On Sat, Apr 28, 2018 at 09:01:45PM +0200, Mark Wielaard wrote:
> gcc/ChangeLog
> 
>   * dwarf2out.c (dwarf2out_finish): Add .debug_addr table header for
>   dwarf_version >= 5.
>   (dwarf_AT): Handle DW_AT_addr_base.
>   (add_top_level_skeleton_die_attrs): Use dwarf_AT for DW_AT_addr_base.

Ok for trunk.

Jakub

Re: [PATCH] DWARF: Add .debug_addr table header for dwarf_version >= 5.

[Mark Wielaard]

On Sat, Apr 28, 2018 at 06:36:02PM +0200, Jakub Jelinek wrote:
> On Sat, Apr 28, 2018 at 05:23:12PM +0200, Mark Wielaard wrote:
> >switch_to_section (debug_addr_section);
> > +  /* GNU DebugFission didn't have a header for .debug_addr.  */
> > +  if (dwarf_version >= 5)
> > +   {
> > + unsigned long addrs_length =
> > +   addr_index_table->elements () * DWARF2_ADDR_SIZE + 4;
> 
> No = at the end of line, it should be at the start of the next line, i.e.
> unsigned long addrs_length
>   = addr_index_table->elements () * DWARF2_ADDR_SIZE + 4;

Oops. Fixed.

> > + if (DWARF_INITIAL_LENGTH_SIZE - DWARF_OFFSET_SIZE == 4)
> > +   dw2_asm_output_data (4, 0x,
> > +"Escape value for 64-bit DWARF extension");
> > + dw2_asm_output_data (DWARF_OFFSET_SIZE, addrs_length,
> > +  "Length of Address Unit");
> > + dw2_asm_output_data (2, 5, "DWARF addr version");
> > + dw2_asm_output_data (1, DWARF2_ADDR_SIZE, "Size of Address");
> > + dw2_asm_output_data (1, 0, "Size of Segment Descriptor");
> > +   }
> >ASM_OUTPUT_LABEL (asm_out_file, debug_addr_section_label);
> >output_addr_table ();
> >  }
> 
> Shouldn't we together with the addition of the .debug_addr section header
> also switch to using DW_AT_addr_base rather than DW_AT_GNU_addr_base, i.e.
> add DW_AT_addr_base into dwarf_AT and use dwarf_AT (DW_AT_addr_base)
> instead of DW_AT_GNU_addr_base?

Yes of course. I was actually checking the DWARF version of the CU, but
having the correct attribute is obviously better.

Updated patch:

GNU DebugFission didn't add table headers for the .debug_addr
tables, but DWARF5 does. The table header makes it possible
for a DWARF consumer to parse the address tables without having
to index all .debug_info CUs first.

We can keep using the .debug_addr section label as is, because the
DW_AT_[GNU_]addr_base attribute points at the actual address index,
which starts right after the table header. So the label is generated
at the correct location whether the header is added first or not.

Add DW_AT_addr_base instead of DW_AT_GNU_addr_base to the skeleton
CU DIE for DWARF5.

gcc/ChangeLog

* dwarf2out.c (dwarf2out_finish): Add .debug_addr table header for
dwarf_version >= 5.
(dwarf_AT): Handle DW_AT_addr_base.
(add_top_level_skeleton_die_attrs): Use dwarf_AT for DW_AT_addr_base.

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index d3d925d..c172387 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -1724,6 +1724,11 @@ dwarf_AT (enum dwarf_attribute at)
return DW_AT_GNU_dwo_name;
   break;
 
+case DW_AT_addr_base:
+  if (dwarf_version < 5)
+   return DW_AT_GNU_addr_base;
+  break;
+
 default:
   break;
 }
@@ -11106,7 +1,7 @@ add_top_level_skeleton_die_attrs (dw_die_ref die)
   if (comp_dir != NULL)
 add_skeleton_AT_string (die, DW_AT_comp_dir, comp_dir);
   add_AT_pubnames (die);
-  add_AT_lineptr (die, DW_AT_GNU_addr_base, debug_addr_section_label);
+  add_AT_lineptr (die, dwarf_AT (DW_AT_addr_base), debug_addr_section_label);
 }
 
 /* Output skeleton debug sections that point to the dwo file.  */
@@ -31293,6 +31298,21 @@ dwarf2out_finish (const char *)
}
 
   switch_to_section (debug_addr_section);
+  /* GNU DebugFission didn't have a header for .debug_addr.  */
+  if (dwarf_version >= 5)
+   {
+ unsigned long addrs_length
+   = addr_index_table->elements () * DWARF2_ADDR_SIZE + 4;
+
+ if (DWARF_INITIAL_LENGTH_SIZE - DWARF_OFFSET_SIZE == 4)
+   dw2_asm_output_data (4, 0x,
+"Escape value for 64-bit DWARF extension");
+ dw2_asm_output_data (DWARF_OFFSET_SIZE, addrs_length,
+  "Length of Address Unit");
+ dw2_asm_output_data (2, 5, "DWARF addr version");
+ dw2_asm_output_data (1, DWARF2_ADDR_SIZE, "Size of Address");
+ dw2_asm_output_data (1, 0, "Size of Segment Descriptor");
+   }
   ASM_OUTPUT_LABEL (asm_out_file, debug_addr_section_label);
   output_addr_table ();
 }

Improve partitioning decisions

[Jan Hubicka]

Hi,
with number of cores increasing our partitioning algorithm needs some love,
because it works quite bad.

This patch fixes few issues with ballanced partitioning algorithm:
 1) I have switched all size/boundary summaries to 64bit arthmetics as they
may overflow for very large units
 2) internal calls within partition was misaccounted
 3) I no longer account external references (as those will never be local
no matter how we partition) 
 4) the range where we search for best split is not +- 1/8 of partition.
 5) we no longer look for worst possible split, but for best one.
 6) I have added bit extra debug info.

I have checked this with Firefox build. Previously we did 31 instead of 32 
partitions
and the difference between largest and smallest was 170%.  Now we do 32 
partitions and get
the difference is "only" about 70%.  The resulting code quality improves a bit 
and
we get 260s for largest partition build time instead of 305s.

I will commit it into the mainline and backport to release branches after some 
soaking
(and 8.1 release), since without the patch partitioning decisions with larger # 
of partitions
are really pretty bad.

Situation is still not perfect - I do not get any speedup for increasing 
partitions to 64
becuase we still get too much of extra streaming. Something to track this 
stage1.

Honza

* lto-partition.c: Include sreal.h
(add_symbol_to_partition_1): Use size instead of self_size
for size estimate.
(account_reference_p): New.
(lto_balanced_map): Use 64bit arithmetics for size calculatoins; 
cleanup;
fix accounting errors in boundary size; add debug output; combine cost
as cost/size instead of cost/internal; reduce the partitioning error to
+- 1/8 of the parttion size.
Index: lto-partition.c
===
--- lto-partition.c (revision 259742)
+++ lto-partition.c (working copy)
@@ -35,6 +35,7 @@
 #include "ipa-prop.h"
 #include "ipa-fnsummary.h"
 #include "lto-partition.h"
+#include "sreal.h"
 
 vec ltrans_partitions;
 
@@ -152,8 +153,8 @@
   if (cgraph_node *cnode = dyn_cast  (node))
 {
   struct cgraph_edge *e;
-  if (!node->alias)
-part->insns += ipa_fn_summaries->get (cnode)->self_size;
+  if (!node->alias && c == SYMBOL_PARTITION)
+part->insns += ipa_fn_summaries->get (cnode)->size;
 
   /* Add all inline clones and callees that are duplicated.  */
   for (e = cnode->callees; e; e = e->next_callee)
@@ -276,8 +277,9 @@
delete partition->initializers_visited;
   partition->initializers_visited = NULL;
 
-  if (!node->alias && (cnode = dyn_cast  (node)))
-partition->insns -= ipa_fn_summaries->get (cnode)->self_size;
+  if (!node->alias && (cnode = dyn_cast  (node))
+  && node->get_partitioning_class () == SYMBOL_PARTITION)
+partition->insns -= ipa_fn_summaries->get (cnode)->size;
   lto_symtab_encoder_delete_node (partition->encoder, node);
   node->aux = (void *)((size_t)node->aux - 1);
 }
@@ -408,7 +410,25 @@
   add_symbol_to_partition (partition, node);
 }
 
+/* Return true if we should account reference from N1 to N2 in cost
+   of partition boundary.  */
 
+bool
+account_reference_p (symtab_node *n1, symtab_node *n2)
+{
+  if (cgraph_node *cnode = dyn_cast  (n1))
+n1 = cnode;
+  /* Do not account recursion - the code below will handle it incorrectly
+ otherwise.  Also do not account references to external symbols.
+ They will never become local.  */
+  if (n1 == n2 
+  || DECL_EXTERNAL (n2->decl)
+  || !n2->definition)
+return false;
+  return true;
+}
+
+
 /* Group cgraph nodes into equally-sized partitions.
 
The partitioning algorithm is simple: nodes are taken in predefined order.
@@ -457,14 +477,14 @@
   auto_vec varpool_order;
   int i;
   struct cgraph_node *node;
-  int original_total_size, total_size = 0, best_total_size = 0;
-  int partition_size;
+  int64_t original_total_size, total_size = 0;
+  int64_t partition_size;
   ltrans_partition partition;
   int last_visited_node = 0;
   varpool_node *vnode;
-  int cost = 0, internal = 0;
-  int best_n_nodes = 0, best_i = 0, best_cost =
-INT_MAX, best_internal = 0;
+  int64_t cost = 0, internal = 0;
+  int best_n_nodes = 0, best_i = 0;
+  int64_t best_cost = -1, best_internal = 0, best_size = 0;
   int npartitions;
   int current_order = -1;
   int noreorder_pos = 0;
@@ -513,7 +533,8 @@
 
   /* Compute partition size and create the first partition.  */
   if (PARAM_VALUE (MIN_PARTITION_SIZE) > max_partition_size)
-fatal_error (input_location, "min partition size cannot be greater than 
max partition size");
+fatal_error (input_location, "min partition size cannot be greater "
+"than max partition size");
 
   partition_size = total_size / n_lto_partitions;
   if (partition_size < PARAM_VALUE (MIN_PARTITION_SIZE))
@@ -521,7 +542,7 @@
 

Re: [PATCH] DWARF: Add .debug_addr table header for dwarf_version >= 5.

[Eric Botcazou]

> I don't know what LLVMish does, or whether it implements the GNU
> DebugFission variant of split DWARF, the DWARF5 variant or something
> else. For GCC -gsplit-dwarf generates GNU DebugFission output
> (including .debug_addr) for older DWARF versions.
> It is described here: https://gcc.gnu.org/wiki/DebugFission

Yes, that's what I was referring to.  DebugFission is the name of the Wiki 
page so let's use "GNU split DWARF" or -gsplit-dwarf in the comment please.

-- 
Eric Botcazou

Re: [C++ PATCH] Fix value initialized decltype(nullptr) in constexpr (PR c++/85553)

[Jason Merrill]

On Fri, Apr 27, 2018 at 7:26 PM, Paolo Carlini  wrote:
> Hi again,
>
> I'm now pretty sure that we have a latent issue in ocp_convert. The bug
> fixed by Jakub shows that we used to not have issues with integer_zero_node.
> That's easy to explain: at the beginning of ocp_convert there is code which
> handles first some special / simple cases when
> same_type_ignoring_top_level_qualifiers_p is true. That code isn't of course
> used for integer_zero_node as source expression, which therefore is handled
> by:
>
>   if (NULLPTR_TYPE_P (type) && e && null_ptr_cst_p (e))
> {
>   if (complain & tf_warning)
> maybe_warn_zero_as_null_pointer_constant (e, loc);
>   return nullptr_node;
> }

Maybe we should move this code up, then.

Jason

Re: [PATCH] DWARF: Add .debug_addr table header for dwarf_version >= 5.

[Jakub Jelinek]

On Sat, Apr 28, 2018 at 05:23:12PM +0200, Mark Wielaard wrote:
> GNU DebugFission didn't add table headers for the .debug_addr
> tables, but DWARF5 does. The table header make it possible
> for a DWARF consumer to parse the address tables without having
> to index all .debug_info CUs first. We can keep using the
> .debug_addr section label as is, because the DW_AT_addr_base
> attribute points at the actual address index, which starts
> right after the table header. So the label is generated at
> the correct location whether the header is added first or not.
> 
> gcc/ChangeLog
> 
>   * dwarf2out.c (dwarf2out_finish): Add .debug_addr table header for
>   dwarf_version >= 5.
> 
> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
> index d3d925d..51d0ca4 100644
> --- a/gcc/dwarf2out.c
> +++ b/gcc/dwarf2out.c
> @@ -31293,6 +31293,21 @@ dwarf2out_finish (const char *)
>   }
>  
>switch_to_section (debug_addr_section);
> +  /* GNU DebugFission didn't have a header for .debug_addr.  */
> +  if (dwarf_version >= 5)
> + {
> +   unsigned long addrs_length =
> + addr_index_table->elements () * DWARF2_ADDR_SIZE + 4;

No = at the end of line, it should be at the start of the next line, i.e.
  unsigned long addrs_length
= addr_index_table->elements () * DWARF2_ADDR_SIZE + 4;

> +
> +   if (DWARF_INITIAL_LENGTH_SIZE - DWARF_OFFSET_SIZE == 4)
> + dw2_asm_output_data (4, 0x,
> +  "Escape value for 64-bit DWARF extension");
> +   dw2_asm_output_data (DWARF_OFFSET_SIZE, addrs_length,
> +"Length of Address Unit");
> +   dw2_asm_output_data (2, 5, "DWARF addr version");
> +   dw2_asm_output_data (1, DWARF2_ADDR_SIZE, "Size of Address");
> +   dw2_asm_output_data (1, 0, "Size of Segment Descriptor");
> + }
>ASM_OUTPUT_LABEL (asm_out_file, debug_addr_section_label);
>output_addr_table ();
>  }

Shouldn't we together with the addition of the .debug_addr section header
also switch to using DW_AT_addr_base rather than DW_AT_GNU_addr_base, i.e.
add DW_AT_addr_base into dwarf_AT and use dwarf_AT (DW_AT_addr_base)
instead of DW_AT_GNU_addr_base?

Jakub

Re: [PATCH] DWARF: Add .debug_addr table header for dwarf_version >= 5.

[Mark Wielaard]

Hi Eric,

On Sat, 2018-04-28 at 18:00 +0200, Eric Botcazou wrote:
> > index d3d925d..51d0ca4 100644
> > --- a/gcc/dwarf2out.c
> > +++ b/gcc/dwarf2out.c
> > @@ -31293,6 +31293,21 @@ dwarf2out_finish (const char *)
> >     }
> > 
> >    switch_to_section (debug_addr_section);
> > +  /* GNU DebugFission didn't have a header for .debug_addr.  */
> > +  if (dwarf_version >= 5)
> 
> That sounds LLVMish...  Did you mean -gsplit-dwarf instead?

I don't know what LLVMish does, or whether it implements the GNU
DebugFission variant of split DWARF, the DWARF5 variant or something
else. For GCC -gsplit-dwarf generates GNU DebugFission output
(including .debug_addr) for older DWARF versions.
It is described here: https://gcc.gnu.org/wiki/DebugFission

This got standardized for DWARF5. But there are some differences. One
particular change is that GNU DebugFission doesn't have address table
headers in .debug_addr sections. But DWARF5 does. This patch fixes
that.

Or did you mean that you expected the code to be guarded by 
if (dwarf_split_debug_info)? It is, just a bit higher up in the code.

Cheers,

Mark

Re: [PATCH] DWARF: Add .debug_addr table header for dwarf_version >= 5.

[Eric Botcazou]

> index d3d925d..51d0ca4 100644
> --- a/gcc/dwarf2out.c
> +++ b/gcc/dwarf2out.c
> @@ -31293,6 +31293,21 @@ dwarf2out_finish (const char *)
>   }
> 
>switch_to_section (debug_addr_section);
> +  /* GNU DebugFission didn't have a header for .debug_addr.  */
> +  if (dwarf_version >= 5)

That sounds LLVMish...  Did you mean -gsplit-dwarf instead?

-- 
Eric Botcazou

Re: [patch] allow '-' for stdout dump

[Sandra Loosemore]

On 04/27/2018 07:12 AM, Nathan Sidwell wrote:

On 04/26/2018 01:31 PM, Sandra Loosemore wrote:

Hmmm, I'm not crazy about putting this material in the middle of a 
discussion about how the compiler comes up with its default dump file 
names.  How about splitting the existing paragraph into 3:


Yeah, that documentation is rather a mess.  I tried rationalizing the 
developer options into different subsections, but that quickly turned 
into a rat hole. Here I've just pulled out the filename creation to an 
earlier paragraph.


WDYT? 


Index: invoke.texi
===
--- invoke.texi (revision 259683)
+++ invoke.texi (working copy)
@@ -13357,6 +13357,25 @@ configuration, such as where it searches
 rarely need to use any of these options for ordinary compilation and
 linking tasks.
 
+Many developer options control dump output, and may take an optional

+@option{=@var{filename}} suffix.  If that is omitted, a default
+filename is determined by appending a pass number & phase letter,
+followed by the pass name to a @var{dumpname}.  The files are created
+in the directory of the output file.  The @var{dumpname} is the name
+of the output file, if explicitly specified and not an executable,
+otherwise it is the source file name.  The pass number is determined
+when a particular pass is registered with the compiler's pass manager.
+Usually passes executed in the order of registration, so this number
+corresponds to the pass execution order.  However, passes registered
+by plugins, passes specific to compilation targets, or passes that are
+otherwise registered after all the other passes are numbered higher
+than a pass named "final", even if they are executed earlier.  The
+phase letter is `i', `l', `r' or `t'.
+
+The default can be overridden by appending a @option{=@var{filename}}
+suffix to the option.  You can specify @code{stdout} or @code{-} to
+refer to standard output, and @code{stderr} for standard error.
+
 @table @gcctabopt
 
 @item -d@var{letters}


Hmmm, I'd like to wordsmith this a bit and clean up the markup, etc. 
How about this?


Many developer options that cause GCC to dump output to a file take an 
optional @samp{=@var{filename}} suffix.
You can specify @samp{stdout} or @samp{-} to dump to standard output, 
and @samp{stderr} for standard error.


If @samp{=@var{filename}} is omitted, a default dump file name is 
constructed by concatenating the base dump file name, a pass number, 
phase letter, and pass name.  The base dump file name is the name of 
output file produced by the compiler if explicitly specified and not an 
executable; otherwise it is the source file name.
The pass number is determined by the order passes are registered with 
the compiler's pass manager.

This is generally the same as the order of execution, but passes
registered by plugins, target-specific passes, or passes that are 
otherwise registered late are numbered higher than the pass named 
@samp{final}, even if they are executed earlier.
The phase letter is one of @samp{i} (inter-procedural analysis), 
@samp{l} (language-specific), @samp{r} (RTL), or @samp{t} (tree).

The files are created in the directory of the output file.

-Sandra

Re: [PATCH] Fix loop-header copying do-while loop detection (PR85116)

[Jeff Law]

On 04/28/2018 12:46 AM, Richard Biener wrote:
> On Fri, 27 Apr 2018, Richard Biener wrote:
> 
>> On Fri, 27 Apr 2018, David Edelsohn wrote:
>>
>>> Hi, Richi
>>>
>>> This patches causes a boostrap failure on AIX.  Everything miscompares.
>>> The code itself is the same, but the DWARF debug information contains many
>>> differences.
>>
>> Does AIX use bootstrap-debug by default?  I don't see how the patch
>> can cause this kind of issue directly but of course it will change
>> CH decisions which may expose latent bugs somewhere.
>>
>> Can you provide more details please, like actual differences?
>> I would have expected the dwarf2out.c change to be a more likely
>> candidate for such symtoms but I trust that you did properly
>> bisect to my patch?
> 
> OK, so the x86-64 -O3 bootstrap failure is not caused by this patch,
> reverting it doesn't fix the issue.
> 
> The difference w/ the patch reverted is in debug info, all (indirect)
> strings reside at different offsets.  If I strip the objects they
> compare identical.
> 
> I'm trying reversal of that dwarf2out patch now.
> 
FWIW, the PA port has been failing a standard -O2 bootstrap for a few days.

r259643 is good, r259672 is bad.

I haven't looked into it any deeper yet.

jeff

[PATCH] DWARF: Add .debug_addr table header for dwarf_version >= 5.

[Mark Wielaard]

GNU DebugFission didn't add table headers for the .debug_addr
tables, but DWARF5 does. The table header make it possible
for a DWARF consumer to parse the address tables without having
to index all .debug_info CUs first. We can keep using the
.debug_addr section label as is, because the DW_AT_addr_base
attribute points at the actual address index, which starts
right after the table header. So the label is generated at
the correct location whether the header is added first or not.

gcc/ChangeLog

* dwarf2out.c (dwarf2out_finish): Add .debug_addr table header for
dwarf_version >= 5.

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index d3d925d..51d0ca4 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -31293,6 +31293,21 @@ dwarf2out_finish (const char *)
}
 
   switch_to_section (debug_addr_section);
+  /* GNU DebugFission didn't have a header for .debug_addr.  */
+  if (dwarf_version >= 5)
+   {
+ unsigned long addrs_length =
+   addr_index_table->elements () * DWARF2_ADDR_SIZE + 4;
+
+ if (DWARF_INITIAL_LENGTH_SIZE - DWARF_OFFSET_SIZE == 4)
+   dw2_asm_output_data (4, 0x,
+"Escape value for 64-bit DWARF extension");
+ dw2_asm_output_data (DWARF_OFFSET_SIZE, addrs_length,
+  "Length of Address Unit");
+ dw2_asm_output_data (2, 5, "DWARF addr version");
+ dw2_asm_output_data (1, DWARF2_ADDR_SIZE, "Size of Address");
+ dw2_asm_output_data (1, 0, "Size of Segment Descriptor");
+   }
   ASM_OUTPUT_LABEL (asm_out_file, debug_addr_section_label);
   output_addr_table ();
 }

Re: [Ping, Fortran, Patch, PR81773, PR83606, coarray, v1] Fix coarray get to array with vector indexing

[Andre Vehreschild]

Hi all,

because pr81773 is a 6-/7-/8-regression I have backported it to gcc-6 as r259741
and gcc-7 as r259742.

Regards,
Andre

On Sat, 14 Apr 2018 16:46:53 +0200
Andre Vehreschild  wrote:

> Hi Paul,
> 
> thank you for the review. Committed as r259385.
> 
> Regards,
>   Andre
> 
> On Sat, 14 Apr 2018 11:53:44 +0100
> Paul Richard Thomas  wrote:
> 
> > Hi Andre,
> > 
> > This is OK for trunk.
> > 
> > Thanks for the patch
> > 
> > Paul
> > 
> > 
> > On 13 April 2018 at 08:34, Andre Vehreschild  wrote:  
> > > Ping
> > >
> > > On Sun, 8 Apr 2018 14:25:50 +0200
> > > Andre Vehreschild  wrote:
> > >
> > >> Hi all,
> > >>
> > >> attached patch fixes (to my knowledge) the two PRs 81773 and 83606 where
> > >> the result of a coarray get() operation is assigned to an array using a
> > >> vector as index.  It took me quite long to get it right, because I had to
> > >> use the scalarizer which I haven't use directly before. So reviewers with
> > >> expertise on using the scalarizer are especially welcome.
> > >>
> > >> Bootstrapped and regtested on x86_64-linux/f27.
> > >>
> > >> Ok for trunk? Backports?
> > >>
> > >> Regards,
> > >>   Andre
> > >
> > >
> > > --
> > > Andre Vehreschild * Email: vehre ad gmx dot de
> > 
> > 
> >   
> 
> 


-- 
Andre Vehreschild * Email: vehre ad gmx dot de 
Index: gcc/fortran/ChangeLog
===
--- gcc/fortran/ChangeLog	(Revision 259739)
+++ gcc/fortran/ChangeLog	(Arbeitskopie)
@@ -1,3 +1,16 @@
+2018-04-28  Andre Vehreschild  
+
+	PR fortran/81773
+	PR fortran/83606
+	Backport from trunk.
+	* dependency.c (gfc_dep_resolver): Coarray indexes are to be ignored
+	during dependency computation.  They define no data dependency.
+	* trans-array.c (conv_array_index_offset): The stride can not be set
+	here, prevent fail.
+	* trans-intrinsic.c (conv_caf_send): Add creation of temporary array
+	for caf_get's result and copying to the array with vectorial
+	indexing.
+
 2018-04-24  Steven G. Kargl  
 
 	PR fortran/85520
Index: gcc/fortran/dependency.c
===
--- gcc/fortran/dependency.c	(Revision 259739)
+++ gcc/fortran/dependency.c	(Arbeitskopie)
@@ -2239,8 +2239,9 @@
 	break;
 
 	  /* Exactly matching and forward overlapping ranges don't cause a
-	 dependency.  */
-	  if (fin_dep < GFC_DEP_BACKWARD)
+	 dependency, when they are not part of a coarray ref.  */
+	  if (fin_dep < GFC_DEP_BACKWARD
+	  && lref->u.ar.codimen == 0 && rref->u.ar.codimen == 0)
 	return 0;
 
 	  /* Keep checking.  We only have a dependency if
Index: gcc/fortran/trans-array.c
===
--- gcc/fortran/trans-array.c	(Revision 259739)
+++ gcc/fortran/trans-array.c	(Arbeitskopie)
@@ -3022,7 +3022,7 @@
 }
 
   /* Multiply by the stride.  */
-  if (!integer_onep (stride))
+  if (stride != NULL && !integer_onep (stride))
 index = fold_build2_loc (input_location, MULT_EXPR, gfc_array_index_type,
 			 index, stride);
 
Index: gcc/fortran/trans-intrinsic.c
===
--- gcc/fortran/trans-intrinsic.c	(Revision 259739)
+++ gcc/fortran/trans-intrinsic.c	(Arbeitskopie)
@@ -1266,34 +1266,124 @@
 }
   else
 {
-  /* If has_vector, pass descriptor for whole array and the
- vector bounds separately.  */
-  gfc_array_ref *ar, ar2;
-  bool has_vector = false;
+  bool has_vector = gfc_has_vector_subscript (lhs_expr);
 
-  if (gfc_is_coindexed (lhs_expr) && gfc_has_vector_subscript (lhs_expr))
+  if (gfc_is_coindexed (lhs_expr) || !has_vector)
 	{
-  has_vector = true;
-  ar = gfc_find_array_ref (lhs_expr);
-	  ar2 = *ar;
-	  memset (ar, '\0', sizeof (*ar));
-	  ar->as = ar2.as;
-	  ar->type = AR_FULL;
+	  /* If has_vector, pass descriptor for whole array and the
+	 vector bounds separately.  */
+	  gfc_array_ref *ar, ar2;
+	  bool has_tmp_lhs_array = false;
+	  if (has_vector)
+	{
+	  has_tmp_lhs_array = true;
+	  ar = gfc_find_array_ref (lhs_expr);
+	  ar2 = *ar;
+	  memset (ar, '\0', sizeof (*ar));
+	  ar->as = ar2.as;
+	  ar->type = AR_FULL;
+	}
+	  lhs_se.want_pointer = 1;
+	  gfc_conv_expr_descriptor (_se, lhs_expr);
+	  /* Using gfc_conv_expr_descriptor, we only get the descriptor, but
+	 that has the wrong type if component references are done.  */
+	  lhs_type = gfc_typenode_for_spec (_expr->ts);
+	  tmp = build_fold_indirect_ref_loc (input_location, lhs_se.expr);
+	  gfc_add_modify (_se.pre, gfc_conv_descriptor_dtype (tmp),
+			  gfc_get_dtype_rank_type (has_vector ? ar2.dimen
+			  : lhs_expr->rank,
+		   lhs_type));
+	  if (has_tmp_lhs_array)
+	{
+	  vec = conv_caf_vector_subscript (, lhs_se.expr, );
+	  *ar = 

Re: [ARM] Fix PR85434: spill of stack protector's guard address

[Richard Sandiford]

Thomas Preudhomme  writes:
> On Arm (Aarch32 and Aarch64) the stack protector's guard is accessed by
> loading its address first before loading its value from it as part of
> the stack_protect_set or stack_protect_check insn pattern. This creates
> the risk of spilling between the two.
>
> It is particularly likely on Aarch32 when compiling PIC code because
> - computing the address takes several instructions (first compute the
>   GOT base and then the GOT entry by adding an offset) which increases
>   the likelyhood of CSE
> - the address computation can be CSEd due to the GOT entry computation
>   being a MEM of the GOT base + an UNSPEC offset rather than an UNSPEC
>   of a MEM like on AArche64.
>
> This patch address both issues by (i) adding some scheduler barriers
> around the stack protector code and (ii) making all memory loads
> involved in computing the guard's address volatile. The use of volatile
> rather than unspec was chosen so that the patterns for computing the
> guard address can be the same as for normal global variable access thus
> reusing more code. Finally the patch also improves the documentation to
> mention the need to be careful when computing the address of the guard.

[...]

> diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
> index deab929..c7ced8f 100644
> --- a/gcc/cfgexpand.c
> +++ b/gcc/cfgexpand.c
> @@ -6156,6 +6156,10 @@ stack_protect_prologue (void)
>tree guard_decl = targetm.stack_protect_guard ();
>rtx x, y;
>  
> +  /* Prevent scheduling of instruction(s) between computation of the guard's
> + address and setting of the canari to avoid any spill of the guard's
> + address if computed outside the setting of the canari.  */
> +  emit_insn (gen_blockage ());
>x = expand_normal (crtl->stack_protect_guard);
>if (guard_decl)
>  y = expand_normal (guard_decl);

Scheduling barriers should definitely make spilling less likely,
but I don't think they avoid it completely.  For that I think we'd need to:

(a) make sure that gen_stack_protect_set gets passed (mem (symbol_ref)),
which we can probably do by using DECL_RTL directly.  (Or failing that,
expand_expr with EXPAND_CONST_ADDRESS should work.)

(b) make the target pattern accept (mem (symbol_ref)) and only legitimise
it during a post-RA split.

The scheduling barriers would then be unnecessary, since the pattern
would be indivisible and self-contained until after RA.

On its own that would probably mean changing all backends to accept
the new style of mem.  One way of avoiding that would be to use the
maybe_expand_insn interface with targetm.code_for_stack_protect_set
instead of calling targetm.gen_stack_protect_set directly.
Arguably that's better anyway.

Thanks,
Richard

Re: [PATCH PR other/77609] Let the assembler choose ELF section types for miscellaneous named sections

[Roland McGrath via gcc-patches]

I'm back for stage 1!

The same patch from https://gcc.gnu.org/ml/gcc-patches/2018-02/msg01549.html
rebases cleanly and I didn't change anything but the date on the log entry
since what I posted there.  The fresh rebase is on the roland/pr77609 git
branch for your convenience.

It has no check-gcc failures on x86_64-linux-gnu.

OK to commit to trunk now?

When will be the right time to raise the question of backporting it,
perhaps shortly after the 8 release?


Thanks,
Roland

Re: [PATCH] Do not set nothrow flag on recursive function with stack checking

[Eric Botcazou]

> This looks not a generic enough fix to me - consider
> 
> void foo(void) { int a[10]; a[0] = 1; a[9] = 1; }
> int main() { try { foo (); } catch (...) {} }
> 
> with -fnon-call-exceptions.  If we'd like to catch the SEGV from stack
> overflows then your fix doesn't handle the non-recursive case nor the case
> where -fstack-check is not supplied.

But -fstack-check is required to detect stack overflows...  Moreover the above 
testcase will be entirely optimized at -O so there is no stack overflow at -O.

> So to me your attempt in fixing this isn't complete but a complete fix would
> be quite pessimizing :/ (for -fnon-call-exceptions)

Then let's at least fix the recursive case since it's simple and cheap.  But I 
can indeed key this on -fnon-call-exceptions explicitly.

> At least all this should be documented somewhere, that is, what to expect
> when trying to catch stack faults in general with -fnon-call-exceptions
> [-fstack-check].

It's already documented that you need -fstack-check to detect stack overflows.
But I can add a blurb to the -fnon-call-exceptions entry about it.

Revised patch attached.

-- 
Eric BotcazouIndex: ipa-pure-const.c
===
--- ipa-pure-const.c	(revision 259642)
+++ ipa-pure-const.c	(working copy)
@@ -674,12 +674,21 @@ check_call (funct_state local, gcall *ca
 local->can_free = true;
 
   /* When not in IPA mode, we can still handle self recursion.  */
-  if (!ipa && callee_t
+  if (!ipa
+  && callee_t
   && recursive_call_p (current_function_decl, callee_t))
 {
   if (dump_file)
-fprintf (dump_file, "Recursive call can loop.\n");
+fprintf (dump_file, "recursive call can loop\n");
   local->looping = true;
+  if (possibly_throws_externally
+	  && flag_non_call_exceptions
+	  && flag_stack_check)
+	{
+	  if (dump_file)
+	fprintf (dump_file, "recursive call can throw\n");
+	  local->can_throw = true;
+	}
 }
   /* Either callee is unknown or we are doing local analysis.
  Look to see if there are any bits available for the callee (such as by
@@ -795,8 +804,7 @@ check_stmt (gimple_stmt_iterator *gsip,
   print_gimple_stmt (dump_file, stmt, 0);
 }
 
-  if (gimple_has_volatile_ops (stmt)
-  && !gimple_clobber_p (stmt))
+  if (gimple_has_volatile_ops (stmt) && !gimple_clobber_p (stmt))
 {
   local->pure_const_state = IPA_NEITHER;
   if (dump_file)
@@ -808,8 +816,7 @@ check_stmt (gimple_stmt_iterator *gsip,
 			ipa ? check_ipa_load : check_load,
 			ipa ? check_ipa_store :  check_store);
 
-  if (gimple_code (stmt) != GIMPLE_CALL
-  && stmt_could_throw_p (stmt))
+  if (gimple_code (stmt) != GIMPLE_CALL && stmt_could_throw_p (stmt))
 {
   if (cfun->can_throw_non_call_exceptions)
 	{
@@ -1822,13 +1829,18 @@ propagate_nothrow (void)
 		 not be interposed.
 		 When callee is compiled with non-call exceptions we also
 		 must check that the declaration is bound to current
-		 body as other semantically equivalent body may still
-		 throw.  */
+		 body as other semantically equivalent body may throw.
+		 Moreover, if the call is recursive, we must consider that
+		 the function may throw a stack overflow exception.  */
 		  if (avail <= AVAIL_INTERPOSABLE
 		  || (!TREE_NOTHROW (y->decl)
 			  && (get_function_state (y)->can_throw
 			  || (opt_for_fn (y->decl, flag_non_call_exceptions)
-  && !e->callee->binds_to_current_def_p (w)
+  && !e->callee->binds_to_current_def_p (w))
+			  || (y == w
+  && opt_for_fn (y->decl,
+		 flag_non_call_exceptions)
+  && flag_stack_check
 		can_throw = true;
 		}
 	  for (ie = w->indirect_calls; ie && !can_throw;
@@ -2288,7 +2300,7 @@ make_pass_warn_function_noreturn (gcc::c
   return new pass_warn_function_noreturn (ctxt);
 }
 
-/* Simple local pass for pure const discovery reusing the analysis from
+/* Simple local pass for nothrow discovery reusing the analysis from
ipa_pure_const.   This pass is effective when executed together with
other optimization passes in early optimization pass queue.  */
 
@@ -2352,8 +2364,9 @@ pass_nothrow::execute (function *)
 	if (is_gimple_call (gsi_stmt (gsi)))
 	  {
 		tree callee_t = gimple_call_fndecl (gsi_stmt (gsi));
-		if (callee_t && recursive_call_p (current_function_decl,
-		  callee_t))
+		if (callee_t
+		&& recursive_call_p (current_function_decl, callee_t)
+		&& !(flag_non_call_exceptions && flag_stack_check))
 		  continue;
 	  }
 	
Index: doc/invoke.texi
===
--- doc/invoke.texi	(revision 259642)
+++ doc/invoke.texi	(working copy)
@@ -12812,6 +12812,9 @@ not exist everywhere.  Moreover, it only
 instructions to throw exceptions, i.e.@: memory references or floating-point
 instructions.  It does not allow exceptions to be thrown from
 arbitrary signal 

Re: [PATCH] Fix loop-header copying do-while loop detection (PR85116)

[Richard Biener]

On Sat, 28 Apr 2018, Richard Biener wrote:

> On Fri, 27 Apr 2018, Richard Biener wrote:
> 
> > On Fri, 27 Apr 2018, David Edelsohn wrote:
> > 
> > > Hi, Richi
> > > 
> > > This patches causes a boostrap failure on AIX.  Everything miscompares.
> > > The code itself is the same, but the DWARF debug information contains many
> > > differences.
> > 
> > Does AIX use bootstrap-debug by default?  I don't see how the patch
> > can cause this kind of issue directly but of course it will change
> > CH decisions which may expose latent bugs somewhere.
> > 
> > Can you provide more details please, like actual differences?
> > I would have expected the dwarf2out.c change to be a more likely
> > candidate for such symtoms but I trust that you did properly
> > bisect to my patch?
> 
> OK, so the x86-64 -O3 bootstrap failure is not caused by this patch,
> reverting it doesn't fix the issue.
> 
> The difference w/ the patch reverted is in debug info, all (indirect)
> strings reside at different offsets.  If I strip the objects they
> compare identical.
> 
> I'm trying reversal of that dwarf2out patch now.

That didn't help.  Reverting the bootstrap -f[no-]checking patch did.

Richard.

[PATCH, i386]: Fix PR84431, suboptimal code for masked shifts

[Uros Bizjak]

2018-04-28  Uros Bizjak  

PR target/84431
* config/i386/i386.md (*ashl3_doubleword_mask): New pattern.
(*ashl3_doubleword_mask_1): Ditto.
(*3_doubleword_mask): Ditto.
(*3_doubleword_mask_1): Ditto.

testsuite/ChangeLog:

2018-04-28  Uros Bizjak  

PR target/84431
* gcc.target/i386/pr84431.c: New test.

Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.

Committed to mainline SVN.

Uros.
Index: config/i386/i386.md
===
--- config/i386/i386.md (revision 259701)
+++ config/i386/i386.md (working copy)
@@ -10357,6 +10357,77 @@
   ""
   "ix86_expand_binary_operator (ASHIFT, mode, operands); DONE;")
 
+(define_insn_and_split "*ashl3_doubleword_mask"
+  [(set (match_operand: 0 "register_operand")
+   (ashift:
+ (match_operand: 1 "register_operand")
+ (subreg:QI
+   (and:SI
+ (match_operand:SI 2 "register_operand" "c")
+ (match_operand:SI 3 "const_int_operand")) 0)))
+   (clobber (reg:CC FLAGS_REG))]
+  "INTVAL (operands[3]) <= ( * BITS_PER_UNIT)-1
+   && can_create_pseudo_p ()"
+  "#"
+  "&& 1"
+  [(parallel
+ [(set (match_dup 6)
+  (ior:DWIH (ashift:DWIH (match_dup 6) (match_dup 2))
+(lshiftrt:DWIH (match_dup 5)
+  (minus:QI (match_dup 8) (match_dup 2)
+  (clobber (reg:CC FLAGS_REG))])
+   (parallel
+ [(set (match_dup 4)
+  (ashift:DWIH (match_dup 5) (match_dup 2)))
+  (clobber (reg:CC FLAGS_REG))])]
+{
+  split_double_mode (mode, [0], 2, [4], [6]);
+
+  operands[8] = GEN_INT ( * BITS_PER_UNIT);
+
+  if (INTVAL (operands[3]) < ( * BITS_PER_UNIT)-1)
+emit_insn (gen_andsi3 (operands[2], operands[2], operands[3]));
+
+  operands[2] = gen_lowpart (QImode, operands[2]);
+
+  if (!rtx_equal_p (operands[6], operands[7]))
+emit_move_insn (operands[6], operands[7]);
+})
+
+(define_insn_and_split "*ashl3_doubleword_mask_1"
+  [(set (match_operand: 0 "register_operand")
+   (ashift:
+ (match_operand: 1 "register_operand")
+ (and:QI
+   (match_operand:QI 2 "register_operand" "c")
+   (match_operand:QI 3 "const_int_operand"
+   (clobber (reg:CC FLAGS_REG))]
+  "INTVAL (operands[3]) <= ( * BITS_PER_UNIT)-1
+   && can_create_pseudo_p ()"
+  "#"
+  "&& 1"
+  [(parallel
+ [(set (match_dup 6)
+  (ior:DWIH (ashift:DWIH (match_dup 6) (match_dup 2))
+(lshiftrt:DWIH (match_dup 5)
+  (minus:QI (match_dup 8) (match_dup 2)
+  (clobber (reg:CC FLAGS_REG))])
+   (parallel
+ [(set (match_dup 4)
+  (ashift:DWIH (match_dup 5) (match_dup 2)))
+  (clobber (reg:CC FLAGS_REG))])]
+{
+  split_double_mode (mode, [0], 2, [4], [6]);
+
+  operands[8] = GEN_INT ( * BITS_PER_UNIT);
+
+  if (INTVAL (operands[3]) < ( * BITS_PER_UNIT)-1)
+emit_insn (gen_andqi3 (operands[2], operands[2], operands[3]));
+
+  if (!rtx_equal_p (operands[6], operands[7]))
+emit_move_insn (operands[6], operands[7]);
+})
+
 (define_insn "*ashl3_doubleword"
   [(set (match_operand:DWI 0 "register_operand" "=")
(ashift:DWI (match_operand:DWI 1 "reg_or_pm1_operand" "0n")
@@ -11038,6 +11109,77 @@
   ""
   [(set_attr "isa" "*,bmi2")])
 
+(define_insn_and_split "*3_doubleword_mask"
+  [(set (match_operand: 0 "register_operand")
+   (any_shiftrt:
+ (match_operand: 1 "register_operand")
+ (subreg:QI
+   (and:SI
+ (match_operand:SI 2 "register_operand" "c")
+ (match_operand:SI 3 "const_int_operand")) 0)))
+   (clobber (reg:CC FLAGS_REG))]
+  "INTVAL (operands[3]) <= ( * BITS_PER_UNIT)-1
+   && can_create_pseudo_p ()"
+  "#"
+  "&& 1"
+  [(parallel
+ [(set (match_dup 4)
+  (ior:DWIH (lshiftrt:DWIH (match_dup 4) (match_dup 2))
+(ashift:DWIH (match_dup 7)
+  (minus:QI (match_dup 8) (match_dup 2)
+  (clobber (reg:CC FLAGS_REG))])
+   (parallel
+ [(set (match_dup 6)
+  (any_shiftrt:DWIH (match_dup 7) (match_dup 2)))
+  (clobber (reg:CC FLAGS_REG))])]
+{
+  split_double_mode (mode, [0], 2, [4], [6]);
+
+  operands[8] = GEN_INT ( * BITS_PER_UNIT);
+
+  if (INTVAL (operands[3]) < ( * BITS_PER_UNIT)-1)
+emit_insn (gen_andsi3 (operands[2], operands[2], operands[3]));
+
+  operands[2] = gen_lowpart (QImode, operands[2]);
+
+  if (!rtx_equal_p (operands[4], operands[5]))
+emit_move_insn (operands[4], operands[5]);
+})
+
+(define_insn_and_split "*3_doubleword_mask_1"
+  [(set (match_operand: 0 "register_operand")
+   (any_shiftrt:
+ (match_operand: 1 "register_operand")
+ (and:QI
+   (match_operand:QI 2 "register_operand" "c")
+   (match_operand:QI 3 "const_int_operand"
+   (clobber (reg:CC FLAGS_REG))]
+  "INTVAL (operands[3]) <= ( * BITS_PER_UNIT)-1
+   && can_create_pseudo_p ()"
+  "#"
+  "&& 1"
+  [(parallel
+ [(set 

Re: [PATCH] Shave some bit of work off verify_gimple_in_cfg

[Richard Biener]

On Fri, 27 Apr 2018, Richard Biener wrote:

> 
> This makes us not record all stmts in the visited_stmts hash-set
> but only those that are possibly valid EH stmts.  Should save us
> some cycles here.
> 
> Bootstrap running on x86_64-unknown-linux-gnu.

This is what i have applied.

Richard.

2018-04-28  Richard Biener  

* tree-cfg.c (verify_gimple_phi): Take a gphi * argument.
(verify_gimple_in_cfg): Rename visited_stmts to visited_throwing_stmts
to reflect use.  Only add interesting stmts.

Index: gcc/tree-cfg.c
===
--- gcc/tree-cfg.c  (revision 259703)
+++ gcc/tree-cfg.c  (working copy)
@@ -5103,7 +5103,7 @@ verify_gimple_stmt (gimple *stmt)
and false otherwise.  */
 
 static bool
-verify_gimple_phi (gimple *phi)
+verify_gimple_phi (gphi *phi)
 {
   bool err = false;
   unsigned i;
@@ -5422,7 +5422,7 @@ verify_gimple_in_cfg (struct function *f
 
   timevar_push (TV_TREE_STMT_VERIFY);
   hash_set visited;
-  hash_set visited_stmts;
+  hash_set visited_throwing_stmts;
 
   /* Collect all BLOCKs referenced by the BLOCK tree of FN.  */
   hash_set blocks;
@@ -5444,8 +5444,6 @@ verify_gimple_in_cfg (struct function *f
  bool err2 = false;
  unsigned i;
 
- visited_stmts.add (phi);
-
  if (gimple_bb (phi) != bb)
{
  error ("gimple_bb (phi) is set to a wrong basic block");
@@ -5501,8 +5499,6 @@ verify_gimple_in_cfg (struct function *f
  tree addr;
  int lp_nr;
 
- visited_stmts.add (stmt);
-
  if (gimple_bb (stmt) != bb)
{
  error ("gimple_bb (stmt) is set to a wrong basic block");
@@ -5552,6 +5548,8 @@ verify_gimple_in_cfg (struct function *f
 that they cannot throw, that we update other data structures
 to match.  */
  lp_nr = lookup_stmt_eh_lp (stmt);
+ if (lp_nr != 0)
+   visited_throwing_stmts.add (stmt);
  if (lp_nr > 0)
{
  if (!stmt_could_throw_p (stmt))
@@ -5575,11 +5573,11 @@ verify_gimple_in_cfg (struct function *f
}
 }
 
-  eh_error_found = false;
   hash_map *eh_table = get_eh_throw_stmt_table (cfun);
+  eh_error_found = false;
   if (eh_table)
 eh_table->traverse
-  (_stmts);
+  (_throwing_stmts);
 
   if (err || eh_error_found)
 internal_error ("verify_gimple failed");

Re: [PATCH] Fix loop-header copying do-while loop detection (PR85116)

[Richard Biener]

On Fri, 27 Apr 2018, Richard Biener wrote:

> On Fri, 27 Apr 2018, David Edelsohn wrote:
> 
> > Hi, Richi
> > 
> > This patches causes a boostrap failure on AIX.  Everything miscompares.
> > The code itself is the same, but the DWARF debug information contains many
> > differences.
> 
> Does AIX use bootstrap-debug by default?  I don't see how the patch
> can cause this kind of issue directly but of course it will change
> CH decisions which may expose latent bugs somewhere.
> 
> Can you provide more details please, like actual differences?
> I would have expected the dwarf2out.c change to be a more likely
> candidate for such symtoms but I trust that you did properly
> bisect to my patch?

OK, so the x86-64 -O3 bootstrap failure is not caused by this patch,
reverting it doesn't fix the issue.

The difference w/ the patch reverted is in debug info, all (indirect)
strings reside at different offsets.  If I strip the objects they
compare identical.

I'm trying reversal of that dwarf2out patch now.

Richard.

Re: ATTRIBUTE_NONSTRING

[Alan Modra]

On Fri, Apr 27, 2018 at 06:24:28PM -0400, Hans-Peter Nilsson wrote:
> On Fri, 27 Apr 2018, Alan Modra wrote:
> 
> > This patch adds ATTRIBUTE_NONSTRING, which will be used to curb
> > -Wstringop-truncation warnings in binutils.  OK to apply?
> >
> > * ansidecl.h (ATTRIBUTE_NONSTRING): Define.
> >
> > diff --git a/include/ansidecl.h b/include/ansidecl.h
> > index c11daff..ec5f34d 100644
> > --- a/include/ansidecl.h
> > +++ b/include/ansidecl.h
> > @@ -283,6 +283,15 @@ So instead we use the macro below and test it against 
> > specific values.  */
> >  # endif /* GNUC >= 4.9 */
> >  #endif /* ATTRIBUTE_NO_SANITIZE_UNDEFINED */
> >
> > +/* Attribute 'nonstring' was valid as of gcc 8.  */
> > +#ifndef ATTRIBUTE_NONSTRING
> > +# if GCC_VERSION >= 8000
> > +#  define ATTRIBUTE_NONSTRING __attribute__ ((nonstring))
> 
> Uglify nonstring (as __nonstring__)?

Yes, that would be better.  I copied the immediately preceding
ATTRIBUTE_NO_SANITIZE_UNDEFINED without thinking.

-- 
Alan Modra
Australia Development Lab, IBM