Re: [PATCH 3/4] enhance overflow and truncation detection in strncpy and strncat (PR 81117)

[Jeff Law]

On 08/06/2017 02:07 PM, Martin Sebor wrote:
> Part 3 of the series contains the meat of the patch: the new
> -Wstringop-truncation option, and enhancements to -Wstringop-
> overflow, and -Wpointer-sizeof-memaccess to detect misuses of
> strncpy and strncat.
> 
> Martin
> 
> gcc-81117-3.diff
> 
> 
> PR c/81117 - Improve buffer overflow checking in strncpy
> 
> gcc/ChangeLog:
> 
>   PR c/81117
>   * builtins.c (compute_objsize): Handle arrays that
>   compute_builtin_object_size likes to fail for.  Make extern.
>   * builtins.h (compute_objsize): Declare.
>   (check_strncpy_sizes): New function.
>   (expand_builtin_strncpy): Call check_strncpy_sizes.
>   * gimple-fold.c (gimple_fold_builtin_strncpy): Implement
>   -Wstringop-truncation.
>   (gimple_fold_builtin_strncat): Same.
>   * gimple.c (gimple_build_call_from_tree): Set call location.
>   * tree-ssa-strlen.c (strlen_to_stridx): New global variable.
>   (maybe_diag_bound_equal_length, is_strlen_related_p): New functions.
>   (handle_builtin_stxncpy, handle_builtin_strncat): Same.
>   (handle_builtin_strlen): Use strlen_to_stridx.
>   (strlen_optimize_stmt): Handle flavors of strncat, strncpy, and
>   stpncpy.
>   Use strlen_to_stridx.
>   (pass_strlen::execute): Release strlen_to_stridx.
>   * doc/invoke.texi (-Wsizeof-pointer-memaccess): Document enhancement.
>   (-Wstringop-truncation): Document new option.
> 
> gcc/c-family/ChangeLog:
> 
>   PR c/81117
>   * c-warn.c (sizeof_pointer_memaccess_warning): Handle arrays.
>   * c.opt (-Wstriingop-truncation): New option.
> 
> gcc/testsuite/ChangeLog:
> 
>   PR c/81117
>   * c-c++-common/Wsizeof-pointer-memaccess3.c: New test.
>   * c-c++-common/Wstringop-overflow.c: Same.
>   * c-c++-common/Wstringop-truncation.c: Same.
>   * c-c++-common/Wsizeof-pointer-memaccess2.c: Adjust.
>   * c-c++-common/attr-nonstring-2.c: New test.
>   * g++.dg/torture/Wsizeof-pointer-memaccess1.C: Adjust.
>   * g++.dg/torture/Wsizeof-pointer-memaccess2.C: Same.
>   * gcc.dg/torture/pr63554.c: Same.
>   * gcc.dg/Walloca-1.c: Disable macro tracking.
> 
> diff --git a/gcc/builtins.c b/gcc/builtins.c
> index 016f68d..1aa9e22 100644
> --- a/gcc/builtins.c
> +++ b/gcc/builtins.c
[ ... ]
> +
> +  if (TREE_CODE (type) == ARRAY_TYPE)
> +{
> +  /* Return the constant size unless it's zero (that's a zero-length
> +  array likely at the end of a struct).  */
> +  tree size = TYPE_SIZE_UNIT (type);
> +  if (size && TREE_CODE (size) == INTEGER_CST
> +   && !integer_zerop (size))
> + return size;
> +}
Q. Do we have a canonical test for the trailing array idiom?   In some
contexts isn't it size 1?  ISTM This test needs slight improvement.
Ideally we'd use some canonical test for detect the trailing array idiom
rather than open-coding it here.  You might look at the array index
warnings in tree-vrp.c to see if it's got a canonical test you can call
or factor and use.




> @@ -3883,6 +3920,30 @@ expand_builtin_strncat (tree exp, rtx)
>return NULL_RTX;
>  }
>  
> +/* Helper to check the sizes of sequences and the destination of calls
> +   to __builtin_strncpy (DST, SRC, LEN) and __builtin___strncpy_chk.
> +   Returns true on success (no overflow warning), false otherwise.  */
> +
> +static bool
> +check_strncpy_sizes (tree exp, tree dst, tree src, tree len)
> +{
> +  tree dstsize = compute_objsize (dst, warn_stringop_overflow - 1);
> +
> +  if (!check_sizes (OPT_Wstringop_overflow_,
> + exp, len, /*maxlen=*/NULL_TREE, src, dstsize))
> +return false;
> +
> +  if (!dstsize || TREE_CODE (len) != INTEGER_CST)
> +return true;
> +
> +  if (tree_int_cst_lt (dstsize, len))
> +warning_at (EXPR_LOCATION (exp), OPT_Wstringop_truncation,
> + "%K%qD specified bound %E exceeds destination size %E",
> + exp, get_callee_fndecl (exp), len, dstsize);
> +
> +  return true;
So in the case where you issue the warning, what should the return value
be?  According to the comment it should be false.  It looks like you got
the wrong return value for the tree_int_cst_lt (dstsize, len) test.





>  
>return false;
> diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c
> index b0563fe..ac6503f 100644
> --- a/gcc/tree-ssa-strlen.c
> +++ b/gcc/tree-ssa-strlen.c

> +
> +/* A helper of handle_builtin_stxncpy.  Check to see if the specified
> +   bound is a) equal to the size of the destination DST and if so, b)
> +   if it's immediately followed by DST[LEN - 1] = '\0'.  If a) holds
> +   and b) does not, warn.  Otherwise, do nothing.  Return true if
> +   diagnostic has been issued.
> +
> +   The purpose is to diagnose calls to strncpy and stpncpy that do
> +   not nul-terminate the copy while allowing for the idiom where
> +   such a call is immediately followed by setting the last element
> +   to nul, as in:
> + char a[32];
> + strncpy (a, 

Re: [PATCH 2/4] enhance overflow and truncation detection in strncpy and strncat (PR 81117)

[Jeff Law]

On 08/06/2017 02:07 PM, Martin Sebor wrote:
> Part 2 of the series adds attribute nostring to annotate arrays
> of and pointers to char with that are intended to store sequences
> of characters that aren't necessarily valid (nul-terminated)
> strings.  In the subsequent patch the attribute is relied on to
> avoid diagnosing strcncpy calls that truncate strings and create
> such copies.  In the future I'd like to also use the attribute
> to diagnose when arrays or pointers with the attribute are passed
> to functions that expect nul-terminated strings (such as strlen
> or strcpy).
> 
> Martin
> 
> 
> gcc-81117-2.diff
> 
> 
> PR c/81117 - Improve buffer overflow checking in strncpy
> 
> gcc/ChangeLog:
> 
>   PR c/81117
>   * builtin-attrs.def (attribute nonstring): New.
>   * doc/extend.texi (attribute nonstring): Document new attribute.
> 
> gcc/c-family/ChangeLog:
> 
>   PR c/81117
>   * c-attribs.c (c_common_attribute_table): Add nonstring entry.
>   (handle_nonstring_attribute): New function.
> 
> gcc/testsuite/ChangeLog:
> 
>   PR c/81117
>   * c-c++-common/attr-nonstring-1.c: New test.
> 
> --- a/gcc/builtin-attrs.def
> +++ b/gcc/builtin-attrs.def
> @@ -93,6 +93,7 @@ DEF_ATTR_IDENT (ATTR_FORMAT, "format")
>  DEF_ATTR_IDENT (ATTR_FORMAT_ARG, "format_arg")
>  DEF_ATTR_IDENT (ATTR_MALLOC, "malloc")
>  DEF_ATTR_IDENT (ATTR_NONNULL, "nonnull")
> +DEF_ATTR_IDENT (ATTR_NONSTRING, "nonstring")
>  DEF_ATTR_IDENT (ATTR_NORETURN, "noreturn")
>  DEF_ATTR_IDENT (ATTR_NOTHROW, "nothrow")
>  DEF_ATTR_IDENT (ATTR_LEAF, "leaf")
So all the attributes here are associated with functions I believe.
You're defining a variable attribute.  In fact, I'm not even sure that
variable attributes get a DEF_ATTR_




> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index b253ccc..1954ca5 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -5835,6 +5835,30 @@ The @code{deprecated} attribute can also be used for 
> functions and
>  types (@pxref{Common Function Attributes},
>  @pxref{Common Type Attributes}).
>  
> +@item nonstring (@var{nonstring})
> +@cindex @code{nonstring} variable attribute
> +The @code{nonstring} variable attribute specifies that an object or member
> +declaration with type array of @code{char} or pointer to @code{char} is
> +intended to store character arrays that do not necessarily contain
> +a terminating @code{NUL} character.  This is useful to avoid warnings
> +when such an array or pointer is used as an argument to a bounded string
> +manipulation function such as @code{strncpy}.  For example, without the
> +attribute, GCC will issue a warning for the call below because it may
> +truncate the copy without appending the terminating NUL character.  Using
> +the attribute makes it possible to suppress the warning.
[ ... ]
I think this is in the wrong section, I believe it belongs in the
"Variable Attributes" section.


Assuming you don't actually need the ATTR_NONSTRING, this patch is fine
with that hunk removed and the documentation moved into the right section.

jeff

Re: [PATCH] rs6000: Use SAVE_MULTIPLE only if we restore what it saves (PR80938)

[Alan Modra]

On Wed, Aug 09, 2017 at 09:28:22PM -0500, Segher Boessenkool wrote:
> On Thu, Aug 10, 2017 at 10:33:05AM +0930, Alan Modra wrote:
> > On Wed, Aug 09, 2017 at 09:06:18PM +, Segher Boessenkool wrote:
> > > We can have SAVE_MULTIPLE while we do not have REST_MULTIPLE.  If the
> > > inline restore does not restore all registers, the CFI for the save
> > > and restore can conflict if things are shrink-wrapped.
> > > 
> > > We could restore all registers that are saved (not ideal),
> > 
> > No, we can't do that.  A -ffixed-reg register should not be restored
> > even if it is saved, as such regs should never be written.  For
> > example, a fixed reg might be counting interrupts.  If you restore it,
> > you may lose count.  Another example is a fixed reg used as some sort
> > of context pointer.  If you restore in a function that sets a new
> > context you're obviously breaking user code.
> 
> Yes, sorry for glossing over this.  We need to handle fixed registers
> specially in most other prologue/epilogue things, too (and we hopefully
> do everywhere it is needed).

We don't.  :-(  I have a fix and will post after testing.

> > > or emit
> > > the CFI notes to say we did (which will work fine,
> > 
> > No, you can't do that either.  Unwinding might then restore a
> > -ffixed-reg register.
> 
> Yep.
> 
> > > but is also not
> > > so great); instead, let's not save the registers that are unused.
> > 
> > Ick, looks like papering over the real problem to me, and will no
> > doubt cause -Os size regressions.
> 
> I think it is very directly solving the real problem.  It isn't likely
> to cause size regressions (look how long it took for this PR to show
> up, so this cannot happen often).
> 
> This only happens if r30 (the PIC reg) is used but r31 isn't; which means
> that a) there are no other registers to save, or b) the function is marked
> as needing a hard frame pointer but eventually doesn't need one.
> 
> (RA picks the registers r31, r30, ... in sequence).
> 
> In the case in the PR, this patch replaces one insn by one (cheaper)
> insn.

And in other cases your patch will prevent stmw when it should be
used.  Testcase attached.  It shows the wrong use of lmw too.

> > Also, if SAVE_MULTIPLE causes this
> > bad interaction with shrink-wrap, does using the out-of-line register
> > save functions cause the same problem?
> 
> I do not know.  Not with the code in the PR (restoring only one or two
> registers isn't done out-of-line, and it's a sibcall exit as well).
> 
> 
> Segher

-- 
Alan Modra
Australia Development Lab, IBM
#ifdef USER_R26
register long r26 __asm__ ("r26");
#endif

int f1 (void)
{
  register long r25 __asm__ ("r25");
  register long r27 __asm__ ("r27");
  register long r28 __asm__ ("r28");
  register long r29 __asm__ ("r29");
  register long r31 __asm__ ("r31");
  __asm__ __volatile__
("#uses %0 %1 %2 %3 %4"
 : "=r" (r25), "=r" (r27), "=r" (r28), "=r" (r29), "=r" (r31));
  return 0;
}

void f2 (char *s)
{
  int col = 0;
  char c;

  void f3 (char c)
  {
extern int f4 (char);
register long r25 __asm__ ("r25");
register long r27 __asm__ ("r27");
register long r28 __asm__ ("r28");
register long r29 __asm__ ("r29");
register long r31 __asm__ ("r31");
if (c == '\t')
  do f3 (' '); while (col % 8 != 0);
else
  {
	__asm__ __volatile__
	  ("#uses %0 %1 %2 %3 %4"
	   : "=r" (r25), "=r" (r27), "=r" (r28), "=r" (r29), "=r" (r31));
	f4 (c);
	col++;
  }
  }

  while ((c = *s++) != 0)
f3 (c);
}

Re: [PATCH 1/4] enhance overflow and truncation detection in strncpy and strncat (PR 81117)

[Jeff Law]

On 08/06/2017 02:07 PM, Martin Sebor wrote:
> The attached patch adds support for a new GCC format specifier,
> G, that behaves like %K but accepts a gcall* argument.  This
> makes it possible to provide inlining context for "artificial"
> inline functions like strncpy (with _FORTIFY_SOURCE) in
> diagnostics issued from the middle end.
> 
> Martin
> 
> gcc-81117-1.diff
> 
> 
> PR c/81117 - Improve buffer overflow checking in strncpy
> 
> gcc/ChangeLog:
> 
>   PR c/81117
>   * tree-diagnostic.c (default_tree_printer): Handle %G.
>   * tree-pretty-print.h (percent_G_format): Declare new function.
>   * tree-pretty-print.c (percent_K_format): Define a static overload.
>   (percent_G_format): New function.
> 
> gcc/c/ChangeLog:
> 
>   PR c/81117
>   * c-objc-common.c (c_objc_common_init): Handle 'G'.
> 
> gcc/c-family/ChangeLog:
> 
>   * c-format.h (T89_G): New macro.
>   * c-format.c (local_gcall_ptr_node): New variable.
>   (init_dynamic_diag_info): Initialize it.
> 
> gcc/cp/ChangeLog:
> 
>   PR c/81117
>   * error.c (cp_printer): Handle 'G'.
> 
> gcc/testsuite/ChangeLog:
> 
>   PR c/81117
>   * gcc.dg/format/gcc_diag-10.c: Exercise %G.
> 
> diff --git a/gcc/tree-diagnostic.c b/gcc/tree-diagnostic.c
> index 52b7e7f..ad58b69 100644
> --- a/gcc/tree-diagnostic.c
> +++ b/gcc/tree-diagnostic.c
> @@ -275,6 +275,10 @@ default_tree_printer (pretty_printer *pp, text_info 
> *text, const char *spec,
>t = va_arg (*text->args_ptr, tree);
>break;
>  
> +case 'G':
> +  percent_G_format (text);
> +  return true;
> +
>  case 'K':
>percent_K_format (text);
>return true;
> diff --git a/gcc/tree-pretty-print.c b/gcc/tree-pretty-print.c
> index 4d8177c..7c4c805 100644
> --- a/gcc/tree-pretty-print.c
> +++ b/gcc/tree-pretty-print.c
> @@ -33,6 +33,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "dumpfile.h"
>  #include "internal-fn.h"
>  #include "gomp-constants.h"
> +#include "gimple.h"
This is an indication you're probably putting the 'G' handling in the
wrong place.  Wouldn't gimple-pretty-print.c be more correct?

That's my only objection to this patch, so if it moves trivially, then
it's pre-approved.  If it's non-trivial, then we'll want another iteration.

jeff

Re: [PATCH 1/3] improve detection of attribute conflicts (PR 81544)

[Martin Sebor]

On 08/09/2017 01:55 PM, Joseph Myers wrote:

On Wed, 9 Aug 2017, Martin Sebor wrote:


the same function with the other of this pair attributes.  I'd
also be okay with not diagnosing this combination if I could
convince myself that it's safe (or can be made safe) and treated
consistently.


I'd expect it to be safe; it might simply happen that some calls are
optimized based on incomplete information (and even that is doubtful, as
all optimizations except front-end folding should be taking place after
all the declarations have been seen).


The following is the example I have in mind.  It compiles silently
without the patch but aborts at runtime.  By warning. the patch
makes it clear that the pure attribute is ignored.  (If/when GCC
starts warning about attribute const violations it will also point
out the global read, but that will help only if the definition sees
the const declaration.)

The problem isn't that the declarations aren't merged at the call
site but rather that the middle-end gives const precedence over
pure so when both attributes are provided the former wins.

  int x;

  int __attribute__ ((const, noclone, noinline)) f (int);

  int main (void)
  {
  int i = f (0);
  x = 7;
  i += f (0);
  if (i != 7) __builtin_abort ();
  }

  int __attribute__ ((pure)) f (int i) { return x + i; }




The problem whose subset the diagnostic detects is declaring
the function const and calling it before the pure declaration
that corresponds to its definition is seen.  I.e., the inverse
of what the ssa-ccp-2.c test does.  I think a mismatch between
the attributes is as suspect as a mismatch in the function's
signature.


In the ssa-ccp-2.c case, it's not clear to me the test intends to use the
same function at all; maybe one of the foo9 functions should be renamed.


I think you're right.  It's likely that the intent is to verify
that a call to either a pure or a const function doesn't defeat
constant propagation.  Let me fix the test.  (This also seems
to be an example where the warning has already proved to be
useful -- it has pointed out a weakness in the test.)


I think it's actually like having a non-prototype declaration and a
prototype one, where a composite type is formed from two compatible types.
(We have a warning in the very specific case of a non-prototype
*definition* followed by a prototype declaration.)


In any event, it sounds to me that conceptually, it might be
useful to be able to specify which of a pair of mutually
exclusive (or redundant) attributes to retain and/or accept:
the first or the second, and not just whether to accept or
drop the most recent one.  That will make it possible to make
the most appropriate decision about each specific pair of
attributes without impacting any of the others.


If they conflict, I'm not sure there's any basis for making a choice
specific to a particular pair of attributes.


I'm not sure either because I haven't done a complete survey.
What I do know is that some conflicts are currently ignored
with both attributes accepted (sometimes leading to confusing
symptoms later), others cause hard errors, and others cause
warnings.  This varies depending on whether the conflict
is on the same declaration or on two distinct ones.  So I'm
thinking that generalizing the machinery to make it possible
to express some or most of these scenarios will make it easy
to apply the patch and use it as is most appropriate in each
situation.



In cases like const and pure where one is a subset of the other, it makes
sense to choose based on the pair of attributes - and not necessarily to
warn under the same conditions as the warnings for conflicting attributes.


One way to deal with this case might be to warn only if pure
is seen on an already used const function.

Let me think about it some more, work up a new patch, and post
it for consideration.

Martin

Re: [PATCH] rs6000: Use SAVE_MULTIPLE only if we restore what it saves (PR80938)

[Segher Boessenkool]

On Thu, Aug 10, 2017 at 10:33:05AM +0930, Alan Modra wrote:
> On Wed, Aug 09, 2017 at 09:06:18PM +, Segher Boessenkool wrote:
> > We can have SAVE_MULTIPLE while we do not have REST_MULTIPLE.  If the
> > inline restore does not restore all registers, the CFI for the save
> > and restore can conflict if things are shrink-wrapped.
> > 
> > We could restore all registers that are saved (not ideal),
> 
> No, we can't do that.  A -ffixed-reg register should not be restored
> even if it is saved, as such regs should never be written.  For
> example, a fixed reg might be counting interrupts.  If you restore it,
> you may lose count.  Another example is a fixed reg used as some sort
> of context pointer.  If you restore in a function that sets a new
> context you're obviously breaking user code.

Yes, sorry for glossing over this.  We need to handle fixed registers
specially in most other prologue/epilogue things, too (and we hopefully
do everywhere it is needed).

> > or emit
> > the CFI notes to say we did (which will work fine,
> 
> No, you can't do that either.  Unwinding might then restore a
> -ffixed-reg register.

Yep.

> > but is also not
> > so great); instead, let's not save the registers that are unused.
> 
> Ick, looks like papering over the real problem to me, and will no
> doubt cause -Os size regressions.

I think it is very directly solving the real problem.  It isn't likely
to cause size regressions (look how long it took for this PR to show
up, so this cannot happen often).

This only happens if r30 (the PIC reg) is used but r31 isn't; which means
that a) there are no other registers to save, or b) the function is marked
as needing a hard frame pointer but eventually doesn't need one.

(RA picks the registers r31, r30, ... in sequence).

In the case in the PR, this patch replaces one insn by one (cheaper)
insn.

> Also, if SAVE_MULTIPLE causes this
> bad interaction with shrink-wrap, does using the out-of-line register
> save functions cause the same problem?

I do not know.  Not with the code in the PR (restoring only one or two
registers isn't done out-of-line, and it's a sibcall exit as well).


Segher

Re: [PATCH] rs6000: Use SAVE_MULTIPLE only if we restore what it saves (PR80938)

[Alan Modra]

On Wed, Aug 09, 2017 at 09:06:18PM +, Segher Boessenkool wrote:
> We can have SAVE_MULTIPLE while we do not have REST_MULTIPLE.  If the
> inline restore does not restore all registers, the CFI for the save
> and restore can conflict if things are shrink-wrapped.
> 
> We could restore all registers that are saved (not ideal),

No, we can't do that.  A -ffixed-reg register should not be restored
even if it is saved, as such regs should never be written.  For
example, a fixed reg might be counting interrupts.  If you restore it,
you may lose count.  Another example is a fixed reg used as some sort
of context pointer.  If you restore in a function that sets a new
context you're obviously breaking user code.

> or emit
> the CFI notes to say we did (which will work fine,

No, you can't do that either.  Unwinding might then restore a
-ffixed-reg register.

> but is also not
> so great); instead, let's not save the registers that are unused.

Ick, looks like papering over the real problem to me, and will no
doubt cause -Os size regressions.  Also, if SAVE_MULTIPLE causes this
bad interaction with shrink-wrap, does using the out-of-line register
save functions cause the same problem?

I haven't looked yet, but at a guess I suspect the correct solution is
to modify cfa_restores in rs6000_emit_epilogue.

-- 
Alan Modra
Australia Development Lab, IBM

[committed] jit: add gcc_jit_type_get_vector

[David Malcolm]

On Wed, 2017-08-09 at 20:42 +1200, Michael Cree wrote:
> On Mon, Aug 07, 2017 at 10:28:57AM -0400, David Malcolm wrote:
> > On Mon, 2017-08-07 at 20:12 +1200, Michael Cree wrote:
> > 
> > Hi Michael
> > 
> > > I am wondering if libgccjit supports vector types, i.e., can one
> > > attach attribute __vector_size__ to the base types?  There does
> > > not
> > > seem to be any indication of this in the manual.
> > 
> > Currently this isn't supported.
> > 
> > Is this a blocker for you using libgccjit?  
> 
> Yeah, it is.  The idea is to generate image processing operators
> on the fly.  There are opportunities for various optimisations that
> can only be decided on at runtime.
> 
> > What would the ideal API
> > look like? 
> > 
> > Maybe something like:
> > 
> >   extern gcc_jit_type *
> >   gcc_jit_type_get_vector (gcc_jit_type *type, unsigned nunits);
> >  
> > with various requirements (type must be integral/floating point;
> > nunits
> > must be a power of two).
> 
> I suspect that would do the job nicely.
> 
> Cheers
> Michael.

I implemented the above (although I switched the 2nd arg to be
"size_t num_units").

It looks like you may not need to explicitly use builtins to
access machine specific simd intrinsics; for example, on x86_64
when I tried multiplying two of these together for float, with
GCC_JIT_BINARY_OP_MULT, which led to this gimple:

jit_v4f_mult (const vector(4)  * a, const vector(4)  * b, 
vector(4)  * c)
{
  initial:
  _1 = *a;
  _2 = *b;
  _3 = _1 * _2;
  *c = _3;
  return;
}

on this x86_64 box it compiled to:

movaps  (%rdi), %xmm0
mulps   (%rsi), %xmm0
movaps  %xmm0, (%rdx)
ret

(i.e. using the "mulps" SIMD instruction).

Successfully bootstrapped on x86_64-pc-linux-gnu;
takes jit.sum from 9349 passes to 9759.

Committed to trunk as r251018.

gcc/jit/ChangeLog:
* docs/cp/topics/types.rst (Vector types): New section.
* docs/topics/compatibility.rst (LIBGCCJIT_ABI_8): New tag.
* docs/topics/types.rst (gcc_jit_context_get_type): Fix typo in
example.
(Vector types): New section.
* docs/_build/texinfo/libgccjit.texi: Regenerate.
* jit-playback.c (gcc::jit::playback::type::get_vector): New
method.
* jit-playback.h (gcc::jit::playback::type::get_vector): New
method.
* jit-recording.c: In namespace gcc::jit::recording::
(type::get_vector): New method.
(memento_of_get_aligned::write_reproducer): Fix typo
in leading comment.
(memento_of_get_vector::replay_into): New method.
(memento_of_get_vector::make_debug_string): New method.
(memento_of_get_vector::write_reproducer): New method.
* jit-recording.h: In namespace gcc::jit::recording::
(type::get_vector): New
 method.
(class memento_of_get_vector): New class.
* libgccjit++.h (gccjit::type::get_vector): New method.
* libgccjit.c (gcc_jit_type_get_vector): New public entrypoint.
* libgccjit.h (LIBGCCJIT_HAVE_gcc_jit_type_get_vector): New
define.
(gcc_jit_type_get_vector): New decl.
* libgccjit.map (LIBGCCJIT_ABI_8): New ABI tag.

gcc/testsuite/ChangeLog:
* jit.dg/all-non-failing-tests.h: Add note about
test-vector-types.cc.
* jit.dg/test-error-gcc_jit_type_get_vector-bad-type.c: New test
case.
* jit.dg/test-error-gcc_jit_type_get_vector-non-power-of-two.c:
New test case.
* jit.dg/test-vector-types.cc: New test case.
---
 gcc/jit/docs/cp/topics/types.rst   |  14 ++
 gcc/jit/docs/topics/compatibility.rst  |   7 +
 gcc/jit/docs/topics/types.rst  |  43 -
 gcc/jit/jit-playback.c |  11 ++
 gcc/jit/jit-playback.h |   1 +
 gcc/jit/jit-recording.c|  56 ++-
 gcc/jit/jit-recording.h|  26 ++-
 gcc/jit/libgccjit++.h  |   8 +
 gcc/jit/libgccjit.c|  28 
 gcc/jit/libgccjit.h|  15 ++
 gcc/jit/libgccjit.map  |   5 +
 gcc/testsuite/jit.dg/all-non-failing-tests.h   |   2 +
 .../test-error-gcc_jit_type_get_vector-bad-type.c  |  30 
 ...rror-gcc_jit_type_get_vector-non-power-of-two.c |  29 
 gcc/testsuite/jit.dg/test-vector-types.cc  | 185 +
 15 files changed, 456 insertions(+), 4 deletions(-)
 create mode 100644 
gcc/testsuite/jit.dg/test-error-gcc_jit_type_get_vector-bad-type.c
 create mode 100644 
gcc/testsuite/jit.dg/test-error-gcc_jit_type_get_vector-non-power-of-two.c
 create mode 100644 gcc/testsuite/jit.dg/test-vector-types.cc

diff --git a/gcc/jit/docs/cp/topics/types.rst b/gcc/jit/docs/cp/topics/types.rst
index e85a492..1df896e 100644
--- a/gcc/jit/docs/cp/topics/types.rst
+++ b/gcc/jit/docs/cp/topics/types.rst
@@ -109,6 +109,20 @@ 

[Committed/AARCH64] Fix gcc.target/aarch64/vect-xorsign_exec.c testcase

[Andrew Pinski]

Hi,
  This testcase checks the assembly and does an execute of it so it
needs --save-temps like the other testcases.

Committed as obvious after test on aarch64-linux-gnu with no regressions.

Thanks,
Andrew

ChangeLog:
* gcc.target/aarch64/vect-xorsign_exec.c: Add --save-temps to the
options passed.
Index: gcc.target/aarch64/vect-xorsign_exec.c
===
--- gcc.target/aarch64/vect-xorsign_exec.c  (revision 251012)
+++ gcc.target/aarch64/vect-xorsign_exec.c  (working copy)
@@ -1,5 +1,5 @@
 /* { dg-do run } */
-/* { dg-options "-O2 -ftree-vectorize -fdump-tree-vect-details" } */
+/* { dg-options "-O2 -ftree-vectorize -fdump-tree-vect-details --save-temps" } 
*/
 
 extern void abort ();
 

Re: [PATCH][testsuite] Add check_effective_target_autoincdec

[Mike Stump]

On Aug 9, 2017, at 5:50 AM, Richard Earnshaw (lists)  
wrote:
> 
> On 09/08/17 12:37, Wilco Dijkstra wrote:
>> Richard Earnshaw wrote:
>>> Except that I think this would be better done as an 'effective target'
>>> test; something like
>>> 
>>> dg-require-effective-target autoincdec
>> 
>> Right I figured out how to do this - not trivial as it needs a secret flag 
>> in the
>> glob call - if anyone knows a better way of doing this I'd like to know!
>> 
>> 
>> Add check_effective_target_autoincdec that returns true if a target
>> runs the auto_inc_dec optimization pass.
>> 
>> OK for commit?
>> 
> 
> LGTM.  Please give Mike 24 hours to respond, before committing.

Ok.  I'm happy for things like this to just go in on port or front-end review 
alone.  It's a pretty standard sort of change, dare say, kinda trivial.

Re: [PATCH] libstdc++: Support std::is_aggregate on clang++ (was [cfe-dev] clang++: std::is_aggregate unusable with clang-5.0/libstdc++-7)

[Jonathan Wakely]

On 02/08/17 00:05 +0900, Katsuhiko Nishimra wrote:

On Mon, Jul 31, 2017 at 03:53:42PM +0100, Jonathan Wakely wrote:

This __has_bultin check only exists for Clang, so should be replaced
by the correct __is_identifier check, not left there in addition to
it.


I see. Actually I've guessed so, and thank you for clarifying it.
I'm attaching a replacing patch. Please take a look at it.


Thanks, I've committed this to GCC trunk and will backport it to the
gcc-7-branch after the GCC 7.2 release (which is due any day now).

I've also committed the attached patch which changes our feature
detection for a __has_unique_object_representations builtin, as I
expect that will also need to use __is_identifier if/when Clang
supports it.

Tested powerpc64le-linux, and also tested using Clang version 5.0.0
(trunk 307530) to confirm that __is_aggregate is correctly detected
and std::is_aggregate is defined.


From 1b22cc531027832cf1eb50b73354f1730edbba54 Mon Sep 17 00:00:00 2001
From: Katsuhiko Nishimra 
Date: Tue, 1 Aug 2017 20:36:58 +0900
Subject: [PATCH] libstdc++: Support std::is_aggregate on clang++

Currently, libstdc++ tries to detect __is_aggregate built-in macro using
__has_builtin, but this fails on clang++ because __has_builtin on
clang++ detects only built-in functions, not built-in macros. This patch
adds a test using __is_identifier. Tested with clang++
6.0.0-svn309616-1~exp1 and g++ 7.1.0-11 on Debian unstable.
---
libstdc++-v3/include/std/type_traits | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libstdc++-v3/include/std/type_traits 
b/libstdc++-v3/include/std/type_traits
index 390b6f40a..ee9c75baf 100644
--- a/libstdc++-v3/include/std/type_traits
+++ b/libstdc++-v3/include/std/type_traits
@@ -2894,9 +2894,9 @@ template 

#if __GNUC__ >= 7
# define _GLIBCXX_HAVE_BUILTIN_IS_AGGREGATE 1
-#elif defined __has_builtin
+#elif defined(__is_identifier)
// For non-GNU compilers:
-# if __has_builtin(__is_aggregate)
+# if ! __is_identifier(__is_aggregate)
#  define _GLIBCXX_HAVE_BUILTIN_IS_AGGREGATE 1
# endif
#endif
--
2.13.3





commit 1b00aa3825ea05fea071f31d29aafff71a002c53
Author: Jonathan Wakely 
Date:   Wed Aug 9 19:07:28 2017 +0100

Fix test for __has_unique_object_representations support in Clang

	* include/std/type_traits (_GLIBCXX_NO_BUILTIN_HAS_UNIQ_OBJ_REP):
	Replace with _GLIBCXX_HAVE_BUILTIN_HAS_UNIQ_OBJ_REP and use
	__is_identifier to set it.

diff --git a/libstdc++-v3/include/std/type_traits b/libstdc++-v3/include/std/type_traits
index ee9c75b..f021c42 100644
--- a/libstdc++-v3/include/std/type_traits
+++ b/libstdc++-v3/include/std/type_traits
@@ -2873,14 +2873,16 @@ template 
 template 
   inline constexpr bool is_convertible_v = is_convertible<_From, _To>::value;
 
-#ifdef __has_builtin
-# if !__has_builtin(__has_unique_object_representations)
-// Try not to break non-GNU compilers that don't support the built-in:
-#  define _GLIBCXX_NO_BUILTIN_HAS_UNIQ_OBJ_REP 1
+#if __GNUC__ >= 7
+# define _GLIBCXX_HAVE_BUILTIN_HAS_UNIQ_OBJ_REP 1
+#elif defined(__is_identifier)
+// For non-GNU compilers:
+# if ! __is_identifier(__has_unique_object_representations)
+#  define _GLIBCXX_HAVE_BUILTIN_HAS_UNIQ_OBJ_REP 1
 # endif
 #endif
 
-#ifndef _GLIBCXX_NO_BUILTIN_HAS_UNIQ_OBJ_REP
+#ifdef _GLIBCXX_HAVE_BUILTIN_HAS_UNIQ_OBJ_REP
 # define __cpp_lib_has_unique_object_representations 201606
   /// has_unique_object_representations
   template
@@ -2890,7 +2892,7 @@ template 
   )>
 { };
 #endif
