Improve Jump Threading 3 of N

[Jeff Law]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1


Found by inspection while working on another threading patch.


DOM tracks equivalences in a stack so that it can restore the state of
the known equivalences as it completes processing a block in the
dominator tree.

The threading code also utilizes this stack so that it can temporarily
record equivalences as it threads through the cfg successors of leafs in
the dominator tree.

The threading code removes the equivalences it records through the
remove_temporary_equivalences call.  However, in the case where we
successfully thread through a block, we end up calling
remove_temporary_equivalences twice from the threading code.  This
results in removing too many equivalences and ultimately missed
optimizations.

Ultimately it's a missing return statement.  This has gone unnoticed
since I introduced the problem in gcc-4.2; of course the only symptom of
this problem is missed optimizations.

Bootstrapped, regression tested on x86_64-unknown-linux-gnu.  Installed
as obvious.


-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJNu4Y3AAoJEBRtltQi2kC7XKQIAJrS/agWlbPkKaUoA6I9wZmA
WSI/k8UrLhnm9QPgAGislM7j+Z3v8kURnIx1zvA/aCrO2eDAOGUhqi16q8x9n4O8
pExfhRoDRgaZhBPofC8cQ4ruW2PSGgKOUkVzsHHX8sA2Sr8lC8yVnHeBc0kxBPva
m9ygcf5Cm/1egXoEhG1m1Egwvu1dv83XUP0JhZJs80GQkCdHO4dWQxowGnTwtJh1
/yP2nYOEwbUblPr/FvD9kAheOFeU4+qktGi7NAhXL39Q9wVb//ma3aW5g2tAfvsa
+Yc6w3vRKmXH0NfVfiCPk9QPSNf4tcl7hEa5+4z9aQwgez1u6cqPZpavlDLP9Gc=
=85sA
-END PGP SIGNATURE-
* tree-ssa-threadedge.c (thread_across_edge): Add missing return.

* gcc.dg/tree-ssa/ssa-dom-thread-4.c: New test.

Index: testsuite/gcc.dg/tree-ssa/ssa-dom-thread-4.c
===
*** testsuite/gcc.dg/tree-ssa/ssa-dom-thread-4.c(revision 0)
--- testsuite/gcc.dg/tree-ssa/ssa-dom-thread-4.c(revision 0)
***
*** 0 
--- 1,63 
+ /* { dg-do compile } */ 
+ /* { dg-options "-O2 -fdump-tree-dom1-details" } */
+ struct bitmap_head_def;
+ typedef struct bitmap_head_def *bitmap;
+ typedef const struct bitmap_head_def *const_bitmap;
+ typedef unsigned long BITMAP_WORD;
+ typedef struct bitmap_element_def
+ {
+   struct bitmap_element_def *next;
+   unsigned int indx;
+ } bitmap_element;
+ 
+ 
+ 
+ 
+ 
+ 
+ 
+ 
+ 
+ unsigned char
+ bitmap_ior_and_compl (bitmap dst, const_bitmap a, const_bitmap b,
+ const_bitmap kill)
+ {
+   unsigned char changed = 0;
+ 
+   bitmap_element *dst_elt;
+   const bitmap_element *a_elt, *b_elt, *kill_elt, *dst_prev;
+ 
+   while (a_elt || b_elt)
+ {
+   unsigned char new_element = 0;
+ 
+   if (b_elt)
+   while (kill_elt && kill_elt->indx < b_elt->indx)
+ kill_elt = kill_elt->next;
+ 
+   if (b_elt && kill_elt && kill_elt->indx == b_elt->indx
+ && (!a_elt || a_elt->indx >= b_elt->indx))
+   {
+ bitmap_element tmp_elt;
+ unsigned ix;
+ 
+ BITMAP_WORD ior = 0;
+ 
+ changed = bitmap_elt_ior (dst, dst_elt, dst_prev,
+   a_elt, &tmp_elt, changed);
+ 
+   }
+ 
+ }
+ 
+ 
+   return changed;
+ }
+ /* The block starting the second conditional has  3 incoming edges,
+we should thread all three, but due to a bug in the threading
+code we missed the edge when the first conditional is false
+(b_elt is zero, which means the second conditional is always
+zero.  */
+ /* { dg-final { scan-tree-dump-times "Threaded" 3 "dom1"} } */
+ /* { dg-final { cleanup-tree-dump "dom1" } } */
+ 
Index: tree-ssa-threadedge.c
===
*** tree-ssa-threadedge.c   (revision 173175)
--- tree-ssa-threadedge.c   (working copy)
*** thread_across_edge (gimple dummy_cond,
*** 771,776 
--- 771,777 
  
  remove_temporary_equivalences (stack);
  register_jump_thread (e, taken_edge);
+ return;
}
  }
  

Re: [patch gimplifier]: Do folding on truth and/or trees

[Hans-Peter Nilsson]

On Wed, 27 Apr 2011, Jakub Jelinek wrote:
> You don't need to build cross binutils, all that is needed is
> configure the cross and build just cc1, don't mind that the build
> fails afterwards and just run it on your testcases by hand to see
> what is in the dumps.

FWIW "make all-gcc" doesn't fail these days (with libgcc at
toplevel); xgcc and cc1 are built the way you want them, yay.
(With reservations for autoconf:ed thingies that can't be tested
without an assembler/linker and thus are defaulted.)

brgds, H-P

Re: [patch] Split Parse Timevar (rev 2) (issue4433076)

[Jason Merrill]

OK.

Jason

[patch, libgfortran] PR48767 Rounding Up followup patch

[Jerry DeLisle]

Hi,

The attached patch does some cleanup and a check for trailing zeros to decide 
whether or not to round.


I have added the additional test cases posted on the bugzilla to the existing 
test case round_3.f08.


Regression tested on x86-64.

OK for trunk and then I will back port the whole enchilada to 4.6.1 in a few 
weeks.  Please consider the starting point of the zero scan carefully.  I have 
not convinced myself that the d * p covers all cases, but it works for all cases 
I have tried.


Regards,

Jerry

2011-04-29  Jerry DeLisle  

PR libgfortran/48787
* io/write_float.def (output_float): Gather up integer declarations and
add new 'p' for scale factor. Use 'p' in place of the 'dtp' reference
everywhere. For ROUND_UP scan the digit string and only perform
rounding if something other than '0' is found.
Index: gcc/testsuite/gfortran.dg/round_3.f08
===
--- gcc/testsuite/gfortran.dg/round_3.f08	(revision 173197)
+++ gcc/testsuite/gfortran.dg/round_3.f08	(working copy)
@@ -4,10 +4,17 @@
 program pr48615
 call checkfmt("(RU,F17.0)", 2.5, "   3.")
 call checkfmt("(RU,-1P,F17.1)", 2.5, "  0.3")
-call checkfmt("(RU,E17.1)", 2.5, "  0.3E+01") ! 0.2E+01
+call checkfmt("(RU,E17.1)", 2.5, "  0.3E+01")
 call checkfmt("(RU,1P,E17.0)", 2.5,  "   3.E+00")
-call checkfmt("(RU,ES17.0)", 2.5,"   3.E+00") ! 2.E+00
+call checkfmt("(RU,ES17.0)", 2.5,"   3.E+00")
 call checkfmt("(RU,EN17.0)", 2.5,"   3.E+00")
+call checkfmt("(RU,F2.0)",  2.0,  "2.")
+call checkfmt("(RU,F6.4)",  2.0,  "2.")
+call checkfmt("(RU,1P,E6.0E2)", 2.0,  "2.E+00")
+call checkfmt("(RU,1P,E7.1E2)", 2.5,  "2.5E+00")
+call checkfmt("(RU,1P,E10.4E2)", 2.5,  "2.5000E+00")
+call checkfmt("(RU,1P,G6.0E2)", 2.0,  "2.E+00")
+call checkfmt("(RU,1P,G10.4E2)", 2.3456e5,  "2.3456E+05")
 
 call checkfmt("(RD,F17.0)", 2.5, "   2.")
 call checkfmt("(RD,-1P,F17.1)", 2.5, "  0.2")
@@ -18,9 +25,9 @@ program pr48615
 
 call checkfmt("(RC,F17.0)", 2.5, "   3.")
 call checkfmt("(RC,-1P,F17.1)", 2.5, "  0.3")
-call checkfmt("(RC,E17.1)", 2.5, "  0.3E+01") ! 0.2E+01
+call checkfmt("(RC,E17.1)", 2.5, "  0.3E+01")
 call checkfmt("(RC,1P,E17.0)", 2.5,  "   3.E+00")
-call checkfmt("(RC,ES17.0)", 2.5,"   3.E+00") ! 2.E+00
+call checkfmt("(RC,ES17.0)", 2.5,"   3.E+00")
 call checkfmt("(RC,EN17.0)", 2.5,"   3.E+00")
 
 call checkfmt("(RN,F17.0)", 2.5, "   2.")
@@ -53,20 +60,20 @@ program pr48615
 
 call checkfmt("(RC,F17.0)", -2.5, "  -3.")
 call checkfmt("(RC,-1P,F17.1)", -2.5, " -0.3")
-call checkfmt("(RC,E17.1)", -2.5, " -0.3E+01") ! -0.2E+01
+call checkfmt("(RC,E17.1)", -2.5, " -0.3E+01")
 call checkfmt("(RC,1P,E17.0)", -2.5,  "  -3.E+00")
-call checkfmt("(RC,ES17.0)", -2.5,"  -3.E+00") ! -2.E+00
+call checkfmt("(RC,ES17.0)", -2.5,"  -3.E+00")
 call checkfmt("(RC,EN17.0)", -2.5,"  -3.E+00")
 
-call checkfmt("(RU,E17.1)", nearest(2.0, 1.0), "  0.3E+01") ! 0.2E+01
-call checkfmt("(RD,E17.1)", nearest(3.0, -1.0),"  0.2E+01") ! 0.3E+01
+call checkfmt("(RU,E17.1)", nearest(2.0, 1.0), "  0.3E+01")
+call checkfmt("(RD,E17.1)", nearest(3.0, -1.0),"  0.2E+01")
 
 contains
 subroutine checkfmt(fmt, x, cmp)
 character(len=*), intent(in) :: fmt
 real, intent(in) :: x
 character(len=*), intent(in) :: cmp
-character(len=40) :: s
+character(len=20) :: s
 
 write(s, fmt) x
 if (s /= cmp) call abort
