Re: [PATCH] Fix store merging (PR tree-optimization/84503)

2018-02-21 Thread Richard Biener
On February 21, 2018 11:28:36 PM GMT+01:00, Jakub Jelinek  
wrote:
>Hi!
>
>The long comment above the new check_no_overlap function below should
>hopefully explain the problem.  coalesce_immediate_stores is splitting
>the
>stores from the same base into groups that we are going to optimize
>individually; for each successfully merged group we emit new stores at
>the
>location of the last store from that group.  And
>coalesce_immediate_stores
>processes stores ordered primarily by increasing bit position and only
>secondarily by the original ordering in the basic block.
>On the following testcases, we have the unfortunate ordering of:
> MEM[(long long int *)p_28] = 0;
> MEM[(long long int *)p_28 + 8B] = 0;
> MEM[(long long int *)p_28 + 16B] = 0;
> MEM[(long long int *)p_28 + 24B] = 0;
> _129 = (int) _130;
> MEM[(int *)p_28 + 8B] = _129;
> MEM[(int *)p_28].a = -1;
>We already have
> MEM[(long long int *)p_28] = 0;
> MEM[(int *)p_28].a = -1;
>in the first group (which is fine) and the following 2 stores to
>consider
>are
> MEM[(long long int *)p_28 + 8B] = 0;
>and
> MEM[(int *)p_28 + 8B] = _129;
>We add the first store to the group, which has the effect of emitting
>the
>merged stores at the location of former MEM[(int *)p_28].a = -1;
>store.  Then we process MEM[(int *)p_28 + 8B] = _129; (which was
>handled
>just as potential bswap possibility and as it overlaps with previous
>and can't be merged with next either, it will be its own group and thus
>reuse the original stmt; the net effect is that we've reordered the
>MEM[(long long int *)p_28 + 8B] = 0; store from before the
>MEM[(int *)p_28 + 8B] = _129; store to after it, but those two do
>alias.
>
>Instead of detecting this situation late, where we could just throw
>away the
>whole group (which would mean we couldn't merge even the
>MEM[(long long int *)p_28] = 0; and MEM[(int *)p_28].a = -1; stores)
>this patch attempts to check before calling merge_into whether it will
>not overlap with later stores we won't be able to emit in the same
>group
>and if it does, it terminates the group right away.
>
>I had to fix also the merged_store_group::merge_into width computation,
>it was mistakenly just counting sum of the bits we've added to the
>group,
>but we really need the distance between the first bit in the group and
>last
>bit, including any gaps in between, but exclusing gaps before the start
>and
>after the end (still within the bitregion).
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK. 

Richard. 

>For 7.x I think we'll need something along the lines of PR82916
>instead,
>while the same testcase fails there, we only handle stores with lhs
>being
>a constant and so the problem is that we aren't terminating the chain
>when
>we should on the variable store.
>
>2018-02-21  Jakub Jelinek  
>
>   PR tree-optimization/84503
>   * gimple-ssa-store-merging.c (merged_store_group::merge_into): Compute
>   width as info->bitpos + info->bitsize - start.
>   (merged_store_group::merge_overlapping): Simplify width computation.
>   (check_no_overlap): New function.
>   (imm_store_chain_info::try_coalesce_bswap): Compute expected
>   start + width and last_order of the group, fail if check_no_overlap
>   fails.
>   (imm_store_chain_info::coalesce_immediate_stores): Don't merge info
>   to group if check_no_overlap fails.
>
>   * gcc.dg/pr84503-1.c: New test.
>   * gcc.dg/pr84503-2.c: New test.
>
>--- gcc/gimple-ssa-store-merging.c.jj  2018-01-16 09:52:26.230235131
>+0100
>+++ gcc/gimple-ssa-store-merging.c 2018-02-21 20:13:45.239129599 +0100
>@@ -1891,12 +1891,11 @@ merged_store_group::do_merge (store_imme
> void
> merged_store_group::merge_into (store_immediate_info *info)
> {
>-  unsigned HOST_WIDE_INT wid = info->bitsize;
>/* Make sure we're inserting in the position we think we're inserting. 
>*/
>   gcc_assert (info->bitpos >= start + width
> && info->bitregion_start <= bitregion_end);
> 
>-  width += wid;
>+  width = info->bitpos + info->bitsize - start;
>   do_merge (info);
> }
> 
>@@ -1909,7 +1908,7 @@ merged_store_group::merge_overlapping (s
> {
>   /* If the store extends the size of the group, extend the width.  */
>   if (info->bitpos + info->bitsize > start + width)
>-width += info->bitpos + info->bitsize - (start + width);
>+width = info->bitpos + info->bitsize - start;
> 
>   do_merge (info);
> }
>@@ -2304,6 +2303,55 @@ gather_bswap_load_refs (vec *refs,
> }
> }
> 
>+/* Check if there are any stores in M_STORE_INFO after index I
>+   (where M_STORE_INFO must be sorted by sort_by_bitpos) that overlap
>+   a potential group ending with END that have their order
>+   smaller than LAST_ORDER.  RHS_CODE is the kind of store in the
>+   group.  Return true if there are no such stores.
>+   Consider:
>+ MEM[(long long int *)p_28] = 0;
>+ MEM[(long long int *)p_28 + 8B] = 

Re: [PATCH] Fix TYPE_EMPTY_P handling - x86_64 ABI issue (PR target/84502)

2018-02-21 Thread Richard Biener
On February 21, 2018 11:37:20 PM GMT+01:00, Jakub Jelinek  
wrote:
>Hi!
>
>The following testcase shows that we set TYPE_EMPTY_P just one of the
>possibly many variants of an aggregate type.
>On the testcase, in one case (in the callee) we check TYPE_EMPTY_P on X
>which is
>the type variant for the using type, and in another case we check it on
>the
>A type instead, which has TYPE_EMPTY_P set.
>
>Bootstrap/regtest pending on x86_64-linux and i686-linux, ok if it
>passes?

OK. 

Richard. 

>2018-02-21  Jakub Jelinek  
>
>   PR target/84502
>   * stor-layout.c (finalize_type_size): Propagate TYPE_EMPTY_P flag
>   to all type variants.
>
>   * g++.dg/torture/pr84502.C: New test.
>
>--- gcc/stor-layout.c.jj   2018-02-13 21:23:29.187981310 +0100
>+++ gcc/stor-layout.c  2018-02-21 21:43:24.783522853 +0100
>@@ -1883,6 +1883,9 @@ finalize_type_size (tree type)
>   && TREE_CODE (TYPE_SIZE_UNIT (type)) != INTEGER_CST)
> TYPE_SIZE_UNIT (type) = variable_size (TYPE_SIZE_UNIT (type));
> 
>+  /* Handle empty records as per the x86-64 psABI.  */
>+  TYPE_EMPTY_P (type) = targetm.calls.empty_record_p (type);
>+
>   /* Also layout any other variants of the type.  */
>   if (TYPE_NEXT_VARIANT (type)
>   || type != TYPE_MAIN_VARIANT (type))
>@@ -1895,6 +1898,7 @@ finalize_type_size (tree type)
>   unsigned int precision = TYPE_PRECISION (type);
>   unsigned int user_align = TYPE_USER_ALIGN (type);
>   machine_mode mode = TYPE_MODE (type);
>+  bool empty_p = TYPE_EMPTY_P (type);
> 
>   /* Copy it into all variants.  */
>   for (variant = TYPE_MAIN_VARIANT (type);
>@@ -1911,11 +1915,9 @@ finalize_type_size (tree type)
> SET_TYPE_ALIGN (variant, valign);
> TYPE_PRECISION (variant) = precision;
> SET_TYPE_MODE (variant, mode);
>+TYPE_EMPTY_P (variant) = empty_p;
>   }
> }
>-
>-  /* Handle empty records as per the x86-64 psABI.  */
>-  TYPE_EMPTY_P (type) = targetm.calls.empty_record_p (type);
> }
> 
>/* Return a new underlying object for a bitfield started with FIELD. 
>*/
>--- gcc/testsuite/g++.dg/torture/pr84502.C.jj  2018-02-21
>22:13:22.279420017 +0100
>+++ gcc/testsuite/g++.dg/torture/pr84502.C 2018-02-21
>22:13:03.727424362 +0100
>@@ -0,0 +1,20 @@
>+// PR target/84502
>+// { dg-do run }
>+
>+template
>+struct A { };
>+using X = A;
>+
>+void
>+foo (X, int a1, int a2, int a3, int a4, int a5, int a6, int a7, int
>a8)
>+{
>+  if (a1 != 0 || a2 != 1 || a3 != 2 || a4 != 3
>+  || a5 != 4 || a6 != 5 || a7 != 6 || a8 != 7)
>+__builtin_abort ();
>+}
>+
>+int
>+main ()
>+{
>+  foo (X{}, 0, 1, 2, 3, 4, 5, 6, 7);
>+}
>
>   Jakub



Re: [PATCH] Fix pdftex miscompilation due to get_range_strlen (PR tree-optimization/84478)

2018-02-21 Thread Richard Biener
On February 21, 2018 4:25:14 AM GMT+01:00, Jeff Law  wrote:
>On 02/20/2018 04:59 PM, Martin Sebor wrote:
>>> It would help if you explained why you think it is a good idea
>>> ignoring the other phi arguments if you have one (or more) where you
>can
>>> determine length.
>> 
>> It's a heuristic that was meant just for the -Wformat-overflow
>> warning.  When making decisions that affect code generation it's
>> obviously not correct to ignore the possibility that unknown
>> arguments may be shorter than the minimum or longer than
>> the maximum.  The fuzzy argument was meant to differentiate
>> between two got but I forgot about it when I added the fix
>> for PR 83671.
>> 
>> For GCC 8 I don't have a preference for how to fix this as long
>> as it doesn't regress the warning tests.
>> 
>> I think the ultimate solution (for GCC 9) may be to either
>> disable the heuristic for code generation purposes (e.g., via
>> another argument/flag) or provide a pointer argument to indicate
>> to the caller that the minimum is based on known strings, and that
>> the real minimum may be zero.
>I'm still getting refamiliar with this code.  But one thing that jumps
>out immediately is how much this reminds me of the discussion we had
>around 77608 -- where I argued that returning something that was not
>conservatively correct was just asking for long term problems.
>
>I realize we're talking about different routines, but the concerns are
>the same -- when we return something that is not conservatively correct
>it's easy for someone to mistakenly use those results for code
>generation purposes.
>
>The fuzzy stuff is in there to reduce the false positive rates and
>we're
>not *supposed* to be using fuzzy results for code generation purposes,
>but as I argued in 77608, it's easy to miss.
>
>I'll reiterate my desire to make this kind of mistake harder to make.

Can you simply provide that interface via a separately named API rather than 
overloading one that is also supposed to be used by optimization? 

Richard. 

>Jeff



Re: [PATCH v2] RISC-V: Support for FreeBSD

2018-02-21 Thread Kito Cheng
Hi Jim:

> I saw the email.  I checked the FSF copyright list and see that it
> hasn't been recorded yet.  The FSF copyright clerk sometimes takes a
> few weeks to respond.  Hopefully this will be competed on the FSF side
> soon.

I don't family with copyright matters, so we can't commit this patch yet
until Ruslan send the signed copy and FSF signed it? right?

thanks :)


Re: [Patch] Document __builtin_extend_pointer

2018-02-21 Thread Jeff Law
On 02/20/2018 10:33 AM, Steve Ellcey wrote:
> While working on PR 83335 I proposed a change to a test case that
> used __builtin_extend_pointer and Richared Earnshaw pointed out
> that this builtin is not documented.  Since I could not find any
> other (reasonable) way to generate an extended address in inline
> assembly other than this builtin I would like to document it for
> use.
> 
> Here is a proposed patch, the one problem I found was the return
> type of the builtin.  I don't know how to describe it other than
> Pmode, but that is not a user visible type.
> 
> See https://gcc.gnu.org/ml/gcc-patches/2018-02/msg01051.html
> for my PR 83335 patch and follow up comments.
> 
> Should I go ahead and add this documentation?
> 
> 
> 2018-02-20  Steve Ellcey  
> 
>   * doc/extend.texi (__builtin_extend_pointer): Document builtin.
I'd change "is different than" to "is smaller than".  It's kind of
implied in the name, but I think it's slightly clearer.

With that.  OK.

jeff

ps.  You might ping Richard Sandiford on the actual patch that fixes
83335.  It looks like it's been waiting for over a month.



Re: [PATCH] avoid bogus -Wstringop-truncation when inlining (PR 84480)

