Re: [PATCH, libgfortran] Add overflow check to xmalloc

[Bernhard Reutner-Fischer]

On 16 June 2014 08:20:09 Janne Blomqvist  wrote:


On Sun, Jun 15, 2014 at 8:23 AM, Bernhard Reutner-Fischer
 wrote:
>
>> >> On Tue, May 20, 2014 at 12:42 AM, Janne Blomqvist
>> >>  wrote:
>> >>> On Thu, May 15, 2014 at 1:00 AM, Janne Blomqvist
>> >>>  wrote:
>>  Hi,
>> 
>>  a common malloc() pattern is "malloc(num_foo * sizeof(foo_t)", that
>>  is, create space for an array of type foo_t with num_foo elements.
>>  There is a slight danger here in that the multiplication can overflow
>>  and wrap around, and then the caller thinks it has a larger array
>>  than
>>  what malloc has actually created. The attached patch changes the
>>  libgfortran xmalloc() function to have an API similar to calloc()
>>  with
>>  two arguments, and the implementation checks for wraparound.
>> >>>
>> >>> Hello,
>> >>>
>> >>> attached is an updated patch which instead introduces a new function,
>> >>> xmallocarray, with the overflow check, and leaves the existing xmalloc
>> >>> as is. Thus avoiding the extra checking in the common case where one
>> >>> of the arguments to xmallocarray would be 1.
>> >>>
>> >>> Tested on x86_64-unknown-linux-gnu, Ok for trunk?
>> >>>
>
>
> I would prefer if xcmalloc would not be named xmallocarray.

Hmm, never heard of that one before, but I have no particular


Great, I fat-fingered it, meant xcalloc.


preference wrt the naming of the function. Ok with that rename?



--
Janne Blomqvist




Sent with AquaMail for Android
http://www.aqua-mail.com

Re: [C++ Patch] PR 33101

[Paolo Carlini]

On 15/06/14 15:37, Jason Merrill wrote:

On 06/15/2014 01:01 AM, Paolo Carlini wrote:

Committed... but, I don't think we are done yet, because it seems to me
that DR577 means that in C++11 mode we have to accept the typedef?!?

Hmm, apparently so.  Might as well apply the DR to C++98 mode as well.
Agreed, definitely makes things easier (probably will be next week, as 
soon as I'm back from the Meeting)


Thanks,
Paolo.

Re: [PATCH, libgfortran] Add overflow check to xmalloc

[Janne Blomqvist]

On Mon, Jun 16, 2014 at 10:01 AM, Bernhard Reutner-Fischer
 wrote:
> On 16 June 2014 08:20:09 Janne Blomqvist  wrote:
>
>> On Sun, Jun 15, 2014 at 8:23 AM, Bernhard Reutner-Fischer
>>  wrote:
>> >
>> >> >> On Tue, May 20, 2014 at 12:42 AM, Janne Blomqvist
>> >> >>  wrote:
>> >> >>> On Thu, May 15, 2014 at 1:00 AM, Janne Blomqvist
>> >> >>>  wrote:
>> >>  Hi,
>> >> 
>> >>  a common malloc() pattern is "malloc(num_foo * sizeof(foo_t)",
>> >>  that
>> >>  is, create space for an array of type foo_t with num_foo elements.
>> >>  There is a slight danger here in that the multiplication can
>> >>  overflow
>> >>  and wrap around, and then the caller thinks it has a larger array
>> >>  than
>> >>  what malloc has actually created. The attached patch changes the
>> >>  libgfortran xmalloc() function to have an API similar to calloc()
>> >>  with
>> >>  two arguments, and the implementation checks for wraparound.
>> >> >>>
>> >> >>> Hello,
>> >> >>>
>> >> >>> attached is an updated patch which instead introduces a new
>> >> >>> function,
>> >> >>> xmallocarray, with the overflow check, and leaves the existing
>> >> >>> xmalloc
>> >> >>> as is. Thus avoiding the extra checking in the common case where
>> >> >>> one
>> >> >>> of the arguments to xmallocarray would be 1.
>> >> >>>
>> >> >>> Tested on x86_64-unknown-linux-gnu, Ok for trunk?
>> >> >>>
>> >
>> >
>> > I would prefer if xcmalloc would not be named xmallocarray.
>>
>> Hmm, never heard of that one before, but I have no particular
>
>
> Great, I fat-fingered it, meant xcalloc.

Ah well, we already have xcalloc, which is a calloc() wrapper. The
intention of the new function here is to be a malloc() wrapper, but
with an overflow check. There is no need to zero the memory, hence
calloc() is not appropriate.



-- 
Janne Blomqvist

Re: [PATCH] Install config/vxworks-dummy.h for plugins on arm (PR plugins/45078)

[Ramana Radhakrishnan]

On 06/13/14 19:17, Jakub Jelinek wrote:

On Thu, Jun 12, 2014 at 07:47:49PM +0200, Jakub Jelinek wrote:

Seems http://gcc.gnu.org/r197156 effectively reverted
the PR45078 fix for arm*-linux* (where unfortunately tm_file
is always overridden).

Was the removal of vxworks-dummy.h from that line intentional
or just some mistake?

Seems one can't build gcc plugins on arm because of this,
because arm.h includes vxworks-dummy.h.


Here is a patch, bootstrapped/regtested on armv7hl-linux,
aarch64-linux etc.  Ok for trunk/4.9?


Ok if no regressions.

Thanks,
ramana



2014-06-13  Jakub Jelinek  

PR plugins/45078
* config.gcc (arm*-*-linux-*): Include vxworks-dummy.h in tm_file.

--- gcc/config.gcc.jj   2014-04-24 23:18:09.552816344 +0200
+++ gcc/config.gcc  2014-06-12 21:25:04.406828996 +0200
@@ -1021,7 +1021,7 @@ arm*-*-linux-*)   # ARM GNU/Linux with E
;;
esac
tmake_file="${tmake_file} arm/t-arm arm/t-arm-elf arm/t-bpabi 
arm/t-linux-eabi"
-   tm_file="$tm_file arm/bpabi.h arm/linux-eabi.h arm/aout.h arm/arm.h"
+   tm_file="$tm_file arm/bpabi.h arm/linux-eabi.h arm/aout.h vxworks-dummy.h 
arm/arm.h"
# Define multilib configuration for arm-linux-androideabi.
case ${target} in
*-androideabi)


Jakub

Re: [PATCH 1/6] Add FOR_EACH_INSN{_INFO}_{DEFS,USES,EQ_USES}

[Andreas Schwab]

Richard Sandiford  writes:

> Index: gcc/auto-inc-dec.c
> ===
> --- gcc/auto-inc-dec.c2014-06-14 20:08:31.421565177 +0100
> +++ gcc/auto-inc-dec.c2014-06-14 20:08:33.433584004 +0100
> @@ -969,7 +969,7 @@ find_inc (bool first_try)
>rtx insn;
>basic_block bb = BLOCK_FOR_INSN (mem_insn.insn);
>rtx other_insn;
> -  df_ref *def_rec;
> +  df_ref def;
>  
>/* Make sure this reg appears only once in this insn.  */
>if (count_occurrences (PATTERN (mem_insn.insn), mem_insn.reg0, 1) != 1)
> @@ -1013,9 +1013,8 @@ find_inc (bool first_try)
>  
>/* Need to assure that none of the operands of the inc instruction are
>   assigned to by the mem insn.  */
> -  for (def_rec = DF_INSN_DEFS (mem_insn.insn); *def_rec; def_rec++)
> +  FOR_EACH_INSN_DEF (def, mem_insn.insn)
>  {
> -  df_ref def = *def_rec;
>unsigned int regno = DF_REF_REGNO (def);
>if ((regno == REGNO (inc_insn.reg0))
> || (regno == REGNO (inc_insn.reg_res)))
> @@ -1342,7 +1341,7 @@ merge_in_block (int max_reg, basic_block
>  
>FOR_BB_INSNS_REVERSE_SAFE (bb, insn, curr)
>  {
> -  unsigned int uid = INSN_UID (insn);
> +  df_insn_info *insn_info = DF_INSN_INFO_GET (insn);
>bool insn_is_add_or_inc = true;
>  
>if (!NONDEBUG_INSN_P (insn))
> @@ -1418,22 +1417,20 @@ merge_in_block (int max_reg, basic_block
>  
>/* If the inc insn was merged with a mem, the inc insn is gone
>and there is noting to update.  */
> -  if (DF_INSN_UID_GET (uid))
> +  if (insn_info)

This is wrong.  As the comment explains, the insn may already be
recycled, and you use a stale pointer.  Since insn_info is a local
variable, it cannot reflect that fact.

Andreas.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH, libgfortran] Add overflow check to xmalloc

[Bernhard Reutner-Fischer]

On 16 June 2014 09:39, Janne Blomqvist  wrote:
> On Mon, Jun 16, 2014 at 10:01 AM, Bernhard Reutner-Fischer
>  wrote:
>> On 16 June 2014 08:20:09 Janne Blomqvist  wrote:
>>
>>> On Sun, Jun 15, 2014 at 8:23 AM, Bernhard Reutner-Fischer
>>>  wrote:
>>> >
>>> >> >> On Tue, May 20, 2014 at 12:42 AM, Janne Blomqvist
>>> >> >>  wrote:
>>> >> >>> On Thu, May 15, 2014 at 1:00 AM, Janne Blomqvist
>>> >> >>>  wrote:
>>> >>  Hi,
>>> >> 
>>> >>  a common malloc() pattern is "malloc(num_foo * sizeof(foo_t)",
>>> >>  that
>>> >>  is, create space for an array of type foo_t with num_foo elements.
>>> >>  There is a slight danger here in that the multiplication can
>>> >>  overflow
>>> >>  and wrap around, and then the caller thinks it has a larger array
>>> >>  than
>>> >>  what malloc has actually created. The attached patch changes the
>>> >>  libgfortran xmalloc() function to have an API similar to calloc()
>>> >>  with
>>> >>  two arguments, and the implementation checks for wraparound.
>>> >> >>>
>>> >> >>> Hello,
>>> >> >>>
>>> >> >>> attached is an updated patch which instead introduces a new
>>> >> >>> function,
>>> >> >>> xmallocarray, with the overflow check, and leaves the existing
>>> >> >>> xmalloc
>>> >> >>> as is. Thus avoiding the extra checking in the common case where
>>> >> >>> one
>>> >> >>> of the arguments to xmallocarray would be 1.
>>> >> >>>
>>> >> >>> Tested on x86_64-unknown-linux-gnu, Ok for trunk?
>>> >> >>>
>>> >
>>> >
>>> > I would prefer if xcmalloc would not be named xmallocarray.
>>>
>>> Hmm, never heard of that one before, but I have no particular
>>
>>
>> Great, I fat-fingered it, meant xcalloc.
>
> Ah well, we already have xcalloc, which is a calloc() wrapper. The
> intention of the new function here is to be a malloc() wrapper, but
> with an overflow check. There is no need to zero the memory, hence
> calloc() is not appropriate.

I see. So i suppose your v2 patch is fine then.

cheers,

Re: breakage with "[PATCH 1/6] Add FOR_EACH_INSN{_INFO}_{DEFS,USES,EQ_USES}"

[Andreas Schwab]

Hans-Peter Nilsson  writes:

> On Sun, 15 Jun 2014, Hans-Peter Nilsson wrote:
>
>> On Sun, 15 Jun 2014, Hans-Peter Nilsson wrote:
>> > On Sun, 15 Jun 2014, Steven Bosscher wrote:
>> > > Can you please try:
>> > >
>> > > [...]
>> >
>> > Thanks.  Looks pretty obvious.  I was heading for the door with
>> > just enough time to report the issue, so I didn't actually look
>> > at the code before.  I'll commit this on your behalf once build
>> > has passed the point of failure.
>>
>> ...which includes not just compiling auto-inc-dec.c but also
>> compiling e.g. libgcc with the compiled compiler.  No such luck,
>> segv in that function (must be from Richard's code, not from the
>> patch as no dumps are output at that time).  Bah.
>
> Ok, I'm out; I don't know why the insn_info is invalid here.
> Hopefully obvious to you or Richard.  PR61516 for this.

Please try this:

diff --git a/gcc/auto-inc-dec.c b/gcc/auto-inc-dec.c
index 64a3706..d84e097 100644
--- a/gcc/auto-inc-dec.c
+++ b/gcc/auto-inc-dec.c
@@ -1341,6 +1341,7 @@ merge_in_block (int max_reg, basic_block bb)
 
   FOR_BB_INSNS_REVERSE_SAFE (bb, insn, curr)
 {
+  unsigned int uid = INSN_UID (insn);
   df_insn_info *insn_info = DF_INSN_INFO_GET (insn);
   bool insn_is_add_or_inc = true;
 
@@ -1417,7 +1418,7 @@ merge_in_block (int max_reg, basic_block bb)
 
   /* If the inc insn was merged with a mem, the inc insn is gone
 and there is noting to update.  */
-  if (insn_info)
+  if (DF_INSN_UID_GET (uid))
{
  df_ref def, use;
 

Andreas.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: Make ipa-ref somewhat less stupid

[Jan Hubicka]

> On 06/10/2014 08:34 AM, Jan Hubicka wrote:
> >Hi,
> >ipa-reference is somewhat stupid and builds its data sets for all variables 
> >including
> >addressable and public one just to prune them out after all bitmaps are 
> >constructed.
> >This used to make sense when the profile generation happened at compile 
> >time, but
> >since ipa_ref datastructure was intrdocued this is a nonsense.
> >
> >Martin: It may be interesting to check if this solves the memory use issues 
> >with
> >chrome.  We also may be able to re-enable ipa-ref with profile-generate as
> >I think all the datastructures are considered to have address taken.
> 
> Hi,
>there is a link to chromium stats: 
> https://drive.google.com/file/d/0B0pisUJ80pO1VmNHeklCRWVkOUU/edit?usp=sharing
> 
> Both compilation were run with '-flto=6', where the upper graph adds 
> '-fprofile-generate'. Memory footprint is IMHO acceptable, but compilation 
> process takes twice longer with profile generation. Yeah, chromium contains a 
> really big code base :)

Yep, I wonder why WPA takes so much longer. Do you think you can build lto1
with --enable-gather-detailed-mem-stats and relink with -fpre-ipa-mem-report
-fpost-ipa-mem-report -fmem-report -Q and send me the output?  It would be nice
to push Chromium under 4GB of WPA :)

Thanks a lot!
Honza

Re: [PATCH] Remove / move copy-prop passes

[Richard Biener]

On Fri, 13 Jun 2014, Richard Biener wrote:

> 
> The following leverages the "extra" work done by FRE/PRE now
> (propagating copies and constants) and removes the copy-prop
> pass run during early optimizations (no passes after it expose
> copy propagation opportunities).  It also moves the 3rd
> copy-prop pass from right after FRE (where it is now pointless)
> to right before ifcombine and phiopt as both benefit from
> copy-propagated IL (as we saw from the last phiopt bug).
> 
> I tested the copyprop removal from early opts with an
> assertion that we don't compute useful lattice values
> in the first pass instance (only for dead stmts).  For this
> the slight adjustment to the PRE/FRE elimination is necessary
> to also catch copy propagation opportunities from ASM outputs.
> 
> Bootstrap / regtest pending on x86_64-unknown-linux-gnu.

Ok, so this exposes some missing poor-mans DCE in the testsuite so
I went ahead and applied the following adjusted PRE adjustments only
sofar (I have a patch addressing the rest but will delay the actual
pass pipeline adjustments until later).

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

Richard.

2014-06-16  Richard Biener  

* tree-ssa-pre.c (eliminate_dom_walker::before_dom_children):
Make all defs available at the end.
(eliminate): If we remove a PHI node schedule cfg-cleanup.

Index: gcc/tree-ssa-pre.c
===
--- gcc/tree-ssa-pre.c  (revision 211694)
+++ gcc/tree-ssa-pre.c  (working copy)
@@ -4188,7 +4188,6 @@ eliminate_dom_walker::before_dom_childre
   b->loop_father->num);
}
  /* Don't keep sprime available.  */
- eliminate_push_avail (lhs);
  sprime = NULL_TREE;
}
}
@@ -4433,10 +4432,11 @@ eliminate_dom_walker::before_dom_childre
VN_INFO (vdef)->valnum = vuse;
}
 
-  /* Make the new value available - for fully redundant LHS we
- continue with the next stmt above.  */
-  if (lhs && TREE_CODE (lhs) == SSA_NAME)
-   eliminate_push_avail (lhs);
+  /* Make new values available - for fully redundant LHS we
+ continue with the next stmt above and skip this.  */
+  def_operand_p defp;
+  FOR_EACH_SSA_DEF_OPERAND (defp, stmt, iter, SSA_OP_DEF)
+   eliminate_push_avail (DEF_FROM_PTR (defp));
 }
 
   /* Replace destination PHI arguments.  */
@@ -4521,7 +4521,11 @@ eliminate (bool do_pre)
 
   gsi = gsi_for_stmt (stmt);
   if (gimple_code (stmt) == GIMPLE_PHI)
-   remove_phi_node (&gsi, true);
+   {
+ remove_phi_node (&gsi, true);
+ /* Removing a PHI node in a block may expose a forwarder block.  */
+ el_todo |= TODO_cleanup_cfg;
+   }
   else
{
  basic_block bb = gimple_bb (stmt);

Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)

[Jakub Jelinek]

On Wed, Jun 11, 2014 at 04:53:32PM +0400, Yury Gribov wrote:
> On 06/11/2014 01:31 PM, Jakub Jelinek wrote:
> >The plan (we had already for 4.9, but didn't get to that yet) is in the end
> >not to lower the checks in asan pass that much, and lower it in sanopt
> >pass later on after performing some inter-bb optimizations.
> >...
> >The reason for the plan is that it will be easier in the sanopt pass to
> >try to remove redundant instrumentation, e.g. when dominator bb already
> >checks a particular address/length and no calls that could change the
> >validity of the checks happen between the dominator bb and the dominated
> >__asan_load/storeX.
> 
> Sounds great, should help get rid of many useless checks.
> Do you guys need a hand with this?

That would be appreciated.

> Like this?
> 
> -Y

> 2014-06-11  Yury Gribov  
>   
>   New asan-instrumentation-with-call-threshold parameter.

Ok, thanks.

Jakub

Re: breakage with "[PATCH 1/6] Add FOR_EACH_INSN{_INFO}_{DEFS,USES,EQ_USES}"

[Ramana Radhakrishnan]

On Mon, Jun 16, 2014 at 9:04 AM, Andreas Schwab  wrote:
> Hans-Peter Nilsson  writes:
>
>> On Sun, 15 Jun 2014, Hans-Peter Nilsson wrote:
>>
>>> On Sun, 15 Jun 2014, Hans-Peter Nilsson wrote:
>>> > On Sun, 15 Jun 2014, Steven Bosscher wrote:
>>> > > Can you please try:
>>> > >
>>> > > [...]
>>> >
>>> > Thanks.  Looks pretty obvious.  I was heading for the door with
>>> > just enough time to report the issue, so I didn't actually look
>>> > at the code before.  I'll commit this on your behalf once build
>>> > has passed the point of failure.
>>>
>>> ...which includes not just compiling auto-inc-dec.c but also
>>> compiling e.g. libgcc with the compiled compiler.  No such luck,
>>> segv in that function (must be from Richard's code, not from the
>>> patch as no dumps are output at that time).  Bah.
>>
>> Ok, I'm out; I don't know why the insn_info is invalid here.
>> Hopefully obvious to you or Richard.  PR61516 for this.
>
> Please try this:
>
> diff --git a/gcc/auto-inc-dec.c b/gcc/auto-inc-dec.c
> index 64a3706..d84e097 100644
> --- a/gcc/auto-inc-dec.c
> +++ b/gcc/auto-inc-dec.c
> @@ -1341,6 +1341,7 @@ merge_in_block (int max_reg, basic_block bb)
>
>FOR_BB_INSNS_REVERSE_SAFE (bb, insn, curr)
>  {
> +  unsigned int uid = INSN_UID (insn);
>df_insn_info *insn_info = DF_INSN_INFO_GET (insn);
>bool insn_is_add_or_inc = true;
>
> @@ -1417,7 +1418,7 @@ merge_in_block (int max_reg, basic_block bb)
>
>/* If the inc insn was merged with a mem, the inc insn is gone
>  and there is noting to update.  */
> -  if (insn_info)
> +  if (DF_INSN_UID_GET (uid))
> {
>   df_ref def, use;

Doesn't appear to help on arm-none-linux-gnueabihf where I tried
applying a slight variant of this after the latest fixups for the
auto-inc-dec breakage.

I've gone through all the stages of breakage on this on my
arm-none-linux-gnueabihf auto-tester too.

regards
Ramana

>
>
> Andreas.
>
> --
> Andreas Schwab, SUSE Labs, sch...@suse.de
> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
> "And now for something completely different."

Re: breakage with "[PATCH 1/6] Add FOR_EACH_INSN{_INFO}_{DEFS,USES,EQ_USES}"

[Steven Bosscher]

On Mon, Jun 16, 2014 at 12:36 AM, Hans-Peter Nilsson wrote:
> On Sun, 15 Jun 2014, Steven Bosscher wrote:
>> On Sun, Jun 15, 2014 at 1:27 PM, Hans-Peter Nilsson wrote:
>> > /tmp/hpautotest-gcc0/gcc/gcc/auto-inc-dec.c: In function 'void
>> > merge_in_block(int, basic_block_def*)':
>> > /tmp/hpautotest-gcc0/gcc/gcc/auto-inc-dec.c:1442: error: 'uid'
>> > was not declared in this scope
>> > make[2]: *** [auto-inc-dec.o] Error 1
>> >
>> > brgds, H-P
>>
>>
>> Bah, this is why I just *hate* all the gcc code that's only compiled
>> if certain #defines are set...
>
> I couldn't agree more.  Might not have been obvious when writing
> the mosly-mechanical patch, still the auto-inc-dec.c name should
> have been a red flag that a representative target should have
> been tested (i.e. not x86_64 and i686).

I agree, but I think you'd agree with me if I say that Richard S. is
one of the few people who almost always goes beyond the normal amount
of testing required for a patch. Breakage like this will just happen
to us all, every once in a while, until we compile all middle-end code
at least, regardless of #defines and whatnot (conditionally compiled
code, from the top of my head: CC0, scheduler, dbrsched, auto-inc-dec,
HAVE_conditional_move, etc...).

Ciao!
Steven

Re: libsanitizer merge from upstream r208536

[Konstantin Serebryany]

On Wed, Jun 11, 2014 at 2:28 PM, Paolo Carlini  wrote:
> Hi,
>
> On 05/22/2014 09:02 PM, Jakub Jelinek wrote:
>>
>> In file included from
>> ../../../../trunk/libsanitizer/asan/asan_interceptors.cc:147:0:
>>
>> ../../../../trunk/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:
>> In function ‘int __interceptor_accept4(int, void*, unsigned int*,
>> int)’:
>>
>> ../../../../trunk/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1821:12:
>> warning: ‘addrlen0’ may be used uninitialized in this function
>> [-Wmaybe-uninitialized]
>> unsigned addrlen0;
>> ^
>>
>> ../../../../trunk/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:
>> In function ‘int __interceptor_accept(int, void*, unsigned int*)’:
>>
>> ../../../../trunk/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1799:12:
>> warning: ‘addrlen0’ may be used uninitialized in this function
>> [-Wmaybe-uninitialized]
>> unsigned addrlen0;
>> ^
>> That sounds like a false positive warning:
>>unsigned addrlen0;
>>if (addrlen) {
>>  COMMON_INTERCEPTOR_READ_RANGE(ctx, addrlen, sizeof(*addrlen));
>>  addrlen0 = *addrlen;
>>}
>>int fd2 = REAL(accept4)(fd, addr, addrlen, f);
>>if (fd2 >= 0) {
>>  if (fd >= 0) COMMON_INTERCEPTOR_FD_SOCKET_ACCEPT(ctx, fd, fd2);
>>  if (addr && addrlen)
>>COMMON_INTERCEPTOR_WRITE_RANGE(ctx, addr, Min(*addrlen, addrlen0));
>>}
>> (unless the COMMON_INTERCEPTOR* macros do too weird stuff), wonder why the
>> predicate aware uninit doesn't handle this.
>
> By the way, I'm still seeing the above. Maybe a maintainer can have a look,
> double check it's just a false positive a shut it up somehow?

I've "fixed" this in upstream trunk:
http://llvm.org/viewvc/llvm-project?view=revision&revision=211008
This will get into GCC with the next merge; or feel free to cherry pick.

Meanwhile, this still smells like a bug in GCC's -Wmaybe-uninitialized
-- I'd encourage you to at least file a bug on it.

--kcc

>
> Thanks!
> Paolo.

Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)

[Yury Gribov]

On 06/16/2014 12:23 PM, Jakub Jelinek wrote:

2014-06-11  Yury Gribov  

New asan-instrumentation-with-call-threshold parameter.


Ok, thanks.


Done in r211699.

Re: [PATCH] proposed fix for bug # 61144

[Jan Hubicka]

> >   /* Variables declared 'const' without an initializer
> >  have zero as the initializer if they may not be
> >  overridden at link or run time.  */
> >   if (!DECL_INITIAL (real_decl)
> >   && (DECL_EXTERNAL (decl) || decl_replaceable_p (decl)))
> > return error_mark_node;
> > 
> > Honza?
> 
> Indeed, this may be a better place to do it as long as
> decl_replaceable_p reliably returns true for weak aliases. If so, the
> following might work:
> 
>if ((!DECL_INITIAL (real_decl) && DECL_EXTERNAL (decl))
>|| decl_replaceable_p (decl)))
>  return error_mark_node;
> 
> On the other hand, I might just separate it out into two separate if
> statements since they should probably have their own comments.

Yep, this looks like correct change.  I used to have FIXME on this but
it seems it went away during some cleanups - the original condition was
comming from expmed's folding and indeed it looked unsafe to me.

This change is OK with the testcase (if it passes testing)

Honza
> 
> I would appreciate help from anyone familiar with GCC internals on
> getting this right.
> 
> Rich

Re: [PATCH] proposed fix for bug # 61144

[Jan Hubicka]

> 
> Are the attached files acceptable?

The testcase looks OK to me, but it already should be fixed on mainline
by patch https://gcc.gnu.org/ml/gcc-patches/2014-05/msg01315.html that
prevents dummy to be marked as constant. 

You can however modify the testcase to have
__attribute__ ((weak)) const int foo=0;

This needs your decl_replaceable change to not be optimized to if (0),
because of the explicit const modifier.

I did not change ctor_for_folding to reject variables above as I was not quite
sure we want to support this kind of interposition and I am still not quite 
certain.
C++ is quite clear about the transformation replacing initialized const by its 
value.

Honza

> 
> Rich

> /* { dg-do run } */
> /* { dg-require-weak "" } */
> /* { dg-require-alias "" } */
> /* { dg-additional-sources "wkali-3a.c" } */
> 
> #include 
> 
> static int dummy = 0;
> extern int foo __attribute__((__weak__, __alias__("dummy")));
> 
> int main(void) {
> 
> if (foo)
> exit(0);
> else
> abort();
> }

> /* { dg-do run } */
> 
> int foo = 1;

PR 61516: auto-inc-dec.c fallout from df changes

[Richard Sandiford]

