Re: [patch] --enable-dynamic-string default for mingw-w64 v2
On 10/01/2011 08:03 AM, JonY wrote: Hi, I followed Paolo's suggestion with the os_defines.h trick. I duplicated os/mingw32/ to os/mingw32-w64/ for this to work, since there aren't any built-in defines to tell the 2 apart unless you include some headers like _mingw.h. Patch attached, comments? These: +#if !defined (_GLIBCXX_FULLY_DYNAMIC_STRING) || (_GLIBCXX_FULLY_DYNAMIC_STRING == 0) can be simplified to just: +#if _GLIBCXX_FULLY_DYNAMIC_STRING == 0 Ok with those changes and a proper ChangeLog entry, of course. Thanks, Paolo.
Re: [patch] --enable-dynamic-string default for mingw-w64 v2
On 10/1/2011 17:15, Paolo Carlini wrote:
> On 10/01/2011 08:03 AM, JonY wrote:
>> Hi,
>>
>> I followed Paolo's suggestion with the os_defines.h trick. I duplicated
>> os/mingw32/ to os/mingw32-w64/ for this to work, since there aren't any
>> built-in defines to tell the 2 apart unless you include some headers
>> like _mingw.h.
>>
>> Patch attached, comments?
> These:
>
> +#if !defined (_GLIBCXX_FULLY_DYNAMIC_STRING) ||
> (_GLIBCXX_FULLY_DYNAMIC_STRING == 0)
>
>
> can be simplified to just:
>
> +#if _GLIBCXX_FULLY_DYNAMIC_STRING == 0
>
>
> Ok with those changes and a proper ChangeLog entry, of course.
>
OK, removed the !defined part and included the ChangeLog, everything OK?
Index: configure.host
===
--- configure.host (revision 179411)
+++ configure.host (working copy)
@@ -260,8 +260,15 @@
atomic_word_dir=os/irix
;;
mingw32*)
-os_include_dir="os/mingw32"
-error_constants_dir="os/mingw32"
+case "$host" in
+ *-w64-*)
+os_include_dir="os/mingw32-w64"
+error_constants_dir="os/mingw32-w64"
+;;
+ *)
+os_include_dir="os/mingw32"
+error_constants_dir="os/mingw32"
+;;
OPT_LDFLAGS="${OPT_LDFLAGS} \$(lt_host_flags)"
;;
netbsd*)
Index: include/bits/basic_string.h
===
--- include/bits/basic_string.h (revision 179411)
+++ include/bits/basic_string.h (working copy)
@@ -201,7 +201,7 @@
void
_M_set_length_and_sharable(size_type __n)
{
-#ifndef _GLIBCXX_FULLY_DYNAMIC_STRING
+#if _GLIBCXX_FULLY_DYNAMIC_STRING == 0
if (__builtin_expect(this != &_S_empty_rep(), false))
#endif
{
@@ -231,7 +231,7 @@
void
_M_dispose(const _Alloc& __a)
{
-#ifndef _GLIBCXX_FULLY_DYNAMIC_STRING
+#if _GLIBCXX_FULLY_DYNAMIC_STRING == 0
if (__builtin_expect(this != &_S_empty_rep(), false))
#endif
{
@@ -252,7 +252,7 @@
_CharT*
_M_refcopy() throw()
{
-#ifndef _GLIBCXX_FULLY_DYNAMIC_STRING
+#if _GLIBCXX_FULLY_DYNAMIC_STRING == 0
if (__builtin_expect(this != &_S_empty_rep(), false))
#endif
__gnu_cxx::__atomic_add_dispatch(&this->_M_refcount, 1);
@@ -430,7 +430,7 @@
* @brief Default constructor creates an empty string.
*/
basic_string()
-#ifndef _GLIBCXX_FULLY_DYNAMIC_STRING
+#if _GLIBCXX_FULLY_DYNAMIC_STRING == 0
: _M_dataplus(_S_empty_rep()._M_refdata(), _Alloc()) { }
#else
: _M_dataplus(_S_construct(size_type(), _CharT(), _Alloc()), _Alloc()){ }
@@ -502,7 +502,7 @@
basic_string(basic_string&& __str) noexcept
: _M_dataplus(__str._M_dataplus)
{
-#ifndef _GLIBCXX_FULLY_DYNAMIC_STRING
+#if _GLIBCXX_FULLY_DYNAMIC_STRING == 0
__str._M_data(_S_empty_rep()._M_refdata());
#else
__str._M_data(_S_construct(size_type(), _CharT(), get_allocator()));
Index: include/bits/basic_string.tcc
===
--- include/bits/basic_string.tcc (revision 179411)
+++ include/bits/basic_string.tcc (working copy)
@@ -80,7 +80,7 @@
_S_construct(_InIterator __beg, _InIterator __end, const _Alloc& __a,
input_iterator_tag)
{
-#ifndef _GLIBCXX_FULLY_DYNAMIC_STRING
+#if _GLIBCXX_FULLY_DYNAMIC_STRING == 0
if (__beg == __end && __a == _Alloc())
return _S_empty_rep()._M_refdata();
#endif
@@ -126,7 +126,7 @@
_S_construct(_InIterator __beg, _InIterator __end, const _Alloc& __a,
forward_iterator_tag)
{
-#ifndef _GLIBCXX_FULLY_DYNAMIC_STRING
+#if _GLIBCXX_FULLY_DYNAMIC_STRING == 0
if (__beg == __end && __a == _Alloc())
return _S_empty_rep()._M_refdata();
#endif
@@ -154,7 +154,7 @@
basic_string<_CharT, _Traits, _Alloc>::
_S_construct(size_type __n, _CharT __c, const _Alloc& __a)
{
-#ifndef _GLIBCXX_FULLY_DYNAMIC_STRING
+#if _GLIBCXX_FULLY_DYNAMIC_STRING == 0
if (__n == 0 && __a == _Alloc())
return _S_empty_rep()._M_refdata();
#endif
@@ -456,7 +456,7 @@
basic_string<_CharT, _Traits, _Alloc>::
_M_leak_hard()
{
-#ifndef _GLIBCXX_FULLY_DYNAMIC_STRING
+#if _GLIBCXX_FULLY_DYNAMIC_STRING == 0
if (_M_rep() == &_S_empty_rep())
return;
#endif
Index: ChangeLog
===
--- ChangeLog (revision 179411)
+++ ChangeLog (working copy)
@@ -1,3 +1,16 @@
+2011-10-01 Jonathan Yong
+
+ * configure.host: Use config/os/mingw32-w64 instead of
+ config/os/mingw32 if vendor key is "w64".
+ * config/os/mingw32-w64: Duplicate from config/os/mingw32.
+ * config/os/mingw32-w64/os_defines.h: Enable
+ _GLIBCXX_FULLY_DYNAMIC_STRING if undefined.
+ * acinclude.m4: Set fully-dynamic-string to 1 when enabled,
+ 0 when disabled or undefined if
Re: [patch] --enable-dynamic-string default for mingw-w64 v2
On 10/01/2011 11:48 AM, JonY wrote: On 10/1/2011 17:15, Paolo Carlini wrote: On 10/01/2011 08:03 AM, JonY wrote: Hi, I followed Paolo's suggestion with the os_defines.h trick. I duplicated os/mingw32/ to os/mingw32-w64/ for this to work, since there aren't any built-in defines to tell the 2 apart unless you include some headers like _mingw.h. Patch attached, comments? These: +#if !defined (_GLIBCXX_FULLY_DYNAMIC_STRING) || (_GLIBCXX_FULLY_DYNAMIC_STRING == 0) can be simplified to just: +#if _GLIBCXX_FULLY_DYNAMIC_STRING == 0 Ok with those changes and a proper ChangeLog entry, of course. OK, removed the !defined part and included the ChangeLog, everything OK? You should have also regenerated configure and config.h.in, right? Otherwise, patch looks good to me, but I guess we need to hear from a mingw maintainer. Kai? Paolo.
Re: [patch] --enable-dynamic-string default for mingw-w64 v2
On Saturday 01 October 2011 07:03:35, JonY wrote: > Hi, > > I followed Paolo's suggestion with the os_defines.h trick. I duplicated > os/mingw32/ to os/mingw32-w64/ for this to work, since there aren't any > built-in defines to tell the 2 apart unless you include some headers > like _mingw.h. Are we really introducing a bunch of duplication between os/mingw32/ and os/mingw32-w64/ ? I didn't see the part that adds the new dir and does all those copies in the patch; where is it? Or have I missed something? Can't we make configure add -D__IM_REALLY_W64_YOU_KNOW to CFLAGS instead? Or come up with a way to point libstd++ to pick up a new mingw32/os_defines_w64.h file instead that does: #include "os_defines.h" // mingw-w64 should use fully-dynamic-string by default #ifndef _GLIBCXX_FULLY_DYNAMIC_STRING #define _GLIBCXX_FULLY_DYNAMIC_STRING 1 #endif -- Pedro Alves
Re: [patch] --enable-dynamic-string default for mingw-w64 v2
On 10/1/2011 18:08, Paolo Carlini wrote: > On 10/01/2011 11:48 AM, JonY wrote: >> On 10/1/2011 17:15, Paolo Carlini wrote: >>> On 10/01/2011 08:03 AM, JonY wrote: Hi, I followed Paolo's suggestion with the os_defines.h trick. I duplicated os/mingw32/ to os/mingw32-w64/ for this to work, since there aren't any built-in defines to tell the 2 apart unless you include some headers like _mingw.h. Patch attached, comments? >>> These: >>> >>> +#if !defined (_GLIBCXX_FULLY_DYNAMIC_STRING) || >>> (_GLIBCXX_FULLY_DYNAMIC_STRING == 0) >>> >>> >>> can be simplified to just: >>> >>> +#if _GLIBCXX_FULLY_DYNAMIC_STRING == 0 >>> >>> >>> Ok with those changes and a proper ChangeLog entry, of course. >>> >> OK, removed the !defined part and included the ChangeLog, everything OK? > You should have also regenerated configure and config.h.in, right? > > Otherwise, patch looks good to me, but I guess we need to hear from a > mingw maintainer. Kai? > > Paolo. > Thanks, but I am having problems sending a proper diff with the regenerated files, they have a lot of unrelated, even if I made sure I am using autoconf 2.64 and automake 1.11.1. I guess its due to libtool versions, any ideas how to fix this? signature.asc Description: OpenPGP digital signature
Re: [patch] --enable-dynamic-string default for mingw-w64 v2
On 10/1/2011 18:33, Pedro Alves wrote: > On Saturday 01 October 2011 07:03:35, JonY wrote: >> Hi, >> >> I followed Paolo's suggestion with the os_defines.h trick. I duplicated >> os/mingw32/ to os/mingw32-w64/ for this to work, since there aren't any >> built-in defines to tell the 2 apart unless you include some headers >> like _mingw.h. > > Are we really introducing a bunch of duplication between > os/mingw32/ and os/mingw32-w64/ ? I didn't see the part that adds the > new dir and does all those copies in the patch; where is it? Or have > I missed something? Can't we make configure add > -D__IM_REALLY_W64_YOU_KNOW to CFLAGS instead? Or come up with a way > to point libstd++ to pick up a new mingw32/os_defines_w64.h file instead > that does: > > #include "os_defines.h" > // mingw-w64 should use fully-dynamic-string by default > #ifndef _GLIBCXX_FULLY_DYNAMIC_STRING > #define _GLIBCXX_FULLY_DYNAMIC_STRING 1 > #endif > The new files are missing from svn diff because I used svn copy to copy the directories, svn diff didn't show them, should I use something else instead? IMHO, mingw.org and mingw-w64 may or may not diverge further in the future, so sprinkling defines to codes isn't very good in the long run. signature.asc Description: OpenPGP digital signature
Re: [patch] --enable-dynamic-string default for mingw-w64 v2
Hi, > Thanks, but I am having problems sending a proper diff with the > regenerated files, they have a lot of unrelated, even if I made sure I > am using autoconf 2.64 and automake 1.11.1. To be clear, regenerated files should **not** be part of the patch submitted for review, but should definitely appear on the ChangeLog and the changes eventually committed. > I guess its due to libtool versions, any ideas how to fix this? Nope, sorry, on Linux, provided the versions are correct - please double check that, I'm traveling - regen works like a charm. You just invoke autoreconf, right? Paolo
[Patch, Fortran, committed] PR 50585: [4.6/4.7 Regression] ICE with assumed length character array argument
Hi all,
I have just committed as obvious a one-line patch to fix a regression
which is triggered by -fwhole-file:
http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=179413
Should I backport to 4.6? And also to 4.5?
Moreover, I noticed that
http://gcc.gnu.org/onlinedocs/gcc-4.6.1/gfortran/Code-Gen-Options.html#Code-Gen-Options
falsely claims that -fwhole-file is not used by default (which it is
since 4.6, see http://gcc.gnu.org/gcc-4.6/changes.html). Attached is a
small documentation patch to fix this. Ok to apply to trunk and 4.6?
Cheers,
Janus
Index: gcc/fortran/invoke.texi
===
--- gcc/fortran/invoke.texi (revision 179412)
+++ gcc/fortran/invoke.texi (working copy)
@@ -1227,10 +1227,8 @@
@item -fwhole-file
@opindex @code{fwhole-file}
-By default, GNU Fortran parses, resolves and translates each procedure
-in a file separately. Using this option modifies this such that the
-whole file is parsed and placed in a single front-end tree. During
-resolution, in addition to all the usual checks and fixups, references
+By default, the whole file is parsed and placed in a single front-end tree.
+During resolution, in addition to all the usual checks and fixups, references
to external procedures that are in the same file effect resolution of
that procedure, if not already done, and a check of the interfaces. The
dependences are resolved by changing the order in which the file is
@@ -1238,6 +1236,9 @@
is translated before the reference and the duplication of backend tree
declarations eliminated.
+This default behavior can be modified by @code{-fno-whole-file} to resolve and
+translate each procedure in a file separately.
+
@item -fsecond-underscore
@opindex @code{fsecond-underscore}
@cindex underscore
Re: Disable early inlining while compiling for coverage (issue5173042)
> Yes, this will improve test coverage option's usability, but please
> provide the example to explain the issues.
>
> David
>
> On Fri, Sep 30, 2011 at 6:12 PM, Sharad Singhai wrote:
> > This patch disables early inlining when --coverage option is
> > specified. This improves coverage data in presence of other
> > optimizations, specially with -O2 where early inlining changes the
> > control flow graph sufficiently enough to generate seemingly very odd
> > source coverage.
BTW early inlining was introduced to make coverage possible on some C++
sources, like tramp3d ;) However the problem here is that since we moved
coverage to SSA, we do it too late. My longer term plan is to separate
coverage and profile feedback passes (so they can't be done both together) and
instrument early for coverage & ensure that everything before coverage is done
is save WRT line info. Inlining alone does not mess up the line info, but most
optimizations we have in early optimization queue do.
We discussed it back when Richi implemented SSA profiling but we didn't do that
basically due to lack of testcases. Would be possible to take one you have
and fill in some PRs? Those are regressions WRT pre-SSA profiling releases (I
think 4.5?)
Honza
> >
> > Bootstrapped okay and regression tests passed.
> >
> > Okay for google/gcc-4_6?
> >
> > 2011-09-30 Sharad Singhai
> >
> > * gcc.c (cc1_options): Added -fno-early-inlining for coverage.
> >
> > Index: gcc.c
> > ===
> > --- gcc.c (revision 179402)
> > +++ gcc.c (working copy)
> > @@ -776,7 +776,7 @@
> > %{!fsyntax-only:%{S:%W{o*}%{!o*:-o %b.s}}}\
> > %{fsyntax-only:-o %j} %{-param*}\
> > %{fmudflap|fmudflapth:-fno-builtin -fno-merge-constants}\
> > - %{coverage:-fprofile-arcs -ftest-coverage}";
> > + %{coverage:-fprofile-arcs -ftest-coverage -fno-early-inlining}";
> >
> > /* If an assembler wrapper is used to invoke post-assembly tools
> > like MAO, --save-temps need to be passed to save the output of
> >
> > --
> > This patch is available for review at http://codereview.appspot.com/5173042
> >
Re: [patch] --enable-dynamic-string default for mingw-w64 v2
On Saturday 01 October 2011 12:15:42, JonY wrote: > On 10/1/2011 18:33, Pedro Alves wrote: > > On Saturday 01 October 2011 07:03:35, JonY wrote: > >> Hi, > >> > >> I followed Paolo's suggestion with the os_defines.h trick. I duplicated > >> os/mingw32/ to os/mingw32-w64/ for this to work, since there aren't any > >> built-in defines to tell the 2 apart unless you include some headers > >> like _mingw.h. > > > > Are we really introducing a bunch of duplication between > > os/mingw32/ and os/mingw32-w64/ ? I didn't see the part that adds the > > new dir and does all those copies in the patch; where is it? Or have > > I missed something? Can't we make configure add > > -D__IM_REALLY_W64_YOU_KNOW to CFLAGS instead? Or come up with a way > > to point libstd++ to pick up a new mingw32/os_defines_w64.h file instead > > that does: > > > > #include "os_defines.h" > > // mingw-w64 should use fully-dynamic-string by default > > #ifndef _GLIBCXX_FULLY_DYNAMIC_STRING > > #define _GLIBCXX_FULLY_DYNAMIC_STRING 1 > > #endif > > > > The new files are missing from svn diff because I used svn copy to copy > the directories, svn diff didn't show them, should I use something else > instead? So that'd be a patch with its own ChangeLog, as your patch applies on top of that already. > IMHO, mingw.org and mingw-w64 may or may not diverge further in the > future, so sprinkling defines to codes isn't very good in the long run. "may or may not" is key. We don't know the future, but we know the present. We do know that code duplication is bad. I can just as well say, IMHO, mingw.org and mingw-w64 may or may not diverge further in the future (ideally they wouldn't), so adding code duplication when we only need one define isn't very good in the long run. But I'm not a maintainer, so I shall just go away. -- Pedro Alves
Re: [google] Add intermediate text format for gcov (issue4595053)
2011/6/14 Sharad Singhai (शरद सिंघई) : > Sorry, Rietveld didn't send out the updated patch along with my mail. > Here it is. > Hi, I tried this patch out on trunk it applies alright, and appears to work fine, (haven't run the testsuite though) any plans on submitting it for inclusion with mainline gcc?
Re: [RFC] Builtin infrastructure change
On Fri, 30 Sep 2011, Michael Meissner wrote: > Is this enough of a savings to continue on? I'm of two minds about it, one is The thing to measure is not so much memory as startup time (how long it takes to compile an empty source file), which is important for libraries and programs using a coding style with lots of small source files. -- Joseph S. Myers jos...@codesourcery.com
Re: [patch] --enable-dynamic-string default for mingw-w64 v2
On 10/1/2011 19:16, Paolo Carlini wrote:
> Hi,
>
>> Thanks, but I am having problems sending a proper diff with the
>> regenerated files, they have a lot of unrelated, even if I made sure I
>> am using autoconf 2.64 and automake 1.11.1.
>
> To be clear, regenerated files should **not** be part of the patch submitted
> for review, but should definitely appear on the ChangeLog and the changes
> eventually committed.
>
After some careful adjustments, I see just configure and config.h.in
changed.
New patch with updated Changelog attached. Be sure to copy
config/os/mingw32 over to config/os/mingw32-w64 before continuing.
>> I guess its due to libtool versions, any ideas how to fix this?
>
> Nope, sorry, on Linux, provided the versions are correct - please double
> check that, I'm traveling - regen works like a charm. You just invoke
> autoreconf, right?
>
> Paolo
Yeah, just "autoreconf" under Cygwin to regenerate them, calling
autoconf and autoheader avoids new libtool getting pulled in.
Index: configure.host
===
--- configure.host (revision 179411)
+++ configure.host (working copy)
@@ -260,8 +260,15 @@
atomic_word_dir=os/irix
;;
mingw32*)
-os_include_dir="os/mingw32"
-error_constants_dir="os/mingw32"
+case "$host" in
+ *-w64-*)
+os_include_dir="os/mingw32-w64"
+error_constants_dir="os/mingw32-w64"
+;;
+ *)
+os_include_dir="os/mingw32"
+error_constants_dir="os/mingw32"
+;;
OPT_LDFLAGS="${OPT_LDFLAGS} \$(lt_host_flags)"
;;
netbsd*)
Index: include/bits/basic_string.h
===
--- include/bits/basic_string.h (revision 179411)
+++ include/bits/basic_string.h (working copy)
@@ -201,7 +201,7 @@
void
_M_set_length_and_sharable(size_type __n)
{
-#ifndef _GLIBCXX_FULLY_DYNAMIC_STRING
+#if _GLIBCXX_FULLY_DYNAMIC_STRING == 0
if (__builtin_expect(this != &_S_empty_rep(), false))
#endif
{
@@ -231,7 +231,7 @@
void
_M_dispose(const _Alloc& __a)
{
-#ifndef _GLIBCXX_FULLY_DYNAMIC_STRING
+#if _GLIBCXX_FULLY_DYNAMIC_STRING == 0
if (__builtin_expect(this != &_S_empty_rep(), false))
#endif
{
@@ -252,7 +252,7 @@
_CharT*
_M_refcopy() throw()
{
-#ifndef _GLIBCXX_FULLY_DYNAMIC_STRING
+#if _GLIBCXX_FULLY_DYNAMIC_STRING == 0
if (__builtin_expect(this != &_S_empty_rep(), false))
#endif
__gnu_cxx::__atomic_add_dispatch(&this->_M_refcount, 1);
@@ -430,7 +430,7 @@
* @brief Default constructor creates an empty string.
*/
basic_string()
-#ifndef _GLIBCXX_FULLY_DYNAMIC_STRING
+#if _GLIBCXX_FULLY_DYNAMIC_STRING == 0
: _M_dataplus(_S_empty_rep()._M_refdata(), _Alloc()) { }
#else
: _M_dataplus(_S_construct(size_type(), _CharT(), _Alloc()), _Alloc()){ }
@@ -502,7 +502,7 @@
basic_string(basic_string&& __str) noexcept
: _M_dataplus(__str._M_dataplus)
{
-#ifndef _GLIBCXX_FULLY_DYNAMIC_STRING
+#if _GLIBCXX_FULLY_DYNAMIC_STRING == 0
__str._M_data(_S_empty_rep()._M_refdata());
#else
__str._M_data(_S_construct(size_type(), _CharT(), get_allocator()));
Index: include/bits/basic_string.tcc
===
--- include/bits/basic_string.tcc (revision 179411)
+++ include/bits/basic_string.tcc (working copy)
@@ -80,7 +80,7 @@
_S_construct(_InIterator __beg, _InIterator __end, const _Alloc& __a,
input_iterator_tag)
{
-#ifndef _GLIBCXX_FULLY_DYNAMIC_STRING
+#if _GLIBCXX_FULLY_DYNAMIC_STRING == 0
if (__beg == __end && __a == _Alloc())
return _S_empty_rep()._M_refdata();
#endif
@@ -126,7 +126,7 @@
_S_construct(_InIterator __beg, _InIterator __end, const _Alloc& __a,
forward_iterator_tag)
{
-#ifndef _GLIBCXX_FULLY_DYNAMIC_STRING
+#if _GLIBCXX_FULLY_DYNAMIC_STRING == 0
if (__beg == __end && __a == _Alloc())
return _S_empty_rep()._M_refdata();
#endif
@@ -154,7 +154,7 @@
basic_string<_CharT, _Traits, _Alloc>::
_S_construct(size_type __n, _CharT __c, const _Alloc& __a)
{
-#ifndef _GLIBCXX_FULLY_DYNAMIC_STRING
+#if _GLIBCXX_FULLY_DYNAMIC_STRING == 0
if (__n == 0 && __a == _Alloc())
return _S_empty_rep()._M_refdata();
#endif
@@ -456,7 +456,7 @@
basic_string<_CharT, _Traits, _Alloc>::
_M_leak_hard()
{
-#ifndef _GLIBCXX_FULLY_DYNAMIC_STRING
+#if _GLIBCXX_FULLY_DYNAMIC_STRING == 0
if (_M_rep() == &_S_empty_rep())
return;
#endif
Index: ChangeLog
===
--- ChangeLog (revision 179411)
+++ ChangeLog (working copy)
@@ -1,3 +1,18 @@
+2011-10-01 Jonathan Yong
+
+ * configure.host: Use config/os/mingw32-w64 instead of
+ c
Re: [patch] --enable-dynamic-string default for mingw-w64 v2
2011/10/1 Pedro Alves : > On Saturday 01 October 2011 12:15:42, JonY wrote: >> On 10/1/2011 18:33, Pedro Alves wrote: >> > On Saturday 01 October 2011 07:03:35, JonY wrote: >> >> Hi, >> >> >> >> I followed Paolo's suggestion with the os_defines.h trick. I duplicated >> >> os/mingw32/ to os/mingw32-w64/ for this to work, since there aren't any >> >> built-in defines to tell the 2 apart unless you include some headers >> >> like _mingw.h. >> > >> > Are we really introducing a bunch of duplication between >> > os/mingw32/ and os/mingw32-w64/ ? I didn't see the part that adds the >> > new dir and does all those copies in the patch; where is it? Or have >> > I missed something? Can't we make configure add >> > -D__IM_REALLY_W64_YOU_KNOW to CFLAGS instead? Or come up with a way >> > to point libstd++ to pick up a new mingw32/os_defines_w64.h file instead >> > that does: >> > >> > #include "os_defines.h" >> > // mingw-w64 should use fully-dynamic-string by default >> > #ifndef _GLIBCXX_FULLY_DYNAMIC_STRING >> > #define _GLIBCXX_FULLY_DYNAMIC_STRING 1 >> > #endif >> > >> >> The new files are missing from svn diff because I used svn copy to copy >> the directories, svn diff didn't show them, should I use something else >> instead? > > So that'd be a patch with its own ChangeLog, as your patch applies on > top of that already. > >> IMHO, mingw.org and mingw-w64 may or may not diverge further in the >> future, so sprinkling defines to codes isn't very good in the long run. > > "may or may not" is key. We don't know the future, but we know > the present. We do know that code duplication is bad. I can just > as well say, Well, this situation isn't ideal. It might be that one day mingw.org and mingw-w64 venture come more near in feature-set. But this is for sure more a long-term issue and not a short or medium-term thing. > IMHO, mingw.org and mingw-w64 may or may not diverge further in the > future (ideally they wouldn't), so adding code duplication when > we only need one define isn't very good in the long run. > > But I'm not a maintainer, so I shall just go away. Well, we diverge already more and more. I plan to provide for libstdc++ some new printf/scanf API features for wide and ascii variants, which is present in mingw-w64 venture, but not in mingw.org venture. We have also other divergencies in other feature-set, which lead already to an add-on header in gcc for specific mingw-w64 targets. Kai
[PATCH] Wire-up missing ARM iwmmxt intrinsics (bugs 35294, 36798, 36966)
--- arm.c.orig 2011-05-05 04:39:40.0 -0400 +++ arm.c 2011-08-19 13:48:21.548405102 -0400 @@ -19218,7 +19218,8 @@ || ! (*insn_data[icode].operand[0].predicate) (target, tmode)) target = gen_reg_rtx (tmode); - gcc_assert (GET_MODE (op0) == mode0 && GET_MODE (op1) == mode1); + gcc_assert ((GET_MODE (op0) == mode0 || GET_MODE (op0) == VOIDmode) + && (GET_MODE (op1) == mode1 || GET_MODE (op1) == VOIDmode)); if (! (*insn_data[icode].operand[1].predicate) (op0, mode0)) op0 = copy_to_mode_reg (mode0, op0); @@ -19814,6 +19815,65 @@ emit_insn (pat); return target; +case ARM_BUILTIN_WSLLH: +case ARM_BUILTIN_WSLLHI: +case ARM_BUILTIN_WSLLW: +case ARM_BUILTIN_WSLLWI: +case ARM_BUILTIN_WSLLD: +case ARM_BUILTIN_WSLLDI: +case ARM_BUILTIN_WSRAH: +case ARM_BUILTIN_WSRAHI: +case ARM_BUILTIN_WSRAW: +case ARM_BUILTIN_WSRAWI: +case ARM_BUILTIN_WSRAD: +case ARM_BUILTIN_WSRADI: +case ARM_BUILTIN_WSRLH: +case ARM_BUILTIN_WSRLHI: +case ARM_BUILTIN_WSRLW: +case ARM_BUILTIN_WSRLWI: +case ARM_BUILTIN_WSRLD: +case ARM_BUILTIN_WSRLDI: +case ARM_BUILTIN_WRORH: +case ARM_BUILTIN_WRORHI: +case ARM_BUILTIN_WRORW: +case ARM_BUILTIN_WRORWI: +case ARM_BUILTIN_WRORD: +case ARM_BUILTIN_WRORDI: +case ARM_BUILTIN_WAND: +case ARM_BUILTIN_WANDN: +case ARM_BUILTIN_WOR: +case ARM_BUILTIN_WXOR: + icode = (fcode == ARM_BUILTIN_WSLLH ? CODE_FOR_ashlv4hi3_di + : fcode == ARM_BUILTIN_WSLLHI ? CODE_FOR_ashlv4hi3_iwmmxt + : fcode == ARM_BUILTIN_WSLLW ? CODE_FOR_ashlv2si3_di + : fcode == ARM_BUILTIN_WSLLWI ? CODE_FOR_ashlv2si3_iwmmxt + : fcode == ARM_BUILTIN_WSLLD ? CODE_FOR_ashldi3_di + : fcode == ARM_BUILTIN_WSLLDI ? CODE_FOR_ashldi3_iwmmxt + : fcode == ARM_BUILTIN_WSRAH ? CODE_FOR_ashrv4hi3_di + : fcode == ARM_BUILTIN_WSRAHI ? CODE_FOR_ashrv4hi3_iwmmxt + : fcode == ARM_BUILTIN_WSRAW ? CODE_FOR_ashrv2si3_di + : fcode == ARM_BUILTIN_WSRAWI ? CODE_FOR_ashrv2si3_iwmmxt + : fcode == ARM_BUILTIN_WSRAD ? CODE_FOR_ashrdi3_di + : fcode == ARM_BUILTIN_WSRADI ? CODE_FOR_ashrdi3_iwmmxt + : fcode == ARM_BUILTIN_WSRLH ? CODE_FOR_lshrv4hi3_di + : fcode == ARM_BUILTIN_WSRLHI ? CODE_FOR_lshrv4hi3_iwmmxt + : fcode == ARM_BUILTIN_WSRLW ? CODE_FOR_lshrv2si3_di + : fcode == ARM_BUILTIN_WSRLWI ? CODE_FOR_lshrv2si3_iwmmxt + : fcode == ARM_BUILTIN_WSRLD ? CODE_FOR_lshrdi3_di + : fcode == ARM_BUILTIN_WSRLDI ? CODE_FOR_lshrdi3_iwmmxt + : fcode == ARM_BUILTIN_WRORH ? CODE_FOR_rorv4hi3_di + : fcode == ARM_BUILTIN_WRORHI ? CODE_FOR_rorv4hi3 + : fcode == ARM_BUILTIN_WRORW ? CODE_FOR_rorv2si3_di + : fcode == ARM_BUILTIN_WRORWI ? CODE_FOR_rorv2si3 + : fcode == ARM_BUILTIN_WRORD ? CODE_FOR_rordi3_di + : fcode == ARM_BUILTIN_WRORDI ? CODE_FOR_rordi3 + : fcode == ARM_BUILTIN_WAND ? CODE_FOR_iwmmxt_anddi3 + : fcode == ARM_BUILTIN_WANDN ? CODE_FOR_iwmmxt_nanddi3 + : fcode == ARM_BUILTIN_WOR? CODE_FOR_iwmmxt_iordi3 + : fcode == ARM_BUILTIN_WXOR ? CODE_FOR_iwmmxt_xordi3 + : CODE_FOR_rordi3); + return arm_expand_binop_builtin (icode, exp, target); + case ARM_BUILTIN_WZERO: target = gen_reg_rtx (DImode); emit_insn (gen_iwmmxt_clrdi (target));
Re: [PATCH, PR50527] Don't assume alignment of vla-related allocas.
On 09/30/2011 03:29 PM, Richard Guenther wrote:
> On Thu, Sep 29, 2011 at 3:15 PM, Tom de Vries wrote:
>> On 09/28/2011 11:53 AM, Richard Guenther wrote:
>>> On Wed, Sep 28, 2011 at 11:34 AM, Tom de Vries
>>> wrote:
Richard,
I got a patch for PR50527.
The patch prevents the alignment of vla-related allocas to be set to
BIGGEST_ALIGNMENT in ccp. The alignment may turn out smaller after folding
the alloca.
Bootstrapped and regtested on x86_64.
OK for trunk?
>>>
>>> Hmm. As gfortran with -fstack-arrays uses VLAs it's probably bad that
>>> the vectorizer then will no longer see that the arrays are properly aligned.
>>>
>>> I'm not sure what the best thing to do is here, other than trying to record
>>> the alignment requirement of the VLA somewhere.
>>>
>>> Forcing the alignment of the alloca replacement decl to BIGGEST_ALIGNMENT
>>> has the issue that it will force stack-realignment which isn't free (and the
>>> point was to make the decl cheaper than the alloca). But that might
>>> possibly be the better choice.
>>>
>>> Any other thoughts?
>>
>> How about the approach in this (untested) patch? Using the DECL_ALIGN of the
>> vla
>> for the new array prevents stack realignment for folded vla-allocas, also for
>> large vlas.
>>
>> This will not help in vectorizing large folded vla-allocas, but I think it's
>> not
>> reasonable to expect BIGGEST_ALIGNMENT when writing a vla (although that has
>> been the case up until we started to fold). If you want to trigger
>> vectorization
>> for a vla, you can still use the aligned attribute on the declaration.
>>
>> Still, the unfolded vla-allocas will have BIGGEST_ALIGNMENT, also without
>> using
>> an attribute on the decl. This patch exploits this by setting it at the end
>> of
>> the 3rd pass_ccp, renamed to pass_ccp_last. This is not very effective in
>> propagation though, because although the ptr_info of the lhs is propagated
>> via
>> copy_prop afterwards, it's not propagated anymore via ccp.
>>
>> Another way to do this would be to set BIGGEST_ALIGNMENT at the end of ccp2
>> and
>> not fold during ccp3.
>
> Ugh, somehow I like this the least ;)
>
> How about lowering VLAs to
>
> p = __builtin_alloca (...);
> p = __builtin_assume_aligned (p, DECL_ALIGN (vla));
>
> and not assume anything for alloca itself if it feeds a
> __builtin_assume_aligned?
>
> Or rather introduce a __builtin_alloca_with_align () and for VLAs do
>
> p = __builtin_alloca_with_align (..., DECL_ALIGN (vla));
>
> that's less awkward to use?
>
> Sorry for not having a clear plan here ;)
>
Using assume_aligned is a more orthogonal way to represent this in gimple, but
indeed harder to use.
Another possibility is to add a 'tree vla_decl' field to struct
gimple_statement_call, which is probably the easiest to implement.
But I think __builtin_alloca_with_align might have a use beyond vlas, so I
decided to try this one. Attached patch implements my first stab at this (now
testing on x86_64).
Is this an acceptable approach?
Thanks,
- Tom
> Richard.
>
Thanks,- Tom
Index: gcc/tree.c
===
--- gcc/tree.c (revision 179210)
+++ gcc/tree.c (working copy)
@@ -9483,9 +9483,18 @@ build_common_builtin_nodes (void)
"alloca", ECF_MALLOC | ECF_NOTHROW | ECF_LEAF);
}
+ ftype = build_function_type_list (ptr_type_node, size_type_node,
+size_type_node, NULL_TREE);
+ local_define_builtin ("__builtin_alloca_with_align", ftype,
+ BUILT_IN_ALLOCA_WITH_ALIGN, "alloca_with_align",
+ ECF_MALLOC | ECF_NOTHROW | ECF_LEAF);
+
/* If we're checking the stack, `alloca' can throw. */
if (flag_stack_check)
-TREE_NOTHROW (built_in_decls[BUILT_IN_ALLOCA]) = 0;
+{
+ TREE_NOTHROW (built_in_decls[BUILT_IN_ALLOCA]) = 0;
+ TREE_NOTHROW (built_in_decls[BUILT_IN_ALLOCA_WITH_ALIGN]) = 0;
+}
ftype = build_function_type_list (void_type_node,
ptr_type_node, ptr_type_node,
Index: gcc/tree-pass.h
===
--- gcc/tree-pass.h (revision 179210)
+++ gcc/tree-pass.h (working copy)
@@ -389,6 +389,7 @@ extern struct gimple_opt_pass pass_iv_op
extern struct gimple_opt_pass pass_tree_loop_done;
extern struct gimple_opt_pass pass_ch;
extern struct gimple_opt_pass pass_ccp;
+extern struct gimple_opt_pass pass_ccp_last;
extern struct gimple_opt_pass pass_phi_only_cprop;
extern struct gimple_opt_pass pass_build_ssa;
extern struct gimple_opt_pass pass_build_alias;
Index: gcc/builtins.c
===
--- gcc/builtins.c (revision 179210)
+++ gcc/builtins.c (working copy)
@@ -5304,6 +5304,7 @@ expand_builtin (tree exp, rtx target, rt
&& !called_as_built_in (fndecl)
&& DECL_ASSEMBLER_NAME_SET_P (fndecl)
&& fcode != BUILT_IN_ALLOCA
+ && fcode != BUILT_IN_ALLOCA_WITH_ALIGN
&& fcode != BUILT_IN_FREE)
Re: Disable early inlining while compiling for coverage (issue5173042)
On Sat, Oct 1, 2011 at 5:17 AM, Jan Hubicka wrote:
>> Yes, this will improve test coverage option's usability, but please
>> provide the example to explain the issues.
>>
>> David
>>
>> On Fri, Sep 30, 2011 at 6:12 PM, Sharad Singhai wrote:
>> > This patch disables early inlining when --coverage option is
>> > specified. This improves coverage data in presence of other
>> > optimizations, specially with -O2 where early inlining changes the
>> > control flow graph sufficiently enough to generate seemingly very odd
>> > source coverage.
>
> BTW early inlining was introduced to make coverage possible on some C++
> sources, like tramp3d ;)
Early inline can be important for FDO performance reasons -- as inline
instances get 'context' sensitive profile data.
>However the problem here is that since we moved
> coverage to SSA, we do it too late. My longer term plan is to separate
> coverage and profile feedback passes (so they can't be done both together) and
> instrument early for coverage & ensure that everything before coverage is done
> is save WRT line info.
Yes, coverage and FDO are two different animals having different
requirements -- they happen to share the same instrumentation
mechanism.
> Inlining alone does not mess up the line info, but most
> optimizations we have in early optimization queue do.
Inlining can do some damage too but to a less extent. For instance,
the exit block of the callee instance merged with caller's bb causing
confusion.
>
> We discussed it back when Richi implemented SSA profiling but we didn't do
> that
> basically due to lack of testcases. Would be possible to take one you have
> and fill in some PRs? Those are regressions WRT pre-SSA profiling releases (I
> think 4.5?)
Yes.
Sharad, I did not see the test case attached? Please file a bug about
this. In the meantime, you can checkin the workaround to google
banches.
thanks,
David
>
> Honza
>> >
>> > Bootstrapped okay and regression tests passed.
>> >
>> > Okay for google/gcc-4_6?
>> >
>> > 2011-09-30 Sharad Singhai
>> >
>> > * gcc.c (cc1_options): Added -fno-early-inlining for coverage.
>> >
>> > Index: gcc.c
>> > ===
>> > --- gcc.c (revision 179402)
>> > +++ gcc.c (working copy)
>> > @@ -776,7 +776,7 @@
>> > %{!fsyntax-only:%{S:%W{o*}%{!o*:-o %b.s}}}\
>> > %{fsyntax-only:-o %j} %{-param*}\
>> > %{fmudflap|fmudflapth:-fno-builtin -fno-merge-constants}\
>> > - %{coverage:-fprofile-arcs -ftest-coverage}";
>> > + %{coverage:-fprofile-arcs -ftest-coverage -fno-early-inlining}";
>> >
>> > /* If an assembler wrapper is used to invoke post-assembly tools
>> > like MAO, --save-temps need to be passed to save the output of
>> >
>> > --
>> > This patch is available for review at http://codereview.appspot.com/5173042
>> >
>
[PATCH] Start adding support for VIS 3.0 instructions.
There are couple more to add, but there are some complications
wrt. v8plus for those that I need to sort out first.
And also I haven't attempted to add the floating-point-and-halve
and related ops yet, see:
http://gcc.gnu.org/ml/gcc/2011-09/msg00385.html
Committed to trunk.
gcc/
* config/sparc/sparc.opt (VIS3): New option.
* doc/invoke.texi: Document it.
* config/sparc/sparc.h: Force TARGET_VIS3 to zero if assembler is
not capable of such instructions.
* config/sparc/sparc-c.c (sparc_target_macros): Define __VIS__
to 0x300 when TARGET_VIS3.
* config/sparc/sparc-modes.def: Create 16-byte vector modes.
* config/sparc/sparc.md (UNSPEC_CMASK8, UNSPEC_CMASK16, UNSPEC_CMASK32,
UNSPEC_FCHKSM16, UNSPEC_PDISTN, UNSPC_FUCMP): New unspecs.
(V64N8, VASS): New mode iterators.
(vis3_shift, vis3_addsub_ss): New code iterators.
(vbits, vconstr): New mode attributes.
(vis3_shift_insn, vis3_addsub_ss_insn): New code attributes.
(cmask8_vis, cmask16_vis, cmask32_vis,
fchksm16_vis, _vis, pdistn_vis,
fmean16_vis, fpadd64_vis, fpsub64_vis, _vis,
fucmp8_vis): New VIS 3.0 instruction patterns.
* config/sparc/sparc.c (sparc_option_override): Set MASK_VIS3 by
default when targetting capable cpus. TARGET_VIS3 implies
TARGET_VIS2 and TARGET_VIS, and clear them when TARGET_FPU is
disabled.
(sparc_vis_init_builtins): Emit new VIS 3.0 builtins.
(sparc_fold_builtin): Do not eliminate cmask{8,16,32} when result
is ignored.
* config/sparc/visintrin.h (__vis_cmask8, __vis_cmask16,
__vis_cmask32, __vis_fchksm16, __vis_fsll16, __vis_fslas16,
__vis_fsrl16, __vis_fsra16, __vis_fsll32, __vis_fslas32,
__vis_fsrl32, __vis_fsra32, __vis_pdistn, __vis_fmean16,
__vis_fpadd64, __vis_fpsub64, __vis_fpadds16, __vis_fpadds16s,
__vis_fpsubs16, __vis_fpsubs16s, __vis_fpadds32, __vis_fpadds32s,
__vis_fpsubs32, __vis_fpsubs32s, __vis_fucmple8, __vis_fucmpne8,
__vis_fucmpgt8, __vis_fucmpeq8): New VIS 3.0 interfaces.
* doc/extend.texi: Document new VIS 3.0 builtins.
gcc/testsuite/
* gcc.target/sparc/cmask.c: New test.
* gcc.target/sparc/fpadds.c: New test.
* gcc.target/sparc/fshift.c: New test.
* gcc.target/sparc/fucmp.c: New test.
* gcc.target/sparc/vis3misc.c: New test.
---
gcc/ChangeLog | 36 ++
gcc/config/sparc/sparc-c.c|7 +-
gcc/config/sparc/sparc-modes.def |1 +
gcc/config/sparc/sparc.c | 142 +++--
gcc/config/sparc/sparc.h |2 +
gcc/config/sparc/sparc.md | 112
gcc/config/sparc/sparc.opt|4 +
gcc/config/sparc/visintrin.h | 196 +
gcc/doc/extend.texi | 45 +++-
gcc/doc/invoke.texi | 13 ++-
gcc/testsuite/ChangeLog |8 ++
gcc/testsuite/gcc.target/sparc/cmask.c| 21 +++
gcc/testsuite/gcc.target/sparc/fpadds.c | 55
gcc/testsuite/gcc.target/sparc/fshift.c | 53
gcc/testsuite/gcc.target/sparc/fucmp.c| 28
gcc/testsuite/gcc.target/sparc/vis3misc.c | 37 ++
16 files changed, 745 insertions(+), 15 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/sparc/cmask.c
create mode 100644 gcc/testsuite/gcc.target/sparc/fpadds.c
create mode 100644 gcc/testsuite/gcc.target/sparc/fshift.c
create mode 100644 gcc/testsuite/gcc.target/sparc/fucmp.c
create mode 100644 gcc/testsuite/gcc.target/sparc/vis3misc.c
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 2b62b96..f8e02d3 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,39 @@
+2011-10-01 David S. Miller
+
+ * config/sparc/sparc.opt (VIS3): New option.
+ * doc/invoke.texi: Document it.
+ * config/sparc/sparc.h: Force TARGET_VIS3 to zero if assembler is
+ not capable of such instructions.
+ * config/sparc/sparc-c.c (sparc_target_macros): Define __VIS__
+ to 0x300 when TARGET_VIS3.
+ * config/sparc/sparc-modes.def: Create 16-byte vector modes.
+ * config/sparc/sparc.md (UNSPEC_CMASK8, UNSPEC_CMASK16, UNSPEC_CMASK32,
+ UNSPEC_FCHKSM16, UNSPEC_PDISTN, UNSPC_FUCMP): New unspecs.
+ (V64N8, VASS): New mode iterators.
+ (vis3_shift, vis3_addsub_ss): New code iterators.
+ (vbits, vconstr): New mode attributes.
+ (vis3_shift_insn, vis3_addsub_ss_insn): New code attributes.
+ (cmask8_vis, cmask16_vis, cmask32_vis,
+ fchksm16_vis, _vis, pdistn_vis,
+ fmean16_vis, fpadd64_vis, fpsub64_vis, _vis,
+ fucmp8_vis): New VIS 3.0 instruction patterns.
+ * config/sparc/sparc.c (sparc_option_override): Set MASK_VIS3 by
+ default when targetting capable cpus. TARGET_VIS
Re: [v3] PR libstdc++/50529
On 09/29/2011 10:59 PM, Paolo Carlini wrote:
Ok to commit ?
Ok. These patches are going also to 4_6-branch.
Paolo
Attached patch applied, thanks Paolo for applying it to 4.6 branch.
2011-10-01 François Dumont
* include/debug/vector (vector<>::erase(iterator, iterator): Check
iterators equality using normal iterators.
* include/debug/deque (deque<>::erase(iterator, iterator):
Likewise.
Regards
Index: include/debug/vector
===
--- include/debug/vector (revision 179319)
+++ include/debug/vector (working copy)
@@ -499,7 +499,7 @@
// 151. can't currently clear() empty container
__glibcxx_check_erase_range(__first, __last);
- if (__first != __last)
+ if (__first.base() != __last.base())
{
difference_type __offset = __first.base() - _Base::begin();
_Base_iterator __res = _Base::erase(__first.base(),
Index: include/debug/deque
===
--- include/debug/deque (revision 179319)
+++ include/debug/deque (working copy)
@@ -465,7 +465,7 @@
// 151. can't currently clear() empty container
__glibcxx_check_erase_range(__first, __last);
- if (__first == __last)
+ if (__first.base() == __last.base())
return __first;
else if (__first.base() == _Base::begin()
|| __last.base() == _Base::end())
Fix typo in ira-color.c:assign_hard_reg
Iain spotted a strange allocation pattern for an ACATS test on PowerPC/Darwin,
which turned out to be caused by this typo.
Tested on i586-suse-linux, applied on the mainline.
2011-10-01 Eric Botcazou
* ira-color.c (assign_hard_reg): Fix typo.
--
Eric Botcazou
Index: gcc/ira-color.c
===
--- gcc/ira-color.c (revision 179413)
+++ gcc/ira-color.c (working copy)
@@ -1769,7 +1769,7 @@ assign_hard_reg (ira_allocno_t a, bool r
if (best_hard_regno >= 0)
{
for (i = hard_regno_nregs[best_hard_regno][mode] - 1; i >= 0; i--)
- allocated_hardreg_p[best_hard_regno + 1] = true;
+ allocated_hardreg_p[best_hard_regno + i] = true;
}
ALLOCNO_HARD_REGNO (a) = best_hard_regno;
ALLOCNO_ASSIGNED_P (a) = true;
Re: Disable early inlining while compiling for coverage (issue5173042)
I have updated PR/45890 with a test case.
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45890
In any case, I am attaching a test case here as well.
Sharad
On Sat, Oct 1, 2011 at 10:19 AM, Xinliang David Li wrote:
>
> On Sat, Oct 1, 2011 at 5:17 AM, Jan Hubicka wrote:
> >> Yes, this will improve test coverage option's usability, but please
> >> provide the example to explain the issues.
> >>
> >> David
> >>
> >> On Fri, Sep 30, 2011 at 6:12 PM, Sharad Singhai wrote:
> >> > This patch disables early inlining when --coverage option is
> >> > specified. This improves coverage data in presence of other
> >> > optimizations, specially with -O2 where early inlining changes the
> >> > control flow graph sufficiently enough to generate seemingly very odd
> >> > source coverage.
> >
> > BTW early inlining was introduced to make coverage possible on some C++
> > sources, like tramp3d ;)
>
> Early inline can be important for FDO performance reasons -- as inline
> instances get 'context' sensitive profile data.
>
> >However the problem here is that since we moved
> > coverage to SSA, we do it too late. My longer term plan is to separate
> > coverage and profile feedback passes (so they can't be done both together)
> > and
> > instrument early for coverage & ensure that everything before coverage is
> > done
> > is save WRT line info.
>
> Yes, coverage and FDO are two different animals having different
> requirements -- they happen to share the same instrumentation
> mechanism.
>
> > Inlining alone does not mess up the line info, but most
> > optimizations we have in early optimization queue do.
>
> Inlining can do some damage too but to a less extent. For instance,
> the exit block of the callee instance merged with caller's bb causing
> confusion.
>
> >
> > We discussed it back when Richi implemented SSA profiling but we didn't do
> > that
> > basically due to lack of testcases. Would be possible to take one you have
> > and fill in some PRs? Those are regressions WRT pre-SSA profiling releases
> > (I think 4.5?)
>
> Yes.
>
> Sharad, I did not see the test case attached? Please file a bug about
> this. In the meantime, you can checkin the workaround to google
> banches.
>
> thanks,
>
> David
>
> >
> > Honza
> >> >
> >> > Bootstrapped okay and regression tests passed.
> >> >
> >> > Okay for google/gcc-4_6?
> >> >
> >> > 2011-09-30 Sharad Singhai
> >> >
> >> > * gcc.c (cc1_options): Added -fno-early-inlining for coverage.
> >> >
> >> > Index: gcc.c
> >> > ===
> >> > --- gcc.c (revision 179402)
> >> > +++ gcc.c (working copy)
> >> > @@ -776,7 +776,7 @@
> >> > %{!fsyntax-only:%{S:%W{o*}%{!o*:-o %b.s}}}\
> >> > %{fsyntax-only:-o %j} %{-param*}\
> >> > %{fmudflap|fmudflapth:-fno-builtin -fno-merge-constants}\
> >> > - %{coverage:-fprofile-arcs -ftest-coverage}";
> >> > + %{coverage:-fprofile-arcs -ftest-coverage -fno-early-inlining}";
> >> >
> >> > /* If an assembler wrapper is used to invoke post-assembly tools
> >> > like MAO, --save-temps need to be passed to save the output of
> >> >
> >> > --
> >> > This patch is available for review at
> >> > http://codereview.appspot.com/5173042
> >> >
> >
gcov.tar.gz
Description: GNU Zip compressed data
Re: [google] Add intermediate text format for gcov (issue4595053)
Sorry, I didn't think about the mainline. Since you find it useful, I will propose it for inclusion in the trunk as well. Thanks, Sharad On Sat, Oct 1, 2011 at 6:23 AM, Matt Rice wrote: > 2011/6/14 Sharad Singhai (शरद सिंघई) : >> Sorry, Rietveld didn't send out the updated patch along with my mail. >> Here it is. >> > > Hi, I tried this patch out on trunk it applies alright, and appears to > work fine, (haven't run the testsuite though) any plans on submitting > it for inclusion with mainline gcc? >
PR preprocessor/36819
Hi,
this minor issue remained open and miscategorized as C++ for many years.
I changed it tentatively to preprocessor and I think we can easily
resolve it as suggested by submitter: apparently there is a small memory
leak happening at beginning of incpath.c:split_quote_chain, and the
below seems the obvious way to plug it. Is it Ok for mainline?
Thanks,
Paolo.
///
2011-10-01 Paolo Carlini
PR preprocessor/36819
* incpath.c (merge_include_chains): Call free_path on
heads[QUOTE] and tails[QUOTE].
Index: incpath.c
===
--- incpath.c (revision 179416)
+++ incpath.c (working copy)
@@ -362,6 +362,8 @@ merge_include_chains (const char *sysroot, cpp_rea
void
split_quote_chain (void)
{
+ free_path (heads[QUOTE], REASON_QUIET);
+ free_path (tails[QUOTE], REASON_QUIET);
heads[QUOTE] = heads[BRACKET];
tails[QUOTE] = tails[BRACKET];
heads[BRACKET] = NULL;
[Patch, Fortran] PR 50547 / cleanup in resolve_formal_arglist
Hi all,
while working on PR50547, I noticed some strange things about
resolve_formal_arglist, so I decided to clean it up a little. The
attached patch does a couple of things:
(1) It removes an error message ("Unable to find a specific INTRINSIC
procedure...") which simply does not make any sense in
resolve_formal_arglist. There is just no way for procedure dummies to
be intrinsics. "svn blame" claims this code was committed by Paul in
r120296 (for PR27900):
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27900#c12
The error message was added both in resolve_actual_arglist (where it
*does* make sense), and in resolve_formal_arglist (where it doesn't).
(2) The error message "Dummy procedure ... not allowed in ELEMENTAL
procedure" is being thrown in two different places (for
functions/subroutines), which is completely unnecessary. I removed one
of them, and made sure the other is triggered for both cases. Also the
testsuite was missing a test for this case, so I added one.
(3) I reshuffled some of the code dealing with pure procedures, in
order to have it in one place.
(4) I reshuffled some of the code dealing with "attr.implicit_pure",
so that it is not so scattered around and better to understand.
During all the reshuffling I removed an early "continue" for dummy
subroutines, and as a consequence had to deal with (not) setting type
and flavor for subroutines.
Regtested on x86_64-unknown-linux-gnu. Ok for trunk?
Cheers,
Janus
2011-10-02 Janus Weil
PR fortran/50547
* resolve.c (resolve_formal_arglist): Remove unneeded error message.
Some reshuffling.
2011-10-02 Janus Weil
PR fortran/50547
* gfortran.dg/elemental_args_check_4.f90: New.
Index: gcc/fortran/resolve.c
===
--- gcc/fortran/resolve.c (revision 179413)
+++ gcc/fortran/resolve.c (working copy)
@@ -269,50 +269,18 @@ resolve_formal_arglist (gfc_symbol *proc)
if (sym->attr.if_source != IFSRC_UNKNOWN)
resolve_formal_arglist (sym);
- /* F08:C1279. */
- if (gfc_pure (proc)
- && sym->attr.flavor == FL_PROCEDURE && !gfc_pure (sym))
+ if (sym->attr.subroutine || sym->attr.external)
{
- gfc_error ("Dummy procedure '%s' of PURE procedure at %L must "
- "also be PURE", sym->name, &sym->declared_at);
- continue;
+ if (sym->attr.flavor == FL_UNKNOWN)
+ gfc_add_flavor (&sym->attr, FL_PROCEDURE, sym->name, &sym->declared_at);
}
-
- if (sym->attr.subroutine || sym->attr.external || sym->attr.intrinsic)
+ else
{
- if (proc->attr.implicit_pure && !gfc_pure(sym))
- proc->attr.implicit_pure = 0;
-
- /* F08:C1289. */
- if (gfc_elemental (proc))
- {
- gfc_error ("Dummy procedure at %L not allowed in ELEMENTAL "
- "procedure", &sym->declared_at);
- continue;
- }
-
- if (sym->attr.function
- && sym->ts.type == BT_UNKNOWN
- && sym->attr.intrinsic)
- {
- gfc_intrinsic_sym *isym;
- isym = gfc_find_function (sym->name);
- if (isym == NULL || !isym->specific)
- {
- gfc_error ("Unable to find a specific INTRINSIC procedure "
- "for the reference '%s' at %L", sym->name,
- &sym->declared_at);
- }
- sym->ts = isym->ts;
- }
-
- continue;
+ if (sym->ts.type == BT_UNKNOWN && !proc->attr.intrinsic
+ && (!sym->attr.function || sym->result == sym))
+ gfc_set_default_type (sym, 1, sym->ns);
}
- if (sym->ts.type == BT_UNKNOWN && !proc->attr.intrinsic
- && (!sym->attr.function || sym->result == sym))
- gfc_set_default_type (sym, 1, sym->ns);
-
gfc_resolve_array_spec (sym->as, 0);
/* We can't tell if an array with dimension (:) is assumed or deferred
@@ -343,44 +311,64 @@ resolve_formal_arglist (gfc_symbol *proc)
if (sym->attr.flavor == FL_UNKNOWN)
gfc_add_flavor (&sym->attr, FL_VARIABLE, sym->name, &sym->declared_at);
- if (gfc_pure (proc) && !sym->attr.pointer
- && sym->attr.flavor != FL_PROCEDURE)
+ if (gfc_pure (proc))
{
- if (proc->attr.function && sym->attr.intent != INTENT_IN)
+ if (sym->attr.flavor == FL_PROCEDURE)
{
- if (sym->attr.value)
- gfc_notify_std (GFC_STD_F2008, "Fortran 2008: Argument '%s' "
-"of pure function '%s' at %L with VALUE "
-"attribute but without INTENT(IN)", sym->name,
-proc->name, &sym->declared_at);
- else
- gfc_error ("Argument '%s' of pure function '%s' at %L must be "
- "INTENT(IN) or VALUE", sym->name, proc->name,
- &sym->declared_at);
+ /* F08:C1279. */
+ if (!gfc_pure (sym))
+ {
+ gfc_error ("Dummy procedure '%s' of PURE procedure at %L must "
+ "also be PURE", sym->name, &sym->declared_at);
+ continue;
+ }
}
-
- if (proc->attr.subroutine && sym->attr.intent == INTENT_UNKNOWN)
+ else if (!sym->attr.pointer)
{
- if (sym->attr.value)
- gfc_notify_std (GFC_STD_F2008, "Fortran 2008: Argument '%s' "
-"of pure subroutine
gcc-patches@gcc.gnu.org
On Mon, Sep 26, 2011 at 7:33 PM, Jason Merrill wrote: > cxx_eval_logical_expression was assuming that a folded first operand of && > would be either boolean_true_node or boolean_false_node, but in fact it can > be a constant with a typedef of bool, which doesn't compare equal with ==. > So we should use tree_int_cst_equal to compare them instead. > > Tested x86_64-pc-linux-gnu, applying to trunk. > Thanks. It is weird though that GCC does not maintain a properly typed internal representation. -- Gaby