2018-02-21 Thread Jeff Law
On 02/21/2018 02:19 PM, Martin Sebor wrote:
> The attached patch eliminates -Wstringop-truncation false
> positives reported in bug 84480 - bogus -Wstringop-truncation
> despite assignment with an inlined string literal.  It does
> that by delegating early strncpy checks during folding to
> the same machinery in tree-ssa-strlen that looks for a NUL
> assignment to the destination the next statement.
> 
> The patch also adds inlining context to the warnings via
> the %G directive.
> 
> Tested on x86_64-linux with no regressions.
> 
> Martin
> 
> gcc-84480.diff
> 
> 
> PR tree-optimization/84480 - bogus -Wstringop-truncation despite assignment 
> with an inlined string literal
> 
> gcc/ChangeLog:
> 
>   PR tree-optimization/84480
>   * gimple-fold.c (gimple_fold_builtin_strcpy): Move warnings
>   to maybe_diag_stxncpy_trunc.  Call it.
>   * tree-ssa-strlen.c (maybe_diag_stxncpy_trunc): Integrate warnings
>   from gimple_fold_builtin_strcpy.  Print inlining stack.
>   (handle_builtin_stxncpy): Print inlining stack.
>   * tree-ssa-strlen.h (maybe_diag_stxncpy_trunc): Declare.
> 
> gcc/testsuite/ChangeLog:
> 
>   PR tree-optimization/84480
>   * c-c++-common/Wstringop-truncation.c: Adjust text of expected warnings.
>   * g++.dg/warn/Wstringop-truncation-1.C: New test.
In general our guidelines are that the users of a .h file should include
any dependencies rather than having the .h file itself include other .h
files (Now that we've detangled the header files we may want to revisit
that guideline, but that's not a gcc-8 item).

It looks like gimple-fold.c and tree-ssa-strlen.c already have the
prereqs.  So in theory you should be able to just remove the bogus
#includes from tree-ssa-strlen.h.

In general we want to avoid adding more warnings to folding code.  But I
think the argument here is that we're already trying to warn within the
folder and just doing a poor job -- so we're removing that
implementation and delegating the warning to a better implementation.
Right?

So I think you just need to remove the bogus #includes from
tree-ssa-strlen and this is OK.

jeff



[PATCH 1/2] rs6000: Use brace blocks in define_insn

2018-02-21 Thread Segher Boessenkool
This patch changes the remaining cases in our machine description files
to use brace blocks instead of double-quoted strings as the output
control string.  This increases readability by making the blocks look
more like normal C code, mostly because backslash quoting is no longer
needed.  It also removes such quoting where it was still there (usually
harmless but always confusing). and it writes "\n\t" as "\;" in one
place where we didn't already.

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


Segher


2018-02-21  Segher Boessenkool  

* config/rs6000/altivec.md: Write output control strings as braced
blocks instead of double-quoted strings.
* config/rs6000/darwin.md: Ditto.
* config/rs6000/rs6000.md: Ditto.
* config/rs6000/vector.md: Ditto.
* config/rs6000/vsx.md: Ditto.

 gcc/config/rs6000/altivec.md | 159 
 gcc/config/rs6000/darwin.md  |  32 ++--
 gcc/config/rs6000/rs6000.md  | 346 +--
 gcc/config/rs6000/vector.md  |  69 +++--
 gcc/config/rs6000/vsx.md |  31 +---
 5 files changed, 249 insertions(+), 388 deletions(-)

diff --git a/gcc/config/rs6000/altivec.md b/gcc/config/rs6000/altivec.md
index a01a3c6..55a9f53 100644
--- a/gcc/config/rs6000/altivec.md
+++ b/gcc/config/rs6000/altivec.md
@@ -1680,13 +1680,12 @@ (define_insn "altivec_vpkpx"
   (match_operand:V4SI 2 "register_operand" "v")]
 UNSPEC_VPKPX))]
   "TARGET_ALTIVEC"
-  "*
-  {
-if (VECTOR_ELT_ORDER_BIG)
-  return \"vpkpx %0,%1,%2\";
-else
-  return \"vpkpx %0,%2,%1\";
-  }"
+{
+  if (VECTOR_ELT_ORDER_BIG)
+return "vpkpx %0,%1,%2";
+  else
+return "vpkpx %0,%2,%1";
+}
   [(set_attr "type" "vecperm")])
 
 (define_insn "altivec_vpksss"
@@ -1695,13 +1694,12 @@ (define_insn "altivec_vpksss"
(match_operand:VP 2 "register_operand" "v")]
   UNSPEC_VPACK_SIGN_SIGN_SAT))]
   ""
-  "*
-  {
-if (VECTOR_ELT_ORDER_BIG)
-  return \"vpksss %0,%1,%2\";
-else
-  return \"vpksss %0,%2,%1\";
-  }"
+{
+  if (VECTOR_ELT_ORDER_BIG)
+return "vpksss %0,%1,%2";
+  else
+return "vpksss %0,%2,%1";
+}
   [(set_attr "type" "vecperm")])
 
 (define_insn "altivec_vpksus"
@@ -1710,13 +1708,12 @@ (define_insn "altivec_vpksus"
(match_operand:VP 2 "register_operand" "v")]
   UNSPEC_VPACK_SIGN_UNS_SAT))]
   ""
-  "*
-  {
-if (VECTOR_ELT_ORDER_BIG)
-  return \"vpksus %0,%1,%2\";
-else
-  return \"vpksus %0,%2,%1\";
-  }"
+{
+  if (VECTOR_ELT_ORDER_BIG)
+return "vpksus %0,%1,%2";
+  else
+return "vpksus %0,%2,%1";
+}
   [(set_attr "type" "vecperm")])
 
 (define_insn "altivec_vpkuus"
@@ -1725,13 +1722,12 @@ (define_insn "altivec_vpkuus"
(match_operand:VP 2 "register_operand" "v")]
   UNSPEC_VPACK_UNS_UNS_SAT))]
   ""
-  "*
-  {
-if (VECTOR_ELT_ORDER_BIG)
-  return \"vpkuus %0,%1,%2\";
-else
-  return \"vpkuus %0,%2,%1\";
-  }"
+{
+  if (VECTOR_ELT_ORDER_BIG)
+return "vpkuus %0,%1,%2";
+  else
+return "vpkuus %0,%2,%1";
+}
   [(set_attr "type" "vecperm")])
 
 (define_insn "altivec_vpkuum"
@@ -1740,13 +1736,12 @@ (define_insn "altivec_vpkuum"
(match_operand:VP 2 "register_operand" "v")]
   UNSPEC_VPACK_UNS_UNS_MOD))]
   ""
-  "*
-  {
-if (VECTOR_ELT_ORDER_BIG)
-  return \"vpkuum %0,%1,%2\";
-else
-  return \"vpkuum %0,%2,%1\";
-  }"
+{
+  if (VECTOR_ELT_ORDER_BIG)
+return "vpkuum %0,%1,%2";
+  else
+return "vpkuum %0,%2,%1";
+}
   [(set_attr "type" "vecperm")])
 
 (define_insn "altivec_vpkuum_direct"
@@ -1755,13 +1750,12 @@ (define_insn "altivec_vpkuum_direct"
(match_operand:VP 2 "register_operand" "v")]
   UNSPEC_VPACK_UNS_UNS_MOD_DIRECT))]
   ""
