[PATCH] correct COUNT and PROB for unrolled loop

[Jiufu Guo]

Hi,
PR68212 mentioned that the COUNT of unrolled loop was not correct, and
comments of this PR also mentioned that loop become 'cold'.  The patches
of the PR fixed part of the issue.  With reference the patch
(https://gcc.gnu.org/ml/gcc-patches/2018-11/msg02368.html) and comment
(https://gcc.gnu.org/ml/gcc-patches/2018-11/msg02380.html), below patch
is drafted to fix other part of this issue.

The following patch fixes the wrong COUNT/PROB of unrolled loop.  And the
patch handles the case where unrolling in unreliable count number can
cause a loop to no longer look hot and therefor not get aligned.  This
patch corrects the PROB of loop exit edge, and corrects RPOB/COUNT of
latch block, and the loop count after last peeling.  This patch scale by
profile_probability::likely () if unrolled count gets unrealistically small.

Bootstrap/regtest on powerpc64le with no new regressions.
And spec2017 result is fine: a couple INT benchmarks that showed around
1.7% improvement, everything else was +/- <= 1%.

Ok for trunk?

Jiufu Guo

2020-02-03  Jiufu Guo   
Pat Haugen  

PR rtl-optimization/68212
* cfgloopmanip.c (duplicate_loop_to_header_edge): Correct COUNT/PROB
for unrolled/peeled blocks.

testsuite/ChangeLog:
2020-02-03  Jiufu Guo   
Pat Haugen  
PR rtl-optimization/68212
* gcc.dg/pr68212.c: New test.


---
 gcc/cfgloopmanip.c | 53 --
 gcc/testsuite/gcc.dg/pr68212.c | 13 +++
 2 files changed, 64 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr68212.c

diff --git a/gcc/cfgloopmanip.c b/gcc/cfgloopmanip.c
index 727e951..ded0046 100644
--- a/gcc/cfgloopmanip.c
+++ b/gcc/cfgloopmanip.c
@@ -31,6 +31,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "gimplify-me.h"
 #include "tree-ssa-loop-manip.h"
 #include "dumpfile.h"
+#include "cfgrtl.h"
 
 static void copy_loops_to (class loop **, int,
   class loop *);
@@ -1258,14 +1259,30 @@ duplicate_loop_to_header_edge (class loop *loop, edge e,
  /* If original loop is executed COUNT_IN times, the unrolled
 loop will account SCALE_MAIN_DEN times.  */
  scale_main = count_in.probability_in (scale_main_den);
+
+ /* If we are guessing at the number of iterations and count_in
+becomes unrealistically small, reset probability.  */
+ if (!(count_in.reliable_p () || loop->any_estimate))
+   {
+ profile_count new_count_in = count_in.apply_probability 
(scale_main);
+ profile_count preheader_count = loop_preheader_edge (loop)->count 
();
+ if (new_count_in.apply_scale (1, 10) < preheader_count)
+   scale_main = profile_probability::likely ();
+   }
+
  scale_act = scale_main * prob_pass_main;
}
   else
{
+ profile_count new_loop_count;
  profile_count preheader_count = e->count ();
- for (i = 0; i < ndupl; i++)
-   scale_main = scale_main * scale_step[i];
  scale_act = preheader_count.probability_in (count_in);
+ /* Compute final preheader count after peeling NDUPL copies.  */
+ for (i = 0; i < ndupl; i++)
+   preheader_count = preheader_count.apply_probability (scale_step[i]);
+ /* Subtract out exit(s) from peeled copies.  */
+ new_loop_count = count_in - (e->count () - preheader_count);
+ scale_main = new_loop_count.probability_in (count_in);
}
 }
 
@@ -1381,6 +1398,38 @@ duplicate_loop_to_header_edge (class loop *loop, edge e,
  scale_bbs_frequencies (new_bbs, n, scale_act);
  scale_act = scale_act * scale_step[j];
}
+
+  /* Need to update PROB of exit edge and corresponding COUNT.  */
+  if (orig && is_latch && (!bitmap_bit_p (wont_exit, j + 1))
+ && bbs_to_scale)
+   {
+ edge new_exit = new_spec_edges[SE_ORIG];
+ profile_count new_count_in = new_exit->src->count;
+ profile_count preheader_count = loop_preheader_edge (loop)->count ();
+ edge e;
+ edge_iterator ei;
+
+ FOR_EACH_EDGE (e, ei, new_exit->src->succs)
+   if (e != new_exit)
+ break;
+
+ gcc_assert (e && e != new_exit);
+
+ new_exit->probability = preheader_count.probability_in (new_count_in);
+ e->probability = new_exit->probability.invert ();
+
+ profile_count new_latch_count
+   = new_exit->src->count.apply_probability (e->probability);
+ profile_count old_latch_count = e->dest->count;
+
+ EXECUTE_IF_SET_IN_BITMAP (bbs_to_scale, 0, i, bi)
+   scale_bbs_frequencies_profile_count (new_bbs + i, 1,
+new_latch_count,
+old_latch_count);
+
+ if (current_ir_type () != IR_GIMPLE)
+   update_br_prob_note (e->src);
+ 

Re: libgo patch committed: Update to Go1.14beta1

[David Abdurachmanov]

On Sat, Feb 1, 2020 at 2:38 PM Andreas Schwab  wrote:

> ../../../libgo/go/syscall/syscall_linux_riscv64.go:7:14: error: imported
> and not used: unsafe
> 7 | import "unsafe"
>   |  ^


I see the same issue in Fedora/RISCV, the last two builds of GCC 10 failed
due to it.

Btw, 1.14 Beta 2 should ship riscv64 support (marked as experimental).
See: https://go-review.googlesource.com/c/go/+/216757/

david

Re: *ping**2 Re: [Patch][Fortran] Fix to strict associate check (PR93427)

[Tobias Burnus]

Another slightly early ping.

Tobias

On 1/31/20 3:24 PM, Tobias Burnus wrote:

*ping* after 4 days.

On 1/27/20 2:49 PM, Tobias Burnus wrote:
Semantically, there is an issue when the function name is used both 
for recursively calling and as result variable. Hence, one should 
only use one own's function name – in context of function calls – if 
one has a separate result variable.


This somehow got messed up with  r10-5722-g4d12437 (3 Jan 2020, 
PR92994) – rejecting also the use of the function name as result 
variable.


Fixed by removing the check. At least the most straight-forward 
invalid use is still rejected as shown by the augmented test case.


OK for the trunk?

Tobias


On 1/25/20 6:37 AM, Andrew Benson wrote:

I opened PR 93427 for the issue below:

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

The following code fails to compile (using git commit
472dc648ce3e7661762931d584d239611ddca964):

module a

type :: t
end type t

contains

recursive function b()
   class(t), pointer :: b
   type(t) :: c
   allocate(t :: b)
   select type (b)
   type is (t)
  b=c
   end select
end function b

end module a



$ gfortran -v
Using built-in specs.
COLLECT_GCC=gfortran
COLLECT_LTO_WRAPPER=/home/abenson/Galacticus/Tools/libexec/gcc/x86_64-pc-linux-gnu/10.0.1/lto-wrapper 


Target: x86_64-pc-linux-gnu
Configured with: ../gcc-git/configure
--prefix=/home/abenson/Galacticus/Tools
--enable-languages=c,c++,fortran
  --disable-multilib
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 10.0.1 20200124 (experimental) (GCC)


$ gfortran -c p.F90 -o p.o
p.F90:12:15:

    12 |   select type (b)
   |   1
Error: Associating entity 'b' at (1) is a procedure name
p.F90:14:5:

    14 |  b=c
   | 1
Error: 'b' at (1) associated to vector-indexed target cannot be used
in a variable definition context (assignment)


The code compiles successfully using ifort 18.0.1. Removing the
"recursive" attribute, or specifying a "result()" variable makes the
errors go away.


--

* Andrew Benson: 
http://users.obs.carnegiescience.edu/abenson/contact.html


* Galacticus: https://github.com/galacticusorg/galacticus

Re: *ping**2 Re: [Patch][Fortran] Fix to strict associate check (PR93427)

[Paul Richard Thomas]

Hi Tobias,

Yes, OK for trunk.

I would have been perfectly happy for you to commit as 'obvious'.

Cheers

Paul

On Mon, 3 Feb 2020 at 08:44, Tobias Burnus  wrote:
>
> Another slightly early ping.
>
> Tobias
>
> On 1/31/20 3:24 PM, Tobias Burnus wrote:
> > *ping* after 4 days.
> >
> > On 1/27/20 2:49 PM, Tobias Burnus wrote:
> >> Semantically, there is an issue when the function name is used both
> >> for recursively calling and as result variable. Hence, one should
> >> only use one own's function name – in context of function calls – if
> >> one has a separate result variable.
> >>
> >> This somehow got messed up with  r10-5722-g4d12437 (3 Jan 2020,
> >> PR92994) – rejecting also the use of the function name as result
> >> variable.
> >>
> >> Fixed by removing the check. At least the most straight-forward
> >> invalid use is still rejected as shown by the augmented test case.
> >>
> >> OK for the trunk?
> >>
> >> Tobias
> >>
> >>
> >> On 1/25/20 6:37 AM, Andrew Benson wrote:
> >>> I opened PR 93427 for the issue below:
> >>>
> >>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93427
> >>>
> >>> The following code fails to compile (using git commit
> >>> 472dc648ce3e7661762931d584d239611ddca964):
> >>>
> >>> module a
> >>>
> >>> type :: t
> >>> end type t
> >>>
> >>> contains
> >>>
> >>> recursive function b()
> >>>class(t), pointer :: b
> >>>type(t) :: c
> >>>allocate(t :: b)
> >>>select type (b)
> >>>type is (t)
> >>>   b=c
> >>>end select
> >>> end function b
> >>>
> >>> end module a
> >>>
> >>>
> >>>
> >>> $ gfortran -v
> >>> Using built-in specs.
> >>> COLLECT_GCC=gfortran
> >>> COLLECT_LTO_WRAPPER=/home/abenson/Galacticus/Tools/libexec/gcc/x86_64-pc-linux-gnu/10.0.1/lto-wrapper
> >>>
> >>> Target: x86_64-pc-linux-gnu
> >>> Configured with: ../gcc-git/configure
> >>> --prefix=/home/abenson/Galacticus/Tools
> >>> --enable-languages=c,c++,fortran
> >>>   --disable-multilib
> >>> Thread model: posix
> >>> Supported LTO compression algorithms: zlib
> >>> gcc version 10.0.1 20200124 (experimental) (GCC)
> >>>
> >>>
> >>> $ gfortran -c p.F90 -o p.o
> >>> p.F90:12:15:
> >>>
> >>> 12 |   select type (b)
> >>>|   1
> >>> Error: Associating entity 'b' at (1) is a procedure name
> >>> p.F90:14:5:
> >>>
> >>> 14 |  b=c
> >>>| 1
> >>> Error: 'b' at (1) associated to vector-indexed target cannot be used
> >>> in a variable definition context (assignment)
> >>>
> >>>
> >>> The code compiles successfully using ifort 18.0.1. Removing the
> >>> "recursive" attribute, or specifying a "result()" variable makes the
> >>> errors go away.
> >>>
> >>>
> >>> --
> >>>
> >>> * Andrew Benson:
> >>> http://users.obs.carnegiescience.edu/abenson/contact.html
> >>>
> >>> * Galacticus: https://github.com/galacticusorg/galacticus



-- 
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein

Re: [RFC] [c-family] PR92867 - Add returns_arg attribute

[Prathamesh Kulkarni]

On Thu, 30 Jan 2020 at 19:17, Richard Biener  wrote:
>
> On Thu, Jan 30, 2020 at 11:49 AM Prathamesh Kulkarni
>  wrote:
> >
> > On Wed, 29 Jan 2020 at 14:38, Richard Biener  
> > wrote:
> > >
> > > On Tue, Jan 28, 2020 at 1:02 PM Jakub Jelinek  wrote:
> > > >
> > > > On Tue, Jan 28, 2020 at 05:09:36PM +0530, Prathamesh Kulkarni wrote:
> > > > > On Tue, 28 Jan 2020 at 17:00, Jakub Jelinek  wrote:
> > > > > >
> > > > > > On Tue, Jan 28, 2020 at 04:56:59PM +0530, Prathamesh Kulkarni wrote:
> > > > > > > Thanks for the suggestions. In the attached patch I bumped up 
> > > > > > > value of
> > > > > > > ERF_RETURNS_ARG_MASK
> > > > > > > to UINT_MAX >> 2, and use highest two bits for ERF_NOALIAS and 
> > > > > > > ERF_RETURNS_ARG.
> > > > > > > And use fn spec "Z" to store the argument number in 
> > > > > > > fn-spec format.
> > > > > > > Does that look OK ?
> > > > > >
> > > > > > No.
> > > > > >
> > > > > > +#define ERF_RETURN_ARG_MASK(UINT_MAX >> 2)
> > > > > >
> > > > > >  /* Nonzero if the return value is equal to the argument number
> > > > > > flags & ERF_RETURN_ARG_MASK.  */
> > > > > > -#define ERF_RETURNS_ARG(1 << 2)
> > > > > > +#define ERF_RETURNS_ARG(1 << (BITS_PER_WORD - 2))
> > > > > >
> > > > > > How is size of host int related to BITS_PER_WORD?  Not to mention 
> > > > > > that
> > > > > > if BITS_PER_WORD is 64 and host int is 32-bit, 1 << (64 - 2) is UB.
> > > > > Oops sorry. I should have used HOST_BITS_PER_INT.
> > > > > I assume that'd be correct ?
> > > >
> > > > It still wouldn't, 1 << (HOST_BITS_PER_INT - 1) is negative number, 
> > > > you'd
> > > > need either 1U and verify all ERF_* flags uses, or avoid using the sign 
> > > > bit.
> > > > The patch has other issues, you don't verify that the argnum fits into
> > > > the bits (doesn't overflow into the other ERF_* bits), in
> > > > +  char *s = (char *) xmalloc (sizeof (char) * BITS_PER_WORD);
> > > > +  s[0] = 'Z';
> > > > +  sprintf (s + 1, "%lu", argnum);
> > > > 1) sizeof (char) is 1 by definition
> > > > 2) it is pointless to allocate it and then deallocate (just use 
> > > > automatic
> > > > array)
> > > > 3) it is unclear how is BITS_PER_WORD related to the length of decimal
> > > > encoded string + Z char + terminating '\0'.  The usual way is for 
> > > > unsigned
> > > > numbers to estimate number of digits by counting 3 digits per each 8 
> > > > bits,
> > > > in your case of course + 2.
> > > >
> > > > More importantly, the "fn spec" attribute isn't used just in
> > > > gimple_call_return_flags, but also in e.g. gimple_call_arg_flags which
> > > > assumes that the return stuff in there is a single char and the reaming
> > > > chars are for argument descriptions, or in decl_return_flags which you
> > > > haven't modified.
> > > >
> > > > I must say I fail to see the point in trying to glue this together into 
> > > > the
> > > > "fn spec" argument so incompatibly, why can't we handle the fn spec 
> > > > with its
> > > > 1-4 returns_arg and returns_arg attribute with arbitrary position
> > > > side-by-side?
> > >
> > > Yeah, I wouldn't have added "fn spec" for "returns_arg" but kept the
> > > query interface thus access it via gimple_call_return_flags and use
> > > ERF_*.  For the flags adjustment just up the maximum argument
> > > to 1<<15 then the argument number is also nicely aligned, no need
> > > to do fancy limiting that depends on the host.  For too large
> > > argument numbers just warn the attribute is ignored.  I'd say even
> > > a max of 255 is sane just the existing limit is a bit too low.
> > Hi,
> > Thanks for the suggestions. In the attached patch, I use TREE_VALUE
> > (attr) to store/retrieve
> > arbitrary argument position, and have bumped ERF_RETURNS_ARG_MASK to 0x3fff.
> > Does it look OK ?
>
> +  warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wattributes,
> + "%qE attribute ignored on a function returning %qT.",
> + name, rettype);
>
> no punctuation in diagnostics (trailing '.'), also elsewhere in the patch.
>
> +  tree fndecl = gimple_call_fndecl (stmt);
> +  attr = lookup_attribute ("returns_arg", DECL_ATTRIBUTES (fndecl));
> +  if (attr)
> +{
> +  unsigned argnum = tree_to_uhwi (TREE_VALUE (TREE_VALUE (attr)));
> +  return ERF_RETURNS_ARG | argnum;
>
> please verify argnum against ERF_ARG_MASK.
>
> +  tree attr = lookup_attribute ("returns_arg", DECL_ATTRIBUTES (decl));
> +  if (attr)
> +TREE_VALUE (attr) = build_int_cst (unsigned_type_node, argnum);
> +  else
> +DECL_ATTRIBUTES (decl)
> +  = tree_cons (get_identifier ("returns_arg"),
> +  build_int_cst (unsigned_type_node, argnum),
> + DECL_ATTRIBUTES (decl));
> +  return NULL_TREE;
>
> what's this for?  for *no_add_attrs = false you get the attribute
> added by the caller.
> Also other positional_argument callers overwrite TREE_VALUE with the result
> from the function.
Ah, thanks for pointing out!
Doe

Re: [OpenACC] bump version for 2.6 plus libgomp.texi update — document acc_attach/acc_detach, acc_*_async, acc_*_finalize(_async) functions

[Tobias Burnus]

I have now installed this patch as 
r10-6403-ge464fc903506b75bef90374ab520b52df317a00e

Namely, the bits:
*https://gcc.gnu.org/ml/gcc-patches/2020-01/msg00600.html  – main patch
*https://gcc.gnu.org/ml/gcc-patches/2020-01/msg01173.html  – remove more
'experimental' status.

Cheers,

Tobias

On 1/20/20 10:39 PM, Sandra Loosemore wrote:

On 1/20/20 3:08 AM, Tobias Burnus wrote:

Hi Sandra,

On 1/20/20 5:39 AM, Sandra Loosemore wrote:
I happen to have noticed a couple weeks ago that this language about 
OpenACC support being experimental appears in multiple places in the 
gfortran manual, […]  The same disclaimer for that option in the 
main GCC manual was removed years ago, so unless the Fortran support 
is much more broken than the C/C++ support, I think it ought to be 
removed from the Fortran manual as well.  […]


I concur. That would be the attached patch (on top of the previous 
patch* in this thread).


This is good, thank you.

-Sandra

Re: [RFC] [c-family] PR92867 - Add returns_arg attribute

[Prathamesh Kulkarni]

On Mon, 3 Feb 2020 at 14:41, Prathamesh Kulkarni
 wrote:
>
> On Thu, 30 Jan 2020 at 19:17, Richard Biener  
> wrote:
> >
> > On Thu, Jan 30, 2020 at 11:49 AM Prathamesh Kulkarni
> >  wrote:
> > >
> > > On Wed, 29 Jan 2020 at 14:38, Richard Biener  
> > > wrote:
> > > >
> > > > On Tue, Jan 28, 2020 at 1:02 PM Jakub Jelinek  wrote:
> > > > >
> > > > > On Tue, Jan 28, 2020 at 05:09:36PM +0530, Prathamesh Kulkarni wrote:
> > > > > > On Tue, 28 Jan 2020 at 17:00, Jakub Jelinek  
> > > > > > wrote:
> > > > > > >
> > > > > > > On Tue, Jan 28, 2020 at 04:56:59PM +0530, Prathamesh Kulkarni 
> > > > > > > wrote:
> > > > > > > > Thanks for the suggestions. In the attached patch I bumped up 
> > > > > > > > value of
> > > > > > > > ERF_RETURNS_ARG_MASK
> > > > > > > > to UINT_MAX >> 2, and use highest two bits for ERF_NOALIAS and 
> > > > > > > > ERF_RETURNS_ARG.
> > > > > > > > And use fn spec "Z" to store the argument number in 
> > > > > > > > fn-spec format.
> > > > > > > > Does that look OK ?
> > > > > > >
> > > > > > > No.
> > > > > > >
> > > > > > > +#define ERF_RETURN_ARG_MASK(UINT_MAX >> 2)
> > > > > > >
> > > > > > >  /* Nonzero if the return value is equal to the argument number
> > > > > > > flags & ERF_RETURN_ARG_MASK.  */
> > > > > > > -#define ERF_RETURNS_ARG(1 << 2)
> > > > > > > +#define ERF_RETURNS_ARG(1 << (BITS_PER_WORD - 2))
> > > > > > >
> > > > > > > How is size of host int related to BITS_PER_WORD?  Not to mention 
> > > > > > > that
> > > > > > > if BITS_PER_WORD is 64 and host int is 32-bit, 1 << (64 - 2) is 
> > > > > > > UB.
> > > > > > Oops sorry. I should have used HOST_BITS_PER_INT.
> > > > > > I assume that'd be correct ?
> > > > >
> > > > > It still wouldn't, 1 << (HOST_BITS_PER_INT - 1) is negative number, 
> > > > > you'd
> > > > > need either 1U and verify all ERF_* flags uses, or avoid using the 
> > > > > sign bit.
> > > > > The patch has other issues, you don't verify that the argnum fits into
> > > > > the bits (doesn't overflow into the other ERF_* bits), in
> > > > > +  char *s = (char *) xmalloc (sizeof (char) * BITS_PER_WORD);
> > > > > +  s[0] = 'Z';
> > > > > +  sprintf (s + 1, "%lu", argnum);
> > > > > 1) sizeof (char) is 1 by definition
> > > > > 2) it is pointless to allocate it and then deallocate (just use 
> > > > > automatic
> > > > > array)
> > > > > 3) it is unclear how is BITS_PER_WORD related to the length of decimal
> > > > > encoded string + Z char + terminating '\0'.  The usual way is for 
> > > > > unsigned
> > > > > numbers to estimate number of digits by counting 3 digits per each 8 
> > > > > bits,
> > > > > in your case of course + 2.
> > > > >
> > > > > More importantly, the "fn spec" attribute isn't used just in
> > > > > gimple_call_return_flags, but also in e.g. gimple_call_arg_flags which
> > > > > assumes that the return stuff in there is a single char and the 
> > > > > reaming
> > > > > chars are for argument descriptions, or in decl_return_flags which you
> > > > > haven't modified.
> > > > >
> > > > > I must say I fail to see the point in trying to glue this together 
> > > > > into the
> > > > > "fn spec" argument so incompatibly, why can't we handle the fn spec 
> > > > > with its
> > > > > 1-4 returns_arg and returns_arg attribute with arbitrary position
> > > > > side-by-side?
> > > >
> > > > Yeah, I wouldn't have added "fn spec" for "returns_arg" but kept the
> > > > query interface thus access it via gimple_call_return_flags and use
> > > > ERF_*.  For the flags adjustment just up the maximum argument
> > > > to 1<<15 then the argument number is also nicely aligned, no need
> > > > to do fancy limiting that depends on the host.  For too large
> > > > argument numbers just warn the attribute is ignored.  I'd say even
> > > > a max of 255 is sane just the existing limit is a bit too low.
> > > Hi,
> > > Thanks for the suggestions. In the attached patch, I use TREE_VALUE
> > > (attr) to store/retrieve
> > > arbitrary argument position, and have bumped ERF_RETURNS_ARG_MASK to 
> > > 0x3fff.
> > > Does it look OK ?
> >
> > +  warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wattributes,
> > + "%qE attribute ignored on a function returning %qT.",
> > + name, rettype);
> >
> > no punctuation in diagnostics (trailing '.'), also elsewhere in the patch.
> >
> > +  tree fndecl = gimple_call_fndecl (stmt);
> > +  attr = lookup_attribute ("returns_arg", DECL_ATTRIBUTES (fndecl));
> > +  if (attr)
> > +{
> > +  unsigned argnum = tree_to_uhwi (TREE_VALUE (TREE_VALUE (attr)));
> > +  return ERF_RETURNS_ARG | argnum;
> >
> > please verify argnum against ERF_ARG_MASK.
> >
> > +  tree attr = lookup_attribute ("returns_arg", DECL_ATTRIBUTES (decl));
> > +  if (attr)
> > +TREE_VALUE (attr) = build_int_cst (unsigned_type_node, argnum);
> > +  else
> > +DECL_ATTRIBUTES (decl)
> > +  = tree_cons (get_identifier ("returns_arg"),
> > +  build_int_cst (un

Re: [PATCH] simplify-rtx: Extend (truncate (*extract ...)) fold [PR87763]

[Maxim Kuvyrkov]

Hi Richard,

You patch below increases code-size on aarch64-linux-gnu with -Os on SPEC2k6 
400.perlbench and 453.povray -- by 1% and 2% respectively.

400.perlbench,perlbench_base.default,   101,939261,951221
453.povray,povray_base.default, 102,707807,721399

Would you please check whether these can be avoided?  [Let me know if you need 
help reproducing this problem.]

Thank you,

--
Maxim Kuvyrkov
https://www.linaro.org

> On Jan 27, 2020, at 7:41 PM, Richard Sandiford  
> wrote:
> 
> In the gcc.target/aarch64/lsl_asr_sbfiz.c part of this PR, we have:
> 
> Failed to match this instruction:
> (set (reg:SI 95)
>(ashift:SI (subreg:SI (sign_extract:DI (subreg:DI (reg:SI 97) 0)
>(const_int 3 [0x3])
>(const_int 0 [0])) 0)
>(const_int 19 [0x13])))
> 
> If we perform the natural simplification to:
> 
> (set (reg:SI 95)
>(ashift:SI (sign_extract:SI (reg:SI 97)
>(const_int 3 [0x3])
>(const_int 0 [0])) 0)
>(const_int 19 [0x13])))
> 
> then the pattern matches.  And it turns out that we do have a
> simplification like that already, but it would only kick in for
> extractions from a reg, not a subreg.  E.g.:
> 
> (set (reg:SI 95)
>(ashift:SI (subreg:SI (sign_extract:DI (reg:DI X)
>(const_int 3 [0x3])
>(const_int 0 [0])) 0)
>(const_int 19 [0x13])))
> 
> would simplify to:
> 
> (set (reg:SI 95)
>(ashift:SI (sign_extract:SI (subreg:SI (reg:DI X) 0)
>(const_int 3 [0x3])
>(const_int 0 [0])) 0)
>(const_int 19 [0x13])))
> 
> IMO the subreg case is even more obviously a simplification
> than the bare reg case, since the net effect is to remove
> either one or two subregs, rather than simply change the
> position of a subreg/truncation.
> 
> However, doing that regressed gcc.dg/tree-ssa/pr64910-2.c
> for -m32 on x86_64-linux-gnu, because we could then simplify
> a :HI zero_extract to a :QI one.  The associated *testqi_ext_3
> pattern did already seem to want to handle QImode extractions:
> 
>  "ix86_match_ccmode (insn, CCNOmode)
>   && ((TARGET_64BIT && GET_MODE (operands[2]) == DImode)
>   || GET_MODE (operands[2]) == SImode
>   || GET_MODE (operands[2]) == HImode
>   || GET_MODE (operands[2]) == QImode)
> 
> but I'm not sure how often the QI case would trigger in practice,
> since the zero_extract mode was restricted to HI and above.  I checked
> the other x86 patterns and couldn't see any other instances of this.
> 
> Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linux-gnu,
> OK to install?
> 
> Richard
> 
> 
> 2020-01-27  Richard Sandiford  
> 
> gcc/
>   PR rtl-optimization/87763
>   * simplify-rtx.c (simplify_truncation): Extend sign/zero_extract
>   simplification to handle subregs as well as bare regs.
>   * config/i386/i386.md (*testqi_ext_3): Match QI extracts too.
> ---
> gcc/config/i386/i386.md | 2 +-
> gcc/simplify-rtx.c  | 4 +++-
> 2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
> index 6e9c9bd2fb6..a125ab350bb 100644
> --- a/gcc/config/i386/i386.md
> +++ b/gcc/config/i386/i386.md
> @@ -8927,7 +8927,7 @@ (define_insn "*testqi_ext_2"
> (define_insn_and_split "*testqi_ext_3"
>   [(set (match_operand 0 "flags_reg_operand")
> (match_operator 1 "compare_operator"
> -   [(zero_extract:SWI248
> +   [(zero_extract:SWI
>(match_operand 2 "nonimmediate_operand" "rm")
>(match_operand 3 "const_int_operand" "n")
>(match_operand 4 "const_int_operand" "n"))
> diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
> index eff1d07a253..db4f9339c15 100644
> --- a/gcc/simplify-rtx.c
> +++ b/gcc/simplify-rtx.c
> @@ -736,7 +736,9 @@ simplify_truncation (machine_mode mode, rtx op,
>  (*_extract:M1 (truncate:M1 (reg:M2)) (len) (pos')) if possible without
>  changing len.  */
>   if ((GET_CODE (op) == ZERO_EXTRACT || GET_CODE (op) == SIGN_EXTRACT)
> -  && REG_P (XEXP (op, 0))
> +  && (REG_P (XEXP (op, 0))
> +   || (SUBREG_P (XEXP (op, 0))
> +   && REG_P (SUBREG_REG (XEXP (op, 0)
>   && GET_MODE (XEXP (op, 0)) == GET_MODE (op)
>   && CONST_INT_P (XEXP (op, 1))
>   && CONST_INT_P (XEXP (op, 2)))
> -- 
> 2.17.1
> 

[committed] Backport to GCC 9: [Patch,Fortran] PR93309 – permit repeated 'implicit none(external)' in sub-scoping unit

[Tobias Burnus]

Backported to GCC 9, cf. attachment.

Tobias

On 1/20/20 3:55 PM, Tobias Burnus wrote:
Using "implicit none" multiple times in a scoping unit is not 
permitted – and checked for.


However, using one in the parent name space and re-confirming it in 
the current name space is permitted – but was before rejected.


OK for the trunk?

Tobias

commit 5c80a1bd426a4aeccff0da54ab80d93d7973590e
Author: Tobias Burnus 
Date:   Mon Feb 3 11:48:17 2020 +0100

Fortran] PR93309 – permit repeated 'implicit none(external)'

Backported from mainline
2020-01-21  Tobias Burnus  

PR fortran/93309
* interface.c (gfc_procedure_use): Also check parent namespace for
'implict none (external)'.
* symbol.c (gfc_get_namespace): Don't set has_implicit_none_export
to parent namespace's setting.

Backported from mainline
2020-01-21  Tobias Burnus  

PR fortran/93309
* gfortran.dg/external_implicit_none_2.f90: New.
---
 gcc/fortran/ChangeLog  | 13 +-
 gcc/fortran/interface.c|  7 +-
 gcc/fortran/symbol.c   |  3 ---
 gcc/testsuite/ChangeLog|  8 +++
 .../gfortran.dg/external_implicit_none_2.f90   | 28 ++
 5 files changed, 54 insertions(+), 5 deletions(-)

diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog
index e0a34420e36..ec87cc9e6cf 100644
--- a/gcc/fortran/ChangeLog
+++ b/gcc/fortran/ChangeLog
@@ -1,3 +1,14 @@
+2020-02-03  Tobias Burnus  
+
+	Backported from mainline
+	2020-01-21  Tobias Burnus  
+
+	PR fortran/93309
+	* interface.c (gfc_procedure_use): Also check parent namespace for
+	'implict none (external)'.
+	* symbol.c (gfc_get_namespace): Don't set has_implicit_none_export
+	to parent namespace's setting.
+
 2020-01-22  Jakub Jelinek  
 
 	* parse.c (parse_omp_structured_block): Handle ST_OMP_TARGET_PARALLEL.
@@ -10,7 +21,7 @@
 
 2020-01-17  Mark Eggleston  
 
-Backport from mainline
+	Backport from mainline
 	Mark Eggleston  
 
 	PR fortran/93236
diff --git a/gcc/fortran/interface.c b/gcc/fortran/interface.c
index bee98129bc1..b5701b1a59a 100644
--- a/gcc/fortran/interface.c
+++ b/gcc/fortran/interface.c
@@ -3671,7 +3671,12 @@ gfc_procedure_use (gfc_symbol *sym, gfc_actual_arglist **ap, locus *where)
  explicitly declared at all if requested.  */
   if (sym->attr.if_source == IFSRC_UNKNOWN && !sym->attr.is_iso_c)
 {
-  if (sym->ns->has_implicit_none_export && sym->attr.proc == PROC_UNKNOWN)
+  bool has_implicit_none_export = false;
+  if (sym->attr.proc == PROC_UNKNOWN)
+	for (gfc_namespace *ns = sym->ns; ns; ns = ns->parent)
+	   if (ns->has_implicit_none_export)
+	 has_implicit_none_export = true;
+  if (has_implicit_none_export)
 	{
 	  const char *guessed
 	= gfc_lookup_function_fuzzy (sym->name, sym->ns->sym_root);
diff --git a/gcc/fortran/symbol.c b/gcc/fortran/symbol.c
index 2b8f86e0881..faaeebf2c09 100644
--- a/gcc/fortran/symbol.c
+++ b/gcc/fortran/symbol.c
@@ -2899,9 +2899,6 @@ gfc_get_namespace (gfc_namespace *parent, int parent_types)
 	}
 }
 
-  if (parent_types && ns->parent != NULL)
-ns->has_implicit_none_export = ns->parent->has_implicit_none_export;
-
   ns->refs = 1;
 
   return ns;
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 9e117a6854a..0f8e7592665 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,11 @@
+2020-02-03  Tobias Burnus  
+
+	Backported from mainline
+	2020-01-21  Tobias Burnus  
+
+	PR fortran/93309
+	* gfortran.dg/external_implicit_none_2.f90: New.
+
 2020-01-30  Kito Cheng  
 
 	Backport from mainline
diff --git a/gcc/testsuite/gfortran.dg/external_implicit_none_2.f90 b/gcc/testsuite/gfortran.dg/external_implicit_none_2.f90
new file mode 100644
index 000..b2b1dd1e6d7
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/external_implicit_none_2.f90
@@ -0,0 +1,28 @@
+! { dg-do compile }
+!
+! PR fortran/93309
+!
+module m
+  implicit none(external)
+contains
+  subroutine s
+implicit none(external) ! OK
+  end subroutine
+end module
+
+module m2
+  implicit none(external)
+contains
+  subroutine s
+call foo(1)  ! { dg-error "not explicitly declared" }
+  end subroutine
+end module
+
+module m3
+  implicit none(external)
+contains
+  subroutine s
+implicit none(external) ! OK
+implicit none(external) ! { dg-error "Duplicate IMPLICIT NONE statement" }
+  end subroutine
+end module

[committed] Backport to GCC 9: [Patch,committed][Fortran] Disable front-end optimization for OpenACC atomic (PR93462)

[Tobias Burnus]

And now committed to GCC 9 branch as well. 
r10-6403-ge464fc903506b75bef90374ab520b52df317a00e


Tobias

On 1/31/20 3:55 PM, Tobias Burnus wrote:

The OpenACC code
!$acc atomic write
   a = f(n) - f(n)
got -ffrontend-optimize'd such that it ICEed in gfc_trans_omp_atomic.

The same issue occurred for OpenMP in PR 92977 and was solved by
disabling the optimization for EXEC_OMP_ATOMIC.

This patch does now the same for OpenACC (EXEC_OACC_ATOMIC).

Committed as obvious to the trunk.

Tobias

commit e5446f2201d93fc9adc913ed320aa70437ff4235
Author: Tobias Burnus 
Date:   Mon Feb 3 12:09:46 2020 +0100

[Fortran] Disable front-end optimization for OpenACC atomic (PR93462)

Backported from mainline
2020-01-31  Tobias Burnus  

PR fortran/93462
* frontend-passes.c (gfc_code_walker): For EXEC_OACC_ATOMIC, set
in_omp_atomic to true prevent front-end optimization.

PR fortran/93462
* gfortran.dg/goacc/atomic-1.f90: New.
---
 gcc/fortran/ChangeLog|  9 +
 gcc/fortran/frontend-passes.c|  1 +
 gcc/testsuite/ChangeLog  |  8 
 gcc/testsuite/gfortran.dg/goacc/atomic-1.f90 | 17 +
 4 files changed, 35 insertions(+)

diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog
index ec87cc9e6cf..ca80c1bcd22 100644
--- a/gcc/fortran/ChangeLog
+++ b/gcc/fortran/ChangeLog
@@ -1,3 +1,12 @@
+2020-02-03  Tobias Burnus  
+
+	Backported from mainline
+	2020-01-31  Tobias Burnus  
+
+	PR fortran/93462
+	* frontend-passes.c (gfc_code_walker): For EXEC_OACC_ATOMIC, set
+	in_omp_atomic to true prevent front-end optimization.
+
 2020-02-03  Tobias Burnus  
 
 	Backported from mainline
diff --git a/gcc/fortran/frontend-passes.c b/gcc/fortran/frontend-passes.c
index 4bb6cbb5d74..a6e710beb33 100644
--- a/gcc/fortran/frontend-passes.c
+++ b/gcc/fortran/frontend-passes.c
@@ -5260,6 +5260,7 @@ gfc_code_walker (gfc_code **c, walk_code_fn_t codefn, walk_expr_fn_t exprfn,
 	  WALK_SUBEXPR (co->ext.dt->extra_comma);
 	  break;
 
+	case EXEC_OACC_ATOMIC:
 	case EXEC_OMP_ATOMIC:
 	  in_omp_atomic = true;
 	  break;
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 0f8e7592665..e3610fd7429 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,11 @@
+2020-02-03  Tobias Burnus  
+
+	Backported from mainline
+	2020-01-31  Tobias Burnus  
+
+	PR fortran/93462
+	* gfortran.dg/goacc/atomic-1.f90: New.
+
 2020-02-03  Tobias Burnus  
 
 	Backported from mainline
diff --git a/gcc/testsuite/gfortran.dg/goacc/atomic-1.f90 b/gcc/testsuite/gfortran.dg/goacc/atomic-1.f90
new file mode 100644
index 000..579f0494b78
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/goacc/atomic-1.f90
@@ -0,0 +1,17 @@
+! { dg-do compile }
+!
+! PR fortran/93462
+!
+! Contributed by G. Steinmetz
+!
+program p
+   integer :: n = 1
+   integer :: a
+!$acc atomic write
+   a = f(n) - f(n)
+contains
+   integer function f(x)
+  integer, intent(in) :: x
+  f = x
+   end
+end

[Pingx3][GCC][PATCH][ARM]Add ACLE intrinsics for dot product (vusdot - vector, vdot - by element) for AArch32 AdvSIMD ARMv8.6 Extension

[Stam Markianos-Wright]

On 1/27/20 3:54 PM, Stam Markianos-Wright wrote:


On 1/16/20 4:05 PM, Stam Markianos-Wright wrote:



On 1/10/20 6:48 PM, Stam Markianos-Wright wrote:



On 12/18/19 1:25 PM, Stam Markianos-Wright wrote:



On 12/13/19 10:22 AM, Stam Markianos-Wright wrote:

Hi all,

This patch adds the ARMv8.6 Extension ACLE intrinsics for dot product
operations (vector/by element) to the ARM back-end.

These are:
usdot (vector), dot (by element).

The functions are optional from ARMv8.2-a as -march=armv8.2-a+i8mm and
for ARM they remain optional as of ARMv8.6-a.

The functions are declared in arm_neon.h, RTL patterns are defined to
generate assembler and tests are added to verify and perform adequate checks.

Regression testing on arm-none-eabi passed successfully.

This patch depends on:

https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02195.html

for ARM CLI updates, and on:

https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00857.html

for testsuite effective_target update.

Ok for trunk?




New diff addressing review comments from Aarch64 version of the patch.

_Change of order of operands in RTL patterns.
_Change tests to use check-function-bodies, compile with optimisation and 
check for exact registers.

_Rename tests to remove "-compile-" in filename.




.Ping!
.


Cheers,
Stam




ACLE documents are at https://developer.arm.com/docs/101028/latest
ISA documents are at https://developer.arm.com/docs/ddi0596/latest

PS. I don't have commit rights, so if someone could commit on my behalf,
that would be great :)


gcc/ChangeLog:

2019-11-28  Stam Markianos-Wright  

 * config/arm/arm-builtins.c (enum arm_type_qualifiers):
 (USTERNOP_QUALIFIERS): New define.
 (USMAC_LANE_QUADTUP_QUALIFIERS): New define.
 (SUMAC_LANE_QUADTUP_QUALIFIERS): New define.
 (arm_expand_builtin_args):
 Add case ARG_BUILTIN_LANE_QUADTUP_INDEX.
 (arm_expand_builtin_1): Add qualifier_lane_quadtup_index.
 * config/arm/arm_neon.h (vusdot_s32): New.
 (vusdot_lane_s32): New.
 (vusdotq_lane_s32): New.
 (vsudot_lane_s32): New.
 (vsudotq_lane_s32): New.
 * config/arm/arm_neon_builtins.def
 (usdot,usdot_lane,sudot_lane): New.
 * config/arm/iterators.md (DOTPROD_I8MM): New.
 (sup, opsuffix): Add .
    * config/arm/neon.md (neon_usdot, dot_lane: New.
 * config/arm/unspecs.md (UNSPEC_DOT_US, UNSPEC_DOT_SU): New.


gcc/testsuite/ChangeLog:

2019-12-12  Stam Markianos-Wright  

 * gcc.target/arm/simd/vdot-compile-2-1.c: New test.
 * gcc.target/arm/simd/vdot-compile-2-2.c: New test.
 * gcc.target/arm/simd/vdot-compile-2-3.c: New test.
 * gcc.target/arm/simd/vdot-compile-2-4.c: New test.

Re: [PR47785] COLLECT_AS_OPTIONS

[Prathamesh Kulkarni]

On Thu, 30 Jan 2020 at 19:10, Richard Biener  wrote:
>
> On Thu, Jan 30, 2020 at 5:31 AM Prathamesh Kulkarni
>  wrote:
> >
> > On Tue, 28 Jan 2020 at 17:17, Richard Biener  
> > wrote:
> > >
> > > On Fri, Jan 24, 2020 at 7:04 AM Prathamesh Kulkarni
> > >  wrote:
> > > >
> > > > On Mon, 20 Jan 2020 at 15:44, Richard Biener 
> > > >  wrote:
> > > > >
> > > > > On Wed, Jan 8, 2020 at 11:20 AM Prathamesh Kulkarni
> > > > >  wrote:
> > > > > >
> > > > > > On Tue, 5 Nov 2019 at 17:38, Richard Biener 
> > > > > >  wrote:
> > > > > > >
> > > > > > > On Tue, Nov 5, 2019 at 12:17 AM Kugan Vivekanandarajah
> > > > > > >  wrote:
> > > > > > > >
> > > > > > > > Hi,
> > > > > > > > Thanks for the review.
> > > > > > > >
> > > > > > > > On Tue, 5 Nov 2019 at 03:57, H.J. Lu  
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > On Sun, Nov 3, 2019 at 6:45 PM Kugan Vivekanandarajah
> > > > > > > > >  wrote:
> > > > > > > > > >
> > > > > > > > > > Thanks for the reviews.
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > On Sat, 2 Nov 2019 at 02:49, H.J. Lu  
> > > > > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > On Thu, Oct 31, 2019 at 6:33 PM Kugan Vivekanandarajah
> > > > > > > > > > >  wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > On Wed, 30 Oct 2019 at 03:11, H.J. Lu 
> > > > > > > > > > > >  wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > On Sun, Oct 27, 2019 at 6:33 PM Kugan Vivekanandarajah
> > > > > > > > > > > > >  wrote:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Hi Richard,
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Thanks for the review.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > On Wed, 23 Oct 2019 at 23:07, Richard Biener 
> > > > > > > > > > > > > >  wrote:
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > On Mon, Oct 21, 2019 at 10:04 AM Kugan 
> > > > > > > > > > > > > > > Vivekanandarajah
> > > > > > > > > > > > > > >  wrote:
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Hi Richard,
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Thanks for the pointers.
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > On Fri, 11 Oct 2019 at 22:33, Richard Biener 
> > > > > > > > > > > > > > > >  wrote:
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > On Fri, Oct 11, 2019 at 6:15 AM Kugan 
> > > > > > > > > > > > > > > > > Vivekanandarajah
> > > > > > > > > > > > > > > > >  wrote:
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > Hi Richard,
> > > > > > > > > > > > > > > > > > Thanks for the review.
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > On Wed, 2 Oct 2019 at 20:41, Richard Biener 
> > > > > > > > > > > > > > > > > >  wrote:
> > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > On Wed, Oct 2, 2019 at 10:39 AM Kugan 
> > > > > > > > > > > > > > > > > > > Vivekanandarajah
> > > > > > > > > > > > > > > > > > >  wrote:
> > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > Hi,
> > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > As mentioned in the PR, attached patch 
> > > > > > > > > > > > > > > > > > > > adds COLLECT_AS_OPTIONS for
> > > > > > > > > > > > > > > > > > > > passing assembler options specified 
> > > > > > > > > > > > > > > > > > > > with -Wa, to the link-time driver.
> > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > The proposed solution only works for 
> > > > > > > > > > > > > > > > > > > > uniform -Wa options across all
> > > > > > > > > > > > > > > > > > > > TUs. As mentioned by Richard Biener, 
> > > > > > > > > > > > > > > > > > > > supporting non-uniform -Wa flags
> > > > > > > > > > > > > > > > > > > > would require either adjusting 
> > > > > > > > > > > > > > > > > > > > partitioning according to flags or
> > > > > > > > > > > > > > > > > > > > emitting multiple object files  from a 
> > > > > > > > > > > > > > > > > > > > single LTRANS CU. We could
> > > > > > > > > > > > > > > > > > > > consider this as a follow up.
> > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > Bootstrapped and regression tests on  
> > > > > > > > > > > > > > > > > > > > arm-linux-gcc. Is this OK for trunk?
> > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > While it works for your simple cases it 
> > > > > > > > > > > > > > > > > > > is unlikely to work in practice since
> > > > > > > > > > > > > > > > > > > your implementation needs the assembler 
> > > > > > > > > > > > > > > > > > > options be present at the link
> > > > > > > > > > > > > > > > > > > command line.  I agree that this might be 
> > > > > > > > > > > > > > > > > > > the way for people to go when
> > > > > > > > > > > > > > > > > > > they face the issue but then it needs to 
> > > > > > > > > > > > > > > > > > > be docume

[PATCH 1/4] [ARC] Update mlo/mhi handling when big-endian CPU.

[Claudiu Zissulescu]

The ARC 600 MUL64 instructions are using mlo/mhi registers to pass the
64-bit result. However, the mlo/mhi registers are not swapping
depending on endianess.  Update multiplication patterns to reflect
this fact.

gcc/
-xx-xx  Claudiu Zissulescu  

* config/arc/arc.md (mulsidi_600): Correctly select mlo/mhi
registers.
(umulsidi_600): Likewise.

testsuite/
-xx-xx  Claudiu Zissulescu  
Petro Karashchenko  

* estsuite/gcc.target/arc/mul64-1.c: New test.
---
 gcc/config/arc/arc.md  | 50 --
 gcc/testsuite/gcc.target/arc/mul64-1.c | 23 
 2 files changed, 55 insertions(+), 18 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/arc/mul64-1.c

diff --git a/gcc/config/arc/arc.md b/gcc/config/arc/arc.md
index 9a96440025f..f19f2c32641 100644
--- a/gcc/config/arc/arc.md
+++ b/gcc/config/arc/arc.md
@@ -2288,19 +2288,26 @@ archs4x, archs4xd"
(set_attr "cond" "canuse,canuse,canuse_limm,canuse")])
 
 (define_insn_and_split "mulsidi_600"
-  [(set (match_operand:DI 0 "register_operand"   
"=c, c,c,  c")
-   (mult:DI (sign_extend:DI (match_operand:SI 1 "register_operand"  
"%Rcq#q, c,c,  c"))
-(sign_extend:DI (match_operand:SI 2 "nonmemory_operand"  
"Rcq#q,cL,L,C32"
-   (clobber (reg:DI MUL64_OUT_REG))]
+  [(set (match_operand:DI 0 "register_operand"   
"=r,r,  r")
+   (mult:DI (sign_extend:DI (match_operand:SI 1 "register_operand"  "%r,r, 
 r"))
+(sign_extend:DI (match_operand:SI 2 "nonmemory_operand" 
"rL,L,C32"
+   (clobber (reg:DI R58_REG))]
   "TARGET_MUL64_SET"
   "#"
-  "TARGET_MUL64_SET"
+  "TARGET_MUL64_SET && reload_completed"
   [(const_int 0)]
-  "emit_insn (gen_mul64 (operands[1], operands[2]));
-   emit_move_insn (operands[0], gen_rtx_REG (DImode, MUL64_OUT_REG));
-   DONE;"
+ {
+   int hi = !TARGET_BIG_ENDIAN;
+   int lo = !hi;
+   rtx lr = operand_subword (operands[0], lo, 0, DImode);
+   rtx hr = operand_subword (operands[0], hi, 0, DImode);
+   emit_insn (gen_mul64 (operands[1], operands[2]));
+   emit_move_insn (lr, gen_rtx_REG (SImode, R58_REG));
+   emit_move_insn (hr, gen_rtx_REG (SImode, R59_REG));
+   DONE;
+ }
   [(set_attr "type" "multi")
-   (set_attr "length" "8")])
+   (set_attr "length" "4,4,8")])
 
 (define_insn "mul64"
   [(set (reg:DI MUL64_OUT_REG)
@@ -2316,19 +2323,26 @@ archs4x, archs4xd"
(set_attr "cond" "canuse,canuse,canuse_limm,canuse")])
 
 (define_insn_and_split "umulsidi_600"
-  [(set (match_operand:DI 0 "register_operand"
"=c,c, c")
-   (mult:DI (zero_extend:DI (match_operand:SI 1 "register_operand"   
"%c,c, c"))
-(sign_extend:DI (match_operand:SI 2 "nonmemory_operand"  
"cL,L,C32"
-   (clobber (reg:DI MUL64_OUT_REG))]
+  [(set (match_operand:DI 0 "register_operand"
"=r,r, r")
+   (mult:DI (zero_extend:DI (match_operand:SI 1 "register_operand"   
"%r,r, r"))
+(zero_extend:DI (match_operand:SI 2 "nonmemory_operand"  
"rL,L,C32"
+   (clobber (reg:DI R58_REG))]
   "TARGET_MUL64_SET"
   "#"
-  "TARGET_MUL64_SET"
+  "TARGET_MUL64_SET && reload_completed"
   [(const_int 0)]
-  "emit_insn (gen_mulu64 (operands[1], operands[2]));
-   emit_move_insn (operands[0], gen_rtx_REG (DImode, MUL64_OUT_REG));
-   DONE;"
+ {
+   int hi = !TARGET_BIG_ENDIAN;
+   int lo = !hi;
+   rtx lr = operand_subword (operands[0], lo, 0, DImode);
+   rtx hr = operand_subword (operands[0], hi, 0, DImode);
+   emit_insn (gen_mulu64 (operands[1], operands[2]));
+   emit_move_insn (lr, gen_rtx_REG (SImode, R58_REG));
+   emit_move_insn (hr, gen_rtx_REG (SImode, R59_REG));
+   DONE;
+ }
   [(set_attr "type" "umulti")
-   (set_attr "length" "8")])
+   (set_attr "length" "4,4,8")])
 
 (define_insn "mulu64"
   [(set (reg:DI MUL64_OUT_REG)
diff --git a/gcc/testsuite/gcc.target/arc/mul64-1.c 
b/gcc/testsuite/gcc.target/arc/mul64-1.c
new file mode 100644
index 000..2543fc33d3f
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arc/mul64-1.c
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-skip-if "MUL64 is ARC600 extension." { ! { clmcpu } } } */
+/* { dg-options "-O2 -mmul64 -mbig-endian -mcpu=arc600" } */
+
+/* Check if mlo/mhi registers are correctly layout when we compile for
+   a big-endian CPU.  */
+
+#include 
+
+uint32_t foo (uint32_t x)
+{
+  return x % 1000;
+}
+
+int32_t bar (int32_t x)
+{
+  return x % 1000;
+}
+
+/* { dg-final { scan-assembler-times "\\s+mul64\\s+" 3 } } */
+/* { dg-final { scan-assembler-times "\\s+mulu64\\s+" 1 } } */
+/* { dg-final { scan-assembler-times "r\[0-9\]+,mhi" 2 } } */
+/* { dg-final { scan-assembler-times "r\[0-9\]+,mlo" 2 } } */
-- 
2.24.1

[PATCH 3/4] [ARC] Deprecate q-class option.

[Claudiu Zissulescu]

This option was used to control the short instruction selection.  However,
there is no difference in cycles if we use or not a short instruction,
and always someone wants a smaller program.

gcc/
-xx-xx  Claudiu Zissulescu  

* config/arc/arc.c (arc_conditional_register_usage): R0-R3 and
R12-R15 are always in ARCOMPACT16_REGS register class.
* config/arc/arc.opt (mq-class): Deprecate.
* config/arc/constraint.md ("q"): Remove dependency on mq-class
option.
* doc/invoke.texi (mq-class): Update text.
* common/config/arc/arc-common.c (arc_option_optimization_table):
Update list.

testsuite/
-xx-xx  Claudiu Zissulescu  

* gcc.target/arc/nps400-1.c: Update test.
---
 gcc/common/config/arc/arc-common.c  | 1 -
 gcc/config/arc/arc.c| 3 +--
 gcc/config/arc/arc.opt  | 2 +-
 gcc/config/arc/constraints.md   | 2 +-
 gcc/doc/invoke.texi | 2 +-
 gcc/testsuite/gcc.target/arc/nps400-1.c | 2 +-
 6 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/gcc/common/config/arc/arc-common.c 
b/gcc/common/config/arc/arc-common.c
index 0f73cc4dd18..0b77dd546e5 100644
--- a/gcc/common/config/arc/arc-common.c
+++ b/gcc/common/config/arc/arc-common.c
@@ -56,7 +56,6 @@ static const struct default_options 
arc_option_optimization_table[] =
 { OPT_LEVELS_SIZE, OPT_fbranch_count_reg, NULL, 0},
 { OPT_LEVELS_SIZE, OPT_fdelayed_branch, NULL, 0 },
 { OPT_LEVELS_SIZE, OPT_fsection_anchors, NULL, 1 },
-{ OPT_LEVELS_SIZE, OPT_mq_class, NULL, 1 },
 { OPT_LEVELS_SIZE, OPT_mcase_vector_pcrel, NULL, 1 },
 { OPT_LEVELS_SIZE, OPT_msize_level_, NULL, 3 },
 { OPT_LEVELS_SIZE, OPT_mmillicode, NULL, 1 },
diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c
index bd1e12b8a1f..960645fdfbf 100644
--- a/gcc/config/arc/arc.c
+++ b/gcc/config/arc/arc.c
@@ -1965,8 +1965,7 @@ arc_conditional_register_usage (void)
   for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
 if (i < ILINK1_REG)
   {
-   if ((TARGET_Q_CLASS || TARGET_RRQ_CLASS)
-   && ((i <= R3_REG) || ((i >= R12_REG) && (i <= R15_REG
+   if ((i <= R3_REG) || ((i >= R12_REG) && (i <= R15_REG)))
  arc_regno_reg_class[i] = ARCOMPACT16_REGS;
else
  arc_regno_reg_class[i] = GENERAL_REGS;
diff --git a/gcc/config/arc/arc.opt b/gcc/config/arc/arc.opt
index 72d72570629..45c2f5c36ad 100644
--- a/gcc/config/arc/arc.opt
+++ b/gcc/config/arc/arc.opt
@@ -316,7 +316,7 @@ Target Var(TARGET_CASE_VECTOR_PC_RELATIVE)
 Use pc-relative switch case tables - this enables case table shortening.
 
 mq-class
-Target Var(TARGET_Q_CLASS)
+Target Warn(%qs is deprecated)
 Enable 'q' instruction alternatives.
 
 mxy
diff --git a/gcc/config/arc/constraints.md b/gcc/config/arc/constraints.md
index 3be2a8abab0..b7a563a72ad 100644
--- a/gcc/config/arc/constraints.md
+++ b/gcc/config/arc/constraints.md
@@ -53,7 +53,7 @@
 (define_register_constraint "x" "R0_REGS"
   "@code{R0} register.")
 
-(define_register_constraint "q" "TARGET_Q_CLASS ? ARCOMPACT16_REGS : NO_REGS"
+(define_register_constraint "q" "ARCOMPACT16_REGS"
   "Registers usable in ARCompact 16-bit instructions: @code{r0}-@code{r3},
@code{r12}-@code{r15}")
 
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 90eab1e7a6d..178832c0729 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -17795,7 +17795,7 @@ code-density feature.
 
 @item -mq-class
 @opindex mq-class
-Enable @samp{q} instruction alternatives.
+Ths option is deprecated.  Enable @samp{q} instruction alternatives.
 This is the default for @option{-Os}.
 
 @item -mRcq
diff --git a/gcc/testsuite/gcc.target/arc/nps400-1.c 
b/gcc/testsuite/gcc.target/arc/nps400-1.c
index 504aad734cc..29486a30ee9 100644
--- a/gcc/testsuite/gcc.target/arc/nps400-1.c
+++ b/gcc/testsuite/gcc.target/arc/nps400-1.c
@@ -1,6 +1,6 @@
 /* { dg-do compile } */
 /* { dg-skip-if "" { ! { clmcpu } } } */
-/* { dg-options "-mcpu=nps400 -mq-class -mbitops -munaligned-access -mcmem -O2 
-fno-strict-aliasing" } */
+/* { dg-options "-mcpu=nps400 -mbitops -munaligned-access -mcmem -O2 
-fno-strict-aliasing" } */
 
 enum npsdp_mem_space_type {
   NPSDP_EXTERNAL_MS = 1
-- 
2.24.1

[PATCH 2/4] [ARC] Use TARGET_INSN_COST.

[Claudiu Zissulescu]

TARGET_INSN_COST gives us a better control over the instruction costs
than classical RTX_COSTS.  A simple cost scheme is in place for the
time being, when optimizing for size, the cost is given by the
instruction length. When optimizing for speed, the cost is 1 for any
recognized instruction, and 2 for any load/store instruction.  The
latter one can be overwritten by using cost attribute for an
instruction.  Due to this change, we need to update also a number of
instruction patterns with a new predicate to better reflect the costs.

gcc/
-xx-xx  Claudiu Zissulescu  

* config/arc/arc.c (arc_insn_cost): New function.
(TARGET_INSN_COST): Define.
* config/arc/arc.md (cost): New attribute.
(add_n): Use arc_nonmemory_operand.
(ashlsi3_insn): Likewise, also update constraints.
(ashrsi3_insn): Likewise.
(rotrsi3): Likewise.
(add_shift): Likewise.
* config/arc/predicates.md (arc_nonmemory_operand): New predicate.

testsuite/
-xx-xx  Claudiu Zissulescu  

* gcc.target/arc/or-cnst-size2.c: Update test.
---
 gcc/config/arc/arc.c | 52 
 gcc/config/arc/arc.md| 47 +-
 gcc/config/arc/predicates.md |  5 ++
 gcc/testsuite/gcc.target/arc/or-cnst-size2.c |  2 +-
 4 files changed, 83 insertions(+), 23 deletions(-)

diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c
index 7fa0fb51a4b..bd1e12b8a1f 100644
--- a/gcc/config/arc/arc.c
+++ b/gcc/config/arc/arc.c
@@ -11786,6 +11786,55 @@ arc_can_use_return_insn (void)
  && !ARC_INTERRUPT_P (arc_compute_function_type (cfun)));
 }
 
+/* Helper for INSN_COST.
+
+   Per Segher Boessenkool: rtx_costs computes the cost for any rtx (an
+   insn, a set, a set source, any random piece of one).  set_src_cost,
+   set_rtx_cost, etc. are helper functions that use that.
+
+   Those functions do not work for parallels.  Also, costs are not
+   additive like this simplified model assumes.  Also, more complex
+   backends tend to miss many cases in their rtx_costs function.
+
+   Many passes that want costs want to know the cost of a full insn.  Like
+   combine.  That's why I created insn_cost: it solves all of the above
+   problems.  */
+
+static int
+arc_insn_cost (rtx_insn *insn, bool speed)
+{
+  int cost;
+  if (recog_memoized (insn) < 0)
+return 0;
+
+  /* If optimizing for size, we want the insn size.  */
+  if (!speed)
+return get_attr_length (insn);
+
+  /* Use cost if provided.  */
+  cost = get_attr_cost (insn);
+  if (cost > 0)
+return cost;
+
+  /* For speed make a simple cost model: memory access is more
+ expensive than any other instruction.  */
+  enum attr_type type = get_attr_type (insn);
+
+  switch (type)
+{
+case TYPE_LOAD:
+case TYPE_STORE:
+  cost = COSTS_N_INSNS (2);
+  break;
+
+default:
+  cost = COSTS_N_INSNS (1);
+  break;
+}
+
+  return cost;
+}
+
 #undef TARGET_USE_ANCHORS_FOR_SYMBOL_P
 #define TARGET_USE_ANCHORS_FOR_SYMBOL_P arc_use_anchors_for_symbol_p
 
@@ -11807,6 +11856,9 @@ arc_can_use_return_insn (void)
 #undef TARGET_MEMORY_MOVE_COST
 #define TARGET_MEMORY_MOVE_COST arc_memory_move_cost
 
+#undef  TARGET_INSN_COST
+#define TARGET_INSN_COST arc_insn_cost
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 
 #include "gt-arc.h"
diff --git a/gcc/config/arc/arc.md b/gcc/config/arc/arc.md
index f19f2c32641..fb25aafb024 100644
--- a/gcc/config/arc/arc.md
+++ b/gcc/config/arc/arc.md
@@ -229,6 +229,10 @@
   ]
 )
 
+;; What is the insn_cost for this insn?  The target hook can still override
+;; this.  For optimizing for size the "length" attribute is used instead.
+(define_attr "cost" "" (const_int 0))
+
 (define_attr "is_sfunc" "no,yes" (const_string "no"))
 
 ;; Insn type.  Used to default other attribute values.
@@ -3140,9 +3144,9 @@ archs4x, archs4xd"
   [(set (match_operand:SI 0 "dest_reg_operand" "=q,r,r")
(plus:SI (mult:SI (match_operand:SI 1 "register_operand" "q,r,r")
  (match_operand:SI 2 "_2_4_8_operand" ""))
-(match_operand:SI 3 "nonmemory_operand" "0,r,Csz")))]
+(match_operand:SI 3 "arc_nonmemory_operand" "0,r,Csz")))]
   ""
-  "add%z2%?\\t%0,%3,%1%&"
+  "add%z2%?\\t%0,%3,%1"
   [(set_attr "type" "shift")
(set_attr "length" "*,4,8")
(set_attr "predicable" "yes,no,no")
@@ -3573,26 +3577,26 @@ archs4x, archs4xd"
 ; to truncate a symbol in a u6 immediate; but that's rather exotic, so only
 ; provide one alternatice for this, without condexec support.
 (define_insn "*ashlsi3_insn"
-  [(set (match_operand:SI 0 "dest_reg_operand"   "=Rcq,Rcqq,Rcqq,Rcw, 
w,   w")
-   (ashift:SI (match_operand:SI 1 "nonmemory_operand" "!0,Rcqq,   0,  0, 
c,cCsz")
-  (match_operand:SI 2 "nonmemory_operand"  "K,  K,RcqqM, 
cL,cL,cCal")))]
+  [(set (match_operand:SI 0 "dest_reg_operand" "=q,q, q, r, r, 
  r")
+   (a

[PATCH 4/4] arc: Don't use if-conversion when optimizing for size.

[Claudiu Zissulescu]

For ARC, predicated instructions are not very friendly with size
optimizations, leading to increased object size. Disable if-conversion
step when optimized for size.

gcc/
-xx-xx  Claudiu Zissulescu  

* common/config/arc/arc-common.c (arc_option_optimization_table):
Disable if-conversion step when optimized for size.
---
 gcc/common/config/arc/arc-common.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gcc/common/config/arc/arc-common.c 
b/gcc/common/config/arc/arc-common.c
index 0b77dd546e5..7f46f547e30 100644
--- a/gcc/common/config/arc/arc-common.c
+++ b/gcc/common/config/arc/arc-common.c
@@ -59,6 +59,7 @@ static const struct default_options 
arc_option_optimization_table[] =
 { OPT_LEVELS_SIZE, OPT_mcase_vector_pcrel, NULL, 1 },
 { OPT_LEVELS_SIZE, OPT_msize_level_, NULL, 3 },
 { OPT_LEVELS_SIZE, OPT_mmillicode, NULL, 1 },
+{ OPT_LEVELS_SIZE, OPT_fif_conversion, NULL, 0 },
 { OPT_LEVELS_1_PLUS, OPT_fomit_frame_pointer, NULL, 1 },
 { OPT_LEVELS_3_PLUS_SPEED_ONLY, OPT_msize_level_, NULL, 0 },
 { OPT_LEVELS_NONE, 0, NULL, 0 }
-- 
2.24.1

Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions

[Richard Earnshaw (lists)]

On 22/01/2020 17:45, Richard Earnshaw (lists) wrote:


[updated based on v2 discussions]

This patch proposes some new (additional) rules for email subject lines
when contributing to GCC.  The goal is to make sure that, as far as
possible, the subject for a patch will form a good summary when the
message is committed to the repository if applied with 'git am'.  Where
possible, I've tried to align these rules with those already in
use for glibc, so that the differences are minimal and only where
necessary.

Some things that differ from existing practice (at least by some people)
are:

- Use ':' rather than '[]'
   - This is more git friendly and works with 'git am'.
- Put bug numbers at the end of the line rather than the beginning.
   - The bug number is useful, but not as useful as the brief summary.
     Also, use the shortened form, as the topic part is more usefully
     conveyed in the proper topic field (see above).


I've not seen any follow-up to this version.  Should we go ahead and 
adopt this?


R.

Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions

[Alexander Monakov]

On Mon, 3 Feb 2020, Richard Earnshaw (lists) wrote:

> I've not seen any follow-up to this version.  Should we go ahead and adopt
> this?

Can we please go with 'committed' (lowercase) rather than all-caps COMMITTED?
Spelling this with all-caps seems like a recent thing on gcc-patches, before
everyone used the lowercase version, which makes more sense (no need to shout
about the thing that didn't need any discussion before applying the patch).

Also, while tools like 'git format-patch' will automatically put [PATCH] in
the subject, for '[COMMITTED]' it will be the human typing that out, and
it makes little sense to require people to meticulously type that out in caps.
Especially when the previous practice was opposite.

Thanks.
Alexander

Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions

[Richard Earnshaw (lists)]

On 03/02/2020 11:54, Alexander Monakov wrote:

On Mon, 3 Feb 2020, Richard Earnshaw (lists) wrote:


I've not seen any follow-up to this version.  Should we go ahead and adopt
this?


Can we please go with 'committed' (lowercase) rather than all-caps COMMITTED?
Spelling this with all-caps seems like a recent thing on gcc-patches, before
everyone used the lowercase version, which makes more sense (no need to shout
about the thing that didn't need any discussion before applying the patch).

Also, while tools like 'git format-patch' will automatically put [PATCH] in
the subject, for '[COMMITTED]' it will be the human typing that out, and
it makes little sense to require people to meticulously type that out in caps.
Especially when the previous practice was opposite.

Thanks.
Alexander



Upper case is what glibc has, though it appears that it's a rule that is 
not strictly followed.  If we change it, then it becomes another 
friction point between developer groups.  Personally, I'd leave it as 
is, then turn a blind eye to such minor non-conformance.


Quite frankly, then bit that matters most is what follows that, since 
that is what gets written into the git commit message.


R.

Re: Make OpenACC 'acc_get_property' with 'acc_device_current' work (was: [PATCH] Add OpenACC 2.6 `acc_get_property' support)

[Harwath, Frederik]

Hi Thomas,

On 30.01.20 16:54, Thomas Schwinge wrote:
> 
> [...] the 'acc_device_current' interface should work already now.
> 
> [...] Please review
> the attached (Tobias the Fortran test cases, please), and test with AMD
> GCN offloading.  If approving this patch, please respond with

I have tested the patch with AMD GCN offloading and I have observed no 
regressions.
The new tests pass as expected and print the correct output.
Great that you have extended the Fortran tests!

> diff --git a/libgomp/oacc-init.c b/libgomp/oacc-init.c
> index ef12b4c16d01..c28c0f689ba2 100644
> --- a/libgomp/oacc-init.c
> +++ b/libgomp/oacc-init.c
> @@ -796,7 +796,9 @@ get_property_any (int ord, acc_device_t d, 
> acc_device_property_t prop)
> size_t
> acc_get_property (int ord, acc_device_t d, acc_device_property_t prop)
> {
> -  if (!known_device_type_p (d))
> +  if (d == acc_device_current)
> +; /* Allowed only for 'acc_get_property', 'acc_get_property_string'.  */
> +  else if (!known_device_type_p (d))
> unknown_device_type_error(d);

I don't like the empty if branch very much. Introducing a variable
(for instance, "bool allowed_device_type = acc_device_current
|| known_device_type(d);") would also provide a place for your comment.
You could also extract a function to avoid duplicating the explanation
in acc_get_property_string.

The patch looks good to me.

Reviewed-by: Frederik Harwath  

Best regards,
Frederik

Re: [PATCH] Prevent IPA-SRA from creating calls to local comdats (PR 92676)

[Martin Jambor]

Hello Honza,

ping.

Thanks,

Martin


On Mon, Dec 16 2019, Martin Jambor wrote:
> Hi,
>
> since r278669 (fix for PR ipa/91956), IPA-SRA makes sure that the clone
> it creates is put into the same same_comdat as the original cgraph_node,
> so that it can call private comdats (such as the ipa-split bits of a
> comdat that is private).
>
> However, that means that if there is non-comdat caller of a public
> comdat that is modified by IPA-SRA, it now finds itself calling a
> private comdat, which call graph verifier does not like (and for a
> reason, in theory it can disappear and since it is private it would not
> be available from other CUs).
>
> The patch fixes this by performing the fix for PR 91956 only when the
> node in question actually calls a local comdat and when it does, also
> making sure that no callers come from a different same_comdat (disabling
> IPA-SRA if both conditions are true), so that it plays by the rules in
> both modes, does not violate the private comdat calling rule and at the
> same time does not disable the transformation unnecessarily.
>
> The patch also fixes up the calls_comdat_local of callers of the
> modified node, despite that not triggering any known issues.  It has
> passed LTO-bootstrap and testing.  What do you think?
>
> Thanks,
>
> Martin
>
>
> 2019-12-16  Martin Jambor  
>
>   PR ipa/92676
>   * ipa-sra.c (struct caller_issues): New fields candidate and
>   call_from_outside_comdat.
>   (check_for_caller_issues): Check for calls from outsied of
>   candidate's same_comdat_group.
>   (check_all_callers_for_issues): Set up issues.candidate, check result
>   of the new check.
>   (process_isra_node_results): Set calls_comdat_local of callers if
>   appropriate.
> ---
>  gcc/ipa-sra.c | 27 +--
>  1 file changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/ipa-sra.c b/gcc/ipa-sra.c
> index 421c0899e11..8612c8fc920 100644
> --- a/gcc/ipa-sra.c
> +++ b/gcc/ipa-sra.c
> @@ -2895,10 +2895,14 @@ ipa_sra_ipa_function_checks (cgraph_node *node)
>  
>  struct caller_issues
>  {
> +  /* The candidate being considered.  */
> +  cgraph_node *candidate;
>/* There is a thunk among callers.  */
>bool thunk;
>/* Call site with no available information.  */
>bool unknown_callsite;
> +  /* Call from outside the the candidate's comdat group.  */
> +  bool call_from_outside_comdat;
>/* There is a bit-aligned load into one of non-gimple-typed arguments. */
>bool bit_aligned_aggregate_argument;
>  };
> @@ -2920,6 +2924,13 @@ check_for_caller_issues (struct cgraph_node *node, 
> void *data)
>thunks.  */
> return true;
>   }
> +  if (issues->candidate->calls_comdat_local
> +   && issues->candidate->same_comdat_group
> +   && !issues->candidate->in_same_comdat_group_p (cs->caller))
> + {
> +   issues->call_from_outside_comdat = true;
> +   return true;
> + }
>  
>isra_call_summary *csum = call_sums->get (cs);
>if (!csum)
> @@ -2942,6 +2953,7 @@ check_all_callers_for_issues (cgraph_node *node)
>  {
>struct caller_issues issues;
>memset (&issues, 0, sizeof (issues));
> +  issues.candidate = node;
>  
>node->call_for_symbol_and_aliases (check_for_caller_issues, &issues, true);
>if (issues.unknown_callsite)
> @@ -2960,6 +2972,13 @@ check_all_callers_for_issues (cgraph_node *node)
>node->dump_name ());
>return true;
>  }
> +  if (issues.call_from_outside_comdat)
> +{
> +  if (dump_file)
> + fprintf (dump_file, "Function would become private comdat called "
> +  "outside of its comdat group.\n");
> +  return true;
> +}
>  
>if (issues.bit_aligned_aggregate_argument)
>  {
> @@ -3759,8 +3778,12 @@ process_isra_node_results (cgraph_node *node,
>  = node->create_virtual_clone (callers, NULL, new_adjustments, "isra",
> suffix_counter);
>suffix_counter++;
> -  if (node->same_comdat_group)
> -new_node->add_to_same_comdat_group (node);
> +  if (node->calls_comdat_local && node->same_comdat_group)
> +{
> +  new_node->add_to_same_comdat_group (node);
> +  for (cgraph_edge *cs = new_node->callers; cs; cs = cs->next_caller)
> + cs->caller->calls_comdat_local = true;
> +}
>new_node->calls_comdat_local = node->calls_comdat_local;
>  
>if (dump_file)
> -- 
> 2.24.0

Re: [PATCH, v4] coroutines: Fix ICE on invalid (PR93458).

[Nathan Sidwell]

On 2/1/20 6:55 AM, Iain Sandoe wrote:

Nathan Sidwell  wrote:


On 1/30/20 9:43 AM, Iain Sandoe wrote:

Hi Nathan,



however. ….

also, what if you find something, but it's not a type template?

… I’ve switched the complain off on lookup_qualified_name and now check for
a type template.


I'm not sure that's helpful.  I think you should still complain on the lookup.  
If that returns error_mark_node, you're done.  If it returns NULL, does it emit 
an error?  If not, you should emit a not found error.  Finally, if it does 
return a thing, check if it's a class template decl (or alias template I guess 
--  DECL_TEMPLATE_RESULT being a TYPE_DECL is close enough) and if not, error 
on that (X is not a template class).  Then the user's fully clued in.


I took the liberty of repeating this treatment in the coroutine handle lookup 
code
(new testcases attached).
so the errors now look like:
"cannot find a valid coroutine traits template using 'std::coroutine_traits’”


hm, 'valid'.  If you find a template_decl, but cannot instantiate it, that 
sounds not valid.  But I suspect you do not diagnose that here, because in 
general you cannot :)


OK. So in my mind there are two cases here:

1. The user doesn’t know to, or forgot to, add  (or can’t spell it).

- this would trigger the top level error and mean that nothing much is going to 
work.

2.

2a The user has a corrupted installation and the  header is broken.
2b The user has been more creative and tried to invent their own header.

- this might result in many more unexpected pathways leading to error.

———

I have revised the code to:

a) re-enable “complain” on the lookup_qualified_name (providing no error for 
missing traits has been emitted).
b) I added an inform “perhaps '#include ' is missing (I couldn’t get 
the rich location missing include logic to fire here).
c) the diagnostic now will consist of:
- any emitted by “lookup_qualified_name()"  (usually, there will be none, 
since we are supplying std namespace as the scope).
- "coroutines require a traits template; cannot find 
‘std::coroutine_traits’”
- "note: perhaps ‘#include ’ is missing”

I think that deals reasonably with the common case (1) without being too 
annoying in terms of diagnostic verbosity.

it leaves  “lookup_qualified_name()" reported errors to deal with the less 
common and more exotic possibilities in (2).

The handle is treated the same way (without the  note, since we must 
have already found or made a usable traits).  In any case, I think bad handle fails 
only really occur under (2) above.

still open to rewording of error messages, of course,

OK / reword?


thanks, looks good now.


nathan


--
Nathan Sidwell

Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions

[Alexander Monakov]

On Mon, 3 Feb 2020, Richard Earnshaw (lists) wrote:

> Upper case is what glibc has, though it appears that it's a rule that is not
> strictly followed.  If we change it, then it becomes another friction point
> between developer groups.  Personally, I'd leave it as is, then turn a blind
> eye to such minor non-conformance.

In that case can we simply say that both 'committed' and 'COMMITTED' are okay,
if we know glibc doesn't follow that rule and anticipate we will not follow
it either?

Thanks.
Alexander

Re: [PATCH Coroutines v1] Handle type deduction of auto and decltype(auto) with indirect_ref expression

[Nathan Sidwell]

On 2/2/20 9:28 PM, JunMa wrote:

在 2020/2/3 上午9:03, JunMa 写道:



I think all you want here is:
  await_expr = convert_from_reference (await_expr);


Thanks, I'll update it.





Regards
JunMa

Hi nathan,
Here is the update.



   /* This will produce the value (if one is provided) from the co_await
  expression.  */
   tree resume_call = TREE_VEC_ELT (awaiter_calls, 2); /* await_resume().  */
+  if (TREE_CODE (resume_call) == INDIRECT_REF)
+/* Sink to await_resume call_expr.  */
+resume_call = TREE_OPERAND (resume_call, 0);


sorry, I should have realized you still needed this bit.  This too is 
stripping a reference access, right?  I.e. TYPE_REF_P (TREE_TYPE 
(resume_call)) is true. You should that as the condition too.


the other part of the patch is fine

nathan

--
Nathan Sidwell

Re: [PATCH] simplify-rtx: Extend (truncate (*extract ...)) fold [PR87763]

[Richard Sandiford]

Maxim Kuvyrkov  writes:
> Hi Richard,
>
> You patch below increases code-size on aarch64-linux-gnu with -Os on SPEC2k6 
> 400.perlbench and 453.povray -- by 1% and 2% respectively.
>
> 400.perlbench,perlbench_base.default, 101,939261,951221
> 453.povray,povray_base.default,   102,707807,721399
>
> Would you please check whether these can be avoided?  [Let me know if you 
> need help reproducing this problem.]

I reverted the patch on Wednesday, see:
https://gcc.gnu.org/ml/gcc-patches/2020-01/msg01962.html

Thanks,
Richard

>
> Thank you,
>
> --
> Maxim Kuvyrkov
> https://www.linaro.org
>
>> On Jan 27, 2020, at 7:41 PM, Richard Sandiford  
>> wrote:
>> 
>> In the gcc.target/aarch64/lsl_asr_sbfiz.c part of this PR, we have:
>> 
>> Failed to match this instruction:
>> (set (reg:SI 95)
>>(ashift:SI (subreg:SI (sign_extract:DI (subreg:DI (reg:SI 97) 0)
>>(const_int 3 [0x3])
>>(const_int 0 [0])) 0)
>>(const_int 19 [0x13])))
>> 
>> If we perform the natural simplification to:
>> 
>> (set (reg:SI 95)
>>(ashift:SI (sign_extract:SI (reg:SI 97)
>>(const_int 3 [0x3])
>>(const_int 0 [0])) 0)
>>(const_int 19 [0x13])))
>> 
>> then the pattern matches.  And it turns out that we do have a
>> simplification like that already, but it would only kick in for
>> extractions from a reg, not a subreg.  E.g.:
>> 
>> (set (reg:SI 95)
>>(ashift:SI (subreg:SI (sign_extract:DI (reg:DI X)
>>(const_int 3 [0x3])
>>(const_int 0 [0])) 0)
>>(const_int 19 [0x13])))
>> 
>> would simplify to:
>> 
>> (set (reg:SI 95)
>>(ashift:SI (sign_extract:SI (subreg:SI (reg:DI X) 0)
>>(const_int 3 [0x3])
>>(const_int 0 [0])) 0)
>>(const_int 19 [0x13])))
>> 
>> IMO the subreg case is even more obviously a simplification
>> than the bare reg case, since the net effect is to remove
>> either one or two subregs, rather than simply change the
>> position of a subreg/truncation.
>> 
>> However, doing that regressed gcc.dg/tree-ssa/pr64910-2.c
>> for -m32 on x86_64-linux-gnu, because we could then simplify
>> a :HI zero_extract to a :QI one.  The associated *testqi_ext_3
>> pattern did already seem to want to handle QImode extractions:
>> 
>>  "ix86_match_ccmode (insn, CCNOmode)
>>   && ((TARGET_64BIT && GET_MODE (operands[2]) == DImode)
>>   || GET_MODE (operands[2]) == SImode
>>   || GET_MODE (operands[2]) == HImode
>>   || GET_MODE (operands[2]) == QImode)
>> 
>> but I'm not sure how often the QI case would trigger in practice,
>> since the zero_extract mode was restricted to HI and above.  I checked
>> the other x86 patterns and couldn't see any other instances of this.
>> 
>> Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linux-gnu,
>> OK to install?
>> 
>> Richard
>> 
>> 
>> 2020-01-27  Richard Sandiford  
>> 
>> gcc/
>>  PR rtl-optimization/87763
>>  * simplify-rtx.c (simplify_truncation): Extend sign/zero_extract
>>  simplification to handle subregs as well as bare regs.
>>  * config/i386/i386.md (*testqi_ext_3): Match QI extracts too.
>> ---
>> gcc/config/i386/i386.md | 2 +-
>> gcc/simplify-rtx.c  | 4 +++-
>> 2 files changed, 4 insertions(+), 2 deletions(-)
>> 
>> diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
>> index 6e9c9bd2fb6..a125ab350bb 100644
>> --- a/gcc/config/i386/i386.md
>> +++ b/gcc/config/i386/i386.md
>> @@ -8927,7 +8927,7 @@ (define_insn "*testqi_ext_2"
>> (define_insn_and_split "*testqi_ext_3"
>>   [(set (match_operand 0 "flags_reg_operand")
>> (match_operator 1 "compare_operator"
>> -  [(zero_extract:SWI248
>> +  [(zero_extract:SWI
>>   (match_operand 2 "nonimmediate_operand" "rm")
>>   (match_operand 3 "const_int_operand" "n")
>>   (match_operand 4 "const_int_operand" "n"))
>> diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
>> index eff1d07a253..db4f9339c15 100644
>> --- a/gcc/simplify-rtx.c
>> +++ b/gcc/simplify-rtx.c
>> @@ -736,7 +736,9 @@ simplify_truncation (machine_mode mode, rtx op,
>>  (*_extract:M1 (truncate:M1 (reg:M2)) (len) (pos')) if possible without
>>  changing len.  */
>>   if ((GET_CODE (op) == ZERO_EXTRACT || GET_CODE (op) == SIGN_EXTRACT)
>> -  && REG_P (XEXP (op, 0))
>> +  && (REG_P (XEXP (op, 0))
>> +  || (SUBREG_P (XEXP (op, 0))
>> +  && REG_P (SUBREG_REG (XEXP (op, 0)
>>   && GET_MODE (XEXP (op, 0)) == GET_MODE (op)
>>   && CONST_INT_P (XEXP (op, 1))
>>   && CONST_INT_P (XEXP (op, 2)))
>> -- 
>> 2.17.1
>> 

Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions

[Segher Boessenkool]

On Mon, Feb 03, 2020 at 02:54:05PM +0300, Alexander Monakov wrote:
> On Mon, 3 Feb 2020, Richard Earnshaw (lists) wrote:
> 
> > I've not seen any follow-up to this version.  Should we go ahead and adopt
> > this?
> 
> Can we please go with 'committed' (lowercase) rather than all-caps COMMITTED?
> Spelling this with all-caps seems like a recent thing on gcc-patches, before
> everyone used the lowercase version, which makes more sense (no need to shout
> about the thing that didn't need any discussion before applying the patch).

Lower case certainly makes more sense.

None of this are *rules*.  We should not pretend they are.  An email
subject should be useful to what the receivers of that email use it for:
see if it very interesting to them, see if it probably not interesting
to them: that's the "smth: " at the start, and the PR number at the end,
and of course the actual subject itself, so we should not put in too
much fluff in the subject, there needs to be room left (in the less than
fifty chars total) for an actual subject :-)

(The example in the patch does not capitalise the subject line, btw.
It should.)


Segher

Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions

[Richard Earnshaw (lists)]

On 03/02/2020 13:54, Segher Boessenkool wrote:

On Mon, Feb 03, 2020 at 02:54:05PM +0300, Alexander Monakov wrote:

On Mon, 3 Feb 2020, Richard Earnshaw (lists) wrote:


I've not seen any follow-up to this version.  Should we go ahead and adopt
this?


Can we please go with 'committed' (lowercase) rather than all-caps COMMITTED?
Spelling this with all-caps seems like a recent thing on gcc-patches, before
everyone used the lowercase version, which makes more sense (no need to shout
about the thing that didn't need any discussion before applying the patch).


Lower case certainly makes more sense.

None of this are *rules*.  We should not pretend they are.  An email
subject should be useful to what the receivers of that email use it for:
see if it very interesting to them, see if it probably not interesting
to them: that's the "smth: " at the start, and the PR number at the end,
and of course the actual subject itself, so we should not put in too
much fluff in the subject, there needs to be room left (in the less than
fifty chars total) for an actual subject :-)

(The example in the patch does not capitalise the subject line, btw.
It should.)


Segher




Where does your '50 chars' limit come from?  It's not in the glibc text, 
and it's not in the linux kernel text either.  AFAICT this is your 
invention and you seem to be the only person proposing it.


I think the linux rule (the whole line, not including the parts that are 
removed on commit, should not exceed 75 characters) is far more sensible 
- which is why this draft states this.


R.

Re: [PATCH] Add --with-diagnostics-urls configuration option and GCC_URLS/TERM_URLS env var

[Segher Boessenkool]

On Sun, Feb 02, 2020 at 08:00:44AM +, Bernd Edlinger wrote:
> >> Okay, thanks.  That is a strong indication that there is no need
> >> to interfere with screen, which proves that any auto-disabling should
> >> have a very specific terminal detection logic.
> > 
> > Jakub says that he tested with a recent gnome-terminal.  That works, of
> > course.  Mnay other terminals will not, and switching what terminal is
> > attached to your screen session will not work well either, as far as I
> > can tell.
> 
> I understood his statement, that the URLs are stripped from the data
> stream by screen and are no longer visible, even if the terminal would
> support them i.e. you connect from a gnome-terminal.

It looks like tmux strips it as well.  But in my earlier testing it did
not?  Confused.  Maybe this was the auto-detect thing, dunno.  I guess
I'll test with more tmux versions when I find some more time for this.

> But work fine when the compiler runs natively in a gnome-terminal.

It is big garbage for me, both with bell (which is much worse on some
other terminals), and with the string terminator escape (ESC \).


Segher

Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions

[Jason Merrill]

On Mon, Feb 3, 2020 at 7:57 AM Alexander Monakov  wrote:

> On Mon, 3 Feb 2020, Richard Earnshaw (lists) wrote:
>
> > Upper case is what glibc has, though it appears that it's a rule that is
> not
> > strictly followed.  If we change it, then it becomes another friction
> point
> > between developer groups.  Personally, I'd leave it as is, then turn a
> blind
> > eye to such minor non-conformance.
>
> In that case can we simply say that both 'committed' and 'COMMITTED' are
> okay,
> if we know glibc doesn't follow that rule and anticipate we will not follow
> it either?
>

And perhaps something shorter?  "committed" is a long word.  [PUSHED]?

Jason

Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions

[Jonathan Wakely]

On Mon, 3 Feb 2020 at 14:00, Richard Earnshaw (lists) wrote:
> Where does your '50 chars' limit come from?  It's not in the glibc text,
> and it's not in the linux kernel text either.  AFAICT this is your
> invention and you seem to be the only person proposing it.

It's a fairly well established convention, e.g.
https://chris.beams.io/posts/git-commit/ and it's what Github suggests
(and whines if you go past it).

> I think the linux rule (the whole line, not including the parts that are
> removed on commit, should not exceed 75 characters) is far more sensible
> - which is why this draft states this.

I'm OK with that.

[COMMITTED] c++: Fix cast to pointer to VLA.

[Jason Merrill]

The C front-end fixed this issue in r257620 by adding a DECL_EXPR from
grokdeclarator.  We don't have an easy way to do that in the C++ front-end,
but it works fine to create and prepend a DECL_EXPR when we are genericizing
the NOP_EXPR for the cast.

The C patch wraps the DECL_EXPR in a BIND_EXPR, but that seems unnecessary
in C++; this is just a hook to run gimplify_type_sizes, we aren't actually
declaring anything that we need to worry about scoping for.

Tested x86_64-pc-linux-gnu, applying to trunk.

PR c++/88256
* cp-gimplify.c (predeclare_vla): New.
(cp_genericize_r) [NOP_EXPR]: Call it.
---
 gcc/cp/cp-gimplify.c  | 31 +++
 .../compile => c-c++-common}/pr84305.c|  2 ++
 2 files changed, 33 insertions(+)
 rename gcc/testsuite/{gcc.c-torture/compile => c-c++-common}/pr84305.c (78%)

diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index 4fb3a1a8b8a..10ab99515e6 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -1188,6 +1188,36 @@ static tree genericize_spaceship (tree expr)
   return genericize_spaceship (type, op0, op1);
 }
 
+/* If EXPR involves an anonymous VLA type, prepend a DECL_EXPR for that type
+   to trigger gimplify_type_sizes; otherwise a cast to pointer-to-VLA confuses
+   the middle-end (c++/88256).  */
+
+static tree
+predeclare_vla (tree expr)
+{
+  tree type = TREE_TYPE (expr);
+  if (type == error_mark_node)
+return expr;
+
+  /* We need to strip pointers for gimplify_type_sizes.  */
+  tree vla = type;
+  while (POINTER_TYPE_P (vla))
+{
+  if (TYPE_NAME (vla))
+   return expr;
+  vla = TREE_TYPE (vla);
+}
+  if (TYPE_NAME (vla) || !variably_modified_type_p (vla, NULL_TREE))
+return expr;
+
+  tree decl = build_decl (input_location, TYPE_DECL, NULL_TREE, vla);
+  DECL_ARTIFICIAL (decl) = 1;
+  TYPE_NAME (vla) = decl;
+  tree dexp = build_stmt (input_location, DECL_EXPR, decl);
+  expr = build2 (COMPOUND_EXPR, type, dexp, expr);
+  return expr;
+}
+
 /* Perform any pre-gimplification lowering of C++ front end trees to
GENERIC.  */
 
@@ -1648,6 +1678,7 @@ cp_genericize_r (tree *stmt_p, int *walk_subtrees, void 
*data)
   break;
 
 case NOP_EXPR:
+  *stmt_p = predeclare_vla (*stmt_p);
   if (!wtd->no_sanitize_p
  && sanitize_flags_p (SANITIZE_NULL | SANITIZE_ALIGNMENT)
  && TYPE_REF_P (TREE_TYPE (stmt)))
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr84305.c 
b/gcc/testsuite/c-c++-common/pr84305.c
similarity index 78%
rename from gcc/testsuite/gcc.c-torture/compile/pr84305.c
rename to gcc/testsuite/c-c++-common/pr84305.c
index 374fa67f593..27150dd5328 100644
--- a/gcc/testsuite/gcc.c-torture/compile/pr84305.c
+++ b/gcc/testsuite/c-c++-common/pr84305.c
@@ -1,3 +1,5 @@
+// { dg-additional-options -O3 }
+
 int res, a, b;
 void *foo;
 static void f2 (int arg) { res = ((int (*)[arg][b]) foo)[0][0][0]; }

base-commit: 44f77a6dea2f312ee1743f3dde465c1b8453ee13
-- 
2.18.1

Re: Make OpenACC 'acc_get_property' with 'acc_device_current' work

[Tobias Burnus]

Hi Thomas,

On 1/30/20 4:54 PM, Thomas Schwinge wrote:


That's still pending.  Recently, 
 "Missing definition
for acc_device_current" got filed; let's (also/first) watch/wait what
comes out of that.


(Still pending.)


Please review the attached (Tobias the Fortran test cases, please),
and test with AMD GCN offloading.  If approving this patch, please respond with
"Reviewed-by: NAME " so that your effort will be recorded in the
commit log, see.


LGTM with the minor nit regarding the integer kind used
for dev_type (default/unspecified vs. "acc_device_kind").
To make you happy:

Reviewed-by: Tobias Burnus 

Thanks,

Tobias


--- a/libgomp/openacc.f90
+++ b/libgomp/openacc.f90
@@ -766,6 +766,7 @@ module openacc
  
! From openacc_kinds

public :: acc_device_kind
+  public :: acc_device_current


Good catch!


+++ b/libgomp/testsuite/libgomp.oacc-fortran/acc_get_property-aux.f90
…
+  subroutine expect_device_memory_properties (dev_type, dev_num, &
+   expected_total_memory)
+integer, intent(in) :: dev_type


I think you should use (w/ or w/o "value" attribute)
   integer (acc_device_kind) :: dev_type
instead for consistency. It does not matter in practice but is nicer.


+   expected_vendor, expected_name, expected_driver)
+integer, intent(in) :: dev_type
+integer, intent(in) :: dev_num
+character*(*), intent(in) :: expected_vendor


Likewise for dev_num.

(Side note: I personally prefer "character(*)" or (even better:) 
"character(len=*)"
to the "character*…" syntax – as I find it more readable. But that your syntax 
is
not even marked as obsolescent and, hence, is perfectly valid.)


+integer, intent(in) :: dev_num


Ditto.

[PATCH][AArch64] Improve popcount expansion

[Wilco Dijkstra]

The popcount expansion uses umov to extend the result and move it back
to the integer register file.  If we model ADDV as a zero-extending
operation, fmov can be used to move back to the integer side. This
results in a ~0.5% speedup on deepsjeng on Cortex-A57.

A typical __builtin_popcount expansion is now:

fmovs0, w0
cnt v0.8b, v0.8b
addvb0, v0.8b
fmovw0, s0

Bootstrap OK, passes regress.

ChangeLog
2020-02-02  Wilco Dijkstra  

gcc/
* config/aarch64/aarch64.md (popcount2): Improve expansion.
* config/aarch64/aarch64-simd.md
(aarch64_zero_extend_reduc_plus_): New pattern.
* config/aarch64/iterators.md (VDQV_E): New iterator.
testsuite/
* gcc.target/aarch64/popcnt2.c: New test.

--
diff --git a/gcc/config/aarch64/aarch64-simd.md 
b/gcc/config/aarch64/aarch64-simd.md
index 
97f46f96968a6bc2f93bbc812931537b819b3b19..34765ff43c1a090a31e2aed64ce95510317ab8c3
 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -2460,6 +2460,17 @@ (define_insn "aarch64_reduc_plus_internal"
   [(set_attr "type" "neon_reduc_add")]
 )
 
+;; ADDV with result zero-extended to SI/DImode (for popcount).
+(define_insn "aarch64_zero_extend_reduc_plus_"
+ [(set (match_operand:GPI 0 "register_operand" "=w")
+   (zero_extend:GPI
+   (unspec: [(match_operand:VDQV_E 1 "register_operand" "w")]
+UNSPEC_ADDV)))]
+ "TARGET_SIMD"
+ "add\\t%0, %1."
+  [(set_attr "type" "neon_reduc_add")]
+)
+
 (define_insn "aarch64_reduc_plus_internalv2si"
  [(set (match_operand:V2SI 0 "register_operand" "=w")
(unspec:V2SI [(match_operand:V2SI 1 "register_operand" "w")]
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 
86c2cdfc7973f4b964ba233cfbbe369b24e0ac10..5edc76ee14b55b2b4323530e10bd22b3ffca483e
 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -4829,7 +4829,6 @@ (define_expand "popcount2"
 {
   rtx v = gen_reg_rtx (V8QImode);
   rtx v1 = gen_reg_rtx (V8QImode);
-  rtx r = gen_reg_rtx (QImode);
   rtx in = operands[1];
   rtx out = operands[0];
   if(mode == SImode)
@@ -4843,8 +4842,7 @@ (define_expand "popcount2"
 }
   emit_move_insn (v, gen_lowpart (V8QImode, in));
   emit_insn (gen_popcountv8qi2 (v1, v));
-  emit_insn (gen_reduc_plus_scal_v8qi (r, v1));
-  emit_insn (gen_zero_extendqi2 (out, r));
+  emit_insn (gen_aarch64_zero_extend_reduc_plus_v8qi (out, v1));
   DONE;
 })
 
diff --git a/gcc/config/aarch64/iterators.md b/gcc/config/aarch64/iterators.md
index 
fc973086cb91ae0dc54eeeb0b832d522539d7982..926779bf2442fa60d184ef17308f91996d6e8d1b
 100644
--- a/gcc/config/aarch64/iterators.md
+++ b/gcc/config/aarch64/iterators.md
@@ -208,6 +208,9 @@ (define_mode_iterator VDQV [V8QI V16QI V4HI V8HI V4SI V2DI])
 ;; Advanced SIMD modes (except V2DI) for Integer reduction across lanes.
 (define_mode_iterator VDQV_S [V8QI V16QI V4HI V8HI V4SI])
 
+;; Advanced SIMD modes for Integer reduction across lanes (zero/sign extended).
+(define_mode_iterator VDQV_E [V8QI V16QI V4HI V8HI])
+
 ;; All double integer narrow-able modes.
 (define_mode_iterator VDN [V4HI V2SI DI])
 
diff --git a/gcc/testsuite/gcc.target/aarch64/popcnt2.c 
b/gcc/testsuite/gcc.target/aarch64/popcnt2.c
new file mode 100644
index 
..e321858afa4d6ecb6fc7348f39f6e5c6c0c46147
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/popcnt2.c
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+unsigned
+foo (int x)
+{
+  return __builtin_popcount (x);
+}
+
+unsigned long
+foo1 (int x)
+{
+  return __builtin_popcount (x);
+}
+
+/* { dg-final { scan-assembler-not {popcount} } } */
+/* { dg-final { scan-assembler-times {cnt\t} 2 } } */
+/* { dg-final { scan-assembler-times {fmov} 4 } } */
+/* { dg-final { scan-assembler-not {umov} } } */
+/* { dg-final { scan-assembler-not {uxtw} } } */
+/* { dg-final { scan-assembler-not {sxtw} } } */

Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions

[Richard Earnshaw (lists)]

On 03/02/2020 14:13, Jonathan Wakely wrote:

On Mon, 3 Feb 2020 at 14:00, Richard Earnshaw (lists) wrote:

Where does your '50 chars' limit come from?  It's not in the glibc text,
and it's not in the linux kernel text either.  AFAICT this is your
invention and you seem to be the only person proposing it.


It's a fairly well established convention, e.g.
https://chris.beams.io/posts/git-commit/ and it's what Github suggests
(and whines if you go past it).



That suggest it as a limit for everything.  If you have a tag and a bug 
number then then that would leave very little for the real part of the 
summary and would likely lead to something meaningless or 
incomprehensible in the remaining characters.  That might be OK for 
small projects, but for something the size of gcc, I think keeping the 
extra flexibility is useful.



I think the linux rule (the whole line, not including the parts that are
removed on commit, should not exceed 75 characters) is far more sensible
- which is why this draft states this.


I'm OK with that.



OK, thanks.

R.

Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions

[Richard Earnshaw (lists)]

On 03/02/2020 14:10, Jason Merrill wrote:

On Mon, Feb 3, 2020 at 7:57 AM Alexander Monakov  wrote:


On Mon, 3 Feb 2020, Richard Earnshaw (lists) wrote:


Upper case is what glibc has, though it appears that it's a rule that is

not

strictly followed.  If we change it, then it becomes another friction

point

between developer groups.  Personally, I'd leave it as is, then turn a

blind

eye to such minor non-conformance.


In that case can we simply say that both 'committed' and 'COMMITTED' are
okay,
if we know glibc doesn't follow that rule and anticipate we will not follow
it either?



And perhaps something shorter?  "committed" is a long word.  [PUSHED]?

Jason



"Committed" is the accepted term in glibc.  If we insist on something 
different, then that becomes a friction point.


On the other hand, I really don't care personally as long as the meaning 
is clear.  I'd be happy with the super-set of all of these, since it 
only appears in email messages, not the final commit.


R.

Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions

[Richard Earnshaw (lists)]

On 03/02/2020 15:13, Richard Earnshaw (lists) wrote:

On 03/02/2020 14:10, Jason Merrill wrote:
On Mon, Feb 3, 2020 at 7:57 AM Alexander Monakov  
wrote:



On Mon, 3 Feb 2020, Richard Earnshaw (lists) wrote:

Upper case is what glibc has, though it appears that it's a rule 
that is

not

strictly followed.  If we change it, then it becomes another friction

point

between developer groups.  Personally, I'd leave it as is, then turn a

blind

eye to such minor non-conformance.


In that case can we simply say that both 'committed' and 'COMMITTED' are
okay,
if we know glibc doesn't follow that rule and anticipate we will not 
follow

it either?



And perhaps something shorter?  "committed" is a long word.  [PUSHED]?

Jason



"Committed" is the accepted term in glibc.  If we insist on something 
different, then that becomes a friction point.




"COMMITTED", not "Committed".

On the other hand, I really don't care personally as long as the meaning 
is clear.  I'd be happy with the super-set of all of these, since it 
only appears in email messages, not the final commit.


R.

Re: [wwwdocs] Updates to contribute.html for git-friendly posting rules

[Segher Boessenkool]

(Old thread, first time I see it though):

On Mon, Jan 20, 2020 at 11:45:28AM +, Richard Earnshaw (lists) wrote:
> The more we make the process lightweight for contributors, the more work 
> we make for maintainers.  If the contribution is sent correctly, then 
> ideally, the patch can be applied with just 'git am' by the maintainer. 

Almost always the maintainer will want to do "git am" followed by
"git commit --amend" *anyway*.  It helps if people follow the same
format often, but making this a stringent requirement, even before we
have experience with it, is just wrong, and does not help anyway.


Segher

Re: [PATCH] correct COUNT and PROB for unrolled loop

[Pat Haugen]

On 2/3/20 2:17 AM, Jiufu Guo wrote:
> +/* { dg-final { scan-rtl-dump-times "REG_BR_PROB 937042044" 1 
> "loop2_unroll"} } */

Sorry I didn't catch this addition to the original testcase earlier, but I 
wonder how stable this test is going to be. If there are future changes to 
default count/probability, or changes in their representation, this may fail 
and need to be updated. The fact that the loop is still getting aligned is the 
main concern.

-Pat

Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions

[Andrew Clayton]

On Mon, 3 Feb 2020 15:04:57 +
"Richard Earnshaw (lists)"  wrote:

> On 03/02/2020 14:13, Jonathan Wakely wrote:
> > On Mon, 3 Feb 2020 at 14:00, Richard Earnshaw (lists) wrote:  
> >> Where does your '50 chars' limit come from?  It's not in the glibc text,
> >> and it's not in the linux kernel text either.  AFAICT this is your
> >> invention and you seem to be the only person proposing it.  
> > 
> > It's a fairly well established convention, e.g.
> > https://chris.beams.io/posts/git-commit/ and it's what Github suggests
> > (and whines if you go past it).
> >   
> 
> That suggest it as a limit for everything.  If you have a tag and a bug 
> number then then that would leave very little for the real part of the 
> summary and would likely lead to something meaningless or 
> incomprehensible in the remaining characters.  That might be OK for 
> small projects, but for something the size of gcc, I think keeping the 
> extra flexibility is useful.

Hopefully I've been following this discussion properly.

It's important to clarify that there are two *limits* in play here.

The subject should generally be kept to 50 chars or less, This is
because the subject is displayed in the output of commands like
'git log --oneline' and 'git shortlog' which indents more than git log.

Then the message body should wrap after 72-75 chars, again as the
output of the likes of 'git log' indent the message by 4 chars.

Using vim as the git commit message editor, vim will show you when your
subject line is over 50 chars and it wraps the message body after 72
chars.

Cheers,
Andrew

Re: [PATCH] correct COUNT and PROB for unrolled loop

[Jeff Law]

On Mon, 2020-02-03 at 10:04 -0600, Pat Haugen wrote:
> On 2/3/20 2:17 AM, Jiufu Guo wrote:
> > +/* { dg-final { scan-rtl-dump-times "REG_BR_PROB 937042044" 1 
> > "loop2_unroll"} } */
> 
> Sorry I didn't catch this addition to the original testcase earlier, but I 
> wonder how stable this test is going to be. If there are future changes to 
> default count/probability, or changes in their representation, this may fail 
> and need to be updated. The fact that the loop is still getting aligned is 
> the main concern.
Unless you're really interested in those probabilities, I'd suggest not
testing for them.  If you really need to test for them, then I'd
suggest testing for them being "close" rather than a specific value for
REG_BR_PROB.

jeff

Re: [PATCH] correct COUNT and PROB for unrolled loop

[Jan Hubicka]

> On Mon, 2020-02-03 at 10:04 -0600, Pat Haugen wrote:
> > On 2/3/20 2:17 AM, Jiufu Guo wrote:
> > > +/* { dg-final { scan-rtl-dump-times "REG_BR_PROB 937042044" 1 
> > > "loop2_unroll"} } */
> > 
> > Sorry I didn't catch this addition to the original testcase earlier, but I 
> > wonder how stable this test is going to be. If there are future changes to 
> > default count/probability, or changes in their representation, this may 
> > fail and need to be updated. The fact that the loop is still getting 
> > aligned is the main concern.
> Unless you're really interested in those probabilities, I'd suggest not
> testing for them.  If you really need to test for them, then I'd
> suggest testing for them being "close" rather than a specific value for
> REG_BR_PROB.

Note that REG_BR_PROB now encodes the actual probability as well as the
profile quality (i.e. it is m_val * 8 + m_quality).
We may want to invent better way to dump them, but it is better to match
for CFG edge probability rather than the REG_BR_PROB_NOTE.

honza
> 
> jeff
> 

Re: [PATCH 2/4] [ARC] Use TARGET_INSN_COST.

[Jeff Law]

On Mon, 2020-02-03 at 12:38 +0100, Claudiu Zissulescu wrote:
> TARGET_INSN_COST gives us a better control over the instruction costs
> than classical RTX_COSTS.  A simple cost scheme is in place for the
> time being, when optimizing for size, the cost is given by the
> instruction length. When optimizing for speed, the cost is 1 for any
> recognized instruction, and 2 for any load/store instruction.  The
> latter one can be overwritten by using cost attribute for an
> instruction.  Due to this change, we need to update also a number of
> instruction patterns with a new predicate to better reflect the costs.
> 
> gcc/
> -xx-xx  Claudiu Zissulescu  
> 
>   * config/arc/arc.c (arc_insn_cost): New function.
>   (TARGET_INSN_COST): Define.
>   * config/arc/arc.md (cost): New attribute.
>   (add_n): Use arc_nonmemory_operand.
>   (ashlsi3_insn): Likewise, also update constraints.
>   (ashrsi3_insn): Likewise.
>   (rotrsi3): Likewise.
>   (add_shift): Likewise.
>   * config/arc/predicates.md (arc_nonmemory_operand): New predicate.
> 
> testsuite/
> -xx-xx  Claudiu Zissulescu  
> 
>   * gcc.target/arc/or-cnst-size2.c: Update test.
My only worry would be asking for the length early in the RTL pipeline
may not be as accurate, but it's supposed to work, so if you're
comfortable with the end results, then OK.

jeff
> 

Re: [PATCH 3/4] [ARC] Deprecate q-class option.

[Jeff Law]

On Mon, 2020-02-03 at 12:38 +0100, Claudiu Zissulescu wrote:
> This option was used to control the short instruction selection.  However,
> there is no difference in cycles if we use or not a short instruction,
> and always someone wants a smaller program.
> 
> gcc/
> -xx-xx  Claudiu Zissulescu  
> 
>   * config/arc/arc.c (arc_conditional_register_usage): R0-R3 and
>   R12-R15 are always in ARCOMPACT16_REGS register class.
>   * config/arc/arc.opt (mq-class): Deprecate.
>   * config/arc/constraint.md ("q"): Remove dependency on mq-class
>   option.
>   * doc/invoke.texi (mq-class): Update text.
>   * common/config/arc/arc-common.c (arc_option_optimization_table):
>   Update list.
OK.
jeff
> 

Re: [PATCH 4/4] arc: Don't use if-conversion when optimizing for size.

[Jeff Law]

On Mon, 2020-02-03 at 12:38 +0100, Claudiu Zissulescu wrote:
> For ARC, predicated instructions are not very friendly with size
> optimizations, leading to increased object size. Disable if-conversion
> step when optimized for size.
> 
> gcc/
> -xx-xx  Claudiu Zissulescu  
> 
>   * common/config/arc/arc-common.c (arc_option_optimization_table):
>   Disable if-conversion step when optimized for size.
OK
jeff
> 

Re: [Patch] Inline optimization for tanh(x)/sinh(x) -> 1.0/cosh(x)

[Jeff Law]

On Fri, 2020-01-31 at 15:50 -0300, Vitor Guidi wrote:
> I took the required steps. The patch is attached to this email, I hope
> I got it right this time. I did not forward the patch to gcc-patches
> the first time, sorry for the inconvenience.
> 
> Thank you for your attention,
> 
> Vitor.
> 
> in gcc/ChangeLog:
> 2020-01-28 Vitor Guidi 
> * match.pd: New substitution rule for tanh(x)/sinh(x) ->
> 1.0/cosh(x).
> 
> in gcc/testsuite/ChangeLog:
> 2020-01-28 Vitor Guidi 
> *gcc.dg/tanhbysinh.c (new): New testcase.
Just a note.  We're currently in stage4 of our development cycle, which
means we're focused fixing regressions.  As a result your patch may not
get a lot of attention until we open the tree for gcc-11 development in
a couple months.

Jeff
> 

Re: [PATCH 1/4] [ARC] Update mlo/mhi handling when big-endian CPU.

[Jeff Law]

On Mon, 2020-02-03 at 12:38 +0100, Claudiu Zissulescu wrote:
> The ARC 600 MUL64 instructions are using mlo/mhi registers to pass the
> 64-bit result. However, the mlo/mhi registers are not swapping
> depending on endianess.  Update multiplication patterns to reflect
> this fact.
> 
> gcc/
> -xx-xx  Claudiu Zissulescu  
> 
>   * config/arc/arc.md (mulsidi_600): Correctly select mlo/mhi
>   registers.
>   (umulsidi_600): Likewise.
> 
> testsuite/
> -xx-xx  Claudiu Zissulescu  
>   Petro Karashchenko  
> 
>   * estsuite/gcc.target/arc/mul64-1.c: New test.
OK
jeff
> 

Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions

[Segher Boessenkool]

On Mon, Feb 03, 2020 at 01:59:58PM +, Richard Earnshaw (lists) wrote:
> On 03/02/2020 13:54, Segher Boessenkool wrote:
> >None of this are *rules*.  We should not pretend they are.  An email
> >subject should be useful to what the receivers of that email use it for:
> >see if it very interesting to them, see if it probably not interesting
> >to them: that's the "smth: " at the start, and the PR number at the end,
> >and of course the actual subject itself, so we should not put in too
> >much fluff in the subject, there needs to be room left (in the less than
> >fifty chars total) for an actual subject :-)
> >
> >(The example in the patch does not capitalise the subject line, btw.
> >It should.)
> 
> Where does your '50 chars' limit come from?  It's not in the glibc text, 
> and it's not in the linux kernel text either.  AFAICT this is your 
> invention and you seem to be the only person proposing it.

Ha, no.

It seems it originally is from
https://github.com/git/git/commit/927a503cd07718ea0f700052043f383253904a56
and it has been part of "man git commit" since
https://github.com/git/git/commit/936f32d3de468db8daf853155ddd5c6b191af60c
(2006 and 2007, resp.)


Segher

Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions

[Segher Boessenkool]

On Mon, Feb 03, 2020 at 01:59:58PM +, Richard Earnshaw (lists) wrote:
> On 03/02/2020 13:54, Segher Boessenkool wrote:
> >None of this are *rules*.  We should not pretend they are.  An email
> >subject should be useful to what the receivers of that email use it for:
> >see if it very interesting to them, see if it probably not interesting
> >to them: that's the "smth: " at the start, and the PR number at the end,
> >and of course the actual subject itself, so we should not put in too
> >much fluff in the subject, there needs to be room left (in the less than
> >fifty chars total) for an actual subject :-)
> >
> >(The example in the patch does not capitalise the subject line, btw.
> >It should.)
> 
> Where does your '50 chars' limit come from?  It's not in the glibc text, 
> and it's not in the linux kernel text either.  AFAICT this is your 
> invention and you seem to be the only person proposing it.
> 
> I think the linux rule (the whole line, not including the parts that are 
> removed on commit, should not exceed 75 characters) is far more sensible 
> - which is why this draft states this.

Oh, and it's not a limit, just a strong recommendation.


Segher

[GCC][PATCH][ARM] Set profile to M for Armv8.1-M

[Mihail Ionescu]

Hi,

We noticed that the profile for armv8.1-m.main was not set in arm-cpus.in
, which led to TARGET_ARM_ARCH_PROFILE and _ARM_ARCH_PROFILE not being
defined properly.



gcc/ChangeLog:

2020-02-03  Mihail Ionescu  

* config/arm/arm-cpus.in: Set profile M
for armv8.1-m.main.


Ok for trunk?

Regards,
Mihail


### Attachment also inlined for ease of reply###


diff --git a/gcc/config/arm/arm-cpus.in b/gcc/config/arm/arm-cpus.in
index 
1805b2b1cd8d6f65a967b4e3945257854a7e0fc1..96f584da325172bd1460251e2de0ad679589d312
 100644
--- a/gcc/config/arm/arm-cpus.in
+++ b/gcc/config/arm/arm-cpus.in
@@ -692,6 +692,7 @@ begin arch armv8.1-m.main
  tune for cortex-m7
  tune flags CO_PROC
  base 8M_MAIN
+ profile M
  isa ARMv8_1m_main
 # fp => FPv5-sp-d16; fp.dp => FPv5-d16
  option dsp add armv7em

diff --git a/gcc/config/arm/arm-cpus.in b/gcc/config/arm/arm-cpus.in
index 
1805b2b1cd8d6f65a967b4e3945257854a7e0fc1..96f584da325172bd1460251e2de0ad679589d312
 100644
--- a/gcc/config/arm/arm-cpus.in
+++ b/gcc/config/arm/arm-cpus.in
@@ -692,6 +692,7 @@ begin arch armv8.1-m.main
  tune for cortex-m7
  tune flags CO_PROC
  base 8M_MAIN
+ profile M
  isa ARMv8_1m_main
 # fp => FPv5-sp-d16; fp.dp => FPv5-d16
  option dsp add armv7em

[GCC][PATCH][ARM] Regenerate arm-tables.opt for Armv8.1-M patch

[Mihail Ionescu]

Hi all,

I've regenerated arm-tables.opt in config/arm to replace the improperly
generated arm-tables.opt file from "[PATCH, GCC/ARM, 2/10] Add command
line support for Armv8.1-M Mainline" (9722215a027b68651c3c7a8af9204d033197e9c0).


2020-02-03  Mihail Ionescu  

* config/arm/arm-tables.opt: Regenerate.

Ok for trunk?

Regards,
Mihail


### Attachment also inlined for ease of reply###


diff --git a/gcc/config/arm/arm-tables.opt b/gcc/config/arm/arm-tables.opt
index 
f295a4cffa2bbb3f8163fb9cef784b5af59aee12..a51a131505d184f120a3cfc51273b419bb0cb103
 100644
--- a/gcc/config/arm/arm-tables.opt
+++ b/gcc/config/arm/arm-tables.opt
@@ -353,13 +353,16 @@ EnumValue
 Enum(arm_arch) String(armv8-m.main) Value(28)
 
 EnumValue
-Enum(arm_arch) String(armv8.1-m.main) Value(29)
+Enum(arm_arch) String(armv8-r) Value(29)
 
 EnumValue
-Enum(arm_arch) String(iwmmxt) Value(30)
+Enum(arm_arch) String(armv8.1-m.main) Value(30)
 
 EnumValue
-Enum(arm_arch) String(iwmmxt2) Value(31)
+Enum(arm_arch) String(iwmmxt) Value(31)
+
+EnumValue
+Enum(arm_arch) String(iwmmxt2) Value(32)
 
 Enum
 Name(arm_fpu) Type(enum fpu_type)

diff --git a/gcc/config/arm/arm-tables.opt b/gcc/config/arm/arm-tables.opt
index 
f295a4cffa2bbb3f8163fb9cef784b5af59aee12..a51a131505d184f120a3cfc51273b419bb0cb103
 100644
--- a/gcc/config/arm/arm-tables.opt
+++ b/gcc/config/arm/arm-tables.opt
@@ -353,13 +353,16 @@ EnumValue
 Enum(arm_arch) String(armv8-m.main) Value(28)
 
 EnumValue
-Enum(arm_arch) String(armv8.1-m.main) Value(29)
+Enum(arm_arch) String(armv8-r) Value(29)
 
 EnumValue
-Enum(arm_arch) String(iwmmxt) Value(30)
+Enum(arm_arch) String(armv8.1-m.main) Value(30)
 
 EnumValue
-Enum(arm_arch) String(iwmmxt2) Value(31)
+Enum(arm_arch) String(iwmmxt) Value(31)
+
+EnumValue
+Enum(arm_arch) String(iwmmxt2) Value(32)
 
 Enum
 Name(arm_fpu) Type(enum fpu_type)

Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions

[Michael Matz]

Hello,

On Mon, 3 Feb 2020, Richard Earnshaw (lists) wrote:

> Where does your '50 chars' limit come from?  It's not in the glibc text, 
> and it's not in the linux kernel text either.  AFAICT this is your 
> invention and you seem to be the only person proposing it.

Nope, it's fairly common, so much so that it's included in the "commonly 
accepted rules" that googling for "git subject lines" gives you (as a 
snippet glimpse from some website), and that vim changes color when 
spilling over 50 chars.  I actually thought it was universal and obvious 
until this thread (which is why I admittedly only did the above google 
right before writing this mail).  For the rationale: 'git log --oneline' 
with hash and author or date should fit the usual 72 char limit.  (An 
11-character hash plus space alone would come out as 60 chars for the 
subject)

That's also the reason why some people (certainly me) are nervous about or 
dislike all the "tags" in the subject line.  E.g. what essential 
information (and subjects are for essential info, right?) is "[committed]" 
(or, even worse "[patch]") supposed to transport?  If the rest of the 
subject doesn't interest me, I don't care if something was committed or 
not; if it _does_ interest me, then I'm going to look at the mail/patch 
either way, if committed or not; at which point the info if the author 
required review or has already committed it could be gives in the body as 
well.  Similar for some other metainfo tags.  (The "subsystem:" is good, 
though).

And if we must have these tags, then why not at least short ones?  Why 
isn't "[cmt]" or something enough?  There will be very few tags, so they 
become mnemonic pretty much immediately.  What becomes clearer when 
writing "[patch v2 1/13]" in comparison to "[v2 1/13]"?


Ciao,
Michael.

[committed, amdgcn] Remove gfx801 "carrizo" support

[Andrew Stubbs]

This patch removes the -march=carrizo option and all the gfx801 related 
bits.


The Carrizo options were inherited from the original GCN port, but 
they've not been tested since (we don't have the hardware), and there's 
no XNACK handling, so it probably wouldn't work reliably. Additionally, 
libgomp is configured for discrete GPUs with independent memory; Carrizo 
would prefer shared memory.


Since this will be the first GCC release with GCN offloading there are 
no backward compatibility issues to consider (even if it weren't broken).


The option can be added back if and when somebody volunteers to fix and 
maintain it.


Andrew
Remove gfx801 "carrizo" support

2020-02-03  Andrew Stubbs  

	gcc/
	* config.gcc: Remove "carrizo" support.
	* config/gcn/gcn-opts.h (processor_type): Likewise.
	* config/gcn/gcn.c (gcn_omp_device_kind_arch_isa): Likewise.
	* config/gcn/gcn.opt (gpu_type): Likewise.
	* config/gcn/t-omp-device: Likewise.

	libgomp/
	* plugin/plugin-gcn.c (EF_AMDGPU_MACH_AMDGCN_GFX801): Remove.
	(gcn_gfx801_s): Remove.
	(isa_hsa_name): Remove gfx801.
	(isa_gcc_name): Remove gfx801/carizzo.
	(isa_code): Remove gfx801.

diff --git a/gcc/config.gcc b/gcc/config.gcc
index 5532a7be6ac..ae5a845fcce 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -4404,7 +4404,7 @@ case "${target}" in
 		for which in arch tune; do
 			eval "val=\$with_$which"
 			case ${val} in
-			"" | carrizo | fiji | gfx900 | gfx906 )
+			"" | fiji | gfx900 | gfx906 )
 # OK
 ;;
 			*)
diff --git a/gcc/config/gcn/gcn-opts.h b/gcc/config/gcn/gcn-opts.h
index aac06e3484e..385d2be8675 100644
--- a/gcc/config/gcn/gcn-opts.h
+++ b/gcc/config/gcn/gcn-opts.h
@@ -20,7 +20,6 @@
 /* Which processor to generate code or schedule for.  */
 enum processor_type
 {
-  PROCESSOR_CARRIZO,
   PROCESSOR_FIJI,
   PROCESSOR_VEGA
 };
diff --git a/gcc/config/gcn/gcn.c b/gcc/config/gcn/gcn.c
index 16c3aa2567e..5bcad7d1d41 100644
--- a/gcc/config/gcn/gcn.c
+++ b/gcc/config/gcn/gcn.c
@@ -2571,8 +2571,6 @@ gcn_omp_device_kind_arch_isa (enum omp_device_kind_arch_isa trait,
 case omp_device_arch:
   return strcmp (name, "gcn") == 0;
 case omp_device_isa:
-  if (strcmp (name, "carrizo") == 0)
-	return gcn_arch == PROCESSOR_CARRIZO;
   if (strcmp (name, "fiji") == 0)
 	return gcn_arch == PROCESSOR_FIJI;
   if (strcmp (name, "gfx900") == 0)
diff --git a/gcc/config/gcn/gcn.opt b/gcc/config/gcn/gcn.opt
index 3b3d441ded9..04c73d64630 100644
--- a/gcc/config/gcn/gcn.opt
+++ b/gcc/config/gcn/gcn.opt
@@ -25,9 +25,6 @@ Enum
 Name(gpu_type) Type(enum processor_type)
 GCN GPU type to use:
 
-EnumValue
-Enum(gpu_type) String(carrizo) Value(PROCESSOR_CARRIZO)
-
 EnumValue
 Enum(gpu_type) String(fiji) Value(PROCESSOR_FIJI)
 
@@ -38,11 +35,11 @@ EnumValue
 Enum(gpu_type) String(gfx906) Value(PROCESSOR_VEGA)
 
 march=
-Target RejectNegative Joined ToLower Enum(gpu_type) Var(gcn_arch) Init(PROCESSOR_CARRIZO)
+Target RejectNegative Joined ToLower Enum(gpu_type) Var(gcn_arch) Init(PROCESSOR_FIJI)
 Specify the name of the target GPU.
 
 mtune=
-Target RejectNegative Joined ToLower Enum(gpu_type) Var(gcn_tune) Init(PROCESSOR_CARRIZO)
+Target RejectNegative Joined ToLower Enum(gpu_type) Var(gcn_tune) Init(PROCESSOR_FIJI)
 Specify the name of the target GPU.
 
 m32
diff --git a/gcc/config/gcn/t-omp-device b/gcc/config/gcn/t-omp-device
index 288f7f065cb..d9809d5f455 100644
--- a/gcc/config/gcn/t-omp-device
+++ b/gcc/config/gcn/t-omp-device
@@ -1,4 +1,4 @@
 omp-device-properties-gcn: $(srcdir)/config/gcn/gcn.c
 	echo kind: gpu > $@
 	echo arch: gcn >> $@
-	echo isa: carrizo fiji gfx900 gfx906 >> $@
+	echo isa: fiji gfx900 gfx906 >> $@
diff --git a/libgomp/plugin/plugin-gcn.c b/libgomp/plugin/plugin-gcn.c
index 25547ef12e8..dc72c90962c 100644
--- a/libgomp/plugin/plugin-gcn.c
+++ b/libgomp/plugin/plugin-gcn.c
@@ -403,7 +403,6 @@ struct gcn_image_desc
See https://llvm.org/docs/AMDGPUUsage.html#amdgpu-ef-amdgpu-mach-table */
 
 typedef enum {
-  EF_AMDGPU_MACH_AMDGCN_GFX801 = 0x028,
   EF_AMDGPU_MACH_AMDGCN_GFX803 = 0x02a,
   EF_AMDGPU_MACH_AMDGCN_GFX900 = 0x02c,
   EF_AMDGPU_MACH_AMDGCN_GFX906 = 0x02f,
@@ -1629,7 +1628,6 @@ elf_gcn_isa_field (Elf64_Ehdr *image)
   return image->e_flags & EF_AMDGPU_MACH_MASK;
 }
 
-const static char *gcn_gfx801_s = "gfx801";
 const static char *gcn_gfx803_s = "gfx803";
 const static char *gcn_gfx900_s = "gfx900";
 const static char *gcn_gfx906_s = "gfx906";
@@ -1642,8 +1640,6 @@ static const char*
 isa_hsa_name (int isa) {
   switch(isa)
 {
-case EF_AMDGPU_MACH_AMDGCN_GFX801:
-  return gcn_gfx801_s;
 case EF_AMDGPU_MACH_AMDGCN_GFX803:
   return gcn_gfx803_s;
 case EF_AMDGPU_MACH_AMDGCN_GFX900:
@@ -1662,8 +1658,6 @@ static const char*
 isa_gcc_name (int isa) {
   switch(isa)
 {
-case EF_AMDGPU_MACH_AMDGCN_GFX801:
-  return "carrizo";
 case EF_AMDGPU_MACH_AMDGCN_GFX803:
   return "fiji";
 default:
@@ -1676,9 +1670,6 @@ isa_gcc_name (int isa) {
 
 static gcn_isa
 isa_code(c

Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions

[Segher Boessenkool]

On Mon, Feb 03, 2020 at 01:59:58PM +, Richard Earnshaw (lists) wrote:
> I think the linux rule (the whole line, not including the parts that are 
> removed on commit, should not exceed 75 characters) is far more sensible 
> - which is why this draft states this.

FWIW, on a slightly older kernel tree:

$ git log --oneline --format=%s|awk '{print length}'|sort -n|uniq -c|sort -r 
-n|head -10
  25059 50
  24872 49
  24563 47
  24547 48
  24261 51
  24050 52
  23773 53
  23527 46
  23231 54
  22907 45

On git.git:
$ git log --oneline --format=%s|awk '{print length}'|sort -n|uniq -c|sort -r 
-n|head -10
   1694 50
   1657 49
   1649 44
   1623 46
   1621 48
   1606 43
   1579 45
   1558 47
   1544 51
   1499 37


Sometimes longer subject lines are unavoidable.  Most of the time they
can be pretty easily avoided, and they hurt usability.  It is okay to
spend a few minutes on it, think of the thousands of people who will
later read it.


Segher

Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions

[Richard Earnshaw (lists)]

On 03/02/2020 17:31, Michael Matz wrote:

Hello,

On Mon, 3 Feb 2020, Richard Earnshaw (lists) wrote:


Where does your '50 chars' limit come from?  It's not in the glibc text,
and it's not in the linux kernel text either.  AFAICT this is your
invention and you seem to be the only person proposing it.


Nope, it's fairly common, so much so that it's included in the "commonly
accepted rules" that googling for "git subject lines" gives you (as a
snippet glimpse from some website), and that vim changes color when
spilling over 50 chars.  I actually thought it was universal and obvious
until this thread (which is why I admittedly only did the above google
right before writing this mail).  For the rationale: 'git log --oneline'
with hash and author or date should fit the usual 72 char limit.  (An
11-character hash plus space alone would come out as 60 chars for the
subject)

That's also the reason why some people (certainly me) are nervous about or
dislike all the "tags" in the subject line.  E.g. what essential
information (and subjects are for essential info, right?) is "[committed]"
(or, even worse "[patch]") supposed to transport?  If the rest of the
subject doesn't interest me, I don't care if something was committed or
not; if it _does_ interest me, then I'm going to look at the mail/patch
either way, if committed or not; at which point the info if the author
required review or has already committed it could be gives in the body as
well.  Similar for some other metainfo tags.  (The "subsystem:" is good,
though).

And if we must have these tags, then why not at least short ones?  Why
isn't "[cmt]" or something enough?  There will be very few tags, so they
become mnemonic pretty much immediately.  What becomes clearer when
writing "[patch v2 1/13]" in comparison to "[v2 1/13]"?



The idea is that the [...] part is NOT part of the commit, only part of 
the email.  'git am' would strip leading [...] automatically unless 
you've configured, or asked git to do otherwise.  So that leading part 
is not counted for the length calculation.


R.



Ciao,
Michael.

[committed] arm: Use move-if-change for updating regenerated files [PR93548]

[Richard Earnshaw (lists)]

The t-arm make fragment currently uses 'mv' to update some files that 
are automatically regenerated, but this causes problems on read-only 
filesystems if the date stamps are incorrect and the files have not 
really changed.  So use move-if-change instead.


PR target/93548
* config/arm/t-arm: ($(srcdir)/config/arm/arm-tune.md,
$(srcdir)/config/arm/arm-tables.opt): Use move-if-change.
diff --git a/gcc/config/arm/t-arm b/gcc/config/arm/t-arm
index e8483d45688..b883f796956 100644
--- a/gcc/config/arm/t-arm
+++ b/gcc/config/arm/t-arm
@@ -70,13 +70,15 @@ $(srcdir)/config/arm/arm-tune.md: $(srcdir)/config/arm/parsecpu.awk \
 	$(srcdir)/config/arm/arm-cpus.in
 	$(AWK) -f $(srcdir)/config/arm/parsecpu.awk -v cmd=md \
 		$(srcdir)/config/arm/arm-cpus.in > arm-tune.new
-	mv arm-tune.new $(srcdir)/config/arm/arm-tune.md
+	$(srcdir)/../move-if-change arm-tune.new \
+$(srcdir)/config/arm/arm-tune.md
 
 $(srcdir)/config/arm/arm-tables.opt: $(srcdir)/config/arm/parsecpu.awk \
   $(srcdir)/config/arm/arm-cpus.in
 	$(AWK) -f $(srcdir)/config/arm/parsecpu.awk -v cmd=opt \
 		$(srcdir)/config/arm/arm-cpus.in > arm-tables.new
-	mv arm-tables.new $(srcdir)/config/arm/arm-tables.opt
+	$(srcdir)/../move-if-change arm-tables.new \
+$(srcdir)/config/arm/arm-tables.opt
 
 arm-cpu.h: s-arm-cpu ; @true
 s-arm-cpu: $(srcdir)/config/arm/parsecpu.awk \

Re: [PATCH][AArch64] Improve popcount expansion

[Andrew Pinski]

On Mon, Feb 3, 2020 at 7:02 AM Wilco Dijkstra  wrote:
>
> The popcount expansion uses umov to extend the result and move it back
> to the integer register file.  If we model ADDV as a zero-extending
> operation, fmov can be used to move back to the integer side. This
> results in a ~0.5% speedup on deepsjeng on Cortex-A57.
>
> A typical __builtin_popcount expansion is now:
>
> fmovs0, w0
> cnt v0.8b, v0.8b
> addvb0, v0.8b
> fmovw0, s0
>
> Bootstrap OK, passes regress.

You might want to add a testcase that the autovectorizers too.
Something like this:
unsigned f(unsigned char *a)
{
 unsigned char b = 0;
 for(int i = 0; i < 16; i++)
   b+=a[i];
 return b;
}
--- CUT ---

Currently we get also:
ldr q0, [x0]
addvb0, v0.16b
umovw0, v0.b[0]
ret

Otherwise LGTM.

Thanks,
Andrew

>
> ChangeLog
> 2020-02-02  Wilco Dijkstra  
>
> gcc/
> * config/aarch64/aarch64.md (popcount2): Improve expansion.
> * config/aarch64/aarch64-simd.md
> (aarch64_zero_extend_reduc_plus_): New pattern.
> * config/aarch64/iterators.md (VDQV_E): New iterator.
> testsuite/
> * gcc.target/aarch64/popcnt2.c: New test.
>
> --
> diff --git a/gcc/config/aarch64/aarch64-simd.md 
> b/gcc/config/aarch64/aarch64-simd.md
> index 
> 97f46f96968a6bc2f93bbc812931537b819b3b19..34765ff43c1a090a31e2aed64ce95510317ab8c3
>  100644
> --- a/gcc/config/aarch64/aarch64-simd.md
> +++ b/gcc/config/aarch64/aarch64-simd.md
> @@ -2460,6 +2460,17 @@ (define_insn "aarch64_reduc_plus_internal"
>[(set_attr "type" "neon_reduc_add")]
>  )
>
> +;; ADDV with result zero-extended to SI/DImode (for popcount).
> +(define_insn "aarch64_zero_extend_reduc_plus_"
> + [(set (match_operand:GPI 0 "register_operand" "=w")
> +   (zero_extend:GPI
> +   (unspec: [(match_operand:VDQV_E 1 "register_operand" "w")]
> +UNSPEC_ADDV)))]
> + "TARGET_SIMD"
> + "add\\t%0, %1."
> +  [(set_attr "type" "neon_reduc_add")]
> +)
> +
>  (define_insn "aarch64_reduc_plus_internalv2si"
>   [(set (match_operand:V2SI 0 "register_operand" "=w")
> (unspec:V2SI [(match_operand:V2SI 1 "register_operand" "w")]
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index 
> 86c2cdfc7973f4b964ba233cfbbe369b24e0ac10..5edc76ee14b55b2b4323530e10bd22b3ffca483e
>  100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -4829,7 +4829,6 @@ (define_expand "popcount2"
>  {
>rtx v = gen_reg_rtx (V8QImode);
>rtx v1 = gen_reg_rtx (V8QImode);
> -  rtx r = gen_reg_rtx (QImode);
>rtx in = operands[1];
>rtx out = operands[0];
>if(mode == SImode)
> @@ -4843,8 +4842,7 @@ (define_expand "popcount2"
>  }
>emit_move_insn (v, gen_lowpart (V8QImode, in));
>emit_insn (gen_popcountv8qi2 (v1, v));
> -  emit_insn (gen_reduc_plus_scal_v8qi (r, v1));
> -  emit_insn (gen_zero_extendqi2 (out, r));
> +  emit_insn (gen_aarch64_zero_extend_reduc_plus_v8qi (out, v1));
>DONE;
>  })
>
> diff --git a/gcc/config/aarch64/iterators.md b/gcc/config/aarch64/iterators.md
> index 
> fc973086cb91ae0dc54eeeb0b832d522539d7982..926779bf2442fa60d184ef17308f91996d6e8d1b
>  100644
> --- a/gcc/config/aarch64/iterators.md
> +++ b/gcc/config/aarch64/iterators.md
> @@ -208,6 +208,9 @@ (define_mode_iterator VDQV [V8QI V16QI V4HI V8HI V4SI 
> V2DI])
>  ;; Advanced SIMD modes (except V2DI) for Integer reduction across lanes.
>  (define_mode_iterator VDQV_S [V8QI V16QI V4HI V8HI V4SI])
>
> +;; Advanced SIMD modes for Integer reduction across lanes (zero/sign 
> extended).
> +(define_mode_iterator VDQV_E [V8QI V16QI V4HI V8HI])
> +
>  ;; All double integer narrow-able modes.
>  (define_mode_iterator VDN [V4HI V2SI DI])
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/popcnt2.c 
> b/gcc/testsuite/gcc.target/aarch64/popcnt2.c
> new file mode 100644
> index 
> ..e321858afa4d6ecb6fc7348f39f6e5c6c0c46147
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/popcnt2.c
> @@ -0,0 +1,21 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +unsigned
> +foo (int x)
> +{
> +  return __builtin_popcount (x);
> +}
> +
> +unsigned long
> +foo1 (int x)
> +{
> +  return __builtin_popcount (x);
> +}
> +
> +/* { dg-final { scan-assembler-not {popcount} } } */
> +/* { dg-final { scan-assembler-times {cnt\t} 2 } } */
> +/* { dg-final { scan-assembler-times {fmov} 4 } } */
> +/* { dg-final { scan-assembler-not {umov} } } */
> +/* { dg-final { scan-assembler-not {uxtw} } } */
> +/* { dg-final { scan-assembler-not {sxtw} } } */
>

Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions

[Michael Matz]

Hi,

On Mon, 3 Feb 2020, Richard Earnshaw (lists) wrote:

> The idea is that the [...] part is NOT part of the commit, only part of 
> the email.

I understand that, but the subject line of this thread says "e-mail 
subject lines", so I thought we were talking about, well, exactly that; 
and I see no value of these tags in e-mails either.

(They might have a low but non-zero value for projects that use 
a single mailing list for patches and generic discussion, but we are not 
such project)

Basically: if they are deemed to clutter the git log for whatever reason, 
then there must be a very good argument for why they not also clutter 
e-mail subject lines, but instead are essential to have there, 
but not in the log.

> 'git am' would strip leading [...] automatically unless 
> you've configured, or asked git to do otherwise.  So that leading part 
> is not counted for the length calculation.

There's still e-mail netiquette which also should be obeyed, or at least 
not contradicted by git netiquette.


Ciao,
Michael.

Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions

[Jakub Jelinek]

On Mon, Feb 03, 2020 at 05:48:57PM +, Michael Matz wrote:
> On Mon, 3 Feb 2020, Richard Earnshaw (lists) wrote:
> 
> > The idea is that the [...] part is NOT part of the commit, only part of 
> > the email.
> 
> I understand that, but the subject line of this thread says "e-mail 
> subject lines", so I thought we were talking about, well, exactly that; 
> and I see no value of these tags in e-mails either.

In email, they do carry information that is useful there, the distinction
whether a patch has been committed already and doesn't need review from
others, or whether it is a patch intended for patch review, or just a RFC
patch that is not yet ready for review, but submitter is looking for some
feedback.

Jakub

Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions

[Richard Earnshaw (lists)]

On 03/02/2020 17:48, Michael Matz wrote:

Hi,

On Mon, 3 Feb 2020, Richard Earnshaw (lists) wrote:


The idea is that the [...] part is NOT part of the commit, only part of
the email.


I understand that, but the subject line of this thread says "e-mail
subject lines", so I thought we were talking about, well, exactly that;
and I see no value of these tags in e-mails either.

(They might have a low but non-zero value for projects that use
a single mailing list for patches and generic discussion, but we are not
such project)

Basically: if they are deemed to clutter the git log for whatever reason,
then there must be a very good argument for why they not also clutter
e-mail subject lines, but instead are essential to have there,
but not in the log.



Well, I'd review a patch differently depending on whether or not it was 
already committed, a patch requiring review or an RFC looking for more 
general comments, so I *do* think such an email prefix is useful.



'git am' would strip leading [...] automatically unless
you've configured, or asked git to do otherwise.  So that leading part
is not counted for the length calculation.


There's still e-mail netiquette which also should be obeyed, or at least
not contradicted by git netiquette.



The 50 char limit seems to come from wanting git log --oneline to not 
wrap in an 80 column terminal.  Whilst laudable, I'm not sure that such 
a limit doesn't become too restrictive and then lead to 
hard-to-understand summaries.


R.

Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions

[Joseph Myers]

On Mon, 3 Feb 2020, Michael Matz wrote:

> I understand that, but the subject line of this thread says "e-mail 
> subject lines", so I thought we were talking about, well, exactly that; 
> and I see no value of these tags in e-mails either.

I agree that [PATCH] is not useful (and in general, anything in the 
subject line before the actual meaningful commit summary should be as 
short as possible).  [RFC] might be useful, but not [PATCH].

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

Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions

[Michael Matz]

Hello,

On Mon, 3 Feb 2020, Jakub Jelinek wrote:

> > > The idea is that the [...] part is NOT part of the commit, only part of 
> > > the email.
> > 
> > I understand that, but the subject line of this thread says "e-mail 
> > subject lines", so I thought we were talking about, well, exactly that; 
> > and I see no value of these tags in e-mails either.
> 
> In email, they do carry information that is useful there, the distinction
> whether a patch has been committed already and doesn't need review from
> others, or whether it is a patch intended for patch review, or just a RFC
> patch that is not yet ready for review, but submitter is looking for some
> feedback.

For tags like [cmt] or [rfc] I don't have much gripe, though I do think 
that info could be given in the body, and that e.g. in e-mail archives 
(where the tags are not removed automatically) they carry the same value 
as in git log, namely zero.

But suggesting that using the subject line for tagging is recommended can 
lead to subjects like

 [PATCH][GCC][Foo][component] Fix foo component bootstrap failure

in an e-mail directed to gcc-patches@gcc.gnu.org (from somewhen last year, 
where Foo/foo was an architecture; I'm really not trying to single out the 
author).  That is, _none_ of the four tags carried any informational 
content.


Ciao,
Michael.

[PATCH] rs6000: Update constraint documentation

[Segher Boessenkool]

This un-documents constraints that cannot (or should not) be used in
inline assembler.  It also improves markup, and presentation in general.

More work is needed, but gradual improvement is easier to do.

Committing to trunk.


Segher


2020-02-03  Segher Boessenkool  

* config/rs6000/constraints.md: Improve documentation.
/
* doc/md.texi (PowerPC and IBM RS6000): Improve documentation.

---
 gcc/config/rs6000/constraints.md | 161 ++--
 gcc/doc/md.texi  | 194 +++
 2 files changed, 187 insertions(+), 168 deletions(-)

diff --git a/gcc/config/rs6000/constraints.md b/gcc/config/rs6000/constraints.md
index 398c894..4074a11 100644
--- a/gcc/config/rs6000/constraints.md
+++ b/gcc/config/rs6000/constraints.md
@@ -21,192 +21,216 @@
 
 ;; Register constraints
 
-(define_register_constraint "f" "rs6000_constraints[RS6000_CONSTRAINT_f]"
-  "@internal")
-
-(define_register_constraint "d" "rs6000_constraints[RS6000_CONSTRAINT_d]"
-  "@internal")
+; Actually defined in common.md:
+; (define_register_constraint "r" "GENERAL_REGS"
+;   "A general purpose register (GPR), @code{r0}@dots{}@code{r31}.")
 
 (define_register_constraint "b" "BASE_REGS"
-  "@internal")
+  "A base register.  Like @code{r}, but @code{r0} is not allowed, so
+   @code{r1}@dots{}@code{r31}.")
 
-(define_register_constraint "h" "SPECIAL_REGS"
-  "@internal")
+(define_register_constraint "f" "rs6000_constraints[RS6000_CONSTRAINT_f]"
+  "A floating point register (FPR), @code{f0}@dots{}@code{f31}.")
 
-(define_register_constraint "c" "CTR_REGS"
-  "@internal")
-
-(define_register_constraint "l" "LINK_REGS"
-  "@internal")
+(define_register_constraint "d" "rs6000_constraints[RS6000_CONSTRAINT_d]"
+  "A floating point register.  This is the same as @code{f} nowadays;
+   historically @code{f} was for single-precision and @code{d} was for
+   double-precision floating point.")
 
 (define_register_constraint "v" "ALTIVEC_REGS"
-  "@internal")
+  "An Altivec vector register (VR), @code{v0}@dots{}@code{v31}.")
+
+(define_register_constraint "wa" "rs6000_constraints[RS6000_CONSTRAINT_wa]"
+  "A VSX register (VSR), @code{vs0}@dots{}@code{vs63}.  This is either an
+  FPR (@code{vs0}@dots{}@code{vs31} are @code{f0}@dots{}@code{f31}) or a VR
+  (@code{vs32}@dots{}@code{vs63} are @code{v0}@dots{}@code{v31}).")
+
+(define_register_constraint "h" "SPECIAL_REGS"
+  "@internal A special register (@code{vrsave}, @code{ctr}, or @code{lr}).")
+
+(define_register_constraint "c" "CTR_REGS"
+  "The count register, @code{ctr}.")
+
+(define_register_constraint "l" "LINK_REGS"
+  "The link register, @code{lr}.")
 
 (define_register_constraint "x" "CR0_REGS"
-  "@internal")
+  "Condition register field 0, @code{cr0}.")
 
 (define_register_constraint "y" "CR_REGS"
-  "@internal")
+  "Any condition register field, @code{cr0}@dots{}@code{cr7}.")
 
 (define_register_constraint "z" "CA_REGS"
-  "@internal")
-
-;; Use w as a prefix to add VSX modes
-;; any VSX register
-(define_register_constraint "wa" "rs6000_constraints[RS6000_CONSTRAINT_wa]"
-  "Any VSX register if the -mvsx option was used or NO_REGS.")
+  "@internal The carry bit, @code{XER[CA]}.")
 
 ;; NOTE: For compatibility, "wc" is reserved to represent individual CR bits.
 ;; It is currently used for that purpose in LLVM.
 
 (define_register_constraint "we" "rs6000_constraints[RS6000_CONSTRAINT_we]"
-  "VSX register if the -mpower9-vector -m64 options were used or NO_REGS.")
+  "@internal Like @code{wa}, if @option{-mpower9-vector} and @option{-m64} are
+   used; otherwise, @code{NO_REGS}.")
 
 ;; NO_REGs register constraint, used to merge mov{sd,sf}, since movsd can use
 ;; direct move directly, and movsf can't to move between the register sets.
 ;; There is a mode_attr that resolves to wa for SDmode and wn for SFmode
-(define_register_constraint "wn" "NO_REGS" "No register (NO_REGS).")
+(define_register_constraint "wn" "NO_REGS"
+  "@internal No register (@code{NO_REGS}).")
 
 (define_register_constraint "wr" "rs6000_constraints[RS6000_CONSTRAINT_wr]"
-  "General purpose register if 64-bit instructions are enabled or NO_REGS.")
+  "@internal Like @code{r}, if @option{-mpowerpc64} is used; otherwise,
+   @code{NO_REGS}.")
 
 (define_register_constraint "wx" "rs6000_constraints[RS6000_CONSTRAINT_wx]"
-  "Floating point register if the STFIWX instruction is enabled or NO_REGS.")
+  "@internal Like @code{d}, if @option{-mpowerpc-gfxopt} is used; otherwise,
+   @code{NO_REGS}.")
 
 (define_register_constraint "wA" "rs6000_constraints[RS6000_CONSTRAINT_wA]"
-  "BASE_REGS if 64-bit instructions are enabled or NO_REGS.")
+  "@internal Like @code{b}, if @option{-mpowerpc64} is used; otherwise,
+   @code{NO_REGS}.")
 
 ;; wB needs ISA 2.07 VUPKHSW
 (define_constraint "wB"
-  "Signed 5-bit constant integer that can be loaded into an altivec register."
+  "@internal Signed 5-bit constant integer that can be loaded into an
+   Altivec regi

Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions

[Michael Matz]

Hello,

On Mon, 3 Feb 2020, Richard Earnshaw (lists) wrote:

> Well, I'd review a patch differently depending on whether or not it was 
> already committed, a patch requiring review or an RFC looking for more 
> general comments, so I *do* think such an email prefix is useful.

As I said: a very good argument must be made; it might be that rfc falls 
into the useful-tag category.

> >> 'git am' would strip leading [...] automatically unless
> >> you've configured, or asked git to do otherwise.  So that leading part
> >> is not counted for the length calculation.
> > 
> > There's still e-mail netiquette which also should be obeyed, or at least
> > not contradicted by git netiquette.
> 
> The 50 char limit seems to come from wanting git log --oneline to not wrap in
> an 80 column terminal.  Whilst laudable, I'm not sure that such a limit
> doesn't become too restrictive and then lead to hard-to-understand summaries.

In my experience hard-to-understand summaries are more related to people 
writing them than to length, IOW, I fear a larger limit like 72 characters 
won't help that.  And, as Segher put it, we aren't really talking about 
limits, only about suggestions, if you _really_ have to mention 
that 40-character function name in which you fixed something in your 
subject, then yeah, you'll go over the 50 chars.  But as recommendation 
the 50 chars make more sense than the 72 chars, IMHO.


Ciao,
Michael.

[PATCH] i386: Define TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY

[H.J. Lu]

Define TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY to make sure that the
ENDBR are emitted before the patch area.  When -mfentry -pg is also used
together, there should be no ENDBR before "call __fentry__".

OK for master if there is no regression?

Thanks.

H.J.
--
gcc/

PR target/93492
* config/i386/i386.c (ix86_asm_output_function_label): Set
function_label_emitted to true.
(ix86_print_patchable_function_entry): New function.

gcc/testsuite/

PR target/93492
* gcc.target/i386/pr93492-1.c: New test.
* gcc.target/i386/pr93492-2.c: Likewise.
* gcc.target/i386/pr93492-3.c: Likewise.


-- 
H.J.
From 5363c0289e3525139939bb678deeda98d06b2556 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" 
Date: Mon, 3 Feb 2020 10:22:57 -0800
Subject: [PATCH] i386: Define TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY

Define TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY to make sure that the
ENDBR are emitted before the patch area.  When -mfentry -pg is also used
together, there should be no ENDBR before "call __fentry__".

gcc/

	PR target/93492
	* config/i386/i386.c (ix86_asm_output_function_label): Set
	function_label_emitted to true.
	(ix86_print_patchable_function_entry): New function.

gcc/testsuite/

	PR target/93492
	* gcc.target/i386/pr93492-1.c: New test.
	* gcc.target/i386/pr93492-2.c: Likewise.
	* gcc.target/i386/pr93492-3.c: Likewise.
---
 gcc/config/i386/i386.c| 46 ++
 gcc/config/i386/i386.h|  3 +
 gcc/testsuite/gcc.target/i386/pr93492-1.c | 73 +++
 gcc/testsuite/gcc.target/i386/pr93492-2.c | 12 
 gcc/testsuite/gcc.target/i386/pr93492-3.c | 13 
 5 files changed, 147 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr93492-1.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr93492-2.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr93492-3.c

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index ffda3e8fd21..dc9bd095e9a 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -1563,6 +1563,9 @@ ix86_asm_output_function_label (FILE *asm_out_file, const char *fname,
 {
   bool is_ms_hook = ix86_function_ms_hook_prologue (decl);
 
+  if (cfun)
+cfun->machine->function_label_emitted = true;
+
   if (is_ms_hook)
 {
   int i, filler_count = (TARGET_64BIT ? 32 : 16);
@@ -9118,6 +9121,45 @@ ix86_output_function_epilogue (FILE *file ATTRIBUTE_UNUSED)
 }
 }
 
+/* Implement TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY.  */
+
+void
+ix86_print_patchable_function_entry (FILE *file,
+ unsigned HOST_WIDE_INT patch_area_size,
+ bool record_p)
+{
+  if (cfun->machine->function_label_emitted)
+{
+  if ((flag_cf_protection & CF_BRANCH)
+	  && !lookup_attribute ("nocf_check",
+TYPE_ATTRIBUTES (TREE_TYPE (cfun->decl)))
+	  && (!flag_manual_endbr
+	  || lookup_attribute ("cf_check",
+   DECL_ATTRIBUTES (cfun->decl)))
+	  && !cgraph_node::get (cfun->decl)->only_called_directly_p ())
+	{
+	  /* Remove ENDBR that follows the patch area.  */
+	  rtx_insn *insn = next_real_nondebug_insn (get_insns ());
+	  if (insn
+	  && INSN_P (insn)
+	  && GET_CODE (PATTERN (insn)) == UNSPEC_VOLATILE
+	  && XINT (PATTERN (insn), 1) == UNSPECV_NOP_ENDBR)
+	delete_insn (insn);
+
+	  /* Remove the queued ENDBR.  */
+	  cfun->machine->endbr_queued_at_entrance = false;
+
+	  /* Insert a ENDBR before the patch area right after the
+	 function label and the .cfi_startproc directive.  */
+	  asm_fprintf (file, "\t%s\n",
+		   TARGET_64BIT ? "endbr64" : "endbr32");
+	}
+}
+
+  default_print_patchable_function_entry (file, patch_area_size,
+	  record_p);
+}
+
 /* Return a scratch register to use in the split stack prologue.  The
split stack prologue is used for -fsplit-stack.  It is the first
instructions in the function, even before the regular prologue.
@@ -22744,6 +22786,10 @@ ix86_run_selftests (void)
 #undef TARGET_ASM_FUNCTION_EPILOGUE
 #define TARGET_ASM_FUNCTION_EPILOGUE ix86_output_function_epilogue
 
+#undef TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY
+#define TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY \
+  ix86_print_patchable_function_entry
+
 #undef TARGET_ENCODE_SECTION_INFO
 #ifndef SUBTARGET_ENCODE_SECTION_INFO
 #define TARGET_ENCODE_SECTION_INFO ix86_encode_section_info
diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h
index 943e9a5c783..46a809afb96 100644
--- a/gcc/config/i386/i386.h
+++ b/gcc/config/i386/i386.h
@@ -2844,6 +2844,9 @@ struct GTY(()) machine_function {
   /* If true, ENDBR is queued at function entrance.  */
   BOOL_BITFIELD endbr_queued_at_entrance : 1;
 
+  /* If true, the function label has been emitted.  */
+  BOOL_BITFIELD function_label_emitted : 1;
+
   /* True if the function needs a stack frame.  */
   BOOL_BITFIELD stack_frame_required : 1;
 
diff --git a/gcc/testsuite/gcc.target/i386/pr93492-1.c b/gcc/testsuite/gcc.target/i386/pr93492-1.c
new file mode 100644
index 000..478c9ba7c04
--- /dev/null
+++ b/gcc/tests

Re: [PATCH] adjust object size computation for union accesses and PHIs (PR 92765)

[Jeff Law]

On Fri, 2020-01-31 at 12:04 -0700, Martin Sebor wrote:
> Attached is a reworked patch since the first one didn't go far
> enough to solve the major problems.  The new solution relies on
> get_range_strlen_dynamic the same way as the sprintf optimization,
> and does away with the determine_min_objsize function and calling
> compute_builtin_object_size.
> 
> To minimize testsuite fallout I extended get_range_strlen to handle
> a couple more kinds expressions(*), but I still had to xfail and
> disable a few tests that were relying on being able to use the type
> of the destination object as the upper bound on the string length.
> 
> Tested on x86_64-linux.
> 
> Martin
> 
> [*] With all the issues around MEM_REFs and types this change needs
> extra scrutiny.  I'm still not sure I fully understand what can and
> what cannot be safely relied on at this level.
> 
> On 1/15/20 6:18 AM, Martin Sebor wrote:
> > The strcmp optimization newly introduced in GCC 10 relies on
> > the size of the smallest referenced array object to determine
> > whether the function can return zero.  When the size of
> > the object is smaller than the length of the other string
> > argument the optimization folds the equality to false.
> > 
> > The bug report has identified a couple of problems here:
> > 1) when the access to the array object is via a pointer to
> > a (possibly indirect) member of a union, in GIMPLE the pointer
> > may actually point to a different member than the one in
> > the original source code.  Thus the size of the array may
> > appear to be smaller than in the source code which can then
> > result in the optimization being invalid.
> > 2) when the pointer in the access may point to two or more
> > arrays of different size (i.e., it's the result of a PHI),
> > assuming it points to the smallest of them can also lead
> > to an incorrect result when the optimization is applied.
> > 
> > The attached patch adjusts the optimization to 1) avoid making
> > any assumptions about the sizes of objects accessed via union
> > types, and b) use the size of the largest object in PHI nodes.
> > 
> > Tested on x86_64-linux.
> > 
> > Martin
> 
> 
> PR tree-optimization/92765 - wrong code for strcmp of a union member
> 
> gcc/ChangeLog:
> 
> PR tree-optimization/92765
> * gimple-fold.c (get_range_strlen_tree): Handle MEM_REF and PARM_DECL.
> * tree-ssa-strlen.c (compute_string_length): Remove.
> (determine_min_objsize): Remove.
> (get_len_or_size): Add an argument.  Call get_range_strlen_dynamic.
> Avoid using type size as the upper bound on string length.
> (handle_builtin_string_cmp): Add an argument.  Adjust.
> (strlen_check_and_optimize_call): Pass additional argument to
> handle_builtin_string_cmp.
> 
> gcc/testsuite/ChangeLog:
> 
> PR tree-optimization/92765
> * g++.dg/tree-ssa/strlenopt-1.C: New test.
> * g++.dg/tree-ssa/strlenopt-2.C: New test.
> * gcc.dg/Warray-bounds-58.c: New test.
> * gcc.dg/Wrestrict-20.c: Avoid a valid -Wformat-overflow.
> * gcc.dg/Wstring-compare.c: Xfail a test.
> * gcc.dg/strcmpopt_2.c: Disable tests.
> * gcc.dg/strcmpopt_4.c: Adjust tests.
> * gcc.dg/strcmpopt_10.c: New test.
> * gcc.dg/strlenopt-69.c: Disable tests.
> * gcc.dg/strlenopt-92.c: New test.
> * gcc.dg/strlenopt-93.c: New test.
> * gcc.dg/strlenopt.h: Declare calloc.
> * gcc.dg/tree-ssa/pr92056.c: Xfail tests until pr93518 is resolved.
> * gcc.dg/tree-ssa/builtin-sprintf-warn-23.c: Correct test (pr93517).
> 
> diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
> index ed225922269..d70ac67e1ca 100644
> --- a/gcc/gimple-fold.c
> +++ b/gcc/gimple-fold.c
> @@ -1280,7 +1280,7 @@ get_range_strlen_tree (tree arg, bitmap *visited, 
> strlen_range_kind rkind,
>c_strlen_data *pdata, unsigned eltsize)
>  {
>gcc_assert (TREE_CODE (arg) != SSA_NAME);
> - 
> +
>/* The length computed by this invocation of the function.  */
>tree val = NULL_TREE;
>  
> @@ -1422,7 +1422,42 @@ get_range_strlen_tree (tree arg, bitmap *visited, 
> strlen_range_kind rkind,
>  type about the length here.  */
>   tight_bound = true;
> }
> -  else if (VAR_P (arg))
> +  else if (TREE_CODE (arg) == MEM_REF
> +  && TREE_CODE (TREE_TYPE (arg)) == ARRAY_TYPE
> +  && TREE_CODE (TREE_TYPE (TREE_TYPE (arg))) == INTEGER_TYPE
> +  && TREE_CODE (TREE_OPERAND (arg, 0)) == ADDR_EXPR)
> +   {
> + /* Handle a MEM_REF into a DECL accessing an array of integers,
> +being conservative about references to extern structures with
> +flexible array members that can be initialized to arbitrary
> +numbers of elements as an extension (static structs are okay).
> +FIXME: Make this less conservative -- see
> +component_ref_size in tree.

Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions

[Segher Boessenkool]

On Mon, Feb 03, 2020 at 06:54:05PM +0100, Jakub Jelinek wrote:
> On Mon, Feb 03, 2020 at 05:48:57PM +, Michael Matz wrote:
> > On Mon, 3 Feb 2020, Richard Earnshaw (lists) wrote:
> > 
> > > The idea is that the [...] part is NOT part of the commit, only part of 
> > > the email.
> > 
> > I understand that, but the subject line of this thread says "e-mail 
> > subject lines", so I thought we were talking about, well, exactly that; 
> > and I see no value of these tags in e-mails either.
> 
> In email, they do carry information that is useful there, the distinction
> whether a patch has been committed already and doesn't need review from
> others, or whether it is a patch intended for patch review, or just a RFC
> patch that is not yet ready for review, but submitter is looking for some
> feedback.

It's irrelevant whether a patch is committed or not whather it needs
review, imnsho :-)

"rfc" is useful, certainly.  It makes clear that the sender would like
some help, and/or that the subject might be controversial, both things
that have more time pressure than most.


Segher

[committed] analyzer: fix ICE due to comparing int and real constants (PR 93547)

[David Malcolm]

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to master as r10-6409-g287ccd3bd6b92f11ec90c52ffccb764aacfadb89.

gcc/analyzer/ChangeLog:
PR analyzer/93547
* constraint-manager.cc
(constraint_manager::get_or_add_equiv_class): Ensure types are
compatible before comparing constants.

gcc/testsuite/ChangeLog:
PR analyzer/93547
* gcc.dg/analyzer/pr93547.c: New test.
---
 gcc/analyzer/constraint-manager.cc  |  4 +++-
 gcc/testsuite/gcc.dg/analyzer/pr93547.c | 14 ++
 2 files changed, 17 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/pr93547.c

diff --git a/gcc/analyzer/constraint-manager.cc 
b/gcc/analyzer/constraint-manager.cc
index f3e31ee0830..4042c50fcb7 100644
--- a/gcc/analyzer/constraint-manager.cc
+++ b/gcc/analyzer/constraint-manager.cc
@@ -928,7 +928,9 @@ constraint_manager::get_or_add_equiv_class (svalue_id sid)
   int i;
   equiv_class *ec;
   FOR_EACH_VEC_ELT (m_equiv_classes, i, ec)
-   if (ec->m_constant)
+   if (ec->m_constant
+   && types_compatible_p (TREE_TYPE (cst),
+  TREE_TYPE (ec->m_constant)))
  {
tree eq = fold_build2 (EQ_EXPR, boolean_type_node,
   cst, ec->m_constant);
diff --git a/gcc/testsuite/gcc.dg/analyzer/pr93547.c 
b/gcc/testsuite/gcc.dg/analyzer/pr93547.c
new file mode 100644
index 000..15189048234
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/pr93547.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+
+void
+wy (int);
+
+int
+f9 (void)
+{
+  int p5 = __builtin_ilogb (__builtin_inf ());
+
+  wy (0);
+
+  return p5;
+}
-- 
2.21.0

[committed] analyzer: fix ICE merging models containing label pointers (PR 93546)

[David Malcolm]

PR analyzer/93546 reports an ICE within region_model::add_region_for_type
when merging two region_models each containing a label pointer.  The
two labels are stored as pointers to symbolic_regions, but these regions
were created with NULL type, leading to an assertion failure when a
merged copy is created.

The labels themselves have void (but not NULL) type.

This patch updates make_region_for_type to use the type of the decl when
creating such regions, rather than implicitly setting the region's type
to NULL, fixing the ICE.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to master as r10-6410-g5e10b9a28be9061b9b0c4aa3cfabe6d478e444e0.

gcc/analyzer/ChangeLog:
PR analyzer/93546
* region-model.cc (region_model::on_call_pre): Update for new
param of symbolic_region ctor.
(region_model::deref_rvalue): Likewise.
(region_model::add_new_malloc_region): Likewise.
(make_region_for_type): Likewise, preserving type.
* region-model.h (symbolic_region::symbolic_region): Add "type"
param and pass it to base class ctor.

gcc/testsuite/ChangeLog:
PR analyzer/93546
* gcc.dg/analyzer/pr93546.c: New test.
---
 gcc/analyzer/region-model.cc|  8 
 gcc/analyzer/region-model.h |  4 ++--
 gcc/testsuite/gcc.dg/analyzer/pr93546.c | 10 ++
 3 files changed, 16 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/pr93546.c

diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 679479c8b5c..38cf3b93b28 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -4163,7 +4163,7 @@ region_model::on_call_pre (const gcall *call, 
region_model_context *ctxt)
{
  region_id frame_rid = get_current_frame_id ();
  region_id new_rid
-   = add_region (new symbolic_region (frame_rid, false));
+   = add_region (new symbolic_region (frame_rid, NULL_TREE, false));
  if (!lhs_rid.null_p ())
{
  svalue_id ptr_sid
@@ -5113,7 +5113,7 @@ region_model::deref_rvalue (svalue_id ptr_sid, 
region_model_context *ctxt)
   We don't know if it on the heap, stack, or a global,
   so use the root region as parent.  */
region_id new_rid
- = add_region (new symbolic_region (m_root_rid, false));
+ = add_region (new symbolic_region (m_root_rid, NULL_TREE, false));
 
/* We need to write the region back into the pointer,
   or we'll get a new, different region each time.
@@ -5455,7 +5455,7 @@ region_model::add_new_malloc_region ()
 {
   region_id heap_rid
 = get_root_region ()->ensure_heap_region (this);
-  return add_region (new symbolic_region (heap_rid, true));
+  return add_region (new symbolic_region (heap_rid, NULL_TREE, true));
 }
 
 /* Attempt to return a tree that represents SID, or return NULL_TREE.
@@ -6006,7 +6006,7 @@ make_region_for_type (region_id parent_rid, tree type)
 
   /* If we have a void *, make a new symbolic region.  */
   if (VOID_TYPE_P (type))
-return new symbolic_region (parent_rid, false);
+return new symbolic_region (parent_rid, type, false);
 
   gcc_unreachable ();
 }
diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
index 70e3eb4c716..7768e45134f 100644
--- a/gcc/analyzer/region-model.h
+++ b/gcc/analyzer/region-model.h
@@ -1606,8 +1606,8 @@ namespace ana {
 class symbolic_region : public region
 {
 public:
-  symbolic_region (region_id parent_rid, bool possibly_null)
-  : region (parent_rid, svalue_id::null (), NULL_TREE),
+  symbolic_region (region_id parent_rid, tree type, bool possibly_null)
+  : region (parent_rid, svalue_id::null (), type),
 m_possibly_null (possibly_null)
   {}
   symbolic_region (const symbolic_region &other);
diff --git a/gcc/testsuite/gcc.dg/analyzer/pr93546.c 
b/gcc/testsuite/gcc.dg/analyzer/pr93546.c
new file mode 100644
index 000..432a6433be5
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/pr93546.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+
+void
+ch (int x1)
+{
+  ({ bx: &&bx; });
+  while (x1 == 0)
+{
+}
+}
-- 
2.21.0

[committed] analyzer: show BBs in .dot dumps

[David Malcolm]

I found this useful whilst debugging PR analyzer/93544

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to master as 73f386581bddc4d630b93eeb0cddd32943bf24e7.

gcc/analyzer/ChangeLog:
* engine.cc (supernode_cluster::dump_dot): Show BB index as
well as SN index.
* supergraph.cc (supernode::dump_dot): Likewise.
---
 gcc/analyzer/engine.cc | 3 ++-
 gcc/analyzer/supergraph.cc | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index 9acec704224..66ca37ea33b 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -2811,7 +2811,8 @@ public:
 (const void *)this);
 gv->indent ();
 gv->println ("style=\"dashed\";");
-gv->println ("label=\"SN: %i\";", m_supernode->m_index);
+gv->println ("label=\"SN: %i (bb: %i)\";",
+m_supernode->m_index, m_supernode->m_bb->index);
 
 int i;
 exploded_node *enode;
diff --git a/gcc/analyzer/supergraph.cc b/gcc/analyzer/supergraph.cc
index a5bf52d8aca..b20daa081d2 100644
--- a/gcc/analyzer/supergraph.cc
+++ b/gcc/analyzer/supergraph.cc
@@ -437,7 +437,7 @@ supernode::dump_dot (graphviz_out *gv, const dump_args_t 
&args) const
   gv->println("style=\"solid\";");
   gv->println("color=\"black\";");
   gv->println("fillcolor=\"lightgrey\";");
-  gv->println("label=\"sn: %i\";", m_index);
+  gv->println("label=\"sn: %i (bb: %i)\";", m_index, m_bb->index);
 
   pretty_printer *pp = gv->get_pp ();
 
-- 
2.21.0

[committed] analyzer: detect zero-assignment in phis (PR 93544)

[David Malcolm]

PR analyzer/93544 reports an ICE when attempting to report a double-free
within diagnostic_manager::prune_for_sm_diagnostic, in which the
variable of interest has become an INTEGER_CST.  Additionally, it picks
a nonsensical path through the function in which the pointer being
double-freed is known to be NULL, which we shouldn't complain about.

The dump shows that it picks the INTEGER_CST when updating var at a phi
node:
considering event 4, with var: ‘iftmp.0_2’, state: ‘start’
updating from ‘iftmp.0_2’ to ‘0B’ based on phi node
  phi: iftmp.0_2 = PHI 
considering event 3, with var: ‘0B’, state: ‘start’
and that it has picked the shortest path through the exploded graph,
and on this path the pointer has been assigned NULL.

The root cause is that the state machine's on_stmt isn't called for phi
nodes (and wouldn't make much sense, as we wouldn't know which arg to
choose).  malloc state machine::on_stmt "sees" a GIMPLE_ASSIGN to NULL
and handles it by transitioning the lhs to the "null" state, but never
"sees" GIMPLE_PHI nodes.

This patch fixes the ICE by wiring up phi-handling with state machines,
so that state machines have an on_phi vfunc.  It updates the only current
user of "is_zero_assignment" (the malloc sm) to implement equivalent
logic for phi nodes.  Doing so ensures that the pointer is in a separate
sm-state for the NULL vs non-NULL cases, and so gets separate exploded
nodes, and hence the path-finding logic chooses the correct path, and
the correct non-NULL phi argument.

The patch also adds some bulletproofing to prune_for_sm_diagnostic to
avoid crashing in the event of a bad path.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to master as r10-6412-g8525d1f5f57b11fe04a97674cc2fc2b7727621d0.

gcc/analyzer/ChangeLog:
PR analyzer/93544
* diagnostic-manager.cc
(diagnostic_manager::prune_for_sm_diagnostic): Bulletproof
against bad choices due to bad paths.
* engine.cc (impl_region_model_context::on_phi): New.
* exploded-graph.h (impl_region_model_context::on_phi): New decl.
* region-model.cc (region_model::on_longjmp): Likewise.
(region_model::handle_phi): Add phi param.  Call the ctxt's on_phi
vfunc.
(region_model::update_for_phis): Pass phi to handle_phi.
* region-model.h (region_model::handle_phi): Add phi param.
(region_model_context::on_phi): New vfunc.
(test_region_model_context::on_phi): New.
* sm-malloc.cc (malloc_state_machine::on_phi): New.
(malloc_state_machine::on_zero_assignment): New.
* sm.h (state_machine::on_phi): New vfunc.

gcc/testsuite/ChangeLog:
PR analyzer/93544
* gcc.dg/analyzer/torture/pr93544.c: New test.
---
 gcc/analyzer/diagnostic-manager.cc|  9 
 gcc/analyzer/engine.cc| 21 
 gcc/analyzer/exploded-graph.h |  2 +
 gcc/analyzer/region-model.cc  |  8 ++-
 gcc/analyzer/region-model.h   | 12 -
 gcc/analyzer/sm-malloc.cc | 52 +++
 gcc/analyzer/sm.h |  7 +++
 .../gcc.dg/analyzer/torture/pr93544.c | 17 ++
 8 files changed, 116 insertions(+), 12 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/torture/pr93544.c

diff --git a/gcc/analyzer/diagnostic-manager.cc 
b/gcc/analyzer/diagnostic-manager.cc
index 023fda9fa7c..1a82d5f22ec 100644
--- a/gcc/analyzer/diagnostic-manager.cc
+++ b/gcc/analyzer/diagnostic-manager.cc
@@ -1087,6 +1087,15 @@ diagnostic_manager::prune_for_sm_diagnostic 
(checker_path *path,
  pp_gimple_stmt_1 (&pp, phi, 0, (dump_flags_t)0);
  log ("  phi: %s", pp_formatted_text (&pp));
}
+ /* If we've chosen a bad exploded_path, then the
+phi arg might be a constant.  Fail gracefully for
+this case.  */
+ if (CONSTANT_CLASS_P (var))
+   {
+ log ("new var is a constant (bad path?);"
+  " setting var to NULL");
+ var = NULL;
+   }
}
}
 
diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index 66ca37ea33b..90f7067dec1 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -663,6 +663,27 @@ impl_region_model_context::on_condition (tree lhs, enum 
tree_code op, tree rhs)
 }
 }
 
+/* Implementation of region_model_context::on_phi vfunc.
+   Notify all state machines about the phi, which could lead to
+   state transitions.  */
+
+void
+impl_region_model_context::on_phi (const gphi *phi, tree rhs)
+{
+  int sm_idx;
+  sm_state_map *smap;
+  FOR_EACH_VEC_ELT (m_new_state->m_checker_states, sm_idx, smap)
+{
+  const state_machine &sm = m_ext_state.get_sm (sm_idx);
+   

Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions

[Segher Boessenkool]

On Mon, Feb 03, 2020 at 06:20:20PM +, Michael Matz wrote:
> On Mon, 3 Feb 2020, Richard Earnshaw (lists) wrote:
> > Well, I'd review a patch differently depending on whether or not it was 
> > already committed, a patch requiring review or an RFC looking for more 
> > general comments, so I *do* think such an email prefix is useful.
> 
> As I said: a very good argument must be made; it might be that rfc falls 
> into the useful-tag category.

Yes, "rfc" can be useful to know *before* reading the mail.

> > The 50 char limit seems to come from wanting git log --oneline to not wrap 
> > in
> > an 80 column terminal.  Whilst laudable, I'm not sure that such a limit
> > doesn't become too restrictive and then lead to hard-to-understand 
> > summaries.
> 
> In my experience hard-to-understand summaries are more related to people 
> writing them than to length, IOW, I fear a larger limit like 72 characters 
> won't help that.

Yup.  If it helps, don't think of it as "summary", think of it as "title".


Segher

Re: [PATCH] Add --with-diagnostics-urls configuration option and GCC_URLS/TERM_URLS env var

[Bernd Edlinger]

On 2/3/20 3:08 PM, Segher Boessenkool wrote:
> On Sun, Feb 02, 2020 at 08:00:44AM +, Bernd Edlinger wrote:
 Okay, thanks.  That is a strong indication that there is no need
 to interfere with screen, which proves that any auto-disabling should
 have a very specific terminal detection logic.
>>>
>>> Jakub says that he tested with a recent gnome-terminal.  That works, of
>>> course.  Mnay other terminals will not, and switching what terminal is
>>> attached to your screen session will not work well either, as far as I
>>> can tell.
>>
>> I understood his statement, that the URLs are stripped from the data
>> stream by screen and are no longer visible, even if the terminal would
>> support them i.e. you connect from a gnome-terminal.
> 
> It looks like tmux strips it as well.  But in my earlier testing it did
> not?  Confused.  Maybe this was the auto-detect thing, dunno.  I guess
> I'll test with more tmux versions when I find some more time for this.
> 

Okay, I see, I will remove the tmux detection until further notice.

>> But work fine when the compiler runs natively in a gnome-terminal.
> 
> It is big garbage for me, both with bell (which is much worse on some
> other terminals), and with the string terminator escape (ESC \).
> 

So gnome terminal is a problem, since it depend heavily on the software
version, VTE library, and gnome-terminal.
Sometimes URLs are functional, sometimes competely buggy.

But, wait a moment, here is the deal:

I can detect old gnome terminals,
they have COLORTERM=gnome-terminal (and produce garbage)

but new gnome terminal with true URL-support have

COLORTERM=truecolor

So how about adding that to the detection logic ?

Jakub, can you confirm that the COLORTERM on your working
gnome-terminal is set to "truecolor" ?

Obviously, my top priority is not breaking URLs on that
working gnome version.


Thanks
Bernd.

Re: [PATCH] Add --with-diagnostics-urls configuration option and GCC_URLS/TERM_URLS env var

[Jakub Jelinek]

On Mon, Feb 03, 2020 at 08:16:52PM +, Bernd Edlinger wrote:
> Jakub, can you confirm that the COLORTERM on your working
> gnome-terminal is set to "truecolor" ?

On the box where I have display attached to yes, but it isn't propagated
through ssh to the workstation that I do GCC development on, $COLORTERM
is unset there.  Only $TERM is set there (to xterm-256color).

Jakub

Re: [PATCH] Add --with-diagnostics-urls configuration option and GCC_URLS/TERM_URLS env var

[Bernd Edlinger]

On 2/3/20 9:26 PM, Jakub Jelinek wrote:
> On Mon, Feb 03, 2020 at 08:16:52PM +, Bernd Edlinger wrote:
>> Jakub, can you confirm that the COLORTERM on your working
>> gnome-terminal is set to "truecolor" ?
> 
> On the box where I have display attached to yes, but it isn't propagated
> through ssh to the workstation that I do GCC development on, $COLORTERM
> is unset there.  Only $TERM is set there (to xterm-256color).
> 

Yes, 

with an old gnome-terminal version (with display corruption),
I see TERM=xterm  COLORTERM=gnome-terminal

When I use a serial terminal to log in a linux box,
I have TERM=linux, and URLs do sometimes work,
sometimes not, sometime cause glitches, that depends
on the terminal where picocom is running.

We could enable auto URLs with TERM=xterm-256color,
Or better switch it off with TERM=linux and TERM=xterm,
which leaves TERM=screen and TERM=screen.xterm-256color
emit the URL escapes, since the screen can handle that
as it looks now.

I am not sure if this is too aggressive or not,
what do you think?


Thanks
Bernd.

Re: [PATCH] Add --with-diagnostics-urls configuration option and GCC_URLS/TERM_URLS env var

[Segher Boessenkool]

On Mon, Feb 03, 2020 at 08:16:52PM +, Bernd Edlinger wrote:
> So gnome terminal is a problem, since it depend heavily on the software
> version, VTE library, and gnome-terminal.
> Sometimes URLs are functional, sometimes competely buggy.
> 
> But, wait a moment, here is the deal:
> 
> I can detect old gnome terminals,
> they have COLORTERM=gnome-terminal (and produce garbage)
> 
> but new gnome terminal with true URL-support have
> 
> COLORTERM=truecolor
> 
> So how about adding that to the detection logic ?

It works on at least one of my older setups, too (will have to check
the rest when I have time, unfortunately the weekend is just past).


Segher

Re: [PATCH] V12 patch #5 of 14, Make -mpcrel default for -mcpu=future on little endian Linux 64-bit systems

[Michael Meissner]

On Fri, Jan 31, 2020 at 07:12:53PM -0600, Segher Boessenkool wrote:
> Hi!
> 
> On Thu, Jan 09, 2020 at 07:40:08PM -0500, Michael Meissner wrote:
> > * config/rs6000/linux64.h (PREFIXED_ADDR_SUPPORTED_BY_OS): Set to
> > 1 to enable prefixed addressing if -mcpu=future.
> > (PCREL_SUPPORTED_BY_OS): Set to 1 to enable PC-relative addressing
> > if -mcpu=future.
> > * config/rs6000/rs6000-cpus.h (ISA_FUTURE_MASKS_SERVER): Do not
> > enable -mprefixed-addr or -mpcrel by default.
> 
> I understand why this is needed for pcrel (or useful at least), but why
> for prefixed addressing in general as well?  What OS support is needed
> for that?
> 
> Put another way, is this just carefulness, or do you run into actual
> problems without it?

Just caution.  I can just do the PCREL.

> > +/* Enable support for pc-relative and numeric prefixed addressing on the
> > +   'future' system.  */
> > +#undef  PREFIXED_ADDR_SUPPORTED_BY_OS
> > +#define PREFIXED_ADDR_SUPPORTED_BY_OS  1
> > +
> > +#undef  PCREL_SUPPORTED_BY_OS
> > +#define PCREL_SUPPORTED_BY_OS  1
> 
> "Numeric prefixed addressing"?  What's that?  Just "and other prefixed
> addressing", maybe?

Using a prefixed address with a large offset, or using a small offset because
the traditional instruction is a DS/DQ instruction and the bottom 2/4 bits are
non-zero.

> (Is it useful to have those two separate at all, btw?  Now, that is while
> we are still developing the code, but also in the future?)
> 
> > +/* Addressing related flags on a future processor.  These are options that 
> > need
> > +   to be cleared if the target OS is not capable of supporting prefixed
> > +   addressing at all (such as 32-bit mode or if the object file format is 
> > not
> > +   ELF v2).  */
> 
> Ah.  If we are missing the needed relocations (or other as/ld support).
> So it is not about OS really, missing toolchain support instead?

It also plays into the dynamic loader of the system.  If the dynamic loader
doesn't support the new relocations, you can't do PCREL.

> 
> > +  /* Only ELFv2 currently supports prefixed/pcrel addressing.  */
> > +  else if (rs6000_current_abi != ABI_ELFv2)
> > +   {
> > + if (TARGET_PCREL && explicit_pcrel)
> > +   error ("%qs requires %qs", "-mpcrel", "-mabi=elfv2");
> > +
> > + else if (TARGET_PREFIXED_ADDR && explicit_prefixed)
> > +   error ("%qs requires %qs", "-mprefixed-addr", "-mabi=elfv2");
> 
> It would be good if the error messages also said "currently" somehow (it
> is not an actual limitation, it's just a matter of code).  "Is only
> supported with -mabi=elfv2", perhaps?

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

Re: [PATCH] V12 patch #2 of 14, Refactor rs6000_adjust_vec_address & rs6000_split_vec_extract_var

[Michael Meissner]

On Fri, Jan 31, 2020 at 11:30:22AM -0600, Segher Boessenkool wrote:
> But why is that the correct thing to do?  Garbage in, garbage out is
> perfectly fine?  Or do we have (e.g.) builtins that specify this masking?
> If so, please say that here.

It has been this way since I added these for power7 or power8, so I'm not
changing the semantics here.

Quoting from the LE abi:

VEC_EXTRACT (ARG1, ARG2)

This function uses modular arithmetic on ARG2 to determine the element number. 
For
example, if ARG2 is out of range, the compiler uses ARG2 modulo the number of 
elements
in the vector to determine the element position.

So if we were to remove the ANDing, we would have to change the ABI.

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

Re: [PATCH] Add --with-diagnostics-urls configuration option and GCC_URLS/TERM_URLS env var

[Bernd Edlinger]

On 2/3/20 10:05 PM, Segher Boessenkool wrote:
> On Mon, Feb 03, 2020 at 08:16:52PM +, Bernd Edlinger wrote:
>> So gnome terminal is a problem, since it depend heavily on the software
>> version, VTE library, and gnome-terminal.
>> Sometimes URLs are functional, sometimes competely buggy.
>>
>> But, wait a moment, here is the deal:
>>
>> I can detect old gnome terminals,
>> they have COLORTERM=gnome-terminal (and produce garbage)
>>
>> but new gnome terminal with true URL-support have
>>
>> COLORTERM=truecolor
>>
>> So how about adding that to the detection logic ?
> 
> It works on at least one of my older setups, too (will have to check
> the rest when I have time, unfortunately the weekend is just past).
> 

Cool.

so here is the next version, which removes tmux, and adds
detection of old gnome-terminal, and linux console sessions,
while also attempting to work with ssh sessions, where we
do we have a bit less reliable information, but I would
think that is still an improvement.  I'd let TERM_URLS and
GCC_URLS override the last two exceptions, as TERM=xterm
can also mean, really xterm, but while that one does not
print garbage, it does not handle the URLs either.


How do you like it this way?

Is it OK for trunk?


Thanks
Bernd.
From 35a6a850680907995e13dcd8497f5190710af0dd Mon Sep 17 00:00:00 2001
From: Bernd Edlinger 
Date: Wed, 29 Jan 2020 15:31:10 +0100
Subject: [PATCH] PR 87488: Add --with-diagnostics-urls configuration option

2020-01-30  David Malcolm  
	Bernd Edlinger  

	PR 87488
	PR other/93168
	* config.in (DIAGNOSTICS_URLS_DEFAULT): New define.
	* configure.ac (--with-diagnostics-urls): New configuration
	option, based on --with-diagnostics-color.
	(DIAGNOSTICS_URLS_DEFAULT): New define.
	* config.h: Regenerate.
	* configure: Regenerate.
	* diagnostic.c (diagnostic_urls_init): Handle -1 for
	DIAGNOSTICS_URLS_DEFAULT from configure-time
	--with-diagnostics-urls=auto-if-env by querying for a GCC_URLS
	and TERM_URLS environment variable.
	* diagnostic-url.h (diagnostic_url_format): New enum type.
	(diagnostic_urls_enabled_p): rename to...
	(parse_env_vars_for_urls): ... this, and change return type.
	* diagnostic-color.c (parse_gcc_urls): New helper function.
	(auto_enable_urls): Disable URLs on xfce4-terminal, gnome-terminal,
	the linux console, and mingw.
	(parse_env_vars_for_urls): Adjust.
	* pretty-print.h (pretty_printer::show_urls): rename to...
	(pretty_printer::url_format): ... this, and change to enum.
	* pretty-print.c (pretty_printer::pretty_printer,
	pp_begin_url, pp_end_url, test_urls): Adjust.
	* doc/install.texi (--with-diagnostics-urls): Document the new
	configuration option.
	* doc/invoke.texi (-fdiagnostics-urls): Add GCC_URLS and TERM_URLS
	vindex reference.  Update description of defaults based on the above.
---
 gcc/config.in  |   6 +++
 gcc/configure  |  41 +++-
 gcc/configure.ac   |  28 ++
 gcc/diagnostic-color.c | 102 ++---
 gcc/diagnostic-url.h   |  18 -
 gcc/diagnostic.c   |  21 --
 gcc/doc/install.texi   |  15 ++--
 gcc/doc/invoke.texi|  39 +--
 gcc/pretty-print.c |  44 ++---
 gcc/pretty-print.h |   5 ++-
 10 files changed, 293 insertions(+), 26 deletions(-)

diff --git a/gcc/config.in b/gcc/config.in
index 4829286..01fb18d 100644
--- a/gcc/config.in
+++ b/gcc/config.in
@@ -76,6 +76,12 @@
 #endif
 
 
+/* The default for -fdiagnostics-urls option */
+#ifndef USED_FOR_TARGET
+#undef DIAGNOSTICS_URLS_DEFAULT
+#endif
+
+
 /* Define 0/1 if static analyzer feature is enabled. */
 #ifndef USED_FOR_TARGET
 #undef ENABLE_ANALYZER
diff --git a/gcc/configure b/gcc/configure
index 5fa565a..f55cdb8 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -1015,6 +1015,7 @@ enable_host_shared
 enable_libquadmath_support
 with_linker_hash_style
 with_diagnostics_color
+with_diagnostics_urls
 enable_default_pie
 '
   ac_precious_vars='build_alias
@@ -1836,6 +1837,11 @@ Optional Packages:
   auto-if-env stands for -fdiagnostics-color=auto if
   GCC_COLOR environment variable is present and
   -fdiagnostics-color=never otherwise
+  --with-diagnostics-urls={never,auto,auto-if-env,always}
+  specify the default of -fdiagnostics-urls option
+  auto-if-env stands for -fdiagnostics-urls=auto if
+  GCC_URLS or TERM_URLS environment variable is
+  present and -fdiagnostics-urls=never otherwise
 
 Some influential environment variables:
   CC  C compiler command
@@ -18974,7 +18980,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 18977 "configure"
+#line 18983 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -19080,7 +19086,7 @@ else
   lt_dlunknown=0; lt_dlno_uscor

[COMMITTED 2/2] c++: Fix constexpr vs. reference parameter.

[Jason Merrill]

[expr.const] specifically rules out mentioning a reference even if its
address is never used, because it implies indirection that is similarly
non-constant for a pointer variable.

Tested x86_64-pc-linux-gnu, applying to trunk.

PR c++/66477
* constexpr.c (cxx_eval_constant_expression) [PARM_DECL]: Don't
defer loading the value of a reference.
---
 gcc/cp/constexpr.c|  2 -
 .../g++.dg/cpp0x/constexpr-array-ptr6.C   |  2 +-
 gcc/testsuite/g++.dg/cpp0x/constexpr-ref12.C  | 46 +++
 .../g++.dg/cpp1y/lambda-generic-const10.C |  2 +-
 4 files changed, 48 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-ref12.C

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index a39ba413d68..3962763fb21 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -5322,8 +5322,6 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, 
tree t,
r = *p;
   else if (lval)
/* Defer in case this is only used for its type.  */;
-  else if (TYPE_REF_P (TREE_TYPE (t)))
-   /* Defer, there's no lvalue->rvalue conversion.  */;
   else if (COMPLETE_TYPE_P (TREE_TYPE (t))
   && is_really_empty_class (TREE_TYPE (t), /*ignore_vptr*/false))
{
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-array-ptr6.C 
b/gcc/testsuite/g++.dg/cpp0x/constexpr-array-ptr6.C
index 3a483989c1f..1c065120314 100644
--- a/gcc/testsuite/g++.dg/cpp0x/constexpr-array-ptr6.C
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-array-ptr6.C
@@ -12,7 +12,7 @@ constexpr auto sz_d = size(array_double);
 static_assert(sz_d == 3, "Array size failure");
 
 void f(bool (¶m)[2]) {
-  static_assert(size(param) == 2, "Array size failure"); // Line 13
+  static_assert(size(param) == 2, "Array size failure"); // { dg-error "" }
   short data[] = {-1, 2, -45, 6, 88, 99, -345};
   static_assert(size(data) == 7, "Array size failure");
 }
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-ref12.C 
b/gcc/testsuite/g++.dg/cpp0x/constexpr-ref12.C
new file mode 100644
index 000..7c3ce66b4c9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-ref12.C
@@ -0,0 +1,46 @@
+// PR c++/66477
+// { dg-do compile { target c++11 } }
+
+struct a { constexpr bool g() const { return true; } };
+constexpr bool g(a&) { return true;}
+constexpr bool h(a) { return true;}
+
+a a1;
+a& ar = a1;
+
+void f(a ap, a& arp)
+{
+  a a2;
+  a& ar2 = a2;
+
+  // Most of these are OK because no data is actually loaded.
+  static_assert (a1.g(),"");
+  static_assert (g(a1),"");
+  static_assert (h(a1),"");
+
+  static_assert (a2.g(),"");
+  static_assert (g(a2),"");
+  static_assert (h(a2),"");
+
+  static_assert (ap.g(),"");
+  static_assert (g(ap),"");
+  static_assert (h(ap),"");
+
+  static_assert (ar.g(),"");
+  static_assert (g(ar),"");
+  static_assert (h(ar),"");
+
+  // But these are specifically prohibited in [expr.const]/4.12:
+  // * an id-expression that refers to a variable or data member of reference
+  //   type unless the reference has a preceding initialization and either
+  // ** it is usable in constant expressions or
+  // ** its lifetime began within the evaluation of e;
+
+  static_assert (ar2.g(),"");  // { dg-error "constant" }
+  static_assert (g(ar2),"");   // { dg-error "constant" }
+  static_assert (h(ar2),"");   // { dg-error "constant" }
+
+  static_assert (arp.g(),"");  // { dg-error "constant" }
+  static_assert (g(arp),"");   // { dg-error "constant" }
+  static_assert (h(arp),"");   // { dg-error "constant" }
+}
diff --git a/gcc/testsuite/g++.dg/cpp1y/lambda-generic-const10.C 
b/gcc/testsuite/g++.dg/cpp1y/lambda-generic-const10.C
index e0080b3d4f6..2f48dae4746 100644
--- a/gcc/testsuite/g++.dg/cpp1y/lambda-generic-const10.C
+++ b/gcc/testsuite/g++.dg/cpp1y/lambda-generic-const10.C
@@ -11,7 +11,7 @@ int main()
   constexpr auto x = f(); //ok, call constexpr const non-static method
 
   [](auto const &f) {
-constexpr auto x = f(); /*ok*/
+constexpr auto x = f();// { dg-error "" }
   }(f);
 
   [&]() {
-- 
2.18.1

[COMMITTED 1/2] c++: Allow parm of empty class type in constexpr.

[Jason Merrill]

Since copying a class object is defined in terms of the copy constructor,
copying an empty class is OK even if it would otherwise not be usable in a
constant expression.  Relatedly, using a parameter as an lvalue is no more
problematic than a local variable, and calling a member function uses the
object as an lvalue.

Tested x86_64-pc-linux-gnu, applying to trunk.

PR c++/91953
* constexpr.c (potential_constant_expression_1) [PARM_DECL]: Allow
empty class type.
[COMPONENT_REF]: A member function reference doesn't use the object
as an rvalue.
---
 gcc/cp/constexpr.c| 19 +++
 .../g++.dg/cpp0x/constexpr-empty14.C  | 10 ++
 gcc/testsuite/g++.dg/cpp1z/constexpr-if12.C   |  3 ++-
 3 files changed, 27 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-empty14.C

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 577022e9b9a..a39ba413d68 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -7013,8 +7013,13 @@ potential_constant_expression_1 (tree t, bool want_rval, 
bool strict, bool now,
   return true;
 
 case PARM_DECL:
-  if (now)
+  if (now && want_rval)
{
+ tree type = TREE_TYPE (t);
+ if (dependent_type_p (type)
+ || is_really_empty_class (type, /*ignore_vptr*/false))
+   /* An empty class has no data to read.  */
+   return true;
  if (flags & tf_error)
error ("%qE is not a constant expression", t);
  return false;
@@ -7270,10 +7275,7 @@ potential_constant_expression_1 (tree t, bool want_rval, 
bool strict, bool now,
 #endif
   return RECUR (t, any);
 
-case REALPART_EXPR:
-case IMAGPART_EXPR:
 case COMPONENT_REF:
-case BIT_FIELD_REF:
 case ARROW_EXPR:
 case OFFSET_REF:
   /* -- a class member access unless its postfix-expression is
@@ -7282,6 +7284,15 @@ potential_constant_expression_1 (tree t, bool want_rval, 
bool strict, bool now,
 postfix-expression being a potential constant expression.  */
   if (type_unknown_p (t))
return true;
+  if (is_overloaded_fn (t))
+   /* In a template, a COMPONENT_REF of a function expresses ob.fn(),
+  which uses ob as an lvalue.  */
+   want_rval = false;
+  gcc_fallthrough ();
+
+case REALPART_EXPR:
+case IMAGPART_EXPR:
+case BIT_FIELD_REF:
   return RECUR (TREE_OPERAND (t, 0), want_rval);
 
 case EXPR_PACK_EXPANSION:
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-empty14.C 
b/gcc/testsuite/g++.dg/cpp0x/constexpr-empty14.C
new file mode 100644
index 000..ca4f9a55e5f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-empty14.C
@@ -0,0 +1,10 @@
+// PR c++/91953
+// { dg-do compile { target c++11 } }
+
+struct S {};
+
+template  void
+foo (S s)
+{
+  constexpr S x = s;
+}
diff --git a/gcc/testsuite/g++.dg/cpp1z/constexpr-if12.C 
b/gcc/testsuite/g++.dg/cpp1z/constexpr-if12.C
index f21a9896ff2..005aa80fc09 100644
--- a/gcc/testsuite/g++.dg/cpp1z/constexpr-if12.C
+++ b/gcc/testsuite/g++.dg/cpp1z/constexpr-if12.C
@@ -2,12 +2,13 @@
 // { dg-do compile { target c++17 } }
 
 struct T {
+  int i;
   constexpr auto foo() { return false; }
 };
 
 template 
 constexpr auto bf(T t) {
-if constexpr(t.foo()) {// { dg-error "constant expression" }
+if constexpr(t.foo()) {
 return false;
 }
 return true;

base-commit: 19e43cbce353b63a05c3b7c39d83a2e32c9f911f
-- 
2.18.1

Re: [PATCH] V12 patch #3 of 14, Improve address validation in rs6000_adjust_vec_address

[Michael Meissner]

On Fri, Jan 31, 2020 at 05:43:20PM -0600, Segher Boessenkool wrote:
> Hi!
> 
> On Thu, Jan 09, 2020 at 07:27:58PM -0500, Michael Meissner wrote:
> > * config/rs6000/rs6000.c (reg_to_non_prefixed): Add forward
> > reference.
> 
> FWIW, it is better to just reorder the code, in most cases.
> 
> > (hard_reg_and_mode_to_addr_mask): Delete, no longer used.
> 
> Just "Delete.".  Changelogs say what, not why; you have the commit
> message for that.
> 
> > + new_addr = gen_rtx_PLUS (Pmode, op0, offset_rtx);
> 
> So this depends on op0 not being r0 here.  Do we guarantee that somehow?
> It isn't obvious, so add an assert for this please?  (Or do I miss
> something obvious?  :-) )

That particular code is inside if CONST_INT_P (op1).  Therefore, op0 cannot be
r0, but I can add an assertion.

> > +/* If the address isn't valid, move the address into the temporary base
> > +   register.  Some reasons it could not be valid include:
> > +   The address offset overflowed the 16 or 34 bit offset size;
> > +   We need to use a DS-FORM load, and the bottom 2 bits are non-zero;
> > +   We need to use a DQ-FORM load, and the bottom 2 bits are non-zero;
> > +   Only X_FORM loads can be done, and the address is D_FORM.  */
> 
> 4 bits for DQ-form?
> 
> Okay for trunk with those tweaks.  Thanks!
> 
> 
> Segher

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

[PATCH] contrib/vimrc: detect more C-like files

[Patrick Palka]

Currently this script doesn't set the indentation style for the standard library
headers under libstdc++/ because they lack a file extension.  But they do
have a modeline, so the file type is still set appropriately by Vim.  So by
inspecting &filetype, we can also detect these standard library headers as
C-like files.

contrib/ChangeLog:

* vimrc (SetStyle): Also look at &filetype to determine whether to
a file is C-like.
---
 contrib/vimrc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/vimrc b/contrib/vimrc
index bbbe1dd449b..e2ccbe0fd10 100644
--- a/contrib/vimrc
+++ b/contrib/vimrc
@@ -39,7 +39,7 @@ function! SetStyle()
   setlocal formatoptions-=ro formatoptions+=cqlt
   let l:ext = fnamemodify(l:fname, ":e")
   let l:c_exts = ['c', 'h', 'cpp', 'cc', 'C', 'H', 'def', 'java']
-  if index(l:c_exts, l:ext) != -1
+  if index(l:c_exts, l:ext) != -1 || &filetype == "cpp" || &filetype == "c"
 setlocal cindent
 setlocal cinoptions=>4,n-2,{2,^-2,:2,=2,g0,f0,h2,p4,t0,+2,(0,u0,w1,m0
   endif
-- 
2.25.0.114.g5b0ca878e0

Re: [PATCH] V12 patch #5 of 14, Make -mpcrel default for -mcpu=future on little endian Linux 64-bit systems

[Segher Boessenkool]

On Mon, Feb 03, 2020 at 04:52:42PM -0500, Michael Meissner wrote:
> > I understand why this is needed for pcrel (or useful at least), but why
> > for prefixed addressing in general as well?  What OS support is needed
> > for that?
> > 
> > Put another way, is this just carefulness, or do you run into actual
> > problems without it?
> 
> Just caution.  I can just do the PCREL.

See below...  It really isn't "OS support", so maybe a better name will
help.

> > > +/* Enable support for pc-relative and numeric prefixed addressing on the
> > > +   'future' system.  */
> > > +#undef  PREFIXED_ADDR_SUPPORTED_BY_OS
> > > +#define PREFIXED_ADDR_SUPPORTED_BY_OS1
> > > +
> > > +#undef  PCREL_SUPPORTED_BY_OS
> > > +#define PCREL_SUPPORTED_BY_OS1
> > 
> > "Numeric prefixed addressing"?  What's that?  Just "and other prefixed
> > addressing", maybe?
> 
> Using a prefixed address with a large offset, or using a small offset because
> the traditional instruction is a DS/DQ instruction and the bottom 2/4 bits are
> non-zero.

Okay.  So pretty much any prefixed load/store instruction.  "Support for
pc-relative and other prefixed addressing".  It's fine to point out pcrel
specifically, but just don't try to detail the rest here :-)

> > > +/* Addressing related flags on a future processor.  These are options 
> > > that need
> > > +   to be cleared if the target OS is not capable of supporting prefixed
> > > +   addressing at all (such as 32-bit mode or if the object file format 
> > > is not
> > > +   ELF v2).  */
> > 
> > Ah.  If we are missing the needed relocations (or other as/ld support).
> > So it is not about OS really, missing toolchain support instead?
> 
> It also plays into the dynamic loader of the system.  If the dynamic loader
> doesn't support the new relocations, you can't do PCREL.

If you are building shared stuff at all.  Right.

So toolchain support and dl support (i.e. binutils and glibc)?
Anything else?

We'll be best off if you separate those out now, because those
restrictions are independent.  Also handled by different people on
different projects ;-)

Thanks,


Segher

Re: [PATCH] i386: Define TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY

[H.J. Lu]

On Mon, Feb 3, 2020 at 10:35 AM H.J. Lu  wrote:
>
> Define TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY to make sure that the
> ENDBR are emitted before the patch area.  When -mfentry -pg is also used
> together, there should be no ENDBR before "call __fentry__".
>
> OK for master if there is no regression?
>
> Thanks.
>
> H.J.
> --
> gcc/
>
> PR target/93492
> * config/i386/i386.c (ix86_asm_output_function_label): Set
> function_label_emitted to true.
> (ix86_print_patchable_function_entry): New function.
>
> gcc/testsuite/
>
> PR target/93492
> * gcc.target/i386/pr93492-1.c: New test.
> * gcc.target/i386/pr93492-2.c: Likewise.
> * gcc.target/i386/pr93492-3.c: Likewise.
>

This version works with both .cfi_startproc and DWARF debug info.


-- 
H.J.
From c4660acd1555f90f0f76f32a59f043a51c866553 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" 
Date: Mon, 3 Feb 2020 10:22:57 -0800
Subject: [PATCH] i386: Define TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY

Define TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY to delay patchable
area generation to ENDBR generation.  It works with both .cfi_startproc
and DWARF debug info.

gcc/

	PR target/93492
	* config/i386/i386-features.c (rest_of_insert_endbranch): Set
	endbr_queued_at_entrance to TYPE_ENDBR.
	* config/i386/i386-protos.h (ix86_output_endbr): New.
	* config/i386/i386.c (ix86_asm_output_function_label): Set
	function_label_emitted to true.
	(ix86_print_patchable_function_entry): New function.
	(ix86_output_endbr): Likewise.
	(x86_function_profiler): Call ix86_output_endbr to generate
	ENDBR.
	(TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY): New.
	* i386.h (endbr_type): New.
	(machine_function): Add patch_area_size, record_patch_area and
	function_label_emitted.  Change endbr_queued_at_entrance to
	enum.
	* config/i386/i386.md (UNSPECV_PATCH_ENDBR): New.
	(patch_endbr): New.

gcc/testsuite/

	PR target/93492
	* gcc.target/i386/pr93492-1.c: New test.
	* gcc.target/i386/pr93492-2.c: Likewise.
	* gcc.target/i386/pr93492-3.c: Likewise.
---
 gcc/config/i386/i386-features.c   |  2 +-
 gcc/config/i386/i386-protos.h |  2 +
 gcc/config/i386/i386.c| 77 ++-
 gcc/config/i386/i386.h| 18 +-
 gcc/config/i386/i386.md   |  9 +++
 gcc/testsuite/gcc.target/i386/pr93492-1.c | 73 +
 gcc/testsuite/gcc.target/i386/pr93492-2.c | 12 
 gcc/testsuite/gcc.target/i386/pr93492-3.c | 13 
 8 files changed, 203 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr93492-1.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr93492-2.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr93492-3.c

diff --git a/gcc/config/i386/i386-features.c b/gcc/config/i386/i386-features.c
index b49e6f8d408..4d3d36e9ade 100644
--- a/gcc/config/i386/i386-features.c
+++ b/gcc/config/i386/i386-features.c
@@ -1963,7 +1963,7 @@ rest_of_insert_endbranch (void)
 {
   /* Queue ENDBR insertion to x86_function_profiler.  */
   if (crtl->profile && flag_fentry)
-	cfun->machine->endbr_queued_at_entrance = true;
+	cfun->machine->endbr_queued_at_entrance = TYPE_ENDBR;
   else
 	{
 	  cet_eb = gen_nop_endbr ();
diff --git a/gcc/config/i386/i386-protos.h b/gcc/config/i386/i386-protos.h
index 266381ca5a6..f9f5a243714 100644
--- a/gcc/config/i386/i386-protos.h
+++ b/gcc/config/i386/i386-protos.h
@@ -38,6 +38,8 @@ extern void ix86_expand_split_stack_prologue (void);
 extern void ix86_output_addr_vec_elt (FILE *, int);
 extern void ix86_output_addr_diff_elt (FILE *, int, int);
 
+extern void ix86_output_endbr (bool);
+
 extern enum calling_abi ix86_cfun_abi (void);
 extern enum calling_abi ix86_function_type_abi (const_tree);
 
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index ffda3e8fd21..e5b2565d5bd 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -1563,6 +1563,9 @@ ix86_asm_output_function_label (FILE *asm_out_file, const char *fname,
 {
   bool is_ms_hook = ix86_function_ms_hook_prologue (decl);
 
+  if (cfun)
+cfun->machine->function_label_emitted = true;
+
   if (is_ms_hook)
 {
   int i, filler_count = (TARGET_64BIT ? 32 : 16);
@@ -9118,6 +9121,73 @@ ix86_output_function_epilogue (FILE *file ATTRIBUTE_UNUSED)
 }
 }
 
+/* Implement TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY.  */
+
+void
+ix86_print_patchable_function_entry (FILE *file,
+ unsigned HOST_WIDE_INT patch_area_size,
+ bool record_p)
+{
+  if (cfun->machine->function_label_emitted)
+{
+  if ((flag_cf_protection & CF_BRANCH)
+	  && !lookup_attribute ("nocf_check",
+TYPE_ATTRIBUTES (TREE_TYPE (cfun->decl)))
+	  && (!flag_manual_endbr
+	  || lookup_attribute ("cf_check",
+   DECL_ATTRIBUTES (cfun->decl)))
+	  && !cgraph_node::get (cfun->decl)->only_called_directly_p ())
+	{
+	  /* Change the queued and normal ENDBR to ENDBR with
+	 patchable area.  */
+	  if (cfun->machine->endbr_queued_at_entrance)
+	cfun->machine->endbr_qu

Re: [PATCH] V12 patch #2 of 14, Refactor rs6000_adjust_vec_address & rs6000_split_vec_extract_var

[Segher Boessenkool]

On Mon, Feb 03, 2020 at 05:02:25PM -0500, Michael Meissner wrote:
> On Fri, Jan 31, 2020 at 11:30:22AM -0600, Segher Boessenkool wrote:
> > But why is that the correct thing to do?  Garbage in, garbage out is
> > perfectly fine?  Or do we have (e.g.) builtins that specify this masking?
> > If so, please say that here.
> 
> It has been this way since I added these for power7 or power8, so I'm not
> changing the semantics here.

Please don't cut away too much context.

> +  /* Mask the element to make sure the element number is between 0 and the
> + maximum number of elements - 1 so that we don't generate an address
> + outside the vector.  */

But why is that the correct thing to do?  Garbage in, garbage out is
perfectly fine?  Or do we have (e.g.) builtins that specify this masking?
If so, please say that here.

The comment should not just say it is masking thigs; that does not help
the reader much, that isn't too hard to see.  But *why* is it masking
things?  Is that a requirement, is that for better performance, is there
no reason?  If there is no reason, and it could be better not to do this,
a FIXME comment would help.

> Quoting from the LE abi:
> 
> VEC_EXTRACT (ARG1, ARG2)
> 
> This function uses modular arithmetic on ARG2 to determine the element 
> number. For
> example, if ARG2 is out of range, the compiler uses ARG2 modulo the number of 
> elements
> in the vector to determine the element position.

Right.  So it is *not* "so that we don't generate an address outside the
vector" -- it is simply because the API definition says we do, so we
should.


Segher

Re: [PATCH] V12 patch #3 of 14, Improve address validation in rs6000_adjust_vec_address

[Segher Boessenkool]

On Mon, Feb 03, 2020 at 06:19:07PM -0500, Michael Meissner wrote:
> On Fri, Jan 31, 2020 at 05:43:20PM -0600, Segher Boessenkool wrote:
> > On Thu, Jan 09, 2020 at 07:27:58PM -0500, Michael Meissner wrote:
> > >   * config/rs6000/rs6000.c (reg_to_non_prefixed): Add forward
> > >   reference.
> > 
> > FWIW, it is better to just reorder the code, in most cases.
> > 
> > >   (hard_reg_and_mode_to_addr_mask): Delete, no longer used.
> > 
> > Just "Delete.".  Changelogs say what, not why; you have the commit
> > message for that.
> > 
> > > +   new_addr = gen_rtx_PLUS (Pmode, op0, offset_rtx);
> > 
> > So this depends on op0 not being r0 here.  Do we guarantee that somehow?
> > It isn't obvious, so add an assert for this please?  (Or do I miss
> > something obvious?  :-) )
> 
> That particular code is inside if CONST_INT_P (op1).  Therefore, op0 cannot be
> r0, but I can add an assertion.

Please do (unless you think I'm just totally silly for not seeing this --
it's not always as obvious in a patch as it will be in the final code).


Segher

[PATCH] avoid issuing -Wrestrict from folder (PR 93519)

[Martin Sebor]

PR 93519 reports a false positive -Wrestrict issued for an inlined call
to strcpy that carefully guards against self-copying.  This is caused
by the caller's arguments substituted into the call during inlining and
before dead code elimination.

The attached patch avoids this by removing -Wrestrict from the folder
and deferring folding perfectly overlapping (and so undefined) calls
to strcpy (and mempcpy, but not memcpy) until much later.  Calls to
perfectly overlapping calls to memcpy are still folded early.

Tested on x86_64-linux.

Martin
PR middle-end/93519 - bogus -Wrestrict for strcpy(d, s) call guarded by d != s

gcc/testsuite/ChangeLog:

	PR middle-end/93519
	* gcc.dg/Wrestrict-14.c: Remove an xfail.
	* gcc.dg/Wrestrict-21.c: New test.
	* gcc.dg/Wrestrict-22.c: New test.
	* gcc.dg/strlenopt-92.c: New test.
	* gcc.dg/strlenopt.h: Declare more functions.

gcc/ChangeLog:

	PR middle-end/93519
	* builtins.c (expand_builtin_mempcpy): Diagnose perfectly overlapping
	copies and return destination.
	(expand_builtin_strcpy): Same.  Defer declaring locals until their
	initial value is known for better readability.
	(expand_builtin_strncpy): Same.
	* gimple-fold.c (gimple_fold_builtin_memory_op): Avoid folding
	perfectly overlapping calls to mempcpy.
	(gimple_fold_builtin_strcpy): Avoid folding perfectly overlapping
	calls..
	* gimple-ssa-warn-restrict.c (maybe_diag_equal_operands): New function.
	(check_bounds_or_overlap): Call it.
	* gimple-ssa-warn-restrict.h (maybe_diag_equal_operands): Declare.
	* tree-ssa-strlen.c (maybe_handle_store_to_self): New function.
	(handle_builtin_strcpy): Call it.
	(handle_builtin_memcpy): Same.

diff --git a/gcc/builtins.c b/gcc/builtins.c
index e4a8694054e..46cb3f8ae10 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -4253,6 +4253,22 @@ expand_builtin_mempcpy (tree exp, rtx target)
   if (!check_memop_access (exp, dest, src, len))
 return NULL_RTX;
 
+  if (operand_equal_p (dest, src, 0))
+{
+  if (!TREE_NO_WARNING (exp))
+	{
+	  location_t loc = EXPR_LOCATION (exp);
+	  tree func = get_callee_fndecl (exp);
+	  warning_at (loc, OPT_Wrestrict,
+			  "%qD source argument is the same as destination",
+		  func);
+	}
+
+  /* Replace perfectly overlapping calls with DST + LEN.  */
+  tree res = fold_build2 (POINTER_PLUS_EXPR, TREE_TYPE (dest), dest, len);
+  return expand_normal (res);
+}
+
   return expand_builtin_mempcpy_args (dest, src, len,
   target, exp, /*retmode=*/ RETURN_END);
 }
@@ -4474,6 +4490,21 @@ expand_builtin_strcpy (tree exp, rtx target)
 		src, destsize);
 }
 
+  if (operand_equal_p (dest, src, 0))
+{
+  if (!TREE_NO_WARNING (exp))
+	{
+	  location_t loc = EXPR_LOCATION (exp);
+	  tree func = get_callee_fndecl (exp);
+	  warning_at (loc, OPT_Wrestrict,
+			  "%qD source argument is the same as destination",
+		  func);
+	}
+
+  /* Replace perfectly overlapping calls with the destination.  */
+  return expand_normal (dest);
+}
+
   if (rtx ret = expand_builtin_strcpy_args (exp, dest, src, target))
 {
   /* Check to see if the argument was declared attribute nonstring
@@ -4810,6 +4841,22 @@ expand_builtin_strncpy (tree exp, rtx target)
 return NULL_RTX;
   tree dest = CALL_EXPR_ARG (exp, 0);
   tree src = CALL_EXPR_ARG (exp, 1);
+
+  if (operand_equal_p (dest, src, 0))
+{
+  if (!TREE_NO_WARNING (exp))
+	{
+	  location_t loc = EXPR_LOCATION (exp);
+	  tree func = get_callee_fndecl (exp);
+	  warning_at (loc, OPT_Wrestrict,
+			  "%qD source argument is the same as destination",
+		  func);
+	}
+
+  /* Replace perfectly overlapping calls with the destination.  */
+  return expand_normal (dest);
+}
+
   /* The number of bytes to write (not the maximum).  */
   tree len = CALL_EXPR_ARG (exp, 2);
 
diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index ed225922269..50b1d627760 100644
--- a/gcc/gimple-fold.c
+++ b/gcc/gimple-fold.c
@@ -725,6 +725,12 @@ gimple_fold_builtin_memory_op (gimple_stmt_iterator *gsi,
  DEST{,+LEN,+LEN-1}.  */
   if (operand_equal_p (src, dest, 0))
 {
+  /* Fail for mempcpy (but not memcpy, and certainly not memmove)
+	 if SRC and DEST are the same.  Handle the overlap later.  */
+  if ((code == BUILT_IN_MEMPCPY || code == BUILT_IN_MEMPCPY_CHK)
+	  && operand_equal_p (src, dest, 0))
+	return false;
+
   /* Avoid diagnosing exact overlap in calls to __builtin_memcpy.
 	 It's safe and may even be emitted by GCC itself (see bug
 	 32667).  */
@@ -1747,37 +1753,17 @@ static bool
 gimple_fold_builtin_strcpy (gimple_stmt_iterator *gsi,
 			tree dest, tree src)
 {
-  gimple *stmt = gsi_stmt (*gsi);
-  location_t loc = gimple_location (stmt);
-  tree fn;
-
-  /* If SRC and DEST are the same (and not volatile), return DEST.  */
+  /* Fail if SRC and DEST are the same.  Handle the overlap later.  */
   if (operand_equal_p (src, dest, 0))
-{
-  /* Issue -Wrestrict unless the pointers are null (those do
-	 not p

Re: [PATCH Coroutines v2] Handle type deduction of auto and decltype(auto) with indirect_ref expression

[JunMa]

在 2020/2/3 下午8:53, Nathan Sidwell 写道:

On 2/2/20 9:28 PM, JunMa wrote:

在 2020/2/3 上午9:03, JunMa 写道:



I think all you want here is:
  await_expr = convert_from_reference (await_expr);


Thanks, I'll update it.





Regards
JunMa

Hi nathan,
Here is the update.



   /* This will produce the value (if one is provided) from the co_await
  expression.  */
   tree resume_call = TREE_VEC_ELT (awaiter_calls, 2); /* 
await_resume().  */

+  if (TREE_CODE (resume_call) == INDIRECT_REF)
+    /* Sink to await_resume call_expr.  */
+    resume_call = TREE_OPERAND (resume_call, 0);


sorry, I should have realized you still needed this bit.  This too is 
stripping a reference access, right?  I.e. TYPE_REF_P (TREE_TYPE 
(resume_call)) is true. You should that as the condition too.



you mean REFERENCE_REF_P (resume_call) ?  here is the update.

Regards
JunMa

the other part of the patch is fine

nathan



---
 gcc/cp/coroutines.cc  | 12 +++--
 .../torture/co-await-14-return-ref-to-auto.C  | 45 +++
 2 files changed, 54 insertions(+), 3 deletions(-)
 create mode 100644 
gcc/testsuite/g++.dg/coroutines/torture/co-await-14-return-ref-to-auto.C

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 8a0ce384425..0c2ec32d7db 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -800,9 +800,12 @@ build_co_await (location_t loc, tree a, suspend_point_kind 
suspend_kind)
   TREE_VEC_ELT (awaiter_calls, 1) = awsp_call; /* await_suspend().  */
   TREE_VEC_ELT (awaiter_calls, 2) = awrs_call; /* await_resume().  */
 
-  return build5_loc (loc, CO_AWAIT_EXPR, TREE_TYPE (awrs_call), a,
-e_proxy, o, awaiter_calls,
-build_int_cst (integer_type_node, (int) suspend_kind));
+  tree await_expr = build5_loc (loc, CO_AWAIT_EXPR,
+   TREE_TYPE (TREE_TYPE (awrs_func)),
+   a, e_proxy, o, awaiter_calls,
+   build_int_cst (integer_type_node,
+  (int) suspend_kind));
+  return convert_from_reference (await_expr);
 }
 
 tree
@@ -1563,6 +1566,9 @@ co_await_expander (tree *stmt, int * /*do_subtree*/, void 
*d)
   /* This will produce the value (if one is provided) from the co_await
  expression.  */
   tree resume_call = TREE_VEC_ELT (awaiter_calls, 2); /* await_resume().  */
+  if (REFERENCE_REF_P (resume_call))
+/* Sink to await_resume call_expr.  */
+resume_call = TREE_OPERAND (resume_call, 0);
   switch (stmt_code)
 {
 default: /* not likely to work .. but... */
diff --git 
a/gcc/testsuite/g++.dg/coroutines/torture/co-await-14-return-ref-to-auto.C 
b/gcc/testsuite/g++.dg/coroutines/torture/co-await-14-return-ref-to-auto.C
new file mode 100644
index 000..0a7c035ef2a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/torture/co-await-14-return-ref-to-auto.C
@@ -0,0 +1,45 @@
+//  { dg-do run }
+
+/* The simplest valued co_await we can do.  */
+
+#include "../coro.h"
+
+// boiler-plate for tests of codegen
+#include "../coro1-ret-int-yield-int.h"
+
+
+coro1
+f ()
+{
+  int t1 = 5;
+  int t2 = 5;
+  auto gX = co_await coro1::suspend_always_intrefprt{t1};
+  if (gX != t1)
+ abort();
+  decltype(auto) gX1 = co_await coro1::suspend_always_intrefprt{t2};
+  if (&gX1 != &t2)
+ abort();
+  co_return t1 + 10;
+}
+
+int main ()
+{
+  PRINT ("main: create coro1");
+  struct coro1 f_coro = f ();
+  if (f_coro.handle.done())
+{
+  PRINT ("main: we should not be 'done' [1]");
+  abort ();
+}
+  PRINT ("main: resuming [1] initial suspend");
+  while (!f_coro.handle.done())
+f_coro.handle.resume();
+  /* we should now have returned with the co_return (15) */
+  if (!f_coro.handle.done())
+{
+  PRINT ("main: we should be 'done' ");
+  abort ();
+}
+  puts ("main: done");
+  return 0;
+}
-- 
2.19.1.3.ge56e4f7

[PATCH 1/3] libstdc++: Apply the move_iterator changes described in P1207R4

[Patrick Palka]

These changes are needed for some of the tests in the constrained algorithm
patch, because they use move_iterator with an uncopyable output_iterator.  The
other changes described in the paper are already applied, it seems.

libstdc++-v3/ChangeLog:

* include/bits/stl_iterator.h (move_iterator::move_iterator): Move the
iterator when initializing _M_current.
(move_iterator::base): Split into two overloads differing in
ref-qualifiers as in P1207R4.
---
 libstdc++-v3/include/bits/stl_iterator.h | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/libstdc++-v3/include/bits/stl_iterator.h 
b/libstdc++-v3/include/bits/stl_iterator.h
index 784d200d22f..1a288a5c785 100644
--- a/libstdc++-v3/include/bits/stl_iterator.h
+++ b/libstdc++-v3/include/bits/stl_iterator.h
@@ -1166,7 +1166,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   explicit _GLIBCXX17_CONSTEXPR
   move_iterator(iterator_type __i)
-  : _M_current(__i) { }
+  : _M_current(std::move(__i)) { }
 
   template
_GLIBCXX17_CONSTEXPR
@@ -1174,9 +1174,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
: _M_current(__i.base()) { }
 
   _GLIBCXX17_CONSTEXPR iterator_type
-  base() const
+  base() const &
+#if __cplusplus > 201703L && __cpp_lib_concepts
+   requires copy_constructible
+#endif
   { return _M_current; }
 
+  _GLIBCXX17_CONSTEXPR iterator_type
+  base() &&
+  { return std::move(_M_current); }
+
   _GLIBCXX17_CONSTEXPR reference
   operator*() const
   { return static_cast(*_M_current); }
-- 
2.25.0.114.g5b0ca878e0

[PATCH 3/3] libstdc++: Implement C++20 range adaptors

[Patrick Palka]

This patch implements [range.adaptors].  It also includes the changes from P3280
and P3278 and P3323, without which many standard examples won't work.

The implementation is mostly dictated by the spec and there was not much room
for implementation discretion.  The most interesting part that was not specified
by the spec is the design of the range adaptors and range adaptor closures,
which I tried to design in a way that minimizes boilerplate and statefulness (so
that e.g. the composition of two stateless closures is stateless).

What is left unimplemented is caching of calls to begin() in filter_view,
drop_view and reverse_view, which is required to guarantee that begin() has
amortized constant time complexity.  I can implement this in a subsequent patch.

"Interesting" parts of the patch are marked with XXX comments.

libstdc++-v3/ChangeLog:

Implement C++20 range adaptors
* include/std/ranges: Include .
(subrange::_S_store_size): Mark as const instead of constexpr to
avoid what seems to be a bug in GCC.
(__detail::__box): Give it defaulted copy and move constructors.
(views::_Single::operator()): Mark constexpr.
(views::_Iota::operator()): Mark constexpr.
(__detail::Empty): Define.
(views::_RangeAdaptor, views::_RangeAdaptorClosure, ref_view, all_view,
views::all, filter_view, views::filter, transform_view,
views::transform, take_view, views::take, take_while_view,
views::take_while, drop_view, views::drop, join_view, views::join,
__detail::require_constant, __detail::tiny_range, split_view,
views::split, views::_Counted, views::counted, common_view,
views::common, reverse_view, views::reverse,
views::__detail::__is_reversible_subrange,
views::__detail::__is_reverse_view, reverse_view, views::reverse,
__detail::__has_tuple_element, elements_view, views::elements,
views::keys, views::values): Define.
* testsuite/std/ranges/adaptors/all.cc: New test.
* testsuite/std/ranges/adaptors/common.cc: Likewise.
* testsuite/std/ranges/adaptors/counted.cc: Likewise.
* testsuite/std/ranges/adaptors/drop.cc: Likewise.
* testsuite/std/ranges/adaptors/drop_while.cc: Likewise.
* testsuite/std/ranges/adaptors/elements.cc: Likewise.
* testsuite/std/ranges/adaptors/filter.cc: Likewise.
* testsuite/std/ranges/adaptors/join.cc: Likewise.
* testsuite/std/ranges/adaptors/reverse.cc: Likewise.
* testsuite/std/ranges/adaptors/split.cc: Likewise.
* testsuite/std/ranges/adaptors/take.cc: Likewise.
* testsuite/std/ranges/adaptors/take_while.cc: Likewise.
* testsuite/std/ranges/adaptors/transform.cc: Likewise.
---
 libstdc++-v3/include/std/ranges   | 2415 -
 .../testsuite/std/ranges/adaptors/all.cc  |  124 +
 .../testsuite/std/ranges/adaptors/common.cc   |   68 +
 .../testsuite/std/ranges/adaptors/counted.cc  |   64 +
 .../testsuite/std/ranges/adaptors/drop.cc |  107 +
 .../std/ranges/adaptors/drop_while.cc |   63 +
 .../testsuite/std/ranges/adaptors/elements.cc |   52 +
 .../testsuite/std/ranges/adaptors/filter.cc   |   97 +
 .../testsuite/std/ranges/adaptors/join.cc |  112 +
 .../testsuite/std/ranges/adaptors/reverse.cc  |   86 +
 .../testsuite/std/ranges/adaptors/split.cc|   82 +
 .../testsuite/std/ranges/adaptors/take.cc |   95 +
 .../std/ranges/adaptors/take_while.cc |   62 +
 .../std/ranges/adaptors/transform.cc  |   86 +
 14 files changed, 3509 insertions(+), 4 deletions(-)
 create mode 100644 libstdc++-v3/testsuite/std/ranges/adaptors/all.cc
 create mode 100644 libstdc++-v3/testsuite/std/ranges/adaptors/common.cc
 create mode 100644 libstdc++-v3/testsuite/std/ranges/adaptors/counted.cc
 create mode 100644 libstdc++-v3/testsuite/std/ranges/adaptors/drop.cc
 create mode 100644 libstdc++-v3/testsuite/std/ranges/adaptors/drop_while.cc
 create mode 100644 libstdc++-v3/testsuite/std/ranges/adaptors/elements.cc
 create mode 100644 libstdc++-v3/testsuite/std/ranges/adaptors/filter.cc
 create mode 100644 libstdc++-v3/testsuite/std/ranges/adaptors/join.cc
 create mode 100644 libstdc++-v3/testsuite/std/ranges/adaptors/reverse.cc
 create mode 100644 libstdc++-v3/testsuite/std/ranges/adaptors/split.cc
 create mode 100644 libstdc++-v3/testsuite/std/ranges/adaptors/take.cc
 create mode 100644 libstdc++-v3/testsuite/std/ranges/adaptors/take_while.cc
 create mode 100644 libstdc++-v3/testsuite/std/ranges/adaptors/transform.cc

diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
index ea558c76c9d..947e223f7f9 100644
--- a/libstdc++-v3/include/std/ranges
+++ b/libstdc++-v3/include/std/ranges
@@ -39,6 +39,7 @@
 #if __cpp_lib_concepts
 
 #include 
+#include  // std::ref
 #include 
 #include 
 #include 
@@ -255,7 +256,8 @@ namespace ranges
 class subrange : public view_interface>
 {
 priv

Re: [PATCH] i386: Define TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY

[H.J. Lu]

On Mon, Feb 3, 2020 at 4:02 PM H.J. Lu  wrote:
>
> On Mon, Feb 3, 2020 at 10:35 AM H.J. Lu  wrote:
> >
> > Define TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY to make sure that the
> > ENDBR are emitted before the patch area.  When -mfentry -pg is also used
> > together, there should be no ENDBR before "call __fentry__".
> >
> > OK for master if there is no regression?
> >
> > Thanks.
> >
> > H.J.
> > --
> > gcc/
> >
> > PR target/93492
> > * config/i386/i386.c (ix86_asm_output_function_label): Set
> > function_label_emitted to true.
> > (ix86_print_patchable_function_entry): New function.
> >
> > gcc/testsuite/
> >
> > PR target/93492
> > * gcc.target/i386/pr93492-1.c: New test.
> > * gcc.target/i386/pr93492-2.c: Likewise.
> > * gcc.target/i386/pr93492-3.c: Likewise.
> >
>
> This version works with both .cfi_startproc and DWARF debug info.
>

-g -fpatchable-function-entry=1  doesn't work together:

[hjl@gnu-cfl-1 pr93492]$ cat y.i
void f(){}
[hjl@gnu-cfl-1 pr93492]$ make y.s
/export/build/gnu/tools-build/gcc-gitlab-debug/build-x86_64-linux/gcc/xgcc
-B/export/build/gnu/tools-build/gcc-gitlab-debug/build-x86_64-linux/gcc/
-g -fpatchable-function-entry=1 -S y.i
[hjl@gnu-cfl-1 pr93492]$ cat y.s
.file "y.i"
.text
.Ltext0:
.globl f
.type f, @function
f:
.section __patchable_function_entries,"aw",@progbits
.align 8
.quad .LPFE1
.text
.LPFE1:
nop
.LFB0:
.file 1 "y.i"
.loc 1 1 9
.cfi_startproc
pushq %rbp
.cfi_def_cfa_offset 16
.cfi_offset 6, -16
movq %rsp, %rbp
.cfi_def_cfa_register 6
.loc 1 1 1
nop
popq %rbp
.cfi_def_cfa 7, 8
ret
.cfi_endproc
.LFE0:
.size f, .-f

I will update my patch to handle it.

-- 
H.J.

libbacktrace patch committed: Always build tests with -g

[Ian Lance Taylor]

This patch ensures that the libbacktrace tests are always built with
-g.  It also builds them with the default warning flags, so I had to
add a few casts to ztest.c to get it pass without warnings.  This
should fix PR 90636.  Bootstrapped and ran libbacktrace tests on
x86_64-pc-linux-gnu.  Committed to mainline.

Ian

2020-02-03  Ian Lance Taylor  

* Makefile.am (libbacktrace_TEST_CFLAGS): Define.
(test_elf32_CFLAGS): Use $(libbacktrace_test_CFLAGS).
(test_elf_64_CFLAGS, test_xcoff_32_CFLAGS): Likewise.
(test_xcoff_64_CFLAGS, test_pecoff_CFLAGS): Likewise.
(test_unknown_CFLAGS, unittest_CFLAGS): Likewise.
(unittest_alloc_CFLAGS, allocfail_CFLAGS): Likewise.
(b2test_CFLAGS, b3test_CFLAGS, btest_CFLAGS): Likewise.
(btest_lto_CFLAGS, btest_alloc_CFLAGS, stest_CFLAGS): Likewise.
(stest_alloc_CFLAGS): Likewise.
* Makefile.in: Regenerate.
* ztest.c (error_callback_compress): Mark vdata unused.
(test_large): Add casts to avoid warnings.
diff --git a/libbacktrace/Makefile.am b/libbacktrace/Makefile.am
index b251b7bc34a..c73f6633a76 100644
--- a/libbacktrace/Makefile.am
+++ b/libbacktrace/Makefile.am
@@ -93,6 +93,9 @@ TESTS =
 # Add test to this variable, if you want it to be build and run.
 BUILDTESTS =
 
+# Flags to use when compiling test programs.
+libbacktrace_TEST_CFLAGS = $(EXTRA_FLAGS) $(WARN_FLAGS) -g
+
 if NATIVE
 check_LTLIBRARIES = libbacktrace_alloc.la
 
@@ -149,41 +152,49 @@ xcoff_%.c: xcoff.c
mv $@.tmp $@
 
 test_elf_32_SOURCES = test_format.c testlib.c
+test_elf_32_CFLAGS = $(libbacktrace_TEST_CFLAGS)
 test_elf_32_LDADD = libbacktrace_noformat.la elf_32.lo
 
 BUILDTESTS += test_elf_32
 
 test_elf_64_SOURCES = test_format.c testlib.c
+test_elf_64_CFLAGS = $(libbacktrace_TEST_CFLAGS)
 test_elf_64_LDADD = libbacktrace_noformat.la elf_64.lo
 
 BUILDTESTS += test_elf_64
 
 test_xcoff_32_SOURCES = test_format.c testlib.c
+test_xcoff_32_CFLAGS = $(libbacktrace_TEST_CFLAGS)
 test_xcoff_32_LDADD = libbacktrace_noformat.la xcoff_32.lo
 
 BUILDTESTS += test_xcoff_32
 
 test_xcoff_64_SOURCES = test_format.c testlib.c
+test_xcoff_64_CFLAGS = $(libbacktrace_TEST_CFLAGS)
 test_xcoff_64_LDADD = libbacktrace_noformat.la xcoff_64.lo
 
 BUILDTESTS += test_xcoff_64
 
 test_pecoff_SOURCES = test_format.c testlib.c
+test_pecoff_CFLAGS = $(libbacktrace_TEST_CFLAGS)
 test_pecoff_LDADD = libbacktrace_noformat.la pecoff.lo
 
 BUILDTESTS += test_pecoff
 
 test_unknown_SOURCES = test_format.c testlib.c
+test_unknown_CFLAGS = $(libbacktrace_TEST_CFLAGS)
 test_unknown_LDADD = libbacktrace_noformat.la unknown.lo
 
 BUILDTESTS += test_unknown
 
 unittest_SOURCES = unittest.c testlib.c
+unittest_CFLAGS = $(libbacktrace_TEST_CFLAGS)
 unittest_LDADD = libbacktrace.la
 
 BUILDTESTS += unittest
 
 unittest_alloc_SOURCES = $(unittest_SOURCES)
+unittest_alloc_CFLAGS = $(libbacktrace_TEST_CFLAGS)
 unittest_alloc_LDADD = libbacktrace_alloc.la
 
 BUILDTESTS += unittest_alloc
@@ -200,6 +211,7 @@ libbacktrace_instrumented_alloc_la_DEPENDENCIES = \
 instrumented_alloc.lo: alloc.c
 
 allocfail_SOURCES = allocfail.c testlib.c
+allocfail_CFLAGS = $(libbacktrace_TEST_CFLAGS)
 allocfail_LDADD = libbacktrace_instrumented_alloc.la
 
 check_PROGRAMS += allocfail
@@ -212,7 +224,7 @@ if HAVE_ELF
 if HAVE_OBJCOPY_DEBUGLINK
 
 b2test_SOURCES = $(btest_SOURCES)
-b2test_CFLAGS = $(btest_CFLAGS)
+b2test_CFLAGS = $(libbacktrace_TEST_CFLAGS)
 b2test_LDFLAGS = -Wl,--build-id
 b2test_LDADD = libbacktrace_elf_for_test.la
 
@@ -222,7 +234,7 @@ TESTS += b2test_buildid
 if HAVE_DWZ
 
 b3test_SOURCES = $(btest_SOURCES)
-b3test_CFLAGS = $(btest_CFLAGS)
+b3test_CFLAGS = $(libbacktrace_TEST_CFLAGS)
 b3test_LDFLAGS = -Wl,--build-id
 b3test_LDADD = libbacktrace_elf_for_test.la
 
@@ -235,7 +247,7 @@ endif HAVE_OBJCOPY_DEBUGLINK
 endif HAVE_ELF
 
 btest_SOURCES = btest.c testlib.c
-btest_CFLAGS = $(AM_CFLAGS) -g -O
+btest_CFLAGS = $(libbacktrace_TEST_CFLAGS) -O
 btest_LDADD = libbacktrace.la
 
 BUILDTESTS += btest
@@ -243,7 +255,7 @@ BUILDTESTS += btest
 if HAVE_ELF
 
 btest_lto_SOURCES = btest.c testlib.c
-btest_lto_CFLAGS = $(AM_CFLAGS) -g -O -flto
+btest_lto_CFLAGS = $(libbacktrace_TEST_CFLAGS) -O -flto
 btest_lto_LDADD = libbacktrace.la
 
 BUILDTESTS += btest_lto
@@ -251,7 +263,7 @@ BUILDTESTS += btest_lto
 endif HAVE_ELF
 
 btest_alloc_SOURCES = $(btest_SOURCES)
-btest_alloc_CFLAGS = $(btest_CFLAGS)
+btest_alloc_CFLAGS = $(libbacktrace_TEST_CFLAGS)
 btest_alloc_LDADD = libbacktrace_alloc.la
 
 BUILDTESTS += btest_alloc
@@ -277,11 +289,13 @@ endif HAVE_OBJCOPY_DEBUGLINK
 endif HAVE_DWZ
 
 stest_SOURCES = stest.c
+stest_CFLAGS = $(libbacktrace_TEST_CFLAGS)
 stest_LDADD = libbacktrace.la
 
 BUILDTESTS += stest
 
 stest_alloc_SOURCES = $(stest_SOURCES)
+stest_alloc_CFLAGS = $(libbacktrace_TEST_CFLAGS)
 stest_alloc_LDADD = libbacktrace_alloc.la
 
 BUILDTESTS += stest_alloc
@@ -289,7 +303,7 @@ BUILDTESTS += stest_alloc
 if HAVE_ELF
 
 ztest_SOURCES = ztest.c testlib.c
-ztest_CFLAGS = -DSRCDIR=\"$(srcdir)\"
+ztest_CFLAGS = $(libbacktrace_TEST_CFLAGS) -DSRCDIR=\"$(src

Re: libgo patch committed: Update to Go1.14beta1

[Ian Lance Taylor]

On Sat, Feb 1, 2020 at 5:38 AM Andreas Schwab  wrote:
>
> ../../../libgo/go/syscall/syscall_linux_riscv64.go:7:14: error: imported and 
> not used: unsafe
> 7 | import "unsafe"
>   |  ^
> ../../../libgo/go/syscall/syscall_linux_riscv64.go:13:1: error: redefinition 
> of 'SetLen'
>13 | func (iov *Iovec) SetLen(length int) {
>   | ^
> ../../../libgo/go/syscall/socket.go:437:1: note: previous definition of 
> 'SetLen' was here
>   437 | func (iov *Iovec) SetLen(length int) {
>   | ^
> ../../../libgo/go/syscall/syscall_linux_riscv64.go:17:1: error: redefinition 
> of 'SetControllen'
>17 | func (msghdr *Msghdr) SetControllen(length int) {
>   | ^
> ../../../libgo/go/syscall/socket.go:441:1: note: previous definition of 
> 'SetControllen' was here
>   441 | func (msghdr *Msghdr) SetControllen(length int) {
>   | ^
> ../../../libgo/go/syscall/syscall_linux_riscv64.go:21:1: error: redefinition 
> of 'SetLen'
>21 | func (cmsg *Cmsghdr) SetLen(length int) {
>   | ^
> ../../../libgo/go/syscall/socket.go:445:1: note: previous definition of 
> 'SetLen' was here
>   445 | func (cmsg *Cmsghdr) SetLen(length int) {
>   | ^
> ../../../libgo/go/syscall/syscall_linux_riscv64.go:14:10: error: incompatible 
> types in assignment (cannot use type uint64 as type Iovec_len_t)
>14 |  iov.Len = uint64(length)
>   |  ^
> ../../../libgo/go/syscall/syscall_linux_riscv64.go:18:20: error: incompatible 
> types in assignment (cannot use type uint64 as type Msghdr_controllen_t)
>18 |  msghdr.Controllen = uint64(length)
>   |^
> ../../../libgo/go/syscall/syscall_linux_riscv64.go:22:11: error: incompatible 
> types in assignment (cannot use type uint64 as type Cmsghdr_len_t)
>22 |  cmsg.Len = uint64(length)
>   |   ^


Thanks.  Fixed like so.  Committed to mainline.

Ian
79530f94e9c53153c4fae3b50a8c938f89db0c32
diff --git a/gcc/go/gofrontend/MERGE b/gcc/go/gofrontend/MERGE
index 40529518b26..27f4ce342e5 100644
--- a/gcc/go/gofrontend/MERGE
+++ b/gcc/go/gofrontend/MERGE
@@ -1,4 +1,4 @@
-d796680b5a78f686ed118578e81d5b1adf48508d
+c94637ad6fd38d4814fb02d094a1a73f19323d71
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
diff --git a/libgo/go/syscall/syscall_linux_riscv64.go 
b/libgo/go/syscall/syscall_linux_riscv64.go
index e9aab94e3a1..16d8709708d 100644
--- a/libgo/go/syscall/syscall_linux_riscv64.go
+++ b/libgo/go/syscall/syscall_linux_riscv64.go
@@ -4,20 +4,6 @@
 
 package syscall
 
-import "unsafe"
-
 func (r *PtraceRegs) PC() uint64 { return r.Pc }
 
 func (r *PtraceRegs) SetPC(pc uint64) { r.Pc = pc }
-
-func (iov *Iovec) SetLen(length int) {
-   iov.Len = uint64(length)
-}
-
-func (msghdr *Msghdr) SetControllen(length int) {
-   msghdr.Controllen = uint64(length)
-}
-
-func (cmsg *Cmsghdr) SetLen(length int) {
-   cmsg.Len = uint64(length)
-}

Re: libgo patch committed: Update to Go1.14beta1

[Ian Lance Taylor]

On Sun, Feb 2, 2020 at 2:27 AM Andreas Schwab  wrote:
>
> I'm getting these errors on aarch64 with -mabi=ilp32:
>
> ../../../../libgo/go/runtime/mpagealloc.go:226:38: error: shift count overflow
>   226 |  chunks [1 << pallocChunksL1Bits]*[1 << pallocChunksL2Bits]pallocData
>   |  ^
> ../../../../libgo/go/runtime/mgcscavenge.go:487:15: error: shift count 
> overflow
>   487 |l2 := (*[1 << 
> pallocChunksL2Bits]pallocData)(atomic.Loadp(unsafe.Pointer(&s.chunks[i.l1()])))
>   |   ^
> ../../../../libgo/go/runtime/mpagealloc.go:138:22: error: shift count overflow
>   138 |   return uint(i) & (1<   |  ^
> ../../../../libgo/go/runtime/mpagealloc.go:129:21: error: integer constant 
> overflow
>   129 |   return uint(i) >> pallocChunksL1Shift
>   | ^
> ../../../../libgo/go/runtime/mpagealloc_64bit.go:34:2: error: integer 
> constant overflow
>34 |  summaryL0Bits,
>   |  ^

I'm not sure that gccgo ever fully worked with aarch64 -mabi=ilp32.
In Go I think that will have to be represented with a new GOARCH
value, arm64p32.

Ian

[PATCH 03/14] Add file support and functions for diagnostic support.

[Bill Schmidt]

2020-02-03  Bill Schmidt  

* config/rs6000/rs6000-genbif.c (bif_file): New filescope
variable.
(ovld_file): Likewise.
(header_file): Likewise.
(init_file): Likewise.
(defines_file): Likewise.
(pgm_path): Likewise.
(bif_path): Likewise.
(ovld_path): Likewise.
(header_path): Likewise.
(init_path): Likewise.
(defines_path): Likewise.
(LINELEN): New defined constant.
(linebuf): New filescope variable.
(line): Likewise.
(pos): Likewise.
(diag): Likewise.
(bif_diag): New function.
(ovld_diag): New function.
---
 gcc/config/rs6000/rs6000-genbif.c | 47 +++
 1 file changed, 47 insertions(+)

diff --git a/gcc/config/rs6000/rs6000-genbif.c 
b/gcc/config/rs6000/rs6000-genbif.c
index a53209ed040..3fb13cb11d6 100644
--- a/gcc/config/rs6000/rs6000-genbif.c
+++ b/gcc/config/rs6000/rs6000-genbif.c
@@ -122,3 +122,50 @@ along with GCC; see the file COPYING3.  If not see
 #include 
 #include 
 #include 
+
+/* Input and output file descriptors and pathnames.  */
+static FILE *bif_file;
+static FILE *ovld_file;
+static FILE *header_file;
+static FILE *init_file;
+static FILE *defines_file;
+
+static const char *pgm_path;
+static const char *bif_path;
+static const char *ovld_path;
+static const char *header_path;
+static const char *init_path;
+static const char *defines_path;
+
+/* Position information.  Note that "pos" is zero-indexed, but users
+   expect one-indexed column information, so representations of "pos"
+   as columns in diagnostic messages must be adjusted.  */
+#define LINELEN 1024
+static char linebuf[LINELEN];
+static int line;
+static int pos;
+
+/* Pointer to a diagnostic function.  */
+void (*diag) (const char *, ...) __attribute__ ((format (printf, 1, 2)))
+  = NULL;
+
+/* Custom diagnostics.  */
+static void __attribute__ ((format (printf, 1, 2)))
+bif_diag (const char * fmt, ...)
+{
+  va_list args;
+  fprintf (stderr, "%s:%d: ", bif_path, line);
+  va_start (args, fmt);
+  vfprintf (stderr, fmt, args);
+  va_end (args);
+}
+
+static void __attribute__ ((format (printf, 1, 2)))
+ovld_diag (const char * fmt, ...)
+{
+  va_list args;
+  fprintf (stderr, "%s:%d: ", ovld_path, line);
+  va_start (args, fmt);
+  vfprintf (stderr, fmt, args);
+  va_end (args);
+}
-- 
2.17.1

[PATCH 01/14] Initial create of rs6000-genbif.c.

[Bill Schmidt]

Includes header documentation and initial set of include directives.

2020-02-03  Bill Schmidt  

* config/rs6000/rs6000-genbif.c: New file.
---
 gcc/config/rs6000/rs6000-genbif.c | 124 ++
 1 file changed, 124 insertions(+)
 create mode 100644 gcc/config/rs6000/rs6000-genbif.c

diff --git a/gcc/config/rs6000/rs6000-genbif.c 
b/gcc/config/rs6000/rs6000-genbif.c
new file mode 100644
index 000..a53209ed040
--- /dev/null
+++ b/gcc/config/rs6000/rs6000-genbif.c
@@ -0,0 +1,124 @@
+/* Generate built-in function initialization and recognition for Power.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   Contributed by Bill Schmidt, IBM 
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+.  */
+
+/* This program generates built-in function initialization and
+   recognition code for Power targets, based on text files that
+   describe the built-in functions and vector overloads:
+
+ rs6000-bif.def   Table of built-in functions
+ rs6000-overload.def  Table of overload functions
+
+   Both files group similar functions together in "stanzas," as
+   described below.
+
+   Each stanza in the built-in function file starts with a line
+   identifying the target mask for which the group of functions is
+   permitted, with the mask in square brackets.  This is the only
+   information allowed on the stanza header line, other than
+   whitespace.  Following the stanza header are two lines for each
+   function: the prototype line and the attributes line.  The
+   prototype line has this format, where the square brackets
+   indicate optional information and angle brackets indicate
+   required information:
+
+ [kind]   ();
+
+   Here [kind] can be one of "const", "pure", or "math";
+is a legal type for a built-in function result;
+is the name by which the function can be called;
+   and  is a comma-separated list of legal types
+   for built-in function arguments.  The argument list may be
+   empty, but the parentheses and semicolon are required.
+
+   The attributes line looks like this:
+
+   {}
+
+   Here  is a unique internal identifier for the built-in
+   function that will be used as part of an enumeration of all
+   built-in functions;  is the define_expand or
+   define_insn that will be invoked when the call is expanded;
+   and  is a comma-separated list of special
+   conditions that apply to the built-in function.  The attribute
+   list may be empty, but the braces are required.
+
+   Attributes are strings, such as these:
+
+ initProcess as a vec_init function
+ set Process as a vec_set function
+ ext Process as a vec_extract function
+ nosoft  Not valid with -msoft-float
+ ldv Needs special handling for vec_ld semantics
+ stv Needs special handling for vec_st semantics
+ reveNeeds special handling for element reversal
+ abs Needs special handling for absolute value
+ predNeeds special handling for comparison predicates
+ htm Needs special handling for transactional memory
+
+   An example stanza might look like this:
+
+[TARGET_ALTIVEC]
+  const vector signed char __builtin_altivec_abs_v16qi (vector signed char);
+ABS_V16QI absv16qi2 {abs}
+  const vector signed short __builtin_altivec_abs_v8hi (vector signed short);
+ABS_V8HI absv8hi2 {abs}
+
+   Note the use of indentation, which is recommended but not required.
+
+   The overload file has more complex stanza headers.  Here the stanza
+   represents all functions with the same overloaded function name:
+
+ [, , ]
+
+   Here the square brackets are part of the syntax,  is a
+   unique internal identifier for the overload that will be used as part
+   of an enumeration of all overloaded functions;  is the
+   name that will appear as a #define in altivec.h; and 
+   is the name that is overloaded in the back end.
+
+   Each function entry again has two lines.  The first line is again a
+   prototype line (this time without [kind]):
+
+   ();
+
+   The second line contains only one token: the  that this
+   particular instance of the overloaded function maps to.  It must
+   match a token that appears in the bif file.
+
+   An example stanza might look like this:
+
+[VEC_ABS, vec_abs, __builtin_vec_abs]
+  vector signed char __builtin_vec_abs (vector signed char);
+ABS_V16QI
+  vector signed short __builtin_vec_abs

[PATCH 00/14] rs6000: Begin replacing built-in support

[Bill Schmidt]

The current built-in support in the rs6000 back end requires at least
a master's degree in spelunking to comprehend.  It's full of cruft,
redundancy, and unused bits of code, and long overdue for a
replacement.  This is the first part of my project to do that.

My intent is to make adding new built-in functions as simple as adding
a few lines to a couple of files, and automatically generating as much
of the initialization, overload resolution, and expansion logic as
possible.  This patch series establishes the format of the input files
and creates a new program (rs6000-genbif) to:

 * Parse the input files into an internal representation;
 * Generate a file of #defines (rs6000-vecdefines.h) for eventual
   inclusion into altivec.h; and
 * Generate an initialization file to create and initialize tables of
   built-in functions and overloads.

Note that none of the code in this patch set affects GCC's operation
at all, with the exception of patch #14.  Patch 14 causes the program
rs6000-genbif to be built and executed, producing the output files,
and linking rs6000-bif.o into the executable.  However, none of the
code in rs6000-bif.o is called, so the only effect is to make the gcc
executable larger.

I'd like to consider at least patches 1-13 as stage 4 material for the
current release.  I'd prefer to also include patch 14 for convenience,
but I understand if that's not deemed acceptable.

I've attempted to break this up into logical pieces for easy
consumption, but some of the patches may still be a bit large.  Please
let me know if you'd like me to break any of them up.

Thanks in advance for the review!

Bill Schmidt (14):
  Initial create of rs6000-genbif.c.
  Add stubs for input files.  These will grow much larger.
  Add file support and functions for diagnostic support.
  Support functions to parse whitespace, lines, identifiers, integers.
  Add support functions for matching types.
  Red-black tree implementation for balanced tree search.
  Add main function with stub functions for parsing and output.
  Add support for parsing rs6000-bif.def.
  Add parsing support for rs6000-overload.def.
  Build function type identifiers and store them.
  Write #defines to rs6000-vecdefines.h.
  Write code to rs6000-bif.h.
  Write code to rs6000-bif.c.
  Incorporate new code into the build machinery.

 gcc/config.gcc|3 +-
 gcc/config/rs6000/rbtree.c|  233 +++
 gcc/config/rs6000/rbtree.h|   51 +
 gcc/config/rs6000/rs6000-bif.def  |  187 ++
 gcc/config/rs6000/rs6000-call.c   |   35 +
 gcc/config/rs6000/rs6000-genbif.c | 2295 +
 gcc/config/rs6000/rs6000-overload.def |5 +
 gcc/config/rs6000/t-rs6000|   22 +
 8 files changed, 2830 insertions(+), 1 deletion(-)
 create mode 100644 gcc/config/rs6000/rbtree.c
 create mode 100644 gcc/config/rs6000/rbtree.h
 create mode 100644 gcc/config/rs6000/rs6000-bif.def
 create mode 100644 gcc/config/rs6000/rs6000-genbif.c
 create mode 100644 gcc/config/rs6000/rs6000-overload.def

-- 
2.17.1

[PATCH 04/14] Support functions to parse whitespace, lines, identifiers, integers.

[Bill Schmidt]

2020-02-03  Bill Schmidt  

* config/rs6000/rs6000-genbif.c (MININT): New defined constant.
(exit_codes): New enum.
(consume_whitespace): New function.
(advance_line): New function.
(safe_inc_pos): New function.
(match_identifier): New function.
(match_integer): New function.
---
 gcc/config/rs6000/rs6000-genbif.c | 99 +++
 1 file changed, 99 insertions(+)

diff --git a/gcc/config/rs6000/rs6000-genbif.c 
b/gcc/config/rs6000/rs6000-genbif.c
index 3fb13cb11d6..197059cc2d2 100644
--- a/gcc/config/rs6000/rs6000-genbif.c
+++ b/gcc/config/rs6000/rs6000-genbif.c
@@ -123,6 +123,10 @@ along with GCC; see the file COPYING3.  If not see
 #include 
 #include 
 
+/* Used as a sentinel for range constraints on integer fields.  No field can
+   be 32 bits wide, so this is a safe sentinel value.  */
+#define MININT INT32_MIN
+
 /* Input and output file descriptors and pathnames.  */
 static FILE *bif_file;
 static FILE *ovld_file;
@@ -145,6 +149,11 @@ static char linebuf[LINELEN];
 static int line;
 static int pos;
 
+/* Exit codes for the shell.  */
+enum exit_codes {
+  EC_INTERR
+};
+
 /* Pointer to a diagnostic function.  */
 void (*diag) (const char *, ...) __attribute__ ((format (printf, 1, 2)))
   = NULL;
@@ -169,3 +178,93 @@ ovld_diag (const char * fmt, ...)
   vfprintf (stderr, fmt, args);
   va_end (args);
 }
+
+/* Pass over unprintable characters and whitespace (other than a newline,
+   which terminates the scan).  */
+static void
+consume_whitespace ()
+{
+  while (pos < LINELEN && isspace(linebuf[pos]) && linebuf[pos] != '\n')
+pos++;
+  return;
+}
+
+/* Get the next nonblank line, returning 0 on EOF, 1 otherwise.  */
+static int
+advance_line (FILE *file)
+{
+  while (1)
+{
+  /* Read ahead one line and check for EOF.  */
+  if (!fgets (linebuf, sizeof(linebuf), file))
+   return 0;
+  line++;
+  pos = 0;
+  consume_whitespace ();
+  if (linebuf[pos] != '\n')
+   return 1;
+}
+}
+
+static inline void
+safe_inc_pos ()
+{
+  if (pos++ >= LINELEN)
+{
+  (*diag) ("line length overrun.\n");
+  exit (EC_INTERR);
+}
+}
+
+/* Match an identifier, returning NULL on failure, else a pointer to a
+   buffer containing the identifier.  */
+static char *
+match_identifier ()
+{
+  int lastpos = pos - 1;
+  while (isalnum (linebuf[lastpos + 1]) || linebuf[lastpos + 1] == '_')
+if (++lastpos >= LINELEN - 1)
+  {
+   (*diag) ("line length overrun.\n");
+   exit (EC_INTERR);
+  }
+
+  if (lastpos < pos)
+return 0;
+
+  char *buf = (char *) malloc (lastpos - pos + 2);
+  memcpy (buf, &linebuf[pos], lastpos - pos + 1);
+  buf[lastpos - pos + 1] = '\0';
+
+  pos = lastpos + 1;
+  return buf;
+}
+
+/* Match an integer and return its value, or MININT on failure.  */
+static int
+match_integer ()
+{
+  int startpos = pos;
+  if (linebuf[pos] == '-')
+safe_inc_pos ();
+
+  int lastpos = pos - 1;
+  while (isdigit (linebuf[lastpos + 1]))
+if (++lastpos >= LINELEN - 1)
+  {
+   (*diag) ("line length overrun in match_integer.\n");
+   exit (EC_INTERR);
+  }
+
+  if (lastpos < pos)
+return MININT;
+
+  pos = lastpos + 1;
+  char *buf = (char *) malloc (lastpos - startpos + 2);
+  memcpy (buf, &linebuf[startpos], lastpos - startpos + 1);
+  buf[lastpos - startpos + 1] = '\0';
+
+  int x;
+  sscanf (buf, "%d", &x);
+  return x;
+}
-- 
2.17.1

[PATCH 02/14] Add stubs for input files. These will grow much larger.

[Bill Schmidt]

This patch adds a subset of the builtin and overload descriptions.
I've also started annotating the old-style descriptions in rs6000-c.c
where I'm deliberately not planning to support new versions of them.
We may have to have some discussion around these at some point, but
this helps me track this as I move through the transition.

2020-02-03  Bill Schmidt  

* config/rs6000/rs6000-bif.def: New file.
* config/rs6000/rs6000-call.c (altivec_overloaded_builtins):
Annotate some deprecated and bogus entries.
* config/rs6000/rs6000-overload.def: New file.
---
 gcc/config/rs6000/rs6000-bif.def  | 187 ++
 gcc/config/rs6000/rs6000-call.c   |  35 +
 gcc/config/rs6000/rs6000-overload.def |   5 +
 3 files changed, 227 insertions(+)
 create mode 100644 gcc/config/rs6000/rs6000-bif.def
 create mode 100644 gcc/config/rs6000/rs6000-overload.def

diff --git a/gcc/config/rs6000/rs6000-bif.def b/gcc/config/rs6000/rs6000-bif.def
new file mode 100644
index 000..85196400993
--- /dev/null
+++ b/gcc/config/rs6000/rs6000-bif.def
@@ -0,0 +1,187 @@
+[TARGET_ALTIVEC]
+  math vf __builtin_altivec_vmaddfp (vf, vf, vf);
+VMADDFP fmav4sf4 {}
+  vss __builtin_altivec_vmhaddshs (vss, vss, vss);
+VMHADDSHS altivec_vmhaddshs {}
+  vss __builtin_altivec_vmhraddshs (vss, vss, vss);
+VMHRADDSHS altivec_vmhraddshs {}
+  const vss __builtin_altivec_vmladduhm (vss, vss, vss);
+VMLADDUHM fmav8hi4 {}
+  const vui __builtin_altivec_vmsumubm (vuc, vuc, vui);
+VMSUMUBM altivec_vmsumubm {}
+  const vsi __builtin_altivec_vmsummbm (vsc, vuc, vsi);
+VMSUMMBM altivec_vmsummbm {}
+  const vui __builtin_altivec_vmsumuhm (vus, vus, vui);
+VMSUMUHM altivec_vmsumuhm {}
+  const vsi __builtin_altivec_vmsumshm (vss, vss, vsi);
+VMSUMSHM altivec_vmsumshm {}
+  vui __builtin_altivec_vmsumuhs (vus, vus, vui);
+VMSUMUHS altivec_vmsumuhs {}
+  vsi __builtin_altivec_vmsumshs (vss, vss, vsi);
+VMSUMSHS altivec_vmsumshs {}
+  math vf __builtin_altivec_vnmsubfp (vf, vf, vf);
+VNMSUBFP nfmsv4sf4 {}
+  const vsq __builtin_altivec_vperm_1ti (vsq, vsq, vuc);
+VPERM_1TI altivec_vperm_v1ti {}
+  const vf __builtin_altivec_vperm_4sf (vf, vf, vuc);
+VPERM_4SF altivec_vperm_v4sf {}
+  const vsi __builtin_altivec_vperm_4si (vsi, vsi, vuc);
+VPERM_4SI altivec_vperm_v4si {}
+  const vss __builtin_altivec_vperm_8hi (vss, vss, vuc);
+VPERM_8HI altivec_vperm_v8hi {}
+  const vsc __builtin_altivec_vperm_16qi (vsc, vsc, vuc);
+VPERM_16QI altivec_vperm_v16qi {}
+  const vuq __builtin_altivec_vperm_1ti_uns (vuq, vuq, vuc);
+VPERM_1TI_UNS altivec_vperm_v1ti_uns {}
+  const vui __builtin_altivec_vperm_4si_uns (vui, vui, vuc);
+VPERM_4SI_UNS altivec_vperm_v4si_uns {}
+  const vus __builtin_altivec_vperm_8hi_uns (vus, vus, vuc);
+VPERM_8HI_UNS altivec_vperm_v8hi_uns {}
+  const vuc __builtin_altivec_vperm_16qi_uns (vuc, vuc, vuc);
+VPERM_16QI_UNS altivec_vperm_v16qi_uns {}
+  const vf __builtin_altivec_vsel_4sf (vf, vf, vbi);
+VSEL_4SF_B vector_select_v4sf {}
+  const vf __builtin_altivec_vsel_4sf (vf, vf, vui);
+VSEL_4SF_U vector_select_v4sf {}
+  const vsi __builtin_altivec_vsel_4si (vsi, vsi, vbi);
+VSEL_4SI_B vector_select_v4si {}
+  const vsi __builtin_altivec_vsel_4si (vsi, vsi, vui);
+VSEL_4SI_U vector_select_v4si {}
+  const vui __builtin_altivec_vsel_4si (vui, vui, vbi);
+VSEL_4SI_UB vector_select_v4si {}
+  const vui __builtin_altivec_vsel_4si (vui, vui, vui);
+VSEL_4SI_UU vector_select_v4si {}
+  const vbi __builtin_altivec_vsel_4si (vbi, vbi, vbi);
+VSEL_4SI_BB vector_select_v4si {}
+  const vbi __builtin_altivec_vsel_4si (vbi, vbi, vui);
+VSEL_4SI_BU vector_select_v4si {}
+  const vss __builtin_altivec_vsel_8hi (vss, vss, vbs);
+VSEL_8HI_B vector_select_v8hi {}
+  const vss __builtin_altivec_vsel_8hi (vss, vss, vus);
+VSEL_8HI_U vector_select_v8hi {}
+  const vus __builtin_altivec_vsel_8hi (vus, vus, vbs);
+VSEL_8HI_UB vector_select_v8hi {}
+  const vus __builtin_altivec_vsel_8hi (vus, vus, vus);
+VSEL_8HI_UU vector_select_v8hi {}
+  const vbs __builtin_altivec_vsel_8hi (vbs, vbs, vbs);
+VSEL_8HI_BB vector_select_v8hi {}
+  const vbs __builtin_altivec_vsel_8hi (vbs, vbs, vus);
+VSEL_8HI_BU vector_select_v8hi {}
+  const vsc __builtin_altivec_vsel_16qi (vsc, vsc, vbc);
+VSEL_16QI_B vector_select_v16qi {}
+  const vsc __builtin_altivec_vsel_16qi (vsc, vsc, vuc);
+VSEL_16QI_U vector_select_v16qi {}
+  const vuc __builtin_altivec_vsel_16qi (vuc, vuc, vbc);
+VSEL_16QI_UB vector_select_v16qi {}
+  const vuc __builtin_altivec_vsel_16qi (vuc, vuc, vuc);
+VSEL_16QI_UU vector_select_v16qi {}
+  const vbc __builtin_altivec_vsel_16qi (vbc, vbc, vbc);
+VSEL_16QI_BB vector_select_v16qi {}
+  const vbc __builtin_altivec_vsel_16qi (vbc, vbc, vuc);
+VSEL_16QI_BU vector_select_v16qi {}
+  const vsq __builtin_altivec_vsel_1ti (vsq, vsq, vuq);
+VSEL_1TI vector_select

[PATCH 07/14] Add main function with stub functions for parsing and output.

[Bill Schmidt]

2020-02-03  Bill Schmidt  

* config/rs6000/rs6000-genbif.c (rbtree.h): New include.
(num_bif_stanzas): New filescope variable.
(num_bifs): Likewise.
(num_ovld_stanzas): Likewise.
(num_ovlds): Likewise.
(exit_codes): Add more enum values.
(bif_rbt): New filescope variable.
(ovld_rbt): Likewise.
(fntype_rbt): Likewise.
(parse_bif): New function stub.
(parse_ovld): Likewise.
(write_header_file): Likewise.
(write_init_file): Likewise.
(write_defines_file): Likewise.
(main): New function.
---
 gcc/config/rs6000/rs6000-genbif.c | 185 ++
 1 file changed, 185 insertions(+)

diff --git a/gcc/config/rs6000/rs6000-genbif.c 
b/gcc/config/rs6000/rs6000-genbif.c
index 7c1082fbe8f..38401224dce 100644
--- a/gcc/config/rs6000/rs6000-genbif.c
+++ b/gcc/config/rs6000/rs6000-genbif.c
@@ -122,6 +122,7 @@ along with GCC; see the file COPYING3.  If not see
 #include 
 #include 
 #include 
+#include "rbtree.h"
 
 /* Used as a sentinel for range constraints on integer fields.  No field can
be 32 bits wide, so this is a safe sentinel value.  */
@@ -155,6 +156,8 @@ enum void_status {
   VOID_OK
 };
 
+static int num_bif_stanzas;
+
 /* Legal base types for an argument or return type.  */
 enum basetype {
   BT_CHAR,
@@ -196,11 +199,33 @@ struct typeinfo {
   int val2;
 };
 
+static int num_bifs;
+static int num_ovld_stanzas;
+static int num_ovlds;
+
 /* Exit codes for the shell.  */
 enum exit_codes {
+  EC_OK,
+  EC_BADARGS,
+  EC_NOBIF,
+  EC_NOOVLD,
+  EC_NOHDR,
+  EC_NOINIT,
+  EC_NODEFINES,
+  EC_PARSEBIF,
+  EC_PARSEOVLD,
+  EC_WRITEHDR,
+  EC_WRITEINIT,
+  EC_WRITEDEFINES,
   EC_INTERR
 };
 
+/* The red-black trees for built-in function identifiers, built-in
+   overload identifiers, and function type descriptors.  */
+static rbt_strings bif_rbt;
+static rbt_strings ovld_rbt;
+static rbt_strings fntype_rbt;
+
 /* Pointer to a diagnostic function.  */
 void (*diag) (const char *, ...) __attribute__ ((format (printf, 1, 2)))
   = NULL;
@@ -721,3 +746,163 @@ match_type (typeinfo *typedata, int voidok)
   consume_whitespace ();
   return match_basetype (typedata);
 }
+
+/* Parse the built-in file.  Return 1 for success, 5 for a parsing failure.  */
+static int
+parse_bif ()
+{
+  return 1;
+}
+
+/* Parse the overload file.  Return 1 for success, 6 for a parsing error.  */
+static int
+parse_ovld ()
+{
+  return 1;
+}
+
+/* Write everything to the header file (rs6000-bif.h).  */
+static int
+write_header_file ()
+{
+  return 1;
+}
+
+/* Write everything to the initialization file (rs6000-bif.c).  */
+static int
+write_init_file ()
+{
+  return 1;
+}
+
+/* Write everything to the include file (rs6000-vecdefines.h).  */
+static int
+write_defines_file ()
+{
+  return 1;
+}
+
+/* Main program to convert flat files into built-in initialization code.  */
+int
+main (int argc, const char **argv)
+{
+  if (argc != 6)
+{
+  fprintf (stderr,
+  "Five arguments required: two input file and three output"
+  "files.\n");
+  exit (EC_BADARGS);
+}
+
+  pgm_path = argv[0];
+  bif_path = argv[1];
+  ovld_path = argv[2];
+  header_path = argv[3];
+  init_path = argv[4];
+  defines_path = argv[5];
+
+  bif_file = fopen (bif_path, "r");
+  if (!bif_file)
+{
+  fprintf (stderr, "Cannot find input built-in file '%s'.\n", bif_path);
+  exit (EC_NOBIF);
+}
+  ovld_file = fopen (ovld_path, "r");
+  if (!ovld_file)
+{
+  fprintf (stderr, "Cannot find input overload file '%s'.\n", ovld_path);
+  exit (EC_NOOVLD);
+}
+  header_file = fopen (header_path, "w");
+  if (!header_file)
+{
+  fprintf (stderr, "Cannot open header file '%s' for output.\n",
+  header_path);
+  exit (EC_NOHDR);
+}
+  init_file = fopen (init_path, "w");
+  if (!init_file)
+{
+  fprintf (stderr, "Cannot open init file '%s' for output.\n", init_path);
+  exit (EC_NOINIT);
+}
+  defines_file = fopen (defines_path, "w");
+  if (!defines_file)
+{
+  fprintf (stderr, "Cannot open defines file '%s' for output.\n",
+  defines_path);
+  exit (EC_NODEFINES);
+}
+
+  /* Initialize the balanced trees containing built-in function ids,
+ overload function ids, and function type declaration ids.  */
+  bif_rbt.rbt_nil = (rbt_string_node *) malloc (sizeof (rbt_string_node));
+  bif_rbt.rbt_nil->color = RBT_BLACK;
+  bif_rbt.rbt_root = bif_rbt.rbt_nil;
+
+  ovld_rbt.rbt_nil = (rbt_string_node *) malloc (sizeof (rbt_string_node));
+  ovld_rbt.rbt_nil->color = RBT_BLACK;
+  ovld_rbt.rbt_root = ovld_rbt.rbt_nil;
+
+  fntype_rbt.rbt_nil = (rbt_string_node *) malloc (sizeof (rbt_string_node));
+  fntype_rbt.rbt_nil->color = RBT_BLACK;
+  fntype_rbt.rbt_root = fntype_rbt.rbt_nil;
+
+  /* Parse the built-in function file.  */
+  num_bif_stanzas = 0;
+  num_bifs = 0;
+  line = 0;
+  if (parse_bif () != 1)
+{
+