Re: [Patch, Fortran] PR 47023: [4.6/4.7 regression] C_Sizeof: Rejects valid code

2011-10-17 Thread Paul Richard Thomas
Dear Janus,

Of course you can commit to 4.6.  Be quick, though; 4.6.2 was due for
release now-ish -
GCC 4.6 branch remains open under normal release branch rules,
accepting regression and documentation fixes.  GCC 4.6.2 is
tentatively planned for late September or early October.

Thanks

Paul

On Sun, Oct 16, 2011 at 9:45 PM, Janus Weil ja...@gcc.gnu.org wrote:
 Hi Paul,

 This is OK for trunk.  Thanks fo rthe patch.

 Thanks. Committed to trunk as r180062. What about 4.6?

 Cheers,
 Janus



 On Sun, Oct 16, 2011 at 2:58 PM, Janus Weil ja...@gcc.gnu.org wrote:
 Hi all,

 here is a patch which fixes the regression in comment #2 of the PR in
 the subject line. What it does is setting the 'ts.is_c_interop' flag
 correctly for constants with kind-parameter specification (such as
 '0.0_c_double'), as is already being done for variables.

 Regtested on x86_64-unknown-linux-gnu. Ok for trunk and 4.6?

 Cheers,
 Janus


 2011-10-16  Janus Weil  ja...@gcc.gnu.org

        PR fortran/47023
        * primary.c (match_kind_param): Detect ISO_C_BINDING kinds.
        (get_kind): Pass on 'is_iso_c' flag.
        (match_integer_constant,match_real_constant,match_logical_constant):
        Set 'ts.is_c_interop'.


 2011-10-16  Janus Weil  ja...@gcc.gnu.org

        PR fortran/47023
        * gfortran.dg/c_kind_tests_3.f03: New.




 --
 The knack of flying is learning how to throw yourself at the ground and miss.
        --Hitchhikers Guide to the Galaxy





-- 
The knack of flying is learning how to throw yourself at the ground and miss.
       --Hitchhikers Guide to the Galaxy


[pph] Function Merging (issue5278047)

2011-10-17 Thread Lawrence Crowl
Add function merging.  First, let function mangled names match.
Second, overwrite existing struct functions for merged functions.
Third, add filtering for already emitted functions.

The result is ICEs in the call graph code.

Add tests for overload sets with more than one member.

Bootstrapped on x64.


2011-10-16   Lawrence Crowl  cr...@google.com

Index: gcc/testsuite/ChangeLog.pph

* g++.dg/pph/c4inline.cc: Change to ICE in cgraph.
* g++.dg/pph/x1keyed.cc: Likewise.
* g++.dg/pph/x1keyno.cc: Likewise.
* g++.dg/pph/x4keyed.cc: Likewise.
* g++.dg/pph/x4keyex.cc: Likewise.
* g++.dg/pph/x4keyno.cc: Likewise.
* g++.dg/pph/x4tmplclass1.cc: Likewise.
* g++.dg/pph/x4tmplclass2.cc: Likewise.
* g++.dg/pph/x4tmplfuncinln.cc: Likewise.
* g++.dg/pph/x4tmplfuncninl.cc: Likewise.
* g++.dg/pph/x6rtti.cc: Likewise.
* g++.dg/pph/x7rtti.cc: Likewise.
* g++.dg/pph/z4tmplclass1.cc: Likewise.
* g++.dg/pph/z4tmplclass2.cc: Likewise.
* g++.dg/pph/z4tmplfuncinln.cc: Likewise.
* g++.dg/pph/z4tmplfuncninl.cc: Likewise.
* g++.dg/pph/x1tmplclass2.cc: Change to missing function.

Index: gcc/cp/ChangeLog.pph

2011-10-16   Lawrence Crowl  cr...@google.com

* pph-streamer-in.c (pph_match_to_overload): Comment on overloading.
(pph_match_to_function): Allow functions to match for merging.  
Comment on overloading.
(pph_match_to_link): Comment on overloading.
(pph_in_struct_function): Implement overwriting a struct function
when merging.
(pph_in_symtab): Add filtering for already emitted functions.
* pph-streamer.h (pph_trace_tree): Add a boolean parameter specifying
whether or not the tree was actually merged.
* pph-streamer.c (pph_trace_tree): Add a boolean parameter specifying
whether or not the tree was actually merged.  Change output to
correspond.  Update callers.


Index: gcc/testsuite/g++.dg/pph/x4tmplfuncninl.cc
===
--- gcc/testsuite/g++.dg/pph/x4tmplfuncninl.cc  (revision 180072)
+++ gcc/testsuite/g++.dg/pph/x4tmplfuncninl.cc  (working copy)
@@ -1,6 +1,5 @@
-// { dg-xfail-if ICE { *-*-* } { -fpph-map=pph.map } }
-// { dg-bogus a0tmplfuncninl_g.h:12:23: internal compiler error: in 
instantiate_decl  { xfail *-*-* } 0 }
-// { dg-excess-errors Template list problems }
+// { dg-xfail-if ICE CGRAPH { *-*-* } { -fpph-map=pph.map } }
+// { dg-bogus x4tmplfuncninl.cc:1:0: internal compiler error: in 
cgraph_create_node, at cgraph.c:502  { xfail *-*-* } 0 }
 
 #include x0tmplfuncninl1.h
 #include x0tmplfuncninl2.h
Index: gcc/testsuite/g++.dg/pph/z4tmplfuncninl.cc
===
--- gcc/testsuite/g++.dg/pph/z4tmplfuncninl.cc  (revision 180072)
+++ gcc/testsuite/g++.dg/pph/z4tmplfuncninl.cc  (working copy)
@@ -1,6 +1,5 @@
-// { dg-xfail-if ICE { *-*-* } { -fpph-map=pph.map } }
-// { dg-bogus a0tmplfuncninl_g.h:12:23: internal compiler error: in 
instantiate_decl  { xfail *-*-* } 0 }
-// { dg-excess-errors Template list problems }
+// { dg-xfail-if ICE CGRAPH { *-*-* } { -fpph-map=pph.map } }
+// { dg-bogus z4tmplfuncninl.cc:1:0: internal compiler error: in 
cgraph_create_node, at cgraph.c:502  { xfail *-*-* } 0 }
 
 #include x0tmplfuncninl3.h
 #include x0tmplfuncninl4.h
Index: gcc/testsuite/g++.dg/pph/x4keyno.cc
===
--- gcc/testsuite/g++.dg/pph/x4keyno.cc (revision 180071)
+++ gcc/testsuite/g++.dg/pph/x4keyno.cc (working copy)
@@ -1,5 +1,8 @@
-// { dg-xfail-if BOGUS MERGE AUXVAR { *-*-* } { -fpph-map=pph.map } }
-// { dg-bogus x4keyno.cc:12:1: error: redefinition of 'const char _ZTS5keyno 
 { xfail *-*-* } 0 }
+// { dg-xfail-if ICE CGRAPH { *-*-* } { -fpph-map=pph.map } }
+// { dg-bogus x4keyno.cc:15:1: internal compiler error: in 
cgraph_analyze_functions, at cgraphunit.c:1193  { xfail *-*-* } 0 }
+// { dg-excess-errors typeinfo name duplicatd }
+
+// Previously.
 // The variable for the typeinfo name for 'keyno' is duplicated.
 
 #include x0keyno1.h
Index: gcc/testsuite/g++.dg/pph/x1keyed.cc
===
--- gcc/testsuite/g++.dg/pph/x1keyed.cc (revision 180071)
+++ gcc/testsuite/g++.dg/pph/x1keyed.cc (working copy)
@@ -1,3 +1,6 @@
+// { dg-xfail-if ICE CGRAPH { *-*-* } { -fpph-map=pph.map } }
+// { dg-bogus x1keyed.cc:12:1: internal compiler error: in 
cgraph_analyze_functions, at cgraphunit.c:1193  { xfail *-*-* } 0 }
+
 #include x0keyed1.h
 
 int keyed::key( int arg ) { return mix( field  arg ); }
Index: gcc/testsuite/g++.dg/pph/x7rtti.cc
===
--- gcc/testsuite/g++.dg/pph/x7rtti.cc  (revision 180071)
+++ gcc/testsuite/g++.dg/pph/x7rtti.cc  (working copy)
@@ -1,18 +1,7 @@
 // FIXME pph: This should be a { dg=do run 

[patch] Fix PR tree-optimization/49960 ,Fix self data dependence

2011-10-17 Thread Razya Ladelsky
This patch fixes the failures described in 
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49960
It also fixes bzips when run with autopar enabled.

In both cases the self dependences are not handled correctly.
In the first case, a non affine access is analyzed:
in the second, the distance vector is not calculated correctly (the 
distance vector considered for for self dependences is always (0,0,...))

As  a result, the loops get wrongfully parallelized.

The patch avoids the special handling of  self dependences, and analyzes 
all dependences in the same way. Specific adjustments
and support for the self dependence cases were made.

Bootstrap and testsuite pass successfully for ppc64-redhat-linux.

OK for trunk?
Thank you,
Razya


ChangeLog:

PR tree-optimization/49960
* tree-data-ref.c (compute_self_dependence): Remove.
 (initialize_data_dependence_relation): Add intializations. 
Remove compute_self_dependence.
 (add_other_self_distances): Add support for two dimensions if 
the second is zero.
 (compute_affine_dependence): Remove the !DDR_SELF_REFERENCE 
condition.
 (compute_all_dependences): Remove call to 
compute_self_dependence. Add call to compute_affine_dependence

testsuite/ChangeLog:

PR tree-optimization/49660
* gcc.dg/autopar/pr49660.c: New test.
   * gcc.dg/autopar/pr49660-1.c: New test.













 

Index: tree-data-ref.c
===
--- tree-data-ref.c (revision 179799)
+++ tree-data-ref.c (working copy)
@@ -1346,7 +1346,6 @@ dr_may_alias_p (const struct data_reference *a, co
   return refs_may_alias_p (addr_a, addr_b);
 }
 
-static void compute_self_dependence (struct data_dependence_relation *);
 
 /* Initialize a data dependence relation between data accesses A and
B.  NB_LOOPS is the number of loops surrounding the references: the
@@ -1386,13 +1385,30 @@ initialize_data_dependence_relation (struct data_r
  the data dependence tests, just initialize the ddr and return.  */
   if (operand_equal_p (DR_REF (a), DR_REF (b), 0))
 {
+  if (loop_nest
+  !object_address_invariant_in_loop_p (VEC_index (loop_p, loop_nest, 
0),
+ DR_BASE_OBJECT (a)))
+   {
+ DDR_ARE_DEPENDENT (res) = chrec_dont_know;
+ return res;
+   }
   DDR_AFFINE_P (res) = true;
   DDR_ARE_DEPENDENT (res) = NULL_TREE;
   DDR_SUBSCRIPTS (res) = VEC_alloc (subscript_p, heap, DR_NUM_DIMENSIONS 
(a));
   DDR_LOOP_NEST (res) = loop_nest;
   DDR_INNER_LOOP (res) = 0;
   DDR_SELF_REFERENCE (res) = true;
-  compute_self_dependence (res);
+  for (i = 0; i  DR_NUM_DIMENSIONS (a); i++)
+   {
+ struct subscript *subscript;
+ 
+ subscript = XNEW (struct subscript);
+ SUB_CONFLICTS_IN_A (subscript) = conflict_fn_not_known ();
+ SUB_CONFLICTS_IN_B (subscript) = conflict_fn_not_known ();
+ SUB_LAST_CONFLICT (subscript) = chrec_dont_know;
+ SUB_DISTANCE (subscript) = chrec_dont_know;
+ VEC_safe_push (subscript_p, heap, DDR_SUBSCRIPTS (res), subscript);
+   }
   return res;
 }
 
