Re: [Patch libfortran] PR 51803 getcwd() failure
Hi Janne, On 01/11/2012 08:37 AM, Janne Blomqvist wrote: Index: runtime/main.c === --- runtime/main.c (revision 183089) +++ runtime/main.c (working copy) @@ -116,8 +116,10 @@ store_exe_path (const char * argv0) memset (buf, 0, sizeof (buf)); #ifdef HAVE_GETCWD cwd = getcwd (buf, sizeof (buf)); + if (!cwd) +cwd = .; #else - cwd = ; + cwd = .; #endif I had preferred a patch like the following. Tobias --- a/libgfortran/runtime/main.c +++ b/libgfortran/runtime/main.c @@ -117,9 +117,16 @@ store_exe_path (const char * argv0) #ifdef HAVE_GETCWD cwd = getcwd (buf, sizeof (buf)); #else - cwd = ; + cwd = NULL; #endif + if (cwd == NULL) +{ + exe_path = argv0; + please_free_exe_path_when_done = 0; + return; +} + /* exe_path will be cwd + / + argv[0] + \0. This will not work if the executable is not in the cwd, but at this point we're out of better ideas. */
Re: [PATCH] Fix ICE in distribute_notes (PR bootstrap/51796)
And for noreturn calls, it doesn't do anything wrong, the problem is that it returns the old size for them. According to the head comment, that's precisely the problem, since it should do something for them, in particular put back the note. -- Eric Botcazou
Re: [patch] Fix crash on function returning variable-sized array
The new void *data pointer could use a comment on what it is and how it's used. Yes; as a matter of fact, it needs to be documented for mostly_copy_tree_r first, and probably just referred to for copy_if_shared_r and copy_if_shared. -- Eric Botcazou
Re: [PATCH] Fix ICE in distribute_notes (PR bootstrap/51796)
On Wed, Jan 11, 2012 at 09:16:04AM +0100, Eric Botcazou wrote: And for noreturn calls, it doesn't do anything wrong, the problem is that it returns the old size for them. According to the head comment, that's precisely the problem, since it should do something for them, in particular put back the note. You're right. So how about this patch (untested so far) instead? In the combiner it still needs to handle the case where REG_NORETURN note hasn't been placed yet, because then fixup_args_size_notes doesn't consider it being a noret call. 2012-01-11 Jakub Jelinek ja...@redhat.com PR bootstrap/51796 * combine.c (distribute_notes): If i3 is a noreturn call, allow old_size to be equal to args_size and make sure the noreturn call gets REG_ARGS_SIZE note. * expr.c (fixup_args_size_notes): Put REG_ARGS_SIZE notes on noreturn calls even when the delta is 0. --- gcc/combine.c.jj2011-12-09 15:21:20.0 +0100 +++ gcc/combine.c 2012-01-11 09:37:40.199961939 +0100 @@ -13281,8 +13281,30 @@ distribute_notes (rtx notes, rtx from_in if (!noop_move_p (i3)) { int old_size, args_size = INTVAL (XEXP (note, 0)); + bool noret_call = false; old_size = fixup_args_size_notes (PREV_INSN (i3), i3, args_size); - gcc_assert (old_size != args_size); + if (CALL_P (i3) !ACCUMULATE_OUTGOING_ARGS) + { + if (find_reg_note (i3, REG_NORETURN, NULL_RTX)) + noret_call = true; + else + { + rtx n; + for (n = next_note; n; n = XEXP (n, 1)) + if (REG_NOTE_KIND (n) == REG_NORETURN) + { + noret_call = true; + break; + } + } + } + gcc_assert (old_size != args_size || noret_call); + /* emit_call_1 adds for !ACCUMULATE_OUTGOING_ARGS +REG_ARGS_SIZE note to all noreturn calls, so ensure +the notes stay on the noreturn call. */ + if (noret_call + find_reg_note (i3, REG_ARGS_SIZE, NULL_RTX) == NULL_RTX) + place = i3; } break; --- gcc/expr.c.jj 2012-01-02 17:36:53.0 +0100 +++ gcc/expr.c 2012-01-11 09:30:08.549680295 +0100 @@ -3642,9 +3642,11 @@ mem_autoinc_base (rtx mem) (1) One or more auto-inc style memory references (aka pushes), (2) One or more addition/subtraction with the SP as destination, (3) A single move insn with the SP as destination, - (4) A call_pop insn. + (4) A call_pop insn, + (5) Noreturn call insns if !ACCUMULATE_OUTGOING_ARGS. - Insns in the sequence that do not modify the SP are ignored. + Insns in the sequence that do not modify the SP are ignored, + except for noreturn calls. The return value is the amount of adjustment that can be trivially verified, via immediate operand or auto-inc. If the adjustment @@ -3789,7 +3791,12 @@ fixup_args_size_notes (rtx prev, rtx las this_delta = find_args_size_adjust (insn); if (this_delta == 0) - continue; + { + if (!CALL_P (insn) + || ACCUMULATE_OUTGOING_ARGS + || find_reg_note (insn, REG_NORETURN, NULL_RTX) == NULL_RTX) + continue; + } gcc_assert (!saw_unknown); if (this_delta == HOST_WIDE_INT_MIN) --- gcc/testsuite/gcc.dg/pr51796.c.jj 2012-01-10 16:43:00.494803970 +0100 +++ gcc/testsuite/gcc.dg/pr51796.c 2012-01-10 16:43:00.494803970 +0100 @@ -0,0 +1,15 @@ +/* PR bootstrap/51796 */ +/* { dg-do compile } */ +/* { dg-options -Os -fno-omit-frame-pointer -fno-tree-dominator-opts -fno-tree-fre -fno-tree-pre } */ + +typedef void (*entry_func) (void) __attribute__ ((noreturn)); +extern entry_func entry_addr; +static void bsd_boot_entry (void) +{ + stop (); +} +void bsd_boot (void) +{ + entry_addr = (entry_func) bsd_boot_entry; + (*entry_addr) (); +} Jakub
Re: [patch] Fix ICEs with functions returning variable-sized array
On Tue, Jan 10, 2012 at 8:27 PM, Eric Botcazou ebotca...@adacore.com wrote: This is a couple of regressions present on the mainline. For the first testcase at O2 -gnatn: +===GNAT BUG DETECTED==+ | 4.7.0 20120102 (experimental) [trunk revision 182780] (i586-suse-linux) GCC error:| | in assign_stack_temp_for_type, at function.c:796 | | Error detected around p1.adb:3:4 For the second testcase: +===GNAT BUG DETECTED==+ | 4.7.0 20120102 (experimental) [trunk revision 182780] (i586-suse-linux) GCC error:| | in declare_return_variable, at tree-inline.c:2904 | | Error detected around p2.adb:3:4 Both are caused by the fnsplit IPA pass being run on a function returning a variable-sized array. In both cases, the part that isn't inlined is made up of a single raise statement, i.e. a no-return call. So fnsplit rewrites the call statement into just: f.part (arguments); In the first case, the compilation aborts when the RTL expander attempts to create a temporary for the return value (which would have variable size) while, in the second case, it aborts on the assertion: gcc_assert (TREE_CODE (TYPE_SIZE_UNIT (callee_type)) == INTEGER_CST); when the inliner attemps to inline the part that wasn't inlined(!). The proposed fix is to turn the part that isn't inlined into a function that returns void. This involves straightforward adjustments to the two versioning machineries (cgraph and tree). Tested on i586-suse-linux, OK for the mainline? Ok. Thanks, Richard. 2012-01-10 Eric Botcazou ebotca...@adacore.com * tree.h (build_function_decl_skip_args): Add boolean parameter. (build_function_type_skip_args): Delete. * tree.c (build_function_type_skip_args): Make static and add SKIP_RETURN parameter. Fix thinko in the handling of variants. (build_function_decl_skip_args): Add SKIP_RETURN parameter and pass it to build_function_type_skip_args. * cgraph.h (cgraph_function_versioning): Add boolean parameter. (tree_function_versioning): Likewise. * cgraph.c (cgraph_create_virtual_clone): Adjust call to build_function_decl_skip_args. * cgraphunit.c (cgraph_function_versioning): Add SKIP_RETURN parameter and pass it to build_function_decl_skip_args/tree_function_versioning. (cgraph_materialize_clone): Adjust call to tree_function_versioning. * ipa-inline-transform.c (save_inline_function_body): Likewise. * trans-mem.c (ipa_tm_create_version): Likewise. * tree-sra.c (modify_function): Likewise for cgraph_function_versioning. * tree-inline.c (declare_return_variable): Remove always-true test. (tree_function_versioning): Add SKIP_RETURN parameter. If the function returns non-void and SKIP_RETURN, create a void-typed RESULT_DECL. * ipa-split.c (split_function): Skip the return value for the split part if it doesn't return. 2012-01-10 Eric Botcazou ebotca...@adacore.com * gnat.dg/opt23.ad[sb]: New test. * gnat.dg/opt23_pkg.ad[sb]: New helper. * gnat.dg/opt24.ad[sb]: New test. -- Eric Botcazou
Re: Add -lssp_nonshared to LINK_SSP_SPEC
On Tue, Jan 10, 2012 at 8:50 PM, Tijl Coosemans t...@freebsd.org wrote: On Tuesday 10 January 2012 15:40:15 Richard Guenther wrote: On Tue, Jan 10, 2012 at 3:38 PM, Richard Guenther richard.guent...@gmail.com wrote: On Tue, Jan 10, 2012 at 3:14 PM, Tijl Coosemans t...@coosemans.org wrote: On targets where libc implements stack protector functions (GNU libc, FreeBSD libc), and where gcc (as an optimisation) generates calls to a locally defined __stack_chk_fail_local instead of directly calling the global function __stack_chk_fail (e.g. -fpic code on i386), one must explicitly specify -lssp_nonshared or -lc -lc_nonshared on the command line to statically link in __stack_chk_fail_local. It would be more convenient if the compiler kept the details of this target specific optimisation hidden by passing -lssp_nonshared to the linker internally. Here's a simple test case that shows the problem on i386-freebsd, but works just fine on e.g. x86_64 targets: % cat test.c int main( void ) { return( 0 ); } % gcc46 -o test test.c -fstack-protector-all -fPIE /var/tmp//ccjYQxKu.o: In function `main': test.c:(.text+0x37): undefined reference to `__stack_chk_fail_local' /usr/local/bin/ld: test: hidden symbol `__stack_chk_fail_local' isn't defined /usr/local/bin/ld: final link failed: Bad value collect2: ld returned 1 exit status I don't have commit access, so please commit when approved. Works fine for me on i?86-linux without -lssp_nonshared (which I do not have, so linking would fail). So my patch would actually break Linux? Yes. Probably because libc.so is a linker script: /* GNU ld script Use the shared library, but some functions are only in the static library, so try that secondarily. */ OUTPUT_FORMAT(elf64-x86-64) GROUP ( /lib64/libc.so.6 /usr/lib64/libc_nonshared.a AS_NEEDED ( /lib64/ld-linux-x86-64.so.2 ) ) and /usr/lib64/libc_nonshared.a provides the symbol. Why not fix it that way on *BSD? I'll discuss it with some FreeBSD developers. Currently FreeBSD doesn't use linker scripts anywhere. Would a FreeBSD specific version of the patch be acceptable? For instance, the version of GCC shipped with FreeBSD has been patched like this: http://svnweb.freebsd.org/base/head/contrib/gcc/config/freebsd-spec.h?r1=195697r2=195696 That looks better to me. Richard.
Re: [PATCH] Fix ICE in distribute_notes (PR bootstrap/51796)
You're right. So how about this patch (untested so far) instead? In the combiner it still needs to handle the case where REG_NORETURN note hasn't been placed yet, because then fixup_args_size_notes doesn't consider it being a noret call. Still too much work for the combiner in my opinion. :-) Can't we arrange for the REG_NORETURN and REG_ARGS_SIZE notes to be originally placed so that the combiner first puts back the former and second the latter? And why can't the latter note be put back by fixup_args_size_notes directly? -- Eric Botcazou
Re: [PATCH] Fix ICE in distribute_notes (PR bootstrap/51796)
On Wed, Jan 11, 2012 at 10:06:55AM +0100, Eric Botcazou wrote: You're right. So how about this patch (untested so far) instead? In the combiner it still needs to handle the case where REG_NORETURN note hasn't been placed yet, because then fixup_args_size_notes doesn't consider it being a noret call. Still too much work for the combiner in my opinion. :-) Can't we arrange for the REG_NORETURN and REG_ARGS_SIZE notes to be originally placed so that the combiner first puts back the former and second the latter? And why can't the IMHO reordering the notes in the chain would be even more work than this. latter note be put back by fixup_args_size_notes directly? fixup_args_size_notes does that if the REG_NORETURN note is already there. The if (noret_call find_reg_note (i3, REG_ARGS_SIZE, NULL_RTX) == NULL_RTX) place = i3; is all that is needed to add it if it wasn't already there. The rest of the patch is needed also for the assert checking. Jakub
Re: [PATCH] Fix ICE in distribute_notes (PR bootstrap/51796)
IMHO reordering the notes in the chain would be even more work than this. Can't we just move down the lines if (ecf_flags ECF_NORETURN) add_reg_note (call_insn, REG_NORETURN, const0_rtx); if (ecf_flags ECF_RETURNS_TWICE) { add_reg_note (call_insn, REG_SETJMP, const0_rtx); cfun-calls_setjmp = 1; } in emit_call_1 with a big comment explaining why the ECF_NORETURN note should come first? -- Eric Botcazou
Re: [PATCH] Adjust 'malloc' attribute documentation to match implementation
On Tue, 10 Jan 2012, Xinliang David Li wrote: of course your new version. I have installed it on the trunk. Richard. thanks, David On Tue, Jan 10, 2012 at 1:31 AM, Richard Guenther rguent...@suse.de wrote: On Mon, 9 Jan 2012, Xinliang David Li wrote: It looks non-ambiguous to me. The new proposed version or the old? Richard. David On Mon, Jan 9, 2012 at 1:05 AM, Richard Guenther rguent...@suse.de wrote: Since GCC 4.4 applying the malloc attribute to realloc-like functions does not work under the documented constraints because the contents of the memory pointed to are not properly transfered from the realloc argument (or treated as pointing to anything, like 4.3 behaved). The following adjusts documentation to reflect implementation reality (we do have an implementation detail that treats the memory blob returned for non-builtins as pointing to any global variable, but that is neither documented nor do I plan to do so - I presume it is to allow allocation + initialization routines to be marked with malloc, but even that area looks susceptible to misinterpretation to me). Any comments? Thanks, Richard. 2012-01-09 Richard Guenther rguent...@suse.de * doc/extend.texi (malloc attribute): Adjust according to implementation. Index: gcc/doc/extend.texi === --- gcc/doc/extend.texi (revision 183001) +++ gcc/doc/extend.texi (working copy) @@ -2771,13 +2771,12 @@ efficient @code{jal} instruction. @cindex @code{malloc} attribute The @code{malloc} attribute is used to tell the compiler that a function may be treated as if any non-@code{NULL} pointer it returns cannot -alias any other pointer valid when the function returns. +alias any other pointer valid when the function returns and that the memory +has undefined content. This will often improve optimization. Standard functions with this property include @code{malloc} and -@code{calloc}. @code{realloc}-like functions have this property as -long as the old pointer is never referred to (including comparing it -to the new pointer) after the function returns a non-@code{NULL} -value. +@code{calloc}. @code{realloc}-like functions do not have this +property as the memory pointed to does not have undefined content. @item mips16/nomips16 @cindex @code{mips16} attribute -- Richard Guenther rguent...@suse.de SUSE / SUSE Labs SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer -- Richard Guenther rguent...@suse.de SUSE / SUSE Labs SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
Re: [PATCH] Fix PR49642 in 4.6, questions about 4.7
On Tue, Jan 10, 2012 at 11:15 PM, William J. Schmidt wschm...@linux.vnet.ibm.com wrote: On Tue, 2012-01-10 at 09:42 -0600, William J. Schmidt wrote: On Tue, 2012-01-10 at 14:53 +0100, Richard Guenther wrote: Btw, this will also disqualify any point below if (__builtin_constant_p (...)) { ... } because after the if join all BBs are dominated by the __builtin_constant_p call. What we want to disallow is splitting at a block that is dominated by the true edge of the condition fed by the __builtin_constant_p result ... True. What we have is: D.1899_68 = __builtin_constant_p (D.1898_67); if (D.1899_68 != 0) goto bb 3; else goto bb 133; So I suppose we have to walk the immediate uses of the LHS of the call, find all that are part of a condition, and mark the target block for nonzero (in this case bb 3) as a forbidden dominator. I can tighten this up. Here's a revised patch for 4.6, following the above. The same patch applies to 4.7, if desired, optionally with an additional variation on the test case to add -fno-tree-fre to the compile step. Bootstrapped and regression tested on powerpc64-linux-gnu. OK for 4.6/trunk? Ok for trunk (and for 4.6 after a while without problems on trunk) with ... Thanks, Bill gcc: 2012-01-10 Bill Schmidt wschm...@linux.vnet.ibm.com PR tree-optimization/49642 * ipa-split.c (forbidden_dominators): New variable. (check_forbidden_calls): New function. (dominated_by_forbidden): Likewise. (consider_split): Check for forbidden dominators. (execute_split_functions): Initialize and free forbidden dominators info; call check_forbidden_calls. gcc/testsuite: 2012-01-10 Bill Schmidt wschm...@linux.vnet.ibm.com PR tree-optimization/49642 * gcc.dg/tree-ssa/pr49642.c: New test. Index: gcc/testsuite/gcc.dg/tree-ssa/pr49642.c === --- gcc/testsuite/gcc.dg/tree-ssa/pr49642.c (revision 0) +++ gcc/testsuite/gcc.dg/tree-ssa/pr49642.c (revision 0) @@ -0,0 +1,49 @@ +/* Verify that ipa-split is disabled following __builtin_constant_p. */ + +/* { dg-do compile } */ +/* { dg-options -O2 -fdump-tree-optimized } */ + +typedef unsigned int u32; +typedef unsigned long long u64; + +static inline __attribute__((always_inline)) __attribute__((const)) +int __ilog2_u32(u32 n) +{ + int bit; + asm (cntlzw %0,%1 : =r (bit) : r (n)); + return 31 - bit; +} + + +static inline __attribute__((always_inline)) __attribute__((const)) +int __ilog2_u64(u64 n) +{ + int bit; + asm (cntlzd %0,%1 : =r (bit) : r (n)); + return 63 - bit; +} + + + +static u64 ehca_map_vaddr(void *caddr); + +struct ehca_shca { + u32 hca_cap_mr_pgsize; +}; + +static u64 ehca_get_max_hwpage_size(struct ehca_shca *shca) +{ + return 1UL ( __builtin_constant_p(shca-hca_cap_mr_pgsize) ? ( (shca-hca_cap_mr_pgsize) 1 ? ilog2_NaN() : (shca-hca_cap_mr_pgsize) (1ULL 63) ? 63 : (shca-hca_cap_mr_pgsize) (1ULL 62) ? 62 : (shca-hca_cap_mr_pgsize) (1ULL 61) ? 61 : (shca-hca_cap_mr_pgsize) (1ULL 60) ? 60 : (shca-hca_cap_mr_pgsize) (1ULL 59) ? 59 : (shca-hca_cap_mr_pgsize) (1ULL 58) ? 58 : (shca-hca_cap_mr_pgsize) (1ULL 57) ? 57 : (shca-hca_cap_mr_pgsize) (1ULL 56) ? 56 : (shca-hca_cap_mr_pgsize) (1ULL 55) ? 55 : (shca-hca_cap_mr_pgsize) (1ULL 54) ? 54 : (shca-hca_cap_mr_pgsize) (1ULL 53) ? 53 : (shca-hca_cap_mr_pgsize) (1ULL 52) ? 52 : (shca-hca_cap_mr_pgsize) (1ULL 51) ? 51 : (shca-hca_cap_mr_pgsize) (1ULL 50) ? 50 : (shca-hca_cap_mr_pgsize) (1ULL 49) ? 49 : (shca-hca_cap_mr_pgsize) (1ULL 48) ? 48 : (shca-hca_cap_mr_pgsize) (1ULL 47) ? 47 : (shca-hca_cap_mr_pgsize) (1ULL 46) ? 46 : (shca-hca_cap_mr_pgsize) (1ULL 45) ? 45 : (shca-hca_cap_mr_pgsize) (1ULL 44) ? 44 : (shca-hca_cap_mr_pgsize) (1ULL 43) ? 43 : (shca-hca_cap_mr_pgsize) (1ULL 42) ? 42 : (shca-hca_cap_mr_pgsize) (1ULL 41) ? 41 : (shca-hca_cap_mr_pgsize) (1ULL 40) ? 40 : (shca-hca_cap_mr_pgsize) (1ULL 39) ? 39 : (shca-hca_cap_mr_pgsize) (1ULL 38) ? 38 : (shca-hca_cap_mr_pgsize) (1ULL 37) ? 37 : (shca-hca_cap_mr_pgsize) (1ULL 36) ? 36 : (shca-hca_cap_mr_pgsize) (1ULL 35) ? 35 : (shca-hca_cap_mr_pgsize) (1ULL 34) ? 34 : (shca-hca_cap_mr_pgsize) (1ULL 33) ? 33 : (shca-hca_cap_mr_pgsize) (1ULL 32) ? 32 : (shca-hca_cap_mr_pgsize) (1ULL 31) ? 31 : (shca-hca_cap_mr_pgsize) (1ULL 30) ? 30 : (shca-hca_cap_mr_pgsize) (1ULL 29) ? 29 : (shca-hca_cap_mr_pgsize) (1ULL 28) ? 28 : (shca-hca_cap_mr_pgsize) (1ULL 27) ? 27 : (shca-hca_cap_mr_pgsize) (1ULL 26) ? 26 : (shca-hca_cap_mr_pgsize) (1ULL 25) ? 25 : (shca-hca_cap_mr_pgsize) (1ULL 24) ? 24 : (shca-hca_cap_mr_pgsize) (1ULL 23) ? 23 : (shca-hca_cap_mr_pgsize) (1ULL 22) ? 22 : (shca-hca_cap_mr_pgsize) (1ULL 21) ? 21 : (shca-hca_cap_mr_pgsize)
Re: Ping: Re: [patch middle-end]: Fix PR/48814 - [4.4/4.5/4.6/4.7 Regression] Incorrect scalar increment result
On Tue, Jan 10, 2012 at 11:58 AM, Kai Tietz ktiet...@googlemail.com wrote: 2012/1/10 Richard Guenther richard.guent...@gmail.com: On Tue, Jan 10, 2012 at 10:58 AM, Kai Tietz ktiet...@googlemail.com wrote: Ping 2012/1/8 Kai Tietz ktiet...@googlemail.com: Hi, this patch makes sure that for increment of postfix-increment/decrement we use also orignal lvalue instead of tmp lhs value for increment. This fixes reported issue about sequence point in PR/48814 ChangeLog 2012-01-08 Kai Tietz kti...@redhat.com PR middle-end/48814 * gimplify.c (gimplify_self_mod_expr): Use for postfix-inc/dec lvalue instead of temporary lhs. Regression tested for x86_64-unknown-linux-gnu for all languages (including Ada and Obj-C++). Ok for apply? Regards, Kai Index: gimplify.c === --- gimplify.c (revision 182720) +++ gimplify.c (working copy) @@ -2258,7 +2258,7 @@ arith_code = POINTER_PLUS_EXPR; } - t1 = build2 (arith_code, TREE_TYPE (*expr_p), lhs, rhs); + t1 = build2 (arith_code, TREE_TYPE (*expr_p), lvalue, rhs); if (postfix) { Hi Richard, Please add testcases. Why does your patch make a difference? lhs is just the gimplified lvalue. yes, exactly this makes a big difference for post-inc/dec. I show you gimple-dump to illustrate this in more detail. I used here -O2 option with using attached patch. gcc without that patch produces following gimple for main: main () { int count.0; int count.1; int D.2721; int D.2725; int D.2726; count.0 = count; -- here we store orginal value 'count' for having array-access-index D.2721 = incr (); - within that function count gets modified arr[count.0] = D.2721; count.1 = count.0 + 1; - Here happens the issue. We increment the saved value of 'count' count = count.1; - By this the modification of count in incr() gets void. ... By the change we make sure to use count's value instead its saved temporary. Patched gcc produces this gimple: main () { int count.0; int count.1; int D.1718; int D.1722; int D.1723; count.0 = count; D.1718 = incr (); arr[count.0] = D.1718; count.0 = count; -- Reload count.0 for post-inc/dec to use count's current value count.1 = count.0 + 1; count = count.1; count.0 = count; Ok, here is the patch with adusted testcase from PR. With your patch we generate wrong code for volatile int count; int arr[4]; void foo() { arr[count++] = 0; } as we load from count two times instead of once. Similar for volatile int count; void bar(int); void foo() { bar(count++); } Please add those as testcases for any revised patch you produce. Thanks, Richard. ChangeLog 2012-01-10 Kai Tietz kti...@redhat.com PR middle-end/48814 * gimplify.c (gimplify_self_mod_expr): Use for postfix-inc/dec lvalue instead of temporary lhs. Regression tested for all languages (including Ada and Obj-C++). Ok for apply? Regards, Kai 2012-01-10 Kai Tietz kti...@redhat.com * gcc.c-torture/execute/pr48814.c: New test. Index: gcc/gcc/gimplify.c === --- gcc.orig/gcc/gimplify.c +++ gcc/gcc/gimplify.c @@ -2258,7 +2258,7 @@ gimplify_self_mod_expr (tree *expr_p, gi arith_code = POINTER_PLUS_EXPR; } - t1 = build2 (arith_code, TREE_TYPE (*expr_p), lhs, rhs); + t1 = build2 (arith_code, TREE_TYPE (*expr_p), lvalue, rhs); if (postfix) { Index: gcc/gcc/testsuite/gcc.c-torture/execute/pr48814.c === --- /dev/null +++ gcc/gcc/testsuite/gcc.c-torture/execute/pr48814.c @@ -0,0 +1,18 @@ +extern void abort (void); + +int arr[] = {1,2,3,4}; +int count = 0; + +int __attribute__((noinline)) +incr (void) +{ + return ++count; +} + +int main() +{ + arr[count++] = incr (); + if (count != 2 || arr[count] != 3) + abort (); + return 0; +}
Re: Ping: Re: [patch middle-end]: Fix PR/48814 - [4.4/4.5/4.6/4.7 Regression] Incorrect scalar increment result
On Wed, Jan 11, 2012 at 11:05 AM, Richard Guenther richard.guent...@gmail.com wrote: On Tue, Jan 10, 2012 at 11:58 AM, Kai Tietz ktiet...@googlemail.com wrote: 2012/1/10 Richard Guenther richard.guent...@gmail.com: On Tue, Jan 10, 2012 at 10:58 AM, Kai Tietz ktiet...@googlemail.com wrote: Ping 2012/1/8 Kai Tietz ktiet...@googlemail.com: Hi, this patch makes sure that for increment of postfix-increment/decrement we use also orignal lvalue instead of tmp lhs value for increment. This fixes reported issue about sequence point in PR/48814 ChangeLog 2012-01-08 Kai Tietz kti...@redhat.com PR middle-end/48814 * gimplify.c (gimplify_self_mod_expr): Use for postfix-inc/dec lvalue instead of temporary lhs. Regression tested for x86_64-unknown-linux-gnu for all languages (including Ada and Obj-C++). Ok for apply? Regards, Kai Index: gimplify.c === --- gimplify.c (revision 182720) +++ gimplify.c (working copy) @@ -2258,7 +2258,7 @@ arith_code = POINTER_PLUS_EXPR; } - t1 = build2 (arith_code, TREE_TYPE (*expr_p), lhs, rhs); + t1 = build2 (arith_code, TREE_TYPE (*expr_p), lvalue, rhs); if (postfix) { Hi Richard, Please add testcases. Why does your patch make a difference? lhs is just the gimplified lvalue. yes, exactly this makes a big difference for post-inc/dec. I show you gimple-dump to illustrate this in more detail. I used here -O2 option with using attached patch. gcc without that patch produces following gimple for main: main () { int count.0; int count.1; int D.2721; int D.2725; int D.2726; count.0 = count; -- here we store orginal value 'count' for having array-access-index D.2721 = incr (); - within that function count gets modified arr[count.0] = D.2721; count.1 = count.0 + 1; - Here happens the issue. We increment the saved value of 'count' count = count.1; - By this the modification of count in incr() gets void. ... By the change we make sure to use count's value instead its saved temporary. Patched gcc produces this gimple: main () { int count.0; int count.1; int D.1718; int D.1722; int D.1723; count.0 = count; D.1718 = incr (); arr[count.0] = D.1718; count.0 = count; -- Reload count.0 for post-inc/dec to use count's current value count.1 = count.0 + 1; count = count.1; count.0 = count; Ok, here is the patch with adusted testcase from PR. With your patch we generate wrong code for volatile int count; int arr[4]; void foo() { arr[count++] = 0; } as we load from count two times instead of once. Similar for volatile int count; void bar(int); void foo() { bar(count++); } Please add those as testcases for any revised patch you produce. I think a proper fix involves changing the order we gimplify the lhs/rhs for calls. Thanks, Richard. ChangeLog 2012-01-10 Kai Tietz kti...@redhat.com PR middle-end/48814 * gimplify.c (gimplify_self_mod_expr): Use for postfix-inc/dec lvalue instead of temporary lhs. Regression tested for all languages (including Ada and Obj-C++). Ok for apply? Regards, Kai 2012-01-10 Kai Tietz kti...@redhat.com * gcc.c-torture/execute/pr48814.c: New test. Index: gcc/gcc/gimplify.c === --- gcc.orig/gcc/gimplify.c +++ gcc/gcc/gimplify.c @@ -2258,7 +2258,7 @@ gimplify_self_mod_expr (tree *expr_p, gi arith_code = POINTER_PLUS_EXPR; } - t1 = build2 (arith_code, TREE_TYPE (*expr_p), lhs, rhs); + t1 = build2 (arith_code, TREE_TYPE (*expr_p), lvalue, rhs); if (postfix) { Index: gcc/gcc/testsuite/gcc.c-torture/execute/pr48814.c === --- /dev/null +++ gcc/gcc/testsuite/gcc.c-torture/execute/pr48814.c @@ -0,0 +1,18 @@ +extern void abort (void); + +int arr[] = {1,2,3,4}; +int count = 0; + +int __attribute__((noinline)) +incr (void) +{ + return ++count; +} + +int main() +{ + arr[count++] = incr (); + if (count != 2 || arr[count] != 3) + abort (); + return 0; +}
Re: Ping: Re: [patch middle-end]: Fix PR/48814 - [4.4/4.5/4.6/4.7 Regression] Incorrect scalar increment result
2012/1/11 Richard Guenther richard.guent...@gmail.com: On Wed, Jan 11, 2012 at 11:05 AM, Richard Guenther richard.guent...@gmail.com wrote: On Tue, Jan 10, 2012 at 11:58 AM, Kai Tietz ktiet...@googlemail.com wrote: 2012/1/10 Richard Guenther richard.guent...@gmail.com: On Tue, Jan 10, 2012 at 10:58 AM, Kai Tietz ktiet...@googlemail.com wrote: Ping 2012/1/8 Kai Tietz ktiet...@googlemail.com: Hi, this patch makes sure that for increment of postfix-increment/decrement we use also orignal lvalue instead of tmp lhs value for increment. This fixes reported issue about sequence point in PR/48814 ChangeLog 2012-01-08 Kai Tietz kti...@redhat.com PR middle-end/48814 * gimplify.c (gimplify_self_mod_expr): Use for postfix-inc/dec lvalue instead of temporary lhs. Regression tested for x86_64-unknown-linux-gnu for all languages (including Ada and Obj-C++). Ok for apply? Regards, Kai Index: gimplify.c === --- gimplify.c (revision 182720) +++ gimplify.c (working copy) @@ -2258,7 +2258,7 @@ arith_code = POINTER_PLUS_EXPR; } - t1 = build2 (arith_code, TREE_TYPE (*expr_p), lhs, rhs); + t1 = build2 (arith_code, TREE_TYPE (*expr_p), lvalue, rhs); if (postfix) { Hi Richard, Please add testcases. Why does your patch make a difference? lhs is just the gimplified lvalue. yes, exactly this makes a big difference for post-inc/dec. I show you gimple-dump to illustrate this in more detail. I used here -O2 option with using attached patch. gcc without that patch produces following gimple for main: main () { int count.0; int count.1; int D.2721; int D.2725; int D.2726; count.0 = count; -- here we store orginal value 'count' for having array-access-index D.2721 = incr (); - within that function count gets modified arr[count.0] = D.2721; count.1 = count.0 + 1; - Here happens the issue. We increment the saved value of 'count' count = count.1; - By this the modification of count in incr() gets void. ... By the change we make sure to use count's value instead its saved temporary. Patched gcc produces this gimple: main () { int count.0; int count.1; int D.1718; int D.1722; int D.1723; count.0 = count; D.1718 = incr (); arr[count.0] = D.1718; count.0 = count; -- Reload count.0 for post-inc/dec to use count's current value count.1 = count.0 + 1; count = count.1; count.0 = count; Ok, here is the patch with adusted testcase from PR. With your patch we generate wrong code for volatile int count; int arr[4]; void foo() { arr[count++] = 0; } as we load from count two times instead of once. Similar for volatile int count; void bar(int); void foo() { bar(count++); } Please add those as testcases for any revised patch you produce. I think a proper fix involves changing the order we gimplify the lhs/rhs for calls. Hmm, I don't see actual wrong code here. But I might missed something in spec. For exampl1 we get: foo () { volatile int count.0; volatile int count.1; volatile int count.2; count.0 = count; arr[count.0] = 0; count.1 = count; count.2 = count.1 + 1; count = count.2; } and here is no wrong code AFAICS. For second example we get: foo () { volatile int count.0; volatile int count.1; volatile int count.2; volatile int count.3; count.0 = count; count.3 = count.0; count.1 = count; count.2 = count.1 + 1; count = count.2; bar (count.3); } Here we don't have wrong code either AFAICS. Passed argument to bar is the value before increment, and count get incremented by 1 before call of bar, which is right. Thanks for more details, Kai
Re: Ping: Re: [patch middle-end]: Fix PR/48814 - [4.4/4.5/4.6/4.7 Regression] Incorrect scalar increment result
On Wed, Jan 11, 2012 at 11:19 AM, Kai Tietz ktiet...@googlemail.com wrote: 2012/1/11 Richard Guenther richard.guent...@gmail.com: On Wed, Jan 11, 2012 at 11:05 AM, Richard Guenther richard.guent...@gmail.com wrote: On Tue, Jan 10, 2012 at 11:58 AM, Kai Tietz ktiet...@googlemail.com wrote: 2012/1/10 Richard Guenther richard.guent...@gmail.com: On Tue, Jan 10, 2012 at 10:58 AM, Kai Tietz ktiet...@googlemail.com wrote: Ping 2012/1/8 Kai Tietz ktiet...@googlemail.com: Hi, this patch makes sure that for increment of postfix-increment/decrement we use also orignal lvalue instead of tmp lhs value for increment. This fixes reported issue about sequence point in PR/48814 ChangeLog 2012-01-08 Kai Tietz kti...@redhat.com PR middle-end/48814 * gimplify.c (gimplify_self_mod_expr): Use for postfix-inc/dec lvalue instead of temporary lhs. Regression tested for x86_64-unknown-linux-gnu for all languages (including Ada and Obj-C++). Ok for apply? Regards, Kai Index: gimplify.c === --- gimplify.c (revision 182720) +++ gimplify.c (working copy) @@ -2258,7 +2258,7 @@ arith_code = POINTER_PLUS_EXPR; } - t1 = build2 (arith_code, TREE_TYPE (*expr_p), lhs, rhs); + t1 = build2 (arith_code, TREE_TYPE (*expr_p), lvalue, rhs); if (postfix) { Hi Richard, Please add testcases. Why does your patch make a difference? lhs is just the gimplified lvalue. yes, exactly this makes a big difference for post-inc/dec. I show you gimple-dump to illustrate this in more detail. I used here -O2 option with using attached patch. gcc without that patch produces following gimple for main: main () { int count.0; int count.1; int D.2721; int D.2725; int D.2726; count.0 = count; -- here we store orginal value 'count' for having array-access-index D.2721 = incr (); - within that function count gets modified arr[count.0] = D.2721; count.1 = count.0 + 1; - Here happens the issue. We increment the saved value of 'count' count = count.1; - By this the modification of count in incr() gets void. ... By the change we make sure to use count's value instead its saved temporary. Patched gcc produces this gimple: main () { int count.0; int count.1; int D.1718; int D.1722; int D.1723; count.0 = count; D.1718 = incr (); arr[count.0] = D.1718; count.0 = count; -- Reload count.0 for post-inc/dec to use count's current value count.1 = count.0 + 1; count = count.1; count.0 = count; Ok, here is the patch with adusted testcase from PR. With your patch we generate wrong code for volatile int count; int arr[4]; void foo() { arr[count++] = 0; } as we load from count two times instead of once. Similar for volatile int count; void bar(int); void foo() { bar(count++); } Please add those as testcases for any revised patch you produce. I think a proper fix involves changing the order we gimplify the lhs/rhs for calls. Hmm, I don't see actual wrong code here. But I might missed something in spec. For exampl1 we get: foo () { volatile int count.0; volatile int count.1; volatile int count.2; count.0 = count; arr[count.0] = 0; count.1 = count; count.2 = count.1 + 1; count = count.2; } count despite being declared volatile and only loaded once in the source is loaded twice in gimple. If it were a HW register which destroys the device after the 2nd load without an intervening store you'd wrecked the device ;) Richard. and here is no wrong code AFAICS. For second example we get: foo () { volatile int count.0; volatile int count.1; volatile int count.2; volatile int count.3; count.0 = count; count.3 = count.0; count.1 = count; count.2 = count.1 + 1; count = count.2; bar (count.3); } Here we don't have wrong code either AFAICS. Passed argument to bar is the value before increment, and count get incremented by 1 before call of bar, which is right. Thanks for more details, Kai
Re: [PATCH] Fix ICE in distribute_notes (PR bootstrap/51796)
On Wed, Jan 11, 2012 at 10:28:55AM +0100, Eric Botcazou wrote: IMHO reordering the notes in the chain would be even more work than this. Can't we just move down the lines if (ecf_flags ECF_NORETURN) add_reg_note (call_insn, REG_NORETURN, const0_rtx); if (ecf_flags ECF_RETURNS_TWICE) { add_reg_note (call_insn, REG_SETJMP, const0_rtx); cfun-calls_setjmp = 1; } in emit_call_1 with a big comment explaining why the ECF_NORETURN note should come first? I'd find that very fragile. E.g. distribute_notes itself reverses the order of all notes, and for no REG_NOTES I think we rely on any ordering. Anyway, here is an (untested) variant of the patch that puts the REG_NORETURN note in first in distribute_notes: 2012-01-11 Jakub Jelinek ja...@redhat.com PR bootstrap/51796 * combine.c (distribute_notes): If i3 is a noreturn call, allow old_size to be equal to args_size and make sure the noreturn call gets REG_ARGS_SIZE note. * expr.c (fixup_args_size_notes): Put REG_ARGS_SIZE notes on noreturn calls even when the delta is 0. --- gcc/combine.c.jj2011-12-09 15:21:20.0 +0100 +++ gcc/combine.c 2012-01-11 09:37:40.199961939 +0100 @@ -13281,8 +13281,28 @@ distribute_notes (rtx notes, rtx from_in if (!noop_move_p (i3)) { int old_size, args_size = INTVAL (XEXP (note, 0)); + /* fixup_args_size_notes looks at REG_NORETURN note, +so ensure the note is placed there first. */ + if (CALL_P (i3)) + { + rtx *np; + for (np = next_note; *np; np = XEXP (*np, 1)) + if (REG_NOTE_KIND (*np) == REG_NORETURN) + { + rtx n = *np; + *np = XEXP (n, 1); + XEXP (n, 1) = REG_NOTES (i3); + REG_NOTES (i3) = n; + break; + } + } old_size = fixup_args_size_notes (PREV_INSN (i3), i3, args_size); - gcc_assert (old_size != args_size); + /* emit_call_1 adds for !ACCUMULATE_OUTGOING_ARGS +REG_ARGS_SIZE note to all noreturn calls, allow that here. */ + gcc_assert (old_size != args_size + || (CALL_P (i3) + !ACCUMULATE_OUTGOING_ARGS + find_reg_note (i3, REG_NORETURN, NULL_RTX))); } break; --- gcc/expr.c.jj 2012-01-02 17:36:53.0 +0100 +++ gcc/expr.c 2012-01-11 09:30:08.549680295 +0100 @@ -3642,9 +3642,11 @@ mem_autoinc_base (rtx mem) (1) One or more auto-inc style memory references (aka pushes), (2) One or more addition/subtraction with the SP as destination, (3) A single move insn with the SP as destination, - (4) A call_pop insn. + (4) A call_pop insn, + (5) Noreturn call insns if !ACCUMULATE_OUTGOING_ARGS. - Insns in the sequence that do not modify the SP are ignored. + Insns in the sequence that do not modify the SP are ignored, + except for noreturn calls. The return value is the amount of adjustment that can be trivially verified, via immediate operand or auto-inc. If the adjustment @@ -3789,7 +3791,12 @@ fixup_args_size_notes (rtx prev, rtx las this_delta = find_args_size_adjust (insn); if (this_delta == 0) - continue; + { + if (!CALL_P (insn) + || ACCUMULATE_OUTGOING_ARGS + || find_reg_note (insn, REG_NORETURN, NULL_RTX) == NULL_RTX) + continue; + } gcc_assert (!saw_unknown); if (this_delta == HOST_WIDE_INT_MIN) --- gcc/testsuite/gcc.dg/pr51796.c.jj 2012-01-10 16:43:00.494803970 +0100 +++ gcc/testsuite/gcc.dg/pr51796.c 2012-01-10 16:43:00.494803970 +0100 @@ -0,0 +1,15 @@ +/* PR bootstrap/51796 */ +/* { dg-do compile } */ +/* { dg-options -Os -fno-omit-frame-pointer -fno-tree-dominator-opts -fno-tree-fre -fno-tree-pre } */ + +typedef void (*entry_func) (void) __attribute__ ((noreturn)); +extern entry_func entry_addr; +static void bsd_boot_entry (void) +{ + stop (); +} +void bsd_boot (void) +{ + entry_addr = (entry_func) bsd_boot_entry; + (*entry_addr) (); +} Jakub
[Patch, Fortran] PR51816 - fix USE of intrinsic operators
This patch fixes two issues related to intrinsic operators: a) No error for nonexisting operators: USE m, operator(*) b) An bogus error if one tried to use-associate the same operator multiple times: USE m, operator(+), operator(+) Those are old issues. New issue (and thus the PR is marked as regression) is that the bogus error now also is printed for: USE m, operator(+) USE m, operator(+) Build and regtested on x86-64-linux. OK for trunk? Tobias 2011-01-11 Tobias Burnus bur...@net-b.de PR fortran/51816 * module.c (read_module): Don't make nonexisting intrinsic operators as found. (rename_list_remove_duplicate): New function. (gfc_use_modules): Use it. 2011-01-11 Tobias Burnus bur...@net-b.de PR fortran/51816 * gfortran.dg/use_18.f90: New. * gfortran.dg/use_19.f90: New. Index: gcc/fortran/module.c === --- gcc/fortran/module.c (revision 183091) +++ gcc/fortran/module.c (working copy) @@ -4465,7 +4465,7 @@ read_module (void) int i; int ambiguous, j, nuse, symbol; pointer_info *info, *q; - gfc_use_rename *u; + gfc_use_rename *u = NULL; gfc_symtree *st; gfc_symbol *sym; @@ -4678,6 +4678,8 @@ read_module (void) } mio_interface (gfc_current_ns-op[i]); + if (u !gfc_current_ns-op[i]) + u-found = 0; } mio_rparen (); @@ -6093,6 +6095,31 @@ gfc_use_module (gfc_use_list *module) } +/* Remove duplicated intrinsic operators from the rename list. */ + +static void +rename_list_remove_duplicate (gfc_use_rename *list) +{ + gfc_use_rename *seek, *last; + + for (; list; list = list-next) +if (list-op != INTRINSIC_USER list-op != INTRINSIC_NONE) + { + last = list; + for (seek = list-next; seek; seek = last-next) + { + if (list-op == seek-op) + { + last-next = seek-next; + free (seek); + } + else + last = seek; + } + } +} + + /* Process all USE directives. */ void @@ -6171,6 +6198,7 @@ gfc_use_modules (void) for (; module_list; module_list = next) { next = module_list-next; + rename_list_remove_duplicate (module_list-rename); gfc_use_module (module_list); if (module_list-intrinsic) free_rename (module_list-rename); Index: gcc/testsuite/gfortran.dg/use_18.f90 === --- gcc/testsuite/gfortran.dg/use_18.f90 (revision 0) +++ gcc/testsuite/gfortran.dg/use_18.f90 (working copy) @@ -0,0 +1,51 @@ +! { dg-do compile } +! +! PR fortran/51816 +! +! Contributed by Harald Anlauf +! +module foo + implicit none + type t + integer :: i + end type t + interface operator (*) + module procedure mult + end interface +contains + function mult (i, j) +type(t), intent(in) :: i, j +integer :: mult +mult = i%i * j%i + end function mult +end module foo + +module bar + implicit none + type t2 + integer :: i + end type t2 + interface operator () + module procedure gt + end interface +contains + function gt (i, j) +type(t2), intent(in) :: i, j +logical :: gt +gt = i%i j%i + end function gt +end module bar + +use bar, only : t2, operator() , operator() +use foo, only : t +use foo, only : operator (*) +use foo, only : t +use foo, only : operator (*) +implicit none +type(t) :: i = t(1), j = t(2) +type(t2) :: k = t2(1), l = t2(2) +print *, i*j +print *, k l +end + +! { dg-final { cleanup-modules foo bar } } Index: gcc/testsuite/gfortran.dg/use_19.f90 === --- gcc/testsuite/gfortran.dg/use_19.f90 (revision 0) +++ gcc/testsuite/gfortran.dg/use_19.f90 (working copy) @@ -0,0 +1,11 @@ +! { dg-do compile } +! +! PR fortran/51816 +! +module m +end module m + +use m, only: operator(/) ! { dg-error Intrinsic operator '/' referenced at .1. not found in module 'm' } +end + +! { dg-final { cleanup-modules m } }
Commit: RX: Add return pattern
Hi Guys, I am checking in the patch below to fix a problem building the RX port. Targets that define the simple_return pattern must also define a return pattern. Otherwise gcc/function.c will fail to build. Cheers Nick gcc/ChangeLog 2012-01-11 Nick Clifton ni...@redhat.com * config/rx/rx.md (return): Define pattern. Index: gcc/config/rx/rx.md === --- gcc/config/rx/rx.md (revision 183092) +++ gcc/config/rx/rx.md (working copy) @@ -340,6 +340,12 @@ (set_attr length 2)] ) +(define_expand return + [(return)] + + rx_expand_epilogue (false); DONE; +) + (define_insn simple_return [(return)]
Re: [PATCH] Fix PR49642 in 4.6, questions about 4.7
I think it should be unconditionally restrict splitting (I suppose on the trunk the __builtin_constant_p is optimized away already). Btw, this will also disqualify any point below if (__builtin_constant_p (...)) { ... } because after the if join all BBs are dominated by the __builtin_constant_p call. What we want to disallow is splitting at a block that is dominated by the true edge of the condition fed by the __builtin_constant_p result Honza? Well, just for record, the final version of patch seems to make sense for me ;) Thanks! It is an interesting side corner to say at least. Honza
Re: [PATCH] Fix PR49642 in 4.6, questions about 4.7
Well, just for record, the final version of patch seems to make sense for me ;) Thanks! It is an interesting side corner to say at least. Of course one could craft an function with two builtin_constant_p calls and the asm statement that is not dominated by either of them but still always constant. I am not sure how far we want to go guaranting that constant propagation happen. I think the simple dominator cases are enough in practice. Honza Honza
Re: [PATCH] Fix PR49642 in 4.6, questions about 4.7
On Wed, Jan 11, 2012 at 10:57:28AM +0100, Richard Guenther wrote: + tree fndecl; + + if (!is_gimple_call (stmt)) + return; + + fndecl = gimple_call_fndecl (stmt); + + if (fndecl + TREE_CODE (fndecl) == FUNCTION_DECL Not needed. + DECL_BUILT_IN (fndecl) DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL + /* At the moment, __builtin_constant_p is the only forbidden + predicate function call (see PR49642). */ + DECL_FUNCTION_CODE (fndecl) == BUILT_IN_CONSTANT_P) Or better yet if (!gimple_call_builtin_p (stmt, BUILT_IN_CONSTANT_P)) return; instead of all the above. Jakub
RE: [Ping] RE: CR16 Port addition
Hello Richard and Joseph, Thanks a lot for reviewing the patch. I will work on the problems pointed out by Richard. As he has mentioned, I can check in those changes post-commit. If so, I would prefer that option. In that case, do we need to wait for someone else's approval or can this be committed now? I don't have commit rights. Please let me know how to proceed. Thanks and Regards, Jayant Sonar [KPIT Cummins, Pune]
Re: [Patch libfortran] PR 51803 getcwd() failure
Dear all, this is a follow up patch, which I think provides a better handling if either getcwd fails or is not availble - or if the pathname in argv[0] already is an absolute patch, in which case concatenating current-working-directory + '/' + argv[0] does not really make sense. Build on x86-64-linux. OK for the trunk? Tobias 2012-01-11 Tobias Burnus bur...@net-b.de PR fortran/51803 * runtime/main.c (store_exe_path): Handle a failure of getcwd and take absolute pathnames into account. Index: libgfortran/runtime/main.c === --- libgfortran/runtime/main.c (revision 183092) +++ libgfortran/runtime/main.c (working copy) @@ -116,12 +116,17 @@ store_exe_path (const char * argv0) memset (buf, 0, sizeof (buf)); #ifdef HAVE_GETCWD cwd = getcwd (buf, sizeof (buf)); - if (!cwd) -cwd = .; #else - cwd = .; + cwd = NULL; #endif + if (!cwd || argv0[0] == DIR_SEPARATOR) +{ + exe_path = argv0; + please_free_exe_path_when_done = 0; + return; +} + /* exe_path will be cwd + / + argv[0] + \0. This will not work if the executable is not in the cwd, but at this point we're out of better ideas. */
Re: [PATCH] Fix ICE in distribute_notes (PR bootstrap/51796)
I'd find that very fragile. E.g. distribute_notes itself reverses the order of all notes, and for no REG_NOTES I think we rely on any ordering. AFAIK distribute_notes is the only routine that redistributes the RTL notes for a fixed insn, so I'm a little skeptical about the purported fragility here. My point is that the complexity should be in fixup_args_size_notes instead of distribute_notes. Tentative patch attached. PR bootstrap/51796 * calls.c (emit_call_1): Make sure REG_NORETURN note comes first. * combine.c (distribute_notes): If i3 is a noreturn call, allow old_size to be equal to args_size if !ACCUMULATE_OUTGOING_ARGS. * expr.c (fixup_args_size_notes): Put REG_ARGS_SIZE notes on noreturn calls even when the delta is 0. * rtlanal.c (find_and_remove_reg_note): New function. * rtl.h (find_and_remove_reg_note): Declare it. -- Eric Botcazou Index: calls.c === --- calls.c (revision 183092) +++ calls.c (working copy) @@ -413,15 +413,6 @@ emit_call_1 (rtx funexp, tree fntree ATT /* Create a nothrow REG_EH_REGION note, if needed. */ make_reg_eh_region_note (call_insn, ecf_flags, 0); - if (ecf_flags ECF_NORETURN) -add_reg_note (call_insn, REG_NORETURN, const0_rtx); - - if (ecf_flags ECF_RETURNS_TWICE) -{ - add_reg_note (call_insn, REG_SETJMP, const0_rtx); - cfun-calls_setjmp = 1; -} - SIBLING_CALL_P (call_insn) = ((ecf_flags ECF_SIBCALL) != 0); /* Restore this now, so that we do defer pops for this call's args @@ -451,6 +442,17 @@ emit_call_1 (rtx funexp, tree fntree ATT else if (!ACCUMULATE_OUTGOING_ARGS (ecf_flags ECF_NORETURN) != 0) add_reg_note (call_insn, REG_ARGS_SIZE, GEN_INT (stack_pointer_delta)); + if (ecf_flags ECF_RETURNS_TWICE) +{ + add_reg_note (call_insn, REG_SETJMP, const0_rtx); + cfun-calls_setjmp = 1; +} + + /* The REG_NORETURN note must come before the REG_ARGS_SIZE note for the + fixup_args_size_notes machinery to work correctly. */ + if (ecf_flags ECF_NORETURN) +add_reg_note (call_insn, REG_NORETURN, const0_rtx); + if (!ACCUMULATE_OUTGOING_ARGS) { /* If returning from the subroutine does not automatically pop the args, Index: combine.c === --- combine.c (revision 182780) +++ combine.c (working copy) @@ -13282,7 +13282,13 @@ distribute_notes (rtx notes, rtx from_in { int old_size, args_size = INTVAL (XEXP (note, 0)); old_size = fixup_args_size_notes (PREV_INSN (i3), i3, args_size); - gcc_assert (old_size != args_size); + /* emit_call_1 adds for !ACCUMULATE_OUTGOING_ARGS REG_ARGS_SIZE + note to all noreturn calls, but fixup_args_size_notes cannot + recompute the amount on its own, so the size is unchanged. */ + gcc_assert (old_size != args_size + || (CALL_P (i3) + !ACCUMULATE_OUTGOING_ARGS + find_reg_note (i3, REG_NORETURN, NULL_RTX))); } break; Index: expr.c === --- expr.c (revision 182780) +++ expr.c (working copy) @@ -3645,9 +3645,11 @@ mem_autoinc_base (rtx mem) (1) One or more auto-inc style memory references (aka pushes), (2) One or more addition/subtraction with the SP as destination, (3) A single move insn with the SP as destination, - (4) A call_pop insn. + (4) A call_pop insn, + (5) Noreturn call insns if !ACCUMULATE_OUTGOING_ARGS. - Insns in the sequence that do not modify the SP are ignored. + Insns in the sequence that do not modify the SP are ignored, + except for noreturn calls. The return value is the amount of adjustment that can be trivially verified, via immediate operand or auto-inc. If the adjustment @@ -3786,23 +3788,42 @@ fixup_args_size_notes (rtx prev, rtx las for (insn = last; insn != prev; insn = PREV_INSN (insn)) { HOST_WIDE_INT this_delta; + rtx note = NULL_RTX; if (!NONDEBUG_INSN_P (insn)) continue; this_delta = find_args_size_adjust (insn); if (this_delta == 0) - continue; + { + /* In the noreturn call case, we need to maintain the invariant + that the REG_NORETURN comes first. So remove it here... */ + if (!CALL_P (insn) + || ACCUMULATE_OUTGOING_ARGS + || (note = find_and_remove_reg_note + (insn, REG_NORETURN, NULL_RTX)) == NULL_RTX) + continue; + } gcc_assert (!saw_unknown); if (this_delta == HOST_WIDE_INT_MIN) saw_unknown = true; add_reg_note (insn, REG_ARGS_SIZE, GEN_INT (args_size)); + + if (this_delta == 0) + { + /* ...and add it back here. */ + XEXP (note, 1) = REG_NOTES (insn); + REG_NOTES (insn) = note; + } + else + { #ifdef STACK_GROWS_DOWNWARD - this_delta = -this_delta; + this_delta = -this_delta; #endif - args_size -=
Re: [Patch libfortran] PR 51803 getcwd() failure
On Wed, Jan 11, 2012 at 14:55, Tobias Burnus bur...@net-b.de wrote: Dear all, this is a follow up patch, which I think provides a better handling if either getcwd fails or is not availble - or if the pathname in argv[0] already is an absolute patch, in which case concatenating current-working-directory + '/' + argv[0] does not really make sense. Checking for an absolute path is already done a few lines up. So if you prefer the kind of approach that you have in your patch, IMHO a more correct patch would be Index: main.c === --- main.c (revision 183091) +++ main.c (working copy) @@ -106,22 +106,26 @@ #endif /* On the simulator argv is not set. */ - if (argv0 == NULL || argv0[0] == '/') + if (argv0 == NULL || argv0[0] == DIR_SEPARATOR) { exe_path = argv0; please_free_exe_path_when_done = 0; return; } - memset (buf, 0, sizeof (buf)); #ifdef HAVE_GETCWD cwd = getcwd (buf, sizeof (buf)); - if (!cwd) -cwd = .; #else - cwd = .; + cwd = NULL; #endif + if (!cwd) +{ + exe_path = argv0; + please_free_exe_path_when_done = 0; + return; +} + /* exe_path will be cwd + / + argv[0] + \0. This will not work if the executable is not in the cwd, but at this point we're out of better ideas. */ Also, I removed the memset() call as superfluous; getcwd() makes sure that the buffer is null terminated or it will return NULL instead of a pointer to the string. For my part this fixed patch would be Ok. -- Janne Blomqvist
Re: [RFC/ARM] Correct mov_notscc pattern
On 11/01/12 13:55, Matthew Gretton-Dann wrote: All, The attached patch corrects the mov_notscc pattern in arm.md. This issue also exists in 4.5 and 4.6, is it okay for me to backport the fix to those branches, as well as trunk? OK? OK all. R. Thanks, Matt gcc/ChangeLog: 2012-01-10 Matthew Gretton-Dann matthew.gretton-d...@arm.com * config/arm/arm.md (mov_notscc): Use MVN for false condition. gcc/testsuite/ChangeLog: 2012-01-10 Matthew Gretton-Dann matthew.gretton-d...@arm.com * testsuite/gcc.c-torture/execute/20120110-1.c: New testcase. 1-RFC-ARM-Correct-mov_notscc-pattern.txt diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md index 0e4bc3e..5620d7d 100644 --- a/gcc/config/arm/arm.md +++ b/gcc/config/arm/arm.md @@ -7726,7 +7726,7 @@ (not:SI (match_operator:SI 1 arm_comparison_operator [(match_operand 2 cc_register ) (const_int 0)])))] TARGET_ARM - mov%D1\\t%0, #0\;mvn%d1\\t%0, #1 + mvn%D1\\t%0, #0\;mvn%d1\\t%0, #1 [(set_attr conds use) (set_attr insn mov) (set_attr length 8)] diff --git a/gcc/testsuite/gcc.c-torture/execute/20120111-1.c b/gcc/testsuite/gcc.c-torture/execute/20120111-1.c new file mode 100644 index 000..eac086e --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/execute/20120111-1.c @@ -0,0 +1,18 @@ +#include stdlib.h +#include stdint.h + +uint32_t f0a (uint64_t arg2) __attribute__((noinline)); + +uint32_t +f0a (uint64_t arg) +{ + return ~(arg -3); +} + +int main() { + uint32_t r1; + r1 = f0a (12094370573988097329ULL); + if (r1 != ~0U) +abort (); + return 0; +}
Re: [Patch libfortran] PR 51803 getcwd() failure
On 01/11/2012 02:08 PM, Janne Blomqvist wrote: Checking for an absolute path is already done a few lines up. So if you prefer the kind of approach that you have in your patch, IMHO a more correct patch would be I had a quick chat with Kai and decided to leave the lower part as is. However, I realized that the check for an absolute path is not correct for Windows. With the help of Kai I came up with the attached version. OK for the trunk? Tobias 2012-01-11 Tobias Burnus bur...@net-b.de * runtime/main.c (store_exe_path): Fix absolute path detection for Windows. Index: libgfortran/runtime/main.c === --- libgfortran/runtime/main.c (revision 183093) +++ libgfortran/runtime/main.c (working copy) @@ -105,15 +105,22 @@ store_exe_path (const char * argv0) } #endif - /* On the simulator argv is not set. */ - if (argv0 == NULL || argv0[0] == '/') + /* If the path is absolute or on an simulator where argv is not set. */ +#ifdef __MINGW32__ + if (argv0 == NULL + || ('A' = argv0[0] argv0[0] = 'Z' argv0[1] == ':') + || ('a' = argv0[0] argv0[0] = 'z' argv0[1] == ':') + || (argv0[0] == '/' argv0[1] == '/') + || (argv0[0] == '\\' argv0[1] == '\\')) +#else + if (argv0 == NULL || argv0[0] == DIR_SEPARATOR) +#endif { exe_path = argv0; please_free_exe_path_when_done = 0; return; } - memset (buf, 0, sizeof (buf)); #ifdef HAVE_GETCWD cwd = getcwd (buf, sizeof (buf)); if (!cwd)
Re: [Patch libfortran] PR 51803 getcwd() failure
Same patch with a minor update: I changed cwd from char * to const char * as I spotted a compile time warning for cwd = .; which was along the lines that by the assignment the const qualifier is lost. Too bad that we cannot enable -Werror for libgfortran. On 01/11/2012 03:04 PM, Tobias Burnus wrote: I had a quick chat with Kai and decided to leave the lower part as is. However, I realized that the check for an absolute path is not correct for Windows. With the help of Kai I came up with the attached version. OK for the trunk? 2012-01-11 Tobias Burnus bur...@net-b.de * runtime/main.c (store_exe_path): Fix absolute path detection for Windows. Index: libgfortran/runtime/main.c === --- libgfortran/runtime/main.c (revision 183093) +++ libgfortran/runtime/main.c (working copy) @@ -86,7 +86,8 @@ store_exe_path (const char * argv0) #define DIR_SEPARATOR '/' #endif - char buf[PATH_MAX], *cwd, *path; + char buf[PATH_MAX], *path; + const char *cwd; /* This can only happen if store_exe_path is called multiple times. */ if (please_free_exe_path_when_done) @@ -105,15 +106,22 @@ store_exe_path (const char * argv0) } #endif - /* On the simulator argv is not set. */ - if (argv0 == NULL || argv0[0] == '/') + /* If the path is absolute or on an simulator where argv is not set. */ +#ifdef __MINGW32__ + if (argv0 == NULL + || ('A' = argv0[0] argv0[0] = 'Z' argv0[1] == ':') + || ('a' = argv0[0] argv0[0] = 'z' argv0[1] == ':') + || (argv0[0] == '/' argv0[1] == '/') + || (argv0[0] == '\\' argv0[1] == '\\')) +#else + if (argv0 == NULL || argv0[0] == DIR_SEPARATOR) +#endif { exe_path = argv0; please_free_exe_path_when_done = 0; return; } - memset (buf, 0, sizeof (buf)); #ifdef HAVE_GETCWD cwd = getcwd (buf, sizeof (buf)); if (!cwd)
Re: [Bug bootstrap/51705] [4.7 Regression] FreeBSD uses unsupported C++11 features when __cplusplus == 201103L
To eliminate any possible ambiguity, the patch is approved On 01/10/12 23:49, andreast at gcc dot gnu.org wrote: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51705 Andreas Toblerandreast at gcc dot gnu.org changed: What|Removed |Added Status|NEW |ASSIGNED --- Comment #46 from Andreas Toblerandreast at gcc dot gnu.org 2012-01-11 07:49:41 UTC --- I'm going to apply the patch from comment #45 this evening. Bootstraped several times, make check in fixincludes successful. This is the CL I prepared: 2012-01-11 Bruce Korbbk...@gnu.org Steven G. Karglka...@gcc.gnu.org Andreas Toblerandre...@fgznet.ch PR bootstrap/57105 PR preprocessor/51776 * inclhack.def (cdef_cplusplus): Add a replacement for [[noreturn]]. * fixincl.x: Regenerate. * tests/base/sys/cdefs.h: Update. * genfixes: Remove the 'Ver.' from the version check. Index: inclhack.def === --- inclhack.def(revision 183089) +++ inclhack.def(working copy) @@ -20,6 +20,7 @@ FIXINC_DEBUG = yes; #endif + /* On AIX when _LARGE_FILES is defined stdio.h defines fopen to * fopen64 etc. and this causes problems when building with g++ * because cstdio udefs everything from stdio.h, leaving us with @@ -1028,6 +1029,22 @@ test_text = '#define vfscanf __svfscanf'; }; +/* + * 'g++ -std=c++11' defines __cplusplus to 201103L, which suggests + * that it conforms to ISO/IEC 14882:2011. Until G++ fully conforms, + * it should not set __cplusplus to that value. It currently does + * not support the [[noreturn]] procedure attribute. + * When it does, this hack should be removed. + * SEE: gcc.gnu.org/bugzilla/show_bug.cgi?id=51776 + */ +fix = { +hackname = cdef_cplusplus; +files = sys/cdefs.h; +select= '\[\[noreturn\]\]'; +c_fix = format; +c_fix_arg = '__attribute__((__noreturn__))'; +test_text = #define _Noreturn[[noreturn]]; +}; /* * Fix various macros used to define ioctl numbers. Index: tests/base/sys/cdefs.h === --- tests/base/sys/cdefs.h (revision 183089) +++ tests/base/sys/cdefs.h (working copy) @@ -9,6 +9,11 @@ +#if defined( CDEF_CPLUSPLUS_CHECK ) +#define _Noreturn __attribute__((__noreturn__)) +#endif /* CDEF_CPLUSPLUS_CHECK */ + + #if defined( FREEBSD_GCC3_BREAKAGE_CHECK ) #if __GNUC__ == 2 __GNUC_MINOR__ = 7 #endif /* FREEBSD_GCC3_BREAKAGE_CHECK */ Index: genfixes === --- genfixes(revision 183089) +++ genfixes(working copy) @@ -62,7 +62,7 @@ AG=autogen $AG set -e -if [ -z `${AG} -v | fgrep 'Ver. 5.'` ] +if [ -z `${AG} -v | fgrep ' 5.'` ] then echo AutoGen appears to be out of date or not correctly installed. echo Please download and install:
Re: [Patch libfortran] PR 51803 getcwd() failure
On Wed, Jan 11, 2012 at 16:04, Tobias Burnus bur...@net-b.de wrote: On 01/11/2012 02:08 PM, Janne Blomqvist wrote: Checking for an absolute path is already done a few lines up. So if you prefer the kind of approach that you have in your patch, IMHO a more correct patch would be I had a quick chat with Kai and decided to leave the lower part as is. However, I realized that the check for an absolute path is not correct for Windows. With the help of Kai I came up with the attached version. OK for the trunk? With the minor change s/an simulator/a simulator/ in the comment, Ok. In practice, I don't think this matters at the moment, since the only place where exe_path is used is when building the argument list for addr2line in the backtrace generation. Which we don't do on Windows anyway, since windows lacks fork, exec, and pipe which we require to launch the subprocess. Making cwd const char* is also Ok. -- Janne Blomqvist
Go patch committed: Fix names for unnamed type functions
The mangled name of an unnamed type can include '.' characters, e.g., for an array of a named type. This can then appear in the name used for a hash or equality function for the type. The code which creates the externally visible identifier looks for '.' characters. In some cases this could cause confusion about the name, and cause name collisions. This patch to the Go frontend fixes the problem by changing the '.' characters in a mangled name used for a type functions. Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu. Committed to mainline. Ian diff -r 9481959e66ce go/types.cc --- a/go/types.cc Tue Jan 10 20:44:18 2012 -0800 +++ b/go/types.cc Wed Jan 11 08:23:15 2012 -0800 @@ -1504,7 +1504,17 @@ std::string base_name; if (name == NULL) -base_name = gogo-pack_hidden_name(this-mangled_name(gogo), false); +{ + // Mangled names can have '.' if they happen to refer to named + // types in some way. That's fine if this is simply a named + // type, but otherwise it will confuse the code that builds + // function identifiers. Remove '.' when necessary. + base_name = this-mangled_name(gogo); + size_t i; + while ((i = base_name.find('.')) != std::string::npos) + base_name[i] = '$'; + base_name = gogo-pack_hidden_name(base_name, false); +} else { // This name is already hidden or not as appropriate.
C++ PATCH for c++/51613 (accepts-invalid with ambiguous template instantiation)
Simple thinko; when copy/pasting this loop from elsewhere we needed to change decls_match to same_type_p since here we're just dealing with types. Tested x86_64-pc-linux-gnu, applying to trunk. commit 32ca2088d87e3a3204cc28f4d7fa543d5616f8b6 Author: Jason Merrill ja...@redhat.com Date: Wed Jan 11 09:05:31 2012 -0500 PR c++/51613 * pt.c (resolve_overloaded_unification): Compare types with same_type_p, not decls_match. diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index bc3dd97..ac72b6d 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -15471,7 +15471,7 @@ resolve_overloaded_unification (tree tparms, elem = tsubst (TREE_TYPE (fn), subargs, tf_none, NULL_TREE); if (try_one_overload (tparms, targs, tempargs, parm, elem, strict, sub_strict, addr_p, explain_p) - (!goodfn || !decls_match (goodfn, elem))) + (!goodfn || !same_type_p (goodfn, elem))) { goodfn = elem; ++good; diff --git a/gcc/testsuite/g++.dg/template/explicit-args5.C b/gcc/testsuite/g++.dg/template/explicit-args5.C new file mode 100644 index 000..d6c9a57 --- /dev/null +++ b/gcc/testsuite/g++.dg/template/explicit-args5.C @@ -0,0 +1,24 @@ +// PR c++/51613 + +templatetypename F, typename T +void apply(F f, T t) +{ +f(t); +} + +templatetypename T +void multi(T) +{ +} + +templatetypename T +void multi(T*) +{ +} + +int main() +{ +apply(multiint, 7); // { dg-error no match } + +return 0; +} commit a03d5b81f06edbe8b919baa4791f2d52ef223582 Author: Jason Merrill ja...@redhat.com Date: Wed Jan 11 09:05:49 2012 -0500 * decl.c (decls_match): Assert that the arguments are decls. diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index 7daac5f..ef43dbf 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -952,6 +952,8 @@ decls_match (tree newdecl, tree olddecl) interested in their types. */ return 0; + gcc_assert (DECL_P (newdecl)); + if (TREE_CODE (newdecl) == FUNCTION_DECL) { tree f1 = TREE_TYPE (newdecl);
Re: RFC: allowing fwprop to propagate subregs
Richard Sandiford wrote: At the moment, fwprop will propagate constants and registers even if no further rtl simplifications are possible: if (REG_P (new_rtx) || CONSTANT_P (new_rtx)) flags |= PR_CAN_APPEAR; What do you think about extending this to subregs? I've been testing your patch (in its latest version including the SUBREG test pointed out by H.J.), and ran into issues where this additional propagation suppresses other optimizations. In particular, I'm seeing this testsuite regression on x86_64: FAIL: gcc.target/i386/pr30315.c scan-assembler-times cmp 4 The problem is that the combined compute result and set condition code insn patterns sometimes no longer match. The source code in question looks like: int c; unsigned char de (unsigned char a, unsigned char b) { unsigned char difference = a - b; if (difference a) c--; return difference; } Before your patch, we generate insns like: (insn 3 4 5 2 (set (reg/v:QI 63 [ a ]) (subreg:QI (reg:SI 64 [ a ]) 0)) pr30315.i:5 66 {*movqi_internal} (expr_list:REG_DEAD (reg:SI 64 [ a ]) (nil))) (insn 5 3 6 2 (set (reg/v:QI 65 [ b ]) (subreg:QI (reg:SI 66 [ b ]) 0)) pr30315.i:5 66 {*movqi_internal} (expr_list:REG_DEAD (reg:SI 66 [ b ]) (nil))) (insn 9 6 10 2 (parallel [ (set (reg/v:QI 59 [ difference ]) (minus:QI (reg/v:QI 63 [ a ]) (reg/v:QI 65 [ b ]))) (clobber (reg:CC 17 flags)) ]) pr30315.i:6 287 {*subqi_1} (expr_list:REG_DEAD (reg/v:QI 65 [ b ]) (expr_list:REG_UNUSED (reg:CC 17 flags) (nil (insn 10 9 11 2 (set (reg:CC 17 flags) (compare:CC (reg/v:QI 63 [ a ]) (reg/v:QI 59 [ difference ]))) pr30315.i:7 4 {*cmpqi_1} (expr_list:REG_DEAD (reg/v:QI 63 [ a ]) (nil))) which get combined into: (insn 4 2 3 2 (set (reg:SI 66 [ b ]) (reg:SI 4 si [ b ])) pr30315.i:5 64 {*movsi_internal} (expr_list:REG_DEAD (reg:SI 4 si [ b ]) (nil))) (insn 3 4 5 2 (set (reg/v:QI 63 [ a ]) (subreg:QI (reg:SI 64 [ a ]) 0)) pr30315.i:5 66 {*movqi_internal} (expr_list:REG_DEAD (reg:SI 64 [ a ]) (nil))) (insn 10 9 11 2 (parallel [ (set (reg:CCC 17 flags) (compare:CCC (minus:QI (reg/v:QI 63 [ a ]) (subreg:QI (reg:SI 66 [ b ]) 0)) (reg/v:QI 63 [ a ]))) (set (reg/v:QI 59 [ difference ]) (minus:QI (reg/v:QI 63 [ a ]) (subreg:QI (reg:SI 66 [ b ]) 0))) ]) pr30315.i:7 322 {*subqi3_cc_overflow} (expr_list:REG_DEAD (reg:SI 66 [ b ]) (expr_list:REG_DEAD (reg/v:QI 63 [ a ]) (nil Now, with your patch we have instead before combine: (insn 9 6 10 2 (parallel [ (set (reg/v:QI 59 [ difference ]) (minus:QI (subreg:QI (reg:SI 64 [ a ]) 0) (subreg:QI (reg:SI 66 [ b ]) 0))) (clobber (reg:CC 17 flags)) ]) pr30315.i:6 287 {*subqi_1} (expr_list:REG_DEAD (reg:SI 66 [ b ]) (expr_list:REG_UNUSED (reg:CC 17 flags) (nil (insn 10 9 11 2 (set (reg:CC 17 flags) (compare:CC (subreg:QI (reg:SI 64 [ a ]) 0) (reg/v:QI 59 [ difference ]))) pr30315.i:7 4 {*cmpqi_1} (expr_list:REG_DEAD (reg:SI 64 [ a ]) (nil))) which combine is unfortunately unable to process: Trying 9 - 10: Failed to match this instruction: (parallel [ (set (reg:CC 17 flags) (compare:CC (subreg:QI (minus:SI (reg:SI 64 [ a ]) (reg:SI 66 [ b ])) 0) (subreg:QI (reg:SI 64 [ a ]) 0))) (set (reg/v:QI 59 [ difference ]) (minus:QI (subreg:QI (reg:SI 64 [ a ]) 0) (subreg:QI (reg:SI 66 [ b ]) 0))) ]) The problem appears to be that an RTX expression like (minus:QI (subreg:QI (reg:SI 64 [ a ]) 0) (subreg:QI (reg:SI 66 [ b ]) 0)) seems to be considered non-canonical by combine, and is instead transformed into (subreg:QI (minus:SI (reg:SI 64 [ a ]) (reg:SI 66 [ b ])) 0) This happens via combine.c:apply_distributive_law. On the one hand, this seems odd, as SUBREGs with a non-trivial expression inside will usually not match any insn pattern. On the other hand, I guess this might still be useful behaviour for combine on platforms that support only word arithmetic, when the SUBREG might be considered a split point. (Of course, this doesn't work for the compute-result-and-cc type patterns.) In either case, it is always odd to have RTX in the insn stream that isn't stable under common simplication ... Do you have any suggestions on how to fix this? If we add the fwprop patch, should we then disable apply_distributive_law for SUBREGs? Thanks, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE ulrich.weig...@de.ibm.com
[Patch, Fortran] PR 51800 - Fix -finit-local-zero with automatic arrays and -fno-automatic
-finit-* creates an initialization for local variables - either as static initializer or by initializing at run time. The latter also works with automatic variables, but was breaking with -fno-automatic, which causes all nonautomatic local variables to be placed in static memory. However, combining -finit-* -fno-automatic with automatic arrays is failing at resolution time. The fix turned out to be rather simple. I wondered about characters strings where the length is a nonconstant specification question (thus: they are also automatic data objects). It turned out that only the initialization was missing - no code generation (trans*.c) change and no other resolution change were required. The first part fixes a regression as -finit-* -fno-automatic could be combined before (albeit without initializing the automatic arrays - but there was no compile error). Build and regtested on x86-64-linux. OK for the trunk? Tobias 2012-01-11 Tobias Burnus bur...@net-b.de PR fortran/51800 * resolve.c (build_default_init_expr): Also initialize nonconstant-length strings with -finit-character=n. 2012-01-11 Tobias Burnus bur...@net-b.de PR fortran/51800 * gfortran.dg/init_flag_8.f90: New. * gfortran.dg/init_flag_9.f90: New. Index: gcc/fortran/resolve.c === --- gcc/fortran/resolve.c (revision 183093) +++ gcc/fortran/resolve.c (working copy) @@ -10143,6 +10143,26 @@ build_default_init_expr (gfc_symbol *sym) gfc_free_expr (init_expr); init_expr = NULL; } + if (!init_expr gfc_option.flag_init_character == GFC_INIT_CHARACTER_ON + sym-ts.u.cl-length) + { + gfc_actual_arglist *arg; + init_expr = gfc_get_expr (); + init_expr-where = sym-declared_at; + init_expr-ts = sym-ts; + init_expr-expr_type = EXPR_FUNCTION; + init_expr-value.function.isym = + gfc_intrinsic_function_by_id (GFC_ISYM_REPEAT); + init_expr-value.function.name = repeat; + arg = gfc_get_actual_arglist (); + arg-expr = gfc_get_character_expr (sym-ts.kind, sym-declared_at, + NULL, 1); + arg-expr-value.character.string[0] + = gfc_option.flag_init_character_value; + arg-next = gfc_get_actual_arglist (); + arg-next-expr = gfc_copy_expr (sym-ts.u.cl-length); + init_expr-value.function.actual = arg; + } break; default: @@ -10169,10 +10189,12 @@ apply_default_init_local (gfc_symbol *sym) if (init == NULL) return; - /* For saved variables, we don't want to add an initializer at - function entry, so we just add a static initializer. */ + /* For saved variables, we don't want to add an initializer at function + entry, so we just add a static initializer. Note that automatic variables + are stack allocated even with -fno-automatic. */ if (sym-attr.save || sym-ns-save_all - || gfc_option.flag_max_stack_var_size == 0) + || (gfc_option.flag_max_stack_var_size == 0 + (!sym-attr.dimension || !is_non_constant_shape_array (sym { /* Don't clobber an existing initializer! */ gcc_assert (sym-value == NULL); Index: gcc/testsuite/gfortran.dg/init_flag_8.f90 === --- gcc/testsuite/gfortran.dg/init_flag_8.f90 (revision 0) +++ gcc/testsuite/gfortran.dg/init_flag_8.f90 (working copy) @@ -0,0 +1,18 @@ +! { dg-do compile } +! { dg-options -fno-automatic -finit-local-zero } +! +! PR fortran/51800 +! +! Contributed by Mario Baumann +! + SUBROUTINE FOO( N, A ) + IMPLICIT NONE + INTEGER :: N + INTEGER :: A(1:N) + INTEGER :: J + INTEGER :: DUMMY(1:N) + DO J=1,N + DUMMY(J) = 0 + A(J) = DUMMY(J) + END DO + END SUBROUTINE FOO Index: gcc/testsuite/gfortran.dg/init_flag_9.f90 === --- gcc/testsuite/gfortran.dg/init_flag_9.f90 (revision 0) +++ gcc/testsuite/gfortran.dg/init_flag_9.f90 (working copy) @@ -0,0 +1,15 @@ +! { dg-do run } +! { dg-options -finit-character=89 } +! +! PR fortran/51800 +! + +subroutine foo(n) + character(len=n) :: str +! print *, str + if (str /= repeat ('Y', n)) call abort() +end subroutine foo + +call foo(3) +call foo(10) +end
Re: [PATCH] Fix ICE in distribute_notes (PR bootstrap/51796)
On Wed, Jan 11, 2012 at 01:59:59PM +0100, Eric Botcazou wrote: I'd find that very fragile. E.g. distribute_notes itself reverses the order of all notes, and for no REG_NOTES I think we rely on any ordering. AFAIK distribute_notes is the only routine that redistributes the RTL notes for a fixed insn, so I'm a little skeptical about the purported fragility here. My point is that the complexity should be in fixup_args_size_notes instead of distribute_notes. Tentative patch attached. If you prefer to do it this way, fine, but I think we should have some ENABLE_RTL_CHECKING verification somewhere that the REG_NORETURN vs. REG_ARGS_SIZE ordering is always correct. PR bootstrap/51796 * calls.c (emit_call_1): Make sure REG_NORETURN note comes first. * combine.c (distribute_notes): If i3 is a noreturn call, allow old_size to be equal to args_size if !ACCUMULATE_OUTGOING_ARGS. * expr.c (fixup_args_size_notes): Put REG_ARGS_SIZE notes on noreturn calls even when the delta is 0. * rtlanal.c (find_and_remove_reg_note): New function. * rtl.h (find_and_remove_reg_note): Declare it. Jakub
[gcov] ignore zero-function objects
I've committed this patch so that libgcov ignores object with no functions. Such objects can occur when it turns out that none of the instrumented functions were emitted -- the translation unit ends up with just data objects. nathan 2012-01-11 Nathan Sidwell nat...@acm.org * libgcov.c (__gcov_init): Ignore objects with no functions. Index: libgcov.c === --- libgcov.c (revision 183102) +++ libgcov.c (working copy) @@ -686,10 +686,14 @@ gcov_exit (void) void __gcov_init (struct gcov_info *info) { - if (!info-version) + if (!info-version || !info-n_functions) return; if (gcov_version (info, info-version, 0)) {
[gcov] Erase stale annotated source
I've committed this gcov patch, which does 2 things: *) if a source file contains no code, erase any annotation file for it. Such a file will be misleading as it will refer to a previous instrumented program. *) dynamically allocate the source line buffer, rather than use a fixed length buffer. nathan 2012-01-11 Nathan Sidwell nat...@acm.org * gcov.c (STRING_SIZE): Remove. (generate_results): Erase annotations for source files with no coverage information. (read_line): New. (output_lines): Use it. Index: gcov.c === --- gcov.c (revision 182755) +++ gcov.c (working copy) @@ -64,8 +64,6 @@ along with Gcov; see the file COPYING3. /* This is the size of the buffer used to read in source file lines. */ -#define STRING_SIZE 200 - struct function_info; struct block_info; struct source_info; @@ -708,24 +706,33 @@ generate_results (const char *file_name) function_summary (src-coverage, File); total_lines += src-coverage.lines; total_executed += src-coverage.lines_executed; - if (flag_gcov_file src-coverage.lines) + if (flag_gcov_file) { char *gcov_file_name = make_gcov_file_name (file_name, src-coverage.name); - FILE *gcov_file = fopen (gcov_file_name, w); - if (gcov_file) + if (src-coverage.lines) { - fnotice (stdout, Creating '%s'\n, gcov_file_name); - output_lines (gcov_file, src); - if (ferror (gcov_file)) + FILE *gcov_file = fopen (gcov_file_name, w); + + if (gcov_file) + { + fnotice (stdout, Creating '%s'\n, gcov_file_name); + output_lines (gcov_file, src); + if (ferror (gcov_file)) fnotice (stderr, Error writing output file '%s'\n, gcov_file_name); - fclose (gcov_file); + fclose (gcov_file); + } + else + fnotice (stderr, Could not open output file '%s'\n, + gcov_file_name); } else - fnotice (stderr, Could not open output file '%s'\n, - gcov_file_name); + { + unlink (gcov_file_name); + fnotice (stdout, Removing '%s'\n, gcov_file_name); + } free (gcov_file_name); } fnotice (stdout, \n); @@ -2188,6 +2195,44 @@ output_branch_count (FILE *gcov_file, in } +static const char * +read_line (FILE *file) +{ + static char *string; + static size_t string_len; + size_t pos = 0; + char *ptr; + + if (!string_len) +{ + string_len = 200; + string = XNEWVEC (char, string_len); +} + + while ((ptr = fgets (string + pos, string_len - pos, file))) +{ + size_t len = strlen (string + pos); + + if (string[pos + len - 1] == '\n') + { + string[pos + len - 1] = 0; + return string; + } + pos += len; + ptr = XNEWVEC (char, string_len * 2); + if (ptr) + { + memcpy (ptr, string, pos); + string = ptr; + string_len += 2; + } + else + pos = 0; +} + + return pos ? string : NULL; +} + /* Read in the source file one line at a time, and output that line to the gcov file preceded by its execution count and other information. */ @@ -2198,8 +2243,7 @@ output_lines (FILE *gcov_file, const sou FILE *source_file; unsigned line_num; /* current line number. */ const line_t *line; /* current line info ptr. */ - char string[STRING_SIZE]; /* line buffer. */ - char const *retval = ; /* status of source file reading. */ + const char *retval = ; /* status of source file reading. */ function_t *fn = NULL; fprintf (gcov_file, %9s:%5d:Source:%s\n, -, 0, src-coverage.name); @@ -2246,31 +2290,20 @@ output_lines (FILE *gcov_file, const sou fprintf (gcov_file, \n); } + if (retval) + retval = read_line (source_file); + /* For lines which don't exist in the .bb file, print '-' before the source line. For lines which exist but were never - executed, print '#' before the source line. Otherwise, - print the execution count before the source line. There are - 16 spaces of indentation added before the source line so that - tabs won't be messed up. */ - fprintf (gcov_file, %9s:%5u:, + executed, print '#' or '=' before the source line. + Otherwise, print the execution count before the source line. + There are 16 spaces of indentation added before the source + line so that tabs won't be messed up. */ + fprintf (gcov_file, %9s:%5u:%s\n, !line-exists ? - : line-count ? format_gcov (line-count, 0, -1) - : line-unexceptional ? # : =, line_num); - - if (retval) - { - /* Copy source line. */ - do - { - retval = fgets (string, STRING_SIZE, source_file); - if (!retval) - break; - fputs (retval, gcov_file); - } - while (!retval[0] || retval[strlen (retval) - 1] != '\n'); - } - if (!retval) - fputs (/*EOF*/\n, gcov_file); + : line-unexceptional ? # : =, line_num, + retval ? retval : /*EOF*/); if (flag_all_blocks) { @@
[Patch, Fortran] PR 51057/PR 51616 - fix quad_2.f90 test case
The test case quad_2.f90 was failing for multiple reasons: * Some systems support REAL(16) but do not have a sqrtl function * Some systems have a REAL(16) data type, but with less precision (106 instread of 113 digits) This patch fixes the issue by adding an effective target macro to check for the former - and by distinguishing the available digits at compile time. The patch was is mostly by Dominique (bid kudos!), I mainly changed the name of the dg check. The patch was tested on several systems by Dominique, Iain and Eric - and I just tested it on x86-64-linux (with libquadmath). I intent to commit the patch tomorrow. Tobias 2012-01-11 Dominique d'Humieres domi...@lps.ens.fr Tobias Burnus bur...@net-b.de PR fortran/51057 PR fortran/51616 * lib/target-supports.exp (check_effective_target_fortran_largest_fp_has_sqrt): New. * gfortran.dg/quad_2.f90: Use it, add pattern for IBM's real(16). Index: gcc/testsuite/lib/target-supports.exp === --- gcc/testsuite/lib/target-supports.exp (Revision 183101) +++ gcc/testsuite/lib/target-supports.exp (Arbeitskopie) @@ -984,6 +984,28 @@ }] } + +# Return 1 if the target supports SQRT for the largest floating-point +# type. (Some targets lack the libm support for this FP type.) +# On most targets, this check effectively checks either whether sqrtl is +# available or on __float128 systems whether libquadmath is installed, +# which provides sqrtq. +# +# When the target name changes, replace the cached result. + +proc check_effective_target_fortran_largest_fp_has_sqrt { } { +return [check_no_compiler_messages fortran_largest_fp_has_sqrt executable { + ! Fortran +use iso_fortran_env, only: real_kinds +integer,parameter:: maxFP = real_kinds(ubound(real_kinds,dim=1)) + real(kind=maxFP), volatile :: x +x = 2.0_maxFP + x = sqrt (x) + end +}] +} + + # Return 1 if the target supports Fortran integer kinds larger than # integer(8), 0 otherwise. # Index: gcc/testsuite/gfortran.dg/quad_2.f90 === --- gcc/testsuite/gfortran.dg/quad_2.f90 (Revision 183101) +++ gcc/testsuite/gfortran.dg/quad_2.f90 (Arbeitskopie) @@ -1,4 +1,5 @@ ! { dg-do run } +! { dg-require-effective-target fortran_largest_fp_has_sqrt } ! ! This test checks whether the largest possible ! floating-point number works. @@ -40,22 +41,36 @@ if (str2 /= 1.) call abort() if (str3 /=1.4142135623730951) call abort() if (str4 /= 1.4142135623730951) call abort() + case (10) if (str1 /=1.) call abort() if (str2 /= 1.) call abort() if (str3 /=1.41421356237309504876) call abort() if (str4 /= 1.41421356237309504876) call abort() + case (16) if (str1 /=1.000) call abort() if (str2 /= 1.000) call abort() - if (str3 /=1.41421356237309504880168872420969798) call abort() - if (str4 /= 1.41421356237309504880168872420969798) call abort() + + if (digits(1.0_qp) == 113) then + ! IEEE 754 binary 128 format + ! e.g. libquadmath/__float128 on i686/x86_64/ia64 + if (str3 /=1.41421356237309504880168872420969798) call abort() + if (str4 /= 1.41421356237309504880168872420969798) call abort() + else if (digits(1.0_qp) == 106) then + ! IBM binary 128 format + if (str3(1:37) /=1.41421356237309504880168872420969) call abort() + if (str4(1:34) /= 1.41421356237309504880168872420969) call abort() + end if + + ! Do a libm run-time test block real(qp), volatile :: fp2a fp2a = 2.0_qp fp2a = sqrt (fp2a) if (abs (fp2a - fp2) sqrt(2.0_qp)-nearest(sqrt(2.0_qp),-1.0_qp)) call abort() end block + case default call abort() end select
Re: RFC: allowing fwprop to propagate subregs
On 01/11/2012 05:55 PM, Ulrich Weigand wrote: In either case, it is always odd to have RTX in the insn stream that isn't stable under common simplication ... Do you have any suggestions on how to fix this? If we add the fwprop patch, should we then disable apply_distributive_law for SUBREGs? This strange canonicalization is also the cause of PR39726, where I also wanted to move subregs inside arithmetic operations. See the attached patch (which is not enough to fix the PR; this is just the combine part). Unfortunately, doing this also caused x86_64 regressions, I think in zero_extract patterns. Paolo change canonicalization Index: gcc/Makefile.in === --- gcc/Makefile.in (branch pr39726) +++ gcc/Makefile.in (working copy) @@ -2844,7 +2844,7 @@ jump.o : jump.c $(CONFIG_H) $(SYSTEM_H) simplify-rtx.o : simplify-rtx.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) \ $(RTL_H) $(REGS_H) hard-reg-set.h $(FLAGS_H) $(REAL_H) insn-config.h \ $(RECOG_H) $(EXPR_H) $(TOPLEV_H) output.h $(FUNCTION_H) $(GGC_H) $(TM_P_H) \ - $(TREE_H) $(TARGET_H) + $(TREE_H) $(TARGET_H) $(OPTABS_H) cgraph.o : cgraph.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(TREE_H) \ langhooks.h $(TOPLEV_H) $(FLAGS_H) $(GGC_H) $(TARGET_H) $(CGRAPH_H) \ gt-cgraph.h output.h intl.h $(BASIC_BLOCK_H) debug.h $(HASHTAB_H) \ Index: gcc/simplify-rtx.c === --- gcc/simplify-rtx.c (branch pr39726) +++ gcc/simplify-rtx.c (working copy) @@ -39,6 +39,7 @@ along with GCC; see the file COPYING3. #include output.h #include ggc.h #include target.h +#include optabs.h /* Simplification and canonicalization of RTL. */ @@ -5191,6 +5192,8 @@ rtx simplify_subreg (enum machine_mode outermode, rtx op, enum machine_mode innermode, unsigned int byte) { + int is_lowpart; + /* Little bit of sanity checking. */ gcc_assert (innermode != VOIDmode); gcc_assert (outermode != VOIDmode); @@ -5300,11 +5303,13 @@ simplify_subreg (enum machine_mode outer return NULL_RTX; } + is_lowpart = subreg_lowpart_offset (outermode, innermode) == byte; + /* Merge implicit and explicit truncations. */ if (GET_CODE (op) == TRUNCATE GET_MODE_SIZE (outermode) GET_MODE_SIZE (innermode) - subreg_lowpart_offset (outermode, innermode) == byte) + is_lowpart) return simplify_gen_unary (TRUNCATE, outermode, XEXP (op, 0), GET_MODE (XEXP (op, 0))); @@ -5343,7 +5348,7 @@ simplify_subreg (enum machine_mode outer The information is used only by alias analysis that can not grog partial register anyway. */ - if (subreg_lowpart_offset (outermode, innermode) == byte) + if (is_lowpart) ORIGINAL_REGNO (x) = ORIGINAL_REGNO (op); return x; } @@ -5393,6 +5398,51 @@ simplify_subreg (enum machine_mode outer return NULL_RTX; } + /* Try to move a subreg inside an arithmetic operation. */ + if (is_lowpart ARITHMETIC_P (op) + GET_MODE_CLASS (outermode) == MODE_INT + GET_MODE_CLASS (innermode) == MODE_INT) +{ + enum insn_code ic; + enum machine_mode cnt_mode; + switch (GET_CODE (op)) + { + case ABS: + case NEG: + return simplify_gen_unary (GET_CODE (op), outermode, + rtl_hooks.gen_lowpart_no_emit + (outermode, XEXP (op, 0)), + outermode); + + case ASHIFT: + /* i386 always uses QImode for the shift count. Get the + appropriate mode from the optab. */ + ic = ashl_optab-handlers[outermode].insn_code; + if (ic != CODE_FOR_nothing) + cnt_mode = insn_data[ic].operand[2].mode; + else + cnt_mode = outermode; + return simplify_gen_binary (GET_CODE (op), outermode, + rtl_hooks.gen_lowpart_no_emit + (outermode, XEXP (op, 0)), + rtl_hooks.gen_lowpart_no_emit + (cnt_mode, XEXP (op, 1))); + + case PLUS: + case MINUS: + case AND: + case IOR: + case XOR: + return simplify_gen_binary (GET_CODE (op), outermode, + rtl_hooks.gen_lowpart_no_emit + (outermode, XEXP (op, 0)), + rtl_hooks.gen_lowpart_no_emit + (outermode, XEXP (op, 1))); + default: + break; + } +} + /* Optimize SUBREG truncations of zero and sign extended values. */ if ((GET_CODE (op) == ZERO_EXTEND || GET_CODE (op) == SIGN_EXTEND) Index: gcc/combine.c === --- gcc/combine.c (branch pr39726) +++ gcc/combine.c (working copy) @@ -11155,47 +11155,6 @@ simplify_comparison (enum rtx_code code, continue; } - /* If this is (and:M1 (subreg:M2 X 0) (const_int C1)) where C1 - fits in both M1 and M2 and the SUBREG is either paradoxical - or represents the low part, permute the SUBREG and the AND - and try again. */ - if (GET_CODE (XEXP (op0, 0)) == SUBREG) - { - unsigned HOST_WIDE_INT c1; - tmode = GET_MODE
Re: [PATCH] Fix ICE in distribute_notes (PR bootstrap/51796)
If you prefer to do it this way, fine, but I think we should have some ENABLE_RTL_CHECKING verification somewhere that the REG_NORETURN vs. REG_ARGS_SIZE ordering is always correct. OK, this is clearly going ouf of proportion. :-) Your latest patch is fine. -- Eric Botcazou
Re: [Patch, Fortran] PR 51800 - Fix -finit-local-zero with automatic arrays and -fno-automatic
On 01/11/2012 06:03 PM, Tobias Burnus wrote: -finit-* creates an initialization for local variables - either as static initializer or by initializing at run time. The latter also works with automatic variables, but was breaking with -fno-automatic, which causes all nonautomatic local variables to be placed in static memory. However, combining -finit-* -fno-automatic with automatic arrays is failing at resolution time. The fix turned out to be rather simple. Good, I wondered how this would work (the reason I thought it would always work with automatic arrays was that it apparently (assembler source and output of trial program) worked for arrays smaller than the limit to place them on the stack. Unfortunately, I forgot to test this against the combination of -finit-* -fno-automatic, which just proves you cannot have too many test cases. I wondered about characters strings where the length is a nonconstant specification question (thus: they are also automatic data objects). It turned out that only the initialization was missing - no code generation (trans*.c) change and no other resolution change were required. The first part fixes a regression as -finit-* -fno-automatic could be combined before (albeit without initializing the automatic arrays - but there was no compile error). Perhaps we can issue a warning. Build and regtested on x86-64-linux. OK for the trunk? Note that I backported this change (noted in PR/51310) to the 4.6 branch, so it's needed there too. Thanks for fixing this ! -- Toon Moene - e-mail: t...@moene.org - phone: +31 346 214290 Saturnushof 14, 3738 XG Maartensdijk, The Netherlands At home: http://moene.org/~toon/; weather: http://moene.org/~hirlam/ Progress of GNU Fortran: http://gcc.gnu.org/wiki/GFortran#news
Re: [Patch, Fortran] PR 51800 - Fix -finit-local-zero with automatic arrays and -fno-automatic
Toon Moene wrote: The first part fixes a regression as -finit-* -fno-automatic could be combined before (albeit without initializing the automatic arrays - but there was no compile error). Perhaps we can issue a warning. Well, with your plus my patch it does initialize automatic arrays and character variables with nonconstant lengths. Thus, there is no need for a warning. Before your patch: No initialization of automatic data objects With your patch: Initialization for automatic arrays, but fails with -fno-automatic. With my patch: Initialization also for nonconst-length character strings, no failures with -fno-automatic. Build and regtested on x86-64-linux. OK for the trunk? Note that I backported this change (noted in PR/51310) to the 4.6 branch, so it's needed there too. Thanks for the reminder! Tobias
Re: [Ping] RE: CR16 Port addition
On 01/11/2012 11:50 PM, Jayant R. Sonar wrote: I don't have commit rights. Please let me know how to proceed. http://sourceware.org/cgi-bin/pdw/ps_form.cgi will take care of the commit rights part. r~
Re: [google][4.6]Add new target builtin to check for amdfam15h processors (issue 5535046)
New patch uploaded with the changes. * i386.c (IX86_BUILTIN_CPU_IS_AMDFAM15H_BDVER1): New enum value. (IX86_BUILTIN_CPU_IS_AMDFAM15H_BDVER2): New enum value. (IX86_BUILTIN_CPU_IS_AMDFAM10H): Rename IX86_BUILTIN_CPU_IS_AMDFAM10. (IX86_BUILTIN_CPU_IS_AMDFAM10H_BARCELONA): Rename IX86_BUILTIN_CPU_IS_AMDFAM10_BARCELONA. (IX86_BUILTIN_CPU_IS_AMDFAM10H_SHANGHAI): Rename IX86_BUILTIN_CPU_IS_AMDFAM10_SHANGHAI. (IX86_BUILTIN_CPU_IS_AMDFAM10H_ISTANBUL): Rename IX86_BUILTIN_CPU_IS_AMDFAM10_ISTANBUL. (fold_builtin_cpu): Process IX86_BUILTIN_CPU_IS_AMDFAM15H_BDVER1 and IX86_BUILTIN_CPU_IS_AMDFAM15H_BDVER2. (M_AMDFAM10H): Rename M_AMDFAM10. (M_AMDFAM10H_BARCELONA): Rename M_AMDFAM10_BARCELONA. (M_AMDFAM10H_SHANGHAI): Rename M_AMDFAM10_SHANGHAI. (M_AMDFAM10H_ISTANBUL): Rename M_AMDFAM10_ISTANBUL. (ix86_init_platform_type_builtins): Make new builtins __builtin_cpu_is_amdfam15_bdver1 and __builtin_cpu_is_amdfam15_bdver2. (ix86_expand_builtin): Expand IX86_BUILTIN_CPU_IS_AMDFAM15_BDVER1 and IX86_BUILTIN_CPU_IS_AMDFAM15_BDVER2. * testsuite/gcc.target/builtin_target.c (fn1): Call new builtins. * i386-cpuinfo.c (__cpu_is_amdfam15h_bdver1): New member in __cpu_model struct. (__cpu_is_amdfam15h_bdver2): New member in __cpu_model struct. (__cpu_model): Rename __cpu_is_amdfam10 to __cpu_is_amdfam10h. Rename __cpu_is_amdfam10_barcelona to __cpu_is_amdfam10h_barcelona. Rename __cpu_is_amdfam10_shanghai to __cpu_is_amdfam10h_shanghai. Rename __cpu_is_amdfam10_istanbul to __cpu_is_amdfam10h_istanbul. (get_amd_cpu): Check for family 15h processors. (cpu_indicator_init): Adjust model and family for AMD processors. Refactor code. Thanks, -Sri. On 2012/01/11 00:05:01, davidxl wrote: http://codereview.appspot.com/5535046/diff/1/gcc/config/i386/i386.c File gcc/config/i386/i386.c (right): http://codereview.appspot.com/5535046/diff/1/gcc/config/i386/i386.c#newcode26032 gcc/config/i386/i386.c:26032: +M_AMDFAM15, Maybe better to change 10 to 10H, and 15 to 15H in the name as the number is hex. Why not split the enum for family 15h into M_AMDFAM15H_BDVER1 and .._BVER2? http://codereview.appspot.com/5535046/diff/1/libgcc/config/i386/i386-cpuinfo.c File libgcc/config/i386/i386-cpuinfo.c (right): http://codereview.appspot.com/5535046/diff/1/libgcc/config/i386/i386-cpuinfo.c#newcode102 libgcc/config/i386/i386-cpuinfo.c:102: break; No family15h model encoding? http://codereview.appspot.com/5535046/
Re: [Patch, Fortran] PR 51800 - Fix -finit-local-zero with automatic arrays and -fno-automatic
On 01/11/2012 09:01 PM, Tobias Burnus wrote: Before your patch: No initialization of automatic data objects With your patch: Initialization for automatic arrays, but fails with -fno-automatic. With my patch: Initialization also for nonconst-length character strings, no failures with -fno-automatic. BTW, does this mean -finit-real=snan would work via an initialization expression at run time after allocation for allocatable arrays (note that this isn't covered, still), i.e., that I could try to fix this in 4.8 in the same way as it works in resolve.c, but then applied to trans-array.c ? Or do I have to apply a different method ? Thanks ! -- Toon Moene - e-mail: t...@moene.org - phone: +31 346 214290 Saturnushof 14, 3738 XG Maartensdijk, The Netherlands At home: http://moene.org/~toon/; weather: http://moene.org/~hirlam/ Progress of GNU Fortran: http://gcc.gnu.org/wiki/GFortran#news
C++ PATCH for c++/51818 (wrong mangling with NSDMI and lambda)
In this testcase we were omitting the mangling scope for a lambda defined in an NSDMI because we weren't using decl_mangling_context where needed. After fixing that, we also needed to fix find_substitution to not treat A as a substitution for foo::bar. Tested x86_64-pc-linux-gnu, applying to trunk since this is an ABI violation that occurs on code that was not previously accepted. commit f055576d031b2521f3ba7c4533556b0651b22479 Author: Jason Merrill ja...@redhat.com Date: Wed Jan 11 14:37:49 2012 -0500 PR c++/51818 * mangle.c (find_substitution): A type is only a substitution match if we're looking for a type. (write_nested_name): Use decl_mangling_context. diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c index f4efa67..60b1870 100644 --- a/gcc/cp/mangle.c +++ b/gcc/cp/mangle.c @@ -615,7 +615,7 @@ find_substitution (tree node) /* NODE is a matched to a candidate if it's the same decl node or if it's the same type. */ if (decl == candidate - || (TYPE_P (candidate) type TYPE_P (type) + || (TYPE_P (candidate) type TYPE_P (node) same_type_p (type, candidate)) || NESTED_TEMPLATE_MATCH (node, candidate)) { @@ -949,7 +949,7 @@ write_nested_name (const tree decl) else { /* No, just use prefix */ - write_prefix (CP_DECL_CONTEXT (decl)); + write_prefix (decl_mangling_context (decl)); write_unqualified_name (decl); } write_char ('E'); diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-mangle3.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-mangle3.C new file mode 100644 index 000..06913a1 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-mangle3.C @@ -0,0 +1,15 @@ +// PR c++/51818 +// { dg-options -std=c++0x } +// { dg-final { scan-assembler _ZN1AC1IN3foo3barMUlvE_EEET_ } } + +struct A +{ + template class T A(T) { } +}; + +struct foo +{ + A bar = []{}; +}; + +foo f;
[Patch][Cilkplus] Array Notation Triplet Implementation
Hello Everyone, This patch is for the C++ Compiler in the Cilkplus branch. It will implement array notation for N-D triplets for modify expressions. Thanking You, Yours Sincerely, Balaji V. Iyer. diff --git a/gcc/cp/ChangeLog.cilk b/gcc/cp/ChangeLog.cilk index 7ac7dde..3ba0406 100644 --- a/gcc/cp/ChangeLog.cilk +++ b/gcc/cp/ChangeLog.cilk @@ -1,3 +1,19 @@ +2012-01-11 Balaji V. Iyer balaji.v.i...@intel.com + + * Make-lang.in: Added cp-array-notation.c file information. + * cp-array-notation.c (replace_array_notations): New function. + (find_rank): Likewise. + (extract_array_notation_exprs): Likewise. + (build_x_array_notation_exprs): Likewise. + * parser.c (cp_parser_array_notation): Likewise. + (cp_parser_nested_name_specifier_opt): Added a check to see if + cilkplus is enabled. + (cp_parser_postfix_open_square_expression): Added a check for CPP_COLON + to handle array notation. + (cp_parser_assignment_expression): Added a check for ARRAY_NOTATION_REF + and if so, then call build_x_array_notation_expr function. + * cp-tree.h: Added prototype for build_x_array_notation_expr. + 2011-11-11 Balaji V. Iyer balaji.v.i...@intel.com * cilk.c (for_local_cb): Changed a const tree cast to (tree *). diff --git a/gcc/cp/Make-lang.in b/gcc/cp/Make-lang.in index ee82235..ec6a347 100644 --- a/gcc/cp/Make-lang.in +++ b/gcc/cp/Make-lang.in @@ -87,7 +87,7 @@ CXX_AND_OBJCXX_OBJS = cp/call.o cp/decl.o cp/expr.o cp/pt.o cp/typeck2.o \ cp/typeck.o cp/cvt.o cp/except.o cp/friend.o cp/init.o cp/method.o \ cp/search.o cp/semantics.o cp/tree.o cp/repo.o cp/dump.o cp/optimize.o \ cp/mangle.o cp/cp-objcp-common.o cp/name-lookup.o cp/cxx-pretty-print.o \ - cp/cp-gimplify.o cp/cilk.o tree-mudflap.o $(CXX_C_OBJS) + cp/cp-gimplify.o cp/cilk.o cp/cp-array-notation.o tree-mudflap.o $(CXX_C_OBJS) # Language-specific object files for C++. CXX_OBJS = cp/cp-lang.o c-family/stub-objc.o $(CXX_AND_OBJCXX_OBJS) @@ -272,6 +272,10 @@ CXX_PRETTY_PRINT_H = cp/cxx-pretty-print.h $(C_PRETTY_PRINT_H) cp/cilk.o: cp/cilk.c cilk.h cp/decl.h $(CXX_TREE_H) $(CONFIG_H) $(SYSTEM_H) \ $(TREE_DUMP_H) +cp/cp-array-notation.o: cp/cp-array-notation.c $(CXX_TREE_H) $(TM_H) \ + $(FLAGS_H) toplev.h $(DIAGNOSTIC_H) convert.h $(C_COMMON_H) $(TARGET_H) \ + output.h c-family/c-objc.h + cp/lex.o: cp/lex.c $(CXX_TREE_H) $(TM_H) $(FLAGS_H) \ $(C_PRAGMA_H) output.h input.h cp/operators.def $(TM_P_H) \ c-family/c-objc.h diff --git a/gcc/cp/cp-array-notation.c b/gcc/cp/cp-array-notation.c new file mode 100644 index 000..de1843a --- /dev/null +++ b/gcc/cp/cp-array-notation.c @@ -0,0 +1,625 @@ +/* This file is part of the Intel(R) Cilk(TM) Plus support + It contains routines to handle Array Notation expression + handling routines in the C++ Compiler. + Copyright (C) 2011, 2012 Free Software Foundation, Inc. + Contributed by Balaji V. Iyer balaji.v.i...@intel.com, + Intel Corporation + + This file is part of GCC. + + GCC is free software; you can redistribute it and/or modify it + under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3, or (at your option) + any later version. + + GCC is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + General Public License for more details. + + You should have received a copy of the GNU General Public License + along with GCC; see the file COPYING3. If not see + http://www.gnu.org/licenses/. */ + +#include config.h +#include system.h +#include coretypes.h +#include tm.h +#include tree.h +#include cp-tree.h +#include c-family/c-common.h +#include c-family/c-objc.h +#include tree-inline.h +#include tree-mudflap.h +#include intl.h +#include toplev.h +#include flags.h +#include output.h +#include timevar.h +#include diagnostic.h +#include cgraph.h +#include tree-iterator.h +#include vec.h +#include target.h +#include gimple.h +#include bitmap.h + + +void replace_array_notations (tree *, tree *, tree *, int); +void find_rank (tree array, int *rank); + +/* This function is to find the rank of an array notation expression. + * For example, an array notation of A[:][:] has a rank of 2. + */ +void +find_rank (tree array, int *rank) +{ + tree ii_tree; + int current_rank = 0, ii = 0; + + if (!array) +return; + else if (TREE_CODE (array) == ARRAY_NOTATION_REF) +{ + for (ii_tree = array; + ii_tree TREE_CODE (ii_tree) == ARRAY_NOTATION_REF; + ii_tree = ARRAY_NOTATION_ARRAY (ii_tree)) + current_rank++; + + if (*rank != 0 *rank != current_rank) + error (Rank Mismatch!); + else if (*rank == 0) + *rank = current_rank; +} + else +{ + if (TREE_CODE (array) == CALL_EXPR + || TREE_CODE (array) == AGGR_INIT_EXPR) +
[PATCH, Ada] Illegal program not detected, self renames, PR15846
Hi, this patch fixes problem when gnat is not able to detect illegal program with self renaming of predefined operation, when renaming operation is defined with selected component of the same package as renaming declaration. (please correct me if I wrong in my explanation) And also this patch fixes ICE when T1 type is tagged record. package renaming6 is type T1 is null record; function = (left, right : in T1) return boolean renames renaming6.=; -- { dg-error subprogram cannot rename itself } end renaming6; Tested on x86_64-pc-linux-gnu. ChangeLog: * gcc/ada/exp_disp.adb (Make_DT): Check if flag Is_Dispatching_Operation is True before getting DT_Position flag , present in function and procedure entities which are dispatching * gcc/ada/sem_ch8.adb (Analyze_Subprogram_Renaming): Added check if renaming entity package is the same as renaming_declaration package, in case if both operations has the same names. * gcc/testsuite/gnat.dg/specs/renamings1.ads: new testcase * gcc/testsuite/gnat.dg/specs/renamings2.ads: new testcase -- Best regards, Alexander Basov Index: gcc/ada/exp_disp.adb === --- gcc/ada/exp_disp.adb(revision 183094) +++ gcc/ada/exp_disp.adb(working copy) @@ -4135,6 +4135,7 @@ Prim := Node (Prim_Elmt); if Present (Interface_Alias (Prim)) +and then Is_Dispatching_Operation (Prim) and then Find_Dispatching_Type (Interface_Alias (Prim)) = Iface then @@ -4247,7 +4248,6 @@ while Present (Prim_Elmt) loop Prim := Node (Prim_Elmt); E:= Ultimate_Alias (Prim); - Prim_Pos := UI_To_Int (DT_Position (E)); -- Do not reference predefined primitives because they are -- located in a separate dispatch table; skip abstract and @@ -4260,7 +4260,8 @@ and then not Is_Abstract_Subprogram (Alias (Prim)) and then not Is_Eliminated (Alias (Prim)) and then (not Is_CPP_Class (Root_Type (Typ)) - or else Prim_Pos CPP_Nb_Prims) + or else UI_To_Int + (DT_Position (E)) CPP_Nb_Prims) and then Find_Dispatching_Type (Interface_Alias (Prim)) = Iface @@ -5764,7 +5765,6 @@ E: Entity_Id; Prim : Entity_Id; Prim_Elmt: Elmt_Id; - Prim_Pos : Nat; Prim_Table : array (Nat range 1 .. Nb_Prim) of Entity_Id; begin @@ -5777,8 +5777,7 @@ -- Retrieve the ultimate alias of the primitive for proper -- handling of renamings and eliminated primitives. - E:= Ultimate_Alias (Prim); - Prim_Pos := UI_To_Int (DT_Position (E)); + E := Ultimate_Alias (Prim); -- Do not reference predefined primitives because they are -- located in a separate dispatch table; skip entities with @@ -5794,7 +5793,8 @@ and then not Is_Abstract_Subprogram (E) and then not Is_Eliminated (E) and then (not Is_CPP_Class (Root_Type (Typ)) - or else Prim_Pos CPP_Nb_Prims) +or else UI_To_Int + (DT_Position (E)) CPP_Nb_Prims) then pragma Assert (UI_To_Int (DT_Position (Prim)) = Nb_Prim); Index: gcc/ada/sem_ch8.adb === --- gcc/ada/sem_ch8.adb (revision 183094) +++ gcc/ada/sem_ch8.adb (working copy) @@ -2662,10 +2662,13 @@ end if; end if; - if not Is_Actual - and then (Old_S = New_S - or else (Nkind (Nam) /= N_Expanded_Name -and then Chars (Old_S) = Chars (New_S))) + if not Is_Actual and then + (Old_S = New_S + or else (Nkind (Nam) /= N_Expanded_Name +and then Chars (Old_S) = Chars (New_S)) + or else (Nkind (Nam) = N_Expanded_Name +and then Scope (New_S) = Entity (Prefix (Nam)) +and then Chars (Old_S) = Chars (New_S))) then Error_Msg_N (subprogram cannot rename itself, N); end if; Index: gcc/testsuite/gnat.dg/specs/renamings1.ads === --- gcc/testsuite/gnat.dg/specs/renamings1.ads (revision 0) +++
Re: [PATCH, Ada] Illegal program not detected, self renames, PR15846
Sorry, fixed patch is attached. 12.01.2012 00:43, Alexander Basov пишет: Hi, this patch fixes problem when gnat is not able to detect illegal program with self renaming of predefined operation, when renaming operation is defined with selected component of the same package as renaming declaration. (please correct me if I wrong in my explanation) And also this patch fixes ICE when T1 type is tagged record. package renaming6 is type T1 is null record; function = (left, right : in T1) return boolean renames renaming6.=; -- { dg-error subprogram cannot rename itself } end renaming6; Tested on x86_64-pc-linux-gnu. ChangeLog: * gcc/ada/exp_disp.adb (Make_DT): Check if flag Is_Dispatching_Operation is True before getting DT_Position flag , present in function and procedure entities which are dispatching * gcc/ada/sem_ch8.adb (Analyze_Subprogram_Renaming): Added check if renaming entity package is the same as renaming_declaration package, in case if both operations has the same names. * gcc/testsuite/gnat.dg/specs/renamings1.ads: new testcase * gcc/testsuite/gnat.dg/specs/renamings2.ads: new testcase -- Best regards, Alexander Basov Index: gcc/ada/exp_disp.adb === --- gcc/ada/exp_disp.adb(revision 183094) +++ gcc/ada/exp_disp.adb(working copy) @@ -4135,6 +4135,7 @@ Prim := Node (Prim_Elmt); if Present (Interface_Alias (Prim)) +and then Is_Dispatching_Operation (Prim) and then Find_Dispatching_Type (Interface_Alias (Prim)) = Iface then @@ -4247,7 +4248,6 @@ while Present (Prim_Elmt) loop Prim := Node (Prim_Elmt); E:= Ultimate_Alias (Prim); - Prim_Pos := UI_To_Int (DT_Position (E)); -- Do not reference predefined primitives because they are -- located in a separate dispatch table; skip abstract and @@ -4260,7 +4260,8 @@ and then not Is_Abstract_Subprogram (Alias (Prim)) and then not Is_Eliminated (Alias (Prim)) and then (not Is_CPP_Class (Root_Type (Typ)) - or else Prim_Pos CPP_Nb_Prims) + or else UI_To_Int + (DT_Position (E)) CPP_Nb_Prims) and then Find_Dispatching_Type (Interface_Alias (Prim)) = Iface @@ -5764,7 +5765,6 @@ E: Entity_Id; Prim : Entity_Id; Prim_Elmt: Elmt_Id; - Prim_Pos : Nat; Prim_Table : array (Nat range 1 .. Nb_Prim) of Entity_Id; begin @@ -5777,8 +5777,7 @@ -- Retrieve the ultimate alias of the primitive for proper -- handling of renamings and eliminated primitives. - E:= Ultimate_Alias (Prim); - Prim_Pos := UI_To_Int (DT_Position (E)); + E := Ultimate_Alias (Prim); -- Do not reference predefined primitives because they are -- located in a separate dispatch table; skip entities with @@ -5794,7 +5793,8 @@ and then not Is_Abstract_Subprogram (E) and then not Is_Eliminated (E) and then (not Is_CPP_Class (Root_Type (Typ)) - or else Prim_Pos CPP_Nb_Prims) +or else UI_To_Int + (DT_Position (E)) CPP_Nb_Prims) then pragma Assert (UI_To_Int (DT_Position (Prim)) = Nb_Prim); Index: gcc/ada/sem_ch8.adb === --- gcc/ada/sem_ch8.adb (revision 183094) +++ gcc/ada/sem_ch8.adb (working copy) @@ -2662,10 +2662,13 @@ end if; end if; - if not Is_Actual - and then (Old_S = New_S - or else (Nkind (Nam) /= N_Expanded_Name -and then Chars (Old_S) = Chars (New_S))) + if not Is_Actual and then + (Old_S = New_S + or else (Nkind (Nam) /= N_Expanded_Name +and then Chars (Old_S) = Chars (New_S)) + or else (Nkind (Nam) = N_Expanded_Name +and then Scope (New_S) = Entity (Prefix (Nam)) +and then Chars (Old_S) = Chars (New_S))) then Error_Msg_N (subprogram cannot rename itself, N); end if; Index: gcc/testsuite/gnat.dg/specs/renamings1.ads ===
Re: [Ping] RE: CR16 Port addition
On Thu, 12 Jan 2012, Richard Henderson wrote: On 01/11/2012 11:50 PM, Jayant R. Sonar wrote: I don't have commit rights. Please let me know how to proceed. http://sourceware.org/cgi-bin/pdw/ps_form.cgi will take care of the commit rights part. I don't see anything matching in copyright.list, which has to be in place first. No blanket assignment for KPIT or KPIT Cummins. Maybe I'm missing something. brgds, H-P
invalid assert in convert_debug_memory_address
The assert is not valid for address spaces that support more than one pointer size, such as the generic space of TPF, mips64, or m32c. 2012-01-11 DJ Delorie d...@redhat.com * cfgexpand.c (convert_debug_memory_address): Allow any valid pointer type, not just the default pointer type. Index: cfgexpand.c === --- cfgexpand.c (revision 183092) +++ cfgexpand.c (working copy) @@ -2490,16 +2490,14 @@ convert_debug_memory_address (enum machi #ifndef POINTERS_EXTEND_UNSIGNED gcc_assert (mode == Pmode || mode == targetm.addr_space.address_mode (as)); gcc_assert (xmode == mode || xmode == VOIDmode); #else rtx temp; - enum machine_mode address_mode = targetm.addr_space.address_mode (as); - enum machine_mode pointer_mode = targetm.addr_space.pointer_mode (as); - gcc_assert (mode == address_mode || mode == pointer_mode); + gcc_assert (targetm.addr_space.valid_pointer_mode (mode, as)); if (GET_MODE (x) == mode || GET_MODE (x) == VOIDmode) return x; if (GET_MODE_PRECISION (mode) GET_MODE_PRECISION (xmode)) x = simplify_gen_subreg (mode, x, xmode,
Go patch committed: Fix comparisons for structs/arrays with padding
If a Go struct or array has any padding, then we can't use memcmp to compare the values, because if a value is allocated on the stack then the padding bytes may not be zeroed out. This patch corrects the Go compiler to check for this possibility, and to use a generated function if it occurs. This required moving the generation of hash/equality functions to after the lowering pass, as otherwise a constant expression used for an array index might not be resolvable. Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu. Committed to mainline. Ian diff -r 569599d42f46 go/expressions.cc --- a/go/expressions.cc Wed Jan 11 08:24:08 2012 -0800 +++ b/go/expressions.cc Wed Jan 11 13:09:30 2012 -0800 @@ -5844,7 +5844,7 @@ // See if we can compare using memcmp. As a heuristic, we use // memcmp rather than field references and comparisons if there are // more than two fields. - if (st-compare_is_identity() st-total_field_count() 2) + if (st-compare_is_identity(gogo) st-total_field_count() 2) return this-lower_compare_to_memcmp(gogo, inserter); Location loc = this-location(); @@ -5919,7 +5919,7 @@ // Call memcmp directly if possible. This may let the middle-end // optimize the call. - if (at-compare_is_identity()) + if (at-compare_is_identity(gogo)) return this-lower_compare_to_memcmp(gogo, inserter); // Call the array comparison function. @@ -12966,10 +12966,10 @@ lower_struct(Gogo*, Type*); Expression* - lower_array(Gogo*, Type*); + lower_array(Type*); Expression* - make_array(Gogo*, Type*, Expression_list*); + make_array(Type*, Expression_list*); Expression* lower_map(Gogo*, Named_object*, Statement_inserter*, Type*); @@ -13036,7 +13036,7 @@ else if (type-struct_type() != NULL) ret = this-lower_struct(gogo, type); else if (type-array_type() != NULL) -ret = this-lower_array(gogo, type); +ret = this-lower_array(type); else if (type-map_type() != NULL) ret = this-lower_map(gogo, function, inserter, type); else @@ -13249,11 +13249,11 @@ // Lower an array composite literal. Expression* -Composite_literal_expression::lower_array(Gogo* gogo, Type* type) +Composite_literal_expression::lower_array(Type* type) { Location location = this-location(); if (this-vals_ == NULL || !this-has_keys_) -return this-make_array(gogo, type, this-vals_); +return this-make_array(type, this-vals_); std::vectorExpression* vals; vals.reserve(this-vals_-size()); @@ -13353,15 +13353,14 @@ for (size_t i = 0; i size; ++i) list-push_back(vals[i]); - return this-make_array(gogo, type, list); + return this-make_array(type, list); } // Actually build the array composite literal. This handles // [...]{...}. Expression* -Composite_literal_expression::make_array(Gogo* gogo, Type* type, - Expression_list* vals) +Composite_literal_expression::make_array(Type* type, Expression_list* vals) { Location location = this-location(); Array_type* at = type-array_type(); @@ -13373,10 +13372,6 @@ Expression* elen = Expression::make_integer(vlen, NULL, location); mpz_clear(vlen); at = Type::make_array_type(at-element_type(), elen); - - // This is after the finalize_methods pass, so run that now. - at-finalize_methods(gogo); - type = at; } if (at-length() != NULL) diff -r 569599d42f46 go/gogo.cc --- a/go/gogo.cc Wed Jan 11 08:24:08 2012 -0800 +++ b/go/gogo.cc Wed Jan 11 13:09:30 2012 -0800 @@ -1149,11 +1149,74 @@ this-specific_type_functions_.push_back(tsf); } +// Look for types which need specific hash or equality functions. + +class Specific_type_functions : public Traverse +{ + public: + Specific_type_functions(Gogo* gogo) +: Traverse(traverse_types), + gogo_(gogo) + { } + + int + type(Type*); + + private: + Gogo* gogo_; +}; + +int +Specific_type_functions::type(Type* t) +{ + Named_object* hash_fn; + Named_object* equal_fn; + switch (t-classification()) +{ +case Type::TYPE_NAMED: + { + if (!t-compare_is_identity(this-gogo_) t-is_comparable()) + t-type_functions(this-gogo_, t-named_type(), NULL, NULL, hash_fn, + equal_fn); + + // If this is a struct type, we don't want to make functions + // for the unnamed struct. + Type* rt = t-named_type()-real_type(); + if (rt-struct_type() == NULL) + { + if (Type::traverse(rt, this) == TRAVERSE_EXIT) + return TRAVERSE_EXIT; + } + else + { + if (rt-struct_type()-traverse_field_types(this) == TRAVERSE_EXIT) + return TRAVERSE_EXIT; + } + + return TRAVERSE_SKIP_COMPONENTS; + } + +case Type::TYPE_STRUCT: +case Type::TYPE_ARRAY: + if (!t-compare_is_identity(this-gogo_) t-is_comparable()) + t-type_functions(this-gogo_, NULL, NULL, NULL, hash_fn, equal_fn); + break; + +default: + break; +} + + return TRAVERSE_CONTINUE; +} + // Write out type specific functions. void Gogo::write_specific_type_functions() {
Re: [Ping] RE: CR16 Port addition
On Wed, 11 Jan 2012, Hans-Peter Nilsson wrote: I don't see anything matching in copyright.list, which has to be in place first. No blanket assignment for KPIT or KPIT Cummins. Maybe I'm missing something. I'd generally understand GCC KPIT Cummins Infosystems Ltd2003-04-15 Transfers changes and enhancements now and in the future. (Name given in copyright.list omitted here) to be such a blanket assignment (with the name at the end being a contact, not indicating a limit to which employees can contribute changes, given the usual wording of entries restricted in some way to particular employees). But you can check with ass...@gnu.org. -- Joseph S. Myers jos...@codesourcery.com
fix ICE caused by profile mismatch (issue5533075)
This patch fixes the ICE when building the histrogram for value profile. It's casued by profile mismatch. Tested with SPEC2000 INT. This is for google branches. 2012-01-11 Rong Xu x...@google.com * gcc/profile.c (compute_value_histograms): ignore the histrogram when the counters not found in gcda file. Index: gcc/profile.c === --- gcc/profile.c (revision 183109) +++ gcc/profile.c (working copy) @@ -790,10 +790,14 @@ gcov_type *histogram_counts[GCOV_N_VALUE_COUNTERS]; gcov_type *act_count[GCOV_N_VALUE_COUNTERS]; gcov_type *aact_count; - bool warned = false; + bool warned[GCOV_N_VALUE_COUNTERS]; + static const char *const ctr_names[] = GCOV_COUNTER_NAMES; for (t = 0; t GCOV_N_VALUE_COUNTERS; t++) -n_histogram_counters[t] = 0; +{ + n_histogram_counters[t] = 0; + warned[t] = 0; +} for (i = 0; i VEC_length (histogram_value, values); i++) { @@ -829,18 +833,17 @@ t = (int) hist-type; aact_count = act_count[t]; + /* If the counter cannot be found in gcda file, skip this + histogram and give a warning. */ if (aact_count == 0) { - /* this can only happen when FDO uses LIPO profiles where - we have HIST_TYPE_INDIR_CALL_TOPN counters in gcda - files. */ - gcc_assert (hist-type == HIST_TYPE_INDIR_CALL); - if (!warned flag_opt_info = OPT_INFO_MIN) -warning (0, cannot find INDIR_CALL counters in func %s., + if (!warned[t] flag_opt_info = OPT_INFO_MIN) +warning (0, cannot find %s counters in function %s., + ctr_names[COUNTER_FOR_HIST_TYPE(t)], IDENTIFIER_POINTER ( DECL_ASSEMBLER_NAME (current_function_decl))); hist-n_counters = 0; - warned = true; + warned[t] = true; continue; } act_count[t] += hist-n_counters; -- This patch is available for review at http://codereview.appspot.com/5533075
[PATCH] PR c++/51633 - ICEs with constexpr constructor
Hello, Consider this short code snippet: struct A { ~A(); }; struct B { A a; constexpr B() {} }; As explained in the audit trail, this is valid code. G++ ICEs on it because build_constexpr_constructor_member_initializers chokes on a CLEANUP_STMT. The CLEANUP_STMT is put into the constructor of B to ensure that the destructor for 'a' is called. As this CLEANUP_STMT is not related to the representation of mem-initializer-list that build_constexpr_constructor_member_initializers was expecting to see as part of its analysis of the constructor body my understanding is that it should just ignore it. I noted that build_data_member_initialization that is called elsewhere in build_constexpr_constructor_member_initializers already knows how to ignore those CLEANUP_STMT, so I tried in this patch to take advantage of that function. The other case of ICE reported in this issue is in: struct A2 { constexpr A2() {} ~A2(); }; struct B2 { A2 a; constexpr B2() { return;} }; where check_constexpr_ctor_body fails to recognize that the body of the constexpr constructor B2 is not empty. It turned out this is because it's not passed the proper last statement of the constructor, before the body (as written in the source code) is parsed. Fixed thus. Bootstrapped and tested on x86_64-unknown-linux-gnu against trunk. gcc/cp/ * semantics.c (cp_parser_ctor_initializer_opt_and_function_body): Set the pointer to the last block of the constructor to the current statement. (build_constexpr_constructor_member_initializers): Get build_data_member_initialization a chance to deal with more statements before we choke. gcc/testsuite/ * g++.dg/cpp0x/constexpr-diag4.C: New test. --- gcc/cp/parser.c |2 +- gcc/cp/semantics.c |2 + gcc/testsuite/g++.dg/cpp0x/constexpr-diag4.C | 28 +++ gcc/testsuite/g++.dg/cpp0x/constexpr-diag5.C | 48 ++ 4 files changed, 79 insertions(+), 1 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-diag4.C create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-diag5.C diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index 0ae55a2..ea9ccfc 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -17418,7 +17418,7 @@ cp_parser_ctor_initializer_opt_and_function_body (cp_parser *parser) cp_parser_function_body changed its state. */ if (check_body_p) { - list = body; + list = cur_stmt_list; if (TREE_CODE (list) == BIND_EXPR) list = BIND_EXPR_BODY (list); if (TREE_CODE (list) == STATEMENT_LIST diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c index fbb74e1..9369179 100644 --- a/gcc/cp/semantics.c +++ b/gcc/cp/semantics.c @@ -5930,6 +5930,8 @@ build_constexpr_constructor_member_initializers (tree type, tree body) break; } } + else if (EXPR_P (body)) +ok = build_data_member_initialization (body, vec); else gcc_assert (errorcount 0); if (ok) diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-diag4.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-diag4.C new file mode 100644 index 000..08373c1 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-diag4.C @@ -0,0 +1,28 @@ +// Origin: PR c++/51633 +// { dg-options -std=c++11 } + +struct A +{ +~A(); +}; + +struct B +{ +A a; +constexpr B() {} +}; + +struct A1 +{ +int a; +~A1(); +}; + +struct B1 +{ +A1 a1; +constexpr B1() {} // { dg-error uninitialized member } +}; + + + diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-diag5.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-diag5.C new file mode 100644 index 000..c0cbfdd --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-diag5.C @@ -0,0 +1,48 @@ +// Origin: PR c++/51633 +// { dg-options -std=c++11 } + +struct A +{ +constexpr A() {} +~A(); +}; + +struct B +{ +A a; +A b; +A c; +constexpr B() {} +}; + +struct C +{ +A a; +constexpr C() {} +}; + +struct D +{ +constexpr D() { return;} // { dg-error does not have empty body } +}; + +struct D1 +{ +A a; +constexpr D1() { return;} // { dg-error does not have empty body } +}; + +struct D2 +{ +A a; +A b; +constexpr D2() { return;} // { dg-error does not have empty body } +}; + +struct D3 +{ +A a; +A b; +A c; +constexpr D3() { return;} // { dg-error does not have empty body } +}; -- 1.7.6.4 -- Dodji
Re: [PATCH] PR c++/51633 - ICEs with constexpr constructor
... watch out trailing blank lines ;) Thanks! Paolo
Re: [Ping] RE: CR16 Port addition
On Wed, 11 Jan 2012, Joseph S. Myers wrote: On Wed, 11 Jan 2012, Hans-Peter Nilsson wrote: I don't see anything matching in copyright.list, which has to be in place first. No blanket assignment for KPIT or KPIT Cummins. Maybe I'm missing something. I'd generally understand GCC KPIT Cummins Infosystems Ltd2003-04-15 Transfers changes and enhancements now and in the future. (Name given in copyright.list omitted here) to be such a blanket assignment (with the name at the end being a contact, not indicating a limit to which employees can contribute changes, given the usual wording of entries restricted in some way to particular employees). But you can check with ass...@gnu.org. From entries earlier in copyright.list for that person (not mr. Sonar), I understood the name you omitted to be *not* a contact but a specific employee. The entries are a bit ambiguous on that point. I guess I misunderstood, then; I certainly have no intention to check further. Welcome and Happy Hacking! brgds, H-P
Re: [PATCH] PR c++/51633 - ICEs with constexpr constructor
Paolo Carlini pcarl...@gmail.com writes: ... watch out trailing blank lines ;) Hopefully fixed below. Thanks for watching. gcc/cp/ * semantics.c (cp_parser_ctor_initializer_opt_and_function_body): Set the pointer to the last block of the constructor to the current statement. (build_constexpr_constructor_member_initializers): Get build_data_member_initialization a chance to deal with more statements before we choke. gcc/testsuite/ * g++.dg/cpp0x/constexpr-diag4.C: New test. --- gcc/cp/parser.c |2 +- gcc/cp/semantics.c |2 + gcc/testsuite/g++.dg/cpp0x/constexpr-diag4.C | 25 + gcc/testsuite/g++.dg/cpp0x/constexpr-diag5.C | 48 ++ 4 files changed, 76 insertions(+), 1 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-diag4.C create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-diag5.C diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index 0ae55a2..ea9ccfc 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -17418,7 +17418,7 @@ cp_parser_ctor_initializer_opt_and_function_body (cp_parser *parser) cp_parser_function_body changed its state. */ if (check_body_p) { - list = body; + list = cur_stmt_list; if (TREE_CODE (list) == BIND_EXPR) list = BIND_EXPR_BODY (list); if (TREE_CODE (list) == STATEMENT_LIST diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c index fbb74e1..9369179 100644 --- a/gcc/cp/semantics.c +++ b/gcc/cp/semantics.c @@ -5930,6 +5930,8 @@ build_constexpr_constructor_member_initializers (tree type, tree body) break; } } + else if (EXPR_P (body)) +ok = build_data_member_initialization (body, vec); else gcc_assert (errorcount 0); if (ok) diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-diag4.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-diag4.C new file mode 100644 index 000..371190e --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-diag4.C @@ -0,0 +1,25 @@ +// Origin: PR c++/51633 +// { dg-options -std=c++11 } + +struct A +{ +~A(); +}; + +struct B +{ +A a; +constexpr B() {} +}; + +struct A1 +{ +int a; +~A1(); +}; + +struct B1 +{ +A1 a1; +constexpr B1() {} // { dg-error uninitialized member } +}; diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-diag5.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-diag5.C new file mode 100644 index 000..c0cbfdd --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-diag5.C @@ -0,0 +1,48 @@ +// Origin: PR c++/51633 +// { dg-options -std=c++11 } + +struct A +{ +constexpr A() {} +~A(); +}; + +struct B +{ +A a; +A b; +A c; +constexpr B() {} +}; + +struct C +{ +A a; +constexpr C() {} +}; + +struct D +{ +constexpr D() { return;} // { dg-error does not have empty body } +}; + +struct D1 +{ +A a; +constexpr D1() { return;} // { dg-error does not have empty body } +}; + +struct D2 +{ +A a; +A b; +constexpr D2() { return;} // { dg-error does not have empty body } +}; + +struct D3 +{ +A a; +A b; +A c; +constexpr D3() { return;} // { dg-error does not have empty body } +}; -- 1.7.6.4 -- Dodji
Go patch committed: Permit converting to string to named []B
The Go language was tweaked to permit converting a string to a slice of the byte or rune type even if the type is given a different name. This patch implements this tweak in the gccgo frontend. Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu. Committed to mainline. Ian diff -r 4a6f27be6981 go/expressions.cc --- a/go/expressions.cc Wed Jan 11 13:19:28 2012 -0800 +++ b/go/expressions.cc Wed Jan 11 15:33:40 2012 -0800 @@ -3382,9 +3382,11 @@ if (type-is_slice_type()) { Type* element_type = type-array_type()-element_type()-forwarded(); - bool is_byte = element_type == Type::lookup_integer_type(uint8); - bool is_int = element_type == Type::lookup_integer_type(int); - if (is_byte || is_int) + bool is_byte = (element_type-integer_type() != NULL + element_type-integer_type()-is_byte()); + bool is_rune = (element_type-integer_type() != NULL + element_type-integer_type()-is_rune()); + if (is_byte || is_rune) { std::string s; if (val-string_constant_value(s)) @@ -3690,8 +3692,7 @@ tree len = a-length_tree(gogo, expr_tree); len = fold_convert_loc(this-location().gcc_location(), integer_type_node, len); - if (e-integer_type()-is_unsigned() - e-integer_type()-bits() == 8) + if (e-integer_type()-is_byte()) { static tree byte_array_to_string_fndecl; ret = Gogo::call_builtin(byte_array_to_string_fndecl, @@ -3706,7 +3707,7 @@ } else { - go_assert(e == Type::lookup_integer_type(int)); + go_assert(e-integer_type()-is_rune()); static tree int_array_to_string_fndecl; ret = Gogo::call_builtin(int_array_to_string_fndecl, this-location(), @@ -3723,8 +3724,7 @@ { Type* e = type-array_type()-element_type()-forwarded(); go_assert(e-integer_type() != NULL); - if (e-integer_type()-is_unsigned() - e-integer_type()-bits() == 8) + if (e-integer_type()-is_byte()) { tree string_to_byte_array_fndecl = NULL_TREE; ret = Gogo::call_builtin(string_to_byte_array_fndecl, @@ -3737,7 +3737,7 @@ } else { - go_assert(e == Type::lookup_integer_type(int)); + go_assert(e-integer_type()-is_rune()); tree string_to_int_array_fndecl = NULL_TREE; ret = Gogo::call_builtin(string_to_int_array_fndecl, this-location(), @@ -8506,19 +8506,19 @@ break; } - Type* e2; if (arg2_type-is_slice_type()) - e2 = arg2_type-array_type()-element_type(); + { + Type* e2 = arg2_type-array_type()-element_type(); + if (!Type::are_identical(e1, e2, true, NULL)) + this-report_error(_(element types must be the same)); + } else if (arg2_type-is_string_type()) - e2 = Type::lookup_integer_type(uint8); + { + if (e1-integer_type() == NULL || !e1-integer_type()-is_byte()) + this-report_error(_(first argument must be []byte)); + } else - { - this-report_error(_(right argument must be a slice or a string)); - break; - } - - if (!Type::are_identical(e1, e2, true, NULL)) - this-report_error(_(element types must be the same)); + this-report_error(_(second argument must be slice or string)); } break; @@ -8542,7 +8542,7 @@ { const Array_type* at = args-front()-type()-array_type(); const Type* e = at-element_type()-forwarded(); - if (e == Type::lookup_integer_type(uint8)) + if (e-integer_type() != NULL e-integer_type()-is_byte()) break; } @@ -9100,7 +9100,8 @@ tree arg2_len; tree element_size; if (arg2-type()-is_string_type() - element_type == Type::lookup_integer_type(uint8)) + element_type-integer_type() != NULL + element_type-integer_type()-is_byte()) { arg2_tree = save_expr(arg2_tree); arg2_val = String_type::bytes_tree(gogo, arg2_tree); diff -r 4a6f27be6981 go/gogo.cc --- a/go/gogo.cc Wed Jan 11 13:19:28 2012 -0800 +++ b/go/gogo.cc Wed Jan 11 15:33:40 2012 -0800 @@ -88,10 +88,12 @@ // to the same Named_object. Named_object* byte_type = this-declare_type(byte, loc); byte_type-set_type_value(uint8_type); + uint8_type-integer_type()-set_is_byte(); // rune is an alias for int. Named_object* rune_type = this-declare_type(rune, loc); rune_type-set_type_value(int_type); + int_type-integer_type()-set_is_rune(); this-add_named_type(Type::make_integer_type(uintptr, true, pointer_size, diff -r 4a6f27be6981 go/types.cc --- a/go/types.cc Wed Jan 11 13:19:28 2012 -0800 +++ b/go/types.cc Wed Jan 11 15:33:40 2012 -0800 @@ -767,7 +767,7 @@ if (lhs-complex_type() != NULL rhs-complex_type() != NULL) return true; - // An integer, or []byte, or []int, may be converted to a string. + // An integer, or []byte, or []rune, may be converted to a string. if (lhs-is_string_type()) { if (rhs-integer_type() != NULL) @@ -776,19 +776,18 @@ { const Type* e = rhs-array_type()-element_type()-forwarded(); if (e-integer_type() != NULL -
Re: [google][4.6]Add new target builtin to check for amdfam15h processors (issue 5535046)
Good for google branches. Please re-submit the combined runtime support patch to trunk for review. thanks, David On Wed, Jan 11, 2012 at 12:06 PM, tmsri...@google.com wrote: New patch uploaded with the changes. * i386.c (IX86_BUILTIN_CPU_IS_AMDFAM15H_BDVER1): New enum value. (IX86_BUILTIN_CPU_IS_AMDFAM15H_BDVER2): New enum value. (IX86_BUILTIN_CPU_IS_AMDFAM10H): Rename IX86_BUILTIN_CPU_IS_AMDFAM10. (IX86_BUILTIN_CPU_IS_AMDFAM10H_BARCELONA): Rename IX86_BUILTIN_CPU_IS_AMDFAM10_BARCELONA. (IX86_BUILTIN_CPU_IS_AMDFAM10H_SHANGHAI): Rename IX86_BUILTIN_CPU_IS_AMDFAM10_SHANGHAI. (IX86_BUILTIN_CPU_IS_AMDFAM10H_ISTANBUL): Rename IX86_BUILTIN_CPU_IS_AMDFAM10_ISTANBUL. (fold_builtin_cpu): Process IX86_BUILTIN_CPU_IS_AMDFAM15H_BDVER1 and IX86_BUILTIN_CPU_IS_AMDFAM15H_BDVER2. (M_AMDFAM10H): Rename M_AMDFAM10. (M_AMDFAM10H_BARCELONA): Rename M_AMDFAM10_BARCELONA. (M_AMDFAM10H_SHANGHAI): Rename M_AMDFAM10_SHANGHAI. (M_AMDFAM10H_ISTANBUL): Rename M_AMDFAM10_ISTANBUL. (ix86_init_platform_type_builtins): Make new builtins __builtin_cpu_is_amdfam15_bdver1 and __builtin_cpu_is_amdfam15_bdver2. (ix86_expand_builtin): Expand IX86_BUILTIN_CPU_IS_AMDFAM15_BDVER1 and IX86_BUILTIN_CPU_IS_AMDFAM15_BDVER2. * testsuite/gcc.target/builtin_target.c (fn1): Call new builtins. * i386-cpuinfo.c (__cpu_is_amdfam15h_bdver1): New member in __cpu_model struct. (__cpu_is_amdfam15h_bdver2): New member in __cpu_model struct. (__cpu_model): Rename __cpu_is_amdfam10 to __cpu_is_amdfam10h. Rename __cpu_is_amdfam10_barcelona to __cpu_is_amdfam10h_barcelona. Rename __cpu_is_amdfam10_shanghai to __cpu_is_amdfam10h_shanghai. Rename __cpu_is_amdfam10_istanbul to __cpu_is_amdfam10h_istanbul. (get_amd_cpu): Check for family 15h processors. (cpu_indicator_init): Adjust model and family for AMD processors. Refactor code. Thanks, -Sri. On 2012/01/11 00:05:01, davidxl wrote: http://codereview.appspot.com/5535046/diff/1/gcc/config/i386/i386.c File gcc/config/i386/i386.c (right): http://codereview.appspot.com/5535046/diff/1/gcc/config/i386/i386.c#newcode26032 gcc/config/i386/i386.c:26032: + M_AMDFAM15, Maybe better to change 10 to 10H, and 15 to 15H in the name as the number is hex. Why not split the enum for family 15h into M_AMDFAM15H_BDVER1 and .._BVER2? http://codereview.appspot.com/5535046/diff/1/libgcc/config/i386/i386-cpuinfo.c File libgcc/config/i386/i386-cpuinfo.c (right): http://codereview.appspot.com/5535046/diff/1/libgcc/config/i386/i386-cpuinfo.c#newcode102 libgcc/config/i386/i386-cpuinfo.c:102: break; No family15h model encoding? http://codereview.appspot.com/5535046/
[wwwdocs] gcc-4.7/porting_to.html
As requested by Jakub. I thought it better to get this in, warts and all, and have it be corrected than to dally around again and have it not checked in. -benjamin2012-01-11 Benjamin Kosnik b...@redhat.com * htdocs/gcc-4.7/porting_to.html: Add. Index: htdocs/gcc-4.7/porting_to.html === RCS file: htdocs/gcc-4.7/porting_to.html diff -N htdocs/gcc-4.7/porting_to.html *** /dev/null 1 Jan 1970 00:00:00 - --- htdocs/gcc-4.7/porting_to.html 12 Jan 2012 00:06:17 - *** *** 0 --- 1,266 + html + + head + titlePorting to GCC 4.7/title + /head + + body + h1Porting to GCC 4.7/h1 + + p + The GCC 4.7 release series differs from previous GCC releases in more + than the usual list of + a href=http://gcc.gnu.org/gcc-4.7/changes.html;changes/a. Some of + these are a result of bug fixing, and some old behaviors have been + intentionally changed in order to support new standards, or relaxed + in standards-conforming ways to facilitate compilation or runtime + performance. Some of these changes are not visible to the naked eye + and will not cause problems when updating from older versions. + /p + + p + However, some of these changes are visible, and can cause grief to + users porting to GCC 4.7. This document is an effort to identify major + issues and provide clear solutions in a quick and easily searched + manner. Additions and suggestions for improvement are welcome. + /p + + h2General issues/h2 + + h3Use of invalid flags when linking/h3 + + p + Earlier releases did not warn or error about completely invalid + options on gcc/g++/gfortran etc. command lines, if nothing was + compiled, but only linking was performed. This is no longer the + case. For example, + /p + + pre + gcc -Wl -o foo foo.o -mflat_namespace + /pre + + p + Now produces the following error + /p + + pre + error: unrecognized command line option â-Wlâ + error: unrecognized command line option â-mflat_namespaceâ + /pre + + p + Invalid options need to be removed from the command line or replaced + by something that is valid. + /p + + h2C language issues/h2 + + h3Boolean type promotion changes/h3 + + p + The C compiler no longer promotes boolean values in arithmetic + statements to integer values. Configure-related code that checks for + C99's lt;stdbool.hgt; may be impacted. If the following line is + newly present in configure logs, then lt;stdbool.hgt; support is + incorrectly configured. + /p + + pre + checking for stdbool.h that conforms to C99... no + /pre + + + h2C++ language issues/h2 + + h3Header dependency changes/h3 + + p + Many of the standard C++ library include files have been edited to no + longer include lt;unistd.hgt; to remove a href=http://gcc.gnu.org/PR49745;namespace pollution/a. + /p + + p + As such, C++ programs that used functions + including codetruncate/code, codesleep/code + or codepipe/code without first including lt;unistd.hgt; will no + longer compile. The diagnostic produced is similar to: + /p + + pre + error: âtruncateâ was not declared in this scope + /pre + + pre + error: âsleepâ was not declared in this scope + /pre + + pre + error: âpipeâ was not declared in this scope + /pre + + pre + error: there are no arguments to 'offsetof' that depend on a template + parameter, so a declaration of 'offsetof' must be available + /pre + + p + Fixing this issue is easy: just include lt;unistd.hgt;. + /p + + h3Note on proper checking for thread support/h3 + + p + At no time, should user-level code use private + GCC-implementation-space macros such as + code_GLIBCXX_HAS_GTHREADS/code to determine at compile-time + concurrency support. Instead, use the POSIX + macro code_REENTRANT/code. + /p + + h3Name lookup changes/h3 + + p + The C++ compiler no longer performs an extra unqualified lookups that + had performed in the past, namely a href=http://gcc.gnu.org/PR24163;dependant base class scope lookups/a and + a href=http://gcc.gnu.org/PR29131;unqualified template function/a + lookups. + /p + + p + C++ programs that depended on the compiler's previous behavior may + longer compile. For example, code such as + /p + + pre + templatetypename T + int t(T i) + { return f(i); } + + int + f(int i) + { return i; } + + int + main() + { + return t(1); + } + /pre + + + p + Will result in the following diagnostic: + /p + pre + In instantiation of âint t(T) [with T = int]â: + required from here + error: âfâ was not declared in this scope, and no declarations were found by argument-dependent lookup at the point of instantiation [-fpermissive] + note: âint f(int)â declared here, later in the translation unit + /pre + + p + To fix, make sure the function codef/code in the code above is + declared before first use in function codet/code. Like so: + /p + + pre + int + f(int i) + { return i; } + + templatetypename T + int t(T i) + { return f(i); } + + int + main() + { +
Re: [google][4.6]Add new target builtin to check for amdfam15h processors (issue 5535046)
Submitted to google 4.6 branch. On Wed, Jan 11, 2012 at 3:51 PM, Xinliang David Li davi...@google.com wrote: Good for google branches. Please re-submit the combined runtime support patch to trunk for review. Will do. Thanks, -Sri. thanks, David On Wed, Jan 11, 2012 at 12:06 PM, tmsri...@google.com wrote: New patch uploaded with the changes. * i386.c (IX86_BUILTIN_CPU_IS_AMDFAM15H_BDVER1): New enum value. (IX86_BUILTIN_CPU_IS_AMDFAM15H_BDVER2): New enum value. (IX86_BUILTIN_CPU_IS_AMDFAM10H): Rename IX86_BUILTIN_CPU_IS_AMDFAM10. (IX86_BUILTIN_CPU_IS_AMDFAM10H_BARCELONA): Rename IX86_BUILTIN_CPU_IS_AMDFAM10_BARCELONA. (IX86_BUILTIN_CPU_IS_AMDFAM10H_SHANGHAI): Rename IX86_BUILTIN_CPU_IS_AMDFAM10_SHANGHAI. (IX86_BUILTIN_CPU_IS_AMDFAM10H_ISTANBUL): Rename IX86_BUILTIN_CPU_IS_AMDFAM10_ISTANBUL. (fold_builtin_cpu): Process IX86_BUILTIN_CPU_IS_AMDFAM15H_BDVER1 and IX86_BUILTIN_CPU_IS_AMDFAM15H_BDVER2. (M_AMDFAM10H): Rename M_AMDFAM10. (M_AMDFAM10H_BARCELONA): Rename M_AMDFAM10_BARCELONA. (M_AMDFAM10H_SHANGHAI): Rename M_AMDFAM10_SHANGHAI. (M_AMDFAM10H_ISTANBUL): Rename M_AMDFAM10_ISTANBUL. (ix86_init_platform_type_builtins): Make new builtins __builtin_cpu_is_amdfam15_bdver1 and __builtin_cpu_is_amdfam15_bdver2. (ix86_expand_builtin): Expand IX86_BUILTIN_CPU_IS_AMDFAM15_BDVER1 and IX86_BUILTIN_CPU_IS_AMDFAM15_BDVER2. * testsuite/gcc.target/builtin_target.c (fn1): Call new builtins. * i386-cpuinfo.c (__cpu_is_amdfam15h_bdver1): New member in __cpu_model struct. (__cpu_is_amdfam15h_bdver2): New member in __cpu_model struct. (__cpu_model): Rename __cpu_is_amdfam10 to __cpu_is_amdfam10h. Rename __cpu_is_amdfam10_barcelona to __cpu_is_amdfam10h_barcelona. Rename __cpu_is_amdfam10_shanghai to __cpu_is_amdfam10h_shanghai. Rename __cpu_is_amdfam10_istanbul to __cpu_is_amdfam10h_istanbul. (get_amd_cpu): Check for family 15h processors. (cpu_indicator_init): Adjust model and family for AMD processors. Refactor code. Thanks, -Sri. On 2012/01/11 00:05:01, davidxl wrote: http://codereview.appspot.com/5535046/diff/1/gcc/config/i386/i386.c File gcc/config/i386/i386.c (right): http://codereview.appspot.com/5535046/diff/1/gcc/config/i386/i386.c#newcode26032 gcc/config/i386/i386.c:26032: + M_AMDFAM15, Maybe better to change 10 to 10H, and 15 to 15H in the name as the number is hex. Why not split the enum for family 15h into M_AMDFAM15H_BDVER1 and .._BVER2? http://codereview.appspot.com/5535046/diff/1/libgcc/config/i386/i386-cpuinfo.c File libgcc/config/i386/i386-cpuinfo.c (right): http://codereview.appspot.com/5535046/diff/1/libgcc/config/i386/i386-cpuinfo.c#newcode102 libgcc/config/i386/i386-cpuinfo.c:102: break; No family15h model encoding? http://codereview.appspot.com/5535046/
Re: [wwwdocs] gcc-4.7/porting_to.html
validation fixups... -benjamin2012-01-11 Benjamin Kosnik b...@redhat.com * htdocs/gcc-4.7/porting_to.html: Fixup for validation. Index: htdocs/gcc-4.7/porting_to.html === RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.7/porting_to.html,v retrieving revision 1.1 diff -c -p -r1.1 porting_to.html *** htdocs/gcc-4.7/porting_to.html 12 Jan 2012 00:08:30 - 1.1 --- htdocs/gcc-4.7/porting_to.html 12 Jan 2012 01:09:24 - *** longer compile. For example, code such a *** 133,139 /p pre ! templatetypename T int t(T i) { return f(i); } --- 133,139 /p pre ! templatelt;typename Tgt; int t(T i) { return f(i); } *** int *** 169,175 f(int i) { return i; } ! templatetypename T int t(T i) { return f(i); } --- 169,175 f(int i) { return i; } ! templatelt;typename Tgt; int t(T i) { return f(i); }
Go patch committed: Initialize void_list_node earlier
I noticed that the function decls for builtin functions were all being marked as varargs. This perplexing problem turned out to be due to void_list_node not being initialized when the builtin functions were declared. Why the frontend needs to initialize void_list_node is beyond me, but I'm not going to change that today. This patch fixes the problem. Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu. Committed to mainline. Ian 2012-01-11 Ian Lance Taylor i...@google.com * go-lang.c (go_langhook_init): Initialize void_list_node before calling go_create_gogo. Index: go-lang.c === --- go-lang.c (revision 183096) +++ go-lang.c (working copy) @@ -88,6 +88,9 @@ go_langhook_init (void) { build_common_tree_nodes (false, false); + /* I don't know why this has to be done explicitly. */ + void_list_node = build_tree_list (NULL_TREE, void_type_node); + /* We must create the gogo IR after calling build_common_tree_nodes (because Gogo::define_builtin_function_trees refers indirectly to, e.g., unsigned_char_type_node) but before calling @@ -97,9 +100,6 @@ go_langhook_init (void) build_common_builtin_nodes (); - /* I don't know why this is not done by any of the above. */ - void_list_node = build_tree_list (NULL_TREE, void_type_node); - /* The default precision for floating point numbers. This is used for floating point constants with abstract type. This may eventually be controllable by a command line option. */
libgo patch committed: Update to weekly.2011-12-14
I have committed a patch to update libgo to the weekly.2011-12-14 release. As usual I am only including the changes to files that are specific to the gccgo frontend. In this case the math package has changed somewhat and I've adapted gccgo to call the libc math functions in several cases. Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu. Committed to mainline. Ian Index: go-lang.c === --- go-lang.c (revision 183096) +++ go-lang.c (working copy) @@ -88,6 +88,9 @@ go_langhook_init (void) { build_common_tree_nodes (false, false); + /* I don't know why this has to be done explicitly. */ + void_list_node = build_tree_list (NULL_TREE, void_type_node); + /* We must create the gogo IR after calling build_common_tree_nodes (because Gogo::define_builtin_function_trees refers indirectly to, e.g., unsigned_char_type_node) but before calling @@ -97,9 +100,6 @@ go_langhook_init (void) build_common_builtin_nodes (); - /* I don't know why this is not done by any of the above. */ - void_list_node = build_tree_list (NULL_TREE, void_type_node); - /* The default precision for floating point numbers. This is used for floating point constants with abstract type. This may eventually be controllable by a command line option. */
C++ PATCH for c++/51565 (ICE with invalid pmf array)
A function type with the x86 fastcall attribute is different from one without it, but standard_conversion was ignoring attributes. Changed to use same_type_p on the static function type rather than trying to duplicate the logic here. Tested x86_64-pc-linux-gnu, applying to trunk. commit 9a052e2c5427f20378a2c683422c701b87532ad9 Author: Jason Merrill ja...@redhat.com Date: Wed Jan 11 17:41:56 2012 -0500 PR c++/51565 * call.c (standard_conversion): For ptrmemfuncs, compare the static_fn_types. diff --git a/gcc/cp/call.c b/gcc/cp/call.c index 57b1765..f7cfbd0 100644 --- a/gcc/cp/call.c +++ b/gcc/cp/call.c @@ -1274,10 +1274,8 @@ standard_conversion (tree to, tree from, tree expr, bool c_cast_p, tree tbase = class_of_this_parm (tofn); if (!DERIVED_FROM_P (fbase, tbase) - || !same_type_p (TREE_TYPE (fromfn), TREE_TYPE (tofn)) - || !compparms (TREE_CHAIN (TYPE_ARG_TYPES (fromfn)), - TREE_CHAIN (TYPE_ARG_TYPES (tofn))) - || cp_type_quals (fbase) != cp_type_quals (tbase)) + || !same_type_p (static_fn_type (fromfn), + static_fn_type (tofn))) return NULL; from = build_memfn_type (fromfn, tbase, cp_type_quals (tbase)); diff --git a/gcc/testsuite/g++.dg/ext/attrib42.C b/gcc/testsuite/g++.dg/ext/attrib42.C new file mode 100644 index 000..39b8e22 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attrib42.C @@ -0,0 +1,12 @@ +// { dg-do compile { target i?86-*-* } } + +struct A { + __attribute__((fastcall)) + void f(); +}; + +int main() +{ +typedef void (A::*FP)(); +FP fp[] = {A::f}; // { dg-error cannot convert } +}
Re: [wwwdocs] gcc-4.7/porting_to.html
validation fixups... More of them -benjamin2012-01-11 Benjamin Kosnik b...@redhat.com * htdocs/gcc-4.7/porting_to.html: Fixup for validation. Index: htdocs/gcc-4.7/porting_to.html === RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.7/porting_to.html,v retrieving revision 1.2 diff -c -r1.2 porting_to.html *** htdocs/gcc-4.7/porting_to.html 12 Jan 2012 01:11:40 - 1.2 --- htdocs/gcc-4.7/porting_to.html 12 Jan 2012 02:51:05 - *** *** 45,52 /p pre ! error: unrecognized command line option â-Wlâ ! error: unrecognized command line option â-mflat_namespaceâ /pre p --- 45,52 /p pre ! error: unrecognized command line option lsquo;-Wlrsquo; ! error: unrecognized command line option lsquo;-mflat_namespacersquo; /pre p *** *** 88,102 /p pre ! error: âtruncateâ was not declared in this scope /pre pre ! error: âsleepâ was not declared in this scope /pre pre ! error: âpipeâ was not declared in this scope /pre pre --- 88,102 /p pre ! error: lsquo;truncatersquo; was not declared in this scope /pre pre ! error: lsquo;sleeprsquo; was not declared in this scope /pre pre ! error: lsquo;pipersquo; was not declared in this scope /pre pre *** *** 153,162 Will result in the following diagnostic: /p pre ! In instantiation of âint t(T) [with T = int]â: required from here ! error: âfâ was not declared in this scope, and no declarations were found by argument-dependent lookup at the point of instantiation [-fpermissive] ! note: âint f(int)â declared here, later in the translation unit /pre p --- 153,162 Will result in the following diagnostic: /p pre ! In instantiation of lsquo;int t(T) [with T = int]rsquo; required from here ! error: lsquo;frsquo; was not declared in this scope, and no declarations were found by argument-dependent lookup at the point of instantiation [-fpermissive] ! note: lsquo;int f(int)rsquo; declared here, later in the translation unit /pre p *** *** 209,216 p pre ! error: redeclaration of âint iâ ! error: âint iâ previously declared here /pre p --- 209,216 p pre ! error: redeclaration of lsquo;int irsquo; ! error: lsquo;int irsquo; previously declared here /pre p *** *** 221,244 h3User-defined literals and whitespace/h3 p ! The C++ compiler in ISO C11 mode -std={c++11,c++0x,gnu++11,gnu++0x} supports user defined literals, which are incompatible with some valid ISO C++03 code. /p p ! In particular, whitespace is now needed after a string literal and before something that could be a valid user defined literal. Take the valid ISO C++03 code /p pre ! const char *p = foobar__TIME__; /pre ! pIn C++03, the code__TIME__/code macro expands to some string literal and is concatenated with the other one. In C++11 code__TIME__/code isn't expanded, instead operator code__TIME__/code is being looked up, resulting in the following diagnostic: /p pre ! error: unable to find string literal operator âoperator __TIME__â /pre p --- 221,250 h3User-defined literals and whitespace/h3 p ! The C++ compiler in ISO C11 mode codestd={c++11,c++0x,gnu++11,gnu++0x}/code supports user defined literals, which are incompatible with some valid ISO C++03 code. /p p ! In particular, whitespace is now needed after a string literal and ! before something that could be a valid user defined literal. Take the ! valid ISO C++03 code /p pre ! const char *p = ldquo;foobarrdquo;__TIME__; /pre ! pIn C++03, the code__TIME__/code macro expands to some string ! literal and is concatenated with the other one. In ! C++11 code__TIME__/code isn't expanded, instead operator ! code__TIME__/code is being looked up, resulting in the ! following diagnostic: /p pre ! error: unable to find string literal operator lsquo;operatorldquo;rdquo; __TIME__rsquo; /pre p
Re: Commit: RX: Add return pattern
On 01/11/2012 10:39 PM, Nick Clifton wrote: +(define_expand return + [(return)] + + rx_expand_epilogue (false); DONE; +) Not an ideal solution, since the availability of this pattern implies it's extremely cheap, and we'll replace jumps to the epilogue with this pattern. A hack-around might be (define_expand return [(return)] false { gcc_unreachable (); }) Or to define an availability predicate similar to i386, testing if the epilogue is trivial, and only a return insn is needed. r~
RE: [RFC] Use REG_EXPR in back-end (introduced by optimization to conditional and/or in ARM back-end)
-Original Message- From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- ow...@gcc.gnu.org] On Behalf Of Jiangning Liu Sent: Wednesday, December 21, 2011 2:48 PM To: 'Richard Henderson' Cc: gcc-patches@gcc.gnu.org; 'Richard Guenther' Subject: RE: [RFC] Use REG_EXPR in back-end (introduced by optimization to conditional and/or in ARM back-end) -Original Message- From: Richard Henderson [mailto:r...@redhat.com] Sent: Tuesday, November 22, 2011 9:55 AM To: Jiangning Liu Cc: gcc-patches@gcc.gnu.org; 'Richard Guenther' Subject: Re: [RFC] Use REG_EXPR in back-end (introduced by optimization to conditional and/or in ARM back-end) On 11/21/2011 05:31 PM, Jiangning Liu wrote: My question is essentially is May I really use REG_EXPR in back- end code? like the patch I gave below? I suppose. Another alternative is to use BImode for booleans. Dunno how much of that you'd be able to gleen from mere rtl expansion or if you'd need boolean decls to be expanded with BImode. Hi Richard, The output of expand pass requires the operands must meet the requirement of general_operand for binary operations, i.e. all RTX operations on top level must meet the hardware instruction requirement. Generally for a hardware not directly supporting a single bit logic operation, we can't generate BI mode rtx dest. So if I simply generate BImode for NE in expand pass, like subreg:SI (ne:BI (reg:SI xxx) (const_int 0))), there would be an assertion failure due to failing to find an appropriate instruction code to operate on a single bit. This looks like a GCC design level issue. How would you suggest generating a legitimate rtx expression containing BImode? Can anybody give me useful suggestions on this issue? My question essentially is may I really use BImode to solve my original problem? Thanks, -Jiangning Thanks, -Jiangning The later would probably need a target hook. I've often wondered how much ia64 would benefit from that too, being able to store bool variables directly in predicate registers. All of this almost certainly must wait until stage1 opens up again though... r~