-  "*
-  {
-if (BYTES_BIG_ENDIAN)
-  return \"vpkuum %0,%1,%2\";
-else
-  return \"vpkuum %0,%2,%1\";
-  }"
+{
+  if (BYTES_BIG_ENDIAN)
+return "vpkuum %0,%1,%2";
+  else
+return "vpkuum %0,%2,%1";
+}
   [(set_attr "type" "vecperm")])
 
 (define_insn "*altivec_vrl"
@@ -2348,7 +2342,6 @@ (define_expand "altivec_copysign_v4sf3"
(use (match_operand:V4SF 1 "register_operand" ""))
(use (match_operand:V4SF 2 "register_operand" ""))]
   "VECTOR_UNIT_ALTIVEC_P (V4SFmode)"
-  "
 {
   rtx mask = gen_reg_rtx (V4SImode);
   rtvec v = rtvec_alloc (4);
@@ -2363,7 +2356,7 @@ (define_expand "altivec_copysign_v4sf3"
   emit_insn (gen_vector_select_v4sf (operands[0], operands[1], operands[2],
 gen_lowpart (V4SFmode, mask)));
   DONE;
-}")
+})
 
 (define_insn "altivec_vsldoi_"
   [(set (match_operand:VM 0 "register_operand" "=v")
@@ -2670,8 +2663,7 @@ (define_expand "build_vector_mask_for_load"
   [(set (match_operand:V16QI 0 

Re: [PATCH] Character length cleanup for Coarray Fortran library

2018-02-21 Thread Damian Rouson
Hi Janne,

To be more specific, the new OpenCoarrays test failures after applying the 
patch are the following ones:

 image_fail_test_1 (Timeout)
 image_fail_and_sync_test_2 (Timeout)
 image_fail_and_sync_test_3 (Timeout)
 image_fail_and_get_test_1 (Timeout)

There are four.  I was mistaken when I wrote that there are three.

Damian

On February 21, 2018 at 12:56:05 PM, Janne Blomqvist 
(blomqvist.ja...@gmail.com) wrote:

On Wed, Feb 21, 2018 at 10:18 PM, Damian Rouson  
 wrote:  
>  
>  
>  
>  
> On February 21, 2018 at 11:35:47 AM, Janne Blomqvist 
> (blomqvist.ja...@gmail.com(mailto:blomqvist.ja...@gmail.com)) wrote:  
>  
>> PING  
>>  
>> Is anybody planning to work on this on the opencoarrays side? Or do  
>> you prefer that this is just committed and you can sort it out on your  
>> own schedule?  
>  
> Hi Janne,  
>  
> The trunk didn’t build for me the first time I tried soon after your initial 
> email and it took me until yesterday to find time to try again. Once I 
> succeeded, the patch created three new test failures. I’m wondering if 
> Andre’s recent patch (r257813) helped reduce the number. All new failures 
> involve the failed-images features developed by Alessandro and Andre, both of 
> whom now have new employment or contracts that prevent making significant 
> contributions to OpenCoarrays (although I hope that will change and am 
> working with Alessandro’s organization to attempt to get some of his time 
> allocated to gfortran/OpenCoarrays development).  
>  
> I think the most sustainable path forward is for me to assist any interested 
> gfortran developer in building OpenCoarrays on your local system and running 
> the test suite. At least that eliminates the reliance on OpenCoarrays 
> developers to run the tests every time a patch impacts OpenCoarrays. If we 
> can work together on building OpenCoarrays on your system, we can also look 
> together at the requisite changes to see if we can eliminate the failures.  
>  
> Oxford University Ph.D. student Daniel Garza visit Sourcery Institute March 
> 17 - June 2 to integrate the building of OpenCoarrays into the GCC build 
> system so I hope this will all be easier sooner.  

Ok, I'll see if I find the time to get opencoarrays built.  



--  
Janne Blomqvist  


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

2018-02-21 Thread Martin Sebor

On 02/13/2018 11:33 AM, Jason Merrill wrote:

On Tue, Feb 13, 2018 at 1:00 PM, Martin Sebor  wrote:

On 02/13/2018 07:47 AM, Jason Merrill wrote:


On Mon, Feb 12, 2018 at 6:39 PM, Martin Sebor  wrote:


I really don't think it's helpful to try to force noreturn
to match between the primary and its specializations.


I continue to disagree.


Can you explain what use case you are concerned about that isn't
already handled by the warning about noreturn function returning?


A specialization that forgot [[noreturn]] and therefore doesn't get
the desired warning.


For reference, the cases I consider important are:

1) "Unimplemented" primary template declared noreturn that throws
   or exits but whose specializations return a useful value and
   make use of attribute malloc (or one of the other blacklisted
   attributes).

2) The converse of (1).  A primary that returns a useful malloc
   like value and some of whose specializations are not/cannot
   be meaningfully implemented and are declared noreturn.


Right, but I still disagree with this use of noreturn, and therefore
don't consider these cases important.


Defining template specializations that differ from the primary
template in their implementation is idiomatic (analogous to
defining overloads or overridden virtual functions).



In any event, I am mainly interested in fixing the two bugs
(one a P1 regression).   If you consider changing the warning
aspect of the patch a condition of accepting the fix please let
me know.  Removing the noreturn keyword from the whitelist is
trivial.


Please do.


Attached is an updated patch with this change and with
the TREE_HAS_VOLATILE bit split up between types and functions.
I've also removed warn_unused_result from the blacklist (see
below).

Bootstrapped on x864_64, regression test is in progress.

Martin

While reviewing other related bugs I noticed 83502.  This patch
doesn't fix the first test case in the bug (attribute noinline
vs always_inline).  Somehow those are still copied from
the primary to the specialization and can cause conflicts.

It does fix the second test case but with the noreturn change
it would issue a bogus -Wmissing-attributes warning for the
explicit specialization below.  Adding the warn_unused_result
attribute to it would then make GCC complain about a conflict
between the added attribute and noreturn, while removing it
would lead to worse code.

  template 
  int __attribute__ ((warn_unused_result)) f (T) { return 0; }

  template <>
  int __attribute__ ((noreturn)) f (int) { throw 0; }

  void fi () { f (0); }

PR c++/83871 - wrong code for attribute const and pure on distinct template specializations
PR c++/83503 - [8 Regression] bogus -Wattributes for const and pure on function template specialization

gcc/ChangeLog:

	PR c++/83871
	* gcc/doc/invoke.texi (-Wmissing-attributes): New option.

gcc/c-family/ChangeLog:

	PR c++/83871
	* c.opt (-Wmissing-attributes): New option.

gcc/cp/ChangeLog:

	PR c++/83871
	PR c++/83503
	* cp-tree.h (warn_spec_missing_attributes): New function.
	((check_explicit_specialization): Add an argument.  Call the above
	function.
	* decl.c (duplicate_decls): Avoid applying primary function template's
	attributes to its explicit specializations.

gcc/testsuite/ChangeLog:

	PR c++/83871
	PR c++/83503
	* g++.dg/ext/attr-const-pure.C: New test.
	* g++.dg/ext/attr-malloc.C: New test.
	* g++.dg/ext/attr-nonnull.C: New test.
	* g++.dg/ext/attr-nothrow.C: New test.
	* g++.dg/ext/attr-nothrow-2.C: New test.
	* g++.dg/ext/attr-returns-nonnull.C: New test.
	* g++.dg/Wmissing-attributes.C: New test.

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 7fb386d..1d0682d 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -781,6 +781,11 @@ Wtemplates
 C++ ObjC++ Var(warn_templates) Warning
 Warn on primary template declaration.
 
+Wmissing-attributes
+C ObjC C++ ObjC++ Var(warn_missing_attributes) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
+Warn about declarations of entities that may be missing attributes
+that related entities have been declared with it.
+
 Wmissing-format-attribute
 C ObjC C++ ObjC++ Warning Alias(Wsuggest-attribute=format)
 ;
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index a6c75ae..6255914 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6469,7 +6469,8 @@ extern void end_specialization			(void);
 extern void begin_explicit_instantiation	(void);
 extern void end_explicit_instantiation		(void);
 extern void check_unqualified_spec_or_inst	(tree, location_t);
-extern tree check_explicit_specialization	(tree, tree, int, int);
+extern tree check_explicit_specialization	(tree, tree, int, int,
+		 tree = NULL_TREE);
 extern int num_template_headers_for_class	(tree);
 extern void check_template_variable		(tree);
 extern tree make_auto(void);
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 3ccea9e..1f012ef 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -1969,10 +1969,21 @@ 

[PATCH] Fix TYPE_EMPTY_P handling - x86_64 ABI issue (PR target/84502)

2018-02-21 Thread Jakub Jelinek
Hi!

The following testcase shows that we set TYPE_EMPTY_P just one of the
possibly many variants of an aggregate type.
On the testcase, in one case (in the callee) we check TYPE_EMPTY_P on X which is
the type variant for the using type, and in another case we check it on the
A type instead, which has TYPE_EMPTY_P set.

Bootstrap/regtest pending on x86_64-linux and i686-linux, ok if it passes?

2018-02-21  Jakub Jelinek  

PR target/84502
* stor-layout.c (finalize_type_size): Propagate TYPE_EMPTY_P flag
to all type variants.

* g++.dg/torture/pr84502.C: New test.

--- gcc/stor-layout.c.jj2018-02-13 21:23:29.187981310 +0100
+++ gcc/stor-layout.c   2018-02-21 21:43:24.783522853 +0100
@@ -1883,6 +1883,9 @@ finalize_type_size (tree type)
   && TREE_CODE (TYPE_SIZE_UNIT (type)) != INTEGER_CST)
 TYPE_SIZE_UNIT (type) = variable_size (TYPE_SIZE_UNIT (type));
 
+  /* Handle empty records as per the x86-64 psABI.  */
+  TYPE_EMPTY_P (type) = targetm.calls.empty_record_p (type);
+
   /* Also layout any other variants of the type.  */
   if (TYPE_NEXT_VARIANT (type)
   || type != TYPE_MAIN_VARIANT (type))
@@ -1895,6 +1898,7 @@ finalize_type_size (tree type)
   unsigned int precision = TYPE_PRECISION (type);
   unsigned int user_align = TYPE_USER_ALIGN (type);
   machine_mode mode = TYPE_MODE (type);
+  bool empty_p = TYPE_EMPTY_P (type);
 
   /* Copy it into all variants.  */
   for (variant = TYPE_MAIN_VARIANT (type);
@@ -1911,11 +1915,9 @@ finalize_type_size (tree type)
  SET_TYPE_ALIGN (variant, valign);
  TYPE_PRECISION (variant) = precision;
  SET_TYPE_MODE (variant, mode);
+ TYPE_EMPTY_P (variant) = empty_p;
}
 }
-
-  /* Handle empty records as per the x86-64 psABI.  */
-  TYPE_EMPTY_P (type) = targetm.calls.empty_record_p (type);
 }
 
 /* Return a new underlying object for a bitfield started with FIELD.  */
--- gcc/testsuite/g++.dg/torture/pr84502.C.jj   2018-02-21 22:13:22.279420017 
+0100
+++ gcc/testsuite/g++.dg/torture/pr84502.C  2018-02-21 22:13:03.727424362 
+0100
@@ -0,0 +1,20 @@
+// PR target/84502
+// { dg-do run }
+
+template
+struct A { };
+using X = A;
+
+void
+foo (X, int a1, int a2, int a3, int a4, int a5, int a6, int a7, int a8)
+{
+  if (a1 != 0 || a2 != 1 || a3 != 2 || a4 != 3
+  || a5 != 4 || a6 != 5 || a7 != 6 || a8 != 7)
+__builtin_abort ();
+}
+
+int
+main ()
+{
+  foo (X{}, 0, 1, 2, 3, 4, 5, 6, 7);
+}

Jakub


Re: PR84229 part 1: Avoid cloning of functions that calls va_arg_pack

2018-02-21 Thread Jakub Jelinek
On Wed, Feb 21, 2018 at 08:09:28PM +0100, Jan Hubicka wrote:
> --- ipa-cp.c  (revision 257844)
> +++ ipa-cp.c  (working copy)
> @@ -630,6 +630,24 @@ determine_versionability (struct cgraph_
>reason = "calls comdat-local function";
>  }
>  
> +  /* Functions calling BUILT_IN_VA_ARG_PACK and BUILT_IN_VA_ARG_PACK_LEN
> + works only when inlined.  Cloning them may still lead to better code

s/works/work/

> + becuase ipa-cp will not give up on cloning further.  If the function is

s/becuase/because/

> + external this however leads to wrong code becuase we may end up 
> producing

s/becuase/because/

> + offline copy of the function.  */
> +  if (DECL_EXTERNAL (node->decl))
> +for (cgraph_edge *edge = node->callees; !reason && edge;
> +  edge = edge->next_callee)
> +  if (DECL_BUILT_IN (edge->callee->decl)
> +   && DECL_BUILT_IN_CLASS (edge->callee->decl) == BUILT_IN_NORMAL)
> +{
> +   if (DECL_FUNCTION_CODE (edge->callee->decl) == BUILT_IN_VA_ARG_PACK)
> + reason = "external function which calls va_arg_pack";
> +   if (DECL_FUNCTION_CODE (edge->callee->decl)
> +   == BUILT_IN_VA_ARG_PACK_LEN)
> + reason = "external function which calls va_arg_pack_len";
> +}
> +
>if (reason && dump_file && !node->alias && !node->thunk.thunk_p)
>  fprintf (dump_file, "Function %s is not versionable, reason: %s.\n",
>node->dump_name (), reason);

Do you have a testcase for this, or is it LTO with too large input?

Jakub


[PATCH rs6000], Move Power 8 tests, fix ICE for vec_unsigned2, vec_signed2

2018-02-21 Thread Carl Love
GCC maintainers:

Per discussions with Segher, we felt it would be best to move the
vec_float2 test to a Power 8 test as it is only defined for Power 8 and
beyond. In doing this, I found that compiling builtins-3-runnable.c
with -mcpu=power7 then generated an ICE for vec_signed2 and
vec_unsigned2.  It seems that once gcc saw the unsupported vec_float2
it exited with -mcpu=power7 it exited before seeing the vec_signed2 and
vec_unsigned2 builtins.  The vec_signed2 and vec_unsigned2 are also
only defined for Power 8 and beyond.

This patch moves the three power 8 builtins to a new test file
builtins-3-runnable-p8.c.  It also fixed the ICE by restricting the
vec_signed2 and vec_unsigned2 builtins to Power 8.  The builtins-3-
runnable.c dg settings are then set to enable the test to run on Power
7.

The patch has been tested on:

 powerpc64-unknown-linux-gnu (Power 8BE)
 powerpc64le-unknown-linux-gnu
(Power 8LE)
 powerpc64le-unknown-linux-gnu (Power 9LE)

and no regressions were found.  Additionally, the tests were compiled
by hand with -mcpu=power7 to ensure gcc doesn't give and ICE.

Please let me know if the patch looks OK or not. Thanks.

   Carl Love

---

gcc/ChangeLog:

2018-02-19  Carl Love  

* config/rs6000/rs6000-builtin.def: Change VSIGNED2 and VUNSIGNED2
macro expansions from BU_VSX_2 to BU_P8V_VSX_2 and BU_VSX_OVERLOAD_2 to
BU_P8V_OVERLOAD_2.
* config/rs6000/rs6000-c.c: Change VSX_BUILTIN_VEC_VSIGNED2 to
P8V_BUILTIN_VEC_VSIGNED2.  Change VSX_BUILTIN_VEC_VUNSIGNED2 to
P8V_BUILTIN_VEC_VUNSIGNED2.

gcc/testsuite/ChangeLog:

2018-02-19  Carl Love  

* gcc.target/powerpc/builtins-3-runnable.c: Move tests for vec_float2,
vec_signed2 and vec_unsigned2 to new Power 8 test file.
* gcc.target/powerpc/builtins-3-runnable-p8.c: New test file for
Power 8 tests.
---
 gcc/config/rs6000/rs6000-builtin.def   |  10 +-
 gcc/config/rs6000/rs6000-c.c   |   4 +-
 .../gcc.target/powerpc/builtins-3-runnable-p8.c| 162 +
 .../gcc.target/powerpc/builtins-3-runnable.c   |  31 +---
 4 files changed, 170 insertions(+), 37 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/powerpc/builtins-3-runnable-p8.c

diff --git a/gcc/config/rs6000/rs6000-builtin.def 
b/gcc/config/rs6000/rs6000-builtin.def
index 876b1d9..f9548a0 100644
--- a/gcc/config/rs6000/rs6000-builtin.def
+++ b/gcc/config/rs6000/rs6000-builtin.def
@@ -1659,9 +1659,6 @@ BU_VSX_2 (CMPLE_U8HI, "cmple_u8hi", CONST,  
vector_ngtuv8hi)
 BU_VSX_2 (CMPLE_U4SI, "cmple_u4si", CONST,  vector_ngtuv4si)
 BU_VSX_2 (CMPLE_U2DI, "cmple_u2di", CONST,  vector_ngtuv2di)
 
-BU_VSX_2 (VEC_VSIGNED2_V2DF,  "vsigned2_v2df",CONST,  vsigned2_v2df)
-BU_VSX_2 (VEC_VUNSIGNED2_V2DF,"vunsigned2_v2df",  CONST,  vunsigned2_v2df)
-
 /* VSX abs builtin functions.  */
 BU_VSX_A (XVABSDP,   "xvabsdp",CONST,  absv2df2)
 BU_VSX_A (XVNABSDP,  "xvnabsdp",   CONST,  vsx_nabsv2df2)
@@ -1852,8 +1849,6 @@ BU_VSX_OVERLOAD_2 (XXMRGHW,  "xxmrghw")
 BU_VSX_OVERLOAD_2 (XXMRGLW,  "xxmrglw")
 BU_VSX_OVERLOAD_2 (XXSPLTD,  "xxspltd")
 BU_VSX_OVERLOAD_2 (XXSPLTW,  "xxspltw")
-BU_VSX_OVERLOAD_2 (VSIGNED2, "vsigned2")
-BU_VSX_OVERLOAD_2 (VUNSIGNED2,   "vunsigned2")
 
 /* 1 argument VSX overloaded builtin functions.  */
 BU_VSX_OVERLOAD_1 (DOUBLE,   "double")
@@ -1917,6 +1912,9 @@ BU_P8V_AV_1 (NEG_V2DF,  "neg_v2df",   CONST,  
negv2df2)
 BU_P8V_VSX_2 (FLOAT2_V2DF,"float2_v2df",   CONST,  float2_v2df)
 BU_P8V_VSX_2 (FLOAT2_V2DI,"float2_v2di",   CONST,  float2_v2di)
 BU_P8V_VSX_2 (UNS_FLOAT2_V2DI,"uns_float2_v2di",CONST,  
uns_float2_v2di)
+BU_P8V_VSX_2 (VEC_VSIGNED2_V2DF,   "vsigned2_v2df",CONST,  vsigned2_v2df)
+BU_P8V_VSX_2 (VEC_VUNSIGNED2_V2DF, "vunsigned2_v2df",  CONST,  vunsigned2_v2df)
+
 
 /* 1 argument altivec instructions added in ISA 2.07.  */
 BU_P8V_AV_1 (ABS_V2DI,   "abs_v2di",   CONST,  absv2di2)
@@ -2063,6 +2061,8 @@ BU_P8V_OVERLOAD_2 (VSUBUDM,   "vsubudm")
 BU_P8V_OVERLOAD_2 (VSUBUQM,"vsubuqm")
 BU_P8V_OVERLOAD_2 (FLOAT2,   "float2")
 BU_P8V_OVERLOAD_2 (UNS_FLOAT2,   "uns_float2")
+BU_P8V_OVERLOAD_2 (VSIGNED2, "vsigned2")
+BU_P8V_OVERLOAD_2 (VUNSIGNED2,   "vunsigned2")
 
 /* ISA 2.07 vector overloaded 3 argument functions.  */
 BU_P8V_OVERLOAD_3 (VADDECUQ,   "vaddecuq")
diff --git a/gcc/config/rs6000/rs6000-c.c b/gcc/config/rs6000/rs6000-c.c
index 6e4a269..cc8e4e1 100644
--- a/gcc/config/rs6000/rs6000-c.c
+++ b/gcc/config/rs6000/rs6000-c.c
@@ -5876,7 +5876,7 @@ const struct altivec_builtin_types 
altivec_overloaded_builtins[] = {
 RS6000_BTI_V4SI, RS6000_BTI_V2DF, 0, 0 },
   { VSX_BUILTIN_VEC_VSIGNEDO, VSX_BUILTIN_VEC_VSIGNEDO_V2DF,
 RS6000_BTI_V4SI, RS6000_BTI_V2DF, 0, 0 },
-  { 

Re: C++ PATCH to fix ICE with invalid cast (PR c++/84493)

2018-02-21 Thread Jason Merrill
OK.

On Wed, Feb 21, 2018 at 5:36 AM, Marek Polacek  wrote:
> Here we can prevent a crash on invalid by using require_open where '{' is
> expected.  require_open uses cp_parser_require whereas consume_open has
> an assert:
> gcc_assert (tok->type == traits_t::open_token_type);
> which triggers here.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2018-02-21  Marek Polacek  
>
> PR c++/84493
> * parser.c (cp_parser_braced_list): Use require_open instead of
> consume_open.
>
> * g++.dg/parse/error59.C: New test.
>
> diff --git gcc/cp/parser.c gcc/cp/parser.c
> index 2bb0d2da5fe..4fa546a086c 100644
> --- gcc/cp/parser.c
> +++ gcc/cp/parser.c
> @@ -21925,7 +21925,7 @@ cp_parser_braced_list (cp_parser* parser, bool* 
> non_constant_p)
>
>/* Consume the `{' token.  */
>matching_braces braces;
> -  braces.consume_open (parser);
> +  braces.require_open (parser);
>/* Create a CONSTRUCTOR to represent the braced-initializer.  */
>initializer = make_node (CONSTRUCTOR);
>/* If it's not a `}', then there is a non-trivial initializer.  */
> diff --git gcc/testsuite/g++.dg/parse/error59.C 
> gcc/testsuite/g++.dg/parse/error59.C
> index e69de29bb2d..2c44e210366 100644
> --- gcc/testsuite/g++.dg/parse/error59.C
> +++ gcc/testsuite/g++.dg/parse/error59.C
> @@ -0,0 +1,6 @@
> +// PR c++/84493
> +
> +void foo()
> +{
> +  (struct {}x){}; // { dg-error "" }
> +}
>
> Marek


[PATCH] avoid bogus -Wstringop-truncation when inlining (PR 84480)

2018-02-21 Thread Martin Sebor

The attached patch eliminates -Wstringop-truncation false
positives reported in bug 84480 - bogus -Wstringop-truncation
despite assignment with an inlined string literal.  It does
that by delegating early strncpy checks during folding to
the same machinery in tree-ssa-strlen that looks for a NUL
assignment to the destination the next statement.

The patch also adds inlining context to the warnings via
the %G directive.

Tested on x86_64-linux with no regressions.

Martin
PR tree-optimization/84480 - bogus -Wstringop-truncation despite assignment with an inlined string literal

gcc/ChangeLog:

	PR tree-optimization/84480
	* gimple-fold.c (gimple_fold_builtin_strcpy): Move warnings
	to maybe_diag_stxncpy_trunc.  Call it.
	* tree-ssa-strlen.c (maybe_diag_stxncpy_trunc): Integrate warnings
	from gimple_fold_builtin_strcpy.  Print inlining stack.
	(handle_builtin_stxncpy): Print inlining stack.
	* tree-ssa-strlen.h (maybe_diag_stxncpy_trunc): Declare.

gcc/testsuite/ChangeLog:

	PR tree-optimization/84480
	* c-c++-common/Wstringop-truncation.c: Adjust text of expected warnings.
	* g++.dg/warn/Wstringop-truncation-1.C: New test.
Index: gcc/gimple-fold.c
===
--- gcc/gimple-fold.c	(revision 257875)
+++ gcc/gimple-fold.c	(working copy)
@@ -65,6 +65,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "intl.h"
 #include "calls.h"
 #include "tree-vector-builder.h"
+#include "tree-ssa-strlen.h"
 
 /* Return true when DECL can be referenced from current unit.
FROM_DECL (if non-null) specify constructor of variable DECL was taken from.
@@ -1710,38 +1711,9 @@ gimple_fold_builtin_strncpy (gimple_stmt_iterator
   if (tree_int_cst_lt (ssize, len))
 return false;
 
-  if (!nonstring)
-{
-  if (tree_int_cst_lt (len, slen))
-	{
-	  tree fndecl = gimple_call_fndecl (stmt);
-	  gcall *call = as_a  (stmt);
+  /* Diagnose truncation that leaves the copy unterminated.  */
+  maybe_diag_stxncpy_trunc (*gsi, src, len);
 
-	  warning_at (loc, OPT_Wstringop_truncation,
-		  (tree_int_cst_equal (size_one_node, len)
-		   ? G_("%G%qD output truncated copying %E byte "
-			"from a string of length %E")
-		   : G_("%G%qD output truncated copying %E bytes "
-			"from a string of length %E")),
-		  call, fndecl, len, slen);
-	}
-  else if (tree_int_cst_equal (len, slen))
-	{
-	  tree fndecl = gimple_call_fndecl (stmt);
-	  gcall *call = as_a  (stmt);
-
-	  warning_at (loc, OPT_Wstringop_truncation,
-		  (tree_int_cst_equal (size_one_node, len)
-		   ? G_("%G%qD output truncated before terminating nul "
-			"copying %E byte from a string of the same "
-			"length")
-		   : G_("%G%qD output truncated before terminating nul "
-			"copying %E bytes from a string of the same "
-			"length")),
-		  call, fndecl, len);
-	}
-}
-
   /* OK transform into builtin memcpy.  */
   tree fn = builtin_decl_implicit (BUILT_IN_MEMCPY);
   if (!fn)
