Re: [PATCH] arm.md: Use the revsh and rev16 instructions for BSWAP:HI RTL in armv6 and up
On 16 April 2012 20:35, Louis Kruger lou...@gmail.com wrote: This patch teaches the arm target about the rev16 and revsh instructions which can efficiently execute the recently added __builtin_bswap16() function (BSWAP:HI in RTL) These instructions are available in thumb and arm modes on armv6 architecture and up Thank you for your patch but before taking this further , I have to ask if you have a valid copyright assignment on file with the FSF .I would like to also see some testcases for this feature added to the regression testsuite in gcc.target/arm. regards, Ramana
Re: [Patch, libfortran] Fix handling of temporary files
On Thu, Apr 19, 2012 at 01:45, Manfred Schwarb manfre...@gmx.ch wrote:
Hi Janne,
- If the program is privileged, we shouldn't trust path style
environment variables. The patch fixes this for TMPDIR and also for
the logic figuring out where addr2line is.
I did not test it, but if I remember right, the use of geteuid() and friends
does prevent static compilation, resp. static compilation does seem
to work, but it needs a matched dynamic library nonetheless,
which means if you relocate your statically linked program to another
box with differing glibc, you get runtime errors?
Or is the use of static programs already broken so it does not matter?
If this security feature would prevent use of static programs, it would
not be worth it, I think.
getuid and friends are already used in other parts of the runtime
library, so if static linking to glibc is broken it was broken before
as well.
The Glibc FAQ says:
2.22. Even statically linked programs need some shared libraries
which is not acceptable for me. What can I do?
{AJ} NSS (for details just type `info libc Name Service Switch') won't
work properly without shared libraries. NSS allows using different services
(e.g. NIS, files, db, hesiod) by just changing one configuration file
(/etc/nsswitch.conf) without relinking any programs. The only disadvantage
is that now static libraries need to access shared libraries. This is
handled transparently by the GNU C library.
I'm not sure of the details of what happens, or under which exact
circumstances it works or not, when one runs a statically linked
program on another machine with a different glibc version.
As an aside, on targets with glibc the patch uses __secure_getenv
rather than directly calling the getuid etc. functions. But that
essentially just moves the problem into glibc so I'm not sure if the
end result is any different.
Also, it seems Solaris no longer ships a static libc at all:
https://blogs.oracle.com/rie/entry/static_linking_where_did_it
Anyways, I'd rather keep this security feature as currently this
should be easy to exploit, making linking to libgfortran from any
SUID/SGID binary quite dangerous (I don't know of any such program,
but you never know what crazy things people come up with..).
--
Janne Blomqvist
[PATCH] fix libstdc++/52604 : __freelist::~__freelist re-initializes _M_thread_freelist
All,
The attached patch re-initializes _M_thread_freelist in
__freelist::~__freelist just after calling
::operator delete(static_castvoid*(_M_thread_freelist_array));
It avoids valgrind errors in __pooltrue::_M_get_thread_id() later invoked.
Testcases :
ext/mt_allocator/deallocate_global_thread-1.cc
ext/mt_allocator/deallocate_global_thread-3.cc
Tested on x86_64-linux (make check, make check-performance).
Laurent
PS : Already approved by Paolo Carlini under the 52604 discussions.
Index: src/c++98/mt_allocator.cc
===
--- src/c++98/mt_allocator.cc(revision 186374)
+++ src/c++98/mt_allocator.cc(working copy)
@@ -48,6 +48,7 @@
{
__gthread_key_delete(_M_key);
::operator delete(static_castvoid*(_M_thread_freelist_array));
+ _M_thread_freelist = 0;
}
}
};
Re: [PATCH][2/2] Fix PR44688, use the recorded maximum iterations in the unroller
On Wed, 18 Apr 2012, Richard Guenther wrote:
This teaches loop-iv (RTL IV analysis) about loop-nb_iterations_upper_bound.
It avoids poinless unrolling of vectorizer prologue loops for now.
Bootstrap and regtest pending on x86_64-unknown-linux-gnu.
Finished. Needs the following testsuite adjustment
* gcc.dg/var-expand1.c: Increase array size to make unrolling
possibly profitable.
Index: gcc/testsuite/gcc.dg/var-expand1.c
===
--- gcc/testsuite/gcc.dg/var-expand1.c.orig 2012-04-19
10:45:32.0 +0200
+++ gcc/testsuite/gcc.dg/var-expand1.c 2012-04-19 10:45:43.475201279
+0200
@@ -6,7 +6,7 @@
extern void abort (void);
-float array[10] = { 1,2,3,4,5,6,7,8,9,10 };
+float array[30] = { 1,2,3,4,5,6,7,8,9,10 };
int foo (int n)
{
because otherwise we see that the loop rolls at most 10 times
and thus unrolling by a factor of 8 is pointless (well, yes, we
are somewhat inflexible with the amount of unrolling here).
Committed.
Richard.
Richard.
2012-04-18 Richard Guenther rguent...@suse.de
PR rtl-optimization/44688
* loop-iv.c (determine_max_iter): Only return max_iter.
(iv_number_of_iterations): Also use the recorded loop bound
on the maximum number of iterations.
* loop-unroll.c (decide_unroll_runtime_iterations): Use
max_iter to avoid unrolling loops that do not roll.
(decide_unroll_stupid): Likewise.
Index: gcc/loop-iv.c
===
*** gcc/loop-iv.c.orig2012-04-18 15:29:39.0 +0200
--- gcc/loop-iv.c 2012-04-18 16:03:21.91147 +0200
*** canonicalize_iv_subregs (struct rtx_iv *
*** 2190,2197
return true;
}
! /* Tries to estimate the maximum number of iterations in LOOP, and store the
!result in DESC. This function is called from iv_number_of_iterations
with
a number of fields in DESC already filled in. OLD_NITER is the original
expression for the number of iterations, before we tried to simplify it.
*/
--- 2190,2197
return true;
}
! /* Tries to estimate the maximum number of iterations in LOOP, and return
the
!result. This function is called from iv_number_of_iterations with
a number of fields in DESC already filled in. OLD_NITER is the original
expression for the number of iterations, before we tried to simplify it.
*/
*** determine_max_iter (struct loop *loop, s
*** 2207,2216
{
nmax = INTVAL (XEXP (niter, 0));
if (!(nmax (nmax + 1)))
! {
! desc-niter_max = nmax;
! return nmax;
! }
}
get_mode_bounds (desc-mode, desc-signed_p, desc-mode, mmin, mmax);
--- 2207,2213
{
nmax = INTVAL (XEXP (niter, 0));
if (!(nmax (nmax + 1)))
! return nmax;
}
get_mode_bounds (desc-mode, desc-signed_p, desc-mode, mmin, mmax);
*** determine_max_iter (struct loop *loop, s
*** 2219,2228
if (GET_CODE (niter) == UDIV)
{
if (!CONST_INT_P (XEXP (niter, 1)))
! {
! desc-niter_max = nmax;
! return nmax;
! }
inc = INTVAL (XEXP (niter, 1));
niter = XEXP (niter, 0);
}
--- 2216,
if (GET_CODE (niter) == UDIV)
{
if (!CONST_INT_P (XEXP (niter, 1)))
! return nmax;
inc = INTVAL (XEXP (niter, 1));
niter = XEXP (niter, 0);
}
*** determine_max_iter (struct loop *loop, s
*** 2241,2247
if (dump_file)
fprintf (dump_file, ;; improved upper bound by one.\n);
}
- desc-niter_max = nmax / inc;
return nmax / inc;
}
--- 2235,2240
*** iv_number_of_iterations (struct loop *lo
*** 2259,2265
enum rtx_code cond;
enum machine_mode mode, comp_mode;
rtx mmin, mmax, mode_mmin, mode_mmax;
! unsigned HOST_WIDEST_INT s, size, d, inv;
HOST_WIDEST_INT up, down, inc, step_val;
int was_sharp = false;
rtx old_niter;
--- 2252,2258
enum rtx_code cond;
enum machine_mode mode, comp_mode;
rtx mmin, mmax, mode_mmin, mode_mmax;
! unsigned HOST_WIDEST_INT s, size, d, inv, max;
HOST_WIDEST_INT up, down, inc, step_val;
int was_sharp = false;
rtx old_niter;
*** iv_number_of_iterations (struct loop *lo
*** 2279,2284
--- 2272,2280
desc-const_iter = false;
desc-niter_expr = NULL_RTX;
desc-niter_max = 0;
+ if (loop-any_upper_bound
+double_int_fits_in_uhwi_p (loop-nb_iterations_upper_bound))
+ desc-niter_max = loop-nb_iterations_upper_bound.low;
cond = GET_CODE (condition);
gcc_assert (COMPARISON_P (condition));
*** iv_number_of_iterations (struct loop *lo
*** 2547,2553
down = INTVAL (CONST_INT_P (iv0.base)
Re: [Patch, libfortran] Fix handling of temporary files
Am 19.04.2012 09:25, schrieb Janne Blomqvist:
On Thu, Apr 19, 2012 at 01:45, Manfred Schwarbmanfre...@gmx.ch wrote:
Hi Janne,
- If the program is privileged, we shouldn't trust path style
environment variables. The patch fixes this for TMPDIR and also for
the logic figuring out where addr2line is.
I did not test it, but if I remember right, the use of geteuid() and friends
does prevent static compilation, resp. static compilation does seem
to work, but it needs a matched dynamic library nonetheless,
which means if you relocate your statically linked program to another
box with differing glibc, you get runtime errors?
Or is the use of static programs already broken so it does not matter?
If this security feature would prevent use of static programs, it would
not be worth it, I think.
getuid and friends are already used in other parts of the runtime
library, so if static linking to glibc is broken it was broken before
as well.
The Glibc FAQ says:
2.22. Even statically linked programs need some shared libraries
which is not acceptable for me. What can I do?
{AJ} NSS (for details just type `info libc Name Service Switch') won't
work properly without shared libraries. NSS allows using different services
(e.g. NIS, files, db, hesiod) by just changing one configuration file
(/etc/nsswitch.conf) without relinking any programs. The only disadvantage
is that now static libraries need to access shared libraries. This is
handled transparently by the GNU C library.
I'm not sure of the details of what happens, or under which exact
circumstances it works or not, when one runs a statically linked
program on another machine with a different glibc version.
The nasty thing is, the runtime error does not happen at program start, but
at the point of actual invocation of the *uid call. So this may come as
a real surprise for the user if such functions are seldom called in the
binary, e.g. in some error path.
And the really ugly thing is, there seems to be an incompatible break between
glibc 2.11 and 2.12 in this very NSS stuff.
I recently stumbled over https://bugs.gentoo.org/show_bug.cgi?id=332927, when
the provider decided to upgrade glibc on the server (sic!).
[ And, it was a c-program that was crashing, my static fortran programs were
still running fine ]
As traditionally a lot of fortran code is statically compiled, I just wanted
to make aware that the more of such calls are added, the more likely users will
get surprises.
I guess relocation of binaries to other boxes is not uncommon in the HPC sector,
and having a crash after some weeks of runtime due to this is not really
funny...
Manfred
Re: PING: [PATCH] Fix PRs c/52283/37985
On 04/18/2012 11:51 AM, Richard Guenther wrote: On Wed, Apr 18, 2012 at 11:06 AM, Manuel López-Ibáñez lopeziba...@gmail.com wrote: On 18 April 2012 10:29, Christian Bruel christian.br...@st.com wrote: Is it OK for trunk, bootstrapped and regtested on x86 I think Joseph Myers is on vacation, and there are no other C FE reviewers, but since this is c-common and convert.c, perhaps Jason and/or Richard can review it? The patch is ok if you put the PR52283 properly into a separate testcase, not by amending gcc.dg/case-const-2.c. Thanks, done at rev #186586. with this change. Thanks, Richard. Thanks, Manuel.
Re: PING: [PATCH] Fix PRs c/52283/37985
On 19 April 2012 11:11, Christian Bruel christian.br...@st.com wrote: On 04/18/2012 11:51 AM, Richard Guenther wrote: On Wed, Apr 18, 2012 at 11:06 AM, Manuel López-Ibáñez lopeziba...@gmail.com wrote: On 18 April 2012 10:29, Christian Bruel christian.br...@st.com wrote: Is it OK for trunk, bootstrapped and regtested on x86 I think Joseph Myers is on vacation, and there are no other C FE reviewers, but since this is c-common and convert.c, perhaps Jason and/or Richard can review it? The patch is ok if you put the PR52283 properly into a separate testcase, not by amending gcc.dg/case-const-2.c. Thanks, done at rev #186586. with this change. Great! Just a minor nit, for future patches. There is the unwritten rule of adding the Changelogs to the commit log, like follows: 2012-04-19 Christian Bruel christian.br...@st.com Manuel López-Ibáñez m...@gcc.gnu.org PR c/52283 PR c/37985 * stmt.c (warn_if_unused_value): Skip NOP_EXPR. * convert.c (convert_to_integer): Don't set TREE_NO_WARNING. testsuite/ * gcc.dg/pr52283.c: New test. * gcc.dg/pr37985.c: New test. Also, if you add to the commit logs the PR numbers like: PR whatever/NUMBER PR whatever2/NUMBER then bugzilla will show the commit on each PR. Cheers, Manuel.
Re: PowerPC prologue and epilogue 5
On Wed, Apr 18, 2012 at 12:45:16AM +0930, Alan Modra wrote: This enables out-of-line save and restore for large frames, and for ABI_AIX when using the static chain. Further testing revealed two problems when compiling nested functions. 1) The logic I had for cr_save_regno is wrong, resulting in one of my NOT_INUSE asserts triggering. 2) In some cases the prologue uses in-line saves while the epilogue uses out-of-line restores. This can lead to restoring regs that haven't been saved. Patch withdrawn. -- Alan Modra Australia Development Lab, IBM
[PATCH] genmodes: remove misleading use of strncpy
Found by inspection.
Seeing this strncpy use without the usual following NUL-termination,
my reflex was that it was a buffer overrun candidate, but then I
realized this is gcc, so that's not very likely.
Looking a little harder, I saw the preceding strlen = sizeof buf test,
which means there is no risk of overrun.
That preceding test means the use of strncpy is misleading, since
with that there is no risk of the name being too long for the buffer,
and hence no reason to use strncpy.
One way to make the code clearer is to use strcpy, as done below.
An alternative is to use memcpy (buf, m-name, strlen (m-name) + 1).
Ok to commit?
2012-04-19 Jim Meyering meyer...@redhat.com
genmodes: remove misleading use of strncpy
* genmodes.c (make_complex_modes): Avoid unnecessary use of strncpy.
We verified above that the string(including trailing NUL) fits in buf,
so just use strcpy.
diff --git a/gcc/genmodes.c b/gcc/genmodes.c
index 8b6f5bc..f786258 100644
--- a/gcc/genmodes.c
+++ b/gcc/genmodes.c
@@ -1,15 +1,15 @@
/* Generate the machine mode enumeration and associated tables.
- Copyright (C) 2003, 2004, 2005, 2006, 2007, 2010
+ Copyright (C) 2003, 2004, 2005, 2006, 2007, 2010, 2012
Free Software Foundation, Inc.
This file is part of GCC.
GCC is free software; you can redistribute it and/or modify it under
the terms of the GNU General Public License as published by the Free
Software Foundation; either version 3, or (at your option) any later
version.
GCC is distributed in the hope that it will be useful, but WITHOUT ANY
WARRANTY; without even the implied warranty of MERCHANTABILITY or
FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
for more details.
@@ -442,27 +442,27 @@ make_complex_modes (enum mode_class cl,
if (strlen (m-name) = sizeof buf)
{
error (%s:%d:mode name \%s\ is too long,
m-file, m-line, m-name);
continue;
}
/* Float complex modes are named SCmode, etc.
Int complex modes are named CSImode, etc.
This inconsistency should be eliminated. */
if (cl == MODE_FLOAT)
{
char *p, *q = 0;
- strncpy (buf, m-name, sizeof buf);
+ strcpy (buf, m-name);
p = strchr (buf, 'F');
if (p == 0)
q = strchr (buf, 'D');
if (p == 0 q == 0)
{
error (%s:%d: float mode \%s\ has no 'F' or 'D',
m-file, m-line, m-name);
continue;
}
if (p != 0)
*p = 'C';
else
--
1.7.10.208.gb4267
[Patch ARM] Fix name of atomic expander for ior
Hi, Following Uros's mail here http://gcc.gnu.org/ml/gcc/2012-04/msg00679.html, I've noticed that ARM is also an affected target. Applied to trunk after testing with a new enough gdb, will be applied to 4.7 once testing completes there. regards, Ramana 2012-04-19 Ramana Radhakrishnan ramana.radhakrish...@linaro.org * config/arm/sync.md (sync_optab): Change ior attribute to or. diff --git a/gcc/config/arm/sync.md b/gcc/config/arm/sync.md index 96de0f3..03838f5 100644 --- a/gcc/config/arm/sync.md +++ b/gcc/config/arm/sync.md @@ -29,7 +29,7 @@ (define_code_iterator syncop [plus minus ior xor and]) (define_code_attr sync_optab - [(ior ior) (xor xor) (and and) (plus add) (minus sub)]) + [(ior or) (xor xor) (and and) (plus add) (minus sub)]) (define_mode_attr sync_sfx [(QI b) (HI h) (SI ) (DI d)])
[Patch,AVR,committed]: Fix PR53033
Fixed this typo in PSImode reads via *(X+const) http://gcc.gnu.org/viewcvs?view=revisionrevision=186588 http://gcc.gnu.org/viewcvs?view=revisionrevision=186589 Committed as obvious. Johann PR target/53033 * config/avr/avr.c (avr_out_load_psi): Fix assembler template for the case *(X+const).
Re: [Patch, libfortran] Fix handling of temporary files
On Thu, Apr 19, 2012 at 11:52, Manfred Schwarb manfre...@gmx.ch wrote:
Am 19.04.2012 09:25, schrieb Janne Blomqvist:
On Thu, Apr 19, 2012 at 01:45, Manfred Schwarbmanfre...@gmx.ch wrote:
Hi Janne,
- If the program is privileged, we shouldn't trust path style
environment variables. The patch fixes this for TMPDIR and also for
the logic figuring out where addr2line is.
I did not test it, but if I remember right, the use of geteuid() and
friends
does prevent static compilation, resp. static compilation does seem
to work, but it needs a matched dynamic library nonetheless,
which means if you relocate your statically linked program to another
box with differing glibc, you get runtime errors?
Or is the use of static programs already broken so it does not matter?
If this security feature would prevent use of static programs, it would
not be worth it, I think.
getuid and friends are already used in other parts of the runtime
library, so if static linking to glibc is broken it was broken before
as well.
The Glibc FAQ says:
2.22. Even statically linked programs need some shared libraries
which is not acceptable for me. What can I do?
{AJ} NSS (for details just type `info libc Name Service Switch') won't
work properly without shared libraries. NSS allows using different
services
(e.g. NIS, files, db, hesiod) by just changing one configuration file
(/etc/nsswitch.conf) without relinking any programs. The only
disadvantage
is that now static libraries need to access shared libraries. This is
handled transparently by the GNU C library.
I'm not sure of the details of what happens, or under which exact
circumstances it works or not, when one runs a statically linked
program on another machine with a different glibc version.
The nasty thing is, the runtime error does not happen at program start, but
at the point of actual invocation of the *uid call. So this may come as
a real surprise for the user if such functions are seldom called in the
binary, e.g. in some error path.
And the really ugly thing is, there seems to be an incompatible break
between
glibc 2.11 and 2.12 in this very NSS stuff.
I recently stumbled over https://bugs.gentoo.org/show_bug.cgi?id=332927,
when
the provider decided to upgrade glibc on the server (sic!).
[ And, it was a c-program that was crashing, my static fortran programs were
still running fine ]
As traditionally a lot of fortran code is statically compiled, I just wanted
to make aware that the more of such calls are added, the more likely users
will
get surprises.
I guess relocation of binaries to other boxes is not uncommon in the HPC
sector,
and having a crash after some weeks of runtime due to this is not really
funny...
That shouldn't be an issue then as with the patch __secure_getenv is
called during library initialization, so if it's going to crash it
will crash immediately.
Also, I suspect getuid and friends are not actually part of the NSS
stuff. The uid/gid/euid/egid are maintained for each process by the
kernel, and NSS only handles translation to/from the numeric id's to
string values, IP address lookup and so forth.
--
Janne Blomqvist
Re: [PATCH] genmodes: remove misleading use of strncpy
On Thu, Apr 19, 2012 at 12:42 PM, Jim Meyering j...@meyering.net wrote:
Found by inspection.
Seeing this strncpy use without the usual following NUL-termination,
my reflex was that it was a buffer overrun candidate, but then I
realized this is gcc, so that's not very likely.
Looking a little harder, I saw the preceding strlen = sizeof buf test,
which means there is no risk of overrun.
That preceding test means the use of strncpy is misleading, since
with that there is no risk of the name being too long for the buffer,
and hence no reason to use strncpy.
One way to make the code clearer is to use strcpy, as done below.
An alternative is to use memcpy (buf, m-name, strlen (m-name) + 1).
Ok to commit?
Without a comment before the strcpy this isn't any more clear than
when using strncpy. Also your issue with the terminating NUL is
still present for the snprintf call done later (in fact the code looks like
it can be optimized, avoiding the strcpy in the path to the snprintf).
strncpy - strcpy definitely looks like premature optimization to me.
Richard.
2012-04-19 Jim Meyering meyer...@redhat.com
genmodes: remove misleading use of strncpy
* genmodes.c (make_complex_modes): Avoid unnecessary use of strncpy.
We verified above that the string(including trailing NUL) fits in buf,
so just use strcpy.
diff --git a/gcc/genmodes.c b/gcc/genmodes.c
index 8b6f5bc..f786258 100644
--- a/gcc/genmodes.c
+++ b/gcc/genmodes.c
@@ -1,15 +1,15 @@
/* Generate the machine mode enumeration and associated tables.
- Copyright (C) 2003, 2004, 2005, 2006, 2007, 2010
+ Copyright (C) 2003, 2004, 2005, 2006, 2007, 2010, 2012
Free Software Foundation, Inc.
This file is part of GCC.
GCC is free software; you can redistribute it and/or modify it under
the terms of the GNU General Public License as published by the Free
Software Foundation; either version 3, or (at your option) any later
version.
GCC is distributed in the hope that it will be useful, but WITHOUT ANY
WARRANTY; without even the implied warranty of MERCHANTABILITY or
FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
for more details.
@@ -442,27 +442,27 @@ make_complex_modes (enum mode_class cl,
if (strlen (m-name) = sizeof buf)
{
error (%s:%d:mode name \%s\ is too long,
m-file, m-line, m-name);
continue;
}
/* Float complex modes are named SCmode, etc.
Int complex modes are named CSImode, etc.
This inconsistency should be eliminated. */
if (cl == MODE_FLOAT)
{
char *p, *q = 0;
- strncpy (buf, m-name, sizeof buf);
+ strcpy (buf, m-name);
p = strchr (buf, 'F');
if (p == 0)
q = strchr (buf, 'D');
if (p == 0 q == 0)
{
error (%s:%d: float mode \%s\ has no 'F' or 'D',
m-file, m-line, m-name);
continue;
}
if (p != 0)
*p = 'C';
else
--
1.7.10.208.gb4267
Re: [Patch, Fortran] PR 52196 add -Wrealloc-lhs(-all)
Updated patch enclosed.
On 02/14/2012 12:42 PM, Tobias Burnus wrote:
in order to gain an overview for our code whether the recent RESHAPE
(and friends) bug affects us and to determine for which assignment a
reallocation happens, useful to mitigate performance issues, I added
-Wrealloc-lhs and -Wrealloc-lhs-all.
The flag -Wrealloc-lhs is the more useful flag: It's about arrays of
intrinsic types, which are more likely to appear in hot loops than
other types of reallocatable variables such as derived types or
(scalar) character variables with deferred length.
On 02/27/2012 09:59 PM, Mikael Morin wrote:
In turn, the warning might be printed even if at the end no realloc code is
generated or present with -O1.
Can it be caused by the frontend not going in the realloc-lhs functions in
some cases? Especially, it seems that there is a missing codimension/coindexed
condition guarding the warning if I compare to the flag_realloc_lhs conditions
in trans-expr.c I would rather move the warnings to a function and call it in
the places where we really generate the extra code, like it's done for
-Warray-temporaries.
Two months later I have finally worked on the patch again and followed
the suggestion and added the checks to trans-expr.c.
Build and regtested on x86-64-linux.
OK for the trunk?
Tobias
PS: If you wonder about the added expr2-rank in gfc_trans_assignment_1:
That check is also in gfc_alloc_allocatable_for_assignment. The only
difference should be the ompws_flags value. [Recall that array =
scalar does not cause a reallocation.]
2012-04-19 Tobias Burnus bur...@net-b.de
PR fortran/52196
* lang.opt (Wrealloc-lhs, Wrealloc-lhs-all): New flags.
* gfortran.h (gfc_option_t): Add them.
* options.c (gfc_init_options, gfc_post_options,
gfc_handle_option): Handle them.
* invoke.texi: Document them.
* trans-expr.c (realloc_lhs_warning): New function.
(gfc_trans_arrayfunc_assign,
alloc_scalar_allocatable_for_assignment,
gfc_trans_assignment_1): Use it.
2012-04-19 Tobias Burnus bur...@net-b.de
PR fortran/52196
* gfortran.dg/realloc_on_assign_14.f90: New.
Index: gcc/fortran/gfortran.h
===
--- gcc/fortran/gfortran.h (revision 186584)
+++ gcc/fortran/gfortran.h (working copy)
@@ -2219,6 +2219,8 @@ typedef struct
int warn_align_commons;
int warn_real_q_constant;
int warn_unused_dummy_argument;
+ int warn_realloc_lhs;
+ int warn_realloc_lhs_all;
int max_errors;
int flag_all_intrinsics;
Index: gcc/fortran/invoke.texi
===
--- gcc/fortran/invoke.texi (revision 186584)
+++ gcc/fortran/invoke.texi (working copy)
@@ -146,9 +146,8 @@ and warnings}.
-Wconversion -Wfunction-elimination -Wimplicit-interface @gol
-Wimplicit-procedure -Wintrinsic-shadow -Wintrinsics-std @gol
-Wline-truncation -Wno-align-commons -Wno-tabs -Wreal-q-constant @gol
--Wsurprising -Wunderflow -Wunused-parameter -fmax-errors=@var{n}
--fsyntax-only @gol
--pedantic -pedantic-errors
+-Wsurprising -Wunderflow -Wunused-parameter -Wrealloc-lhs Wrealloc-lhs-all @gol
+-fmax-errors=@var{n} -fsyntax-only -pedantic -pedantic-errors
}
@item Debugging Options
@@ -919,7 +918,24 @@ off via @option{-Wno-align-commons}. See also @opt
Warn if any calls to functions are eliminated by the optimizations
enabled by the @option{-ffrontend-optimize} option.
+@item -Wrealloc-lhs
+@opindex @code{Wrealloc-lhs}
+@cindex Reallocate the LHS in assignments, notification
+Warn when the compiler might insert code to for allocation or reallocation of
+an allocatable array variable of intrinsic type in intrinsic assignments. In
+hot loops, the Fortran 2003 reallocation feature may reduce the performance.
+If the array is already allocated with the correct shape, consider using a
+whole-array array-spec (e.g. @code{(:,:,:)}) for the variable on the left-hand
+side to prevent the reallocation check. Note that in some cases the warning
+is shown, even if the compiler will optimize reallocation checks away. For
+instance, when the right-hand side contains the same variable multiplied by
+a scalar. See also @option{-frealloc-lhs}.
+@item -Wrealloc-lhs-all
+@opindex @code{Wrealloc-lhs-all}
+Warn when the compiler inserts code to for allocation or reallocation of an
+allocatable variable; this includes scalars and derived types.
+
@item -Werror
@opindex @code{Werror}
@cindex warnings, to errors
@@ -1561,7 +1577,8 @@ need to be in effect. The parentheses protection i
@cindex Reallocate the LHS in assignments
An allocatable left-hand side of an intrinsic assignment is automatically
(re)allocated if it is either unallocated or has a different shape. The
-option is enabled by default except when @option{-std=f95} is given.
+option is enabled by default except when @option{-std=f95} is given. See
+also @option{-Wrealloc-lhs}.
@item -faggressive-function-elimination
@opindex
Re: [PATCH] genmodes: remove misleading use of strncpy
On Thu, Apr 19, 2012 at 02:01:36PM +0200, Richard Guenther wrote: strncpy - strcpy definitely looks like premature optimization to me. I disagree. Almost all uses of strncpy are wrong. If the string length is smaller, than it is unlikely useful that it clears all remaining bytes (sometimes many of them) - exception might be security sensitive code, if the string length is known to be sizeof buf - 1, then strcpy or even memcpy should be used, and if the string length is larger, then no '\0' is appended, so it is unlikely the right thing. Yes, Jim should probably add a comment. Jakub
Re: [PATCH] genmodes: remove misleading use of strncpy
Richard Guenther wrote:
On Thu, Apr 19, 2012 at 12:42 PM, Jim Meyering j...@meyering.net wrote:
Found by inspection.
Seeing this strncpy use without the usual following NUL-termination,
my reflex was that it was a buffer overrun candidate, but then I
realized this is gcc, so that's not very likely.
Looking a little harder, I saw the preceding strlen = sizeof buf test,
which means there is no risk of overrun.
That preceding test means the use of strncpy is misleading, since
with that there is no risk of the name being too long for the buffer,
and hence no reason to use strncpy.
One way to make the code clearer is to use strcpy, as done below.
An alternative is to use memcpy (buf, m-name, strlen (m-name) + 1).
Thanks for the quick feedback.
Without a comment before the strcpy this isn't any more clear than
Good point.
How about with this folded in?
diff --git a/gcc/genmodes.c b/gcc/genmodes.c
index f786258..3833017 100644
--- a/gcc/genmodes.c
+++ b/gcc/genmodes.c
@@ -452,6 +452,7 @@ make_complex_modes (enum mode_class cl,
if (cl == MODE_FLOAT)
{
char *p, *q = 0;
+ /* We verified above that m-name+NUL fits in buf. */
strcpy (buf, m-name);
p = strchr (buf, 'F');
if (p == 0)
when using strncpy. Also your issue with the terminating NUL is
still present for the snprintf call done later (in fact the code looks like
it can be optimized, avoiding the strcpy in the path to the snprintf).
Isn't it different with snprintf?
While strncpy does *NOT* necessarily NUL-terminate,
snprintf explicitly does, always.
IMHO, if you'd like to avoid the duplication in the strcpy/snprintf
path, that deserves to be a different change.
[PATCH][IRA] Avoid undefined behavior in ira_allocno_object_iter_cond
This gave me headaches debugging a VRP miscompile of ira-build.c.
Number of iteration analysis concluded that the allocno object
iterators do not iterate because it sees accesses to -objects[n]
for a loop i = 0; i n; ++i. This is because
ira_allocno_object_iter_cond was written in a very fancy way,
optimizing the number of source lines (appearantly).
Fixed as follows.
A bootstrap regtest is currently running (together with the
alleged VRP modification). I will commit this if it succeeds.
Richard.
2012-04-19 Richard Guenther rguent...@suse.de
* ira-int.h (ira_allocno_object_iter_cond): Avoid out-of-bound
array access.
Index: gcc/ira-int.h
===
--- gcc/ira-int.h (revision 186584)
+++ gcc/ira-int.h (working copy)
@@ -1138,8 +1138,13 @@ static inline bool
ira_allocno_object_iter_cond (ira_allocno_object_iterator *i, ira_allocno_t a,
ira_object_t *o)
{
- *o = ALLOCNO_OBJECT (a, i-n);
- return i-n++ ALLOCNO_NUM_OBJECTS (a);
+ int n = i-n++;
+ if (n ALLOCNO_NUM_OBJECTS (a))
+{
+ *o = ALLOCNO_OBJECT (a, n);
+ return true;
+}
+ return false;
}
/* Loop over all objects associated with allocno A. In each
Re: [PATCH] genmodes: remove misleading use of strncpy
On Thu, Apr 19, 2012 at 2:13 PM, Jim Meyering j...@meyering.net wrote:
Richard Guenther wrote:
On Thu, Apr 19, 2012 at 12:42 PM, Jim Meyering j...@meyering.net wrote:
Found by inspection.
Seeing this strncpy use without the usual following NUL-termination,
my reflex was that it was a buffer overrun candidate, but then I
realized this is gcc, so that's not very likely.
Looking a little harder, I saw the preceding strlen = sizeof buf test,
which means there is no risk of overrun.
That preceding test means the use of strncpy is misleading, since
with that there is no risk of the name being too long for the buffer,
and hence no reason to use strncpy.
One way to make the code clearer is to use strcpy, as done below.
An alternative is to use memcpy (buf, m-name, strlen (m-name) + 1).
Thanks for the quick feedback.
Without a comment before the strcpy this isn't any more clear than
Good point.
How about with this folded in?
diff --git a/gcc/genmodes.c b/gcc/genmodes.c
index f786258..3833017 100644
--- a/gcc/genmodes.c
+++ b/gcc/genmodes.c
@@ -452,6 +452,7 @@ make_complex_modes (enum mode_class cl,
if (cl == MODE_FLOAT)
{
char *p, *q = 0;
+ /* We verified above that m-name+NUL fits in buf. */
strcpy (buf, m-name);
p = strchr (buf, 'F');
if (p == 0)
That's better. Or even cache the strlen result and use memcpy here
as Jakub suggested.
when using strncpy. Also your issue with the terminating NUL is
still present for the snprintf call done later (in fact the code looks like
it can be optimized, avoiding the strcpy in the path to the snprintf).
Isn't it different with snprintf?
While strncpy does *NOT* necessarily NUL-terminate,
snprintf explicitly does, always.
Sure, my point was that the
if (strlen (m-name) = sizeof buf)
{
error (%s:%d:mode name \%s\ is too long,
m-file, m-line, m-name);
continue;
}
check does not account for the (conditional) prepending of 'C' and the
snprintf would silently discard the last character of the name.
IMHO, if you'd like to avoid the duplication in the strcpy/snprintf
path, that deserves to be a different change.
Sure.
The patch is ok with caching strlen and using memcpy.
Richard.
Re: [PATCH] genmodes: remove misleading use of strncpy
On Thu, Apr 19, 2012 at 02:31:37PM +0200, Richard Guenther wrote: That's better. Or even cache the strlen result and use memcpy here as Jakub suggested. tree-ssa-strlen.c will do that for you when optimizing in this case ;) Jakub
Re: [PATCH] genmodes: remove misleading use of strncpy
On Thu, Apr 19, 2012 at 2:34 PM, Jakub Jelinek ja...@redhat.com wrote: On Thu, Apr 19, 2012 at 02:31:37PM +0200, Richard Guenther wrote: That's better. Or even cache the strlen result and use memcpy here as Jakub suggested. tree-ssa-strlen.c will do that for you when optimizing in this case ;) Does it? But only if you used strcpy, no? Richard. Jakub
Re: [PATCH] genmodes: remove misleading use of strncpy
On Thu, Apr 19, 2012 at 02:38:11PM +0200, Richard Guenther wrote: On Thu, Apr 19, 2012 at 2:34 PM, Jakub Jelinek ja...@redhat.com wrote: On Thu, Apr 19, 2012 at 02:31:37PM +0200, Richard Guenther wrote: That's better. Or even cache the strlen result and use memcpy here as Jakub suggested. tree-ssa-strlen.c will do that for you when optimizing in this case ;) Does it? But only if you used strcpy, no? Yes. I don't think it would handle the strncpy, it is in that case not about caching the value, but also additional comparison and knowing it is sufficiently large. Jakub
Re: [C++ Patch] PR 52363
Hi, On 04/18/2012 08:30 PM, Paolo Carlini wrote: Thus, it seems to me that with above change it's just that we don't have any hope of fixing latent SFINAE bugs in turney called by build_user_type_conversion_1 per the above. And, at this time, we don't have any evidence of such bugs. We've just been dealing with SFINAE bugs in tourney; I would expect that they can also occur for conversions using a single-argument constructor. Ok... I think you're right that we should just pass complain through build_user_type_conversion_1. Looking at the patch, I think we need to pass it through implicit_conversion and reference_binding as well. No big deal, here is a patch extending my last posted version to implicit_conversion and reference_binding. Booted and tested x86_64-linux. Thanks! Paolo. / /cp 2012-04-19 Paolo Carlini paolo.carl...@oracle.com PR c++/52363 * call.c (tourney, perform_overload_resolution, build_operator_new_call, build_user_type_conversion_1, build_user_type_conversion, perform_overload_resolution, implicit_conversion, reference_binding): Add tsubst_flags_t parameter. (joust): Likewise, use it to handle SFINAE as if pedantic. (build_list_conv, build_array_conv, build_complex_conv, add_function_candidate, add_conv_candidate, build_builtin_candidate, add_list_candidates, build_integral_nontype_arg_conv, perform_overload_resolution, build_new_function_call, build_operator_new_call, build_op_call_1, conditional_conversion, build_conditional_expr_1, build_new_op_1, convert_like_real, build_over_call, build_new_method_call_1, can_convert_arg, can_convert_arg_bad, perform_implicit_conversion_flags, perform_direct_initialization_if_possible, initialize_reference): Adjust. * init.c (build_new_1): Likewise. * cvt.c (ocp_convert, build_type_conversion, build_expr_type_conversion): Likewise. * cp-tree.h (build_user_type_conversion, build_operator_new_call): Adjust declaration. /testsuite 2012-04-19 Paolo Carlini paolo.carl...@oracle.com PR c++/52363 * testsuite/g++.dg/cpp0x/sfinae35.C: New. * testsuite/g++.dg/cpp0x/sfinae36.C: Likewise. Index: cp/init.c === --- cp/init.c (revision 186586) +++ cp/init.c (working copy) @@ -2381,7 +2381,7 @@ build_new_1 (VEC(tree,gc) **placement, tree type, alloc_call = build_operator_new_call (fnname, placement, size, cookie_size, - alloc_fn); + alloc_fn, complain); } } Index: cp/call.c === --- cp/call.c (revision 186586) +++ cp/call.c (working copy) @@ -142,9 +142,10 @@ static struct obstack conversion_obstack; static bool conversion_obstack_initialized; struct rejection_reason; -static struct z_candidate * tourney (struct z_candidate *); +static struct z_candidate * tourney (struct z_candidate *, tsubst_flags_t); static int equal_functions (tree, tree); -static int joust (struct z_candidate *, struct z_candidate *, bool); +static int joust (struct z_candidate *, struct z_candidate *, bool, + tsubst_flags_t); static int compare_ics (conversion *, conversion *); static tree build_over_call (struct z_candidate *, int, tsubst_flags_t); static tree build_java_interface_fn_ref (tree, tree); @@ -160,7 +161,8 @@ static tree convert_like_real (conversion *, tree, bool, tsubst_flags_t); static void op_error (enum tree_code, enum tree_code, tree, tree, tree, bool); -static struct z_candidate *build_user_type_conversion_1 (tree, tree, int); +static struct z_candidate *build_user_type_conversion_1 (tree, tree, int, +tsubst_flags_t); static void print_z_candidate (const char *, struct z_candidate *); static void print_z_candidates (location_t, struct z_candidate *); static tree build_this (tree); @@ -191,9 +193,11 @@ static struct z_candidate *add_conv_candidate static struct z_candidate *add_function_candidate (struct z_candidate **, tree, tree, tree, const VEC(tree,gc) *, tree, tree, int); -static conversion *implicit_conversion (tree, tree, tree, bool, int); +static conversion *implicit_conversion (tree, tree, tree, bool, int, + tsubst_flags_t); static conversion *standard_conversion (tree, tree, tree, bool, int); -static conversion *reference_binding (tree, tree, tree, bool, int); +static conversion *reference_binding (tree, tree, tree, bool, int, + tsubst_flags_t); static conversion *build_conv (conversion_kind,
[PATCH] Fix PR53031
This fixes PR53031 and reverts back to using the number of latch
iterations in VRP when computing ranges for induction variables.
Now after ira_allocno_object_iter_cond is fixed this no longer
breaks bootstrap.
Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
Richard.
2012-04-19 Richard Guenther rguent...@suse.de
PR tree-optimization/53031
* tree-vrp.c (adjust_range_with_scev): Revert back to
using max_loop_iterations.
Index: gcc/tree-vrp.c
===
--- gcc/tree-vrp.c (revision 186589)
+++ gcc/tree-vrp.c (working copy)
@@ -3420,7 +3420,9 @@ adjust_range_with_scev (value_range_t *v
{
double_int nit;
- if (max_stmt_executions (loop, nit))
+ /* We are only entering here for loop header PHI nodes, so using
+the number of latch executions is the correct thing to use. */
+ if (max_loop_iterations (loop, nit))
{
value_range_t maxvr = { VR_UNDEFINED, NULL_TREE, NULL_TREE, NULL };
double_int dtmp;
[obvious] Fix va_end calls
Hi,
I observed this with an alternate implementation of the stdarg stuff that
allocs new va_lists for va_start and va_copy, and frees them on va_end.
Some routines of diagnostic.c called va_end while the va_list was still
stored away in the diag objects and used in report_diagnostic.
I've audited all other calls of va_end in the compiler and these were the
only problematic ones. Committed as obvious in r186590 after regstrapping
on x86_64-linux, all languages.
Ciao,
Michael.
--
* diagnostic.c (emit_diagnostic): Move va_end call after user
of the va_list.
(warning, warning_at, pedwarn, permerror): Ditto.
Index: diagnostic.c
===
--- diagnostic.c(revision 186580)
+++ diagnostic.c(working copy)
@@ -714,6 +714,7 @@ emit_diagnostic (diagnostic_t kind, loca
{
diagnostic_info diagnostic;
va_list ap;
+ bool ret;
va_start (ap, gmsgid);
if (kind == DK_PERMERROR)
@@ -727,9 +728,10 @@ emit_diagnostic (diagnostic_t kind, loca
if (kind == DK_WARNING || kind == DK_PEDWARN)
diagnostic.option_index = opt;
}
- va_end (ap);
- return report_diagnostic (diagnostic);
+ ret = report_diagnostic (diagnostic);
+ va_end (ap);
+ return ret;
}
/* An informative note at LOCATION. Use this for additional details on an
error
@@ -771,13 +773,15 @@ warning (int opt, const char *gmsgid, ..
{
diagnostic_info diagnostic;
va_list ap;
+ bool ret;
va_start (ap, gmsgid);
diagnostic_set_info (diagnostic, gmsgid, ap, input_location, DK_WARNING);
diagnostic.option_index = opt;
+ ret = report_diagnostic (diagnostic);
va_end (ap);
- return report_diagnostic (diagnostic);
+ return ret;
}
/* A warning at LOCATION. Use this for code which is correct according to the
@@ -789,12 +793,14 @@ warning_at (location_t location, int opt
{
diagnostic_info diagnostic;
va_list ap;
+ bool ret;
va_start (ap, gmsgid);
diagnostic_set_info (diagnostic, gmsgid, ap, location, DK_WARNING);
diagnostic.option_index = opt;
+ ret = report_diagnostic (diagnostic);
va_end (ap);
- return report_diagnostic (diagnostic);
+ return ret;
}
/* A pedantic warning at LOCATION: issues a warning unless
@@ -815,12 +821,14 @@ pedwarn (location_t location, int opt, c
{
diagnostic_info diagnostic;
va_list ap;
+ bool ret;
va_start (ap, gmsgid);
diagnostic_set_info (diagnostic, gmsgid, ap, location, DK_PEDWARN);
diagnostic.option_index = opt;
+ ret = report_diagnostic (diagnostic);
va_end (ap);
- return report_diagnostic (diagnostic);
+ return ret;
}
/* A permissive error at LOCATION: issues an error unless
@@ -835,13 +843,15 @@ permerror (location_t location, const ch
{
diagnostic_info diagnostic;
va_list ap;
+ bool ret;
va_start (ap, gmsgid);
diagnostic_set_info (diagnostic, gmsgid, ap, location,
permissive_error_kind (global_dc));
diagnostic.option_index = permissive_error_option (global_dc);
+ ret = report_diagnostic (diagnostic);
va_end (ap);
- return report_diagnostic (diagnostic);
+ return ret;
}
/* A hard error: the code is definitely ill-formed, and an object file
Re: [PATCH] Add -fdump-rtl-pass-quiet
On 18/04/12 22:09, Xinliang David Li wrote: Flags in category 1) -- There are four types of information that can be dumped (should be controlled by flag set 1) ): 1.a) transformation decisions -- such as -fdump-tree-vectorize-transformations (to be added) 1.b) Intermediate representations -- related to what you propose 1.c) debug traces (flag to be added) 1.d) stats (memory usage, etc). That's all very nice, but apart from name bikeshedding, it shouldn't interfere with my patch at all. The same goes for most of the rest of your categories: the patches may conflict, but the purpose does not. The default should be dumping all 4 types, but can be individually controlled, e.g, dump transformation + IR: -fdump-tree-all-transformations-ir Do you already have an implementation for this? If so, please post it and we'll let a maintainer choose which they like best. Most of the existing flags are here. For instance, the following second level flags are used to control IR dump: vops, slim, verbose (may need to be renamed), raw, blocks, address, etc. The flag 'details' is used to control the debug trace level. To me, the existing (default), verbose, details option set makes sense, and adding quiet to that set seemed appropriate. I'm not sure I like the idea of renaming existing options, even if they aren't likely to break much. These are new proposed flags: To control IR dump before the pass or after the pass. Why? Is there a case where the dump from after the previous pass is not the same as the dump before the next? With the exception of the transformations stuff, as you call it, perturbing my diffs, I've not had any problems in this area. Sharad will be working on cleaning up the dump infrastructure, so that work will cover what you need. I look forward to seeing it. Hopefully the documentation will get a good once over as well! :) In the meantime, Mr Maintainers, can I commit my patch while we wait for the new world order? I'm happy to change the option name quiet to something else if necessary. Thanks Andrew
Re: [PATCH] genmodes: remove misleading use of strncpy
Richard Guenther wrote:
On Thu, Apr 19, 2012 at 2:13 PM, Jim Meyering j...@meyering.net wrote:
Richard Guenther wrote:
On Thu, Apr 19, 2012 at 12:42 PM, Jim Meyering j...@meyering.net wrote:
Found by inspection.
Seeing this strncpy use without the usual following NUL-termination,
my reflex was that it was a buffer overrun candidate, but then I
realized this is gcc, so that's not very likely.
Looking a little harder, I saw the preceding strlen = sizeof buf test,
which means there is no risk of overrun.
That preceding test means the use of strncpy is misleading, since
with that there is no risk of the name being too long for the buffer,
and hence no reason to use strncpy.
One way to make the code clearer is to use strcpy, as done below.
An alternative is to use memcpy (buf, m-name, strlen (m-name) + 1).
Thanks for the quick feedback.
Without a comment before the strcpy this isn't any more clear than
Good point.
How about with this folded in?
diff --git a/gcc/genmodes.c b/gcc/genmodes.c
index f786258..3833017 100644
--- a/gcc/genmodes.c
+++ b/gcc/genmodes.c
@@ -452,6 +452,7 @@ make_complex_modes (enum mode_class cl,
if (cl == MODE_FLOAT)
{
char *p, *q = 0;
+ /* We verified above that m-name+NUL fits in buf. */
strcpy (buf, m-name);
p = strchr (buf, 'F');
if (p == 0)
That's better. Or even cache the strlen result and use memcpy here
as Jakub suggested.
when using strncpy. Also your issue with the terminating NUL is
still present for the snprintf call done later (in fact the code looks like
it can be optimized, avoiding the strcpy in the path to the snprintf).
Isn't it different with snprintf?
While strncpy does *NOT* necessarily NUL-terminate,
snprintf explicitly does, always.
Sure, my point was that the
if (strlen (m-name) = sizeof buf)
{
error (%s:%d:mode name \%s\ is too long,
m-file, m-line, m-name);
continue;
}
check does not account for the (conditional) prepending of 'C' and the
snprintf would silently discard the last character of the name.
Yes, you're right.
It's good to avoid snprintf and its truncation, too.
IMHO, if you'd like to avoid the duplication in the strcpy/snprintf
path, that deserves to be a different change.
Sure.
The patch is ok with caching strlen and using memcpy.
Like this, I presume:
[alternatively, declare and compute m_len on a separate line,
just before the comparison:
size_t m_len = strlen (m-name);
I'd actually prefer that, but don't know if decl-after-stmt is ok here.
]
2012-04-19 Jim Meyering meyer...@redhat.com
genmodes: remove misleading use of strncpy
* genmodes.c (make_complex_modes): Avoid unnecessary use of strncpy.
We verified above that the string(including trailing NUL) fits in buf,
so just use memcpy.
diff --git a/gcc/genmodes.c b/gcc/genmodes.c
index 8b6f5bc..484a6ac 100644
--- a/gcc/genmodes.c
+++ b/gcc/genmodes.c
@@ -1,5 +1,5 @@
/* Generate the machine mode enumeration and associated tables.
- Copyright (C) 2003, 2004, 2005, 2006, 2007, 2010
+ Copyright (C) 2003, 2004, 2005, 2006, 2007, 2010, 2012
Free Software Foundation, Inc.
This file is part of GCC.
@@ -435,11 +435,13 @@ make_complex_modes (enum mode_class cl,
for (m = modes[cl]; m; m = m-next)
{
+ size_t m_len;
+
/* Skip BImode. FIXME: BImode probably shouldn't be MODE_INT. */
if (m-precision == 1)
continue;
- if (strlen (m-name) = sizeof buf)
+ if ((m_len = strlen (m-name)) = sizeof buf)
{
error (%s:%d:mode name \%s\ is too long,
m-file, m-line, m-name);
@@ -452,7 +454,8 @@ make_complex_modes (enum mode_class cl,
if (cl == MODE_FLOAT)
{
char *p, *q = 0;
- strncpy (buf, m-name, sizeof buf);
+ /* We verified above that m-name+NUL fits in buf. */
+ memcpy (buf, m-name, m_len + 1);
p = strchr (buf, 'F');
if (p == 0)
q = strchr (buf, 'D');
--
1.7.10.208.gb4267
Re: [PATCH] Fix PR52977
Hi,
On Mon, 16 Apr 2012, Richard Guenther wrote:
This fixes PR52977 - for PCH to work with its pointer relocation code we
have to avoid dereferencing pointers to compute array lengths in
structures. So we have to unfortunately keep duplicated info about
VECTOR_CST vector lengths.
That's a pity and caused me to look a bit at gengtype. We can do better,
and compute the lengths before all fields are accessed (and mangled in the
PCH case). With the patch below we emit such code:
case TS_VECTOR:
{
size_t l0 = (size_t)(TYPE_VECTOR_SUBPARTS (TREE_TYPE
((tree)((*x).generic.vector;
if ((void *)(x) == this_obj)
op (((*x).generic.vector.typed.type), cookie);
{
size_t i0;
for (i0 = 0; i0 != l0; i0++) {
if ((void *)(x) == this_obj)
op (((*x).generic.vector.elts[i0]), cookie);
}
}
}
Note how .generic.vector.typed.type is only mangled after TREE_TYPE is
used to calculate the length.
I regstrapped this on x86_64-linux, all languages, no regression. Okay
for trunk (saving the all-important memory for vector constants again :)
)?
Ciao,
Michael.
--
PR middle-end/52977
* tree.h (VECTOR_CST_NELTS): Use part number of types again.
(struct tree_vector): Adjust GTY length.
* tree.c (make_vector_stat): Don't set VECTOR_CST_NELTS.
* gengtype.c (struct walk_type_data): Add in_record_p and loopcounter
members.
(walk_type, TYPE_POINTER, TYPE_ARRAY): Handle case where our
caller emitted the length calulation already.
(walk_type, TYPE_UNION, TYPE_STRUCT): Emit length calculations
before handling any of the fields for structs.
Index: tree.h
===
--- tree.h (revision 186580)
+++ tree.h (working copy)
@@ -1534,14 +1534,13 @@ struct GTY(()) tree_complex {
};
/* In a VECTOR_CST node. */
-#define VECTOR_CST_NELTS(NODE) (VECTOR_CST_CHECK (NODE)-vector.length)
+#define VECTOR_CST_NELTS(NODE) (TYPE_VECTOR_SUBPARTS (TREE_TYPE (NODE)))
#define VECTOR_CST_ELTS(NODE) (VECTOR_CST_CHECK (NODE)-vector.elts)
#define VECTOR_CST_ELT(NODE,IDX) (VECTOR_CST_CHECK (NODE)-vector.elts[IDX])
struct GTY(()) tree_vector {
struct tree_typed typed;
- unsigned length;
- tree GTY ((length (%h.length))) elts[1];
+ tree GTY ((length (TYPE_VECTOR_SUBPARTS (TREE_TYPE ((tree)%h) elts[1];
};
#include symtab.h
Index: tree.c
===
--- tree.c (revision 186580)
+++ tree.c (working copy)
@@ -1329,7 +1329,6 @@ make_vector_stat (unsigned len MEM_STAT_
TREE_SET_CODE (t, VECTOR_CST);
TREE_CONSTANT (t) = 1;
- VECTOR_CST_NELTS (t) = len;
return t;
}
Index: gengtype.c
===
--- gengtype.c (revision 186580)
+++ gengtype.c (working copy)
@@ -2291,6 +2291,8 @@ struct walk_type_data
const char *reorder_fn;
bool needs_cast_p;
bool fn_wants_lvalue;
+ bool in_record_p;
+ int loopcounter;
};
/* Print a mangled name representing T to OF. */
@@ -2592,7 +2594,7 @@ walk_type (type_p t, struct walk_type_da
}
else
{
- int loopcounter = d-counter++;
+ int loopcounter = d-loopcounter;
const char *oldval = d-val;
const char *oldprevval3 = d-prev_val[3];
char *newval;
@@ -2602,7 +2604,10 @@ walk_type (type_p t, struct walk_type_da
oprintf (d-of, %*ssize_t i%d;\n, d-indent, , loopcounter);
oprintf (d-of, %*sfor (i%d = 0; i%d != (size_t)(, d-indent,
, loopcounter, loopcounter);
- output_escaped_param (d, length, length);
+ if (!d-in_record_p)
+ output_escaped_param (d, length, length);
+ else
+ oprintf (d-of, l%d, loopcounter);
oprintf (d-of, ); i%d++) {\n, loopcounter);
d-indent += 2;
d-val = newval = xasprintf (%s[i%d], oldval, loopcounter);
@@ -2624,7 +2629,7 @@ walk_type (type_p t, struct walk_type_da
case TYPE_ARRAY:
{
- int loopcounter = d-counter++;
+ int loopcounter;
const char *oldval = d-val;
char *newval;
@@ -2633,6 +2638,11 @@ walk_type (type_p t, struct walk_type_da
if (t-u.a.p-kind == TYPE_SCALAR)
break;
+ if (length)
+ loopcounter = d-loopcounter;
+ else
+ loopcounter = d-counter++;
+
/* When walking an array, compute the length and store it in a
local variable before walking the array elements, instead of
recomputing the length expression each time through the loop.
@@ -2643,13 +2653,16 @@ walk_type (type_p t, struct walk_type_da
oprintf (d-of, %*s{\n, d-indent, );
d-indent += 2;
Re: [PATCH] Fix PR52977
On Thu, 19 Apr 2012, Michael Matz wrote:
Hi,
On Mon, 16 Apr 2012, Richard Guenther wrote:
This fixes PR52977 - for PCH to work with its pointer relocation code we
have to avoid dereferencing pointers to compute array lengths in
structures. So we have to unfortunately keep duplicated info about
VECTOR_CST vector lengths.
That's a pity and caused me to look a bit at gengtype. We can do better,
and compute the lengths before all fields are accessed (and mangled in the
PCH case). With the patch below we emit such code:
case TS_VECTOR:
{
size_t l0 = (size_t)(TYPE_VECTOR_SUBPARTS (TREE_TYPE
((tree)((*x).generic.vector;
if ((void *)(x) == this_obj)
op (((*x).generic.vector.typed.type), cookie);
{
size_t i0;
for (i0 = 0; i0 != l0; i0++) {
if ((void *)(x) == this_obj)
op (((*x).generic.vector.elts[i0]), cookie);
}
}
}
Note how .generic.vector.typed.type is only mangled after TREE_TYPE is
used to calculate the length.
I regstrapped this on x86_64-linux, all languages, no regression. Okay
for trunk (saving the all-important memory for vector constants again :)
)?
Heh. Not for the memory usage but for the non-duplicate info.
Ok.
Thanks,
Richard.
Ciao,
Michael.
--
PR middle-end/52977
* tree.h (VECTOR_CST_NELTS): Use part number of types again.
(struct tree_vector): Adjust GTY length.
* tree.c (make_vector_stat): Don't set VECTOR_CST_NELTS.
* gengtype.c (struct walk_type_data): Add in_record_p and loopcounter
members.
(walk_type, TYPE_POINTER, TYPE_ARRAY): Handle case where our
caller emitted the length calulation already.
(walk_type, TYPE_UNION, TYPE_STRUCT): Emit length calculations
before handling any of the fields for structs.
Index: tree.h
===
--- tree.h(revision 186580)
+++ tree.h(working copy)
@@ -1534,14 +1534,13 @@ struct GTY(()) tree_complex {
};
/* In a VECTOR_CST node. */
-#define VECTOR_CST_NELTS(NODE) (VECTOR_CST_CHECK (NODE)-vector.length)
+#define VECTOR_CST_NELTS(NODE) (TYPE_VECTOR_SUBPARTS (TREE_TYPE (NODE)))
#define VECTOR_CST_ELTS(NODE) (VECTOR_CST_CHECK (NODE)-vector.elts)
#define VECTOR_CST_ELT(NODE,IDX) (VECTOR_CST_CHECK (NODE)-vector.elts[IDX])
struct GTY(()) tree_vector {
struct tree_typed typed;
- unsigned length;
- tree GTY ((length (%h.length))) elts[1];
+ tree GTY ((length (TYPE_VECTOR_SUBPARTS (TREE_TYPE ((tree)%h)
elts[1];
};
#include symtab.h
Index: tree.c
===
--- tree.c(revision 186580)
+++ tree.c(working copy)
@@ -1329,7 +1329,6 @@ make_vector_stat (unsigned len MEM_STAT_
TREE_SET_CODE (t, VECTOR_CST);
TREE_CONSTANT (t) = 1;
- VECTOR_CST_NELTS (t) = len;
return t;
}
Index: gengtype.c
===
--- gengtype.c(revision 186580)
+++ gengtype.c(working copy)
@@ -2291,6 +2291,8 @@ struct walk_type_data
const char *reorder_fn;
bool needs_cast_p;
bool fn_wants_lvalue;
+ bool in_record_p;
+ int loopcounter;
};
/* Print a mangled name representing T to OF. */
@@ -2592,7 +2594,7 @@ walk_type (type_p t, struct walk_type_da
}
else
{
- int loopcounter = d-counter++;
+ int loopcounter = d-loopcounter;
const char *oldval = d-val;
const char *oldprevval3 = d-prev_val[3];
char *newval;
@@ -2602,7 +2604,10 @@ walk_type (type_p t, struct walk_type_da
oprintf (d-of, %*ssize_t i%d;\n, d-indent, , loopcounter);
oprintf (d-of, %*sfor (i%d = 0; i%d != (size_t)(, d-indent,
, loopcounter, loopcounter);
- output_escaped_param (d, length, length);
+ if (!d-in_record_p)
+ output_escaped_param (d, length, length);
+ else
+ oprintf (d-of, l%d, loopcounter);
oprintf (d-of, ); i%d++) {\n, loopcounter);
d-indent += 2;
d-val = newval = xasprintf (%s[i%d], oldval, loopcounter);
@@ -2624,7 +2629,7 @@ walk_type (type_p t, struct walk_type_da
case TYPE_ARRAY:
{
- int loopcounter = d-counter++;
+ int loopcounter;
const char *oldval = d-val;
char *newval;
@@ -2633,6 +2638,11 @@ walk_type (type_p t, struct walk_type_da
if (t-u.a.p-kind == TYPE_SCALAR)
break;
+ if (length)
+ loopcounter = d-loopcounter;
+ else
+ loopcounter = d-counter++;
+
/* When walking an array, compute the length and store it in a
local variable before walking the array elements, instead of
recomputing the
[PATCH] Do not vectorize loops that do not roll
I noticed we vectorize
int objects[2];
void foo (int n)
{
int i;
for (i = 0; i n; ++i)
objects[i] = i;
}
which is of course pointless (with a vectorization factor 2).
The following patch avoids doing this.
Bootstrap regtest pending on x86_64-unknown-linux-gnu.
Richard.
2012-04-19 Richard Guenther rguent...@suse.de
* tree-vect-loop.c (vect_analyze_loop_operations): Do not
vectorize loops that can never run more often than the
vectorization factor.
Index: gcc/tree-vect-loop.c
===
*** gcc/tree-vect-loop.c(revision 186589)
--- gcc/tree-vect-loop.c(working copy)
*** vect_analyze_loop_operations (loop_vec_i
*** 1235,1240
--- 1235,1241
int min_scalar_loop_bound;
unsigned int th;
bool only_slp_in_loop = true, ok;
+ HOST_WIDE_INT max_niter;
if (vect_print_dump_info (REPORT_DETAILS))
fprintf (vect_dump, === vect_analyze_loop_operations ===);
*** vect_analyze_loop_operations (loop_vec_i
*** 1407,1414
vectorization_factor = %d, niters = HOST_WIDE_INT_PRINT_DEC,
vectorization_factor, LOOP_VINFO_INT_NITERS (loop_vinfo));
! if (LOOP_VINFO_NITERS_KNOWN_P (loop_vinfo)
!(LOOP_VINFO_INT_NITERS (loop_vinfo) vectorization_factor))
{
if (vect_print_dump_info (REPORT_UNVECTORIZED_LOCATIONS))
fprintf (vect_dump, not vectorized: iteration count too small.);
--- 1408,1417
vectorization_factor = %d, niters = HOST_WIDE_INT_PRINT_DEC,
vectorization_factor, LOOP_VINFO_INT_NITERS (loop_vinfo));
! if ((LOOP_VINFO_NITERS_KNOWN_P (loop_vinfo)
! (LOOP_VINFO_INT_NITERS (loop_vinfo) vectorization_factor))
! || ((max_niter = max_loop_iterations_int (loop)) != -1
! max_niter vectorization_factor))
{
if (vect_print_dump_info (REPORT_UNVECTORIZED_LOCATIONS))
fprintf (vect_dump, not vectorized: iteration count too small.);
libstdc++ / mt_allocator.cc when using gthreads
All,
The enclosed testcases (very close to
ext/mt_allocator/deallocate_global_thread-1.cc) exposes a pattern where
the following sequence is called (when __gthread_active_p is enabled) :
__gthread_key_create(key,::_M_destroy_thread_key);
__gthread_setspecific(key, (void*)_M_id);
__gthread_key_delete(key);
__gthread_setspecific(key, (void*)_M_id);
Even, if this works perfectly using pthreads, it seems to me that this
is an illegal use of threads. When using another thread implementation,
(i.e. when the key is implemented as a pointer). In my case:
- the key_create allocates the memory, and returns the pointer to the
allocated memory
- the key_delete frees this memory,
- the setspecific uses this freed memory.
If I m right saying this is an illegal use of threads, a possible quick
patch for mt_allocator.cc could be the one enclosed.
Let me know what you think of this approach.
Thanks.
Laurent
// 20.4.1.1 allocator members
#include list
#include string
#include stdexcept
#include cstdio
#include ext/mt_allocator.h
#include pthread.h
typedef std::string value_type;
using __gnu_cxx::__pool;
using __gnu_cxx::__common_pool_policy;
typedef __common_pool_policy__pool, true policy_type;
typedef __gnu_cxx::__mt_allocvalue_type, policy_type allocator_type;
typedef std::char_traitsvalue_type traits_type;
typedef std::listvalue_type, allocator_type list_type;
list_type l;
void *my_thread_process (void * arg)
{
printf (Thread %s\n, (char*)arg);
pthread_exit (0);
}
int main()
{
pthread_t th1;
if (pthread_create (th1, NULL, my_thread_process, (void*)1) 0) {
fprintf (stderr, pthread_create error for thread 1\n);
exit (1);
}
l.push_back(bayou bend);
return 0;
}
2012-04-19 Laurent Alfonsi laurent.alfo...@st.com
* src/c++98/mt_allocator.cc: Define/use an illegal thread key
(struct __freelist, __freelist::~__freelist()): set to illegal key
(pooltrue::_M_get_thread_id()): Check _M_key before using it
Index: src/c++98/mt_allocator.cc
===
--- src/c++98/mt_allocator.cc.orig
+++ src/c++98/mt_allocator.cc
@@ -34,19 +34,22 @@
namespace
{
#ifdef __GTHREADS
+ const __gthread_key_t ILLEGAL_THREAD_KEY = -1;
+
struct __freelist
{
typedef __gnu_cxx::__pooltrue::_Thread_record _Thread_record;
_Thread_record*_M_thread_freelist;
_Thread_record*_M_thread_freelist_array;
size_t _M_max_threads;
-__gthread_key_t_M_key;
+__gthread_key_t_M_key = ILLEGAL_THREAD_KEY;
~__freelist()
{
if (_M_thread_freelist_array)
{
__gthread_key_delete(_M_key);
+ _M_key = ILLEGAL_THREAD_KEY;
::operator delete(static_castvoid*(_M_thread_freelist_array));
_M_thread_freelist = 0;
}
@@ -640,7 +643,8 @@
}
}
-__gthread_setspecific(freelist._M_key, (void*)_M_id);
+ if (freelist._M_key!=ILLEGAL_THREAD_KEY)
+ __gthread_setspecific(freelist._M_key, (void*)_M_id);
}
return _M_id = _M_options._M_max_threads ? 0 : _M_id;
}
[PATCH] Fix symtab dumping
Committed as obvious. Richard. 2012-04-19 Richard Guenther rguent...@suse.de * symtab.c (dump_symtab_base): Use TREE_STRING_POINTER for dumping DECL_SECTION_NAME. Index: gcc/symtab.c === --- gcc/symtab.c(revision 186591) +++ gcc/symtab.c(working copy) @@ -402,7 +402,7 @@ dump_symtab_base (FILE *f, symtab_node n fprintf (f, one_only); if (DECL_SECTION_NAME (node-symbol.decl)) fprintf (f, section_name:%s, -IDENTIFIER_POINTER (DECL_SECTION_NAME (node-symbol.decl))); +TREE_STRING_POINTER (DECL_SECTION_NAME (node-symbol.decl))); if (DECL_VISIBILITY_SPECIFIED (node-symbol.decl)) fprintf (f, visibility_specified); if (DECL_VISIBILITY (node-symbol.decl))
Re: [PATCH] genmodes: remove misleading use of strncpy
On Thu, Apr 19, 2012 at 3:01 PM, Jim Meyering j...@meyering.net wrote:
Richard Guenther wrote:
On Thu, Apr 19, 2012 at 2:13 PM, Jim Meyering j...@meyering.net wrote:
Richard Guenther wrote:
On Thu, Apr 19, 2012 at 12:42 PM, Jim Meyering j...@meyering.net wrote:
Found by inspection.
Seeing this strncpy use without the usual following NUL-termination,
my reflex was that it was a buffer overrun candidate, but then I
realized this is gcc, so that's not very likely.
Looking a little harder, I saw the preceding strlen = sizeof buf test,
which means there is no risk of overrun.
That preceding test means the use of strncpy is misleading, since
with that there is no risk of the name being too long for the buffer,
and hence no reason to use strncpy.
One way to make the code clearer is to use strcpy, as done below.
An alternative is to use memcpy (buf, m-name, strlen (m-name) + 1).
Thanks for the quick feedback.
Without a comment before the strcpy this isn't any more clear than
Good point.
How about with this folded in?
diff --git a/gcc/genmodes.c b/gcc/genmodes.c
index f786258..3833017 100644
--- a/gcc/genmodes.c
+++ b/gcc/genmodes.c
@@ -452,6 +452,7 @@ make_complex_modes (enum mode_class cl,
if (cl == MODE_FLOAT)
{
char *p, *q = 0;
+ /* We verified above that m-name+NUL fits in buf. */
strcpy (buf, m-name);
p = strchr (buf, 'F');
if (p == 0)
That's better. Or even cache the strlen result and use memcpy here
as Jakub suggested.
when using strncpy. Also your issue with the terminating NUL is
still present for the snprintf call done later (in fact the code looks like
it can be optimized, avoiding the strcpy in the path to the snprintf).
Isn't it different with snprintf?
While strncpy does *NOT* necessarily NUL-terminate,
snprintf explicitly does, always.
Sure, my point was that the
if (strlen (m-name) = sizeof buf)
{
error (%s:%d:mode name \%s\ is too long,
m-file, m-line, m-name);
continue;
}
check does not account for the (conditional) prepending of 'C' and the
snprintf would silently discard the last character of the name.
Yes, you're right.
It's good to avoid snprintf and its truncation, too.
IMHO, if you'd like to avoid the duplication in the strcpy/snprintf
path, that deserves to be a different change.
Sure.
The patch is ok with caching strlen and using memcpy.
Like this, I presume:
[alternatively, declare and compute m_len on a separate line,
just before the comparison:
size_t m_len = strlen (m-name);
I'd actually prefer that, but don't know if decl-after-stmt is ok here.
]
Works for me with ...
2012-04-19 Jim Meyering meyer...@redhat.com
genmodes: remove misleading use of strncpy
* genmodes.c (make_complex_modes): Avoid unnecessary use of strncpy.
We verified above that the string(including trailing NUL) fits in buf,
so just use memcpy.
diff --git a/gcc/genmodes.c b/gcc/genmodes.c
index 8b6f5bc..484a6ac 100644
--- a/gcc/genmodes.c
+++ b/gcc/genmodes.c
@@ -1,5 +1,5 @@
/* Generate the machine mode enumeration and associated tables.
- Copyright (C) 2003, 2004, 2005, 2006, 2007, 2010
+ Copyright (C) 2003, 2004, 2005, 2006, 2007, 2010, 2012
Free Software Foundation, Inc.
This file is part of GCC.
@@ -435,11 +435,13 @@ make_complex_modes (enum mode_class cl,
for (m = modes[cl]; m; m = m-next)
{
+ size_t m_len;
+
/* Skip BImode. FIXME: BImode probably shouldn't be MODE_INT. */
if (m-precision == 1)
continue;
- if (strlen (m-name) = sizeof buf)
+ if ((m_len = strlen (m-name)) = sizeof buf)
... the strlen in a separate stmt like
m_len = strlen (m-name);
if (m_len = sizeof (buf))
Thanks,
Richard.
{
error (%s:%d:mode name \%s\ is too long,
m-file, m-line, m-name);
@@ -452,7 +454,8 @@ make_complex_modes (enum mode_class cl,
if (cl == MODE_FLOAT)
{
char *p, *q = 0;
- strncpy (buf, m-name, sizeof buf);
+ /* We verified above that m-name+NUL fits in buf. */
+ memcpy (buf, m-name, m_len + 1);
p = strchr (buf, 'F');
if (p == 0)
q = strchr (buf, 'D');
--
1.7.10.208.gb4267
Re: [PATCH] genmodes: remove misleading use of strncpy
Richard Guenther wrote: ... The patch is ok with caching strlen and using memcpy. Like this, I presume: [alternatively, declare and compute m_len on a separate line, just before the comparison: size_t m_len = strlen (m-name); I'd actually prefer that, but don't know if decl-after-stmt is ok here. ] Works for me with ... ... - if (strlen (m-name) = sizeof buf) + if ((m_len = strlen (m-name)) = sizeof buf) ... the strlen in a separate stmt like m_len = strlen (m-name); if (m_len = sizeof (buf)) Thanks. Adjusted and committed.
Re: libstdc++ / mt_allocator.cc when using gthreads
Hi, All, The enclosed testcases (very close to ext/mt_allocator/deallocate_global_thread-1.cc) exposes a pattern where the following sequence is called (when __gthread_active_p is enabled) : __gthread_key_create(key,::_M_destroy_thread_key); __gthread_setspecific(key, (void*)_M_id); __gthread_key_delete(key); __gthread_setspecific(key, (void*)_M_id); I didn't look into the issue in any detail - and I'm really glad that you are interested in improving mt_allocator - but I think that before going for quick patches, we should strive for fixing the underlying issue, eg prevent the above from occurring. After all, if you are right, the issue is very, very old, and nobody complained yet, thus better try very hard to fix it for real, finally. Do you have any plans for that? Thanks, Paolo.
Re: PING: [PATCH] Fix PRs c/52283/37985
On Thu, Apr 19, 2012 at 3:17 AM, Manuel López-Ibáñez lopeziba...@gmail.com wrote: On 19 April 2012 11:11, Christian Bruel christian.br...@st.com wrote: On 04/18/2012 11:51 AM, Richard Guenther wrote: On Wed, Apr 18, 2012 at 11:06 AM, Manuel López-Ibáñez lopeziba...@gmail.com wrote: On 18 April 2012 10:29, Christian Bruel christian.br...@st.com wrote: Is it OK for trunk, bootstrapped and regtested on x86 I think Joseph Myers is on vacation, and there are no other C FE reviewers, but since this is c-common and convert.c, perhaps Jason and/or Richard can review it? The patch is ok if you put the PR52283 properly into a separate testcase, not by amending gcc.dg/case-const-2.c. Thanks, done at rev #186586. with this change. Great! Just a minor nit, for future patches. There is the unwritten rule of adding the Changelogs to the commit log, like follows: 2012-04-19 Christian Bruel christian.br...@st.com Manuel López-Ibáñez m...@gcc.gnu.org PR c/52283 PR c/37985 * stmt.c (warn_if_unused_value): Skip NOP_EXPR. * convert.c (convert_to_integer): Don't set TREE_NO_WARNING. testsuite/ * gcc.dg/pr52283.c: New test. * gcc.dg/pr37985.c: New test. gcc.dg/pr52283.c failed on Linux/x86: FAIL: gcc.dg/pr52283.c (test for excess errors) -- H.J.
Re: [patch] Cleanup tree-switch-conversion a bit
On Tue, Apr 17, 2012 at 9:15 AM, Steven Bosscher stevenb@gmail.com wrote: My goal for GCC 4.8 is to do just that: Move switch expansion to GIMPLE and add value profiling for switch expressions. And the idea is to put all that code in tree-switch-conversion.c. But there are a few clean-ups I wish to do on that code before that. First, there is a global pass info structure that contains useful data for all forms of GIMPLE_SWITCH lowering. I've un-globalized that data with the attached patch. While there, I made the dump messages uniform. Bootstrapped and tested on powerpc-unknown-linux-gnu. OK? I think it caused: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53043 -- H.J.
Re: libstdc++ / mt_allocator.cc when using gthreads
Well, I don't know mt_allocator enough to know if this is a fix for real or a quick fix. Regards, Laurent On 04/19/12 16:26, Paolo Carlini wrote: Hi, All, The enclosed testcases (very close to ext/mt_allocator/deallocate_global_thread-1.cc) exposes a pattern where the following sequence is called (when __gthread_active_p is enabled) : __gthread_key_create(key,::_M_destroy_thread_key); __gthread_setspecific(key, (void*)_M_id); __gthread_key_delete(key); __gthread_setspecific(key, (void*)_M_id); I didn't look into the issue in any detail - and I'm really glad that you are interested in improving mt_allocator - but I think that before going for quick patches, we should strive for fixing the underlying issue, eg prevent the above from occurring. After all, if you are right, the issue is very, very old, and nobody complained yet, thus better try very hard to fix it for real, finally. Do you have any plans for that? Thanks, Paolo.
Re: [patch] Cleanup tree-switch-conversion a bit
On Thu, Apr 19, 2012 at 4:55 PM, H.J. Lu hjl.to...@gmail.com wrote:
On Tue, Apr 17, 2012 at 9:15 AM, Steven Bosscher stevenb@gmail.com
wrote:
My goal for GCC 4.8 is to do just that: Move switch expansion to
GIMPLE and add value profiling for switch expressions.
And the idea is to put all that code in tree-switch-conversion.c. But
there are a few clean-ups I wish to do on that code before that.
First, there is a global pass info structure that contains useful data
for all forms of GIMPLE_SWITCH lowering. I've un-globalized that
data with the attached patch. While there, I made the dump messages
uniform.
Bootstrapped and tested on powerpc-unknown-linux-gnu. OK?
I think it caused:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53043
Yes.
* gcc.target/i386/pr45830.c: Update scan-tree-dump.
Index: gcc.target/i386/pr45830.c
===
--- gcc.target/i386/pr45830.c (revision 186596)
+++ gcc.target/i386/pr45830.c (working copy)
@@ -26,6 +26,6 @@ foo (int *a)
}
}
-/* { dg-final { scan-tree-dump Expanding as bit test is preferable
switchconv } } */
+/* { dg-final { scan-tree-dump expanding as bit test is preferable
switchconv } } */
/* { dg-final { scan-assembler-not CSWTCH } } */
/* { dg-final { cleanup-tree-dump switchconv } } */
[patch] Move switch-conversion after profiling
Hello, If we want to use profiling to expand switches in GIMPLE, we'll have to run switch-conversion after profiling. Bootstrapped and tested on x86_64-unknown-linux-gnu. OK? Ciao! Steven * passes.c (pass_convert_switch): Move after profiling and pure_const. Index: passes.c === --- passes.c(revision 186586) +++ passes.c(working copy) @@ -1326,10 +1326,10 @@ init_optimization_passes (void) NEXT_PASS (pass_cd_dce); NEXT_PASS (pass_early_ipa_sra); NEXT_PASS (pass_tail_recursion); - NEXT_PASS (pass_convert_switch); NEXT_PASS (pass_cleanup_eh); NEXT_PASS (pass_profile); NEXT_PASS (pass_local_pure_const); + NEXT_PASS (pass_convert_switch); /* Split functions creates parts that are not run through early optimizations again. It is thus good idea to do this late. */
Re: [Patch, libfortran] Fix handling of temporary files
On Thu, Apr 19, 2012 at 14:43, Janne Blomqvist
blomqvist.ja...@gmail.com wrote:
Also, I suspect getuid and friends are not actually part of the NSS
stuff. The uid/gid/euid/egid are maintained for each process by the
kernel, and NSS only handles translation to/from the numeric id's to
string values, IP address lookup and so forth.
.. as can be seen with the example below:
#include unistd.h
#include sys/types.h
#include pwd.h
#include stdio.h
int main(void)
{
uid_t uid = getuid();
//struct passwd *pwd = getpwuid(uid);
//printf(%s , pwd-pw_name);
printf(%d %d %d %d\n, uid, geteuid(), getgid(), getegid());
return 0;
}
$ gcc getuid.c -static
$ strace ./a.out
(No need to open further files)
Uncommenting the two commented lines:
$ gcc getuid.c -static
/tmp/cc6B1i8n.o: In function `main':
getuid.c:(.text+0x16): warning: Using 'getpwuid' in statically linked
applications requires at runtime the shared libraries from the glibc
version used for linking
$ strace ./a.out
(Evidence of communicating with nscd per the NSS configuration on this system)
Hence get{uid,euid,gid,egid} should not have any problems with static linking.
--
Janne Blomqvist
Re: [PATCH, i386, middle-end, tessuite] Intel TSX's HLE.
Folks, Thanks a lot for prompts! I've updated my patch, so cmparing to previous it is: - have dedicated hook var, to define target dep. bits in mem model. Check (and warning) is performed in get_memmodel - prefix emit performed through dedicated operand type (%K) Updated ChangeLog entry: 2012-04-19 Kirill Yukhin kirill.yuk...@intel.com * builtins.c (get_memmodel):Check of upper bound with respect to target dependent mask. (expand_builtin_atomic_compare_exchange): Mask memmodel values. * config/i386/cpuid.h (bit_HLE): New. * config/i386/driver-i386.c (host_detect_local_cpu): Detect HLE support. * config/i386/i386-protos.h (ix86_generate_hle_prefix): New. * config/i386/i386-c.c (ix86_target_macros_internal): Set HLE defines. (ix86_target_string)-mhle: New. (ix86_option_override_internal)PTA_HLE: Ditto. (ix86_valid_target_attribute_inner_p)OPT_mhle: Ditto. * config/i386/i386.c (ix86_target_string)OPTION_MASK_ISA_HLE: New. (ix86_valid_target_attribute_inner_p)OPT_mhle: Ditto. (ix86_print_operand): Generate HLE lck prefixes. (TARGET_MEMMODEL_MASK): New. * config/i386/i386.h (OPTION_ISA_HLE): Ditto. (IX86_HLE_ACQUIRE): Ditto. (IX86_HLE_RELEASE): Ditto. * config/i386/i386.h (ix86_generate_hle_prefix): Ditto. * config/i386/i386.opt (mhle): Ditto. * config/i386/sync.md(atomic_compare_and_swapmode): Pass success model to instruction emitter. (atomic_fetch_addmode): Ditto. (atomic_exchangemode): Ditto. (atomic_addmode): Ditto. (atomic_submode): Ditto. (atomic_codemode): Ditto. (*atomic_compare_and_swap_doubledi_pic): Ditto. (atomic_compare_and_swap_singlemode): Define and use argument for success model. (atomic_compare_and_swap_doublemode): Ditto. * configure.ac: Check if assembler support HLE prefixes. * configure: Regenerate. * config.in: Ditto. testsuite/ChangeLog unchanged. Patch attached. Thanks, K hle-3.gcc.patch Description: Binary data
Re: PowerPC prologue and epilogue 5
On Thu, Apr 19, 2012 at 08:00:15PM +0930, Alan Modra wrote:
On Wed, Apr 18, 2012 at 12:45:16AM +0930, Alan Modra wrote:
This enables out-of-line save and restore for large frames, and for
ABI_AIX when using the static chain.
Further testing revealed two problems when compiling nested
functions.
1) The logic I had for cr_save_regno is wrong, resulting in one of my
NOT_INUSE asserts triggering.
Fixed in this revised patch. Bootstrapped etc. powerpc-linux.
2) In some cases the prologue uses in-line saves while the epilogue
uses out-of-line restores. This can lead to restoring regs that
haven't been saved.
This turned out to be a pre-existing problem, patch in PR50340.
* config/rs6000/rs6000.c (rs6000_savres_strategy): Allow
out-of-line save/restore for large frames. Don't disable
out-of-line saves on ABI_AIX when using static chain reg.
(rs6000_emit_prologue): Adjust cr_save_regno on ABI_AIX to not
clobber static chain reg, and tweak for out-of-line gpr saves
that use r1.
diff -urp gcc-alan5a/gcc/config/rs6000/rs6000.c
gcc-alan5/gcc/config/rs6000/rs6000.c
--- gcc-alan5a/gcc/config/rs6000/rs6000.c 2012-04-19 21:24:46.643632761
+0930
+++ gcc-alan5/gcc/config/rs6000/rs6000.c2012-04-19 20:55:02.214727782
+0930
@@ -17432,8 +17432,7 @@ rs6000_savres_strategy (rs6000_stack_t *
strategy |= SAVRES_MULTIPLE;
if (crtl-calls_eh_return
- || cfun-machine-ra_need_lr
- || info-total_size 32767)
+ || cfun-machine-ra_need_lr)
strategy |= (SAVE_INLINE_FPRS | REST_INLINE_FPRS
| SAVE_INLINE_GPRS | REST_INLINE_GPRS);
@@ -17454,8 +17453,8 @@ rs6000_savres_strategy (rs6000_stack_t *
/* Don't bother to try to save things out-of-line if r11 is occupied
by the static chain. It would require too much fiddling and the
static chain is rarely used anyway. FPRs are saved w.r.t the stack
- pointer on Darwin. */
- if (using_static_chain_p)
+ pointer on Darwin, and AIX uses r1 or r12. */
+ if (using_static_chain_p DEFAULT_ABI != ABI_AIX)
strategy |= ((DEFAULT_ABI == ABI_DARWIN
? 0 : SAVE_INLINE_FPRS | REST_INLINE_FPRS)
| SAVE_INLINE_GPRS);
@@ -19555,11 +19554,16 @@ rs6000_emit_prologue (void)
}
}
- /* If we need to save CR, put it into r12 or r11. */
- cr_save_regno = DEFAULT_ABI == ABI_AIX !saving_GPRs_inline ? 11 : 12;
+ /* If we need to save CR, put it into r12 or r11. Choose r12 except when
+ r12 will be needed by out-of-line gpr restore. */
+ cr_save_regno = (DEFAULT_ABI == ABI_AIX
+ !(strategy (SAVE_INLINE_GPRS
+ | SAVE_NOINLINE_GPRS_SAVES_LR))
+ ? 11 : 12);
if (!WORLD_SAVE_P (info)
info-cr_save_p
- REGNO (frame_reg_rtx) != cr_save_regno)
+ REGNO (frame_reg_rtx) != cr_save_regno
+ !(using_static_chain_p cr_save_regno == 11))
{
rtx set;
--
Alan Modra
Australia Development Lab, IBM
Re: [PATCH, i386, middle-end, tessuite] Intel TSX's HLE.
On 04/19/2012 11:21 AM, Kirill Yukhin wrote:
+++ b/gcc/builtins.c
@@ -5338,6 +5338,7 @@ static enum memmodel
get_memmodel (tree exp)
{
rtx op;
+ unsigned memmodel_mask = (116) - 1;
...
@@ -5398,11 +5409,14 @@ expand_builtin_atomic_compare_exchange (enum
machine_mode mode, tree exp,
enum memmodel success, failure;
tree weak;
bool is_weak;
+ /* Suppose that higher bits are target dependant. */
+ unsigned memmodel_mask = (116) - 1;
My only other comment is that I think the MEMMODEL_MASK ought to be defined
globally where the MEMMODEL enum is defined instead of defining it everywhere
it is needed.
Andrew
Re: libstdc++ / mt_allocator.cc when using gthreads
On 04/19/2012 05:02 PM, Laurent Alfonsi wrote: Well, I don't know mt_allocator enough to know if this is a fix for real or a quick fix. I'm pretty sure we could do better. Whether in practice we are going to do better, any time soon, it's another matter ;) Paolo.
Re: [RFA] dwarf2.h (dwarf_location_atom): Add DW_OP_GNU_addr_index.
Hi Doug, 2012-04-18 Doug Evansd...@google.com * dwarf2.h (dwarf_location_atom): Add DW_OP_GNU_addr_index. Approved - please apply. Cheers Nick
Re: [PATCH] Add -fdump-rtl-pass-quiet
On Thu, Apr 19, 2012 at 5:58 AM, Andrew Stubbs a...@codesourcery.com wrote: On 18/04/12 22:09, Xinliang David Li wrote: Flags in category 1) -- There are four types of information that can be dumped (should be controlled by flag set 1) ): 1.a) transformation decisions -- such as -fdump-tree-vectorize-transformations (to be added) 1.b) Intermediate representations -- related to what you propose 1.c) debug traces (flag to be added) 1.d) stats (memory usage, etc). That's all very nice, but apart from name bikeshedding, it shouldn't interfere with my patch at all. No -- I just want to let you know there is similar effort planed for better coordination. The maintainer will make a call on your patch. The same goes for most of the rest of your categories: the patches may To me, the existing (default), verbose, details option set makes sense, and adding quiet to that set seemed appropriate. I'm not sure I like the idea of renaming existing options, even if they aren't likely to break much. It is confusing. 'verbose' applies to IR, while 'details' applies to debug traces. These are new proposed flags: To control IR dump before the pass or after the pass. Why? Is there a case where the dump from after the previous pass is not the same as the dump before the next? For productivity -- without it, you either need to remember all pass ordering, or always dump-all -- the latter can be prohibitive for very large files. thanks, David
Re: [PATCH] Add -fdump-rtl-pass-quiet
On 19/04/12 17:21, Xinliang David Li wrote: To me, the existing (default), verbose, details option set makes sense, and adding quiet to that set seemed appropriate. I'm not sure I like the idea of renaming existing options, even if they aren't likely to break much. It is confusing. 'verbose' applies to IR, while 'details' applies to debug traces. Ah, so -fdump-rtl-all-quiet-verbose might actually be meaningful? Interesting, I had not spotted that. Like I said, the documentation could use a once over. These are new proposed flags: To control IR dump before the pass or after the pass. Why? Is there a case where the dump from after the previous pass is not the same as the dump before the next? For productivity -- without it, you either need to remember all pass ordering, or always dump-all -- the latter can be prohibitive for very large files. Fair enough. Andrew
Re: [PATCH, i386, middle-end, tessuite] Intel TSX's HLE.
On Thu, Apr 19, 2012 at 5:21 PM, Kirill Yukhin kirill.yuk...@gmail.com wrote: Folks, Thanks a lot for prompts! I've updated my patch, so cmparing to previous it is: - have dedicated hook var, to define target dep. bits in mem model. Check (and warning) is performed in get_memmodel - prefix emit performed through dedicated operand type (%K) +#else + fputs (\n ASM_BYTE 0xf3\n\t); +#endif Ouch, my bad, you need to add file argument here (no need to repost patch because of this). FYI, you have wrong indentation here. Otherwise, OK as far as x86 is concerned, but you will need separate approval for middle-end part. Thanks, Uros.
[lra] patch to fix most mips testsuite regressions
The following patch fixes most GCC testsuite regressions of LRA on mips64.
Committed as rev. 186598.
2012-04-19 Vladimir Makarov vmaka...@redhat.com
* config/mips/mips.c (mips_expand_fcc_reload): Remove code for
LRA. Add an assertion.
(mips_secondary_reload): New function.
(TARGET_SECONDARY_RELOAD): Redefine.
Index: config/mips/mips.c
===
--- config/mips/mips.c (revision 186357)
+++ config/mips/mips.c (working copy)
@@ -6661,30 +6661,15 @@ mips_expand_fcc_reload (rtx dest, rtx sr
{
rtx fp1, fp2;
+ gcc_assert (! flag_lra);
/* Change the source to SFmode. */
if (MEM_P (src))
src = adjust_address (src, SFmode, 0);
else if (REG_P (src) || GET_CODE (src) == SUBREG)
-{
- if (! flag_lra)
- src = gen_rtx_REG (SFmode, true_regnum (src));
- else if (GET_MODE (src) != SFmode)
- src = gen_rtx_SUBREG (SFmode,
- GET_CODE (src) == SUBREG ? SUBREG_REG (src) : src,
- 0);
-}
+src = gen_rtx_REG (SFmode, true_regnum (src));
- if (flag_lra)
-{
- enum reg_class rclass = lra_get_allocno_class (REGNO (scratch));
- fp1 = lra_create_new_reg (SFmode, NULL_RTX, rclass, new scratch);
- fp2 = lra_create_new_reg (SFmode, NULL_RTX, rclass, zero);
-}
- else
-{
- fp1 = gen_rtx_REG (SFmode, REGNO (scratch));
- fp2 = gen_rtx_REG (SFmode, REGNO (scratch) + MAX_FPRS_PER_FMT);
-}
+ fp1 = gen_rtx_REG (SFmode, REGNO (scratch));
+ fp2 = gen_rtx_REG (SFmode, REGNO (scratch) + MAX_FPRS_PER_FMT);
mips_emit_move (copy_rtx (fp1), src);
mips_emit_move (copy_rtx (fp2), CONST0_RTX (SFmode));
emit_insn (gen_slt_sf (dest, fp2, fp1));
@@ -11156,6 +11141,18 @@ mips_preferred_reload_class (rtx x, reg_
return rclass;
}
+/* Implement TARGET_SECONDARY_RELOAD. */
+
+static reg_class_t
+mips_secondary_reload (bool in_p, rtx x, reg_class_t reload_class_i,
+ enum machine_mode reload_mode,
+ secondary_reload_info *sri)
+{
+ if (flag_lra)
+return NO_REGS;
+ return default_secondary_reload (in_p, x, reload_class_i, reload_mode, sri);
+}
+
/* RCLASS is a class involved in a REGISTER_MOVE_COST calculation.
Return a canonical class to represent it in later calculations. */
@@ -17354,6 +17351,9 @@ mips_expand_vec_minmax (rtx target, rtx
#undef TARGET_PREFERRED_RELOAD_CLASS
#define TARGET_PREFERRED_RELOAD_CLASS mips_preferred_reload_class
+#undef TARGET_SECONDARY_RELOAD
+#define TARGET_SECONDARY_RELOAD mips_secondary_reload
+
#undef TARGET_ASM_FILE_START
#define TARGET_ASM_FILE_START mips_file_start
#undef TARGET_ASM_FILE_START_FILE_DIRECTIVE
Re: [PATCH][IRA] Avoid undefined behavior in ira_allocno_object_iter_cond
On 04/19/2012 08:14 AM, Richard Guenther wrote:
This gave me headaches debugging a VRP miscompile of ira-build.c.
Number of iteration analysis concluded that the allocno object
iterators do not iterate because it sees accesses to -objects[n]
for a loop i = 0; i n; ++i. This is because
ira_allocno_object_iter_cond was written in a very fancy way,
optimizing the number of source lines (appearantly).
Fixed as follows.
A bootstrap regtest is currently running (together with the
alleged VRP modification). I will commit this if it succeeds.
Thanks, Richard.
2012-04-19 Richard Guentherrguent...@suse.de
* ira-int.h (ira_allocno_object_iter_cond): Avoid out-of-bound
array access.
Index: gcc/ira-int.h
===
--- gcc/ira-int.h (revision 186584)
+++ gcc/ira-int.h (working copy)
@@ -1138,8 +1138,13 @@ static inline bool
ira_allocno_object_iter_cond (ira_allocno_object_iterator *i, ira_allocno_t a,
ira_object_t *o)
{
- *o = ALLOCNO_OBJECT (a, i-n);
- return i-n++ ALLOCNO_NUM_OBJECTS (a);
+ int n = i-n++;
+ if (n ALLOCNO_NUM_OBJECTS (a))
+{
+ *o = ALLOCNO_OBJECT (a, n);
+ return true;
+}
+ return false;
}
/* Loop over all objects associated with allocno A. In each
Re: Continue strict-volatile-bitfields fixes
Hi!
On Wed, 18 Apr 2012 18:16:32 +0200, Bernd Schmidt ber...@codesourcery.com
wrote:
On 04/18/2012 06:14 PM, Richard Earnshaw wrote:
On 18/04/12 16:37, Thomas Schwinge wrote:
gcc/testsuite/
* gcc.dg/tree-ssa/20030922-1.c: Compile with
-fno-strict-volatile-bitfields.
* gcc.dg/tree-ssa/foldconst-3.c: Likewise.
* gcc.dg/tree-ssa/vrp15.c: Likewise.
None of these have any volatile bitfields, so the option should be a no-op.
That's what I thought, too. :-)
The problem is that we have to treat normal bitfields differently as
well, since a variable may later be declared as volatile.
Here is my current test case, reduced from gcc.dg/tree-ssa/20030922-1.c:
extern void abort (void);
enum tree_code
{
BIND_EXPR,
};
struct tree_common
{
enum tree_code code:8;
};
void
voidify_wrapper_expr (struct tree_common *common)
{
switch (common-code)
{
case BIND_EXPR:
if (common-code != BIND_EXPR)
abort ();
}
}
The diff for -fdump-tree-all between compiling with
-fno-strict-volatile-bitfields and -fstrict-volatile-bitfields begins
with the following, right in 20030922-1.c.003t.original:
diff -ru fnsvb/20030922-1.c.003t.original fsvb/20030922-1.c.003t.original
--- fnsvb/20030922-1.c.003t.original2012-04-19 16:51:18.322150866 +0200
+++ fsvb/20030922-1.c.003t.original 2012-04-19 16:49:18.132088498 +0200
@@ -7,7 +7,7 @@
switch ((int) common-code)
{
case 0:;
- if (common-code != 0)
+ if ((BIT_FIELD_REF *common, 32, 0 255) != 0)
{
abort ();
}
That is, for -fno-strict-volatile-bitfields the second instance of
»common-code« it is a component_ref, whereas for
-fstrict-volatile-bitfields it is a bit_field_ref. The former will be
optimized as intended, the latter will not. So, what should be emitted
in the -fstrict-volatile-bitfields case? A component_ref -- but this is
what Bernd's commit r182545 (for PR51200) changed, I think? Or should
later optimization stages learn to better deal with a bit_field_ref, and
where would this have to happen?
Grüße,
Thomas
pgp95HjdOdpvt.pgp
Description: PGP signature
-fdump-tree-original, bit_field_ref (was: Continue strict-volatile-bitfields fixes)
Hi!
On Thu, 19 Apr 2012 19:46:17 +0200, I wrote:
diff -ru fnsvb/20030922-1.c.003t.original fsvb/20030922-1.c.003t.original
--- fnsvb/20030922-1.c.003t.original2012-04-19 16:51:18.322150866 +0200
+++ fsvb/20030922-1.c.003t.original 2012-04-19 16:49:18.132088498 +0200
@@ -7,7 +7,7 @@
switch ((int) common-code)
{
case 0:;
- if (common-code != 0)
+ if ((BIT_FIELD_REF *common, 32, 0 255) != 0)
{
abort ();
}
That is, for -fno-strict-volatile-bitfields the second instance of
»common-code« it is a component_ref, whereas for
-fstrict-volatile-bitfields it is a bit_field_ref.
Looking at 20030922-1.c.001t.tu I see this:
@2838 bit_field_reftype: @25
Where are the operands? Is the following patch correct/OK to check in?
gcc/
* tree-dump.c (dequeue_and_dump) BIT_FIELD_REF: Dump the three child
nodes.
Index: tree-dump.c
===
--- tree-dump.c (revision 186430)
+++ tree-dump.c (working copy)
@@ -596,6 +596,7 @@ dequeue_and_dump (dump_info_p di)
break;
case COMPONENT_REF:
+case BIT_FIELD_REF:
dump_child (op 0, TREE_OPERAND (t, 0));
dump_child (op 1, TREE_OPERAND (t, 1));
dump_child (op 2, TREE_OPERAND (t, 2));
Now I see:
@2838 bit_field_reftype: @25 op 0: @2847op 1: @2848
op 2: @20
Grüße,
Thomas
pgpGosNel0lpz.pgp
Description: PGP signature
Re: PowerPC prologue and epilogue
On Tue, Apr 17, 2012 at 11:08 AM, Alan Modra amo...@gmail.com wrote: This is the first in a series of patches cleaning up rs6000 prologue and epilogue generating code. This one is just the formatting/style changes plus renaming two variables to better reflect their usage, and moving code around. The patch series has been bootstrapped and regression tested powerpc-linux, powerpc64-linux and powerpc-linux-gnuspe. Please test on darwin and aix. * config/rs6000/rs6000.c (rs6000_emit_savres_rtx): Formatting. (rs6000_emit_prologue, rs6000_emit_epilogue): Likewise. Rename sp_offset to frame_off. Move world save code earlier. This first patch in the series is okay. I will start testing the next ones on AIX. Thanks, David
Re: [C++ Patch] PR 52363
On 04/19/2012 08:45 AM, Paolo Carlini wrote:
@@ -812,7 +816,7 @@ build_list_conv (tree type, tree ctor, int flags)
{
conversion *sub
= implicit_conversion (elttype, TREE_TYPE (val), val,
- false, flags);
+ false, flags, tf_warning_or_error);
I'm still seeing several new occurrences of tf_warning_or_error in
overload resolution code. :)
Jason
Patch to fix compiler ICE due to bad PRE
Compiling the attached test case with -O2 using the trunk compiler,
the compiler will ICE. The proposed patch is also attached. The test
is under going, but I like to have discussion on the right fix first.
thanks,
David
Analysis:
-
Here is what is happening:
BB6 : (Successor: BB8)
MEM_19 = phi(MEM_24, MEM_25)
t_9 = operator new (100)@MEM_19 (-- MEM_26)
mem_ref(pp_6, -16).x@MEM_26 = t_9 (--MEM_27) -- (1)
BB8:
...
MEM_20 = PHI(MEM_27, MEM_24)
d.2348_15 = mem_ref(pp_6, -16).x@MEM_20
In the loop header BB3 (which dominates BB6 and BB8),
BB3:
..
pp_31 = phi (pp_6, 0);
...
pp_6 = pp_31 + 16
In PRE, ANTIC_IN(BB8) includes mem_ref(pp_31,0),x@MEM_20. After phi
translate, ANTIC_OUT(BB6) gets mem_ref(pp_31,0).x@MEM_27. However
this expression gets propagated into ANTIC_IN(BB6) even though the
memory expression is killed by (1). The root cause is that the alias
oracle reports that mem_ref(pp_6, -16) and mem_ref(pp_31, 0) are not
aliased as their points-to set do not intersect.
As as consequence of the bad aliasing, the operator new calls gets
transformed into an gimple assignment -- this gimple assignment
becomes the defining statement for MEM_26. In a later UD chain walk,
the memory chain stops (and triggers the assert) at this new
assignment statement because there is no associated vuse for it.
Test case
--
The case is synthesized -- the real world examples involve lots of inlining etc.
int g, *gp[100];
struct V {
int* x;
int y;
};
void foo (V **p, V* end, int i)
{
*p = 0;
V* pp = *p;
int s = 100;
for (; pp end; )
{
pp++;
(pp-1)-x = g;
if (g)
{
if (g10)
g++;
int *t = (int*) operator new (100);
(pp-1)-x = t;
}
else
s--;
gp[end-pp] = (pp-1)-x + s;
}
}
Patch
-
Index: tree-ssa-structalias.c
===
--- tree-ssa-structalias.c (revision 186600)
+++ tree-ssa-structalias.c (working copy)
@@ -6137,6 +6137,9 @@ pt_solutions_intersect_1 (struct pt_solu
if (pt1-anything || pt2-anything)
return true;
+ if (pt1-null pt2-null)
+return true;
+
/* If either points to unknown global memory and the other points to
any global memory they alias. */
if ((pt1-nonlocal
Re: [C++ Patch] PR 52363
Hi,
On 04/19/2012 08:45 AM, Paolo Carlini wrote:
@@ -812,7 +816,7 @@ build_list_conv (tree type, tree ctor, int flags)
{
conversion *sub
= implicit_conversion (elttype, TREE_TYPE (val), val,
- false, flags);
+ false, flags, tf_warning_or_error);
I'm still seeing several new occurrences of tf_warning_or_error in overload
resolution code. :)
Yes, that's life ;) Seriously, if we want to avoid all those having to do with
implicit_conversion in one swoop, the eventual patch will be pretty big, should
I just go on and on?
Paolo
Re: [C++ Patch] PR 52363
On 04/19/2012 03:00 PM, Paolo Carlini wrote: Yes, that's life ;) Seriously, if we want to avoid all those having to do with implicit_conversion in one swoop, the eventual patch will be pretty big, should I just go on and on? I think so. Once we start down this road, we should follow it to the end. Jason
[PATCH] genmodes: don't truncate a mode name of length = 7
Richard Guenther wrote:
Sure, my point was that the
if (strlen (m-name) = sizeof buf)
{
error (%s:%d:mode name \%s\ is too long,
m-file, m-line, m-name);
continue;
}
check does not account for the (conditional) prepending of 'C' and the
snprintf would silently discard the last character of the name.
Here's a patch for that. (passes make bootstrap)
Incidentally it removes the name-length limitation of 7 and it hoists
the xmalloc, which induces the single added free-before-continue.
There was some clean-up along the way: q was easy to eliminate, and
I pulled the two identical snprintf calls out of the if block and
replaced them with the buf[0] assignment and memcpy.
2012-04-19 Jim Meyering meyer...@redhat.com
* genmodes.c (make_complex_modes): Don't truncate a mode name of
length 7 or more when prepending a C. Suggested by Richard Guenther.
gcc/genmodes.c | 32 +---
1 file changed, 13 insertions(+), 19 deletions(-)
diff --git a/gcc/genmodes.c b/gcc/genmodes.c
index 6bbeb6f..84517b9 100644
--- a/gcc/genmodes.c
+++ b/gcc/genmodes.c
@@ -427,7 +427,6 @@ make_complex_modes (enum mode_class cl,
{
struct mode_data *m;
struct mode_data *c;
- char buf[8];
enum mode_class cclass = complex_class (cl);
if (cclass == MODE_RANDOM)
@@ -435,6 +434,7 @@ make_complex_modes (enum mode_class cl,
for (m = modes[cl]; m; m = m-next)
{
+ char *p, *buf;
size_t m_len;
/* Skip BImode. FIXME: BImode probably shouldn't be MODE_INT. */
@@ -442,40 +442,34 @@ make_complex_modes (enum mode_class cl,
continue;
m_len = strlen (m-name);
- if (m_len = sizeof buf)
- {
- error (%s:%d:mode name \%s\ is too long,
-m-file, m-line, m-name);
- continue;
- }
+ /* The leading 1 + is in case we prepend a C below. */
+ buf = (char *) xmalloc (1 + m_len + 1);
/* Float complex modes are named SCmode, etc.
Int complex modes are named CSImode, etc.
This inconsistency should be eliminated. */
+ p = 0;
if (cl == MODE_FLOAT)
{
- char *p, *q = 0;
- /* We verified above that m-name+NUL fits in buf. */
memcpy (buf, m-name, m_len + 1);
p = strchr (buf, 'F');
- if (p == 0)
- q = strchr (buf, 'D');
- if (p == 0 q == 0)
+ if (p == 0 strchr (buf, 'D') == 0)
{
error (%s:%d: float mode \%s\ has no 'F' or 'D',
m-file, m-line, m-name);
+ free (buf);
continue;
}
-
- if (p != 0)
- *p = 'C';
- else
- snprintf (buf, sizeof buf, C%s, m-name);
}
+ if (p != 0)
+ *p = 'C';
else
- snprintf (buf, sizeof buf, C%s, m-name);
+ {
+ buf[0] = 'C';
+ memcpy (buf + 1, m-name, m_len + 1);
+ }
- c = new_mode (cclass, xstrdup (buf), file, line);
+ c = new_mode (cclass, buf, file, line);
c-component = m;
}
}
--
1.7.10.208.gb4267
[RFA] Update config.sub to 2012-04-18 version.
Hello everyone, I wasn't sure if I needed approval for this patch or not, but better be safe than sorry. I'll apply to both GCC and then src when I receive confirmation that it's OK to apply. I would like to update the config.sub script to the latest version from the official config repo. The bits that I need are just the ones related to LynxOS, but I figured it'd be better be consistent with the official version. I can change the patch to just commit the one hunk for LynxOS, if people prefer. But I think we'd become inconsistent. ChangeLog: * config.sub: Update to 2012-04-18 version from official repo. OK to apply? --- config.sub | 42 +++--- 1 files changed, 27 insertions(+), 15 deletions(-) diff --git a/config.sub b/config.sub index 78176a4..59bb593 100755 --- a/config.sub +++ b/config.sub @@ -2,9 +2,9 @@ # Configuration validation subroutine script. # Copyright (C) 1992, 1993, 1994, 1995, 1996, 1997, 1998, 1999, # 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, -# 2011 Free Software Foundation, Inc. +# 2011, 2012 Free Software Foundation, Inc. -timestamp='2011-10-29' +timestamp='2012-04-18' # This file is (in principle) common to ALL GNU software. # The presence of a machine in this file suggests that SOME GNU software @@ -21,9 +21,7 @@ timestamp='2011-10-29' # GNU General Public License for more details. # # You should have received a copy of the GNU General Public License -# along with this program; if not, write to the Free Software -# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA -# 02110-1301, USA. +# along with this program; if not, see http://www.gnu.org/licenses/. # # As a special exception to the GNU General Public License, if you # distribute this file as part of a program that contains a @@ -76,8 +74,8 @@ version=\ GNU config.sub ($timestamp) Copyright (C) 1992, 1993, 1994, 1995, 1996, 1997, 1998, 1999, 2000, -2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 Free -Software Foundation, Inc. +2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012 +Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. @@ -132,6 +130,10 @@ case $maybe_os in os=-$maybe_os basic_machine=`echo $1 | sed 's/^\(.*\)-\([^-]*-[^-]*\)$/\1/'` ;; + android-linux) +os=-linux-android +basic_machine=`echo $1 | sed 's/^\(.*\)-\([^-]*-[^-]*\)$/\1/'`-unknown +;; *) basic_machine=`echo $1 | sed 's/-[^-]*$//'` if [ $basic_machine != $1 ] @@ -223,6 +225,12 @@ case $os in -isc*) basic_machine=`echo $1 | sed -e 's/86-.*/86-pc/'` ;; + -lynx*178) + os=-lynxos178 + ;; + -lynx*5) + os=-lynxos5 + ;; -lynx*) os=-lynxos ;; @@ -247,6 +255,7 @@ case $basic_machine in # Some are omitted here because they have special meanings below. 1750a | 580 \ | a29k \ + | aarch64 | aarch64_be \ | alpha | alphaev[4-8] | alphaev56 | alphaev6[78] | alphapca5[67] \ | alpha64 | alpha64ev[4-8] | alpha64ev56 | alpha64ev6[78] | alpha64pca5[67] \ | am33_2.0 \ @@ -319,8 +328,7 @@ case $basic_machine in c6x) basic_machine=tic6x-unknown ;; - m6811 | m68hc11 | m6812 | m68hc12 | picochip) - # Motorola 68HC11/12. + m6811 | m68hc11 | m6812 | m68hc12 | m68hcs12x | picochip) basic_machine=$basic_machine-unknown os=-none ;; @@ -333,7 +341,10 @@ case $basic_machine in strongarm | thumb | xscale) basic_machine=arm-unknown ;; - + xgate) + basic_machine=$basic_machine-unknown + os=-none + ;; xscaleeb) basic_machine=armeb-unknown ;; @@ -356,6 +367,7 @@ case $basic_machine in # Recognize the basic CPU types with company name. 580-* \ | a29k-* \ + | aarch64-* | aarch64_be-* \ | alpha-* | alphaev[4-8]-* | alphaev56-* | alphaev6[78]-* \ | alpha64-* | alpha64ev[4-8]-* | alpha64ev56-* | alpha64ev6[78]-* \ | alphapca5[67]-* | alpha64pca5[67]-* | arc-* \ @@ -719,7 +731,6 @@ case $basic_machine in i370-ibm* | ibm*) basic_machine=i370-ibm ;; -# I'm not sure what Sysv32 means. Should this be sysv3.2? i*86v32) basic_machine=`echo $1 | sed -e 's/86.*/86-pc/'` os=-sysv32 @@ -816,6 +827,10 @@ case $basic_machine in ms1-*) basic_machine=`echo $basic_machine | sed -e 's/ms1-/mt-/'` ;; + msys) + basic_machine=i386-pc + os=-msys +
MIPS Android patch
Hi Maxim, Richard,
I built cross-toolchains for 3 different targets as follows.
1. mips-linux-gnu
2. mips-linux-gnu --enable-targets=all
3. mips64-linux-gnu
These targets are affected by this MIPS Android patch.
Then, I checked the output from gcc -dumpspecs before and after applying
the patch.
The specs have 6 places of differences for Android due to new defines in
linux-common.h.
I am also building GCC natively, and will test GCC natively later.
Any feedback? Thanks!
Regards,
Chao-ying
libgcc/ChangeLog
2012-04-19 Chao-ying Fu f...@mips.com
* unwind-dw2-fde-dip.c: Define USE_PT_GNU_EH_FRAME for BIONIC.
gcc/ChangeLog
2012-04-19 Chao-ying Fu f...@mips.com
* config.gcc (mips64*-*-linux*): Append mips/linux-common.h to tm_file.
(mips*-*-linux*): Append mips/linux-common.h to tm_file.
* config/mips/gnu-user.h (GNU_USER_SUBTARGET_CC1_SPEC): New define.
(SUBTARGET_CC1_SPEC): Use GNU_USER_SUBTARGET_CC1_SPEC.
(GNU_USER_TARGET_LINK_SPEC): New define.
(LINK_SPEC): Use GNU_USER_TARGET_LINK_SPEC.
(GNU_USER_TARGET_LIB_SPEC): New define.
(LIB_SPEC): Use GNU_USER_TARGET_LIB_SPEC.
(GNU_USER_TARGET_MATHFILE_SPEC): New define.
(ENDFILE_SPEC): Use GNU_USER_TARGET_MATHFILE_SPEC and
GNU_USER_TARGET_ENDFILE_SPEC.
* config/mips/gnu-user64.h (GNU_USER_TARGET_LIB_SPEC): New define.
(LIB_SPEC): Use GNU_USER_TARGET_LIB_SPEC.
(LINK_SPEC): Use GNU_USER_TARGET_LINK_SPEC.
(GNU_USER_TARGET_LIB_SPEC): New define.
* config/mips/linux-common.h: New file.
Index: libgcc/unwind-dw2-fde-dip.c
===
--- libgcc/unwind-dw2-fde-dip.c (revision 186580)
+++ libgcc/unwind-dw2-fde-dip.c (working copy)
@@ -54,6 +54,11 @@
#endif
#if !defined(inhibit_libc) defined(HAVE_LD_EH_FRAME_HDR) \
+ defined(__BIONIC__)
+# define USE_PT_GNU_EH_FRAME
+#endif
+
+#if !defined(inhibit_libc) defined(HAVE_LD_EH_FRAME_HDR) \
defined(__FreeBSD__) __FreeBSD__ = 7
# define ElfW __ElfN
# define USE_PT_GNU_EH_FRAME
Index: gcc/config.gcc
===
--- gcc/config.gcc (revision 186580)
+++ gcc/config.gcc (working copy)
@@ -1712,7 +1712,7 @@
extra_options=${extra_options} netbsd.opt netbsd-elf.opt
;;
mips64*-*-linux* | mipsisa64*-*-linux*)
- tm_file=dbxelf.h elfos.h gnu-user.h linux.h glibc-stdint.h ${tm_file}
mips/gnu-user.h mips/gnu-user64.h mips/linux64.h
+ tm_file=dbxelf.h elfos.h gnu-user.h linux.h glibc-stdint.h ${tm_file}
mips/gnu-user.h mips/gnu-user64.h mips/linux64.h mips/linux-common.h
tmake_file=${tmake_file} mips/t-linux64
tm_defines=${tm_defines} MIPS_ABI_DEFAULT=ABI_N32
case ${target} in
@@ -1738,6 +1738,7 @@
tm_file=${tm_file} mips/gnu-user64.h mips/linux64.h
tmake_file=${tmake_file} mips/t-linux64
fi
+ tm_file=${tm_file} mips/linux-common.h
case ${target} in
mipsisa32r2*)
tm_defines=${tm_defines} MIPS_ISA_DEFAULT=33
Index: gcc/config/mips/gnu-user.h
===
--- gcc/config/mips/gnu-user.h (revision 186580)
+++ gcc/config/mips/gnu-user.h (working copy)
@@ -45,8 +45,10 @@
/* A standard GNU/Linux mapping. On most targets, it is included in
CC1_SPEC itself by config/linux.h, but mips.h overrides CC1_SPEC
and provides this hook instead. */
+#undef GNU_USER_SUBTARGET_CC1_SPEC
+#define GNU_USER_SUBTARGET_CC1_SPEC %{profile:-p}
#undef SUBTARGET_CC1_SPEC
-#define SUBTARGET_CC1_SPEC %{profile:-p}
+#define SUBTARGET_CC1_SPEC GNU_USER_SUBTARGET_CC1_SPEC
/* -G is incompatible with -KPIC which is the default, so only allow objects
in the small data section if the user explicitly asks for it. */
@@ -54,8 +56,8 @@
#define MIPS_DEFAULT_GVALUE 0
/* Borrowed from sparc/linux.h */
-#undef LINK_SPEC
-#define LINK_SPEC \
+#undef GNU_USER_TARGET_LINK_SPEC
+#define GNU_USER_TARGET_LINK_SPEC \
%(endian_spec) \
%{shared:-shared} \
%{!shared: \
@@ -63,6 +65,8 @@
%{rdynamic:-export-dynamic} \
-dynamic-linker GNU_USER_DYNAMIC_LINKER } \
%{static:-static}}
+#undef LINK_SPEC
+#define LINK_SPEC GNU_USER_TARGET_LINK_SPEC
#undef SUBTARGET_ASM_SPEC
#define SUBTARGET_ASM_SPEC \
@@ -89,12 +93,14 @@
#undef ASM_OUTPUT_REG_PUSH
#undef ASM_OUTPUT_REG_POP
-#undef LIB_SPEC
-#define LIB_SPEC \
+#undef GNU_USER_TARGET_LIB_SPEC
+#define GNU_USER_TARGET_LIB_SPEC \
%{pthread:-lpthread} \
%{shared:-lc} \
%{!shared: \
%{profile:-lc_p} %{!profile:-lc}}
+#undef LIB_SPEC
+#define LIB_SPEC GNU_USER_TARGET_LIB_SPEC
#ifdef HAVE_AS_NO_SHARED
/* Default to -mno-shared for non-PIC. */
@@ -133,7 +139,10 @@
LINUX_DRIVER_SELF_SPECS
/* Similar to standard Linux, but adding -ffast-math support. */
+#undef GNU_USER_TARGET_MATHFILE_SPEC
+#define
[PATCH] Fix PR44214
This enhances constant folding for division by complex and vector
constants. When -freciprocal-math is present, such divisions are
converted into multiplies by the constant reciprocal. When an exact
reciprocal is available, this is done for vector constants when
optimizing. I did not implement logic for exact reciprocals of complex
constants because either (a) the complexity doesn't justify the
likelihood of occurrence, or (b) I'm lazy. Your choice. ;)
Bootstrapped with no new regressions on powerpc64-unknown-linux-gnu. Ok
for trunk?
Thanks,
Bill
gcc:
2012-04-19 Bill Schmidt wschm...@linux.vnet.ibm.com
PR rtl-optimization/44214
* fold-const.c (exact_inverse): New function.
(fold_binary_loc): Fold vector and complex division by constant into
multiply by recripocal with flag_reciprocal_math; fold vector division
by constant into multiply by reciprocal with exact inverse.
gcc/testsuite:
2012-04-19 Bill Schmidt wschm...@linux.vnet.ibm.com
PR rtl-optimization/44214
* gcc.target/powerpc/pr44214-1.c: New test.
* gcc.dg/pr44214-2.c: Likewise.
* gcc.target/powerpc/pr44214-3.c: Likewise.
Index: gcc/fold-const.c
===
--- gcc/fold-const.c(revision 186573)
+++ gcc/fold-const.c(working copy)
@@ -9693,6 +9693,48 @@ fold_addr_of_array_ref_difference (location_t loc,
return NULL_TREE;
}
+/* If the real or vector real constant CST of type TYPE has an exact
+ inverse, return it, else return NULL. */
+
+static tree
+exact_inverse (tree type, tree cst)
+{
+ REAL_VALUE_TYPE r;
+ tree unit_type, *elts;
+ enum machine_mode mode;
+ unsigned vec_nelts, i;
+
+ switch (TREE_CODE (cst))
+{
+case REAL_CST:
+ r = TREE_REAL_CST (cst);
+
+ if (exact_real_inverse (TYPE_MODE (type), r))
+ return build_real (type, r);
+
+ return NULL_TREE;
+
+case VECTOR_CST:
+ vec_nelts = VECTOR_CST_NELTS (cst);
+ elts = XALLOCAVEC (tree, vec_nelts);
+ unit_type = TREE_TYPE (type);
+ mode = TYPE_MODE (unit_type);
+
+ for (i = 0; i vec_nelts; i++)
+ {
+ r = TREE_REAL_CST (VECTOR_CST_ELT (cst, i));
+ if (!exact_real_inverse (mode, r))
+ return NULL_TREE;
+ elts[i] = build_real (unit_type, r);
+ }
+
+ return build_vector (type, elts);
+
+default:
+ return NULL_TREE;
+}
+}
+
/* Fold a binary expression of code CODE and type TYPE with operands
OP0 and OP1. LOC is the location of the resulting expression.
Return the folded expression if folding is successful. Otherwise,
@@ -11734,23 +11776,25 @@ fold_binary_loc (location_t loc,
so only do this if -freciprocal-math. We can actually
always safely do it if ARG1 is a power of two, but it's hard to
tell if it is or not in a portable manner. */
- if (TREE_CODE (arg1) == REAL_CST)
+ if (TREE_CODE (arg1) == REAL_CST
+ || (TREE_CODE (arg1) == COMPLEX_CST
+ COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg1)))
+ || (TREE_CODE (arg1) == VECTOR_CST
+ VECTOR_FLOAT_TYPE_P (TREE_TYPE (arg1
{
if (flag_reciprocal_math
- 0 != (tem = const_binop (code, build_real (type, dconst1),
+ 0 != (tem = fold_binary (code, type, build_one_cst (type),
arg1)))
return fold_build2_loc (loc, MULT_EXPR, type, arg0, tem);
- /* Find the reciprocal if optimizing and the result is exact. */
- if (optimize)
+ /* Find the reciprocal if optimizing and the result is exact.
+TODO: Complex reciprocal not implemented. */
+ if (optimize
+ TREE_CODE (arg1) != COMPLEX_CST)
{
- REAL_VALUE_TYPE r;
- r = TREE_REAL_CST (arg1);
- if (exact_real_inverse (TYPE_MODE(TREE_TYPE(arg0)), r))
- {
- tem = build_real (type, r);
- return fold_build2_loc (loc, MULT_EXPR, type,
- fold_convert_loc (loc, type, arg0), tem);
- }
+ tree inverse = exact_inverse (TREE_TYPE (arg0), arg1);
+
+ if (inverse)
+ return fold_build2_loc (loc, MULT_EXPR, type, arg0, inverse);
}
}
/* Convert A/B/C to A/(B*C). */
Index: gcc/testsuite/gcc.target/powerpc/pr44214-3.c
===
--- gcc/testsuite/gcc.target/powerpc/pr44214-3.c(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr44214-3.c(revision 0)
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options -O2 -mcpu=power7 -fdump-tree-optimized } */
+
+void do_div (vector double *a, vector double *b)
+{
+ *a = *b / (vector double) { 2.0, 2.0 };
+}
+
+/* Since 2.0 has an exact reciprocal, constant folding should multiply *b
+ by the
libgo patch committed: Fix testsuite for PPC
The change to the libgo testsuite script to look at data symbols for function descriptors on PowerPC caused the script to pick up interface method tables and the like for types named Test*. This patch adds a grep -v to trim those out again. Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu and powerpc64-unknown-linux-gnu. Committed to mainline and 4.7 branch. Ian diff -r 2ba470ed797c libgo/testsuite/gotest --- a/libgo/testsuite/gotest Mon Apr 16 16:03:29 2012 -0700 +++ b/libgo/testsuite/gotest Thu Apr 19 21:44:55 2012 -0700 @@ -347,18 +347,18 @@ pattern='Test([^a-z].*)?' # The -p option tells GNU nm not to sort. # The -v option tells Solaris nm to sort by value. - tests=$($NM -p -v _gotest_.o $xofile | egrep $text .*\.$pattern'$' | grep -v '\..*\..*\.' | fgrep -v '$' | sed 's/.* //' | sed 's/.*\.\(.*\.\)/\1/') + tests=$($NM -p -v _gotest_.o $xofile | egrep $text .*\.$pattern'$' | grep -v '\..*\..*\.' | fgrep -v '$' | fgrep -v ' __go_' | sed 's/.* //' | sed 's/.*\.\(.*\.\)/\1/') if [ x$tests = x ]; then echo 'gotest: warning: no tests matching '$pattern in _gotest_.o $xofile 12 exit 2 fi # benchmarks are named BenchmarkFoo. pattern='Benchmark([^a-z].*)?' - benchmarks=$($NM -p -v _gotest_.o $xofile | egrep ' T .*\.'$pattern'$' | grep -v '\..*\..*\.' | fgrep -v '$' | sed 's/.* //' | sed 's/.*\.\(.*\.\)/\1/') + benchmarks=$($NM -p -v _gotest_.o $xofile | egrep $test .*\.$pattern'$' | grep -v '\..*\..*\.' | fgrep -v '$' | fgrep -v ' __go_' | sed 's/.* //' | sed 's/.*\.\(.*\.\)/\1/') # examples are named ExampleFoo pattern='Example([^a-z].*)?' - examples=$($NM -p -v _gotest_.o $xofile | egrep ' T .*\.'$pattern'$' | grep -v '\..*\..*\.' | fgrep -v '$' | sed 's/.* //' | sed 's/.*\.\(.*\.\)/\1/') + examples=$($NM -p -v _gotest_.o $xofile | egrep $text .*\.$pattern'$' | grep -v '\..*\..*\.' | fgrep -v '$' | fgrep -v ' __go_' | sed 's/.* //' | sed 's/.*\.\(.*\.\)/\1/') # package spec echo 'package main'
libgo patch committed: Ignore stacks when deciding when to GC
This patch to libgo ignores space allocated for goroutine stacks when
deciding when to next run the garbage collector. On systems that
support split stacks this does nothing, as on those systems the stacks
are allocated by the split stack code. On systems that do not support
split stacks this doesn't let the large amount of memory allocated for
stacks cause the system to delay GC unreasonably.
This patch also does a couple of other minor things. The heap bitmap
bits are allocated in page size units. This fixes a problem on PPC
GNU/Linux which uses a larger page size than x86.
Also this clears the ucontext_t field in the G structure when it is put
on the free list. This prevents the GC from thinking that old register
values, now meaningless, are valid pointers requiring data to be kept in
memory.
Bootstrapped on x86_64-unknown-linux-gnu and
powerpc64-unknown-linux-gnu. Committed to mainline and 4.7 branch.
Ian
diff -r c8d973732b42 libgo/runtime/malloc.goc
--- a/libgo/runtime/malloc.goc Thu Apr 19 21:46:43 2012 -0700
+++ b/libgo/runtime/malloc.goc Thu Apr 19 21:50:27 2012 -0700
@@ -72,7 +72,7 @@
npages = size PageShift;
if((size PageMask) != 0)
npages++;
- s = runtime_MHeap_Alloc(runtime_mheap, npages, 0, !(flag FlagNoGC));
+ s = runtime_MHeap_Alloc(runtime_mheap, npages, 0, 1);
if(s == nil)
runtime_throw(out of memory);
size = npagesPageShift;
diff -r c8d973732b42 libgo/runtime/mgc0.c
--- a/libgo/runtime/mgc0.c Thu Apr 19 21:46:43 2012 -0700
+++ b/libgo/runtime/mgc0.c Thu Apr 19 21:50:27 2012 -0700
@@ -4,6 +4,8 @@
// Garbage collector.
+#include unistd.h
+
#include runtime.h
#include arch.h
#include malloc.h
@@ -918,7 +920,7 @@
uint64 stacks_sys;
stacks_inuse = 0;
- stacks_sys = 0;
+ stacks_sys = runtime_stacks_sys;
for(m=runtime_allm; m; m=m-alllink) {
runtime_purgecachedstats(m);
// stacks_inuse += m-stackalloc-inuse;
@@ -1020,7 +1022,7 @@
stealcache();
cachestats();
- mstats.next_gc = mstats.heap_alloc+mstats.heap_alloc*gcpercent/100;
+ mstats.next_gc = mstats.heap_alloc+(mstats.heap_alloc-runtime_stacks_sys)*gcpercent/100;
m-gcing = 0;
m-locks++; // disable gc during the mallocs in newproc
@@ -1329,6 +1331,8 @@
void
runtime_MHeap_MapBits(MHeap *h)
{
+ size_t page_size;
+
// Caller has added extra mappings to the arena.
// Add extra mappings of bitmap words as needed.
// We allocate extra bitmap pieces in chunks of bitmapChunk.
@@ -1342,6 +1346,9 @@
if(h-bitmap_mapped = n)
return;
+ page_size = getpagesize();
+ n = (n+page_size-1) ~(page_size-1);
+
runtime_SysMap(h-arena_start - n, n - h-bitmap_mapped);
h-bitmap_mapped = n;
}
diff -r c8d973732b42 libgo/runtime/proc.c
--- a/libgo/runtime/proc.c Thu Apr 19 21:46:43 2012 -0700
+++ b/libgo/runtime/proc.c Thu Apr 19 21:50:27 2012 -0700
@@ -46,6 +46,8 @@
# define StackMin 2 * 1024 * 1024
#endif
+uintptr runtime_stacks_sys;
+
static void schedule(G*);
typedef struct Sched Sched;
@@ -1091,6 +1093,7 @@
m-lockedg = nil;
}
gp-idlem = nil;
+ runtime_memclr(gp-context, sizeof gp-context);
gfput(gp);
if(--runtime_sched.gcount == 0)
runtime_exit(0);
@@ -1288,6 +1291,7 @@
*ret_stacksize = stacksize;
newg-gcinitial_sp = *ret_stack;
newg-gcstack_size = stacksize;
+ runtime_xadd(runtime_stacks_sys, stacksize);
#endif
}
return newg;
diff -r c8d973732b42 libgo/runtime/runtime.h
--- a/libgo/runtime/runtime.h Thu Apr 19 21:46:43 2012 -0700
+++ b/libgo/runtime/runtime.h Thu Apr 19 21:50:27 2012 -0700
@@ -463,3 +463,8 @@
};
void __go_register_gc_roots(struct root_list*);
+
+// Size of stack space allocated using Go's allocator.
+// This will be 0 when using split stacks, as in that case
+// the stacks are allocated by the splitstack library.
+extern uintptr runtime_stacks_sys;
[PATCH, reload] Fix bug pr52804, RELOAD pass reloads wrong register on ARM for cortex-m0
Hi, Previously I reported pr52804 in bugzilla about reload pass reloads wrong register. After investigation I believe it is a bug in reload pass and here comes the patch. You can refer to http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52804 for details. In short, I think the confliction of reloads with type RELOAD_FOR_INPADDR_ADDRESS and type RELOAD_FOR_INPUT_ADDRESS should be handled in reload_reg_reaches_end_p. Also I think RELOAD_FOR_OUTPUT_ADDRESS/RELOAD_FOR_OUTADDR_ADDRESS have the issue symmetrically, though I have no test case for it. I have tested the patch for x86 and arm. Is it OK? I think it is a bug of reload, and I understand reload pass should be rarely touched, so any comments are highly appreciated on this topic. Thanks very much. 2012-04-20 Bin Cheng bin.ch...@arm.com PR target/52804 * reload1.c (reload_reg_reaches_end_p): Check whether successor reload with type RELOAD_FOR_INPUT_ADDRESS kills reload register of current one with type RELOAD_FOR_INPADDR_ADDRESS.Index: gcc/reload1.c === --- gcc/reload1.c (revision 185992) +++ gcc/reload1.c (working copy) @@ -5429,6 +5429,13 @@ if (TEST_HARD_REG_BIT (reload_reg_used_in_input[i], regno)) return 0; + /* Reload register of reload with type RELOAD_FOR_INPADDR_ADDRESS +could be killed if the register is also used by reload with type +RELOAD_FOR_INPUT_ADDRESS, so check it. */ + if (type == RELOAD_FOR_INPADDR_ADDRESS + TEST_HARD_REG_BIT (reload_reg_used_in_input_addr[opnum], regno)) + return 0; + for (i = opnum + 1; i reload_n_operands; i++) if (TEST_HARD_REG_BIT (reload_reg_used_in_input_addr[i], regno) || TEST_HARD_REG_BIT (reload_reg_used_in_inpaddr_addr[i], regno)) @@ -5503,6 +5510,13 @@ || TEST_HARD_REG_BIT (reload_reg_used_in_outaddr_addr[i], regno)) return 0; + /* Reload register of reload with type RELOAD_FOR_OUTADDR_ADDRESS +could be killed if the register is also used by reload with type +RELOAD_FOR_OUTPUT_ADDRESS, so check it. */ + if (type == RELOAD_FOR_OUTADDR_ADDRESS + TEST_HARD_REG_BIT (reload_reg_used_in_outaddr_addr[opnum], regno)) + return 0; + return 1; default:
