Re: [SFN+LVU+IEPM v4 7/9] [LVU] Introduce location views

[Alexandre Oliva]

On Jan 25, 2018, Alexandre Oliva  wrote:

> On Jan 24, 2018, Jakub Jelinek  wrote:
>> On Tue, Dec 12, 2017 at 12:52:18AM -0200, Alexandre Oliva wrote:
>>> +DW_LLE_GNU_view_pair = 0x09,
>>> +#define DW_LLE_view_pair DW_LLE_GNU_view_pair

>> This looks wrong.  The proposal has not been accepted yet, so you
>> really can't know if DW_LLE_view_pair will be like that or whether it
>> will have value of 9.  Unfortunately, the DWARF standard doesn't specify a
>> vendor range for DW_LLE_* values.  I'd use 0xf0 or so, and don't pretend
>> there is DW_LLE_view_pair at all, just use DW_LLE_GNU_view_pair everywhere.
>> Jason, what do you think?

> My reasoning was that, since we'd only emit this as an
> explicitly-requested backward-incompatible extension to DWARF-5 (by
> specifying -gdwarf-6 in this patch, but the option spelling could be
> changed), any LLE number whatsoever would do.  The point of the #define
> was to write the code in GCC so that, once DW_LLE_view_pair was
> standardized (if it ever was), we'd just set up an enum for it and we'd
> be done, but that would only happen at DWARF6+ anyway, so we'd be able
> to tell, since we'd then have actual DWARF6, rather than DWARF5 with an
> incompatible extensions (which is what we emit with the current
> -gdwarf-6 option; see below).

Also...  I have implemented DW_LLE_view_pair support in binutils's debug
info dumping code, based on the proposed extension, and that has already
been released, so changing the number in GCC would make it incompatible
with the already-released binutils.

You may notice I've renamed the GCC option to emit this extension in the
latest version of the patch.

-- 
Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

Re: [SFN+LVU+IEPM v4 7/9] [LVU] Introduce location views

[Alexandre Oliva]

Here's the updated version of the LVU patch, integrating changes
requested or proposed by yourself and by Jakub.


for  include/ChangeLog

* dwarf2.def (DW_AT_GNU_locviews): New.
* dwarf2.h (enum dwarf_location_list_entry_type): Add
DW_LLE_GNU_view_pair.
(DW_LLE_view_pair): Define.

for  gcc/ChangeLog

* common.opt (gvariable-location-views): New.
(gvariable-location-views=incompat5): New.
* config.in: Rebuilt.
* configure: Rebuilt.
* configure.ac: Test assembler for view support.
* dwarf2asm.c (dw2_asm_output_symname_uleb128): New.
* dwarf2asm.h (dw2_asm_output_symname_uleb128): Declare.
* dwarf2out.c (var_loc_view): New typedef.
(struct dw_loc_list_struct): Add vl_symbol, vbegin, vend.
(dwarf2out_locviews_in_attribute): New.
(dwarf2out_locviews_in_loclist): New.
(dw_val_equal_p): Compare val_view_list of dw_val_class_view_lists.
(enum dw_line_info_opcode): Add LI_adv_address.
(struct dw_line_info_table): Add view.
(RESET_NEXT_VIEW, RESETTING_VIEW_P): New macros.
(DWARF2_ASM_VIEW_DEBUG_INFO): Define default.
(zero_view_p): New variable.
(ZERO_VIEW_P): New macro.
(output_asm_line_debug_info): New.
(struct var_loc_node): Add view.
(add_AT_view_list, AT_loc_list): New.
(add_var_loc_to_decl): Add view param.  Test it against last.
(new_loc_list): Add view params.  Record them.
(AT_loc_list_ptr): Handle loc and view lists.
(view_list_to_loc_list_val_node): New.
(print_dw_val): Handle dw_val_class_view_list.
(size_of_die): Likewise.
(value_format): Likewise.
(loc_list_has_views): New.
(gen_llsym): Set vl_symbol too.
(maybe_gen_llsym, skip_loc_list_entry): New.
(dwarf2out_maybe_output_loclist_view_pair): New.
(output_loc_list): Output view list or entries too.
(output_view_list_offset): New.
(output_die): Handle dw_val_class_view_list.
(output_dwarf_version): New.
(output_compilation_unit_header): Use it.
(output_skeleton_debug_sections): Likewise.
(output_rnglists, output_line_info): Likewise.
(output_pubnames, output_aranges): Update version comments.
(output_one_line_info_table): Output view numbers in asm comments.
(dw_loc_list): Determine current endview, pass it to new_loc_list.
Call maybe_gen_llsym.
(loc_list_from_tree_1): Adjust.
(add_AT_location_description): Create view list attribute if
needed, check it's absent otherwise.
(convert_cfa_to_fb_loc_list): Adjust.
(maybe_emit_file): Call output_asm_line_debug_info for test.
(dwarf2out_var_location): Reset views as needed.  Precompute
add_var_loc_to_decl args.  Call get_attr_min_length only if we have the
attribute.  Set view.
(new_line_info_table): Reset next view.
(set_cur_line_info_table): Call output_asm_line_debug_info for test.
(dwarf2out_source_line): Likewise.  Output view resets and labels to
the assembler, or select appropriate line info opcodes.
(prune_unused_types_walk_attribs): Handle dw_val_class_view_list.
(optimize_string_length): Catch it.  Adjust.
(resolve_addr): Copy vl_symbol along with ll_symbol.  Handle
dw_val_class_view_list, and remove it if no longer needed.
(hash_loc_list): Hash view numbers.
(loc_list_hasher::equal): Compare them.
(optimize_location_lists): Check whether a view list symbol is
needed, and whether the locview attribute is present, and
whether they match.  Remove the locview attribute if no longer
needed.
(index_location_lists): Call skip_loc_list_entry for test.
(dwarf2out_finish): Call output_asm_line_debug_info for test.
Use output_dwarf_version.
* dwarf2out.h (enum dw_val_class): Add dw_val_class_view_list.
(struct dw_val_node): Add val_view_list.
* final.c (SEEN_NEXT_VIEW): New.
(set_next_view_needed): New.
(clear_next_view_needed): New.
(maybe_output_next_view): New.
(final_start_function): Rename to...
(final_start_function_1): ... this.  Take pointer to FIRST,
add SEEN parameter.  Emit param bindings in the initial view.
(final_start_function): Reintroduce SEEN-less interface.
(final): Rename to...
(final_1): ... this.  Take SEEN parameter.  Output final pending
next view at the end.
(final): Reintroduce seen-less interface.
(final_scan_insn): Output pending next view before switching
sections or ending a block.  Mark the next view as needed when
outputting variable locations.  Notify debug backend of section
changes, and of location view changes.
(rest_of_handle_final): Adjust.
* 

Re: [SFN+LVU+IEPM v4 7/9] [LVU] Introduce location views

[Alexandre Oliva]

Here's an incremental patch with the changes I made in response to your
requests.  I'll post the complete updated patch momentarily.

diff --git a/gcc/common.opt b/gcc/common.opt
index 55d9cdd714ff..7e024fdab124 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -2957,9 +2957,12 @@ Common Driver Report Var(flag_gtoggle)
 Toggle debug information generation.
 
 gvariable-location-views
-Common Driver Var(debug_variable_location_views) Init(2)
+Common Driver Var(debug_variable_location_views, 1) Init(2)
 Augment variable location lists with progressive views.
 
+gvariable-location-views=incompat5
+Common Driver RejectNegative Var(debug_variable_location_views, -1) Init(2)
+
 gvms
 Common Driver JoinedOrMissing Negative(gxcoff)
 Generate debug information in VMS format.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 7f09fcb64968..045aab78ce51 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -7220,8 +7220,10 @@ compiling with optimization (@option{-Os}, @option{-O}, 
@option{-O2},
 @dots{}), and outputting DWARF 2 debug information at the normal level.
 
 @item -gvariable-location-views
+@item -gvariable-location-views=incompat5
 @item -gno-variable-location-views
 @opindex gvariable-location-views
+@opindex gvariable-location-views=incompat5
 @opindex gno-variable-location-views
 Augment variable location lists with progressive view numbers implied
 from the line number table.  This enables debug information consumers to
@@ -7234,8 +7236,16 @@ number tables and location lists are fully 
backward-compatible, so they
 can be consumed by debug information consumers that are not aware of
 these augmentations, but they won't derive any benefit from them either.
 This is enabled by default when outputting DWARF 2 debug information at
-the normal level, as long as @code{-fvar-tracking-assignments} is
-enabled and @code{-gstrict-dwarf} is not.
+the normal level, as long as @option{-fvar-tracking-assignments} is
+enabled and @option{-gstrict-dwarf} is not.
+
+There is a proposed representation for view numbers that is not backward
+compatible with the location list format introduced in DWARF 5, that can
+be enabled with @option{-gvariable-location-views=incompat5}.  This
+option may be removed in the future, is only provided as a reference
+implementation of the proposed representation.  Debug information
+consumers are not expected to support this extended format, and they
+would be rendered unable to decode location lists using it.
 
 @item -gz@r{[}=@var{type}@r{]}
 @opindex gz
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 9ef7fa3516d4..3cf79270b72c 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -1350,7 +1350,7 @@ dwarf_stack_op_name (unsigned int op)
 static inline bool
 dwarf2out_locviews_in_attribute ()
 {
-  return debug_variable_location_views && dwarf_version <= 5;
+  return debug_variable_location_views == 1;
 }
 
 /* Return TRUE iff we're to output location view lists as part of the
@@ -1362,7 +1362,7 @@ dwarf2out_locviews_in_loclist ()
 #ifndef DW_LLE_view_pair
   return false;
 #else
-  return debug_variable_location_views && dwarf_version >= 6;
+  return debug_variable_location_views == -1;
 #endif
 }
 
@@ -3164,7 +3164,7 @@ skeleton_chain_node;
 #endif
 
 /* A bit is set in ZERO_VIEW_P if we are using the assembler-supported
-   view computation, and it is refers to a view identifier for which
+   view computation, and it refers to a view identifier for which we
will not emit a label because it is known to map to a view number
zero.  We won't allocate the bitmap if we're not using assembler
support for location views, but we have to make the variable
@@ -27344,23 +27344,40 @@ dwarf2out_source_line (unsigned int line, unsigned 
int column,
  zero_view_p = BITMAP_GGC_ALLOC ();
  bitmap_set_bit (zero_view_p, 0);
}
- if (RESETTING_VIEW_P (table->view))
+ if (!RESETTING_VIEW_P (table->view))
+   {
+ /* When we're using the assembler to compute view
+numbers, we output symbolic labels after "view" in
+.loc directives, and the assembler will set them for
+us, so that we can refer to the view numbers in
+location lists.  The only exceptions are when we know
+a view will be zero: "-0" is a forced reset, used
+e.g. in the beginning of functions, whereas "0" tells
+the assembler to check that there was a PC change
+since the previous view, in a way that implicitly
+resets the next view.  */
+ fputs (" view ", asm_out_file);
+ char label[MAX_ARTIFICIAL_LABEL_BYTES];
+ ASM_GENERATE_INTERNAL_LABEL (label, "LVU", table->view);
+ assemble_name (asm_out_file, label);
+ table->view = ++lvugid;
+   }
+ else
{
  if (!table->in_use)
fputs (" 

Re: [PATCH] Fix floating point handling in dom (PR tree-optimization/84235)

[Richard Biener]

On February 6, 2018 9:40:37 PM GMT+01:00, Jakub Jelinek  
wrote:
>Hi!
>
>As the following testcase shows, dom2 miscompiles floating point x - x
>into 0.0 even when x could be infinity and x - x then a NaN.
>The corresponding match.pd optimization is:
>/* Simplify x - x.
>   This is unsafe for certain floats even in non-IEEE formats.
>   In IEEE, it is unsafe because it does wrong for NaNs.
>   Also note that operand_equal_p is always false if an operand
>   is volatile.  */
>(simplify
> (minus @0 @0)
> (if (!FLOAT_TYPE_P (type) || !HONOR_NANS (type))
>  { build_zero_cst (type); }))
>The patch makes it match what match.pd does.
>We also have:
> /* X / X is one.  */
> (simplify
>  (div @0 @0)
>/* But not for 0 / 0 so that we can get the proper warnings and errors.
> And not for _Fract types where we can't build 1.  */
>  (if (!integer_zerop (@0) && !ALL_FRACT_MODE_P (TYPE_MODE (type)))
>   { build_one_cst (type); }))
>We can ignore the 0 / 0 case, we have both operands SSA_NAMEs and
>match.pd
>only avoids optimizing away literal 0 / 0, but the rest is valid, some
>fract
>types don't have a way to express 1.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK. 

I wonder why we have to re-implement all this in DOM. there's enough of match 
and simplify interfaces to make it use that? 

Richard. 


>2018-02-06  Jakub Jelinek  
>
>   PR tree-optimization/84235
>   * tree-ssa-scopedtables.c
>   (avail_exprs_stack::simplify_binary_operation): Fir MINUS_EXPR, punt
>   if the subtraction is performed in floating point type where NaNs are
>   honored.  For *DIV_EXPR, punt for ALL_FRACT_MODE_Ps where we can't
>   build 1.  Formatting fix.
>
>   * gcc.c-torture/execute/ieee/pr84235.c: New test.
>
>--- gcc/tree-ssa-scopedtables.c.jj 2018-01-03 10:19:54.528533857 +0100
>+++ gcc/tree-ssa-scopedtables.c2018-02-06 14:58:08.944673984 +0100
>@@ -182,8 +182,15 @@ avail_exprs_stack::simplify_binary_opera
> case BIT_AND_EXPR:
>   return gimple_assign_rhs1 (stmt);
> 
>-case BIT_XOR_EXPR:
> case MINUS_EXPR:
>+  /* This is unsafe for certain floats even in non-IEEE
>+ formats.  In IEEE, it is unsafe because it does
>+ wrong for NaNs.  */
>+  if (FLOAT_TYPE_P (result_type)
>+  && HONOR_NANS (result_type))
>+break;
>+  /* FALLTHRU */
>+case BIT_XOR_EXPR:
> case TRUNC_MOD_EXPR:
> case CEIL_MOD_EXPR:
> case FLOOR_MOD_EXPR:
>@@ -195,6 +202,9 @@ avail_exprs_stack::simplify_binary_opera
> case FLOOR_DIV_EXPR:
> case ROUND_DIV_EXPR:
> case EXACT_DIV_EXPR:
>+  /* Avoid _Fract types where we can't build 1.  */
>+  if (ALL_FRACT_MODE_P (TYPE_MODE (result_type)))
>+break;
>   return build_one_cst (result_type);
> 
> default:
>@@ -204,8 +214,8 @@ avail_exprs_stack::simplify_binary_opera
>   break;
> }
> 
>-default:
>-  break;
>+  default:
>+break;
>   }
>   }
> }
>--- gcc/testsuite/gcc.c-torture/execute/ieee/pr84235.c.jj  2018-02-06
>15:04:26.528454766 +0100
>+++ gcc/testsuite/gcc.c-torture/execute/ieee/pr84235.c 2018-02-06
>15:05:06.836341334 +0100
>@@ -0,0 +1,11 @@
>+/* PR tree-optimization/84235 */
>+
>+int
>+main ()
>+{
>+  double d = 1.0 / 0.0;
>+  _Bool b = d == d && (d - d) != (d - d);
>+  if (!b)
>+__builtin_abort ();
>+  return 0;
>+}
>
>   Jakub

Re: [SFN+LVU+IEPM v4 7/9] [LVU] Introduce location views

[Alexandre Oliva]

On Jan 30, 2018, Richard Sandiford  wrote:

>> But it is my understanding that both of the following are correct:
>> 
>> return (verylongcondition
>> && otherlongcondition__);
>> 
>> return verylongcondition
>>   && otherlongcondition__;
>> 
>> The first, because the parenthesized expression is continued with
>> indentation to match the parenthesis, the second because the return
>> statement is continued with the correct indentation for the continuation
>> of a statement.

> Thought it had to be the first.  When they talk about indenting leading
> operators, the conventions say:

>   Insert extra parentheses so that Emacs will indent the code properly.

> which at least implies that not inserting parantheses and indenting by
> two spaces isn't "properly".

Hmm, in my reading, the "properly" there was to contrast with the IMHO
improper, excessive manual indentation in the example that follows it:

  v = rup->ru_utime.tv_sec*1000 + rup->ru_utime.tv_usec/1000
  + rup->ru_stime.tv_sec*1000 + rup->ru_stime.tv_usec/1000;

but I think it's just as legitimate to instead understand that the above
indentation is proper, and that the only problem with it is that
mechanical reindent would turn it into

  v = rup->ru_utime.tv_sec*1000 + rup->ru_utime.tv_usec/1000
+ rup->ru_stime.tv_sec*1000 + rup->ru_stime.tv_usec/1000;

which, in *this* case, would be wrong, because you'd have '=' and '+'
operators, of different precedence, at the same level of indentation.

But in the case of return, there's no such issue with operator
precedence.

I'll raise this issue at bug-standa...@gnu.org and request
clarification.

> And I think most GCC code does stick to that (i.e. use the bracketed form).

Let's see what GNU decides, then we can decide whether to add a
GCC-specific constraint, if GNU doesn't impose it already.

Thanks!

-- 
Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

Re: [SFN+LVU+IEPM v4 7/9] [LVU] Introduce location views

[Alexandre Oliva]

On Feb  6, 2018, Jason Merrill  wrote:

> On 12/11/2017 09:52 PM, Alexandre Oliva wrote:
>> +/* output symbol LAB1 as an unsigned LEB128 quantity.  */
>> +
>> +void
>> +dw2_asm_output_symname_uleb128 (const char *lab1 ATTRIBUTE_UNUSED,
>> +const char *comment, ...)

> I'm having trouble understanding the use of symbols for views.  The
> configure test for gas support doesn't obviously define a symbol
> anywhere; is it implicitly defined by its use in the location
> directive? This could use more documentation in dwarf2out.

Yeah.  It's the .loc statement that assigns a view number to the view
number symbol.  That's documented in the gas manual, but I guess it
won't hurt to add something to dwarf2out too.  Will do.

>> +   view computation, and it is refers to a view identifier for which

> "it refers"

Thanks

> Why do we need to use a non-zero view identifier for a zero view?  Why
> can't we always use 0 instead of the bitmap?

We assign view ids before we can determine whether the assigned view id
will be a zero view.  That's because we scan insns forward, and debug
binds take effect at the next .loc directive, which might be hundreds of
insns after the first reference (lots of intervening debug binds before
the insn that will take the next view), and insns that would cause the
.loc directive to be at a different address from that of the previous
.loc might be anywhere between those two loc-generating insns, before or
after the bind.  So, by the time we have to assign an id to the view, we
don't know whether we'll find an insn that sets RESETTING_VIEW_P before
we reach the loc-emitting insn that will take that view id.

It made more sense to me to assign the ids uniformly, and then mark
those that we find to be zero when we reach them, than to scan forward
(potentially O(n^2)) to tell in advance.  This also reduces differences
in view id tracking between gcc-internal and asm view assignment.


>> DW_LNS_fixed_advance_pc is the only opcode that may change the
>> address without resetting the view.  It is available for compilers
>> to use when an address change is uncertain, e.g., when advancing
>> past opcodes that may expand to an empty sequence,
>> e.g. possibly-empty alignment sequences, optional no-operation
>> opcodes or the like.

>> +  if (debug_variable_location_views && table->view)
>> +push_dw_line_info_entry (table, LI_adv_address, label_num);

> It looks like you'll always use DW_LNS_fixed_advance_pc when we don't
> have .loc support.  Does that mean that the view never gets reset?

No, table->view will be zero if we have crossed an insn that
RESET_NEXT_VIEW, and then we'll use a LI_set_address.

> I'm uncomfortable with the special handling of this opcode; it isn't
> special in DWARF5 except as a fallback if more compact encodings
> aren't usable.  Currently GCC is even more conservative than this, and
> always use a relocation (DW_LNS_set_address); if we can use D_L_f_a_p
> instead, I would expect that to help with link times.  It seems wrong
> to use it only in the context of view support.

We didn't use it before, but if we use, that's ok.  We don't *have* to
reset the view number to zero at a PC change.  It would be all right to
never reset it, as long as the compiler and the debug info consumer
agrees about it.  Since in some cases the compiler has to assign views
itself (when the assembler doesn't support views), but it can't tell
whether there will be a PC change (e.g. at an align), we need some
mechanism that predictably refrains from resetting the view number,
otherwise the compiler might pick a view number based on a possibly
incorrect assumption about whether the align changes PC or not, and then
it might not match what the consumer, that knows whether the PC
advanced, would compute.  To make it concrete:

# ...
.L352:
# .loc 1 533 view 3 (internal compiler tracking, no assembler support)
mov r12 <- whatever
# DEBUG x => r12
.balign 32
.L353:
# .loc 1 480 view 

Consider you're the compiler and you're generating a view-augmented
loclist for variable x.  You have to indicate the view number at .L353
for the range that starts there, and possibly for the range that ends
there.

But is the view number 4 or 0?  Namely, does .balign advance PC or not?
The compiler can't possibly know.  So if it guesses .balign does advance
PC and assign a view number zero at .L353, but it guesses wrong, that
will be indistinguishable from view number zero at say .L350, because
the PC is the same.  The debug info consumer will see no PC change and
assign view number 4 to the line number table entry correspoding to the
.loc directive with view , but it won't find any loclist referencing
that view number.  Conversely, if the compiler guessed .balign did not
advance PC, it would assign view number 4 to .L353, but then, with your
suggestion, the debug info consumer would instead assign it view number
zero, and we'd be out of sync.

That's why we need an 

Re: [RFC PATCH] avoid applying attributes to explicit specializations (PR 83871)

[Martin Sebor]

On 02/05/2018 02:52 PM, Jason Merrill wrote:

On 02/04/2018 07:07 PM, Martin Sebor wrote:

To resolve the underlying root cause of the P1 bug c++/83503
- bogus -Wattributes for const and pure on function template
specialization, that we discussed last week, I've taken a stab
at making the change to avoid applying primary template's
attributes to its explicit specializations.  (The bug tracking
the underlying root cause is 83871 - wrong code for attribute
const and pure on distinct template specializations).

The attached patch is what I have so far.  It's incomplete
and not all the tests pass for a couple of reasons:

1) it only handles function templates (not class templates),
and I have no tests for those yet,


Class templates may already work the way you expect; at least aligned
does, though that doesn't involve TYPE_ATTRIBUTES.

Hmm, it seems that we currently don't propagate unused even to implicit
instantiations, a bug in the other direction:

template  struct [[gnu::unused]] A { };

int main()
{
  A a; // shouldn't warn
}


I opened bug 84221 to track it.  It's a regression WRT 4.7.

For types, it's not completely clear to me what should be
expected for attribute deprecated.  Not inheriting the
attribute means that users would be able to explicitly
specialize a deprecated primary template which is in most
cases contrary to the intent of the attribute.

On the other hand, inheriting it means that there would be
no good way to deprecate the primary without also deprecating
its explicit specializations (because declaring the explicit
specializations would trigger the warning).  The use case
for this was mentioned by Richard in the core discussion
(deprecating the std::numeric_limits primary).

I can't think of any way to make it work.  The only solution
that comes to mind is to use the name of the source file (or
header) in which the primary is defined and allow explicit
specializations to be defined in it while issuing the warning
for those defined in other files.  But this definitely seems
like GCC 9 material.




2) it isn't effective for the nonnull and returns_nonnull
attributes because they are treated as type attributes,
3) the change to deal with attributes on function arguments
may be unnecessary (I couldn't come up with any that would
be propagated from the primary -- are there some?).


Yes, I think this is unnecessary.


Okay, thanks for confirming that.




Before I proceed with it I first want to make sure that it should
be fixed for GCC 8,


Some of it, I think.  Probably not the whole issue.


that duplicate_decls is the right place for these changes


I think so.


and that (2) should be fixed by treating those
and other such attributes by applying them to function decls.


This seems out of bounds for GCC 8.  It would also mean that we couldn't
use such attributes on pointers to functions.


+  TREE_NOTHROW (newdecl) |= TREE_NOTHROW (olddecl);


TREE_NOTHROW is mostly a non-attribute property, so I'd leave it out of
this.


__attribute__ ((nothrow))?  The patch includes a test case with
wrong-code due to inheriting the attribute.  With exception
specifications having to match between the primary and its
specializations it's the only way to make them different.
I've left this unchanged but let me know if I'm missing
something.




+  DECL_IS_MALLOC (olddecl) = DECL_IS_MALLOC (newdecl);


If a template is declared to be malloc, IMO we should really warn if a
specialization is missing that attribute, it seems certain to be a mistake.


I tend to agree that it's likely a mistake.  Though warning
in the front-end will lead to false positives if the function
isn't malloc.  Ideally, this would be detected in the middle-
end (where -Wsuggest-attribute=malloc is handled) but I suspect
it's too late for that.  I've added a simple warning for it.


In general, I think we should (optionally) warn if a template has
attributes and a specialization doesn't, as a user might have been
relying on the current behavior.


I've added a new option, -Wmissing-attribute.  In bug 81824
Joseph asked for such a warning for C (for function resolvers
and aliases) and so I'll use the same option for both (I expect
it's too late to handle 81824 in GCC 8 but I'll finish it in
GCC 9).  Adding the warning required passing some additional
attributes around and so more churn.


+  if (!merge_attr)
+{
+  /* Remove the two function attributes that are, in fact,
+ treated as (quasi) type attributes.  */
+  tree attrs = TYPE_ATTRIBUTES (newtype);
+  tree newattrs = remove_attribute ("nonnull", attrs);
+  newattrs = remove_attribute ("returns_nonnull", attrs);
+  if (newattrs != attrs)
+TYPE_ATTRIBUTES (newtype) = newattrs;
+}


Instead of this, we should avoid calling merge_types and just use
TREE_TYPE (newdecl) for newtype.


Ah, great, thanks.  That works and fixes the outstanding FAILs
in the tests.

Attached is an updated patch.  It 

[PATCH] PR fortran/82049 -- resolve a charlen if possible

[Steve Kargl]

The attached patch fixes PR fortran/82049.  Prior to this
patch, gfortran was fouling up the resolution of the charlen
expression in the testcase.  The immediately tries to resolve
the length while parsing the type-spec.

While here, I've introduced an optimization that causes
gfc_match_type_spec() to return early if the gfortran isn't
going to be getting a type-spec.  This is done be peeking
at the next character, if it is in [a-z], the we don't have
a type spec.  OK to commit?


2018-02-06  Steven G. Kargl  

PR fortran/82049
* match.c (gfc_match_type_spec): If the charlen is non-NULL, then
try to resolve it.  While here return early if possible.

2018-02-06  Steven G. Kargl  

PR fortran/82049
* gfortran.dg/assumed_charlen_parameter.f90: New test.


-- 
Steve
Index: gcc/fortran/match.c
===
--- gcc/fortran/match.c	(revision 257391)
+++ gcc/fortran/match.c	(working copy)
@@ -2054,11 +2054,17 @@ gfc_match_type_spec (gfc_typespec *ts)
 {
   match m;
   locus old_locus;
-  char name[GFC_MAX_SYMBOL_LEN + 1];
+  char c, name[GFC_MAX_SYMBOL_LEN + 1];
 
   gfc_clear_ts (ts);
   gfc_gobble_whitespace ();
   old_locus = gfc_current_locus;
+
+  /* If c isn't [a-z], then return immediately.  */
+  c = gfc_peek_ascii_char ();
+  if (!ISALPHA(c))
+return MATCH_NO;
+
   type_param_spec_list = NULL;
 
   if (match_derived_type_spec (ts) == MATCH_YES)
@@ -2099,6 +2105,8 @@ gfc_match_type_spec (gfc_typespec *ts)
   ts->type = BT_CHARACTER;
 
   m = gfc_match_char_spec (ts);
+  if (ts->u.cl && ts->u.cl->length)
+	gfc_resolve_expr (ts->u.cl->length);
 
   if (m == MATCH_NO)
 	m = MATCH_YES;
Index: gcc/testsuite/gfortran.dg/assumed_charlen_parameter.f90
===
--- gcc/testsuite/gfortran.dg/assumed_charlen_parameter.f90	(nonexistent)
+++ gcc/testsuite/gfortran.dg/assumed_charlen_parameter.f90	(working copy)
@@ -0,0 +1,9 @@
+! { dg-do compile }
+! PR fortran/82049
+! Original code contributed by John Harper 
+program ice ! f2003
+  implicit none
+  character(*), parameter:: a = 'ice', b = '*'
+  character(*), parameter:: c(2) = [character(len(a)) :: a, b]
+  print "(2A4)",adjustr(c)
+end program ice

Go patch committed: Make single Btype for methods table of identical interface types

[Ian Lance Taylor]

This patch to the Go frontend by Cherry Zhang makes a single backend
type for methods table of identical interface types.  Otherwise
identical interface types will get an identical backend type, but can
have different method tables, which is inconsistent and can leave
incomplete placeholdser around.  This patch fixes the problem.
Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
to mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 257415)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-02f11a2d5cf0db2c2675c13d92bb69529f2175dd
+5fe998e4a18cc1dbbd4869be5c8202bda55adb33
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: gcc/go/gofrontend/types.cc
===
--- gcc/go/gofrontend/types.cc  (revision 257415)
+++ gcc/go/gofrontend/types.cc  (working copy)
@@ -9096,6 +9096,8 @@ Interface_type::get_backend_empty_interf
   return empty_interface_type;
 }
 
+Interface_type::Bmethods_map Interface_type::bmethods_map;
+
 // Return a pointer to the backend representation of the method table.
 
 Btype*
