Re: [PATCH] Fix wrong code with truncated string literals (PR 86711/86714)

2018-08-14 Thread Jeff Law
On 08/03/2018 03:38 PM, Bernd Edlinger wrote:
> On 08/03/18 23:15, Jeff Law wrote:
>> On 07/30/2018 02:21 PM, Bernd Edlinger wrote:
>>> On 07/30/18 21:52, Martin Sebor wrote:
 On 07/30/2018 09:24 AM, Bernd Edlinger wrote:
> On 07/30/18 01:05, Martin Sebor wrote:
>> On 07/29/2018 04:56 AM, Bernd Edlinger wrote:
>>> Hi!
>>>
>>> This fixes two wrong code bugs where string_constant
>>> returns over length string constants.  Initializers
>>> like that are rejected in C++, but valid in C.
>>
>> If by valid you are referring to declarations like the one in
>> the added test:
>>
>>   const char a[2][3] = { "1234", "xyz" };
>>
>> then (as I explained), the excess elements in "1234" make
>> the char[3] initialization and thus the test case undefined.
>> I have resolved bug 86711 as invalid on those grounds.
>>
>> Bug 86711 has a valid test case that needs to be fixed, along
>> with bug 86688 that I raised for the same underlying problem:
>> considering the excess nul as part of the string.  As has been
>> discussed in a separate bug, rather than working around
>> the excessively long strings in the middle-end, it would be
>> preferable to avoid creating them to begin with.
>>
>> I'm already working on a fix for bug 86688, in part because
>> I introduced the code change and also because I'm making other
>> changes in this area -- bug 86552.  Both of these in response
>> to your comments.
>>
>
> Sorry, I must admit, I have completely lost track on how many things
> you are trying to work in parallel.
>
> Nevertheless I started to review you pr86552 patch here:
> https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01593.html
>
> But so far you did not respond to me.
>
> Well actually I doubt your patch does apply to trunk,
> maybe you start to re-base that one, and post it again
> instead?

 I read your comments and have been busy working on enhancing
 the patch (among other things).  There are a large number of
 additional contexts where constant strings are expected and
 where a missing nul needs to be detected.  Some include
 additional instances of strlen calls that my initial patch
 didn't handle, many more others that involve other string
 functions.  I have posted an updated patch that applies
 cleanly and that handles the first set.

 There is also a class of problems involving constant character
 arrays initialized by a braced list, as in char [] = { x, y, z };
 Those are currently not recognized as strings even if they are
 nul-terminated, but they are far more likely to be a source of
 these problems than string literals, certainly in C++ where
 string initializers must fit in the array.  I am testing a patch
 to convert those into STRING_CST so they can be handled as well.

 Since initializing arrays with more elements than fit is
 undefined in C and since the behavior is undefined at compile
 time it seems to me that rejecting such initializers with
 a hard error (as opposed to a warning) would be appropriate
 and obviate having to deal with them in the middle-end.

>>>
>>> We do not want to change what is currently accepted by the
>>> front end. period.
>> ?!?  I don't follow this.  When we can detect an error, we should issue
>> a suitable diagnostic.  As is mentioned later in the thread, some
>> diagnostics are considered "pedantic" and are conditional.  But I don't
>> see how you get to "We do not want to change what is currently accepted
>> by the front end. period."  That seems like far too strong of a statement.
>>
> 
> What I mean here is: we should fix a middle-end consistency issue with
> the string constants.  When that is done, we can decide to make
> change a pedantic error into a hard error, but IMHO it should be an 
> independent
> patch of it's own.
Note Jakub has indicated why changing this to a hard error would be a
mistake and what the semantics ought to be in this scenario.  so ignore
what I said about turning this into a hard error.  I think we do
truncation per the defined semantics and present the middle end with
"canonicalized" literals.

> 
> By the way, I found that Fortran has non-zero terminated strings with
> index range starting from 1.
> Then there are also overlength strings in Fortran, that have excess precision.
> Should we forbid that Fortran feature as well?
I have no clue.

> 
> Then Ada and Go do not have zero-terminated strings, but I do not know
> if these can hit the strlen pass.
I wouldn't think so, but maybe in code that's meant to interoperate with
C.  Or if they implemented a C looking strlen and GCC recognized it and
turned it into a strlen call.  I don't think we do anything like that
now, but you never know when someone might add it.

jeff


Re: [PATCH] Fix wrong code with truncated string literals (PR 86711/86714)

2018-08-14 Thread Jeff Law
On 08/03/2018 03:28 PM, Jakub Jelinek wrote:
> On Fri, Aug 03, 2018 at 03:16:41PM -0600, Jeff Law wrote:
>> On 07/31/2018 12:33 AM, Jakub Jelinek wrote:
>>> On Mon, Jul 30, 2018 at 10:01:38PM -0600, Martin Sebor wrote:
> We do not want to change what is currently accepted by the
> front end. period.

 On whose behalf are you making such categorical statements?
 It was Jakub and Richard's suggestion in bug 86714 to reject
 the undefined excessive initializers and I happen to like
 the idea.  I don't recall anyone making a decision about what
>>>
>>> It was not my suggestion and it is already rejected with -pedantic-errors,
>>> which is all that is needed, reject it in pedantic mode.
>>> Otherwise there is a warning emitted by default which is enough.
>> But I think that's a mistake.  I think a hard error at this point is
>> warranted and the safest thing to do.
> 
> The normal behavior when it isn't an error is that the initializer is
> truncated.  That is what happens if I use
> struct S { char a[4]; };
> struct S r = { "abc" };
> struct S s = { "abcd" };
> struct S t = { "abcde" };
> 
> C says that in the s.a initializer is actually just 'a', 'b',
> 'c', 'd' rather than 'a', 'b', 'c', 'd', '\0', so there is a silent
> truncation in the language naturally, so the extension that truncates even
> more with a warning enabled by default is IMHO natural and doesn't need to
> be more pedantic.  We've always truncated, so there should be no surprises.
I wasn't aware C mandated a behavior here.  With that in mind I think we
truncate per the C semantics and get on with our lives :-)

Jeff


Re: PING [PATCH] warn for strlen of arrays with missing nul (PR 86552, 86711, 86714) )

2018-08-14 Thread Jeff Law
On 08/03/2018 07:00 AM, Bernd Edlinger wrote:
> On 08/02/18 22:34, Martin Sebor wrote:
>> On 08/02/2018 12:56 PM, Bernd Edlinger wrote:
>>> On 08/02/18 15:26, Bernd Edlinger wrote:
>
>    /* If the length can be computed at compile-time, return it.  */
> -  len = c_strlen (src, 0);
> +  tree array;
> +  tree len = c_strlen (src, 0, );

 You know the c_strlen tries to compute wide character sizes,
 but strlen does not do that, strlen (L"abc") should give 1
 (or 0 on a BE machine)
 I wonder if that is correct.

>>> [snip]
>
>  static tree
> -fold_builtin_strlen (location_t loc, tree type, tree arg)
> +fold_builtin_strlen (location_t loc, tree fndecl, tree type, tree arg)
>  {
>    if (!validate_arg (arg, POINTER_TYPE))
>  return NULL_TREE;
>    else
>  {
> -  tree len = c_strlen (arg, 0);
> -
> +  tree arr = NULL_TREE;
> +  tree len = c_strlen (arg, 0, );

 Is it possible to write a test case where strlen(L"test") reaches this 
 point?
 what will c_strlen return then?

>>>
>>> Yes, of course it is:
>>>
>>> $ cat y.c
>>> int f(char *x)
>>> {
>>>    return __builtin_strlen(x);
>>> }
>>>
>>> int main ()
>>> {
>>>    return f((char*)"abcdef"[0]);
>>> }
>>> $ gcc -O3 -S y.c
>>> $ cat y.s
>>> main:
>>> .LFB1:
>>> .cfi_startproc
>>> movl    $6, %eax
>>> ret
>>> .cfi_endproc
>>>
>>> The reason is that c_strlen tries to fold wide chars at all.
>>> I do not know when that was introduced, was that already before your last 
>>> patches?
>>
>> The function itself was introduced in 1992 if not earlier,
>> before wide strings even existed.  AFAICS, it has always
>> accepted strings of all widths.  Until r241489 (in GCC 7)
>> it computed their length in bytes, not characters.  I don't
>> know if that was on purpose or if it was just never changed
>> to compute the length in characters when wide strings were
>> first introduced.  From the name I assume it's the latter.
>> The difference wasn't detected until sprintf tests were added
>> for wide string directives.  The ChangeLog description for
>> the change reads: Correctly handle wide strings.  I didn't
>> consider pathological cases like strlen (L"abc").  It
>> shouldn't be difficult to change to fix this case.
>>
> 
> Oh, oh, oh
> 
> $ cat y3.c
> int main ()
> {
>char c[100];
>int x = __builtin_sprintf (c, "%S", L"\u");
> 
>__builtin_printf("%d %ld\n", x,__builtin_strlen(c));
> }
> 
> $ gcc-4.8 -O3 -std=c99 y3.c
> $ ./a.out
> -1 0
> $ gcc -O3 y3.c
> $ ./a.out
> 1 0
> $ echo $LANG
> de_DE.UTF-8
> 
> I would have expected L"\u" to converted to UTF-8
> or another encoding, so the return value if sprintf is
> far from obvious, and probably language dependent.
FWIW, Martin has a patch (under review) that I think will fix this and
includes a testcase that is likely inspired by the code above.

> 
> Why do you think it is a good idea to use really every
> opportunity to optimize totally unnecessary things like
> using the return value from the sprintf function as it is?
> 
> Did you never think this adds a significant maintenance
> burden on the rest of us as well?
It largely came along for free during the implementation of the sprintf
warnings.  That's changed a bit over time, but it's still the case that
the sprintf warnings do all the analysis necessary to optimize the
sprintf return value.

As both Martin and I have stated before the real goal is getting good
warnings from sprintf.  Optimization is a distant second.

jeff


Re: [PATCH] Add a character size parameter to c_strlen/get_range_strlen

2018-08-14 Thread Jeff Law
On 08/14/2018 08:08 AM, Martin Sebor wrote:
> On 08/14/2018 04:25 AM, Bernd Edlinger wrote:
>> Hi,
>>
>> this patch is a follow-up to my patch here:
>> https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01800.html
> 
> As I said multiple times, this patch is redundant -- the issue
> is fixed by the solution I posted back in July:
> 
>   https://gcc.gnu.org/ml/gcc-patches/2018-08/msg00155.html
> 
> and that I have been continuing to build upon (I posted a new
> update yesterday).
So it seems the question here is whether or not Bernd agrees that it's
redundant or he may question if your patch is going to go forward since
it hasn't been acted upon.  He didn't say either way which is
unfortunate.  But I don't think it's meant to be malicious.

Let's not go back to where we were last week.  I think that discussion
got far too heated.

--


Standard procedure in this kind of situation where we have two patches
that are handling the same issue is to bake them off and choose one
based on the technical merits.  To that end...

It would be helpful if you could compare/contrast your patch to Bernd's.
 ie, are there cases that will be handled by one patch better than the
other.  Are there any implementation details that favor one patch over
the other.

Similarly it'd be helpful to have Bernd do the same thing for his patch
compare and contrasting it to yours.


Jeff


Re: [PATCH] Make strlen range computations more conservative

2018-08-14 Thread Jeff Law
On 08/10/2018 10:56 AM, Martin Sebor wrote:
> On 08/08/2018 11:36 PM, Jeff Law wrote:
>> On 08/02/2018 09:42 AM, Martin Sebor wrote:
>>
>>> The warning bits are definitely not okay by me.  The purpose
>>> of the warnings (-W{format,sprintf}-{overflow,truncation} is
>>> to detect buffer overflows.  When a warning doesn't have access
>>> to string length information for dynamically created strings
>>> (like the strlen pass does) it uses array sizes as a proxy.
>>> This is useful both to detect possible buffer overflows and
>>> to prevent false positives for overflows that cannot happen
>>> in correctly written programs.
>> So how much of this falling-back to array sizes as a proxy would become
>> unnecessary if sprintf had access to the strlen pass as an analysis
>> module?
>>
>> As you know we've been kicking that around and from my investigations
>> that doesn't really look hard to do.  Encapsulate the data structures in
>> a class, break up the statement handling into analysis and optimization
>> and we should be good to go.  I'm hoping to start prototyping this week.
>>
>> If we think that has a reasonable chance to eliminate the array-size
>> fallback, then that seems like the most promising path forward.
> 
> We discussed this idea this morning so let me respond here and
> reiterate the answer.  Using the strlen data will help detect
> buffer overflow where the array size isn't available but it
> cannot replace the array size heuristic. Here's a simple
> example:
> 
>   struct S { char a[8]; };
> 
>   char d[8];
>   void f (struct S *s, int i)
>   {
>     sprintf (d, "%s-%i",  s[i].a, i);
>   }
> 
> We don't know the length of s->a but we do know that it can
> be up to 7 bytes long (assuming it's nul-terminated of course)
> so we know the sprintf call can overflow.  Conversely, if
> the size of the destination is increased to 20  the sprintf
> call cannot overflow so the diagnostic can be avoided.
> 
> Removing the array size heuristic would force us to either give
> up on diagnosing the first case or issue false positives for
> the second case.  I think the second alternative would make
> the warning too noisy to be useful.
> 
> The strlen pass will help detect buffer overflows in cases
> where the array size isn't known (e.g., with dynamically
> allocated buffers) but where the length of the string store
> in the array is known.  It will also help avoid false positives
> in cases where the string stored in an array of known size is
> known to be too short to cause an overflow.  For instance here:
> 
>   struct S { char a[8]; };
> 
>   char d[8];
>   void f (struct S *s, int i)
>   {
>     if (strlen (s->a) < 4 && i >= 0 && i < 100)
>   sprintf (d, "%s-%i",  s->a, i);
>   }
ACK.  Thanks for explaining things here too.  I can't speak for others,
but seeing examples along with the explanation is easier for me to absorb.

For Bernd and others -- after kicking things around a bit with Martin,
what we're recommending is that compute_string_length we compute the
bounds using both GIMPLE and C semantics and return both.

Anything which influences code generation or optimization must use the
GIMPLE semantics.  Warnings may use the C semantics in an effort to
improve preciseness.

Martin has some other stuff to flush out of his queue, then he'll be
focused on the changes to compute_string_length.
Jeff


Re: [PATCH][x86] Match movss and movsd "blend" instructions

2018-08-14 Thread Jeff Law
On 08/11/2018 03:54 AM, Allan Sandfeld Jensen wrote:
> On Samstag, 11. August 2018 11:18:39 CEST Jakub Jelinek wrote:
>> On Sat, Aug 11, 2018 at 10:59:26AM +0200, Allan Sandfeld Jensen wrote:
>>> +/* A subroutine of ix86_expand_vec_perm_builtin_1.  Try to implement D
>>> +   using movss or movsd.  */
>>> +static bool
>>> +expand_vec_perm_movs (struct expand_vec_perm_d *d)
>>> +{
>>> +  machine_mode vmode = d->vmode;
>>> +  unsigned i, nelt = d->nelt;
>>> +  rtx x;
>>> +
>>> +  if (d->one_operand_p)
>>> +return false;
>>> +
>>> +  if (TARGET_SSE2 && (vmode == V2DFmode || vmode == V4SFmode))
>>> +;
>>> +  else
>>> +return false;
>>> +
>>> +  /* Only the first element is changed. */
>>
>> Two spaces after .
>>
>>> +  if (d->perm[0] != nelt && d->perm[0] != 0)
>>> +return false;
>>> +  for (i = 1; i < nelt; ++i) {
>>> +{
>>> +  if (d->perm[i] != i + nelt - d->perm[0])
>>> +return false;
>>> +}
>>> +  }
>>
>> Extraneous {}s (both pairs, the outer ones even badly indented).
>>
>> Otherwise LGTM.
>>
> Updated:
> 
> Note as an infrequent contributor don't have commit access, so I need someone 
> reviewing to also commit.
I fixed up the ChangeLog, extracted the test from the original patch and
committed all the bits to the trunk.

Thanks,
jeff


Re: [PATCH][Middle-end]patch for fixing PR 86519

2018-08-14 Thread Jeff Law
On 08/14/2018 08:57 AM, Qing Zhao wrote:
> Hi,
> 
> PR 86519:New test case gcc.dg/strcmpopt_6.c fails with its introduction in 
> r262636.
> 
> ***the root cause is:
> 
> for the following call to memcmp:   __builtin_memcmp (s->s, "a", 3);
> the specified length 3 is larger than the length of "a", it's clearly an 
> out-of-bound access. This new testing case is try to claim that,
> For such out-of-bound access, we should NOT expand this call at all. The new 
> added in-lining expansion was prohibited under
> such situation, However, the expansion to hardware compare insn (old code) is 
> NOT prohibited under such situation. 
> on powerPC, the above call to memcmp is expanded to hardware compare insn. 
> therefore, the testing case failed.
> 
> ***in addition to the above major issue, there is also one minor issue with 
> the new testing case itself:
> 
> dg-final { scan-rtl-dump-times "__builtin_memcmp" 6 "expand” }
> this is trying to scan the dumped .expand file to match the string 
> “__builtin_memcmp” exactly 6 times. however, the # of times that
> the string “__builtin_memcmp” appears in the .expand file varies on different 
> target or optimization level, in order to avoid such
> instability, instead of scanning the .expand file to match the string 
> “__builtin_memcmp”,  scanning the final assembly file to match
> the string “memcmp”.
> 
> please review the attached simple patch.
> 
> thanks.
> 
> Qing
> 
> gcc/ChangeLog:
> 
> +2018-08-14  Qing Zhao  
> +
> +   PR testsuite/86519
> +   * builtins.c (expand_builtin_memcmp): Do not expand the call
> +   when overflow is detected.
> +
> gcc/testsuite/ChangeLog:
> 
> +2018-08-14  Qing Zhao 
> +
> +   PR testsuite/86519
> +   * gcc.dg/strcmpopt_6.c: Scan the assembly file instead of
> +   the .expand file.
OK.
jeff


Re: [PATCH] Fix merging of 2 predictors (PR tree-optimization/86925).

2018-08-14 Thread Jeff Law
On 08/13/2018 06:58 AM, Martin Liška wrote:
> Hi.
> 
> Following patch handles and issue seen in Linux kernel. It's about
> __builtin_expects seen in a PHI node.
> 
> Another issue I saw is compilation with -frounding-math. In that case
> we should use non-rounding math for folding of probability value in
> __builtin_with_probability.
> 
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
> 
> Ready to be installed?
> Martin
> 
> gcc/ChangeLog:
> 
> 2018-08-13  Martin Liska  
> 
> PR tree-optimization/86925
>   * predict.c (expr_expected_value_1): When taking
> later predictor, assign also probability.
> Use fold_build2_initializer_loc in order to fold
> the expression in -frounding-math.
> 
> gcc/testsuite/ChangeLog:
> 
> 2018-08-13  Martin Liska  
> 
> PR tree-optimization/86925
>   * gcc.dg/predict-20.c: New test.
>   * gcc.dg/predict-21.c: New test.
OK.
jeff


Re: fix reassoc cut

2018-08-14 Thread Jeff Law
On 08/14/2018 10:49 AM, Alexandre Oliva wrote:
> I guess this was a cut  I caught it during code review.  It
> doesn't seem to matter much: I haven't been able to get any codegen
> difference from this change, not in a bootstrap, not in test runs.  I
> even put a gcc_unreachable() in case the test on rhs2 were to return
> false (which would never happen after the test on rhs1 passed, when rhs1
> and rhs2 were the same), and still no hits in stage3.  Anyway...
> 
> Regstrapped on x86_64-linux-gnu.  Ok to install?
> 
> From: Alexandre Oliva 
> 
> for  gcc/ChangeLog
> 
>   * tree-ssa-reassoc.c (is_reassociable_op): Fix cut
OK.
jeff


Re: [PATCH] lra: fix FPE when dumping

2018-08-14 Thread Jeff Law
On 08/14/2018 01:33 PM, Vladimir Makarov wrote:
> On 08/14/2018 03:42 AM, Ilya Leoshkevich wrote:
>> The following S/390 code
>>
>>  struct {} b;
>>  void c() {
>>    __asm__("la 0,%0\n"
>>    "la 1,%1\n"
>>    :
>>    : "m" (b), "m" (b)
>>    : "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7", "r8",
>>  "r9", "r10", "r12", "r14");
>>  }
>>
>> results in
>>
>>  internal compiler error: Floating point exception
>>
>> when building with
>>
>>  -fira-verbose=9 -fdump-rtl-all
>>
>> gcc/ChangeLog:
>>
>> 2018-07-25  Ilya Leoshkevich  
>>
>>  PR target/86547
>> * lra-lives.c (remove_some_program_points_and_update_live_ranges):
>>  Check whether lra_live_max_point is 0 before dividing.
> Sure, the patch is ok for me and for committing it into the trunk.
> 
> Thank you, Ilya.
Thanks.  Committed to the trunk.  Bug closed (since it looks like the
LRA loop has been fixed as well).


Jeff


Re: [PATCH] avoid MEM_REF when looking for poor-man's flexible arrays (PR 86914)

2018-08-14 Thread Jeff Law
On 08/14/2018 01:01 PM, Martin Sebor wrote:
> Attached is a patch to avoid calling array_at_struct_end_p()
> with a MEM_REF argument.  The function returns false even if
> the reference does point into such a flexible array member,
> as in:
> 
>   struct A { char i, a[1]; };
>   void f (struct A *p)
>   {
> return strlen (p->a + 1);
>   }
> 
> This fix will likely be made obsolete once the string length
> range optimization is relaxed but since that's a bigger change
> I think GCC might as well emit the correct code for this case
> until then.
> 
> Martin
> 
> gcc-86914.diff
> 
> 
> PR tree-optimization/86914 - wrong code with strlen() of poor-man's flexible 
> array member plus offset
> 
> gcc/ChangeLog:
> 
>   PR tree-optimization/86914
>   * tree-ssa-strlen.c (maybe_set_strlen_range): Avoid MEM_REF.
> 
> gcc/testsuite/ChangeLog:
> 
>   PR tree-optimization/86914
>   * gcc.dg/strlenopt-57.c: New test.
OK.
jeff


Re: [RFC PATCH, i386]: Deprecate -mmitigate-rop

2018-08-14 Thread Jeff Law
On 08/10/2018 05:42 AM, Uros Bizjak wrote:
> This option is fairly ineffective, and in the light of CET, nobody
> seems interested to improve it. Deprecate the option, so it won't lure
> developers to the land of false security.
> 
> 2018-08-10  Uros Bizjak  
> 
> * config/i386/i386.opt (mmitigate-rop): Mark as deprecated.
> * doc/invoke.texi (mmitigate-rop): Remove.
> * config/i386/i386.c: Do not include "regrename.h".
> (ix86_rop_should_change_byte_p, reg_encoded_number)
> (ix86_get_modrm_for_rop, set_rop_modrm_reg_bits, ix86_mitigate_rop):
> Remove.
> (ix86_reorg): Remove call to ix86_mitigate_rop.
> * config/i386/i386.md (attr "modrm_class"): Remove.
> (cmp_ccno_1, mov_xor, movstrict_xor,
> x86_movcc_0_m1. x86_movcc_0_m1_se)
> (x86_movcc_0_m1_neg): Remove modrm_class attribute override.
> 
> testsuite/Changelog:
> 
> 2018-08-10  Uros Bizjak  
> 
> * gcc.target/i386/rop1.c: Remove.
> * gcc.target/i386/pr83554 (dg-options): Remove -mmitigate-rop.
> 
> Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
> 
> Any opinion against the deprecation?
As I noted privately, Red Hat is no longer investing in this space due
to the introduction of CET.

I'll ack deprecation.

jeff


Re: [PATCH] convert braced initializers to strings (PR 71625)

2018-08-14 Thread Martin Sebor

On 08/14/2018 09:24 AM, Martin Sebor wrote:

On 08/14/2018 09:08 AM, Martin Sebor wrote:

On 08/14/2018 07:27 AM, James Greenhalgh wrote:

On Wed, Aug 08, 2018 at 07:17:07PM -0500, Martin Sebor wrote:

On 08/08/2018 05:08 AM, Jason Merrill wrote:

On Wed, Aug 8, 2018 at 9:04 AM, Martin Sebor  wrote:

On 08/07/2018 02:57 AM, Jason Merrill wrote:


On Wed, Aug 1, 2018 at 12:49 AM, Martin Sebor 
wrote:


On 07/31/2018 07:38 AM, Jason Merrill wrote:





Done in the attached patch.  I've also avoided dealing with
zero-length arrays and added tests to make sure their size
stays is regardless of the form of their initializer and
the appropriate warnings are issued.

Using build_string() rather than build_string_literal() needed
a tweak in digest_init_r().  It didn't break anything but since
the array type may not have a domain yet, neither will the
string.  It looks like that may get adjusted later on but I've
temporarily guarded the code with #if 1.  If the change is
fine I'll remove the #if before committing.

This initial patch only handles narrow character initializers
(i.e., those with TYPE_STRING_FLAG set).  Once this gets some
exposure I'd like to extend it to other character types,
including wchar_t.


Hi Martin,

This causes issues for the AArch64 tests (full list below).

I see an error message on the following construct:

  void foo (void)
  {
__Poly8_t x[4] = { 1, 2, 3, 4 };
  }

  init.c:3:20: error: array of inappropriate type initialized from
string constant
  3 |   __Poly8_t x[4] = { 1, 2, 3, 4 };
|

__Poly8_t is a type we define in our backend, through a convoluted
set of
functions, which operates a lot like an unsigned, QI mode type.


I see the error with my aarch64 cross-compiler .  The new code
that does the conversion of array initializers to STRING_CSTs
looks for the TYPE_STRING_FLAG() to be set on the type of
the array elements.  Perhaps __Poly8_t should not have the flag
set?  (If it needs it then I think we'd have to only consider
named character types.)


The change below gets rid of the compilation error.  I don't
know if it's appropriate for the aarch64 back end:

Index: gcc/config/aarch64/aarch64-builtins.c
===
--- gcc/config/aarch64/aarch64-builtins.c(revision 263537)
+++ gcc/config/aarch64/aarch64-builtins.c(working copy)
@@ -643,6 +643,7 @@ aarch64_init_simd_builtin_types (void)
   /* Poly types are a world of their own.  */
   aarch64_simd_types[Poly8_t].eltype = aarch64_simd_types[Poly8_t].itype =
 build_distinct_type_copy (unsigned_intQI_type_node);
+  TYPE_STRING_FLAG (aarch64_simd_types[Poly8_t].eltype) = false;
   aarch64_simd_types[Poly16_t].eltype =
aarch64_simd_types[Poly16_t].itype =
 build_distinct_type_copy (unsigned_intHI_type_node);
   aarch64_simd_types[Poly64_t].eltype =
aarch64_simd_types[Poly64_t].itype =



A second set of tests fail due to changed inlining behaviour for
functions
with char array initialization:

  gcc.target/aarch64/vset_lane_1.c
  gcc.target/aarch64/vneg_s.c
  gcc.target/aarch64/vclz.c


I'm not sure what's going on here.  The tests are very big and
take forever to compile with an aarch64 cross-compiler, and I'm
not sure what to look for.  Can you provide a smaller test case
that shows the issue?


I wonder if these changes might be due to the same problem:
the tests define and initialize arrays of the Int8x16_t type
which is initialized to intQI_type_node, i.e., the signed
form of Poly8_t.  Does the conversion to STRING_CST cause
a performance degradation or is it just that the tests end
up with equivalent but slightly different assembly?

The tests also use int8_t and uint8_t for the expected results.
Those are typedefs for signed and unsigned char, respectively.
Is the conversion to strings for those fine?

Martin


VRP: rewrite the division code (to handle corner cases including 0)

2018-08-14 Thread Aldy Hernandez

Howdy!

In auditing the *_DIV_EXPR code I noticed that we were really botching 
some divisions where the divisor included 0.


Particularly interesting was that we were botching something as simple 
as dividing by [0,0].  We were also incorrectly calculating things like 
[-2,-2] / [0, ], where we should have removed the 0 from the divisor.


Also, the symbolic special casing could be handled by just treating 
symbolic ranges as [-MIN, +MAX] and letting the common code handle then. 
 Similarly for anti ranges, which actually never happen except for the 
constant case, since they've been normalized earlier.


All in all, it was much easier to normalize all the symbolic ranges and 
treat everything generically by performing the division in two chunks... 
the negative numbers and the (non-zero) positive numbers.  And finally, 
unioning the results.  This makes everything much simpler to read with 
minimal special casing.


Finally, my apologies for including a tiny change to the 
POINTER_PLUS_EXPR handling code as well.  It came about the same set of 
auditing tests.


It turns out we can handle POINTER_PLUS_EXPR(~[0,0], [X,Y]) without 
bailing as VR_VARYING in extract_range_from_binary_expr_1.  In doing so, 
I also noticed that ~[0,0] is not the only non-null.  We could also have 
~[0,2] and still know that the pointer is not zero.  I have adjusted 
range_is_nonnull accordingly.


(Yes, we can get something like ~[0,2] for a pointer for things like the 
following in libgcc:


  if (segment_arg == (void *) (uintptr_type) 1)
...
  else if (segment_arg == (void *) (uintptr_type) 2)
return NULL;
  else if (segment_arg != NULL)
segment = (struct stack_segment *) segment_arg;
)

BTW, I am still not happy with the entire interface to wide-int-range.*, 
and have another pending patchset that will simplify things even 
further.  I think everyone will be pleased ;-).


OK pending another round of tests?

Aldy
gcc/

	* tree-vrp.c (abs_extent_range): Remove.
	(range_includes_zero_p): Move further up in file.
	(range_is_nonnull): Make it return true if VR does not contain 0,
	not just if VR is ~[0,0].
	(extract_range_into_wide_ints): Pass wide ints by reference.
	(extract_range_from_binary_expr_1): Handle the case where
	POINTER_PLUS_EXPR's pointer is known to be non-zero.
	Rewrite the *DIV_EXPR code.
	Pass wide ints by reference in all calls to
	extract_range_into_wide_ints.
	* wide-int-range.cc (wide_int_range_div): New.
	* wide-int-range.h (wide_int_range_div): New.
	(wide_int_range_includes_zero_p): New.
	(wide_int_range_zero_p): New.

diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index d553a254878..adfdc01f2d1 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -478,42 +478,6 @@ set_value_range_to_null (value_range *vr, tree type)
   set_value_range_to_value (vr, build_int_cst (type, 0), vr->equiv);
 }
 
-
-/* If abs (min) < abs (max), set VR to [-max, max], if
-   abs (min) >= abs (max), set VR to [-min, min].  */
-
-static void
-abs_extent_range (value_range *vr, tree min, tree max)
-{
-  int cmp;
-
-  gcc_assert (TREE_CODE (min) == INTEGER_CST);
-  gcc_assert (TREE_CODE (max) == INTEGER_CST);
-  gcc_assert (INTEGRAL_TYPE_P (TREE_TYPE (min)));
-  gcc_assert (!TYPE_UNSIGNED (TREE_TYPE (min)));
-  min = fold_unary (ABS_EXPR, TREE_TYPE (min), min);
-  max = fold_unary (ABS_EXPR, TREE_TYPE (max), max);
-  if (TREE_OVERFLOW (min) || TREE_OVERFLOW (max))
-{
-  set_value_range_to_varying (vr);
-  return;
-}
-  cmp = compare_values (min, max);
-  if (cmp == -1)
-min = fold_unary (NEGATE_EXPR, TREE_TYPE (min), max);
-  else if (cmp == 0 || cmp == 1)
-{
-  max = min;
-  min = fold_unary (NEGATE_EXPR, TREE_TYPE (min), min);
-}
-  else
-{
-  set_value_range_to_varying (vr);
-  return;
-}
-  set_and_canonicalize_value_range (vr, VR_RANGE, min, max, NULL);
-}
-
 /* Return true, if VAL1 and VAL2 are equal values for VRP purposes.  */
 
 bool
@@ -538,14 +502,34 @@ vrp_bitmap_equal_p (const_bitmap b1, const_bitmap b2)
 	  && bitmap_equal_p (b1, b2)));
 }
 