Index: gcc/testsuite/c-c++-common/Wstringop-truncation.c
===
--- gcc/testsuite/c-c++-common/Wstringop-truncation.c	(revision 257875)
+++ gcc/testsuite/c-c++-common/Wstringop-truncation.c	(working copy)
@@ -188,7 +188,7 @@ void test_strncpy_ptr (char *d, const char* s, con
   CPY (d, CHOOSE (s, t), 2);
 
   CPY (d, CHOOSE ("", "123"), 1);   /* { dg-warning ".strncpy\[^\n\r\]* output may be truncated copying 1 byte from a string of length 3" } */
-  CPY (d, CHOOSE ("1", "123"), 1);  /* { dg-warning ".strncpy\[^\n\r\]* output truncated copying 1 byte from a string of length 1" } */
+  CPY (d, CHOOSE ("1", "123"), 1);  /* { dg-warning ".strncpy\[^\n\r\]* output truncated before terminating nul copying 1 byte from a string of the same length" } */
   CPY (d, CHOOSE ("12", "123"), 1); /* { dg-warning ".strncpy\[^\n\r\]* output truncated copying 1 byte from a string of length 2" } */
   CPY (d, CHOOSE ("123", "12"), 1); /* { dg-warning ".strncpy\[^\n\r\]* output truncated copying 1 byte from a string of length 2" } */
 
@@ -331,7 +331,7 @@ void test_strncpy_array (Dest *pd, int i, const ch
 /* This might be better written using memcpy() but it's safe so
it probably shouldn't be diagnosed.  It currently triggers
a warning because of bug 81704.  */
-strncpy (dst7, "0123456", sizeof dst7);   /* { dg-bogus "truncated" "bug 81704" { xfail *-*-* } } */
+strncpy (dst7, "0123456", sizeof dst7);   /* { dg-bogus "\\\[-Wstringop-truncation]" "bug 81704" { xfail *-*-* } } */
 dst7[sizeof dst7 - 1] = '\0';
 sink (dst7);
   }
@@ -350,7 +350,7 @@ void test_strncpy_array (Dest *pd, int i, const ch
   }
 
   {
-strncpy (pd->a5, "01234", sizeof pd->a5);   /* { dg-bogus "truncated" "bug 81704" { xfail *-*-* } } */
+strncpy (pd->a5, "01234", sizeof pd->a5);   /* { dg-bogus "\\\[-Wstringop-truncation]" "bug 81704" { xfail *-*-* } } */
 pd->a5[sizeof pd->a5 - 1] = '\0';
 sink 

Re: [PATCH] Character length cleanup for Coarray Fortran library

2018-02-21 Thread Izaak Beekman
Reach out to Damian or me if you encounter any frustration. You should be
all set with a recent version of CMake and MPICH or OpenMPI installed. It
should look something like:

git clone https://github.com/sourceryinstitute/OpenCoarrays
cd OpenCoarrays
mkdir build
cd build
export FC=/path/to/gfortran/build/bin/gfortran
export CC=/path/to/gcc/build/bin/gcc
cmake ..
make -j
make check

Thanks,
Zaak

On Wed, Feb 21, 2018 at 3:56 PM Janne Blomqvist 
wrote:

> On Wed, Feb 21, 2018 at 10:18 PM, Damian Rouson
>  wrote:
> >
> >
> >
> >
> > On February 21, 2018 at 11:35:47 AM, Janne Blomqvist (
> blomqvist.ja...@gmail.com(mailto:blomqvist.ja...@gmail.com)) wrote:
> >
> >> PING
> >>
> >> Is anybody planning to work on this on the opencoarrays side? Or do
> >> you prefer that this is just committed and you can sort it out on your
> >> own schedule?
> >
> > Hi Janne,
> >
> > The trunk didn’t build for me the first time I tried soon after your
> initial email and it took me until yesterday to find time to try again.
> Once I succeeded, the patch created three new test failures.  I’m wondering
> if Andre’s recent patch (r257813) helped reduce the number.  All new
> failures involve the failed-images features developed by Alessandro and
> Andre, both of whom now have new employment or contracts that prevent
> making significant contributions to OpenCoarrays (although I hope that will
> change and am working with Alessandro’s organization to attempt to get some
> of his time allocated to gfortran/OpenCoarrays development).
> >
> > I think the most sustainable path forward is for me to assist any
> interested gfortran developer in building OpenCoarrays on your local system
> and running the test suite.  At least that eliminates the reliance on
> OpenCoarrays developers to run the tests every time a patch impacts
> OpenCoarrays.  If we can work together on building OpenCoarrays on your
> system, we can also look together at the requisite changes to see if we can
> eliminate the failures.
> >
> > Oxford University Ph.D. student Daniel Garza visit Sourcery Institute
> March 17 - June 2 to integrate the building of OpenCoarrays into the GCC
> build system so I hope this will all be easier sooner.
>
> Ok, I'll see if I find the time to get opencoarrays built.
>
>
>
> --
> Janne Blomqvist
>


Re: [PATCH] Character length cleanup for Coarray Fortran library

2018-02-21 Thread Janne Blomqvist
On Wed, Feb 21, 2018 at 10:18 PM, Damian Rouson
 wrote:
>
>
>
>
> On February 21, 2018 at 11:35:47 AM, Janne Blomqvist 
> (blomqvist.ja...@gmail.com(mailto:blomqvist.ja...@gmail.com)) wrote:
>
>> PING
>>
>> Is anybody planning to work on this on the opencoarrays side? Or do
>> you prefer that this is just committed and you can sort it out on your
>> own schedule?
>
> Hi Janne,
>
> The trunk didn’t build for me the first time I tried soon after your initial 
> email and it took me until yesterday to find time to try again.  Once I 
> succeeded, the patch created three new test failures.  I’m wondering if 
> Andre’s recent patch (r257813) helped reduce the number.  All new failures 
> involve the failed-images features developed by Alessandro and Andre, both of 
> whom now have new employment or contracts that prevent making significant 
> contributions to OpenCoarrays (although I hope that will change and am 
> working with Alessandro’s organization to attempt to get some of his time 
> allocated to gfortran/OpenCoarrays development).
>
> I think the most sustainable path forward is for me to assist any interested 
> gfortran developer in building OpenCoarrays on your local system and running 
> the test suite.  At least that eliminates the reliance on OpenCoarrays 
> developers to run the tests every time a patch impacts OpenCoarrays.  If we 
> can work together on building OpenCoarrays on your system, we can also look 
> together at the requisite changes to see if we can eliminate the failures.
>
> Oxford University Ph.D. student Daniel Garza visit Sourcery Institute March 
> 17 - June 2 to integrate the building of OpenCoarrays into the GCC build 
> system so I hope this will all be easier sooner.

Ok, I'll see if I find the time to get opencoarrays built.



-- 
Janne Blomqvist


Re: [PATCH] Character length cleanup for Coarray Fortran library

2018-02-21 Thread Janne Blomqvist
PING

Is anybody planning to work on this on the opencoarrays side? Or do
you prefer that this is just committed and you can sort it out on your
 own schedule?

On Sat, Feb 10, 2018 at 11:20 PM, Janne Blomqvist
 wrote:
> On Sat, Feb 10, 2018 at 9:34 PM, Damian Rouson
>  wrote:
>>
>> I’ll try applying the patch and testing it.
>
> As such, I'm sure that will fail as the patch changes the coarray API
> and thus needs corresponding changes on the OpenCoarrays side as well.
>
>>  I have about a 50% success rate with getting patches to apply cleanly.  Is 
>> this available on an svn or git branch that I can just checkout rather than 
>> attempting to apply the patch?  I find that process to be much more reliable.
>>
>> Going forward, please submit OpenCoarrays questions or issues via our issues 
>> page:
>>
>> https://github.com/sourceryinstitute/OpenCoarrays/issues
>
> Ok, I just filed https://github.com/sourceryinstitute/OpenCoarrays/issues/497
>
>> Most OpenCoarrays contributors don’t monitor the gfortran mailing list 
>> closely so we’re unlikely to see emails to the list in a timely manner.
>>
>> Also, when making changes that impact OpenCoarrays, please build 
>> OpenCoarrays and run our test suite.  We recently made several changes to 
>> our installation documentation and installer script based on feedback 
>> received from Steve Kargl.  One result is a set of instructions written by 
>> Zaak with GCC developers as an intended audience:
>>
>> https://github.com/sourceryinstitute/OpenCoarrays/blob/master/INSTALL
>>
>> Several other installation changes inspired by Steve’s feedback are listed 
>> here:
>>
>> https://github.com/sourceryinstitute/OpenCoarrays/issues/478
>>
>> and these will make it into a release shortly, but are already all available 
>> via git clone. I’m looking forward to hosting Oxford University Ph.D. 
>> student Daniel Garza, from March 17 to June 2, to work on finishing the 
>> integration of OpenCoarrays into the GCC autotools scripts so that 
>> OpenCoarrays will be built anytime gfortran is built so hopefully it will be 
>> even easier for everyone to build and test with OpenCoarrays soon.
>
> Thanks; I should look into building opencoarrays at some point. For
> this patch I was hoping that someone familiar with the OpenCoarrays
> code base could do the equivalent fix there and test it, as it rather
> mechanical and simple.
>
>
> --
> Janne Blomqvist



-- 
Janne Blomqvist


Re: [PATCH] Character length cleanup for Coarray Fortran library

2018-02-21 Thread Damian Rouson
 



On February 21, 2018 at 11:35:47 AM, Janne Blomqvist 
(blomqvist.ja...@gmail.com(mailto:blomqvist.ja...@gmail.com)) wrote:

> PING  
>  
> Is anybody planning to work on this on the opencoarrays side? Or do  
> you prefer that this is just committed and you can sort it out on your  
> own schedule?  

Hi Janne,

The trunk didn’t build for me the first time I tried soon after your initial 
email and it took me until yesterday to find time to try again.  Once I 
succeeded, the patch created three new test failures.  I’m wondering if Andre’s 
recent patch (r257813) helped reduce the number.  All new failures involve the 
failed-images features developed by Alessandro and Andre, both of whom now have 
new employment or contracts that prevent making significant contributions to 
OpenCoarrays (although I hope that will change and am working with Alessandro’s 
organization to attempt to get some of his time allocated to 
gfortran/OpenCoarrays development).

I think the most sustainable path forward is for me to assist any interested 
gfortran developer in building OpenCoarrays on your local system and running 
the test suite.  At least that eliminates the reliance on OpenCoarrays 
developers to run the tests every time a patch impacts OpenCoarrays.  If we can 
work together on building OpenCoarrays on your system, we can also look 
together at the requisite changes to see if we can eliminate the failures.

Oxford University Ph.D. student Daniel Garza visit Sourcery Institute March 17 
- June 2 to integrate the building of OpenCoarrays into the GCC build system so 
I hope this will all be easier sooner.

Damian 




Re: [PATCH] Add "native" as a valid option value for -march= on i386 (PR driver/83193).

2018-02-21 Thread Jakub Jelinek
On Wed, Feb 21, 2018 at 08:33:21AM +0100, Martin Liška wrote:
> >From 8f1783a9017ec06c578fd644e46168ec5763d5ca Mon Sep 17 00:00:00 2001
> From: marxin 
> Date: Tue, 20 Feb 2018 14:21:05 +0100
> Subject: [PATCH 3/3] Add "native" as a valid option value for -march= on i386
>  (PR driver/83193).
> 
> gcc/ChangeLog:
> 
> 2018-02-20  Martin Liska  
> 
>   PR driver/83193
>   * config/i386/i386.c (ix86_option_override_internal):
>   Add "native" as a possible value.
> ---
>  gcc/config/i386/i386.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index d54e7301e84..9f2c5218ae5 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -4193,6 +4193,11 @@ ix86_option_override_internal (bool main_args_p,
>   || ((processor_alias_table[i].flags & PTA_64BIT) != 0)))
> candidates.safe_push (processor_alias_table[i].name);
>  
> +#ifdef HAVE_LOCAL_CPU_DETECT
> +  /* Add also "native" as possible value.  */
> +  candidates.safe_push ("native");
> +#endif

This just adds "native" as possible value for -march, but shouldn't it be
also for -mtune, i.e. around line 4268?

> +
>char *s;
>const char *hint
>   = candidates_list_and_hint (opts->x_ix86_arch_string, s, candidates);
> -- 
> 2.16.1
> 


Jakub


Re: PR84229 part 1: Avoid cloning of functions that calls va_arg_pack

2018-02-21 Thread Jan Hubicka
> On Wed, Feb 21, 2018 at 08:09:28PM +0100, Jan Hubicka wrote:
> > --- ipa-cp.c(revision 257844)
> > +++ ipa-cp.c(working copy)
> > @@ -630,6 +630,24 @@ determine_versionability (struct cgraph_
> >reason = "calls comdat-local function";
> >  }
> >  
> > +  /* Functions calling BUILT_IN_VA_ARG_PACK and BUILT_IN_VA_ARG_PACK_LEN
> > + works only when inlined.  Cloning them may still lead to better code
> 
> s/works/work/
> 
> > + becuase ipa-cp will not give up on cloning further.  If the function 
> > is
> 
> s/becuase/because/
> 
> > + external this however leads to wrong code becuase we may end up 
> > producing
> 
> s/becuase/because/

Thanks, fixed.

> 
> > + offline copy of the function.  */
> > +  if (DECL_EXTERNAL (node->decl))
> > +for (cgraph_edge *edge = node->callees; !reason && edge;
> > +edge = edge->next_callee)
> > +  if (DECL_BUILT_IN (edge->callee->decl)
> > + && DECL_BUILT_IN_CLASS (edge->callee->decl) == BUILT_IN_NORMAL)
> > +{
> > + if (DECL_FUNCTION_CODE (edge->callee->decl) == BUILT_IN_VA_ARG_PACK)
> > +   reason = "external function which calls va_arg_pack";
> > + if (DECL_FUNCTION_CODE (edge->callee->decl)
> > + == BUILT_IN_VA_ARG_PACK_LEN)
> > +   reason = "external function which calls va_arg_pack_len";
> > +}
> > +
> >if (reason && dump_file && !node->alias && !node->thunk.thunk_p)
> >  fprintf (dump_file, "Function %s is not versionable, reason: %s.\n",
> >  node->dump_name (), reason);
> 
> Do you have a testcase for this, or is it LTO with too large input?

There is reduced testcase in the PR which probably should be rejected by C
frontend because it is uses va_arg_pack on non-variadic function.
I plan to send additional patches and then come with a testcase (because current
one should be early inlined and thus it will stop testing ipa-cp)

Honza


PR84229 part 1: Avoid cloning of functions that calls va_arg_pack

2018-02-21 Thread Jan Hubicka
Hi,
this fixes first part of the issue in PR84229. The actual testcase dies because
ipa-cp decides to cone function calling va_arg_pack which is later not inlined.
Such functions works only when they are inlined and thus it makes no sense to
inline them.  I disabled cloning only for external function (where this can
make us refuse valid code) becuase otherwise we could prevent some useful
cloning which depends on this.

The underlying issue is deeper. This bug triggers while building firefox
with LTO and -O3 on trunk and GCC 7 branch as well. It is fortified 
implementation
of open which for some reason is not alwyas_inline in libc headers and we decide
to not early inline it as it looks large (it has a lot of logic that will be 
optimized
out after inlining).

I think we miss fortification diagnostics this way. I have some followup patches
that makes the function body look smaller but not small enough for early 
inlining.
I do not see how this is supposed to work well without always_inline attribute.

Bootstrapped/regtested x86_64-linux, comitted.

Honza

PR c/84229
* ipa-cp.c (determine_versionability): Do not version functions caling
va_arg_pack.
Index: ipa-cp.c
===
--- ipa-cp.c(revision 257844)
+++ ipa-cp.c(working copy)
@@ -630,6 +630,24 @@ determine_versionability (struct cgraph_
   reason = "calls comdat-local function";
 }
 
+  /* Functions calling BUILT_IN_VA_ARG_PACK and BUILT_IN_VA_ARG_PACK_LEN
+ works only when inlined.  Cloning them may still lead to better code
+ becuase ipa-cp will not give up on cloning further.  If the function is
+ external this however leads to wrong code becuase we may end up producing
+ offline copy of the function.  */
+  if (DECL_EXTERNAL (node->decl))
+for (cgraph_edge *edge = node->callees; !reason && edge;
+edge = edge->next_callee)
+  if (DECL_BUILT_IN (edge->callee->decl)
+ && DECL_BUILT_IN_CLASS (edge->callee->decl) == BUILT_IN_NORMAL)
+{
+ if (DECL_FUNCTION_CODE (edge->callee->decl) == BUILT_IN_VA_ARG_PACK)
+   reason = "external function which calls va_arg_pack";
+ if (DECL_FUNCTION_CODE (edge->callee->decl)
+ == BUILT_IN_VA_ARG_PACK_LEN)
+   reason = "external function which calls va_arg_pack_len";
+}
+
   if (reason && dump_file && !node->alias && !node->thunk.thunk_p)
 fprintf (dump_file, "Function %s is not versionable, reason: %s.\n",
 node->dump_name (), reason);


[PATCH, rs6000] tighten target statements for a p8 and p9 specific tests

2018-02-21 Thread Will Schmidt

Hi, 
  Update the dg-requires statements for these tests to specify the target.
These tests may otherwise fail on a system missing p9 assembler support.
(Seen in a p6 environment).

OK for trunk?
Thanks,
-Will

[testsuite]

2018-02-21  Will Schmidt  

* gcc.target/powerpc/pr80695-p8.c: Update dg-requires stanza.
* gcc.target/powerpc/pr80695-p9.c: Update dg-requires stanza.

diff --git a/gcc/testsuite/gcc.target/powerpc/pr80695-p8.c 
b/gcc/testsuite/gcc.target/powerpc/pr80695-p8.c
index 165079a..b5d007e 100644
--- a/gcc/testsuite/gcc.target/powerpc/pr80695-p8.c
+++ b/gcc/testsuite/gcc.target/powerpc/pr80695-p8.c
@@ -1,6 +1,6 @@
-/* { dg-do compile { target { powerpc*-*-* } } } */
+/* { dg-do compile { target { powerpc_p8vector_ok } } } */
 /* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { 
"-mcpu=power8" } } */
 /* { dg-require-effective-target vect_int } */
 /* { dg-options "-mcpu=power8 -O3 -fdump-tree-slp-details" } */
 
 /* PR80695: Verify cost model for vec_construct on POWER8.  */
diff --git a/gcc/testsuite/gcc.target/powerpc/pr80695-p9.c 
b/gcc/testsuite/gcc.target/powerpc/pr80695-p9.c
index a81f90a..302b1c5 100644
--- a/gcc/testsuite/gcc.target/powerpc/pr80695-p9.c
+++ b/gcc/testsuite/gcc.target/powerpc/pr80695-p9.c
@@ -1,6 +1,6 @@
-/* { dg-do compile { target { powerpc*-*-* } } } */
+/* { dg-do compile { target { powerpc_p9vector_ok } } } */
 /* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { 
"-mcpu=power9" } } */
 /* { dg-require-effective-target vect_int } */
 /* { dg-options "-mcpu=power9 -O3 -fdump-tree-slp-details" } */
 
 /* PR80695: Verify cost model for vec_construct on POWER9.  */




[PATCH, rs6000] Update altivec-7 testcase(s).

2018-02-21 Thread Will Schmidt
Hi, 
  This patch moves the vsx related content from the altivec-7-be test into
a new vsx-7-be test.
This fixes up some test failures as seen on older power systems.

OK for trunk?
Thanks,
Will

[testsuite]

2018-02-21  Will Schmidt  

* gcc.target/powerpc/altivec-7-be.c: Remove VSX content.
* gcc.target/powerpc/altivec-7.h: Remove VSX content.
* gcc.target/powerpc/vsx-7-be.c: New test. (Add VSX content).

diff --git a/gcc/testsuite/gcc.target/powerpc/altivec-7-be.c 
b/gcc/testsuite/gcc.target/powerpc/altivec-7-be.c
index cbc31e6..9024159 100644
--- a/gcc/testsuite/gcc.target/powerpc/altivec-7-be.c
+++ b/gcc/testsuite/gcc.target/powerpc/altivec-7-be.c
@@ -19,17 +19,12 @@
 */
 
 /* { dg-final { scan-assembler-times "vpkpx" 2 } } */
 /* { dg-final { scan-assembler-times "vmulesb" 1 } } */
 /* { dg-final { scan-assembler-times "vmulosb" 1 } } */
-/* { dg-final { scan-assembler-times "lxvd2x" 6 } } */
 /* { dg-final { scan-assembler-times "lvewx" 2 } } */
 /* { dg-final { scan-assembler-times "lvxl" 1 } } */
 /* { dg-final { scan-assembler-times "vupklsh" 1 } } */
 /* { dg-final { scan-assembler-times "vupkhsh" 1 } } */
-/* { dg-final { scan-assembler-times "xxlnor" 4 } } */
-/* { dg-final { scan-assembler-times "xxland" 4 } } */
-/* { dg-final { scan-assembler-times "xxlxor" 5 } } */
-/* { dg-final { scan-assembler-times "vupkhpx" 1 } } */
 
 /* Source code for the test in altivec-7.h */
 #include "altivec-7.h"
diff --git a/gcc/testsuite/gcc.target/powerpc/altivec-7.h 
b/gcc/testsuite/gcc.target/powerpc/altivec-7.h
index ff87deb..4dedcd8 100644
--- a/gcc/testsuite/gcc.target/powerpc/altivec-7.h
+++ b/gcc/testsuite/gcc.target/powerpc/altivec-7.h
@@ -15,11 +15,10 @@ vector signed int *vecint;
 vector signed short *vecshort;
 vector unsigned char *vecuchar;
 vector unsigned int *vecuint;
 vector unsigned short *vecushort;
 vector float *vecfloat;
-vector double *vecdouble;
 
 int main ()
 {
   *vecfloat++ = vec_andc((vector bool int)vecint[0], vecfloat[1]);
   *vecfloat++ = vec_andc(vecfloat[0], (vector bool int)vecint[1]);
@@ -41,10 +40,8 @@ int main ()
   *vecushort++ = vec_vxor(vecushort[0], (vector bool short)vecshort[1]);
   *vecuint++ = vec_ld(var_int[0], uintp[1]);
   *vecuint++ = vec_lvx(var_int[0], uintp[1]);
   *vecuint++ = vec_vmsumubm(vecuchar[0], vecuchar[1], vecuint[2]);
   *vecuchar++ = vec_xor(vecuchar[0], (vector unsigned char)vecchar[1]);
-  *vecdouble++ = vec_unpackl(vecfloat[0]);
-  *vecdouble++ = vec_unpackh(vecfloat[0]);
 
   return 0;
 }
diff --git a/gcc/testsuite/gcc.target/powerpc/vsx-7-be.c 
b/gcc/testsuite/gcc.target/powerpc/vsx-7-be.c
new file mode 100644
index 000..d5bb309
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/vsx-7-be.c
@@ -0,0 +1,42 @@
+/* { dg-do compile { target powerpc64-*-* } } */
+/* { dg-require-effective-target powerpc_vsx_ok } */
+/* { dg-options "-mvsx" } */
+
+/* This is an extension of altivec-7-be.c, with vsx target features included. 
*/
+
+/* Expected results for Big Endian:
+(from altivec-7.h)
+ vec_packpx vpkpx
+ vec_ld lxv2x
+ vec_ldelvewx
+ vec_ldllxvl
+ vec_lvewx  lvewx
+ vec_andc   xxnor
+xxland
+ vec_vxor   xxlxor
+ vec_vmsumubm   vmsumubm
+ vec_vmulesbvmulesb
+ vec_vmulosbvmulosb
+(from vsx-7.h)
+ vec_unpacklvupkhsh
+ vec_unpackhvupklsh
+*/
+
+/* { dg-final { scan-assembler-times "vpkpx" 2 } } */
+/* { dg-final { scan-assembler-times "vmulesb" 1 } } */
+/* { dg-final { scan-assembler-times "vmulosb" 1 } } */
+/* { dg-final { scan-assembler-times "lxvd2x" 6 } } */
+/* { dg-final { scan-assembler-times "lvewx" 2 } } */
+/* { dg-final { scan-assembler-times "lvxl" 1 } } */
+/* { dg-final { scan-assembler-times "vupklsh" 1 } } */
+/* { dg-final { scan-assembler-times "vupkhsh" 1 } } */
+/* { dg-final { scan-assembler-times "xxlnor" 4 } } */
+/* { dg-final { scan-assembler-times "xxland" 4 } } */
+/* { dg-final { scan-assembler-times "xxlxor" 5 } } */
+/* { dg-final { scan-assembler-times "vupkhpx" 1 } } */
+
+/* Source code for the 'altivec' test in altivec-7.h */
+/* Source code for the 'vsx' required tests in vsx-7.h */
+
+#include "altivec-7.h"
+#include "vsx-7.h"
diff --git a/gcc/testsuite/gcc.target/powerpc/vsx-7.h 
b/gcc/testsuite/gcc.target/powerpc/vsx-7.h
new file mode 100644
index 000..fe55472
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/vsx-7.h
@@ -0,0 +1,18 @@
+
+/* This test code is included into vsx-7-be.c.
+ * this is meant to supplement code in altivec-7.h.  */
+
+#include 
+
+
+vector float *vecfloat;
+vector double *vecdouble;
+
+int main2 ()
+{
+
+  *vecdouble++ = vec_unpackl(vecfloat[0]);
+  *vecdouble++ = vec_unpackh(vecfloat[0]);
+
+  

[PATCH, rs6000] fold-vec-mult* testcase updates for power9

2018-02-21 Thread Will Schmidt
Hi,

An update for the fold-vec-mult-int128-p9.c test to include more of
the instructions generated for a vec_mul() with a power9 target.

[testsuite]

2018-02-21  Will Schmidt  

* fold-vec-mult-int128-p9.c: Add maddld insn to expected output.

diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-mult-int128-p9.c 
b/gcc/testsuite/gcc.target/powerpc/fold-vec-mult-int128-p9.c
index 6571884..5d3d4aa 100644
--- a/gcc/testsuite/gcc.target/powerpc/fold-vec-mult-int128-p9.c
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-mult-int128-p9.c
@@ -20,7 +20,7 @@ vector unsigned __int128
 test2 (vector unsigned __int128 x, vector unsigned __int128 y)
 {
   return vec_mul (x, y);
 }
 
-/* { dg-final { scan-assembler-times {\mmulld\M} 4 } } */
+/* { dg-final { scan-assembler-times {\mmulld\M|\mmaddld\M} 6 } } */
 /* { dg-final { scan-assembler-times {\mmulhdu\M} 2 } } */




Re: [PATCH] Fix pdftex miscompilation due to get_range_strlen (PR tree-optimization/84478)

2018-02-21 Thread Martin Sebor

On 02/20/2018 08:25 PM, Jeff Law wrote:

On 02/20/2018 04:59 PM, Martin Sebor wrote:

It would help if you explained why you think it is a good idea
ignoring the other phi arguments if you have one (or more) where you can
determine length.


It's a heuristic that was meant just for the -Wformat-overflow
warning.  When making decisions that affect code generation it's
obviously not correct to ignore the possibility that unknown
arguments may be shorter than the minimum or longer than
the maximum.  The fuzzy argument was meant to differentiate
between two got but I forgot about it when I added the fix
for PR 83671.

For GCC 8 I don't have a preference for how to fix this as long
as it doesn't regress the warning tests.

I think the ultimate solution (for GCC 9) may be to either
disable the heuristic for code generation purposes (e.g., via
another argument/flag) or provide a pointer argument to indicate
to the caller that the minimum is based on known strings, and that
the real minimum may be zero.

I'm still getting refamiliar with this code.  But one thing that jumps
out immediately is how much this reminds me of the discussion we had
around 77608 -- where I argued that returning something that was not
conservatively correct was just asking for long term problems.

I realize we're talking about different routines, but the concerns are
the same -- when we return something that is not conservatively correct
it's easy for someone to mistakenly use those results for code
generation purposes.

The fuzzy stuff is in there to reduce the false positive rates and we're
not *supposed* to be using fuzzy results for code generation purposes,
but as I argued in 77608, it's easy to miss.

I'll reiterate my desire to make this kind of mistake harder to make.


I agree.  I'll take care of it in stage 1.

Martin



Re: [PATCH] Fix pdftex miscompilation due to get_range_strlen (PR tree-optimization/84478, take 2 and 3)

2018-02-21 Thread Jakub Jelinek
On Tue, Feb 20, 2018 at 08:56:11PM -0700, Jeff Law wrote:
> On 02/20/2018 02:34 PM, Jakub Jelinek wrote:
> > On Tue, Feb 20, 2018 at 01:13:13PM -0700, Martin Sebor wrote:
> >> A safer and even more conservative alternative that should be
> >> equivalent to your approach while avoiding the sprintf regressions
> >> is to add another mode to the function and have it clear *minlen
> >> as an option.  This lets the strlen code obtain the conservative
> >> lower bound without compromising the sprintf warnings.
> > 
> > I fail to see what it would be good for to set *MINLEN to zero and
> > *MAXLEN to all ones for the non-warning use cases, we simply don't know
> > anything about it, both NULL_TREEs i.e. returning false is better.  I'm
> > offering two alternate patches which use
> > fuzzy == 0 for the previous !fuzzy, fuzzy == 1 for conservatively correct
> > code that assumes strlen can't cross field/variable boundaries in
> > compliant programs and fuzzy == 2 which does that + whatever the warning
> > code wants.  Additionally, I've rewritten the COND_EXPR handling, so that
> > it matches exactly the PHI handling.
> > 
> > The first patch doesn't change the 2 argument get_range_strlen and changes
> > gimple_fold_builtin_strlen to use the 6 argument one, the second patch
> > changes also the 2 argument get_range_strlen similarly to what you've done
> > in your patch.
> > 
> > Tested on x86_64-linux and i686-linux, ok for trunk if it passes
> > bootstrap/regtest?  Which one?
> I'd lean towards the second -- essentially trying to keep the 6 operand
> version for internal (recursive) use only.
> 
> In  my mind I'd like to encapsulate the 6 operand version so that it
> can't be directly called and the only public interface is the 3 operand
> version using strict/no-strict.
> 
> Let's go with that.  We can try to clean this up further during gcc-9.

Ok, committed the second patch after bootstrapping/regtesting it on
x86_64-linux and i686-linux.

There is one thing that might still be desirable to change, because right
now the default arg for the non-internal get_range_strlen is the warning
non-conservative one, so if somebody adds another use for code generation it
might break stuff again.  Perhaps we should either flip the default or make
the third argument a required argument and adjust all the 6 callers.

Jakub


Re: [PATCH] Add "native" as a valid option value for -march= on i386 (PR driver/83193).

2018-02-21 Thread Jakub Jelinek
On Wed, Feb 21, 2018 at 03:19:22PM +0100, Martin Liška wrote:
> On 02/21/2018 03:08 PM, Jakub Jelinek wrote:
> > This just adds "native" as possible value for -march, but shouldn't it be
> > also for -mtune, i.e. around line 4268?
> 
> Thanks for note. There's updated version.
> 
> Is it ok now?
> Thanks,
> Martin

> >From 980016be748b8f6a917f497d256c62a054bdece8 Mon Sep 17 00:00:00 2001
> From: marxin 
> Date: Tue, 20 Feb 2018 14:21:05 +0100
> Subject: [PATCH] Add "native" as a valid option value for -march= on i386 (PR
>  driver/83193).
> 
> gcc/ChangeLog:
> 
> 2018-02-20  Martin Liska  
> 
>   PR driver/83193
>   * config/i386/i386.c (ix86_option_override_internal):
>   Add "native" as a possible value for -march and -mtune.

Ok, thanks.

> ---
>  gcc/config/i386/i386.c | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index d54e7301e84..18d9084fd30 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -4193,6 +4193,11 @@ ix86_option_override_internal (bool main_args_p,
>   || ((processor_alias_table[i].flags & PTA_64BIT) != 0)))
> candidates.safe_push (processor_alias_table[i].name);
>  
> +#ifdef HAVE_LOCAL_CPU_DETECT
> +  /* Add also "native" as possible value.  */
> +  candidates.safe_push ("native");
> +#endif
> +
>char *s;
>const char *hint
>   = candidates_list_and_hint (opts->x_ix86_arch_string, s, candidates);
> @@ -4265,6 +4270,11 @@ ix86_option_override_internal (bool main_args_p,
>   || ((processor_alias_table[i].flags & PTA_64BIT) != 0))
> candidates.safe_push (processor_alias_table[i].name);
>  
> +#ifdef HAVE_LOCAL_CPU_DETECT
> +  /* Add also "native" as possible value.  */
> +  candidates.safe_push ("native");
> +#endif
> +
>char *s;
>const char *hint
>   = candidates_list_and_hint (opts->x_ix86_tune_string, s, candidates);
> -- 
> 2.16.1
> 


Jakub


Re: [PATCH] Add "native" as a valid option value for -march= on i386 (PR driver/83193).

2018-02-21 Thread Martin Liška
On 02/21/2018 03:08 PM, Jakub Jelinek wrote:
> This just adds "native" as possible value for -march, but shouldn't it be
> also for -mtune, i.e. around line 4268?

Thanks for note. There's updated version.

Is it ok now?
Thanks,
Martin
>From 980016be748b8f6a917f497d256c62a054bdece8 Mon Sep 17 00:00:00 2001
From: marxin 
Date: Tue, 20 Feb 2018 14:21:05 +0100
Subject: [PATCH] Add "native" as a valid option value for -march= on i386 (PR
 driver/83193).

gcc/ChangeLog:

2018-02-20  Martin Liska  

	PR driver/83193
	* config/i386/i386.c (ix86_option_override_internal):
	Add "native" as a possible value for -march and -mtune.
---
 gcc/config/i386/i386.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index d54e7301e84..18d9084fd30 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -4193,6 +4193,11 @@ ix86_option_override_internal (bool main_args_p,
 		|| ((processor_alias_table[i].flags & PTA_64BIT) != 0)))
 	  candidates.safe_push (processor_alias_table[i].name);
 