@@ -9104,6 +9106,21 @@ Interface_type::get_backend_methods(Gogo
   if (this->bmethods_ != NULL && !this->bmethods_is_placeholder_)
 return this->bmethods_;
 
+  std::pair val;
+  val.first = this;
+  val.second.btype = NULL;
+  val.second.is_placeholder = false;
+  std::pair ins =
+Interface_type::bmethods_map.insert(val);
+  if (!ins.second
+  && ins.first->second.btype != NULL
+  && !ins.first->second.is_placeholder)
+{
+  this->bmethods_ = ins.first->second.btype;
+  this->bmethods_is_placeholder_ = false;
+  return this->bmethods_;
+}
+
   Location loc = this->location();
 
   std::vector
@@ -9160,10 +9177,14 @@ Interface_type::get_backend_methods(Gogo
   Btype* st = gogo->backend()->struct_type(mfields);
   Btype* ret = gogo->backend()->pointer_type(st);
 
-  if (this->bmethods_ != NULL && this->bmethods_is_placeholder_)
-gogo->backend()->set_placeholder_pointer_type(this->bmethods_, ret);
+  if (ins.first->second.btype != NULL
+  && ins.first->second.is_placeholder)
+gogo->backend()->set_placeholder_pointer_type(ins.first->second.btype,
+  ret);
   this->bmethods_ = ret;
+  ins.first->second.btype = ret;
   this->bmethods_is_placeholder_ = false;
+  ins.first->second.is_placeholder = false;
   return ret;
 }
 
@@ -9174,10 +9195,25 @@ Interface_type::get_backend_methods_plac
 {
   if (this->bmethods_ == NULL)
 {
+  std::pair val;
+  val.first = this;
+  val.second.btype = NULL;
+  val.second.is_placeholder = false;
+  std::pair ins =
+Interface_type::bmethods_map.insert(val);
+  if (!ins.second && ins.first->second.btype != NULL)
+{
+  this->bmethods_ = ins.first->second.btype;
+  this->bmethods_is_placeholder_ = ins.first->second.is_placeholder;
+  return this->bmethods_;
+}
+
   Location loc = this->location();
-  this->bmethods_ = gogo->backend()->placeholder_pointer_type("", loc,
- false);
+  Btype* bt = gogo->backend()->placeholder_pointer_type("", loc, false);
+  this->bmethods_ = bt;
+  ins.first->second.btype = bt;
   this->bmethods_is_placeholder_ = true;
+  ins.first->second.is_placeholder = true;
 }
   return this->bmethods_;
 }
Index: gcc/go/gofrontend/types.h
===
--- gcc/go/gofrontend/types.h   (revision 257415)
+++ gcc/go/gofrontend/types.h   (working copy)
@@ -3185,6 +3185,20 @@ class Interface_type : public Type
   bool
   assume_identical(const Interface_type*, const Interface_type*) const;
 
+  struct Bmethods_map_entry
+  {
+Btype *btype;
+bool is_placeholder;
+  };
+
+  // A mapping from Interface_type to the backend type of its bmethods_,
+  // used to ensure that the backend representation of identical types
+  // is identical.
+  typedef Unordered_map_hash(const Interface_type*, Bmethods_map_entry,
+ Type_hash_identical, Type_identical) Bmethods_map;
+
+  static Bmethods_map bmethods_map;
+
   // The list of methods associated with the interface from the
   // parser.  This will be NULL for the empty interface.  This may
   // include unnamed interface types.

Re: [PATCH] PR libgcc/59714 complex division is surprising on aarch64

[Joseph Myers]

On Tue, 6 Feb 2018, vladimir.mezent...@oracle.com wrote:

> We see 'struct _FP_STRUCT_LAYOUT' is declared twice (in lines 206 and

_FP_STRUCT_LAYOUT is not a struct tag.  It is a macro that may be defined 
to empty, or to an attribute, and defaults to empty in soft-fp.h if not 
otherwise defined in sfp-machine.h (for example, the x86 sfp-machine.h 
defines it for MinGW where default struct layout would otherwise be 
inappropriate).

If you're seeing it used as a struct tag, that means you're missing an 
inclusion of soft-fp.h which is required to be able to include the other 
soft-fp headers.  But really I suspect that using soft-fp headers is a 
mistake here, because of the dependence on per-machine sfp-machine.h that 
should generally be irrelevant for your purposes (beyond having some way 
to declare appropriate struct layouts).

> P.S.:
> gcc/libgcc/soft-fp/ and  glibc/soft-fp/ are not synchronized now. For
> example:

It's not necessary to update after every change in glibc if that change is 
not relevant to GCC.  But if any change is required in GCC it should be 
made first in glibc, and then all files updated from the versions in 
glibc.

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

Re: [PATCH] PR libgcc/59714 complex division is surprising on aarch64

[vladimir . mezentsev]

On 02/06/2018 08:53 AM, Joseph Myers wrote:
> The files in libgcc/soft-fp must be verbatim copies of the master sources
> in glibc.  So you can't make any local changes to them, and if you think 
> changes are needed you need to patch things upstream in glibc first, with 
> a proper extended explanation of why the fix is needed and why it is safe 
> and the appropriate design for the fix.  There's nothing at all in this 
> patch submission to explain that change.

It is a bug/typo  in gcc/libgcc/soft-fp/quad.h when _FP_W_TYPE_SIZE is
64 (for example, on aarch64) .
It looks like a code after line 202 was never used.
% cat -n libgcc/soft-fp/quad.h
...
   201   
   202    #else   /* not _FP_W_TYPE_SIZE < 64 */
   203    union _FP_UNION_Q
   204    {
   205      TFtype flt /* __attribute__ ((mode (TF))) */ ;
   206      struct *_FP_STRUCT_LAYOUT*
   207      {
   208        _FP_W_TYPE a, b;
   209      } longs;
   210      struct *_FP_STRUCT_LAYOUT*
   211      {
   212    # if __BYTE_ORDER == __BIG_ENDIAN
   213        unsigned sign    : 1;
   214        unsigned exp : _FP_EXPBITS_Q;
   215        _FP_W_TYPE frac1 : _FP_FRACBITS_Q - (_FP_IMPLBIT_Q != 0) -
_FP_W_TYPE_SIZE;
   216        _FP_W_TYPE frac0 : _FP_W_TYPE_SIZE;
   217    # else
   218        _FP_W_TYPE frac0 : _FP_W_TYPE_SIZE;
   219        _FP_W_TYPE frac1 : _FP_FRACBITS_Q - (_FP_IMPLBIT_Q != 0) -
_FP_W_TYPE_SIZE;
   220        unsigned exp : _FP_EXPBITS_Q;
   221        unsigned sign    : 1;
   222    # endif
   223      } bits;
   224    };

We see 'struct _FP_STRUCT_LAYOUT' is declared twice (in lines 206 and
210) inside union _FP_UNION_Q.
Compiler reports warning.


-Vladimir

P.S.:
gcc/libgcc/soft-fp/ and  glibc/soft-fp/ are not synchronized now. For
example:

% diff gcc/libgcc/soft-fp/quad.h  glibc/soft-fp/
3c3
<    Copyright (C) 1997-2016 Free Software Foundation, Inc.
---
>    Copyright (C) 1997-2018 Free Software Foundation, Inc.
96c96
<   } bits __attribute__ ((packed));
---
>   } bits;

Re: PR 84154: Fix checking -mibt and -mshstk options for control flow protection

[Paolo Carlini]

Hi,

On 07/02/2018 01:02, Tsimbalist, Igor V wrote:
The issue is known and is covered by 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84248. The patch has been 
posted

https://gcc.gnu.org/ml/gcc-patches/2018-02/msg00276.html

Ok, thanks, I missed the above.

Paolo.

RE: PR 84154: Fix checking -mibt and -mshstk options for control flow protection

[Tsimbalist, Igor V]

> -Original Message-
> From: Paolo Carlini [mailto:paolo.carl...@oracle.com]
> Sent: Wednesday, February 7, 2018 12:46 AM
> To: Tsimbalist, Igor V ; gcc-
> patc...@gcc.gnu.org
> Cc: Nick Clifton ; hjl.to...@gmail.com; Uros Bizjak
> 
> Subject: Re: PR 84154: Fix checking -mibt and -mshstk options for control
> flow protection
> 
> Hi,
> 
> on a rather old x86_64-linux machine GCC doesn't build anymore with
> r257414:
> 
> libtool: compile:  /xxx/Gcc/svn-dirs/trunk-build/./gcc/xg++
> -B/xxx/Gcc/svn-dirs/trunk-build/./gcc/ -nostdinc++ -nostdinc++
> -I/xxx/Gcc/svn-dirs/trunk-build/x86_64-pc-linux-gnu/libstdc++-
> v3/include/x86_64-pc-linux-gnu
> -I/xxx/Gcc/svn-dirs/trunk-build/x86_64-pc-linux-gnu/libstdc++-v3/include
> -I/xxx/Gcc/svn-dirs/trunk/libstdc++-v3/libsupc++
> -I/xxx/Gcc/svn-dirs/trunk/libstdc++-v3/include/backward
> -I/xxx/Gcc/svn-dirs/trunk/libstdc++-v3/testsuite/util
> -L/xxx/Gcc/svn-dirs/trunk-build/x86_64-pc-linux-gnu/libstdc++-v3/src
> -L/xxx/Gcc/svn-dirs/trunk-build/x86_64-pc-linux-gnu/libstdc++-v3/src/.libs
> -L/xxx/Gcc/svn-dirs/trunk-build/x86_64-pc-linux-gnu/libstdc++-
> v3/libsupc++/.libs
> -B/xxx/Gcc/svn-dirs/trunk-build/x86_64-pc-linux-gnu/libstdc++-v3/src/.libs
> -B/xxx/Gcc/svn-dirs/trunk-build/x86_64-pc-linux-gnu/libstdc++-
> v3/libsupc++/.libs
> -B/xxx/Gcc/svn-dirs/trunk-install/x86_64-pc-linux-gnu/bin/
> -B/xxx/Gcc/svn-dirs/trunk-install/x86_64-pc-linux-gnu/lib/ -isystem
> /xxx/Gcc/svn-dirs/trunk-install/x86_64-pc-linux-gnu/include -isystem
> /xxx/Gcc/svn-dirs/trunk-install/x86_64-pc-linux-gnu/sys-include
> -DHAVE_CONFIG_H -I. -I../../../trunk/libitm
> -I../../../trunk/libitm/config/linux/x86
> -I../../../trunk/libitm/config/linux -I../../../trunk/libitm/config/x86
> -I../../../trunk/libitm/config/posix
> -I../../../trunk/libitm/config/generic -I../../../trunk/libitm -mrtm
> -Wall -pthread -Werror -fcf-protection -mcet -std=gnu++0x
> -funwind-tables -fno-exceptions -fno-rtti -fabi-version=4 -g -O2
> -D_GNU_SOURCE -MT beginend.lo -MD -MP -MF .deps/beginend.Tpo -c
> ../../../trunk/libitm/beginend.cc  -fPIC -DPIC -o .libs/beginend.o
> In file included from
> /xxx/Gcc/svn-dirs/trunk-build/gcc/include/x86intrin.h:27,
>   from ../../../trunk/libitm/config/x86/target.h:26,
>   from ../../../trunk/libitm/libitm_i.h:74,
>   from ../../../trunk/libitm/barrier.cc:25:
> /xxx/Gcc/svn-dirs/trunk-build/gcc/include/ia32intrin.h:56:28: internal
> compiler error: in ix86_option_override_internal, at config/i386/i386.c:4952
>   #pragma GCC target("sse4.2")

The issue is known and is covered by 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84248. The patch has been posted

https://gcc.gnu.org/ml/gcc-patches/2018-02/msg00276.html

Igor

> Paolo.

Re: PR 84154: Fix checking -mibt and -mshstk options for control flow protection

[Paolo Carlini]

Hi,

on a rather old x86_64-linux machine GCC doesn't build anymore with r257414:

libtool: compile:  /xxx/Gcc/svn-dirs/trunk-build/./gcc/xg++ 
-B/xxx/Gcc/svn-dirs/trunk-build/./gcc/ -nostdinc++ -nostdinc++ 
-I/xxx/Gcc/svn-dirs/trunk-build/x86_64-pc-linux-gnu/libstdc++-v3/include/x86_64-pc-linux-gnu 
-I/xxx/Gcc/svn-dirs/trunk-build/x86_64-pc-linux-gnu/libstdc++-v3/include 
-I/xxx/Gcc/svn-dirs/trunk/libstdc++-v3/libsupc++ 
-I/xxx/Gcc/svn-dirs/trunk/libstdc++-v3/include/backward 
-I/xxx/Gcc/svn-dirs/trunk/libstdc++-v3/testsuite/util 
-L/xxx/Gcc/svn-dirs/trunk-build/x86_64-pc-linux-gnu/libstdc++-v3/src 
-L/xxx/Gcc/svn-dirs/trunk-build/x86_64-pc-linux-gnu/libstdc++-v3/src/.libs 
-L/xxx/Gcc/svn-dirs/trunk-build/x86_64-pc-linux-gnu/libstdc++-v3/libsupc++/.libs 
-B/xxx/Gcc/svn-dirs/trunk-build/x86_64-pc-linux-gnu/libstdc++-v3/src/.libs 
-B/xxx/Gcc/svn-dirs/trunk-build/x86_64-pc-linux-gnu/libstdc++-v3/libsupc++/.libs 
-B/xxx/Gcc/svn-dirs/trunk-install/x86_64-pc-linux-gnu/bin/ 
-B/xxx/Gcc/svn-dirs/trunk-install/x86_64-pc-linux-gnu/lib/ -isystem 
/xxx/Gcc/svn-dirs/trunk-install/x86_64-pc-linux-gnu/include -isystem 
/xxx/Gcc/svn-dirs/trunk-install/x86_64-pc-linux-gnu/sys-include 
-DHAVE_CONFIG_H -I. -I../../../trunk/libitm 
-I../../../trunk/libitm/config/linux/x86 
-I../../../trunk/libitm/config/linux -I../../../trunk/libitm/config/x86 
-I../../../trunk/libitm/config/posix 
-I../../../trunk/libitm/config/generic -I../../../trunk/libitm -mrtm 
-Wall -pthread -Werror -fcf-protection -mcet -std=gnu++0x 
-funwind-tables -fno-exceptions -fno-rtti -fabi-version=4 -g -O2 
-D_GNU_SOURCE -MT beginend.lo -MD -MP -MF .deps/beginend.Tpo -c 
../../../trunk/libitm/beginend.cc  -fPIC -DPIC -o .libs/beginend.o
In file included from 
/xxx/Gcc/svn-dirs/trunk-build/gcc/include/x86intrin.h:27,

 from ../../../trunk/libitm/config/x86/target.h:26,
 from ../../../trunk/libitm/libitm_i.h:74,
 from ../../../trunk/libitm/barrier.cc:25:
/xxx/Gcc/svn-dirs/trunk-build/gcc/include/ia32intrin.h:56:28: internal 
compiler error: in ix86_option_override_internal, at config/i386/i386.c:4952

 #pragma GCC target("sse4.2")

Paolo.

[PATCH] Small -ftrapv improvement

[Jakub Jelinek]

Hi!

As mentioned on IRC, operation_could_trap_helper_p returns true
for division or modulo with -ftrapv; the operations could trap in certain
cases (e.g. division by -1 of minimum signed value, but we don't have any
library functions for division/modulo for -ftrapv nor instrument it in any
way).  The following just makes it match what we implement.

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

2018-02-06  Jakub Jelinek  

* tree-eh.c (operation_could_trap_helper_p): Ignore honor_trapv for
*DIV_EXPR and *MOD_EXPR.

