RE: Skip gcc.dg/tree-ssa/isolate-*.c for AVR Target

[K_s, Vishnu]

Added comment to avr.c on why disabling -fdelete-null-pointer-checks. Patch
Attached in the mail. 

Previously approved patch can be found here - 
http://gcc.gnu.org/ml/gcc-patches/2014-03/msg01697.html.

I don't have commit access to the source, please commit both the patches
If they are OK.

Regards
Vishnu KS

2014-03-31  Vishnu K S  

  * gcc/config/avr/avr.c: Add comment on why disabling 
  -fdelete-null-pointer-checks


diff --git a/gcc/config/avr/avr.c b/gcc/config/avr/avr.c
index 8ca7de0..c400bff 100644
--- a/gcc/config/avr/avr.c
+++ b/gcc/config/avr/avr.c
@@ -290,6 +290,12 @@ avr_to_int_mode (rtx x)
 static void
 avr_option_override (void)
 {
+  /* Disable -fdelete-null-pointer-checks option for AVR target.
+ For this option compiler assumes that dereferencing of a null pointer
+ would halt the program. For AVR this assumption is not true and
+ programs can safely dereference null pointers. Changes made by this
+ option may not work properly for AVR. So disable this option. */
+
   flag_delete_null_pointer_checks = 0;

   /* caller-save.c looks for call-clobbered hard registers that are assigned
> -Original Message-
> From: Richard Biener [mailto:richard.guent...@gmail.com]
> Sent: Saturday, April 05, 2014 3:16 PM
> To: Jeff Law
> Cc: K_s, Vishnu; gcc-patches@gcc.gnu.org
> Subject: Re: Skip gcc.dg/tree-ssa/isolate-*.c for AVR Target
> 
> On Fri, Apr 4, 2014 at 5:37 PM, Jeff Law  wrote:
> > On 03/28/14 04:16, K_s, Vishnu wrote:
> >>
> >> Hi all,
> >>
> >> The tests added in gcc.dg/tree-ssa/isolate-*.c is failing for AVR
> target,
> >> Because the isolate erroneous path pass needs -fdelete-null-pointer-
> checks
> >> option to be enabled. For AVR target that option is disabled, this
> cause
> >> the tests to fail. Following Patch skip the isolate-* tests if
> >> "keeps_null_pointer_checks" is true.
> >>
> >> 2014-03-28  Vishnu K S 
> >>
> >> * gcc/testsuite/gcc.dg/tree-ssa/isolate-1.c: Skip test for
> AVR
> >> * gcc/testsuite/gcc.dg/tree-ssa/isolate-2.c: Ditto
> >> * gcc/testsuite/gcc.dg/tree-ssa/isolate-3.c: Ditto
> >> * gcc/testsuite/gcc.dg/tree-ssa/isolate-4.c: Ditto
> >> * gcc/testsuite/gcc.dg/tree-ssa/isolate-5.c: Ditto
> >
> > This is fine for the trunk.  Please go ahead and install.
> >
> > However, we generally discourage ports from turning off passes like
> this and
> > particularly so without a comment as to why a pass is turned off.
> >
> > That code was added to the AVR port here:
> >
> > http://gcc.gnu.org/ml/gcc-patches/2007-03/msg01968.html
> >
> >
> > If you could add a comment to the AVR port indicating that
> > delete-null-pointer-checks is disabled because the hardware does not
> fault
> > on a NULL dereference, it would be greatly appreciated.  Consider
> that
> > comment addition pre-approved, just post it to the list for archival
> > purposes.
> 
> Note that it is recommended (and documented) practice to treat
> -fdelete-null-pointer-checks that way for targets that support objects
> starting at 0.
> 
> Richard.
> 
> > Thanks,
> > Jeff
> >
> >

[ping] [ARM] Fix PR target/60504

[Eric Botcazou]

This fixes the broken EH tables generated for the ARM:
  http://gcc.gnu.org/ml/gcc-patches/2014-03/msg01333.html

Thanks in advance.

-- 
Eric Botcazou

Re: [C++] Weffc++/Wnon-virtual-dtor confusion

[Nathan Sidwell]

On 04/06/14 10:50, Markus Trippelsdorf wrote:

On 2014.04.06 at 09:13 +0100, Nathan Sidwell wrote:

On 04/04/14 18:33, Nathan Sidwell wrote:


I'm testing a patch that makes the test in the loop:

if (TREE_PUBLIC (base_binfo)


Hm, binfo's aren't noted that way, it's encoded in BINFO_ACCESS and
SET_BINFO_ACCESS in search.c.  We'd need to move those back to cp.h or expose an
interface in search.c.  This is looking like a rat hole ...


That was my rustiness, it is not as complicated.

Here is a patch to implement the behaviour we discussed:
*) only consider public bases
*) only consider polymorphic bases,  unless Weffc++ is also specified

I have tested this in the usual manner, and the new testcases pass with the 
patch and fail without it.


Markus, if you could give this a try and see whether it fixes the problem you 
reported, that'd be great.


Jason, I shall leave it to your discretion as to whether we should continue with 
this patch, or revert the original one (for 4.9).


nathan
2014-04-07  Nathan Sidwell  

* doc/invoke (Wnon-virtual-dtor): Update to match implementation.
(Weffc++): Likewise.

cp/
* class.c (check_bases_and_members): Warn about non-virtual dtors
in public bases only.  Check warn_eccp before complaining about
non-polymorphic bases.

testsuite/
* g++.dg/warn/Wnvdtor-2.C: Add more cases.
* g++.dg/warn/Wnvdtor-3.C: Likewise.
* g++.dg/warn/Wnvdtor-4.C: Likewise.

Index: cp/class.c
===
--- cp/class.c  (revision 209122)
+++ cp/class.c  (working copy)
@@ -5570,21 +5570,24 @@ check_bases_and_members (tree t)
   TYPE_HAS_COMPLEX_MOVE_ASSIGN (t) |= TYPE_CONTAINS_VPTR_P (t);
   TYPE_HAS_COMPLEX_DFLT (t) |= TYPE_CONTAINS_VPTR_P (t);
 
-  /* Warn if a base of a polymorphic type has an accessible
+  /* Warn if a public base of a polymorphic type has an accessible
  non-virtual destructor.  It is only now that we know the class is
  polymorphic.  Although a polymorphic base will have a already
  been diagnosed during its definition, we warn on use too.  */
   if (TYPE_POLYMORPHIC_P (t) && warn_nonvdtor)
 {
-  tree binfo, base_binfo;
+  tree binfo = TYPE_BINFO (t);
+  vec *accesses = BINFO_BASE_ACCESSES (binfo);
+  tree base_binfo;
   unsigned i;
   
-  for (binfo = TYPE_BINFO (t), i = 0;
-  BINFO_BASE_ITERATE (binfo, i, base_binfo); i++)
+  for (i = 0; BINFO_BASE_ITERATE (binfo, i, base_binfo); i++)
{
  tree basetype = TREE_TYPE (base_binfo);
 
- if (accessible_nvdtor_p (basetype))
+ if ((*accesses)[i] == access_public_node
+ && (TYPE_POLYMORPHIC_P (basetype) || warn_ecpp)
+ && accessible_nvdtor_p (basetype))
warning (OPT_Wnon_virtual_dtor,
 "base class %q#T has accessible non-virtual destructor",
 basetype);
Index: doc/invoke.texi
===
--- doc/invoke.texi (revision 209122)
+++ doc/invoke.texi (working copy)
@@ -2670,10 +2670,10 @@ the compiler to never throw an exception
 @opindex Wnon-virtual-dtor
 @opindex Wno-non-virtual-dtor
 Warn when a class has virtual functions and an accessible non-virtual
-destructor itself or in a base class, or has in which case it is
-possible but unsafe to delete an instance of a derived class through a
-pointer to the base class.  This warning is automatically enabled if
-@option{-Weffc++} is specified.
+destructor itself or in an accessible polymorphic base class, in which
+case it is possible but unsafe to delete an instance of a derived
+class through a pointer to the class itself or base class.  This
+warning is automatically enabled if @option{-Weffc++} is specified.
 
 @item -Wreorder @r{(C++ and Objective-C++ only)}
 @opindex Wreorder
@@ -2743,7 +2743,9 @@ Never overload @code{&&}, @code{||}, or
 @end itemize
 
 This option also enables @option{-Wnon-virtual-dtor}, which is also
-one of the effective C++ recommendations.
+one of the effective C++ recommendations.  However, the check is
+extended to warn about the lack of virtual destructor in accessible
+non-polymorphic bases classes too.
 
 When selecting this option, be aware that the standard library
 headers do not obey all of these guidelines; use @samp{grep -v}
Index: testsuite/g++.dg/warn/Wnvdtor-2.C
===
--- testsuite/g++.dg/warn/Wnvdtor-2.C   (revision 209122)
+++ testsuite/g++.dg/warn/Wnvdtor-2.C   (working copy)
@@ -54,4 +54,23 @@ public:
 };
 
 struct H {};
-struct I : H {};
+
+struct I1 : H
+{};
+struct I2 : private H
+{};
+
+struct J1 : H
+{ virtual ~J1 ();};
+struct J2 : private H
+{ virtual ~J2 ();};
+
+struct K // { dg-warning "accessible non-virtual destructor" }
+{
+  virtual void k ();
+};
+
+struct L1 : K // { dg-warning "accessible non-virtual

Re: [PATCH] Fix PR59626, _FORTIFY_SOURCE wrappers and LTO

[Richard Biener]

On Fri, 4 Apr 2014, Jan Hubicka wrote:

> > On Fri, 4 Apr 2014, Jan Hubicka wrote:
> > 
> > > Hi,
> > > here is an updated version of my earlier ipa.c change.  It turns out that 
> > > the
> > > problem was that I did not drop always_inline.
> > > In this version I just drop always_inline attribute on all functions 
> > > whose body
> > > is removed.  The patch will affect non-LTO compilation, too, but IMO only 
> > > by
> > > making us to not inline&diagnose the cases where indirect call to always 
> > > inline
> > > is turned direct in between early opts and inline. Does that seem 
> > > acceptable?
> > > (I personally would preffer this over inventing yet another way to 
> > > special case
> > > always_inline for LTO only; we never made any strong promises on 
> > > always_inline
> > > and indirect calls)
> > 
> > I think it's fine if indirect calls to always-inline fns go to an
> > offline copy (or cause a link-time error if there is no offline copy).
> > I've always thought that taking the address of an always_inline fn
> > should get you the address of an "external" symbol (an inline "clone"
> > isn't addressable).
> > 
> > So the patch is fine with me.
> 
> OK, I comitted it.  I forgot to mention that with fortified bootstrap I got
> quite few extra warning on return value of some functions being ignored
> (such as fscanf). Are those expected to appear only with fortification and
> are we expected to cowardly ignore them?

Yes, they are only expected to appear with fortification and yes, we
ignore them ;)

Richard.

> Honza
> > 
> > Thanks,
> > Richard.
> > 
> > > Honza
> > > 
> > >   * lto-cgraph.c (input_overwrite_node): Check that partitioning
> > >   flags are set only during streaming.
> > >   * ipa.c (process_references, walk_polymorphic_call_targets,
> > >   symtab_remove_unreachable_nodes): Drop bodies of always inline
> > >   after early inlining.
> > >   (symtab_remove_unreachable_nodes): Remove always_inline attribute.
> > > 
> > >   * gcc.dg/lto/pr59626_0.c: New testcase.
> > >   * gcc.dg/lto/pr59626_1.c: New testcase.
> > > Index: lto-cgraph.c
> > > ===
> > > --- lto-cgraph.c  (revision 209062)
> > > +++ lto-cgraph.c  (working copy)
> > > @@ -1001,6 +1001,9 @@ input_overwrite_node (struct lto_file_de
> > >node->thunk.thunk_p = bp_unpack_value (bp, 1);
> > >node->resolution = bp_unpack_enum (bp, ld_plugin_symbol_resolution,
> > >LDPR_NUM_KNOWN);
> > > +  gcc_assert (flag_ltrans
> > > +   || (!node->in_other_partition
> > > +   && !node->used_from_other_partition));
> > >  }
> > >  
> > >  /* Return string alias is alias of.  */
> > > @@ -1169,6 +1172,9 @@ input_varpool_node (struct lto_file_decl
> > >node->same_comdat_group = (symtab_node *) (intptr_t) ref;
> > >node->resolution = streamer_read_enum (ib, ld_plugin_symbol_resolution,
> > >   LDPR_NUM_KNOWN);
> > > +  gcc_assert (flag_ltrans
> > > +   || (!node->in_other_partition
> > > +   && !node->used_from_other_partition));
> > >  
> > >return node;
> > >  }
> > > Index: ipa.c
> > > ===
> > > --- ipa.c (revision 209062)
> > > +++ ipa.c (working copy)
> > > @@ -139,7 +139,10 @@ process_references (struct ipa_ref_list
> > >  
> > >if (node->definition && !node->in_other_partition
> > > && ((!DECL_EXTERNAL (node->decl) || node->alias)
> > > -   || (before_inlining_p
> > > +   || (((before_inlining_p
> > > + && (cgraph_state < CGRAPH_STATE_IPA_SSA
> > > + || !lookup_attribute ("always_inline",
> > > +   DECL_ATTRIBUTES (node->decl)
> > > /* We use variable constructors during late complation for
> > >constant folding.  Keep references alive so partitioning
> > >knows about potential references.  */
> > > @@ -191,7 +194,10 @@ walk_polymorphic_call_targets (pointer_s
> > > /* Prior inlining, keep alive bodies of possible targets for
> > >devirtualization.  */
> > >  if (n->definition
> > > -&& before_inlining_p)
> > > +&& (before_inlining_p
> > > +&& (cgraph_state < CGRAPH_STATE_IPA_SSA
> > > +|| !lookup_attribute ("always_inline",
> > > +  DECL_ATTRIBUTES (n->decl)
> > >pointer_set_insert (reachable, n);
> > >  
> > > /* Even after inlining we want to keep the possible targets in the
> > > @@ -491,6 +497,12 @@ symtab_remove_unreachable_nodes (bool be
> > > node->alias = false;
> > > node->thunk.thunk_p = false;
> > > node->weakref = false;
> > > +   /* After early inlining we drop always_inline attributes on
> > > +  bodies of functions that are still referenced (have their
> > > +  address taken).  */
> > > +   D

Re: [PATCH] Fix my name in contrib.texi

[Richard Biener]

On Mon, 7 Apr 2014, Gerald Pfeifer wrote:

> On Tue, 18 Mar 2014, Richard Biener wrote:
> > Committed as obvious.
> 
> htdocs/svn.html has four occurrences, and a few other pages also
> have some.  Though I don't think it's appropriate to adjust the 
> like of news.html, should I update svn.html?

Sure, go ahead.

Thanks,
Richard.

Re: [C++] Weffc++/Wnon-virtual-dtor confusion

[Markus Trippelsdorf]

On 2014.04.07 at 08:39 +0100, Nathan Sidwell wrote:
> On 04/06/14 10:50, Markus Trippelsdorf wrote:
> > On 2014.04.06 at 09:13 +0100, Nathan Sidwell wrote:
> >> On 04/04/14 18:33, Nathan Sidwell wrote:
> >>
> >>> I'm testing a patch that makes the test in the loop:
> >>>
> >>> if (TREE_PUBLIC (base_binfo)
> >>
> >> Hm, binfo's aren't noted that way, it's encoded in BINFO_ACCESS and
> >> SET_BINFO_ACCESS in search.c.  We'd need to move those back to cp.h or 
> >> expose an
> >> interface in search.c.  This is looking like a rat hole ...
> 
> That was my rustiness, it is not as complicated.
> 
> Here is a patch to implement the behaviour we discussed:
> *) only consider public bases
> *) only consider polymorphic bases,  unless Weffc++ is also specified
> 
> I have tested this in the usual manner, and the new testcases pass with the 
> patch and fail without it.
> 
> Markus, if you could give this a try and see whether it fixes the problem you 
> reported, that'd be great.

Well, all I've tested is if the warnings during an LLVM build vanish.
And this is indeed the case with your new patch, too.

-- 
Markus

Re: Warn when returning the address of a temporary (middle-end)

[Richard Biener]

On Sat, Apr 5, 2014 at 3:52 PM, Marc Glisse  wrote:
> Hello,
>
> we have front-end warnings about returning the address of a local variable.
> However, quite often in C++, people don't directly return the address of a
> temporary, it goes through a few functions which hide that fact. After some
> inlining, the fact that we are returning the address of a local variable can
> become obvious to the compiler, to the point where I have used, for
> debugging purposes, grep 'return &' on the optimized dump produced with -O3
> -fkeep-inline-functions (I then had to sort through the global/local
> variables).
>
> fold_stmt looks like a good place for this, but it could go almost anywhere.
> It has to happen after enough inlining / copy propagation to make it useful
> though.
>
> One hard part is avoiding duplicate warnings. Replacing the address with 0
> is a convenient way to do that, so I did it both for my new warning and for
> the existing C/C++ ones. The patch breaks
> gfortran.dg/warn_target_lifetime_2.f90 because it ends up warning twice. I
> didn't touch that front-end because I don't know fortran, and the warning
> message "Pointer at .1. in pointer assignment might outlive the pointer
> target" doesn't seem very confident that the thing really is broken enough
> to be replaced by 0. I only tested (bootstrap+regression) the default
> languages, so ada/go may have a similar issue, to be handled if the approach
> seems ok. (I personally wouldn't care about duplicate warnings, but I know
> some people can't help complaining about it)
>
> This doesn't actually fix PR 60517, for that I was thinking of checking for
> memory reads if the first stop of walk_aliased_vdefs is a clobber (could
> also check __builtin_free), though I don't know in which pass to do that
> yet.

Note that this will break "working" programs where inlining causes
those references to no longer be "returned" (though maybe they
already break with the clobbers we insert).  I remember that POOMA/PETE
was full of this ... (and made it reliably work by flattening all call trees
with such calls).

Richard.

> 2014-04-05  Marc Glisse  
>
> PR c++/60517
> gcc/c/
> * c-typeck.c (c_finish_return): Return 0 instead of the address of
> a local variable.
> gcc/cp/
> * typeck.c (check_return_expr): Likewise.
> (maybe_warn_about_returning_address_of_local): Tell the caller if
> we warned.
> gcc/
> * gimple-fold.c (fold_stmt_1): Warn if returning the address of a
> local variable.
> gcc/testsuite/
> * c-c++-common/addrtmp.c: New testcase.
> * c-c++-common/uninit-G.c: Adjust.
>
> --
> Marc Glisse
> Index: gcc/c/c-typeck.c
> ===
> --- gcc/c/c-typeck.c(revision 209157)
> +++ gcc/c/c-typeck.c(working copy)
> @@ -9254,23 +9254,25 @@ c_finish_return (location_t loc, tree re
>   inner = TREE_OPERAND (inner, 0);
>
>   while (REFERENCE_CLASS_P (inner)
>  && TREE_CODE (inner) != INDIRECT_REF)
> inner = TREE_OPERAND (inner, 0);
>
>   if (DECL_P (inner)
>   && !DECL_EXTERNAL (inner)
>   && !TREE_STATIC (inner)
>   && DECL_CONTEXT (inner) == current_function_decl)
> -   warning_at (loc,
> -   OPT_Wreturn_local_addr, "function returns
> address "
> -   "of local variable");
> +   {
> + warning_at (loc, OPT_Wreturn_local_addr,
> + "function returns address of local variable");
> + t = build_zero_cst (TREE_TYPE (res));
> +   }
>   break;
>
> default:
>   break;
> }
>
>   break;
> }
>
>retval = build2 (MODIFY_EXPR, TREE_TYPE (res), res, t);
> Index: gcc/cp/typeck.c
> ===
> --- gcc/cp/typeck.c (revision 209157)
> +++ gcc/cp/typeck.c (working copy)
> @@ -49,21 +49,21 @@ static tree convert_for_assignment (tree
>  static tree cp_pointer_int_sum (enum tree_code, tree, tree,
> tsubst_flags_t);
>  static tree rationalize_conditional_expr (enum tree_code, tree,
>   tsubst_flags_t);
>  static int comp_ptr_ttypes_real (tree, tree, int);
>  static bool comp_except_types (tree, tree, bool);
>  static bool comp_array_types (const_tree, const_tree, bool);
>  static tree pointer_diff (tree, tree, tree, tsubst_flags_t);
>  static tree get_delta_difference (tree, tree, bool, bool, tsubst_flags_t);
>  static void casts_away_constness_r (tree *, tree *, tsubst_flags_t);
>  static bool casts_away_constness (tree, tree, tsubst_flags_t);
> -static void maybe_warn_about_returning_address_of_local (tree);
> +static bool maybe_warn_about_returning_address_of_local (tree);
>  static tree lookup_destructor (tre

Re: RFA: Testsuite PATCH to add support for dlopen tests

[Richard Biener]

On Sun, Apr 6, 2014 at 6:21 PM, Mike Stump  wrote:
> On Apr 4, 2014, at 9:53 AM, Jason Merrill  wrote:
>> richi asked for a testcase for 60731, and since we didn't already have 
>> support for tests using dlopen, I had to add it.  Does this approach make 
>> sense?
>
> Seems reasonable.  I wonder if it works on solaris and darwin...  I think 
> darwin pre 10.5 might not like dlclose, but, those are old enough that I'm 
> not going to worry about it.

Works for me as well.

Thanks,
Richard.

Re: Warn when returning the address of a temporary (middle-end)

[Marc Glisse]

On Mon, 7 Apr 2014, Richard Biener wrote:


One hard part is avoiding duplicate warnings. Replacing the address with 0
is a convenient way to do that, so I did it both for my new warning and for
the existing C/C++ ones. The patch breaks
gfortran.dg/warn_target_lifetime_2.f90 because it ends up warning twice. I
didn't touch that front-end because I don't know fortran, and the warning
message "Pointer at .1. in pointer assignment might outlive the pointer
target" doesn't seem very confident that the thing really is broken enough
to be replaced by 0. I only tested (bootstrap+regression) the default
languages, so ada/go may have a similar issue, to be handled if the approach
seems ok. (I personally wouldn't care about duplicate warnings, but I know
some people can't help complaining about it)

This doesn't actually fix PR 60517, for that I was thinking of checking for
memory reads if the first stop of walk_aliased_vdefs is a clobber (could
also check __builtin_free), though I don't know in which pass to do that
yet.


Note that this will break "working" programs where inlining causes
those references to no longer be "returned" (though maybe they
already break with the clobbers we insert).  I remember that POOMA/PETE
was full of this ... (and made it reliably work by flattening all call trees
with such calls).


I mostly see programs that are written and tested using "debug mode" and 
that appear to be working, but break as soon as they are compiled in 
"release mode" because of dangling references. Breaking them more seems 
like a helpful thing to me ;-) (especially with a warning).


But I would be happy not replacing the pointer and only emitting a 
warning, if people don't mind duplicate warnings or if we can find another 
way to avoid them.



+   if (warning_at (gimple_location (stmt), OPT_Wreturn_local_addr,
+   "function returns address of local variable"))


That's a front-end option, I should either move it to gcc/common.opt or 
use a different option.


--
Marc Glisse

Re: Warn when returning the address of a temporary (middle-end)

[Eric Botcazou]

> One hard part is avoiding duplicate warnings. Replacing the address with 0
> is a convenient way to do that, so I did it both for my new warning and
> for the existing C/C++ ones. The patch breaks
> gfortran.dg/warn_target_lifetime_2.f90 because it ends up warning twice. I
> didn't touch that front-end because I don't know fortran, and the warning
> message "Pointer at .1. in pointer assignment might outlive the pointer
> target" doesn't seem very confident that the thing really is broken enough
> to be replaced by 0. I only tested (bootstrap+regression) the default
> languages, so ada/go may have a similar issue, to be handled if the
> approach seems ok.

Ada is designed to make such a thing impossible (although you can work around 
the design with kludges like 'Unrestricted_Access) so the compiler will stop.

-- 
Eric Botcazou

[libitm] Check if GCC uses assembler cfi support

[Rainer Orth]

The Solaris 11.2 x86 assembler is going to gain support for the cfi
directives.  To do things right this time, it will create read-only
.eh_frame sections, unlike what gas and gcc currently do.  The goal is
to let /bin/ld handle the required merging of read-only and read-write
.eh_frame sections, but that's not there yet and won't make the S11.2
release.  For the time being, gcc/configure.ac (gcc_cv_as_cfi_directive)
takes care of that and disables the support, but there's one issue left:
libitm does the cfi directive detection on its own, finds the assembler
support and uses it in config/x86/sjlj.S, which breaks linking the
32-bit libitm.so:

ld: fatal: file .libs/aatree.o; section [15].eh_frame and file .libs/sjlj.o; 
section [3].eh_frame have incompatibile attributes and cannot be merged into a 
single output section
collect2: error: ld returned 1 exit status
make[4]: *** [libitm.la] Error 1

To avoid this, libitm and gcc need to agree whether or not to use
assembler cfi support.

The following patch avoid this by checking __GCC_HAVE_DWARF2_CFI_ASM.

Alternatively, one could do away with
config/asmcfi.m4 (GCC_AS_CFI_PSEUDO_OP), and only check for
__GCC_HAVE_DWARF2_CFI_ASM under the assumption that libitm will only be
used in-tree with a matching gcc.

I've avoided changing config/asmcfi.m4 since that's also used by libffi,
which is external to the gcc tree and can be used with non-gcc
compilers.

The patch allowed i386-pc-solaris2.11 bootstraps to finish without
regressions on trunk and 4.8 branch, still need to test 4.7 branch.

Ok for trunk, 4.8 and 4.7 branches once testing completes?

Rainer


2014-04-07  Rainer Orth  

* config/generic/asmcfi.h: Also check for
__GCC_HAVE_DWARF2_CFI_ASM.

# HG changeset patch
# Parent e003a40e0bb68a5019d8492cec81e453514988b0
Check if GCC uses assembler cfi support

diff --git a/libitm/config/generic/asmcfi.h b/libitm/config/generic/asmcfi.h
--- a/libitm/config/generic/asmcfi.h
+++ b/libitm/config/generic/asmcfi.h
@@ -24,7 +24,7 @@
 
 #include "config.h"
 
-#ifdef HAVE_AS_CFI_PSEUDO_OP
+#if defined(HAVE_AS_CFI_PSEUDO_OP) && defined(__GCC_HAVE_DWARF2_CFI_ASM)
 
 #define cfi_startproc			.cfi_startproc
 #define cfi_endproc			.cfi_endproc
@@ -50,4 +50,4 @@
 #define cfi_restore(r)
 #define cfi_undefined(r)
 
-#endif /* HAVE_AS_CFI_PSEUDO_OP */
+#endif /* HAVE_AS_CFI_PSEUDO_OP && __GCC_HAVE_DWARF2_CFI_ASM */

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

Re: conditional notes after 'pedwarn'

[Jason Merrill]

OK.

Jason

[PATCH][committed] Fix couple of typos and outdated function reference in comment

[Kyrill Tkachov]

Hi all,

I've committed the attached patch to the introductory comment in haifa-sched.c 
as obvious with r209184.
The function compute_block_backward_dependences was renamed to 
compute_block_dependences in 2007but the comment was not updated.


Kyrill

2014-04-07  Kyrylo Tkachov  

* haifa-sched.c: Fix outdated function reference and minor
grammar errors in introductory comment.diff --git a/gcc/haifa-sched.c b/gcc/haifa-sched.c
index e0d4674..2d66e5c 100644
--- a/gcc/haifa-sched.c
+++ b/gcc/haifa-sched.c
@@ -20,7 +20,7 @@ along with GCC; see the file COPYING3.  If not see
 .  */
 
 /* Instruction scheduling pass.  This file, along with sched-deps.c,
-   contains the generic parts.  The actual entry point is found for
+   contains the generic parts.  The actual entry point for
the normal instruction scheduling pass is found in sched-rgn.c.
 
We compute insn priorities based on data dependencies.  Flow
@@ -77,12 +77,12 @@ along with GCC; see the file COPYING3.  If not see
 
Before reload, an extended analysis of interblock data dependences
is required for interblock scheduling.  This is performed in
-   compute_block_backward_dependences ().
+   compute_block_dependences ().
 
Dependencies set up by memory references are treated in exactly the
same way as other dependencies, by using insn backward dependences
INSN_BACK_DEPS.  INSN_BACK_DEPS are translated into forward dependences
-   INSN_FORW_DEPS the purpose of forward list scheduling.
+   INSN_FORW_DEPS for the purpose of forward list scheduling.
 
Having optimized the critical path, we may have also unduly
extended the lifetimes of some registers.  If an operation requires

Re: [C++] Weffc++/Wnon-virtual-dtor confusion

[Jason Merrill]

On 04/07/2014 03:39 AM, Nathan Sidwell wrote:

Jason, I shall leave it to your discretion as to whether we should
continue with this patch, or revert the original one (for 4.9).


This is OK.  Post 4.9 we might try removing the warn_ecpp check and see 
how that is received.


Jason

[Patch testsuite] Fix missing curly brace in gcc.target/arm/pr60657.c

[Ramana Radhakrishnan]

As subject says.

Applied as obvious.

Ramana

2014-04-07  Ramana Radhakrishnan  

* gcc.target/arm/pr60657.c: Fix missing curly brace.



--
Ramana Radhakrishnan
Principal Engineer
ARM Ltd.Index: gcc/testsuite/ChangeLog
===
--- gcc/testsuite/ChangeLog (revision 209183)
+++ gcc/testsuite/ChangeLog (working copy)
@@ -1,3 +1,7 @@
+2014-04-07  Ramana Radhakrishnan  
+
+   * gcc.target/arm/pr60657.c: Fix missing curly brace.
+
 2014-04-07  Richard Biener  
 
PR middle-end/60750
Index: gcc/testsuite/gcc.target/arm/pr60657.c
===
--- gcc/testsuite/gcc.target/arm/pr60657.c  (revision 209183)
+++ gcc/testsuite/gcc.target/arm/pr60657.c  (working copy)
@@ -10,4 +10,4 @@ bar (int x, int y)
   y = ;
   if (x & (1 << y))
 foo ();
-
+}

[PATCH][AArch64] Wire up TARGET_SIMD and TARGET_FLOAT properly

[Kyrill Tkachov]

Hi all,

Currently if we specify something like -march=armv8-a+nosimd or 
-mcpu=+nosimd the backend will not use it properly to setup the 
TARGET_SIMD internal boolean that the backend checks for SIMD availability. In the 
end, the only effective way of disabling SIMD instructions was to specify 
-mgeneral-regs-only.

This patch fixes the situation by wiring up TARGET_SIMD (and TARGET_FLOAT and 
TARGET_CRYPTO) to take ito account the relevant AARCH64_ISA_* flags.

This is a bug-fix for an issue that exists in 4.8 as well. Patch for that 
branch coming soon.
In the meantime I think this should go in at this stage.

Ok for trunk?

Thanks,
Kyrill

2014-04-04  Kyrylo Tkachov  

* config/aarch64/aarch64.h (TARGET_CPU_CPP_BUILTINS): Check
TARGET_SIMD rather than TARGET_GENERAL_REGS_ONLY.
(TARGET_SIMD): Take AARCH64_ISA_SIMD into account.
(TARGET_FLOAT): Take AARCH64_ISA_FP into account.
(TARGET_CRYPTO): Take TARGET_SIMD into account.
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index 7962aa4..2fd6df4 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -32,7 +32,7 @@
   else		\
 	builtin_define ("__AARCH64EL__");		\
 			\
-  if (!TARGET_GENERAL_REGS_ONLY)			\
+  if (TARGET_SIMD)	\
 	builtin_define ("__ARM_NEON");			\
 			\
   switch (aarch64_cmodel)\
@@ -83,9 +83,9 @@
 #define WORDS_BIG_ENDIAN (BYTES_BIG_ENDIAN)
 
 /* AdvSIMD is supported in the default configuration, unless disabled by
-   -mgeneral-regs-only.  */
-#define TARGET_SIMD !TARGET_GENERAL_REGS_ONLY
-#define TARGET_FLOAT !TARGET_GENERAL_REGS_ONLY
+   -mgeneral-regs-only or by the +nosimd extension.  */
+#define TARGET_SIMD (!TARGET_GENERAL_REGS_ONLY && AARCH64_ISA_SIMD)
+#define TARGET_FLOAT (!TARGET_GENERAL_REGS_ONLY && AARCH64_ISA_FP)
 
 #define UNITS_PER_WORD		8
 
@@ -185,8 +185,8 @@ extern unsigned long aarch64_isa_flags;
 extern unsigned long aarch64_tune_flags;
 #define AARCH64_TUNE_SLOWMUL   (aarch64_tune_flags & AARCH64_FL_SLOWMUL)
 
-/* Crypto is an optional feature.  */
-#define TARGET_CRYPTO AARCH64_ISA_CRYPTO
+/* Crypto is an optional extension to AdvSIMD.  */
+#define TARGET_CRYPTO (TARGET_SIMD && AARCH64_ISA_CRYPTO)
 
 /* Standard register usage.  */
 

[PATCH] Fix PR60766

[Richard Biener]

This fixes PR60766, a case where IVOPTs generating signed expressions
from originally unsigned ones breaks because that introduces undefined
overflow and VRP manages to exploit that fact.  The solution is to
use unsigned arithmetic when doing simplification using tree-affine.
In general that's always mandated as that facility doesn't care for
association limits that signed expressions have.  Rather than trying
to address this generally the folllowing fixes the single place
that this bug runs into.

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk
sofar.

Richard.

2014-04-07  Richard Biener  

PR tree-optimization/60766
* tree-ssa-loop-ivopts.c (cand_value_at): Compute in an
unsigned type.
(may_eliminate_iv): Convert cand_value_at result to desired
type.

* gcc.dg/torture/pr60766.c: New testcase.

Index: gcc/tree-ssa-loop-ivopts.c
===
*** gcc/tree-ssa-loop-ivopts.c  (revision 209181)
--- gcc/tree-ssa-loop-ivopts.c  (working copy)
*** cand_value_at (struct loop *loop, struct
*** 4236,4243 
tree steptype = type;
if (POINTER_TYPE_P (type))
  steptype = sizetype;
  
!   tree_to_aff_combination (iv->step, steptype, &step);
tree_to_aff_combination (niter, TREE_TYPE (niter), &nit);
aff_combination_convert (&nit, steptype);
aff_combination_mult (&nit, &step, &delta);
--- 4236,4245 
tree steptype = type;
if (POINTER_TYPE_P (type))
  steptype = sizetype;
+   steptype = unsigned_type_for (type);
  
!   tree_to_aff_combination (iv->step, TREE_TYPE (iv->step), &step);
!   aff_combination_convert (&step, steptype);
tree_to_aff_combination (niter, TREE_TYPE (niter), &nit);
aff_combination_convert (&nit, steptype);
aff_combination_mult (&nit, &step, &delta);
*** cand_value_at (struct loop *loop, struct
*** 4245,4250 
--- 4247,4254 
  aff_combination_add (&delta, &step);
  
tree_to_aff_combination (iv->base, type, val);
+   if (!POINTER_TYPE_P (type))
+ aff_combination_convert (val, steptype);
aff_combination_add (val, &delta);
  }
  
*** may_eliminate_iv (struct ivopts_data *da
*** 4623,4629 
  
cand_value_at (loop, cand, use->stmt, desc->niter, &bnd);
  
!   *bound = aff_combination_to_tree (&bnd);
*comp = iv_elimination_compare (data, use);
  
/* It is unlikely that computing the number of iterations using division
--- 4627,4634 
  
cand_value_at (loop, cand, use->stmt, desc->niter, &bnd);
  
!   *bound = fold_convert (TREE_TYPE (cand->iv->base),
!aff_combination_to_tree (&bnd));
*comp = iv_elimination_compare (data, use);
  
/* It is unlikely that computing the number of iterations using division
Index: gcc/testsuite/gcc.dg/torture/pr60766.c
===
*** gcc/testsuite/gcc.dg/torture/pr60766.c  (revision 0)
--- gcc/testsuite/gcc.dg/torture/pr60766.c  (working copy)
***
*** 0 
--- 1,15 
+ /* { dg-do run } */
+ 
+ int m = 9;
+ 
+ int main()
+ {
+   int n, x;
+ 
+   n = m;
+   for (x = 0; x <= n; x++)
+ if (n == x + (x + 1) + (x + 2))
+   return 0;
+ 
+   __builtin_abort();
+ }

Re: [PATCH] Adjust builtin-bswap-6/7

[Richard Biener]

On Fri, Apr 4, 2014 at 7:51 PM, Jeff Law  wrote:
> On 04/04/14 10:18, Andreas Krebbel wrote:
>>
>> Hi,
>>
>> the attached patch modifies the builtin-bswap-6/7 testcases in order
>> to prevent GCC from using math instead of a compare.  Only with a
>> compare the folding in combine actually takes place.
>>
>> Whether the return value is produce with a compare or not depends
>> again on the value of branch cost.
>>
>> Ideally we would be able to do the folding also with the math trick
>> but it is probably not that easy since we have already lost the
>> information that in the end all we need is a 0 or a 1.
>>
>> Ok?
>>
>> Bye,
>>
>> -Andreas-
>>
>> 2014-04-04  Andreas Krebbel  
>>
>> * gcc.dg/builtin-bswap-6.c: Adjust return value to disable GCC
>> optimization.
>> * gcc.dg/builtin-bswap-7.c: Likewise.
>
> OK.

The adjusted testcases now fail on x86_64/i?86 at least.  See PR60776.

Richard.

> Jeff
>

Re: [ARM] Fix PR target/60504

[Ramana Radhakrishnan]

Sorry about the delayed review. I seem to have missed this earlier.

On Tue, Mar 25, 2014 at 12:21 PM, Eric Botcazou  wrote:
> Hi,
>
> because of popular demand we switched the Ada compiler to ZCX, i.e. table-
> driven EH scheme, on ARM/Linux, only to discover that GCC doesn't generate
> correct EH tables, which means that the Ada compiler cannot catch exceptions.

>
> See also the earlier message posted by Tristan:
>   http://gcc.gnu.org/ml/gcc/2013-12/msg00157.html
>
> The problem is the discrepancy between the declared encoding and the actual
> encoding of the TType.  The latter is specified by the EABI: symbols must be
> relocated with an R_ARM_TARGET2 relocation, as visible in the assembly:
>
> .LLSDACSE0:
> .byte   0x1 @ Action record table
> .byte   0
> .align  2
> .word   _ZTIi(TARGET2)
>
> R_ARM_TARGET2 means DW_EH_PE_pcrel | DW_EH_PE_indirect in DWARF parlance on
> Linux, but the declared TType encoding is different:
>
> .LLSDA0:
> .byte   0xff@ @LPStart format (omit)
> .byte   0   @ @TType format (absolute)
> .uleb128 .LLSDATT0-.LLSDATTD0   @ @TType base offset
>
> i.e. it's the DW_EH_PE_absptr default.
>
> This works in C++ because of the _GLIBCXX_OVERRIDE_TTYPE_ENCODING kludge,
> which overrides the declared encoding during the parsing of the EH table:

Ok and I suspect the kludge has to continue for some more time.

>
>   // Find @TType, the base of the handler and exception spec type data.
>   info->ttype_encoding = *p++;
>   if (info->ttype_encoding != DW_EH_PE_omit)
> {
> #if _GLIBCXX_OVERRIDE_TTYPE_ENCODING
>   /* Older ARM EABI toolchains set this value incorrectly, so use a
>  hardcoded OS-specific format.  */
>   info->ttype_encoding = _GLIBCXX_OVERRIDE_TTYPE_ENCODING;
> #endif
>   p = read_uleb128 (p, &tmp);
>   info->TType = p + tmp;
> }
>   else
> info->TType = 0;
>
> This comes from http://gcc.gnu.org/ml/gcc-patches/2011-09/msg00765.html

>
> Note that the above comment suggests that Paul's intent was to fix the bogus
> declared encoding in newer toolchains, but he didn't because of a misplaced
> #endif as spotted by Tristan.  Hence the attached patch, which aligns the
> declared encoding with the actual encoding and yields the correct:
>
> .LLSDA0:
> .byte   0xff@ @LPStart format (omit)
> .byte   0x90@ @TType format (indirect pcrel)
> .uleb128 .LLSDATT0-.LLSDATTD0   @ @TType base offset
>
> which fixes the Ada failures in the process.  It was kindly tested by Bernd
> Edlinger on armv7l-unknown-linux-gnueabihf (all languages) with no regressions
> so we would like to have it for 4.9.x.

Ok if no regressions.

Ramana


>
>
> 2014-04-25  Douglas B Rupp  
>
> PR target/60504
> * config/arm/arm.h (ASM_PREFERRED_EH_DATA_FORMAT): Expose from
> ARM_TARGET2_DWARF_FORMAT.
>
>
>
> 2014-04-25  Eric Botcazou  
>
> * gnat.dg/test_raise_from_pure.adb: UnXFAIL for ARM.
>
>
> --
> Eric Botcazou

Re: [libitm] Check if GCC uses assembler cfi support

[Richard Henderson]

On 04/07/2014 04:33 AM, Rainer Orth wrote:
> The patch allowed i386-pc-solaris2.11 bootstraps to finish without
> regressions on trunk and 4.8 branch, still need to test 4.7 branch.
> 
> Ok for trunk, 4.8 and 4.7 branches once testing completes?
> 
>   Rainer
> 
> 
> 2014-04-07  Rainer Orth  
> 
>   * config/generic/asmcfi.h: Also check for
>   __GCC_HAVE_DWARF2_CFI_ASM.

Looks fine.


r~

Re: [PATCH, ARM] Fix PR60609 (Error: value of 256 too large for field of 1 bytes)

[Charles Baylis]

On 4 April 2014 15:50, Ramana Radhakrishnan  wrote:
> Additionally the testing has only considered Thumb2 - since we also do
> jumptable shortening for Thumb1 and given this late change it's worth
> also testing this on Thumb1 and making sure there are no regressions.
> Maybe Joey can help there if you aren't set up to do this.
>
> Ok if no regressions and modulo RM objections.

I have tested v5t Thumb-1 with no regressions on qemu.

> One minor Changelog nit.
>
> 2014-04-02  Charles Baylis  
>
> PR target/60609
> * config/arm/arm.h (ASM_OUTPUT_CASE_END) Remove.
> (LABEL_ALIGN_AFTER_BARRIER) Align barriers which occur after
> ADDR_DIFF_VEC.
>
> s/)/):/g above.

Noted.

Re: [PATCH] Adjust builtin-bswap-6/7

[Andreas Krebbel]

On Mon, Apr 07, 2014 at 04:19:06PM +0200, Richard Biener wrote:
> The adjusted testcases now fail on x86_64/i?86 at least.  See PR60776.

They seem to require at least -O2 on x86 with that change. Ok to apply?

diff --git a/gcc/testsuite/gcc.dg/builtin-bswap-6.c 
b/gcc/testsuite/gcc.dg/builtin-bswap-6.c
index 6f0c782..f346c2f 100644
--- a/gcc/testsuite/gcc.dg/builtin-bswap-6.c
+++ b/gcc/testsuite/gcc.dg/builtin-bswap-6.c
@@ -1,7 +1,7 @@
 /* { dg-do compile { target arm*-*-* alpha*-*-* i?86-*-* powerpc*-*-* 
rs6000-*-* x86_64-*-* s390*-*-* } } */
 /* { dg-require-effective-target stdint_types } */
-/* { dg-options "-O -fdump-rtl-combine" } */
-/* { dg-options "-O -fdump-rtl-combine -march=z900" { target s390-*-* } } */
+/* { dg-options "-O2 -fdump-rtl-combine" } */
+/* { dg-options "-O2 -fdump-rtl-combine -march=z900" { target s390-*-* } } */
 
 /* The test intentionally returns 1/2 instead of the obvious 0/1 to
prevent GCC from calculating the return value with arithmetic
diff --git a/gcc/testsuite/gcc.dg/builtin-bswap-7.c 
b/gcc/testsuite/gcc.dg/builtin-bswap-7.c
index 0eecdd8..98529f2 100644
--- a/gcc/testsuite/gcc.dg/builtin-bswap-7.c
+++ b/gcc/testsuite/gcc.dg/builtin-bswap-7.c
@@ -1,7 +1,7 @@
 /* { dg-do compile { target arm*-*-* alpha*-*-* ia64*-*-* x86_64-*-* s390x-*-* 
powerpc*-*-* rs6000-*-* } } */
 /* { dg-require-effective-target stdint_types } */
 /* { dg-require-effective-target lp64 } */
-/* { dg-options "-O -fdump-rtl-combine" } */
+/* { dg-options "-O2 -fdump-rtl-combine" } */
 
 /* The test intentionally returns 1/2 instead of the obvious 0/1 to
prevent GCC from calculating the return value with arithmetic

[Ping] [Patch C++] RFC Fix PR57958

[Dinar Temirbulatov]

Hi,
I revised the patch from
http://gcc.gnu.org/ml/gcc-patches/2014-04/msg00083.html.
This change fixes PR57958 (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57958)
For this code:
auto fn = [] (const Foo& x) {
return (x);
  };

  {
Foo a;
fn(a);
  }
Current version of trunk generates following gimple:

main(int, char**)::&)> (const struct __lambda0
* const __closure, const struct Foo & x)
{
  struct Foo D.2089;

  Foo::Foo (&D.2089, x);
  try
{
  return ;
}
  finally
{
  Foo::~Foo (&D.2089);
  D.2089 = {CLOBBER};
}
}

And after fix applied gimple product looks like this:

main(int, char**)::&)> (const struct __lambda0
* const __closure, const struct Foo & x)
{
  Foo::Foo (, x);
  return ;
}

looking at the code that I think caused the issue:
cp/typeck.c:8600
  /* We can't initialize a register from a AGGR_INIT_EXPR.  */
  else if (! cfun->returns_struct
   && TREE_CODE (retval) == TARGET_EXPR
   && TREE_CODE (TREE_OPERAND (retval, 1)) == AGGR_INIT_EXPR)
retval = build2 (COMPOUND_EXPR, TREE_TYPE (retval), retval,
 TREE_OPERAND (retval, 0));
It is not clear to me the propose of constructing COMPOUND_EXPR here,
comment says: "We can't initialize a register from a AGGR_INIT_EXPR.",
but how on the front-end side we could determine what is going to end
up in a register and what is not? The code looks like a hack for some
middle-end issue awhile back?  I hoped to trigger any code generation
problem by removing the code and testing regression testsuite on
x86_64-pc-linux-gnu, but no regression have occured. Attached patch
was tested on x86_64-pc-linux-gnu with no new regressions. Ok to apply
to trunk?
 Thanks in advance, Dinar.


PR57958.patch
Description: Binary data

Re: [PATCH] Initialize sanitizer builtins (PR sanitizer/60745)

[Marek Polacek]

On Sun, Apr 06, 2014 at 09:46:12AM -0700, Mike Stump wrote:
> On Apr 5, 2014, at 10:36 AM, Marek Polacek  wrote:
> > 
> > I'll wait a day or two for possible comments.
> 
> So, my only comment would be it would be nice to document the flags that can 
> vary between pch creation and use time, and which cause an invalidation of 
> the pch file.  This flag then is documented as safe to change and not 
> invalidate the pch file.

That might be nice, but currently there isn't any such list, right?

Marek

Re: Warn when returning the address of a temporary (middle-end)

[Jeff Law]

On 04/05/14 07:52, Marc Glisse wrote:

Hello,

we have front-end warnings about returning the address of a local
variable. However, quite often in C++, people don't directly return the
address of a temporary, it goes through a few functions which hide that
fact. After some inlining, the fact that we are returning the address of
a local variable can become obvious to the compiler, to the point where
I have used, for debugging purposes, grep 'return &' on the optimized
dump produced with -O3 -fkeep-inline-functions (I then had to sort
through the global/local variables).

fold_stmt looks like a good place for this, but it could go almost
anywhere. It has to happen after enough inlining / copy propagation to
make it useful though.
I was wondering if this would be better implemented as a propagation 
problem so that cases where some, but not all paths to the return 
statement have &local which reaches the return.  ie


...
if (foo)
  x = &local
else
  x = &global

return x;

ISTM it ought to be a standard propagation problem and that the 
problematical ADDR_EXPRs are all going to be in PHI nodes.  So I think 
you just search for problematical ADDR_EXPRs in PHI nodes as your seeds, 
then forward propagate through the CFG.  Ultimately looking for any 
cases where those ADDR_EXPRs ultimately reach the return statement.


Thoughts?

jeff

Re: Warn when returning the address of a temporary (middle-end)

[Marc Glisse]

On Mon, 7 Apr 2014, Jeff Law wrote:


On 04/05/14 07:52, Marc Glisse wrote:

Hello,

we have front-end warnings about returning the address of a local
variable. However, quite often in C++, people don't directly return the
address of a temporary, it goes through a few functions which hide that
fact. After some inlining, the fact that we are returning the address of
a local variable can become obvious to the compiler, to the point where
I have used, for debugging purposes, grep 'return &' on the optimized
dump produced with -O3 -fkeep-inline-functions (I then had to sort
through the global/local variables).

fold_stmt looks like a good place for this, but it could go almost
anywhere. It has to happen after enough inlining / copy propagation to
make it useful though.
I was wondering if this would be better implemented as a propagation problem 
so that cases where some, but not all paths to the return statement have 
&local which reaches the return.  ie


...
if (foo)
 x = &local
else
 x = &global

return x;

ISTM it ought to be a standard propagation problem and that the problematical 
ADDR_EXPRs are all going to be in PHI nodes.  So I think you just search for 
problematical ADDR_EXPRs in PHI nodes as your seeds, then forward propagate 
through the CFG.  Ultimately looking for any cases where those ADDR_EXPRs 
ultimately reach the return statement.


Thoughts?


I would tend to start from the return statements (assuming the return type 
is a pointer), look at the defining statement, do things if it is an 
assignment of an addr_expr, and recurse if it is a PHI. But maybe my brain 
is cabled backwards ;-)


It would be good to piggy back on a pass that already does something 
similar, if we go that way. Do you know a convenient one?


I am also afraid we may get more false positives, but maybe not.

Last, the simple version actually works well enough that it discovered at 
least one real bug in real code, and I am afraid that by refining it too 
much we'll delay and get nothing in the end (my time and my knowledge of 
the compiler are limited enough to make it a real possibility). But I 
admit that's not a good argument.


Thanks for the comments,

--
Marc Glisse

Fixing PR60773

[Cong Hou]

In the patch of
PR60656(http://gcc.gnu.org/ml/gcc-patches/2014-03/msg01668.html), the
test case requires GCC to vectorize the widen-mult pattern from si to
di types. This may result in test failures on some platforms that
don't support this pattern. This patch adds a new target
vect_widen_mult_si_to_di_pattern to fix this issue.

Bootstrapped and tested on x86_64.

OK for trunk?


thanks,
Cong




diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 414a745..ea860e7 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,11 @@
+2014-04-07  Cong Hou  
+
+ PR testsuite/60773
+ * testsuite/lib/target-supports.exp:
+ Add check_effective_target_vect_widen_mult_si_to_di_pattern.
+ * gcc.dg/vect/pr60656.c: Update the test by checking if the targets
+ vect_widen_mult_si_to_di_pattern and vect_long are supported.
+
 2014-03-28  Cong Hou  

  PR tree-optimization/60656
diff --git a/gcc/testsuite/gcc.dg/vect/pr60656.c
b/gcc/testsuite/gcc.dg/vect/pr60656.c
index ebaab62..b80e008 100644
--- a/gcc/testsuite/gcc.dg/vect/pr60656.c
+++ b/gcc/testsuite/gcc.dg/vect/pr60656.c
@@ -1,5 +1,7 @@
 /* { dg-require-effective-target vect_int } */
+/* { dg-require-effective-target vect_long } */

+#include 
 #include "tree-vect.h"

 __attribute__ ((noinline)) long
@@ -12,7 +14,7 @@ foo ()
   for(i = 0; i < 4; ++i)
 {
   long P = v[i];
-  s += P*P*P;
+  s += P * P * P;
 }
   return s;
 }
@@ -27,7 +29,7 @@ bar ()
   for(i = 0; i < 4; ++i)
 {
   long P = v[i];
-  s += P*P*P;
+  s += P * P * P;
   __asm__ volatile ("");
 }
   return s;
@@ -35,11 +37,12 @@ bar ()

 int main()
 {
+  check_vect ();
+
   if (foo () != bar ())
 abort ();
   return 0;
 }

-/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" } } */
+/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" {
target vect_widen_mult_si_to_di_pattern } } } */
 /* { dg-final { cleanup-tree-dump "vect" } } */
-
diff --git a/gcc/testsuite/lib/target-supports.exp
b/gcc/testsuite/lib/target-supports.exp
index bee8471..6d9d689 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -3732,6 +3732,27 @@ proc
check_effective_target_vect_widen_mult_hi_to_si_pattern { } {
 }

 # Return 1 if the target plus current options supports a vector
+# widening multiplication of *int* args into *long* result, 0 otherwise.
+#
+# This won't change for different subtargets so cache the result.
+
+proc check_effective_target_vect_widen_mult_si_to_di_pattern { } {
+global et_vect_widen_mult_si_to_di_pattern
+
+if [info exists et_vect_widen_mult_si_to_di_pattern_saved] {
+verbose
"check_effective_target_vect_widen_mult_si_to_di_pattern: using cached
result" 2
+} else {
+if {[istarget ia64-*-*]
+  || [istarget i?86-*-*]
+  || [istarget x86_64-*-*] } {
+set et_vect_widen_mult_si_to_di_pattern_saved 1
+}
+}
+verbose "check_effective_target_vect_widen_mult_si_to_di_pattern:
returning $et_vect_widen_mult_si_to_di_pattern_saved" 2
+return $et_vect_widen_mult_si_to_di_pattern_saved
+}
+
+# Return 1 if the target plus current options supports a vector
 # widening shift, 0 otherwise.
 #
 # This won't change for different subtargets so cache the result.

[patch] fix some libstdc++ testsuite bugs

[Jonathan Wakely]

This fixes some errors in the testsuite which aren't caught by G++
(for known reasons).

Tested x86_64-linux, committed to trunk.


commit 9c8945c0d7adf909f8b46c0331d66a00af4187b9
Author: Jonathan Wakely 
Date:   Mon Apr 7 18:17:03 2014 +0100

Fix testsuite bugs.

* testsuite/20_util/exchange/1.cc: Add missing return statements.
* testsuite/20_util/pair/40925.cc: Avoid most vexing parse.
* testsuite/22_locale/codecvt_byname/50714.cc: Add missing exception
specifications.

diff --git a/libstdc++-v3/testsuite/20_util/exchange/1.cc 
b/libstdc++-v3/testsuite/20_util/exchange/1.cc
index 923920e..2f8825d 100644
--- a/libstdc++-v3/testsuite/20_util/exchange/1.cc
+++ b/libstdc++-v3/testsuite/20_util/exchange/1.cc
@@ -78,8 +78,8 @@ void test04()
 To() = default;
 To(const To&) = default;
 To(const From&) = delete;
-To& operator=(const From&) { value = 1; }
-To& operator=(From&&) { value = 2; }
+To& operator=(const From&) { value = 1; return *this; }
+To& operator=(From&&) { value = 2; return *this; }
   };
 
   To t;
diff --git a/libstdc++-v3/testsuite/20_util/pair/40925.cc 
b/libstdc++-v3/testsuite/20_util/pair/40925.cc
index 599fd38..5b34a62 100644
--- a/libstdc++-v3/testsuite/20_util/pair/40925.cc
+++ b/libstdc++-v3/testsuite/20_util/pair/40925.cc
@@ -1,4 +1,4 @@
-// { dg-options "-std=gnu++0x" }
+// { dg-options "-std=gnu++11" }
 // { dg-do compile }
 
 // Copyright (C) 2009-2014 Free Software Foundation, Inc.
@@ -63,5 +63,5 @@ void test01()
   std::pair p15(move_only(), ip);
   std::pair p16(move_only(), mp);
 
-  std::pair p17(move_only(), move_only());
+  std::pair p17{move_only(), move_only()};
 }
diff --git a/libstdc++-v3/testsuite/22_locale/codecvt_byname/50714.cc 
b/libstdc++-v3/testsuite/22_locale/codecvt_byname/50714.cc
index e40717b..d42caa5 100644
--- a/libstdc++-v3/testsuite/22_locale/codecvt_byname/50714.cc
+++ b/libstdc++-v3/testsuite/22_locale/codecvt_byname/50714.cc
@@ -52,12 +52,12 @@ namespace std
 
   template<>
   int
-  codecvt::do_encoding() const
+  codecvt::do_encoding() const throw()
   { return 0; }
 
   template<>
   bool
-  codecvt::do_always_noconv() const
+  codecvt::do_always_noconv() const throw()
   { return false; }
 
   template<>
@@ -69,7 +69,7 @@ namespace std
 
   template<>
   int
-  codecvt::do_max_length() const
+  codecvt::do_max_length() const throw()
   { return 4; }
 }
 

Re: [Ping] [Patch C++] RFC Fix PR57958

[Jason Merrill]

On 04/07/2014 12:56 PM, Dinar Temirbulatov wrote:

  /* We can't initialize a register from a AGGR_INIT_EXPR.  */
  else if (! cfun->returns_struct
   && TREE_CODE (retval) == TARGET_EXPR
   && TREE_CODE (TREE_OPERAND (retval, 1)) == AGGR_INIT_EXPR)
retval = build2 (COMPOUND_EXPR, TREE_TYPE (retval), retval,
 TREE_OPERAND (retval, 0));


That code is extremely ancient.  I agree that it seems kind of useless, 
but the fact that we're hitting it indicates a bug: cfun->returns_struct 
should have been set by apply_deduced_return_type.  I guess we need to 
call complete_type before aggregate_value_p.  Let's fix that now, and 
then remove this chunk of code after 4.9 branches.



+++ b/gcc/testsuite/g++.dg/cpp1y/pr57958.C


The cpp1y directory is for C++14 tests; C++11 tests should go in cpp0x. 
 Perhaps we should rename the directories.  :)



+// { dg-options -std=c++11 }
+// { dg-do run }


Please use { dg-do run { target c++11 } } rather than specify -std in 
dg-options.


Jason

Re: [Ping] [Patch C++] RFC Fix PR57958

[Jason Merrill]

On 04/07/2014 03:46 PM, Jason Merrill wrote:

I guess we need to call complete_type before aggregate_value_p.


complete_type_or_else, actually.

Jason

[GOOGLE] AutoFDO+LIPO should invoke add_fake_edge too

[Dehao Chen]

This patch calls add_fake_edge for the AutoFDO+LIPO path.

Bootstrapped and passed regression test and performance test.

OK for google-4_8?

Thanks,
Dehao
Index: gcc/auto-profile.c
===
--- gcc/auto-profile.c  (revision 209123)
+++ gcc/auto-profile.c  (working copy)
@@ -190,6 +190,9 @@ class function_instance {
   /* Mark LOC as annotated.  */
   void mark_annotated (location_t loc);
 
+  /* Save all call targets under this function_instance in RET.  */
+  void get_all_possible_call_targets (std::set *ret) const;
+
 private:
   function_instance (unsigned name, gcov_type head_count)
   : name_(name), total_count_(0), head_count_(head_count) {}
@@ -555,6 +558,22 @@ void function_instance::mark_annotated (location_t
   iter->second.annotated = true;
 }
 
+void function_instance::get_all_possible_call_targets (
+std::set *ret) const
+{
+  for (callsite_map::const_iterator iter = callsites.begin();
+   iter != callsites.end(); ++iter)
+{
+  ret->insert (iter->second->name());
+  iter->second->get_all_possible_call_targets (ret);
+}
+  for (position_count_map::const_iterator iter = pos_counts.begin();
+   iter != pos_counts.end(); ++iter)
+for (icall_target_map::const_iterator t_iter = 
iter->second.targets.begin();
+t_iter != iter->second.targets.end(); ++t_iter)
+  ret->insert (t_iter->first);
+}
+
 /* Read the inlinied indirect call target profile for STMT and store it in
MAP, return the total count for all inlined indirect calls.  */
 
@@ -1666,6 +1685,23 @@ afdo_callsite_hot_enough_for_early_inline (struct
 return false;
 }
 
+/* Stores all possible call targets for NODE to RET.  */
+
+void get_all_possible_call_targets (struct cgraph_node *node,
+   std::vector *ret)
+{
+  std::set index_set;
+  const autofdo::function_instance *func =
+  autofdo::afdo_source_profile->get_function_instance_by_decl (
+ node->symbol.decl);
+  if (func == NULL)
+return;
+  func->get_all_possible_call_targets (&index_set);
+  for (std::set::const_iterator iter = index_set.begin();
+   iter != index_set.end(); ++iter)
+ret->push_back (autofdo::afdo_function_name_map->get_name (*iter));
+}
+
 struct simple_ipa_opt_pass pass_ipa_auto_profile =
 {
  {
Index: gcc/auto-profile.h
===
--- gcc/auto-profile.h  (revision 209123)
+++ gcc/auto-profile.h  (working copy)
@@ -21,6 +21,8 @@ along with GCC; see the file COPYING3.  If not see
 #ifndef AUTO_PROFILE_H
 #define AUTO_PROFILE_H
 
+#include 
+
 /* Read, process, finalize AutoFDO data structures.  */
 extern void init_auto_profile (void);
 extern void end_auto_profile (void);
@@ -28,4 +30,7 @@ extern void end_auto_profile (void);
 /* Returns TRUE if EDGE is hot enough to be inlined early.  */
 extern bool afdo_callsite_hot_enough_for_early_inline (struct cgraph_edge *);
 
+/* Stores all possible call targets for NODE to RET.  */
+extern void get_all_possible_call_targets (struct cgraph_node *,
+  std::vector *);
 #endif /* AUTO_PROFILE_H */
Index: gcc/cgraphbuild.c
===
--- gcc/cgraphbuild.c   (revision 209123)
+++ gcc/cgraphbuild.c   (working copy)
@@ -37,6 +37,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "except.h"
 #include "l-ipo.h"
 #include "ipa-inline.h"
+#include "auto-profile.h"
 
 /* Context of record_reference.  */
 struct record_reference_ctx
@@ -244,6 +245,25 @@ add_fake_indirect_call_edges (struct cgraph_node *
   if (!L_IPO_COMP_MODE)
 return;
 
+  if (flag_auto_profile)
+{
+  std::vector targets;
+  get_all_possible_call_targets (node, &targets);
+  for (std::vector::const_iterator iter = targets.begin();
+  iter != targets.end(); ++iter)
+   {
+ struct cgraph_node * callee = (
+ cgraph_node_for_asm (get_identifier (*iter)));
+ if (!callee)
+   continue;
+ if (cgraph_pre_profiling_inlining_done)
+   callee = cgraph_lipo_get_resolved_node (callee->symbol.decl);
+ if (callee)
+   cgraph_create_edge (node, callee, NULL, 1, 0);
+   }
+  return;
+}
+
   ic_counts
   = get_coverage_counts_no_warn (DECL_STRUCT_FUNCTION (node->symbol.decl),
  GCOV_COUNTER_ICALL_TOPNV, &n_counts);

Re: [GOOGLE] AutoFDO+LIPO should invoke add_fake_edge too

[Xinliang David Li]

ok (after fixing the format -- a  function name starts a new line in
function def).

David

On Mon, Apr 7, 2014 at 12:49 PM, Dehao Chen  wrote:
> This patch calls add_fake_edge for the AutoFDO+LIPO path.
>
> Bootstrapped and passed regression test and performance test.
>
> OK for google-4_8?
>
> Thanks,
> Dehao

Test failure: gcc.dg/tree-ssa/ssa-ifcombine-13.c

[Steve Ellcey]

Jakub,

This test that you added on March 12th has always been failing
for me on MIPS.  It looks like you expect MIPS to optimize the
if statement but mips.h has:

#define LOGICAL_OP_NON_SHORT_CIRCUIT 0

So the if always shows up in ssa-ifcombine-13.c.169t.optimized.

Should MIPS be removed from the options and dg-final lists?

Steve Ellcey
sell...@mips.com

[PATCH] Fix PR c++/60764

[Patrick Palka]

Hello,

This patch fixes fixes an erroneous -Wnonnull warning when calling a
constructor declared with __attribute__ ((nonnull (1))).  The issue is
that a NULL pointer is used as the placeholder object for the "this"
parameter of the constructor method call until a target object is
determined and substituted for the placeholder object during
gimplification.  Meanwhile, check_function_arguments is called in
build_over_call which notices that there is a NULL pointer argument for
a parameter that's been marked nonnull, thus issuing the -Wnonnull
warning.

This patch modifies the C++ frontend to use build_dummy_object() instead
of a NULL pointer as the placeholder object to a constructor method
call.  This suppresses the warning issued by check_function_arguments
because the object returned by build_dummy_object() is not determined to
be a NULL pointer.

I bootstrapped and regtested this patch on x86_64-unknown-linux-gnu.

PR c++/60764
* call.c (build_user_type_coversion): Use build_dummy_object
to create the placeholder object for a constructor method call.
(build_special_member_call): Likewise.
(build_over_call): Check for the placeholder object with
is_dummy_object.
(build_new_method_call_1): Likewise.  Don't attempt to resolve
a dummy object for a constructor method call.
---
 gcc/cp/call.c| 14 +-
 gcc/testsuite/g++.dg/warn/nonnull2.C | 10 ++
 2 files changed, 15 insertions(+), 9 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/warn/nonnull2.C

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index ae0d4ff..ff5c834 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -3519,8 +3519,7 @@ build_user_type_conversion_1 (tree totype, tree expr, int 
flags,
 {
   int ctorflags = flags;
 
-  first_arg = build_int_cst (build_pointer_type (totype), 0);
-  first_arg = build_fold_indirect_ref (first_arg);
+  first_arg = build_dummy_object (totype);
 
   /* We should never try to call the abstract or base constructor
 from here.  */
@@ -7096,7 +7095,7 @@ build_over_call (struct z_candidate *cand, int flags, 
tsubst_flags_t complain)
 ctor is trivial, do a bitwise copy with a simple TARGET_EXPR for a
 temp or an INIT_EXPR otherwise.  */
   fa = argarray[0];
-  if (integer_zerop (fa))
+  if (is_dummy_object (fa))
{
  if (TREE_CODE (arg) == TARGET_EXPR)
return arg;
@@ -7434,10 +7433,7 @@ build_special_member_call (tree instance, tree name, 
vec **args,
 
   /* Handle the special case where INSTANCE is NULL_TREE.  */
   if (name == complete_ctor_identifier && !instance)
-{
-  instance = build_int_cst (build_pointer_type (class_type), 0);
-  instance = build1 (INDIRECT_REF, class_type, instance);
-}
+instance = build_dummy_object (class_type);
   else
 {
   if (name == complete_dtor_identifier
@@ -7747,8 +7743,7 @@ build_new_method_call_1 (tree instance, tree fns, 
vec **args,
 
   if (init)
{
- if (INDIRECT_REF_P (instance)
- && integer_zerop (TREE_OPERAND (instance, 0)))
+ if (is_dummy_object (instance))
return get_target_expr_sfinae (init, complain);
  init = build2 (INIT_EXPR, TREE_TYPE (instance), instance, init);
  TREE_SIDE_EFFECTS (init) = true;
@@ -7847,6 +7842,7 @@ build_new_method_call_1 (tree instance, tree fns, 
vec **args,
}
 
  if (TREE_CODE (TREE_TYPE (fn)) == METHOD_TYPE
+ && !DECL_CONSTRUCTOR_P (fn)
  && is_dummy_object (instance))
{
  instance = maybe_resolve_dummy (instance);
diff --git a/gcc/testsuite/g++.dg/warn/nonnull2.C 
b/gcc/testsuite/g++.dg/warn/nonnull2.C
new file mode 100644
index 000..10515a4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/nonnull2.C
@@ -0,0 +1,10 @@
+// PR c++/60764
+// { dg-options "-Wall" }
+
+struct foo
+{
+  foo () __attribute__ ((nonnull (1)));
+};
+
+const foo &x = foo (); // { dg-bogus "null argument" }
+foo y = foo (); // { dg-bogus "null argument" }
-- 
1.9.1

[PATCH] Fix PR c++/60765

[Patrick Palka]

Hi,

This patch teaches the C++ frontend how to handle function attributes
attached to pointer-to-member-function declarations.

I bootstrapped and regtested this patch on x86_64-unknown-linux-gnu.

2014-04-05  Patrick Palka  

PR c++/60765
* decl2.c (cplus_decl_attributes): Handle
pointer-to-member-function declarations.
---
 gcc/cp/decl2.c  | 10 +-
 gcc/testsuite/g++.dg/cpp0x/gen-attrs-36-1.C |  2 +-
 gcc/testsuite/g++.dg/ext/attrib49.C | 20 
 3 files changed, 30 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/ext/attrib49.C

diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index 80f15a4..91a75f3 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -1427,7 +1427,15 @@ cplus_decl_attributes (tree *decl, tree attributes, int 
flags)
   if (TREE_CODE (*decl) == TEMPLATE_DECL)
 decl = &DECL_TEMPLATE_RESULT (*decl);
 
-  decl_attributes (decl, attributes, flags);
+  if (TREE_TYPE (*decl) && TYPE_PTRMEMFUNC_P (TREE_TYPE (*decl)))
+{
+  attributes
+   = decl_attributes (decl, attributes, flags | ATTR_FLAG_FUNCTION_NEXT);
+  decl_attributes (&TYPE_PTRMEMFUNC_FN_TYPE (TREE_TYPE (*decl)),
+  attributes, flags);
+}
+  else
+decl_attributes (decl, attributes, flags);
 
   if (TREE_CODE (*decl) == TYPE_DECL)
 SET_IDENTIFIER_TYPE_VALUE (DECL_NAME (*decl), TREE_TYPE (*decl));
diff --git a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-36-1.C 
b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-36-1.C
index 2aae9ca..131af36 100644
--- a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-36-1.C
+++ b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-36-1.C
@@ -6,6 +6,6 @@ typedef int (*F [[gnu::warn_unused_result]]) (int);
 
 typedef int (*F2 [[gnu::warn_unused_result]]) (int);
 
-typedef int (S::*F3 [[gnu::warn_unused_result]]) (int); // { dg-warning "only 
applies to function types" }
+typedef int (S::*F3 [[gnu::warn_unused_result]]) (int);
 
 typedef int [[gnu::warn_unused_result]] (*F5) (int); // { dg-warning "ignored" 
}
diff --git a/gcc/testsuite/g++.dg/ext/attrib49.C 
b/gcc/testsuite/g++.dg/ext/attrib49.C
new file mode 100644
index 000..99c6154
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/attrib49.C
@@ -0,0 +1,20 @@
+// PR c++/60765
+// { dg-options "-Wall -Wunused-parameter" }
+
+struct foo
+{
+} x;
+
+void (foo::*g) (int *) __attribute__ ((nonnull (2)));
+
+void
+fun1 (void (foo::*f) (int *) __attribute__ ((nonnull (2
+{
+(x.*f) ((int *) 0); // { dg-warning "null argument" }
+}
+
+void
+fun2 (void (foo::*f) () __attribute__ ((nonnull, unused))) // { dg-bogus 
"unused" }
+{
+(x.*g) ((int *) 0); // { dg-warning "null argument" }
+}
-- 
1.9.1

Re: [PATCH] Initialize sanitizer builtins (PR sanitizer/60745)

[Mike Stump]

On Apr 7, 2014, at 10:57 AM, Marek Polacek  wrote:
> On Sun, Apr 06, 2014 at 09:46:12AM -0700, Mike Stump wrote:
>> On Apr 5, 2014, at 10:36 AM, Marek Polacek  wrote:
>>> 
>>> I'll wait a day or two for possible comments.
>> 
>> So, my only comment would be it would be nice to document the flags that can 
>> vary between pch creation and use time, and which cause an invalidation of 
>> the pch file.  This flag then is documented as safe to change and not 
>> invalidate the pch file.
> 
> That might be nice, but currently there isn't any such list, right?

No, I don’t think there is.  Generally speaking, very few flags are safe to 
change.

Re: Warn when returning the address of a temporary (middle-end)

[Jeff Law]

On 04/07/14 12:51, Marc Glisse wrote:



I would tend to start from the return statements (assuming the return
type is a pointer), look at the defining statement, do things if it is
an assignment of an addr_expr, and recurse if it is a PHI. But maybe my
brain is cabled backwards ;-)

It works either way.



I am also afraid we may get more false positives, but maybe not.
The only false positives should come from paths which are unexecutable. 
 One could argue that if we find any that we should warn, then isolate 
the path so that we get an immediate runtime trap rather than letting 
the address of the local escape through the return value.


That in turn would argue for dumping it into 
gimple-isolate-erroneous-paths ;-)




Last, the simple version actually works well enough that it discovered
at least one real bug in real code, and I am afraid that by refining it
too much we'll delay and get nothing in the end (my time and my
knowledge of the compiler are limited enough to make it a real
possibility). But I admit that's not a good argument.
The difference is I see the enhanced version as being simple enough that 
we ought to just do it.


jeff

Re: [4.8, PATCH, rs6000] (Re: [PATCH, rs6000] More efficient vector permute for little endian)

[Bill Schmidt]

Hi,

I'm withdrawing this request as I just discovered it will sometimes be
advantageous to use vnand rather than vnor; will rework this and get
back to you.

Thanks,
Bill

On Fri, 2014-04-04 at 15:45 -0500, Bill Schmidt wrote:
> On Thu, 2014-03-20 at 20:38 -0500, Bill Schmidt wrote:
> > The original workaround for vector permute on a little endian platform
> > includes subtracting each element of the permute control vector from 31.
> > Because the upper 3 bits of each element are unimportant, this was
> > implemented as subtracting the whole vector from a splat of -1.  On
> > reflection this can be done more efficiently with a vector nor
> > operation.  This patch makes that change.
> 
> This patch was approved and committed to trunk and to the IBM 4.8
> branch.  I would like approval to commit it to the FSF 4.8 branch as
> well.  Per Richard Henderson's previous comment, I have changed the
> patch slightly to avoid the use of emit_move_insn.
> 
> Bootstrapped and tested on powerpc64le-unknown-linux-gnu.  Previous
> version burned in on trunk and IBM 4.8 branch for about two weeks.  Is
> this ok for FSF 4.8?
> 
> Thanks,
> Bill
> 
> 
> 2014-04-04  Bill Schmidt  
> 
>   * config/rs6000/rs6000.c (rs6000_expand_vector_set): Generate a
>   pattern for vector nor instead of subtract from splat(-1).
>   (altivec_expand_vec_perm_const_le): Likewise.
> 
> 
> Index: gcc/config/rs6000/rs6000.c
> ===
> --- gcc/config/rs6000/rs6000.c(revision 209122)
> +++ gcc/config/rs6000/rs6000.c(working copy)
> @@ -5621,12 +5621,10 @@ rs6000_expand_vector_set (rtx target, rtx val, int
>else 
>  {
>/* Invert selector.  */
> -  rtx splat = gen_rtx_VEC_DUPLICATE (V16QImode,
> -  gen_rtx_CONST_INT (QImode, -1));
> +  rtx notx = gen_rtx_NOT (V16QImode, force_reg (V16QImode, x));
> +  rtx andx = gen_rtx_AND (V16QImode, notx, notx);
>rtx tmp = gen_reg_rtx (V16QImode);
> -  emit_move_insn (tmp, splat);
> -  x = gen_rtx_MINUS (V16QImode, tmp, force_reg (V16QImode, x));
> -  emit_move_insn (tmp, x);
> +  emit_insn (gen_rtx_SET (VOIDmode, tmp, andx));
>  
>/* Permute with operands reversed and adjusted selector.  */
>x = gen_rtx_UNSPEC (mode, gen_rtvec (3, reg, target, tmp),
> @@ -30335,18 +30333,18 @@ altivec_expand_vec_perm_const_le (rtx operands[4])
>  
>  /* Similarly to altivec_expand_vec_perm_const_le, we must adjust the
> permute control vector.  But here it's not a constant, so we must
> -   generate a vector splat/subtract to do the adjustment.  */
> +   generate a vector NOR to do the adjustment.  */
>  
>  void
>  altivec_expand_vec_perm_le (rtx operands[4])
>  {
> -  rtx splat, unspec;
> +  rtx notx, andx, unspec;
>rtx target = operands[0];
>rtx op0 = operands[1];
>rtx op1 = operands[2];
>rtx sel = operands[3];
>rtx tmp = target;
> -  rtx splatreg = gen_reg_rtx (V16QImode);
> +  rtx norreg = gen_reg_rtx (V16QImode);
>enum machine_mode mode = GET_MODE (target);
>  
>/* Get everything in regs so the pattern matches.  */
> @@ -30359,18 +30357,14 @@ altivec_expand_vec_perm_le (rtx operands[4])
>if (!REG_P (target))
>  tmp = gen_reg_rtx (mode);
>  
> -  /* SEL = splat(31) - SEL.  */
> -  /* We want to subtract from 31, but we can't vspltisb 31 since
> - it's out of range.  -1 works as well because only the low-order
> - five bits of the permute control vector elements are used.  */
> -  splat = gen_rtx_VEC_DUPLICATE (V16QImode,
> -  gen_rtx_CONST_INT (QImode, -1));
> -  emit_move_insn (splatreg, splat);
> -  sel = gen_rtx_MINUS (V16QImode, splatreg, sel);
> -  emit_move_insn (splatreg, sel);
> +  /* Invert the selector with a VNOR.  */
> +  notx = gen_rtx_NOT (V16QImode, sel);
> +  andx = gen_rtx_AND (V16QImode, notx, notx);
> +  emit_insn (gen_rtx_SET (VOIDmode, norreg, andx));
>  
>/* Permute with operands reversed and adjusted selector.  */
> -  unspec = gen_rtx_UNSPEC (mode, gen_rtvec (3, op1, op0, splatreg), 
> UNSPEC_VPERM);
> +  unspec = gen_rtx_UNSPEC (mode, gen_rtvec (3, op1, op0, norreg),
> +UNSPEC_VPERM);
>  
>/* Copy into target, possibly by way of a register.  */
>if (!REG_P (target))
> 

Fix wrong code issue in ipa-devirt

[Jan Hubicka]

Hi,
while looking into devirtualization dumps, I noticed that I got one "false"
typed out as "true". Not sure what I was thinking of.

Martin, this may fix the wrong code issue you see in Firefox, hopefully.

Will commit it after x86_64-linux testing finishes tomorrow.
Honza

* ipa-devirt.c (maybe_record_node): Set completep to false
instead of true when node is not recorded.
Index: ipa-devirt.c
===
--- ipa-devirt.c(revision 209170)
+++ ipa-devirt.c(working copy)
@@ -650,7 +650,7 @@ maybe_record_node (vec  &
   else if (completep
   && !type_in_anonymous_namespace_p
 (method_class_type (TREE_TYPE (target
-*completep = true;
+*completep = false;
 }
 
 /* See if BINFO's type match OUTER_TYPE.  If so, lookup 

Re: Warn when returning the address of a temporary (middle-end)

[Marc Glisse]

On Mon, 7 Apr 2014, Jeff Law wrote:


I am also afraid we may get more false positives, but maybe not.
The only false positives should come from paths which are unexecutable.  One 
could argue that if we find any that we should warn, then isolate the path so 
that we get an immediate runtime trap rather than letting the address of the 
local escape through the return value.


That in turn would argue for dumping it into gimple-isolate-erroneous-paths 
;-)


I wonder if trapping may be too strong? If a function sometimes returns a 
pointer to a local variable but the caller always ignores the return value 
in those cases, we can warn, but maybe we don't want to introduce a trap? 
Replacing the address with a null pointer seemed like a compromise, it 
will trap when you try to read it, but not if you ignore it. But if you 
think we can trap, ok.


--
Marc Glisse

Re: Test failure: gcc.dg/tree-ssa/ssa-ifcombine-13.c

[Jakub Jelinek]

On Mon, Apr 07, 2014 at 01:23:57PM -0700, Steve Ellcey  wrote:
> This test that you added on March 12th has always been failing
> for me on MIPS.  It looks like you expect MIPS to optimize the
> if statement but mips.h has:
> 
> #define LOGICAL_OP_NON_SHORT_CIRCUIT 0

I wasn't aware of this.

> So the if always shows up in ssa-ifcombine-13.c.169t.optimized.
> 
> Should MIPS be removed from the options and dg-final lists?

Probably yes in that case.

Jakub

Re: [gomp4] Add tables generation

[Jakub Jelinek]

On Sat, Apr 05, 2014 at 05:22:09PM +0200, Bernd Schmidt wrote:
> On 04/05/2014 05:04 PM, Thomas Schwinge wrote:
> >Is it a linker bug that I need to add something like the following?
> >
> >--- libgcc/ompstuff.c
> >+++ libgcc/ompstuff.c
> >@@ -40,6 +40,12 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  
> >If not, see
> >  #include "libgcc_tm.h"
> >
> >  #if defined(HAVE_GAS_HIDDEN) && defined(ENABLE_OFFLOADING)
> >+# if 1
> >+/* TODO: Without the following, will get missing symbols for __start_* and
> >+   __stop_*.  Linker bug?  */
> >+static void *_funcs[0] __attribute__ ((section ("__gnu_offload_funcs"))) = 
> >{ };
> >+static void *_vars[0] __attribute__ ((section ("__gnu_offload_vars"))) = { 
> >};
> >+# endif
> 
> Things seemed to work over here, but now I'm not certain whether the
> __start_/__stop_ functionality is GNU ld specific? Maybe we should
> just go back to the previous version of this patch which didn't try
> to use this.

Somebody needs to try it with gold, I think it should support the same.
As for other linkers, don't we need linker plugin support anyway, which is
not available for other linkers?

Jakub

Re: [PATCH] Fix PR59626, _FORTIFY_SOURCE wrappers and LTO

[Ryan Hill]

On Fri, 4 Apr 2014 08:18:26 +0200
Jan Hubicka  wrote:

> Hi,
> here is an updated version of my earlier ipa.c change.  It turns out that the
> problem was that I did not drop always_inline.
> In this version I just drop always_inline attribute on all functions whose
> body is removed.  The patch will affect non-LTO compilation, too, but IMO
> only by making us to not inline&diagnose the cases where indirect call to
> always inline is turned direct in between early opts and inline. Does that
> seem acceptable? (I personally would preffer this over inventing yet another
> way to special case always_inline for LTO only; we never made any strong
> promises on always_inline and indirect calls)
> 
> Honza
> 
>   * lto-cgraph.c (input_overwrite_node): Check that partitioning
>   flags are set only during streaming.
>   * ipa.c (process_references, walk_polymorphic_call_targets,
>   symtab_remove_unreachable_nodes): Drop bodies of always inline
>   after early inlining.
>   (symtab_remove_unreachable_nodes): Remove always_inline attribute.
> 
>   * gcc.dg/lto/pr59626_0.c: New testcase.
>   * gcc.dg/lto/pr59626_1.c: New testcase.

Would it be possible to backport this to 4.8?  It fixes a number of issues
we're seeing, including a couple unrelated to LTO (eg. PR50506).


-- 
Ryan Hillpsn: dirtyepic_sk
   gcc-porting/toolchain/wxwidgets @ gentoo.org

47C3 6D62 4864 0E49 8E9E  7F92 ED38 BD49 957A 8463


signature.asc
Description: PGP signature