-#undef _GLIBCXX_NO_BUILTIN_HAS_UNIQ_OBJ_REP
+#undef _GLIBCXX_HAVE_BUILTIN_HAS_UNIQ_OBJ_REP
 
 #if __GNUC__ >= 7
 # define _GLIBCXX_HAVE_BUILTIN_IS_AGGREGATE 1

Go patch committed: Fix shift type determination

[Ian Lance Taylor]

In Go it's possible to construct an lshift expression using
unsafe.Sizeof that is technically a compile-time constant but can't be
evaluated without going through backend methods.  This patch by Than
McIntosh ensures that in this case Type::make_non_abstract_type is
called on the numeric operand of the shift (as opposed to leaving as
abstract), to avoid an assert later on in the compiler flow.  This
fixes https://golang.org/issue/21372.  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 250992)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-5fd112e5c2968e94761c41519c451d789e23a92b
+480fdfa9dd416bd17115a94fa6021c4dd805fc39
 
 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 250873)
+++ gcc/go/gofrontend/expressions.cc(working copy)
@@ -5655,7 +5655,7 @@ Binary_expression::do_determine_type(con
 
   Type_context subcontext(*context);
 
-  if (is_constant_expr)
+  if (is_constant_expr && !is_shift_op)
 {
   subcontext.type = NULL;
   subcontext.may_be_abstract = true;

Re: [PATCH, i386]: Make stack canary location customizable (PR target/81708)

[Uros Bizjak]

On Tue, Aug 8, 2017 at 6:54 PM, Uros Bizjak  wrote:
> Hello!
>
> Attached patch introduces -mstack-protector-guard-reg=  and
> -mstack-protector-guard-offset= options to make stack canary location
> customizable. These are the same options powerpc has.

Attached addition adds -mstack-protector-guard-symbol= option that
overrides the offset to TLS stack protector canary with a symbol name.
Using this option, stack protector canary can be loaded from specified
symbol, relative to guard reg:

gcc -O2 -fstack-protector-all -mstack-protector-guard=tls
-mstack-protector-guard-reg=gs -mstack-protector-guard-symbol=my_guard

movq%gs:my_guard(%rip), %rax
movq%rax, 8(%rsp)
xorl%eax, %eax
movq8(%rsp), %rax
xorq%gs:my_guard(%rip), %rax

2017-08-09  Uros Bizjak  

PR target/81708
* config/i386/i386.opt (mstack-protector-guard-symbol=): New option
* config/i386/i386.c (ix86_stack_protect_guard): Use
ix86_stack_protect_guard_symbol_str to generate varible declaration.
* doc/invoke.texi (x86 Options): Document
-mstack-protector-guard-symbol= option.

testsuite/ChangeLog:

2017-08-09  Uros Bizjak  

PR target/81708
* gcc.target/i386/stack-prot-sym.c: New test.

Patch was bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.

I plan to commit the patch to mainline SVN in a couple of days.

Uros.
Index: config/i386/i386.c
===
--- config/i386/i386.c  (revision 250999)
+++ config/i386/i386.c  (working copy)
@@ -45858,6 +45858,8 @@ ix86_mangle_type (const_tree type)
 }
 }
 
+static GTY(()) tree ix86_tls_stack_chk_guard_decl;
+
 static tree
 ix86_stack_protect_guard (void)
 {
@@ -45864,15 +45866,47 @@ ix86_stack_protect_guard (void)
   if (TARGET_SSP_TLS_GUARD)
 {
   tree type_node = lang_hooks.types.type_for_mode (ptr_mode, 1);
-
   int qual = ENCODE_QUAL_ADDR_SPACE (ix86_stack_protector_guard_reg);
+  tree type = build_qualified_type (type_node, qual);
+  tree t;
 
-  tree type = build_qualified_type (type_node, qual);
-  tree asptrtype = build_pointer_type (type);
-  tree sspoff = build_int_cst (asptrtype,
-  ix86_stack_protector_guard_offset);
-  tree t = build2 (MEM_REF, asptrtype, sspoff,
-  build_int_cst (asptrtype, 0));
+  if (global_options_set.x_ix86_stack_protector_guard_symbol_str)
+   {
+ t = ix86_tls_stack_chk_guard_decl;
+
+ if (t == NULL)
+   {
+ rtx x;
+
+ t = build_decl
+   (UNKNOWN_LOCATION, VAR_DECL,
+get_identifier (ix86_stack_protector_guard_symbol_str),
+type);
+ TREE_STATIC (t) = 1;
+ TREE_PUBLIC (t) = 1;
+ DECL_EXTERNAL (t) = 1;
+ TREE_USED (t) = 1;
+ TREE_THIS_VOLATILE (t) = 1;
+ DECL_ARTIFICIAL (t) = 1;
+ DECL_IGNORED_P (t) = 1;
+
+ /* Do not share RTL as the declaration is visible outside of
+current function.  */
+ x = DECL_RTL (t);
+ RTX_FLAG (x, used) = 1;
+
+ ix86_tls_stack_chk_guard_decl = t;
+   }
+   }
+  else
+   {
+ tree asptrtype = build_pointer_type (type);
+
+ t = build_int_cst (asptrtype, ix86_stack_protector_guard_offset);
+ t = build2 (MEM_REF, asptrtype, t,
+ build_int_cst (asptrtype, 0));
+   }
+
   return t;
 }
 
Index: config/i386/i386.opt
===
--- config/i386/i386.opt(revision 250999)
+++ config/i386/i386.opt(working copy)
@@ -938,6 +938,10 @@ Use the given offset for addressing the stack-prot
 TargetVariable
 HOST_WIDE_INT ix86_stack_protector_guard_offset = 0
 
+mstack-protector-guard-symbol=
+Target RejectNegative Joined Integer Var(ix86_stack_protector_guard_symbol_str)
+Use the given symbol for addressing the stack-protector guard.
+
 mmitigate-rop
 Target Var(flag_mitigate_rop) Init(0)
 Attempt to avoid generating instruction sequences containing ret bytes.
Index: doc/invoke.texi
===
--- doc/invoke.texi (revision 250999)
+++ doc/invoke.texi (working copy)
@@ -1216,7 +1216,8 @@ See RS/6000 and PowerPC Options.
 -mavx256-split-unaligned-load  -mavx256-split-unaligned-store @gol
 -malign-data=@var{type}  -mstack-protector-guard=@var{guard} @gol
 -mstack-protector-guard-reg=@var{reg} @gol
--mstack-protector-guard-offset=@var{offset}  -mmitigate-rop @gol
+-mstack-protector-guard-offset=@var{offset} @gol
+-mstack-protector-guard-symbol=@var{symbol} -mmitigate-rop @gol
 -mgeneral-regs-only  -mcall-ms2sysv-xlogues}
 
 @emph{x86 Windows Options}
@@ -22753,9 +22754,11 @@ The @option{-mno-compat-align-parm} 

[PATCH] rs6000: Use SAVE_MULTIPLE only if we restore what it saves (PR80938)

[Segher Boessenkool]

We can have SAVE_MULTIPLE while we do not have REST_MULTIPLE.  If the
inline restore does not restore all registers, the CFI for the save
and restore can conflict if things are shrink-wrapped.

We could restore all registers that are saved (not ideal), or emit
the CFI notes to say we did (which will work fine, but is also not
so great); instead, let's not save the registers that are unused.

Tested on powerpc64-linux {-m32,-m64}; committing to trunk.


Segher


2017-08-09  Segher Boessenkool  

PR target/80938
* config/rs6000/rs6000.c (rs6000_savres_strategy): Don't use
SAVE_MULTIPLE if not all the registers that saves, should be saved.

---
 gcc/config/rs6000/rs6000.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 0f0b1ff..e8cdd25 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -24437,6 +24437,21 @@ rs6000_savres_strategy (rs6000_stack_t *info,
   else if (!lr_save_p && info->first_gp_reg_save > 29)
 strategy |= SAVE_INLINE_GPRS | REST_INLINE_GPRS;
 
+  /* We can only use save multiple if we need to save all the registers from
+ first_gp_reg_save.  Otherwise, the CFI gets messed up (we save some
+ register we do not restore).  */
+  if (strategy & SAVE_MULTIPLE)
+{
+  int i;
+
+  for (i = info->first_gp_reg_save; i < 32; i++)
+   if (fixed_reg_p (i) || !save_reg_p (i))
+ {
+   strategy &= ~SAVE_MULTIPLE;
+   break;
+ }
+}
+
   /* We can only use load multiple or the out-of-line routines to
  restore gprs if we've saved all the registers from
  first_gp_reg_save.  Otherwise, we risk loading garbage.
-- 
1.9.3

Re: [PATCH, rs6000] (v2) enable early debug and disable switch for gimple folding

[Segher Boessenkool]

On Wed, Aug 09, 2017 at 09:39:08AM -0500, Will Schmidt wrote:
> I also fixed the (missing) space after
> cast for the existing debug code.

Thanks for that :-)


>   * config/rs6000/rs6000.c: (rs6000_option_override_internal) Add blurb
>   to indicate when early gimple folding has been disabled.

The colon goes after the closing parenthesis.

>   (rs6000_invalid_builtin): whitespace.

Capital W.  Or, say what this does?  "Fix whitespace."

> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index 1fb9861..f970f9e 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -4180,10 +4180,14 @@ rs6000_option_override_internal (bool global_init_p)
>  {
>warning (0, N_("-maltivec=le not allowed for big-endian targets"));
>rs6000_altivec_element_order = 0;
>  }
>  
> +  if (!rs6000_fold_gimple)
> + fprintf (stderr,
> + "gimple folding of rs6000 builtins has been disabled.\n");

That first " should line up with the s in stderr.

Should this print a message though?

> +  size_t uns_fncode = (size_t) fn_code;
> +  enum insn_code icode = rs6000_builtin_info[uns_fncode].icode;
> +  const char *fn_name1 = rs6000_builtin_info[uns_fncode].name;
> +  const char *fn_name2 = ((icode != CODE_FOR_nothing)
> +   ? get_insn_name ((int) icode)
> +   : "nothing");

Similarly, the ? and : should line up with the second (.
(You can also remove the superfluous outer parens).

> +  if (TARGET_DEBUG_BUILTIN)
> +  {
> +  fprintf (stderr, "rs6000_gimple_fold_builtin %d %s %s\n",
> +fn_code, fn_name1, fn_name2);
> +  }

Wrong indent on the {}; but you don't need those here anyway, it's a
single statement.

>  default:
> + if (TARGET_DEBUG_BUILTIN)
> +   fprintf (stderr, "gimple builtin intrinsic not matched:%d %s %s \n",
> + fn_code, fn_name1, fn_name2);

No space before \n please.

> +mfold-gimple
> +Target Report Var(rs6000_fold_gimple, 1) Init(1)
> +Enable early gimple folding of builtins.

You can drop the ", 1" I think?

Okay with those nits fixed.


Segher

Re: [PATCH 1/3] improve detection of attribute conflicts (PR 81544)

[Joseph Myers]

On Wed, 9 Aug 2017, Martin Sebor wrote:

> the same function with the other of this pair attributes.  I'd
> also be okay with not diagnosing this combination if I could
> convince myself that it's safe (or can be made safe) and treated
> consistently.

I'd expect it to be safe; it might simply happen that some calls are 
optimized based on incomplete information (and even that is doubtful, as 
all optimizations except front-end folding should be taking place after 
all the declarations have been seen).

> The problem whose subset the diagnostic detects is declaring
> the function const and calling it before the pure declaration
> that corresponds to its definition is seen.  I.e., the inverse
> of what the ssa-ccp-2.c test does.  I think a mismatch between
> the attributes is as suspect as a mismatch in the function's
> signature.

In the ssa-ccp-2.c case, it's not clear to me the test intends to use the 
same function at all; maybe one of the foo9 functions should be renamed.

I think it's actually like having a non-prototype declaration and a 
prototype one, where a composite type is formed from two compatible types.  
(We have a warning in the very specific case of a non-prototype 
*definition* followed by a prototype declaration.)

> In any event, it sounds to me that conceptually, it might be
> useful to be able to specify which of a pair of mutually
> exclusive (or redundant) attributes to retain and/or accept:
> the first or the second, and not just whether to accept or
> drop the most recent one.  That will make it possible to make
> the most appropriate decision about each specific pair of
> attributes without impacting any of the others.

If they conflict, I'm not sure there's any basis for making a choice 
specific to a particular pair of attributes.

In cases like const and pure where one is a subset of the other, it makes 
sense to choose based on the pair of attributes - and not necessarily to 
warn under the same conditions as the warnings for conflicting attributes.

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

*ping* [patch, fortran] Fix PR 60355, missing error for BIND(C) outside module scope

[Thomas Koenig]

Am 02.08.2017 um 15:19 schrieb Thomas Koenig:

the attached patch is a bit smaller than it looks, because most of
it is due to reformatting a large comment.  It is rather simple -
checking for an incorrectly placed BIND(C) variable was sometimes not
done because the test was mixed in with other tests where implicitly
typed variables were excluded.

Regression-tested. OK for trunk?


Ping.

Patch at:

https://gcc.gnu.org/ml/fortran/2017-08/msg00010.html

Regards

Thomas

Re: [PATCH][v2] Fix target attribute handling (PR c++/81355).

[Jason Merrill]

On Wed, Aug 9, 2017 at 6:45 AM, Martin Liška  wrote:
> On 08/08/2017 08:03 PM, Martin Sebor wrote:
>> On 08/07/2017 10:59 PM, Martin Liška wrote:
>>> On 08/02/2017 09:56 PM, Martin Sebor wrote:
 On 08/02/2017 01:04 PM, Jeff Law wrote:
> On 07/28/2017 05:13 AM, Martin Liška wrote:
>> Hello.
>>
>> Following patch skips empty strings in 'target' attribute.
>> Patch can bootstrap on ppc64le-redhat-linux and survives regression 
>> tests.
>>
>> Ready to be installed?
>> Martin
>>
>> gcc/ChangeLog:
>>
>> 2017-07-13  Martin Liska  
>>
>> PR c++/81355
>> * attribs.c (sorted_attr_string): Skip empty strings.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2017-07-13  Martin Liska  
>>
>> PR c++/81355
>> * g++.dg/other/pr81355.C: New test.
> OK.  THough one could legitimately ask if this ought to be a parsing 
> error.

 I would suggest to at least issue a warning.  It seems that
 the empty string must almost certainly be a bug here, say due
 to unintended macro expansion.

 Otherwise, if it should remain silently (and intentionally)
 accepted, I recommend to document it.  As it is, the manual
 says that the "string" argument is equivalent to compiling
 with -mstring which for "" would be rejected by the driver.

 Martin
>>>
>>> Thanks you both for feedback. I decided to come up with an error message 
>>> for that.
>>>
>>> Feedback appreciated.
>>
>> My only comment is on the text of the error message.  I think
>> the %' directive is supposed to be used instead of a bare
>> apostrophe.  But rather than using the somewhat informal "can't"
>> I would suggest to follow other similar diagnostics that might
>> print something like:
>>
>>   empty string in attribute %
>>
>> (analogous to "empty precision in %s format" or "empty scalar
>> initializer" etc. in gcc.pot)
>>
>> or
>>
>>   attribute % argument may not be empty
>>
>> (similar to "output filename may not be empty").
>>
>> Martin
>
> Hi.
>
> Thank you both for review, both notes resolved in v3.
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>
> Ready to be installed?

OK.

Jason

Re: [PATCH 1/3] improve detection of attribute conflicts (PR 81544)

[Martin Sebor]

On 08/09/2017 11:13 AM, Joseph Myers wrote:

On Wed, 9 Aug 2017, Martin Sebor wrote:


diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c
b/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c
index 146b76c..58a4742 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c
@@ -113,7 +113,7 @@ int test9 (int *intarr)

 int test99 (int *intarr)
 {
-  extern int foo9 (int) __attribute__ ((pure));
+  extern int foo9 (int) __attribute__ ((const));
   int h, v;
   g9 = 9;
   h = foo9 (g9);



And why this?  I'd avoid modifying existing tests like that unless
it's directly related to the new diagnostic.


The same function is declared earlier on in the file with attribute
const and the redeclaration with attribute pure triggers a warning
due to the two being considered mutually exclusive.


const + pure is a *redundant* combination, but I think it should always
have the meaning of const regardless of the order in which they appear or
whether they appear on separate declarations.  That's different from
combinations that are actually nonsensical.


To a large degree I agree with this characterization.  I think
of one as a subset of the other.  FWIW, my initial approach was
to introduce the concept of attribute subsets (as in pure being
a subset of the restrictions of const). But in discussing it
with Marek we felt it was too complicated and that mutual
exclusivity was good enough here.  I'd be fine with reintroducing
the subset/superset distinction, or any other that makes sense
and helps find potential bugs that might result from redeclaring
the same function with the other of this pair attributes.  I'd
also be okay with not diagnosing this combination if I could
convince myself that it's safe (or can be made safe) and treated
consistently.


It's not clear to me that
const + pure should be diagnosed by default any more than declaring the
same function twice, with the const attribute both time, should be
diagnosed.


The problem whose subset the diagnostic detects is declaring
the function const and calling it before the pure declaration
that corresponds to its definition is seen.  I.e., the inverse
of what the ssa-ccp-2.c test does.  I think a mismatch between
the attributes is as suspect as a mismatch in the function's
signature.

In any event, it sounds to me that conceptually, it might be
useful to be able to specify which of a pair of mutually
exclusive (or redundant) attributes to retain and/or accept:
the first or the second, and not just whether to accept or
drop the most recent one.  That will make it possible to make
the most appropriate decision about each specific pair of
attributes without impacting any of the others.

If this sounds right to you (and others) let me make that
change and post an updated patch.

But If I misunderstood and this solution wouldn't help please
let me know.

Martin

PS I've also wondered if -Wattributes is the right option to use
for these sorts of conflicts/redundancies.  It is the only option
used now but the manual makes it sound that -Wignored-attributes
should be expected.  AFAICS, -Wignored-attributes is only used by
the C++ front end for templates.  Should it be extended to these
cases as well?

Re: [PATCH] matching tokens: C++ parts (v3)

[Jason Merrill]

On Tue, Aug 8, 2017 at 5:01 PM, David Malcolm  wrote:
> On Mon, 2017-08-07 at 14:25 -0400, Jason Merrill wrote:
>
> Thanks for looking at this.
>
>> On 08/01/2017 04:21 PM, David Malcolm wrote:
>> > @@ -27632,6 +27769,9 @@ cp_parser_sizeof_operand (cp_parser*
>> > parser, enum rid keyword)
>> >  {
>> >tree type = NULL_TREE;
>> >
>> > +  matching_parens parens;
>> > +  parens.peek_open (parser);
>>
>> I was puzzled by this until I found that
>> cp_parser_compound_literal_p
>> consumes the open paren.  Let's remove that in favor of calling
>> consume_open here, so we don't need peek_open anymore.
>
> Done.
>
>> About passing parser in or not, I'm happy with the current approach;
>> adding things to the stack isn't free in a highly recursive program
>> like GCC.
>
> Thanks; I'll keep "parser" out of the new classes then.
>
> Here's an updated "v3" patch.
>
> Successfully bootstrapped on x86_64-pc-linux-gnu in conjunction
> with the other patches (1 and 2 of the v2 kit).
>
> OK for trunk, assuming the other patches are approved? (patch 2 in the kit,
> for the C frontend, still needs approval).

OK.

Jason

C++ PATCH for c++/81525, auto and generic lambda

[Jason Merrill]

In this testcase, when building up an extra version of N to refer to
when instantiating the generic lambda, we were mistakenly replacing
the 'auto' with a template argument for the generic lambda.

Tested x86_64-pc-linux-gnu, applying to trunk and 7.
commit e42e1cc162267b85adfb624daf1b96fc2f5a6f5b
Author: Jason Merrill 
Date:   Mon Aug 7 15:05:51 2017 -0400

PR c++/81525 - wrong constant value with generic lambda

* pt.c (tsubst_decl) [VAR_DECL]: Avoid clobbering auto.
(tsubst_copy) [VAR_DECL]: Handle auto.

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 3d6f4b5..0f899b9 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -12875,7 +12875,15 @@ tsubst_decl (tree t, tree args, tsubst_flags_t 
complain)
&& VAR_HAD_UNKNOWN_BOUND (t)
&& type != error_mark_node)
  type = strip_array_domain (type);
+   tree auto_node = type_uses_auto (type);
+   int len = TREE_VEC_LENGTH (args);
+   if (auto_node)
+ /* Mask off any template args past the variable's context so we
+don't replace the auto with an unrelated argument.  */
+ TREE_VEC_LENGTH (args) = TEMPLATE_TYPE_LEVEL (auto_node) - 1;
type = tsubst (type, args, complain, in_decl);
+   if (auto_node)
+ TREE_VEC_LENGTH (args) = len;
  }