Index: libgfortran/io/write_float.def
===
--- libgfortran/io/write_float.def	(revision 173197)
+++ libgfortran/io/write_float.def	(working copy)
@@ -67,11 +67,9 @@ output_float (st_parameter_dt *dtp, const fnode *f
 {
   char *out;
   char *digits;
-  int e;
+  int e, w, d, p, i;
   char expchar, rchar;
   format_token ft;
-  int w;
-  int d;
   /* Number of digits before the decimal point.  */
   int nbefore;
   /* Number of zeros after the decimal point.  */
@@ -82,12 +80,12 @@ output_float (st_parameter_dt *dtp, const fnode *f
   int nzero_real;
   int leadzero;
   int nblanks;
-  int i;
   sign_t sign;
 
   ft = f->format;
   w = f->u.real.w;
   d = f->u.real.d;
+  p = dtp->u.p.scale_factor;
 
   rchar = '5';
   nzero_real = -1;
@@ -119,14 +117,14 @@ output_float (st_parameter_dt *dtp, const fnode *f
   switch (ft)
 {
 case FMT_F:
-  if (d == 0 && e <= 0 && dtp->u.p.scale_factor == 0)
+  if (d == 0 && e <= 0 && 

Re: [google][patch] Add new -gmlt option for min. debug info with line tables (issue4440072)

[Cary Coutant]

>> OK for google/main?
>
> Yes.

Thanks, committed to google/main.

-cary

[google]: add a wrong dce regression test case

[Xinliang David Li]

The following test makes sure compiler does not wrongly eliminate
store to ireg.ax.

Ok for trunk?

David


2011-04-29  Xinliang David Li  

* gcc.dg/tree-ssa/alias_bug.c: New test.
Index: testsuite/gcc.dg/tree-ssa/alias_bug.c
===
--- testsuite/gcc.dg/tree-ssa/alias_bug.c	(revision 0)
+++ testsuite/gcc.dg/tree-ssa/alias_bug.c	(revision 0)
@@ -0,0 +1,61 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fno-strict-aliasing -fdump-tree-optimized" } */
+
+typedef unsigned u32;
+typedef unsigned short u16;
+typedef unsigned char u8;
+struct biosregs {
+ union {
+  struct {
+   u32 edi;
+   u32 esi;
+   u32 ebp;
+   u32 _esp;
+   u32 ebx;
+   u32 edx;
+   u32 ecx;
+   u32 eax;
+   u32 _fsgs;
+   u32 _dses;
+   u32 eflags;
+  };
+  struct {
+   u16 di, hdi;
+   u16 si, hsi;
+   u16 bp, hbp;
+   u16 _sp, _hsp;
+   u16 bx, hbx;
+   u16 dx, hdx;
+   u16 cx, hcx;
+   u16 ax, hax;
+   u16 gs, fs;
+   u16 es, ds;
+   u16 flags, hflags;
+  };
+  struct {
+   u8 dil, dih, edi2, edi3;
+   u8 sil, sih, esi2, esi3;
+   u8 bpl, bph, ebp2, ebp3;
+   u8 _spl, _sph, _esp2, _esp3;
+   u8 bl, bh, ebx2, ebx3;
+   u8 dl, dh, edx2, edx3;
+   u8 cl, ch, ecx2, ecx3;
+   u8 al, ah, eax2, eax3;
+  };
+ };
+};
+void initregs(struct biosregs *regs);
+void intcall(u8 int_no, const struct biosregs *ireg, struct biosregs *oreg);
+static u32 *const gp = (u32*) 0x32;
+void keyboard_set_repeat(void)
+{
+ struct biosregs ireg;
+ *gp = 10;
+ initregs(&ireg);
+ ireg.ax = 0x0305;
+ intcall(0x16, &ireg, ((void *)0));
+}
+
+/* { dg-final { scan-tree-dump-times "ireg.*ax" 1 "optimized"} } */
+ 
+/* { dg-final { cleanup-tree-dump "optimized" } } */

Re: [google][patch] Track discriminators by instruction instead of by basic block (issue4441075)

[Cary Coutant]

>> 2011-04-28  Cary Coutant  
>>
>>        * basic-block.h (struct basic_block_def): Remove discriminator field.
>>        * cfghooks.c (split_block): Remove discriminator field.
>>        * cfglayout.c (insn_discriminator): New function.
>>        * final.c (discriminator): Remove.
>>        (override_discriminator): New file-scope variable.
>>        (final_start_function): Remove tracking of discriminator by basic
>>        block.
>>        (final_scan_insn): Track discriminator by instruction.
>>        (notice_source_line): Check for discriminator override. Get
>>        discriminator from instruction.
>>        * gimple-pretty-print.c (dump_gimple_stmt): Print discriminator.
>>        (dump_bb_header): Don't print discriminator.
>>        * input.c: Include vecprim.h.
>>        (discriminator_location_locations): New variable.
>>        (discriminator_location_discriminators): New variable.
>>        (min_discriminator_location): New variable.
>>        (expand_location): Use map_discriminator_location.
>>        (location_with_discriminator): New function.
>>        (has_discriminator): New function.
>>        (map_discriminator_location): New function.
>>        (get_discriminator_from_locus): New function.
>>        * input.h (location_with_discriminator): New function.
>>        (has_discriminator): New function.
>>        (map_discriminator_location): New function.
>>        (get_discriminator_from_locus): New function.
>>        * print-rtl.c (print_rtx): Print discriminator.
>>        * rtl.h (insn_discriminator): New function.
>>        * tree-cfg.c: Include input.h.
>>        (assign_discriminator): Assign discriminators to instructions rather
>>        than to the basic block.
>>        * tree-pretty-print.c (dump_location): Print discriminator.
>
> OK if testing passes.

Thanks, committed to google/main.

-cary

[google]: initialize language field for clone function struct

[Xinliang David Li]

During function cloning, the language field of the src func is not
copied. This can lead to null dereference when gcc calls into langhook
functions.  Unfortunately, I lost track of the test case.

Ok for trunk ?

Thanks,

David


2011-04-29  Xinliang David Li  

* tree-inline.c (ininitialize_cfun): Initialize
language field for clone cfun.
Index: tree-inline.c
===
--- tree-inline.c	(revision 173176)
+++ tree-inline.c	(working copy)
@@ -2087,6 +2087,7 @@ initialize_cfun (tree new_fndecl, tree c
   cfun->returns_struct = src_cfun->returns_struct;
   cfun->returns_pcc_struct = src_cfun->returns_pcc_struct;
   cfun->after_tree_profile = src_cfun->after_tree_profile;
+  cfun->language = src_cfun->language;
 
   init_empty_tree_cfg ();
 

Re: [google] LIPO regression tests and bug fixes (issue4444076)

[Xinliang David Li]

On Fri, Apr 29, 2011 at 4:16 PM, Jan Hubicka  wrote:
> Hi,
> It seems that majority of testcases are independent of lipo. We could probably
> enjoy more of testing on mainline, so could you please take those working on
> mainline and make mainline patch and let me know what of the tests are not 
> working
> there?

Actually those test cases are cloned from tree-prof directory into the
lipo sub-directory. The difference is that lipo.exp file passes
additional -fripa flag.  The missing tests for LIPO are ones with
multiple source with non trivial module group testing -- I have not
added those yet.

>
> We probably ought to fix the pass name... We already have "ipa-profile" for 
> profile
> propagation.  What about "gcov", unless we could come with something better?

Yes -- tree_profile_ipa and ipa_profile confuses many people.


>> +int a[1000];
>> +int b[1000];
>> +int size=1;
>> +int max=1;
>> +main()
>> +{
>> +  int i;
>> +  for (i=0;i> +    {
>> +      __builtin_memcpy (a, b, size * sizeof (a[0]));
>> +      asm("");
>> +    }
>> +   return 0;
>> +}
>> +/* { dg-final-use { scan-ipa-dump "Single value 4 stringop" 
>> "tree_profile_ipa"} } */
>> +/* Really this ought to simplify into assignment, but we are not there yet. 
>>  */
>> +/* a[0] = b[0] is what we fold the resulting memcpy into.  */
>> +/* { dg-final-use { scan-tree-dump " = MEM.*&b" "optimized"} } */
>> +/* { dg-final-use { scan-tree-dump "MEM.*&a\\\] = " "optimized"} } */
>> +/* { dg-final-use { cleanup-tree-dump "optimized" } } */
>> +/* { dg-final-use { cleanup-ipa-dump "tree_profile_ipa" } } */
>> Index: testsuite/gcc.dg/tree-prof/lipo/pr34999.c
>> ===
>> --- testsuite/gcc.dg/tree-prof/lipo/pr34999.c (revision 0)
>> +++ testsuite/gcc.dg/tree-prof/lipo/pr34999.c (revision 0)
>> @@ -0,0 +1,45 @@
>> +/* Same test as built-in-setjmp.c.  Includes the case where
>> +   the source block of a crossing fallthru edge ends with a call.  */
>> +/* { dg-require-effective-target freorder } */
>> +/* { dg-options "-O2 -freorder-blocks-and-partition" } */
>> +
>> +extern int strcmp(const char *, const char *);
>> +extern char *strcpy(char *, const char *);
>> +extern void abort(void);
>> +extern void exit(int);
>> +
>> +void *buf[20];
>> +
>> +void __attribute__((noinline))
>> +sub2 (void)
>> +{
>> +  __builtin_longjmp (buf, 1);
>> +}
>> +
>> +int
>> +main ()
>> +{
>> +  char *p = (char *) __builtin_alloca (20);
>> +
>> +  strcpy (p, "test");
>> +
>> +  if (__builtin_setjmp (buf))
>> +    {
>> +      if (strcmp (p, "test") != 0)
>> +     abort ();
>> +
>> +      exit (0);
>> +    }
>> +
>> +  {
>> +    int *q = (int *) __builtin_alloca (p[2] * sizeof (int));
>> +    int i;
>> +
>> +    for (i = 0; i < p[2]; i++)
>> +      q[i] = 0;
>> +
>> +    while (1)
>> +      sub2 ();
>> +  }
>> +}
>> +
>> Index: testsuite/gcc.dg/tree-prof/lipo/stringop-2.c
>> ===
>> --- testsuite/gcc.dg/tree-prof/lipo/stringop-2.c      (revision 0)
>> +++ testsuite/gcc.dg/tree-prof/lipo/stringop-2.c      (revision 0)
>> @@ -0,0 +1,20 @@
>> +/* { dg-options "-O2 -fdump-tree-optimized -fdump-ipa-tree_profile_ipa" } */
>> +int a[1000];
>> +int b[1000];
>> +int size=1;
>> +int max=1;
>> +main()
>> +{
>> +  int i;
>> +  for (i=0;i> +    {
>> +      __builtin_memset (a, 10, size * sizeof (a[0]));
>> +      asm("");
>> +    }
>> +   return 0;
>> +}
>> +/* { dg-final-use { scan-ipa-dump "Single value 4 stringop" 
>> "tree_profile_ipa"} } */
>> +/* The versioned memset of size 4 should be optimized to an assignment.  */
>> +/* { dg-final-use { scan-tree-dump "a\\\[0\\\] = 168430090" "optimized"} } 
>> */
>> +/* { dg-final-use { cleanup-tree-dump "optimized" } } */
>> +/* { dg-final-use { cleanup-ipa-dump "tree_profile_ipa" } } */
>> Index: testsuite/gcc.dg/tree-prof/lipo/update-loopch.c
>> ===
>> --- testsuite/gcc.dg/tree-prof/lipo/update-loopch.c   (revision 0)
>> +++ testsuite/gcc.dg/tree-prof/lipo/update-loopch.c   (revision 0)
>> @@ -0,0 +1,21 @@
>> +/* { dg-options "-O2 -fdump-ipa-tree_profile_ipa-blocks 
>> -fdump-tree-optimized-blocks" } */
>> +int max = 3;
>> +int a[8];
>> +int
>> +main ()
>> +{
>> +  int i;
>> +  for (i = 0; i < max; i++)
>> +    {
>> +      a[i % 8]++;
>> +    }
>> +  return 0;
>> +}
>> +/* Loop header copying will peel away the initial conditional, so the loop 
>> body
>> +   is once reached directly from entry point of function, rest via loopback
>> +   edge.  */
>> +/* { dg-final-use { scan-ipa-dump "count:3" "tree_profile_ipa"} } */
>> +/* { dg-final-use { scan-tree-dump "count:2" "optimized"} } */
>> +/* { dg-final-use { scan-tree-dump-not "Invalid sum" "optimized"} } */
>> +/* { dg-final-use { cleanup-ipa-dump "tree_profile_ipa" } } */
>> +/* { dg-final-use { cleanup-tree-dump "optimized" } } */
>> Index: 
>> testsuite/gcc.dg/tree-prof/lipo/indir-call-

Re: [PATCH] Cleanup of cgraph topological ordering functions

[Jan Hubicka]

> 2011-04-29  Martin Jambor  
> 
>   * cgraph.h (cgraph_postorder): Remove declaration.
>   * ipa-utils.h (ipa_free_postorder_info): Declare.
>   (ipa_reverse_postorder): Likewise.
>   * cgraphunit.c: Include ipa-utils.h.
>   (cgraph_expand_all_functions): Update call to ipa_reverse_postorder.
>   * ipa-inline.c: Include ipa-utils.h.
>   (ipa_inline): Update call to ipa_reverse_postorder.
>   * ipa-pure-const.c (propagate_pure_const): Update call to
>   ipa_reduced_postorder and ipa_print_order.  Call
>   ipa_free_postorder_info to clean up.
>   (propagate_nothrow): Likewise.
>   * ipa-reference.c (propagate): Removed a useless call to
>   ipa_utils_reduced_inorder, updated a call to ipa_reduced_postorder
>   and ipa_print_order.  Call ipa_free_postorder_info to clean up.
>   * ipa.c: Include ipa-utils.h.
>   (ipa_profile): Update call to ipa_reverse_postorder.
>   (cgraph_postorder): Moved to...
>   * ipa-utils.c (ipa_reverse_postorder): ...here and renamed.
>   (ipa_utils_print_order): Renamed to ipa_print_order.
>   (ipa_utils_reduced_inorder): Renamed to ipa_reduced_postorder. Updated
>   comments.
>   (ipa_free_postorder_info): New function.
>   * passes.c: Include ipa-utils.h.
>   (do_per_function_toporder): Update call to ipa_reverse_postorder.
>   (ipa_write_summaries): Likewise.
> 
>   * Makefile.in (passes.o): Add IPA_UTILS_H to dependencies.
>   (cgraphunit.o): Likewise.
>   (ipa.o): Likewise.
>   (ipa-inline.o): Likewise.
> 
> lto/
>   * lto.c: Include ipa-utils.h.
>   (lto_balanced_map): Update call to ipa_reverse_postorder.
>   * Make-lang.in (lto/lto.o): Add IPA_UTILS_H to dependencies.

OK,
thanks

Honza

Re: Disable tracer by default for profile use (issue4428074)

[Jan Hubicka]

> 2011/4/29 Sharad Singhai (? ???) :
> > On Thu, Apr 28, 2011 at 4:38 PM, Richard Guenther
> >  wrote:
> >>
> >> On Fri, Apr 29, 2011 at 1:34 AM, Xinliang David Li 
> >> wrote:
> >> > Sharad can provide some some performance data -- we have seen up to 2%
> >> > degradation to with tracer turned on for one of google's most
> >> > important program. Perhaps Sharad can collect some SPEC numbers.
> >> >
> >> > I agree a better approach should be to fix the problem in tracer
> >> > instead of turning it off in trunk.
> >>
> >> Esp. not turning it off for profile-use only where it should have the most
> >> precise input.
> >>
> >
> > Yes. As I explained, I am not proposing to turn -ftracer off in the trunk.
> 
> Ah, sorry for the confusion on my side.  If you can back up the change
> with SPEC performance numbers it's ok for trunk as well.  But indeed
> coverage of -ftracer will be zero then, and I wonder what it is useful
> for if it doesn't even do good with FDO ...

Tracer definitely did measurable SPEC speedups at a time it was implemented.
So we are most likely running into some bug, like misupdated profile or
tracer confusing something more important.  If you provide me some testcase,
I can try to debug it.

Honza

[PATCH] Cleanup of cgraph topological ordering functions

[Martin Jambor]

Hi,

today while discussing the two functions that we have in gcc to
topologically sort the call graph, Honza asked me to put them both
into the ipa-utils file and rename them.

So this patch moves cgraph_postorder there and renames it to
ipa_reverse_postorder, and similarly, it renames
ipa_utils_reduced_inorder to ipa_reduced_postorder because that is
what it is.  The names are perhaps too similar but the number of
parameters required for the latter function should be enough to
distinguish between the two well.  The new names should help users to
pick the one that better suits their needs.

The functions are quite similar and in future we might want to merge
them but that was not the goal today.  They are also a bit different,
ipa_reverse_postorder works on callers edges rather than on callees,
does not have an option to reduce SCCs or ignore some edges, the
latter is more efficient and automatically ignores edges from
always_inline functions to non-always_inline ones.

Additionally, there are a few cleanups.  ipa_utils_print_order also
lost the utils part from its prefix and I have introduced
ipa_free_postorder_info to deallocate the structures
ipa_reduced_postorder allocates and assigns th aux pointers to them.
Finally, propagate() function in ipa-reference.c seemed to make an
extra topological sorting for no good reason so I removed that.

Bootstrapped and tested on x86_64-linux.  OK for trunk?

Thanks,

Martin



2011-04-29  Martin Jambor  

* cgraph.h (cgraph_postorder): Remove declaration.
* ipa-utils.h (ipa_free_postorder_info): Declare.
(ipa_reverse_postorder): Likewise.
* cgraphunit.c: Include ipa-utils.h.
(cgraph_expand_all_functions): Update call to ipa_reverse_postorder.
* ipa-inline.c: Include ipa-utils.h.
(ipa_inline): Update call to ipa_reverse_postorder.
* ipa-pure-const.c (propagate_pure_const): Update call to
ipa_reduced_postorder and ipa_print_order.  Call
ipa_free_postorder_info to clean up.
(propagate_nothrow): Likewise.
* ipa-reference.c (propagate): Removed a useless call to
ipa_utils_reduced_inorder, updated a call to ipa_reduced_postorder
and ipa_print_order.  Call ipa_free_postorder_info to clean up.
* ipa.c: Include ipa-utils.h.
(ipa_profile): Update call to ipa_reverse_postorder.
(cgraph_postorder): Moved to...
* ipa-utils.c (ipa_reverse_postorder): ...here and renamed.
(ipa_utils_print_order): Renamed to ipa_print_order.
(ipa_utils_reduced_inorder): Renamed to ipa_reduced_postorder. Updated
comments.
(ipa_free_postorder_info): New function.
* passes.c: Include ipa-utils.h.
(do_per_function_toporder): Update call to ipa_reverse_postorder.
(ipa_write_summaries): Likewise.

* Makefile.in (passes.o): Add IPA_UTILS_H to dependencies.
(cgraphunit.o): Likewise.
(ipa.o): Likewise.
(ipa-inline.o): Likewise.

lto/
* lto.c: Include ipa-utils.h.
(lto_balanced_map): Update call to ipa_reverse_postorder.
* Make-lang.in (lto/lto.o): Add IPA_UTILS_H to dependencies.


Index: src/gcc/Makefile.in
===
--- src.orig/gcc/Makefile.in
+++ src/gcc/Makefile.in
@@ -2847,7 +2847,7 @@ passes.o : passes.c $(CONFIG_H) $(SYSTEM
hosthooks.h $(CGRAPH_H) $(COVERAGE_H) $(TREE_PASS_H) $(TREE_DUMP_H) \
$(GGC_H) $(INTEGRATE_H) $(CPPLIB_H) $(OPTS_H) $(TREE_FLOW_H) 
$(TREE_INLINE_H) \
gt-passes.h $(DF_H) $(PREDICT_H) $(LTO_HEADER_H) $(LTO_SECTION_OUT_H) \
-   $(PLUGIN_H)
+   $(PLUGIN_H) $(IPA_UTILS_H)
 
 plugin.o : plugin.c $(PLUGIN_H) $(CONFIG_H) $(SYSTEM_H) coretypes.h \
$(DIAGNOSTIC_CORE_H) $(TREE_H) $(TREE_PASS_H) intl.h $(PLUGIN_VERSION_H) 
$(GGC_H)
@@ -2997,7 +2997,7 @@ cgraphunit.o : cgraphunit.c $(CONFIG_H)
$(TREE_FLOW_H) $(TREE_PASS_H) debug.h $(DIAGNOSTIC_H) \
$(FIBHEAP_H) output.h $(PARAMS_H) $(RTL_H) $(TIMEVAR_H) $(IPA_PROP_H) \
gt-cgraphunit.h tree-iterator.h $(COVERAGE_H) $(TREE_DUMP_H) \
-   tree-pretty-print.h gimple-pretty-print.h ipa-inline.h
+   tree-pretty-print.h gimple-pretty-print.h ipa-inline.h $(IPA_UTILS_H)
 cgraphbuild.o : cgraphbuild.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) \
$(TREE_H) langhooks.h $(CGRAPH_H) intl.h pointer-set.h $(GIMPLE_H) \
$(TREE_FLOW_H) $(TREE_PASS_H) $(IPA_UTILS_H) $(EXCEPT_H) \
@@ -3007,7 +3007,8 @@ varpool.o : varpool.c $(CONFIG_H) $(SYST
$(GGC_H) $(TIMEVAR_H) debug.h $(TARGET_H) output.h $(GIMPLE_H) \
$(TREE_FLOW_H) gt-varpool.h
 ipa.o : ipa.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(CGRAPH_H) \
-   $(TREE_PASS_H) $(TIMEVAR_H) $(GIMPLE_H) $(GGC_H) pointer-set.h
+   $(TREE_PASS_H) $(TIMEVAR_H) $(GIMPLE_H) $(GGC_H) pointer-set.h \
+   $(IPA_UTILS_H)
 ipa-prop.o : ipa-prop.c $(CONFIG_H) $(SYSTEM_H) coretypes.h \
langhooks.h $(GGC_H) $(TARGET_H) $(CGRAPH_H) $(IPA_PROP_H) $(DIAGNOSTIC_H) \
$(TR

Re: [google] LIPO regression tests and bug fixes (issue4444076)

[Jan Hubicka]

Hi,
It seems that majority of testcases are independent of lipo. We could probably
enjoy more of testing on mainline, so could you please take those working on
mainline and make mainline patch and let me know what of the tests are not 
working
there? 
> Index: testsuite/gcc.dg/tree-prof/lipo/inliner-1.c
> ===
> --- testsuite/gcc.dg/tree-prof/lipo/inliner-1.c   (revision 0)
> +++ testsuite/gcc.dg/tree-prof/lipo/inliner-1.c   (revision 0)
> @@ -0,0 +1,42 @@
> +/* { dg-options "-O2 -fdump-tree-optimized" } */
> +int a;
> +int b[100];
> +void abort (void);
> +
> +inline void
> +cold_function ()
> +{
> +  int i;
> +  for (i = 0; i < 99; i++)
> +if (b[i] / (b[i+1] + 1))
> +  abort ();
> +}
> +
> +inline void
> +hot_function ()
> +{
> +  int i;
> +  for (i = 0; i < 99; i++)
> +if (b[i] / (b[i+1] + 1))
> +  abort ();
> +}
> +
> +main ()
> +{
> +  int i;
> +  for (i = 0; i < 100; i++)
> +{
> +  if (a)
> +cold_function ();
> +  else
> +hot_function ();
> +}
> +  return 0;
> +}
> +
> +/* cold function should be inlined, while hot function should not.  
> +   Look for "cold_function () [tail call];" call statement not for the
> +   declaration or other apperances of the string in dump.  */
> +/* { dg-final-use { scan-tree-dump "cold_function ..;" "optimized"} } */
> +/* { dg-final-use { scan-tree-dump-not "hot_function ..;" "optimized"} } */
> +/* { dg-final-use { cleanup-tree-dump "optimized" } } */
> Index: testsuite/gcc.dg/tree-prof/lipo/bb-reorg.c
> ===
> --- testsuite/gcc.dg/tree-prof/lipo/bb-reorg.c(revision 0)
> +++ testsuite/gcc.dg/tree-prof/lipo/bb-reorg.c(revision 0)
> @@ -0,0 +1,39 @@
> +/* { dg-require-effective-target freorder } */
> +/* { dg-options "-O2 -freorder-blocks-and-partition" } */
> +
> +#include 
> +
> +#define SIZE 1000
> +int t0 = 0;
> +const char *t2[SIZE];
> +char buf[SIZE];
> +
> +void
> +foo (void)
> +{
> +  char *s = buf;
> +  t0 = 1;
> +
> +  for (;;)
> +{
> +  if (*s == '\0')
> + break;
> +  else
> + {
> +   t2[t0] = s;
> +   t0++;
> + }
> +  *s++ = '\0';
> +}
> +  t2[t0] = NULL;
> +}
> +
> +
> +int
> +main ()
> +{
> +  strcpy (buf, "hello");
> +  foo ();
> +  return 0; 
> +}
> +
> Index: testsuite/gcc.dg/tree-prof/lipo/stringop-1.c
> ===
> --- testsuite/gcc.dg/tree-prof/lipo/stringop-1.c  (revision 0)
> +++ testsuite/gcc.dg/tree-prof/lipo/stringop-1.c  (revision 0)
> @@ -0,0 +1,22 @@
> +/* { dg-options "-O2 -fdump-tree-optimized -fdump-ipa-tree_profile_ipa" } */

We probably ought to fix the pass name... We already have "ipa-profile" for 
profile
propagation.  What about "gcov", unless we could come with something better?
> +int a[1000];
> +int b[1000];
> +int size=1;
> +int max=1;
> +main()
> +{
> +  int i;
> +  for (i=0;i +{
> +  __builtin_memcpy (a, b, size * sizeof (a[0]));
> +  asm("");
> +}
> +   return 0;
> +}
> +/* { dg-final-use { scan-ipa-dump "Single value 4 stringop" 
> "tree_profile_ipa"} } */
> +/* Really this ought to simplify into assignment, but we are not there yet.  
> */
> +/* a[0] = b[0] is what we fold the resulting memcpy into.  */
> +/* { dg-final-use { scan-tree-dump " = MEM.*&b" "optimized"} } */
> +/* { dg-final-use { scan-tree-dump "MEM.*&a\\\] = " "optimized"} } */
> +/* { dg-final-use { cleanup-tree-dump "optimized" } } */
> +/* { dg-final-use { cleanup-ipa-dump "tree_profile_ipa" } } */
> Index: testsuite/gcc.dg/tree-prof/lipo/pr34999.c
> ===
> --- testsuite/gcc.dg/tree-prof/lipo/pr34999.c (revision 0)
> +++ testsuite/gcc.dg/tree-prof/lipo/pr34999.c (revision 0)
> @@ -0,0 +1,45 @@
> +/* Same test as built-in-setjmp.c.  Includes the case where
> +   the source block of a crossing fallthru edge ends with a call.  */
> +/* { dg-require-effective-target freorder } */
> +/* { dg-options "-O2 -freorder-blocks-and-partition" } */
> +
> +extern int strcmp(const char *, const char *);
> +extern char *strcpy(char *, const char *);
> +extern void abort(void);
> +extern void exit(int);
> +
> +void *buf[20];
> +
> +void __attribute__((noinline))
> +sub2 (void)
> +{
> +  __builtin_longjmp (buf, 1);
> +}
> +
> +int
> +main ()
> +{
> +  char *p = (char *) __builtin_alloca (20);
> +
> +  strcpy (p, "test");
> +
> +  if (__builtin_setjmp (buf))
> +{
> +  if (strcmp (p, "test") != 0)
> + abort ();
> +
> +  exit (0);
> +}
> +
> +  {
> +int *q = (int *) __builtin_alloca (p[2] * sizeof (int));
> +int i;
> +
> +for (i = 0; i < p[2]; i++)
> +  q[i] = 0;
> +
> +while (1)
> +  sub2 ();
> +  }
> +}
> +
> Index: testsuite/gcc.dg/tree-prof/lipo/stringop-2.c
> ===
> --- testsuite/gcc.

Re: [google]: Add new test case on integer literal address

[Xinliang David Li]

The old bug was that the second call to func got CSEed.

David

On Fri, Apr 29, 2011 at 4:08 PM, Xinliang David Li  wrote:
> This test case was extracted from kernel source exposed in some older
> version of gcc. Works fine in trunk, but need to add a test for it.
>
> Ok for trunk?
>
> Thanks,
>
> David
>

[google]: Add new test case on integer literal address

[Xinliang David Li]

This test case was extracted from kernel source exposed in some older
version of gcc. Works fine in trunk, but need to add a test for it.

Ok for trunk?

Thanks,

David
Index: testsuite/gcc.dg/tree-ssa/integer-addr.c
===
--- testsuite/gcc.dg/tree-ssa/integer-addr.c	(revision 0)
+++ testsuite/gcc.dg/tree-ssa/integer-addr.c	(revision 0)
@@ -0,0 +1,29 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized -fno-strict-aliasing" } */
+/* Test with fixed address */
+static int *foo =  (int *) (unsigned long) 0x780;
+
+int func(void) __attribute__ ((noinline));
+
+extern int bar(void);
+
+int func(void)
+{
+   if (*foo) {
+  return 1;
+   }
+   return 0;
+
+}
+
+int foobar(void)
+{
+
+   if (func()) {
+  *foo = 1;
+   }
+   return func();
+}
+
+/* { dg-final { scan-tree-dump-times "= func" 2 "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */
Index: testsuite/ChangeLog
===
--- testsuite/ChangeLog	(revision 173176)
+++ testsuite/ChangeLog	(working copy)
@@ -1,3 +1,7 @@
+2011-04-29  Xinliang David Li  
+	
+	* gcc.dg/tree-ssa/integer-addr.c: New test.
+
 2011-04-29  Tobias Burnus  
 
 	PR fortran/48810

Re: Toplevel cleanup: disable Java when libffi not supported

[Joseph S. Myers]

On Fri, 29 Apr 2011, Tom Tromey wrote:

> > "Joseph" == Joseph S Myers  writes:
> 
> Joseph> This patch, relative to a tree with
> Joseph>  applied,
> Joseph> continues the cleanup of toplevel cases relating to disabling Java or
> Joseph> Java libraries by arranging for Java to be disabled (via
> Joseph> unsupported_languages) on targets not supporting libffi
> 
> It does make sense to build libjava without libffi.
> 
> It just means that libjava has somewhat degraded functionality -- libffi
> is needed for the interpreter, for JNI, and for some kinds of reflection
> (Method.invoke).
> 
> I am not sure if there are any existing targets where libjava works but
> libffi does not.  Looking at libjava/configure.host, maybe `arm*-elf'
> (but maybe configure.host is out of data, it is certainly possible).

arm*-elf are targets where libffi's configure thinks it is supported.

> There is also --without-libffi, though; I didn't look to see what your
> patch does with this.
> 
> A better gate for libjava might be boehm-gc.  It is required to run any
> sort of real program.

I should perhaps reiterate that I think the toplevel handles 
unsupported_languages in a suboptimal way.  What it should mean in my view 
is that the languages don't get enabled by default (or by "all" in 
--enable-languages) but can still be enabled by specifying them manually 
in --enable-languages - whereas at present it forces the language to be 
disabled even if the user enables it explicitly.

So Java (and Go) should be disabled by default when libffi isn't 
supported, because they'd try by default to build libffi and so a default 
build will fail.  And much the same applies with boehm-gc.  But if a user 
explicitly enables Java on such a target, libffi should still be disabled 
as unsupported but the front end and other libraries should be built (and 
quite possibly fail).  (Ideally this would have generic logic - for each 
front end, disable it by default if any of the target libraries for that 
language aren't supported.  But that first requires a generic way to get 
information from a library directory about what targets that library 
supports.)

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

Re: [pph] Enable nested namespaces (issue4431071)

[dnovillo]

Looks OK.  Some comments below.


http://codereview.appspot.com/4431071/diff/1/gcc/c-family/c.opt
File gcc/c-family/c.opt (right):

http://codereview.appspot.com/4431071/diff/1/gcc/c-family/c.opt#newcode943
gcc/c-family/c.opt:943:
+fpph-dump-tree
+C++ Var(flag_pph_dump_tree)
+-fpph-dump-treeDump global namespace tree around PPH reads/writes.
+

We should just add to the existing -fdump-translation-unit.  Maybe not
so important now.

http://codereview.appspot.com/4431071/diff/1/gcc/cp/name-lookup.c
File gcc/cp/name-lookup.c (right):

http://codereview.appspot.com/4431071/diff/1/gcc/cp/name-lookup.c#newcode1186
gcc/cp/name-lookup.c:1186: {
+tree
+pushdecl_into_namespace (tree dcl, tree nsp)
+{

Needs comment.

http://codereview.appspot.com/4431071/

Re: [google] Check if the nonnull attribute is applied to 'this' (issue4446070)

[Mike Stump]

On Apr 29, 2011, at 1:08 PM, Basile Starynkevitch wrote:
> Not really.

How's this then:

http://stackoverflow.com/questions/2474018/when-does-invoking-a-member-function-on-a-null-instance-result-in-undefined-behav

?  :-)

[Fortran] RFC patch for gfc_trans_deferred_vars (PR 48786)

[Tobias Burnus]

Dear all,

gfc_trans_deferred_vars is a bit of a mess; there is first a block which 
handles function results of the type proc_sym->result == proc_sym. 
Afterwards, deferred variables - local, dummys, and proc_sym->result (!= 
proc_sym) are handled.


The problem is that for allocatable results (esp. of CLASS type) and for 
deferred-length strings, the same initialization has to happen as for 
function results.


Consequence: There is code partial duplication - and some code should be 
duplicated, but is not; that causes the issue with the current code.


Attached patch tries to fix that; it fixes Arjan's wrong-code issue and 
it also reduces the code size; however, I do not think that it makes the 
code very readable.


What do you think? How can this be improved? Or should the patch be 
committed as is? (The patch was regtested on x86-64-linux.)


Tobias
diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
index f80c9db..3db38eb 100644
--- a/gcc/fortran/trans-decl.c
+++ b/gcc/fortran/trans-decl.c
@@ -3355,7 +3355,7 @@ gfc_trans_deferred_vars (gfc_symbol * proc_sym, gfc_wrapped_block * block)
   gfc_symbol *sym;
   gfc_formal_arglist *f;
   stmtblock_t tmpblock;
-  bool seen_trans_deferred_array = false;
+  bool seen_trans_deferred_array = false, processed_proc = false;
   tree tmp = NULL;
   gfc_expr *e;
   gfc_se se;
@@ -3391,37 +3391,8 @@ gfc_trans_deferred_vars (gfc_symbol * proc_sym, gfc_wrapped_block * block)
 	}
   else if (proc_sym->ts.type == BT_CHARACTER)
 	{
-	  if (proc_sym->ts.deferred)
-	{
-	  tmp = NULL;
-	  gfc_save_backend_locus (&loc);
-	  gfc_set_backend_locus (&proc_sym->declared_at);
-	  gfc_start_block (&init);
-	  /* Zero the string length on entry.  */
-	  gfc_add_modify (&init, proc_sym->ts.u.cl->backend_decl,
-			  build_int_cst (gfc_charlen_type_node, 0));
-	  /* Null the pointer.  */
-	  e = gfc_lval_expr_from_sym (proc_sym);
-	  gfc_init_se (&se, NULL);
-	  se.want_pointer = 1;
-	  gfc_conv_expr (&se, e);
-	  gfc_free_expr (e);
-	  tmp = se.expr;
-	  gfc_add_modify (&init, tmp,
-			  fold_convert (TREE_TYPE (se.expr),
-	null_pointer_node));
-	  gfc_restore_backend_locus (&loc);
-
-	  /* Pass back the string length on exit.  */
-	  tmp = proc_sym->ts.u.cl->passed_length;
-	  tmp = build_fold_indirect_ref_loc (input_location, tmp);
-	  tmp = fold_convert (gfc_charlen_type_node, tmp);
-	  tmp = fold_build2_loc (input_location, MODIFY_EXPR,
- gfc_charlen_type_node, tmp,
- proc_sym->ts.u.cl->backend_decl);
-	  gfc_add_init_cleanup (block, gfc_finish_block (&init), tmp);
-	}
-	  else if (TREE_CODE (proc_sym->ts.u.cl->backend_decl) == VAR_DECL)
+	  if (TREE_CODE (proc_sym->ts.u.cl->backend_decl) == VAR_DECL
+	  && !proc_sym->ts.deferred)
 	gfc_trans_dummy_character (proc_sym, proc_sym->ts.u.cl, block);
 	}
   else
@@ -3437,14 +3408,32 @@ gfc_trans_deferred_vars (gfc_symbol * proc_sym, gfc_wrapped_block * block)
   init_intent_out_dt (proc_sym, block);
   gfc_restore_backend_locus (&loc);
 
-  for (sym = proc_sym->tlink; sym != proc_sym; sym = sym->tlink)
+  for (sym = proc_sym->tlink; ; sym = sym->tlink)
 {
   bool sym_has_alloc_comp = (sym->ts.type == BT_DERIVED)
    && sym->ts.u.derived->attr.alloc_comp;
   if (sym->assoc)
 	continue;
 
-  if (sym->attr.dimension)
+  /* Handle sym == proc_sym only once to avoid an endless loop.  */
+  if (sym == proc_sym)
+	{
+	  if (processed_proc)
+	break;
+	  processed_proc = true;
+	}
+
+  /* For function results, which do not need an initialization,
+	 end the loop.  */
+  if (sym == proc_sym
+	  && (sym != proc_sym->result
+	  || !(sym->attr.allocatable || sym->ts.deferred
+		   || sym_has_alloc_comp
+		   || (sym->ts.type == BT_CLASS
+		   && CLASS_DATA (sym)->attr.allocatable
+	break;
+
+  if (sym->attr.dimension && sym != proc_sym)
 	{
 	  switch (sym->as->type)
 	{
@@ -3521,7 +3510,7 @@ gfc_trans_deferred_vars (gfc_symbol * proc_sym, gfc_wrapped_block * block)
 	  if (sym_has_alloc_comp && !seen_trans_deferred_array)
 	gfc_trans_deferred_array (sym, block);
 	}
-  else if ((!sym->attr.dummy || sym->ts.deferred)
+  else if (! sym->attr.dimension && (!sym->attr.dummy || sym->ts.deferred)
 		&& (sym->attr.allocatable
 		|| (sym->ts.type == BT_CLASS
 			&& CLASS_DATA (sym)->attr.allocatable)))
@@ -3551,9 +3540,9 @@ gfc_trans_deferred_vars (gfc_symbol * proc_sym, gfc_wrapped_block * block)
 	null_pointer_node));
 		}
 
-	  if ((sym->attr.dummy ||sym->attr.result)
-		&& sym->ts.type == BT_CHARACTER
-		&& sym->ts.deferred)
+	  if ((sym->attr.dummy || sym->attr.result || sym == proc_sym)
+		  && sym->ts.type == BT_CHARACTER
+		  && sym->ts.deferred)
 		{
 		  /* Character length passed by reference.  */
 		  tmp = sym->ts.u.cl->passed_length;
@@ -3582,7 +3571,7 @@ gfc_trans_deferred_vars (gfc_s

[Patch, Fortran] PR 48800 - fix "IMPORT :: symbol"

[Tobias Burnus]

Nearly obvious patch. Build and regtested on x86-64-linux.

OK for the trunk?

Tobias
2011-04-30  Tobias Burnus  

	PR fortran/48800
	* decl.c (gfc_match_import): Don't try to find the
	symbol if already found.

2011-04-30  Tobias Burnus  

	PR fortran/48800
	* gfortran.dg/interface_36.f90: New.

diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c
index 9901fb1..dfbca29 100644
--- a/gcc/fortran/decl.c
+++ b/gcc/fortran/decl.c
@@ -2995,7 +2995,7 @@ gfc_match_import (void)
 	   gfc_error ("Type name '%s' at %C is ambiguous", name);
 	   return MATCH_ERROR;
 	}
-	  else if (gfc_current_ns->proc_name->ns->parent !=  NULL
+	  else if (!sym && gfc_current_ns->proc_name->ns->parent !=  NULL
 		   && gfc_find_symbol (name,
    gfc_current_ns->proc_name->ns->parent,
    1, &sym))
--- /dev/null	2011-04-29 18:38:34.095891993 +0200
+++ gcc/gcc/testsuite/gfortran.dg/interface_36.f90	2011-04-29 19:10:43.0 +0200
@@ -0,0 +1,28 @@
+! { dg-do compile }
+!
+! PR fortran/48800
+!
+! Contributed by Daniel Carrera
+!
+ pure function runge_kutta_step(t, r_, dr, h) result(res)
+ real, intent(in) :: t, r_(:), h
+ real, dimension(:), allocatable :: k1, k2, k3, k4, res
+ integer :: N
+
+ interface
+ pure function dr(t, r_)  ! { dg-error "cannot have a deferred shape" }
+ real, intent(in) :: t, r_(:)
+ real :: dr(:)
+ end function
+ end interface
+
+ N = size(r_)
+ allocate(k1(N),k2(N),k3(N),k4(N),res(N))
+
+ k1 = dr(t, r_)
+ k2 = dr(t + h/2, r_ + k1*h/2)
+ k3 = dr(t + h/2, r_ + k2*h/2)
+ k4 = dr(t + h  , r_ + k3*h)
+
+ res = r_ + (k1 + 2*k2 + 2*k3 + k4) * h/6
+ end function

Re: [pph] DECL_INITIAL instead of DECL_ARG_TYPE (issue4442102)

[Diego Novillo]

On Fri, Apr 29, 2011 at 17:28, Lawrence Crowl  wrote:
> In the pph file, save and restore DECL_INITIAL instead of just
> PARM_DECL DECL_ARG_TYPE.
>
> Index: gcc/cp/ChangeLog.pph
>
> 2011-04-29  Lawrence Crowl 
>
>        * pph-streamer-out.c (pph_stream_write_tree): Write DECL_INITIAL
>        instead of PARM_DECL DECL_ARG_TYPE.
>        * pph-streamer-in.c (pph_stream_read_tree): Read DECL_INITIAL
>        instead of PARM_DECL DECL_ARG_TYPE.

OK.


Diego.

Re: [pph] Save/restore PARM_DECL DECL_ARG_TYPE (issue4441079)

[Lawrence Crowl]

On 4/29/11, Diego Novillo  wrote:
> On Apr 29, 2011 Richard Guenther  wrote:
> > On Apr 29, 2011 Lawrence Crowl  wrote:
> > > This patch saves and restores the PARM_DECL DECL_ARG_TYPE in the
> > > PPH file.
> >
> > Should be already streamed via lto_output_ts_decl_common_tree_pointers
> > as it is aliased to DECL_INITIAL.
>
> No, I moved the streaming of DECL_INITIAL to a streamer hook because
> in the gimple case we do more than streaming the initial value.  We
> get the varpool node for the symbol and only stream the initial value
> if we can find it.
>
> Lawrence, I think we should just simply stream DECL_INITIAL in the
> DECL_P case.  We are missing initializer expressions in every
> variable, otherwise.

Done.  It seems to have fixed one more unexpected failure.

-- 
Lawrence Crowl

[pph] DECL_INITIAL instead of DECL_ARG_TYPE (issue4442102)

[Lawrence Crowl]

In the pph file, save and restore DECL_INITIAL instead of just
PARM_DECL DECL_ARG_TYPE.

Index: gcc/cp/ChangeLog.pph

2011-04-29  Lawrence Crowl 

* pph-streamer-out.c (pph_stream_write_tree): Write DECL_INITIAL
instead of PARM_DECL DECL_ARG_TYPE.
* pph-streamer-in.c (pph_stream_read_tree): Read DECL_INITIAL
instead of PARM_DECL DECL_ARG_TYPE.

Index: gcc/cp/pph-streamer-in.c
===
--- gcc/cp/pph-streamer-in.c(revision 173189)
+++ gcc/cp/pph-streamer-in.c(working copy)
@@ -783,6 +783,8 @@ pph_stream_read_tree (struct lto_input_b
 
   if (DECL_P (expr))
 {
+  DECL_INITIAL (expr) = pph_input_tree (stream);
+
   if (TREE_CODE (expr) == FUNCTION_DECL
  || TREE_CODE (expr) == NAMESPACE_DECL
  || TREE_CODE (expr) == PARM_DECL
@@ -791,8 +793,6 @@ pph_stream_read_tree (struct lto_input_b
  pph_stream_read_lang_specific (stream, expr);
  if (TREE_CODE (expr) == FUNCTION_DECL)
DECL_SAVED_TREE (expr) = pph_input_tree (stream);
- else if (TREE_CODE (expr) == PARM_DECL)
-   DECL_ARG_TYPE (expr) = pph_input_tree (stream);
}
 
   if (TREE_CODE (expr) == TYPE_DECL)
Index: gcc/cp/pph-streamer-out.c
===
--- gcc/cp/pph-streamer-out.c   (revision 173189)
+++ gcc/cp/pph-streamer-out.c   (working copy)
@@ -787,6 +787,8 @@ pph_stream_write_tree (struct output_blo
 
   if (DECL_P (expr))
 {
+  pph_output_tree_or_ref_1 (stream, DECL_INITIAL (expr), ref_p, 3);
+
   if (TREE_CODE (expr) == FUNCTION_DECL
  || TREE_CODE (expr) == NAMESPACE_DECL
  || TREE_CODE (expr) == PARM_DECL
@@ -796,8 +798,6 @@ pph_stream_write_tree (struct output_blo
 
  if (TREE_CODE (expr) == FUNCTION_DECL)
pph_output_tree_or_ref_1 (stream, DECL_SAVED_TREE (expr), ref_p, 3);
- else if (TREE_CODE (expr) == PARM_DECL)
-   pph_output_tree_or_ref_1 (stream, DECL_ARG_TYPE (expr), ref_p, 3);
}
 
   if (TREE_CODE (expr) == TYPE_DECL)

--
This patch is available for review at http://codereview.appspot.com/4442102

Propagate BB predicates in ipa-inline-analysis

[Jan Hubicka]

Hi,
this more or less complettes the infrastructure for predicates by adding
logic propagating predicates across CFG.  I also added switch statement
handling and __builtin_constant_p construct, so we "understand" functions
using those as one in the new testcase.

It also turned out that the predicate handling code should be more aggressive
on simplification and canonicalization so the propagation converges quickly.

Bootstrapped/regtested x86_64-linux, will commit it shortly.

Honza

* gcc.dg/tree-ssa/inline-10.c: New testcase.
* gcc.dg/tree-ssa/inline-9.c: Disable partial inlining.
* ipa-inline.h (clause_t): Turn into unsigned int.
* ipa-inline-analysis.c (add_clause): Do more simplification.
(and_predicates): Shortcut more cases.
(predicates_equal_p): Move forward; check that clauses are properly
ordered.
(or_predicates): Shortcut more cases.
(edge_execution_predicate): Rewrite as...
(set_cond_stmt_execution_predicate): ... this function; handle
__builtin_constant_p.
(set_switch_stmt_execution_predicate): New .
(compute_bb_predicates): New.
(will_be_nonconstant_predicate): Update TODO.
(estimate_function_body_sizes): Use compute_bb_predicates
and free them later, always try to estimate if stmt is constant.
(estimate_time_after_inlining, estimate_size_after_inlining):
Gracefully handle optimized out edges.
(read_predicate): Fix off by one error.
Index: testsuite/gcc.dg/tree-ssa/inline-9.c
===
*** testsuite/gcc.dg/tree-ssa/inline-9.c(revision 173181)
--- testsuite/gcc.dg/tree-ssa/inline-9.c(working copy)
***
*** 1,5 
  /* { dg-do compile } */
! /* { dg-options "-Os -fdump-tree-optimized" } */
  
  /* When optimizing for size, t should be inlined when it expands to one call 
only.  */
  extern int q(int);
--- 1,5 
  /* { dg-do compile } */
! /* { dg-options "-Os -fdump-tree-optimized -fno-partial-inlining" } */ 
  /* When optimizing for size, t should be inlined when it expands to one call 
only.  */
  extern int q(int);
Index: testsuite/gcc.dg/tree-ssa/inline-10.c
===
*** testsuite/gcc.dg/tree-ssa/inline-10.c   (revision 0)
--- testsuite/gcc.dg/tree-ssa/inline-10.c   (revision 0)
***
*** 0 
--- 1,38 
+ /* { dg-do compile } */
+ /* { dg-options "-Os -fdump-tree-optimized -fno-partial-inlining" } */
+ void do_something1(void);
+ void do_something2(void);
+ void do_something3(void);
+ void do_something4(void);
+ void do_something5(void);
+ void do_something_big(int);
+ 
+ int do_something (int size)
+ {
+   if (__builtin_constant_p (size))
+ switch (size)
+   {
+   case 1:do_something1 (); break;
+   case 2:do_something2 (); break;
+   case 5:do_something1 ();  do_something1 ();
+   case 3:do_something3 (); break;
+   case 4:do_something4 (); break;
+   }
+   else
+ do_something_big (size);
+ }
+ extern int n;
+ main()
+ {
+   do_something (2);
+   do_something (3);
+   do_something (5);
+   do_something (70);
+ }
+ /* All calls should be inlined, except for do_something (5).  */
+ /* { dg-final { scan-tree-dump-not "do_something1" "optimized" } } */
+ /* { dg-final { scan-tree-dump-times "do_something2" 1 "optimized" } } */
+ /* { dg-final { scan-tree-dump-times "do_something3" 1 "optimized" } } */
+ /* { dg-final { scan-tree-dump-times "do_something \\(5\\)" 1 "optimized" } } 
*/
+ /* { dg-final { scan-tree-dump-not "do_something \\(70\\)" "optimized" } } */
+ /* { dg-final { cleanup-tree-dump "optimized" } } */
Index: ipa-inline.h
===
*** ipa-inline.h(revision 173181)
--- ipa-inline.h(working copy)
*** typedef VEC(condition,gc) *conditions;
*** 48,54 
 must be true in order for clause to be true.  */
  
  #define MAX_CLAUSES 8
! typedef int clause_t;
  struct GTY(()) predicate
  {
clause_t clause[MAX_CLAUSES + 1];
--- 48,54 
 must be true in order for clause to be true.  */
  
  #define MAX_CLAUSES 8
! typedef unsigned int clause_t;
  struct GTY(()) predicate
  {
clause_t clause[MAX_CLAUSES + 1];
Index: ipa-inline-analysis.c
===
*** ipa-inline-analysis.c   (revision 173181)
--- ipa-inline-analysis.c   (working copy)
*** add_condition (struct inline_summary *su
*** 227,245 
  }
  
  
! /* Add clause CLAUSE into the predicate.  */
  
  static inline void
  add_clause (struct predicate *p, clause_t clause)
  {
int i;
int insert_here = -1;
  
/* True clause.  */
if (!clause)
  return;
  
!   /* Flase clause makes the whole predicate false.  Kill the other variants.  
*/
if (clause == (1 << predicate_false_conditi

[pph] Added missing ChangeLog file in gcc/fortran

[Diego Novillo]

When I modified the fortran #include callback to return bool, I forgot
to add the ChangeLog file for it.  Fixed with rev. 173188.


Diego.

Re: [pph] Relativize pph test commands. (issue4445076)

[Diego Novillo]

On Fri, Apr 29, 2011 at 17:00, Lawrence Crowl  wrote:
> Copy the pph map file from the source area to the run area to avoid
> absolute paths to the map file.  These were causing tests to not match
> up in the test comparison scripts.
>
> Index: gcc/testsuite/ChangeLog.pph
>
> 2011-04-29  Lawrence Crowl  
>
>        * g++.dg/pph/pph.exp: Copy pph map file to avoid absolute paths in
>        test commands.

OK.


Diego.

Re: [google] Use different peeling parameters with available profile (issue4438079)

[शरद सिंघई]

Thanks a bunch!

Sharad

On Fri, Apr 29, 2011 at 1:58 PM,  wrote:
>
> On 2011/04/29 19:21:00, Diego Novillo wrote:
>>
>> On 2011/04/29 00:02:40, singhai wrote:
>
>> > 2011-04-28  Sharad Singhai  
>> >
>> >       gcc/ChangeLog.google-main
>> >       * params.def: Add new parameters to control peeling.
>> >       * tree-ssa-loop-ivcanon.c (try_unroll_loop_completely): Use
>> >       different peeling parameters when profile feedback is available.
>> >       * loop-unroll.c (decide_peel_once_rolling): Ditto.
>> >       (decide_peel_completely): Ditto.
>> >       * doc/invoke.texi: Document new peeling parameters.
>> >
>> >       testsuite/ChangeLog.google-main
>> >       * gcc.dg/vect/O3-vect-pr34223.c: Add extra peeling
>> >       parameters.
>> >       * gcc.dg/vect/vect.exp: Allow reading flags in individual
>> >       tests.
>
>> OK for google/main.
>
>
> Committed rev 173187.
>
>
> Diego.
>
> http://codereview.appspot.com/4438079/

[pph] Relativize pph test commands. (issue4445076)

[Lawrence Crowl]

Copy the pph map file from the source area to the run area to avoid
absolute paths to the map file.  These were causing tests to not match
up in the test comparison scripts.

Index: gcc/testsuite/ChangeLog.pph

2011-04-29  Lawrence Crowl  

* g++.dg/pph/pph.exp: Copy pph map file to avoid absolute paths in
test commands.


Index: gcc/testsuite/g++.dg/pph/pph.exp
===
--- gcc/testsuite/g++.dg/pph/pph.exp(revision 173124)
+++ gcc/testsuite/g++.dg/pph/pph.exp(working copy)
@@ -32,7 +32,9 @@ set scenarios [list "" ]
 set hdr_tests [lsort [glob -nocomplain $srcdir/$subdir/\[cdx\]*.h]]
 set neg_tests [lsort [glob -nocomplain $srcdir/$subdir/\[dy\]*.cc]]
 set pos_tests [lsort [glob -nocomplain $srcdir/$subdir/\[cpx\]*.cc]]
-set mapflag -fpph-map=$srcdir/$subdir/pph.map
+
+gcc_copy_files $srcdir/$subdir/pph.map .
+set mapflag -fpph-map=pph.map
 
 foreach scenario $scenarios {
 

--
This patch is available for review at http://codereview.appspot.com/4445076

Re: [Patch, fortran] PR48462 - [4.6/4.7 Regression] realloc on assignment: matmul Segmentation Fault with Allocatable Array + PR48746

[Thomas Koenig]

Hello Paul,

there's another point: The sizes are also not set correctly.

ig25@linux-fd1f:~/Krempel/H> cat mm.f90
program main
  implicit none
  integer, parameter :: m=10, n=12, count=4
  double precision :: a(m, count), b(count, n), c(m, n)
  double precision, dimension(:,:), allocatable :: tmp

  call random_number(a)
  call random_number(b)
  tmp = matmul(a,b)
  print *,size(tmp,1)
  print *,size(tmp,2)
end program main
ig25@linux-fd1f:~/Krempel/H> gfortran mm.f90
ig25@linux-fd1f:~/Krempel/H> ./a.out
   1
   1
ig25@linux-fd1f:~/Krempel/H>

Re: [google] Use different peeling parameters with available profile (issue4438079)

[dnovillo]

On 2011/04/29 19:21:00, Diego Novillo wrote:

On 2011/04/29 00:02:40, singhai wrote:



> 2011-04-28  Sharad Singhai  
>
>gcc/ChangeLog.google-main
>* params.def: Add new parameters to control peeling.
>* tree-ssa-loop-ivcanon.c (try_unroll_loop_completely): Use
>different peeling parameters when profile feedback is available.
>* loop-unroll.c (decide_peel_once_rolling): Ditto.
>(decide_peel_completely): Ditto.
>* doc/invoke.texi: Document new peeling parameters.
>
>testsuite/ChangeLog.google-main
>* gcc.dg/vect/O3-vect-pr34223.c: Add extra peeling
>parameters.
>* gcc.dg/vect/vect.exp: Allow reading flags in individual
>tests.



OK for google/main.



Committed rev 173187.


Diego.

http://codereview.appspot.com/4438079/

Re: Disable tracer by default for profile use (issue4428074)

[dnovillo]

On 2011/04/28 18:53:24, Diego Novillo wrote:


OK.


Committed rev 173186.


Diego.

http://codereview.appspot.com/4428074/

Re: [PATCH] Typed DWARF stack

[Jason Merrill]

On 04/16/2011 04:11 AM, Jakub Jelinek wrote:

+ case dw_val_class_const_double:
+   {
+ unsigned HOST_WIDE_INT first, second;
+ l = 2 * HOST_BITS_PER_WIDE_INT / HOST_BITS_PER_CHAR;
+
+ dw2_asm_output_data (1, l, NULL);
+ if (WORDS_BIG_ENDIAN)
+   {
+ first = val2->v.val_double.high;
+ second = val2->v.val_double.low;
+   }
+ else
+   {
+ first = val2->v.val_double.low;
+ second = val2->v.val_double.high;
+   }
+ dw2_asm_output_data (HOST_BITS_PER_WIDE_INT / HOST_BITS_PER_CHAR,
+  first, NULL);
+ dw2_asm_output_data (HOST_BITS_PER_WIDE_INT / HOST_BITS_PER_CHAR,
+  second, NULL);
+   }


How about moving the 2* into the first call to dw2_asm_output_data so 
that the later calls can just use 'l'?



+   unsigned r = val1->v.val_unsigned;
+   unsigned long o = get_base_type_offset (val2->v.val_die_ref.die);
+   if (for_eh_or_skip>= 0)
+ r = DWARF2_FRAME_REG_OUT (r, for_eh_or_skip);
+   gcc_assert (size_of_uleb128 (r)
+   == size_of_uleb128 (val1->v.val_unsigned)&&  o);


Maybe move this assert inside the if?


+  /* Sort by increasing usage count, as when readding the last


"reading"

OK with these changes.

Jason

Re: [google]Add support for sampled profile collection (issue4438083)

[Jan Hubicka]

Hi,
> + Honza
> 
> This patch may be a candidate for trunk as well. This feature not only
> allows profile collection with much less overhead (for multi-thread
> programs with hot regions, the slow down can be significant due to
> cache ping-pong effect of counter update) without sacrificing too much
> the performance.

Hmm, it is interesting idea.  
> 
> A known limitation is that value profiling is not yet sampled, but it
> does not seem to cause problems.

For the performance alone, we probably don't need to care that much
given the fact that we value porfile only relatively expensive operations.
But if we want to have the turn off/on feature, then i gueess we need to
guard everything.  It is not much of pain to add the code generating
conditionals everywhere, after all.
> > +@item -fprofile-generate-sampling
> > +@opindex -fprofile-generate-sampling
> > +
> > +Enable sampling for instrumented binaries.  Instead of recording every 
> > event,
> > +record only every N-th event, where N (the sampling rate) can be set either
> > +at compile time using
> > +@option{--param profile-generate-sampling-rate=@var{value}}, or
> > +at execution start time through environment variable 
> > @samp{GCOV_SAMPLING_RATE}.
> > +
> > +At this time sampling applies only to branch counters.  A sampling rate of 
> > 100
> > +decreases instrumentated binary slowdown from up to 20x for heavily 
> > threaded
> > +applications down to around 2x.  @option{-fprofile-correction} is always
> > +needed with sampling.

You probably want to make -fprofile-correction to be implied in this case. 
Perhaps even drop it into the gcov infos so we don't require -fprofile-generate
and -fprofile-use values to match here.

Also --param is generally intended for GCC developers, I would go with
-fprofile-generate-sampling=num.

Note that other way of getting sampling cost down is probably to apply the
estimated profile to minimal spanning tree construction.  It should put the 
counters
on less executed paths instead of often executed and at mainline we now 
construct
estimated profile before profiling (this is kind of ordering problem as for the
statistics we need to do otherwise, but I would be happy with extra statistics
pass that would be enabled only for my collecting script).

Option is also to have counters in automatic vars and flush them upon function
return (perhaps with the sampling rate trick and optional locking for full
thread safety)...  It might end up cheaper than adding many ifs everywhere.

> > Index: gcc/profile.h
> > ===
> > --- gcc/profile.h       (revision 173136)
> > +++ gcc/profile.h       (working copy)
> > @@ -47,4 +47,10 @@ extern gcov_type sum_edge_counts (VEC (edge, gc) *
> >  extern void init_node_map (void);
> >  extern void del_node_map (void);
> >
> > +/* Implement sampling to avoid writing to edge counters very often.
> > +   Many concurrent writes to the same counters, or to counters that share
> > +   the same cache line leads to up to 30x slowdown on an application 
> > running
> > +   on 8 CPUs.  With sampling, the slowdown reduced to 2x.  */
> > +extern void add_sampling_to_edge_counters (void);

The comment belong to place function is implemented.
> >
> > +fprofile-generate-sampling
> > +Common Var(flag_profile_generate_sampling)
> > +Turn on instrumentation sampling with -fprofile-generate with rate set by 
> > --param profile-generate-sampling-rate or environment variable 
> > GCOV_SAMPLING_RATE

I think -fprofile-generate and friends need some @ shuggar.  Probably @option.
> > Index: gcc/tree-profile.c
> > ===
> > --- gcc/tree-profile.c  (revision 173136)
> > +++ gcc/tree-profile.c  (working copy)
> > @@ -31,6 +31,8 @@ along with GCC; see the file COPYING3.  If not see
> >  #include "coretypes.h"
> >  #include "tm.h"
> >  #include "flags.h"
> > +#include "target.h"
> > +#include "output.h"
> >  #include "regs.h"
> >  #include "function.h"
> >  #include "basic-block.h"
> > @@ -44,9 +46,14 @@ along with GCC; see the file COPYING3.  If not see
> >  #include "value-prof.h"
> >  #include "cgraph.h"
> >  #include "output.h"
> > +#include "params.h"
> > +#include "profile.h"

For mainline, don't forget about dependencies.  Do we really need all of this?
> > +/* Insert STMT_IF around given sequence of consecutive statements in the
> > +   same basic block starting with STMT_START, ending with STMT_END.  */
> > +
> > +static void
> > +insert_if_then (gimple stmt_start, gimple stmt_end, gimple stmt_if)
> > +{
> > +  gimple_stmt_iterator gsi;
> > +  basic_block bb_original, bb_before_if, bb_after_if;
> > +  edge e_if_taken, e_then_join;
> > +
> > +  gsi = gsi_for_stmt (stmt_start);
> > +  gsi_insert_before (&gsi, stmt_if, GSI_SAME_STMT);
> > +  bb_original = gsi_bb (gsi);
> > +  e_if_taken = split_block (bb_original, stmt_if);
> > +  e_if_taken->flags &= ~EDGE_FALLTHRU;
> > +  e_if_taken->fl

Re: [google] Use R_ARM_GOT_PREL to simplify global address loading from GOT (issue4433079)

[Diego Novillo]

On Fri, Apr 29, 2011 at 01:24, Carrot Wei  wrote:

>>> http://codereview.appspot.com/4433079/diff/1/gcc/hooks.c#newcode287
>>> gcc/hooks.c:287: return NULL;
>>> +hook_rtx_void_null (void)
>>> +{
>>> +  return NULL;
>>>
>>> s/NULL/NULL_RTX/
>>>
>> done.
>>
> Oops. File hooks.c doesn't include rtl.h, so either rtl.h should be
> included, or return NULL instead, like other functions in this file.

OK, no problem.  Just use NULL then.  No point dragging a new file
just to get a slightly different version of 0.


Diego.

Re: [google][patch] Add new -gmlt option for min. debug info with line tables (issue4440072)

[Diego Novillo]

On Fri, Apr 29, 2011 at 16:36, Cary Coutant  wrote:
> I'd like to put this patch into google/main while it's being reviewed for 
> trunk.
>
> Original message: http://gcc.gnu.org/ml/gcc-patches/2011-04/msg02075.html
>
> Rietveld: http://codereview.appspot.com/4440072
>
> Bootstraps on x86_64, with testsuite output identical to a vanilla build.
>
> OK for google/main?

Yes.


Diego.

Re: [patch] Add new -gmlt option for min. debug info with line tables (issue4440072)

[Diego Novillo]

On Wed, Apr 27, 2011 at 16:24, Cary Coutant  wrote:
>>> set_debug_level should not use global state; this needs to check
>>> opts->x_debug_info_level (not the global debug_info_level) and set
>>> opts->x_generate_debug_line_table.
>>
>> Oops, missed that. Thanks!
>
> I've uploaded the revised patch to
> http://codereview.appspot.com/4440072 -- should I also post it here?

Yeah, not everyone on the list is using the web interface.


Diego.

[google][patch] Add new -gmlt option for min. debug info with line tables (issue4440072)

[Cary Coutant]

I'd like to put this patch into google/main while it's being reviewed for trunk.

Original message: http://gcc.gnu.org/ml/gcc-patches/2011-04/msg02075.html

Rietveld: http://codereview.appspot.com/4440072

Bootstraps on x86_64, with testsuite output identical to a vanilla build.

OK for google/main?

-cary

Re: [Patch, fortran] PR48462 - [4.6/4.7 Regression] realloc on assignment: matmul Segmentation Fault with Allocatable Array + PR48746

[Thomas Koenig]

Dear Paul,

first, thanks for the patch.

There is one thing it does not appear to do correctly: It should
also set the dtype on the variable itself:

ig25@linux-fd1f:~/Krempel/H> cat mm.f90
program main
  implicit none
  integer, parameter :: m=10, n=12, count=4
  double precision :: a(m, count), b(count, n), c(m, n)
  double precision, dimension(:,:), allocatable :: tmp

  call random_number(a)
  call random_number(b)
  tmp = matmul(a,b)
  print *,tmp
end program main
ig25@linux-fd1f:~/Krempel/H> gfortran -fdump-tree-original mm.f90
ig25@linux-fd1f:~/Krempel/H> ./a.out
At line 10 of file mm.f90 (unit = 6, file = 'stdout')
Internal Error: list_formatted_write(): Bad type

Apparently, the dtype of tmp is never set:

ig25@linux-fd1f:~/Krempel/H> grep tmp mm.f90.003t.original
  struct array2_real(kind=8) tmp;
  tmp.data = 0B;
D.1573 = tmp;
  D.1574 = (void *) tmp.data;
tmp.data = D.1573.data;
_gfortran_transfer_array_write (&dt_parm.4, &tmp, 8, 0);
  if (tmp.data != 0B)
  __builtin_free ((void *) tmp.data);
  tmp.data = 0B;

Regards,

Thomas

Re: [google] Add new warning -Wreal-conversion (issue4436068)

[Diego Novillo]

On 04/29/2011 11:02 AM, Nathan Froyd wrote:

On Fri, Apr 29, 2011 at 10:59:31AM -0400, Diego Novillo wrote:

* g++.dg/warn/Wreal-conversion-1.C: New.
* gcc.dg/Wreal-conversion-1.c: New.


Could a single copy of the test be placed in c-c++-common, instead?

-Nathan


Committed to google/main

2011-04-29  Diego Novillo  

* c-c++-common/Wreal-conversion-1.c: Move from gcc.dg.
* g++.dg/warn/Wreal-conversion-1.C: Remove.

diff --git a/gcc/testsuite/c-c++-common/Wreal-conversion-1.c 
b/gcc/testsuite/c-c++-common/Wreal-conversion-1.c

new file mode 100644
index 000..84e7293
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wreal-conversion-1.c
@@ -0,0 +1,29 @@
+// { dg-do compile }
+// { dg-options "-Wreal-conversion" }
+
+#include 
+
+int
+func1 (int a)
+{
+  double f = a;
+  return f;// { dg-warning "conversion to" }
+}
+
+double func3 ();
+
+void
+func2 ()
+{
+  double g = 3.2;
+  float f;
+  int t = g;   // { dg-warning "conversion to" }
+  int p;
+  p = f;   // { dg-warning "conversion to" }
+  func1 (g);   // { dg-warning "conversion to" }
+  char c = f;  // { dg-warning "conversion to" }
+  size_t s;
+  p = s;
+  int q;
+  q = func3 ();// { dg-warning "conversion to" }
+}
diff --git a/gcc/testsuite/g++.dg/warn/Wreal-conversion-1.C 
b/gcc/testsuite/g++.dg/warn/Wreal-conversion-1.C

deleted file mode 100644
index 2079486..000
--- a/gcc/testsuite/g++.dg/warn/Wreal-conversion-1.C
+++ /dev/null
@@ -1,30 +0,0 @@
-// { dg-do compile }
-// { dg-options "-Wreal-conversion" }
-
-#include 
-
-int
-func1 (int a)
-{
-  double f = a;
-  return f;// { dg-warning "conversion to" }
-}
-
-double func3 ();
-
-void
-func2 ()
-{
-  double g = 3.2;
-  float f;
-  int t = g;   // { dg-warning "conversion to" }
-  bool b = g;
-  int p;
-  p = f;   // { dg-warning "conversion to" }
-  func1 (g);   // { dg-warning "conversion to" }
-  char c = f;  // { dg-warning "conversion to" }
-  size_t s;
-  p = s;
-  int q;
-  q = func3 ();// { dg-warning "conversion to" }
-}
diff --git a/gcc/testsuite/gcc.dg/Wreal-conversion-1.c 
b/gcc/testsuite/gcc.dg/Wreal-conversion-1.c

deleted file mode 100644
index 84e7293..000
--- a/gcc/testsuite/gcc.dg/Wreal-conversion-1.c
+++ /dev/null
@@ -1,29 +0,0 @@
-// { dg-do compile }
-// { dg-options "-Wreal-conversion" }
-
-#include 
-
-int
-func1 (int a)
-{
-  double f = a;
-  return f;// { dg-warning "conversion to" }
-}
-
-double func3 ();
-
-void
-func2 ()
-{
-  double g = 3.2;
-  float f;
-  int t = g;   // { dg-warning "conversion to" }
-  int p;
-  p = f;   // { dg-warning "conversion to" }
-  func1 (g);   // { dg-warning "conversion to" }
-  char c = f;  // { dg-warning "conversion to" }
-  size_t s;
-  p = s;
-  int q;
-  q = func3 ();// { dg-warning "conversion to" }
-}

Re: [google] Check if the nonnull attribute is applied to 'this' (issue4446070)

[Basile Starynkevitch]

On Fri, 29 Apr 2011 12:32:24 -0700
Mike Stump  wrote:

> On Apr 29, 2011, at 11:46 AM, Basile Starynkevitch wrote:
> > 
> > This bring me to a question. Does the C++ standard imply that this is
> > never a null pointer?
> 
> Does this:
> 
> 4 Certain other operations are described in this International  Standard
>   as  undefined  (for  example,  the  effect  of  dereferencing the null
>   pointer).  [Note: this International Standard imposes no  requirements
>   on the behavior of programs that contain undefined behavior.  ]
> 
> answer your question?  
Not really.

> Note, this is exactly the same as C.  You can't do it in C either:
> 
> struct A {
>   int x;
> } *pa = 0;
> 
> int main() {
>   pa->x = 1;
> }
> 
> If you run it, it will crash on all good OSes.  In C++, it will also crash.

The examples I gave did not crash, because they are non-virtual member
functions. I gave as example

>   class Myclass {
>  int x;
>public:
>  Myclass(int z=0): x(z) {};
>  int get(void) const { if (this) return x; else return 0; };
>  ~Myclass() { };
>   };

Please notice that get above is a non-virtual member *function*

In the old days when C++ was a translator to C, the
generated C code would be something like

int Myclass__get (MyClass* this)
{ if (this) return this->x; else return 0; }

And such C code won't crash and is conforming to (old and current) C
standard[s].

My example did not dereference a null pointer! I did test some similar
code and on my Linux machine it did work as I expect.

So I still don't know if this can be null or not. After all, it is 
(in C++ parlance) a pointer, not a reference, and C++ pointers can be
null.

with gcc version 4.6.1 20110421 (prerelease) (Debian 4.6.0-5)
(Debian/Experimental on AMD64) the following file

/// file nullthis.cc
  class Myclass {
 int x;
   public:
 Myclass(int z=0): x(z) {};
 int get(void) const { if (this) return x; else return 0; };
 ~Myclass() { };
  };

extern "C" int myget (Myclass *c) { return c->get(); };
 eof nullthis.cc

is compiled with g++ -Wall -O3 -fverbose-asm -S nullthis.cc
without warnings, and the only produced function in nullthis.s is
.globl  myget
.type   myget, @function
myget:
.LFB7:
.cfi_startproc
xorl%eax, %eax  # D.2112
testq   %rdi, %rdi  # c
je  .L2 #,
movl(%rdi), %eax# MEM[(const struct Myclass
*)c_1(D)].x, D.2112 .L2:
rep
ret
.cfi_endproc
.LFE7:
.size   myget, .-myget
.ident  "GCC: (Debian 4.6.0-5) 4.6.1 20110421
(prerelease)"

This is exactly what I expect. But I don't know if it conforms to
standard.

When compiling with g++ -Wall -O3 -fdump-tree-all -fverbose-asm -S
nullthis.cc I have (among others) a dump file  
% cat nullthis.cc.131t.phiopt3  

;; Function int myget(Myclass*) (myget)

int myget(Myclass*) (struct Myclass * c)
{
  int D.2112;

:
  if (c_1(D) != 0B)
goto ;
  else
goto ;

:
  D.2112_4 = MEM[(const struct Myclass *)c_1(D)].x;

:
  # D.2112_5 = PHI <0(2), D.2112_4(3)>
  return D.2112_5;

}
which is also what I expect. I have no idea if it is conforming to
standards. But I notice that the test about this is remaining in the
code. So apparently GCC 4.6 does not make the hypothesis that this is
never null, otherwise it would have optimized the test by removing it.
What I don't know (and I am asking) is if such an hypothetical
optimization is conforming to standards. My biased feeling is that it
is not (because I found no explicit & unambigous mention in standard
legal text that this is never null).

Of course, calling a virtual member function on null is obviously an
undefined behavior (and SIGSEGV under Linux).

Regards.
-- 
Basile STARYNKEVITCH http://starynkevitch.net/Basile/
email: basilestarynkevitchnet mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***

Re: (build) Patch to fix cp/cfns.gperf building issues

[Nicola Pero]

Alexandre

thanks for the review :-)

> -pedantic review: how about outputting to a temporary file (say
> cp/cfns.hT) and only renaming to the intended name on success, so that,
> if gperf crashes or we reboot part-way through it, we don't end up with
> a partially-generated file that will seem to be up to date?

It's a good suggestion.

On the other hand, now that the rule is maintainer-only, and considering
it's used once every few years, I wouldn't personally want to spend more
time on it. ;-)

Thanks

Fix 64-bit libjava breakage on Solaris 2/SPARC (PR middle-end/48819)

[Rainer Orth]

This patch fixes 350 64-bit libjava execution failures on Solaris
2/SPARC.  It was provided and approved by Richard in the PR.

Tested by rebuilding libjava with the patched jc1 on
sparc-sun-solaris2.11.  All testsuite failures are now gone.

Installed on mainline.

Rainer


2011-04-29  Richard Guenther  

PR middle-end/48819
* constants.c (build_constants_constructor): Use ptr_type_node for
temp.

Index: gcc/java/constants.c
===
--- gcc/java/constants.c(revision 173182)
+++ gcc/java/constants.c(working copy)
@@ -1,6 +1,6 @@
 /* Handle the constant pool of the Java(TM) Virtual Machine.
Copyright (C) 1997, 1998, 1999, 2000, 2001, 2003, 2004, 2005, 2006,
-   2007, 2008, 2010  Free Software Foundation, Inc.
+   2007, 2008, 2010, 2011  Free Software Foundation, Inc.
 
 This file is part of GCC.
 
@@ -544,10 +544,7 @@
temp <<= ((POINTER_SIZE > 32) ? POINTER_SIZE - 32 : 0);
 
   CONSTRUCTOR_PREPEND_VALUE (t, get_tag_node 
(outgoing_cpool->tags[i]));
-  CONSTRUCTOR_PREPEND_VALUE (d,
- fold_convert (ptr_type_node, 
-   (build_int_cst (NULL_TREE,
-   temp;
+  CONSTRUCTOR_PREPEND_VALUE (d, build_int_cst (ptr_type_node, temp));
}
break;
 
-- 
-
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [google] Check if the nonnull attribute is applied to 'this' (issue4446070)

[Mike Stump]

On Apr 29, 2011, at 11:46 AM, Basile Starynkevitch wrote:
> On Fri, 29 Apr 2011 11:08:24 -0400 (EDT)
> dnovi...@google.com (Diego Novillo) wrote:
> 
>> This patch from Le-Chun Wu adds support to check whether a nonnull
>> attribute is applied to 'this' pointer for non-static methods.
> 
> This bring me to a question. Does the C++ standard imply that this is
> never a null pointer?

Does this:

4 Certain other operations are described in this International  Standard
  as  undefined  (for  example,  the  effect  of  dereferencing the null
  pointer).  [Note: this International Standard imposes no  requirements
  on the behavior of programs that contain undefined behavior.  ]

answer your question?  Note, this is exactly the same as C.  You can't do it in 
C either:

struct A {
  int x;
} *pa = 0;

int main() {
  pa->x = 1;
}

If you run it, it will crash on all good OSes.  In C++, it will also crash.

Re: [google] Check if the nonnull attribute is applied to 'this' (issue4446070)

[Diego Novillo]

On Fri, Apr 29, 2011 at 14:46, Basile Starynkevitch
 wrote:
> On Fri, 29 Apr 2011 11:08:24 -0400 (EDT)
> dnovi...@google.com (Diego Novillo) wrote:
>
>> This patch from Le-Chun Wu adds support to check whether a nonnull
>> attribute is applied to 'this' pointer for non-static methods.
>
> This bring me to a question. Does the C++ standard imply that this is
> never a null pointer? (Last time I looked into a C++ standard, I was
> not able to give a firm answer).

I didn't find a specific section either, but that's my understanding.
The this pointer can never be NULL.  Jason, Lawrence?


Diego.

Re: [google] Use different peeling parameters with available profile (issue4438079)

[dnovillo]

On 2011/04/29 00:02:40, singhai wrote:


2011-04-28  Sharad Singhai  



gcc/ChangeLog.google-main
* params.def: Add new parameters to control peeling.
* tree-ssa-loop-ivcanon.c (try_unroll_loop_completely): Use
different peeling parameters when profile feedback is available.
* loop-unroll.c (decide_peel_once_rolling): Ditto.
(decide_peel_completely): Ditto.
* doc/invoke.texi: Document new peeling parameters.



testsuite/ChangeLog.google-main
* gcc.dg/vect/O3-vect-pr34223.c: Add extra peeling
parameters.
* gcc.dg/vect/vect.exp: Allow reading flags in individual
tests.


OK for google/main.


Diego.

http://codereview.appspot.com/4438079/

Re: branch optimizations

[Mike Stump]

Someone pointed out a patch by Zdenek Dvorak which implements TSP-based basic 
block reordering:

   http://gcc.gnu.org/ml/gcc-patches/2004-03/msg02335.html

to me.  From the spec numbers presented, it looks enticing.  Zdenek, do you 
think this is still a reasonable direction to go?  If so, I'd like to ping it 
for review.  One thing I don't like about it is that it does static prediction, 
though, I'd be happy to alter it for dynamic prediction.  In my patch:

  http://gcc.gnu.org/ml/gcc/2011-04/msg00365.html

I do handle dynamic prediction (hidden behind the CHEAPER target macro), 
though, the math is machine independent, once you have the costs and the 
expected mis-prediction rate.

Did I read the numbers right, 4.7% across a fair bit of spec on x86?  That 
seems like a worthwhile type of number, yes?

Re: [PATCH] Fix switch conversion (PR tree-optimization/48809)

[Jeff Law]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 04/29/11 10:43, Jakub Jelinek wrote:
> Hi!
> 
> The following patch fixes a bug in tree-switch-conversion.c with
> signed index_expr's.  build_arrays would compute index_expr - range_min
> in index_expr's type and use that as index into CSWTCH.N array,
> which is wrong, because in this case index_expr 98 - (-62) computed
> in signed char type results in signed overflow and we end up
> loading from CSWTCH.2[-96].  Apparently for the bounds checking
> we perform the same index_expr - range_min computation, but in
> corresponding unsigned type.  This patch computes it just once in
> unsigned type, so that overflow isn't undefined.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk/4.6/4.5/4.4?
> 
> 2011-04-29  Jakub Jelinek  
> 
>   PR tree-optimization/48809
>   * tree-switch-conversion.c (build_arrays): Compute tidx in unsigned
>   type.
>   (gen_inbound_check): Don't compute index_expr - range_min in utype
>   again, instead reuse SSA_NAME initialized in build_arrays.
>   Remove two useless gsi_for_stmt calls.
> 
>   * gcc.c-torture/execute/pr48809.c: New test.
OK.

jeff
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJNuwpHAAoJEBRtltQi2kC71SAH/1VFBhEB7SXDEogMFx59LD1j
G1D+1DBL9qQEDRXMQi+aA6H8Y8WxfUDsUTQUD3rrg9HxZDEaUz55P46nc6dd/DXS
72tFm5OW5bucJbnHocxu+PeAedk8w0cmjxdnXA38ezPhqpeqUKK/ojhPPcGSErHH
7Z9YUdDHGbAcZACjvd1qTObfp4NUG0PTZ7d6+ZOsAt7RwxgECcUJuyJhZaQdxZVV
MTF96cNa205xqOIBYwGCyW5bvFhgL4kDvdUMSln2CURXyl/N+JqeGnuGEElItM+A
8ryLWBIyzuUFHNML6FK6wrROs/QPiUdan2RqDyW46WT8UdE3PgVakT/7c9E42sc=
=395m
-END PGP SIGNATURE-

Re: [google] Check if the nonnull attribute is applied to 'this' (issue4446070)

[Basile Starynkevitch]

On Fri, 29 Apr 2011 11:08:24 -0400 (EDT)
dnovi...@google.com (Diego Novillo) wrote:

> This patch from Le-Chun Wu adds support to check whether a nonnull
> attribute is applied to 'this' pointer for non-static methods.

This bring me to a question. Does the C++ standard imply that this is
never a null pointer? (Last time I looked into a C++ standard, I was
not able to give a firm answer).

In other words, with 
  class Myclass {
 int x;
   public:
 Myclass(int z=0): x(z) {};
 int get(void) const { if (this) return x; else return 0; };
 ~Myclass() { };
  };
can a C++ standard conforming compiler optimize the get () member
function to 
  inline int Myclass::get(void) { return x; };
(I tend to believe that such an optimization is incorrect, but I am not
sure). can a C++ program call
   Myclass *t = null_ptr;
   return t->get();
(I might believe this is not conforming to standards)

I feel that testing that this is not null is occasionnally useful. For
instance, if I coded a trivial interpreter (e.g. of a Lua or Python or
Lisp-like language) I would be tempted, if that language has a nil
value, to implement values as pointers to objects and to represent nil
as a C++ null_ptr. So I might even want to code something like

// my abstract top superclass for values
class Value {
  virtual ~Value() {};
  Value () {};
  virtual void do_print(void) const = 0;
public:
  void print(void) const { if (this) this->do_print(); };
};

class IntegerValue : public Value {
  int x;
public:
  IntegerValue (int z) : Value(), x(z) {}:
  virtual void do_print (void) { cout << x; };
};

But I am not sure that such code is standard C++. However, GCC seems to
compile it as I expect! At least I still hope that future versions of
GCC would accept it (perhaps in some permissive mode, or with warnings)

Regards.

PS: By standard C++, I mean some recent or future C++ ISO standard (eg C++1x or 
C++0x).

-- 
Basile STARYNKEVITCH http://starynkevitch.net/Basile/
email: basilestarynkevitchnet mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***

Re: [google]Add support for sampled profile collection (issue4438083)

[Mike Stump]

On Apr 29, 2011, at 9:12 AM, Silvius Rus wrote:
>  When you build with
> -fprofile-generate=./fdoprof, the output gets dumped under
> ./fdoprof/..., but there seems to be no easy way to know this path
> within the profiling process.  For Google servers this makes
> collection fragile.  The user generally does not have access to the
> server file system.  I implemented a collection mechanism so the user
> can tell the server "copy the profiles from ./fdoprof to some shared
> location".  But now you need to pass the exact path when building
> *and* collecting profiles, which may get separated in time, thus the
> process is fragile.  It would be good to keep a copy of the path
> prefix and have it accessible through a public interface as well.
> Then once an instrumented binary is built, it will know the root of
> the profile output directory and thus relieve the user from the
> responsibility of knowing it.

So, does GCOV_PREFIX_STRIP and GCOV_PREFIX help you obtain
more certainty and reliability?

Re: [google] Add new warning -Wreal-conversion (issue4436068)

[Diego Novillo]

On Fri, Apr 29, 2011 at 11:02, Nathan Froyd  wrote:
> On Fri, Apr 29, 2011 at 10:59:31AM -0400, Diego Novillo wrote:
>>       * g++.dg/warn/Wreal-conversion-1.C: New.
>>       * gcc.dg/Wreal-conversion-1.c: New.
>
> Could a single copy of the test be placed in c-c++-common, instead?

Good point.  Will fix.


Diego.

Re: libgo patch committed: Inherit environment in http/cgi

[Ian Lance Taylor]

Rainer Orth  writes:

>> This libgo patch brings over a patch to the master Go library to inherit
>> environment variables in http/cgi.  This should fix PR go/48503.
>
> not really :-)  It needs the following supplement to handle Solaris and
> IRIX.  I'm not only including LD_LIBRARY_PATH (although this would suffice
> for the testcase to pass), but also the ABI variants thereof.

Thanks.  Committed.

Ian

Re: [google] Patch to support calling multi-versioned functions via new GCC builtin. (issue4440078)

[Sriraman Tallam]

Hi Richard,

  Thanks for the comments. Please find inline responses.

On Fri, Apr 29, 2011 at 1:56 AM, Richard Guenther
 wrote:
>
> On Fri, Apr 29, 2011 at 4:52 AM, Sriraman Tallam  wrote:
> > I want this patch to be considered for google/main now. This is intended to 
> > be submitted to trunk for review soon.
> > This patch has been tested with crosstool bootstrap using buildit and by 
> > running all tests.
> >
> >
> > Patch Description :
> > ==
> >
> > Frequently executed functions in programs are some times compiled
> > into different versions to take advantage of some specific
> > advanced features present in some of the types of platforms on
> > which the program is executed. For instance, SSE4 versus no-SSE4.
> > Existing techniques to do multi-versioning, for example using
> > indirect calls, block compiler optimizations, mainly inlining.
>
> The general idea of supporting versioning is good, thanks for working on it.
> Comments below.
>
> > This patch adds a new GCC pass to support multi-versioned calls
> > through a new builtin, __builtin_dispatch.  The __builtin_dispatch
> > call is converted into a simple if-then construct to call the
> > appropriate version at run-time. There are no indirect calls so
> > inlining is not affected.
>
> I am not sure that combining the function choice and its invocation
> to a single builtin is good.  First, you use variadic arguments for
> the actual function arguments which will cause vararg promotions
> to apply and thus it will be impossible to dispatch to functions
> taking single precision float values (and dependent on ABI details
> many more similar issues).  Second, you restrict yourself to
> only two versions of a function - that looks quite limited and this
> restriction is not easy to lift with your proposed interface.
>
> Thus, I would have kept regular (but indirect) calls in the source
> program and only exposed the dispatch via a builtin, like ...
>
> > This patch also supports a function unswitching optimization via
> > cloning of functions, only along hot paths, that call multi-versioned
> > functions so that the hot multi-versioned paths can be independently
> > optimized for performance. The cloning optimization is switched on
> > via a flag, -fclone-hot-version-paths.
> >
> > Only two versions are supported in this patch. Example use :
> >
> >  int popcnt_sse4(unsigned int x) __attribute__((__target__("popcnt")));
> >  int popcnt_sse4(unsigned int x)
> >  {
> >    int count = __builtin_popcount(x);
> >    return count;
> >  }
> >
> >  int popcnt(unsigned int x) __attribute__((__target__("no-popcnt")));
> >  int popcnt(unsigned int x)
> >  {
> >    int count = __builtin_popcount(x);
> >    return count;
> >  }
> >
> >  int testsse() __attribute__((version_selector));
> >  int main ()
> >  {
> >    ...
> >    /* The multi-versioned call. */
> >    ret = __builtin_dispatch (testsse,  (void*)popcnt_sse4, (void*)popcnt, 
> > 25);
> >    ...
> >  }
>
> int main()
> {
>  int (*ppcnt)(unsigned int) = __builtin_dispatch (testsse,
> popcnt_sse4, popcnt);
>  ret = (*ppcnt) (25);
> }
>
> which would allow the testsse function to return the argument number of
> the function to select.
>
> [snip]
>
> >  When to use the "version_selector" attribute ?
> >  ---
> >
> >  Functions are marked with attribute "version_selector" only if
> >  they are run-time constants.  Example of such functions would
> >  be those that test if a specific feature is available on a
> >  particular architecture.  Such functions must return a positive
> >  integer. For two-way functions, those that test if a feature
> >  is present or not must return 1 or 0 respectively.
> >
> >  This patch will make constructors that call these function once
> >  and assign the outcome to a global variable. From then on, only
> >  the value of the global will be used to decide which version to
> >  execute.
>
> That's a nice idea, but why not simply process all functions with
> a const attribute and no arguments this way?  IMHO
>
> int testsse (void) __attribute__((const));
>
> is as good and avoids the new attribute completely.  The pass would
> also benefit other uses of this idiom (giving a way to have global
> dynamic initializers in C).  The functions may not be strictly 'const'
> in a way the compiler autodetects this attribute but it presents
> exactly the promises to the compiler that allow this optimization.

Since, const functions cannot, not according to the definition atleast
,  read files or memory, I was thinking along the lines of making it a
 "pure" function (For example, The version_selector function could be
reading "/proc/cpuinfo" and looking for certain features). The reason
I invented a new attribute was because I assumed it is not legal to
hoist the call site of a pure function into a constructor and
substitute the value computed simply because it may not be valid to
call the pure function ahead of the intended time

Re: [google] Check if the nonnull attribute is applied to 'this' (issue4446070)

[dnovillo]

On 2011/04/29 15:12:52, richard.guenther_gmail.com wrote:


>
> +



spurious white-space change.


Thanks.  Fixed.


Diego.

http://codereview.appspot.com/4446070/

Re: [google]Add support for sampled profile collection (issue4438083)

[Easwaran Raman]

On Fri, Apr 29, 2011 at 9:12 AM, Silvius Rus  wrote:
>> How is code-size affected with this patch, non-instrumented vs.
>> regular-instrumented vs. sample-instrumented?
>
> I don't have the numbers, but the increase in code size from
> regular-instrumented to sample-instrumented is larger than that from
> non-instrumented to sample-instrumented.  Easwaran, could you please
> build a few binaries at -O2, -O2 -fprofile-generate and -O2
> -fprofile-generate -fprofile-generate-sampling, and send out the text
> size ratios considering -O2 as the baseline?
>
>> > @@ -292,6 +474,15 @@ gimple_gen_edge_profiler (int edgeno, edge e)
>> >                                        gimple_assign_lhs (stmt1), one);
>> >   gimple_assign_set_lhs (stmt2, make_ssa_name (gcov_type_tmp_var, stmt2));
>> >   stmt3 = gimple_build_assign (unshare_expr (ref), gimple_assign_lhs 
>> > (stmt2));
>> > +
>> > +  if (flag_profile_generate_sampling)
>> > +    {
>> > +      slot = htab_find_slot_with_hash (instrumentation_to_be_sampled, 
>> > stmt1,
>> > +                                       htab_hash_pointer (stmt1), INSERT);
>> > +      gcc_assert (!*slot);
>> > +      *slot = stmt1;
>> > +    }
>> > +
>>
>> What's the reason to delay the sampling transform and not apply it here?
>> At least a comment should be added for this.
>
> Sorry, I just don't remember.

Is this because altering the CFG while iterating over the CFG edges in
instrument_edges (profile.c) is not safe?

-Easwaran


>> > +DEFPARAM (PARAM_PROFILE_GENERATE_SAMPLING_RATE,
>> > +         "profile-generate-sampling-rate",
>> > +         "sampling rate with -fprofile-generate-sampling",
>> > +         100, 0, 20)
>>
>> Zero is really ok?
>
> It wouldn't break anything, but a rate of 0 doesn't make sense.  It
> should be 1.  And the default should be 101 instead of 100 (and
> generally a prime).
>
> While we're at it, let me bring up an important practical FDO issue
> related to this.  As David mentioned, besides reducing overhead we
> have used this to implement selective collection.  This essentially
> requires libgcov to provide a basic public interface:
> reset()
> start()
> stop()
> save()
> I implemented them by playing with the sampling rate, but a clearer
> and supported interface would be useful.  Also, there was one annoying
> detail that I could not figure out.  When you build with
> -fprofile-generate=./fdoprof, the output gets dumped under
> ./fdoprof/..., but there seems to be no easy way to know this path
> within the profiling process.  For Google servers this makes
> collection fragile.  The user generally does not have access to the
> server file system.  I implemented a collection mechanism so the user
> can tell the server "copy the profiles from ./fdoprof to some shared
> location".  But now you need to pass the exact path when building
> *and* collecting profiles, which may get separated in time, thus the
> process is fragile.  It would be good to keep a copy of the path
> prefix and have it accessible through a public interface as well.
> Then once an instrumented binary is built, it will know the root of
> the profile output directory and thus relieve the user from the
> responsibility of knowing it.
>
> Silvius
>

[google] Add missing reference numbers to ChangeLogs

[Diego Novillo]

I forgot to update the ChangeLog entries with the reference
numbers in my previous patches.

Committed to google/main.


Diego.


diff --git a/gcc/ChangeLog.google-main b/gcc/ChangeLog.google-main
index 771435b..dfebff2 100644
--- a/gcc/ChangeLog.google-main
+++ b/gcc/ChangeLog.google-main
@@ -19,6 +19,8 @@
 
 2011-04-27  Le-Chun Wu  
 
+   Google ref 42718.
+
* doc/extend.texi (Wnonnull): Add documentation for C++.
* doc/invoke.texi (Wnonnull): Add documentation for C++.
 
@@ -29,6 +31,8 @@
 
 2011-04-27  Le-Chun Wu  
 
+   Google ref 39127.
+
* c-decl.c (warn_if_shadowing): Use the warning code corresponding
to the given -Wshadow- variant.
* common.opt (Wshadow-local): New option.
@@ -39,6 +43,8 @@
 
 2011-04-27  Silvius Rus  
 
+   Google ref 39984.
+
* doc/invoke.texi (fno-strict-enum-precision): Document.
* gimplify.c (gimplify_switch_expr): If
-fno-strict-enum-precision is given, do not consider enum
diff --git a/gcc/c-family/ChangeLog.google-main 
b/gcc/c-family/ChangeLog.google-main
index a6a66e0..c328b63 100644
--- a/gcc/c-family/ChangeLog.google-main
+++ b/gcc/c-family/ChangeLog.google-main
@@ -1,14 +1,20 @@
 2011-04-27  Le-Chun Wu  
 
+   Google ref 45339.
+
* c-common.c (handle_nonnull_attribute): Check whether the nonnull
attribute is applied to the 'this' pointer for non-static methods.
 
 2011-04-27  Le-Chun Wu  
 
+   Google ref 42718.
+
* c.opt (Wnonnull): Enable for C++.
 
 2011-04-27  Le-Chun Wu  
 
+   Google ref 39133.
+
* c.opt (Wreal-conversion): New flag.
* c-common.c (conversion_warning): Use it.
* c-opts.c (c_common_post_options): Initialize it.
diff --git a/gcc/cp/ChangeLog.google-main b/gcc/cp/ChangeLog.google-main
index eb35c74..607fd72 100644
--- a/gcc/cp/ChangeLog.google-main
+++ b/gcc/cp/ChangeLog.google-main
@@ -1,17 +1,23 @@
 2011-04-28  Diego Novillo  
Le-Chun Wu  
 
+   Google ref 40484.
+
* call.c (conversion_null_warnings): Also handle assignments
when warning about NULL conversions.
 
 2011-04-27  Le-Chun Wu  
 
+   Google ref 39127.
+
* name-lookup.c (pushdecl_maybe_friend): When emitting a
shadowing warning, use the code corresponding to the
given -Wshadow- variant.
 
 2011-04-27  Le-Chun Wu  
 
+   Google ref 40484.
+
* cp-tree.h (LOOKUP_EXPLICIT_TMPL_ARGS): Define.
* call.c (build_new_function_call): Set it for TEMPLATE_ID_EXPRs.
(build_over_call): Use it to determine whether to emit a NULL
diff --git a/gcc/testsuite/ChangeLog.google-main 
b/gcc/testsuite/ChangeLog.google-main
index 3a76097..1518b7b 100644
--- a/gcc/testsuite/ChangeLog.google-main
+++ b/gcc/testsuite/ChangeLog.google-main
@@ -1,3 +1,10 @@
+2011-04-29  Diego Novillo  
+
+   Google ref 40484.
+
+   * g++.old-deja/g++.other/null3.C: Expect warning about
+   converting boolean to a pointer.
+
 2011-04-28  Diego Novillo  
 
* g++.dg/other/no-strict-enum-precision-1.C: Re-indent using GNU style.
@@ -15,6 +22,8 @@
 
 2011-04-28  Le-Chun Wu  
 
+   Google ref 40484.
+
* g++.dg/warn/Wconversion-null-2.C: Do not expect a NULL
  warning in implicitly instantiated templates.
 
@@ -54,19 +63,27 @@

 2011-04-27  Le-Chun Wu  
 
+   Google ref 45339.
+
* g++.dg/warn/nonnull2.C: New.
 
 2011-04-27  Le-Chun Wu  
 
+   Google ref 42718.
+
* g++.dg/warn/Wnonnull-1.C: New.
 
 2011-04-27  Le-Chun Wu  
 
+   Google ref 39133.
+
* g++.dg/warn/Wreal-conversion-1.C: New.
* gcc.dg/Wreal-conversion-1.c: New.
 
 2011-04-27  Le-Chun Wu  
 
+   Google ref 39127.
+
* g++.dg/warn/Wshadow-compatible-local-1.C: New.
* g++.dg/warn/Wshadow-local-1.C: New.
* g++.dg/warn/Wshadow-local-2.C: New.
@@ -77,11 +94,15 @@
 
 2011-04-27  Le-Chun Wu  
 
+   Google ref 40484.
+
* g++.dg/warn/Wnull-conversion-1.C: New.
* g++.dg/warn/Wnull-conversion-2.C: New.
 
 2011-04-27  Silvius Rus  
 
+   Google ref 39984.
+
* g++.dg/other/no-strict-enum-precision-1.C: New.
* g++.dg/other/no-strict-enum-precision-2.C: New.
* g++.dg/other/no-strict-enum-precision-3.C: New.

Re: [Patch, fortran] PR48462 - [4.6/4.7 Regression] realloc on assignment: matmul Segmentation Fault with Allocatable Array + PR48746

[Steve Kargl]

On Fri, Apr 29, 2011 at 06:55:11PM +0200, Paul Richard Thomas wrote:
> Dear All,
> 
> These are both quite trivial fixes and can be understood from
> ChangeLogs and comments in the patch.
> 
> Bootstrapped and regtested on FC9/x86_64 - OK for trunk and 4.6?

OK for both.

-- 
Steve

[Patch, fortran] PR48462 - [4.6/4.7 Regression] realloc on assignment: matmul Segmentation Fault with Allocatable Array + PR48746

[Paul Richard Thomas]

Dear All,

These are both quite trivial fixes and can be understood from
ChangeLogs and comments in the patch.

Bootstrapped and regtested on FC9/x86_64 - OK for trunk and 4.6?

Cheers

Paul

2011-04-29  Paul Thomas  

PR fortran/48462
* trans-expr.c (arrayfunc_assign_needs_temporary): Deal with
automatic reallocation when the lhs is a target.

PR fortran/48462
* trans-expr.c (fcncall_realloc_result): Make sure that the
result dtype field is set before the function call.

2011-04-29  Paul Thomas  

PR fortran/48462
* gfortran.dg/realloc_on_assign_7.f03: Modify to test for lhs
being a target.

PR fortran/48746
* gfortran.dg/realloc_on_assign_7.f03: Add subroutine pr48746.
Index: gcc/fortran/trans-expr.c
===
*** gcc/fortran/trans-expr.c	(revision 173130)
--- gcc/fortran/trans-expr.c	(working copy)
*** arrayfunc_assign_needs_temporary (gfc_ex
*** 5444,5452 
  return true;
  
/* If we have reached here with an intrinsic function, we do not
!  need a temporary.  */
if (expr2->value.function.isym)
! return false;
  
/* If the LHS is a dummy, we need a temporary if it is not
   INTENT(OUT).  */
--- 5444,5455 
  return true;
  
/* If we have reached here with an intrinsic function, we do not
!  need a temporary except in the particular case that reallocation
!  on assignment is active and the lhs is allocatable and a target.  */
if (expr2->value.function.isym)
! return (gfc_option.flag_realloc_lhs
! 	  && sym->attr.allocatable
! 	  && sym->attr.target);
  
/* If the LHS is a dummy, we need a temporary if it is not
   INTENT(OUT).  */
*** fcncall_realloc_result (gfc_se *se)
*** 5547,5552 
--- 5550,5558 
desc = build_fold_indirect_ref_loc (input_location, se->expr);
res_desc = gfc_evaluate_now (desc, &se->pre);
gfc_conv_descriptor_data_set (&se->pre, res_desc, null_pointer_node);
+   /* Unallocated, the descriptor does not have a dtype.  */
+   tmp = gfc_conv_descriptor_dtype (res_desc);
+   gfc_add_modify (&se->pre, tmp, gfc_get_dtype (TREE_TYPE (desc)));
se->expr = gfc_build_addr_expr (TREE_TYPE (se->expr), res_desc);
  
/* Free the lhs after the function call and copy the result data to
*** fcncall_realloc_result (gfc_se *se)
*** 5556,5565 
gfc_add_expr_to_block (&se->post, tmp);
tmp = gfc_conv_descriptor_data_get (res_desc);
gfc_conv_descriptor_data_set (&se->post, desc, tmp);
- 
-   /* Unallocated, the descriptor does not have a dtype.  */
-   tmp = gfc_conv_descriptor_dtype (desc);
-   gfc_add_modify (&se->post, tmp, gfc_get_dtype (TREE_TYPE (desc)));
  }
  
  
--- 5562,5567 
Index: gcc/testsuite/gfortran.dg/realloc_on_assign_7.f03
===
*** gcc/testsuite/gfortran.dg/realloc_on_assign_7.f03	(revision 173130)
--- gcc/testsuite/gfortran.dg/realloc_on_assign_7.f03	(working copy)
***
*** 1,6 
--- 1,8 
  ! { dg-do run }
  ! Check the fix for PR48462 in which the assignments involving matmul
  ! seg faulted because a was automatically freed before the assignment.
+ ! Since it is related, the test for the fix of PR48746 has been added
+ ! as a subroutine by that name.
  !
  ! Contributed by John Nedney  
  !
*** program main
*** 8,30 
implicit none
integer, parameter :: dp = kind(0.0d0)
real(kind=dp), allocatable :: delta(:,:)

call foo
call bar
  contains
  !
  ! Original reduced version from comment #2
subroutine foo
  implicit none
- real(kind=dp), allocatable :: a(:,:)
  real(kind=dp), allocatable :: b(:,:)
  
- allocate(a(3,3))
  allocate(b(3,3))
  allocate(delta(3,3))
  
- b = reshape ([1d0, 0d0, 0d0, 0d0, 1d0, 0d0, 0d0, 0d0, 1d0], [3,3])
  a = reshape ([1d0, 2d0, 3d0, 4d0, 5d0, 6d0, 7d0, 8d0, 9d0], [3,3])
  
  a = matmul( matmul( a, b ), b )
  delta = (a - reshape ([1d0, 2d0, 3d0, 4d0, 5d0, 6d0, 7d0, 8d0, 9d0], [3,3]))**2
--- 10,41 
implicit none
integer, parameter :: dp = kind(0.0d0)
real(kind=dp), allocatable :: delta(:,:)
+   real(kind=dp), allocatable, target :: a(:,:)
+   real(kind=dp), pointer :: aptr(:,:)
+ 
+   allocate(a(3,3))
+   aptr => a

call foo
+   if (.not. associated (aptr, a)) call abort () ! reallocated to same size - remains associated
call bar
+   if (.not. associated (aptr, a)) call abort () ! reallocated to smaller size - remains associated
+   call foobar
+   if (associated (aptr, a)) call abort () ! reallocated to larger size - disassociates
+ 
+   call pr48746
  contains
  !
  ! Original reduced version from comment #2
subroutine foo
  implicit none
  real(kind=dp), allocatable :: b(:,:)
  
  allocate(b(3,3))
  allocate(delta(3,3))
  
  a = reshape ([1d0, 2d0, 3d0,

[PATCH] Fix switch conversion (PR tree-optimization/48809)

[Jakub Jelinek]

Hi!

The following patch fixes a bug in tree-switch-conversion.c with
signed index_expr's.  build_arrays would compute index_expr - range_min
in index_expr's type and use that as index into CSWTCH.N array,
which is wrong, because in this case index_expr 98 - (-62) computed
in signed char type results in signed overflow and we end up
loading from CSWTCH.2[-96].  Apparently for the bounds checking
we perform the same index_expr - range_min computation, but in
corresponding unsigned type.  This patch computes it just once in
unsigned type, so that overflow isn't undefined.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk/4.6/4.5/4.4?

2011-04-29  Jakub Jelinek  

PR tree-optimization/48809
* tree-switch-conversion.c (build_arrays): Compute tidx in unsigned
type.
(gen_inbound_check): Don't compute index_expr - range_min in utype
again, instead reuse SSA_NAME initialized in build_arrays.
Remove two useless gsi_for_stmt calls.

* gcc.c-torture/execute/pr48809.c: New test.

--- gcc/tree-switch-conversion.c.jj 2010-12-02 11:51:32.0 +0100
+++ gcc/tree-switch-conversion.c2011-04-29 15:23:57.0 +0200
@@ -1,6 +1,6 @@
 /* Switch Conversion converts variable initializations based on switch
statements to initializations from a static array.
-   Copyright (C) 2006, 2008, 2009, 2010 Free Software Foundation, Inc.
+   Copyright (C) 2006, 2008, 2009, 2010, 2011 Free Software Foundation, Inc.
Contributed by Martin Jambor 
 
 This file is part of GCC.
@@ -656,7 +656,7 @@ static void
 build_arrays (gimple swtch)
 {
   tree arr_index_type;
-  tree tidx, sub, tmp;
+  tree tidx, sub, tmp, utype;
   gimple stmt;
   gimple_stmt_iterator gsi;
   int i;
@@ -664,14 +664,20 @@ build_arrays (gimple swtch)
 
   gsi = gsi_for_stmt (swtch);
 
+  /* Make sure we do not generate arithmetics in a subrange.  */
+  utype = TREE_TYPE (info.index_expr);
+  if (TREE_TYPE (utype))
+utype = lang_hooks.types.type_for_mode (TYPE_MODE (TREE_TYPE (utype)), 1);
+  else
+utype = lang_hooks.types.type_for_mode (TYPE_MODE (utype), 1);
+
   arr_index_type = build_index_type (info.range_size);
-  tmp = create_tmp_var (TREE_TYPE (info.index_expr), "csti");
+  tmp = create_tmp_var (utype, "csui");
   add_referenced_var (tmp);
   tidx = make_ssa_name (tmp, NULL);
-  sub = fold_build2_loc (loc, MINUS_EXPR,
-TREE_TYPE (info.index_expr), info.index_expr,
-fold_convert_loc (loc, TREE_TYPE (info.index_expr),
-  info.range_min));
+  sub = fold_build2_loc (loc, MINUS_EXPR, utype,
+fold_convert_loc (loc, utype, info.index_expr),
+fold_convert_loc (loc, utype, info.range_min));
   sub = force_gimple_operand_gsi (&gsi, sub,
  false, NULL, true, GSI_SAME_STMT);
   stmt = gimple_build_assign (tidx, sub);
@@ -780,12 +786,7 @@ gen_inbound_check (gimple swtch)
   tree label_decl2 = create_artificial_label (UNKNOWN_LOCATION);
   tree label_decl3 = create_artificial_label (UNKNOWN_LOCATION);
   gimple label1, label2, label3;
-
-  tree utype;
-  tree tmp_u_1, tmp_u_2, tmp_u_var;
-  tree cast;
-  gimple cast_assign, minus_assign;
-  tree ulb, minus;
+  tree utype, tidx;
   tree bound;
 
   gimple cond_stmt;
@@ -799,49 +800,24 @@ gen_inbound_check (gimple swtch)
   gcc_assert (info.default_values);
   bb0 = gimple_bb (swtch);
 
-  /* Make sure we do not generate arithmetics in a subrange.  */
-  if (TREE_TYPE (TREE_TYPE (info.index_expr)))
-utype = lang_hooks.types.type_for_mode
-  (TYPE_MODE (TREE_TYPE (TREE_TYPE (info.index_expr))), 1);
-  else
-utype = lang_hooks.types.type_for_mode
-  (TYPE_MODE (TREE_TYPE (info.index_expr)), 1);
+  tidx = gimple_assign_lhs (info.arr_ref_first);
+  utype = TREE_TYPE (tidx);
 
   /* (end of) block 0 */
   gsi = gsi_for_stmt (info.arr_ref_first);
-  tmp_u_var = create_tmp_var (utype, "csui");
-  add_referenced_var (tmp_u_var);
-  tmp_u_1 = make_ssa_name (tmp_u_var, NULL);
-
-  cast = fold_convert_loc (loc, utype, info.index_expr);
-  cast_assign = gimple_build_assign (tmp_u_1, cast);
-  SSA_NAME_DEF_STMT (tmp_u_1) = cast_assign;
-  gsi_insert_before (&gsi, cast_assign, GSI_SAME_STMT);
-  update_stmt (cast_assign);
-
-  ulb = fold_convert_loc (loc, utype, info.range_min);
-  minus = fold_build2_loc (loc, MINUS_EXPR, utype, tmp_u_1, ulb);
-  minus = force_gimple_operand_gsi (&gsi, minus, false, NULL, true,
-   GSI_SAME_STMT);
-  tmp_u_2 = make_ssa_name (tmp_u_var, NULL);
-  minus_assign = gimple_build_assign (tmp_u_2, minus);
-  SSA_NAME_DEF_STMT (tmp_u_2) = minus_assign;
-  gsi_insert_before (&gsi, minus_assign, GSI_SAME_STMT);
-  update_stmt (minus_assign);
+  gsi_next (&gsi);
 
   bound = fold_convert_loc (loc, utype, info.range_size);
-  cond_stmt = gimple_build_cond (LE_EXPR, tmp_u_2, bound, NULL_TREE, 
NULL_TREE);
+  cond_stmt

Re: Toplevel cleanup: disable Java when libffi not supported

[Ralf Corsepius]

On 04/29/2011 06:26 PM, Tom Tromey wrote:

"Joseph" == Joseph S Myers  writes:


Joseph>  This patch, relative to a tree with
Joseph>    applied,
Joseph>  continues the cleanup of toplevel cases relating to disabling Java or
Joseph>  Java libraries by arranging for Java to be disabled (via
Joseph>  unsupported_languages) on targets not supporting libffi

It does make sense to build libjava without libffi.

... and it does make sense to build libffi without libjava, e.g. for libgo.


There is also --without-libffi, though; I didn't look to see what your
patch does with this.


--with/without-libffi is trouble zone of its own.
At least the libgo part in gcc-4.6 doesn't work at all:
c.f. http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48407

Ralf

Re: Toplevel cleanup: disable Java when libffi not supported

[Tom Tromey]

> "Joseph" == Joseph S Myers  writes:

Joseph> This patch, relative to a tree with
Joseph>  applied,
Joseph> continues the cleanup of toplevel cases relating to disabling Java or
Joseph> Java libraries by arranging for Java to be disabled (via
Joseph> unsupported_languages) on targets not supporting libffi

It does make sense to build libjava without libffi.

It just means that libjava has somewhat degraded functionality -- libffi
is needed for the interpreter, for JNI, and for some kinds of reflection
(Method.invoke).

I am not sure if there are any existing targets where libjava works but
libffi does not.  Looking at libjava/configure.host, maybe `arm*-elf'
(but maybe configure.host is out of data, it is certainly possible).

There is also --without-libffi, though; I didn't look to see what your
patch does with this.

A better gate for libjava might be boehm-gc.  It is required to run any
sort of real program.

Joseph> The ideal would be for information about libffi-supported targets to
Joseph> come from a configure fragment in the libffi/ directory, as per item 4
Joseph> in .

This would help libjava's configury as well.

Tom

Re: [google] Patch to support calling multi-versioned functions via new GCC builtin. (issue4440078)

[Xinliang David Li]

Here is the background for this feature:

1) People relies on function multi-version to explore hw features and
squeeze performance, but there is no standard ways of doing so, either
a) using indirect function calls with function pointers set at program
initialization; b) using manual dispatch at each callsite; b) using
features like IFUNC.  The dispatch mechanism needs to be promoted to
the language level and becomes the first class citizen;
2) Most importantly, the above non standard approaches block
interprocedural optimizations such as inlining. With the introduction
of buitlin_dispatch and the clear semantics, compiler can do more
aggressive optimization.

Multi-way dispatch will be good, but most of the use cases we have
seen is 2-way -- a fast path + a default path.

Who are the targeted consumer of the feature?

1) For developers who like to MV function manually;
2) For user directed function cloning

   e.g,
int foo (...) __attribute__((clone ("sse")):

3) Automatic function cloning determined by compiler.


>
> I am not sure that combining the function choice and its invocation
> to a single builtin is good.  First, you use variadic arguments for
> the actual function arguments which will cause vararg promotions
> to apply and thus it will be impossible to dispatch to functions
> taking single precision float values (and dependent on ABI details
> many more similar issues).  Second, you restrict yourself to
> only two versions of a function - that looks quite limited and this
> restriction is not easy to lift with your proposed interface.

See above. Multi-way dispatch can be added similarly.


>
> That's a nice idea, but why not simply process all functions with
> a const attribute and no arguments this way?  IMHO
>
> int testsse (void) __attribute__((const));
>
> is as good and avoids the new attribute completely.  The pass would
> also benefit other uses of this idiom (giving a way to have global
> dynamic initializers in C).  The functions may not be strictly 'const'
> in a way the compiler autodetects this attribute but it presents
> exactly the promises to the compiler that allow this optimization.
>
> Thus, please split this piece out into a separate patch and use
> the const attribute.


Sounds good.

>
>>  What happens with cloning, -fclone-hot-version-paths ?
>>  -
>
> Now, here you lost me somewhat, because I didn't look into the
> patch details and I am missing an example on how the lowered
> IL looks before that cloning.  So for now I suppose this
> -fclone-hot-version-paths
> is to expose direct calls to the IL.  If you would lower __builtin_dispatch
> to a style like
>
>  int sel = selector ();
>  switch (sel)
>     {
>     case 0:
>       fn = popcnt;
>      break;
>     case 1:
>       fn = popcnt_sse4;
>       break;
>     }
>   res = (*fn) (25);
>
> then rather than a new pass specialized for __builtin_dispatch existing
> optimization passes that are able to do tracing like VRP and DOM
> via jump-threading or the tracer pass should be able to do this
> optimization for you.  If they do not use FDO in a good way it is better
> to improve them for this case instead of writing a new pass.

What you describe may not work

1) the function selection may happen in a different function;
2) Compiler will need to hoist the selection into the program
initializer to avoid overhead

As an example of why dispatch hoisting and call chain cloning is needed:

void foo();
void bar();

void doit_v1();
void doit_v2();
bool check_v () __attribute__((const));

int test();


void bar()
{

for (.)
 {
 foo();
 
 }
}

void foo()
{
   ...
   for (...)
   {
  __builtin_dispatch(check_v, doit_v1, doit_v2,...);
 ...
   }
}


int test ()
{
..
   bar ();
}


The feature testing and dispatch is embedded in a 2-deep loop nest
crossing function boundaries. The call paths test --> bar --> foo
needs to be cloned. This is done by hoisting dispatch up the call
chain -- it ends up :


void bar_v1()
{
   
   for (..)
{
  foo_v1 ();
}
 ..
}

void bar_v2 ()
{
...
for (..)
{
  foo_v2();
}
   ..
}

void foo_v1 ()
{
   ..
   for ()
{
  doit_v1()
}
}

void foo_v2 ()
{
..
for ()
{
   doit_v2()
}
}

int test ()
{
 __builtin_dispatch(check_v, bar_v1, bar_v2);
..

}



Thanks,

David



>
> Note that we already have special FDO support for indirect to direct
> call promotion, so that might work already.
>
> Thus, with all this, __builtin_dispatch would be more like syntactic
> sugar to avoid writing above switch statement yourself.  If you restrict
> that sugar to a constant number of candidates it can be replaced by
> a macro (or several ones, specialized for the number of candidates).
>
> Richard.
>
>>  For the call graph that looks like this before cloning :
>>
>> func_cold > func_hot > findOnes > IsPopCntSupported > popcnt

Re: [google]Add support for sampled profile collection (issue4438083)

[Silvius Rus]

> How is code-size affected with this patch, non-instrumented vs.
> regular-instrumented vs. sample-instrumented?

I don't have the numbers, but the increase in code size from
regular-instrumented to sample-instrumented is larger than that from
non-instrumented to sample-instrumented.  Easwaran, could you please
build a few binaries at -O2, -O2 -fprofile-generate and -O2
-fprofile-generate -fprofile-generate-sampling, and send out the text
size ratios considering -O2 as the baseline?

> > @@ -292,6 +474,15 @@ gimple_gen_edge_profiler (int edgeno, edge e)
> >                                        gimple_assign_lhs (stmt1), one);
> >   gimple_assign_set_lhs (stmt2, make_ssa_name (gcov_type_tmp_var, stmt2));
> >   stmt3 = gimple_build_assign (unshare_expr (ref), gimple_assign_lhs 
> > (stmt2));
> > +
> > +  if (flag_profile_generate_sampling)
> > +    {
> > +      slot = htab_find_slot_with_hash (instrumentation_to_be_sampled, 
> > stmt1,
> > +                                       htab_hash_pointer (stmt1), INSERT);
> > +      gcc_assert (!*slot);
> > +      *slot = stmt1;
> > +    }
> > +
>
> What's the reason to delay the sampling transform and not apply it here?
> At least a comment should be added for this.

Sorry, I just don't remember.

> > +DEFPARAM (PARAM_PROFILE_GENERATE_SAMPLING_RATE,
> > +         "profile-generate-sampling-rate",
> > +         "sampling rate with -fprofile-generate-sampling",
> > +         100, 0, 20)
>
> Zero is really ok?

It wouldn't break anything, but a rate of 0 doesn't make sense.  It
should be 1.  And the default should be 101 instead of 100 (and
generally a prime).

While we're at it, let me bring up an important practical FDO issue
related to this.  As David mentioned, besides reducing overhead we
have used this to implement selective collection.  This essentially
requires libgcov to provide a basic public interface:
reset()
start()
stop()
save()
I implemented them by playing with the sampling rate, but a clearer
and supported interface would be useful.  Also, there was one annoying
detail that I could not figure out.  When you build with
-fprofile-generate=./fdoprof, the output gets dumped under
./fdoprof/..., but there seems to be no easy way to know this path
within the profiling process.  For Google servers this makes
collection fragile.  The user generally does not have access to the
server file system.  I implemented a collection mechanism so the user
can tell the server "copy the profiles from ./fdoprof to some shared
location".  But now you need to pass the exact path when building
*and* collecting profiles, which may get separated in time, thus the
process is fragile.  It would be good to keep a copy of the path
prefix and have it accessible through a public interface as well.
Then once an instrumented binary is built, it will know the root of
the profile output directory and thus relieve the user from the
responsibility of knowing it.

Silvius

[Patch, libfortran] PR 48488 Fix comments

[Janne Blomqvist]

Hi all,

now that we fixed real output, I update the comments per the patch
below and committed.

Index: io/write.c
===
--- io/write.c  (revision 173168)
+++ io/write.c  (working copy)
@@ -1456,10 +1456,18 @@ set_fnode_default (st_parameter_dt *dtp,
   break;
 }
 }
-/* Output a real number with default format.
-   This is 1PG14.7E2 for REAL(4), 1PG23.15E3 for REAL(8),
-   1PG28.19E4 for REAL(10) and 1PG43.34E4 for REAL(16).  */
-// FX -- FIXME: should we change the default format for __float128-real(16)?
+
+/* Output a real number with default format.  To guarantee that a
+   binary -> decimal -> binary roundtrip conversion recovers the
+   original value, IEEE 754-2008 requires 9, 17, 21 and 36 significant
+   digits for REAL kinds 4, 8, 10, and 16, respectively. Thus, we use
+   1PG16.9E2 for REAL(4), 1PG25.17E3 for REAL(8), 1PG30.21E4 for
+   REAL(10) and 1PG45.36E4 for REAL(16). The exception is that the
+   Fortran standard requires outputting an extra digit when the scale
+   factor is 1 and when the magnitude of the value is such that E
+   editing is used. However, gfortran compensates for this, and thus
+   for list formatted the same number of significant digits is
+   generated both when using F and E editing.  */

 void
 write_real (st_parameter_dt *dtp, const char *source, int length)
@@ -1472,6 +1480,8 @@ write_real (st_parameter_dt *dtp, const
   dtp->u.p.scale_factor = org_scale;
 }

+/* Similar to list formatted REAL output, for kPG0 where k > 0 we
+   compensate for the extra digit.  */

 void
 write_real_g0 (st_parameter_dt *dtp, const char *source, int length, int d)


-- 
Janne Blomqvist

Re: [PATCH] Canonicalize compares in combine [2/3] Modifications to try_combine()

[Jeff Law]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 04/26/11 05:44, Chung-Lin Tang wrote:
> 
> Hi Jeff, thanks for reviewing this quite convoluted patch :)
FWIW, I don't think it's necessarily your patch that is convoluted, but
instead the original code.  It's also the case that I haven't spent
nearly as much time in the combiner as I have in other parts of GCC.


