Re: [patch] Fix type merging deficiency during WPA

[Eric Botcazou]

> I see that gimple_canonical_types_compatible_p winds up using
> operand_equal_p on expressions which represent the array size and that is
> why you need walk_simple_constant_arithmetic and operand_equal_p change.

Not just array size, but also offsets via gimple_compare_field_offset.

> I wonder how much code quality we would lose by simply treating only
> constantly sized arrays and considering all VLAs of the same type to be
> compatible? There are interesting issues WRT C standard and the canonical
> types of arrays have just secondary role (i.e. they are hardly used by
> themselves and only are used to compute canonical types of structures) and
> there may be lower hanging fruits in TBAA improvement than trying to handle
> sturctures containng VLAs right.

Yes, array types themselves can probably be kludged around, but the main issue 
in Ada are record types with dynamic offsets, which are first-class citizens.

-- 
Eric Botcazou

RE: [PATCH 1/7] SMS remove dependence on doloop: Use loop induction variable analysis in SMS pass

[Shiva Chen]

Hi, Jeff

Thanks for the tips.
I update the patches to pass regression test and x86 bootstrap when sms enabled 
as default.

Shiva

-Original Message-
From: Jeff Law [mailto:l...@redhat.com] 
Sent: Thursday, June 23, 2016 12:43 AM
To: Shiva Chen; GCC Patches; bschm...@redhat.com; Shiva Chen
Subject: Re: [PATCH 1/7] SMS remove dependence on doloop: Use loop induction 
variable analysis in SMS pass

On 05/05/2016 12:01 AM, Shiva Chen wrote:
> Hi,
>
> SMS transformation would change the kernel loop iteration count.
> To do this, SMS pass will find the register contain loop count and 
> generate the instructions to adjust loop count.
>
> Currently, SMS will try to find count_reg by recognizing doloop_end 
> pattern which means if the target didn't define doloop_end pattern or 
> the loop not suitable for doloop optimization, SMS pass won't active.
>
> The patch use induction variable analysis to find count_reg instead of 
> find doloop_end pattern.
So these patches need to be bootstrapped and regression tested.  Since SMS is 
not the default on any major platforms, I would recommend first hacking SMS to 
be enabled by default.  That isn't a patch you're going to submit, but instead 
it allows you to do bootstrap and regression testing on x86_64, ppc64 or 
whatever desktop/server machines you have access to.

I did that with patch #1 just to see what would happen and I get an assert 
triggered in generate_prolog_epilog:

   gcc_assert (REG_P (sub_reg));

Where sub_reg and count_reg are:

(subreg:SI (reg:DI 146 [ ivtmp.11 ]) 0)


AFAICT (reg:DI 146) is the actual IV, but the test actually occurs in
SImode:


(insn 80 79 82 7 (parallel [
 (set (reg:DI 146 [ ivtmp.11 ])
 (plus:DI (reg:DI 146 [ ivtmp.11 ])
 (const_int -1 [0x])))
 (clobber (reg:CC 17 flags))
 ]) 212 {*adddi_1}
  (expr_list:REG_UNUSED (reg:CC 17 flags)
 (nil)))

(insn 82 80 83 7 (set (reg:CCGOC 17 flags)
 (compare:CCGOC (subreg:SI (reg:DI 146 [ ivtmp.11 ]) 0)
 (const_int 0 [0]))) 
../../../../gcc/libstdc++-v3/libsupc++/hash_bytes.cc:54 3 {*cmpsi_ccno_1}
  (nil))

(jump_insn 83 82 84 7 (set (pc)
 (if_then_else (ge (reg:CCGOC 17 flags)
 (const_int 0 [0]))
 (label_ref:DI 81)
 (pc))) 
../../../../gcc/libstdc++-v3/libsupc++/hash_bytes.cc:54 635 {*jcc_1}
  (expr_list:REG_DEAD (reg:CCGOC 17 flags)
 (int_list:REG_BR_PROB 8500 (nil)))
  -> 81)


So you either need to be filtering out cases where the IV is in a 
different mode than its test or handling SUBREGs better throughout the code.

I haven't looked deeply at any of the patches yet.  I won't until 
they're in better shape from a correctness standpoint.

Jeff



0001-Use-loop-induction-variable-analysis-in-SMS-pass.patch
Description: 0001-Use-loop-induction-variable-analysis-in-SMS-pass.patch


0002-Use-get_simple_loop_desc-to-get-loop-iteration-count.patch
Description: 0002-Use-get_simple_loop_desc-to-get-loop-iteration-count.patch


0003-Update-loop-versioning-for-the-loop-with-step-1.patch
Description: 0003-Update-loop-versioning-for-the-loop-with-step-1.patch


0004-update-kernel-loop-iteration-adjustment-instruction-.patch
Description: 0004-update-kernel-loop-iteration-adjustment-instruction-.patch


0005-update-generate_prolog_epilog-for-the-case-without-d.patch
Description: 0005-update-generate_prolog_epilog-for-the-case-without-d.patch


0006-skip-the-loop-if-the-loop-size-too-small.patch
Description: 0006-skip-the-loop-if-the-loop-size-too-small.patch


0007-To-identify-READ_WRITE_REG-as-loop-induction-variabl.patch
Description: 0007-To-identify-READ_WRITE_REG-as-loop-induction-variabl.patch


0008-Fix-True-dependence-by-CC-register-wrap-around-and-c.patch
Description: 0008-Fix-True-dependence-by-CC-register-wrap-around-and-c.patch

Re: [PATCH] - improve sprintf buffer overflow detection (middle-end/49905)

[Florian Weimer]

On 07/01/2016 08:15 PM, Martin Sebor wrote:

diff --git a/gcc/passes.c b/gcc/passes.c
index 0565cfa..008799c 100644
--- a/gcc/passes.c
+++ b/gcc/passes.c
@@ -2425,8 +2425,15 @@ execute_pass_list_1 (opt_pass *pass)

   if (cfun == NULL)
return;
+
+  // inform (0, "executing pass: %s", pass->name);
+
   if (execute_one_pass (pass) && pass->sub)
-execute_pass_list_1 (pass->sub);
+   {
+ // inform (0, "executing subpass: %s", pass->sub->name);
+ execute_pass_list_1 (pass->sub);
+   }
+
   pass = pass->next;
 }
   while (pass);


This seems to be some leftover from debugging.

Florian

Re: Patch ping

[Richard Biener]

On Mon, 11 Jul 2016, Jakub Jelinek wrote:

> Hi!
> 
> I'd like to ping
> PR71716 - fix hang with long double atomics
> http://gcc.gnu.org/ml/gcc-patches/2016-07/msg00045.html

Ok.

Thanks,
Richard.

Re: [PATCH] Allow fwprop to undo vectorization harm (PR68961)

[Richard Biener]

On Sun, 10 Jul 2016, Uros Bizjak wrote:

> On Wed, Jul 6, 2016 at 3:18 PM, Richard Biener  wrote:
> 
> >> > 2016-07-04  Richard Biener  
> >> >
> >> > PR rtl-optimization/68961
> >> > * fwprop.c (propagate_rtx): Allow SUBREGs of VEC_CONCAT and CONCAT
> >> > to simplify to a non-constant.
> >> >
> >> > * gcc.target/i386/pr68961.c: New testcase.
> >>
> >> Thanks, LGTM.
> >
> > Bootstrapped and tested on x86_64-unknown-linux-gnu, it causes
> >
> > FAIL: gcc.target/i386/sse2-load-multi.c scan-assembler-times movup 2
> >
> > as the peephole created for that testcase no longer applies as fwprop
> > does
> >
> > In insn 10, replacing
> >  (vec_concat:V2DF (vec_select:DF (reg:V2DF 91)
> > (parallel [
> > (const_int 0 [0])
> > ]))
> > (mem:DF (reg/f:DI 95) [0  S8 A128]))
> >  with (vec_concat:V2DF (reg:DF 93 [ MEM[(const double *)&a + 8B] ])
> > (mem:DF (reg/f:DI 95) [0  S8 A128]))
> > Changed insn 10
> >
> > resulting in
> >
> > movsd   a+8(%rip), %xmm0
> > movhpd  a+16(%rip), %xmm0
> >
> > again rather than movupd.
> >
> > Uros, there is probably a missing peephole for the new form - can you
> > fix this as a followup or should I hold on this patch for a bit longer?
> 
> No, please proceed with the patch, I'll fix this fallout with a
> followup patch in a couple of days.

Applied as r238238.  Is the following x86 change ok then which
adjusts the vectorizer vector construction cost to sth more sensible?
I have adjusted the generic implementation in targhooks.c this way
a few weeks ago already.

Thanks,
Richard.

2016-07-12  Richard Biener  

* targhooks.c (default_builtin_vectorization_cost): Adjust
vec_construct cost.
* config/i386/i386.c (ix86_builtin_vectorization_cost): Likewise.

