Re: RFA: MN10300: Add support for SETLB and Lcc instructions
Hi Richard, Thanks for the review. Attached is a revised patch which addresses the issues that you raised: +/* A special mode used to distinguish the Lcc branch instruction + from the Bcc branch instruction. */ +CC_MODE (CC_LCC); Why not just use an unspec on the lcc insn. I should have done. The CC_LCC mode was a holdover from when I was trying to get the doloop patterns to work. The doloop pattern recognizer does not accept UNSPECs, so I had to use a unique CC mode instead. The revised patch uses an UNSPEC. + /* What we're doing here is looking for a conditional branch which + branches backwards without crossing any non-debug labels. I.e. + the loop has to enter from the top. */ Ug. Surely we can re-use the existing loop detection code to find the inner-most loops. We can, but it is a lot more complicated than just performing a simple scan. Still I have tried my best to follow up on this. One thing that does worry me however is that I encountered a segmentation fault whilst trying to free the loop data. (See a comment in the patch for more details). I tried to follow the example of other passes that use the loop information, but I must have gotten something wrong. Any pointers on how to fix this are greatly appreciated. Cheers Nick gcc/ChangeLog 2011-03-08 Nick Clifton * config/mn10300/mn10300.c: Include cfgloop.h. (DUMP): New macro. (mn10300_insert_setlb_lcc): New function. Inserts a SETLB and a Lcc or a FLcc insn into the instruction stream. (mn10300_block_contains_call): New function. Returns true if the given basic block contains a CALL insn. (mn10300_loop_contains_call_insn): New function. Returns true if the given loop contains a CALL insn. (mn10300_scan_for_setlb_lcc): New function. Finds opportunities to use the SETLB and Lcc or FLcc insns. (mn10300_reorg): Invoke mn10300_scan_for_setlb_lcc. (TARGET_FLAGS): Add MASK_ALLOW_SETLB. * config/mn10300/mn10300.opt (msetlb): New option. Used to disable the SETLB optimization. * config/mn10300/mn10300.h (TARGET_CPU_CPP_BUILTINS): Add __SETLB__ or __NO_SETLB__. * config/mn10300/mn10300.md (UNSPEC_SETLB): New constant. (cmpsi): Make visible. (setlb): New pattern. (Lcc): New pattern. (FLcc): New pattern. * doc/invoke.texi: Document -mno-setlb option. mn10300.setlb.patch.3 Description: Unix manual page
Re: [v3] libstdc++/47145
On 8 March 2011 00:14, Benjamin Kosnik wrote: > > Conditionally set XSL_STYLE_DIR at configure time for either debian or > fedora/RHEL based systems. As discussed in bugzilla. > > For convenience, this patch steps around the STYLESHEET_FLAG question by > just punting to the net for validation only. But it still uses --nonet in that step, right? So it doesn't use the network at all, the mapping from the canonical stylesheet URL to a local file is done by the system's XML catalog, /etc/xml/catalog on my Fedora box. I thought we could do the same for the actual transformation rules too, using the canonical URL with --nonet and letting the system map it to a local file, and failing if the file is not present, because --nonet prevents it fetching a remote stylesheet. I suspect this is fine for the majority of people now and good enough for 4.6 Jonathan
[PATCH] unbreak gcc.dg/tree-ssa/ssa-ccp-33.c on m68k (PR testsuite/47954)
gcc.dg/tree-ssa/ssa-ccp-33.c fails with gcc trunk on m68k-linux: ssa-ccp-33.c:(.text+0x2a): undefined reference to `link_error' ssa-ccp-33.c:(.text+0x2a): undefined reference to `link_error' FAIL: gcc.dg/tree-ssa/ssa-ccp-33.c (test for excess errors) ssa-ccp-33.c:(.text+0x2a): undefined reference to `link_error' The test case checks that the compiler is able to eliminate a runtime check that an aligned pointer-to-int remains aligned after a loop of increments. It uses sizeof to compute the alignment of int, but on m68k (and possibly others) the alignment of int is less than its size. The compiler is then unable to eliminate the broken alignment check, and the call to link_error () is not removed. Fixed by using __alignof__ instead. Regression tested on m68k-linux where it eliminated the FAIL for gcc.dg/tree-ssa/ssa-ccp-33.c. Also tested on i686-linux, no changes there. Ok for trunk? (Richard G. pre-approved this change on the PR entry, however I cannot commit it myself.) gcc/testsuite/ 2011-03-08 Mikael Pettersson PR testsuite/47954 * gcc.dg/tree-ssa/ssa-ccp-33.c: Use __alignof__ not sizeof to compute alignment. --- gcc-4.6-20110305/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-33.c.~1~ 2010-08-06 13:47:31.0 +0200 +++ gcc-4.6-20110305/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-33.c 2011-03-08 10:34:13.0 +0100 @@ -8,7 +8,7 @@ void foo(int n) int *p; for (p = a; n != 0; --n, ++p) ; - if ((__SIZE_TYPE__)p & (sizeof (int) - 1)) + if ((__SIZE_TYPE__)p & (__alignof__ (int) - 1)) link_error (); } int main()
Re: [trans-mem] PR 47952 Re: weak aliases, .tm_clone_table, and binutils confusion
Hi Richard, On 03/08/2011 01:50 AM, Richard Henderson wrote: On 03/06/2011 10:54 PM, Patrick Marlier wrote: Well, I have patched trans-mem.c to update the name of the COMDAT_GROUP in ipa_tm_create_version(). I know this is not the way to do this but I hope it can at least help you. This part is clearly correct. I've tidied up your patch a bit and committed the following. Please update the PR with a full compilable test case so I can determine what else might need fixing. I have just added a testcase to the PR but not reduced at all. Actually the problem with your submitted patch is that the following happens: .section .text._ZGTtNSt14_List_iteratorIN4Game12BuildProjectEEC2EPSt15_List_node_base,"axG",@progbits,_ZGTt67_ZNSt14_List_iteratorIN4Game12BuildProjectEEC5EPSt15_List_node_base,comdat This is why my patch was *nasty* (string manipulation) because I can't find a way to demangle it properly. About this line of my patch for aliases: DECL_WEAK (tm_alias) = DECL_WEAK (alias->decl); You can reproduce it with the same testcase, looking at these symbols: .weak _ZNSt14_List_iteratorIN4Game12BuildProjectEEC1EPSt15_List_node_base .globl _ZGTtNSt14_List_iteratorIN4Game12BuildProjectEEC1EPSt15_List_node_base The clone version should be also weak. Patrick Marlier.
Re: [PATCH] Don't warn with -Wstrict-overflow about X + C1 == C2 transformations (PR tree-optimization/48022)
On Mon, Mar 7, 2011 at 8:15 PM, Jakub Jelinek wrote:
> Hi!
>
> EQ/NE comparisons don't really assume that overflow doesn't happen unlike
>>/>=/etc., so it is strange that we warn about it.
> As the warning happens on simple strcmp uses when using glibc string.h,
> it is extra annoying.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?
Ok.
Thanks,
Richard.
> 2011-03-07 Jakub Jelinek
>
> PR tree-optimization/48022
> * fold-const.c (fold_comparison): Don't call fold_overflow_warning
> for EQ/NE_EXPR.
>
> * gcc.dg/pr48022-1.c: New test.
> * gcc.dg/pr48022-2.c: New test.
>
> --- gcc/fold-const.c.jj 2011-03-03 09:11:43.0 +0100
> +++ gcc/fold-const.c 2011-03-07 17:44:02.0 +0100
> @@ -8572,10 +8572,11 @@ fold_comparison (location_t loc, enum tr
> && (TREE_CODE (lhs) != INTEGER_CST
> || !TREE_OVERFLOW (lhs)))
> {
> - fold_overflow_warning ("assuming signed overflow does not occur "
> - "when changing X +- C1 cmp C2 to "
> - "X cmp C1 +- C2",
> - WARN_STRICT_OVERFLOW_COMPARISON);
> + if (code != EQ_EXPR && code != NE_EXPR)
> + fold_overflow_warning ("assuming signed overflow does not occur "
> + "when changing X +- C1 cmp C2 to "
> + "X cmp C1 +- C2",
> + WARN_STRICT_OVERFLOW_COMPARISON);
> return fold_build2_loc (loc, code, type, variable, lhs);
> }
> }
> --- gcc/testsuite/gcc.dg/pr48022-1.c.jj 2011-03-07 17:46:55.0 +0100
> +++ gcc/testsuite/gcc.dg/pr48022-1.c 2011-03-07 17:47:18.0 +0100
> @@ -0,0 +1,14 @@
> +/* PR tree-optimization/48022 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -Wstrict-overflow" } */
> +
> +int
> +foo (const char *x)
> +{
> + unsigned long l = 1;
> + const unsigned char *s = (const unsigned char *) (const char *) (x);
> + int r = s[0] - ((const unsigned char *) (const char *) ("/"))[0];
> + if (l > 0 && r == 0)
> + r = (s[1] - ((const unsigned char *) (const char *) ("/"))[1]);
> + return r;
> +}
> --- gcc/testsuite/gcc.dg/pr48022-2.c.jj 2011-03-07 17:47:26.0 +0100
> +++ gcc/testsuite/gcc.dg/pr48022-2.c 2011-03-07 17:47:45.0 +0100
> @@ -0,0 +1,11 @@
> +/* PR tree-optimization/48022 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -Wstrict-overflow" } */
> +
> +#include
> +
> +int
> +foo (const char *x)
> +{
> + return strcmp (x, "/");
> +}
>
> Jakub
>
Re: [patch testsuite g++.dg]: Adjust tree-ssa/pr21082 testcase for LLP64
On Mon, Mar 7, 2011 at 8:42 PM, Kai Tietz wrote:
> PING
Ok.
Thanks,
Richard.
> 2011/2/23 Kai Tietz :
>> Hi,
>>
>> ChangeLog
>>
>> 2011-02-23 Kai Tietz
>>
>> * g++.dg/tree-ssa/pr21082.C: Use __INTPTR_TYPE__ instead of 'long'
>> type.
>>
>> Ok for apply?
>>
>> Regards,
>> Kai
>>
>
>
>
> --
> | (\_/) This is Bunny. Copy and paste
> | (='.'=) Bunny into your signature to help
> | (")_(") him gain world domination
>
Re: avoid useless if-before-free tests
Joseph S. Myers wrote:
Thank you for the prompt feedback.
> On Sat, 5 Mar 2011, Jim Meyering wrote:
>
>> diff --git a/gcc/config/i386/gmm_malloc.h b/gcc/config/i386/gmm_malloc.h
>> index 7a7e840..8993fc7 100644
>> --- a/gcc/config/i386/gmm_malloc.h
>> +++ b/gcc/config/i386/gmm_malloc.h
>> @@ -67,8 +67,7 @@ _mm_malloc (size_t size, size_t align)
>> static __inline__ void
>> _mm_free (void * aligned_ptr)
>> {
>> - if (aligned_ptr)
>> -free (((void **) aligned_ptr) [-1]);
>> + free (((void **) aligned_ptr) [-1]);
>> }
>
> This one looks suspicious; it's not if (p) free (p); but if (p) free
> (something-derived-from-p);.
Good catch. That is an invalid transformation. I've reverted it.
It is also the first one like that that I've seen.
Calling free (((void **) 0) [-1]); would not go down well.
>> diff --git a/libjava/classpath/native/fdlibm/dtoa.c
>> b/libjava/classpath/native/fdlibm/dtoa.c
>> index 458e629..92aa793 100644
>
> http://gcc.gnu.org/codingconventions.html says Classpath changes should go
> via Classpath upstream, not directly into GCC. I don't know if that's
> still accurate.
Thanks for the tip and for Cc'ing java-patches.
I've omitted the classpath/ changes.
>> diff --git a/zlib/contrib/minizip/unzip.c b/zlib/contrib/minizip/unzip.c
>> index 9ad4766..644ef1b 100644
>
> We definitely don't want to make local changes to zlib for this sort of
> issue, though importing a new upstream version of zlib (making sure the
> local configure code still works) should be fine for 4.7.
I've also omitted zlib/ and intl/ changes.
Is libgo/ in the same boat? Only one of its files is affected, but
I found no ChangeLog for libgo and no libgo-related ChangeLog entries.
I could always add an entry at the top,
* libgo/runtime/go-select.c (__go_select): ...
Re: [PATCH] unbreak gcc.dg/tree-ssa/ssa-ccp-33.c on m68k (PR testsuite/47954)
On Tue, Mar 8, 2011 at 10:43 AM, Mikael Pettersson wrote: > gcc.dg/tree-ssa/ssa-ccp-33.c fails with gcc trunk on m68k-linux: > > ssa-ccp-33.c:(.text+0x2a): undefined reference to `link_error' > ssa-ccp-33.c:(.text+0x2a): undefined reference to `link_error' > FAIL: gcc.dg/tree-ssa/ssa-ccp-33.c (test for excess errors) > ssa-ccp-33.c:(.text+0x2a): undefined reference to `link_error' > > The test case checks that the compiler is able to eliminate a > runtime check that an aligned pointer-to-int remains aligned after > a loop of increments. It uses sizeof to compute the alignment > of int, but on m68k (and possibly others) the alignment of int > is less than its size. The compiler is then unable to eliminate > the broken alignment check, and the call to link_error () is not > removed. > > Fixed by using __alignof__ instead. Regression tested on m68k-linux > where it eliminated the FAIL for gcc.dg/tree-ssa/ssa-ccp-33.c. > Also tested on i686-linux, no changes there. > > Ok for trunk? > > (Richard G. pre-approved this change on the PR entry, however I > cannot commit it myself.) Committed. Richard. > gcc/testsuite/ > > 2011-03-08 Mikael Pettersson > > PR testsuite/47954 > * gcc.dg/tree-ssa/ssa-ccp-33.c: Use __alignof__ not > sizeof to compute alignment. > > --- gcc-4.6-20110305/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-33.c.~1~ > 2010-08-06 13:47:31.0 +0200 > +++ gcc-4.6-20110305/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-33.c 2011-03-08 > 10:34:13.0 +0100 > @@ -8,7 +8,7 @@ void foo(int n) > int *p; > for (p = a; n != 0; --n, ++p) > ; > - if ((__SIZE_TYPE__)p & (sizeof (int) - 1)) > + if ((__SIZE_TYPE__)p & (__alignof__ (int) - 1)) > link_error (); > } > int main() >
[patch libiberty include]: Add additional helper functions for directory-separator searching
Hello,
This patch introduce directory-separator search routines to libiberty.
IMHO filename_cmp.c is suiteable for those functions, but if there are
objections about that I can move it into a separate source-file. It
helps to avoid a commonly used pattern about dir-separator searching
in code, which requires #if-conditions to check if DOS paths are used
and introduces additional internal variables.
the pattern
const char *filename = strrchr (xloc.file, '/');
#ifdef HAVE_DOS_BASED_FILE_SYSTEM
const char *filename2 = strrchr (xloc.file, '\\');
if (!filename || (filename2 && filename2 > filename))
filename = filename2;
can be written by this patch as
const char *filename = filename_dirrchr (xloc.file);
ChangeLog include/
2011-03-08 Kai Tietz
* filenames.h (filename_dirchr): New prototype.
(filename_dirrchr): Likewise.
ChangeLog libiberty/
2011-03-08 Kai Tietz
* filename_cmp.c (filename_dirchr): New function.
(filename_dirrchr): Likewise.
* functions.texi: Regenerated.
Tested for x86_64-pc-linux-gnu and x86_64-w64-mingw32. Ok for apply?
Regards,
Kai
Index: gcc/include/filenames.h
===
--- gcc.orig/include/filenames.h2011-02-28 19:16:35.0 +0100
+++ gcc/include/filenames.h 2011-03-08 11:11:10.909109700 +0100
@@ -75,6 +75,8 @@ extern int filename_cmp (const char *s1,
extern int filename_ncmp (const char *s1, const char *s2,
size_t n);
+extern char *filename_dirchr (const char *p);
+extern char *filename_dirrchr (const char *p);
#ifdef __cplusplus
}
Index: gcc/libiberty/filename_cmp.c
===
--- gcc.orig/libiberty/filename_cmp.c 2011-02-28 19:16:35.0 +0100
+++ gcc/libiberty/filename_cmp.c2011-03-08 11:43:32.797198100 +0100
@@ -125,3 +125,70 @@ filename_ncmp (const char *s1, const cha
return 0;
#endif
}
+
+/*
+
+@deftypefn Extension int filename_dirchr (const char *@var{p})
+
+The returned value is similar to what @code{strchr} would return for
+searching for a directory separator.
+
+This function does not normalize file name. However, it does handle
+the fact that on DOS-like file systems, forward and backward slashes
+are directory separators.
+
+@end deftypefn
+
+*/
+
+char *
+filename_dirchr (const char *p)
+{
+ char *r;
+#ifdef HAVE_DOS_BASED_FILE_SYSTEM
+ char *r2;
+#endif
+ if (!p)
+return NULL;
+ r = strchr (p, '/');
+#ifdef HAVE_DOS_BASED_FILE_SYSTEM
+ r2 = strchr (p, '\\');
+ if (!r || (r2 && r2 < r))
+r = r2;
+#endif
+ return r;
+}
+
+/*
+
+@deftypefn Extension int filename_dirrchr (const char *@var{p})
+
+The returned value is similar to what @code{strrchr} would return for
+searching for a directory separator.
+
+This function does not normalize file name. However, it does handle
+the fact that on DOS-like file systems, forward and backward slashes
+are directory separators.
+
+@end deftypefn
+
+*/
+
+char *
+filename_dirrchr (const char *p)
+{
+ char *r;
+#ifdef HAVE_DOS_BASED_FILE_SYSTEM
+ char *r2;
+#endif
+
+ if (!p)
+return NULL;
+ r = strrchr (p, '/');
+#ifdef HAVE_DOS_BASED_FILE_SYSTEM
+ r2 = strrchr (p, '\\');
+ if (!r || (r2 && r2 > r))
+r = r2;
+#endif
+ return r;
+}
Index: gcc/libiberty/functions.texi
===
--- gcc.orig/libiberty/functions.texi 2011-02-28 19:16:35.0 +0100
+++ gcc/libiberty/functions.texi2011-03-08 11:43:42.314406700 +0100
@@ -296,6 +296,30 @@ and backward slashes are equal.
@end deftypefn
+@c filename_cmp.c:131
+@deftypefn Extension int filename_dirchr (const char *@var{p})
+
+The returned value is similar to what @code{strchr} would return for
+searching for a directory separator.
+
+This function does not normalize file name. However, it does handle
+the fact that on DOS-like file systems, forward and backward slashes
+are directory separators.
+
+@end deftypefn
+
+@c filename_cmp.c:164
+@deftypefn Extension int filename_dirrchr (const char *@var{p})
+
+The returned value is similar to what @code{strrchr} would return for
+searching for a directory separator.
+
+This function does not normalize file name. However, it does handle
+the fact that on DOS-like file systems, forward and backward slashes
+are directory separators.
+
+@end deftypefn
+
@c filename_cmp.c:81
@deftypefn Extension int filename_ncmp (const char *@var{s1}, const char
*@var{s2}, size_t @var{n})
Re: [patch testsuite g++.dg]: Adjust tree-ssa/pr21082 testcase for LLP64
2011/3/8 Richard Guenther :
> On Mon, Mar 7, 2011 at 8:42 PM, Kai Tietz wrote:
>> PING
>
> Ok.
>
> Thanks,
> Richard.
>
>> 2011/2/23 Kai Tietz :
>>> Hi,
>>>
>>> ChangeLog
>>>
>>> 2011-02-23 Kai Tietz
>>>
>>> * g++.dg/tree-ssa/pr21082.C: Use __INTPTR_TYPE__ instead of 'long'
>>> type.
>>>
>>> Ok for apply?
>>>
>>> Regards,
>>> Kai
>>>
>>
>>
>>
>> --
>> | (\_/) This is Bunny. Copy and paste
>> | (='.'=) Bunny into your signature to help
>> | (")_(") him gain world domination
>>
>
Applied at rev. 170774.
Thanks,
Kai
Re: [patch libiberty include]: Add additional helper functions for directory-separator searching
> Date: Tue, 8 Mar 2011 11:56:45 +0100
> From: Kai Tietz
>
> +@deftypefn Extension int filename_dirchr (const char *@var{p})
> +
> +The returned value is similar to what @code{strchr} would return for
> +searching for a directory separator.
> +
> +This function does not normalize file name. However, it does handle
> +the fact that on DOS-like file systems, forward and backward slashes
> +are directory separators.
This is very mysterious. The documentation should explain how this is
"handled", or else the user will have no choice but to look in the
sources. And description "by similarity" doesn't help, because this
function is obviously different from strchr in _some_ ways, but you
don't say how.
While at that, explain the problem this solves, or else the raison
d'etre of this function will not be understood. We do want this
function to be used instead of just strchr, don't we? For it to be
used, its purpose and advantages should be well understood.
Btw, why do we need filename_dirchr? The use case for
filename_dirrchr is clear, but in what situations will we need the
other one?
> + if (!r || (r2 && r2 < r))
Why do you test for r2 being non-NULL? You are not going to
dereference it in the next comparison, and NULL is comparable as any
other value.
> +@deftypefn Extension int filename_dirrchr (const char *@var{p})
> +
> +The returned value is similar to what @code{strrchr} would return for
> +searching for a directory separator.
> +
> +This function does not normalize file name. However, it does handle
> +the fact that on DOS-like file systems, forward and backward slashes
> +are directory separators.
Same comments about this doc.
> + if (!r || (r2 && r2 > r))
And same comment here about testing r2 for non-NULL value.
Please also wait for others to review, as I'm not authorized to
approve the changes.
Thanks for working on this.
Re: [patch libiberty include]: Add additional helper functions for directory-separator searching
2011/3/8 Eli Zaretskii :
>> Date: Tue, 8 Mar 2011 11:56:45 +0100
>> From: Kai Tietz
>>
>> +@deftypefn Extension int filename_dirchr (const char *@var{p})
>> +
>> +The returned value is similar to what @code{strchr} would return for
>> +searching for a directory separator.
>> +
>> +This function does not normalize file name. However, it does handle
>> +the fact that on DOS-like file systems, forward and backward slashes
>> +are directory separators.
>
> This is very mysterious. The documentation should explain how this is
> "handled", or else the user will have no choice but to look in the
> sources. And description "by similarity" doesn't help, because this
> function is obviously different from strchr in _some_ ways, but you
> don't say how.
>
> While at that, explain the problem this solves, or else the raison
> d'etre of this function will not be understood. We do want this
> function to be used instead of just strchr, don't we? For it to be
> used, its purpose and advantages should be well understood.
>
> Btw, why do we need filename_dirchr? The use case for
> filename_dirrchr is clear, but in what situations will we need the
> other one?
As the comment notes. strchr/strrchr searches for one character. This
is for unix-file-system normally slash. On DOS based file-systems
there are two characters representing a directory-separator. Slash and
Backslash. Therefore this routine takes care that both are handled
similiar to a single character searching.
>> + if (!r || (r2 && r2 < r))
>
> Why do you test for r2 being non-NULL? You are not going to
> dereference it in the next comparison, and NULL is comparable as any
> other value.
As if we found slash, we don't want to override function's result by
backslash not found. If the null-check wouldn't be present condition
would be always true for r2 == NULL as, NULL is always less then a
pointer. But r shall be modified only if r2 (backslash) was found
before r (slash).
(same logic but here from right to left for the strrchr-case)
Regards,
Kai
[Patch, AVR] Housekeeping: Hookize REGISTER_MOVE_COST, MEMORY_MOVE_COST
This patch moves deprecated REGISTER_MOVE_COST resp. MEMORY_MOVE_COST
from avr.h to target hook avr_register_move_cost resp.
avr_memory_move_cost in avr.c.
No functionality added or removed; costs are unchanged.
2011-03-08 Georg-Johann Lay
* config/avr/avr.h (REGISTER_MOVE_COST, MEMORY_MOVE_COST): Remove.
* config/avr/avr.c (TARGET_REGISTER_MOVE_COST,
TARGET_MEMORY_MOVE_COST): Define.
(avr_register_move_cost, avr_memory_move_cost): New Functions.
Index: config/avr/avr.c
===
--- config/avr/avr.c (revision 170704)
+++ config/avr/avr.c (working copy)
@@ -82,6 +82,8 @@ static unsigned int avr_section_type_fla
static void avr_reorg (void);
static void avr_asm_out_ctor (rtx, int);
static void avr_asm_out_dtor (rtx, int);
+static int avr_register_move_cost (enum machine_mode, reg_class_t, reg_class_t);
+static int avr_memory_move_cost (enum machine_mode, reg_class_t, bool);
static int avr_operand_rtx_cost (rtx, enum machine_mode, enum rtx_code, bool);
static bool avr_rtx_costs (rtx, int, int, int *, bool);
static int avr_address_cost (rtx, bool);
@@ -174,6 +176,10 @@ static const struct default_options avr_
#define TARGET_INSERT_ATTRIBUTES avr_insert_attributes
#undef TARGET_SECTION_TYPE_FLAGS
#define TARGET_SECTION_TYPE_FLAGS avr_section_type_flags
+#undef TARGET_REGISTER_MOVE_COST
+#define TARGET_REGISTER_MOVE_COST avr_register_move_cost
+#undef TARGET_MEMORY_MOVE_COST
+#define TARGET_MEMORY_MOVE_COST avr_memory_move_cost
#undef TARGET_RTX_COSTS
#define TARGET_RTX_COSTS avr_rtx_costs
#undef TARGET_ADDRESS_COST
@@ -5132,6 +5138,32 @@ order_regs_for_local_alloc (void)
}
+/* Implement `TARGET_REGISTER_MOVE_COST' */
+
+static int
+avr_register_move_cost (enum machine_mode mode ATTRIBUTE_UNUSED,
+reg_class_t from, reg_class_t to)
+{
+ return (from == STACK_REG ? 6
+ : to == STACK_REG ? 12
+ : 2);
+}
+
+
+/* Implement `TARGET_MEMORY_MOVE_COST' */
+
+static int
+avr_memory_move_cost (enum machine_mode mode, reg_class_t rclass ATTRIBUTE_UNUSED,
+ bool in ATTRIBUTE_UNUSED)
+{
+ return (mode == QImode ? 2
+ : mode == HImode ? 4
+ : mode == SImode ? 8
+ : mode == SFmode ? 8
+ : 16);
+}
+
+
/* Mutually recursive subroutine of avr_rtx_cost for calculating the
cost of an RTX operand given its context. X is the rtx of the
operand, MODE is its mode, and OUTER is the rtx_code of this
Index: config/avr/avr.h
===
--- config/avr/avr.h (revision 170704)
+++ config/avr/avr.h (working copy)
@@ -447,15 +447,6 @@ do { \
#define LEGITIMATE_CONSTANT_P(X) 1
-#define REGISTER_MOVE_COST(MODE, FROM, TO) ((FROM) == STACK_REG ? 6 \
- : (TO) == STACK_REG ? 12 \
- : 2)
-
-#define MEMORY_MOVE_COST(MODE,CLASS,IN) ((MODE)==QImode ? 2 : \
- (MODE)==HImode ? 4 : \
- (MODE)==SImode ? 8 : \
- (MODE)==SFmode ? 8 : 16)
-
#define BRANCH_COST(speed_p, predictable_p) 0
#define SLOW_BYTE_ACCESS 0
Re: [patch libiberty include]: Add additional helper functions for directory-separator searching
> Date: Tue, 8 Mar 2011 12:25:37 +0100 > From: Kai Tietz > Cc: binut...@sourceware.org, gdb-patc...@sourceware.org, > gcc-patches@gcc.gnu.org > > > Btw, why do we need filename_dirchr? The use case for > > filename_dirrchr is clear, but in what situations will we need the > > other one? > > As the comment notes. strchr/strrchr searches for one character. This > is for unix-file-system normally slash. On DOS based file-systems > there are two characters representing a directory-separator. Slash and > Backslash. Therefore this routine takes care that both are handled > similiar to a single character searching. We are miscommunicating. I was asking when would a program want to find the _first_ directory separator character in a file name. Searching for the last one is a frequent use case: when you want to create a file in the same directory as another, or when you are looking for a basename of a file. But when do you need the first slash? > >> + if (!r || (r2 && r2 < r)) > > > > Why do you test for r2 being non-NULL? You are not going to > > dereference it in the next comparison, and NULL is comparable as any > > other value. > > As if we found slash, we don't want to override function's result by > backslash not found. If the null-check wouldn't be present condition > would be always true for r2 == NULL as, NULL is always less then a > pointer. But r shall be modified only if r2 (backslash) was found > before r (slash). > (same logic but here from right to left for the strrchr-case) But in strrchr-case, r2 cannot be greater than r1 if it is NULL, right?
Re: [patch libiberty include]: Add additional helper functions for directory-separator searching
Eli Zaretskii writes: > NULL is comparable as any other value. Only for equality. For relational operators the operands must point to the same object. Andreas. -- Andreas Schwab, sch...@redhat.com GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84 5EC7 45C6 250E 6F00 984E "And now for something completely different."
Re: [patch libiberty include]: Add additional helper functions for directory-separator searching
2011/3/8 Eli Zaretskii : >> Date: Tue, 8 Mar 2011 12:25:37 +0100 >> From: Kai Tietz >> Cc: binut...@sourceware.org, gdb-patc...@sourceware.org, >> gcc-patches@gcc.gnu.org >> >> > Btw, why do we need filename_dirchr? The use case for >> > filename_dirrchr is clear, but in what situations will we need the >> > other one? >> >> As the comment notes. strchr/strrchr searches for one character. This >> is for unix-file-system normally slash. On DOS based file-systems >> there are two characters representing a directory-separator. Slash and >> Backslash. Therefore this routine takes care that both are handled >> similiar to a single character searching. > > We are miscommunicating. I was asking when would a program want to > find the _first_ directory separator character in a file name. > Searching for the last one is a frequent use case: when you want to > create a file in the same directory as another, or when you are > looking for a basename of a file. But when do you need the first > slash? See for example remote-fileio.c in remote_fileio_extract_ptr_w_len() as an example. There is more then one use-case. >> >> + if (!r || (r2 && r2 < r)) >> > >> > Why do you test for r2 being non-NULL? You are not going to >> > dereference it in the next comparison, and NULL is comparable as any >> > other value. >> >> As if we found slash, we don't want to override function's result by >> backslash not found. If the null-check wouldn't be present condition >> would be always true for r2 == NULL as, NULL is always less then a >> pointer. But r shall be modified only if r2 (backslash) was found >> before r (slash). >> (same logic but here from right to left for the strrchr-case) > > But in strrchr-case, r2 cannot be greater than r1 if it is NULL, > right? It can. It is a matter of signness of pointer comparision. Regards, Kai
[PATCH] Decrease number of threads used by goroutines.go test
Hi!
On Fedora/RHEL we default to ulimit -Su 1024 (just soft-limit, to better
avoid non-root for bombs apparently).
goroutines.go test by default attempts to spawn 1 threads, which means
not only that goroutines.go test fails (no big deal), but that random other
tests that happen to be tested at the same time (I'm testing with make -j48)
sometimes fail too.
The following patch makes sure the tests spawns at most $[`ulimit -u`/4]
threads on Linux native, and just limits number of threads to 64 for
cross-testing and other OSes.
Tested on x86_64-linux with various values of ulimit -Su. Ok for trunk?
2011-03-08 Jakub Jelinek
* go.test/go-test.exp: For goroutines.go test pass
max($[`ulimit -u`/4], 1) as first argument, or 64 as a safe
default.
--- gcc/testsuite/go.test/go-test.exp.jj2011-01-15 11:26:32.0
+0100
+++ gcc/testsuite/go.test/go-test.exp 2011-03-08 13:23:36.078402148 +0100
@@ -265,6 +265,23 @@ proc go-gc-tests { } {
verbose -log "$test: go_execute_args is $go_execute_args"
set index [string last " $progargs" $test_line]
set test_line [string replace $test_line $index end]
+ } elseif { [string match "*go.test/test/chan/goroutines.go" $test] } {
+ # goroutines.go spawns by default 1 threads, which is too much
+ # for many OSes.
+ set go_execute_args 64
+ if { [ishost "*-linux*" ] && ![is_remote host] && ![is_remote
target] } {
+ # On Linux when using low ulimit -u limit, use maximum of
+ # a quarter of that limit and 1
+ set go_execute_args [lindex [remote_exec host {sh -c ulimit\
-u}] 1]
+ if { [string is integer -strict $go_execute_args] } {
+ set go_execute_args [expr $go_execute_args / 4]
+ if { $go_execute_args > 1 } { set go_execute_args
1 }
+ if { $go_execute_args < 16 } { set go_execute_args 16 }
+ } else {
+ set go_execute_args 64
+ }
+ }
+ verbose -log "$test: go_execute_args is $go_execute_args"
}
if { $test_line == "// \$G \$D/\$F\.go && \$L \$F\.\$A && \./\$A\.out
>tmp.go &&" \
Jakub
Re: [patch libiberty include]: Add additional helper functions for directory-separator searching
On Tuesday 08 March 2011 12:01:24, Kai Tietz wrote:
> See for example remote-fileio.c in remote_fileio_extract_ptr_w_len()
> as an example. There is more then one use-case.
That '/' has nothing to do with path separators. It's simply
a separator between a pointer and a length fields. E.g.,
@item Request:
@samp{Fopen,@var{pathptr}/@var{len},@var{flags},@var{mode}}
@var{pathptr} is a pointer that points to the real path
in the inferior's memory, not a path string.
--
Pedro Alves
Re: [patch libiberty include]: Add additional helper functions for directory-separator searching
2011/3/8 Pedro Alves :
> On Tuesday 08 March 2011 12:01:24, Kai Tietz wrote:
>> See for example remote-fileio.c in remote_fileio_extract_ptr_w_len()
>> as an example. There is more then one use-case.
>
> That '/' has nothing to do with path separators. It's simply
> a separator between a pointer and a length fields. E.g.,
>
> @item Request:
> @samp{Fopen,@var{pathptr}/@var{len},@var{flags},@var{mode}}
>
> @var{pathptr} is a pointer that points to the real path
> in the inferior's memory, not a path string.
>
> --
> Pedro Alves
>
Well, a better example is elfstab_offset_sections() in elfread.c.
Another is in find_file_and_directory() in dwarf2read.c file.
Regards,
Kai
Re: [PATCH] unbreak gcc.dg/tree-ssa/ssa-ccp-33.c on m68k (PR testsuite/47954)
Richard Guenther writes: > On Tue, Mar 8, 2011 at 10:43 AM, Mikael Pettersson wrote: > > gcc.dg/tree-ssa/ssa-ccp-33.c fails with gcc trunk on m68k-linux: > > > > ssa-ccp-33.c:(.text+0x2a): undefined reference to `link_error' > > ssa-ccp-33.c:(.text+0x2a): undefined reference to `link_error' > > FAIL: gcc.dg/tree-ssa/ssa-ccp-33.c (test for excess errors) > > ssa-ccp-33.c:(.text+0x2a): undefined reference to `link_error' > > > > The test case checks that the compiler is able to eliminate a > > runtime check that an aligned pointer-to-int remains aligned after > > a loop of increments. It uses sizeof to compute the alignment > > of int, but on m68k (and possibly others) the alignment of int > > is less than its size. The compiler is then unable to eliminate > > the broken alignment check, and the call to link_error () is not > > removed. > > > > Fixed by using __alignof__ instead. Regression tested on m68k-linux > > where it eliminated the FAIL for gcc.dg/tree-ssa/ssa-ccp-33.c. > > Also tested on i686-linux, no changes there. > > > > Ok for trunk? > > > > (Richard G. pre-approved this change on the PR entry, however I > > cannot commit it myself.) > > Committed. > Richard. Thanks!
[PATCH] avoid memory overrun in a test leading to potential double-free
I ran "make check" and was dismayed to see that glibc detected a double-free. At first I thought it must be my fault, since I'd been removing useless tests before free, but no... Running "valgrind ./test-expandargv" confirmed it: ==29710== Conditional jump or move depends on uninitialised value(s) ==29710==at 0x400E14: run_replaces (test-expandargv.c:121) ==29710==by 0x400F63: writeout_test (test-expandargv.c:151) ==29710==by 0x401037: run_tests (test-expandargv.c:188) ==29710==by 0x40124C: main (test-expandargv.c:264) >From f60778ef0f07983b0ba72ed97fe52b687de28abb Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Tue, 8 Mar 2011 13:54:13 +0100 Subject: [PATCH] avoid memory overrun in a test leading to potential double-free * testsuite/test-expandargv.c (writeout_test): Fix off-by-one error: i.e., do copy the trailing NUL byte. --- libiberty/ChangeLog |6 ++ libiberty/testsuite/test-expandargv.c |2 +- 2 files changed, 7 insertions(+), 1 deletions(-) diff --git a/libiberty/ChangeLog b/libiberty/ChangeLog index dc92638..802cf96 100644 --- a/libiberty/ChangeLog +++ b/libiberty/ChangeLog @@ -1,6 +1,12 @@ +2011-03-08 Jim Meyering + + avoid memory overrun in a test leading to potential double-free + * testsuite/test-expandargv.c (writeout_test): Fix off-by-one error: + i.e., do copy the trailing NUL byte. + 2011-02-28 Kai Tietz * filename_cmp.c (filename_ncmp): New function. * functions.texi: Regenerated. 2011-02-03 Ralf Wildenhues diff --git a/libiberty/testsuite/test-expandargv.c b/libiberty/testsuite/test-expandargv.c index c16a032..57b96b3 100644 --- a/libiberty/testsuite/test-expandargv.c +++ b/libiberty/testsuite/test-expandargv.c @@ -201,13 +201,13 @@ writeout_test (int test, const char * test_data) /* Generate RW copy of data for replaces */ len = strlen (test_data); parse = malloc (sizeof (char) * (len + 1)); if (parse == NULL) fatal_error (__LINE__, "Failed to malloc parse.", errno); - memcpy (parse, test_data, sizeof (char) * len); + memcpy (parse, test_data, sizeof (char) * (len + 1)); /* Run all possible replaces */ run_replaces (parse); fwrite (parse, len, sizeof (char), fd); free (parse); fclose (fd); -- 1.7.4.1.299.ga459d
Re: [PATCH] PR c++/47957
OK. Jason
Re: [patch libiberty include]: Add additional helper functions for directory-separator searching
On Tuesday 08 March 2011 12:48:11, Kai Tietz wrote: > Well, a better example is elfstab_offset_sections() in elfread.c. /* The ELF symbol info doesn't include path names, so strip the path (if any) from the psymtab filename. */ while (0 != (p = strchr (filename, '/'))) filename = p + 1; Looks like its looking for the last path separator, so it might as well use filename_dirrchr instead. > Another is in find_file_and_directory() in dwarf2read.c file. Workaround for Irix. Certainly that '/' should not depend on the host gdb is running on. -- Pedro Alves
[PATCH] PR 47836, Some Cross Compiler can't build target-libiberty or target-zlib
Hello
This is more generalized way to give a user the ability to override
the generation of target libraries, that are enabled per default.
For example with the configure switch --disable-target-zlib,
target-zlib is added to the script variable noconfigdirs and this
target library will not be built.
regards
Thomas
PS.
If it helps. I've already done copyright assignment for future changes.
But, not explicit for the configure scripts.
Also I don't have write permission, (nor I'm requesting for this).
2011-03-08 Thomas Klein
PR 47836
* configure.ac: accept --disable-target-.. from user
* configure: Regenerate.
Index: configure.ac
===
--- configure.ac(revision 170774)
+++ configure.ac(working copy)
@@ -2081,6 +2081,28 @@ case ,${enable_languages},:${enable_objc_gc} in
;;
esac
+# a user forced "--disable-target-.." was given
+# add this to the ingnore list if not already present
+for target_lib_var in $target_libraries
+do
+ var=`$as_echo "$target_lib_var" | sed 's/[[-+.]]/_/g'`
+ eval is_enabled=\$enable_$var
+ if test x$is_enabled = xno ; then
+append_var=yes
+for var in $noconfigdirs $skipdirs
+do
+ if test x$var = x$target_lib_var ; then
+append_var=no
+ break
+ fi
+done
+if test x$append_var = xyes ; then
+ noconfigdirs="$noconfigdirs $target_lib_var"
+ echo "add $target_lib_var to noconfigdirs"
+fi
+ fi
+done
+
# Remove the entries in $skipdirs and $noconfigdirs from $configdirs,
# $build_configdirs and $target_configdirs.
# If we have the source for $noconfigdirs entries, add them to $notsupp.
Index: configure
===
--- configure (revision 170774)
+++ configure (working copy)
@@ -6546,6 +6546,28 @@ case ,${enable_languages},:${enable_objc_gc} in
;;
esac
+# a user forced "--disable-target-.." was given
+# add this to the ingnore list if not already present
+for target_lib_var in $target_libraries
+do
+ var=`$as_echo "$target_lib_var" | sed 's/[-+.]/_/g'`
+ eval is_enabled=\$enable_$var
+ if test x$is_enabled = xno ; then
+append_var=yes
+for var in $noconfigdirs $skipdirs
+do
+ if test x$var = x$target_lib_var ; then
+append_var=no
+ break
+ fi
+done
+if test x$append_var = xyes ; then
+ noconfigdirs="$noconfigdirs $target_lib_var"
+ echo "add $target_lib_var to noconfigdirs"
+fi
+ fi
+done
+
# Remove the entries in $skipdirs and $noconfigdirs from $configdirs,
# $build_configdirs and $target_configdirs.
# If we have the source for $noconfigdirs entries, add them to $notsupp.
Re: [patch libiberty include]: Add additional helper functions for directory-separator searching
2011/3/8 Pedro Alves : > On Tuesday 08 March 2011 12:48:11, Kai Tietz wrote: > >> Well, a better example is elfstab_offset_sections() in elfread.c. > > /* The ELF symbol info doesn't include path names, so strip the path > (if any) from the psymtab filename. */ > while (0 != (p = strchr (filename, '/'))) > filename = p + 1; > > Looks like its looking for the last path separator, so > it might as well use filename_dirrchr instead. True, see patch I've posted about filename_cmp. I replaced it there by a strrchr search. >> Another is in find_file_and_directory() in dwarf2read.c file. > > Workaround for Irix. Certainly that '/' should not depend > on the host gdb is running on. Right. But well, I was asked if strchr is used in combination with paths. And so I've shown. If those uses could be rewritten is a different story and might be true. Kai
Re: [PATCH v2] Re: avoid useless if-before-free tests
Jim Meyering writes: > I've taken the liberty of letting my editor remove trailing > blanks in ChangeLog files. I hope that's ok. Also in ChangeLogs, > I converted some leading 8-space (and 7-space) sequences to single TABs. Please move this to a separate patch: this is completely unrelated to the change at hand. And please don't change the alignment of entries with multiple email addresses. > I found no ChangeLog for libgo and no other libgo-related entries. > I suspect that means I should omit this change because it belongs upstream. Indeed, cf. libgo/README*. I've Cc'ed Ian who maintains this part of GCC. > For now, I've added an entry in the top-level ChangeLog file: > * libgo/runtime/go-select.c (__go_select): > It's easier to remove than to add. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [PING^2] [PR46003, C++] Fix the assertion failure in build_target_expr
On 03/07/2011 01:27 PM, Yufeng Zhang wrote: On 03/03/2011 11:39 PM, Jason Merrill wrote: I think rather that the problem is in build_conditional_expr; it should have a template case that just determines the appropriate type and then builds up a COND_EXPR with that type from the unconverted operands, like we do for calls in finish_call_expr. Thanks for the suggestion. I spent some more time looking into the related code. If I understand it correctly, I think this is just what build_x_conditional_expr in ./cp/typeck.c does: Yes. The problem is that build_conditional_expr is actually trying to do the conversions; it should just determine what the conversions would be and then stop there. That's what we do with calls--we determine all the desired conversions and pass them into build_over_call, which just ignores them. Actually performing the conversions can wait until instantiation tme. Different from CALL_EXPR, the TREE_TYPE of a COND_EXPR is determined by the TREE_TYPEs of its latter two operands. When the types of the two operands are different, a conversion is attempted on each operand type to match the type of the other operand; while for a CALL_EXPR, its TREE_TYPE is independent from the TREE_TYPE(s) of its arg(s). I don't think the difference is significant; a call to an overloaded function also depends on the types of its arguments. We need to determine the conversions from argument to parameter types in order to select the right function, just as in a ?: expression we need to determine the conversions from the 2nd and 3rd operands to a result type. Jason
Re: [patch libiberty include]: Add additional helper functions for directory-separator searching
Hi,
I update the patch and put those new function into a separate file.
ChangeLog include/
2011-03-08 Kai Tietz
* filenames.h (filename_dirchr): New prototype.
(filename_dirrchr): Likewise.
ChangeLog libiberty/
2011-03-08 Kai Tietz
* filename_chr.c: New file.
* Makefile.in (CFILES): Add filename_chr.c file.
(REQUIRED_OFILES): Add filename_chr.o
(filename_chr.o): New rule.
* functions.texi: Regenerated.
Tested for x86_64-pc-linux-gnu and x86_64-w64-mingw32. Ok for apply?
Regards,
Kai
Index: gcc/include/filenames.h
===
--- gcc.orig/include/filenames.h2011-02-28 19:16:35.0 +0100
+++ gcc/include/filenames.h 2011-03-08 11:11:10.909109700 +0100
@@ -75,6 +75,8 @@ extern int filename_cmp (const char *s1,
extern int filename_ncmp (const char *s1, const char *s2,
size_t n);
+extern char *filename_dirchr (const char *p);
+extern char *filename_dirrchr (const char *p);
#ifdef __cplusplus
}
Index: gcc/libiberty/functions.texi
===
--- gcc.orig/libiberty/functions.texi 2011-02-28 19:16:35.0 +0100
+++ gcc/libiberty/functions.texi2011-03-08 16:02:12.147905400 +0100
@@ -296,6 +296,30 @@ and backward slashes are equal.
@end deftypefn
+@c filename_chr.c:32
+@deftypefn Extension int filename_dirchr (const char *@var{p})
+
+The returned value is similar to what @code{strchr} would return for
+searching for a directory separator.
+
+This function does not normalize file name. However, it does handle
+the fact that on DOS-like file systems, forward and backward slashes
+are directory separators.
+
+@end deftypefn
+
+@c filename_chr.c:65
+@deftypefn Extension int filename_dirrchr (const char *@var{p})
+
+The returned value is similar to what @code{strrchr} would return for
+searching for a directory separator.
+
+This function does not normalize file name. However, it does handle
+the fact that on DOS-like file systems, forward and backward slashes
+are directory separators.
+
+@end deftypefn
+
@c filename_cmp.c:81
@deftypefn Extension int filename_ncmp (const char *@var{s1}, const char
*@var{s2}, size_t @var{n})
Index: gcc/libiberty/Makefile.in
===
--- gcc.orig/libiberty/Makefile.in 2010-11-21 14:28:05.0 +0100
+++ gcc/libiberty/Makefile.in 2011-03-08 16:01:09.254418900 +0100
@@ -127,8 +127,8 @@ CFILES = alloca.c argv.c asprintf.c atex
calloc.c choose-temp.c clock.c concat.c cp-demangle.c \
cp-demint.c cplus-dem.c crc32.c\
dyn-string.c\
- fdmatch.c ffs.c fibheap.c filename_cmp.c floatformat.c \
- fnmatch.c fopen_unlocked.c \
+ fdmatch.c ffs.c fibheap.c filename_cmp.c filename_chr.c \
+ floatformat.c fnmatch.c fopen_unlocked.c\
getcwd.c getopt.c getopt1.c getpagesize.c getpwd.c getruntime.c \
gettimeofday.c \
hashtab.c hex.c \
@@ -168,7 +168,8 @@ REQUIRED_OFILES =
\
./choose-temp.$(objext) ./concat.$(objext) \
./cp-demint.$(objext) ./crc32.$(objext) ./dyn-string.$(objext) \
./fdmatch.$(objext) ./fibheap.$(objext) \
- ./filename_cmp.$(objext) ./floatformat.$(objext)\
+ ./filename_cmp.$(objext) ./filename_chr.$(objext) \
+ ./floatformat.$(objext) \
./fnmatch.$(objext) ./fopen_unlocked.$(objext) \
./getopt.$(objext) ./getopt1.$(objext) ./getpwd.$(objext) \
./getruntime.$(objext) ./hashtab.$(objext) ./hex.$(objext) \
@@ -653,6 +654,13 @@ $(CONFIGURED_OFILES): stamp-picdir
else true; fi
$(COMPILE.c) $(srcdir)/filename_cmp.c $(OUTPUT_OPTION)
+./filename_chr.$(objext): $(srcdir)/filename_chr.c config.h
$(INCDIR)/filenames.h \
+ $(INCDIR)/safe-ctype.h
+ if [ x"$(PICFLAG)" != x ]; then \
+ $(COMPILE.c) $(PICFLAG) $(srcdir)/filename_chr.c -o pic/$@; \
+ else true; fi
+ $(COMPILE.c) $(srcdir)/filename_chr.c $(OUTPUT_OPTION)
+
./floatformat.$(objext): $(srcdir)/floatformat.c config.h $(INCDIR)/ansidecl.h
\
$(INCDIR)/floatformat.h $(INCDIR)/libiberty.h
if [ x"$(PICFLAG)" != x ]; then \
Index: gcc/libiberty/filename_chr.c
===
--- /dev/null 1970-01-01 00:00:00.0 +
+++ gcc/libiberty/filename_chr.c2011-03-08 15:55:30.86300 +0100
@@ -0,0 +1,95 @@
+/*
Re: [PATCH] Decrease number of threads used by goroutines.go test
Jakub Jelinek writes: > 2011-03-08 Jakub Jelinek > > * go.test/go-test.exp: For goroutines.go test pass > max($[`ulimit -u`/4], 1) as first argument, or 64 as a safe > default. How about if we do this unless the environment variable GCCGO_RUN_ALL_TESTS is set, so that people have a way to run the full testsuite. I can also change the libgo testsuite to only run the networking tests if that environment variable is set. This patch is OK with that change. Thanks for doing it. Ian
Re: [PATCH] Decrease number of threads used by goroutines.go test
On Tue, Mar 08, 2011 at 07:20:28AM -0800, Ian Lance Taylor wrote: > Jakub Jelinek writes: > > > 2011-03-08 Jakub Jelinek > > > > * go.test/go-test.exp: For goroutines.go test pass > > max($[`ulimit -u`/4], 1) as first argument, or 64 as a safe > > default. > > How about if we do this unless the environment variable > GCCGO_RUN_ALL_TESTS is set, so that people have a way to run the full > testsuite. I can also change the libgo testsuite to only run the > networking tests if that environment variable is set. This patch is OK > with that change. Thanks for doing it. Perhaps we could just use $[`ulimit -u`-100] or similar limit instead in that case, but if you know you can't run more than 1024 threads simultaneously and run it anyway, it is always going to fail and likely break other tests too. Sure, the "safe" fallback of 64 maybe shouldn't be used in that case. Jakub
Re: [PATCH] Decrease number of threads used by goroutines.go test
Ian Lance Taylor writes: > Jakub Jelinek writes: > >> 2011-03-08 Jakub Jelinek >> >> * go.test/go-test.exp: For goroutines.go test pass >> max($[`ulimit -u`/4], 1) as first argument, or 64 as a safe >> default. > > How about if we do this unless the environment variable > GCCGO_RUN_ALL_TESTS is set, so that people have a way to run the full > testsuite. I can also change the libgo testsuite to only run the > networking tests if that environment variable is set. This patch is OK > with that change. Thanks for doing it. Alternatively, one might use GCC_TEST_RUN_EXPENSIVE, which is already used to control dg-require-effective-target run_expensive_tests. This avoids separate mechanisms per testsuite/language. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [PATCH v2] Re: avoid useless if-before-free tests
Rainer Orth writes: >> I found no ChangeLog for libgo and no other libgo-related entries. >> I suspect that means I should omit this change because it belongs upstream. > > Indeed, cf. libgo/README*. I've Cc'ed Ian who maintains this part of > GCC. Yes, libgo is upstream but in any case I'd prefer that you not make the change to libgo/runtime/go-select.c. I'm aware that free (NULL) is a no-op but I wrote the code that way because it is extremely unlikely that allocated_buffer != NULL. I am simply inlining the common case. Thanks. Ian
Re: [patch libiberty include]: Add additional helper functions for directory-separator searching
Umm, sorry. I found a wrong copy & paste. So I re-sent the corrected
patch. Additionally I adjuste the changes in Makefile.in so, that
alphabetic order remains.
Kai
Index: gcc/include/filenames.h
===
--- gcc.orig/include/filenames.h2011-02-28 19:16:35.0 +0100
+++ gcc/include/filenames.h 2011-03-08 11:11:10.909109700 +0100
@@ -75,6 +75,8 @@ extern int filename_cmp (const char *s1,
extern int filename_ncmp (const char *s1, const char *s2,
size_t n);
+extern char *filename_dirchr (const char *p);
+extern char *filename_dirrchr (const char *p);
#ifdef __cplusplus
}
Index: gcc/libiberty/functions.texi
===
--- gcc.orig/libiberty/functions.texi 2011-02-28 19:16:35.0 +0100
+++ gcc/libiberty/functions.texi2011-03-08 16:26:29.547971700 +0100
@@ -296,6 +296,30 @@ and backward slashes are equal.
@end deftypefn
+@c filename_chr.c:32
+@deftypefn Extension int filename_dirchr (const char *@var{p})
+
+The returned value is similar to what @code{strchr} would return for
+searching for a directory separator.
+
+This function does not normalize file name. However, it does handle
+the fact that on DOS-like file systems, forward and backward slashes
+are directory separators.
+
+@end deftypefn
+
+@c filename_chr.c:65
+@deftypefn Extension int filename_dirrchr (const char *@var{p})
+
+The returned value is similar to what @code{strrchr} would return for
+searching for a directory separator.
+
+This function does not normalize file name. However, it does handle
+the fact that on DOS-like file systems, forward and backward slashes
+are directory separators.
+
+@end deftypefn
+
@c filename_cmp.c:81
@deftypefn Extension int filename_ncmp (const char *@var{s1}, const char
*@var{s2}, size_t @var{n})
Index: gcc/libiberty/Makefile.in
===
--- gcc.orig/libiberty/Makefile.in 2010-11-21 14:28:05.0 +0100
+++ gcc/libiberty/Makefile.in 2011-03-08 16:24:17.703229600 +0100
@@ -127,8 +127,8 @@ CFILES = alloca.c argv.c asprintf.c atex
calloc.c choose-temp.c clock.c concat.c cp-demangle.c \
cp-demint.c cplus-dem.c crc32.c\
dyn-string.c\
- fdmatch.c ffs.c fibheap.c filename_cmp.c floatformat.c \
- fnmatch.c fopen_unlocked.c \
+ fdmatch.c ffs.c fibheap.c filename_chr.c filename_cmp.c \
+ floatformat.c fnmatch.c fopen_unlocked.c\
getcwd.c getopt.c getopt1.c getpagesize.c getpwd.c getruntime.c \
gettimeofday.c \
hashtab.c hex.c \
@@ -168,7 +168,8 @@ REQUIRED_OFILES =
\
./choose-temp.$(objext) ./concat.$(objext) \
./cp-demint.$(objext) ./crc32.$(objext) ./dyn-string.$(objext) \
./fdmatch.$(objext) ./fibheap.$(objext) \
- ./filename_cmp.$(objext) ./floatformat.$(objext)\
+ ./filename_chr.$(objext) ./filename_cmp.$(objext) \
+ ./floatformat.$(objext) \
./fnmatch.$(objext) ./fopen_unlocked.$(objext) \
./getopt.$(objext) ./getopt1.$(objext) ./getpwd.$(objext) \
./getruntime.$(objext) ./hashtab.$(objext) ./hex.$(objext) \
@@ -646,6 +647,13 @@ $(CONFIGURED_OFILES): stamp-picdir
else true; fi
$(COMPILE.c) $(srcdir)/fibheap.c $(OUTPUT_OPTION)
+./filename_chr.$(objext): $(srcdir)/filename_chr.c config.h
$(INCDIR)/filenames.h \
+ $(INCDIR)/safe-ctype.h
+ if [ x"$(PICFLAG)" != x ]; then \
+ $(COMPILE.c) $(PICFLAG) $(srcdir)/filename_chr.c -o pic/$@; \
+ else true; fi
+ $(COMPILE.c) $(srcdir)/filename_chr.c $(OUTPUT_OPTION)
+
./filename_cmp.$(objext): $(srcdir)/filename_cmp.c config.h
$(INCDIR)/filenames.h \
$(INCDIR)/safe-ctype.h
if [ x"$(PICFLAG)" != x ]; then \
Index: gcc/libiberty/filename_chr.c
===
--- /dev/null 1970-01-01 00:00:00.0 +
+++ gcc/libiberty/filename_chr.c2011-03-08 16:22:51.303258200 +0100
@@ -0,0 +1,95 @@
+/* File name character searching routines.
+
+ Copyright (C) 2011 Free Software Foundation, Inc.
+
+ This program 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 2, or (at your option)
+ any later version.
+
+ This program is distributed in the hope that it will be useful,
+ b
[PATCH] Rerun df_analyze if delete_trivially_dead_insns deleted something during IRA (PR debug/47881)
Hi!
If delete_trivially_dead_insns deletes some insn, DF state might be
out of date, and, what's worse, if there are pseudos set multiple times
referenced in debug_insns, where the debug_insn references were ok
before the deletions, but the deletions shortened lifetime of such a pseudo
in certain location, nothing resets the debug insns afterwards and we end up
with -fcompare-debug failures because the lifetime of the pseudos is
different between -g0 and -g.
Fixed by rerunning df_analyze () if delete_trivially_dead_insns removed
anything, bootstrapped/regtested on x86_64-linux and i686-linux.
Ok for trunk?
2011-03-07 Jakub Jelinek
PR debug/47881
* ira.c (ira): Call df_analyze again if delete_trivially_dead_insns
removed anything.
* gcc.dg/pr47881.c: New test.
--- gcc/ira.c.jj2011-02-21 15:37:42.0 +0100
+++ gcc/ira.c 2011-03-07 12:33:59.0 +0100
@@ -3232,7 +3232,8 @@ ira (FILE *f)
check_allocation ();
#endif
- delete_trivially_dead_insns (get_insns (), max_reg_num ());
+ if (delete_trivially_dead_insns (get_insns (), max_reg_num ()))
+df_analyze ();
init_reg_equiv_memory_loc ();
--- gcc/testsuite/gcc.dg/pr47881.c.jj 2011-03-08 14:12:04.0 +0100
+++ gcc/testsuite/gcc.dg/pr47881.c 2011-03-08 14:11:46.0 +0100
@@ -0,0 +1,24 @@
+/* PR debug/47881 */
+/* { dg-do compile } */
+/* { dg-options "-O -fcompare-debug -fno-dce -funroll-loops -fno-web" } */
+
+extern int data[];
+
+int
+foo (int *t, int *f, int n)
+{
+ int i = 0, a, b, c, d;
+ while (data[*f] && n)
+n--;
+ for (; i < n; i += 4)
+{
+ a = data[*(f++) & 0x7f];
+ c = data[*(f++) & 0x7f];
+ c = data[*(f++) & 0x7f];
+ d = data[*(f++) & 0x7f];
+ if ((a & 0x80) || (b & 0x80) || (c & 0x80) || (d & 0x80))
+ return 1;
+ *(t++) = 16;
+}
+ return 0;
+}
Jakub
Re: [PATCH v2] Re: avoid useless if-before-free tests
On Tue, Mar 08, 2011 at 07:28:37AM -0800, Ian Lance Taylor wrote: > Rainer Orth writes: > > >> I found no ChangeLog for libgo and no other libgo-related entries. > >> I suspect that means I should omit this change because it belongs upstream. > > > > Indeed, cf. libgo/README*. I've Cc'ed Ian who maintains this part of > > GCC. > > Yes, libgo is upstream but in any case I'd prefer that you not make the > change to libgo/runtime/go-select.c. I'm aware that free (NULL) is a > no-op but I wrote the code that way because it is extremely unlikely > that allocated_buffer != NULL. I am simply inlining the common case. > Thanks. I guess such reason is sometimes legitimate, but it would be nice (and better for generated code) to make it explicit in that case, i.e. if (__builtin_expect (allocated_buffer != NULL, 0)) free (allocated_buffer); could be a sign for anyone coming after Jim that this case is on purpose and should be left as is. allocated_buffer != NULL is normally predicted as true, not false. Jakub
Re: [4.7] Avoid global state in v850_handle_option
Hi Joseph, Tested building cc1 and xgcc for cross to v850-elf. Will commit to trunk for 4.7 in the absence of target maintainer objections. No objections - please apply. 2011-03-07 Joseph Myers * config/v850/v850-opts.h: New. * config/v850/v850.c (small_memory): Replace with small_memory_physical_max array. Make that array static const. (v850_handle_memory_option): Take integer value of argument. Take gcc_options pointer, option text and location. Return void. Update for changes to small memory structures. (v850_handle_option): Access target_flags via opts pointer. Don't assert that global structures are in use. Update calls to v850_handle_memory_option. (v850_encode_data_area): Update references to small memory settings. * config/v850/v850.h (struct small_memory_info, small_memory): Remove. (enum small_memory_type): Move to v850-opts.h. * config/v850/v850.opt (config/v850/v850-opts.h): New HeaderInclude entry. (small_memory_max): New Variable entry. (msda): Replace by pair of options msda= and msda-. Use UInteger. (mtda, mzda): Likewise.
Re: [PATCH] Decrease number of threads used by goroutines.go test
On Tue, Mar 08, 2011 at 04:27:04PM +0100, Rainer Orth wrote: > >> 2011-03-08 Jakub Jelinek > >> > >>* go.test/go-test.exp: For goroutines.go test pass > >>max($[`ulimit -u`/4], 1) as first argument, or 64 as a safe > >>default. > > > > How about if we do this unless the environment variable > > GCCGO_RUN_ALL_TESTS is set, so that people have a way to run the full > > testsuite. I can also change the libgo testsuite to only run the > > networking tests if that environment variable is set. This patch is OK > > with that change. Thanks for doing it. > > Alternatively, one might use GCC_TEST_RUN_EXPENSIVE, which is already > used to control dg-require-effective-target run_expensive_tests. This > avoids separate mechanisms per testsuite/language. I guess [getenv GCC_TEST_RUN_EXPENSIVE] != "" could be a usable test here, if false it could always use 64 threads or something like that, if true it should IMNSHO still bound it to at most max($[`ulimit -u`/2], 1) when ulimit -u is available, because running the thread when it is known to break other things is a bad idea. But of course if Ian wants to guard networking libgo tests with some env var, GCC_TEST_RUN_EXPENSIVE probably isn't an env var to use... Jakub
Re: [PATCH] Rerun df_analyze if delete_trivially_dead_insns deleted something during IRA (PR debug/47881)
On 03/08/2011 10:33 AM, Jakub Jelinek wrote:
Hi!
If delete_trivially_dead_insns deletes some insn, DF state might be
out of date, and, what's worse, if there are pseudos set multiple times
referenced in debug_insns, where the debug_insn references were ok
before the deletions, but the deletions shortened lifetime of such a pseudo
in certain location, nothing resets the debug insns afterwards and we end up
with -fcompare-debug failures because the lifetime of the pseudos is
different between -g0 and -g.
Fixed by rerunning df_analyze () if delete_trivially_dead_insns removed
anything, bootstrapped/regtested on x86_64-linux and i686-linux.
Ok for trunk?
Ok for me. Thanks for the patch, Jakub.
2011-03-07 Jakub Jelinek
PR debug/47881
* ira.c (ira): Call df_analyze again if delete_trivially_dead_insns
removed anything.
* gcc.dg/pr47881.c: New test.
--- gcc/ira.c.jj2011-02-21 15:37:42.0 +0100
+++ gcc/ira.c 2011-03-07 12:33:59.0 +0100
@@ -3232,7 +3232,8 @@ ira (FILE *f)
check_allocation ();
#endif
- delete_trivially_dead_insns (get_insns (), max_reg_num ());
+ if (delete_trivially_dead_insns (get_insns (), max_reg_num ()))
+df_analyze ();
init_reg_equiv_memory_loc ();
--- gcc/testsuite/gcc.dg/pr47881.c.jj 2011-03-08 14:12:04.0 +0100
+++ gcc/testsuite/gcc.dg/pr47881.c 2011-03-08 14:11:46.0 +0100
@@ -0,0 +1,24 @@
+/* PR debug/47881 */
+/* { dg-do compile } */
+/* { dg-options "-O -fcompare-debug -fno-dce -funroll-loops -fno-web" } */
+
+extern int data[];
+
+int
+foo (int *t, int *f, int n)
+{
+ int i = 0, a, b, c, d;
+ while (data[*f]&& n)
+n--;
+ for (; i< n; i += 4)
+{
+ a = data[*(f++)& 0x7f];
+ c = data[*(f++)& 0x7f];
+ c = data[*(f++)& 0x7f];
+ d = data[*(f++)& 0x7f];
+ if ((a& 0x80) || (b& 0x80) || (c& 0x80) || (d& 0x80))
+ return 1;
+ *(t++) = 16;
+}
+ return 0;
+}
Jakub
Re: [PATCH] Decrease number of threads used by goroutines.go test
Jakub Jelinek writes: > On Tue, Mar 08, 2011 at 07:20:28AM -0800, Ian Lance Taylor wrote: >> Jakub Jelinek writes: >> >> > 2011-03-08 Jakub Jelinek >> > >> >* go.test/go-test.exp: For goroutines.go test pass >> >max($[`ulimit -u`/4], 1) as first argument, or 64 as a safe >> >default. >> >> How about if we do this unless the environment variable >> GCCGO_RUN_ALL_TESTS is set, so that people have a way to run the full >> testsuite. I can also change the libgo testsuite to only run the >> networking tests if that environment variable is set. This patch is OK >> with that change. Thanks for doing it. > > Perhaps we could just use $[`ulimit -u`-100] or similar limit instead > in that case, but if you know you can't run more than 1024 threads > simultaneously and run it anyway, it is always going to fail and likely > break other tests too. Sure, the "safe" fallback of 64 maybe shouldn't be > used in that case. I wouldn't worry about it too much because this is all going to change completely at some point anyhow, hopefully sooner rather than later. I'm going to change libgo to not use a separate thread for every goroutine. I'd just like to have a way to run the full testsuite for now. Ian
Re: [PATCH] Decrease number of threads used by goroutines.go test
Rainer Orth writes: > Ian Lance Taylor writes: > >> Jakub Jelinek writes: >> >>> 2011-03-08 Jakub Jelinek >>> >>> * go.test/go-test.exp: For goroutines.go test pass >>> max($[`ulimit -u`/4], 1) as first argument, or 64 as a safe >>> default. >> >> How about if we do this unless the environment variable >> GCCGO_RUN_ALL_TESTS is set, so that people have a way to run the full >> testsuite. I can also change the libgo testsuite to only run the >> networking tests if that environment variable is set. This patch is OK >> with that change. Thanks for doing it. > > Alternatively, one might use GCC_TEST_RUN_EXPENSIVE, which is already > used to control dg-require-effective-target run_expensive_tests. This > avoids separate mechanisms per testsuite/language. Works for me. Thanks. Ian
Re: [PATCH] Decrease number of threads used by goroutines.go test
Jakub Jelinek writes: > I guess [getenv GCC_TEST_RUN_EXPENSIVE] != "" could be a usable test here, > if false it could always use 64 threads or something like that, if true > it should IMNSHO still bound it to at most max($[`ulimit -u`/2], 1) > when ulimit -u is available, because running the thread when it is known > to break other things is a bad idea. Right. > But of course if Ian wants to guard networking libgo tests with some env > var, GCC_TEST_RUN_EXPENSIVE probably isn't an env var to use... Indeed, although I'd prefer a different solution, as outlined in PR go/48017. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [PATCH] Decrease number of threads used by goroutines.go test
Jakub Jelinek writes: > On Tue, Mar 08, 2011 at 04:27:04PM +0100, Rainer Orth wrote: >> >> 2011-03-08 Jakub Jelinek >> >> >> >> * go.test/go-test.exp: For goroutines.go test pass >> >> max($[`ulimit -u`/4], 1) as first argument, or 64 as a safe >> >> default. >> > >> > How about if we do this unless the environment variable >> > GCCGO_RUN_ALL_TESTS is set, so that people have a way to run the full >> > testsuite. I can also change the libgo testsuite to only run the >> > networking tests if that environment variable is set. This patch is OK >> > with that change. Thanks for doing it. >> >> Alternatively, one might use GCC_TEST_RUN_EXPENSIVE, which is already >> used to control dg-require-effective-target run_expensive_tests. This >> avoids separate mechanisms per testsuite/language. > > I guess [getenv GCC_TEST_RUN_EXPENSIVE] != "" could be a usable test here, > if false it could always use 64 threads or something like that, if true > it should IMNSHO still bound it to at most max($[`ulimit -u`/2], 1) > when ulimit -u is available, because running the thread when it is known > to break other things is a bad idea. > But of course if Ian wants to guard networking libgo tests with some env > var, GCC_TEST_RUN_EXPENSIVE probably isn't an env var to use... I don't really care what environment variable we use, I just want some way to run the full test, without having DejaGNU silently change the test on me. It's perfectly reasonable to have the default check ulimit, I just want some way to not check it. Ian
Re: avoid useless if-before-free tests
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 03/05/11 07:05, Jim Meyering wrote: > Hello, > > Someone asked me about this yesterday, and since I've been carrying > this patch series for over a year -- it's not high priority -- > this seems like a good time finally to post it. > > I've been removing if-before-free tests for a few years now. > Here are some of the projects that have endured this janitorial work: You know, it probably wouldn't be that hard to have GCC perform this analysis for you using its dataflow framework. Checking for a free call which is dominated by a test if the argument is null would be a pretty simple check. Jeff -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iQEcBAEBAgAGBQJNdlqeAAoJEBRtltQi2kC746gIAImSP5BZAQ/d9wF94EQcBsWQ Qo4hn+LK6G2h5R2yVwB9dMf4CTZygxnowlFTxtG9QXMlzMCQp61jHJNLJLpgY4Qz HLMjPGS8uMUa28pJkeRU0ZIMy5hDGID24F0FChnrpNalBnlCvP0xXsnZEcXi8Ei2 3VkOMx87MAnVT0k7omZSnMy2HeKqmnP9xQMGM+ISEAFJuiGYeb5Os3T7IRprJjia DSOjSF20O8TTV6543pUrMvdzrEYtTTmsv1UKejiyGMDpHrs2qNHsyqiFBO/FLGLA bdNplowv5xTTrlPy/4zYXewvl3XLr8okrk8/c0Y4dIKq/g5jIB6pYVXilGSOVa0= =WHAM -END PGP SIGNATURE-
Re: avoid useless if-before-free tests
On 22:47 Mon 07 Mar , Joseph S. Myers wrote: > On Mon, 7 Mar 2011, Dr Andrew John Hughes wrote: > > > > http://gcc.gnu.org/codingconventions.html says Classpath changes should > > > go > > > via Classpath upstream, not directly into GCC. I don't know if that's > > > still accurate. > > > > > > > That's still true. This seems to be the first message I've received in this > > thread, so I'm not even aware of what these changes are. Were the earlier > > messages not sent to this list? > > The original patch went only to gcc-patches. > > http://gcc.gnu.org/ml/gcc-patches/2011-03/msg00252.html > Thanks for the link. I'd like some explanation of why these changes are necessary before we start adding them to Classpath. Are we just assuming that all free implementations will ignore NULL now? With regards to fdlibm, there is a further upstream for this where these changes should be made first. fdlibm is also used by OpenJDK. > -- > Joseph S. Myers > jos...@codesourcery.com -- Andrew :) Free Java Software Engineer Red Hat, Inc. (http://www.redhat.com) Support Free Java! Contribute to GNU Classpath and IcedTea http://www.gnu.org/software/classpath http://icedtea.classpath.org PGP Key: F5862A37 (https://keys.indymedia.org/) Fingerprint = EA30 D855 D50F 90CD F54D 0698 0713 C3ED F586 2A37
Re: [PATCH v2] Re: avoid useless if-before-free tests
Rainer Orth wrote: > Jim Meyering writes: >> I've taken the liberty of letting my editor remove trailing >> blanks in ChangeLog files. I hope that's ok. Also in ChangeLogs, >> I converted some leading 8-space (and 7-space) sequences to single TABs. > > Please move this to a separate patch: this is completely unrelated to It's moved. I'll post v3 soon. > the change at hand. And please don't change the alignment of entries > with multiple email addresses. Changing 8-spaces to a TAB does not affect alignment when you're looking at the ChangeLog file itself with standard tab setting. Perhaps you looked at a hunk like the following and mistook it for one that introduces an alignment change? 2011-11-04 Eric Botcazou -Jakub Jelinek + Jakub Jelinek It does not.
Re: avoid useless if-before-free tests
Dr Andrew John Hughes wrote: > On 22:47 Mon 07 Mar , Joseph S. Myers wrote: >> On Mon, 7 Mar 2011, Dr Andrew John Hughes wrote: >> >> > > http://gcc.gnu.org/codingconventions.html says Classpath changes should >> > > go >> > > via Classpath upstream, not directly into GCC. I don't know if that's >> > > still accurate. >> > > >> > >> > That's still true. This seems to be the first message I've received in >> > this >> > thread, so I'm not even aware of what these changes are. Were the earlier >> > messages not sent to this list? >> >> The original patch went only to gcc-patches. >> >> http://gcc.gnu.org/ml/gcc-patches/2011-03/msg00252.html >> > > Thanks for the link. > > I'd like some explanation of why these changes are necessary before we start > adding them to Classpath. Are we just assuming that all free implementations > will ignore NULL now? IMHO, they're not officially "necessary", but rather nice to have, since they eliminate code that is now obviously obsolete. Those tests have been unnecessary for at least 5 years. The efficiency (of removing the redundant test) is never the issue for me, personally. My main argument for making the change is improved maintainability/readability: - less logic (esp. when the expression is more complicated) - no surprise (for reviewers who stopped using such tests years ago) - more compact, so more lines fit on a page/screen - removing unused code is always worthwhile Sort of along the same lines as removing anachronistic casts of malloc/calloc/realloc return values in C code. No longer needed, but many people continue to use them for no good reason.
C++ PATCH for c++/45651 (ICE with explicit template instantiation and unnamed namespace)
The linkage handling of an explicit instantiation of an undefined
template in instantiate_decl was interacting badly with the linkage
magic for anonymous namespaces. Fixed thus.
Tested x86_64-pc-linux-gnu, applied to trunk.
commit bb206a6c192120614fa6e3c78a2ba2add6f5c3f2
Author: Jason Merrill
Date: Tue Mar 8 10:54:00 2011 -0500
PR c++/45651
* pt.c (instantiate_decl): Don't clear DECL_INTERFACE_KNOWN on
!TREE_PUBLIC decls.
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 076224c..48f9382 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -17224,8 +17224,13 @@ instantiate_decl (tree d, int defer_ok,
if (!pattern_defined && expl_inst_class_mem_p
&& DECL_EXPLICIT_INSTANTIATION (d))
{
- DECL_NOT_REALLY_EXTERN (d) = 0;
- DECL_INTERFACE_KNOWN (d) = 0;
+ /* Leave linkage flags alone on instantiations with anonymous
+visibility. */
+ if (TREE_PUBLIC (d))
+ {
+ DECL_NOT_REALLY_EXTERN (d) = 0;
+ DECL_INTERFACE_KNOWN (d) = 0;
+ }
SET_DECL_IMPLICIT_INSTANTIATION (d);
}
diff --git a/gcc/testsuite/g++.dg/template/anon5.C
b/gcc/testsuite/g++.dg/template/anon5.C
new file mode 100644
index 000..34599c0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/anon5.C
@@ -0,0 +1,6 @@
+// PR c++/45651
+
+namespace { template struct A {}; }
+template struct B { void f(A); };
+template struct B<1>;
+template void B::f(A) {}
C++ PATCH for c++/47705 (ICE with non-pointer argument to pointer template parameter)
We were asserting that any argument to a non-type template parameter of
pointer type must be an address. Which is true of valid code (apart
from null pointer values), but not necessarily of invalid code, where we
should complain rather than crash.
Tested x86_64-pc-linux-gnu, applied to trunk.
commit 0aa8b389e5b3d863edd4e9969cadf2af5f2c1907
Author: Jason Merrill
Date: Tue Mar 8 11:02:49 2011 -0500
PR c++/47705
* pt.c (convert_nontype_argument): Don't crash on non-pointer
argument to pointer parameter.
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 48f9382..cda9df8 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -5369,15 +5369,20 @@ convert_nontype_argument (tree type, tree expr,
tsubst_flags_t complain)
qualification conversion. Let's strip everything. */
else if (TYPE_PTROBV_P (type))
{
- STRIP_NOPS (expr);
- gcc_assert (TREE_CODE (expr) == ADDR_EXPR);
- gcc_assert (TREE_CODE (TREE_TYPE (expr)) == POINTER_TYPE);
- /* Skip the ADDR_EXPR only if it is part of the decay for
-an array. Otherwise, it is part of the original argument
-in the source code. */
- if (TREE_CODE (TREE_TYPE (TREE_OPERAND (expr, 0))) == ARRAY_TYPE)
- expr = TREE_OPERAND (expr, 0);
- expr_type = TREE_TYPE (expr);
+ tree sub = expr;
+ STRIP_NOPS (sub);
+ if (TREE_CODE (sub) == ADDR_EXPR)
+ {
+ gcc_assert (TREE_CODE (TREE_TYPE (sub)) == POINTER_TYPE);
+ /* Skip the ADDR_EXPR only if it is part of the decay for
+an array. Otherwise, it is part of the original argument
+in the source code. */
+ if (TREE_CODE (TREE_TYPE (TREE_OPERAND (sub, 0))) == ARRAY_TYPE)
+ expr = TREE_OPERAND (sub, 0);
+ else
+ expr = sub;
+ expr_type = TREE_TYPE (expr);
+ }
}
}
diff --git a/gcc/testsuite/g++.dg/template/nontype21.C
b/gcc/testsuite/g++.dg/template/nontype21.C
new file mode 100644
index 000..c8e73d2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/nontype21.C
@@ -0,0 +1,10 @@
+// PR c++/47705
+
+template class Something {
+};
+
+extern char const xyz;
+
+class SomethingElse:public Something { // { dg-error "const char *" }
+};
+
C++ PATCH for c++/47488 (ICE on string literal in function template signature)
We don't know how to mangle a STRING_CST yet, but we don't need to
crash. There is a recently added mangling in the ABI document, but it
is inadequate, so I'd like to get that fixed before adding it to G++.
Tested x86_64-pc-linux-gnu, applied to trunk.
commit 2dceb38738c4aedc2e64bf8e8aa4c621b15e61dd
Author: Jason Merrill
Date: Tue Mar 8 12:07:09 2011 -0500
PR c++/47488
* mangle.c (write_template_arg_literal) [STRING_CST]: Sorry.
diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c
index c46ba30..f063d47 100644
--- a/gcc/cp/mangle.c
+++ b/gcc/cp/mangle.c
@@ -2764,6 +2764,10 @@ write_template_arg_literal (const tree value)
write_real_cst (value);
break;
+case STRING_CST:
+ sorry ("string literal in function template signature");
+ break;
+
default:
gcc_unreachable ();
}
Re: [v3] libstdc++/47145
> > For convenience, this patch steps around the STYLESHEET_FLAG > > question by just punting to the net for validation only. > > But it still uses --nonet in that step, right? So it doesn't use the > network at all, the mapping from the canonical stylesheet URL to a > local file is done by the system's XML catalog, /etc/xml/catalog on my > Fedora box. No, for validation only I took out --nonet. Meaning, 'make doc-validate-docbook' searches for a schema on the net and validates against that, not a local thing. It takes longer now to validate, but I don't think it is onerous. At least, that was my intent. Double check my work please. Ideally we could move to RelaxNG anyway, at some point. uBut we will probably have to do something similar about finding the canonical thing, regardless of the specific schema used to validate. > I thought we could do the same for the actual transformation rules > too, using the canonical URL with --nonet and letting the system map > it to a local file, and failing if the file is not present, because > --nonet prevents it fetching a remote stylesheet. I couldn't figure out how to make that work. I tried setting XSL_STYLE_DIR to the docbook 5 URL but no dice. I'm not quite sure what the problem is. > I suspect this is fine for the majority of people now and good enough > for 4.6 Yeah, me too. It seems to be the way we need to go, at the very least. Baby steps. Let's see if this at least unifies the linux users. -benjamin
[Committed] bswap testcase on s390 - use -march=z900
Hi,
the s390 (31 bit) bswap pattern is only available with z900 or higher.
So the testcase fails without specifying -march=z900 (or higher).
Committed to mainline.
Bye,
-Andreas-
2011-03-08 Andreas Krebbel
* gcc.dg/optimize-bswapsi-1.c: Use -march=z900 on s390.
Index: gcc/testsuite/gcc.dg/optimize-bswapsi-1.c
===
*** gcc/testsuite/gcc.dg/optimize-bswapsi-1.c.orig
--- gcc/testsuite/gcc.dg/optimize-bswapsi-1.c
***
*** 1,6
--- 1,7
/* { dg-do compile { target arm*-*-* alpha*-*-* i?86-*-* powerpc*-*-*
rs6000-*-* x86_64-*-* s390*-*-* } } */
/* { dg-require-effective-target stdint_types } */
/* { dg-options "-O2 -fdump-tree-bswap" } */
+ /* { dg-options "-O2 -fdump-tree-bswap -march=z900" { target s390-*-* } } */
#include
[PATCH v3] Re: avoid useless if-before-free tests
Jim Meyering wrote:
> Joseph S. Myers wrote:
> ...
>> We definitely don't want to make local changes to zlib for this sort of
>> issue, though importing a new upstream version of zlib (making sure the
>> local configure code still works) should be fine for 4.7.
>
> Thanks again for the feedback.
> I've omitted changes to the intl/, zlib/ and classpath/ directories
> reverted the problem you spotted, and added ChangeLog entries.
...
Relative to v2, I've added libgo/ to the list of exempt directories and added
this recently discussed gfc_free patch, at the request of Tobias Burnus.
Also, I corrected an error in fortran's ChangeLog and removed all
whitespace changes from all ChangeLog files.
diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog
index 2f694ff..f7ffa9f 100644
--- a/gcc/fortran/ChangeLog
+++ b/gcc/fortran/ChangeLog
@@ -2,6 +2,7 @@
* gfortranspec.c (lang_specific_pre_link): Remove useless
if-before-free.
+ * misc.c (gfc_free): Likewise.
2011-03-06 Paul Thomas
Jerry DeLisle
diff --git a/gcc/fortran/misc.c b/gcc/fortran/misc.c
index 4dd186f..8a343a0 100644
--- a/gcc/fortran/misc.c
+++ b/gcc/fortran/misc.c
@@ -47,8 +47,7 @@ gfc_free (void *p)
{
/* The parentheses around free are needed in order to call not
the redefined free of gfortran.h. */
- if (p != NULL)
-(free) (p);
+ (free) (p);
}
>From 0d18b70a8821ab2fc58b5ed592ed611e05a29c7f Mon Sep 17 00:00:00 2001
From: Jim Meyering
Date: Mon, 3 Jan 2011 16:52:37 +0100
Subject: [PATCH 1/2] discourage unnecessary use of if before free
* README.Portability: Explain why "if (P) free (P)" is best avoided.
---
gcc/README.Portability | 23 ---
1 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/gcc/README.Portability b/gcc/README.Portability
index 32a33e2..e099a3f 100644
--- a/gcc/README.Portability
+++ b/gcc/README.Portability
@@ -51,14 +51,24 @@ foo (bar, )
needs to be coded in some other way.
-free and realloc
-
+Avoid unnecessary test before free
+--
-Some implementations crash upon attempts to free or realloc the null
-pointer. Thus if mem might be null, you need to write
+Since SunOS 4 stopped being a reasonable portability target,
+(which happened around 2007) there has been no need to guard
+against "free (NULL)". Thus, any guard like the following
+constitutes a redundant test:
- if (mem)
-free (mem);
+ if (P)
+free (P);
+
+It is better to avoid the test.[*]
+Instead, simply free P, regardless of whether it is NULL.
+
+[*] However, if your profiling exposes a test like this in a
+performance-critical loop, say where P is nearly always NULL, and
+the cost of calling free on a NULL pointer would be prohibitively
+high, please let us know.
Trigraphs
@@ -194,4 +204,3 @@ o Passing incorrect types to fprintf and friends.
o Adding a function declaration for a module declared in another file to
a .c file instead of to a .h file.
-
--
1.7.4.1.299.ga459d
>From 6b02a3fdfc7169bd49a52465e990700844f68b22 Mon Sep 17 00:00:00 2001
From: Jim Meyering
Date: Tue, 8 Mar 2011 12:19:24 +0100
Subject: [PATCH 2/2] remove useless if-before-free tests
Change "if (E) free (E);" to "free (E);" everywhere except in the
libgo/, intl/, zlib/ and classpath/ directories.
Also transform equivalent variants like
"if (E != NULL) free (E);" and allow an extra cast on the
argument to free. Otherwise, the tested and freed "E"
expressions must be identical, modulo white space.
---
gcc/ChangeLog | 39 +
gcc/ada/ChangeLog |4 ++
gcc/ada/initialize.c|3 +-
gcc/c-family/ChangeLog |5 ++
gcc/c-family/c-format.c |6 +--
gcc/calls.c | 15 ++
gcc/cfgcleanup.c|3 +-
gcc/collect2.c |3 +-
gcc/config/i386/i386.c |3 +-
gcc/config/mcore/mcore.c|3 +-
gcc/coverage.c |3 +-
gcc/cp/ChangeLog|4 ++
gcc/cp/tree.c |3 +-
gcc/cse.c |6 +--
gcc/cselib.c|3 +-
gcc/df-core.c | 15 ++
gcc/fortran/ChangeLog |6 +++
gcc/fortran/gfortranspec.c |5 +-
gcc/fortran/misc.c |3 +-
gcc/function.c |3 +-
gcc/gcc.c | 15 ++
gcc/gcov.c |6 +--
gcc/gensupport.c| 12 ++
gcc/graphite-clast-to-gimple.c |3 +-
gcc/graphite-sese-to-poly.c |3 +-
gcc/haifa-sched.c |3 +-
gcc/ipa-prop.c |3 +-
gcc/ipa-pure-const.c|3 +-
gcc/ipa-reference.c |3 +-
gcc/ira-costs.c | 15 ++
gcc/ira
fix for pr47837
Please review the attached patch, it does some simplification of the
complicated logical or expressions (x1 or x2 or x3 ...) constructed
from control flow analysis into simpler form.
Bootstraps and works on s390x for both testcases.
Bootstraps on x86-64. Regression testing is on going (it takes forever
(whole night already) to finish possibly because the lto test in
c-torture ..).
Ok for trunk?
David
2011-03-08 Xinliang David Li
PR c/47837
* tree-ssa-uninit.c (pred_chain_length_cmp): New function.
(normalize_preds): New function.
(is_use_properly_guarded): Normalize def predicates.
Index: tree-ssa-uninit.c
===
--- tree-ssa-uninit.c (revision 170150)
+++ tree-ssa-uninit.c (working copy)
@@ -1605,6 +1605,153 @@ is_superset_of (VEC(use_pred_info_t, hea
return true;
}
+/* Comparison function used by qsort. It is used to
+ sort predicate chains to allow predicate
+ simplication. */
+
+static int
+pred_chain_length_cmp (const void *p1, const void *p2)
+{
+ use_pred_info_t i1, i2;
+ VEC(use_pred_info_t, heap) * const*chain1
+ = (VEC(use_pred_info_t, heap) * const*)p1;
+ VEC(use_pred_info_t, heap) * const*chain2
+ = (VEC(use_pred_info_t, heap) * const*)p2;
+
+ if (VEC_length (use_pred_info_t, *chain1)
+ != VEC_length (use_pred_info_t, *chain2))
+return (VEC_length (use_pred_info_t, *chain1)
+- VEC_length (use_pred_info_t, *chain2));
+
+ i1 = VEC_index (use_pred_info_t, *chain1, 0);
+ i2 = VEC_index (use_pred_info_t, *chain2, 0);
+
+ /* Allow predicates with similar prefix come together. */
+ if (!i1->invert && i2->invert)
+return -1;
+ else if (i1->invert && !i2->invert)
+return 1;
+
+ return gimple_uid (i1->cond) - gimple_uid (i2->cond);
+}
+
+/* x OR (!x AND y) is equivalent to x OR y.
+ This function normalizes x1 OR (!x1 AND x2) OR (!x1 AND !x2 AND x3)
+ into x1 OR x2 OR x3. PREDS is the predicate chains, and N is
+ the number of chains. Returns true if normalization happens. */
+
+static bool
+normalize_preds (VEC(use_pred_info_t, heap) **preds, size_t *n)
+{
+ size_t i, j, ll;
+ VEC(use_pred_info_t, heap) *pred_chain;
+ VEC(use_pred_info_t, heap) *x = 0;
+ use_pred_info_t xj = 0, nxj = 0;
+
+ if (*n < 2)
+return false;
+
+ /* First sort the chains in ascending order of lengths. */
+ qsort (preds, *n, sizeof (void *), pred_chain_length_cmp);
+ pred_chain = preds[0];
+ ll = VEC_length (use_pred_info_t, pred_chain);
+ if (ll != 1)
+ {
+ if (ll == 2)
+ {
+ use_pred_info_t xx, yy, xx2, nyy;
+ VEC(use_pred_info_t, heap) *pred_chain2 = preds[1];
+ if (VEC_length (use_pred_info_t, pred_chain2) != 2)
+ return false;
+
+ /* See if simplication x AND y OR x AND !y is possible. */
+ xx = VEC_index (use_pred_info_t, pred_chain, 0);
+ yy = VEC_index (use_pred_info_t, pred_chain, 1);
+ xx2 = VEC_index (use_pred_info_t, pred_chain2, 0);
+ nyy = VEC_index (use_pred_info_t, pred_chain2, 1);
+ if (gimple_cond_lhs (xx->cond) != gimple_cond_lhs (xx2->cond)
+ || gimple_cond_rhs (xx->cond) != gimple_cond_rhs (xx2->cond)
+ || gimple_cond_code (xx->cond) != gimple_cond_code (xx2->cond)
+ || (xx->invert != xx2->invert))
+ return false;
+ if (gimple_cond_lhs (yy->cond) != gimple_cond_lhs (nyy->cond)
+ || gimple_cond_rhs (yy->cond) != gimple_cond_rhs (nyy->cond)
+ || gimple_cond_code (yy->cond) != gimple_cond_code (nyy->cond)
+ || (yy->invert == nyy->invert))
+ return false;
+
+ /* Now merge the first two chains. */
+ free (yy);
+ free (nyy);
+ free (xx2);
+ VEC_free (use_pred_info_t, heap, pred_chain);
+ VEC_free (use_pred_info_t, heap, pred_chain2);
+ pred_chain = 0;
+ VEC_safe_push (use_pred_info_t, heap, pred_chain, xx);
+ preds[0] = pred_chain;
+ for (i = 1; i < *n - 1; i++)
+ preds[i] = preds[i+1];
+
+ preds[*n - 1] = 0;
+ *n = *n - 1;
+ }
+ }
+
+ VEC_safe_push (use_pred_info_t, heap, x,
+ VEC_index (use_pred_info_t, pred_chain, 0));
+
+ for (i = 1; i < *n; i++)
+{
+ pred_chain = preds[i];
+ if (VEC_length (use_pred_info_t, pred_chain) != i + 1)
+return false;
+
+ for (j = 0; j < i; j++)
+{
+ xj = VEC_index (use_pred_info_t, x, j);
+ nxj = VEC_index (use_pred_info_t, pred_chain, j);
+
+ /* Check if nxj is !xj */
+ if (gimple_cond_lhs (xj->cond) != gimple_cond_lhs (nxj->cond)
+ || gimple_cond_rhs (xj->cond) != gimple_cond_rhs (nxj->cond)
+ || gimple_cond_code (xj->cond) != gimple_cond_code (nxj->cond)
+ || (xj->invert == nxj->invert))
+return false;
+}
+
+ VEC_safe_push (use_pred_info_t, he
Re: fix for pr47837
On Tue, Mar 8, 2011 at 9:54 AM, Xinliang David Li wrote: > Please review the attached patch, it does some simplification of the > complicated logical or expressions (x1 or x2 or x3 ...) constructed > from control flow analysis into simpler form. > > Bootstraps and works on s390x for both testcases. This is done by Andreas Krebble. David > > Bootstraps on x86-64. Regression testing is on going (it takes forever > (whole night already) to finish possibly because the lto test in > c-torture ..). > > Ok for trunk? > > David > > 2011-03-08 Xinliang David Li > > PR c/47837 > * tree-ssa-uninit.c (pred_chain_length_cmp): New function. > (normalize_preds): New function. > (is_use_properly_guarded): Normalize def predicates. >
[PATCH] Decrease number of threads used by goroutines.go test (take 2)
On Tue, Mar 08, 2011 at 08:04:23AM -0800, Ian Lance Taylor wrote:
> I don't really care what environment variable we use, I just want some
> way to run the full test, without having DejaGNU silently change the
> test on me. It's perfectly reasonable to have the default check ulimit,
> I just want some way to not check it.
Ok, here is an updated patch which uses both proposed env vars:
GCCGO_RUN_ALL_TESTS=1 makes it fail for me as before (i.e. 1 threads)
GCC_TEST_RUN_EXPENSIVE=1 makes it run with max($[`ulimit -u`/4], 1)
threads on Linux native, 1 everywhere else
by default it is run just with 64 threads
2011-03-08 Jakub Jelinek
* go.test/go-test.exp: For goroutines.go test if GCCGO_RUN_ALL_TESTS
is not set in the environment, pass 64 as first argument when not
running expensive tests or pass max($[`ulimit -u`/4], 1) on
Linux native.
--- gcc/testsuite/go.test/go-test.exp.jj2011-01-15 11:26:32.0
+0100
+++ gcc/testsuite/go.test/go-test.exp 2011-03-08 13:23:36.078402148 +0100
@@ -265,6 +265,27 @@ proc go-gc-tests { } {
verbose -log "$test: go_execute_args is $go_execute_args"
set index [string last " $progargs" $test_line]
set test_line [string replace $test_line $index end]
+ } elseif { [string match "*go.test/test/chan/goroutines.go" $test] \
+ && [getenv GCCGO_RUN_ALL_TESTS] == "" } {
+ # goroutines.go spawns by default 1 threads, which is too much
+ # for many OSes.
+ if { [getenv GCC_TEST_RUN_EXPENSIVE] == "" } {
+ set go_execute_args 64
+ } elseif { [ishost "*-linux*" ] && ![is_remote host] && ![is_remote
target] } {
+ # On Linux when using low ulimit -u limit, use maximum of
+ # a quarter of that limit and 1 even when running expensive
+ # tests, otherwise parallel tests might fail after fork
failures.
+ set nproc [lindex [remote_exec host {sh -c ulimit\ -u}] 1]
+ if { [string is integer -strict $nproc] } {
+ set nproc [expr $nproc / 4]
+ if { $nproc > 1 } { set nproc 1 }
+ if { $nproc < 16 } { set nproc 16 }
+ set go_execute_args $nproc
+ }
+ }
+ if { "$go_execute_args" != "" } {
+ verbose -log "$test: go_execute_args is $go_execute_args"
+ }
}
if { $test_line == "// \$G \$D/\$F\.go && \$L \$F\.\$A && \./\$A\.out
>tmp.go &&" \
Jakub
Re: fix for pr47837
On Tue, Mar 8, 2011 at 12:54, Xinliang David Li wrote: > Bootstraps on x86-64. Regression testing is on going (it takes forever > (whole night already) to finish possibly because the lto test in > c-torture ..). I don't think so. We have been having weird kernel problems in our machines when running dejagnu. Try using the kernel I described in a separate message. Diego.
Re: [Patch, AVR] Housekeeping: Hookize REGISTER_MOVE_COST, MEMORY_MOVE_COST
2011/3/8 Georg-Johann Lay : > This patch moves deprecated REGISTER_MOVE_COST resp. MEMORY_MOVE_COST > from avr.h to target hook avr_register_move_cost resp. > avr_memory_move_cost in avr.c. > > No functionality added or removed; costs are unchanged. > > > 2011-03-08 Georg-Johann Lay > > * config/avr/avr.h (REGISTER_MOVE_COST, MEMORY_MOVE_COST): Remove. > * config/avr/avr.c (TARGET_REGISTER_MOVE_COST, > TARGET_MEMORY_MOVE_COST): Define. > (avr_register_move_cost, avr_memory_move_cost): New Functions. Applied. Denis.
Re: [patch libiberty include]: Add additional helper functions for directory-separator searching
> Date: Tue, 8 Mar 2011 16:29:51 +0100 > From: Kai Tietz > > Umm, sorry. I found a wrong copy & paste. So I re-sent the corrected > patch. Additionally I adjuste the changes in Makefile.in so, that > alphabetic order remains. I think we don't need filename_dirchr, only filename_dirrchr. And you didn't change anything in the documentation to address my comments earlier today.
Re: [patch libiberty include]: Add additional helper functions for directory-separator searching
> From: Pedro Alves > Date: Tue, 8 Mar 2011 13:33:02 + > Cc: Kai Tietz , > gcc-patches@gcc.gnu.org, > Eli Zaretskii , > binut...@sourceware.org > > On Tuesday 08 March 2011 12:48:11, Kai Tietz wrote: > > > Well, a better example is elfstab_offset_sections() in elfread.c. > > /* The ELF symbol info doesn't include path names, so strip the path > (if any) from the psymtab filename. */ > while (0 != (p = strchr (filename, '/'))) > filename = p + 1; > > Looks like its looking for the last path separator, so > it might as well use filename_dirrchr instead. Exactly. > > Another is in find_file_and_directory() in dwarf2read.c file. > > Workaround for Irix. Certainly that '/' should not depend > on the host gdb is running on. It actually should use IS_ABSOLUTE_FILE_NAME, if any portability enhancement is needed here. In my experience, the strchr analog is not needed, only the strrchr one (which could be used quite a lot). The few places that use strchr now should actually be rewritten to search from the end, because that's what they need.
Re: [PATCH] Decrease number of threads used by goroutines.go test (take 2)
Jakub Jelinek writes: > Ok, here is an updated patch which uses both proposed env vars: > > GCCGO_RUN_ALL_TESTS=1 makes it fail for me as before (i.e. 1 threads) > > GCC_TEST_RUN_EXPENSIVE=1 makes it run with max($[`ulimit -u`/4], 1) > threads on Linux native, 1 everywhere else Why should this be Linux-specific? I think the same logic applies everywhere. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [PATCH] Decrease number of threads used by goroutines.go test (take 2)
On Tue, Mar 08, 2011 at 07:40:38PM +0100, Rainer Orth wrote: > Jakub Jelinek writes: > > > Ok, here is an updated patch which uses both proposed env vars: > > > > GCCGO_RUN_ALL_TESTS=1 makes it fail for me as before (i.e. 1 threads) > > > > GCC_TEST_RUN_EXPENSIVE=1 makes it run with max($[`ulimit -u`/4], 1) > > threads on Linux native, 1 everywhere else > > Why should this be Linux-specific? I think the same logic applies > everywhere. Because ulimit -u is Linux specific? At least, google doesn't show any hints about any other OSes having such limit, neither RLIMIT_NPROC nor ulimit -u. Jakub
Re: [PATCH v2] Re: avoid useless if-before-free tests
Jim Meyering writes: >> the change at hand. And please don't change the alignment of entries >> with multiple email addresses. > > Changing 8-spaces to a TAB does not affect alignment when you're > looking at the ChangeLog file itself with standard tab setting. > > Perhaps you looked at a hunk like the following and mistook it > for one that introduces an alignment change? > > 2011-11-04 Eric Botcazou > -Jakub Jelinek > +Jakub Jelinek > > It does not. I'm pretty sure it does: before, you have 12 SPC, afterwards you have TAB + 3 SPC, which is equivalent to 11 SPC in my book. I honestly don't see the point of this whitespace change unless done across all ChangeLogs, not just a few that you happen to touch. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [patch libiberty include]: Add additional helper functions for directory-separator searching
On Tuesday 08 March 2011 18:37:49, Eli Zaretskii wrote:
> > > Another is in find_file_and_directory() in dwarf2read.c file.
> >
> > Workaround for Irix. Certainly that '/' should not depend
> > on the host gdb is running on.
>
> It actually should use IS_ABSOLUTE_FILE_NAME, if any portability
> enhancement is needed here.
The point of the code, according to its comment,
is to workaround an issue with the debug info output by the
native Irix compiler. You wouldn't want a cross-Irix,
Windows-hosted gdb looking for '\' or a drive prefix in order
to decide whether to apply the workaround. In other words,
we _always_ want to check for literal '/' here:
if (*comp_dir != NULL)
{
/* Irix 6.2 native cc prepends .: to the compilation
directory, get rid of it. */
char *cp = strchr (*comp_dir, ':');
if (cp && cp != *comp_dir && cp[-1] == '.' && cp[1] == '/')
*comp_dir = cp + 1;
}
--
Pedro Alves
Re: [patch libiberty include]: Add additional helper functions for directory-separator searching
2011/3/8 Eli Zaretskii : >> From: Pedro Alves >> Date: Tue, 8 Mar 2011 13:33:02 + >> Cc: Kai Tietz , >> gcc-patches@gcc.gnu.org, >> Eli Zaretskii , >> binut...@sourceware.org >> >> On Tuesday 08 March 2011 12:48:11, Kai Tietz wrote: >> >> > Well, a better example is elfstab_offset_sections() in elfread.c. >> >> /* The ELF symbol info doesn't include path names, so strip the path >> (if any) from the psymtab filename. */ >> while (0 != (p = strchr (filename, '/'))) >> filename = p + 1; >> >> Looks like its looking for the last path separator, so >> it might as well use filename_dirrchr instead. > > Exactly. > >> > Another is in find_file_and_directory() in dwarf2read.c file. >> >> Workaround for Irix. Certainly that '/' should not depend >> on the host gdb is running on. > > It actually should use IS_ABSOLUTE_FILE_NAME, if any portability > enhancement is needed here. > > In my experience, the strchr analog is not needed, only the strrchr > one (which could be used quite a lot). The few places that use strchr > now should actually be rewritten to search from the end, because > that's what they need. > Here I am not that sure. For example in gcc's gengtype.c (read_input_list) is a use-case for strchr on filenames, which can't be expressed by strrchr. Please be aware that libiberty is shared between different ventures. I admit that filenames/paths are searched normal from right to left. But there might be cases a left to right search is suitable. Regards, Kai
Re: fix for pr47837
On 03/08/2011 12:54 PM, Xinliang David Li wrote:
Please review the attached patch, it does some simplification of the
complicated logical or expressions (x1 or x2 or x3 ...) constructed
from control flow analysis into simpler form.
Bootstraps and works on s390x for both testcases.
Bootstraps on x86-64. Regression testing is on going (it takes forever
(whole night already) to finish possibly because the lto test in
c-torture ..).
Ok for trunk?
As a general comment, do you think we will start adding more and more of
these special pattern matchers into uninit analysis? I'm wondering how
much effort should we make into creating something more generic.
Right now it's this pattern, but there may be others. It could grow
pretty big and ugly.
2011-03-08 Xinliang David Li
PR c/47837
* tree-ssa-uninit.c (pred_chain_length_cmp): New function.
(normalize_preds): New function.
(is_use_properly_guarded): Normalize def predicates.
Could you add some tests? I realize this fixes 390 testcases, but are
there tests where we could explicitly check that this is triggering?
Index: tree-ssa-uninit.c
===
--- tree-ssa-uninit.c (revision 170150)
+++ tree-ssa-uninit.c (working copy)
@@ -1605,6 +1605,153 @@ is_superset_of (VEC(use_pred_info_t, hea
return true;
}
+/* Comparison function used by qsort. It is used to
+ sort predicate chains to allow predicate
+ simplication. */
s/simplication/simplification/
There is another instance of this later.
+
+static int
+pred_chain_length_cmp (const void *p1, const void *p2)
+{
+ use_pred_info_t i1, i2;
+ VEC(use_pred_info_t, heap) * const*chain1
+ = (VEC(use_pred_info_t, heap) * const*)p1;
+ VEC(use_pred_info_t, heap) * const*chain2
+ = (VEC(use_pred_info_t, heap) * const*)p2;
space around '*'.
+
+ if (VEC_length (use_pred_info_t, *chain1)
+ != VEC_length (use_pred_info_t, *chain2))
+return (VEC_length (use_pred_info_t, *chain1)
+- VEC_length (use_pred_info_t, *chain2));
+
+ i1 = VEC_index (use_pred_info_t, *chain1, 0);
+ i2 = VEC_index (use_pred_info_t, *chain2, 0);
+
+ /* Allow predicates with similar prefix come together. */
+ if (!i1->invert && i2->invert)
+return -1;
+ else if (i1->invert && !i2->invert)
+return 1;
+
+ return gimple_uid (i1->cond) - gimple_uid (i2->cond);
The UIDs are not necessarily stable from call to call. Would this be a
problem? It doesn't seem that it would.
+}
+
+/* x OR (!x AND y) is equivalent to x OR y.
+ This function normalizes x1 OR (!x1 AND x2) OR (!x1 AND !x2 AND x3)
+ into x1 OR x2 OR x3. PREDS is the predicate chains, and N is
+ the number of chains. Returns true if normalization happens. */
+
+static bool
+normalize_preds (VEC(use_pred_info_t, heap) **preds, size_t *n)
+{
+ size_t i, j, ll;
+ VEC(use_pred_info_t, heap) *pred_chain;
+ VEC(use_pred_info_t, heap) *x = 0;
+ use_pred_info_t xj = 0, nxj = 0;
+
+ if (*n < 2)
+return false;
+
+ /* First sort the chains in ascending order of lengths. */
+ qsort (preds, *n, sizeof (void *), pred_chain_length_cmp);
+ pred_chain = preds[0];
+ ll = VEC_length (use_pred_info_t, pred_chain);
+ if (ll != 1)
+ {
+ if (ll == 2)
+ {
Why not just one 'if (ll == 2)'?
+ use_pred_info_t xx, yy, xx2, nyy;
+ VEC(use_pred_info_t, heap) *pred_chain2 = preds[1];
+ if (VEC_length (use_pred_info_t, pred_chain2) != 2)
+ return false;
+
+ /* See if simplication x AND y OR x AND !y is possible. */
+ xx = VEC_index (use_pred_info_t, pred_chain, 0);
+ yy = VEC_index (use_pred_info_t, pred_chain, 1);
+ xx2 = VEC_index (use_pred_info_t, pred_chain2, 0);
+ nyy = VEC_index (use_pred_info_t, pred_chain2, 1);
+ if (gimple_cond_lhs (xx->cond) != gimple_cond_lhs (xx2->cond)
+ || gimple_cond_rhs (xx->cond) != gimple_cond_rhs (xx2->cond)
+ || gimple_cond_code (xx->cond) != gimple_cond_code (xx2->cond)
+ || (xx->invert != xx2->invert))
+ return false;
+ if (gimple_cond_lhs (yy->cond) != gimple_cond_lhs (nyy->cond)
+ || gimple_cond_rhs (yy->cond) != gimple_cond_rhs (nyy->cond)
+ || gimple_cond_code (yy->cond) != gimple_cond_code (nyy->cond)
+ || (yy->invert == nyy->invert))
+ return false;
So, this has been modifying the input array, what happens if it at some
point during the iteration, it decides to return false? The caller will
need to cope with the modified version of 'preds'?
+
+ /* Now merge the first two chains. */
+ free (yy);
+ free (nyy);
+ free (xx2);
+ VEC_free (use_pred_info_t, heap, pred_chain);
+ VEC_free (use_pred_info_t, heap, pred_chain2);
+ pred_chain = 0;
+ VEC_safe_push (use_pred_info_t, heap, pred_chain, xx);
+ preds[0] = pred_chain;
Re: [PATCH] Decrease number of threads used by goroutines.go test (take 2)
Jakub Jelinek writes: >> Why should this be Linux-specific? I think the same logic applies >> everywhere. > > Because ulimit -u is Linux specific? At least, google doesn't show any > hints about any other OSes having such limit, neither RLIMIT_NPROC nor > ulimit -u. At best, it's shell-specific: Solaris 11 /bin/sh (which is ksh93) does have it, although admittedly previous Solaris/IRIX/Tru64 UNIX shells don't. On the other hand, bash has it on all of those systems. Why not simply test if ulimit -u doesn't error and then use it? I'd very much prefer this to a solution that is unnecessarily OS-specific. Thanks. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [PATCH] Decrease number of threads used by goroutines.go test (take 2)
On Tue, Mar 08, 2011 at 07:56:38PM +0100, Rainer Orth wrote: > Jakub Jelinek writes: > At best, it's shell-specific: Solaris 11 /bin/sh (which is ksh93) does > have it, although admittedly previous Solaris/IRIX/Tru64 UNIX shells > don't. On the other hand, bash has it on all of those systems. > > Why not simply test if ulimit -u doesn't error and then use it? I'd > very much prefer this to a solution that is unnecessarily OS-specific. I'm happy to drop the [ ishost "*-linux*" ] && if you are going to look for failures on weirdo OSes. I have no idea what ulimit -u does on anything but Linux, while the tcl code only uses its value if it printed a number, whether it is something similar to limit on number of each user's threads or something completely else is unclear. Jakub
Re: [PATCH] Decrease number of threads used by goroutines.go test (take 2)
Jakub Jelinek writes: > I'm happy to drop the [ ishost "*-linux*" ] && if you are going to look for > failures on weirdo OSes. I have no idea what ulimit -u does on anything but > Linux, while the tcl code only uses its value if it printed a number, > whether it is something similar to limit on number of each user's threads > or something completely else is unclear. In both bash and every non-bash shell I have that implements it at all, ulimit -u does exactly the same as on Linux. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [PATCH v2] Re: avoid useless if-before-free tests
Rainer Orth wrote: > Jim Meyering writes: >>> the change at hand. And please don't change the alignment of entries >>> with multiple email addresses. >> >> Changing 8-spaces to a TAB does not affect alignment when you're >> looking at the ChangeLog file itself with standard tab setting. >> >> Perhaps you looked at a hunk like the following and mistook it >> for one that introduces an alignment change? >> >> 2011-11-04 Eric Botcazou >> -Jakub Jelinek >> + Jakub Jelinek >> >> It does not. > > I'm pretty sure it does: before, you have 12 SPC, afterwards you have > TAB + 3 SPC, which is equivalent to 11 SPC in my book. Here's the precise excerpt from my patch: 2011-11-04 Eric Botcazou -Jakub Jelinek + Jakub Jelinek That has TAB + 4, so induced no alignment change. > I honestly don't see the point of this whitespace change unless done > across all ChangeLogs, not just a few that you happen to touch. As I said, it's gone, now, from my patch. If no one objects, I'll normalize all ChangeLog files.
[PATCH] Decrease number of threads used by goroutines.go test (take 3)
On Tue, Mar 08, 2011 at 08:10:31PM +0100, Rainer Orth wrote:
> Jakub Jelinek writes:
>
> > I'm happy to drop the [ ishost "*-linux*" ] && if you are going to look for
> > failures on weirdo OSes. I have no idea what ulimit -u does on anything but
> > Linux, while the tcl code only uses its value if it printed a number,
> > whether it is something similar to limit on number of each user's threads
> > or something completely else is unclear.
>
> In both bash and every non-bash shell I have that implements it at all,
> ulimit -u does exactly the same as on Linux.
Ok then:
2011-03-08 Jakub Jelinek
* go.test/go-test.exp: For goroutines.go test if GCCGO_RUN_ALL_TESTS
is not set in the environment, pass 64 as first argument when not
running expensive tests or pass max($[`ulimit -u`/4], 1) on
native where ulimit -u is supported.
--- gcc/testsuite/go.test/go-test.exp.jj2011-01-15 11:26:32.0
+0100
+++ gcc/testsuite/go.test/go-test.exp 2011-03-08 13:23:36.078402148 +0100
@@ -265,6 +265,27 @@ proc go-gc-tests { } {
verbose -log "$test: go_execute_args is $go_execute_args"
set index [string last " $progargs" $test_line]
set test_line [string replace $test_line $index end]
+ } elseif { [string match "*go.test/test/chan/goroutines.go" $test] \
+ && [getenv GCCGO_RUN_ALL_TESTS] == "" } {
+ # goroutines.go spawns by default 1 threads, which is too much
+ # for many OSes.
+ if { [getenv GCC_TEST_RUN_EXPENSIVE] == "" } {
+ set go_execute_args 64
+ } elseif { ![is_remote host] && ![is_remote target] } {
+ # When using low ulimit -u limit, use maximum of
+ # a quarter of that limit and 1 even when running expensive
+ # tests, otherwise parallel tests might fail after fork
failures.
+ set nproc [lindex [remote_exec host {sh -c ulimit\ -u}] 1]
+ if { [string is integer -strict $nproc] } {
+ set nproc [expr $nproc / 4]
+ if { $nproc > 1 } { set nproc 1 }
+ if { $nproc < 16 } { set nproc 16 }
+ set go_execute_args $nproc
+ }
+ }
+ if { "$go_execute_args" != "" } {
+ verbose -log "$test: go_execute_args is $go_execute_args"
+ }
}
if { $test_line == "// \$G \$D/\$F\.go && \$L \$F\.\$A && \./\$A\.out
>tmp.go &&" \
Jakub
Re: [PATCH v2] Re: avoid useless if-before-free tests
Jim Meyering writes: >> I honestly don't see the point of this whitespace change unless done >> across all ChangeLogs, not just a few that you happen to touch. > > As I said, it's gone, now, from my patch. > If no one objects, I'll normalize all ChangeLog files. No objection per se, although it's for the RMs to decide about the timing. If you do, it would be good to fix other issues flagged by Emacs's Change Log mode, like trailing whitespace. Thanks. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [PATCH] avoid memory overrun in a test leading to potential double-free
> avoid memory overrun in a test leading to potential double-free > * testsuite/test-expandargv.c (writeout_test): Fix off-by-one error: > i.e., do copy the trailing NUL byte. Ok. Thanks!
Re: [PATCH v3] Re: avoid useless if-before-free tests
I'm OK with the libiberty parts.
Re: [PATCH v2] Re: avoid useless if-before-free tests
On Tue, Mar 08, 2011 at 08:20:22PM +0100, Rainer Orth wrote: > Jim Meyering writes: > > >> I honestly don't see the point of this whitespace change unless done > >> across all ChangeLogs, not just a few that you happen to touch. > > > > As I said, it's gone, now, from my patch. > > If no one objects, I'll normalize all ChangeLog files. > > No objection per se, although it's for the RMs to decide about the > timing. The good timing for the if (x) free (x); patch is right after stage 1 reopens, which will be hopefully RSN. Jakub
Re: [patch libiberty include]: Add additional helper functions for directory-separator searching
> I think we don't need filename_dirchr, only filename_dirrchr. I see no harm in having both, for completeness, though. One could argue they should be in separate files, but they're trivial functions on non-dos-fs systems. What bothers me about this patch is that we're adding yet another set of functions that don't discriminate between the host filesystem, the target filesystem, and the build filesystem.
Re: RFA/RFC: Enable both gold and ld in a single toolchain
On Tue, Aug 17, 2010 at 04:58, Nick Clifton wrote: > Hi Raymes, > >> What is the status of this patch? I see the binutils part applied but >> not the gcc part. >> >> http://gcc.gnu.org/ml/gcc-patches/2010-04/msg00402.html > > Mark Mitchell recently posted a review of the patch and it is currently in > my queue of things to look at. Unfortunately I have a few other, higher > priority tasks on my plate at the moment. But I will get back to the patch > as soon as I can. > > Cheers > Nick > > Hey Nick, Any news on this patch? :) We may be interested in using it. Thanks. Diego.
Re: [PATCH] Decrease number of threads used by goroutines.go test (take 2)
Jakub Jelinek writes: > 2011-03-08 Jakub Jelinek > > * go.test/go-test.exp: For goroutines.go test if GCCGO_RUN_ALL_TESTS > is not set in the environment, pass 64 as first argument when not > running expensive tests or pass max($[`ulimit -u`/4], 1) on > Linux native. This is OK, and it's also OK if you remove the ishost conditional as Rainer suggests. Thanks. Ian
Re: [patch libiberty include]: Add additional helper functions for directory-separator searching
> Date: Tue, 8 Mar 2011 19:51:14 +0100 > From: Kai Tietz > Cc: Pedro Alves , gdb-patc...@sourceware.org, > gcc-patches@gcc.gnu.org, binut...@sourceware.org > > > In my experience, the strchr analog is not needed, only the strrchr > > one (which could be used quite a lot). The few places that use strchr > > now should actually be rewritten to search from the end, because > > that's what they need. > > > > Here I am not that sure. For example in gcc's gengtype.c > (read_input_list) is a use-case for strchr on filenames, which can't > be expressed by strrchr. I don't see any reason to have in libiberty a function that has a single use.
Re: [patch libiberty include]: Add additional helper functions for directory-separator searching
> Date: Tue, 8 Mar 2011 14:41:00 -0500 > From: DJ Delorie > CC: ktiet...@googlemail.com, binut...@sourceware.org, > gdb-patc...@sourceware.org, gcc-patches@gcc.gnu.org > > > I see no harm in having both, for completeness, though. I don't see any need for completeness in this case. But it's your call. I still think that the documentation should be fixed, though.
Re: fix for pr47837
On Tue, Mar 8, 2011 at 10:55 AM, Diego Novillo wrote:
> On 03/08/2011 12:54 PM, Xinliang David Li wrote:
>>
>> Please review the attached patch, it does some simplification of the
>> complicated logical or expressions (x1 or x2 or x3 ...) constructed
>> from control flow analysis into simpler form.
>>
>> Bootstraps and works on s390x for both testcases.
>>
>> Bootstraps on x86-64. Regression testing is on going (it takes forever
>> (whole night already) to finish possibly because the lto test in
>> c-torture ..).
>>
>> Ok for trunk?
>
> As a general comment, do you think we will start adding more and more of
> these special pattern matchers into uninit analysis? I'm wondering how much
> effort should we make into creating something more generic.
Good question. The answer is probably only for common expressions like
this one. We don't have generic way of doing expression reassociation
that can be invoked on the fly -- only in the future.
> Right now it's this pattern, but there may be others. It could grow pretty
> big and ugly.
Any generic way is also going to be big and ugly just like any
simplification/folding functions.
> Could you add some tests? I realize this fixes 390 testcases, but are there
> tests where we could explicitly check that this is triggering?
Added.
>
> s/simplication/simplification/
> There is another instance of this later.
corrected.
>
>> + VEC(use_pred_info_t, heap) * const*chain2
>> + = (VEC(use_pred_info_t, heap) * const*)p2;
>
> space around '*'.
corrected.
\>
> Why not just one 'if (ll == 2)'?
good catch -- missed an 'else' branch.
>
> So, this has been modifying the input array, what happens if it at some
> point during the iteration, it decides to return false? The caller will
> need to cope with the modified version of 'preds'?
It is outside the loop. which is an independent simplification -- the
number of chains are updated and the resulting chains are valid even
the latter replace does not happen.
> Space around '+'.
Added.
> Please add a comment describing what this loop does.
Done.
> End comment with '. */'
Done.
Thanks,
David
2011-03-08 Xinliang David Li
* gcc.dg/uninit-pred-7_d.c: New test.
* gcc.dg/uninit-pred-8_d.c: New test.
Index: tree-ssa-uninit.c
===
--- tree-ssa-uninit.c (revision 170150)
+++ tree-ssa-uninit.c (working copy)
@@ -1605,6 +1605,157 @@ is_superset_of (VEC(use_pred_info_t, hea
return true;
}
+/* Comparison function used by qsort. It is used to
+ sort predicate chains to allow predicate
+ simplification. */
+
+static int
+pred_chain_length_cmp (const void *p1, const void *p2)
+{
+ use_pred_info_t i1, i2;
+ VEC(use_pred_info_t, heap) * const *chain1
+ = (VEC(use_pred_info_t, heap) * const *)p1;
+ VEC(use_pred_info_t, heap) * const *chain2
+ = (VEC(use_pred_info_t, heap) * const *)p2;
+
+ if (VEC_length (use_pred_info_t, *chain1)
+ != VEC_length (use_pred_info_t, *chain2))
+return (VEC_length (use_pred_info_t, *chain1)
+- VEC_length (use_pred_info_t, *chain2));
+
+ i1 = VEC_index (use_pred_info_t, *chain1, 0);
+ i2 = VEC_index (use_pred_info_t, *chain2, 0);
+
+ /* Allow predicates with similar prefix come together. */
+ if (!i1->invert && i2->invert)
+return -1;
+ else if (i1->invert && !i2->invert)
+return 1;
+
+ return gimple_uid (i1->cond) - gimple_uid (i2->cond);
+}
+
+/* x OR (!x AND y) is equivalent to x OR y.
+ This function normalizes x1 OR (!x1 AND x2) OR (!x1 AND !x2 AND x3)
+ into x1 OR x2 OR x3. PREDS is the predicate chains, and N is
+ the number of chains. Returns true if normalization happens. */
+
+static bool
+normalize_preds (VEC(use_pred_info_t, heap) **preds, size_t *n)
+{
+ size_t i, j, ll;
+ VEC(use_pred_info_t, heap) *pred_chain;
+ VEC(use_pred_info_t, heap) *x = 0;
+ use_pred_info_t xj = 0, nxj = 0;
+
+ if (*n < 2)
+return false;
+
+ /* First sort the chains in ascending order of lengths. */
+ qsort (preds, *n, sizeof (void *), pred_chain_length_cmp);
+ pred_chain = preds[0];
+ ll = VEC_length (use_pred_info_t, pred_chain);
+ if (ll != 1)
+ {
+ if (ll == 2)
+ {
+ use_pred_info_t xx, yy, xx2, nyy;
+ VEC(use_pred_info_t, heap) *pred_chain2 = preds[1];
+ if (VEC_length (use_pred_info_t, pred_chain2) != 2)
+ return false;
+
+ /* See if simplification x AND y OR x AND !y is possible. */
+ xx = VEC_index (use_pred_info_t, pred_chain, 0);
+ yy = VEC_index (use_pred_info_t, pred_chain, 1);
+ xx2 = VEC_index (use_pred_info_t, pred_chain2, 0);
+ nyy = VEC_index (use_pred_info_t, pred_chain2, 1);
+ if (gimple_cond_lhs (xx->cond) != gimple_cond_lhs (xx2->cond)
+ || gimple_cond_rhs (xx->cond) != gimple_cond_rhs (xx2->cond)
+ || gimple_cond_code (xx->cond) != gimple_cond_code (xx2->cond)
+ || (xx->invert != xx2
Re: [v3] libstdc++/47145
On 8 March 2011 17:30, Benjamin Kosnik wrote: > >> > For convenience, this patch steps around the STYLESHEET_FLAG >> > question by just punting to the net for validation only. >> >> But it still uses --nonet in that step, right? So it doesn't use the >> network at all, the mapping from the canonical stylesheet URL to a >> local file is done by the system's XML catalog, /etc/xml/catalog on my >> Fedora box. > > No, for validation only I took out --nonet. Meaning, 'make > doc-validate-docbook' searches for a schema on the net and validates > against that, not a local thing. It takes longer now to validate, but I > don't think it is onerous. > > At least, that was my intent. Double check my work please. Ah, sorry, I missed that bit of the change, will take another look.
Re: C++ PATCH for c++/47705 (ICE with non-pointer argument to pointer template parameter)
On 03/08/2011 12:25 PM, Jason Merrill wrote:
We were asserting that any argument to a non-type template parameter of
pointer type must be an address. Which is true of valid code (apart from
null pointer values), but not necessarily of invalid code, where we
should complain rather than crash.
Dodji had an idea for a different way to fix this crash: avoid the call
to decay_conversion which adds the NOP to convert 'const int' to 'int'
by limiting it to the case where the argument is an array. I think I
like this way better.
Tested x86_64-pc-linux-gnu, applied to trunk.
commit c9e36778318c240777889a403693e95488a13b6d
Author: Jason Merrill
Date: Tue Mar 8 14:02:21 2011 -0500
PR c++/47705
* pt.c (convert_nontype_argument): Only call decay_conversion on
arrays.
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index cda9df8..2ca2cd0 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -5314,7 +5314,8 @@ convert_nontype_argument (tree type, tree expr,
tsubst_flags_t complain)
/* Add the ADDR_EXPR now for the benefit of
value_dependent_expression_p. */
- if (TYPE_PTROBV_P (type))
+ if (TYPE_PTROBV_P (type)
+ && TREE_CODE (TREE_TYPE (expr)) == ARRAY_TYPE)
expr = decay_conversion (expr);
/* If we are in a template, EXPR may be non-dependent, but still
@@ -5369,20 +5370,15 @@ convert_nontype_argument (tree type, tree expr,
tsubst_flags_t complain)
qualification conversion. Let's strip everything. */
else if (TYPE_PTROBV_P (type))
{
- tree sub = expr;
- STRIP_NOPS (sub);
- if (TREE_CODE (sub) == ADDR_EXPR)
- {
- gcc_assert (TREE_CODE (TREE_TYPE (sub)) == POINTER_TYPE);
- /* Skip the ADDR_EXPR only if it is part of the decay for
-an array. Otherwise, it is part of the original argument
-in the source code. */
- if (TREE_CODE (TREE_TYPE (TREE_OPERAND (sub, 0))) == ARRAY_TYPE)
- expr = TREE_OPERAND (sub, 0);
- else
- expr = sub;
- expr_type = TREE_TYPE (expr);
- }
+ STRIP_NOPS (expr);
+ gcc_assert (TREE_CODE (expr) == ADDR_EXPR);
+ gcc_assert (TREE_CODE (TREE_TYPE (expr)) == POINTER_TYPE);
+ /* Skip the ADDR_EXPR only if it is part of the decay for
+an array. Otherwise, it is part of the original argument
+in the source code. */
+ if (TREE_CODE (TREE_TYPE (TREE_OPERAND (expr, 0))) == ARRAY_TYPE)
+ expr = TREE_OPERAND (expr, 0);
+ expr_type = TREE_TYPE (expr);
}
}
diff --git a/gcc/testsuite/g++.dg/template/nontype21.C
b/gcc/testsuite/g++.dg/template/nontype21.C
index c0f5319..69cab54 100644
--- a/gcc/testsuite/g++.dg/template/nontype21.C
+++ b/gcc/testsuite/g++.dg/template/nontype21.C
@@ -4,4 +4,4 @@ template class Something { };
extern char const xyz;
-class SomethingElse:public Something { }; // { dg-error "const char *" }
+class SomethingElse:public Something { }; // { dg-error "xyz. is a
variable" }
Re: [Patch] PR c++/26256
2011/3/5 Jason Merrill :
> On 03/04/2011 03:11 AM, Fabien Chêne wrote:
>>
>> Hmm, I've implemented what you were suggesting, and I don't understand
>> the following check in supplement_binding:
>>
>> else if (TREE_CODE (bval) == TYPE_DECL&& DECL_ARTIFICIAL (bval))
>> {
>> /* The old binding was a type name. It was placed in
>> VALUE field because it was thought, at the point it was
>> declared, to be the only entity with such a name. Move the
>> type name into the type slot; it is now hidden by the new
>> binding. */
>> binding->type = bval;
>> binding->value = decl;
>> binding->value_is_inherited = false;
>> }
>>
>> Why is it usefull ? It prevents the following illegal code from being
>> rejected:
>>
>> struct A
>> {
>> struct type {};
>> typedef int type;
>> };
>
> That's a bug. I guess the check above needs to make sure that decl is not a
> TYPE_DECL.
OK, FYI I have opened PR c++/48010 for this bug, which I am going to fix first.
--
Fabien
Re: fix for pr47837
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 03/08/11 11:55, Diego Novillo wrote: > On 03/08/2011 12:54 PM, Xinliang David Li wrote: >> Please review the attached patch, it does some simplification of the >> complicated logical or expressions (x1 or x2 or x3 ...) constructed >> from control flow analysis into simpler form. >> >> Bootstraps and works on s390x for both testcases. >> >> Bootstraps on x86-64. Regression testing is on going (it takes forever >> (whole night already) to finish possibly because the lto test in >> c-torture ..). >> >> Ok for trunk? > > As a general comment, do you think we will start adding more and more of > these special pattern matchers into uninit analysis? I'm wondering how > much effort should we make into creating something more generic. > > Right now it's this pattern, but there may be others. It could grow > pretty big and ugly. We have a real problem in that our underlying analysis to eliminate unexecutable edges is the CFG needs help, particularly for path sensitive cases. Given that I'm seeing a real interest in other analysis that ultimately have problems similar to those for uninitialized variable analysis, building too much goo into tree-ssa-uninit doesn't seem like a long term solution. Jeff -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iQEcBAEBAgAGBQJNdqfpAAoJEBRtltQi2kC7eaEH/RW9KeI/ak0ZuRa3q1vABWlz ludq1GhcFC3PETXN7c89a9kfNF3fsSCEUrDWI+klddQVTuJW00915ZcK361Q9K91 ra/uGXJA1N2Uk/sVyb939Q3LkXtyCUrHGT/AIJe8e6FzXEZYCFt1UqOk5O0SVcqb VNAkZIHagdrGkGBpdn0nyDwf8nJly9iLq6koBPX1gRKXfeboMRUBSno0smqRi4GA 91JLYRwLx/Xydwyxg4hPTdhDZZKWbLhr8exrdvJCJ/eFJBpqtyVVtt5yS+km6Gbv xe/p/LOVfydNLgLeoAlEPrGIBmp/p5DOtg4MqLt51whJZ7TTveECwNdh3/57mXI= =BIpv -END PGP SIGNATURE-
[PATCH] Fix MEM_IN_STRUCT_P/MEM_SCALAR_P during expansion (PR rtl-optimization/47866)
Hi!
torture/vector-2.c testcase is miscompiled on ia64, apparently because
a store uses a result of POST_MODIFY, which during sched1 without cselib
is considered variable and is marked as MEM_IN_STRUCT_P while all other
memory stores/loads for that variable are MEM_SCALAR_P and
fixed_scalar_and_varying_struct_p in alias.c thus says that
a fixed scalar can't alias with a varying struct.
I believe the bug is in saying that the store is MEM_IN_STRUCT_P, if all
other stores/loads from that area are MEM_SCALAR_P (the variable
is a 16 byte vector, i.e. TImode variable on ia64), then this store
should be MEM_SCALAR_P too.
In *.optimized dump this is:
vector(4) int t;
vector(4) int t;
vector(4) int a0;
int D.2001;
int D.2000;
int D.1999;
int D.1998;
:
t = { 0, 0, 0, 0 };
BIT_FIELD_REF = 1;
a0_18 = t;
D.1998_3 = BIT_FIELD_REF ;
D.1999_4 = BIT_FIELD_REF ;
D.2000_5 = BIT_FIELD_REF ;
D.2001_6 = BIT_FIELD_REF ;
printf ("%d %d %d %d\n", D.1998_3, D.1999_4, D.2000_5, D.2001_6);
t = { 0, 0, 0, 0 };
BIT_FIELD_REF = 1;
a0_19 = t;
D.1998_8 = BIT_FIELD_REF ;
D.1999_9 = BIT_FIELD_REF ;
D.2000_10 = BIT_FIELD_REF ;
D.2001_11 = BIT_FIELD_REF ;
printf ("%d %d %d %d\n", D.1998_8, D.1999_9, D.2000_10, D.2001_11);
and as t isn't AGGREGATE_TYPE nor COMPLEX_TYPE and is a decl,
it is marked MEM_SCALAR_P and e.g. set_mem_attributes_minus_bitpos
once MEM_SCALAR_P is set doesn't change it to MEM_IN_STRUCT_P
because of BIT_FIELD_REF etc. The BIT_FIELD_REF = 1
stores are done through store_field though, and for some reason
the code sets MEM_IN_STRUCT_P in that codepath unconditionally.
Changing this fixes the testcase, bootstrapped/regtested on x86_64-linux and
i686-linux. I'll try to test this on ia64-linux tomorrow.
2011-03-08 Jakub Jelinek
PR rtl-optimization/47866
* expr.c (store_field): If MEM_SCALAR_P (target), don't use
MEM_SET_IN_STRUCT_P (to_rtx, 1), just set MEM_IN_STRUCT_P (to_rtx)
if target wasn't scalar.
--- gcc/expr.c.jj 2011-02-04 16:45:02.0 +0100
+++ gcc/expr.c 2011-03-08 20:49:19.531545778 +0100
@@ -5924,7 +5924,8 @@ store_field (rtx target, HOST_WIDE_INT b
if (to_rtx == target)
to_rtx = copy_rtx (to_rtx);
- MEM_SET_IN_STRUCT_P (to_rtx, 1);
+ if (!MEM_SCALAR_P (to_rtx))
+ MEM_IN_STRUCT_P (to_rtx) = 1;
if (!MEM_KEEP_ALIAS_SET_P (to_rtx) && MEM_ALIAS_SET (to_rtx) != 0)
set_mem_alias_set (to_rtx, alias_set);
Jakub
Re: [patch libiberty include]: Add additional helper functions for directory-separator searching
Actually, is there any case where lbasename wouldn't work instead of filename_dirrchr? (gdb is already making use of unix_lbasename / dos_lbasename at places where it needs to handle host vs target paths). -- Pedro Alves
Re: fix for pr47837
On Tue, Mar 8, 2011 at 2:04 PM, Jeff Law wrote: > -BEGIN PGP SIGNED MESSAGE- > Hash: SHA1 > > On 03/08/11 11:55, Diego Novillo wrote: >> On 03/08/2011 12:54 PM, Xinliang David Li wrote: >>> Please review the attached patch, it does some simplification of the >>> complicated logical or expressions (x1 or x2 or x3 ...) constructed >>> from control flow analysis into simpler form. >>> >>> Bootstraps and works on s390x for both testcases. >>> >>> Bootstraps on x86-64. Regression testing is on going (it takes forever >>> (whole night already) to finish possibly because the lto test in >>> c-torture ..). >>> >>> Ok for trunk? >> >> As a general comment, do you think we will start adding more and more of >> these special pattern matchers into uninit analysis? I'm wondering how >> much effort should we make into creating something more generic. >> >> Right now it's this pattern, but there may be others. It could grow >> pretty big and ugly. > We have a real problem in that our underlying analysis to eliminate > unexecutable edges is the CFG needs help, particularly for path > sensitive cases. > > Given that I'm seeing a real interest in other analysis that ultimately > have problems similar to those for uninitialized variable analysis, > building too much goo into tree-ssa-uninit doesn't seem like a long term > solution. Understood. Is it ok for short term until the long term solution exists -- this is a small incremental patch which has real benefit (reducing false positives). Thanks, David > > Jeff > -BEGIN PGP SIGNATURE- > Version: GnuPG v1.4.11 (GNU/Linux) > Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ > > iQEcBAEBAgAGBQJNdqfpAAoJEBRtltQi2kC7eaEH/RW9KeI/ak0ZuRa3q1vABWlz > ludq1GhcFC3PETXN7c89a9kfNF3fsSCEUrDWI+klddQVTuJW00915ZcK361Q9K91 > ra/uGXJA1N2Uk/sVyb939Q3LkXtyCUrHGT/AIJe8e6FzXEZYCFt1UqOk5O0SVcqb > VNAkZIHagdrGkGBpdn0nyDwf8nJly9iLq6koBPX1gRKXfeboMRUBSno0smqRi4GA > 91JLYRwLx/Xydwyxg4hPTdhDZZKWbLhr8exrdvJCJ/eFJBpqtyVVtt5yS+km6Gbv > xe/p/LOVfydNLgLeoAlEPrGIBmp/p5DOtg4MqLt51whJZ7TTveECwNdh3/57mXI= > =BIpv > -END PGP SIGNATURE- >
Re: [PATCH] Decrease number of threads used by goroutines.go test (take 2)
On Mar 8, 2011, at 10:44 AM, Jakub Jelinek wrote: > Because ulimit -u is Linux specific? Seems to work on darwin (266).
[PATCH] Re-fix PR target/47755 on powerpc VSX
PR 47755 fixed some bugs on VSX with V2DI constants, but the patch itself had
problems. In particular, the easy_altivec_constant support assumed the largest
int size was SImode. This would cause the compiler to generate VSPLTI{W,S,B}
to load up a constant instead of loading it up from memory.
This patch only allows (vector long long) { 0, 0 } and
(vector long long) { -1, -1 } as easy V2DI constants. There are some other
constants that could be generated using the Altivec instructions in the future.
I bootstrapped this patch and had no regressions with make check. Is it ok to
install?
[gcc]
2011-03-08 Michael Meissner
PR target/47755
* config/rs6000/rs6000.c (easy_altivec_constant): Correctly handle
V2DI/V2DF constants. Only all 0's or all 1's are easy.
(output_vec_const_move): Ditto.
[gcc/testsuite]
2011-03-08 Michael Meissner
PR target/47755
* gcc.target/powerpc/pr47755-2.c: New file.
Index: gcc/config/rs6000/rs6000.c
===
--- gcc/config/rs6000/rs6000.c (revision 170788)
+++ gcc/config/rs6000/rs6000.c (working copy)
@@ -4946,6 +4946,29 @@ easy_altivec_constant (rtx op, enum mach
else if (mode != GET_MODE (op))
return false;
+ /* V2DI/V2DF was added with VSX. Only allow 0 and all 1's as easy
+ constants. */
+ if (mode == V2DFmode)
+return zero_constant (op, mode);
+
+ if (mode == V2DImode)
+{
+ /* In case the compiler is built 32-bit, CONST_DOUBLE constants are not
+easy. */
+ if (GET_CODE (CONST_VECTOR_ELT (op, 0)) != CONST_INT
+ || GET_CODE (CONST_VECTOR_ELT (op, 1)) != CONST_INT)
+ return false;
+
+ if (zero_constant (op, mode))
+ return true;
+
+ if (INTVAL (CONST_VECTOR_ELT (op, 0)) == -1
+ && INTVAL (CONST_VECTOR_ELT (op, 1)) == -1)
+ return true;
+
+ return false;
+}
+
/* Start with a vspltisw. */
step = GET_MODE_NUNITS (mode) / 4;
copies = 1;
@@ -5022,8 +5045,16 @@ output_vec_const_move (rtx *operands)
vec = operands[1];
mode = GET_MODE (dest);
- if (TARGET_VSX && zero_constant (vec, mode))
-return "xxlxor %x0,%x0,%x0";
+ if (TARGET_VSX)
+{
+ if (zero_constant (vec, mode))
+ return "xxlxor %x0,%x0,%x0";
+
+ if (mode == V2DImode
+ && INTVAL (CONST_VECTOR_ELT (vec, 0)) == -1
+ && INTVAL (CONST_VECTOR_ELT (vec, 1)) == -1)
+ return "vspltisw %0,-1";
+}
if (TARGET_ALTIVEC)
{
Index: gcc/testsuite/gcc.target/powerpc/pr47755-2.c
===
--- gcc/testsuite/gcc.target/powerpc/pr47755-2.c(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr47755-2.c(revision 0)
@@ -0,0 +1,134 @@
+/* { dg-do run { target { powerpc*-*-* } } } */
+/* { dg-skip-if "" { powerpc*-*-darwin* } { "*" } { "" } } */
+/* { dg-require-effective-target powerpc_vsx_ok } */
+/* { dg-options "-O3 -mcpu=power7" } */
+
+/* PR 47755: Make sure compiler generates correct code for various
+ V2DI constants. */
+
+#ifdef DEBUG
+#include
+
+static int num_errors;
+#define FAIL_LL(A, B) \
+ (num_errors++, printf ("Fail (%i, %i)\n", (int)(A), (int)(B)))
+#define FAIL_I(A, B, C, D) \
+ (num_errors++, \
+ printf ("Fail (%i, %i, %i, %i)\n", (int)(A), (int)(B), (int)(C), (int)(D)))
+
+#else
+extern void abort (void) __attribute__((__noreturn__));
+#define FAIL_LL(A, B) abort ()
+#define FAIL_I(A, B, C, D) abort ()
+#endif
+
+static test_ll (vector long long, long long, long long)
__attribute__((__noinline__));
+
+static
+test_ll (vector long long v, long long a, long long b)
+{
+ union {
+vector long long v;
+long long ll[2];
+ } u;
+
+ u.v = v;
+ if (u.ll[0] != a && u.ll[1] != b)
+FAIL_LL (a, b);
+}
+
+#define TEST_LL(A,B) test_ll ((vector long long){ (A), (B) }, (A), (B))
+
+static test_i (vector int, int, int, int, int) __attribute__((__noinline__));
+
+static
+test_i (vector int v, int a, int b, int c, int d)
+{
+ union {
+vector int v;
+int i[4];
+ } u;
+
+ u.v = v;
+ if (u.i[0] != a && u.i[1] != b && u.i[2] != c && u.i[3] != d)
+FAIL_I (a, b, c, d);
+}
+
+#define TEST_I(A,B,C,D) \
+ test_i ((vector int){ (A), (B), (C), (D) }, (A), (B), (C), (D))
+
+int
+main (void)
+{
+ TEST_LL (-2LL, -2LL);
+ TEST_LL (-2LL, -1LL);
+ TEST_LL (-2LL, 0LL);
+ TEST_LL (-2LL, 1LL);
+ TEST_LL (-2LL, 2LL);
+
+ TEST_LL (-1LL, -2LL);
+ TEST_LL (-1LL, -1LL);
+ TEST_LL (-1LL, 0LL);
+ TEST_LL (-1LL, 1LL);
+ TEST_LL (-1LL, 2LL);
+
+ TEST_LL (0LL, -2LL);
+ TEST_LL (0LL, -1LL);
+ TEST_LL (0LL, 0LL);
+ TEST_LL (0LL, 1LL);
+ TEST_LL (0LL, 2LL);
+
+ TEST_LL (1LL, -2LL);
+ TEST_LL (1LL, -1LL);
+ TEST_LL (1LL, 0LL);
+ TEST_LL (1LL, 1LL);
+ TEST_LL (1LL, 2LL);
+
+ TEST_LL (2LL, -2LL);
+ TEST_LL (2LL, -1LL);
+ TEST_LL (2LL, 0LL);
+ TEST_LL (2LL, 1LL);
+ TEST_LL (2LL, 2LL);
+
+ /* We could use VSPLTI instructions for these tests. *
Re: [PATCH] Re-fix PR target/47755 on powerpc VSX
On Tue, Mar 8, 2011 at 6:56 PM, Michael Meissner
wrote:
> PR 47755 fixed some bugs on VSX with V2DI constants, but the patch itself had
> problems. In particular, the easy_altivec_constant support assumed the
> largest
> int size was SImode. This would cause the compiler to generate VSPLTI{W,S,B}
> to load up a constant instead of loading it up from memory.
>
> This patch only allows (vector long long) { 0, 0 } and
> (vector long long) { -1, -1 } as easy V2DI constants. There are some other
> constants that could be generated using the Altivec instructions in the
> future.
>
> I bootstrapped this patch and had no regressions with make check. Is it ok to
> install?
>
> [gcc]
> 2011-03-08 Michael Meissner
>
> PR target/47755
> * config/rs6000/rs6000.c (easy_altivec_constant): Correctly handle
> V2DI/V2DF constants. Only all 0's or all 1's are easy.
> (output_vec_const_move): Ditto.
>
> [gcc/testsuite]
> 2011-03-08 Michael Meissner
>
> PR target/47755
> * gcc.target/powerpc/pr47755-2.c: New file.
Should the CONST_INT test be wrapped in #ifdef HOST_BITS_PER_WIDE_INT?
Okay.
Thanks, David
Re: [PATCH] Re-fix PR target/47755 on powerpc VSX
On Tue, Mar 08, 2011 at 07:12:27PM -0500, David Edelsohn wrote:
> On Tue, Mar 8, 2011 at 6:56 PM, Michael Meissner
> wrote:
> > PR 47755 fixed some bugs on VSX with V2DI constants, but the patch itself
> > had
> > problems. In particular, the easy_altivec_constant support assumed the
> > largest
> > int size was SImode. This would cause the compiler to generate
> > VSPLTI{W,S,B}
> > to load up a constant instead of loading it up from memory.
> >
> > This patch only allows (vector long long) { 0, 0 } and
> > (vector long long) { -1, -1 } as easy V2DI constants. There are some other
> > constants that could be generated using the Altivec instructions in the
> > future.
> >
> > I bootstrapped this patch and had no regressions with make check. Is it ok
> > to
> > install?
> >
> > [gcc]
> > 2011-03-08 Michael Meissner
> >
> > PR target/47755
> > * config/rs6000/rs6000.c (easy_altivec_constant): Correctly handle
> > V2DI/V2DF constants. Only all 0's or all 1's are easy.
> > (output_vec_const_move): Ditto.
> >
> > [gcc/testsuite]
> > 2011-03-08 Michael Meissner
> >
> > PR target/47755
> > * gcc.target/powerpc/pr47755-2.c: New file.
>
> Should the CONST_INT test be wrapped in #ifdef HOST_BITS_PER_WIDE_INT?
I dunno. Given that it is harmless on 64-bit, I would tend to not put the
#ifdef there. But I would have no objection to adding it.
> Okay.
>
> Thanks, David
--
Michael Meissner, IBM
5 Technology Place Drive, M/S 2757, Westford, MA 01886-3141, USA
meiss...@linux.vnet.ibm.com fax +1 (978) 399-6899
[tree-ssa] conversion between dissimilar-sized pointers is not useless
Affects tpf, mips64, and m32c. Hand-checked tpf by inspection, m32c tests running now. Look OK so far? * tree-ssa.c (useless_type_conversion_p): Conversions between pointers of different modes are not useless. Index: tree-ssa.c === --- tree-ssa.c (revision 170807) +++ tree-ssa.c (working copy) @@ -1227,6 +1227,14 @@ != TYPE_ADDR_SPACE (TREE_TYPE (inner_type))) return false; + /* Some targets support multiple pointer sizes, others support +partial-int modes for some pointer types. Do not lose casts +between these. */ + if (TYPE_SIZE (inner_type) != TYPE_SIZE (outer_type) + || (GET_MODE_CLASS (TYPE_MODE (inner_type)) + != GET_MODE_CLASS (TYPE_MODE (outer_type + return false; + /* Do not lose casts to restrict qualified pointers. */ if ((TYPE_RESTRICT (outer_type) != TYPE_RESTRICT (inner_type))
Re: RFA: MN10300: Add support for SETLB and Lcc instructions
> + /* If the comparison has not already been split out of the branch
> + then do so now. */
> + if (REGNO (cmp_reg) != CC_REG)
> +{
> + rtx cmp;
> +
> + cmp = emit_insn_before (gen_cmpsi (cmp_reg, XEXP (comparison, 1)),
> branch);
> +
> + DUMP ("Extracted comparison from branch:", cmp);
> +}
This should never ever fire. These should have been split already,
at least with optimization, on which this pass is dependant. Just
assert here, if you like.
> +mn10300_block_contains_call (struct basic_block_def * block)
basic_block block
> + for (insn = block->il.rtl->head_;
> + insn != NULL_RTX;
> + insn = NEXT_INSN (insn))
> +{
> + if (CALL_P (insn))
> + return true;
> +
> + if (insn == block->il.rtl->end_)
> + break;
> +}
FOR_BB_INSNS (block, insn)
if (CALL_P (insn))
return true;
> + df_analyze ();
> + if (flow_loops_find (& loops) > 0)
> +{
> + unsigned int i;
> + loop_p loop;
> +
> + FOR_EACH_VEC_ELT (loop_p, loops.larray, i, loop)
You need compute_bb_for_insn, and to set up current_loops.
Then you can use
FOR_EACH_LOOP(liter, loop, LI_ONLY_INNERMOST)
instead of iterating over the array by hand. This will eliminate
at least the fake loop and non-innermost loop check that you do
by hand. Of course, current_loops must be cleared at the end.
> + else if (loop->header != loop->latch)
> + reason = "it is not a simple do-while loop";
Why? A loop with multiple blocks should be fine with Lcc.
I do see that you've got loop->header and loop->latch backward
further below; perhaps that was your reason for this?
> + rtx branch = loop->header->il.rtl->end_;
BB_END (loop->latch)
> + if (single_set (branch) == NULL_RTX
> + || GET_CODE (SET_SRC (single_set (branch))) != IF_THEN_ELSE)
!any_condjump_p (branch)
> + label = loop->latch->il.rtl->head_;
BB_HEAD (loop->header)
> + /* If necessary, extract the label from the branch insn. */
> + if (! LABEL_P (label))
This should never ever fire -- how else can the loop have been created?
Again, you can just assert here.
> + /* FIXME: Calling flow_loops_free() appears to be the correct thing to
> do,
> + but it results in a seg-fault when building regex.c in the target
> libiberty
> + library. I have no idea why; so I have disabled the call for now. */
> + /* flow_loops_free (& loops); */
I'm having a look into this. It's definitely non-obvious...
r~
Re: [patch libiberty include]: Add additional helper functions for directory-separator searching
> From: Pedro Alves > Date: Tue, 8 Mar 2011 22:37:59 + > Cc: DJ Delorie , > Eli Zaretskii , > ktiet...@googlemail.com, > binut...@sourceware.org, > gcc-patches@gcc.gnu.org > > Actually, is there any case where lbasename wouldn't > work instead of filename_dirrchr? Almost: lbasename returns a pointer one character _after_ the last slash. It also skips the drive letter on DOS/Windows (which might be TRT, actually). It would be reasonable to rewrite filename_dirrchr in terms of lbasename, though, by backing up the pointer returned by lbasename if it points to a slash, and otherwise returning NULL. The case of "d:foo" should also be resolved (probably, return a pointer to the colon).
libgo patch committed: Fix search for next prime
This patch to libgo fixes the search for the next prime to use for the
number of buckets. Bootstrapped and ran Go testsuite on
x86_64-unknown-linux-gnu. Committed to mainline.
The problem was reported in PR 47910, which this fixes.
Ian
diff -r 87faad0d0c52 libgo/runtime/go-new-map.c
--- a/libgo/runtime/go-new-map.c Mon Mar 07 15:35:24 2011 -0800
+++ b/libgo/runtime/go-new-map.c Tue Mar 08 21:27:32 2011 -0800
@@ -85,7 +85,7 @@
{
size_t mid;
- mid = (low + high / 2);
+ mid = (low + high) / 2;
/* Here LOW <= MID < HIGH. */
libgo patch committed: Only run net tests if GCCGO_RUN_ALL_TESTS
This patch to libgo only runs the networking dependent tests if GCCGO_RUN_ALL_TESTS is set in the environment. This is PR 48017. In that PR Rainer suggests having the tests drop back to UNSUPPORTED or UNRESOLVED if they fail to open a network connection. That is tempting but I don't agree with it, because it means that the tests will never fail. It might be better to have some way to test whether the network is available and usable before running the tests. However, that is in itself problematic, as people running all the gcc tests don't necessarily want their machine to start opening network connections. So I think the compromise of requiring an environment variable works well. Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu. Committed to mainline. Ian diff -r 49a9a1dcc2e8 libgo/Makefile.am --- a/libgo/Makefile.am Tue Mar 08 21:28:00 2011 -0800 +++ b/libgo/Makefile.am Tue Mar 08 21:51:48 2011 -0800 @@ -2952,13 +2952,13 @@ fmt/check \ gob/check \ html/check \ - http/check \ + $(if $(GCCGO_RUN_ALL_TESTS),http/check) \ io/check \ json/check \ log/check \ math/check \ mime/check \ - net/check \ + $(if $(GCCGO_RUN_ALL_TESTS),net/check) \ netchan/check \ os/check \ patch/check \ @@ -2974,7 +2974,7 @@ strconv/check \ strings/check \ sync/check \ - syslog/check \ + $(if $(GCCGO_RUN_ALL_TESTS),syslog/check) \ tabwriter/check \ template/check \ time/check \
Fix pr48032, powerpc64 -mcmodel=medium invalid ld offset
This patch
a) Moves the offsettable_ok_by_alignment call from rs6000_emit_move to
legitimate_constant_pool_address_p, and
b) teaches offsettable_ok_by_alignment how to handle -fsection-anchors
addresses, and
c) teaches offsettable_ok_by_alignment about other SYMBOL_REF_DECL
trees I see there, various constant trees and CONSTRUCTOR.
About (a), well, that's just the way I should have written the
cmodel=medium optimization in the first place. There is no alignment
reason to not create a cmodel=medium address (ie. addis,addi relative
to toc pointer), it's just that they do need to be sufficiently
aligned to use in a MEM. We want reg=cmodel_medium_losum; mem[reg]
whenever we can do so, rather than creating a toc entry, but must not
allow the sequence to be combined into mem[cmodel_medium_losum] if
cmodel_medium_losum is not offsettable to access all the bytes of mem.
Perhaps legitimate_constant_pool_address_p should be renamed. I
didn't do that because it already allows the non-constant-pool
cmodel=medium addresses, and the name is long enough now. I commented
the function instead.
(b) is necessary because -fsection-anchors unfortunately loses the
real SYMBOL_REF_DECL when substituting anchor+offset into MEMs. The
way I imlemented this is why legitimate_constant_pool_address_p needs
the MEM mode. I suppose it would be possible to keep the original
SYMBOL_REF_DECL around in the rtl by some means or find the decl in
the struct object_block info, but both of these options seem like
overkill to me. Note that I pass QImode to l_c_p_a_p from
rs6000_mode_dependent_address, print_operand_address, and the 'R'
constraint to indicate that any cmodel=medium address is permissable.
(c) was developed specifically to fix problems found on
ibm/gcc-4_5-branch. I'd still like to commit this on mainline even
though it seems that mainline creates VAR_DECLs for constants.
Bootstrapped and regression tested powerpc64-linux. OK to apply?
PR target/48032
* config/rs6000/rs6000.c (offsettable_ok_by_alignment): Do not
presume symbol_refs without a symbol_ref_decl are suitably
aligned, nor other trees we may see here. Handle anchor symbols.
(legitimate_constant_pool_address_p): Comment. Add mode param.
Check cmodel=medium addresses. Adjust all calls.
(rs6000_emit_move): Don't call offsettable_ok_by_alignment on
creating cmodel=medium optimized access to locals.
* config/rs6000/constraints.md (R): Pass QImode to
legitimate_constant_pool_address_p.
* config/rs6000/predicates.md (input_operand): Pass mode to
legitimate_constant_pool_address_p.
* config/rs6000/rs6000-protos.h (legitimate_constant_pool_address_p):
Update prototype.
Index: gcc/config/rs6000/rs6000.c
===
--- gcc/config/rs6000/rs6000.c (revision 170807)
+++ gcc/config/rs6000/rs6000.c (working copy)
@@ -5768,6 +5768,91 @@ virtual_stack_registers_memory_p (rtx op
&& regnum <= LAST_VIRTUAL_POINTER_REGISTER);
}
+/* Return true if memory accesses to OP are known to never straddle
+ a 32k boundary. */
+
+static bool
+offsettable_ok_by_alignment (rtx op, HOST_WIDE_INT offset,
+enum machine_mode mode)
+{
+ tree decl, type;
+ unsigned HOST_WIDE_INT dsize, dalign;
+
+ if (GET_CODE (op) != SYMBOL_REF)
+return false;
+
+ decl = SYMBOL_REF_DECL (op);
+ if (!decl)
+{
+ /* -fsection-anchors loses the original SYMBOL_REF_DECL when
+replacing memory addresses with an anchor plus offset. We
+could find the decl by rummaging around in the block->objects
+VEC for the given offset but that seems like too much work. */
+ dalign = 1;
+ if (SYMBOL_REF_HAS_BLOCK_INFO_P (op)
+ && SYMBOL_REF_ANCHOR_P (op)
+ && SYMBOL_REF_BLOCK (op) != NULL)
+ {
+ struct object_block *block = SYMBOL_REF_BLOCK (op);
+ HOST_WIDE_INT lsb, mask;
+
+ /* Given the alignment of the block.. */
+ dalign = block->alignment;
+ mask = dalign / BITS_PER_UNIT - 1;
+
+ /* ..and the combined offset of the anchor and any offset
+to this block object.. */
+ offset += SYMBOL_REF_BLOCK_OFFSET (op);
+ lsb = offset & -offset;
+
+ /* ..find how many bits of the alignment we know for the
+object. */
+ mask &= lsb - 1;
+ dalign = mask + 1;
+ }
+ return dalign >= GET_MODE_SIZE (mode);
+}
+
+ if (DECL_P (decl))
+{
+ if (TREE_CODE (decl) == FUNCTION_DECL)
+ return true;
+
+ if (!DECL_SIZE_UNIT (decl))
+ return false;
+
+ if (!host_integerp (DECL_SIZE_UNIT (decl), 1))
+ return false;
+
+ dsize = tree_low_cst (DECL_SIZE_UNIT (decl), 1);
+ if (dsize > 32768)
+ return false;
+
+ dalign = DECL_ALIGN_UNIT (decl);
+ return dalign >= dsize;
+}
+
libgo patch committed: Ignore EINTR in runtime_lock_full
This patch to libgo ignores EINTR when calling sem_wait in
runtime_lock_full. This is based on a patch from Rainer in PR 48019.
Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu.
Committed to mainline.
Ian
diff -r bdcef618b22e libgo/runtime/thread.c
--- a/libgo/runtime/thread.c Tue Mar 08 21:56:24 2011 -0800
+++ b/libgo/runtime/thread.c Tue Mar 08 22:29:23 2011 -0800
@@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
+#include
#include "runtime.h"
#include "go-assert.h"
@@ -32,8 +33,12 @@
static void
runtime_lock_full(Lock *l)
{
- if(sem_wait(&l->sem) != 0)
- runtime_throw("sem_wait failed");
+ for(;;){
+ if(sem_wait(&l->sem) == 0)
+ return;
+ if(errno != EINTR)
+ runtime_throw("sem_wait failed");
+ }
}
void
libgo patch committed: Ignore EINTR in connect
This libgo patch ignores an EINTR which occurs while calling connect on
a socket. Bootstrapped and ran Go testsuite on
x86_64-unknown-linux-gnu. Committed to mainline.
This is for PR 48019.
Ian
diff -r eb43a92af99e libgo/go/net/sock.go
--- a/libgo/go/net/sock.go Tue Mar 08 22:31:00 2011 -0800
+++ b/libgo/go/net/sock.go Tue Mar 08 22:45:17 2011 -0800
@@ -54,6 +54,9 @@
if ra != nil {
e = syscall.Connect(s, ra)
+ for e == syscall.EINTR {
+ e = syscall.Connect(s, ra)
+ }
if e != 0 {
closesocket(s)
return nil, os.Errno(e)