I'd replaced the uid initialisation with an insn_info initialisation,
but it needed to be initialised at the point of insn_info test.

Tested against the testcase in the PR and applied as obvious.

Richard


gcc/
PR bootstrap/61516
* auto-inc-dec.c (merge_in_block): Fix location of insn_info
initialization.  Replace remaining use of uid.

Index: gcc/auto-inc-dec.c
===
--- gcc/auto-inc-dec.c  2014-06-16 08:37:09.767790766 +0100
+++ gcc/auto-inc-dec.c  2014-06-16 08:37:10.710805753 +0100
@@ -1341,7 +1341,6 @@ merge_in_block (int max_reg, basic_block
 
   FOR_BB_INSNS_REVERSE_SAFE (bb, insn, curr)
 {
-  df_insn_info *insn_info = DF_INSN_INFO_GET (insn);
   bool insn_is_add_or_inc = true;
 
   if (!NONDEBUG_INSN_P (insn))
@@ -1417,7 +1416,7 @@ merge_in_block (int max_reg, basic_block
 
   /* If the inc insn was merged with a mem, the inc insn is gone
 and there is noting to update.  */
-  if (insn_info)
+  if (df_insn_info *insn_info = DF_INSN_INFO_GET (insn))
{
  df_ref def, use;
 
@@ -1439,7 +1438,8 @@ merge_in_block (int max_reg, basic_block
}
}
   else if (dump_file)
-   fprintf (dump_file, "skipping update of deleted insn %d\n", uid);
+   fprintf (dump_file, "skipping update of deleted insn %d\n",
+INSN_UID (insn));
 }
 
   /* If we were successful, try again.  There may have been several

Re: Fix vectorizer conditions on updating alignment

[Jan Hubicka]

> On Fri, Jun 13, 2014 at 12:14 AM, Jan Hubicka  wrote:
> > Hi,
> > while updating vect_can_force_dr_alignment_p for section API I noticed the
> > predicate is bit confused about when it can update the alignment.
> >
> > We need to check that decl_binds_to_current_def_p and in case we compile
> > a partition also that the symbol is not homed in other partition.
> > Previous code was wrong i.e. for COMDATs, weaks or -fpic.
> >
> > Also when having an alias, only way to promote the alignment is to bump
> > up alignment of target.
> >
> > On the other hand comment about DECL_IN_CONSTANT_POOL seems confused - we 
> > have
> > no sharing across partitions. I assume it was old hack and removed it.
> 
> I don't think that code was confused.  It's because of the way we emit
> the constant pool and use its hash (we duplicate the decl).  Do a svn
> blame and see for the associated PR which you now broke again
> I guess.  It wasn't about LTO I think.

It is middle-end/50494.

I have re-instantiated the check and also added back TREE_ASM_WRITTEN that is 
needed
in the case of -fno-toplevel-reorder.  I will try to look into the constant 
pool output
machinery but indeed it is a nasty problem.

Thanks,
Honza

Index: ChangeLog
===
--- ChangeLog   (revision 211689)
+++ ChangeLog   (working copy)
@@ -1,5 +1,10 @@
 2014-06-15  Jan Hubicka  
 
+   * tree-vect-data-refs.c (vect_can_force_dr_alignment_p): Check again
+   DECL_IN_CONSTANT_POOL and TREE_ASM_WRITTEN.
+
+2014-06-15  Jan Hubicka  
+
* c-family/c-common.c (handle_tls_model_attribute): Use 
set_decl_tls_model.
* cgraph.h (struct varpool_node): Add tls_model.
* tree.c (decl_tls_model, set_decl_tls_model): New functions.
Index: tree-vect-data-refs.c
===
--- tree-vect-data-refs.c   (revision 211688)
+++ tree-vect-data-refs.c   (working copy)
@@ -5317,7 +5317,13 @@ vect_can_force_dr_alignment_p (const_tre
   if (TREE_CODE (decl) != VAR_DECL)
 return false;
 
-  gcc_assert (!TREE_ASM_WRITTEN (decl));
+  /* With -fno-toplevel-reorder we may have already output the constant.  */
+  if (TREE_ASM_WRITTEN (decl))
+return false;
+
+  /* Constant pool entries may be shared and not properly merged by LTO.  */
+  if (DECL_IN_CONSTANT_POOL (decl))
+return false;
 
   if (TREE_PUBLIC (decl) || DECL_EXTERNAL (decl))
 {

Re: Patch improving spilling general regs into vector regs

[Christophe Lyon]

Hi,

This patches causes a failure to build GCC (since commit 211655), on
all ARM and Aarch64 targets I track.

I can see failures when building libgcc (_mulsc3.o, _muldc3.o,
_divdc3.o), the error message being:
0xa07f6f crash_signal
/tmp/214822_1.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/toplev.c:337
Please submit a full bug report,

Christophe.


On 13 June 2014 22:33, Vladimir Makarov  wrote:
>   Hi, the following patch improves LRA spilling general reg pseduos
> into vector regs.
>
>   Before the patch only *regular* spilled pseudos of general regs
> class are assigned to vector regs on the LRA spill sub-pass.  The
> patch improves this by assigning vector regs to *inheritance* pseudos
> on LRA assign sub-pass (it should be done here as if an inheritance
> pseudo does not get a hard reg on the sub-pass, it is removed right
> after the sub-pass).
>
>   As the result on a big benchmark (about 500K lines of fortran code),
> LRA assigns about 25% more spilled pseudos of general regs class to
> vector regs (6123 vs. 4827) in 64-bit mode and about 85% in 32-bit
> mode (22691 vs 12171).
>
>   The patch also fixes a bug in spilling pseudos into vector regs
> which might result in LRA cycling.  The fix is in i386.c which wrongly
> returned SSE_REGS class for NO_REGS pseudos.  Pseudos denoting
> secondary memory have such class and they might get vector regs and it
> results in secondary memory transformation again and again.  The fix
> is pretty obvious so I am checking it in without an approval.
>
>   The patch was bootstrapped and tested on x86-64 with switching on
> spilling general reg pseudos into vector regs.
>
>
>   Committed as rev. 211655.
>
> 2014-06-13  Vladimir Makarov  
>
> * lra-assign.c (assign_by_spills): Add code to assign vector regs
> to inheritance pseudos.
> * config/i386/i386.c (ix86_spill_class): Add check on NO_REGS.

Re: libsanitizer merge from upstream r208536

[Paolo Carlini]

Hi,

On 16/06/14 10:42, Konstantin Serebryany wrote:
I've "fixed" this in upstream trunk: 
http://llvm.org/viewvc/llvm-project?view=revision&revision=211008 This 
will get into GCC with the next merge; or feel free to cherry pick.

Excellent thanks a lot.
Meanwhile, this still smells like a bug in GCC's -Wmaybe-uninitialized 
-- I'd encourage you to at least file a bug on it.
Yes, but we should first double check the issue isn't already known and, 
otherwise, figure out a minimized testcase.


Thanks,
Paolo.

Re: [PATCH][RFC] Make FRE/PRE apply copy/constant propagation

[Richard Biener]

On Sat, 14 Jun 2014, Eric Botcazou wrote:

> > 2014-06-13  Richard Biener  
> > 
> > * tree-ssa-pre.c (eliminate_dom_walker::before_dom_children):
> > Rewrite to propagate the VN result into all uses where
> > possible and to remove stmts becoming dead because of that.
> > (eliminate): Generalize stmt removal handling, remove in
> > reverse dominator order to support proper debug stmt
> > generation.  Update stmts before removing stmts.
> > * tree-ssa-propagate.c (propagate_tree_value): Remove
> > bogus assert.
> 
> This breaks Ada bootstrap with non-standard build options (-gnatpgn).  I have 
> attached a reduced testcase, but you need the Ada SJLJ scheme to see it so 
> the 
> following procedure can be used:
> 
>  1. Put p.ad[bs] in $(BUILDDIR)
>  2. Do "gcc/gnat1 -quiet p.adb -I $(SRCDIR)/gcc/ada -I gcc/ada -O2 -gnatn"
> 
> eric@polaris:~/build/gcc/native> gcc/gnat1 -quiet p.adb -I ~/svn/gcc/gcc/ada -
> I gcc/ada -O2 -gnatn
> +===GNAT BUG DETECTED==+
> | 4.10.0 20140614 (experimental) [trunk revision 211664] (x86_64-suse-linux) 
> GCC error:|
> | in forward_edge_to_pdom, at tree-ssa-dce.c:1042  |
> | Error detected around /home/eric/svn/gcc/gcc/ada/sinfo.adb:6010:8|

Seems to be an issue with the assert

1041  /* The resulting PHI if not dead can only be degenerate.  
*/
1042  gcc_assert (degenerate_phi_p (phi));

and the fact that the PHI is abnormal and thus while it really _is_
degenerate value-wise, we can't easily prove it (by that predicate).

Not sure what to do here but adjust it to

  gcc_assert (degenerate_phi_p (phi) || SSA_NAME_OCCURS_IN_ABNORMAL_PHI 
(gimple_phi_result (phi)));

So - can you check if bootstrap works with that?

Thanks,
Richard.

Re: Ping [PATCH, fortran] PR 41936 Memory leakage with allocatables and user-defined operators

[Dominique Dhumieres]

Dear Paul,

Late thanks for the review, the patch has been committed to trunk at r211405.

> OK for trunk and, I would suggest 4.9.

I am now trying to do the back port to 4.9 and I got two failures:

FAIL: gfortran.dg/alloc_comp_basics_1.f90  -O*   scan-tree-dump-times original 
"builtin_free" 18
FAIL: gfortran.dg/alloc_comp_constructor_1.f90  -O*   scan-tree-dump-times 
original "builtin_free" 19

In both cases the number of "builtin_free" is 21. I have run the executables 
for the patched/unpatched
4.9 through valgrind without error in both cases.

I can indeed make the tests pass by updating the counst in 
scan-tree-dump-times, but
I'ld like some advice before committing that.

TIA

Dominique

Re: [ARM] Fix build failure due to movsi_compare0 (PR 61430)

[James Greenhalgh]

On Fri, Jun 13, 2014 at 05:46:45PM +0100, Vladimir Makarov wrote:
> On 2014-06-11, 1:17 PM, Chung-Lin Tang wrote:
> > Looking at this too, as an LRA exercise. I don't really think the
> > pattern is wrong, rather LRA should just avoid creating the copy in this
> > case; it's a result of operand constraining, after all.
> > 
> > Attached is the small LRA patch, pending testing. Vladimir should weight
> > in on this.
> > 
> 
> The patch is safe and ok.  Thanks for working on it, Chung-Lin.

As this patch fixes a build failure on ARM I'd like to have it applied
today. If I don't hear anything which would stop me, I'll commit this on
Chung-Lin's behalf in a few hours.

Cheers,
James

> 
> > * ira-lives.c (process_bb_lives): Skip creating copy during
> > insn sca when src/dest has constrained to same regno.
> > 
> 
> 

RE: [PATCH] Fix PR ipa/61190

[Bernd Edlinger]

Hi Honza,

On Mon, 2 Jun 2014 18:12:10, Jan Hubicka wrote:
>
>> Hi,
>>
>> On Mon, 2 Jun 2014 12:06:12, Richard Biener wrote:
>>>
>>> On Mon, Jun 2, 2014 at 11:00 AM, Bernd Edlinger
>>>  wrote:
 Hi,

 the test case g++.old-deja/g++.mike/p4736b.C is mis-compiled with with all
 optimization levels, except -O0 and -Og. This probably started with gcc 
 4.7.x.

 The constructor Main::Main() is first inlined and then completely optimized
 away in the dce1 pass. But the thunk is still using the vtable, and 
 therefore
 crashes.
>
> Why the ctor is eliminated? Is it because ipa-pure-const identifies the thunk 
> as const?
> I think we need to update it to notice the read of vtable in those thunks. I 
> will
> take a look.
>

have you been able to find an alternative solution yet?


Thanks
Bernd.

> Honza

 Well, the problem seems to be, that the thunk code is
 not emitted in the normal way using gimple code,
 but instead by the i386 back-end with a callback.

 This makes the thunk invisible to the ipa-pure-const pass,
 which assumes that all values just flow straight thu the thunk.

 See ipa-pure-const.c (analyze_function):

 if (fn->thunk.thunk_p || fn->alias)
 {
 /* Thunk gets propagated through, so nothing interesting happens. */
 gcc_assert (ipa);
 return l;
 }

 But this is not true for a virtual base class:
 The thunk itself references the vtable, so if nothing else might need
 the vtable, the optimizer may remove the initalization of the vtable,
 which happened in this example.

 The most simple work around would be the attached patch, which
 simply falls back to emitting gimple code, if virtual base classes
 are used.

 Boot-Strapped and Regression-Tested on x86_64-linux-gnu.
 Ok for trunk?
>>>
>>> Honza should know better. I'd rather skip the above for
>>> thunks going the output_mi_thunk way.
>>>
>>
>> Ahh Yes, that was of course my first attempt...
>>
>> But there is not BB to enumerate in that case.
>> And the loop below just crashed in:
>>
>>   FOR_EACH_BB_FN (this_block, cfun)
>>
>>
>> There is also another interesting thing to mention: when the
>> output_mi_thunk outputs the virtual thunk, there is no inlining at all.
>>
>> But with this patch, the thunk inlines the called function,
>> and therefore the generated code looks rather nifty, compared to
>> the state before the patch.
>>
>>
>> Thanks
>> Bernd.
>>
>>> That is, the target hook docs should be updated to reflect which
>>> kind of thunks it is supposed to handle.
>>>
>>> Richard.
>>>

 Thanks
 Bernd.

>>
  

Re: [ARM] Fix build failure due to movsi_compare0 (PR 61430)

[Chung-Lin Tang]

On 14/6/16 5:55 PM, James Greenhalgh wrote:
> On Fri, Jun 13, 2014 at 05:46:45PM +0100, Vladimir Makarov wrote:
>> On 2014-06-11, 1:17 PM, Chung-Lin Tang wrote:
>>> Looking at this too, as an LRA exercise. I don't really think the
>>> pattern is wrong, rather LRA should just avoid creating the copy in this
>>> case; it's a result of operand constraining, after all.
>>>
>>> Attached is the small LRA patch, pending testing. Vladimir should weight
>>> in on this.
>>>
>>
>> The patch is safe and ok.  Thanks for working on it, Chung-Lin.
> 
> As this patch fixes a build failure on ARM I'd like to have it applied
> today. If I don't hear anything which would stop me, I'll commit this on
> Chung-Lin's behalf in a few hours.
> 
> Cheers,
> James

I just committed it, after a testsuite run on x86_64 (to be sure). And
thanks to James for doing the ARM tests.

Thanks,
Chung-Lin

>>
>>> * ira-lives.c (process_bb_lives): Skip creating copy during
>>> insn sca when src/dest has constrained to same regno.
>>>
>>
>>

[PATCH 2/5] Existing call graph infrastructure enhancement

[mliska]

Hi,
this small patch prepares remaining needed infrastructure for the new pass.

Changelog:

2014-06-13  Martin Liska  
Honza Hubicka  

* ipa-utils.h (polymorphic_type_binfo_p): Function marked external
instead of static.
* ipa-devirt.c (polymorphic_type_binfo_p): Likewise.
* ipa-prop.h (count_formal_params): Likewise.
* ipa-prop.c (count_formal_params): Likewise.
* ipa-utils.c (ipa_merge_profiles): Be more tolerant if we merge
profiles for semantically equivalent functions.
* passes.c (do_per_function): If we load body of a function during WPA,
this condition should behave same.
* varpool.c (ctor_for_folding): More tolerant assert for variable
aliases created during WPA.

diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c
index d733461..18592d7 100644
--- a/gcc/ipa-devirt.c
+++ b/gcc/ipa-devirt.c
@@ -176,7 +176,7 @@ struct GTY(()) odr_type_d
inheritance (because vtables are shared).  Look up the BINFO of type
and check presence of its vtable.  */
 
-static inline bool
+bool
 polymorphic_type_binfo_p (tree binfo)
 {
   /* See if BINFO's type has an virtual table associtated with it.  */
diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index b67deed..60bda71 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -210,7 +210,7 @@ ipa_populate_param_decls (struct cgraph_node *node,
 
 /* Return how many formal parameters FNDECL has.  */
 
-static inline int
+int
 count_formal_params (tree fndecl)
 {
   tree parm;
diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h
index cb23698..87573ff 100644
--- a/gcc/ipa-prop.h
+++ b/gcc/ipa-prop.h
@@ -529,6 +529,7 @@ void ipa_free_all_edge_args (void);
 void ipa_free_all_structures_after_ipa_cp (void);
 void ipa_free_all_structures_after_iinln (void);
 void ipa_register_cgraph_hooks (void);
+int count_formal_params (tree fndecl);
 
 /* This function ensures the array of node param infos is big enough to
accommodate a structure for all nodes and reallocates it if not.  */
diff --git a/gcc/ipa-utils.c b/gcc/ipa-utils.c
index 8e7c7cb..bc2b958 100644
--- a/gcc/ipa-utils.c
+++ b/gcc/ipa-utils.c
@@ -660,13 +660,8 @@ ipa_merge_profiles (struct cgraph_node *dst,
   if (dst->tp_first_run > src->tp_first_run && src->tp_first_run)
 dst->tp_first_run = src->tp_first_run;
 
-  if (src->profile_id)
-{
-  if (!dst->profile_id)
-   dst->profile_id = src->profile_id;
-  else
-   gcc_assert (src->profile_id == dst->profile_id);
-}
+  if (src->profile_id && !dst->profile_id)
+dst->profile_id = src->profile_id;
 
   if (!dst->count)
 return;
diff --git a/gcc/ipa-utils.h b/gcc/ipa-utils.h
index a2c985a..996249a 100644
--- a/gcc/ipa-utils.h
+++ b/gcc/ipa-utils.h
@@ -72,6 +72,8 @@ struct odr_type_d;
 typedef odr_type_d *odr_type;
 void build_type_inheritance_graph (void);
 void update_type_inheritance_graph (void);
+bool polymorphic_type_binfo_p (tree binfo);
+
 vec 
 possible_polymorphic_call_targets (tree, HOST_WIDE_INT,
   ipa_polymorphic_call_context,
diff --git a/gcc/passes.c b/gcc/passes.c
index 4366251..9fdfe51 100644
--- a/gcc/passes.c
+++ b/gcc/passes.c
@@ -1506,7 +1506,7 @@ do_per_function (void (*callback) (function *, void 
*data), void *data)
 {
   struct cgraph_node *node;
   FOR_EACH_DEFINED_FUNCTION (node)
-   if (node->analyzed && gimple_has_body_p (node->decl)
+   if (node->analyzed && (gimple_has_body_p (node->decl) && !in_lto_p)
&& (!node->clone_of || node->decl != node->clone_of->decl))
  callback (DECL_STRUCT_FUNCTION (node->decl), data);
 }
diff --git a/gcc/varpool.c b/gcc/varpool.c
index ff67127..5cc558e 100644
--- a/gcc/varpool.c
+++ b/gcc/varpool.c
@@ -293,6 +293,7 @@ ctor_for_folding (tree decl)
   if (decl != real_decl)
 {
   gcc_assert (!DECL_INITIAL (decl)
+ || (node->alias && varpool_alias_target (node) == real_node)
  || DECL_INITIAL (decl) == error_mark_node);
   if (lookup_attribute ("weakref", DECL_ATTRIBUTES (decl)))
{
-- 
1.8.4.5

[PATCH 4/5] Existing tests fix

[mliska]

Hi,
  many tests rely on a precise number of scanned functions in a dump file. If 
IPA ICF decides to merge some function and(or) read-only variables, counts do 
not match.

Martin

Changelog:

2014-06-13  Martin Liska  
Honza Hubicka  

* c-c++-common/rotate-1.c: Text
* c-c++-common/rotate-2.c: New test.
* c-c++-common/rotate-3.c: Likewise.
* c-c++-common/rotate-4.c: Likewise.
* g++.dg/cpp0x/rv-return.C: Likewise.
* g++.dg/cpp0x/rv1n.C: Likewise.
* g++.dg/cpp0x/rv1p.C: Likewise.
* g++.dg/cpp0x/rv2n.C: Likewise.
* g++.dg/cpp0x/rv3n.C: Likewise.
* g++.dg/cpp0x/rv4n.C: Likewise.
* g++.dg/cpp0x/rv5n.C: Likewise.
* g++.dg/cpp0x/rv6n.C: Likewise.
* g++.dg/cpp0x/rv7n.C: Likewise.
* gcc.dg/ipa/ipacost-1.c: Likewise.
* gcc.dg/ipa/ipacost-2.c: Likewise.
* gcc.dg/ipa/ipcp-agg-6.c: Likewise.
* gcc.dg/ipa/remref-2a.c: Likewise.
* gcc.dg/ipa/remref-2b.c: Likewise.
* gcc.dg/pr46309-2.c: Likewise.
* gcc.dg/torture/ipa-pta-1.c: Likewise.
* gcc.dg/tree-ssa/andor-3.c: Likewise.
* gcc.dg/tree-ssa/andor-4.c: Likewise.
* gcc.dg/tree-ssa/andor-5.c: Likewise.
* gcc.dg/vect/no-vfa-pr29145.c: Likewise.
* gcc.dg/vect/vect-cond-10.c: Likewise.
* gcc.dg/vect/vect-cond-9.c: Likewise.
* gcc.dg/vect/vect-widen-mult-const-s16.c: Likewise.
* gcc.dg/vect/vect-widen-mult-const-u16.c: Likewise.
* gcc.dg/vect/vect-widen-mult-half-u8.c: Likewise.
* gcc.target/i386/bmi-1.c: Likewise.
* gcc.target/i386/bmi-2.c: Likewise.
* gcc.target/i386/pr56564-2.c: Likewise.
* g++.dg/opt/pr30965.C: Likewise.
* g++.dg/tree-ssa/pr19637.C: Likewise.
* gcc.dg/guality/csttest.c: Likewise.
* gcc.dg/ipa/iinline-4.c: Likewise.
* gcc.dg/ipa/iinline-7.c: Likewise.
* gcc.dg/ipa/ipa-pta-13.c: Likewise.

diff --git a/gcc/testsuite/c-c++-common/rotate-1.c 
b/gcc/testsuite/c-c++-common/rotate-1.c
index afdaa28..bca9dd8 100644
--- a/gcc/testsuite/c-c++-common/rotate-1.c
+++ b/gcc/testsuite/c-c++-common/rotate-1.c
@@ -1,6 +1,6 @@
 /* Check rotate pattern detection.  */
 /* { dg-do compile } */
-/* { dg-options "-O2 -fdump-tree-optimized" } */
+/* { dg-options "-O2 -fno-ipa-icf -fdump-tree-optimized" } */
 /* { dg-final { scan-tree-dump-times "r\[<>]\[<>]" 96 "optimized" } } */
 /* { dg-final { cleanup-tree-dump "optimized" } } */
 
diff --git a/gcc/testsuite/c-c++-common/rotate-2.c 
b/gcc/testsuite/c-c++-common/rotate-2.c
index 109fd32..4ffa218 100644
--- a/gcc/testsuite/c-c++-common/rotate-2.c
+++ b/gcc/testsuite/c-c++-common/rotate-2.c
@@ -1,6 +1,6 @@
 /* Check rotate pattern detection.  */
 /* { dg-do compile } */
-/* { dg-options "-O2 -fdump-tree-optimized" } */
+/* { dg-options "-O2 -fno-ipa-icf -fdump-tree-optimized" } */
 /* Rotates should be recognized only in functions with | instead of + or ^,
or in functions that have constant shift counts (unused attribute on y).  */
 /* { dg-final { scan-tree-dump-times "r\[<>]\[<>]" 48 "optimized" } } */
diff --git a/gcc/testsuite/c-c++-common/rotate-3.c 
b/gcc/testsuite/c-c++-common/rotate-3.c
index 8dc8313..aaa9f50 100644
--- a/gcc/testsuite/c-c++-common/rotate-3.c
+++ b/gcc/testsuite/c-c++-common/rotate-3.c
@@ -1,6 +1,6 @@
 /* Check rotate pattern detection.  */
 /* { dg-do compile } */
-/* { dg-options "-O2 -fdump-tree-optimized" } */
+/* { dg-options "-O2 -fno-ipa-icf -fdump-tree-optimized" } */
 /* { dg-final { scan-tree-dump-times "r\[<>]\[<>]" 96 "optimized" } } */
 /* { dg-final { cleanup-tree-dump "optimized" } } */
 
diff --git a/gcc/testsuite/c-c++-common/rotate-4.c 
b/gcc/testsuite/c-c++-common/rotate-4.c
index 2f433b3..0a21177 100644
--- a/gcc/testsuite/c-c++-common/rotate-4.c
+++ b/gcc/testsuite/c-c++-common/rotate-4.c
@@ -1,6 +1,6 @@
 /* Check rotate pattern detection.  */
 /* { dg-do compile } */
-/* { dg-options "-O2 -fdump-tree-optimized" } */
+/* { dg-options "-O2 -fno-ipa-icf -fdump-tree-optimized" } */
 /* Rotates should be recognized only in functions with | instead of + or ^,
or in functions that have constant shift counts (unused attribute on y).  */
 /* { dg-final { scan-tree-dump-times "r\[<>]\[<>]" 48 "optimized" } } */
diff --git a/gcc/testsuite/g++.dg/cpp0x/rv-return.C 
b/gcc/testsuite/g++.dg/cpp0x/rv-return.C
index 12a15aa..6d57209 100644
--- a/gcc/testsuite/g++.dg/cpp0x/rv-return.C
+++ b/gcc/testsuite/g++.dg/cpp0x/rv-return.C
@@ -1,5 +1,6 @@
 // PR c++/41815
 // { dg-do compile { target c++11 } }
+// { dg-options "-fno-ipa-icf" }
 
 template struct same_type;
 template struct same_type {};
diff --git a/gcc/testsuite/g++.dg/cpp0x/rv1n.C 
b/gcc/testsuite/g++.dg/cpp0x/rv1n.C
index 204ca31..f5e7568 100644
--- a/gcc/testsuite/g++.dg/cpp0x/rv1n.C
+++ b/gcc/testsuite/g++.dg/cpp0x/rv1n.C
@@ -1,8 +1,10 @@
 // I, Howard Hinnant, hereby place this code in the public domain.
+/* { dg-additio

[PATCH 5/5] New tests introduction

[mliska]

Hi,
   this is a new collection of tests for IPA ICF pass.

Martin

Changelog:

2014-06-13  Martin Liska  
Honza Hubicka  

* gcc/testsuite/g++.dg/ipa/ipa-se-1.C: New test.
* gcc/testsuite/g++.dg/ipa/ipa-se-2.C: Likewise.
* gcc/testsuite/g++.dg/ipa/ipa-se-3.C: Likewise.
* gcc/testsuite/g++.dg/ipa/ipa-se-4.C: Likewise.
* gcc/testsuite/g++.dg/ipa/ipa-se-5.C: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-1.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-10.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-11.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-12.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-13.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-14.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-15.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-16.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-17.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-18.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-19.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-2.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-20.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-21.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-22.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-23.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-24.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-25.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-26.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-27.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-28.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-3.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-4.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-5.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-6.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-7.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-8.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-9.c: Likewise.

diff --git a/gcc/testsuite/g++.dg/ipa/ipa-se-1.C 
b/gcc/testsuite/g++.dg/ipa/ipa-se-1.C
new file mode 100644
index 000..d27abf4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/ipa-se-1.C
@@ -0,0 +1,43 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-ipa-icf"  } */
+
+class A
+{
+public:
+  __attribute__ ((noinline))
+  virtual int Foo2()
+  {
+return v;
+  }
+
+  float f;
+  int v;
+};
+
+class B
+{
+public:
+  __attribute__ ((noinline))
+  int Bar2()
+  {
+return v;
+  }
+
+  float f, aaa;
+  int v;
+};
+
+int main()
+{
+  A a;
+  B b;
+
+  a.Foo2();
+  b.Bar2();
+
+  return 12345;
+}
+
+/* { dg-final { scan-ipa-dump-not "Semantic equality hit:" "icf"  } } */
+/* { dg-final { scan-ipa-dump "Equal symbols: 0" "icf"  } } */
+/* { dg-final { cleanup-ipa-dump "icf" } } */
diff --git a/gcc/testsuite/g++.dg/ipa/ipa-se-2.C 
b/gcc/testsuite/g++.dg/ipa/ipa-se-2.C
new file mode 100644
index 000..48badd7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/ipa-se-2.C
@@ -0,0 +1,40 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-ipa-icf"  } */
+
+class A
+{
+public:
+  __attribute__ ((noinline))
+  int Foo2()
+  {
+return 1;
+  }
+
+  int v;
+  float f;
+};
+
+class B
+{
+public:
+  __attribute__ ((noinline))
+  int Bar2()
+  {
+return 1;
+  }
+
+  int v;
+  float f, aaa;
+};
+
+int main()
+{
+  A a;
+  B b;
+
+  return a.Foo2() + b.Bar2();
+}
+
+/* { dg-final { scan-ipa-dump "Semantic equality hit:" "icf"  } } */
+/* { dg-final { scan-ipa-dump "Equal symbols: 1" "icf"  } } */
+/* { dg-final { cleanup-ipa-dump "icf" } } */
diff --git a/gcc/testsuite/g++.dg/ipa/ipa-se-3.C 
b/gcc/testsuite/g++.dg/ipa/ipa-se-3.C
new file mode 100644
index 000..042f789
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/ipa-se-3.C
@@ -0,0 +1,36 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-ipa-icf"  } */
+
+__attribute__ ((noinline))
+int zero()
+{
+  return 0;
+}
+
+__attribute__ ((noinline))
+int nula()
+{
+  return 0;
+}
+
+__attribute__ ((noinline))
+int foo()
+{
+  return zero();
+}
+
+__attribute__ ((noinline))
+int bar()
+{
+  return nula();
+}
+
+int main()
+{
+  return foo() + bar();
+}
+
+/* { dg-final { scan-ipa-dump "Semantic equality hit:.*bar.*->.*foo.*" "icf"  
} } */
+/* { dg-final { scan-ipa-dump "Semantic equality hit:.*nula.*->.*zero.*" "icf" 
 } } */
+/* { dg-final { scan-ipa-dump "Equal symbols: 2" "icf"  } } */
+/* { dg-final { cleanup-ipa-dump "icf" } } */
diff --git a/gcc/testsuite/g++.dg/ipa/ipa-se-4.C 
b/gcc/testsuite/g++.dg/ipa/ipa-se-4.C
new file mode 100644
index 000..4ac2cf9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/ipa-se-4.C
@@ -0,0 +1,49 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-ipa-icf -fno-inline" } */
+
+namespace {
+struct A
+{
+  virtual void foo(void) {}
+};
+struct B: virtual A
+{
+  virtual void foo(void) {}
+};
+struct C: virtual A
+{
+  virtual void bar(void) {}
+};
+struct D: virtual A
+{
+  virtual void sparta(void) {}
+};
+struct E: B,C

[PATCH 1/5] New Identical Code Folding IPA pass

[mliska]

Hello,
  I am sorry for wrong reply address in previously sent thread. After working 
for quite a long time, I would like to introduce new IPA pass. Goal of the pass 
is to merge semantically equivalent functions and read-only variables.
  If we prove that a function A is an equivalent to a function B, depending on 
circumstances, an alias, thunk or a function redirection is created.

  The optimization is inspired by Microsoft /OPT:ICF optimization 
(http://msdn.microsoft.com/en-us/library/bxwfs976.aspx) that merges COMDAT 
sections with each function reside in a separate section.
  Apart from that, GOLD linker implemented equivalent implementation for GNU 
toolchain: SAFE ICF (http://research.google.com/pubs/pub36912.html). Both 
implementations suffer from a collection of functions where an address was 
taken for comparison purpose. GOLD linker adds more conservative --icf=safe 
option.

  You may ask, why the GNU GCC does need such a new optimization. The compiler, 
having simply better knowledge of a compiled source file, is capable of 
reaching better results, especially if Link-Time optimization is enabled. Apart 
from that, GCC implementation adds support for read-only variables like 
construction vtables (mentioned in: 
http://hubicka.blogspot.cz/2014/02/devirtualization-in-c-part-3-building.html).

  The pass is capable of building GIMP, Inkscape and Firefox with enabled LTO. 
Majority of issues connected to testsuite has been fixed, there is a few of IPA 
tests I need to consult with Jan, how to fix it correctly.

  Statistics about the pass:
  Inkscape: 11.95 MB -> 11.44 MB (-4.27%)
  Firefox: 70.12 MB -> 70.12 MB (-3.07%)

  SPEC 2K6 statistics show how many equivalent functions (and variables) are 
proved by GOLD and GCC ICF:
  
++--+-++--+--+--+-+
  | SPEC   | GOLD ICF | IPA ICF | difference | difference perc. | 
intersection | Just in GOLD | Just in ICF |
  
++--+-++--+--+--+-+
  | 400.perlbench  |   26 |  53 | 27 |  203.85% |   
26 |0 |  27 |
  | 401.bzip2  |0 |   1 |  1 |0.00% |   
 0 |0 |   1 |
  | 403.gcc|   88 | 223 |135 |  253.41% |   
81 |7 | 142 |
  | 410.bwaves |0 |   0 |  0 |0.00% |   
 0 |0 |   0 |
  | 416.gamess |9 |  23 | 14 |  255.56% |   
 9 |0 |  14 |
  | 429.mcf|0 |   0 |  0 |0.00% |   
 0 |0 |   0 |
  | 433.milc   |0 |   8 |  8 |0.00% |   
 0 |0 |   8 |
  | 434.zeusmp |0 |   1 |  1 |0.00% |   
 0 |0 |   1 |
  | 436.cactusADM  |   19 |  55 | 36 |  289.47% |   
14 |5 |  41 |
  | 437.leslie3d   |0 |   0 |  0 |0.00% |   
 0 |0 |   0 |
  | 444.namd   |0 |   0 |  0 |0.00% |   
 0 |0 |   0 |
  | 445.gobmk  |0 | 332 |332 |0.00% |   
 0 |0 | 332 |
  | 453.povray |   73 |  93 | 20 |  127.40% |   
60 |   13 |  33 |
  | 454.calculix   |3 |   6 |  3 |  200.00% |   
 3 |0 |   3 |
  | 456.hmmer  |0 |   1 |  1 |0.00% |   
 0 |0 |   1 |
  | 458.sjeng  |0 |   1 |  1 |0.00% |   
 0 |0 |   1 |
  | 459.GemsFDTD   |0 |   0 |  0 |0.00% |   
 0 |0 |   0 |
  | 462.libquantum |0 |   0 |  0 |0.00% |   
 0 |0 |   0 |
  | 464.h264ref|0 |   4 |  4 |0.00% |   
 0 |0 |   4 |
  | 470.lbm|0 |   0 |  0 |0.00% |   
 0 |0 |   0 |
  | 471.omnetpp|  134 | 179 | 45 |  133.58% |   
   127 |7 |  52 |
  | 473.astar  |0 |   1 |  1 |0.00% |   
 0 |0 |   1 |
  | 481.wrf|   51 |  75 | 24 |  147.06% |   
50 |1 |  25 |
  | 482.sphinx3|0 |   1 |  1 |0.00% |   
 0 |0 |   1 |
  
+

Re: [PATCH][genattrtab] Fix memory corruption, allocate enough memory for all bypassed reservations

[Kyrill Tkachov]

On 13/06/14 16:51, Jeff Law wrote:

On 06/13/14 03:56, Kyrill Tkachov wrote:

Hi all,

I noticed a memory corruption bug while adding some scheduler bypasses
in the arm backend.
genattrtab would segfault while processing the bypasses. Valgrind
confirmed this.

The problem is that when processing the bypassed reservations,
make_automaton_pairs allocates memory in proportion to the number of
defined bypasses rather than the number of bypassed reservations. This
means that if the number of bypassed reservations is sufficiently larger
than the number of bypasses, the loop will overwrite unallocated memory.

I also observed this effect on aarch64, but there was no segfault there,
presumably because the number of reservations in aarch64 is much smaller
than arm at the moment (we only use two pipeline descriptions in aarch64).

This patch fixes that and valgrind confirms that there's no out of
bounds accesses now.

Bootstrapped and tested arm-none-linux-gnueabihf,
aarch64-none-linux-gnu, x86_64-linux.

Ok for trunk?

Thanks,
Kyrill

2014-06-13  Kyrylo Tkachov 

  * genattrtab.c (n_bypassed): New variable.
  (process_bypasses): Initialise n_bypassed.
  Count number of bypassed reservations.
  (make_automaton_attrs): Allocate space for bypassed reservations
  rather than number of bypasses.

genattrtab-bypasses.patch


diff --git a/gcc/genattrtab.c b/gcc/genattrtab.c
index c5ce51c..2b6b3ce 100644
--- a/gcc/genattrtab.c
+++ b/gcc/genattrtab.c
@@ -4766,6 +4766,7 @@ struct bypass_list

   static struct bypass_list *all_bypasses;
   static size_t n_bypasses;
+static size_t n_bypassed;

   static void
   gen_bypass_1 (const char *s, size_t len)
@@ -4811,12 +4812,19 @@ process_bypasses (void)
 struct bypass_list *b;
 struct insn_reserv *r;

+  n_bypassed = 0;
+
 /* The reservation list is likely to be much longer than the bypass
list.  */
 for (r = all_insn_reservs; r; r = r->next)
   for (b = all_bypasses; b; b = b->next)
 if (fnmatch (b->pattern, r->name, 0) == 0)
-   r->bypassed = true;
+{
+  if (!r->bypassed)
+n_bypassed++;
+
+  r->bypassed = true;
+}
   }

Might as well go ahead and put the r->bypassed = true assignment inside
the if (!r->bypassed) conditional.  It probably doesn't matter in terms
of real performance, but it's easy to do.  In fact, once you hit that
case ISTM the inner loop is pointless.  So I think it ought to look like:

/* The reservation list is likely to be much longer than the bypass
   list.  */
for (r = all_insn_reservs; r; r = r->next)
  for (b = all_bypasses; b; b = b->next)
if (fnmatch (b->pattern, r->name, 0) == 0)
{
  r->bypassed = true;
  n_bypassed++;
  break;
}


Or am I missing something?


Doh, you're right. I did consider it but for some reason thought we 
might want to iterate over all of the bypasses anyway. Breaking out 
seems good.


How about this?
Tested on arm and aarch64 and confirmed with valgrind that no out of 
bounds accesses occur.

I kicked off an x86_64 bootstrap but don't expect any problems.

Thanks,
Kyrillcommit 676b85f7a7cc1446482334dcaad457ac328875a8
Author: Kyrylo Tkachov 
Date:   Fri Jun 13 11:09:57 2014 +0100

[genattrtab] Fix memory corruption with bypasses

diff --git a/gcc/genattrtab.c b/gcc/genattrtab.c
index c5ce51c..9db2ade 100644
--- a/gcc/genattrtab.c
+++ b/gcc/genattrtab.c
@@ -4766,6 +4766,7 @@ struct bypass_list
 
 static struct bypass_list *all_bypasses;
 static size_t n_bypasses;
+static size_t n_bypassed;
 
 static void
 gen_bypass_1 (const char *s, size_t len)
@@ -4811,12 +4812,18 @@ process_bypasses (void)
   struct bypass_list *b;
   struct insn_reserv *r;
 
+  n_bypassed = 0;
+
   /* The reservation list is likely to be much longer than the bypass
  list.  */
   for (r = all_insn_reservs; r; r = r->next)
 for (b = all_bypasses; b; b = b->next)
   if (fnmatch (b->pattern, r->name, 0) == 0)
-	r->bypassed = true;
+{
+  n_bypassed++;
+  r->bypassed = true;
+  break;
+}
 }
 
 /* Check that attribute NAME is used in define_insn_reservation condition
@@ -5075,7 +5082,7 @@ make_automaton_attrs (void)
   process_bypasses ();
 
   byps_exp = rtx_alloc (COND);
-  XVEC (byps_exp, 0) = rtvec_alloc (n_bypasses * 2);
+  XVEC (byps_exp, 0) = rtvec_alloc (n_bypassed * 2);
   XEXP (byps_exp, 1) = make_numeric_value (0);
   for (decl = all_insn_reservs, i = 0;
 	   decl;

Re: [PATCH, Pointer Bounds Checker 35/x] Fix object size emitted for structures with flexible arrays

[Ilya Enkovich]

2014-06-13 19:40 GMT+04:00 Jeff Law :
> On 06/12/14 17:38, Ilya Enkovich wrote:
>>>
>>> It looks ok to me - did you test with all languages?  In particular did
>>> you test Ada?
>>
>>
>> I configure compiler with no language disabling and then run bootstrap
>> and make check. Does it mean all languages are covered? Will make more
>> testing if required.
>
> In addition to Richi's comment able --enable-languages= ...  You also need
> an Ada compiler installed to build Ada.   Most Linux distributions provide
> Ada packages of some  sort.  It may be called ada or gnat.
>

Thanks for tips! I added --enable-languages=all,ada,obj-c++,go and got
no new failures with this patch.

Ilya

> Jeff
>
>

Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)

[Dominique Dhumieres]

> Done in r211699.

This breaks bootstrap on x86_64-apple-darwin13:

/opt/gcc/build_w/./prev-gcc/xg++ -B/opt/gcc/build_w/./prev-gcc/ 
-B/opt/gcc/gcc4.10w/x86_64-apple-darwin13.2.0/bin/ -nostdinc++ 
-B/opt/gcc/build_w/prev-x86_64-apple-darwin13.2.0/libstdc++-v3/src/.libs 
-B/opt/gcc/build_w/prev-x86_64-apple-darwin13.2.0/libstdc++-v3/libsupc++/.libs  
-I/opt/gcc/build_w/prev-x86_64-apple-darwin13.2.0/libstdc++-v3/include/x86_64-apple-darwin13.2.0
  -I/opt/gcc/build_w/prev-x86_64-apple-darwin13.2.0/libstdc++-v3/include  
-I/opt/gcc/work/libstdc++-v3/libsupc++ 
-L/opt/gcc/build_w/prev-x86_64-apple-darwin13.2.0/libstdc++-v3/src/.libs 
-L/opt/gcc/build_w/prev-x86_64-apple-darwin13.2.0/libstdc++-v3/libsupc++/.libs 
-c   -g -O2  -gtoggle -DIN_GCC-fno-exceptions -fno-rtti 
-fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings 
-Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic 
-Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common 
 -DHAVE_CONFIG_H -I. -I. -I../../work/gcc -I../../work/gcc/. 
-I../../work/gcc/../include -I./../intl -I../../work/gcc/../libcpp/include 
-I/opt/mp/include  -I../../work/gcc/../libdecnumber 
-I../../work/gcc/../libdecnumber/dpd -I../libdecnumber 
-I../../work/gcc/../libbacktrace -DCLOOG_INT_GMP  -I/opt/mp/include  -o asan.o 
-MT asan.o -MMD -MP -MF ./.deps/asan.TPo ../../work/gcc/asan.c
../../work/gcc/asan.c: In function 'void build_check_stmt(location_t, tree, 
tree, long long int, gimple_stmt_iterator*, bool, bool, bool, bool, unsigned 
int, bool, bool)':
../../work/gcc/asan.c:1810:34: error: 't' may be used uninitialized in this 
function [-Werror=maybe-uninitialized]
gimple_seq_last (seq)));
  ^
cc1plus: all warnings being treated as errors

TIA

Dominique

[PATCH] Implement -fsanitize=bounds and internal calls in FEs

[Marek Polacek]

The following is quite large patch for something rather simple as
bounds checking.  The idea is to instrument ARRAY_REFs in such a way
that iff the array index is greater than the max value of the TYPE_DOMAIN
of the ARRAY_TYPE, call __ubsan builtin and report error. 
My first attempts failed, however - e.g. if we add the instrumentation (the
conditional, not an internal call) in some early pass, VRP spuriously warns
about array bounds - it can't figure out that some path in the jump threaded
code never executes.  Another attempt was adding internal calls in the
.ubsan pass right after .ssa - same thing that we do for NULL sanitization -
and expand those in the .sanopt pass.
But this doesn't work either, the issue is with VLAs, where after
gimplification TYPE_MAX_VALUE of TYPE_DOMAIN is not usable - so this is
dead in the water too.

So next thing to do was to generate internal calls in the front ends;
we can use TYPE_MAX_VALUE of TYPE_DOMAIN therein.  But we don't have
the infrastructure for that, so I had to implement it.  To avoid
creating a new tree code, I'm reusing CALL_EXPR whose CALL_EXPR_FN
is NULL_TREE, but it has new "ifn" field in tree_base.u, which says
which internal function it is.  The .ifn field is accessed by new macro
CALL_EXPR_IFN.
Now everything seems to work, even though it might not be up to snuff
yet.

Jason/Joseph, could you please look at the C++/C FE parts?
Jakub/Richi, could you please look at the gimplification part?  I'm
pretty sure I missed something in there (LHS of internal calls?  
TREE_SIDE_EFFECTs?).
Pretty printing of internal calls is lame - maybe add a string param
to DEF_INTERNAL_FN?  I just wanted to make sure we don't ICE on it.

Regtested, bootstraped, "boostrap-ubsan-ed" on x86_64-linux.

2014-06-16  Marek Polacek  

* asan.c (pass_sanopt::execute): Handle IFN_UBSAN_BOUNDS.
* flag-types.h (enum sanitize_code): Add SANITIZE_BOUNDS and or it
into SANITIZE_UNDEFINED.
* gimplify.c (gimplify_call_expr): Add gimplification of internal
functions created in the FEs.
* internal-fn.c: Move "internal-fn.h" after "tree.h".
(expand_UBSAN_BOUNDS): New function.
* internal-fn.def (UBSAN_BOUNDS): New internal function.
* internal-fn.h: Don't define internal functions here.
* opts.c (common_handle_option): Add -fsanitize=bounds.
* sanitizer.def (BUILT_IN_UBSAN_HANDLE_OUT_OF_BOUNDS,
BUILT_IN_UBSAN_HANDLE_OUT_OF_BOUNDS_ABORT): Add.
* tree-core.h: Define internal functions here.
(struct tree_base): Add ifn field.
* tree-pretty-print.c (print_call_name): Handle functions without
CALL_EXPR_FN.
* tree.c (get_callee_fndecl): Likewise.
(build_call_expr_internal_loc): New function.
* tree.def (CALL_EXPR): Update description.
* tree.h (CALL_EXPR_IFN): Define.
(build_call_expr_internal_loc): Declare.
* ubsan.c (get_ubsan_type_info_for_type): Return 0 for non-arithmetic
types.
(ubsan_type_descriptor): Change bool parameter to enum
ubsan_print_style.  Adjust the code.  Add handling of
UBSAN_PRINT_ARRAY.
(ubsan_expand_bounds_btn): New function.
(ubsan_expand_null_ifn): Adjust ubsan_type_descriptor call.
(ubsan_build_overflow_builtin): Likewise.
(instrument_bool_enum_load): Likewise.
(ubsan_instrument_float_cast): Likewise.
* ubsan.h (enum ubsan_print_style): New enum.
(ubsan_expand_bounds_btn): Declare.
(ubsan_type_descriptor): Adjust declaration.  Use a default parameter.
c-family/
* c-common.c: Inlcude "c-ubsan.h".
(c_fully_fold_internal): Instrument array bounds.
* c-ubsan.c: Include "internal-fn.h".
(ubsan_instrument_division): Mark instrumented arrays as having
side effects.  Adjust ubsan_type_descriptor call.
(ubsan_instrument_shift): Likewise.
(ubsan_instrument_vla): Adjust ubsan_type_descriptor call.
(ubsan_instrument_bounds): New function.
* c-ubsan.h (ubsan_instrument_bounds): Declare.
cp/
* cp-gimplify.c (cp_genericize_r): Instrument array bounds.
testsuite/
* c-c++-common/ubsan/bounds-1.c: New test.
* c-c++-common/ubsan/bounds-2.c: New test.
* c-c++-common/ubsan/bounds-3.c: New test.

diff --git gcc/asan.c gcc/asan.c
index 651cfed..e0ca376 100644
--- gcc/asan.c
+++ gcc/asan.c
@@ -2708,6 +2708,9 @@ pass_sanopt::execute (function *fun)
  case IFN_UBSAN_NULL:
ubsan_expand_null_ifn (gsi);
break;
+ case IFN_UBSAN_BOUNDS:
+   ubsan_expand_bounds_btn (&gsi);
+   break;
  default:
break;
  }
@@ -2718,6 +2721,10 @@ pass_sanopt::execute (function *fun)
  print_gimple_stmt (dump_file, stmt, 0, dump_flags);
  fprintf (dump_file, "\n");
}
+
+ /* ubsan_expand_bounds_

Regimplification enhancements 1/3

[Bernd Schmidt]

For the ptx port, I've needed to write a new pass which ensures all 
objects go into address spaces as required by the machine. This uses the 
regimplification code in gimplify-me.c, and that requires some fixes and 
upgrades.


Here's the first. When address spaces change, an ADDR_EXPR may have to 
be changed to ADDR_SPACE_CONVERT_EXPR, and these two have different 
representations in gimple. This patch stops the regimplification code 
from creating invalid gimple in such a case.


Bootstrapped and tested on x86_64-linux, ok?


Bernd
commit 00edd2d382d406b2f729885f08aa16928552d9d0
Author: Bernd Schmidt 
Date:   Wed Jun 11 18:41:09 2014 +0200

Fix an issue with regimplification.

This is in preparation for the lower-address-spaces pass for the ptx port. We
need to teach the regimplifier how to handle the case when an ADDR_EXPR turns
into something else, like an ADDR_SPACE_CONVERT_EXPR.

	gcc/
	* gimplify-me.c (gimple_regimplify_operands): Handle case where a
	GIMPLE_SINGLE_RHS is turned into something else.

diff --git a/gcc/gimplify-me.c b/gcc/gimplify-me.c
index 05e986a..467ec6c 100644
--- a/gcc/gimplify-me.c
+++ b/gcc/gimplify-me.c
@@ -248,6 +248,22 @@ gimple_regimplify_operands (gimple stmt, gimple_stmt_iterator *gsi_p)
 	gimplify_expr (&op, &pre, NULL, is_gimple_val, fb_rvalue);
 	  gimple_set_op (stmt, i - 1, op);
 	}
+  if (is_gimple_assign (stmt)
+	  && num_ops == 2
+	  && (get_gimple_rhs_class (gimple_expr_code (stmt))
+	  == GIMPLE_SINGLE_RHS)
+	  && (get_gimple_rhs_class (TREE_CODE (gimple_op (stmt, 1)))
+	  != GIMPLE_SINGLE_RHS))
+	{
+	  tree rhs = gimple_assign_rhs1 (stmt);
+	  tree temp = create_tmp_reg (TREE_TYPE (rhs), NULL);
+	  if (gimple_in_ssa_p (cfun))
+	temp = make_ssa_name (temp, NULL);
+	  gimple_assign_set_rhs1 (stmt, temp);
+	  gimple_assign_set_rhs_code (stmt, TREE_CODE (temp));
+	  gimple pre_stmt = gimple_build_assign (temp, rhs);
+	  gimple_seq_add_stmt_without_update (&pre, pre_stmt);
+	}
 
   lhs = gimple_get_lhs (stmt);
   /* If the LHS changed it in a way that requires a simple RHS,

Re: Go patch committed: Pass -t to native linker on Solaris

[Rainer Orth]

Ian Lance Taylor  writes:

> This patch changes the gccgo driver to pass -t to the native linker on
> Solaris.  This avoids warnings like
>
> ld: warning: symbol 'go$zerovalue' has differing sizes:
> (file hello.o value=0x8; file
> i386-pc-solaris2.11/libgo/.libs/libgo.so value=0x800);
> hello.o definition taken and updated with larger size
>
> The symbol go$zerovalue is a common symbol and it's normal for it to
> have different sizes in different object files and shared libraries.
> The linker should take the larger size, which is what it does.
> Unfortunately, by default, it emits a warning, which in this case is
> useless.  This patch passes the -t option to disable the warning.
>
> Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu.
> Bootstrapped and ran a few tests on x86_64-sun-solaris.  Committed to
> mainline.

Works fine, thanks.  Just before your patch arrived, I meant to test the
following, slightly more general patch.  Perhaps it's an option to
handle other quirks like this?

Rainer


2014-06-13  Rainer Orth  

gcc/go:
PR go/61496
* gospec.c (lang_specific_driver) [TARGET_GO_LINK_OPTIONS]: Pass
target specific link options.

gcc:
PR go/61496
* config/sol2.h [!USE_GLD] (TARGET_GO_LINK_OPTIONS): Define.

# HG changeset patch
# Parent 59e0e6f1eecd53ddab49b2cd5015be221b5e505a
Suppress Solaris ld warnings about go$zerovalue sizes (PR go/61496)

diff --git a/gcc/config/sol2.h b/gcc/config/sol2.h
--- a/gcc/config/sol2.h
+++ b/gcc/config/sol2.h
@@ -303,6 +303,12 @@ along with GCC; see the file COPYING3.  
 /* collect2.c can only parse GNU nm -n output.  Solaris nm needs -png to
produce the same format.  */
 #define NM_FLAGS "-png"
+
+#ifndef USE_GLD
+/* Solaris ld warns about common symbols of differing sizes, which Go uses
+   for go$zerovalue.  Pass -t to suppress that warning.  */
+#define TARGET_GO_LINK_OPTIONS "-t"
+#endif
 
 /* The system headers under Solaris 2 are C++-aware since 2.0.  */
 #define NO_IMPLICIT_EXTERN_C
diff --git a/gcc/go/gospec.c b/gcc/go/gospec.c
--- a/gcc/go/gospec.c
+++ b/gcc/go/gospec.c
@@ -395,6 +395,12 @@ lang_specific_driver (struct cl_decoded_
 }
 #endif
 
+#ifdef TARGET_GO_LINK_OPTIONS
+  /* Pass target specific linker options if present.  */
+  generate_option (OPT_Wl_, TARGET_GO_LINK_OPTIONS, 1, CL_DRIVER,
+		   &new_decoded_options[j++]);
+#endif
+
   *in_decoded_options_count = j;
   *in_decoded_options = new_decoded_options;
   *in_added_libraries = added_libraries;

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

Regimplification enhancements 3/3

[Bernd Schmidt]

There's code in regimplification that makes us use an extra temporary
when we encounter a call returning a non-BLKmode structure. This seems
somewhat inefficient and unnecessary, and when used from the
lower-addr-spaces pass I'm working on it leads to problems further
down that look like tree-ssa bugs that I wasn't able to clearly
disentangle.

Here's what happens on compile/pr51761.c.  Regimplification has the
following effect, creating an extra temporary _6:

-  D.1378 = fooD.1373 (aD.1377);
+  _6 = fooD.1373 (aD.1377);
+  # .MEMD.1382 = VDEF <.MEMD.1382>
+  D.1378 = _6;

SRA turns this into:

  _6 = fooD.1373 (aD.1377);
  # VUSE <.MEM_3>
  SR$2_7 = MEM[(struct S *)&_6];

Somehow, the address of &_6 doesn't count as a use, and the DCE pass 
decides it is unused:


  Eliminating unnecessary statements:
  Deleting LHS of call: _6 = foo (a);

However, the statement
  SR$2_7 = MEM[(struct S *)&_6];
is still present, and we have an SSA name without a definition, leading 
to a crash.


Rather than figure all this out, I decided to try making the 
regimplification not generate the extra copy in the first place. The 
testsuite seems to agree with me that it's unnecessary. Bootstrapped and 
tested on x86_64-linux, ok?



Bernd
	* gimplify-me.c (gimple_regimplify_operands): Avoid unnecessary
	temporary for struct returns.

diff --git a/gcc/gimplify-me.c b/gcc/gimplify-me.c
index 05eaeb0..2d1181b 100644
--- a/gcc/gimplify-me.c
+++ b/gcc/gimplify-me.c
@@ -304,25 +304,9 @@ gimple_regimplify_operands (gimple stmt, gimple_stmt_iterator *gsi_p)
 		need_temp = true;
 		}
 	}
-	  else
-	{
-	  if (is_gimple_reg_type (TREE_TYPE (lhs)))
-		need_temp = true;
-	  else if (TYPE_MODE (TREE_TYPE (lhs)) != BLKmode)
-		{
-		  if (is_gimple_call (stmt))
-		{
-		  tree fndecl = gimple_call_fndecl (stmt);
+	  else if (is_gimple_reg_type (TREE_TYPE (lhs)))
+	need_temp = true;
 
-		  if (!aggregate_value_p (TREE_TYPE (lhs), fndecl)
-			  && !(fndecl && DECL_RESULT (fndecl)
-			   && DECL_BY_REFERENCE (DECL_RESULT (fndecl
-			need_temp = true;
-		}
-		  else
-		need_temp = true;
-		}
-	}
 	  if (need_temp)
 	{
 	  tree temp = create_tmp_reg (TREE_TYPE (lhs), NULL);

Regimplification enhancements 2/3

[Bernd Schmidt]

This fixes an issue that showed up when regimplifying a call with a 
WITH_SIZE_EXPR for one of its arguments. At the moment we can end up 
trying to make a temporary and aborting because we don't know what size 
it ought to be.  The problem is that we call gimplify_expr with the 
wrong predicate: gimplify_arg uses is_gimple_lvalue in some cases 
instead of is_gimple_val, and regimplification needs to match that.


Bootstrapped and tested on x86_64-linux, ok?


Bernd
commit c1296ac4f4e7e8f0fb9c87d71ca8194a8eac0067
Author: Bernd Schmidt 
Date:   Wed Jun 11 18:41:09 2014 +0200

Fix an issue with regimplification.

	gcc/
	* gimplify-me.c (gimple_regimplify_operands): Ensure that for
	calls we use the same predicate as in gimplify_arg.

diff --git a/gcc/gimplify-me.c b/gcc/gimplify-me.c
index 467ec6c..05eaeb0 100644
--- a/gcc/gimplify-me.c
+++ b/gcc/gimplify-me.c
@@ -245,7 +245,17 @@ gimple_regimplify_operands (gimple stmt, gimple_stmt_iterator *gsi_p)
 	  gimplify_expr (&op, &pre, NULL, is_gimple_call_addr, fb_rvalue);
 	}
 	  else
-	gimplify_expr (&op, &pre, NULL, is_gimple_val, fb_rvalue);
+	{
+	  bool (*test) (tree) = is_gimple_val;
+	  fallback_t fb = fb_rvalue;
+	  if (is_gimple_call (stmt)
+		  && !is_gimple_reg_type (TREE_TYPE (op)))
+		{
+		  test = is_gimple_lvalue;
+		  fb = fb_either;
+		}
+	  gimplify_expr (&op, &pre, NULL, test, fb);
+	}
 	  gimple_set_op (stmt, i - 1, op);
 	}
   if (is_gimple_assign (stmt)

Re: [PATCH] Implement -fsanitize=bounds and internal calls in FEs

[Jakub Jelinek]

On Mon, Jun 16, 2014 at 12:39:07PM +0200, Marek Polacek wrote:
> Jason/Joseph, could you please look at the C++/C FE parts?

As mentioned on IRC, you need to differentiate between taking address
and not taking address.

struct S { int a; int b; } s[4], *t;
int *a, *b, *c;
void *d;
int e[4][4];

void
foo ()
{
  t = &s[4];  // Should be fine
  a = &s[4].a; // Error
  b = &s[4].b; // Error
  d = &e[4];  // Should be fine
  c = &e[4][0]; // Error
}

So, supposedly when e.g. in cp_genericize_r, for ADDR_EXPR 
allow off-by-one, for all other ARRAY_REFs (e.g. those not appearing
inside of ADDR_EXPR, or not directly inside of ADDR_EXPR, e.g. with
COMPONENT_REF or another ARRAY_REF in between) disallow off-by-one.

Jakub

Re: breakage with "[PATCH 1/6] Add FOR_EACH_INSN{_INFO}_{DEFS,USES,EQ_USES}"

[Hans-Peter Nilsson]

On Mon, 16 Jun 2014, Steven Bosscher wrote:
> On Mon, Jun 16, 2014 at 12:36 AM, Hans-Peter Nilsson wrote:
> > On Sun, 15 Jun 2014, Steven Bosscher wrote:
> >> On Sun, Jun 15, 2014 at 1:27 PM, Hans-Peter Nilsson wrote:
> >> > /tmp/hpautotest-gcc0/gcc/gcc/auto-inc-dec.c: In function 'void
> >> > merge_in_block(int, basic_block_def*)':
> >> > /tmp/hpautotest-gcc0/gcc/gcc/auto-inc-dec.c:1442: error: 'uid'
> >> > was not declared in this scope
> >> > make[2]: *** [auto-inc-dec.o] Error 1
> >> >
> >> > brgds, H-P
> >>
> >>
> >> Bah, this is why I just *hate* all the gcc code that's only compiled
> >> if certain #defines are set...
> >
> > I couldn't agree more.  Might not have been obvious when writing
> > the mosly-mechanical patch, still the auto-inc-dec.c name should
> > have been a red flag that a representative target should have
> > been tested (i.e. not x86_64 and i686).
>
> I agree, but I think you'd agree with me if I say that Richard S. is
> one of the few people who almost always goes beyond the normal amount
> of testing required for a patch.

His testing efforts relative many others I'd agree on (and I
seem to have a knack for being caught when things still go bad
with his patches), but I'd say the amount of testing that
Richard S usually applies is entirely appropriate and I wish
others would follow!  Also, looks like he fixed it.  (Thanks!)

> Breakage like this will just happen
> to us all, every once in a while, until we compile all middle-end code
> at least, regardless of #defines and whatnot (conditionally compiled
> code, from the top of my head: CC0, scheduler, dbrsched, auto-inc-dec,
> HAVE_conditional_move, etc...).

The patch looked pretty mechanical.  I feared the rabbit-hole,
but the stage 2 issue was apparently just the use of stale data
from the loop.  Exercising that required actually running the
code when testing.

brgds, H-P

Re: C++ PATCH to add -Wabi=n

[Rainer Orth]

Jason Merrill  writes:

> Now that -fabi-version defaults to 0, -Wabi isn't very useful.  But for
> people interested in compatibility with earlier versions, this patch allows
> you to say -Wabi=2 to get any relevant warnings.  This patch also adjusts
> the compatibility aliases to default to backward compatibility with
> -fabi-version=2.

This patch

2014-06-12  Jason Merrill  

* toplev.c (process_options): Reject -fabi-version=1.

which was committed as part of the above, but not posted AFAICS,
introduced a testsuite failure on i386-pc-solaris2.1? and
x86_64-unknown-linux-gnu with -m32:

FAIL: g++.dg/abi/layout4.C -std=gnu++98 (test for excess errors)
WARNING: g++.dg/abi/layout4.C -std=gnu++98 compilation failed to produce 
executable
FAIL: g++.dg/abi/layout4.C -std=gnu++11 (test for excess errors)
WARNING: g++.dg/abi/layout4.C -std=gnu++11 compilation failed to produce 
executable
FAIL: g++.dg/abi/layout4.C -std=gnu++1y (test for excess errors)
WARNING: g++.dg/abi/layout4.C -std=gnu++1y compilation failed to produce 
executable

Excess errors:
/vol/gcc/src/hg/trunk/local/gcc/testsuite/g++.dg/abi/layout4.C:1:0: error: 
'-fabi-version=1' is no longer supported

Rainer

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

Re: Regimplification enhancements 1/3

[Richard Biener]

On Mon, Jun 16, 2014 at 12:56 PM, Bernd Schmidt  wrote:
> For the ptx port, I've needed to write a new pass which ensures all objects
> go into address spaces as required by the machine. This uses the
> regimplification code in gimplify-me.c, and that requires some fixes and
> upgrades.

Can you explain how and why you need re-gimplification here?  IMHO
you should apply the necessary steps yourself, not put in GENERIC
into GIMPLE and rely on the gimplifier.

> Here's the first. When address spaces change, an ADDR_EXPR may have to be
> changed to ADDR_SPACE_CONVERT_EXPR, and these two have different
> representations in gimple. This patch stops the regimplification code from
> creating invalid gimple in such a case.
>
> Bootstrapped and tested on x86_64-linux, ok?

+  if (is_gimple_assign (stmt)
+ && num_ops == 2
+ && (get_gimple_rhs_class (gimple_expr_code (stmt))
+ == GIMPLE_SINGLE_RHS)
+ && (get_gimple_rhs_class (TREE_CODE (gimple_op (stmt, 1)))
+ != GIMPLE_SINGLE_RHS))

err ... that's a very crappy gimple condition that isn't going to reliably work.

You can't turn a x = &foo; into x = (convert)&foo; this way abusing
the re-gimplification routines.

The re-gimplification routines do _not_ accept random GENERIC
operands.  They were written to handle registers becoming non-registers
and nothing more.

Richard.

>
> Bernd

Re: Regimplification enhancements 2/3

[Richard Biener]

On Mon, Jun 16, 2014 at 12:57 PM, Bernd Schmidt  wrote:
> This fixes an issue that showed up when regimplifying a call with a
> WITH_SIZE_EXPR for one of its arguments. At the moment we can end up trying
> to make a temporary and aborting because we don't know what size it ought to
> be.  The problem is that we call gimplify_expr with the wrong predicate:
> gimplify_arg uses is_gimple_lvalue in some cases instead of is_gimple_val,
> and regimplification needs to match that.
>
> Bootstrapped and tested on x86_64-linux, ok?

I think the same is true for assigns but they may go the two-arg
single-rhs re-gimplification.

IMHO the code is factored in a very bad way but your patch looks
correct to me.

Thus, ok.

Thanks,
Richard.

>
> Bernd

Re: Regimplification enhancements 3/3

[Richard Biener]

On Mon, Jun 16, 2014 at 12:57 PM, Bernd Schmidt  wrote:
> There's code in regimplification that makes us use an extra temporary
> when we encounter a call returning a non-BLKmode structure. This seems
> somewhat inefficient and unnecessary, and when used from the
> lower-addr-spaces pass I'm working on it leads to problems further
> down that look like tree-ssa bugs that I wasn't able to clearly
> disentangle.
>
> Here's what happens on compile/pr51761.c.  Regimplification has the
> following effect, creating an extra temporary _6:
>
> -  D.1378 = fooD.1373 (aD.1377);
> +  _6 = fooD.1373 (aD.1377);
> +  # .MEMD.1382 = VDEF <.MEMD.1382>
> +  D.1378 = _6;
>
> SRA turns this into:
>
>   _6 = fooD.1373 (aD.1377);
>   # VUSE <.MEM_3>
>   SR$2_7 = MEM[(struct S *)&_6];

clearly bogus - _6 is a register, you can't use a MEM on it.

> Somehow, the address of &_6 doesn't count as a use, and the DCE pass decides
> it is unused:
>
>   Eliminating unnecessary statements:
>   Deleting LHS of call: _6 = foo (a);
>
> However, the statement
>   SR$2_7 = MEM[(struct S *)&_6];
> is still present, and we have an SSA name without a definition, leading to a
> crash.
>
> Rather than figure all this out, I decided to try making the
> regimplification not generate the extra copy in the first place. The
> testsuite seems to agree with me that it's unnecessary. Bootstrapped and
> tested on x86_64-linux, ok?

Ok.  The code looks bogus anyway in that it generates a SSA name
for sth not is_gimple_reg_type ().

Thanks,
Richard.

>
> Bernd

[PATCH, ARM] Don't use NEON for autovectorization in big-endian mode

[Julian Brown]

Hi,

As discussed several times previously, support for NEON in ARM
big-endian mode is quite broken because of differing assumptions about
lane ordering made by the ARM EABI and the set of NEON intrinsics on
the one hand, and the vectorizer on the other.

Fixing this "properly" would involve quite a large overhaul of the NEON
backend implementation, and such an overhaul does not appear to be
forthcoming. Unfortunately this leaves big-endian mode with a problem:
even if the user is not explicitly using NEON intrinsics, compiling
with NEON and the vectorizer enabled (i.e. -O3) can quite easily lead
to incorrect code being generated.

This is the patch we've been using internally for a while to work
around the problem. When applied:

* We do not allow Neon vectors to be used for autovectorization.
  Vectorization is not disabled completely: ARM core registers (e.g.
  four chars packed into a core register) can still be used to vectorize
  loops in limited circumstances. I think this is mildly preferable to
  forcing -ftree-vectorize to be off entirely for big-endian NEON.

* Intrinsics are not touched. Those which attempt to mix generic vector
  operations with the ABI-defined vector types (i.e. those which are
  implemented with __builtin_shuffle) are, I think, technically
  incorrect -- but in the sense of two wrongs making a right, so the
  end result appears to work.

* Generic vectors (i.e. direct use of __attribute__((vector_size(foo)))
  types) will continue to behave strangely in big-endian mode.

This of course continues to be suboptimal, but at least in *the
common case* we stop generating bad code.

Testing in big-endian mode on user-space QEMU (ARMv7-A, NEON, softfp)
shows (apart from some noise) test diffs as attached. Notice the large
number of removed execution failures, in particular.

OK to apply?

Thanks,

Julian

ChangeLog

gcc/
* config/arm/arm.c (arm_array_mode_supported_p): No array modes for
big-endian NEON.
(arm_preferred_simd_mode): Don't use NEON vectors for
autovectorization in big-endian mode.
(arm_autovectorize_vector_sizes): Don't iterate over other vector
sizes for big-endian NEON.

gcc/testsuite/
* lib/target-supports.exp (check_vect_support_and_set_flags): Don't
run vect tests for big-endian ARM NEON.
* gcc.target/arm/neon/vect-vcvt.c: XFAIL for !arm_little_endian.
* gcc.target/arm/neon/vect-vcvtq.c: Likewise.
* gcc.target/arm/neon-vshl-imm-1.c: Likewise.
* gcc.target/arm/neon-vshr-imm-1.c: Likewise.
* gcc.target/arm/neon-vmls-1.c: Likewise.
* gcc.target/arm/neon-vmla-1.c: Likewise.
* gcc.target/arm/neon-vfma-1.c: Likewise.
* gcc.target/arm/neon-vfms-1.c: Likewise.
* gcc.target/arm/neon-vorn-vbic.c: Likewise.
* gcc.target/arm/neon-vlshr-imm-1.c: Likewise.
* gcc.target/arm/neon-vcond-ltgt.c: Likewise.
* gcc.target/arm/neon-vcond-gt.c: Likewise.
* gcc.target/arm/neon-vcond-unordered.c: Likewise.
Index: gcc/config/arm/arm.c
===
--- gcc/config/arm/arm.c	(revision 210209)
+++ gcc/config/arm/arm.c	(working copy)
@@ -28813,7 +28813,7 @@ static bool
 arm_array_mode_supported_p (enum machine_mode mode,
 			unsigned HOST_WIDE_INT nelems)
 {
-  if (TARGET_NEON
+  if (TARGET_NEON && !BYTES_BIG_ENDIAN
   && (VALID_NEON_DREG_MODE (mode) || VALID_NEON_QREG_MODE (mode))
   && (nelems >= 2 && nelems <= 4))
 return true;
@@ -28828,7 +28828,7 @@ arm_array_mode_supported_p (enum machine
 static enum machine_mode
 arm_preferred_simd_mode (enum machine_mode mode)
 {
-  if (TARGET_NEON)
+  if (TARGET_NEON && !BYTES_BIG_ENDIAN)
 switch (mode)
   {
   case SFmode:
@@ -29845,7 +29845,8 @@ arm_vector_alignment (const_tree type)
 static unsigned int
 arm_autovectorize_vector_sizes (void)
 {
-  return TARGET_NEON_VECTORIZE_DOUBLE ? 0 : (16 | 8);
+  return (TARGET_NEON_VECTORIZE_DOUBLE || (TARGET_NEON && BYTES_BIG_ENDIAN))
+	 ? 0 : (16 | 8);
 }
 
 static bool
Index: gcc/testsuite/gcc.target/arm/neon/vect-vcvtq.c
===
--- gcc/testsuite/gcc.target/arm/neon/vect-vcvtq.c	(revision 210209)
+++ gcc/testsuite/gcc.target/arm/neon/vect-vcvtq.c	(working copy)
@@ -24,5 +24,5 @@ int convert()
   return 0;
 }
 
-/* { dg-final { scan-tree-dump-times "vectorized 2 loops" 1 "vect" } } */
+/* { dg-final { scan-tree-dump-times "vectorized 2 loops" 1 "vect" { xfail { ! arm_little_endian } } } } */
 /* { dg-final { cleanup-tree-dump "vect" } } */
Index: gcc/testsuite/gcc.target/arm/neon/vect-vcvt.c
===
--- gcc/testsuite/gcc.target/arm/neon/vect-vcvt.c	(revision 210209)
+++ gcc/testsuite/gcc.target/arm/neon/vect-vcvt.c	(working copy)
@@ -24,5 +24,5 @@ int convert()
   return 0;
 }
 
-/* { dg-final { scan-tree-dump-times "vectorized 2 loops" 1 "vect" } } */
+/* { dg-final { scan-tree-dump-times "vectorized 2 loops" 1 "vect

Re: Regimplification enhancements 1/3

[Bernd Schmidt]

On 06/16/2014 01:24 PM, Richard Biener wrote:

On Mon, Jun 16, 2014 at 12:56 PM, Bernd Schmidt  wrote:

For the ptx port, I've needed to write a new pass which ensures all objects
go into address spaces as required by the machine. This uses the
regimplification code in gimplify-me.c, and that requires some fixes and
upgrades.


Can you explain how and why you need re-gimplification here?  IMHO
you should apply the necessary steps yourself, not put in GENERIC
into GIMPLE and rely on the gimplifier.


That's not actually what's happening - the pass modifies the address 
spaces of all objects (local & global vars etc.) and I have one 
additional patch in gimplify.c that recognizes address space mismatches 
on an ADDR_EXPR (also testing a new TYPE_QUAL_AS_IMPLICIT bit to ensure 
it only triggers in situations it's intended to) and inserts the 
necessary ADDR_SPACE_CONVERT_EXPR. With that I can pass all statements 
through regimplification to get a consistent view of the world again.



You can't turn a x = &foo; into x = (convert)&foo; this way abusing
the re-gimplification routines.
The re-gimplification routines do _not_ accept random GENERIC
operands.  They were written to handle registers becoming non-registers
and nothing more.


It seems to work fine is all I can say. What it was written for and what 
it can be useful for are two different things.



Bernd

C++ PATCHes for c++/61488, 61500 (PMF/template issues)

[Jason Merrill]

A couple of small bugs involving pointers to member functions in 
templates.  In 61500, we were failing to handle MEMBER_REF in 
lvalue_kind.  In 61488, we were failing to handle unlowered PMF syntax 
used as a template argument.


Tested x86_64-pc-linux-gnu, applying to trunk.
commit ef46b757ccae754f3edc937acfa39bcc19e57111
Author: Jason Merrill 
Date:   Fri Jun 13 15:01:15 2014 -0400

	PR c++/61488
	* pt.c (check_valid_ptrmem_cst_expr): Fix for template context.

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 8858908..df57293 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -5350,6 +5350,10 @@ check_valid_ptrmem_cst_expr (tree type, tree expr,
 return true;
   if (cxx_dialect >= cxx11 && null_member_pointer_value_p (expr))
 return true;
+  if (processing_template_decl
+  && TREE_CODE (expr) == ADDR_EXPR
+  && TREE_CODE (TREE_OPERAND (expr, 0)) == OFFSET_REF)
+return true;
   if (complain & tf_error)
 {
   error ("%qE is not a valid template argument for type %qT",
diff --git a/gcc/testsuite/g++.dg/template/ptrmem28.C b/gcc/testsuite/g++.dg/template/ptrmem28.C
new file mode 100644
index 000..0379960
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/ptrmem28.C
@@ -0,0 +1,10 @@
+// PR c++/61488
+
+struct A {
+  typedef int (A::*cont_func)();
+  template  void wait(int);
+  int notify();
+
+  void fix() { wait<&A::notify>(0); } // OK
+  template  void repair() { wait<&A::notify>(0); }
+};
commit 7a2deb52336c87960d6c75439978302d0facdda8
Author: jason 
Date:   Mon Jun 16 11:45:37 2014 +

	PR c++/61500
	* tree.c (lvalue_kind): Handle MEMBER_REF and DOTSTAR_EXPR.

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

diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index 587ae80..3616605 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -102,6 +102,16 @@ lvalue_kind (const_tree ref)
 case IMAGPART_EXPR:
   return lvalue_kind (TREE_OPERAND (ref, 0));
 
+case MEMBER_REF:
+case DOTSTAR_EXPR:
+  if (TREE_CODE (ref) == MEMBER_REF)
+	op1_lvalue_kind = clk_ordinary;
+  else
+	op1_lvalue_kind = lvalue_kind (TREE_OPERAND (ref, 0));
+  if (TYPE_PTRMEMFUNC_P (TREE_TYPE (TREE_OPERAND (ref, 1
+	op1_lvalue_kind = clk_none;
+  return op1_lvalue_kind;
+
 case COMPONENT_REF:
   op1_lvalue_kind = lvalue_kind (TREE_OPERAND (ref, 0));
   /* Look at the member designator.  */
diff --git a/gcc/testsuite/g++.dg/template/ptrmem27.C b/gcc/testsuite/g++.dg/template/ptrmem27.C
new file mode 100644
index 000..8c63f9c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/ptrmem27.C
@@ -0,0 +1,22 @@
+// PR c++/61500
+
+struct X {
+  int i;
+  int j;
+
+  int foo(int X::* ptr);
+
+  template 
+  int bar();
+};
+
+int X::foo(int X::* ptr) {
+  int* p = &(this->*ptr);  // OK.
+  return *p;
+}
+
+template 
+int X::bar() {
+  int* p = &(this->*ptr);  // gcc 4.9.0: OK in C++98 mode, fails in C++11 mode.
+  return *p;
+}

Re: Another AIX Bootstrap failure

[David Edelsohn]

Honza,

Thanks for reverting the patch. I will check if this resolves the
current bootstrap problem.

I was suggesting that you create a branch for all of the visibility
changes to make it easier to track the various original patches and
later correction patches from you.

I don't know if the gen* programs hang because of the visibility
changes or because of the change in sections. The change in sections
could conflict with the GCC code to handle AIX XCOFF CSECTs for
functions.

AIX recently added support for ELF-like visibility. AIX previously
supported the equivalent of visibility through "export" files. The
recent problems could be due to issues with assembly file ordering,
but they also could be related to the visibility changes affecting the
way that GCC emits code to branch to global functions.

Also, your recent ChangeLog entry included a Subversion conflict marker:


2014-06-15  Jan Hubicka  

* c-family/c-common.c (handle_tls_model_attribute): Use set_decl_tls_mod
el.
* c-family/c-common.c (handle_tls_model_attribute): Use
set_decl_tls_model.
>>> .r211699
* cgraph.h (struct varpool_node): Add tls_model.
* tree.c (decl_tls_model, set_decl_tls_model): New functions.
* tree.h (DECL_TLS_MODEL): Update.


Thanks, David



On Mon, Jun 16, 2014 at 12:35 AM, Jan Hubicka  wrote:
>> Honza,
>>
>> The cgraph patch in r211600 broke AIX bootstrap again.  I cannot find
>> the corresponding patch in the GCC Patches mailing list, so I do not
>> see where this was discussed or approved.
>
> Sorry, I remember writting mail about this patch but also can't find it.  The
> patch introduces reset_section that is used when function or variable is
> brought local by the visibility code. In this case we previously incorrectly
> cleared user sections and we forgot to introduce implicit sections with
> -fdata-section and -ffunction-section.
>
> I will revert this change and lets debug it.
>
>>
>> With the patch in r211600, the "gen*" programs in stage2 go into an
>> endless loop.
>>
>> Please revert these comdat patches.  These were not tested appropriately.
>>
>> Please create a branch and we can debug the problems on AIX.
>
> What patches you refer to? I already disabled the localization within
> initializers on AIX (well all targets w/o MAKE_ONE_ONLY support) and
> I am testing patch reverting this change (I want to keep reset section
> just remove the call to resolve unique section) and will commit once
> it converges.
>
> I will send you patch to enable those two changes and lets look into why
> they break. What is common to both patches is that they deal with static
> symbol in named sections, perhaps that is not correctly supported by AIX
> toolchain...
>
> My apologizes for the breakage. I am attaching the patch for reference.
> Honza
>
>
> * symtab.c (symtab_node::reset_section): New method.
> * cgraph.c (cgraph_node_cannot_be_local_p_1): Accept non-local
> for localization.
> * cgraph.h (reset_section): Declare.
> * ipa-inline-analysis.c (do_estimate_growth): Check for comdat groups;
> do not consider comdat locals.
> * cgraphclones.c (set_new_clone_decl_and_node_flags): Get section
> for new symbol.
> * ipa-visiblity.c (cgraph_externally_visible_p): Cleanup.
> (update_visibility_by_resolution_info): Consider UNDEF; fix checking;
> reset sections of symbols dragged out of the comdats.
> (function_and_variable_visibility): Reset sections of localized 
> symbols.
> Index: symtab.c
> ===
> --- symtab.c(revision 211489)
> +++ symtab.c(working copy)
> @@ -1176,6 +1176,21 @@ symtab_node::set_section (const char *se
>symtab_for_node_and_aliases (this, set_section_1, const_cast *>(section), true);
>  }
>
> +/* Reset section of NODE.  That is when NODE is being brought local
> +   we may want to clear section produced for comdat group and depending
> +   on function-sections produce now, local, unique section for it.  */
> +
> +void
> +symtab_node::reset_section ()
> +{
> +  if (!this->implicit_section)
> +return;
> +  this->set_section (NULL);
> +  resolve_unique_section (this->decl, 0,
> + is_a  (this)
> + ? flag_function_sections : flag_data_sections);
> +}
> +
>  /* Worker for symtab_resolve_alias.  */
>
>  static bool
> Index: cgraph.c
> ===
> --- cgraph.c(revision 211488)
> +++ cgraph.c(working copy)
> @@ -2169,6 +2169,7 @@ cgraph_node_cannot_be_local_p_1 (struct
> && !node->forced_by_abi
> && !symtab_used_from_object_file_p (node)
> && !node->same_comdat_group)
> +  || DECL_EXTERNAL (node->decl)
>|| !node->externally_visible));
>  }
>
> @@ -2259,14 +2260,14 @@ cgraph_make_node_local_1 (struct cgraph_

Re: C++ PATCH to add -Wabi=n

[Jason Merrill]

On 06/16/2014 01:24 PM, Rainer Orth wrote:

This patch

2014-06-12  Jason Merrill  

* toplev.c (process_options): Reject -fabi-version=1.

which was committed as part of the above, but not posted AFAICS,


https://gcc.gnu.org/ml/gcc-patches/2014-06/msg01054.html


introduced a testsuite failure on i386-pc-solaris2.1? and
x86_64-unknown-linux-gnu with -m32:

FAIL: g++.dg/abi/layout4.C -std=gnu++98 (test for excess errors)


Fixed, thanks.

Jason

Re: Another AIX Bootstrap failure

[Ramana Radhakrishnan]

On Mon, Jun 16, 2014 at 5:35 AM, Jan Hubicka  wrote:
>> Honza,
>>
>> The cgraph patch in r211600 broke AIX bootstrap again.  I cannot find
>> the corresponding patch in the GCC Patches mailing list, so I do not
>> see where this was discussed or approved.
>
> Sorry, I remember writting mail about this patch but also can't find it.  The
> patch introduces reset_section that is used when function or variable is
> brought local by the visibility code. In this case we previously incorrectly
> cleared user sections and we forgot to introduce implicit sections with
> -fdata-section and -ffunction-section.
>
> I will revert this change and lets debug it.
>

It looks like this broke arm-none-linux-gnueabihf bootstraps with
section conflicts in strstream.c. Is that the kind of failures you are
seeing on AIX ?

/work/trunk-nightlies/builds/build-210919/armv7l-unknown-linux-gnueabihf/libstdc++-v3/include/backward/strstream:126:9:
error: std::istrstream::_ZTVSt10istrstream.localalias.0 causes a
section type conflict with std::istrstream::_ZTVSt10istrstream

 Can you please test it on this platform before reapplying / or let me
know when you have the smaller patches. I'll see what I can do.

regards
Ramana

>>
>> With the patch in r211600, the "gen*" programs in stage2 go into an
>> endless loop.
>>
>> Please revert these comdat patches.  These were not tested appropriately.
>>
>> Please create a branch and we can debug the problems on AIX.
>
> What patches you refer to? I already disabled the localization within
> initializers on AIX (well all targets w/o MAKE_ONE_ONLY support) and
> I am testing patch reverting this change (I want to keep reset section
> just remove the call to resolve unique section) and will commit once
> it converges.
>
> I will send you patch to enable those two changes and lets look into why
> they break. What is common to both patches is that they deal with static
> symbol in named sections, perhaps that is not correctly supported by AIX
> toolchain...
>
> My apologizes for the breakage. I am attaching the patch for reference.
> Honza
>
>
> * symtab.c (symtab_node::reset_section): New method.
> * cgraph.c (cgraph_node_cannot_be_local_p_1): Accept non-local
> for localization.
> * cgraph.h (reset_section): Declare.
> * ipa-inline-analysis.c (do_estimate_growth): Check for comdat groups;
> do not consider comdat locals.
> * cgraphclones.c (set_new_clone_decl_and_node_flags): Get section
> for new symbol.
> * ipa-visiblity.c (cgraph_externally_visible_p): Cleanup.
> (update_visibility_by_resolution_info): Consider UNDEF; fix checking;
> reset sections of symbols dragged out of the comdats.
> (function_and_variable_visibility): Reset sections of localized 
> symbols.
> Index: symtab.c
> ===
> --- symtab.c(revision 211489)
> +++ symtab.c(working copy)
> @@ -1176,6 +1176,21 @@ symtab_node::set_section (const char *se
>symtab_for_node_and_aliases (this, set_section_1, const_cast *>(section), true);
>  }
>
> +/* Reset section of NODE.  That is when NODE is being brought local
> +   we may want to clear section produced for comdat group and depending
> +   on function-sections produce now, local, unique section for it.  */
> +
> +void
> +symtab_node::reset_section ()
> +{
> +  if (!this->implicit_section)
> +return;
> +  this->set_section (NULL);
> +  resolve_unique_section (this->decl, 0,
> + is_a  (this)
> + ? flag_function_sections : flag_data_sections);
> +}
> +
>  /* Worker for symtab_resolve_alias.  */
>
>  static bool
> Index: cgraph.c
> ===
> --- cgraph.c(revision 211488)
> +++ cgraph.c(working copy)
> @@ -2169,6 +2169,7 @@ cgraph_node_cannot_be_local_p_1 (struct
> && !node->forced_by_abi
> && !symtab_used_from_object_file_p (node)
> && !node->same_comdat_group)
> +  || DECL_EXTERNAL (node->decl)
>|| !node->externally_visible));
>  }
>
> @@ -2259,14 +2260,14 @@ cgraph_make_node_local_1 (struct cgraph_
>  {
>symtab_make_decl_local (node->decl);
>
> -  node->set_section (NULL);
>node->set_comdat_group (NULL);
>node->externally_visible = false;
>node->forced_by_abi = false;
>node->local.local = true;
> -  node->set_section (NULL);
> +  node->reset_section ();
>node->unique_name = (node->resolution == LDPR_PREVAILING_DEF_IRONLY
> - || node->resolution == 
> LDPR_PREVAILING_DEF_IRONLY_EXP);
> +  || node->unique_name
> +  || node->resolution == 
> LDPR_PREVAILING_DEF_IRONLY_EXP);
>node->resolution = LDPR_PREVAILING_DEF_IRONLY;
>gcc_assert (cgraph_function_body_availability (node)

[committed] Add missing Var for Wswitch-bool (PR c/60439)

[Marek Polacek]

Wswitch-bool option was missing a 'Var' field, which means
-Wno-switch-bool didn't suppress the warning.

I'm committing the following as obvious.

2014-06-16  Marek Polacek  

PR c/60439
* c.opt (Wswitch-bool): Add Var.

diff --git gcc/c-family/c.opt gcc/c-family/c.opt
index d2e047f..91f8275 100644
--- gcc/c-family/c.opt
+++ gcc/c-family/c.opt
@@ -543,7 +543,7 @@ C ObjC C++ ObjC++ Var(warn_switch_enum) Warning
 Warn about all enumerated switches missing a specific case
 
 Wswitch-bool
-C ObjC C++ ObjC++ Warning Init(1)
+C ObjC C++ ObjC++ Var(warn_switch_bool) Warning Init(1)
 Warn about switches with boolean controlling expression
 
 Wmissing-format-attribute

Marek

Re: Regimplification enhancements 1/3

[Richard Biener]

On Mon, Jun 16, 2014 at 1:45 PM, Bernd Schmidt  wrote:
> On 06/16/2014 01:24 PM, Richard Biener wrote:
>>
>> On Mon, Jun 16, 2014 at 12:56 PM, Bernd Schmidt 
>> wrote:
>>>
>>> For the ptx port, I've needed to write a new pass which ensures all
>>> objects
>>> go into address spaces as required by the machine. This uses the
>>> regimplification code in gimplify-me.c, and that requires some fixes and
>>> upgrades.
>>
>>
>> Can you explain how and why you need re-gimplification here?  IMHO
>> you should apply the necessary steps yourself, not put in GENERIC
>> into GIMPLE and rely on the gimplifier.
>
>
> That's not actually what's happening - the pass modifies the address spaces
> of all objects (local & global vars etc.) and I have one additional patch in
> gimplify.c that recognizes address space mismatches on an ADDR_EXPR (also
> testing a new TYPE_QUAL_AS_IMPLICIT bit to ensure it only triggers in
> situations it's intended to) and inserts the necessary
> ADDR_SPACE_CONVERT_EXPR. With that I can pass all statements through
> regimplification to get a consistent view of the world again.

Uh.  So what do you do?  You'd place an object in a different address-space
and when seeing &foo then simply lie to GIMPLE and say the address
is really not in that address space?  What you you do when such
pointer is dereferenced then?

It looks wrong to me to do such semantic changing transforms in
gimplification.

Richard.

>> You can't turn a x = &foo; into x = (convert)&foo; this way abusing
>> the re-gimplification routines.
>> The re-gimplification routines do _not_ accept random GENERIC
>> operands.  They were written to handle registers becoming non-registers
>> and nothing more.
>
>
> It seems to work fine is all I can say. What it was written for and what it
> can be useful for are two different things.
>
>
> Bernd
>

Re: Patch improving spilling general regs into vector regs

[James Greenhalgh]

On Mon, Jun 16, 2014 at 10:25:36AM +0100, Christophe Lyon wrote:
> Hi,
> 
> This patches causes a failure to build GCC (since commit 211655), on
> all ARM and Aarch64 targets I track.
> 
> I can see failures when building libgcc (_mulsc3.o, _muldc3.o,
> _divdc3.o), the error message being:
> 0xa07f6f crash_signal
> 
> /tmp/214822_1.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/toplev.c:337
> Please submit a full bug report,
> 

I'm also seeing this building libgcc's linux-atomic-64bit.c. Reduced
testcase attached, and a slightly more detailed backtrace:

(gdb) bt
#0  0x in ?? ()
#1  0x0054b626 in assign_by_spills () at 
/home/jamgre01//gcc-clean/gcc/lra-assigns.c:1434
#2  0x0054b8da in lra_assign () at 
/home/jamgre01//gcc-clean/gcc/lra-assigns.c:1499
#3  0x00545dd2 in lra (f=0x0) at /home/jamgre01//gcc-clean/gcc/lra.c:2230
#4  0x004ff7a6 in do_reload () at /home/jamgre01//gcc-clean/gcc/ira.c:5325
#5  0x004ffb4a in (anonymous namespace)::pass_reload::execute (this=0x11bb8e0) 
at /home/jamgre01//gcc-clean/gcc/ira.c:5486
#6  0x005c6da0 in execute_one_pass (pass=0x11bb8e0) at 
/home/jamgre01//gcc-clean/gcc/passes.c:2180
#7  0x005c6f8e in execute_pass_list_1 (pass=0x11bb8e0) at 
/home/jamgre01//gcc-clean/gcc/passes.c:2233
#8  0x005c6fb4 in execute_pass_list_1 (pass=0x11bac80) at 
/home/jamgre01//gcc-clean/gcc/passes.c:2234
#9  0x005c6fea in execute_pass_list (fn=0xb6c63bc8, pass=0x11b8880) at 
/home/jamgre01//gcc-clean/gcc/passes.c:2244
#10 0x002ef55a in expand_function (node=0xb6c582a0) at 
/home/jamgre01//gcc-clean/gcc/cgraphunit.c:1787
#11 0x002efaaa in expand_all_functions () at 
/home/jamgre01//gcc-clean/gcc/cgraphunit.c:1921
#12 0x002f03f4 in compile () at /home/jamgre01//gcc-clean/gcc/cgraphunit.c:2265
#13 0x002f056c in finalize_compilation_unit () at 
/home/jamgre01//gcc-clean/gcc/cgraphunit.c:2342
#14 0x00183b5a in c_write_global_declarations () at 
/home/jamgre01//gcc-clean/gcc/c/c-decl.c:10452
#15 0x006a444e in compile_file () at /home/jamgre01//gcc-clean/gcc/toplev.c:562
#16 0x006a6aa8 in do_compile () at /home/jamgre01//gcc-clean/gcc/toplev.c:1918
#17 0x006a6c34 in toplev_main (argc=4, argv=0xbefff014) at 
/home/jamgre01//gcc-clean/gcc/toplev.c:1994
#18 0x00cdb332 in main (argc=4, argv=0xbefff014) at 
/home/jamgre01//gcc-clean/gcc/main.c:36

Thanks,
James

/* /home/jamgre01/build/clean/./gcc/cc1 besttry.c -O2 -fPIC   */
# 1 "besttry.c"
typedef int(__kernel_cmpxchg64_t)(const long long *oldval,
  const long long *newval, long long *ptr);
long long __attribute__((visibility("hidden")))
__sync_fetch_and_nand_8(long long *ptr, long long val) {
  int failure;
  long long tmp, tmp2;
  do {
tmp = *ptr;
tmp2 = ~(tmp & val);
failure = (*(__kernel_cmpxchg64_t *)0x0f60)(&tmp, &tmp2, ptr);
  } while (failure != 0);
}

Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)

[Yury Gribov]

On 06/16/2014 02:34 PM, Dominique Dhumieres wrote:

Done in r211699.


This breaks bootstrap on x86_64-apple-darwin13:


Hm, perhaps I didn't run full bootstrap after last round of review.
Does attached patch solve the problem for you? I've started bootstrap
but it'll take couple of hours to complete.

-Y
diff --git a/gcc/asan.c b/gcc/asan.c
index 19e1524..281a795 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -1636,6 +1636,13 @@ build_check_stmt (location_t location, tree base, tree len,
 
   gcc_assert (!(size_in_bytes > 0 && !non_zero_len_p));
 
+  if (start_instrumented && end_instrumented)
+{
+  if (!before_p)
+gsi_next (iter);
+  return;
+}
+
   if (len)
 len = unshare_expr (len);
   else
@@ -1735,7 +1742,7 @@ build_check_stmt (location_t location, tree base, tree len,
   gsi_insert_after (&gsi, g, GSI_NEW_STMT);
   tree base_addr = gimple_assign_lhs (g);
 
-  tree t;
+  tree t = NULL_TREE;
   if (real_size_in_bytes >= 8)
 {
   tree shadow = build_shadow_mem_access (&gsi, location, base_addr,

Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)

[Dominique Dhumieres]

I have done the change to tree t = NULL_TREE; then bootstrap fails with

Bootstrap comparison failure!
gcc/plugin.o differs
gcc/toplev.o differs

I'll try your full patch later (currently bootstrapping r211698).

Thanks,

Dominique

[PATCH 4.8 ARM] Backport r210219: "Neon Intrinsics TLC - remove ML"

[Alan Lawrence]

I'm about to backport the ZIP/UZP/TRN fix for ARM big-endian in r211369, that 
patches arm_neon.h, before that we need to remove the OCAML code that 
autogenerates arm_neon.h...ok?


--Alancommit 458ddd61009cef4f3bc0d02c6ea74a1da7a4b505
Author: ramana 
Date:   Thu May 8 14:35:40 2014 +

Backport r210219 from mainline: "Neon intrinsics TLC - remove ML"

	2014-05-08  Ramana Radhakrishnan  

	* config/arm/arm_neon.h: Update comment.
	* config/arm/neon-docgen.ml: Delete.
	* config/arm/neon-gen.ml: Delete.
	* doc/arm-neon-intrinsics.texi: Update comment.

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index c9ab54f..80fa36b 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,14 @@
+2014-06-16  Alan Lawrence  
+
+	Backport r210219 from mainline
+
+	2014-05-08  Ramana Radhakrishnan  
+
+	* config/arm/arm_neon.h: Update comment.
+	* config/arm/neon-docgen.ml: Delete.
+	* config/arm/neon-gen.ml: Delete.
+	* doc/arm-neon-intrinsics.texi: Update comment.
+
 2014-06-13  Peter Bergner  
 
 	Backport from mainline
diff --git a/gcc/config/arm/arm_neon.h b/gcc/config/arm/arm_neon.h
index 4d945ce..4381c2a 100644
--- a/gcc/config/arm/arm_neon.h
+++ b/gcc/config/arm/arm_neon.h
@@ -1,5 +1,4 @@
-/* ARM NEON intrinsics include file. This file is generated automatically
-   using neon-gen.ml.  Please do not edit manually.
+/* ARM NEON intrinsics include file.
 
Copyright (C) 2006-2013 Free Software Foundation, Inc.
Contributed by CodeSourcery.
diff --git a/gcc/config/arm/neon-docgen.ml b/gcc/config/arm/neon-docgen.ml
deleted file mode 100644
index f17314f..000
--- a/gcc/config/arm/neon-docgen.ml
+++ /dev/null
@@ -1,344 +0,0 @@
-(* ARM NEON documentation generator.
-
-   Copyright (C) 2006-2013 Free Software Foundation, Inc.
-   Contributed by CodeSourcery.
-
-   This file is part of GCC.
-
-   GCC is free software; you can redistribute it and/or modify it under
-   the terms of the GNU General Public License as published by the Free
-   Software Foundation; either version 3, or (at your option) any later
-   version.
-
-   GCC is distributed in the hope that it will be useful, but WITHOUT ANY
-   WARRANTY; without even the implied warranty of MERCHANTABILITY or
-   FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
-   for more details.
-
-   You should have received a copy of the GNU General Public License
-   along with GCC; see the file COPYING3.  If not see
-   .
-
-   This is an O'Caml program.  The O'Caml compiler is available from:
-
- http://caml.inria.fr/
-
-   Or from your favourite OS's friendly packaging system. Tested with version
-   3.09.2, though other versions will probably work too.
-
-   Compile with:
- ocamlc -c neon.ml
- ocamlc -o neon-docgen neon.cmo neon-docgen.ml
-
-   Run with:
- /path/to/neon-docgen /path/to/gcc/doc/arm-neon-intrinsics.texi
-*)
-
-open Neon
-
-(* The combined "ops" and "reinterp" table.  *)
-let ops_reinterp = reinterp @ ops
-
-(* Helper functions for extracting things from the "ops" table.  *)
-let single_opcode desired_opcode () =
-  List.fold_left (fun got_so_far ->
-  fun row ->
-match row with
-  (opcode, _, _, _, _, _) ->
-if opcode = desired_opcode then row :: got_so_far
-   else got_so_far
- ) [] ops_reinterp
-
-let multiple_opcodes desired_opcodes () =
-  List.fold_left (fun got_so_far ->
-  fun desired_opcode ->
-(single_opcode desired_opcode ()) @ got_so_far)
- [] desired_opcodes
-
-let ldx_opcode number () =
-  List.fold_left (fun got_so_far ->
-  fun row ->
-match row with
-  (opcode, _, _, _, _, _) ->
-match opcode with
-  Vldx n | Vldx_lane n | Vldx_dup n when n = number ->
-row :: got_so_far
-  | _ -> got_so_far
- ) [] ops_reinterp
-
-let stx_opcode number () =
-  List.fold_left (fun got_so_far ->
-  fun row ->
-match row with
-  (opcode, _, _, _, _, _) ->
-match opcode with
-  Vstx n | Vstx_lane n when n = number ->
-row :: got_so_far
-  | _ -> got_so_far
- ) [] ops_reinterp
-
-let tbl_opcode () =
-  List.fold_left (fun got_so_far ->
-  fun row ->
-match row with
-  (opcode, _, _, _, _, _) ->
-match opcode with
-  Vtbl _ -> row :: got_so_far
-  | _ -> got_so_far
- ) [] ops_reinterp
-
-let tbx_opcode () =
-  List.fold_left (fun got_so_far ->
-  fun row ->
-   

[RFC][ARM]: Fix reload spill failure (PR 60617)

[Venkataramanan Kumar]

Hi Maintainers,

This patch fixes the PR 60617 that occurs when we turn on reload pass
in thumb2 mode.

It occurs for the pattern "*ior_scc_scc" that gets generated for the 3
argument of the below function call.

JIT:emitStoreInt32(dst,regT0m, (op1 == dst || op2 == dst)));


(snip---)
(insn 634 633 635 27 (parallel [
(set (reg:SI 3 r3)
(ior:SI (eq:SI (reg/v:SI 110 [ dst ]) <== This operand
r5 is registers gets assigned
(reg/v:SI 112 [ op2 ]))
(eq:SI (reg/v:SI 110 [ dst ]) <== This operand
(reg/v:SI 111 [ op1 ]
(clobber (reg:CC 100 cc))
]) ../Source/JavaScriptCore/jit/JITArithmetic32_64.cpp:179 300
{*ior_scc_scc
(snip---)

The issue here is that the above pattern demands 5 registers (LO_REGS).

But when we are in reload, registers r0 is used for pointer to the
class, r1 and r2 for first and second argument. r7 is used for stack
pointer.

So we are left with r3,r4,r5 and r6. But the above patterns needs five
LO_REGS. Hence we get spill failure when processing the last register
operand in that pattern,

In ARM port,  TARGET_LIKELY_SPILLED_CLASS is defined for Thumb-1 and
for thumb 2 mode there is mention of using LO_REG in the comment as
below.

"Care should be taken to avoid adding thumb-2 patterns that require
many low registers"

So conservative fix is not to allow this pattern for Thumb-2 mode.

I allowed these pattern for Thumb2 when we have constant operands for
comparison. That makes the target tests arm/thum2-cond-cmp-1.c to
thum2-cond-cmp-4.c pass.

Regression tested with gcc 4.9 branch since in trunk this bug is
masked revision 209897.

Please provide your suggestion on this patch

regards,
Venkat.
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index 0284f95..e8fbb11 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -10654,7 +10654,7 @@
 [(match_operand:SI 4 "s_register_operand" "r")
  (match_operand:SI 5 "arm_add_operand" "rIL")])))
(clobber (reg:CC CC_REGNUM))]
-  "TARGET_32BIT
+  "TARGET_ARM
&& (arm_select_dominance_cc_mode (operands[3], operands[6], DOM_CC_X_OR_Y)
!= CCmode)"
   "#"
@@ -10675,6 +10675,36 @@
(set_attr "type" "multiple")]
 )
 
+(define_insn_and_split "*ior_scc_scc_imm"
+  [(set (match_operand:SI 0 "s_register_operand" "=Ts")
+(ior:SI (match_operator:SI 3 "arm_comparison_operator"
+ [(match_operand:SI 1 "s_register_operand" "r")
+  (match_operand:SI 2 "arm_addimm_operand" "IL")])
+(match_operator:SI 6 "arm_comparison_operator"
+ [(match_operand:SI 4 "s_register_operand" "r")
+  (match_operand:SI 5 "arm_addimm_operand" "IL")])))
+   (clobber (reg:CC CC_REGNUM))]
+  "TARGET_THUMB2
+   && (arm_select_dominance_cc_mode (operands[3], operands[6], DOM_CC_X_OR_Y)
+   != CCmode)"
+  "#"
+  "TARGET_THUMB2 && reload_completed"
+  [(set (match_dup 7)
+(compare
+ (ior:SI
+  (match_op_dup 3 [(match_dup 1) (match_dup 2)])
+  (match_op_dup 6 [(match_dup 4) (match_dup 5)]))
+ (const_int 0)))
+   (set (match_dup 0) (ne:SI (match_dup 7) (const_int 0)))]
+  "operands[7]
+ = gen_rtx_REG (arm_select_dominance_cc_mode (operands[3], operands[6],
+  DOM_CC_X_OR_Y),
+CC_REGNUM);"
+  [(set_attr "conds" "clob")
+   (set_attr "length" "16")
+   (set_attr "type" "multiple")]
+)
+
 ; If the above pattern is followed by a CMP insn, then the compare is 
 ; redundant, since we can rework the conditional instruction that follows.
 (define_insn_and_split "*ior_scc_scc_cmp"
@@ -10714,7 +10744,7 @@
 [(match_operand:SI 4 "s_register_operand" "r")
  (match_operand:SI 5 "arm_add_operand" "rIL")])))
(clobber (reg:CC CC_REGNUM))]
-  "TARGET_32BIT
+  "TARGET_ARM 
&& (arm_select_dominance_cc_mode (operands[3], operands[6], DOM_CC_X_AND_Y)
!= CCmode)"
   "#"
@@ -10737,6 +10767,38 @@
(set_attr "type" "multiple")]
 )
 
+(define_insn_and_split "*and_scc_scc_imm"
+  [(set (match_operand:SI 0 "s_register_operand" "=Ts")
+(and:SI (match_operator:SI 3 "arm_comparison_operator"
+ [(match_operand:SI 1 "s_register_operand" "r")
+  (match_operand:SI 2 "arm_addimm_operand" "IL")])
+(match_operator:SI 6 "arm_comparison_operator"
+ [(match_operand:SI 4 "s_register_operand" "r")
+  (match_operand:SI 5 "arm_addimm_operand" "IL")])))
+   (clobber (reg:CC CC_REGNUM))]
+  "TARGET_THUMB2 
+   && (arm_select_dominance_cc_mode (operands[3], operands[6], DOM_CC_X_AND_Y)
+   != CCmode)"
+  "#"
+  "TARGET_THUMB2 && reload_completed
+   && (arm_select_dominance_cc_mode (operands[3], operands[6], DOM_CC_X_AND_Y)
+   != CCmode)"
+  [(set (match_dup 7)
+(compare
+ (and:SI
+  (match

Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)

[Yury Gribov]

On 06/16/2014 04:52 PM, Dominique Dhumieres wrote:

I have done the change to tree t = NULL_TREE; then bootstrap fails with

Bootstrap comparison failure!
gcc/plugin.o differs
gcc/toplev.o differs


Interesting, I didn't know that asan.c runs during normal bootstrap.

-Y

Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)

[Jakub Jelinek]

On Mon, Jun 16, 2014 at 04:57:54PM +0400, Yury Gribov wrote:
> On 06/16/2014 04:52 PM, Dominique Dhumieres wrote:
> >I have done the change to tree t = NULL_TREE; then bootstrap fails with
> >
> >Bootstrap comparison failure!
> >gcc/plugin.o differs
> >gcc/toplev.o differs
> 
> Interesting, I didn't know that asan.c runs during normal bootstrap.

It does not.  So the above is IMHO very likely completely unrelated, perhaps
Darwin specific, issue.

Jakub

[patch libatomic]: Add basic support for mingw targets

[Kai Tietz]

Hello,

this patch adds basic support for libatomic for mingw targets using
win32 and for mingw targets using posix threading model.

The win32 implemenation might need for initialization of mutexes a
critical section.  If issue occures we can still add that.  For now
all testcases are passing for native and posix-threading model mingw
(32-bit and 64-bit).

ChangeLog

2014-06-16  Kai Tietz  

* Makefile.am (libatomic_la_LDFLAGS): Add lt_host_flags.
* configure.ac (ACX_LT_HOST_FLAGS): New.
(target_thread_file): New.
* configure.tgt (mingw): Add mingw support.
* config/mingw/host-config.h: New file.
* config/mingw/lock.c: Likewise.
* Makefile.in: Regenerated.
* configure: Likewise.
* aclocal.m4: Likewise.
* testsuite/Makefile.in: Likewise.

Tested for i686-w64-mingw32, and x86_64-w64-mingw32.  Additionally
done regression test for x86_64-unknown-linux-gnu.

Ok for apply?

Regards,
Kai

Index: Makefile.am
===
--- Makefile.am(Revision 211705)
+++ Makefile.am(Arbeitskopie)
@@ -66,7 +66,7 @@ libatomic_version_dep =
 endif
 libatomic_version_info = -version-info $(libtool_VERSION)

-libatomic_la_LDFLAGS = $(libatomic_version_info) $(libatomic_version_script)
+libatomic_la_LDFLAGS = $(libatomic_version_info)
$(libatomic_version_script) $(lt_host_flags)
 libatomic_la_SOURCES = gload.c gstore.c gcas.c gexch.c glfree.c lock.c init.c \
 fenv.c

Index: config/mingw/host-config.h
===
--- config/mingw/host-config.h(Revision 0)
+++ config/mingw/host-config.h(Arbeitskopie)
@@ -0,0 +1,54 @@
+/* Copyright (C) 2014 Free Software Foundation, Inc.
+   Contributed by Kai Tietz .
+
+   This file is part of the GNU Atomic Library (libatomic).
+
+   Libatomic is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   Libatomic is distributed in the hope that it will be useful, but WITHOUT ANY
+   WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+   FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+   more details.
+
+   Under Section 7 of GPL version 3, you are granted additional
+   permissions described in the GCC Runtime Library Exception, version
+   3.1, as published by the Free Software Foundation.
+
+   You should have received a copy of the GNU General Public License and
+   a copy of the GCC Runtime Library Exception along with this program;
+   see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
+   .  */
+
+/* Included after all more target-specific host-config.h.  */
+
+#ifndef protect_start_end
+# ifdef HAVE_ATTRIBUTE_VISIBILITY
+#  pragma GCC visibility push(hidden)
+# endif
+
+void libat_lock_1 (void *ptr);
+void libat_unlock_1 (void *ptr);
+
+static inline UWORD
+protect_start (void *ptr)
+{
+  libat_lock_1 (ptr);
+  return 0;
+}
+
+static inline void
+protect_end (void *ptr, UWORD dummy UNUSED)
+{
+  libat_unlock_1 (ptr);
+}
+
+# define protect_start_end 1
+# ifdef HAVE_ATTRIBUTE_VISIBILITY
+#  pragma GCC visibility pop
+# endif
+#endif /* protect_start_end */
+
+#include_next 

Eigenschaftsänderungen: config/mingw/host-config.h
___
Added: svn:eol-style
## -0,0 +1 ##
+native
\ No newline at end of property
Added: svn:mime-type
## -0,0 +1 ##
+text/plain
\ No newline at end of property
Index: config/mingw/lock.c
===
--- config/mingw/lock.c(Revision 0)
+++ config/mingw/lock.c(Arbeitskopie)
@@ -0,0 +1,124 @@
+/* Copyright (C) 2014 Free Software Foundation, Inc.
+   Contributed by Kai Tietz .
+
+   This file is part of the GNU Atomic Library (libatomic).
+
+   Libatomic is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   Libatomic is distributed in the hope that it will be useful, but WITHOUT ANY
+   WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+   FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+   more details.
+
+   Under Section 7 of GPL version 3, you are granted additional
+   permissions described in the GCC Runtime Library Exception, version
+   3.1, as published by the Free Software Foundation.
+
+   You should have received a copy of the GNU General Public License and
+   a copy of the GCC Runtime Library Exception along with this program;
+   see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
+   .  */
+
+#define UWORD __shadow_UWORD
+#

[PATCH] Fix LIM PHI hosting cost

[Richard Biener]

There are two basic errors in the computation - the first is when
min_cost is never adjusted it remains at UINT_MAX (happens when
all PHI arguments are not defined in any loop).  Second is if
the PHI argument is not defined in the same loop as the PHI - then
we accumulate the cost of already invariant stmts (inconsistently
with the LIM cost modelling).

Bootstrap and regtest running on x86_64-unknown-linux-gnu.

Richard.

2014-06-16  Richard Biener  

* tree-ssa-loop-im.c (determine_max_movement): Adjust cost
of PHI node moving.

* gcc.dg/tree-ssa/ssa-lim-12.c: New testcase.

Index: gcc/tree-ssa-loop-im.c
===
*** gcc/tree-ssa-loop-im.c  (revision 211698)
--- gcc/tree-ssa-loop-im.c  (working copy)
*** determine_max_movement (gimple stmt, boo
*** 729,742 
}
  if (!add_dependency (val, lim_data, loop, false))
return false;
! def_data = get_lim_data (SSA_NAME_DEF_STMT (val));
! if (def_data)
{
! min_cost = MIN (min_cost, def_data->cost);
! total_cost += def_data->cost;
}
}
  
lim_data->cost += min_cost;
  
if (gimple_phi_num_args (stmt) > 1)
--- 732,752 
}
  if (!add_dependency (val, lim_data, loop, false))
return false;
! 
! gimple def_stmt = SSA_NAME_DEF_STMT (val);
! if (gimple_bb (def_stmt)
! && gimple_bb (def_stmt)->loop_father == loop)
{
! def_data = get_lim_data (def_stmt);
! if (def_data)
!   {
! min_cost = MIN (min_cost, def_data->cost);
! total_cost += def_data->cost;
!   }
}
}
  
+   min_cost = MIN (min_cost, total_cost);
lim_data->cost += min_cost;
  
if (gimple_phi_num_args (stmt) > 1)
Index: gcc/testsuite/gcc.dg/tree-ssa/ssa-lim-12.c
===
*** gcc/testsuite/gcc.dg/tree-ssa/ssa-lim-12.c  (revision 0)
--- gcc/testsuite/gcc.dg/tree-ssa/ssa-lim-12.c  (working copy)
***
*** 0 
--- 1,27 
+ /* { dg-do compile } */
+ /* { dg-options "-O -fdump-tree-lim1" } */
+ 
+ int a[1024];
+ 
+ void foo (int x, int z)
+ {
+   int i;
+   int y = -x;
+   for (i = 0; i < 1024; ++i)
+ a[i] = x ? y : z;
+ }
+ 
+ void bar (int x, int z)
+ {
+   int j;
+   for (j = 0; j < 1024; ++j)
+ {
+   int i;
+   int y = -j + z;
+   for (i = 0; i < 1024; ++i)
+   a[i] = x ? y : j;
+ }
+ }
+ 
+ /* { dg-final { scan-tree-dump-times "!= 0 ? " 2 "lim1" } } */
+ /* { dg-final { cleanup-tree-dump "lim1" } } */

[PATCH] Remove fully propagated stmts during substitute_and_fold

[Richard Biener]

This adjusts substitute_and_fold similar to how the FRE/PRE propagation 
stage works which means we properly cleanup after ourselves with
respect to DCE (if we are allowed to).  The patch removes the
poor-mans DCE code (checking for has_zero_uses) as that doesn't
really belong here and as it doesn't work reliably anymore with
doing this in a DOM walk (which is to make debug stmt generation
happy).

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

Richard.

2014-06-16  Richard Biener  

* tree-ssa-propagate.c: Include domwalk.h.
(substitute_and_fold): Outline main worker into a domwalker ...
(substitute_and_fold_dom_walker::before_dom_children): ... here.
Schedule stmts we can fully propagate for removal.  Remove
poor-mans DCE.
(substitute_and_fold): Apply a dominator walk to perform
substitution.  Process stmts scheduled for removal here.

* gcc.dg/tree-ssa/20041122-1.c: Adjust.
* gcc.dg/tree-ssa/forwprop-21.c: Likewise.
* gcc.dg/tree-ssa/vrp35.c: Revert previous adjustments.
* gcc.dg/tree-ssa/vrp36.c: Likewise.
* gcc.dg/vect/nodump-forwprop-22.c: Adjust.

Index: gcc/tree-ssa-propagate.c
===
*** gcc/tree-ssa-propagate.c.orig   2014-06-16 14:16:21.325565287 +0200
--- gcc/tree-ssa-propagate.c2014-06-16 14:26:20.748524017 +0200
***
*** 49,54 
--- 49,55 
  #include "tree-ssa-propagate.h"
  #include "langhooks.h"
  #include "value-prof.h"
+ #include "domwalk.h"
  
  /* This file implements a generic value propagation engine based on
 the same propagation used by the SSA-CCP algorithm [1].
*** replace_phi_args_in (gimple phi, ssa_pro
*** 1017,1241 
  }
  
  
! /* Perform final substitution and folding of propagated values.
! 
!PROP_VALUE[I] contains the single value that should be substituted
!at every use of SSA name N_I.  If PROP_VALUE is NULL, no values are
!substituted.
! 
!If FOLD_FN is non-NULL the function will be invoked on all statements
!before propagating values for pass specific simplification.
  
!DO_DCE is true if trivially dead stmts can be removed.
  
!If DO_DCE is true, the statements within a BB are walked from
!last to first element.  Otherwise we scan from first to last element.
! 
!Return TRUE when something changed.  */
  
! bool
! substitute_and_fold (ssa_prop_get_value_fn get_value_fn,
!ssa_prop_fold_stmt_fn fold_fn,
!bool do_dce)
  {
!   basic_block bb;
!   bool something_changed = false;
!   unsigned i;
  
!   if (!get_value_fn && !fold_fn)
! return false;
  
!   if (dump_file && (dump_flags & TDF_DETAILS))
! fprintf (dump_file, "\nSubstituting values and folding statements\n\n");
  
!   memset (&prop_stats, 0, sizeof (prop_stats));
  
!   /* Substitute lattice values at definition sites.  */
!   if (get_value_fn)
! for (i = 1; i < num_ssa_names; ++i)
!   {
!   tree name = ssa_name (i);
!   tree val;
!   gimple def_stmt;
!   gimple_stmt_iterator gsi;
! 
!   if (!name
!   || virtual_operand_p (name))
! continue;
! 
!   def_stmt = SSA_NAME_DEF_STMT (name);
!   if (gimple_nop_p (def_stmt)
!   /* Do not substitute ASSERT_EXPR rhs, this will confuse VRP.  */
!   || (gimple_assign_single_p (def_stmt)
!   && gimple_assign_rhs_code (def_stmt) == ASSERT_EXPR)
!   || !(val = (*get_value_fn) (name))
!   || !may_propagate_copy (name, val))
! continue;
! 
!   gsi = gsi_for_stmt (def_stmt);
!   if (is_gimple_assign (def_stmt))
! {
!   gimple_assign_set_rhs_with_ops (&gsi, TREE_CODE (val),
!   val, NULL_TREE);
!   gcc_assert (gsi_stmt (gsi) == def_stmt);
!   if (maybe_clean_eh_stmt (def_stmt))
! gimple_purge_dead_eh_edges (gimple_bb (def_stmt));
!   update_stmt (def_stmt);
! }
!   else if (is_gimple_call (def_stmt))
! {
!   int flags = gimple_call_flags (def_stmt);
! 
!   /* Don't optimize away calls that have side-effects.  */
!   if ((flags & (ECF_CONST|ECF_PURE)) == 0
!   || (flags & ECF_LOOPING_CONST_OR_PURE))
! continue;
!   if (update_call_from_tree (&gsi, val)
!   && maybe_clean_or_replace_eh_stmt (def_stmt, gsi_stmt (gsi)))
! gimple_purge_dead_eh_edges (gimple_bb (gsi_stmt (gsi)));
! }
!   else if (gimple_code (def_stmt) == GIMPLE_PHI)
! {
!   gimple new_stmt = gimple_build_assign (name, val);
!   gimple_stmt_iterator gsi2;
!   gsi2 = gsi_after_labels (gimple_bb (def_stmt));
!   gsi_insert_before (&gsi2, new_stmt, GSI_SAME_STMT);
!   remove_phi_node (&gsi, false);
! }
! 
!   something_changed = true;
!   }
! 
!   /* Pr

[PATCH] Fix PR61482

[Richard Biener]

The following fixes PR61482, avoiding to call set_value_range
with [-INF(OVF), +INF(OVF)].

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

Richard.

Index: gcc/tree-vrp.c
===
--- gcc/tree-vrp.c  (revision 211698)
+++ gcc/tree-vrp.c  (working copy)
@@ -3892,15 +3892,6 @@ adjust_range_with_scev (value_range_t *v
max = init;
   else
min = init;
-
-  /* If we would create an invalid range, then just assume we
-know absolutely nothing.  This may be over-conservative,
-but it's clearly safe, and should happen only in unreachable
- parts of code, or for invalid programs.  */
-  if (compare_values (min, max) == 1)
-   return;
-
-  set_value_range (vr, VR_RANGE, min, max, vr->equiv);
 }
   else if (vr->type == VR_RANGE)
 {
@@ -3933,16 +3924,20 @@ adjust_range_with_scev (value_range_t *v
  || compare_values (tmax, max) == -1)
max = tmax;
}
+}
+  else
+return;
 
-  /* If we just created an invalid range with the minimum
-greater than the maximum, we fail conservatively.
-This should happen only in unreachable
-parts of code, or for invalid programs.  */
-  if (compare_values (min, max) == 1)
-   return;
+  /* If we just created an invalid range with the minimum
+ greater than the maximum, we fail conservatively.
+ This should happen only in unreachable
+ parts of code, or for invalid programs.  */
+  if (compare_values (min, max) == 1
+  || (is_negative_overflow_infinity (min)
+ && is_positive_overflow_infinity (max)))
+return;
 
-  set_value_range (vr, VR_RANGE, min, max, vr->equiv);
-}
+  set_value_range (vr, VR_RANGE, min, max, vr->equiv);
 }
 
 
Index: gcc/testsuite/g++.dg/torture/pr61482.C
===
--- gcc/testsuite/g++.dg/torture/pr61482.C  (revision 0)
+++ gcc/testsuite/g++.dg/torture/pr61482.C  (working copy)
@@ -0,0 +1,29 @@
+// { dg-do compile }
+
+class A
+{
+public:
+int m_fn1 ();
+};
+class B
+{
+  void m_fn2 (const int &p1);
+  A mThebesLayerDataStack;
+};
+int b, c;
+void B::m_fn2 (const int &p1)
+{
+  if (c && b)
+{
+  int i;
+  i = mThebesLayerDataStack.m_fn1 ();
+  for (; i >= 0;)
+   {
+ ++i;
+ break;
+   }
+  --i;
+  for (; i >= 0; --i)
+   mThebesLayerDataStack.m_fn1 ();
+}
+}

Re: [PATCH] Fix PR61482

[Richard Biener]

On Mon, 16 Jun 2014, Richard Biener wrote:

> 
> The following fixes PR61482, avoiding to call set_value_range
> with [-INF(OVF), +INF(OVF)].
> 
> Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

Err, finger faster than eye.  Here's the changelog.

Richard.

2014-06-16  Richard Biener  

PR tree-optimization/61482
* tree-vrp.c (adjust_range_with_scev): Avoid setting of
[-INF(OVF), +INF(OVF)] range.

* g++.dg/torture/pr61482.C: New testcase.


> Richard.
> 
> Index: gcc/tree-vrp.c
> ===
> --- gcc/tree-vrp.c(revision 211698)
> +++ gcc/tree-vrp.c(working copy)
> @@ -3892,15 +3892,6 @@ adjust_range_with_scev (value_range_t *v
>   max = init;
>else
>   min = init;
> -
> -  /* If we would create an invalid range, then just assume we
> -  know absolutely nothing.  This may be over-conservative,
> -  but it's clearly safe, and should happen only in unreachable
> - parts of code, or for invalid programs.  */
> -  if (compare_values (min, max) == 1)
> - return;
> -
> -  set_value_range (vr, VR_RANGE, min, max, vr->equiv);
>  }
>else if (vr->type == VR_RANGE)
>  {
> @@ -3933,16 +3924,20 @@ adjust_range_with_scev (value_range_t *v
> || compare_values (tmax, max) == -1)
>   max = tmax;
>   }
> +}
> +  else
> +return;
>  
> -  /* If we just created an invalid range with the minimum
> -  greater than the maximum, we fail conservatively.
> -  This should happen only in unreachable
> -  parts of code, or for invalid programs.  */
> -  if (compare_values (min, max) == 1)
> - return;
> +  /* If we just created an invalid range with the minimum
> + greater than the maximum, we fail conservatively.
> + This should happen only in unreachable
> + parts of code, or for invalid programs.  */
> +  if (compare_values (min, max) == 1
> +  || (is_negative_overflow_infinity (min)
> +   && is_positive_overflow_infinity (max)))
> +return;
>  
> -  set_value_range (vr, VR_RANGE, min, max, vr->equiv);
> -}
> +  set_value_range (vr, VR_RANGE, min, max, vr->equiv);
>  }
>  
>  
> Index: gcc/testsuite/g++.dg/torture/pr61482.C
> ===
> --- gcc/testsuite/g++.dg/torture/pr61482.C(revision 0)
> +++ gcc/testsuite/g++.dg/torture/pr61482.C(working copy)
> @@ -0,0 +1,29 @@
> +// { dg-do compile }
> +
> +class A
> +{
> +public:
> +int m_fn1 ();
> +};
> +class B
> +{
> +  void m_fn2 (const int &p1);
> +  A mThebesLayerDataStack;
> +};
> +int b, c;
> +void B::m_fn2 (const int &p1)
> +{
> +  if (c && b)
> +{
> +  int i;
> +  i = mThebesLayerDataStack.m_fn1 ();
> +  for (; i >= 0;)
> + {
> +   ++i;
> +   break;
> + }
> +  --i;
> +  for (; i >= 0; --i)
> + mThebesLayerDataStack.m_fn1 ();
> +}
> +}
> 

-- 
Richard Biener 
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer

Re: [PATCH] proposed fix for bug # 61144

[Rich Felker]

On Mon, Jun 16, 2014 at 11:06:04AM +0200, Jan Hubicka wrote:
> > 
> > Are the attached files acceptable?
> 
> The testcase looks OK to me, but it already should be fixed on mainline
> by patch https://gcc.gnu.org/ml/gcc-patches/2014-05/msg01315.html that
> prevents dummy to be marked as constant. 
> 
> You can however modify the testcase to have
> __attribute__ ((weak)) const int foo=0;

And the same for weak alias rather than straight weak definition like
the above?

> This needs your decl_replaceable change to not be optimized to if (0),
> because of the explicit const modifier.

The case I care about actually has "dummy" as const (with the intent
that it be allocated in a read-only section if the dummy definition is
used). So for me it's important that this regression be fixed too.

> I did not change ctor_for_folding to reject variables above as I was not quite
> sure we want to support this kind of interposition and I am still not quite 
> certain.
> C++ is quite clear about the transformation replacing initialized const by 
> its value.

My concern is about C, not C++. This kind of interposition has always
been supported in unix C, even prior to GCC, going back to sysv or
earlier, as a documented feature (historically #pragma weak). It
should not regress. If fixing it results in an regression with regards
to optimizability of C++, perhaps this could be made
language-specific, or (better) the C++ front-end could add an
additional internal-use-only attribute to weak definitions it
generates internally that permits constant-folding them, while not
breaking the semantics for weak definitions provided by the user at
the source level.

Rich

Re: Go patch committed: Pass -t to native linker on Solaris

[Ian Lance Taylor]

On Mon, Jun 16, 2014 at 3:56 AM, Rainer Orth
 wrote:
>
> Works fine, thanks.  Just before your patch arrived, I meant to test the
> following, slightly more general patch.  Perhaps it's an option to
> handle other quirks like this?

This approach is fine with me, but the target macro should be
documented somwhere.  We need to document that multiple options have
to be separated with commas, not spaces.

Ian

Re: [PATCH] gcc/dwarf2asm.c: Add static_output_delta() with var_list for dw2_asm_output_delta()

[Joseph S. Myers]

On Mon, 16 Jun 2014, Chen Gang wrote:

> +static_output_delta (int size, const char *lab1, const char *lab2,

static_ is not a helpful naming convention, since the fact that a function 
is static is nothing to do with what that function does.  Try something 
like dw2_asm_voutput_delta to indicate a va_list version of 
dw2_asm_output_delta.

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

Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)

[Dominique Dhumieres]

I have bootstrapped r211707 with the full patch (among others!).

> It does not.  So the above is IMHO very likely completely unrelated,
> perhaps Darwin specific, issue.

More likely related to the fact that I only resumed bootstrap after the change
tree t = NULL_TREE; and an update from r211700 to r211701, instead of starting
a clean bootstrap.

Cheers,

Dominiaue

Re: [PATCH 8/8] Add a common .md file and define standard constraints there

[Steve Ellcey]

On Sat, 2014-06-14 at 08:49 +0100, Richard Sandiford wrote:

> The patch below fixes the testcase.  Please could you give it a spin
> and see whether there's any other fallout?  I assume this would have
> shown up in a testsuite run if you'd been able to get that far.
> 
> Thanks,
> Richard

The patch fixed my build and the testsuite results looked good too.

Steve Ellcey
sell...@mips.com

Re: Go patch committed: Pass -t to native linker on Solaris

[Ian Lance Taylor]

On Mon, Jun 16, 2014 at 6:58 AM, Ian Lance Taylor  wrote:
> On Mon, Jun 16, 2014 at 3:56 AM, Rainer Orth
>  wrote:
>>
>> Works fine, thanks.  Just before your patch arrived, I meant to test the
>> following, slightly more general patch.  Perhaps it's an option to
>> handle other quirks like this?
>
> This approach is fine with me, but the target macro should be
> documented somwhere.  We need to document that multiple options have
> to be separated with commas, not spaces.

Also I think it would be better to write it along the lines of the
following, to allow runtime determination of whether the option is
needed.


#ifdef TARGET_GO_LINK_OPTIONS
  /* Pass target specific linker options if present.  */
  {
const char *p = TARGET_GO_LINK_OPTIONS;
if (p != NULL && *p != '\0')
  generate_option (OPT_Wl_, TARGET_GO_LINK_OPTIONS, 1, CL_DRIVER,
   &new_decoded_options[j++]);
  }
#endif


Ian

[Patch LRA] PR 61522 - Handle NULL targetm.spill_class

[Ramana Radhakrishnan]

Hi,

	This handles NULL targetm.spill_class in assign_by_spills. This showed 
up as a segfault during a build for arm-none-linux-gnueabi(hf).


Fix pre-approved by richi on IRC , verified that bootstrap continues 
from where things broke further on a tree (that reverts 211600 which is 
the next breakage causing commit on arm-none-linux-gnueabihf)


I'll apply this in about 45 minutes when I get back to my desk if no one 
objects.


regards
Ramana


2014-06-16  Ramana Radhakrishnan  

PR rtl-optimization/61522
* lra-assigns.c (assign_by_spills): Handle NULL 
targetm.spill_class.
diff --git a/gcc/lra-assigns.c b/gcc/lra-assigns.c
index cea4c33..5de18e1 100644
--- a/gcc/lra-assigns.c
+++ b/gcc/lra-assigns.c
@@ -1420,7 +1420,7 @@ assign_by_spills (void)
 alternatives of insns containing the pseudo.  */
  bitmap_set_bit (&changed_pseudo_bitmap, regno);
}
- else
+ else if (targetm.spill_class)
{
  enum reg_class rclass = lra_get_allocno_class (regno);
  enum reg_class spill_class;
@@ -1438,12 +1438,12 @@ assign_by_spills (void)
  if (hard_regno < 0)
regno_allocno_class_array[regno] = rclass;
  else
-   {
- setup_reg_classes
-   (regno, spill_class, spill_class, spill_class);
- assign_hard_regno (hard_regno, regno);
- bitmap_set_bit (&changed_pseudo_bitmap, regno);
-   }
+ {
+   setup_reg_classes
+ (regno, spill_class, spill_class, spill_class);
+   assign_hard_regno (hard_regno, regno);
+   bitmap_set_bit (&changed_pseudo_bitmap, regno);
+ }
}
}
 }

Re: Another AIX Bootstrap failure

[Jan Hubicka]

> Honza,
> 
> Thanks for reverting the patch. I will check if this resolves the
> current bootstrap problem.
> 
> I was suggesting that you create a branch for all of the visibility
> changes to make it easier to track the various original patches and
> later correction patches from you.
> 
> I don't know if the gen* programs hang because of the visibility
> changes or because of the change in sections. The change in sections
> could conflict with the GCC code to handle AIX XCOFF CSECTs for
> functions.
> 
> AIX recently added support for ELF-like visibility. AIX previously
> supported the equivalent of visibility through "export" files. The
> recent problems could be due to issues with assembly file ordering,
> but they also could be related to the visibility changes affecting the
> way that GCC emits code to branch to global functions.

I comitted the revert now (my original testing got struct on ICE in
auto-inc-dec pass that is unrelated).  I probably won't have time to analye
what went wrong until Wednesday. The patch did not really play with
ELF visibilities it was again related to bringing symbols local.
I tried a case disabling the new conditional on clearning user section
but that did not help. The patch basically collected few cleanups
and fixes of corner case.  Last change is fix in the inline heuristics
to not try to enale DECL_ONE_ONLY section sharing on targets not supporting
it.  Obviously it should not lead to wrong code, since any inlining decision
change should not, but I am testing it independnely now.

Honza

Re: [Patch LRA] PR 61522 - Handle NULL targetm.spill_class

[Jakub Jelinek]

On Mon, Jun 16, 2014 at 04:05:52PM +0100, Ramana Radhakrishnan wrote:
>   This handles NULL targetm.spill_class in assign_by_spills. This showed 
> up
> as a segfault during a build for arm-none-linux-gnueabi(hf).
> 
> Fix pre-approved by richi on IRC , verified that bootstrap continues from
> where things broke further on a tree (that reverts 211600 which is the next
> breakage causing commit on arm-none-linux-gnueabihf)
> 
> I'll apply this in about 45 minutes when I get back to my desk if no one
> objects.

Please only commit the first hunk, not the second.  That looks like
incorrectly indenting something that has been correctly indented before.

> 2014-06-16  Ramana Radhakrishnan  
> 
> PR rtl-optimization/61522
> * lra-assigns.c (assign_by_spills): Handle NULL targetm.spill_class.

> diff --git a/gcc/lra-assigns.c b/gcc/lra-assigns.c
> index cea4c33..5de18e1 100644
> --- a/gcc/lra-assigns.c
> +++ b/gcc/lra-assigns.c
> @@ -1420,7 +1420,7 @@ assign_by_spills (void)
>alternatives of insns containing the pseudo.  */
> bitmap_set_bit (&changed_pseudo_bitmap, regno);
>   }
> -   else
> +   else if (targetm.spill_class)
>   {
> enum reg_class rclass = lra_get_allocno_class (regno);
> enum reg_class spill_class;
> @@ -1438,12 +1438,12 @@ assign_by_spills (void)
> if (hard_regno < 0)
>   regno_allocno_class_array[regno] = rclass;
> else
> - {
> -   setup_reg_classes
> - (regno, spill_class, spill_class, spill_class);
> -   assign_hard_regno (hard_regno, regno);
> -   bitmap_set_bit (&changed_pseudo_bitmap, regno);
> - }
> +   {
> + setup_reg_classes
> +   (regno, spill_class, spill_class, spill_class);
> + assign_hard_regno (hard_regno, regno);
> + bitmap_set_bit (&changed_pseudo_bitmap, regno);
> +   }
>   }
>   }
>  }


Jakub

RE: [RFC][ARM]: Fix reload spill failure (PR 60617)

[Ajit Kumar Agarwal]

You can assign the same register to more than operand based on the Liveness. It 
will be tricky if on the basis of Liveness  available registers not found. In 
that you need to spill one of the operands and use the registers assigned to 
the next operand. This forms the basis of spilling one of the values to assign 
the same registers to another operand.

This pattern is very specific to LO_REGS and I am sure there will low registers 
pressure for such operands. Is this pattern used  for dedicated registers for 
the operands?

Thanks & Regards
Ajit

-Original Message-
From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-ow...@gcc.gnu.org] On 
Behalf Of Venkataramanan Kumar
Sent: Monday, June 16, 2014 6:23 PM
To: gcc-patches@gcc.gnu.org; Richard Earnshaw; Ramana Radhakrishnan; Marcus 
Shawcroft; Patch Tracking; Maxim Kuvyrkov
Subject: [RFC][ARM]: Fix reload spill failure (PR 60617)

Hi Maintainers,

This patch fixes the PR 60617 that occurs when we turn on reload pass in thumb2 
mode.

It occurs for the pattern "*ior_scc_scc" that gets generated for the 3 argument 
of the below function call.

JIT:emitStoreInt32(dst,regT0m, (op1 == dst || op2 == dst)));