+#ifdef HAVE_LOCAL_CPU_DETECT
+  /* Add also "native" as possible value.  */
+  candidates.safe_push ("native");
+#endif
+
   char *s;
   const char *hint
 	= candidates_list_and_hint (opts->x_ix86_arch_string, s, candidates);
@@ -4265,6 +4270,11 @@ ix86_option_override_internal (bool main_args_p,
 	|| ((processor_alias_table[i].flags & PTA_64BIT) != 0))
 	  candidates.safe_push (processor_alias_table[i].name);
 
+#ifdef HAVE_LOCAL_CPU_DETECT
+  /* Add also "native" as possible value.  */
+  candidates.safe_push ("native");
+#endif
+
   char *s;
   const char *hint
 	= candidates_list_and_hint (opts->x_ix86_tune_string, s, candidates);
-- 
2.16.1



Re: Add "native" as a valid option value for -march= on arm (PR driver/83193).

2018-02-21 Thread Martin Liška
On 02/21/2018 10:23 AM, Kyrill Tkachov wrote:
> Have you tested this with a native and a cross-compiler like the aarch64 
> version?

Yes, you tested that ;) I'm going to install the patch.

Martin


Re: [PATCH PR82096][gcc-7, gcc-6] Backport: Fix ICE in int_mode_for_mode, at stor-layout.c:403 with arm-linux-gnueabi