-/* Return true if VR is ~[0, 0].  */
+
+/* Return 1 if [MIN, MAX] includes the value zero, 0 if it does not
+   include the value zero, -2 if we cannot tell.  */
+
+int
+range_includes_zero_p (tree min, tree max)
+{
+  tree zero = build_int_cst (TREE_TYPE (min), 0);
+  return value_inside_range (zero, min, max);
+}
+
+/* Return true if VR does not contain 0.  */
 
 bool
 range_is_nonnull (value_range *vr)
 {
-  return vr->type == VR_ANTI_RANGE
-	 && integer_zerop (vr->min)
-	 && integer_zerop (vr->max);
+  if (vr->type == VR_VARYING
+  || vr->type == VR_UNDEFINED
+  || TREE_CODE (vr->min) != INTEGER_CST
+  || TREE_CODE (vr->max) != INTEGER_CST)
+return false;
+  if (vr->type == VR_RANGE)
+return !range_includes_zero_p (vr->min, vr->max);
+  /* ~[0,0] is not the only non-null. We could also have ~[0,5].  */
+  else if (vr->type == VR_ANTI_RANGE)
+return range_includes_zero_p (vr->min, 

C++ PATCH for c++/65043, missing narrowing warning with bool

2018-08-14 Thread Marek Polacek
We weren't complaining about narrowing when converting to bool because
standard_conversion wasn't setting check_narrowing for the converting to bool
case; it only sets it at the end of the function.  Moreover, check_narrowing
wasn't catching real_type -> boolean_type at all, which is wrong;
[dcl.init.list] says that a narrowing conversion is an implicit conversion from
a floating-point type to an integer type, and bool is an integer type.

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

2018-08-14  Marek Polacek  

PR c++/65043
* call.c (standard_conversion): Set check_narrowing.
* typeck2.c (check_narrowing): Use CP_INTEGRAL_TYPE_P rather
than comparing with INTEGER_TYPE.

* g++.dg/concepts/pr67595.C: Add dg-warning.
* g++.dg/cpp0x/Wnarrowing11.C: New test.
* g++.dg/cpp0x/Wnarrowing12.C: New test.
* g++.dg/cpp0x/rv-cast5.C: Add static_cast.

diff --git gcc/cp/call.c gcc/cp/call.c
index 62654a9e407..ded279a0f43 100644
--- gcc/cp/call.c
+++ gcc/cp/call.c
@@ -1387,6 +1387,8 @@ standard_conversion (tree to, tree from, tree expr, bool 
c_cast_p,
conv->rank = cr_pbool;
  if (NULLPTR_TYPE_P (from) && (flags & LOOKUP_ONLYCONVERTING))
conv->bad_p = true;
+ if (flags & LOOKUP_NO_NARROWING)
+   conv->check_narrowing = true;
  return conv;
}
 
diff --git gcc/cp/typeck2.c gcc/cp/typeck2.c
index 674d08762b5..b521f79e8c3 100644
--- gcc/cp/typeck2.c
+++ gcc/cp/typeck2.c
@@ -912,7 +912,7 @@ check_narrowing (tree type, tree init, tsubst_flags_t 
complain, bool const_only)
   if (const_only && !TREE_CONSTANT (init))
 return ok;
 
-  if (TREE_CODE (type) == INTEGER_TYPE
+  if (CP_INTEGRAL_TYPE_P (type)
   && TREE_CODE (ftype) == REAL_TYPE)
 ok = false;
   else if (INTEGRAL_OR_ENUMERATION_TYPE_P (ftype)
diff --git gcc/testsuite/g++.dg/concepts/pr67595.C 
gcc/testsuite/g++.dg/concepts/pr67595.C
index 63162fb4c72..76d1fe62132 100644
--- gcc/testsuite/g++.dg/concepts/pr67595.C
+++ gcc/testsuite/g++.dg/concepts/pr67595.C
@@ -4,7 +4,7 @@ template  concept bool allocatable = requires{{new 
X}->X * };
 template  concept bool semiregular = allocatable;
 template  concept bool readable = requires{requires semiregular};
 template  int weak_input_iterator = requires{{0}->readable};
-template  bool input_iterator{weak_input_iterator};
+template  bool input_iterator{weak_input_iterator}; // { 
dg-warning "narrowing conversion" }
 template  bool forward_iterator{input_iterator};
 template  bool bidirectional_iterator{forward_iterator};
 template 
diff --git gcc/testsuite/g++.dg/cpp0x/Wnarrowing11.C 
gcc/testsuite/g++.dg/cpp0x/Wnarrowing11.C
index e69de29bb2d..5b7323633c7 100644
--- gcc/testsuite/g++.dg/cpp0x/Wnarrowing11.C
+++ gcc/testsuite/g++.dg/cpp0x/Wnarrowing11.C
@@ -0,0 +1,30 @@
+// PR c++/65043
+// { dg-do compile { target c++11 } }
+
+struct X
+{
+  X(bool) { }
+};
+
+struct Y
+{
+  Y(char) { }
+};
+
+struct Z
+{
+  Z(char16_t) { }
+};
+
+struct W
+{
+  W(char32_t) { }
+};
+
+int main() 
+{
+  X x{1.2}; // { dg-error "narrowing conversion" }
+  Y y{1.2}; // { dg-error "narrowing conversion" }
+  Z z{1.2}; // { dg-error "narrowing conversion" }
+  W w{1.2}; // { dg-error "narrowing conversion" }
+}
diff --git gcc/testsuite/g++.dg/cpp0x/Wnarrowing12.C 
gcc/testsuite/g++.dg/cpp0x/Wnarrowing12.C
index e69de29bb2d..83b4d3a58c6 100644
--- gcc/testsuite/g++.dg/cpp0x/Wnarrowing12.C
+++ gcc/testsuite/g++.dg/cpp0x/Wnarrowing12.C
@@ -0,0 +1,32 @@
+// PR c++/65043
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wnarrowing" }
+
+struct X
+{
+  X(bool) { }
+};
+
+struct Y
+{
+  Y(char) { }
+};
+
+struct Z
+{
+  Z(char16_t) { }
+};
+
+struct W
+{
+  W(char32_t) { }
+};
+
+int main() 
+{
+  double d = 1.2;
+  X x{d}; // { dg-warning "narrowing conversion" }
+  Y y{d}; // { dg-warning "narrowing conversion" }
+  Z z{d}; // { dg-warning "narrowing conversion" }
+  W w{d}; // { dg-warning "narrowing conversion" }
+}
diff --git gcc/testsuite/g++.dg/cpp0x/rv-cast5.C 
gcc/testsuite/g++.dg/cpp0x/rv-cast5.C
index c2473e266b6..5233078f7b4 100644
--- gcc/testsuite/g++.dg/cpp0x/rv-cast5.C
+++ gcc/testsuite/g++.dg/cpp0x/rv-cast5.C
@@ -8,5 +8,5 @@ struct hold {
 
 int main()
 {
-  hold{42}();
+  hold{static_cast(42)}();
 }


[PATCH, rs6000] Fix PR86731 vec_sl()

2018-08-14 Thread Will Schmidt
Hi, 

  Here is a first pass at fixing PR86731, which is an issue introduced
when gimple folding the vec_sl() intrinsic.

This has been sniff tested (successfully) on a power7.  Full regtests for
linux/Powerpc systems is pending.  I expect I'll need to tweak some of
the testcase scan-assembler stanzas after reviewing those results, but I
wanted to get this out for review sooner versus later.  :-)

Assuming good results, is this OK for trunk and backport to 8?
Thanks,
-Will

[gcc]

2018-08-14  Will Schmidt  

PR target/86731
* config/rs6000/rs6000.c (rs6000_gimple_fold_builtin): Update logic
  around folding of vec_sl() to handle out of range shift values.

[testsuite]

2018-08-14  Will Schmidt  

PR target/86731
* gcc.target/powerpc/fold-vec-shift-altivectest-1.c: New test.
* gcc.target/powerpc/fold-vec-shift-altivectest-2.c: New test.
* gcc.target/powerpc/fold-vec-shift-altivectest-3.c: New test.

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index ec92e6a..0a84290 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -15481,20 +15481,48 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi)
   builtin_altivec_vsl{b,h,w} -> vsl{b,h,w}.  */
 case ALTIVEC_BUILTIN_VSLB:
 case ALTIVEC_BUILTIN_VSLH:
 case ALTIVEC_BUILTIN_VSLW:
 case P8V_BUILTIN_VSLD:
-  arg0 = gimple_call_arg (stmt, 0);
-  if (INTEGRAL_TYPE_P (TREE_TYPE (TREE_TYPE (arg0)))
- && !TYPE_OVERFLOW_WRAPS (TREE_TYPE (TREE_TYPE (arg0
-   return false;
-  arg1 = gimple_call_arg (stmt, 1);
-  lhs = gimple_call_lhs (stmt);
-  g = gimple_build_assign (lhs, LSHIFT_EXPR, arg0, arg1);
-  gimple_set_location (g, gimple_location (stmt));
-  gsi_replace (gsi, g, true);
-  return true;
+  {
+   location_t loc;
+   gimple_seq stmts = NULL;
+   arg0 = gimple_call_arg (stmt, 0);
+   tree arg0_type = TREE_TYPE (arg0);
+   if (INTEGRAL_TYPE_P (TREE_TYPE (arg0_type))
+   && !TYPE_OVERFLOW_WRAPS (TREE_TYPE (arg0_type)))
+   return false;
+   arg1 = gimple_call_arg (stmt, 1);
+   tree arg1_type = TREE_TYPE (arg1);
+   tree unsigned_arg1_type = unsigned_type_for (TREE_TYPE (arg1));
+   tree unsigned_element_type = unsigned_type_for (TREE_TYPE (arg1_type));
+   loc = gimple_location (stmt);
+   lhs = gimple_call_lhs (stmt);
+   /* Force arg1 into the range valid matching the arg0 type.  */
+   /* Build a vector consisting of the max valid bit-size values.  */
+   int n_elts = VECTOR_CST_NELTS (arg1);
+   int tree_size_in_bits = TREE_INT_CST_LOW (size_in_bytes (arg1_type))
+   * BITS_PER_UNIT;
+   tree element_size = build_int_cst (unsigned_element_type,
+  tree_size_in_bits / n_elts);
+   tree_vector_builder elts (unsigned_type_for (arg1_type), n_elts, 1);
+   for (int i = 0; i < n_elts; i++)
+ elts.safe_push (element_size);
+   tree modulo_tree = elts.build ();
+   /* Modulo the provided shift value against that vector.  */
+   tree unsigned_arg1 = gimple_build (, VIEW_CONVERT_EXPR,
+  unsigned_arg1_type, arg1);
+   tree new_arg1 = gimple_build (, loc, TRUNC_MOD_EXPR,
+ unsigned_arg1_type, unsigned_arg1,
+ modulo_tree);
+   gsi_insert_seq_before (gsi, stmts, GSI_SAME_STMT);
+   /* And finally, do the shift.  */
+   g = gimple_build_assign (lhs, LSHIFT_EXPR, arg0, new_arg1);
+   gimple_set_location (g, gimple_location (stmt));
+   gsi_replace (gsi, g, true);
+   return true;
+  }
 /* Flavors of vector shift right.  */
 case ALTIVEC_BUILTIN_VSRB:
 case ALTIVEC_BUILTIN_VSRH:
 case ALTIVEC_BUILTIN_VSRW:
 case P8V_BUILTIN_VSRD:
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-shift-altivectest-1.c 
b/gcc/testsuite/gcc.target/powerpc/fold-vec-shift-altivectest-1.c
new file mode 100644
index 000..e0546bf
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-shift-altivectest-1.c
@@ -0,0 +1,73 @@
+/* PR86731.  Verify that the rs6000 gimple-folding code handles the
+   left shift operation properly.  This is a testcase variation that
+   explicitly specifies -fwrapv, which is a condition for the
+   gimple folding of the vec_sl() intrinsic.  */
+
+/* { dg-do compile } */
+/* { dg-require-effective-target powerpc_altivec_ok } */
+/* { dg-options "-maltivec -O3 -fwrapv " } */
+
+#include 
+/* original test as reported.  */
+vector unsigned int splat(void)
+{
+vector unsigned int mzero = vec_splat_u32(-1);
+return (vector unsigned int) vec_sl(mzero, mzero);
+}
+
+/* more testcase variations.  */
+vector unsigned char splatu1(void)
+{
+vector unsigned char mzero = vec_splat_u8(-1);
+return 

Re: [PATCH] convert braced initializers to strings (PR 71625)

2018-08-14 Thread Martin Sebor

On 08/14/2018 03:14 PM, Joseph Myers wrote:

On Tue, 14 Aug 2018, James Greenhalgh wrote:


Hi Martin,

This causes issues for the AArch64 tests (full list below).


This change (r263511) also breaks the glibc build for alpha-linux-gnu with
build-many-glibcs.py (using mainline GCC and binutils).  The error I see
is:

/scratch/jmyers/glibc-bot/install/compilers/alpha-linux-gnu/lib/gcc/alpha-glibc-linux-gnu/9.0.0/../../../../alpha-glibc-linux-gnu/bin/ld:
/scratch/jmyers/glibc-bot/build/compilers/alpha-linux-gnu/glibc/alpha-linux-gnu/libc.a(plural.o):
in function `__gettextparse':
/scratch/jmyers/glibc-bot/build/compilers/alpha-linux-gnu/glibc/alpha-linux-gnu/intl/plural.c:1108:(.text+0x218):
relocation truncated to fit: GPRELHIGH against `.sdata'
/scratch/jmyers/glibc-bot/install/compilers/alpha-linux-gnu/lib/gcc/alpha-glibc-linux-gnu/9.0.0/../../../../alpha-glibc-linux-gnu/bin/ld:
/scratch/jmyers/glibc-bot/build/compilers/alpha-linux-gnu/glibc/alpha-linux-gnu/intl/plural.c:1348:(.text+0x300):
relocation truncated to fit: GPRELHIGH against `.sdata'
collect2: error: ld returned 1 exit status
../Rules:224: recipe for target
'/scratch/jmyers/glibc-bot/build/compilers/alpha-linux-gnu/glibc/alpha-linux-gnu/elf/sln'
failed
make[3]: ***
[/scratch/jmyers/glibc-bot/build/compilers/alpha-linux-gnu/glibc/alpha-linux-gnu/elf/sln]
Error 1

This is with Bison 3.0.4, should the version used to produce intl/plural.c
prove relevant.


Can you send me the translation unit and the options it was compiled
with that triggered the errors?

Thanks
Martin


Re: [PATCH] convert braced initializers to strings (PR 71625)

2018-08-14 Thread Joseph Myers
On Tue, 14 Aug 2018, James Greenhalgh wrote:

> Hi Martin,
> 
> This causes issues for the AArch64 tests (full list below).

This change (r263511) also breaks the glibc build for alpha-linux-gnu with 
build-many-glibcs.py (using mainline GCC and binutils).  The error I see 
is:

/scratch/jmyers/glibc-bot/install/compilers/alpha-linux-gnu/lib/gcc/alpha-glibc-linux-gnu/9.0.0/../../../../alpha-glibc-linux-gnu/bin/ld:
 
/scratch/jmyers/glibc-bot/build/compilers/alpha-linux-gnu/glibc/alpha-linux-gnu/libc.a(plural.o):
 
in function `__gettextparse':
/scratch/jmyers/glibc-bot/build/compilers/alpha-linux-gnu/glibc/alpha-linux-gnu/intl/plural.c:1108:(.text+0x218):
 
relocation truncated to fit: GPRELHIGH against `.sdata'
/scratch/jmyers/glibc-bot/install/compilers/alpha-linux-gnu/lib/gcc/alpha-glibc-linux-gnu/9.0.0/../../../../alpha-glibc-linux-gnu/bin/ld:
 
/scratch/jmyers/glibc-bot/build/compilers/alpha-linux-gnu/glibc/alpha-linux-gnu/intl/plural.c:1348:(.text+0x300):
 
relocation truncated to fit: GPRELHIGH against `.sdata'
collect2: error: ld returned 1 exit status
../Rules:224: recipe for target 
'/scratch/jmyers/glibc-bot/build/compilers/alpha-linux-gnu/glibc/alpha-linux-gnu/elf/sln'
 
failed
make[3]: *** 
[/scratch/jmyers/glibc-bot/build/compilers/alpha-linux-gnu/glibc/alpha-linux-gnu/elf/sln]
 
Error 1

This is with Bison 3.0.4, should the version used to produce intl/plural.c 
prove relevant.

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


Re: [PATCH] [C] Warn when calculating abs(unsigned_value)

2018-08-14 Thread Joseph Myers
On Tue, 14 Aug 2018, Martin Jambor wrote:

> when you try compiling a call to function abs and provide an unsigned
> int in the argument in C++, you will get an error about ambiguous
> overload.  In C however, it will pass without silently.  The following
> patch adds a warning for these cases, because I think it is likely that
> such code does not do what the author intended.

abs of unsigned short (promoted to int) seems harmless; I don't see any 
tests to make sure it doesn't warn.  Really the issue seems more like abs 
(or labs / llabs / imaxabs) of an argument whose value might be changed by 
the conversion to int.  Except that's more like a subset of -Wconversion, 
and highly likely to have false positives (for a long argument that is 
actually always in the range of int), so maybe a restriction to unsigned 
arguments makes sense for this warning, but should still only be for 
unsigned arguments at least as wide as the argument type of the abs 
function in question.

> +   else if (DECL_FUNCTION_CODE (expr.value) == BUILT_IN_ABS

This looks like it would only handle abs, not labs / llabs / imaxabs.

> +@code{<} or @code{>=}.  When compiling C, also warn when calculating
> +an absolute value from an unsigned type.  This warning is also enabled

But this would suggest any absolute value function, not just abs.

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


Re: [PATCH] Fix PR19315.

2018-08-14 Thread Joseph Myers
On Tue, 14 Aug 2018, Iain Sandoe wrote:

> 2018-08-14  Iain Sandoe 
> 
> gcc/c:
> 
>   PR c/19315
>   * c-decl.c (finish_decl): Don't add the 'extern' storage class to
>   objects of unknown size.
> 
> gcc/testsuite:
> 
>   PR c/19315
>   gcc.dg/graphite/pr82451.c: Make array 'a' an extern.
>   gcc.dg/redecl-10.c: Expect warnings for the static vars with unknown
>   size.

OK.

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


Re: [PATCH] Deprecate std::get_temporary_buffer

2018-08-14 Thread Ville Voutilainen
On 14 August 2018 at 23:34, Jonathan Wakely  wrote:
> This was deprecated in C++17, and has been removed from the current
> draft. This adds the dprecated attribute for C++17 and later.
>
> We can't actually remove it for C++2a because we use it (indirectly)
> in stl_algo.h. We could rename it to __get_temporary_buffer for our
> internal uses, and then add back get_temporary_buffer as a simple
> forwarding function guarded by:
>
> #if __cplusplus <= 201703L || _GLIBCXX_USE_DEPRECATED
>
> This patch doesn't do that though.
>
> Does anybody object to adding the deprecated attribute here?

Not me.

> Does anybody want to argue in favour of completely removing it for
> C++2a?

Not me.


Re: [C++ Patch] Tweak check_previous_goto_1 to emit hard errors instead of permerrors in some cases

2018-08-14 Thread Nathan Sidwell

On 08/09/2018 08:07 AM, Paolo Carlini wrote:

Hi,

over the years we reworked and improved the code in decl.c checking gotos quite 
a bit. Lately, in some specific unsafe cases, identify_goto issues upfront an 
error instead of a permerror, whereas it used to always issue a permerror. Over 
the last weeks a few colleagues of mine noticed that we don't do that, 
escalating a permerror to a plain error, in a case which is certainly unsafe - 
decl_jump_unsafe returns 2 - thus, if the user passes -fpermissive we end up 
emitting assembly completely missing labels. The straightforward patchlet below 
passes testing on x86_64-linux.


Thanks, Paolo.

/



ok, thanks

--
Nathan Sidwell


[PATCH] Deprecate std::get_temporary_buffer

2018-08-14 Thread Jonathan Wakely

This was deprecated in C++17, and has been removed from the current
draft. This adds the dprecated attribute for C++17 and later.

We can't actually remove it for C++2a because we use it (indirectly)
in stl_algo.h. We could rename it to __get_temporary_buffer for our
internal uses, and then add back get_temporary_buffer as a simple
forwarding function guarded by:

#if __cplusplus <= 201703L || _GLIBCXX_USE_DEPRECATED

This patch doesn't do that though.

Does anybody object to adding the deprecated attribute here?

Does anybody want to argue in favour of completely removing it for
C++2a?


diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
index 182c0ed2ff3..ed2f92d0089 100644
--- a/libstdc++-v3/ChangeLog
+++ b/libstdc++-v3/ChangeLog
@@ -1,5 +1,9 @@
 2018-08-14  Jonathan Wakely  
 
+	* include/bits/stl_tempbuf.h (get_temporary_buffer)
+	(return_temporary_buffer): Add deprecated attribute for C++17.
+	(_Temporary_buffer): Disable -Wdeprecated-declarations warnings.
+
 	PR libstdc++/86954
 	* include/bits/stl_tempbuf.h (return_temporary_buffer): Use
 	non-placement delete.
diff --git a/libstdc++-v3/include/bits/stl_tempbuf.h b/libstdc++-v3/include/bits/stl_tempbuf.h
index 0abd3c12de7..2ccaf3e3bd0 100644
--- a/libstdc++-v3/include/bits/stl_tempbuf.h
+++ b/libstdc++-v3/include/bits/stl_tempbuf.h
@@ -81,6 +81,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
* Provides the nothrow exception guarantee.
*/
   template
+#if __cplusplus >= 201703L
+_GLIBCXX_DEPRECATED
+#endif
 pair<_Tp*, ptrdiff_t>
 get_temporary_buffer(ptrdiff_t __len) _GLIBCXX_NOEXCEPT
 {
@@ -108,11 +111,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
*  Frees the memory pointed to by __p.
*/
   template
+#if __cplusplus >= 201703L
+_GLIBCXX_DEPRECATED
+#endif
 inline void
 return_temporary_buffer(_Tp* __p)
 { ::operator delete(__p); }
 
-
   /**
*  This class is used in two places: stl_algo.h and ext/memory,
*  where it is wrapped as the temporary_buffer class.  See
@@ -162,11 +167,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
*/
   _Temporary_buffer(_ForwardIterator __seed, size_type __original_len);
 
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
   ~_Temporary_buffer()
   {
 	std::_Destroy(_M_buffer, _M_buffer + _M_len);
 	std::return_temporary_buffer(_M_buffer);
   }
+#pragma GCC diagnostic pop
 
 private:
   // Disable copy constructor and assignment operator.
@@ -239,6 +247,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  __ucr(__first, __last, __seed);
 }
 
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
   template
 _Temporary_buffer<_ForwardIterator, _Tp>::
 _Temporary_buffer(_ForwardIterator __seed, size_type __original_len)
@@ -262,6 +272,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  __throw_exception_again;
 	}
 }
+#pragma GCC diagnostic pop
 
 _GLIBCXX_END_NAMESPACE_VERSION
 } // namespace


Re: [PATCH] Use getentropy() for seeding PRNG

2018-08-14 Thread Janne Blomqvist
On Tue, Aug 14, 2018 at 11:18 PM, Rainer Orth 
wrote:

> Hi Janne,
>
> > PING
> >
> > On Fri, Aug 3, 2018 at 5:05 PM, Janne Blomqvist <
> blomqvist.ja...@gmail.com>
> > wrote:
> >
> >> On Fri, Aug 3, 2018 at 4:28 PM, Jakub Jelinek  wrote:
> >>
> >>> On Fri, Aug 03, 2018 at 04:19:03PM +0300, Janne Blomqvist wrote:
> >>> > --- a/libgfortran/intrinsics/random.c
> >>> > +++ b/libgfortran/intrinsics/random.c
> >>> > @@ -309,12 +309,9 @@ getosrandom (void *buf, size_t buflen)
> >>> >for (size_t i = 0; i < buflen / sizeof (unsigned int); i++)
> >>> >  rand_s ([i]);
> >>> >return buflen;
> >>> > +#elif defined(HAVE_GETENTROPY)
> >>> > +  return getentropy (buf, buflen);
> >>> >  #else
> >>>
> >>> I wonder if it wouldn't be better to use getentropy only if it is
> >>> successful
> >>> and fall back to whatever you were doing before.
> >>>
> >>> E.g. on Linux, I believe getentropy in glibc just uses the getrandom
> >>> syscall, which has only been introduced in Linux kernel 3.17.
> >>>
> >>
> >> Yes, that is my understanding as well.
> >>
> >>
> >>> So, if somebody is running glibc 2.25 or later on kernel < 3.17, it
> will
> >>> fail, even though reads from /dev/urandom could work.
> >>>
> >>
> >> Hmm, fair enough. So replace the random.c part of the patch with
> >>
> >> diff --git a/libgfortran/intrinsics/random.c b/libgfortran/intrinsics/
> >> random.c
> >> index 234c5ff95fd..229fa6995c0 100644
> >> --- a/libgfortran/intrinsics/random.c
> >> +++ b/libgfortran/intrinsics/random.c
> >> @@ -310,11 +310,10 @@ getosrandom (void *buf, size_t buflen)
> >>  rand_s ([i]);
> >>return buflen;
> >>  #else
> >> -  /*
> >> - TODO: When glibc adds a wrapper for the getrandom() system call
> >> - on Linux, one could use that.
> >> -
> >> - TODO: One could use getentropy() on OpenBSD.  */
> >> +#ifdef HAVE_GETENTROPY
> >> +  if (getentropy (buf, buflen) == 0)
> >> +return 0;
> >> +#endif
> >>int flags = O_RDONLY;
> >>  #ifdef O_CLOEXEC
> >>flags |= O_CLOEXEC;
> >>
> >>
> >>
> >> Just to be sure, I regtested this slightly modified patch as well. Ok
> for
> >> trunk?
>
> the patch broke Solaris 11.3+ bootstrap:
>
> /vol/gcc/src/hg/trunk/local/libgfortran/intrinsics/random.c: In function
> 'getosrandom':
> /vol/gcc/src/hg/trunk/local/libgfortran/intrinsics/random.c:314:7: error:
> implicit declaration of function 'getentropy'; did you mean 'get_nprocs'?
> [-Werror=implicit-function-declaration]
> 314 |   if (getentropy (buf, buflen) == 0)
> |   ^~
> |   get_nprocs
>
>
> According to the manpage, one needs to include  to get the
> getentropy declaration.
>
> Fixed as follows.  Bootstraps on i386-pc-solaris2.11,
> i386-pc-solaris2.10 (which lacks getentropy), sparc-sun-solaris2.11, and
> x86_64-pc-linux-gnu still running, but all already into make check.
>
> Ok for mainline?
>
> Rainer
>
>
Sorry about that, I wasn't aware that Solaris also has added getentropy.
Patch looks good, Ok for trunk!




-- 
Janne Blomqvist


[PATCH] PR libstdc++/86954 use non-placement operator delete

2018-08-14 Thread Jonathan Wakely

As explained in the PR, there's no reason to call the nothrow delete,
we can just use the normal one.

PR libstdc++/86954
* include/bits/stl_tempbuf.h (return_temporary_buffer): Use
non-placement delete.

Tested x86_64-linux, committed to trunk.


commit dc7ca8956ad18f583feeb55e011e1333f65fd6da
Author: Jonathan Wakely 
Date:   Tue Aug 14 20:52:44 2018 +0100

PR libstdc++/86954 use non-placement operator delete

PR libstdc++/86954
* include/bits/stl_tempbuf.h (return_temporary_buffer): Use
non-placement delete.

diff --git a/libstdc++-v3/include/bits/stl_tempbuf.h 
b/libstdc++-v3/include/bits/stl_tempbuf.h
index 159ee27a5d3..0abd3c12de7 100644
--- a/libstdc++-v3/include/bits/stl_tempbuf.h
+++ b/libstdc++-v3/include/bits/stl_tempbuf.h
@@ -88,10 +88,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__gnu_cxx::__numeric_traits::__max / sizeof(_Tp);
   if (__len > __max)
__len = __max;
-  
-  while (__len > 0) 
+
+  while (__len > 0)
{
- _Tp* __tmp = static_cast<_Tp*>(::operator new(__len * sizeof(_Tp), 
+ _Tp* __tmp = static_cast<_Tp*>(::operator new(__len * sizeof(_Tp),
std::nothrow));
  if (__tmp != 0)
return std::pair<_Tp*, ptrdiff_t>(__tmp, __len);
@@ -110,7 +110,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template
 inline void
 return_temporary_buffer(_Tp* __p)
-{ ::operator delete(__p, std::nothrow); }
+{ ::operator delete(__p); }
 
 
   /**


Re: [PATCH] Use getentropy() for seeding PRNG

2018-08-14 Thread Rainer Orth
Hi Janne,

> PING
>
> On Fri, Aug 3, 2018 at 5:05 PM, Janne Blomqvist 
> wrote:
>
>> On Fri, Aug 3, 2018 at 4:28 PM, Jakub Jelinek  wrote:
>>
>>> On Fri, Aug 03, 2018 at 04:19:03PM +0300, Janne Blomqvist wrote:
>>> > --- a/libgfortran/intrinsics/random.c
>>> > +++ b/libgfortran/intrinsics/random.c
>>> > @@ -309,12 +309,9 @@ getosrandom (void *buf, size_t buflen)
>>> >for (size_t i = 0; i < buflen / sizeof (unsigned int); i++)
>>> >  rand_s ([i]);
>>> >return buflen;
>>> > +#elif defined(HAVE_GETENTROPY)
>>> > +  return getentropy (buf, buflen);
>>> >  #else
>>>
>>> I wonder if it wouldn't be better to use getentropy only if it is
>>> successful
>>> and fall back to whatever you were doing before.
>>>
>>> E.g. on Linux, I believe getentropy in glibc just uses the getrandom
>>> syscall, which has only been introduced in Linux kernel 3.17.
>>>
>>
>> Yes, that is my understanding as well.
>>
>>
>>> So, if somebody is running glibc 2.25 or later on kernel < 3.17, it will
>>> fail, even though reads from /dev/urandom could work.
>>>
>>
>> Hmm, fair enough. So replace the random.c part of the patch with
>>
>> diff --git a/libgfortran/intrinsics/random.c b/libgfortran/intrinsics/
>> random.c
>> index 234c5ff95fd..229fa6995c0 100644
>> --- a/libgfortran/intrinsics/random.c
>> +++ b/libgfortran/intrinsics/random.c
>> @@ -310,11 +310,10 @@ getosrandom (void *buf, size_t buflen)
>>  rand_s ([i]);
>>return buflen;
>>  #else
>> -  /*
>> - TODO: When glibc adds a wrapper for the getrandom() system call
>> - on Linux, one could use that.
>> -
>> - TODO: One could use getentropy() on OpenBSD.  */
>> +#ifdef HAVE_GETENTROPY
>> +  if (getentropy (buf, buflen) == 0)
>> +return 0;
>> +#endif
>>int flags = O_RDONLY;
>>  #ifdef O_CLOEXEC
>>flags |= O_CLOEXEC;
>>
>>
>>
>> Just to be sure, I regtested this slightly modified patch as well. Ok for
>> trunk?

the patch broke Solaris 11.3+ bootstrap:

/vol/gcc/src/hg/trunk/local/libgfortran/intrinsics/random.c: In function 
'getosrandom':
/vol/gcc/src/hg/trunk/local/libgfortran/intrinsics/random.c:314:7: error: 
implicit declaration of function 'getentropy'; did you mean 'get_nprocs'? 
[-Werror=implicit-function-declaration]
314 |   if (getentropy (buf, buflen) == 0)
|   ^~
|   get_nprocs


According to the manpage, one needs to include  to get the
getentropy declaration.

Fixed as follows.  Bootstraps on i386-pc-solaris2.11,
i386-pc-solaris2.10 (which lacks getentropy), sparc-sun-solaris2.11, and
x86_64-pc-linux-gnu still running, but all already into make check.

Ok for mainline?

Rainer

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


2018-08-14  Rainer Orth  

* configure.ac: Check for .
* configure, config.h.in: Regenerate.
* intrinsics/random.c [HAVE_SYS_RANDOM_H]: Include .

# HG changeset patch
# Parent  8cc4b3b19e87eb3221d688c25887efd199d44f89
Include  for getentropy on Solaris

diff --git a/libgfortran/configure.ac b/libgfortran/configure.ac
--- a/libgfortran/configure.ac
+++ b/libgfortran/configure.ac
@@ -275,8 +275,9 @@ AC_TYPE_UINTPTR_T
 AC_CHECK_TYPES([ptrdiff_t])
 
 # check header files (we assume C89 is available, so don't check for that)
-AC_CHECK_HEADERS_ONCE(unistd.h sys/time.h sys/times.h sys/resource.h \
-sys/types.h sys/stat.h sys/wait.h floatingpoint.h ieeefp.h fenv.h fptrap.h \
+AC_CHECK_HEADERS_ONCE(unistd.h sys/random.h sys/time.h sys/times.h \
+sys/resource.h sys/types.h sys/stat.h sys/wait.h \
+floatingpoint.h ieeefp.h fenv.h fptrap.h \
 fpxcp.h pwd.h complex.h xlocale.h)
 
 GCC_HEADER_STDINT(gstdint.h)
diff --git a/libgfortran/intrinsics/random.c b/libgfortran/intrinsics/random.c
--- a/libgfortran/intrinsics/random.c
+++ b/libgfortran/intrinsics/random.c
@@ -37,6 +37,9 @@ see the files COPYING3 and COPYING.RUNTI
 #include 
 #include 
 #include "time_1.h"
+#ifdef HAVE_SYS_RANDOM_H
+#include 
+#endif
 
 #ifdef __MINGW32__
 #define HAVE_GETPID 1


Re: [PATCH] Fix P81033 for FDEs in partitioned code.

2018-08-14 Thread Mike Stump
On Aug 14, 2018, at 4:20 AM, Iain Sandoe  wrote:
> When function sub-sections are enabled, Darwin’s assembler needs the FDE 
> local start
> label for each sub-section to follow a linker-visible one so that the FDE 
> will be correctly
> associated with the code of the subsection.
> 
> The current code in final.c emits a linker-visible symbol, as needed by 
> several targets.
> However the local label used to define the FDE start precedes the 
> linker-visible one
> which, for Darwin causes it (the FDE start) to be associated with the 
> previous linker-
> visible symbol (or the section start if there isn’t one).  This applies 
> regardless of the 
> actual address of the label, for toolchain assemblers that have strict 
> interpretation of
> the Darwin sub-sections-via-symbols ABI.
> 
> The patch adds a new local label (analogous to the "LFBn” emitted for the 
> regular
> function starts) just after the linker-visible label emitted after switching 
> text section.
> The FDE second entry is made to point to this instead of the LcoldStartn one. 
>  This
> should be a no-op for targets using .cfi_ and for targets without 
> sub-sections-via-symbols.
> 
> Bootstrapped on x86_64 and i686 linux and on a number of Darwin platforms 
> (from 
> i686-darwin9 to x86_64-darwin17).
> 
> OK for trunk?
> open branches? (although it's a regression on 8, it’s a latent wrong-code on 
> all branches)

I'm fine with the darwin aspects of it, but I think it needs review/approval by 
final.c/dwarf people.

Re: [PATCH, Darwin] Remove unnecessary target hook.

2018-08-14 Thread Mike Stump
On Aug 13, 2018, at 1:55 PM, Iain Sandoe  wrote:
> For Darwin when we switch between text sections a linker-visible symbol is
> required to preserve the linker’s  “atom model”.  Some time ago we implemented
> TARGET_ASM_FUNCTION_SWITCHED_TEXT_SECTIONS to provide this.
> 
> A suitable symbol is now emitted directly from final.c so the target hook 
> version
> needs to be removed to avoid multiple symbols at the same address.
> 
> OK for trunk?
> open branches?

Ok.

Re: [PATCH, Darwin, Obvious] Fix PR81685, by adding DWARF 5 section names.

2018-08-14 Thread Mike Stump
On Aug 14, 2018, at 4:38 AM, Iain Sandoe  wrote:
> Where possible (i.e. they are currently defined), I’ve synced the Darwin 
> names with the ones
> emitted by clang.

Thanks.

Re: Improve safe iterator move semantic

2018-08-14 Thread François Dumont

On 10/08/2018 13:26, Ville Voutilainen wrote:

On 10 August 2018 at 13:47, Jonathan Wakely  wrote:

Doing a test like this with TSan should be the absolute minimum
required for any change to the mutex locking policy.

Agreed. Concurrency code is something that our test suite is not
well-equipped to test (because
it doesn't support TSan and such yet), so it would seem prudent to
stress-test such patches
via testsuite-external means.


Yes, sorry about that, I am over confident in the testsuite indeed.

And I also totally forgot this use case of 2 threads playing with 
iterators on the same container. So I didn't even try to find out if any 
test could be good to simulate it.


Now, considering this episode, I am incline to just delete safe iterator 
move semantic. I might propose a patch to do so later.





I'm not aware of people complaining about the performance of debug
mode anyway. Everybody I speak to is happy to accept a performance hit
in order to get checking.
Good to know, I just hope you will accept one more patch regarding 
another performance hint of debug mode when using deque::iterator.

Yep; while it's nice to have performance improvements in debug mode,
there are probably more
important and significant ways to improve it..


I think https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86843 would be
more helpful to our users, as it would allow some Debug Mode checks to
be enabled in programs that can't currently use it (because
recompiling the entire program is not possible).

..like this one, which would be a major usability improvement.


I'll consider this one, but I fear not for gcc 9.0.

François



Re: [PATCH] Backport of RISC-V support for libffi go closures

2018-08-14 Thread Jim Wilson

On 08/14/2018 02:00 AM, Andreas Schwab wrote:

Backport of RISC-V support for libffi go closures
* src/riscv/ffi.c (ffi_call_go, ffi_prep_go_closure): New
functions.
(ffi_call_int): Renamed from ffi_call.
(ffi_call_asm, ffi_closure_inner): Adjust interface.
* src/riscv/ffitarget.h (FFI_GO_CLOSURES): Define.
* src/riscv/sysv.S (ffi_go_closure_asm): New function.
(ffi_closure_asm, ffi_call_asm): Update for adjusted interfaces.


OK.

Jim


Re: [PATCH] lra: fix FPE when dumping

2018-08-14 Thread Vladimir Makarov

On 08/14/2018 03:42 AM, Ilya Leoshkevich wrote:

The following S/390 code

 struct {} b;
 void c() {
   __asm__("la 0,%0\n"
   "la 1,%1\n"
   :
   : "m" (b), "m" (b)
   : "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7", "r8",
 "r9", "r10", "r12", "r14");
 }

results in

 internal compiler error: Floating point exception

when building with

 -fira-verbose=9 -fdump-rtl-all

gcc/ChangeLog:

2018-07-25  Ilya Leoshkevich  

 PR target/86547
* lra-lives.c (remove_some_program_points_and_update_live_ranges):
 Check whether lra_live_max_point is 0 before dividing.

Sure, the patch is ok for me and for committing it into the trunk.

Thank you, Ilya.


Re: [Patch, Fortran] PR 86116: Ambiguous generic interface not recognised

2018-08-14 Thread Janus Weil
Am Di., 14. Aug. 2018 um 16:16 Uhr schrieb Fritz Reese :
>
> Looks OK to me.

Thanks, Fritz. Committed as r263540.

Since this PR is a regression, it should probably be backported to the
release branches as well. However, I'll a wait a week or two with
that, in order to check for possible problems on trunk ...

Cheers,
Janus



> On Tue, Aug 14, 2018 at 4:12 AM Janus Weil  wrote:
> >
> > ping!
> >
> >
> > Am So., 5. Aug. 2018 um 15:23 Uhr schrieb Janus Weil :
> > >
> > > Hi all,
> > >
> > > the attached patch fixes PR 86116 by splitting up the function
> > > 'compare_type' into two variants: One that is used for checking
> > > generic interfaces and operators (keeping the old name), and one that
> > > is used for checking dummy functions and procedure pointer assignments
> > > ('compare_type_characteristics'). The latter calls the former, but
> > > includes an additional check that must not be done when checking
> > > generics.
> > >
> > > Regtests cleanly on x86_64-linux-gnu. Ok for trunk?
> > >
> > > Cheers,
> > > Janus
> > >
> > >
> > > 2018-08-05  Janus Weil  
> > >
> > > PR fortran/86116
> > > * interface.c (compare_type): Remove a CLASS/TYPE check.
> > > (compare_type_characteristics): New function that behaves like the old
> > > 'compare_type'.
> > > (gfc_check_dummy_characteristics, gfc_check_result_characteristics):
> > > Call 'compare_type_characteristics' instead of 'compare_type'.
> > >
> > > 2018-08-05  Janus Weil  
> > >
> > > PR fortran/86116
> > > * gfortran.dg/generic_34.f90: New test case.


[PATCH] avoid MEM_REF when looking for poor-man's flexible arrays (PR 86914)

2018-08-14 Thread Martin Sebor

Attached is a patch to avoid calling array_at_struct_end_p()
with a MEM_REF argument.  The function returns false even if
the reference does point into such a flexible array member,
as in:

  struct A { char i, a[1]; };
  void f (struct A *p)
  {
return strlen (p->a + 1);
  }

This fix will likely be made obsolete once the string length
range optimization is relaxed but since that's a bigger change
I think GCC might as well emit the correct code for this case
until then.

Martin
PR tree-optimization/86914 - wrong code with strlen() of poor-man's flexible array member plus offset

gcc/ChangeLog:

	PR tree-optimization/86914
	* tree-ssa-strlen.c (maybe_set_strlen_range): Avoid MEM_REF.

gcc/testsuite/ChangeLog:

	PR tree-optimization/86914
	* gcc.dg/strlenopt-57.c: New test.

Index: gcc/testsuite/gcc.dg/strlenopt-57.c
===
--- gcc/testsuite/gcc.dg/strlenopt-57.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/strlenopt-57.c	(working copy)
@@ -0,0 +1,49 @@
+/* PR tree-optimization/86914 - wrong code with strlen() of poor-man's
+   flexible array member plus offset
+   { dg-do compile }
+   { dg-options "-O2 -Wall -fdump-tree-optimized" } */
+
+#include "strlenopt.h"
+
+struct A0 { char i, a[0]; };
+struct A1 { char i, a[1]; };
+struct A9 { char i, a[9]; };
+struct Ax { char i, a[]; };
+
+extern int a[];
+
+extern struct A0 a0;
+extern struct A1 a1;
+extern struct A9 a9;
+extern struct Ax ax;
+
+void test_var_flexarray_cst_off (void)
+{
+  /* Use arbitrary constants greater than 16 in case GCC ever starts
+ unrolling strlen() calls with small array arguments.  */
+  a[0] = 17 < strlen (a0.a + 1);
+  a[1] = 19 < strlen (a1.a + 1);
+  a[2] = 23 < strlen (a9.a + 9);
+  a[3] = 29 < strlen (ax.a + 3);
+}
+
+void test_ptr_flexarray_cst_off (struct A0 *p0, struct A1 *p1,
+ struct A9 *p9, struct Ax *px)
+{
+  a[0] = 17 < strlen (p0->a + 1);
+  a[1] = 19 < strlen (p1->a + 1);
+  a[2] = 23 < strlen (p9->a + 9);
+  a[3] = 29 < strlen (px->a + 3);
+}
+
+void test_ptr_flexarray_var_off (struct A0 *p0, struct A1 *p1,
+ struct A9 *p9, struct Ax *px,
+ int i)
+{
+  a[0] = 17 < strlen (p0->a + i);
+  a[1] = 19 < strlen (p1->a + i);
+  a[2] = 23 < strlen (p9->a + i);
+  a[3] = 29 < strlen (px->a + i);
+}
+
+/* { dg-final { scan-tree-dump-times "strlen" 12 "optimized" } } */
Index: gcc/tree-ssa-strlen.c
===
--- gcc/tree-ssa-strlen.c	(revision 263537)
+++ gcc/tree-ssa-strlen.c	(working copy)
@@ -1153,7 +1153,9 @@ maybe_set_strlen_range (tree lhs, tree src, tree b
 	 suggests if it's treated as a poor-man's flexible array member.  */
   src = TREE_OPERAND (src, 0);
   bool src_is_array = TREE_CODE (TREE_TYPE (src)) == ARRAY_TYPE;
-  if (src_is_array && !array_at_struct_end_p (src))
+  if (src_is_array
+	  && TREE_CODE (src) != MEM_REF
+	  && !array_at_struct_end_p (src))
 	{
 	  tree type = TREE_TYPE (src);
 	  if (tree size = TYPE_SIZE_UNIT (type))


Re: [RFC][PATCH][mid-end] Optimize immediate choice in comparisons.

2018-08-14 Thread Vlad Lazar

On 13/08/18 15:00, Richard Sandiford wrote:

Vlad Lazar  writes:

diff --git a/gcc/expmed.h b/gcc/expmed.h
index 
2890d9c9bbd034f01030dd551d544bf73e73b784..86a32a643fdd0fc9f396bd2c7904244bd484df16
 100644
--- a/gcc/expmed.h
+++ b/gcc/expmed.h
@@ -702,6 +702,10 @@ extern rtx emit_store_flag (rtx, enum rtx_code, rtx, rtx, 
machine_mode,
   extern rtx emit_store_flag_force (rtx, enum rtx_code, rtx, rtx,
  machine_mode, int, int);
   
+extern void canonicalize_comparison (machine_mode, enum rtx_code *, rtx *);

+
+extern enum rtx_code canonicalized_cmp_code (enum rtx_code);


It would probably be better to make the second function static (local
to expmed.c), since it's only used internally by canonicalize_comparison.
Switching the order of the two functions in expmed.c would avoid the need
for a forward declaration.


@@ -6153,6 +6156,97 @@ emit_store_flag_force (rtx target, enum rtx_code code, 
rtx op0, rtx op1,
   
 return target;

   }
+
+/* Choose the more appropiate immediate in comparisons.  The purpose of this


Maybe worth saying "scalar integer comparisons", since the function
doesn't handle vector or floating-point comparisons.


+   is to end up with an immediate which can be loaded into a register in fewer
+   moves, if possible.
+
+   For each integer comparison there exists an equivalent choice:
+ i)   a >  b or a >= b + 1
+ ii)  a <= b or a <  b + 1
+ iii) a >= b or a >  b - 1
+ iv)  a <  b or a <= b - 1
+
+   The function is called in the gimple expanders which handle GIMPLE_ASSIGN
+   and GIMPLE_COND.  It assumes that the first operand of the comparison is a
+   register and the second is a constant.


This last paragraph doesn't seem accurate any more.  Probably best to
drop it, since there's no real need to say who the callers are if we
describe the interface.


+   mode is the mode of the first operand.
+   code points to the comparison code.
+   imm points to the rtx containing the immediate.  */


GCC's convention is to put parameter names in caps in comments,
so "MODE is...", etc.

On the IMM line, it might be worth adding "*IMM must satisfy
CONST_SCALAR_INT_P on entry and continues to satisfy CONST_SCALAR_INT_P
on exit."


+void canonicalize_comparison (machine_mode mode, enum rtx_code *code, rtx *imm)
+{
+  int to_add = 0;
+
+  if (!SCALAR_INT_MODE_P (mode))
+return;
+
+  /* Extract the immediate value from the rtx.  */
+  wide_int imm_val = wi::shwi (INTVAL (*imm), mode);


This should be:

   rtx_mode_t imm_val (*imm, mode);

so that it copes with all CONST_SCALAR_INT_P, not just CONST_INT.


+  if (*code == GT || *code == GTU || *code == LE || *code == LEU)
+to_add = 1;
+
+  if (*code == GE || *code == GEU || *code == LT || *code == LTU)
+to_add = -1;


Might be better to have:

   if (*code == GT || *code == GTU || *code == LE || *code == LEU)
 to_add = 1;
   else if (*code == GE || *code == GEU || *code == LT || *code == LTU)
 to_add = -1;
   else
 return;

so that we exit early for other comparisons, rather than doing wasted work.


+  /* Check for overflow/underflow in the case of signed values and
+ wrapping around in the case of unsigned values.  If any occur
+ cancel the optimization.  */
+  wide_int max_val = wi::max_value (mode, SIGNED);
+  wide_int min_val = wi::min_value (mode, SIGNED);
+  if ((wi::cmps (imm_val, max_val) == 0 && to_add == 1)
+  || (wi::cmps (imm_val, min_val) == 0 && to_add == -1))
+return;


This needs to use the SIGNED/UNSIGNED choice appropriate for the
comparison (SIGNED for GT, UNSIGNED for GTU, etc.).  There doesn't
seem to be an existing function that says whether an rtx comparison
operation is signed or not (bit of a surprise), but there is
unsigned_condition, which returns the unsigned form of an rtx
comparison operator.  It might be worth adding something like:

   /* Return true if integer comparison operator CODE interprets its operands
  as unsigned.  */

   inline bool
   unsigned_condition_p (rtx_code code)
   {
 return unsigned_condition (code) == code;
   }

and using that to choose between SIGNED and UNSIGNED.

Rather than using wi::cmp, a more direct way of checking for overflow
is to change this:


+  /* Generate a mov instruction for both cases and see whether the change
+ was profitable.  */
+  wide_int imm_modif = wi::add (imm_val, to_add);


to use the overflow form of wi::add, i.e.:

   wide_int imm_modif = wi::add (imm_val, to_add, sign, );

and return if "overflow" is set.


+
+  rtx reg = gen_reg_rtx (mode);


gen_reg_rtx creates a new pseudo register that essentially stays
around until we've finished compiling the function.  It's better to use:

   gen_rtx_REG (mode, LAST_VIRTUAL_REGISTER + 1);

for cost calculations, so that we don't waste pseudo register numbers.


+  rtx new_imm = GEN_INT (imm_modif.to_shwi ());


This should be:

   rtx new_imm = immed_wide_int_const (imm_modif, mode);

to cope with non-CONST_INT 

fix reassoc cut

2018-08-14 Thread Alexandre Oliva
I guess this was a cut  I caught it during code review.  It
doesn't seem to matter much: I haven't been able to get any codegen
difference from this change, not in a bootstrap, not in test runs.  I
even put a gcc_unreachable() in case the test on rhs2 were to return
false (which would never happen after the test on rhs1 passed, when rhs1
and rhs2 were the same), and still no hits in stage3.  Anyway...

Regstrapped on x86_64-linux-gnu.  Ok to install?

From: Alexandre Oliva 

for  gcc/ChangeLog

* tree-ssa-reassoc.c (is_reassociable_op): Fix cut
---
 gcc/tree-ssa-reassoc.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/tree-ssa-reassoc.c b/gcc/tree-ssa-reassoc.c
index 41a37ab29899..6b0bf5c03548 100644
--- a/gcc/tree-ssa-reassoc.c
+++ b/gcc/tree-ssa-reassoc.c
@@ -618,7 +618,7 @@ is_reassociable_op (gimple *stmt, enum tree_code code, 
struct loop *loop)
   && has_single_use (gimple_assign_lhs (stmt)))
 {
   tree rhs1 = gimple_assign_rhs1 (stmt);
-  tree rhs2 = gimple_assign_rhs1 (stmt);
+  tree rhs2 = gimple_assign_rhs2 (stmt);
   if (TREE_CODE (rhs1) == SSA_NAME
  && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (rhs1))
return false;


-- 
Alexandre Oliva, freedom fighter   https://FSFLA.org/blogs/lxo
Be the change, be Free! FSF Latin America board member
GNU Toolchain EngineerFree Software Evangelist


Re: [PATCH] Use more DECL_BUILT_IN_P macro.

2018-08-14 Thread Martin Sebor

On 08/14/2018 03:06 AM, Martin Liška wrote:

Hi.

The patch adds more usages of the new macro. I hope it improves
readability of code.


I think it does :)  I see that most invocations of it in your
patch are with BUILT_IN_NORMAL as the second argument.  Is
the argument implied by the last argument?  E.g., in

  DECL_BUILT_IN_P (fndecl, BUILT_IN_NORMAL, BUILT_IN_VA_ARG_PACK)

is there a way to determine that BUILT_IN_VA_ARG_PACK implies
DECL_BUILT_IN_CLASS(fndecl), so that the second argument could
be eliminated?  (Both to make the invocation less verbose and
to avoid the mistake of using the macro with the wrong class.)

If not, adding DECL_NORMAL_BUILT_IN_P() would make it possible
to omit it.

Martin



Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

Ready to be installed?
Martin

gcc/ChangeLog:

2018-08-10  Martin Liska  

* tree.h (DECL_BUILT_IN_P): Add also check
for TREE_CODE (NODE) == FUNCTION_DECL.
* builtins.c (fold_call_expr): Use DECL_BUILT_IN_P macro.
(fold_builtin_call_array): Likewise.
* cgraph.c (cgraph_update_edges_for_call_stmt_node): Likewise.
(cgraph_edge::verify_corresponds_to_fndecl): Likewise.
(cgraph_node::verify_node): Likewise.
* cgraphclones.c (cgraph_node::create_clone): Likewise.
* dse.c (scan_insn): Likewise.
* fold-const.c (fold_binary_loc): Likewise.
* gimple-pretty-print.c (dump_gimple_call): Likewise.
* gimple.c (gimple_call_builtin_p): Likewise.
* gimplify.c (gimplify_call_expr): Likewise.
(gimple_boolify): Likewise.
(gimplify_modify_expr): Likewise.
* ipa-fnsummary.c (compute_fn_summary): Likewise.
* omp-low.c (setjmp_or_longjmp_p): Likewise.
* trans-mem.c (is_tm_irrevocable): Likewise.
(is_tm_abort): Likewise.
* tree-cfg.c (stmt_can_terminate_bb_p): Likewise.
* tree-inline.c (copy_bb): Likewise.
* tree-sra.c (scan_function): Likewise.
* tree-ssa-ccp.c (optimize_stack_restore): Likewise.
(pass_fold_builtins::execute): Likewise.
* tree-ssa-dom.c (dom_opt_dom_walker::optimize_stmt): Likewise.
* tree-ssa-loop-im.c (stmt_cost): Likewise.
* tree-stdarg.c (optimize_va_list_gpr_fpr_size): Likewise.

gcc/c/ChangeLog:

2018-08-10  Martin Liska  

* c-parser.c (c_parser_postfix_expression_after_primary):
Use DECL_BUILT_IN_P macro.

gcc/cp/ChangeLog:

2018-08-10  Martin Liska  

* constexpr.c (constexpr_fn_retval): Use DECL_BUILT_IN_P macro.
(cxx_eval_builtin_function_call): Likewise.
* cp-gimplify.c (cp_gimplify_expr): Likewise.
(cp_fold): Likewise.
* semantics.c (finish_call_expr): Likewise.
* tree.c (builtin_valid_in_constant_expr_p): Likewise.
---
 gcc/builtins.c| 10 ++
 gcc/c/c-parser.c  |  9 +++--
 gcc/cgraph.c  | 13 ++---
 gcc/cgraphclones.c|  4 ++--
 gcc/cp/constexpr.c| 12 +---
 gcc/cp/cp-gimplify.c  | 12 
 gcc/cp/semantics.c|  4 +---
 gcc/cp/tree.c |  5 ++---
 gcc/dse.c |  5 ++---
 gcc/fold-const.c  |  4 +---
 gcc/gimple-pretty-print.c |  4 +---
 gcc/gimple.c  |  3 +--
 gcc/gimplify.c| 14 --
 gcc/ipa-fnsummary.c   |  8 
 gcc/omp-low.c |  5 ++---
 gcc/trans-mem.c   |  8 ++--
 gcc/tree-cfg.c|  3 +--
 gcc/tree-inline.c |  4 ++--
 gcc/tree-sra.c|  4 ++--
 gcc/tree-ssa-ccp.c|  8 ++--
 gcc/tree-ssa-dom.c|  4 +---
 gcc/tree-ssa-loop-im.c|  4 +---
 gcc/tree-stdarg.c |  6 ++
 gcc/tree.h|  4 +++-
 24 files changed, 56 insertions(+), 101 deletions(-)






Re: [PATCH] convert braced initializers to strings (PR 71625)

2018-08-14 Thread Martin Sebor

On 08/14/2018 09:08 AM, Martin Sebor wrote:

On 08/14/2018 07:27 AM, James Greenhalgh wrote:

On Wed, Aug 08, 2018 at 07:17:07PM -0500, Martin Sebor wrote:

On 08/08/2018 05:08 AM, Jason Merrill wrote:

On Wed, Aug 8, 2018 at 9:04 AM, Martin Sebor  wrote:

On 08/07/2018 02:57 AM, Jason Merrill wrote:


On Wed, Aug 1, 2018 at 12:49 AM, Martin Sebor 
wrote:


On 07/31/2018 07:38 AM, Jason Merrill wrote:





Done in the attached patch.  I've also avoided dealing with
zero-length arrays and added tests to make sure their size
stays is regardless of the form of their initializer and
the appropriate warnings are issued.

Using build_string() rather than build_string_literal() needed
a tweak in digest_init_r().  It didn't break anything but since
the array type may not have a domain yet, neither will the
string.  It looks like that may get adjusted later on but I've
temporarily guarded the code with #if 1.  If the change is
fine I'll remove the #if before committing.

This initial patch only handles narrow character initializers
(i.e., those with TYPE_STRING_FLAG set).  Once this gets some
exposure I'd like to extend it to other character types,
including wchar_t.


Hi Martin,

This causes issues for the AArch64 tests (full list below).

I see an error message on the following construct:

  void foo (void)
  {
__Poly8_t x[4] = { 1, 2, 3, 4 };
  }

  init.c:3:20: error: array of inappropriate type initialized from
string constant
  3 |   __Poly8_t x[4] = { 1, 2, 3, 4 };
|

__Poly8_t is a type we define in our backend, through a convoluted set of
functions, which operates a lot like an unsigned, QI mode type.


I see the error with my aarch64 cross-compiler .  The new code
that does the conversion of array initializers to STRING_CSTs
looks for the TYPE_STRING_FLAG() to be set on the type of
the array elements.  Perhaps __Poly8_t should not have the flag
set?  (If it needs it then I think we'd have to only consider
named character types.)


The change below gets rid of the compilation error.  I don't
know if it's appropriate for the aarch64 back end:

Index: gcc/config/aarch64/aarch64-builtins.c
===
--- gcc/config/aarch64/aarch64-builtins.c   (revision 263537)
+++ gcc/config/aarch64/aarch64-builtins.c   (working copy)
@@ -643,6 +643,7 @@ aarch64_init_simd_builtin_types (void)
   /* Poly types are a world of their own.  */
   aarch64_simd_types[Poly8_t].eltype = aarch64_simd_types[Poly8_t].itype =
 build_distinct_type_copy (unsigned_intQI_type_node);
+  TYPE_STRING_FLAG (aarch64_simd_types[Poly8_t].eltype) = false;
   aarch64_simd_types[Poly16_t].eltype = 
aarch64_simd_types[Poly16_t].itype =

 build_distinct_type_copy (unsigned_intHI_type_node);
   aarch64_simd_types[Poly64_t].eltype = 
aarch64_simd_types[Poly64_t].itype =




A second set of tests fail due to changed inlining behaviour for
functions
with char array initialization:

  gcc.target/aarch64/vset_lane_1.c
  gcc.target/aarch64/vneg_s.c
  gcc.target/aarch64/vclz.c


I'm not sure what's going on here.  The tests are very big and
take forever to compile with an aarch64 cross-compiler, and I'm
not sure what to look for.  Can you provide a smaller test case
that shows the issue?

Martin



Thanks,
James

-

New failures in:


gcc.target/aarch64/advsimd-intrinsics/vmax.c
gcc.target/aarch64/simd/vzipqp8_1.c
gcc.target/aarch64/vldN_dup_1.c
gcc.target/aarch64/advsimd-intrinsics/vcle.c
gcc.target/aarch64/advsimd-intrinsics/vadd.c
gcc.target/aarch64/advsimd-intrinsics/vhadd.c
gcc.target/aarch64/advsimd-intrinsics/vmull_n.c
gcc.target/aarch64/advsimd-intrinsics/vrndn.c
gcc.target/aarch64/simd/vtrnqp8_1.c
gcc.target/aarch64/advsimd-intrinsics/vpadal.c
gcc.target/aarch64/advsimd-intrinsics/vtrn_half.c
gcc.target/aarch64/advsimd-intrinsics/vqdmlal_n.c
gcc.target/aarch64/advsimd-intrinsics/vqdmulh.c
gcc.target/aarch64/advsimd-intrinsics/vqsub.c
gcc.target/aarch64/advsimd-intrinsics/vqdmlal_lane.c
gcc.target/aarch64/advsimd-intrinsics/vqdmlsl_n.c
gcc.target/aarch64/advsimd-intrinsics/vuzp_half.c
gcc.target/aarch64/advsimd-intrinsics/vst1_lane.c
gcc.target/aarch64/advsimd-intrinsics/vmla_lane.c
gcc.target/aarch64/advsimd-intrinsics/vqdmulh_lane.c
gcc.target/aarch64/advsimd-intrinsics/vrsqrte.c
gcc.target/aarch64/advsimd-intrinsics/vneg.c
gcc.target/aarch64/simd/vuzpqp8_1.c
gcc.target/aarch64/advsimd-intrinsics/vcale.c
gcc.target/aarch64/advsimd-intrinsics/vmla_n.c
gcc.target/aarch64/advsimd-intrinsics/vsub.c
gcc.target/aarch64/advsimd-intrinsics/vrev.c
gcc.target/aarch64/advsimd-intrinsics/vmul.c
gcc.target/aarch64/advsimd-intrinsics/vldX.c
gcc.target/aarch64/advsimd-intrinsics/vsubl.c
gcc.target/aarch64/advsimd-intrinsics/vfms.c
gcc.target/aarch64/advsimd-intrinsics/vmlsl.c
gcc.target/aarch64/advsimd-intrinsics/vsli_n.c
gcc.target/aarch64/advsimd-intrinsics/vcombine.c
gcc.target/aarch64/advsimd-intrinsics/vmul_n.c
gcc.target/aarch64/advsimd-intrinsics/vldX_dup.c

Re: [PATCH] Fix build with ISL 0.20

2018-08-14 Thread Matthias Klose
On 01.08.2018 09:13, Richard Biener wrote:
> 
> The following fixes build with ISL 0.20, tested by building with
> ISL 0.20 and 0.15 (the oldest supported ISL).
> 
> Applied to trunk, will commit to the branches as well.

that was committed to the 7 and 8 branches, but not the 6 branch. Now done as 
well.

Matthias

> 
> Richard.
> 
> 2018-08-01  Richard Biener  
> 
>   PR bootstrap/86724
>   * graphite.h: Include isl/id.h and isl/space.h to allow build
>   with ISL 0.20.
> 
> Index: gcc/graphite.h
> ===
> --- gcc/graphite.h(revision 263190)
> +++ gcc/graphite.h(working copy)
> @@ -37,6 +37,8 @@ along with GCC; see the file COPYING3.
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  
>  typedef struct poly_dr *poly_dr_p;
>  
> 



Re: [PATCH] convert braced initializers to strings (PR 71625)

2018-08-14 Thread Martin Sebor

On 08/14/2018 07:27 AM, James Greenhalgh wrote:

On Wed, Aug 08, 2018 at 07:17:07PM -0500, Martin Sebor wrote:

On 08/08/2018 05:08 AM, Jason Merrill wrote:

On Wed, Aug 8, 2018 at 9:04 AM, Martin Sebor  wrote:

On 08/07/2018 02:57 AM, Jason Merrill wrote:


On Wed, Aug 1, 2018 at 12:49 AM, Martin Sebor  wrote:


On 07/31/2018 07:38 AM, Jason Merrill wrote:





Done in the attached patch.  I've also avoided dealing with
zero-length arrays and added tests to make sure their size
stays is regardless of the form of their initializer and
the appropriate warnings are issued.

Using build_string() rather than build_string_literal() needed
a tweak in digest_init_r().  It didn't break anything but since
the array type may not have a domain yet, neither will the
string.  It looks like that may get adjusted later on but I've
temporarily guarded the code with #if 1.  If the change is
fine I'll remove the #if before committing.

This initial patch only handles narrow character initializers
(i.e., those with TYPE_STRING_FLAG set).  Once this gets some
exposure I'd like to extend it to other character types,
including wchar_t.


Hi Martin,

This causes issues for the AArch64 tests (full list below).

I see an error message on the following construct:

  void foo (void)
  {
__Poly8_t x[4] = { 1, 2, 3, 4 };
  }

  init.c:3:20: error: array of inappropriate type initialized from string 
constant
  3 |   __Poly8_t x[4] = { 1, 2, 3, 4 };
|

__Poly8_t is a type we define in our backend, through a convoluted set of
functions, which operates a lot like an unsigned, QI mode type.


I see the error with my aarch64 cross-compiler .  The new code
that does the conversion of array initializers to STRING_CSTs
looks for the TYPE_STRING_FLAG() to be set on the type of
the array elements.  Perhaps __Poly8_t should not have the flag
set?  (If it needs it then I think we'd have to only consider
named character types.)


A second set of tests fail due to changed inlining behaviour for functions
with char array initialization:

  gcc.target/aarch64/vset_lane_1.c
  gcc.target/aarch64/vneg_s.c
  gcc.target/aarch64/vclz.c


I'm not sure what's going on here.  The tests are very big and
take forever to compile with an aarch64 cross-compiler, and I'm
not sure what to look for.  Can you provide a smaller test case
that shows the issue?

Martin



Thanks,
James

-

New failures in:


gcc.target/aarch64/advsimd-intrinsics/vmax.c
gcc.target/aarch64/simd/vzipqp8_1.c
gcc.target/aarch64/vldN_dup_1.c
gcc.target/aarch64/advsimd-intrinsics/vcle.c
gcc.target/aarch64/advsimd-intrinsics/vadd.c
gcc.target/aarch64/advsimd-intrinsics/vhadd.c
gcc.target/aarch64/advsimd-intrinsics/vmull_n.c
gcc.target/aarch64/advsimd-intrinsics/vrndn.c
gcc.target/aarch64/simd/vtrnqp8_1.c
gcc.target/aarch64/advsimd-intrinsics/vpadal.c
gcc.target/aarch64/advsimd-intrinsics/vtrn_half.c
gcc.target/aarch64/advsimd-intrinsics/vqdmlal_n.c
gcc.target/aarch64/advsimd-intrinsics/vqdmulh.c
gcc.target/aarch64/advsimd-intrinsics/vqsub.c
gcc.target/aarch64/advsimd-intrinsics/vqdmlal_lane.c
gcc.target/aarch64/advsimd-intrinsics/vqdmlsl_n.c
gcc.target/aarch64/advsimd-intrinsics/vuzp_half.c
gcc.target/aarch64/advsimd-intrinsics/vst1_lane.c
gcc.target/aarch64/advsimd-intrinsics/vmla_lane.c
gcc.target/aarch64/advsimd-intrinsics/vqdmulh_lane.c
gcc.target/aarch64/advsimd-intrinsics/vrsqrte.c
gcc.target/aarch64/advsimd-intrinsics/vneg.c
gcc.target/aarch64/simd/vuzpqp8_1.c
gcc.target/aarch64/advsimd-intrinsics/vcale.c
gcc.target/aarch64/advsimd-intrinsics/vmla_n.c
gcc.target/aarch64/advsimd-intrinsics/vsub.c
gcc.target/aarch64/advsimd-intrinsics/vrev.c
gcc.target/aarch64/advsimd-intrinsics/vmul.c
gcc.target/aarch64/advsimd-intrinsics/vldX.c
gcc.target/aarch64/advsimd-intrinsics/vsubl.c
gcc.target/aarch64/advsimd-intrinsics/vfms.c
gcc.target/aarch64/advsimd-intrinsics/vmlsl.c
gcc.target/aarch64/advsimd-intrinsics/vsli_n.c
gcc.target/aarch64/advsimd-intrinsics/vcombine.c
gcc.target/aarch64/advsimd-intrinsics/vmul_n.c
gcc.target/aarch64/advsimd-intrinsics/vldX_dup.c
gcc.target/aarch64/advsimd-intrinsics/vpaddl.c
gcc.target/aarch64/advsimd-intrinsics/vqshrn_n.c
gcc.target/aarch64/advsimd-intrinsics/vstX_lane.c
gcc.target/aarch64/advsimd-intrinsics/vqtbX.c
gcc.target/aarch64/advsimd-intrinsics/vext.c
gcc.target/aarch64/advsimd-intrinsics/vtrn.c
gcc.target/aarch64/advsimd-intrinsics/vtst.c
gcc.target/aarch64/advsimd-intrinsics/vbic.c
gcc.target/aarch64/advsimd-intrinsics/vqdmlsl.c
gcc.target/aarch64/advsimd-intrinsics/vqshl.c
gcc.target/aarch64/advsimd-intrinsics/vrsqrts.c
gcc.target/aarch64/advsimd-intrinsics/vqdmull_n.c
gcc.target/aarch64/advsimd-intrinsics/vqdmlsl_lane.c
gcc.target/aarch64/advsimd-intrinsics/vqdmulh_n.c
gcc.target/aarch64/advsimd-intrinsics/vsubw.c
gcc.target/aarch64/advsimd-intrinsics/vdup_lane.c
gcc.target/aarch64/advsimd-intrinsics/vget_high.c
gcc.target/aarch64/advsimd-intrinsics/vuzp.c
gcc.target/aarch64/advsimd-intrinsics/vqshl_n.c

[PATCH][Middle-end]patch for fixing PR 86519

2018-08-14 Thread Qing Zhao
Hi,

PR 86519:New test case gcc.dg/strcmpopt_6.c fails with its introduction in 
r262636.

***the root cause is:

for the following call to memcmp:   __builtin_memcmp (s->s, "a", 3);
the specified length 3 is larger than the length of "a", it's clearly an 
out-of-bound access. This new testing case is try to claim that,
For such out-of-bound access, we should NOT expand this call at all. The new 
added in-lining expansion was prohibited under
such situation, However, the expansion to hardware compare insn (old code) is 
NOT prohibited under such situation. 
on powerPC, the above call to memcmp is expanded to hardware compare insn. 
therefore, the testing case failed.

***in addition to the above major issue, there is also one minor issue with the 
new testing case itself:

dg-final { scan-rtl-dump-times "__builtin_memcmp" 6 "expand” }
this is trying to scan the dumped .expand file to match the string 
“__builtin_memcmp” exactly 6 times. however, the # of times that
the string “__builtin_memcmp” appears in the .expand file varies on different 
target or optimization level, in order to avoid such
instability, instead of scanning the .expand file to match the string 
“__builtin_memcmp”,  scanning the final assembly file to match
the string “memcmp”.

please review the attached simple patch.

thanks.

Qing

gcc/ChangeLog:

+2018-08-14  Qing Zhao  
+
+   PR testsuite/86519
+   * builtins.c (expand_builtin_memcmp): Do not expand the call
+   when overflow is detected.
+
gcc/testsuite/ChangeLog:

+2018-08-14  Qing Zhao 
+
+   PR testsuite/86519
+   * gcc.dg/strcmpopt_6.c: Scan the assembly file instead of
+   the .expand file.
+



PR86519.patch
Description: Binary data







Re: [aarch64}: added variable issue rate feature for falkor

2018-08-14 Thread Kyrill Tkachov

Hi Kai,

On 13/08/18 17:48, Kai Tietz wrote:

I repost updated patch containing ChangeLog entry.

Regards,
Kai


I think I understand what this patch does, please correct me if I'm wrong.
You model the processors micro-ops and some A64 instructions use multiple 
micro-ops.
This is what the falkor_variable_issue attribute specifies.
In TARGET_SCHED_VARIABLE_ISSUE you count the issue slots that the micro-ops take and how 
much "space" is left over, which is stored in leftover_uops
and you use leftover_uops in TARGET_SCHED_REORDER to tell the scheduler how 
many more micro-ops it can issue on that cycle.

And with that change the issue_rate is no longer the *instruction* issue rate, 
but rather the *micro-op* issue rate.
Overall this looks very similar to the implementation of this functionality in 
the powerpc port (rs6000).
Is this correct?

I have a few comments on the implementation inline...

Jim Wilson
Kai Tietz

* config/aarch64.c (aarch64_sched_reorder): Implement
TARGET_SCHED_REORDER hook.
(aarch64_variable_issue): Implement TARGET_SCHED_VARIABLE_ISSUE
hook.
(TARGET_SCHED_REORDER): Define.
(TARGET_SCHED_VARIABLE_ISSUE): Likewise.
* config/aarch64/falkor.md (falkor_variable_issue): New.

Index: aarch64/aarch64.c
===
--- aarch64.orig/aarch64.c
+++ aarch64/aarch64.c
@@ -914,7 +914,7 @@ static const struct tune_params qdf24xx_
   _branch_cost,
   _approx_modes,
   4, /* memmov_cost  */
-  4, /* issue_rate  */
+  8, /* issue_rate  */
   (AARCH64_FUSE_MOV_MOVK | AARCH64_FUSE_ADRP_ADD
| AARCH64_FUSE_MOVK_MOVK), /* fuseable_ops  */
   "16",  /* function_align.  */
@@ -17551,6 +17551,105 @@ aarch64_run_selftests (void)
 
 #endif /* #if CHECKING_P */
 
+/* The number of micro ops left over after issuing the last instruction in a

+   cycle.  This is subtracted from the next cycle before we start issuing 
insns.
+   This is initialized to 0 at the start of every basic block.  */
+static int leftover_uops = 0;
+

I believe the scheduler provides hooks specifically for storing 
backend-specific scheduling state so we should
avoid creating such static variables in aarch64.c. Can you use the 
TARGET_SCHED_*_SCHED_CONTEXT family of hooks here?
Then it will be up to the scheduler midend to keep track of the state and 
between basic blocks, functions etc.

 +/* Implement TARGET_SCHED_REORDER.  */
+
+static int
+aarch64_sched_reorder (FILE *file, int verbose,
+  rtx_insn **ready ATTRIBUTE_UNUSED,
+  int *n_readyp ATTRIBUTE_UNUSED,
+  int clock)
+{
+  int can_issue_more = aarch64_sched_issue_rate ();
+
+  if ((enum attr_tune) aarch64_tune == TUNE_FALKOR)
+{
+  /* The start of a basic block.  */
+  if (clock == 0)
+   {
+ if (leftover_uops && file && (verbose > 3))
+   fprintf (file, ";;\tLeftover uops ignored at bb start.\n");
+
+ leftover_uops = 0;
+   }
+
+  /* Account for issue slots left over from previous cycle.  This value
+can be larger than the number of issue slots per cycle, so we need
+to check it here before scheduling any instructions.  */
+  else if (leftover_uops)
+   {
+ can_issue_more -= leftover_uops;
+
+ if (file && (verbose > 3))
+   {
+ fprintf (file, ";;\tUse %d issue slots for leftover uops.\n",
+  leftover_uops);
+ fprintf (file, ";;\t%d issue slots left.\n", can_issue_more);
+   }
+
+ leftover_uops = 0;
+
+ if (can_issue_more < 0)
+   {
+ leftover_uops = 0 - can_issue_more;
+ can_issue_more = 0;
+
+ if (file && (verbose > 3))
+   {
+ fprintf (file, ";;skipping issue cycle.\n");
+ fprintf (file, ";;\t%d uops left over.\n", leftover_uops);
+   }
+   }
+   }
+}
+
+  return can_issue_more;
+}
+
+/* Implement TARGET_SCHED_VARIABLE_ISSUE.  */
+

A comment here like you have for TARGET_SCHED_REORDER describing what this 
function accomplishes would be very helpful.

 +static int
+aarch64_variable_issue (FILE *file, int verbose,
+   rtx_insn *insn, int more)
+{
+  if (GET_CODE (PATTERN (insn)) != USE
+  && GET_CODE (PATTERN (insn)) != CLOBBER)
+{
+  if ((enum attr_tune) aarch64_tune != TUNE_FALKOR)
+   more -= 1;
+  else
+   {
+ int issue_slots = get_attr_falkor_variable_issue (insn);
+ more -= issue_slots;
+


We generally try to avoid having explicit CPU-specific checks like this in the 
aarch64 backend.
Instead we try to keep all the CPU-specific tuning information in the CPU 
tuning structures.

This particular infrastructure looks like it could be used for other CPUs in 
the future. In order for that to happen we don't want
to have a check of aarch64_tune, but 

[PATCH] S/390: Remove UNSPEC_LTREL_BASE

2018-08-14 Thread Ilya Leoshkevich
It was needed only for g5/g6 machines, which are now gone.

gcc/ChangeLog:

2018-08-14  Ilya Leoshkevich  

* config/s390/s390.c (s390_decompose_constant_pool_ref):
Remove UNSPEC_LTREL_BASE check.
(annotate_constant_pool_refs): Likewise.
(find_constant_pool_ref): Likewise.
(find_ltrel_base): Removed.
(replace_ltrel_base): Removed.
(s390_mainpool_finish): Remove replace_ltrel_base call.
(s390_chunkify_start): Remove pending LTREL_BASE logic.
(s390_chunkify_finish): Remove replace_ltrel_base call.
* config/s390/s390.md: Remove UNSPEC_LTREL_BASE.
---
 gcc/config/s390/s390.c  | 131 ++--
 gcc/config/s390/s390.md |   1 -
 2 files changed, 5 insertions(+), 127 deletions(-)

diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index 4b0c6e7c147..285fdaacafa 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -2748,9 +2748,8 @@ s390_short_displacement (rtx disp)
   return false;
 }
 
-/* Attempts to split `ref', which should be either UNSPEC_LTREF or
-   UNSPEC_LTREL_BASE, into (base + `disp').  In case pool base is not known,
-   caller-provided `pool_base' is used.  If successful, also determines the
+/* Attempts to split `ref', which should be UNSPEC_LTREF, into (base + `disp').
+   If successful, also determines the
following characteristics of `ref': `is_ptr' - whether it can be an
LA argument, `is_base_ptr' - whether the resulting base is a well-known
base register (stack/frame pointer, etc), `is_pool_ptr` - whether it is
@@ -2758,8 +2757,7 @@ s390_short_displacement (rtx disp)
literal pool pointers per insn during or after reload (`B' constraint).  */
 static bool
 s390_decompose_constant_pool_ref (rtx *ref, rtx *disp, bool *is_ptr,
- bool *is_base_ptr, bool *is_pool_ptr,
- rtx pool_base)
+ bool *is_base_ptr, bool *is_pool_ptr)
 {
   if (!*ref)
 return true;
@@ -2778,13 +2776,6 @@ s390_decompose_constant_pool_ref (rtx *ref, rtx *disp, 
bool *is_ptr,
*ref = XVECEXP (*ref, 0, 1);
break;
 
-  case UNSPEC_LTREL_BASE:
-   if (XVECLEN (*ref, 0) == 1)
- *ref = pool_base, *is_pool_ptr = true;
-   else
- *ref = XVECEXP (*ref, 0, 1);
-   break;
-
   default:
return false;
   }
@@ -2921,12 +2912,12 @@ s390_decompose_address (rtx addr, struct s390_address 
*out)
 
   /* Validate base register.  */
   if (!s390_decompose_constant_pool_ref (, , , _ptr,
-_pool, fake_pool_base))
+_pool))
 return false;
 
   /* Validate index register.  */
   if (!s390_decompose_constant_pool_ref (, , , _ptr,
-_pool, fake_pool_base))
+_pool))
 return false;
 
   /* Prefer to use pointer as base, not index.  */
@@ -8156,16 +8147,6 @@ annotate_constant_pool_refs (rtx *x)
}
 }
 
-  /* Annotate LTREL_BASE as well.  */
-  if (GET_CODE (*x) == UNSPEC
-  && XINT (*x, 1) == UNSPEC_LTREL_BASE)
-{
-  rtx base = cfun->machine->base_reg;
-  *x = gen_rtx_UNSPEC (Pmode, gen_rtvec (2, XVECEXP (*x, 0, 0), base),
- UNSPEC_LTREL_BASE);
-  return;
-}
-
   fmt = GET_RTX_FORMAT (GET_CODE (*x));
   for (i = GET_RTX_LENGTH (GET_CODE (*x)) - 1; i >= 0; i--)
 {
@@ -8195,10 +8176,6 @@ find_constant_pool_ref (rtx x, rtx *ref)
   int i, j;
   const char *fmt;
 
-  /* Ignore LTREL_BASE references.  */
-  if (GET_CODE (x) == UNSPEC
-  && XINT (x, 1) == UNSPEC_LTREL_BASE)
-return;
   /* Likewise POOL_ENTRY insns.  */
   if (GET_CODE (x) == UNSPEC_VOLATILE
   && XINT (x, 1) == UNSPECV_POOL_ENTRY)
@@ -8281,73 +8258,6 @@ replace_constant_pool_ref (rtx *x, rtx ref, rtx offset)
 }
 }
 
-/* Check whether X contains an UNSPEC_LTREL_BASE.
-   Return its constant pool symbol if found, NULL_RTX otherwise.  */
-
-static rtx
-find_ltrel_base (rtx x)
-{
-  int i, j;
-  const char *fmt;
-
-  if (GET_CODE (x) == UNSPEC
-  && XINT (x, 1) == UNSPEC_LTREL_BASE)
-return XVECEXP (x, 0, 0);
-
-  fmt = GET_RTX_FORMAT (GET_CODE (x));
-  for (i = GET_RTX_LENGTH (GET_CODE (x)) - 1; i >= 0; i--)
-{
-  if (fmt[i] == 'e')
-   {
- rtx fnd = find_ltrel_base (XEXP (x, i));
- if (fnd)
-   return fnd;
-   }
-  else if (fmt[i] == 'E')
-   {
- for (j = 0; j < XVECLEN (x, i); j++)
-   {
- rtx fnd = find_ltrel_base (XVECEXP (x, i, j));
- if (fnd)
-   return fnd;
-   }
-   }
-}
-
-  return NULL_RTX;
-}
-
-/* Replace any occurrence of UNSPEC_LTREL_BASE in X with its base.  */
-
-static void
-replace_ltrel_base (rtx *x)
-{
-  int i, j;
-  const char *fmt;
-
-  if (GET_CODE (*x) == UNSPEC
-  && XINT (*x, 1) == 

Re: [Patch, Fortran] PR 86116: Ambiguous generic interface not recognised

2018-08-14 Thread Fritz Reese
Looks OK to me.

On Tue, Aug 14, 2018 at 4:12 AM Janus Weil  wrote:
>
> ping!
>
>
> Am So., 5. Aug. 2018 um 15:23 Uhr schrieb Janus Weil :
> >
> > Hi all,
> >
> > the attached patch fixes PR 86116 by splitting up the function
> > 'compare_type' into two variants: One that is used for checking
> > generic interfaces and operators (keeping the old name), and one that
> > is used for checking dummy functions and procedure pointer assignments
> > ('compare_type_characteristics'). The latter calls the former, but
> > includes an additional check that must not be done when checking
> > generics.
> >
> > Regtests cleanly on x86_64-linux-gnu. Ok for trunk?
> >
> > Cheers,
> > Janus
> >
> >
> > 2018-08-05  Janus Weil  
> >
> > PR fortran/86116
> > * interface.c (compare_type): Remove a CLASS/TYPE check.
> > (compare_type_characteristics): New function that behaves like the old
> > 'compare_type'.
> > (gfc_check_dummy_characteristics, gfc_check_result_characteristics):
> > Call 'compare_type_characteristics' instead of 'compare_type'.
> >
> > 2018-08-05  Janus Weil  
> >
> > PR fortran/86116
> > * gfortran.dg/generic_34.f90: New test case.


Re: [PATCH] Simplify overflow checks in duration literals

2018-08-14 Thread Jonathan Wakely

On 14/08/18 09:59 -0400, Marek Polacek wrote:

On Tue, Aug 14, 2018 at 02:55:17PM +0100, Jonathan Wakely wrote:

* include/std/chrono (__check_overflow): Simplify definition.
(_Checked_integral_constant): Remove.

Tested x86_64-linux, committed to trunk.





commit c0dec9d05de4695136694f78f2f76e3c9f15b3f1
Author: Jonathan Wakely 
Date:   Tue Aug 14 14:33:38 2018 +0100

Simplify overflow checks in duration literals

* include/std/chrono (__check_overflow): Simplify definition.
(_Checked_integral_constant): Remove.

diff --git a/libstdc++-v3/include/std/chrono b/libstdc++-v3/include/std/chrono
index da03fdccce4..871c896144a 100644
--- a/libstdc++-v3/include/std/chrono
+++ b/libstdc++-v3/include/std/chrono
@@ -900,24 +900,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   {
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wliteral-suffix"
-template
-  struct _Checked_integral_constant
-  : integral_constant<_Rep, static_cast<_Rep>(_Val)>
-  {
-   static_assert(_Checked_integral_constant::value >= 0
- && _Checked_integral_constant::value == _Val,
- "literal value cannot be represented by duration type");
-  };
-
 template
   constexpr _Dur __check_overflow()
   {
using _Val = __parse_int::_Parse_int<_Digits...>;
-   using _Rep = typename _Dur::rep;
-   // TODO: should be simply integral_constant<_Rep, _Val::value>
-   // but GCC doesn't reject narrowing conversions to _Rep.


Note that I fixed 57891 yesterday.  Or is this either 65043 or 78244?


Yes, thanks for that! That was what made me revisit the TODO. But
using integral_constant there doesn't produce the required error,
because it's in a system header. The standard requires it to be
ill-formed, so I can't rely on the narrowing diagnostic :-(

But I realised it could be simplified anyway (and that could have been
done years ago, without waiting for the fixes to narrowing in template
arguments).




Re: [PATCH] Add a character size parameter to c_strlen/get_range_strlen

2018-08-14 Thread Martin Sebor

On 08/14/2018 04:25 AM, Bernd Edlinger wrote:

Hi,

this patch is a follow-up to my patch here:
https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01800.html


As I said multiple times, this patch is redundant -- the issue
is fixed by the solution I posted back in July:

  https://gcc.gnu.org/ml/gcc-patches/2018-08/msg00155.html

and that I have been continuing to build upon (I posted a new
update yesterday).

You deliberately continue to make my work difficult, not just
making enhancements to help detect bugs you yourself complained
about, but also fix bugs I introduce myself.

Martin



Re: [PATCH] Simplify overflow checks in duration literals

2018-08-14 Thread Marek Polacek
On Tue, Aug 14, 2018 at 02:55:17PM +0100, Jonathan Wakely wrote:
>   * include/std/chrono (__check_overflow): Simplify definition.
>   (_Checked_integral_constant): Remove.
> 
> Tested x86_64-linux, committed to trunk.
> 
> 

> commit c0dec9d05de4695136694f78f2f76e3c9f15b3f1
> Author: Jonathan Wakely 
> Date:   Tue Aug 14 14:33:38 2018 +0100
> 
> Simplify overflow checks in duration literals
> 
> * include/std/chrono (__check_overflow): Simplify definition.
> (_Checked_integral_constant): Remove.
> 
> diff --git a/libstdc++-v3/include/std/chrono b/libstdc++-v3/include/std/chrono
> index da03fdccce4..871c896144a 100644
> --- a/libstdc++-v3/include/std/chrono
> +++ b/libstdc++-v3/include/std/chrono
> @@ -900,24 +900,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>{
>  #pragma GCC diagnostic push
>  #pragma GCC diagnostic ignored "-Wliteral-suffix"
> -template
> -  struct _Checked_integral_constant
> -  : integral_constant<_Rep, static_cast<_Rep>(_Val)>
> -  {
> - static_assert(_Checked_integral_constant::value >= 0
> -   && _Checked_integral_constant::value == _Val,
> -   "literal value cannot be represented by duration type");
> -  };
> -
>  template
>constexpr _Dur __check_overflow()
>{
>   using _Val = __parse_int::_Parse_int<_Digits...>;
> - using _Rep = typename _Dur::rep;
> - // TODO: should be simply integral_constant<_Rep, _Val::value>
> - // but GCC doesn't reject narrowing conversions to _Rep.

Note that I fixed 57891 yesterday.  Or is this either 65043 or 78244?

> - using _CheckedVal = _Checked_integral_constant<_Rep, _Val::value>;
> - return _Dur{_CheckedVal::value};
> + constexpr typename _Dur::rep __repval = _Val::value;
> + static_assert(__repval >= 0 && __repval == _Val::value,
> +   "literal value cannot be represented by duration type");
> + return _Dur(__repval);
>}
>  
>  constexpr chrono::duration>


Marek


Re: [PATCH] Use getentropy() for seeding PRNG

2018-08-14 Thread Fritz Reese
On Mon, Aug 13, 2018 at 4:12 PM Janne Blomqvist
 wrote:
>
> On Mon, Aug 13, 2018 at 5:36 PM, Fritz Reese  wrote:
>>
>> On Fri, Aug 3, 2018 at 9:19 AM Janne Blomqvist
>>  wrote:
>> >
>> > The getentropy function, found on Linux, OpenBSD, and recently also
>> > FreeBSD, can be used to get random bytes to initialize the PRNG.  It
>> > is similar to the traditional way of reading from /dev/urandom, but
>> > being a system call rather than a special file, it doesn't suffer from
>> > problems like running out of file descriptors, or failure when running
>> > in a container where /dev/urandom is not available.
>> >
>> > Regtested on x86_64-pc-linux-gnu, Ok for trunk?
>>
>> Actually, getentropy() is similar to reading from /dev/random, where
>> getrandom() is similar to reading from /dev/urandom.
>
>
> No, getentropy is similar to getrandom with the flags argument == 0. Which is 
> similar to reading /dev/urandom, except that just after boot if enough 
> entropy hasn't yet been gathered, it may block instead of returning some 
> not-quite-random data. But once it has been initialized, it will never block 
> again.
>
> I agree that reading from /dev/random is overkill, but this patch isn't doing 
> the equivalent of that.
>

Fair enough, I misread the documentation on getentropy(). Then I
concur with Jakub, patch looks OK.

Fritz


[PATCH] Simplify overflow checks in duration literals

2018-08-14 Thread Jonathan Wakely

* include/std/chrono (__check_overflow): Simplify definition.
(_Checked_integral_constant): Remove.

Tested x86_64-linux, committed to trunk.


commit c0dec9d05de4695136694f78f2f76e3c9f15b3f1
Author: Jonathan Wakely 
Date:   Tue Aug 14 14:33:38 2018 +0100

Simplify overflow checks in duration literals

* include/std/chrono (__check_overflow): Simplify definition.
(_Checked_integral_constant): Remove.

diff --git a/libstdc++-v3/include/std/chrono b/libstdc++-v3/include/std/chrono
index da03fdccce4..871c896144a 100644
--- a/libstdc++-v3/include/std/chrono
+++ b/libstdc++-v3/include/std/chrono
@@ -900,24 +900,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   {
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wliteral-suffix"
-template
-  struct _Checked_integral_constant
-  : integral_constant<_Rep, static_cast<_Rep>(_Val)>
-  {
-   static_assert(_Checked_integral_constant::value >= 0
- && _Checked_integral_constant::value == _Val,
- "literal value cannot be represented by duration type");
-  };
-
 template
   constexpr _Dur __check_overflow()
   {
using _Val = __parse_int::_Parse_int<_Digits...>;
-   using _Rep = typename _Dur::rep;
-   // TODO: should be simply integral_constant<_Rep, _Val::value>
-   // but GCC doesn't reject narrowing conversions to _Rep.
-   using _CheckedVal = _Checked_integral_constant<_Rep, _Val::value>;
-   return _Dur{_CheckedVal::value};
+   constexpr typename _Dur::rep __repval = _Val::value;
+   static_assert(__repval >= 0 && __repval == _Val::value,
+ "literal value cannot be represented by duration type");
+   return _Dur(__repval);
   }
 
 constexpr chrono::duration>


Re: [PATCH] convert braced initializers to strings (PR 71625)

2018-08-14 Thread James Greenhalgh
On Wed, Aug 08, 2018 at 07:17:07PM -0500, Martin Sebor wrote:
> On 08/08/2018 05:08 AM, Jason Merrill wrote:
> > On Wed, Aug 8, 2018 at 9:04 AM, Martin Sebor  wrote:
> >> On 08/07/2018 02:57 AM, Jason Merrill wrote:
> >>>
> >>> On Wed, Aug 1, 2018 at 12:49 AM, Martin Sebor  wrote:
> 
>  On 07/31/2018 07:38 AM, Jason Merrill wrote:



> Done in the attached patch.  I've also avoided dealing with
> zero-length arrays and added tests to make sure their size
> stays is regardless of the form of their initializer and
> the appropriate warnings are issued.
> 
> Using build_string() rather than build_string_literal() needed
> a tweak in digest_init_r().  It didn't break anything but since
> the array type may not have a domain yet, neither will the
> string.  It looks like that may get adjusted later on but I've
> temporarily guarded the code with #if 1.  If the change is
> fine I'll remove the #if before committing.
> 
> This initial patch only handles narrow character initializers
> (i.e., those with TYPE_STRING_FLAG set).  Once this gets some
> exposure I'd like to extend it to other character types,
> including wchar_t.

Hi Martin,

This causes issues for the AArch64 tests (full list below).

I see an error message on the following construct:

  void foo (void)
  {
__Poly8_t x[4] = { 1, 2, 3, 4 };
  }

  init.c:3:20: error: array of inappropriate type initialized from string 
constant
  3 |   __Poly8_t x[4] = { 1, 2, 3, 4 };
|

__Poly8_t is a type we define in our backend, through a convoluted set of
functions, which operates a lot like an unsigned, QI mode type.

A second set of tests fail due to changed inlining behaviour for functions
with char array initialization:

  gcc.target/aarch64/vset_lane_1.c
  gcc.target/aarch64/vneg_s.c
  gcc.target/aarch64/vclz.c

Thanks,
James

-

New failures in: 


gcc.target/aarch64/advsimd-intrinsics/vmax.c
gcc.target/aarch64/simd/vzipqp8_1.c
gcc.target/aarch64/vldN_dup_1.c
gcc.target/aarch64/advsimd-intrinsics/vcle.c
gcc.target/aarch64/advsimd-intrinsics/vadd.c
gcc.target/aarch64/advsimd-intrinsics/vhadd.c
gcc.target/aarch64/advsimd-intrinsics/vmull_n.c
gcc.target/aarch64/advsimd-intrinsics/vrndn.c
gcc.target/aarch64/simd/vtrnqp8_1.c
gcc.target/aarch64/advsimd-intrinsics/vpadal.c
gcc.target/aarch64/advsimd-intrinsics/vtrn_half.c
gcc.target/aarch64/advsimd-intrinsics/vqdmlal_n.c
gcc.target/aarch64/advsimd-intrinsics/vqdmulh.c
gcc.target/aarch64/advsimd-intrinsics/vqsub.c
gcc.target/aarch64/advsimd-intrinsics/vqdmlal_lane.c
gcc.target/aarch64/advsimd-intrinsics/vqdmlsl_n.c
gcc.target/aarch64/advsimd-intrinsics/vuzp_half.c
gcc.target/aarch64/advsimd-intrinsics/vst1_lane.c
gcc.target/aarch64/advsimd-intrinsics/vmla_lane.c
gcc.target/aarch64/advsimd-intrinsics/vqdmulh_lane.c
gcc.target/aarch64/advsimd-intrinsics/vrsqrte.c
gcc.target/aarch64/advsimd-intrinsics/vneg.c
gcc.target/aarch64/simd/vuzpqp8_1.c
gcc.target/aarch64/advsimd-intrinsics/vcale.c
gcc.target/aarch64/advsimd-intrinsics/vmla_n.c
gcc.target/aarch64/advsimd-intrinsics/vsub.c
gcc.target/aarch64/advsimd-intrinsics/vrev.c
gcc.target/aarch64/advsimd-intrinsics/vmul.c
gcc.target/aarch64/advsimd-intrinsics/vldX.c
gcc.target/aarch64/advsimd-intrinsics/vsubl.c
gcc.target/aarch64/advsimd-intrinsics/vfms.c
gcc.target/aarch64/advsimd-intrinsics/vmlsl.c
gcc.target/aarch64/advsimd-intrinsics/vsli_n.c
gcc.target/aarch64/advsimd-intrinsics/vcombine.c
gcc.target/aarch64/advsimd-intrinsics/vmul_n.c
gcc.target/aarch64/advsimd-intrinsics/vldX_dup.c
gcc.target/aarch64/advsimd-intrinsics/vpaddl.c
gcc.target/aarch64/advsimd-intrinsics/vqshrn_n.c
gcc.target/aarch64/advsimd-intrinsics/vstX_lane.c
gcc.target/aarch64/advsimd-intrinsics/vqtbX.c
gcc.target/aarch64/advsimd-intrinsics/vext.c
gcc.target/aarch64/advsimd-intrinsics/vtrn.c
gcc.target/aarch64/advsimd-intrinsics/vtst.c
gcc.target/aarch64/advsimd-intrinsics/vbic.c
gcc.target/aarch64/advsimd-intrinsics/vqdmlsl.c
gcc.target/aarch64/advsimd-intrinsics/vqshl.c
gcc.target/aarch64/advsimd-intrinsics/vrsqrts.c
gcc.target/aarch64/advsimd-intrinsics/vqdmull_n.c
gcc.target/aarch64/advsimd-intrinsics/vqdmlsl_lane.c
gcc.target/aarch64/advsimd-intrinsics/vqdmulh_n.c
gcc.target/aarch64/advsimd-intrinsics/vsubw.c
gcc.target/aarch64/advsimd-intrinsics/vdup_lane.c
gcc.target/aarch64/advsimd-intrinsics/vget_high.c
gcc.target/aarch64/advsimd-intrinsics/vuzp.c
gcc.target/aarch64/advsimd-intrinsics/vqshl_n.c
gcc.target/aarch64/advsimd-intrinsics/vrsra_n.c
gcc.target/aarch64/advsimd-intrinsics/vcgt.c
gcc.target/aarch64/advsimd-intrinsics/vld1_dup.c
gcc.target/aarch64/advsimd-intrinsics/vaddhn.c
gcc.target/aarch64/advsimd-intrinsics/vqshlu_n.c
gcc.target/aarch64/advsimd-intrinsics/vabs.c
gcc.target/aarch64/advsimd-intrinsics/vshll_n.c
gcc.target/aarch64/advsimd-intrinsics/vsubhn.c
gcc.target/aarch64/advsimd-intrinsics/vmlal.c
gcc.target/aarch64/advsimd-intrinsics/vqdmlal.c
gcc.target/aarch64/advsimd-intrinsics/vrecpe.c
gcc.target/aarch64/advsimd-intrinsics/vqneg.c

[PATCH] PR libstdc++/86846 Alternative to pointer-width atomics

2018-08-14 Thread Jonathan Wakely

Define a class using std::mutex for when std::atomic
cannot be used to implement the default memory resource.

When std::mutex constructor is not constexpr the constant_init trick
won't work, so just define a global and use init_priority for it. The
compiler warns about using reserved priority, so put the definition in a
header file using #pragma GCC system_header to suppress the warning.

PR libstdc++/86846
* src/c++17/default_resource.h: New file, defining default_res.
* src/c++17/memory_resource.cc [ATOMIC_POINTER_LOCK_FREE != 2]
(atomic_mem_res): Define alternative for atomic
using a mutex instead of atomics.

Tested x86_64-linux, committed to trunk.


commit 81de951109506b684d079f0fbc7aebae2c228ba4
Author: Jonathan Wakely 
Date:   Mon Aug 13 12:34:56 2018 +0100

PR libstdc++/86846 Alternative to pointer-width atomics

Define a class using std::mutex for when std::atomic
cannot be used to implement the default memory resource.

When std::mutex constructor is not constexpr the constant_init trick
won't work, so just define a global and use init_priority for it. The
compiler warns about using reserved priority, so put the definition in a
header file using #pragma GCC system_header to suppress the warning.

PR libstdc++/86846
* src/c++17/default_resource.h: New file, defining default_res.
* src/c++17/memory_resource.cc [ATOMIC_POINTER_LOCK_FREE != 2]
(atomic_mem_res): Define alternative for atomic
using a mutex instead of atomics.

diff --git a/libstdc++-v3/src/c++17/default_resource.h 
b/libstdc++-v3/src/c++17/default_resource.h
new file mode 100644
index 000..522cee13b90
--- /dev/null
+++ b/libstdc++-v3/src/c++17/default_resource.h
@@ -0,0 +1,11 @@
+// This is only in a header so we can use the system_header pragma,
+// to suppress the warning caused by using a reserved init_priority.
+#pragma GCC system_header
+
+#if ATOMIC_POINTER_LOCK_FREE == 2 || defined(__GTHREAD_MUTEX_INIT)
+# error "This file should not be included for this build"
+#endif
+
+struct {
+  atomic_mem_res obj = _res.obj;
+} default_res __attribute__ ((init_priority (100)));
diff --git a/libstdc++-v3/src/c++17/memory_resource.cc 
b/libstdc++-v3/src/c++17/memory_resource.cc
index c3ae2b69f71..bd8f32d931e 100644
--- a/libstdc++-v3/src/c++17/memory_resource.cc
+++ b/libstdc++-v3/src/c++17/memory_resource.cc
@@ -25,6 +25,10 @@
 #include 
 #include 
 #include 
+#if ATOMIC_POINTER_LOCK_FREE != 2
+# include// std::mutex, std::lock_guard
+# include // std::exchange
+#endif
 
 namespace std _GLIBCXX_VISIBILITY(default)
 {
@@ -81,7 +85,42 @@ namespace pmr
 
 constant_init newdel_res{};
 constant_init null_res{};
-constant_init> default_res{_res.obj};
+#if ATOMIC_POINTER_LOCK_FREE == 2
+using atomic_mem_res = atomic;
+# define _GLIBCXX_ATOMIC_MEM_RES_CAN_BE_CONSTANT_INITIALIZED
+#else
+// Can't use pointer-width atomics, define a type using a mutex instead:
+struct atomic_mem_res
+{
+# ifdef __GTHREAD_MUTEX_INIT
+#  define _GLIBCXX_ATOMIC_MEM_RES_CAN_BE_CONSTANT_INITIALIZED
+  // std::mutex has constexpr constructor
+  constexpr
+# endif
+  atomic_mem_res(memory_resource* r) : val(r) { }
+
+  mutex mx;
+  memory_resource* val;
+
+  memory_resource* load()
+  {
+   lock_guard lock(mx);
+   return val;
+  }
+
+  memory_resource* exchange(memory_resource* r)
+  {
+   lock_guard lock(mx);
+   return std::exchange(val, r);
+  }
+};
+#endif // ATOMIC_POINTER_LOCK_FREE == 2
+
+#ifdef _GLIBCXX_ATOMIC_MEM_RES_CAN_BE_CONSTANT_INITIALIZED
+constant_init default_res{_res.obj};
+#else
+# include "default_resource.h"
+#endif
   } // namespace
 
   memory_resource*


[PING 4][PATCH] [v4][aarch64] Avoid tag collisions for loads falkor

2018-08-14 Thread Siddhesh Poyarekar

Ping!

On 07/24/2018 12:37 PM, Siddhesh Poyarekar wrote:

Hi,

This is a rewrite of the tag collision avoidance patch that Kugan had
written as a machine reorg pass back in February.

The falkor hardware prefetching system uses a combination of the
source, destination and offset to decide which prefetcher unit to
train with the load.  This is great when loads in a loop are
sequential but sub-optimal if there are unrelated loads in a loop that
tag to the same prefetcher unit.

This pass attempts to rename the desination register of such colliding
loads using routines available in regrename.c so that their tags do
not collide.  This shows some performance gains with mcf and xalancbmk
(~5% each) and will be tweaked further.  The pass is placed near the
fag end of the pass list so that subsequent passes don't inadvertantly
end up undoing the renames.

A full gcc bootstrap and testsuite ran successfully on aarch64, i.e. it
did not introduce any new regressions.  I also did a make-check with
-mcpu=falkor to ensure that there were no regressions.  The couple of
regressions I found were target-specific and were related to scheduling
and cost differences and are not correctness issues.

Changes from v3:
- Avoid renaming argument/return registers and registers that have a
   specific architectural meaning, i.e. stack pointer, frame pointer,
   etc.  Try renaming their aliases instead.

Changes from v2:
- Ignore SVE instead of asserting that falkor does not support sve

Changes from v1:

- Fixed up issues pointed out by Kyrill
- Avoid renaming R0/V0 since they could be return values
- Fixed minor formatting issues.

2018-07-02  Siddhesh Poyarekar  
Kugan Vivekanandarajah  

* config/aarch64/falkor-tag-collision-avoidance.c: New file.
* config.gcc (extra_objs): Build it.
* config/aarch64/t-aarch64 (falkor-tag-collision-avoidance.o):
Likewise.
* config/aarch64/aarch64-passes.def
(pass_tag_collision_avoidance): New pass.
* config/aarch64/aarch64.c (qdf24xx_tunings): Add
AARCH64_EXTRA_TUNE_RENAME_LOAD_REGS to tuning_flags.
(aarch64_classify_address): Remove static qualifier.
(aarch64_address_info, aarch64_address_type): Move to...
* config/aarch64/aarch64-protos.h: ... here.
(make_pass_tag_collision_avoidance): New function.
* config/aarch64/aarch64-tuning-flags.def (rename_load_regs):
New tuning flag.

CC: james.greenha...@arm.com
CC: kyrylo.tkac...@foss.arm.com
---
  gcc/config.gcc|   2 +-
  gcc/config/aarch64/aarch64-passes.def |   1 +
  gcc/config/aarch64/aarch64-protos.h   |  49 +
  gcc/config/aarch64/aarch64-tuning-flags.def   |   2 +
  gcc/config/aarch64/aarch64.c  |  48 +-
  .../aarch64/falkor-tag-collision-avoidance.c  | 881 ++
  gcc/config/aarch64/t-aarch64  |   9 +
  7 files changed, 946 insertions(+), 46 deletions(-)
  create mode 100644 gcc/config/aarch64/falkor-tag-collision-avoidance.c

diff --git a/gcc/config.gcc b/gcc/config.gcc
index 78e84c2b864..8f5e458e8a6 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -304,7 +304,7 @@ aarch64*-*-*)
extra_headers="arm_fp16.h arm_neon.h arm_acle.h"
c_target_objs="aarch64-c.o"
cxx_target_objs="aarch64-c.o"
-   extra_objs="aarch64-builtins.o aarch-common.o cortex-a57-fma-steering.o"
+   extra_objs="aarch64-builtins.o aarch-common.o cortex-a57-fma-steering.o 
falkor-tag-collision-avoidance.o"
target_gtfiles="\$(srcdir)/config/aarch64/aarch64-builtins.c"
target_has_targetm_common=yes
;;
diff --git a/gcc/config/aarch64/aarch64-passes.def 
b/gcc/config/aarch64/aarch64-passes.def
index 87747b420b0..f61a8870aa1 100644
--- a/gcc/config/aarch64/aarch64-passes.def
+++ b/gcc/config/aarch64/aarch64-passes.def
@@ -19,3 +19,4 @@
 .  */
  
  INSERT_PASS_AFTER (pass_regrename, 1, pass_fma_steering);

+INSERT_PASS_AFTER (pass_machine_reorg, 1, pass_tag_collision_avoidance);
diff --git a/gcc/config/aarch64/aarch64-protos.h 
b/gcc/config/aarch64/aarch64-protos.h
index af5db9c5953..647ad7a9c37 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -288,6 +288,49 @@ struct tune_params
const struct cpu_prefetch_tune *prefetch;
  };
  
+/* Classifies an address.

+
+   ADDRESS_REG_IMM
+   A simple base register plus immediate offset.
+
+   ADDRESS_REG_WB
+   A base register indexed by immediate offset with writeback.
+
+   ADDRESS_REG_REG
+   A base register indexed by (optionally scaled) register.
+
+   ADDRESS_REG_UXTW
+   A base register indexed by (optionally scaled) zero-extended register.
+
+   ADDRESS_REG_SXTW
+   A base register indexed by (optionally scaled) sign-extended register.
+
+   ADDRESS_LO_SUM
+   A LO_SUM rtx with a base register and "LO12" symbol relocation.
+
+   ADDRESS_SYMBOLIC:
+   A 

Re: [PATCH][GCC][AArch64] Limit movmem copies to TImode copies.

2018-08-14 Thread Sudakshina Das

Hi Tamar

On 13/08/18 17:27, Tamar Christina wrote:

Hi Thomas,

Thanks for the review.

I’ll correct the typo before committing if I have no other changes required by 
a maintainer.

Regards,
Tamar.



I am not a maintainer but I would like to point out something in your
patch. I think you test case will fail with -mabi=ilp32

FAIL: gcc.target/aarch64/large_struct_copy_2.c (test for excess errors)
Excess errors:
/work/trunk/src/gcc/gcc/testsuite/gcc.target/aarch64/large_struct_copy_2.c:18:27: 
warning: overflow in conversion from 'long

long int' to 'long int' changes value from '4073709551611' to
'2080555003' [-Woverflow]

We have had more such recent failures and James gave a very neat
way to make sure the mode comes out what you intend it to here:
https://gcc.gnu.org/ml/gcc-patches/2018-08/msg00233.html

I would just ask you to change the data types accordingly and test it
with -mabi=ilp32.

Thanks
Sudi


From: Thomas Preudhomme 
Sent: Monday, August 13, 2018 14:37
To: Tamar Christina 
Cc: gcc-patches@gcc.gnu.org; nd ; James Greenhalgh 
; Richard Earnshaw ; Marcus Shawcroft 

Subject: Re: [PATCH][GCC][AArch64] Limit movmem copies to TImode copies.

Hi Tamar,

Thanks for your patch.

Just one comment about your ChangeLog entry for the testsuiet change: shouldn't 
it mention that it is a new testcase? The patch you attached seems to create 
the file.

Best regards,

Thomas

On Mon, 13 Aug 2018 at 10:33, Tamar Christina 
mailto:tamar.christ...@arm.com>> wrote:
Hi All,

On AArch64 we have integer modes larger than TImode, and while we can generate
moves for these they're not as efficient.

So instead make sure we limit the maximum we can copy to TImode.  This means
copying a 16 byte struct will issue 1 TImode copy, which will be done using a
single STP as we expect but an CImode sized copy won't issue CImode operations.

Bootstrapped and regtested on aarch4-none-linux-gnu and no issues.
Crosstested aarch4_be-none-elf and no issues.

Ok for trunk?

Thanks,
Tamar

gcc/
2018-08-13  Tamar Christina  
mailto:tamar.christ...@arm.com>>

 * config/aarch64/aarch64.c (aarch64_expand_movmem): Set TImode max.

gcc/testsuite/
2018-08-13  Tamar Christina  
mailto:tamar.christ...@arm.com>>

 * gcc.target/aarch64/large_struct_copy_2.c: Add assembler scan.

--





[PATCH] PR libstdc++/85343 overload __throw_ios_failure to take errno

2018-08-14 Thread Jonathan Wakely

[ios::failure] p2: "When throwing ios_base::failure exceptions,
implementations should provide values of ec that identify the specific
reason for the failure."

This adds a new overload of __throw_ios_failure that can be passed
errno, to store error_code(errno, system_category()) in the exception
object.

PR libstdc++/85343
* acinclude.m4 (libtool_VERSION): Bump version.
* config/abi/pre/gnu.ver (GLIBCXX_3.4.26): Add new symbol version.
Export new symbol.
* configure: Regenerate.
* doc/xml/manual/abi.xml: Document new versions.
* include/bits/fstream.tcc (basic_filebuf::underflow)
(basic_filebuf::xsgetn): Pass errno to __throw_ios_failure.
* include/bits/functexcept.h (__throw_ios_failure(const char*, int)):
Declare new overload.
* src/c++11/cxx11-ios_failure.cc (__ios_failure): Add new constructor
and static member function.
(__throw_ios_failure(const char*, int)): Define.
* src/c++98/ios_failure.cc [!_GLIBCXX_USE_DUAL_ABI]
(__throw_ios_failure(const char*, int)): Define.
* testsuite/util/testsuite_abi.cc: Update known and latest versions.

Tested x86_64-linux, committed to trunk.


commit 204e5918142eca04dbe1cba92fbf20e2791ad7e4
Author: Jonathan Wakely 
Date:   Wed Apr 11 13:42:34 2018 +0100

PR libstdc++/85343 overload __throw_ios_failure to take errno

[ios::failure] p2: "When throwing ios_base::failure exceptions,
implementations should provide values of ec that identify the specific
reason for the failure."

This adds a new overload of __throw_ios_failure that can be passed
errno, to store error_code(errno, system_category()) in the exception
object.

PR libstdc++/85343
* acinclude.m4 (libtool_VERSION): Bump version.
* config/abi/pre/gnu.ver (GLIBCXX_3.4.26): Add new symbol version.
Export new symbol.
* configure: Regenerate.
* doc/xml/manual/abi.xml: Document new versions.
* include/bits/fstream.tcc (basic_filebuf::underflow)
(basic_filebuf::xsgetn): Pass errno to __throw_ios_failure.
* include/bits/functexcept.h (__throw_ios_failure(const char*, 
int)):
Declare new overload.
* src/c++11/cxx11-ios_failure.cc (__ios_failure): Add new 
constructor
and static member function.
(__throw_ios_failure(const char*, int)): Define.
* src/c++98/ios_failure.cc [!_GLIBCXX_USE_DUAL_ABI]
(__throw_ios_failure(const char*, int)): Define.
* testsuite/util/testsuite_abi.cc: Update known and latest versions.

diff --git a/libstdc++-v3/config/abi/pre/gnu.ver 
b/libstdc++-v3/config/abi/pre/gnu.ver
index 593783da1aa..03b23200a1a 100644
--- a/libstdc++-v3/config/abi/pre/gnu.ver
+++ b/libstdc++-v3/config/abi/pre/gnu.ver
@@ -2046,6 +2046,9 @@ GLIBCXX_3.4.26 {
 _ZNSt3pmr25monotonic_buffer_resource13_M_new_bufferE[jmy][jmy];
 _ZNSt3pmr25monotonic_buffer_resource18_M_release_buffersEv;
 
+# std::__throw_ios_failure(const char*, int);
+_ZSt19__throw_ios_failurePKci;
+
 } GLIBCXX_3.4.25;
 
 # Symbols in the support library (libsupc++) have their own tag.
diff --git a/libstdc++-v3/doc/xml/manual/abi.xml 
b/libstdc++-v3/doc/xml/manual/abi.xml
index 733c803ffac..8859e965000 100644
--- a/libstdc++-v3/doc/xml/manual/abi.xml
+++ b/libstdc++-v3/doc/xml/manual/abi.xml
@@ -268,6 +268,7 @@ compatible.
 GCC 7.1.0: libstdc++.so.6.0.23
 GCC 7.2.0: libstdc++.so.6.0.24
 GCC 8.0.0: libstdc++.so.6.0.25
+GCC 9.0.0: libstdc++.so.6.0.26
 
 
   Note 1: Error should be libstdc++.so.3.0.3.
@@ -338,6 +339,7 @@ compatible.
 GCC 7.1.0: GLIBCXX_3.4.23, CXXABI_1.3.11
 GCC 7.2.0: GLIBCXX_3.4.24, CXXABI_1.3.11
 GCC 8.0.0: GLIBCXX_3.4.25, CXXABI_1.3.11
+GCC 9.0.0: GLIBCXX_3.4.26, CXXABI_1.3.11
 
 
 
diff --git a/libstdc++-v3/include/bits/fstream.tcc 
b/libstdc++-v3/include/bits/fstream.tcc
index 6205db75340..ed98f13e0e0 100644
--- a/libstdc++-v3/include/bits/fstream.tcc
+++ b/libstdc++-v3/include/bits/fstream.tcc
@@ -38,6 +38,7 @@
 
 #include 
 #include// for swap
+#include 
 
 namespace std _GLIBCXX_VISIBILITY(default)
 {
@@ -471,7 +472,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
"invalid byte sequence in file"));
  else
__throw_ios_failure(__N("basic_filebuf::underflow "
-   "error reading the file"));
+   "error reading the file"), errno);
}
   return __ret;
 }
@@ -717,7 +718,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
  __len = _M_file.xsgetn(reinterpret_cast(__s), __n);
  if (__len == -1)
__throw_ios_failure(__N("basic_filebuf::xsgetn "
-   "error reading the file"));
+   "error reading the file"), errno);
 

[PATCH] [C] Warn when calculating abs(unsigned_value)

2018-08-14 Thread Martin Jambor
Hi,

when you try compiling a call to function abs and provide an unsigned
int in the argument in C++, you will get an error about ambiguous
overload.  In C however, it will pass without silently.  The following
patch adds a warning for these cases, because I think it is likely that
such code does not do what the author intended.

I thought it a bit of an overkill to add a new warning group because of
this, so I added it to -Wtype-limits, which seemed the best fit.  As a
consequence, the warning is on with -Wextra.  The warning can be
suppressed with an explicit type-cast (which at the moment is implicit),
if someone really used it for some bit-tricks.

Bootstrapped and tested on x86_64-linux, also with make info.  What do
you think, is this a good idea?  Is it perhaps OK for trunk?

Thanks,

Martin



2018-08-14  Martin Jambor  

* doc/invoke.texi (Warning Options): Document warning on calculating
absolute value of an unsigned value.

c/ChangeLog:
* c-parser.c (c_parser_postfix_expression_after_primary): Warn if
calculating an absolute value of an unsigned value.

testsuite/ChangeLog
* gcc.dg/warn-abs-unsigned-1.c: New test.
---
 gcc/c/c-parser.c   | 20 ++--
 gcc/doc/invoke.texi|  5 +++--
 gcc/testsuite/gcc.dg/warn-abs-unsigned-1.c | 21 +
 3 files changed, 38 insertions(+), 8 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/warn-abs-unsigned-1.c

diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 7a926285f3a..bf3b8cc75f9 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -9160,13 +9160,21 @@ c_parser_postfix_expression_after_primary (c_parser 
*parser,
  sizeof_arg,
  sizeof_ptr_memacc_comptypes);
  if (TREE_CODE (expr.value) == FUNCTION_DECL
- && DECL_BUILT_IN_CLASS (expr.value) == BUILT_IN_NORMAL
- && DECL_FUNCTION_CODE (expr.value) == BUILT_IN_MEMSET
- && vec_safe_length (exprlist) == 3)
+ && DECL_BUILT_IN_CLASS (expr.value) == BUILT_IN_NORMAL)
{
- tree arg0 = (*exprlist)[0];
- tree arg2 = (*exprlist)[2];
- warn_for_memset (expr_loc, arg0, arg2, literal_zero_mask);
+ if (DECL_FUNCTION_CODE (expr.value) == BUILT_IN_MEMSET
+ && vec_safe_length (exprlist) == 3)
+   {
+ tree arg0 = (*exprlist)[0];
+ tree arg2 = (*exprlist)[2];
+ warn_for_memset (expr_loc, arg0, arg2, literal_zero_mask);
+   }
+ else if (DECL_FUNCTION_CODE (expr.value) == BUILT_IN_ABS
+  && vec_safe_length (exprlist) == 1
+  && TYPE_UNSIGNED (TREE_TYPE ((*exprlist)[0])))
+   warning_at (expr_loc, OPT_Wtype_limits,
+   "calculation of an absolute value of "
+   "a value of unsigned type");
}
 
  start = expr.get_start ();
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index d7fd0e17555..b757ac0d0c8 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -6179,8 +6179,9 @@ This warning is enabled by default.
 Warn if a comparison is always true or always false due to the limited
 range of the data type, but do not warn for constant expressions.  For
 example, warn if an unsigned variable is compared against zero with
-@code{<} or @code{>=}.  This warning is also enabled by
-@option{-Wextra}.
+@code{<} or @code{>=}.  When compiling C, also warn when calculating
+an absolute value from an unsigned type.  This warning is also enabled
+by @option{-Wextra}.
 
 @include cppwarnopts.texi
 
diff --git a/gcc/testsuite/gcc.dg/warn-abs-unsigned-1.c 
b/gcc/testsuite/gcc.dg/warn-abs-unsigned-1.c
new file mode 100644
index 000..8d2a6d87e6b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/warn-abs-unsigned-1.c
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-options "-Wtype-limits" } */
+
+#include 
+
+unsigned __attribute__((noipa))
+get_input (void)
+{
+  return 22;
+}
+
+volatile unsigned g;
+
+int
+main(int argc __attribute__((unused)), char **argv __attribute__((unused)))
+{
+  unsigned u = get_input ();
+  u = abs (u);  /* { dg-warning "calculation of an absolute value of a value 
of unsigned type" } */
+  g = u;
+  return 0;
+}
-- 
2.18.0



[PATCH, Darwin, Obvious] Fix PR81685, by adding DWARF 5 section names.

2018-08-14 Thread Iain Sandoe
Hi

I plan to apply this as obvious unless there are any comments in the next day 
or so.

The ICE is caused by missing mach-o section names for the ones introduced for 
DWARF5.

Where possible (i.e. they are currently defined), I’ve synced the Darwin names 
with the ones
emitted by clang.

thanks
Iain

gcc/

   config/darwin.h: (DEBUG_STR_OFFSETS_SECTION, DEBUG_LOCLISTS_SECTION,
   DEBUG_RNGLISTS_SECTION) new macros.  (DEBUG_PUBNAMES_SECTION,
   DEBUG_PUBTYPES_SECTION) update to include GNU variant.
---
 gcc/config/darwin.h | 33 +
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/gcc/config/darwin.h b/gcc/config/darwin.h
index 62c2ae8cc0..d49f146338 100644
--- a/gcc/config/darwin.h
+++ b/gcc/config/darwin.h
@@ -436,18 +436,20 @@ extern GTY(()) int darwin_ms_struct;
 
 #define DWARF2_DEBUGGING_INFO 1
 
-#define DEBUG_FRAME_SECTION"__DWARF,__debug_frame,regular,debug"
-#define DEBUG_INFO_SECTION "__DWARF,__debug_info,regular,debug"
-#define DEBUG_ABBREV_SECTION   "__DWARF,__debug_abbrev,regular,debug"
-#define DEBUG_ARANGES_SECTION  "__DWARF,__debug_aranges,regular,debug"
-#define DEBUG_MACINFO_SECTION  "__DWARF,__debug_macinfo,regular,debug"
-#define DEBUG_LINE_SECTION "__DWARF,__debug_line,regular,debug"
-#define DEBUG_LOC_SECTION  "__DWARF,__debug_loc,regular,debug"
-#define DEBUG_PUBNAMES_SECTION "__DWARF,__debug_pubnames,regular,debug"
-#define DEBUG_PUBTYPES_SECTION "__DWARF,__debug_pubtypes,regular,debug"
-#define DEBUG_STR_SECTION  "__DWARF,__debug_str,regular,debug"
-#define DEBUG_RANGES_SECTION   "__DWARF,__debug_ranges,regular,debug"
-#define DEBUG_MACRO_SECTION "__DWARF,__debug_macro,regular,debug"
+#define DEBUG_FRAME_SECTION  "__DWARF,__debug_frame,regular,debug"
+#define DEBUG_INFO_SECTION   "__DWARF,__debug_info,regular,debug"
+#define DEBUG_ABBREV_SECTION "__DWARF,__debug_abbrev,regular,debug"
+#define DEBUG_ARANGES_SECTION"__DWARF,__debug_aranges,regular,debug"
+#define DEBUG_MACINFO_SECTION"__DWARF,__debug_macinfo,regular,debug"
+#define DEBUG_LINE_SECTION   "__DWARF,__debug_line,regular,debug"
+#define DEBUG_LOC_SECTION"__DWARF,__debug_loc,regular,debug"
+#define DEBUG_LOCLISTS_SECTION"__DWARF,__debug_loclists,regular,debug"
+
+#define DEBUG_STR_SECTION"__DWARF,__debug_str,regular,debug"
+#define DEBUG_STR_OFFSETS_SECTION "__DWARF,__debug_str_offs,regular,debug"
+#define DEBUG_RANGES_SECTION "__DWARF,__debug_ranges,regular,debug"
+#define DEBUG_RNGLISTS_SECTION"__DWARF,__debug_rnglists,regular,debug"
+#define DEBUG_MACRO_SECTION   "__DWARF,__debug_macro,regular,debug"
 
 #define DEBUG_LTO_INFO_SECTION   "__GNU_DWARF_LTO,__debug_info,regular,debug"
 #define DEBUG_LTO_ABBREV_SECTION  
"__GNU_DWARF_LTO,__debug_abbrev,regular,debug"
@@ -457,6 +459,13 @@ extern GTY(()) int darwin_ms_struct;
 #define DEBUG_LTO_MACRO_SECTION   "__GNU_DWARF_LTO,__debug_macro,regular,debug"
 
 #define TARGET_WANT_DEBUG_PUB_SECTIONS true
+#define DEBUG_PUBNAMES_SECTION   ((debug_generate_pub_sections == 2) \
+   ? "__DWARF,__debug_gnu_pubn,regular,debug" \
+   : "__DWARF,__debug_pubnames,regular,debug")
+
+#define DEBUG_PUBTYPES_SECTION   ((debug_generate_pub_sections == 2) \
+   ? "__DWARF,__debug_gnu_pubt,regular,debug" \
+   : "__DWARF,__debug_pubtypes,regular,debug")
 
 /* When generating stabs debugging, use N_BINCL entries.  */
 
-- 
2.17.1




Re: [PATCH] libstdc++-v3: Have aligned_alloc() on Newlib

2018-08-14 Thread Szabolcs Nagy

On 13/08/18 19:57, Jonathan Wakely wrote:

On 13/08/18 13:04 +0100, Jonathan Wakely wrote:

On 13/08/18 12:55 +0100, Szabolcs Nagy wrote:

On 09/08/18 10:08, Jonathan Wakely wrote:

Yes please, on trunk and 7 and 8. It's better to use the standard
aligned_alloc if available.



but the newlib aligned_alloc is broken on baremetal targets,
it is implemented using posix_memalign which is not provided
by the newlib malloc implementation (except on cygwin)


Ouch, OK, let's revert it. Using memalign is fine.

The original problem that I think Sebastian was trying to solve should
be fixed by r263409 anyway (and was backported to all branches).


I've committed this to trunk and will do so on the branches too.



thanks, this fixed the issue for me on trunk.


Re: [PATCH] Rope iterators: don't retain pointers when copied

2018-08-14 Thread Jonathan Wakely

On 20/05/18 21:28 -0700, Jeremy Sawicki wrote:

Rope iterators sometimes contain pointers to an internal buffer
inside the iterator itself.  When such an iterator is copied, the
copy incorrectly retains pointers to the original.

This patch takes the simple approach of not copying the cached
information when the internal buffer is being used, instead
requiring it to be recomputed when the copied iterator is
dereferenced.  An alternative would be to adjust the pointers so
they refer to the buffer in the copy.

I tested on Linux x64 with "/configure", "make bootstrap",
and "make -k check", three times: (1) with no changes (as a
baseline), (2) with only the new test (to make sure it fails), and
(3) with the new test and bug fix (to make sure the test passes and
nothing else changes).


* include/ext/rope (_Rope_iterator_base(const _Rope_iterator_base&))
(_Rope_const_iterator::operator=(const _Rope_const_iterator&))
(_Rope_iterator::operator=(const _Rope_iterator&)):
Copied/assigned rope iterators don't retain pointers to the
iterator they were copied/assigned from.
* testsuite/ext/rope/7.cc: New.


Jeremy's copyright assignment is complete, so I'm committing this
patch. Tested x86_64-linux.

Thanks very much!




Index: libstdc++-v3/include/ext/rope
===
--- libstdc++-v3/include/ext/rope   (revision 260403)
+++ libstdc++-v3/include/ext/rope   (working copy)
@@ -1119,7 +1119,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION

  _Rope_iterator_base(const _Rope_iterator_base& __x)
  {
-if (0 != __x._M_buf_ptr)
+if (0 != __x._M_buf_ptr && __x._M_buf_start != __x._M_tmp_buf)
  *this = __x;
else
  {
@@ -1166,7 +1166,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
  _Rope_const_iterator&
  operator=(const _Rope_const_iterator& __x)
  {
-if (0 != __x._M_buf_ptr)
+if (0 != __x._M_buf_ptr && __x._M_buf_start != __x._M_tmp_buf)
  *(static_cast<_Rope_iterator_base<_CharT, _Alloc>*>(this)) = __x;
else
  {
@@ -1345,7 +1345,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_RopeRep* __old = this->_M_root;

_RopeRep::_S_ref(__x._M_root);
-if (0 != __x._M_buf_ptr)
+if (0 != __x._M_buf_ptr && __x._M_buf_start != __x._M_tmp_buf)
  {
_M_root_rope = __x._M_root_rope;
*(static_cast<_Rope_iterator_base<_CharT, _Alloc>*>(this)) = __x;
Index: libstdc++-v3/testsuite/ext/rope/7.cc
===
--- libstdc++-v3/testsuite/ext/rope/7.cc(nonexistent)
+++ libstdc++-v3/testsuite/ext/rope/7.cc(working copy)
@@ -0,0 +1,95 @@
+// Copyright (C) 2018 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
+// .
+
+// rope (SGI extension)
+
+// { dg-do run }
+
+#include 
+#include 
+
+void
+test01()
+{
+  using __gnu_cxx::crope;
+
+  char str_a[] =
+""
+""
+""
+""
+"";
+  char str_b[] =
+""
+""
+""
+""
+"";
+
+  // Create ropes with leaf nodes longer than __lazy_threshold = 128
+  // so substring nodes will be created by the next step
+  crope leaf_rope_a(str_a);
+  crope leaf_rope_b(str_b);
+
+  // Create ropes with substring nodes referencing the leaf nodes
+  // of the prior ropes
+  crope substring_rope_a(leaf_rope_a.begin() + 1,
+ leaf_rope_a.begin() + 199);
+  crope substring_rope_b(leaf_rope_b.begin() + 1,
+ leaf_rope_b.begin() + 199);
+
+  // Create iterators to substring_rope_a
+  crope::const_iterator cit_orig = substring_rope_a.begin();
+  crope::iterator mit_orig = substring_rope_a.mutable_begin();
+
+  // Dereference the iterators so they cache a portion of the substring
+  // node in their internal buffers
+  *cit_orig;
+  *mit_orig;
+
+  // Copy the 

[PATCH] Fix P81033 for FDEs in partitioned code.

2018-08-14 Thread Iain Sandoe
Hi,

A more detailed explanation is in 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81033#c37

When function sub-sections are enabled, Darwin’s assembler needs the FDE local 
start
label for each sub-section to follow a linker-visible one so that the FDE will 
be correctly
associated with the code of the subsection.

The current code in final.c emits a linker-visible symbol, as needed by several 
targets.
However the local label used to define the FDE start precedes the 
linker-visible one
which, for Darwin causes it (the FDE start) to be associated with the previous 
linker-
visible symbol (or the section start if there isn’t one).  This applies 
regardless of the 
actual address of the label, for toolchain assemblers that have strict 
interpretation of
the Darwin sub-sections-via-symbols ABI.

The patch adds a new local label (analogous to the "LFBn” emitted for the 
regular
function starts) just after the linker-visible label emitted after switching 
text section.
The FDE second entry is made to point to this instead of the LcoldStartn one.  
This
should be a no-op for targets using .cfi_ and for targets without 
sub-sections-via-symbols.

Bootstrapped on x86_64 and i686 linux and on a number of Darwin platforms (from 
i686-darwin9 to x86_64-darwin17).

OK for trunk?
open branches? (although it's a regression on 8, it’s a latent wrong-code on 
all branches)

thanks
Iain

2018-08-14  Iain Sandoe  

gcc:

PR target/81033
* dwarf2out.c (FUNC_SECOND_SECT_LABEL): New.
(dwarf2out_switch_text_section): Generate subsection label and
assign it to the FDE second section start.
* final.c (final_scan_insn_1): Emit second section label when
required.
---
 gcc/dwarf2out.c | 11 +--
 gcc/final.c |  3 +++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 9ed473088e..c7557eb51d 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -297,6 +297,10 @@ static unsigned int rnglist_idx;
 #define FUNC_BEGIN_LABEL   "LFB"
 #endif
 
+#ifndef FUNC_SECOND_SECT_LABEL
+#define FUNC_SECOND_SECT_LABEL "LFSB"
+#endif
+
 #ifndef FUNC_END_LABEL
 #define FUNC_END_LABEL "LFE"
 #endif
@@ -1212,21 +1216,24 @@ static void set_cur_line_info_table (section *);
 void
 dwarf2out_switch_text_section (void)
 {
+  char label[MAX_ARTIFICIAL_LABEL_BYTES];
   section *sect;
   dw_fde_ref fde = cfun->fde;
 
   gcc_assert (cfun && fde && fde->dw_fde_second_begin == NULL);
 
+  ASM_GENERATE_INTERNAL_LABEL (label, FUNC_SECOND_SECT_LABEL,
+  current_function_funcdef_no);
+
+  fde->dw_fde_second_begin = xstrdup (label);
   if (!in_cold_section_p)
 {
   fde->dw_fde_end = crtl->subsections.cold_section_end_label;
-  fde->dw_fde_second_begin = crtl->subsections.hot_section_label;
   fde->dw_fde_second_end = crtl->subsections.hot_section_end_label;
 }
   else
 {
   fde->dw_fde_end = crtl->subsections.hot_section_end_label;
-  fde->dw_fde_second_begin = crtl->subsections.cold_section_label;
   fde->dw_fde_second_end = crtl->subsections.cold_section_end_label;
 }
   have_multiple_function_sections = true;
diff --git a/gcc/final.c b/gcc/final.c
index 842e5e067d..6943c073d9 100644
--- a/gcc/final.c
+++ b/gcc/final.c
@@ -2232,6 +2232,9 @@ final_scan_insn_1 (rtx_insn *insn, FILE *file, int 
optimize_p ATTRIBUTE_UNUSED,
  ASM_OUTPUT_LABEL (asm_out_file,
IDENTIFIER_POINTER (cold_function_name));
 #endif
+ if (dwarf2out_do_frame ()
+ && cfun->fde->dw_fde_second_begin != NULL)
+   ASM_OUTPUT_LABEL (asm_out_file, cfun->fde->dw_fde_second_begin);
}
  break;
 
-- 
2.17.1



[PATCH] Fix PR19315.

2018-08-14 Thread Iain Sandoe
Hi,

As noted in the PR, GCC C currently has an "undocumented extension" that
silently makes zero-sized static objects into 'extern' as well as
'static’.  The warning that would normally be emitted is suppressed without
 ‘-pedantic’.

This means that they don't get allocated (appearing in the object as an
undefined extern).  This is surprising behaviour and can cause assembly-
time fails on at least one target, since such arrays can still be
referenced in the generated code.  For most targets, presumably there
would be a link-time fail - also a 'surprising' result for the end user.

Effectively,  ISTM we accept wrong code silently without the “-pedantic” flag.

It would seem that if there was a compelling use-case for this, then we
should expose it via a flag, rather than making it the default.

The patch removes the code marking these objects as extern.

Two tests in the test-suite needed change, neither of which appears to
be intentionally testing this "extension".  The amended graphite test
generates identical code.

Bootstrapped on x86_64-linux-gnu (all langs), x86_64-apple-darwin.

OK for trunk?

iain

2018-08-14  Iain Sandoe 

gcc/c:

PR c/19315
* c-decl.c (finish_decl): Don't add the 'extern' storage class to
objects of unknown size.

gcc/testsuite:

PR c/19315
gcc.dg/graphite/pr82451.c: Make array 'a' an extern.
gcc.dg/redecl-10.c: Expect warnings for the static vars with unknown
size.
---
 gcc/c/c-decl.c  | 8 
 gcc/testsuite/gcc.dg/graphite/pr82451.c | 2 +-
 gcc/testsuite/gcc.dg/redecl-10.c| 4 ++--
 3 files changed, 3 insertions(+), 11 deletions(-)

diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index ed1dd28cac..da42add073 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -4969,14 +4969,6 @@ finish_decl (tree decl, location_t init_loc, tree init,
case 2:
  if (do_default)
error ("array size missing in %q+D", decl);
- /* If a `static' var's size isn't known,
-make it extern as well as static, so it does not get
-allocated.
-If it is not `static', then do not mark extern;
-finish_incomplete_decl will give it a default size
-and it will get allocated.  */
- else if (!pedantic && TREE_STATIC (decl) && !TREE_PUBLIC (decl))
-   DECL_EXTERNAL (decl) = 1;
  break;
 
case 3:
diff --git a/gcc/testsuite/gcc.dg/graphite/pr82451.c 
b/gcc/testsuite/gcc.dg/graphite/pr82451.c
index 802b931fdd..b2c439bb56 100644
--- a/gcc/testsuite/gcc.dg/graphite/pr82451.c
+++ b/gcc/testsuite/gcc.dg/graphite/pr82451.c
@@ -1,7 +1,7 @@
 /* { dg-do compile } */
 /* { dg-options "-O -floop-parallelize-all" } */
 
-static int a[];
+extern int a[];
 int b[1];
 int c;
 static void
diff --git a/gcc/testsuite/gcc.dg/redecl-10.c b/gcc/testsuite/gcc.dg/redecl-10.c
index 525961e7e3..0864311f64 100644
--- a/gcc/testsuite/gcc.dg/redecl-10.c
+++ b/gcc/testsuite/gcc.dg/redecl-10.c
@@ -5,7 +5,7 @@
 /* { dg-do compile } */
 /* { dg-options "-g" } */
 
-static int w[];
+static int w[]; /* { dg-warning "array 'w' assumed to have one element" } */
 void
 f (void)
 {
@@ -19,7 +19,7 @@ g (void)
   extern int x[] = { 3, 4, 5 }; /* { dg-error "has both" } */
 }
 
-static int y[];
+static int y[]; /* { dg-warning "array 'y' assumed to have one element" } */
 void
 h (void)
 {
-- 
2.17.1




Re: [PING] [PATCH] Fix wrong code with truncated string literals (PR 86711/86714)

2018-08-14 Thread Bernd Edlinger
On 08/13/18 16:27, Martin Sebor wrote:
> As I said below, the patch for PR 86552, 86711, 86714 that
> was first posted on July 19 fixes both of these issues and
> also diagnoses calls with unterminated strings:
> 
>    https://gcc.gnu.org/ml/gcc-patches/2018-08/msg00155.html
> 

Sorry, but you you read my comment on your patch here:

https://gcc.gnu.org/ml/gcc-patches/2018-08/msg00184.html



So I am strictly opposed to implementing new warnings now,
which just papers over existing design issues in this area.

I think the most important problem in the strlen folding is that
c_strlen and get_range_strlen do not have the the information
what kind of string length they should return,
(as measured in char/wchar16/wchar32)
Most of the time only length in char makes any sense.

Only for sprintf processing a different char type may be
requested, but the sprintf code in the middle end does not
know which is the wchar type.

I have sent a followup patch that passes char size units
to c_strlen and get_range_strlen here:
https://gcc.gnu.org/ml/gcc-patches/2018-08/msg00795.html

I hope you understand, that I would like to fix design issues
before implementing new features, since that will obviously
become more difficult when even more features are being
implemented without fixing some basics first.


Bernd.

[PATCH] Add a character size parameter to c_strlen/get_range_strlen

2018-08-14 Thread Bernd Edlinger
Hi,

this patch is a follow-up to my patch here:
https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01800.html

Since most calls of c_strlen and get_range_strlen expect
a string length in bytes of a zero-terminated string, there is
a need for a new parameter eltsize, which is per default 1,
but can be used in gimple-ssa-sprintf.c to specify the
expected character size.

Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?


Thanks
Bernd.2018-08-14  Bernd Edlinger  

	* builtins.c (c_strlen): Add new parameter eltsize.
	* builtins.h (c_strlen): Adjust prototype.
	* expr.c (string_constant): Add new parameter mem_size.
	* expr.h (string_constant): Adjust protoype.
	* gimple-fold.c (get_range_strlen): Add new parameter eltsize.
	* gimple-fold.h (get_range_strlen): Adjust prototype.
	* gimple-ssa-sprintf.c (get_string_length): Add new parameter eltsize.
	(format_string): Call get_string_length with eltsize.

2018-08-14  Bernd Edlinger  

	* gcc.dg/strlenopt-49.c: Adjust test case.
	* gcc.dg/tree-ssa/builtin-sprintf-warn-18.c: Likewise.

diff -pur gcc-9-20180812-1/gcc/builtins.c gcc-9-20180812-2/gcc/builtins.c
--- gcc-9-20180812-1/gcc/builtins.c	2018-08-13 20:57:43.318627928 +0200
+++ gcc-9-20180812-2/gcc/builtins.c	2018-08-14 06:22:14.547504925 +0200
@@ -568,13 +568,13 @@ string_length (const void *ptr, unsigned
accesses.  Note that this implies the result is not going to be emitted
into the instruction stream.
 
-   The value returned is of type `ssizetype'.
+   ELTSIZE is 1 for normal single byte character strings, and 2 or
+   4 for wide characer strings.  ELTSIZE is by default 1.
 
-   Unfortunately, string_constant can't access the values of const char
-   arrays with initializers, so neither can we do so here.  */
+   The value returned is of type `ssizetype'.  */
 
 tree
-c_strlen (tree src, int only_value)
+c_strlen (tree src, int only_value, unsigned eltsize)
 {
   STRIP_NOPS (src);
   if (TREE_CODE (src) == COND_EXPR
@@ -582,27 +582,28 @@ c_strlen (tree src, int only_value)
 {
   tree len1, len2;
 
-  len1 = c_strlen (TREE_OPERAND (src, 1), only_value);
-  len2 = c_strlen (TREE_OPERAND (src, 2), only_value);
+  len1 = c_strlen (TREE_OPERAND (src, 1), only_value, eltsize);
+  len2 = c_strlen (TREE_OPERAND (src, 2), only_value, eltsize);
   if (tree_int_cst_equal (len1, len2))
 	return len1;
 }
 
   if (TREE_CODE (src) == COMPOUND_EXPR
   && (only_value || !TREE_SIDE_EFFECTS (TREE_OPERAND (src, 0
-return c_strlen (TREE_OPERAND (src, 1), only_value);
+return c_strlen (TREE_OPERAND (src, 1), only_value, eltsize);
 
   location_t loc = EXPR_LOC_OR_LOC (src, input_location);
 
   /* Offset from the beginning of the string in bytes.  */
   tree byteoff;
-  src = string_constant (src, );
+  tree memsize;
+  src = string_constant (src, , );
   if (src == 0)
 return NULL_TREE;
 
   /* Determine the size of the string element.  */
-  unsigned eltsize
-= tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (src;
+  if (eltsize != tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (src)
+return NULL_TREE;
 
   /* Set MAXELTS to sizeof (SRC) / sizeof (*SRC) - 1, the maximum possible
  length of SRC.  Prefer TYPE_SIZE() to TREE_STRING_LENGTH() if possible
@@ -613,14 +614,10 @@ c_strlen (tree src, int only_value)
   HOST_WIDE_INT strelts = TREE_STRING_LENGTH (src);
   strelts = strelts / eltsize - 1;
 
-  HOST_WIDE_INT maxelts = strelts;
-  tree type = TREE_TYPE (src);
-  if (tree size = TYPE_SIZE_UNIT (type))
-if (tree_fits_shwi_p (size))
-  {
-	maxelts = tree_to_shwi (size);
-	maxelts = maxelts / eltsize - 1;
-  }
+  if (!tree_fits_uhwi_p (memsize))
+return NULL_TREE;
+
+  HOST_WIDE_INT maxelts = tree_to_uhwi (memsize) / eltsize - 1;
 
   /* PTR can point to the byte representation of any string type, including
  char* and wchar_t*.  */
@@ -628,19 +625,23 @@ c_strlen (tree src, int only_value)
 
   if (byteoff && TREE_CODE (byteoff) != INTEGER_CST)
 {
+  /* For empty strings the result should be zero.  */
+  if (maxelts == 0)
+	return ssize_int (0);
+
+  /* The code below works only for single byte character types.  */
+  if (eltsize != 1)
+	return NULL_TREE;
+
   /* If the string has an internal NUL character followed by any
 	 non-NUL characters (e.g., "foo\0bar"), we can't compute
 	 the offset to the following NUL if we don't know where to
 	 start searching for it.  */
   unsigned len = string_length (ptr, eltsize, strelts);
-  if (len < strelts)
-	{
-	  /* Return when an embedded null character is found.  */
-	  return NULL_TREE;
-	}
 
-  if (!maxelts)
-	return ssize_int (0);
+  /* Return when an embedded null character is found or none at all.  */
+  if (len < strelts || len > maxelts)
+	return NULL_TREE;
 
   /* We don't know the starting offset, but we do know that the string
 	 has no internal zero bytes.  If the offset falls within the bounds
@@ 

Re: Two fixes for pretty-print.c for mingw-w64

2018-08-14 Thread JonY
On 08/14/2018 05:55 AM, Liu Hao wrote:
> 在 2018/8/14 13:54, Liu Hao 写道:
>> The two patches attached have addressed two issues in the ANSI escape
>> sequence translator I sent before.  Please review, and consider
>> backporting these to gcc-8-branch.
>>
>>
>>
>>
> 
> And here are SVN changelogs for these two patches, in this order:
> 
> 
> 
> * gcc/pretty-print.c (mingw_ansi_fputs): Do not call _close() on the handle
> returned by _get_osf_handle().
> 
> 
> 
> * gcc/pretty-print.c (eat_esc_sequence): Swap the foreground and background
> colors if the COMMON_LVB_REVERSE_VIDEO flag is set, and clear it
> thereafter,
> as it only works for DBCS.
> 
> 
> 

Patch and log looks OK.




signature.asc
Description: OpenPGP digital signature


Re: [arm-8-branch] Add Linaro version string and macros

2018-08-14 Thread Ramana Radhakrishnan
On Fri, Aug 10, 2018 at 5:00 PM, Yvan Roux  wrote:
> On Fri, 10 Aug 2018 at 17:01, Ramana Radhakrishnan
>  wrote:
>>
>> On Fri, Aug 10, 2018 at 3:35 PM, Yvan Roux  wrote:
>> > On Fri, 10 Aug 2018 at 14:31, Yvan Roux  wrote:
>> >>
>> >> On Fri, 10 Aug 2018 at 13:45, Ramana Radhakrishnan
>> >>  wrote:
>> >> >
>> >> > On Fri, Aug 10, 2018 at 11:09 AM, Yvan Roux  
>> >> > wrote:
>> >> > > Hi,
>> >> > >
>> >> > > This patch adds Linaro version string and release macros to ARM GCC 8
>> >> > > vendor branch.
>> >> > >
>> >> > > Ok to commit?
>> >> > >
>> >> >
>> >> >
>> >> > Ok if no regressions. (I'm assuming you've built and eyeballed that
>> >> > the pre-processor macros are being produced). (I have a patch to
>> >> > www-docs for this branch that I'm writing up and should try and get
>> >> > out today. Though it would be nice to have tests for these if
>> >> > possible.
>> >>
>> >> I've not passed the regression testsuite since this patch is part of
>> >> Linaro vendor branches for a long time, I've just built an x86_64 c
>> >> compiler from arm-8-branch and verified the version string and macros:
>> >>
>> >> $ ~/wip/arm-8-install/bin/gcc --version
>> >> gcc (Linaro GCC 8.2-2018.08~dev) 8.2.1 20180802
>> >>
>> >> $ ~/wip/arm-8-install/bin/gcc -E -dM - < /dev/null | grep LINARO
>> >> #define __LINARO_SPIN__ 0
>> >> #define __LINARO_RELEASE__ 201808
>> >>
>> >> I can add some tests, but it will take some time to remember me how
>> >> these kind of thing is tested in the testsuite ;)
>> >
>> > Updated version of the patch, with a test case for Linaro macros.  The
>> > test is not strict w/r to the version or spin number to avoid having
>> > to update it every release or re-spin, do you think it is sufficient ?
>> >
>> > Testsuite ran with the included test PASS.
>> >
>> > gcc/ChangeLog
>> > 2018-08-10  Yvan Roux  
>> >
>> > * LINARO-VERSION: New file.
>> > * configure.ac: Add Linaro version string.
>> > * configure: Regenerate.
>> > * Makefile.in (LINAROVER, LINAROVER_C, LINAROVER_S): Define.
>> > (CFLAGS-cppbuiltin.o): Add LINAROVER macro definition.
>> > (cppbuiltin.o): Depend on $(LINAROVER).
>> > * cppbuiltin.c (parse_linarover): New.
>> > (define_GNUC__): Define __LINARO_RELEASE__ and __LINARO_SPIN__ macros.
>> >
>> > gcc/testsuite/ChangeLog
>> > 2018-08-10  Yvan Roux  
>> >
>> > * gcc.dg/cpp/linaro-macros.c: New test.
>>
>>
>> Looks good, thanks  - can you put the Changelog in a Changelog.arm file ?
>
> Sure, does it need the FSF Copyright lines at the end like other ChangeLogs ?

Yep it's like any other upstream branch.

ramana
>
> Thanks
> Yvan
>
>> regards
>> Ramana


[PATCH] Use more DECL_BUILT_IN_P macro.

2018-08-14 Thread Martin Liška
Hi.

The patch adds more usages of the new macro. I hope it improves
readability of code.

Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

Ready to be installed?
Martin

gcc/ChangeLog:

2018-08-10  Martin Liska  

* tree.h (DECL_BUILT_IN_P): Add also check
for TREE_CODE (NODE) == FUNCTION_DECL.
* builtins.c (fold_call_expr): Use DECL_BUILT_IN_P macro.
(fold_builtin_call_array): Likewise.
* cgraph.c (cgraph_update_edges_for_call_stmt_node): Likewise.
(cgraph_edge::verify_corresponds_to_fndecl): Likewise.
(cgraph_node::verify_node): Likewise.
* cgraphclones.c (cgraph_node::create_clone): Likewise.
* dse.c (scan_insn): Likewise.
* fold-const.c (fold_binary_loc): Likewise.
* gimple-pretty-print.c (dump_gimple_call): Likewise.
* gimple.c (gimple_call_builtin_p): Likewise.
* gimplify.c (gimplify_call_expr): Likewise.
(gimple_boolify): Likewise.
(gimplify_modify_expr): Likewise.
* ipa-fnsummary.c (compute_fn_summary): Likewise.
* omp-low.c (setjmp_or_longjmp_p): Likewise.
* trans-mem.c (is_tm_irrevocable): Likewise.
(is_tm_abort): Likewise.
* tree-cfg.c (stmt_can_terminate_bb_p): Likewise.
* tree-inline.c (copy_bb): Likewise.
* tree-sra.c (scan_function): Likewise.
* tree-ssa-ccp.c (optimize_stack_restore): Likewise.
(pass_fold_builtins::execute): Likewise.
* tree-ssa-dom.c (dom_opt_dom_walker::optimize_stmt): Likewise.
* tree-ssa-loop-im.c (stmt_cost): Likewise.
* tree-stdarg.c (optimize_va_list_gpr_fpr_size): Likewise.

gcc/c/ChangeLog:

2018-08-10  Martin Liska  

* c-parser.c (c_parser_postfix_expression_after_primary):
Use DECL_BUILT_IN_P macro.

gcc/cp/ChangeLog:

2018-08-10  Martin Liska  

* constexpr.c (constexpr_fn_retval): Use DECL_BUILT_IN_P macro.
(cxx_eval_builtin_function_call): Likewise.
* cp-gimplify.c (cp_gimplify_expr): Likewise.
(cp_fold): Likewise.
* semantics.c (finish_call_expr): Likewise.
* tree.c (builtin_valid_in_constant_expr_p): Likewise.
---
 gcc/builtins.c| 10 ++
 gcc/c/c-parser.c  |  9 +++--
 gcc/cgraph.c  | 13 ++---
 gcc/cgraphclones.c|  4 ++--
 gcc/cp/constexpr.c| 12 +---
 gcc/cp/cp-gimplify.c  | 12 
 gcc/cp/semantics.c|  4 +---
 gcc/cp/tree.c |  5 ++---
 gcc/dse.c |  5 ++---
 gcc/fold-const.c  |  4 +---
 gcc/gimple-pretty-print.c |  4 +---
 gcc/gimple.c  |  3 +--
 gcc/gimplify.c| 14 --
 gcc/ipa-fnsummary.c   |  8 
 gcc/omp-low.c |  5 ++---
 gcc/trans-mem.c   |  8 ++--
 gcc/tree-cfg.c|  3 +--
 gcc/tree-inline.c |  4 ++--
 gcc/tree-sra.c|  4 ++--
 gcc/tree-ssa-ccp.c|  8 ++--
 gcc/tree-ssa-dom.c|  4 +---
 gcc/tree-ssa-loop-im.c|  4 +---
 gcc/tree-stdarg.c |  6 ++
 gcc/tree.h|  4 +++-
 24 files changed, 56 insertions(+), 101 deletions(-)


diff --git a/gcc/builtins.c b/gcc/builtins.c
index 867d153d798..e6279979544 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -9629,10 +9629,7 @@ fold_call_expr (location_t loc, tree exp, bool ignore)
   if (nargs && TREE_CODE (CALL_EXPR_ARG (exp, nargs - 1)) == CALL_EXPR)
 	{
 	  tree fndecl2 = get_callee_fndecl (CALL_EXPR_ARG (exp, nargs - 1));
-	  if (fndecl2
-	  && TREE_CODE (fndecl2) == FUNCTION_DECL
-	  && DECL_BUILT_IN_CLASS (fndecl2) == BUILT_IN_NORMAL
-	  && DECL_FUNCTION_CODE (fndecl2) == BUILT_IN_VA_ARG_PACK)
+	  if (DECL_BUILT_IN_P (fndecl2, BUILT_IN_NORMAL, BUILT_IN_VA_ARG_PACK))
 	return NULL_TREE;
 	}
 
@@ -9675,10 +9672,7 @@ fold_builtin_call_array (location_t loc, tree,
   if (n && TREE_CODE (argarray[n - 1]) == CALL_EXPR)
 	{
 	  tree fndecl2 = get_callee_fndecl (argarray[n - 1]);
-	  if (fndecl2
-	  && TREE_CODE (fndecl2) == FUNCTION_DECL
-	  && DECL_BUILT_IN_CLASS (fndecl2) == BUILT_IN_NORMAL
-	  && DECL_FUNCTION_CODE (fndecl2) == BUILT_IN_VA_ARG_PACK)
+	  if (DECL_BUILT_IN_P (fndecl2, BUILT_IN_NORMAL, BUILT_IN_VA_ARG_PACK))
 	return NULL_TREE;
 	}
   if (avoid_folding_inline_builtin (fndecl))
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 7a926285f3a..6e44ee7e366 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -9159,9 +9159,7 @@ c_parser_postfix_expression_after_primary (c_parser *parser,
 	  expr.value, exprlist,
 	  sizeof_arg,
 	  sizeof_ptr_memacc_comptypes);
-	  if (TREE_CODE (expr.value) == FUNCTION_DECL
-	  && DECL_BUILT_IN_CLASS (expr.value) == BUILT_IN_NORMAL
-	  && DECL_FUNCTION_CODE (expr.value) == BUILT_IN_MEMSET
+	  if (DECL_BUILT_IN_P (expr.value, BUILT_IN_NORMAL, BUILT_IN_MEMSET)
 	  && vec_safe_length (exprlist) == 3)
 	{
 

[PATCH 0/3] Improvements to switch expansion code

2018-08-14 Thread marxin
Hi.

With my rapid simplification of balanced tree emission we regressed
on number of tests needed (PR 86847). Apart from that profile info
was missing (INV) for jump tables and bit tests. That's fixed in another
2 patches. I'm explaining details of the patches in corresponding emails.

Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

marxin (3):
  Fix probabilities for jump table (PR tree-optimization/86702).
  Fix probability for bit-tests.
  Improve switch code emission for a balanced tree (PR
tree-optimization/86847).

 gcc/testsuite/gcc.dg/tree-ssa/switch-2.c |  25 ++
 gcc/testsuite/gcc.dg/tree-ssa/switch-3.c |  20 ++
 gcc/testsuite/gcc.dg/tree-ssa/vrp105.c   |  37 ---
 gcc/tree-switch-conversion.c | 329 +++
 gcc/tree-switch-conversion.h |  47 +++-
 5 files changed, 366 insertions(+), 92 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/switch-2.c
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/switch-3.c
 delete mode 100644 gcc/testsuite/gcc.dg/tree-ssa/vrp105.c

-- 
2.18.0



[PATCH 1/3] Fix probabilities for jump table (PR tree-optimization/86702).

2018-08-14 Thread marxin
The patch set even probability to jump tables based on number of
cases which are handled on each edge.

gcc/ChangeLog:

2018-08-06  Martin Liska  

PR tree-optimization/86702
* tree-switch-conversion.c (jump_table_cluster::emit):
Make probabilities even for values in jump table
according to number of cases handled.
(switch_decision_tree::compute_cases_per_edge): Pass
argument to reset_out_edges_aux function.
(switch_decision_tree::analyze_switch_statement): Likewise.
* tree-switch-conversion.h (switch_decision_tree::reset_out_edges_aux):
Make it static.
---
 gcc/tree-switch-conversion.c | 40 ++--
 gcc/tree-switch-conversion.h | 11 +-
 2 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/gcc/tree-switch-conversion.c b/gcc/tree-switch-conversion.c
index 492cd365a30..0b5c93118f6 100644
--- a/gcc/tree-switch-conversion.c
+++ b/gcc/tree-switch-conversion.c
@@ -1064,6 +1064,9 @@ void
 jump_table_cluster::emit (tree index_expr, tree,
 			  tree default_label_expr, basic_block default_bb)
 {
+  unsigned HOST_WIDE_INT range = get_range (get_low (), get_high ());
+  unsigned HOST_WIDE_INT nondefault_range = 0;
+
   /* For jump table we just emit a new gswitch statement that will
  be latter lowered to jump table.  */
   auto_vec  labels;
@@ -1080,6 +1083,39 @@ jump_table_cluster::emit (tree index_expr, tree,
 unshare_expr (default_label_expr), labels);
   gimple_stmt_iterator gsi = gsi_start_bb (m_case_bb);
   gsi_insert_after (, s, GSI_NEW_STMT);
+
+  /* Set up even probabilities for all cases.  */
+  for (unsigned i = 0; i < m_cases.length (); i++)
+{
+  simple_cluster *sc = static_cast (m_cases[i]);
+  edge case_edge = find_edge (m_case_bb, sc->m_case_bb);
+  unsigned HOST_WIDE_INT case_range
+	= sc->get_range (sc->get_low (), sc->get_high ());
+  nondefault_range += case_range;
+
+  /* case_edge->aux is number of values in a jump-table that are covered
+	 by the case_edge.  */
+  case_edge->aux = (void *) ((intptr_t) (case_edge->aux) + case_range);
+}
+
+  edge default_edge = gimple_switch_default_edge (s);
+  default_edge->probability = profile_probability::never ();
+
+  for (unsigned i = 0; i < m_cases.length (); i++)
+{
+  simple_cluster *sc = static_cast (m_cases[i]);
+  edge case_edge = find_edge (m_case_bb, sc->m_case_bb);
+  case_edge->probability
+	= profile_probability::always ().apply_scale ((intptr_t)case_edge->aux,
+		  range);
+}
+
+  /* Number of non-default values is probability of default edge.  */
+  default_edge->probability
++= profile_probability::always ().apply_scale (nondefault_range,
+		   range).invert ();
+
+  switch_decision_tree::reset_out_edges_aux (s);
 }
 
 /* Find jump tables of given CLUSTERS, where all members of the vector
@@ -1571,7 +1607,7 @@ bit_test_cluster::hoist_edge_and_branch_if_true (gimple_stmt_iterator *gsip,
 void
 switch_decision_tree::compute_cases_per_edge ()
 {
-  reset_out_edges_aux ();
+  reset_out_edges_aux (m_switch);
   int ncases = gimple_switch_num_labels (m_switch);
   for (int i = ncases - 1; i >= 1; --i)
 {
@@ -1614,7 +1650,7 @@ switch_decision_tree::analyze_switch_statement ()
   m_case_bbs.quick_push (case_edge->dest);
 }
 
-  reset_out_edges_aux ();
+  reset_out_edges_aux (m_switch);
 
   /* Find jump table clusters.  */
   vec output = jump_table_cluster::find_jump_tables (clusters);
diff --git a/gcc/tree-switch-conversion.h b/gcc/tree-switch-conversion.h
index 4beac785f05..af2f47a07e6 100644
--- a/gcc/tree-switch-conversion.h
+++ b/gcc/tree-switch-conversion.h
@@ -513,10 +513,6 @@ struct switch_decision_tree
   /* Attempt to expand CLUSTERS as a decision tree.  Return true when
  expanded.  */
   bool try_switch_expansion (vec );
-
-  /* Reset the aux field of all outgoing edges of switch basic block.  */
-  inline void reset_out_edges_aux ();
-
   /* Compute the number of case labels that correspond to each outgoing edge of
  switch statement.  Record this information in the aux field of the edge.
  */
@@ -576,6 +572,9 @@ struct switch_decision_tree
 	  basic_block label_bb,
 	  profile_probability prob);
 
+  /* Reset the aux field of all outgoing edges of switch basic block.  */
+  static inline void reset_out_edges_aux (gswitch *swtch);
+
   /* Switch statement.  */
   gswitch *m_switch;
 
@@ -838,9 +837,9 @@ struct switch_conversion
 };
 
 void
-switch_decision_tree::reset_out_edges_aux ()
+switch_decision_tree::reset_out_edges_aux (gswitch *swtch)
 {
-  basic_block bb = gimple_bb (m_switch);
+  basic_block bb = gimple_bb (swtch);
   edge e;
   edge_iterator ei;
   FOR_EACH_EDGE (e, ei, bb->succs)


[PATCH 3/3] Improve switch code emission for a balanced tree (PR tree-optimization/86847).

2018-08-14 Thread marxin
This is the most complex patch. It follows original implementation and
does following improvements that were part of original code:

a) for a node with both children (that don't have children) and only single case
values handled: emit series of 3 compares and jump to default
b) for a node with only one child (that doesn't have a child) and only single
case values handled: emit 2 compares and jump to default
c) for a node of a range without a child, emit if (index - low) <= (high - low))
d) profile emission is precise, taken also from previous implementation

These changes + VRP should move us back to code quality we had in GCC 8.

gcc/ChangeLog:

2018-08-13  Martin Liska  

PR tree-optimization/86847
* tree-switch-conversion.c (switch_decision_tree::dump_case_nodes):
Dump also subtree probability.
(switch_decision_tree::do_jump_if_equal): New function.
(switch_decision_tree::emit_case_nodes): Handle special
situations in balanced tree that can be emitted much simpler.
Fix calculation of probabilities that happen in tree expansion.
* tree-switch-conversion.h (struct cluster): Add
is_single_value_p.
(struct simple_cluster): Likewise.
(struct case_tree_node): Add new function has_child.
(do_jump_if_equal): New.

gcc/testsuite/ChangeLog:

2018-08-13  Martin Liska  

* gcc.dg/tree-ssa/switch-3.c: New test.
* gcc.dg/tree-ssa/vrp105.c: Remove.
---
 gcc/testsuite/gcc.dg/tree-ssa/switch-3.c |  20 ++
 gcc/testsuite/gcc.dg/tree-ssa/vrp105.c   |  37 
 gcc/tree-switch-conversion.c | 243 ---
 gcc/tree-switch-conversion.h |  24 +++
 4 files changed, 256 insertions(+), 68 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/switch-3.c
 delete mode 100644 gcc/testsuite/gcc.dg/tree-ssa/vrp105.c

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/switch-3.c b/gcc/testsuite/gcc.dg/tree-ssa/switch-3.c
new file mode 100644
index 000..44981e1d186
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/switch-3.c
@@ -0,0 +1,20 @@
+/* { dg-options "-O2 -fdump-tree-switchlower1" } */
+
+int cipher_to_alg(int cipher)
+{
+  switch (cipher)  
+{
+  case 8:   return 2;  
+  case 16:  return 3;  
+  case 32:  return 4;  
+  case 64:  return 6;  
+  case 256: return 9;  
+  case 512: return 10; 
+  case 2048: return 11;
+  case 4096: return 12;
+  case 8192: return 13;
+}
+  return 0;
+} 
+
+/* { dg-final { scan-tree-dump-times "if \\(cipher\[^\n ]*" 12 "switchlower1" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/vrp105.c b/gcc/testsuite/gcc.dg/tree-ssa/vrp105.c
deleted file mode 100644
index 7cdd4dd8f3a..000
--- a/gcc/testsuite/gcc.dg/tree-ssa/vrp105.c
+++ /dev/null
@@ -1,37 +0,0 @@
-/* PR tree-optimization/18046  */
-/* { dg-options "-O2 -fdump-tree-vrp2-details" }  */
-/* { dg-final { scan-tree-dump-times "Threaded jump" 1 "vrp2" } }  */
-/* In the 2nd VRP pass (after PRE) we expect to thread the default label of the
-   1st switch straight to that of the 2nd switch.  */
-
-extern void foo (void);
-extern void bar (void);
-
-extern int i;
-void
-test (void)
-{
-  switch (i)
-{
-case 0:
-  foo ();
-  break;
-case 1:
-  bar ();
-  break;
-default:
-  break;
-}
-
-  switch (i)
-{
-case 0:
-  foo ();
-  break;
-case 1:
-  bar ();
-  break;
-default:
-  break;
-}
-}
diff --git a/gcc/tree-switch-conversion.c b/gcc/tree-switch-conversion.c
index cd771438214..78914bbe81a 100644
--- a/gcc/tree-switch-conversion.c
+++ b/gcc/tree-switch-conversion.c
@@ -2008,7 +2008,9 @@ switch_decision_tree::dump_case_nodes (FILE *f, case_tree_node *root,
   fprintf (f, "%*s", indent_step * indent_level, "");
   root->m_c->dump (f);
   root->m_c->m_prob.dump (f);
-  fputs ("\n", f);
+  fputs (" subtree: ", f);
+  root->m_c->m_subtree_prob.dump (f);
+  fputs (")\n", f);
 
   dump_case_nodes (f, root->m_right, indent_step, indent_level);
 }
@@ -2054,6 +2056,33 @@ switch_decision_tree::emit_cmp_and_jump_insns (basic_block bb, tree op0,
   return false_edge->dest;
 }
 
+/* Generate code to jump to LABEL if OP0 and OP1 are equal in mode MODE.
+   PROB is the probability of jumping to LABEL_BB.  */
+
+basic_block
+switch_decision_tree::do_jump_if_equal (basic_block bb, tree op0, tree op1,
+	basic_block label_bb,
+	profile_probability prob)
+{
+  op1 = fold_convert (TREE_TYPE (op0), op1);
+
+  gcond *cond = gimple_build_cond (EQ_EXPR, op0, op1, NULL_TREE, NULL_TREE);
+  gimple_stmt_iterator gsi = gsi_last_bb (bb);
+  gsi_insert_before (, cond, GSI_SAME_STMT);
+
+  gcc_assert (single_succ_p (bb));
+
+  /* Make a new basic block where false branch will take place.  */
+  edge false_edge = split_block (bb, cond);
+  false_edge->flags 

[PATCH 2/3] Fix probability for bit-tests.

2018-08-14 Thread marxin
The patch set even probability to bit test based on number of
cases which are handled on each edge.

gcc/ChangeLog:

2018-08-06  Martin Liska  

* tree-switch-conversion.c (bit_test_cluster::find_bit_tests):
Add new argument to bit_test_cluster constructor.
(bit_test_cluster::emit): Set bits really number of values
handlel by a test.
(bit_test_cluster::hoist_edge_and_branch_if_true): Add
probability argument.
* tree-switch-conversion.h (struct bit_test_cluster):
Add m_handles_entire_switch.

gcc/testsuite/ChangeLog:

2018-08-06  Martin Liska  

* gcc.dg/tree-ssa/switch-2.c: New test.
---
 gcc/testsuite/gcc.dg/tree-ssa/switch-2.c | 25 +
 gcc/tree-switch-conversion.c | 46 +---
 gcc/tree-switch-conversion.h | 12 +--
 3 files changed, 67 insertions(+), 16 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/switch-2.c

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/switch-2.c b/gcc/testsuite/gcc.dg/tree-ssa/switch-2.c
new file mode 100644
index 000..710825dc257
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/switch-2.c
@@ -0,0 +1,25 @@
+/* { dg-do compile { target { { x86_64-*-* aarch64-*-* ia64-*-* powerpc64-*-* } && lp64 } } } */
+/* { dg-options "-O2 -fdump-tree-switchlower1" } */
+
+int global;
+
+int foo (int x)
+{
+  switch (x) {
+case 0:
+case 10:
+  return 1;
+case 20:
+case 30:
+case 62:
+  return 2;
+case 1000:
+case 1010:
+case 1025 ... 1030:
+  return 1;
+default:
+  return 0;
+  }
+}
+
+/* { dg-final { scan-tree-dump ";; GIMPLE switch case clusters: BT:0-62 BT:1000-1030" "switchlower1" } } */
diff --git a/gcc/tree-switch-conversion.c b/gcc/tree-switch-conversion.c
index 0b5c93118f6..cd771438214 100644
--- a/gcc/tree-switch-conversion.c
+++ b/gcc/tree-switch-conversion.c
@@ -1282,12 +1282,16 @@ bit_test_cluster::find_bit_tests (vec )
 return clusters.copy ();
 
   /* Find and build the clusters.  */
-  for (int end = l;;)
+  for (unsigned end = l;;)
 {
   int start = min[end].m_start;
 
   if (is_beneficial (clusters, start, end - 1))
-	output.safe_push (new bit_test_cluster (clusters, start, end - 1));
+	{
+	  bool entire = start == 0 && end == clusters.length ();
+	  output.safe_push (new bit_test_cluster (clusters, start, end - 1,
+		  entire));
+	}
   else
 	for (int i = end - 1; i >=  start; i--)
 	  output.safe_push (clusters[i]);
@@ -1437,6 +1441,7 @@ bit_test_cluster::emit (tree index_expr, tree index_type,
   tree minval = get_low ();
   tree maxval = get_high ();
   tree range = int_const_binop (MINUS_EXPR, maxval, minval);
+  unsigned HOST_WIDE_INT bt_range = get_range (minval, maxval);
 
   /* Go through all case labels, and collect the case labels, profile
  counts, and other information we need to build the branch tests.  */
@@ -1455,11 +1460,11 @@ bit_test_cluster::emit (tree index_expr, tree index_type,
 	  test[k].mask = wi::zero (prec);
 	  test[k].target_bb = n->m_case_bb;
 	  test[k].label = n->m_case_label_expr;
-	  test[k].bits = 1;
+	  test[k].bits = 0;
 	  count++;
 	}
-  else
-	test[k].bits++;
+
+  test[k].bits += n->get_range (n->get_low (), n->get_high ());
 
   lo = tree_to_uhwi (int_const_binop (MINUS_EXPR, n->get_low (), minval));
   if (n->get_high () == NULL_TREE)
@@ -1516,14 +1521,20 @@ bit_test_cluster::emit (tree index_expr, tree index_type,
   /*simple=*/true, NULL_TREE,
   /*before=*/true, GSI_SAME_STMT);
 
-  /* if (idx > range) goto default */
-  range = force_gimple_operand_gsi (,
+  if (m_handles_entire_switch)
+{
+  /* if (idx > range) goto default */
+  range
+	= force_gimple_operand_gsi (,
 fold_convert (unsigned_index_type, range),
 /*simple=*/true, NULL_TREE,
 /*before=*/true, GSI_SAME_STMT);
-  tmp = fold_build2 (GT_EXPR, boolean_type_node, idx, range);
-  basic_block new_bb = hoist_edge_and_branch_if_true (, tmp, default_bb);
-  gsi = gsi_last_bb (new_bb);
+  tmp = fold_build2 (GT_EXPR, boolean_type_node, idx, range);
+  basic_block new_bb
+	= hoist_edge_and_branch_if_true (, tmp, default_bb,
+	 profile_probability::unlikely ());
+  gsi = gsi_last_bb (new_bb);
+}
 
   /* csui = (1 << (word_mode) idx) */
   csui = make_ssa_name (word_type_node);
@@ -1536,17 +1547,23 @@ bit_test_cluster::emit (tree index_expr, tree index_type,
   gsi_insert_before (, shift_stmt, GSI_SAME_STMT);
   update_stmt (shift_stmt);
 
+  profile_probability prob = profile_probability::always ();
+
   /* for each unique set of cases:
if (const & csui) goto target  */
   for (k = 0; k < count; k++)
 {
+  prob = profile_probability::always ().apply_scale (test[k].bits,
+			 bt_range);
+  bt_range -= test[k].bits;
   tmp = wide_int_to_tree (word_type_node, test[k].mask);
   tmp = fold_build2 (BIT_AND_EXPR, word_type_node, csui, tmp);
   tmp = 

[PATCH] Backport of RISC-V support for libffi go closures

2018-08-14 Thread Andreas Schwab
This is a backport of commit 4cb776bc80 in the libffi repository.
Tested on riscv64-suse-linux
.

Andreas.

Backport of RISC-V support for libffi go closures
* src/riscv/ffi.c (ffi_call_go, ffi_prep_go_closure): New
functions.
(ffi_call_int): Renamed from ffi_call.
(ffi_call_asm, ffi_closure_inner): Adjust interface.
* src/riscv/ffitarget.h (FFI_GO_CLOSURES): Define.
* src/riscv/sysv.S (ffi_go_closure_asm): New function.
(ffi_closure_asm, ffi_call_asm): Update for adjusted interfaces.

diff --git a/libffi/src/riscv/ffi.c b/libffi/src/riscv/ffi.c
index b664ee7d4e..8c5a860506 100644
--- a/libffi/src/riscv/ffi.c
+++ b/libffi/src/riscv/ffi.c
@@ -324,9 +324,12 @@ ffi_status ffi_prep_cif_machdep_var(ffi_cif *cif, unsigned 
int nfixedargs, unsig
 }
 
 /* Low level routine for calling functions */
-extern void ffi_call_asm(void *stack, struct call_context *regs, void 
(*fn)(void)) FFI_HIDDEN;
+extern void ffi_call_asm (void *stack, struct call_context *regs,
+ void (*fn) (void), void *closure) FFI_HIDDEN;
 
-void ffi_call(ffi_cif *cif, void (*fn)(void), void *rvalue, void **avalue)
+static void
+ffi_call_int (ffi_cif *cif, void (*fn) (void), void *rvalue, void **avalue,
+ void *closure)
 {
 /* this is a conservative estimate, assuming a complex return value and
that all remaining arguments are long long / __int128 */
@@ -366,13 +369,26 @@ void ffi_call(ffi_cif *cif, void (*fn)(void), void 
*rvalue, void **avalue)
 for (i = 0; i < cif->nargs; i++)
 marshal(, cif->arg_types[i], i >= cif->riscv_nfixedargs, avalue[i]);
 
-ffi_call_asm((void*)alloc_base, cb.aregs, fn);
+ffi_call_asm ((void *) alloc_base, cb.aregs, fn, closure);
 
 cb.used_float = cb.used_integer = 0;
 if (!return_by_ref && rvalue)
 unmarshal(, cif->rtype, 0, rvalue);
 }
 
+void
+ffi_call (ffi_cif *cif, void (*fn) (void), void *rvalue, void **avalue)
+{
+  ffi_call_int(cif, fn, rvalue, avalue, NULL);
+}
+
+void
+ffi_call_go (ffi_cif *cif, void (*fn) (void), void *rvalue,
+void **avalue, void *closure)
+{
+  ffi_call_int(cif, fn, rvalue, avalue, closure);
+}
+
 extern void ffi_closure_asm(void) FFI_HIDDEN;
 
 ffi_status ffi_prep_closure_loc(ffi_closure *closure, ffi_cif *cif, void 
(*fun)(ffi_cif*,void*,void**,void*), void *user_data, void *codeloc)
@@ -406,11 +422,31 @@ ffi_status ffi_prep_closure_loc(ffi_closure *closure, 
ffi_cif *cif, void (*fun)(
 return FFI_OK;
 }
 
+extern void ffi_go_closure_asm (void) FFI_HIDDEN;
+
+ffi_status
+ffi_prep_go_closure (ffi_go_closure *closure, ffi_cif *cif,
+void (*fun) (ffi_cif *, void *, void **, void *))
+{
+  if (cif->abi <= FFI_FIRST_ABI || cif->abi >= FFI_LAST_ABI)
+return FFI_BAD_ABI;
+
+  closure->tramp = (void *) ffi_go_closure_asm;
+  closure->cif = cif;
+  closure->fun = fun;
+
+  return FFI_OK;
+}
+
 /* Called by the assembly code with aregs pointing to saved argument registers
and stack pointing to the stacked arguments.  Return values passed in
registers will be reloaded from aregs. */
-void FFI_HIDDEN ffi_closure_inner(size_t *stack, call_context *aregs, 
ffi_closure *closure) {
-ffi_cif *cif = closure->cif;
+void FFI_HIDDEN
+ffi_closure_inner (ffi_cif *cif,
+  void (*fun) (ffi_cif *, void *, void **, void *),
+  void *user_data,
+  size_t *stack, call_context *aregs)
+{
 void **avalue = alloca(cif->nargs * sizeof(void*));
 /* storage for arguments which will be copied by unmarshal().  We could
theoretically avoid the copies in many cases and use at most 128 bytes
@@ -436,7 +472,7 @@ void FFI_HIDDEN ffi_closure_inner(size_t *stack, 
call_context *aregs, ffi_closur
 avalue[i] = unmarshal(, cif->arg_types[i],
 i >= cif->riscv_nfixedargs, astorage + i*MAXCOPYARG);
 
-(closure->fun)(cif, rvalue, avalue, closure->user_data);
+fun (cif, rvalue, avalue, user_data);
 
 if (!return_by_ref && cif->rtype->type != FFI_TYPE_VOID) {
 cb.used_integer = cb.used_float = 0;
diff --git a/libffi/src/riscv/ffitarget.h b/libffi/src/riscv/ffitarget.h
index fcaa899153..75e6462f5b 100644
--- a/libffi/src/riscv/ffitarget.h
+++ b/libffi/src/riscv/ffitarget.h
@@ -59,6 +59,7 @@ typedef enum ffi_abi {
 /*  Definitions for closures - */
 
 #define FFI_CLOSURES 1
+#define FFI_GO_CLOSURES 1
 #define FFI_TRAMPOLINE_SIZE 24
 #define FFI_NATIVE_RAW_API 0
 #define FFI_EXTRA_CIF_FIELDS unsigned riscv_nfixedargs; unsigned riscv_unused;
diff --git a/libffi/src/riscv/sysv.S b/libffi/src/riscv/sysv.S
index 2d098651d0..522d0b0055 100644
--- a/libffi/src/riscv/sysv.S
+++ b/libffi/src/riscv/sysv.S
@@ -67,8 +67,8 @@
   intreg pad[rv32 ? 2 : 0];
   intreg save_fp, save_ra;
   }
-  void ffi_call_asm(size_t *stackargs, struct call_context 

Re: [libgomp, testsuite] Support parallel testing in libgomp (PR libgomp/66005)

2018-08-14 Thread Martin Liška
On 05/08/2015 10:40 AM, Thomas Schwinge wrote:
> Hi!
> 
> On Thu, 7 May 2015 13:39:40 +0200, Jakub Jelinek  wrote:
>> On Thu, May 07, 2015 at 01:26:57PM +0200, Rainer Orth wrote:
>>> As reported in the PR, with the addition of all those OpenACC tests,
>>> libgomp make check times have skyrocketed since the testsuite is still
>>> run sequentially.
> 
> ACK.  And, thanks for looking into that!
> 
>>> Fixing this proved trivial: I managed to almost literally copy the
>>> solution from libstdc++-v3/testsuite/Makefile.am, with a minimal change
>>> to libgomp.exp so the generated libgomp-test-support.exp file is found
>>> in both the sequential and parallel cases.  This isn't an issue in
>>> libstdc++ since all necessary variables are stored in a single
>>> site.exp.
>>
>> It is far from trivial though.
>> The point is that most of the OpenMP tests are parallelized with the
>> default OMP_NUM_THREADS, so running the tests in parallel oversubscribes the
>> machine a lot, the higher number of hw threads the more.
> 
> Do you agree that we have two classes of test cases in libgomp: 1) test
> cases that don't place a considerably higher load on the machine compared
> to "normal" (single-threaded) execution tests, because they're just
> testing some functionality that is not expected to actively depend
> on/interfere with parallelism.  If needed, and/or if not already done,
> such test cases can be parameterized (OMP_NUM_THREADS, OpenACC num_gangs,
> num_workers, vector_length clauses, and so on) for low parallelism
> levels.  And, 2) test cases that place a considerably higher load on the
> machine compared to "normal" (single-threaded) execution tests, because
> they're testing some functionality that actively depends on/interferes
> with some kind of parallelism.  What about marking such tests specially,
> such that DejaGnu will only ever schedule one of them for execution at
> the same time?  For example, a new dg-* directive to run them wrapped
> through »flock [libgomp/testsuite/serial.lock] [a.out]« or some such?

Looks the thread got stuck. Anyway I've just noticed how slow libgomp.exp tests
are on a recent Intel Machine with 160 HT cores. I'm attaching graph with CPU
utilization and 'ps ax | grep expect' log file that shows which tests are 
running.
Roughly, after 10 minutes I see drop in utilization and then libgomp.exp is 
running
mainly serially.

So I believe splitting tests in libgomp.exp to serial and parallel would make 
sense.
Another another idea is to overwrite OMP_NUM_THREADS to a reasonable number 
which
will enable also parallel execution of parallel tests?

Thanks,
Martin

> 
>> If we go forward with some parallelization of the tests, we at least should
>> try to export something like OMP_WAIT_POLICY=passive so that the
>> oversubscribed machine would at least not spend too much time in spinning.
> 
> (Will again have the problem that DejaGnu doesn't provide infrastructure
> to communicate environment variables to boards in remote testing.)
> 
>> And perhaps reconsider running all OpenACC threads 3 times, just allow
>> user to select which offloading target they want to test (host fallback,
>> the host nonshm hack, PTX, XeonPHI in the future?), and test just that
>> (that is pretty much how OpenMP offloading testing works).
> 
> My rationale is: if you configure GCC to support a set of offloading
> devices (more than one), you'll also want to get the test coverage that
> indeed all these work as expected.  (It currently doesn't matter, but...)
> that's something I'd like to see improved in the libgomp OpenMP
> offloading testing (once it supports more than one architecture for
> offloading).
> 
>> For tests that
>> always want to test host fallback, I hope OpenACC offers clauses to force
>> the host fallback.
> 
> Yes.
> 
> 
> Grüße,
>  Thomas
> 



make-check.tar.bz2
Description: application/bzip


Re: [Patch, Fortran] PR 86116: Ambiguous generic interface not recognised

2018-08-14 Thread Janus Weil
ping!


Am So., 5. Aug. 2018 um 15:23 Uhr schrieb Janus Weil :
>
> Hi all,
>
> the attached patch fixes PR 86116 by splitting up the function
> 'compare_type' into two variants: One that is used for checking
> generic interfaces and operators (keeping the old name), and one that
> is used for checking dummy functions and procedure pointer assignments
> ('compare_type_characteristics'). The latter calls the former, but
> includes an additional check that must not be done when checking
> generics.
>
> Regtests cleanly on x86_64-linux-gnu. Ok for trunk?
>
> Cheers,
> Janus
>
>
> 2018-08-05  Janus Weil  
>
> PR fortran/86116
> * interface.c (compare_type): Remove a CLASS/TYPE check.
> (compare_type_characteristics): New function that behaves like the old
> 'compare_type'.
> (gfc_check_dummy_characteristics, gfc_check_result_characteristics):
> Call 'compare_type_characteristics' instead of 'compare_type'.
>
> 2018-08-05  Janus Weil  
>
> PR fortran/86116
> * gfortran.dg/generic_34.f90: New test case.


[PATCH] lra: fix FPE when dumping

2018-08-14 Thread Ilya Leoshkevich
The following S/390 code

struct {} b;
void c() {
  __asm__("la 0,%0\n"
  "la 1,%1\n"
  :
  : "m" (b), "m" (b)
  : "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7", "r8",
"r9", "r10", "r12", "r14");
}

results in

internal compiler error: Floating point exception

when building with

-fira-verbose=9 -fdump-rtl-all

gcc/ChangeLog:

2018-07-25  Ilya Leoshkevich  

PR target/86547
* lra-lives.c (remove_some_program_points_and_update_live_ranges):
Check whether lra_live_max_point is 0 before dividing.
---
 gcc/lra-lives.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gcc/lra-lives.c b/gcc/lra-lives.c
index 433c819d9e3..565c68b430a 100644
--- a/gcc/lra-lives.c
+++ b/gcc/lra-lives.c
@@ -1153,7 +1153,8 @@ remove_some_program_points_and_update_live_ranges (void)
   n++;
   if (lra_dump_file != NULL)
 fprintf (lra_dump_file, "Compressing live ranges: from %d to %d - %d%%\n",
-lra_live_max_point, n, 100 * n / lra_live_max_point);
+lra_live_max_point, n,
+lra_live_max_point ? 100 * n / lra_live_max_point : 100);
   if (n < lra_live_max_point)
 {
   lra_live_max_point = n;
-- 
2.18.0



[PATCH] S/390: Remove literal pool chunkification loop

2018-08-14 Thread Ilya Leoshkevich
Since there is no branch splitting anymore, the loop is no longer
necessary: pool chunkification can be done in one step.

gcc/ChangeLog:

2018-08-13  Ilya Leoshkevich  

* config/s390/s390.c (s390_reorg): remove loop
---
 gcc/config/s390/s390.c | 69 ++
 1 file changed, 16 insertions(+), 53 deletions(-)

diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index cc6f3489998..8b3987317a5 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -14023,7 +14023,6 @@ s390_adjust_loops ()
 static void
 s390_reorg (void)
 {
-  bool pool_overflow = false;
   rtx_insn *insn;
   int hw_before, hw_after;
 
@@ -14035,62 +14034,26 @@ s390_reorg (void)
   split_all_insns_noflow ();
 
   /* Install the main literal pool and the associated base
- register load insns.
+ register load insns.  The literal pool might be > 4096 bytes in
+ size, so that some of its elements cannot be directly accessed.
 
- In addition, there are two problematic situations we need
- to correct:
-
- - the literal pool might be > 4096 bytes in size, so that
-   some of its elements cannot be directly accessed
-
- - a branch target might be > 64K away from the branch, so that
-   it is not possible to use a PC-relative instruction.
-
- To fix those, we split the single literal pool into multiple
+ To fix this, we split the single literal pool into multiple
  pool chunks, reloading the pool base register at various
  points throughout the function to ensure it always points to
- the pool chunk the following code expects, and / or replace
- PC-relative branches by absolute branches.
-
- However, the two problems are interdependent: splitting the
- literal pool can move a branch further away from its target,
- causing the 64K limit to overflow, and on the other hand,
- replacing a PC-relative branch by an absolute branch means
- we need to put the branch target address into the literal
- pool, possibly causing it to overflow.
-
- So, we loop trying to fix up both problems until we manage
- to satisfy both conditions at the same time.  Note that the
- loop is guaranteed to terminate as every pass of the loop
- strictly decreases the total number of PC-relative branches
- in the function.  (This is not completely true as there
- might be branch-over-pool insns introduced by chunkify_start.
- Those never need to be split however.)  */
-
-  for (;;)
-{
-  struct constant_pool *pool = NULL;
-
-  /* Collect the literal pool.  */
-  if (!pool_overflow)
-   {
- pool = s390_mainpool_start ();
- if (!pool)
-   pool_overflow = true;
-   }
-
-  /* If literal pool overflowed, start to chunkify it.  */
-  if (pool_overflow)
-   pool = s390_chunkify_start ();
+ the pool chunk the following code expects.  */
 
-  /* If we made it up to here, both conditions are satisfied.
-Finish up literal pool related changes.  */
-  if (pool_overflow)
-   s390_chunkify_finish (pool);
-  else
-   s390_mainpool_finish (pool);
-
-  break;
+  /* Collect the literal pool.  */
+  struct constant_pool *pool = s390_mainpool_start ();
+  if (pool)
+{
+  /* Finish up literal pool related changes.  */
+  s390_mainpool_finish (pool);
+}
+  else
+{
+  /* If literal pool overflowed, chunkify it.  */
+  pool = s390_chunkify_start ();
+  s390_chunkify_finish (pool);
 }
 
   /* Generate out-of-pool execute target insns.  */
-- 
2.18.0



Re: Fix even more merging PIC and PIE options

2018-08-14 Thread Martin Liška
On 08/10/2018 04:33 PM, Jan Hubicka wrote:
> Hi,
> this patch should fix merging of PIC and PIE options so we always resort
> to the least common denominator of the object files compiled (i.e. 
> linking together -fpic and -fPIE will result in -fpie binary).
> Note that we also use information about format of output passed by linker
> plugin so we will disable pic/pie when resulting binary is not relocatable.
> However for shared libraries and pie we want to pick right mode that makes
> sense.
> 
> I wrote simple script that tries all possible options of combining two files.
> Mode picked is specified in the output file.
> 
> To support targets that default to pic/pie well, I had to also hack
> lto_write_options to always stream what mode was used in the given file.
> 
> lto-bootstrapped/regtested x86_64-linux, OK?
> 
> Honza

Thank you Honza for it. Would it be then subject for backporting into GCC-8 
branch?

Martin

> 
>   PR lto/86517
>   * lto-opts.c (lto_write_options): Always stream PIC/PIE mode.
>   * lto-wrapper.c (merge_and_complain): Fix merging of PIC/PIE
> Index: lto-opts.c
> ===
> --- lto-opts.c(revision 263356)
> +++ lto-opts.c(working copy)
> @@ -78,6 +78,22 @@ lto_write_options (void)
>&& !global_options.x_flag_openacc)
>  append_to_collect_gcc_options (_obstack, _p,
>  "-fno-openacc");
> +  /* Append PIC/PIE mode because its default depends on target and it is
> + subject of merging in lto-wrapper.  */
> +  if ((!global_options_set.x_flag_pic || global_options.x_flag_pic == 0)
> +  && !global_options_set.x_flag_pie)
> +{
> +   append_to_collect_gcc_options (_obstack, _p,
> +   global_options.x_flag_pic == 2
> +   ? "-fPIC"
> +   : global_options.x_flag_pic == 1
> +   ? "-fpic"
> +   : global_options.x_flag_pie == 2
> +   ? "-fPIE"
> +   : global_options.x_flag_pie == 1
> +   ? "-fpie"
> +   : "-fno-pie");
> +}
>  
>/* Append options from target hook and store them to offload_lto section.  
> */
>if (lto_stream_offload_p)
> Index: lto-wrapper.c
> ===
> --- lto-wrapper.c (revision 263356)
> +++ lto-wrapper.c (working copy)
> @@ -408,6 +408,11 @@ merge_and_complain (struct cl_decoded_op
>   It is a common mistake to mix few -fPIC compiled objects into otherwise
>   non-PIC code.  We do not want to build everything with PIC then.
>  
> + Similarly we merge PIE options, however in addition we keep
> +  -fPIC + -fPIE = -fPIE
> +  -fpic + -fPIE = -fpie
> +  -fPIC/-fpic + -fpie = -fpie
> +
>   It would be good to warn on mismatches, but it is bit hard to do as
>   we do not know what nothing translates to.  */
>  
> @@ -415,11 +420,38 @@ merge_and_complain (struct cl_decoded_op
>  if ((*decoded_options)[j].opt_index == OPT_fPIC
>  || (*decoded_options)[j].opt_index == OPT_fpic)
>{
> - if (!pic_option
> - || (pic_option->value > 0) != ((*decoded_options)[j].value > 0))
> -   remove_option (decoded_options, j, decoded_options_count);
> - else if (pic_option->opt_index == OPT_fPIC
> -  && (*decoded_options)[j].opt_index == OPT_fpic)
> + /* -fno-pic in one unit implies -fno-pic everywhere.  */
> + if ((*decoded_options)[j].value == 0)
> +   j++;
> + /* If we have no pic option or merge in -fno-pic, we still may turn
> +existing pic/PIC mode into pie/PIE if -fpie/-fPIE is present.  */
> + else if ((pic_option && pic_option->value == 0)
> +  || !pic_option)
> +   {
> + if (pie_option)
> +   {
> + bool big = (*decoded_options)[j].opt_index == OPT_fPIC
> +&& pie_option->opt_index == OPT_fPIE;
> + (*decoded_options)[j].opt_index = big ? OPT_fPIE : OPT_fpie;
> + if (pie_option->value)
> +   (*decoded_options)[j].canonical_option[0] = big ? "-fPIE" : 
> "-fpie";
> + else
> +   (*decoded_options)[j].canonical_option[0] = big ? "-fno-pie" 
> : "-fno-pie";
> + (*decoded_options)[j].value = pie_option->value;
> + j++;
> +   }
> + else if (pic_option)
> +   {
> + (*decoded_options)[j] = *pic_option;
> + j++;
> +   }
> + /* We do not know if target defaults to pic or not, so just remove
> +option if it is missing in one unit but enabled in other.  */
> + else
> +   remove_option (decoded_options, j, decoded_options_count);
> +   }
> + 

Re: Two fixes for pretty-print.c for mingw-w64

2018-08-14 Thread Liu Hao

在 2018/8/14 13:54, Liu Hao 写道:
The two patches attached have addressed two issues in the ANSI escape 
sequence translator I sent before.  Please review, and consider 
backporting these to gcc-8-branch.







And here are SVN changelogs for these two patches, in this order:



* gcc/pretty-print.c (mingw_ansi_fputs): Do not call _close() on the handle
returned by _get_osf_handle().



* gcc/pretty-print.c (eat_esc_sequence): Swap the foreground and background
colors if the COMMON_LVB_REVERSE_VIDEO flag is set, and clear it thereafter,
as it only works for DBCS.



--
Best regards,
LH_Mouse



Re: [PATCH] Handle TARGET_SPLIT_COMPLEX_ARG in default VA_ARG_EXPR implementation (ping)

2018-08-14 Thread Chung-Lin Tang

Ping.

On 2018/8/2 5:17 PM, Chung-Lin Tang wrote:

Hi, during testing of the new in-review C-SKY port, we discovered some FAILs 
due to
the default va_args gimplifying (targhooks.c:std_gimplify_va_arg_expr) not 
properly
having the logic to handle the TARGET_SPLIT_COMPLEX_ARG hook.

It appears that all other targets that happen to use TARGET_SPLIT_COMPLEX_ARG 
also
defines TARGET_GIMPLIFY_VA_ARG_EXPR, so this went undiscovered.
The C-SKY port happens to only have TARGET_SPLIT_COMPLEX_ARG defined.

This patch completes this handling in std_gimplify_va_arg_expr(), though at the
moment it's only really exercised by the C-SKY port, which we tested to fix 
several
_Complex va_args related FAILs and without regressions.
(the patch fragment is actually adapted from the xtensa port, FWIW)

Is this okay for trunk?

Thanks,
Chung-Lin

2018-08-02  Chung-Lin Tang  

     * targhooks.c (std_gimplify_va_arg_expr): Properly handle case of when
     TARGET_SPLIT_COMPLEX_ARG is defined.