if (VAR_P (r))
  {
@@ -14656,6 +14664,10 @@ tsubst_copy (tree t, tree args, tsubst_flags_t 
complain, tree in_decl)
DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (r)
  = TREE_CONSTANT (r) = true;
  DECL_INITIAL (r) = init;
+ if (tree auto_node = type_uses_auto (TREE_TYPE (r)))
+   TREE_TYPE (r)
+ = do_auto_deduction (TREE_TYPE (r), init, auto_node,
+  complain, adc_variable_type);
}
  gcc_assert (cp_unevaluated_operand || TREE_STATIC (r)
  || decl_constant_var_p (r)
diff --git a/gcc/testsuite/g++.dg/cpp1y/lambda-generic-const4.C 
b/gcc/testsuite/g++.dg/cpp1y/lambda-generic-const4.C
new file mode 100644
index 000..52f4373
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/lambda-generic-const4.C
@@ -0,0 +1,20 @@
+// PR c++/81525
+// { dg-do compile { target c++14 } }
+
+template  struct A {
+  constexpr operator int () const { return i; }
+};
+template  constexpr A a = {};
+
+template  void foo (F f) {
+  f (A<0>{});
+}
+template 
+void bar (T) {
+  constexpr auto N = a<1>;
+  auto f = [&] (auto i) {
+static_assert (static_cast(N) == 1, "");
+  };
+  foo (f);
+}
+int main () { bar (0); }

Re: [PATCH] enhance overflow and truncation detection in strncpy and strncat (PR 81117)

[Jeff Law]

On 08/06/2017 02:07 PM, Martin Sebor wrote:
>>>
>>> You're right that there is no truncation and the effect is
>>> the same but only in the unlikely case when the destination
>>> is empty.  Otherwise the result is truncated.
>> Maybe this is where I'm confused.  How does the destination play into
>> truncation issues?  I've always been under the impression that the
>> destination has to be large enough to hold the result, but that it
>> doesn't affect truncation of the source.
> 
> No, you're right.  It's me who's been confused, either thinking
> of strncpy or -Wstringop-overflow.  The difference between the
> two warnings is just one byte in some cases and I got them mixed
> up.  Sorry about that and thanks for spotting this silly  mistake!
> I've updated the code to issue -Wstringop-overflow here and added
> a better example to the manual.
Thanks.  I kept looking thinking I must have missed something somewhere...


> 
> 1) In the following, the strncpy call would normally trigger
> -Wstringop-truncation because of the possible missing terminating
> NUL, but because the immediately following statement inserts the
> NUL the call is safe.
> 
>strncpy (d, s, sizeof d);   // possible missing NUL
>d[sizeof d - 1] = '\0'; // okay, NUL add here
At first I wondered if this was an optimization opportunity.  BUt
thinking more about it, I don't think it is, unless you happen to know
that sizeof d == sizeof s, which I doubt happens often enough to matter.


> 
> 2) Building Glibc made me realize that in my effort to detect
> the (common) misuses of strncpy I neglected the original (and
> only intended but now rare) use case of filling a buffer
> without necessarily NUL-terminating it (as in struct dirent::
> d_name).  To allow it the patch adds a new attribute that can
> be used to annotate char arrays and pointers that are intended
> not to be NUL-terminated.  This suppresses the truncation
> warning.  When the patch is approved I'll propose the (trivial)
> Glibc changes.  In the future, the attribute will also let GCC
> warn when passing such objects to functions that expect a NUL-
> terminated string argument (e.g., strlen or strcpy).
Seems reasonable.

> 
> 3) Finally, to add inlining context to diagnostics issued by
> the middle end, I've added a new %G directive to complement
> %K by accepting a gcall* argument.
Also seems reasonable.  I think we've wanted something like this for a
while.


> 
> To make the patch easier to review I've broken it up into
> four parts:
> 
>   1. Add %G.
>   2. Add attribute nostring.
>   3. Implement -Wstringop-truncation and enhance -Wstringop-
>  overflow (the meat of the patch).
>   4. Fix up GCC to compile with the new and enhanced warnings.
I'll try to get to them today.

Thanks,
jeff

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

[Jason Merrill]

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

Tested x86_64-pc-linux-gnu, applying to trunk.
commit 616193fb149d17e8a671a46c03dca98aecbcc752
Author: Jason Merrill 
Date:   Tue Aug 8 12:49:42 2017 -0400

PR c++/81359 - Unparsed NSDMI error from SFINAE context.

* init.c (get_nsdmi): Add complain parm.
* typeck2.c (digest_nsdmi_init): Add complain parm.
(process_init_constructor_record): Pass complain to get_nsdmi.
* pt.c (maybe_instantiate_noexcept): Add complain parm, return bool.
* method.c (get_defaulted_eh_spec): Add complain parm.  Pass it into
synthesized_method_walk.
(synthesized_method_walk): Adjust.
(walk_field_subobs): Pass complain to get_nsdmi.
(defaulted_late_check): Skip other checks if deleted.
* decl2.c (mark_used): Pass complain to maybe_instantiate_noexcept.
* call.c (build_aggr_conv): Pass complain to get_nsdmi.
* parser.c (defarg_location): New.
* error.c (location_of): Use it.

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index fdd3731..4903119 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -916,7 +916,7 @@ build_aggr_conv (tree type, tree ctor, int flags, 
tsubst_flags_t complain)
   if (i < CONSTRUCTOR_NELTS (ctor))
val = CONSTRUCTOR_ELT (ctor, i)->value;
   else if (DECL_INITIAL (field))
-   val = get_nsdmi (field, /*ctor*/false);
+   val = get_nsdmi (field, /*ctor*/false, complain);
   else if (TREE_CODE (ftype) == REFERENCE_TYPE)
/* Value-initialization of reference is ill-formed.  */
return NULL;
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index 508570b..94738bd 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -8093,8 +8093,9 @@ resolve_address_of_overloaded_function (tree target_type,
  continue;
 
/* In C++17 we need the noexcept-qualifier to compare types.  */
-   if (flag_noexcept_type)
- maybe_instantiate_noexcept (fn);
+   if (flag_noexcept_type
+   && !maybe_instantiate_noexcept (fn, complain))
+ continue;
 
/* See if there's a match.  */
tree fntype = static_fn_type (fn);
@@ -8176,7 +8177,7 @@ resolve_address_of_overloaded_function (tree target_type,
 
  /* In C++17 we need the noexcept-qualifier to compare types.  */
  if (flag_noexcept_type)
-   maybe_instantiate_noexcept (instantiation);
+   maybe_instantiate_noexcept (instantiation, complain);
 
  /* See if there's a match.  */
  tree fntype = static_fn_type (instantiation);
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 115cdaf..3a0bd16 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6297,7 +6297,7 @@ extern tree get_type_value(tree);
 extern tree build_zero_init(tree, tree, bool);
 extern tree build_value_init   (tree, tsubst_flags_t);
 extern tree build_value_init_noctor(tree, tsubst_flags_t);
-extern tree get_nsdmi  (tree, bool);
+extern tree get_nsdmi  (tree, bool, tsubst_flags_t);
 extern tree build_offset_ref   (tree, tree, bool,
 tsubst_flags_t);
 extern tree throw_bad_array_new_length (void);
@@ -6355,7 +6355,7 @@ extern bool trivial_fn_p  (tree);
 extern tree forward_parm   (tree);
 extern bool is_trivially_xible (enum tree_code, tree, tree);
 extern bool is_xible   (enum tree_code, tree, tree);
-extern tree get_defaulted_eh_spec  (tree);
+extern tree get_defaulted_eh_spec  (tree, tsubst_flags_t = 
tf_warning_or_error);
 extern void after_nsdmi_defaulted_late_checks   (tree);
 extern bool maybe_explain_implicit_delete  (tree);
 extern void explain_implicit_non_constexpr (tree);
@@ -6385,6 +6385,7 @@ extern tree cp_convert_range_for (tree, tree, tree, tree, 
unsigned int, bool);
 extern bool parsing_nsdmi (void);
 extern bool parsing_default_capturing_generic_lambda_in_template (void);
 extern void inject_this_parameter (tree, cp_cv_quals);
+extern location_t defarg_location (tree);
 
 /* in pt.c */
 extern bool check_template_shadow  (tree);
@@ -6448,7 +6449,7 @@ extern int more_specialized_fn(tree, 
tree, int);
 extern void do_decl_instantiation  (tree, tree);
 extern void do_type_instantiation  (tree, tree, tsubst_flags_t);
 extern bool always_instantiate_p   (tree);
-extern void maybe_instantiate_noexcept (tree);

Re: [PATCH] i386: Don't use frame pointer without stack access

[H.J. Lu]

On Wed, Aug 9, 2017 at 10:28 AM, H.J. Lu  wrote:
> On Wed, Aug 9, 2017 at 8:26 AM, Andi Kleen  wrote:
>>> This must be much more specific.  How does it impact:
>>>
>>> 1. LTO
>>> 2. Function inlining.
>>> 3. Partial function inlining.
>>> 4. Shrink-wrapping.
>>>
>>> Any of them can impact function stack frame.
>>
>> It doesn't. It's just to get back to the previous state.
>>
>> Also these others already have explicit options to disable them.
>>
>
> How about
>
> item -fkeep-frame-pointer
> @opindex fkeep-frame-pointer
> Keep the frame pointer in a register for functions.  This option always
> forces a new stack frame for all functions regardless of whether
> @option{-fomit-frame-pointer} is enabled or not.  Disabled by default.
>

Here is the updated patch with -fkeep-frame-pointer.

OK for trunk?

-- 
H.J.
From 6f5b42bbe2baa7977dcbf388219dabcb4a6b546d Mon Sep 17 00:00:00 2001
From: "H.J. Lu" 
Date: Sun, 6 Aug 2017 06:24:36 -0700
Subject: [PATCH] i386: Don't use frame pointer without stack access

When there is no stack access, there is no need to use frame pointer
even if -fno-omit-frame-pointer is used and caller's frame pointer is
unchanged.  A command-line option, -fkeep-frame-pointer, is added to
always force a new stack frame.

gcc/

	PR target/81736
	* common.opt (-fomit-frame-pointer): Override
	-fkeep-frame-pointer.
	(-fkeep-frame-pointer): New command-line option.
	* config/i386/i386.c (ix86_finalize_stack_realign_flags): Renamed
	to ...
	(ix86_finalize_stack_frame_flags): This.  Also clear
	frame_pointer_needed if -fkeep-frame-pointer isn't used and
	,-fno-omit-frame-pointer is used without stack access.
	(ix86_expand_prologue): Replace ix86_finalize_stack_realign_flags
	with ix86_finalize_stack_frame_flags.
	(ix86_expand_epilogue): Likewise.
	(ix86_expand_split_stack_prologue): Likewise.
	* doc/invoke.texi: Document -fkeep-frame-pointer.  Update
	-fomit-frame-pointer.  Add a note for -fno-omit-frame-pointer.
	* toplev.c (process_options): Disable -fomit-frame-pointer if
	-fkeep-frame-pointer is used.

gcc/testsuite/

	PR target/81736
	* gcc.target/i386/pr81736-1a.c: New test.
	* gcc.target/i386/pr81736-1b.c: Likewise.
	* gcc.target/i386/pr81736-1c.c: Likewise.
	* gcc.target/i386/pr81736-1d.c: Likewise.
	* gcc.target/i386/pr81736-1e.c: Likewise.
	* gcc.target/i386/pr81736-1f.c: Likewise.
	* gcc.target/i386/pr81736-1g.c: Likewise.
	* gcc.target/i386/pr81736-2.c: Likewise.
	* gcc.target/i386/pr81736-3.c: Likewise.
	* gcc.target/i386/pr81736-4.c: Likewise.
	* gcc.target/i386/pr81736-5.c: Likewise.
	* gcc.target/i386/pr81736-6.c: Likewise.
	* gcc.target/i386/pr81736-7.c: Likewise.
---
 gcc/common.opt |  6 +-
 gcc/config/i386/i386.c | 24 +---
 gcc/doc/invoke.texi| 13 -
 gcc/testsuite/gcc.target/i386/pr81736-1a.c | 13 +
 gcc/testsuite/gcc.target/i386/pr81736-1b.c |  7 +++
 gcc/testsuite/gcc.target/i386/pr81736-1c.c |  7 +++
 gcc/testsuite/gcc.target/i386/pr81736-1d.c |  7 +++
 gcc/testsuite/gcc.target/i386/pr81736-1e.c |  7 +++
 gcc/testsuite/gcc.target/i386/pr81736-1f.c |  7 +++
 gcc/testsuite/gcc.target/i386/pr81736-1g.c |  7 +++
 gcc/testsuite/gcc.target/i386/pr81736-2.c  | 14 ++
 gcc/testsuite/gcc.target/i386/pr81736-3.c  | 11 +++
 gcc/testsuite/gcc.target/i386/pr81736-4.c  | 11 +++
 gcc/testsuite/gcc.target/i386/pr81736-5.c  | 20 
 gcc/testsuite/gcc.target/i386/pr81736-6.c  | 16 
 gcc/testsuite/gcc.target/i386/pr81736-7.c  | 13 +
 gcc/toplev.c   |  4 
 17 files changed, 174 insertions(+), 13 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr81736-1a.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr81736-1b.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr81736-1c.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr81736-1d.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr81736-1e.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr81736-1f.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr81736-1g.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr81736-2.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr81736-3.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr81736-4.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr81736-5.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr81736-6.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr81736-7.c

diff --git a/gcc/common.opt b/gcc/common.opt
index 1cb1c83d306..b73564d7145 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1881,9 +1881,13 @@ EnumValue
 Enum(offload_abi) String(lp64) Value(OFFLOAD_ABI_LP64)
 
 fomit-frame-pointer
-Common Report Var(flag_omit_frame_pointer) Optimization
+Common Report Var(flag_omit_frame_pointer) Negative(fkeep-frame-pointer) Optimization
 When 

Re: [PATCH] PR libstdc++/81751 don't call fflush(NULL)

[Jonathan Wakely]

On 09/08/17 18:39 +0200, Paolo Carlini wrote:

Hi,


On 9 Aug 2017, at 17:56, Jonathan Wakely  wrote:

This fixes a couple of problems in __gnu_cxx::stdio_filebuf,
specifically in the __basic_file::sys_open(FILE*, openmode) function
it uses when constructed from an existing FILE stream.

Firstly, r86756 changed __basic_file::sys_open(FILE*, openmode) to put
the call to sync() before the assignment that sets _M_cfile. This
means the fflush(_M_cfile) call has a null argument, and flushes all
open streams. I don't think that was intentional, and we only want to
flush the FILE we're constructing the streambuf with. (I think this is
a regression from 3.4.3, which just flushed the one stream).

Secondly, we zero errno so that we can tell if fflush sets it. We need
to restore the original value to meet the promise we make at
https://gcc.gnu.org/onlinedocs/libstdc++/manual/errno.html

Paolo, does the this->sync() to fflush(__file) change look right?


Yes, I went through the audit trail of libstdc++/81751 and the change looks 
good. Certainly we didn't intentionally want the behavior of fflush(0)!


OK, thanks - I've committed the change to trunk now.

Re: [PATCH] i386: Don't use frame pointer without stack access

[H.J. Lu]

On Wed, Aug 9, 2017 at 8:26 AM, Andi Kleen  wrote:
>> This must be much more specific.  How does it impact:
>>
>> 1. LTO
>> 2. Function inlining.
>> 3. Partial function inlining.
>> 4. Shrink-wrapping.
>>
>> Any of them can impact function stack frame.
>
> It doesn't. It's just to get back to the previous state.
>
> Also these others already have explicit options to disable them.
>

How about

item -fkeep-frame-pointer
@opindex fkeep-frame-pointer
Keep the frame pointer in a register for functions.  This option always
forces a new stack frame for all functions regardless of whether
@option{-fomit-frame-pointer} is enabled or not.  Disabled by default.


-- 
H.J.

libgo patch committed: Fix math.Ldexp for large powers

[Ian Lance Taylor]

The libgo implementation of math.Ldexp declared the libc "ldexp" as
taking an 'int' exponent argument, which is not quite right for 64-bit
platforms (exp arg is always int32); this could yield incorrect
results for exponent values outside the range of Minint32/Maxint32.
This patch by Than McIntosh fixes this by upating the type for the
libc version of ldexp, and adding guards to screen for out-of-range
exponents.  This fixes https://golang.org/issue/21323.  Bootstrapped
and ran Go tests on x86_64-pc-linux-gnu.  Committed to mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 250873)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-db685a1a9aa8b3b916dd6d1284895e01d73158e1
+5fd112e5c2968e94761c41519c451d789e23a92b
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: libgo/go/math/ldexp.go
===
--- libgo/go/math/ldexp.go  (revision 250873)
+++ libgo/go/math/ldexp.go  (working copy)
@@ -13,10 +13,15 @@ package math
 // Ldexp(NaN, exp) = NaN
 
 //extern ldexp
-func libc_ldexp(float64, int) float64
+func libc_ldexp(float64, int32) float64
 
 func Ldexp(frac float64, exp int) float64 {
-   r := libc_ldexp(frac, exp)
+   if exp > MaxInt32 {
+   exp = MaxInt32
+   } else if exp < MinInt32 {
+   exp = MinInt32
+   }
+   r := libc_ldexp(frac, int32(exp))
return r
 }
 

Re: [PATCH 1/3] improve detection of attribute conflicts (PR 81544)

[Joseph Myers]

On Wed, 9 Aug 2017, Martin Sebor wrote:

> > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c
> > > b/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c
> > > index 146b76c..58a4742 100644
> > > --- a/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c
> > > +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c
> > > @@ -113,7 +113,7 @@ int test9 (int *intarr)
> > > 
> > >  int test99 (int *intarr)
> > >  {
> > > -  extern int foo9 (int) __attribute__ ((pure));
> > > +  extern int foo9 (int) __attribute__ ((const));
> > >int h, v;
> > >g9 = 9;
> > >h = foo9 (g9);
> > > 
> > 
> > And why this?  I'd avoid modifying existing tests like that unless
> > it's directly related to the new diagnostic.
> 
> The same function is declared earlier on in the file with attribute
> const and the redeclaration with attribute pure triggers a warning
> due to the two being considered mutually exclusive.

const + pure is a *redundant* combination, but I think it should always 
have the meaning of const regardless of the order in which they appear or 
whether they appear on separate declarations.  That's different from 
combinations that are actually nonsensical.  It's not clear to me that 
const + pure should be diagnosed by default any more than declaring the 
same function twice, with the const attribute both time, should be 
diagnosed.

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

Re: [PATCH 0/5] RFC, WIP: RTL cost improvements

[Segher Boessenkool]

On Wed, Aug 09, 2017 at 05:54:40PM +0100, Richard Sandiford wrote:
> Segher Boessenkool  writes:
> > We need it, for example, to properly cost the various define_insn_and_split
> > (for which "type" is only an approximation, and is woefully inadequate
> > for determining cost).
> 
> But define_insn_and_splits could override the cost explicitly if they
> need to.  That seems neater than testing for them in C.

All 190 of them?  Not counting those that are define_insn+define_split
(we still have way too many of those).

Neat, indeed, but not altogether practical :-(


Segher

Re: [PATCH 0/5] RFC, WIP: RTL cost improvements

[Segher Boessenkool]

On Tue, Aug 08, 2017 at 10:41:05AM -0600, Jeff Law wrote:
> On 08/05/2017 11:15 AM, Segher Boessenkool wrote:
> > For size cost I currently use just "length", but I haven't looked at
> > size cost much at all yet.
> I think that's fine.  "length" is pretty standardized at this point and
> it's the right metric.  For ports that don't bother defining a length
> attribute, punt in some reasonable manner.

I do this is in the target hook, not in generic code.  Commonizing
two lines of code (at the cost of extra complexity for targets that
do *not* want to handle things that way) is not worth it IMO.


Segher

Re: [PATCH 0/5] RFC, WIP: RTL cost improvements

[Richard Sandiford]

Segher Boessenkool  writes:
> On Wed, Aug 09, 2017 at 09:56:31AM -0600, Jeff Law wrote:
>> On 08/08/2017 10:54 AM, Richard Sandiford wrote:
>> > I was thinking we should have separate attributes for size and speed
>> > from the outset.   How about size_cost and speed_cost?  It'd be up
>> > to the target to decide whether to define them as the sums of various
>> > sub-attributes (like it's the target's decision how to break "enabled"
>> > down).
>> But size_cost should be equivalent to the already standardized length
>> attribute.  So I'm struggling to see a need for that.
>
> "length" is (by necessity) pessimistic, cost doesn't have to be.  Not
> that it will make much difference as far as I can see.
>
>> > The advantage of doing it all in attributes is that the generator might
>> > be able to help optimise the process of checking for costs.  No promises
>> > though :-)
>>  :-)
>
> I was thinking I could have "cost" (for rs6000) have a default value
> that looks at "type".  This can then optimise things a little bit.
> But, we probably still need the "function" version as well, for the more
> complex cases.  Simpler targets can do this of course.
>
>> > TBH I think we should start with the attributes as well-defined names
>> > and only add the hook if we find we still need it.
>
> We need it, for example, to properly cost the various define_insn_and_split
> (for which "type" is only an approximation, and is woefully inadequate
> for determining cost).

But define_insn_and_splits could override the cost explicitly if they
need to.  That seems neater than testing for them in C.

>> > I can't really see
>> > what a C function would give over keeping the costs with the instruction
>> > definitions.
>
> There are many insn patterns that can share the cost computation.
>
>> I'd think the C function would mostly be helpful on a cisc target where
>> operands are potentially complex.   But I don't feel strongly enough
>> about it to argue -- I'm happy to go with consensus and fault in
>> adjustments if we need them.
>
> Making this a hook is by far the most flexible.

Maybe, but if so, that would probably also have been true for "enabled"
and "length".  We gained something by not making them hooks.

That said...

> One place where operands are nasty on rs6000 is subregs of float.  Those
> are used all over the place, and they aren't turned into (expensive!)
> insns until LRA.  In the insn patterns they look just like a SI (or DI)
> reg :-(
>
> Another example is unaligned MEMs, which can be very expensive, much
> more expensive than aligned MEMs.

...thanks, I agree these are convincing.

Richard

>
> The common theme is that we care most about cost in the not-so-happy
> cases, and those are often not easy to express in attributes.
>
>
> Segher

Re: [PATCH 0/5] RFC, WIP: RTL cost improvements

[Segher Boessenkool]

On Wed, Aug 09, 2017 at 09:56:31AM -0600, Jeff Law wrote:
> On 08/08/2017 10:54 AM, Richard Sandiford wrote:
> > I was thinking we should have separate attributes for size and speed
> > from the outset.   How about size_cost and speed_cost?  It'd be up
> > to the target to decide whether to define them as the sums of various
> > sub-attributes (like it's the target's decision how to break "enabled"
> > down).
> But size_cost should be equivalent to the already standardized length
> attribute.  So I'm struggling to see a need for that.

"length" is (by necessity) pessimistic, cost doesn't have to be.  Not
that it will make much difference as far as I can see.

> > The advantage of doing it all in attributes is that the generator might
> > be able to help optimise the process of checking for costs.  No promises
> > though :-)
>  :-)

I was thinking I could have "cost" (for rs6000) have a default value
that looks at "type".  This can then optimise things a little bit.
But, we probably still need the "function" version as well, for the more
complex cases.  Simpler targets can do this of course.

> > TBH I think we should start with the attributes as well-defined names
> > and only add the hook if we find we still need it.

We need it, for example, to properly cost the various define_insn_and_split
(for which "type" is only an approximation, and is woefully inadequate
for determining cost).

> > I can't really see
> > what a C function would give over keeping the costs with the instruction
> > definitions.

There are many insn patterns that can share the cost computation.

> I'd think the C function would mostly be helpful on a cisc target where
> operands are potentially complex.   But I don't feel strongly enough
> about it to argue -- I'm happy to go with consensus and fault in
> adjustments if we need them.

Making this a hook is by far the most flexible.

One place where operands are nasty on rs6000 is subregs of float.  Those
are used all over the place, and they aren't turned into (expensive!)
insns until LRA.  In the insn patterns they look just like a SI (or DI)
reg :-(

Another example is unaligned MEMs, which can be very expensive, much
more expensive than aligned MEMs.

The common theme is that we care most about cost in the not-so-happy
cases, and those are often not easy to express in attributes.


Segher

Re: [PATCH] PR libstdc++/81751 don't call fflush(NULL)

[Paolo Carlini]

Hi,

> On 9 Aug 2017, at 17:56, Jonathan Wakely  wrote:
> 
> This fixes a couple of problems in __gnu_cxx::stdio_filebuf,
> specifically in the __basic_file::sys_open(FILE*, openmode) function
> it uses when constructed from an existing FILE stream.
> 
> Firstly, r86756 changed __basic_file::sys_open(FILE*, openmode) to put
> the call to sync() before the assignment that sets _M_cfile. This
> means the fflush(_M_cfile) call has a null argument, and flushes all
> open streams. I don't think that was intentional, and we only want to
> flush the FILE we're constructing the streambuf with. (I think this is
> a regression from 3.4.3, which just flushed the one stream).
> 
> Secondly, we zero errno so that we can tell if fflush sets it. We need
> to restore the original value to meet the promise we make at
> https://gcc.gnu.org/onlinedocs/libstdc++/manual/errno.html
> 
> Paolo, does the this->sync() to fflush(__file) change look right?

Yes, I went through the audit trail of libstdc++/81751 and the change looks 
good. Certainly we didn't intentionally want the behavior of fflush(0)!

Thanks!
Paolo

Re: [PATCH 6/6] qsort comparator consistency checking

[Jeff Law]

On 08/03/2017 10:23 AM, Alexander Monakov wrote:
> On Thu, 3 Aug 2017, Jakub Jelinek wrote:
>> Do we really need to rename and poison anything?  qsort () in the source is
>> something that is most obvious to developers, so trying to force them to use
>> something different will just mean extra thing to learn.
> 
> Yep, I'd prefer to have a solution that keeps C-style qsort invocations as-is.
Revisiting, I'm in agreement with you.

> 
>> The _5th macro isn't that bad either, appart from using reserved namespace
>> identifiers (it really should be something like qsort_5th and the arguments
>> shouldn't start with underscores).
> 
> I didn't understand what Jeff found "ugly" about it; I wonder what epithets
> would be applied then to more, ahem, unusual parts of GCC.
I doubt anyone would want to hear what I might say about other code.
I'm sure I wouldn't want my kids reading how I might refer to certain
parts of GCC.

jeff

Re: [PATCH 6/6] qsort comparator consistency checking

[Jeff Law]

On 08/03/2017 09:52 AM, Jakub Jelinek wrote:
> On Thu, Aug 03, 2017 at 09:33:13AM -0600, Jeff Law wrote:
>> On 08/03/2017 08:24 AM, Alexander Monakov wrote:
>>> On Wed, 2 Aug 2017, Jeff Law wrote:
>> Well, there's not *that* many qsort calls.  My quick grep shows 94 and
>> its a very mechanical change.  Then a poison in system.h to ensure raw
>> calls to qsort don't return.
>>>
>>> Note that poisoning qsort outlaws vec::qsort too; it would need to be mass-
>>> renamed as well (to vec::sort, presumably).  It seems there are 83 or more
>>> calls to vec::qsort.
>> Ugh :(  That's an unfortunate implementation of poisoning :(  Consider a
>> patch to rename those too pre-approved.
> 
> Do we really need to rename and poison anything?  qsort () in the source is
> something that is most obvious to developers, so trying to force them to use
> something different will just mean extra thing to learn.
Thinking about this again, you're probably right.   I failed to
distinguish between this case and something like malloc.  For qsort, if
we're using the numbering hack, introduction of a new qsort call will
result in a qsort call that is properly checked.  In contrast we simply
don't want folks calling malloc & friends directly under any circumstances.

Sorry for taking everyone down a bogus path here.

Jeff

Re: [PATCH] Switch on *.cc tests for g++ ASan

[Jeff Law]

On 08/07/2017 11:59 PM, Slava Barinov wrote:
>   * g++.dg/asan/asan.exp: Switch on *.cc tests.
> 
> Signed-off-by: Slava Barinov 
> ---
>  gcc/testsuite/ChangeLog| 4 
>  gcc/testsuite/g++.dg/asan/asan.exp | 2 +-
>  2 files changed, 5 insertions(+), 1 deletion(-)
Thanks.  Installed.

jeff

Re: [PATCH] Forbid section anchors for ASan build (PR sanitizer/81697)

[Jeff Law]

On 08/08/2017 12:46 AM, Vyacheslav Barinov wrote:
> Hello,
> 
> Andrew Pinski  writes:
> 
>> On Mon, Aug 7, 2017 at 11:15 PM, Slava Barinov  wrote:
>>>gcc/
>>>* varasm.c (use_object_blocks_p): Forbid section anchors for ASan
>>>
>>>gcc/testsuite/
>>>* g++.dg/asan/global-alignment.cc: New test to test global
>>>variables alignment.
>>
>>
>> Can you describe this a little bit more?  What is going wrong here?
>> Is it because there is no red zone between the variables?
> I've described situation in bugzilla PR 81697 with compiled files dumps, but
> briefly yes, red zone size is incorrect between global variables.
> 
> On certain platforms (we checked at least arm and ppc) the flow is following:
> make_decl_rtl() calls create_block_symbol() on decl tree with ASan global
> variable describing a string. But then get_variable_section() returns wrong
> section (.rodata.str1.4 in my case) because check in asan_protect_global()
> returns false due to !DECL_RTL_SET_P check.
> 
> When variable is placed not into .rodata, but into .rodata.str ld treats it as
> string and just shrinks the large part of red zone silently (gold at least
> prints warning about wrong string alignment). But in run time libasan expects
> that red zone is still there and reports false positives.
> 
> In order to prevent the setting of RTL before ASan handling I tried to
> reproduce the x86_64 behavior and found that use_object_blocks_p() returns
> false for that platform. Accordingly to the function comment it reports if
> 'current compilation mode benefits from grouping', and just switching it off
> for any ASan builds resolves the issue.
> 
>> Also I noticed you are using .cc as the testcase file name, why don't
>> you use .C instead and then you won't need the other patch which you
>> just posted.
> Okay, attaching rebased version.
In many ways it'd be better if asan could be made to work with section
anchors.  Doing so means the code we're running for asan more closely
matches what we're running in the normal case.

But I'll defer to Jakub and Marek as they know the sanitizers far better
than I do and are more familiar with the expected tradeoffs.

jeff

Re: [PATCH 0/5] RFC, WIP: RTL cost improvements

[Richard Sandiford]

Jeff Law  writes:
> On 08/08/2017 10:54 AM, Richard Sandiford wrote:
 For speed cost I primarily use "type", modified by the number of machine
 insns a pattern generates (quite a few are split); and I get the number
 of machine insns just from "length" again, which for rs6000 is easy and
 correct in most cases.  Some other targets may need something else.
>>>

 I also have an attribute "cost" that can be used to override the
 default calculation; that seems useful to standardise on.  I've pondered
 a "cost_adjust" that will be added to the calculated cost instead, but
 it hasn't been useful so far.
>>> Let's go ahead and "reserve" cost and cost_adjust for this purpose.  If
>>> we find that cost_adjust isn't actually necessary, then so be it, it's
>>> not a big deal to me.
>> 
>> I was thinking we should have separate attributes for size and speed
>> from the outset.   How about size_cost and speed_cost?  It'd be up
>> to the target to decide whether to define them as the sums of various
>> sub-attributes (like it's the target's decision how to break "enabled"
>> down).
> But size_cost should be equivalent to the already standardized length
> attribute.  So I'm struggling to see a need for that.

COSTS_N_INSNS lets you add sub-instruction costs, so that you can
slightly prefer faster N-byte instructions to slower N-byte instructions.

> Again, no strong opinions on how to structure the speed cost other than
> to standardize a name.
>
>
>> 
>> The advantage of doing it all in attributes is that the generator might
>> be able to help optimise the process of checking for costs.  No promises
>> though :-)
>  :-)
>
>> 
>> TBH I think we should start with the attributes as well-defined names
>> and only add the hook if we find we still need it.  I can't really see
>> what a C function would give over keeping the costs with the instruction
>> definitions.
> I'd think the C function would mostly be helpful on a cisc target where
> operands are potentially complex.   But I don't feel strongly enough
> about it to argue -- I'm happy to go with consensus and fault in
> adjustments if we need them.

I think even the complex operands can be handled by attributes: you can
have one enum attribute that describes the complexity, perhaps with the
default value calling out to a C function if necessary, and then make
the default value of the cost attribute switch on that enum attribute.

Thanks,
Richard

Re: [PING #2] [PATCH] enhance -Wrestrict to handle string built-ins (PR 78918)

[Jeff Law]

On 08/06/2017 05:08 PM, Martin Sebor wrote:

>>
>> Well, simply because the way as implemented isn't a must-alias query
>> but maybe one that's good enough for warnings (reduces false positives
>> but surely doesn't eliminate them).
> 
> I'm very interested in reducing the rate of false positives in
> these and all other warnings.  As I mentioned in my comments,
> I did my best to weed them out of the implementation by building
> GDB, Glibc, Busybox, and the Linux kernel.  That of course isn't
> a guarantee that there aren't any.  But the first implementation
> of any non-trivial feature is never perfect, and hardly any
> warning of sufficient complexity is free of false positives, no
> matter here it's implemented (the middle-end, front-end, or
> a standalone static analysis tool).  If you spotted some cases
> I had missed I'd certainly be grateful for examples.  Otherwise,
> they will undoubtedly be reported as more software is exposed
> to the warning and, if possible, fixed, as happens with all
> other warnings.
I think Richi is saying that the must alias query you've built isn't
proper/correct.  It's less about false positives for Richi and more
about building a proper must alias query if I understand him correctly.

I suspect he's also saying that you can't reasonably build must-alias on
top of a may-alias query framework.  They're pretty different queries.

If you need something that is close to, but not quite a must alias
query, then you're going to have to make a argument for that and you
can't call it a must alias query.


> 
>> There's a must alias query already, in stmt_kills_ref_p.  It's a matter
>> of refactoring to make a refs_must_alias_p.
>>
>> Then propose that "overlap range" stuff separately.
> 
> I appreciate constructive suggestions for improvements  and I will
> look into the stmt_kills_ref_p suggestion.  But since my work
> elicited such an strong response from you I feel I should explain:
> I tried to make my changes as unintrusive as possible.  The alias
> oracle is a new area for me and I didn't want to make mistakes in
> the process of making overly extensive modifications to it.
Note that I've got a reasonably good handle on how we use
stmt_kills_ref_p.  So I can help with questions on that side.

Jeff

Re: [PATCH][Arm] Test suite failures resulting from deprecation of -mstructure-size-boundary

[Richard Earnshaw (lists)]

On 09/08/17 16:05, Michael Collison wrote:
> Patch updated to remove -mstructure-size-boundary from tests based on 
> comments from Richard. Outdated comments also removed.
> 
> Okay for trunk?

OK.

R.

> 
> 2017-08-01  Michael Collison  
> 
>   * testsuite/g++.dg/ext/packed8.C: Remove -mstructure-size boundary
>   option and fix comment.
>   * testsuite/g++.dg/init/array16.C: Remove -mstructure-size boundary
>   option and fix comment.
>   * testsuite/g++.dg/other/crash-4.C: Remove -mstructure-size boundary
>   option and fix comment.
>   * testsuite/gcc.dg/builtin-stringop-chk-1.c: Remove
>   -mstructure-size boundary option.
> 
> -Original Message-
> From: Richard Earnshaw (lists) [mailto:richard.earns...@arm.com] 
> Sent: Wednesday, August 9, 2017 1:21 AM
> To: Michael Collison ; gcc-patches@gcc.gnu.org
> Cc: nd 
> Subject: Re: [PATCH][Arm] Test suite failures resulting from deprecation of 
> -mstructure-size-boundary
> 
> On 09/08/17 06:25, Michael Collison wrote:
>> Because the comment (for example) in g+=.dg/ext/packed8.C says
>>
>>  // NOTE: This test assumes packed structure layout differs from unpacked
>> //   structure layout.  This isn't true, e.g., with the default
>> //   arm-none-elf options.
>>
>> If we could just delete the -mstructure-size-boundary=8 option why was it 
>> added in the first place?
>>
> 
> Because the comment it out of date.  It was added at a time when the default 
> structure size boundard (pre eabi) was 32.
> 
> R.
> 
>> -Original Message-
>> From: Richard Earnshaw (lists) [mailto:richard.earns...@arm.com]
>> Sent: Monday, August 7, 2017 5:32 AM
>> To: Michael Collison ; 
>> gcc-patches@gcc.gnu.org
>> Cc: nd 
>> Subject: Re: [PATCH][Arm] Test suite failures resulting from 
>> deprecation of -mstructure-size-boundary
>>
>> On 06/08/17 00:25, Michael Collison wrote:
>>> This patch fixes test case failures on arm targets due to 
>>> '-mstructure-size-boundary' being deprecated. The test cases were failing 
>>> because a warning was being issued and due to the fact that the size of 
>>> packed and unpacked structures is the same after deprecating 
>>> '-mstructure-size-boundary'
>>>
>>> Okay for trunk?
>>>
>>> 2017-08-04  Michael Collison  
>>>
>>> * testsuite/g++.dg/ext/packed8.C: Skip test for arm_eabi.
>>> * testsuite/g++.dg/init/array16.C: Skip test for arm_eabi.
>>> * testsuite/g++.dg/other/crash-4.C: Skip test for arm_eabi.
>>> * testsuite/gcc.dg/builtin-stringop-chk-1.c: Skip test for arm_eabi.
>>>
>>
>> Why would we want to skip the test?  If you delete the 
>> -mstructure-size-boundary option then the test should execute correctly on 
>> all arm-eabi based platforms and most other ARM platforms where 8 was the 
>> default anyway.
>>
>> R.
>>
>>>
>>> pr7519v1.patch
>>>
>>>
>>> diff --git a/gcc/testsuite/g++.dg/ext/packed8.C
>>> b/gcc/testsuite/g++.dg/ext/packed8.C
>>> index 91ee8b3..4f38670 100644
>>> --- a/gcc/testsuite/g++.dg/ext/packed8.C
>>> +++ b/gcc/testsuite/g++.dg/ext/packed8.C
>>> @@ -2,7 +2,7 @@
>>>  // NOTE: This test assumes packed structure layout differs from unpacked
>>>  //   structure layout.  This isn't true, e.g., with the default
>>>  //   arm-none-elf options.
>>> -// { dg-options "-mstructure-size-boundary=8" { target arm*-*-* } }
>>> +// { dg-skip-if "packed structure layout does not differ from 
>>> +unpacked layout" { { arm*-*-* } && { arm_eabi } } }
>>>  
>>>  class A
>>>  {
>>> diff --git a/gcc/testsuite/g++.dg/init/array16.C
>>> b/gcc/testsuite/g++.dg/init/array16.C
>>> index 188d1a8..3334e25 100644
>>> --- a/gcc/testsuite/g++.dg/init/array16.C
>>> +++ b/gcc/testsuite/g++.dg/init/array16.C
>>> @@ -1,7 +1,7 @@
>>>  // Causes timeout for the MMIX simulator on a 3GHz P4 and we can't 
>>> // have "compile" for some targets and "run" for others.
>>>  // { dg-do run { target { ! mmix-*-* } } } -// { dg-options 
>>> "-mstructure-size-boundary=8" { target arm*-*-* } }
>>> +// { dg-skip-if "packed structure layout does not differ from 
>>> +unpacked layout" { { arm*-*-* } && { arm_eabi } } }
>>>  
>>>  // Copyright (C) 2004 Free Software Foundation, Inc.
>>>  // Contributed by Nathan Sidwell 8 Dec 2004 
>>>  diff --git 
>>> a/gcc/testsuite/g++.dg/other/crash-4.C
>>> b/gcc/testsuite/g++.dg/other/crash-4.C
>>> index a77fe05..8530f44 100644
>>> --- a/gcc/testsuite/g++.dg/other/crash-4.C
>>> +++ b/gcc/testsuite/g++.dg/other/crash-4.C
>>> @@ -7,7 +7,7 @@
>>>  // NOTE: This test assumes packed structure layout differs from unpacked
>>>  //   structure layout.  This isn't true, e.g., with the default
>>>  //   arm-none-elf options.
>>> -// { dg-options "-mstructure-size-boundary=8" { target arm*-*-* } }
>>> +// { dg-skip-if "packed structure layout does not differ" { {
>>> +arm*-*-* } && { arm_eabi } } }
>>>  

