Re: [C++0x] contiguous bitfields race implementation

[Richard Guenther]

On Wed, Aug 31, 2011 at 8:09 PM, Aldy Hernandez  wrote:
>
>> Did you test Ada and enable the C++ memory model? ;)
>
> See my earlier comment on Ada.  Who would ever use the C++ memory model on
> Ada?

People interoperating Ada with C++.  Our bug triager Zdenek who
figures out the --param?

>> Btw, even if the bitfield we access (and thus the whole region) is at a
>> constant offset, the field _following_ the bitregion (the one you query
>> above with get_inner_reference) can be at variable offset.  I suggest
>> to simply not include any padding in that case (which would be,
>> TREE_CODE (DECL_FIELD_OFFSET (fld)) != INTEGER_CST).
>
> I still have not found a place where we get a variable offset here (after
> folding the computation).  How about we put a gcc_assert() along with a big
> fat comment with your above suggestion when we encounter this.  Or can you
> give me an example of this case?

My point is, the middle-end infrastructure makes it possible for this
case to appear, and it seems to be easy to handle conservatively.
There isn't a need to wait for users to run into an ICE or an assert we put
there IMHO.  If I'd be fluent in Ada I'd write you a testcase, but I ain't.

>>> Is what you want, that we call get_inner_reference once, and then use
>>> DECL_FIELD_OFFSET+DECL_FIELD_BIT_OFFSET to calculate any subsequent bit
>>> offset?  I found this to be quite tricky with padding, and such, but am
>>> willing to give it a whirl again.
>>
>> Yes.
>
> I have added a comment to this effect, and will address it along with the
> get_inner_reference() removal you have suggested as a followup.

Thanks,
Richard.

Re: [C++0x] contiguous bitfields race implementation

[Arnaud Charlet]

> >> Did you test Ada and enable the C++ memory model? ;)
> >
> > See my earlier comment on Ada.  Who would ever use the C++ memory model on
> > Ada?
> 
> People interoperating Ada with C++.  Our bug triager Zdenek who
> figures out the --param?

Right, that's one example. There are also actually some similarities between
the C++ memory model and the Ada language, so it's not so unconcievable
that Ada would like to take advantage of some of these capabilities.

Arno

Re: [Ada] Implementation of aspects within generic units

[Richard Guenther]

On Wed, Aug 31, 2011 at 6:34 PM, Arnaud Charlet  wrote:
>> Program received signal EXC_BAD_ACCESS, Could not access memory.
>> Reason: KERN_PROTECTION_FAILURE at address: 0x
>> 0x001c4fa0 in lib__writ__write_ali ()
>> (gdb) bt
>> #0  0x001c4fa0 in lib__writ__write_ali ()
>> #1  0x0036799a in _ada_gnat1drv ()
>> #2  0x000305f5 in gnat_parse_file ()
>
> I just triple checked, and revision 178381 is OK for me on
> x86_64-unknown-linux-gnu.
>
> Unfortunately without debug info, the above traceback isn't
> giving much info.
>
> Just a shot in the dark, can you try to pinpoint at which revision
> things started to break? That'd be useful.
>
> In particular I'd be curious to know if revision 178376 has the failure or 
> not.
>
> Same for revision 178311

My bootstrap compiler is GCC 4.3.4, the error occurs in stage3 (well, when
building the RTS).  It will take some time to check the revs you quoted.

Richard.

RE: [Patch, testsuite] Skip case struct-layout-1 for targets using short enums.

[Terry Guo]

Hello Jakub,

> 
> On Thu, Sep 01, 2011 at 02:32:51PM +0800, Terry Guo wrote:
> > FAIL: gcc.dg/compat/struct-layout-1 c_compat_x_tst.o compile
> > FAIL: gcc.dg/compat/struct-layout-1 c_compat_y_tst.o compile
> > UNRESOLVED: gcc.dg/compat/struct-layout-1 c_compat_x_tst.o-
> c_compat_y_tst.o
> > link
> > UNRESOLVED: gcc.dg/compat/struct-layout-1 c_compat_x_tst.o-
> c_compat_y_tst.o
> > execute
> >
> > On my machine, the error message is "error: width of 'a' exceeds its
> type".
> > My GCC is cross built for arm-none-eabi based on upstream GCC 4.6.
> The
> > command I run this case is simply as: "make check-gcc
> > RUNTESTFLAGS="--target_board=arm-none-eabi-qemu/-mthumb/-mcpu=cortex-
> m3
> > compat.exp=struct-layout-1*". Are there anything else I should do to
> pass
> > this case?
> 
> Look into gcc/testsuite/gcc/gcc.log, search for struct-layout-
> 1_generate.exe
> and see whether -e has been passed to it?  If not, debug the tcl bits
> which
> are supposed to pass it, but for some reason don't, if yes, look into
> the
> generator under debugger why it generates the large enum bitfield
> bitsizes
> anyway (you can cut'n'paste the *generate.exe command line, compile it
> with
> -g and rerun it under debugger...).
> 

What you said is for compat/struct-layout-1.exp. What I said is for
compat/compat.exp. The case contains three files struct-layout-1_x.c,
struct-layout-1_y.c and struct-layout-1_main.c. All of them are independent
files and not generated by generate.exe. Meanwhile do you think it is
necessary to keep this cases in compat.exp given we already have
struct-layout-1.exp?

BR,
Terry

Re: [Ada] Implementation of aspects within generic units

[Arnaud Charlet]

> My bootstrap compiler is GCC 4.3.4, the error occurs in stage3 (well, when
> building the RTS).  It will take some time to check the revs you quoted.

OK. Still trying to reproduce here and trying to figure out blindly what could
be the cause of this behavior for now.

Olivier is also trying to reproduce. Perhaps he or Eric will have an idea
on what could be going wrong.

Just in case, here are the versions of gmp and co I'm using:

gmp-4.3.2
mpc-0.8.1
mpfr-2.4.2

I also have other pending changes to merge. Perhaps one of these changes may
"fix" or make this problem go away, although that's also a shot in the dark,
since a quick review of the changes didn't shed any light/candidate.

Arno

Re: [Patch, testsuite] Skip case struct-layout-1 for targets using short enums.

[Jakub Jelinek]

On Thu, Sep 01, 2011 at 03:09:34PM +0800, Terry Guo wrote:
> What you said is for compat/struct-layout-1.exp. What I said is for
> compat/compat.exp. The case contains three files struct-layout-1_x.c,
> struct-layout-1_y.c and struct-layout-1_main.c. All of them are independent
> files and not generated by generate.exe. Meanwhile do you think it is
> necessary to keep this cases in compat.exp given we already have
> struct-layout-1.exp?

The files are there to test the infrastructure.  I'd say just try
2011-09-01  Jakub Jelinek  

* gcc.dg/compat/struct-layout-1_test.h: Decrease bitfield size
to work even with -fshort-enums.

--- gcc/testsuite/gcc.dg/compat/struct-layout-1_test.h  2008-09-05 
12:54:32.0 +0200
+++ gcc/testsuite/gcc.dg/compat/struct-layout-1_test.h  2011-09-01 
09:21:20.200426411 +0200
@@ -1,5 +1 @@
-#if (__SIZEOF_INT__ >= 4) 
-T(0,enum E2 a:31;,B(0,a,e2_m1,e2_0))
-#else
-T(0,enum E2 a:15;,B(0,a,e2_m1,e2_0))
-#endif
+T(0,enum E2 a:7;,B(0,a,e2_m1,e2_0))

instead.

Jakub

Re: [PATCH] Make vectorizer dumps more comparable

[Ira Rosen]

gcc-patches-ow...@gcc.gnu.org wrote on 24/08/2011 02:20:50 PM:
>
> This avoids the file/location clutter in front of each line
> in the vectorizer dump.  While this is useful for people
> requesting -fvectorizer-verbose=N in dump files this makes
> you unable to compare dumps for testcases on a branch and trunk.
> It also makes lines excessively long because the testsuite
> filename paths are so long.  Very annoying.
>
> (I'd argue also that -fvectorizer-verbose=N dumps to the dump
> file if available and not always to stderr is bogus, but well ...)
>
> This patch has made my life a lot easier debugging the data dependence
> stuff.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, installed on trunk.
>
> Richard.
>
> 2011-08-24  Richard Guenther  
>
>* tree-vectorizer.c (vect_print_dump_info): Avoid the
>file and location clutter when dumping to dump files.


IMO it's a bad idea. It's now impossible to find anything when compiling a
big file. How about only removing the file name?