> Yes I'm not doing anything weird here, although the definition of the
> CANONICALIZE_COMPARISON macro does not seem to forbid (other ports) from
> doing so, as long as the comparison result is equivalent. It is
> currently called only at the end of simplify_comparison(), which itself
> possibly modifies the compare operands.
> 
> Considering not a lot of targets currently use CANONICALIZE_COMPARISON,
> maybe it would be feasible to slightly change its interface? Maybe
> adding a bool parameter to indicate whether the individual operands have
> to be retained equivalent.
That can certainly be done if necessary.  I'm just not sure it's needed,
at least not at this time.  If you want to make that change, feel free
to submit it.

> 
>>
>> I've generally found that avoiding const0_rtx is wise.  Instead I
>> suggest using CONST0_RTX (mode) to get the constant in the proper mode.
>>
> 
> Do you mean the (op1 == const0_rtx) test?
Yes.  It's a fairly minor issue.


> 
>> Given the tangled mess this code happens to be, could you please do a
>> bootstrap test on target with and without SELECT_CC_MODE.  x86 would be
>> a great example of the former, powerpc the latter (use the build farm if
>> you need to).   For ppc, just bootstrapping would be fine; I don't think
>> a full regression test is needed, just some verification that we don't
>> break ppc build should be sufficient.
>>
> 
> Bootstrap and test on x86 is completed already, both 32 and 64 bit.
> Bootstrap on ARM itself is also completed, using a Pandaboard. I'll try
> doing a ppc build too.
Thanks. Given that x86 & ARM are both SELECT_CC_MODE targets, I suspect
they're OK.  I mostly want a sniff test on a target that doesn't define
SELECT_CC_MODE (ppc for example).