@@ -3119,8 +3135,11 @@ add_other_self_distances (struct data_dependence_r
{
  if (DDR_NUM_SUBSCRIPTS (ddr) != 1)
{
- DDR_ARE_DEPENDENT (ddr) = chrec_dont_know;
- return;
+ if (DDR_NUM_SUBSCRIPTS (ddr) != 2 || !integer_zerop 
(DR_ACCESS_FN (DDR_A (ddr), 1)))
+   {
+ DDR_ARE_DEPENDENT (ddr) = chrec_dont_know;
+ return;
+   }
}
 
  access_fun = DR_ACCESS_FN (DDR_A (ddr), 0);
@@ -4037,8 +4056,7 @@ compute_affine_dependence (struct data_dependence_
 }
 
   /* Analyze only when the dependence relation is not yet known.  */
-  if (DDR_ARE_DEPENDENT (ddr) == NULL_TREE
-   !DDR_SELF_REFERENCE (ddr))
+  if (DDR_ARE_DEPENDENT (ddr) == NULL_TREE)
 {
   dependence_stats.num_dependence_tests++;
 
@@ -4113,39 +4131,6 @@ compute_affine_dependence (struct data_dependence_
 fprintf (dump_file, )\n);
 }
 
-/* This computes the dependence relation for the same data
-   reference into DDR.  */
-
-static void
-compute_self_dependence (struct data_dependence_relation *ddr)
-{
-  unsigned int i;
-  struct subscript *subscript;
-
-  if (DDR_ARE_DEPENDENT (ddr) != NULL_TREE)
-return;
-
-  for (i = 0; VEC_iterate (subscript_p, DDR_SUBSCRIPTS (ddr), i, subscript);
-   i++)
-{
-  if (SUB_CONFLICTS_IN_A (subscript))
-   free_conflict_function (SUB_CONFLICTS_IN_A (subscript));
-  if (SUB_CONFLICTS_IN_B (subscript))
-   free_conflict_function (SUB_CONFLICTS_IN_B (subscript));
-
-  /* The accessed index overlaps for each iteration.  */
-  SUB_CONFLICTS_IN_A (subscript)
-   = conflict_fn (1, affine_fn_cst 

Re: [C++ Patch] PR 32614

2011-10-17 Thread Richard Guenther
On Sun, Oct 16, 2011 at 1:03 PM, Gabriel Dos Reis
g...@integrable-solutions.net wrote:
 On Sun, Oct 16, 2011 at 5:42 AM, Richard Guenther
 richard.guent...@gmail.com wrote:
 On Sun, Oct 16, 2011 at 12:31 PM, Paolo Carlini
 paolo.carl...@oracle.com wrote:
 On 10/16/2011 12:28 PM, Gabriel Dos Reis wrote:

 On Sun, Oct 16, 2011 at 5:03 AM, Paolo Carlinipaolo.carl...@oracle.com
  wrote:

 Hi,

 in this simple documentation PR, Tom noticed that we have a (very long
 standing) inconsistency between the default value of -fmessage-length for
 C++ as documented and as implemented: in fact it's 0 in
 cxx-pretty-print.c,
 like all the other front-ends. At the time of the PR people briefly
 envisage
 changing the default but the discussion didn't go anywhere, I think we
 can
 as well do the below, for the time being at least, remove the
 inconsistency.

 Ok?

 I still think the default for g++ should be 72.  Notice that other
 front-ends have set it to zero because they feared something, unlike the
 C++ frontend.

 To be clear, I have no strong opinion. But last time Richard Gunther
 strongly disagreed (and now he is a Global Reviewer ;) Thus, just let me
 know guys...

 0 is just so much more convenient for consumers and all consumers that
 care for line lengths can properly wrap around.  So I don't see a good
 reason to have -fmessage-length at all.

 This is an argument that should have been made more than a decade ago
 and -fmessage-length was *requested* by users who did care about
 line length, and implemented for g++.

I wasn't around at that time, so sorry for not raising my opinion earlier ;)

Richard.


Re: [PATCH] Clear DECL_GIMPLE_REG_P when making parameter copy addressable (PR tree-optimization/50735)

2011-10-17 Thread Richard Guenther
On Sun, Oct 16, 2011 at 5:47 PM, Jakub Jelinek ja...@redhat.com wrote:
 Hi!

 gimplify_parameters uses create_tmp_reg, but sometimes it decides to make
 it addressable (if the PARM_DECL is addressable).  If so, it must not be
 DECL_GIMPLE_REG_P.

 Alternatively we could call create_tmp_reg only if !TREE_ADDRESSABLE and
 call create_tmp_var instead for TREE_ADDRESSABLE (+ set TREE_ADDRESSABLE).

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

I think this should be exactly the other way around, using create_tmp_var,
copying TREE_ADDRESSABLE and setting DECL_GIMPLE_REG_P if it is not addressable.

Ok with that change.

Thanks,
Richard.

 2011-10-16  Jakub Jelinek  ja...@redhat.com

        PR tree-optimization/50735
        * function.c (gimplify_parameters): When making local TREE_ADDRESSABLE,
        also clear DECL_GIMPLE_REG_P.

 --- gcc/function.c.jj   2011-10-14 08:21:56.0 +0200
 +++ gcc/function.c      2011-10-15 12:43:23.0 +0200
 @@ -3624,7 +3624,10 @@ gimplify_parameters (void)
                     not the PARMs.  Keep the parms address taken
                     as we'll query that flag during gimplification.  */
                  if (TREE_ADDRESSABLE (parm))
 -                   TREE_ADDRESSABLE (local) = 1;
 +                   {
 +                     TREE_ADDRESSABLE (local) = 1;
 +                     DECL_GIMPLE_REG_P (local) = 0;
 +                   }
                }
              else
                {

        Jakub



Re: [PATCH 1/5] Use MADV_DONTNEED for freeing in garbage collector

2011-10-17 Thread Richard Guenther
On Sun, Oct 16, 2011 at 8:33 PM, Andi Kleen a...@firstfloor.org wrote:
 On Sun, Oct 16, 2011 at 12:38:16PM +0200, Richard Guenther wrote:
 On Sun, Oct 16, 2011 at 7:30 AM, Andi Kleen a...@firstfloor.org wrote:
  Andi Kleen a...@firstfloor.org writes:
 
  From: Andi Kleen a...@linux.intel.com
 
  Use the Linux MADV_DONTNEED call to unmap free pages in the garbage
  collector.Then keep the unmapped pages in the free list. This avoid
  excessive memory fragmentation on large LTO bulds, which can lead
  to gcc bumping into the Linux vm_max_map limit per process.
 
  Could I have a decision on this patch please? The problem in PR50636
  is still there and this is the minimum fix to fix it on Linux
  as far as I know.
 
  If this patch is not the right way to go I would
  appreciate some guidance on an alternative (but low cost)
  implementation. Note I don't have capacity for any overly
  complicated solutions.

 The patch looks generally ok, but you are never giving back pages to the

 It gives back pages, just not virtual address space. But I guess that is
 what you meant.

 On the other hand this patch can actually give you more virtual
 address space when you need large regions (2 pages or so).
 The reason is that the old allocation pattern fragments the whole
 address space badly and only leaves these small holes. With the madvise
 patch that does not happen, ggc is all in a compacted chunk.

Sure, but we do compete with the glibc heap with virtual memory usage
(I wonder if GGC should simply use malloc/free ...).  So I am worried
that we run out of address space earlier this way.  But I guess we can
revisit this when we run into actual problems ...

 system, and as we have other memory allocations that do not use the
 ggc pools you drain virtual memory on 32bit hosts.  Is any other patch
 in this series compensating for it?  If not I'd say we should munmap the
 pages when a full mapped range (2MB) is free.  Can you rename

 I wrote such a patch initially, but ran into various problems, so
 I dropped it from the series. I can revisit it.

Yes, please revisit it.  It should be as simple as scanning for a
large chunk in free_pages I suppose.

 'unmapped' to 'discarded' please?  That would be less confusing.

 Ok I can do that.

 Was that an approval?

Ok with the rename.

Thanks,
Richard.

 -Andi



Re: [patch] Fix PR tree-optimization/49960 ,Fix self data dependence

2011-10-17 Thread Richard Guenther
On Mon, Oct 17, 2011 at 8:23 AM, Razya Ladelsky ra...@il.ibm.com wrote:
 This patch fixes the failures described in
 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49960
 It also fixes bzips when run with autopar enabled.

 In both cases the self dependences are not handled correctly.
 In the first case, a non affine access is analyzed:
 in the second, the distance vector is not calculated correctly (the
 distance vector considered for for self dependences is always (0,0,...))

 As  a result, the loops get wrongfully parallelized.

 The patch avoids the special handling of  self dependences, and analyzes
 all dependences in the same way. Specific adjustments
 and support for the self dependence cases were made.

Can you elaborate on

@@ -3119,8 +3135,11 @@ add_other_self_distances (struct data_dependence_r
{
  if (DDR_NUM_SUBSCRIPTS (ddr) != 1)
{
- DDR_ARE_DEPENDENT (ddr) = chrec_dont_know;
- return;
+ if (DDR_NUM_SUBSCRIPTS (ddr) != 2 || !integer_zerop 
(DR_ACCESS_FN
(DDR_A (ddr), 1)))
+   {
+ DDR_ARE_DEPENDENT (ddr) = chrec_dont_know;
+ return;
+   }
}

  access_fun = DR_ACCESS_FN (DDR_A (ddr), 0);

?  It needed a comment before, and now so even more.

The rest of the patch is ok, I suppose the above hunk is to enhance
something, not
to fix the bug?

Thanks,
Richard.

 Bootstrap and testsuite pass successfully for ppc64-redhat-linux.

 OK for trunk?
 Thank you,
 Razya


 ChangeLog:

        PR tree-optimization/49960
        * tree-data-ref.c (compute_self_dependence): Remove.
             (initialize_data_dependence_relation): Add intializations.
 Remove compute_self_dependence.
             (add_other_self_distances): Add support for two dimensions if
 the second is zero.
             (compute_affine_dependence): Remove the !DDR_SELF_REFERENCE
 condition.
             (compute_all_dependences): Remove call to
 compute_self_dependence. Add call to compute_affine_dependence

 testsuite/ChangeLog:

        PR tree-optimization/49660
        * gcc.dg/autopar/pr49660.c: New test.
           * gcc.dg/autopar/pr49660-1.c: New test.


















Re: [patch, testsuite, ARM] Skip architecture option in pr42575.c

2011-10-17 Thread Ramana Radhakrishnan
On 28 September 2011 09:48, Joey Ye joey...@arm.com wrote:
 2011-09-28  Joey Ye  joey...@arm.com

        * gcc.target/arm/pr42575.c: Remove architecture option.

OK.

FTR - Joey had checked that it ran ok with an optional march=armv5te
in RUNTESTFLAGS .


Ramana


Re: [PATCH 3/7] Emit macro expansion related diagnostics

2011-10-17 Thread Dodji Seketeli
Jason Merrill ja...@redhat.com writes:

 On 10/13/2011 01:12 PM, Dodji Seketeli wrote:
 +  while (true)
 +{
 +  if (!linemap_macro_expansion_map_p (map0)
 + || !linemap_macro_expansion_map_p (map1)
 + || map0 == map1)
 +   break;

 I'd put the test in the condition, but if you find it clearer this
 way, I guess that's fine.

I've done the change.

Let's go ahead and check all this in.

Thanks.

I'll send the final set of patches I'll commit, to ease future
references.  It will not contain the patch which subject was Kill
pedantic warnings on system headers macros as I'll need to change the C
(and probably the C++) FE to make each declspec have its own location
first.

-- 
Dodji


[patch] Update gcc.dg/vect/vect-21.c

2011-10-17 Thread Ira Rosen
Hi,

With Jakub's patch for bool types the 3 loops in gcc.dg/vect/vect-21.c
are now vectorizable on targets that support vector conditions.

Tested on powerpc64-suse-linux.
Committed.

Ira

testsuite/ChangeLog:

   * gcc.dg/vect/vect-21.c: Expect the loops to get vectorized on
   targets that support vector condition.

Index: testsuite/gcc.dg/vect/vect-21.c
===
--- testsuite/gcc.dg/vect/vect-21.c (revision 180075)
+++ testsuite/gcc.dg/vect/vect-21.c (working copy)
@@ -123,7 +123,7 @@ int main (void)
   return main1 ();
 }

-/* { dg-final { scan-tree-dump-times vectorized 3 loops 1 vect {
xfail *-*-* } } } */
+/* { dg-final { scan-tree-dump-times vectorized 3 loops 1 vect {
target vect_condition } } } */
 /* { dg-final { scan-tree-dump-times Vectorizing an unaligned
access 0 vect } } */

 /* { dg-final { cleanup-tree-dump vect } } */


Re: [PATCH] fix -Wsign-compare error in objc-act.c (PR objc/50743)

2011-10-17 Thread Nicola Pero
 (I don't have svn write access so I'll need someone else to commit it if
 it's approved.)

I can apply it for you.  But ... do you have a copyright assignment in place
for contributions to GCC ?  The patch looks small and trivial enough that
I think I can apply it without a signed copyright assignment form, but if you
plan on contributing more, it would make sense to sign one. :-)

Thanks


[PATCH 0/6] Tracking locations of tokens resulting from macro expansion

2011-10-17 Thread Dodji Seketeli
This set of patches to track locations of tokens access macro
expansion was reviewed and accepted at
http://gcc.gnu.org/ml/gcc-patches/2011-10/msg01346.html.

I have bootstrapped and tested it on x86_64-unknown-linux-gnu against
trunk and I am checking it in now.

Thanks.


[PATCH 4/6] Support -fdebug-cpp option

2011-10-17 Thread Dodji Seketeli
This patch adds -fdebug-cpp option. When used with -E this dumps the
relevant macro map before every single token. This clutters the output
a lot but has proved to be invaluable in tracking some bugs during the
development of the virtual location support.

gcc/ChangeLog
2011-10-15  Tom Tromey  tro...@redhat.com
Dodji Seketeli  do...@redhat.com

* doc/cppopts.texi: Document -fdebug-cpp.
* doc/invoke.texi: Add -fdebug-cpp to the list of preprocessor
options.

gcc/c-family/ChangeLog
2011-10-15  Tom Tromey  tro...@redhat.com
Dodji Seketeli  do...@redhat.com

* c.opt (fdebug-cpp): New option.
* c-opts.c (c_common_handle_option): Handle the option.
* c-ppoutput.c (maybe_print_line_1): New static function. Takes an
output stream in parameter. Factorized from ...
(maybe_print_line): ... this. Dump location debug information when
-fdebug-cpp is in effect.
(print_line_1): New static function. Takes an output stream in
parameter. Factorized from ...
(print_line): ... here. Dump location information when -fdebug-cpp
is in effect.
(scan_translation_unit): Dump location information when
-fdebug-cpp is in effect.

libcpp/ChangeLog
2011-10-15  Tom Tromey  tro...@redhat.com
Dodji Seketeli  do...@redhat.com

* include/cpplib.h (struct cpp_options)debug: New struct member.
* include/line-map.h (linemap_dump_location): Declare ...
* line-map.c (linemap_dump_location): ... new function.

---
 gcc/ChangeLog |7 +
 gcc/c-family/ChangeLog|   16 
 gcc/c-family/c-opts.c |4 +++
 gcc/c-family/c-ppoutput.c |   57 
 gcc/c-family/c.opt|4 +++
 gcc/doc/cppopts.texi  |   13 ++
 gcc/doc/invoke.texi   |2 +-
 libcpp/ChangeLog  |7 +
 libcpp/include/cpplib.h   |4 +++
 libcpp/include/line-map.h |4 +++
 libcpp/line-map.c |   38 ++
 11 files changed, 144 insertions(+), 12 deletions(-)

diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c
index 3184539..6869d5c 100644
--- a/gcc/c-family/c-opts.c
+++ b/gcc/c-family/c-opts.c
@@ -628,6 +628,10 @@ c_common_handle_option (size_t scode, const char *arg, int 
value,
   cpp_opts-preprocessed = value;
   break;
 
+case OPT_fdebug_cpp:
+  cpp_opts-debug = 1;
+  break;
+
 case OPT_ftrack_macro_expansion:
   if (value)
value = 2;
diff --git a/gcc/c-family/c-ppoutput.c b/gcc/c-family/c-ppoutput.c
index 892f1ea..df46ce4 100644
--- a/gcc/c-family/c-ppoutput.c
+++ b/gcc/c-family/c-ppoutput.c
@@ -59,7 +59,9 @@ static void account_for_newlines (const unsigned char *, 
size_t);
 static int dump_macro (cpp_reader *, cpp_hashnode *, void *);
 static void dump_queued_macros (cpp_reader *);
 
+static void print_line_1 (source_location, const char*, FILE *);
 static void print_line (source_location, const char *);
+static void maybe_print_line_1 (source_location, FILE *);
 static void maybe_print_line (source_location);
 static void do_line_change (cpp_reader *, const cpp_token *,
source_location, int);
@@ -243,7 +245,12 @@ scan_translation_unit (cpp_reader *pfile)
  in_pragma = false;
}
   else
-   cpp_output_token (token, print.outf);
+   {
+ if (cpp_get_options (parse_in)-debug)
+ linemap_dump_location (line_table, token-src_loc,
+print.outf);
+ cpp_output_token (token, print.outf);
+   }
 
   if (token-type == CPP_COMMENT)
account_for_newlines (token-val.str.text, token-val.str.len);
@@ -297,8 +304,9 @@ scan_translation_unit_trad (cpp_reader *pfile)
 /* If the token read on logical line LINE needs to be output on a
different line to the current one, output the required newlines or
a line marker, and return 1.  Otherwise return 0.  */
+
 static void
-maybe_print_line (source_location src_loc)
+maybe_print_line_1 (source_location src_loc, FILE *stream)
 {
   int src_line = LOCATION_LINE (src_loc);
   const char *src_file = LOCATION_FILE (src_loc);
@@ -306,7 +314,7 @@ maybe_print_line (source_location src_loc)
   /* End the previous line of text.  */
   if (print.printed)
 {
-  putc ('\n', print.outf);
+  putc ('\n', stream);
   print.src_line++;
   print.printed = 0;
 }
@@ -318,22 +326,37 @@ maybe_print_line (source_location src_loc)
 {
   while (src_line  print.src_line)
{
- putc ('\n', print.outf);
+ putc ('\n', stream);
  print.src_line++;
}
 }
   else
-print_line (src_loc, );
+print_line_1 (src_loc, , stream);
+
+}
+
+/* If the token read on logical line LINE needs to be output on a
+   different line to the current one, output the required newlines or
+   a line marker, and return 1.  Otherwise 

[PATCH 6/6] Reduce memory waste due to non-power-of-2 allocs

2011-10-17 Thread Dodji Seketeli
This patch basically arranges for the allocation size of line_map
buffers to be as close as possible to a power of two.  This
*significantly* decreases peak memory consumption as (macro) maps are
numerous and stay live during all the compilation.

The patch adds a new ggc_round_alloc_size interface to the ggc
allocator.  In each of the two main allocator implementations ('page'
and 'zone') the function has been extracted from the main allocation
function code and returns the actual size of the allocated memory
region, thus giving a chance to the caller to maximize the amount of
memory it actually uses from the allocated memory region.  In the
'none' allocator implementation (that uses xmalloc) the
ggc_round_alloc_size just returns the requested allocation size.

gcc/ChangeLog
2011-10-15  Tom Tromey  tro...@redhat.com
Dodji Seketeli  do...@redhat.com

* ggc.h (ggc_round_alloc_size): Declare new public entry point.
* ggc-none.c (ggc_round_alloc_size): New public stub function.
* ggc-page.c (ggc_alloced_size_order_for_request): New static
function.  Factorized from ggc_internal_alloc_stat.
(ggc_round_alloc_size): New public function.  Uses
ggc_alloced_size_order_for_request.
(ggc_internal_alloc_stat): Use ggc_alloced_size_order_for_request.
* ggc-zone.c (ggc_round_alloc_size): New public function extracted
from ggc_internal_alloc_zone_stat.
(ggc_internal_alloc_zone_stat): Use ggc_round_alloc_size.
* toplev.c (general_init): Initialize
line_table-alloced_size_for_request.

libcpp/ChangeLog
2011-10-15  Tom Tromey  tro...@redhat.com
Dodji Seketeli  do...@redhat.com

* include/line-map.h (struct line_maps::alloced_size_for_request):
New member.
* line-map.c (new_linemap): Use set-alloced_size_for_request to
get the actual allocated size of line maps.

---
 gcc/ChangeLog |   16 +
 gcc/ggc-none.c|9 +++
 gcc/ggc-page.c|   53 +++-
 gcc/ggc-zone.c|   27 --
 gcc/ggc.h |2 +
 gcc/toplev.c  |1 +
 libcpp/ChangeLog  |8 ++
 libcpp/include/line-map.h |8 ++
 libcpp/line-map.c |   39 -
 9 files changed, 138 insertions(+), 25 deletions(-)

diff --git a/gcc/ggc-none.c b/gcc/ggc-none.c
index 97d25b9..e57d617 100644
--- a/gcc/ggc-none.c
+++ b/gcc/ggc-none.c
@@ -39,6 +39,15 @@ ggc_alloc_typed_stat (enum gt_types_enum ARG_UNUSED (gte), 
size_t size
   return xmalloc (size);
 }
 
+/* For a given size of memory requested for allocation, return the
+   actual size that is going to be allocated.  */
+
+size_t
+ggc_round_alloc_size (size_t requested_size)
+{
+  return requested_size;
+}
+
 void *
 ggc_internal_alloc_stat (size_t size MEM_STAT_DECL)
 {
diff --git a/gcc/ggc-page.c b/gcc/ggc-page.c
index 624f029..f919a6b 100644
--- a/gcc/ggc-page.c
+++ b/gcc/ggc-page.c
@@ -1054,6 +1054,47 @@ static unsigned char size_lookup[NUM_SIZE_LOOKUP] =
   9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9
 };
 
+/* For a given size of memory requested for allocation, return the
+   actual size that is going to be allocated, as well as the size
+   order.  */
+
+static void
+ggc_round_alloc_size_1 (size_t requested_size,
+   size_t *size_order,
+   size_t *alloced_size)
+{
+  size_t order, object_size;
+
+  if (requested_size  NUM_SIZE_LOOKUP)
+{
+  order = size_lookup[requested_size];
+  object_size = OBJECT_SIZE (order);
+}
+  else
+{
+  order = 10;
+  while (requested_size  (object_size = OBJECT_SIZE (order)))
+order++;
+}
+
+  if (size_order)
+*size_order = order;
+  if (alloced_size)
+*alloced_size = object_size;
+}
+
+/* For a given size of memory requested for allocation, return the
+   actual size that is going to be allocated.  */
+
+size_t
+ggc_round_alloc_size (size_t requested_size)
+{
+  size_t size = 0;
+  
+  ggc_round_alloc_size_1 (requested_size, NULL, size);
+  return size;
+}
+
 /* Typed allocation function.  Does nothing special in this collector.  */
 
 void *
@@ -1072,17 +1113,7 @@ ggc_internal_alloc_stat (size_t size MEM_STAT_DECL)
   struct page_entry *entry;
   void *result;
 
-  if (size  NUM_SIZE_LOOKUP)
-{
-  order = size_lookup[size];
-  object_size = OBJECT_SIZE (order);
-}
-  else
-{
-  order = 10;
-  while (size  (object_size = OBJECT_SIZE (order)))
-   order++;
-}
+  ggc_round_alloc_size_1 (size, order, object_size);
 
   /* If there are non-full pages for this size allocation, they are at
  the head of the list.  */
diff --git a/gcc/ggc-zone.c b/gcc/ggc-zone.c
index d0c1d79..79c8c03 100644
--- a/gcc/ggc-zone.c
+++ b/gcc/ggc-zone.c
@@ -1073,6 +1073,24 @@ free_chunk (char *ptr, size_t size, struct alloc_zone 
*zone)
 

[PATCH 5/6] Add line map statistics to -fmem-report output

2011-10-17 Thread Dodji Seketeli
This patch adds statistics about line maps' memory consumption and
macro expansion to the output of -fmem-report.  It has been useful in
trying to reduce the memory consumption of the macro maps support.

gcc/ChangeLog
2011-10-15  Tom Tromey  tro...@redhat.com
Dodji Seketeli  do...@redhat.com
 
* input.c (ONE_K, ONE_M, SCALE, STAT_LABEL, FORMAT_AMOUNT): New
macros.
(num_expanded_macros_counter, num_macro_tokens_counter): Declare
new counters.
(dump_line_table_statistics): Define new function.
* input.h (dump_line_table_statistics): Declare new function.
* toplev.c (dump_memory_report): Call dump_line_table_statistics.

libcpp/ChangeLog
2011-10-15  Tom Tromey  tro...@redhat.com
Dodji Seketeli  do...@redhat.com
 
* line-map.h (struct linemap_stats): Declare new struct.
(linemap_get_statistics): Declare ...
* line-map.c (linemap_get_statistics):  ... new function.
* macro.c (num_expanded_macros_counter, num_macro_tokens_counter):
Declare new counters.
(enter_macro_context, replace_args): Update
num_macro_tokens_counter.
(cpp_get_token_1): Update num_expanded_macros_counter.

---
 gcc/ChangeLog |   11 +
 gcc/input.c   |   96 +
 gcc/input.h   |2 +
 gcc/toplev.c  |1 +
 libcpp/ChangeLog  |   12 ++
 libcpp/include/line-map.h |   21 ++
 libcpp/line-map.c |   63 +
 libcpp/macro.c|   29 --
 8 files changed, 231 insertions(+), 4 deletions(-)

diff --git a/gcc/input.c b/gcc/input.c
index 89af274..41842b7 100644
--- a/gcc/input.c
+++ b/gcc/input.c
@@ -46,3 +46,99 @@ expand_location (source_location loc)
 LRK_SPELLING_LOCATION);
   return xloc;
 }
+
+#define ONE_K 1024
+#define ONE_M (ONE_K * ONE_K)
+
+/* Display a number as an integer multiple of either:
+   - 1024, if said integer is = to 10 K (in base 2)
+   - 1024 * 1024, if said integer is = 10 M in (base 2)
+ */
+#define SCALE(x) ((unsigned long) ((x)  10 * ONE_K \
+ ? (x) \
+ : ((x)  10 * ONE_M \
+? (x) / ONE_K \
+: (x) / ONE_M)))
+
+/* For a given integer, display either:
+   - the character 'k', if the number is higher than 10 K (in base 2)
+ but strictly lower than 10 M (in base 2)
+   - the character 'M' if the number is higher than 10 M (in base2)
+   - the charcter ' ' if the number is strictly lower  than 10 K  */
+#define STAT_LABEL(x) ((x)  10 * ONE_K ? ' ' : ((x)  10 * ONE_M ? 'k' : 'M'))
+
+/* Display an integer amount as multiple of 1K or 1M (in base 2).
+   Display the correct unit (either k, M, or ' ') after the amout, as
+   well.  */
+#define FORMAT_AMOUNT(size) SCALE (size), STAT_LABEL (size)
+
+/* Dump statistics to stderr about the memory usage of the line_table
+   set of line maps.  This also displays some statistics about macro
+   expansion.  */
+
+void
+dump_line_table_statistics (void)
+{
+  struct linemap_stats s;
+  size_t total_used_map_size,
+macro_maps_size,
+total_allocated_map_size;
+
+  memset (s, 0, sizeof (s));
+
+  linemap_get_statistics (line_table, s);
+
+  macro_maps_size = s.macro_maps_used_size
++ s.macro_maps_locations_size;
+
+  total_allocated_map_size = s.ordinary_maps_allocated_size
++ s.macro_maps_allocated_size
++ s.macro_maps_locations_size;
+
+  total_used_map_size = s.ordinary_maps_used_size
++ s.macro_maps_used_size
++ s.macro_maps_locations_size;
+
+  fprintf (stderr, Number of expanded macros: %5lu\n,
+   s.num_expanded_macros);
+  if (s.num_expanded_macros != 0)
+fprintf (stderr, Average number of tokens per macro expansion:  %5lu\n,
+ s.num_macro_tokens / s.num_expanded_macros);
+  fprintf (stderr,
+   \nLine Table allocations during the 
+   compilation process\n);
+  fprintf (stderr, Number of ordinary maps used:%5lu%c\n,
+   SCALE (s.num_ordinary_maps_used),
+   STAT_LABEL (s.num_ordinary_maps_used));
+  fprintf (stderr, Ordinary map used size:  %5lu%c\n,
+   SCALE (s.ordinary_maps_used_size),
+   STAT_LABEL (s.ordinary_maps_used_size));
+  fprintf (stderr, Number of ordinary maps allocated:   %5lu%c\n,
+   SCALE (s.num_ordinary_maps_allocated),
+   STAT_LABEL (s.num_ordinary_maps_allocated));
+  fprintf (stderr, Ordinary maps allocated size:%5lu%c\n,
+   SCALE (s.ordinary_maps_allocated_size),
+   STAT_LABEL (s.ordinary_maps_allocated_size));
+  fprintf (stderr, Number of macro maps used:   %5lu%c\n,
+   SCALE (s.num_macro_maps_used),
+   STAT_LABEL (s.num_macro_maps_used));
+  fprintf (stderr, Macro maps used size:%5lu%c\n,
+   SCALE 

Re: [PATCH] fortran/50407 -- Format strings from user-defined operator or kind type string

2011-10-17 Thread Tobias Burnus

On 10/16/2011 09:38 PM, Steve Kargl wrote:

The attach patch fixes the construction of a format string
from a user-defined operator or from a string with a kind
type prefix.


OK. Thanks for the patch.

Tobias


2011-10-16  Steven G. Karglka...@gcc.gnu.org

* io.c (match_dt_format): Match a user-defined operator or a kind
type prefixed string.

2011-10-16  Steven G. Karglka...@gcc.gnu.org

* gfortran.dg/format_string.f: New test.





Re: resent2 [PATCH] Fix ICE in redirect_jump, at jump.c:1497 PR50496

2011-10-17 Thread Bernd Schmidt
On 10/15/11 16:21, Eric Botcazou wrote:
 so the correct fix is very likely something like:
 
 Index: cfgrtl.c
 ===
 --- cfgrtl.c(revision 179844)
 +++ cfgrtl.c(working copy)
 @@ -1024,13 +1024,20 @@ patch_jump_insn (rtx insn, rtx old_label
 
if (!currently_expanding_to_rtl || JUMP_LABEL (insn) == old_label)
 {
 + rtx new_label;
 +
   /* If the insn doesn't go where we think, we're confused.  */
   gcc_assert (JUMP_LABEL (insn) == old_label);
 
 + if (new_bb == EXIT_BLOCK_PTR)
 +   new_label = ret_rtx;
 + else
 +   new_label = block_label (new_bb);
 +
   /* If the substitution doesn't succeed, die.  This can happen
  if the back end emitted unrecognizable instructions or if
  target is exit block on some arches.  */
 - if (!redirect_jump (insn, block_label (new_bb), 0))
 + if (!redirect_jump (insn, new_label, 0))
 {
   gcc_assert (new_bb == EXIT_BLOCK_PTR);
   return false;
 
 
 Bernd, should all the callers of redirect_jump/redirect_jump_1 be audited for 
 this pattern (there are 3 of them in cfgrtl.c for example)?

I think first we'll need to find the caller and make sure it really
wants a return and not a simple_return.


Bernd


Re: [PATCH] fortran/50514 -- Fix static chekcing of ISHFT[C] arguments.

2011-10-17 Thread Tobias Burnus

On 10/15/2011 11:21 PM, Steve Kargl wrote:

OK for trunk?

2011-10-15  Steven G. Karglka...@gcc.gnu.org

* gfortran.dg/ishft_3.f90:  Update test.


I am not so happy with complete test replacements. How about adding it 
as new test case?




2011-10-15  Steven G. Karglka...@gcc.gnu.org

* check.c (less_than_bitsize1): Check|shift|  = bit_size(i).
(gfc_check_ishftc):  Check|shift|  = bit_size(i) and check
that size is positive.


I somehow find less_than_bitsize1's

+  if (strncmp (arg2, ISHFT, 5) == 0)

not that elegant and would prefer another argument, which tells the 
function that it should take the absolute value of the argument; 
however, given that ISHFT seems to be the only function which allows 
negative arguments, one could also leave it.


OK with considering the two remarks. Thanks for the patch!

Tobias

PS: As you have probably seen, I found a related issue regarding 
DSHIFTL/DSHIFTR: PR 50753.



Index: testsuite/gfortran.dg/ishft_3.f90
===
--- testsuite/gfortran.dg/ishft_3.f90   (revision 179208)
+++ testsuite/gfortran.dg/ishft_3.f90   (working copy)
@@ -1,11 +1,38 @@
  ! { dg-do compile }
+! PR fortran/50514
  program ishft_3
-  integer i, j
-  write(*,*) ishftc( 3, 2, 3 )
-  write(*,*) ishftc( 3, 2, i )
-  write(*,*) ishftc( 3, i, j )
-  write(*,*) ishftc( 3, 128 ) ! { dg-error exceeds BIT_SIZE of first }
-  write(*,*) ishftc( 3, 0, 128 )  ! { dg-error exceeds BIT_SIZE of first }
-  write(*,*) ishftc( 3, 0, 0 )! { dg-error Invalid third argument }
-  write(*,*) ishftc( 3, 3, 2 )! { dg-error exceeds third argument }
+
+   implicit none
+
+   integer j, m
+
+   m = 42
+   !
+   ! These should compile.
+   !
+   j = ishft(m, 16)
+   j = ishft(m, -16)
+   j = ishftc(m, 16)
+   j = ishftc(m, -16)
+   !
+   ! These should issue an error.
+   !
+   j = ishft(m, 640)! { dg-error absolute value of SHIFT }
+   j = ishftc(m, 640)   ! { dg-error absolute value of SHIFT }
+   j = ishft(m, -640)   ! { dg-error absolute value of SHIFT }
+   j = ishftc(m, -640)  ! { dg-error absolute value of SHIFT }
+
+   ! abs(SHIFT) must be= SIZE
+
+   j = ishftc(m,  1, 2)
+   j = ishftc(m,  1, 2)
+   j = ishftc(m, -1, 2)
+   j = ishftc(m, -1, 2)
+
+   j = ishftc(m,  10, 2)! { dg-error absolute value of SHIFT }
+   j = ishftc(m,  10, 2)! { dg-error absolute value of SHIFT }
+   j = ishftc(m, -10, 2)! { dg-error absolute value of SHIFT }
+   j = ishftc(m, -10, 2)! { dg-error absolute value of SHIFT }
+
+   j = ishftc(m, 1, -2) ! { dg-error must be positive }
  end program

Index: fortran/check.c
===
--- fortran/check.c (revision 179208)
+++ fortran/check.c (working copy)
@@ -318,6 +318,22 @@ less_than_bitsize1 (const char *arg1, gf
  {
gfc_extract_int (expr2,i2);
i3 = gfc_validate_kind (BT_INTEGER, expr1-ts.kind, false);
+
+  /* For ISHFT[C],|shift|  = bit_size(i).  */
+  if (strncmp (arg2, ISHFT, 5) == 0)
+   {
+ if (i2  0)
+   i2 = -i2;
+
+ if (i2  gfc_integer_kinds[i3].bit_size)
+   {
+ gfc_error (The absolute value of SHIFT at %L must be less 
+than or equal to BIT_SIZE('%s'),
+   expr2-where, arg1);
+ return FAILURE;
+   }
+   }
+
if (or_equal)
{
  if (i2  gfc_integer_kinds[i3].bit_size)
@@ -1961,6 +1977,9 @@ gfc_check_ishft (gfc_expr *i, gfc_expr *
|| type_check (shift, 1, BT_INTEGER) == FAILURE)
  return FAILURE;

+  if (less_than_bitsize1 (I, i, ISHFT, shift, true) == FAILURE)
+return FAILURE;
+
return SUCCESS;
  }

@@ -1972,7 +1991,35 @@ gfc_check_ishftc (gfc_expr *i, gfc_expr
|| type_check (shift, 1, BT_INTEGER) == FAILURE)
  return FAILURE;

-  if (size != NULL  type_check (size, 2, BT_INTEGER) == FAILURE)
+  if (size != NULL)
+{
+  int i2, i3;
+
+  if (type_check (size, 2, BT_INTEGER) == FAILURE)
+   return FAILURE;
+
+  if (less_than_bitsize1 (I, i, SIZE, size, true) == FAILURE)
+   return FAILURE;
+
+  gfc_extract_int (size,i3);
+  if (i3= 0)
+   {
+ gfc_error (SIZE at %L must be positive,size-where);
+ return FAILURE;
+   }
+
+  gfc_extract_int (shift,i2);
+  if (i2  0)
+   i2 = -i2;
+
+  if (i2  i3)
+   {
+ gfc_error (The absolute value of SHIFT at %L must be less than 
+or equal to SIZE at %L,shift-where,size-where);
+ return FAILURE;
+   }
+}
+  else if (less_than_bitsize1 (I, i, ISHFTC, shift, true) == FAILURE)
  return FAILURE;

return SUCCESS;


Re: [C++ Patch] PR 32614

2011-10-17 Thread Richard Guenther
On Mon, Oct 17, 2011 at 11:42 AM, Paolo Carlini
paolo.carl...@oracle.com wrote:
 FWIW, I still believe that tweaking the documentation to match the long
 standing reality, would be a small improvement. I'm not going to further
 insist, anyway.

The original patch is ok.

Thanks,
Richard.

 Paolo.



Re: [C++ Patch] PR 32614

2011-10-17 Thread Gabriel Dos Reis
On Mon, Oct 17, 2011 at 4:42 AM, Paolo Carlini paolo.carl...@oracle.com wrote:
 FWIW, I still believe that tweaking the documentation to match the long
 standing reality, would be a small improvement. I'm not going to further
 insist, anyway.

It isn't improvement.
The improvement would be to restore the documented default.


Re: [C++ Patch] PR 32614

2011-10-17 Thread Gabriel Dos Reis
On Mon, Oct 17, 2011 at 5:25 AM, Richard Guenther
richard.guent...@gmail.com wrote:
 On Mon, Oct 17, 2011 at 11:42 AM, Paolo Carlini
 paolo.carl...@oracle.com wrote:
 FWIW, I still believe that tweaking the documentation to match the long
 standing reality, would be a small improvement. I'm not going to further
 insist, anyway.

 The original patch is ok.
Richard, I don't think it is.


Re: [C++ Patch] PR 32614

2011-10-17 Thread Richard Guenther
On Mon, Oct 17, 2011 at 12:26 PM, Gabriel Dos Reis
g...@integrable-solutions.net wrote:
 On Mon, Oct 17, 2011 at 4:42 AM, Paolo Carlini paolo.carl...@oracle.com 
 wrote:
 FWIW, I still believe that tweaking the documentation to match the long
 standing reality, would be a small improvement. I'm not going to further
 insist, anyway.

 It isn't improvement.
 The improvement would be to restore the documented default.

I disagree.  Well, both would be an improvement.  Restoring the documented
default would be a move in the wrong direction though.  Why have different
defaults for different languages anyway?  How long has the new default
be in effect?

Richard.


Re: [C++ Patch] PR 32614

2011-10-17 Thread Gabriel Dos Reis
On Mon, Oct 17, 2011 at 5:28 AM, Richard Guenther
richard.guent...@gmail.com wrote:
 On Mon, Oct 17, 2011 at 12:26 PM, Gabriel Dos Reis
 g...@integrable-solutions.net wrote:
 On Mon, Oct 17, 2011 at 4:42 AM, Paolo Carlini paolo.carl...@oracle.com 
 wrote:
 FWIW, I still believe that tweaking the documentation to match the long
 standing reality, would be a small improvement. I'm not going to further
 insist, anyway.

 It isn't improvement.
 The improvement would be to restore the documented default.

 I disagree.  Well, both would be an improvement.  Restoring the documented
 default would be a move in the wrong direction though.  Why have different
 defaults for different languages anyway?

That is a question for the other front-ends which are not obliged to pick
just about anything implemented for g++.

  How long has the new default be in effect?

 Richard.



Re: [patch tree-optimization]: Improve handling of conditional-branches on targets with high branch costs

2011-10-17 Thread Richard Guenther
On Fri, Oct 14, 2011 at 9:43 PM, Kai Tietz ktiet...@googlemail.com wrote:
 Hello,

 So I committed the gimplify patch separate.  And here is the remaining
 fold-const patch.
 The important tests here are in gcc.dg/tree-ssa/builtin-expect[1-4].c, which
 cover the one special-case for branching. Also tree-ssa/20040204-1.c covers
 tests for branching code (on targets having high-engough BRANCH_COST and no
 special-casing - like MIPS, S/390, and AVR.

 ChangeLog

 2011-10-14  Kai Tietz  kti...@redhat.com

        * fold-const.c (simple_operand_p_2): New function.
        (fold_truthop): Rename to
        (fold_truth_andor_1): function name.
        Additionally remove branching creation for logical and/or.
        (fold_truth_andor): Handle branching creation for logical and/or here.

 Bootstrapped and regression-tested for all languages plus Ada and
 Obj-C++ on x86_64-pc-linux-gnu.
 Ok for apply?

Ok with ...

 Regards,
 Kai

 Index: gcc/gcc/fold-const.c
 ===
 --- gcc.orig/gcc/fold-const.c
 +++ gcc/gcc/fold-const.c
 @@ -112,13 +112,13 @@ static tree decode_field_reference (loca
  static int all_ones_mask_p (const_tree, int);
  static tree sign_bit_p (tree, const_tree);
  static int simple_operand_p (const_tree);
 +static bool simple_operand_p_2 (tree);
  static tree range_binop (enum tree_code, tree, tree, int, tree, int);
  static tree range_predecessor (tree);
  static tree range_successor (tree);
  static tree fold_range_test (location_t, enum tree_code, tree, tree, tree);
  static tree fold_cond_expr_with_comparison (location_t, tree, tree,
 tree, tree);
  static tree unextend (tree, int, int, tree);
 -static tree fold_truthop (location_t, enum tree_code, tree, tree, tree);
  static tree optimize_minmax_comparison (location_t, enum tree_code,
                                        tree, tree, tree);
  static tree extract_muldiv (tree, tree, enum tree_code, tree, bool *);
 @@ -3500,7 +3500,7 @@ optimize_bit_field_compare (location_t l
   return lhs;
  }

 -/* Subroutine for fold_truthop: decode a field reference.
 +/* Subroutine for fold_truth_andor_1: decode a field reference.

    If EXP is a comparison reference, we return the innermost reference.

 @@ -3668,7 +3668,7 @@ sign_bit_p (tree exp, const_tree val)
   return NULL_TREE;
  }

 -/* Subroutine for fold_truthop: determine if an operand is simple enough
 +/* Subroutine for fold_truth_andor_1: determine if an operand is simple 
 enough
    to be evaluated unconditionally.  */

  static int
 @@ -3678,7 +3678,7 @@ simple_operand_p (const_tree exp)
   STRIP_NOPS (exp);

   return (CONSTANT_CLASS_P (exp)
 -         || TREE_CODE (exp) == SSA_NAME
 +         || TREE_CODE (exp) == SSA_NAME
          || (DECL_P (exp)
               ! TREE_ADDRESSABLE (exp)
               ! TREE_THIS_VOLATILE (exp)
 @@ -3692,6 +3692,46 @@ simple_operand_p (const_tree exp)
                 registers aren't expensive.  */
               (! TREE_STATIC (exp) || DECL_REGISTER (exp;
  }
 +
 +/* Subroutine for fold_truth_andor: determine if an operand is simple enough
 +   to be evaluated unconditionally.
 +   I addition to simple_operand_p, we assume that comparisons and logic-not
 +   operations are simple, if their operands are simple, too.  */
 +
 +static bool
 +simple_operand_p_2 (tree exp)
 +{
 +  enum tree_code code;
 +
 +  /* Strip any conversions that don't change the machine mode.  */
 +  STRIP_NOPS (exp);
 +
 +  code = TREE_CODE (exp);
 +
 +  if (TREE_CODE_CLASS (code) == tcc_comparison)
 +    return (!tree_could_trap_p (exp)
 +            simple_operand_p_2 (TREE_OPERAND (exp, 0))
 +            simple_operand_p_2 (TREE_OPERAND (exp, 1)));

recurse with simple_operand_p.

 +
 +  if (TREE_SIDE_EFFECTS (exp)
 +      || tree_could_trap_p (exp))

Move this check before the tcc_comparison check and remove the
then redundant tree_could_trap_p check there.

 +    return false;
 +
 +  switch (code)
 +    {
 +    case SSA_NAME:
 +      return true;

Do not handle here, it's handled in simple_operand_p.

 +    case TRUTH_NOT_EXPR:
 +      return simple_operand_p_2 (TREE_OPERAND (exp, 0));
 +    case BIT_NOT_EXPR:
 +      if (TREE_CODE (TREE_TYPE (exp)) != BOOLEAN_TYPE)
 +       return false;

Remove the BIT_NOT_EXPR handling.  Thus, simply change this switch
to

if (code == TRUTH_NOT_EXPR)
  return simple_operand_p_2 (TREE_OPERAND (exp, 0));

return simple_operand_p (exp);

 +      return simple_operand_p_2 (TREE_OPERAND (exp, 0));
 +    default:
 +      return simple_operand_p (exp);
 +    }
 +}
 +

  /* The following functions are subroutines to fold_range_test and allow it to
    try to change a logical combination of comparisons into a range test.
 @@ -4888,7 +4928,7 @@ fold_range_test (location_t loc, enum tr
   return 0;
  }

 -/* Subroutine for fold_truthop: C is an INTEGER_CST interpreted as a P
 +/* Subroutine for fold_truth_andor_1: C is an INTEGER_CST interpreted as a P
    bit value.  Arrange things 

Re: [Patch, Fortran] PR 47023: [4.6/4.7 regression] C_Sizeof: Rejects valid code

2011-10-17 Thread Janus Weil
 Of course you can commit to 4.6.

Ok. Btw, my patch had a small regression (PR50752), which was very
quickly reported by Joost. I have just committed the obvious fix as
r180079.


 Be quick, though; 4.6.2 was due for release now-ish -

I know. I'll try to do it today.

Cheers,
Janus



 On Sun, Oct 16, 2011 at 9:45 PM, Janus Weil ja...@gcc.gnu.org wrote:
 Hi Paul,

 This is OK for trunk.  Thanks fo rthe patch.

 Thanks. Committed to trunk as r180062. What about 4.6?

 Cheers,
 Janus



 On Sun, Oct 16, 2011 at 2:58 PM, Janus Weil ja...@gcc.gnu.org wrote:
 Hi all,

 here is a patch which fixes the regression in comment #2 of the PR in
 the subject line. What it does is setting the 'ts.is_c_interop' flag
 correctly for constants with kind-parameter specification (such as
 '0.0_c_double'), as is already being done for variables.

 Regtested on x86_64-unknown-linux-gnu. Ok for trunk and 4.6?

 Cheers,
 Janus


 2011-10-16  Janus Weil  ja...@gcc.gnu.org

        PR fortran/47023
        * primary.c (match_kind_param): Detect ISO_C_BINDING kinds.
        (get_kind): Pass on 'is_iso_c' flag.
        (match_integer_constant,match_real_constant,match_logical_constant):
        Set 'ts.is_c_interop'.


 2011-10-16  Janus Weil  ja...@gcc.gnu.org

        PR fortran/47023
        * gfortran.dg/c_kind_tests_3.f03: New.




 --
 The knack of flying is learning how to throw yourself at the ground and 
 miss.
        --Hitchhikers Guide to the Galaxy





 --
 The knack of flying is learning how to throw yourself at the ground and miss.
        --Hitchhikers Guide to the Galaxy



Re: [C++ Patch] PR 32614

2011-10-17 Thread Paolo Carlini

On 10/17/2011 12:26 PM, Gabriel Dos Reis wrote:

On Mon, Oct 17, 2011 at 4:42 AM, Paolo Carlinipaolo.carl...@oracle.com  wrote:

FWIW, I still believe that tweaking the documentation to match the long
standing reality, would be a small improvement. I'm not going to further
insist, anyway.

It isn't improvement.
The improvement would be to restore the documented default.
Well either my English is even weaker than I thought, or restoring 
doesn't apply here: the line of code at  issue, 
pp_set_line_maximum_length (pp, 0), has been added by Gaby in Rev 70777, 
and nothing similar with 72 as second argument existed before.


Paolo.


Re: [PATCH 3/6] Emit macro expansion related diagnostics

2011-10-17 Thread Richard Guenther
On Mon, Oct 17, 2011 at 11:57 AM, Dodji Seketeli do...@redhat.com wrote:
 In this third instalment the diagnostic machinery -- when faced with
 the virtual location of a token resulting from macro expansion -- uses
 the new linemap APIs to unwind the stack of macro expansions that led
 to that token and emits a [hopefully] more useful message than what we
 have today.

 diagnostic_report_current_module has been slightly changed to use the
 location given by client code instead of the global input_location
 variable.  This results in more precise diagnostic locations in
 general but then the patch adjusts some C++ tests which output changed
 as a result of this.

 Three new regression tests have been added.

 The mandatory screenshot goes like this:

 [dodji@adjoa gcc]$ cat -n test.c
     1    #define OPERATE(OPRD1, OPRT, OPRD2) \
     2      OPRD1 OPRT OPRD2;
     3
     4    #define SHIFTL(A,B) \
     5      OPERATE (A,,B)
     6
     7    #define MULT(A) \
     8      SHIFTL (A,1)
     9
    10    void
    11    g ()
    12    {
    13      MULT (1.0);/* 1.0  1; -- so this is an error.  */
    14    }

 [dodji@adjoa gcc]$ ./cc1 -quiet -ftrack-macro-expansion test.c
 test.c: In function 'g':
 test.c:5:14: erreur: invalid operands to binary  (have 'double' and 'int')
 test.c:2:9: note: in expansion of macro 'OPERATE'
 test.c:5:3: note: expanded from here
 test.c:5:14: note: in expansion of macro 'SHIFTL'
 test.c:8:3: note: expanded from here
 test.c:8:3: note: in expansion of macro 'MULT2'
 test.c:13:3: note: expanded from here

This broke bootstrap on x86_64-linux.

/space/rguenther/src/svn/trunk/libcpp/line-map.c: In function
'source_location linemap_macro_map_loc_to_exp_point(const line_map*,
source_location)':
/space/rguenther/src/svn/trunk/libcpp/line-map.c:628:12: error:
variable 'token_no' set but not used [-Werror=unused-but-set-variable]
cc1plus: all warnings being treated as errors

make[3]: *** [line-map.o] Error 1


 gcc/ChangeLog
 2011-10-15  Tom Tromey  tro...@redhat.com
            Dodji Seketeli  do...@redhat.com

        * gcc/diagnostic.h (diagnostic_report_current_module): Add a
        location parameter.
        * diagnostic.c (diagnostic_report_current_module): Add a location
        parameter to the function definition.  Use it instead of
        input_location.  Resolve the virtual location rather than just
        looking up its map and risking to touch a resulting macro map.
        (default_diagnostic_starter): Pass the relevant diagnostic
        location to diagnostic_report_current_module.
        * tree-diagnostic.c (maybe_unwind_expanded_macro_loc): New.
        (virt_loc_aware_diagnostic_finalizer): Likewise.
        (diagnostic_report_current_function): Pass the
        relevant location to diagnostic_report_current_module.
        * tree-diagnostic.h (virt_loc_aware_diagnostic_finalizer): Declare
        new function.
        * toplev.c (general_init): By default, use the new
        virt_loc_aware_diagnostic_finalizer as diagnostic finalizer.
        * Makefile.in: Add vec.h dependency to tree-diagnostic.c.

 gcc/cp/ChangeLog
 2011-10-15  Tom Tromey  tro...@redhat.com
            Dodji Seketeli  do...@redhat.com

        * error.c (cp_diagnostic_starter): Pass the relevant location to
        diagnostic_report_current_module.
        (cp_diagnostic_finalizer): Call virt_loc_aware_diagnostic_finalizer.

 gcc/testsuite/ChangeLog
 2011-10-15  Tom Tromey  tro...@redhat.com
            Dodji Seketeli  do...@redhat.com

        * lib/prune.exp (prune_gcc_output):  Prune output referring to
        included files.
        * gcc.dg/cpp/macro-exp-tracking-1.c: New test.
        * gcc.dg/cpp/macro-exp-tracking-2.c: Likewise.
        * gcc.dg/cpp/macro-exp-tracking-3.c: Likewise.
        * gcc.dg/cpp/pragma-diagnostic-2.c: Likewise.

 ---
  gcc/ChangeLog                                   |   21 +++
  gcc/Makefile.in                                 |    3 +-
  gcc/cp/ChangeLog                                |    7 +
  gcc/cp/error.c                                  |    5 +-
  gcc/diagnostic.c                                |   13 +-
  gcc/diagnostic.h                                |    2 +-
  gcc/testsuite/ChangeLog                         |   10 ++
  gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-1.c |   21 +++
  gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-2.c |   21 +++
  gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-3.c |   14 ++
  gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-4.c |   14 ++
  gcc/testsuite/gcc.dg/cpp/pragma-diagnostic-2.c  |   34 +
  gcc/testsuite/lib/prune.exp                     |    1 +
  gcc/toplev.c                                    |    3 +
  gcc/tree-diagnostic.c                           |  182 
 ++-
  gcc/tree-diagnostic.h                           |    3 +-
  16 files changed, 343 insertions(+), 11 deletions(-)
  create mode 100644 gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-1.c
  create mode 100644 

Re: [PATCH] Fix pr50717: widening multiply bad code

2011-10-17 Thread Richard Guenther
On Sat, Oct 15, 2011 at 11:27 PM, Andrew Stubbs a...@codesourcery.com wrote:
 This patch fixes a bug in which the widening multiply-and-accumulate
 optimization failed to take the intermediate types into account.

 The effect of this is that the compiler would do what the programmer
 expected to happen, rather than what the C standard requires to happen (in
 many cases), so obviously this needed fixing. :)

 It still needs to optimize the cases where the optimization doesn't actually
 change anything (because it's known that overflow cannot occur), so I don't
 want to completely disallow extends between multiply and plus, and I believe
 this patch achieves this.

 OK?

Ok.

THanks,
Richard.

 Andrew



Re: resent2 [PATCH] Fix ICE in redirect_jump, at jump.c:1497 PR50496

2011-10-17 Thread Bernd Schmidt
On 10/15/11 16:21, Eric Botcazou wrote:
  PR middle-end/50496
  * cfgrtl.c (try_redirect_by_replacing_jump): Treat EXIT_BLOCK_PTR case
  separately before call to redirect_jump(). Add assertion.
  (patch_jump_insn): Same.
 
 This will definitely disable redirections to the exit block.

Yes. However, in the case that caused the PR, this was attempted with
reload_completed == 0, so we cannot generate return patterns anyway and
must fail.

I think the original patch is OK.


Bernd


Re: [PR50672, PATCH] Fix ice triggered by -ftree-tail-merge: verify_ssa failed: no immediate_use list

2011-10-17 Thread Richard Guenther
On Sun, Oct 16, 2011 at 12:05 PM, Tom de Vries tom_devr...@mentor.com wrote:
 On 10/14/2011 12:00 PM, Richard Guenther wrote:
 On Fri, Oct 14, 2011 at 1:12 AM, Tom de Vries tom_devr...@mentor.com wrote:
 On 10/12/2011 02:19 PM, Richard Guenther wrote:
 On Wed, Oct 12, 2011 at 8:35 AM, Tom de Vries vr...@codesourcery.com 
 wrote:
 Richard,

 I have a patch for PR50672.

 When compiling the testcase from the PR with -ftree-tail-merge, the 
 scenario is
 as follows:

 We start out tail_merge_optimize with blocks 14 and 20, which are alike, 
 but not
 equal, since they have different successors:
 ...
  # BLOCK 14 freq:690
  # PRED: 25 [61.0%]  (false,exec)

  if (wD.2197_57(D) != 0B)
    goto bb 15;
  else
    goto bb 16;
  # SUCC: 15 [78.4%]  (true,exec) 16 [21.6%]  (false,exec)


  # BLOCK 20 freq:2900
  # PRED: 29 [100.0%]  (fallthru) 31 [100.0%]  (fallthru)

  # .MEMD.2447_209 = PHI .MEMD.2447_125(29), .MEMD.2447_129(31)
  if (wD.2197_57(D) != 0B)
    goto bb 5;
  else
    goto bb 6;
  # SUCC: 5 [85.0%]  (true,exec) 6 [15.0%]  (false,exec)
 ...

 In the first iteration, we merge block 5 with block 15 and block 6 with 
 block
 16. After that, the blocks 14 and 20 are equal.

 In the second iteration, the blocks 14 and 20 are merged, by redirecting 
 the
 incoming edges of block 20 to block 14, and removing block 20.

 Block 20 also contains the definition of .MEMD.2447_209. Removing the 
 definition
 delinks the vuse of .MEMD.2447_209 in block 5:
 ...
  # BLOCK 5 freq:6036
  # PRED: 20 [85.0%]  (true,exec)

  # PT = nonlocal escaped
  D.2306_58 = thisD.2200_10(D)-D.2156;
  # .MEMD.2447_132 = VDEF .MEMD.2447_209
  # USE = anything
  # CLB = anything
  drawLineD.2135 (D.2306_58, wD.2197_57(D), gcD.2198_59(D));
  goto bb 17;
  # SUCC: 17 [100.0%]  (fallthru,exec)
 ...

 And block 5 is retained and block 15 is discarded?


 Indeed.

 After the pass, when executing the TODO_update_ssa_only_virtuals, we 
 update the
 drawLine call in block 5 using rewrite_update_stmt, which calls
 maybe_replace_use for the vuse operand.

 However, maybe_replace_use doesn't have an effect since the old vuse and 
 the new
 vuse happen to be the same (rdef == use), so SET_USE is not called and 
 the vuse
 remains delinked:
 ...
  if (rdef  rdef != use)
    SET_USE (use_p, rdef);
 ...

 The patch fixes this by forcing SET_USE for delinked uses.

 That isn't the correct fix.  Whoever unlinks the vuse (by removing its
 definition) has to replace it with something valid, which is either the
 bare symbol .MEM, or the VUSE associated with the removed VDEF
 (thus, as unlink_stmt_vdef does).


 Another try. For each deleted bb, we call unlink_stmt_vdef for the 
 statements,
 and replace the .MEM phi uses with the bare .MEM symbol.

 Bootstrapped and reg-tested on x86_64.

 Ok for trunk?

 Better.  For

 +
 +      FOR_EACH_IMM_USE_STMT (use_stmt, iter, res)
 +       {
 +         FOR_EACH_IMM_USE_ON_STMT (use_p, iter)
 +           SET_USE (use_p, SSA_NAME_VAR (res));
 +       }

 you can use mark_virtual_phi_result_for_renaming (phi) instead.

 +  for (i = gsi_last_bb (bb); !gsi_end_p (i); gsi_prev_nondebug (i))
 +    unlink_stmt_vdef (gsi_stmt (i));

 is that actually necessary?  That is, isn't the block that follows a
 deleted block always starting with a virtual PHI?

 Block 20 is deleted. Block 5 follows the deleted block 20. Block 5 does not
 start with a virtual PHI.

 If not it should
 be enough to walk to the first stmt that uses a virtual operand
 and similar to the PHI case replace all its uses with the bare
 symbol.

 I think we need to handle the exposed uses (meaning outside the block) of 
 vdefs
 in each deleted block. The only vdef that can have exposed uses is the last 
 vdef
 in the block. So we should find the last vdef (gimple with gimple_vdef or
 virtual PHI) and handle the related uses.

 Bootstrapped and regtested on x86_64. OK for trunk?

Hmmm.  I can see that this will fail when the block has a store
but not a virtual PHI node, thus, when renaming does not reach
a bare .MEM symbol walking the use-def chain from the VUSE
of the store.  What release_last_vdef should do is trigger a rename
of subsequent VUSEs in successors of the deleted block.  Which
requires you to do something like

static void
rename_last_vdef (basic_block bb)
{
+  gimple_stmt_iterator i;
+
+  for (i = gsi_last_bb (bb); !gsi_end_p (i); gsi_prev_nondebug (i))
+{
+  gimple stmt = gsi_stmt (i);
+  if (gimple_vdef (stmt) == NULL_TREE)
+   continue;
+
+  mark_virtual_operand_for_renaming (gimple_vdef (stmt));
return;
  }

+  for (i = gsi_start_phis (bb); !gsi_end_p (i); gsi_next (i))
+{
+  gimple phi = gsi_stmt (i);
+  tree res = gimple_phi_result (phi);
+
+  if (is_gimple_reg (res))
+   continue;
+
+  mark_virtual_phi_result_for_renaming (phi);
+  return;
+}
}

And split out the

  result_var = SSA_NAME_VAR (result_ssa);
  FOR_EACH_IMM_USE_STMT (stmt, iter, result_ssa)
{
  

Re: [C++ Patch] PR 32614

2011-10-17 Thread Richard Guenther
On Mon, 17 Oct 2011, Paolo Carlini wrote:

 On 10/17/2011 12:26 PM, Gabriel Dos Reis wrote:
  On Mon, Oct 17, 2011 at 4:42 AM, Paolo Carlinipaolo.carl...@oracle.com
  wrote:
   FWIW, I still believe that tweaking the documentation to match the long
   standing reality, would be a small improvement. I'm not going to further
   insist, anyway.
  It isn't improvement.
  The improvement would be to restore the documented default.
 Well either my English is even weaker than I thought, or restoring doesn't
 apply here: the line of code at  issue, pp_set_line_maximum_length (pp, 0),
 has been added by Gaby in Rev 70777, and nothing similar with 72 as second
 argument existed before.

Thus clearly the documentation is wrong ;)  So, I'll approve the patch
for the release branches (where we don't want to change the default).
We can bikeshed a bit more for trunk.

Richard.


Re: [C++ Patch] PR 32614

2011-10-17 Thread Gabriel Dos Reis
On Mon, Oct 17, 2011 at 5:53 AM, Richard Guenther rguent...@suse.de wrote:
 On Mon, 17 Oct 2011, Paolo Carlini wrote:

 On 10/17/2011 12:26 PM, Gabriel Dos Reis wrote:
  On Mon, Oct 17, 2011 at 4:42 AM, Paolo Carlinipaolo.carl...@oracle.com
  wrote:
   FWIW, I still believe that tweaking the documentation to match the long
   standing reality, would be a small improvement. I'm not going to further
   insist, anyway.
  It isn't improvement.
  The improvement would be to restore the documented default.
 Well either my English is even weaker than I thought, or restoring doesn't
 apply here: the line of code at  issue, pp_set_line_maximum_length (pp, 0),
 has been added by Gaby in Rev 70777, and nothing similar with 72 as second
 argument existed before.

 Thus clearly the documentation is wrong ;)

;-)
Not necessarily.  Paolo does not say why that line was added.
I don't remember adding that line to change the default.


 So, I'll approve the patch
 for the release branches (where we don't want to change the default).
 We can bikeshed a bit more for trunk.

 Richard.



Re: [patch tree-optimization]: Improve handling of conditional-branches on targets with high branch costs

2011-10-17 Thread Kai Tietz
2011/10/17 Richard Guenther richard.guent...@gmail.com:
 On Fri, Oct 14, 2011 at 9:43 PM, Kai Tietz ktiet...@googlemail.com wrote:
 Hello,

 So I committed the gimplify patch separate.  And here is the remaining
 fold-const patch.
 The important tests here are in gcc.dg/tree-ssa/builtin-expect[1-4].c, which
 cover the one special-case for branching. Also tree-ssa/20040204-1.c covers
 tests for branching code (on targets having high-engough BRANCH_COST and no
 special-casing - like MIPS, S/390, and AVR.

 ChangeLog

 2011-10-14  Kai Tietz  kti...@redhat.com

        * fold-const.c (simple_operand_p_2): New function.
        (fold_truthop): Rename to
        (fold_truth_andor_1): function name.
        Additionally remove branching creation for logical and/or.
        (fold_truth_andor): Handle branching creation for logical and/or here.

 Bootstrapped and regression-tested for all languages plus Ada and
 Obj-C++ on x86_64-pc-linux-gnu.
 Ok for apply?

 Ok with ...

 Regards,
 Kai

 Index: gcc/gcc/fold-const.c
 ===
 --- gcc.orig/gcc/fold-const.c
 +++ gcc/gcc/fold-const.c
 @@ -112,13 +112,13 @@ static tree decode_field_reference (loca
  static int all_ones_mask_p (const_tree, int);
  static tree sign_bit_p (tree, const_tree);
  static int simple_operand_p (const_tree);
 +static bool simple_operand_p_2 (tree);
  static tree range_binop (enum tree_code, tree, tree, int, tree, int);
  static tree range_predecessor (tree);
  static tree range_successor (tree);
  static tree fold_range_test (location_t, enum tree_code, tree, tree, tree);
  static tree fold_cond_expr_with_comparison (location_t, tree, tree,
 tree, tree);
  static tree unextend (tree, int, int, tree);
 -static tree fold_truthop (location_t, enum tree_code, tree, tree, tree);
  static tree optimize_minmax_comparison (location_t, enum tree_code,
                                        tree, tree, tree);
  static tree extract_muldiv (tree, tree, enum tree_code, tree, bool *);
 @@ -3500,7 +3500,7 @@ optimize_bit_field_compare (location_t l
   return lhs;
  }

 -/* Subroutine for fold_truthop: decode a field reference.
 +/* Subroutine for fold_truth_andor_1: decode a field reference.

    If EXP is a comparison reference, we return the innermost reference.

 @@ -3668,7 +3668,7 @@ sign_bit_p (tree exp, const_tree val)
   return NULL_TREE;
  }

 -/* Subroutine for fold_truthop: determine if an operand is simple enough
 +/* Subroutine for fold_truth_andor_1: determine if an operand is simple 
 enough
    to be evaluated unconditionally.  */

  static int
 @@ -3678,7 +3678,7 @@ simple_operand_p (const_tree exp)
   STRIP_NOPS (exp);

   return (CONSTANT_CLASS_P (exp)
 -         || TREE_CODE (exp) == SSA_NAME
 +         || TREE_CODE (exp) == SSA_NAME
          || (DECL_P (exp)
               ! TREE_ADDRESSABLE (exp)
               ! TREE_THIS_VOLATILE (exp)
 @@ -3692,6 +3692,46 @@ simple_operand_p (const_tree exp)
                 registers aren't expensive.  */
               (! TREE_STATIC (exp) || DECL_REGISTER (exp;
  }
 +
 +/* Subroutine for fold_truth_andor: determine if an operand is simple enough
 +   to be evaluated unconditionally.
 +   I addition to simple_operand_p, we assume that comparisons and logic-not
 +   operations are simple, if their operands are simple, too.  */
 +
 +static bool
 +simple_operand_p_2 (tree exp)
 +{
 +  enum tree_code code;
 +
 +  /* Strip any conversions that don't change the machine mode.  */
 +  STRIP_NOPS (exp);
 +
 +  code = TREE_CODE (exp);
 +
 +  if (TREE_CODE_CLASS (code) == tcc_comparison)
 +    return (!tree_could_trap_p (exp)
 +            simple_operand_p_2 (TREE_OPERAND (exp, 0))
 +            simple_operand_p_2 (TREE_OPERAND (exp, 1)));

 recurse with simple_operand_p.

No, as this again would reject simple operations and additionally
wouldn't check for trapping.

 +
 +  if (TREE_SIDE_EFFECTS (exp)
 +      || tree_could_trap_p (exp))

 Move this check before the tcc_comparison check and remove the
 then redundant tree_could_trap_p check there.

Ok

 +    return false;
 +
 +  switch (code)
 +    {
 +    case SSA_NAME:
 +      return true;

 Do not handle here, it's handled in simple_operand_p.

Well, was more a short-cut here.

 +    case TRUTH_NOT_EXPR:
 +      return simple_operand_p_2 (TREE_OPERAND (exp, 0));
 +    case BIT_NOT_EXPR:
 +      if (TREE_CODE (TREE_TYPE (exp)) != BOOLEAN_TYPE)
 +       return false;

 Remove the BIT_NOT_EXPR handling.  Thus, simply change this switch
 to

Why should we reject simple ~X operations from gimplified code here?
I admit that from FE-code we won't see that, as always an integer-cast
is done for foo (_Bool x) { ... if (~x) ... }, but from
gimplified-code this is the general description of an boolean-typed !=
0?

 if (code == TRUTH_NOT_EXPR)
  return simple_operand_p_2 (TREE_OPERAND (exp, 0));

 return simple_operand_p (exp);

 +      return simple_operand_p_2 (TREE_OPERAND (exp, 0));
 +    default:
 +  

Re: [patch] C6X unwinding/exception handling

2011-10-17 Thread Matthias Klose
On 10/10/2011 02:23 PM, Paul Brook wrote:
 Index: libjava/exception.cc
 ===
 --- libjava/exception.cc (revision 179739)
 +++ libjava/exception.cc (working copy)
 @@ -135,6 +135,7 @@
  {
_Unwind_Ptr Start;
_Unwind_Ptr LPStart;
 +  _Unwind_Ptr ttype_base;
const unsigned char *TType;
const unsigned char *action_table;
unsigned char ttype_encoding;
 @@ -184,7 +185,7 @@
_Unwind_Ptr ptr;

ptr = (_Unwind_Ptr) (info-TType - (i * 4));
 -  ptr = _Unwind_decode_target2(ptr);
 +  ptr = _Unwind_decode_typeinfo_ptr (info-ttype_base, (_Unwind_Word)
 ptr);

return reinterpret_castvoid **(ptr);
  }
 @@ -325,6 +326,7 @@

// Parse the LSDA header.
p = parse_lsda_header (context, language_specific_data, info);
 +  info.ttype_base = base_of_encoded_value (info.ttype_encoding, context);
  #ifdef HAVE_GETIPINFO
ip = _Unwind_GetIPInfo (context, ip_before_insn);
  #else
 
 No.  The purpose of my patch was to remove the arm specific code.
 
 The only difference I can see in this bit of code is that libstdc++ uses 
 base_of_encoded_value/read_encoded_value_with_base whereas libjava uses 
 context/read_encoded_value.
 
 I expect you want something like the patch below (completely untested).

I checked the attached patch, test results at
http://gcc.gnu.org/ml/gcc-testresults/2011-10/msg01377.html

which are the same as with my suggested patch.

Ok for the trunk?

 Matthias
libjava/
2011-10-17  Paul Brook  p...@codesourcery.com

* exception.cc (parse_lsda_header): hardcode ttype_encoding for older
ARM EABI toolchains.
(get_ttype_entry) Remove __ARM_EABI_UNWINDER__ variant.

libobjc/
2011-10-17  Paul Brook  p...@codesourcery.com
Matthias Klose  d...@ubuntu.com

* exception.c (parse_lsda_header): hardcode ttype_encoding for older
ARM EABI toolchains.
(get_ttype_entry) Remove __ARM_EABI_UNWINDER__ variant.

Index: libjava/exception.cc
===
--- libjava/exception.cc(revision 180086)
+++ libjava/exception.cc(working copy)
@@ -161,6 +161,11 @@
   info-ttype_encoding = *p++;
   if (info-ttype_encoding != DW_EH_PE_omit)
 {
+#if _GLIBCXX_OVERRIDE_TTYPE_ENCODING
+  /* Older ARM EABI toolchains set this value incorrectly, so use a
+hardcoded OS-specific format.  */
+  info-ttype_encoding = _GLIBCXX_OVERRIDE_TTYPE_ENCODING;
+#endif
   p = read_uleb128 (p, tmp);
   info-TType = p + tmp;
 }
@@ -176,22 +181,7 @@
   return p;
 }
 
-#ifdef __ARM_EABI_UNWINDER__
-
 static void **
-get_ttype_entry(_Unwind_Context *, lsda_header_info* info, _uleb128_t i)
-{
-  _Unwind_Ptr ptr;
-
-  ptr = (_Unwind_Ptr) (info-TType - (i * 4));
-  ptr = _Unwind_decode_target2(ptr);
-  
-  return reinterpret_castvoid **(ptr);
-}
-
-#else
-
-static void **
 get_ttype_entry (_Unwind_Context *context, lsda_header_info *info, long i)
 {
   _Unwind_Ptr ptr;
@@ -202,8 +192,6 @@
   return reinterpret_castvoid **(ptr);
 }
 
-#endif
-
 // Using a different personality function name causes link failures
 // when trying to mix code using different exception handling models.
 #ifdef SJLJ_EXCEPTIONS

Index: libobjc/exception.c
===
--- libobjc/exception.c (revision 180086)
+++ libobjc/exception.c (working copy)
@@ -159,6 +159,11 @@
   info-ttype_encoding = *p++;
   if (info-ttype_encoding != DW_EH_PE_omit)
 {
+#if _GLIBCXX_OVERRIDE_TTYPE_ENCODING
+  /* Older ARM EABI toolchains set this value incorrectly, so use a
+hardcoded OS-specific format.  */
+  info-ttype_encoding = _GLIBCXX_OVERRIDE_TTYPE_ENCODING;
+#endif
   p = read_uleb128 (p, tmp);
   info-TType = p + tmp;
 }
@@ -174,27 +179,7 @@
   return p;
 }
 
-#ifdef __ARM_EABI_UNWINDER__
-
 static Class
-get_ttype_entry (struct lsda_header_info *info, _uleb128_t i)
-{
-  _Unwind_Ptr ptr;
-  
-  ptr = (_Unwind_Ptr) (info-TType - (i * 4));
-  ptr = _Unwind_decode_target2 (ptr);
-
-  /* NULL ptr means catch-all.  Note that if the class is not found,
- this will abort the program.  */
-  if (ptr)
-return objc_getRequiredClass ((const char *) ptr);
-  else
-return 0;
-}
-
-#else
-
-static Class
 get_ttype_entry (struct lsda_header_info *info, _Unwind_Word i)
 {
   _Unwind_Ptr ptr;
@@ -211,8 +196,6 @@
 return 0;
 }
 
-#endif
-
 /* Using a different personality function name causes link failures
when trying to mix code using different exception handling
models.  */


Re: [patch RFC,PR50038]

2011-10-17 Thread Ilya Enkovich
Ping.

Could please someone check if my approach is OK and it is worth to
continue work on patch for PR50038?

Thanks
Ilya

2011/10/11 Ilya Enkovich enkovich@gmail.com:
 2011/10/4 Richard Henderson r...@redhat.com:
 On 10/04/2011 08:42 AM, Joseph S. Myers wrote:
 On Tue, 4 Oct 2011, Ilya Tocar wrote:

 Hi everyone,

 This patch fixes PR 50038 (redundant zero extensions) by modifying
 implicit-zee pass
 to also remove unneeded zero extensions from QImode to SImode.

 Hardcoding particular modes like this in the target-independent parts of
 the compiler is fundamentally ill-conceived.  Right now it hardcodes the
 (SImode, DImode) pair.  You're adding hardcoding of (QImode, SImode) as
 well.  But really it should consider all pairs of (integer mode, wider
 integer mode), with the machine description (or target hooks) determining
 which pairs are relevant on a particular target.  Changing it not to
 hardcode particular modes would be better than adding a second pair.


 That along with not hard-coding ZERO_EXTEND.

 Both MIPS and Alpha have much the same free operations, but with SIGN_EXTEND.

 I remember rejecting one iteration of this pass with this hard-coded, but the
 pass was apparently approved by someone else without that being corrected.


 r~


 Hello guys,

 Could you please look at my patch version? I tried to remove all
 unnecessary mode restrictions and cover SIGN_EXTEND case. I did not
 test this patch yet, just checked it worked on reproducer from
 PR50038.

 Thanks
 Ilya
 ---
 gcc/

        * implicit-zee.c (ext_cand): New.
        (ext_cand_pool): Likewise.
        (add_ext_candidate): Likewise.
        (zee_init): Likewise.
        (zee_cleanup): Likewise.
        (combine_set_zero_extend): Get extend candidate as new parameter.
        Now handle sign extend cases and all modes.
        (transform_ifelse): Likewise.
        (merge_def_and_ze): Likewise.
        (combine_reaching_defs): Change parameter type.
        (zero_extend_info): Changed insn_list type.
        (add_removable_zero_extend): Relaxed mode and code filter.
        (find_removable_zero_extends): Changed return type.
        (find_and_remove_ze): Var type changes.
        (rest_of_handle_zee): Initialization and cleanup added.



Re: [C++ Patch] PR 32614

2011-10-17 Thread Richard Guenther
On Mon, 17 Oct 2011, Gabriel Dos Reis wrote:

 On Mon, Oct 17, 2011 at 5:53 AM, Richard Guenther rguent...@suse.de wrote:
  On Mon, 17 Oct 2011, Paolo Carlini wrote:
 
  On 10/17/2011 12:26 PM, Gabriel Dos Reis wrote:
   On Mon, Oct 17, 2011 at 4:42 AM, Paolo Carlinipaolo.carl...@oracle.com
   wrote:
FWIW, I still believe that tweaking the documentation to match the long
standing reality, would be a small improvement. I'm not going to 
further
insist, anyway.
   It isn't improvement.
   The improvement would be to restore the documented default.
  Well either my English is even weaker than I thought, or restoring 
  doesn't
  apply here: the line of code at  issue, pp_set_line_maximum_length (pp, 0),
  has been added by Gaby in Rev 70777, and nothing similar with 72 as second
  argument existed before.
 
  Thus clearly the documentation is wrong ;)
 
 ;-)
 Not necessarily.  Paolo does not say why that line was added.
 I don't remember adding that line to change the default.

The initial patch, split between rev. 31343 and 31999 indeed added
 
+  /* Enable automatic line wrapping by default */
+  set_message_length (72);

to C++ lang_decode_option.  Later it got appearantly lost somehow,
probably during some of the Great Option Reorgs.

I still think automatic wrapping (at 72 columns!?  A terminal
is 80x24!) should not be done by default.  You probably will break
a lot of existing scripts that assume the default of zero.

Richard.

Re: [C++ Patch] PR 32614

2011-10-17 Thread Gabriel Dos Reis
On Mon, Oct 17, 2011 at 5:38 AM, Paolo Carlini paolo.carl...@oracle.com wrote:
 On 10/17/2011 12:26 PM, Gabriel Dos Reis wrote:

 On Mon, Oct 17, 2011 at 4:42 AM, Paolo Carlinipaolo.carl...@oracle.com
  wrote:

 FWIW, I still believe that tweaking the documentation to match the long
 standing reality, would be a small improvement. I'm not going to further
 insist, anyway.

 It isn't improvement.
 The improvement would be to restore the documented default.

 Well either my English is even weaker than I thought, or restoring doesn't
 apply here: the line of code at  issue, pp_set_line_maximum_length (pp, 0),
 has been added by Gaby in Rev 70777, and nothing similar with 72 as second
 argument existed before.

Looking at the changset, now I remember:  That line was part of a change set
that was improving the *C* pretty-printer I added earlier and to
maximize  sharing
more cose between the C and C++ pretty printers.  The zero length was added
as an attempt to respect the *C* front-end desire not have line wrapping.
Richard talked about other front-ends, but at the time, there was only
two front-ends
who were using the pretty printers: C and C++.  The C front-end was adopting
bits of the C++ front-end.  Every other front-end were doing whatever
they wanted.
It wasn't like there was a bit debate with other front-ends to decide
what the default
should be for all.


Re: [C++ Patch] PR 32614

2011-10-17 Thread Paolo Carlini

On 10/17/2011 12:56 PM, Gabriel Dos Reis wrote:

Thus clearly the documentation is wrong ;)

;-)
Not necessarily.  Paolo does not say why that line was added.
I don't remember adding that line to change the default.
Indeed, as far as I can see, you added that line while *preserving* the 
existing behavior and preparing the C++ variant of the pretty_print 
machinery. Thus, AFAICS, 72 never existed anywhere and, strictly 
speaking, there is nothing to *restore*.


But I may be wrong, I don't own viewcvs, I just quickly queried it.

Paolo.


Re: [C++ Patch] PR 32614

2011-10-17 Thread Gabriel Dos Reis
On Mon, Oct 17, 2011 at 6:08 AM, Richard Guenther rguent...@suse.de wrote:

 The initial patch, split between rev. 31343 and 31999 indeed added

Thanks for helping tracking history.

 +  /* Enable automatic line wrapping by default */
 +  set_message_length (72);

 to C++ lang_decode_option.  Later it got appearantly lost somehow,
 probably during some of the Great Option Reorgs.

Yes, most likely.


 I still think automatic wrapping (at 72 columns!?  A terminal
 is 80x24!) should not be done by default.

The choice of 72 at the time was based on the 80 of the terminal
and Emacs guidelines.   The requests I have  heared most vocally was to
increase the length of line, not to suppress the wrapping by default.

 You probably will break
 a lot of existing scripts that assume the default of zero.

 Richard.


Re: [C++ Patch] PR 32614

2011-10-17 Thread Gabriel Dos Reis
On Mon, Oct 17, 2011 at 6:09 AM, Paolo Carlini paolo.carl...@oracle.com wrote:
 On 10/17/2011 12:56 PM, Gabriel Dos Reis wrote:

 Thus clearly the documentation is wrong ;)

 ;-)
 Not necessarily.  Paolo does not say why that line was added.
 I don't remember adding that line to change the default.

 Indeed, as far as I can see, you added that line while *preserving* the
 existing behavior and preparing the C++ variant of the pretty_print
 machinery. Thus, AFAICS, 72 never existed anywhere and, strictly speaking,
 there is nothing to *restore*.

I do not know what you mean by there is nothing to restore.
Look at the other mail by Richard.  The C pretty-printer *post*-dated
the C++ pretty printer.


 But I may be wrong, I don't own viewcvs, I just quickly queried it.

 Paolo.



Re: [C++ Patch] PR 32614

2011-10-17 Thread Paolo Carlini

On 10/17/2011 01:08 PM, Richard Guenther wrote:

The initial patch, split between rev. 31343 and 31999 indeed added

+  /* Enable automatic line wrapping by default */
+  set_message_length (72);

to C++ lang_decode_option.  Later it got appearantly lost somehow,
probably during some of the Great Option Reorgs.
Wow, 31343, I didn't look back so much. Now the issue is whether, after 
*so* many years with 0, people would *really* consider seeing the 
default changed to 72, at variance with C and all the other front-ends. 
I seriously doubt it. Do we have strong evidence of that? For example, 
is there something similar in other C++ front-end?


Paolo.


Re: [patch tree-optimization]: Improve handling of conditional-branches on targets with high branch costs

2011-10-17 Thread Richard Guenther
On Mon, Oct 17, 2011 at 12:59 PM, Kai Tietz ktiet...@googlemail.com wrote:
 2011/10/17 Richard Guenther richard.guent...@gmail.com:
 On Fri, Oct 14, 2011 at 9:43 PM, Kai Tietz ktiet...@googlemail.com wrote:
 Hello,

 So I committed the gimplify patch separate.  And here is the remaining
 fold-const patch.
 The important tests here are in gcc.dg/tree-ssa/builtin-expect[1-4].c, which
 cover the one special-case for branching. Also tree-ssa/20040204-1.c covers
 tests for branching code (on targets having high-engough BRANCH_COST and no
 special-casing - like MIPS, S/390, and AVR.

 ChangeLog

 2011-10-14  Kai Tietz  kti...@redhat.com

        * fold-const.c (simple_operand_p_2): New function.
        (fold_truthop): Rename to
        (fold_truth_andor_1): function name.
        Additionally remove branching creation for logical and/or.
        (fold_truth_andor): Handle branching creation for logical and/or 
 here.

 Bootstrapped and regression-tested for all languages plus Ada and
 Obj-C++ on x86_64-pc-linux-gnu.
 Ok for apply?

 Ok with ...

 Regards,
 Kai

 Index: gcc/gcc/fold-const.c
 ===
 --- gcc.orig/gcc/fold-const.c
 +++ gcc/gcc/fold-const.c
 @@ -112,13 +112,13 @@ static tree decode_field_reference (loca
  static int all_ones_mask_p (const_tree, int);
  static tree sign_bit_p (tree, const_tree);
  static int simple_operand_p (const_tree);
 +static bool simple_operand_p_2 (tree);
  static tree range_binop (enum tree_code, tree, tree, int, tree, int);
  static tree range_predecessor (tree);
  static tree range_successor (tree);
  static tree fold_range_test (location_t, enum tree_code, tree, tree, tree);
  static tree fold_cond_expr_with_comparison (location_t, tree, tree,
 tree, tree);
  static tree unextend (tree, int, int, tree);
 -static tree fold_truthop (location_t, enum tree_code, tree, tree, tree);
  static tree optimize_minmax_comparison (location_t, enum tree_code,
                                        tree, tree, tree);
  static tree extract_muldiv (tree, tree, enum tree_code, tree, bool *);
 @@ -3500,7 +3500,7 @@ optimize_bit_field_compare (location_t l
   return lhs;
  }

 -/* Subroutine for fold_truthop: decode a field reference.
 +/* Subroutine for fold_truth_andor_1: decode a field reference.

    If EXP is a comparison reference, we return the innermost reference.

 @@ -3668,7 +3668,7 @@ sign_bit_p (tree exp, const_tree val)
   return NULL_TREE;
  }

 -/* Subroutine for fold_truthop: determine if an operand is simple enough
 +/* Subroutine for fold_truth_andor_1: determine if an operand is simple 
 enough
    to be evaluated unconditionally.  */

  static int
 @@ -3678,7 +3678,7 @@ simple_operand_p (const_tree exp)
   STRIP_NOPS (exp);

   return (CONSTANT_CLASS_P (exp)
 -         || TREE_CODE (exp) == SSA_NAME
 +         || TREE_CODE (exp) == SSA_NAME
          || (DECL_P (exp)
               ! TREE_ADDRESSABLE (exp)
               ! TREE_THIS_VOLATILE (exp)
 @@ -3692,6 +3692,46 @@ simple_operand_p (const_tree exp)
                 registers aren't expensive.  */
               (! TREE_STATIC (exp) || DECL_REGISTER (exp;
  }
 +
 +/* Subroutine for fold_truth_andor: determine if an operand is simple 
 enough
 +   to be evaluated unconditionally.
 +   I addition to simple_operand_p, we assume that comparisons and logic-not
 +   operations are simple, if their operands are simple, too.  */
 +
 +static bool
 +simple_operand_p_2 (tree exp)
 +{
 +  enum tree_code code;
 +
 +  /* Strip any conversions that don't change the machine mode.  */
 +  STRIP_NOPS (exp);
 +
 +  code = TREE_CODE (exp);
 +
 +  if (TREE_CODE_CLASS (code) == tcc_comparison)
 +    return (!tree_could_trap_p (exp)
 +            simple_operand_p_2 (TREE_OPERAND (exp, 0))
 +            simple_operand_p_2 (TREE_OPERAND (exp, 1)));

 recurse with simple_operand_p.

 No, as this again would reject simple operations and additionally
 wouldn't check for trapping.

?  Your code allows arbitrarily complex expressions.  Also
tree_could_trap_p obviously extents to operands.


 +
 +  if (TREE_SIDE_EFFECTS (exp)
 +      || tree_could_trap_p (exp))

 Move this check before the tcc_comparison check and remove the
 then redundant tree_could_trap_p check there.

 Ok

 +    return false;
 +
 +  switch (code)
 +    {
 +    case SSA_NAME:
 +      return true;

 Do not handle here, it's handled in simple_operand_p.

 Well, was more a short-cut here.

 +    case TRUTH_NOT_EXPR:
 +      return simple_operand_p_2 (TREE_OPERAND (exp, 0));
 +    case BIT_NOT_EXPR:
 +      if (TREE_CODE (TREE_TYPE (exp)) != BOOLEAN_TYPE)
 +       return false;

 Remove the BIT_NOT_EXPR handling.  Thus, simply change this switch
 to

 Why should we reject simple ~X operations from gimplified code here?

Because this is FE triggered code.  From gimple you won't ever see
such complex expressions (never even the TRUTH_AND*_EXPR variants).

 I admit that from FE-code we won't see that, as always an 

Re: [C++ Patch] PR 32614

2011-10-17 Thread Paolo Carlini

On 10/17/2011 01:16 PM, Gabriel Dos Reis wrote:

On Mon, Oct 17, 2011 at 6:09 AM, Paolo Carlinipaolo.carl...@oracle.com  wrote:

On 10/17/2011 12:56 PM, Gabriel Dos Reis wrote:

Thus clearly the documentation is wrong ;)

;-)
Not necessarily.  Paolo does not say why that line was added.
I don't remember adding that line to change the default.

Indeed, as far as I can see, you added that line while *preserving* the
existing behavior and preparing the C++ variant of the pretty_print
machinery. Thus, AFAICS, 72 never existed anywhere and, strictly speaking,
there is nothing to *restore*.

I do not know what you mean by there is nothing to restore.
Look at the other mail by Richard.  The C pretty-printer *post*-dated
the C++ pretty printer.
Hey, I don't own viewcvs, of svn, for that matter, you could also dare 
to help a bit with this crazy archeological task, can't you?!? I looked 
back only untile 70777, and that point and a bit earlier there where 
already no 72s around, thus, right *nothing to restore*. Now we are 
learning that *even earlier* we had a 72. Fine. Now, after so many 
years, are we really sure that our users would consider an 
*improvement* a 72? I'm honestly not sure at all. Again, what the best 
C++ front-ends around do, by default? I'm sincerely curious.


Paolo.


Re: [C++ Patch] PR 32614

2011-10-17 Thread Gabriel Dos Reis
On Mon, Oct 17, 2011 at 6:14 AM, Paolo Carlini paolo.carl...@oracle.com wrote:
 On 10/17/2011 01:08 PM, Richard Guenther wrote:

 The initial patch, split between rev. 31343 and 31999 indeed added

 +  /* Enable automatic line wrapping by default */
 +  set_message_length (72);

 to C++ lang_decode_option.  Later it got appearantly lost somehow,
 probably during some of the Great Option Reorgs.

 Wow, 31343, I didn't look back so much.

I never understood why you doubted that the default was 72
way before I added the C pretty printer.  I can understand you
don't like it; but that should not involve doubting the facts.

 Now the issue is whether, after *so*
 many years with 0, people would *really* consider seeing the default changed
 to 72, at variance with C and all the other front-ends.

Again this argument is making a sort of revisionism.
The 72 default was added to g++, and other front-ends (in reality
at the time, only C could be affected) decided not to.  Over the years, we
have moved to share more and more codes with other front-ends.
The fact that other front-ends have been using more and more of the
code that was designed for g++ should not be argument to change the
default for g++.  It should be other reasons.

 I seriously doubt
 it. Do we have strong evidence of that? For example, is there something
 similar in other C++ front-end?

 Paolo.



Re: [C++ Patch] PR 32614

2011-10-17 Thread Paolo Carlini

On 10/17/2011 01:24 PM, Gabriel Dos Reis wrote:
Again this argument is making a sort of revisionism. The 72 default 
was added to g++, and other front-ends (in reality at the time, only C 
could be affected) decided not to. Over the years, we have moved to 
share more and more codes with other front-ends. The fact that other 
front-ends have been using more and more of the code that was designed 
for g++ should not be argument to change the default for g++. It 
should be other reasons. 
At this point, after something like 10 years, I think we badly need 
other reasons to change the C++ default. You are arguing as if we just 
inadvertently changed the C++ default yesterday. If I were a C++ 
front-end maintainer today, I would **strongly** oppose any change to 72 
not strongly motivated at least by a comparison with other high quality 
front-ends and some feedback from the users asking a different default. 
Do we have any of the latter?


Paolo.



Re: [C++ Patch] PR 32614

2011-10-17 Thread Richard Guenther
On Mon, 17 Oct 2011, Gabriel Dos Reis wrote:

 On Mon, Oct 17, 2011 at 6:08 AM, Richard Guenther rguent...@suse.de wrote:
 
  The initial patch, split between rev. 31343 and 31999 indeed added
 
 Thanks for helping tracking history.
 
  +  /* Enable automatic line wrapping by default */
  +  set_message_length (72);
 
  to C++ lang_decode_option.  Later it got appearantly lost somehow,
  probably during some of the Great Option Reorgs.
 
 Yes, most likely.
 
 
  I still think automatic wrapping (at 72 columns!?  A terminal
  is 80x24!) should not be done by default.
 
 The choice of 72 at the time was based on the 80 of the terminal
 and Emacs guidelines.

I see (and yes, editors probably do not break lines by default - but
my terminals do, and I usually enlarge them to be able to decipher
C++ diagnostics).  I still think that not breaking existing consumers
is more important than to restore something that hasn't been in
effect for years.

Oh, and yes, making documentation match reality is an improvement.

Let's wait for Jason to break the tie.

Richard.

Re: [C++ Patch] PR 32614

2011-10-17 Thread Gabriel Dos Reis
On Mon, Oct 17, 2011 at 6:19 AM, Paolo Carlini paolo.carl...@oracle.com wrote:
 On 10/17/2011 01:16 PM, Gabriel Dos Reis wrote:

 On Mon, Oct 17, 2011 at 6:09 AM, Paolo Carlinipaolo.carl...@oracle.com
  wrote:

 On 10/17/2011 12:56 PM, Gabriel Dos Reis wrote:

 Thus clearly the documentation is wrong ;)

 ;-)
 Not necessarily.  Paolo does not say why that line was added.
 I don't remember adding that line to change the default.

 Indeed, as far as I can see, you added that line while *preserving* the
 existing behavior and preparing the C++ variant of the pretty_print
 machinery. Thus, AFAICS, 72 never existed anywhere and, strictly
 speaking,
 there is nothing to *restore*.

 I do not know what you mean by there is nothing to restore.
 Look at the other mail by Richard.  The C pretty-printer *post*-dated
 the C++ pretty printer.

 Hey, I don't own viewcvs, of svn, for that matter, you could also dare to
 help a bit with this crazy archeological task, can't you?!?

Let's not be quick to judgment and throw more rocks before we get all
the facts. Please understand that I have been helping
and looking at past changesets and present history. I appreciate that Richard
did not think I was just be delusional and helped going back further.
 I can help by presenting history.  It is not my fault when you choose
to doubt or ignore.
That isn't under my control.


Re: [patch tree-optimization]: Improve handling of conditional-branches on targets with high branch costs

2011-10-17 Thread Kai Tietz
2011/10/17 Richard Guenther richard.guent...@gmail.com:
 On Mon, Oct 17, 2011 at 12:59 PM, Kai Tietz ktiet...@googlemail.com wrote:
 2011/10/17 Richard Guenther richard.guent...@gmail.com:
 On Fri, Oct 14, 2011 at 9:43 PM, Kai Tietz ktiet...@googlemail.com wrote:
 Hello,

 So I committed the gimplify patch separate.  And here is the remaining
 fold-const patch.
 The important tests here are in gcc.dg/tree-ssa/builtin-expect[1-4].c, 
 which
 cover the one special-case for branching. Also tree-ssa/20040204-1.c covers
 tests for branching code (on targets having high-engough BRANCH_COST and no
 special-casing - like MIPS, S/390, and AVR.

 ChangeLog

 2011-10-14  Kai Tietz  kti...@redhat.com

        * fold-const.c (simple_operand_p_2): New function.
        (fold_truthop): Rename to
        (fold_truth_andor_1): function name.
        Additionally remove branching creation for logical and/or.
        (fold_truth_andor): Handle branching creation for logical and/or 
 here.

 Bootstrapped and regression-tested for all languages plus Ada and
 Obj-C++ on x86_64-pc-linux-gnu.
 Ok for apply?

 Ok with ...

 Regards,
 Kai

 Index: gcc/gcc/fold-const.c
 ===
 --- gcc.orig/gcc/fold-const.c
 +++ gcc/gcc/fold-const.c
 @@ -112,13 +112,13 @@ static tree decode_field_reference (loca
  static int all_ones_mask_p (const_tree, int);
  static tree sign_bit_p (tree, const_tree);
  static int simple_operand_p (const_tree);
 +static bool simple_operand_p_2 (tree);
  static tree range_binop (enum tree_code, tree, tree, int, tree, int);
  static tree range_predecessor (tree);
  static tree range_successor (tree);
  static tree fold_range_test (location_t, enum tree_code, tree, tree, 
 tree);
  static tree fold_cond_expr_with_comparison (location_t, tree, tree,
 tree, tree);
  static tree unextend (tree, int, int, tree);
 -static tree fold_truthop (location_t, enum tree_code, tree, tree, tree);
  static tree optimize_minmax_comparison (location_t, enum tree_code,
                                        tree, tree, tree);
  static tree extract_muldiv (tree, tree, enum tree_code, tree, bool *);
 @@ -3500,7 +3500,7 @@ optimize_bit_field_compare (location_t l
   return lhs;
  }

 -/* Subroutine for fold_truthop: decode a field reference.
 +/* Subroutine for fold_truth_andor_1: decode a field reference.

    If EXP is a comparison reference, we return the innermost reference.

 @@ -3668,7 +3668,7 @@ sign_bit_p (tree exp, const_tree val)
   return NULL_TREE;
  }

 -/* Subroutine for fold_truthop: determine if an operand is simple enough
 +/* Subroutine for fold_truth_andor_1: determine if an operand is simple 
 enough
    to be evaluated unconditionally.  */

  static int
 @@ -3678,7 +3678,7 @@ simple_operand_p (const_tree exp)
   STRIP_NOPS (exp);

   return (CONSTANT_CLASS_P (exp)
 -         || TREE_CODE (exp) == SSA_NAME
 +         || TREE_CODE (exp) == SSA_NAME
          || (DECL_P (exp)
               ! TREE_ADDRESSABLE (exp)
               ! TREE_THIS_VOLATILE (exp)
 @@ -3692,6 +3692,46 @@ simple_operand_p (const_tree exp)
                 registers aren't expensive.  */
               (! TREE_STATIC (exp) || DECL_REGISTER (exp;
  }
 +
 +/* Subroutine for fold_truth_andor: determine if an operand is simple 
 enough
 +   to be evaluated unconditionally.
 +   I addition to simple_operand_p, we assume that comparisons and 
 logic-not
 +   operations are simple, if their operands are simple, too.  */
 +
 +static bool
 +simple_operand_p_2 (tree exp)
 +{
 +  enum tree_code code;
 +
 +  /* Strip any conversions that don't change the machine mode.  */
 +  STRIP_NOPS (exp);
 +
 +  code = TREE_CODE (exp);
 +
 +  if (TREE_CODE_CLASS (code) == tcc_comparison)
 +    return (!tree_could_trap_p (exp)
 +            simple_operand_p_2 (TREE_OPERAND (exp, 0))
 +            simple_operand_p_2 (TREE_OPERAND (exp, 1)));

 recurse with simple_operand_p.

 No, as this again would reject simple operations and additionally
 wouldn't check for trapping.

 ?  Your code allows arbitrarily complex expressions.  Also
 tree_could_trap_p obviously extents to operands.

Ah, ok. I wasn't aware that it walks into tree.


 +
 +  if (TREE_SIDE_EFFECTS (exp)
 +      || tree_could_trap_p (exp))

 Move this check before the tcc_comparison check and remove the
 then redundant tree_could_trap_p check there.

 Ok

 +    return false;
 +
 +  switch (code)
 +    {
 +    case SSA_NAME:
 +      return true;

 Do not handle here, it's handled in simple_operand_p.

 Well, was more a short-cut here.

 +    case TRUTH_NOT_EXPR:
 +      return simple_operand_p_2 (TREE_OPERAND (exp, 0));
 +    case BIT_NOT_EXPR:
 +      if (TREE_CODE (TREE_TYPE (exp)) != BOOLEAN_TYPE)
 +       return false;

 Remove the BIT_NOT_EXPR handling.  Thus, simply change this switch
 to

 Why should we reject simple ~X operations from gimplified code here?

 Because this is FE triggered code.  From gimple you won't ever see
 such complex 

Re: [C++ Patch] PR 32614

2011-10-17 Thread Gabriel Dos Reis
On Mon, Oct 17, 2011 at 6:29 AM, Richard Guenther rguent...@suse.de wrote:
 On Mon, 17 Oct 2011, Gabriel Dos Reis wrote:

 On Mon, Oct 17, 2011 at 6:08 AM, Richard Guenther rguent...@suse.de wrote:

  The initial patch, split between rev. 31343 and 31999 indeed added

 Thanks for helping tracking history.

  +  /* Enable automatic line wrapping by default */
  +  set_message_length (72);
 
  to C++ lang_decode_option.  Later it got appearantly lost somehow,
  probably during some of the Great Option Reorgs.

 Yes, most likely.

 
  I still think automatic wrapping (at 72 columns!?  A terminal
  is 80x24!) should not be done by default.

 The choice of 72 at the time was based on the 80 of the terminal
 and Emacs guidelines.

 I see (and yes, editors probably do not break lines by default - but
 my terminals do, and I usually enlarge them to be able to decipher
 C++ diagnostics).

These days, it is common for terminals to have line wrap.  At the time,
it wasn't common.  Another suggestion I have heard when the line break
was introduced, was to do it based on the characteristics of the output
stream -- for example, adding colors (hello Benjamin!) which I believe
SuSE added, and which demands testing the output stream. Another
suggestion was people wanted to look at COLUMNS to automatically
set the line length -- this comes from people using more than 80.
-fmessage-length=0 had an internal necessity: the testsuites.  I modified
the testsuite framework to automatically set the length to 0.

   I still think that not breaking existing consumers
 is more important than to restore something that hasn't been in
 effect for years.

 Oh, and yes, making documentation match reality is an improvement.

 Let's wait for Jason to break the tie.

 Richard.


Re: [patch tree-optimization]: Improve handling of conditional-branches on targets with high branch costs

2011-10-17 Thread Richard Guenther
On Mon, Oct 17, 2011 at 1:31 PM, Kai Tietz ktiet...@googlemail.com wrote:
 2011/10/17 Richard Guenther richard.guent...@gmail.com:
 On Mon, Oct 17, 2011 at 12:59 PM, Kai Tietz ktiet...@googlemail.com wrote:
 2011/10/17 Richard Guenther richard.guent...@gmail.com:
 On Fri, Oct 14, 2011 at 9:43 PM, Kai Tietz ktiet...@googlemail.com wrote:
 Hello,

 So I committed the gimplify patch separate.  And here is the remaining
 fold-const patch.
 The important tests here are in gcc.dg/tree-ssa/builtin-expect[1-4].c, 
 which
 cover the one special-case for branching. Also tree-ssa/20040204-1.c 
 covers
 tests for branching code (on targets having high-engough BRANCH_COST and 
 no
 special-casing - like MIPS, S/390, and AVR.

 ChangeLog

 2011-10-14  Kai Tietz  kti...@redhat.com

        * fold-const.c (simple_operand_p_2): New function.
        (fold_truthop): Rename to
        (fold_truth_andor_1): function name.
        Additionally remove branching creation for logical and/or.
        (fold_truth_andor): Handle branching creation for logical and/or 
 here.

 Bootstrapped and regression-tested for all languages plus Ada and
 Obj-C++ on x86_64-pc-linux-gnu.
 Ok for apply?

 Ok with ...

 Regards,
 Kai

 Index: gcc/gcc/fold-const.c
 ===
 --- gcc.orig/gcc/fold-const.c
 +++ gcc/gcc/fold-const.c
 @@ -112,13 +112,13 @@ static tree decode_field_reference (loca
  static int all_ones_mask_p (const_tree, int);
  static tree sign_bit_p (tree, const_tree);
  static int simple_operand_p (const_tree);
 +static bool simple_operand_p_2 (tree);
  static tree range_binop (enum tree_code, tree, tree, int, tree, int);
  static tree range_predecessor (tree);
  static tree range_successor (tree);
  static tree fold_range_test (location_t, enum tree_code, tree, tree, 
 tree);
  static tree fold_cond_expr_with_comparison (location_t, tree, tree,
 tree, tree);
  static tree unextend (tree, int, int, tree);
 -static tree fold_truthop (location_t, enum tree_code, tree, tree, tree);
  static tree optimize_minmax_comparison (location_t, enum tree_code,
                                        tree, tree, tree);
  static tree extract_muldiv (tree, tree, enum tree_code, tree, bool *);
 @@ -3500,7 +3500,7 @@ optimize_bit_field_compare (location_t l
   return lhs;
  }

 -/* Subroutine for fold_truthop: decode a field reference.
 +/* Subroutine for fold_truth_andor_1: decode a field reference.

    If EXP is a comparison reference, we return the innermost reference.

 @@ -3668,7 +3668,7 @@ sign_bit_p (tree exp, const_tree val)
   return NULL_TREE;
  }

 -/* Subroutine for fold_truthop: determine if an operand is simple enough
 +/* Subroutine for fold_truth_andor_1: determine if an operand is simple 
 enough
    to be evaluated unconditionally.  */

  static int
 @@ -3678,7 +3678,7 @@ simple_operand_p (const_tree exp)
   STRIP_NOPS (exp);

   return (CONSTANT_CLASS_P (exp)
 -         || TREE_CODE (exp) == SSA_NAME
 +         || TREE_CODE (exp) == SSA_NAME
          || (DECL_P (exp)
               ! TREE_ADDRESSABLE (exp)
               ! TREE_THIS_VOLATILE (exp)
 @@ -3692,6 +3692,46 @@ simple_operand_p (const_tree exp)
                 registers aren't expensive.  */
               (! TREE_STATIC (exp) || DECL_REGISTER (exp;
  }
 +
 +/* Subroutine for fold_truth_andor: determine if an operand is simple 
 enough
 +   to be evaluated unconditionally.
 +   I addition to simple_operand_p, we assume that comparisons and 
 logic-not
 +   operations are simple, if their operands are simple, too.  */
 +
 +static bool
 +simple_operand_p_2 (tree exp)
 +{
 +  enum tree_code code;
 +
 +  /* Strip any conversions that don't change the machine mode.  */
 +  STRIP_NOPS (exp);
 +
 +  code = TREE_CODE (exp);
 +
 +  if (TREE_CODE_CLASS (code) == tcc_comparison)
 +    return (!tree_could_trap_p (exp)
 +            simple_operand_p_2 (TREE_OPERAND (exp, 0))
 +            simple_operand_p_2 (TREE_OPERAND (exp, 1)));

 recurse with simple_operand_p.

 No, as this again would reject simple operations and additionally
 wouldn't check for trapping.

 ?  Your code allows arbitrarily complex expressions.  Also
 tree_could_trap_p obviously extents to operands.

 Ah, ok. I wasn't aware that it walks into tree.


 +
 +  if (TREE_SIDE_EFFECTS (exp)
 +      || tree_could_trap_p (exp))

 Move this check before the tcc_comparison check and remove the
 then redundant tree_could_trap_p check there.

 Ok

 +    return false;
 +
 +  switch (code)
 +    {
 +    case SSA_NAME:
 +      return true;

 Do not handle here, it's handled in simple_operand_p.

 Well, was more a short-cut here.

 +    case TRUTH_NOT_EXPR:
 +      return simple_operand_p_2 (TREE_OPERAND (exp, 0));
 +    case BIT_NOT_EXPR:
 +      if (TREE_CODE (TREE_TYPE (exp)) != BOOLEAN_TYPE)
 +       return false;

 Remove the BIT_NOT_EXPR handling.  Thus, simply change this switch
 to

 Why should we reject simple ~X operations from gimplified code here?

 

Re: [C++ Patch] PR 32614

2011-10-17 Thread Gabriel Dos Reis
On Mon, Oct 17, 2011 at 6:26 AM, Paolo Carlini paolo.carl...@oracle.com wrote:
 I would **strongly** oppose any change to 72 not strongly
 motivated at least by a comparison with other high quality front-ends

I love it when you make arguments like this.  It makes me smile, and
I like smiling when I just get off bed :-)

Just another fact: the decision of line wrapping was based in comparison
to what another high quality front-end was doing.  It wasn't a design decision
made out of tin air.

I suspect you do not even agree with the fact that the change was accidental,
not intended.  Yet, I bet you will shortly tell me that I am not helping with
giving history behind the changes.


Re: [C++ Patch] PR 32614

2011-10-17 Thread Paolo Carlini

On 10/17/2011 01:44 PM, Gabriel Dos Reis wrote:

On Mon, Oct 17, 2011 at 6:26 AM, Paolo Carlinipaolo.carl...@oracle.com  wrote:

I would **strongly** oppose any change to 72 not strongly
motivated at least by a comparison with other high quality front-ends

I love it when you make arguments like this.  It makes me smile, and
I like smiling when I just get off bed :-)
I don't see why. In any case please consider also the second half of 
that phrase, the users, like Richard for example, or me. And please help 
re-assessing the situation wrt the other front-ends *today* not in the 
stone age.


Paolo.


Re: [PATCH 3/6] Emit macro expansion related diagnostics

2011-10-17 Thread Dodji Seketeli
Richard Guenther richard.guent...@gmail.com writes:

 This broke bootstrap on x86_64-linux.

 /space/rguenther/src/svn/trunk/libcpp/line-map.c: In function
 'source_location linemap_macro_map_loc_to_exp_point(const line_map*,
 source_location)':
 /space/rguenther/src/svn/trunk/libcpp/line-map.c:628:12: error:
 variable 'token_no' set but not used [-Werror=unused-but-set-variable]
 cc1plus: all warnings being treated as errors

Sigh.

I guess the reason why my testing hasn't caught this is that I am
bootstrapping with --enable-checking and so on my system ENABLE_CHECKING
is defined and my GCC_VERSION = 2007.

I am bootstrapping and testing the obvious patch below with
--disable-bootstrap and I am planning to check it in if it passes, under
the obvious rule.

Sorry for the inconvenience.

commit e957242a9a8ec8f297e05ca0dae1d63bf543fad8
Author: Dodji Seketeli do...@redhat.com
Date:   Mon Oct 17 13:33:41 2011 +0200

Fix bootstrapping with --disable-checking

libcpp/ChangeLog

* line-map.c (linemap_macro_map_loc_to_exp_point): Avoid setting a
variable without using it if ENABLE_CHECKING is not defined.

diff --git a/libcpp/line-map.c b/libcpp/line-map.c
index 87b8bfe..a1d0fbb 100644
--- a/libcpp/line-map.c
+++ b/libcpp/line-map.c
@@ -625,14 +625,12 @@ source_location
 linemap_macro_map_loc_to_exp_point (const struct line_map *map,
source_location location)
 {
-  unsigned token_no;
-
   linemap_assert (linemap_macro_expansion_map_p (map)
   location = MAP_START_LOCATION (map));
 
   /* Make sure LOCATION is correct.  */
-  token_no = location - MAP_START_LOCATION (map);
-  linemap_assert (token_no   MACRO_MAP_NUM_MACRO_TOKENS (map));
+  linemap_assert ((location - MAP_START_LOCATION (map))
+   MACRO_MAP_NUM_MACRO_TOKENS (map));
 
   return MACRO_MAP_EXPANSION_POINT_LOCATION (map);
 }
-- 
Dodji


Re: [C++ Patch] PR 32614

2011-10-17 Thread Gabriel Dos Reis
On Mon, Oct 17, 2011 at 6:48 AM, Paolo Carlini paolo.carl...@oracle.com wrote:
 On 10/17/2011 01:44 PM, Gabriel Dos Reis wrote:

 On Mon, Oct 17, 2011 at 6:26 AM, Paolo Carlinipaolo.carl...@oracle.com
  wrote:

 I would **strongly** oppose any change to 72 not strongly
 motivated at least by a comparison with other high quality front-ends

 I love it when you make arguments like this.  It makes me smile, and
 I like smiling when I just get off bed :-)

 I don't see why.

If I reveal that then I may very well lose a valuable source of
smile inducer :-)

  In any case please consider also the second half of that
 phrase, the users, like Richard for example, or me.

you should consider I did -- just like I give you the benefits
of doubt (for example, I do not assume you want to be rude
for entertainment purposes).

 And please help
 re-assessing the situation wrt the other front-ends *today* not in the stone
 age.

 Paolo.



Re: [VTA, PR49310] O(n+m)-ish emit_notes

2011-10-17 Thread Jakub Jelinek
On Tue, Oct 11, 2011 at 04:19:52PM -0300, Alexandre Oliva wrote:
 Here's what I've got so far.  Regstrapped on x86_64-linux-gnu and
 i686-linux-gnu.  Ok to install?

I see
+FAIL: gcc.c-torture/compile/pr19080.c  -O3 -g  (internal compiler error)
+FAIL: gcc.c-torture/compile/pr19080.c  -O3 -g  (test for excess errors)
regression with this patch on x86_64-linux (--enable-checking=yes,rtl), ICE
in var-tracking.  Can you please look at that?

+/* Enumeration type used to discriminate various types of one-part
+   variables.  */
+typedef enum onepart_enum {

Just a style nit, isn't { supposed to go on the next line?

+  /* Not a one-part variable.  */
+  NOT_ONEPART = 0,
+  /* A one-part DECL that is not a DEBUG_EXPR_DECL.  */
+  ONEPART_VDECL = 1,
+  /* A DEBUG_EXPR_DECL.  */
+  ONEPART_DEXPR = 2,
+  /* A VALUE.  */
+  ONEPART_VALUE = 3
+} onepart_enum_t;
+
 /* Structure describing where the variable is located.  */
 typedef struct variable_def
 {

Otherwise looks ok.

Jakub


[PATCH][RFC] Fix PR50716, override type alignment knowledge

2011-10-17 Thread Richard Guenther

This changes alignment computation of a MEM during expansion
from MAX (TYPE_ALIGN (TREE_TYPE (exp)), get_object_alignment (exp))
to something weaker (with possibly less alignment) when we
were able to compute an explicit misaligned value (thus,
get_object_alignment_1 (exp, misalign) would return an
alignment of 8, but misaligned by 4, for example - in which
case get_object_alignment () would return 4).  This is to
less often fall into the trap that the frontends and the middle-end
are not careful about providing a type with properly alignment
for misaligned memory references (classical example is
struct __attribute__((packed)) { int i; } a; a, where
a has int * type, not int __attribute__((aligned(1))) * type).

The patch unbreaks the testcase in the PR (which is strictly
invalid) by using precise misalign information we are able to
compute at -O1 and up:

typedef int vec __attribute__((vector_size(16)));
int main ()
{
 int * arr = __builtin_malloc (1024);
 vec *p = (vec *) arr[1];
 *p = (vec){1, 2, 3, 4};
 return *(char *)p;
}

on x86_64 without the patch we emit movaps while with the
patch we use a movups instruction (which is being nice to
our users).

I didn't bother to try thinking of a heuristic when to
use the misaligned info and when not - always using it
makes the most sense consistency-wise.  I had considered
only using TYPE_ALIGN () when get_object_alignment_1 returns
BITS_PER_UNIT for the alignment (which sort-of means
don't know).

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

Any comments?

Thanks,
Richard.

2011-10-17  Richard Guenther  rguent...@suse.de

PR middle-end/50716
* expr.c (get_object_or_type_alignment): New function.
(expand_assignment): Use it.
(expand_expr_real_1): Likewise.

Index: gcc/expr.c
===
*** gcc/expr.c  (revision 180077)
--- gcc/expr.c  (working copy)
*** get_bit_range (unsigned HOST_WIDE_INT *b
*** 4544,4549 
--- 4544,4568 
  }
  }
  
+ /* Return the alignment of the object EXP, also considering its type
+when we do not know of explicit misalignment.
+???  Note that generally the type of an expression is not kept
+consistent with misalignment information by the frontend, for
+example when taking the address of a member of a packed structure.
+So taking into account type information for alignment is generally
+wrong, but is done here as a compromise.  */
+ 
+ static unsigned int
+ get_object_or_type_alignment (tree exp)
+ {
+   unsigned HOST_WIDE_INT misalign;
+   unsigned int align = get_object_alignment_1 (exp, misalign);
+   align = MAX (TYPE_ALIGN (TREE_TYPE (exp)), align);
+   if (misalign != 0)
+ align = (misalign  -misalign);
+   return align;
+ }
+ 
  /* Expand an assignment that stores the value of FROM into TO.  If NONTEMPORAL
 is true, try generating a nontemporal store.  */
  
*** expand_assignment (tree to, tree from, b
*** 4553,4559 
rtx to_rtx = 0;
rtx result;
enum machine_mode mode;
!   int align;
enum insn_code icode;
  
/* Don't crash if the lhs of the assignment was erroneous.  */
--- 4572,4578 
rtx to_rtx = 0;
rtx result;
enum machine_mode mode;
!   unsigned int align;
enum insn_code icode;
  
/* Don't crash if the lhs of the assignment was erroneous.  */
*** expand_assignment (tree to, tree from, b
*** 4571,4578 
if ((TREE_CODE (to) == MEM_REF
 || TREE_CODE (to) == TARGET_MEM_REF)
 mode != BLKmode
!((align = MAX (TYPE_ALIGN (TREE_TYPE (to)), get_object_alignment 
(to)))
!  (signed) GET_MODE_ALIGNMENT (mode))
 ((icode = optab_handler (movmisalign_optab, mode))
  != CODE_FOR_nothing))
  {
--- 4590,4597 
if ((TREE_CODE (to) == MEM_REF
 || TREE_CODE (to) == TARGET_MEM_REF)
 mode != BLKmode
!((align = get_object_or_type_alignment (to))
!  GET_MODE_ALIGNMENT (mode))
 ((icode = optab_handler (movmisalign_optab, mode))
  != CODE_FOR_nothing))
  {
*** expand_expr_real_1 (tree exp, rtx target
*** 9241,9247 
addr_space_t as = TYPE_ADDR_SPACE (TREE_TYPE (exp));
struct mem_address addr;
enum insn_code icode;
!   int align;
  
get_address_description (exp, addr);
op0 = addr_for_mem_ref (addr, as, true);
--- 9260,9266 
addr_space_t as = TYPE_ADDR_SPACE (TREE_TYPE (exp));
struct mem_address addr;
enum insn_code icode;
!   unsigned int align;
  
get_address_description (exp, addr);
op0 = addr_for_mem_ref (addr, as, true);
*** expand_expr_real_1 (tree exp, rtx target
*** 9249,9257 
temp = gen_rtx_MEM (mode, op0);
set_mem_attributes (temp, exp, 0);
set_mem_addr_space (temp, as);
!   align = MAX (TYPE_ALIGN (TREE_TYPE (exp)), get_object_alignment (exp));

[PATCH] Fix PR50729

2011-10-17 Thread Richard Guenther

This fixes PR50729, so much for not implementing it by using
value-ranges ... opens up the door for more bugs ;)

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

Richard.

2011-10-17  Richard Guenther  rguent...@suse.de

PR tree-optimization/50729
* tree-vrp.c (extract_range_from_unary_expr_1): Remove
redundant test.
(simplify_conversion_using_ranges): Properly test the
intermediate result.

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

Index: gcc/tree-vrp.c
===
*** gcc/tree-vrp.c  (revision 180077)
--- gcc/tree-vrp.c  (working copy)
*** extract_range_from_unary_expr_1 (value_r
*** 2913,2927 
 determining if it evaluates to NULL [0, 0] or non-NULL (~[0, 0]).  */
if (POINTER_TYPE_P (type))
{
! if (CONVERT_EXPR_CODE_P (code))
!   {
! if (range_is_nonnull (vr0))
!   set_value_range_to_nonnull (vr, type);
! else if (range_is_null (vr0))
!   set_value_range_to_null (vr, type);
! else
!   set_value_range_to_varying (vr);
!   }
  else
set_value_range_to_varying (vr);
  return;
--- 2913,2922 
 determining if it evaluates to NULL [0, 0] or non-NULL (~[0, 0]).  */
if (POINTER_TYPE_P (type))
{
! if (range_is_nonnull (vr0))
!   set_value_range_to_nonnull (vr, type);
! else if (range_is_null (vr0))
!   set_value_range_to_null (vr, type);
  else
set_value_range_to_varying (vr);
  return;
*** simplify_conversion_using_ranges (gimple
*** 7288,7297 
  TYPE_UNSIGNED (TREE_TYPE (middleop)));
middlemax = double_int_ext (innermax, TYPE_PRECISION (TREE_TYPE (middleop)),
  TYPE_UNSIGNED (TREE_TYPE (middleop)));
!   /* If the middle values do not represent a proper range fail.  */
!   if (double_int_cmp (middlemin, middlemax,
! TYPE_UNSIGNED (TREE_TYPE (middleop)))  0)
  return false;
if (!double_int_equal_p (double_int_ext (middlemin,
   TYPE_PRECISION (finaltype),
   TYPE_UNSIGNED (finaltype)),
--- 7283,7299 
  TYPE_UNSIGNED (TREE_TYPE (middleop)));
middlemax = double_int_ext (innermax, TYPE_PRECISION (TREE_TYPE (middleop)),
  TYPE_UNSIGNED (TREE_TYPE (middleop)));
!   /* If the middle values are not equal to the original values fail.
!  But only if the inner cast truncates (thus we ignore differences
!  in extension to handle the case going from a range to an anti-range
!  and back).  */
!   if ((TYPE_PRECISION (TREE_TYPE (innerop))
! TYPE_PRECISION (TREE_TYPE (middleop)))
!(!double_int_equal_p (innermin, middlemin)
! || !double_int_equal_p (innermax, middlemax)))
  return false;
+   /* Require that the final conversion applied to both the original
+  and the intermediate range produces the same result.  */
if (!double_int_equal_p (double_int_ext (middlemin,
   TYPE_PRECISION (finaltype),
   TYPE_UNSIGNED (finaltype)),
Index: gcc/testsuite/gcc.dg/torture/pr50729.c
===
*** gcc/testsuite/gcc.dg/torture/pr50729.c  (revision 0)
--- gcc/testsuite/gcc.dg/torture/pr50729.c  (revision 0)
***
*** 0 
--- 1,20 
+ /* { dg-do run } */
+ /* { dg-require-effective-target int32plus } */
+ 
+ extern void abort (void);
+ unsigned short __attribute__((noinline))
+ foo (int i)
+ {
+   if (i = 0
+i = 0x40)
+ return (unsigned short)(signed char)i;
+   return i;
+ }
+ int main()
+ {
+   int i;
+   for (i = 0; i  0x; ++i)
+ if (foo(i) != (unsigned short)(signed char) i)
+   abort ();
+   return 0;
+ }


Re: [patch tree-optimization]: Improve handling of conditional-branches on targets with high branch costs

2011-10-17 Thread Kai Tietz
Ok, I see.  This might be profitable to do that.  So fold_truth_op
hunk looks like this

@@ -5149,13 +5176,6 @@ fold_truthop (location_t loc, enum tree_
   build2 (BIT_IOR_EXPR, TREE_TYPE (ll_arg),
   ll_arg, rl_arg),
   build_int_cst (TREE_TYPE (ll_arg), 0));
-
-  if (LOGICAL_OP_NON_SHORT_CIRCUIT)
-   {
- if (code != orig_code || lhs != orig_lhs || rhs != orig_rhs)
-   return build2_loc (loc, code, truth_type, lhs, rhs);
- return NULL_TREE;
-   }
 }

   /* See if the comparisons can be merged.  Then get all the parameters for
@@ -8380,13 +8400,77 @@ fold_truth_andor (location_t loc, enum t
  lhs is another similar operation, try to merge its rhs with our
  rhs.  Then try to merge our lhs and rhs.  */
   if (TREE_CODE (arg0) == code
-   0 != (tem = fold_truthop (loc, code, type,
-  TREE_OPERAND (arg0, 1), arg1)))
+   0 != (tem = fold_truth_andor_1 (loc, code, type,
+TREE_OPERAND (arg0, 1), arg1)))
 return fold_build2_loc (loc, code, type, TREE_OPERAND (arg0, 0), tem);

-  if ((tem = fold_truthop (loc, code, type, arg0, arg1)) != 0)
+  if ((tem = fold_truth_andor_1 (loc, code, type, arg0, arg1)) != 0)
 return tem;

+  if ((BRANCH_COST (optimize_function_for_speed_p (cfun),
+   false) = 2)
+   LOGICAL_OP_NON_SHORT_CIRCUIT
+   simple_operand_p_2 (arg1))
+{
+  enum tree_code ncode;
+
+  if (code == TRUTH_ANDIF_EXPR || code == TRUTH_ORIF_EXPR)
+{
+ ncode = (code == TRUTH_ANDIF_EXPR ? TRUTH_AND_EXPR : TRUTH_OR_EXPR);
+
+ /* Transform ((A AND-IF B) AND-IF C) into (A AND-IF (B AND C)),
+or ((A OR-IF B) OR-IF C) into (A OR-IF (B OR C))
+We don't want to pack more than two leafs to a non-IF AND/OR
+expression.
+If tree-code of left-hand operand isn't an AND/OR-IF code and not
+equal to CODE, then we don't want to add right-hand operand.
+If the inner right-hand side of left-hand operand has
+side-effects, or isn't simple, then we can't add to it,
+as otherwise we might destroy if-sequence.  */
+ if (TREE_CODE (arg0) == code
+ /* Needed for sequence points to handle trappings, and
+side-effects.  */
+  simple_operand_p_2 (TREE_OPERAND (arg0, 1)))
+   {
+ tem = fold_build2_loc (loc, ncode, type, TREE_OPERAND (arg0, 1),
+arg1);
+ return fold_build2_loc (loc, code, type, TREE_OPERAND (arg0, 0),
+ tem);
+   }
+ /* Transform (A AND-IF B) into (A AND B), or (A OR-IF B)
+into (A OR B).
+For sequence point consistancy, we need to check for trapping,
+and side-effects.  */
+ else if (simple_operand_p_2 (arg0))
+   return fold_build2_loc (loc, ncode, type, arg0, arg1);
+   }
+  else
+{
+ ncode = (code == TRUTH_AND_EXPR ? TRUTH_ANDIF_EXPR
+ : TRUTH_ORIF_EXPR);
+ /* Transform ((A AND-IF B) AND C) into (A AND-IF (B AND C)),
+or ((A OR-IF B) OR C) into (A OR-IF (B OR C))
+We don't want to pack more than two leafs to a non-IF AND/OR
+expression.
+If tree-code of left-hand operand isn't an AND/OR-IF code and not
+equal to NCODE, then we don't want to add right-hand operand.
+If the inner right-hand side of left-hand operand has
+side-effects, or isn't simple, then we can't add to it,
+as otherwise we might destroy if-sequence.  */
+ if (TREE_CODE (arg0) == ncode
+ /* Needed for sequence points to handle trappings, and
+side-effects.  */
+  simple_operand_p_2 (TREE_OPERAND (arg0, 1)))
+   {
+ tem = fold_build2_loc (loc, code, type, TREE_OPERAND (arg0, 1),
+arg1);
+ return fold_build2_loc (loc, ncode, type,
+ TREE_OPERAND (arg0, 0), tem);
+   }
+   }
+
+}
+
   return NULL_TREE;
 }

Ok, with other changes you mentioned?

Regards,
Kai


Re: [pph] Refactor Vectors and Chains (issue5263051)

2011-10-17 Thread Diego Novillo
On Sun, Oct 16, 2011 at 22:09, Lawrence Crowl cr...@google.com wrote:
 Factor the vector and chain output routines to remove boolean control
 parameters. The functions pph_out_tree_vec_1 and pph_out_chain_1 split
 their conditional parts of their implementation into their use cases,
 calling each other as needed.

 pph_out_tree_vec - nothing special
 pph_out_tree_vec_unchain - unchaining
 pph_out_mergeable_tree_vec - merging, unchaining, reversing
 pph_out_tree_vec_filtered - filtering
 pph_out_chain - nothing special
 pph_out_chain_filtered - filtering
 pph_out_mergeable_chain_filtered - merging, unchaining, reversing, filtering

But, you are duplicating code that the previous patch had explicitly
commonized.  Why?  It's easier to keep the core streaming logic in one
function, to have a single point of debugging when dealing with sync
problems.


 This change fixes an ordering bug, but now causes an ICE to surface,
 which will be addressed later.

What ordering bug?


Diego.


Re: [PATCH, testsuite, i386] FMA3 testcases + typo fix in MD

2011-10-17 Thread Kirill Yukhin
Thanks!

K

On Sat, Oct 15, 2011 at 3:08 PM, Uros Bizjak ubiz...@gmail.com wrote:
 On Sat, Oct 15, 2011 at 10:32 AM, Uros Bizjak ubiz...@gmail.com wrote:

 --- /dev/null
 +++ b/gcc/testsuite/gcc.target/i386/fma_double_1.c
 @@ -0,0 +1,19 @@
 +/* { dg-do compile } */
 +/* { dg-prune-output .*warning: 'sseregparm' attribute ignored.* } */

 That prunes too much.  gcc-dg-prune brackets the regexp so that it only
 matches within a single line, but your use of .* (which also matches
 newlines) overrides that and will cause the *whole* output to be thrown
 away, including any unrelated error messages.

 Perhaps e can add -Wno-attributes to dg-options and remove dg-prune-output.

 I will look at this possibility and create a patch.

 It works.

 2011-10-15  Uros Bizjak  ubiz...@gmail.com

        * gcc.target/i386/fma_float_?.c (dg-prune_output): Remove.
        (dg-options): Add -Wno-attributes.
        * gcc.target/i386/fma_double_?.c: Ditto.
        * gcc.target/i386/fma_run_float_?.c: Ditto.
        * gcc.target/i386/fma_run_double_?.c: Ditto.
        * gcc.target/i386/l_fma_float_?.c: Dtto.
        * gcc.target/i386/l_fma_double_?.c: Ditto.
        * gcc.target/i386/l_fma_run_float_?.c: Ditto.
        * gcc.target/i386/l_fma_run_double_?.c: Ditto.

 Attached patch was tested on x86_64-pc-linux-gnu {,-m32} and committed to SVN.

 Uros.



[Patch, Fortran ] PR 50016: Slow Fortran I/O on Windows and flushing/_commit

2011-10-17 Thread Tobias Burnus
This patch adds a call to _commit() on _WIN32 for the FLUSH subroutine 
and the FLUSH statement. It removes the _commit from gfortran's buf_flush.


Background:
* gfortran internally buffers the I/O, but it calls the nonbuffering 
open/write/read functions (and not, e.g., fopen/fwrite/fread). On 
Unix/Darwin/Linux system, the changes become immediately visible to all 
processes after a write().
* On Windows, there seems to be a file-descriptor specific system buffer 
such that the data only becomes available to other processes after 
either a _commit or after closing the file.
* The Windows _commit() is a combination of a (system) buffer flush - as 
a write() already implies on POSIX systems, but it also ensures that 
the data ends up on the disk, which on POSIX systems is done via fsync().


Currently, _commit() is also called for the internal buf_flush, which 
can happen very often and thus delays the I/O a lot (cf. PR 50016). With 
this patch, it is only called when the user explicitly called FLUSH(). 
Contrary to non-_WIN32 systems, this also flushes to the hard disk, 
which causes some slow down. However, as the user (should) only rarely 
call the function, it should not pose a real performance issue.


The patch should ensure that the values can be read via other 
file-descriptors within the same program or from other programs; on the 
same time, it prevents excessive _commit() calling as it is done with 
the current code.


An alternative would be not to call _commit() at all, leaving it to the 
user. That would match the fsync() result on POSIX systems, but I think 
having a file not available in the current status for other processes 
causes more confusion that it helps. Thus, I think for FLUSH, one should 
really make sure other processes get the right data.


The patch was build and regtested on x86-64-linux (where it should be a 
noop).

OK for the trunk?

Can someone test it on MinGW/MinGW-64?

Tobias

PS: For the discussion, see 
http://gcc.gnu.org/ml/fortran/2011-10/msg00079.html


PPS: I regard this patch as rather independent of INQUIRE even if the 
issue first occurred with INQUIRE (PR 44698). Thus, it is independent 
from whether one calls stat() for an open file or whether one uses a 
different method like seek(SEEK_END) + tell() to obtain the file size.


2011-10-17  Tobias Burnus  bur...@net-b.de

	PR fortran/50016
	* intrinsic.texi (FLUSH): Document that it calls _commit
	on MinGW(-w64).

2011-10-17  Tobias Burnus  bur...@net-b.de

	PR fortran/50016
	* io/file_pos.c (st_flush): Call _commit on MinGW(-w64).
	* io/intrinsics.c (flush_i4, flush_i8): Ditto.
	* io/unix.c (flush_all_units_1, flush_all_units): Ditto.
	(buf_flush): Remove _commit call.

diff --git a/gcc/fortran/intrinsic.texi b/gcc/fortran/intrinsic.texi
index 11f87a5..1a0e930 100644
--- a/gcc/fortran/intrinsic.texi
+++ b/gcc/fortran/intrinsic.texi
@@ -4775,7 +4775,10 @@ that the data is committed to disk.
 On POSIX systems, you can request that all data is transferred  to  the
 storage device by calling the @code{fsync} function, with the POSIX file
 descriptor of the I/O unit as argument (retrieved with GNU intrinsic
-@code{FNUM}). The following example shows how:
+@code{FNUM}). The following example shows how. On Windows (MinGW and
+MinGW-w64), the @code{FLUSH} subroutine and @code{FLUSH} statement always
+call @code{_commit} to ensure that other file descriptors see the current
+file status; @code{_commit} additionally flushes the data to the disk.
 
 @smallexample
   ! Declare the interface for POSIX fsync function
diff --git a/libgfortran/io/file_pos.c b/libgfortran/io/file_pos.c
index b034f38..4efdc1f 100644
--- a/libgfortran/io/file_pos.c
+++ b/libgfortran/io/file_pos.c
@@ -452,6 +452,10 @@ st_flush (st_parameter_filepos *fpp)
 fbuf_flush (u, u-mode);
 
   sflush (u-s);
+#ifdef _WIN32
+  /* Without _commit, changes are not visible to other file descriptors.  */
+  _commit (u-s-fd);
+#endif
   unlock_unit (u);
 }
   else
diff --git a/libgfortran/io/intrinsics.c b/libgfortran/io/intrinsics.c
index f48bd77..eaf2e17 100644
--- a/libgfortran/io/intrinsics.c
+++ b/libgfortran/io/intrinsics.c
@@ -207,6 +207,11 @@ flush_i4 (GFC_INTEGER_4 *unit)
   if (us != NULL)
 	{
 	  sflush (us-s);
+#ifdef _WIN32
+	  /* Without _commit, changes are not visible
+	 to other file descriptors.  */
+	  _commit (u-s-fd);
+#endif
 	  unlock_unit (us);
 	}
 }
@@ -230,6 +235,11 @@ flush_i8 (GFC_INTEGER_8 *unit)
   if (us != NULL)
 	{
 	  sflush (us-s);
+#ifdef _WIN32
+	  /* Without _commit, changes are not visible
+	 to other file descriptors.  */
+	  _commit (u-s-fd);
+#endif
 	  unlock_unit (us);
 	}
 }
diff --git a/libgfortran/io/unix.c b/libgfortran/io/unix.c
index 25cb559..a88e83b 100644
--- a/libgfortran/io/unix.c
+++ b/libgfortran/io/unix.c
@@ -443,10 +443,6 @@ buf_flush (unix_stream * s)
   if (s-ndirty != 0)
 return -1;
 
-#ifdef _WIN32
-  _commit (s-fd);
-#endif
-
   return 0;
 }
 

Re: [ARM] Fix PR49641

2011-10-17 Thread Richard Earnshaw
On 14/10/11 14:31, Bernd Schmidt wrote:
 On 07/13/11 16:03, Richard Earnshaw wrote:
 * config/arm/arm.c (store_multiple_sequence): Avoid cases where
 the base reg is stored iff compiling for Thumb1.

 * gcc.target/arm/pr49641.c: New test.
 
 Ping.  Richard, you replied to the mail but didn't comment on the patch.
 
 
 Bernd
 


Sorry, I thought I'd made it clear that I don't think the compiler
should ever use STM with write-back if the base register is in the
stored list.  We must certainly never do it if the base register is not
the first register in the list as this has always been unpredictable.

BTW, this is not Thumb1 specific, it applies at all times.


So, no the patch is not OK as it stands.

R.



Re: [patch tree-optimization]: Improve handling of conditional-branches on targets with high branch costs

2011-10-17 Thread Richard Guenther
On Mon, Oct 17, 2011 at 2:22 PM, Kai Tietz ktiet...@googlemail.com wrote:
 Ok, I see.  This might be profitable to do that.  So fold_truth_op
 hunk looks like this

 @@ -5149,13 +5176,6 @@ fold_truthop (location_t loc, enum tree_
                           build2 (BIT_IOR_EXPR, TREE_TYPE (ll_arg),
                                   ll_arg, rl_arg),
                           build_int_cst (TREE_TYPE (ll_arg), 0));
 -
 -      if (LOGICAL_OP_NON_SHORT_CIRCUIT)
 -       {
 -         if (code != orig_code || lhs != orig_lhs || rhs != orig_rhs)
 -           return build2_loc (loc, code, truth_type, lhs, rhs);
 -         return NULL_TREE;
 -       }
     }

   /* See if the comparisons can be merged.  Then get all the parameters for
 @@ -8380,13 +8400,77 @@ fold_truth_andor (location_t loc, enum t
      lhs is another similar operation, try to merge its rhs with our
      rhs.  Then try to merge our lhs and rhs.  */
   if (TREE_CODE (arg0) == code
 -       0 != (tem = fold_truthop (loc, code, type,
 -                                  TREE_OPERAND (arg0, 1), arg1)))
 +       0 != (tem = fold_truth_andor_1 (loc, code, type,
 +                                        TREE_OPERAND (arg0, 1), arg1)))
     return fold_build2_loc (loc, code, type, TREE_OPERAND (arg0, 0), tem);

 -  if ((tem = fold_truthop (loc, code, type, arg0, arg1)) != 0)
 +  if ((tem = fold_truth_andor_1 (loc, code, type, arg0, arg1)) != 0)
     return tem;

 +  if ((BRANCH_COST (optimize_function_for_speed_p (cfun),
 +                   false) = 2)
 +       LOGICAL_OP_NON_SHORT_CIRCUIT
 +       simple_operand_p_2 (arg1))
 +    {
 +      enum tree_code ncode;
 +
 +      if (code == TRUTH_ANDIF_EXPR || code == TRUTH_ORIF_EXPR)
 +        {
 +         ncode = (code == TRUTH_ANDIF_EXPR ? TRUTH_AND_EXPR : TRUTH_OR_EXPR);
 +
 +         /* Transform ((A AND-IF B) AND-IF C) into (A AND-IF (B AND C)),
 +            or ((A OR-IF B) OR-IF C) into (A OR-IF (B OR C))
 +            We don't want to pack more than two leafs to a non-IF AND/OR
 +            expression.
 +            If tree-code of left-hand operand isn't an AND/OR-IF code and not
 +            equal to CODE, then we don't want to add right-hand operand.
 +            If the inner right-hand side of left-hand operand has
 +            side-effects, or isn't simple, then we can't add to it,
 +            as otherwise we might destroy if-sequence.  */
 +         if (TREE_CODE (arg0) == code
 +             /* Needed for sequence points to handle trappings, and
 +                side-effects.  */
 +              simple_operand_p_2 (TREE_OPERAND (arg0, 1)))
 +           {
 +             tem = fold_build2_loc (loc, ncode, type, TREE_OPERAND (arg0, 1),
 +                                    arg1);
 +             return fold_build2_loc (loc, code, type, TREE_OPERAND (arg0, 0),
 +                                     tem);
 +           }
 +         /* Transform (A AND-IF B) into (A AND B), or (A OR-IF B)
 +            into (A OR B).
 +            For sequence point consistancy, we need to check for trapping,
 +            and side-effects.  */
 +         else if (simple_operand_p_2 (arg0))
 +           return fold_build2_loc (loc, ncode, type, arg0, arg1);
 +       }
 +      else
 +        {
 +         ncode = (code == TRUTH_AND_EXPR ? TRUTH_ANDIF_EXPR
 +                                         : TRUTH_ORIF_EXPR);
 +         /* Transform ((A AND-IF B) AND C) into (A AND-IF (B AND C)),
 +            or ((A OR-IF B) OR C) into (A OR-IF (B OR C))
 +            We don't want to pack more than two leafs to a non-IF AND/OR
 +            expression.
 +            If tree-code of left-hand operand isn't an AND/OR-IF code and not
 +            equal to NCODE, then we don't want to add right-hand operand.
 +            If the inner right-hand side of left-hand operand has
 +            side-effects, or isn't simple, then we can't add to it,
 +            as otherwise we might destroy if-sequence.  */
 +         if (TREE_CODE (arg0) == ncode
 +             /* Needed for sequence points to handle trappings, and
 +                side-effects.  */
 +              simple_operand_p_2 (TREE_OPERAND (arg0, 1)))
 +           {
 +             tem = fold_build2_loc (loc, code, type, TREE_OPERAND (arg0, 1),
 +                                    arg1);
 +             return fold_build2_loc (loc, ncode, type,
 +                                     TREE_OPERAND (arg0, 0), tem);
 +           }
 +       }
 +
 +    }
 +
   return NULL_TREE;
  }

 Ok, with other changes you mentioned?

This can be done without so much code duplication.

 Regards,
 Kai



Re: [pph] Make libcpp symbol validation a warning (issue5235061)

2011-10-17 Thread Diego Novillo
On Fri, Oct 14, 2011 at 20:27, Gabriel Charette gcharet...@gmail.com wrote:
 Yes, I understand that.

 But when the second 2.pph is skipped when reading foo.pph, the reading
 of its line_table is also skipped (as foo.pph doesn't contain the
 line_table information for 2.h, 2.pph does and adds it when its
 included as a child, but if it's skipped, the line_table info for 2.h
 should never make it in the line_table), so I don't see why this is an
 issue for the line_table (other than the assert about the number of
 line table entries read). What I was suggesting is that as far as the
 assert is concerned it would be stronger to count the number of
 skipped child headers on read and assert num_read+num_skipped ==
 num_expected_childs basically (it is still only an assert so no big
 deal I guess).

Ah, I see what you are saying.  I didn't really bother too much with
that assert.  Since I was not reading the line table again, I figured
both asserts were triggering because of the different values coming
from the skipped file, so I left them out.


 Essentially this patch fixes the last bug we had in the line_table
 merging (i.e. that guarded out headers in the non-pph version weren't
 guarded out in the pph version) and this is a good thing. I'm just
 being picky about weakening asserts!


 I still think it would be nice to have a way to test constructs like
 the line_table at the end of parsing (maybe a new flag, as I was
 suggesting in my previous email, as gcc doesn't allow for modular
 testing) and compare pph and non-pph versions. Testing at this level
 would potentially be much better than trying to understand tricky test
 failures from the ground up. This is beyond the scope of this patch of
 course, but something to keep in mind I think.

Yeah.  I'll come back to it at a later point.


Diego.


Re: [patch] dwarf2out: Drop the size + performance overhead of DW_AT_sibling

2011-10-17 Thread Tom Tromey
 Tristan == Tristan Gingold ging...@adacore.com writes:

Tom Another way to look at it is that there have been many changes to GCC's
Tom DWARF output in the last few years.  Surely these have broken these
Tom DWARF consumers more than this change possibly could.

Tristan Yes, but there is -gstrict-dwarf to stay compatible with old behavior.

Yes, but this change is strictly compliant.  What makes it different
from any other change that was made to make GCC more compliant?
Hypothetical consumers could also break on those changes.

Tom


Re: [PATCH][ARM] -m{cpu,tune,arch}=native

2011-10-17 Thread Richard Earnshaw
On 20/09/11 11:51, Andrew Stubbs wrote:
 On 09/09/11 12:55, Richard Earnshaw wrote:
 The part number field is meaningless outside of the context of a a
 specific vendor -- only taken as a pair can they refer to a specific
 part.  So why is the vendor field hard-coded rather than factored into
 the table of parts.

 Maybe it would be better to have a table of tables, with the top-level
 table being indexed by vendor id.  Something like
 
 Yes, but since I only have part numbers for one vendor, I left that sort 
 of thing out on the principle that it's best not to add complexity until 
 you need it.
 
 Anyway, I have done it now, so here it is. :)
 
 I've also fixed the problem that if it didn't recognise the CPU, it 
 defaulted to the hard default, ignoring the --with-cpu configured default.
 
 OK?
 
 Andrew
 
 
 tune-native.patch
 
 
 2011-09-20  Andrew Stubbs  a...@codesourcery.com
 
   gcc/
   * config.host (arm*-*-linux*): Add driver-arm.o and x-arm.
   * config/arm/arm.opt: Add 'native' processor_type and
   arm_arch enum values.
   * config/arm/arm.h (host_detect_local_cpu): New prototype.
   (EXTRA_SPEC_FUNCTIONS): New define.
   (MCPU_MTUNE_NATIVE_SPECS): New define.
   (DRIVER_SELF_SPECS): New define.
   * config/arm/driver-arm.c: New file.
   * config/arm/x-arm: New file.
   * doc/invoke.texi (ARM Options): Document -mcpu=native,
   -mtune=native and -march=native.
 

There's a presumption in host_detect_local_cpu() that CPU implementer
will appear before  CPU part in the output of /proc/cpuinfo.  That's
probably a pretty safe assumption (and it appears that it will handle
that case relatively safely -- ie not crash).  I'll let you decide
whether that's important enough to fix.

However another problem I've just spotted is that you never close the
file descriptor if you fail to parse the output properly (ie jump to
not_found).  That should be fixed before this is committed.

OK with the second issue resolved.


R.



[C++ Patch] PR 44524

2011-10-17 Thread Paolo Carlini

Hi,

here submitter requests a more accurate error message for X.Y where X is 
a pointer to class type. Thus the below, tested x86_64-linux.


Ok for mainline?

Thanks,
Paolo.

//
/cp
2011-10-17  Paolo Carlini  paolo.carl...@oracle.com

PR c++/44524
* typeck.c (build_class_member_access_expr): Provide a better error
message for X.Y where X is a pointer to class type.
(finish_class_member_access_expr): Likewise.

/testsuite
2011-10-17  Paolo Carlini  paolo.carl...@oracle.com

PR c++/44524
* g++.dg/parse/error41.C: New.
* g++.dg/parse/error20.C: Adjust.
Index: testsuite/g++.dg/parse/error20.C
===
--- testsuite/g++.dg/parse/error20.C(revision 180087)
+++ testsuite/g++.dg/parse/error20.C(working copy)
@@ -12,7 +12,7 @@ struct C {
 };
 int main() {
   C c;
-  A(c.p.i); // { dg-error 9:request for member 'i' in 'c.C::p', which is of 
non-class type 'B }
+  A(c.p.i); // { dg-error 9:request for member 'i' in 'c.C::p', which has 
pointer type 'B }
   return 0;
 }
 
Index: testsuite/g++.dg/parse/error41.C
===
--- testsuite/g++.dg/parse/error41.C(revision 0)
+++ testsuite/g++.dg/parse/error41.C(revision 0)
@@ -0,0 +1,11 @@
+// PR c++/44524
+
+templatetypename, typename
+struct map
+{
+  bool empty();
+};
+
+int bar(mapint, float *X) {
+  return X.empty();  // { dg-error which has pointer type 'map }
+}
Index: cp/typeck.c
===
--- cp/typeck.c (revision 180087)
+++ cp/typeck.c (working copy)
@@ -2128,8 +2128,16 @@ build_class_member_access_expr (tree object, tree
   if (!CLASS_TYPE_P (object_type))
 {
   if (complain  tf_error)
-   error (request for member %qD in %qE, which is of non-class type %qT,
-  member, object, object_type);
+   {
+ if (POINTER_TYPE_P (object_type)
+  CLASS_TYPE_P (TREE_TYPE (object_type)))
+   error (request for member %qD in %qE, which has pointer 
+  type %qT (maybe you meant to use %-% ?),
+  member, object, object_type);
+ else
+   error (request for member %qD in %qE, which is of non-class 
+  type %qT, member, object, object_type);
+   }
   return error_mark_node;
 }
 
@@ -2508,8 +2516,16 @@ finish_class_member_access_expr (tree object, tree
   if (!CLASS_TYPE_P (object_type))
 {
   if (complain  tf_error)
-   error (request for member %qD in %qE, which is of non-class type %qT,
-  name, object, object_type);
+   {
+ if (POINTER_TYPE_P (object_type)
+  CLASS_TYPE_P (TREE_TYPE (object_type)))
+   error (request for member %qD in %qE, which has pointer 
+  type %qT (maybe you meant to use %-% ?),
+  name, object, object_type);
+ else
+   error (request for member %qD in %qE, which is of non-class 
+  type %qT, name, object, object_type);
+   }
   return error_mark_node;
 }
 


Re: [patch tree-optimization]: Improve handling of conditional-branches on targets with high branch costs

2011-10-17 Thread Kai Tietz
Sure,

Is simplier and also handles (A T[-IF] (B T-IF C) - (A T B) T-IF C
case, which can happen by framing in conditions.

@@ -8380,13 +8400,65 @@ fold_truth_andor (location_t loc, enum t
  lhs is another similar operation, try to merge its rhs with our
  rhs.  Then try to merge our lhs and rhs.  */
   if (TREE_CODE (arg0) == code
-   0 != (tem = fold_truthop (loc, code, type,
-  TREE_OPERAND (arg0, 1), arg1)))
+   0 != (tem = fold_truth_andor_1 (loc, code, type,
+TREE_OPERAND (arg0, 1), arg1)))
 return fold_build2_loc (loc, code, type, TREE_OPERAND (arg0, 0), tem);

-  if ((tem = fold_truthop (loc, code, type, arg0, arg1)) != 0)
+  if ((tem = fold_truth_andor_1 (loc, code, type, arg0, arg1)) != 0)
 return tem;

+  if ((BRANCH_COST (optimize_function_for_speed_p (cfun),
+   false) = 2)
+   LOGICAL_OP_NON_SHORT_CIRCUIT)
+{
+  enum tree_code ncode, icode;
+
+  ncode = (code == TRUTH_ANDIF_EXPR || code == TRUTH_AND_EXPR)
+ ? TRUTH_AND_EXPR : TRUTH_OR_EXPR;
+  icode = ncode == TRUTH_AND_EXPR ? TRUTH_ANDIF_EXPR : TRUTH_ORIF_EXPR;
+
+  /* Transform ((A AND-IF B) AND[-IF] C) into (A AND-IF (B AND C)),
+or ((A OR-IF B) OR[-IF] C) into (A OR-IF (B OR C))
+We don't want to pack more than two leafs to a non-IF AND/OR
+expression.
+If tree-code of left-hand operand isn't an AND/OR-IF code and not
+equal to IF-CODE, then we don't want to add right-hand operand.
+If the inner right-hand side of left-hand operand has
+side-effects, or isn't simple, then we can't add to it,
+as otherwise we might destroy if-sequence.  */
+  if (TREE_CODE (arg0) == icode
+  simple_operand_p_2 (arg1)
+ /* Needed for sequence points to handle trappings, and
+side-effects.  */
+  simple_operand_p_2 (TREE_OPERAND (arg0, 1)))
+   {
+ tem = fold_build2_loc (loc, ncode, type, TREE_OPERAND (arg0, 1),
+arg1);
+ return fold_build2_loc (loc, icode, type, TREE_OPERAND (arg0, 0),
+ tem);
+   }
+   /* Same as abouve but for (A AND[-IF] (B AND-IF C)) - ((A AND B) 
AND-IF C),
+  or (A OR[-IF] (B OR-IF C) - ((A OR B) OR-IF C).  */
+  else if (TREE_CODE (arg1) == icode
+  simple_operand_p_2 (arg0)
+ /* Needed for sequence points to handle trappings, and
+side-effects.  */
+  simple_operand_p_2 (TREE_OPERAND (arg1, 0)))
+   {
+ tem = fold_build2_loc (loc, ncode, type,
+arg0, TREE_OPERAND (arg1, 0));
+ return fold_build2_loc (loc, icode, type, tem,
+ TREE_OPERAND (arg1, 1));
+   }
+  /* Transform (A AND-IF B) into (A AND B), or (A OR-IF B)
+into (A OR B).
+For sequence point consistancy, we need to check for trapping,
+and side-effects.  */
+  else if (code == icode  simple_operand_p_2 (arg0)
+simple_operand_p_2 (arg1))
+   return fold_build2_loc (loc, ncode, type, arg0, arg1);
+}
+
   return NULL_TREE;
 }

Regards,
Kai


Re: [C++ Patch] PR 44524

2011-10-17 Thread Gabriel Dos Reis
On Mon, Oct 17, 2011 at 8:32 AM, Paolo Carlini paolo.carl...@oracle.com wrote:
 Hi,

 here submitter requests a more accurate error message for X.Y where X is a
 pointer to class type. Thus the below, tested x86_64-linux.

 Ok for mainline?

s/is of pointer type/has pointer type/g



 Thanks,
 Paolo.

 //



Re: [PATCH 3/6] Emit macro expansion related diagnostics

2011-10-17 Thread Dodji Seketeli
Finally here is what I am checking in, which passes bootstrap with
--disable-checking --enable-languages=all,ada -- modulo this other bug
that breaks bootstrap as well
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50709.

It's been OKed off-line by Tom Tromey.

commit c1cd2be336ceec75cf40ac5f32cc4f72b8fc5da3
Author: Dodji Seketeli do...@redhat.com
Date:   Mon Oct 17 13:33:41 2011 +0200

Fix bootstrapping with --disable-checking

libcpp/ChangeLog

* line-map.c (linemap_macro_map_loc_to_exp_point): Avoid setting a
variable without using it if ENABLE_CHECKING is not defined.  Mark
the LOCATION parameter as being unused.

diff --git a/libcpp/line-map.c b/libcpp/line-map.c
index 87b8bfe..43e2856 100644
--- a/libcpp/line-map.c
+++ b/libcpp/line-map.c
@@ -621,18 +621,16 @@ linemap_macro_expansion_map_p (const struct line_map *map)
Read the comments of struct line_map and struct line_map_macro in
line-map.h to understand what a macro expansion point is.  */
 
-source_location
+static source_location
 linemap_macro_map_loc_to_exp_point (const struct line_map *map,
-   source_location location)
+   source_location location ATTRIBUTE_UNUSED)
 {
-  unsigned token_no;
-
   linemap_assert (linemap_macro_expansion_map_p (map)
   location = MAP_START_LOCATION (map));
 
   /* Make sure LOCATION is correct.  */
-  token_no = location - MAP_START_LOCATION (map);
-  linemap_assert (token_no   MACRO_MAP_NUM_MACRO_TOKENS (map));
+  linemap_assert ((location - MAP_START_LOCATION (map))
+   MACRO_MAP_NUM_MACRO_TOKENS (map));
 
   return MACRO_MAP_EXPANSION_POINT_LOCATION (map);
 }
-- 
Dodji


Re: [C++ Patch] PR 44524

2011-10-17 Thread Paolo Carlini

On 10/17/2011 03:39 PM, Gabriel Dos Reis wrote:

On Mon, Oct 17, 2011 at 8:32 AM, Paolo Carlinipaolo.carl...@oracle.com  wrote:

Hi,

here submitter requests a more accurate error message for X.Y where X is a
pointer to class type. Thus the below, tested x86_64-linux.

Ok for mainline?

s/is of pointer type/has pointer type/g
Thanks, changed in my local tree. By the way, I wondered that, then 
found a couple of instances of 'has pointer' in the tree, but only in 
comments, if I remember correctly.


Paolo.



Re: [patch] C6X unwinding/exception handling

2011-10-17 Thread Paul Brook
 I checked the attached patch, test results at
 http://gcc.gnu.org/ml/gcc-testresults/2011-10/msg01377.html
 
 which are the same as with my suggested patch.
 
 Ok for the trunk?

I probably don't have authority to approve this, but looks OK to me.

Paul


Re: Out-of-order update of new_spill_reg_store[]

2011-10-17 Thread Bernd Schmidt
 gcc/
   * reload1.c (reload_regs_reach_end_p): Replace with...
   (reload_reg_rtx_reaches_end_p): ...this function.
   (new_spill_reg_store): Update commentary.
   (emit_input_reload_insns): Don't clear new_spill_reg_store here.
   (emit_output_reload_insns): Check reload_reg_rtx_reaches_end_p
   before setting new_spill_reg_store.
   (emit_reload_insns): Use a separate loop to clear new_spill_reg_store.
   Use reload_reg_rtx_reaches_end_p instead of reload_regs_reach_end_p.
   Also use reload_reg_rtx_reaches_end_p when recording inheritance
   information for non-spill reload registers.

Just an update to say that based on our discussion I think the general
approach is OK, but I'm still trying to figure out what exactly this
piece of code is doing, and whether the changes to it make sense:

 @@ -8329,30 +8329,33 @@ emit_reload_insns (struct insn_chain *ch
the storing insn so that we may delete this insn with
delete_output_reload.  */
 src_reg = reload_reg_rtx_for_output[r];
 -
 -   /* If this is an optional reload, try to find the source reg
 -  from an input reload.  */
 -   if (! src_reg)
 +   if (src_reg
 +reload_reg_rtx_reaches_end_p (src_reg, r))
 + store_insn = new_spill_reg_store[REGNO (src_reg)];
 +   else
   {
 +   /* If this is an optional reload, try to find the
 +  source reg from an input reload.  */
 rtx set = single_set (insn);
 if (set  SET_DEST (set) == rld[r].out)
   {
 int k;
 +   rtx cand;
  
 src_reg = SET_SRC (set);
 store_insn = insn;
 for (k = 0; k  n_reloads; k++)
 - {
 -   if (rld[k].in == src_reg)
 - {
 -   src_reg = reload_reg_rtx_for_input[k];
 -   break;
 - }
 - }
 + if (rld[k].in == src_reg)
 +   {
 + cand = reload_reg_rtx_for_input[k];
 + if (reload_reg_rtx_reaches_end_p (cand, k))
 +   {
 + src_reg = cand;
 + break;
 +   }
 +   }
   }
   }
 -   else
 - store_insn = new_spill_reg_store[REGNO (src_reg)];
 if (src_reg  REG_P (src_reg)
  REGNO (src_reg)  FIRST_PSEUDO_REGISTER)
   {


Bernd


Re: [PATCH 0/9] [RFC] Expand SMS functionality

2011-10-17 Thread Richard Sandiford
Roman Zhuykov zhr...@ispras.ru writes:
 [PATCH 4/9] Move the SMS pass earlier
 http://gcc.gnu.org/ml/gcc-patches/2011-07/msg01811.html

I don't think this is a good idea.  To get good results, SMS really
needs to run as close to the register allocator as possible, otherwise
later passes might disrupt the schedule.

Richard


Re: [patch] C6X unwinding/exception handling

2011-10-17 Thread Nicola Pero
 I checked the attached patch, test results at
 http://gcc.gnu.org/ml/gcc-testresults/2011-10/msg01377.html
 
 which are the same as with my suggested patch.
 
 Ok for the trunk?

 I probably don't have authority to approve this, but looks OK to me.

The libobjc bits are Ok for trunk.

Thanks



Re: [PATCH, i386 tests] New tests to check vectorization for AVX2 insns.

2011-10-17 Thread Kirill Yukhin
Thanks for inputs, Jakub!

I am attaching updated patch.

Updated testsuite/ChangeLog entry:
2011-10-17  Kirill Yukhin  kirill.yuk...@intel.com

* gcc.target/i386/avx2-vpop-check.h: New header.
* gcc.target/i386/avx2-vpaddd-3.c: New test.
* gcc.target/i386/avx2-vpaddw-3.c: Ditto.
* gcc.target/i386/avx2-vpaddb-3.c: Ditto.
* gcc.target/i386/avx2-vpaddq-3.c: Ditto.
* gcc.target/i386/avx2-vpand-3.c: Ditto.
* gcc.target/i386/avx2-vpmulld-3.c: Ditto.
* gcc.target/i386/avx2-vpmullw-3.c: Ditto.
* gcc.target/i386/avx2-vpsrad-3.c: Ditto.
* gcc.target/i386/avx2-vpsraw-3.c: Ditto.
* gcc.target/i386/avx2-vpsrld-3.c: Ditto.
* gcc.target/i386/avx2-vpsrlw-3.c: Ditto.
* gcc.target/i386/avx2-vpsubb-3.c: Ditto.
* gcc.target/i386/avx2-vpsubd-3.c: Ditto.
* gcc.target/i386/avx2-vpsubq-3.c: Ditto.
* gcc.target/i386/avx2-vpsubw-3.c: Ditto.


Could you please guys have a look?

K


avx2.vect-3.tests.gcc.patch
Description: Binary data


Re: [PATCH] Simplify and fix restrict handling

2011-10-17 Thread Richard Guenther
On Fri, 14 Oct 2011, Richard Guenther wrote:

 
 This follows up Michas testcase where we fail to handle the
 conservatively propagated restrict tags properly.  The following
 patch simplifies handling of restrict in the oracle and thus
 only excludes NONLOCAL (as designed), but not ESCAPED from
 conflict checking.
 
 Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

So, after some regressions caused by this patch and some more thinking
(about more possible issues) I concluded that we can simplify things
even more by not making restrict vars point to NONLOCAL, but only
to their tag (but marking that as global and able to have a points-to
set).  This way the special-casing of NONLOCAL vs. restrict can go
away, and with it all its possible problems.  Restrict is now
similar to malloc () memory that escapes.

Hopefully this one is without regressions ;)

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

It seems we can remove DECL_IS_RESTRICTED_P again, as well as
the internal is_restrict_var flag.  I'll do that if the patch
tests ok (the is_restrict_var flag removal immediately, the
DECL_IS_RESTRICTED_P removal as followup as it touches Fortran).

Thanks,
Richard.

2011-10-17  Richard Guenther  rguent...@suse.de

* tree-ssa-alias.h (struct pt_solution): Remove
vars_contains_restrict member.
(pt_solutions_same_restrict_base): Remove.
(pt_solution_set): Adjust.
* tree-ssa-alias.c (ptr_deref_may_alias_decl_p): Remove
vars_contains_restrict handling.
(dump_points_to_solution): Likewise.
(ptr_derefs_may_alias_p): Do not call pt_solutions_same_restrict_base.
* tree-ssa-structalias.c (make_constraint_from_restrict):
Make the tag global.
(create_variable_info_for): Do not make restrict vars point
to NONLOCAL.
(intra_create_variable_infos): Likewise.
(find_what_var_points_to): Remove vars_contains_restrict handling.
(pt_solution_set): Adjust.
(pt_solution_ior_into): Likewise.
(pt_solutions_same_restrict_base): Remove.
* cfgexpand.c (update_alias_info_with_stack_vars): Adjust.
* gimple-pretty-print.c (pp_points_to_solution): Likewise.

Index: gcc/tree-ssa-alias.c
===
--- gcc/tree-ssa-alias.c(revision 180077)
+++ gcc/tree-ssa-alias.c(working copy)
@@ -219,13 +219,6 @@ ptr_deref_may_alias_decl_p (tree ptr, tr
   if (!pi)
 return true;
 
-  /* If the decl can be used as a restrict tag and we have a restrict
- pointer and that pointers points-to set doesn't contain this decl
- then they can't alias.  */
-  if (DECL_RESTRICTED_P (decl)
-   pi-pt.vars_contains_restrict)
-return bitmap_bit_p (pi-pt.vars, DECL_PT_UID (decl));
-
   return pt_solution_includes (pi-pt, decl);
 }
 
@@ -316,11 +309,6 @@ ptr_derefs_may_alias_p (tree ptr1, tree
   if (!pi1 || !pi2)
 return true;
 
-  /* If both pointers are restrict-qualified try to disambiguate
- with restrict information.  */
-  if (!pt_solutions_same_restrict_base (pi1-pt, pi2-pt))
-return false;
-
   /* ???  This does not use TBAA to prune decls from the intersection
  that not both pointers may access.  */
   return pt_solutions_intersect (pi1-pt, pi2-pt);
@@ -426,8 +414,6 @@ dump_points_to_solution (FILE *file, str
   dump_decl_set (file, pt-vars);
   if (pt-vars_contains_global)
fprintf (file,  (includes global vars));
-  if (pt-vars_contains_restrict)
-   fprintf (file,  (includes restrict tags));
 }
 }
 
Index: gcc/tree-ssa-alias.h
===
--- gcc/tree-ssa-alias.h(revision 180077)
+++ gcc/tree-ssa-alias.h(working copy)
@@ -54,8 +54,6 @@ struct GTY(()) pt_solution
   /* Nonzero if the pt_vars bitmap includes a global variable.  */
   unsigned int vars_contains_global : 1;
 
-  /* Nonzero if the pt_vars bitmap includes a restrict tag variable.  */
-  unsigned int vars_contains_restrict : 1;
 
   /* Set of variables that this pointer may point to.  */
   bitmap vars;
@@ -130,10 +128,8 @@ extern bool pt_solution_singleton_p (str
 extern bool pt_solution_includes_global (struct pt_solution *);
 extern bool pt_solution_includes (struct pt_solution *, const_tree);
 extern bool pt_solutions_intersect (struct pt_solution *, struct pt_solution 
*);
-extern bool pt_solutions_same_restrict_base (struct pt_solution *,
-struct pt_solution *);
 extern void pt_solution_reset (struct pt_solution *);
-extern void pt_solution_set (struct pt_solution *, bitmap, bool, bool);
+extern void pt_solution_set (struct pt_solution *, bitmap, bool);
 extern void pt_solution_set_var (struct pt_solution *, tree);
 
 extern void dump_pta_stats (FILE *);
Index: gcc/tree-ssa-structalias.c
===
--- 

[Patch,AVR] Housekeeping insn attributes remove assembler dialect

2011-10-17 Thread Georg-Johann Lay
This is more code clean-up for insn attributes.

It removes mcu_have_movw, mcu_mega and defines enabled and isa
attributes instead.

The isa attribute which triggers enabled is a replacement for AVR_HAVE_MOVW
assembler dialect.  We don't actually support assembler dialects but different
ISAs so that an attribute that reflects ISA capabilities seems more appropriate
and straight forward here. Moreover, it's easier to write down different
instruction lengths (which wouldn't occur if it was just an assembler dialect).

The only notable change is to epilogue_restores:

(set (reg:HI REG_Y)
 (plus:HI (reg:HI REG_Y)
  (match_operand:HI 0 immediate_operand i,i)))
-   (set (reg:HI REG_SP)
-(reg:HI REG_Y))
+   (set (reg:HI REG_SP)
+(plus:HI (reg:HI REG_Y)
+ (match_dup 0)))

The original code does not quite represent what the insn does:

A PARALLEL's actions all happen simultaneously, not in sequence.
Thus, an instruction sequence like
Y  = Y + const
SP = Y
has wo be written in RTL as
Y  = Y + const
SP = Y + const
and *not* as
Y  = Y + const
SP = Y

The patch passes without regressions.

The patch passes with 2 more regressions if the test suite is run with
-mcall-prologues, but the two additional run-fails
./gcc.dg/sibcall-3.c
./gcc.dg/sibcall-4.c
are because tail call optimization is turned off at -mcall-prologues,
see avr.c:avr_function_ok_for_sibcall().

Ok for trunk?

Johann

* config/avr/avr.h (ASSEMBLER_DIALECT): Remove.
* config/avr/avr.md (mcu_have_movw, mcu_mega): Remove attributes.
(adjust_len): Add alternative call.
(isa, enabled): New insn attributes.
(length): Use match_test with AVR_HAVE_JMP_CALL instead of
mcu_mega attribute.
(*sbrx_branchmode): Ditto.
(*sbrx_and_branchmode): Ditto.
(*sbix_branch): Ditto.
(*sbix_branch_bit7): Ditto.
(*sbix_branch_tmp): Ditto.
(*sbix_branch_tmp_bit7): Ditto.
(jump): Ditto.
(negsi2): Use attribute isa instead of assembler dialect.
(extendhisi2): Ditto.
(call_insn, call_value_insn): Set adjust_len attribute.
(indirect_jump): Indent to coding rules.
(call_prologue_saves): Use isa attribute instead of mcu_mega.
(epilogue_restores): Ditto.  Fix setting of SP as described in the
RTX pattern.
(*indirect_jump): Fusion of *jcindirect_jump, *njcindirect_jump
and *indirect_jump_avr6.
(*tablejump): Fusion of *tablejump_rjmp and *tablejump_lib.
(*jcindirect_jump, *njcindirect_jump, *indirect_jump_avr6): Remove.
(*tablejump_rjmp, *tablejump_lib): Remove.
* config/avr/avr.c (adjust_insn_length): Handle ADJUST_LEN_CALL.

Index: config/avr/avr.md
===
--- config/avr/avr.md	(revision 180076)
+++ config/avr/avr.md	(working copy)
@@ -84,17 +84,6 @@ (define_attr cc none,set_czn,set_zn,s
 (define_attr type branch,branch1,arith,xcall
   (const_string arith))
 
-(define_attr mcu_have_movw yes,no
-  (const (if_then_else (symbol_ref AVR_HAVE_MOVW)
-		   (const_string yes)
-		   (const_string no
-
-(define_attr mcu_mega yes,no
-  (const (if_then_else (symbol_ref AVR_HAVE_JMP_CALL)
-		   (const_string yes)
-		   (const_string no
-  
-
 ;; The size of instructions in bytes.
 ;; XXX may depend from cc
 
@@ -124,7 +113,7 @@ (define_attr length 
  (const_int 3)
  (const_int 4)))
 	 (eq_attr type xcall)
-	 (if_then_else (eq_attr mcu_mega no)
+	 (if_then_else (match_test !AVR_HAVE_JMP_CALL)
 		   (const_int 1)
 		   (const_int 2))]
 (const_int 2)))
@@ -133,11 +122,10 @@ (define_attr length 
 ;; Following insn attribute tells if and how the adjustment has to be
 ;; done:
 ;; no No adjustment needed; attribute length is fine.
-;; yesAnalyse pattern in adjust_insn_length by hand.
 ;; Otherwise do special processing depending on the attribute.
 
 (define_attr adjust_len
-  out_bitop, out_plus, addto_sp, tsthi, tstsi, compare,
+  out_bitop, out_plus, addto_sp, tsthi, tstsi, compare, call,
mov8, mov16, mov32, reload_in16, reload_in32,
ashlqi, ashrqi, lshrqi,
ashlhi, ashrhi, lshrhi,
@@ -145,6 +133,50 @@ (define_attr adjust_len
no
   (const_string no))
 
+;; Flavours of instruction set architecture (ISA), used in enabled attribute
+
+;; mov:   ISA has no MOVW
+;; movw:  ISA has MOVW
+;; rjmp:  ISA has no CALL/JMP
+;; jmp:   ISA has CALL/JMP
+;; ijmp:  ISA has no EICALL/EIJMP
+;; eijmp: ISA has EICALL/EIJMP
+
+(define_attr isa
+  mov,movw, rjmp,jmp, ijmp,eijmp,
+   standard
+  (const_string standard))
+
+(define_attr enabled 
+  (cond [(eq_attr isa standard)
+ (const_int 1)
+ 
+ (and (eq_attr isa mov)
+  (match_test !AVR_HAVE_MOVW))
+ (const_int 1)
+
+ 

Re: [PATCH, i386 tests] New tests to check vectorization for AVX2 insns.

2011-10-17 Thread Jakub Jelinek
On Mon, Oct 17, 2011 at 06:27:04PM +0400, Kirill Yukhin wrote:
 Thanks for inputs, Jakub!
 
 I am attaching updated patch.
 
 Updated testsuite/ChangeLog entry:
 2011-10-17  Kirill Yukhin  kirill.yuk...@intel.com
 
 * gcc.target/i386/avx2-vpop-check.h: New header.
 * gcc.target/i386/avx2-vpaddd-3.c: New test.
 * gcc.target/i386/avx2-vpaddw-3.c: Ditto.
 * gcc.target/i386/avx2-vpaddb-3.c: Ditto.
 * gcc.target/i386/avx2-vpaddq-3.c: Ditto.
 * gcc.target/i386/avx2-vpand-3.c: Ditto.
 * gcc.target/i386/avx2-vpmulld-3.c: Ditto.
 * gcc.target/i386/avx2-vpmullw-3.c: Ditto.
 * gcc.target/i386/avx2-vpsrad-3.c: Ditto.
 * gcc.target/i386/avx2-vpsraw-3.c: Ditto.
 * gcc.target/i386/avx2-vpsrld-3.c: Ditto.
 * gcc.target/i386/avx2-vpsrlw-3.c: Ditto.
 * gcc.target/i386/avx2-vpsubb-3.c: Ditto.
 * gcc.target/i386/avx2-vpsubd-3.c: Ditto.
 * gcc.target/i386/avx2-vpsubq-3.c: Ditto.
 * gcc.target/i386/avx2-vpsubw-3.c: Ditto.
 
 
 Could you please guys have a look?

This is ok for the trunk.

Jakub


RE: [Patch,AVR] Print no-return functions as JMP

2011-10-17 Thread Paul_Koning
There is no real post morten debugging on AVR as there is nothing like core 
dump.

But if there is any kind of debugging at all, you might use the debugger to put 
a breakpoint in abort(), and if so, having that invoked via jmp means you don't 
know who called it.  So you'd want a way to suppress that optimization.

paul


Re: [patch tree-optimization]: Improve handling of conditional-branches on targets with high branch costs

2011-10-17 Thread Richard Guenther
On Mon, Oct 17, 2011 at 3:36 PM, Kai Tietz ktiet...@googlemail.com wrote:
 Sure,

 Is simplier and also handles (A T[-IF] (B T-IF C) - (A T B) T-IF C
 case, which can happen by framing in conditions.

 @@ -8380,13 +8400,65 @@ fold_truth_andor (location_t loc, enum t
      lhs is another similar operation, try to merge its rhs with our
      rhs.  Then try to merge our lhs and rhs.  */
   if (TREE_CODE (arg0) == code
 -       0 != (tem = fold_truthop (loc, code, type,
 -                                  TREE_OPERAND (arg0, 1), arg1)))
 +       0 != (tem = fold_truth_andor_1 (loc, code, type,
 +                                        TREE_OPERAND (arg0, 1), arg1)))
     return fold_build2_loc (loc, code, type, TREE_OPERAND (arg0, 0), tem);

 -  if ((tem = fold_truthop (loc, code, type, arg0, arg1)) != 0)
 +  if ((tem = fold_truth_andor_1 (loc, code, type, arg0, arg1)) != 0)
     return tem;

 +  if ((BRANCH_COST (optimize_function_for_speed_p (cfun),
 +                   false) = 2)
 +       LOGICAL_OP_NON_SHORT_CIRCUIT)
 +    {
 +      enum tree_code ncode, icode;
 +
 +      ncode = (code == TRUTH_ANDIF_EXPR || code == TRUTH_AND_EXPR)
 +             ? TRUTH_AND_EXPR : TRUTH_OR_EXPR;
 +      icode = ncode == TRUTH_AND_EXPR ? TRUTH_ANDIF_EXPR : TRUTH_ORIF_EXPR;
 +
 +      /* Transform ((A AND-IF B) AND[-IF] C) into (A AND-IF (B AND C)),
 +        or ((A OR-IF B) OR[-IF] C) into (A OR-IF (B OR C))
 +        We don't want to pack more than two leafs to a non-IF AND/OR
 +        expression.
 +        If tree-code of left-hand operand isn't an AND/OR-IF code and not
 +        equal to IF-CODE, then we don't want to add right-hand operand.
 +        If the inner right-hand side of left-hand operand has
 +        side-effects, or isn't simple, then we can't add to it,
 +        as otherwise we might destroy if-sequence.  */
 +      if (TREE_CODE (arg0) == icode
 +          simple_operand_p_2 (arg1)
 +         /* Needed for sequence points to handle trappings, and
 +            side-effects.  */
 +          simple_operand_p_2 (TREE_OPERAND (arg0, 1)))
 +       {
 +         tem = fold_build2_loc (loc, ncode, type, TREE_OPERAND (arg0, 1),
 +                                arg1);
 +         return fold_build2_loc (loc, icode, type, TREE_OPERAND (arg0, 0),
 +                                 tem);
 +       }
 +       /* Same as abouve but for (A AND[-IF] (B AND-IF C)) - ((A AND B) 
 AND-IF C),
 +          or (A OR[-IF] (B OR-IF C) - ((A OR B) OR-IF C).  */
 +      else if (TREE_CODE (arg1) == icode
 +          simple_operand_p_2 (arg0)
 +         /* Needed for sequence points to handle trappings, and
 +            side-effects.  */
 +          simple_operand_p_2 (TREE_OPERAND (arg1, 0)))
 +       {
 +         tem = fold_build2_loc (loc, ncode, type,
 +                                arg0, TREE_OPERAND (arg1, 0));
 +         return fold_build2_loc (loc, icode, type, tem,
 +                                 TREE_OPERAND (arg1, 1));
 +       }
 +      /* Transform (A AND-IF B) into (A AND B), or (A OR-IF B)
 +        into (A OR B).
 +        For sequence point consistancy, we need to check for trapping,
 +        and side-effects.  */
 +      else if (code == icode  simple_operand_p_2 (arg0)
 +                simple_operand_p_2 (arg1))
 +       return fold_build2_loc (loc, ncode, type, arg0, arg1);
 +    }
 +
   return NULL_TREE;
  }

Ok with the rest of the changes I requested.

Richard.

 Regards,
 Kai



FYI: minor fix in gcc/configure

2011-10-17 Thread Tom Tromey
I'm checking this in as obvious.

Sergio pointed out, via this patch, that gcc/configure didn't properly
emit whether sys/std.h was discovered.

Tested by re-running configure and examining the output.

Tom

2011-10-17  Sergio Durigan Junior  sergi...@redhat.com

* configure.ac: Display `yes' if the SystemTap header has been
found.
* configure: Regenerate.

Index: configure.ac
===
--- configure.ac(revision 180092)
+++ configure.ac(working copy)
@@ -4578,6 +4578,7 @@
 AC_MSG_CHECKING(sys/sdt.h in the target C library)
 have_sys_sdt_h=no
 if test -f $target_header_dir/sys/sdt.h; then
+  have_sys_sdt_h=yes
   AC_DEFINE(HAVE_SYS_SDT_H, 1,
 [Define if your target C library provides sys/sdt.h])
 fi


Re: [Patch,AVR] Print no-return functions as JMP

2011-10-17 Thread Georg-Johann Lay
paul_kon...@dell.com schrieb:
 There is no real post morten debugging on AVR as there is nothing like
 core dump.
 
 But if there is any kind of debugging at all, you might use the debugger to
 put a breakpoint in abort(), and if so, having that invoked via jmp means
 you don't know who called it.  So you'd want a way to suppress that
 optimization.
 
 paul

Regarding the number of objections, I think it's best to drop this patch.

The clean-up to the call insns is contained in
http://gcc.gnu.org/ml/gcc-patches/2011-10/msg01530.html

Thanks for all of your feedback!

Johann


Re: [PATCH, i386 tests] New tests to check vectorization for AVX2 insns.

2011-10-17 Thread Kirill Yukhin
Thanks, guys, could anybody please commit that?

K

On Mon, Oct 17, 2011 at 6:33 PM, Jakub Jelinek ja...@redhat.com wrote:
 On Mon, Oct 17, 2011 at 06:27:04PM +0400, Kirill Yukhin wrote:
 Thanks for inputs, Jakub!

 I am attaching updated patch.

 Updated testsuite/ChangeLog entry:
 2011-10-17  Kirill Yukhin  kirill.yuk...@intel.com

         * gcc.target/i386/avx2-vpop-check.h: New header.
         * gcc.target/i386/avx2-vpaddd-3.c: New test.
         * gcc.target/i386/avx2-vpaddw-3.c: Ditto.
         * gcc.target/i386/avx2-vpaddb-3.c: Ditto.
         * gcc.target/i386/avx2-vpaddq-3.c: Ditto.
         * gcc.target/i386/avx2-vpand-3.c: Ditto.
         * gcc.target/i386/avx2-vpmulld-3.c: Ditto.
         * gcc.target/i386/avx2-vpmullw-3.c: Ditto.
         * gcc.target/i386/avx2-vpsrad-3.c: Ditto.
         * gcc.target/i386/avx2-vpsraw-3.c: Ditto.
         * gcc.target/i386/avx2-vpsrld-3.c: Ditto.
         * gcc.target/i386/avx2-vpsrlw-3.c: Ditto.
         * gcc.target/i386/avx2-vpsubb-3.c: Ditto.
         * gcc.target/i386/avx2-vpsubd-3.c: Ditto.
         * gcc.target/i386/avx2-vpsubq-3.c: Ditto.
         * gcc.target/i386/avx2-vpsubw-3.c: Ditto.


 Could you please guys have a look?

 This is ok for the trunk.

        Jakub



[v3] Remove duplicate symbol in gnu.ver (PR bootstrap/50715)

2011-10-17 Thread Rainer Orth
As described in the PR, after Benjamin's patch

2011-10-10  Benjamin Kosnik  b...@redhat.com

PR libstdc++/49818
* config/abi/pre/gnu.ver (CXXABI_1.3.6): Add symbols.

__cxa_get_exception_ptr is now present in both CXXABI_1.3.1 and
CXXABI_1.3.6, which breaks Solaris bootstrap with Sun ld:

ld: fatal: libstdc++-symbols.ver-sun: 5210: symbol '__cxa_get_exception_ptr' is 
already defined in file: libstdc++-symbols.ver-sun: symbol version conflict

The duplicate serves no purpose I can see, and even with both entries,
__cxa_get_exception_ptr is bound to version CXXABI_1.3.1 only on
x86_64-unknown-linux-gnu, so it seems safe to remove.

This restores Solaris bootstrap, tested on i386-pc-solaris2.11.

Ok for mainline?

Rainer


2011-10-14  Rainer Orth  r...@cebitec.uni-bielefeld.de

PR bootstrap/50715
* config/abi/pre/gnu.ver (CXXABI_1.3.6): Remove duplicate
__cxa_get_exception_ptr.

# HG changeset patch
# Parent fbadbe8d19738b0abbefa6cabc32fa04d969b09f
Remove duplicate symbol in gnu.ver (PR bootstrap/50715)

diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver
--- a/libstdc++-v3/config/abi/pre/gnu.ver
+++ b/libstdc++-v3/config/abi/pre/gnu.ver
@@ -1496,7 +1496,6 @@ CXXABI_1.3.6 {
 
 __cxa_allocate_dependent_exception;
 __cxa_free_dependent_exception;
-__cxa_get_exception_ptr;
 __cxa_deleted_virtual;
 
-} CXXABI_1.3.5;
\ No newline at end of file
+} CXXABI_1.3.5;

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


Re: [v3] Remove duplicate symbol in gnu.ver (PR bootstrap/50715)

2011-10-17 Thread Paolo Carlini

On 10/17/2011 05:00 PM, Rainer Orth wrote:

As described in the PR, after Benjamin's patch

2011-10-10  Benjamin Kosnikb...@redhat.com

 PR libstdc++/49818
 * config/abi/pre/gnu.ver (CXXABI_1.3.6): Add symbols.

__cxa_get_exception_ptr is now present in both CXXABI_1.3.1 and
CXXABI_1.3.6, which breaks Solaris bootstrap with Sun ld:

ld: fatal: libstdc++-symbols.ver-sun: 5210: symbol '__cxa_get_exception_ptr' is 
already defined in file: libstdc++-symbols.ver-sun: symbol version conflict

The duplicate serves no purpose I can see, and even with both entries,
__cxa_get_exception_ptr is bound to version CXXABI_1.3.1 only on
x86_64-unknown-linux-gnu, so it seems safe to remove.

This restores Solaris bootstrap, tested on i386-pc-solaris2.11.

Ok for mainline?

Ok, thanks.

Paolo.


Re: [PATCH, alpha, v2]: Fix PR target/50737, FAIL: Throw_3 -O3 execution

2011-10-17 Thread Richard Henderson
On 10/16/2011 01:45 PM, Uros Bizjak wrote:
 libgcc/ChangeLog:
 
 2011-10-16  Uros Bizjak  ubiz...@gmail.com
   Eric Botcazou  ebotca...@adacore.com
 
   PR target/50737
   * config/alpha/linux-unwind.h (alpha_fallback_frame_state): Set
   fs-signal_frame to 1.
 
 libjava/ChangeLog:
 
 2011-10-16  Uros Bizjak  ubiz...@gmail.com
   Eric Botcazou  ebotca...@adacore.com
 
   PR target/50737
   * include/dwarf2-signal.h [__alpha__]: Remove MAKE_THROW_FRAME
   definition.

Ok.


r~


Re: [Patch, Fortran ] PR 50016: Slow Fortran I/O on Windows and flushing/_commit

2011-10-17 Thread Janne Blomqvist
On Mon, Oct 17, 2011 at 15:49, Tobias Burnus bur...@net-b.de wrote:
 This patch adds a call to _commit() on _WIN32 for the FLUSH subroutine and
 the FLUSH statement. It removes the _commit from gfortran's buf_flush.

Like I argued in this message

http://gcc.gnu.org/ml/fortran/2011-10/msg00094.html ,

I think this is a gross mistake. libgfortran should not require
_commit nor fsync in any situation. Those calls are useful for writing
databases and other applications which must make data integrity
guarantees, and are prepared to pay the performance cost associated
with it. It's absolutely not something a language support library
should do unless the language spec explicitly requires such data
integrity guarantees.

 Background:
 * gfortran internally buffers the I/O, but it calls the nonbuffering
 open/write/read functions (and not, e.g., fopen/fwrite/fread). On
 Unix/Darwin/Linux system, the changes become immediately visible to all
 processes after a write().
 * On Windows, there seems to be a file-descriptor specific system buffer
 such that the data only becomes available to other processes after either a
 _commit or after closing the file.
 * The Windows _commit() is a combination of a (system) buffer flush - as a
 write() already implies on POSIX systems, but it also ensures that the
 data ends up on the disk, which on POSIX systems is done via fsync().

I admit I'm somewhat confused by this issue, and despite extensive
googling I haven't been able to come up with a clear explanation for
the behavior seen in PR 44698. That write() would be buffered on
windows makes no sense to me, and I have found no documentation
supporting this view. The closest what I found was in the
documentation for WriteFile and WriteFileEX (which are the win32
native calls, write() is a wrapper around one of these (the EX version
if available, presumably)):

http://msdn.microsoft.com/en-us/library/windows/desktop/aa365748%28v=vs.85%29.aspx

When writing to a file, the last write time is not fully updated
until all handles used for writing have been closed. Therefore, to
ensure an accurate last write time, close the file handle immediately
after writing to the file.

This suggests that metadata updates are not immediately visible to
other processes. Or at least it makes sense that it would update the
size at the same time it updates the last write time. Which would
explain why stat(/path/to/file) would show an old size, as the size
hasn't necessarily yet been updated to the directory, although the
file data itself is already transferred to the OS. And, while I
haven't checked it, it would make sense that fstat(fd,...) would
return the up to date info, as that would just need to check the data
via the process local file descriptor table rather than looking up the
metadata via the directory.

That is, I guess that the implementation is something along the lines
of these calls not being buffered (thus no data lost if the process
crashes after WriteFile(EX)), but the kernel maintains a per-handle
metadata cache which is flushed to the filesystem during batched
metadata updates (and close(), _commit(), or process ending), at which
point it also becomes visible to other processes. Or something like
that.

That being said, this is just me speculating. If someone knows better,
please feel free to share.

And, while I'm at it, this kind of relaxed consistency is not
unheard of in the unix world either. Consider NFS, where data and
metadata may not be flushed to the server until fsync() or close() is
called, or the attribute cache timeout forces the writeout(?), and
thus it's possible for clients to have an inconsistent view of a file.

In both cases the remedy is the same; if this kind of consistency
matters, the user should close the file or fsync()/_commit() before
expecting that the OS metadata is consistent. I think that's a better
option than sprinkling _commit() all over the library.

So I would rather prefer my own patch from the URL above. Also, I
think it would be nice if we could get this fix into 4.6.2..

-- 
Janne Blomqvist


Re: [Patch, Fortran ] PR 50016: Slow Fortran I/O on Windows and flushing/_commit

2011-10-17 Thread Tobias Burnus

Hi Janne,

On 10/17/2011 05:30 PM, Janne Blomqvist wrote:

On Mon, Oct 17, 2011 at 15:49, Tobias Burnusbur...@net-b.de  wrote:

This patch adds a call to _commit() on _WIN32 for the FLUSH subroutine and
the FLUSH statement. It removes the _commit from gfortran's buf_flush.

Like I argued in this message 
http://gcc.gnu.org/ml/fortran/2011-10/msg00094.html, I think this is a gross 
mistake.

[...]

And I think it is a mistake to not make the data available to other 
processes as it is indicated by the Fortran 2008 standard:


Execution of a FLUSH statement causes data written to an external le 
to be available to other processes, or causes data placed in an external 
file by means other than Fortran to be available to a READ statement. 
These actions are processor dependent.


Thus, I think it makes sense for FLUSH to call _commit on Windows.

If you don't want to have a slow down: Simply do not call FLUSH.


libgfortran should not require _commit nor fsync in any situation. Those calls 
are useful for writing databases and other applications which must make data 
integrity guarantees, and are prepared to pay the performance cost associated 
with it. It's absolutely not something a language support library should do 
unless the language spec explicitly requires such data integrity guarantees.


Well, Fortran does not need to write the data to the file, however, the 
purpose of FLUSH is that I can, e.g., run execute_command_line with the 
file the program just has written. It will work on Unix/Linux but not on 
MinGW/MinGW-w64 without a _commit (or without closing the file).



That write() would be buffered on windows makes no sense to me


Why shouldn't it be buffed? Typical Windows programs open files with an 
exclusive lock and as Windows never had the pipes and many small 
programs as Unix did, having a per-file-descriptor buffer is easier to 
implement, avoids multi-thread issues and is potentially faster. If a 
program wants to make the data available, it can just _commit it or 
close the file handle - that way one also has a perfect data integrity.



And, while I'm at it, this kind of relaxed consistency is not
unheard of in the unix world either. Consider NFS, where data and
metadata may not be flushed to the server until fsync() or close() is
called, or the attribute cache timeout forces the writeout(?), and
thus it's possible for clients to have an inconsistent view of a file.


Well, most of the time it works well on the same system: If I call 
execute_command_line, the data is up to date. The issue with NFS only 
occurs if I want to access the data remotely, which is another issue. If 
one wants to do that, one can use a parallel access with, e.g., HDF5 or 
MPIv2 or the Coarray TS (to be written and implemented).



In both cases the remedy is the same; if this kind of consistency matters, the 
user should close the file or fsync()/_commit() before expecting that the OS 
metadata is consistent. I think that's a better option than sprinkling 
_commit() all over the library.


No, for the required consistency, FLUSH is enough (including calling 
_commit on MinGW/MinGW-w64). It makes sure that if the program crashes, 
the data is still there, it makes the data available for other processes.


Only if one wants to have complete integrity, one can call fsync. 
However, with NFS, Lustre et al., I am not 100% sure that the data is 
immediately available on all other clients after fsync returned.



So I would rather prefer my own patch from the URL above. Also, I
think it would be nice if we could get this fix into 4.6.2..


I also would like to see this fixed for 4.6.2. However, a prerequisite 
is that we agree on how to implement it.


Regarding your patch: I think it does not solve the FLUSH issue. For the 
file size itself, I think the patch is okay, but frankly, I think for 
the performance it does not really matter which approach is taken. And I 
do not like the test-suite part of your patch.


Tobias


Re: [Patch, Fortran] PR 47023: [4.6/4.7 regression] C_Sizeof: Rejects valid code

2011-10-17 Thread Janus Weil
 Be quick, though; 4.6.2 was due for release now-ish -

 I know. I'll try to do it today.

Alrighty. Just committed the patch to the 4.6 branch as r180099.


Seems we're down to three open Fortran front-end regressions for the
upcoming 4.6.2 release:

 * http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42954
(TARGET_*_CPP_BUILDINS issues with gfortran)
 * http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50410 (ICE in record_reference)
 * http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50684 (Incorrect error
for move_alloc on element inside intent(in) pointer)

I had started on the third one, but I'm not sure if I can finish it in
time for 4.6.2. Anyone up to looking at the second one?

Cheers,
Janus



 On Sun, Oct 16, 2011 at 9:45 PM, Janus Weil ja...@gcc.gnu.org wrote:
 Hi Paul,

 This is OK for trunk.  Thanks fo rthe patch.

 Thanks. Committed to trunk as r180062. What about 4.6?

 Cheers,
 Janus



 On Sun, Oct 16, 2011 at 2:58 PM, Janus Weil ja...@gcc.gnu.org wrote:
 Hi all,

 here is a patch which fixes the regression in comment #2 of the PR in
 the subject line. What it does is setting the 'ts.is_c_interop' flag
 correctly for constants with kind-parameter specification (such as
 '0.0_c_double'), as is already being done for variables.

 Regtested on x86_64-unknown-linux-gnu. Ok for trunk and 4.6?

 Cheers,
 Janus


 2011-10-16  Janus Weil  ja...@gcc.gnu.org

        PR fortran/47023
        * primary.c (match_kind_param): Detect ISO_C_BINDING kinds.
        (get_kind): Pass on 'is_iso_c' flag.
        
 (match_integer_constant,match_real_constant,match_logical_constant):
        Set 'ts.is_c_interop'.


 2011-10-16  Janus Weil  ja...@gcc.gnu.org

        PR fortran/47023
        * gfortran.dg/c_kind_tests_3.f03: New.




 --
 The knack of flying is learning how to throw yourself at the ground and 
 miss.
        --Hitchhikers Guide to the Galaxy





 --
 The knack of flying is learning how to throw yourself at the ground and miss.
        --Hitchhikers Guide to the Galaxy




Re: [PATCH, ARM] Unaligned accesses for builtin memcpy [2/2]

2011-10-17 Thread Ramana Radhakrishnan
Hi Julian,


There are a couple of minor formatting nits.

+static int
+arm_movmemqi_unaligned (rtx *operands)



+  /* Inlined memcpy using ldr/str/ldrh/strh can be quite big: try to limit
+ size of code if optimizing for size.  We'll use ldm/stm if 
src_aligned
+   or dst_aligned though: allow more interleaving in those cases since the
+   resulting code can be smaller.  */

Watch out the or being misaligned compared to the other text.

+  /* Note that the loop created by arm_block_move_unaligned_loop may be
+ subject to loop unrolling, which makes tuning this condition a little
+   redundant.  */

Same with `redundant'

On 13 October 2011 18:53, Julian Brown jul...@codesourcery.com wrote:
 On Wed, 28 Sep 2011 14:33:17 +0100

 No, sorry, I don't have any benchmark results available at present. I
 think we'd have to have terrifically bad luck for it to be a
 performance degradation, though...

Hmmm OK - but please watch out for any bug reports or any test-suite
fallout this week with
multilibs other than what you might have tested.


 I re-tested the patch for good measure, in case of bitrot (and the new
 tests pass with the patch applied, of course). OK to apply now?

OK by me with those changes.

cheers
Ramana


Re: [Patch,AVR] Housekeeping insn attributes remove assembler dialect

2011-10-17 Thread Denis Chertykov
2011/10/17 Georg-Johann Lay a...@gjlay.de:
 This is more code clean-up for insn attributes.

 It removes mcu_have_movw, mcu_mega and defines enabled and isa
 attributes instead.

 The isa attribute which triggers enabled is a replacement for 
 AVR_HAVE_MOVW
 assembler dialect.  We don't actually support assembler dialects but different
 ISAs so that an attribute that reflects ISA capabilities seems more 
 appropriate
 and straight forward here. Moreover, it's easier to write down different
 instruction lengths (which wouldn't occur if it was just an assembler 
 dialect).

 The only notable change is to epilogue_restores:

    (set (reg:HI REG_Y)
         (plus:HI (reg:HI REG_Y)
                  (match_operand:HI 0 immediate_operand i,i)))
 -   (set (reg:HI REG_SP)
 -        (reg:HI REG_Y))
 +   (set (reg:HI REG_SP)
 +        (plus:HI (reg:HI REG_Y)
 +                 (match_dup 0)))

 The original code does not quite represent what the insn does:

 A PARALLEL's actions all happen simultaneously, not in sequence.
 Thus, an instruction sequence like
    Y  = Y + const
    SP = Y
 has wo be written in RTL as
    Y  = Y + const
    SP = Y + const
 and *not* as
    Y  = Y + const
    SP = Y

 The patch passes without regressions.

 The patch passes with 2 more regressions if the test suite is run with
 -mcall-prologues, but the two additional run-fails
        ./gcc.dg/sibcall-3.c
        ./gcc.dg/sibcall-4.c
 are because tail call optimization is turned off at -mcall-prologues,
 see avr.c:avr_function_ok_for_sibcall().

 Ok for trunk?


Approved.

Denis.


Re: [PATCH] fortran/50514 -- Fix static chekcing of ISHFT[C] arguments.

2011-10-17 Thread Steve Kargl
On Mon, Oct 17, 2011 at 12:22:03PM +0200, Tobias Burnus wrote:
 On 10/15/2011 11:21 PM, Steve Kargl wrote:
 OK for trunk?
 
 2011-10-15  Steven G. Karglka...@gcc.gnu.org
 
  * gfortran.dg/ishft_3.f90:  Update test.
 
 I am not so happy with complete test replacements. How about adding it 
 as new test case?
 

Well, the old testcase is 

%cat ishft_3.f90
! { dg-do compile }
program ishft_3
  integer i, j
  write(*,*) ishftc( 3, 2, 3 )
  write(*,*) ishftc( 3, 2, i )
  write(*,*) ishftc( 3, i, j )
  write(*,*) ishftc( 3, 128 ) ! { dg-error exceeds BIT_SIZE of first }
  write(*,*) ishftc( 3, 0, 128 )  ! { dg-error exceeds BIT_SIZE of first }
  write(*,*) ishftc( 3, 0, 0 )! { dg-error Invalid third argument }
  write(*,*) ishftc( 3, 3, 2 )! { dg-error exceeds third argument }
end program

1) It's misnamed.  There is no testing of ishft.
2) i and j are undefined, so lines 4 and 5 are invalid.
   Even if i and j were defined, there is nothing special
   about those lines in that gfortran generates a function
   call to a runtime routine.  Note, this a dg-do compile
   test.
3) The four dg-error strings would need to be updated to
   the new error messages.

The only line that would survive is the first line, which
is covered in the updated testcase.

 2011-10-15  Steven G. Karglka...@gcc.gnu.org
 
  * check.c (less_than_bitsize1): Check|shift|  = bit_size(i).
  (gfc_check_ishftc):  Check|shift|  = bit_size(i) and check
  that size is positive.
 
 I somehow find less_than_bitsize1's
 
 +  if (strncmp (arg2, ISHFT, 5) == 0)
 
 not that elegant and would prefer another argument, which tells the 
 function that it should take the absolute value of the argument; 
 however, given that ISHFT seems to be the only function which allows 
 negative arguments, one could also leave it.
 

In looking at the other uses of less_than_bitsize1() I
could pass arg2=NULL for ISHFT[C], and then the code
would become

 if (arg2 == NULL) { /* Special case for ISHFT[C].  */

Would that be better?

-- 
Steve


Re: [PATCH 3/6] Emit macro expansion related diagnostics

2011-10-17 Thread H.J. Lu
On Mon, Oct 17, 2011 at 6:41 AM, Dodji Seketeli do...@redhat.com wrote:
 Finally here is what I am checking in, which passes bootstrap with
 --disable-checking --enable-languages=all,ada -- modulo this other bug
 that breaks bootstrap as well
 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50709.

 It's been OKed off-line by Tom Tromey.

 commit c1cd2be336ceec75cf40ac5f32cc4f72b8fc5da3
 Author: Dodji Seketeli do...@redhat.com
 Date:   Mon Oct 17 13:33:41 2011 +0200

    Fix bootstrapping with --disable-checking

    libcpp/ChangeLog

        * line-map.c (linemap_macro_map_loc_to_exp_point): Avoid setting a
        variable without using it if ENABLE_CHECKING is not defined.  Mark
        the LOCATION parameter as being unused.


There are at least 2 bootstrap problems:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50760


-- 
H.J.


[PATCH] Handle USING_MADVISE without USING_MMAP

2011-10-17 Thread Andi Kleen

I plan to check in the following patch as obvious after it passed
testing. 

cygwin has a MADV_DONTNEED, but does not use mmap. The ifdefs for
madvise assumed this wouldn't happen and it broke the cygwin build.

Just don't set USING_MADVISE when USING_MMAP is not set. Thanks
to Kai Titz for testing.

-Andi

2011-10-17  Andi Kleen  a...@linux.intel.com

   * ggc-page.c (USING_MADVISE): Adjust ifdef to check for
   USING_MMAP.

diff --git a/gcc/ggc-page.c b/gcc/ggc-page.c
index 9b35291..2da99db 100644
--- a/gcc/ggc-page.c
+++ b/gcc/ggc-page.c
@@ -50,7 +50,7 @@ along with GCC; see the file COPYING3.  If not see
 #define USING_MALLOC_PAGE_GROUPS
 #endif
 
-#if defined(HAVE_MADVISE)  defined(MADV_DONTNEED)
+#if defined(HAVE_MADVISE)  defined(MADV_DONTNEED)  defined(USING_MMAP)
 # define USING_MADVISE
 #endif
 
-- 
1.7.5.4

-- 
a...@linux.intel.com -- Speaking for myself only.


Re: [C++ Patch] PR 32614

2011-10-17 Thread Jason Merrill

On 10/17/2011 07:48 AM, Paolo Carlini wrote:

And please help
re-assessing the situation wrt the other front-ends *today* not in the
stone age.


EDG always wraps diagnostics at ~80 columns.

clang wraps diagnostics at $COLUMNS when stderr is going to a terminal, 
and doesn't wrap otherwise.


The clang behavior seems like the right way to go.

Jason


[C++ Patch] PR 50757

2011-10-17 Thread Paolo Carlini

Hi,

exactly like the recently fixed c++/17212. Tested x86_64-linux.

Ok for mainline?

Thanks,
Paolo.

/
/gcc
2011-10-17  Paolo Carlini  paolo.carl...@oracle.com

PR c++/50757
* c-family/c.opt ([Wnonnull]): Add C++ and Objective-C++.
* doc/invoke.texi: Update.

/testsuite
2011-10-17  Paolo Carlini  paolo.carl...@oracle.com

PR c++/50757
* g++.dg/warn/format7.C: New.
* obj-c++.dg/warn7.mm: Likewise.
Index: doc/invoke.texi
===
--- doc/invoke.texi (revision 180100)
+++ doc/invoke.texi (working copy)
@@ -3223,7 +3222,7 @@ Enable @option{-Wformat} plus format checks not in
 @option{-Wformat}.  Currently equivalent to @samp{-Wformat
 -Wformat-nonliteral -Wformat-security -Wformat-y2k}.
 
-@item -Wnonnull @r{(C and Objective-C only)}
+@item -Wnonnull
 @opindex Wnonnull
 @opindex Wno-nonnull
 Warn about passing a null pointer for arguments marked as
Index: c-family/c.opt
===
--- c-family/c.opt  (revision 180100)
+++ c-family/c.opt  (working copy)
@@ -510,7 +510,7 @@ C++ ObjC++ Var(warn_nonvdtor) Warning
 Warn about non-virtual destructors
 
 Wnonnull
-C ObjC Var(warn_nonnull) Warning
+C ObjC C++ ObjC++ Var(warn_nonnull) Warning
 Warn about NULL being passed to argument slots marked as requiring non-NULL
 
 Wnormalized=
Index: testsuite/g++.dg/warn/format7.C
===
--- testsuite/g++.dg/warn/format7.C (revision 0)
+++ testsuite/g++.dg/warn/format7.C (revision 0)
@@ -0,0 +1,10 @@
+// PR c++/50757
+// { dg-options -Wformat -Wno-nonnull }
+
+extern void *f (void *__s) __attribute__ ((__nonnull__ (1)));
+
+int main()
+{
+  void* const s = 0;
+  f(s);
+}
Index: testsuite/obj-c++.dg/warn7.mm
===
--- testsuite/obj-c++.dg/warn7.mm   (revision 0)
+++ testsuite/obj-c++.dg/warn7.mm   (revision 0)
@@ -0,0 +1,10 @@
+// PR c++/50757
+// { dg-options -Wformat -Wno-nonnull }
+
+extern void *f (void *__s) __attribute__ ((__nonnull__ (1)));
+
+int main()
+{
+  void* const s = 0;
+  f(s);
+}


Re: [C++ Patch] PR 32614

2011-10-17 Thread Paolo Carlini

On 10/17/2011 07:31 PM, Jason Merrill wrote:
clang wraps diagnostics at $COLUMNS when stderr is going to a 
terminal, and doesn't wrap otherwise.


The clang behavior seems like the right way to go.

Thanks Jason. I'll see how to implement this.

Paolo.


Re: [C++ Patch] PR 50757

2011-10-17 Thread Jason Merrill

OK.

Jason


Re: [C++ Patch] PR 44524

2011-10-17 Thread Jason Merrill

OK.

Jason


  1   2   >