(snip---)
(insn 634 633 635 27 (parallel [
(set (reg:SI 3 r3)
(ior:SI (eq:SI (reg/v:SI 110 [ dst ]) <== This operand
r5 is registers gets assigned
(reg/v:SI 112 [ op2 ]))
(eq:SI (reg/v:SI 110 [ dst ]) <== This operand
(reg/v:SI 111 [ op1 ]
(clobber (reg:CC 100 cc))
]) ../Source/JavaScriptCore/jit/JITArithmetic32_64.cpp:179 300 
{*ior_scc_scc
(snip---)

The issue here is that the above pattern demands 5 registers (LO_REGS).

But when we are in reload, registers r0 is used for pointer to the class, r1 
and r2 for first and second argument. r7 is used for stack pointer.

So we are left with r3,r4,r5 and r6. But the above patterns needs five LO_REGS. 
Hence we get spill failure when processing the last register operand in that 
pattern,

In ARM port,  TARGET_LIKELY_SPILLED_CLASS is defined for Thumb-1 and for thumb 
2 mode there is mention of using LO_REG in the comment as below.

"Care should be taken to avoid adding thumb-2 patterns that require many low 
registers"

So conservative fix is not to allow this pattern for Thumb-2 mode.

I allowed these pattern for Thumb2 when we have constant operands for 
comparison. That makes the target tests arm/thum2-cond-cmp-1.c to 
thum2-cond-cmp-4.c pass.

Regression tested with gcc 4.9 branch since in trunk this bug is masked 
revision 209897.

Please provide your suggestion on this patch

regards,
Venkat.

Re: [PATCH] proposed fix for bug # 61144

[Jan Hubicka]

> On Mon, Jun 16, 2014 at 11:06:04AM +0200, Jan Hubicka wrote:
> > > 
> > > Are the attached files acceptable?
> > 
> > The testcase looks OK to me, but it already should be fixed on mainline
> > by patch https://gcc.gnu.org/ml/gcc-patches/2014-05/msg01315.html that
> > prevents dummy to be marked as constant. 
> > 
> > You can however modify the testcase to have
> > __attribute__ ((weak)) const int foo=0;
> 
> And the same for weak alias rather than straight weak definition like
> the above?

Yes, if you add const to your testcase, it will get miscompiled by mainline
again. 
> 
> > This needs your decl_replaceable change to not be optimized to if (0),
> > because of the explicit const modifier.
> 
> The case I care about actually has "dummy" as const (with the intent
> that it be allocated in a read-only section if the dummy definition is
> used). So for me it's important that this regression be fixed too.

Yep, GCC since 90's was optimizing reads from weak const attributes, but it
because worse because I added code walking through aliases.
> 
> > I did not change ctor_for_folding to reject variables above as I was not 
> > quite
> > sure we want to support this kind of interposition and I am still not quite 
> > certain.
> > C++ is quite clear about the transformation replacing initialized const by 
> > its value.
> 
> My concern is about C, not C++. This kind of interposition has always
> been supported in unix C, even prior to GCC, going back to sysv or
> earlier, as a documented feature (historically #pragma weak). It
> should not regress. If fixing it results in an regression with regards
> to optimizability of C++, perhaps this could be made
> language-specific, or (better) the C++ front-end could add an
> additional internal-use-only attribute to weak definitions it
> generates internally that permits constant-folding them, while not
> breaking the semantics for weak definitions provided by the user at
> the source level.

Yes, I see your point and clearly we should not optimize with explicit weak 
attribute.
I wonder if decl_replaceable_p is however correct check here or if we want 
explicit check
for weak visibility.

I am concerned about const variables w/o weak attribute with -fPIC (because for
those decl_replaceable_p returns true, too). Consider following testcase:
struct t
{
static const int dummy=0;
const int *m();
} t;
int
main()
{
  return *t.m();
}
int
main2()
{
  return t.dummy;
}
const int *
t::m()
{
  return &dummy;
}

Here main2 is optimized by C++ FE to return 0, while backend is affraid to 
optimize
main() after inlining anticipating that dummy may be interposed. However moving 
t::m
inside of the unit will make dummy comdat and it will get optimizing.
Adding another method and keying the t into other unit will make it optimized, 
too.

This is not very consistent. But perhaps we need a flag from C++ FE to tell us
what variables may not be interposed, because perhaps the c variant with -fPIC
const int dummy=0;
int
main()
{
  return t;
}

Jason?

A C variant of the testcase:

const int dummy=0;

const static int * d=&dummy;
int
main()
{
  return dummy;
}
int
main2()
{
  return *d;
}

seems optimized to return 0 (with -fPIC) for ages, too, but here at least
frontend won't substitute first dummy for 0.

Honza

[PING] Enable fuse-caller-save for ARM/Aarch64

[Tom de Vries]

Richard,

Ping for these patches:
- [PATCH, ARM] Enable fuse-caller-save for ARM
  https://gcc.gnu.org/ml/gcc-patches/2014-06/msg8.html
- [PATCH, AARCH64] Enable fuse-caller-save for AARCH64
  https://gcc.gnu.org/ml/gcc-patches/2014-06/msg4.html

The patches enable the -fuse-caller-save optimization for architectures ARM and
AARCH64.

Thanks,
- Tom

Re: [Patch LRA] PR 61522 - Handle NULL targetm.spill_class

[Vladimir Makarov]

On 2014-06-16, 11:05 AM, Ramana Radhakrishnan wrote:

Hi,

 This handles NULL targetm.spill_class in assign_by_spills. This
showed up as a segfault during a build for arm-none-linux-gnueabi(hf).

Fix pre-approved by richi on IRC , verified that bootstrap continues
from where things broke further on a tree (that reverts 211600 which is
the next breakage causing commit on arm-none-linux-gnueabihf)

I'll apply this in about 45 minutes when I get back to my desk if no one
objects.




Ops, I've already committed the following patch (sorry I've not read 
email before doing this):


I apologize for my mistake with the original patch.

2014-06-16  Vladimir Makarov  

PR rtl-optimization/61522
* lra-assigns.c (assign_by_spills): Check null
targetm.spill_class.

Index: lra-assigns.c
===
--- lra-assigns.c   (revision 211710)
+++ lra-assigns.c   (working copy)
@@ -1425,7 +1425,8 @@
  enum reg_class rclass = lra_get_allocno_class (regno);
  enum reg_class spill_class;

- if (lra_reg_info[regno].restore_regno < 0
+ if (targetm.spill_class == NULL
+ || lra_reg_info[regno].restore_regno < 0
  || ! bitmap_bit_p (&lra_inheritance_pseudos, regno)
  || (spill_class
  = ((enum reg_class)

RE: RFA: Make LRA temporarily eliminate addresses before testing constraints

[Robert Suchanek]

Pinging for approval.

This part of the patch will be needed for MIPS16. The second part to enable
LRA in MIPS has been already approved.

> Hi Richard, 
> 
> >> Robert: you also had an LRA change, but is it still needed after this one?
> >> If so, could you repost it and explain the case it handles? 
>
> For just turning the LRA for the MIPS backend is not needed but we have issues
> with the code size for MIPS16. LRA inserted a lot of reloads and the code size
> increased on average by about 10% IIRC. To fix this, a number of patterns 
> have to accept the stack pointer and a new class, M16_SP_REGS with 
> M16_REGS + $sp was added.
> 
> However, this triggered a reloading problem as the stack pointer was rejected
> by the back end and LRA tried to insert base+disp with the displacement not
> always present. It only affects $sp not directly accessible as in MIPS16 case.
>
> Regards,
> Robert
> 
> gcc/  
>   * lra-constraints.c (base_to_reg): New function.   
>   (process_address): Use new function.   
> 
> diff --git gcc/lra-constraints.c gcc/lra-constraints.c
> index 08716fe..d5ed37f 100644
> --- gcc/lra-constraints.c
> +++ gcc/lra-constraints.c
> @@ -2686,6 +2686,39 @@ process_alt_operands (int only_alternative)
>return ok_p;
>  }
>  
> +/* Make reload base reg from address AD.  */
> +static rtx
> +base_to_reg (struct address_info *ad)
> +{
> +  enum reg_class cl;
> +  int code = -1;
> +  rtx new_inner = NULL_RTX;
> +  rtx new_reg = NULL_RTX;
> +  rtx insn;
> +  rtx last_insn = get_last_insn();
> +
> +  lra_assert (ad->base == ad->base_term && ad->disp == ad->disp_term);
> +  cl = base_reg_class (ad->mode, ad->as, ad->base_outer_code,
> +   get_index_code (ad));
> +  new_reg = lra_create_new_reg (GET_MODE (*ad->base_term), NULL_RTX,
> +cl, "base");
> +  new_inner = simplify_gen_binary (PLUS, GET_MODE (new_reg), new_reg,
> +   ad->disp_term == NULL
> +   ? gen_int_mode (0, ad->mode)
> +   : *ad->disp_term);
> +  if (!valid_address_p (ad->mode, new_inner, ad->as))
> +return NULL_RTX;
> +  insn = emit_insn (gen_rtx_SET (ad->mode, new_reg, *ad->base_term));
> +  code = recog_memoized (insn);
> +  if (code < 0)
> +{
> +  delete_insns_since (last_insn);
> +  return NULL_RTX;
> +}
> +
> +  return new_inner;
> +}
> +
>  /* Make reload base reg + disp from address AD.  Return the new pseudo.  */
>  static rtx
>  base_plus_disp_to_reg (struct address_info *ad)
> @@ -2908,6 +2941,8 @@ process_address_1 (int nop, rtx *before, rtx *after)
>  
>   3) the address is a frame address with an invalid offset.
>  
> + 4) the address is a frame address with an invalid base.
> +
>   All these cases involve a non-autoinc address, so there is no
>   point revalidating other types.  */
>if (ad.autoinc_p || valid_address_p (&ad))
> @@ -2989,14 +3024,19 @@ process_address_1 (int nop, rtx *before, rtx *after)
>int regno;
>enum reg_class cl;
>rtx set, insns, last_insn;
> +  /* Try to reload base into register only if the base is invalid
> + for the address but with valid offset, case (4) above.  */
> +  start_sequence ();
> +  new_reg = base_to_reg (&ad);
> +
>/* base + disp => new base, cases (1) and (3) above.  */
>/* Another option would be to reload the displacement into an
>index register.  However, postreload has code to optimize
>address reloads that have the same base and different
>displacements, so reloading into an index register would
>not necessarily be a win.  */
> -  start_sequence ();
> -  new_reg = base_plus_disp_to_reg (&ad);
> +  if (new_reg == NULL_RTX)
> +new_reg = base_plus_disp_to_reg (&ad);
>insns = get_insns ();
>last_insn = get_last_insn ();
>/* If we generated at least two insns, try last insn source as

Re: [Patch LRA] PR 61522 - Handle NULL targetm.spill_class

[Ramana Radhakrishnan]

On Mon, Jun 16, 2014 at 5:11 PM, Vladimir Makarov  wrote:
> On 2014-06-16, 11:05 AM, Ramana Radhakrishnan wrote:
>>
>> Hi,
>>
>>  This handles NULL targetm.spill_class in assign_by_spills. This
>> showed up as a segfault during a build for arm-none-linux-gnueabi(hf).
>>
>> Fix pre-approved by richi on IRC , verified that bootstrap continues
>> from where things broke further on a tree (that reverts 211600 which is
>> the next breakage causing commit on arm-none-linux-gnueabihf)
>>
>> I'll apply this in about 45 minutes when I get back to my desk if no one
>> objects.
>>
>>
>
> Ops, I've already committed the following patch (sorry I've not read email
> before doing this):
>
> I apologize for my mistake with the original patch.
>
> 2014-06-16  Vladimir Makarov  
>
> PR rtl-optimization/61522
> * lra-assigns.c (assign_by_spills): Check null
> targetm.spill_class.
>
> Index: lra-assigns.c
> ===
> --- lra-assigns.c   (revision 211710)
> +++ lra-assigns.c   (working copy)
> @@ -1425,7 +1425,8 @@
>
>   enum reg_class rclass = lra_get_allocno_class (regno);
>   enum reg_class spill_class;
>
> - if (lra_reg_info[regno].restore_regno < 0
> + if (targetm.spill_class == NULL
> + || lra_reg_info[regno].restore_regno < 0
>   || ! bitmap_bit_p (&lra_inheritance_pseudos, regno)
>   || (spill_class
>   = ((enum reg_class)

Any reason the check couldn't be like the earlier patch ?

i.e. else if (targetm.spill_class)
 {



}


Ramana

>

Re: Another AIX Bootstrap failure

[Ramana Radhakrishnan]

On Mon, Jun 16, 2014 at 4:08 PM, Jan Hubicka  wrote:
>> Honza,
>>
>> Thanks for reverting the patch. I will check if this resolves the
>> current bootstrap problem.
>>
>> I was suggesting that you create a branch for all of the visibility
>> changes to make it easier to track the various original patches and
>> later correction patches from you.
>>
>> I don't know if the gen* programs hang because of the visibility
>> changes or because of the change in sections. The change in sections
>> could conflict with the GCC code to handle AIX XCOFF CSECTs for
>> functions.
>>
>> AIX recently added support for ELF-like visibility. AIX previously
>> supported the equivalent of visibility through "export" files. The
>> recent problems could be due to issues with assembly file ordering,
>> but they also could be related to the visibility changes affecting the
>> way that GCC emits code to branch to global functions.
>
> I comitted the revert now (my original testing got struct on ICE in
> auto-inc-dec pass that is unrelated).  I probably won't have time to analye
> what went wrong until Wednesday. The patch did not really play with
> ELF visibilities it was again related to bringing symbols local.
> I tried a case disabling the new conditional on clearning user section
> but that did not help. The patch basically collected few cleanups
> and fixes of corner case.  Last change is fix in the inline heuristics
> to not try to enale DECL_ONE_ONLY section sharing on targets not supporting
> it.  Obviously it should not lead to wrong code, since any inlining decision
> change should not, but I am testing it independnely now.

Can you please verify the testcase in PR61523 doesn't fail with your
reworked patch for arm-none-linux-gnueabihf ?

Thanks,
Ramana

>
> Honza

Re: [libgfortran, patch] Fix compilation on HP/UX 10

[Jeff Law]

On 06/14/14 21:59, Janne Blomqvist wrote:

Ok.

(as an aside, has anyone ever heard of anyone actually using gfortran on hp-ux?)
Folks certainly used the much older g77 implementation on hpux.  Since 
the switch to gfortran?  No clue.  I'm kindof amazed anyone is trying to 
work on hpux10.20.  hpux11 I can almost understand, but 10.20?


jeff

Re: [Patch LRA] PR 61522 - Handle NULL targetm.spill_class

[Vladimir Makarov]

On 2014-06-16, 12:14 PM, Ramana Radhakrishnan wrote:

On Mon, Jun 16, 2014 at 5:11 PM, Vladimir Makarov  wrote:



Any reason the check couldn't be like the earlier patch ?

i.e. else if (targetm.spill_class)
  {



}



I've just preferred to put most conditions in one place.  That is the 
only reason.  But if you want you can commit your variant.


I would have approved your patch, if I read your email before.  I was in 
hurry to fix it as the original patch broke most LRA targets (ones 
without the hook definition) and the mistake itself was embarrassing for me.

Re: [PATCH] proposed fix for bug # 61144

[Rich Felker]

On Mon, Jun 16, 2014 at 06:05:19PM +0200, Jan Hubicka wrote:
> > > This needs your decl_replaceable change to not be optimized to if (0),
> > > because of the explicit const modifier.
> > 
> > The case I care about actually has "dummy" as const (with the intent
> > that it be allocated in a read-only section if the dummy definition is
> > used). So for me it's important that this regression be fixed too.
> 
> Yep, GCC since 90's was optimizing reads from weak const attributes, but it
> because worse because I added code walking through aliases.

Ah, yes. BTW the reason I've avoided weak references and straight-out
weak definitions in favor of using weak aliases (aside from aliases
saving space in some cases) is that weak-alias seems to be the
baseline feature that's "always worked safely" whereas the others have
had bugs on and off in the past. And I really hope we can avoid having
more than a single release where it's broken. Some distros are already
pulling in 4.9.0 and I'm getting more bug reports.

> > > I did not change ctor_for_folding to reject variables above as I was not 
> > > quite
> > > sure we want to support this kind of interposition and I am still not 
> > > quite certain.
> > > C++ is quite clear about the transformation replacing initialized const 
> > > by its value.
> > 
> > My concern is about C, not C++. This kind of interposition has always
> > been supported in unix C, even prior to GCC, going back to sysv or
> > earlier, as a documented feature (historically #pragma weak). It
> > should not regress. If fixing it results in an regression with regards
> > to optimizability of C++, perhaps this could be made
> > language-specific, or (better) the C++ front-end could add an
> > additional internal-use-only attribute to weak definitions it
> > generates internally that permits constant-folding them, while not
> > breaking the semantics for weak definitions provided by the user at
> > the source level.
> 
> Yes, I see your point and clearly we should not optimize with explicit weak 
> attribute.
> I wonder if decl_replaceable_p is however correct check here or if we want 
> explicit check
> for weak visibility.

Isn't anything weak inherently replacable? It seems like using
decl_replacable_p, if that predicate is correctly implemented, would
be completely safe, since it should be true for a superset of weak.

> I am concerned about const variables w/o weak attribute with -fPIC (because 
> for
> those decl_replaceable_p returns true, too). Consider following testcase:
> struct t
> {
> static const int dummy=0;
> const int *m();
> } t;
> int
> main()
> {
>   return *t.m();
> }
> int
> main2()
> {
>   return t.dummy;
> }
> const int *
> t::m()
> {
>   return &dummy;
> }
> 
> Here main2 is optimized by C++ FE to return 0, while backend is affraid to 
> optimize
> main() after inlining anticipating that dummy may be interposed. However 
> moving t::m
> inside of the unit will make dummy comdat and it will get optimizing.
> Adding another method and keying the t into other unit will make it 
> optimized, too.
> 
> This is not very consistent. But perhaps we need a flag from C++ FE to tell us
> what variables may not be interposed, because perhaps the c variant with -fPIC
> const int dummy=0;
> int
> main()
> {
>   return t;
> }
> 
> Jason?
> 
> A C variant of the testcase:
> 
> const int dummy=0;
> 
> const static int * d=&dummy;
> int
> main()
> {
>   return dummy;
> }
> int
> main2()
> {
>   return *d;
> }
> 
> seems optimized to return 0 (with -fPIC) for ages, too, but here at least
> frontend won't substitute first dummy for 0.

I see the issue of whether interposition should be supported in
non-weak situations like this as completely separate from pr61144.
Strictly speaking, it's undefined behavior to have multiple
definitions of the same symbol. ELF semantics allow such interposition
for shared library use mainly because creating a shared library hides
translation-unit boundaries whereby symbol replacement with the
equivalent static library may have had well-defined behavior due to
the conflicting translation units not getting pulled in by the linker.
But there's no fundamental reason to need to support interposition
against symbols defined in the same translation unit, since in the
corresponding static-library usage it would result in a link-time
error.

With the weak symbols issue (pr61144), on the other hand, the bug is
that the well-established semantics of weak binding are not being
honored by the compiler.

Of course it may still be _desirable_ to support the sort of
interposition you described for definitions in the same translation
unit, but whether you want to support that should be treated as a
separate issue IMO, for the above reasons.

Rich

Re: [PATCH][genattrtab] Fix memory corruption, allocate enough memory for all bypassed reservations

[Jeff Law]

On 06/16/14 04:12, Kyrill Tkachov wrote:



Doh, you're right. I did consider it but for some reason thought we
might want to iterate over all of the bypasses anyway. Breaking out
seems good.

How about this?
Tested on arm and aarch64 and confirmed with valgrind that no out of
bounds accesses occur.
I kicked off an x86_64 bootstrap but don't expect any problems.

Thanks,
Kyrill

genattrtab-bypasses.patch


commit 676b85f7a7cc1446482334dcaad457ac328875a8
Author: Kyrylo Tkachov
Date:   Fri Jun 13 11:09:57 2014 +0100

 [genattrtab] Fix memory corruption with bypasses
I'm an idiot.  n_bypassed is used to size the vector, so you do have to 
walk the entire list.


Jeff

Re: Another AIX Bootstrap failure

[David Edelsohn]

Honza,

FYI, your bootstrap on gcc111 is hung in the exact same place as I
have observed: all of the stage2 gen* programs spinning.

- David


On Mon, Jun 16, 2014 at 11:08 AM, Jan Hubicka  wrote:
>> Honza,
>>
>> Thanks for reverting the patch. I will check if this resolves the
>> current bootstrap problem.
>>
>> I was suggesting that you create a branch for all of the visibility
>> changes to make it easier to track the various original patches and
>> later correction patches from you.
>>
>> I don't know if the gen* programs hang because of the visibility
>> changes or because of the change in sections. The change in sections
>> could conflict with the GCC code to handle AIX XCOFF CSECTs for
>> functions.
>>
>> AIX recently added support for ELF-like visibility. AIX previously
>> supported the equivalent of visibility through "export" files. The
>> recent problems could be due to issues with assembly file ordering,
>> but they also could be related to the visibility changes affecting the
>> way that GCC emits code to branch to global functions.
>
> I comitted the revert now (my original testing got struct on ICE in
> auto-inc-dec pass that is unrelated).  I probably won't have time to analye
> what went wrong until Wednesday. The patch did not really play with
> ELF visibilities it was again related to bringing symbols local.
> I tried a case disabling the new conditional on clearning user section
> but that did not help. The patch basically collected few cleanups
> and fixes of corner case.  Last change is fix in the inline heuristics
> to not try to enale DECL_ONE_ONLY section sharing on targets not supporting
> it.  Obviously it should not lead to wrong code, since any inlining decision
> change should not, but I am testing it independnely now.
>
> Honza

Re: Regimplification enhancements 1/3

[Mike Stump]

On Jun 16, 2014, at 3:56 AM, Bernd Schmidt  wrote:
> For the ptx port, I've needed to write a new pass which ensures all objects 
> go into address spaces as required by the machine.

I have such a machine and I’ve always approached the problem from the front end 
side.  I ensure the right space up front, and if someone doesn’t get the right 
space, I add code to let the port choose the space for that object (literal 
constants I wanna play around with some for example).  On my port there is no 
changing spaces once one is selected.

I’m curious if your port needs to change a space (and why, I’m wondering if I 
can improve my port by changing spaces)?

One known issue for my port is if I run out of memory in one space, I don’t 
magically fall over to another space, but rather just fail.  The other is space 
selection for the stack.  I let it default, and then during operand printing I 
select the space I’ve chosen for the default.  The scheduling is complicated as 
I have to handle two spaces (normal, plus default), instead of just one.

Re: Regimplification enhancements 1/3

[Bernd Schmidt]

On 06/16/2014 07:26 PM, Mike Stump wrote:

On Jun 16, 2014, at 3:56 AM, Bernd Schmidt 
wrote:

For the ptx port, I've needed to write a new pass which ensures all
objects go into address spaces as required by the machine.


I have such a machine and I’ve always approached the problem from the
front end side.  I ensure the right space up front, and if someone
doesn’t get the right space, I add code to let the port choose the
space for that object (literal constants I wanna play around with
some for example).  On my port there is no changing spaces once one
is selected.


There are two reasons why I can't do this in the frontends - one, Joseph 
has already rejected a C frontend patch, and two, this needs to work 
with OpenACC offloading - i.e. code is initially compiled by an x86 host 
compiler, then a ptx lto1 reads it in and needs to make it valid for 
that target.



I’m curious if your port needs to change a space (and why, I’m
wondering if I can improve my port by changing spaces)?


No, address spaces are fixed for globals, locals, and rodata. But in the 
early stages of the compiler, for the above reasons we need to pretend 
that there aren't any address spaces involved, and only the 
lower-addr-spaces pass moves everything to the right place.



Bernd

Re: RFA: Make LRA temporarily eliminate addresses before testing constraints

[Vladimir Makarov]

On 2014-06-16, 12:12 PM, Robert Suchanek wrote:

Pinging for approval.

This part of the patch will be needed for MIPS16. The second part to enable
LRA in MIPS has been already approved.



  Sorry, Robert.  I thought you are waiting for some Richard's comment 
(actually he knows the code well and wrote address decoding in rtlanal.c).


  The patch is ok for me and makes LRA even more portable as it adds a 
new profitable address transformation and the code can be useful for 
other targets too.


  Thanks.



Hi Richard,


Robert: you also had an LRA change, but is it still needed after this one?
If so, could you repost it and explain the case it handles?


For just turning the LRA for the MIPS backend is not needed but we have issues
with the code size for MIPS16. LRA inserted a lot of reloads and the code size
increased on average by about 10% IIRC. To fix this, a number of patterns
have to accept the stack pointer and a new class, M16_SP_REGS with
M16_REGS + $sp was added.

However, this triggered a reloading problem as the stack pointer was rejected
by the back end and LRA tried to insert base+disp with the displacement not
always present. It only affects $sp not directly accessible as in MIPS16 case.

Regards,
Robert

gcc/
* lra-constraints.c (base_to_reg): New function.
(process_address): Use new function.

diff --git gcc/lra-constraints.c gcc/lra-constraints.c
index 08716fe..d5ed37f 100644
--- gcc/lra-constraints.c
+++ gcc/lra-constraints.c
@@ -2686,6 +2686,39 @@ process_alt_operands (int only_alternative)
return ok_p;
  }

+/* Make reload base reg from address AD.  */
+static rtx
+base_to_reg (struct address_info *ad)
+{
+  enum reg_class cl;
+  int code = -1;
+  rtx new_inner = NULL_RTX;
+  rtx new_reg = NULL_RTX;
+  rtx insn;
+  rtx last_insn = get_last_insn();
+
+  lra_assert (ad->base == ad->base_term && ad->disp == ad->disp_term);
+  cl = base_reg_class (ad->mode, ad->as, ad->base_outer_code,
+   get_index_code (ad));
+  new_reg = lra_create_new_reg (GET_MODE (*ad->base_term), NULL_RTX,
+cl, "base");
+  new_inner = simplify_gen_binary (PLUS, GET_MODE (new_reg), new_reg,
+   ad->disp_term == NULL
+   ? gen_int_mode (0, ad->mode)
+   : *ad->disp_term);
+  if (!valid_address_p (ad->mode, new_inner, ad->as))
+return NULL_RTX;
+  insn = emit_insn (gen_rtx_SET (ad->mode, new_reg, *ad->base_term));
+  code = recog_memoized (insn);
+  if (code < 0)
+{
+  delete_insns_since (last_insn);
+  return NULL_RTX;
+}
+
+  return new_inner;
+}
+
  /* Make reload base reg + disp from address AD.  Return the new pseudo.  */
  static rtx
  base_plus_disp_to_reg (struct address_info *ad)
@@ -2908,6 +2941,8 @@ process_address_1 (int nop, rtx *before, rtx *after)

   3) the address is a frame address with an invalid offset.

+ 4) the address is a frame address with an invalid base.
+
   All these cases involve a non-autoinc address, so there is no
   point revalidating other types.  */
if (ad.autoinc_p || valid_address_p (&ad))
@@ -2989,14 +3024,19 @@ process_address_1 (int nop, rtx *before, rtx *after)
int regno;
enum reg_class cl;
rtx set, insns, last_insn;
+  /* Try to reload base into register only if the base is invalid
+ for the address but with valid offset, case (4) above.  */
+  start_sequence ();
+  new_reg = base_to_reg (&ad);
+
/* base + disp => new base, cases (1) and (3) above.  */
/* Another option would be to reload the displacement into an
 index register.  However, postreload has code to optimize
 address reloads that have the same base and different
 displacements, so reloading into an index register would
 not necessarily be a win.  */
-  start_sequence ();
-  new_reg = base_plus_disp_to_reg (&ad);
+  if (new_reg == NULL_RTX)
+new_reg = base_plus_disp_to_reg (&ad);
insns = get_insns ();
last_insn = get_last_insn ();
/* If we generated at least two insns, try last insn source as

Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)

[Yury Gribov]

On 06/16/2014 04:47 PM, Yury Gribov wrote:

On 06/16/2014 02:34 PM, Dominique Dhumieres wrote:

Done in r211699.


This breaks bootstrap on x86_64-apple-darwin13:


Hm, perhaps I didn't run full bootstrap after last round of review.
Does attached patch solve the problem for you? I've started bootstrap
but it'll take couple of hours to complete.


Bootstrapped successfully on x64 with proposed patch.
The original commit indeed failed to bootstrap 
(https://gcc.gnu.org/ml/gcc-testresults/2014-06/msg01419.html).


Ok to commit fix?

-Y



diff --git a/gcc/asan.c b/gcc/asan.c
index 19e1524..281a795 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -1636,6 +1636,13 @@ build_check_stmt (location_t location, tree base, tree len,
 
   gcc_assert (!(size_in_bytes > 0 && !non_zero_len_p));
 
+  if (start_instrumented && end_instrumented)
+{
+  if (!before_p)
+gsi_next (iter);
+  return;
+}
+
   if (len)
 len = unshare_expr (len);
   else
@@ -1735,7 +1742,7 @@ build_check_stmt (location_t location, tree base, tree len,
   gsi_insert_after (&gsi, g, GSI_NEW_STMT);
   tree base_addr = gimple_assign_lhs (g);
 
-  tree t;
+  tree t = NULL_TREE;
   if (real_size_in_bytes >= 8)
 {
   tree shadow = build_shadow_mem_access (&gsi, location, base_addr,

Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)

[Jakub Jelinek]

On Mon, Jun 16, 2014 at 10:21:39PM +0400, Yury Gribov wrote:
> On 06/16/2014 04:47 PM, Yury Gribov wrote:
> >On 06/16/2014 02:34 PM, Dominique Dhumieres wrote:
> >>>Done in r211699.
> >>
> >>This breaks bootstrap on x86_64-apple-darwin13:
> >
> >Hm, perhaps I didn't run full bootstrap after last round of review.
> >Does attached patch solve the problem for you? I've started bootstrap
> >but it'll take couple of hours to complete.
> 
> Bootstrapped successfully on x64 with proposed patch.
> The original commit indeed failed to bootstrap
> (https://gcc.gnu.org/ml/gcc-testresults/2014-06/msg01419.html).
> 
> Ok to commit fix?

Ok (with proper ChangeLog entry).

> --- a/gcc/asan.c
> +++ b/gcc/asan.c
> @@ -1636,6 +1636,13 @@ build_check_stmt (location_t location, tree base, tree 
> len,
>  
>gcc_assert (!(size_in_bytes > 0 && !non_zero_len_p));
>  
> +  if (start_instrumented && end_instrumented)
> +{
> +  if (!before_p)
> +gsi_next (iter);
> +  return;
> +}
> +
>if (len)
>  len = unshare_expr (len);
>else
> @@ -1735,7 +1742,7 @@ build_check_stmt (location_t location, tree base, tree 
> len,
>gsi_insert_after (&gsi, g, GSI_NEW_STMT);
>tree base_addr = gimple_assign_lhs (g);
>  
> -  tree t;
> +  tree t = NULL_TREE;
>if (real_size_in_bytes >= 8)
>  {
>tree shadow = build_shadow_mem_access (&gsi, location, base_addr,


Jakub

Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)

[Yury Gribov]

Bootstrapped successfully on x64 with proposed patch.
The original commit indeed failed to bootstrap
(https://gcc.gnu.org/ml/gcc-testresults/2014-06/msg01419.html).

Ok to commit fix?


Ok (with proper ChangeLog entry).


r211713.

-Y

Re: Another AIX Bootstrap failure

[David Edelsohn]

On Mon, Jun 16, 2014 at 12:35 AM, Jan Hubicka  wrote:

> @@ -512,9 +516,9 @@ function_and_variable_visibility (bool w
>  next = next->same_comdat_group)
> {
>   next->set_comdat_group (NULL);
> - if (!next->alias)
> -   next->set_section (NULL);
>   symtab_make_decl_local (next->decl);
> + if (!node->alias)
> +   node->reset_section ();

>   next->unique_name = ((next->resolution == 
> LDPR_PREVAILING_DEF_IRONLY
> || next->unique_name
> || next->resolution == 
> LDPR_PREVAILING_DEF_IRONLY_EXP)

Honza, did you really intend to change the above from next->alias and
next->set_section () to node->alias and node->reset_section () ?  That
doesn't look right.

- David

[PATCH, Cilk+, PR57541] Additional fix for issues witn array notations

[Zamyatin, Igor]

Hi All!

The patch fixes ICE in array notation for the cases of incorrect arguments of 
Cilk+ builtins and undeclared initial index.

Is it ok for trunk and 4.9?

Thanks,
Igor

diff --git a/gcc/c/ChangeLog b/gcc/c/ChangeLog
index 54d0de7..56e1b0b 100644
--- a/gcc/c/ChangeLog
+++ b/gcc/c/ChangeLog
@@ -1,3 +1,12 @@
+2014-06-16  Igor Zamyatin  
+
+   PR middle-end/57541
+   * c-array-notation.c (fix_builtin_array_notation_fn):
+   Check for 0 arguments in builtin call. Check that bultin argument is
+   correct.
+   * c-parser.c (c_parser_array_notation): Check for incorrect initial
+   index.
+
diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index 053155a..a2c66a8 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,10 @@
+2014-06-16  Igor Zamyatin  
+
+   PR middle-end/57541
+   * cp-array-notation.c (expand_sec_reduce_builtin):
+   Check that bultin argument is correct.
+   * call.c (build_cxx_call): Check for 0 arguments in builtin call.
+
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index f9f03d9..f08600c 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2014-06-16  Igor Zamyatin  
+
+   PR middle-end/57541
+   * c-c++-common/cilk-plus/AN/pr57541.c: New case added.
+   * c-c++-common/cilk-plus/AN/pr57541-2.c: New test.
+


diff --git a/gcc/c/c-array-notation.c b/gcc/c/c-array-notation.c
index b4015b8..67a8931 100644
--- a/gcc/c/c-array-notation.c
+++ b/gcc/c/c-array-notation.c
@@ -214,6 +214,13 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree 
*new_var)
   if (an_type == BUILT_IN_NONE)
 return NULL_TREE;

+  /* Builtin call should contain at least one argument.  */
+  if (call_expr_nargs (an_builtin_fn) == 0)
+    {
+  error_at (EXPR_LOCATION (an_builtin_fn), "Invalid builtin arguments");
+  return error_mark_node;
+    }
+
   if (an_type == BUILT_IN_CILKPLUS_SEC_REDUCE
   || an_type == BUILT_IN_CILKPLUS_SEC_REDUCE_MUTATING)
 {
@@ -238,7 +245,10 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree 
*new_var)
 return error_mark_node;
  
   if (rank == 0)
-    return an_builtin_fn;
+    {
+  error_at (location, "Invalid builtin arguments");
+  return error_mark_node;
+    }
   else if (rank > 1 
   && (an_type == BUILT_IN_CILKPLUS_SEC_REDUCE_MAX_IND
   || an_type == BUILT_IN_CILKPLUS_SEC_REDUCE_MIN_IND))
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index abd636c..a04a23e 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -14097,7 +14097,7 @@ c_parser_array_notation (location_t loc, c_parser 
*parser, tree initial_index,
   tree value_tree = NULL_TREE, type = NULL_TREE, array_type = NULL_TREE;
   tree array_type_domain = NULL_TREE; 
 
-  if (array_value == error_mark_node)
+  if (array_value == error_mark_node || initial_index == error_mark_node)
 {
   /* No need to continue.  If either of these 2 were true, then an error
 must be emitted already.  Thus, no need to emit them twice.  */

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 75a6a4a..e021a38 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -7415,6 +7415,11 @@ build_cxx_call (tree fn, int nargs, tree *argarray,
  || bif == BUILT_IN_CILKPLUS_SEC_REDUCE
  || bif == BUILT_IN_CILKPLUS_SEC_REDUCE_MUTATING)
    { 
+ if (call_expr_nargs (fn) == 0)
+   {
+ error_at (EXPR_LOCATION (fn), "Invalid builtin arguments");
+     return error_mark_node;
+   }
  /* for bif == BUILT_IN_CILKPLUS_SEC_REDUCE_ALL_ZERO or
 BUILT_IN_CILKPLUS_SEC_REDUCE_ANY_ZERO or
 BUILT_IN_CILKPLUS_SEC_REDUCE_ANY_NONZERO or 
diff --git a/gcc/cp/cp-array-notation.c b/gcc/cp/cp-array-notation.c
index 0538e55..b45449b 100644
--- a/gcc/cp/cp-array-notation.c
+++ b/gcc/cp/cp-array-notation.c
@@ -250,7 +250,10 @@ expand_sec_reduce_builtin (tree an_builtin_fn, tree 
*new_var)
   if (!find_rank (location, an_builtin_fn, an_builtin_fn, true, &rank))
   return error_mark_node;
   if (rank == 0)
-    return an_builtin_fn;
+    {
+  error_at (location, "Invalid builtin arguments");
+  return error_mark_node;
+    }
   else if (rank > 1 
   && (an_type == BUILT_IN_CILKPLUS_SEC_REDUCE_MAX_IND
   || an_type == BUILT_IN_CILKPLUS_SEC_REDUCE_MIN_IND))

diff --git a/gcc/testsuite/c-c++-common/cilk-plus/AN/pr57541-2.c 
b/gcc/testsuite/c-c++-common/cilk-plus/AN/pr57541-2.c
new file mode 100644
index 000..83325a7
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/cilk-plus/AN/pr57541-2.c
@@ -0,0 +1,15 @@
+/* PR middle-end/57541 */
+/* { dg-do compile } */
+/* { dg-options "-fcilkplus" } */
+
+int foo1 ()
+{
+  int a;
+  a = __sec_reduce_add (1); /* { dg-error "Invalid builtin arguments" } */
+}
+
+int foo2 ()
+{
+  int a;
+  a = __sec_reduce_add (); /* { dg-error "Invalid builtin arguments" } */
+}
diff --git a/gcc/testsuite/c-c++-common/cilk-plus/AN/pr57541.c 
b/gcc/testsuite/c-

Re: Another AIX Bootstrap failure

[Jan Hubicka]

> Honza,
> 
> FYI, your bootstrap on gcc111 is hung in the exact same place as I
> have observed: all of the stage2 gen* programs spinning.

Yes (will kill it now), I was tracking down what change it is.
It is actually the inliner hunk that is independent of rest of changes
(and one that should be obviously safe, as famous last words):

Index: ipa-inline-analysis.c
===
--- ipa-inline-analysis.c   (revision 211488)
+++ ipa-inline-analysis.c   (working copy)
@@ -3877,7 +3877,7 @@ do_estimate_growth (struct cgraph_node *
   /* COMDAT functions are very often not shared across multiple units
  since they come from various template instantiations.
  Take this into account.  */
-  else if (DECL_COMDAT (node->decl)
+  else if (node->externally_visible && node->get_comdat_group ()
   && cgraph_can_remove_if_no_direct_calls_p (node))
d.growth -= (info->size
 * (100 - PARAM_VALUE (PARAM_COMDAT_SHARING_PROBABILITY))
@@ -3928,7 +3928,7 @@ growth_likely_positive (struct cgraph_no
   && (ret = node_growth_cache[node->uid]))
 return ret > 0;
   if (!cgraph_will_be_removed_from_program_if_no_direct_calls (node)
-  && (!DECL_COMDAT (node->decl)
+  && (!node->externally_visible || !node->get_comdat_group ()
  || !cgraph_can_remove_if_no_direct_calls_p (node)))
 return true;
   max_callers = inline_summary (node)->size * 4 / edge_growth + 2;

This hunk promotes more inlining of COMDAT functions in anticipation that
if they get inlined in each module independently, they will be optimized out
and that in C++ high percentage of comdat functions is actually used by
one unit only.

The change disables this heuristic tweek for targets without comdat support,
like AIX is. Here we do not want to compute probability of section sharing, 
becuase there are no comdat sections (and if there is other mechanism, we should
let middle-end know). The main motviation was however COMDAT locals on ELF
(introduced by cdtor decloning) where we want to handle them as statics
rather than COMDATs.

No mater of what these functions return, the final program should be correct-
they only affect inlining decisions, not correctness of the inliner.  We ought
to debug this.  Do you have any clue what goes wrong?

If you don't mind, I would like to commit back the rest of changes 
(reset_section)
cleanups after testing on AIX + testing the aforementioned ARM testcase. 

I will also make more AIX firendly version of this patch. But please, lets debug
the problem, so we don't have random wrong code issues here. 
Honza

Re: Regimplification enhancements 1/3

[Mike Stump]

On Jun 16, 2014, at 10:49 AM, Bernd Schmidt  wrote:
> 
> There are two reasons why I can't do this in the frontends - one, Joseph has 
> already rejected a C frontend patch,

I’d like to think there is an acceptable way to get the right memory space on 
things...

> and two, this needs to work with OpenACC offloading - i.e. code is initially 
> compiled by an x86 host compiler, then a ptx lto1 reads it in and needs to 
> make it valid for that target.

Ah yes, that would do it, thanks.  I can see my port as an offload target…  
I’ll have to keep on eye on OpenACC and gcc.

One more patch for PR61325

[Vladimir Makarov]

  Here is one more patch concerning

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

The following patch prevents complex address transformation just by 
checking a change in address validity of subreg memory substitution.


The patch was bootstrapped and tested on x86-64.

Committed as rev. 211716 to gcc-4.9-branch and as rev. 211715 to the trunk.

2014-06-16  Vladimir Makarov  

PR rtl-optimization/61325
* lra-constraints.c (valid_address_p): Add forward declaration.
(simplify_operand_subreg): Check address validity before and after
alter_reg of memory subreg.


Index: lra-constraints.c
===
--- lra-constraints.c   (revision 211654)
+++ lra-constraints.c   (working copy)
@@ -1231,6 +1231,8 @@
 }
 }
 
+static int valid_address_p (enum machine_mode mode, rtx addr, addr_space_t as);
+
 /* Make reloads for subreg in operand NOP with internal subreg mode
REG_MODE, add new reloads for further processing.  Return true if
any reload was generated.  */
@@ -1261,12 +1263,28 @@
  equivalences in function lra_constraints) and because for spilled
  pseudos we allocate stack memory enough for the biggest
  corresponding paradoxical subreg.  */
-  if ((MEM_P (reg)
-   && (! SLOW_UNALIGNED_ACCESS (mode, MEM_ALIGN (reg))
-  || MEM_ALIGN (reg) >= GET_MODE_ALIGNMENT (mode)))
-  || (REG_P (reg) && REGNO (reg) < FIRST_PSEUDO_REGISTER))
+  if (MEM_P (reg)
+  && (! SLOW_UNALIGNED_ACCESS (mode, MEM_ALIGN (reg))
+ || MEM_ALIGN (reg) >= GET_MODE_ALIGNMENT (mode)))
 {
+  rtx subst, old = *curr_id->operand_loc[nop];
+
   alter_subreg (curr_id->operand_loc[nop], false);
+  subst = *curr_id->operand_loc[nop];
+  lra_assert (MEM_P (subst));
+  if (! valid_address_p (GET_MODE (reg), XEXP (reg, 0),
+MEM_ADDR_SPACE (reg))
+ || valid_address_p (GET_MODE (subst), XEXP (subst, 0),
+ MEM_ADDR_SPACE (subst)))
+   return true;
+  /* If the address was valid and became invalid, prefer to reload
+the memory.  Typical case is when the index scale should
+correspond the memory.  */
+  *curr_id->operand_loc[nop] = old;
+}
+  else if (REG_P (reg) && REGNO (reg) < FIRST_PSEUDO_REGISTER)
+{
+  alter_subreg (curr_id->operand_loc[nop], false);
   return true;
 }
   /* Put constant into memory when we have mixed modes.  It generates

Re: Another AIX Bootstrap failure

[Jan Hubicka]

> On Mon, Jun 16, 2014 at 12:35 AM, Jan Hubicka  wrote:
> 
> > @@ -512,9 +516,9 @@ function_and_variable_visibility (bool w
> >  next = next->same_comdat_group)
> > {
> >   next->set_comdat_group (NULL);
> > - if (!next->alias)
> > -   next->set_section (NULL);
> >   symtab_make_decl_local (next->decl);
> > + if (!node->alias)
> > +   node->reset_section ();
> 
> >   next->unique_name = ((next->resolution == 
> > LDPR_PREVAILING_DEF_IRONLY
> > || next->unique_name
> > || next->resolution == 
> > LDPR_PREVAILING_DEF_IRONLY_EXP)
> 
> Honza, did you really intend to change the above from next->alias and
> next->set_section () to node->alias and node->reset_section () ?  That
> doesn't look right.

You are right, this is a pasto in anoying code duplication here.  I fixed that
in my local copy of the patch.  I have a followup patch in the ipa-visibility
TLC to merge the code duplication here, I am just holding it until we debug the
issues. 

This bug will not affect AIX, becuase there are no comdat groups.  It only
wastes a bit of code size on ELF systems because of extra alignment coming
from the named section.

The hang happens in 
(gdb) bt
#0  0x100be244 in __gnu_parallel::_Settings::_Settings() ()
#1  0x10008d54 in _GLOBAL__FI_libstdc___so ()
#2  0x10008e88 in _GLOBAL__AIXI_libstdc___so ()
#3  0x100be954 in _GLOBAL__FI_genconstants ()
#4  0xd017fa54 in mod_init1 () from /usr/lib/libc.a(shr.o)
#5  0xd017f774 in __modinit () from /usr/lib/libc.a(shr.o)
#6  0x11a0 in __start ()

I suppose it may be crtbegin/crtend miscompile.  Any insight would be welcome,
otherwise I will try to progress on debugging tonight or tomorrow.

Honza

Re: Another AIX Bootstrap failure

[David Edelsohn]

On Mon, Jun 16, 2014 at 5:44 PM, Jan Hubicka  wrote:

> If you don't mind, I would like to commit back the rest of changes 
> (reset_section)
> cleanups after testing on AIX + testing the aforementioned ARM testcase.

Please go ahead and commit the parts of the patch that do not cause problems.

Thanks, DAvid

Re: [PATCH] gcc/dwarf2asm.c: Add static_output_delta() with var_list for dw2_asm_output_delta()

[Chen Gang]

On 06/16/2014 10:37 PM, Joseph S. Myers wrote:
> On Mon, 16 Jun 2014, Chen Gang wrote:
> 
>> +static_output_delta (int size, const char *lab1, const char *lab2,
> 
> static_ is not a helpful naming convention, since the fact that a function 
> is static is nothing to do with what that function does.  Try something 
> like dw2_asm_voutput_delta to indicate a va_list version of 
> dw2_asm_output_delta.
> 

Hmm... OK, I will send patch v2 for it. In future, it may need be as
extern function like vprintf() for printf(), which need not change its
name, again.

But at present, it remains as static until really find a caller has to
call it outside (let extern functions as few as possible, unless related
code really can be shared).

For me (may it only be personal hobby): adding 'static_' prefix can make
it easy to name internal functions, and also let readers (especially
'grep' from shell) know about it is static, easily.


Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed

[PATCH v2] gcc/dwarf2asm.c: Add dw2_asm_voutput_delta() with var_list for dw2_asm_output_delta()

[Chen Gang]

dw2_asm_output_vms_delta() calls dw2_asm_output_delta() in an abnormal
way, so need add a new function just like vprintf() for printf(), and
then the related call will be in normal way.

The related warning:

  ../../gcc/gcc/dwarf2asm.c: In function ‘void dw2_asm_output_vms_delta(int, 
const char*, const char*, const char*, ...)’:
  ../../gcc/gcc/dwarf2asm.c:167:50: warning: format not a string literal and no 
format arguments [-Wformat-security]


Signed-off-by: Chen Gang 
---
 gcc/ChangeLog   |  6 ++
 gcc/dwarf2asm.c | 21 +
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index edb3fc0..01a1cc1 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@
+2014-06-14  Chen Gang 
+
+   * dwarf2asm.c (dw2_asm_output_delta): Add dw2_asm_voutput_delta() for
+   dw2_asm_output_delta(), just like vprintf() for printf(), so that
+   dw2_asm_output_vms_delta() can use it in normal way.
+
 2014-06-13  Vladimir Makarov  
 
* lra-assign.c (assign_by_spills): Add code to assign vector regs
diff --git a/gcc/dwarf2asm.c b/gcc/dwarf2asm.c
index 1372b23..376b939 100644
--- a/gcc/dwarf2asm.c
+++ b/gcc/dwarf2asm.c
@@ -123,14 +123,10 @@ dw2_asm_output_data (int size, unsigned HOST_WIDE_INT 
value,
impossible to do here, since the ASM_SET_OP for the difference
symbol must appear after both symbols are defined.  */
 
-void
-dw2_asm_output_delta (int size, const char *lab1, const char *lab2,
- const char *comment, ...)
+static void
+dw2_asm_voutput_delta (int size, const char *lab1, const char *lab2,
+ const char *comment, va_list ap)
 {
-  va_list ap;
-
-  va_start (ap, comment);
-
 #ifdef ASM_OUTPUT_DWARF_DELTA
   ASM_OUTPUT_DWARF_DELTA (asm_out_file, size, lab1, lab2);
 #else
@@ -145,7 +141,16 @@ dw2_asm_output_delta (int size, const char *lab1, const 
char *lab2,
   vfprintf (asm_out_file, comment, ap);
 }
   fputc ('\n', asm_out_file);
+}
 
+void
+dw2_asm_output_delta (int size, const char *lab1, const char *lab2,
+ const char *comment, ...)
+{
+  va_list ap;
+
+  va_start (ap, comment);
+  dw2_asm_voutput_delta (size, lab1, lab2, comment, ap);
   va_end (ap);
 }
 
@@ -164,7 +169,7 @@ dw2_asm_output_vms_delta (int size ATTRIBUTE_UNUSED,
 #ifndef ASM_OUTPUT_DWARF_VMS_DELTA
   /* VMS Delta is only special on ia64-vms, but this function also gets
  called on alpha-vms so it has to do something sane.  */
-  dw2_asm_output_delta (size, lab1, lab2, comment);
+  dw2_asm_voutput_delta (size, lab1, lab2, comment, ap);
 #else
   ASM_OUTPUT_DWARF_VMS_DELTA (asm_out_file, size, lab1, lab2);
   if (flag_debug_asm && comment)
-- 
1.9.2.459.g68773ac

Re: Another AIX Bootstrap failure

[David Edelsohn]

It looks like _Settings is miscompiled, or more specifically a
function descriptor has a bogus value

   0x100bcbac <_ZN14__gnu_parallel9_SettingsC1Ev>:  addir12,r2,-168
   0x100bcbb0 <_ZN14__gnu_parallel9_SettingsC1Ev+4>:stw r2,20(r1)
   0x100bcbb4 <_ZN14__gnu_parallel9_SettingsC1Ev+8>:lwz r0,0(r12)
   0x100bcbb8 <_ZN14__gnu_parallel9_SettingsC1Ev+12>:   lwz r2,4(r12)
   0x100bcbbc <_ZN14__gnu_parallel9_SettingsC1Ev+16>:   mtctr   r0
=> 0x100bcbc0 <_ZN14__gnu_parallel9_SettingsC1Ev+20>:   bctr

(gdb) x/x $r2-168
0x3002f90c <__dbargs+11440>:0x100bcbac
(gdb) x/x $r12
0x3002f90c <__dbargs+11440>:0x100bcbac
(gdb) p/x $ctr
$5 = 0x100bcbac

For some reason, the additional inlining is creating the wrong address
for the target.

- David



On Mon, Jun 16, 2014 at 6:06 PM, Jan Hubicka  wrote:
>> On Mon, Jun 16, 2014 at 12:35 AM, Jan Hubicka  wrote:
>>
>> > @@ -512,9 +516,9 @@ function_and_variable_visibility (bool w
>> >  next = next->same_comdat_group)
>> > {
>> >   next->set_comdat_group (NULL);
>> > - if (!next->alias)
>> > -   next->set_section (NULL);
>> >   symtab_make_decl_local (next->decl);
>> > + if (!node->alias)
>> > +   node->reset_section ();
>> 
>> >   next->unique_name = ((next->resolution == 
>> > LDPR_PREVAILING_DEF_IRONLY
>> > || next->unique_name
>> > || next->resolution == 
>> > LDPR_PREVAILING_DEF_IRONLY_EXP)
>>
>> Honza, did you really intend to change the above from next->alias and
>> next->set_section () to node->alias and node->reset_section () ?  That
>> doesn't look right.
>
> You are right, this is a pasto in anoying code duplication here.  I fixed that
> in my local copy of the patch.  I have a followup patch in the ipa-visibility
> TLC to merge the code duplication here, I am just holding it until we debug 
> the
> issues.
>
> This bug will not affect AIX, becuase there are no comdat groups.  It only
> wastes a bit of code size on ELF systems because of extra alignment coming
> from the named section.
>
> The hang happens in
> (gdb) bt
> #0  0x100be244 in __gnu_parallel::_Settings::_Settings() ()
> #1  0x10008d54 in _GLOBAL__FI_libstdc___so ()
> #2  0x10008e88 in _GLOBAL__AIXI_libstdc___so ()
> #3  0x100be954 in _GLOBAL__FI_genconstants ()
> #4  0xd017fa54 in mod_init1 () from /usr/lib/libc.a(shr.o)
> #5  0xd017f774 in __modinit () from /usr/lib/libc.a(shr.o)
> #6  0x11a0 in __start ()
>
> I suppose it may be crtbegin/crtend miscompile.  Any insight would be welcome,
> otherwise I will try to progress on debugging tonight or tomorrow.
>
> Honza