Thanks,
jeff
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJNuuDyAAoJEBRtltQi2kC7gMQH/3hANrSFa3MSCTUvNxaiv9sy
HLoVYgcVXxNCNwYm/uSxZ3titiqyqB7ySrYPEezXCqdaNJeUSk+5v9w23wszyoel
bITxU6FC1P9wgHCRVNc9NsxpBJeCZleDSPHcrqAdbW8yO6J59qtaaRAlsBjgz11f
Isg1X+apG0Cy6DZ8knNRDVv9+HyJNLTYCg+90sSQv2Of1obp0b34sq/C2e03+oHz
gVBkgCvkLazcvYxLrdyCgXfyXvNMbMfSeVclJzpikJZAOAJBCwceYd16wg9ohBZR
y1g31oZ4teYcLz6c+yWEiWmZpVbeLnZSB9/bkvwP9lrQwkmKyxP33kdYHl/VP1E=
=ZZwe
-END PGP SIGNATURE-

Re: [Patch, libfortran] Some path handling fixes

[Jerry DeLisle]

On 04/29/2011 08:20 AM, Janne Blomqvist wrote:

Hello all,

since about a decade or so ago, POSIX specifies that PATH_MAX includes
the trailing null byte (previously it was undefined). However,
libgfortran has incorrectly assumed the opposite, and has thus created
temporary buffers of length PATH_MAX + 1 for holding paths when
converting from Fortran to C style strings. Secondly, these buffers
are allocated on the stack, and on Linux PATH_MAX is 4096 (presumably
other targets use something similar), which is quite a large object to
put on the stack. This can however be easily avoided in the common
case by using VLA's or alloca() as most paths are quite short, and we
know the size of the Fortran string to convert.