2018-02-21 Thread Sudakshina Das

On 16/02/18 15:40, Sudakshina Das wrote:

On 22/01/18 15:23, Richard Biener wrote:

On Mon, Jan 22, 2018 at 4:10 PM, Sudakshina Das  wrote:

Hi

This is a patch to backport r256526 and r256941 (Fix case fix) of 
trunk to

fix emit_store_flag_force () function to fix the ICE. The original
discussion is at 
https://gcc.gnu.org/ml/gcc-patches/2018-01/msg00219.html

and https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01058.html

Is this ok for gcc-7-branch?
Testing : Ran regression testing with bootstrapped 
arm-none-linux-gnueabihf.


The branch is currently frozen so please wait until after the GCC 7.3 
release.


Committed as r257741


Backported to gcc-6 as r257871

Thanks
Sudi



Thanks
Sudi



Thanks,
Richard.


Thanks
Sudi

ChangeLog entries:

*** gcc/ChangeLog ***

2018-01-22  Sudakshina Das  

 Backport from mainline:
 2018-01-10  Sudakshina Das  

 PR target/82096
 * expmed.c (emit_store_flag_force): Swap if const op0
 and change VOIDmode to mode of op0.

*** gcc/testsuite/ChangeLog ***

2018-01-22  Sudakshina Das  

 Backport from mainline:
 2018-01-10  Sudakshina Das  

 PR target/82096
 * gcc.c-torture/compile/pr82096.c: New test.