Re: [PING #2] [PATCH] enhance -Wrestrict to handle string built-ins (PR 78918)

[Jeff Law]

On 08/03/2017 02:45 AM, Richard Biener wrote:
> 
> Well, simply because the way as implemented isn't a must-alias query
> but maybe one that's good enough for warnings (reduces false positives
> but surely doesn't eliminate them).
OK.  So it's more about building a proper must-alias query and less
about exploiting the alias oracle to give us more precise information.
That was the source of my confusion.

> 
> There's a must alias query already, in stmt_kills_ref_p.  It's a matter
> of refactoring to make a refs_must_alias_p.
Funny you should mention that -- stmt_kills_ref_p is what I had in mind
when Martin first posted this work.  DSE essentially raises a must-alias
query via that interface and I'd fully support refactoring
stmt_kills_ref_p so that it would be used for DSE as well as Martin's work.

> 
> Then propose that "overlap range" stuff separately.
That works for me.

> 
> But I'm against lumping this all in as an innocent change suggesting
> the machinery can do sth (must alias) when it really can't.  I also
> do not like adding a "must alias" bool to the may-alias APIs as
> the implementation is fundamentally may-alias, must-alias really
> is very different.
That works for me.


> 
> And to repeat, no, I do not want a "good-enough-for-warnings" must-alias
> in an API that's supposed to be used by optimizations where "good enough"
> is not good enough.
Understood and agreed.

I'll note that I suspect Martin's desire to reduce false positives and
potentially weaken things here is driven by the pushback he's had on
implementing warnings with a "too high" false positive rate in the past.

jeff

Re: [PATCH 0/5] RFC, WIP: RTL cost improvements

[Jeff Law]

On 08/08/2017 10:54 AM, Richard Sandiford wrote:
>>> For speed cost I primarily use "type", modified by the number of machine
>>> insns a pattern generates (quite a few are split); and I get the number
>>> of machine insns just from "length" again, which for rs6000 is easy and
>>> correct in most cases.  Some other targets may need something else.
>>
>>>
>>> I also have an attribute "cost" that can be used to override the
>>> default calculation; that seems useful to standardise on.  I've pondered
>>> a "cost_adjust" that will be added to the calculated cost instead, but
>>> it hasn't been useful so far.
>> Let's go ahead and "reserve" cost and cost_adjust for this purpose.  If
>> we find that cost_adjust isn't actually necessary, then so be it, it's
>> not a big deal to me.
> 
> I was thinking we should have separate attributes for size and speed
> from the outset.   How about size_cost and speed_cost?  It'd be up
> to the target to decide whether to define them as the sums of various
> sub-attributes (like it's the target's decision how to break "enabled"
> down).
But size_cost should be equivalent to the already standardized length
attribute.  So I'm struggling to see a need for that.

Again, no strong opinions on how to structure the speed cost other than
to standardize a name.


> 
> The advantage of doing it all in attributes is that the generator might
> be able to help optimise the process of checking for costs.  No promises
> though :-)
 :-)

> 
> TBH I think we should start with the attributes as well-defined names
> and only add the hook if we find we still need it.  I can't really see
> what a C function would give over keeping the costs with the instruction
> definitions.
I'd think the C function would mostly be helpful on a cisc target where
operands are potentially complex.   But I don't feel strongly enough
about it to argue -- I'm happy to go with consensus and fault in
adjustments if we need them.

jeff

[PATCH] PR libstdc++/81751 don't call fflush(NULL)

[Jonathan Wakely]

This fixes a couple of problems in __gnu_cxx::stdio_filebuf,
specifically in the __basic_file::sys_open(FILE*, openmode) function
it uses when constructed from an existing FILE stream.

Firstly, r86756 changed __basic_file::sys_open(FILE*, openmode) to put
the call to sync() before the assignment that sets _M_cfile. This
means the fflush(_M_cfile) call has a null argument, and flushes all
open streams. I don't think that was intentional, and we only want to
flush the FILE we're constructing the streambuf with. (I think this is
a regression from 3.4.3, which just flushed the one stream).

Secondly, we zero errno so that we can tell if fflush sets it. We need
to restore the original value to meet the promise we make at
https://gcc.gnu.org/onlinedocs/libstdc++/manual/errno.html

Paolo, does the this->sync() to fflush(__file) change look right?

PR libstdc++/79820
PR libstdc++/81751
* config/io/basic_file_stdio.cc (sys_open(FILE*, ios_base::openmode)):
Call fflush on the stream instead of calling sync() while _M_cfile is
null. Restore original value of errno.
* testsuite/ext/stdio_filebuf/char/79820.cc: New.
* testsuite/ext/stdio_filebuf/char/81751.cc: New.

Tested powerpc64le-linux, not yet committed.

commit 72565d197cc91a04882a398d9d242a0581d6cc09
Author: Jonathan Wakely 
Date:   Tue Aug 8 23:20:40 2017 +0100

PR libstdc++/81751 don't call fflush(NULL)

PR libstdc++/79820
PR libstdc++/81751
* config/io/basic_file_stdio.cc (sys_open(FILE*, ios_base::openmode)):
Call fflush on the stream instead of calling sync() while _M_cfile is
null. Restore original value of errno.
* testsuite/ext/stdio_filebuf/char/79820.cc: New.
* testsuite/ext/stdio_filebuf/char/81751.cc: New.

diff --git a/libstdc++-v3/config/io/basic_file_stdio.cc 
b/libstdc++-v3/config/io/basic_file_stdio.cc
index e736701..eeb1e5e 100644
--- a/libstdc++-v3/config/io/basic_file_stdio.cc
+++ b/libstdc++-v3/config/io/basic_file_stdio.cc
@@ -195,11 +195,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 __basic_file* __ret = NULL;
 if (!this->is_open() && __file)
   {
-   int __err;
+   int __err, __save_errno = errno;
+   // POSIX guarantees that fflush sets errno on error, but C doesn't.
errno = 0;
do
- __err = this->sync();
+ __err = fflush(__file);
while (__err && errno == EINTR);
+   errno = __save_errno;
if (!__err)
  {
_M_cfile = __file;
diff --git a/libstdc++-v3/testsuite/ext/stdio_filebuf/char/79820.cc 
b/libstdc++-v3/testsuite/ext/stdio_filebuf/char/79820.cc
new file mode 100644
index 000..ba566f8
--- /dev/null
+++ b/libstdc++-v3/testsuite/ext/stdio_filebuf/char/79820.cc
@@ -0,0 +1,39 @@
+// Copyright (C) 2017 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// .
+
+// { dg-require-fileio "" }
+
+#include 
+#include 
+#include 
+#include 
+
+void
+test01()
+{
+  FILE* f = std::fopen("79820.txt", "w");
+  std::fclose(f);
+  errno = 127;
+  __gnu_cxx::stdio_filebuf b(f, std::ios::out, BUFSIZ);
+  VERIFY(errno == 127); // PR libstdc++/79820
+}
+
+int
+main()
+{
+  test01();
+}
diff --git a/libstdc++-v3/testsuite/ext/stdio_filebuf/char/81751.cc 
b/libstdc++-v3/testsuite/ext/stdio_filebuf/char/81751.cc
new file mode 100644
index 000..22191dcb
--- /dev/null
+++ b/libstdc++-v3/testsuite/ext/stdio_filebuf/char/81751.cc
@@ -0,0 +1,53 @@
+// Copyright (C) 2017 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// .
+
+// { dg-require-fileio "" }
+
+#include 

Re: [PATCH] i386: Don't use frame pointer without stack access

[Andi Kleen]

> This must be much more specific.  How does it impact:
> 
> 1. LTO
> 2. Function inlining.
> 3. Partial function inlining.
> 4. Shrink-wrapping.
> 
> Any of them can impact function stack frame.

It doesn't. It's just to get back to the previous state.

Also these others already have explicit options to disable them.


-Andi

Re: [PATCH] i386: Don't use frame pointer without stack access

[H.J. Lu]

On Wed, Aug 9, 2017 at 8:05 AM, Arjan van de Ven  wrote:
> On 8/9/2017 8:04 AM, Andi Kleen wrote:
>>
>> I would add a new option
>> -fforce-frame-pointer
>> that gives the old -fno-omit-frame-pointer back, so that
>> users relying on frame pointers everywhere have a workaround.

This must be much more specific.  How does it impact:

1. LTO
2. Function inlining.
3. Partial function inlining.
4. Shrink-wrapping.

Any of them can impact function stack frame.

>
> that function should also fix the current situation where the framepointer
> is not useful
> because all actual code was moved to be outside the frame pointer code
> (e.g.  push/mov/pop followed by the actual function code)
>



-- 
H.J.

RE: [PATCH][Arm] Test suite failures resulting from deprecation of -mstructure-size-boundary

[Michael Collison]

Patch updated to remove -mstructure-size-boundary from tests based on comments 
from Richard. Outdated comments also removed.

Okay for trunk?

2017-08-01  Michael Collison  

* testsuite/g++.dg/ext/packed8.C: Remove -mstructure-size boundary
option and fix comment.
* testsuite/g++.dg/init/array16.C: Remove -mstructure-size boundary
option and fix comment.
* testsuite/g++.dg/other/crash-4.C: Remove -mstructure-size boundary
option and fix comment.
* testsuite/gcc.dg/builtin-stringop-chk-1.c: Remove
-mstructure-size boundary option.

-Original Message-
From: Richard Earnshaw (lists) [mailto:richard.earns...@arm.com] 
Sent: Wednesday, August 9, 2017 1:21 AM
To: Michael Collison ; gcc-patches@gcc.gnu.org
Cc: nd 
Subject: Re: [PATCH][Arm] Test suite failures resulting from deprecation of 
-mstructure-size-boundary

On 09/08/17 06:25, Michael Collison wrote:
> Because the comment (for example) in g+=.dg/ext/packed8.C says
> 
>  // NOTE: This test assumes packed structure layout differs from unpacked
> //   structure layout.  This isn't true, e.g., with the default
> //   arm-none-elf options.
> 
> If we could just delete the -mstructure-size-boundary=8 option why was it 
> added in the first place?
> 

Because the comment it out of date.  It was added at a time when the default 
structure size boundard (pre eabi) was 32.

R.

> -Original Message-
> From: Richard Earnshaw (lists) [mailto:richard.earns...@arm.com]
> Sent: Monday, August 7, 2017 5:32 AM
> To: Michael Collison ; 
> gcc-patches@gcc.gnu.org
> Cc: nd 
> Subject: Re: [PATCH][Arm] Test suite failures resulting from 
> deprecation of -mstructure-size-boundary
> 
> On 06/08/17 00:25, Michael Collison wrote:
>> This patch fixes test case failures on arm targets due to 
>> '-mstructure-size-boundary' being deprecated. The test cases were failing 
>> because a warning was being issued and due to the fact that the size of 
>> packed and unpacked structures is the same after deprecating 
>> '-mstructure-size-boundary'
>>
>> Okay for trunk?
>>
>> 2017-08-04  Michael Collison  
>>
>>  * testsuite/g++.dg/ext/packed8.C: Skip test for arm_eabi.
>>  * testsuite/g++.dg/init/array16.C: Skip test for arm_eabi.
>>  * testsuite/g++.dg/other/crash-4.C: Skip test for arm_eabi.
>>  * testsuite/gcc.dg/builtin-stringop-chk-1.c: Skip test for arm_eabi.
>>
> 
> Why would we want to skip the test?  If you delete the 
> -mstructure-size-boundary option then the test should execute correctly on 
> all arm-eabi based platforms and most other ARM platforms where 8 was the 
> default anyway.
> 
> R.
> 
>>
>> pr7519v1.patch
>>
>>
>> diff --git a/gcc/testsuite/g++.dg/ext/packed8.C
>> b/gcc/testsuite/g++.dg/ext/packed8.C
>> index 91ee8b3..4f38670 100644
>> --- a/gcc/testsuite/g++.dg/ext/packed8.C
>> +++ b/gcc/testsuite/g++.dg/ext/packed8.C
>> @@ -2,7 +2,7 @@
>>  // NOTE: This test assumes packed structure layout differs from unpacked
>>  //   structure layout.  This isn't true, e.g., with the default
>>  //   arm-none-elf options.
>> -// { dg-options "-mstructure-size-boundary=8" { target arm*-*-* } }
>> +// { dg-skip-if "packed structure layout does not differ from 
>> +unpacked layout" { { arm*-*-* } && { arm_eabi } } }
>>  
>>  class A
>>  {
>> diff --git a/gcc/testsuite/g++.dg/init/array16.C
>> b/gcc/testsuite/g++.dg/init/array16.C
>> index 188d1a8..3334e25 100644
>> --- a/gcc/testsuite/g++.dg/init/array16.C
>> +++ b/gcc/testsuite/g++.dg/init/array16.C
>> @@ -1,7 +1,7 @@
>>  // Causes timeout for the MMIX simulator on a 3GHz P4 and we can't 
>> // have "compile" for some targets and "run" for others.
>>  // { dg-do run { target { ! mmix-*-* } } } -// { dg-options 
>> "-mstructure-size-boundary=8" { target arm*-*-* } }
>> +// { dg-skip-if "packed structure layout does not differ from 
>> +unpacked layout" { { arm*-*-* } && { arm_eabi } } }
>>  
>>  // Copyright (C) 2004 Free Software Foundation, Inc.
>>  // Contributed by Nathan Sidwell 8 Dec 2004 
>>  diff --git 
>> a/gcc/testsuite/g++.dg/other/crash-4.C
>> b/gcc/testsuite/g++.dg/other/crash-4.C
>> index a77fe05..8530f44 100644
>> --- a/gcc/testsuite/g++.dg/other/crash-4.C
>> +++ b/gcc/testsuite/g++.dg/other/crash-4.C
>> @@ -7,7 +7,7 @@
>>  // NOTE: This test assumes packed structure layout differs from unpacked
>>  //   structure layout.  This isn't true, e.g., with the default
>>  //   arm-none-elf options.
>> -// { dg-options "-mstructure-size-boundary=8" { target arm*-*-* } }
>> +// { dg-skip-if "packed structure layout does not differ" { {
>> +arm*-*-* } && { arm_eabi } } }
>>  
>>  struct a
>>  {
>> diff --git a/gcc/testsuite/gcc.dg/builtin-stringop-chk-1.c
>> b/gcc/testsuite/gcc.dg/builtin-stringop-chk-1.c
>> index e265578..d839097 100644
>> --- 

Re: [PATCH] i386: Don't use frame pointer without stack access

[Arjan van de Ven]

On 8/9/2017 8:04 AM, Andi Kleen wrote:

I would add a new option
-fforce-frame-pointer
that gives the old -fno-omit-frame-pointer back, so that
users relying on frame pointers everywhere have a workaround.


that function should also fix the current situation where the framepointer is 
not useful
because all actual code was moved to be outside the frame pointer code
(e.g.  push/mov/pop followed by the actual function code)

Re: [PATCH] i386: Don't use frame pointer without stack access

[Andi Kleen]

"H.J. Lu"  writes:
>
> Like this?
>
> Note that @option{-fno-omit-frame-pointer} doesn't force a new stack
> frame for all functions if it isn't otherwise needed, and hence doesn't
> guarantee a new frame pointer for all functions.
>
> Here is the updated patch with a note for -fno-omit-frame-pointer.
>
> OK for trunk?

I suspect there will be still some unwinder or code patching
setups which rely on frame pointer everywhere become broken. But
doing the optimization for -fno-omit-frame-pointer by default
seems reasonable.

I would add a new option
-fforce-frame-pointer
that gives the old -fno-omit-frame-pointer back, so that
users relying on frame pointers everywhere have a workaround.

-Andi

Re: [PATCH, rs6000] enable early debug and disable switch for gimple folding

[Will Schmidt]

On Tue, 2017-08-08 at 17:31 -0500, Segher Boessenkool wrote:
> Hi!
> 
> On Tue, Aug 08, 2017 at 04:14:56PM -0500, Will Schmidt wrote:
> > * config/rs6000/rs6000.c: rs6000_option_override_internal() Add 
> > blurb
> > to indicate when early gimple folding has been disabled.
> 
>   * config/rs6000/rs6000.c (rs6000_option_override_internal): ...
> 
> 
> > rs6000_gimple_fold_builtin(): Add debug content.
> 
>   (rs6000_gimple_fold_builtin): ...
> 
> > @@ -16157,10 +16161,26 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator 
> > *gsi)
> >gcc_checking_assert (fndecl && DECL_BUILT_IN_CLASS (fndecl) == 
> > BUILT_IN_MD);
> >enum rs6000_builtins fn_code
> >  = (enum rs6000_builtins) DECL_FUNCTION_CODE (fndecl);
> >tree arg0, arg1, lhs;
> >  
> > +  size_t uns_fncode = (size_t)fn_code;
> 
> Space after cast.  More of this in the rest of the patch.
> 
> > +  if (TARGET_DEBUG_BUILTIN)
> > +  {
> > +  fprintf (stderr, "rs6000_gimple_fold_builtin %d %s %s \n",
> > +  fn_code,fn_name1,fn_name2);
> 
> No space before \n, space after commas.
> 
> > +  if (rs6000_gimple_folding_disable)
> > + return false;
> 
> One space indented too many there.
> 
> > diff --git a/gcc/config/rs6000/rs6000.opt b/gcc/config/rs6000/rs6000.opt
> > index e94aa07..4372b00 100644
> > --- a/gcc/config/rs6000/rs6000.opt
> > +++ b/gcc/config/rs6000/rs6000.opt
> > @@ -146,10 +146,14 @@ Generate AltiVec instructions using little-endian 
> > element order.
> >  
> >  maltivec=be
> >  Target Report RejectNegative Var(rs6000_altivec_element_order, 2)
> >  Generate AltiVec instructions using big-endian element order.
> >  
> > +mgimple-folding=off
> > +Target Report RejectNegative Var(rs6000_gimple_folding_disable, 1)
> > +Disable early gimple folding of builtins.
> 
> Please use -mgimple-folding instead, or better, -mfold-gimple, along
> with its no- variant?  So no RejectNegative.
> 
> It's probably easier to read if the internal var is the positive
> version, too?

Thanks for the review.  (v2) incoming momentarily. 


> 
> 
> Segher
> 

[PATCH, rs6000] (v2) enable early debug and disable switch for gimple folding

[Will Schmidt]

Hi, 

[Patch, rs6000] (v2) enable early debug and disable switch for gimple folding

Enable debug options related to gimple folding for the rs6000 target.
Adds some output to the existing -mdebug=builtin option
Add a -mgimple-folding=off option to disable the early rs6000 gimple folding.

(V2 updates)
Changed variable name to rs6000_fold_gimple. 
Changed option name to "fold-gimple", so -mfold-gimple and -mno-fold-gimple 
options
both work smoothly.
Whitespace updates as noted per review. I also fixed the (missing) space after
cast for the existing debug code.

OK for trunk?

Thanks,
-Will


[gcc]

2017-08-09  Will Schmidt  

* config/rs6000/rs6000.c: (rs6000_option_override_internal) Add blurb
to indicate when early gimple folding has been disabled.
(rs6000_gimple_fold_builtin): Add debug content.
(rs6000_invalid_builtin): whitespace.
(rs6000_expand_builtin): Whitespace.
* config/rs6000/rs6000.opt: Add option for -mfold-gimple.

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 1fb9861..f970f9e 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -4180,10 +4180,14 @@ rs6000_option_override_internal (bool global_init_p)
 {
   warning (0, N_("-maltivec=le not allowed for big-endian targets"));
   rs6000_altivec_element_order = 0;
 }
 
+  if (!rs6000_fold_gimple)
+ fprintf (stderr,
+ "gimple folding of rs6000 builtins has been disabled.\n");
+
   /* Add some warnings for VSX.  */
   if (TARGET_VSX)
 {
   const char *msg = NULL;
   if (!TARGET_HARD_FLOAT || !TARGET_SINGLE_FLOAT || !TARGET_DOUBLE_FLOAT)
@@ -16052,11 +16056,11 @@ paired_expand_predicate_builtin (enum insn_code 
icode, tree exp, rtx target)
appropriate target options being set.  */
 
 static void
 rs6000_invalid_builtin (enum rs6000_builtins fncode)
 {
-  size_t uns_fncode = (size_t)fncode;
+  size_t uns_fncode = (size_t) fncode;
   const char *name = rs6000_builtin_info[uns_fncode].name;
   HOST_WIDE_INT fnmask = rs6000_builtin_info[uns_fncode].mask;
 
   gcc_assert (name != NULL);
   if ((fnmask & RS6000_BTM_CELL) != 0)
@@ -16157,10 +16161,26 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi)
   gcc_checking_assert (fndecl && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_MD);
   enum rs6000_builtins fn_code
 = (enum rs6000_builtins) DECL_FUNCTION_CODE (fndecl);
   tree arg0, arg1, lhs;
 
+  size_t uns_fncode = (size_t) fn_code;
+  enum insn_code icode = rs6000_builtin_info[uns_fncode].icode;
+  const char *fn_name1 = rs6000_builtin_info[uns_fncode].name;
+  const char *fn_name2 = ((icode != CODE_FOR_nothing)
+ ? get_insn_name ((int) icode)
+ : "nothing");
+
+  if (TARGET_DEBUG_BUILTIN)
+  {
+  fprintf (stderr, "rs6000_gimple_fold_builtin %d %s %s\n",
+  fn_code, fn_name1, fn_name2);
+  }
+
+  if (!rs6000_fold_gimple)
+return false;
+
   /* Generic solution to prevent gimple folding of code without a LHS.  */
   if (!gimple_call_lhs (stmt))
 return false;
 
   switch (fn_code)
@@ -16516,10 +16536,13 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi)
gsi_insert_seq_before (gsi, stmts, GSI_SAME_STMT);
update_call_from_tree (gsi, res);
return true;
   }
 default:
+   if (TARGET_DEBUG_BUILTIN)
+ fprintf (stderr, "gimple builtin intrinsic not matched:%d %s %s \n",
+   fn_code, fn_name1, fn_name2);
   break;
 }
 
   return false;
 }
