Re: [rtl, i386] vec_merge simplification
On Wed, 27 Mar 2013, Eric Botcazou wrote: int is getting small to store one bit per vector element (V32QI...) so I switched to hwint after checking that Zadeck's patches don't touch this. unsigned HOST_WIDE_INT is indeed the correct type to use for mask manipulation but please use UINTVAL instead of INTVAL with it. And: + unsigned HOST_WIDE_INT mask = (HOST_WIDE_INT_1 << n_elts) - 1; can be flagged as relying on undefined behavior (we fixed a bunch of cases a couple of years ago) so use: unsigned HOST_WIDE_INT mask = ((unsigned HOST_WIDE_INT) 1 << n_elts) - 1; By the way, shouldn't this be: unsigned HOST_WIDE_INT mask = ((unsigned HOST_WIDE_INT) 2 << (n_elts - 1)) - 1; so it doesn't cause undefined behavior for V64QI? -- Marc Glisse
Re: [PATCH, committed] Fix PR 45472
On 27.02.2013 13:03, Andrey Belevantsev wrote:
Hello,
For this volatile-related issue (no volatile bits on volatile fields of a
non-volatile struct) AFAIU there is an agreement of fixing the front-ends
if needed, but anyways the patch to the selective scheduler is required
that properly merges expressions so that the may_trap_p bit is preserved.
So the following patch was successfully tested on ia64 and x86-64, approved
by Alexander offline, committed to trunk. The patch needs backport to
other branches in about two weeks.
Andrey
PR middle-end/45472
gcc/
* sel-sched-ir.c (merge_expr): Also change vinsn of merged expr
when the may_trap_p bit of the exprs being merged differs.
Reorder tests for speculativeness in the logical and operator.
testsuite/
* gcc.dg/45472.c: New test.
Now backported to 4.7 and 4.6 with Jakub's patch for the sel-sched-ir.c
memory leak added.
Andrey
Index: gcc/ChangeLog
===
*** gcc/ChangeLog (revision 197298)
--- gcc/ChangeLog (revision 197299)
***
*** 1,6
--- 1,25
2013-04-01 Andrey Belevantsev
Backport from mainline
+ 2013-02-27 Andrey Belevantsev
+
+ PR middle-end/45472
+
+ * sel-sched-ir.c (merge_expr): Also change vinsn of merged expr
+ when the may_trap_p bit of the exprs being merged differs.
+ Reorder tests for speculativeness in the logical and operator.
+
+ Backport from mainline
+2013-03-05 Jakub Jelinek
+
+ PR middle-end/56461
+ * sel-sched-ir.c (free_sched_pools): Release
+ succs_info_pool.stack[succs_info_pool.max_top] vectors too
+ if succs_info_pool.max_top isn't -1.
+
+ 2013-04-01 Andrey Belevantsev
+
+ Backport from mainline
2012-02-19 Andrey Belevantsev
PR middle-end/55889
Index: gcc/testsuite/gcc.dg/pr45472.c
===
*** gcc/testsuite/gcc.dg/pr45472.c (revision 0)
--- gcc/testsuite/gcc.dg/pr45472.c (revision 197299)
***
*** 0
--- 1,63
+ /* { dg-do compile { target powerpc*-*-* ia64-*-* x86_64-*-* } } */
+ /* { dg-options "-O -fschedule-insns2 -fselective-scheduling2" } */
+
+ struct S
+ {
+ volatile long vl;
+ int i;
+ };
+ struct S s1, s2;
+
+ void
+ foo (int j, int c)
+ {
+ int i;
+ for (i = 0; i <= j; i++)
+ {
+ if (c)
+ s2.vl += s1.vl;
+ s1 = s2;
+ }
+ }
+ /* { dg-do compile { target powerpc*-*-* ia64-*-* x86_64-*-* } } */
+ /* { dg-options "-O -fschedule-insns2 -fselective-scheduling2" } */
+
+ struct S
+ {
+ volatile long vl;
+ int i;
+ };
+ struct S s1, s2;
+
+ void
+ foo (int j, int c)
+ {
+ int i;
+ for (i = 0; i <= j; i++)
+ {
+ if (c)
+ s2.vl += s1.vl;
+ s1 = s2;
+ }
+ }
+ /* { dg-do compile { target powerpc*-*-* ia64-*-* x86_64-*-* } } */
+ /* { dg-options "-O -fschedule-insns2 -fselective-scheduling2" } */
+
+ struct S
+ {
+ volatile long vl;
+ int i;
+ };
+ struct S s1, s2;
+
+ void
+ foo (int j, int c)
+ {
+ int i;
+ for (i = 0; i <= j; i++)
+ {
+ if (c)
+ s2.vl += s1.vl;
+ s1 = s2;
+ }
+ }
Index: gcc/testsuite/ChangeLog
===
*** gcc/testsuite/ChangeLog (revision 197298)
--- gcc/testsuite/ChangeLog (revision 197299)
***
*** 1,3
--- 1,11
+ 2013-04-01 Andrey Belevantsev
+
+ Backport from mainline
+ 2013-02-27 Andrey Belevantsev
+
+ PR middle-end/45472
+ * gcc.dg/pr45472.c: New test.
+
2013-03-26 Richard Biener
Backport from mainline
Index: gcc/sel-sched-ir.c
===
*** gcc/sel-sched-ir.c (revision 197298)
--- gcc/sel-sched-ir.c (revision 197299)
*** merge_expr (expr_t to, expr_t from, insn
*** 1862,1869
/* Make sure that speculative pattern is propagated into exprs that
have non-speculative one. This will provide us with consistent
speculative bits and speculative patterns inside expr. */
! if (EXPR_SPEC_DONE_DS (to) == 0
! && EXPR_SPEC_DONE_DS (from) != 0)
change_vinsn_in_expr (to, EXPR_VINSN (from));
merge_expr_data (to, from, split_point);
--- 1862,1873
/* Make sure that speculative pattern is propagated into exprs that
have non-speculative one. This will provide us with consistent
speculative bits and speculative patterns inside expr. */
! if ((EXPR_SPEC_DONE_DS (from) != 0
!&& EXPR_SPEC_DONE_DS (to) == 0)
! /* Do likewise for volatile insns, so that we always retain
!the may_trap_p bit on the resulting expression. */
! || (VINSN_MAY_TRAP_P (EXPR_VINSN (from))
! && !VINSN_MAY_TRAP_P (EXPR_VINSN (to
change_v
Re: Fix PR 56077
On 22.02.2013 17:30, Andrey Belevantsev wrote: Hello, As found by Jakub and explained in the PR audit trail by Alexander, this patch fixes the selective scheduler merge glitch of 2008 that added the unnecessary JUMP_P check to the flush_pending_lists call. I have removed the check and expanded the binary negation for clarity. The patch was tested on x86-64, ia64, and ppc64 to be safe. The patch should be conservatively safe at this stage as it adds more flushes and thus more dependencies to the scheduler. The original test is fixed, but I don't know how to add the test checking assembly insns order to the testsuite. OK for trunk? Andrey 2012-02-22 Alexander Monakov Andrey Belevantsev PR middle-end/56077 * sched-deps.c (sched_analyze_insn): When reg_pending_barrier, flush pending lists also on non-jumps. Now backported to 4.7 and 4.6. Andrey Index: gcc/ChangeLog === *** gcc/ChangeLog (revision 197296) --- gcc/ChangeLog (revision 197297) *** *** 1,3 --- 1,13 + 2013-04-01 Andrey Belevantsev + + Backport from mainline + 2013-02-25 Andrey Belevantsev + Alexander Monakov + + PR middle-end/56077 + * sched-deps.c (sched_analyze_insn): When reg_pending_barrier, + flush pending lists also on non-jumps. Adjust comment. + 2013-03-30 Gerald Pfeifer * doc/invoke.texi (AVR Options): Tweak link for AVR-LibC user manual. Index: gcc/sched-deps.c === *** gcc/sched-deps.c(revision 197296) --- gcc/sched-deps.c(revision 197297) *** sched_analyze_insn (struct deps_desc *de *** 3262,3270 SET_REGNO_REG_SET (&deps->reg_last_in_use, i); } ! /* Flush pending lists on jumps, but not on speculative checks. */ ! if (JUMP_P (insn) && !(sel_sched_p () ! && sel_insn_is_speculation_check (insn))) flush_pending_lists (deps, insn, true, true); reg_pending_barrier = NOT_A_BARRIER; --- 3262,3270 SET_REGNO_REG_SET (&deps->reg_last_in_use, i); } ! /* Don't flush pending lists on speculative checks for !selective scheduling. */ ! if (!sel_sched_p () || !sel_insn_is_speculation_check (insn)) flush_pending_lists (deps, insn, true, true); reg_pending_barrier = NOT_A_BARRIER;
Re: [PATCH] Fix PR 55889
On 19.02.2013 17:13, Andrey Belevantsev wrote:
Hello,
As we discussed in the PR, the problem here is that the selective scheduler
does not expect that renaming a hard register to a pseudo would result in
extra dependencies. The dependencies come from implicit clobbers code of
sched-deps.c, so I've made a minimal patch that checks only for that case
using the same function from IRA and avoids it.
The patch fixes the test case on AIX as reported by David and also
bootstraps and tests fine on x86-64 and ia64, ok for trunk?
The second patch is restoring debug printing in the scheduler that was
accidentally broken, possibly by Steven's cleanups (now a pattern is
expected where an insn was previously). I will commit is as obvious
separately.
Andrey
2012-02-19 Andrey Belevantsev
PR middle-end/55889
* sel-sched.c: Include ira.h.
(implicit_clobber_conflict_p): New function.
(moveup_expr): Use it.
* Makefile.in (sel-sched.o): Depend on ira.h.
Now backported this to 4.7 and 4.6.
Andrey
Index: gcc/ChangeLog
===
*** gcc/ChangeLog (revision 197297)
--- gcc/ChangeLog (revision 197298)
***
*** 1,6
--- 1,18
2013-04-01 Andrey Belevantsev
Backport from mainline
+ 2012-02-19 Andrey Belevantsev
+
+ PR middle-end/55889
+
+ * sel-sched.c: Include ira.h.
+ (implicit_clobber_conflict_p): New function.
+ (moveup_expr): Use it.
+ * Makefile.in (sel-sched.o): Depend on ira.h.
+
+ 2013-04-01 Andrey Belevantsev
+
+ Backport from mainline
2013-02-25 Andrey Belevantsev
Alexander Monakov
Index: gcc/sel-sched.c
===
*** gcc/sel-sched.c (revision 197297)
--- gcc/sel-sched.c (revision 197298)
*** along with GCC; see the file COPYING3.
*** 45,50
--- 45,51
#include "rtlhooks-def.h"
#include "output.h"
#include "emit-rtl.h"
+ #include "ira.h"
#ifdef INSN_SCHEDULING
#include "sel-sched-ir.h"
*** moving_insn_creates_bookkeeping_block_p
*** 2113,2118
--- 2114,2174
return TRUE;
}
+ /* Return true when the conflict with newly created implicit clobbers
+between EXPR and THROUGH_INSN is found because of renaming. */
+ static bool
+ implicit_clobber_conflict_p (insn_t through_insn, expr_t expr)
+ {
+ HARD_REG_SET temp;
+ rtx insn, reg, rhs, pat;
+ hard_reg_set_iterator hrsi;
+ unsigned regno;
+ bool valid;
+
+ /* Make a new pseudo register. */
+ reg = gen_reg_rtx (GET_MODE (EXPR_LHS (expr)));
+ max_regno = max_reg_num ();
+ maybe_extend_reg_info_p ();
+
+ /* Validate a change and bail out early. */
+ insn = EXPR_INSN_RTX (expr);
+ validate_change (insn, &SET_DEST (PATTERN (insn)), reg, true);
+ valid = verify_changes (0);
+ cancel_changes (0);
+ if (!valid)
+ {
+ if (sched_verbose >= 6)
+ sel_print ("implicit clobbers failed validation, ");
+ return true;
+ }
+
+ /* Make a new insn with it. */
+ rhs = copy_rtx (VINSN_RHS (EXPR_VINSN (expr)));
+ pat = gen_rtx_SET (VOIDmode, reg, rhs);
+ start_sequence ();
+ insn = emit_insn (pat);
+ end_sequence ();
+
+ /* Calculate implicit clobbers. */
+ extract_insn (insn);
+ preprocess_constraints ();
+ ira_implicitly_set_insn_hard_regs (&temp);
+ AND_COMPL_HARD_REG_SET (temp, ira_no_alloc_regs);
+
+ /* If any implicit clobber registers intersect with regular ones in
+ through_insn, we have a dependency and thus bail out. */
+ EXECUTE_IF_SET_IN_HARD_REG_SET (temp, 0, regno, hrsi)
+ {
+ vinsn_t vi = INSN_VINSN (through_insn);
+ if (bitmap_bit_p (VINSN_REG_SETS (vi), regno)
+ || bitmap_bit_p (VINSN_REG_CLOBBERS (vi), regno)
+ || bitmap_bit_p (VINSN_REG_USES (vi), regno))
+ return true;
+ }
+
+ return false;
+ }
+
/* Modifies EXPR so it can be moved through the THROUGH_INSN,
performing necessary transformations. Record the type of transformation
made in PTRANS_TYPE, when it is not NULL. When INSIDE_INSN_GROUP,
*** moveup_expr (expr_t expr, insn_t through
*** 2245,2250
--- 2301,2317
if (!enable_schedule_as_rhs_p || !EXPR_SEPARABLE_P (expr))
return MOVEUP_EXPR_NULL;
+ /* When renaming a hard register to a pseudo before reload, extra
+dependencies can occur from the implicit clobbers of the insn.
+Filter out such cases here. */
+ if (!reload_completed && REG_P (EXPR_LHS (expr))
+ && HARD_REGISTER_P (EXPR_LHS (expr))
+ && implicit_clobber_conflict_p (through_insn, expr))
+ {
+ if (sched_verbose >= 6)
+ sel_print ("implicit clobbers conflict detected, ");
+ return MOVEUP_EXPR_NULL;
+ }
EXPR_TARGET_AVAILABLE (expr) = false;
was_target_conflict = true;
Re: [libitm,PATCH] Fix bootstrap due to __always_inline in libitm
Hi, On 04/01/2013 02:00 AM, Gerald Pfeifer wrote: Andi's patch broke bootstrap on all FreeBSD platforms, which took me a bit to realize since he did not update the ChangeLog: 2013-03-23 Andi Kleen * local_atomic (__always_inline): Add. (__calculate_memory_order, atomic_thread_fence, atomic_signal_fence, test_and_set, clear, store, load, exchange, compare_exchange_weak, compare_exchange_strong, fetch_add, fetch_sub, fetch_and, fetch_or, fetch_xor): Add __always_inline to force inlining. The problem is the he added the following to local_atomic #ifndef __always_inline #define __always_inline inline __attribute__((always_inline)) #endif whereas /usr/include/sys/cdefs.h on FreeBSD has the following #define__always_inline __attribute__((__always_inline__)) and hence misses the inline (plus libitm/common.h already has ALWAYS_INLINE for that purpose). First blush it seems to me that we should consistently use ALWAYS_INLINE in this file too. I am fixing this by adding an explicit inline to those cases where necessary. I did not add it to struct members, which are considered inline by default (and believe Andi's patch may have been a bit over- eager from that perspective). But not that inline isn't the same as always_inline... Paolo.
[wwwdocs] Remove redirected link to AMD x86-64 release
This page now goes to the general AMD press release page, which we don't want to link to (and which does not have the original information anymore). So, gone it is. Gerald Index: readings.html === RCS file: /cvs/gcc/wwwdocs/htdocs/readings.html,v retrieving revision 1.230 diff -u -3 -p -r1.230 readings.html --- readings.html 31 Mar 2013 21:15:48 - 1.230 +++ readings.html 1 Apr 2013 10:05:03 - @@ -147,7 +147,6 @@ Intel®64 and IA-32 Architectures Sof http://www.agner.org/optimize/";>http://www.agner.org/optimize/ http://www.sandpile.org";>www.sandpile.org: Christian Ludloff's technical x86 processor information. - http://www.amd.com/us-en/Corporate/VirtualPressRoom/0,,51_104_857_875^2371,00.html";>AMD's x86-64 architecture i860
[Patch, testsuite] Fix sra-13.c for 16 bit int
Fix test case sra-13.c that assumed int is always 4 bytes. Regards, Pitchumani 2013-04-01 Pitchumani Sivanupandi testsuite * gcc.dg/tree-ssa/sra-13.c: Fix for 16 bit int --- gcc/testsuite/gcc.dg/tree-ssa/sra-13.c (revision 197081) +++ gcc/testsuite/gcc.dg/tree-ssa/sra-13.c (working copy) @@ -95,7 +95,7 @@ b = 0; gu1.b.l = 2000; s = bar (); - if (s != 2000) + if (s != (int)2000) __builtin_abort (); if (gu2.b.l != 2000) __builtin_abort ();
Re: [Patch, Fortran, OOP] PR 56500: "IMPLICIT CLASS(...)" wrongly rejected
Ping! (I hope I didn't "anti-advertise" this patch too much ;) 2013/3/20 Janus Weil : > Hi all, > > here is a simple patch which fixes some problems with IMPLICT > CLASS(...) statements. Actually that's not a feature I would seriously > recommend anyone to use, but the Fortran standard allows it, so I > guess we should better support it ;) > > The short patch attached here replaces all previous draft patches in > the PR and simultaneously fixes both comment 0 and 1. Regtested on > x86_64-unknown-linux-gnu. Ok for trunk? > > Cheers, > Janus > > > 2013-03-20 Janus Weil > > PR fortran/56500 > * symbol.c (gfc_set_default_type): Build class container for > IMPLICIT CLASS. > > 2013-03-20 Janus Weil > > PR fortran/56500 > * gfortran.dg/implicit_class_1.f90: New.
Re: [Patch, Fortran, OOP] PR 56500: "IMPLICIT CLASS(...)" wrongly rejected
On 04/01/2013 07:03 AM, Janus Weil wrote: > Ping! > > (I hope I didn't "anti-advertise" this patch too much ;) > > OK for trunk. Jerry
Re: [Patch, Fortran, OOP] PR 56500: "IMPLICIT CLASS(...)" wrongly rejected
Hi Janus, Ping! (I hope I didn't "anti-advertise" this patch too much ;) 2013/3/20 Janus Weil : Hi all, here is a simple patch which fixes some problems with IMPLICT CLASS(...) statements. Actually that's not a feature I would seriously recommend anyone to use, but the Fortran standard allows it, so I guess we should better support it ;) The patch is OK. Regards Thomas
[PATCH] [MIPS] Support microMIPS HI/QI moves
2013-04-01 Catherine Moore
* config/mips/mips.md (*movhi_internal, *movqi_internal): New
operands. Record compression.
Index: mips.md
===
--- mips.md (revision 197114)
+++ mips.md (working copy)
@@ -4538,13 +4538,14 @@
})
(define_insn "*movhi_internal"
- [(set (match_operand:HI 0 "nonimmediate_operand" "=d,d,d,m,*a,*d")
- (match_operand:HI 1 "move_operand" "d,I,m,dJ,*d*J,*a"))]
+ [(set (match_operand:HI 0 "nonimmediate_operand" "=d,!u,d,!u,d,ZU,m,*a,*d")
+ (match_operand:HI 1 "move_operand" "d,J,I,ZU,m,!u,dJ,*d*J,*a"))]
"!TARGET_MIPS16
&& (register_operand (operands[0], HImode)
|| reg_or_0_operand (operands[1], HImode))"
{ return mips_output_move (operands[0], operands[1]); }
- [(set_attr "move_type" "move,const,load,store,mtlo,mflo")
+ [(set_attr "move_type" "move,const,const,load,load,store,store,mtlo,mflo")
+ (set_attr "compression" "all,micromips,*,micromips,*,micromips,*,*,*")
(set_attr "mode" "HI")])
(define_insn "*movhi_mips16"
@@ -4613,13 +4614,14 @@
})
(define_insn "*movqi_internal"
- [(set (match_operand:QI 0 "nonimmediate_operand" "=d,d,d,m,*a,*d")
- (match_operand:QI 1 "move_operand" "d,I,m,dJ,*d*J,*a"))]
+ [(set (match_operand:QI 0 "nonimmediate_operand" "=d,!u,d,!u,d,ZV,m,*a,*d")
+ (match_operand:QI 1 "move_operand" "d,J,I,ZW,m,!u,dJ,*d*J,*a"))]
"!TARGET_MIPS16
&& (register_operand (operands[0], QImode)
|| reg_or_0_operand (operands[1], QImode))"
{ return mips_output_move (operands[0], operands[1]); }
- [(set_attr "move_type" "move,const,load,store,mtlo,mflo")
+ [(set_attr "move_type" "move,const,const,load,load,store,store,mtlo,mflo")
+ (set_attr "compression" "all,micromips,*,micromips,*,micromips,*,*,*")
(set_attr "mode" "QI")])
(define_insn "*movqi_mips16"
[PATCH, libgomp, libatomic] Fix configure for systems without libpthread
On Android pthread is integrated into libc. Attached patch fixes configures for this case by trying to build test without -pthread -lpthread. 2013-04-01 Pavel Chupin Fix libatomic and libgomp configure for systems without libpthread * libatomic/configure.ac: Add test without -pthread -lpthread. * libgomp/configure.ac: Ditto. * libatomic/configure: Regenerate. * libgomp/configure: Regenerate. OK for trunk? -- Pavel Chupin Intel Corporation 0001-Fix-libatomic-and-libgomp-configure-for-systems-with.patch Description: Binary data
[PATCH, testsuite]: Fix FAIL: obj-c++.dg/try-catch-13.mm -fgnu-runtime (test for errors, line 12)
Hello!
A part of an error has been demoted to a note recently. Attached
patch fixes a testcase in the obj-c++ testsuite.
2013-04-01 Uros Bizjak
* obj-c++.dg/try-catch-13.mm: Use dg-message for
"initializing argument" note.
Tested on x86_64-pc-linux-gnu.
OK for mainline?
Uros.
Index: obj-c++.dg/try-catch-13.mm
===
--- obj-c++.dg/try-catch-13.mm (revision 197296)
+++ obj-c++.dg/try-catch-13.mm (working copy)
@@ -22,7 +22,7 @@
typeof(q) k = 66;
some_func (&j);
/* { dg-error "invalid conversion" "" { target *-*-* } 23 } */
-/* { dg-error "initializing argument" "" { target *-*-* } 12 } */
+/* { dg-message "initializing argument" "" { target *-*-* } 12 } */
some_func (&k);
}
@catch (id exc) {
@@ -39,7 +39,7 @@
/* { dg-error "invalid conversion" "" { target *-*-* } 38 } */
/* The following is disabled as it is already checked above and the testsuites
seems
to count multiple different identical errors on the same line only once */
-/* dg-error "initializing argument" "" { target *-*-* } 12 */
+/* dg-message "initializing argument" "" { target *-*-* } 12 */
}
@catch (id exc) {
@throw;
@@ -54,7 +54,7 @@
/* { dg-error "invalid conversion" "" { target *-*-* } 53 } */
/* The following is disabled as it is already checked above and the testsuites
seems
to count multiple different identical errors on the same line only once */
-/* dg-error "initializing argument" "" { target *-*-* } 12 */
+/* dg-message "initializing argument" "" { target *-*-* } 12 */
some_func (&k);
}
@catch (id exc) {
Re: [Patch, Fortran, OOP] PR 56500: "IMPLICIT CLASS(...)" wrongly rejected
>> Ping! >> >> (I hope I didn't "anti-advertise" this patch too much ;) >> >> >> 2013/3/20 Janus Weil : >>> >>> Hi all, >>> >>> here is a simple patch which fixes some problems with IMPLICT >>> CLASS(...) statements. Actually that's not a feature I would seriously >>> recommend anyone to use, but the Fortran standard allows it, so I >>> guess we should better support it ;) > > > The patch is OK. Thanks, Thomas and Jerry, for the almost simultaneous reviews :) I have committed the patch as r197306. Btw, is there a reason the commit is not being listed in the Bugzilla PR automatically? (Did this feature get lost in the last server upgrade, or have I just messed up the ChangeLog format?) Cheers, Janus
[patch] PR56798 (committed)
Fix an error I made in one of the non-mechanical changes of r197234. PR middle-end/56798 * cfgbuild.c (inside_basic_block_p): Restore check broken at r197234. Index: cfgbuild.c === --- cfgbuild.c (revision 197267) +++ cfgbuild.c (working copy) @@ -51,7 +51,7 @@ inside_basic_block_p (const_rtx insn) case CODE_LABEL: /* Avoid creating of basic block for jumptables. */ return (NEXT_INSN (insn) == 0 - || ! JUMP_TABLE_DATA_P (insn)); + || ! JUMP_TABLE_DATA_P (NEXT_INSN (insn))); case JUMP_INSN: case CALL_INSN:
Re: SLP for vectors
On Sat, 30 Mar 2013, Marc Glisse wrote:
* tree-flow-inline.h (get_addr_base_and_unit_offset_1): Handle
BIT_FIELD_REF.
I wrote a safer version of this for PR52436:
case BIT_FIELD_REF:
- return NULL_TREE;
+ {
+ HOST_WIDE_INT this_off = TREE_INT_CST_LOW (TREE_OPERAND (exp, 2));
+ if (this_off % BITS_PER_UNIT)
+ return NULL_TREE;
+ byte_offset += this_off / BITS_PER_UNIT;
+ }
+ break;
[C++ Patch] Mini bunch of mini clean-ups
Hi,
three items:
- Remove DECL_UNBOUND_CLASS_TEMPLATE_P, unused outside cp-tree.h.
- Use get_containing_scope in 4 places (obvious I guess)
- Use existing predicates in 2 places (likewise)
Tested x86_64-linux. Ok?
Thanks,
Paolo.
PS: In my opinion we should also rename DECL_FUNCTION_MEMBER_P to
DECL_MEMBER_FUNCTION_P. Objections?
///
2013-04-01 Paolo Carlini
* cp-tree.h (DECL_UNBOUND_CLASS_TEMPLATE_P): Remove.
(DECL_FUNCTION_TEMPLATE_P): Adjust.
* cxx-pretty-print.c (pp_cxx_nested_name_specifier,
pp_cxx_qualified_id): Use get_containing_scope.
* parser.c (cp_parser_class_head): Likewise.
* pt.c (push_template_decl_real): Likewise.
* decl2.c (import_export_decl): Use DECL_TEMPLOID_INSTANTIATION.
* pt.c (unify): Use CP_INTEGRAL_TYPE_P.
Index: cp-tree.h
===
--- cp-tree.h (revision 197302)
+++ cp-tree.h (working copy)
@@ -3730,14 +3730,10 @@ more_aggr_init_expr_args_p (const aggr_init_expr_a
#define DECL_TEMPLATE_TEMPLATE_PARM_P(NODE) \
(TREE_CODE (NODE) == TEMPLATE_DECL && DECL_TEMPLATE_PARM_P (NODE))
-/* Nonzero if NODE is a TEMPLATE_DECL representing an
- UNBOUND_CLASS_TEMPLATE tree node. */
-#define DECL_UNBOUND_CLASS_TEMPLATE_P(NODE) \
- (TREE_CODE (NODE) == TEMPLATE_DECL && !DECL_TEMPLATE_RESULT (NODE))
-
-#define DECL_FUNCTION_TEMPLATE_P(NODE) \
- (TREE_CODE (NODE) == TEMPLATE_DECL \
- && !DECL_UNBOUND_CLASS_TEMPLATE_P (NODE) \
+/* Nonzero for a DECL that represents a function template. */
+#define DECL_FUNCTION_TEMPLATE_P(NODE) \
+ (TREE_CODE (NODE) == TEMPLATE_DECL\
+ && DECL_TEMPLATE_RESULT (NODE) != NULL_TREE \
&& TREE_CODE (DECL_TEMPLATE_RESULT (NODE)) == FUNCTION_DECL)
/* Nonzero for a DECL that represents a class template or alias
Index: cxx-pretty-print.c
===
--- cxx-pretty-print.c (revision 197302)
+++ cxx-pretty-print.c (working copy)
@@ -260,7 +260,7 @@ pp_cxx_nested_name_specifier (cxx_pretty_printer *
{
if (!SCOPE_FILE_SCOPE_P (t) && t != pp->enclosing_scope)
{
- tree scope = TYPE_P (t) ? TYPE_CONTEXT (t) : DECL_CONTEXT (t);
+ tree scope = get_containing_scope (t);
pp_cxx_nested_name_specifier (pp, scope);
pp_cxx_template_keyword_if_needed (pp, scope, t);
pp_cxx_unqualified_id (pp, t);
@@ -308,7 +308,7 @@ pp_cxx_qualified_id (cxx_pretty_printer *pp, tree
default:
{
- tree scope = TYPE_P (t) ? TYPE_CONTEXT (t) : DECL_CONTEXT (t);
+ tree scope = get_containing_scope (t);
if (scope != pp->enclosing_scope)
{
pp_cxx_nested_name_specifier (pp, scope);
Index: decl2.c
===
--- decl2.c (revision 197302)
+++ decl2.c (working copy)
@@ -2572,8 +2572,7 @@ import_export_decl (tree decl)
else
comdat_p = true;
}
- else if (DECL_TEMPLATE_INSTANTIATION (decl)
- || DECL_FRIEND_PSEUDO_TEMPLATE_INSTANTIATION (decl))
+ else if (DECL_TEMPLOID_INSTANTIATION (decl))
{
/* DECL is an implicit instantiation of a function or static
data member. */
Index: parser.c
===
--- parser.c(revision 197302)
+++ parser.c(working copy)
@@ -18773,9 +18773,7 @@ cp_parser_class_head (cp_parser* parser,
for (scope = TREE_TYPE (type);
scope && TREE_CODE (scope) != NAMESPACE_DECL;
- scope = (TYPE_P (scope)
- ? TYPE_CONTEXT (scope)
- : DECL_CONTEXT (scope)))
+ scope = get_containing_scope (scope))
if (TYPE_P (scope)
&& CLASS_TYPE_P (scope)
&& CLASSTYPE_TEMPLATE_INFO (scope)
Index: pt.c
===
--- pt.c(revision 197302)
+++ pt.c(working copy)
@@ -4803,9 +4803,7 @@ push_template_decl_real (tree decl, bool is_friend
/* Can happen in erroneous input. */
break;
else
- current = (TYPE_P (current)
-? TYPE_CONTEXT (current)
-: DECL_CONTEXT (current));
+ current = get_containing_scope (current);
}
/* Check that the parms are used in the appropriate qualifying scopes
@@ -16750,8 +16748,7 @@ unify (tree tparms, tree targs, tree parm, tree ar
else if (same_type_p (TREE_TYPE (arg), tparm))
/* OK */;
else if ((strict & UNIFY_ALLOW_INTEGER)
- && (TREE_CODE (tparm) == INTEGER_TYPE
- || TREE_CODE (tparm) == BOOLEAN_TYPE))
+ && CP_INTEGRAL_TYPE_P (tparm))
/* Convert the ARG to the type of PARM; the deduced non-type
templat
Re: [C++ Patch] Mini bunch of mini clean-ups
On 04/01/2013 12:41 PM, Paolo Carlini wrote: +#define DECL_FUNCTION_TEMPLATE_P(NODE) \ + (TREE_CODE (NODE) == TEMPLATE_DECL\ + && DECL_TEMPLATE_RESULT (NODE) != NULL_TREE \ Do we need the NULL_TREE check? That is, do we still build a template with no result? PS: In my opinion we should also rename DECL_FUNCTION_MEMBER_P to DECL_MEMBER_FUNCTION_P. Objections? The current name makes it clearer (to me, anyway) that the macro assumes that we already know we're looking at a FUNCTION_DECL. Jason
Re: [C++ Patch] Mini bunch of mini clean-ups
Hi, On 04/01/2013 06:50 PM, Jason Merrill wrote: On 04/01/2013 12:41 PM, Paolo Carlini wrote: +#define DECL_FUNCTION_TEMPLATE_P(NODE) \ + (TREE_CODE (NODE) == TEMPLATE_DECL \ + && DECL_TEMPLATE_RESULT (NODE) != NULL_TREE\ Do we need the NULL_TREE check? That is, do we still build a template with no result? Honestly, I don't know, I followed DECL_TYPE_TEMPLATE_P, I can see what happens if I remove it... PS: In my opinion we should also rename DECL_FUNCTION_MEMBER_P to DECL_MEMBER_FUNCTION_P. Objections? The current name makes it clearer (to me, anyway) that the macro assumes that we already know we're looking at a FUNCTION_DECL. Ok, the subtlety wasn't clean to me looking at the other predicates and the comment. Let's leave it alone for now. Paolo.
Re: [C++ Patch] Mini bunch of mini clean-ups
Hi again On 04/01/2013 06:55 PM, Paolo Carlini wrote: Hi, On 04/01/2013 06:50 PM, Jason Merrill wrote: On 04/01/2013 12:41 PM, Paolo Carlini wrote: +#define DECL_FUNCTION_TEMPLATE_P(NODE) \ + (TREE_CODE (NODE) == TEMPLATE_DECL \ + && DECL_TEMPLATE_RESULT (NODE) != NULL_TREE\ Do we need the NULL_TREE check? That is, do we still build a template with no result? Honestly, I don't know, I followed DECL_TYPE_TEMPLATE_P, I can see what happens if I remove it... We have an ICE for g++.dg/template/qualttp17.C. Is this something we want to further investigate now or shall I just leave the check in? Thanks, Paolo.
New German PO file for 'gcc' (version 4.8.0)
Hello, gentle maintainer. This is a message from the Translation Project robot. A revised PO file for textual domain 'gcc' has been submitted by the German team of translators. The file is available at: http://translationproject.org/latest/gcc/de.po (This file, 'gcc-4.8.0.de.po', has just now been sent to you in a separate email.) All other PO files for your package are available in: http://translationproject.org/latest/gcc/ Please consider including all of these in your next release, whether official or a pretest. Whenever you have a new distribution with a new version number ready, containing a newer POT file, please send the URL of that distribution tarball to the address below. The tarball may be just a pretest or a snapshot, it does not even have to compile. It is just used by the translators when they need some extra translation context. The following HTML page has been updated: http://translationproject.org/domain/gcc.html If any question arises, please contact the translation coordinator. Thank you for all your work, The Translation Project robot, in the name of your translation coordinator.
Re: [C++ Patch] Mini bunch of mini clean-ups
On 04/01/2013 01:11 PM, Paolo Carlini wrote: We have an ICE for g++.dg/template/qualttp17.C. Is this something we want to further investigate now or shall I just leave the check in? Leave the check. The patch is OK. Jason
[patch,libfortran] PR56660 Fails to read NAMELIST with certain form array syntax
Hi all,
When doing namelist reads, nml_read_obj calls itself recursively to read through
arrays. Short lists are allowed so we have to have a way to detect if we have a
short read or a real error.
We do this by flagging errors and then backing out of the read and checking to
see if what we error-ed on was a valid object name rather than data. This is
problematic for reading strings or logicals, since the data can look like names.
To resolve the problem, we use a line_buffer to hold reads as we look ahead and
if we find an error we rewind, bail out of the read, and proceed to the next
read cycle which looks for an object name followed by an "=" sign.
With this particular bug, nml_read_obj was clearing the error flag itself with
the read so that rather then bailing out, it tried to continue reading data
until it was done, then the subsequent read failed looking for a valid name,
which had been passed by.
The problem is resolved by moving the error flag reset outside nml_read_obj just
before the call to nml_read_obj. Also, we test the flag on entering
nml_read_obj, and if it is set, we bail out right away, a do nothing, until the
parent nml_read_obj finishes its loops.
Regression tested on x86-64. Test case attached.
OK for trunk?
Jerry
2013-04-01 Jerry DeLisle
PR libfortran/56660
* io/list_read.c (nml_read_obj): Do not reset the read error flag
inside nml_read_obj. If the read error flag is found set just exit.
Fix some whitespace on comments.
(nml_read_obj_data): Reset the read error flag before the first call
to nml_read_object.
Index: list_read.c
===
--- list_read.c (revision 197290)
+++ list_read.c (working copy)
@@ -2490,9 +2490,9 @@ nml_read_obj (st_parameter_dt *dtp, namelist_info
size_t obj_name_len;
void * pdata;
- /* This object not touched in name parsing. */
-
- if (!nl->touched)
+ /* If we have encountered a previous read error or this object has not been
+ touched in name parsing, just return. */
+ if (dtp->u.p.nml_read_error || !nl->touched)
return true;
dtp->u.p.repeat_count = 0;
@@ -2532,10 +2532,8 @@ nml_read_obj (st_parameter_dt *dtp, namelist_info
- GFC_DESCRIPTOR_LBOUND(nl,dim))
* GFC_DESCRIPTOR_STRIDE(nl,dim) * nl->size);
- /* Reset the error flag and try to read next value, if
- dtp->u.p.repeat_count=0 */
+ /* If we are finished with the repeat count, try to read next value. */
- dtp->u.p.nml_read_error = 0;
nml_carry = 0;
if (--dtp->u.p.repeat_count <= 0)
{
@@ -2564,8 +2562,8 @@ nml_read_obj (st_parameter_dt *dtp, namelist_info
break;
case BT_REAL:
- /* Need to copy data back from the real location to the temp in order
- to handle nml reads into arrays. */
+ /* Need to copy data back from the real location to the temp in
+ order to handle nml reads into arrays. */
read_real (dtp, pdata, len);
memcpy (dtp->u.p.value, pdata, dlen);
break;
@@ -3022,6 +3020,7 @@ get_name:
nl = first_nl;
}
+ dtp->u.p.nml_read_error = 0;
if (!nml_read_obj (dtp, nl, 0, pprev_nl, nml_err_msg, nml_err_msg_size,
clow, chigh))
goto nml_err_ret;
! { dg-do run }
! PR56660 Fails to read NAMELIST with certain form array syntax
type ptracer
character(len = 2) :: sname
logical :: lini
end type ptracer
type(ptracer) , dimension(3) :: tracer
namelist/naml1/ tracer
tracer(:) = ptracer('XXX', .false.)
open (99, file='nml.dat', status="replace")
write(99,*) "&naml1"
!write(99,*) " tracer(2) = 'bb' , .true."
write(99,*) " tracer(:) = 'aa' , .true."
write(99,*) " tracer(2) = 'bb' , .true."
write(99,*) "/"
rewind(99)
read (99, nml=naml1)
close (99, status="delete")
if (tracer(1)%sname.ne.'aa') call abort()
if (.not.tracer(1)%lini) call abort()
if (tracer(2)%sname.ne.'bb') call abort()
if (.not.tracer(2)%lini) call abort()
if (tracer(3)%sname.ne.'XX') call abort()
if (tracer(3)%lini) call abort()
!write (*, nml=naml1)
end
Re: [patch,libfortran] PR56660 Fails to read NAMELIST with certain form array syntax
Am 01.04.2013 20:33, schrieb Jerry DeLisle:
With this particular bug, nml_read_obj was clearing the error flag itself with
the read so that rather then bailing out, it tried to continue reading data
until it was done, then the subsequent read failed looking for a valid name,
which had been passed by.
The problem is resolved by moving the error flag reset outside nml_read_obj just
before the call to nml_read_obj. Also, we test the flag on entering
nml_read_obj, and if it is set, we bail out right away, a do nothing, until the
parent nml_read_obj finishes its loops.
Regression tested on x86-64. Test case attached.
OK for trunk?
OK - and thanks for the patch!
* * *
Do we want to close PR 56786? While it is a regression from 4.6, it
seems to only occur for slightly invalid namelists - unless, I missed
some fineprint. Or do we want to backport it to all/some of 4.6 / 4.7 /
4.8?
* * *
The test case below is motivated by this PR56660 and the original
PR49791. It fails with "Cannot match namelist object name 'cc'". I was
wondering whether we should support that extended read (= vendor
extension) or not. Ifort and g95 do; pathf95, Crayftn, pgf95 and NAG don't.
(gfortran supports the extended read for simpler variants, e.g. PR49791,
comment 0 and comment 18.)
Tobias
type ptracer
character(len = 2) :: sname
logical :: lini
end type ptracer
type(ptracer) , dimension(3) :: tracer
namelist/naml1/ tracer
tracer(:) = ptracer('XXX', .false.)
write (*, nml=naml1)
open (99, file='nml.dat', status="replace")
write(99,*) "&naml1"
write(99,*) " tracer(:) = 'aa' , .true."
write(99,*) " tracer(2) = 'bb' , .true., 'cc', .true." ! Extended
read: Two DT elements
write(99,*) "/"
rewind(99)
read (99, nml=naml1)
write (*, nml=naml1)
close (99, status="delete")
end
Re: Fill more delay slots in conditional returns
> I'm not sure what this is supposed to do. How is pat == target ever > going to be true when ANY_RETURN_P (pat) is true? Isn't target supposed > to be a CODE_LABEL or NULL? What am I missing? The simple_return change from Bernd. The JUMP_LABEL of a JUMP_INSN is now either a CODE_LABEL or a RETURN or a SIMPLE_RETURN. > What does the RTL for your conditional return look like > > (if_then_else (cond) (pc) (return)) > > Where the (pc) and (return) can be reversed as well? That's what the > later hunks in get_branch_condition seem to imply. It's the usual support for branches and inverted branches applied to returns, so I'm not sure what you're asking here. :-) > The cleanup stuff is OK, check that in whenver you'd like. Thanks. -- Eric Botcazou
Re: [patch] cilkplus: Array notation for C patch
On 03/29/13 16:57, Iyer, Balaji V wrote: Hello Joseph, Aldy et al., I reworded couple comments (e.g changed builtin with built-in, etc) and added a header comment to the c-array-notation.c that explains the overall process. I am attaching a fixed patch. Ok, this latest patch that Balaji has posted has all of our changes against trunk. It is the current state of the cilkplus-merge branch. Joseph, fire away :).
Re: [var-template] Accept variable template declaration
On Sun, Mar 31, 2013 at 12:13 AM, Jason Merrill wrote: > > On 03/29/2013 10:56 PM, Gabriel Dos Reis wrote: >> >>int wanted = num_template_headers_for_class (ctx); > > > I think you want to add one to wanted if decl is a primary template. OK but I am a little bit dense here: why is that necessary? -- Gaby
Re: [var-template] Accept variable template declaration
On 04/01/2013 04:53 PM, Gabriel Dos Reis wrote: On Sun, Mar 31, 2013 at 12:13 AM, Jason Merrill wrote: On 03/29/2013 10:56 PM, Gabriel Dos Reis wrote: int wanted = num_template_headers_for_class (ctx); I think you want to add one to wanted if decl is a primary template. OK but I am a little bit dense here: why is that necessary? By 'primary' I mean something that is itself a template, not just a member of a class template. Previously variables couldn't be primary templates, but I believe your proposal is to change that; in that case, we should expect to see a template header for such a template, in addition to any headers for enclosing classes. Jason
Re: [patch] cilkplus: Array notation for C patch
On 03/29/13 16:57, Iyer, Balaji V wrote:
+bool
+find_rank (location_t loc, tree orig_expr, tree array, bool ignore_builtin_fn,
+ size_t *rank)
+{
+ tree ii_tree;
Balaji, I believe what Joseph meant with saving the location_t is so we
can give meaningful error messages when encountering tests like this
(distilled from one of the tests you added):
houston:/build/cilkplus-merge1/gcc$ cat a.c
int array[10][10];
int array2[10];
void foo()
{
array[:][:] = array2[:];
}
Currently we are issuing this error:
houston:/build/cilkplus-merge1/gcc$ ./cc1 a.c -fcilkplus -quiet
a.c: In function 'foo':
a.c:6:15: error: rank mismatch between 'array' and 'array2'
array[:][:] = array2[:];
^
Ideally, we should issue something like (notice the carets):
error: rank mismatch between 'array' and 'array2'
array[:][:] = array2[:];
^
note: original mismatching rank at:
array[:][:] = array2[:];
^
Or something similar... the note can be emitted with inform(). So you
need to save the original rank location to emit the note.
[Patch, Fortran, OOP] PR 56284: ICE with alternate return in type-bound procedure
Hi all, here is a small patch which does two things: 1) It fixes the ICE in the subject line (in a rather obvious way). 2) It warns about alternate-return arguments (which is an obsolescent feature), and adds -std=legacy to some test cases to suppress the warning. Regarding the second point, one should mention that we already have a warning for "alternate return", but this only triggers if there is an actual RETURN statement (which is not the case for the test code in the PR). The warning I'm adding triggers on the appearance of an asterisk argument. This induces a certain redundancy, i.e. we warn about both the alternate-return argument and the alternate RETURN statement. The question is if we want to keep this, or whether on can remove the old warning for the RETURN statement (which could be done in a follow-up patch). The patch is regtested on x86_64-unknown-linux-gnu. Ok for trunk? Cheers, Janus 2013-04-01 Janus Weil PR fortran/56284 PR fortran/40881 * decl.c (gfc_match_formal_arglist): Warn about alternate-return arguments. * interface.c (check_dummy_characteristics): Return if symbols are NULL. 2013-04-01 Janus Weil PR fortran/56284 PR fortran/40881 * gfortran.dg/altreturn_8.f90: New. * gfortran.dg/altreturn_2.f90: Add -std=legacy. * gfortran.dg/intrinsic_actual_3.f90: Ditto. * gfortran.dg/invalid_interface_assignment.f90: Ditto. pr56284.diff Description: Binary data altreturn_8.f90 Description: Binary data
RE: [patch] cilkplus: Array notation for C patch
> -Original Message-
> From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-
> ow...@gcc.gnu.org] On Behalf Of Aldy Hernandez
> Sent: Monday, April 01, 2013 5:02 PM
> To: Iyer, Balaji V
> Cc: 'Joseph Myers'; 'gcc-patches'
> Subject: Re: [patch] cilkplus: Array notation for C patch
>
> On 03/29/13 16:57, Iyer, Balaji V wrote:
>
> > +bool
> > +find_rank (location_t loc, tree orig_expr, tree array, bool
> > ignore_builtin_fn,
> > + size_t *rank)
> > +{
> > + tree ii_tree;
>
> Balaji, I believe what Joseph meant with saving the location_t is so we can
> give
> meaningful error messages when encountering tests like this (distilled from
> one
> of the tests you added):
>
> houston:/build/cilkplus-merge1/gcc$ cat a.c int array[10][10]; int array2[10];
>
> void foo()
> {
>array[:][:] = array2[:];
> }
>
> Currently we are issuing this error:
>
> houston:/build/cilkplus-merge1/gcc$ ./cc1 a.c -fcilkplus -quiet
> a.c: In function 'foo':
> a.c:6:15: error: rank mismatch between 'array' and 'array2'
> array[:][:] = array2[:];
> ^
>
> Ideally, we should issue something like (notice the carets):
>
> error: rank mismatch between 'array' and 'array2'
> array[:][:] = array2[:];
> ^
OK, I understand it a bit better now. Please let me know if I am mistaken here:
We need to do EXPR_LOCATION (array2) and then issue the error
> note: original mismatching rank at:
> array[:][:] = array2[:];
> ^
>
> Or something similar... the note can be emitted with inform(). So you need to
> save the original rank location to emit the note.
...and we take the location information in "loc" and print that out in inform?
Did I get it right?
Thanks,
Balaji V. Iyer.
Re: [var-template] Accept variable template declaration
On Mon, Apr 1, 2013 at 3:58 PM, Jason Merrill wrote: > On 04/01/2013 04:53 PM, Gabriel Dos Reis wrote: >> >> On Sun, Mar 31, 2013 at 12:13 AM, Jason Merrill wrote: >>> >>> >>> On 03/29/2013 10:56 PM, Gabriel Dos Reis wrote: int wanted = num_template_headers_for_class (ctx); >>> >>> >>> I think you want to add one to wanted if decl is a primary template. >> >> >> OK but I am a little bit dense here: why is that necessary? > > > By 'primary' I mean something that is itself a template, not just a member > of a class template. Previously variables couldn't be primary templates, > but I believe your proposal is to change that; in that case, we should > expect to see a template header for such a template, in addition to any > headers for enclosing classes. Ah, yes; got it. Thanks! -- Gaby
C++ PATCH for c++/56794 (range for and variadic template)
We try to resolve types at template definition time if expressions don't
depend on template parameters, but this testcase shows a case of
dependency we weren't considering: the bounds of an array can depend on
the length of the initializer.
Tested x86_64-pc-linux-gnu, applying to 4.7, 4.8, trunk.
commit ab873d428ec43d33d1e34567749104cc5a9544ad
Author: Jason Merrill
Date: Mon Apr 1 15:45:54 2013 -0400
PR c++/56794
* parser.c (cp_parser_range_for): Don't try to do auto deduction
in a template if the type of the range is incomplete.
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index f29e80d..05c03f4 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -9615,7 +9615,10 @@ cp_parser_range_for (cp_parser *parser, tree scope, tree init, tree range_decl)
range_expr = error_mark_node;
stmt = begin_range_for_stmt (scope, init);
finish_range_for_decl (stmt, range_decl, range_expr);
- if (!type_dependent_expression_p (range_expr)
+ if (range_expr != error_mark_node
+ && !type_dependent_expression_p (range_expr)
+ /* The length of an array might be dependent. */
+ && COMPLETE_TYPE_P (TREE_TYPE (range_expr))
/* do_auto_deduction doesn't mess with template init-lists. */
&& !BRACE_ENCLOSED_INITIALIZER_P (range_expr))
do_range_for_auto_deduction (range_decl, range_expr);
diff --git a/gcc/testsuite/g++.dg/cpp0x/range-for24.C b/gcc/testsuite/g++.dg/cpp0x/range-for24.C
new file mode 100644
index 000..b4a5b18
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/range-for24.C
@@ -0,0 +1,15 @@
+// PR c++/56794
+// { dg-require-effective-target c++11 }
+
+template
+static void Colors()
+{
+static const int colors[] = { values... };
+
+for(auto c: colors) { }
+}
+
+int main()
+{
+Colors<0,1,2> ();
+}
C++ PATCH for c++/56793 (scoped enum in class)
Here, when we saw a.B::Y we got confused because B is not a class. But
this seems to me like a reasonable thing to say.
Tested x86_64-pc-linux-gnu, applying to trunk and 4.8.
commit 2ea3a64073bdfe37cfd812e5b297fa9831d10e71
Author: Jason Merrill
Date: Mon Apr 1 16:16:49 2013 -0400
PR c++/56793
* typeck.c (finish_class_member_access_expr): Handle enum scope.
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 763fc0b..043d52f 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -2684,6 +2684,23 @@ finish_class_member_access_expr (tree object, tree name, bool template_p,
return error_mark_node;
}
+ if (TREE_CODE (scope) == ENUMERAL_TYPE)
+ {
+ /* Looking up a member enumerator (c++/56793). */
+ if (!TYPE_CLASS_SCOPE_P (scope)
+ || !DERIVED_FROM_P (TYPE_CONTEXT (scope), object_type))
+ {
+ if (complain & tf_error)
+ error ("%<%D::%D%> is not a member of %qT",
+ scope, name, object_type);
+ return error_mark_node;
+ }
+ tree val = lookup_enumerator (scope, name);
+ if (TREE_SIDE_EFFECTS (object))
+ val = build2 (COMPOUND_EXPR, TREE_TYPE (val), object, val);
+ return val;
+ }
+
gcc_assert (CLASS_TYPE_P (scope));
gcc_assert (identifier_p (name) || TREE_CODE (name) == BIT_NOT_EXPR);
diff --git a/gcc/testsuite/g++.dg/cpp0x/enum25.C b/gcc/testsuite/g++.dg/cpp0x/enum25.C
new file mode 100644
index 000..cb2cf8f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/enum25.C
@@ -0,0 +1,18 @@
+// PR c++/56793
+// { dg-require-effective-target c++11 }
+
+struct A
+{
+ enum struct B {X, Y} b;
+} a;
+
+enum struct D {X,Y};
+struct C { } c;
+
+int main ()
+{
+ if (a.b == a.B::Y)
+a.b = A::B::X;
+
+ c.D::Y; // { dg-error "not a member" }
+}
C++ PATCH for c++/56772 (array new with initializer-list)
Here trying to process the initializer at template time didn't work;
let's not bother in this case.
Tested x86_64-pc-linux-gnu, applying to trunk and 4.8.
commit 48ec5c5cc4c8f5640dd25b3b455141cde868361c
Author: Jason Merrill
Date: Mon Apr 1 16:30:48 2013 -0400
PR c++/56772
* init.c (build_new): Don't try to process an array initializer
at template definition time.
diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index ab6af14..7b7de02 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -2920,6 +2920,7 @@ build_new (vec **placement, tree type, tree nelts,
if (dependent_type_p (type)
|| any_type_dependent_arguments_p (*placement)
|| (nelts && type_dependent_expression_p (nelts))
+ || (nelts && *init)
|| any_type_dependent_arguments_p (*init))
return build_raw_new_expr (*placement, type, nelts, *init,
use_global_new);
diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist68.C b/gcc/testsuite/g++.dg/cpp0x/initlist68.C
new file mode 100644
index 000..7cfe1a3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/initlist68.C
@@ -0,0 +1,20 @@
+// PR c++/56772
+// { dg-require-effective-target c++11 }
+
+typedef __SIZE_TYPE__ size_t;
+void* operator new[](size_t, void *p) { return p; }
+template
+void f ()
+{
+ size_t coord [2][2];
+ new (&coord) size_t [2][2]
+ {
+ {0,0},
+ {0,0},
+ };
+}
+
+int main ()
+{
+ f<>();
+}
Re: [Patch, Fortran, OOP] PR 56284: ICE with alternate return in type-bound procedure
Janus Weil wrote: here is a small patch which does two things: 1) It fixes the ICE in the subject line (in a rather obvious way). 2) It warns about alternate-return arguments (which is an obsolescent feature), and adds -std=legacy to some test cases to suppress the warning. I would prefer if you could use "-std=gnu", I find -std=legacy is too special. Alternate-returns are perfectly valid in Fortran 2008 - the standard just requires the obsolescent warning, which -std=f95/f2003/f2008/f2008ts activate. And by default "-pedantic". Using "" or "-std=gnu" should be sufficient to disable "-pedantic". Regarding the second point, one should mention that we already have a warning for "alternate return", but this only triggers if there is an actual RETURN statement (which is not the case for the test code in the PR). The warning I'm adding triggers on the appearance of an asterisk argument. Shouldn't one then remove the other warning, which should be then unreachable? If so, please do so. This induces a certain redundancy, i.e. we warn about both the alternate-return argument and the alternate RETURN statement. The question is if we want to keep this, or whether on can remove the old warning for the RETURN statement (which could be done in a follow-up patch). Or it is not unreachable but leads to double-diagnostic which is not better, either. The patch is regtested on x86_64-unknown-linux-gnu. Ok for trunk? OK with the -std= change. (Please run "make RUNTESTFLAGS="dg.exp=..f90" prior committal to check whether it indeed works with -std=gnu.) Thanks for the patch! Tobias PS: Don't forget to copy the commit message to Bugzilla - thanks! 2013-04-01 Janus Weil PR fortran/56284 PR fortran/40881 * decl.c (gfc_match_formal_arglist): Warn about alternate-return arguments. * interface.c (check_dummy_characteristics): Return if symbols are NULL. 2013-04-01 Janus Weil PR fortran/56284 PR fortran/40881 * gfortran.dg/altreturn_8.f90: New. * gfortran.dg/altreturn_2.f90: Add -std=legacy. * gfortran.dg/intrinsic_actual_3.f90: Ditto. * gfortran.dg/invalid_interface_assignment.f90: Ditto.
Re: [Fortran-dev][Patch] Add "rank" field and use it
Tobias Burnus wrote: This patch adds a "rank" field to the descriptor and uses it. Before the information was in the dtype. At the same time, it bumps the maximal rank from 7 to 15 as required by Fortran 2008. I have now committed the patch as Rev. 197323 to Fortran-dev - and updated it to the trunk version (Rev. 197332). Tobias
Re: [Patch, Fortran, OOP] PR 56284: ICE with alternate return in type-bound procedure
>> here is a small patch which does two things: >> 1) It fixes the ICE in the subject line (in a rather obvious way). >> 2) It warns about alternate-return arguments (which is an obsolescent >> feature), and adds -std=legacy to some test cases to suppress the >> warning. > > I would prefer if you could use "-std=gnu", I find -std=legacy is too > special. Alternate-returns are perfectly valid in Fortran 2008 - the > standard just requires the obsolescent warning, which > -std=f95/f2003/f2008/f2008ts activate. And by default "-pedantic". Using "" > or "-std=gnu" should be sufficient to disable "-pedantic". Ok with me. However, the other altreturn_* tests all have -std=legacy (for whatever reason). Doesn't make a whole lot of difference, I guess ... Out of curiosity I just checked: The gfortran.dg directory has ~120 tests with -std=legacy and ~60 with -std=gnu. >> Regarding the second point, one should mention that we already have a >> warning for "alternate return", but this only triggers if there is an >> actual RETURN statement (which is not the case for the test code in >> the PR). The warning I'm adding triggers on the appearance of an >> asterisk argument. > > Shouldn't one then remove the other warning, which should be then > unreachable? If so, please do so. Yes, will do so in a follow-up patch. >> This induces a certain redundancy, i.e. we warn about both the >> alternate-return argument and the alternate RETURN statement. The >> question is if we want to keep this, or whether on can remove the old >> warning for the RETURN statement (which could be done in a follow-up >> patch). > > Or it is not unreachable but leads to double-diagnostic which is not better, > either. Right. >> The patch is regtested on x86_64-unknown-linux-gnu. Ok for trunk? > > OK with the -std= change. (Please run "make > RUNTESTFLAGS="dg.exp=..f90" prior committal to check whether it indeed > works with -std=gnu.) All of them still work with std=gnu. > Thanks for the patch! Thanks for the review! Cheers, Janus >> 2013-04-01 Janus Weil >> >> PR fortran/56284 >> PR fortran/40881 >> * decl.c (gfc_match_formal_arglist): Warn about alternate-return >> arguments. >> * interface.c (check_dummy_characteristics): Return if symbols are >> NULL. >> >> 2013-04-01 Janus Weil >> >> PR fortran/56284 >> PR fortran/40881 >> * gfortran.dg/altreturn_8.f90: New. >> * gfortran.dg/altreturn_2.f90: Add -std=legacy. >> * gfortran.dg/intrinsic_actual_3.f90: Ditto. >> * gfortran.dg/invalid_interface_assignment.f90: Ditto. > > pr56284_v2.diff Description: Binary data
[patch] trivial replacements for SET_INSN_DELETED and BLOCK_FOR_INSN as lhs
Hello,
The SET_INSN_DELETED vs. set_insn_deleted thing is just an incomplete cleanup.
Using set_block_for_insn instead of using BLOCK_FOR_INSN is the "proper" way.
So, mechanical replacements. If no-one objects and the usual
bootstrap+testing passes, I'll commit this later this week as obvious.
Ciao!
Steven
* basic-block.h (set_block_for_insn): Remove define.
* rtl.h (BLOCK_FOR_INSN): Rename define to INSN_BASIC_BLOCK,
reincarnate as static inline function.
(set_block_for_insn): Reincarnate as static inline function.
(SET_INSN_DELETED): Remove define.
* emit-rtl.c (make_insn_raw): Use set_block_for_insn.
(make_debug_insn_raw, make_jump_insn_raw, make_call_insn_raw,
emit_note_before, emit_note_after, emit_jump_table_data,
emit_note_copy, emit_note): Likewise.
* cfgrtl.c (compute_bb_for_insn, free_bb_for_insn): Likewise.
* bb-reorder.c (fix_crossing_unconditional_branches): Likewise.
(insert_section_boundary_note): Likewise.
* resource.c (init_resource_info, free_resource_info): Likewise.
* sel-sched.c (move_nop_to_previous_block): Likewise.
* config/c6x/c6x.c (gen_one_bundle): Likewise.
* reload1.c (reload): Use set_insn_deleted instead of SET_INSN_DELETED.
(delete_dead_insn): Likewise.
* combine.c (try_combine, distribute_notes): Likewise.
* lra.c (lra_set_insn_deleted): Likewise.
* ifcvt.c (cond_exec_process_insns): Likewise.
* recog.c (split_insn): Likewise.
* config/pa/pa.c (pa_output_lbranch, pa_output_millicode_call,
pa_output_call, pa_combine_instructions): Likewise.
* config/sh/sh.c (split_branches): Likewise.
Index: basic-block.h
===
--- basic-block.h (revision 197322)
+++ basic-block.h (working copy)
@@ -413,8 +413,6 @@ struct GTY(()) control_flow_graph {
/* The two blocks that are always in the cfg. */
#define NUM_FIXED_BLOCKS (2)
-#define set_block_for_insn(INSN, BB) (BLOCK_FOR_INSN (INSN) = BB)
-
extern void compute_bb_for_insn (void);
extern unsigned int free_bb_for_insn (void);
extern void update_bb_for_insn (basic_block);
Index: rtl.h
===
--- rtl.h (revision 197322)
+++ rtl.h (working copy)
@@ -824,8 +824,24 @@ extern void rtl_check_failed_flag (const char *, c
#define PREV_INSN(INSN)XEXP (INSN, 1)
#define NEXT_INSN(INSN)XEXP (INSN, 2)
-#define BLOCK_FOR_INSN(INSN) XBBDEF (INSN, 3)
+/* The basic block for an insn (or note).
+ Should be set/read with set_block_for_insn/BLOCK_FOR_INSN. */
+#define INSN_BASIC_BLOCK(INSN) XBBDEF (INSN, 3)
+/* Helper functions, to make debugging CFG changes easer.
+ BLOCK_FOR_INSN used to be a macro, hence the name in capitals. */
+static inline void
+set_block_for_insn (rtx insn, basic_block bb)
+{
+ INSN_BASIC_BLOCK (insn) = bb;
+}
+
+static inline basic_block
+BLOCK_FOR_INSN (const_rtx insn)
+{
+ return INSN_BASIC_BLOCK (insn);
+}
+
/* The body of an insn. */
#define PATTERN(INSN) XEXP (INSN, 4)
@@ -946,7 +962,6 @@ extern const char * const reg_note_name[];
/* Opaque data. */
#define NOTE_DATA(INSN)RTL_CHECKC1 (INSN, 4, NOTE)
#define NOTE_DELETED_LABEL_NAME(INSN) XCSTR (INSN, 4, NOTE)
-#define SET_INSN_DELETED(INSN) set_insn_deleted (INSN);
#define NOTE_BLOCK(INSN) XCTREE (INSN, 4, NOTE)
#define NOTE_EH_HANDLER(INSN) XCINT (INSN, 4, NOTE)
#define NOTE_BASIC_BLOCK(INSN) XCBBDEF (INSN, 4, NOTE)
Index: emit-rtl.c
===
--- emit-rtl.c (revision 197322)
+++ emit-rtl.c (working copy)
@@ -3674,7 +3674,7 @@ make_insn_raw (rtx pattern)
INSN_CODE (insn) = -1;
REG_NOTES (insn) = NULL;
INSN_LOCATION (insn) = curr_insn_location ();
- BLOCK_FOR_INSN (insn) = NULL;
+ set_block_for_insn (insn, NULL);
#ifdef ENABLE_RTL_CHECKING
if (insn
@@ -3707,7 +3707,7 @@ make_debug_insn_raw (rtx pattern)
INSN_CODE (insn) = -1;
REG_NOTES (insn) = NULL;
INSN_LOCATION (insn) = curr_insn_location ();
- BLOCK_FOR_INSN (insn) = NULL;
+ set_block_for_insn (insn, NULL);
return insn;
}
@@ -3727,7 +3727,7 @@ make_jump_insn_raw (rtx pattern)
REG_NOTES (insn) = NULL;
JUMP_LABEL (insn) = NULL;
INSN_LOCATION (insn) = curr_insn_location ();
- BLOCK_FOR_INSN (insn) = NULL;
+ set_block_for_insn (insn, NULL);
return insn;
}
@@ -3747,7 +3747,7 @@ make_call_insn_raw (rtx pattern)
REG_NOTES (insn) = NULL;
CALL_INSN_FUNCTION_USAGE (insn) = NULL;
INSN_LOCATION (insn) = curr_insn_location ();
- BLOCK_FOR_INSN (insn) = NULL;
+ set_block_for_insn (insn, NULL);
return insn;
}
@@ -4239,7 +4239,7 @@ emit_note_before (enum insn_note subtype, rtx befo
rtx note = rtx_alloc (NOTE);
INSN_UID (note) = cur_insn_uid++;
NOTE_KIND (note) = subtype;
- BLOCK_FOR_INSN (note
Re: [patch] cilkplus: Array notation for C patch
We need to do EXPR_LOCATION (array2) and then issue the error note: original mismatching rank at: array[:][:] = array2[:]; >>^ Yes. Or something similar... the note can be emitted with inform(). So you need to save the original rank location to emit the note. ...and we take the location information in "loc" and print that out in inform? Did I get it right? Yes. Provided "loc" is the location of the original location.
[patch] Rework libstdc++ contributing docs
At Gerald's suggestion this reworks the libstdc++ contributing docs to refer to the general GCC contributing docs instead of repeating the same info. * doc/xml/manual/appendix_contributing.xml: Remove broken link and defer to general documentation on contributing to GCC. * doc/html/*: Regenerate. Committed to trunk. diff --git a/libstdc++-v3/doc/xml/manual/appendix_contributing.xml b/libstdc++-v3/doc/xml/manual/appendix_contributing.xml index ab69093..f5dc67f 100644 --- a/libstdc++-v3/doc/xml/manual/appendix_contributing.xml +++ b/libstdc++-v3/doc/xml/manual/appendix_contributing.xml @@ -18,9 +18,12 @@ - The GNU C++ Library follows an open development model. Active - contributors are assigned maintainer-ship responsibility, and given - write access to the source repository. First time contributors + The GNU C++ Library is part of GCC and follows the same development model, + so the general rules for + http://www.w3.org/1999/xlink"; xlink:href="http://gcc.gnu.org/contribute.html";>contributing + to GCC apply. Active + contributors are assigned maintainership responsibility, and given + write access to the source repository. First-time contributors should follow this procedure: @@ -44,7 +47,7 @@ organization is http://www.w3.org/1999/xlink"; xlink:href="http://www.ansi.org";>ANSI. (And if you've already registered with them you can - http://www.w3.org/1999/xlink"; xlink:href="http://webstore.ansi.org/RecordDetail.aspx?sku=INCITS%2fISO%2fIEC+14882-2003";>buy the standard on-line.) + http://www.w3.org/1999/xlink"; xlink:href="http://webstore.ansi.org/RecordDetail.aspx?sku=INCITS%2fISO%2fIEC+14882-2012";>buy the standard on-line.) @@ -58,15 +61,6 @@ - The newsgroup dedicated to standardization issues is - comp.std.c++: the - http://www.w3.org/1999/xlink"; xlink:href="http://www.comeaucomputing.com/csc/faq.html";>FAQ - for this group is quite useful. - - - - - Peruse the http://www.w3.org/1999/xlink"; xlink:href="http://www.gnu.org/prep/standards/";>GNU Coding Standards, and chuckle when you hit the part @@ -95,10 +89,7 @@ Assignment - Small changes can be accepted without a copyright assignment form on - file. New code and additions to the library need completed copyright - assignment form on file at the FSF. Note: your employer may be required - to fill out appropriate disclaimer forms as well. + See the http://www.w3.org/1999/xlink"; xlink:href="http://gcc.gnu.org/contribute.html#legal";>legal prerequisites for all GCC contributions. @@ -119,12 +110,6 @@ - For more information about getting a copyright assignment, please see - http://www.w3.org/1999/xlink"; xlink:href="http://www.gnu.org/prep/maintain/html_node/Legal-Matters.html";>Legal - Matters. - - - Please contact Benjamin Kosnik at bkoz+ass...@redhat.com if you are confused about the assignment or have general licensing questions. When
Re: Fill more delay slots in conditional returns
On 04/01/2013 01:47 PM, Eric Botcazou wrote: I'm not sure what this is supposed to do. How is pat == target ever going to be true when ANY_RETURN_P (pat) is true? Isn't target supposed to be a CODE_LABEL or NULL? What am I missing? The simple_return change from Bernd. The JUMP_LABEL of a JUMP_INSN is now either a CODE_LABEL or a RETURN or a SIMPLE_RETURN. Ah, OK, yea, it makes perfect sense now. Approved. If you wanted to get ambitious, rewriting reorg.c to compute and use dependency links to drive its candidate selection instead of the insane tracking code it uses would be a huge step forward, both in terms of cleanliness. It'd also help compile-time performance; we do a lot of work to track resources that would be totally unnecessary. I don't work on targets with delay slots anymore, so it's not something I'm likely to ever tackle myself. jeff
Re: [PATCH] PR55033: Fix
On Sat, Mar 30, 2013 at 06:29:36PM -0400, David Edelsohn wrote:
> How can we make progress to get this patch committed on trunk, 4.8 and 4.7?
I have OKs for the config/i386/winnt.c and config/rs6000/rs6000.c
parts. I just need someone who is authorized to review patches to
varasm.c, and is willing to risk their reputation to at least comment
on the patch. Even a loud "NO" would be better than silence. As it
stands, I'm sorry I offered the patch, even though I still believe the
patch is correct from a design viewpoint, better than Sebastian's
patch that just tackled the specific case of ".sdata2". His patch is
obviously easier to review, we'd be OK on powerpc..
Let's try again with all the information in one place, and perhaps a
better explanation. http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55033
is about a problem on powerpc eabi, where gcc generates wrong section
flags (SECTION_WRITE) for an array initializer that should live in
.sdata2, a read-only section, and then hits an internal consistency
check because gcc gets the correct flags for .sdata2 in other cases.
See attached C testcase.
Sebastian offered a patch to default_section_type_flags that would
correct the flags, but his approach is going back to the mess we had
before Richard Henderson gave us categorize_decl_for_section. I made
the comment that we ought to use categorize_decl_for_section for
selecting the section flags, if we've used categorize_decl_for_section
to select the section name. Specifically, default_elf_select_section
should allow this to happen. rth agreed with this design approach in
http://gcc.gnu.org/ml/gcc-patches/2004-11/msg02487.html, but that
particular patch of mine was flawed, and I fixed a followup PR in a
non-ideal way.
So http://gcc.gnu.org/ml/gcc-patches/2012-10/msg02172.html is actually
a fix for a very old patch of mine. I can well understand a reviewer
looking at the patch and scratching their heads a little. Not so much
due to the latest patch, but because existing code in this area is
suspicious. For instance, you might wonder why it is correct to have
if (decl && !DECL_P (decl))
decl = NULL_TREE;
before calling get_section(). The answer is that get_section() is not
prepared to handle !DECL_P trees when reporting errors. Arguably it
should be modified to do that.
--
Alan Modra
Australia Development Lab, IBM
/* { dg-do compile { target powerpc*-*-eabi* powerpc*-*-elf* powerpc*-*-linux* } } */
/* { dg-require-effective-target ilp32 } */
/* { dg-options "-mcpu=8540 -msoft-float -meabi -msdata" } */
void f(void);
struct s {
int *p;
int *q;
};
extern int a;
extern const struct s c;
const struct s c = { &a, 0 };
void f(void)
{
char buf[4] = { 0, 1, 2, 3 };
}
Re: extend fwprop optimization
1.c attached. On Mon, Apr 1, 2013 at 10:43 PM, Wei Mi wrote: > I attached the patch.4 based on r197308. r197308 changes shift-and > type truncation from define_insn_and_split to define_insn. patch.4 > changes ix86_rtx_costs for shift-and type rtx to get the correct cost > for the result after the shift-and truncation. > > With the patch.1 ~ patch.4, fwprop extension could handle the > motivational case 1.c attached by removing all the redundent "x & 63" > operations. > > patch.1~patch.4 regression and bootstrap ok on > x86_64-unknown-linux-gnu. Is it ok for trunk? > > Thanks, > Wei. > > On Sun, Mar 17, 2013 at 12:15 AM, Wei Mi wrote: >> Hi, >> >> On Sat, Mar 16, 2013 at 3:48 PM, Steven Bosscher >> wrote: >>> On Tue, Mar 12, 2013 at 8:18 AM, Wei Mi wrote: For the motivational case, I need insn splitting to get the cost right. insn splitting is not very intrusive. All I need is to call split_insns func. >>> >>> It may not look very intrusive, but there's a lot happening in the >>> back ground. You're creating a lot of new RTL, and then just throw it >>> away again. You fake the compiler into thinking you're much deeper in >>> the pipeline than you really are. You're assuming there are no >>> side-effects other than that some insn gets split, but there are back >>> ends where splitters may have side-effects. >> >> Ok, then I will remove the split_insns call. >> >>> >>> Even though I've asked twice now, you still have not explained this >>> motivational case, except to say that there is one. *What* are you >>> trying to do, *what* is not happening without the splits, and *what* >>> happens if you split. Only if you explain that in a lot more detail >>> than "I have a motivational case" then we can look into what is a >>> proper solution. >> >> :-). Sorry, I didn't say it clearly. The motivational case is the one >> mentioned in the following posts (split_insns changes a << (b & 63) to >> a << b). >> http://gcc.gnu.org/ml/gcc/2013-01/msg00181.html >> http://gcc.gnu.org/ml/gcc-patches/2013-02/msg01144.html >> >> If I remove the split_insns call and related cost estimation >> adjustment, the fwprop 18-->22 and 18-->23 will punt, because fwprop >> here looks like a reverse process of cse, the total cost after fwprop >> change is increased. >> >> Def insn 18: >> Use insn 23 >> Use insn 22 >> >> If we include the split_insns cost estimation adjustment. >> extra benefit by removing def insn 18 = 5 >> change[0]: benefit = 0, verified - ok // The cost of insn 22 will >> not change after fwprop + insn splitting. >> change[1]: benefit = 0, verified - ok // The insn 23 is the same with >> insn 22 >> Total benefit is 5, fwprop will go on. >> >> If we remove the split_insns cost estimation adjustment. >> extra benefit by removing def insn 18 = 5 >> change[0]: benefit = -4, verified - ok // The costs of insn 22 and >> insn 23 will increase after fwprop. >> change[1]: benefit = -4, verified - ok // The insn 23 is the same >> with insn 22 >> Total benefit is -3, fwprop will punt. >> >> How about adding the (a << (b&63) ==> a << b) transformation in >> simplify_binary_operation_1, becuase (a << (b&63) ==> a << b) is a >> kind of architecture specific expr simplification? Then fwprop could >> do the propagation as I expect. >> >>> >>> The problem with some of the splitters is that they exist to break up >>> RTL from 'expand' to initially keep some pattern together to allow the >>> code transformation passes to handle the pattern as one instruction. >>> This made sense when RTL was the only intermediate representation and >>> splitting too early would inhibit some optimizations. But I would >>> expect most (if not all) such cases to be less relevant because of the >>> GIMPLE middle-end work. The only splitters you can trigger are the >>> pre-reload splitters (all the reload_completed conditions obviously >>> can't trigger if you're splitting from fwprop). Perhaps those >>> splitters can/should run earlier, or be made obsolete by expanding >>> directly to the post-splitting insns. >>> >>> Unfortunately, it's not possible to tell for your case, because you >>> haven't explained it yet... >>> >>> So how about keep split_insns and remove peephole in the cost estimation func? >>> >>> I'd strongly oppose this. I do not believe this is necessary, and I >>> think it's conceptually wrong. >>> >>> > What happens if you propagate into an insn that uses the same register > twice? Will the DU chains still be valid (I don't think that's > guaranteed)? I think the DU chains still be valid. If propagate into the insn that uses the same register twice, the two uses will be replaced when the first use is seen (propagate_rtx_1 will propagate all the occurrances of the same reg in the use insn). When the second use is seen, the df_use and use insn in its insn_info are still available. forward_propagate_into will early return after check
Re: extend fwprop optimization
On Tue, Apr 2, 2013 at 7:43 AM, Wei Mi wrote: > I attached the patch.4 based on r197308. r197308 changes shift-and > type truncation from define_insn_and_split to define_insn. patch.4 > changes ix86_rtx_costs for shift-and type rtx to get the correct cost > for the result after the shift-and truncation. > > With the patch.1 ~ patch.4, fwprop extension could handle the > motivational case 1.c attached by removing all the redundent "x & 63" > operations. > > patch.1~patch.4 regression and bootstrap ok on > x86_64-unknown-linux-gnu. Is it ok for trunk? > 2013-04-01 Wei Mi > > * config/i386/i386.c (ix86_rtx_costs): Set proper rtx cost for > ashl3_mask, *3_mask and *3_mask in i386.md. Patch 4 is OK for mainline and also for release branches that were changed by your previous i386.md patch. Thanks, Uros.