Index: tree-vectorizer.c
===
--- tree-vectorizer.c   (revision 178374)
+++ tree-vectorizer.c   (working copy)
@@ -149,16 +149,12 @@ vect_print_dump_info (enum vect_verbosit
   if (!current_function_decl || !vect_dump)
 return false;

-  if (dump_file)
-fprintf (vect_dump, "\n");
-
-  else if (vect_location == UNKNOWN_LOC)
-fprintf (vect_dump, "\n%s:%d: note: ",
-DECL_SOURCE_FILE (current_function_decl),
+  if (vect_location == UNKNOWN_LOC)
+fprintf (vect_dump, "\nline %d: ",
 DECL_SOURCE_LINE (current_function_decl));
   else
-fprintf (vect_dump, "\n%s:%d: note: ",
-LOC_FILE (vect_location), LOC_LINE (vect_location));
+fprintf (vect_dump, "\nline %d: ",
+LOC_LINE (vect_location));

   return true;
 }

Ira

>
> Index: gcc/tree-vectorizer.c
> ===
> --- gcc/tree-vectorizer.c   (revision 178028)
> +++ gcc/tree-vectorizer.c   (working copy)
> @@ -149,7 +149,10 @@ vect_print_dump_info (enum vect_verbosit
>if (!current_function_decl || !vect_dump)
>  return false;
>
> -  if (vect_location == UNKNOWN_LOC)
> +  if (dump_file)
> +fprintf (vect_dump, "\n");
> +
> +  else if (vect_location == UNKNOWN_LOC)
>  fprintf (vect_dump, "\n%s:%d: note: ",
>  DECL_SOURCE_FILE (current_function_decl),
>  DECL_SOURCE_LINE (current_function_decl));
>

Re: [PATCH] Make vectorizer dumps more comparable

[Richard Guenther]

On Thu, 1 Sep 2011, Ira Rosen wrote:

> 
> 
> gcc-patches-ow...@gcc.gnu.org wrote on 24/08/2011 02:20:50 PM:
> >
> > This avoids the file/location clutter in front of each line
> > in the vectorizer dump.  While this is useful for people
> > requesting -fvectorizer-verbose=N in dump files this makes
> > you unable to compare dumps for testcases on a branch and trunk.
> > It also makes lines excessively long because the testsuite
> > filename paths are so long.  Very annoying.
> >
> > (I'd argue also that -fvectorizer-verbose=N dumps to the dump
> > file if available and not always to stderr is bogus, but well ...)
> >
> > This patch has made my life a lot easier debugging the data dependence
> > stuff.
> >
> > Bootstrapped and tested on x86_64-unknown-linux-gnu, installed on trunk.
> >
> > Richard.
> >
> > 2011-08-24  Richard Guenther  
> >
> >* tree-vectorizer.c (vect_print_dump_info): Avoid the
> >file and location clutter when dumping to dump files.
> 
> 
> IMO it's a bad idea. It's now impossible to find anything when compiling a
> big file. How about only removing the file name?

How about, as Micha suggested, print the location of the loop
we currently investigate from vectorize_loops () where we
call find_loop_location () instead?

Richard.

> Index: tree-vectorizer.c
> ===
> --- tree-vectorizer.c   (revision 178374)
> +++ tree-vectorizer.c   (working copy)
> @@ -149,16 +149,12 @@ vect_print_dump_info (enum vect_verbosit
>if (!current_function_decl || !vect_dump)
>  return false;
> 
> -  if (dump_file)
> -fprintf (vect_dump, "\n");
> -
> -  else if (vect_location == UNKNOWN_LOC)
> -fprintf (vect_dump, "\n%s:%d: note: ",
> -DECL_SOURCE_FILE (current_function_decl),
> +  if (vect_location == UNKNOWN_LOC)
> +fprintf (vect_dump, "\nline %d: ",
>  DECL_SOURCE_LINE (current_function_decl));
>else
> -fprintf (vect_dump, "\n%s:%d: note: ",
> -LOC_FILE (vect_location), LOC_LINE (vect_location));
> +fprintf (vect_dump, "\nline %d: ",
> +LOC_LINE (vect_location));
> 
>return true;
>  }
> 
> Ira
> 
> >
> > Index: gcc/tree-vectorizer.c
> > ===
> > --- gcc/tree-vectorizer.c   (revision 178028)
> > +++ gcc/tree-vectorizer.c   (working copy)
> > @@ -149,7 +149,10 @@ vect_print_dump_info (enum vect_verbosit
> >if (!current_function_decl || !vect_dump)
> >  return false;
> >
> > -  if (vect_location == UNKNOWN_LOC)
> > +  if (dump_file)
> > +fprintf (vect_dump, "\n");
> > +
> > +  else if (vect_location == UNKNOWN_LOC)
> >  fprintf (vect_dump, "\n%s:%d: note: ",
> >  DECL_SOURCE_FILE (current_function_decl),
> >  DECL_SOURCE_LINE (current_function_decl));
> >
> 
> 

-- 
Richard Guenther 
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

RE: [Patch, testsuite] Skip case struct-layout-1 for targets using short enums.

[Terry Guo]

Hello Jakub,

> The files are there to test the infrastructure.  I'd say just try
> 2011-09-01  Jakub Jelinek  
> 
>   * gcc.dg/compat/struct-layout-1_test.h: Decrease bitfield size
>   to work even with -fshort-enums.
> 
> --- gcc/testsuite/gcc.dg/compat/struct-layout-1_test.h2008-09-05
> 12:54:32.0 +0200
> +++ gcc/testsuite/gcc.dg/compat/struct-layout-1_test.h2011-09-01
> 09:21:20.200426411 +0200
> @@ -1,5 +1 @@
> -#if (__SIZEOF_INT__ >= 4)
> -T(0,enum E2 a:31;,B(0,a,e2_m1,e2_0))
> -#else
> -T(0,enum E2 a:15;,B(0,a,e2_m1,e2_0))
> -#endif
> +T(0,enum E2 a:7;,B(0,a,e2_m1,e2_0))
> 
> instead.
> 

I just tried the patch, it works well. The case get passed. Would you please
commit it into trunk and GCC 4.6 branch? If you want me to do something
else, please let me know. Thanks.

BR,
Terry

Re: Add unwind information to mips epilogues

[Richard Sandiford]

Bernd Schmidt  writes:
> On 08/31/11 20:43, Richard Sandiford wrote:
>> Bernd Schmidt  writes:
>>> This is necessary when adding shrink-wrapping; otherwise dwarf2cfi sees
>>> inconsistent information and aborts.
>>>
>>> Tested on mips64-elf together with the rest of the shrink-wrapping
>>> patches. Ok?
>> 
>> It looks like the current code doesn't handle the RESTORE instruction.
>> Could you also test that somehow?  A mipsisa32-elf run with -mips16
>> ought to work, but some sort of spot-checking of shrink-wrapping +
>> RESTORE would be fine if that's easier.
>
> Will look into that. You mean the mips16e_build_save_restore function?

Yeah.

>> Also, for the frame_pointer_required case, it looks like there's a
>> window between the restoration of the frame pointer and the deallocation
>> of the stack in which the CFA is still defined in terms of the frame
>> pointer register.  Is that significant?  If not (e.g. because we
>> should never need to unwind at that point) then why do we still
>> update the CFA here:
>
> CC'ing rth, as I'm still not terribly familiar with these issues.

Me too.

>>> @@ -10324,12 +10330,26 @@ mips_expand_epilogue (bool sibcall_p)
>>>if (!TARGET_MIPS16)
>>> target = stack_pointer_rtx;
>>>  
>>> -  emit_insn (gen_add3_insn (target, base, adjust));
>>> +  insn = emit_insn (gen_add3_insn (target, base, adjust));
>>> +  if (!frame_pointer_needed && target == stack_pointer_rtx)
>>> +   {
>>> + RTX_FRAME_RELATED_P (insn) = 1;
>>> + add_reg_note (insn, REG_CFA_DEF_CFA,
>>> +   plus_constant (stack_pointer_rtx, step2));
>>> +   }
>
> Here, with !frame_pointer_needed, SP should be the CFA register, so if
> we modify it we have to use REG_CFA_DEF_CFA. Either that, or I'm confused.

What I meant was: if we can ignore the state between the restoration
of the frame pointer and final stack deallocation, why can't we also
ignore the state between this intermediate deallocation and the final one?
I.e. why isn't it enough to attach a DEF_CFA to the final deallocation only?
In principle, I mean; I realise dwarf2cfi might not like it.

Richard

Re: [PATCH] Make vectorizer dumps more comparable

[Ira Rosen]

Richard Guenther  wrote on 01/09/2011 10:33:23 AM:

> On Thu, 1 Sep 2011, Ira Rosen wrote:
>
> >
> >
> > gcc-patches-ow...@gcc.gnu.org wrote on 24/08/2011 02:20:50 PM:
> > >
> > > This avoids the file/location clutter in front of each line
> > > in the vectorizer dump.  While this is useful for people
> > > requesting -fvectorizer-verbose=N in dump files this makes
> > > you unable to compare dumps for testcases on a branch and trunk.
> > > It also makes lines excessively long because the testsuite
> > > filename paths are so long.  Very annoying.
> > >
> > > (I'd argue also that -fvectorizer-verbose=N dumps to the dump
> > > file if available and not always to stderr is bogus, but well ...)
> > >
> > > This patch has made my life a lot easier debugging the data
dependence
> > > stuff.
> > >
> > > Bootstrapped and tested on x86_64-unknown-linux-gnu, installed on
trunk.
> > >
> > > Richard.
> > >
> > > 2011-08-24  Richard Guenther  
> > >
> > >* tree-vectorizer.c (vect_print_dump_info): Avoid the
> > >file and location clutter when dumping to dump files.
> >
> >
> > IMO it's a bad idea. It's now impossible to find anything when
compiling a
> > big file. How about only removing the file name?
>
> How about, as Micha suggested, print the location of the loop
> we currently investigate from vectorize_loops () where we
> call find_loop_location () instead?

The problem is that a dump of a single loop can be pretty long, and "start
to analyze loop..."/"finish to analyze loop..." may be not visible enough.
I am OK with adding these printings though (in addition to line numbers).

I understand why you didn't like to see the file location, but what's the
problem with the line number?

Ira

>
> Richard.
>
> > Index: tree-vectorizer.c
> > ===
> > --- tree-vectorizer.c   (revision 178374)
> > +++ tree-vectorizer.c   (working copy)
> > @@ -149,16 +149,12 @@ vect_print_dump_info (enum vect_verbosit
> >if (!current_function_decl || !vect_dump)
> >  return false;
> >
> > -  if (dump_file)
> > -fprintf (vect_dump, "\n");
> > -
> > -  else if (vect_location == UNKNOWN_LOC)
> > -fprintf (vect_dump, "\n%s:%d: note: ",
> > -DECL_SOURCE_FILE (current_function_decl),
> > +  if (vect_location == UNKNOWN_LOC)
> > +fprintf (vect_dump, "\nline %d: ",
> >  DECL_SOURCE_LINE (current_function_decl));
> >else
> > -fprintf (vect_dump, "\n%s:%d: note: ",
> > -LOC_FILE (vect_location), LOC_LINE (vect_location));
> > +fprintf (vect_dump, "\nline %d: ",
> > +LOC_LINE (vect_location));
> >
> >return true;
> >  }
> >
> > Ira
> >
> > >
> > > Index: gcc/tree-vectorizer.c
> > > ===
> > > --- gcc/tree-vectorizer.c   (revision 178028)
> > > +++ gcc/tree-vectorizer.c   (working copy)
> > > @@ -149,7 +149,10 @@ vect_print_dump_info (enum vect_verbosit
> > >if (!current_function_decl || !vect_dump)
> > >  return false;
> > >
> > > -  if (vect_location == UNKNOWN_LOC)
> > > +  if (dump_file)
> > > +fprintf (vect_dump, "\n");
> > > +
> > > +  else if (vect_location == UNKNOWN_LOC)
> > >  fprintf (vect_dump, "\n%s:%d: note: ",
> > >  DECL_SOURCE_FILE (current_function_decl),
> > >  DECL_SOURCE_LINE (current_function_decl));
> > >
> >
> >
>
> --
> Richard Guenther 
> SUSE / SUSE Labs
> SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

Re: [Ada] Implementation of aspects within generic units

[Arnaud Charlet]

> OK. Still trying to reproduce here and trying to figure out blindly what
> could be the cause of this behavior for now.

I could finally reproduce on another machine (i686-linux), so I am now
doing a binary search to find out precisely what change caused this
failure, so no need for other people to do it.

I'll send more info ASAP (pending bootstrap/cpu time).

Arno

Re: [Ada] Implementation of aspects within generic units

[Richard Guenther]

On Thu, Sep 1, 2011 at 9:06 AM, Richard Guenther
 wrote:
> On Wed, Aug 31, 2011 at 6:34 PM, Arnaud Charlet  wrote:
>>> Program received signal EXC_BAD_ACCESS, Could not access memory.
>>> Reason: KERN_PROTECTION_FAILURE at address: 0x
>>> 0x001c4fa0 in lib__writ__write_ali ()
>>> (gdb) bt
>>> #0  0x001c4fa0 in lib__writ__write_ali ()
>>> #1  0x0036799a in _ada_gnat1drv ()
>>> #2  0x000305f5 in gnat_parse_file ()
>>
>> I just triple checked, and revision 178381 is OK for me on
>> x86_64-unknown-linux-gnu.
>>
>> Unfortunately without debug info, the above traceback isn't
>> giving much info.
>>
>> Just a shot in the dark, can you try to pinpoint at which revision
>> things started to break? That'd be useful.
>>
>> In particular I'd be curious to know if revision 178376 has the failure or 
>> not.
>>
>> Same for revision 178311
>
> My bootstrap compiler is GCC 4.3.4, the error occurs in stage3 (well, when
> building the RTS).  It will take some time to check the revs you quoted.

glibc 2.11.1, on SLE11 SP1, binutils from what will be SP2, 2.21.1.

r178311, no-go:

error: "exp_light.ali" not found, "exp_light.adb" must be compiled
make[3]: *** [ada/b_gnat1.adb] Error 5
make[3]: *** Waiting for unfinished jobs

r178376, broken.

r178316, which maybe fixed the r178311 issue: ok.
Richard.

> Richard.
>

Re: [PATCH] Make vectorizer dumps more comparable

[Richard Sandiford]

Ira Rosen  writes:
>> How about, as Micha suggested, print the location of the loop
>> we currently investigate from vectorize_loops () where we
>> call find_loop_location () instead?
>
> The problem is that a dump of a single loop can be pretty long, and "start
> to analyze loop..."/"finish to analyze loop..." may be not visible enough.
> I am OK with adding these printings though (in addition to line numbers).
>
> I understand why you didn't like to see the file location, but what's the
> problem with the line number?

+1 FWIW.  I found these per-line locations really useful when doing
the strided load/store stuff.

Richard

[patch] Fix PR tree-optimization/50178

[Ira Rosen]

Hi,

When vectorizing a function call we replace the original call with a
dummy statement to ensure that DCE later removes it. We also remove
its stmt_vec_info, which causes the segfault when we try to access it
through related pattern stmt. The following patch updates related
pattern stmt to be the dummy stmt.

Bootstrapped and tested on powerpc64-suse-linux.
OK for 4.6?

Thanks,
Ira

ChangeLog:

 PR tree-optimization/50178
 * tree-vect-stmts.c (vectorizable_call): Update the related
pattern statement
 before deleting the original call.
 (vect_transform_stmt): Don't expect the related pattern statement match the
 original statement after transformation.

testsuite/ChangeLog:

 PR tree-optimization/50178
 * gfortran.dg/vect/pr50178.f90: New test.

Index: testsuite/gfortran.dg/vect/pr50178.f90
===
--- testsuite/gfortran.dg/vect/pr50178.f90  (revision 0)
+++ testsuite/gfortran.dg/vect/pr50178.f90  (revision 0)
@@ -0,0 +1,29 @@
+! { dg-do compile }
+
+module yemdyn
+   implicit none
+   integer, parameter :: jpim = selected_int_kind(9)
+   integer, parameter :: jprb = selected_real_kind(13,300)
+   real(kind=jprb) :: elx
+   real(kind=jprb), allocatable :: xkcoef(:)
+   integer(kind=jpim),allocatable :: ncpln(:), npne(:)
+end module yemdyn
+
+subroutine suedyn
+
+   use yemdyn
+
+   implicit none
+
+   integer(kind=jpim) :: jm, jn
+   real(kind=jprb) :: zjm, zjn, zxxx
+
+   jn=0
+   do jm=0,ncpln(jn)
+  zjm=real(jm,jprb) / elx
+  xkcoef(npne(jn)+jm) = - zxxx*(zjm**2)**0.5_jprb
+   end do
+
+end subroutine suedyn
+
+! { dg-final { cleanup-tree-dump "vect" } }
Index: tree-vect-stmts.c
===
--- tree-vect-stmts.c   (revision 178373)
+++ tree-vect-stmts.c   (working copy)
@@ -1583,6 +1583,14 @@ vectorizable_call (gimple stmt, gimple_stmt_iterat
   new_stmt = gimple_build_assign (gimple_call_lhs (stmt),
  build_zero_cst (type));
   set_vinfo_for_stmt (new_stmt, stmt_info);
+  /* For pattern statements make the related statement to point to
+ NEW_STMT in order to be able to retrieve the original statement
+ information later.  */
+  if (is_pattern_stmt_p (stmt_info))
+{
+  gimple related = STMT_VINFO_RELATED_STMT (stmt_info);
+  STMT_VINFO_RELATED_STMT (vinfo_for_stmt (related)) = new_stmt;
+}
   set_vinfo_for_stmt (stmt, NULL);
   STMT_VINFO_STMT (stmt_info) = new_stmt;
   gsi_replace (gsi, new_stmt, false);
@@ -4957,11 +4965,7 @@ vect_transform_stmt (gimple stmt, gimple_stmt_iter
 the stmt_info of ORIG_STMT_IN_PATTERN.  See more details in the
 documentation of vect_pattern_recog.  */
  if (STMT_VINFO_IN_PATTERN_P (stmt_vinfo))
-   {
- gcc_assert (STMT_VINFO_RELATED_STMT (stmt_vinfo)
-   == orig_scalar_stmt);
- STMT_VINFO_VEC_STMT (stmt_vinfo) = vec_stmt;
-   }
+   STMT_VINFO_VEC_STMT (stmt_vinfo) = vec_stmt;
}
 }

Re: [PATCH] Make vectorizer dumps more comparable

[Richard Guenther]

On Thu, 1 Sep 2011, Ira Rosen wrote:

> 
> 
> Richard Guenther  wrote on 01/09/2011 10:33:23 AM:
> 
> > On Thu, 1 Sep 2011, Ira Rosen wrote:
> >
> > >
> > >
> > > gcc-patches-ow...@gcc.gnu.org wrote on 24/08/2011 02:20:50 PM:
> > > >
> > > > This avoids the file/location clutter in front of each line
> > > > in the vectorizer dump.  While this is useful for people
> > > > requesting -fvectorizer-verbose=N in dump files this makes
> > > > you unable to compare dumps for testcases on a branch and trunk.
> > > > It also makes lines excessively long because the testsuite
> > > > filename paths are so long.  Very annoying.
> > > >
> > > > (I'd argue also that -fvectorizer-verbose=N dumps to the dump
> > > > file if available and not always to stderr is bogus, but well ...)
> > > >
> > > > This patch has made my life a lot easier debugging the data
> dependence
> > > > stuff.
> > > >
> > > > Bootstrapped and tested on x86_64-unknown-linux-gnu, installed on
> trunk.
> > > >
> > > > Richard.
> > > >
> > > > 2011-08-24  Richard Guenther  
> > > >
> > > >* tree-vectorizer.c (vect_print_dump_info): Avoid the
> > > >file and location clutter when dumping to dump files.
> > >
> > >
> > > IMO it's a bad idea. It's now impossible to find anything when
> compiling a
> > > big file. How about only removing the file name?
> >
> > How about, as Micha suggested, print the location of the loop
> > we currently investigate from vectorize_loops () where we
> > call find_loop_location () instead?
> 
> The problem is that a dump of a single loop can be pretty long, and "start
> to analyze loop..."/"finish to analyze loop..." may be not visible enough.
> I am OK with adding these printings though (in addition to line numbers).
> 
> I understand why you didn't like to see the file location, but what's the
> problem with the line number?

Well, it seems to be different what everybody else does and it's
highly redundant for a whole bunch of lines.

But, it solves my diff issue and the overly long lines as well.

Your patch changes both dump-file and stderr printing though,
I did want to preserve stderr printing.

For the dump-file I'd drop the 'line ' prefix and just print '%d: '.

Btw, the diagnostic machinery does _not_ print locations
for note (""), the location information is supposed to be printed
in the heading warning/error.  Thus, a much better format for stderr
would be

file.c:12: LOOP NOT VECTORIZED
note: unsupported stmt ''

as the further notes will be printed with the 'loop location' which
is confusing when dumping statements

Richard.

> Ira
> 
> >
> > Richard.
> >
> > > Index: tree-vectorizer.c
> > > ===
> > > --- tree-vectorizer.c   (revision 178374)
> > > +++ tree-vectorizer.c   (working copy)
> > > @@ -149,16 +149,12 @@ vect_print_dump_info (enum vect_verbosit
> > >if (!current_function_decl || !vect_dump)
> > >  return false;
> > >
> > > -  if (dump_file)
> > > -fprintf (vect_dump, "\n");
> > > -
> > > -  else if (vect_location == UNKNOWN_LOC)
> > > -fprintf (vect_dump, "\n%s:%d: note: ",
> > > -DECL_SOURCE_FILE (current_function_decl),
> > > +  if (vect_location == UNKNOWN_LOC)
> > > +fprintf (vect_dump, "\nline %d: ",
> > >  DECL_SOURCE_LINE (current_function_decl));
> > >else
> > > -fprintf (vect_dump, "\n%s:%d: note: ",
> > > -LOC_FILE (vect_location), LOC_LINE (vect_location));
> > > +fprintf (vect_dump, "\nline %d: ",
> > > +LOC_LINE (vect_location));
> > >
> > >return true;
> > >  }
> > >
> > > Ira
> > >
> > > >
> > > > Index: gcc/tree-vectorizer.c
> > > > ===
> > > > --- gcc/tree-vectorizer.c   (revision 178028)
> > > > +++ gcc/tree-vectorizer.c   (working copy)
> > > > @@ -149,7 +149,10 @@ vect_print_dump_info (enum vect_verbosit
> > > >if (!current_function_decl || !vect_dump)
> > > >  return false;
> > > >
> > > > -  if (vect_location == UNKNOWN_LOC)
> > > > +  if (dump_file)
> > > > +fprintf (vect_dump, "\n");
> > > > +
> > > > +  else if (vect_location == UNKNOWN_LOC)
> > > >  fprintf (vect_dump, "\n%s:%d: note: ",
> > > >  DECL_SOURCE_FILE (current_function_decl),
> > > >  DECL_SOURCE_LINE (current_function_decl));
> > > >
> > >
> > >
> >
> > --
> > Richard Guenther 
> > SUSE / SUSE Labs
> > SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
> > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
> 
> 

-- 
Richard Guenther 
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

Re: [patch] Fix PR tree-optimization/50178

[Richard Guenther]

On Thu, Sep 1, 2011 at 10:12 AM, Ira Rosen  wrote:
> Hi,
>
> When vectorizing a function call we replace the original call with a
> dummy statement to ensure that DCE later removes it. We also remove
> its stmt_vec_info, which causes the segfault when we try to access it
> through related pattern stmt. The following patch updates related
> pattern stmt to be the dummy stmt.
>
> Bootstrapped and tested on powerpc64-suse-linux.
> OK for 4.6?

Ok.

Thanks,
Richard.

> Thanks,
> Ira
>
> ChangeLog:
>
>     PR tree-optimization/50178
>     * tree-vect-stmts.c (vectorizable_call): Update the related
> pattern statement
>     before deleting the original call.
>     (vect_transform_stmt): Don't expect the related pattern statement match 
> the
>     original statement after transformation.
>
> testsuite/ChangeLog:
>
>     PR tree-optimization/50178
>     * gfortran.dg/vect/pr50178.f90: New test.
>
> Index: testsuite/gfortran.dg/vect/pr50178.f90
> ===
> --- testsuite/gfortran.dg/vect/pr50178.f90      (revision 0)
> +++ testsuite/gfortran.dg/vect/pr50178.f90      (revision 0)
> @@ -0,0 +1,29 @@
> +! { dg-do compile }
> +
> +module yemdyn
> +   implicit none
> +   integer, parameter :: jpim = selected_int_kind(9)
> +   integer, parameter :: jprb = selected_real_kind(13,300)
> +   real(kind=jprb) :: elx
> +   real(kind=jprb), allocatable :: xkcoef(:)
> +   integer(kind=jpim),allocatable :: ncpln(:), npne(:)
> +end module yemdyn
> +
> +subroutine suedyn
> +
> +   use yemdyn
> +
> +   implicit none
> +
> +   integer(kind=jpim) :: jm, jn
> +   real(kind=jprb) :: zjm, zjn, zxxx
> +
> +   jn=0
> +   do jm=0,ncpln(jn)
> +      zjm=real(jm,jprb) / elx
> +      xkcoef(npne(jn)+jm) = - zxxx*(zjm**2)**0.5_jprb
> +   end do
> +
> +end subroutine suedyn
> +
> +! { dg-final { cleanup-tree-dump "vect" } }
> Index: tree-vect-stmts.c
> ===
> --- tree-vect-stmts.c   (revision 178373)
> +++ tree-vect-stmts.c   (working copy)
> @@ -1583,6 +1583,14 @@ vectorizable_call (gimple stmt, gimple_stmt_iterat
>   new_stmt = gimple_build_assign (gimple_call_lhs (stmt),
>                                  build_zero_cst (type));
>   set_vinfo_for_stmt (new_stmt, stmt_info);
> +  /* For pattern statements make the related statement to point to
> +     NEW_STMT in order to be able to retrieve the original statement
> +     information later.  */
> +  if (is_pattern_stmt_p (stmt_info))
> +    {
> +      gimple related = STMT_VINFO_RELATED_STMT (stmt_info);
> +      STMT_VINFO_RELATED_STMT (vinfo_for_stmt (related)) = new_stmt;
> +    }
>   set_vinfo_for_stmt (stmt, NULL);
>   STMT_VINFO_STMT (stmt_info) = new_stmt;
>   gsi_replace (gsi, new_stmt, false);
> @@ -4957,11 +4965,7 @@ vect_transform_stmt (gimple stmt, gimple_stmt_iter
>             the stmt_info of ORIG_STMT_IN_PATTERN.  See more details in the
>             documentation of vect_pattern_recog.  */
>          if (STMT_VINFO_IN_PATTERN_P (stmt_vinfo))
> -           {
> -             gcc_assert (STMT_VINFO_RELATED_STMT (stmt_vinfo)
> -                           == orig_scalar_stmt);
> -             STMT_VINFO_VEC_STMT (stmt_vinfo) = vec_stmt;
> -           }
> +           STMT_VINFO_VEC_STMT (stmt_vinfo) = vec_stmt;
>        }
>     }
>

Re: [patch] Fix PR tree-optimization/50178

[Jakub Jelinek]

On Thu, Sep 01, 2011 at 10:14:29AM +0200, Richard Guenther wrote:
> On Thu, Sep 1, 2011 at 10:12 AM, Ira Rosen  wrote:
> > When vectorizing a function call we replace the original call with a
> > dummy statement to ensure that DCE later removes it. We also remove
> > its stmt_vec_info, which causes the segfault when we try to access it
> > through related pattern stmt. The following patch updates related
> > pattern stmt to be the dummy stmt.
> >
> > Bootstrapped and tested on powerpc64-suse-linux.
> > OK for 4.6?
> 
> Ok.

Please also commit the testcase into the trunk.

> >     PR tree-optimization/50178
> >     * tree-vect-stmts.c (vectorizable_call): Update the related
> > pattern statement
> >     before deleting the original call.
> >     (vect_transform_stmt): Don't expect the related pattern statement match 
> > the
> >     original statement after transformation.
> >
> > testsuite/ChangeLog:
> >
> >     PR tree-optimization/50178
> >     * gfortran.dg/vect/pr50178.f90: New test.

Jakub

Re: [patch] Fix PR tree-optimization/50178

[Ira Rosen]

On 1 September 2011 11:16, Jakub Jelinek  wrote:
> On Thu, Sep 01, 2011 at 10:14:29AM +0200, Richard Guenther wrote:
>> On Thu, Sep 1, 2011 at 10:12 AM, Ira Rosen  wrote:
>> > When vectorizing a function call we replace the original call with a
>> > dummy statement to ensure that DCE later removes it. We also remove
>> > its stmt_vec_info, which causes the segfault when we try to access it
>> > through related pattern stmt. The following patch updates related
>> > pattern stmt to be the dummy stmt.
>> >
>> > Bootstrapped and tested on powerpc64-suse-linux.
>> > OK for 4.6?
>>
>> Ok.
>
> Please also commit the testcase into the trunk.

Tested on powerpc64-suse-linux.
Committed to trunk.

Ira

testsuite/ChangeLog:

PR tree-optimization/50178
* gfortran.dg/vect/pr50178.f90: New test.

Index: testsuite/gfortran.dg/vect/pr50178.f90
===
--- testsuite/gfortran.dg/vect/pr50178.f90  (revision 0)
+++ testsuite/gfortran.dg/vect/pr50178.f90  (revision 0)
@@ -0,0 +1,29 @@
+! { dg-do compile }
+
+module yemdyn
+   implicit none
+   integer, parameter :: jpim = selected_int_kind(9)
+   integer, parameter :: jprb = selected_real_kind(13,300)
+   real(kind=jprb) :: elx
+   real(kind=jprb), allocatable :: xkcoef(:)
+   integer(kind=jpim),allocatable :: ncpln(:), npne(:)
+end module yemdyn
+
+subroutine suedyn
+
+   use yemdyn
+
+   implicit none
+
+   integer(kind=jpim) :: jm, jn
+   real(kind=jprb) :: zjm, zjn, zxxx
+
+   jn=0
+   do jm=0,ncpln(jn)
+  zjm=real(jm,jprb) / elx
+  xkcoef(npne(jn)+jm) = - zxxx*(zjm**2)**0.5_jprb
+   end do
+
+end subroutine suedyn
+
+! { dg-final { cleanup-tree-dump "vect" } }


>
>> >     PR tree-optimization/50178
>> >     * tree-vect-stmts.c (vectorizable_call): Update the related
>> > pattern statement
>> >     before deleting the original call.
>> >     (vect_transform_stmt): Don't expect the related pattern statement 
>> > match the
>> >     original statement after transformation.
>> >
>> > testsuite/ChangeLog:
>> >
>> >     PR tree-optimization/50178
>> >     * gfortran.dg/vect/pr50178.f90: New test.
>
>        Jakub
>

Re: [PATCH] Make vectorizer dumps more comparable

[Ira Rosen]

Richard Guenther  wrote on 01/09/2011 11:13:29 AM:

> > > > IMO it's a bad idea. It's now impossible to find anything when
> > compiling a
> > > > big file. How about only removing the file name?
> > >
> > > How about, as Micha suggested, print the location of the loop
> > > we currently investigate from vectorize_loops () where we
> > > call find_loop_location () instead?
> >
> > The problem is that a dump of a single loop can be pretty long, and
"start
> > to analyze loop..."/"finish to analyze loop..." may be not visible
enough.
> > I am OK with adding these printings though (in addition to line
numbers).
> >
> > I understand why you didn't like to see the file location, but what's
the
> > problem with the line number?
>
> Well, it seems to be different what everybody else does and it's
> highly redundant for a whole bunch of lines.
>
> But, it solves my diff issue and the overly long lines as well.
>
> Your patch changes both dump-file and stderr printing though,
> I did want to preserve stderr printing.

OK.

>
> For the dump-file I'd drop the 'line ' prefix and just print '%d: '.

OK.

>
> Btw, the diagnostic machinery does _not_ print locations
> for note (""), the location information is supposed to be printed
> in the heading warning/error.  Thus, a much better format for stderr
> would be
>
> file.c:12: LOOP NOT VECTORIZED
> note: unsupported stmt ''
>
> as the further notes will be printed with the 'loop location' which
> is confusing when dumping statements

We usually print only one line, like

file.c:12: note:  

so I don't really understand this part.

Ira

>
> Richard.
>
> > Ira
> >
> > >
> > > Richard.
> > >
> > > > Index: tree-vectorizer.c
> > > > ===
> > > > --- tree-vectorizer.c   (revision 178374)
> > > > +++ tree-vectorizer.c   (working copy)
> > > > @@ -149,16 +149,12 @@ vect_print_dump_info (enum vect_verbosit
> > > >if (!current_function_decl || !vect_dump)
> > > >  return false;
> > > >
> > > > -  if (dump_file)
> > > > -fprintf (vect_dump, "\n");
> > > > -
> > > > -  else if (vect_location == UNKNOWN_LOC)
> > > > -fprintf (vect_dump, "\n%s:%d: note: ",
> > > > -DECL_SOURCE_FILE (current_function_decl),
> > > > +  if (vect_location == UNKNOWN_LOC)
> > > > +fprintf (vect_dump, "\nline %d: ",
> > > >  DECL_SOURCE_LINE (current_function_decl));
> > > >else
> > > > -fprintf (vect_dump, "\n%s:%d: note: ",
> > > > -LOC_FILE (vect_location), LOC_LINE (vect_location));
> > > > +fprintf (vect_dump, "\nline %d: ",
> > > > +LOC_LINE (vect_location));
> > > >
> > > >return true;
> > > >  }
> > > >
> > > > Ira
> > > >
> > > > >
> > > > > Index: gcc/tree-vectorizer.c
> > > > >
===
> > > > > --- gcc/tree-vectorizer.c   (revision 178028)
> > > > > +++ gcc/tree-vectorizer.c   (working copy)
> > > > > @@ -149,7 +149,10 @@ vect_print_dump_info (enum vect_verbosit
> > > > >if (!current_function_decl || !vect_dump)
> > > > >  return false;
> > > > >
> > > > > -  if (vect_location == UNKNOWN_LOC)
> > > > > +  if (dump_file)
> > > > > +fprintf (vect_dump, "\n");
> > > > > +
> > > > > +  else if (vect_location == UNKNOWN_LOC)
> > > > >  fprintf (vect_dump, "\n%s:%d: note: ",
> > > > >  DECL_SOURCE_FILE (current_function_decl),
> > > > >  DECL_SOURCE_LINE (current_function_decl));
> > > > >
> > > >
> > > >
> > >
> > > --
> > > Richard Guenther 
> > > SUSE / SUSE Labs
> > > SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
> > > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
> >
> >
>
> --
> Richard Guenther 
> SUSE / SUSE Labs
> SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

Re: [PATCH Atom][PR middle-end/44382] Tree reassociation improvement

[Richard Guenther]

On Wed, Aug 31, 2011 at 2:17 PM, Ilya Enkovich  wrote:
> Hello Richard,
>
> Thanks for the review!
>
>> The fact that you have to adjust gcc.dg/tree-ssa/pr38533.c looks problematic
>> to me.  Can you investigate the problem report, look at the geneated
>> code with the atom default of the param and see whether it's still
>> reasonably "fixed" (maybe you'd already done this)?
> This test was created as a regression test to the problem in
> linearize_expr_tree which moves all statements down to the first
> modified one during reassociation increasing registers pressure. Test
> has a lot of definitions which are all ORed like this:
>  def r1
>  def r2
>  s = r1 or r2
>  def r3
>  s = s or r3
>  def r4
>  s = s or r4
>  ...
> and it checks that register pressure is not increased after
> reassociation by simple scan of two sequential defs. If we use
> reassociation width higher than 1 then test will fails because we need
> to increase register pressure to get parallelism and finally get code
> like this:
>  def r1
>  def r2
>  def r3
>  t1 = r1 or r2
>  s = s or r3
>  def r4
>  def r5
>  s = s or t1
>  t1 = r4 or r5
>  ...
> So, I had to fix a test.

Ok.  Thanks for explaining.

>> +  /* Check if we may use less width and still compute sequence for
>> +     the same time.  It will allow us to reduce registers usage.  */
>> +  while (width > 1
>> +        && get_required_cycles (ops_num, width - 1) == cycles_best)
>> +    width--;
>>
>> I suppose get_required_cycles () is monotonic in width?  Would it
>> make sense to binary search the best width then (I realize the
>> default is 1 and the only target differing has 2, but ...)?  Maybe at
>> least add a comment to this effect?  Or not decrement by one
>> but instead divide by two on each iteration (which is the same for 1 and 2 
>> ...)?
>> It's also all a mapping that is constant - we should be able to
>> pre-compute it for all values, eventually "compressing" it to a
>> much simpler formula width = f (cpu_width, ops_num)?
> I replaced sequential search with a binary search. I did not pay much
> attention to this function because do not think it is really time
> consuming compared to other parts of reassociation phase. Am I too
> optimistic? If you think it might significantly affect compile time I
> can introduce a table of pre-computed values (or make it later as a
> separate fix).

+  /* Get the minimal time required for sequence computation.  */
+  cycles_best = get_required_cycles (ops_num, width);
+
+  /* Check if we may use less width and still compute sequence for
+ the same time.  It will allow us to reduce registers usage.  */
+  width_min = 1;
+  while (width > width_min)
+{
+  int width_mid = (width + width_min) / 2;
+
+  if (get_required_cycles (ops_num, width_mid) == cycles_best)
+   width = width_mid;
+  else if (width_min < width_mid)
+   width_min = width_mid;
+  else
+   break;
+}

this seems to not allow cycles_best to drop with lower width, but
that it can't should be an implementation detail of get_required_cycles.
To make it not so, can you add a comment before the loop, like

  /* get_required_cycles is monotonically increasing with lower width
 so we can perform a binary search for the minimal width that still
 results in the optimal cycle count.  */

> I made all other fixes as you suggested.

With the above change the non-x86 specifc parts are ok.  Please get
approval for them from a x86 maintainer.

Thanks,
Richard.


> Bootstrapped and checked on x86_64-linux.
>
> Thanks,
> Ilya
> ---
> gcc/
>
> 2011-08-31  Enkovich Ilya  
>
>        PR middle-end/44382
>        * target.def (reassociation_width): New hook.
>
>        * doc/tm.texi.in (reassociation_width): Likewise.
>
>        * doc/tm.texi (reassociation_width): Likewise.
>
>        * doc/invoke.texi (tree-reassoc-width): New param documented.
>
>        * hooks.h (hook_int_uint_mode_1): New default hook.
>
>        * hooks.c (hook_int_uint_mode_1): Likewise.
>
>        * config/i386/i386.h (ix86_tune_indices): Add
>        X86_TUNE_REASSOC_INT_TO_PARALLEL and
>        X86_TUNE_REASSOC_FP_TO_PARALLEL.
>
>        (TARGET_REASSOC_INT_TO_PARALLEL): New.
>        (TARGET_REASSOC_FP_TO_PARALLEL): Likewise.
>
>        * config/i386/i386.c (initial_ix86_tune_features): Add
>        X86_TUNE_REASSOC_INT_TO_PARALLEL and
>        X86_TUNE_REASSOC_FP_TO_PARALLEL.
>
>        (ix86_reassociation_width) implementation of
>        new hook for i386 target.
>
>        * params.def (PARAM_TREE_REASSOC_WIDTH): New param added.
>
>        * tree-ssa-reassoc.c (get_required_cycles): New function.
>        (get_reassociation_width): Likewise.
>        (swap_ops_for_binary_stmt): Likewise.
>        (rewrite_expr_tree_parallel): Likewise.
>
>        (rewrite_expr_tree): Refactored. Part of code moved into
>        swap_ops_for_binary_stmt.
>
>        (reassociate_bb): Now checks reassociation width to be used
>        and call rewrite_expr_tree_parallel inst

Re: [PATCH] Make vectorizer dumps more comparable

[Richard Guenther]

On Thu, 1 Sep 2011, Ira Rosen wrote:

> 
> 
> Richard Guenther  wrote on 01/09/2011 11:13:29 AM:
> 
> > > > > IMO it's a bad idea. It's now impossible to find anything when
> > > compiling a
> > > > > big file. How about only removing the file name?
> > > >
> > > > How about, as Micha suggested, print the location of the loop
> > > > we currently investigate from vectorize_loops () where we
> > > > call find_loop_location () instead?
> > >
> > > The problem is that a dump of a single loop can be pretty long, and
> "start
> > > to analyze loop..."/"finish to analyze loop..." may be not visible
> enough.
> > > I am OK with adding these printings though (in addition to line
> numbers).
> > >
> > > I understand why you didn't like to see the file location, but what's
> the
> > > problem with the line number?
> >
> > Well, it seems to be different what everybody else does and it's
> > highly redundant for a whole bunch of lines.
> >
> > But, it solves my diff issue and the overly long lines as well.
> >
> > Your patch changes both dump-file and stderr printing though,
> > I did want to preserve stderr printing.
> 
> OK.
> 
> >
> > For the dump-file I'd drop the 'line ' prefix and just print '%d: '.
> 
> OK.
> 
> >
> > Btw, the diagnostic machinery does _not_ print locations
> > for note (""), the location information is supposed to be printed
> > in the heading warning/error.  Thus, a much better format for stderr
> > would be
> >
> > file.c:12: LOOP NOT VECTORIZED
> > note: unsupported stmt ''
> >
> > as the further notes will be printed with the 'loop location' which
> > is confusing when dumping statements
> 
> We usually print only one line, like
> 
> file.c:12: note:  
> 
> so I don't really understand this part.

It's a general note.  With -ftree-vectorizer-verbose=5 we dump a lot
of information with the same location (that of the loop header),
but the individual messages refer not only to the overall loop
but to specific statements, etc.

This all is of course an artifact of sharing the dump file code
with the reporting code.

Richard.

> Ira
> 
> >
> > Richard.
> >
> > > Ira
> > >
> > > >
> > > > Richard.
> > > >
> > > > > Index: tree-vectorizer.c
> > > > > ===
> > > > > --- tree-vectorizer.c   (revision 178374)
> > > > > +++ tree-vectorizer.c   (working copy)
> > > > > @@ -149,16 +149,12 @@ vect_print_dump_info (enum vect_verbosit
> > > > >if (!current_function_decl || !vect_dump)
> > > > >  return false;
> > > > >
> > > > > -  if (dump_file)
> > > > > -fprintf (vect_dump, "\n");
> > > > > -
> > > > > -  else if (vect_location == UNKNOWN_LOC)
> > > > > -fprintf (vect_dump, "\n%s:%d: note: ",
> > > > > -DECL_SOURCE_FILE (current_function_decl),
> > > > > +  if (vect_location == UNKNOWN_LOC)
> > > > > +fprintf (vect_dump, "\nline %d: ",
> > > > >  DECL_SOURCE_LINE (current_function_decl));
> > > > >else
> > > > > -fprintf (vect_dump, "\n%s:%d: note: ",
> > > > > -LOC_FILE (vect_location), LOC_LINE (vect_location));
> > > > > +fprintf (vect_dump, "\nline %d: ",
> > > > > +LOC_LINE (vect_location));
> > > > >
> > > > >return true;
> > > > >  }
> > > > >
> > > > > Ira
> > > > >
> > > > > >
> > > > > > Index: gcc/tree-vectorizer.c
> > > > > >
> ===
> > > > > > --- gcc/tree-vectorizer.c   (revision 178028)
> > > > > > +++ gcc/tree-vectorizer.c   (working copy)
> > > > > > @@ -149,7 +149,10 @@ vect_print_dump_info (enum vect_verbosit
> > > > > >if (!current_function_decl || !vect_dump)
> > > > > >  return false;
> > > > > >
> > > > > > -  if (vect_location == UNKNOWN_LOC)
> > > > > > +  if (dump_file)
> > > > > > +fprintf (vect_dump, "\n");
> > > > > > +
> > > > > > +  else if (vect_location == UNKNOWN_LOC)
> > > > > >  fprintf (vect_dump, "\n%s:%d: note: ",
> > > > > >  DECL_SOURCE_FILE (current_function_decl),
> > > > > >  DECL_SOURCE_LINE (current_function_decl));
> > > > > >
> > > > >
> > > > >
> > > >
> > > > --
> > > > Richard Guenther 
> > > > SUSE / SUSE Labs
> > > > SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
> > > > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
> > >
> > >
> >
> > --
> > Richard Guenther 
> > SUSE / SUSE Labs
> > SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
> > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
> 
> 

-- 
Richard Guenther 
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

Re: [Ada] Implementation of aspects within generic units

[Arnaud Charlet]

After doing a binary search, the first revision which breaks bootstrap on
my environment with Ada enabled is the following:

r178353 | vries | 2011-08-31 09:04:25 +0200 (Wed, 31 Aug 2011) | 8 lines

2011-08-31  Tom de Vries  

PR middle-end/43513
* Makefile.in (tree-ssa-ccp.o): Add $(PARAMS_H) to rule.
* tree-ssa-ccp.c (params.h): Include.
(fold_builtin_alloca_for_var): New function.
(ccp_fold_stmt): Use fold_builtin_alloca_for_var.

Which makes sense, since Ada uses alloca a lot, much more than other languages.

In other words, none of the changes in the Ada repository is reponsible for
this regression.

So Tom and/or Richard, could you please have a look at this regression? TIA.

Arno

Re: [Ada] Implementation of aspects within generic units

[Dominique Dhumieres]

> After doing a binary search, the first revision which breaks bootstrap on
> my environment with Ada enabled is the following:
> 
> r178353 | vries | 2011-08-31 09:04:25 +0200 (Wed, 31 Aug 2011) | 8 lines
> ...

This is probably related to pr50251 also caused by r178353.

Dominique

Re: [Ada] Implementation of aspects within generic units - PR50251

[Arnaud Charlet]

> > After doing a binary search, the first revision which breaks bootstrap on
> > my environment with Ada enabled is the following:
> > 
> > r178353 | vries | 2011-08-31 09:04:25 +0200 (Wed, 31 Aug 2011) | 8 lines
> > ...
> 
> This is probably related to pr50251 also caused by r178353.

Thanks. So this means we have test cases in other languages than Ada also
to investigate then (C and Fortran).

Re: [PATCH Atom][PR middle-end/44382] Tree reassociation improvement

[Ilya Enkovich]

>
> this seems to not allow cycles_best to drop with lower width, but
> that it can't should be an implementation detail of get_required_cycles.
> To make it not so, can you add a comment before the loop, like
>
>  /* get_required_cycles is monotonically increasing with lower width
>     so we can perform a binary search for the minimal width that still
>     results in the optimal cycle count.  */
>

Fixed. Thanks!

>
> With the above change the non-x86 specifc parts are ok.  Please get
> approval for them from a x86 maintainer.
>

Could please someone review x86 part?

Thanks,
Ilya
---
gcc/

2011-09-01  Enkovich Ilya  

PR middle-end/44382
* target.def (reassociation_width): New hook.

* doc/tm.texi.in (reassociation_width): Likewise.

* doc/tm.texi (reassociation_width): Likewise.

* doc/invoke.texi (tree-reassoc-width): New param documented.

* hooks.h (hook_int_uint_mode_1): New default hook.

* hooks.c (hook_int_uint_mode_1): Likewise.

* config/i386/i386.h (ix86_tune_indices): Add
X86_TUNE_REASSOC_INT_TO_PARALLEL and
X86_TUNE_REASSOC_FP_TO_PARALLEL.

(TARGET_REASSOC_INT_TO_PARALLEL): New.
(TARGET_REASSOC_FP_TO_PARALLEL): Likewise.

* config/i386/i386.c (initial_ix86_tune_features): Add
X86_TUNE_REASSOC_INT_TO_PARALLEL and
X86_TUNE_REASSOC_FP_TO_PARALLEL.

(ix86_reassociation_width) implementation of
new hook for i386 target.

* params.def (PARAM_TREE_REASSOC_WIDTH): New param added.

* tree-ssa-reassoc.c (get_required_cycles): New function.
(get_reassociation_width): Likewise.
(swap_ops_for_binary_stmt): Likewise.
(rewrite_expr_tree_parallel): Likewise.

(rewrite_expr_tree): Refactored. Part of code moved into
swap_ops_for_binary_stmt.

(reassociate_bb): Now checks reassociation width to be used
and call rewrite_expr_tree_parallel instead of rewrite_expr_tree
if needed.

gcc/testsuite/

2011-09-01  Enkovich Ilya  

* gcc.dg/tree-ssa/pr38533.c (dg-options): Added option
--param tree-reassoc-width=1.

* gcc.dg/tree-ssa/reassoc-24.c: New test.
* gcc.dg/tree-ssa/reassoc-25.c: Likewise.


PR44382.diff
Description: Binary data

[Ada] Add queue containers to standard library

[Arnaud Charlet]

Ada 2012 added queue containers to the standard library; see AI05-0159 for
details.

Tested on x86_64-pc-linux-gnu, committed on trunk

2011-09-01  Matthew Heaney  

* Makefile.rtl, impunit.adb: Add a-csquin.ads, a-cusyqu.ad[sb],
a-cuprqu.ad[sb], a-cbsyqu.ad[sb], a-cbprqu.ad[sb]
* a-csquin.ads: New Ada 2012 unit that specifies the queue interface
* a-cusyqu.ads, a-cusyqu.adb: New Ada 2012 unit that specifies the
unbounded queue container.
* a-cbsyqu.ads, a-cbsyqu.adb: New Ada 2012 unit that specifies the
bounded queue container.
* a-cuprqu.ads, a-cuprqu.adb: New Ada 2012 unit that specifies the
unbounded priority queue container.
* a-cbprqu.ads, a-cbprqu.adb: New Ada 2012 unit that specifies the
bounded priority queue container.

Index: impunit.adb
===
--- impunit.adb (revision 178381)
+++ impunit.adb (working copy)
@@ -519,6 +519,11 @@
  "a-comutr",-- Ada.Containers.Multiway_Trees
  "a-cimutr",-- Ada.Containers.Indefinite_Multiway_Trees
  "a-cbmutr",-- Ada.Containers.Bounded_Multiway_Trees
+ "a-csquin",-- Ada.Containers.Synchronized_Queue_Interfaces
+ "a-cusyqu",-- Ada.Containers.Unbounded_Synchronized_Queues
+ "a-cuprqu",-- Ada.Containers.Unbounded_Priority_Queues
+ "a-cbsyqu",-- Ada.Containers.Bounded_Synchronized_Queues
+ "a-cbprqu",-- Ada.Containers.Bounded_Priority_Queues
  "a-extiin",-- Ada.Execution_Time.Interrupts
  "a-iteint",-- Ada.Iterator_Interfaces
  "a-synbar",-- Ada.Synchronous_Barriers
Index: a-cusyqu.adb
===
--- a-cusyqu.adb(revision 0)
+++ a-cusyqu.adb(revision 0)
@@ -0,0 +1,174 @@
+--
+--  --
+-- GNAT LIBRARY COMPONENTS  --
+--  --
+--   ADA.CONTAINERS.UNBOUNDED_SYNCHRONIZED_QUEUES   --
+--  --
+-- B o d y  --
+--  --
+--  Copyright (C) 2004-2011, Free Software Foundation, Inc. --
+--  --
+-- GNAT is free software;  you can  redistribute it  and/or modify it under --
+-- terms of the  GNU General Public License as published  by the Free Soft- --
+-- ware  Foundation;  either version 3,  or (at your option) any later ver- --
+-- sion.  GNAT is distributed in the hope that it will be useful, but WITH- --
+-- OUT ANY WARRANTY;  without even the  implied warranty of MERCHANTABILITY --
+-- or FITNESS FOR A PARTICULAR PURPOSE. --
+--  --
+-- As a special exception 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--
+-- .  --
+--  --
+-- This unit was originally developed by Matthew J Heaney.  --
+--
+
+with Ada.Unchecked_Deallocation;
+
+package body Ada.Containers.Unbounded_Synchronized_Queues is
+
+   package body Implementation is
+
+  ---
+  -- Local Subprograms --
+  ---
+
+  procedure Free is
+ new Ada.Unchecked_Deallocation (Node_Type, Node_Access);
+
+  -
+  -- Dequeue --
+  -
+
+  procedure Dequeue
+(List: in out List_Type;
+ Element : out Queue_Interfaces.Element_Type)
+  is
+ X : Node_Access;
+
+  begin
+ Element := List.First.Element;
+
+ X := List.First;
+ List.First := List.First.Next;
+
+ if List.First = null then
+List.Last := null;
+ end if;
+
+ List.Length := List.Length - 1;
+
+ Free (X);
+  end Dequeue;
+
+  -
+  -- Enqueue --
+  -

Re: [Ada] Implementation of aspects within generic units

[Richard Guenther]

On Thu, Sep 1, 2011 at 11:34 AM, Arnaud Charlet  wrote:
> After doing a binary search, the first revision which breaks bootstrap on
> my environment with Ada enabled is the following:
>
> r178353 | vries | 2011-08-31 09:04:25 +0200 (Wed, 31 Aug 2011) | 8 lines
>
> 2011-08-31  Tom de Vries  
>
>        PR middle-end/43513
>        * Makefile.in (tree-ssa-ccp.o): Add $(PARAMS_H) to rule.
>        * tree-ssa-ccp.c (params.h): Include.
>        (fold_builtin_alloca_for_var): New function.
>        (ccp_fold_stmt): Use fold_builtin_alloca_for_var.
>
> Which makes sense, since Ada uses alloca a lot, much more than other 
> languages.
>
> In other words, none of the changes in the Ada repository is reponsible for
> this regression.
>
> So Tom and/or Richard, could you please have a look at this regression? TIA.

Probably worth checking

Index: gcc/tree-ssa-ccp.c
===
--- gcc/tree-ssa-ccp.c  (revision 178394)
+++ gcc/tree-ssa-ccp.c  (working copy)
@@ -1714,7 +1714,7 @@ fold_builtin_alloca_for_var (gimple stmt
   block = gimple_block (stmt);
   if (!(cfun->after_inlining
 && TREE_CODE (BLOCK_SUPERCONTEXT (block)) == FUNCTION_DECL))
-threshold /= 10;
+return NULL_TREE;
   if (size > threshold)
 return NULL_TREE;

should be reproducable with low stack ulimit.

Richard.

> Arno
>

[Ada] Add support for the CPU aspect

[Arnaud Charlet]

This patch adds the support for the CPU aspect to control the allocation of
tasks to processors.

The following test case should compile quietly in Ada 2012 mode, and execute
without any message (task T should execute on the first processor):

pragma Ada_2012;

procedure Test_CPU_Aspect is
   task T with CPU => 1;

   task body T is
   begin
  null;
   end T;
begin
   null;
end Test_CPU_Aspect;

Tested on x86_64-pc-linux-gnu, committed on trunk

2011-09-01  Jose Ruiz  

* aspects.ads (Aspect_Id, Aspect_Argument, Aspect_Names): Add the CPU
aspect.
* aspects.adb (Canonical_Aspect): Add entry for the CPU aspect.
* sem_ch13.adb (Analyze_Aspect_Specifications): Analyze the CPU aspect
in a similar way as we do for the Priority or Dispatching_Domain aspect.
* s-mudido-affinity.adb (Dispatching_Domain_Tasks,
Dispatching_Domains_Frozen): Move this local data to package
System.Tasking because with the CPU aspect we need to have access
to this data when creating the task in System.Tasking.Stages.Create_Task
* s-taskin.ads (Dispatching_Domain_Tasks, Dispatching_Domains_Frozen):
Move these variables from the body of
System.Multiprocessors.Dispatching_Domains because with the CPU aspect
we need to have access to this data when creating the task in
System.Tasking.Stages.Create_Task.
* s-taskin.adb (Initialize): Signal the allocation of the environment
task to a CPU, if any, so that we know whether the CPU can be
transferred to a different dispatching domain.
* s-tassta.adb (Create_Task): Check whether the CPU to which this task
is being allocated belongs to the dispatching domain where the task
lives. Signal the allocation of the task to a CPU, if any, so that we
know whether the CPU can be transferred to a different dispatching
domain.

Index: s-tassta.adb
===
--- s-tassta.adb(revision 178381)
+++ s-tassta.adb(working copy)
@@ -493,6 +493,8 @@
   Len   : Natural;
   Base_CPU  : System.Multiprocessors.CPU_Range;
 
+  use type System.Multiprocessors.CPU_Range;
+
   pragma Unreferenced (Relative_Deadline);
   --  EDF scheduling is not supported by any of the target platforms so
   --  this parameter is not passed any further.
@@ -540,10 +542,6 @@
 else System.Multiprocessors.CPU_Range (CPU));
   end if;
 
-  --  ??? If we want to handle the interaction between pragma CPU and
-  --  dispatching domains we would need to signal that this task is being
-  --  allocated to a processor.
-
   --  Find parent P of new Task, via master level number
 
   P := Self_ID;
@@ -658,6 +656,36 @@
   Unlock (Self_ID);
   Unlock_RTS;
 
+  --  The CPU associated to the task (if any) must belong to the
+  --  dispatching domain.
+
+  if Base_CPU /= System.Multiprocessors.Not_A_Specific_CPU and then
+(Base_CPU not in T.Common.Domain'Range
+ or else not T.Common.Domain (Base_CPU))
+  then
+ Initialization.Undefer_Abort_Nestable (Self_ID);
+ raise Tasking_Error with "CPU not in dispatching domain";
+  end if;
+
+  --  In order to handle the interaction between pragma CPU and
+  --  dispatching domains we need to signal that this task is being
+  --  allocated to a processor. This is needed only for tasks belonging to
+  --  the system domain (the creation of new dispatching domains can only
+  --  take processors from the system domain) and only before the
+  --  environment task calls the main procedure (dispatching domains cannot
+  --  be created after this).
+
+  if Base_CPU /= System.Multiprocessors.Not_A_Specific_CPU
+and then T.Common.Domain = System.Tasking.System_Domain
+and then not System.Tasking.Dispatching_Domains_Frozen
+  then
+ --  Increase the number of tasks attached to the CPU to which this
+ --  task is being moved.
+
+ Dispatching_Domain_Tasks (Base_CPU) :=
+   Dispatching_Domain_Tasks (Base_CPU) + 1;
+  end if;
+
   --  Note: we should not call 'new' while holding locks since new
   --  may use locks (e.g. RTS_Lock under Windows) itself and cause a
   --  deadlock.
Index: s-mudido-affinity.adb
===
--- s-mudido-affinity.adb   (revision 178381)
+++ s-mudido-affinity.adb   (working copy)
@@ -41,21 +41,6 @@
 
package ST renames System.Tasking;
 
-   
-   -- Local data --
-   
-
-   Dispatching_Domain_Tasks : array (CPU'First .. Number_Of_CPUs) of Natural :=
-(others => 0);
-   --  We need to store whether there are tasks allocated to concrete
-   --  processors in the default system dispatching domain because 

[Ada] Implement aliased parameters for Ada 2012 (AI 142)

[Arnaud Charlet]

This patch is the first step for implementation of aliased parameters
in Ada 2012 mode. It recognizes the keyword aliased on parameters and
ensures that they are passed by reference. The following program
prints out the value 5.

 1. pragma Ada_2012;
 2. with Text_IO; use Text_IO;
 3. procedure AliasP1 is
 4.G : Integer;
 5.
 6.procedure K  (R : aliased in out Integer) is
 7.begin
 8.   G := 2;
 9.   R := 5;
10.   Put_Line (G'Img);
11.end;
12. begin
13.K (G);
14. end;

This is only the first step, We have not dealt with access checking
etc yet that is required.

Tested on x86_64-pc-linux-gnu, committed on trunk

2011-09-01  Robert Dewar  

* einfo.ads (Is_Aliased): Fix existing documentation and add note on
possibility of this flag being set for formals in Ada 2012 mode.
* par-ch6.adb (P_Formal_Part): Handle aliased for parameters for Ada
2012.
* sem_ch6.adb (Process_Formals): Handle aliased parameters in Ada 2012
mode.
* sinfo.adb (Aliased_Present): Allowed in N_Parameter_Specification for
Ada 2012.
* sinfo.ads (Aliased_Present): Allowed in N_Parameter_Specification for
Ada 2012.

Index: sinfo.adb
===
--- sinfo.adb   (revision 178381)
+++ sinfo.adb   (working copy)
@@ -206,7 +206,8 @@
begin
   pragma Assert (False
 or else NT (N).Nkind = N_Component_Definition
-or else NT (N).Nkind = N_Object_Declaration);
+or else NT (N).Nkind = N_Object_Declaration
+or else NT (N).Nkind = N_Parameter_Specification);
   return Flag4 (N);
end Aliased_Present;
 
@@ -3265,7 +3266,8 @@
begin
   pragma Assert (False
 or else NT (N).Nkind = N_Component_Definition
-or else NT (N).Nkind = N_Object_Declaration);
+or else NT (N).Nkind = N_Object_Declaration
+or else NT (N).Nkind = N_Parameter_Specification);
   Set_Flag4 (N, Val);
end Set_Aliased_Present;
 
Index: sinfo.ads
===
--- sinfo.ads   (revision 178381)
+++ sinfo.ads   (working copy)
@@ -2322,7 +2322,7 @@
   --  N_Object_Declaration
   --  Sloc points to first identifier
   --  Defining_Identifier (Node1)
-  --  Aliased_Present (Flag4) set if ALIASED appears
+  --  Aliased_Present (Flag4)
   --  Constant_Present (Flag17) set if CONSTANT appears
   --  Null_Exclusion_Present (Flag11)
   --  Object_Definition (Node4) subtype indic./array type def./access def.
@@ -4514,8 +4514,8 @@
   --
 
   --  PARAMETER_SPECIFICATION ::=
-  --DEFINING_IDENTIFIER_LIST : MODE [NULL_EXCLUSION] SUBTYPE_MARK
-  --  [:= DEFAULT_EXPRESSION]
+  --DEFINING_IDENTIFIER_LIST : [ALIASED] MODE [NULL_EXCLUSION]
+  --  SUBTYPE_MARK [:= DEFAULT_EXPRESSION]
   --  | DEFINING_IDENTIFIER_LIST : ACCESS_DEFINITION
   --  [:= DEFAULT_EXPRESSION]
 
@@ -4527,9 +4527,12 @@
   --  Prev_Ids flags to preserve the original source form as described
   --  in the section on "Handling of Defining Identifier Lists".
 
+  --  ALIASED can only be present in Ada 2012 mode
+
   --  N_Parameter_Specification
   --  Sloc points to first identifier
   --  Defining_Identifier (Node1)
+  --  Aliased_Present (Flag4)
   --  In_Present (Flag15)
   --  Out_Present (Flag17)
   --  Null_Exclusion_Present (Flag11)
Index: einfo.ads
===
--- einfo.ads   (revision 178381)
+++ einfo.ads   (working copy)
@@ -1997,8 +1997,9 @@
 --   of pragma Ada_12 or Ada_2012.
 
 --Is_Aliased (Flag15)
---   Present in objects whose declarations carry the keyword aliased,
---   and on record components that have the keyword.
+--   Present in all entities. Set for objects and types whose declarations
+--   carry the keyword aliased, and on record components that have the
+--   keyword. For Ada 2012, also applies to formal parameters.
 
 --Is_AST_Entry (Flag132)
 --   Present in entry entities. Set if a valid pragma AST_Entry applies
@@ -4773,6 +4774,7 @@
--Is_Ada_2005_Only(Flag185)
--Is_Ada_2012_Only(Flag199)
--Is_Bit_Packed_Array (Flag122)  (base type only)
+   --Is_Aliased  (Flag15)
--Is_Character_Type   (Flag63)
--Is_Child_Unit   (Flag73)
--Is_Compilation_Unit (Flag149)
@@ -4994,7 +4996,6 @@
--Component_Alignment (special)  (base type only)
--Has_Component_Size_Clause   (Flag68)   (base type only)
--Has_Pragma_Pack (Flag121)  (impl base type only)
-   --Is_Aliased 

[Ada] Expression_With_Actions and finalization

[Arnaud Charlet]

This patch modifies the expansion of Expression_With_Actions in order to handle
the finalization of a function call nested deep within the condition of an if
statement.


-- Source --


--  main.adb

with Ada.Finalization; use Ada.Finalization;
with Ada.Text_IO;  use Ada.Text_IO;

procedure Main is
   type Ctrl is new Controlled with record
 Id : Natural := 0;
   end record;

   procedure Finalize (Obj : in out Ctrl) is
   begin
  Put_Line ("  finalize Id:" & Obj.Id'Img);
   end Finalize;

   function Make_Ctrl (Id : Natural) return Ctrl is
   begin
  return (Controlled with Id => Id);
   end Make_Ctrl;

   function Equal (Left : Ctrl; Right : Ctrl) return Boolean is
   begin
  return Left.Id = Right.Id;
   end Equal;

   procedure Test
 (Obj : Ctrl;
  Id  : Natural;
  Check_1 : Integer;
  Check_2 : Integer)
   is
   begin
  Put_Line ("before if statement");
  if Check_1 > 1
and then
  (Check_2 > 1
 or else Equal (Obj, Make_Ctrl (Id)))
  then
 Put_Line ("inside if statement");
  end if;
   Put_Line ("after if statement");
   end Test;

   Id  : constant Natural := 1234;
   Obj : constant Ctrl := (Controlled with Id => Id);
begin
   Test (Obj, Id, 2, 1);
end Main;

-
-- Compilation and expected output --
-

$ gnatmake -q -gnat05 main.adb
$ ./main
before if statement
  finalize Id: 1234
  finalize Id: 1234
inside if statement
after if statement
  finalize Id: 1234

Tested on x86_64-pc-linux-gnu, committed on trunk

2011-09-01  Hristian Kirtchev  

* exp_ch4.adb (Find_Insertion_Node): New routine. Determines the proper
insertion node in a tree of nested Expression_With_Actions nodes.
(Process_Transient_Object): In the case where a complex if statement
has been converted into nested Expression_With_Actions nodes, the
"hook" object and the associated access type must be inserted before
the top most Expression_With_Actions.

Index: exp_ch4.adb
===
--- exp_ch4.adb (revision 178381)
+++ exp_ch4.adb (working copy)
@@ -4415,10 +4415,32 @@
   --
 
   procedure Process_Transient_Object (Decl : Node_Id) is
- Ins_Nod : constant Node_Id := Parent (N);
- --  To avoid the insertion of generated code in the list of Actions,
- --  Insert_Action must look at the parent field of the EWA.
 
+ function Find_Insertion_Node return Node_Id;
+ --  Complex if statements may be converted into nested EWAs. In this
+ --  case, any generated code must be inserted before the if statement
+ --  to ensure proper visibility of the "hook" objects. This routine
+ --  returns the top most short circuit operator or the parent of the
+ --  EWA if no nesting was detected.
+
+ -
+ -- Find_Insertion_Node --
+ -
+
+ function Find_Insertion_Node return Node_Id is
+Par : Node_Id := N;
+
+ begin
+--  Climb up the branches of a complex if statement
+
+while Nkind_In (Parent (Par), N_And_Then, N_Op_Not, N_Or_Else) loop
+   Par := Parent (Par);
+end loop;
+
+return Par;
+ end Find_Insertion_Node;
+
+ Ins_Nod   : constant Node_Id:= Find_Insertion_Node;
  Loc   : constant Source_Ptr := Sloc (Decl);
  Obj_Id: constant Entity_Id  := Defining_Identifier (Decl);
  Obj_Typ   : constant Entity_Id  := Etype (Obj_Id);

[Ada] finalization and controlled transient variable

[Arnaud Charlet]

This patch adds code to handle the finalization of a controlled transient
variable used as an actual of a subprogram call when the call raises an
exception.


-- Source --


--  main.adb:

with Ada.Finalization; use Ada.Finalization;
with Ada.Text_IO;  use Ada.Text_IO;

procedure Main is
   type Ctrl is new Controlled with record
  Id : Natural := 0;
   end record;

   procedure Adjust (Obj : in out Ctrl);
   procedure Finalize (Obj : in out Ctrl);

   procedure Adjust (Obj : in out Ctrl) is
  New_Id : constant Natural := Obj.Id + 1;
   begin
  Put_Line ("  adjust Id:" & Obj.Id'Img & " ->" & New_Id'Img);
  Obj.Id := New_Id;
   end Adjust;

   procedure Finalize (Obj : in out Ctrl) is
   begin
  Put_Line ("  finalize Id:" & Obj.Id'Img);
   end Finalize;

   function Make_Ctrl (Id : Natural) return Ctrl is
   begin
  return (Controlled with Id => Id);
   end Make_Ctrl;

   type Ctrl_Array is array (1 .. 1) of Ctrl;

   procedure Raise_PE (Do_It : Boolean; Objs : Ctrl_Array) is
  pragma Unreferenced (Objs);
   begin
  if Do_It then
 raise Program_Error;
  end if;
   end Raise_PE;

   Obj : constant Ctrl := Make_Ctrl (1);

begin
   Put_Line ("before exception");
   Raise_PE (True, (1 => Obj));
   Put_Line ("after exception");

exception
   when Program_Error => Put_Line ("exception caught");
end Main;

-
-- Compilation and expected output --
-

$ gnatmake -q -gnat05 main.adb
$ ./main
  adjust Id: 1 -> 2
  finalize Id: 1
  adjust Id: 2 -> 3
  finalize Id: 2
before exception
  adjust Id: 3 -> 4
  finalize Id: 4
exception caught
  finalize Id: 3

Tested on x86_64-pc-linux-gnu, committed on trunk

2011-09-01  Hristian Kirtchev  

* exp_ch7.adb (Find_Insertion_List): New routine.
(Process_Transient_Objects): Add code to handle the abnormal
finalization of a controlled transient associated with a subprogram
call. Since transients are cleaned up right after the associated
context, an exception raised during a subprogram call may bypass the
finalization code.

Index: exp_ch7.adb
===
--- exp_ch7.adb (revision 178381)
+++ exp_ch7.adb (working copy)
@@ -4198,18 +4198,52 @@
  Last_Object  : Node_Id;
  Related_Node : Node_Id)
   is
- Finalizer_Data  : Finalization_Exception_Data;
- Finalizer_Decls : List_Id;
- Built   : Boolean := False;
- Desig   : Entity_Id;
- Fin_Block   : Node_Id;
- Last_Fin: Node_Id := Empty;
- Loc : Source_Ptr;
- Obj_Id  : Entity_Id;
- Obj_Ref : Node_Id;
- Obj_Typ : Entity_Id;
- Stmt: Node_Id;
+ function Find_Insertion_List return List_Id;
+ --  Return the statement list of the enclosing sequence of statements
 
+ -
+ -- Find_Insertion_List --
+ -
+
+ function Find_Insertion_List return List_Id is
+Par : Node_Id;
+
+ begin
+--  Climb up the tree looking for the enclosing sequence of
+--  statements.
+
+Par := N;
+while Present (Par)
+  and then Nkind (Par) /= N_Handled_Sequence_Of_Statements
+loop
+   Par := Parent (Par);
+end loop;
+
+return Statements (Par);
+ end Find_Insertion_List;
+
+ --  Local variables
+
+ Requires_Hooking : constant Boolean :=
+  Nkind_In (N, N_Function_Call,
+   N_Procedure_Call_Statement);
+
+ Built : Boolean := False;
+ Desig_Typ : Entity_Id;
+ Fin_Block : Node_Id;
+ Fin_Data  : Finalization_Exception_Data;
+ Fin_Decls : List_Id;
+ Last_Fin  : Node_Id := Empty;
+ Loc   : Source_Ptr;
+ Obj_Id: Entity_Id;
+ Obj_Ref   : Node_Id;
+ Obj_Typ   : Entity_Id;
+ Stmt  : Node_Id;
+ Stmts : List_Id;
+ Temp_Id   : Entity_Id;
+
+  --  Start of processing for Process_Transient_Objects
+
   begin
  --  Examine all objects in the list First_Object .. Last_Object
 
@@ -4224,34 +4258,151 @@
 
   and then Stmt /= Related_Node
 then
-   Loc := Sloc (Stmt);
-   Obj_Id  := Defining_Identifier (Stmt);
-   Obj_Typ := Base_Type (Etype (Obj_Id));
-   Desig   := Obj_Typ;
+   Loc   := Sloc (Stmt);
+   Obj_Id:= Defining_Identifier (Stmt);
+   Obj_Typ   := Base_Type (Etype (Obj_Id));
+   Desig_Typ := Obj_Typ;
 
Set_Is_Processed_Transient (Obj_Id);
 
--  Handle ac

[Ada] Check that actual for aliased parameter is aliased

[Arnaud Charlet]

This next step in implementing Ada 2012 AI 142 adds the check that
the actual for an aliased parameter is itself aliased, as shown by
the following example:

 1. pragma Ada_2012;
 2. with Text_IO; use Text_IO;
 3. procedure AliasP3 is
 4.G : Integer;
 5.
 6.procedure K  (R : aliased in out Integer) is
 7.begin
 8.   G := 2;
 9.   R := 5;
10.   Put_Line (G'Img);
11.end;
12. begin
13.K (G);
  |
>>> actual for aliased formal "R" must be aliased object

14. end;

The previously filed test which is supposed to execute is modified
to be:

 1. pragma Ada_2012;
 2. with Text_IO; use Text_IO;
 3. procedure AliasP1 is
 4.G : aliased Integer;
 5.
 6.procedure K  (R : aliased in out Integer) is
 7.begin
 8.   G := 2;
 9.   R := 5;
10.   Put_Line (G'Img);
11.end;
12. begin
13.K (G);
14. end;

which compiles fine and generates the output 5.

An additional test is filed showing aliased parameters working OK:

 1. pragma Ada_2012;
 2. with Text_IO; use Text_IO;
 3. procedure AliasP2 is
 4.G : aliased Integer;
 5.
 6.procedure K  (R : aliased in out Integer) is
 7.   type A is access all Integer;
 8.   AA : A;
 9.begin
10.   G := 2;
11.   AA := R'access;
12.   AA.all := 6;
13.   Put_Line (G'Img);
14.end;
15. begin
16.K (G);
17. end;

which compiles fine and generates the output 6

Tested on x86_64-pc-linux-gnu, committed on trunk

2011-09-01  Robert Dewar  

* exp_ch6.adb (Expand_Call): Check actual for aliased parameter is
aliased.

Index: exp_ch6.adb
===
--- exp_ch6.adb (revision 178381)
+++ exp_ch6.adb (working copy)
@@ -2208,8 +2208,8 @@
   --  as we go through the loop, since this is a convenient place to do it.
   --  (Though it seems that this would be better done in Expand_Actuals???)
 
-  Formal  := First_Formal (Subp);
-  Actual  := First_Actual (Call_Node);
+  Formal := First_Formal (Subp);
+  Actual := First_Actual (Call_Node);
   Param_Count := 1;
   while Present (Formal) loop
 
@@ -2235,7 +2235,7 @@
CW_Interface_Formals_Present
  or else
(Ekind (Etype (Formal)) = E_Class_Wide_Type
-  and then Is_Interface (Etype (Etype (Formal
+ and then Is_Interface (Etype (Etype (Formal
  or else
(Ekind (Etype (Formal)) = E_Anonymous_Access_Type
  and then Is_Interface (Directly_Designated_Type
@@ -2616,6 +2616,15 @@
 end if;
  end if;
 
+ --  For Ada 2012, if a parameter is aliased, the actual must be an
+ --  aliased object.
+
+ if Is_Aliased (Formal) and then not Is_Aliased_View (Actual) then
+Error_Msg_NE
+  ("actual for aliased formal& must be aliased object",
+   Actual, Formal);
+ end if;
+
  --  For IN OUT and OUT parameters, ensure that subscripts are valid
  --  since this is a left side reference. We only do this for calls
  --  from the source program since we assume that compiler generated
@@ -2667,9 +2676,7 @@
--  or IN OUT parameter! We do reset the Is_Known_Valid flag
--  since the subprogram could have returned in invalid value.
 
-   if (Ekind (Formal) = E_Out_Parameter
- or else
-   Ekind (Formal) = E_In_Out_Parameter)
+   if Ekind_In (Formal, E_Out_Parameter, E_In_Out_Parameter)
  and then Is_Assignable (Ent)
then
   Sav := Last_Assignment (Ent);

[Ada] gnatmake and non ASCII unit names

[Arnaud Charlet]

When gnatmake is invoked on a project file where there are unit names
with non ASCII characters that have a specific naming scheme, the
the compiler is unable to find the correct source file names.
This patch corrects this.
The test is to run gnatmake on a project such as:

project Prj is
   for Main use ("main.adb");
   package Naming is
 for Spec ("C250002_["C1"]") use "c250002_aa.ads";
 for Body ("C250002_["C1"]") use "c250002_aa.adb";
   end Naming;
end Prj;

Tested on x86_64-pc-linux-gnu, committed on trunk

2011-09-01  Vincent Celier  

* prj-env.adb (Create_Config_Pragmas_File.Check): Put all naming
exceptions in the config pragmas file.

Index: prj-env.adb
===
--- prj-env.adb (revision 178381)
+++ prj-env.adb (working copy)
@@ -526,9 +526,10 @@
  while Element (Iter) /= No_Source loop
 Source := Element (Iter);
 
-if Source.Index >= 1
-  and then not Source.Locally_Removed
+if not Source.Locally_Removed
   and then Source.Unit /= null
+  and then
+(Source.Index >= 1 or else Source.Naming_Exception)
 then
Put (Source);
 end if;

Re: [PATCH] Make vectorizer dumps more comparable

[Ira Rosen]

Richard Guenther  wrote on 01/09/2011 12:26:25 PM:

> > > Well, it seems to be different what everybody else does and it's
> > > highly redundant for a whole bunch of lines.
> > >
> > > But, it solves my diff issue and the overly long lines as well.
> > >
> > > Your patch changes both dump-file and stderr printing though,
> > > I did want to preserve stderr printing.
> >
> > OK.
> >
> > >
> > > For the dump-file I'd drop the 'line ' prefix and just print '%d: '.
> >
> > OK.
> >
> > >
> > > Btw, the diagnostic machinery does _not_ print locations
> > > for note (""), the location information is supposed to be printed
> > > in the heading warning/error.  Thus, a much better format for stderr
> > > would be
> > >
> > > file.c:12: LOOP NOT VECTORIZED
> > > note: unsupported stmt ''
> > >
> > > as the further notes will be printed with the 'loop location' which
> > > is confusing when dumping statements
> >
> > We usually print only one line, like
> >
> > file.c:12: note:  
> >
> > so I don't really understand this part.
>
> It's a general note.  With -ftree-vectorizer-verbose=5 we dump a lot
> of information with the same location (that of the loop header),
> but the individual messages refer not only to the overall loop
> but to specific statements, etc.
>
> This all is of course an artifact of sharing the dump file code
> with the reporting code.

I see.

Here is the new patch, I'll commit it after testing (on
powerpc64-suse-linux) if there are no objections.

Ira


ChangeLog:

* tree-vectorizer.c (vect_print_dump_info): Print line
number when dumping to a file.
(vectorize_loops): Add new messages to dump file.


Index: tree-vectorizer.c
===
--- tree-vectorizer.c   (revision 178396)
+++ tree-vectorizer.c   (working copy)
@@ -149,16 +149,12 @@ vect_print_dump_info (enum vect_verbosity_levels v
   if (!current_function_decl || !vect_dump)
 return false;

-  if (dump_file)
-fprintf (vect_dump, "\n");
-
-  else if (vect_location == UNKNOWN_LOC)
+  if (vect_location == UNKNOWN_LOC)
 fprintf (vect_dump, "\n%s:%d: note: ",
 DECL_SOURCE_FILE (current_function_decl),
 DECL_SOURCE_LINE (current_function_decl));
   else
-fprintf (vect_dump, "\n%s:%d: note: ",
-LOC_FILE (vect_location), LOC_LINE (vect_location));
+fprintf (vect_dump, "\n%d: ", LOC_LINE (vect_location));

   return true;
 }
@@ -199,12 +195,22 @@ vectorize_loops (void)
loop_vec_info loop_vinfo;

vect_location = find_loop_location (loop);
+if (vect_location != UNKNOWN_LOC
+&& vect_verbosity_level > REPORT_NONE)
+ fprintf (vect_dump, "\nAnalyzing loop at %s:%d\n",
+LOC_FILE (vect_location), LOC_LINE (vect_location));
+
loop_vinfo = vect_analyze_loop (loop);
loop->aux = loop_vinfo;

if (!loop_vinfo || !LOOP_VINFO_VECTORIZABLE_P (loop_vinfo))
  continue;

+if (vect_location != UNKNOWN_LOC
+&& vect_verbosity_level > REPORT_NONE)
+  fprintf (vect_dump, "\n\nVectorizing loop at %s:%d\n",
+LOC_FILE (vect_location), LOC_LINE (vect_location));
+
vect_transform_loop (loop_vinfo);
num_vectorized_loops++;
   }

[Ada] Handle non-default C++ constructor call of non-tagged type

[Arnaud Charlet]

This patch enables the frontend support for calling C++ constructors
of non-tagged types. This is required since the equivalent Ada type
of a C++ class that has no virtual methods is a non-tagged limited
record type. After this patch the following test executes well.

class demo
{
  int value;
public:
  demo (int init_value);
  void display (void);
};

#include "stdio.h"
#include "demo_class.h"

demo::demo (int init_value)
{
  this->value = init_value;
}

void demo::display ()
{
  printf ("value %d\n", value);
}


with Demo_Class_H;
procedure Call_Cpp is
   Obj2 : aliased Demo_Class_H.Class_Demo.demo :=
Demo_Class_H.Class_Demo.New_Demo (42);
begin
   Demo_Class_H.Class_Demo.Display (Obj2'Access);
end Call_Cpp;

Command:
  g++ -c -fdump-ada-spec -C demo_class.h
  gnatchop -w -gnat05 all.ada
  gprbuild -q demo.gpr
  ./call_cpp

Output:
  value 42

Tested on x86_64-pc-linux-gnu, committed on trunk

2011-09-01  Javier Miranda  

* exp_ch3.adb (Expand_N_Object_Declaration): Handle non-default
constructor calls associated with non-tagged record types.

Index: exp_ch3.adb
===
--- exp_ch3.adb (revision 178381)
+++ exp_ch3.adb (working copy)
@@ -5129,9 +5129,13 @@
  Loc;
end;
 
-elsif Is_Tagged_Type (Typ)
-  and then Is_CPP_Constructor_Call (Expr)
-then
+--  Handle C++ constructor calls. Note that we do not check that
+--  Typ is a tagged type since the equivalent Ada type of a C++
+--  class that has no virtual methods is a non-tagged limited
+--  record type.
+
+elsif Is_CPP_Constructor_Call (Expr) then
+
--  The call to the initialization procedure does NOT freeze the
--  object being initialized.
 

[Ada] Add mode for ignoring failures to read ALI files for back-ends

[Arnaud Charlet]

The verification back-end may need to work in a mode in which not all ALI files
are available. Update Read_Withed_ALIs to work in that mode. This does not
impact the compiler.

Tested on x86_64-pc-linux-gnu, committed on trunk

2011-09-01  Yannick Moy  

* ali-util.adb, ali-util.ads (Read_Withed_ALIs): Add parameter
Ignore_Errors to ignore failures to read ALI files when True.

Index: ali-util.adb
===
--- ali-util.adb(revision 178381)
+++ ali-util.adb(working copy)
@@ -6,7 +6,7 @@
 --  --
 -- B o d y  --
 --  --
---  Copyright (C) 1992-2010, Free Software Foundation, Inc. --
+--  Copyright (C) 1992-2011, Free Software Foundation, Inc. --
 --  --
 -- GNAT is free software;  you can  redistribute it  and/or modify it under --
 -- terms of the  GNU General Public License as published  by the Free Soft- --
@@ -212,7 +212,10 @@
-- Read_Withed_ALIs --
--
 
-   procedure Read_Withed_ALIs (Id : ALI_Id) is
+   procedure Read_Withed_ALIs
+ (Id: ALI_Id;
+  Ignore_Errors : Boolean := False)
+   is
   Afile  : File_Name_Type;
   Text   : Text_Buffer_Ptr;
   Idread : ALI_Id;
@@ -234,59 +237,67 @@
 then
Text := Read_Library_Info (Afile);
 
-   --  Return with an error if source cannot be found. We used to
-   --  skip this check when we did not compile library generics
-   --  separately, but we now always do, so there is no special
-   --  case here anymore.
+   --  Unless Ignore_Errors is true, return with an error if source
+   --  cannot be found. We used to skip this check when we did not
+   --  compile library generics separately, but we now always do,
+   --  so there is no special case here anymore.
 
if Text = null then
-  Error_Msg_File_1 := Afile;
-  Error_Msg_File_2 := Withs.Table (W).Sfile;
-  Error_Msg ("{ not found, { must be compiled");
-  Set_Name_Table_Info (Afile, Int (No_Unit_Id));
-  return;
-   end if;
 
-   --  Enter in ALIs table
+  if not Ignore_Errors then
+ Error_Msg_File_1 := Afile;
+ Error_Msg_File_2 := Withs.Table (W).Sfile;
+ Error_Msg ("{ not found, { must be compiled");
+ Set_Name_Table_Info (Afile, Int (No_Unit_Id));
+ return;
+  end if;
 
-   Idread :=
- Scan_ALI
-   (F => Afile,
-T => Text,
-Ignore_ED => False,
-Err   => False);
+   else
+  --  Enter in ALIs table
 
-   Free (Text);
+  Idread :=
+Scan_ALI
+  (F => Afile,
+   T => Text,
+   Ignore_ED => False,
+   Err   => False);
 
-   if ALIs.Table (Idread).Compile_Errors then
-  Error_Msg_File_1 := Withs.Table (W).Sfile;
-  Error_Msg ("{ had errors, must be fixed, and recompiled");
-  Set_Name_Table_Info (Afile, Int (No_Unit_Id));
+  Free (Text);
 
-   elsif ALIs.Table (Idread).No_Object then
-  Error_Msg_File_1 := Withs.Table (W).Sfile;
-  Error_Msg ("{ must be recompiled");
-  Set_Name_Table_Info (Afile, Int (No_Unit_Id));
-   end if;
+  if ALIs.Table (Idread).Compile_Errors
+and then not Ignore_Errors
+  then
+ Error_Msg_File_1 := Withs.Table (W).Sfile;
+ Error_Msg ("{ had errors, must be fixed, and recompiled");
+ Set_Name_Table_Info (Afile, Int (No_Unit_Id));
 
-   --  If the Unit is an Interface to a Stand-Alone Library,
-   --  set the Interface flag in the Withs table, so that its
-   --  dependant are not considered for elaboration order.
+  elsif ALIs.Table (Idread).No_Object
+and then not Ignore_Errors
+  then
+ Error_Msg_File_1 := Withs.Table (W).Sfile;
+ Error_Msg ("{ must be recompiled");
+ Set_Name_Table_Info (Afile, Int (No_Unit_Id));
+  end if;
 
-   if ALIs.Table (I

[Ada] ALI lines for with'ed units should be complete in Alfa mode

[Arnaud Charlet]

In Alfa mode, complete information is required so that back-end can retrieve
Alfa information from suitable ALI files.

Tested on x86_64-pc-linux-gnu, committed on trunk

2011-09-01  Yannick Moy  

* lib-writ.adb (Write_With_Lines): Always output complete information
on "with" line in Alfa mode, as this is required by formal verification
back-end.

Index: lib-writ.adb
===
--- lib-writ.adb(revision 178381)
+++ lib-writ.adb(working copy)
@@ -796,6 +796,12 @@
   or else
  Nkind (Unit (Cunit)) in N_Generic_Renaming_Declaration)
 and then Generic_May_Lack_ALI (Fname))
+
+  --  In Alfa mode, always generate the dependencies on ALI
+  --  files, which are required to compute frame conditions
+  --  of subprograms.
+
+  or else Alfa_Mode
 then
Write_Info_Tab (25);
 

Re: [Ada] Implementation of aspects within generic units

[Tom de Vries]

On 09/01/2011 12:33 PM, Richard Guenther wrote:
> On Thu, Sep 1, 2011 at 11:34 AM, Arnaud Charlet  wrote:
>> After doing a binary search, the first revision which breaks bootstrap on
>> my environment with Ada enabled is the following:
>>
>> r178353 | vries | 2011-08-31 09:04:25 +0200 (Wed, 31 Aug 2011) | 8 lines
>>
>> 2011-08-31  Tom de Vries  
>>
>>PR middle-end/43513
>>* Makefile.in (tree-ssa-ccp.o): Add $(PARAMS_H) to rule.
>>* tree-ssa-ccp.c (params.h): Include.
>>(fold_builtin_alloca_for_var): New function.
>>(ccp_fold_stmt): Use fold_builtin_alloca_for_var.
>>
>> Which makes sense, since Ada uses alloca a lot, much more than other 
>> languages.
>>
>> In other words, none of the changes in the Ada repository is reponsible for
>> this regression.
>>
>> So Tom and/or Richard, could you please have a look at this regression? TIA.
> 
> Probably worth checking
> 
> Index: gcc/tree-ssa-ccp.c
> ===
> --- gcc/tree-ssa-ccp.c  (revision 178394)
> +++ gcc/tree-ssa-ccp.c  (working copy)
> @@ -1714,7 +1714,7 @@ fold_builtin_alloca_for_var (gimple stmt
>block = gimple_block (stmt);
>if (!(cfun->after_inlining
>  && TREE_CODE (BLOCK_SUPERCONTEXT (block)) == FUNCTION_DECL))
> -threshold /= 10;
> +return NULL_TREE;
>if (size > threshold)
>  return NULL_TREE;
> 
> should be reproducable with low stack ulimit.
> 
> Richard.
> 
>> Arno
>>

I'm doing an ada bootstrap now, trying to reproduce this. My apologies for this
breakage.

Thanks,
- Tom

[Ada] References to the formals of a subprogram stub

[Arnaud Charlet]

If a subprogram stub has not previous spec, it defines the formals that will
be used in the proper body, and these formals must have the appropriate cross-
references, for source navigation purposes.

The command

   gcc -c a.adb
   grep Bar a.ali

must yield:

   3U14 Bar 3>19 3>33 3|2b11 5l5 5t8

---
package A is
   procedure Foo;
end A;
---
package body A is
   type T is new Integer;
   procedure Bar (P1 : Integer; P2 : T) is separate;
   procedure Foo is
   begin
  null;
   end Foo;
end A;
---
separate (A)
procedure Bar (P1 : Integer; P2 : T) is
begin
   null;
end Bar;

Tested on x86_64-pc-linux-gnu, committed on trunk

2011-09-01  Ed Schonberg  

* sem_ch6.adb (Analyze_Subprogram_Body_Helper): Generate references to
the formals of a subprogram stub that acts as a spec.

Index: sem_ch6.adb
===
--- sem_ch6.adb (revision 178401)
+++ sem_ch6.adb (working copy)
@@ -2565,10 +2565,14 @@
 Set_Contract (Body_Id, Make_Contract (Sloc (Body_Id)));
 Generate_Reference
   (Body_Id, Body_Id, 'b', Set_Ref => False, Force => True);
-Generate_Reference_To_Formals (Body_Id);
 Install_Formals (Body_Id);
 Push_Scope (Body_Id);
  end if;
+
+ --  For stubs and bodies with no previous spec, generate references to
+ --  formals.
+
+ Generate_Reference_To_Formals (Body_Id);
   end if;
 
   --  If the return type is an anonymous access type whose designated type
@@ -2600,7 +2604,7 @@
 
   --  If this is the proper body of a stub, we must verify that the stub
   --  conforms to the body, and to the previous spec if one was present.
-  --  we know already that the body conforms to that spec. This test is
+  --  We know already that the body conforms to that spec. This test is
   --  only required for subprograms that come from source.
 
   if Nkind (Parent (N)) = N_Subunit
@@ -2626,8 +2630,8 @@
 
if not Conformant then
 
-  --  The stub was taken to be a new declaration. Indicate
-  --  that it lacks a body.
+  --  The stub was taken to be a new declaration. Indicate that
+  --  it lacks a body.
 
   Set_Has_Completion (Old_Id, False);
end if;
@@ -2651,7 +2655,7 @@
   end if;
 
   --  Ada 2005 (AI-262): In library subprogram bodies, after the analysis
-  --  if its specification we have to install the private withed units.
+  --  of the specification we have to install the private withed units.
   --  This holds for child units as well.
 
   if Is_Compilation_Unit (Body_Id)
@@ -2763,8 +2767,8 @@
 
  if Present (Last_Real_Spec_Entity) then
 
---  No body entities (happens when the only real spec entities
---  come from precondition and postcondition pragmas)
+--  No body entities (happens when the only real spec entities come
+--  from precondition and postcondition pragmas).
 
 if No (Last_Entity (Body_Id)) then
Set_First_Entity
@@ -2781,8 +2785,8 @@
 Set_Last_Entity (Body_Id, Last_Entity (Spec_Id));
 Set_Last_Entity (Spec_Id, Last_Real_Spec_Entity);
 
- --  Case where there are no spec entities, in this case there can
- --  be no body entities either, so just move everything.
+ --  Case where there are no spec entities, in this case there can be
+ --  no body entities either, so just move everything.
 
  else
 pragma Assert (No (Last_Entity (Body_Id)));
@@ -2804,7 +2808,7 @@
   --  might be the following common idiom for a stubbed function:
   --  statement of the procedure raises an exception. In particular this
   --  deals with the common idiom of a stubbed function, which might
-  --  appear as something like
+  --  appear as something like:
 
   -- function F (A : Integer) return Some_Type;
   --X : Some_Type;

[Ada] Inlining calls with by-reference parameters

[Arnaud Charlet]

If a subprogram to be inlined has a by-reference parameter, the value of the
actual must be captured in a renaming declaration to avoid an improper copy.
This was done for limited types and tagged types but not for types with by-
reference components.

The following must execute quietly:

with D2; use D2;
with Text_Io; use Text_Io;
procedure Always_Test is
  generic
with procedure Action (By_Ref : By_Ref_T);
  procedure Scan_G;

  procedure Scan_G is
  begin
Action (Data.By_Ref);
  end Scan_G;

  procedure Scan_Inline_Always is new Scan_G (Action_Inline_Always);

  procedure Scan is new Scan_G (Action);
begin

  Data := (12, (34,  (Me_tagged => 46)), 78);

  Scan;
  Scan_Inline_Always;
end Always_Test;
---
package D2 is
  type Tagged_T is tagged record Me_Tagged : Positive; end record;
  type By_Ref_T is
record
  Me : Positive;
  Make_By_Ref : Tagged_T;
end record;

  type Container_T is
record
  Me_Bef : Positive;
  By_Ref : By_Ref_T;
  Me_After : Positive;
end record;

  Data : Container_T;

  procedure Action_Inline_Always (By_Ref : By_Ref_T);
  pragma Inline_Always (Action_Inline_Always);

  procedure Action (By_Ref : By_Ref_T);
end D2;
---
with Text_Io; use Text_Io;
with System; use System;
package body D2 is
  procedure Common_Action (From : String;
   Addr : System.Address) is
  begin
if Addr = Data.By_Ref'Address then
  null;
else
  Put_Line ("FAILED");
end if;
  end Common_Action;

  procedure Action_Inline_Always (By_Ref : By_Ref_T) is
  begin
Common_Action ("Action_Inline_Always", By_Ref'Address);
  end Action_Inline_Always;

  procedure Action (By_Ref : By_Ref_T) is
  begin
Common_Action ("Action", By_Ref'Address);
  end Action;
end D2;

Tested on x86_64-pc-linux-gnu, committed on trunk

2011-09-01  Ed Schonberg  

* exp_ch6.adb (Expand_Inlined_Call): If an actual is a by_reference
type, declare a renaming for it, not an object declaration.

Index: exp_ch6.adb
===
--- exp_ch6.adb (revision 178403)
+++ exp_ch6.adb (working copy)
@@ -4188,6 +4188,7 @@
 if Ekind (F) = E_In_Parameter
   and then not Is_Limited_Type (Etype (A))
   and then not Is_Tagged_Type  (Etype (A))
+  and then not Is_By_Reference_Type (Etype (A))
   and then
(not Is_Array_Type (Etype (A))
  or else not Is_Object_Reference (A)

[PATCH][ARM] pr50193: ICE on a | (b << negative-constant)

[Andrew Stubbs]

This patch should fix the bug in pr50193.

The problem is that the arith_shiftsi pattern accepted any arbitrary 
constant as the shift amount (via the shift_amount_operand predicate) 
where in fact the constant must be in the range 0..32.


This patch fixes the problem by merely checking that the constant is 
positive. I've confirmed that values larger than the mode-size are not a 
problem because the compiler optimizes those away earlier, even at -O0.


OK?

Andrew
2011-09-01  Andrew Stubbs  

	gcc/
	* config/arm/predicates.md (shift_amount_operand): Ensure shift
	amount is positive.

	gcc/testsuite/
	* gcc.dg/pr50193-1.c: New file.

--- a/gcc/config/arm/predicates.md
+++ b/gcc/config/arm/predicates.md
@@ -132,7 +132,8 @@
 (define_predicate "shift_amount_operand"
   (ior (and (match_test "TARGET_ARM")
 	(match_operand 0 "s_register_operand"))
-   (match_operand 0 "const_int_operand")))
+   (and (match_operand 0 "const_int_operand")
+	(match_test "INTVAL (op) > 0"
 
 (define_predicate "arm_add_operand"
   (ior (match_operand 0 "arm_rhs_operand")
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr50193-1.c
@@ -0,0 +1,10 @@
+/* PR 50193: ARM: ICE on a | (b << negative-constant) */
+/* Ensure that the compiler doesn't ICE.  */
+
+/* { dg-options "-O2" } */
+
+int
+foo(int a, int b)
+{
+  return a | (b << -3); /* { dg-warning "left shift count is negative" } */
+}

Re: [PATCH] Make VEC_COND_EXPR a GIMPLE_TERNARY_RHS

[Richard Guenther]

On Wed, 31 Aug 2011, Richard Guenther wrote:

> 
> $subject says it all, COND_EXPR to follow.  rhs1 will still be
> a "single" tree, namely a is_gimple_condexpr () operand.  That's
> not easy to change (without forcing it to a separate statement,
> thus making it a is_gimple_val) because it embeds another tree code.
> 
> Bootstrapped and tested on x86_64-unknown-linux-gnu.
> 
> Any serious objections?

None, though the following implements it for both COND_EXPR and
VEC_COND_EXPR.

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

Richard.

2011-08-31  Richard Guenther  

* expr.c (expand_expr_real_2): Move COND_EXPR and VEC_COND_EXPR
handling here, from ...
(expand_expr_real_1): ... here.
* gimple-pretty-print.c (dump_ternary_rhs): Handle COND_EXPR
and VEC_COND_EXPR.
* gimple.c (gimple_rhs_class_table): Make COND_EXPR and VEC_COND_EXPR
a GIMPLE_TERNARY_RHS.
* tree-cfg.c (verify_gimple_assign_ternary): Handle COND_EXPR
and VEC_COND_EXPR here ...
(verify_gimple_assign_single): ... not here.
* gimple-fold.c (fold_gimple_assign): Move COND_EXPR folding.
* tree-object-size.c (cond_expr_object_size): Adjust.
(collect_object_sizes_for): Likewise.
* tree-scalar-evolution.c (interpret_expr): Don't handle
ternary RHSs.
* tree-ssa-forwprop.c (forward_propagate_into_cond): Fix and
simplify.
(ssa_forward_propagate_and_combine): Adjust.
* tree-ssa-loop-im.c (move_computations_stmt): Build the COND_EXPR
as ternary.
* tree-ssa-threadedge.c (fold_assignment_stmt): Adjust.
* tree-vect-loop.c (vect_is_simple_reduction_1): Likewise.
* tree-vect-stmt.c (vectorizable_condition): Likewise.
* tree-vrp.c (extract_range_from_cond_expr): Likewise.
(extract_range_from_assignment): Likewise.

Index: trunk/gcc/expr.c
===
*** trunk.orig/gcc/expr.c   2011-09-01 12:08:51.0 +0200
--- trunk/gcc/expr.c2011-09-01 12:10:04.0 +0200
*** expand_expr_real_2 (sepops ops, rtx targ
*** 8636,8641 
--- 8636,8699 
  return temp;
}
  
+ case COND_EXPR:
+   /* A COND_EXPR with its type being VOID_TYPE represents a
+conditional jump and is handled in
+expand_gimple_cond_expr.  */
+   gcc_assert (!VOID_TYPE_P (type));
+ 
+   /* Note that COND_EXPRs whose type is a structure or union
+are required to be constructed to contain assignments of
+a temporary variable, so that we can evaluate them here
+for side effect only.  If type is void, we must do likewise.  */
+ 
+   gcc_assert (!TREE_ADDRESSABLE (type)
+ && !ignore
+ && TREE_TYPE (treeop1) != void_type_node
+ && TREE_TYPE (treeop2) != void_type_node);
+ 
+   /* If we are not to produce a result, we have no target.  Otherwise,
+if a target was specified use it; it will not be used as an
+intermediate target unless it is safe.  If no target, use a
+temporary.  */
+ 
+   if (modifier != EXPAND_STACK_PARM
+ && original_target
+ && safe_from_p (original_target, treeop0, 1)
+ && GET_MODE (original_target) == mode
+ #ifdef HAVE_conditional_move
+ && (! can_conditionally_move_p (mode)
+ || REG_P (original_target))
+ #endif
+ && !MEM_P (original_target))
+   temp = original_target;
+   else
+   temp = assign_temp (type, 0, 0, 1);
+ 
+   do_pending_stack_adjust ();
+   NO_DEFER_POP;
+   op0 = gen_label_rtx ();
+   op1 = gen_label_rtx ();
+   jumpifnot (treeop0, op0, -1);
+   store_expr (treeop1, temp,
+ modifier == EXPAND_STACK_PARM,
+ false);
+ 
+   emit_jump_insn (gen_jump (op1));
+   emit_barrier ();
+   emit_label (op0);
+   store_expr (treeop2, temp,
+ modifier == EXPAND_STACK_PARM,
+ false);
+ 
+   emit_label (op1);
+   OK_DEFER_POP;
+   return temp;
+ 
+ case VEC_COND_EXPR:
+   target = expand_vec_cond_expr (type, treeop0, treeop1, treeop2, target);
+   return target;
+ 
  default:
gcc_unreachable ();
  }
*** expand_expr_real_1 (tree exp, rtx target
*** 9878,9941 
  
return op0;
  
- case COND_EXPR:
-   /* A COND_EXPR with its type being VOID_TYPE represents a
-conditional jump and is handled in
-expand_gimple_cond_expr.  */
-   gcc_assert (!VOID_TYPE_P (type));
- 
- /* Note that COND_EXPRs whose type is a structure or union
-are required to be constructed to contain assignments of
-a temporary variable, so that we can evaluate them here
-for side effect only.  If type is void, we must do likewise.  */
- 
- gcc_assert (!TREE_ADDRESSABLE (

ARM: Emit conditions in push_multi

[Bernd Schmidt]

Shrink-wrapping tests on ARM had one additional failure, which I could
track down to a stmfd instruction being emitted where an stmhifd was
intended. The following patch fixes the testcase; full tests running
now. Ok?


Bernd
* config/arm/arm.md (push_multi): Emit predicates.

Index: gcc/config/arm/arm.md
===
--- gcc/config/arm/arm.md   (revision 178135)
+++ gcc/config/arm/arm.md   (working copy)
@@ -10581,14 +10581,14 @@ (define_insn "*push_multi"
In Thumb mode always use push, and the assembler will pick
something appropriate.  */
 if (num_saves == 1 && TARGET_ARM)
-  output_asm_insn (\"str\\t%1, [%m0, #-4]!\", operands);
+  output_asm_insn (\"str%?\\t%1, [%m0, #-4]!\", operands);
 else
   {
int i;
char pattern[100];
 
if (TARGET_ARM)
-   strcpy (pattern, \"stmfd\\t%m0!, {%1\");
+   strcpy (pattern, \"stm%(fd%)\\t%m0!, {%1\");
else
strcpy (pattern, \"push\\t{%1\");
 

Fix PR50260

[Michael Matz]

Hi,

the last change in ipa-split generated a new use of a previously unused 
PARM_DECL.  When one does this one has to call add_referenced_var.  Not 
doing so can cause segfault when accessing the (not initialized) var 
annotation.  So, fixed with the patch.

I took the opportunity to remove all explicit calls to get_var_ann because 
add_referenced_var is doing so as first thing.  Reviewing the code showed 
one potentially problematic case in tree-ssa-pre.c where a new variable 
was created without calling add_referenced_var.  It's only for 
place-holder SSA names but those can conceivably leak into the program 
stream by being reused during expression generation.

Currently regstrapping on x86_64-linux (without Ada).  Okay for trunk?


Ciao,
Michael.
-- 
PR middle-end/50260
* ipa-split.c (split_function): Call add_referenced_var.

* tree-ssa-phiopt.c (cond_store_replacement): Don't call get_var_ann.
(cond_if_else_store_replacement_1): Ditto.
* tree-ssa-pre.c (get_representative_for): Ditto.
(create_expression_by_pieces): Ditto.
(insert_into_preds_of_block): Ditto.
* tree-sra.c (create_access_replacement): Ditto.
(get_replaced_param_substitute): Ditto.

testsuite/
* gfortran.fortran-torture/compile/pr50260.f90: New test.

Index: ipa-split.c
===
--- ipa-split.c (revision 178408)
+++ ipa-split.c (working copy)
@@ -988,6 +988,9 @@ split_function (struct split_point *spli
arg = gimple_default_def (cfun, parm);
if (!arg)
  {
+   /* This parm wasn't used up to now, but is going to be used,
+  hence register it.  */
+   add_referenced_var (parm);
arg = make_ssa_name (parm, gimple_build_nop ());
set_default_def (parm, arg);
  }
Index: tree-ssa-phiopt.c
===
--- tree-ssa-phiopt.c   (revision 178408)
+++ tree-ssa-phiopt.c   (working copy)
@@ -1269,10 +1269,7 @@ cond_store_replacement (basic_block midd
   /* 2) Create a temporary where we can store the old content
 of the memory touched by the store, if we need to.  */
   if (!condstoretemp || TREE_TYPE (lhs) != TREE_TYPE (condstoretemp))
-{
-  condstoretemp = create_tmp_reg (TREE_TYPE (lhs), "cstore");
-  get_var_ann (condstoretemp);
-}
+condstoretemp = create_tmp_reg (TREE_TYPE (lhs), "cstore");
   add_referenced_var (condstoretemp);
 
   /* 3) Insert a load from the memory of the store to the temporary
@@ -1355,10 +1352,7 @@ cond_if_else_store_replacement_1 (basic_
   /* 2) Create a temporary where we can store the old content
of the memory touched by the store, if we need to.  */
   if (!condstoretemp || TREE_TYPE (lhs) != TREE_TYPE (condstoretemp))
-{
-  condstoretemp = create_tmp_reg (TREE_TYPE (lhs), "cstore");
-  get_var_ann (condstoretemp);
-}
+condstoretemp = create_tmp_reg (TREE_TYPE (lhs), "cstore");
   add_referenced_var (condstoretemp);
 
   /* 3) Create a PHI node at the join block, with one argument
Index: tree-ssa-pre.c
===
--- tree-ssa-pre.c  (revision 178408)
+++ tree-ssa-pre.c  (working copy)
@@ -1399,7 +1399,7 @@ get_representative_for (const pre_expr e
   if (!pretemp || exprtype != TREE_TYPE (pretemp))
 {
   pretemp = create_tmp_reg (exprtype, "pretmp");
-  get_var_ann (pretemp);
+  add_referenced_var (pretemp);
 }
 
   name = make_ssa_name (pretemp, gimple_build_nop ());
@@ -3178,10 +3178,7 @@ create_expression_by_pieces (basic_block
   /* Build and insert the assignment of the end result to the temporary
  that we will return.  */
   if (!pretemp || exprtype != TREE_TYPE (pretemp))
-{
-  pretemp = create_tmp_reg (exprtype, "pretmp");
-  get_var_ann (pretemp);
-}
+pretemp = create_tmp_reg (exprtype, "pretmp");
 
   temp = pretemp;
   add_referenced_var (temp);
@@ -3441,10 +3438,7 @@ insert_into_preds_of_block (basic_block
 
   /* Now build a phi for the new variable.  */
   if (!prephitemp || TREE_TYPE (prephitemp) != type)
-{
-  prephitemp = create_tmp_var (type, "prephitmp");
-  get_var_ann (prephitemp);
-}
+prephitemp = create_tmp_var (type, "prephitmp");
 
   temp = prephitemp;
   add_referenced_var (temp);
Index: tree-sra.c
===
--- tree-sra.c  (revision 178408)
+++ tree-sra.c  (working copy)
@@ -1825,7 +1825,6 @@ create_access_replacement (struct access
   tree repl;
 
   repl = create_tmp_var (access->type, "SR");
-  get_var_ann (repl);
   add_referenced_var (repl);
   if (rename)
 mark_sym_for_renaming (repl);
@@ -4106,7 +4105,6 @@ get_replaced_param_substitute (struct ip
   DECL_NAME (repl) = get_identifier (pretty_name);
   obstack_free (&name_obstack, pretty_name);
 
-  get_var_an

Re: Fix PR50260

[Martin Jambor]

Hi,

On Thu, Sep 01, 2011 at 02:41:15PM +0200, Michael Matz wrote:
> Hi,
> 
> the last change in ipa-split generated a new use of a previously unused 
> PARM_DECL.  When one does this one has to call add_referenced_var.  Not 
> doing so can cause segfault when accessing the (not initialized) var 
> annotation.  So, fixed with the patch.

This is how I have fixed the issue when I ran into it independently
about an hour ago and it is probably my preferred way of dealing with
this, at least for now (that is until Honza makes up his mind what to
do with the type attributes in fnsplit).

Thanks a lot,

Martin

> 
> I took the opportunity to remove all explicit calls to get_var_ann because 
> add_referenced_var is doing so as first thing.  Reviewing the code showed 
> one potentially problematic case in tree-ssa-pre.c where a new variable 
> was created without calling add_referenced_var.  It's only for 
> place-holder SSA names but those can conceivably leak into the program 
> stream by being reused during expression generation.
> 
> Currently regstrapping on x86_64-linux (without Ada).  Okay for trunk?
> 
> 
> Ciao,
> Michael.
> -- 
>   PR middle-end/50260
>   * ipa-split.c (split_function): Call add_referenced_var.
> 
>   * tree-ssa-phiopt.c (cond_store_replacement): Don't call get_var_ann.
>   (cond_if_else_store_replacement_1): Ditto.
>   * tree-ssa-pre.c (get_representative_for): Ditto.
>   (create_expression_by_pieces): Ditto.
>   (insert_into_preds_of_block): Ditto.
>   * tree-sra.c (create_access_replacement): Ditto.
>   (get_replaced_param_substitute): Ditto.
> 
> testsuite/
>   * gfortran.fortran-torture/compile/pr50260.f90: New test.
> 
> Index: ipa-split.c
> ===
> --- ipa-split.c   (revision 178408)
> +++ ipa-split.c   (working copy)
> @@ -988,6 +988,9 @@ split_function (struct split_point *spli
>   arg = gimple_default_def (cfun, parm);
>   if (!arg)
> {
> + /* This parm wasn't used up to now, but is going to be used,
> +hence register it.  */
> + add_referenced_var (parm);
>   arg = make_ssa_name (parm, gimple_build_nop ());
>   set_default_def (parm, arg);
> }
> Index: tree-ssa-phiopt.c
> ===
> --- tree-ssa-phiopt.c (revision 178408)
> +++ tree-ssa-phiopt.c (working copy)
> @@ -1269,10 +1269,7 @@ cond_store_replacement (basic_block midd
>/* 2) Create a temporary where we can store the old content
>  of the memory touched by the store, if we need to.  */
>if (!condstoretemp || TREE_TYPE (lhs) != TREE_TYPE (condstoretemp))
> -{
> -  condstoretemp = create_tmp_reg (TREE_TYPE (lhs), "cstore");
> -  get_var_ann (condstoretemp);
> -}
> +condstoretemp = create_tmp_reg (TREE_TYPE (lhs), "cstore");
>add_referenced_var (condstoretemp);
>  
>/* 3) Insert a load from the memory of the store to the temporary
> @@ -1355,10 +1352,7 @@ cond_if_else_store_replacement_1 (basic_
>/* 2) Create a temporary where we can store the old content
>   of the memory touched by the store, if we need to.  */
>if (!condstoretemp || TREE_TYPE (lhs) != TREE_TYPE (condstoretemp))
> -{
> -  condstoretemp = create_tmp_reg (TREE_TYPE (lhs), "cstore");
> -  get_var_ann (condstoretemp);
> -}
> +condstoretemp = create_tmp_reg (TREE_TYPE (lhs), "cstore");
>add_referenced_var (condstoretemp);
>  
>/* 3) Create a PHI node at the join block, with one argument
> Index: tree-ssa-pre.c
> ===
> --- tree-ssa-pre.c(revision 178408)
> +++ tree-ssa-pre.c(working copy)
> @@ -1399,7 +1399,7 @@ get_representative_for (const pre_expr e
>if (!pretemp || exprtype != TREE_TYPE (pretemp))
>  {
>pretemp = create_tmp_reg (exprtype, "pretmp");
> -  get_var_ann (pretemp);
> +  add_referenced_var (pretemp);
>  }
>  
>name = make_ssa_name (pretemp, gimple_build_nop ());
> @@ -3178,10 +3178,7 @@ create_expression_by_pieces (basic_block
>/* Build and insert the assignment of the end result to the temporary
>   that we will return.  */
>if (!pretemp || exprtype != TREE_TYPE (pretemp))
> -{
> -  pretemp = create_tmp_reg (exprtype, "pretmp");
> -  get_var_ann (pretemp);
> -}
> +pretemp = create_tmp_reg (exprtype, "pretmp");
>  
>temp = pretemp;
>add_referenced_var (temp);
> @@ -3441,10 +3438,7 @@ insert_into_preds_of_block (basic_block
>  
>/* Now build a phi for the new variable.  */
>if (!prephitemp || TREE_TYPE (prephitemp) != type)
> -{
> -  prephitemp = create_tmp_var (type, "prephitmp");
> -  get_var_ann (prephitemp);
> -}
> +prephitemp = create_tmp_var (type, "prephitmp");
>  
>temp = prephitemp;
>add_referenced_var (temp);
> Index: tree-sra.c
> ===

Re: Rename across basic block boundaries

[Bernd Schmidt]

On 08/26/11 14:57, Richard Sandiford wrote:
>> +  /* There must be some kind of conflict.  Set the unusable for all
>> + overlapping registers.  */
>> +  min_reg = chain->regno;
>> +  if (incoming_nregs < 0)
>> +min_reg += incoming_nregs;
>> +  max_reg = chain->regno + chain->nregs;
>> +  for (i = min_reg; i < max_reg; i++)
>> +ri->incoming[i].unusable = true;
> 
> In the incoming_nregs < 0 case, we only need to set
> ri->incoming[chain->regno + incoming_nregs] itself, right,
> not the other registers between that and ri->incoming[chain->regno]?
> If so, I think it'd be clearer to have:
> 
>   /* There must be some kind of conflict.  Prevent both the old and
>  new ranges from being used.  */
>   if (incoming_nregs < 0)
> ri->incoming[chain->regno + incoming_nregs].unusable = true;
>   for (i = 0; i < chain->nregs; i++)
> ri->incoming[chain->regno + i].unusable = true;
> 
> When I first looked at the code, I was wondering why we changed every
> register in (chain->regno + incoming_nregs, chain_regno), but none in
> [chain->regno + chain->nregs, OLD_END).  Seems like we should do neither
> (as in the above suggestion) or both.

I think both was the original intention (OLD_END being min_reg +
ri->incoming[min_reg].nregs, right?), but yours works too.

>> +  /* Process all basic blocks by using a worklist, adding unvisited 
>> successor
>> + blocks whenever we reach the end of one basic blocks.  This ensures 
>> that
>> + whenever possible, we only process a block after at least one of its
>> + predecessors, which provides a "seeding" effect to make the logic in
>> + set_incoming_from_chain and init_rename_info useful.  */
> 
> Wouldn't a reverse post-order (inverted_post_order_compute) allow even
> more pre-opening (as well as being less code)?

I can't really tell from the comments what that function is supposed to
produce. I've made a change to use it to order the bbs, but that made rr
visit basic blocks without seeing any of their predecessors first, so it
seems unsuitable. So I've left that alone.

New version below.


Bernd
* regrename.c (struct du_head): Make nregs signed.
(closed_chains): Remove.
(create_new_chain): Return the new chain.
(chain_from_id): New static function.
(dump_def_use_chain): Change argument to be an int, indicating
the first ID to print.  All callers changed.
(merge_overlapping_regs): Use chain_from_id.  Assert that
chains don't conflict with themselves.
(rename_chains): Take no argument.  Iterate over id_to_chain
rather to find chains to rename.  Clear tick before the main
loop.
(struct incoming_reg_info): New struct.
(struct bb_rename_info): New struct.
(init_rename_info, set_incoming_from_chain, merge_chains): New
static functions.
(regrename_analyze): New static function, broken out of
regrename_optimize.  Record and make use of open chain information
at basic block boundaries, and merge chains where possible.
(scan_rtx_reg): Make this_nregs signed.  Don't update
closed_chains.
(build_def_use): Return a bool to indicate success.  All callers
changed.  Don't initialize global data here.
(regrename_optimize): Move most code out of here into
regrename_analyze.
* regs.h (add_range_to_hard_reg_set, remove_range_from_hard_reg_set,
range_overlaps_hard_reg_set_p, range_in_hard_reg_set_p): New
static inline functions.
* vec.h (FOR_EACH_VEC_ELT_FROM): New macro.

Index: gcc/regrename.c
===
--- gcc/regrename.c (revision 178293)
+++ gcc/regrename.c (working copy)
@@ -47,18 +47,24 @@
 
  1. Local def/use chains are built: within each basic block, chains are
opened and closed; if a chain isn't closed at the end of the block,
-   it is dropped.
+   it is dropped.  We pre-open chains if we have already examined a
+   predecessor block and found chains live at the end which match
+   live registers at the start of the new block.
 
- 2. For each chain, the set of possible renaming registers is computed.
+ 2. We try to combine the local chains across basic block boundaries by
+comparing chains that were open at the start or end of a block to
+   those in successor/predecessor blocks.
+
+ 3. For each chain, the set of possible renaming registers is computed.
This takes into account the renaming of previously processed chains.
Optionally, a preferred class is computed for the renaming register.
 
- 3. The best renaming register is computed for the chain in the above set,
+ 4. The best renaming register is computed for the chain in the above set,
using a round-robin allocation.  If a preferred class exists, then the
round-robin allocation is done within the class first, if poss

[Ada] Error on call to function with limited result and unknown discriminants

[Arnaud Charlet]

The compiler incorrectly reports an error about "uninitialized unconstrained
allocation" on a call to a limited build-in-place function when the result type
has a partial view with unknown discriminants, the full view is constrained and
the call is the expression of a return for an enclosing build-in-place function.
The build-in-place expansion process treats the return statement and the call
inconsistently with respect to constrained vs. unconstrained properties,
resulting in full build-in-place return statement expansion that causes an
illegal allocator to be created (returning the unconstrained partial view).
The correction is to handle all cases where a return expression in a
build-in-place function is itself a build-in-place call by simply passing
along the implicit build-in-place parameters of the enclosing function,
which is more efficient in any case and will possibliy also fix some latent
bug cases.

The following test must compile and execute quietly with -gnat05:


package Unk_Discrim_BIP_Pkg is

   type Root is new Ada.Finalization.Limited_Controlled with null record;

   type Priv_With_Unk_Discrims (<>) is new Root with private;

   function Specific_BIP_Func return Priv_With_Unk_Discrims;

   function Value (Obj : Priv_With_Unk_Discrims) return Integer;

private

   type Priv_With_Unk_Discrims is new Root with record
  I : Integer := 0;
   end record;

end Unk_Discrim_BIP_Pkg;


package body Unk_Discrim_BIP_Pkg is

   function Specific_BIP_Func return Priv_With_Unk_Discrims is
   begin
  return Result : Priv_With_Unk_Discrims do
 Result.I := 123;
  end return;
   end Specific_BIP_Func;

   function Value (Obj : Priv_With_Unk_Discrims) return Integer is
   begin
  return Obj.I;
   end Value;

end Unk_Discrim_BIP_Pkg;


with Unk_Discrim_BIP_Pkg;  use Unk_Discrim_BIP_Pkg;

procedure Unk_Discrim_BIP_Bug is

   function Class_Wide_BIP_Func return Root'Class is
   begin
  return Specific_BIP_Func;
   end Class_Wide_BIP_Func;

begin
   if Value (Priv_With_Unk_Discrims (Class_Wide_BIP_Func)) /= 123 then
  raise Program_Error;
   end if;
end Unk_Discrim_BIP_Bug;

Tested on x86_64-pc-linux-gnu, committed on trunk

2011-09-01  Gary Dismukes  

* exp_ch6.ads (Needs_BIP_Alloc_Form): New utility function.
* exp_ch6.adb (Make_Build_In_Place_Call_In_Object_Declaration):
Test for case where call
initializes an object of a return statement before testing for
a constrained call, to ensure that all such cases get handled
by simply passing on the caller's parameters. Also, in that
case call Needs_BIP_Alloc_Form to determine whether to pass on
the BIP_Alloc_Form parameter of the enclosing function rather
than testing Is_Constrained. Add similar tests for the return
of a BIP call to later processing to ensure consistent handling.
(Needs_BIP_Alloc_Form): New utility function.
* sem_ch6.adb: (Create_Extra_Formals): Replace test for adding
a BIP_Alloc_Form formal with call to new utility function
Needs_BIP_Alloc_Form.

Index: exp_ch6.adb
===
--- exp_ch6.adb (revision 178410)
+++ exp_ch6.adb (working copy)
@@ -4198,7 +4198,6 @@
Constant_Present=> True,
Object_Definition   => New_Occurrence_Of (Temp_Typ, Loc),
Expression  => New_A);
-
 else
Decl :=
  Make_Object_Renaming_Declaration (Loc,
@@ -7579,61 +7578,40 @@
 
   Result_Subt := Etype (Function_Id);
 
-  --  In the constrained case, add an implicit actual to the function call
-  --  that provides access to the declared object. An unchecked conversion
-  --  to the (specific) result type of the function is inserted to handle
-  --  the case where the object is declared with a class-wide type.
+  --  If the the object is a return object of an enclosing build-in-place
+  --  function, then the implicit build-in-place parameters of the
+  --  enclosing function are simply passed along to the called function.
+  --  (Unfortunately, this won't cover the case of extension aggregates
+  --  where the ancestor part is a build-in-place unconstrained function
+  --  call that should be passed along the caller's parameters. Currently
+  --  those get mishandled by reassigning the result of the call to the
+  --  aggregate return object, when the call result should really be
+  --  directly built in place in the aggregate and not in a temporary. ???)
 
-  if Is_Constrained (Underlying_Type (Result_Subt)) then
- Caller_Object :=
-Make_Unchecked_Type_Conversion (Loc,
-  Subtype_Mark => New_Reference_To (Result_Subt, Loc),
-  Expression   => New_Reference_To (Obj_Def_Id, Loc));
-
- --  When the function has a controlling result, an allocation-form
-  

[Ada] Correction to aggregates in instance bodies

[Arnaud Charlet]

This patch fixes an obscure bug where a record aggregate would be incorrectly
flagged as illegal, when the record is inside an instance of a generic, and the
type is private.
No small test case is available.

Tested on x86_64-pc-linux-gnu, committed on trunk

2011-09-01  Bob Duff  

* sem_aggr.adb (Resolve_Aggregate): Need to treat "in instance
body" the same as "in inlined body", because visibility shouldn't
apply there.

Index: sem_aggr.adb
===
--- sem_aggr.adb(revision 178398)
+++ sem_aggr.adb(working copy)
@@ -1229,7 +1229,7 @@
 
   elsif Is_Private_Type (Typ)
 and then Present (Full_View (Typ))
-and then In_Inlined_Body
+and then (In_Inlined_Body or else In_Instance_Body)
 and then Is_Composite_Type (Full_View (Typ))
   then
  Resolve (N, Full_View (Typ));

Re: [PATCH][ARM] pr50193: ICE on a | (b << negative-constant)

[Joseph S. Myers]

On Thu, 1 Sep 2011, Andrew Stubbs wrote:

> This patch fixes the problem by merely checking that the constant is positive.
> I've confirmed that values larger than the mode-size are not a problem because
> the compiler optimizes those away earlier, even at -O0.

Do you mean that you have observed for some testcases that they get 
optimized away - or do you have reasons (if so, please state them) to 
believe that any possible path through the compiler that would result in a 
larger constant here (possibly as a result of constant propagation and 
other optimizations) will always result in it being optimized away as 
well?  If it's just observation it would be better to put the complete 
check in here.

Quite of few of the Csmith-generated bug reports from John Regehr have 
involved constants appearing in unexpected places as a result of 
transformations in the compiler.  It would probably be a good idea for 
someone to try using Csmith to find ARM compiler bugs (both ICEs and 
wrong-code); pretty much all the bugs reported have been testing on x86 
and x86_64, so it's likely there are quite a few bugs in the ARM back end 
that could be found that way.

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

Fix testsuite profiling support checks

[Bernd Schmidt]

The following change

2011-05-26  Rainer Orth  
PR gcov-profile/48845

causes testsuite failures on C6X. Specifically,

@@ -495,11 +495,16 @@ proc check_profiling_available { test_wh
 # These conditions depend on the argument so examine them before
 # looking at the cache variable.

+# Tree profiling requires TLS runtime support.
+if { $test_what == "-fprofile-generate" } {
+   return [check_effective_target_tls_runtime]
+}
+

skips all the further tests in that function if
check_effective_target_tls_runtime returns true. (I'm not sure why it
does that on C6X - it seems to link in emutls stuff?) We then fail with
unresolved __gcov symbols.

Fixed with the following patch. I've removed the new avr special case,
assuming it was intended to fix the same issue. Ok?


Bernd
* testsuite/lib/target-supports.exp (check_profiling_available):
Don't rely solely on TLS tests for -fprofile-generate, fall
through to the other code.

Index: gcc/testsuite/lib/target-supports.exp
===
--- gcc/testsuite/lib/target-supports.exp   (revision 178293)
+++ gcc/testsuite/lib/target-supports.exp   (working copy)
@@ -459,12 +459,9 @@ proc check_profiling_available { test_wh
 
 # Tree profiling requires TLS runtime support.
 if { $test_what == "-fprofile-generate" } {
-   # AVR does not support profile generation because
-   # it does not implement needed support functions.
-   if { [istarget avr-*-*] } {
+   if { ![check_effective_target_tls_runtime] } {
return 0
}
-   return [check_effective_target_tls_runtime]
 }
 
 # Support for -p on solaris2 relies on mcrt1.o which comes with the

[Ada] Use the CPU_ZERO and CPU_SET linux macros

[Arnaud Charlet]

CPU sets are properly handled in linux using the macros defined for this
effect. This way we avoid problems with the endianness of the target if we
try to modify the bit set by hand.

No functional change.

Tested on x86_64-pc-linux-gnu, committed on trunk

2011-09-01  Jose Ruiz  

* s-taprop-linux.adb (Create_Task, Set_Task_Affinity): Use the linux
macros for handling CPU sets (CPU_ZERO, CPU_SET) instead of modifying
directly the bit array.
* s-osinte-linux.ads (CPU_ZERO, CPU_SET): Import these wrappers around
the linux macros with the same name.
* adaint.h, adaint.c (__gnat_cpu_zero, __gnat_cpu_set): Create these
wrappers around the CPU_ZERO and CPU_SET linux macros.

Index: s-osinte-linux.ads
===
--- s-osinte-linux.ads  (revision 178381)
+++ s-osinte-linux.ads  (working copy)
@@ -7,7 +7,7 @@
 --  S p e c --
 --  --
 -- Copyright (C) 1991-1994, Florida State University--
---  Copyright (C) 1995-2010, Free Software Foundation, Inc. --
+--  Copyright (C) 1995-2011, Free Software Foundation, Inc. --
 --  --
 -- GNAT is free software;  you can  redistribute it  and/or modify it under --
 -- terms of the  GNU General Public License as published  by the Free Soft- --
@@ -482,6 +482,14 @@
end record;
pragma Convention (C, cpu_set_t);
 
+   procedure CPU_ZERO (cpuset : access cpu_set_t);
+   pragma Import (C, CPU_ZERO, "__gnat_cpu_zero");
+   --  Wrapper around the CPU_ZERO C macro
+
+   procedure CPU_SET (cpu : int; cpuset : access cpu_set_t);
+   pragma Import (C, CPU_SET, "__gnat_cpu_set");
+   --  Wrapper around the CPU_SET C macro
+
function pthread_setaffinity_np
  (thread : pthread_t;
   cpusetsize : size_t;
Index: s-taprop-linux.adb
===
--- s-taprop-linux.adb  (revision 178398)
+++ s-taprop-linux.adb  (working copy)
@@ -869,9 +869,12 @@
 
   elsif T.Common.Base_CPU /= System.Multiprocessors.Not_A_Specific_CPU then
  declare
-CPU_Set : aliased cpu_set_t := (bits => (others => False));
+CPU_Set : aliased cpu_set_t;
+
  begin
-CPU_Set.bits (Integer (T.Common.Base_CPU)) := True;
+System.OS_Interface.CPU_ZERO (CPU_Set'Access);
+System.OS_Interface.CPU_SET
+  (int (T.Common.Base_CPU), CPU_Set'Access);
 Result :=
   pthread_attr_setaffinity_np
 (Attributes'Access,
@@ -905,14 +908,18 @@
  Multiprocessors.Number_Of_CPUs => True))
   then
  declare
-CPU_Set : aliased cpu_set_t := (bits => (others => False));
+CPU_Set : aliased cpu_set_t;
 
  begin
+System.OS_Interface.CPU_ZERO (CPU_Set'Access);
+
 --  Set the affinity to all the processors belonging to the
 --  dispatching domain.
 
 for Proc in T.Common.Domain'Range loop
-   CPU_Set.bits (Integer (Proc)) := T.Common.Domain (Proc);
+   if T.Common.Domain (Proc) then
+  System.OS_Interface.CPU_SET (int (Proc), CPU_Set'Access);
+   end if;
 end loop;
 
 Result :=
@@ -1394,8 +1401,9 @@
   then
  declare
 type cpu_set_t_ptr is access all cpu_set_t;
+CPU_Set : aliased cpu_set_t;
+CPU_Set_Ptr : cpu_set_t_ptr := null;
 
-CPU_Set : cpu_set_t_ptr := null;
 Result  : Interfaces.C.int;
 
  begin
@@ -1406,16 +1414,17 @@
 if T.Common.Base_CPU /= Multiprocessors.Not_A_Specific_CPU then
 
--  Set the affinity to an unique CPU
+   System.OS_Interface.CPU_ZERO (CPU_Set'Access);
+   System.OS_Interface.CPU_SET
+ (int (T.Common.Base_CPU), CPU_Set'Access);
+   CPU_Set_Ptr := CPU_Set'Access;
 
-   CPU_Set := new cpu_set_t'(bits => (others => False));
-   CPU_Set.bits (Integer (T.Common.Base_CPU)) := True;
-
 --  Handle Task_Info
 
 elsif T.Common.Task_Info /= null
   and then T.Common.Task_Info.CPU_Affinity /= Task_Info.Any_CPU
 then
-   CPU_Set := T.Common.Task_Info.CPU_Affinity'Access;
+   CPU_Set_Ptr := T.Common.Task_Info.CPU_Affinity'Access;
 
 --  Handle dispatching domains
 
@@ -1431,11 +1440,13 @@
--  domain other than the default one, or when the default one
--  has been modified.
 
-   CPU_Set := new cpu_set_t'(bits => (others => False));
+   System.OS_Interface.CPU_ZERO (CPU_Set'A

[Ada] Compiler fails renaming attribute with restricted run time

[Arnaud Charlet]

When using a restricted run time (such as ZFP or Ravenscar), the compiler
failed when there was a renaming of an attribute as subprogram because
it tried to load a package not available. This patch avoids this problem
by checking before trying to load the offending package.

The following must compile quietly:

package Rename_Attribute is
   function Integer_Pred (J: in Integer) return Integer renames Integer'Pred;
end Rename_Attribute;

Tested on x86_64-pc-linux-gnu, committed on trunk

2011-09-01  Jose Ruiz  

* sem_ch8.adb (Attribute_Renaming): Add missing check to avoid loading
package System.Aux_Dec when using restricted run-time libraries which
do not have this package.

Index: sem_ch8.adb
===
--- sem_ch8.adb (revision 178381)
+++ sem_ch8.adb (working copy)
@@ -3290,10 +3290,12 @@
   --  type is still not frozen). We exclude from this processing generic
   --  formal subprograms found in instantiations and AST_Entry renamings.
 
-  --  We must exclude VM targets because entity AST_Handler is defined in
-  --  package System.Aux_Dec which is not available in those platforms.
+  --  We must exclude VM targets and restricted run-time libraries because
+  --  entity AST_Handler is defined in package System.Aux_Dec which is not
+  --  available in those platforms.
 
   if VM_Target = No_VM
+and then not Restricted_Profile
 and then not Present (Corresponding_Formal_Spec (N))
 and then Etype (Nam) /= RTE (RE_AST_Handler)
   then

[Ada] improve speed for many tagged types

[Arnaud Charlet]

This patch improves compilation speed when compiling packages with huge numbers
of tagged types and interfaces, with complicated inheritance patterns.
No test is available -- the problem only occurred for enormous packages.

Tested on x86_64-pc-linux-gnu, committed on trunk

2011-09-01  Bob Duff  

* sem_attr.adb (Analyze_Access_Attribute): Do not call
Kill_Current_Values for P'Unrestricted_Access, where P is library level

Index: sem_attr.adb
===
--- sem_attr.adb(revision 178400)
+++ sem_attr.adb(working copy)
@@ -601,30 +601,35 @@
 
 Build_Access_Subprogram_Type (P);
 
---  For unrestricted access, kill current values, since this
---  attribute allows a reference to a local subprogram that
---  could modify local variables to be passed out of scope
+--  For P'Access or P'Unrestricted_Access, where P is a nested
+--  subprogram, we might be passing P to another subprogram (but we
+--  don't check that here), which might call P. P could modify
+--  local variables, so we need to kill current values. It is
+--  important not to do this for library-level subprograms, because
+--  Kill_Current_Values is very inefficient in the case of library
+--  level packages with lots of tagged types.
 
-if Aname = Name_Unrestricted_Access then
+if Is_Library_Level_Entity (Entity (Prefix (N))) then
+   null;
 
-   --  Do not kill values on nodes initializing dispatch tables
-   --  slots. The construct Prim_Ptr!(Prim'Unrestricted_Access)
-   --  is currently generated by the expander only for this
-   --  purpose. Done to keep the quality of warnings currently
-   --  generated by the compiler (otherwise any declaration of
-   --  a tagged type cleans constant indications from its scope).
+--  Do not kill values on nodes initializing dispatch tables
+--  slots. The construct Prim_Ptr!(Prim'Unrestricted_Access)
+--  is currently generated by the expander only for this
+--  purpose. Done to keep the quality of warnings currently
+--  generated by the compiler (otherwise any declaration of
+--  a tagged type cleans constant indications from its scope).
 
-   if Nkind (Parent (N)) = N_Unchecked_Type_Conversion
- and then (Etype (Parent (N)) = RTE (RE_Prim_Ptr)
- or else
-   Etype (Parent (N)) = RTE (RE_Size_Ptr))
- and then Is_Dispatching_Operation
-(Directly_Designated_Type (Etype (N)))
-   then
-  null;
-   else
-  Kill_Current_Values;
-   end if;
+elsif Nkind (Parent (N)) = N_Unchecked_Type_Conversion
+  and then (Etype (Parent (N)) = RTE (RE_Prim_Ptr)
+  or else
+Etype (Parent (N)) = RTE (RE_Size_Ptr))
+  and then Is_Dispatching_Operation
+ (Directly_Designated_Type (Etype (N)))
+then
+   null;
+
+else
+   Kill_Current_Values;
 end if;
 
 return;

Re: [PATCH] Make vectorizer dumps more comparable

[Michael Matz]

Hi,

On Thu, 1 Sep 2011, Richard Sandiford wrote:

> Ira Rosen  writes:
> >> How about, as Micha suggested, print the location of the loop
> >> we currently investigate from vectorize_loops () where we
> >> call find_loop_location () instead?
> >
> > The problem is that a dump of a single loop can be pretty long, and "start
> > to analyze loop..."/"finish to analyze loop..." may be not visible enough.
> > I am OK with adding these printings though (in addition to line numbers).
> >
> > I understand why you didn't like to see the file location, but what's the
> > problem with the line number?
> 
> +1 FWIW.  I found these per-line locations really useful when doing
> the strided load/store stuff.

Really?  Because the dumper always prints the location of the loop (i.e. 
its first line), not the location of the individual statements.  Therefore 
anything searching for file:line number will match all lines connected 
with dealing with one loop, there's no differentiation between them.  And 
hence I also don't see Iras point.  Searching for file:line from 
file start before Richis changes would get you to the start where the loop 
is dealt with, and then not a bit further because all lines would be so 
prefixed.   Searching from file end would get you to the end of the loop 
processing (with the final decision), and also not further because of the 
same prefix everywhere.

So, no, I don't see how to prefix every line with the same prefix provides 
anything useful.  Can you show me how it was useful to you?  (Or you Ira?)

FWIW my proposal would have been to do this:
file:123 starting loop so-and-so
... unprefixed lines all dealing with this loop
file:123 finished loop so-and-so


Ciao,
Michael.

Re: Initial shrink-wrapping patch

[Richard Sandiford]

Bernd Schmidt  writes:
> +/* A for_each_rtx subroutine of record_hard_reg_sets.  */
> +static int
> +record_hard_reg_uses_1 (rtx *px, void *data)
> +{
> +  rtx x = *px;
> +  HARD_REG_SET *pused = (HARD_REG_SET *)data;
> +
> +  if (REG_P (x) && REGNO (x) < FIRST_PSEUDO_REGISTER)
> +{
> +  int nregs = hard_regno_nregs[REGNO (x)][GET_MODE (x)];
> +  while (nregs-- > 0)
> + SET_HARD_REG_BIT (*pused, REGNO (x) + nregs);
> +}

add_to_hard_reg_set (pused, GET_MODE (x), REGNO (x));

> +/* A subroutine of requires_stack_frame_p, called via for_each_rtx.
> +   If any change is made, set CHANGED
> +   to true.  */

Strange line break, and comment doesn't match code (no "changed" variable).
Probably moot though because:

> +/* Return true if INSN requires the stack frame to be set up.
> +   PROLOGUE_USED contains the hard registers used in the function
> +   prologue.  */
> +static bool
> +requires_stack_frame_p (rtx insn, HARD_REG_SET prologue_used)
> +{
>[...]
> +  if (for_each_rtx (&PATTERN (insn), frame_required_for_rtx, NULL))
> +return true;
> +  CLEAR_HARD_REG_SET (hardregs);
> +  note_stores (PATTERN (insn), record_hard_reg_sets, &hardregs);
> +  if (hard_reg_set_intersect_p (hardregs, prologue_used))
> +return true;
> +  AND_COMPL_HARD_REG_SET (hardregs, call_used_reg_set);
> +  for (regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++)
> +if (TEST_HARD_REG_BIT (hardregs, regno)
> + && df_regs_ever_live_p (regno))
> +  return true;

...I suppose this ought to use DF instead.

Does something guarantee that non-local gotos are marked as
needing a frame?

Why do we need to check specifically for pic_offset_table_rtx?  Isn't it
handled by the generic "live registers set by the prologue" test?
Reason for asking is that pic_offset_table_rtx can be a global value,
such as for mips*-elf.

> -/* Insert gen_return at the end of block BB.  This also means updating
> -   block_for_insn appropriately.  */
> +
> +static rtx
> +gen_return_pattern (bool simple_p)
> +{
> +#ifdef HAVE_simple_return
> +  return simple_p ? gen_simple_return () : gen_return ();
> +#else
> +  gcc_assert (!simple_p);
> +  return gen_return ();
> +#endif
> +}

Missing function comment.

> +
> +/* Insert an appropriate return pattern at the end of block BB.  This
> +   also means updating block_for_insn appropriately.  */
>  
>  static void
> -emit_return_into_block (basic_block bb)
> +emit_return_into_block (bool simple_p, basic_block bb)

Pedantic, but the comment should reference SIMPLE_P.

> +#ifdef HAVE_simple_return
> +  /* Try to perform a kind of shrink-wrapping, making sure the
> + prologue/epilogue is emitted only around those parts of the
> + function that require it.  */
> +
> +  if (flag_shrink_wrap && HAVE_simple_return && !flag_non_call_exceptions
> +  && HAVE_prologue && !crtl->calls_eh_return)
> +{

Maybe it should be obvious, but could you add a comment explaining why
flag_non_call_exceptions and crtl->calls_eh_return need to be checked
explicitly?  I can see why we don't want to optimise functions that
call eh_return, but I was curious why it wasn't handled by the general
insn-level analysis.

Would checking prologue_seq != NULL be better than HAVE_prologue?

> +  HARD_REG_SET prologue_clobbered, prologue_used, live_on_edge;
> +  rtx p_insn;
> +
> +  VEC(basic_block, heap) *vec;
> +  basic_block bb;
> +  bitmap_head bb_antic_flags;
> +  bitmap_head bb_on_list;
> +
> +  /* Compute the registers set and used in the prologue.  */
> +  CLEAR_HARD_REG_SET (prologue_clobbered);
> +  CLEAR_HARD_REG_SET (prologue_used);
> +  for (p_insn = prologue_seq; p_insn; p_insn = NEXT_INSN (p_insn))
> + {
> +   HARD_REG_SET this_used;
> +   if (!NONDEBUG_INSN_P (p_insn))
> + continue;
> +
> +   CLEAR_HARD_REG_SET (this_used);
> +   note_uses (&PATTERN (p_insn), record_hard_reg_uses,
> +  &this_used);
> +   AND_COMPL_HARD_REG_SET (this_used, prologue_clobbered);
> +   IOR_HARD_REG_SET (prologue_used, this_used);
> +   note_stores (PATTERN (p_insn), record_hard_reg_sets,
> +&prologue_clobbered);
> + }

Should this iterate over split_prologue_seq as well?

Could we combine prologue_seq and split_prologue_seq into a single sequence?

> +  bitmap_initialize (&bb_antic_flags, &bitmap_default_obstack);
> +  bitmap_initialize (&bb_on_list, &bitmap_default_obstack);
> +
> +  vec = VEC_alloc (basic_block, heap, n_basic_blocks);
> +
> +  FOR_EACH_BB (bb)
> + {
> +   rtx insn;
> +   FOR_BB_INSNS (bb, insn)
> + {
> +   if (requires_stack_frame_p (insn, prologue_used))
> + {
> +   bitmap_set_bit (&bb_flags, bb->index);
> +   VEC_quick_push (basic_block, vec, bb);
> +   break;
> + }
> + }

Excess { and } in for loop.

> + }
> +
> +  /* For every basic block that needs a prologu

[Ada] Naming exceptions inherited in extending projects

[Arnaud Charlet]

When a project A with naming exceptions in its package Naming is extended
by a project B with no package Naming, the naming exceptions are
inherited in project B. If there are source files in the source
directories of project B that correspond to the inherited naming
exceptions, these files are sources of project B.

Example:

project A is
   package Naming is
  for Body ("pkg2") use "pkg2.ada";
  for Spec ("pkg2") use "pkg2_.ada";
   end Naming;
end A;

project B extends "a/a.gpr" is
   for Main use ("main.adb");
end B;

The files pkg2.ada and pkg_.ada need to be found in the source
directory of project A. If there are also files pkg2.ada and/or pkg2_.ada
in the source directory of project B, they are sources of B, so they
should be used when linking the executable.

Tested on x86_64-pc-linux-gnu, committed on trunk

2011-09-01  Vincent Celier  

* prj-proc.adb, prj.ads, prj-nmsc.adb, prj-util.adb, prj-util.ads,
prj-env.adb: Implement inheritance of naming exceptions in extending
projects.

Index: prj-proc.adb
===
--- prj-proc.adb(revision 178412)
+++ prj-proc.adb(working copy)
@@ -398,69 +398,62 @@
  Arr := Shared.Arrays.Table (A1);
  A1  := Arr.Next;
 
- if not Restricted
-   or else
- (Arr.Name /= Snames.Name_Body   and then
-  Arr.Name /= Snames.Name_Spec   and then
-  Arr.Name /= Snames.Name_Implementation and then
-  Arr.Name /= Snames.Name_Specification)
- then
---  Remove the Next component
+ --  Remove the Next component
 
-Arr.Next := No_Array;
-Array_Table.Increment_Last (Shared.Arrays);
+ Arr.Next := No_Array;
+ Array_Table.Increment_Last (Shared.Arrays);
 
---  Create new Array declaration
+ --  Create new Array declaration
 
-if To.Arrays = No_Array then
-   To.Arrays := Array_Table.Last (Shared.Arrays);
-else
-   Shared.Arrays.Table (A2).Next :=
- Array_Table.Last (Shared.Arrays);
-end if;
+ if To.Arrays = No_Array then
+To.Arrays := Array_Table.Last (Shared.Arrays);
+ else
+Shared.Arrays.Table (A2).Next :=
+  Array_Table.Last (Shared.Arrays);
+ end if;
 
-A2 := Array_Table.Last (Shared.Arrays);
+ A2 := Array_Table.Last (Shared.Arrays);
 
---  Don't store the array as its first element has not been set yet
+ --  Don't store the array as its first element has not been set yet
 
---  Copy the array elements of the array
+ --  Copy the array elements of the array
 
-E1 := Arr.Value;
-Arr.Value := No_Array_Element;
-while E1 /= No_Array_Element loop
+ E1 := Arr.Value;
+ Arr.Value := No_Array_Element;
+ while E1 /= No_Array_Element loop
 
-   --  Copy the array element
+--  Copy the array element
 
-   Elm := Shared.Array_Elements.Table (E1);
-   E1 := Elm.Next;
+Elm := Shared.Array_Elements.Table (E1);
+E1 := Elm.Next;
 
-   --  Remove the Next component
+--  Remove the Next component
 
-   Elm.Next := No_Array_Element;
+Elm.Next := No_Array_Element;
 
-   --  Change the location
+Elm.Restricted := Restricted;
+--  Change the location
 
-   Elm.Value.Location := New_Loc;
-   Array_Element_Table.Increment_Last (Shared.Array_Elements);
+Elm.Value.Location := New_Loc;
+Array_Element_Table.Increment_Last (Shared.Array_Elements);
 
-   --  Create new array element
+--  Create new array element
 
-   if Arr.Value = No_Array_Element then
-  Arr.Value :=
-Array_Element_Table.Last (Shared.Array_Elements);
-   else
-  Shared.Array_Elements.Table (E2).Next :=
-Array_Element_Table.Last (Shared.Array_Elements);
-   end if;
+if Arr.Value = No_Array_Element then
+   Arr.Value :=
+ Array_Element_Table.Last (Shared.Array_Elements);
+else
+   Shared.Array_Elements.Table (E2).Next :=
+ Array_Element_Table.Last (Shared.Array_Elements);
+end if;
 
-   E2 := Array_Element_Table.Last (Shared.Array_Elements);
-   Shared.Array_Elements.Table (E2) := Elm;
-end loop;
+E2 := Array_Element_Table.Last (Shared.Array_Elements);
+Shared.Array_Elements.Table (E2) := Elm;
+ end loop;
 
---  Finally, store the new array
+ --  Finally, store the new array
 
-Shared.Arrays

Re: Rename across basic block boundaries

[Richard Sandiford]

Bernd Schmidt  writes:
> On 08/26/11 14:57, Richard Sandiford wrote:
>>> +  /* There must be some kind of conflict.  Set the unusable for all
>>> + overlapping registers.  */
>>> +  min_reg = chain->regno;
>>> +  if (incoming_nregs < 0)
>>> +min_reg += incoming_nregs;
>>> +  max_reg = chain->regno + chain->nregs;
>>> +  for (i = min_reg; i < max_reg; i++)
>>> +ri->incoming[i].unusable = true;
>> 
>> In the incoming_nregs < 0 case, we only need to set
>> ri->incoming[chain->regno + incoming_nregs] itself, right,
>> not the other registers between that and ri->incoming[chain->regno]?
>> If so, I think it'd be clearer to have:
>> 
>>   /* There must be some kind of conflict.  Prevent both the old and
>>  new ranges from being used.  */
>>   if (incoming_nregs < 0)
>> ri->incoming[chain->regno + incoming_nregs].unusable = true;
>>   for (i = 0; i < chain->nregs; i++)
>> ri->incoming[chain->regno + i].unusable = true;
>> 
>> When I first looked at the code, I was wondering why we changed every
>> register in (chain->regno + incoming_nregs, chain_regno), but none in
>> [chain->regno + chain->nregs, OLD_END).  Seems like we should do neither
>> (as in the above suggestion) or both.
>
> I think both was the original intention (OLD_END being min_reg +
> ri->incoming[min_reg].nregs, right?),

Right.  There'd have been a max operation on max_reg too.

> but yours works too.

Ok, great.

>>> +  /* Process all basic blocks by using a worklist, adding unvisited 
>>> successor
>>> + blocks whenever we reach the end of one basic blocks.  This ensures 
>>> that
>>> + whenever possible, we only process a block after at least one of its
>>> + predecessors, which provides a "seeding" effect to make the logic in
>>> + set_incoming_from_chain and init_rename_info useful.  */
>> 
>> Wouldn't a reverse post-order (inverted_post_order_compute) allow even
>> more pre-opening (as well as being less code)?
>
> I can't really tell from the comments what that function is supposed to
> produce.

Sorry, I thought it was supposed to produce a reverse postorder, but...

> I've made a change to use it to order the bbs, but that made rr
> visit basic blocks without seeing any of their predecessors first,

...I guess not. :-)  pre_and_rev_post_order_compute should though.
Could you try that instead?

Richard

Re: [C++0x] contiguous bitfields race implementation

[Aldy Hernandez]

My point is, the middle-end infrastructure makes it possible for this
case to appear, and it seems to be easy to handle conservatively.
There isn't a need to wait for users to run into an ICE or an assert we put
there IMHO.  If I'd be fluent in Ada I'd write you a testcase, but I ain't.


Ughh, this is getting messier.

Ok, I propose keeping track of the field prior (lastfld), calling 
get_inner_reference() and adding DECL_SIZE (or tbitsize if you prefer) 
to calculate maxbits without the padding.


Notice the comment at the top.  We can get rid of yet another call to 
get_inner_reference later.


Is this what you had in mind?

BTW, we don't need to round up to the next byte here, do we?

Thanks.
Aldy

  /* If we found the end of the bit field sequence, include the
 padding up to the next field...  */
  if (fld)
{
  tree end_offset, t;
  HOST_WIDE_INT end_bitpos;

  /* FIXME: Only call get_inner_reference once (at the beginning
 of the bit region), and use
 DECL_FIELD_OFFSET+DECL_FIELD_BIT_OFFSET throughout to
 calculate any subsequent bit offset.  */

  /* Even if the bitfield we access (and thus the whole region) is
 at a constant offset, the field _following_ the bitregion can
 be at variable offset.  In this case, do not include any
 padding.  This is mostly for Ada.  */
  if (TREE_CODE (DECL_FIELD_OFFSET (fld)) != INTEGER_CST)
{
  get_inner_reference (build3 (COMPONENT_REF,
   TREE_TYPE (exp),
   TREE_OPERAND (exp, 0),
   lastfld, NULL_TREE),
   &tbitsize, &end_bitpos, &end_offset,
   &tmode, &tunsignedp, &tvolatilep, true);

  /* Calculate the size of the bit region up the last
 bitfield, excluding any subsequent padding.

 t = (end_byte_off - start_byte_offset) * 8 + end_bit_off */
  end_offset = end_offset ? end_offset : size_zero_node;
  t = fold_build2 (PLUS_EXPR, size_type_node,
   fold_build2 (MULT_EXPR, size_type_node,
fold_build2 (MINUS_EXPR, size_type_node,
 end_offset,
 *byte_offset),
build_int_cst (size_type_node,
   BITS_PER_UNIT)),
   build_int_cst (size_type_node,
  end_bitpos));
  /* Add the bitsize of the last field.  */
  t = fold_build2 (PLUS_EXPR, size_type_node,
   t, DECL_SIZE (lastfld));

  *maxbits = tree_low_cst (t, 1);
  return;
}
...
...
...

Re: [Patch, fortran] Fix PR fortran/50050 breakage: ICE on valid with null pointer initialization

[Mikael Morin]

On Wednesday 24 August 2011 10:58:19 Tobias Burnus wrote:
> On 08/24/2011 12:01 AM, Mikael Morin wrote:
> > this is an attempt to fix my recent breakage for PR50050.
> > I forgot that shape can't always be known, and thus, that for some
> > expressions, the shape field is a NULL pointer.
> > 
> > Neither bootstraped, nor regression tested, but it is in progress. My
> > machine does its best (which is not a lot) to have this properly
> > compiled and tested (and then committed) as soon as possible.
> > Otherwise OK for 4.{4..7} ?
> 
> OK for 4.6 and 4.7 (after regtesting).
> 
> As 4.4 and 4.5 are not affected by this regression and only by the
> original bug (comment 0 of PR 50050), please wait a while before
> committing the combined patch to older branches. (One could also
> consider whether having 50050c0 fixed on 4.6/4.7 is sufficient.)
> 
> Tobias

Hello,

Despite your call for less non-regression backports, I'm going to use the 
following (slightly less intrusive) variant for 4.5 and 4.4. I can't stand 
knowing this bug exists in the wild.

Mikael

PS: the testcase alloc_comp_initializer_3.f90 doesn't trigger on 4.5, so this 
patch fixes a silent memory out of bound (instead of an ICE). I add the 
testcase anyway, in case it could trigger on some targets.
2011-09-01  Mikael Morin  

* resolve.c (gfc_expr_to_initialize): Don't copy rank.
Free copied shape.  Recalculate shape and rank.
Index: resolve.c
===
--- resolve.c	(révision 178386)
+++ resolve.c	(copie de travail)
@@ -6172,10 +6172,19 @@ gfc_expr_to_initialize (gfc_expr *e)
 	for (i = 0; i < ref->u.ar.dimen; i++)
 	  ref->u.ar.start[i] = ref->u.ar.end[i] = ref->u.ar.stride[i] = NULL;
 
-	result->rank = ref->u.ar.dimen;
 	break;
   }
 
+  if (result->shape != NULL)
+{
+  for (i = 0; i < result->rank; i++)
+	mpz_clear (result->shape[i]);
+  gfc_free (result->shape);
+  result->shape = NULL;
+}
+
+  /* Recalculate rank, shape, etc.  */
+  gfc_resolve_expr (result);
   return result;
 }
 
2011-09-01  Mikael Morin  

* gfortran.dg/alloc_comp_initializer_3.f90: New test.
* gfortran.dg/pointer_comp_init.f90: New test.
! { dg-do compile }
!
! PR fortran/50050
! Out of bound whilst releasing initialization of allocate object
!
! Contributed by someone 

program bug
  implicit none
  type foo
integer, pointer :: a => null()
  end type
  type(foo), dimension(:,:), allocatable :: data
  allocate(data(1:1,1)) ! This used to lead to an ICE
end program
! { dg-do compile }
!
! PR fortran/50050
! ICE whilst trying to access NULL shape.

! Reduced from the FoX library http://www1.gly.bris.ac.uk/~walker/FoX/
! Contributed by Andrew Benson 

module m_common_attrs
  implicit none

  type dict_item
  end type dict_item

  type dict_item_ptr
 type(dict_item), pointer :: d => null()
  end type dict_item_ptr

contains

  subroutine add_item_to_dict()
type(dict_item_ptr), pointer :: tempList(:)
integer :: n

allocate(tempList(0:n+1)) 
  end subroutine add_item_to_dict

end module m_common_attrs

! { dg-final { cleanup-modules "m_common_attrs" } }

Re: [PATCH] Make vectorizer dumps more comparable

[Richard Sandiford]

Michael Matz  writes:
>> Ira Rosen  writes:
>> >> How about, as Micha suggested, print the location of the loop
>> >> we currently investigate from vectorize_loops () where we
>> >> call find_loop_location () instead?
>> >
>> > The problem is that a dump of a single loop can be pretty long, and "start
>> > to analyze loop..."/"finish to analyze loop..." may be not visible enough.
>> > I am OK with adding these printings though (in addition to line numbers).
>> >
>> > I understand why you didn't like to see the file location, but what's the
>> > problem with the line number?
>> 
>> +1 FWIW.  I found these per-line locations really useful when doing
>> the strided load/store stuff.
>
> Really?  Because the dumper always prints the location of the loop (i.e. 
> its first line), not the location of the individual statements.  Therefore 
> anything searching for file:line number will match all lines connected 
> with dealing with one loop, there's no differentiation between them.  And 
> hence I also don't see Iras point.  Searching for file:line from 
> file start before Richis changes would get you to the start where the loop 
> is dealt with, and then not a bit further because all lines would be so 
> prefixed.   Searching from file end would get you to the end of the loop 
> processing (with the final decision), and also not further because of the 
> same prefix everywhere.
>
> So, no, I don't see how to prefix every line with the same prefix provides 
> anything useful.  Can you show me how it was useful to you?

I suppose it's just personal preference.  I look at the dumps using less,
and search for the line number.  That highlights all the lines in the
loop I care about.  It just seemed like a very nice visual cue when
the relevant part of the dump was longer than a page.

Richard

Re: [PATCH, ARM] Unaligned accesses for packed structures [1/2]

[Ulrich Weigand]

Julian Brown wrote:

> The problem is, if we're using little-endian bit numbering for memory
> locations in big-endian-bytes mode, we need to define an origin from
> which to count "backwards" from. For the current implementation, this
> will now be (I believe) one word beyond the base address of the access
> in question, which is IMO slightly bizarre, to say the least.

It doesn't seem all that bizarre to me.  Conceptually, the extraction
(etc.) operation works on an operand of integer type (usually, word sized)
in two steps:
- read the operand from its storage location, resulting in a plain
  (conceptual) integer value
- extract certain numbered bits from that integer value

The first step, reading the operand from the storage location,
depends on BYTES_BIG_ENDIAN (as all memory reads do).  It does not
depend on BITS_BIG_ENDIAN at all.

The second step, extracting numbered bits, depends on BITS_BIG_ENDIAN,
which provides the link between bit number and its value.  This step
however does not depend on BYTES_BIG_ENDIAN at all.  [However, if
BITS_BIG_ENDIAN is true, you need to consider the total size of the
operand in order to perform the conversion, since bit 0 then refers
to the most-significant bit, and which bit that is depends on the
size ...  But this still does not depend on *BYTES_BIG_ENDIAN*.]

Thus, the two flags can be considered really independent, and any
combination is quite well-defined.


When actually implementing the extraction, you will of course deviate
from that conceptual sequence, e.g. by avoiding to read certain bytes
if you know none of the bits in them will end up in the final result.
But while this might result in some computations that may not 
immediately look obvious, in the end this is just an optimization
step and doesn't change that the endian flags remain well-defined ...


Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  ulrich.weig...@de.ibm.com

Re: [C++0x] contiguous bitfields race implementation

[Aldy Hernandez]

[Jason, can you pontificate on tail-padding and the upcoming C++ 
standard with regards to bitfields?]



so you wasn't convinced about my worries about tail-padding re-use?


To answer your question, I believe we can't touch past the last field 
(into the padding) if the subsequent record will be packed into the 
first's padding.


struct A {
  int a : 17;
};
struct B : public A {
  char c;
};

So here, if  gets packed into the tail-padding of A, we can't touch 
the padding of A when storing into .  These are different structures, 
and I assume would be treated as nested structures, which are distinct 
memory locations.


Is there a way of distinguishing this particular variant (possible 
tail-packing), or will we have to disallow storing into the record tail 
padding altogether?  That would seriously suck.


Aldy

Re: [C++0x] contiguous bitfields race implementation

[Jason Merrill]

On 09/01/2011 10:52 AM, Aldy Hernandez wrote:

To answer your question, I believe we can't touch past the last field
(into the padding) if the subsequent record will be packed into the
first's padding.


Right.


struct A {
  int a : 17;
};
struct B : public A {
  char c;
};

So here, if  gets packed into the tail-padding of A, we can't touch
the padding of A when storing into .


But that doesn't apply to this testcase because A is a POD class, so we 
don't mess with its tail padding.



Is there a way of distinguishing this particular variant (possible
tail-packing), or will we have to disallow storing into the record tail
padding altogether? That would seriously suck.


Basically you can only touch the size of the CLASSTYPE_AS_BASE variant. 
 For many classes this will be the same as the size of the class itself.


Jason

Ping: [rs6000] Fix creation of invalid CONST_VECTORs

[Richard Sandiford]

Fixes generation of invalid CONST_VECTORs in the powerpc backend:

http://gcc.gnu.org/ml/gcc-patches/2011-08/msg01861.html

Pat Haugen confirms that it also works on GNU/Linux (thanks).

Richard

Re: [C++0x] contiguous bitfields race implementation

[Aldy Hernandez]

Is there a way of distinguishing this particular variant (possible
tail-packing), or will we have to disallow storing into the record tail
padding altogether? That would seriously suck.


Basically you can only touch the size of the CLASSTYPE_AS_BASE variant.
For many classes this will be the same as the size of the class itself.


All this code is in the middle end, so we're language agnostic.

What do we need here, a hook to query the front-end, or is it too late? 
 Or will we have to play it conservative and never touch the padding 
(regardless of language)?


Aldy

Re: [C++0x] contiguous bitfields race implementation

[Jason Merrill]

On 09/01/2011 11:10 AM, Aldy Hernandez wrote:

Basically you can only touch the size of the CLASSTYPE_AS_BASE variant.
For many classes this will be the same as the size of the class itself.


All this code is in the middle end, so we're language agnostic.

What do we need here, a hook to query the front-end, or is it too late?
Or will we have to play it conservative and never touch the padding
(regardless of language)?


I think it would make sense to expose this information to the back end 
somehow.  A hook would do the trick: call it type_data_size or 
type_min_size or some such, which in the C++ front end would return 
TYPE_SIZE (CLASSTYPE_AS_BASE (t)) for classes or just TYPE_SIZE for 
other types.


Jason

Re: [PATCH][ARM] pr50193: ICE on a | (b << negative-constant)

[Andrew Stubbs]

On 01/09/11 14:21, Joseph S. Myers wrote:

On Thu, 1 Sep 2011, Andrew Stubbs wrote:


This patch fixes the problem by merely checking that the constant is positive.
I've confirmed that values larger than the mode-size are not a problem because
the compiler optimizes those away earlier, even at -O0.


Do you mean that you have observed for some testcases that they get
optimized away - or do you have reasons (if so, please state them) to
believe that any possible path through the compiler that would result in a
larger constant here (possibly as a result of constant propagation and
other optimizations) will always result in it being optimized away as
well?  If it's just observation it would be better to put the complete
check in here.


OK, fair enough, redundant or not, here's a patch with belt and braces.

OK now?


Quite of few of the Csmith-generated bug reports from John Regehr have
involved constants appearing in unexpected places as a result of
transformations in the compiler.  It would probably be a good idea for
someone to try using Csmith to find ARM compiler bugs (both ICEs and
wrong-code); pretty much all the bugs reported have been testing on x86
and x86_64, so it's likely there are quite a few bugs in the ARM back end
that could be found that way.


Interesting, I've suggested it within Linaro.

Thanks

Andrew

Re: [PATCH][ARM] pr50193: ICE on a | (b << negative-constant)

[Andrew Stubbs]

On 01/09/11 16:26, Andrew Stubbs wrote:

OK, fair enough, redundant or not, here's a patch with belt and braces.

OK now?


And again, with the patch 

Andrew
2011-09-01  Andrew Stubbs  

	gcc/
	* config/arm/predicates.md (shift_amount_operand): Ensure shift
	amount is in the range 1..mode_size-1.

	gcc/testsuite/
	* gcc.dg/pr50193-1.c: New file.

--- a/gcc/config/arm/predicates.md
+++ b/gcc/config/arm/predicates.md
@@ -132,7 +132,9 @@
 (define_predicate "shift_amount_operand"
   (ior (and (match_test "TARGET_ARM")
 	(match_operand 0 "s_register_operand"))
-   (match_operand 0 "const_int_operand")))
+   (and (match_operand 0 "const_int_operand")
+	(match_test "INTVAL (op) > 0
+			 && INTVAL (op) <= GET_MODE_PRECISION (mode)"
 
 (define_predicate "arm_add_operand"
   (ior (match_operand 0 "arm_rhs_operand")
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr50193-1.c
@@ -0,0 +1,10 @@
+/* PR 50193: ARM: ICE on a | (b << negative-constant) */
+/* Ensure that the compiler doesn't ICE.  */
+
+/* { dg-options "-O2" } */
+
+int
+foo(int a, int b)
+{
+  return a | (b << -3); /* { dg-warning "left shift count is negative" } */
+}

[v3] Remove noexcept mistakenly added to the unordered_* move cons

[Paolo Carlini]

Hi,

while working on libstdc++/50257 I noticed this recent mistake of mine: 
given our current impl of the unordered containers, the move constructor 
allocates memory, thus can certainly throw, similarly to deque, for 
example. Also, I reordered the operations in the body of the move 
constructor, because we weren't updating __ht._M_rehash_policy before 
using _M_next_bkt on it.


Tested x86_64-linux, committed to mainline (the issue with the body of 
the move cons exists also in 4.6.x, should be fixed there too, probably)


Thanks,
Paolo.


2011-09-01  Paolo Carlini  

* include/bits/hashtable.h (_Hashtable<>::_Hashtable(_Hashtable&&)):
Remove noexcept, the move constructor allocates memory; rearrange
the code in the body to consistently update __ht._M_rehash_policy
before using _M_next_bkt on it.
* include/debug/unordered_map: Adjust.
* include/debug/unordered_set: Likewise.
* include/profile/unordered_map: Likewise.
* include/profile/unordered_set: Likewise.
* testsuite/23_containers/unordered_map/cons/
noexcept_move_construct.cc: Remove.
* testsuite/23_containers/unordered_set/cons/
noexcept_move_construct.cc: Likewise.
* testsuite/23_containers/unordered_multimap/cons/
noexcept_move_construct.cc: Likewise.
* testsuite/23_containers/unordered_multiset/cons/
noexcept_move_construct.cc: Likewise.
Index: include/debug/unordered_map
===
--- include/debug/unordered_map (revision 178397)
+++ include/debug/unordered_map (working copy)
@@ -104,8 +104,6 @@
   : _Base(__x) { }
 
   unordered_map(unordered_map&& __x)
-  noexcept(__and_,
- is_nothrow_copy_constructible<_Pred>>::value)
   : _Base(std::move(__x)) { }
 
   unordered_map(initializer_list __l,
@@ -485,8 +483,6 @@
   : _Base(__x) { }
 
   unordered_multimap(unordered_multimap&& __x)
-  noexcept(__and_,
- is_nothrow_copy_constructible<_Pred>>::value)
   : _Base(std::move(__x)) { }
 
   unordered_multimap(initializer_list __l,
Index: include/debug/unordered_set
===
--- include/debug/unordered_set (revision 178397)
+++ include/debug/unordered_set (working copy)
@@ -104,8 +104,6 @@
   : _Base(__x) { }
 
   unordered_set(unordered_set&& __x)
-  noexcept(__and_,
- is_nothrow_copy_constructible<_Pred>>::value)
   : _Base(std::move(__x)) { }
 
   unordered_set(initializer_list __l,
@@ -480,8 +478,6 @@
   : _Base(__x) { }
 
   unordered_multiset(unordered_multiset&& __x)
-  noexcept(__and_,
- is_nothrow_copy_constructible<_Pred>>::value)
   : _Base(std::move(__x)) { }
 
   unordered_multiset(initializer_list __l,
Index: include/profile/unordered_map
===
--- include/profile/unordered_map   (revision 178397)
+++ include/profile/unordered_map   (working copy)
@@ -104,8 +104,6 @@
   }
 
   unordered_map(unordered_map&& __x)
-  noexcept(__and_,
- is_nothrow_copy_constructible<_Pred>>::value)
   : _Base(std::move(__x)) 
   {
 __profcxx_hashtable_construct(this, _Base::bucket_count());
@@ -374,8 +372,6 @@
   }
 
   unordered_multimap(unordered_multimap&& __x)
-  noexcept(__and_,
- is_nothrow_copy_constructible<_Pred>>::value)
   : _Base(std::move(__x))
   {
 __profcxx_hashtable_construct(this, _Base::bucket_count());
Index: include/profile/unordered_set
===
--- include/profile/unordered_set   (revision 178397)
+++ include/profile/unordered_set   (working copy)
@@ -103,8 +103,6 @@
   }
 
   unordered_set(unordered_set&& __x)
-  noexcept(__and_,
- is_nothrow_copy_constructible<_Pred>>::value)
   : _Base(std::move(__x)) 
   { 
 __profcxx_hashtable_construct(this, _Base::bucket_count());
@@ -348,8 +346,6 @@
   }
 
   unordered_multiset(unordered_multiset&& __x)
-  noexcept(__and_,
- is_nothrow_copy_constructible<_Pred>>::value)
   : _Base(std::move(__x))
   {
 __profcxx_hashtable_construct(this, _Base::bucket_count());
Index: include/bits/hashtable.h
===
--- include/bits/hashtable.h(revision 178397)
+++ include/bits/hashtable.h(working copy)
@@ -674,8 +674,6 @@
 _Hashtable<_Key, _Value, _Allocator, _ExtractKey, _Equal,
   _H1, _H2, _Hash, _RehashPolicy, __chc, __cit, __uk>::
 _Hashtable(_Hashtable&& __ht)
-noexcept(__and_,
-   is_nothrow_copy_constructible<_H1>>::value)
 : __detail::_Rehash_base<_Re

Re: [PATCH][ARM] pr50193: ICE on a | (b << negative-constant)

[Jakub Jelinek]

On Thu, Sep 01, 2011 at 04:29:25PM +0100, Andrew Stubbs wrote:
> On 01/09/11 16:26, Andrew Stubbs wrote:
> >OK, fair enough, redundant or not, here's a patch with belt and braces.
> >
> >OK now?
> 
> And again, with the patch 
> 
> Andrew

> 2011-09-01  Andrew Stubbs  
> 
>   gcc/
>   * config/arm/predicates.md (shift_amount_operand): Ensure shift
>   amount is in the range 1..mode_size-1.
> 
>   gcc/testsuite/
>   * gcc.dg/pr50193-1.c: New file.
> 
> --- a/gcc/config/arm/predicates.md
> +++ b/gcc/config/arm/predicates.md
> @@ -132,7 +132,9 @@
>  (define_predicate "shift_amount_operand"
>(ior (and (match_test "TARGET_ARM")
>   (match_operand 0 "s_register_operand"))
> -   (match_operand 0 "const_int_operand")))
> +   (and (match_operand 0 "const_int_operand")
> + (match_test "INTVAL (op) > 0
> +  && INTVAL (op) <= GET_MODE_PRECISION (mode)"

IN_RANGE (INTVAL (op), 0, GET_MODE_PRECISION (mode) - 1) ?

1) shift by 0 is well defined (though not sure if arm backend
   supports it)
2) shift by mode precision is undefined
3) isn't mode here the mode of the shift operand rather than of the shift
   itself (perhaps on arm in all patterns they are the same - 
   a brief look at arm.amd suggest they are and they are SImode in all
   cases, so perhaps just IN_RANGE (INTVAL (op), 0, 31) ?)

Jakub

Re: Ping: [rs6000] Fix creation of invalid CONST_VECTORs

[David Edelsohn]

On Thu, Sep 1, 2011 at 11:08 AM, Richard Sandiford
 wrote:
> Fixes generation of invalid CONST_VECTORs in the powerpc backend:
>
>    http://gcc.gnu.org/ml/gcc-patches/2011-08/msg01861.html
>
> Pat Haugen confirms that it also works on GNU/Linux (thanks).

Okay.

Thanks, David

Re: [PATCH][ARM] pr50193: ICE on a | (b << negative-constant)

[Andrew Stubbs]

On 01/09/11 16:51, Jakub Jelinek wrote:

IN_RANGE (INTVAL (op), 0, GET_MODE_PRECISION (mode) - 1) ?


Ok, I can make that change.


1) shift by 0 is well defined (though not sure if arm backend
supports it)


Yeah, I suppose I could allow 0, but I don't know why it would be 
helpful? I mean, it's not like there's an operation that is only 
available with a no-op shift (I suppose you could say that all the ARM 
opcodes are like that, but neither the md file nor the official 
canonical assembler notation represents it that way).


Conversely, assuming it's even possible for a zero shift to get this 
far, I imagined it would be possible for combine to use the 
arith_shiftsi pattern and therefore prevent it from combining the 
and/or/add/whatever into something else more worthwhile, so I chose to 
insist on greater than zero.



2) shift by mode precision is undefined


The LSL and ROR require the shift amount to be 1..31 while the LSR and 
ASR require 1..32. Anecdotally speaking, gcc optimizes away (and warns 
about) shifts greater than 31, so I chose mode-1.



3) isn't mode here the mode of the shift operand rather than of the shift
itself (perhaps on arm in all patterns they are the same -
a brief look at arm.amd suggest they are and they are SImode in all
cases, so perhaps just IN_RANGE (INTVAL (op), 0, 31) ?)


Well, I didn't want to hard-code the number, but yes, that would be 
equivalent at present, I think.


I wasn't sure how to find the mode of shift operand in the predicate 
though, so I've assumed they're always the same size.  How would one 
find the proper mode in a predicate?


Andrew

[PING] [PATCH] PR c++/33255 - Support -Wunused-local-typedefs warning

[Dodji Seketeli]

I am friendly pinging this patch.


Dodji Seketeli  a écrit:

> Hello,
>
> Jason Merrill  writes:
>
>> On 08/06/2011 06:57 AM, Dodji Seketeli wrote:
>> > @@ -4340,6 +4340,8 @@ c_sizeof_or_alignof_type (location_t loc,
>> >value = fold_convert_loc (loc, size_type_node, value);
>> >gcc_assert (!TYPE_IS_SIZETYPE (TREE_TYPE (value)));
>> >
>> > +  maybe_record_local_typedef_use (type);
>> 
>> Why is this still needed?
>
> It is not.  I just forgot to remove it.  Done now.
>
>> 
>> > +/* Return TRUE if DECL is declared in the current function.  */
>> > +
>> > +bool
>> > +local_decl_p (tree decl)
>> 
>> I don't think we need to work so hard to compare contexts to
>> current_function_decl; just checking that the decl is in some function
>> should be enough, i.e. decl_function_context is non-null.
>
> Thanks.  I didn't know about decl_function_context.  I grepped for
> functions containing "local" in their names but I was doomed to
> failure.  I am using that now.
>
>> 
>> > +  static int unused_local_typedefs_warn_count;
>> 
>> This could use a comment.
>
> Added.
>
>> 
>> > +/* If T is a typedef variant type or a TYPE_DECL declared locally,
>> > +   mark it as used.  */
>> > +
>> > +void
>> > +maybe_record_local_typedef_use (tree t)
>> 
>> Does this still need to handle types?  And the meaning of TREE_USED
>> shouldn't change depending on a warning flag or whether a decl is
>> local or not; let's just mark all TYPE_DECLs as used.
>
> OK, done.
>
>> 
>> Seems like we don't need local_decl_p at all; at the definition point,
>> if we're in a function, a new declaration must be local enough for our
>> purposes.
>> 
>> > +void
>> > +cp_maybe_record_local_typedef_use (tree t)
>> > +{
>> > +  /*  If the current function is being instantiated, bail out.  */
>> 
>> As above, I think we shouldn't put so many conditions on setting
>> TREE_USED.
>
> I have removed cp_maybe_record_local_typedef_use altogether and just
> use maybe_record_typedef_use now.
>
>> 
>> > @@ -19572,6 +19572,9 @@ cp_parser_lookup_name (cp_parser *parser, tree 
>> > name,
>> >if (DECL_P (decl))
>> >  check_accessibility_of_qualified_id (decl, object_type, 
>> > parser->scope);
>> >
>> > +  cp_maybe_record_local_typedef_use (decl);
>> > +  cp_maybe_record_local_typedef_use (parser->scope);
>> 
>> Why do we need to handle parser->scope?  Wasn't it previously looked up?
>
> Sigh, my bad..  Fixed.
>
> Below is the updated patch that passed bootstrap and regression tests
> on x86_64-unknown-linux-gnu against trunk.
>
> Thanks.
>
> From: Dodji Seketeli 
> Date: Mon, 25 Jul 2011 19:02:07 +0200
> Subject: [PATCH] PR c++/33255 - Support -Wunused-local-typedefs warning
>
> gcc/
>
>   * c-decl.c (lookup_name): Use the new
>   maybe_record_typedef_use.
>   (pushdecl): Use the new
>   record_locally_defined_typedef.
>   (store_parm_decls): Allocate cfun->language.
>   (finish_function): Use the new maybe_warn_unused_local_typedefs,
>   and mark cfun->language to be collected.
>   (c_push_function_context): Allocate cfun->language here only if
>   needed.
>   (c_pop_function_context): Likewise, mark cfun->language
>   for collection only when it should be done.
>   * c-typeck.c (c_expr_sizeof_type, c_cast_expr): Use the new
>   maybe_record_local_typedef_use.
>
> gcc/c-family
>
>   * c-common.h (struct c_language_function::local_typedefs): New
>   field.
>   (record_locally_defined_typedef, maybe_record_typedef_use)
>   (maybe_warn_unused_local_typedefs): Declare new functions.
>   * c-common.c (record_locally_defined_typedef)
>   (maybe_record_typedef_use)
>   (maybe_warn_unused_local_typedefs): Define new functions.
>   * c.opt: Declare new -Wunused-local-typedefs flag.
>
> gcc/cp
>
>   * name-lookup.c (pushdecl_maybe_friend_1): Use the new
>   record_locally_defined_typedef.
>   * decl.c (finish_function): Use the new
>   maybe_warn_unused_local_typedefs.
>   (grokfield): Use the new record_locally_defined_typedef.
>   * parser.c (lookup_name): Use the new maybe_record_typedef_use.
>
> gcc/doc/
>
>   * invoke.texi: Update documentation for -Wunused-local-typedefs.
>
> gcc/testsuite/
>
>   * g++.dg/warn/Wunused-local-typedefs.C: New test file.
>   * c-c++-common/Wunused-local-typedefs.c: Likewise.
>
> libstdc++-v3/
>
>   * include/ext/bitmap_allocator.h
>   (__detail::__mini_vector::__lower_bound): Remove unused typedef.
>   * src/istream.cc (std::operator>>(basic_istream& __in,
>   basic_string& __str)): Likewise.
>   (std::getline): Likewise.
>   * src/valarray.cc (__valarray_product): Likewise.
> ---
>  gcc/c-decl.c   |   38 +-
>  gcc/c-family/c-common.c|   71 ++
>  gcc/c-family/c-common.h|7 +
>  gcc/c-family/c.opt |4 +
>  gcc/cp/decl.c  

Re: Add unwind information to mips epilogues

[Bernd Schmidt]

On 08/31/11 20:43, Richard Sandiford wrote:
> Bernd Schmidt  writes:
>> This is necessary when adding shrink-wrapping; otherwise dwarf2cfi sees
>> inconsistent information and aborts.
>>
>> Tested on mips64-elf together with the rest of the shrink-wrapping
>> patches. Ok?
> 
> It looks like the current code doesn't handle the RESTORE instruction.
> Could you also test that somehow?  A mipsisa32-elf run with -mips16
> ought to work

Hmm, I'm having some trouble with that.
 * mipsisa32-elf doesn't build on trunk
 * 4.6 doesn't allow plain "-mips16"; I have to add "-msoft-float"
 * With that, 4.6 produces rather a lot of testsuite failures.
   (>400 execute failures in my current run and it's not even gotten
   very far)

Just to make sure - am I missing anything here, or is this stuff just in
bad shape?


Bernd

[PATCH] Make devirtualization use BINFO_VTABLE instead of BINFO_VIRTUALS

[Martin Jambor]

Hi,

the patch below is an updated version of an RFC patch I have sent here
some three weeks ago:

http://gcc.gnu.org/ml/gcc-patches/2011-08/msg00975.html

However, this is no longer an RFC but a request for approval because I
have amended it to work also on architectures that use function
descriptors in their VMTs.

The original mail describes the patch in a bit more detail, the main
points are that:

- This patch significantly reduces memory footprint of WPA when LTOing
  C++ code.  Memory peak of WPA of Firefox decreases by by 36% from
  4554 MB to 2893 MB and that of 483.xalancbmk drops by 21% from
  555205 kB to 439121 kB.  The number of type-based devirtualizations
  is the same for both applications.  (I have verified all these
  numbers today.)

- Nevertheless, this method of devirtualization cannot automatically
  de-thunkize this-adjusting thunks and newly direct calls to them
  cannot be inlined because the inliner does not have this capability
  now.  This is in fact a regression from 4.6, and testcases
  ivinline-7.C and ivinline-9.C had to be XFAILed exactly for this
  reason.  The size of the memory savings and the fact that we do not
  devirtualize that much now make this an acceptable tradeoff, though.

- The main change of this patch is actually quite small, it is the
  reimplementation of gimple_get_virt_method_for_binfo and BINFO
  streaming.  However, because of the missing de-thunkization, all
  various delta parameters and fields could go away and associated
  code is dead.  I removed some of it but not all because it might be
  useful soon (such as edge optimization info streaming functions and
  gimple_adjust_this_by_delta).  I can remove those too upon request.

I have changed the testcase devirt-3.C slightly so that the called
function was not the first in the MVT, this has proved to be useful
when adjusting the patch for ia64.

I have bootstrapped and tested this patch on x86_64-linux (all
languages including obj-c++ and Ada) and ia64-linux (without Ada).  I
have also verified that LTOed Firefox browses and 483.xalancbmk gives
the expected testing output.  So, OK for trunk?

Thanks,

Martin



2011-08-31  Martin Jambor  

* cgraph.h (cgraph_indirect_call_info): Removed field thunk_delta.
* gimple-fold.c (gimple_get_virt_method_for_binfo): Rewritten to use
BINFO_VTABLE.  Parameter delta removed, all callers updated.
* tree.c (free_lang_data_in_binfo): Clear BINFO_VIRTUALs instead
BINFO_VTABLE.
* cgraph.c (cgraph_make_edge_direct): Removed parameter delta, updated
all calls.
* cgraphunit.c (cgraph_redirect_edge_call_stmt_to_callee): Removed
handling of thunk_delta.
* ipa-cp.c (get_indirect_edge_target): Removed parameter delta.
(devirtualization_time_bonus): Do not handle thunk deltas.
(ipcp_discover_new_direct_edges): Likewise.
* ipa-prop.c (ipa_make_edge_direct_to_target): Likewise.
(try_make_edge_direct_simple_call): Likewise.
(try_make_edge_direct_virtual_call): Likewise.
* lto-cgraph.c (output_cgraph_opt_summary_p): Likewise.  Mark
parameter set as unused.
(output_edge_opt_summary): Likewise.  Mark both parameters as unused.
* lto-cgraph.c (output_cgraph_opt_summary_p): Likewise.  Mark
parameter set as unused.
(output_edge_opt_summary): Likewise.  Mark both parameters as unused.
(input_edge_opt_summary): Likewise.
* lto-streamer-out.c (lto_output_ts_binfo_tree_pointers): Do not stream
BINFO_VIRTUALS at all.
* lto-streamer-in.c (lto_input_ts_binfo_tree_pointers): Likewise.

* testsuite/g++.dg/ipa/devirt-3.C: Added a distraction method.
* testsuite/g++.dg/ipa/ivinline-7.C: Added a test for direct call
discovery, xfailed test for inlining.
* testsuite/g++.dg/ipa/ivinline-9.C: Likewise.

Index: src/gcc/gimple-fold.c
===
--- src.orig/gcc/gimple-fold.c
+++ src/gcc/gimple-fold.c
@@ -1381,51 +1381,6 @@ gimple_fold_builtin (gimple stmt)
   return result;
 }
 
-/* Return a declaration of a function which an OBJ_TYPE_REF references. TOKEN
-   is integer form of OBJ_TYPE_REF_TOKEN of the reference expression.
-   KNOWN_BINFO carries the binfo describing the true type of
-   OBJ_TYPE_REF_OBJECT(REF).  If a call to the function must be accompanied
-   with a this adjustment, the constant which should be added to this pointer
-   is stored to *DELTA.  If REFUSE_THUNKS is true, return NULL if the function
-   is a thunk (other than a this adjustment which is dealt with by DELTA). */
-
-tree
-gimple_get_virt_method_for_binfo (HOST_WIDE_INT token, tree known_binfo,
- tree *delta)
-{
-  HOST_WIDE_INT i;
-  tree v, fndecl;
-
-  v = BINFO_VIRTUALS (known_binfo);
-  /* If there is no virtual methods leave the OBJ_TYPE_REF alone.  */
-  if (!v)
-return NULL_TREE;
-  i = 0;

Re: [libiberty patch] Add demangler support for cloned function symbols (PR 40831)

[Cary Coutant]

Ping?

http://gcc.gnu.org/ml/gcc-patches/2011-08/msg01626.html

-cary

On Fri, Aug 19, 2011 at 5:57 PM, Cary Coutant  wrote:
>> OK, thanks. Dmitry G. also commented that the patch does not work "for
>> `_Z3fooi.1988' or `_Z3fooi.part.9.165493.constprop.775.31805'."
>> Apparently, there can be multiple numeric suffixes, and a cloned
>> function can be cloned again. Is it worth trying to identify the kinds
>> of cloning in the demangled name, or should I just look for a generic
>> pattern instead?
>
> Here's an updated patch that generalizes the clone suffix pattern recognition.
>
> Does this look OK for trunk?
>
> I've bootstrapped and regression tested on x86_64.
>
> -cary
>
>
> 2011-08-19   Cary Coutant  
>
>        * include/demangle.h (enum demangle_component_type):
>        * libiberty/cp-demangle.c (struct d_info):
>        (CP_STATIC_IF_GLIBCPP_V3):
>        (struct d_print_info):
>        * libiberty/testsuite/demangle-expected (DFA):
>
> include/ChangeLog:
>
>        PR 40831
>        * demangle.h (enum demangle_component_type): Add
>        DEMANGLE_COMPONENT_CLONE.
>
> libiberty/ChangeLog:
>
>        PR 40831
>        * cp-demangle.c (d_make_comp): Add new component type.
>        (cplus_demangle_mangled_name): Check for clone suffixes.
>        (d_parmlist): Don't error out if we see '.'.
>        (d_clone_suffix): New function.
>        (d_print_comp): Print info for clone suffixes.
>        * testsuite/demangle-expected: Add new testcases.
>

Re: Add unwind information to mips epilogues

[Richard Sandiford]

Bernd Schmidt  writes:
> On 08/31/11 20:43, Richard Sandiford wrote:
>> Bernd Schmidt  writes:
>>> This is necessary when adding shrink-wrapping; otherwise dwarf2cfi sees
>>> inconsistent information and aborts.
>>>
>>> Tested on mips64-elf together with the rest of the shrink-wrapping
>>> patches. Ok?
>> 
>> It looks like the current code doesn't handle the RESTORE instruction.
>> Could you also test that somehow?  A mipsisa32-elf run with -mips16
>> ought to work
>
> Hmm, I'm having some trouble with that.
>  * mipsisa32-elf doesn't build on trunk
>  * 4.6 doesn't allow plain "-mips16"; I have to add "-msoft-float"
>  * With that, 4.6 produces rather a lot of testsuite failures.
>(>400 execute failures in my current run and it's not even gotten
>very far)
>
> Just to make sure - am I missing anything here, or is this stuff just in
> bad shape?

Gah, my bad, sorry.  I'd forgotten mipsisa32-elf is EABI32
rather than o32.  mips-elf with -mips32/-mips16 would test
what I was after.  Or the kind of spot-checks I was thinking
of could be done with any mips* compiler.  Compile some tests
that are interesting for shrinking wrapping with:

-c -mips16 -mips32 -mabi=32 -Owhatever

and see if (a) it compiles, (b) it uses shrink-wrap and RESTORE
and (c) the CFI dump (from readelf -wf or whatever) looks OK.

Thanks for trying though.  I'll look into the mipsisa32-elf build failure
and (if I get time) the EABI32 -mips16/-msoft-float problem.

(For the record, I do test -mips16 fairly regularly, but at a lower
ISA level.  I suppose I should try MIPS16e more myself too)

Richard

Re: [PATCH] Make devirtualization use BINFO_VTABLE instead of BINFO_VIRTUALS

[Jan Hubicka]

> - Nevertheless, this method of devirtualization cannot automatically
>   de-thunkize this-adjusting thunks and newly direct calls to them
>   cannot be inlined because the inliner does not have this capability
>   now.  This is in fact a regression from 4.6, and testcases
>   ivinline-7.C and ivinline-9.C had to be XFAILed exactly for this
>   reason.  The size of the memory savings and the fact that we do not
>   devirtualize that much now make this an acceptable tradeoff, though.

OK, the "dethunkization" was ugly anyway.  I guess we will need to add support
for handling thunks in inliner but that can go incrementally.

> 2011-08-31  Martin Jambor  
> 
>   * cgraph.h (cgraph_indirect_call_info): Removed field thunk_delta.
>   * gimple-fold.c (gimple_get_virt_method_for_binfo): Rewritten to use
>   BINFO_VTABLE.  Parameter delta removed, all callers updated.
>   * tree.c (free_lang_data_in_binfo): Clear BINFO_VIRTUALs instead
>   BINFO_VTABLE.
>   * cgraph.c (cgraph_make_edge_direct): Removed parameter delta, updated
>   all calls.
>   * cgraphunit.c (cgraph_redirect_edge_call_stmt_to_callee): Removed
>   handling of thunk_delta.
>   * ipa-cp.c (get_indirect_edge_target): Removed parameter delta.
>   (devirtualization_time_bonus): Do not handle thunk deltas.
>   (ipcp_discover_new_direct_edges): Likewise.
>   * ipa-prop.c (ipa_make_edge_direct_to_target): Likewise.
>   (try_make_edge_direct_simple_call): Likewise.
>   (try_make_edge_direct_virtual_call): Likewise.
>   * lto-cgraph.c (output_cgraph_opt_summary_p): Likewise.  Mark
>   parameter set as unused.
>   (output_edge_opt_summary): Likewise.  Mark both parameters as unused.
>   * lto-cgraph.c (output_cgraph_opt_summary_p): Likewise.  Mark
>   parameter set as unused.
>   (output_edge_opt_summary): Likewise.  Mark both parameters as unused.
>   (input_edge_opt_summary): Likewise.
>   * lto-streamer-out.c (lto_output_ts_binfo_tree_pointers): Do not stream
>   BINFO_VIRTUALS at all.
>   * lto-streamer-in.c (lto_input_ts_binfo_tree_pointers): Likewise.
> 
>   * testsuite/g++.dg/ipa/devirt-3.C: Added a distraction method.
>   * testsuite/g++.dg/ipa/ivinline-7.C: Added a test for direct call
>   discovery, xfailed test for inlining.
>   * testsuite/g++.dg/ipa/ivinline-9.C: Likewise.

The cgraph bits of the patch are OK, but I will leave the gimple-fold bits for 
Richi's approval.
Can't we drop computation of BINFO_VIRTUALS from C++ FE completely now?

Honza

Re: [4.7][google]Support for getting CPU type and feature information at run-time. (issue4893046)

[Sriraman Tallam]

On Sun, Aug 28, 2011 at 5:31 PM, Xinliang David Li  wrote:
> Sri, please add a new api to do cpu_indicator initialization on demand
> to be used in IFUNC context. Perhaps also add some debug check to make
> sure no conflicting cpu model is set.
>
> Ok for google branches for now while the discussion continues.

I made the changes and committed to the google branch for now.

* config/i386/i386-cpuinfo.c: New file.
* config/i386/t-cpuinfo: New file.
* config.host: Add t-cpuinfo to link i386-cpuinfo.o with libgcc

* config/i386/i386.c (build_struct_with_one_bit_fields): New function.
(make_var_decl): New function.
(get_field_from_struct): New function.
(fold_builtin_target): New function.
(ix86_fold_builtin): New function.
(ix86_expand_builtin): Expand new builtins by folding them.
(TARGET_FOLD_BUILTIN): New macro.
(IX86_BUILTIN_CPU_SUPPORTS_CMOV): New enum value.
(IX86_BUILTIN_CPU_SUPPORTS_MMX): New enum value.
(IX86_BUILTIN_CPU_SUPPORTS_POPCOUNT): New enum value.
(IX86_BUILTIN_CPU_SUPPORTS_SSE): New enum value.
(IX86_BUILTIN_CPU_SUPPORTS_SSE2): New enum value.
(IX86_BUILTIN_CPU_SUPPORTS_SSE3): New enum value.
(IX86_BUILTIN_CPU_SUPPORTS_SSSE3): New enum value.
(IX86_BUILTIN_CPU_SUPPORTS_SSE4_1): New enum value.
(IX86_BUILTIN_CPU_SUPPORTS_SSE4_2): New enum value.
(IX86_BUILTIN_CPU_INIT): New enum value.
(IX86_BUILTIN_CPU_IS_AMD): New enum value.
(IX86_BUILTIN_CPU_IS_INTEL): New enum value.
(IX86_BUILTIN_CPU_IS_INTEL_ATOM): New enum value.
(IX86_BUILTIN_CPU_IS_INTEL_CORE2): New enum value.
(IX86_BUILTIN_CPU_IS_INTEL_COREI7_NEHALEM): New enum value.
(IX86_BUILTIN_CPU_IS_INTEL_COREI7_WESTMERE): New enum value.
(IX86_BUILTIN_CPU_IS_INTEL_COREI7_SANDYBRIDGE): New enum value.
(IX86_BUILTIN_CPU_IS_AMDFAM10_BARCELONA): New enum value.
(IX86_BUILTIN_CPU_IS_AMDFAM10_SHANGHAI): New enum value.
(IX86_BUILTIN_CPU_IS_AMDFAM10_ISTANBUL): New enum value.
* config/i386/libgcc-glibc.ver (__cpu_indicator_init): Export symbol.
(__cpu_model): Export symbol.
(__cpu_features): Export symbol.
* config/i386/i386-builtin-types.def: New function type.

* gcc.dg/builtin_target.c: New test.


Thanks,
-Sri.

>
> thanks,
>
> David
>
> On Thu, Aug 25, 2011 at 5:37 PM, Sriraman Tallam  wrote:
>> Hi,
>>
>>  Thanks for all the comments. I am attaching a new patch
>> incorporating all of the changes mentioned, mainly :
>>
>> 1) Make __cpu_indicator_init a constructor in libgcc and guard to call
>> it only once.
>> 2) Add symbol versions.
>> 3) Move all builtins to the i386 port.
>> 4) Add check for atom processor.
>> 5) No separate passes to fold the builtins.
>>
>> Please let me know what you think.
>> Thanks,
>> -Sri.
>>
>>        * config/i386/i386.c (build_struct_with_one_bit_fields): New function.
>>        (make_var_decl): New function.
>>        (get_field_from_struct): New function.
>>        (fold_builtin_target): New function.
>>        (ix86_fold_builtin): New function.
>>        (ix86_expand_builtin): Expand new builtins by folding them.
>>        (TARGET_FOLD_BUILTIN): New macro.
>>        (IX86_BUILTIN_CPU_SUPPORTS_CMOV): New enum value.
>>        (IX86_BUILTIN_CPU_SUPPORTS_MMX): New enum value.
>>        (IX86_BUILTIN_CPU_SUPPORTS_POPCOUNT): New enum value.
>>        (IX86_BUILTIN_CPU_SUPPORTS_SSE): New enum value.
>>        (IX86_BUILTIN_CPU_SUPPORTS_SSE2): New enum value.
>>        (IX86_BUILTIN_CPU_SUPPORTS_SSE3): New enum value.
>>        (IX86_BUILTIN_CPU_SUPPORTS_SSSE3): New enum value.
>>        (IX86_BUILTIN_CPU_SUPPORTS_SSE4_1): New enum value.
>>        (IX86_BUILTIN_CPU_SUPPORTS_SSE4_2): New enum value.
>>        (IX86_BUILTIN_CPU_IS_AMD): New enum value.
>>        (IX86_BUILTIN_CPU_IS_INTEL): New enum value.
>>        (IX86_BUILTIN_CPU_IS_INTEL_ATOM): New enum value.
>>        (IX86_BUILTIN_CPU_IS_INTEL_CORE2): New enum value.
>>        (IX86_BUILTIN_CPU_IS_INTEL_COREI7_NEHALEM): New enum value.
>>        (IX86_BUILTIN_CPU_IS_INTEL_COREI7_WESTMERE): New enum value.
>>        (IX86_BUILTIN_CPU_IS_INTEL_COREI7_SANDYBRIDGE): New enum value.
>>        (IX86_BUILTIN_CPU_IS_AMDFAM10_BARCELONA): New enum value.
>>        (IX86_BUILTIN_CPU_IS_AMDFAM10_SHANGHAI): New enum value.
>>        (IX86_BUILTIN_CPU_IS_AMDFAM10_ISTANBUL): New enum value.
>>        * config/i386/libgcc-glibc.ver (__cpu_indicator_init): Export symbol.
>>        (__cpu_model): Export symbol.
>>        (__cpu_features): Export symbol.
>>        * config/i386/i386-builtin-types.def: New function type.
>>
>>        * config/i386/i386-cpuinfo.c: New file.
>>        * config/i386/t-cpuinfo: New file.
>>        * config.host: Add t-cpuinfo to link i386-cpuinfo.o with libgcc
>>
>>        * gcc.dg/builtin_target.c: New test.
>>
>>
>>
>>
>> On Tue, Aug 23, 2011 at 4:35 AM, Michael Matz  wro

Re: Add unwind information to mips epilogues

[Richard Sandiford]

Richard Sandiford  writes:
> Gah, my bad, sorry.  I'd forgotten mipsisa32-elf is EABI32
> rather than o32.  mips-elf with -mips32/-mips16 would test
> what I was after.

Sigh.  Obviously not my day.  I just remembered that the default
mips-elf simulator won't accept -mips32 instructions, so you need
to play some tricks.  Could you just try some of the spot checks
instead?  I'll test GNU/Linux on qemu once everything is in.

Richard

Re: Add unwind information to mips epilogues

[Bernd Schmidt]

On 09/01/11 21:19, Richard Sandiford wrote:
> Richard Sandiford  writes:
>> Gah, my bad, sorry.  I'd forgotten mipsisa32-elf is EABI32
>> rather than o32.  mips-elf with -mips32/-mips16 would test
>> what I was after.
> 
> Sigh.  Obviously not my day.  I just remembered that the default
> mips-elf simulator won't accept -mips32 instructions, so you need
> to play some tricks.  Could you just try some of the spot checks
> instead?  I'll test GNU/Linux on qemu once everything is in.

I seem to have made some progress getting 4.6 mipsisa32-elf to accept
"-mips16 -msoft-float", now with only 287 unexpected failures. The
-mips32 case shouldn't be too different from mips64 testing, should it?


Bernd

Re: Add unwind information to mips epilogues

[Richard Sandiford]

Bernd Schmidt  writes:
> On 09/01/11 21:19, Richard Sandiford wrote:
>> Richard Sandiford  writes:
>>> Gah, my bad, sorry.  I'd forgotten mipsisa32-elf is EABI32
>>> rather than o32.  mips-elf with -mips32/-mips16 would test
>>> what I was after.
>> 
>> Sigh.  Obviously not my day.  I just remembered that the default
>> mips-elf simulator won't accept -mips32 instructions, so you need
>> to play some tricks.  Could you just try some of the spot checks
>> instead?  I'll test GNU/Linux on qemu once everything is in.
>
> I seem to have made some progress getting 4.6 mipsisa32-elf to accept
> "-mips16 -msoft-float", now with only 287 unexpected failures.

Unfortunately it'll be the wrong ABI though.  SAVE and RESTORE
are very much geared to o32 and won't be used for EABI32.
mipsisa32-elfoabi would be OK though.

Sorry again for messing this up...

Richard

[Patch, Fortran, OOP] PR 50227: [4.7 Regression] ICE-on-valid with allocatable class variable

[Janus Weil]

Hi all,

here is a small patch fixing a recent OOP regression. It feels a bit
like it's only fixing the effect instead of the cause (since I haven't
really found the cause). But anyway, it does fix the ICE and it is
obvious enough so that I'll commit it by Sunday if no one protests in
the meantime.

Regtested on x86_64-unknown-linux-gnu.

Cheers,
Janus


2011-09-01  Janus Weil  

PR fortran/50227
* trans-types.c (gfc_sym_type): Check for proc_name.

2011-09-01  Janus Weil  

PR fortran/50227
* gfortran.dg/class_45a.f03: New.
* gfortran.dg/class_45b.f03: New.
Index: gcc/fortran/trans-types.c
===
--- gcc/fortran/trans-types.c	(revision 178408)
+++ gcc/fortran/trans-types.c	(working copy)
@@ -2109,7 +2109,8 @@ gfc_sym_type (gfc_symbol * sym)
 {
   /* We must use pointer types for potentially absent variables.  The
 	 optimizers assume a reference type argument is never NULL.  */
-  if (sym->attr.optional || sym->ns->proc_name->attr.entry_master)
+  if (sym->attr.optional
+	  || (sym->ns->proc_name && sym->ns->proc_name->attr.entry_master))
 	type = build_pointer_type (type);
   else
 	{


class_45a.f03
Description: Binary data


class_45b.f03
Description: Binary data

[Patch, C] options generation and language count

[Gary Funck]

2011-09-01  Gary Funck 

* opts.c (print_specific_help): Fix off-by-one compare in
assertion check.
* opts.h (CL_PARAMS, CL_WARNING, CL_OPTIMIZATION, CL_DRIVER,
CL_TARGET, CL_COMMON, CL_JOINED, CL_SEPARATE, CL_UNDOCUMENTED):
Increase by +5 to allow for more languages.
* Makefile.in (options.c): Extract max. number of languages value
from opts.h, and pass to optc-gen.awk script.
* optc-gen.awk: Use max_lang value and issue error if number of
languages exceeds implementation-defined limit.

This patch extracts the shift count used in the definition of
CL_PARAMS to determine the maximum number of language supported
by the implementation.  optc-gen.awk implements the check and
will exit with an error if the number of languages exceeds the limit.

Patch, attached.  Please review.

Thanks,

- Gary
Index: gcc/ChangeLog
===
--- gcc/ChangeLog   (revision 178389)
+++ gcc/ChangeLog   (working copy)
@@ -1,3 +1,15 @@
+2011-09-01  Gary Funck 
+
+   * opts.c (print_specific_help): Fix off-by-one compare in
+   assertion check.
+   * opts.h (CL_PARAMS, CL_WARNING, CL_OPTIMIZATION, CL_DRIVER,
+   CL_TARGET, CL_COMMON, CL_JOINED, CL_SEPARATE, CL_UNDOCUMENTED):
+   Increase by +5 to allow for more languages.
+   * Makefile.in (options.c): Extract max. number of languages value
+   from opts.h, and pass to optc-gen.awk script.
+   * optc-gen.awk: Use max_lang value and issue error if number of
+   languages exceeds implementation-defined limit.
+
 2011-08-31  Richard Sandiford  
 
* config/i386/i386.md: Use (match_test ...) for attribute tests.
Index: gcc/opts.c
===
--- gcc/opts.c  (revision 178389)
+++ gcc/opts.c  (working copy)
@@ -1125,7 +1125,7 @@ print_specific_help (unsigned int includ
 
   /* Sanity check: Make sure that we do not have more
  languages than we have bits available to enumerate them.  */
-  gcc_assert ((1U << cl_lang_count) < CL_MIN_OPTION_CLASS);
+  gcc_assert ((1U << cl_lang_count) <= CL_MIN_OPTION_CLASS);
 
   /* If we have not done so already, obtain
  the desired maximum width of the output.  */
Index: gcc/opts.h
===
--- gcc/opts.h  (revision 178389)
+++ gcc/opts.h  (working copy)
@@ -127,12 +127,12 @@ extern const unsigned int cl_options_cou
 extern const char *const lang_names[];
 extern const unsigned int cl_lang_count;
 
-#define CL_PARAMS   (1U << 11) /* Fake entry.  Used to display 
--param info with --help.  */
-#define CL_WARNING (1U << 12) /* Enables an (optional) warning 
message.  */
-#define CL_OPTIMIZATION(1U << 13) /* Enables an (optional) 
optimization.  */
-#define CL_DRIVER  (1U << 14) /* Driver option.  */
-#define CL_TARGET  (1U << 15) /* Target-specific option.  */
-#define CL_COMMON  (1U << 16) /* Language-independent.  */
+#define CL_PARAMS   (1U << 16) /* Fake entry.  Used to display 
--param info with --help.  */
+#define CL_WARNING (1U << 17) /* Enables an (optional) warning 
message.  */
+#define CL_OPTIMIZATION(1U << 18) /* Enables an (optional) 
optimization.  */
+#define CL_DRIVER  (1U << 19) /* Driver option.  */
+#define CL_TARGET  (1U << 20) /* Target-specific option.  */
+#define CL_COMMON  (1U << 21) /* Language-independent.  */
 
 #define CL_MIN_OPTION_CLASSCL_PARAMS
 #define CL_MAX_OPTION_CLASSCL_COMMON
@@ -142,9 +142,9 @@ extern const unsigned int cl_lang_count;
This distinction is important because --help will not list options
which only have these higher bits set.  */
 
-#define CL_JOINED  (1U << 17) /* If takes joined argument.  */
-#define CL_SEPARATE(1U << 18) /* If takes a separate argument.  */
-#define CL_UNDOCUMENTED(1U << 19) /* Do not output with 
--help.  */
+#define CL_JOINED  (1U << 22) /* If takes joined argument.  */
+#define CL_SEPARATE(1U << 23) /* If takes a separate argument.  */
+#define CL_UNDOCUMENTED(1U << 24) /* Do not output with 
--help.  */
 
 /* Flags for an enumerated option argument.  */
 #define CL_ENUM_CANONICAL  (1 << 0) /* Canonical for this value.  */
Index: gcc/optc-gen.awk
===
--- gcc/optc-gen.awk(revision 178389)
+++ gcc/optc-gen.awk(working copy)
@@ -30,6 +30,15 @@
 
 # Dump that array of options into a C file.
 END {
+# MAX_LANG is the maximum number of languages that can be defined.
+# Its value is extracted from the value of CL_PARAMS in opts.h
+# and is passed on the command line as '-v max_lang=...'.
+if (n_langs > max_lang) {
+  print "Error: the number of defined languages (" n_l

[Ada] mutex attributes not initialized in the linux run-time library

[Arnaud Charlet]

Before this patch, the mutex attribute was used (by Initialize_TCB from
the environment task) before it was initialized later by Initialize. This
patch creates and initializes mutex and condition variable attributes locally
when needed to avoid this problem.

The following test case should compile and execute without producing any
output when recompiling the run-time library.

gnatmake -f -q -gnato -a -gnatws initialization
---
pragma Normalize_Scalars;

procedure Initialization is
   task T;

   task body T is
   begin
  null;
   end T;
begin
   null;
end Initialization;

Tested on x86_64-pc-linux-gnu, committed on trunk

2011-09-02  Jose Ruiz  

* s-taprop-linux.adb (Initialize_Lock, Initialize_TCB,
Initialize): Define and initialize the
mutex attributes and condition variable attributes locally.

Index: s-taprop-linux.adb
===
--- s-taprop-linux.adb  (revision 178418)
+++ s-taprop-linux.adb  (working copy)
@@ -97,12 +97,6 @@
Dispatching_Policy : Character;
pragma Import (C, Dispatching_Policy, "__gl_task_dispatching_policy");
 
-   --  The following are effectively constants, but they need to be initialized
-   --  by calling a pthread_ function.
-
-   Mutex_Attr   : aliased pthread_mutexattr_t;
-   Cond_Attr: aliased pthread_condattr_t;
-
Foreign_Task_Elaborated : aliased Boolean := True;
--  Used to identified fake tasks (i.e., non-Ada Threads)
 
@@ -261,9 +255,13 @@
is
   pragma Unreferenced (Prio);
 
-  Result : Interfaces.C.int;
+  Mutex_Attr : aliased pthread_mutexattr_t;
+  Result : Interfaces.C.int;
 
begin
+  Result := pthread_mutexattr_init (Mutex_Attr'Access);
+  pragma Assert (Result = 0);
+
   Result := pthread_mutex_init (L, Mutex_Attr'Access);
 
   pragma Assert (Result = 0 or else Result = ENOMEM);
@@ -279,9 +277,13 @@
is
   pragma Unreferenced (Level);
 
-  Result : Interfaces.C.int;
+  Mutex_Attr : aliased pthread_mutexattr_t;
+  Result : Interfaces.C.int;
 
begin
+  Result := pthread_mutexattr_init (Mutex_Attr'Access);
+  pragma Assert (Result = 0);
+
   Result := pthread_mutex_init (L, Mutex_Attr'Access);
 
   pragma Assert (Result = 0 or else Result = ENOMEM);
@@ -762,7 +764,9 @@

 
procedure Initialize_TCB (Self_ID : Task_Id; Succeeded : out Boolean) is
-  Result : Interfaces.C.int;
+  Mutex_Attr : aliased pthread_mutexattr_t;
+  Cond_Attr  : aliased pthread_condattr_t;
+  Result : Interfaces.C.int;
 
begin
   --  Give the task a unique serial number
@@ -774,6 +778,9 @@
   Self_ID.Common.LL.Thread := Null_Thread_Id;
 
   if not Single_Lock then
+ Result := pthread_mutexattr_init (Mutex_Attr'Access);
+ pragma Assert (Result = 0);
+
  Result := pthread_mutex_init (Self_ID.Common.LL.L'Access,
Mutex_Attr'Access);
  pragma Assert (Result = 0 or else Result = ENOMEM);
@@ -784,6 +791,9 @@
  end if;
   end if;
 
+  Result := pthread_condattr_init (Cond_Attr'Access);
+  pragma Assert (Result = 0);
+
   Result := pthread_cond_init (Self_ID.Common.LL.CV'Access,
 Cond_Attr'Access);
   pragma Assert (Result = 0 or else Result = ENOMEM);
@@ -1027,7 +1037,9 @@

 
procedure Initialize (S : in out Suspension_Object) is
-  Result : Interfaces.C.int;
+  Mutex_Attr : aliased pthread_mutexattr_t;
+  Cond_Attr  : aliased pthread_condattr_t;
+  Result : Interfaces.C.int;
 
begin
   --  Initialize internal state (always to False (RM D.10(6)))
@@ -1037,6 +1049,9 @@
 
   --  Initialize internal mutex
 
+  Result := pthread_mutexattr_init (Mutex_Attr'Access);
+  pragma Assert (Result = 0);
+
   Result := pthread_mutex_init (S.L'Access, Mutex_Attr'Access);
 
   pragma Assert (Result = 0 or else Result = ENOMEM);
@@ -1047,6 +1062,9 @@
 
   --  Initialize internal condition variable
 
+  Result := pthread_condattr_init (Cond_Attr'Access);
+  pragma Assert (Result = 0);
+
   Result := pthread_cond_init (S.CV'Access, Cond_Attr'Access);
 
   pragma Assert (Result = 0 or else Result = ENOMEM);
@@ -1340,12 +1358,6 @@
  end if;
   end loop;
 
-  Result := pthread_mutexattr_init (Mutex_Attr'Access);
-  pragma Assert (Result = 0);
-
-  Result := pthread_condattr_init (Cond_Attr'Access);
-  pragma Assert (Result = 0);
-
   Initialize_Lock (Single_RTS_Lock'Access, RTS_Lock_Level);
 
   --  Initialize the global RTS lock

[Ada] Take into account unresolved calls in Alfa mode

[Arnaud Charlet]

In Alfa mode, protected subprogram calls are not expanded, so that unresolved
calls must be allowed in a place where they should not be allowed in normal
mode.

Tested on x86_64-pc-linux-gnu, committed on trunk

2011-09-02  Yannick Moy  

* exp_ch6.adb (Is_Build_In_Place_Function_Call): in Alfa mode, allow
unresolved calls.

Index: exp_ch6.adb
===
--- exp_ch6.adb (revision 178412)
+++ exp_ch6.adb (working copy)
@@ -6084,7 +6084,7 @@
 
  Build_Protected_Subprogram_Call (N,
Name => New_Occurrence_Of (Subp, Sloc (N)),
-   Rec  =>  Convert_Concurrent (Rec, Etype (Rec)),
+   Rec  => Convert_Concurrent (Rec, Etype (Rec)),
External => True);
 
   else
@@ -6797,6 +6797,16 @@
 
  elsif Nkind (Name (Exp_Node)) = N_Explicit_Dereference then
 Function_Id := Etype (Name (Exp_Node));
+
+ --  In Alfa mode, protected subprogram calls are not expanded, so that
+ --  we may end up with a call that is neither resolved to an entity,
+ --  nor an indirect call.
+
+ elsif Alfa_Mode then
+return False;
+
+ else
+raise Program_Error;
  end if;
 
  return Is_Build_In_Place_Function (Function_Id);

[Ada] Spurious warnings on iterators over subcomponents

[Arnaud Charlet]

Iterator of the form "of" generate renamings for the container elements. If
the container that is the domain of iteration is a subcomponent of a larger
object, and an assignment is performed on the element, the larger object must
be marked as modified as well.

The following must compile quietly:

   gcc -c -gnatwa -gnat12 loop_for_of_false_warning.adb

---
procedure Loop_For_Of_False_Warning is
   type Discri_Type is (Case_A, Case_B);
   type Discri2_Type is (Case2_A, Case2_B);

   type R_Type (D : Discri_Type := Case_A) is record
  N : Integer;

  case D is
 when Case_A =>
I : Integer;

 when Case_B =>
F : Float;
  end case;
   end record;

   type T_Type is array (Positive range 1 .. 10) of R_Type;

   type R2_Type (D : Discri2_Type := Case2_A) is record
  case D is
 when Case2_A =>
T_A : T_Type;

 when Case2_B =>
T_B : T_Type;
  end case;
   end record;

   procedure Process (V : in out R2_Type)
   is
   begin
  if V.D = Case2_B then
 for Element of V.T_B
 loop
Element.N := 10;
 end loop;
   end Process;

   X : R2_Type;
begin
   Process (X);
end Loop_For_Of_False_Warning;

Tested on x86_64-pc-linux-gnu, committed on trunk

2011-09-02  Ed Schonberg  

* sem_util.adb (Note_Possible_Modification): If the entity
being modified is the renaming generated for an Ada2012 iterator
element, the enclosing container or array is modified as well.

Index: sem_util.adb
===
--- sem_util.adb(revision 178381)
+++ sem_util.adb(working copy)
@@ -10711,6 +10711,18 @@
then
   Exp := Renamed_Object (Ent);
   goto Continue;
+
+   --  The expression may be the renaming of a subcomponent of an
+   --  array or container. The assignment to the subcomponent is
+   --  a modification of the container.
+
+   elsif Comes_From_Source (Original_Node (Exp))
+ and then
+   Nkind_In (Original_Node (Exp),
+ N_Selected_Component, N_Indexed_Component)
+   then
+  Exp := Prefix (Original_Node (Exp));
+  goto Continue;
end if;
 
--  Generate a reference only if the assignment comes from