--- gcc/tree-eh.c.jj2018-02-01 11:07:24.0 +0100
+++ gcc/tree-eh.c   2018-02-03 17:52:40.57329 +0100
@@ -2436,7 +2436,7 @@ operation_could_trap_helper_p (enum tree
 case ROUND_MOD_EXPR:
 case TRUNC_MOD_EXPR:
 case RDIV_EXPR:
-  if (honor_snans || honor_trapv)
+  if (honor_snans)
return true;
   if (fp_operation)
return flag_trapping_math;

Jakub

RE: PR 84154: Fix checking -mibt and -mshstk options for control flow protection

[Tsimbalist, Igor V]

> -Original Message-
> From: Rainer Orth [mailto:r...@cebitec.uni-bielefeld.de]
> Sent: Tuesday, February 6, 2018 11:50 PM
> To: Tsimbalist, Igor V 
> Cc: gcc-patches@gcc.gnu.org; Nick Clifton ;
> hjl.to...@gmail.com; Uros Bizjak 
> Subject: Re: PR 84154: Fix checking -mibt and -mshstk options for control
> flow protection
> 
> Hi Igor,
> 
> > Here is the updated patch. Please note the subject should say PR 84145.
> 
> the two new testcases FAIL on all non-x86 targets (I've seen that on
> sparc-sun-solaris2.11, there's a gcc-testresults posting for
> powerpc64le-unknown-linux-gnu, and PR testsuite/84243 reports it for
> aarch64-none-linux-gnu:
> 
> +FAIL: c-c++-common/fcf-protection-6.c  -std=gnu++11  (test for errors,
> line )
> +FAIL: c-c++-common/fcf-protection-6.c  -std=gnu++11 (test for excess
> errors)
> +FAIL: c-c++-common/fcf-protection-6.c  -std=gnu++14  (test for errors,
> line )
> +FAIL: c-c++-common/fcf-protection-6.c  -std=gnu++14 (test for excess
> errors)
> +FAIL: c-c++-common/fcf-protection-6.c  -std=gnu++98  (test for errors,
> line )
> +FAIL: c-c++-common/fcf-protection-6.c  -std=gnu++98 (test for excess
> errors)
> 
> Excess errors:
> xg++: error: unrecognized command line option '-mshstk'
> 
> +FAIL: c-c++-common/fcf-protection-7.c  -std=gnu++11  (test for errors,
> line )
> +FAIL: c-c++-common/fcf-protection-7.c  -std=gnu++11 (test for excess
> errors)
> +FAIL: c-c++-common/fcf-protection-7.c  -std=gnu++14  (test for errors,
> line )
> +FAIL: c-c++-common/fcf-protection-7.c  -std=gnu++14 (test for excess
> errors)
> +FAIL: c-c++-common/fcf-protection-7.c  -std=gnu++98  (test for errors,
> line )
> +FAIL: c-c++-common/fcf-protection-7.c  -std=gnu++98 (test for excess
> errors)
> 
> Excess errors:
> xg++: error: unrecognized command line option '-mibt'
> 
> I think the right way to handle that is to pass -mshstk resp. -mibt on
> x86 only.  The following patch does this; tested with the appropriate
> runtest invocation on i386-pc-solaris2.11 and sparc-sun-solaris2.11.
> 
> Ok for mainline?

Agree with the fix. Thanks for taking care of this issue.

Igor

>   Rainer
> 
> --
> -
> Rainer Orth, Center for Biotechnology, Bielefeld University
> 
> 
> 2018-02-06  Rainer Orth  
> 
>   PR testsuite/84243
>   * c-c++-common/fcf-protection-6.c: Only pass -mshstk on x86
>   targets.
>   * c-c++-common/fcf-protection-7.c: Likewise for -mibt.

Re: [PATCH] RL78 new "vector" function attribute

[DJ Delorie]

Sebastian Perta  writes:
> I've updated the patch (extend.texi) as you suggested.
> Please let me know if this is OK to check-in, thank you!

Looks OK to me, but wait a day or two for a docs person to comment on...

> -On RX targets, you may specify one or more vector numbers as arguments
> +On RX and RL78 targets, you may specify one or more vector numbers as 
> arguments

...if the new line is too long and if a paragraph reformat is warranted.

Thanks!

Re: PR 84154: Fix checking -mibt and -mshstk options for control flow protection

[Rainer Orth]

Hi Igor,

> Here is the updated patch. Please note the subject should say PR 84145.

the two new testcases FAIL on all non-x86 targets (I've seen that on
sparc-sun-solaris2.11, there's a gcc-testresults posting for
powerpc64le-unknown-linux-gnu, and PR testsuite/84243 reports it for
aarch64-none-linux-gnu:

+FAIL: c-c++-common/fcf-protection-6.c  -std=gnu++11  (test for errors, line )
+FAIL: c-c++-common/fcf-protection-6.c  -std=gnu++11 (test for excess errors)
+FAIL: c-c++-common/fcf-protection-6.c  -std=gnu++14  (test for errors, line )
+FAIL: c-c++-common/fcf-protection-6.c  -std=gnu++14 (test for excess errors)
+FAIL: c-c++-common/fcf-protection-6.c  -std=gnu++98  (test for errors, line )
+FAIL: c-c++-common/fcf-protection-6.c  -std=gnu++98 (test for excess errors)

Excess errors:
xg++: error: unrecognized command line option '-mshstk'

+FAIL: c-c++-common/fcf-protection-7.c  -std=gnu++11  (test for errors, line )
+FAIL: c-c++-common/fcf-protection-7.c  -std=gnu++11 (test for excess errors)
+FAIL: c-c++-common/fcf-protection-7.c  -std=gnu++14  (test for errors, line )
+FAIL: c-c++-common/fcf-protection-7.c  -std=gnu++14 (test for excess errors)
+FAIL: c-c++-common/fcf-protection-7.c  -std=gnu++98  (test for errors, line )
+FAIL: c-c++-common/fcf-protection-7.c  -std=gnu++98 (test for excess errors)

Excess errors:
xg++: error: unrecognized command line option '-mibt'

I think the right way to handle that is to pass -mshstk resp. -mibt on
x86 only.  The following patch does this; tested with the appropriate
runtest invocation on i386-pc-solaris2.11 and sparc-sun-solaris2.11.

Ok for mainline?

Rainer

-- 
-
Rainer Orth, Center for Biotechnology, Bielefeld University


2018-02-06  Rainer Orth  

PR testsuite/84243
* c-c++-common/fcf-protection-6.c: Only pass -mshstk on x86
targets.
* c-c++-common/fcf-protection-7.c: Likewise for -mibt.

# HG changeset patch
# Parent  bc1af87b4176f6f5ddc900980a18df826cbf4553
Don't pass x86-only options on non-x86 targets in c-c++-common/fcf-protection-[67].c

diff --git a/gcc/testsuite/c-c++-common/fcf-protection-6.c b/gcc/testsuite/c-c++-common/fcf-protection-6.c
--- a/gcc/testsuite/c-c++-common/fcf-protection-6.c
+++ b/gcc/testsuite/c-c++-common/fcf-protection-6.c
@@ -1,4 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-fcf-protection=branch -mshstk" } */
+/* { dg-options "-fcf-protection=branch" } */
+/* { dg-additional-options "-mshstk" { target { i?86-*-* x86_64-*-* } } } */
 /* { dg-error "'-fcf-protection=branch' requires Intel CET.*-mcet or -mibt option" "" { target { "i?86-*-* x86_64-*-*" } } 0 } */
 /* { dg-error "'-fcf-protection=branch' is not supported for this target" "" { target { ! "i?86-*-* x86_64-*-*" } } 0 } */
diff --git a/gcc/testsuite/c-c++-common/fcf-protection-7.c b/gcc/testsuite/c-c++-common/fcf-protection-7.c
--- a/gcc/testsuite/c-c++-common/fcf-protection-7.c
+++ b/gcc/testsuite/c-c++-common/fcf-protection-7.c
@@ -1,4 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-fcf-protection=return -mibt" } */
+/* { dg-options "-fcf-protection=return" } */
+/* { dg-additional-options "-mibt" { target { i?86-*-* x86_64-*-* } } } */
 /* { dg-error "'-fcf-protection=return' requires Intel CET.*-mcet or -mshstk option" "" { target { "i?86-*-* x86_64-*-*" } } 0 } */
 /* { dg-error "'-fcf-protection=return' is not supported for this target" "" { target { ! "i?86-*-* x86_64-*-*" } } 0 } */

[og7, committed] Fix implicit fallthrough warning in expand_omp_target

[Tom de Vries]

[ was: Re: [PATCH,WIP] Use functional parameters for data mappings in 
OpenACC child functions ]


On 02/06/2018 12:46 PM, Tom de Vries wrote:

On 12/21/2017 10:46 PM, Cesar Philippidis wrote:


I've committed this patch to openacc-gcc-7-branch.



diff --git a/gcc/omp-expand.c b/gcc/omp-expand.c
index bf1f127d8d6..f674c74ec82 100644
--- a/gcc/omp-expand.c
+++ b/gcc/omp-expand.c



    offloaded = is_gimple_omp_offloaded (entry_stmt);
    switch (gimple_omp_target_kind (entry_stmt))
  {
+    case GF_OMP_TARGET_KIND_OACC_PARALLEL:
+  oacc_parallel = true;
  case GF_OMP_TARGET_KIND_REGION:
  case GF_OMP_TARGET_KIND_UPDATE:
  case GF_OMP_TARGET_KIND_ENTER_DATA:
  case GF_OMP_TARGET_KIND_EXIT_DATA:
-    case GF_OMP_TARGET_KIND_OACC_PARALLEL:
  case GF_OMP_TARGET_KIND_OACC_KERNELS:
  case GF_OMP_TARGET_KIND_OACC_UPDATE:
  case GF_OMP_TARGET_KIND_OACC_ENTER_EXIT_DATA:


This broke openacc-gcc-7-branch bootstrap:
...
gcc/omp-expand.c: In function 'void expand_omp_target(omp_region*)':
gcc/omp-expand.c:7110:21: error: this statement may fall through 
[-Werror=implicit-fallthrough=]

    oacc_parallel = true;
    ~~^~
gcc/omp-expand.c:7111:5: note: here
  case GF_OMP_TARGET_KIND_REGION:
  ^~~~
...


Fixed in attached patch. Committed.

Thanks,
- Tom
Fix implicit fallthrough warning in expand_omp_target

2018-02-06  Tom de Vries  

	* omp-expand.c (expand_omp_target): Fix implicit fallthrough warning.

---
 gcc/omp-expand.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gcc/omp-expand.c b/gcc/omp-expand.c
index f674c74..d6ddf6e 100644
--- a/gcc/omp-expand.c
+++ b/gcc/omp-expand.c
@@ -7108,6 +7108,7 @@ expand_omp_target (struct omp_region *region)
 {
 case GF_OMP_TARGET_KIND_OACC_PARALLEL:
   oacc_parallel = true;
+  gcc_fallthrough ();
 case GF_OMP_TARGET_KIND_REGION:
 case GF_OMP_TARGET_KIND_UPDATE:
 case GF_OMP_TARGET_KIND_ENTER_DATA:

Re: [C++ PATCH] Fix ICEs due to cp_parser_postfix_dot_deref_expression workaround (PR c++/84082, take 2)

[Jason Merrill]

On Tue, Feb 6, 2018 at 3:28 PM, Jakub Jelinek  wrote:
> On Fri, Feb 02, 2018 at 03:13:21PM -0500, Jason Merrill wrote:
>> > Or should I change the switch body to do other kind = overrides?
>>
>> I think we want to avoid clobbering NON_LVALUE_EXPR location wrappers, too.
>>
>> This has gotten large enough that it should break out into its own function.
>>
>> OK with those changes.
>
> So like this?  Bootstrapped/regtested on x86_64-linux and i686-linux.
>
> +case NON_LVALUE_EXPR:
> +  if (location_wrapper_p (*postfix_expression))

I don't think we need this test, as NON_LVALUE_EXPR should only appear
as a location wrapper.  OK with it removed.

Jason

Re: [patch, libfortran] Use flexible array members for array descriptor

[Thomas Koenig]

Am 06.02.2018 um 09:53 schrieb Janne Blomqvist:

 /* Make the INTEGER*4 array for passing to date_and_time.  */
-  gfc_array_i4 *avalues = xmalloc (sizeof (gfc_array_i4));
+  gfc_array_i4 *avalues = xmalloc (sizeof (gfc_full_array_i4));


Since date_and_time requires the values array to always be rank 1,
can't this be "xmalloc (sizeof (gfc_array_i4) +
sizeof(dimension_data))" ?


I think we can be fairly sure that this would be OK at the moment, since
(I think) there are no gaps in our descriptors at the moment. Anybody
invents an architecture where this is not the case, we introduce
a bug. This way is safer.

According to the C standard (C11 6.7.2.1.18 and example 6.7.2.1.20),
this is guaranteed to work.


OK, I didn't know that, and I see how that would save memory.
I'll make that change (with a comment) if this patch ends up being
committed.

Regards

Thomas

Re: [PATCH] PowerPC PR target/84154, fix floating point to small integer conversion regression

[Michael Meissner]

On Tue, Feb 06, 2018 at 11:15:21AM -0600, Segher Boessenkool wrote:
> Hi!
> 
> On Mon, Feb 05, 2018 at 05:57:51PM -0500, Michael Meissner wrote:
> > > We have
> > > 
> > > (define_code_attr su [(sign_extend  "s")
> > >   (zero_extend  "u")
> > >   (fix  "s")
> > >   (unsigned_fix "s")
> > >   (float"s")
> > >   (unsigned_float   "u")])
> > > 
> > > and "s" for unsigned_fix seems like it should be "u".  Very surprising
> > > otherwise (if this is needed in some case, it should just write "s" there
> > > instead of "").
> 
> What about this?

As we discussed before, this was a bug, and I've already committed the fix
separately.

> > I found as long as I avoid putting the  or  in the output template
> > (i.e. use an output statement instead of a template) it works.  It only
> > seems to fail in the IEEE128 case, and not in the SFDF case.  I will submit 
> > a
> > bug report  on it after this gets checked in, as it will be simpler to 
> > provide
> > a patch that people can test.
> 
> Thanks.
> 
> > +(define_insn_and_split "fix_trunc2"
> > +  [(set (match_operand: 0 "gpc_reg_operand" "=wJ,wJwK,r")
> > +   (any_fix:QHI (match_operand:SFDF 1 "gpc_reg_operand" "wJ,wJwK,wa")))
> > +   (clobber (match_scratch:SI 2 "=X,X,wi"))]
> > +  "TARGET_DIRECT_MOVE"
> > +  "@
> > +   fctiwz %0,%1
> > +   xscvdpxws %x0,%x1
> 
> This one cannot work with the  definition above, for example.
> 
> > +(define_insn "fix_2_hw"
> > +  [(set (match_operand:SDI 0 "altivec_register_operand" "=v")
> > +   (any_fix:SDI (match_operand:IEEE128 1 "altivec_register_operand" "v")))]
> > +  "TARGET_FLOAT128_HW && FLOAT128_IEEE_P (mode)"
> > +{
> > +  return ( == UNSIGNED_FIX) ? "xscvqpuz %0,%1" : "xscvqpsz 
> > %0,%1";
> > +}
> 
> And it may well be why you need this.
> 
> Rest looks good :-)

Here is the patch reworked.  It bootstraps on both little/big endian power8,
and all of the tests run.  Can I install this into trunk now, and into GCC 7
after a soak period (along with the previous patch)?

[gcc]
2018-02-06  Michael Meissner  

PR target/84154
* config/rs6000/rs6000.md (fix_trunc2):
Convert from define_expand to be define_insn_and_split.  Rework
float/double/_Float128 conversions to QI/HI/SImode to work with
both ISA 2.07 (power8) or ISA 3.0 (power9).  Fix regression where
conversions to QI/HImode types did a store and then a load to
truncate the value.  For conversions to VSX registers, don't split
the insn, instead emit the code directly.  Use the code iterator
any_fix to combine signed and unsigned conversions.
(fix_truncsi2_p8): Likewise.
(fixuns_trunc2): Likewise.
(fix_trunc2): Likewise.
(fix_trunc2): Likewise.
(fix_di2_hw): Likewise.
(fixuns_di2_hw): Likewise.
(fix_si2_hw): Likewise.
(fixuns_si2_hw): Likewise.
(fix_2_hw): Likewise.
(fix_trunc2): Likewise.
(fctiwz__smallint): Rename fctiwz__smallint to
fix_truncsi2_p8.
(fix_trunc2_internal): Delete, no longer
used.
(fixuns_trunc2_internal): Likewise.
(fix__mem): Likewise.
(fctiwz__mem): Likewise.
(fix__mem): Likewise.
(fix_trunc2_mem): On ISA 3.0, prevent
the register allocator from doing a direct move to the GPRs to do
a store, and instead use the ISA 3.0 store byte/half-word from
vector register instruction.  For IEEE 128-bit floating point,
also optimize stores of 32-bit ints.
(fix_trunc2_mem): Likewise.

[gcc/testsuite]
2018-02-06  Michael Meissner  

PR target/84154
* gcc.target/powerpc/pr84154-1.c: New tests.
* gcc.target/powerpc/pr84154-2.c: Likewise.
* gcc.target/powerpc/pr84154-3.c: Likewise.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.vnet.ibm.com, phone: +1 (978) 899-4797
Index: gcc/config/rs6000/rs6000.md
===
--- gcc/config/rs6000/rs6000.md (revision 257269)
+++ gcc/config/rs6000/rs6000.md (working copy)
@@ -5700,43 +5700,60 @@ (define_insn "*fix_truncdi2_fctidz
xscvdpsxds %x0,%x1"
   [(set_attr "type" "fp")])
 
-(define_expand "fix_trunc2"
-  [(parallel [(set (match_operand: 0 "nonimmediate_operand")
-  (fix:QHI (match_operand:SFDF 1 "gpc_reg_operand")))
- (clobber (match_scratch:DI 2))])]
-  "TARGET_P9_VECTOR && TARGET_DIRECT_MOVE_64BIT"
+;; If have ISA 3.0, QI/HImode values can go in both VSX registers and GPR
+;; registers.  If we have ISA 2.07, we don't allow QI/HImode values in the
+;; vector registers, so we need to do direct moves to the GPRs, but SImode
+;; values can go in VSX registers.  Keeping the direct move part 

Re: wwwdocs: Some release notes for powerpc for GCC 8

[Gerald Pfeifer]

On Thu, 1 Feb 2018, Segher Boessenkool wrote:
>> Just the move ofseems 
>> unclear to me?  Can you omit that?
> The entries are alphabetic, except that one.  Should have put it in
> a separate patch, sorry.

Actually it was fine as I just realize.

I got confused by hppa and nvptx (which is sorted properly) vs
PA-RISC and NVPTX (which is not).  Obviously we should sort by
what folks see, not the HTML IDs.  Sorry about the confusion.

Gerald

RE: [PATCH] i386: Mask out the CF_SET bit for -fcf-protection check

[Tsimbalist, Igor V]

> -Original Message-
> From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-
> ow...@gcc.gnu.org] On Behalf Of H.J. Lu
> Sent: Tuesday, February 6, 2018 10:09 PM
> To: gcc-patches@gcc.gnu.org
> Cc: Uros Bizjak ; Tsimbalist, Igor V
> 
> Subject: [PATCH] i386: Mask out the CF_SET bit for -fcf-protection check
> 
> Since ix86_option_override_internal sets the CF_SET bit in
> flag_cf_protection and it can be called more than once via pragma,
> we need to mask out the CF_SET bit when checking flag_cf_protection.
> 
> OK for trunk if there is no regression?

Ok from CET viewpoint.

Thanks,
Igor

> H.J.
> ---
>   PR target/84248
>   * config/i386/i386.c (ix86_option_override_internal): Mask out
>   the CF_SET bit when checking -fcf-protection.
> ---
>  gcc/config/i386/i386.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index 6c612c77987..ef7ff89bcbb 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -4913,12 +4913,12 @@ ix86_option_override_internal (bool
> main_args_p,
>= build_target_option_node (opts);
> 
>/* Do not support control flow instrumentation if CET is not enabled.  */
> -  if (opts->x_flag_cf_protection != CF_NONE)
> +  cf_protection_level cf_protection
> += (cf_protection_level) (opts->x_flag_cf_protection & ~CF_SET);
> +  if (cf_protection != CF_NONE)
>  {
> -  switch (flag_cf_protection)
> +  switch (cf_protection)
>   {
> - case CF_NONE:
> -   break;
>   case CF_BRANCH:
> if (! TARGET_IBT_P (opts->x_ix86_isa_flags2))
>   {
> @@ -4953,7 +4953,7 @@ ix86_option_override_internal (bool main_args_p,
>   }
> 
>opts->x_flag_cf_protection =
> - (cf_protection_level) (opts->x_flag_cf_protection | CF_SET);
> + (cf_protection_level) (cf_protection | CF_SET);
>  }
> 
>if (ix86_tune_features [X86_TUNE_AVOID_128FMA_CHAINS])
> --
> 2.14.3

RE: [PATCH] Use -fcf-protection=return in cet-intrin-4.c

[Tsimbalist, Igor V]

> -Original Message-
> From: Lu, Hongjiu
> Sent: Tuesday, February 6, 2018 10:03 PM
> To: gcc-patches@gcc.gnu.org
> Cc: Uros Bizjak ; Tsimbalist, Igor V
> 
> Subject: [PATCH] Use -fcf-protection=return in cet-intrin-4.c
> 
> Since -fcf-protection requires both -mshstk and -mibt, use
> -fcf-protection=return with -mshstk in cet-intrin-4.c.
> 
> OK for trunk?

Ok from CET viewpoint.

Igor

> H.J.
> --
>   PR target/84243
>   * gcc.target/i386/cet-intrin-4.c (dg-options): Use
>   -fcf-protection=return.
> ---
>  gcc/testsuite/gcc.target/i386/cet-intrin-4.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gcc/testsuite/gcc.target/i386/cet-intrin-4.c
> b/gcc/testsuite/gcc.target/i386/cet-intrin-4.c
> index 76ec160543f..437a4cd690c 100644
> --- a/gcc/testsuite/gcc.target/i386/cet-intrin-4.c
> +++ b/gcc/testsuite/gcc.target/i386/cet-intrin-4.c
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O -fcf-protection -mshstk" } */
> +/* { dg-options "-O -fcf-protection=return -mshstk" } */
>  /* { dg-final { scan-assembler "rdsspd|incsspd\[ \t]+(%|)eax" { target ia32 
> } } }
> */
>  /* { dg-final { scan-assembler "rdssp\[dq]\[ \t]+(%|)\[re]ax"  { target { ! 
> ia32 } }
> } } */
>  /* { dg-final { scan-assembler "incssp\[dq]\[ \t]+(%|)\[re]di" { target { ! 
> ia32 } }
> } } */
> --
> 2.14.3

Re: [SFN+LVU+IEPM v4 7/9] [LVU] Introduce location views

[Jason Merrill]

On 12/11/2017 09:52 PM, Alexandre Oliva wrote:

+/* output symbol LAB1 as an unsigned LEB128 quantity.  */
+
+void
+dw2_asm_output_symname_uleb128 (const char *lab1 ATTRIBUTE_UNUSED,
+   const char *comment, ...)


I'm having trouble understanding the use of symbols for views.  The 
configure test for gas support doesn't obviously define a symbol 
anywhere; is it implicitly defined by its use in the location directive? 
 This could use more documentation in dwarf2out.



+   view computation, and it is refers to a view identifier for which


"it refers"

Why do we need to use a non-zero view identifier for a zero view?  Why 
can't we always use 0 instead of the bitmap?



  Rationale: location lists can refer to address and view, not
  op_index, so views are reset at address changes, not at op_index
  changes.  Opcodes that advance op_index only will only reset the
  view when they happen to advance the address, e.g. by exceeding
  maximum_operations_per_instruction in op_index.

  DW_LNS_fixed_advance_pc is the only opcode that may change the
  address without resetting the view.  It is available for compilers
  to use when an address change is uncertain, e.g., when advancing
  past opcodes that may expand to an empty sequence,
  e.g. possibly-empty alignment sequences, optional no-operation
  opcodes or the like.



+  if (debug_variable_location_views && table->view)
+   push_dw_line_info_entry (table, LI_adv_address, label_num);


It looks like you'll always use DW_LNS_fixed_advance_pc when we don't 
have .loc support.  Does that mean that the view never gets reset?


I'm uncomfortable with the special handling of this opcode; it isn't 
special in DWARF5 except as a fallback if more compact encodings aren't 
usable.  Currently GCC is even more conservative than this, and always 
use a relocation (DW_LNS_set_address); if we can use D_L_f_a_p instead, 
I would expect that to help with link times.  It seems wrong to use it 
only in the context of view support.


Would it make sense to say for *all* opcodes that the view is reset if 
and only if the address actually changes?


How are we coordinating the line number table and location list versions 
of the view counter?


Jason

[PATCH] i386: Mask out the CF_SET bit for -fcf-protection check

[H.J. Lu]

Since ix86_option_override_internal sets the CF_SET bit in
flag_cf_protection and it can be called more than once via pragma,
we need to mask out the CF_SET bit when checking flag_cf_protection.

OK for trunk if there is no regression?

H.J.
---
PR target/84248
* config/i386/i386.c (ix86_option_override_internal): Mask out
the CF_SET bit when checking -fcf-protection.
---
 gcc/config/i386/i386.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 6c612c77987..ef7ff89bcbb 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -4913,12 +4913,12 @@ ix86_option_override_internal (bool main_args_p,
   = build_target_option_node (opts);
 
   /* Do not support control flow instrumentation if CET is not enabled.  */
-  if (opts->x_flag_cf_protection != CF_NONE)
+  cf_protection_level cf_protection
+= (cf_protection_level) (opts->x_flag_cf_protection & ~CF_SET);
+  if (cf_protection != CF_NONE)
 {
-  switch (flag_cf_protection)
+  switch (cf_protection)
{
-   case CF_NONE:
- break;
case CF_BRANCH:
  if (! TARGET_IBT_P (opts->x_ix86_isa_flags2))
{
@@ -4953,7 +4953,7 @@ ix86_option_override_internal (bool main_args_p,
}
 
   opts->x_flag_cf_protection =
-   (cf_protection_level) (opts->x_flag_cf_protection | CF_SET);
+   (cf_protection_level) (cf_protection | CF_SET);
 }
 
   if (ix86_tune_features [X86_TUNE_AVOID_128FMA_CHAINS])
-- 
2.14.3

[PATCH] Use -fcf-protection=return in cet-intrin-4.c

[H.J. Lu]

Since -fcf-protection requires both -mshstk and -mibt, use
-fcf-protection=return with -mshstk in cet-intrin-4.c.

OK for trunk?

H.J.
--
PR target/84243
* gcc.target/i386/cet-intrin-4.c (dg-options): Use
-fcf-protection=return.
---
 gcc/testsuite/gcc.target/i386/cet-intrin-4.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.target/i386/cet-intrin-4.c 
b/gcc/testsuite/gcc.target/i386/cet-intrin-4.c
index 76ec160543f..437a4cd690c 100644
--- a/gcc/testsuite/gcc.target/i386/cet-intrin-4.c
+++ b/gcc/testsuite/gcc.target/i386/cet-intrin-4.c
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O -fcf-protection -mshstk" } */
+/* { dg-options "-O -fcf-protection=return -mshstk" } */
 /* { dg-final { scan-assembler "rdsspd|incsspd\[ \t]+(%|)eax" { target ia32 } 
} } */
 /* { dg-final { scan-assembler "rdssp\[dq]\[ \t]+(%|)\[re]ax"  { target { ! 
ia32 } } } } */
 /* { dg-final { scan-assembler "incssp\[dq]\[ \t]+(%|)\[re]di" { target { ! 
ia32 } } } } */
-- 
2.14.3

[PATCH] Allow (again) const variables with zero (or no) initializers in .bss* named sections (PR middle-end/84237)

[Jakub Jelinek]

Hi!

The last year's bss_initializer_p change apparently broke xen.  While it is
reasonable not to put const variables into .bss* sections by default,
refusing to put them when the user asks for it through using section
attribute seems unnecessary.  If users screws up, he can get section flag
conflicts diagnosed, if not, such as in this case when there is a separate
.bss.page_aligned.const section, it should work as before.

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

2018-02-06  Jakub Jelinek  

PR middle-end/84237
* output.h (bss_initializer_p): Add NAMED argument, defaulted to false.
* varasm.c (bss_initializer_p): Add NAMED argument, if true, ignore
TREE_READONLY bit.
(get_variable_section): For decls in named .bss* sections pass true as
second argument to bss_initializer_p.

* gcc.dg/pr84237.c: New test.

--- gcc/output.h.jj 2018-01-03 10:19:55.412533998 +0100
+++ gcc/output.h2018-02-06 15:32:28.122737867 +0100
@@ -552,7 +552,7 @@ extern void output_file_directive (FILE
 extern unsigned int default_section_type_flags (tree, const char *, int);
 
 extern bool have_global_bss_p (void);
-extern bool bss_initializer_p (const_tree);
+extern bool bss_initializer_p (const_tree, bool = false);
 
 extern void default_no_named_section (const char *, unsigned int, tree);
 extern void default_elf_asm_named_section (const char *, unsigned int, tree);
--- gcc/varasm.c.jj 2018-01-18 21:11:58.664207128 +0100
+++ gcc/varasm.c2018-02-06 15:34:16.270569690 +0100
@@ -983,11 +983,11 @@ decode_reg_name (const char *name)
 /* Return true if DECL's initializer is suitable for a BSS section.  */
 
 bool
-bss_initializer_p (const_tree decl)
+bss_initializer_p (const_tree decl, bool named)
 {
   /* Do not put non-common constants into the .bss section, they belong in
- a readonly section.  */
-  return ((!TREE_READONLY (decl) || DECL_COMMON (decl))
+ a readonly section, except when NAMED is true.  */
+  return ((!TREE_READONLY (decl) || DECL_COMMON (decl) || named)
  && (DECL_INITIAL (decl) == NULL
  /* In LTO we have no errors in program; error_mark_node is used
 to mark offlined constructors.  */
@@ -1165,7 +1165,8 @@ get_variable_section (tree decl, bool pr
 {
   section *sect = get_named_section (decl, NULL, reloc);
 
-  if ((sect->common.flags & SECTION_BSS) && !bss_initializer_p (decl))
+  if ((sect->common.flags & SECTION_BSS)
+ && !bss_initializer_p (decl, true))
{
  error_at (DECL_SOURCE_LOCATION (decl),
"only zero initializers are allowed in section %qs",
--- gcc/testsuite/gcc.dg/pr84237.c.jj   2018-02-06 15:52:00.574944214 +0100
+++ gcc/testsuite/gcc.dg/pr84237.c  2018-02-06 15:51:40.638971424 +0100
@@ -0,0 +1,5 @@
+/* PR middle-end/84237 */
+/* { dg-do compile { target *-*-linux* } } */
+/* { dg-options "" } */
+
+const char __attribute__((__section__(".bss.page_aligned.const"), 
__aligned__(4096))) zero_page[4096];

Jakub

[PATCH] Fix floating point handling in dom (PR tree-optimization/84235)

[Jakub Jelinek]

Hi!

As the following testcase shows, dom2 miscompiles floating point x - x
into 0.0 even when x could be infinity and x - x then a NaN.
The corresponding match.pd optimization is:
/* Simplify x - x.
   This is unsafe for certain floats even in non-IEEE formats.
   In IEEE, it is unsafe because it does wrong for NaNs.
   Also note that operand_equal_p is always false if an operand
   is volatile.  */
(simplify
 (minus @0 @0)
 (if (!FLOAT_TYPE_P (type) || !HONOR_NANS (type))
  { build_zero_cst (type); }))
The patch makes it match what match.pd does.
We also have:
 /* X / X is one.  */
 (simplify
  (div @0 @0)
  /* But not for 0 / 0 so that we can get the proper warnings and errors.
 And not for _Fract types where we can't build 1.  */
  (if (!integer_zerop (@0) && !ALL_FRACT_MODE_P (TYPE_MODE (type)))
   { build_one_cst (type); }))
We can ignore the 0 / 0 case, we have both operands SSA_NAMEs and match.pd
only avoids optimizing away literal 0 / 0, but the rest is valid, some fract
types don't have a way to express 1.

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

2018-02-06  Jakub Jelinek  

PR tree-optimization/84235
* tree-ssa-scopedtables.c
(avail_exprs_stack::simplify_binary_operation): Fir MINUS_EXPR, punt
if the subtraction is performed in floating point type where NaNs are
honored.  For *DIV_EXPR, punt for ALL_FRACT_MODE_Ps where we can't
build 1.  Formatting fix.

* gcc.c-torture/execute/ieee/pr84235.c: New test.

--- gcc/tree-ssa-scopedtables.c.jj  2018-01-03 10:19:54.528533857 +0100
+++ gcc/tree-ssa-scopedtables.c 2018-02-06 14:58:08.944673984 +0100
@@ -182,8 +182,15 @@ avail_exprs_stack::simplify_binary_opera
  case BIT_AND_EXPR:
return gimple_assign_rhs1 (stmt);
 
- case BIT_XOR_EXPR:
  case MINUS_EXPR:
+   /* This is unsafe for certain floats even in non-IEEE
+  formats.  In IEEE, it is unsafe because it does
+  wrong for NaNs.  */
+   if (FLOAT_TYPE_P (result_type)
+   && HONOR_NANS (result_type))
+ break;
+   /* FALLTHRU */
+ case BIT_XOR_EXPR:
  case TRUNC_MOD_EXPR:
  case CEIL_MOD_EXPR:
  case FLOOR_MOD_EXPR:
@@ -195,6 +202,9 @@ avail_exprs_stack::simplify_binary_opera
  case FLOOR_DIV_EXPR:
  case ROUND_DIV_EXPR:
  case EXACT_DIV_EXPR:
+   /* Avoid _Fract types where we can't build 1.  */
+   if (ALL_FRACT_MODE_P (TYPE_MODE (result_type)))
+ break;
return build_one_cst (result_type);
 
  default:
@@ -204,8 +214,8 @@ avail_exprs_stack::simplify_binary_opera
break;
  }
 
- default:
-   break;
+   default:
+ break;
}
}
 }
--- gcc/testsuite/gcc.c-torture/execute/ieee/pr84235.c.jj   2018-02-06 
15:04:26.528454766 +0100
+++ gcc/testsuite/gcc.c-torture/execute/ieee/pr84235.c  2018-02-06 
15:05:06.836341334 +0100
@@ -0,0 +1,11 @@
+/* PR tree-optimization/84235 */
+
+int
+main ()
+{
+  double d = 1.0 / 0.0;
+  _Bool b = d == d && (d - d) != (d - d);
+  if (!b)
+__builtin_abort ();
+  return 0;
+}

Jakub

Re: Fix LRA subreg calculation for big-endian targets

[Vladimir Makarov]

On 02/02/2018 09:17 AM, Richard Sandiford wrote:

Richard Sandiford  writes:

Segher Boessenkool  writes:

Hi!

On Fri, Jan 26, 2018 at 01:25:51PM +, Richard Sandiford wrote:

  if (SCALAR_INT_MODE_P (inmode))
new_out_reg = gen_lowpart_SUBREG (outmode, reg);
  else
-   new_out_reg = gen_rtx_SUBREG (outmode, reg, 0);
+   {
+ poly_uint64 offset = subreg_lowpart_offset (outmode, inmode);
+ new_out_reg = gen_rtx_SUBREG (outmode, reg, offset);
+   }

Is this now not exactly the same as the SCALAR_INT_MODE_P case?  The mode
of "reg" is inmode, after all?

Bah, yes.  Don't know how I missed that. :-(  I think I must have
been reading it as SCALAR_INT_P, and thinking this was some weird
VOIDmode thing.

Will fix.

Like so.  Tested as before.  OK to install?



Yes.  Thank you, Richard.

2018-02-02  Richard Sandiford  

gcc/
* lra-constraints.c (match_reload): Unconditionally use
gen_lowpart_SUBREG, rather than selecting between that
and equivalent gen_rtx_SUBREG code.

Index: gcc/lra-constraints.c
===
--- gcc/lra-constraints.c   2018-01-31 14:14:16.701405568 +
+++ gcc/lra-constraints.c   2018-02-02 14:14:50.701951577 +
@@ -942,13 +942,7 @@ match_reload (signed char out, signed ch
  reg = new_in_reg
= lra_create_new_reg_with_unique_value (inmode, in_rtx,
goal_class, "");
- if (SCALAR_INT_MODE_P (inmode))
-   new_out_reg = gen_lowpart_SUBREG (outmode, reg);
- else
-   {
- poly_uint64 offset = subreg_lowpart_offset (outmode, inmode);
- new_out_reg = gen_rtx_SUBREG (outmode, reg, offset);
-   }
+ new_out_reg = gen_lowpart_SUBREG (outmode, reg);
  LRA_SUBREG_P (new_out_reg) = 1;
  /* If the input reg is dying here, we can use the same hard
 register for REG and IN_RTX.  We do it only for original
@@ -965,13 +959,7 @@ match_reload (signed char out, signed ch
  reg = new_out_reg
= lra_create_new_reg_with_unique_value (outmode, out_rtx,
goal_class, "");
- if (SCALAR_INT_MODE_P (outmode))
-   new_in_reg = gen_lowpart_SUBREG (inmode, reg);
- else
-   {
- poly_uint64 offset = subreg_lowpart_offset (inmode, outmode);
- new_in_reg = gen_rtx_SUBREG (inmode, reg, offset);
-   }
+ new_in_reg = gen_lowpart_SUBREG (inmode, reg);
  /* NEW_IN_REG is non-paradoxical subreg.  We don't want
 NEW_OUT_REG living above.  We add clobber clause for
 this.  This is just a temporary clobber.  We can remove

[C++ PATCH] Fix ICEs due to cp_parser_postfix_dot_deref_expression workaround (PR c++/84082, take 2)

[Jakub Jelinek]

On Fri, Feb 02, 2018 at 03:13:21PM -0500, Jason Merrill wrote:
> > Or should I change the switch body to do other kind = overrides?
> 
> I think we want to avoid clobbering NON_LVALUE_EXPR location wrappers, too.
> 
> This has gotten large enough that it should break out into its own function.
> 
> OK with those changes.

So like this?  Bootstrapped/regtested on x86_64-linux and i686-linux.

2018-02-06  Jakub Jelinek  

PR c++/84082
* parser.c (cp_parser_dot_deref_incomplete): New function.
(cp_parser_postfix_dot_deref_expression): Use it.

* g++.dg/template/incomplete11.C: New test.
* g++.dg/parse/crash67.C: Expect an incomplete type diagnostics too.

--- gcc/cp/parser.c.jj  2018-01-31 19:07:35.401199026 +0100
+++ gcc/cp/parser.c 2018-02-03 19:08:37.828664789 +0100
@@ -7387,6 +7387,63 @@ cp_parser_postfix_open_square_expression
   return postfix_expression;
 }
 
+/* A subroutine of cp_parser_postfix_dot_deref_expression.  Handle dot
+   dereference of incomplete type, returns true if error_mark_node should
+   be returned from caller, otherwise adjusts *SCOPE, *POSTFIX_EXPRESSION
+   and *DEPENDENT_P.  */
+
+bool
+cp_parser_dot_deref_incomplete (tree *scope, cp_expr *postfix_expression,
+   bool *dependent_p)
+{
+  /* In a template, be permissive by treating an object expression
+ of incomplete type as dependent (after a pedwarn).  */
+  diagnostic_t kind = (processing_template_decl
+  && MAYBE_CLASS_TYPE_P (*scope) ? DK_PEDWARN : DK_ERROR);
+
+  switch (TREE_CODE (*postfix_expression))
+{
+case CAST_EXPR:
+case REINTERPRET_CAST_EXPR:
+case CONST_CAST_EXPR:
+case STATIC_CAST_EXPR:
+case DYNAMIC_CAST_EXPR:
+case IMPLICIT_CONV_EXPR:
+case VIEW_CONVERT_EXPR:
+  kind = DK_ERROR;
+  break;
+case NON_LVALUE_EXPR:
+  if (location_wrapper_p (*postfix_expression))
+   kind = DK_ERROR;
+  break;
+case OVERLOAD:
+  /* Don't emit any diagnostic for OVERLOADs.  */
+  kind = DK_IGNORED;
+  break;
+default:
+  /* Avoid clobbering e.g. DECLs.  */
+  if (!EXPR_P (*postfix_expression))
+   kind = DK_ERROR;
+  break;
+}
+
+  if (kind == DK_IGNORED)
+return false;
+
+  location_t exploc = location_of (*postfix_expression);
+  cxx_incomplete_type_diagnostic (exploc, *postfix_expression, *scope, kind);
+  if (!MAYBE_CLASS_TYPE_P (*scope))
+return true;
+  if (kind == DK_ERROR)
+*scope = *postfix_expression = error_mark_node;
+  else if (processing_template_decl)
+{
+  *dependent_p = true;
+  *scope = TREE_TYPE (*postfix_expression) = NULL_TREE;
+}
+  return false;
+}
+
 /* A subroutine of cp_parser_postfix_expression that also gets hijacked
by cp_parser_builtin_offsetof.  We're looking for
 
@@ -7451,26 +7508,9 @@ cp_parser_postfix_dot_deref_expression (
{
  scope = complete_type (scope);
  if (!COMPLETE_TYPE_P (scope)
- /* Avoid clobbering e.g. OVERLOADs or DECLs.  */
- && EXPR_P (postfix_expression))
-   {
- /* In a template, be permissive by treating an object expression
-of incomplete type as dependent (after a pedwarn).  */
- diagnostic_t kind = (processing_template_decl
-  && MAYBE_CLASS_TYPE_P (scope)
-  ? DK_PEDWARN
-  : DK_ERROR);
- cxx_incomplete_type_diagnostic
-   (location_of (postfix_expression),
-postfix_expression, scope, kind);
- if (!MAYBE_CLASS_TYPE_P (scope))
-   return error_mark_node;
- if (processing_template_decl)
-   {
- dependent_p = true;
- scope = TREE_TYPE (postfix_expression) = NULL_TREE;
-   }
-   }
+ && cp_parser_dot_deref_incomplete (, _expression,
+_p))
+   return error_mark_node;
}
 
   if (!dependent_p)
--- gcc/testsuite/g++.dg/template/incomplete11.C.jj 2018-02-03 
17:56:10.536083614 +0100
+++ gcc/testsuite/g++.dg/template/incomplete11.C2018-02-03 
17:56:10.536083614 +0100
@@ -0,0 +1,10 @@
+// PR c++/84082
+// { dg-do compile }
+// { dg-options "" }
+
+struct A;
+
+template void foo()
+{
+  static int a[A().operator=(A())];// { dg-error "invalid use of 
incomplete type 'struct A'" }
+}
--- gcc/testsuite/g++.dg/parse/crash67.C.jj 2018-01-31 19:07:33.571220455 
+0100
+++ gcc/testsuite/g++.dg/parse/crash67.C2018-02-03 17:56:10.536083614 
+0100
@@ -3,4 +3,4 @@
 
 class x0;
 template  x2() {  // { dg-error "declared|type" }
-x0 x3 = x3.  // { dg-error "expected" }
+x0 x3 = x3.  // { dg-error "expected|incomplete type" }


Jakub

Re: [PATCH] PowerPC PR target/84154, fix floating point to small integer conversion regression

[Michael Meissner]

On Tue, Feb 06, 2018 at 11:15:21AM -0600, Segher Boessenkool wrote:
> Hi!
> 
> On Mon, Feb 05, 2018 at 05:57:51PM -0500, Michael Meissner wrote:
> > > We have
> > > 
> > > (define_code_attr su [(sign_extend  "s")
> > >   (zero_extend  "u")
> > >   (fix  "s")
> > >   (unsigned_fix "s")
> > >   (float"s")
> > >   (unsigned_float   "u")])
> > > 
> > > and "s" for unsigned_fix seems like it should be "u".  Very surprising
> > > otherwise (if this is needed in some case, it should just write "s" there
> > > instead of "").
> 
> What about this?

You were right.  I kept missing this.  Sorry.

Here is the patch for just this fix.  As we've discussed, I'll apply this patch
directly as being obvious, while waiting for the other builds.

[gcc]
2018-02-06  Michael Meissner  

PR target/84154
* config/rs6000/rs6000.md (su code attribute): Use "u" for
unsigned_fix, not "s".

Index: gcc/config/rs6000/rs6000.md
===
--- gcc/config/rs6000/rs6000.md (revision 257269)
+++ gcc/config/rs6000/rs6000.md (working copy)
@@ -550,7 +550,7 @@ (define_code_attr u  [(sign_extend  "")
 (define_code_attr su [(sign_extend "s")
  (zero_extend  "u")
  (fix  "s")
- (unsigned_fix "s")
+ (unsigned_fix "u")
  (float"s")
  (unsigned_float   "u")])
 

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

Re: [PATCH, rs6000] Fix PR83926, ICE using __builtin_vsx_{div,udiv,mul}_2di builtins

[Peter Bergner]

On 2/6/18 11:39 AM, Segher Boessenkool wrote:
> Yes, if you make [u]divdi3 work for SDI (i.e. also for -m32) we'll never
> have to special-case it again (also [u]moddi3?)

Maybe, but there are no explicit calls to those, so I left them
as is, since if we touch those, then we probably need to look at
almost all logical/arithmetic patterns...and that is a lot of work
not needed to fix this specific issue.


> Yes, if you make [u]divdi3 work for SDI (i.e. also for -m32) we'll never
> have to special-case it again (also [u]moddi3?)

Heh, looking at "mod3", I see it calls gen_div3(), but since
there are no explicit calls to gen_moddi3(), we don't really have a problem.


> But this then also will be used by expand, when generation a DImode
> divide.  Does it generate code at least as good as what the generic code
> generates?  It probably will, but check please.

The change from GPR to SDI iterator and my change to explicitly call
the __divdi3() lib function for DImode divides ends up generating the
exact same code generated for the following test case that we get with
the non patched compiler:

  long long
  foo (long long a, long long b)
  {
return a / b;
  }

Peter

Re: Extend aligned_membuf<> usage

[François Dumont]

On 05/02/2018 18:16, Jonathan Wakely wrote:

On 01/02/18 22:48 +0100, François Dumont wrote:

Hi

    As we just bump version namespace it might be interesting to do 
the following change now. What do you think ?


I'd rather not make the code harder to read in all those places just
for an optional mode that nobody uses.



I just consider this mode as a nice way to track all abi changes that we 
will be able to commit one day when we decide an abi breaking change.



Wouldn't it make more sense to simply make __aligned_buffer identical
to __aligned_membuf for the versioned-namespace? Then at least the
conditional code is only in one place.

Yes, __aligned_buffer is indeed not used, we could do that. I'll propose 
a patch to do so.

Re: [build, v3] Enable COMDAT group support on Solaris 10

[Rainer Orth]

Rainer Orth  writes:

> While investigating PR bootstrap/84017, it turned out that it *is*
> possible to enable COMDAT group support on Solaris 10 with Solaris ld in
> some circumstances.  I'm posting my findings and the resulting patch
> here for reference only; this is certainly not GCC 8 material.
>
> Besides, there's a abi_check failure in libstdc++ which makes it unclear
> if this desirable at all.
>
> So here's the deal: I remembered that Solaris ld has a -z relaxreloc
> option, documented as
>
>  -z relaxreloc
>
>  ld normally issues a fatal  error  upon  encountering  a
>  relocation  using a symbol that references an eliminated
>  COMDAT section. If -z relaxreloc is enabled, ld  instead
>  redirects  such  relocations to the equivalent symbol in
>  the COMDAT section that was kept.  -z  relaxreloc  is  a
>  specialized  option,  mainly  of  interest  to  compiler
>  authors, and is not intended for general use.
>
> It was only introduced in Solaris 10 Update 10, so won't help on earlier
> versions.
>
> When ld supports that option, I always pass it when linking and enable
> comdat_group (and hidden_linkonce) in configure.ac.
>
> Test results are astonishingly good:
>
> * On Solaris 10/x86, I get three tests where ld SEGVs with -flto
>   (skipped/xfailed in the present patch).
>
> * On both Solaris 10/SPARC and x86, libstdc++-abi/abi_check FAILs:
>
> # of added symbols:  2
> # of missing symbols:0
> # of undesignated symbols:   0
> # of incompatible symbols:   2
>
> 2 added symbols
> 0
> _ZNSt7__cxx1115basic_stringbufIwSt11char_traitsIwESaIwEED2Ev
> std::__cxx11::basic_stringbuf std::allocator >::~basic_stringbuf()
> version status: incompatible
> GLIBCXX_3.4.21
> type: function
> status: added
>
> 1
> _ZNSt7__cxx1115basic_stringbufIcSt11char_traitsIcESaIcEED2Ev
> std::__cxx11::basic_stringbuf std::allocator >::~basic_stringbuf()
> version status: incompatible
> GLIBCXX_3.4.21
> type: function
> status: added
>
>
> 2 incompatible symbols
> 0
> _ZNSt7__cxx1115basic_stringbufIwSt11char_traitsIwESaIwEED2Ev
> std::__cxx11::basic_stringbuf std::allocator >::~basic_stringbuf()
> version status: incompatible
> GLIBCXX_3.4.21
> type: function
> status: added
>
>
> 1
> _ZNSt7__cxx1115basic_stringbufIcSt11char_traitsIcESaIcEED2Ev
> std::__cxx11::basic_stringbuf std::allocator >::~basic_stringbuf()
> version status: incompatible
> GLIBCXX_3.4.21
> type: function
> status: added
>
>   Those symbols are present in the Solaris 11 baselines (where COMDAT
>   group always works), which is also used with Solaris 10 and GNU ld.
>
>   I'm not yet sure how to deal with this: we'd have to introduce 3
>   baselines:
>
>   * Solaris 10 without COMDAT group support
>   * Solaris 10 with COMDAT group support
>   * Solaris 11
>
>   Seems like a lot of trouble to me actually.  I've not done anything
>   about this yet, so as is the patch shows the abi_check failures.

I've now revised the patch a bit, especially for the abi_check failure:

* Since Solaris 10 with gld (which already did support COMDAT group)
  already used the *-solaris2.11 baselines, I've extended that to any
  Solaris 10 configuration with COMDAT group support.  This needed a
  configure test in libstdc++-v3/acinclude.m4 matching what the
  comdat_group effective-target keyword does.  However, I couldn't use
  a simple AC_TRY_COMPILE with -S added to CXXFLAGS: that will always
  fail since the macro checks for the existence of conftest.o as first
  indication of success, which of course doesn't exist with -S.

* I've also tried to enable USE_HIDDEN_LINKONCE on Solaris 10/x86 with
  COMDAT group, but that leads to four more testcases where ld SEGVs, so
  I've left it disabled.

Otherwise, nothing remarkable: bootstrapped without regressions on
i386-pc-solaris2.10 and sparc-sun-solaris2.10 (both as/ld and gas/ld)
without and with --disable-comdat; also bootstrapped on some
*-*-solaris2.11 configurations, no regressions.

I'm just posting this for comment here; I may or may not commit this in
the GCC 9 timeframe.

Rainer

-- 
-
Rainer Orth, Center for Biotechnology, Bielefeld University


2018-01-28  Rainer Orth  

libstdc++-v3:
* acinclude.m4 (GLIBCXX_CONFIGURE): Check for COMDAT group
support.
* configure: Regenerate.
* configure.host (*-*-solaris2.1[0-9])): Check $comdat_group to
select baselines.

gcc/testsuite:
* g++.dg/lto/pr65475c_0.C: Skip on Solaris 10/x86 with ld/comdat.
* g++.dg/lto/pr68057_0.C: Likewise.
* gcc.dg/debug/pr41893-1.c: Xfail on Solaris 10/x86 with ld/comdat
for -gdwarf-2 -g3.

gcc:
* 

Re: [build] Fix HAVE_GAS_CFI_DIRECTIVE for x86_64-pc-solaris2.*

[Rainer Orth]

Rainer Orth  writes:

> When compiling MariaDB 10.2 (which uses cfi directives in inline asm,
> but gets the conditionals wrong) on Solaris 11.4 with an
> x86_64-pc-solaris2.11 gcc using gas, I noticed that this one incorrectly
> doesn't use cfi directives although it could/should.
>
> Looking closer, I noticed that the logic in gcc/configure.ac's
> gcc_cv_as_cfi_directive test is not only hard to follow due to deeply
> nested ifs, but plain wrong in the current case: for 64-bit Solaris/x86,
> the .eh_frame section is (and should be) read-only, unlike the 32-bit
> case, but is incorrectly rejected.
>
> Unlike what we currently do, configure.ac needs to verify that both the
> 32 and 64-bit .eh_frame sections are as the toolchain requires them to
> be, which is what the current patch does.
>
> Since .eh_frame section test already appeared repeatedly, making the
> whole code hard to read due to the redundancy, I've moved it to a shell
> function.  While gcc/configure.ac itself currently doesn't use those
> directly, the generated configure already does, so we should be save
> here.
>
> Tested with a standalone script with the necessary boilerplate added on
> Solaris 10, 11.3 and 11.4 with /bin/as, bundled gas (2.15, 2.23.1,
> 2.29), and gas 2.30 on both sparc and x86.
>
> Also bootstrapped without regressions on i386-pc-solaris2.11 and
> sparc-sun-solaris2.11 (as/ld, gas/ld, both 32 and 64-bit).
>
> While this only touches Solaris-specific code, it would be nice if one
> of the build maintainers could have a look.
>
> Ok for mainline?

After a week with no word from the build maintainers, I've installed the
patch on mainline since it's purely Solaris-specific.

Rainer

-- 
-
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PATCH, rs6000] Fix PR83926, ICE using __builtin_vsx_{div,udiv,mul}_2di builtins

[Segher Boessenkool]

On Tue, Feb 06, 2018 at 10:36:41AM -0600, Peter Bergner wrote:
> On 2/6/18 10:20 AM, David Edelsohn wrote:
> > Do the gen_XXXdi3 calls work if you use SDI iterator instead of GPR
> > iterator, as Segher suggested?
> 
> Well it works _if_ we use the first patch that changes the gen_*
> patterns.  If we go this route, I agree we should use the SDI
> iterator instead of GPR.
> 
> 
> > Otherwise, this seems like the more correct approach to not conflict
> > with the semantics expected by the patterns.
> 
> It's up to you and Segher which patch you think is cleaner/more preferable.
> The benefit of the gen_* patch is that if any code added in the future
> calls those gen_* routines, then they'll work with no changes.  Otherwise,
> the new code would have to do something similar to this latest patch.
> Kind of a "six of one, half dozen of the other" sort of thing.
> I'm fine either way.

Yes, if you make [u]divdi3 work for SDI (i.e. also for -m32) we'll never
have to special-case it again (also [u]moddi3?)

But this then also will be used by expand, when generation a DImode
divide.  Does it generate code at least as good as what the generic code
generates?  It probably will, but check please.


Segher

Re: [PATCH]] PR target/81084: Fork documentation for powerpcspe and clean up

[Andrew Jenner]

On 06/02/2018 17:19, Joseph Myers wrote:

On Tue, 6 Feb 2018, Andrew Jenner wrote:


Okay to commit to trunk?


As you're powerpcspe maintainer I'm not sure what you're asking for
approval for.  Are you asking rs6000 port maintainers to approve the
removal of the documentation for options that no longer exist in that
port?


That was part of my thinking. Also, I wasn't sure if I had write without 
approval permission for powerpcspe-specific parts of doc/*, and I wanted 
to err on the safe side. Given that you have said that, I guess that 
means I do.


I've also decided that the rs6000 doc changes are obvious (the 
corresponding options are already removed from the backend itself), so I 
have committed the patch.


Thanks,

Andrew

Re: [PATCH]] PR target/81084: Fork documentation for powerpcspe and clean up

[Joseph Myers]

On Tue, 6 Feb 2018, Andrew Jenner wrote:

> Okay to commit to trunk?

As you're powerpcspe maintainer I'm not sure what you're asking for 
approval for.  Are you asking rs6000 port maintainers to approve the 
removal of the documentation for options that no longer exist in that 
port?

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

Re: [PATCH] PowerPC PR target/84154, fix floating point to small integer conversion regression

[Segher Boessenkool]

Hi!

On Mon, Feb 05, 2018 at 05:57:51PM -0500, Michael Meissner wrote:
> > We have
> > 
> > (define_code_attr su [(sign_extend  "s")
> >   (zero_extend  "u")
> >   (fix  "s")
> >   (unsigned_fix "s")
> >   (float"s")
> >   (unsigned_float   "u")])
> > 
> > and "s" for unsigned_fix seems like it should be "u".  Very surprising
> > otherwise (if this is needed in some case, it should just write "s" there
> > instead of "").

What about this?

> I found as long as I avoid putting the  or  in the output template
> (i.e. use an output statement instead of a template) it works.  It only
> seems to fail in the IEEE128 case, and not in the SFDF case.  I will submit a
> bug report  on it after this gets checked in, as it will be simpler to provide
> a patch that people can test.

Thanks.

> +(define_insn_and_split "fix_trunc2"
> +  [(set (match_operand: 0 "gpc_reg_operand" "=wJ,wJwK,r")
> + (any_fix:QHI (match_operand:SFDF 1 "gpc_reg_operand" "wJ,wJwK,wa")))
> +   (clobber (match_scratch:SI 2 "=X,X,wi"))]
> +  "TARGET_DIRECT_MOVE"
> +  "@
> +   fctiwz %0,%1
> +   xscvdpxws %x0,%x1

This one cannot work with the  definition above, for example.

> +(define_insn "fix_2_hw"
> +  [(set (match_operand:SDI 0 "altivec_register_operand" "=v")
> + (any_fix:SDI (match_operand:IEEE128 1 "altivec_register_operand" "v")))]
> +  "TARGET_FLOAT128_HW && FLOAT128_IEEE_P (mode)"
> +{
> +  return ( == UNSIGNED_FIX) ? "xscvqpuz %0,%1" : "xscvqpsz 
> %0,%1";
> +}

And it may well be why you need this.

Rest looks good :-)


Segher

Re: [PATCH] PR libgcc/59714 complex division is surprising on aarch64

[Joseph Myers]

On Tue, 6 Feb 2018, vladimir.mezent...@oracle.com wrote:

>  I compared with __builtin_ilogb. There is a same performance degradation.

I'm not clear on exactly what you compared with.

Do you mean the *existing* __builtin_ilogb?  That's only supported for 
constant arguments that are not zero, infinity or NaN, and with an i386 
optab for -funsafe-math-optimizations.  So it's not a useful comparison at 
all, because it would call a libm function.

Rather, if built-in functions were used for this, they'd be *new* built-in 
functions.  It would be entirely possible to implement __builtin_ilogb (at 
least for -fno-math-errno) that actually expands inline, using target 
hooks to provide the values of FP_ILOGB0 / FP_ILOGBNAN, though the 
expansion might be too big to be generally useful.

But a built-in function specifically to extract the exponent field of a 
floating-point number (without doing any biasing or adjusting for special 
cases) would be a more plausible comparison.  The existing description of 
floating-point number formats in real.h includes specification of 
signbit_ro and signbit_rw.  It would be possible to have such a 
specification of the location of an exponent field, where there's one that 
can be located simply within the floating-point number, and then 
machine-independent code to expand such a built-in function.

I'm not asserting that you should add such a function.  I *am* asserting 
that the patch proposal needs to include serious consideration of various 
options such as:

* Adding such a built-in function.

* Adding predefined macros (conditional on -fbuilding-libgcc) that assert 
that a particular floating-point mode has a particular IEEE format.

Such predefined macros have a clear precedent - we already have 
__LIBGCC_%s_MANT_DIG__, __LIBGCC_HAS_%s_MODE__, __LIBGCC_%s_FUNC_EXT__, 
__LIBGCC_%s_EXCESS_PRECISION__.  So why not something to describe IEEE 
properties of a machine mode?

The use of sfp-machine.h is problematic because, while having 
sfp-machine.h implies use of at least some IEEE formats, there is no 
implication the other way round: sfp-machine.h is only present if at least 
one format uses software floating point, not if all use hardware floating 
point (or use software floating point in libc not libgcc, or use software 
floating point from fp-bit instead of soft-fp).  That's not a particularly 
sensible criterion for whether to use the new logic.

Furthermore, it's entirely possible that only some formats are IEEE - 
thus, on powerpc64le, __LIBGCC_HAS_TF_MODE__ is true, and soft-fp.h is 
present, but TFmode is actually IBM long double not IEEE binary128, so 
your logic would handle that case incorrectly.  Thus you need "mode X has 
IEEE format Y" logic that avoids the case where mode X exists, and IEEE 
format Y is supported, and sfp-machine.h exists, but mode X has another 
format, because that's actually the case right now for TFmode on 
powerpc64le.

Again, I'm not saying you should add such predefined macros - but at a 
first look I think they are a better approach than the use of presence of 
sfp-machine.h to indicate which modes have IEEE formats, and I think they 
are among the options that any patch submission needs to discuss carefully 
if rejecting.

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

Re: RFA: Sanitize deprecation messages (PR 84195)

[Martin Sebor]

On 02/06/2018 05:39 AM, Nick Clifton wrote:

Hi Martin,


My only suggestions are to consider how control characters and
excessively messages are handled in other contexts and adopt
the same approach here.


Are there other places where user-supplied messages are displayed by gcc ?


There are a few that I know of.  I added an incomplete list to
the bug but I'm pretty sure I missed some.





In the tests I did, control characters
were mostly escaped (e.g., as \n or as \x0a) rather than replaced
with spaces.


Ooo - what tests were these ?


Some of those David already mentioned (from my recent comment
on the bug):

1) #error and #warning where the string is printed as it appears
   in the source (i.e., with escape sequences),
2) -Wformat where control characters are escaped,
3) -Wformat-overflow where control characters are not escaped
   unless -fexec-charset= is specified.  I consider it a bug
   and plan to fix it.


I agree that it there is a standard way to handle these characters
then the deprecated attribute ought to do the same.  (Plus hopefully
there is already a function in gcc to do this kind of processing,
so the deprecation code can just call it).


My preference is the same as David's: escape control characters
(using standard C escape sequences, i.e., \a, \n, etc, or \xff
for those that don't have the shorthand notation).





I haven't thought too hard about how excessively
long messages should be handled, or about the interactions with
-fmessage-length= (i.e., if messages longer than the limit should
be broken up.)


I believe that this will happen automatically.  The diagnostic display
routine will automatically insert newlines into the output if the
message length is non-zero, and the message extends past this limit.
(Well provided that the message contains spaces.  The newlines are
only inserted in place of space characters so a very long, single
word message will not be split...)


You're right, it does seem to happen automatically for strings
in #warning and attribute deprecated.

Martin

Re: PR tree-optimization/84225: do not pass non-integers to operation_no_trapping_overflow

[Aldy Hernandez]

On 02/06/2018 10:52 AM, Jakub Jelinek wrote:

On Tue, Feb 06, 2018 at 10:43:21AM -0500, Aldy Hernandez wrote:

In this PR we are ICEing here:

bool
operation_no_trapping_overflow (tree type, enum tree_code code)
{
   gcc_checking_assert (ANY_INTEGRAL_TYPE_P (type));

...because we are being passed a pointer type from find_trapping_overflow.

Fixed by avoiding passing non-integrals from find_trapping_overflow.

Pre-approved by Jakub.  Committed.

Tested on x86-64 Linux.



gcc/

PR tree-optimization/84225
* tree-eh.c (find_trapping_overflow): Only call
operation_no_trapping_overflow when ANY_INTEGRAL_TYPE_P.


Can you please add a testcase too?
While the reported ICE was on an existing testcase, it was with
non-standard options on it.
So e.g.
gcc.dg/pr84225.c
that would contain:

/* PR tree-optimization/84225 */
/* { dg-do compile { target int32plus } } */
/* { dg-options "-Ofast -ftrapv" } */

#include "torture/pr69714.c"


Sorry for being sloppy on this.  I didn't even know we could #include 
other tests without the dg-do stuff being included and messing things up.


I have committed the code below, and have verified that it passes with 
the previous patch, and ICEs without the patch.


Thanks.

commit e783f8fb7d9a7b0972b4ebe4b86aa541904de692 (HEAD -> 
pr84225-jakub-trapv)

Author: Aldy Hernandez 
Date:   Tue Feb 6 12:06:40 2018 -0500

PR tree-optimization/84225
Add test for previous commit for PR84225.

diff --git a/gcc/testsuite/gcc.dg/pr84225.c b/gcc/testsuite/gcc.dg/pr84225.c
new file mode 100644
index 000..f57266c9a26
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr84225.c
@@ -0,0 +1,5 @@
+/* PR tree-optimization/84225 */
+/* { dg-do compile { target int32plus } } */
+/* { dg-options "-Ofast -ftrapv" } */
+
+#include "torture/pr69714.c"

Re: Do not lose track of resolution info due to tree merging

[Richard Biener]

On February 6, 2018 2:29:42 PM GMT+01:00, Jan Hubicka  wrote:
>> 
>> I think unify_scc is only ever called from WPA so you can elide the
>flag_ltrans checks. 
>> 
>> Otherwise OK. 
>
>Thanks, I have updated this and also dropped the sanity check from
>symtab.c
>because, well, it finds another bugs in target versioning I will handle
>incrementally.  I also noticed I need to rule out the check for
>register
>variables and builtins because we do not stream those into the lto
>symtab.  I
>would say that real_symbol_p should return false for register variable
>and we
>should stream bulitins but that is for independent change, too.
>
>I suppose we should backport this to all release branches.

After some soaking, yes.

>Honza
>
>   PR lto/81004
>   * lto.c: Include builtins.h
>   (register_resolution): Merge resolutions in case trees was
>   merged across units.
>   (lto_maybe_register_decl): Break out from ...
>   (lto_read_decls): ... here.
>   (unify_scc): Also register decls here.
>   (read_cgraph_and_symbols): Sanity check that all resolutions was
>   read.
>Index: lto.c
>===
>--- lto.c  (revision 257382)
>+++ lto.c  (working copy)
>@@ -54,6 +54,7 @@ along with GCC; see the file COPYING3.
> #include "stringpool.h"
> #include "fold-const.h"
> #include "attribs.h"
>+#include "builtins.h"
> 
> 
>/* Number of parallel tasks to run, -1 if we want to use GNU Make
>jobserver.  */
>@@ -830,12 +831,20 @@ static void
> register_resolution (struct lto_file_decl_data *file_data, tree decl,
>enum ld_plugin_symbol_resolution resolution)
> {
>+  bool existed;
>   if (resolution == LDPR_UNKNOWN)
> return;
>   if (!file_data->resolution_map)
> file_data->resolution_map
>   = new hash_map;
>-  file_data->resolution_map->put (decl, resolution);
>+  ld_plugin_symbol_resolution_t 
>+ = file_data->resolution_map->get_or_insert (decl, );
>+  gcc_assert (!existed || res == resolution);
>+  if (!existed
>+  || resolution == LDPR_PREVAILING_DEF_IRONLY
>+  || resolution == LDPR_PREVAILING_DEF
>+  || resolution == LDPR_PREVAILING_DEF_IRONLY_EXP)
>+res = resolution;
> }
> 
> /* Register DECL with the global symbol table and change its
>@@ -878,6 +887,18 @@ lto_register_function_decl_in_symtab (st
>decl, get_resolution (data_in, ix));
> }
> 
>+/* Check if T is a decl and needs register its resolution info.  */
>+
>+static void
>+lto_maybe_register_decl (struct data_in *data_in, tree t, unsigned ix)
>+{
>+  if (TREE_CODE (t) == VAR_DECL)
>+lto_register_var_decl_in_symtab (data_in, t, ix);
>+  else if (TREE_CODE (t) == FUNCTION_DECL
>+ && !DECL_BUILT_IN (t))
>+lto_register_function_decl_in_symtab (data_in, t, ix);
>+}
>+
> 
> /* For the type T re-materialize it in the type variant list and
>the pointer/reference-to chains.  */
>@@ -1617,7 +1638,10 @@ unify_scc (struct data_in *data_in, unsi
> /* Fixup the streamer cache with the prevailing nodes according
>to the tree node mapping computed by compare_tree_sccs.  */
> if (len == 1)
>-  streamer_tree_cache_replace_tree (cache, pscc->entries[0], from);
>+  {
>+lto_maybe_register_decl (data_in, pscc->entries[0], from);
>+streamer_tree_cache_replace_tree (cache, pscc->entries[0],
>from);
>+  }
> else
>   {
> tree *map2 = XALLOCAVEC (tree, 2 * len);
>@@ -1625,6 +1649,7 @@ unify_scc (struct data_in *data_in, unsi
>   {
> map2[i*2] = (tree)(uintptr_t)(from + i);
> map2[i*2+1] = scc->entries[i];
>+lto_maybe_register_decl (data_in, scc->entries[i], from + i);
>   }
> qsort (map2, len, 2 * sizeof (tree), cmp_tree);
> qsort (map, len, 2 * sizeof (tree), cmp_tree);
>@@ -1761,13 +1786,7 @@ lto_read_decls (struct lto_file_decl_dat
>   cache_integer_cst (t);
> if (!flag_ltrans)
>   {
>-/* Register variables and functions with the
>-   symbol table.  */
>-if (TREE_CODE (t) == VAR_DECL)
>-  lto_register_var_decl_in_symtab (data_in, t, from + i);
>-else if (TREE_CODE (t) == FUNCTION_DECL
>- && !DECL_BUILT_IN (t))
>-  lto_register_function_decl_in_symtab (data_in, t, from + i);
>+lto_maybe_register_decl (data_in, t, from + i);
> /* Scan the tree for references to global functions or
>variables and record those for later fixup.  */
> if (mentions_vars_p (t))
>@@ -2873,13 +2892,21 @@ read_cgraph_and_symbols (unsigned nfiles
> 
>   /* Store resolutions into the symbol table.  */
> 
>-  ld_plugin_symbol_resolution_t *res;
>   FOR_EACH_SYMBOL 

Re: [PATCH 2/7] Support >26 operands in generation code.

[Richard Sandiford]

Alan Hayward  writes:
> This patch adds support for CLOBBER_HIGH in the generation code.
>
> An aarch64 will require 31 clobber high expressions, plus two
> clobbers.
>
> The exisiting gen code restricts to 26 vector operands by virtue
> of using the operators [a-z]. This patch extends this to 52 by
> supporting [a-zA-Z].

Although the main CLOBBER_HIGH patch will obviously need to wait
until GCC 9 now, we can work around it for GCC 8 by making the
tlsdesc pattern clobber all vector and predicate registers for SVE.
We'll still need the support for more than 26 operands in order
to do that though.

> 2017-11-16  Alan Hayward  
>
>[...]
>   * genextract.c (push_pathstr_operand): New function to
>   support [a-zA-Z].
>   (walk_rtx): Call push_pathstr_operand.
>   (print_path): Support [a-zA-Z].
>[...]
> diff --git a/gcc/genextract.c b/gcc/genextract.c
> index 
> 258d234d2729bf16b152b90bb1833a37a6eb0bdc..e1fb716e459b9bd219e89cf36c30556d520305a2
>  100644
> --- a/gcc/genextract.c
> +++ b/gcc/genextract.c
> @@ -33,9 +33,10 @@ along with GCC; see the file COPYING3.  If not see
>
> The string for each operand describes that path to the operand and
> contains `0' through `9' when going into an expression and `a' through
> -   `z' when going into a vector.  We assume here that only the first operand
> -   of an rtl expression is a vector.  genrecog.c makes the same assumption
> -   (and uses the same representation) and it is currently true.  */
> +   `z' then 'A' through to 'Z' when going into a vector.  We assume here that
> +   only the first operand of an rtl expression is a vector.  genrecog.c makes
> +   the same assumption (and uses the same representation) and it is currently
> +   true.  */
>
>  typedef char *locstr;
>
> @@ -80,6 +81,22 @@ struct accum_extract
>  /* Forward declarations.  */
>  static void walk_rtx (md_rtx_info *, rtx, struct accum_extract *);
>
> +#define UPPER_OFFSET ('A' - ('z' - 'a' + 1))
> +
> +/* Convert OPERAND into a character - either into [a-zA-Z] for vector 
> operands
> +   or [0-9] for integer operands - and push onto the end of the path ACC.  */
> +static void
> +push_pathstr_operand (int operand, bool is_vector,
> +  struct accum_extract *acc)
> +{
> +  if (is_vector && 'a' + operand > 'z')
> +acc->pathstr.safe_push (operand + UPPER_OFFSET);
> +  else if (is_vector)
> +acc->pathstr.safe_push (operand + 'a');
> +  else
> +acc->pathstr.safe_push (operand + '0');
> +}
> +
>  static void
>  gen_insn (md_rtx_info *info)
>  {
> @@ -98,7 +115,7 @@ gen_insn (md_rtx_info *info)
>else
>  for (i = XVECLEN (insn, 1) - 1; i >= 0; i--)
>{
> - acc.pathstr.safe_push ('a' + i);
> + push_pathstr_operand (i, true, );
>   walk_rtx (info, XVECEXP (insn, 1, i), );
>   acc.pathstr.pop ();
>}
> @@ -208,7 +225,7 @@ static void
>  walk_rtx (md_rtx_info *info, rtx x, struct accum_extract *acc)
>  {
>RTX_CODE code;
> -  int i, len, base;
> +  int i, len;
>const char *fmt;
>
>if (x == 0)
> @@ -234,10 +251,9 @@ walk_rtx (md_rtx_info *info, rtx x, struct accum_extract 
> *acc)
>VEC_safe_set_locstr (info, >oplocs, XINT (x, 0),
>  VEC_char_to_string (acc->pathstr));
>
> -  base = (code == MATCH_OPERATOR ? '0' : 'a');
>for (i = XVECLEN (x, 2) - 1; i >= 0; i--)
>   {
> -   acc->pathstr.safe_push (base + i);
> +   push_pathstr_operand (i, code != MATCH_OPERATOR, acc);
> walk_rtx (info, XVECEXP (x, 2, i), acc);
> acc->pathstr.pop ();
>  }
> @@ -252,10 +268,9 @@ walk_rtx (md_rtx_info *info, rtx x, struct accum_extract 
> *acc)
>if (code == MATCH_DUP)
>   break;
>
> -  base = (code == MATCH_OP_DUP ? '0' : 'a');
>for (i = XVECLEN (x, 1) - 1; i >= 0; i--)
>  {
> -   acc->pathstr.safe_push (base + i);
> +   push_pathstr_operand (i, code != MATCH_OP_DUP, acc);
> walk_rtx (info, XVECEXP (x, 1, i), acc);
> acc->pathstr.pop ();
>  }
> @@ -271,7 +286,7 @@ walk_rtx (md_rtx_info *info, rtx x, struct accum_extract 
> *acc)
>  {
>if (fmt[i] == 'e' || fmt[i] == 'u')
>   {
> -   acc->pathstr.safe_push ('0' + i);
> +   push_pathstr_operand (i, false, acc);
> walk_rtx (info, XEXP (x, i), acc);
> acc->pathstr.pop ();
>   }
> @@ -280,7 +295,7 @@ walk_rtx (md_rtx_info *info, rtx x, struct accum_extract 
> *acc)
> int j;
> for (j = XVECLEN (x, i) - 1; j >= 0; j--)
>   {
> -   acc->pathstr.safe_push ('a' + j);
> +   push_pathstr_operand (j, true, acc);
> walk_rtx (info, XVECEXP (x, i, j), acc);
> acc->pathstr.pop ();
>   }
> @@ -311,7 +326,7 @@ print_path (const char *path)
>
>for (i = len - 1; i >= 0 ; i--)
>  {
> -  if (ISLOWER (path[i]))
> +  if (ISLOWER (path[i]) || ISUPPER (path[i]))
>   fputs ("XVECEXP (", 

Re: [PATCH] PR libgcc/59714 complex division is surprising on aarch64

[Joseph Myers]

On Mon, 5 Feb 2018, vladimir.mezent...@oracle.com wrote:

> My implementation avoids these issues.
> I use soft-fp instead built-in functions because performance with built-in
> functions (like __builtin_ilogb) is in two times worse.
> Also libm is needed for built-in functions.

I think you still need to provide a more detailed explanation of the 
implementation approach (discussing the issues and choices made, how and 
why it's safe on all systems, how your code avoids any problems with 
subnormals, infinities and NaNs, etc.).

As far as I can tell, the argument for better performance compared to 
built-in functions is that a generally useful built-in function would need 
special handling of infinities, NaNs, zero and subnormals, which you 
avoid by e.g. extracting exponent fields directly.  This means that a 
detailed justification is needed of how it's safe for the exponent fields 
of such values to be passed straight through your various functions 
without special cases 0 - probably most of that should go in comments in 
the code explaining what happens for such values and why it successfully 
avoids overflow and underflow.

The files in libgcc/soft-fp must be verbatim copies of the master sources 
in glibc.  So you can't make any local changes to them, and if you think 
changes are needed you need to patch things upstream in glibc first, with 
a proper extended explanation of why the fix is needed and why it is safe 
and the appropriate design for the fix.  There's nothing at all in this 
patch submission to explain that change.

You shouldn't have any commented-out code like your "+//#include 
"soft-fp/soft-fp.h"" in the submitted patch.

All the new inline functions need more precise comments explaining what 
their semantics are in cases such as zero, subnormals, infinities, etc., 
or stating that the semantics are that such arguments must not be passed 
to those functions.  (They also need to be formatted according to the GNU 
Coding Standards - space before '(', function name starts a new line.)

> +/* Return an exponent N, such that neither (2**(ce-N))**2 nor (2**(de-N))**2
> +   cause an overflow. Also try to avoid underflow. */

So (for example), for this comment, I'd expect information about: what is 
the valid range of exponents N that this function may return (given that, 
I suppose, you want 2**N to be a representable floating-point value), and 
what are the valid ranges of CE and DE for which this function guarantees 
to achieve those semantics with an exponent within the required range?  
And are those semantics strictly for the actual numbers specified, or, 
given how a subnormal value may result in an exponent -EXPBIAS here 
despite the actual exponent being smaller, are there additional semantics 
required in the case where CE or DE is -EXPBIAS?

Once there are comments on these functions giving all the required 
semantics, we can start to reason about whether those semantics ensure the 
required higher-level properties (regarding overflow / underflow in 
__div*c3 functions) in the case where subnormals are involved.  I'd expect 
such reasoning to appear in comments in the code.

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

Re: [PATCH, rs6000] Deprecate -mno-speculate-indirect-jumps

[Bill Schmidt]

On Feb 6, 2018, at 10:33 AM, Segher Boessenkool  
wrote:
> 
> Hi Bill,
> 
> On Mon, Feb 05, 2018 at 12:28:34PM -0600, Bill Schmidt wrote:
>> It's been determined that we won't recommend use of the recently added
>> undocumented option -mno-speculate-indirect-jumps.  This patch provides
>> a warning indicating that the option is deprecated.  We will leave the
>> code in place for potential emergency use (not expected), and plan to
>> remove it completely in GCC 9.
> 
>> +  /* Deprecate use of -mno-speculate-indirect-jumps.  */
>> +  if (!rs6000_speculate_indirect_jumps)
>> +warning (0, "%qs is deprecated and not recommended in any 
>> circumstances",
>> + "-mno-speculate-indirect-jumps");
> 
> Wow that is a stern warning :-)
> 
>> --- gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-1.c  (revision 
>> 257367)
>> +++ gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-1.c  (working copy)
>> @@ -1,5 +1,6 @@
>> /* { dg-do compile } */
>> /* { dg-additional-options "-mno-speculate-indirect-jumps" } */
>> +/* { dg-warning "'-mno-speculate-indirect-jumps' is deprecated" "" { target 
>> *-*-* } 0 } */
> 
> Just
> 
> /* { dg-warning "'-mno-speculate-indirect-jumps' is deprecated" } */
> 
> will do I think?

Unfortunately not -- I need to specify line 0, and without this we fail because 
the warning gets associated with line 3.

Thanks for the review!

Bill
> 
> (Some tests use . instead of ' but the latter works as well).
> 
> You could just add "-w" to the options instead.
> 
> Okay for trunk with or without that changed.  Okay for 7 too.  Thanks!
> 
> 
> Segher
> 

Re: [PATCH] Fix ICE with CET and -g (PR target/84146)

[Uros Bizjak]

On Tue, Feb 6, 2018 at 3:57 PM, Tsimbalist, Igor V
 wrote:
>> -Original Message-
>> From: Jakub Jelinek [mailto:ja...@redhat.com]
>> Sent: Wednesday, January 31, 2018 9:57 PM
>> To: Uros Bizjak ; Kirill Yukhin
>> 
>> Cc: gcc-patches@gcc.gnu.org; Tsimbalist, Igor V
>> 
>> Subject: [PATCH] Fix ICE with CET and -g (PR target/84146)
>>
>> Hi!
>>
>> We ICE on the following test because rest_of_insert_endbranch
>> separates a setjmp call from the following
>> NOTE_INSN_CALL_ARG_LOCATION
>> that must always immediately follow the call.
>> No other note or debug insn (which aren't around after var-tracking anyway)
>> needs to follow the call, so the loop it was doing is unnecessary, on the
>> other side, we are already at a late state where bb boundaries are fuzzy and
>> we are going to throw away the cfg before doing final.
>>
>> So, all we need is just check if the call is followed by this note and
>> if yes, emit the endbr after it.
>>
>> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> Just in case, ok from CET viewpoint.

LGTM, now that we have review from the author.

Thanks,
Uros.

> Thanks,
> Igor
>
>> 2018-01-31  Jakub Jelinek  
>>
>>   PR target/84146
>>   * config/i386/i386.c (rest_of_insert_endbranch): Only skip
>>   NOTE_INSN_CALL_ARG_LOCATION after a call, not anything else,
>>   and skip it regardless of bb boundaries.  Use CALL_P macro,
>>   don't test INSN_P (insn) together with CALL_P or JUMP_P check
>>   unnecessarily, formatting fix.
>>
>>   * gcc.target/i386/pr84146.c: New test.
>>
>> --- gcc/config/i386/i386.c.jj 2018-01-31 09:26:18.341505667 +0100
>> +++ gcc/config/i386/i386.c2018-01-31 14:13:33.815243832 +0100
>> @@ -2609,31 +2609,27 @@ rest_of_insert_endbranch (void)
>>for (insn = BB_HEAD (bb); insn != NEXT_INSN (BB_END (bb));
>>  insn = NEXT_INSN (insn))
>>   {
>> -   if (INSN_P (insn) && GET_CODE (insn) == CALL_INSN)
>> +   if (CALL_P (insn))
>>   {
>> if (find_reg_note (insn, REG_SETJMP, NULL) == NULL)
>>   continue;
>> /* Generate ENDBRANCH after CALL, which can return more
>> than
>>twice, setjmp-like functions.  */
>>
>> -   /* Skip notes and debug insns that must be next to the
>> -  call insn.  ??? This might skip a lot more than
>> -  that...  ??? Skipping barriers and emitting code
>> -  after them surely looks like a mistake; we probably
>> -  won't ever hit it, for we'll hit BB_END first.  */
>> +   /* Skip notes that must immediately follow the call insn.  */
>> rtx_insn *next_insn = insn;
>> -   while ((next_insn != BB_END (bb))
>> -   && (DEBUG_INSN_P (NEXT_INSN (next_insn))
>> -   || NOTE_P (NEXT_INSN (next_insn))
>> -   || BARRIER_P (NEXT_INSN (next_insn
>> - next_insn = NEXT_INSN (next_insn);
>> +   if (NEXT_INSN (insn)
>> +   && NOTE_P (NEXT_INSN (insn))
>> +   && (NOTE_KIND (NEXT_INSN (insn))
>> +   == NOTE_INSN_CALL_ARG_LOCATION))
>> + next_insn = NEXT_INSN (insn);
>>
>> cet_eb = gen_nop_endbr ();
>> emit_insn_after_setloc (cet_eb, next_insn, INSN_LOCATION
>> (insn));
>> continue;
>>   }
>>
>> -   if (INSN_P (insn) && JUMP_P (insn) && flag_cet_switch)
>> +   if (JUMP_P (insn) && flag_cet_switch)
>>   {
>> rtx target = JUMP_LABEL (insn);
>> if (target == NULL_RTX || ANY_RETURN_P (target))
>> @@ -2668,7 +2664,7 @@ rest_of_insert_endbranch (void)
>> if ((LABEL_P (insn) && LABEL_PRESERVE_P (insn))
>> || (NOTE_P (insn)
>> && NOTE_KIND (insn) == NOTE_INSN_DELETED_LABEL))
>> -/* TODO.  Check /s bit also.  */
>> + /* TODO.  Check /s bit also.  */
>>   {
>> cet_eb = gen_nop_endbr ();
>> emit_insn_after (cet_eb, insn);
>> --- gcc/testsuite/gcc.target/i386/pr84146.c.jj2018-01-31
>> 16:32:28.099929916 +0100
>> +++ gcc/testsuite/gcc.target/i386/pr84146.c   2018-01-31
>> 14:04:17.796122397 +0100
>> @@ -0,0 +1,14 @@
>> +/* PR target/84146 */
>> +/* { dg-do compile } */
>> +/* { dg-options "-O2 -g -mcet -fcf-protection=full" } */
>> +
>> +int __setjmp (void **);
>> +void *buf[64];
>> +
>> +void
>> +foo (void)
>> +{
>> +  __setjmp (buf);
>> +  for (;;)
>> +;
>> +}
>>
>>   Jakub

Re: [PATCH, rs6000] Fix PR83926, ICE using __builtin_vsx_{div,udiv,mul}_2di builtins

[Peter Bergner]

On 2/6/18 10:20 AM, David Edelsohn wrote:
> Do the gen_XXXdi3 calls work if you use SDI iterator instead of GPR
> iterator, as Segher suggested?

Well it works _if_ we use the first patch that changes the gen_*
patterns.  If we go this route, I agree we should use the SDI
iterator instead of GPR.


> Otherwise, this seems like the more correct approach to not conflict
> with the semantics expected by the patterns.

It's up to you and Segher which patch you think is cleaner/more preferable.
The benefit of the gen_* patch is that if any code added in the future
calls those gen_* routines, then they'll work with no changes.  Otherwise,
the new code would have to do something similar to this latest patch.
Kind of a "six of one, half dozen of the other" sort of thing.
I'm fine either way.

Peter

Re: [PATCH, rs6000] Deprecate -mno-speculate-indirect-jumps

[Segher Boessenkool]

Hi Bill,

On Mon, Feb 05, 2018 at 12:28:34PM -0600, Bill Schmidt wrote:
> It's been determined that we won't recommend use of the recently added
> undocumented option -mno-speculate-indirect-jumps.  This patch provides
> a warning indicating that the option is deprecated.  We will leave the
> code in place for potential emergency use (not expected), and plan to
> remove it completely in GCC 9.

> +  /* Deprecate use of -mno-speculate-indirect-jumps.  */
> +  if (!rs6000_speculate_indirect_jumps)
> +warning (0, "%qs is deprecated and not recommended in any circumstances",
> +  "-mno-speculate-indirect-jumps");

Wow that is a stern warning :-)

> --- gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-1.c   (revision 
> 257367)
> +++ gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-1.c   (working copy)
> @@ -1,5 +1,6 @@
>  /* { dg-do compile } */
>  /* { dg-additional-options "-mno-speculate-indirect-jumps" } */
> +/* { dg-warning "'-mno-speculate-indirect-jumps' is deprecated" "" { target 
> *-*-* } 0 } */

Just

/* { dg-warning "'-mno-speculate-indirect-jumps' is deprecated" } */

will do I think?

(Some tests use . instead of ' but the latter works as well).

You could just add "-w" to the options instead.

Okay for trunk with or without that changed.  Okay for 7 too.  Thanks!


Segher

Re: PATCH to fix bogus warning with -Wstringop-truncation -g (PR tree-optimization/84228)

[Martin Sebor]

Ok for trunk, though generally looking at just next stmt is very
fragile, might be
better to look at the strncpy's vuse immediate uses if they are
within the
same basic block and either don't alias with it, or are the store it is
looking for, or something similar.


I guess some
FOR_EACH_IMM_USE_FAST (...)
  {
if (is_gimple_debug (USE_STMT (use_p)))
  continue;
...
would be better.


Jeff and I had a fairly extensive discussion about how best to
handle debug statements.  I had prototyped his suggestion along
these lines but ran into false positives and other difficulties.
The details are here:

  https://gcc.gnu.org/ml/gcc-patches/2017-11/msg00805.html

I thought I ended up just using gsi_next_nondebug() and added
a test for it but I guess I must be misremembering.  The patch
review spanned a couple of months so it's even possible I forgot
to make the change.


I looked though my hard drive for the test I remember writing
for this.  I couldn't find it but from memory here's roughly
what it did:

  char d[3];

  void f (const char *s)
  {
int i;
__builtin_strncpy (d, s, sizeof d);   // false positive with -g
i = 0;
d[sizeof d - 1] = 0;
  }

Martin

Re: [PATCH, rs6000] Fix PR83926, ICE using __builtin_vsx_{div,udiv,mul}_2di builtins

[David Edelsohn]

On Tue, Feb 6, 2018 at 10:43 AM, Peter Bergner  wrote:
> On 2/6/18 6:42 AM, Peter Bergner wrote:
>> On 2/5/18 10:48 PM, David Edelsohn wrote:
>>> On Mon, Feb 5, 2018 at 9:43 PM, Peter Bergner  wrote:
 I did also try calling expand_divmod() here which did generate correct
 code, the problem was that it wasn't as clean/optimized as the change
 to gen_divdi3.
>>>
>>> Why not fix it at the site of the call to gen_divdi3 instead of the
>>> divdi3 pattern?
>>
>> Well as I said above, I did try that and we got worse code.  That said,
>> I unconditionally called expand_divmod() instead of calling gen_divdi3()
>> when we can (TARGET_POWERPC64).  Let me retry with that change and see
>> what kind of code gen we get.
>
> Ok, calling expand_divmod() in the !TARGET_POWERPC64 case still generates
> worse code.  However, if I instead explicitly generate the call to the
> div/udiv lib functions, then I seem to get the same code as the gen_*
> patch gave...at least for my unit tests I've been using.  Let me
> bootstrap/regtest the following, which I assume you're more happy with?

Do the gen_XXXdi3 calls work if you use SDI iterator instead of GPR
iterator, as Segher suggested?

Otherwise, this seems like the more correct approach to not conflict
with the semantics expected by the patterns.

We'll see what Segher has to say.

Thanks, David

PATCH to fix ICE with -Wstringop-overflow and VLA (PR tree-optimization/84238)

[Marek Polacek]

Here we ICE because get_range_strlen's result might not be an array of
two integer constants -- for a VLA the array might contain a non-constant.
So beef up the check before converting to wide_int.

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

2018-02-06  Marek Polacek  

PR tree-optimization/84238
* tree-ssa-strlen.c (maybe_diag_stxncpy_trunc): Verify the result of
get_range_strlen.

* gcc.dg/Wstringop-overflow-3.c: New test.

diff --git gcc/testsuite/gcc.dg/Wstringop-overflow-3.c 
gcc/testsuite/gcc.dg/Wstringop-overflow-3.c
index e69de29bb2d..8e2531f6b4e 100644
--- gcc/testsuite/gcc.dg/Wstringop-overflow-3.c
+++ gcc/testsuite/gcc.dg/Wstringop-overflow-3.c
@@ -0,0 +1,13 @@
+/* PR tree-optimization/84238 */
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+char a[1];
+int b;
+char *strncpy (char *, char *, __SIZE_TYPE__);
+void
+c ()
+{
+  char d[b];
+  strncpy (a, [3], 3); /* { dg-warning "writing 3 bytes into a region of 
size 1" } */
+}
diff --git gcc/tree-ssa-strlen.c gcc/tree-ssa-strlen.c
index f0f6535017b..94ed2bedc03 100644
--- gcc/tree-ssa-strlen.c
+++ gcc/tree-ssa-strlen.c
@@ -1899,7 +1899,10 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi, tree 
src, tree cnt)
 {
   tree range[2];
   get_range_strlen (src, range);
-  if (range[0])
+  if (range[0] != NULL_TREE
+ && TREE_CODE (range[0]) == INTEGER_CST
+ && range[1] != NULL_TREE
+ && TREE_CODE (range[1]) == INTEGER_CST)
{
  lenrange[0] = wi::to_wide (range[0], prec);
  lenrange[1] = wi::to_wide (range[1], prec);

Marek

[PATCH, committed] PR target/81084: Hide more irrelevant powerpcspe options

[Andrew Jenner]

I have committed this patch to suppress some more irrelevant options 
from the output of "gcc --target-help" for the powerpcspe backend.


I have tested this by rebuilding GCC and checking the output of "gcc 
--target-help".



2018-02-02  Andrew Jenner  

* config/powerpcspe/powerpcspe.opt: (msimple-fpu, mfpu) Add 
Undocumented.

* config/powerpcspe/sysv4.opt (mbit-align): Likewise.
Index: gcc/config/powerpcspe/powerpcspe.opt
===
--- gcc/config/powerpcspe/powerpcspe.opt(revision 257416)
+++ gcc/config/powerpcspe/powerpcspe.opt(working copy)
@@ -516,11 +516,11 @@ Target RejectNegative Var(rs6000_double_
 Double-precision floating point unit.
 
 msimple-fpu
-Target RejectNegative Var(rs6000_simple_fpu) Save
+Target Undocumented RejectNegative Var(rs6000_simple_fpu) Save
 Floating point unit does not support divide & sqrt.
 
 mfpu=
-Target RejectNegative Joined Enum(fpu_type_t) Var(rs6000_fpu_type) 
Init(FPU_NONE)
+Target Undocumented RejectNegative Joined Enum(fpu_type_t) 
Var(rs6000_fpu_type) Init(FPU_NONE)
 -mfpu= Specify FP (sp, dp, sp-lite, dp-lite) (implies -mxilinx-fpu).
 
 Enum
Index: gcc/config/powerpcspe/sysv4.opt
===
--- gcc/config/powerpcspe/sysv4.opt (revision 257416)
+++ gcc/config/powerpcspe/sysv4.opt (working copy)
@@ -44,7 +44,7 @@ EnumValue
 Enum(rs6000_tls_size) String(64) Value(64)
 
 mbit-align
-Target Report Var(TARGET_NO_BITFIELD_TYPE) Save
+Target Undocumented Report Var(TARGET_NO_BITFIELD_TYPE) Save
 Align to the base type of the bit-field.
 
 mstrict-align

Re: [PATCH]] PR target/81084: Fork documentation for powerpcspe and clean up

[Andrew Jenner]

On 02/02/2018 21:29, Joseph Myers wrote:

On Fri, 2 Feb 2018, Andrew Jenner wrote:


This patch adds a section to invoke.texi for the new PowerPC SPE backend,
mostly copied from the PowerPC backend but with irrelevant options removed.


I think a lot of the remaining options are also irrelevant to powerpcspe.


I have updated the patch to remove -msimple-fpu, -mfpu=, 
-m(no-)bit-align and -m(no-)float128-hardware. I'm aware there may be 
others - I'll remove them when I figure out what they are.



The patch also removes documentation of the SPE-specific options -mspe,
-mno-spe and -mfloat-gprs from the PowerPC backend (these options have already
been removed from the PowerPC backend's code).


It also seems to remove mention of -mcpu=8540, but that's still perfectly
meaningful for the PowerPC back end - it's just limited to soft-float
there.


Ah, good point. My updated patch restores that as well.

As before, I have built GCC with these changes and inspected the 
generated html files.


Okay to commit to trunk?

Thanks,

Andrew

	* doc/invoke.texi: Add section for the PowerPC SPE backend. Remove 
irrelevant options.
Index: gcc/doc/invoke.texi
===
--- gcc/doc/invoke.texi (revision 257342)
+++ gcc/doc/invoke.texi (working copy)
@@ -984,6 +984,46 @@ Objective-C and Objective-C++ Dialects}.
 @emph{PowerPC Options}
 See RS/6000 and PowerPC Options.
 
+@emph{PowerPC SPE Options}
+@gccoptlist{-mcpu=@var{cpu-type} @gol
+-mtune=@var{cpu-type} @gol
+-mmfcrf  -mno-mfcrf  -mpopcntb  -mno-popcntb @gol
+-mfull-toc   -mminimal-toc  -mno-fp-in-toc  -mno-sum-in-toc @gol
+-m32  -mxl-compat  -mno-xl-compat @gol
+-malign-power  -malign-natural @gol
+-msoft-float  -mhard-float  -mmultiple  -mno-multiple @gol
+-msingle-float  -mdouble-float @gol
+-mupdate  -mno-update @gol
+-mavoid-indexed-addresses  -mno-avoid-indexed-addresses @gol
+-mstrict-align  -mno-strict-align  -mrelocatable @gol
+-mno-relocatable  -mrelocatable-lib  -mno-relocatable-lib @gol
+-mtoc  -mno-toc  -mlittle  -mlittle-endian  -mbig  -mbig-endian @gol
+-msingle-pic-base @gol
+-mprioritize-restricted-insns=@var{priority} @gol
+-msched-costly-dep=@var{dependence_type} @gol
+-minsert-sched-nops=@var{scheme} @gol
+-mcall-sysv  -mcall-netbsd @gol
+-maix-struct-return  -msvr4-struct-return @gol
+-mabi=@var{abi-type}  -msecure-plt  -mbss-plt @gol
+-mblock-move-inline-limit=@var{num} @gol
+-misel  -mno-isel @gol
+-misel=yes  -misel=no @gol
+-mspe  -mno-spe @gol
+-mspe=yes  -mspe=no @gol
+-mfloat-gprs=yes  -mfloat-gprs=no  -mfloat-gprs=single  -mfloat-gprs=double 
@gol
+-mprototype  -mno-prototype @gol
+-msim  -mmvme  -mads  -myellowknife  -memb  -msdata @gol
+-msdata=@var{opt}  -mvxworks  -G @var{num} @gol
+-mrecip  -mrecip=@var{opt}  -mno-recip  -mrecip-precision @gol
+-mno-recip-precision @gol
+-mpointers-to-nested-functions  -mno-pointers-to-nested-functions @gol
+-msave-toc-indirect  -mno-save-toc-indirect @gol
+-mcompat-align-parm  -mno-compat-align-parm @gol
+-mfloat128  -mno-float128 @gol
+-mgnu-attribute  -mno-gnu-attribute @gol
+-mstack-protector-guard=@var{guard} -mstack-protector-guard-reg=@var{reg} @gol
+-mstack-protector-guard-offset=@var{offset}}
+
 @emph{RISC-V Options}
 @gccoptlist{-mbranch-cost=@var{N-instruction} @gol
 -mplt  -mno-plt @gol
@@ -1036,13 +1076,10 @@ See RS/6000 and PowerPC Options.
 -mblock-move-inline-limit=@var{num} @gol
 -misel  -mno-isel @gol
 -misel=yes  -misel=no @gol
--mspe  -mno-spe @gol
--mspe=yes  -mspe=no @gol
 -mpaired @gol
 -mvrsave  -mno-vrsave @gol
 -mmulhw  -mno-mulhw @gol
 -mdlmzb  -mno-dlmzb @gol
--mfloat-gprs=yes  -mfloat-gprs=no  -mfloat-gprs=single  -mfloat-gprs=double 
@gol
 -mprototype  -mno-prototype @gol
 -msim  -mmvme  -mads  -myellowknife  -memb  -msdata @gol
 -msdata=@var{opt}  -mvxworks  -G @var{num} @gol
@@ -14354,6 +14391,7 @@ platform.
 * PDP-11 Options::
 * picoChip Options::
 * PowerPC Options::
+* PowerPC SPE Options::
 * RISC-V Options::
 * RL78 Options::
 * RS/6000 and PowerPC Options::
@@ -22026,6 +22064,804 @@ these warnings.
 
 These are listed under @xref{RS/6000 and PowerPC Options}.
 
+@node PowerPC SPE Options
+@subsection PowerPC SPE Options
+@cindex PowerPC SPE options
+
+These @samp{-m} options are defined for PowerPC SPE:
+@table @gcctabopt
+@item -mmfcrf
+@itemx -mno-mfcrf
+@itemx -mpopcntb
+@itemx -mno-popcntb
+@opindex mmfcrf
+@opindex mno-mfcrf
+@opindex mpopcntb
+@opindex mno-popcntb
+You use these options to specify which instructions are available on the
+processor you are using.  The default value of these options is
+determined when configuring GCC@.  Specifying the
+@option{-mcpu=@var{cpu_type}} overrides the specification of these
+options.  We recommend you use the @option{-mcpu=@var{cpu_type}} option
+rather than the options listed above.
+
+The @option{-mmfcrf} option allows GCC to generate the move from
+condition register field instruction implemented on the POWER4
+processor and other processors that 

Re: PATCH to fix bogus warning with -Wstringop-truncation -g (PR tree-optimization/84228)

[Martin Sebor]

On 02/06/2018 06:23 AM, Marek Polacek wrote:

On Tue, Feb 06, 2018 at 01:57:36PM +0100, Jakub Jelinek wrote:

On Tue, Feb 06, 2018 at 01:46:21PM +0100, Marek Polacek wrote:

--- gcc/testsuite/c-c++-common/Wstringop-truncation-3.c
+++ gcc/testsuite/c-c++-common/Wstringop-truncation-3.c
@@ -0,0 +1,20 @@
+/* PR tree-optimization/84228 */
+/* { dg-do compile } */
+/* { dg-options "-Wstringop-truncation -O2 -g" } */
+
+char *strncpy (char *, const char *, __SIZE_TYPE__);
+struct S
+{
+  char arr[64];
+};
+
+int
+foo (struct S *p1, const char *a)
+{
+  if (a)
+goto err;
+  strncpy (p1->arr, a, sizeof p1->arr); /* { dg-bogus "specified bound" } */


Just curious, what debug stmt is in between those?


Just

  strncpy (_1, 0B, 64);
  # DEBUG BEGIN_STMT
  p1_5(D)->arr[3] = 0;


Wouldn't it be better to force them a couple of debug stmts?
Say
  int b = 5, c = 6, d = 7;
at the start of the function and
  b = 8; c = 9; d = 10;
in between strncpy and the '\0' store?


Yep.  I tweaked the testcase.  Now we have

  strncpy (_1, 0B, 64);
  # DEBUG BEGIN_STMT
  # DEBUG b => 8
  # DEBUG BEGIN_STMT
  # DEBUG c => 9
  # DEBUG BEGIN_STMT
  # DEBUG d => 10
  # DEBUG BEGIN_STMT
  p1_5(D)->arr[3] = 0;


+  p1->arr[3] = '\0';
+err:
+  return 0;
+}
diff --git gcc/tree-ssa-strlen.c gcc/tree-ssa-strlen.c
index c3cf432a921..f0f6535017b 100644
--- gcc/tree-ssa-strlen.c
+++ gcc/tree-ssa-strlen.c
@@ -1849,7 +1849,7 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi, tree 
src, tree cnt)

   /* Look for dst[i] = '\0'; after the stxncpy() call and if found
  avoid the truncation warning.  */
-  gsi_next ();
+  gsi_next_nondebug ();
   gimple *next_stmt = gsi_stmt (gsi);

   if (!gsi_end_p (gsi) && is_gimple_assign (next_stmt))


Ok for trunk, though generally looking at just next stmt is very fragile, might 
be
better to look at the strncpy's vuse immediate uses if they are within the
same basic block and either don't alias with it, or are the store it is
looking for, or something similar.


I guess some
FOR_EACH_IMM_USE_FAST (...)
  {
if (is_gimple_debug (USE_STMT (use_p)))
  continue;
...
would be better.


Jeff and I had a fairly extensive discussion about how best to
handle debug statements.  I had prototyped his suggestion along
these lines but ran into false positives and other difficulties.
The details are here:

  https://gcc.gnu.org/ml/gcc-patches/2017-11/msg00805.html

I thought I ended up just using gsi_next_nondebug() and added
a test for it but I guess I must be misremembering.  The patch
review spanned a couple of months so it's even possible I forgot
to make the change.

Martin




Thanks,

2018-02-06  Marek Polacek  

PR tree-optimization/84228
* tree-ssa-strlen.c (maybe_diag_stxncpy_trunc): Skip debug statements.

* c-c++-common/Wstringop-truncation-3.c: New test.

diff --git gcc/testsuite/c-c++-common/Wstringop-truncation-3.c 
gcc/testsuite/c-c++-common/Wstringop-truncation-3.c
index e69de29bb2d..ba6b7de094b 100644
--- gcc/testsuite/c-c++-common/Wstringop-truncation-3.c
+++ gcc/testsuite/c-c++-common/Wstringop-truncation-3.c
@@ -0,0 +1,22 @@
+/* PR tree-optimization/84228 */
+/* { dg-do compile } */
+/* { dg-options "-Wstringop-truncation -O2 -g" } */
+
+char *strncpy (char *, const char *, __SIZE_TYPE__);
+struct S
+{
+  char arr[64];
+};
+
+int
+foo (struct S *p1, const char *a)
+{
+  int b = 5, c = 6, d = 7;
+  if (a)
+goto err;
+  strncpy (p1->arr, a, sizeof p1->arr); /* { dg-bogus "specified bound" } */
+  b = 8; c = 9; d = 10;
+  p1->arr[3] = '\0';
+err:
+  return 0;
+}
diff --git gcc/tree-ssa-strlen.c gcc/tree-ssa-strlen.c
index c3cf432a921..f0f6535017b 100644
--- gcc/tree-ssa-strlen.c
+++ gcc/tree-ssa-strlen.c
@@ -1849,7 +1849,7 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi, tree 
src, tree cnt)

   /* Look for dst[i] = '\0'; after the stxncpy() call and if found
  avoid the truncation warning.  */
-  gsi_next ();
+  gsi_next_nondebug ();
   gimple *next_stmt = gsi_stmt (gsi);

   if (!gsi_end_p (gsi) && is_gimple_assign (next_stmt))

Marek

Re: PR tree-optimization/84225: do not pass non-integers to operation_no_trapping_overflow

[Jakub Jelinek]

On Tue, Feb 06, 2018 at 10:43:21AM -0500, Aldy Hernandez wrote:
> In this PR we are ICEing here:
> 
> bool
> operation_no_trapping_overflow (tree type, enum tree_code code)
> {
>   gcc_checking_assert (ANY_INTEGRAL_TYPE_P (type));
> 
> ...because we are being passed a pointer type from find_trapping_overflow.
> 
> Fixed by avoiding passing non-integrals from find_trapping_overflow.
> 
> Pre-approved by Jakub.  Committed.
> 
> Tested on x86-64 Linux.

> gcc/
> 
>   PR tree-optimization/84225
>   * tree-eh.c (find_trapping_overflow): Only call
>   operation_no_trapping_overflow when ANY_INTEGRAL_TYPE_P.

Can you please add a testcase too?
While the reported ICE was on an existing testcase, it was with
non-standard options on it.
So e.g.
gcc.dg/pr84225.c
that would contain:

/* PR tree-optimization/84225 */
/* { dg-do compile { target int32plus } } */
/* { dg-options "-Ofast -ftrapv" } */

#include "torture/pr69714.c"

or so should hopefully do the job, just test with cc1 before and after this
change to verify it ICEd and doesn't ICE anymore.

Jakub

Re: [PATCH, rs6000] Add builtin support for vec_insert4b, vec_extract4b

[Segher Boessenkool]

Hi  Carl,

On Thu, Feb 01, 2018 at 11:31:55AM -0800, Carl Love wrote:
> The following patch contains fixes for the builtins to add and extract
> a word from a char vector.  The existing names for the builtins that do
> this are not consistent with the ABI.  Additionally, the supported
> arguments are not all consistent with the ABI.  This patch fixes the
> support for the insert and extract word builtins so they are consistent
> with the "64-Bit ELF V2 ABI Specification".

The patch is hard to review because it does multiple things at once.
Would have been easier if you first add the new builtins and then delete
the old one.  But I'll try :-)

> Let me know if the patch looks OK or not.  Let me know if you want to
> include it in stage 4 or wait for the next release.  Thanks.

It should also go to GCC 7, right?

> 2018-01-31  Carl Love  
> 
>   * config/rs6000/altivec.h: Change vec_vextract4b to vec_extract4b

You have a tab before vec_extract4b there.

>   and vec_vinsert4b to vec_insert4b.
>   * config/rs6000/rs6000-builtin.def: Remove VEXTRACT4B, VINSERT4B
>   and VINSERT4B_DI definitions. Add INSERT4B and  EXTRACT4B definitions.

Two spaces after dot (numerous times, this is the first one).  You have
a tab before EXTRACT4B.

* config/rs6000/rs6000-builtin.def (VEXTRACT4B, VINSERT4B): Delete.
(EXTRACT4B, INSERT4B): New.

(And similar to all below).

>   *doc/extend.texi: Remove documentation for the vec_vextract4b. Fix
>   documentation for vec_insert4b.  Add documentation for vec_extract4b.

Space after *.

> 2018-01-31  Carl Love  
>   * gcc.target/powerpc/builtins-7-p9-runnable.c: New runnable test file.
>   * gcc.target/powerpc/p9_vinsert4b-1.c: Remove file.
>   * gcc.target/powerpc/p9_vinsert4b-2.c: Remove file.

The files are called p9-vinsert* in fact.  Is all what they tested now in
builtins-7-p9-runnable.c ?

> +(define_insn "extract4b"
> +  [(set (match_operand:V2DI 0 "vsx_register_operand")
> + (unspec:V2DI [(match_operand:V16QI 1 "vsx_register_operand" "wa")
> +   (match_operand:QI 2 "const_0_to_12_operand" "n")]
> +  UNSPEC_XXEXTRACTUW))]
>"TARGET_P9_VECTOR"
>  {
>if (!VECTOR_ELT_ORDER_BIG)
>  operands[2] = GEN_INT (12 - INTVAL (operands[2]));
>  
> +  return "xxextractuw %0,%1,%2";
> +})

You need %x0 and %x1 here I think?

> -vector signed char vec_insert4b (vector int, vector signed char, const int);
> +vector unsigned char vec_insert4b (vector int, vector unsigned char,
> +   const signed int);

Maybe just write "int" instead of "signed int"?

> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/builtins-7-p9-runnable.c
> @@ -0,0 +1,168 @@
> +/* { dg-do run { target { powerpc64*-*-* && p9vector_hw } } } */

As always: why powerpc64* instead of powerpc*?


Segher

Re: [PATCH, rs6000] Fix PR83926, ICE using __builtin_vsx_{div,udiv,mul}_2di builtins

[Peter Bergner]

On 2/6/18 6:42 AM, Peter Bergner wrote:
> On 2/5/18 10:48 PM, David Edelsohn wrote:
>> On Mon, Feb 5, 2018 at 9:43 PM, Peter Bergner  wrote:
>>> I did also try calling expand_divmod() here which did generate correct
>>> code, the problem was that it wasn't as clean/optimized as the change
>>> to gen_divdi3.
>>
>> Why not fix it at the site of the call to gen_divdi3 instead of the
>> divdi3 pattern?
> 
> Well as I said above, I did try that and we got worse code.  That said,
> I unconditionally called expand_divmod() instead of calling gen_divdi3()
> when we can (TARGET_POWERPC64).  Let me retry with that change and see
> what kind of code gen we get.

Ok, calling expand_divmod() in the !TARGET_POWERPC64 case still generates
worse code.  However, if I instead explicitly generate the call to the
div/udiv lib functions, then I seem to get the same code as the gen_*
patch gave...at least for my unit tests I've been using.  Let me
bootstrap/regtest the following, which I assume you're more happy with?

Peter

gcc/
PR target/83926
* config/rs6000/vsx.md (vsx_mul_v2di): Handle generating a 64-bit
multiply in 32-bit mode.
(vsx_div_v2di): Handle generating a 64-bit signed divide in 32-bit mode.
(vsx_udiv_v2di): Handle generating a 64-bit unsigned divide in 32-bit
mode.

gcc/testsuite/
PR target/83926
* gcc.target/powerpc/pr83926.c: New test.

Index: vsx.md
===
--- vsx.md  (revision 257390)
+++ vsx.md  (working copy)
@@ -1650,10 +1650,22 @@
   rtx op5 = gen_reg_rtx (DImode);
   emit_insn (gen_vsx_extract_v2di (op3, op1, GEN_INT (0)));
   emit_insn (gen_vsx_extract_v2di (op4, op2, GEN_INT (0)));
-  emit_insn (gen_muldi3 (op5, op3, op4));
+  if (TARGET_POWERPC64)
+emit_insn (gen_muldi3 (op5, op3, op4));
+  else
+{
+  rtx ret = expand_mult (DImode, op3, op4, NULL, 0, false);
+  emit_move_insn (op5, ret);
+}
   emit_insn (gen_vsx_extract_v2di (op3, op1, GEN_INT (1)));
   emit_insn (gen_vsx_extract_v2di (op4, op2, GEN_INT (1)));
-  emit_insn (gen_muldi3 (op3, op3, op4));
+  if (TARGET_POWERPC64)
+emit_insn (gen_muldi3 (op3, op3, op4));
+  else
+{
+  rtx ret = expand_mult (DImode, op3, op4, NULL, 0, false);
+  emit_move_insn (op3, ret);
+}
   emit_insn (gen_vsx_concat_v2di (op0, op5, op3));
   DONE;
 }"
@@ -1688,10 +1700,30 @@
   rtx op5 = gen_reg_rtx (DImode);
   emit_insn (gen_vsx_extract_v2di (op3, op1, GEN_INT (0)));
   emit_insn (gen_vsx_extract_v2di (op4, op2, GEN_INT (0)));
-  emit_insn (gen_divdi3 (op5, op3, op4));
+  if (TARGET_POWERPC64)
+emit_insn (gen_divdi3 (op5, op3, op4));
+  else
+{
+  rtx libfunc = optab_libfunc (sdiv_optab, DImode);
+  rtx target = emit_library_call_value (libfunc,
+   op5, LCT_NORMAL, DImode,
+   op3, DImode,
+   op4, DImode);
+  emit_move_insn (op5, target);
+}
   emit_insn (gen_vsx_extract_v2di (op3, op1, GEN_INT (1)));
   emit_insn (gen_vsx_extract_v2di (op4, op2, GEN_INT (1)));
-  emit_insn (gen_divdi3 (op3, op3, op4));
+  if (TARGET_POWERPC64)
+emit_insn (gen_divdi3 (op3, op3, op4));
+  else
+{
+  rtx libfunc = optab_libfunc (sdiv_optab, DImode);
+  rtx target = emit_library_call_value (libfunc,
+   op3, LCT_NORMAL, DImode,
+   op3, DImode,
+   op4, DImode);
+  emit_move_insn (op3, target);
+}
   emit_insn (gen_vsx_concat_v2di (op0, op5, op3));
   DONE;
 }"
@@ -1716,10 +1748,30 @@
   rtx op5 = gen_reg_rtx (DImode);
   emit_insn (gen_vsx_extract_v2di (op3, op1, GEN_INT (0)));
   emit_insn (gen_vsx_extract_v2di (op4, op2, GEN_INT (0)));
-  emit_insn (gen_udivdi3 (op5, op3, op4));
+  if (TARGET_POWERPC64)
+emit_insn (gen_udivdi3 (op5, op3, op4));
+  else
+{
+  rtx libfunc = optab_libfunc (udiv_optab, DImode);
+  rtx target = emit_library_call_value (libfunc,
+   op5, LCT_NORMAL, DImode,
+   op3, DImode,
+   op4, DImode);
+  emit_move_insn (op5, target);
+}
   emit_insn (gen_vsx_extract_v2di (op3, op1, GEN_INT (1)));
   emit_insn (gen_vsx_extract_v2di (op4, op2, GEN_INT (1)));
-  emit_insn (gen_udivdi3 (op3, op3, op4));
+  if (TARGET_POWERPC64)
+emit_insn (gen_udivdi3 (op3, op3, op4));
+  else
+{
+  rtx libfunc = optab_libfunc (udiv_optab, DImode);
+  rtx target = emit_library_call_value (libfunc,
+   op3, LCT_NORMAL, DImode,
+   op3, DImode,
+   op4, DImode);
+  emit_move_insn (op3, target);
+}
   emit_insn 

PR tree-optimization/84225: do not pass non-integers to operation_no_trapping_overflow

[Aldy Hernandez]

In this PR we are ICEing here:

bool
operation_no_trapping_overflow (tree type, enum tree_code code)
{
  gcc_checking_assert (ANY_INTEGRAL_TYPE_P (type));

...because we are being passed a pointer type from find_trapping_overflow.

Fixed by avoiding passing non-integrals from find_trapping_overflow.

Pre-approved by Jakub.  Committed.

Tested on x86-64 Linux.
gcc/

	PR tree-optimization/84225
	* tree-eh.c (find_trapping_overflow): Only call
	operation_no_trapping_overflow when ANY_INTEGRAL_TYPE_P.

diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c
index 75385f7b53f..9862ed9fdda 100644
--- a/gcc/tree-eh.c
+++ b/gcc/tree-eh.c
@@ -2729,6 +2729,7 @@ static tree
 find_trapping_overflow (tree *tp, int *walk_subtrees, void *data)
 {
   if (EXPR_P (*tp)
+  && ANY_INTEGRAL_TYPE_P (TREE_TYPE (*tp))
   && !operation_no_trapping_overflow (TREE_TYPE (*tp), TREE_CODE (*tp)))
 return *tp;
   if (IS_TYPE_OR_DECL_P (*tp)

Re: Go patch committed: Avoid negative zero in float constants

[Ian Lance Taylor]

On Mon, Feb 5, 2018 at 11:26 AM, Ian Lance Taylor  wrote:
> On Mon, Feb 5, 2018 at 10:47 AM, Rainer Orth
>  wrote:
>>
>>> This patch to the Go frontend checks for negative numbers with very
>>> small magnitudes that will round to negative zero, and forces them to
>>> positive zero instead.  This implements the spec clarification in
>>> https://golang.org/cl/14727.  The test is in
>>> https://golang.org/cl/91895.  This fixes golang.org/issue/12621.
>>> Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
>>> to mainline.
>>
>> unfortunately, this broke bootstrap with mpfr 2.4.2, which is still the
>> minimum version documented in install.texi:
>
> Bother.  Thanks for the note.  I've rolled back the patch, as follows.

I have now reimplemented the patch, using only MPFR 2.4.2 API.  This
version is better anyhow.  Committed as follows after testing.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 257413)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-1927b40e59e7c2067ecb03384b331d1be3cb5eea
+02f11a2d5cf0db2c2675c13d92bb69529f2175dd
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: gcc/go/gofrontend/expressions.cc
===
--- gcc/go/gofrontend/expressions.cc(revision 257393)
+++ gcc/go/gofrontend/expressions.cc(working copy)
@@ -16158,10 +16158,16 @@ Numeric_constant::set_float(Type* type,
   this->clear();
   this->classification_ = NC_FLOAT;
   this->type_ = type;
+
   // Numeric constants do not have negative zero values, so remove
   // them here.  They also don't have infinity or NaN values, but we
   // should never see them here.
-  if (mpfr_zero_p(val))
+  int bits = 0;
+  if (type != NULL
+  && type->float_type() != NULL
+  && !type->float_type()->is_abstract())
+bits = type->float_type()->bits();
+  if (Numeric_constant::is_float_neg_zero(val, bits))
 mpfr_init_set_ui(this->u_.float_val, 0, GMP_RNDN);
   else
 mpfr_init_set(this->u_.float_val, val, GMP_RNDN);
@@ -16175,8 +16181,60 @@ Numeric_constant::set_complex(Type* type
   this->clear();
   this->classification_ = NC_COMPLEX;
   this->type_ = type;
+
+  // Avoid negative zero as in set_float.
+  int bits = 0;
+  if (type != NULL
+  && type->complex_type() != NULL
+  && !type->complex_type()->is_abstract())
+bits = type->complex_type()->bits() / 2;
+
+  mpfr_t real;
+  mpfr_init_set(real, mpc_realref(val), GMP_RNDN);
+  if (Numeric_constant::is_float_neg_zero(real, bits))
+mpfr_set_ui(real, 0, GMP_RNDN);
+
+  mpfr_t imag;
+  mpfr_init_set(imag, mpc_imagref(val), GMP_RNDN);
+  if (Numeric_constant::is_float_neg_zero(imag, bits))
+mpfr_set_ui(imag, 0, GMP_RNDN);
+
   mpc_init2(this->u_.complex_val, mpc_precision);
-  mpc_set(this->u_.complex_val, val, MPC_RNDNN);
+  mpc_set_fr_fr(this->u_.complex_val, real, imag, MPC_RNDNN);
+
+  mpfr_clear(real);
+  mpfr_clear(imag);
+}
+
+// Return whether VAL, at a precision of BITS, is a negative zero.
+// BITS may be zero in which case it is ignored.
+
+bool
+Numeric_constant::is_float_neg_zero(const mpfr_t val, int bits)
+{
+  if (!mpfr_signbit(val))
+return false;
+  if (mpfr_zero_p(val))
+return true;
+  mp_exp_t min_exp;
+  switch (bits)
+{
+case 0:
+  return false;
+case 32:
+  // In a denormalized float32 the exponent is -126, and there are
+  // 24 bits of which at least the last must be 1, so the smallest
+  // representable non-zero exponent is -126 - (24 - 1) == -149.
+  min_exp = -149;
+  break;
+case 64:
+  // Minimum exponent is -1022, there are 53 bits.
+  min_exp = -1074;
+  break;
+default:
+  go_unreachable();
+}
+  return mpfr_get_exp(val) < min_exp;
 }
 
 // Get an int value.
Index: gcc/go/gofrontend/expressions.h
===
--- gcc/go/gofrontend/expressions.h (revision 257393)
+++ gcc/go/gofrontend/expressions.h (working copy)
@@ -4220,6 +4220,9 @@ class Numeric_constant
   bool
   check_complex_type(Complex_type*, bool, Location);
 
+  static bool
+  is_float_neg_zero(const mpfr_t, int bits);
+
   // The kinds of constants.
   enum Classification
   {

libgo patch committed: Correct runtime structfield type to match reflect

[Ian Lance Taylor]

This libgo patch corrects the runtime structfield type to match the
reflect type, which is the one actually generated by the compiler.
The offset field in structfield has changed to offsetAnon, and now
requires a shift to get the actual offset value.  This fixes
https://golang.org/issue/23391.  Bootstrapped and ran Go testsuite on
x86_64-pc-linux-gnu.  Committed to mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 257393)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-c02c71187c9794b50444e2858c582e66a3442ee8
+1927b40e59e7c2067ecb03384b331d1be3cb5eea
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: libgo/go/runtime/cgocall.go
===
--- libgo/go/runtime/cgocall.go (revision 257357)
+++ libgo/go/runtime/cgocall.go (working copy)
@@ -189,7 +189,7 @@ func cgoCheckArg(t *_type, p unsafe.Poin
return
}
for _, f := range st.fields {
-   cgoCheckArg(f.typ, add(p, f.offset), true, top, msg)
+   cgoCheckArg(f.typ, add(p, f.offset()), true, top, msg)
}
case kindPtr, kindUnsafePointer:
if indir {
Index: libgo/go/runtime/type.go
===
--- libgo/go/runtime/type.go(revision 257357)
+++ libgo/go/runtime/type.go(working copy)
@@ -113,11 +113,19 @@ type ptrtype struct {
 }
 
 type structfield struct {
-   name*string // nil for embedded fields
-   pkgPath *string // nil for exported Names; otherwise import path
-   typ *_type  // type of field
-   tag *string // nil if no tag
-   offset  uintptr // byte offset of field within struct
+   name   *string // nil for embedded fields
+   pkgPath*string // nil for exported Names; otherwise import path
+   typ*_type  // type of field
+   tag*string // nil if no tag
+   offsetAnon uintptr // byte offset of field<<1 | isAnonymous
+}
+
+func (f *structfield) offset() uintptr {
+   return f.offsetAnon >> 1
+}
+
+func (f *structfield) anon() bool {
+   return f.offsetAnon&1 != 0
 }
 
 type structtype struct {

[PATCH, v2] Recognize a missed usage of a sbfiz instruction

[Luis Machado]

Thanks for the feedback Kyrill. I've adjusted the v2 patch based on your
suggestions and re-tested the changes. Everything is still sane.

Since this is ARM-specific and fairly specific, i wonder if it would be
reasonable to consider it for inclusion at the current stage.

Regards,
Luis

Changes in v2:

- Added more restrictive predicates to operands 2, 3 and 4.
- Removed pattern conditional.
- Switched to long long for 64-bit signed integer for the testcase.

---

A customer reported the following missed opportunities to combine a couple
instructions into a sbfiz.

int sbfiz32 (int x)
{
  return x << 29 >> 10;
}

long long sbfiz64 (long long x)
{
  return x << 58 >> 20;
}

This gets converted to the following pattern:

(set (reg:SI 98)
(ashift:SI (sign_extend:SI (reg:HI 0 x0 [ xD.3334 ]))
(const_int 6 [0x6])))

Currently, gcc generates the following:

sbfiz32:
lsl x0, x0, 29
asr x0, x0, 10
ret

sbfiz64:
lsl x0, x0, 58
asr x0, x0, 20
ret

It could generate this instead:

sbfiz32:
sbfiz   w0, w0, 19, 3
ret

sbfiz64::
sbfiz   x0, x0, 38, 6
ret

The unsigned versions already generate ubfiz for the same code, so the lack of
a sbfiz pattern may have been an oversight.

This particular sbfiz pattern shows up in both CPU2006 (~ 80 hits) and
CPU2017 (~ 280 hits). It's not a lot, but seems beneficial in any case. No
significant performance differences, probably due to the small number of
occurrences.

2018-02-06  Luis Machado  

gcc/
* config/aarch64/aarch64.md (*ashift_extv_bfiz): New pattern.

2018-02-06  Luis Machado  

gcc/testsuite/
* gcc.target/aarch64/lsl_asr_sbfiz.c: New test.
---
 gcc/config/aarch64/aarch64.md| 13 +
 gcc/testsuite/gcc.target/aarch64/lsl_asr_sbfiz.c | 24 
 2 files changed, 37 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/aarch64/lsl_asr_sbfiz.c

diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 5a2a930..e8284ae 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -4828,6 +4828,19 @@
   [(set_attr "type" "bfx")]
 )
 
+;; Match sbfiz pattern in a shift left + shift right operation.
+
+(define_insn "*ashift_extv_bfiz"
+  [(set (match_operand:GPI 0 "register_operand" "=r")
+   (ashift:GPI (sign_extract:GPI (match_operand:GPI 1 "register_operand" 
"r")
+ (match_operand 2 
"aarch64_simd_shift_imm_offset_" "n")
+ (match_operand 3 
"aarch64_simd_shift_imm_" "n"))
+(match_operand 4 "aarch64_simd_shift_imm_" "n")))]
+  ""
+  "sbfiz\\t%0, %1, %4, %2"
+  [(set_attr "type" "bfx")]
+)
+
 ;; When the bit position and width of the equivalent extraction add up to 32
 ;; we can use a W-reg LSL instruction taking advantage of the implicit
 ;; zero-extension of the X-reg.
diff --git a/gcc/testsuite/gcc.target/aarch64/lsl_asr_sbfiz.c 
b/gcc/testsuite/gcc.target/aarch64/lsl_asr_sbfiz.c
new file mode 100644
index 000..106433d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/lsl_asr_sbfiz.c
@@ -0,0 +1,24 @@
+/* { dg-do compile } */
+/* { dg-options "-O3" } */
+
+/* Check that a LSL followed by an ASR can be combined into a single SBFIZ
+   instruction.  */
+
+/* Using W-reg */
+
+int
+sbfiz32 (int x)
+{
+  return x << 29 >> 10;
+}
+
+/* Using X-reg */
+
+long long
+sbfiz64 (long long x)
+{
+  return x << 58 >> 20;
+}
+
+/* { dg-final { scan-assembler "sbfiz\tw" } } */
+/* { dg-final { scan-assembler "sbfiz\tx" } } */
-- 
2.7.4

RE: [PATCH] Fix ICE with CET and -g (PR target/84146)

[Tsimbalist, Igor V]

> -Original Message-
> From: Jakub Jelinek [mailto:ja...@redhat.com]
> Sent: Wednesday, January 31, 2018 9:57 PM
> To: Uros Bizjak ; Kirill Yukhin
> 
> Cc: gcc-patches@gcc.gnu.org; Tsimbalist, Igor V
> 
> Subject: [PATCH] Fix ICE with CET and -g (PR target/84146)
> 
> Hi!
> 
> We ICE on the following test because rest_of_insert_endbranch
> separates a setjmp call from the following
> NOTE_INSN_CALL_ARG_LOCATION
> that must always immediately follow the call.
> No other note or debug insn (which aren't around after var-tracking anyway)
> needs to follow the call, so the loop it was doing is unnecessary, on the
> other side, we are already at a late state where bb boundaries are fuzzy and
> we are going to throw away the cfg before doing final.
> 
> So, all we need is just check if the call is followed by this note and
> if yes, emit the endbr after it.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Just in case, ok from CET viewpoint.

Thanks,
Igor

> 2018-01-31  Jakub Jelinek  
> 
>   PR target/84146
>   * config/i386/i386.c (rest_of_insert_endbranch): Only skip
>   NOTE_INSN_CALL_ARG_LOCATION after a call, not anything else,
>   and skip it regardless of bb boundaries.  Use CALL_P macro,
>   don't test INSN_P (insn) together with CALL_P or JUMP_P check
>   unnecessarily, formatting fix.
> 
>   * gcc.target/i386/pr84146.c: New test.
> 
> --- gcc/config/i386/i386.c.jj 2018-01-31 09:26:18.341505667 +0100
> +++ gcc/config/i386/i386.c2018-01-31 14:13:33.815243832 +0100
> @@ -2609,31 +2609,27 @@ rest_of_insert_endbranch (void)
>for (insn = BB_HEAD (bb); insn != NEXT_INSN (BB_END (bb));
>  insn = NEXT_INSN (insn))
>   {
> -   if (INSN_P (insn) && GET_CODE (insn) == CALL_INSN)
> +   if (CALL_P (insn))
>   {
> if (find_reg_note (insn, REG_SETJMP, NULL) == NULL)
>   continue;
> /* Generate ENDBRANCH after CALL, which can return more
> than
>twice, setjmp-like functions.  */
> 
> -   /* Skip notes and debug insns that must be next to the
> -  call insn.  ??? This might skip a lot more than
> -  that...  ??? Skipping barriers and emitting code
> -  after them surely looks like a mistake; we probably
> -  won't ever hit it, for we'll hit BB_END first.  */
> +   /* Skip notes that must immediately follow the call insn.  */
> rtx_insn *next_insn = insn;
> -   while ((next_insn != BB_END (bb))
> -   && (DEBUG_INSN_P (NEXT_INSN (next_insn))
> -   || NOTE_P (NEXT_INSN (next_insn))
> -   || BARRIER_P (NEXT_INSN (next_insn
> - next_insn = NEXT_INSN (next_insn);
> +   if (NEXT_INSN (insn)
> +   && NOTE_P (NEXT_INSN (insn))
> +   && (NOTE_KIND (NEXT_INSN (insn))
> +   == NOTE_INSN_CALL_ARG_LOCATION))
> + next_insn = NEXT_INSN (insn);
> 
> cet_eb = gen_nop_endbr ();
> emit_insn_after_setloc (cet_eb, next_insn, INSN_LOCATION
> (insn));
> continue;
>   }
> 
> -   if (INSN_P (insn) && JUMP_P (insn) && flag_cet_switch)
> +   if (JUMP_P (insn) && flag_cet_switch)
>   {
> rtx target = JUMP_LABEL (insn);
> if (target == NULL_RTX || ANY_RETURN_P (target))
> @@ -2668,7 +2664,7 @@ rest_of_insert_endbranch (void)
> if ((LABEL_P (insn) && LABEL_PRESERVE_P (insn))
> || (NOTE_P (insn)
> && NOTE_KIND (insn) == NOTE_INSN_DELETED_LABEL))
> -/* TODO.  Check /s bit also.  */
> + /* TODO.  Check /s bit also.  */
>   {
> cet_eb = gen_nop_endbr ();
> emit_insn_after (cet_eb, insn);
> --- gcc/testsuite/gcc.target/i386/pr84146.c.jj2018-01-31
> 16:32:28.099929916 +0100
> +++ gcc/testsuite/gcc.target/i386/pr84146.c   2018-01-31
> 14:04:17.796122397 +0100
> @@ -0,0 +1,14 @@
> +/* PR target/84146 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -g -mcet -fcf-protection=full" } */
> +
> +int __setjmp (void **);
> +void *buf[64];
> +
> +void
> +foo (void)
> +{
> +  __setjmp (buf);
> +  for (;;)
> +;
> +}
> 
>   Jakub

Re: PR 84154: Fix checking -mibt and -mshstk options for control flow protection

[Uros Bizjak]

On Tue, Feb 6, 2018 at 3:08 PM, Tsimbalist, Igor V
 wrote:
>> -Original Message-
>> From: Nick Clifton [mailto:ni...@redhat.com]
>> Sent: Tuesday, February 6, 2018 1:16 PM
>> To: Tsimbalist, Igor V ; hjl.to...@gmail.com
>> Cc: gcc-patches@gcc.gnu.org
>> Subject: Re: PR 84154: Fix checking -mibt and -mshstk options for control
>> flow protection
>>
>> Hi Igor,
>>
>> >>   Attached is a potential patch for PR 84145:
>> >>
>> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84145
>>
>> > Coincidentally, I have worked on the same patch.
>>
>> Great minds, etc :-)
>>
>> > Please look at the patch, I uploaded it to the bug. The main differences 
>> > are
>> >
>> > - updated the output messages to be more informative;
>> > - updated  the tests and add couple of new tests to check the messages;
>> > - fixed a typo in the doc file related to fcf-protection;
>> >
>> > I am ok with the changes in i386.c but would like to update the messages.
>> Could you incorporate my changes and proceed? Or would you like me to
>> finish the fix?
>>
>> If you are happy to finish the fix then please do so.  Your fix is
>> more thorough than mine, so I am happy to see it go on.  Although
>> I should say that I am not an x86 maintainer, so I cannot approve
>> it.
>
> Here is the updated patch. Please note the subject should say PR 84145.
>
> Ok for trunk?

LGTM.

Thanks,
Uros.

RE: PR 84154: Fix checking -mibt and -mshstk options for control flow protection

[Tsimbalist, Igor V]

> -Original Message-
> From: Nick Clifton [mailto:ni...@redhat.com]
> Sent: Tuesday, February 6, 2018 1:16 PM
> To: Tsimbalist, Igor V ; hjl.to...@gmail.com
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: PR 84154: Fix checking -mibt and -mshstk options for control
> flow protection
> 
> Hi Igor,
> 
> >>   Attached is a potential patch for PR 84145:
> >>
> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84145
> 
> > Coincidentally, I have worked on the same patch.
> 
> Great minds, etc :-)
> 
> > Please look at the patch, I uploaded it to the bug. The main differences are
> >
> > - updated the output messages to be more informative;
> > - updated  the tests and add couple of new tests to check the messages;
> > - fixed a typo in the doc file related to fcf-protection;
> >
> > I am ok with the changes in i386.c but would like to update the messages.
> Could you incorporate my changes and proceed? Or would you like me to
> finish the fix?
> 
> If you are happy to finish the fix then please do so.  Your fix is
> more thorough than mine, so I am happy to see it go on.  Although
> I should say that I am not an x86 maintainer, so I cannot approve
> it.

Here is the updated patch. Please note the subject should say PR 84145.

Ok for trunk?

> Cheers
>   Nick
> 



0001-Fix-checking-mibt-and-mshstk-options-for-control-flo.patch
Description: 0001-Fix-checking-mibt-and-mshstk-options-for-control-flo.patch

Re: RFA: Sanitize deprecation messages (PR 84195)

[David Malcolm]

On Tue, 2018-02-06 at 12:39 +, Nick Clifton wrote:
> Hi Martin,
> 
> > My only suggestions are to consider how control characters and
> > excessively messages are handled in other contexts and adopt
> > the same approach here.
> 
> Are there other places where user-supplied messages are displayed by
> gcc ?


#pragma GCC warning and error (as opposed to #warning and #error) also
print control chars without escaping:

$cat /tmp/prag.c 
#error "foo\nbar"
#warning "foo\nbar"
#pragma GCC error "foo\nbar"
#pragma GCC warning "foo\nbar"

$ gcc -c /tmp/prag.c
/tmp/prag.c:1:2: error: #error "foo\nbar"
 #error "foo\nbar"
  ^
/tmp/prag.c:2:2: warning: #warning "foo\nbar" [-Wcpp]
 #warning "foo\nbar"
  ^~~
/tmp/prag.c:3:19: error: foo
bar
 #pragma GCC error "foo\nbar"
   ^~
/tmp/prag.c:4:21: warning: foo
bar
 #pragma GCC warning "foo\nbar"
 ^~


Note the newlines in the above output.

There may be other places.

My preference is either the status quo, or to escape the control
characters.

The patch is missing a test case.

> 
> > In the tests I did, control characters
> > were mostly escaped (e.g., as \n or as \x0a) rather than replaced
> > with spaces. 
> 
> Ooo - what tests were these ?
> 
> I agree that it there is a standard way to handle these characters 
> then the deprecated attribute ought to do the same.  (Plus hopefully
> there is already a function in gcc to do this kind of processing,
> so the deprecation code can just call it).

There ought to be; I don't know it off the top of my head.

> > I haven't thought too hard about how excessively
> > long messages should be handled, or about the interactions with
> > -fmessage-length= (i.e., if messages longer than the limit should
> > be broken up.)
> 
> I believe that this will happen automatically.  The diagnostic
> display
> routine will automatically insert newlines into the output if the 
> message length is non-zero, and the message extends past this limit.
> (Well provided that the message contains spaces.  The newlines are
> only inserted in place of space characters so a very long, single
> word message will not be split...)
> 
> Cheers
>   Nick

PING [PATCH] -mjsr option bug fix

[Sebastian Perta]

PING

> -Original Message-
> From: Sebastian Perta [mailto:sebastian.pe...@renesas.com]
> Sent: 08 January 2018 10:57
> To: gcc-patches@gcc.gnu.org
> Subject: [PATCH] -mjsr option bug fix
> 
> Hi,
> 
> The -mjsr option in RX should ensure the that BSR instruction is not
> generated, only JSR instruction should be generated.
> However this does not work as expected: BSR instruction still gets
generated
> even if -mjsr is passed in the command line.
> This is reproducible even if test cases from the gcc testsuite, for
example:
> gcc.c-torture\compile\920625-1.c
> gcc.c-torture\compile\20051216-1.c
> gcc.dg\torture\builtin-explog-1.c
> 
> The following patch fixes this issue by adding a new constraint to
call_internal
> and call_value_internal.
> The patch also contains a test case which I created as follows:
> 1. I copied gcc.c-torture\compile\20051216-1.c  to gcc.target\rx and
renamed
> to mjsr.c
> 2. added the following lines to scan the assembly files for BSR. If BSR is
> present the test fails.
> /* { dg-do compile } */
> /* { dg-options "-O2 -mjsr" } */
> /* { dg-final { scan-assembler-not "bsr" } } */
> 
> Regression test is OK, tested with the following command:
> make -k check-gcc RUNTESTFLAGS=--target_board=rx-sim
> 
> Please let me know if this is OK. Thank you!
> 
> Best Regards,
> Sebastian
> 
> Index: ChangeLog
> ==
> =
> --- ChangeLog (revision 256278)
> +++ ChangeLog (working copy)
> @@ -1,3 +1,10 @@
> +2018-01-05  Sebastian Perta  
> +
> + * config/rx/constraints.md: added new constraint
> CALL_OP_SYMBOL_REF
> + to allow or block "symbol_ref" depending on value of TARGET_JSR
> + * config/rx/rx.md: use CALL_OP_SYMBOL_REF in call_internal and
> + call_value_internal insns
> +
>  2018-01-05  Richard Sandiford  
> 
>   * tree-vect-data-refs.c (vect_compute_data_ref_alignment): Don't
> Index: config/rx/constraints.md
> ==
> =
> --- config/rx/constraints.md  (revision 256278)
> +++ config/rx/constraints.md  (working copy)
> @@ -106,3 +106,9 @@
> )
>)
>  )
> +
> +(define_constraint "CALL_OP_SYMBOL_REF"
> +"constraint for call instructions using symbol ref"
> +(and (match_test "!TARGET_JSR")
> + (match_code "symbol_ref"))
> +)
> Index: config/rx/rx.md
> ==
> =
> --- config/rx/rx.md   (revision 256278)
> +++ config/rx/rx.md   (working copy)
> @@ -438,7 +438,7 @@
>  )
> 
>  (define_insn "call_internal"
> -  [(call (mem:QI (match_operand:SI 0 "rx_call_operand" "r,Symbol"))
> +  [(call (mem:QI (match_operand:SI 0 "rx_call_operand"
> "r,CALL_OP_SYMBOL_REF"))
>(const_int 0))
> (clobber (reg:CC CC_REG))]
>""
> @@ -466,7 +466,7 @@
> 
>  (define_insn "call_value_internal"
>[(set (match_operand  0 "register_operand" "=r,r")
> - (call (mem:QI (match_operand:SI 1 "rx_call_operand"   "r,Symbol"))
> + (call (mem:QI (match_operand:SI 1 "rx_call_operand"
> "r,CALL_OP_SYMBOL_REF"))
> (const_int 0)))
> (clobber (reg:CC CC_REG))]
>""
> Index: testsuite/gcc.target/rx/mjsr.c
> ==
> =
> --- testsuite/gcc.target/rx/mjsr.c(nonexistent)
> +++ testsuite/gcc.target/rx/mjsr.c(working copy)
> @@ -0,0 +1,134 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -mjsr" } */
> +
> +void *malloc (__SIZE_TYPE__);
> +void *realloc (void *, __SIZE_TYPE__);
> +
> +struct A { double x, y; };
> +struct B { double x0, y0, x1, y1; };
> +struct C { int n_points; int dir; struct B bbox; struct A *points; };
> +struct D { int n_segs; struct C segs[1]; };
> +
> +void foo (int, int, int *, int, int *, struct A **, int *, int *,
> +   struct D *, int *, struct D **, int *, int **);
> +int baz (struct A, struct A, struct A, struct A);
> +
> +static void
> +bar (struct D *svp, int *n_points_max,
> + struct A p, int *seg_map, int *active_segs, int i)
> +{
> +  int asi, n_points;
> +  struct C *seg;
> +
> +  asi = seg_map[active_segs[i]];
> +  seg = >segs[asi];
> +  n_points = seg->n_points;
> +  seg->points = ((struct A *)
> + realloc (seg->points, (n_points_max[asi] <<= 1) * sizeof
> (struct A)));
> +  seg->points[n_points] = p;
> +  seg->bbox.y1 = p.y;
> +  seg->n_points++;
> +}
> +
> +struct D *
> +test (struct D *vp)
> +{
> +  int *active_segs, n_active_segs, *cursor, seg_idx;
> +  double y, share_x;
> +  int tmp1, tmp2, asi, i, j, *n_ips, *n_ips_max, n_segs_max;
> +  struct A **ips, p_curs, *pts;
> +  struct D *new_vp;
> +  int *n_points_max, *seg_map, first_share;
> +
> +  n_segs_max = 16;
> +  new_vp = (struct D *) malloc (sizeof (struct D) +
> + (n_segs_max - 1) * sizeof (struct C));
> +  new_vp->n_segs = 0;
> +
> +  if (vp->n_segs == 0)
> +return new_vp;
> +
> +  

PING [PATCH] RX movsicc degrade fix

[Sebastian Perta]

PING

> -Original Message-
> From: Sebastian Perta [mailto:sebastian.pe...@renesas.com]
> Sent: 09 January 2018 14:46
> To: 'gcc-patches@gcc.gnu.org' 
> Subject: [PATCH] RX movsicc degrade fix
> 
> Hello,
> 
> In recent versions of GCC the define_expand "movsicc" has stopped being
> used by GCC (approx. 4.7.x/4.8.x onwards)
> The reason for this is that the first operand of if_then_else has SI mode
and
> it shouldn't have. If we take a look at movsicc for all other targets we
see this
> is true.
> The fix in rx.md is basically a copy paste from v850.md
> 
> The patch also adds a testcase in gcc.target/rx to make sure this degrade
> does not occur again.
> 
> 
> Regression test is OK with one observation (see below), tested with the
> following command:
> make -k check-gcc RUNTESTFLAGS=--target_board=rx-sim
> 
> 
> I have the following fail (which was not present before):
> FAIL: gcc.dg/loop-8.c scan-rtl-dump-times loop2_invariant "Decided" 1
> (found 0 times)
> 
> This is because the patch is effective in this test case and the dump is
not the
> same, I checked the asm code manually and it is OK.
> Is it possible to disable parts of a test case, not the whole test case (*
{ dg-
> final { scan-rtl-dump-times "Decided" 1 "loop2_invariant" } } */  from
loop-8.c
> in this example) for a particular target?
> 
> The total numbers of failures remains the same because the following FAIL
is
> not present anymore with this patch:
> FAIL: gcc.dg/ifcvt-4.c scan-rtl-dump ce1 "2 true changes made"
> 
> 
> Please let me know if this is OK. Thank you!
> 
> Best Regards,
> Sebastian
> 
> 
> Index: ChangeLog
> ==
> =
> --- ChangeLog (revision 256382)
> +++ ChangeLog (working copy)
> @@ -1,3 +1,8 @@
> +2018-01-09  Sebastian Perta  
> +
> + *config/rx.md: updated "movsicc" expand to be matched by GCC
> + *testsuite/gcc.target/rx/movsicc.c: new test case
> +
>  2018-01-09  Richard Biener  
> 
>   PR tree-optimization/83668
> Index: config/rx/rx.md
> ==
> =
> --- config/rx/rx.md   (revision 256382)
> +++ config/rx/rx.md   (working copy)
> @@ -733,12 +733,17 @@
>  (define_expand "movsicc"
>[(parallel
>  [(set (match_operand:SI  0 "register_operand")
> -   (if_then_else:SI (match_operand:SI 1 "comparison_operator")
> +   (if_then_else:SI (match_operand 1 "comparison_operator")
>  (match_operand:SI 2 "nonmemory_operand")
>  (match_operand:SI 3 "nonmemory_operand")))
>   (clobber (reg:CC CC_REG))])]
>""
>  {
> +  /* Make sure that we have an integer comparison...  */
> +  if (GET_MODE (XEXP (operands[1], 0)) != CCmode
> +  && GET_MODE (XEXP (operands[1], 0)) != SImode)
> +FAIL;
> +
>/* One operand must be a constant or a register, the other must be a
> register.  */
>if (   ! CONSTANT_P (operands[2])
>&& ! CONSTANT_P (operands[3])
> Index: testsuite/gcc.target/rx/movsicc.c
> ==
> =
> --- testsuite/gcc.target/rx/movsicc.c (nonexistent)
> +++ testsuite/gcc.target/rx/movsicc.c (working copy)
> @@ -0,0 +1,94 @@
> +/* { dg-do compile } */
> +/* { dg-options "-Os" } */
> +
> +typedef unsigned char u8;
> +typedef unsigned short u16;
> +signed int Xa, Xb;
> +
> +signed int stzreg_beq(int i, int a, int b)
> +{
> +  signed int x;
> +  x = a;
> +  if (i)
> +x = b;
> +  return x;
> +}
> +
> +/* { dg-final { scan-assembler "bne 1f" } } */
> +
> +signed int stzreg_bge(int i, int a, int b, int c)
> +{
> +  signed int x;
> +  x = a;
> +  if (i +x = b;
> +  return x;
> +}
> +
> +/* { dg-final { scan-assembler "blt 1f" } } */
> +
> +signed int stzreg_bgt(int i, int a, int b)
> +{
> +  signed int x;
> +  x = a;
> +  if (i<10)
> +x = b;
> +  return x;
> +}
> +
> +/* { dg-final { scan-assembler "ble 1f" } } */
> +
> +signed int stzreg_ble(int i, int a, int b)
> +{
> +  signed int x;
> +  x = a;
> +  if (i>0)
> +x = b;
> +  return x;
> +}
> +
> +/* { dg-final { scan-assembler "bgt 1f" } } */
> +
> +signed int stzreg_blt(int i, int a, int b)
> +{
> +  signed int x;
> +  x = a;
> +  if (i<0)
> +x = b;
> +  return x;
> +}
> +
> +/* { dg-final { scan-assembler "blt 1f" } } */
> +
> +signed int stzreg_bne(int i, int a, int b)
> +{
> +  signed int x;
> +  x = a;
> +  if (!i)
> +x = b;
> +  return x;
> +}
> +
> +/* { dg-final { scan-assembler "beq 1f" } } */
> +
> +signed int stzimm_le( int i, int a )
> +{
> +  signed int x;
> +  x = a;
> +  if (i>0)
> +x = 5;
> +  return x;
> +}
> +
> +/* { dg-final { scan-assembler "ble 1f" } } */
> +
> +signed int stzimm_le_r( int i, int a )
> +{
> +  signed int x;
> +  x = a;
> +  if (i<0)
> +x = 5;
> +  return x;
> +}
> +
> +/* { dg-final { scan-assembler "bge 1f" } } */

RE: [PATCH] RL78 new "vector" function attribute

[Sebastian Perta]

Hi DJ,

I've updated the patch (extend.texi) as you suggested.
Please let me know if this is OK to check-in, thank you!

Best Regards,
Sebastian

Index: config/rl78/rl78.c
===
--- config/rl78/rl78.c(revision 257142)
+++ config/rl78/rl78.c(working copy)
@@ -809,7 +809,6 @@
 bool * no_add_attrs)
 {
   gcc_assert (DECL_P (* node));
-  gcc_assert (args == NULL_TREE);

   if (TREE_CODE (* node) != FUNCTION_DECL)
 {
@@ -868,6 +867,28 @@
   return NULL_TREE;
 }

+/* Check "vector" attribute.  */
+
+static tree
+rl78_handle_vector_attribute (tree * node,
+tree   name,
+tree   args,
+intflags ATTRIBUTE_UNUSED,
+bool * no_add_attrs)
+{
+  gcc_assert (DECL_P (* node));
+  gcc_assert (args != NULL_TREE);
+
+  if (TREE_CODE (* node) != FUNCTION_DECL)
+{
+  warning (OPT_Wattributes, "%qE attribute only applies to functions",
+   name);
+  * no_add_attrs = true;
+}
+
+  return NULL_TREE;
+}
+
 #undef  TARGET_ATTRIBUTE_TABLE
 #define TARGET_ATTRIBUTE_TABLErl78_attribute_table

@@ -876,7 +897,7 @@
 {
   /* Name, min_len, max_len, decl_req, type_req, fn_type_req,
  affects_type_identity, handler, exclude.  */
-  { "interrupt",  0, 0, true, false, false, false,
+  { "interrupt",  0, -1, true, false, false, false,
 rl78_handle_func_attribute, NULL },
   { "brk_interrupt",  0, 0, true, false, false, false,
 rl78_handle_func_attribute, NULL },
@@ -884,6 +905,8 @@
 rl78_handle_naked_attribute, NULL },
   { "saddr",  0, 0, true, false, false, false,
 rl78_handle_saddr_attribute, NULL },
+  { "vector", 1, -1, true, false, false,
+rl78_handle_vector_attribute, false },
   { NULL, 0, 0, false, false, false, false, NULL, NULL }
 };

@@ -1583,6 +1606,62 @@
 #undef  TARGET_ASM_FUNCTION_PROLOGUE
 #define TARGET_ASM_FUNCTION_PROLOGUErl78_start_function

+static void
+add_vector_labels (FILE *file, const char *aname)
+{
+  tree vec_attr;
+  tree val_attr;
+  const char *vname = "vect";
+  const char *s;
+  int vnum;
+
+  /* This node is for the vector/interrupt tag itself */
+  vec_attr = lookup_attribute (aname, DECL_ATTRIBUTES (current_function_decl));
+  if (!vec_attr)
+return;
+
+  /* Now point it at the first argument */
+  vec_attr = TREE_VALUE (vec_attr);
+
+  /* Iterate through the arguments.  */
+  while (vec_attr)
+{
+  val_attr = TREE_VALUE (vec_attr);
+  switch (TREE_CODE (val_attr))
+{
+case STRING_CST:
+  s = TREE_STRING_POINTER (val_attr);
+  goto string_id_common;
+
+case IDENTIFIER_NODE:
+  s = IDENTIFIER_POINTER (val_attr);
+
+string_id_common:
+  if (strcmp (s, "$default") == 0)
+{
+  fprintf (file, "\t.global\t$tableentry$default$%s\n", vname);
+  fprintf (file, "$tableentry$default$%s:\n", vname);
+}
+  else
+vname = s;
+  break;
+
+case INTEGER_CST:
+  vnum = TREE_INT_CST_LOW (val_attr);
+
+  fprintf (file, "\t.global\t$tableentry$%d$%s\n", vnum, vname);
+  fprintf (file, "$tableentry$%d$%s:\n", vnum, vname);
+  break;
+
+default:
+  ;
+}
+
+  vec_attr = TREE_CHAIN (vec_attr);
+}
+
+}
+
 /* We don't use this to actually emit the function prologue.  We use
this to insert a comment in the asm file describing the
function.  */
@@ -1590,6 +1669,9 @@
 rl78_start_function (FILE *file)
 {
   int i;
+
+  add_vector_labels (file, "interrupt");
+  add_vector_labels (file, "vector");

   if (cfun->machine->framesize == 0)
 return;
Index: doc/extend.texi
===
--- doc/extend.texi(revision 257142)
+++ doc/extend.texi(working copy)
@@ -5182,7 +5182,7 @@
 function entry and exit sequences suitable for use in an interrupt handler
 when this attribute is present.

-On RX targets, you may specify one or more vector numbers as arguments
+On RX and RL78 targets, you may specify one or more vector numbers as arguments
 to the attribute, as well as naming an alternate table name.
 Parameters are handled sequentially, so one handler can be assigned to
 multiple entries in multiple tables.  One may also pass the magic
Index: testsuite/gcc.target/rl78/test_auto_vector.c
===
--- testsuite/gcc.target/rl78/test_auto_vector.c(nonexistent)
+++ testsuite/gcc.target/rl78/test_auto_vector.c(working copy)
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+
+void __attribute__ ((interrupt (5))) interrupt_5_handler ();
+
+void interrupt_5_handler ()
+{
+}
+
+void __attribute__ ((vector (4))) interrupt_4_handler ();
+
+void interrupt_4_handler ()
+{
+}
+
+void __attribute__ ((interrupt)) interrupt_handler ();
+
+void interrupt_handler ()
+{
+}
+
+/* { dg-final { scan-assembler "tableentry" } } */

> -Original Message-
> From: DJ Delorie [mailto:d...@redhat.com]
> Sent: 29 January 2018 21:11
> To: Sebastian Perta 
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH] RL78 new 

Re: Do not lose track of resolution info due to tree merging

[Jan Hubicka]

> 
> I think unify_scc is only ever called from WPA so you can elide the 
> flag_ltrans checks. 
> 
> Otherwise OK. 

Thanks, I have updated this and also dropped the sanity check from symtab.c
because, well, it finds another bugs in target versioning I will handle
incrementally.  I also noticed I need to rule out the check for register
variables and builtins because we do not stream those into the lto symtab.  I
would say that real_symbol_p should return false for register variable and we
should stream bulitins but that is for independent change, too.

I suppose we should backport this to all release branches.

Honza

PR lto/81004
* lto.c: Include builtins.h
(register_resolution): Merge resolutions in case trees was
merged across units.
(lto_maybe_register_decl): Break out from ...
(lto_read_decls): ... here.
(unify_scc): Also register decls here.
(read_cgraph_and_symbols): Sanity check that all resolutions was
read.
Index: lto.c
===
--- lto.c   (revision 257382)
+++ lto.c   (working copy)
@@ -54,6 +54,7 @@ along with GCC; see the file COPYING3.
 #include "stringpool.h"
 #include "fold-const.h"
 #include "attribs.h"
+#include "builtins.h"
 
 
 /* Number of parallel tasks to run, -1 if we want to use GNU Make jobserver.  
*/
@@ -830,12 +831,20 @@ static void
 register_resolution (struct lto_file_decl_data *file_data, tree decl,
 enum ld_plugin_symbol_resolution resolution)
 {
+  bool existed;
   if (resolution == LDPR_UNKNOWN)
 return;
   if (!file_data->resolution_map)
 file_data->resolution_map
   = new hash_map;
-  file_data->resolution_map->put (decl, resolution);
+  ld_plugin_symbol_resolution_t 
+ = file_data->resolution_map->get_or_insert (decl, );
+  gcc_assert (!existed || res == resolution);
+  if (!existed
+  || resolution == LDPR_PREVAILING_DEF_IRONLY
+  || resolution == LDPR_PREVAILING_DEF
+  || resolution == LDPR_PREVAILING_DEF_IRONLY_EXP)
+res = resolution;
 }
 
 /* Register DECL with the global symbol table and change its
@@ -878,6 +887,18 @@ lto_register_function_decl_in_symtab (st
 decl, get_resolution (data_in, ix));
 }
 
+/* Check if T is a decl and needs register its resolution info.  */
+
+static void
+lto_maybe_register_decl (struct data_in *data_in, tree t, unsigned ix)
+{
+  if (TREE_CODE (t) == VAR_DECL)
+lto_register_var_decl_in_symtab (data_in, t, ix);
+  else if (TREE_CODE (t) == FUNCTION_DECL
+  && !DECL_BUILT_IN (t))
+lto_register_function_decl_in_symtab (data_in, t, ix);
+}
+
 
 /* For the type T re-materialize it in the type variant list and
the pointer/reference-to chains.  */
@@ -1617,7 +1638,10 @@ unify_scc (struct data_in *data_in, unsi
  /* Fixup the streamer cache with the prevailing nodes according
 to the tree node mapping computed by compare_tree_sccs.  */
  if (len == 1)
-   streamer_tree_cache_replace_tree (cache, pscc->entries[0], from);
+   {
+ lto_maybe_register_decl (data_in, pscc->entries[0], from);
+ streamer_tree_cache_replace_tree (cache, pscc->entries[0], from);
+   }
  else
{
  tree *map2 = XALLOCAVEC (tree, 2 * len);
@@ -1625,6 +1649,7 @@ unify_scc (struct data_in *data_in, unsi
{
  map2[i*2] = (tree)(uintptr_t)(from + i);
  map2[i*2+1] = scc->entries[i];
+ lto_maybe_register_decl (data_in, scc->entries[i], from + i);
}
  qsort (map2, len, 2 * sizeof (tree), cmp_tree);
  qsort (map, len, 2 * sizeof (tree), cmp_tree);
@@ -1761,13 +1786,7 @@ lto_read_decls (struct lto_file_decl_dat
cache_integer_cst (t);
  if (!flag_ltrans)
{
- /* Register variables and functions with the
-symbol table.  */
- if (TREE_CODE (t) == VAR_DECL)
-   lto_register_var_decl_in_symtab (data_in, t, from + i);
- else if (TREE_CODE (t) == FUNCTION_DECL
-  && !DECL_BUILT_IN (t))
-   lto_register_function_decl_in_symtab (data_in, t, from + i);
+ lto_maybe_register_decl (data_in, t, from + i);
  /* Scan the tree for references to global functions or
 variables and record those for later fixup.  */
  if (mentions_vars_p (t))
@@ -2873,13 +2892,21 @@ read_cgraph_and_symbols (unsigned nfiles
 
   /* Store resolutions into the symbol table.  */
 
-  ld_plugin_symbol_resolution_t *res;
   FOR_EACH_SYMBOL (snode)
-if (snode->real_symbol_p ()
-   && snode->lto_file_data
-   && snode->lto_file_data->resolution_map
-   && (res = snode->lto_file_data->resolution_map->get 

Re: PATCH to fix bogus warning with -Wstringop-truncation -g (PR tree-optimization/84228)

[Marek Polacek]

On Tue, Feb 06, 2018 at 01:57:36PM +0100, Jakub Jelinek wrote:
> On Tue, Feb 06, 2018 at 01:46:21PM +0100, Marek Polacek wrote:
> > --- gcc/testsuite/c-c++-common/Wstringop-truncation-3.c
> > +++ gcc/testsuite/c-c++-common/Wstringop-truncation-3.c
> > @@ -0,0 +1,20 @@
> > +/* PR tree-optimization/84228 */
> > +/* { dg-do compile } */
> > +/* { dg-options "-Wstringop-truncation -O2 -g" } */
> > +
> > +char *strncpy (char *, const char *, __SIZE_TYPE__);
> > +struct S
> > +{
> > +  char arr[64];
> > +};
> > +
> > +int
> > +foo (struct S *p1, const char *a)
> > +{
> > +  if (a)
> > +goto err;
> > +  strncpy (p1->arr, a, sizeof p1->arr); /* { dg-bogus "specified bound" } 
> > */
> 
> Just curious, what debug stmt is in between those?

Just

  strncpy (_1, 0B, 64);
  # DEBUG BEGIN_STMT
  p1_5(D)->arr[3] = 0;

> Wouldn't it be better to force them a couple of debug stmts?
> Say
>   int b = 5, c = 6, d = 7;
> at the start of the function and
>   b = 8; c = 9; d = 10;
> in between strncpy and the '\0' store?

Yep.  I tweaked the testcase.  Now we have

  strncpy (_1, 0B, 64);
  # DEBUG BEGIN_STMT
  # DEBUG b => 8
  # DEBUG BEGIN_STMT
  # DEBUG c => 9
  # DEBUG BEGIN_STMT
  # DEBUG d => 10
  # DEBUG BEGIN_STMT
  p1_5(D)->arr[3] = 0;

> > +  p1->arr[3] = '\0';
> > +err:
> > +  return 0;
> > +}
> > diff --git gcc/tree-ssa-strlen.c gcc/tree-ssa-strlen.c
> > index c3cf432a921..f0f6535017b 100644
> > --- gcc/tree-ssa-strlen.c
> > +++ gcc/tree-ssa-strlen.c
> > @@ -1849,7 +1849,7 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi, 
> > tree src, tree cnt)
> >  
> >/* Look for dst[i] = '\0'; after the stxncpy() call and if found
> >   avoid the truncation warning.  */
> > -  gsi_next ();
> > +  gsi_next_nondebug ();
> >gimple *next_stmt = gsi_stmt (gsi);
> >  
> >if (!gsi_end_p (gsi) && is_gimple_assign (next_stmt))
> 
> Ok for trunk, though generally looking at just next stmt is very fragile, 
> might be
> better to look at the strncpy's vuse immediate uses if they are within the
> same basic block and either don't alias with it, or are the store it is
> looking for, or something similar.

I guess some
FOR_EACH_IMM_USE_FAST (...)
  {
if (is_gimple_debug (USE_STMT (use_p)))
  continue;
...
would be better.

Thanks,

2018-02-06  Marek Polacek  

PR tree-optimization/84228
* tree-ssa-strlen.c (maybe_diag_stxncpy_trunc): Skip debug statements.

* c-c++-common/Wstringop-truncation-3.c: New test.

diff --git gcc/testsuite/c-c++-common/Wstringop-truncation-3.c 
gcc/testsuite/c-c++-common/Wstringop-truncation-3.c
index e69de29bb2d..ba6b7de094b 100644
--- gcc/testsuite/c-c++-common/Wstringop-truncation-3.c
+++ gcc/testsuite/c-c++-common/Wstringop-truncation-3.c
@@ -0,0 +1,22 @@
+/* PR tree-optimization/84228 */
+/* { dg-do compile } */
+/* { dg-options "-Wstringop-truncation -O2 -g" } */
+
+char *strncpy (char *, const char *, __SIZE_TYPE__);
+struct S
+{
+  char arr[64];
+};
+
+int
+foo (struct S *p1, const char *a)
+{
+  int b = 5, c = 6, d = 7;
+  if (a)
+goto err;
+  strncpy (p1->arr, a, sizeof p1->arr); /* { dg-bogus "specified bound" } */
+  b = 8; c = 9; d = 10;
+  p1->arr[3] = '\0';
+err:
+  return 0;
+}
diff --git gcc/tree-ssa-strlen.c gcc/tree-ssa-strlen.c
index c3cf432a921..f0f6535017b 100644
--- gcc/tree-ssa-strlen.c
+++ gcc/tree-ssa-strlen.c
@@ -1849,7 +1849,7 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi, tree 
src, tree cnt)
 
   /* Look for dst[i] = '\0'; after the stxncpy() call and if found
  avoid the truncation warning.  */
-  gsi_next ();
+  gsi_next_nondebug ();
   gimple *next_stmt = gsi_stmt (gsi);
 
   if (!gsi_end_p (gsi) && is_gimple_assign (next_stmt))

Marek

Re: [PATCH, rs6000] Fix PR83926, ICE using __builtin_vsx_{div,udiv,mul}_2di builtins

[Segher Boessenkool]

On Tue, Feb 06, 2018 at 06:42:06AM -0600, Peter Bergner wrote:
> On 2/5/18 10:48 PM, David Edelsohn wrote:
> > On Mon, Feb 5, 2018 at 9:43 PM, Peter Bergner  wrote:
> >> I did also try calling expand_divmod() here which did generate correct
> >> code, the problem was that it wasn't as clean/optimized as the change
> >> to gen_divdi3.
> > 
> > Why not fix it at the site of the call to gen_divdi3 instead of the
> > divdi3 pattern?
> 
> Well as I said above, I did try that and we got worse code.  That said,
> I unconditionally called expand_divmod() instead of calling gen_divdi3()
> when we can (TARGET_POWERPC64).  Let me retry with that change and see
> what kind of code gen we get.

You can make the div3 define_expand for SDI instead of for GPR
(like we do for many other arithmetic and logical operations already)
if that is nicer.  (And udiv3, which doesn't have an expander
yet).


Segher

Re: PATCH to fix bogus warning with -Wstringop-truncation -g (PR tree-optimization/84228)

[Jakub Jelinek]

On Tue, Feb 06, 2018 at 01:46:21PM +0100, Marek Polacek wrote:
> --- gcc/testsuite/c-c++-common/Wstringop-truncation-3.c
> +++ gcc/testsuite/c-c++-common/Wstringop-truncation-3.c
> @@ -0,0 +1,20 @@
> +/* PR tree-optimization/84228 */
> +/* { dg-do compile } */
> +/* { dg-options "-Wstringop-truncation -O2 -g" } */
> +
> +char *strncpy (char *, const char *, __SIZE_TYPE__);
> +struct S
> +{
> +  char arr[64];
> +};
> +
> +int
> +foo (struct S *p1, const char *a)
> +{
> +  if (a)
> +goto err;
> +  strncpy (p1->arr, a, sizeof p1->arr); /* { dg-bogus "specified bound" } */

Just curious, what debug stmt is in between those?
Wouldn't it be better to force them a couple of debug stmts?
Say
  int b = 5, c = 6, d = 7;
at the start of the function and
  b = 8; c = 9; d = 10;
in between strncpy and the '\0' store?

> +  p1->arr[3] = '\0';
> +err:
> +  return 0;
> +}
> diff --git gcc/tree-ssa-strlen.c gcc/tree-ssa-strlen.c
> index c3cf432a921..f0f6535017b 100644
> --- gcc/tree-ssa-strlen.c
> +++ gcc/tree-ssa-strlen.c
> @@ -1849,7 +1849,7 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi, 
> tree src, tree cnt)
>  
>/* Look for dst[i] = '\0'; after the stxncpy() call and if found
>   avoid the truncation warning.  */
> -  gsi_next ();
> +  gsi_next_nondebug ();
>gimple *next_stmt = gsi_stmt (gsi);
>  
>if (!gsi_end_p (gsi) && is_gimple_assign (next_stmt))

Ok for trunk, though generally looking at just next stmt is very fragile, might 
be
better to look at the strncpy's vuse immediate uses if they are within the
same basic block and either don't alias with it, or are the store it is
looking for, or something similar.

Jakub

PATCH to fix bogus warning with -Wstringop-truncation -g (PR tree-optimization/84228)

[Marek Polacek]

When -Wstringop-truncation sees a strncpy call where the specified bound
is equal to the size of the destination, it looks at the next statement
to see if it's dst[i] = '\0';, and if it is, it doesn't warn.  But it
needs to look at the next nondebug statement, otherwise we can get a
false positive with -g, as this testcase shows.

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

2018-02-06  Marek Polacek  

PR tree-optimization/84228
* tree-ssa-strlen.c (maybe_diag_stxncpy_trunc): Skip debug statements.

* c-c++-common/Wstringop-truncation-3.c: New test.

diff --git gcc/testsuite/c-c++-common/Wstringop-truncation-3.c 
gcc/testsuite/c-c++-common/Wstringop-truncation-3.c
index e69de29bb2d..e15e6a42b07 100644
--- gcc/testsuite/c-c++-common/Wstringop-truncation-3.c
+++ gcc/testsuite/c-c++-common/Wstringop-truncation-3.c
@@ -0,0 +1,20 @@
+/* PR tree-optimization/84228 */
+/* { dg-do compile } */
+/* { dg-options "-Wstringop-truncation -O2 -g" } */
+
+char *strncpy (char *, const char *, __SIZE_TYPE__);
+struct S
+{
+  char arr[64];
+};
+
+int
+foo (struct S *p1, const char *a)
+{
+  if (a)
+goto err;
+  strncpy (p1->arr, a, sizeof p1->arr); /* { dg-bogus "specified bound" } */
+  p1->arr[3] = '\0';
+err:
+  return 0;
+}
diff --git gcc/tree-ssa-strlen.c gcc/tree-ssa-strlen.c
index c3cf432a921..f0f6535017b 100644
--- gcc/tree-ssa-strlen.c
+++ gcc/tree-ssa-strlen.c
@@ -1849,7 +1849,7 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi, tree 
src, tree cnt)
 
   /* Look for dst[i] = '\0'; after the stxncpy() call and if found
  avoid the truncation warning.  */
-  gsi_next ();
+  gsi_next_nondebug ();
   gimple *next_stmt = gsi_stmt (gsi);
 
   if (!gsi_end_p (gsi) && is_gimple_assign (next_stmt))

Marek

Re: [PATCH, rs6000] Fix PR83926, ICE using __builtin_vsx_{div,udiv,mul}_2di builtins

[Peter Bergner]

On 2/5/18 10:48 PM, David Edelsohn wrote:
> On Mon, Feb 5, 2018 at 9:43 PM, Peter Bergner  wrote:
>> I did also try calling expand_divmod() here which did generate correct
>> code, the problem was that it wasn't as clean/optimized as the change
>> to gen_divdi3.
> 
> Why not fix it at the site of the call to gen_divdi3 instead of the
> divdi3 pattern?

Well as I said above, I did try that and we got worse code.  That said,
I unconditionally called expand_divmod() instead of calling gen_divdi3()
when we can (TARGET_POWERPC64).  Let me retry with that change and see
what kind of code gen we get.

Peter

Re: RFA: Sanitize deprecation messages (PR 84195)

[Nick Clifton]

Hi Martin,

> My only suggestions are to consider how control characters and
> excessively messages are handled in other contexts and adopt
> the same approach here.

Are there other places where user-supplied messages are displayed by gcc ?


> In the tests I did, control characters
> were mostly escaped (e.g., as \n or as \x0a) rather than replaced
> with spaces. 

Ooo - what tests were these ?

I agree that it there is a standard way to handle these characters 
then the deprecated attribute ought to do the same.  (Plus hopefully
there is already a function in gcc to do this kind of processing,
so the deprecation code can just call it).


> I haven't thought too hard about how excessively
> long messages should be handled, or about the interactions with
> -fmessage-length= (i.e., if messages longer than the limit should
> be broken up.)

I believe that this will happen automatically.  The diagnostic display
routine will automatically insert newlines into the output if the 
message length is non-zero, and the message extends past this limit.
(Well provided that the message contains spaces.  The newlines are
only inserted in place of space characters so a very long, single
word message will not be split...)

Cheers
  Nick

Re: [SLP/AArch64] Fix unpack handling for big-endian SVE

[James Greenhalgh]

On Fri, Jan 26, 2018 at 02:21:11PM +, Richard Sandiford wrote:
> I hadn't realised that on big-endian targets, VEC_UNPACK*HI_EXPR unpacks
> the low-numbered lanes and VEC_UNPACK*LO_EXPR unpacks the high-numbered
> lanes.  This meant that both the SVE patterns and the handling of
> fully-masked loops were wrong.
> 
> The patch deals with that by making sure that all vec_unpack* optabs
> are define_expands, using BYTES_BIG_ENDIAN to choose the appropriate
> define_insn.  This in turn meant that we can get rid of the duplication
> between the signed and unsigned patterns for predicates.  (We provide
> implementations of both the signed and unsigned optabs because the sign
> doesn't matter for predicates: every element contains only one
> significant bit.)
> 
> Also, the float unpacks need to unpack one half of the input vector,
> but the unpacked upper bits are "don't care".  There are two obvious
> ways of handling that: use an unpack (filling with zeros) or use a ZIP
> (filling with a duplicate of the low bits).  The code previously used
> unpacks, but the sequence involved a subreg that is semantically an
> element reverse on big-endian targets.  Using the ZIP patterns avoids
> that, and at the moment there's no reason to prefer one over the other
> for performance reasons, so the patch switches to ZIP unconditionally.
> As the comment says, it would be easy to optimise this later if UUNPK
> turns out to be better for some implementations.
> 
> Tested on aarch64_be-elf, aarch64-linux-gnu and x86_64-linux-gnu.
> I think the tree-vect-loop-manip.c part is obvious, but OK for the
> AArch64 bits?

The AArch64 bits are OK by me, if this direction is appropriate for
the tree-vect-loop-manip.c parts.

Thanks,
James

> 
> Richard
> 
> 
> 2018-01-26  Richard Sandiford  
> 
> gcc/
>   * tree-vect-loop-manip.c (vect_maybe_permute_loop_masks):
>   Reverse the choice between VEC_UNPACK_LO_EXPR and VEC_UNPACK_HI_EXPR
>   for big-endian.
>   * config/aarch64/iterators.md (hi_lanes_optab): New int attribute.
>   * config/aarch64/aarch64-sve.md
>   (*aarch64_sve_): Rename to...
>   (aarch64_sve_): ...this.
>   (*extend2): Rename to...
>   (aarch64_sve_extend2): ...this.
>   (vec_unpack__): Turn into a define_expand,
>   renaming the old pattern to...
>   (aarch64_sve_punpk_): ...this.  Only define
>   unsigned packs.
>   (vec_unpack__): Turn into a
>   define_expand, renaming the old pattern to...
>   (aarch64_sve_unpk_): ...this.
>   (*vec_unpacku___no_convert): Delete.
>   (vec_unpacks__): Take BYTES_BIG_ENDIAN into
>   account when deciding which SVE instruction the optab should use.
>   (vec_unpack_float__vnx4si): Likewise.
> 
> gcc/testsuite/
>   * gcc.target/aarch64/sve/unpack_fcvt_signed_1.c: Expect zips rather
>   than unpacks.
>   * gcc.target/aarch64/sve/unpack_fcvt_unsigned_1.c: Likewise.
>   * gcc.target/aarch64/sve/unpack_float_1.c: Likewise.

Re: [PATCH] Use ptr_mode to save/restore pointers in builtin jmpbuf

[H.J. Lu]

On Tue, Feb 6, 2018 at 3:38 AM, Richard Earnshaw (lists)
 wrote:
> On 02/02/18 20:55, Eric Botcazou wrote:
>>> But, that is not what the builtin setjmp/longjmp tests have.
>>
>> Yes, but I don't think that we want to risk breaking a working compiler on
>> some targets because peculiar tests don't pass on another.  I think that
>> init_eh is OK for x32 so SJLJ exceptions work and the issue is only with the
>> undocumented builtin setjmp/longjmp.
>>
>> What happens on Aarch64 -milp32 for example?  Would it be OK to save/restore
>> only 32-bit values?  And MIPS n32?
>>
>
> No.  At least, not for the frame pointer on AArch64.  If the caller has
> used FP as a normal register, then all 64 bits must be preserved.  The
> same is true of any other register that might also be used as a
> non-frame-related register.

If FP is used as a normal register, it won't be saved as frame pointer
in builtin
jmp buffer.



-- 
H.J.

Re: PR 84154: Fix checking -mibt and -mshstk options for control flow protection

[Nick Clifton]

Hi Igor,

>>   Attached is a potential patch for PR 84145:
>>
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84145

> Coincidentally, I have worked on the same patch.

Great minds, etc :-)

> Please look at the patch, I uploaded it to the bug. The main differences are
> 
> - updated the output messages to be more informative;
> - updated  the tests and add couple of new tests to check the messages;
> - fixed a typo in the doc file related to fcf-protection;
> 
> I am ok with the changes in i386.c but would like to update the messages. 
> Could you incorporate my changes and proceed? Or would you like me to finish 
> the fix?

If you are happy to finish the fix then please do so.  Your fix is
more thorough than mine, so I am happy to see it go on.  Although
I should say that I am not an x86 maintainer, so I cannot approve
it.

Cheers
  Nick

Re: [PATCH,WIP] Use functional parameters for data mappings in OpenACC child functions

[Tom de Vries]

On 12/21/2017 10:46 PM, Cesar Philippidis wrote:


I've committed this patch to openacc-gcc-7-branch.



diff --git a/gcc/omp-expand.c b/gcc/omp-expand.c
index bf1f127d8d6..f674c74ec82 100644
--- a/gcc/omp-expand.c
+++ b/gcc/omp-expand.c



offloaded = is_gimple_omp_offloaded (entry_stmt);
switch (gimple_omp_target_kind (entry_stmt))
  {
+case GF_OMP_TARGET_KIND_OACC_PARALLEL:
+  oacc_parallel = true;
  case GF_OMP_TARGET_KIND_REGION:
  case GF_OMP_TARGET_KIND_UPDATE:
  case GF_OMP_TARGET_KIND_ENTER_DATA:
  case GF_OMP_TARGET_KIND_EXIT_DATA:
-case GF_OMP_TARGET_KIND_OACC_PARALLEL:
  case GF_OMP_TARGET_KIND_OACC_KERNELS:
  case GF_OMP_TARGET_KIND_OACC_UPDATE:
  case GF_OMP_TARGET_KIND_OACC_ENTER_EXIT_DATA:


This broke openacc-gcc-7-branch bootstrap:
...
gcc/omp-expand.c: In function 'void expand_omp_target(omp_region*)':
gcc/omp-expand.c:7110:21: error: this statement may fall through 
[-Werror=implicit-fallthrough=]

   oacc_parallel = true;
   ~~^~
gcc/omp-expand.c:7111:5: note: here
 case GF_OMP_TARGET_KIND_REGION:
 ^~~~
...

Thanks,
- Tom

Re: [PATCH] Use ptr_mode to save/restore pointers in builtin jmpbuf

[Richard Earnshaw (lists)]

On 02/02/18 20:55, Eric Botcazou wrote:
>> But, that is not what the builtin setjmp/longjmp tests have.
> 
> Yes, but I don't think that we want to risk breaking a working compiler on 
> some targets because peculiar tests don't pass on another.  I think that 
> init_eh is OK for x32 so SJLJ exceptions work and the issue is only with the 
> undocumented builtin setjmp/longjmp.
> 
> What happens on Aarch64 -milp32 for example?  Would it be OK to save/restore 
> only 32-bit values?  And MIPS n32?
> 

No.  At least, not for the frame pointer on AArch64.  If the caller has
used FP as a normal register, then all 64 bits must be preserved.  The
same is true of any other register that might also be used as a
non-frame-related register.

R.

Re: [PATCH][AArch64] PR target/84164: Relax predicate in *aarch64__reg_di3_mask2

[Richard Earnshaw (lists)]

On 02/02/18 15:43, Kyrill Tkachov wrote:
> Hi Richard,
> 
> On 02/02/18 15:25, Richard Earnshaw (lists) wrote:
>> On 02/02/18 15:10, Kyrill Tkachov wrote:
>>> Hi all,
>>>
>>> In this [8 Regression] PR we ICE because we can't recognise the insn:
>>> (insn 59 58 43 7 (set (reg:DI 124)
>>>  (rotatert:DI (reg:DI 125 [ c ])
>>>  (subreg:QI (and:SI (reg:SI 128)
>>>  (const_int 65535 [0x])) 0)))
>>
>> Aren't we heading off down the wrong path here?
>>
>> (subreg:QI (and:SI (reg:SI 128) (const_int 65535 [0x])) 0))
>>
>> can be simplified to
>>
>> (subreg:QI (reg:SI 128) 0)
>>
>> since the AND operation is redundant as we're only looking at the bottom
>> 8 bits.
> 
> I have tried implementing such a transformation in combine [1]
> but it was not clear that it was universally beneficial.
> See the linked thread and PR 70119 for the discussion (the thread
> continues into the next month).

Is that really the same thing?  The example there was using a mask that
was narrower than the subreg and thus not redundant.  The case here is
where the mask is completely redundant because we are only looking at
the bottom 8 bits of the result (which are not changed by the AND
operation).

R.

> 
> This patch fixes the existing patterns, such as they are,
> with the explicit subreg matching and associated warts.
> 
> We can resurrect the combine simplification discussion at some point
> but for the GCC 8 it would be safer to fix the existing pattern.
> 
> Thanks,
> Kyrill
> 
> [1] https://gcc.gnu.org/ml/gcc/2016-02/msg00357.html
> 
>> R.
>>
>>> This was created by the *aarch64_reg_3_neg_mask2 splitter.
>>> The point of these splitters and patterns is to eliminate redundant
>>> masking of the shift (or rotate in this case) amount when shifting
>>> by a variable amount.  For example in AND x3, x3, 0x ; ROR x1,
>>> x2, x3
>>> we can eliminate the AND because the ROR instruction implicitly "MOD"s
>>> the shift amount in X3 by 64. So this pattern is supposed to match the
>>> following:
>>>
>>> (define_insn "*aarch64__reg_di3_mask2"
>>>    [(set (match_operand:DI 0 "register_operand" "=r")
>>>  (SHIFT:DI
>>>    (match_operand:DI 1 "register_operand" "r")
>>>    (match_operator 4 "subreg_lowpart_operator"
>>>     [(and:SI (match_operand:SI 2 "register_operand" "r")
>>>  (match_operand 3 "aarch64_shift_imm_di" "Usd"))])))]
>>>
>>> The rotation amount mask is supposed to be operand 3 but the predicate
>>> for it here
>>> is aarch64_shift_imm_di which only allows values in [0, 63], whereas we
>>> want to allow
>>> any value that doesn't touch the bottom GET_MODE_BITSIZE (DImode) bits,
>>> which is what
>>> the pattern predicate tests. So the predicate on operands 3 is too
>>> strict.
>>>
>>> This patch just makes it const_int_operand since the pattern predicates
>>> has the correct
>>> validation for its values. This is in line with what the
>>> *aarch64_reg_3_neg_mask2
>>> and *aarch64_reg_3_minus_mask splitters accept (and they are the
>>> ones that generate
>>> this insn pattern).
>>>
>>> Bootstrapped and tested on aarch64-none-linux-gnu.
>>>
>>> Ok for trunk?
>>> Thanks,
>>> Kyrill
>>>
>>> 2018-02-02  Kyrylo Tkachov  
>>>
>>>  PR target/84164
>>>  * config/aarch64/aarch64.md (*aarch64__reg_di3_mask2):
>>>  Use const_int_operand predicate for operand 3.
>>>
>>> 2018-02-02  Kyrylo Tkachov  
>>>
>>>  PR target/84164
>>>  * gcc.c-torture/compile/pr84164.c: New test.
>>>
>>> aarch64-mask.patch
>>>
>>>
>>> diff --git a/gcc/config/aarch64/aarch64.md
>>> b/gcc/config/aarch64/aarch64.md
>>> index
>>> 04b2d203fa168bebcf6f93a13e3bd67a5998935a..eef0d1a780dd1c886e1bebb9992c552fb9d5b57c
>>> 100644
>>> --- a/gcc/config/aarch64/aarch64.md
>>> +++ b/gcc/config/aarch64/aarch64.md
>>> @@ -4358,8 +4358,8 @@ (define_insn "*aarch64__reg_di3_mask2"
>>>     (match_operand:DI 1 "register_operand" "r")
>>>     (match_operator 4 "subreg_lowpart_operator"
>>>  [(and:SI (match_operand:SI 2 "register_operand" "r")
>>> - (match_operand 3 "aarch64_shift_imm_di" "Usd"))])))]
>>> -  "((~INTVAL (operands[3]) & (GET_MODE_BITSIZE (DImode)-1)) == 0)"
>>> +    (match_operand 3 "const_int_operand" "n"))])))]
>>> +  "((~INTVAL (operands[3]) & (GET_MODE_BITSIZE (DImode) - 1)) == 0)"
>>>   {
>>>     rtx xop[3];
>>>     xop[0] = operands[0];
>>> diff --git a/gcc/testsuite/gcc.c-torture/compile/pr84164.c
>>> b/gcc/testsuite/gcc.c-torture/compile/pr84164.c
>>> new file mode 100644
>>> index
>>> ..d663fe5d6649e495f3e956e6a3bc938236a4bf91
>>>
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.c-torture/compile/pr84164.c
>>> @@ -0,0 +1,17 @@
>>> +/* PR target/84164.  */
>>> +
>>> +int b, d;
>>> +unsigned long c;
>>> +
>>> +static inline __attribute__ ((always_inline)) void
>>> +bar (int e)
>>> +{
>>> +  e += __builtin_sub_overflow_p (b, d, c);
>>> +  c = c << ((short) 

RE: PR 84154: Fix checking -mibt and -mshstk options for control flow protection

[Tsimbalist, Igor V]

> -Original Message-
> From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-
> ow...@gcc.gnu.org] On Behalf Of Nick Clifton
> Sent: Monday, February 5, 2018 4:15 PM
> To: hjl.to...@gmail.com
> Cc: gcc-patches@gcc.gnu.org
> Subject: RFA: PR 84154: Fix checking -mibt and -mshstk options for control
> flow protection
> 
> Hi H.J.
> 
>   Attached is a potential patch for PR 84145:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84145
> 
>   The problem was that the code to check that the --mibt and/or -mshstk
>   options have been correctly enabled for control flow protection did
>   not take into account that the wrong option might have been enabled.
> 
>   So the patch inverts the test, looking at the value of
>   flag_cf_protection first and then checking to see if the needed x86
>   specific options have been enabled.  This gives results like this:
> 
> 
>% gcc -c main.c
>% gcc -c main.c -fcf-protection=full
> cc1: error: '-fcf-protection=full' requires CET support on this target. Use -
> mcet or both of -mibt and -mshstk options to enable CET
>% gcc -c main.c -fcf-protection=full -mcet
>% gcc -c main.c -fcf-protection=full -mibt
> cc1: error: '-fcf-protection=full' requires CET support on this target. Use -
> mcet or both of -mibt and -mshstk options to enable CET
>% gcc -c main.c -fcf-protection=full -mibt -mshstk
>% gcc -c main.c -fcf-protection=branch
> cc1: error: '-fcf-protection=branch' requires CET support on this target. Use 
> -
> mcet or -mibt to enable CET
>% gcc -c main.c -fcf-protection=branch -mcet
>% gcc -c main.c -fcf-protection=branch -mibt
>% gcc -c main.c -fcf-protection=branch -mshstk
> cc1: error: '-fcf-protection=branch' requires CET support on this target. Use 
> -
> mcet or -mibt to enable CET
>% gcc -c main.c -fcf-protection=return
> cc1: error: '-fcf-protection=return' requires CET support on this target. Use 
> -
> mcet or -mshstk to enable CET
>% gcc -c main.c -fcf-protection=return -mcet
>% gcc -c main.c -fcf-protection=return -mibt
> cc1: error: '-fcf-protection=return' requires CET support on this target. Use 
> -
> mcet or -mshstk to enable CET
>% gcc -c main.c -fcf-protection=return -mshstk
>%
> 
>   What do you think ?  Is the patch OK for the mainline ?

Coincidentally, I have worked on the same patch. Please look at the patch, I 
uploaded it to the bug. The main differences are

- updated the output messages to be more informative;
- updated  the tests and add couple of new tests to check the messages;
- fixed a typo in the doc file related to fcf-protection;

I am ok with the changes in i386.c but would like to update the messages. Could 
you incorporate my changes and proceed? Or would you like me to finish the fix?

Thanks,
Igor

> Cheers
>   Nick
> 
> gcc/ChangeLog
> 2018-02-05  Nick Clifton  
> 
>   PR 84145
>   * config/i386/i386.c (ix86_option_override_internal): Rework
>   checks for -fcf-protection and -mibt/-mshstk.
> 
> Index: gcc/config/i386/i386.c
> ===
> 
> --- gcc/config/i386/i386.c(revision 257389)
> +++ gcc/config/i386/i386.c(working copy)
> @@ -4915,30 +4915,43 @@
>/* Do not support control flow instrumentation if CET is not enabled.  */
>if (opts->x_flag_cf_protection != CF_NONE)
>  {
> -  if (!(TARGET_IBT_P (opts->x_ix86_isa_flags2)
> - || TARGET_SHSTK_P (opts->x_ix86_isa_flags)))
> +  switch (flag_cf_protection)
>   {
> -   if (flag_cf_protection == CF_FULL)
> + case CF_NONE:
> +   break;
> + case CF_BRANCH:
> +   if (! TARGET_IBT_P (opts->x_ix86_isa_flags2))
>   {
> -   error ("%<-fcf-protection=full%> requires CET support "
> -  "on this target. Use -mcet or one of -mibt, "
> -  "-mshstk options to enable CET");
> +   error ("%<-fcf-protection=branch%> requires CET support "
> +  "on this target. Use -mcet or -mibt to enable CET");
> +   flag_cf_protection = CF_NONE;
> +   return false;
>   }
> -   else if (flag_cf_protection == CF_BRANCH)
> +   break;
> + case CF_RETURN:
> +   if (! TARGET_SHSTK_P (opts->x_ix86_isa_flags))
>   {
> -   error ("%<-fcf-protection=branch%> requires CET support "
> -  "on this target. Use -mcet or one of -mibt, "
> -  "-mshstk options to enable CET");
> +   error ("%<-fcf-protection=return%> requires CET support "
> +  "on this target. Use -mcet or -mshstk to enable CET");
> +   flag_cf_protection = CF_NONE;
> +   return false;
>   }
> -   else if (flag_cf_protection == CF_RETURN)
> +   break;
> + case CF_FULL:
> +   if (   ! TARGET_IBT_P (opts->x_ix86_isa_flags2)
> +  || ! TARGET_SHSTK_P (opts->x_ix86_isa_flags))
>   {
> -   error 

Re: [PATCH][GCC][ARM] Silence ARM_ARCH redefinition warning and keep better track of architecturs already emitted.

[Kyrill Tkachov]

On 05/02/18 11:52, Tamar Christina wrote:

Hi All,

This patch silences the warnings that were emitted when certain ACLE macros 
were being redefined
when using the new target pragma or attribute to change architecture level.

It also keeps better track of which arch directives have already been emitted. 
This allows special
cases that existed before to keep working as they did before.

Regtested on arm-none-eabi and no regressions.
Bootstrapped and regtested on arm-none-Linux-gnueabihf and no issues.

Ok for trunk and backport to the GCC-8 branch?



Thanks Tamar, this is in line with what we do for aarch64 as well.
Ok for trunk.
There is no GCC 8 branch, and this is GCC 8 functionality so this shouldn't
need to be backported anywhere.

Kyrill


Thanks,
Tamar


gcc/
2018-02-05  Tamar Christina  

PR target/82641
* config/arm/arm.c (arm_print_asm_arch_directives): Record already
emitted arch directives.
* config/arm/arm-c.c (arm_cpu_builtins): Undefine __ARM_ARCH and
__ARM_FEATURE_COPROC before changing architectures.

gcc/testsuite
2018-02-05  Tamar Christina  

PR target/82641
* gcc.target/arm/pragma_arch_switch_2.c: New.

--

Re: Avoid cc1 SEGV in gcc.dg/rtl/x86_64/final.c (PR target/79975)

[Rainer Orth]

Rainer Orth  writes:

> As described in the PR, cc1 SEGVs compiling gcc.dg/rtl/x86_64/final.c on
> targets that default to -fno-dwarf2-cfi-asm.  However, the test is
> compile-only, so doesn't depend on the toolchain's support for cfi
> directives.  Besides, it scans the output for those, so depends on
> -fdwarf2-cfi-asm anyway, so I'm adding it explicitly.
>
> Tested with the appropriate runtest invocation on i386-pc-solaris2.11
> with as (-fno-dwarf2-cfi-asm default), gas (-fdwarf2-cfi-asm default),
> and x86_64-pc-linux-gnu.
>
> Ok for mainline and gcc-7 branch?

There were no comments in a week and I'm pretty confident the patch
counts as obvious, so I've checked it into mainline and gcc-7 branch.

Rainer

-- 
-
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [patch][i386] Adding pconfig, wbnoinvd and wbinvd intrinsics

[Uros Bizjak]

> This patch adds new intrinsics: pconfig, wbnoinvd and wbinvd.
>
> 05.02.2018  Olga Makhotina  
>
> gcc/
> * common/config/i386/i386-common.c (OPTION_MASK_ISA_PCONFIG_SET,
> OPTION_MASK_ISA_PCONFIG_UNSET, OPTION_MASK_ISA_WBNOINVD_SET,
> OPTION_MASK_ISA_WBNOINVD_UNSET): New definitions.
> (ix86_handle_option): Handle -mpconfig and -mwbnoinvd.
> * config.gcc (pconfigintrin.h, wbnoinvdintrin.h) : Add headers.
> * config/i386/cpuid.h (bit_PCONFIG, bit_WBNOINVD): New.
> * config/i386/driver-i386.c (host_detect_local_cpu): Detect -mpconfig
> and -mwbnoinvd.
> * config/i386/i386-builtin.def (__builtin_ia32_wbnoinvd,
> __builtin_ia32_wbinvd): New builtins.
> (SPECIAL_ARGS2): New.
> * config/i386/i386-c.c (__WBNOINVD__, __PCONFIG__): New.
> (SPECIAL_ARGS2): New.
> * config/i386/i386.c (ix86_target_string): Add -mpconfig and -mwbnoinvd.
> (ix86_valid_target_attribute_inner_p): Ditto.
> (ix86_init_mmx_sse_builtins): Add special_args2.
> * config/i386/i386.h (TARGET_PCONFIG, TARGET_PCONFIG_P, TARGET_WBNOINVD,
> TARGET_WBNOINVD_P): New.
> * config/i386/i386.md (UNSPECV_WBINVD, UNSPECV_WBNOINVD): New.
> (define_insn "wbinvd", define_insn "wbnoinvd"): New.
> * config/i386/i386.opt: Add -mpconfig and -mwbnoinvd.
> * config/i386/immintrin.h (_wbinvd): New intrinsic.
> * config/i386/sgxintrin.h (_enclv_u32): Ditto.
> * config/i386/pconfigintrin.h: New file.
> * config/i386/wbnoinvdintrin.h: Ditto.
> * config/i386/x86intrin.h: Add headers pconfigintrin.h and wbnoinvdintrin.h.
> * doc/invoke.texi (-mpconfig, -mwbnoinvd): New.
>
> gcc/testsuite/
> * g++.dg/other/i386-2.C: Add -mpconfig and -mwbnoinvd.
> * g++.dg/other/i386-3.C: Ditto.
> * gcc.target/i386/sse-12.c: Ditto.
> * gcc.target/i386/sse-13.c: Ditto.
> * gcc.target/i386/sse-14.c: Ditto.
> * gcc.target/i386/sgx.c (_enclv_u32): New tests.
> * gcc.target/i386/sse-23.c: Add pconfig and wbnoinvd.
> * gcc.target/i386/wbinvd-1.c: New test.
> * gcc.target/i386/wbnoinvd-1.c: Ditto.
> * gcc.target/i386/pconfig-1.c: Ditto.
>
> Is it ok for trunk?

Please split out SGX changes to a separate patch.

OK for mainline with the above change.

Thanks,
Uros.

Re: [PATCH][AArch64] Use LDP/STP in shrinkwrapping

[James Greenhalgh]

On Fri, Jan 05, 2018 at 12:22:44PM +, Wilco Dijkstra wrote:
> The shrinkwrap optimization added late in GCC 7 allows each callee-save to
> be delayed and done only across blocks which need a particular callee-save.
> Although this reduces unnecessary memory traffic on code paths that need
> few callee-saves, it typically uses LDR/STR rather than LDP/STP.  The
> number of LDP/STP instructions is reduced by ~7%. This means more memory
> accesses and increased codesize, ~1.0% on average.
> 
> To improve this, if a particular callee-save must be saved/restored, also
> add the adjacent callee-save to allow use of LDP/STP.  This significantly
> reduces codesize (for example gcc_r, povray_r, parest_r, xalancbmk_r are
> 1% smaller).  This is a simple fix which can be backported.  A more advanced
> approach would scan blocks for pairs of callee-saves, but that requires a
> rewrite of all the callee-save code which is too late at this stage.
> 
> An example epilog in a shrinkwrapped function before:
> 
> ldpx21, x22, [sp,#16]
> ldrx23, [sp,#32]
> ldrx24, [sp,#40]
> ldpx25, x26, [sp,#48]
> ldrx27, [sp,#64]
> ldrx28, [sp,#72]
> ldrx30, [sp,#80]
> ldrd8, [sp,#88]
> ldpx19, x20, [sp],#96
> ret
> 
> And after this patch:
> 
> ldrd8, [sp,#88]
> ldpx21, x22, [sp,#16]
> ldpx23, x24, [sp,#32]
> ldpx25, x26, [sp,#48]
> ldpx27, x28, [sp,#64]
> ldrx30, [sp,#80]
> ldpx19, x20, [sp],#96
> ret
> 
> Passes bootstrap, OK for commit (and backport to GCC7)?

OK.

Thanks,
James

> 
> ChangeLog:
> 2018-01-05  Wilco Dijkstra  
> 
>   * config/aarch64/aarch64.c (aarch64_components_for_bb):
>   Increase LDP/STP opportunities by adding adjacent callee-saves.

[PR tree-optimization/84224] do not ICE on malformed allocas

[Aldy Hernandez]

The -Walloca pass can receive a malformed alloca, courtesy of someone 
providing a faulty prototype.  This was causing an ICE because we 
assumed alloca calls had at least one argument, which the testcase does not:


+void *alloca ();
+__typeof__(alloca ()) a () { return alloca (); }

I don't believe it should be the responsibility of the 
-Walloca-larger-than=* pass to warn against such things, so I propose we 
just ignore this.


I also think we should handle this testcase, regardless of the target 
having an alloca builtin, since the testcase includes its own 
prototype.  Thus, the missing "{dg-require-effect-target alloca}".


OK?

gcc/

	PR tree-optimization/84224
	* gimple-ssa-warn-alloca.c (execute): Do not ICE on malformed
	allocas.

diff --git a/gcc/gimple-ssa-warn-alloca.c b/gcc/gimple-ssa-warn-alloca.c
index 941810a997e..7457a16d21f 100644
--- a/gcc/gimple-ssa-warn-alloca.c
+++ b/gcc/gimple-ssa-warn-alloca.c
@@ -443,9 +443,10 @@ pass_walloca::execute (function *fun)
 	  gimple *stmt = gsi_stmt (si);
 	  location_t loc = gimple_location (stmt);
 
-	  if (!gimple_alloca_call_p (stmt))
+	  if (!gimple_alloca_call_p (stmt)
+	  /* A faulty prototype can yield a malformed alloca() call.  */
+	  || gimple_call_num_args (stmt) < 1)
 	continue;
-	  gcc_assert (gimple_call_num_args (stmt) >= 1);
 
 	  const bool is_vla
 	= gimple_call_alloca_for_var_p (as_a  (stmt));
diff --git a/gcc/testsuite/gcc.dg/Walloca-16.c b/gcc/testsuite/gcc.dg/Walloca-16.c
new file mode 100644
index 000..3ee96a9570a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Walloca-16.c
@@ -0,0 +1,6 @@
+/* PR tree-optimization/84224 */
+/* { dg-do compile } */
+/* { dg-options "-O0 -Walloca" } */
+
+void *alloca ();
+__typeof__(alloca ()) a () { return alloca (); }

Re: [PATCH] PR libgcc/59714 complex division is surprising on aarch64

[vladimir . mezentsev]

On 01/29/2018 12:51 PM, Joseph Myers wrote:
> On Mon, 29 Jan 2018, vladimir.mezent...@oracle.com wrote:
>
>>> What about powerpc __divkc3?
>>>
>>> What was the rationale for using soft-fp rather than adding appropriate 
>>> built-in functions as suggested in a comment?
>> I had a version with built-in functions and I can restore it.
>>
>> Let's design what we want to use soft-fp or built-in function.
>> I'd prefer to use built-in functions but performance is in two times worse.
>>
>> The test below demonstrates performance degradation:
> So, this is comparing __builtin_frexp (extracting both exponent and 
> mantissa, including appropriate handling of exponents of subnormal values) 
> with just extracting the exponent and with no special handling of zero / 
> infinity / NaN / subnormal values.
 I compared with __builtin_ilogb. There is a same performance degradation.

> We need to consider what functionality is actually needed for this 
> scaling, if we do wish to use such scaling based on integer exponents.  If 
> what's needed corresponds exactly to some standard functions such as ilogb 
> and scalbn with all their special cases, then built-in versions of those 
> standard functions may make sense.

 The IEEE format for double is      .
We need a function like extract_exponent_field.

  All standard function make an additional work to get  exponent.
It is a reason why we see performance degradation.

% cat r2.c
#include 
int main(int argc, char** argv)
{
  double d = 0x0.004p-1023;
  printf("d=%-24.13a  exp=%d\n", d, __builtin_ilogb(d));
  return 0;
}

% gcc -O2 -lm r2.c ; ./a.out
d=0x0.00200p-1022   *exp=-1033
*
-Vladimir

>   If what's needed does not correspond 
> to standard functions and does not need to handle such special cases, 
> that's more of an indication for examining the representation in libgcc - 
> but it's still necessary to handle the many different variant 
> floating-point formats supported, or to safely avoid using the new code 
> for formats it can't handle.
>

Re: [patch, libfortran] Use flexible array members for array descriptor

[Janne Blomqvist]

On Mon, Feb 5, 2018 at 10:53 PM, Thomas Koenig  wrote:
> Am 05.02.2018 um 13:09 schrieb Janne Blomqvist:
>>
>> On Sun, Feb 4, 2018 at 9:49 PM, Thomas Koenig 
>> wrote:
>>>
>>> Hello world,
>>>
>>> in the attached patch, I have used flexible array members for
>>> using the different descriptor types (following Richi's advice).
>>> This does not change the binary ABI, but the library code
>>> maches what we are actually doing in the front end.  I have
>>> not yet given up hope of enabling LTO for the library :-)
>>> and this, I think, will be a prerequisite.
>>>
>>> OK for trunk?
>>
>>
>> Given that Jakub and Richi apparently weren't yet unanimous in their
>> recommendations on the best path forward, maybe wait a bit for the
>> smoke to clear?
>
>
> Make sense.  Depending on what the solution is, this (or a variant)
> will probably part of the patch.
>
>> In the meantime, a few comments:
>>
>> 1) Is there some particular benefit to all those macroized
>> descriptors, given that the only thing different is the type of the
>> base_addr pointer? Wouldn't it be simpler to just have a single
>> descriptor type with base_addr a void pointer, then typecast that
>> pointer to whatever type is needed?
>
>
> I don't particulary like going through void* pointers - it is
> clearer to leave an int* as an int*.

Ok. Well, I disagree, but not vehemently, so it's fine as it is if you think so.

>> 2)
>>
>> Index: intrinsics/date_and_time.c
>> ===
>> --- intrinsics/date_and_time.c (Revision 257347)
>> +++ intrinsics/date_and_time.c (Arbeitskopie)
>> @@ -268,7 +268,7 @@ secnds (GFC_REAL_4 *x)
>> GFC_REAL_4 temp1, temp2;
>>
>> /* Make the INTEGER*4 array for passing to date_and_time.  */
>> -  gfc_array_i4 *avalues = xmalloc (sizeof (gfc_array_i4));
>> +  gfc_array_i4 *avalues = xmalloc (sizeof (gfc_full_array_i4));
>>
>>
>> Since date_and_time requires the values array to always be rank 1,
>> can't this be "xmalloc (sizeof (gfc_array_i4) +
>> sizeof(dimension_data))" ?
>
>
> I think we can be fairly sure that this would be OK at the moment, since
> (I think) there are no gaps in our descriptors at the moment. Anybody
> invents an architecture where this is not the case, we introduce
> a bug. This way is safer.

According to the C standard (C11 6.7.2.1.18 and example 6.7.2.1.20),
this is guaranteed to work. That is, sizeof() of a struct with a
flexible array member must include whatever padding is necessary so
that the flexible array member can be naturally aligned (or well, I
guess "must" only if the target doesn't handle misaligned accesses,
otherwise it's a QoI issue). In any case, if this does not work
properly for some target, a bug against the C frontend is in order.

-- 
Janne Blomqvist

Remove -q passed to grep in gcc-plugin.m4

[Eric Botcazou]

The option is not supported on the original Solaris' grep.  Given that there 
is also a redirection to /dev/null at the end, it is superfluous.

Tested on x86_64-suse-linux, applied on the mainline as obvious.


2018-02-06  Eric Botcazou  

config/
* gcc-plugin.m4 (GCC_ENABLE_PLUGINS): Remove -q option passed to grep.


2018-02-06  Eric Botcazou  

gcc/
* configure: Regenerate.

-- 
Eric BotcazouIndex: gcc-plugin.m4
===
--- gcc-plugin.m4	(revision 257404)
+++ gcc-plugin.m4	(working copy)
@@ -60,14 +60,14 @@ AC_DEFUN([GCC_ENABLE_PLUGINS],
  if test "x$export_sym_check" != x; then
echo "int main() {return 0;} int foobar() {return 0;}" > conftest.c
${CC} ${CFLAGS} ${LDFLAGS} conftest.c -o conftest$ac_exeext > /dev/null 2>&1
-   if $export_sym_check conftest$ac_exeext | grep -q foobar > /dev/null; then
+   if $export_sym_check conftest$ac_exeext | grep foobar > /dev/null; then
 	 : # No need to use a flag
 	 AC_MSG_RESULT([yes])
else
 	 AC_MSG_RESULT([yes])
 	 AC_MSG_CHECKING([for -rdynamic])
 	 ${CC} ${CFLAGS} ${LDFLAGS} -rdynamic conftest.c -o conftest$ac_exeext > /dev/null 2>&1
-	 if $export_sym_check conftest$ac_exeext | grep -q foobar > /dev/null; then
+	 if $export_sym_check conftest$ac_exeext | grep foobar > /dev/null; then
 	   plugin_rdynamic=yes
 	   pluginlibs="-rdynamic"
 	 else

[patch][i386] Fix for PR83828

[Makhotina, Olga]

Hi,

This patch repairs vpopcnt tests.

06.02.2018  Olga Makhotina  

gcc/testsuite/
PR target/83828
* gcc.target/i386/avx512bitalg-vpopcntb-1.c: Fix test.
* gcc.target/i386/avx512bitalg-vpopcntw-1.c: Ditto.
* gcc.target/i386/avx512bitalg-vpshufbitqmb-1.c: Ditto.
* gcc.target/i386/avx512vpopcntdq-vpopcntd-1.c: Ditto.
* gcc.target/i386/avx512vpopcntdq-vpopcntq-1.c: Ditto.

Is it ok for trunk?

Thanks,
Olga.



0001-avx512-test.patch
Description: 0001-avx512-test.patch