@@ -16549,11 +16572,11 @@ rs6000_expand_builtin (tree exp, rtx target, rtx 
subtarget ATTRIBUTE_UNUSED,
   if (TARGET_DEBUG_BUILTIN)
 {
   enum insn_code icode = rs6000_builtin_info[uns_fcode].icode;
   const char *name1 = rs6000_builtin_info[uns_fcode].name;
   const char *name2 = ((icode != CODE_FOR_nothing)
-  ? get_insn_name ((int)icode)
+  ? get_insn_name ((int) icode)
   : "nothing");
   const char *name3;
 
   switch (rs6000_builtin_info[uns_fcode].attr & RS6000_BTC_TYPE_MASK)
{
@@ -16569,11 +16592,11 @@ rs6000_expand_builtin (tree exp, rtx target, rtx 
subtarget ATTRIBUTE_UNUSED,
 
 
   fprintf (stderr,
   "rs6000_expand_builtin, %s (%d), insn = %s (%d), type=%s%s\n",
   (name1) ? name1 : "---", fcode,
-  (name2) ? name2 : "---", (int)icode,
+  (name2) ? name2 : "---", (int) icode,
   name3,
   func_valid_p ? "" : ", not valid");
 }   
 
   if (!func_valid_p)
diff --git a/gcc/config/rs6000/rs6000.opt b/gcc/config/rs6000/rs6000.opt
index e94aa07..65e4724 100644
--- a/gcc/config/rs6000/rs6000.opt
+++ b/gcc/config/rs6000/rs6000.opt
@@ -146,10 +146,14 @@ Generate AltiVec instructions using little-endian element 
order.
 
 maltivec=be
 Target Report RejectNegative 

Re: [PATCH 1/3] improve detection of attribute conflicts (PR 81544)

[Martin Sebor]

On 08/09/2017 05:53 AM, Marek Polacek wrote:

(Not a proper review really.)

On Tue, Aug 08, 2017 at 10:13:07AM -0600, Martin Sebor wrote:

@@ -490,7 +583,9 @@ decl_attributes (tree *node, tree attributes, int flags)
   | (int) ATTR_FLAG_ARRAY_NEXT))
{
  /* Pass on this attribute to be tried again.  */
- returned_attrs = tree_cons (name, args, returned_attrs);
+ tree attr = tree_cons (name, args, NULL_TREE);
+ returned_attrs = chainon (returned_attrs, attr);
+ // returned_attrs = tree_cons (name, args, returned_attrs);
  continue;
}
  else
@@ -535,7 +630,9 @@ decl_attributes (tree *node, tree attributes, int flags)
  else if (flags & (int) ATTR_FLAG_FUNCTION_NEXT)
{
  /* Pass on this attribute to be tried again.  */
- returned_attrs = tree_cons (name, args, returned_attrs);
+ tree attr = tree_cons (name, args, NULL_TREE);
+ returned_attrs = chainon (returned_attrs, attr);
+ // returned_attrs = tree_cons (name, args, returned_attrs);


What's with these // lines?  I suppose they weren't supposed to be here.


Yes, the commented lines should be removed.  Thanks for pointing
them out.  I'll do that in the next revision...




+  /* If the attribute was sussceefully handled on its own and is


"successfully"


...along with fixing typos like this one...




diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c
index e970ab2..8bbfcb5 100644
--- a/gcc/c-family/c-warn.c
+++ b/gcc/c-family/c-warn.c
@@ -2143,36 +2143,19 @@ diagnose_mismatched_attributes (tree olddecl, tree 
newdecl)
   newdecl);

   /* Diagnose inline __attribute__ ((noinline)) which is silly.  */
+  const char* noinline = "noinline";


"const char *noinline"


...and adjusting these kinds of things.

...

--- a/libstdc++-v3/include/ext/mt_allocator.h
+++ b/libstdc++-v3/include/ext/mt_allocator.h
@@ -355,7 +355,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   }

   // XXX GLIBCXX_ABI Deprecated
-  _GLIBCXX_CONST void
+  void
   _M_destroy_thread_key(void*) throw ();

   size_t


Why this change?  Seems unrelated.


This appears to be a pointless use of attribute const:

  https://gcc.gnu.org/ml/gcc/2017-08/msg00027.html

It was highlighted by tightening up the attribute const handler
also included in the patch.  I forgot to call it out separately
or CC Jonathan/libstdc++ on it (perhaps it should have been
split out into a small patch of its own).




diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c 
b/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c
index 146b76c..58a4742 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c
@@ -113,7 +113,7 @@ int test9 (int *intarr)

 int test99 (int *intarr)
 {
-  extern int foo9 (int) __attribute__ ((pure));
+  extern int foo9 (int) __attribute__ ((const));
   int h, v;
   g9 = 9;
   h = foo9 (g9);



And why this?  I'd avoid modifying existing tests like that unless
it's directly related to the new diagnostic.


The same function is declared earlier on in the file with attribute
const and the redeclaration with attribute pure triggers a warning
due to the two being considered mutually exclusive.

Thanks for the review!

Martin

[PATCH] Enable libitm DLL build on Cygwin/Mingw

[JonY]

Fixes libtool calls in libitm. Patch OK for trunk?

2017-08-09  Jonathan Yong  <10wa...@gmail.com>

* configure.ac: Check libtool flags.
* Makefile.am: Use lt_host_flags.
* configure: Regenerated.
* Makefile.in: Regenerated.
* testsuite/Makefile.in: Regenerated.


Index: configure.ac
===
--- configure.ac(revision 250986)
+++ configure.ac(working copy)
@@ -147,6 +147,7 @@
 
 # Configure libtool
 AM_PROG_LIBTOOL
+ACX_LT_HOST_FLAGS
 AC_SUBST(enable_shared)
 AC_SUBST(enable_static)
 
Index: Makefile.am
===
--- Makefile.am (revision 250986)
+++ Makefile.am (working copy)
@@ -54,7 +54,7 @@
 # want or need libstdc++.
 libitm_la_DEPENDENCIES = $(libitm_version_dep)
 libitm_la_LINK = $(LINK) $(libitm_la_LDFLAGS)
-libitm_la_LDFLAGS = $(libitm_version_info) $(libitm_version_script)
+libitm_la_LDFLAGS = $(libitm_version_info) $(libitm_version_script) 
$(lt_host_flags)
 
 libitm_la_SOURCES = \
aatree.cc alloc.cc alloc_c.cc alloc_cpp.cc barrier.cc beginend.cc \



signature.asc
Description: OpenPGP digital signature

Re: [PATCH][testsuite] Add check_effective_target_autoincdec

[Richard Earnshaw (lists)]

On 09/08/17 12:37, Wilco Dijkstra wrote:
> Richard Earnshaw wrote:
>> Except that I think this would be better done as an 'effective target'
>> test; something like
>>
>> dg-require-effective-target autoincdec
> 
> Right I figured out how to do this - not trivial as it needs a secret flag in 
> the
> glob call - if anyone knows a better way of doing this I'd like to know!
> 
> 
> Add check_effective_target_autoincdec that returns true if a target
> runs the auto_inc_dec optimization pass.
> 
> OK for commit?
> 

LGTM.  Please give Mike 24 hours to respond, before committing.

R.

> ChangeLog:
> 2017-08-08  Wilco Dijkstra  
> 
> gcc/testsuite/
> 
>   * gcc.dg/pr46932.c: Use effective_target autoincdec.
>   * lib/target-supports.exp: Add check_effective_target_autoincdec.
> --
> 
> diff --git a/gcc/testsuite/gcc.dg/pr46932.c b/gcc/testsuite/gcc.dg/pr46932.c
> index 
> 4eb1a99e1bd9403f8b1c5d0d71ef731ad4a65128..2b39990d036035d22e226b98351a4900a5dbb309
>  100644
> --- a/gcc/testsuite/gcc.dg/pr46932.c
> +++ b/gcc/testsuite/gcc.dg/pr46932.c
> @@ -1,7 +1,5 @@
>  /* { dg-options "-O2 -fdump-rtl-auto_inc_dec" } */
> -
> -/* Build on targets which have pre increment.  */
> -/* { dg-do compile { target aarch64*-*-* arm*-*-* rs6000-*-* powerpc*-*-* 
> arc*-*-* m32r-*-* tic6x-*-* } } */
> +/* { dg-require-effective-target autoincdec } */
>  
>  /* Check that accesses based on the frame pointer do not
> use auto increment.  */
> diff --git a/gcc/testsuite/lib/target-supports.exp 
> b/gcc/testsuite/lib/target-supports.exp
> index 
> 5a6562794b2bdd5f370fc5b26d688f02779a..5219fbf4671e83a6fa7affdab926115e8a23f9cb
>  100644
> --- a/gcc/testsuite/lib/target-supports.exp
> +++ b/gcc/testsuite/lib/target-supports.exp
> @@ -8482,3 +8482,18 @@ proc check_effective_target_arm_coproc4_ok { } {
>  return [check_cached_effective_target arm_coproc4_ok \
>   check_effective_target_arm_coproc4_ok_nocache]
>  }
> +
> +# Return 1 if the target supports the auto_inc_dec optimization pass.
> +proc check_effective_target_autoincdec { } {
> +if { ![check_no_compiler_messages auto_incdec assembly { void f () { }
> +  } "-O2 -fdump-rtl-auto_inc_dec" ] } {
> +  return 0
> +}
> +
> +set dumpfile [glob -nocomplain 
> "auto_incdec[pid].c.\[0-9\]\[0-9\]\[0-9\]r.auto_inc_dec"]
> +if { [file exists $dumpfile ] } {
> + file delete $dumpfile
> + return 1
> +}
> +return 0
> +}
> 

C PATCH to boolify a few parameters

[Marek Polacek]

Bool bool bool.

Bootstrapped/regtested on x86_64-linux, applying to trunk.

2017-08-09  Marek Polacek  

* c-common.c (pointer_int_sum): Use true/false instead of 1/0.
(c_common_truthvalue_conversion): Likewise.
* c-omp.c (c_finish_omp_atomic): Likewise.
* c-common.h (build_binary_op): Update declaration.

* c-decl.c (build_enumerator): Use true/false instead of 1/0.
* c-tree.h (build_external_ref): Update declaration.
* c-typeck.c (build_array_ref): Use true/false instead of 1/0.
(build_external_ref): Change the type of a parameter to bool.
(parser_build_binary_op): Use true/false instead of 1/0.
(pointer_diff): Likewise.
(build_modify_expr): Likewise.
(set_designator): Change the type of a parameter to bool.
(set_init_index): Use true/false instead of 1/0.
(set_init_label): Likewise.
(output_init_element): Change the type of a parameter to bool.
(output_pending_init_elements): Use true/false instead of 1/0.
(process_init_element): Likewise.
(build_binary_op): Change the type of a parameter to bool.

* parser.c (cp_parser_perform_range_for_lookup): Use false instead of
0.
* typeck.c (build_binary_op): Change the type of a parameter to bool.
(cp_truthvalue_conversion):  Use true instead of 1.

diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
index feb0904bcbf..a9c0614b436 100644
--- gcc/c-family/c-common.c
+++ gcc/c-family/c-common.c
@@ -3133,7 +3133,8 @@ pointer_int_sum (location_t loc, enum tree_code 
resultcode,
 can result in a sum or difference with different type args.  */
   ptrop = build_binary_op (EXPR_LOCATION (TREE_OPERAND (intop, 1)),
   subcode, ptrop,
-  convert (int_type, TREE_OPERAND (intop, 1)), 1);
+  convert (int_type, TREE_OPERAND (intop, 1)),
+  true);
   intop = convert (int_type, TREE_OPERAND (intop, 0));
 }
 
@@ -3306,7 +3307,7 @@ c_common_truthvalue_conversion (location_t location, tree 
expr)
TREE_OPERAND (expr, 0)),
c_common_truthvalue_conversion (location,
TREE_OPERAND (expr, 1)),
- 0);
+ false);
   goto ret;
 
 case NEGATE_EXPR:
@@ -3457,7 +3458,7 @@ c_common_truthvalue_conversion (location_t location, tree 
expr)
c_common_truthvalue_conversion
   (location,
build_unary_op (location, IMAGPART_EXPR, t, false)),
-  0));
+  false));
   goto ret;
 }
 