The attached patch fixes the problems discussed above. Regtested on
x86_64-unknown-linux-gnu, Ok for trunk?


OK and thanks for the patch.

Jerry

Re: [Patch, libfortran] Some path handling fixes

[Steve Kargl]

On Fri, Apr 29, 2011 at 06:20:17PM +0300, Janne Blomqvist wrote:
> 
> since about a decade or so ago, POSIX specifies that PATH_MAX includes
> the trailing null byte (previously it was undefined). However,
> libgfortran has incorrectly assumed the opposite, and has thus created
> temporary buffers of length PATH_MAX + 1 for holding paths when
> converting from Fortran to C style strings. Secondly, these buffers
> are allocated on the stack, and on Linux PATH_MAX is 4096 (presumably
> other targets use something similar), which is quite a large object to
> put on the stack. This can however be easily avoided in the common
> case by using VLA's or alloca() as most paths are quite short, and we
> know the size of the Fortran string to convert.
> 
> The attached patch fixes the problems discussed above. Regtested on
> x86_64-unknown-linux-gnu, Ok for trunk?
> 

OK.

-- 
Steve

Re: [google] Do not emit NULL warnings for implicit template args (issue4436067)

[Jason Merrill]

OK.

Jason

Re: [PING][PATCH] Change rclass argument type in memory_move_cost function from enum reg_class to reg_class_t.

[Jeff Law]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 04/27/11 12:17, Anatoly Sokolov wrote:
> Hello.
> 
> Patch ping.
> 
>>   This patch change memory_move_cost function to stop using back end 
>> specific 
>> type 'enum reg_class' in favor to reg_class_t. Also this allow do small 
>> cleanup in ia64_register_move_cost function.
> 
> http://gcc.gnu.org/ml/gcc-patches/2011-04/msg01433.html
OK.  Though frankly, I don't see how spitting enum reg_class from
reg_class_t buys us anything.

Jeff
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJNutwxAAoJEBRtltQi2kC7CNkH/RCGZhKRQ3RHratv2/zDWwMB
PP/p+W9k7eXPO0ZZjAo1PTrkAFP1XLTmjfHMYIOO9sym7Y2CYbbbjC8Vp5s/vz2w
orskLLmiM1PbA9hI+eASjQzUSFPOLGQA/nehV1M8EzvRsJ1cxv5oX8OJ9KJI/HgP
EhhFTZ97Jz/3tR12zvqpnzC6DwO9TpD2KrjkAd2AL35IcGgP9piYhprCiZouKxAZ
ASxdb1Cey81q1mw6olr6/5tuyI0BQOxkjZhtfzdF5D4j+C7FpUGanyehYZMNKy/M
orcWq73cy7jiRh5NLvJydTswECFbB6DqGsHpxngWMPY85cskPZxiDd0zTJNtNDU=
=e673
-END PGP SIGNATURE-

Re: [Patch, Fortran] Fix regressions PRs 48810 and 48800: wrong access flag and missing deferred-shape diagnostics

[Jerry DeLisle]

On 04/28/2011 04:18 PM, Tobias Burnus wrote:

The attached patch fixes two regressions:

a) PR 48810: For type-bound procedures, the access flags should be checked only
for the generic function, not for the specific function the generic resolves to.
(4.6/4.7 rejects-valid regression.)

b) PR 48800: Function-results shall not be assumed-shape arrays, unless they are
allocatable or a pointer. gfortran was missing a diagnostic for that. (The
diagnostic never existed, but with 4.6/4.7 it now ICEs as realloc on assignment
does not like it.)

Build and regtested on x86-64-linux.
OK for the trunk and the 4.6 branch?



Yes, OK and we just keep chipping away at these bugs and we'll be in good shape!

Thanks,

Jerry

Re: [C++ Patch] PR 48606

[Jason Merrill]

OK.

Jason

[Patch, libfortran] Some path handling fixes

[Janne Blomqvist]

Hello all,

since about a decade or so ago, POSIX specifies that PATH_MAX includes
the trailing null byte (previously it was undefined). However,
libgfortran has incorrectly assumed the opposite, and has thus created
temporary buffers of length PATH_MAX + 1 for holding paths when
converting from Fortran to C style strings. Secondly, these buffers
are allocated on the stack, and on Linux PATH_MAX is 4096 (presumably
other targets use something similar), which is quite a large object to
put on the stack. This can however be easily avoided in the common
case by using VLA's or alloca() as most paths are quite short, and we
know the size of the Fortran string to convert.

The attached patch fixes the problems discussed above. Regtested on
x86_64-unknown-linux-gnu, Ok for trunk?

2011-04-29  Janne Blomqvist  

* io/unix.c (min): New macro.
(unpack_filename): Return errno number for errors.
(regular_file): Use appropriately sized buffer for path.
(compare_file_filename): Likewise.
(find_file): Likewise.
(delete_file): Likewise.
(file_exists): Likewise.
(file_size): Likewise.
(inquire_sequential): Likewise.
(inquire_direct): Likewise.
(inquire_formatted): Likewise.
(inquire_access): Likewise.


-- 
Janne Blomqvist
diff --git a/libgfortran/io/unix.c b/libgfortran/io/unix.c
index ee2fd17..4e4bc3b 100644
--- a/libgfortran/io/unix.c
+++ b/libgfortran/io/unix.c
@@ -41,6 +41,13 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 #include 
 
 
+/* min macro that evaluates its arguments only once.  */
+#define min(a,b)		\
+  ({ typeof (a) _a = (a);	\
+typeof (b) _b = (b);	\
+_a < _b ? _a : _b; })
+
+
 /* For mingw, we don't identify files by their inode number, but by a
64-bit identifier created from a BY_HANDLE_FILE_INFORMATION. */
 #ifdef __MINGW32__