Index: gcc/config/i386/i386.c
===
--- gcc/config/i386/i386.c  (revision 237196)
+++ gcc/config/i386/i386.c  (working copy)
@@ -49503,8 +49520,6 @@ static int
 ix86_builtin_vectorization_cost (enum vect_cost_for_stmt type_of_cost,
  tree vectype, int)
 {
-  unsigned elements;
-
   switch (type_of_cost)
 {
   case scalar_stmt:
@@ -49546,8 +49561,7 @@ ix86_builtin_vectorization_cost (enum ve
 return ix86_cost->vec_stmt_cost;
 
   case vec_construct:
-   elements = TYPE_VECTOR_SUBPARTS (vectype);
-   return ix86_cost->vec_stmt_cost * (elements / 2 + 1);
+   return ix86_cost->vec_stmt_cost * (TYPE_VECTOR_SUBPARTS (vectype) - 1);
 
   default:
 gcc_unreachable ();

Re: [PATCH] Allow fwprop to undo vectorization harm (PR68961)

[Uros Bizjak]

On Tue, Jul 12, 2016 at 10:58 AM, Richard Biener  wrote:
> On Sun, 10 Jul 2016, Uros Bizjak wrote:
>
>> On Wed, Jul 6, 2016 at 3:18 PM, Richard Biener  wrote:
>>
>> >> > 2016-07-04  Richard Biener  
>> >> >
>> >> > PR rtl-optimization/68961
>> >> > * fwprop.c (propagate_rtx): Allow SUBREGs of VEC_CONCAT and CONCAT
>> >> > to simplify to a non-constant.
>> >> >
>> >> > * gcc.target/i386/pr68961.c: New testcase.
>> >>
>> >> Thanks, LGTM.
>> >
>> > Bootstrapped and tested on x86_64-unknown-linux-gnu, it causes
>> >
>> > FAIL: gcc.target/i386/sse2-load-multi.c scan-assembler-times movup 2
>> >
>> > as the peephole created for that testcase no longer applies as fwprop
>> > does
>> >
>> > In insn 10, replacing
>> >  (vec_concat:V2DF (vec_select:DF (reg:V2DF 91)
>> > (parallel [
>> > (const_int 0 [0])
>> > ]))
>> > (mem:DF (reg/f:DI 95) [0  S8 A128]))
>> >  with (vec_concat:V2DF (reg:DF 93 [ MEM[(const double *)&a + 8B] ])
>> > (mem:DF (reg/f:DI 95) [0  S8 A128]))
>> > Changed insn 10
>> >
>> > resulting in
>> >
>> > movsd   a+8(%rip), %xmm0
>> > movhpd  a+16(%rip), %xmm0
>> >
>> > again rather than movupd.
>> >
>> > Uros, there is probably a missing peephole for the new form - can you
>> > fix this as a followup or should I hold on this patch for a bit longer?
>>
>> No, please proceed with the patch, I'll fix this fallout with a
>> followup patch in a couple of days.
>
> Applied as r238238.  Is the following x86 change ok then which
> adjusts the vectorizer vector construction cost to sth more sensible?
> I have adjusted the generic implementation in targhooks.c this way
> a few weeks ago already.
>
> Thanks,
> Richard.
>
> 2016-07-12  Richard Biener  
>
> * targhooks.c (default_builtin_vectorization_cost): Adjust
> vec_construct cost.
> * config/i386/i386.c (ix86_builtin_vectorization_cost): Likewise.

Looks OK to me, but let's also give Intel chance to comment.

Uros.

> Index: gcc/config/i386/i386.c
> ===
> --- gcc/config/i386/i386.c  (revision 237196)
> +++ gcc/config/i386/i386.c  (working copy)
> @@ -49503,8 +49520,6 @@ static int
>  ix86_builtin_vectorization_cost (enum vect_cost_for_stmt type_of_cost,
>   tree vectype, int)
>  {
> -  unsigned elements;
> -
>switch (type_of_cost)
>  {
>case scalar_stmt:
> @@ -49546,8 +49561,7 @@ ix86_builtin_vectorization_cost (enum ve
>  return ix86_cost->vec_stmt_cost;
>
>case vec_construct:
> -   elements = TYPE_VECTOR_SUBPARTS (vectype);
> -   return ix86_cost->vec_stmt_cost * (elements / 2 + 1);
> +   return ix86_cost->vec_stmt_cost * (TYPE_VECTOR_SUBPARTS (vectype) - 
> 1);
>
>default:
>  gcc_unreachable ();

Re: [PATCH] Allow fwprop to undo vectorization harm (PR68961)

[Richard Biener]

On Tue, 12 Jul 2016, Uros Bizjak wrote:

> On Tue, Jul 12, 2016 at 10:58 AM, Richard Biener  wrote:
> > On Sun, 10 Jul 2016, Uros Bizjak wrote:
> >
> >> On Wed, Jul 6, 2016 at 3:18 PM, Richard Biener  wrote:
> >>
> >> >> > 2016-07-04  Richard Biener  
> >> >> >
> >> >> > PR rtl-optimization/68961
> >> >> > * fwprop.c (propagate_rtx): Allow SUBREGs of VEC_CONCAT and CONCAT
> >> >> > to simplify to a non-constant.
> >> >> >
> >> >> > * gcc.target/i386/pr68961.c: New testcase.
> >> >>
> >> >> Thanks, LGTM.
> >> >
> >> > Bootstrapped and tested on x86_64-unknown-linux-gnu, it causes
> >> >
> >> > FAIL: gcc.target/i386/sse2-load-multi.c scan-assembler-times movup 2
> >> >
> >> > as the peephole created for that testcase no longer applies as fwprop
> >> > does
> >> >
> >> > In insn 10, replacing
> >> >  (vec_concat:V2DF (vec_select:DF (reg:V2DF 91)
> >> > (parallel [
> >> > (const_int 0 [0])
> >> > ]))
> >> > (mem:DF (reg/f:DI 95) [0  S8 A128]))
> >> >  with (vec_concat:V2DF (reg:DF 93 [ MEM[(const double *)&a + 8B] ])
> >> > (mem:DF (reg/f:DI 95) [0  S8 A128]))
> >> > Changed insn 10
> >> >
> >> > resulting in
> >> >
> >> > movsd   a+8(%rip), %xmm0
> >> > movhpd  a+16(%rip), %xmm0
> >> >
> >> > again rather than movupd.
> >> >
> >> > Uros, there is probably a missing peephole for the new form - can you
> >> > fix this as a followup or should I hold on this patch for a bit longer?
> >>
> >> No, please proceed with the patch, I'll fix this fallout with a
> >> followup patch in a couple of days.
> >
> > Applied as r238238.  Is the following x86 change ok then which
> > adjusts the vectorizer vector construction cost to sth more sensible?
> > I have adjusted the generic implementation in targhooks.c this way
> > a few weeks ago already.
> >
> > Thanks,
> > Richard.
> >
> > 2016-07-12  Richard Biener  
> >
> > * targhooks.c (default_builtin_vectorization_cost): Adjust
> > vec_construct cost.
> > * config/i386/i386.c (ix86_builtin_vectorization_cost): Likewise.
> 
> Looks OK to me, but let's also give Intel chance to comment.

Btw, the motivation is that the cost of large initializers like for
v16qi or v32qi is underestimated currently.  You end up with
15 or 31 vinsert calls (or similar with other ISAs) and you can't do
better than elements - 1 operations.  It doesn't really matter
for smaller vectors of course (seen for CPU v6 x264)

Richard.

Re: [PATCH] disable IPA-cp cloning for functions with target_clones attribute

[Martin Jambor]

Hi,

On Fri, Jun 24, 2016 at 01:41:05PM -0700, Evgeny Stupachenko wrote:
> Hi,
> 
> Fix ICE when IPA-cp and target_clones are applied to the same function.
> Is the patch ok for trunk?

I can't approve anything but since I wrote most of IPA-CP, it may
count that I am fine with the patch.

However, it should also be backported to the 6 branch.

In the future, it is useful to file a bugzilla PR for issues like
this, even if you also provide a fix.  It helps with tracking
backports and with a PR, you would have probably caught my attention
sooner.

In any event, thanks for addressing this.

Martin

> 
> Thanks,
> Evgeny
> 
> 2016-06-24  Evgeny Stupachenko  
> 
> gcc/
> * ipa-cp.c (determine_versionability): Do not create constprop clones,
> when target_clones attribute is set.
> diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
> index 2710494..4b642ba 100644
> --- a/gcc/ipa-cp.c
> +++ b/gcc/ipa-cp.c
> @@ -533,6 +533,13 @@ determine_versionability (struct cgraph_node *node,
>  coexist, but that may not be worth the effort.  */
>reason = "function has SIMD clones";
>  }
> +  else if (lookup_attribute ("target_clones", DECL_ATTRIBUTES (node->decl)))
> +{
> +  /* Ideally we should clone the target clones themselves and create
> +copies of them, so IPA-cp and target clones can happily
> +coexist, but that may not be worth the effort.  */
> +  reason = "function target_clones attribute";
> +}
>/* Don't clone decls local to a comdat group; it breaks and for C++
>  decloned constructors, inlining is always better anyway.  */
>else if (node->comdat_local_p ())
> diff --git a/gcc/testsuite/gcc.target/i386/mvc8.c
> b/gcc/testsuite/gcc.target/i386/mvc8.c
> new file mode 100644
> index 000..e9ab9e1
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/mvc8.c
> @@ -0,0 +1,14 @@
> +/* { dg-do compile } */
> +/* { dg-require-ifunc "" } */
> +/* { dg-options "-O3 -fno-inline" } */
> +/* { dg-final { scan-assembler-not "constprop" } } */
> +__attribute__((target_clones("arch=core-avx2","arch=slm","default")))
> +void foo (float *a, int b) {
> +*a = (float)b;
> +}
> +float a;
> +int main() {
> +  int i;
> +  for (i = 0; i < 1024; i++)
> +foo (&a, 5);
> +}

[PATCH] Lift alignment restrictions from inlining register size memcpy

[Richard Biener]

The following patch does $subject which is requested in a comment in
PR50417 as this restriction defeats the purpose of having memcpy
as a portable and efficient way to circumvent strict-aliasing violations
(or even as a portable and efficient way to do unaligned loads).

Bootstrap / regtest running on x86_64-unknown-linux-gnu (quite pointless
as there is no change on that arch).

I have added a testcase that should exercise most relevant cases so
we can look for fallout on "interesting" targets.

Boostrap / regtest on strict-alignment platforms welcome.

I do expect that with -Os expanding unaligned-unaligned moves
might be a size pessimization compared to a libcall (or what
the targets block move expander does).  But the whole point is
to remove abstraction penalty so it isn't an easy stmt-local
decision to make.  Comments on this front welcome as well.

Unless I hear some positives I'll let the patch sit here as I
don't really care too much about those pesky targets (and
targets can choose to opt-in by providing movmisalign optabs
anyway so they don't go the store/extract_bit_field path).

Thanks,
Richard.

2016-07-12  Richard Biener  

* gimple-fold.c (gimple_fold_builtin_memory_op): Lift alignment
restrictions from inlining register size memcpy.

* gcc.dg/torture/builtin-memcpy.c: New testcase.

Index: gcc/gimple-fold.c
===
*** gcc/gimple-fold.c   (revision 238237)
--- gcc/gimple-fold.c   (working copy)
*** gimple_fold_builtin_memory_op (gimple_st
*** 705,718 
  if (type
  && TYPE_MODE (type) != BLKmode
  && (GET_MODE_SIZE (TYPE_MODE (type)) * BITS_PER_UNIT
! == ilen * 8)
! /* If the destination pointer is not aligned we must be able
!to emit an unaligned store.  */
! && (dest_align >= GET_MODE_ALIGNMENT (TYPE_MODE (type))
! || !SLOW_UNALIGNED_ACCESS (TYPE_MODE (type), dest_align)
! || (optab_handler (movmisalign_optab, TYPE_MODE (type))
! != CODE_FOR_nothing)))
{
  tree srctype = type;
  tree desttype = type;
  if (src_align < GET_MODE_ALIGNMENT (TYPE_MODE (type)))
--- 705,718 
  if (type
  && TYPE_MODE (type) != BLKmode
  && (GET_MODE_SIZE (TYPE_MODE (type)) * BITS_PER_UNIT
! == ilen * 8))
{
+ /* RTL expansion handles misaligned destination / source
+MEM_REFs either via target provided movmisalign or
+via extract/store_bit_field for targets that set
+SLOW_UNALIGNED_ACCESS for the move.  For move
+quantities up to MOVE_MAX this should be always
+more efficient than a libcall to memcpy.  */
  tree srctype = type;
  tree desttype = type;
  if (src_align < GET_MODE_ALIGNMENT (TYPE_MODE (type)))
*** gimple_fold_builtin_memory_op (gimple_st
*** 721,767 
  tree tem = fold_const_aggregate_ref (srcmem);
  if (tem)
srcmem = tem;
! else if (src_align < GET_MODE_ALIGNMENT (TYPE_MODE (type))
!  && SLOW_UNALIGNED_ACCESS (TYPE_MODE (type),
!src_align)
!  && (optab_handler (movmisalign_optab,
! TYPE_MODE (type))
!  == CODE_FOR_nothing))
!   srcmem = NULL_TREE;
! if (srcmem)
{
! gimple *new_stmt;
! if (is_gimple_reg_type (TREE_TYPE (srcmem)))
!   {
! new_stmt = gimple_build_assign (NULL_TREE, srcmem);
! if (gimple_in_ssa_p (cfun))
!   srcmem = make_ssa_name (TREE_TYPE (srcmem),
!   new_stmt);
! else
!   srcmem = create_tmp_reg (TREE_TYPE (srcmem));
! gimple_assign_set_lhs (new_stmt, srcmem);
! gimple_set_vuse (new_stmt, gimple_vuse (stmt));
! gsi_insert_before (gsi, new_stmt, GSI_SAME_STMT);
!   }
! if (dest_align < GET_MODE_ALIGNMENT (TYPE_MODE (type)))
!   desttype = build_aligned_type (type, dest_align);
! new_stmt
!   = gimple_build_assign (fold_build2 (MEM_REF, desttype,
!   dest, off0),
!  srcmem);
  gimple_set_v

Re: [PATCH] - improve sprintf buffer overflow detection (middle-end/49905)

[Florian Weimer]

On 07/01/2016 08:15 PM, Martin Sebor wrote:

The attached patch enhances compile-time checking for buffer overflow
and output truncation in non-trivial calls to the sprintf family of
functions under a new option -Wformat-length=[12].  This initial
patch handles printf directives with string, integer, and simple
floating arguments but eventually I'd like to extend it all other
functions and directives for which it makes sense.


I tried your patch with the following code, which is close to a 
real-world example:


#include 

void print (const char *);

void
format_1 (unsigned address)
{
  unsigned char a = address >> 24;
  unsigned char b = address >> 16;
  unsigned char c = address >> 8;
  unsigned char d = address;
  char buf[15];
  sprintf ("%u.%u.%u.%u", buf, a, b, c, d);
  print (buf);
}

void
format_2 (unsigned address)
{
  char buf[15];
  sprintf ("%u.%u.%u.%u", buf,
   address >> 24,
   (address >> 16) & 0xff,
   (address >> 8) & 0xff,
   address & 0xff);
  print (buf);
}

I didn't get a warning (with -O2 and -Wformat-length=1 or 
-Wformat-length=2).  If the warning is implemented in builtin folding, I 
guess this has to be expected because there is no range information, and 
warning for all %us similar to those in the example would produce too 
many false positives.


Florian

Re: [PATCH] - improve sprintf buffer overflow detection (middle-end/49905)

[Jakub Jelinek]

On Tue, Jul 12, 2016 at 11:51:50AM +0200, Florian Weimer wrote:
> On 07/01/2016 08:15 PM, Martin Sebor wrote:
> >The attached patch enhances compile-time checking for buffer overflow
> >and output truncation in non-trivial calls to the sprintf family of
> >functions under a new option -Wformat-length=[12].  This initial
> >patch handles printf directives with string, integer, and simple
> >floating arguments but eventually I'd like to extend it all other
> >functions and directives for which it makes sense.
> 
> I tried your patch with the following code, which is close to a real-world
> example:
> 
> #include 
> 
> void print (const char *);
> 
> void
> format_1 (unsigned address)
> {
>   unsigned char a = address >> 24;
>   unsigned char b = address >> 16;
>   unsigned char c = address >> 8;
>   unsigned char d = address;
>   char buf[15];
>   sprintf ("%u.%u.%u.%u", buf, a, b, c, d);

Are you sure this is real-world code?  sprintf's first argument is the
buffer and second the format string, so if this doesn't warn at compile
time, it will surely crash at runtime when trying to store into .rodata.

Jakub

Re: [PATCH] - improve sprintf buffer overflow detection (middle-end/49905)

[Florian Weimer]

On 07/12/2016 11:54 AM, Jakub Jelinek wrote:

On Tue, Jul 12, 2016 at 11:51:50AM +0200, Florian Weimer wrote:

On 07/01/2016 08:15 PM, Martin Sebor wrote:

The attached patch enhances compile-time checking for buffer overflow
and output truncation in non-trivial calls to the sprintf family of
functions under a new option -Wformat-length=[12].  This initial
patch handles printf directives with string, integer, and simple
floating arguments but eventually I'd like to extend it all other
functions and directives for which it makes sense.


I tried your patch with the following code, which is close to a real-world
example:

#include 

void print (const char *);

void
format_1 (unsigned address)
{
  unsigned char a = address >> 24;
  unsigned char b = address >> 16;
  unsigned char c = address >> 8;
  unsigned char d = address;
  char buf[15];
  sprintf ("%u.%u.%u.%u", buf, a, b, c, d);


Are you sure this is real-world code?  sprintf's first argument is the
buffer and second the format string, so if this doesn't warn at compile
time, it will surely crash at runtime when trying to store into .rodata.


Argh!  You are right, I swapped the arguments.

And further attempts showed that I was missing -D_FORTIFY_SOURCE=2. 
With it, I get a nice diagnostic.  Wow!


Florian

Re: [PATCH 2/2] Optimize fortran loops with +-1 step.

[Richard Biener]

On Sun, Jul 10, 2016 at 9:37 AM, Andreas Schwab  wrote:
> marxin  writes:
>
>> diff --git a/gcc/testsuite/gfortran.dg/ldist-1.f90 
>> b/gcc/testsuite/gfortran.dg/ldist-1.f90
>> index ea3990d..2030328 100644
>> --- a/gcc/testsuite/gfortran.dg/ldist-1.f90
>> +++ b/gcc/testsuite/gfortran.dg/ldist-1.f90
>> @@ -32,4 +32,4 @@ end Subroutine PADEC
>>  ! There are 5 legal partitions in this code.  Based on the data
>>  ! locality heuristic, this loop should not be split.
>>
>> -! { dg-final { scan-tree-dump-not "distributed: split to" "ldist" } }
>> +! { dg-final { scan-tree-dump "distributed: split to" "ldist" } }
>
> FAIL: gfortran.dg/ldist-1.f90   -O   scan-tree-dump ldist "distributed: split 
> to"

That change is certainly spurious (not in ChangeLog) and bogus given the
comment before the scan.

Richard.

> Andreas.
>
> --
> Andreas Schwab, sch...@linux-m68k.org
> GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
> "And now for something completely different."

Re: [PATCH, PR ipa/71633] Fix inlining into thunks

[Richard Biener]

On Mon, Jul 11, 2016 at 12:22 PM, Ilya Enkovich  wrote:
> Hi,
>
> Currently when we expand thunk in inliner we assume its body
> has a single call.  This is wrong for cases when thunk is
> instrumented.  It means we might try to continue inlining
> for wrong edge.  This simple patch fixes it.
>
> Bootstrapped and regtested on x86_64-unknown-linux-gnu.
> OK for trunk?

Ok.

Thanks,
Richard.

> Thanks,
> Ilya
> --
> gcc/
>
> 2016-07-11  Ilya Enkovich  
>
> PR ipa/71633
> * ipa-inline-transform.c (inline_call): Support
> instrumented thunks.
>
> gcc/testsuite/
>
> 2016-07-11  Ilya Enkovich  
>
> PR ipa/71633
> * g++.dg/pr71633.C: New test.
>
>
> diff --git a/gcc/ipa-inline-transform.c b/gcc/ipa-inline-transform.c
> index 9ac1efc..a4ae305 100644
> --- a/gcc/ipa-inline-transform.c
> +++ b/gcc/ipa-inline-transform.c
> @@ -319,10 +319,14 @@ inline_call (struct cgraph_edge *e, bool 
> update_original,
>  to = to->global.inlined_to;
>if (to->thunk.thunk_p)
>  {
> +  struct cgraph_node *target = to->callees->callee;
>if (in_lto_p)
> to->get_untransformed_body ();
>to->expand_thunk (false, true);
> -  e = to->callees;
> +  /* When thunk is instrumented we may have multiple callees.  */
> +  for (e = to->callees; e && e->callee != target; e = e->next_callee)
> +   ;
> +  gcc_assert (e);
>  }
>
>
> diff --git a/gcc/testsuite/g++.dg/pr71633.C b/gcc/testsuite/g++.dg/pr71633.C
> new file mode 100644
> index 000..bb69bbb
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/pr71633.C
> @@ -0,0 +1,28 @@
> +/* PR71633 */
> +// { dg-do compile { target i?86-*-* x86_64-*-* } }
> +/* { dg-options "-fcheck-pointer-bounds -mmpx -O2" } */
> +
> +class c1
> +{
> +  virtual void fn1 ();
> +};
> +
> +class c2
> +{
> +  virtual int *fn2 () const;
> +};
> +
> +class c3 : c1, c2
> +{
> +  int *fn2 () const;
> +  int *fn3 (int) const;
> +};
> +
> +int *c3::fn2 () const
> +{
> +}
> +
> +int *c3::fn3 (int p) const
> +{
> +  return fn3 (p);
> +}

Re: [PATCH] Fix Fortran DO loop fallback

[Richard Biener]

On Mon, Jul 11, 2016 at 4:44 PM, Jeff Law  wrote:
> On 07/08/2016 08:26 AM, Martin Liška wrote:
>>
>> Hello
>>
>> Following patch fixes fallout caused by the patch set:
>> https://gcc.gnu.org/ml/gcc-regression/2016-07/msg00097.html
>>
>> Ready after it finished regression tests?
>> Thanks,
>> Martin
>>
>>
>> 0001-Fix-Fortran-DO-loop-fallback.patch
>>
>>
>> From c5dd7ad62f795cce560c7f1bb8767b7ed9298d8a Mon Sep 17 00:00:00 2001
>> From: marxin 
>> Date: Fri, 8 Jul 2016 15:51:54 +0200
>> Subject: [PATCH] Fix Fortran DO loop fallback
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2016-07-08  Martin Liska  
>>
>> * gfortran.dg/ldist-1.f90: Update expected dump scan.
>> * gfortran.dg/pr42108.f90: Likewise.
>> * gfortran.dg/vect/pr62283.f: Likewise.
>
> Shouldn't ldist-1.f90 be scan-tree-dump-not?  It seems like you change it
> from that just last week, but it wasn't mentioned in the ChangeLog.

gfortran.dg/pr42108.f90 also looks pointless now?  I suppose there is nothing
to hoist after the change?  Do we now have an option to revert back to old
behavior?  If so it would be better to use that flag here.

diff --git a/gcc/testsuite/gfortran.dg/vect/pr62283.f
b/gcc/testsuite/gfortran.dg/vect/pr62283.f
index 7df3d99..2933f51 100644
--- a/gcc/testsuite/gfortran.dg/vect/pr62283.f
+++ b/gcc/testsuite/gfortran.dg/vect/pr62283.f
@@ -13,4 +13,4 @@ C { dg-additional-options "-fvect-cost-model=dynamic" }
   beta=3.141593
   y=y+beta*x
   end
-C { dg-final { scan-tree-dump-times "vectorized 1 loops" 2 "vect" {
target { vect_hw_misalign } } } }
+C { dg-final { scan-tree-dump "vectorized 1 loops" "vect" { target {
vect_hw_misalign } } } }

so why do we no longer vectorize 1 loops in two functions?  The
testcase works for me
unchanged.

Richard.

> OK with that change.
>
> jeff
>
>

RE: [PATCH] Fix FFI return type for closures in the java interpreter

[Matthew Fortune]

Tom Tromey  writes:
> > "Matthew" == Matthew Fortune  writes:
> 
> Matthew> I'm not sure this will matter if the only arch is x86 as
> Matthew> ffi_arg will be 32-bit anyway there.
> 
> Aha, right.  Thanks for looking.
> 
> Matthew> There would need to be a
> Matthew> 64bit arch using the raw api. I don't really understand what
> Matthew> the raw api is, the references to it in the code seemed
> Matthew> cryptic.
> 
> IIRC it's to exploit the x86 calling convention to make ffi calls a bit
> more efficient for libgcj.

Sorry for the long delay...

I have tested this now with -m32 multilib on x86_64-pc-linux-gnu and there
are no regressions.

> Matthew> libjava/
> Matthew>  * interpret-run.cc: Use ffi_arg for FFI integer return types.
> Matthew> libjava/testsuite/
> Matthew>  * libjava.jar/arraysort.java: New file.
> Matthew>  * libjava.jar/arraysort.jar: New file.
> Matthew>  * libjava.jar/arraysort.out: New file.
> Matthew>  * libjava.jar/arraysort.xfail: New file.
>
> This is ok.
> Could you check?  I think a -m32 build ought to show it.  Maybe your
> x86-64 build already did this?

Still OK to commit?

Thanks,
Matthew

Re: Fix loop_only_exit_p

[Richard Biener]

On Mon, 11 Jul 2016, Jan Hubicka wrote:

> Hi,
> while looking into loop code I noticed that loop_only_exit_p seems overly
> simplistic. The purpose of this predicate is to decide whether the loop
> must terminate by the exit given (so for example it is known that the number
> of iteration test in that particular exit won't overflow because undefined
> effect will happen then).
> 
> The code checks for calls with side effects, but it misses the fact that loop
> can exit by an external EH (with -fnon-call-exceptions) or by an asm 
> statement.
> Exactly the same problem is being solved by the gcov code where it is 
> necessary
> which tries to verify that given the first statement of bb is executed, the
> control flow will continue by one of the outgoing edges (which is necesssary
> to make Kirhoff law applicable).
> 
> This patch thus unifies the logic and also fixes probably ages long bug about
> ignoring external EH in profiling code.
> 
> Bootstrapped/regtested x86_64-linux, seems sane?

I'd rather not expose/change need_fake_edge_p as that has a very
specific purpose.

Why don't you simply add a call to is_ctrl_altering_stmt on the
last stmt of the block in loop_only_exit_p?  It's a waste of
time doing stuff on every stmt that can only make a difference
on the last one of a BB.

Richard.

> Honza
> 
>   * gimple.h (stmt_can_terminate_bb_p): New function.
>   * tree-cfg.c (need_fake_edge_p): Rename to ...
>   (stmt_can_terminate_bb_p): ... this; return true if stmt can
>   throw external; handle const and pure calls.
>   * tree-ssa-loop-niter.c (loop_only_exit_p): Use it.
> Index: gimple.h
> ===
> --- gimple.h  (revision 238191)
> +++ gimple.h  (working copy)
> @@ -1526,6 +1526,7 @@ extern void gimple_seq_set_location (gim
>  extern void gimple_seq_discard (gimple_seq);
>  extern void maybe_remove_unused_call_args (struct function *, gimple *);
>  extern bool gimple_inexpensive_call_p (gcall *);
> +extern bool stmt_can_terminate_bb_p (gimple *);
>  
>  /* Formal (expression) temporary table handling: multiple occurrences of
> the same scalar expression are evaluated into the same temporary.  */
> Index: tree-cfg.c
> ===
> --- tree-cfg.c(revision 238191)
> +++ tree-cfg.c(working copy)
> @@ -7919,15 +7919,20 @@ gimple_block_ends_with_condjump_p (const
>  }
>  
>  
> -/* Return true if we need to add fake edge to exit at statement T.
> -   Helper function for gimple_flow_call_edges_add.  */
> +/* Return true if statement T may terminate execution of BB in ways not
> +   explicitly represtented in the CFG.  */
>  
> -static bool
> -need_fake_edge_p (gimple *t)
> +bool
> +stmt_can_terminate_bb_p (gimple *t)
>  {
>tree fndecl = NULL_TREE;
>int call_flags = 0;
>  
> +  /* Eh exception not handled internally terminates execution of the whole
> + function.  */
> +  if (stmt_can_throw_external (t))
> +return true;
> +
>/* NORETURN and LONGJMP calls already have an edge to exit.
>   CONST and PURE calls do not need one.
>   We don't currently check for CONST and PURE here, although
> @@ -7960,6 +7965,13 @@ need_fake_edge_p (gimple *t)
>edge e;
>basic_block bb;
>  
> +  if (call_flags & (ECF_PURE | ECF_CONST)
> +   && !(call_flags & ECF_LOOPING_CONST_OR_PURE))
> + return false;
> +
> +  /* Function call may do longjmp, terminate program or do other things.
> +  Special case noreturn that have non-abnormal edges out as in this case
> +  the fact is sufficiently represented by lack of edges out of T.  */
>if (!(call_flags & ECF_NORETURN))
>   return true;
>  
> @@ -8024,7 +8036,7 @@ gimple_flow_call_edges_add (sbitmap bloc
>if (!gsi_end_p (gsi))
>   t = gsi_stmt (gsi);
>  
> -  if (t && need_fake_edge_p (t))
> +  if (t && stmt_can_terminate_bb_p (t))
>   {
> edge e;
>  
> @@ -8059,7 +8071,7 @@ gimple_flow_call_edges_add (sbitmap bloc
> do
>   {
> stmt = gsi_stmt (gsi);
> -   if (need_fake_edge_p (stmt))
> +   if (stmt_can_terminate_bb_p (stmt))
>   {
> edge e;
>  
> Index: tree-ssa-loop-niter.c
> ===
> --- tree-ssa-loop-niter.c (revision 238191)
> +++ tree-ssa-loop-niter.c (working copy)
> @@ -2159,7 +2159,6 @@ loop_only_exit_p (const struct loop *loo
>basic_block *body;
>gimple_stmt_iterator bsi;
>unsigned i;
> -  gimple *call;
>  
>if (exit != single_exit (loop))
>  return false;
> @@ -2168,17 +2167,8 @@ loop_only_exit_p (const struct loop *loo
>for (i = 0; i < loop->num_nodes; i++)
>  {
>for (bsi = gsi_start_bb (body[i]); !gsi_end_p (bsi); gsi_next (&bsi))
> - {
> -   call = gsi_stmt (bsi);
> -   if (gimple_code (call) != GIMPLE_CALL)
> - cont

Re: Implement -Wswitch-fallthrough: lto

[Richard Biener]

On Mon, Jul 11, 2016 at 9:58 PM, Marek Polacek  wrote:
> 2016-07-11  Marek Polacek  
>
> PR c/7652
> * lto-plugin.c (lto_fallthrough): Define.
> (parse_table_entry): Use it.
>
> diff --git gcc/lto-plugin/lto-plugin.c gcc/lto-plugin/lto-plugin.c
> index 51afc52..ffdf54a 100644
> --- gcc/lto-plugin/lto-plugin.c
> +++ gcc/lto-plugin/lto-plugin.c
> @@ -77,6 +77,12 @@ along with this program; see the file COPYING3.  If not see
>  # define O_BINARY 0
>  #endif
>
> +#if __GNUC__ >= 7
> +# define lto_fallthrough() __builtin_fallthrough ()
> +#else
> +# define lto_fallthrough()
> +#endif
> +
>  /* Segment name for LTO sections.  This is only used for Mach-O.
> FIXME: This needs to be kept in sync with darwin.c.  */
>
> @@ -254,6 +260,7 @@ parse_table_entry (char *p, struct ld_plugin_symbol 
> *entry,
>   break;
> }
>  /* FALL-THROUGH.  */
> +lto_fallthrough ();

This shows me that it would be nice to simply get this correct by
parsing comments...

I don't like __builtin_fallthrough () too much.

Richard.

>  case ss_uscore:
>entry->name = concat ("_", p, NULL);
>break;

Re: [PATCH, ARM 6/7, ping1] Add support for CB(N)Z and (U|S)DIV to ARMv8-M Baseline

[Thomas Preudhomme]

Hi Kyrill,

On Friday 20 May 2016 14:22:48 Kyrill Tkachov wrote:
> Hi Thomas,
> 
> On 17/05/16 11:14, Thomas Preudhomme wrote:
> > Ping?
> > 
> > *** gcc/ChangeLog ***
> > 
> > 2015-11-13  Thomas Preud'homme  
> > 
> >  * config/arm/arm.c (arm_print_operand_punct_valid_p): Make %?
> >  valid
> >  for Thumb-1.
> >  * config/arm/arm.h (TARGET_HAVE_CBZ): Define.
> >  (TARGET_IDIV): Set for all Thumb targets provided they have
> >  hardware
> >  divide feature.
> >  * config/arm/thumb1.md (thumb1_cbz): New insn.
> > 
> > diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
> > index
> > f42e996e5a7ce979fe406b8261d50fb2ba005f6b..347b5b0a5cc0bc1e3b5020c8124d968e
> > 76ce48a4 100644
> > --- a/gcc/config/arm/arm.h
> > +++ b/gcc/config/arm/arm.h
> > @@ -271,9 +271,12 @@ extern void (*arm_lang_output_object_attributes_hook)
> > (void);
> > 
> >   /* Nonzero if this chip provides the movw and movt instructions.  */
> >   #define TARGET_HAVE_MOVT  (arm_arch_thumb2 || arm_arch8)
> > 
> > +/* Nonzero if this chip provides the cb{n}z instruction.  */
> > +#define TARGET_HAVE_CBZ(arm_arch_thumb2 || arm_arch8)
> > +
> > 
> >   /* Nonzero if integer division instructions supported.  */
> >   #define TARGET_IDIV   ((TARGET_ARM && arm_arch_arm_hwdiv) \
> > 
> > -|| (TARGET_THUMB2 && arm_arch_thumb_hwdiv))
> > +|| (TARGET_THUMB && arm_arch_thumb_hwdiv))
> > 
> >   /* Nonzero if disallow volatile memory access in IT block.  */
> >   #define TARGET_NO_VOLATILE_CE (arm_arch_no_volatile_ce)
> > 
> > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> > index
> > 13b4b71ac8f9c1da8ef1945f7ff6985ca59f6832..445972ce0e3fd27d4411840ff69e9edb
> > b23994fc 100644
> > --- a/gcc/config/arm/arm.c
> > +++ b/gcc/config/arm/arm.c
> > @@ -22684,7 +22684,7 @@ arm_print_operand_punct_valid_p (unsigned char
> > code)> 
> >   {
> >   
> > return (code == '@' || code == '|' || code == '.'
> > 
> >   || code == '(' || code == ')' || code == '#'
> > 
> > - || (TARGET_32BIT && (code == '?'))
> > + || code == '?'
> > 
> >   || (TARGET_THUMB2 && (code == '!'))
> >   || (TARGET_THUMB && (code == '_')));
> >   
> >   }
> 
> Hmm, I'm not a fan of this change. arm_print_operand_punct_valid_p is an
> implementation of a target hook that is used to validate user-provided
> inline asm as well and is therefore the right place to reject such invalid
> constructs.
> 
> This is just working around the fact that the output template for the
> [u]divsi3 patterns has a '%?' in it that is illegal in Thumb1 and will not
> be used for ARMv8-M Baseline anyway. I'd prefer it if you add a second
> alternative to those patterns and emit the sdiv/udiv mnemonic without the
> '%?' and enable that for the v8mb arch attribute (and mark the existing
> alternative as requiring the "32" arch attribute).
> > diff --git a/gcc/config/arm/thumb1.md b/gcc/config/arm/thumb1.md
> > index
> > 4572456b8bc98503061846cad94bc642943db3a2..1b01ef6ce731fe3ff37c3d8c048fb9d5
> > e7829b35 100644
> > --- a/gcc/config/arm/thumb1.md
> > +++ b/gcc/config/arm/thumb1.md
> > @@ -973,6 +973,92 @@
> > 
> > DONE;
> >   
> >   })
> > 
> > +;; A pattern for the cb(n)z instruction added in ARMv8-M baseline
> > profile,
> > +;; adapted from cbranchsi4_insn.  Modifying cbranchsi4_insn instead leads
> > to +;; code generation difference for ARMv6-M because the minimum length
> > of the +;; instruction becomes 2 even for it due to a limitation in
> > genattrtab's +;; handling of pc in the length condition.
> > +(define_insn "thumb1_cbz"
> > +  [(set (pc) (if_then_else
> > + (match_operator 0 "equality_operator"
> > +  [(match_operand:SI 1 "s_register_operand" "l")
> > +   (const_int 0)])
> > + (label_ref (match_operand 2 "" ""))
> > + (pc)))]
> > +  "TARGET_THUMB1 && TARGET_HAVE_MOVT"
> > +{
> 
> s/TARGET_HAVE_MOVT/TARGET_HAVE_CBZ/
> 
> > +  if (get_attr_length (insn) == 2)
> > +{
> > +  if (GET_CODE (operands[0]) == EQ)
> > +   return "cbz\t%1, %l2";
> > +  else
> > +   return "cbnz\t%1, %l2";
> > +}
> > +  else
> > +{
> > +  rtx t = cfun->machine->thumb1_cc_insn;
> > +  if (t != NULL_RTX)
> > +   {
> > + if (!rtx_equal_p (cfun->machine->thumb1_cc_op0, operands[1])
> > + || !rtx_equal_p (cfun->machine->thumb1_cc_op1, operands[2]))
> > +   t = NULL_RTX;
> > + if (cfun->machine->thumb1_cc_mode == CC_NOOVmode)
> > +   {
> > + if (!noov_comparison_operator (operands[0], VOIDmode))
> > +   t = NULL_RTX;
> > +   }
> > + else if (cfun->machine->thumb1_cc_mode != CCmode)
> > +   t = NULL_RTX;
> > +   }
> > +  if (t == NULL_RTX)
> > +   {
> > + output_asm_insn ("cmp\t%1, #0", operands);
> > + cfun->machine->thumb1_cc_insn = insn;
> > + cfun->machine->thumb1_cc_op0 = operands[1];
> > + cfun->machine->thumb1_cc_op1 

Re: Implement -Wswitch-fallthrough

[Richard Biener]

On Mon, Jul 11, 2016 at 9:43 PM, Marek Polacek  wrote:
> The switch fallthrough has been widely considered a design defect in C, a
> misfeature or, to use Marshall Cline's definition, evil.  The overwhelming
> majority of the time you don't want to fall through to the next case, but it 
> is
> easy to forget to "break" at the end of the case, making this far too error
> prone.  Yet GCC (and probably other compilers, too) doesn't have the ability 
> to
> warn in this case.  A feature request for such warning was opened back in 
> 2002,
> but it's mostly been untouched since.  But then the [[fallthrough]] attribute 
> was
> approved for C++17 [1], and that's what has got me to do all this.

I don't like this too much given the churn it requires in GCC itself.
If [[fallthrough]]
was approved for C++17 is there sth similar proposed for C?  Like a keyword
__Fallthrough?

Richard.

> The following series attempts to implement the fabled -Wswitch-fallthrough
> warning.  The warning would be quite easy to implement were it not for control
> statements, nested scopes, gotos, and such.  I took great pains to try to
> handle all of that before I succeeded in getting all the pieces in place.  So
> GCC ought to do the right thing for e.g.:
>
>   switch (n)
> {
> case 1:
>   if (n > 20)
> break;
>   else if (n > 10)
> {
>   foo (9);
>   __builtin_fallthrough ();
> }
>   else
> foo (8); // warn here
> case 2:;
> }
>
> Since approximately 3% of fall throughs are desirable, I added a new builtin,
> __builtin_fallthrough, that prevents the warning from occurring.  It can only
> be used in a switch; the compiler will issue an error otherwise.  The new C++
> attribute can then be implemented in terms of this builtin.  There was an idea
> floating around about not warning for /* FALLTHRU */ etc. comments but I
> rejected this after I'd spent time thinking about it: the preprocessor can't
> know that we're in a switch statement so we might reject valid programs, the
> "falls through" comments vary wildly, occur even in if-statements, etc.  The
> warning doesn't warn when the block is empty, e.g. on case 1: case 2: ...  The
> warning does, inevitably, trigger on Duff's devices.  Too bad.  I guess I'd
> suggest using #pragma GCC warning then.  (I think Perl uses the "continue"
> keyword for exactly this purpose -- an interesting idea...)
>
> After I'd completed the warning, I kicked off a bootstrap so as to add various
> gcc_fallthrough calls.  There were a good amount of them, as expected.  I also
> grepped various FALLTHRU/Falls through/...fall thru.../... comments in config/
> and added gcc_fallthroughs to make it easier for people.  Wherever I wasn't
> sure that the gcc_fallthrough was appropriate, I added an 'XXX' comment.  This
> must not be relied upon as I don't know most of the compiler code.  The same
> goes for relevant libraries where I introduced various gomp_fallthrough
> macros conditional on __GNUC__ (I'd tried to use a configure check but then
> decided I won't put up with all the vagaries of autoconf).
>
> This patch is accompanied by more than 2000 lines of new tests to get the
> warning covered though I'm sure folks will come up with other test cases
> that I hadn't considered (hi Martin S. ;).
>
> This warning is enabled by default for C/C++.  I was more inclined to put this
> into -Wall, but our common.opt machinery doesn't seem to allow that (ugh!).
> The clang version I have installed doesn't have this warning so I couldn't
> compare my implementation with others.
>
> I plan to plunge into the C++ [[fallthrough]] thingie after this core part is
> dealt with.
>
> Since the patch is too large, I split it into various parts, the core and then
> various __builtin_fallthrough additions.
>
> This patch has been tested on powerpc64le-unknown-linux-gnu, 
> aarch64-linux-gnu,
> x86_64-redhat-linux, and also on powerpc-ibm-aix7.1.0.0, though due to PR71816
> I was unable to bootstrap.
>
> Thanks,
>
> [1] http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0188r0.pdf
>
> Marek

Re: Implement -Wswitch-fallthrough: lto

[Marek Polacek]

On Tue, Jul 12, 2016 at 12:22:55PM +0200, Richard Biener wrote:
> > @@ -254,6 +260,7 @@ parse_table_entry (char *p, struct ld_plugin_symbol 
> > *entry,
> >   break;
> > }
> >  /* FALL-THROUGH.  */
> > +lto_fallthrough ();
> 
> This shows me that it would be nice to simply get this correct by
> parsing comments...
 
Yea, seems that became a prerequisite for moving this forward.  Not sure if
I'll be able to manage that, but let me try what Jakub and David suggested
(turn the commend into a pragma).

> I don't like __builtin_fallthrough () too much.

Understood.

Marek

Re: [PATCH], PowerPC support to enable -mlra and/or -mfloat128

[Richard Biener]

On Mon, Jul 11, 2016 at 10:07 PM, Michael Meissner
 wrote:
> These configuration switches will allow the PowerPC GCC developers to switch
> defaults in the compiler to debug the code, before making the decision to flip
> the default permanently.  In the future, when the defaults have been changed,
> these configuration options would allow developers to go back to the previous
> versions without modifying the code using the --disable- form.
>
> The first option is --enable-lra, which changes the compiler so that the
> default is to use the LRA register allocator instead of the older RELOAD
> allocator. The PowerPC team would like to switch the default, but there is a
> critical bug in LRA that must be fixed before we can change the default:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69847
>
> The second option is --enable-float128, which changes the compiler so that the
> default for VSX systems is to enable the __float128 keyword to allow users
> access to the IEEE 128-bit floating point implementation without having to use
> the keyword.
>
> Both of these switches are debug switches, and are not meant to be used by
> non-developers.
>
> The --enable-lra swich causes the following tests to fail:
>
> * testsuite/gcc.target/powerpc/bool3-p7.c
> * testsuite/gcc.target/powerpc/bool3-p8.c
>
> See bug 71846 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71846) for more
> details.
>
> The --enable-float128 switch causes libquadmath and libstdc++ to fail because
> we do not yet have enough of the support in the compiler to allow these
> libraries to build.  It is our intention, that we will use the
> --enable-float128 option and work on getting the libraries fixed.  If I build
> just a C compiler and disable building libquadmath, there are no regressions 
> in
> the C tests with __float128 enabled or disabled.
>
> Can I check these options into the trunk as non-default options?

Instead of adding more configury can we please enable LRA on trunk by default
_now_?  Otherwise the amount of testing it recieves won't really increase.

Richard.

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

Re: Implement -Wswitch-fallthrough

[Marek Polacek]

On Tue, Jul 12, 2016 at 12:27:31PM +0200, Richard Biener wrote:
> On Mon, Jul 11, 2016 at 9:43 PM, Marek Polacek  wrote:
> > The switch fallthrough has been widely considered a design defect in C, a
> > misfeature or, to use Marshall Cline's definition, evil.  The overwhelming
> > majority of the time you don't want to fall through to the next case, but 
> > it is
> > easy to forget to "break" at the end of the case, making this far too error
> > prone.  Yet GCC (and probably other compilers, too) doesn't have the 
> > ability to
> > warn in this case.  A feature request for such warning was opened back in 
> > 2002,
> > but it's mostly been untouched since.  But then the [[fallthrough]] 
> > attribute was
> > approved for C++17 [1], and that's what has got me to do all this.
> 
> I don't like this too much given the churn it requires in GCC itself.
> If [[fallthrough]]
> was approved for C++17 is there sth similar proposed for C?  Like a keyword
> __Fallthrough?

I don't think there is.

Marek

[PATCH] Fix FFI return type for proxy classes

[Matthew Fortune]

Hi,

As mentioned in:

https://gcc.gnu.org/ml/gcc-patches/2016-06/msg01827.html

there is a bug in lang/reflect/natVMProxy.cc where the return types
are not promoted to ffi_arg for integer types smaller than a word.

This bug will not show up on little endian architectures as the issue
gets fixed by _Jv_CallAnyMethodA which casts the (corrupted) return
value down to the correct type again effectively discarding the upper
corrupt bits. On big endian this does not work as the valid bits are
in the wrong position so the casts in _Jv_CallAnyMethodA just make
a bad value worse.

Many thanks to Tom for explaining how to trigger the affected code.
I've added a test that has proxied functions returning each of the
integer types to cover this issue. All being well that will be
sufficient to prevent regressions in this area as it was somewhat
tricky to get to the root of this! I hope the coding style of the
test is OK, I don't know the rules for java formatting in GNU
projects.

Tested on: x86_64-pc-linux-gnu (default and -m32), mips-linux-gnu
mipsel-linux-gnuabi64 with no regressions. The new test only failed
on mips-linux-gnu prior to patching libjava.

Thanks,
Matthew

libjava/
* java/lang/reflect/natVMProxy.cc (unbox): Use ffi_arg for
integer return types smaller than a word.
* testsuite/libjava.jar/ReturnInvocationHandler.java: New file.
* testsuite/libjava.jar/ReturnProxyTest.jar: Likewise.
* testsuite/libjava.jar/ReturnProxyTest.java: Likewise.
* testsuite/libjava.jar/ReturnProxyTest.out: Likewise.
* testsuite/libjava.jar/ReturnProxyTest.xfail: Likewise.
* testsuite/libjava.jar/ReturnTypes.java: Likewise.
* testsuite/libjava.jar/ReturnTypesImpl.java: Likewise.
---
 libjava/java/lang/reflect/natVMProxy.cc|  10 
 .../libjava.jar/ReturnInvocationHandler.java   |  24 ++
 libjava/testsuite/libjava.jar/ReturnProxyTest.jar  | Bin 0 -> 2671 bytes
 libjava/testsuite/libjava.jar/ReturnProxyTest.java |  27 +
 libjava/testsuite/libjava.jar/ReturnProxyTest.out  |  12 +
 .../testsuite/libjava.jar/ReturnProxyTest.xfail|   1 +
 libjava/testsuite/libjava.jar/ReturnTypes.java |   9 +++
 libjava/testsuite/libjava.jar/ReturnTypesImpl.java |  27 +
 8 files changed, 105 insertions(+), 5 deletions(-)
 create mode 100644 libjava/testsuite/libjava.jar/ReturnInvocationHandler.java
 create mode 100644 libjava/testsuite/libjava.jar/ReturnProxyTest.jar
 create mode 100644 libjava/testsuite/libjava.jar/ReturnProxyTest.java
 create mode 100644 libjava/testsuite/libjava.jar/ReturnProxyTest.out
 create mode 100644 libjava/testsuite/libjava.jar/ReturnProxyTest.xfail
 create mode 100644 libjava/testsuite/libjava.jar/ReturnTypes.java
 create mode 100644 libjava/testsuite/libjava.jar/ReturnTypesImpl.java

diff --git a/libjava/java/lang/reflect/natVMProxy.cc 
b/libjava/java/lang/reflect/natVMProxy.cc
index e46263d..19cde20 100644
--- a/libjava/java/lang/reflect/natVMProxy.cc
+++ b/libjava/java/lang/reflect/natVMProxy.cc
@@ -272,17 +272,17 @@ unbox (jobject o, jclass klass, void *rvalue, FFI_TYPE 
type)
   if (klass == JvPrimClass (byte))
 {
   _Jv_CheckCast (&Byte::class$, o);
-  *(jbyte*)rvalue = ((Byte*)o)->byteValue();
+  *(ffi_arg*)rvalue = ((Byte*)o)->byteValue();
 }
   else if (klass == JvPrimClass (short))
 {
   _Jv_CheckCast (&Short::class$, o);
-  *(jshort*)rvalue = ((Short*)o)->shortValue();
+  *(ffi_arg*)rvalue = ((Short*)o)->shortValue();
 }
   else if (klass == JvPrimClass (int))
 {
   _Jv_CheckCast (&Integer::class$, o);
-  *(jint*)rvalue = ((Integer*)o)->intValue();
+  *(ffi_arg*)rvalue = ((Integer*)o)->intValue();
 }
   else if (klass == JvPrimClass (long))
 {
@@ -302,12 +302,12 @@ unbox (jobject o, jclass klass, void *rvalue, FFI_TYPE 
type)
   else if (klass == JvPrimClass (boolean))
 {
   _Jv_CheckCast (&Boolean::class$, o);
-  *(jboolean*)rvalue = ((Boolean*)o)->booleanValue();
+  *(ffi_arg*)rvalue = ((Boolean*)o)->booleanValue();
 }
   else if (klass == JvPrimClass (char))
 {
   _Jv_CheckCast (&Character::class$, o);
-  *(jchar*)rvalue = ((Character*)o)->charValue();
+  *(ffi_arg*)rvalue = ((Character*)o)->charValue();
 }
   else
 JvFail ("Bad ffi type in proxy");
diff --git a/libjava/testsuite/libjava.jar/ReturnInvocationHandler.java 
b/libjava/testsuite/libjava.jar/ReturnInvocationHandler.java
new file mode 100644
index 000..18b52b7
--- /dev/null
+++ b/libjava/testsuite/libjava.jar/ReturnInvocationHandler.java
@@ -0,0 +1,24 @@
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Method;
+
+public class ReturnInvocationHandler implements InvocationHandler
+{
+  private Object obj;
+  public ReturnInvocationHandler(Object obj)
+  {
+this.obj = obj;
+  }
+  public Object invoke(Object proxy, Method m, Object[] args) throws Thr

Re: [PATCHv2, ARM, libgcc] New aeabi_idiv function for armv6-m

[Andre Vieira (lists)]

On 11/07/16 18:09, Andre Vieira (lists) wrote:
> On 06/07/16 11:52, Andre Vieira (lists) wrote:
>> On 01/07/16 14:40, Ramana Radhakrishnan wrote:
>>>
>>>
>>> On 13/10/15 18:01, Andre Vieira wrote:
 This patch ports the aeabi_idiv routine from Linaro Cortex-Strings 
 (https://git.linaro.org/toolchain/cortex-strings.git), which was 
 contributed by ARM under Free BSD license.

 The new aeabi_idiv routine is used to replace the one in 
 libgcc/config/arm/lib1funcs.S. This replacement happens within the Thumb1 
 wrapper. The new routine is under LGPLv3 license.
>>>
>>> This is not under LGPLv3 . It is under GPLv3 with the runtime library 
>>> exception license, there's a difference. Assuming your licensing 
>>> expectation is ok  read on for more of a review.
>>>

 The main advantage of this version is that it can improve the performance 
 of the aeabi_idiv function for Thumb1. This solution will also increase 
 the code size. So it will only be used if __OPTIMIZE_SIZE__ is not defined.

 Make check passed for armv6-m.

 libgcc/ChangeLog:
 2015-08-10  Hale Wang  
 Andre Vieira  

   * config/arm/lib1funcs.S: Add new wrapper.

 0001-integer-division.patch


 From 832a3d6af6f06399f70b5a4ac3727d55960c93b7 Mon Sep 17 00:00:00 2001
 From: Andre Simoes Dias Vieira 
 Date: Fri, 21 Aug 2015 14:23:28 +0100
 Subject: [PATCH] new wrapper idivmod

 ---
  libgcc/config/arm/lib1funcs.S | 250 
 --
  1 file changed, 217 insertions(+), 33 deletions(-)

 diff --git a/libgcc/config/arm/lib1funcs.S b/libgcc/config/arm/lib1funcs.S
 index 
 252efcbd5385cc58a5ce1e48c6816d36a6f4c797..c9e544114590da8cde88382bea0f67206e593816
  100644
 --- a/libgcc/config/arm/lib1funcs.S
 +++ b/libgcc/config/arm/lib1funcs.S
 @@ -306,34 +306,12 @@ LSYM(Lend_fde):
  #ifdef __ARM_EABI__
  .macro THUMB_LDIV0 name signed
  #if defined(__ARM_ARCH_6M__)
 -  .ifc \signed, unsigned
 -  cmp r0, #0
 -  beq 1f
 -  mov r0, #0
 -  mvn r0, r0  @ 0x
 -1:
 -  .else
 -  cmp r0, #0
 -  beq 2f
 -  blt 3f
 +
 +  push{r0, lr}
mov r0, #0
 -  mvn r0, r0
 -  lsr r0, r0, #1  @ 0x7fff
 -  b   2f
 -3:mov r0, #0x80
 -  lsl r0, r0, #24 @ 0x8000
 -2:
 -  .endif
 -  push{r0, r1, r2}
 -  ldr r0, 4f
 -  adr r1, 4f
 -  add r0, r1
 -  str r0, [sp, #8]
 -  @ We know we are not on armv4t, so pop pc is safe.
 -  pop {r0, r1, pc}
 -  .align  2
 -4:
 -  .word   __aeabi_idiv0 - 4b
 +  bl  SYM(__aeabi_idiv0)
 +  pop {r1, pc}
 +
>>>
>>> I'd still retain the comment about pop pc here because there's often a 
>>> misconception of merging armv4t and armv6m code.
>>>
  #elif defined(__thumb2__)
.syntax unified
.ifc \signed, unsigned
 @@ -945,7 +923,170 @@ LSYM(Lover7):
add dividend, work
.endif
  LSYM(Lgot_result):
 -.endm 
 +.endm
 +
 +#if defined(__prefer_thumb__) && !defined(__OPTIMIZE_SIZE__)
 +/* If performance is preferred, the following functions are provided.  */
 +
>>>
>>> Comment above #if please and also check elsewhere in patch.
>>>
 +/* Branch to div(n), and jump to label if curbit is lo than divisior.  */
 +.macro BranchToDiv n, label
 +  lsr curbit, dividend, \n
 +  cmp curbit, divisor
 +  blo \label
 +.endm
 +
 +/* Body of div(n).  Shift the divisor in n bits and compare the divisor
 +   and dividend.  Update the dividend as the substruction result.  */
 +.macro DoDiv n
 +  lsr curbit, dividend, \n
 +  cmp curbit, divisor
 +  bcc 1f
 +  lsl curbit, divisor, \n
 +  sub dividend, dividend, curbit
 +
 +1:adc result, result
 +.endm
 +
 +/* The body of division with positive divisor.  Unless the divisor is very
 +   big, shift it up in multiples of four bits, since this is the amount of
 +   unwinding in the main division loop.  Continue shifting until the 
 divisor
 +   is larger than the dividend.  */
 +.macro THUMB1_Div_Positive
 +  mov result, #0
 +  BranchToDiv #1, LSYM(Lthumb1_div1)
 +  BranchToDiv #4, LSYM(Lthumb1_div4)
 +  BranchToDiv #8, LSYM(Lthumb1_div8)
 +  BranchToDiv #12, LSYM(Lthumb1_div12)
 +  BranchToDiv #16, LSYM(Lthumb1_div16)
 +LSYM(Lthumb1_div_large_positive):
 +  mov result, #0xff
 +  lsl divisor, divisor, #8
 +  rev result, result
 +  lsr curbit, dividend, #16
 +  cmp curbit, divisor
 +  blo 1f
 +  asr result, #8
 +  lsl divisor, divisor, #8
 +  beq LSYM(Ldivbyzero_

Re: [PATCH, rs6000] Fix PR target/71733, ICE with -mcpu=power9 -mno-vsx

[Alan Modra]

On Mon, Jul 11, 2016 at 03:26:46PM +0200, Ulrich Weigand wrote:
> However, there still seems to be another underlying problem: reload
> should never generate invalid instructions.  (Playing with '?' to
> fix *inefficient* instructions is fine, but we shouldn't rely on '?'
> to fix *invalid* instructions.)
> 
> If reload -for whatever reason- wants to emit a vr->gpr move, then it
> should generate a valid instruction sequence to do so, via secondary
> reloads if necessary.  I think it would be a good idea to investigate
> why that isn't happening in this test case.  [ There is a chance that
> the underlying bug will reappear in a different context, even after
> the '?' change is applied. ]

The vr->gpr move uses secondary memory.  The memory allocated by
assign_stack_local, called from get_secondary_memory is

(mem/c:V16QI (plus:DI (reg/f:DI 113 sfp)
(const_int 32 [0x20])) [0  S16 A128])

and that is incorrectly simplified by eliminate_regs to

(mem/c:V16QI (reg/f:DI 1 1))

for use by the strict_memory_address_space_p check, which of course
then passes.

eliminate_regs sees this entry in the elimination table:
(gdb) p *ep
$7 = {from = 113, to = 1, initial_offset = -32, can_eliminate = 1, 
can_eliminate_previous = 1, offset = -32, previous_offset = -32, 
ref_outside_mem = 0, from_rtx = 0x7688a300, to_rtx = 0x7688a2e8}

The offset in the elimination table was correct before
get_secondary_memory called assign_stack_local, but not after
frame_offset was changed when allocating a new stack slot.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [patch] Fix type merging deficiency during WPA

[Richard Biener]

On Mon, Jul 11, 2016 at 3:28 PM, Eric Botcazou  wrote:
>> So sth like
>>
>> Index: gcc/lto-streamer-out.c
>> ===
>> --- gcc/lto-streamer-out.c  (revision 238039)
>> +++ gcc/lto-streamer-out.c  (working copy)
>> @@ -996,7 +996,7 @@ hash_tree (struct streamer_tree_cache_d
>>else
>>  hstate.add_flag (TREE_NO_WARNING (t));
>>hstate.add_flag (TREE_NOTHROW (t));
>> -  hstate.add_flag (TREE_STATIC (t));
>> +  //hstate.add_flag (TREE_STATIC (t));
>>hstate.add_flag (TREE_PROTECTED (t));
>>hstate.add_flag (TREE_DEPRECATED (t));
>>if (code != TREE_BINFO)
>> @@ -1050,7 +1050,7 @@ hash_tree (struct streamer_tree_cache_d
>>hstate.add_flag (DECL_ARTIFICIAL (t));
>>hstate.add_flag (DECL_USER_ALIGN (t));
>>hstate.add_flag (DECL_PRESERVE_P (t));
>> -  hstate.add_flag (DECL_EXTERNAL (t));
>> +  //hstate.add_flag (DECL_EXTERNAL (t));
>>hstate.add_flag (DECL_GIMPLE_REG_P (t));
>>hstate.commit_flag ();
>>hstate.add_int (DECL_ALIGN (t));
>> Index: gcc/lto/lto.c
>> ===
>> --- gcc/lto/lto.c   (revision 238039)
>> +++ gcc/lto/lto.c   (working copy)
>> @@ -1263,7 +1263,8 @@ compare_tree_sccs_1 (tree t1, tree t2, t
>>  tree t1_ = (E1), t2_ = (E2); \
>>  if (t1_ != t2_ \
>> && (!t1_ || !t2_ \
>> -   || !TREE_VISITED (t2_) \
>> +   || (!TREE_VISITED (t2_) \
>> +   && !same_decls (t1_, t2_)) \
>>
>> || (!TREE_ASM_WRITTEN (t2_) \
>>
>> && !compare_tree_sccs_1 (t1_, t2_, map \
>>return false; \
>>
>> plus the magic new function same_decls which would check if the trees are
>> var/function decls with the same assembler name. The function can't use
>> the cgraph as that is not yet read in unfortunately - I still think we
>> should do that so we can see the prevailing nodes early.
>
> OK, thanks for the hint, the attached patch where the call to same_decls is
> placed differently works on the testcase (modulo the gigi change I posted
> earlier today) but not on the original code...  It looks like the hash value
> computed in lto-streamer-out.c is stricter than the hash value computed for
> canonical types, I'm trying to pinpoint the difference.

As tree merging really replaces trees it has to error on the side of not merging
while canonical type merging has to error on the side of "merging" to make
types alias.

Btw, for the LTO_SET_PREVAIL can you introduce a LTO_SET_PREVAIL_EXPR
and use that for the fields that possibly can be expressions plus simply use
walk_tree for those (in case they are EXPR_P)?  Please split that part
out as well.

Richard.


>
> * lto-streamer-out.c (hash_tree): Do not add the TREE_STATIC and
> DECL_EXTERNAL flags for variable or function declarations.
> lto/
> * lto.c (same_global_decl_p): New predicate.
> (compare_tree_sccs_1): Use it in the comparison of pointer fields.
> (walk_simple_constant_arithmetic): New function.
> (LTO_SET_PREVAIL): Use it to fix up DECL nodes in simple expressions.
>
> --
> Eric Botcazou

Re: [patch] Fix type merging deficiency during WPA

[Richard Biener]

On Mon, Jul 11, 2016 at 5:14 PM, Jan Hubicka  wrote:
> Hi,
> Sorry for jumping in late, only now I had chance to read through the whole 
> discussion.
> I was looking into similar problem some time ago.
>
>> Index: lto-streamer-out.c
>> ===
>> --- lto-streamer-out.c(revision 238156)
>> +++ lto-streamer-out.c(working copy)
>> @@ -996,7 +996,9 @@ hash_tree (struct streamer_tree_cache_d
>>else
>>  hstate.add_flag (TREE_NO_WARNING (t));
>>hstate.add_flag (TREE_NOTHROW (t));
>> -  hstate.add_flag (TREE_STATIC (t));
>> +  /* We want to unify DECL nodes in pointer fields of global types.  */
>> +  if (!(VAR_OR_FUNCTION_DECL_P (t)))
>> +hstate.add_flag (TREE_STATIC (t));
>>hstate.add_flag (TREE_PROTECTED (t));
>>hstate.add_flag (TREE_DEPRECATED (t));
>>if (code != TREE_BINFO)
>> @@ -1050,7 +1052,9 @@ hash_tree (struct streamer_tree_cache_d
>>hstate.add_flag (DECL_ARTIFICIAL (t));
>>hstate.add_flag (DECL_USER_ALIGN (t));
>>hstate.add_flag (DECL_PRESERVE_P (t));
>> -  hstate.add_flag (DECL_EXTERNAL (t));
>> +  /* We want to unify DECL nodes in pointer fields of global types.  */
>> +  if (!(VAR_OR_FUNCTION_DECL_P (t)))
>> + hstate.add_flag (DECL_EXTERNAL (t));
>
> It is fine to merge decls across static and external flags, but I am not sure 
> this is
> a safe solution to the problem. In C it is perfectly normal to have one decl 
> more specified
> or with different attributes.  Like:
>
> extern int a __attribute__ ((warning("bar"));
>
> int a=7 __attribute__ ((warning("foo")));
>
> which must prevent merging otherwise the warnings will go out wrong way. In 
> GCC
> 6 timeframe we decided to live with duplicated declarations and added support
> for syntactic aliases. Tree merging should be optional WRT correctness, so I 
> think
> bug is in the canonical type calculation:
>
>   /* For array types hash the domain bounds and the string flag.  */
>   if (TREE_CODE (type) == ARRAY_TYPE && TYPE_DOMAIN (type))
> {
>   hstate.add_int (TYPE_STRING_FLAG (type));
>   /* OMP lowering can introduce error_mark_node in place of
>  random local decls in types.  */
>   if (TYPE_MIN_VALUE (TYPE_DOMAIN (type)) != error_mark_node)
> inchash::add_expr (TYPE_MIN_VALUE (TYPE_DOMAIN (type)), hstate);
>   if (TYPE_MAX_VALUE (TYPE_DOMAIN (type)) != error_mark_node)
> inchash::add_expr (TYPE_MAX_VALUE (TYPE_DOMAIN (type)), hstate);
> }
>
> which boils down to:
>
>   if (tclass == tcc_declaration)
> {
>   /* DECL's have a unique ID */
>   hstate.add_wide_int (DECL_UID (t));
> }
>
> (in asd_expr)

Well, I think the patch wouldn't merge those it simply treats them the same
for references to outside of a tree SCC.  So it's kind of a hack...

I think the real fix is to apply the linker resolution to the cgraph early
(which means reading the cgraph before reading global decls), keeping
a decl-ref to symtab node hash for the purpose of tree merging.

> It is bit ugly, but I think for static/external/public decls we need to use
> assembler name here (as we can't rely on symtab to be around all the time)
> which will result in unstability WRT symbol renaming and also give false
> positives for static symbols. But even for static symbols it is not guaranteed
> they are not duplicated.  It may be possible to use combination of assembler
> name and translation unit that is available from symtab node, but not all 
> decls
> will have it defined.

Yeah, but assembler name is in the cgraph which is not read in yet...
(so I wonder how same_global_decl_p in the patch works ...).  Oh, it isn't
yet in the symbol table...

Still I guess it should only compare if DECL_ASSEMBLER_NAME_SET_P.

And I think the patch (without the LTO_SET_PREVAIL bits) is sensible.

Richard.

>
> Honza

Re: [PATCH], PowerPC support to enable -mlra and/or -mfloat128

[Segher Boessenkool]

On Tue, Jul 12, 2016 at 12:29:04PM +0200, Richard Biener wrote:
> Instead of adding more configury can we please enable LRA on trunk by default
> _now_?  Otherwise the amount of testing it recieves won't really increase.

I agree we should change default_lra_p to return true instead of false.
However, that won't change what the rs6000 port uses (it has its own
implementation of that hook; the uses can select LRA vs. reload with
-mlra).  Other ports that have LRA selectable are in the same boat.
For ports that always use LRA, everything works no matter what.

For rs6000, we cannot change just yet, the performance regressions are
just too big.


Segher

Re: [PATCH], PowerPC support to enable -mlra and/or -mfloat128

[Bernd Schmidt]

On 07/12/2016 12:29 PM, Richard Biener wrote:


Instead of adding more configury can we please enable LRA on trunk by default
_now_?  Otherwise the amount of testing it recieves won't really increase.


I hope you mean for ppc only, otherwise you're breaking a lot of ports...


Bernd

Re: Implement -Wswitch-fallthrough

[Bernd Schmidt]

On 07/11/2016 09:43 PM, Marek Polacek wrote:

This warning is enabled by default for C/C++.  I was more inclined to put this
into -Wall, but our common.opt machinery doesn't seem to allow that (ugh!).


Add me to the list of folks opposed to enabling this unconditionally or 
as part of any other -W flag. The false positive rate would be enormous 
as shown by your patch series.



Bernd

Re: [PATCH], PowerPC support to enable -mlra and/or -mfloat128

[Richard Biener]

On Tue, Jul 12, 2016 at 1:21 PM, Bernd Schmidt  wrote:
> On 07/12/2016 12:29 PM, Richard Biener wrote:
>
>> Instead of adding more configury can we please enable LRA on trunk by
>> default
>> _now_?  Otherwise the amount of testing it recieves won't really increase.
>
>
> I hope you mean for ppc only, otherwise you're breaking a lot of ports...

Of course.  Simply add Init(1) to rs6000.opt mlra.

Richard.



>
> Bernd

Re: Implement -Wswitch-fallthrough

[Marek Polacek]

On Tue, Jul 12, 2016 at 01:24:15PM +0200, Bernd Schmidt wrote:
> On 07/11/2016 09:43 PM, Marek Polacek wrote:
> > This warning is enabled by default for C/C++.  I was more inclined to put 
> > this
> > into -Wall, but our common.opt machinery doesn't seem to allow that (ugh!).
> 
> Add me to the list of folks opposed to enabling this unconditionally or as
> part of any other -W flag. The false positive rate would be enormous as
> shown by your patch series.

I feel that, given all the pushback, I have to explain that it was never my
intent to enable this by default.  I was thinking of -Wall/-Wextra (given
-Wmisleading-indentation is in -Wall).  It's been enabled by default only to
get better testing coverage when doing various bootstraps and I failed to
move it to -Wextra, or even out of it.

As discussed elsewhere, I/we'll have to suppress the warning for various
"falls through" comments, which should cut the verbosity substantially.
Whether to put this warning into -Wall/-Wextra when that's done is still
to be discussed, but we all know that if a warning is out of -Wall/-Wextra,
no one will use it.

Marek

[ARM] no-data-is-text-relative & msingle-pic-base

[Nathan Sidwell]

Ramana,
could you review this?

original thread https://gcc.gnu.org/ml/gcc-patches/2016-05/msg00630.html, 
reproduced below:



currently, the documentation for -mno-pic-data-is-text-relative (-mno-PDITR) 
says
'Assume that each data segments are relative to text segment at load time.
 Therefore, it permits addressing data using PC-relative operations.
 This option is on by default for targets other than VxWorks RTP.'

However, if you use just this option, you still end up with a pic-register init 
sequence that presumes a fixed mapping. That's a surprise. Joey tells me its 
expected use is with -msingle-pic-base (-mSPB), which reserves a global register 
to point at the (single) GOT. That's what I had expected the -mno-PDITR option 
to have implied.


Apparently there are legitimate reasons one might want the -mno-PDITR behaviour 
without -mSPB. I don't know what those are though.


Anyway, IMHO that is the rare case and the more common case is that one would 
want to have -mnoPDITR imply -mSPB. (The reverse probably doesn't apply.)


This patch does 3 things.
1) have -mno-PDITR imply -mSPB, unless one has explicitly provided -m[no-]SPB.
2) clarified  the -m[no-]PDITR documentation.
3) Added some testcases -- there didn't appear to be any.

ok?

nathan
2016-05-09  Nathan Sidwell  

	gcc/
	* config/arm/arm.c (arm_option_override): Set MASK_SINGLE_PIC_BASE
	when -mno-pic-data-is-text-relative is in effect, by default.
	* doc/invoke.texi (mpic-data-is-text-relative): Document new
	behavior and clarify.

	gcc/testsuite/
	* gcc.target/arm/data-rel-1.c: New.
	* gcc.target/arm/data-rel-2.c: New.
	* gcc.target/arm/data-rel-3.c: New.

Index: config/arm/arm.c
===
--- config/arm/arm.c	(revision 235980)
+++ config/arm/arm.c	(working copy)
@@ -3298,6 +3298,20 @@ arm_option_override (void)
 	}
 }
 
+  if (TARGET_VXWORKS_RTP)
+{
+  if (!global_options_set.x_arm_pic_data_is_text_relative)
+	arm_pic_data_is_text_relative = 0;
+}
+  else if (flag_pic
+	   && !arm_pic_data_is_text_relative
+	   && !(global_options_set.x_target_flags & MASK_SINGLE_PIC_BASE))
+/* When text & data segments don't have a fixed displacement, the
+   intended use is with a single, read only, pic base register.
+   Unless the user explicitly requested not to do that, set
+   it.  */
+target_flags |= MASK_SINGLE_PIC_BASE;
+
   /* If stack checking is disabled, we can use r10 as the PIC register,
  which keeps r9 available.  The EABI specifies r9 as the PIC register.  */
   if (flag_pic && TARGET_SINGLE_PIC_BASE)
@@ -3329,10 +3343,6 @@ arm_option_override (void)
 	arm_pic_register = pic_register;
 }
 
-  if (TARGET_VXWORKS_RTP
-  && !global_options_set.x_arm_pic_data_is_text_relative)
-arm_pic_data_is_text_relative = 0;
-
   /* Enable -mfix-cortex-m3-ldrd by default for Cortex-M3 cores.  */
   if (fix_cm3_ldrd == 2)
 {
Index: doc/invoke.texi
===
--- doc/invoke.texi	(revision 235980)
+++ doc/invoke.texi	(working copy)
@@ -14197,9 +14197,12 @@ otherwise the default is @samp{R10}.
 
 @item -mpic-data-is-text-relative
 @opindex mpic-data-is-text-relative
-Assume that each data segments are relative to text segment at load time.
-Therefore, it permits addressing data using PC-relative operations.
-This option is on by default for targets other than VxWorks RTP.
+Assume that the displacement between the text and data segments is fixed
+at static link time.  This permits using PC-relative addressing
+operations to access data known to be in the data segment.  For
+non-VxWorks RTP targets, this option is enabled by default.  When
+disabled on such targets, it will enable @option{-msingle-pic-base} by
+default.
 
 @item -mpoke-function-name
 @opindex mpoke-function-name
Index: testsuite/gcc.target/arm/data-rel-1.c
===
--- testsuite/gcc.target/arm/data-rel-1.c	(nonexistent)
+++ testsuite/gcc.target/arm/data-rel-1.c	(working copy)
@@ -0,0 +1,12 @@
+/* { dg-options "-fPIC -mno-pic-data-is-text-relative" } */
+/* { dg-final { scan-assembler-not "j-\\(.LPIC"  } } */
+/* { dg-final { scan-assembler-not "_GLOBAL_OFFSET_TABLE_-\\(.LPIC" } } */
+/* { dg-final { scan-assembler "j\\(GOT\\)" } } */
+/* { dg-final { scan-assembler "(ldr|mov)\tr\[0-9\]+, \\\[?r9" } } */
+
+static int j;
+
+int *Foo ()
+{
+  return &j;
+}
Index: testsuite/gcc.target/arm/data-rel-2.c
===
--- testsuite/gcc.target/arm/data-rel-2.c	(nonexistent)
+++ testsuite/gcc.target/arm/data-rel-2.c	(working copy)
@@ -0,0 +1,11 @@
+/* { dg-options "-fPIC -mno-pic-data-is-text-relative -mno-single-pic-base" } */
+/* { dg-final { scan-assembler-not "j-\\(.LPIC"  } } */
+/* { dg-final { scan-assembler "_GLOBAL_OFFSET_TABLE_-\\(.LPIC" } } */
+/* { dg-final { scan-assembler "j\\(GOT\\)" } } */
+
+static i

Re: [PATCH, rs6000] Fix PR target/71733, ICE with -mcpu=power9 -mno-vsx

[Ulrich Weigand]

Alan Modra wrote:
> On Mon, Jul 11, 2016 at 03:26:46PM +0200, Ulrich Weigand wrote:
> > However, there still seems to be another underlying problem: reload
> > should never generate invalid instructions.  (Playing with '?' to
> > fix *inefficient* instructions is fine, but we shouldn't rely on '?'
> > to fix *invalid* instructions.)
> > 
> > If reload -for whatever reason- wants to emit a vr->gpr move, then it
> > should generate a valid instruction sequence to do so, via secondary
> > reloads if necessary.  I think it would be a good idea to investigate
> > why that isn't happening in this test case.  [ There is a chance that
> > the underlying bug will reappear in a different context, even after
> > the '?' change is applied. ]
> 
> The vr->gpr move uses secondary memory.  The memory allocated by
> assign_stack_local, called from get_secondary_memory is
> 
> (mem/c:V16QI (plus:DI (reg/f:DI 113 sfp)
> (const_int 32 [0x20])) [0  S16 A128])
> 
> and that is incorrectly simplified by eliminate_regs to
> 
> (mem/c:V16QI (reg/f:DI 1 1))
> 
> for use by the strict_memory_address_space_p check, which of course
> then passes.
> 
> eliminate_regs sees this entry in the elimination table:
> (gdb) p *ep
> $7 = {from = 113, to = 1, initial_offset = -32, can_eliminate = 1, 
> can_eliminate_previous = 1, offset = -32, previous_offset = -32, 
> ref_outside_mem = 0, from_rtx = 0x7688a300, to_rtx = 0x7688a2e8}
> 
> The offset in the elimination table was correct before
> get_secondary_memory called assign_stack_local, but not after
> frame_offset was changed when allocating a new stack slot.

OK, I see.  Yes, the first time around that will be wrong.
However, if get_secondary_mem called assign_stack_local, the
stack size will be changing, which is why the main loop
around calculate_needs_all_insns() in reload() should set
the "something_changed" flag and restart the whole process.
(Which means the reloads for the first pass will be wrong,
but that doesn't matter since they will be ignored.)

The second time around, get_secondary_mem should reuse the
same stack slot it already allocated, and the elimination
offsets should already be set to accommodate that stack slot,
which means the second time around, the correct RTX should be
generated for the memory access.

Is this not happening somehow?

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  ulrich.weig...@de.ibm.com

Re: [PATCH], PR 71805, Fix PowerPC ISA 3.0 xxperm/xxpermr usage

[Segher Boessenkool]

On Tue, Jul 12, 2016 at 12:28:12AM -0400, Michael Meissner wrote:
> In this patch, I have fixed the xxperm instructions to expect the second input
> argument to be the same as the output argument instead of the first.  I also
> fixed the xxpermr instruction, which suffers from the same problem.  I added
> the test case, and made it specific to power9, in case the original test case
> is changed.
> 
> I have done a bootstrap build with no regressions.  Can I install this patch
> into both the trunk and GCC 6, which also shows the bug?

Okay for both.  Thanks,


Segher

Re: [PATCH 2/2] Optimize fortran loops with +-1 step.

[Martin Liška]

On 07/12/2016 12:07 PM, Richard Biener wrote:
> That change is certainly spurious (not in ChangeLog) and bogus given the
> comment before the scan.
> 
> Richard.

Hi.

I'll revert the change in ldist-1.f90 which I accidentally committed a week ago,
it's going to be part of second version of the patch

M.

Re: [RFC, v2] Test coverage for --param boundary values

[Martin Liška]

On 07/11/2016 04:48 PM, Jeff Law wrote:
> Seems reasonable to me -- OK for the trunk, but please be on the lookout for 
> these tests failing on other targets.
> 
> jeff

Thanks, the patch also works on a ppc64le-linux-gnu system.
I'm going to install the patch after I'll install fix for:
https://gcc.gnu.org/ml/gcc-patches/2016-07/msg00416.html

Otherwise, I would introduce new FAILs.

Martin

Re: [PATCH], PowerPC support to enable -mlra and/or -mfloat128

[Segher Boessenkool]

On Tue, Jul 12, 2016 at 01:31:35PM +0200, Richard Biener wrote:
> On Tue, Jul 12, 2016 at 1:21 PM, Bernd Schmidt  wrote:
> > On 07/12/2016 12:29 PM, Richard Biener wrote:
> >
> >> Instead of adding more configury can we please enable LRA on trunk by
> >> default
> >> _now_?  Otherwise the amount of testing it recieves won't really increase.
> >
> >
> > I hope you mean for ppc only, otherwise you're breaking a lot of ports...
> 
> Of course.  Simply add Init(1) to rs6000.opt mlra.

This is blocked on PR69847.  The plan is still to enable it this stage 1.

How can LRA ever be made default for all targets without breaking those
that do not want to change?


Segher

Re: [PATCH] Fix Fortran DO loop fallback

[Martin Liška]

On 07/12/2016 12:14 PM, Richard Biener wrote:
> On Mon, Jul 11, 2016 at 4:44 PM, Jeff Law  wrote:
>> On 07/08/2016 08:26 AM, Martin Liška wrote:
>>>
>>> Hello
>>>
>>> Following patch fixes fallout caused by the patch set:
>>> https://gcc.gnu.org/ml/gcc-regression/2016-07/msg00097.html
>>>
>>> Ready after it finished regression tests?
>>> Thanks,
>>> Martin
>>>
>>>
>>> 0001-Fix-Fortran-DO-loop-fallback.patch
>>>
>>>
>>> From c5dd7ad62f795cce560c7f1bb8767b7ed9298d8a Mon Sep 17 00:00:00 2001
>>> From: marxin 
>>> Date: Fri, 8 Jul 2016 15:51:54 +0200
>>> Subject: [PATCH] Fix Fortran DO loop fallback
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 2016-07-08  Martin Liska  
>>>
>>> * gfortran.dg/ldist-1.f90: Update expected dump scan.
>>> * gfortran.dg/pr42108.f90: Likewise.
>>> * gfortran.dg/vect/pr62283.f: Likewise.
>>
>> Shouldn't ldist-1.f90 be scan-tree-dump-not?  It seems like you change it
>> from that just last week, but it wasn't mentioned in the ChangeLog.
> 
> gfortran.dg/pr42108.f90 also looks pointless now?  I suppose there is nothing
> to hoist after the change?  Do we now have an option to revert back to old
> behavior?  If so it would be better to use that flag here.

No, there's no option. So, as the test-case now looks pointless, should I mark 
it
with xfail now?

> 
> diff --git a/gcc/testsuite/gfortran.dg/vect/pr62283.f
> b/gcc/testsuite/gfortran.dg/vect/pr62283.f
> index 7df3d99..2933f51 100644
> --- a/gcc/testsuite/gfortran.dg/vect/pr62283.f
> +++ b/gcc/testsuite/gfortran.dg/vect/pr62283.f
> @@ -13,4 +13,4 @@ C { dg-additional-options "-fvect-cost-model=dynamic" }
>beta=3.141593
>y=y+beta*x
>end
> -C { dg-final { scan-tree-dump-times "vectorized 1 loops" 2 "vect" {
> target { vect_hw_misalign } } } }
> +C { dg-final { scan-tree-dump "vectorized 1 loops" "vect" { target {
> vect_hw_misalign } } } }
> 
> so why do we no longer vectorize 1 loops in two functions?  The
> testcase works for me
> unchanged.

Yeah, it works on -m64, however as we're able to merge the functions with -m32 
(-fipa-icf),
then I recommend to disable ICF for the test-case.

Reason why the pair of functions on x86_64 is that:

test3 (real(kind=4)[4] * restrict x, real(kind=4)[4] * restrict y)
{
  :

  :
  # S.0_6 = PHI <1(2), S.0_12(4)>
  if (S.0_6 > 4)
goto ;
  else
goto ;
...

test2 (real(kind=4)[4] * restrict x, real(kind=4)[4] * restrict y)
{
  integer(kind=4) i;

  :

  :
  # i_6 = PHI <1(2), i_12(4)>
...

On x86_64 types of 'S.0_6' and 'i' are not compatible. As I've just read tree 
dump files,
  # S.0_6 = PHI <1(2), S.0_12(4)>
  if (S.0_6 > 4)

is optimized out by VRP, which runs after IPA ICF.

I'll send patch right after we'll agree on pr42108.f90.

Thanks,
Martin

> 
> Richard.
> 
>> OK with that change.
>>
>> jeff
>>
>>

Re: [PATCH] - improve sprintf buffer overflow detection (middle-end/49905)

[Bernd Schmidt]

On 07/01/2016 08:15 PM, Martin Sebor wrote:

The attached patch enhances compile-time checking for buffer overflow
and output truncation in non-trivial calls to the sprintf family of
functions under a new option -Wformat-length=[12].  This initial
patch handles printf directives with string, integer, and simple
floating arguments but eventually I'd like to extend it all other
functions and directives for which it makes sense.


On the whole I think this looks good.


+  /* Check the whole format strings and ist arguments for possible
+buffer overflow or truncation.  */


"its"


 struct function_format_info
 {
+  built_in_function fncode;
   int format_type; /* type of format (printf, scanf, etc.) 
*/
   unsigned HOST_WIDE_INT format_num;   /* number of format argument */
   unsigned HOST_WIDE_INT first_arg_num;/* number of first arg (zero 
for varargs) */
+  /* The destination object size or -1 if unknown.  */
+  unsigned HOST_WIDE_INT objsize;
+
+  /* True for functions whose output is bounded by a size argument
+ (e.g., snprintf and vsnprintf).  */
+  bool bounded;
 };


While you're here could you fix the existing comments to be formatted 
like the ones you're adding (in front of the thing they're documenting, 
etc.)?



+static void
+check_format_length (location_t,
+format_check_results *,
+const function_format_info *,
+const format_char_info *,
+const char *, size_t, size_t,
+const conversion_spec *, tree);


Only break the line before the function name for definitions, not 
declarations.



+unsigned char c = chr & 0xff;
+return flags [c / (CHAR_BIT * sizeof *flags)]
+  & (1 << (c % (CHAR_BIT * sizeof *flags)));


> +unsigned char c = chr & 0xff;
> +flags [c / (CHAR_BIT * sizeof *flags)]
> +  |= (1 << (c % (CHAR_BIT * sizeof *flags)));

Multi-line expressions should be wrapped in parentheses for correct 
formatting. Also, no space before [] brackets.



+/* Return the logarithm of tree node X in BASE, incremented by 1 when
+   the optional PLUS sign is True, plus the length of the octal ('0')
+   or hexadecimal ('0x') prefix when PREFIX is True.  Return -1 when
+   X cannot be represented.  */


Maybe this could be clearer as
/* Return the number of characters required to print X, which must be
   an INTEGER_CST, in BASE.  PLUS indicates whether a plus sign should
   be prepended to positive numbers, and PREFIX indicates whether an
   octal ('O') or hexadecimal ('0x') prefix should be prepended to
   nonzero numbesr.  Return -1 if X cannot be represented.  */


+  if (prefix && absval)


This surprised me, but I checked and printf really does not seem to 
print 0x0.



+/* Return a range representing the minimum and maximum number of bytes
+   that the conversion specification SPEC will write on output for the
+   integer argument ARG.  */


Should indicate that ARG can be null, and what happens in that case. 
It's not entirely clear to me actually why it would be NULL sometimes.



+width = (TREE_CODE (spec->star_width) == INTEGER_CST)
+? tree_to_shwi (spec->star_width) : 0;



+prec = (TREE_CODE (spec->star_precision) == INTEGER_CST)
+   ? tree_to_shwi (spec->star_precision) : 0;


Wrapping. Lose parens around the condition, and wrap the whole expression.


+  /* A type of the argument to the directive, either deduced from
+ the actual non-constant argument if one is known, or from
+ the directive itself when none has been provided because it's
+ a va_list.  */
+  tree argtype = NULL_TREE;


"The type" maybe? Also, the structure of this function gets confusing 
around this point. It looks like we set argtype for everything except 
integer constants, then have a big block handling nonnull argtype and 
returning, then we have another block dealing with the other case. I 
think it would be clearer to check for INTEGER_CST first, deal with it, 
and then doing the type-based estimation.



+  else if (TREE_CODE (arg) == INTEGER_CST)
+res.bounded = true;


The assignment appears to be repeated at the end of the function.


+width = (TREE_CODE (spec->star_width) == INTEGER_CST)
+? tree_to_shwi (spec->star_width) : 0;



+prec = (TREE_CODE (spec->star_precision) == INTEGER_CST)
+? tree_to_shwi (spec->star_precision) : 0;


See other instances. These lines occur several times in multiple functions.


+  int expdigs = -1;/* Number of exponent digits or -1 when unknown.  */
+  int negative = -1;   /* 1 when arg < 0, 0 when arg >= 0, -1 when unknown.  */


I think we should avoid end-of-line comments. I tend to sometimes feel 
they are ok-ish in struct definitions, but let's not have them in code.



+  res.min = (0 < negative || spec->get_flag ('+') || spec->get_flag (' '))
+   + 1 /* unit */ + (prec < 0 ? 7 : prec ? prec + 1 : 0)
+   + 2 /* e+ */ + 

Re: [PATCH], PowerPC support to enable -mlra and/or -mfloat128

[Bernd Schmidt]

On 07/12/2016 02:15 PM, Segher Boessenkool wrote:

How can LRA ever be made default for all targets without breaking those
that do not want to change?


LRA advocates would have to fix the issues with it that prevent it from 
being usable on certain targets. Based on my initial experiments with 
Blackfin, and LRA review comments I had based on c6x requirements, I 
suspect register elimination will have to be rewritten to start with.



Bernd

Re: [PATCH], PowerPC support to enable -mlra and/or -mfloat128

[Segher Boessenkool]

On Tue, Jul 12, 2016 at 02:40:28PM +0200, Bernd Schmidt wrote:
> On 07/12/2016 02:15 PM, Segher Boessenkool wrote:
> >How can LRA ever be made default for all targets without breaking those
> >that do not want to change?
> 
> LRA advocates would have to fix the issues with it that prevent it from 
> being usable on certain targets. Based on my initial experiments with 
> Blackfin, and LRA review comments I had based on c6x requirements, I 
> suspect register elimination will have to be rewritten to start with.

Do you have PR #s for those issues?


Segher

Re: [PATCH], PowerPC support to enable -mlra and/or -mfloat128

[Bernd Schmidt]

On 07/12/2016 02:45 PM, Segher Boessenkool wrote:

On Tue, Jul 12, 2016 at 02:40:28PM +0200, Bernd Schmidt wrote:

On 07/12/2016 02:15 PM, Segher Boessenkool wrote:

How can LRA ever be made default for all targets without breaking those
that do not want to change?


LRA advocates would have to fix the issues with it that prevent it from
being usable on certain targets. Based on my initial experiments with
Blackfin, and LRA review comments I had based on c6x requirements, I
suspect register elimination will have to be rewritten to start with.


Do you have PR #s for those issues?


No. You can reproduce issues with Blackfin easily by enabling LRA for 
it, and I described the C6X issues when the LRA patches were posted for 
review.



Bernd

Re: [PATCH], PowerPC support to enable -mlra and/or -mfloat128

[Segher Boessenkool]

On Tue, Jul 12, 2016 at 02:40:28PM +0200, Bernd Schmidt wrote:
> On 07/12/2016 02:15 PM, Segher Boessenkool wrote:
> >How can LRA ever be made default for all targets without breaking those
> >that do not want to change?
> 
> LRA advocates would have to fix the issues with it that prevent it from 
> being usable on certain targets.

That would be the case if the plan was to remove reload.  But the current
proposal is only to change the default, and the affected targets can easily
implement the hook (to say "no") if LRA won't work for them.


Segher

Re: [patch, fortran] PR66310 Problems with intrinsic repeat for large number of copies

[FX]

> 2016-07-11  Jerry DeLisle  
> 
>   PR fortran/66310
>   * simplify.c (gfc_simplify_repeat): Set max repeat to huge - 1 to allow
>   one byte for null terminating the resulting string constant.

OK, thanks

Re: [PATCH, rs6000, libgcc] Implement temporary solution for __divkc3 and __mulkc3

[Segher Boessenkool]

On Mon, Jul 11, 2016 at 02:38:50PM -0500, Bill Schmidt wrote:
> It was recently brought to my attention that glibc needs access to complex
> multiply and divide for IEEE-128 floating-point in GCC 6.2 in order to move
> ahead with the library implementation work.  This patch enables this support
> using only target-specific changes to avoid any possible effect on other
> targets.  This is not the correct long-term approach, and I am working on a
> patch that instead makes use of the common infrastructure.  The plan is to
> use the current patch for GCC 6, and replace it with the other approach in
> GCC 7 shortly.
> 
> Thus this patch copies the common code for complex multiply and divide out
> of libgcc2.c into separate Power-specific files, and specializes it for
> the KC type.  It adds a couple of straightforward tests to verify that the
> approach works.  I've tested the code generated for these tests on a POWER9
> simulator as well as a POWER8 machine.
> 
> Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no regressions.
> I've also asked the glibc team to verify that this serves their requirements.
> Is this ok for trunk, and for gcc-6-branch after a short burn-in period?

Okay for both.  Ugly :-)


Segher

Re: [PATCH] Fix Fortran DO loop fallback

[Richard Biener]

On Tue, Jul 12, 2016 at 2:31 PM, Martin Liška  wrote:
> On 07/12/2016 12:14 PM, Richard Biener wrote:
>> On Mon, Jul 11, 2016 at 4:44 PM, Jeff Law  wrote:
>>> On 07/08/2016 08:26 AM, Martin Liška wrote:

 Hello

 Following patch fixes fallout caused by the patch set:
 https://gcc.gnu.org/ml/gcc-regression/2016-07/msg00097.html

 Ready after it finished regression tests?
 Thanks,
 Martin


 0001-Fix-Fortran-DO-loop-fallback.patch


 From c5dd7ad62f795cce560c7f1bb8767b7ed9298d8a Mon Sep 17 00:00:00 2001
 From: marxin 
 Date: Fri, 8 Jul 2016 15:51:54 +0200
 Subject: [PATCH] Fix Fortran DO loop fallback

 gcc/testsuite/ChangeLog:

 2016-07-08  Martin Liska  

 * gfortran.dg/ldist-1.f90: Update expected dump scan.
 * gfortran.dg/pr42108.f90: Likewise.
 * gfortran.dg/vect/pr62283.f: Likewise.
>>>
>>> Shouldn't ldist-1.f90 be scan-tree-dump-not?  It seems like you change it
>>> from that just last week, but it wasn't mentioned in the ChangeLog.
>>
>> gfortran.dg/pr42108.f90 also looks pointless now?  I suppose there is nothing
>> to hoist after the change?  Do we now have an option to revert back to old
>> behavior?  If so it would be better to use that flag here.
>
> No, there's no option. So, as the test-case now looks pointless, should I 
> mark it
> with xfail now?

The scan for 1 *n_ after FRE looks still useful.  Btw, the testcase
doesn't fail for me,
we _do_ hoist the division in PRE, just not with -m32 anymore.  Can
you confirm this?


>>
>> diff --git a/gcc/testsuite/gfortran.dg/vect/pr62283.f
>> b/gcc/testsuite/gfortran.dg/vect/pr62283.f
>> index 7df3d99..2933f51 100644
>> --- a/gcc/testsuite/gfortran.dg/vect/pr62283.f
>> +++ b/gcc/testsuite/gfortran.dg/vect/pr62283.f
>> @@ -13,4 +13,4 @@ C { dg-additional-options "-fvect-cost-model=dynamic" }
>>beta=3.141593
>>y=y+beta*x
>>end
>> -C { dg-final { scan-tree-dump-times "vectorized 1 loops" 2 "vect" {
>> target { vect_hw_misalign } } } }
>> +C { dg-final { scan-tree-dump "vectorized 1 loops" "vect" { target {
>> vect_hw_misalign } } } }
>>
>> so why do we no longer vectorize 1 loops in two functions?  The
>> testcase works for me
>> unchanged.
>
> Yeah, it works on -m64, however as we're able to merge the functions with 
> -m32 (-fipa-icf),
> then I recommend to disable ICF for the test-case.
>
> Reason why the pair of functions on x86_64 is that:
>
> test3 (real(kind=4)[4] * restrict x, real(kind=4)[4] * restrict y)
> {
>   :
>
>   :
>   # S.0_6 = PHI <1(2), S.0_12(4)>
>   if (S.0_6 > 4)
> goto ;
>   else
> goto ;
> ...
>
> test2 (real(kind=4)[4] * restrict x, real(kind=4)[4] * restrict y)
> {
>   integer(kind=4) i;
>
>   :
>
>   :
>   # i_6 = PHI <1(2), i_12(4)>
> ...
>
> On x86_64 types of 'S.0_6' and 'i' are not compatible. As I've just read tree 
> dump files,
>   # S.0_6 = PHI <1(2), S.0_12(4)>
>   if (S.0_6 > 4)
>
> is optimized out by VRP, which runs after IPA ICF.
>
> I'll send patch right after we'll agree on pr42108.f90.
>
> Thanks,
> Martin
>
>>
>> Richard.
>>
>>> OK with that change.
>>>
>>> jeff
>>>
>>>
>

Re: [patch, fortran] PR66310 Problems with intrinsic repeat for large number of copies

[Dominique d'Humières]

> > 2016-07-11  Jerry DeLisle  
> > 
> > PR fortran/66310
> > * simplify.c (gfc_simplify_repeat): Set max repeat to huge - 1 to allow
> > one byte for null terminating the resulting string constant.
>
> OK, thanks
Please hold on. I still see several problem with the patch applied. One is

   program p
  character :: z = 'z'
  print *, repeat(z, huge(1)-2**9)
   end

a.out(67209,0x7fff77e0b000) malloc: *** mach_vm_map(size=18446744071562067968) 
failed (error code=3)
*** error: can't allocate region
*** set a breakpoint in malloc_error_break to debug
Operating system error: Cannot allocate memory
Memory allocation failure in realloc

on x86_64-apple-darwin15.5 with/without the patch. print *, repeat(z, 
huge(1)-2**9-1) "works".

I’ll try to report the other problems in the PR later today.

Dominique

Re: [PATCH, rs6000] Fix PR target/71733, ICE with -mcpu=power9 -mno-vsx

[Alan Modra]

On Tue, Jul 12, 2016 at 02:02:43PM +0200, Ulrich Weigand wrote:
> The second time around, get_secondary_mem should reuse the
> same stack slot it already allocated, and the elimination
> offsets should already be set to accommodate that stack slot,
> which means the second time around, the correct RTX should be
> generated for the memory access.
> 
> Is this not happening somehow?

Duh, yes, of course.  Second time around the mem is
(mem/c:V16QI (plus:DI (reg/f:DI 1 1)
(const_int -16 [0xfff0])) [0  S16 A128])
so we're checking the correct offset.

The problem now is that this passes rs6000_legitimate_address_p due to
mode_supports_vsx_dform_quad and quad_address_p being true.  That
doesn't seem correct for -mno-vsx.

-- 
Alan Modra
Australia Development Lab, IBM

C++ OpenACC routine directive testing: templated, and "auto", trailing return type syntax

[Thomas Schwinge]

Hi!

Templated, and "auto", trailing return type syntax with the C++ OpenACC
routine directive all works, but doesn't have test coverage.  OK for
trunk?

commit 7a387329674b07b8eb7e07cff665250284b4524b
Author: Thomas Schwinge 
Date:   Thu Jul 7 16:12:15 2016 +0200

C++ OpenACC routine directive testing: templated, and "auto", trailing 
return type syntax

libgomp/
* testsuite/libgomp.oacc-c++/routine-1-auto.C: New file.
* testsuite/libgomp.oacc-c++/routine-1-template-auto.C: Likewise.
* testsuite/libgomp.oacc-c++/routine-1-template-trailing-return-type.C:
Likewise.
* testsuite/libgomp.oacc-c++/routine-1-template.C: Likewise.
* testsuite/libgomp.oacc-c++/routine-1-trailing-return-type.C:
Likewise.
* testsuite/libgomp.oacc-c-c++-common/routine-1.c: Adjust.
---
 libgomp/testsuite/libgomp.oacc-c++/routine-1-auto.C  |  9 +
 libgomp/testsuite/libgomp.oacc-c++/routine-1-template-auto.C |  8 
 .../routine-1-template-trailing-return-type.C|  8 
 libgomp/testsuite/libgomp.oacc-c++/routine-1-template.C  |  8 
 .../libgomp.oacc-c++/routine-1-trailing-return-type.C|  9 +
 libgomp/testsuite/libgomp.oacc-c-c++-common/routine-1.c  | 12 ++--
 6 files changed, 52 insertions(+), 2 deletions(-)

diff --git libgomp/testsuite/libgomp.oacc-c++/routine-1-auto.C 
libgomp/testsuite/libgomp.oacc-c++/routine-1-auto.C
new file mode 100644
index 000..f4b54e5
--- /dev/null
+++ libgomp/testsuite/libgomp.oacc-c++/routine-1-auto.C
@@ -0,0 +1,9 @@
+// Routine with "auto" return type.
+
+// { dg-additional-options "-fno-exceptions" }
+
+#define TEMPLATE
+#define TYPE int
+#define RETURN_1 auto
+#define RETURN_2
+#include "../libgomp.oacc-c-c++-common/routine-1.c"
diff --git libgomp/testsuite/libgomp.oacc-c++/routine-1-template-auto.C 
libgomp/testsuite/libgomp.oacc-c++/routine-1-template-auto.C
new file mode 100644
index 000..444f1f3
--- /dev/null
+++ libgomp/testsuite/libgomp.oacc-c++/routine-1-template-auto.C
@@ -0,0 +1,8 @@
+// Templated routine with "auto" return type.
+
+// { dg-additional-options "-fno-exceptions" }
+
+#define TEMPLATE template
+#define RETURN_1 auto
+#define RETURN_2
+#include "../libgomp.oacc-c-c++-common/routine-1.c"
diff --git 
libgomp/testsuite/libgomp.oacc-c++/routine-1-template-trailing-return-type.C 
libgomp/testsuite/libgomp.oacc-c++/routine-1-template-trailing-return-type.C
new file mode 100644
index 000..bfe2787
--- /dev/null
+++ libgomp/testsuite/libgomp.oacc-c++/routine-1-template-trailing-return-type.C
@@ -0,0 +1,8 @@
+// Templated routine using trailing return type syntax.
+
+// { dg-additional-options "-fno-exceptions" }
+
+#define TEMPLATE template
+#define RETURN_1 auto
+#define RETURN_2 -> TYPE
+#include "../libgomp.oacc-c-c++-common/routine-1.c"
diff --git libgomp/testsuite/libgomp.oacc-c++/routine-1-template.C 
libgomp/testsuite/libgomp.oacc-c++/routine-1-template.C
new file mode 100644
index 000..a7e0323
--- /dev/null
+++ libgomp/testsuite/libgomp.oacc-c++/routine-1-template.C
@@ -0,0 +1,8 @@
+// Templated routine.
+
+// { dg-additional-options "-fno-exceptions" }
+
+#define TEMPLATE template
+#define RETURN_1 TYPE
+#define RETURN_2
+#include "../libgomp.oacc-c-c++-common/routine-1.c"
diff --git libgomp/testsuite/libgomp.oacc-c++/routine-1-trailing-return-type.C 
libgomp/testsuite/libgomp.oacc-c++/routine-1-trailing-return-type.C
new file mode 100644
index 000..3074ba4
--- /dev/null
+++ libgomp/testsuite/libgomp.oacc-c++/routine-1-trailing-return-type.C
@@ -0,0 +1,9 @@
+// Routine using trailing return type syntax.
+
+// { dg-additional-options "-fno-exceptions" }
+
+#define TEMPLATE
+#define TYPE int
+#define RETURN_1 auto
+#define RETURN_2 -> TYPE
+#include "../libgomp.oacc-c-c++-common/routine-1.c"
diff --git libgomp/testsuite/libgomp.oacc-c-c++-common/routine-1.c 
libgomp/testsuite/libgomp.oacc-c-c++-common/routine-1.c
index f112457..2a36b3b 100644
--- libgomp/testsuite/libgomp.oacc-c-c++-common/routine-1.c
+++ libgomp/testsuite/libgomp.oacc-c-c++-common/routine-1.c
@@ -1,10 +1,18 @@
 // { dg-additional-options "-fno-exceptions" }
 
-#include 
+// Defaults, if not "#include"d from ../libgomp.oacc-c++/routine-1-*.C.
+#ifndef TEMPLATE
+# define TEMPLATE
+# define TYPE int
+# define RETURN_1 TYPE
+# define RETURN_2 
+#endif
+
 #include 
 
 #pragma acc routine
-int fact(int n)
+TEMPLATE
+RETURN_1 fact(TYPE n) RETURN_2
 {
   if (n == 0 || n == 1)
 return 1;


Grüße
 Thomas

Re: C++ OpenACC routine directive testing: templated, and "auto", trailing return type syntax

[Jakub Jelinek]

On Tue, Jul 12, 2016 at 03:50:14PM +0200, Thomas Schwinge wrote:
> Hi!
> 
> Templated, and "auto", trailing return type syntax with the C++ OpenACC
> routine directive all works, but doesn't have test coverage.  OK for
> trunk?
> 
> commit 7a387329674b07b8eb7e07cff665250284b4524b
> Author: Thomas Schwinge 
> Date:   Thu Jul 7 16:12:15 2016 +0200
> 
> C++ OpenACC routine directive testing: templated, and "auto", trailing 
> return type syntax
> 
>   libgomp/
>   * testsuite/libgomp.oacc-c++/routine-1-auto.C: New file.
>   * testsuite/libgomp.oacc-c++/routine-1-template-auto.C: Likewise.
>   * testsuite/libgomp.oacc-c++/routine-1-template-trailing-return-type.C:
>   Likewise.
>   * testsuite/libgomp.oacc-c++/routine-1-template.C: Likewise.
>   * testsuite/libgomp.oacc-c++/routine-1-trailing-return-type.C:
>   Likewise.
>   * testsuite/libgomp.oacc-c-c++-common/routine-1.c: Adjust.

Ok.  Though looking at the testcases, they will crash if malloc fails
and all the allocations of small buffers look kind of pointless.
Wouldn't
  s = (int *) malloc (sizeof (int) * n);
  g = (int *) malloc (sizeof (int) * n);
  w = (int *) malloc (sizeof (int) * n);
  v = (int *) malloc (sizeof (int) * n);
  gw = (int *) malloc (sizeof (int) * n);
  gv = (int *) malloc (sizeof (int) * n);
  wv = (int *) malloc (sizeof (int) * n);
  gwv = (int *) malloc (sizeof (int) * n);
be better replaced with
  int buf[80];
  s = buf;
  g = s + n;
  w = g + n;
...
?

Jakub

Re: Implement -Wswitch-fallthrough

[NightStrike]

On Mon, Jul 11, 2016 at 4:34 PM, Jakub Jelinek  wrote:
> On Mon, Jul 11, 2016 at 10:23:30PM +0200, Marek Polacek wrote:
>> On Mon, Jul 11, 2016 at 01:18:02PM -0700, Andi Kleen wrote:
>> > > I explained why supporting the classic lint style comment wouldn't fly.
>> >
>> > Not convincing, it worked fine for 30+ years of lints.
>>
>> So how can the compiler handle
>> /* Never ever fall through here */
>> ?
>
> It can't.  But it perhaps could handle a couple of documented most common
> comment styles, perhaps only if followed by break token, and turn say
> /* FALLTHROUGH */
> break
> (and say:
> /* FALL THROUGH */
> /* FALLTHRU */
> /* FALL THRU */
> /*-fallthrough*/
> /* Fallthrough */
> /* Fall through */
> /* Fallthru */
> /* Fall thru */
> /* fallthrough */
> /* fall through */
> /* fallthru */
> /* fall thru */
> // FALLTHROUGH
> // FALL THROUGH
> // FALLTHRU
> // FALL THRU
> //-fallthrough
> // Fallthrough
> // Fall through
> // Fallthru
> // Fall thru
> // fallthrough
> // fall through
> // fallthru
> // fall thru

>From http://security.coverity.com/blog/2013/Sep/gimme-a-break.html:

We also suppress a case label if there is a comment that matches
[^#]fall.?thro?u, even if it's not on the last line.

Maybe that's enough?

The page has other exclusions that might be desirable.

[PATCH] Add tests and docs for LWG 2212 support

[Jonathan Wakely]

* testsuite/20_util/pair/astuple/astuple.cc: Only include .
* testsuite/23_containers/array/tuple_interface/tuple_element.cc:
Only include .
* testsuite/23_containers/array/tuple_interface/tuple_size.cc:
Likewise.
* doc/xml/manual/intro.xml; Document LWG 2212 support.
* doc/html*: Regenerate.

We already implement LWG 2212 but this tweaks the tests to verify it.

Tested x86_64-linux, committed to trunk.

commit 8c5a11cb7545d1a701596665392a8f4bf29a1489
Author: redi 
Date:   Tue Jul 12 14:00:26 2016 +

Add tests and docs for LWG 2212 support

* testsuite/20_util/pair/astuple/astuple.cc: Only include .
* testsuite/23_containers/array/tuple_interface/tuple_element.cc:
Only include .
* testsuite/23_containers/array/tuple_interface/tuple_size.cc:
Likewise.
* doc/xml/manual/intro.xml; Document LWG 2212 support.
* doc/html*: Regenerate.

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@238244 
138bc75d-0d04-0410-961f-82ee72b054a4

diff --git a/libstdc++-v3/doc/xml/manual/intro.xml 
b/libstdc++-v3/doc/xml/manual/intro.xml
index 2aa9ba7..a47a3ec 100644
--- a/libstdc++-v3/doc/xml/manual/intro.xml
+++ b/libstdc++-v3/doc/xml/manual/intro.xml
@@ -934,6 +934,14 @@ requirements of the license of GCC.
 Use the referenceable type concept.
 
 
+http://www.w3.org/1999/xlink"; 
xlink:href="../ext/lwg-defects.html#2212">2212:
+   tuple_size for const pair request 
 header
+
+The tuple_size and tuple_element
+partial specializations are defined in  which
+is included by .
+
+
 http://www.w3.org/1999/xlink"; 
xlink:href="../ext/lwg-defects.html#2313">2313:
tuple_size should always derive from 
integral_constant
 
diff --git a/libstdc++-v3/testsuite/20_util/pair/astuple/astuple.cc 
b/libstdc++-v3/testsuite/20_util/pair/astuple/astuple.cc
index 8b6fca3..3a458bb 100644
--- a/libstdc++-v3/testsuite/20_util/pair/astuple/astuple.cc
+++ b/libstdc++-v3/testsuite/20_util/pair/astuple/astuple.cc
@@ -18,8 +18,10 @@
 // with this library; see the file COPYING3.  If not see
 // .
 
+// NB: Don't include any other headers in this file.
+// LWG 2212 requires  to define tuple_size and
+// tuple_element specializations.
 #include 
-#include 
 
 typedef std::pair test_type;
 
@@ -32,6 +34,7 @@ static_assert( std::tuple_size::value == 2,
 template
   using Tuple_elt = typename std::tuple_element::type;
 
+// This relies on the fact that  includes :
 using std::is_same;
 
 static_assert( is_same, test_type::first_type>::value,
diff --git 
a/libstdc++-v3/testsuite/23_containers/array/tuple_interface/tuple_element.cc 
b/libstdc++-v3/testsuite/23_containers/array/tuple_interface/tuple_element.cc
index 96e0bfb..65da753 100644
--- 
a/libstdc++-v3/testsuite/23_containers/array/tuple_interface/tuple_element.cc
+++ 
b/libstdc++-v3/testsuite/23_containers/array/tuple_interface/tuple_element.cc
@@ -18,15 +18,18 @@
 // with this library; see the file COPYING3.  If not see
 // .
 
+// NB: Don't include any other headers in this file.
+// LWG 2212 requires  to define tuple_element specializations.
 #include 
-#include 
-#include 
 
 void
 test01() 
 { 
   bool test __attribute__((unused)) = true;
-  using namespace std;
+  using std::array;
+  using std::tuple_element;
+  // This relies on the fact that  includes :
+  using std::is_same;
 
   const size_t len = 3;
   typedef array array_type;
diff --git 
a/libstdc++-v3/testsuite/23_containers/array/tuple_interface/tuple_size.cc 
b/libstdc++-v3/testsuite/23_containers/array/tuple_interface/tuple_size.cc
index 4a95fa7..6fe501e 100644
--- a/libstdc++-v3/testsuite/23_containers/array/tuple_interface/tuple_size.cc
+++ b/libstdc++-v3/testsuite/23_containers/array/tuple_interface/tuple_size.cc
@@ -18,14 +18,18 @@
 // with this library; see the file COPYING3.  If not see
 // .
 
+// NB: Don't include any other headers in this file.
+// LWG 2212 requires  to define tuple_size specializations.
 #include 
-#include 
 
 void
 test01() 
 { 
   bool test __attribute__((unused)) = true;
-  using namespace std;
+  using std::array;
+  using std::tuple_size;
+  // This relies on the fact that  includes :
+  using std::is_same;
 
   {
 const size_t len = 5;

Re: Implement -Wswitch-fallthrough: rs6000

[Segher Boessenkool]

Hi Marek,

On Mon, Jul 11, 2016 at 09:59:39PM +0200, Marek Polacek wrote:
> 2016-07-11  Marek Polacek  
> 
>   PR c/7652
>   * config/rs6000/rs6000.c (rs6000_builtin_vectorized_libmass): Likewise.

Likewise?  Like what?  :-)

> --- gcc/gcc/config/rs6000/rs6000.c
> +++ gcc/gcc/config/rs6000/rs6000.c
> @@ -5459,6 +5459,7 @@ rs6000_builtin_vectorized_libmass (combined_fn fn, tree 
> type_out,
>  CASE_CFN_POW:
>n_args = 2;
>/* fall through */
> +  gcc_fallthrough ();

As mentioned elsewhere, please remove comments saying just "fall through"
when adding the new statement.

> @@ -14398,6 +14401,8 @@ altivec_expand_ld_builtin (tree exp, rtx target, bool 
> *expandedp)
>break;
>  case ALTIVEC_BUILTIN_LD_INTERNAL_2di:
>icode = CODE_FOR_vector_altivec_load_v2di;
> +  /* XXX Really?  */
> +  gcc_fallthrough ();
>  case ALTIVEC_BUILTIN_LD_INTERNAL_1ti:
>icode = CODE_FOR_vector_altivec_load_v1ti;
>break;

Yeah, that's a bug obviously.  We'll fix it.

> @@ -30191,6 +30200,7 @@ rs6000_adjust_cost (rtx_insn *insn, rtx link, 
> rtx_insn *dep_insn, int cost)
>  && (INSN_CODE (dep_insn) >= 0)
>  && (get_attr_type (dep_insn) == TYPE_MFFGPR))
>return 2;
> + gcc_fallthrough ();
>  
>default:
>  break;

Better to put an extra "break" here.  That is usually true if the next
statement (after one or more labels) is a break.

> --- gcc/gcc/config/rs6000/rs6000.md
> +++ gcc/gcc/config/rs6000/rs6000.md
> @@ -8094,6 +8094,8 @@
>  case 0:
>if (TARGET_STRING)
>  return \"stswi %1,%P0,16\";
> +  /* XXX Really fallthru?  */
> +  gcc_fallthrough ();
>  case 1:
>return \"#\";

Yes, really :-)  This code is probably better written without a switch
statement at all, but oh well.

I like the new warning, and I'd like to see it in -Wextra.  It finds real
problems that are easy to correct.  It does trigger a lot on existing code,
but -Wextra is not -Wall.


Segher

Re: Improve insert/emplace robustness to self insertion

[Jonathan Wakely]

On 08/07/16 17:38 +0100, Jonathan Wakely wrote:

On 06/07/16 21:46 +0200, François Dumont wrote:

Don't you plan to add it to the testsuite ?


Done with the attached aptch.


Just for completeness, I'm also adding the example from LWG 2164,
which is related.

Tested x86_64, committed to trunk.


commit 23cb1117d3b5073097ab15fcf9c0245aa98de067
Author: redi 
Date:   Tue Jul 12 14:00:11 2016 +

Add std::vector::emplace() testcase from LWG 2164

	* testsuite/23_containers/vector/modifiers/emplace/self_emplace.cc:
	Add testcase from LWG 2164.

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@238243 138bc75d-0d04-0410-961f-82ee72b054a4

diff --git a/libstdc++-v3/testsuite/23_containers/vector/modifiers/emplace/self_emplace.cc b/libstdc++-v3/testsuite/23_containers/vector/modifiers/emplace/self_emplace.cc
index d452b5b..8712216 100644
--- a/libstdc++-v3/testsuite/23_containers/vector/modifiers/emplace/self_emplace.cc
+++ b/libstdc++-v3/testsuite/23_containers/vector/modifiers/emplace/self_emplace.cc
@@ -135,6 +135,20 @@ test04()
   VERIFY( va[0]._i == 1 );
 }
 
+void
+test05()
+{
+  // LWG DR 2164
+  std::vector v;
+  v.reserve(4);
+  v = { 1, 2, 3 };
+  v.emplace(v.begin(), v.back());
+  VERIFY( v[0] == 3 );
+  VERIFY( v[1] == 1 );
+  VERIFY( v[2] == 2 );
+  VERIFY( v[3] == 3 );
+}
+
 int main()
 {
   test01();

Re: Implement -Wswitch-fallthrough

[Jakub Jelinek]

On Tue, Jul 12, 2016 at 04:08:02PM +0200, Marek Polacek wrote:
> On Tue, Jul 12, 2016 at 09:57:01AM -0400, NightStrike wrote:
> > From http://security.coverity.com/blog/2013/Sep/gimme-a-break.html:
>  
> Thanks, this is useful.
> 
> > We also suppress a case label if there is a comment that matches
> > [^#]fall.?thro?u, even if it's not on the last line.
> 
> Our current plan is to handle sth like
> [ \t]*FALL(S | |-)?THR(OUGH|U)\.?[ \t]*
> [ \t]*Fall(s | |-)?[Tt]hr(ough|u)\.?[ \t]*
> [ \t]*fall(s | |-)?thr(ough|u)\.?[ \t]*

And
/*-fallthrough*/
/*@fallthrough@*/
//-fallthrough
//@fallthrough@
(those 4 as standardized lint comments only in lowercase, the
above regexps are meant to match the whole comment content, i.e.
everything between /* and */ or // and \n.

Jakub

Re: Implement -Wswitch-fallthrough

[Marek Polacek]

On Tue, Jul 12, 2016 at 09:57:01AM -0400, NightStrike wrote:
> From http://security.coverity.com/blog/2013/Sep/gimme-a-break.html:
 
Thanks, this is useful.

> We also suppress a case label if there is a comment that matches
> [^#]fall.?thro?u, even if it's not on the last line.

Our current plan is to handle sth like
[ \t]*FALL(S | |-)?THR(OUGH|U)\.?[ \t]*
[ \t]*Fall(s | |-)?[Tt]hr(ough|u)\.?[ \t]*
[ \t]*fall(s | |-)?thr(ough|u)\.?[ \t]*

> Maybe that's enough?

We'll see ;).

> The page has other exclusions that might be desirable.

Yea, I wonder how do they recognize Duff's device, in particular.
But I think it shouldn't be a big deal if we warn on them.

Marek

Re: [PATCH, rs6000] Fix PR target/71733, ICE with -mcpu=power9 -mno-vsx

[Ulrich Weigand]

Alan Modra wrote:
> On Tue, Jul 12, 2016 at 02:02:43PM +0200, Ulrich Weigand wrote:
> > The second time around, get_secondary_mem should reuse the
> > same stack slot it already allocated, and the elimination
> > offsets should already be set to accommodate that stack slot,
> > which means the second time around, the correct RTX should be
> > generated for the memory access.
> > 
> > Is this not happening somehow?
> 
> Duh, yes, of course.  Second time around the mem is
> (mem/c:V16QI (plus:DI (reg/f:DI 1 1)
> (const_int -16 [0xfff0])) [0  S16 A128])
> so we're checking the correct offset.
> 
> The problem now is that this passes rs6000_legitimate_address_p due to
> mode_supports_vsx_dform_quad and quad_address_p being true.  That
> doesn't seem correct for -mno-vsx.

Hmm, I see in rs6000_option_override_internal:

  /* ISA 3.0 D-form instructions require p9-vector and upper-regs.  */
  if ((TARGET_P9_DFORM_SCALAR || TARGET_P9_DFORM_VECTOR) && !TARGET_P9_VECTOR)
{
  if (rs6000_isa_flags_explicit & OPTION_MASK_P9_VECTOR)
error ("-mpower9-dform requires -mpower9-vector");
  rs6000_isa_flags &= ~(OPTION_MASK_P9_DFORM_SCALAR
| OPTION_MASK_P9_DFORM_VECTOR);
}


and *later*:

  /* ISA 3.0 vector instructions include ISA 2.07.  */
  if (TARGET_P9_VECTOR && !TARGET_P8_VECTOR)
{
  if (rs6000_isa_flags_explicit & OPTION_MASK_P8_VECTOR)
error ("-mpower9-vector requires -mpower8-vector");
  rs6000_isa_flags &= ~OPTION_MASK_P9_VECTOR;
}

That seems the wrong way around ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  ulrich.weig...@de.ibm.com

Re: [PATCH] Fix PR rtl-optimization/71634

[Bernd Schmidt]

On 07/08/2016 04:27 PM, Martin Liška wrote:

On 07/08/2016 02:54 PM, Martin Liška wrote:

On 07/08/2016 01:59 PM, Bernd Schmidt wrote:


Gah, that's not right, that'll swap the numbers of kept/removed loops.

I think the right answer is simply
  for (i = 0; i < n - IRA_MAX_LOOPS_NUM; i++)


Bernd


Thank you for the help, I've been testing the suggested change.

Martin



It survives regression tests and bootstrap.
May I install the patch?


Sure.


Bernd

Re: Implement -Wswitch-fallthrough: rs6000

[Marek Polacek]

On Tue, Jul 12, 2016 at 09:10:54AM -0500, Segher Boessenkool wrote:
> Hi Marek,
> 
> On Mon, Jul 11, 2016 at 09:59:39PM +0200, Marek Polacek wrote:
> > 2016-07-11  Marek Polacek  
> > 
> > PR c/7652
> > * config/rs6000/rs6000.c (rs6000_builtin_vectorized_libmass): Likewise.
> 
> Likewise?  Like what?  :-)
 
Yikes, I goofed this up when spliting the big patch.

> > --- gcc/gcc/config/rs6000/rs6000.c
> > +++ gcc/gcc/config/rs6000/rs6000.c
> > @@ -5459,6 +5459,7 @@ rs6000_builtin_vectorized_libmass (combined_fn fn, 
> > tree type_out,
> >  CASE_CFN_POW:
> >n_args = 2;
> >/* fall through */
> > +  gcc_fallthrough ();
> 
> As mentioned elsewhere, please remove comments saying just "fall through"
> when adding the new statement.
 
Yep, I'm working on this.

> > @@ -14398,6 +14401,8 @@ altivec_expand_ld_builtin (tree exp, rtx target, 
> > bool *expandedp)
> >break;
> >  case ALTIVEC_BUILTIN_LD_INTERNAL_2di:
> >icode = CODE_FOR_vector_altivec_load_v2di;
> > +  /* XXX Really?  */
> > +  gcc_fallthrough ();
> >  case ALTIVEC_BUILTIN_LD_INTERNAL_1ti:
> >icode = CODE_FOR_vector_altivec_load_v1ti;
> >break;
> 
> Yeah, that's a bug obviously.  We'll fix it.
 
Great.

> > @@ -30191,6 +30200,7 @@ rs6000_adjust_cost (rtx_insn *insn, rtx link, 
> > rtx_insn *dep_insn, int cost)
> >  && (INSN_CODE (dep_insn) >= 0)
> >  && (get_attr_type (dep_insn) == TYPE_MFFGPR))
> >return 2;
> > +   gcc_fallthrough ();
> >  
> >default:
> >  break;
> 
> Better to put an extra "break" here.  That is usually true if the next
> statement (after one or more labels) is a break.
 
The next version of the warning should recognize this scenario and shouldn't
warn, thus no change will be needed.

> > --- gcc/gcc/config/rs6000/rs6000.md
> > +++ gcc/gcc/config/rs6000/rs6000.md
> > @@ -8094,6 +8094,8 @@
> >  case 0:
> >if (TARGET_STRING)
> >  return \"stswi %1,%P0,16\";
> > +  /* XXX Really fallthru?  */
> > +  gcc_fallthrough ();
> >  case 1:
> >return \"#\";
> 
> Yes, really :-)  This code is probably better written without a switch
> statement at all, but oh well.
 
Yeah, or it at least should have a comment.

> I like the new warning, and I'd like to see it in -Wextra.  It finds real
> problems that are easy to correct.  It does trigger a lot on existing code,
> but -Wextra is not -Wall.

The next version of the warning should be significantly less verbose, because
it will take "falls through" comments into account.

Thanks.

Marek

Re: Implement -Wswitch-fallthrough

[Bernd Schmidt]

On 07/12/2016 04:14 PM, Jakub Jelinek wrote:

On Tue, Jul 12, 2016 at 04:08:02PM +0200, Marek Polacek wrote:

On Tue, Jul 12, 2016 at 09:57:01AM -0400, NightStrike wrote:

From http://security.coverity.com/blog/2013/Sep/gimme-a-break.html:


Thanks, this is useful.


We also suppress a case label if there is a comment that matches
[^#]fall.?thro?u, even if it's not on the last line.


Our current plan is to handle sth like
[ \t]*FALL(S | |-)?THR(OUGH|U)\.?[ \t]*
[ \t]*Fall(s | |-)?[Tt]hr(ough|u)\.?[ \t]*
[ \t]*fall(s | |-)?thr(ough|u)\.?[ \t]*


And
/*-fallthrough*/
/*@fallthrough@*/
//-fallthrough
//@fallthrough@
(those 4 as standardized lint comments only in lowercase, the
above regexps are meant to match the whole comment content, i.e.
everything between /* and */ or // and \n.


I think this approach is doomed to fail, because it takes only one 
language into account, and fails with preprocessed sources.



Bernd

Re: [C++ RFC/Patch] PR c++/71665

[Paolo Carlini]

Hi Jason,

and sorry about the delay in following up, a few days of vacations...

On 30/06/2016 19:49, Jason Merrill wrote:


I think we should check the type before calling cxx_constant_value.

Ok, I got the point. I'm not sure however how far we want to go with 
this and which kind of consistency we want to achieve (vs error messages 
issued in other similar circumstances). The below certainly passes 
testing on x86_64-linux.


Thanks,
Paolo.

///

Index: cp/decl.c
===
--- cp/decl.c   (revision 238239)
+++ cp/decl.c   (working copy)
@@ -13585,7 +13585,9 @@ build_enumerator (tree name, tree value, tree enum
 
  if (value != NULL_TREE)
{
- value = cxx_constant_value (value);
+ if (TREE_CODE (value) != COMPONENT_REF
+ || !is_overloaded_fn (value))
+   value = cxx_constant_value (value);
 
  if (TREE_CODE (value) != INTEGER_CST
  || ! INTEGRAL_OR_ENUMERATION_TYPE_P (TREE_TYPE (value)))
Index: testsuite/g++.dg/cpp0x/pr71665.C
===
--- testsuite/g++.dg/cpp0x/pr71665.C(revision 0)
+++ testsuite/g++.dg/cpp0x/pr71665.C(working copy)
@@ -0,0 +1,8 @@
+// PR c++/71665
+// { dg-do compile { target c++11 } }
+
+class A 
+{
+  int f ();
+  enum { a = f }; // { dg-error "enumerator value" }
+};

Re: Implement -Wswitch-fallthrough

[Jakub Jelinek]

On Tue, Jul 12, 2016 at 04:22:30PM +0200, Bernd Schmidt wrote:
> On 07/12/2016 04:14 PM, Jakub Jelinek wrote:
> >On Tue, Jul 12, 2016 at 04:08:02PM +0200, Marek Polacek wrote:
> >>On Tue, Jul 12, 2016 at 09:57:01AM -0400, NightStrike wrote:
> >>>From http://security.coverity.com/blog/2013/Sep/gimme-a-break.html:
> >>
> >>Thanks, this is useful.
> >>
> >>>We also suppress a case label if there is a comment that matches
> >>>[^#]fall.?thro?u, even if it's not on the last line.
> >>
> >>Our current plan is to handle sth like
> >>[ \t]*FALL(S | |-)?THR(OUGH|U)\.?[ \t]*
> >>[ \t]*Fall(s | |-)?[Tt]hr(ough|u)\.?[ \t]*
> >>[ \t]*fall(s | |-)?thr(ough|u)\.?[ \t]*
> >
> >And
> >/*-fallthrough*/
> >/*@fallthrough@*/
> >//-fallthrough
> >//@fallthrough@
> >(those 4 as standardized lint comments only in lowercase, the
> >above regexps are meant to match the whole comment content, i.e.
> >everything between /* and */ or // and \n.
> 
> I think this approach is doomed to fail, because it takes only one language
> into account, and fails with preprocessed sources.

It takes C/C++/ObjC/ObjC++ into account (I believe Go has fallthrough
keyword in the language, but perhaps it handles everything in the FE).
For C++11, people can use [[gnu::fallthrough]], for
C++17 [[fallthrough]] (standardized), and we are
going to offer some other way in all cases (__builtin_fallthrough () and/or
#pragma GCC fallthrough).  The parsing of comments is just an attempt to
handle the most common case in user-friendly way.

For non-integrated preprocessing, the options are either let people
use some other way than comment, recommend them to use -C or -CC,
add some new option like -C/-CC that emits during preprocessing
standardized /*FALLTHROUGH*/ comments only when before case/default
keywords or identifier followed by semicolon into the -E output,
or emitting #pragma GCC fallthrough in that case instead.

As usual for warnings, changes are contentious, on the other side as could
be seen in the patchset Marek posted it discovered multiple real bugs in GCC
already.

Jakub

Re: Implement -Wswitch-fallthrough: rs6000

[Bernd Schmidt]

On 07/12/2016 04:19 PM, Marek Polacek wrote:



@@ -30191,6 +30200,7 @@ rs6000_adjust_cost (rtx_insn *insn, rtx link, rtx_insn 
*dep_insn, int cost)
 && (INSN_CODE (dep_insn) >= 0)
 && (get_attr_type (dep_insn) == TYPE_MFFGPR))
   return 2;
+   gcc_fallthrough ();

   default:
 break;


Better to put an extra "break" here.  That is usually true if the next
statement (after one or more labels) is a break.


The next version of the warning should recognize this scenario and shouldn't
warn, thus no change will be needed.


Actually I think this is one case where you could unconditionally warn - 
falling through to a break seems like bad practice, it invites errors 
when someone places new code before the break.



Bernd

Re: Implement -Wswitch-fallthrough: rs6000

[Jakub Jelinek]

On Tue, Jul 12, 2016 at 04:42:21PM +0200, Bernd Schmidt wrote:
> On 07/12/2016 04:19 PM, Marek Polacek wrote:
> >
> >>>@@ -30191,6 +30200,7 @@ rs6000_adjust_cost (rtx_insn *insn, rtx link, 
> >>>rtx_insn *dep_insn, int cost)
> >>> && (INSN_CODE (dep_insn) >= 0)
> >>> && (get_attr_type (dep_insn) == TYPE_MFFGPR))
> >>>   return 2;
> >>>+  gcc_fallthrough ();
> >>>
> >>>   default:
> >>> break;
> >>
> >>Better to put an extra "break" here.  That is usually true if the next
> >>statement (after one or more labels) is a break.
> >
> >The next version of the warning should recognize this scenario and shouldn't
> >warn, thus no change will be needed.
> 
> Actually I think this is one case where you could unconditionally warn -
> falling through to a break seems like bad practice, it invites errors when
> someone places new code before the break.

You get the warning when you place the new code there.
I think the falling into a case with only break; or falling into default:;}
is harmless.

Jakub

Re: [PATCH] - improve sprintf buffer overflow detection (middle-end/49905)

[Martin Sebor]

On 07/12/2016 04:01 AM, Florian Weimer wrote:

On 07/12/2016 11:54 AM, Jakub Jelinek wrote:

On Tue, Jul 12, 2016 at 11:51:50AM +0200, Florian Weimer wrote:

On 07/01/2016 08:15 PM, Martin Sebor wrote:

The attached patch enhances compile-time checking for buffer overflow
and output truncation in non-trivial calls to the sprintf family of
functions under a new option -Wformat-length=[12].  This initial
patch handles printf directives with string, integer, and simple
floating arguments but eventually I'd like to extend it all other
functions and directives for which it makes sense.


I tried your patch with the following code, which is close to a
real-world
example:

#include 

void print (const char *);

void
format_1 (unsigned address)
{
  unsigned char a = address >> 24;
  unsigned char b = address >> 16;
  unsigned char c = address >> 8;
  unsigned char d = address;
  char buf[15];
  sprintf ("%u.%u.%u.%u", buf, a, b, c, d);


Are you sure this is real-world code?  sprintf's first argument is the
buffer and second the format string, so if this doesn't warn at compile
time, it will surely crash at runtime when trying to store into .rodata.


Argh!  You are right, I swapped the arguments.

And further attempts showed that I was missing -D_FORTIFY_SOURCE=2. With
it, I get a nice diagnostic.  Wow!


Thanks for giving it a try!  Based on the feedback I received
I've since updated the patch and will post the latest version
for review soon.  In simple cases like this one it warns even
without _FORTIFY_SOURCE or optimization (though without the
latter it doesn't benefit from VRP information).  Let me see
about adding a warning to detect and warn when the arguments
are transposed.

$ /build/gcc-49905/gcc/xgcc -B /build/gcc-49905/gcc -S -Wall -Wextra 
-Wpedantic -Wformat-length=2 xyz.c


xyz.c: In function ‘format_1’:
xyz.c:13:29: warning: may write a terminating nul past the end of the 
destination [-Wformat-length=]

   sprintf (buf, "%u.%u.%u.%u", a, b, c, d);
 ^
xyz.c:13:3: note: destination size is ‘15’ bytes, output size between 
‘8’ and ‘16’

   sprintf (buf, "%u.%u.%u.%u", a, b, c, d);
   ^~~~
xyz.c: In function ‘format_2’:
xyz.c:21:3: warning: ‘%u’ directive writing between ‘1’ and ‘10’ bytes 
into a region of size ‘4’ [-Wformat-length=]

   sprintf (buf, "%u.%u.%u.%u",
   ^
xyz.c:21:3: note: using the range [‘1u’, ‘2147483648u’] for directive 
argument
xyz.c:21:3: note: destination size is ‘15’ bytes, output size between 
‘4’ and ‘22’

   sprintf (buf, "%u.%u.%u.%u",
   ^~~~
address >> 24,
~~
(address >> 16) & 0xff,
~~~
(address >> 8) & 0xff,
~~
address & 0xff);
~~~
xyz.c:21:3: note: destination size is ‘15’ bytes, output size between 
‘6’ and ‘33’


Martin

Re: [PATCH, ARM 6/7, ping1] Add support for CB(N)Z and (U|S)DIV to ARMv8-M Baseline

[Kyrill Tkachov]

On 12/07/16 11:26, Thomas Preudhomme wrote:

Hi Kyrill,


Hi Thomas,



On Friday 20 May 2016 14:22:48 Kyrill Tkachov wrote:

Hi Thomas,

On 17/05/16 11:14, Thomas Preudhomme wrote:

Ping?

*** gcc/ChangeLog ***

2015-11-13  Thomas Preud'homme  

  * config/arm/arm.c (arm_print_operand_punct_valid_p): Make %?
  valid
  for Thumb-1.
  * config/arm/arm.h (TARGET_HAVE_CBZ): Define.
  (TARGET_IDIV): Set for all Thumb targets provided they have
  hardware
  divide feature.
  * config/arm/thumb1.md (thumb1_cbz): New insn.

diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index
f42e996e5a7ce979fe406b8261d50fb2ba005f6b..347b5b0a5cc0bc1e3b5020c8124d968e
76ce48a4 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -271,9 +271,12 @@ extern void (*arm_lang_output_object_attributes_hook)
(void);

   /* Nonzero if this chip provides the movw and movt instructions.  */
   #define TARGET_HAVE_MOVT (arm_arch_thumb2 || arm_arch8)

+/* Nonzero if this chip provides the cb{n}z instruction.  */
+#define TARGET_HAVE_CBZ(arm_arch_thumb2 || arm_arch8)
+

   /* Nonzero if integer division instructions supported.  */
   #define TARGET_IDIV  ((TARGET_ARM && arm_arch_arm_hwdiv) \

-|| (TARGET_THUMB2 && arm_arch_thumb_hwdiv))
+|| (TARGET_THUMB && arm_arch_thumb_hwdiv))

   /* Nonzero if disallow volatile memory access in IT block.  */
   #define TARGET_NO_VOLATILE_CE(arm_arch_no_volatile_ce)

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index
13b4b71ac8f9c1da8ef1945f7ff6985ca59f6832..445972ce0e3fd27d4411840ff69e9edb
b23994fc 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -22684,7 +22684,7 @@ arm_print_operand_punct_valid_p (unsigned char
code)>
   {
   
 return (code == '@' || code == '|' || code == '.'
 
   	  || code == '(' || code == ')' || code == '#'


- || (TARGET_32BIT && (code == '?'))
+ || code == '?'

  || (TARGET_THUMB2 && (code == '!'))
  || (TARGET_THUMB && (code == '_')));
   
   }

Hmm, I'm not a fan of this change. arm_print_operand_punct_valid_p is an
implementation of a target hook that is used to validate user-provided
inline asm as well and is therefore the right place to reject such invalid
constructs.

This is just working around the fact that the output template for the
[u]divsi3 patterns has a '%?' in it that is illegal in Thumb1 and will not
be used for ARMv8-M Baseline anyway. I'd prefer it if you add a second
alternative to those patterns and emit the sdiv/udiv mnemonic without the
'%?' and enable that for the v8mb arch attribute (and mark the existing
alternative as requiring the "32" arch attribute).

diff --git a/gcc/config/arm/thumb1.md b/gcc/config/arm/thumb1.md
index
4572456b8bc98503061846cad94bc642943db3a2..1b01ef6ce731fe3ff37c3d8c048fb9d5
e7829b35 100644
--- a/gcc/config/arm/thumb1.md
+++ b/gcc/config/arm/thumb1.md
@@ -973,6 +973,92 @@

 DONE;
   
   })


+;; A pattern for the cb(n)z instruction added in ARMv8-M baseline
profile,
+;; adapted from cbranchsi4_insn.  Modifying cbranchsi4_insn instead leads
to +;; code generation difference for ARMv6-M because the minimum length
of the +;; instruction becomes 2 even for it due to a limitation in
genattrtab's +;; handling of pc in the length condition.
+(define_insn "thumb1_cbz"
+  [(set (pc) (if_then_else
+ (match_operator 0 "equality_operator"
+  [(match_operand:SI 1 "s_register_operand" "l")
+   (const_int 0)])
+ (label_ref (match_operand 2 "" ""))
+ (pc)))]
+  "TARGET_THUMB1 && TARGET_HAVE_MOVT"
+{

s/TARGET_HAVE_MOVT/TARGET_HAVE_CBZ/


+  if (get_attr_length (insn) == 2)
+{
+  if (GET_CODE (operands[0]) == EQ)
+   return "cbz\t%1, %l2";
+  else
+   return "cbnz\t%1, %l2";
+}
+  else
+{
+  rtx t = cfun->machine->thumb1_cc_insn;
+  if (t != NULL_RTX)
+   {
+ if (!rtx_equal_p (cfun->machine->thumb1_cc_op0, operands[1])
+ || !rtx_equal_p (cfun->machine->thumb1_cc_op1, operands[2]))
+   t = NULL_RTX;
+ if (cfun->machine->thumb1_cc_mode == CC_NOOVmode)
+   {
+ if (!noov_comparison_operator (operands[0], VOIDmode))
+   t = NULL_RTX;
+   }
+ else if (cfun->machine->thumb1_cc_mode != CCmode)
+   t = NULL_RTX;
+   }
+  if (t == NULL_RTX)
+   {
+ output_asm_insn ("cmp\t%1, #0", operands);
+ cfun->machine->thumb1_cc_insn = insn;
+ cfun->machine->thumb1_cc_op0 = operands[1];
+ cfun->machine->thumb1_cc_op1 = operands[2];
+ cfun->machine->thumb1_cc_mode = CCmode;
+   }
+  else
+   /* Ensure we emit the right type of condition code on the jump.  */
+   XEXP (operands[0], 0) = gen_rtx_REG (cfun->machine->thumb1_cc_mode,
+CC_RE

Re: [PATCH] Fix Fortran DO loop fallback

[Martin Liška]

On 07/12/2016 03:15 PM, Richard Biener wrote:
> The scan for 1 *n_ after FRE looks still useful.  Btw, the testcase
> doesn't fail for me,
> we _do_ hoist the division in PRE, just not with -m32 anymore.  Can
> you confirm this?

Yeah, it works for both -m32 and -m64.
This is final version of the patch, may I install it?

Thanks,
Martin
>From e23c13dcc2f5e1967b0941c7a627241e3b6759e8 Mon Sep 17 00:00:00 2001
From: marxin 
Date: Fri, 8 Jul 2016 15:51:54 +0200
Subject: [PATCH] Fix Fortran DO loop fallback

gcc/testsuite/ChangeLog:

2016-07-08  Martin Liska  

	* gfortran.dg/ldist-1.f90: Revert change introduces in r238114.
	* gfortran.dg/pr42108.f90: Add -fno-ipa-icf to additional
	options.
	* gfortran.dg/vect/pr62283.f: Update expected dump scan.
---
 gcc/testsuite/gfortran.dg/ldist-1.f90| 2 +-
 gcc/testsuite/gfortran.dg/pr42108.f90| 2 --
 gcc/testsuite/gfortran.dg/vect/pr62283.f | 2 +-
 3 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/gcc/testsuite/gfortran.dg/ldist-1.f90 b/gcc/testsuite/gfortran.dg/ldist-1.f90
index 2030328..ea3990d 100644
--- a/gcc/testsuite/gfortran.dg/ldist-1.f90
+++ b/gcc/testsuite/gfortran.dg/ldist-1.f90
@@ -32,4 +32,4 @@ end Subroutine PADEC
 ! There are 5 legal partitions in this code.  Based on the data
 ! locality heuristic, this loop should not be split.
 
-! { dg-final { scan-tree-dump "distributed: split to" "ldist" } }
+! { dg-final { scan-tree-dump-not "distributed: split to" "ldist" } }
diff --git a/gcc/testsuite/gfortran.dg/pr42108.f90 b/gcc/testsuite/gfortran.dg/pr42108.f90
index eb93604..a913aa4 100644
--- a/gcc/testsuite/gfortran.dg/pr42108.f90
+++ b/gcc/testsuite/gfortran.dg/pr42108.f90
@@ -21,7 +21,5 @@ subroutine  eval(foo1,foo2,foo3,foo4,x,n,nnd)
   end do
 end subroutine eval
 
-! We should have hoisted the division
-! { dg-final { scan-tree-dump "in all uses of countm1\[^\n\]* / " "pre" } }
 ! There should be only one load from n left
 ! { dg-final { scan-tree-dump-times "\\*n_" 1 "fre1" } }
diff --git a/gcc/testsuite/gfortran.dg/vect/pr62283.f b/gcc/testsuite/gfortran.dg/vect/pr62283.f
index 7df3d99..4d8cba1 100644
--- a/gcc/testsuite/gfortran.dg/vect/pr62283.f
+++ b/gcc/testsuite/gfortran.dg/vect/pr62283.f
@@ -1,5 +1,5 @@
 C { dg-do compile }
-C { dg-additional-options "-fvect-cost-model=dynamic" }
+C { dg-additional-options "-fvect-cost-model=dynamic -fno-ipa-icf" }
   subroutine test2(x,y)
   real x(4),y(4)
   beta=3.141593
-- 
2.8.4

Re: [PATCH] - improve sprintf buffer overflow detection (middle-end/49905)

[David Malcolm]

On Tue, 2016-07-12 at 08:45 -0600, Martin Sebor wrote:
> On 07/12/2016 04:01 AM, Florian Weimer wrote:
> > On 07/12/2016 11:54 AM, Jakub Jelinek wrote:
> > > On Tue, Jul 12, 2016 at 11:51:50AM +0200, Florian Weimer wrote:
> > > > On 07/01/2016 08:15 PM, Martin Sebor wrote:
> > > > > The attached patch enhances compile-time checking for buffer
> > > > > overflow
> > > > > and output truncation in non-trivial calls to the sprintf
> > > > > family of
> > > > > functions under a new option -Wformat-length=[12].  This
> > > > > initial
> > > > > patch handles printf directives with string, integer, and
> > > > > simple
> > > > > floating arguments but eventually I'd like to extend it all
> > > > > other
> > > > > functions and directives for which it makes sense.
> > > > 
> > > > I tried your patch with the following code, which is close to a
> > > > real-world
> > > > example:
> > > > 
> > > > #include 
> > > > 
> > > > void print (const char *);
> > > > 
> > > > void
> > > > format_1 (unsigned address)
> > > > {
> > > >   unsigned char a = address >> 24;
> > > >   unsigned char b = address >> 16;
> > > >   unsigned char c = address >> 8;
> > > >   unsigned char d = address;
> > > >   char buf[15];
> > > >   sprintf ("%u.%u.%u.%u", buf, a, b, c, d);
> > > 
> > > Are you sure this is real-world code?  sprintf's first argument
> > > is the
> > > buffer and second the format string, so if this doesn't warn at
> > > compile
> > > time, it will surely crash at runtime when trying to store into
> > > .rodata.
> > 
> > Argh!  You are right, I swapped the arguments.
> > 
> > And further attempts showed that I was missing -D_FORTIFY_SOURCE=2.
> > With
> > it, I get a nice diagnostic.  Wow!

Does it warn for the code that Florian actually posted?  I tried it
with a recent (unpatched) build of trunk and got no warning (with -O2 
-Wall -D_FORTIFY_SOURCE=2), but it strikes me that we ought to warn if
someone passes about the above code (for the uninitialized format
string, at least; I don't know if it's legal to pass a string literal
as the destination).

Should I file a PR for this?

> Thanks for giving it a try!  Based on the feedback I received
> I've since updated the patch and will post the latest version
> for review soon.  In simple cases like this one it warns even
> without _FORTIFY_SOURCE or optimization (though without the
> latter it doesn't benefit from VRP information).  Let me see
> about adding a warning to detect and warn when the arguments
> are transposed.
> 
> $ /build/gcc-49905/gcc/xgcc -B /build/gcc-49905/gcc -S -Wall -Wextra 
> -Wpedantic -Wformat-length=2 xyz.c
> 
> xyz.c: In function ‘format_1’:
> xyz.c:13:29: warning: may write a terminating nul past the end of the
> destination [-Wformat-length=]
> sprintf (buf, "%u.%u.%u.%u", a, b, c, d);
>   ^
> xyz.c:13:3: note: destination size is ‘15’ bytes, output size between
> ‘8’ and ‘16’
> sprintf (buf, "%u.%u.%u.%u", a, b, c, d);
> ^~~~

Style question: should the numbers in these diagnostic messages be in
quotes?  That looks a bit strange to me.  It seems clearer to me to
have:

xyz.c:13:3: note: destination size is 15 bytes, output size between 8
and 16

> xyz.c: In function ‘format_2’:
> xyz.c:21:3: warning: ‘%u’ directive writing between ‘1’ and ‘10’
> bytes 
> into a region of size ‘4’ [-Wformat-length=]
> sprintf (buf, "%u.%u.%u.%u",
> ^
> xyz.c:21:3: note: using the range [‘1u’, ‘2147483648u’] for directive
> argument
> xyz.c:21:3: note: destination size is ‘15’ bytes, output size between
> ‘4’ and ‘22’
> sprintf (buf, "%u.%u.%u.%u",
> ^~~~
>  address >> 24,
>  ~~
>  (address >> 16) & 0xff,
>  ~~~
>  (address >> 8) & 0xff,
>  ~~
>  address & 0xff);
>  ~~~
> xyz.c:21:3: note: destination size is ‘15’ bytes, output size between
> ‘6’ and ‘33’
> 
> Martin

Re: [PATCH] Fix PR rtl-optimization/71634

[Martin Liška]

On 07/12/2016 04:18 PM, Bernd Schmidt wrote:
> 
> 
> On 07/08/2016 04:27 PM, Martin Liška wrote:
>> On 07/08/2016 02:54 PM, Martin Liška wrote:
>>> On 07/08/2016 01:59 PM, Bernd Schmidt wrote:

 Gah, that's not right, that'll swap the numbers of kept/removed loops.

 I think the right answer is simply
   for (i = 0; i < n - IRA_MAX_LOOPS_NUM; i++)


 Bernd
>>>
>>> Thank you for the help, I've been testing the suggested change.
>>>
>>> Martin
>>>
>>
>> It survives regression tests and bootstrap.
>> May I install the patch?
> 
> Sure.
> 
> 
> Bernd
> 

All active branches are also affected, hope it's fine that I'll commit the same
patch as soon as regression tests and bootstrap finishes?

Thanks,
Martin

Re: [PATCH], PowerPC support to enable -mlra and/or -mfloat128

[Bernd Schmidt]

On 07/12/2016 02:48 PM, Bernd Schmidt wrote:

No. You can reproduce issues with Blackfin easily by enabling LRA for
it, and I described the C6X issues when the LRA patches were posted for
review.


That was here:
  https://gcc.gnu.org/ml/gcc-patches/2012-10/msg00235.html

The Blackfin thing happens frequently with -fomit-frame-pointer when we have

(insn 66 64 70 8 (set (reg:SI 96 [ ivtmp.32 ])
(reg/f:SI 15 FP)) 19 {*movsi_insn}

Which LRA transforms to an invalid insn:

(insn 66 64 70 8 (set (reg:SI 15 FP [orig:96 ivtmp.32 ] [96])
(plus:SI (reg/f:SI 14 SP)
(const_int 4 [0x4]))) 50 {addsi3}
 (expr_list:REG_EQUAL (reg/f:SI 15 FP)
(nil)))

Haven't fully debugged it but it looks like an instance of the same 
problem: not using the correct register numbers in elimination. An FP+FP 
addition would be fine (which is how I'm guessing we arrived at this 
pattern), but once you substitute the real register number you get an 
invalid insn. So LRA is somewhat defective in this area.



Bernd

Re: [PATCH 2/3] Run profile feedback tests with autofdo

[Bin.Cheng]

On Thu, Jun 23, 2016 at 3:37 PM, Andi Kleen  wrote:
> From: ak 
>
> Extend the existing bprob and tree-prof tests to also run with autofdo.
> The test runtimes are really a bit too short for autofdo, but it's
> a reasonable sanity check.
>
> This only works natively for now.
>
> dejagnu doesn't seem to support a wrapper for unix tests, so I had
> to open code running these tests.  That should be ok due to the
> native run restrictions.
>
> gcc/testsuite/:
>
> 2016-06-23  Andi Kleen  
>
> * g++.dg/bprob/bprob.exp: Support autofdo.
> * g++.dg/tree-prof/tree-prof.exp: dito.
> * gcc.dg/tree-prof/tree-prof.exp: dito.
> * gcc.misc-tests/bprob.exp: dito.
> * gfortran.dg/prof/prof.exp: dito.
> * lib/profopt.exp: dito.
> * lib/target-supports.exp: Check for autofdo.
>
> git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@237732 
> 138bc75d-0d04-0410-961f-82ee72b054a4
> ---
>  gcc/testsuite/ChangeLog  | 10 
>  gcc/testsuite/g++.dg/bprob/bprob.exp |  8 +++
>  gcc/testsuite/g++.dg/tree-prof/tree-prof.exp |  8 +++
>  gcc/testsuite/gcc.dg/tree-prof/tree-prof.exp |  8 +++
>  gcc/testsuite/gcc.misc-tests/bprob.exp   |  7 +++
>  gcc/testsuite/gfortran.dg/prof/prof.exp  |  7 +++
>  gcc/testsuite/lib/profopt.exp| 82 
> +++-
>  gcc/testsuite/lib/target-supports.exp| 37 +
>  8 files changed, 164 insertions(+), 3 deletions(-)
>
> diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
> index 287baf6..55f8dbf 100644
> --- a/gcc/testsuite/ChangeLog
> +++ b/gcc/testsuite/ChangeLog
> @@ -1,3 +1,13 @@
> +2016-06-23  Andi Kleen  
> +
> +   * g++.dg/bprob/bprob.exp: Support autofdo.
> +   * g++.dg/tree-prof/tree-prof.exp: dito.
> +   * gcc.dg/tree-prof/tree-prof.exp: dito.
> +   * gcc.misc-tests/bprob.exp: dito.
> +   * gfortran.dg/prof/prof.exp: dito.
> +   * lib/profopt.exp: dito.
> +   * lib/target-supports.exp: Check for autofdo.
> +
>  2016-06-23  Martin Liska  
>
> * gcc.dg/pr71619.c: New test.
> diff --git a/gcc/testsuite/g++.dg/bprob/bprob.exp 
> b/gcc/testsuite/g++.dg/bprob/bprob.exp
> index d07..4818298 100644
> --- a/gcc/testsuite/g++.dg/bprob/bprob.exp
> +++ b/gcc/testsuite/g++.dg/bprob/bprob.exp
> @@ -53,6 +53,7 @@ if $tracelevel then {
>
>  set profile_options "-fprofile-arcs"
>  set feedback_options "-fbranch-probabilities"
> +set profile_wrapper ""
>
>  # Main loop.
>  foreach profile_option $profile_options feedback_option $feedback_options {
> @@ -65,4 +66,11 @@ foreach profile_option $profile_options feedback_option 
> $feedback_options {
>  }
>  }
>
> +foreach src [lsort [glob -nocomplain $srcdir/$subdir/*.C]] {
> +if ![runtest_file_p $runtests $src] then {
> +continue
> +}
> +auto-profopt-execute $src
> +}
> +
>  set PROFOPT_OPTIONS $bprob_save_profopt_options
> diff --git a/gcc/testsuite/g++.dg/tree-prof/tree-prof.exp 
> b/gcc/testsuite/g++.dg/tree-prof/tree-prof.exp
> index 7a4b5cb..26ee0b3 100644
> --- a/gcc/testsuite/g++.dg/tree-prof/tree-prof.exp
> +++ b/gcc/testsuite/g++.dg/tree-prof/tree-prof.exp
> @@ -44,6 +44,7 @@ set PROFOPT_OPTIONS [list {}]
>  # profile data.
>  set profile_option "-fprofile-generate -D_PROFILE_GENERATE"
>  set feedback_option "-fprofile-use -D_PROFILE_USE"
> +set profile_wrapper ""
>
>  foreach src [lsort [glob -nocomplain $srcdir/$subdir/*.C]] {
>  # If we're only testing specific files and this isn't one of them, skip 
> it.
> @@ -53,4 +54,11 @@ foreach src [lsort [glob -nocomplain $srcdir/$subdir/*.C]] 
> {
>  profopt-execute $src
>  }
>
> +foreach src [lsort [glob -nocomplain $srcdir/$subdir/*.C]] {
> +if ![runtest_file_p $runtests $src] then {
> +continue
> +}
> +auto-profopt-execute $src
> +}
> +
>  set PROFOPT_OPTIONS $treeprof_save_profopt_options
> diff --git a/gcc/testsuite/gcc.dg/tree-prof/tree-prof.exp 
> b/gcc/testsuite/gcc.dg/tree-prof/tree-prof.exp
> index 650ad8d..aaccf19 100644
> --- a/gcc/testsuite/gcc.dg/tree-prof/tree-prof.exp
> +++ b/gcc/testsuite/gcc.dg/tree-prof/tree-prof.exp
> @@ -44,6 +44,7 @@ set PROFOPT_OPTIONS [list {}]
>  # profile data.
>  set profile_option "-fprofile-generate -D_PROFILE_GENERATE"
>  set feedback_option "-fprofile-use -D_PROFILE_USE"
> +set profile_wrapper ""
>
>  foreach src [lsort [glob -nocomplain $srcdir/$subdir/*.c]] {
>  # If we're only testing specific files and this isn't one of them, skip 
> it.
> @@ -53,4 +54,11 @@ foreach src [lsort [glob -nocomplain $srcdir/$subdir/*.c]] 
> {
>  profopt-execute $src
>  }
>
> +foreach src [lsort [glob -nocomplain $srcdir/$subdir/*.c]] {
> +if ![runtest_file_p $runtests $src] then {
> +continue
> +}
> +auto-profopt-execute $src
> +}
> +
>  set PROFOPT_OPTIONS $treeprof_save_profopt_options
> diff --git a/gcc/testsuite/gcc.misc-tests/bprob.exp 
> b/gcc/testsuite/gcc.misc-tests/bprob.exp
> index 52dcb1f..132bfe3 100644
> 

Re: [PATCH] Introduce new param: AVG_LOOP_NITER

[Martin Liška]

On 07/11/2016 04:35 PM, Jeff Law wrote:
> Don't you need a corresponding change to invoke.texi?
> 
> OK with that change.
> 
> jeff

Thanks, I've just mentioned the param in invoke.texi, installed as r238252.

Martin
>From c4c456c2bea9225c287f7af0e88f39cf780f815d Mon Sep 17 00:00:00 2001
From: marxin 
Date: Tue, 21 Jun 2016 14:52:31 +0200
Subject: [PATCH] Introduce new param: AVG_LOOP_NITER

gcc/ChangeLog:

2016-06-21  Martin Liska  

	* params.def: Add avg-loop niter.
	* tree-ssa-loop-ivopts.c (avg_loop_niter): Use the param.
	* cfgloopanal.c (expected_loop_iterations_unbounded): Likewise.
	* doc/invoke.texi: Document the new parameter.
---
 gcc/cfgloopanal.c  | 9 -
 gcc/doc/invoke.texi| 3 +++
 gcc/params.def | 5 +
 gcc/tree-ssa-loop-ivopts.c | 7 +++
 4 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/gcc/cfgloopanal.c b/gcc/cfgloopanal.c
index c163986..2739f44 100644
--- a/gcc/cfgloopanal.c
+++ b/gcc/cfgloopanal.c
@@ -241,10 +241,9 @@ expected_loop_iterations_unbounded (const struct loop *loop,
   if (read_profile_p)
 *read_profile_p = false;
 
-  /* Average loop rolls about 3 times. If we have no profile at all, it is
- best we can do.  */
+  /* If we have no profile at all, use AVG_LOOP_NITER.  */
   if (profile_status_for_fn (cfun) == PROFILE_ABSENT)
-expected = 3;
+expected = PARAM_VALUE (PARAM_AVG_LOOP_NITER);
   else if (loop->latch->count || loop->header->count)
 {
   gcov_type count_in, count_latch;
@@ -282,9 +281,9 @@ expected_loop_iterations_unbounded (const struct loop *loop,
 
   if (freq_in == 0)
 	{
-	  /* If we have no profile at all, expect 3 iterations.  */
+	  /* If we have no profile at all, use AVG_LOOP_NITER iterations.  */
 	  if (!freq_latch)
-	expected = 3;
+	expected = PARAM_VALUE (PARAM_AVG_LOOP_NITER);
 	  else
 	expected = freq_latch * 2;
 	}
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 997faa1..88fff05 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -9150,6 +9150,9 @@ If the number of candidates in the set is smaller than this value,
 always try to remove unnecessary ivs from the set
 when adding a new one.
 
+@item avg-loop-niter
+Average number of iterations of a loop.
+
 @item scev-max-expr-size
 Bound on size of expressions used in the scalar evolutions analyzer.
 Large expressions slow the analyzer.
diff --git a/gcc/params.def b/gcc/params.def
index 894b7f3..b86d592 100644
--- a/gcc/params.def
+++ b/gcc/params.def
@@ -527,6 +527,11 @@ DEFPARAM(PARAM_IV_ALWAYS_PRUNE_CAND_SET_BOUND,
 	 "If number of candidates in the set is smaller, we always try to remove unused ivs during its optimization.",
 	 10, 0, 0)
 
+DEFPARAM(PARAM_AVG_LOOP_NITER,
+	 "avg-loop-niter",
+	 "Average number of iterations of a loop.",
+	 10, 1, 0)
+
 DEFPARAM(PARAM_SCEV_MAX_EXPR_SIZE,
  	 "scev-max-expr-size",
 	 "Bound on size of expressions used in the scalar evolutions analyzer.",
diff --git a/gcc/tree-ssa-loop-ivopts.c b/gcc/tree-ssa-loop-ivopts.c
index f5f6e78..20cf9ef 100644
--- a/gcc/tree-ssa-loop-ivopts.c
+++ b/gcc/tree-ssa-loop-ivopts.c
@@ -115,8 +115,6 @@ along with GCC; see the file COPYING3.  If not see
 /* The infinite cost.  */
 #define INFTY 1000
 
-#define AVG_LOOP_NITER(LOOP) 5
-
 /* Returns the expected number of loop iterations for LOOP.
The average trip count is computed from profile data if it
exists. */
@@ -128,8 +126,9 @@ avg_loop_niter (struct loop *loop)
   if (niter == -1)
 {
   niter = likely_max_stmt_executions_int (loop);
-  if (niter == -1 || niter > AVG_LOOP_NITER (loop))
-	return AVG_LOOP_NITER (loop);
+
+  if (niter == -1 || niter > PARAM_VALUE (PARAM_AVG_LOOP_NITER))
+	return PARAM_VALUE (PARAM_AVG_LOOP_NITER);
 }
 
   return niter;
-- 
2.8.4

Re: [PATCH] - improve sprintf buffer overflow detection (middle-end/49905)

[Martin Sebor]

Does it warn for the code that Florian actually posted?  I tried it
with a recent (unpatched) build of trunk and got no warning (with -O2
-Wall -D_FORTIFY_SOURCE=2), but it strikes me that we ought to warn if
someone passes about the above code (for the uninitialized format
string, at least; I don't know if it's legal to pass a string literal
as the destination).

Should I file a PR for this?


It doesn't warn for it because in C, string literals are of type
char[] and so implicitly convertible to char* (shudder).  GCC
does warn for it with -Wwrite-strings or -Wdiscarded-qualifiers,
and will give a pedantic warning in C++ (it seems an error would
be preferable).




Thanks for giving it a try!  Based on the feedback I received
I've since updated the patch and will post the latest version
for review soon.  In simple cases like this one it warns even
without _FORTIFY_SOURCE or optimization (though without the
latter it doesn't benefit from VRP information).  Let me see
about adding a warning to detect and warn when the arguments
are transposed.

$ /build/gcc-49905/gcc/xgcc -B /build/gcc-49905/gcc -S -Wall -Wextra
-Wpedantic -Wformat-length=2 xyz.c

xyz.c: In function ‘format_1’:
xyz.c:13:29: warning: may write a terminating nul past the end of the
destination [-Wformat-length=]
 sprintf (buf, "%u.%u.%u.%u", a, b, c, d);
   ^
xyz.c:13:3: note: destination size is ‘15’ bytes, output size between
‘8’ and ‘16’
 sprintf (buf, "%u.%u.%u.%u", a, b, c, d);
 ^~~~


Style question: should the numbers in these diagnostic messages be in
quotes?  That looks a bit strange to me.  It seems clearer to me to
have:


You're probably right.  I suspect I have a tendency to overuse
the quotes (e.g, the -Wplacement-new warning also quotes the
sizes).  If there aren't yet (I vague recall coming across
something on the GCC Wiki but can't find it now), it would be
helpful to put in place some diagnostic style conventions like
there are for formatting code to guide us in cases like this.
I'm willing to help put the document together or add this to
it if one already exists.

Martin

[PATCH] Remove unused operator delete overloads (LWG 2458)

[Jonathan Wakely]

* libsupc++/new: Remove nothrow sized deletes (LWG 2458).
* doc/xml/manual/intro.xml: Document DR 2458 status.
* doc/html*: Regenerate.

Tested x86_64-linux, committed to trunk.

commit b5a6c1e6027f23c5501e9767821711d4506f9412
Author: redi 
Date:   Tue Jul 12 14:31:04 2016 +

Remove unused operator delete overloads (LWG 2458)

* libsupc++/new: Remove nothrow sized deletes (LWG 2458).
* doc/xml/manual/intro.xml: Document DR 2458 status.
* doc/html*: Regenerate.

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@238246 
138bc75d-0d04-0410-961f-82ee72b054a4

diff --git a/libstdc++-v3/doc/xml/manual/intro.xml 
b/libstdc++-v3/doc/xml/manual/intro.xml
index a47a3ec..7b836cd 100644
--- a/libstdc++-v3/doc/xml/manual/intro.xml
+++ b/libstdc++-v3/doc/xml/manual/intro.xml
@@ -1019,6 +1019,13 @@ requirements of the license of GCC.
 Make noexcept specifications conditional.
 
 
+http://www.w3.org/1999/xlink"; 
xlink:href="../ext/lwg-defects.html#2458">2458:
+   N3778 and new library deallocation signatures
+   
+
+Remove unused overloads.
+
+
 http://www.w3.org/1999/xlink"; 
xlink:href="../ext/lwg-defects.html#2459">2459:
std::polar should require a non-negative rho

diff --git a/libstdc++-v3/libsupc++/new b/libstdc++-v3/libsupc++/new
index 5cd6269..8e8a327 100644
--- a/libstdc++-v3/libsupc++/new
+++ b/libstdc++-v3/libsupc++/new
@@ -135,12 +135,6 @@ void operator delete(void*, const std::nothrow_t&) 
_GLIBCXX_USE_NOEXCEPT
   __attribute__((__externally_visible__));
 void operator delete[](void*, const std::nothrow_t&) _GLIBCXX_USE_NOEXCEPT
   __attribute__((__externally_visible__));
-#if __cpp_sized_deallocation
-void operator delete(void*, std::size_t, const std::nothrow_t&) 
_GLIBCXX_USE_NOEXCEPT
-  __attribute__((__externally_visible__));
-void operator delete[](void*, std::size_t, const std::nothrow_t&) 
_GLIBCXX_USE_NOEXCEPT
-  __attribute__((__externally_visible__));
-#endif
 
 // Default placement versions of operator new.
 inline void* operator new(std::size_t, void* __p) _GLIBCXX_USE_NOEXCEPT

Re: Implement -Wswitch-fallthrough

[Jeff Law]

On 07/12/2016 04:30 AM, Marek Polacek wrote:

On Tue, Jul 12, 2016 at 12:27:31PM +0200, Richard Biener wrote:

On Mon, Jul 11, 2016 at 9:43 PM, Marek Polacek  wrote:

The switch fallthrough has been widely considered a design defect in C, a
misfeature or, to use Marshall Cline's definition, evil.  The overwhelming
majority of the time you don't want to fall through to the next case, but it is
easy to forget to "break" at the end of the case, making this far too error
prone.  Yet GCC (and probably other compilers, too) doesn't have the ability to
warn in this case.  A feature request for such warning was opened back in 2002,
but it's mostly been untouched since.  But then the [[fallthrough]] attribute 
was
approved for C++17 [1], and that's what has got me to do all this.


I don't like this too much given the churn it requires in GCC itself.
If [[fallthrough]]
was approved for C++17 is there sth similar proposed for C?  Like a keyword
__Fallthrough?


I don't think there is.
But maybe there should be ;-)  We've got a couple folks (Martin S and 
Carlos) that participate in that standards body and could propose it.


jeff

Re: [ARM] no-data-is-text-relative & msingle-pic-base

[Ramana Radhakrishnan]

On 12/07/16 13:02, Nathan Sidwell wrote:
> Ramana,
> could you review this?


Sorry missed this.
> 
> original thread https://gcc.gnu.org/ml/gcc-patches/2016-05/msg00630.html, 
> reproduced below:
> 
> 
> currently, the documentation for -mno-pic-data-is-text-relative (-mno-PDITR) 
> says
> 'Assume that each data segments are relative to text segment at load time.
>  Therefore, it permits addressing data using PC-relative operations.
>  This option is on by default for targets other than VxWorks RTP.'
> 
> However, if you use just this option, you still end up with a pic-register 
> init sequence that presumes a fixed mapping. That's a surprise. Joey tells me 
> its expected use is with -msingle-pic-base (-mSPB), which reserves a global 
> register to point at the (single) GOT. That's what I had expected the 
> -mno-PDITR option to have implied.
> 
> Apparently there are legitimate reasons one might want the -mno-PDITR 
> behaviour without -mSPB. I don't know what those are though.
> 
> Anyway, IMHO that is the rare case and the more common case is that one would 
> want to have -mnoPDITR imply -mSPB. (The reverse probably doesn't apply.)
> 
> This patch does 3 things.
> 1) have -mno-PDITR imply -mSPB, unless one has explicitly provided -m[no-]SPB.
> 2) clarified  the -m[no-]PDITR documentation.
> 3) Added some testcases -- there didn't appear to be any.
> 
> ok?
> 

Ok and thank you for the testcases. -mno-PDITR => -mSPB by default (in the 
absence of -mno-SPB on the command line) seems correct after having done the 
necessary archeology.

I am also slightly inclined to go further and error out if someone uses 
-mno-PDITR with -mno-SPB on the command line, after all as you say -mno-PDITR 
implies a non-fixed mapping while -mno-SPB implies there is some fixed mapping 
some where currently in the compiler. I don't see how the twain can meet. That 
can happen as a follow-up - the current patch is by itself a step improvement.


Thanks,
Ramana

> nathan

Re: [ARM] no-data-is-text-relative & msingle-pic-base

[Nathan Sidwell]

On 07/12/16 12:01, Ramana Radhakrishnan wrote:


I am also slightly inclined to go further and error out if someone uses 
-mno-PDITR with -mno-SPB on the command line, after all as you say -mno-PDITR 
implies a non-fixed mapping while -mno-SPB implies there is some fixed mapping 
some where currently in the compiler. I don't see how the twain can meet.


That was my original thought too, but Joey told me that such use cases exist.

nathan

Re: Implement -Wswitch-fallthrough

[Marek Polacek]

On Tue, Jul 12, 2016 at 09:59:37AM -0600, Jeff Law wrote:
> On 07/12/2016 04:30 AM, Marek Polacek wrote:
> > On Tue, Jul 12, 2016 at 12:27:31PM +0200, Richard Biener wrote:
> > > If [[fallthrough]]
> > > was approved for C++17 is there sth similar proposed for C?  Like a 
> > > keyword
> > > __Fallthrough?
> > 
> > I don't think there is.
> But maybe there should be ;-)  We've got a couple folks (Martin S and
> Carlos) that participate in that standards body and could propose it.

I think you're right.  I'd happily participate in proposing / implementing
__Fallthrough.

Marek

Re: Implement -Wswitch-fallthrough

[Jeff Law]

On 07/11/2016 01:43 PM, Marek Polacek wrote:

The switch fallthrough has been widely considered a design defect in C, a
misfeature or, to use Marshall Cline's definition, evil.  The overwhelming
majority of the time you don't want to fall through to the next case, but it is
easy to forget to "break" at the end of the case, making this far too error
prone.  Yet GCC (and probably other compilers, too) doesn't have the ability to
warn in this case.  A feature request for such warning was opened back in 2002,
but it's mostly been untouched since.  But then the [[fallthrough]] attribute 
was
approved for C++17 [1], and that's what has got me to do all this.

[ ... ]
This is going to be rather contentious.  So as we're hashing through 
that, can you split off the patches which fix bugs rather than those 
which add fallthru annotations?

jeff

Re: [PATCH] - improve sprintf buffer overflow detection (middle-end/49905)

[Manuel López-Ibáñez]

On 12/07/16 16:59, Martin Sebor wrote:

You're probably right.  I suspect I have a tendency to overuse
the quotes (e.g, the -Wplacement-new warning also quotes the
sizes).  If there aren't yet (I vague recall coming across
something on the GCC Wiki but can't find it now), it would be
helpful to put in place some diagnostic style conventions like
there are for formatting code to guide us in cases like this.
I'm willing to help put the document together or add this to
it if one already exists.


https://gcc.gnu.org/wiki/DiagnosticsGuidelines

RE: [ARM] no-data-is-text-relative & msingle-pic-base

[Joey Ye]

Ramana, Nathan,

My opinion was "there is nothing wrong to run application 
-mno-pic-data-is-text-relative without -msingle-pic-base in a system that 
offset of data and text it fixed. I am not convenience we should ban it." 

However, what Ramana is suggesting is to error out -mno-PDITR with explicit 
-mno-SPB, which I don't have a strong preference embrace it or not as it will 
be just a rare and wired command line combination.

Thanks,
Joey

> -Original Message-
> From: Nathan Sidwell [mailto:nathanmsidw...@gmail.com] On Behalf Of
> Nathan Sidwell
> Sent: 12 July 2016 17:07
> To: Ramana Radhakrishnan
> Cc: GCC Patches; Joey Ye; nd
> Subject: Re: [ARM] no-data-is-text-relative & msingle-pic-base
> 
> On 07/12/16 12:01, Ramana Radhakrishnan wrote:
> 
> > I am also slightly inclined to go further and error out if someone uses -
> mno-PDITR with -mno-SPB on the command line, after all as you say -mno-
> PDITR implies a non-fixed mapping while -mno-SPB implies there is some
> fixed mapping some where currently in the compiler. I don't see how the
> twain can meet.
> 
> That was my original thought too, but Joey told me that such use cases
> exist.
> 
> nathan

Re: [PATCH], PowerPC support to enable -mlra and/or -mfloat128

[Segher Boessenkool]

On Tue, Jul 12, 2016 at 05:17:04PM +0200, Bernd Schmidt wrote:
> The Blackfin thing happens frequently with -fomit-frame-pointer when we have
> 
> (insn 66 64 70 8 (set (reg:SI 96 [ ivtmp.32 ])
> (reg/f:SI 15 FP)) 19 {*movsi_insn}
> 
> Which LRA transforms to an invalid insn:
> 
> (insn 66 64 70 8 (set (reg:SI 15 FP [orig:96 ivtmp.32 ] [96])
> (plus:SI (reg/f:SI 14 SP)
> (const_int 4 [0x4]))) 50 {addsi3}
>  (expr_list:REG_EQUAL (reg/f:SI 15 FP)
> (nil)))
> 
> Haven't fully debugged it but it looks like an instance of the same 
> problem: not using the correct register numbers in elimination. An FP+FP 
> addition would be fine (which is how I'm guessing we arrived at this 
> pattern), but once you substitute the real register number you get an 
> invalid insn. So LRA is somewhat defective in this area.

Do you have a testcase?  This sounds like fun :-)


Segher

Re: [PATCH] Fix FFI return type for closures in the java interpreter

[Tom Tromey]

> "Matthew" == Matthew Fortune  writes:

Matthew> Sorry for the long delay...

No problem.

>> This is ok.
>> Could you check?  I think a -m32 build ought to show it.  Maybe your
>> x86-64 build already did this?

Matthew> Still OK to commit?

Yes, thanks.

Tom

Re: [PATCH], PowerPC support to enable -mlra and/or -mfloat128

[Bernd Schmidt]

On 07/12/2016 06:31 PM, Segher Boessenkool wrote:


Do you have a testcase?  This sounds like fun :-)


This is bfin-elf, gcc.c-torture/compile/pr59417.c, -O3 -fomit-frame-pointer.


Bernd

Re: [PATCH] Fix FFI return type for proxy classes

[Tom Tromey]

> "Matthew" == Matthew Fortune  writes:

Matthew> Tested on: x86_64-pc-linux-gnu (default and -m32), mips-linux-gnu
Matthew> mipsel-linux-gnuabi64 with no regressions. The new test only failed
Matthew> on mips-linux-gnu prior to patching libjava.

Matthew> libjava/
Matthew>* java/lang/reflect/natVMProxy.cc (unbox): Use ffi_arg for
Matthew>integer return types smaller than a word.
Matthew>* testsuite/libjava.jar/ReturnInvocationHandler.java: New file.
Matthew>* testsuite/libjava.jar/ReturnProxyTest.jar: Likewise.
Matthew>* testsuite/libjava.jar/ReturnProxyTest.java: Likewise.
Matthew>* testsuite/libjava.jar/ReturnProxyTest.out: Likewise.
Matthew>* testsuite/libjava.jar/ReturnProxyTest.xfail: Likewise.
Matthew>* testsuite/libjava.jar/ReturnTypes.java: Likewise.
Matthew>* testsuite/libjava.jar/ReturnTypesImpl.java: Likewise.

Thanks for writing this.
This is ok.

Tom

Re: Implement -Wswitch-fallthrough

[Martin Sebor]

On 07/12/2016 10:04 AM, Marek Polacek wrote:

On Tue, Jul 12, 2016 at 09:59:37AM -0600, Jeff Law wrote:

On 07/12/2016 04:30 AM, Marek Polacek wrote:

On Tue, Jul 12, 2016 at 12:27:31PM +0200, Richard Biener wrote:

If [[fallthrough]]
was approved for C++17 is there sth similar proposed for C?  Like a keyword
__Fallthrough?


I don't think there is.

But maybe there should be ;-)  We've got a couple folks (Martin S and
Carlos) that participate in that standards body and could propose it.


I think you're right.  I'd happily participate in proposing / implementing
__Fallthrough.


Given that the C++ attribute has been approved for C++ 17 I think
it would make sense to consider its equivalent for C2X.  I can help
you put a proposal together for the upcoming C meeting in October.

Martin

Re: Implement -Wswitch-fallthrough

[Marek Polacek]

On Tue, Jul 12, 2016 at 11:40:01AM -0600, Martin Sebor wrote:
> On 07/12/2016 10:04 AM, Marek Polacek wrote:
> > On Tue, Jul 12, 2016 at 09:59:37AM -0600, Jeff Law wrote:
> > > On 07/12/2016 04:30 AM, Marek Polacek wrote:
> > > > On Tue, Jul 12, 2016 at 12:27:31PM +0200, Richard Biener wrote:
> > > > > If [[fallthrough]]
> > > > > was approved for C++17 is there sth similar proposed for C?  Like a 
> > > > > keyword
> > > > > __Fallthrough?
> > > > 
> > > > I don't think there is.
> > > But maybe there should be ;-)  We've got a couple folks (Martin S and
> > > Carlos) that participate in that standards body and could propose it.
> > 
> > I think you're right.  I'd happily participate in proposing / implementing
> > __Fallthrough.
> 
> Given that the C++ attribute has been approved for C++ 17 I think
> it would make sense to consider its equivalent for C2X.  I can help
> you put a proposal together for the upcoming C meeting in October.

Sounds good.  I think by that time the warning will have been committed
and tested and so we should have enough time for that.  Thanks Martin.  

Marek

Re: [patch, fortran] PR66310 Problems with intrinsic repeat for large number of copies

[Jerry DeLisle]

On 07/12/2016 06:26 AM, Dominique d'Humières wrote:
>>> 2016-07-11  Jerry DeLisle  
>>>
>>> PR fortran/66310
>>> * simplify.c (gfc_simplify_repeat): Set max repeat to huge - 1 to allow
>>> one byte for null terminating the resulting string constant.
>>
>> OK, thanks
> Please hold on. I still see several problem with the patch applied. One is
> 
>program p
>   character :: z = 'z'
>   print *, repeat(z, huge(1)-2**9)
>end
> 
> a.out(67209,0x7fff77e0b000) malloc: *** 
> mach_vm_map(size=18446744071562067968) failed (error code=3)
> *** error: can't allocate region
> *** set a breakpoint in malloc_error_break to debug
> Operating system error: Cannot allocate memory
> Memory allocation failure in realloc
> 
> on x86_64-apple-darwin15.5 with/without the patch. print *, repeat(z, 
> huge(1)-2**9-1) "works".
> 
> I’ll try to report the other problems in the PR later today.
> 
> Dominique
> 
> 

I am holding. On my system I get:

Operating system error: Cannot allocate memory
Memory allocation failure in xrealloc

Which is what I expect.  The error is in trying to allocate the buffer in
write_blockm which it simply can not do.  If you think about it, the numbers are
hugely ridiculous.

Can you try ncopies = (huge(1)-1)/4 and see what you get?

Jerry

[PATCH, i386]: Remove unneeded truncations to DImode in x86_64{,_zext}_immediate_operand predicates.

[Uros Bizjak]

The immediate is checked that it fits SImode just below the unneeded
truncation to DImode.

2016-07-12  Uros Bizjak  

* config/i386/predicates.md (x86_64_immediate_operand)
: Remove unneeded truncation to DImode.
: Ditto.
(x86_64_zext_immediate_operand) : Ditto.

Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.

Committed to mainline SVN.

Uros.
diff --git a/gcc/config/i386/predicates.md b/gcc/config/i386/predicates.md
index 6854c37..219674e 100644
--- a/gcc/config/i386/predicates.md
+++ b/gcc/config/i386/predicates.md
@@ -156,7 +156,7 @@
 {
 case CONST_INT:
   {
-HOST_WIDE_INT val = trunc_int_for_mode (INTVAL (op), DImode);
+HOST_WIDE_INT val = INTVAL (op);
 return trunc_int_for_mode (val, SImode) == val;
   }
 case SYMBOL_REF:
@@ -199,14 +199,13 @@
{
  rtx op1 = XEXP (XEXP (op, 0), 0);
  rtx op2 = XEXP (XEXP (op, 0), 1);
- HOST_WIDE_INT offset;
 
  if (ix86_cmodel == CM_LARGE)
return false;
  if (!CONST_INT_P (op2))
return false;
 
- offset = trunc_int_for_mode (INTVAL (op2), DImode);
+ HOST_WIDE_INT offset = INTVAL (op2);
  if (trunc_int_for_mode (offset, SImode) != offset)
return false;
 
@@ -306,14 +305,13 @@
{
  rtx op1 = XEXP (XEXP (op, 0), 0);
  rtx op2 = XEXP (XEXP (op, 0), 1);
- HOST_WIDE_INT offset;
 
  if (ix86_cmodel == CM_LARGE)
return false;
  if (!CONST_INT_P (op2))
return false;
 
- offset = trunc_int_for_mode (INTVAL (op2), DImode);
+ HOST_WIDE_INT offset = INTVAL (op2);
  if (trunc_int_for_mode (offset, SImode) != offset)
return false;
 

Re: [PATCH], PowerPC support to enable -mlra and/or -mfloat128

[Michael Meissner]

On Tue, Jul 12, 2016 at 01:21:47PM +0200, Bernd Schmidt wrote:
> On 07/12/2016 12:29 PM, Richard Biener wrote:
> 
> >Instead of adding more configury can we please enable LRA on trunk by default
> >_now_?  Otherwise the amount of testing it recieves won't really increase.
> 
> I hope you mean for ppc only, otherwise you're breaking a lot of ports...

Yes I mean for PowerPC only.  I can change the switches name to:

--enable-powerpc-lra
--enable-powerpc-float128

if it would be clearer.  If you would prefer the switches to be undocumented,
and just private switches, that is fine also.

I view --enable-lra/--enble-powerpc-lra as a temporary switch to allow us to
straddle the issue until the performance blocker (PR 69847) is resolved, and
the PowerPC compiler switches to LRA in GCC 7.  Ideally, it would be nice to
eliminate the support for reload in the PowerPC backend (and hence this
switch).  But until we fully transition to lra, it would be useful to have a
way to switch what the default is without having to edit the sandbox.

Likewise, --enable-float128/--enable-powerpc-float128 is a transition switch.
It would be nice to have __float128 on by default without using the -mfloat128
switch, but we have a lot of work in the library arena before we can do this.

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

[gomp4] backport fixes for PR71704

[Cesar Philippidis]

This patch contains both Jakub's OpenMP and my OpenACC fixes for
PR71704. For reference, the discussion for the original patches can be
found here .

I'll apply this patch to gomp-4_0-branch shortly.

Cesar
2016-07-12  Cesar Philippidis  

	Backport from trunk:
	2016-07-08  Cesar Philippidis  

	gcc/fortran/
	* parse.c (matcha): Define.
	(decode_oacc_directive): Add spec_only local var and set it.  Use
	matcha to parse acc directives except for routine and declare.  Return
	ST_GET_FCN_CHARACTERISTICS if a non-declarative directive could be
	matched.

	gcc/testsuite/
	* gfortran.dg/goacc/pr71704.f90: New test.

	2016-06-01  Jakub Jelinek  

	gcc/fortran/
	* parse.c (case_decl): Move ST_OMP_* to ...
	(case_omp_decl): ... here, new macro.
	(verify_st_order): For case_omp_decl, complain about
	p->state >= ORDER_EXEC, but don't change p->state otherwise.

	gcc/testsuite
	* gfortran.dg/gomp/order-1.f90: New test.
	* gfortran.dg/gomp/order-2.f90: New test.


diff --git a/gcc/fortran/parse.c b/gcc/fortran/parse.c
index 7bce47f..ccc5d6c 100644
--- a/gcc/fortran/parse.c
+++ b/gcc/fortran/parse.c
@@ -585,21 +585,12 @@ decode_statement (void)
   return ST_NONE;
 }
 
-/* Like match, but set a flag simd_matched if keyword matched.  */
-#define matchs(keyword, subr, st)\
+/* Like match and if spec_only, goto do_spec_only without actually
+   matching.  */
+#define matcha(keyword, subr, st)\
 do {			\
-  if (match_word_omp_simd (keyword, subr, &old_locus,	\
-			   &simd_matched) == MATCH_YES)	\
-	return st;		\
-  else			\
-	undo_new_statement ();  	\
-} while (0);
-
-/* Like match, but don't match anything if not -fopenmp.  */
-#define matcho(keyword, subr, st)\
-do {			\
-  if (!flag_openmp)		\
-	;			\
+  if (spec_only && gfc_match (keyword) == MATCH_YES)	\
+	goto do_spec_only;	\
   else if (match_word (keyword, subr, &old_locus)		\
 	   == MATCH_YES)	\
 	return st;		\
@@ -612,6 +603,7 @@ decode_oacc_directive (void)
 {
   locus old_locus;
   char c;
+  bool spec_only = false;
 
   gfc_enforce_clean_symbol_state ();
 
@@ -626,6 +618,10 @@ decode_oacc_directive (void)
   return ST_NONE;
 }
 
+  if (gfc_current_state () == COMP_FUNCTION
+  && gfc_current_block ()->result->ts.kind == -1)
+spec_only = true;
+
   gfc_unset_implicit_pure (NULL);
 
   old_locus = gfc_current_locus;
@@ -639,49 +635,52 @@ decode_oacc_directive (void)
   switch (c)
 {
 case 'a':
-  match ("atomic", gfc_match_oacc_atomic, ST_OACC_ATOMIC);
+  matcha ("atomic", gfc_match_oacc_atomic, ST_OACC_ATOMIC);
   break;
 case 'c':
-  match ("cache", gfc_match_oacc_cache, ST_OACC_CACHE);
+  matcha ("cache", gfc_match_oacc_cache, ST_OACC_CACHE);
   break;
 case 'd':
-  match ("data", gfc_match_oacc_data, ST_OACC_DATA);
+  matcha ("data", gfc_match_oacc_data, ST_OACC_DATA);
   match ("declare", gfc_match_oacc_declare, ST_OACC_DECLARE);
   break;
 case 'e':
-  match ("end atomic", gfc_match_omp_eos, ST_OACC_END_ATOMIC);
-  match ("end data", gfc_match_omp_eos, ST_OACC_END_DATA);
-  match ("end host_data", gfc_match_omp_eos, ST_OACC_END_HOST_DATA);
-  match ("end kernels loop", gfc_match_omp_eos, ST_OACC_END_KERNELS_LOOP);
-  match ("end kernels", gfc_match_omp_eos, ST_OACC_END_KERNELS);
-  match ("end loop", gfc_match_omp_eos, ST_OACC_END_LOOP);
-  match ("end parallel loop", gfc_match_omp_eos, ST_OACC_END_PARALLEL_LOOP);
-  match ("end parallel", gfc_match_omp_eos, ST_OACC_END_PARALLEL);
-  match ("enter data", gfc_match_oacc_enter_data, ST_OACC_ENTER_DATA);
-  match ("exit data", gfc_match_oacc_exit_data, ST_OACC_EXIT_DATA);
+  matcha ("end atomic", gfc_match_omp_eos, ST_OACC_END_ATOMIC);
+  matcha ("end data", gfc_match_omp_eos, ST_OACC_END_DATA);
+  matcha ("end host_data", gfc_match_omp_eos, ST_OACC_END_HOST_DATA);
+  matcha ("end kernels loop", gfc_match_omp_eos, ST_OACC_END_KERNELS_LOOP);
+  matcha ("end kernels", gfc_match_omp_eos, ST_OACC_END_KERNELS);
+  matcha ("end loop", gfc_match_omp_eos, ST_OACC_END_LOOP);
+  matcha ("end parallel loop", gfc_match_omp_eos,
+	  ST_OACC_END_PARALLEL_LOOP);
+  matcha ("end parallel", gfc_match_omp_eos, ST_OACC_END_PARALLEL);
+  matcha ("enter data", gfc_match_oacc_enter_data, ST_OACC_ENTER_DATA);
+  matcha ("exit data", gfc_match_oacc_exit_data, ST_OACC_EXIT_DATA);
   break;
 case 'h':
-  match ("host_data", gfc_match_oacc_host_data, ST_OACC_HOST_DATA);
+  matcha ("host_data", gfc_match_oacc_host_data, ST_OACC_HOST_DATA);
   break;
 case 'p':
-  match ("parallel loop", gfc_match_oacc_parallel_loop, ST_OACC_PARALLEL_LOOP);
-  match ("parallel", gfc_match_oacc_parallel, ST_OACC_PARALLEL);
+  matcha ("parallel loop", gfc_match_oacc_parallel_loop,
+	  ST_OACC_PARALLEL_LOOP);
+   

Go patch committed: add escape analysis notes to export data

[Ian Lance Taylor]

This patch by Chris Manghane changes the Go frontend to add escape
analysis notes to the export data, and to read them in when importing
a package.  Escape analysis is still not enabled by default.
Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
to mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 238057)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-c8fdad389ce6f439a02fb654d231053b47ff4e02
+5ea5c078829ae83bccb598772fff7c1a04e23e65
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: gcc/go/gofrontend/export.cc
===
--- gcc/go/gofrontend/export.cc (revision 236804)
+++ gcc/go/gofrontend/export.cc (working copy)
@@ -436,6 +436,21 @@ Export::write_type(const Type* type)
 this->type_refs_[type] = index;
 }
 
+// Export escape note.
+
+void
+Export::write_escape(std::string* note)
+{
+  if (note != NULL && *note != "esc:0x0")
+{
+  this->write_c_string(" ");
+  char buf[50];
+  go_assert(note->find("esc:") != std::string::npos);
+  snprintf(buf, sizeof buf, "<%s>", note->c_str());
+  this->write_c_string(buf);
+}
+}
+
 // Add the builtin types to the export table.
 
 void
Index: gcc/go/gofrontend/export.h
===
--- gcc/go/gofrontend/export.h  (revision 236804)
+++ gcc/go/gofrontend/export.h  (working copy)
@@ -161,6 +161,11 @@ class Export : public String_dump
   void
   write_type(const Type*);
 
+  // Write the escape note to the export stream.  If NOTE is NULL, write
+  // nothing.
+  void
+  write_escape(std::string* note);
+
  private:
   Export(const Export&);
   Export& operator=(const Export&);
Index: gcc/go/gofrontend/gogo.cc
===
--- gcc/go/gofrontend/gogo.cc   (revision 236804)
+++ gcc/go/gofrontend/gogo.cc   (working copy)
@@ -4794,6 +4794,7 @@ Function::export_func_with_type(Export*
   exp->write_c_string("(");
   const Typed_identifier* receiver = fntype->receiver();
   exp->write_name(receiver->name());
+  exp->write_escape(receiver->note());
   exp->write_c_string(" ");
   exp->write_type(receiver->type());
   exp->write_c_string(") ");
@@ -4817,6 +4818,7 @@ Function::export_func_with_type(Export*
  else
exp->write_c_string(", ");
  exp->write_name(p->name());
+ exp->write_escape(p->note());
  exp->write_c_string(" ");
  if (!is_varargs || p + 1 != parameters->end())
exp->write_type(p->type());
@@ -4850,6 +4852,7 @@ Function::export_func_with_type(Export*
  else
exp->write_c_string(", ");
  exp->write_name(p->name());
+ exp->write_escape(p->note());
  exp->write_c_string(" ");
  exp->write_type(p->type());
}
@@ -4875,9 +4878,11 @@ Function::import_func(Import* imp, std::
 {
   imp->require_c_string("(");
   std::string name = imp->read_name();
+  std::string escape_note = imp->read_escape();
   imp->require_c_string(" ");
   Type* rtype = imp->read_type();
   *preceiver = new Typed_identifier(name, rtype, imp->location());
+  (*preceiver)->set_note(escape_note);
   imp->require_c_string(") ");
 }
 
@@ -4894,6 +4899,7 @@ Function::import_func(Import* imp, std::
   while (true)
{
  std::string name = imp->read_name();
+ std::string escape_note = imp->read_escape();
  imp->require_c_string(" ");
 
  if (imp->match_c_string("..."))
@@ -4905,8 +4911,9 @@ Function::import_func(Import* imp, std::
  Type* ptype = imp->read_type();
  if (*is_varargs)
ptype = Type::make_array_type(ptype, NULL);
- parameters->push_back(Typed_identifier(name, ptype,
-imp->location()));
+ Typed_identifier t = Typed_identifier(name, ptype, imp->location());
+ t.set_note(escape_note);
+ parameters->push_back(t);
  if (imp->peek_char() != ',')
break;
  go_assert(!*is_varargs);
@@ -4934,10 +4941,13 @@ Function::import_func(Import* imp, std::
  while (true)
{
  std::string name = imp->read_name();
+ std::string note = imp->read_escape();
  imp->require_c_string(" ");
  Type* rtype = imp->read_type();
- results->push_back(Typed_identifier(name, rtype,
- imp->location()));
+ Typed_identifier t = Typed_identifier(name, rtype,
+   imp->location());
+ t.set_note(note);
+ results->push_back(t);

Re: Implement -Wswitch-fallthrough

[Martin Sebor]

This patch is accompanied by more than 2000 lines of new tests to get the
warning covered though I'm sure folks will come up with other test cases
that I hadn't considered (hi Martin S. ;).

This warning is enabled by default for C/C++.  I was more inclined to put this
into -Wall, but our common.opt machinery doesn't seem to allow that (ugh!).
The clang version I have installed doesn't have this warning so I couldn't
compare my implementation with others.

I plan to plunge into the C++ [[fallthrough]] thingie after this core part is
dealt with.


I think this is a useful feature though like others I'm not
entirely sure that a built-in is the right interface.  I think
I would find a pragma or an attribute preferable.  At a minimum,
it would obviate some questions raised by my testing (i.e.,
whether the built-in be accepted when an attribute would
otherwise not be syntactically allowed).

I applied the core patch and ran a few experiments with it on
the assumption that __builtin_fallthrough() should behave roughly
correspondingly to [[fallthrough]].  I.e., be rejected where
[[fallthrough]] is rejected (but where attributes are otherwise
valid) and be accepted where the latter is accepted.  This may
not be intended but the text added to the manual is too vague
to tell.  I also compared the results to Clang's [[fallthrough]]
to make sure I wasn't missing something (or too much).

I ran into a few of what seems like subtle bugs or limitations
some of which I'm not sure are going to be solvable in the middle
end (at least not after the control flow graph has been built)
because by the time the middle end sees the code (certainly by
the time it gets to expansion) some of it has been eliminated.
An illustrative example of this class of problems might be this
function:

  void f (void)
  {
if (0) __builtin_fallthrough ();   // never diagnosed

int i = 0;
if (i) __builtin_fallthrough ();   // not diagnosed with -O
  }

With the built-in replaced by [[fallthrough]] Clang diagnoses
both of these.

This may be an okay limitation for __builtin_fallthrough since
it's GCC-specific, but it won't do for the C++ attribute or for
the C attribute if one is added with matching constraints.

The tests I tried are in the attached file.  Most of them are
edge cases but some I think point out more serious problems
(the checker getting confused/disabled by a prior switch
statement).

Hopefully this will be useful.

Martin
#ifdef __clang__
#  define FALLTHROUGH [[fallthrough]]
#  define __builtin_fallthrough() (void)0
#else
#  define FALLTHROUGH __builtin_fallthrough ()
#endif

void f0 (void)
{
  // This would be an invalid [[fallthrough]] and GCC rejects it
  // as expected with:
  //   error: invalid use of '__builtin_fallthrough ()'
  FALLTHROUGH;

  {
// This would be invalid [[fallthrough]] but is silently accepted 
// by GCC.
0 ? __builtin_fallthrough () : (void)0;
  }
  {
switch (0)
  // This is invalid with [[fallthrough]] because there's no
  // preceding statement within the switch statement, and no
  // subsequent one.  Clang rejects it but GCC silently accepts
  // it (it issues -Wswitch-unreachable).
  FALLTHROUGH;
  }

  {
// This is invalid but GCC gives:
//   warning: not preceding a label
// It seems that it should give an error like in the first case
// above.  I suspect it's confused by the preceding switch.
// Clang gives an error.
FALLTHROUGH;
  }
}

void f1 (void)
{
  {
switch (0)
  // This also seems invalid because there is no preceding
  // statement and no subsequent case-labeled statement but
  // GCC silently accepts it (as does Clang).  It seems that
  // GCC gets tricked by the A label.
  default: FALLTHROUGH; A: ;
  }

  {
// Unlike the same block at the end of f0 this is an error
// as it should be`.
FALLTHROUGH;
  }
}

void f2 (int i)
{
  {
switch (i)
{
  case 0:
// Both of the following are invalid with [[fallthrough]]
// because neither is followed by a case-labeled statement.
// GCC only only rejects the second of the two.  Clang
// accepts both.
FALLTHROUGH;
FALLTHROUGH;
}
  }
}

// The following function triggers an ICE.
void f3 (void)
{
  {
// Invalid but without the ICE silently accepted by GCC.
int i = 0;
if (i) __builtin_fallthrough ();
  }
  {
switch (0)
  __builtin_fallthrough ();
  }

  __builtin_fallthrough ();
}

Re: [PATCH], PowerPC support to enable -mlra and/or -mfloat128

[Segher Boessenkool]

On Tue, Jul 12, 2016 at 03:44:37PM -0400, Michael Meissner wrote:
> > I hope you mean for ppc only, otherwise you're breaking a lot of ports...
> 
> Yes I mean for PowerPC only.  I can change the switches name to:
> 
>   --enable-powerpc-lra
>   --enable-powerpc-float128
> 
> if it would be clearer.

That would be nice I think.  For the PowerPC port it is okay with that;
but I don't think I can approve the configure parts.


Segher

Re: [PATCH] Lift alignment restrictions from inlining register size memcpy

[Eric Botcazou]

> I have added a testcase that should exercise most relevant cases so
> we can look for fallout on "interesting" targets.

It passes on SPARC/Solaris 32-bit.

> Boostrap / regtest on strict-alignment platforms welcome.

Preliminary testing looks good, with a few expected regressions I guess:

+XPASS: gcc.dg/memmove-4.c scan-tree-dump-not optimized "memmove"
+FAIL: gcc.dg/strlenopt-8.c scan-tree-dump-times strlen "strlen (" 0
+FAIL: gcc.dg/strlenopt-8.c scan-tree-dump-times strlen "memcpy (" 4

> Unless I hear some positives I'll let the patch sit here as I
> don't really care too much about those pesky targets (and
> targets can choose to opt-in by providing movmisalign optabs
> anyway so they don't go the store/extract_bit_field path).

I'll conduct more thorough testing over the next few days.

-- 
Eric Botcazou

Re: [PATCH] - improve sprintf buffer overflow detection (middle-end/49905)

[Martin Sebor]

On 07/12/2016 06:32 AM, Bernd Schmidt wrote:

On 07/01/2016 08:15 PM, Martin Sebor wrote:

The attached patch enhances compile-time checking for buffer overflow
and output truncation in non-trivial calls to the sprintf family of
functions under a new option -Wformat-length=[12].  This initial
patch handles printf directives with string, integer, and simple
floating arguments but eventually I'd like to extend it all other
functions and directives for which it makes sense.


On the whole I think this looks good.


Thanks the detailed review!


Beyond these I have no objections to the patch but ideally a C frontend
maintainer would given an explicit ack as well.


In response to prior comments from Jakub and Richard I have actually
moved the patch to the middle end, into a pass of its own where it
works with LTO, and where it can also be used to optimize branches
based on the functions' return value (when they are known to be
exact).

I will make the changes you suggested (those that apply) and post
an updated patch for review soon that should be closer to final
than the initial prototype.

Martin

Re: [PATCH] Allow fwprop to undo vectorization harm (PR68961)

[Uros Bizjak]

On Sun, Jul 10, 2016 at 10:12 AM, Uros Bizjak  wrote:
> On Wed, Jul 6, 2016 at 3:18 PM, Richard Biener  wrote:
>
>>> > 2016-07-04  Richard Biener  
>>> >
>>> > PR rtl-optimization/68961
>>> > * fwprop.c (propagate_rtx): Allow SUBREGs of VEC_CONCAT and CONCAT
>>> > to simplify to a non-constant.
>>> >
>>> > * gcc.target/i386/pr68961.c: New testcase.
>>>
>>> Thanks, LGTM.
>>
>> Bootstrapped and tested on x86_64-unknown-linux-gnu, it causes
>>
>> FAIL: gcc.target/i386/sse2-load-multi.c scan-assembler-times movup 2
>>
>> as the peephole created for that testcase no longer applies as fwprop
>> does
>>
>> In insn 10, replacing
>>  (vec_concat:V2DF (vec_select:DF (reg:V2DF 91)
>> (parallel [
>> (const_int 0 [0])
>> ]))
>> (mem:DF (reg/f:DI 95) [0  S8 A128]))
>>  with (vec_concat:V2DF (reg:DF 93 [ MEM[(const double *)&a + 8B] ])
>> (mem:DF (reg/f:DI 95) [0  S8 A128]))
>> Changed insn 10
>>
>> resulting in
>>
>> movsd   a+8(%rip), %xmm0
>> movhpd  a+16(%rip), %xmm0
>>
>> again rather than movupd.
>>
>> Uros, there is probably a missing peephole for the new form - can you
>> fix this as a followup or should I hold on this patch for a bit longer?
>
> No, please proceed with the patch, I'll fix this fallout with a
> followup patch in a couple of days.

Fixed with attached patch.

2016-07-13  Uros Bizjak  

PR rtl-optimization/68961
* config/i386/sse.md (movsd/movhpd to movupd peephole2s): Add new
peephole variant.  Use sse_reg_operand predicates.

Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.

Committed to mainline SVN.

Uros.
Index: config/i386/sse.md
===
--- config/i386/sse.md  (revision 238258)
+++ config/i386/sse.md  (working copy)
@@ -1169,10 +1169,10 @@
 
 ;; Merge movsd/movhpd to movupd for TARGET_SSE_UNALIGNED_LOAD_OPTIMAL targets.
 (define_peephole2
-  [(set (match_operand:V2DF 0 "register_operand")
+  [(set (match_operand:V2DF 0 "sse_reg_operand")
(vec_concat:V2DF (match_operand:DF 1 "memory_operand")
 (match_operand:DF 4 "const0_operand")))
-   (set (match_operand:V2DF 2 "register_operand")
+   (set (match_operand:V2DF 2 "sse_reg_operand")
(vec_concat:V2DF (vec_select:DF (match_dup 2)
(parallel [(const_int 0)]))
 (match_operand:DF 3 "memory_operand")))]
@@ -1181,13 +1181,25 @@
   [(set (match_dup 2) (match_dup 4))]
   "operands[4] = adjust_address (operands[1], V2DFmode, 0);")
 
+(define_peephole2
+  [(set (match_operand:DF 0 "sse_reg_operand")
+   (match_operand:DF 1 "memory_operand"))
+   (set (match_operand:V2DF 2 "sse_reg_operand")
+   (vec_concat:V2DF (match_operand:DF 4 "sse_reg_operand")
+(match_operand:DF 3 "memory_operand")))]
+  "TARGET_SSE2 && TARGET_SSE_UNALIGNED_LOAD_OPTIMAL
+   && REGNO (operands[4]) == REGNO (operands[2])
+   && ix86_operands_ok_for_move_multiple (operands, true, DFmode)"
+  [(set (match_dup 2) (match_dup 4))]
+  "operands[4] = adjust_address (operands[1], V2DFmode, 0);")
+
 ;; Merge movlpd/movhpd to movupd for TARGET_SSE_UNALIGNED_STORE_OPTIMAL 
targets.
 (define_peephole2
   [(set (match_operand:DF 0 "memory_operand")
-   (vec_select:DF (match_operand:V2DF 1 "register_operand")
+   (vec_select:DF (match_operand:V2DF 1 "sse_reg_operand")
   (parallel [(const_int 0)])))
(set (match_operand:DF 2 "memory_operand")
-   (vec_select:DF (match_operand:V2DF 3 "register_operand")
+   (vec_select:DF (match_operand:V2DF 3 "sse_reg_operand")
   (parallel [(const_int 1)])))]
   "TARGET_SSE2 && TARGET_SSE_UNALIGNED_STORE_OPTIMAL
&& ix86_operands_ok_for_move_multiple (operands, false, DFmode)"

Re: [v3 PATCH] Implement P0307R2, Making Optional Greater Equal Again.

[Jonathan Wakely]

On 11/07/16 23:41 +0300, Ville Voutilainen wrote:

@@ -785,41 +785,60 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
}
};

+  template
+using __optional_relop_t =
+enable_if_t::value, bool>;


Should this be is_convertible<_Tp, bool> instead?


  template
-constexpr bool
+constexpr auto
operator!=(const optional<_Tp>& __lhs, _Tp const& __rhs)


Dunno why this has _Tp const& rather than const _Tp&, could you fix it
while you're in the file anyway? It's a bit confusing to have one
place using a different style.