@@ -3466,10 +3467,10 @@ c_common_truthvalue_conversion (location_t location, 
tree expr)
   tree fixed_zero_node = build_fixed (TREE_TYPE (expr),
  FCONST0 (TYPE_MODE
   (TREE_TYPE (expr;
-  return build_binary_op (location, NE_EXPR, expr, fixed_zero_node, 1);
+  return build_binary_op (location, NE_EXPR, expr, fixed_zero_node, true);
 }
   else
-return build_binary_op (location, NE_EXPR, expr, integer_zero_node, 1);
+return build_binary_op (location, NE_EXPR, expr, integer_zero_node, true);
 
  ret:
   protected_set_expr_location (expr, location);
diff --git gcc/c-family/c-common.h gcc/c-family/c-common.h
index a29f1ade25d..b44e1bd78d6 100644
--- gcc/c-family/c-common.h
+++ gcc/c-family/c-common.h
@@ -960,7 +960,7 @@ extern tree build_real_imag_expr (location_t, enum 
tree_code, tree);
a variant of the C language.  They are used in c-common.c.  */
 
 extern tree build_unary_op (location_t, enum tree_code, tree, bool);
-extern tree build_binary_op (location_t, enum tree_code, tree, tree, int);
+extern tree build_binary_op (location_t, enum tree_code, tree, tree, bool);
 extern tree perform_integral_promotions (tree);
 
 /* These functions must be defined by each front-end which implements
diff --git gcc/c-family/c-omp.c gcc/c-family/c-omp.c
index 900f5a33943..eef7ac0c769 100644
--- gcc/c-family/c-omp.c
+++ gcc/c-family/c-omp.c
@@ -276,14 +276,14 @@ c_finish_omp_atomic (location_t loc, enum tree_code code,
   lhs = build3_loc (loc, BIT_FIELD_REF, TREE_TYPE (blhs), lhs,
bitsize_int (bitsize), bitsize_int (bitpos));
   if (swapped)
-   rhs = build_binary_op (loc, opcode, rhs, lhs, 1);
+   rhs = build_binary_op (loc, opcode, rhs, lhs, true);
   else if (opcode != NOP_EXPR)
-   rhs = build_binary_op (loc, opcode, lhs, rhs, 1);
+   rhs = build_binary_op (loc, opcode, lhs, rhs, true);
   opcode = NOP_EXPR;
 }
   else if (swapped)
 {
-  rhs = build_binary_op (loc, opcode, rhs, lhs, 1);
+  rhs = build_binary_op (loc, opcode, rhs, lhs, true);
   opcode = NOP_EXPR;
 }
   bool 

Re: [PATCH] i386: Don't use frame pointer without stack access

[H.J. Lu]

On Wed, Aug 9, 2017 at 4:59 AM, Michael Matz  wrote:
> Hi,
>
> On Wed, 9 Aug 2017, H.J. Lu wrote:
>
>> > OK, but then both -f[no-]omit-frame-pointer do not have clearly defined 
>> > semantics and thus shouldn't be exposed to the user?
>>
>> -f[no-]omit-frame-pointer apply to cases where a new stack frame
>> is needed.  -fno-omit-frame-pointer allows you to unwind each
>> stack frame, not necessarily each function, via frame pointer.
>>  -fno-omit-frame-pointer may not create a new stack frame for
>> each function, similar to LTO or function inlining.  But you can still
>> unwind via frame pointer.  We never guarantee that we will create
>> a new stack frame for each function.  Some functions are inlined
>> completely.  Some just use the caller's stack frame.
>
> Maybe something along these lines should be added to the docu of
> fno-omit-frame-pointer then.  At least "Note that -fno-omit-frame-pointer
> doesn't force a frame for all functions if it isn't otherwise needed, and
> hence doesn't guarantee a frame pointer for all functions." .
>

Like this?

Note that @option{-fno-omit-frame-pointer} doesn't force a new stack
frame for all functions if it isn't otherwise needed, and hence doesn't
guarantee a new frame pointer for all functions.

Here is the updated patch with a note for -fno-omit-frame-pointer.

OK for trunk?

-- 
H.J.
From 4f6197699ab0904671919187c71d4a54594c1431 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" 
Date: Sun, 6 Aug 2017 06:24:36 -0700
Subject: [PATCH] i386: Don't use frame pointer without stack access

When there is no stack access, there is no need to use frame pointer
even if -fno-omit-frame-pointer is used and caller's frame pointer is
unchanged.

gcc/

	PR target/81736
	* config/i386/i386.c (ix86_finalize_stack_realign_flags): Renamed
	to ...
	(ix86_finalize_stack_frame_flags): This.  Also clear
	frame_pointer_needed if -fno-omit-frame-pointer is used without
	stack access.
	(ix86_expand_prologue): Replace ix86_finalize_stack_realign_flags
	with ix86_finalize_stack_frame_flags.
	(ix86_expand_epilogue): Likewise.
	(ix86_expand_split_stack_prologue): Likewise.
	* doc/invoke.texi: Add a note for -fno-omit-frame-pointer.

gcc/testsuite/

	PR target/81736
	* gcc.target/i386/pr81736-1.c: New test.
	* gcc.target/i386/pr81736-2.c: Likewise.
	* gcc.target/i386/pr81736-3.c: Likewise.
	* gcc.target/i386/pr81736-4.c: Likewise.
	* gcc.target/i386/pr81736-5.c: Likewise.
	* gcc.target/i386/pr81736-6.c: Likewise.
	* gcc.target/i386/pr81736-7.c: Likewise.
---
 gcc/config/i386/i386.c| 23 ---
 gcc/doc/invoke.texi   |  4 
 gcc/testsuite/gcc.target/i386/pr81736-1.c | 13 +
 gcc/testsuite/gcc.target/i386/pr81736-2.c | 14 ++
 gcc/testsuite/gcc.target/i386/pr81736-3.c | 11 +++
 gcc/testsuite/gcc.target/i386/pr81736-4.c | 11 +++
 gcc/testsuite/gcc.target/i386/pr81736-5.c | 20 
 gcc/testsuite/gcc.target/i386/pr81736-6.c | 16 
 gcc/testsuite/gcc.target/i386/pr81736-7.c | 13 +
 9 files changed, 114 insertions(+), 11 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr81736-1.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr81736-2.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr81736-3.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr81736-4.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr81736-5.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr81736-6.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr81736-7.c

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 3f8519777f7..85aa4d4fbf5 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -14179,10 +14179,11 @@ output_probe_stack_range (rtx reg, rtx end)
   return "";
 }
 
-/* Finalize stack_realign_needed flag, which will guide prologue/epilogue
-   to be generated in correct form.  */
+/* Finalize stack_realign_needed and frame_pointer_needed flags, which
+   will guide prologue/epilogue to be generated in correct form.  */
+
 static void
-ix86_finalize_stack_realign_flags (void)
+ix86_finalize_stack_frame_flags (void)
 {
   /* Check if stack realign is really needed after reload, and
  stores result in cfun */
@@ -14205,13 +14206,13 @@ ix86_finalize_stack_realign_flags (void)
 }
 
   /* If the only reason for frame_pointer_needed is that we conservatively
- assumed stack realignment might be needed, but in the end nothing that
- needed the stack alignment had been spilled, clear frame_pointer_needed
- and say we don't need stack realignment.  */
-  if (stack_realign
+ assumed stack realignment might be needed or -fno-omit-frame-pointer
+ is used, but in the end nothing that needed the stack alignment had
+ been spilled nor stack access, clear frame_pointer_needed and say we
+ don't need stack realignment.  */
+  if ((stack_realign || !flag_omit_frame_pointer)
   

Re: [PATCH] i386: Don't use frame pointer without stack access

[Michael Matz]

Hi,

On Wed, 9 Aug 2017, H.J. Lu wrote:

> > OK, but then both -f[no-]omit-frame-pointer do not have clearly defined 
> > semantics and thus shouldn't be exposed to the user?
> 
> -f[no-]omit-frame-pointer apply to cases where a new stack frame
> is needed.  -fno-omit-frame-pointer allows you to unwind each
> stack frame, not necessarily each function, via frame pointer.
>  -fno-omit-frame-pointer may not create a new stack frame for
> each function, similar to LTO or function inlining.  But you can still
> unwind via frame pointer.  We never guarantee that we will create
> a new stack frame for each function.  Some functions are inlined
> completely.  Some just use the caller's stack frame.

Maybe something along these lines should be added to the docu of 
fno-omit-frame-pointer then.  At least "Note that -fno-omit-frame-pointer 
doesn't force a frame for all functions if it isn't otherwise needed, and 
hence doesn't guarantee a frame pointer for all functions." .


Ciao,
Michael.

Re: [PATCH 1/3] improve detection of attribute conflicts (PR 81544)

[Marek Polacek]

(Not a proper review really.)

On Tue, Aug 08, 2017 at 10:13:07AM -0600, Martin Sebor wrote:
> @@ -490,7 +583,9 @@ decl_attributes (tree *node, tree attributes, int flags)
>  | (int) ATTR_FLAG_ARRAY_NEXT))
>   {
> /* Pass on this attribute to be tried again.  */
> -   returned_attrs = tree_cons (name, args, returned_attrs);
> +   tree attr = tree_cons (name, args, NULL_TREE);
> +   returned_attrs = chainon (returned_attrs, attr);
> +   // returned_attrs = tree_cons (name, args, returned_attrs);
> continue;
>   }
> else
> @@ -535,7 +630,9 @@ decl_attributes (tree *node, tree attributes, int flags)
> else if (flags & (int) ATTR_FLAG_FUNCTION_NEXT)
>   {
> /* Pass on this attribute to be tried again.  */
> -   returned_attrs = tree_cons (name, args, returned_attrs);
> +   tree attr = tree_cons (name, args, NULL_TREE);
> +   returned_attrs = chainon (returned_attrs, attr);
> +   // returned_attrs = tree_cons (name, args, returned_attrs);

What's with these // lines?  I suppose they weren't supposed to be here.

> +  /* If the attribute was sussceefully handled on its own and is

"successfully"

> diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c
> index e970ab2..8bbfcb5 100644
> --- a/gcc/c-family/c-warn.c
> +++ b/gcc/c-family/c-warn.c
> @@ -2143,36 +2143,19 @@ diagnose_mismatched_attributes (tree olddecl, tree 
> newdecl)
>  newdecl);
>  
>/* Diagnose inline __attribute__ ((noinline)) which is silly.  */
> +  const char* noinline = "noinline";

"const char *noinline"

> --- a/gcc/cp/name-lookup.c
> +++ b/gcc/cp/name-lookup.c
> @@ -2291,6 +2291,71 @@ set_local_extern_decl_linkage (tree decl, bool 
> shadowed)
>  }
>  }
>  
> +/* Given a new DECL that hasn't been pushed yet, try to find the last
> +   DECL for the same symbol and return it, oterwise return null.  */
> +
> +tree
> +find_decl (tree decl)
> +{
> +  if (!DECL_P (decl))
> +return NULL_TREE;
> +
> +  /* if (!DECL_TEMPLATE_PARM_P (decl) && current_function_decl) */
> +  /*   set_decl_context_in_fn (current_function_decl, decl); */

Looks like some stray lines.

> +
> +  /* The binding level we will be pushing into.  During local class
> + pushing, we want to push to the containing scope.  */
> +  cp_binding_level *level = current_binding_level;
> +  while (level->kind == sk_class)
> +level = level->level_chain;
> +
> +  tree name = DECL_NAME (decl);
> +  if (!name)
> +return NULL_TREE;
> +
> +  cxx_binding *binding = NULL; /* Local scope binding.  */
> +  tree ns = NULL_TREE; /* Searched namespace.  */
> +  tree *slot = NULL; /* Binding slot in namespace.  */
> +  tree old = NULL_TREE;
> +
> +  if (level->kind == sk_namespace)
> +{
> +  /* We look in the decl's namespace for an existing
> +  declaration, even though we push into the current
> +  namespace.  */
> +  ns = (DECL_NAMESPACE_SCOPE_P (decl)
> + ? CP_DECL_CONTEXT (decl) : current_namespace);
> +  /* Create the binding, if this is current namespace, because
> +  that's where we'll be pushing anyway.  */
> +  slot = find_namespace_slot (ns, name, ns == current_namespace);
> +  if (slot)
> + old = MAYBE_STAT_DECL (*slot);
> +}
> +  else
> +{
> +  binding = find_local_binding (level, name);
> +  if (binding)
> + old = binding->value;
> +}
> +
> +  /* if (current_function_decl && VAR_OR_FUNCTION_DECL_P (decl) */
> +  /* && DECL_EXTERNAL (decl)) */
> +  /*   set_local_extern_decl_linkage (decl, old != NULL_TREE); */

Here too.

> +  if (old == error_mark_node)
> +old = NULL_TREE;
> +
> +  for (ovl_iterator iter (old); iter; ++iter)
> +{
> +  if (iter.using_p ())
> + ; /* Ignore using decls here.  */
> +  else if (decls_match (decl, *iter, /*record_decls=*/false))
> + return *iter;
> +}
> +
> +  return NULL_TREE;
> +}
> +
>  /* Record DECL as belonging to the current lexical scope.  Check for
> errors (such as an incompatible declaration for the same name
> already seen in the same scope).  IS_FRIEND is true if DECL is
> diff --git a/gcc/cp/name-lookup.h b/gcc/cp/name-lookup.h
> index 2e2075c..ce0826e 100644
> --- a/gcc/cp/name-lookup.h
> +++ b/gcc/cp/name-lookup.h
> @@ -339,4 +339,6 @@ extern void pop_nested_namespace (tree);
>  extern void push_to_top_level (void);
>  extern void pop_from_top_level (void);
>  
> +extern tree find_decl (tree);
> +
>  #endif /* GCC_CP_NAME_LOOKUP_H */
> diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
> index 8f18665..6cbf4b2 100644
> --- a/gcc/cp/tree.c
> +++ b/gcc/cp/tree.c
> @@ -4314,10 +4314,10 @@ const struct attribute_spec cxx_attribute_table[] =
>/* { name, min_len, max_len, decl_req, type_req, fn_type_req, handler,
> affects_type_identity } */
>{ "init_priority",  1, 1, true,  false, false,
> -

Re: [PATCH] Convert character arrays to string csts

[Martin Liška]

On 08/09/2017 11:43 AM, Richard Biener wrote:
> I only have the patch I sent you so I can't re-diff.
> 
> Richard.

Hi.

I'm sending rebased version of the patch. However the patch
eats all my memory when e.g. building ../../../libgcc/libgcov-merge.c.
If you have time, please try to make it working for _CST. I can continue
then.

Thanks,
Martin
diff --git a/gcc/gimple-expr.c b/gcc/gimple-expr.c
index c1771fcf1d0..eb22456c241 100644
--- a/gcc/gimple-expr.c
+++ b/gcc/gimple-expr.c
@@ -631,7 +631,7 @@ is_gimple_address (const_tree t)
   op = TREE_OPERAND (op, 0);
 }
 
-  if (CONSTANT_CLASS_P (op) || TREE_CODE (op) == MEM_REF)
+  if (TREE_CODE (op) == MEM_REF)
 return true;
 
   switch (TREE_CODE (op))
@@ -667,11 +667,10 @@ is_gimple_invariant_address (const_tree t)
 {
   const_tree op0 = TREE_OPERAND (op, 0);
   return (TREE_CODE (op0) == ADDR_EXPR
-	  && (CONSTANT_CLASS_P (TREE_OPERAND (op0, 0))
-		  || decl_address_invariant_p (TREE_OPERAND (op0, 0;
+	  && decl_address_invariant_p (TREE_OPERAND (op0, 0)));
 }
 
-  return CONSTANT_CLASS_P (op) || decl_address_invariant_p (op);
+  return decl_address_invariant_p (op);
 }
 
 /* Return true if T is a gimple invariant address at IPA level
@@ -693,11 +692,10 @@ is_gimple_ip_invariant_address (const_tree t)
 {
   const_tree op0 = TREE_OPERAND (op, 0);
   return (TREE_CODE (op0) == ADDR_EXPR
-	  && (CONSTANT_CLASS_P (TREE_OPERAND (op0, 0))
-		  || decl_address_ip_invariant_p (TREE_OPERAND (op0, 0;
+	  && decl_address_ip_invariant_p (TREE_OPERAND (op0, 0)));
 }
 
-  return CONSTANT_CLASS_P (op) || decl_address_ip_invariant_p (op);
+  return decl_address_ip_invariant_p (op);
 }
 
 /* Return true if T is a GIMPLE minimal invariant.  It's a restricted
@@ -732,7 +730,7 @@ is_gimple_reg (tree t)
   if (virtual_operand_p (t))
 return false;
 
-  if (TREE_CODE (t) == SSA_NAME)
+  if (TREE_CODE (t) == SSA_NAME || TREE_CODE (t) == CONST_DECL)
 return true;
 
   if (!is_gimple_variable (t))
@@ -825,8 +823,7 @@ is_gimple_mem_ref_addr (tree t)
   return (is_gimple_reg (t)
 	  || TREE_CODE (t) == INTEGER_CST
 	  || (TREE_CODE (t) == ADDR_EXPR
-	  && (CONSTANT_CLASS_P (TREE_OPERAND (t, 0))
-		  || decl_address_invariant_p (TREE_OPERAND (t, 0);
+	  && decl_address_invariant_p (TREE_OPERAND (t, 0;
 }
 
 /* Hold trees marked addressable during expand.  */
diff --git a/gcc/gimple-expr.h b/gcc/gimple-expr.h
index 6e969164a37..04de209c092 100644
--- a/gcc/gimple-expr.h
+++ b/gcc/gimple-expr.h
@@ -94,9 +94,7 @@ is_gimple_id (tree t)
   return (is_gimple_variable (t)
 	  || TREE_CODE (t) == FUNCTION_DECL
 	  || TREE_CODE (t) == LABEL_DECL
-	  || TREE_CODE (t) == CONST_DECL
-	  /* Allow string constants, since they are addressable.  */
-	  || TREE_CODE (t) == STRING_CST);
+	  || TREE_CODE (t) == CONST_DECL);
 }
 
 /* Return true if OP, an SSA name or a DECL is a virtual operand.  */
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 86623e09f5d..d56d1dfe8ab 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -2871,7 +2871,8 @@ gimplify_compound_lval (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
  so as to match the min_lval predicate.  Failure to do so may result
  in the creation of large aggregate temporaries.  */
   tret = gimplify_expr (p, pre_p, post_p, is_gimple_min_lval,
-			fallback | fb_lvalue);
+			expr_stack.length () > 0
+			? fb_lvalue : fallback | fb_lvalue);
   ret = MIN (ret, tret);
 
   /* And finally, the indices and operands of ARRAY_REF.  During this
@@ -11503,7 +11504,17 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
 	 that in the GIMPLE IL.  */
 	  if (TREE_OVERFLOW_P (*expr_p))
 	*expr_p = drop_tree_overflow (*expr_p);
-	  ret = GS_ALL_DONE;
+
+	  if (fallback & fb_rvalue)
+	ret = GS_ALL_DONE;
+	  else
+	{
+	  tree cdecl = build_decl (UNKNOWN_LOCATION, CONST_DECL,
+   NULL_TREE, TREE_TYPE (*expr_p));
+	  DECL_INITIAL (cdecl) = *expr_p;
+	  *expr_p = cdecl;
+	  ret = GS_OK;
+	}
 	  break;
 
 	case CONST_DECL:

[PATCH][testsuite] Add check_effective_target_autoincdec

[Wilco Dijkstra]

Richard Earnshaw wrote:
> Except that I think this would be better done as an 'effective target'
> test; something like
>
> dg-require-effective-target autoincdec

Right I figured out how to do this - not trivial as it needs a secret flag in 
the
glob call - if anyone knows a better way of doing this I'd like to know!


Add check_effective_target_autoincdec that returns true if a target
runs the auto_inc_dec optimization pass.

OK for commit?

ChangeLog:
2017-08-08  Wilco Dijkstra  

gcc/testsuite/

* gcc.dg/pr46932.c: Use effective_target autoincdec.
* lib/target-supports.exp: Add check_effective_target_autoincdec.
--

diff --git a/gcc/testsuite/gcc.dg/pr46932.c b/gcc/testsuite/gcc.dg/pr46932.c
index 
4eb1a99e1bd9403f8b1c5d0d71ef731ad4a65128..2b39990d036035d22e226b98351a4900a5dbb309
 100644
--- a/gcc/testsuite/gcc.dg/pr46932.c
+++ b/gcc/testsuite/gcc.dg/pr46932.c
@@ -1,7 +1,5 @@
 /* { dg-options "-O2 -fdump-rtl-auto_inc_dec" } */
-
-/* Build on targets which have pre increment.  */
-/* { dg-do compile { target aarch64*-*-* arm*-*-* rs6000-*-* powerpc*-*-* 
arc*-*-* m32r-*-* tic6x-*-* } } */
+/* { dg-require-effective-target autoincdec } */
 
 /* Check that accesses based on the frame pointer do not
use auto increment.  */
diff --git a/gcc/testsuite/lib/target-supports.exp 
b/gcc/testsuite/lib/target-supports.exp
index 
5a6562794b2bdd5f370fc5b26d688f02779a..5219fbf4671e83a6fa7affdab926115e8a23f9cb
 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -8482,3 +8482,18 @@ proc check_effective_target_arm_coproc4_ok { } {
 return [check_cached_effective_target arm_coproc4_ok \
check_effective_target_arm_coproc4_ok_nocache]
 }
+
+# Return 1 if the target supports the auto_inc_dec optimization pass.
+proc check_effective_target_autoincdec { } {
+if { ![check_no_compiler_messages auto_incdec assembly { void f () { }
+} "-O2 -fdump-rtl-auto_inc_dec" ] } {
+  return 0
+}
+
+set dumpfile [glob -nocomplain 
"auto_incdec[pid].c.\[0-9\]\[0-9\]\[0-9\]r.auto_inc_dec"]
+if { [file exists $dumpfile ] } {
+   file delete $dumpfile
+   return 1
+}
+return 0
+}

Re: [PATCH] i386: Don't use frame pointer without stack access

[H.J. Lu]

On Wed, Aug 9, 2017 at 4:22 AM, Richard Biener
 wrote:
> On August 9, 2017 9:53:05 AM GMT+02:00, Richard Sandiford 
>  wrote:
>>Richard Biener  writes:
>>> On August 8, 2017 7:36:35 PM GMT+02:00, Richard Sandiford
>>>  wrote:
Richard Sandiford  writes:
> Richard Biener  writes:
>> On August 8, 2017 6:38:30 PM GMT+02:00, "H.J. Lu"
 wrote:
>>>On Mon, Aug 7, 2017 at 1:05 PM, Richard Sandiford
>>> wrote:
 Arjan van de Ven  writes:
> On 8/7/2017 8:43 AM, Jakub Jelinek wrote:
>> On Mon, Aug 07, 2017 at 08:39:24AM -0700, H.J. Lu wrote:
>>> When Linux/x86-64 kernel is compiled with
-fno-omit-frame-pointer.
>>> this optimization removes more than 730
>>>
>>> pushq %rbp
>>> movq %rsp, %rbp
>>> popq %rbp
>>
>> If you don't want the frame pointer, why are you compiling
>>with
>> -fno-omit-frame-pointer?  Are you going to add
>> -fforce-no-omit-frame-pointer or something similar so that
people
>>>can
>> actually get what they are asking for?  This doesn't really
>>make
>>>sense.
>> It is perfectly fine to omit frame pointer by default, when it
>>>isn't
>> required for something, but if the user asks for it, we
shouldn't
>>>ignore his
>> request.
>>
>
>
> wanting a framepointer is very nice and desired...  ... but if
the
> optimizer/ins scheduler moves instructions outside of the
>>frame'd
> portion, (it does it for cases like below as well), the value
>>is
> already negative for these functions that don't have stack use.
>
> :
> movall_schedules@@Base-0x38460,%rax
> push   %rbp
> mov%rsp,%rbp
> pop%rbp
> cmpq   $0x0,(%rax)
> setne  %al
> movzbl %al,%eax
> retq

 Yeah, and it could be even weirder for big single-block
>>functions.
 I think GCC has been doing this kind of scheduling of prologue
>>and
 epilogue instructions for a while, so there hasn*t really been a
 guarantee which parts of the function will have a new FP and
>>which
 will still have the old one.

 Also, with an arbitrarily-picked host compiler (GCC 6.3.1),
>>>shrink-wrapping
 kicks in when the following is compiled with -O3
>>>-fno-omit-frame-pointer:

 void f (int *);
 void
 g (int *x)
 {
   for (int i = 0; i < 1000; ++i)
 x[i] += 1;
   if (x[0])
 {
   int temp;
   f ();
 }
 }

 so only the block with the call to f sets up FP.  The relatively
 long-running loop runs with the caller's FP.

 I hope we can go for a target-independent position that what
>>HJ*s
 patch does is OK...

>>>
>>>In light of this,  I am resubmitting my patch.  I added 3 more
>>>testcases
>>>and also handle:
>>>
>>>typedef int v8si __attribute__ ((vector_size (32)));
>>>
>>>void
>>>foo (v8si *out_start, v8si *out_end, v8si *regions)
>>>{
>>>v8si base = regions[3];
>>>*out_start = base;
>>>*out_end = base;
>>>}
>>>
>>>OK for trunk?
>>
>> The invoker specified -fno-omit-frame-pointer, why did you
>>eliminate
it?
>> I'd argue it's OK when neither -f nor -fno- is explicitly
>>specified
>> irrespective of the default in case we document the change but an
>> explicit -fno- is pretty clear.
>
> I don't buy that we're ignoring the user.  -fomit-frame-pointer
>>says
> that, when you're creating a frame, it's OK not to set up the frame
> pointer.  Forcing it off means that if you create a frame, you need
> to set up the frame pointer too.  But it doesn't say anything about
> whether the frame itself is needed.  I.e. it's
-fno-omit-frame*-pointer*
> rather than -fno-omit-frame.
>>>
>>> Isn't that a bit splitting hairs if you look at (past) history?
>>
>>I guess it would have been splitting hairs in the days when they
>>amounted to the same thing, i.e. when there was no behaviour that
>>would match "-fomit-frame" and when the prologue and epilogue were
>>glued to the start and end of the function.  But that was quite a
>>long time ago.  Shrink-wrapping at least means that omitting the frame
>>and omitting the frame pointer are different things, and it seems
>>fair that -fomit-frame-pointer has followed the natural meaning.
>>
>>> You could also interpret 

[PATCH] PR81747, ICE in operator[]

[Alan Modra]

The testcase in this PR is failing in cse2 when processing the
following basic block.

(note 31 30 389 9 [bb 9] NOTE_INSN_BASIC_BLOCK)
(jump_insn 389 31 39 9 (parallel [
(set (pc)
(if_then_else (ne (reg:DI 138)
(const_int 1 [0x1]))
(label_ref:DI 39)
(pc)))
(set (reg:DI 138)
(plus:DI (reg:DI 138)
(const_int -1 [0x])))
(clobber (scratch:CC))
(clobber (scratch:DI))
]) "/home/alan/src/tmp/pr81747.c":19 832 {ctrdi_internal1}
 (int_list:REG_BR_PROB 38000 (nil))
 -> 39)
;;  succ:   13 [always (guessed)]  (FALLTHRU,LOOP_EXIT)

The insn is rather odd.  It meets single_set by virtue of reg 138
being dead, and since the branch destination is the fall-through, the
whole basic block could be deleted..

Regardless of whether this insn should have been deleted earlier, it
seems wrong to try to infer anything about the condition based on the
jump destination when we have a degenerate conditional jump that
branches to its fall-through.

Bootstrapped and regression tested powerpc64le-linux.  OK?

PR rtl-optimization/81747
* cse.c (cse_extended_basic_block): Don't attempt to record
equivalences for degenerate conditional jumps that branch
to their fall-through.

diff --git a/gcc/cse.c b/gcc/cse.c
index 6a968d1..85be372 100644
--- a/gcc/cse.c
+++ b/gcc/cse.c
@@ -6640,6 +6640,7 @@ cse_extended_basic_block (struct cse_basic_block_data 
*ebb_data)
 equivalences due to the condition being tested.  */
   insn = BB_END (bb);
   if (path_entry < path_size - 1
+ && EDGE_COUNT (bb->succs) == 2
  && JUMP_P (insn)
  && single_set (insn)
  && any_condjump_p (insn))

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] i386: Don't use frame pointer without stack access

[Richard Biener]

On August 9, 2017 9:53:05 AM GMT+02:00, Richard Sandiford 
 wrote:
>Richard Biener  writes:
>> On August 8, 2017 7:36:35 PM GMT+02:00, Richard Sandiford
>>  wrote:
>>>Richard Sandiford  writes:
 Richard Biener  writes:
> On August 8, 2017 6:38:30 PM GMT+02:00, "H.J. Lu"
>>> wrote:
>>On Mon, Aug 7, 2017 at 1:05 PM, Richard Sandiford
>> wrote:
>>> Arjan van de Ven  writes:
 On 8/7/2017 8:43 AM, Jakub Jelinek wrote:
> On Mon, Aug 07, 2017 at 08:39:24AM -0700, H.J. Lu wrote:
>> When Linux/x86-64 kernel is compiled with
>>>-fno-omit-frame-pointer.
>> this optimization removes more than 730
>>
>> pushq %rbp
>> movq %rsp, %rbp
>> popq %rbp
>
> If you don't want the frame pointer, why are you compiling
>with
> -fno-omit-frame-pointer?  Are you going to add
> -fforce-no-omit-frame-pointer or something similar so that
>>>people
>>can
> actually get what they are asking for?  This doesn't really
>make
>>sense.
> It is perfectly fine to omit frame pointer by default, when it
>>isn't
> required for something, but if the user asks for it, we
>>>shouldn't
>>ignore his
> request.
>


 wanting a framepointer is very nice and desired...  ... but if
>>>the
 optimizer/ins scheduler moves instructions outside of the
>frame'd
 portion, (it does it for cases like below as well), the value
>is
 already negative for these functions that don't have stack use.

 :
 movall_schedules@@Base-0x38460,%rax
 push   %rbp
 mov%rsp,%rbp
 pop%rbp
 cmpq   $0x0,(%rax)
 setne  %al
 movzbl %al,%eax
 retq
>>>
>>> Yeah, and it could be even weirder for big single-block
>functions.
>>> I think GCC has been doing this kind of scheduling of prologue
>and
>>> epilogue instructions for a while, so there hasn*t really been a
>>> guarantee which parts of the function will have a new FP and
>which
>>> will still have the old one.
>>>
>>> Also, with an arbitrarily-picked host compiler (GCC 6.3.1),
>>shrink-wrapping
>>> kicks in when the following is compiled with -O3
>>-fno-omit-frame-pointer:
>>>
>>> void f (int *);
>>> void
>>> g (int *x)
>>> {
>>>   for (int i = 0; i < 1000; ++i)
>>> x[i] += 1;
>>>   if (x[0])
>>> {
>>>   int temp;
>>>   f ();
>>> }
>>> }
>>>
>>> so only the block with the call to f sets up FP.  The relatively
>>> long-running loop runs with the caller's FP.
>>>
>>> I hope we can go for a target-independent position that what
>HJ*s
>>> patch does is OK...
>>>
>>
>>In light of this,  I am resubmitting my patch.  I added 3 more
>>testcases
>>and also handle:
>>
>>typedef int v8si __attribute__ ((vector_size (32)));
>>
>>void
>>foo (v8si *out_start, v8si *out_end, v8si *regions)
>>{
>>v8si base = regions[3];
>>*out_start = base;
>>*out_end = base;
>>}
>>
>>OK for trunk?
>
> The invoker specified -fno-omit-frame-pointer, why did you
>eliminate
>>>it?
> I'd argue it's OK when neither -f nor -fno- is explicitly
>specified
> irrespective of the default in case we document the change but an
> explicit -fno- is pretty clear.

 I don't buy that we're ignoring the user.  -fomit-frame-pointer
>says
 that, when you're creating a frame, it's OK not to set up the frame
 pointer.  Forcing it off means that if you create a frame, you need
 to set up the frame pointer too.  But it doesn't say anything about
 whether the frame itself is needed.  I.e. it's
>>>-fno-omit-frame*-pointer*
 rather than -fno-omit-frame.
>>
>> Isn't that a bit splitting hairs if you look at (past) history?
>
>I guess it would have been splitting hairs in the days when they
>amounted to the same thing, i.e. when there was no behaviour that
>would match "-fomit-frame" and when the prologue and epilogue were
>glued to the start and end of the function.  But that was quite a
>long time ago.  Shrink-wrapping at least means that omitting the frame
>and omitting the frame pointer are different things, and it seems
>fair that -fomit-frame-pointer has followed the natural meaning.
>
>> You could also interpret -fno-omit-frame-pointer as obviously forcing
>a
>> frame as otherwise there's nothing to omit...
>
>But applying that kind of interpretation to something like
>-maccumulate-outgoing-args would make inlining all calls 

[PATCH v2] Python testcases to check DWARF output

[Pierre-Marie de Rodat]

Hello,

This is a followup on the submission of this series of patches: 



First, thank you all for the feedback I got so far. :-) I have updated 
the patch based on it:


  * As Matthias asked, in gcc-python.exp I first check if “python3” can
be used, and if not I fall back on “python”.

  * As David suggested, I updated the codebase to work with Python 2.6
and I added tests for the complex and easily testable part of the
Python codebase. The testsuite uses the standard “unittest” package,
and uses the non-standard “coverage” package, if available, to
compute code coverage. I did not go as far as testing everything,
but I think the set of tests is reasonable for now: see
. I’ll continue to enhance the set
of tests if this proposal goes forward. :-)

  * Finally, Richard asked if we could have DIE matchers that are
inlined in the testcase source instead of requiring a separate
Python file for simple cases. Given that there’s no way to have
multi-line DejaGNU directives and that useful patterns are just too
long to be kept on single lines, I was not sure about the way to do
this. I still made up something that should be enough: “>>>” markups
in the source code, and Python expressions for matchers between
them. See examples in gcc.dg/debug/dwarf2-py for examples.

As always, tested on my x86_64-linux box. Thoughts? Thank you in advance!

CL for the first patch:
gcc/testsuite/

* lib/gcc-python.exp: New test library.
* python/testutils.py: New Python helper.

… and for the second one:
gcc/testsuite/

* .gitignore: Add gcc/testsuite/python/htmlcov
* lib/gcc-dwarf.exp: New helper files.
* python/dwarfutils/__init__.py,
python/dwarfutils/data.py,
python/dwarfutils/helpers.py,
python/dwarfutils/objdump.py,
python/dwarfutils/scan_dwarf.py,
python/scan-dwarf.py: New Python helpers.
* python/dwarfutils/test_data.py,
python/dwarfutils/test_objdump.py,
python/runtests.py,
python/test_testutils.py: New testsuite for these Python
helpers.
* gcc.dg/debug/dwarf2-py/dwarf2-py.exp,
gnat.dg/dwarf/dwarf.exp: New test drivers.
* gcc.dg/debug/dwarf2-py/dwarf-die1.c,
gcc.dg/debug/dwarf2-py/dwarf-float.c,
gcc.dg/debug/dwarf2-py/sso.c,
gcc.dg/debug/dwarf2-py/sso.py,
gcc.dg/debug/dwarf2-py/var2.c,
gcc.dg/debug/dwarf2-py/var2.py,
gnat.dg/dwarf/debug9.adb,
gnat.dg/dwarf/debug9.py,
gnat.dg/dwarf/debug11.adb,
gnat.dg/dwarf/debug11.py,
gnat.dg/dwarf/debug12.adb,
gnat.dg/dwarf/debug12.ads: New tests.

--
Pierre-Marie de Rodat
>From 29a482344c7a39d441da2dbed4ba08828c70a28f Mon Sep 17 00:00:00 2001
From: Pierre-Marie de Rodat 
Date: Wed, 26 Jul 2017 14:38:12 +0200
Subject: [PATCH 1/2] Introduce testsuite support to run Python tests

gcc/testsuite/

	* lib/gcc-python.exp: New test library.
	* python/testutils.py: New Python helper.
---
 gcc/testsuite/lib/gcc-python.exp  | 129 ++
 gcc/testsuite/python/testutils.py |  74 ++
 2 files changed, 203 insertions(+)
 create mode 100644 gcc/testsuite/lib/gcc-python.exp
 create mode 100644 gcc/testsuite/python/testutils.py

diff --git a/gcc/testsuite/lib/gcc-python.exp b/gcc/testsuite/lib/gcc-python.exp
new file mode 100644
index 000..33f929433d2
--- /dev/null
+++ b/gcc/testsuite/lib/gcc-python.exp
@@ -0,0 +1,129 @@
+# Copyright (C) 2017 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+# 
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+# 
+# You should have received a copy of the GNU General Public License
+# along with GCC; see the file COPYING3.  If not see
+# .
+
+# Helpers to run a Python interpreter
+#
+# Python scripts are required to be compatible with Python 2.7 and Python 3+.
+
+load_lib "remote.exp"
+
+# Return whether a working Python interpreter is available. If there is one,
+# set the PYTHON_INTERPRETER environment variable to the interpreter to use.
+
+proc check-python-available { } {
+if [ check-python-available-helper "python3" ] {
+	setenv PYTHON_INTERPRETER "python3"
+	return 1
+}
+
+if [ check-python-available-helper "python" ] 

Re: [PATCH][v2] Fix target attribute handling (PR c++/81355).

[Martin Liška]

On 08/08/2017 08:03 PM, Martin Sebor wrote:
> On 08/07/2017 10:59 PM, Martin Liška wrote:
>> On 08/02/2017 09:56 PM, Martin Sebor wrote:
>>> On 08/02/2017 01:04 PM, Jeff Law wrote:
 On 07/28/2017 05:13 AM, Martin Liška wrote:
> Hello.
>
> Following patch skips empty strings in 'target' attribute.
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>
> Ready to be installed?
> Martin
>
> gcc/ChangeLog:
>
> 2017-07-13  Martin Liska  
>
> PR c++/81355
> * attribs.c (sorted_attr_string): Skip empty strings.
>
> gcc/testsuite/ChangeLog:
>
> 2017-07-13  Martin Liska  
>
> PR c++/81355
> * g++.dg/other/pr81355.C: New test.
 OK.  THough one could legitimately ask if this ought to be a parsing error.
>>>
>>> I would suggest to at least issue a warning.  It seems that
>>> the empty string must almost certainly be a bug here, say due
>>> to unintended macro expansion.
>>>
>>> Otherwise, if it should remain silently (and intentionally)
>>> accepted, I recommend to document it.  As it is, the manual
>>> says that the "string" argument is equivalent to compiling
>>> with -mstring which for "" would be rejected by the driver.
>>>
>>> Martin
>>
>> Thanks you both for feedback. I decided to come up with an error message for 
>> that.
>>
>> Feedback appreciated.
> 
> My only comment is on the text of the error message.  I think
> the %' directive is supposed to be used instead of a bare
> apostrophe.  But rather than using the somewhat informal "can't"
> I would suggest to follow other similar diagnostics that might
> print something like:
> 
>   empty string in attribute %
> 
> (analogous to "empty precision in %s format" or "empty scalar
> initializer" etc. in gcc.pot)
> 
> or
> 
>   attribute % argument may not be empty
> 
> (similar to "output filename may not be empty").
> 
> Martin

Hi.

Thank you both for review, both notes resolved in v3.
Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

Ready to be installed?
Martin
>From 227a41219ebb9a10bda80767f5b5f3e460a3e9b9 Mon Sep 17 00:00:00 2001
From: marxin 
Date: Wed, 12 Jul 2017 15:51:45 +0200
Subject: [PATCH] Fix target attribute handling (PR c++/81355).

gcc/ChangeLog:

2017-07-13  Martin Liska  

	PR c++/81355
	* c-attribs.c (handle_target_attribute):
	Report warning for an empty string argument of target attribute.

gcc/testsuite/ChangeLog:

2017-07-13  Martin Liska  

	PR c++/81355
	* g++.dg/other/pr81355.C: New test.
---
 gcc/c-family/c-attribs.c | 13 +
 gcc/testsuite/g++.dg/other/pr81355.C | 14 ++
 2 files changed, 27 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/other/pr81355.C

diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index 626ffa1cde7..b4252229beb 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -3116,6 +3116,19 @@ handle_target_attribute (tree *node, tree name, tree args, int flags,
 		  flags))
 *no_add_attrs = true;
 
+  /* Check that there's no empty string in values of the attribute.  */
+  for (tree t = args; t != NULL_TREE; t = TREE_CHAIN (t))
+{
+  tree value = TREE_VALUE (t);
+  if (TREE_CODE (value) == STRING_CST
+	  && TREE_STRING_LENGTH (value) == 1
+	  && TREE_STRING_POINTER (value)[0] == '\0')
+	{
+	  warning (OPT_Wattributes, "empty string in attribute %");
+	  *no_add_attrs = true;
+	}
+}
+
   return NULL_TREE;
 }
 