@@ -996,10 +1003,10 @@ int
 unpack_filename (char *cstring, const char *fstring, int len)
 {
   if (fstring == NULL)
-return 1;
+return EFAULT;
   len = fstrlen (fstring, len);
   if (len >= PATH_MAX)
-return 1;
+return ENAMETOOLONG;
 
   memmove (cstring, fstring, len);
   cstring[len] = '\0';
@@ -1124,15 +1131,17 @@ tempfile (st_parameter_open *opp)
 static int
 regular_file (st_parameter_open *opp, unit_flags *flags)
 {
-  char path[PATH_MAX + 1];
+  char path[min(PATH_MAX, opp->file_len + 1)];
   int mode;
   int rwflag;
   int crflag;
   int fd;
+  int err;
 
-  if (unpack_filename (path, opp->file, opp->file_len))
+  err = unpack_filename (path, opp->file, opp->file_len);
+  if (err)
 {
-  errno = ENOENT;		/* Fake an OS error */
+  errno = err;		/* Fake an OS error */
   return -1;
 }
 
@@ -1406,7 +1415,7 @@ st_printf (const char *format, ...)
 int
 compare_file_filename (gfc_unit *u, const char *name, int len)
 {
-  char path[PATH_MAX + 1];
+  char path[min(PATH_MAX, len + 1)];
   struct stat st;
 #ifdef HAVE_WORKING_STAT
   unix_stream *s;
@@ -1506,7 +1515,7 @@ find_file0 (gfc_unit *u, FIND_FILE0_DECL)
 gfc_unit *
 find_file (const char *file, gfc_charlen_type file_len)
 {
-  char path[PATH_MAX + 1];
+  char path[min(PATH_MAX, file_len + 1)];
   struct stat st[1];
   gfc_unit *u;
 #if defined(__MINGW32__) && !HAVE_WORKING_STAT
@@ -1625,11 +1634,12 @@ flush_all_units (void)
 int
 delete_file (gfc_unit * u)
 {
-  char path[PATH_MAX + 1];
+  char path[min(PATH_MAX, u->file_len + 1)];
+  int err = unpack_filename (path, u->file, u->file_len);
 
-  if (unpack_filename (path, u->file, u->file_len))
+  if (err)
 {/* Shouldn't be possible */
-  errno = ENOENT;
+  errno = err;
   return 1;
 }
 
@@ -1643,7 +1653,7 @@ delete_file (gfc_unit * u)
 int
 file_exists (const char *file, gfc_charlen_type file_len)
 {
-  char path[PATH_MAX + 1];
+  char path[min(PATH_MAX, file_len + 1)];
 
   if (unpack_filename (path, file, file_len))
 return 0;
@@ -1657,7 +1667,7 @@ file_exists (const char *file, gfc_charlen_type file_len)
 GFC_IO_INT
 file_size (const char *file, gfc_charlen_type file_len)
 {
-  char path[PATH_MAX + 1];
+  char path[min(PATH_MAX, file_len + 1)];
   struct stat statbuf;
 
   if (unpack_filename (path, file, file_len))
@@ -1678,7 +1688,7 @@ static const char yes[] = "YES", no[] = "NO", unknown[] = "UNKNOWN";
 const char *
 inquire_sequential (const char *string, int len)
 {
-  char path[PATH_MAX + 1];
+  char path[min(PATH_MAX, len + 1)];
   struct stat statbuf;
 
   if (string == NULL ||
@@ -1702,7 +1712,7 @@ inquire_sequential (const char *string, int len)
 const char *
 inquire_direct (const char *string, int len)
 {
-  char path[PATH_MAX + 1];
+  char path[min(PATH_MAX, len + 1)];
   struct stat statbuf;
 
   if (string == NULL ||
@@ -1726,7 +1736,7 @@ inquire_direct (const char *string, int len)
 const char *
 inquire_formatted (const char *string, int len)
 {
-  char path[PATH_MAX + 1];
+  char path[min(PATH_MAX, len + 1)];
   struct stat statbuf;
 
   if (string == NULL

Re: [google] Check if the nonnull attribute is applied to 'this' (issue4446070)

[Richard Guenther]

On Fri, Apr 29, 2011 at 5:08 PM, Diego Novillo  wrote:
> This patch from Le-Chun Wu adds support to check whether a nonnull
> attribute is applied to 'this' pointer for non-static methods.
>
> OK for trunk?  Applied to google/main
>
> 2011-04-27  Le-Chun Wu  
>
>        Google ref 45339.
>
>        * c-common.c (handle_nonnull_attribute): Check whether the nonnull
>        attribute is applied to the 'this' pointer for non-static methods.
>
> testsuite/ChangeLog.google-main
> 2011-04-27  Le-Chun Wu  
>
>        * g++.dg/warn/nonnull2.C: New.
>
> diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
> index c6dc649..a1702f8 100644
> --- a/gcc/c-family/c-common.c
> +++ b/gcc/c-family/c-common.c
> @@ -7434,7 +7434,7 @@ handle_nonnull_attribute (tree *node, tree ARG_UNUSED 
> (name),
>
>   /* Argument list specified.  Verify that each argument number references
>      a pointer argument.  */
> -  for (attr_arg_num = 1; args; args = TREE_CHAIN (args))
> +  for (attr_arg_num = 1; args; args = TREE_CHAIN (args), attr_arg_num++)
>     {
>       tree argument;
>       unsigned HOST_WIDE_INT arg_num = 0, ck_num;
> @@ -7466,6 +7466,7 @@ handle_nonnull_attribute (tree *node, tree ARG_UNUSED 
> (name),
>              return NULL_TREE;
>            }
>
> +

spurious white-space change.

>          if (TREE_CODE (TREE_VALUE (argument)) != POINTER_TYPE)
>            {
>              error ("nonnull argument references non-pointer operand 
> (argument %lu, operand %lu)",
> @@ -7473,6 +7474,11 @@ handle_nonnull_attribute (tree *node, tree ARG_UNUSED 
> (name),
>              *no_add_attrs = true;
>              return NULL_TREE;
>            }
> +
> +          if (TREE_CODE (type) == METHOD_TYPE && arg_num == 1)
> +            warning (OPT_Wattributes,
> +                     "nonnull argument references 'this' pointer (argument 
> %lu, operand %lu)",
> +                     (unsigned long) attr_arg_num, (unsigned long) arg_num);
>        }
>     }
>
> diff --git a/gcc/testsuite/g++.dg/warn/nonnull2.C 
> b/gcc/testsuite/g++.dg/warn/nonnull2.C
> new file mode 100644
> index 000..03006b1
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/nonnull2.C
> @@ -0,0 +1,14 @@
> +// Test that "nonnull" attribute should not be applied to 'this' pointer.
> +// { dg-do compile }
> +
> +#define NULL 0
> +
> +class Foo {
> + public:
> +  void method1(const int *ptr) __attribute__((nonnull(1, 2))); // { 
> dg-warning "nonnull argument references 'this' pointer" }
> +  void method2(int *ptr1, int a, int *ptr2) __attribute__((nonnull(2, 3, 
> 4))); // { dg-error "nonnull argument references non-pointer operand" }
> +  static void func3(int *ptr) __attribute__((nonnull(1))); // should not warn
> +  Foo(char *str) __attribute__((nonnull())) {}
> +};
> +
> +int func4(int *ptr1, int a) __attribute__((nonnull(1))); // should not warn
> --
> 1.7.3.1
>
>
> --
> This patch is available for review at http://codereview.appspot.com/4446070
>

[google] Check if the nonnull attribute is applied to 'this' (issue4446070)

[Diego Novillo]

This patch from Le-Chun Wu adds support to check whether a nonnull
attribute is applied to 'this' pointer for non-static methods.

OK for trunk?  Applied to google/main

2011-04-27  Le-Chun Wu  

Google ref 45339.

* c-common.c (handle_nonnull_attribute): Check whether the nonnull
attribute is applied to the 'this' pointer for non-static methods.

testsuite/ChangeLog.google-main
2011-04-27  Le-Chun Wu  

* g++.dg/warn/nonnull2.C: New.

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index c6dc649..a1702f8 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -7434,7 +7434,7 @@ handle_nonnull_attribute (tree *node, tree ARG_UNUSED 
(name),
 
   /* Argument list specified.  Verify that each argument number references
  a pointer argument.  */
-  for (attr_arg_num = 1; args; args = TREE_CHAIN (args))
+  for (attr_arg_num = 1; args; args = TREE_CHAIN (args), attr_arg_num++)
 {
   tree argument;
   unsigned HOST_WIDE_INT arg_num = 0, ck_num;
@@ -7466,6 +7466,7 @@ handle_nonnull_attribute (tree *node, tree ARG_UNUSED 
(name),
  return NULL_TREE;
}
 
+
  if (TREE_CODE (TREE_VALUE (argument)) != POINTER_TYPE)
{
  error ("nonnull argument references non-pointer operand (argument 
%lu, operand %lu)",
@@ -7473,6 +7474,11 @@ handle_nonnull_attribute (tree *node, tree ARG_UNUSED 
(name),
  *no_add_attrs = true;
  return NULL_TREE;
}
+
+  if (TREE_CODE (type) == METHOD_TYPE && arg_num == 1)
+warning (OPT_Wattributes,
+ "nonnull argument references 'this' pointer (argument 
%lu, operand %lu)",
+ (unsigned long) attr_arg_num, (unsigned long) arg_num);
}
 }
 
diff --git a/gcc/testsuite/g++.dg/warn/nonnull2.C 
b/gcc/testsuite/g++.dg/warn/nonnull2.C
new file mode 100644
index 000..03006b1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/nonnull2.C
@@ -0,0 +1,14 @@
+// Test that "nonnull" attribute should not be applied to 'this' pointer.
+// { dg-do compile }
+
+#define NULL 0
+
+class Foo {
+ public:
+  void method1(const int *ptr) __attribute__((nonnull(1, 2))); // { dg-warning 
"nonnull argument references 'this' pointer" }
+  void method2(int *ptr1, int a, int *ptr2) __attribute__((nonnull(2, 3, 4))); 
// { dg-error "nonnull argument references non-pointer operand" }
+  static void func3(int *ptr) __attribute__((nonnull(1))); // should not warn
+  Foo(char *str) __attribute__((nonnull())) {}
+};
+
+int func4(int *ptr1, int a) __attribute__((nonnull(1))); // should not warn
-- 
1.7.3.1


--
This patch is available for review at http://codereview.appspot.com/4446070

Re: [google] Enable -Wnonnull for C++ (issue4431076)

[Diego Novillo]

On Fri, Apr 29, 2011 at 11:04, Diego Novillo  wrote:
> This patch from Le-Chun Wu enables -Wnonnull in C++ as well as C.
>
> OK for trunk?  Applied to google/main.
>
> 2011-04-27  Le-Chun Wu  
>
>        * c.opt (Wnonnull): Enable for C++.

Google ref 42718

[google] Enable -Wnonnull for C++ (issue4431076)

[Diego Novillo]

This patch from Le-Chun Wu enables -Wnonnull in C++ as well as C.

OK for trunk?  Applied to google/main.

2011-04-27  Le-Chun Wu  

* c.opt (Wnonnull): Enable for C++.

ChangeLog.google-main
2011-04-27  Le-Chun Wu  

* doc/extend.texi (Wnonnull): Add documentation for C++.
* doc/invoke.texi (Wnonnull): Add documentation for C++.

testsuite/ChangeLog.google-main
2011-04-27  Le-Chun Wu  

* g++.dg/warn/Wnonnull-1.C: New.

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index ce742fa..63f41a3 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -510,7 +510,7 @@ C++ ObjC++ Var(warn_nonvdtor) Warning
 Warn about non-virtual destructors
 
 Wnonnull
-C ObjC Var(warn_nonnull) Warning
+C C++ ObjC Var(warn_nonnull) Warning
 Warn about NULL being passed to argument slots marked as requiring non-NULL
 
 Wnormalized=
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index eaad089..af9e98a 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -2943,6 +2943,10 @@ as non-null, and the @option{-Wnonnull} option is 
enabled, a warning
 is issued.  The compiler may also choose to make optimizations based
 on the knowledge that certain function arguments will not be null.
 
+Since non-static C++ methods have an implicit @code{this} argument, the
+arguments of such methods should be counted from two, not one, when
+giving values for @var{arg-index}.
+
 If no argument index list is given to the @code{nonnull} attribute,
 all pointer arguments are marked as non-null.  To illustrate, the
 following declaration is equivalent to the previous example:
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 5f6d79d..33a6eb8 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -3194,7 +3194,7 @@ Enable @option{-Wformat} plus format checks not included 
in
 @option{-Wformat}.  Currently equivalent to @samp{-Wformat
 -Wformat-nonliteral -Wformat-security -Wformat-y2k}.
 
-@item -Wnonnull @r{(C and Objective-C only)}
+@item -Wnonnull @r{(C, C++, Objective-C, and Objective-C++ only)}
 @opindex Wnonnull
 @opindex Wno-nonnull
 Warn about passing a null pointer for arguments marked as
diff --git a/gcc/testsuite/g++.dg/warn/Wnonnull-1.C 
b/gcc/testsuite/g++.dg/warn/Wnonnull-1.C
new file mode 100644
index 000..837c6d6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wnonnull-1.C
@@ -0,0 +1,16 @@
+// { dg-do compile }
+// { dg-options "-Wnonnull" }
+
+#include 
+
+class Foo {
+  char *name;
+ public:
+  void func1(const int *ptr) __attribute__((nonnull(2))) {}
+  Foo(char *str) __attribute__((nonnull)) : name(str) {}
+};
+
+void Bar() {
+  Foo foo(NULL);   // { dg-warning "null argument where non-null required" }
+  foo.func1(NULL); // { dg-warning "null argument where non-null required" }
+}
-- 
1.7.3.1


--
This patch is available for review at http://codereview.appspot.com/4431076

Re: [google] Add new warning -Wreal-conversion (issue4436068)

[Nathan Froyd]

On Fri, Apr 29, 2011 at 10:59:31AM -0400, Diego Novillo wrote:
>   * g++.dg/warn/Wreal-conversion-1.C: New.
>   * gcc.dg/Wreal-conversion-1.c: New.

Could a single copy of the test be placed in c-c++-common, instead?

-Nathan

[PATCH,c++] delete TREE_NEGATED_INT

[Nathan Froyd]

As $SUBJECT suggests.  It is write-only; I'm not sure what it was ever
used for.

Tested on x86_64-unknonw-linux-gnu.  OK to commit?

-Nathan

gcc/cp/
* cp-tree.h (TREE_NEGATED_INT): Delete.
* semantics.c (finish_unary_op_expr): Don't try to set it.

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index a65998d..899959e 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -2937,10 +2937,6 @@ more_aggr_init_expr_args_p (const 
aggr_init_expr_arg_iterator *iter)
 #define TYPENAME_IS_RESOLVING_P(NODE) \
   (TREE_LANG_FLAG_2 (TYPENAME_TYPE_CHECK (NODE)))
 
-/* Nonzero in INTEGER_CST means that this int is negative by dint of
-   using a twos-complement negated operand.  */
-#define TREE_NEGATED_INT(NODE) TREE_LANG_FLAG_0 (INTEGER_CST_CHECK (NODE))
-
 /* [class.virtual]
 
A class that declares or inherits a virtual function is called a
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 815824a..99ca28b 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -2302,19 +2302,7 @@ tree
 finish_unary_op_expr (enum tree_code code, tree expr)
 {
   tree result = build_x_unary_op (code, expr, tf_warning_or_error);
-  /* Inside a template, build_x_unary_op does not fold the
- expression. So check whether the result is folded before
- setting TREE_NEGATED_INT.  */
-  if (code == NEGATE_EXPR && TREE_CODE (expr) == INTEGER_CST
-  && TREE_CODE (result) == INTEGER_CST
-  && !TYPE_UNSIGNED (TREE_TYPE (result))
-  && INT_CST_LT (result, integer_zero_node))
-{
-  /* RESULT may be a cached INTEGER_CST, so we must copy it before
-setting TREE_NEGATED_INT.  */
-  result = copy_node (result);
-  TREE_NEGATED_INT (result) = 1;
-}
+
   if (TREE_OVERFLOW_P (result) && !TREE_OVERFLOW_P (expr))
 overflow_warning (input_location, result);
 

[google] Add new warning -Wreal-conversion (issue4436068)

[Diego Novillo]

This patch from Le-Chun Wu adds a new warning flag "-Wreal-conversion"
that warns about implicit type conversions from real (double or float)
values to integral values.

OK for trunk?  Applied to google/main.

2011-04-27  Le-Chun Wu  

Google ref 39133

* c.opt (Wreal-conversion): New flag.
* c-common.c (conversion_warning): Use it.
* c-opts.c (c_common_post_options): Initialize it.
* doc/invoke.texi (Wreal-conversion): Document it.

testsuite/ChangeLog.google-main:
2011-04-27  Le-Chun Wu  

* g++.dg/warn/Wreal-conversion-1.C: New.
* gcc.dg/Wreal-conversion-1.c: New.

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 13f13a8..c6dc649 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -1941,7 +1941,7 @@ conversion_warning (tree type, tree expr)
   tree expr_type = TREE_TYPE (expr);
   location_t loc = EXPR_LOC_OR_HERE (expr);
 
-  if (!warn_conversion && !warn_sign_conversion)
+  if (!warn_conversion && !warn_sign_conversion && !warn_real_conversion)
 return;
 
   /* If any operand is artificial, then this expression was generated
@@ -1984,7 +1984,9 @@ conversion_warning (tree type, tree expr)
   && TREE_CODE (type) == INTEGER_TYPE)
 {
   if (!real_isinteger (TREE_REAL_CST_PTR (expr), TYPE_MODE 
(expr_type)))
-give_warning = true;
+warning (OPT_Wreal_conversion,
+ "conversion to %qT from %qT may alter its value",
+ type, expr_type);
 }
   /* Warn for an integer constant that does not fit into integer type.  */
   else if (TREE_CODE (expr_type) == INTEGER_TYPE
@@ -2053,7 +2055,9 @@ conversion_warning (tree type, tree expr)
   /* Warn for real types converted to integer types.  */
   if (TREE_CODE (expr_type) == REAL_TYPE
   && TREE_CODE (type) == INTEGER_TYPE)
-give_warning = true;
+warning (OPT_Wreal_conversion,
+ "conversion to %qT from %qT may alter its value",
+ type, expr_type);
 
   else if (TREE_CODE (expr_type) == INTEGER_TYPE
&& TREE_CODE (type) == INTEGER_TYPE)
diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c
index c3d1bbe..994bf90 100644
--- a/gcc/c-family/c-opts.c
+++ b/gcc/c-family/c-opts.c
@@ -939,6 +939,12 @@ c_common_post_options (const char **pfilename)
   if (warn_enum_compare == -1)
 warn_enum_compare = c_dialect_cxx () ? 1 : 0;
 
+  /* Enable warning for converting real values to integral values
+ when -Wconversion is specified (unless disabled through
+ -Wno-real-conversion).  */
+  if (warn_real_conversion == -1)
+warn_real_conversion = warn_conversion;
+
   /* -Wpacked-bitfield-compat is on by default for the C languages.  The
  warning is issued in stor-layout.c which is not part of the front-end so
  we need to selectively turn it on here.  */
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index af38a1f..ce742fa 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -323,6 +323,10 @@ Wself-assign-non-pod
 C++ ObjC++ Var(warn_self_assign_non_pod) Init(0) Warning
 Warn when a variable of a non-POD type is assigned to itself
 
+Wreal-conversion
+C ObjC C++ ObjC++ Var(warn_real_conversion) Init(-1) Warning
+Warn for implicit type conversions from real to integral values
+
 Wsign-conversion
 C ObjC C++ ObjC++ Var(warn_sign_conversion) Init(-1)
 Warn for implicit type conversions between signed and unsigned integers
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index fa247b6..5f6d79d 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -257,7 +257,7 @@ Objective-C and Objective-C++ Dialects}.
 -Woverlength-strings  -Wpacked  -Wpacked-bitfield-compat  -Wpadded @gol
 -Wparentheses  -Wpedantic-ms-format -Wno-pedantic-ms-format @gol
 -Wpointer-arith  -Wno-pointer-to-int-cast @gol
--Wredundant-decls  -Wreturn-type -Wripa-opt-mismatch @gol
+-Wreal-conversion  -Wredundant-decls  -Wreturn-type -Wripa-opt-mismatch @gol
 -Wself-assign  -Wself-assign-non-pod  -Wsequence-point  -Wshadow @gol
 -Wshadow-compatible-local -Wshadow-local @gol
 -Wsign-compare  -Wsign-conversion  -Wstack-protector @gol
@@ -4232,6 +4232,12 @@ unsigned integers are disabled by default in C++ unless
 Do not warn for conversions between @code{NULL} and non-pointer
 types. @option{-Wconversion-null} is enabled by default.
 
+@item -Wreal-conversion
+@opindex Wreal-conversion
+@opindex Wno-real-conversion
+Warn for implicit type conversions from real (@code{double} or @code{float})
+to integral values.
+
 @item -Wempty-body
 @opindex Wempty-body
 @opindex Wno-empty-body
diff --git a/gcc/testsuite/g++.dg/warn/Wreal-conversion-1.C 
b/gcc/testsuite/g++.dg/warn/Wreal-conversion-1.C
new file mode 100644
index 000..d96fe50
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wreal-conversion-1.C
@@ -0,0 +1,26 @@
+// { dg-do compile }
+// { dg-options "-Wreal-conversion" }
+
+#include 
+
+int func1

[google] Add two new -Wshadow warnings (issue4452058)

[Diego Novillo]

This patch from Le-Chun Wu adds two two new shadow warning flags for
C and C++:

-Wshadow-local which warns if a local variable shadows another local
variable or parameter,

-Wshadow-compatible-local which warns if a local variable shadows another
local variable or parameter whose type is compatible with that of the
shadowing variable.

OK for trunk?  Applied to google/main


2011-04-27  Le-Chun Wu  

Google ref 39127.

* c-decl.c (warn_if_shadowing): Use the warning code corresponding
to the given -Wshadow- variant.
* common.opt (Wshadow-local): New option.
(Wshadow-compatible-local): New option.
* invoke.texi: Document Wshadow-local and Wshadow-compatible-local.
* opts.c (common_handle_option): Handle OPT_Wshadow and
OPT_Wshadow_local.
Do not enable Wshadow-local nor Wshadow-compatible-local if
Wshadow is disabled.

cp/ChangeLog.google-main
2011-04-27  Le-Chun Wu  

* name-lookup.c (pushdecl_maybe_friend): When emitting a
shadowing warning, use the code corresponding to the
given -Wshadow- variant.

testsuite/ChangeLog.google-main
2011-04-27  Le-Chun Wu  

* g++.dg/warn/Wshadow-compatible-local-1.C: New.
* g++.dg/warn/Wshadow-local-1.C: New.
* g++.dg/warn/Wshadow-local-2.C: New.
* gcc.dg/Wshadow-compatible-local-1.c: New.
* gcc.dg/Wshadow-local-1.c: New.
* gcc.dg/Wshadow-local-2.c: New.
* gcc.dg/Wshadow-local-3.c: New.

diff --git a/gcc/c-decl.c b/gcc/c-decl.c
index 112e814..4a6a17d 100644
--- a/gcc/c-decl.c
+++ b/gcc/c-decl.c
@@ -2565,7 +2565,9 @@ warn_if_shadowing (tree new_decl)
   struct c_binding *b;
 
   /* Shadow warnings wanted?  */
-  if (!warn_shadow
+  if (!(warn_shadow
+|| warn_shadow_local
+|| warn_shadow_compatible_local)
   /* No shadow warnings for internally generated vars.  */
   || DECL_IS_BUILTIN (new_decl)
   /* No shadow warnings for vars made for inlining.  */
@@ -2579,30 +2581,55 @@ warn_if_shadowing (tree new_decl)
tree old_decl = b->decl;
 
if (old_decl == error_mark_node)
- {
-   warning (OPT_Wshadow, "declaration of %q+D shadows previous "
-"non-variable", new_decl);
-   break;
- }
+ warning (OPT_Wshadow, "declaration of %q+D shadows previous "
+  "non-variable", new_decl);
else if (TREE_CODE (old_decl) == PARM_DECL)
- warning (OPT_Wshadow, "declaration of %q+D shadows a parameter",
-  new_decl);
+  {
+enum opt_code warning_code;
+
+/* If '-Wshadow-compatible-local' is specified without other
+   -Wshadow flags, we will warn only when the types of the
+   shadowing variable (i.e. new_decl) and the shadowed variable
+   (old_decl) are compatible.  */
+if (comptypes (TREE_TYPE (old_decl), TREE_TYPE (new_decl)))
+  warning_code = OPT_Wshadow_compatible_local;
+else
+  warning_code = OPT_Wshadow_local;
+warning (warning_code,
+ "declaration of %q+D shadows a parameter", new_decl);
+warning_at (DECL_SOURCE_LOCATION (old_decl), warning_code,
+   "shadowed declaration is here");
+  }
else if (DECL_FILE_SCOPE_P (old_decl))
- warning (OPT_Wshadow, "declaration of %q+D shadows a global "
-  "declaration", new_decl);
+  {
+warning (OPT_Wshadow, "declaration of %q+D shadows a global "
+ "declaration", new_decl);
+warning_at (DECL_SOURCE_LOCATION (old_decl), OPT_Wshadow,
+   "shadowed declaration is here");
+  }
else if (TREE_CODE (old_decl) == FUNCTION_DECL
 && DECL_BUILT_IN (old_decl))
- {
-   warning (OPT_Wshadow, "declaration of %q+D shadows "
-"a built-in function", new_decl);
-   break;
- }
+ warning (OPT_Wshadow, "declaration of %q+D shadows "
+  "a built-in function", new_decl);
else
- warning (OPT_Wshadow, "declaration of %q+D shadows a previous local",
-  new_decl);
-
-   warning_at (DECL_SOURCE_LOCATION (old_decl), OPT_Wshadow,
-   "shadowed declaration is here");
+  {
+enum opt_code warning_code;
+
+/* If '-Wshadow-compatible-local' is specified without other
+   -Wshadow flags, we will warn only when the types of the
+   shadowing variable (i.e. new_decl) and the shadowed variable
+   (old_decl) are compatible.  */
+if (comptypes (TREE_TYPE (old_decl), TREE_TYPE (new_decl)))
+  warning_code = OPT_Wshadow_compatible_local;
+else
+  warning_code = OPT_Wshadow_local;
+warning (warning_

[google] Do not emit NULL warnings for implicit template args (issue4436067)

[Diego Novillo]

This patch from Le-Chun Wu distinguishes explicit from implicit
template arguments to avoid emitting NULL warnings for implicit
arguments.

Additionally, Le-Chun extended the pointer to boolean conversion
warnings to also warn when assigning a boolean value to a pointer.

Le-Chun, if I'm missing anything, please correct me.

Jason, OK for trunk?  Applied to google/main.

2011-04-27  Le-Chun Wu  

Google ref 40484.

* cp-tree.h (LOOKUP_EXPLICIT_TMPL_ARGS): Define.
* call.c (build_new_function_call): Set it for TEMPLATE_ID_EXPRs.
(build_over_call): Use it to determine whether to emit a NULL
warning for template function instantiations.
(build_new_method_call): Set LOOKUP_EXPLICIT_TMPL_ARGS if
EXPLICIT_TARGS is set.

testsuite/ChangeLog.google-main
2011-04-27  Le-Chun Wu  

* g++.dg/warn/Wnull-conversion-1.C: New.
* g++.dg/warn/Wnull-conversion-2.C: New.

cp/ChangeLog.google-main
2011-04-28  Diego Novillo  
Le-Chun Wu  

* call.c (conversion_null_warnings): Also handle assignments
when warning about NULL conversions.

testsuite/ChangeLog.google-main
2011-04-28  Le-Chun Wu  

* g++.dg/warn/Wconversion-null-2.C: Do not expect a NULL
  warning in implicitly instantiated templates.

2011-04-29  Diego Novillo  

* g++.old-deja/g++.other/null3.C: Expect warning about converting
boolean to a pointer.

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 32aa5ca..f56a67d 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -3624,7 +3624,16 @@ build_new_function_call (tree fn, VEC(tree,gc) **args, 
bool koenig_p,
   result = error_mark_node;
 }
   else
-result = build_over_call (cand, LOOKUP_NORMAL, complain);
+{
+  int flags = LOOKUP_NORMAL;
+  /* If fn is template_id_expr, the call has explicit template arguments
+ (e.g. func(5)), communicate this info to build_over_call
+ through flags so that later we can use it to decide whether to warn
+ about peculiar null pointer conversion.  */
+  if (TREE_CODE (fn) == TEMPLATE_ID_EXPR)
+flags |= LOOKUP_EXPLICIT_TMPL_ARGS;
+  result = build_over_call (cand, flags, complain);
+}
 
   /* Free all the conversions we allocated.  */
   obstack_free (&conversion_obstack, p);
@@ -5264,10 +5273,16 @@ conversion_null_warnings (tree totype, tree expr, tree 
fn, int argnum)
 }
 
   /* Issue warnings if "false" is converted to a NULL pointer */
-  else if (expr == boolean_false_node && fn && POINTER_TYPE_P (t))
-warning_at (input_location, OPT_Wconversion_null,
-   "converting % to pointer type for argument %P of %qD",
-   argnum, fn);
+  else if (expr == boolean_false_node && POINTER_TYPE_P (t))
+{
+  if (fn)
+   warning_at (input_location, OPT_Wconversion_null,
+   "converting % to pointer type for argument %P "
+   "of %qD", argnum, fn);
+  else
+   warning_at (input_location, OPT_Wconversion_null,
+   "converting % to pointer type %qT", t);
+}
 }
 
 /* Perform the conversions in CONVS on the expression EXPR.  FN and
@@ -6163,6 +6178,7 @@ build_over_call (struct z_candidate *cand, int flags, 
tsubst_flags_t complain)
 {
   tree type = TREE_VALUE (parm);
   tree arg = VEC_index (tree, args, arg_index);
+  bool conversion_warning = true;
 
   conv = convs[i];
 
@@ -6172,6 +6188,32 @@ build_over_call (struct z_candidate *cand, int flags, 
tsubst_flags_t complain)
  && !TREE_ADDRESSABLE (type))
conv = conv->u.next;
 
+  /* If the argument is NULL and used to (implicitly) instantiate a
+ template function (and bind one of the template arguments to
+ the type of 'long int'), we don't want to warn about passing NULL
+ to non-pointer argument.
+ For example, if we have this template function:
+
+   template void func(T x) {}
+
+ we want to warn (when -Wconversion is enabled) in this case:
+
+   void foo() {
+ func(NULL);
+   }
+
+ but not in this case:
+
+   void foo() {
+ func(NULL);
+   }
+  */
+  if (arg == null_node
+  && DECL_TEMPLATE_INFO (fn)
+  && cand->template_decl
+  && !(flags & LOOKUP_EXPLICIT_TMPL_ARGS))
+conversion_warning = false;
+
   /* Warn about initializer_list deduction that isn't currently in the
 working draft.  */
   if (cxx_dialect > cxx98
@@ -6202,7 +6244,10 @@ build_over_call (struct z_candidate *cand, int flags, 
tsubst_flags_t complain)
}
}
 
-  val = convert_like_with_context (conv, arg, fn, i-is_method, complain);
+  val = convert_like_with_context (conv, arg, fn, i-is_method,
+  conversion_warning
+  ? complain
+  : complain & (~t

[PATCH][4/n] Fix build_int_cst callers with NULL type

[Richard Guenther]

Another round.  Missing is now tree-data-ref.c which better should
use double_ints.  Frontends also need a look, so do backends.

Bootstrap & regtest running on x86_64-unknown-linux-gnu, will apply
after that succeeded.

Richard.

2011-04-29  Richard Guenther  

* builtins.c (fold_builtin_classify_type): Use integer_type_node
for the type of the result.
(fold_builtin_isascii): Likewise.
(fold_builtin_toascii): Use integer_type_node where appropriate.
(fold_builtin_logb): Likewise.
(fold_builtin_frexp): Likewise.
(fold_builtin_strstr): Likewise.
(fold_builtin_strpbrk): Likewise.
(fold_builtin_fputs): Likewise.
(fold_builtin_sprintf): Likewise.
(fold_builtin_snprintf): Likewise.
(fold_builtin_printf): Likewise.
(do_mpfr_remquo): Use a proper type for the assigned constant.
(do_mpfr_lgamma_r): Likewise.
* dwarf2out.c (resolve_one_addr): Use size_int.
* except.c (init_eh): Likewise.
(assign_filter_values): Use integer_type_node for filter values.
(sjlj_emit_dispatch_table): Use integer_type_node for dispatch
indices.
* tree-cfg.c (move_stmt_eh_region_tree_nr): Use integer_type_node
for EH region numbers.
* tree-vrp.c (simplify_div_or_mod_using_ranges): Use integer_type_node
for the shift amount.

Index: gcc/builtins.c
===
*** gcc/builtins.c  (revision 173155)
--- gcc/builtins.c  (working copy)
*** static tree
*** 6774,6782 
  fold_builtin_classify_type (tree arg)
  {
if (arg == 0)
! return build_int_cst (NULL_TREE, no_type_class);
  
!   return build_int_cst (NULL_TREE, type_to_class (TREE_TYPE (arg)));
  }
  
  /* Fold a call to __builtin_strlen with argument ARG.  */
--- 6774,6782 
  fold_builtin_classify_type (tree arg)
  {
if (arg == 0)
! return build_int_cst (integer_type_node, no_type_class);
  
!   return build_int_cst (integer_type_node, type_to_class (TREE_TYPE (arg)));
  }
  
  /* Fold a call to __builtin_strlen with argument ARG.  */
*** fold_builtin_isascii (location_t loc, tr
*** 9134,9143 
  {
/* Transform isascii(c) -> ((c & ~0x7f) == 0).  */
arg = fold_build2 (BIT_AND_EXPR, integer_type_node, arg,
!build_int_cst (NULL_TREE,
~ (unsigned HOST_WIDE_INT) 0x7f));
return fold_build2_loc (loc, EQ_EXPR, integer_type_node,
! arg, integer_zero_node);
  }
  }
  
--- 9134,9143 
  {
/* Transform isascii(c) -> ((c & ~0x7f) == 0).  */
arg = fold_build2 (BIT_AND_EXPR, integer_type_node, arg,
!build_int_cst (integer_type_node,
~ (unsigned HOST_WIDE_INT) 0x7f));
return fold_build2_loc (loc, EQ_EXPR, integer_type_node,
! arg, integer_zero_node);
  }
  }
  
*** fold_builtin_toascii (location_t loc, tr
*** 9151,9157 
  
/* Transform toascii(c) -> (c & 0x7f).  */
return fold_build2_loc (loc, BIT_AND_EXPR, integer_type_node, arg,
! build_int_cst (NULL_TREE, 0x7f));
  }
  
  /* Fold a call to builtin isdigit with argument ARG.  */
--- 9151,9157 
  
/* Transform toascii(c) -> (c & 0x7f).  */
return fold_build2_loc (loc, BIT_AND_EXPR, integer_type_node, arg,
! build_int_cst (integer_type_node, 0x7f));
  }
  
  /* Fold a call to builtin isdigit with argument ARG.  */
*** fold_builtin_logb (location_t loc, tree
*** 9342,9348 
   exponent and subtract 1.  */
if (REAL_MODE_FORMAT (TYPE_MODE (TREE_TYPE (arg)))->b == 2)
  return fold_convert_loc (loc, rettype,
!  build_int_cst (NULL_TREE,
  REAL_EXP (value)-1));
break;
}
--- 9342,9348 
   exponent and subtract 1.  */
if (REAL_MODE_FORMAT (TYPE_MODE (TREE_TYPE (arg)))->b == 2)
  return fold_convert_loc (loc, rettype,
!  build_int_cst (integer_type_node,
  REAL_EXP (value)-1));
break;
}
*** fold_builtin_frexp (location_t loc, tree
*** 9430,9436 
  REAL_VALUE_TYPE frac_rvt = *value;
  SET_REAL_EXP (&frac_rvt, 0);
  frac = build_real (rettype, frac_rvt);
! exp = build_int_cst (NULL_TREE, REAL_EXP (value));
}
break;
default:
--- 9430,9436 
  REAL_VALUE_TYPE frac_rvt = *value;
  SET_REAL_EXP (&frac_rvt, 0);
  frac = build_real (rettype, frac_rvt);
! exp = build_int_cst (integer_type_node, REAL_EXP (value));
}
break;
default:
*** fold_builtin_strstr (location_t 

[PATCH] convert forced_labels to a VEC

[Nathan Froyd]

As $SUBJECT suggests.  Just like the nonlocal_goto_handler_labels, the
real benefit is a proper container instead of an EXPR_LIST.

in_expr_list_p is unused after this patch; I will delete it as obvious
in a followon patch if this patch is approved.

Tested on x86_64-unknown-linux-gnu.  OK to commit?

-Nathan

* cfgbuild.c (struct expr_status) [x_forced_labels]: Change to a
VEC.
(note_forced_label): New function.
* stmt.c (force_label_rtx, expand_label): Call it.
* except.c (sjlj_emit_dispatch_table): Likewise.
* cfgbuild.c (make_edges): Adjust for new type of forced_labels.
* sched-rgn.c (is_cfg_nonregular): Likewise.
* reload1.c (set_initial_label_offsets): Likewise.
* jump.c (rebuild_jump_labels): Likewise.
* cfgrtl.c (can_delete_label_p): Likewise.  Change return type
to
bool.

diff --git a/gcc/cfgbuild.c b/gcc/cfgbuild.c
index 6f0d69e..642f57e 100644
--- a/gcc/cfgbuild.c
+++ b/gcc/cfgbuild.c
@@ -216,7 +216,7 @@ make_edges (basic_block min, basic_block max, int update_p)
   /* Heavy use of computed goto in machine-generated code can lead to
  nearly fully-connected CFGs.  In that case we spend a significant
  amount of time searching the edge lists for duplicates.  */
-  if (forced_labels || cfun->cfg->max_jumptable_ents > 100)
+  if (!VEC_empty (rtx, forced_labels) || cfun->cfg->max_jumptable_ents > 100)
 edge_cache = sbitmap_alloc (last_basic_block);
 
   /* By nature of the way these get numbered, ENTRY_BLOCK_PTR->next_bb block
@@ -296,8 +296,9 @@ make_edges (basic_block min, basic_block max, int update_p)
 everything on the forced_labels list.  */
  else if (computed_jump_p (insn))
{
- for (x = forced_labels; x; x = XEXP (x, 1))
-   make_label_edge (edge_cache, bb, XEXP (x, 0), EDGE_ABNORMAL);
+ unsigned int ix;
+ FOR_EACH_VEC_ELT_REVERSE (rtx, forced_labels, ix, x)
+   make_label_edge (edge_cache, bb, x, EDGE_ABNORMAL);
}
 
  /* Returns create an exit out.  */
diff --git a/gcc/cfgrtl.c b/gcc/cfgrtl.c
index c450ca0..e01b7bb 100644
--- a/gcc/cfgrtl.c
+++ b/gcc/cfgrtl.c
@@ -65,7 +65,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "df.h"
 
 static int can_delete_note_p (const_rtx);
-static int can_delete_label_p (const_rtx);
+static bool can_delete_label_p (const_rtx);
 static basic_block rtl_split_edge (edge);
 static bool rtl_move_block_after (basic_block, basic_block);
 static int rtl_verify_flow_info (void);
@@ -101,13 +101,20 @@ can_delete_note_p (const_rtx note)
 
 /* True if a given label can be deleted.  */
 
-static int
+static bool
 can_delete_label_p (const_rtx label)
 {
-  return (!LABEL_PRESERVE_P (label)
- /* User declared labels must be preserved.  */
- && LABEL_NAME (label) == 0
- && !in_expr_list_p (forced_labels, label));
+  rtx x;
+  unsigned int ix;
+
+  if (LABEL_PRESERVE_P (label) || LABEL_NAME (label) != 0)
+return false;
+
+  FOR_EACH_VEC_ELT_REVERSE (rtx, forced_labels, ix, x)
+if (x == label)
+  return false;
+
+  return true;
 }
 
 /* Delete INSN by patching it out.  Return the next insn.  */
diff --git a/gcc/except.c b/gcc/except.c
index 5c6359e..858dde7 100644
--- a/gcc/except.c
+++ b/gcc/except.c
@@ -1233,8 +1233,7 @@ sjlj_emit_dispatch_table (rtx dispatch_label, int 
num_dispatch)
  label on the nonlocal_goto_label list.  Since we're modeling these
  CFG edges more exactly, we can use the forced_labels list instead.  */
   LABEL_PRESERVE_P (dispatch_label) = 1;
-  forced_labels
-= gen_rtx_EXPR_LIST (VOIDmode, dispatch_label, forced_labels);
+  note_forced_label (dispatch_label);
 #endif
 
   /* Load up exc_ptr and filter values from the function context.  */
diff --git a/gcc/function.h b/gcc/function.h
index fa44958..5b1ebfe 100644
--- a/gcc/function.h
+++ b/gcc/function.h
@@ -137,7 +137,7 @@ struct GTY(()) expr_status {
   rtx x_apply_args_value;
 
   /* List of labels that must never be deleted.  */
-  rtx x_forced_labels;
+  VEC(rtx,gc) *x_forced_labels;
 };
 
 typedef struct call_site_record_d *call_site_record;
@@ -463,6 +463,14 @@ extern GTY(()) struct rtl_data x_rtl;
want to do differently.  */
 #define crtl (&x_rtl)
 
+/* Add X to the forced_labels list.  */
+
+static inline void
+note_forced_label (rtx x)
+{
+  VEC_safe_push (rtx, gc, forced_labels, x);
+}
+
 struct GTY(()) stack_usage
 {
   /* # of bytes of static stack space allocated by the function.  */
diff --git a/gcc/jump.c b/gcc/jump.c
index 39fc234..7977f3d 100644
--- a/gcc/jump.c
+++ b/gcc/jump.c
@@ -76,7 +76,8 @@ static int returnjump_p_1 (rtx *, void *);
 static void
 rebuild_jump_labels_1 (rtx f, bool count_forced)
 {
-  rtx insn;
+  rtx x;
+  unsigned int ix;
 
   timevar_push (TV_REBUILD_JUMP);
   init_label_info (f);
@@ -87,9 +88,9 @@ rebuild_jump_labels_1 (rtx f, bool count_forced)
  count doesn't drop to z

[PATCH] convert nonlocal_goto_handler_labels to a VEC

[Nathan Froyd]

As $SUBJECT suggests.  The memory savings from this conversion is
negligible; the real benefit, IMHO, is use of a proper container instead
of EXPR_LIST.

remove_node_from_expr_list is unused after this patch; I will delete it
as an obvious followon patch if this patch is approved.

Tested on x86_64-unknown-linux-gnu.  OK to commit?

-Nathan

* function.h (struct rtl_data) [x_nonlocal_goto_handler_labels]:
Convert to a VEC.
(note_nonlocal_goto_handler_label): New function.
(remove_from_nonlocal_goto_handler_labels): New function.
* builtins.c (expand_builtin): Call them.
* cfgrtl.c (delete_insn): Call
remove_from_nonlocal_goto_handler_labels.
* stmt.c (expand_label): Call note_nonlocal_goto_handler_label.
* cfgbuild.c (make_edges): Adjust for new type of
nonlocal_goto_handler_labels.
* cfglayout.c (cfg_layout_initialize): Likewise.
* except.c (insn_nothrow_p): Likewise.
* recog.c (peep2_attempt): Likewise.
* sched-rgn.c (is_cfg_nonregular): Likewise.

diff --git a/gcc/builtins.c b/gcc/builtins.c
index b2534ce..839b212 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -6168,9 +6168,7 @@ expand_builtin (tree exp, rtx target, rtx subtarget, enum 
machine_mode mode,
 
  /* This is copied from the handling of non-local gotos.  */
  expand_builtin_setjmp_setup (buf_addr, label_r);
- nonlocal_goto_handler_labels
-   = gen_rtx_EXPR_LIST (VOIDmode, label_r,
-nonlocal_goto_handler_labels);
+ note_nonlocal_goto_handler_label (label_r);
  /* ??? Do not let expand_label treat us as such since we would
 not want to be both on the list of non-local labels and on
 the list of forced labels.  */
@@ -6188,7 +6186,7 @@ expand_builtin (tree exp, rtx target, rtx subtarget, enum 
machine_mode mode,
 
  /* Remove the dispatcher label from the list of non-local labels
 since the receiver labels have been added to it above.  */
- remove_node_from_expr_list (label_r, &nonlocal_goto_handler_labels);
+ remove_from_nonlocal_goto_handler_labels (label_r);
  return const0_rtx;
}
   break;
diff --git a/gcc/cfgbuild.c b/gcc/cfgbuild.c
index 6f0d69e..82b5e63 100644
--- a/gcc/cfgbuild.c
+++ b/gcc/cfgbuild.c
@@ -338,7 +338,8 @@ make_edges (basic_block min, basic_block max, int update_p)
  /* Add any appropriate EH edges.  */
  rtl_make_eh_edge (edge_cache, bb, insn);
 
- if (code == CALL_INSN && nonlocal_goto_handler_labels)
+ if (code == CALL_INSN
+ && !VEC_empty (rtx, nonlocal_goto_handler_labels))
{
  /* ??? This could be made smarter: in some cases it's possible
 to tell that certain calls will not do a nonlocal goto.
@@ -347,9 +348,13 @@ make_edges (basic_block min, basic_block max, int update_p)
 those functions or to other nested functions that use them
 could possibly do nonlocal gotos.  */
  if (can_nonlocal_goto (insn))
-   for (x = nonlocal_goto_handler_labels; x; x = XEXP (x, 1))
- make_label_edge (edge_cache, bb, XEXP (x, 0),
-  EDGE_ABNORMAL | EDGE_ABNORMAL_CALL);
+   {
+ unsigned int ix;
+ FOR_EACH_VEC_ELT_REVERSE (rtx, nonlocal_goto_handler_labels,
+   ix, x)
+   make_label_edge (edge_cache, bb, x,
+EDGE_ABNORMAL | EDGE_ABNORMAL_CALL);
+   }
}
}
 
diff --git a/gcc/cfglayout.c b/gcc/cfglayout.c
index 548e21f..38db3f4 100644
--- a/gcc/cfglayout.c
+++ b/gcc/cfglayout.c
@@ -1291,6 +1291,7 @@ cfg_layout_initialize (unsigned int flags)
 {
   rtx x;
   basic_block bb;
+  unsigned int ix;
 
   initialize_original_copy_tables ();
 
@@ -1299,9 +1300,9 @@ cfg_layout_initialize (unsigned int flags)
   record_effective_endpoints ();
 
   /* Make sure that the targets of non local gotos are marked.  */
-  for (x = nonlocal_goto_handler_labels; x; x = XEXP (x, 1))
+  FOR_EACH_VEC_ELT_REVERSE (rtx, nonlocal_goto_handler_labels, ix, x)
 {
-  bb = BLOCK_FOR_INSN (XEXP (x, 0));
+  bb = BLOCK_FOR_INSN (x);
   bb->flags |= BB_NON_LOCAL_GOTO_TARGET;
 }
 
diff --git a/gcc/cfgrtl.c b/gcc/cfgrtl.c
index c450ca0..a3e1202 100644
--- a/gcc/cfgrtl.c
+++ b/gcc/cfgrtl.c
@@ -134,7 +134,7 @@ delete_insn (rtx insn)
  NOTE_DELETED_LABEL_NAME (insn) = name;
}
 
-  remove_node_from_expr_list (insn, &nonlocal_goto_handler_labels);
+  remove_from_nonlocal_goto_handler_labels (insn);
 }
 
   if (really_delete)
diff --git a/gcc/except.c b/gcc/except.c
index 5c6359e..c8da137 100644
--- a/gcc/except.c
+++ b/gcc/except.c
@@ -1819,7 +1819,7 @@ insn_nothrow_p (const_rtx insn)
 bool
 can_nonlocal_goto (const

[PATCH][3/n] Fix built_int_cst callers with NULL type - expand_shift reorg

[Richard Guenther]

Noting that there is a single caller that calls expand_shift with a 
possible non-constant shift value this patch renames expand_shift
to expand_variable_shift and changes expand_shift to take a constant
integer shift amout, updating all callers to drop various variants
of building an integer constant.  Optimizing expand_variable_shift
and expand_shift is possible, but I'll leave that to a followup
and/or somebody else.

Bootstrap and regtest running on x86_64-unknown-linux-gnu, I'll apply
this after it succeeded.

Richard.

2011-04-29  Richard Guenther  

* expr.h (expand_shift): Rename to ...
(expand_variable_shift): ... this.
(expand_shift): Take a constant shift amount.
* expmed.c (expand_shift): Rename to ...
(expand_variable_shift): ... this.
(expand_shift): New wrapper around expand_variable_shift.
* expr.c (convert_move, emit_group_load_1, emit_group_store,
optimize_bitfield_assignment_op, store_field, expand_expr_real_2,
expand_expr_real_1, reduce_to_bit_field_precision): Adjust.
* expmed.c (store_fixed_bit_field, extract_bit_field_1,
extract_fixed_bit_field, extract_split_bit_field, expand_mult_const,
expand_mult, expand_widening_mult, expand_mult_highpart_adjust,
extract_high_half, expand_sdiv_pow2, expand_divmod, emit_cstore,
emit_store_flag_1, emit_store_flag): Likewise.
* builtins.c (expand_builtin_signbit): Likewise.
* calls.c (load_register_parameters): Likewise.
* function.c (assign_parm_setup_block): Likewise.
* lower-subreg.c (resolve_shift_zext): Likewise.
* optabs.c (widen_bswap, expand_abs_nojump,
expand_one_cmpl_abs_nojump, expand_float): Likewise.
* spu/spu.c (spu_expand_extv): Likewise.
* sparc/sparc.c (sparc32_initialize_trampoline): Likewise.

Index: gcc/expr.h
===
*** gcc/expr.h  (revision 173151)
--- gcc/expr.h  (working copy)
*** extern rtx expand_call (tree, rtx, int);
*** 488,495 
  extern void fixup_tail_calls (void);
  
  #ifdef TREE_CODE
! extern rtx expand_shift (enum tree_code, enum machine_mode, rtx, tree, rtx,
!int);
  extern rtx expand_divmod (int, enum tree_code, enum machine_mode, rtx, rtx,
  rtx, int);
  #endif
--- 488,497 
  extern void fixup_tail_calls (void);
  
  #ifdef TREE_CODE
! extern rtx expand_variable_shift (enum tree_code, enum machine_mode,
! rtx, tree, rtx, int);
! extern rtx expand_shift (enum tree_code, enum machine_mode, rtx, int, rtx,
!int);
  extern rtx expand_divmod (int, enum tree_code, enum machine_mode, rtx, rtx,
  rtx, int);
  #endif
Index: gcc/expr.c
===
*** gcc/expr.c  (revision 173151)
--- gcc/expr.c  (working copy)
*** convert_move (rtx to, rtx from, int unsi
*** 616,622 
{
  enum machine_mode intermediate;
  rtx tmp;
! tree shift_amount;
  
  /* Search for a mode to convert via.  */
  for (intermediate = from_mode; intermediate != VOIDmode;
--- 616,622 
{
  enum machine_mode intermediate;
  rtx tmp;
! int shift_amount;
  
  /* Search for a mode to convert via.  */
  for (intermediate = from_mode; intermediate != VOIDmode;
*** convert_move (rtx to, rtx from, int unsi
*** 636,644 
  
  /* No suitable intermediate mode.
 Generate what we need with shifts.  */
! shift_amount = build_int_cst (NULL_TREE,
!   GET_MODE_BITSIZE (to_mode)
!   - GET_MODE_BITSIZE (from_mode));
  from = gen_lowpart (to_mode, force_reg (from_mode, from));
  tmp = expand_shift (LSHIFT_EXPR, to_mode, from, shift_amount,
  to, unsignedp);
--- 636,643 
  
  /* No suitable intermediate mode.
 Generate what we need with shifts.  */
! shift_amount = (GET_MODE_BITSIZE (to_mode)
! - GET_MODE_BITSIZE (from_mode));
  from = gen_lowpart (to_mode, force_reg (from_mode, from));
  tmp = expand_shift (LSHIFT_EXPR, to_mode, from, shift_amount,
  to, unsignedp);
*** emit_group_load_1 (rtx *tmps, rtx dst, r
*** 1753,1759 
  
if (shift)
tmps[i] = expand_shift (LSHIFT_EXPR, mode, tmps[i],
!   build_int_cst (NULL_TREE, shift), tmps[i], 0);
  }
  }
  
--- 1752,1758 
  
if (shift)
tmps[i] = expand_shift (LSHIFT_EXPR, mode, tmps[i],
!   shift, tmps[i], 0);
  }
  }
  
*** emit_group_store (rtx orig_dst, rtx src,
*** 2050,2057 
{

[PATCH][2/n] Fix build_int_cst callers with NULL type

[Richard Guenther]

Some more obvious cases in the middle-end.  Next is expand_shift
and callers.

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

Richard.

2011-04-29  Richard Guenther  

* tree-inline.c (remap_eh_region_tree_nr): Use integer_type_node
for the remapped region number.
* predict.c (build_predict_expr): Use integer_type_node for the
predict kind.
* fold-const.c (fold_binary_loc): Use integer_type_node for
the shift amount.  Use a proper type for the PLUS_EXPR operand.

Index: gcc/tree-inline.c
===
*** gcc/tree-inline.c   (revision 173151)
--- gcc/tree-inline.c   (working copy)
*** remap_eh_region_tree_nr (tree old_t_nr,
*** 1204,1210 
old_nr = tree_low_cst (old_t_nr, 0);
new_nr = remap_eh_region_nr (old_nr, id);
  
!   return build_int_cst (NULL, new_nr);
  }
  
  /* Helper for copy_bb.  Remap statement STMT using the inlining
--- 1204,1210 
old_nr = tree_low_cst (old_t_nr, 0);
new_nr = remap_eh_region_nr (old_nr, id);
  
!   return build_int_cst (integer_type_node, new_nr);
  }
  
  /* Helper for copy_bb.  Remap statement STMT using the inlining
Index: gcc/predict.c
===
*** gcc/predict.c   (revision 173151)
--- gcc/predict.c   (working copy)
*** tree
*** 2291,2297 
  build_predict_expr (enum br_predictor predictor, enum prediction taken)
  {
tree t = build1 (PREDICT_EXPR, void_type_node,
!  build_int_cst (NULL, predictor));
SET_PREDICT_EXPR_OUTCOME (t, taken);
return t;
  }
--- 2291,2297 
  build_predict_expr (enum br_predictor predictor, enum prediction taken)
  {
tree t = build1 (PREDICT_EXPR, void_type_node,
!  build_int_cst (integer_type_node, predictor));
SET_PREDICT_EXPR_OUTCOME (t, taken);
return t;
  }
Index: gcc/fold-const.c
===
*** gcc/fold-const.c(revision 173151)
--- gcc/fold-const.c(working copy)
*** fold_binary_loc (location_t loc,
*** 11533,11539 
  
return fold_build2_loc (loc, RSHIFT_EXPR, type,
  TREE_OPERAND (arg0, 0),
! build_int_cst (NULL_TREE, pow2));
  }
}
  
--- 11533,11539 
  
return fold_build2_loc (loc, RSHIFT_EXPR, type,
  TREE_OPERAND (arg0, 0),
! build_int_cst (integer_type_node, pow2));
  }
}
  
*** fold_binary_loc (location_t loc,
*** 11565,11571 
   WARN_STRICT_OVERFLOW_MISC);
  
  sh_cnt = fold_build2_loc (loc, PLUS_EXPR, TREE_TYPE (sh_cnt),
!   sh_cnt, build_int_cst (NULL_TREE, pow2));
  return fold_build2_loc (loc, RSHIFT_EXPR, type,
  fold_convert_loc (loc, type, arg0), sh_cnt);
}
--- 11565,11573 
   WARN_STRICT_OVERFLOW_MISC);
  
  sh_cnt = fold_build2_loc (loc, PLUS_EXPR, TREE_TYPE (sh_cnt),
!   sh_cnt,
!   build_int_cst (TREE_TYPE (sh_cnt),
!  pow2));
  return fold_build2_loc (loc, RSHIFT_EXPR, type,
  fold_convert_loc (loc, type, arg0), sh_cnt);
}

Re: [pph] Save/restore PARM_DECL DECL_ARG_TYPE (issue4441079)

[Diego Novillo]

On Fri, Apr 29, 2011 at 04:29, Richard Guenther
 wrote:
> On Fri, Apr 29, 2011 at 3:10 AM, Lawrence Crowl  wrote:
>> This patch saves and restores the PARM_DECL DECL_ARG_TYPE in the PPH file.
>
> Should be already streamed via lto_output_ts_decl_common_tree_pointers
> as it is aliased to DECL_INITIAL.

No, I moved the streaming of DECL_INITIAL to a streamer hook because
in the gimple case we do more than streaming the initial value.  We
get the varpool node for the symbol and only stream the initial value
if we can find it.

Lawrence, I think we should just simply stream DECL_INITIAL in the
DECL_P case.  We are missing initializer expressions in every
variable, otherwise.


Diego.

Re: [patch, 4.5] Fix PR middle-end/43085 (make profiledbootstrap crashes due to dataflow bug)

[Ulrich Weigand]

Eric Botcazou wrote:
> > I guess that patch does indeed address both correctness and performance
> > aspects, but I wasn't sure attempting to pull those apart was a safer
> > option, given that at least in its current form it has seen testing in
> > mainline ...
> 
> Reasonable enough indeed.
> 
> > I'd certainly be happy to try out a more stripped down patch; could you be
> > more specific about exactly which parts you want me to omit?
> 
> The "We can perform the transformation if" thing, but in the end I agree that 
> it's probably better to backport the unmodified changes.  Thanks.

OK, thanks.

Bye,
Ulrich

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

Re: better wpa [1/n]: merge types during read-in

[Michael Matz]

Hi,

On Thu, 21 Apr 2011, Richard Guenther wrote:

> >> > It would have been nice to have the top-level tree merging as a
> >> > separate patch, as I am not convinced it is correct, but see below ...
> >>
> >> I'll split it out.
> >
> > Like so (also including the other remarks).
> >
> > Regstrapping on x86_64-linux in progress.
> 
> Ok if it passed.

r173155.


Ciao,
Michael.

[PATCH][1/n] Fix build_int_cst callers with NULL type

[Richard Guenther]

First batch, obvious in tail of grep.

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

Richard.

2011-04-29  Richard Guenther  

* tree-nested.c (get_trampoline_type): Use size_int.
(get_nl_goto_field): Likewise.
* tree-eh.c (lower_try_finally_switch): Use integer_type_node
for all indexes.
(lower_eh_constructs_2): Likewise.
(lower_resx): Likewise.
(lower_eh_dispatch): Likewise.
* tree-mudflap.c (mf_build_string): Use size_int.
(mudflap_register_call): Use integer_type_node for the flag.
(mudflap_enqueue_constant): Use size_int.
* tree-chrec.c (reset_evolution_in_loop): Copy CHREC_VAR
instead of rebuilding it.

Index: gcc/tree-nested.c
===
--- gcc/tree-nested.c   (revision 173151)
+++ gcc/tree-nested.c   (working copy)
@@ -486,7 +486,7 @@ get_trampoline_type (struct nesting_info
   align = STACK_BOUNDARY;
 }
 
-  t = build_index_type (build_int_cst (NULL_TREE, size - 1));
+  t = build_index_type (size_int (size - 1));
   t = build_array_type (char_type_node, t);
   t = build_decl (DECL_SOURCE_LOCATION (info->context),
  FIELD_DECL, get_identifier ("__data"), t);
@@ -561,7 +561,7 @@ get_nl_goto_field (struct nesting_info *
   size = size + 1;
 
   type = build_array_type
-   (type, build_index_type (build_int_cst (NULL_TREE, size)));
+   (type, build_index_type (size_int (size)));
 
   field = make_node (FIELD_DECL);
   DECL_NAME (field) = get_identifier ("__nl_goto_buf");
Index: gcc/tree-eh.c
===
--- gcc/tree-eh.c   (revision 173151)
+++ gcc/tree-eh.c   (working copy)
@@ -1336,11 +1336,12 @@ lower_try_finally_switch (struct leh_sta
   if (tf->may_fallthru)
 {
   x = gimple_build_assign (finally_tmp,
-  build_int_cst (NULL, fallthru_index));
+  build_int_cst (integer_type_node,
+ fallthru_index));
   gimple_seq_add_stmt (&tf->top_p_seq, x);
 
   last_case = build3 (CASE_LABEL_EXPR, void_type_node,
- build_int_cst (NULL, fallthru_index),
+ build_int_cst (integer_type_node, fallthru_index),
  NULL, create_artificial_label (tf_loc));
   VEC_quick_push (tree, case_label_vec, last_case);
   last_case_index++;
@@ -1358,14 +1359,14 @@ lower_try_finally_switch (struct leh_sta
   emit_post_landing_pad (&eh_seq, tf->region);
 
   x = gimple_build_assign (finally_tmp,
-  build_int_cst (NULL, eh_index));
+  build_int_cst (integer_type_node, eh_index));
   gimple_seq_add_stmt (&eh_seq, x);
 
   x = gimple_build_goto (finally_label);
   gimple_seq_add_stmt (&eh_seq, x);
 
   last_case = build3 (CASE_LABEL_EXPR, void_type_node,
- build_int_cst (NULL, eh_index),
+ build_int_cst (integer_type_node, eh_index),
  NULL, create_artificial_label (tf_loc));
   VEC_quick_push (tree, case_label_vec, last_case);
   last_case_index++;
@@ -1397,7 +1398,8 @@ lower_try_finally_switch (struct leh_sta
   if (q->index < 0)
{
  x = gimple_build_assign (finally_tmp,
-  build_int_cst (NULL, return_index));
+  build_int_cst (integer_type_node,
+ return_index));
  gimple_seq_add_stmt (&mod, x);
  do_return_redirection (q, finally_label, mod, &return_val);
  switch_id = return_index;
@@ -1405,7 +1407,7 @@ lower_try_finally_switch (struct leh_sta
   else
{
  x = gimple_build_assign (finally_tmp,
-  build_int_cst (NULL, q->index));
+  build_int_cst (integer_type_node, q->index));
  gimple_seq_add_stmt (&mod, x);
  do_goto_redirection (q, finally_label, mod, tf);
  switch_id = q->index;
@@ -1418,7 +1420,7 @@ lower_try_finally_switch (struct leh_sta
   tree case_lab;
   void **slot;
   case_lab = build3 (CASE_LABEL_EXPR, void_type_node,
- build_int_cst (NULL, switch_id),
+ build_int_cst (integer_type_node, switch_id),
 NULL, create_artificial_label (tf_loc));
   /* We store the cont_stmt in the pointer map, so that we can recover
  it in the loop below.  */
@@ -1863,7 +1865,8 @@ lower_eh_constructs_2 (struct leh_state
 this zero argument with the current catch region number.  */
  if (state->ehp_region)
{
- tree nr = build_int_cst (NULL, state->ehp_region->index);
+ 

[PATCH][6/n] Alias housekeeping

[Richard Guenther]

This makes PTA handle OBJ_TYPE_REF (not sure why I thought we need
to give up).

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

Richard.

2011-04-29  Richard Guenther  

* tree-ssa-structalias.c (get_fi_for_callee): Restructure.
Handle OBJ_TYPE_REF.
(find_func_aliases_for_call): Use it more consistently.

Index: gcc/tree-ssa-structalias.c
===
*** gcc/tree-ssa-structalias.c  (revision 173067)
--- gcc/tree-ssa-structalias.c  (working copy)
*** handle_pure_call (gimple stmt, VEC(ce_s,
*** 3925,3959 
  static varinfo_t
  get_fi_for_callee (gimple call)
  {
!   tree decl;
  
!   gcc_assert (!gimple_call_internal_p (call));
  
/* If we can directly resolve the function being called, do so.
   Otherwise, it must be some sort of indirect expression that
   we should still be able to handle.  */
!   decl = gimple_call_fndecl (call);
if (decl)
  return get_vi_for_tree (decl);
  
!   decl = gimple_call_fn (call);
!   /* The function can be either an SSA name pointer or,
!  worse, an OBJ_TYPE_REF.  In this case we have no
   clue and should be getting ANYFN (well, ANYTHING for now).  */
!   if (TREE_CODE (decl) == SSA_NAME)
! {
!   if (TREE_CODE (decl) == SSA_NAME
! && (TREE_CODE (SSA_NAME_VAR (decl)) == PARM_DECL
! || TREE_CODE (SSA_NAME_VAR (decl)) == RESULT_DECL)
! && SSA_NAME_IS_DEFAULT_DEF (decl))
!   decl = SSA_NAME_VAR (decl);
!   return get_vi_for_tree (decl);
! }
!   else if (TREE_CODE (decl) == INTEGER_CST
!  || TREE_CODE (decl) == OBJ_TYPE_REF)
  return get_varinfo (anything_id);
!   else
! gcc_unreachable ();
  }
  
  /* Create constraints for the builtin call T.  Return true if the call
--- 3925,3953 
  static varinfo_t
  get_fi_for_callee (gimple call)
  {
!   tree decl, fn = gimple_call_fn (call);
  
!   if (fn && TREE_CODE (fn) == OBJ_TYPE_REF)
! fn = OBJ_TYPE_REF_EXPR (fn);
  
/* If we can directly resolve the function being called, do so.
   Otherwise, it must be some sort of indirect expression that
   we should still be able to handle.  */
!   decl = gimple_call_addr_fndecl (fn);
if (decl)
  return get_vi_for_tree (decl);
  
!   /* If the function is anything other than a SSA name pointer we have no
   clue and should be getting ANYFN (well, ANYTHING for now).  */
!   if (!fn || TREE_CODE (fn) != SSA_NAME)
  return get_varinfo (anything_id);
! 
!   if ((TREE_CODE (SSA_NAME_VAR (fn)) == PARM_DECL
!|| TREE_CODE (SSA_NAME_VAR (fn)) == RESULT_DECL)
!   && SSA_NAME_IS_DEFAULT_DEF (fn))
! fn = SSA_NAME_VAR (fn);
! 
!   return get_vi_for_tree (fn);
  }
  
  /* Create constraints for the builtin call T.  Return true if the call
*** find_func_aliases_for_call (gimple t)
*** 4199,4209 
&& find_func_aliases_for_builtin_call (t))
  return;
  
if (!in_ipa_mode
!   || gimple_call_internal_p (t)
!   || (fndecl
! && (!(fi = lookup_vi_for_tree (fndecl))
! || !fi->is_fn_info)))
  {
VEC(ce_s, heap) *rhsc = NULL;
int flags = gimple_call_flags (t);
--- 4193,4201 
&& find_func_aliases_for_builtin_call (t))
  return;
  
+   fi = get_fi_for_callee (t);
if (!in_ipa_mode
!   || (fndecl && !fi->is_fn_info))
  {
VEC(ce_s, heap) *rhsc = NULL;
int flags = gimple_call_flags (t);
*** find_func_aliases_for_call (gimple t)
*** 4231,4238 
tree lhsop;
unsigned j;
  
-   fi = get_fi_for_callee (t);
- 
/* Assign all the passed arguments to the appropriate incoming
 parameters of the function.  */
for (j = 0; j < gimple_call_num_args (t); j++)
--- 4223,4228 
*** find_func_aliases_for_call (gimple t)
*** 4271,4277 
  VEC_free(ce_s, heap, tem);
}
  FOR_EACH_VEC_ELT (ce_s, lhsc, j, lhsp)
! process_constraint (new_constraint (*lhsp, rhs));
}
  
/* If we pass the result decl by reference, honor that.  */
--- 4261,4267 
  VEC_free(ce_s, heap, tem);
}
  FOR_EACH_VEC_ELT (ce_s, lhsc, j, lhsp)
!   process_constraint (new_constraint (*lhsp, rhs));
}
  
/* If we pass the result decl by reference, honor that.  */
*** find_func_aliases_for_call (gimple t)
*** 4286,4292 
  get_constraint_for_address_of (lhsop, &rhsc);
  lhs = get_function_part_constraint (fi, fi_result);
  FOR_EACH_VEC_ELT (ce_s, rhsc, j, rhsp)
! process_constraint (new_constraint (lhs, *rhsp));
  VEC_free (ce_s, heap, rhsc);
}
  
--- 4276,4282 
  get_constraint_for_address_of (lhsop, &rhsc);
  lhs = get_function_part_constraint (fi, fi_result);
  FOR_EACH_VEC_ELT (ce_s, rhsc, j, rhsp)
!   proce

Re: libgo patch committed: Inherit environment in http/cgi

[Rainer Orth]

Ian,

> This libgo patch brings over a patch to the master Go library to inherit
> environment variables in http/cgi.  This should fix PR go/48503.

not really :-)  It needs the following supplement to handle Solaris and
IRIX.  I'm not only including LD_LIBRARY_PATH (although this would suffice
for the testcase to pass), but also the ABI variants thereof.

Bootstrapped without regressions on i386-pc-solaris2.11 and
sparc-sun-solaris2.11.  Not yet tested on mips-sgi-irix6.5, but seems
pretty obvious.

Rainer


2011-04-26  Rainer Orth  

PR go/48503
* go/http/cgi/host.go (osDefaultInheritEnv): Pass LD_LIBRARY_PATH
and ABI variants on irix and solaris.

diff --git a/libgo/go/http/cgi/host.go b/libgo/go/http/cgi/host.go
--- a/libgo/go/http/cgi/host.go
+++ b/libgo/go/http/cgi/host.go
@@ -36,7 +36,9 @@ var osDefaultInheritEnv = map[string][]s
"darwin":  []string{"DYLD_LIBRARY_PATH"},
"freebsd": []string{"LD_LIBRARY_PATH"},
"hpux":[]string{"LD_LIBRARY_PATH", "SHLIB_PATH"},
+   "irix":[]string{"LD_LIBRARY_PATH", "LD_LIBRARYN32_PATH", 
"LD_LIBRARY64_PATH"},
"linux":   []string{"LD_LIBRARY_PATH"},
+   "solaris": []string{"LD_LIBRARY_PATH", "LD_LIBRARY_PATH_32", 
"LD_LIBRARY_PATH_64"},
"windows": []string{"SystemRoot", "COMSPEC", "PATHEXT", "WINDIR"},
 }
 

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

Re: [google]Add support for sampled profile collection (issue4438083)

[Richard Guenther]

On Fri, Apr 29, 2011 at 1:42 AM, Easwaran Raman  wrote:
> This patch from Silvius Rus  adds support for sampled edge profile collection 
> to reduce instrumentation run overhead. Bootstraps and no test regressions. 
> Ok for google/main?
>
> 2011-04-28  Silvius Rus  
>
>        * doc/invoke.texi: Document -fprofile-generate-sampling option.
>        * gcov-io.h (__gcov_set_sampling_rate): New declaration.
>        * profile.c (branch_prob): Add support for sampled profile
>        collection.
>        * profile.h (add_sampling_to_edge_counters): New declaration.
>        * common.opt (fprofile-generate-sampling): New option.
>        * tree-profile: Include header files; define EDGE_COUNTER_STMT_COUNT.
>        (instrumentation_to_be_sampled, gcov_sample_counter_decl)
>        (gcov_sampling_rate_decl): New globals.
>        (insert_if_then, add_sampling_wrapper, 
> is_instrumentation_to_be_sampled)
>        (add_sampling_to_edge_counters, gimple_init_instrumentation_sampling):
>        New functions.
>        (gimple_init_edge_profiler): Call gimple_init_instrumentation_sampling.
>        (gimple_gen_edge_profiler): Mark start of instrumentation block.
>        * libgcov.c (__gcov_sampling_rate): New extern declaration.
>        (gcov_sampling_rate_initialized, __gcov_sample_counter): New globals.
>        (gcov_exit): Set sampling rate; minor coding style fixes.
>        * params.def (PARAM_PROFILE_GENERATE_SAMPLING_RATE): New parameter.
>
> Index: gcc/doc/invoke.texi
> ===
> --- gcc/doc/invoke.texi (revision 173136)
> +++ gcc/doc/invoke.texi (working copy)
> @@ -375,7 +375,7 @@ Objective-C and Objective-C++ Dialects}.
>  -fpartial-inlining -fpeel-loops -fpredictive-commoning @gol
>  -fprefetch-loop-arrays @gol
>  -fprofile-correction -fprofile-dir=@var{path} -fprofile-generate @gol
> --fprofile-generate=@var{path} @gol
> +-fprofile-generate=@var{path} -fprofile-generate-sampling @gol
>  -fprofile-use -fprofile-use=@var{path} -fprofile-values @gol
>  -freciprocal-math -fregmove -frename-registers -freorder-blocks @gol
>  -freorder-blocks-and-partition -freorder-functions @gol
> @@ -7923,6 +7923,20 @@ The following options are enabled: @code{-fprofile
>  If @var{path} is specified, GCC will look at the @var{path} to find
>  the profile feedback data files. See @option{-fprofile-dir}.
>
> +@item -fprofile-generate-sampling
> +@opindex -fprofile-generate-sampling
> +
> +Enable sampling for instrumented binaries.  Instead of recording every event,
> +record only every N-th event, where N (the sampling rate) can be set either
> +at compile time using
> +@option{--param profile-generate-sampling-rate=@var{value}}, or
> +at execution start time through environment variable 
> @samp{GCOV_SAMPLING_RATE}.
> +
> +At this time sampling applies only to branch counters.  A sampling rate of 
> 100
> +decreases instrumentated binary slowdown from up to 20x for heavily threaded
> +applications down to around 2x.  @option{-fprofile-correction} is always
> +needed with sampling.
> +
>  @item -fprofile-use
>  @itemx -fprofile-use=@var{path}
>  @opindex fprofile-use
> @@ -9138,6 +9152,9 @@ recognize.
>  If you want to pass an option that takes an argument, you must use
>  @option{-Xassembler} twice, once for the option and once for the argument.
>
> +@item profile-generate-sampling-rate
> +Set the sampling rate with @option{-fprofile-generate-sampling}.
> +
>  @end table
>
>  @node Link Options
> Index: gcc/gcov-io.h
> ===
> --- gcc/gcov-io.h       (revision 173136)
> +++ gcc/gcov-io.h       (working copy)
> @@ -544,6 +544,9 @@ struct dyn_imp_mod
>  /* Register a new object file module.  */
>  extern void __gcov_init (struct gcov_info *) ATTRIBUTE_HIDDEN;
>
> +/* Set sampling rate to RATE.  */
> +extern void __gcov_set_sampling_rate (unsigned int rate);
> +
>  /* Called before fork, to avoid double counting.  */
>  extern void __gcov_flush (void) ATTRIBUTE_HIDDEN;
>
> Index: gcc/profile.c
> ===
> --- gcc/profile.c       (revision 173136)
> +++ gcc/profile.c       (working copy)
> @@ -1210,6 +1210,9 @@ branch_prob (void)
>
>       /* Commit changes done by instrumentation.  */
>       gsi_commit_edge_inserts ();
> +
> +      if (flag_profile_generate_sampling)
> +        add_sampling_to_edge_counters ();
>     }
>
>   free_aux_for_edges ();
> Index: gcc/profile.h
> ===
> --- gcc/profile.h       (revision 173136)
> +++ gcc/profile.h       (working copy)
> @@ -47,4 +47,10 @@ extern gcov_type sum_edge_counts (VEC (edge, gc) *
>  extern void init_node_map (void);
>  extern void del_node_map (void);
>
> +/* Implement sampling to avoid writing to edge counters very often.
> +   Many concurrent writes to the same counters, or to counters that share
> +   the same cache line leads t

Re: [google] Patch to support calling multi-versioned functions via new GCC builtin. (issue4440078)

[Richard Guenther]

On Fri, Apr 29, 2011 at 4:52 AM, Sriraman Tallam  wrote:
> I want this patch to be considered for google/main now. This is intended to 
> be submitted to trunk for review soon.
> This patch has been tested with crosstool bootstrap using buildit and by 
> running all tests.
>
>
> Patch Description :
> ==
>
> Frequently executed functions in programs are some times compiled
> into different versions to take advantage of some specific
> advanced features present in some of the types of platforms on
> which the program is executed. For instance, SSE4 versus no-SSE4.
> Existing techniques to do multi-versioning, for example using
> indirect calls, block compiler optimizations, mainly inlining.

The general idea of supporting versioning is good, thanks for working on it.
Comments below.

> This patch adds a new GCC pass to support multi-versioned calls
> through a new builtin, __builtin_dispatch.  The __builtin_dispatch
> call is converted into a simple if-then construct to call the
> appropriate version at run-time. There are no indirect calls so
> inlining is not affected.

I am not sure that combining the function choice and its invocation
to a single builtin is good.  First, you use variadic arguments for
the actual function arguments which will cause vararg promotions
to apply and thus it will be impossible to dispatch to functions
taking single precision float values (and dependent on ABI details
many more similar issues).  Second, you restrict yourself to
only two versions of a function - that looks quite limited and this
restriction is not easy to lift with your proposed interface.

Thus, I would have kept regular (but indirect) calls in the source
program and only exposed the dispatch via a builtin, like ...

> This patch also supports a function unswitching optimization via
> cloning of functions, only along hot paths, that call multi-versioned
> functions so that the hot multi-versioned paths can be independently
> optimized for performance. The cloning optimization is switched on
> via a flag, -fclone-hot-version-paths.
>
> Only two versions are supported in this patch. Example use :
>
>  int popcnt_sse4(unsigned int x) __attribute__((__target__("popcnt")));
>  int popcnt_sse4(unsigned int x)
>  {
>    int count = __builtin_popcount(x);
>    return count;
>  }
>
>  int popcnt(unsigned int x) __attribute__((__target__("no-popcnt")));
>  int popcnt(unsigned int x)
>  {
>    int count = __builtin_popcount(x);
>    return count;
>  }
>
>  int testsse() __attribute__((version_selector));
>  int main ()
>  {
>    ...
>    /* The multi-versioned call. */
>    ret = __builtin_dispatch (testsse,  (void*)popcnt_sse4, (void*)popcnt, 25);
>    ...
>  }

int main()
{
  int (*ppcnt)(unsigned int) = __builtin_dispatch (testsse,
popcnt_sse4, popcnt);
  ret = (*ppcnt) (25);
}

which would allow the testsse function to return the argument number of
the function to select.

[snip]

>  When to use the "version_selector" attribute ?
>  ---
>
>  Functions are marked with attribute "version_selector" only if
>  they are run-time constants.  Example of such functions would
>  be those that test if a specific feature is available on a
>  particular architecture.  Such functions must return a positive
>  integer. For two-way functions, those that test if a feature
>  is present or not must return 1 or 0 respectively.
>
>  This patch will make constructors that call these function once
>  and assign the outcome to a global variable. From then on, only
>  the value of the global will be used to decide which version to
>  execute.

That's a nice idea, but why not simply process all functions with
a const attribute and no arguments this way?  IMHO

int testsse (void) __attribute__((const));

is as good and avoids the new attribute completely.  The pass would
also benefit other uses of this idiom (giving a way to have global
dynamic initializers in C).  The functions may not be strictly 'const'
in a way the compiler autodetects this attribute but it presents
exactly the promises to the compiler that allow this optimization.

Thus, please split this piece out into a separate patch and use
the const attribute.

>  What happens with cloning, -fclone-hot-version-paths ?
>  -

Now, here you lost me somewhat, because I didn't look into the
patch details and I am missing an example on how the lowered
IL looks before that cloning.  So for now I suppose this
-fclone-hot-version-paths
is to expose direct calls to the IL.  If you would lower __builtin_dispatch
to a style like

  int sel = selector ();
  switch (sel)
 {
 case 0:
   fn = popcnt;
  break;
 case 1:
   fn = popcnt_sse4;
   break;
 }
   res = (*fn) (25);

then rather than a new pass specialized for __builtin_dispatch existing
optimization passes that are able to do tracing like VRP and DOM
via jump-threading or the tracer pass should be ab

Re: [libffi] Provide unwind info for Tru64 UNIX in osf.S

[Rainer Orth]

Richard Henderson  writes:

>> 2011-04-28  Rainer Orth  
>> 
>>  * src/alpha/osf.S (UA_SI, FDE_ENCODING, FDE_ENCODE, FDE_ARANGE):
>>  Define.
>>  Use them to handle ELF vs. ECOFF differences.
>>  [__osf__] (_GLOBAL__F_ffi_call_osf): Define.
>
> Looks good.  If there are any hidden Linux problems, we can
> fix them up with another patch.

I've installed it on mainline now.  Unless 4.5 and 4.6 testing over the
weekend shows any problems, I'll install there, too.  The 4.5 branch
shouldn't be affected by the bug Uros mentioned, so the Linux side could
be tested there.

Thanks.
Rainer

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

Re: [PR 48093] document -mtls-dialect opt (and GCC_COMPARE_DEBUG env)

[Rainer Orth]

Alexandre,

> Index: gcc/doc/invoke.texi
> ===
> --- gcc/doc/invoke.texi.orig  2011-04-27 15:59:46.0 -0300
> +++ gcc/doc/invoke.texi   2011-04-29 01:31:58.603523692 -0300
> @@ -12581,6 +12581,13 @@ SYSV ABI.  You can control this behavior
>  using the function attribute @samp{ms_abi}/@samp{sysv_abi}.
>  @xref{Function Attributes}.
>  
> +@item -mtls-dialect=@var{type}
> +@opindex mtls-dialect
> +Generate code to access thread-local storage using the @samp{gnu} or
> +@samp{gnu2} conventions.  @samp{gnu} is the conservative default;
> +@samp{gnu2} is more efficient, but it may add compile- and run-time
> +requirements that are cannot be satisfied on all systems.
 ^^^

Thanks for taking care of this.

Rainer

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

Re: [libffi] Provide unwind info for Tru64 UNIX in osf.S

[Uros Bizjak]

Hello!

> On 04/28/2011 10:55 AM, Rainer Orth wrote:
> > I cannot test the Alpha/Linux side, though.

> Neither can I, at the moment.

The bootstrap fails due to link error [1] on alphaev68-pc-linux-gnu
for some time ...

[1] http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47230

Uros.

Re: [pph] Save/restore PARM_DECL DECL_ARG_TYPE (issue4441079)

[Richard Guenther]

On Fri, Apr 29, 2011 at 3:10 AM, Lawrence Crowl  wrote:
> This patch saves and restores the PARM_DECL DECL_ARG_TYPE in the PPH file.

Should be already streamed via lto_output_ts_decl_common_tree_pointers
as it is aliased to DECL_INITIAL.

Richard.

> Index: gcc/cp/ChangeLog.pph
>
> 2011-04-28  Lawrence Crowl 
>
>        * pph-streamer-out.c (pph_stream_write_tree): Write PARM_DECL
>        DECL_ARG_TYPE.
>        * pph-streamer-in.c (pph_stream_read_tree): Read PARM_DECL
>        DECL_ARG_TYPE.
>
>
> Index: gcc/cp/pph-streamer-in.c
> ===
> --- gcc/cp/pph-streamer-in.c    (revision 173124)
> +++ gcc/cp/pph-streamer-in.c    (working copy)
> @@ -791,6 +791,8 @@ pph_stream_read_tree (struct lto_input_b
>          pph_stream_read_lang_specific (stream, expr);
>          if (TREE_CODE (expr) == FUNCTION_DECL)
>            DECL_SAVED_TREE (expr) = pph_input_tree (stream);
> +         else if (TREE_CODE (expr) == PARM_DECL)
> +           DECL_ARG_TYPE (expr) = pph_input_tree (stream);
>        }
>
>       if (TREE_CODE (expr) == TYPE_DECL)
> Index: gcc/cp/pph-streamer-out.c
> ===
> --- gcc/cp/pph-streamer-out.c   (revision 173124)
> +++ gcc/cp/pph-streamer-out.c   (working copy)
> @@ -796,6 +796,8 @@ pph_stream_write_tree (struct output_blo
>
>          if (TREE_CODE (expr) == FUNCTION_DECL)
>            pph_output_tree_or_ref_1 (stream, DECL_SAVED_TREE (expr), ref_p, 
> 3);
> +         else if (TREE_CODE (expr) == PARM_DECL)
> +           pph_output_tree_or_ref_1 (stream, DECL_ARG_TYPE (expr), ref_p, 3);
>        }
>
>       if (TREE_CODE (expr) == TYPE_DECL)
>
> --
> This patch is available for review at http://codereview.appspot.com/4441079
>

Re: Disable tracer by default for profile use (issue4428074)

[Richard Guenther]

2011/4/29 Sharad Singhai (शरद सिंघई) :
> On Thu, Apr 28, 2011 at 4:38 PM, Richard Guenther
>  wrote:
>>
>> On Fri, Apr 29, 2011 at 1:34 AM, Xinliang David Li 
>> wrote:
>> > Sharad can provide some some performance data -- we have seen up to 2%
>> > degradation to with tracer turned on for one of google's most
>> > important program. Perhaps Sharad can collect some SPEC numbers.
>> >
>> > I agree a better approach should be to fix the problem in tracer
>> > instead of turning it off in trunk.
>>
>> Esp. not turning it off for profile-use only where it should have the most
>> precise input.
>>
>
> Yes. As I explained, I am not proposing to turn -ftracer off in the trunk.

Ah, sorry for the confusion on my side.  If you can back up the change
with SPEC performance numbers it's ok for trunk as well.  But indeed
coverage of -ftracer will be zero then, and I wonder what it is useful
for if it doesn't even do good with FDO ...

Richard.

> After some tuning the pass should be improved.
> As of now, during the performance analysis of the internal benchmarks, I
> have seen up to 2% performance loss with -ftracer and hence turned it off
> during FDO.
> I would do SPEC performance numbers and send those out.
> Thanks,
> Sharad
>
>>
>> Richard.
>>
>> > David
>> >
>> >
>> > On Thu, Apr 28, 2011 at 4:23 PM, Richard Guenther
>> >  wrote:
>> >> On Thu, Apr 28, 2011 at 8:53 PM, Diego Novillo 
>> >> wrote:
>> >>> On Thu, Apr 28, 2011 at 14:51, Sharad Singhai 
>> >>> wrote:
>>  This patch disables -ftracer for profile use. Okay for google/main?
>> >>>
>> >>> Could you elaborate on why doing this is beneficial?  Are you
>> >>> proposing this for trunk as well?
>> >>
>> >> It indeed doesn't make sense to me at all.
>> >>
>> >> Richard.
>> >>
>>  2011-04-28  Sharad Singhai  
>> 
>>         Google Ref 40087
>>         * opts.c (common_handle_option): Disable -ftracer for profile
>>  use.
>>         * doc/invoke.texi: Update doc that -ftracer is no longer
>>  enabled for FDO.
>> >>>
>> >>> OK.
>> >>>
>> >>>
>> >>> Diego.
>> >>>
>> >>
>> >
>
>

Re: [PATCH PING] c++-specific bits of tree-slimming patches

[Alexandre Oliva]

On Apr 22, 2011, Jason Merrill  wrote:

> On 04/22/2011 02:13 AM, Mike Stump wrote:
>> http://gcc.gnu.org/ml/gcc/2005-04/msg00161.html
>> 
>> has the details of why the code was put in.

> Right.  At the time, we were sorting the goto queue based on pointer
> values, which caused the problem.  We no longer do that, so we
> shouldn't need this workaround (deferring creation of case label
> labels) anymore.

> What do you think, Alex?

Yeah, this change looks safe now.

-- 
Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist  Red Hat Brazil Compiler Engineer