Re: Add "native" as a valid option value for -march= on arm (PR driver/83193).

2018-02-21 Thread Kyrill Tkachov


On 21/02/18 09:53, Martin Liška wrote:

On 02/21/2018 10:23 AM, Kyrill Tkachov wrote:

Have you tested this with a native and a cross-compiler like the aarch64 
version?

No, I don't have a machine. Can you please do that?


Sure, I've bootstrapped and tested it on arm-none-linux-gnueabihf
and an arm-none-eabi cross-compiler. Confirmed that "native" comes up
as a suggestion for a native compiler and not for a cross-compiler.
So the approach looks good. Please address the comment in my previous reply
and this will be ok for trunk.

Thanks,
Kyrill


Thanks,
Martin




Re: [SFN+LVU+IEPM v4 9/9] [IEPM] Introduce inline entry point markers

2018-02-21 Thread Uros Bizjak
On Wed, Feb 21, 2018 at 11:11 AM, Alexandre Oliva  wrote:
> On Feb 15, 2018, Szabolcs Nagy  wrote:
>
>> i see assembler slow downs with these location view patches
>> i opened https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84408
>
>
> [LVU] reset view at function entry, omit views at line zero
>
> Location views might be associated with locations that lack line
> number information (line number zero), but since we omit .loc
> directives that would have been issued with line number zero, we also
> omit the symbolic view numbers that would have been issued at such
> points.
>
> Resetting views at function entry points address some of these issues,
> and alleviate the huge chains of symbolic views that have burdened
> assemblers since we disabled -ginternal-reset-location-views by
> default, but other problems of undefined views remain when it's not
> the whole function that lacks line number info, just parts of it.
>
> So, when we encounter a request to output a view that may have been
> referenced, but we decide to omit the .loc because the line is zero,
> we will now omit the view as well, i.e., we will internally regard
> that view as zero-numbered.
>
> Regstrapped on x86_64-linux-gnu and i686-linux-gnu.  Ok to install?
>
> Uros, could you please confirm whether this fixes the 84404 go problem
> you reported on alpha?  I'm guessing it's the same issue.  TIA,

I can confirm, that all "leb128 operand is an undefined symbol" errors
are gone. I'm running full alpha native bootstrap & regression test,
but I don't expect any surprises here.

Thanks,
Uros.

> for  gcc/ChangeLog
>
> PR debug/84404
> PR debug/84408
> * dwarf2out.c (struct dw_line_info_table): Update comments for
> view == -1.
> (FORCE_RESET_NEXT_VIEW): New.
> (FORCE_RESETTING_VIEW_P): New.
> (RESETTING_VIEW_P): Check for -1 too.
> (ZERO_VIEW_P): Likewise.
> (new_line_info_table): Force-reset next view.
> (dwarf2out_begin_function): Likewise.
> (dwarf2out_source_line): Simplify zero_view_p initialization.
> Test FORCE_RESETTING_VIEW_P and RESETTING_VIEW_P instead of
> view directly.  Omit view when omitting .loc at line 0.
>
> for  gcc/testsuite/ChangeLog
>
> PR debug/84404
> PR debug/84408
> * gcc.dg/graphite/pr84404.c: New.
> ---
>  gcc/dwarf2out.c |   89 
> ---
>  gcc/testsuite/gcc.dg/graphite/pr84404.c |   18 ++
>  2 files changed, 87 insertions(+), 20 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/graphite/pr84404.c
>
> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
> index 5e88c7bacf06..7bbe20e495ac 100644
> --- a/gcc/dwarf2out.c
> +++ b/gcc/dwarf2out.c
> @@ -2940,8 +2940,8 @@ struct GTY(()) dw_line_info_table {
>   If it is 0, it is known that the NEXT view will be the first view
>   at the given PC.
>
> - If it is -1, we've advanced PC but we haven't emitted a line location 
> yet,
> - so we shouldn't use this view number.
> + If it is -1, we're forcing the view number to be reset, e.g. at a
> + function entry.
>
>   The meaning of other nonzero values depends on whether we're
>   computing views internally or leaving it for the assembler to do
> @@ -2951,8 +2951,10 @@ struct GTY(()) dw_line_info_table {
>   going to ask the assembler to assign.  */
>var_loc_view view;
>
> +#define FORCE_RESET_NEXT_VIEW(x) ((x) = (var_loc_view)-1)
>  #define RESET_NEXT_VIEW(x) ((x) = (var_loc_view)0)
> -#define RESETTING_VIEW_P(x) ((x) == (var_loc_view)0)
> +#define FORCE_RESETTING_VIEW_P(x) ((x) == (var_loc_view)-1)
> +#define RESETTING_VIEW_P(x) ((x) == (var_loc_view)0 || 
> FORCE_RESETTING_VIEW_P (x))
>
>vec *entries;
>  };
> @@ -2985,7 +2987,7 @@ maybe_reset_location_view (rtx_insn *insn, 
> dw_line_info_table *table)
>else if (get_attr_min_length (insn) > 0)
>  reset = 1;
>
> -  if (reset > 0)
> +  if (reset > 0 && !RESETTING_VIEW_P (table->view))
>  RESET_NEXT_VIEW (table->view);
>  }
>
> @@ -3235,9 +3237,10 @@ static GTY(()) bitmap zero_view_p;
> that must be view number zero.  Otherwise, ZERO_VIEW_P is allocated
> and views label numbers recorded in it are the ones known to be
> zero.  */
> -#define ZERO_VIEW_P(N) (zero_view_p\
> -   ? bitmap_bit_p (zero_view_p, (N))   \
> -   : (N) == 0)
> +#define ZERO_VIEW_P(N) ((N) == (var_loc_view)0 \
> +   || (N) == (var_loc_view)-1  \
> +   || (zero_view_p \
> +   && bitmap_bit_p (zero_view_p, (N
>
>  /* Return true iff we're to emit .loc directives for the assembler to
> generate line number sections.
> @@ -27210,6 +27213,18 @@ create_label:
>   

PING: [PATCH] Use dlsym to check if libdl is needed for plugin

2018-02-21 Thread H.J. Lu
On Wed, Oct 18, 2017 at 5:25 PM, H.J. Lu  wrote:
> config/plugins.m4 has
>
>  if test "$plugins" = "yes"; then
> AC_SEARCH_LIBS([dlopen], [dl])
>   fi
>
> Plugin uses dlsym, but libasan.so only intercepts dlopen, not dlsym:
>
> [hjl@gnu-tools-1 binutils-text]$ nm -D /lib64/libasan.so.4| grep " dl"
> 00038580 W dlclose
>  U dl_iterate_phdr
> 0004dc50 W dlopen
>  U dlsym
>  U dlvsym
> [hjl@gnu-tools-1 binutils-text]$
>
> Testing dlopen for libdl leads to false negative when -fsanitize=address
> is used.  It results in link failure:
>
> ../bfd/.libs/libbfd.a(plugin.o): undefined reference to symbol 
> 'dlsym@@GLIBC_2.16'
>
> dlsym should be used to check if libdl is needed for plugin.
>
> OK for master?
>
> H.J.
> ---
> bfd/
>
> PR gas/22318
> * configure: Regenerated.
>
> binutils/
>
> PR gas/22318
> * configure: Regenerated.
>
> config/
>
> * plugins.m4 (AC_PLUGINS): Use dlsym to check if libdl is needed.
>
> gas/
>
> PR gas/22318
> * configure: Regenerated.
>
> gprof/
>
> PR gas/22318
> * configure: Regenerated.
>
> ld/
>
> PR gas/22318
> * configure: Regenerated.
> ---
>  bfd/configure  | 24 
>  binutils/configure | 24 
>  config/plugins.m4  |  2 +-
>  gas/configure  | 24 
>  gprof/configure| 24 
>  ld/configure   | 24 
>  6 files changed, 61 insertions(+), 61 deletions(-)
>
> diff --git a/bfd/configure b/bfd/configure
> index 32ee062e80..f019a61fb0 100755
> --- a/bfd/configure
> +++ b/bfd/configure
> @@ -11826,9 +11826,9 @@ else
>  fi
>
>if test "$plugins" = "yes"; then
> -{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for library containing 
> dlopen" >&5
> -$as_echo_n "checking for library containing dlopen... " >&6; }
> -if test "${ac_cv_search_dlopen+set}" = set; then :
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for library containing 
> dlsym" >&5
> +$as_echo_n "checking for library containing dlsym... " >&6; }
> +if test "${ac_cv_search_dlsym+set}" = set; then :
>$as_echo_n "(cached) " >&6
>  else
>ac_func_search_save_LIBS=$LIBS
> @@ -11841,11 +11841,11 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
>  #ifdef __cplusplus
>  extern "C"
>  #endif
> -char dlopen ();
> +char dlsym ();
>  int
>  main ()
>  {
> -return dlopen ();
> +return dlsym ();
>;
>return 0;
>  }
> @@ -11858,25 +11858,25 @@ for ac_lib in '' dl; do
>  LIBS="-l$ac_lib  $ac_func_search_save_LIBS"
>fi
>if ac_fn_c_try_link "$LINENO"; then :
> -  ac_cv_search_dlopen=$ac_res
> +  ac_cv_search_dlsym=$ac_res
>  fi
>  rm -f core conftest.err conftest.$ac_objext \
>  conftest$ac_exeext
> -  if test "${ac_cv_search_dlopen+set}" = set; then :
> +  if test "${ac_cv_search_dlsym+set}" = set; then :
>break
>  fi
>  done
> -if test "${ac_cv_search_dlopen+set}" = set; then :
> +if test "${ac_cv_search_dlsym+set}" = set; then :
>
>  else
> -  ac_cv_search_dlopen=no
> +  ac_cv_search_dlsym=no
>  fi
>  rm conftest.$ac_ext
>  LIBS=$ac_func_search_save_LIBS
>  fi
> -{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_search_dlopen" >&5
> -$as_echo "$ac_cv_search_dlopen" >&6; }
> -ac_res=$ac_cv_search_dlopen
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_search_dlsym" >&5
> +$as_echo "$ac_cv_search_dlsym" >&6; }
> +ac_res=$ac_cv_search_dlsym
>  if test "$ac_res" != no; then :
>test "$ac_res" = "none required" || LIBS="$ac_res $LIBS"
>
> diff --git a/binutils/configure b/binutils/configure
> index 22e1b1736e..9691ec23f0 100755
> --- a/binutils/configure
> +++ b/binutils/configure
> @@ -11622,9 +11622,9 @@ else
>  fi
>
>if test "$plugins" = "yes"; then
> -{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for library containing 
> dlopen" >&5
> -$as_echo_n "checking for library containing dlopen... " >&6; }
> -if test "${ac_cv_search_dlopen+set}" = set; then :
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for library containing 
> dlsym" >&5
> +$as_echo_n "checking for library containing dlsym... " >&6; }
> +if test "${ac_cv_search_dlsym+set}" = set; then :
>$as_echo_n "(cached) " >&6
>  else
>ac_func_search_save_LIBS=$LIBS
> @@ -11637,11 +11637,11 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
>  #ifdef __cplusplus
>  extern "C"
>  #endif
> -char dlopen ();
> +char dlsym ();
>  int
>  main ()
>  {
> -return dlopen ();
> +return dlsym ();
>;
>return 0;
>  }
> @@ -11654,25 +11654,25 @@ for ac_lib in '' dl; do
>  LIBS="-l$ac_lib  $ac_func_search_save_LIBS"
>fi
>if ac_fn_c_try_link "$LINENO"; then :
> -  ac_cv_search_dlopen=$ac_res
> +  ac_cv_search_dlsym=$ac_res
>  fi
>  rm -f core conftest.err conftest.$ac_objext \
>  conftest$ac_exeext
> -  if test "${ac_cv_search_dlopen+set}" = set; then :
> +  if test 

C++ PATCH to fix ICE with invalid cast (PR c++/84493)

2018-02-21 Thread Marek Polacek
Here we can prevent a crash on invalid by using require_open where '{' is
expected.  require_open uses cp_parser_require whereas consume_open has
an assert:
gcc_assert (tok->type == traits_t::open_token_type);
which triggers here.

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

2018-02-21  Marek Polacek  

PR c++/84493
* parser.c (cp_parser_braced_list): Use require_open instead of
consume_open.

* g++.dg/parse/error59.C: New test.

diff --git gcc/cp/parser.c gcc/cp/parser.c
index 2bb0d2da5fe..4fa546a086c 100644
--- gcc/cp/parser.c
+++ gcc/cp/parser.c
@@ -21925,7 +21925,7 @@ cp_parser_braced_list (cp_parser* parser, bool* 
non_constant_p)
 
   /* Consume the `{' token.  */
   matching_braces braces;
-  braces.consume_open (parser);
+  braces.require_open (parser);
   /* Create a CONSTRUCTOR to represent the braced-initializer.  */
   initializer = make_node (CONSTRUCTOR);
   /* If it's not a `}', then there is a non-trivial initializer.  */
diff --git gcc/testsuite/g++.dg/parse/error59.C 
gcc/testsuite/g++.dg/parse/error59.C
index e69de29bb2d..2c44e210366 100644
--- gcc/testsuite/g++.dg/parse/error59.C
+++ gcc/testsuite/g++.dg/parse/error59.C
@@ -0,0 +1,6 @@
+// PR c++/84493
+
+void foo()
+{
+  (struct {}x){}; // { dg-error "" }
+}

Marek


Re: [SFN+LVU+IEPM v4 9/9] [IEPM] Introduce inline entry point markers

2018-02-21 Thread Alexandre Oliva
On Jan 24, 2018, Jakub Jelinek  wrote:

>> --- a/gcc/tree-ssa-live.c
>> +++ b/gcc/tree-ssa-live.c
>> @@ -520,6 +520,11 @@ remove_unused_scope_block_p (tree scope, bool 
>> in_ctor_dtor_block)
>> else if (!BLOCK_SUPERCONTEXT (scope)
>> || TREE_CODE (BLOCK_SUPERCONTEXT (scope)) == FUNCTION_DECL)
>> unused = false;
>> +   /* Preserve the block, it is referenced by at least the inline
>> +  entry point marker.  */
>> +   else if (debug_nonbind_markers_p
>> +&& inlined_function_outer_scope_p (scope))
>> + unused = false;
>> /* Innermost blocks with no live variables nor statements can be always
>> eliminated.  */
>> else if (!nsubblocks)
>> @@ -548,11 +553,13 @@ remove_unused_scope_block_p (tree scope, bool 
>> in_ctor_dtor_block)
>> }
>> else if (BLOCK_VARS (scope) || BLOCK_NUM_NONLOCALIZED_VARS (scope))
>> unused = false;
>> -   /* See if this block is important for representation of inlined function.
>> -  Inlined functions are always represented by block with
>> -  block_ultimate_origin being set to FUNCTION_DECL and 
>> DECL_SOURCE_LOCATION
>> -  set...  */
>> -   else if (inlined_function_outer_scope_p (scope))
>> +   /* See if this block is important for representation of inlined
>> +  function.  Inlined functions are always represented by block
>> +  with block_ultimate_origin being set to FUNCTION_DECL and
>> +  DECL_SOURCE_LOCATION set, unless they expand to nothing...  But
>> +  see above for the case of statement frontiers.  */
>> +   else if (!debug_nonbind_markers_p
>> +&& inlined_function_outer_scope_p (scope))
>> unused = false;

> Wonder what the above hunks will do for LTO memory consumption.  We'll see.


[IEPM] don't preserve lexical blocks just for debug inline markers

This patch stops preserving scope blocks just because they are inlined
function scopes, when cleaning up unused scope blocks.  This change
was introduced along with IEPM, but it preserved lots of blocks, and
output debug information for them, although no code from the inlined
function remained after optimization.

The additional preserved blocks took up compile-time memory, and
significant disk space and link time, in some cases more than 25%.
This is deemed excessive, compared with the reasonably small benefit
of allowing one to single-step into an inlined function using a
view-capable debugger.

There was another way of marking inlined function scopes as unused,
based on the markers referencing them during stmt scanning, but that
still preserved too much.

So, this patch restores the pre-IEPM logic of preservation of scopes.
Should a scope block referenced by an inline entry marker be found to
be unused in remove_unused_scope_block_p, the marker will be cleaned
up right after that, in clear_unused_block_pointer, so we won't keep
a dangling reference to a dropped block.

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

for  gcc/ChangeLog

* tree-ssa-live.c (remove_unused_scope_block_p): Do not
preserve inline entry blocks for the sake of debug inline
entry point markers alone.
(remove_unused_locals): Suggest in comments a better place to
force the preservation of inline entry blocks that are
otherwise unused, but do not preserve them.
---
 gcc/tree-ssa-live.c |   15 ++-
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/gcc/tree-ssa-live.c b/gcc/tree-ssa-live.c
index 62bb3c5de659..62316bac7929 100644
--- a/gcc/tree-ssa-live.c
+++ b/gcc/tree-ssa-live.c
@@ -520,11 +520,6 @@ remove_unused_scope_block_p (tree scope, bool 
in_ctor_dtor_block)
else if (!BLOCK_SUPERCONTEXT (scope)
 || TREE_CODE (BLOCK_SUPERCONTEXT (scope)) == FUNCTION_DECL)
  unused = false;
-   /* Preserve the block, it is referenced by at least the inline
-  entry point marker.  */
-   else if (debug_inline_points
-   && inlined_function_outer_scope_p (scope))
- unused = false;
/* Innermost blocks with no live variables nor statements can be always
   eliminated.  */
else if (!nsubblocks)
@@ -556,10 +551,8 @@ remove_unused_scope_block_p (tree scope, bool 
in_ctor_dtor_block)
/* See if this block is important for representation of inlined
   function.  Inlined functions are always represented by block
   with block_ultimate_origin being set to FUNCTION_DECL and
-  DECL_SOURCE_LOCATION set, unless they expand to nothing...  But
-  see above for the case of statement frontiers.  */
-   else if (!debug_inline_points
-   && inlined_function_outer_scope_p (scope))
+  DECL_SOURCE_LOCATION set, unless they expand to nothing...  */
+   else if (inlined_function_outer_scope_p (scope))
  unused = false;
else
/* Verfify that only blocks with source location set
@@ -741,6 +734,10 @@ remove_unused_locals (void)
  gimple *stmt = gsi_stmt (gsi);
  tree b = gimple_block (stmt);
 
+ /* If we wanted to mark the 

Re: [SFN+LVU+IEPM v4 9/9] [IEPM] Introduce inline entry point markers

2018-02-21 Thread Alexandre Oliva
On Feb 15, 2018, Szabolcs Nagy  wrote:

> i see assembler slow downs with these location view patches
> i opened https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84408


[LVU] reset view at function entry, omit views at line zero

Location views might be associated with locations that lack line
number information (line number zero), but since we omit .loc
directives that would have been issued with line number zero, we also
omit the symbolic view numbers that would have been issued at such
points.

Resetting views at function entry points address some of these issues,
and alleviate the huge chains of symbolic views that have burdened
assemblers since we disabled -ginternal-reset-location-views by
default, but other problems of undefined views remain when it's not
the whole function that lacks line number info, just parts of it.

So, when we encounter a request to output a view that may have been
referenced, but we decide to omit the .loc because the line is zero,
we will now omit the view as well, i.e., we will internally regard
that view as zero-numbered.

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

Uros, could you please confirm whether this fixes the 84404 go problem
you reported on alpha?  I'm guessing it's the same issue.  TIA,

for  gcc/ChangeLog

PR debug/84404
PR debug/84408
* dwarf2out.c (struct dw_line_info_table): Update comments for
view == -1.
(FORCE_RESET_NEXT_VIEW): New.
(FORCE_RESETTING_VIEW_P): New.
(RESETTING_VIEW_P): Check for -1 too.
(ZERO_VIEW_P): Likewise.
(new_line_info_table): Force-reset next view.
(dwarf2out_begin_function): Likewise.
(dwarf2out_source_line): Simplify zero_view_p initialization.
Test FORCE_RESETTING_VIEW_P and RESETTING_VIEW_P instead of
view directly.  Omit view when omitting .loc at line 0.

for  gcc/testsuite/ChangeLog

PR debug/84404
PR debug/84408
* gcc.dg/graphite/pr84404.c: New.
---
 gcc/dwarf2out.c |   89 ---
 gcc/testsuite/gcc.dg/graphite/pr84404.c |   18 ++
 2 files changed, 87 insertions(+), 20 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/graphite/pr84404.c

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 5e88c7bacf06..7bbe20e495ac 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -2940,8 +2940,8 @@ struct GTY(()) dw_line_info_table {
  If it is 0, it is known that the NEXT view will be the first view
  at the given PC.
 
- If it is -1, we've advanced PC but we haven't emitted a line location yet,
- so we shouldn't use this view number.
+ If it is -1, we're forcing the view number to be reset, e.g. at a
+ function entry.
 
  The meaning of other nonzero values depends on whether we're
  computing views internally or leaving it for the assembler to do
@@ -2951,8 +2951,10 @@ struct GTY(()) dw_line_info_table {
  going to ask the assembler to assign.  */
   var_loc_view view;
 
+#define FORCE_RESET_NEXT_VIEW(x) ((x) = (var_loc_view)-1)
 #define RESET_NEXT_VIEW(x) ((x) = (var_loc_view)0)
-#define RESETTING_VIEW_P(x) ((x) == (var_loc_view)0)
+#define FORCE_RESETTING_VIEW_P(x) ((x) == (var_loc_view)-1)
+#define RESETTING_VIEW_P(x) ((x) == (var_loc_view)0 || FORCE_RESETTING_VIEW_P 
(x))
 
   vec *entries;
 };