diff --git a/gcc/testsuite/g++.dg/other/pr81355.C b/gcc/testsuite/g++.dg/other/pr81355.C
new file mode 100644
index 000..89d1b419581
--- /dev/null
+++ b/gcc/testsuite/g++.dg/other/pr81355.C
@@ -0,0 +1,14 @@
+/* { dg-do compile { target x86_64-*-* } } */
+
+__attribute__((target("default")))
+int foo() {return 1;}
+
+__attribute__((target("arch=core2", "")))
+int foo2() {return 2;} /* { dg-warning "empty string in attribute .target." } */
+
+__attribute__((target("sse4.2", "", "")))
+int foo3() {return 2;} /* { dg-warning "empty string in attribute .target." } */
+
+int main() {
+return foo() + foo2() + foo3();
+}
-- 
2.13.3

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

[H.J. Lu]

On Tue, Aug 8, 2017 at 10:36 PM, Richard Biener
 wrote:
> On August 9, 2017 12:18:41 AM GMT+02:00, "H.J. Lu"  
> wrote:
>>This patch adds -static-pie to GCC driver to create static PIE.  A
>>static
>>position independent executable (PIE) is similar to static executable,
>>but can be loaded at any address without a dynamic linker.  All linker
>>input files must be compiled with -fpie or -fPIE and linker must
>>support
>>--no-dynamic-linker to avoid linking with dynamic linker.  "-z text" is
>>also needed to prevent dynamic relocations in read-only segments.
>>
>>OK for trunk?
>
> What's wrong with -static -pie?
>

-static -pie behaved differently depending on if --enable-default-pie
was used:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81523

I just checked in a patch to make -static always  override -pie.


-- 
H.J.

Re: [PATCH] Convert character arrays to string csts

[Richard Biener]

On August 9, 2017 11:15:44 AM GMT+02:00, "Martin Liška"  wrote:
>On 08/08/2017 03:18 PM, Richard Biener wrote:
>> On Tue, Aug 8, 2017 at 1:47 PM, Martin Liška  wrote:
>>> On 11/09/2016 11:22 AM, Richard Biener wrote:
 On Fri, Nov 4, 2016 at 2:33 PM, Martin Liška 
>wrote:
> On 11/03/2016 02:00 PM, Jan Hubicka wrote:
>>> On 11/03/2016 01:12 PM, Martin Liška wrote:
 +  tree init = DECL_INITIAL (decl);
 +  if (init
 +  && TREE_READONLY (decl)
 +  && can_convert_ctor_to_string_cst (init))
 +DECL_INITIAL (decl) = build_string_cst_from_ctor (init);
>>>
>>> I'd merge these two new functions since they're only ever called
>>> together. We'd then have something like
>>>
>>> if (init && TREE_READONLY (decl))
>>>   init = convert_ctor_to_string_cst (init);
>>> if (init)
>>>   DECL_INITIAL (decl) = init;
>
> Done.
>
>>>
>>> I'll defer to Jan on whether finalize_decl seems like a good
>place
>>> to do this.
>>
>> I think finalize_decl may be bit too early because frontends may
>expects the
>> ctors to be in a way they produced them.  We only want to convert
>those arrays
>> seen by middle-end so I would move the logic to
>varpool_node::analyze
>>
>> Otherwise the patch seems fine to me.
>>
>> Honza
>>>
 diff --git
>a/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple.c
>b/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple.c
 index 283bd1c..b2d1fd5 100644
 --- a/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple.c
 +++ b/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple.c
 @@ -4,12 +4,15 @@
 char *buffer1;
 char *buffer2;

 +const char global[] = {'a', 'b', 'c', 'd', '\0'};
 +
 #define SIZE 1000

 int
 main (void)
 {
   const char* const foo1 = "hello world";
 +  const char local[] = "abcd";

   buffer1 = __builtin_malloc (SIZE);
   __builtin_strcpy (buffer1, foo1);
 @@ -45,6 +48,10 @@ main (void)
 __builtin_abort ();
   if (__builtin_memchr (foo1, null, 12) != foo1 + 11)
 __builtin_abort ();
 +  if (__builtin_memchr (global, null, 5) == 0)
 +__builtin_abort ();
 +  if (__builtin_memchr (local, null, 5) == 0)
 +__builtin_abort ();
>>>
>>> How is that a meaningful test? This seems to work even with an
>>> unpatched gcc. I'd have expected something that shows a benefit
>for
>>> doing this conversion, and maybe also a test that shows it isn't
>>> done in cases where it's not allowed.
>
> It's meaningful as it scans that there's no __builtin_memchr in
>optimized dump.
> I'm adding new tests that does the opposite test.
>
>>>
 tree
 -build_string_literal (int len, const char *str)
 +build_string_literal (int len, const char *str, bool
>build_addr_expr)
>>>
>>> New arguments should be documented in the function comment.
>
> Yep, improved.
>
>>>
 +/* Return TRUE when CTOR can be converted to a string
>constant.  */
>>>
>>> "if", not "when".
>
> Done.
>
>>>
 +  unsigned HOST_WIDE_INT elements = CONSTRUCTOR_NELTS (ctor);
 +  FOR_EACH_CONSTRUCTOR_ELT (CONSTRUCTOR_ELTS (ctor), idx, key,
>value)
 +{
 +  if (key == NULL_TREE
 + || TREE_CODE (key) != INTEGER_CST
 + || !tree_fits_uhwi_p (value)
 + || !useless_type_conversion_p (TREE_TYPE (value),
>char_type_node))
 +   return false;
>>>
>>> Shouldn't all elements have the same type, or do you really have
>to
>>> call useless_type_conversion in a loop?
>>>
 +  /* Allow zero character just at the end of a string.  */
 +  if (integer_zerop (value))
 +   return idx == elements - 1;
>>>
>>> Don't you also have to explicitly check it's there?
>>>
>>>
>>> Bernd
>
>
> Patch can bootstrap on ppc64le-redhat-linux and survives
>regression tests.

 I'm curious about the

 @@ -1495,7 +1495,8 @@ gimplify_decl_expr (tree *stmt_p, gimple_seq
>*seq_p)
 {
   if (!TREE_STATIC (decl))
 {
 - DECL_INITIAL (decl) = NULL_TREE;
 + if (!TREE_READONLY (decl) || TREE_CODE (init) !=
>STRING_CST)
 +   DECL_INITIAL (decl) = NULL_TREE;
   init = build2 (INIT_EXPR, void_type_node, decl,
>init);
   gimplify_and_add (init, seq_p);
   ggc_free (init);

 change.  Why keep DECL_INITIAL if you build a INIT_EXPR anyway?

 @@ -4438,6 +4439,19 @@ gimplify_init_constructor (tree *expr_p,
 gimple_seq *pre_p, gimple_seq *post_p,
 

Re: [PATCH] Convert character arrays to string csts

[Martin Liška]

On 08/08/2017 03:18 PM, Richard Biener wrote:
> On Tue, Aug 8, 2017 at 1:47 PM, Martin Liška  wrote:
>> On 11/09/2016 11:22 AM, Richard Biener wrote:
>>> On Fri, Nov 4, 2016 at 2:33 PM, Martin Liška  wrote:
 On 11/03/2016 02:00 PM, Jan Hubicka wrote:
>> On 11/03/2016 01:12 PM, Martin Liška wrote:
>>> +  tree init = DECL_INITIAL (decl);
>>> +  if (init
>>> +  && TREE_READONLY (decl)
>>> +  && can_convert_ctor_to_string_cst (init))
>>> +DECL_INITIAL (decl) = build_string_cst_from_ctor (init);
>>
>> I'd merge these two new functions since they're only ever called
>> together. We'd then have something like
>>
>> if (init && TREE_READONLY (decl))
>>   init = convert_ctor_to_string_cst (init);
>> if (init)
>>   DECL_INITIAL (decl) = init;

 Done.

>>
>> I'll defer to Jan on whether finalize_decl seems like a good place
>> to do this.
>
> I think finalize_decl may be bit too early because frontends may expects 
> the
> ctors to be in a way they produced them.  We only want to convert those 
> arrays
> seen by middle-end so I would move the logic to varpool_node::analyze
>
> Otherwise the patch seems fine to me.
>
> Honza
>>
>>> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple.c 
>>> b/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple.c
>>> index 283bd1c..b2d1fd5 100644
>>> --- a/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple.c
>>> +++ b/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple.c
>>> @@ -4,12 +4,15 @@
>>> char *buffer1;
>>> char *buffer2;
>>>
>>> +const char global[] = {'a', 'b', 'c', 'd', '\0'};
>>> +
>>> #define SIZE 1000
>>>
>>> int
>>> main (void)
>>> {
>>>   const char* const foo1 = "hello world";
>>> +  const char local[] = "abcd";
>>>
>>>   buffer1 = __builtin_malloc (SIZE);
>>>   __builtin_strcpy (buffer1, foo1);
>>> @@ -45,6 +48,10 @@ main (void)
>>> __builtin_abort ();
>>>   if (__builtin_memchr (foo1, null, 12) != foo1 + 11)
>>> __builtin_abort ();
>>> +  if (__builtin_memchr (global, null, 5) == 0)
>>> +__builtin_abort ();
>>> +  if (__builtin_memchr (local, null, 5) == 0)
>>> +__builtin_abort ();
>>
>> How is that a meaningful test? This seems to work even with an
>> unpatched gcc. I'd have expected something that shows a benefit for
>> doing this conversion, and maybe also a test that shows it isn't
>> done in cases where it's not allowed.

 It's meaningful as it scans that there's no __builtin_memchr in optimized 
 dump.
 I'm adding new tests that does the opposite test.

>>
>>> tree
>>> -build_string_literal (int len, const char *str)
>>> +build_string_literal (int len, const char *str, bool build_addr_expr)
>>
>> New arguments should be documented in the function comment.

 Yep, improved.

>>
>>> +/* Return TRUE when CTOR can be converted to a string constant.  */
>>
>> "if", not "when".

 Done.

>>
>>> +  unsigned HOST_WIDE_INT elements = CONSTRUCTOR_NELTS (ctor);
>>> +  FOR_EACH_CONSTRUCTOR_ELT (CONSTRUCTOR_ELTS (ctor), idx, key, value)
>>> +{
>>> +  if (key == NULL_TREE
>>> + || TREE_CODE (key) != INTEGER_CST
>>> + || !tree_fits_uhwi_p (value)
>>> + || !useless_type_conversion_p (TREE_TYPE (value), char_type_node))
>>> +   return false;
>>
>> Shouldn't all elements have the same type, or do you really have to
>> call useless_type_conversion in a loop?
>>
>>> +  /* Allow zero character just at the end of a string.  */
>>> +  if (integer_zerop (value))
>>> +   return idx == elements - 1;
>>
>> Don't you also have to explicitly check it's there?
>>
>>
>> Bernd


 Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>>
>>> I'm curious about the
>>>
>>> @@ -1495,7 +1495,8 @@ gimplify_decl_expr (tree *stmt_p, gimple_seq *seq_p)
>>> {
>>>   if (!TREE_STATIC (decl))
>>> {
>>> - DECL_INITIAL (decl) = NULL_TREE;
>>> + if (!TREE_READONLY (decl) || TREE_CODE (init) != STRING_CST)
>>> +   DECL_INITIAL (decl) = NULL_TREE;
>>>   init = build2 (INIT_EXPR, void_type_node, decl, init);
>>>   gimplify_and_add (init, seq_p);
>>>   ggc_free (init);
>>>
>>> change.  Why keep DECL_INITIAL if you build a INIT_EXPR anyway?
>>>
>>> @@ -4438,6 +4439,19 @@ gimplify_init_constructor (tree *expr_p,
>>> gimple_seq *pre_p, gimple_seq *post_p,
>>> break;
>>>   }
>>>
>>> +   /* Replace a ctor with a string constant with possible.  */
>>> +   if (TREE_READONLY (object)
>>> +   && VAR_P (object))
>>> + 

Re: [PATCH][Arm] Test suite failures resulting from deprecation of -mstructure-size-boundary

[Richard Earnshaw (lists)]

On 09/08/17 06:25, Michael Collison wrote:
> Because the comment (for example) in g+=.dg/ext/packed8.C says
> 
>  // NOTE: This test assumes packed structure layout differs from unpacked
> //   structure layout.  This isn't true, e.g., with the default
> //   arm-none-elf options.
> 
> If we could just delete the -mstructure-size-boundary=8 option why was it 
> added in the first place?
> 

Because the comment it out of date.  It was added at a time when the
default structure size boundard (pre eabi) was 32.

R.

> -Original Message-
> From: Richard Earnshaw (lists) [mailto:richard.earns...@arm.com] 
> Sent: Monday, August 7, 2017 5:32 AM
> To: Michael Collison ; gcc-patches@gcc.gnu.org
> Cc: nd 
> Subject: Re: [PATCH][Arm] Test suite failures resulting from deprecation of 
> -mstructure-size-boundary
> 
> On 06/08/17 00:25, Michael Collison wrote:
>> This patch fixes test case failures on arm targets due to 
>> '-mstructure-size-boundary' being deprecated. The test cases were failing 
>> because a warning was being issued and due to the fact that the size of 
>> packed and unpacked structures is the same after deprecating 
>> '-mstructure-size-boundary'
>>
>> Okay for trunk?
>>
>> 2017-08-04  Michael Collison  
>>
>>  * testsuite/g++.dg/ext/packed8.C: Skip test for arm_eabi.
>>  * testsuite/g++.dg/init/array16.C: Skip test for arm_eabi.
>>  * testsuite/g++.dg/other/crash-4.C: Skip test for arm_eabi.
>>  * testsuite/gcc.dg/builtin-stringop-chk-1.c: Skip test for arm_eabi.
>>
> 
> Why would we want to skip the test?  If you delete the 
> -mstructure-size-boundary option then the test should execute correctly on 
> all arm-eabi based platforms and most other ARM platforms where 8 was the 
> default anyway.
> 
> R.
> 
>>
>> pr7519v1.patch
>>
>>
>> diff --git a/gcc/testsuite/g++.dg/ext/packed8.C 
>> b/gcc/testsuite/g++.dg/ext/packed8.C
>> index 91ee8b3..4f38670 100644
>> --- a/gcc/testsuite/g++.dg/ext/packed8.C
>> +++ b/gcc/testsuite/g++.dg/ext/packed8.C
>> @@ -2,7 +2,7 @@
>>  // NOTE: This test assumes packed structure layout differs from unpacked
>>  //   structure layout.  This isn't true, e.g., with the default
>>  //   arm-none-elf options.
>> -// { dg-options "-mstructure-size-boundary=8" { target arm*-*-* } }
>> +// { dg-skip-if "packed structure layout does not differ from 
>> +unpacked layout" { { arm*-*-* } && { arm_eabi } } }
>>  
>>  class A
>>  {
>> diff --git a/gcc/testsuite/g++.dg/init/array16.C 
>> b/gcc/testsuite/g++.dg/init/array16.C
>> index 188d1a8..3334e25 100644
>> --- a/gcc/testsuite/g++.dg/init/array16.C
>> +++ b/gcc/testsuite/g++.dg/init/array16.C
>> @@ -1,7 +1,7 @@
>>  // Causes timeout for the MMIX simulator on a 3GHz P4 and we can't  
>> // have "compile" for some targets and "run" for others.
>>  // { dg-do run { target { ! mmix-*-* } } } -// { dg-options 
>> "-mstructure-size-boundary=8" { target arm*-*-* } }
>> +// { dg-skip-if "packed structure layout does not differ from 
>> +unpacked layout" { { arm*-*-* } && { arm_eabi } } }
>>  
>>  // Copyright (C) 2004 Free Software Foundation, Inc.
>>  // Contributed by Nathan Sidwell 8 Dec 2004  
>> diff --git a/gcc/testsuite/g++.dg/other/crash-4.C 
>> b/gcc/testsuite/g++.dg/other/crash-4.C
>> index a77fe05..8530f44 100644
>> --- a/gcc/testsuite/g++.dg/other/crash-4.C
>> +++ b/gcc/testsuite/g++.dg/other/crash-4.C
>> @@ -7,7 +7,7 @@
>>  // NOTE: This test assumes packed structure layout differs from unpacked
>>  //   structure layout.  This isn't true, e.g., with the default
>>  //   arm-none-elf options.
>> -// { dg-options "-mstructure-size-boundary=8" { target arm*-*-* } }
>> +// { dg-skip-if "packed structure layout does not differ" { { 
>> +arm*-*-* } && { arm_eabi } } }
>>  
>>  struct a
>>  {
>> diff --git a/gcc/testsuite/gcc.dg/builtin-stringop-chk-1.c 
>> b/gcc/testsuite/gcc.dg/builtin-stringop-chk-1.c
>> index e265578..d839097 100644
>> --- a/gcc/testsuite/gcc.dg/builtin-stringop-chk-1.c
>> +++ b/gcc/testsuite/gcc.dg/builtin-stringop-chk-1.c
>> @@ -2,7 +2,7 @@
>> are emitted properly.  */
>>  /* { dg-do compile } */
>>  /* { dg-options "-O2 -Wno-format -std=gnu99 
>> -ftrack-macro-expansion=0" } */
>> -/* { dg-additional-options "-mstructure-size-boundary=8" { target 
>> arm*-*-* } } */
>> +// { dg-skip-if "packed structure layout does not differ from 
>> +unpacked layout" { { arm*-*-* } && { arm_eabi } } }
>>  // { dg-skip-if "packed attribute missing for t" { "epiphany-*-*" } }
>>  
>>  extern void abort (void);
>>
> 

Re: [PATCH] i386: Don't use frame pointer without stack access

[Richard Sandiford]

Richard Biener  writes:
> On August 8, 2017 7:36:35 PM GMT+02:00, Richard Sandiford
>  wrote:
>>Richard Sandiford  writes:
>>> Richard Biener  writes:
 On August 8, 2017 6:38:30 PM GMT+02:00, "H.J. Lu"
>> wrote:
>On Mon, Aug 7, 2017 at 1:05 PM, Richard Sandiford
> wrote:
>> Arjan van de Ven  writes:
>>> On 8/7/2017 8:43 AM, Jakub Jelinek wrote:
 On Mon, Aug 07, 2017 at 08:39:24AM -0700, H.J. Lu wrote:
> When Linux/x86-64 kernel is compiled with
>>-fno-omit-frame-pointer.
> this optimization removes more than 730
>
> pushq %rbp
> movq %rsp, %rbp
> popq %rbp

 If you don't want the frame pointer, why are you compiling with
 -fno-omit-frame-pointer?  Are you going to add
 -fforce-no-omit-frame-pointer or something similar so that
>>people
>can
 actually get what they are asking for?  This doesn't really make
>sense.
 It is perfectly fine to omit frame pointer by default, when it
>isn't
 required for something, but if the user asks for it, we
>>shouldn't
>ignore his
 request.

>>>
>>>
>>> wanting a framepointer is very nice and desired...  ... but if
>>the
>>> optimizer/ins scheduler moves instructions outside of the frame'd
>>> portion, (it does it for cases like below as well), the value is
>>> already negative for these functions that don't have stack use.
>>>
>>> :
>>> movall_schedules@@Base-0x38460,%rax
>>> push   %rbp
>>> mov%rsp,%rbp
>>> pop%rbp
>>> cmpq   $0x0,(%rax)
>>> setne  %al
>>> movzbl %al,%eax
>>> retq
>>
>> Yeah, and it could be even weirder for big single-block functions.
>> I think GCC has been doing this kind of scheduling of prologue and
>> epilogue instructions for a while, so there hasn*t really been a
>> guarantee which parts of the function will have a new FP and which
>> will still have the old one.
>>
>> Also, with an arbitrarily-picked host compiler (GCC 6.3.1),
>shrink-wrapping
>> kicks in when the following is compiled with -O3
>-fno-omit-frame-pointer:
>>
>> void f (int *);
>> void
>> g (int *x)
>> {
>>   for (int i = 0; i < 1000; ++i)
>> x[i] += 1;
>>   if (x[0])
>> {
>>   int temp;
>>   f ();
>> }
>> }
>>
>> so only the block with the call to f sets up FP.  The relatively
>> long-running loop runs with the caller's FP.
>>
>> I hope we can go for a target-independent position that what HJ*s
>> patch does is OK...
>>
>
>In light of this,  I am resubmitting my patch.  I added 3 more
>testcases
>and also handle:
>
>typedef int v8si __attribute__ ((vector_size (32)));
>
>void
>foo (v8si *out_start, v8si *out_end, v8si *regions)
>{
>v8si base = regions[3];
>*out_start = base;
>*out_end = base;
>}
>
>OK for trunk?

 The invoker specified -fno-omit-frame-pointer, why did you eliminate
>>it?
 I'd argue it's OK when neither -f nor -fno- is explicitly specified
 irrespective of the default in case we document the change but an
 explicit -fno- is pretty clear.
>>>
>>> I don't buy that we're ignoring the user.  -fomit-frame-pointer says
>>> that, when you're creating a frame, it's OK not to set up the frame
>>> pointer.  Forcing it off means that if you create a frame, you need
>>> to set up the frame pointer too.  But it doesn't say anything about
>>> whether the frame itself is needed.  I.e. it's
>>-fno-omit-frame*-pointer*
>>> rather than -fno-omit-frame.
>
> Isn't that a bit splitting hairs if you look at (past) history?

I guess it would have been splitting hairs in the days when they
amounted to the same thing, i.e. when there was no behaviour that
would match "-fomit-frame" and when the prologue and epilogue were
glued to the start and end of the function.  But that was quite a
long time ago.  Shrink-wrapping at least means that omitting the frame
and omitting the frame pointer are different things, and it seems
fair that -fomit-frame-pointer has followed the natural meaning.

> You could also interpret -fno-omit-frame-pointer as obviously forcing a
> frame as otherwise there's nothing to omit...

But applying that kind of interpretation to something like
-maccumulate-outgoing-args would make inlining all calls within a
function invalid, since there'd no longer be arguments to accumulate.

I think this kind of disagreement just emphasises that if we really
need a "always emit a prologue at the very start, an epilogue at the
very end, and always use 

Re: [PATCH 2/3] Matching tokens: C parts (v2)

[Marek Polacek]

On Tue, Aug 08, 2017 at 04:37:28PM -0400, David Malcolm wrote:
> Jason said (for the C++ part of the patch):
> > About passing parser in or not, I'm happy with the current approach; 
> > adding things to the stack isn't free in a highly recursive program 
> > like GCC.
> (in https://gcc.gnu.org/ml/gcc-patches/2017-08/msg00535.html )
> 
> Jeff: is the C patch OK for trunk (assuming the rest of the kit is
> approved), or were you just reporting that you spot-checked the
> mechanical changes and that you hadn't seen any problems with those?

The C part is ok.

Marek