@@ -2985,7 +2987,7 @@ maybe_reset_location_view (rtx_insn *insn, 
dw_line_info_table *table)
   else if (get_attr_min_length (insn) > 0)
 reset = 1;
 
-  if (reset > 0)
+  if (reset > 0 && !RESETTING_VIEW_P (table->view))
 RESET_NEXT_VIEW (table->view);
 }
 
@@ -3235,9 +3237,10 @@ static GTY(()) bitmap zero_view_p;
that must be view number zero.  Otherwise, ZERO_VIEW_P is allocated
and views label numbers recorded in it are the ones known to be
zero.  */
-#define ZERO_VIEW_P(N) (zero_view_p\
-   ? bitmap_bit_p (zero_view_p, (N))   \
-   : (N) == 0)
+#define ZERO_VIEW_P(N) ((N) == (var_loc_view)0 \
+   || (N) == (var_loc_view)-1  \
+   || (zero_view_p \
+   && bitmap_bit_p (zero_view_p, (N
 
 /* Return true iff we're to emit .loc directives for the assembler to
generate line number sections.
@@ -27210,6 +27213,18 @@ create_label:
  last_postcall_label = ggc_strdup (loclabel);
}
   newloc->label = last_postcall_label;
+  /* ??? This view is at last_label, not last_label-1, but we
+could only assume view at last_label-1 is zero if we could
+assume calls always have length greater than one.  This is
+probably true in general, though there might be a rare
+exception to this rule, e.g. if a call insn is optimized out
+by target magic.  Then, even the -1 in 

Re: Add "native" as a valid option value for -march= on arm (PR driver/83193).

2018-02-21 Thread Martin Liška
On 02/21/2018 10:23 AM, Kyrill Tkachov wrote:
> Have you tested this with a native and a cross-compiler like the aarch64 
> version?

No, I don't have a machine. Can you please do that?

Thanks,
Martin


Re: Add "native" as a valid option value for -march= on arm (PR driver/83193).

2018-02-21 Thread Kyrill Tkachov

Hi Martin,

On 21/02/18 07:34, Martin Liška wrote:

Hi.

This is equivalent patch for ARM target.


Thanks for fixing this!


Ready for trunk?


Have you tested this with a native and a cross-compiler like the aarch64 
version?


Thanks,
Martin


From 656a883bc5239439ba80743f15a8df704501ee71 Mon Sep 17 00:00:00 2001
From: marxin
Date: Tue, 20 Feb 2018 14:09:22 +0100
Subject: [PATCH 1/3] Add "native" as a valid option value for -march= on arm
 (PR driver/83193).

gcc/ChangeLog:

2018-02-20  Martin Liska

PR driver/83193
* common/config/arm/arm-common.c (arm_print_hint_for_arch_option):
Add "native" as a possible value.
* config/arm/arm.h (HAVE_LOCAL_CPU_DETECT): Define the macro
when native cpu detection is available.
---
 gcc/common/config/arm/arm-common.c | 6 ++
 gcc/config/arm/arm.h   | 1 +
 2 files changed, 7 insertions(+)

diff --git a/gcc/common/config/arm/arm-common.c 
b/gcc/common/config/arm/arm-common.c
index fc585e0b0ee..50f0bad3e36 100644
--- a/gcc/common/config/arm/arm-common.c
+++ b/gcc/common/config/arm/arm-common.c
@@ -353,6 +353,12 @@ arm_print_hint_for_arch_option (const char *target,
   auto_vec candidates;
   for (; list->common.name != NULL; list++)
 candidates.safe_push (list->common.name);
+
+#ifdef HAVE_LOCAL_CPU_DETECT
+  /* Add also "native" as possible value.  */
+  candidates.safe_push ("native");
+#endif
+


On arm we also support "native" as a value for -mcpu and -mtune. These are both 
handled by
the arm_print_hint_for_cpu_option function in the same file. Can you please add 
this snippet
to them as well?

Thanks,
Kyrill



Re: [PATCH] Fix pdftex miscompilation due to get_range_strlen (PR tree-optimization/84478)

2018-02-21 Thread Jakub Jelinek
On Tue, Feb 20, 2018 at 08:31:06PM -0700, Jeff Law wrote:
> On 02/20/2018 05:14 PM, Jakub Jelinek wrote:
> > On Tue, Feb 20, 2018 at 04:59:12PM -0700, Martin Sebor wrote:
> >>> It would help if you explained why you think it is a good idea
> >>> ignoring the other phi arguments if you have one (or more) where you can
> >>> determine length.
> >>
> >> It's a heuristic that was meant just for the -Wformat-overflow
> >> warning.  When making decisions that affect code generation it's
> >> obviously not correct to ignore the possibility that unknown
> >> arguments may be shorter than the minimum or longer than
> >> the maximum.  The fuzzy argument was meant to differentiate
> >> between two got but I forgot about it when I added the fix
> >> for PR 83671.
> > 
> > get_range_strlen (the 2 argument one) is right now called:
> > 3 times from builtins.c for -Wstringop-overflow
> > once from gimple-ssa-sprintf.c for -Wformat-overflow
> > once from tree-ssa-strlen.c for -Wstringop-truncation
> > once from gimple-fold.c for gimple_fold_builtin_strlen value ranges
> Presumably it's the last one that's causing problems and were we should
> focus our effort.

Sure.  And that is what my patches do, not change anything for the
warning cases and fix up the non-warning one.

> > Looking at strlenopt-40.c testcase, in the test you clearly rely on
> > fuzzy argument being set for the value ranges case
> > (gimple_fold_builtin_strlen), otherwise many of the
> > subtests would fail.
> Which tests specifically?  I did a real quick scan and didn't see
> anything in there that depended on incorrect behavior for the call from
> gimple_fold_builtin_strlen.  BUt it was a quick scan and I could have
> missed something.

elim_global_arrays, elim_pointer_to_arrays etc.  Basically, calling strlen
on an array of known fixed length and checking that the upper bound of it is
at most the array size minus 1.

Jakub