Re: rfa: shrink rtl_bb_info
On Thu, May 3, 2012 at 6:05 PM, Michael Matz wrote:
> Hi,
>
> Richi made some good points about my last patch, related to struct
> rtl_bb_info. Even though it's not that important to shrink it (only few
> BBs are in RTL mode) there's no need to waste space. My last patch
> created an unused pointer-sized hole in RTL mode, which we can just as
> well use for members of rtl_bb_info. I've simply chosen the first, head_,
> because it entails few overall churn due to the use of BB_HEAD macro
> (header/footer are used directly), and because it's, well, the first :)
>
> We also thought about the visited member, but on review it's actually a
> silly member which we can also remove. There's only three uses of it
> currently:
>
> * cfglayout.c: set-only never used
> * bb-reorder: duplicate_computed_gotos: used as bool flag
> * bb-reorder: tracing, used not as flag, but as trace indicator
>
> The first is useless, the second can be replaced by a proper bb->flags
> (BB_VISITED, newly introduced) and the third by an field in the
> side data structure that bb-reorder already employs. Et voila, one word
> less for rtl_bb_info, making it overall one word smaller than before my
> last patch :)
>
> The churn in the patch is due to the il.rtl --> il.x.rtl renaming. I wish
> we would accept anonymous structs for our sources, but we don't.
Well, as you touch all places that refer to header/footer why not
introduce macros to access them ...
> Currently regstrapping on x86_64-linux, okay if that passes?
Ok with the above change.
Thanks,
Richard.
>
> Ciao,
> Michael.
> -
> 2012-05-03 Michael Matz
>
> * basic-block.h (struct rtl_bb_info): Remove visited member and
> move head_ member to ...
> (struct basic_block_def.basic_block_il_dependent): ... the new
> member x, replacing but containing old member rtl.
> (enum bb_flags): New BB_VISITED flag.
>
> * jump.c (mark_all_labels): Adjust.
> * cfgcleanup.c (try_optimize_cfg): Adjust.
> * cfglayout.c (record_effective_endpoints): Adjust.
> (relink_block_chain): Ditto (and don't fiddle with visited).
> (fixup_reorder_chain): Adjust.
> (fixup_fallthru_exit_predecessor): Ditto.
> (cfg_layout_duplicate_bb): Ditto.
> * combine.c (update_cfg_for_uncondjump): Adjust.
> * bb-reorder.c (struct bbro_basic_block_data_def): Add visited
> member.
> (bb_visited_trace): New accessor.
> (mark_bb_visited): Move in front.
> (rotate_loop): Use bb_visited_trace.
> (find_traces_1_round): Ditto.
> (emit_barrier_after): Ditto.
> (copy_bb): Ditto, and initialize visited on resize.
> (reorder_basic_blocks): Initize visited member.
> (duplicate_computed_gotos): Clear bb flags at start, use
> BB_VISITED flags.
>
> * cfgrtl.c (try_redirect_by_replacing_jump): Adjust.
> (rtl_verify_flow_info_1): Ditto.
> (cfg_layout_split_block): Ditto.
> (cfg_layout_delete_block): Ditto.
> (cfg_layout_merge_blocks): Ditto.
> (init_rtl_bb_info): Adjust and initialize il.x.head_ member.
>
> Index: jump.c
> ===
> --- jump.c (revision 187098)
> +++ jump.c (working copy)
> @@ -275,13 +275,13 @@ mark_all_labels (rtx f)
> /* In cfglayout mode, there may be non-insns between the
> basic blocks. If those non-insns represent tablejump data,
> they contain label references that we must record. */
> - for (insn = bb->il.rtl->header; insn; insn = NEXT_INSN (insn))
> + for (insn = bb->il.x.rtl->header; insn; insn = NEXT_INSN (insn))
> if (INSN_P (insn))
> {
> gcc_assert (JUMP_TABLE_DATA_P (insn));
> mark_jump_label (PATTERN (insn), insn, 0);
> }
> - for (insn = bb->il.rtl->footer; insn; insn = NEXT_INSN (insn))
> + for (insn = bb->il.x.rtl->footer; insn; insn = NEXT_INSN (insn))
> if (INSN_P (insn))
> {
> gcc_assert (JUMP_TABLE_DATA_P (insn));
> Index: cfgcleanup.c
> ===
> --- cfgcleanup.c (revision 187098)
> +++ cfgcleanup.c (working copy)
> @@ -2586,21 +2586,21 @@ try_optimize_cfg (int mode)
>
> if (current_ir_type () == IR_RTL_CFGLAYOUT)
> {
> - if (b->il.rtl->footer
> - && BARRIER_P (b->il.rtl->footer))
> + if (b->il.x.rtl->footer
> + && BARRIER_P (b->il.x.rtl->footer))
> FOR_EACH_EDGE (e, ei, b->preds)
> if ((e->flags & EDGE_FALLTHRU)
> - && e->src->il.rtl->footer == NULL)
> + && e->src->il.x.rtl->footer == NULL)
Re: [PATCH] Improve COND_EXPR expansion
On Tue, May 1, 2012 at 9:45 AM, Richard Henderson wrote: > On 04/30/2012 08:22 PM, Andrew Pinski wrote: >> >> * expr.c (convert_tree_comp_to_rtx): New function. >> (expand_expr_real_2): Try using conditional moves for COND_EXPRs if they >> exist. >> * config/i386/i386.c (ix86_expand_int_movcc): Disallow comparison >> modes of DImode for 32bits and TImode. > > > Looks ok, modulo some unwrapped long lines. > Those lines might not be so long if this were split out as a subroutine... > > I'll not quibble about that though... Here is a new patch which splits this out into a subroutine which reduces the long lines. I added some extra comments that might make some stuff like the extra can_conditionally_move_p check. OK? Bootstrapped and tested on x86_64-linux-gnu. Thanks, Andrew Pinski ChangeLog: * expr.c (get_def_for_expr_class): New function. (convert_tree_comp_to_rtx): New function. (expand_cond_expr_using_cmove): New function. (expand_expr_real_2 ): Call expand_cond_expr_using_cmove first and return if it succeeds. Remove the check for HAVE_conditional_move since we should have already converted it to a conditional move. * config/i386/i386.c (ix86_expand_int_movcc): Disallow comparison modes of DImode for 32bits and TImode.
Re: [PATCH] Improve COND_EXPR expansion
On Fri, May 4, 2012 at 12:29 AM, Andrew Pinski
wrote:
> On Tue, May 1, 2012 at 9:45 AM, Richard Henderson wrote:
>> On 04/30/2012 08:22 PM, Andrew Pinski wrote:
>>>
>>> * expr.c (convert_tree_comp_to_rtx): New function.
>>> (expand_expr_real_2): Try using conditional moves for COND_EXPRs if they
>>> exist.
>>> * config/i386/i386.c (ix86_expand_int_movcc): Disallow comparison
>>> modes of DImode for 32bits and TImode.
>>
>>
>> Looks ok, modulo some unwrapped long lines.
>> Those lines might not be so long if this were split out as a subroutine...
>>
>> I'll not quibble about that though...
>
> Here is a new patch which splits this out into a subroutine which
> reduces the long lines. I added some extra comments that might make
> some stuff like the extra can_conditionally_move_p check.
>
> OK? Bootstrapped and tested on x86_64-linux-gnu.
This time with the patch.
-- Andrew
>
> Thanks,
> Andrew Pinski
>
> ChangeLog:
> * expr.c (get_def_for_expr_class): New function.
> (convert_tree_comp_to_rtx): New function.
> (expand_cond_expr_using_cmove): New function.
> (expand_expr_real_2 ): Call
> expand_cond_expr_using_cmove first and return if it succeeds.
> Remove the check for HAVE_conditional_move since we should have
> already converted it to a conditional move.
>
> * config/i386/i386.c (ix86_expand_int_movcc): Disallow comparison
> modes of DImode for 32bits and TImode.
Index: gcc/expr.c
===
--- gcc/expr.c (revision 187138)
+++ gcc/expr.c (working copy)
@@ -2346,6 +2346,26 @@ get_def_for_expr (tree name, enum tree_c
return def_stmt;
}
+
+/* Return the defining gimple statement for SSA_NAME NAME if it is an
+ assigment and the class of the expresion on the RHS is CLASS. Return
+ NULL otherwise. */
+
+static gimple
+get_def_for_expr_class (tree name, enum tree_code_class tclass)
+{
+ gimple def_stmt;
+
+ if (TREE_CODE (name) != SSA_NAME)
+return NULL;
+
+ def_stmt = get_gimple_for_ssa_name (name);
+ if (!def_stmt
+ || TREE_CODE_CLASS (gimple_assign_rhs_code (def_stmt)) != tclass)
+return NULL;
+
+ return def_stmt;
+}
/* Determine whether the LEN bytes generated by CONSTFUN can be
@@ -7344,6 +7364,64 @@ highest_pow2_factor_for_target (const_tr
return MAX (factor, talign);
}
+/* Convert the tree comparision code TCODE to the rtl one where the
+ signedness is UNSIGNEDP. */
+
+static enum rtx_code
+convert_tree_comp_to_rtx (enum tree_code tcode, int unsignedp)
+{
+ enum rtx_code code;
+ switch (tcode)
+{
+case EQ_EXPR:
+ code = EQ;
+ break;
+case NE_EXPR:
+ code = NE;
+ break;
+case LT_EXPR:
+ code = unsignedp ? LTU : LT;
+ break;
+case LE_EXPR:
+ code = unsignedp ? LEU : LE;
+ break;
+case GT_EXPR:
+ code = unsignedp ? GTU : GT;
+ break;
+case GE_EXPR:
+ code = unsignedp ? GEU : GE;
+ break;
+case UNORDERED_EXPR:
+ code = UNORDERED;
+ break;
+case ORDERED_EXPR:
+ code = ORDERED;
+ break;
+case UNLT_EXPR:
+ code = UNLT;
+ break;
+case UNLE_EXPR:
+ code = UNLE;
+ break;
+case UNGT_EXPR:
+ code = UNGT;
+ break;
+case UNGE_EXPR:
+ code = UNGE;
+ break;
+case UNEQ_EXPR:
+ code = UNEQ;
+ break;
+case LTGT_EXPR:
+ code = LTGT;
+ break;
+
+default:
+ gcc_unreachable ();
+}
+ return code;
+}
+
/* Subroutine of expand_expr. Expand the two operands of a binary
expression EXP0 and EXP1 placing the results in OP0 and OP1.
The value may be stored in TARGET if TARGET is nonzero. The
@@ -7782,6 +7860,96 @@ expand_expr_real (tree exp, rtx target,
return ret;
}
+/* Try to expand the conditional expression which is represented by
+ TREEOP0 ? TREEOP1 : TREEOP2 using conditonal moves. If succeseds
+ return the rtl reg which repsents the result. Otherwise return NULL_RTL.
*/
+
+static rtx
+expand_cond_expr_using_cmove (tree treeop0, tree treeop1, tree treeop2)
+{
+#ifdef HAVE_conditional_move
+ rtx insn;
+ rtx op00, op01, op1, op2;
+ enum rtx_code comparison_code;
+ enum machine_mode comparison_mode;
+ gimple srcstmt;
+ rtx temp;
+ tree type = TREE_TYPE (treeop1);
+ int unsignedp = TYPE_UNSIGNED (type);
+ enum machine_mode mode = TYPE_MODE (type);
+
+ temp = assign_temp (type, 0, 0, 1);
+
+ /* If we cannot do a conditional move on the mode, try doing it
+ with the promoted mode. */
+ if (!can_conditionally_move_p (mode))
+mode = promote_mode (type, mode, &unsignedp);
+
+ if (!can_conditionally_move_p (mode))
+return NULL_RTX;
+
+ start_sequence ();
+ expand_operands (treeop1, treeop2,
+ temp, &op1, &op2, EXPAND_NORMAL);
+
+ if (TREE_CODE (treeop0) == SSA_NAME
+ && (srcstmt = get_def_for_expr_class (treeop0, tcc_comparison)))
+{
+ tree type = TREE_TYPE (gimple_assign_rhs1 (srcstmt));
+ enum tree_code cmpcode = gimple_assig
RE: [PING] iwMMXt patches
> -Original Message- > From: Matt Turner [mailto:matts...@gmail.com] > To: Xinyu Qi > Cc: Ramana Radhakrishnan; GCC Patches > Subject: Re: [PING] iwMMXt patches > > On Thu, May 3, 2012 at 12:59 AM, Xinyu Qi wrote: > >> From: Matt Turner [mailto:matts...@gmail.com] > >> To: Xinyu Qi > >> Cc: Ramana Radhakrishnan; GCC Patches > >> Subject: Re: [PING] iwMMXt patches > >> > >> On Tue, Apr 17, 2012 at 4:17 PM, Matt Turner > wrote: > >> > Are these patches ready to go in? It looks like they were ack'd. > >> > > >> > http://gcc.gnu.org/ml/gcc-patches/2011-10/msg01815.html > >> > http://gcc.gnu.org/ml/gcc-patches/2011-10/msg01817.html > >> > http://gcc.gnu.org/ml/gcc-patches/2011-10/msg01816.html > >> > http://gcc.gnu.org/ml/gcc-patches/2011-10/msg01818.html > >> > http://gcc.gnu.org/ml/gcc-patches/2011-10/msg01819.html > >> > > >> > We (OLPC) will need these patches for reasonable iwMMXt performance > >> > and the ability to use VFP and iwMMXt together. > >> > > >> > Thanks, > >> > Matt > >> > >> Xinyu, > >> > >> With these patches I don't see a new -mcpu flag. Isn't a tune/cpu flag > >> the normal way to activate this code? > >> > >> Other .md files have statements like (eq_attr "tune" "cortexa8"), but > >> I don't see how to turn on the marvell-f-iwmmxt attribute, ie (eq_attr > >> "marvell_f_iwmmxt" "yes"). > >> > >> Please let me know. > >> > >> Thanks, > >> Matt > > > > Hi Matt, > > > > I updated the patches several months ago by following the review opinions > form Richard Earnshaw [richard.earns...@arm.com] > > (unfortunately, no further feedback) > > The newest patches are > > http://gcc.gnu.org/ml/gcc-patches/2011-12/msg01787.html > > http://gcc.gnu.org/ml/gcc-patches/2011-12/msg01788.html > > http://gcc.gnu.org/ml/gcc-patches/2011-12/msg01789.html > > http://gcc.gnu.org/ml/gcc-patches/2011-12/msg01786.html > > http://gcc.gnu.org/ml/gcc-patches/2011-12/msg01599.html > > The main discussion is in > > http://gcc.gnu.org/ml/gcc-patches/2011-12/msg01786.html > > > > No new -mcpu flag is introduced in the patches. You can simply turn on > marvell-f-iwmmxt by -mcpu=iwmmxt2(or -march=iwmmxt2). > > (Of course it is odd to treat the "iwmmxt2" as a name of cpu) > > > > > > Thanks, > > Xinyu > > Thanks for the email, Xinyu! > > We (OLPC) will test the patches and then I'll resubmit them to > gcc-patches@ and try to get them included. They're definitely needed > for us, since they fix PR35294 (iwmmxt shift and logical intrinsics > are broken). > > By the way, are there patches for add general instruction scheduling > support for Marvell CPUs like the Armada 610? > > Thanks again, > > Matt Hi Matt, Thank you very much! It is our pleasure to contribute some Marvell cores' pipeline description to GCC, and we are preparing the patch right now. We would submit it as soon as it is ready. (It might take several days to work out). Thanks again, Xinyu
Re: PR c++/24985 caret diagnostics for macro expansions
Manuel López-Ibáñez writes:
> This patch enables caret diagnostics for macro expansions. So now we will get:
>
> /home/manuel/macro-clang.c:2:91: error: invalid operands to binary <
> (have ‘struct mystruct’ and ‘float’)
> #define MYMAX(A,B) __extension__ ({ __typeof__(A) __a = (A);
> __typeof__(B) __b = (B); __a < __b ? __b : __a; })
>
> ^
> /home/manuel/macro-clang.c:2:91: note: in expansion of macro 'MYMAX'
> #define MYMAX(A,B) __extension__ ({ __typeof__(A) __a = (A);
> __typeof__(B) __b = (B); __a < __b ? __b : __a; })
>
> ^
> /home/manuel/macro-clang.c:9:3: note: expanded from here
>MYMAX(p, f);
>^
>
> In my opinion, the macro unwinder is too verbose. I think the output
> should be just:
>
> 2.91: error: invalid...
> #define MYMAX(A,B) __extension__ ({ __typeof__(A) __a = (A);
> __typeof__(B) __b = (B); __a < __b ? __b : __a; })
>
> ^
> 9.3: note: in expansion of macro 'MYMAX'
>MYMAX(p, f);
>^
Yes, I think that would be cool. I wanted to look into that once I am
done with what I am doing, unless someone beats me to it, of course.
:-)
--
Dodji
Re: improve collect2 to preserve a RO output as the linker does
>> * collect2.c (may_unlink_output_file): New global. ... > This is OK. Installed, thanks :)
Re: fix spec function evaluations part of an outer string construction
On May 3, 2012, at 22:50 , Joseph S. Myers wrote: >> -* gcc.c (eval_spec_function): Finalize/restore the current string >> obstack state as part of the context push/pop operations. > > OK. Installed, thanks :)
RE: Patch to enable --with-multilib-list for arm-none-eabi target
Hello Joseph, Can you help to review this patch? The basic idea is to provide a set of predefined Multilib and enable user to select the required ones from --with-multilib-list. BR, Terry > -Original Message- > From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- > ow...@gcc.gnu.org] On Behalf Of Terry Guo > Sent: Wednesday, May 02, 2012 1:49 PM > To: gcc-patches@gcc.gnu.org > Subject: Patch to enable --with-multilib-list for arm-none-eabi target > > Hello, > > This patch provides a bunch of predefined MULTILIB for various arm- > none-eabi targets. The MULTILIB_EXCEPTIONS isn't needed because the > patch takes advantage of the new term MULTILIB_REQUIRED which is > introduced at http://gcc.gnu.org/ml/gcc-patches/2012-04/msg00975.html. > The existing configure option --with-multilib-list is enabled as an > interface for user to specify needed libraries from the predefined set. > For example if only need to build Multilib for targets armv7-m and > armv7e-m, then the gcc can be configured with --with-multilib- > list=armv7-m,armv7e-m. > > Please note that the predefined Multilib set in this patch doesn't > cover all possible cases like big-endian version Multilib. User can > extend this set and just build needed ones through --with-multili-list. > So far this patch can recognize armv6-m, armv7-m, armv7e-m, armv7-r and > armv7-a as the options used in --with-multilib-list. > > Tested on trunk for arm-none-eabi target. Is it ok to trunk? > > BR, > Terry > > 2012-05-02 Terry Guo > > * config.gcc: Include more MULTILIB implementations for ARM EABI. > * config/arm/t-mlibs: New file to define more MULTILIB > implementations for ARM targets. > * configure.ac: Enable --with-multilib-list for ARM > and export it for being used in Makefile.in. > * configure: Regenerated. > * Makefile.in: Import configure option --with-multilib-list.
Re: patch ping: Add static branch predict heuristic of comparing IV to loop_bound variable
> gcc/ChangeLog
> 2012-05-04 Dehao Chen
>
> * predict.c (find_qualified_ssa_name): New
> (find_ssa_name_in_expr): New
> (find_ssa_name_in_assign_stmt): New
> (is_comparison_with_loop_invariant_p): New
> (is_bound_expr_similar): New
> (predict_iv_comparison): New
> (predict_loops): Add heuristic for loop-nested branches that compare an
> induction variable to a loop bound variable.
> * predict.def (PRED_LOOP_IV_COMPARE): New macro
>
> gcc/testsuite/ChangeLog
> 2012-05-04 Dehao Chen
>
> * gcc.dg/predict-1.c: Check if LOOP_IV_COMPARE static predict
> heuristic is working properly.
> * gcc.dg/predict-2.c: Likewise.
> * gcc/dg/predict-3.c: Likewise.
> * gcc/dg/predict-4.c: Likewise.
> * gcc/dg/predict-5.c: Likewise.
> * gcc/dg/predict-6.c: Likewise.
>
> Index: gcc/predict.c
> ===
> --- gcc/predict.c (revision 187140)
> +++ gcc/predict.c (working copy)
> @@ -946,6 +946,358 @@
> }
> }
>
> +/* Check if T1 and T2 satisfy the IV_COMPARE condition.
> + Return the SSA_NAME if the condition satisfies, NULL otherwise.
> +
> + T1 and T2 should be one of the following cases:
> + 1. T1 is SSA_NAME, T2 is NULL
> + 2. T1 is SSA_NAME, T2 is INTEGER_CST between [-4, 4]
> + 3. T2 is SSA_NAME, T1 is INTEGER_CST between [-4, 4] */
> +
> +static tree
> +strips_small_constant (tree t1, tree t2)
> +{
> + tree ret = NULL;
> + int value = 0;
> +
> + if (!t1)
> +return NULL;
> + else if (TREE_CODE (t1) == SSA_NAME)
> +ret = t1;
> + else if (TREE_CODE (t1) == INTEGER_CST && host_integerp (t1, 0))
host_integer_p will check TREE_CODE (t1) == INTEGER_CST for you.
> + /* If loop bound, base and compare bound are all constents, we can
constants.
> + calculate the probability directly. */
> + if (TREE_CODE (loop_bound_var) == INTEGER_CST
> + && TREE_CODE (compare_var) == INTEGER_CST
> + && TREE_CODE (compare_base) == INTEGER_CST
> + && host_integerp (loop_bound_var, 0)
> + && host_integerp (compare_var, 0)
> + && host_integerp (compare_base, 0))
Again host_integerp will test that for you.
> + probability = (double) REG_BR_PROB_BASE * compare_count / loop_count;
> + predict_edge (then_edge, PRED_LOOP_IV_COMPARE, probability);
> + return;
> +}
> +
> + if (expr_coherent_p (loop_bound_var, compare_var))
You probably want to thave two predictor, one when the probability is known
declared as first match predictor with high probability and one when probability
is unknown combined the usual way...
Otherwise the patch seems good to me.
Honza
Re: [Patch, libfortran] Fix handling of temporary files
Dear Janne,
sorry for the really slow review.
Janne Blomqvist wrote:
> the attached patch implements some fixes for handling STATUS="SCRATCH" files.
...
> Currently, we check [...] but not the POSIX and GNU standard TMPDIR [...]
> If the program is privileged, we shouldn't trust path style environment
> variables.
Good chatch!
+On the MingW target, the directory returned by the @code{GetTempPath}
+#define P_tmpdir _P_tmpdir /* MingW */
It's spelt MinGW ("Minimalist GNU for Windows").
* GFORTRAN_STDERR_UNIT:: Unit number for standard error
-* GFORTRAN_TMPDIR:: Directory for scratch files
+* TMPDIR:: Directory for scratch files
* GFORTRAN_UNBUFFERED_ALL:: Do not buffer I/O for all units.
I would re-order the list and place TMPDIR either at the top or at
the bottom. Given that it is a more useful environment variable,
placing it at the top might make sense.
(You should then also move the actual text.)
+ /* Check for special case that tempdir contains slash
or backslash at end. */
(second line looks wrongly indented.)
Additionally, I would add a couple of articles:
"Check for the special case that tempdir contains a slash
or backslash at the end"
or, better,
"Check for the special case that tempdir ends with a slash
or backslash"
+tempfile_open (const char * tempdir, char ** fname)
GNU style: No spaces after "*".
+}
+
+/* tempfile()-- Generate a temporary filename for a scratch file and
Second empty line missing.
+ if (!tempdir)
+return -1;
...
+ size_t tempdirlen = strlen (tempdir);
I am surprised that it compiles. I thought GCC is compiled with C89
settings such that variables have to be declared before the the
expression statements start. Did you check whether it compiles with
any warnings?
Otherwise, the patch looks fine.
Thanks for the patch!
Tobias
PS: Side note, I find the document produced by the ISO/IEC Project 22.24772:
"Guidance for Avoiding Vulnerabilities through Language Selection"
useful; while it is rather lengthy and not that readably written, it still
contains some helpful bits. However, it does not really address the
environment variable issue which you mentioned (ignoring 6.47.1 and
3.1.2.12 which are only loosely related).
Cf.
http://gcc.gnu.org/wiki/GFortranStandards#ISO.2BAC8-IEC_Project_22.24772:_Guidance_for_Avoiding_Vulnerabilities_through_Language_Selection_and_Use
Re: [patch] Fix PR target/48496 #2
Jakub, > 2012-04-26 Eric Botcazou > > PR target/48496 > * recog.c (constrain_operands): If extra constraints are present, > accept pseudo-registers with equivalent memory locations during reload. Is this version OK with you? -- Eric Botcazou
Re: [patch] Fix PR target/48496 #2
On Fri, May 04, 2012 at 12:19:39PM +0200, Eric Botcazou wrote: > > 2012-04-26 Eric Botcazou > > > > PR target/48496 > > * recog.c (constrain_operands): If extra constraints are present, > > accept pseudo-registers with equivalent memory locations during reload. > > Is this version OK with you? Ok. Jakub
[PATCH] Fix PR53214
This fixes PR53214 - we should not mark flto-report, fwpa or
ftrans as Optimization, otherwise you can control them with
the optimize attribute and they get munged when streamed to
ltrans units from WPA.
Will commit this to trunk and 4.7 after giving it a round of testing.
Richard.
2012-05-04 Richard Guenther
* common.opt (flto-report): Do not mark as Optimization.
lto/
* lang.opt (fwpa): Do not mark as Optimization.
(fltrans): Likewise.
* gcc.dg/lto/pr53214_0.c: New testcase.
Index: gcc/common.opt
===
*** gcc/common.opt (revision 187148)
--- gcc/common.opt (working copy)
*** Common Joined RejectNegative UInteger Va
*** 1424,1430
-flto-compression-level= Use zlib compression level for
IL
flto-report
! Common Report Var(flag_lto_report) Init(0) Optimization
Report various link-time optimization statistics
fmath-errno
--- 1424,1430
-flto-compression-level= Use zlib compression level for
IL
flto-report
! Common Report Var(flag_lto_report) Init(0)
Report various link-time optimization statistics
fmath-errno
Index: gcc/lto/lang.opt
===
*** gcc/lto/lang.opt(revision 187148)
--- gcc/lto/lang.opt(working copy)
*** Language
*** 25,31
LTO
fltrans
! LTO Report Var(flag_ltrans) Optimization
Run the link-time optimizer in local transformation (LTRANS) mode.
fltrans-output-list=
--- 25,31
LTO
fltrans
! LTO Report Var(flag_ltrans)
Run the link-time optimizer in local transformation (LTRANS) mode.
fltrans-output-list=
*** LTO Joined Var(ltrans_output_list)
*** 33,39
Specify a file to which a list of files output by LTRANS is written.
fwpa
! LTO Driver Report Var(flag_wpa) Optimization
Run the link-time optimizer in whole program analysis (WPA) mode.
fresolution=
--- 33,39
Specify a file to which a list of files output by LTRANS is written.
fwpa
! LTO Driver Report Var(flag_wpa)
Run the link-time optimizer in whole program analysis (WPA) mode.
fresolution=
Index: gcc/testsuite/gcc.dg/lto/pr53214_0.c
===
--- gcc/testsuite/gcc.dg/lto/pr53214_0.c(revision 0)
+++ gcc/testsuite/gcc.dg/lto/pr53214_0.c(revision 0)
@@ -0,0 +1,8 @@
+/* { dg-lto-do run } */
+
+double a(double) __attribute__ ((optimize(1), used));
+double a(double r)
+{
+ return r;
+}
+int main () { return 0; }
[patch] Fix plugin-enabled LTO with cross-native build
Hi,
we ran into a problem with plugin-enabled LTO and a cross-native build of the
compiler for Windows: collect2.c isn't able to find the linker in LTO mode.
The reason is that, at configure time, PLUGIN_LD is set to the $gcc_cv_ld, but
the machinery in collect2.c is set up for suffixes instead:
static const char *const ld_suffix= "ld";
static const char *const plugin_ld_suffix = PLUGIN_LD;
static const char *const real_ld_suffix = "real-ld";
static const char *const collect_ld_suffix = "collect-ld";
static const char *const nm_suffix= "nm";
static const char *const gnm_suffix = "gnm";
#ifdef LDD_SUFFIX
static const char *const ldd_suffix = LDD_SUFFIX;
#endif
static const char *const strip_suffix = "strip";
static const char *const gstrip_suffix = "gstrip";
so the attached patch attempts to pass a suffix in this case too.
Tested on i586-suse-linux and with the aforementioned cross-native build, OK
for the mainline?
2012-05-04 Eric Botcazou
* configure.ac (PLUGIN_LD): Rename into...
(PLUGIN_LD_SUFFIX): ...this and strip the target_alias triplet.
* config.in: Regenerate.
* configure: Likewise.
* collect2.c (main): Set plugin_ld_suffix to PLUGIN_LD_SUFFIX.
--
Eric Botcazou
Index: configure.ac
===
--- configure.ac (revision 187074)
+++ configure.ac (working copy)
@@ -2001,15 +2001,15 @@ else
fi])
ORIGINAL_PLUGIN_LD_FOR_TARGET=$gcc_cv_ld
-PLUGIN_LD=`basename $gcc_cv_ld`
+PLUGIN_LD_SUFFIX=`basename $gcc_cv_ld | sed -e "s,$target_alias-,,"`
AC_ARG_WITH(plugin-ld,
[AS_HELP_STRING([[--with-plugin-ld=[ARG]]], [specify the plugin linker])],
[if test x"$withval" != x; then
ORIGINAL_PLUGIN_LD_FOR_TARGET="$withval"
- PLUGIN_LD="$withval"
+ PLUGIN_LD_SUFFIX=`echo $withval | sed -e "s,$target_alias-,,"`
fi])
AC_SUBST(ORIGINAL_PLUGIN_LD_FOR_TARGET)
-AC_DEFINE_UNQUOTED(PLUGIN_LD, "$PLUGIN_LD", [Specify plugin linker])
+AC_DEFINE_UNQUOTED(PLUGIN_LD_SUFFIX, "$PLUGIN_LD_SUFFIX", [Specify plugin linker])
# Check to see if we are using gold instead of ld
AC_MSG_CHECKING(whether we are using gold)
Index: collect2.c
===
--- collect2.c (revision 187074)
+++ collect2.c (working copy)
@@ -1018,7 +1018,7 @@ int
main (int argc, char **argv)
{
static const char *const ld_suffix = "ld";
- static const char *const plugin_ld_suffix = PLUGIN_LD;
+ static const char *const plugin_ld_suffix = PLUGIN_LD_SUFFIX;
static const char *const real_ld_suffix = "real-ld";
static const char *const collect_ld_suffix = "collect-ld";
static const char *const nm_suffix = "nm";
Re: [Patch, libfortran] Fix handling of temporary files
On Fri, May 4, 2012 at 12:49 PM, Tobias Burnus
wrote:
> Dear Janne,
>
> sorry for the really slow review.
>
> Janne Blomqvist wrote:
>> the attached patch implements some fixes for handling STATUS="SCRATCH" files.
> ...
>> Currently, we check [...] but not the POSIX and GNU standard TMPDIR [...]
>> If the program is privileged, we shouldn't trust path style environment
>> variables.
>
> Good chatch!
>
>
> +On the MingW target, the directory returned by the @code{GetTempPath}
> +#define P_tmpdir _P_tmpdir /* MingW */
>
> It's spelt MinGW ("Minimalist GNU for Windows").
Will fix.
> * GFORTRAN_STDERR_UNIT:: Unit number for standard error
> -* GFORTRAN_TMPDIR:: Directory for scratch files
> +* TMPDIR:: Directory for scratch files
> * GFORTRAN_UNBUFFERED_ALL:: Do not buffer I/O for all units.
>
> I would re-order the list and place TMPDIR either at the top or at
> the bottom. Given that it is a more useful environment variable,
> placing it at the top might make sense.
> (You should then also move the actual text.)
Good point. Will do.
> + /* Check for special case that tempdir contains slash
> or backslash at end. */
>
> (second line looks wrongly indented.)
>
> Additionally, I would add a couple of articles:
> "Check for the special case that tempdir contains a slash
> or backslash at the end"
> or, better,
> "Check for the special case that tempdir ends with a slash
> or backslash"
That sentence was already present before the patch, but I'll fix that as well.
> +tempfile_open (const char * tempdir, char ** fname)
>
> GNU style: No spaces after "*".
Hmm, I thought that GNU style was that there should be spaces both
before and after the "*". But now that I look at
https://www.gnu.org/prep/standards/standards.html#Formatting it seems
that the "*" should indeed be grouped together with the variable name.
Which is nice, since I never liked what I thought the GNU style was!
;-) I just wonder where I got that idea from..
> +}
> +
> +/* tempfile()-- Generate a temporary filename for a scratch file and
>
> Second empty line missing.
Right, will fix.
> + if (!tempdir)
> + return -1;
> ...
> + size_t tempdirlen = strlen (tempdir);
>
> I am surprised that it compiles. I thought GCC is compiled with C89
> settings such that variables have to be declared before the the
> expression statements start. Did you check whether it compiles with
> any warnings?
libgfortran is compiled with -std=gnu99, so this is Ok. We use C99
features quite a lot in libgfortran, FWIW.
IIRC there were no warnings.
> Otherwise, the patch looks fine.
>
> Thanks for the patch!
Thanks for the review!
> Tobias
>
> PS: Side note, I find the document produced by the ISO/IEC Project 22.24772:
> "Guidance for Avoiding Vulnerabilities through Language Selection"
> useful; while it is rather lengthy and not that readably written, it still
> contains some helpful bits. However, it does not really address the
> environment variable issue which you mentioned (ignoring 6.47.1 and
> 3.1.2.12 which are only loosely related).
> Cf.
> http://gcc.gnu.org/wiki/GFortranStandards#ISO.2BAC8-IEC_Project_22.24772:_Guidance_for_Avoiding_Vulnerabilities_through_Language_Selection_and_Use
Some info wrt secure tmp file handling and env variables:
http://www.dwheeler.com/secure-programs/Secure-Programs-HOWTO/avoid-race.html#TEMPORARY-FILES
(see code example, which does more or less what the secure_getenv
fallback in the patch does. Actually, that entire HOWTO is interesting
in general)
https://www.gnu.org/software/libc/manual/html_node/Temporary-Files.html
--
Janne Blomqvist
[PATCH] Fix PR50602
-freg-struct-return and -fpcc-struct-return are two common ABI
options which have to be consistent in all input files when
doing LTO (inconsistencies may lead to ICEs and "miscompiles").
Their setting needs to be determined from the linker inputs
and its best to ignore those on the linker command line
(unless we want to bother diagnosing mismatches there).
The following implements this (we'll have a hard time diagnosing
frontend / target specific ABI option mismatches - at least without
a new magic option file annotation).
I'll commit this after some testing.
Thanks,
Richard.
2012-05-04 Richard Guenther
PR lto/50602
* lto-wrapper.c (merge_and_complain): Complain about mismatches
of -freg-struct-return and -fpcc-struct-return.
(run_gcc): Pass through -freg-struct-return and -fpcc-struct-return
from the input file options and ignore those from the link
command line.
Index: gcc/lto-wrapper.c
===
--- gcc/lto-wrapper.c (revision 187148)
+++ gcc/lto-wrapper.c (working copy)
@@ -414,6 +414,16 @@ merge_and_complain (struct cl_decoded_op
if (j == *decoded_options_count)
append_option (decoded_options, decoded_options_count, foption);
break;
+
+ case OPT_freg_struct_return:
+ case OPT_fpcc_struct_return:
+ for (j = 0; j < *decoded_options_count; ++j)
+ if ((*decoded_options)[j].opt_index == foption->opt_index)
+ break;
+ if (j == *decoded_options_count)
+ fatal ("Option %s not used consistently in all LTO input files",
+ foption->orig_option_with_args_text);
+ break;
}
}
}
@@ -558,6 +568,8 @@ run_gcc (unsigned argc, char *argv[])
case OPT_fcommon:
case OPT_fexceptions:
case OPT_fgnu_tm:
+ case OPT_freg_struct_return:
+ case OPT_fpcc_struct_return:
break;
default:
@@ -619,6 +631,12 @@ run_gcc (unsigned argc, char *argv[])
/* We've handled these LTO options, do not pass them on. */
continue;
+ case OPT_freg_struct_return:
+ case OPT_fpcc_struct_return:
+ /* Ignore these, they are determined by the input files.
+??? We fail to diagnose a possible mismatch here. */
+ continue;
+
default:
break;
}
[PATCH] Fix PR53168
This fixes PR53168 - and removes some of the more odd code in
phi-translation (I've added a testcase to make sure the intended
PREs still happen). There is still some issues in phi translation
I believe, but those are for later.
Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
Richard.
2012-05-04 Richard Guenther
PR tree-optimization/53168
* tree-ssa-pre.c (phi_translate_1): Only handle type-punned
memory reads when the result is a constant we can pun.
* gcc.dg/torture/pr53168.c: New testcase.
* gcc.dg/tree-ssa/ssa-pre-30.c: Likewise.
Index: gcc/tree-ssa-pre.c
===
*** gcc/tree-ssa-pre.c (revision 187148)
--- gcc/tree-ssa-pre.c (working copy)
*** phi_translate_1 (pre_expr expr, bitmap_s
*** 1659,1665
{
unsigned int new_val_id;
pre_expr constant;
- bool converted = false;
tree result = vn_reference_lookup_pieces (newvuse, ref->set,
ref->type,
--- 1659,1664
*** phi_translate_1 (pre_expr expr, bitmap_s
*** 1668,1679
if (result)
VEC_free (vn_reference_op_s, heap, newoperands);
! if (result
! && !useless_type_conversion_p (ref->type, TREE_TYPE (result)))
{
! result = fold_build1 (VIEW_CONVERT_EXPR, ref->type, result);
! converted = true;
}
else if (!result && newref
&& !useless_type_conversion_p (ref->type, newref->type))
{
--- 1667,1695
if (result)
VEC_free (vn_reference_op_s, heap, newoperands);
! /* We can always insert constants, so if we have a partial
! redundant constant load of another type try to translate it
! to a constant of appropriate type. */
! if (result && is_gimple_min_invariant (result))
{
! tree tem = result;
! if (!useless_type_conversion_p (ref->type, TREE_TYPE (result)))
! {
! tem = fold_unary (VIEW_CONVERT_EXPR, ref->type, result);
! if (tem && !is_gimple_min_invariant (tem))
! tem = NULL_TREE;
! }
! if (tem)
! return get_or_alloc_expr_for_constant (tem);
}
+
+ /* If we'd have to convert things we would need to validate
+ if we can insert the translated expression. So fail
+ here for now - we cannot insert an alias with a different
+ type in the VN tables either, as that would assert. */
+ if (result
+ && !useless_type_conversion_p (ref->type, TREE_TYPE (result)))
+ return NULL;
else if (!result && newref
&& !useless_type_conversion_p (ref->type, newref->type))
{
*** phi_translate_1 (pre_expr expr, bitmap_s
*** 1681,1741
return NULL;
}
- if (result && is_gimple_min_invariant (result))
- {
- gcc_assert (!newoperands);
- return get_or_alloc_expr_for_constant (result);
- }
-
expr = (pre_expr) pool_alloc (pre_expr_pool);
expr->kind = REFERENCE;
expr->id = 0;
! if (converted)
! {
! vn_nary_op_t nary;
! tree nresult;
!
! gcc_assert (CONVERT_EXPR_P (result)
! || TREE_CODE (result) == VIEW_CONVERT_EXPR);
!
! nresult = vn_nary_op_lookup_pieces (1, TREE_CODE (result),
! TREE_TYPE (result),
! &TREE_OPERAND (result, 0),
! &nary);
! if (nresult && is_gimple_min_invariant (nresult))
! return get_or_alloc_expr_for_constant (nresult);
!
! expr->kind = NARY;
! if (nary)
! {
! PRE_EXPR_NARY (expr) = nary;
! constant = fully_constant_expression (expr);
! if (constant != expr)
! return constant;
!
! new_val_id = nary->value_id;
! get_or_alloc_expression_id (expr);
! }
! else
! {
! new_val_id = get_next_value_id ();
! VEC_safe_grow_cleared (bitmap_set_t, heap,
! value_expressions,
! get_max_value_id() + 1);
! nary = vn_nary_op_insert_pieces (1, TREE_CODE (result),
!TREE_TYPE (
Re: [C++ Patch] fix semi-random template specialization ICE
Hi, On Thu, May 03, 2012 at 03:17:23PM -0300, Alexandre Oliva wrote: > I've recently started getting “libstdc++-v3/include/functional:2057:63: > internal compiler error: tree check: expected tree_vec, have error_mark > in comp_template_args_with_info, at cp/pt.c:7038” on i686-linux-gnu, > building libstdc++-v3/src/c++11/functexcept.cc -fPIC, at stage1 and on > non-bootstrapped builds. The problem would not occur on > x86_64-linux-gnu with the -m32 multilib. I suppose this is PR 53209. Thanks for dealing with this! Martin > > Jakub reported getting similar errors in the testsuite, but not in the > libstdc++-v3 build. > > Bisection revealted the patch that exposed the latent error was r186948, > but I gather it only introduced more potentially-failing specializations > in libstdc++-v3 at spots that wouldn't trigger the bug before. > > I couldn't pinpoint the exact source of randomness that causes the build > to fail at precisely the same point on a given machine at a certain > stage, but not on others. What I do know is that it occurs while > iterating on a hash table, which, depending on how the hash is computed, > may explain why we visit some nodes before others depending on > environmentally-deterministic causes. > > Anyway, the problem is that, for some unsuitable candidate template > specializations, tsubst returns error_mark_node, which tsubst_decl > stores in argvec, and later on register_specialization gets this > error_mark_node and tries to access it as a tree_vec. > > The trivial patch that avoids the misbehavior is returning > error_mark_node as soon as we get that for argvec. Bootstrapped on > i686-pc-linux-gnu and x86_64-linux-gnu, regstrapped on the latter. > > Ok to install? > > for gcc/cp/ChangeLog > from Alexandre Oliva > > * pt.c (tsubst_decl): Bail out if argvec is error_mark_node. > > Index: gcc/cp/pt.c > === > --- gcc/cp/pt.c.orig 2012-04-30 15:34:44.018432544 -0300 > +++ gcc/cp/pt.c 2012-04-30 15:34:47.988375071 -0300 > @@ -10626,6 +10626,8 @@ tsubst_decl (tree t, tree args, tsubst_f > tmpl = DECL_TI_TEMPLATE (t); > gen_tmpl = most_general_template (tmpl); > argvec = tsubst (DECL_TI_ARGS (t), args, complain, in_decl); > + if (argvec == error_mark_node) > + RETURN (error_mark_node); > hash = hash_tmpl_and_args (gen_tmpl, argvec); > spec = retrieve_specialization (gen_tmpl, argvec, hash); > } > > > -- > Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/ > You must be the change you wish to see in the world. -- Gandhi > Be Free! -- http://FSFLA.org/ FSF Latin America board member > Free Software Evangelist Red Hat Brazil Compiler Engineer
Re: [Patch, Fortran] PR53111 - fix -std=f95 diagnostic regression (constructor patch)
Dear Tobias, The patch is OK for 4.7 and trunk - thanks. The commit of the PR41600 is delayed until tomorrow or Sunday because of PR53191. I have regtested a fix for it and, since it is 'obvious' I will commit it with the main patch. In fact, it extends the constraint C614 (see resolve_ref) to include class expressions and removes the ad hoc error in the PR41600 patch. Cheers Paul On 3 May 2012 11:45, Tobias Burnus wrote: > Dear all, > > attached you find a patch for a -std=f95 diagnostic issue related to F2003 > constructors, which is a 4.7/4.8 rejects-valid regression. > > It was build and regtested on x86-64-linux. > OK for 4.7 and the trunk? > > Note that a valid-F95 special case is still rejected with -std=f95 (cf. > attached test case), but I believe that the current patch is sufficient. > > > Secondly, I try to get again an overview about pending patches. I found: > > a) Pending review > > - Janne's Fix handling of temporary files > http://gcc.gnu.org/ml/fortran/2012-05/msg6.html > - Janne's PR 49010/24518 MOD/MODULO fixes, take 2 > http://gcc.gnu.org/ml/fortran/2012-05/msg7.html > - Janne's PR 52428 Reading of large negative values and range checking > http://gcc.gnu.org/ml/fortran/2012-05/msg8.html > - Thomas' string comparison patch > http://gcc.gnu.org/ml/fortran/2012-04/msg00068.html > - This patch ;-) > > b) Pending committal: > > - Paul's SELECT TYPE patch > http://gcc.gnu.org/ml/fortran/2012-05/msg0.html > - Bernhard's cleanup-module patch > http://gcc.gnu.org/ml/fortran/2012-04/msg00105.html > > c) More controversial patches: > > - FX's backtrace patch > http://gcc.gnu.org/ml/fortran/2012-04/msg00131.html > - Janus' proc-pointer patch (ABI breakage) > > http://gcc.gnu.org/ml/fortran/2012-04/msg00033.html > > > d) RFA patch > > - My C_F_pointer patch > http://gcc.gnu.org/ml/fortran/2012-04/msg00115.html > > I hope that I found all pending patches. > > > Thirdly, we are accumulating regressions. The current count is 12. There are > also a couple of other bugs, e.g. related to namelist reads, which should be > get some attention. > > Tobias -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
[PATCH libcpp]: Avoid crash in interpret_float_suffix
Hi,
the function libcpp/expr.c:interpret_float_suffix allows its argument LEN to be
0, but in this case it tries to read before the buffer S. It is not a real
issue, except in case of overflow: on VMS with 64bit pointers but 32bit
size_t, the following code:
s[len-1]
is evaluated as
s[0x]
which is likely (and does) crash cc1.
To avoid this nasty effect, I just added a guard.
Bootstrapped and regtested on i386/GNU linux.
Ok for trunk ?
Tristan.
libcpp/
2012-05-04 Tristan Gingold
* expr.c (interpret_float_suffix): Add a guard.
diff --git a/libcpp/expr.c b/libcpp/expr.c
index d56e56a..ca1c3d1 100644
--- a/libcpp/expr.c
+++ b/libcpp/expr.c
@@ -110,12 +110,13 @@ interpret_float_suffix (const uchar *s, size_t len)
}
/* Recognize a fixed-point suffix. */
- switch (s[len-1])
-{
-case 'k': case 'K': flags = CPP_N_ACCUM; break;
-case 'r': case 'R': flags = CPP_N_FRACT; break;
-default: break;
-}
+ if (len != 0)
+switch (s[len-1])
+ {
+ case 'k': case 'K': flags = CPP_N_ACCUM; break;
+ case 'r': case 'R': flags = CPP_N_FRACT; break;
+ default: break;
+ }
/* Continue processing a fixed-point suffix. The suffix is case
insensitive except for ll or LL. Order is significant. */
[PATCH]: handle size_t /= ptr size in tree-ssa-strlen.c
Hi,
get_string_length assumes that precision of size_t is the same as precision of
a pointer, which isn't always true at least on VMS.
This patch simply add a conversion in case of mismatch.
Tested on ia64/VMS by cross-bootstrapping.
Bootstrapped and regtested on i386/GNU Linux.
Ok for trunk ?
Tristan.
2012-05-04 Tristan Gingold
* tree-ssa-strlen.c (get_string_length): Convert lhs if needed.
diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c
index a37633a..bda5ffb 100644
--- a/gcc/tree-ssa-strlen.c
+++ b/gcc/tree-ssa-strlen.c
@@ -427,6 +427,12 @@ get_string_length (strinfo si)
NULL);
add_referenced_var (lhs_var);
tem = gimple_call_arg (stmt, 0);
+ if (!useless_type_conversion_p (sizetype, TREE_TYPE (lhs)))
+{
+ lhs = convert_to_ptrofftype (lhs);
+ lhs = force_gimple_operand_gsi (&gsi, lhs, true, NULL_TREE,
+ true, GSI_SAME_STMT);
+}
lenstmt
= gimple_build_assign_with_ops (POINTER_PLUS_EXPR,
make_ssa_name (lhs_var, NULL),
Re: [Patch, Fortran] PR53175 - Fix another fallout of the TREE_PUBLIC=0 module variable patch
Dear Tobias, This is OK for trunk. Thanks for the patch. Paul On 3 May 2012 20:41, Tobias Burnus wrote: > A PRIVATE module variable, which is used in the specification expression of > a function result variable cannot be TREE_PUBLIC()=0, unless the function > itself is PRIVATE and also not accessible via type-bound procedures or a > generic name. (The issue is gfortran specific as most compilers do not seem > to save the specification expression in the .mod file.) > > Build and regtested on x86-64-linux. > OK for the trunk? > > Tobias > -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [PATCH 2/2] Minor refactoring of tree-vect-patterns.c
Richard Guenther wrote:
> On Mon, Apr 30, 2012 at 6:19 PM, Ulrich Weigand wrote:
> > Hello,
> >
> > as a second step in refactoring this patch introduces a routine
> > vect_find_single_use to determine whether a defining statement
> > has one single use within the current vectorization domain.
> >
> > The helper is then called wherever that check is currently
> > open-coded. =A0There should be no change in behaviour.
> >
> > Tested on i386-linux-gnu and arm-linux-gnueabi with no regressions.
> >
> > OK for mainline?
>
> You can use single_imm_use to avoid the loop and simplify the factored
> routine.
>
> Ok with that change.
I've checked in the version appended below.
Thanks,
Ulrich
ChangeLog:
* tree-vect-patterns.c (vect_single_imm_use): New function.
(vect_recog_widen_mult_pattern): Use it instead of open-coding loop.
(vect_recog_over_widening_pattern): Likewise.
(vect_recog_widen_shift_pattern): Likewise.
Index: gcc-head/gcc/tree-vect-patterns.c
===
--- gcc-head.orig/gcc/tree-vect-patterns.c 2012-05-04 14:23:18.0
+0200
+++ gcc-head/gcc/tree-vect-patterns.c 2012-05-04 14:23:38.0 +0200
@@ -119,6 +119,25 @@ vect_same_loop_or_bb_p (gimple stmt1, gi
return true;
}
+/* If the LHS of DEF_STMT has a single use, and that statement is
+ in the same loop or basic block, return it. */
+
+static gimple
+vect_single_imm_use (gimple def_stmt)
+{
+ tree lhs = gimple_assign_lhs (def_stmt);
+ use_operand_p use_p;
+ gimple use_stmt;
+
+ if (!single_imm_use (lhs, &use_p, &use_stmt))
+return NULL;
+
+ if (!vect_same_loop_or_bb_p (def_stmt, use_stmt))
+return NULL;
+
+ return use_stmt;
+}
+
/* Check whether NAME, an ssa-name used in USE_STMT,
is a result of a type promotion or demotion, such that:
DEF_STMT: NAME = NOP (name0)
@@ -636,31 +655,18 @@ vect_recog_widen_mult_pattern (VEC (gimp
Use unsigned TYPE as the type for WIDEN_MULT_EXPR. */
if (TYPE_UNSIGNED (type) != TYPE_UNSIGNED (half_type0))
{
- tree lhs = gimple_assign_lhs (last_stmt), use_lhs;
- imm_use_iterator imm_iter;
- use_operand_p use_p;
- int nuses = 0;
- gimple use_stmt = NULL;
+ gimple use_stmt;
+ tree use_lhs;
tree use_type;
if (TYPE_UNSIGNED (type) == TYPE_UNSIGNED (half_type1))
return NULL;
- FOR_EACH_IMM_USE_FAST (use_p, imm_iter, lhs)
-{
- if (is_gimple_debug (USE_STMT (use_p)))
- continue;
- use_stmt = USE_STMT (use_p);
- nuses++;
-}
-
- if (nuses != 1 || !is_gimple_assign (use_stmt)
- || gimple_assign_rhs_code (use_stmt) != NOP_EXPR)
+ use_stmt = vect_single_imm_use (last_stmt);
+ if (!use_stmt || !is_gimple_assign (use_stmt)
+ || gimple_assign_rhs_code (use_stmt) != NOP_EXPR)
return NULL;
- if (!vect_same_loop_or_bb_p (last_stmt, use_stmt))
- return NULL;
-
use_lhs = gimple_assign_lhs (use_stmt);
use_type = TREE_TYPE (use_lhs);
if (!INTEGRAL_TYPE_P (use_type)
@@ -1165,10 +1171,7 @@ vect_recog_over_widening_pattern (VEC (g
{
gimple stmt = VEC_pop (gimple, *stmts);
gimple pattern_stmt = NULL, new_def_stmt, prev_stmt = NULL, use_stmt = NULL;
- tree op0, op1, vectype = NULL_TREE, lhs, use_lhs, use_type;
- imm_use_iterator imm_iter;
- use_operand_p use_p;
- int nuses = 0;
+ tree op0, op1, vectype = NULL_TREE, use_lhs, use_type;
tree var = NULL_TREE, new_type = NULL_TREE, tmp, new_oprnd;
bool first;
tree type = NULL;
@@ -1192,18 +1195,8 @@ vect_recog_over_widening_pattern (VEC (g
}
/* STMT can be performed on a smaller type. Check its uses. */
- lhs = gimple_assign_lhs (stmt);
- nuses = 0;
- FOR_EACH_IMM_USE_FAST (use_p, imm_iter, lhs)
-{
- if (is_gimple_debug (USE_STMT (use_p)))
-continue;
- use_stmt = USE_STMT (use_p);
- nuses++;
-}
-
- if (nuses != 1 || !is_gimple_assign (use_stmt)
- || !vect_same_loop_or_bb_p (stmt, use_stmt))
+ use_stmt = vect_single_imm_use (stmt);
+ if (!use_stmt || !is_gimple_assign (use_stmt))
return NULL;
/* Create pattern statement for STMT. */
@@ -1454,12 +1447,6 @@ vect_recog_widen_shift_pattern (VEC (gim
Use unsigned TYPE as the type for WIDEN_LSHIFT_EXPR. */
if (TYPE_UNSIGNED (type) != TYPE_UNSIGNED (half_type0))
{
- tree lhs = gimple_assign_lhs (last_stmt), use_lhs;
- imm_use_iterator imm_iter;
- use_operand_p use_p;
- int nuses = 0;
- tree use_type;
-
if (over_widen)
{
/* In case of over-widening pattern, S4 should be ORIG_STMT itself.
@@ -1472,21 +1459,14 @@ vect_recog_widen_shift_pattern (VEC (gim
}
else
{
- FOR_EACH_IMM_USE_FAST (use_p, imm_iter, lhs)
-{
- if (is_gimple_debug (USE_S
RE: Patch to enable --with-multilib-list for arm-none-eabi target
On Fri, 4 May 2012, Terry Guo wrote: > Hello Joseph, > > Can you help to review this patch? The basic idea is to provide a set of > predefined Multilib and enable user to select the required ones from > --with-multilib-list. I'd think it ought to be ARM target maintainers who review this patch. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH]: handle size_t /= ptr size in tree-ssa-strlen.c
On Fri, May 4, 2012 at 2:27 PM, Tristan Gingold wrote:
> Hi,
>
> get_string_length assumes that precision of size_t is the same as precision
> of a pointer, which isn't always true at least on VMS.
> This patch simply add a conversion in case of mismatch.
>
> Tested on ia64/VMS by cross-bootstrapping.
> Bootstrapped and regtested on i386/GNU Linux.
>
> Ok for trunk ?
Hmm, isn't the issue that size_t precision is not the same precision as
sizetype? You probably should use ptrofftype_p (TREE_TYPE (lhs))
instead of spelling out 'sizetype' here.
Richard.
> Tristan.
>
> 2012-05-04 Tristan Gingold
>
> * tree-ssa-strlen.c (get_string_length): Convert lhs if needed.
>
> diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c
> index a37633a..bda5ffb 100644
> --- a/gcc/tree-ssa-strlen.c
> +++ b/gcc/tree-ssa-strlen.c
> @@ -427,6 +427,12 @@ get_string_length (strinfo si)
> NULL);
> add_referenced_var (lhs_var);
> tem = gimple_call_arg (stmt, 0);
> + if (!useless_type_conversion_p (sizetype, TREE_TYPE (lhs)))
> + {
> + lhs = convert_to_ptrofftype (lhs);
> + lhs = force_gimple_operand_gsi (&gsi, lhs, true, NULL_TREE,
> + true, GSI_SAME_STMT);
> + }
> lenstmt
> = gimple_build_assign_with_ops (POINTER_PLUS_EXPR,
> make_ssa_name (lhs_var, NULL),
>
[patch] PR tree-optimization/52633 - ICE due to vectorizer pattern detection collision
Richard Guenther wrote:
> On Tue, 24 Apr 2012, Ulrich Weigand wrote:
> > However, even so, it might actually be preferable to just handle such
> > cases within vect_recog_widen_shift_pattern itself. Indeed, the routine
> > already looks for another subsequent type cast, in order to handle
> > unsigned shift variants. Maybe it simply ought to always look for
> > another cast, and detect over-widening situations itself?
> >
> >
> > Does this look reasonable? Any comments or suggestions appreciated!
>
> Yes, getting rid of this fragile interaction by doing more work in
> vect_recog_widen_shift_pattern sounds like the correct thing to do.
The following patch implements this. Tested with no regressions on
armv7l-linux-gnueabi and i686-linux-gnu; a couple of vectorizer tests
had to be adapted since now the widening-shift patttern matches
instead of the over-widening pattern (as expected).
Performance testing on ARM showed no regressions (and possible minor
improvements in some cases).
OK for mainline?
Bye,
Ulrich
ChangeLog:
gcc/
PR tree-optimization/52633
* tree-vect-patterns.c (vect_vect_recog_func_ptrs): Swap order of
vect_recog_widen_shift_pattern and vect_recog_over_widening_pattern.
(vect_recog_over_widening_pattern): Remove handling of code that was
already detected as over-widening pattern. Remove special handling
of "unsigned" cases. Instead, support general case of conversion
of the shift result to another type.
gcc/testsuite/
PR tree-optimization/52633
* gcc.dg/vect/vect-over-widen-1.c: Two patterns should now be
recognized as widening shifts instead of over-widening.
* gcc.dg/vect/vect-over-widen-1-big-array.c: Likewise.
* gcc.dg/vect/vect-over-widen-4.c: Likewise.
* gcc.dg/vect/vect-over-widen-4-big-array.c: Likewise.
* gcc.target/arm/pr52633.c: New test.
Index: gcc-head/gcc/tree-vect-patterns.c
===
--- gcc-head.orig/gcc/tree-vect-patterns.c 2012-05-04 14:23:38.0
+0200
+++ gcc-head/gcc/tree-vect-patterns.c 2012-05-04 14:24:06.0 +0200
@@ -63,8 +63,8 @@ static vect_recog_func_ptr vect_vect_rec
vect_recog_widen_sum_pattern,
vect_recog_dot_prod_pattern,
vect_recog_pow_pattern,
- vect_recog_over_widening_pattern,
vect_recog_widen_shift_pattern,
+ vect_recog_over_widening_pattern,
vect_recog_vector_vector_shift_pattern,
vect_recog_sdivmod_pow2_pattern,
vect_recog_mixed_size_cond_pattern,
@@ -1309,16 +1309,20 @@ vect_recog_over_widening_pattern (VEC (g
where type 'TYPE' is at least double the size of type 'type'.
- Also detect unsigned cases:
+ Also detect cases where the shift result is immediately converted
+ to another type 'result_type' that is no larger in size than 'TYPE'.
+ In those cases we perform a widen-shift that directly results in
+ 'result_type', to avoid a possible over-widening situation:
- unsigned type a_t;
- unsigned TYPE u_res_T;
+ type a_t;
TYPE a_T, res_T;
+ result_type res_result;
S1 a_t = ;
S2 a_T = (TYPE) a_t;
S3 res_T = a_T << CONST;
- S4 u_res_T = (unsigned TYPE) res_T;
+ S4 res_result = (result_type) res_T;
+ '--> res_result' = a_t w<< CONST;
And a case when 'TYPE' is 4 times bigger than 'type'. In that case we
create an additional pattern stmt for S2 to create a variable of an
@@ -1359,60 +1363,21 @@ vect_recog_widen_shift_pattern (VEC (gim
gimple def_stmt0;
tree oprnd0, oprnd1;
tree type, half_type0;
- gimple pattern_stmt, orig_stmt = NULL;
+ gimple pattern_stmt;
tree vectype, vectype_out = NULL_TREE;
tree dummy;
tree var;
enum tree_code dummy_code;
int dummy_int;
VEC (tree, heap) * dummy_vec;
- gimple use_stmt = NULL;
- bool over_widen = false;
+ gimple use_stmt;
bool promotion;
if (!is_gimple_assign (last_stmt) || !vinfo_for_stmt (last_stmt))
return NULL;
- orig_stmt = last_stmt;
if (STMT_VINFO_IN_PATTERN_P (vinfo_for_stmt (last_stmt)))
-{
- /* This statement was also detected as over-widening operation (it can't
- be any other pattern, because only over-widening detects shifts).
- LAST_STMT is the final type demotion statement, but its related
- statement is shift. We analyze the related statement to catch cases:
-
- orig code:
- type a_t;
- itype res;
- TYPE a_T, res_T;
-
- S1 a_T = (TYPE) a_t;
- S2 res_T = a_T << CONST;
- S3 res = (itype)res_T;
-
- (size of type * 2 <= size of itype
- and size of itype * 2 <= size of TYPE)
-
- code after over-widening pattern detection:
-
- S1 a_T = (TYPE) a_t;
- --> a_it = (itype) a_t;
- S2 res_T = a_T << CONST;
- S3 res = (itype)res_T; <--- LAST_STMT
- --> res = a_it
Re: [PATCH]: handle size_t /= ptr size in tree-ssa-strlen.c
On May 4, 2012, at 2:34 PM, Richard Guenther wrote:
> On Fri, May 4, 2012 at 2:27 PM, Tristan Gingold wrote:
>> Hi,
>>
>> get_string_length assumes that precision of size_t is the same as precision
>> of a pointer, which isn't always true at least on VMS.
>> This patch simply add a conversion in case of mismatch.
>>
>> Tested on ia64/VMS by cross-bootstrapping.
>> Bootstrapped and regtested on i386/GNU Linux.
>>
>> Ok for trunk ?
>
> Hmm, isn't the issue that size_t precision is not the same precision as
> sizetype?
Yes, that's right.
> You probably should use ptrofftype_p (TREE_TYPE (lhs))
> instead of spelling out 'sizetype' here.
Will try. Thanks for the hint.
Tristan.
>
> Richard.
>
>> Tristan.
>>
>> 2012-05-04 Tristan Gingold
>>
>>* tree-ssa-strlen.c (get_string_length): Convert lhs if needed.
>>
>> diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c
>> index a37633a..bda5ffb 100644
>> --- a/gcc/tree-ssa-strlen.c
>> +++ b/gcc/tree-ssa-strlen.c
>> @@ -427,6 +427,12 @@ get_string_length (strinfo si)
>>NULL);
>> add_referenced_var (lhs_var);
>> tem = gimple_call_arg (stmt, 0);
>> + if (!useless_type_conversion_p (sizetype, TREE_TYPE (lhs)))
>> +{
>> + lhs = convert_to_ptrofftype (lhs);
>> + lhs = force_gimple_operand_gsi (&gsi, lhs, true, NULL_TREE,
>> + true, GSI_SAME_STMT);
>> +}
>> lenstmt
>>= gimple_build_assign_with_ops (POINTER_PLUS_EXPR,
>>make_ssa_name (lhs_var, NULL),
>>
[PATCH] Properly check result of compute_all_dependences in prefetching
This is the last caller of compute_all_dependences that does
not check its return value.
Currently bootstrapping and testing on x86_64-unknown-linux-gnu.
Richard.
2012-05-04 Richard Guenther
* tree-ssa-loop-prefetch.c (determine_loop_nest_reuse): Return
whether we failed to compute data dependences.
(loop_prefetch_arrays): Fail if we failed.
Index: gcc/tree-ssa-loop-prefetch.c
===
*** gcc/tree-ssa-loop-prefetch.c(revision 187152)
--- gcc/tree-ssa-loop-prefetch.c(working copy)
*** self_reuse_distance (data_reference_p dr
*** 1495,1503
/* Determines the distance till the first reuse of each reference in REFS
in the loop nest of LOOP. NO_OTHER_REFS is true if there are no other
!memory references in the loop. */
! static void
determine_loop_nest_reuse (struct loop *loop, struct mem_ref_group *refs,
bool no_other_refs)
{
--- 1495,1503
/* Determines the distance till the first reuse of each reference in REFS
in the loop nest of LOOP. NO_OTHER_REFS is true if there are no other
!memory references in the loop. Return false if the analysis fails. */
! static bool
determine_loop_nest_reuse (struct loop *loop, struct mem_ref_group *refs,
bool no_other_refs)
{
*** determine_loop_nest_reuse (struct loop *
*** 1515,1521
ddr_p dep;
if (loop->inner)
! return;
/* Find the outermost loop of the loop nest of loop (we require that
there are no sibling loops inside the nest). */
--- 1515,1521
ddr_p dep;
if (loop->inner)
! return true;
/* Find the outermost loop of the loop nest of loop (we require that
there are no sibling loops inside the nest). */
*** determine_loop_nest_reuse (struct loop *
*** 1585,1591
ref->independent_p = true;
}
! compute_all_dependences (datarefs, &dependences, vloops, true);
FOR_EACH_VEC_ELT (ddr_p, dependences, i, dep)
{
--- 1585,1592
ref->independent_p = true;
}
! if (!compute_all_dependences (datarefs, &dependences, vloops, true))
! return false;
FOR_EACH_VEC_ELT (ddr_p, dependences, i, dep)
{
*** determine_loop_nest_reuse (struct loop *
*** 1664,1669
--- 1665,1672
fprintf (dump_file, " ref %p distance %u\n",
(void *) ref, ref->reuse_distance);
}
+
+ return true;
}
/* Determine whether or not the trip count to ahead ratio is too small based
*** loop_prefetch_arrays (struct loop *loop)
*** 1826,1832
if (nothing_to_prefetch_p (refs))
goto fail;
! determine_loop_nest_reuse (loop, refs, no_other_refs);
/* Step 3: determine unroll factor. */
unroll_factor = determine_unroll_factor (loop, refs, ninsns, &desc,
--- 1829,1836
if (nothing_to_prefetch_p (refs))
goto fail;
! if (!determine_loop_nest_reuse (loop, refs, no_other_refs))
! goto fail;
/* Step 3: determine unroll factor. */
unroll_factor = determine_unroll_factor (loop, refs, ninsns, &desc,
Re: [patch] PR tree-optimization/52633 - ICE due to vectorizer pattern detection collision
On Fri, 4 May 2012, Ulrich Weigand wrote:
> Richard Guenther wrote:
> > On Tue, 24 Apr 2012, Ulrich Weigand wrote:
> > > However, even so, it might actually be preferable to just handle such
> > > cases within vect_recog_widen_shift_pattern itself. Indeed, the routine
> > > already looks for another subsequent type cast, in order to handle
> > > unsigned shift variants. Maybe it simply ought to always look for
> > > another cast, and detect over-widening situations itself?
> > >
> > >
> > > Does this look reasonable? Any comments or suggestions appreciated!
> >
> > Yes, getting rid of this fragile interaction by doing more work in
> > vect_recog_widen_shift_pattern sounds like the correct thing to do.
>
> The following patch implements this. Tested with no regressions on
> armv7l-linux-gnueabi and i686-linux-gnu; a couple of vectorizer tests
> had to be adapted since now the widening-shift patttern matches
> instead of the over-widening pattern (as expected).
>
> Performance testing on ARM showed no regressions (and possible minor
> improvements in some cases).
>
> OK for mainline?
Ok.
Thanks,
Richard.
> Bye,
> Ulrich
>
>
> ChangeLog:
>
> gcc/
> PR tree-optimization/52633
> * tree-vect-patterns.c (vect_vect_recog_func_ptrs): Swap order of
> vect_recog_widen_shift_pattern and vect_recog_over_widening_pattern.
> (vect_recog_over_widening_pattern): Remove handling of code that was
> already detected as over-widening pattern. Remove special handling
> of "unsigned" cases. Instead, support general case of conversion
> of the shift result to another type.
>
> gcc/testsuite/
> PR tree-optimization/52633
> * gcc.dg/vect/vect-over-widen-1.c: Two patterns should now be
> recognized as widening shifts instead of over-widening.
> * gcc.dg/vect/vect-over-widen-1-big-array.c: Likewise.
> * gcc.dg/vect/vect-over-widen-4.c: Likewise.
> * gcc.dg/vect/vect-over-widen-4-big-array.c: Likewise.
> * gcc.target/arm/pr52633.c: New test.
>
>
> Index: gcc-head/gcc/tree-vect-patterns.c
> ===
> --- gcc-head.orig/gcc/tree-vect-patterns.c2012-05-04 14:23:38.0
> +0200
> +++ gcc-head/gcc/tree-vect-patterns.c 2012-05-04 14:24:06.0 +0200
> @@ -63,8 +63,8 @@ static vect_recog_func_ptr vect_vect_rec
> vect_recog_widen_sum_pattern,
> vect_recog_dot_prod_pattern,
> vect_recog_pow_pattern,
> - vect_recog_over_widening_pattern,
> vect_recog_widen_shift_pattern,
> + vect_recog_over_widening_pattern,
> vect_recog_vector_vector_shift_pattern,
> vect_recog_sdivmod_pow2_pattern,
> vect_recog_mixed_size_cond_pattern,
> @@ -1309,16 +1309,20 @@ vect_recog_over_widening_pattern (VEC (g
>
>where type 'TYPE' is at least double the size of type 'type'.
>
> - Also detect unsigned cases:
> + Also detect cases where the shift result is immediately converted
> + to another type 'result_type' that is no larger in size than 'TYPE'.
> + In those cases we perform a widen-shift that directly results in
> + 'result_type', to avoid a possible over-widening situation:
>
> - unsigned type a_t;
> - unsigned TYPE u_res_T;
> + type a_t;
>TYPE a_T, res_T;
> + result_type res_result;
>
>S1 a_t = ;
>S2 a_T = (TYPE) a_t;
>S3 res_T = a_T << CONST;
> - S4 u_res_T = (unsigned TYPE) res_T;
> + S4 res_result = (result_type) res_T;
> + '--> res_result' = a_t w<< CONST;
>
>And a case when 'TYPE' is 4 times bigger than 'type'. In that case we
>create an additional pattern stmt for S2 to create a variable of an
> @@ -1359,60 +1363,21 @@ vect_recog_widen_shift_pattern (VEC (gim
>gimple def_stmt0;
>tree oprnd0, oprnd1;
>tree type, half_type0;
> - gimple pattern_stmt, orig_stmt = NULL;
> + gimple pattern_stmt;
>tree vectype, vectype_out = NULL_TREE;
>tree dummy;
>tree var;
>enum tree_code dummy_code;
>int dummy_int;
>VEC (tree, heap) * dummy_vec;
> - gimple use_stmt = NULL;
> - bool over_widen = false;
> + gimple use_stmt;
>bool promotion;
>
>if (!is_gimple_assign (last_stmt) || !vinfo_for_stmt (last_stmt))
> return NULL;
>
> - orig_stmt = last_stmt;
>if (STMT_VINFO_IN_PATTERN_P (vinfo_for_stmt (last_stmt)))
> -{
> - /* This statement was also detected as over-widening operation (it
> can't
> - be any other pattern, because only over-widening detects shifts).
> - LAST_STMT is the final type demotion statement, but its related
> - statement is shift. We analyze the related statement to catch
> cases:
> -
> - orig code:
> - type a_t;
> - itype res;
> - TYPE a_T, res_T;
> -
> - S1 a_T = (TYPE) a_t;
> - S2 res_T = a_T << CONST;
> - S3 res = (itype)res_T;
> -
> - (size of type * 2 <= size of itype
> - and size of i
Re: strengthen protection against REG_EQUIV/EQUAL on !REG set
Hello Richard, Re $subject, at http://gcc.gnu.org/ml/gcc-patches/2012-04/msg01515.html You suggested: >> Would be nice to use a single function that knows about the extra >> contraints here. Maybe something like the attached? << 2012-04-24 ... * rtl.h (set_for_reg_notes): Declare. * emit-rtl.c (set_for_reg_notes): New function. (set_unique_reg_note): Use it. * optabs.c (add_equal_note): Likewise. >> I had answered: > Looks cleaner indeed. Do you want me to test ? I gave it a try. Your patch bootstraps and regtests fine on mainline for x86-linux. May I commit ? Thanks in advance for your feedback, With Kind Regards, Olivier
Re: patch ping: Add static branch predict heuristic of comparing IV to loop_bound variable
Hi, Honza,
Thanks for the prompt response. Attached is the updated patch.
Passed bootstrap and all regression tests.
Thanks,
Dehao
Index: gcc/testsuite/gcc.dg/predict-3.c
===
--- gcc/testsuite/gcc.dg/predict-3.c(revision 0)
+++ gcc/testsuite/gcc.dg/predict-3.c(revision 0)
@@ -0,0 +1,25 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-profile_estimate" } */
+
+extern int global;
+
+int bar(int);
+
+void foo (int bound)
+{
+ int i, ret = 0;
+ for (i = 0; i <= bound; i++)
+{
+ if (i < bound - 2)
+ global += bar (i);
+ if (i <= bound)
+ global += bar (i);
+ if (i + 1 < bound)
+ global += bar (i);
+ if (i != bound)
+ global += bar (i);
+}
+}
+
+/* { dg-final { scan-tree-dump-times "loop iv compare heuristics:
100.0%" 4 "profile_estimate"} } */
+/* { dg-final { cleanup-tree-dump "profile_estimate" } } */
Index: gcc/testsuite/gcc.dg/predict-4.c
===
--- gcc/testsuite/gcc.dg/predict-4.c(revision 0)
+++ gcc/testsuite/gcc.dg/predict-4.c(revision 0)
@@ -0,0 +1,19 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-profile_estimate" } */
+
+extern int global;
+
+int bar(int);
+
+void foo (int bound)
+{
+ int i, ret = 0;
+ for (i = 0; i < 10; i++)
+{
+ if (i < 5)
+ global += bar (i);
+}
+}
+
+/* { dg-final { scan-tree-dump "loop iv compare heuristics: 50.0%"
"profile_estimate"} } */
+/* { dg-final { cleanup-tree-dump "profile_estimate" } } */
Index: gcc/testsuite/gcc.dg/predict-1.c
===
--- gcc/testsuite/gcc.dg/predict-1.c(revision 0)
+++ gcc/testsuite/gcc.dg/predict-1.c(revision 0)
@@ -0,0 +1,27 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-profile_estimate" } */
+
+extern int global;
+
+int bar(int);
+
+void foo (int bound)
+{
+ int i, ret = 0;
+ for (i = 0; i < bound; i++)
+{
+ if (i > bound)
+ global += bar (i);
+ if (i >= bound + 2)
+ global += bar (i);
+ if (i > bound - 2)
+ global += bar (i);
+ if (i + 2 > bound)
+ global += bar (i);
+ if (i == 10)
+ global += bar (i);
+}
+}
+
+/* { dg-final { scan-tree-dump-times "loop iv compare heuristics:
0.0%" 5 "profile_estimate"} } */
+/* { dg-final { cleanup-tree-dump "profile_estimate" } } */
Index: gcc/testsuite/gcc.dg/predict-5.c
===
--- gcc/testsuite/gcc.dg/predict-5.c(revision 0)
+++ gcc/testsuite/gcc.dg/predict-5.c(revision 0)
@@ -0,0 +1,25 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-profile_estimate" } */
+
+extern int global;
+
+int bar (int);
+
+void foo (int base, int bound)
+{
+ int i, ret = 0;
+ for (i = base; i <= bound; i++)
+{
+ if (i > base)
+ global += bar (i);
+ if (i > base + 1)
+ global += bar (i);
+ if (i >= base + 3)
+ global += bar (i);
+ if (i - 2 >= base)
+ global += bar (i);
+}
+}
+
+/* { dg-final { scan-tree-dump-times "loop iv compare heuristics:
100.0%" 4 "profile_estimate"} } */
+/* { dg-final { cleanup-tree-dump "profile_estimate" } } */
Index: gcc/testsuite/gcc.dg/predict-2.c
===
--- gcc/testsuite/gcc.dg/predict-2.c(revision 0)
+++ gcc/testsuite/gcc.dg/predict-2.c(revision 0)
@@ -0,0 +1,27 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-profile_estimate" } */
+
+extern int global;
+
+int bar(int);
+
+void foo (int base, int bound)
+{
+ int i, ret = 0;
+ for (i = base; i < bound; i++)
+{
+ if (i > bound * bound)
+ global += bar (i);
+ if (i > bound + 10)
+ global += bar (i);
+ if (i <= bound + 10)
+ global += bar (i);
+ if (i > base + 10)
+ global += bar (i);
+ if (i < base - 10)
+ global += bar (i);
+}
+}
+
+/* { dg-final { scan-tree-dump-not "loop iv compare heuristics"
"profile_estimate"} } */
+/* { dg-final { cleanup-tree-dump "profile_estimate" } } */
Index: gcc/testsuite/gcc.dg/predict-6.c
===
--- gcc/testsuite/gcc.dg/predict-6.c(revision 0)
+++ gcc/testsuite/gcc.dg/predict-6.c(revision 0)
@@ -0,0 +1,25 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-profile_estimate" } */
+
+extern int global;
+
+int bar (int);
+
+void foo (int base, int bound)
+{
+ int i, ret = 0;
+ for (i = base; i <= bound; i++)
+{
+ if (i < base)
+ global += bar (i);
+ if (i < base + 1)
+ global += bar (i);
+ if (i <= base + 3)
+ global += bar (i);
+ if (i - 1 < base)
+ global += bar (i);
+}
+}
+
+/* { dg-final { scan-tree-dump-times "loop iv compare heuristics:
0.0%" 4 "profile_estimate"} } */
+/* { dg-final { cleanup-tree-dump "profile_estimate"
Re: [PATCH libcpp]: Avoid crash in interpret_float_suffix
Tristan Gingold a écrit:
> the function libcpp/expr.c:interpret_float_suffix allows its argument
> LEN to be 0, but in this case it tries to read before the buffer S.
> It is not a real issue, except in case of overflow: on VMS with 64bit
> pointers but 32bit size_t, the following code: s[len-1] is evaluated
> as s[0x] which is likely (and does) crash cc1.
>
> To avoid this nasty effect, I just added a guard.
>
> Bootstrapped and regtested on i386/GNU linux.
>
> Ok for trunk ?
I can not approve or deny this patch, but for what it's worth, it looks
fine to me.
[...]
> +++ b/libcpp/expr.c
> @@ -110,12 +110,13 @@ interpret_float_suffix (const uchar *s, size_t len)
> }
>
>/* Recognize a fixed-point suffix. */
> - switch (s[len-1])
> -{
> -case 'k': case 'K': flags = CPP_N_ACCUM; break;
> -case 'r': case 'R': flags = CPP_N_FRACT; break;
> -default: break;
> -}
> + if (len != 0)
> +switch (s[len-1])
> + {
> + case 'k': case 'K': flags = CPP_N_ACCUM; break;
> + case 'r': case 'R': flags = CPP_N_FRACT; break;
> + default: break;
> + }
>
>/* Continue processing a fixed-point suffix. The suffix is case
> insensitive except for ll or LL. Order is significant. */
Thanks.
--
Dodji
Re: strengthen protection against REG_EQUIV/EQUAL on !REG set
Olivier Hainque writes: > Hello Richard, > > Re $subject, at http://gcc.gnu.org/ml/gcc-patches/2012-04/msg01515.html > > You suggested: >>> Would be nice to use a single function that knows about the extra >>> contraints here. Maybe something like the attached? > > << 2012-04-24 ... > > * rtl.h (set_for_reg_notes): Declare. > * emit-rtl.c (set_for_reg_notes): New function. > (set_unique_reg_note): Use it. > * optabs.c (add_equal_note): Likewise. >>> > > I had answered: >> Looks cleaner indeed. Do you want me to test ? > > I gave it a try. Your patch bootstraps and regtests fine on mainline for > x86-linux. Sorry, was going to test this earlier, but got distracted by lower-subreg stuff. I need to fix the subreg handling so that we check whether the inner part of a SUBREG is a REG (done in my copy at home). I also wanted to make sure there were no asm differences due to notes being wrongly dropped. Hope to do that this weekend. Richard
[Committed] S/390: Fix insn conditions of memory op patterns
Hi, the attached patch fixes a segfault which might occur when evaluating the insn conditions of three memory operation patterns in the s390 back-end after reloaded decided to pick an alternative where clobbering a scratch isn't necessary. Fixed by moving the mode check from the insn condition into the match_scratch statement. Committed to mainline. Bye, -Andreas- 2012-05-04 Andreas Krebbel * config/s390/s390.md (*movmem_short, *clrmem_short) (*cmpmem_short): Move the mode check from the insn condition to the match_scratch. --- gcc/config/s390/s390.md | 15 !!! 1 file changed, 15 modifications(!) Index: gcc/config/s390/s390.md === *** gcc/config/s390/s390.md.orig --- gcc/config/s390/s390.md *** *** 2658,2666 (match_operand:BLK 1 "memory_operand" "Q,Q,Q,Q")) (use (match_operand 2 "nonmemory_operand" "n,a,a,a")) (use (match_operand 3 "immediate_operand" "X,R,X,X")) !(clobber (match_scratch 4 "=X,X,X,&a"))] ! "(GET_MODE (operands[2]) == Pmode || GET_MODE (operands[2]) == VOIDmode) !&& GET_MODE (operands[4]) == Pmode" "#" [(set_attr "type" "cs") (set_attr "cpu_facility" "*,*,z10,*")]) --- 2658,2665 (match_operand:BLK 1 "memory_operand" "Q,Q,Q,Q")) (use (match_operand 2 "nonmemory_operand" "n,a,a,a")) (use (match_operand 3 "immediate_operand" "X,R,X,X")) !(clobber (match_scratch:P 4 "=X,X,X,&a"))] ! "(GET_MODE (operands[2]) == Pmode || GET_MODE (operands[2]) == VOIDmode)" "#" [(set_attr "type" "cs") (set_attr "cpu_facility" "*,*,z10,*")]) *** *** 2867,2876 (const_int 0)) (use (match_operand 1 "nonmemory_operand" "n,a,a,a")) (use (match_operand 2 "immediate_operand" "X,R,X,X")) !(clobber (match_scratch 3 "=X,X,X,&a")) (clobber (reg:CC CC_REGNUM))] ! "(GET_MODE (operands[1]) == Pmode || GET_MODE (operands[1]) == VOIDmode) !&& GET_MODE (operands[3]) == Pmode" "#" [(set_attr "type" "cs") (set_attr "cpu_facility" "*,*,z10,*")]) --- 2866,2874 (const_int 0)) (use (match_operand 1 "nonmemory_operand" "n,a,a,a")) (use (match_operand 2 "immediate_operand" "X,R,X,X")) !(clobber (match_scratch:P 3 "=X,X,X,&a")) (clobber (reg:CC CC_REGNUM))] ! "(GET_MODE (operands[1]) == Pmode || GET_MODE (operands[1]) == VOIDmode)" "#" [(set_attr "type" "cs") (set_attr "cpu_facility" "*,*,z10,*")]) *** *** 3040,3048 (match_operand:BLK 1 "memory_operand" "Q,Q,Q,Q"))) (use (match_operand 2 "nonmemory_operand" "n,a,a,a")) (use (match_operand 3 "immediate_operand" "X,R,X,X")) !(clobber (match_scratch 4 "=X,X,X,&a"))] ! "(GET_MODE (operands[2]) == Pmode || GET_MODE (operands[2]) == VOIDmode) !&& GET_MODE (operands[4]) == Pmode" "#" [(set_attr "type" "cs") (set_attr "cpu_facility" "*,*,z10,*")]) --- 3038,3045 (match_operand:BLK 1 "memory_operand" "Q,Q,Q,Q"))) (use (match_operand 2 "nonmemory_operand" "n,a,a,a")) (use (match_operand 3 "immediate_operand" "X,R,X,X")) !(clobber (match_scratch:P 4 "=X,X,X,&a"))] ! "(GET_MODE (operands[2]) == Pmode || GET_MODE (operands[2]) == VOIDmode)" "#" [(set_attr "type" "cs") (set_attr "cpu_facility" "*,*,z10,*")])
Re: [C++ Patch] PR 53166
OK. Jason
libgo patch committed: Update to Go 1.0.1 release
This patch updates libgo to the Go 1.0.1 release. This is a relatively
small collection of bug fixes, with no API changes. Bootstrapped and
ran Go testsuite on x86_64-unknown-linux-gnu. Committed to mainline and
4.7 branch.
Ian
diff -r a152568e294a libgo/MERGE
--- a/libgo/MERGE Thu May 03 22:23:14 2012 -0700
+++ b/libgo/MERGE Fri May 04 07:51:01 2012 -0700
@@ -1,4 +1,4 @@
-dc5e410f0b4c
+2ccfd4b451d3
The first line of this file holds the Mercurial revision number of the
last merge done from the master library sources.
diff -r a152568e294a libgo/go/crypto/tls/cipher_suites.go
--- a/libgo/go/crypto/tls/cipher_suites.go Thu May 03 22:23:14 2012 -0700
+++ b/libgo/go/crypto/tls/cipher_suites.go Fri May 04 07:51:01 2012 -0700
@@ -23,8 +23,8 @@
// In the case that the key agreement protocol doesn't use a
// ServerKeyExchange message, generateServerKeyExchange can return nil,
// nil.
- generateServerKeyExchange(*Config, *clientHelloMsg, *serverHelloMsg) (*serverKeyExchangeMsg, error)
- processClientKeyExchange(*Config, *clientKeyExchangeMsg, uint16) ([]byte, error)
+ generateServerKeyExchange(*Config, *Certificate, *clientHelloMsg, *serverHelloMsg) (*serverKeyExchangeMsg, error)
+ processClientKeyExchange(*Config, *Certificate, *clientKeyExchangeMsg, uint16) ([]byte, error)
// On the client side, the next two methods are called in order.
diff -r a152568e294a libgo/go/crypto/tls/handshake_server.go
--- a/libgo/go/crypto/tls/handshake_server.go Thu May 03 22:23:14 2012 -0700
+++ b/libgo/go/crypto/tls/handshake_server.go Fri May 04 07:51:01 2012 -0700
@@ -112,37 +112,38 @@
hello.nextProtoNeg = true
hello.nextProtos = config.NextProtos
}
- if clientHello.ocspStapling && len(config.Certificates[0].OCSPStaple) > 0 {
+
+ if len(config.Certificates) == 0 {
+ return c.sendAlert(alertInternalError)
+ }
+ cert := &config.Certificates[0]
+ if len(clientHello.serverName) > 0 {
+ c.serverName = clientHello.serverName
+ cert = config.getCertificateForName(clientHello.serverName)
+ }
+
+ if clientHello.ocspStapling && len(cert.OCSPStaple) > 0 {
hello.ocspStapling = true
}
finishedHash.Write(hello.marshal())
c.writeRecord(recordTypeHandshake, hello.marshal())
- if len(config.Certificates) == 0 {
- return c.sendAlert(alertInternalError)
- }
-
certMsg := new(certificateMsg)
- if len(clientHello.serverName) > 0 {
- c.serverName = clientHello.serverName
- certMsg.certificates = config.getCertificateForName(clientHello.serverName).Certificate
- } else {
- certMsg.certificates = config.Certificates[0].Certificate
- }
+ certMsg.certificates = cert.Certificate
finishedHash.Write(certMsg.marshal())
c.writeRecord(recordTypeHandshake, certMsg.marshal())
if hello.ocspStapling {
certStatus := new(certificateStatusMsg)
certStatus.statusType = statusTypeOCSP
- certStatus.response = config.Certificates[0].OCSPStaple
+ certStatus.response = cert.OCSPStaple
finishedHash.Write(certStatus.marshal())
c.writeRecord(recordTypeHandshake, certStatus.marshal())
}
keyAgreement := suite.ka()
- skx, err := keyAgreement.generateServerKeyExchange(config, clientHello, hello)
+ skx, err := keyAgreement.generateServerKeyExchange(config, cert, clientHello, hello)
if err != nil {
c.sendAlert(alertHandshakeFailure)
return err
@@ -288,7 +289,7 @@
finishedHash.Write(certVerify.marshal())
}
- preMasterSecret, err := keyAgreement.processClientKeyExchange(config, ckx, c.vers)
+ preMasterSecret, err := keyAgreement.processClientKeyExchange(config, cert, ckx, c.vers)
if err != nil {
c.sendAlert(alertHandshakeFailure)
return err
diff -r a152568e294a libgo/go/crypto/tls/handshake_server_test.go
--- a/libgo/go/crypto/tls/handshake_server_test.go Thu May 03 22:23:14 2012 -0700
+++ b/libgo/go/crypto/tls/handshake_server_test.go Fri May 04 07:51:01 2012 -0700
@@ -37,9 +37,12 @@
testConfig = new(Config)
testConfig.Time = func() time.Time { return time.Unix(0, 0) }
testConfig.Rand = zeroSource{}
- testConfig.Certificates = make([]Certificate, 1)
+ testConfig.Certificates = make([]Certificate, 2)
testConfig.Certificates[0].Certificate = [][]byte{testCertificate}
testConfig.Certificates[0].PrivateKey = testPrivateKey
+ testConfig.Certificates[1].Certificate = [][]byte{testSNICertificate}
+ testConfig.Certificates[1].PrivateKey = testPrivateKey
+ testConfig.BuildNameToCertificate()
testConfig.CipherSuites = []uint16{TLS_RSA_WITH_RC4_128_SHA}
testConfig.InsecureSkipVerify = true
}
@@ -176,6 +179,13 @@
testServerScript(t, "SSLv3", sslv3ServerScript, testConfig, nil)
}
+// TestHandshakeServerSNI involves a client sending an SNI extension of
+// "snitest.com", which happens to match the CN of testSNICertificate. The test
+// verifies that the server correctly selects that certificate.
+func TestHandshakeServerSNI(t *testing.T) {
+ testServerScript(t, "SNI", selectCertificateBySNIScript, testConfig, nil)
+}
+
type clientauthTest struct {
name string
clientauth
Re: PR c++/24985 caret diagnostics for macro expansions
On 05/03/2012 08:30 PM, Manuel López-Ibáñez wrote: Do you have an opinion about the macro unwinder? I agree. Jason
Re: [Patch, fortran] PR 49010/24518 MOD/MODULO fixes, take 2
On 02/05/2012 21:22, Janne Blomqvist wrote: > PING #2 > > On Thu, Apr 26, 2012 at 12:20 AM, Janne Blomqvist > wrote: >> PING! >> >> On Thu, Apr 19, 2012 at 00:46, Janne Blomqvist >> wrote: >>> Hi, >>> >>> the attached patch implements a few fixes and cleanups for the MOD and >>> MODULO intrinsics. >>> >>> - When the arguments are constant, use mpfr_fmod instead of the naive >>> algorithms which are numerically unstable for large arguments. This >>> extends the PR 24518 fix to constant arguments as well, and makes the >>> compile-time evaluation match the runtime implementation which also >>> uses fmod in the same manner. >>> >>> - Remove the old fallback path for the case builtin_fmod is not >>> available, as the builtin is AFAICS always available. >>> >>> - Specify the behavior wrt. the sign and magnitude of the result, >>> mention this in the documentation. This includes signed zeros which >>> now behave similar to other finite values. I.e. for MOD(A, P) the >>> result has the sign of A and a magnitude less than that of P, and for >>> MODULO(A, P) the result has the sign of P and a magnitude less than >>> that of P. As a (minor?) caveat, at runtime this depends on the >>> implementation of the fmod function in the target C library. But, a >>> fmod that follows C99 Annex F implements this behavior. >>> >>> Regtested on x86_64-unknown-linux-gnu, Ok for trunk? >>> >>> 2012-04-19 Janne Blomqvist >>> >>>PR fortran/49010 >>>PR fortran/24518 >>>* intrinsic.texi (MOD, MODULO): Mention sign and magnitude of result. >>>* simplify.c (gfc_simplify_mod): Use mpfr_fmod. >>>(gfc_simplify_modulo): Likewise, use copysign to fix the result if >>>zero. >>>* trans-intrinsic.c (gfc_conv_intrinsic_mod): Remove fallback as >>>builtin_fmod is always available. For modulo, call copysign to fix >>>the result when signed zeros are enabled. >>> >>> >>> -- >>> Janne Blomqvist >> >> >> >> -- >> Janne Blomqvist > > > Hello, this looks good. OK with a testcase. Thanks, and sorry for the delay. Mikael
Re: strengthen protection against REG_EQUIV/EQUAL on !REG set
On May 4, 2012, at 16:16 , Richard Sandiford wrote: > Sorry, was going to test this earlier, but got distracted by > lower-subreg stuff. No problem at all. I just happened to have had an opportunity to test as part of a series of miscellaneous other submissions. > I need to fix the subreg handling so that > we check whether the inner part of a SUBREG is a REG (done in > my copy at home). Indeed. That was in the original patch and I missed the difference in the alternate version. > I also wanted to make sure there were no > asm differences due to notes being wrongly dropped. Ah, nice :) Thanks much for your feedback, Olivier
Re: [PATCH] PR preprocessor/7263 - Avoid pedantic warnings on system headers macro tokens
It seems like for most of the declspecs, having both the location and a
flag/count is unnecessary; instead of checking the flag, we can check
whether the location is set. That won't work for the "long long long"
check, but it should work for the others. You'll need to change
_check_decl_spec to take the new declspec and location rather than have
it iterate every time, but that seems worth doing anyway.
So I don't think we need both cp_decl_spec and cp_decl_spec_word.
error_at (location, "%<__thread%> before %qD", ridpointers[keyword]);
decl_specs->specs[(int) ds_thread] = 0;
+ decl_specs->locations[cpdw_thread] = location;
This error should use the stored __thread location, and then set it to 0
rather than the location argument.
+ decl_specs->locations[cpdw_builtin_type_spec] = location;
if (!decl_specs->type)
{
decl_specs->type = type_spec;
decl_specs->type_definition_p = false;
- decl_specs->type_location = location;
+ decl_specs->locations[cpdw_type_spec] = location;
Why do we need cpdw_builtin_type_spec?
Jason
Re: [PATCH] PR preprocessor/7263 - Avoid pedantic warnings on system headers macro tokens
Jason Merrill writes:
>> + decl_specs->locations[cpdw_builtin_type_spec] = location;
>>if (!decl_specs->type)
>> {
>>decl_specs->type = type_spec;
>>decl_specs->type_definition_p = false;
>> - decl_specs->type_location = location;
>> + decl_specs->locations[cpdw_type_spec] = location;
>
> Why do we need cpdw_builtin_type_spec?
To unify the way we access locations for the specifiers. Otherwise, it
looks awkward to have a lone decl_specs->type_location for the type
specifier, whereas the other specifiers have their locations in
decl_specs->locations. But if you don't like that, I'll just revert it.
--
Dodji
[PATCH, i386]: Fix PR 53228, target attributes in libcpp/lex.c cause illegal instructions to be used elsewhere
Hello!
There is a problem with TARGET_CMOVE handling, where in
ix86_option_override_internal, we set TARGET_CMOVE behind option
processing system back. This flag is not cleared during target
attribute handling, and we emit CMOV elsewhere.
Attached patch redefines TARGET_CMOVE as dynamic check, avoiding
re-initialization of ix86_arch_features array.
2012-05-04 Uros Bizjak
PR target/53228
* config/i386/i386.h (X86_ARCH_CMOV): Rename from X86_ARCH_CMOVE.
(TARGET_CMOV): Rename from TARGET_CMOVE.
(TARGET_CMOVE): New define.
* config/i386/i386.c (ix86_option_override_internal): Use TARGET_CMOV.
Do not set TARGET_CMOVE here.
Patch was bootstrapped and regression tested on x86_64-pc-linux-gnu
{,-m32}. I also checked that lex.i test from the PR doesn't generate
unwanted cmov insns.
Committed to mainline SVN, will be backported to all release branches.
Uros.
Index: i386.c
===
--- i386.c (revision 187099)
+++ i386.c (working copy)
@@ -2190,7 +2190,7 @@ unsigned char ix86_arch_features[X86_ARCH_LAST];
/* Feature tests against the various architecture variations, used to create
ix86_arch_features based on the processor mask. */
static unsigned int initial_ix86_arch_features[X86_ARCH_LAST] = {
- /* X86_ARCH_CMOVE: Conditional move was added for pentiumpro. */
+ /* X86_ARCH_CMOV: Conditional move was added for pentiumpro. */
~(m_386 | m_486 | m_PENT | m_K6),
/* X86_ARCH_CMPXCHG: Compare and exchange was added for 80486. */
@@ -3504,7 +3504,7 @@ ix86_option_override_internal (bool main_args_p)
-mtune (rather than -march) points us to a processor that has them.
However, the VIA C3 gives a SIGILL, so we only do that for i686 and
higher processors. */
- if (TARGET_CMOVE
+ if (TARGET_CMOV
&& (processor_alias_table[i].flags & (PTA_PREFETCH_SSE | PTA_SSE)))
x86_prefetch_sse = true;
break;
@@ -3780,12 +3780,6 @@ ix86_option_override_internal (bool main_args_p)
target_flags |= MASK_ACCUMULATE_OUTGOING_ARGS;
}
- /* For sane SSE instruction set generation we need fcomi instruction.
- It is safe to enable all CMOVE instructions. Also, RDRAND intrinsic
- expands to a sequence that includes conditional move. */
- if (TARGET_SSE || TARGET_RDRND)
-TARGET_CMOVE = 1;
-
/* Figure out what ASM_GENERATE_INTERNAL_LABEL builds as a prefix. */
{
char *p;
Index: i386.h
===
--- i386.h (revision 187099)
+++ i386.h (working copy)
@@ -430,7 +430,7 @@ extern unsigned char ix86_tune_features[X86_TUNE_L
/* Feature tests against the various architecture variations. */
enum ix86_arch_indices {
- X86_ARCH_CMOVE, /* || TARGET_SSE */
+ X86_ARCH_CMOV,
X86_ARCH_CMPXCHG,
X86_ARCH_CMPXCHG8B,
X86_ARCH_XADD,
@@ -441,12 +441,17 @@ enum ix86_arch_indices {
extern unsigned char ix86_arch_features[X86_ARCH_LAST];
-#define TARGET_CMOVE ix86_arch_features[X86_ARCH_CMOVE]
+#define TARGET_CMOVix86_arch_features[X86_ARCH_CMOV]
#define TARGET_CMPXCHG ix86_arch_features[X86_ARCH_CMPXCHG]
#define TARGET_CMPXCHG8B ix86_arch_features[X86_ARCH_CMPXCHG8B]
#define TARGET_XADDix86_arch_features[X86_ARCH_XADD]
#define TARGET_BSWAP ix86_arch_features[X86_ARCH_BSWAP]
+/* For sane SSE instruction set generation we need fcomi instruction.
+ It is safe to enable all CMOVE instructions. Also, RDRAND intrinsic
+ expands to a sequence that includes conditional move. */
+#define TARGET_CMOVE (TARGET_CMOV || TARGET_SSE || TARGET_RDRND)
+
#define TARGET_FISTTP (TARGET_SSE3 && TARGET_80387)
extern int x86_prefetch_sse;
Re: [PATCH] Take branch misprediction effects into account when RTL loop unrolling (issue 6099055)
It might be better to separate the data structure name change (niter_desc to loop_desc) into a different patch. Other than that, the patch looks good to me (for google branches only) Unroller really needs more heuristics like this instead of just looking at size. David http://codereview.appspot.com/6099055/
[PATCH C/C++] Enable -Wunused-local-typedefs when -Wall or -Wunused is on
Hello,
Now that the libstdc++ testsuite is run with -ftrack-macro-location by
default, this patch triggers the -Wunused-local-typedefs warning when
-Wunused (and -Wall) is turned on.
The patch has been reviewed and accepted[1] a while ago, but was waiting
for the -ftrack-macro-expansion work to go in first.
Bootstrapped and tested again on x86_64-unknown-linux-gnu against
trunk.
Applied to the mainline.
[1]: http://gcc.gnu.org/ml/gcc-patches/2011-09/msg00605.html
gcc/
* opts.c (finish_options): Activate -Wunused-local-typedefs if
-Wunused is activated.
* doc/invoke.texi: Update blurb of -Wunused-local-typedefs.
---
gcc/ChangeLog |7 +++
gcc/doc/invoke.texi |1 +
gcc/opts.c |4
3 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 2e3fa0c..d372a4f 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,10 @@
+2012-05-04 Dodji Seketeli
+
+ Enable -Wunused-local-typedefs when -Wall or -Wunused is on
+ * opts.c (finish_options): Activate -Wunused-local-typedefs if
+ -Wunused is activated.
+ * doc/invoke.texi: Update blurb of -Wunused-local-typedefs.
+
2012-05-04 Andreas Krebbel
* config/s390/s390.md (*movmem_short, *clrmem_short)
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 5550c71..86698fc 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -3622,6 +3622,7 @@ To suppress this warning use the @samp{unused} attribute
@item -Wunused-local-typedefs @r{(C, Objective-C, C++ and Objective-C++ only)}
@opindex Wunused-local-typedefs
Warn when a typedef locally defined in a function is not used.
+This warning is enabled by @option{-Wall}.
@item -Wunused-parameter
@opindex Wunused-parameter
diff --git a/gcc/opts.c b/gcc/opts.c
index 22c7590..b6c786f 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -835,6 +835,10 @@ finish_options (struct gcc_options *opts, struct
gcc_options *opts_set,
if (opts->x_warn_unused_value == -1)
opts->x_warn_unused_value = opts->x_warn_unused;
+ /* Wunused-local-typedefs is enabled by -Wunused or -Wall. */
+ if (opts->x_warn_unused_local_typedefs == -1)
+opts->x_warn_unused_local_typedefs = opts->x_warn_unused;
+
/* This replaces set_Wextra. */
if (opts->x_warn_uninitialized == -1)
opts->x_warn_uninitialized = opts->x_extra_warnings;
--
Dodji
Re: [PATCH C/C++] Enable -Wunused-local-typedefs when -Wall or -Wunused is on
On 05/04/2012 06:40 PM, Dodji Seketeli wrote: Hello, Now that the libstdc++ testsuite is run with -ftrack-macro-location by default, this patch triggers the -Wunused-local-typedefs warning when -Wunused (and -Wall) is turned on. Yeah!!! Paolo.
Re: [PATCH] Improve COND_EXPR expansion
On 05/04/12 00:30, Andrew Pinski wrote:
> +static rtx
> +expand_cond_expr_using_cmove (tree treeop0, tree treeop1, tree treeop2)
> +{
> +#ifdef HAVE_conditional_move
You'll need ATTRIBUTE_UNUSED markers here.
Otherwise ok.
r~
Merge from gcc-4_7-branch to gccgo branch
I've merged revision 187165 from gcc-4_7-branch to the gccgo branch. Ian
[patch] Remove ATTR_FLAGs for branch probabilities
Hello, This patch removes some unused ATTR_FLAG values, that were added by Jeff in r3904 (sic) but apparently have never been used. The purpose of the flags is to pass an indication for the probability that a branch is taken to the "define_delay"s, but only the HP-PA port uses ATTR_FLAG, and even then it only uses ATTR_FLAG_forward and ATTR_FLAG_backward. Tested with a cross-compiler from x86_64-linux-gnu to hppa-linux-gnu and verified that there are no new warnings. OK for trunk? Ciao! Steven janitor_dbr_sched.diff Description: Binary data
Re: [PATCH] PR preprocessor/7263 - Avoid pedantic warnings on system headers macro tokens
On 05/04/2012 12:32 PM, Dodji Seketeli wrote:
Jason Merrill writes:
+ decl_specs->locations[cpdw_builtin_type_spec] = location;
if (!decl_specs->type)
{
decl_specs->type = type_spec;
decl_specs->type_definition_p = false;
- decl_specs->type_location = location;
+ decl_specs->locations[cpdw_type_spec] = location;
Why do we need cpdw_builtin_type_spec?
To unify the way we access locations for the specifiers. Otherwise, it
looks awkward to have a lone decl_specs->type_location for the type
specifier, whereas the other specifiers have their locations in
decl_specs->locations. But if you don't like that, I'll just revert it.
No, I agree that cpdw_type_spec makes sense. I just don't see why we
need the _builtin_ variant.
Jason
Re: fix mem+32760 ICE on powerpc - provide/use predicates weaker than mode_dependent_address_p
This is achieved by the introduction of a TARGET_MAY_NARROW_ACCESS target hook, which defaults to !mode_dependent_address_p and is redefined for powerpc. The patch uses this hook directly instead of the former predicate in a couple of places where this was the intent already, as well as new "valid_access_mode_change_p" function to direct to one or the or the other depending on provided original and destination modes. This provides a better match for actual internal needs, allows to get rid of the powerpc back-end twists (no need to lie in mode_dependent_address_p any more) and cures the observed internal compiler error. The patch was bootstrapped and regression testsed for mainline on both x86_64 and powerpc-linux. We (AdaCore) also have been using a gcc-4.5 minor variation of it without problem for a couple of years on powerpc-aix. OK to commit ? * target.def (TARGET_MAY_NARROW_ACCESS_TO): New hook. * doc/tm.texi[.in] (TARGET_MAY_NARROW_ACCESS_TO): Document. * targhooks.c (default_may_narrow_access_to): Default implementation. * targhooks.h (default_may_narrow_access_to): Declare. * config/rs6000/rs6000.c (rs6000_may_narrow_access_to): Specific implementation. (rs6000_mode_dependent_address): Stop lying for + const_int. (rs6000_offsettable_memref_p): Adjust comments accordingly. * expr.c (convert_move): Use may_narrow_access_to instead of mode_dependent_address_p where appropriate. * recog.c (offsettable_address_addr_space_p): Likewise. (valid_access_mode_change_p): New function. * recog.h (valid_access_mode_change_p): Declare. * simplify-rtx.c (simplify_subreg): Use it instead of mode_dependent_address_p where appropriate. testsuite/ * gcc.dg/offsetmem.c: New test. I cannot approve the common parts of the patch, but the rs6000 parts are okay. The new comment in rs6000_mode_dependent_address adapted from the original comment above the hook should mention VSX in addition to Altivec for clarity. Thanks, David
Re: [patch] Remove ATTR_FLAGs for branch probabilities
On 05/04/12 11:08, Steven Bosscher wrote: > * rtl.def (ATTR_FLAG): Remove probability indicating flags. > * genattr.c (main): Remove ATTR_FLAG_likely, ATTR_FLAG_unlikely, > ATTR_FLAG_very_likely, and ATTR_FLAG_very_unlikely. > * reorg.c (get_jump_flags): Do not set the removed flags. Ok. r~
Go patch committed: Fix parse of for with possible composite literal
This patch to the Go frontend fixes a bad parse. For code like
for first := true; first; first = false {
the frontend would erroneously assume that "false {" could be the start
of a composite literal. It can't be, because composite literals in a
for statement must be parenthesized. Bootstrapped and ran Go testsuite
on x86_64-unknown-linux-gnu. Committed to mainline and gccgo branch.
Ian
diff -r eb21fb5e5da4 go/parse.cc
--- a/go/parse.cc Fri May 04 07:55:30 2012 -0700
+++ b/go/parse.cc Fri May 04 12:28:35 2012 -0700
@@ -126,18 +126,22 @@
// ExpressionList = Expression { "," Expression } .
+// If MAY_BE_COMPOSITE_LIT is true, an expression may be a composite
+// literal.
+
// If MAY_BE_SINK is true, the expressions in the list may be "_".
Expression_list*
-Parse::expression_list(Expression* first, bool may_be_sink)
+Parse::expression_list(Expression* first, bool may_be_sink,
+ bool may_be_composite_lit)
{
Expression_list* ret = new Expression_list();
if (first != NULL)
ret->push_back(first);
while (true)
{
- ret->push_back(this->expression(PRECEDENCE_NORMAL, may_be_sink, true,
- NULL));
+ ret->push_back(this->expression(PRECEDENCE_NORMAL, may_be_sink,
+ may_be_composite_lit, NULL));
const Token* token = this->peek_token();
if (!token->is_op(OPERATOR_COMMA))
@@ -1425,7 +1429,7 @@
else
{
this->advance_token();
- expr_list = this->expression_list(NULL, false);
+ expr_list = this->expression_list(NULL, false, true);
*last_type = type;
if (*last_expr_list != NULL)
delete *last_expr_list;
@@ -1575,13 +1579,13 @@
if (this->peek_token()->is_op(OPERATOR_EQ))
{
this->advance_token();
- init = this->expression_list(NULL, false);
+ init = this->expression_list(NULL, false, true);
}
}
else
{
this->advance_token();
- init = this->expression_list(NULL, false);
+ init = this->expression_list(NULL, false, true);
}
this->init_vars(&til, type, init, false, location);
@@ -1988,6 +1992,9 @@
// In order to support both "a, b := 1, 0" and "a, b = 1, 0" we accept
// tuple assignments here as well.
+// If MAY_BE_COMPOSITE_LIT is true, the expression on the right hand
+// side may be a composite literal.
+
// If P_RANGE_CLAUSE is not NULL, then this will recognize a
// RangeClause.
@@ -1997,6 +2004,7 @@
void
Parse::simple_var_decl_or_assignment(const std::string& name,
Location location,
+ bool may_be_composite_lit,
Range_clause* p_range_clause,
Type_switch* p_type_switch)
{
@@ -2053,14 +2061,15 @@
exprs->push_back(this->id_to_expression(p->name(),
p->location()));
- Expression_list* more_exprs = this->expression_list(NULL, true);
+ Expression_list* more_exprs =
+ this->expression_list(NULL, true, may_be_composite_lit);
for (Expression_list::const_iterator p = more_exprs->begin();
p != more_exprs->end();
++p)
exprs->push_back(*p);
delete more_exprs;
- this->tuple_assignment(exprs, p_range_clause);
+ this->tuple_assignment(exprs, may_be_composite_lit, p_range_clause);
return;
}
}
@@ -2076,11 +2085,12 @@
Expression_list* init;
if (p_type_switch == NULL)
-init = this->expression_list(NULL, false);
+init = this->expression_list(NULL, false, may_be_composite_lit);
else
{
bool is_type_switch = false;
- Expression* expr = this->expression(PRECEDENCE_NORMAL, false, true,
+ Expression* expr = this->expression(PRECEDENCE_NORMAL, false,
+ may_be_composite_lit,
&is_type_switch);
if (is_type_switch)
{
@@ -2099,7 +2109,7 @@
else
{
this->advance_token();
- init = this->expression_list(expr, false);
+ init = this->expression_list(expr, false, may_be_composite_lit);
}
}
@@ -3065,7 +3075,7 @@
const Token* token = this->advance_token();
if (!token->is_op(OPERATOR_RPAREN))
{
- args = this->expression_list(NULL, false);
+ args = this->expression_list(NULL, false, true);
token = this->peek_token();
if (token->is_op(OPERATOR_ELLIPSIS))
{
@@ -3578,6 +3588,7 @@
{
identifier = this->gogo_->pack_hidden_name(identifier, is_exported);
this->simple_var_decl_or_assignment(identifier, location,
+ may_be_composite_lit,
p_range_clause,
(token->is_op(OPERATOR_COLONEQ)
? p_type_switch
@@ -3613,7 +3624,7 @@
this->inc_dec_stat(this->verify_not_sink(exp));
else if (token->is_op(OPERATOR_COMMA)
|| token->is_op(OPERATOR_EQ))
-this->assignment(exp, p_range_clause);
+this->assignment(exp, may_be_composite_lit, p_range_clause);
else if (token->is_op(OPERATOR_PLUSEQ)
|| token->is_op(OPERATOR_MINUSEQ)
|| token->is_op(OPERATOR_OREQ)
@@ -3625,7 +3636,8 @@
|| token->is_op(OPERATOR_RSHIFTEQ)
|| token->is_op(OPERATOR_ANDEQ)
|| token->is_op(OPERA
[patch] Make reload_cse_regs static
Hello,
This patch makes reload_cse_regs static because it is never called
outside of postreload.c.
Will commit next week, after the usual testing and if no-one objects.
Ciao!
Steven
* postreload.c (reload_cse_regs): Make static.
* reload.h (reload_cse_regs): Remove prototype.
Index: postreload.c
===
--- postreload.c(revision 187177)
+++ postreload.c(working copy)
@@ -64,7 +64,8 @@ static void move2add_note_store (rtx, co
/* Call cse / combine like post-reload optimization phases.
FIRST is the first instruction. */
-void
+
+static void
reload_cse_regs (rtx first ATTRIBUTE_UNUSED)
{
bool moves_converted;
Index: reload.h
===
--- reload.h(revision 187177)
+++ reload.h(working copy)
@@ -411,9 +411,6 @@ extern int push_reload (rtx, rtx, rtx *,
enum machine_mode, enum machine_mode,
int, int, int, enum reload_type);
-/* Functions in postreload.c: */
-extern void reload_cse_regs (rtx);
-
/* Functions in reload1.c: */
/* Initialize the reload pass once per compilation. */
Re: [patch] PR29442 - split insn-attrtab.c in three parts
On 05/04/12 06:54, Steven Bosscher wrote: > Attached is another attempt. It's a much larger patch because I've now > had to change all write_* functions in genattrtab.c to pass around a > FILE pointer. But it's not a gross as the first attempt. Hopefully you > agree :-) I do agree! > Bootstrapped&tested again on x86_64-unknown-linux-gnu. OK? Ok. r~
Another merge from gcc-4_7-branch to gccgo
I have merged gcc-4_7-branch revision 187177 to gccgo branch. Ian
[C++ Patch] Minor cxx_eval_constant_expression clean-up
Hi,
I was having a look to c++/52282 and noticed the this code can be tidied
a little bit (similarly to case ADDR_EXPR). Patch tested x86_64-linux.
Ok?
Thanks,
Paolo.
/
2012-05-04 Paolo Carlini
* semantics.c (cxx_eval_constant_expression, case CONVERT_EXPR): Tidy.
Index: semantics.c
===
--- semantics.c (revision 187162)
+++ semantics.c (working copy)
@@ -7757,18 +7757,16 @@ cxx_eval_constant_expression (const constexpr_call
case NOP_EXPR:
{
tree oldop = TREE_OPERAND (t, 0);
- tree op = oldop;
- tree to = TREE_TYPE (t);
- op = cxx_eval_constant_expression (call, TREE_OPERAND (t, 0),
- allow_non_constant, addr,
- non_constant_p);
+ tree op = cxx_eval_constant_expression (call, oldop,
+ allow_non_constant, addr,
+ non_constant_p);
if (*non_constant_p)
return t;
if (op == oldop)
/* We didn't fold at the top so we could check for ptr-int
conversion. */
return fold (t);
- r = fold_build1 (TREE_CODE (t), to, op);
+ r = fold_build1 (TREE_CODE (t), TREE_TYPE (t), op);
/* Conversion of an out-of-range value has implementation-defined
behavior; the language considers it different from arithmetic
overflow, which is undefined. */
Re: [PATCH 2/2] Minor refactoring of tree-vect-patterns.c
On Fri, May 4, 2012 at 5:30 AM, Ulrich Weigand wrote: > Richard Guenther wrote: >> On Mon, Apr 30, 2012 at 6:19 PM, Ulrich Weigand wrote: >> > Hello, >> > >> > as a second step in refactoring this patch introduces a routine >> > vect_find_single_use to determine whether a defining statement >> > has one single use within the current vectorization domain. >> > >> > The helper is then called wherever that check is currently >> > open-coded. =A0There should be no change in behaviour. >> > >> > Tested on i386-linux-gnu and arm-linux-gnueabi with no regressions. >> > >> > OK for mainline? >> >> You can use single_imm_use to avoid the loop and simplify the factored >> routine. >> >> Ok with that change. > > I've checked in the version appended below. > > ChangeLog: > > * tree-vect-patterns.c (vect_single_imm_use): New function. > (vect_recog_widen_mult_pattern): Use it instead of open-coding loop. > (vect_recog_over_widening_pattern): Likewise. > (vect_recog_widen_shift_pattern): Likewise. It looks like you checked in a version of this patch to gcc-4_7-branch. However, the version you committed used "false" where it should have used "NULL", causing a bootstrap failure in stage 2 when not using building in release mode: ../../gccgo-gcc/gcc/tree-vect-patterns.c: In function ‘gimple_statement_d* vect_single_imm_use(gimple)’: ../../gccgo-gcc/gcc/tree-vect-patterns.c:104:12: error: converting ‘false’ to pointer type ‘gimple’ [-Werror=conversion-null] ../../gccgo-gcc/gcc/tree-vect-patterns.c:107:12: error: converting ‘false’ to pointer type ‘gimple’ [-Werror=conversion-null] I committed the appended patch to fix this problem. Please double-check the code to make sure it is correct. Thanks. Patch bootstrapped on x86_64-unknown-linux-gnu but not tested. Committed to 4.7 branch as obvious. Ian 2012-05-04 Ian Lance Taylor * tree-vect-patterns.c (vect_single_imm_use): Correct return values from false to NULL. Index: gcc/tree-vect-patterns.c === --- gcc/tree-vect-patterns.c(revision 187181) +++ gcc/tree-vect-patterns.c(working copy) @@ -101,10 +101,10 @@ vect_single_imm_use (gimple def_stmt) return NULL; if (!gimple_bb (use_stmt)) -return false; +return NULL; if (!flow_bb_inside_loop_p (loop, gimple_bb (use_stmt))) -return false; +return NULL; gcc_assert (vinfo_for_stmt (use_stmt)); return use_stmt;
Re: fix mem+32760 ICE on powerpc - provide/use predicates weaker than mode_dependent_address_p
On May 4, 2012, at 20:42 , David Edelsohn wrote: > I cannot approve the common parts of the patch, but the rs6000 parts are okay. That's a good start, Thanks! > The new comment in rs6000_mode_dependent_address adapted from the > original comment above the hook should mention VSX in addition to > Altivec for clarity. Sure. I'll be happy to update if (when) the patch eventually gets in. Thanks for your feedback, Eager to hear comments on the common parts :) With Kind Regards, Olivier
Re: [C++ Patch] Minor cxx_eval_constant_expression clean-up
OK. Jason
[PATCH] Take branch misprediction effects into account when RTL loop unrolling (issue6099055)
On David's suggestion, I have removed the changes that rename niter_desc to
loop_desc from this patch to focus the patch on the unrolling changes. I can
submit a cleanup patch to do the renaming as soon as this one goes in.
Bootstrapped and tested on x86_64-unknown-linux-gnu. Ok for trunk?
Thanks,
Teresa
Here is the new description of improvements from the original patch:
Improved patch based on feedback. Main changes are:
1) Improve efficiency by caching loop analysis results in the loop auxiliary
info structure hanging off the loop structure. Added a new routine,
analyze_loop_insns, to fill in information about the average and total number
of branches, as well as whether there are any floating point set and call
instructions in the loop. The new routine is invoked when we first create a
loop's niter_desc struct, and the caller (get_simple_loop_desc) has been
modified to handle creating a niter_desc for the fake outermost loop.
2) Improvements to max_unroll_with_branches:
- Treat the fake outermost loop (the procedure body) as we would a hot outer
loop, i.e. compute the max unroll looking at its nested branches, instead of
shutting off unrolling when we reach the fake outermost loop.
- Pull the checks previously done in the caller into the routine (e.g.
whether the loop iterates frequently or contains fp instructions).
- Fix a bug in the previous version that sometimes caused overflow in the
new unroll factor.
3) Remove float variables, and use integer computation to compute the
average number of branches in the loop.
4) Detect more types of floating point computations in the loop by walking
all set instructions, not just single sets.
2012-05-04 Teresa Johnson
* doc/invoke.texi: Update the documentation with new params.
* loop-unroll.c (max_unroll_with_branches): New function.
(decide_unroll_constant_iterations, decide_unroll_runtime_iterations):
Add heuristic to avoid increasing branch mispredicts when unrolling.
(decide_peel_simple, decide_unroll_stupid): Retrieve number of
branches from niter_desc instead of via function that walks loop.
* loop-iv.c (get_simple_loop_desc): Invoke new analyze_loop_insns
function, and add guards to enable this function to work for the
outermost loop.
* cfgloop.c (insn_has_fp_set, analyze_loop_insns): New functions.
(num_loop_branches): Remove.
* cfgloop.h (struct loop_desc): Added new fields to cache additional
loop analysis information.
(num_loop_branches): Remove.
(analyze_loop_insns): Declare.
* params.def (PARAM_MIN_ITER_UNROLL_WITH_BRANCHES): New param.
(PARAM_UNROLL_OUTER_LOOP_BRANCH_BUDGET): Ditto.
Index: doc/invoke.texi
===
--- doc/invoke.texi (revision 187013)
+++ doc/invoke.texi (working copy)
@@ -8842,6 +8842,12 @@ The maximum number of insns of an unswitched loop.
@item max-unswitch-level
The maximum number of branches unswitched in a single loop.
+@item min-iter-unroll-with-branches
+Minimum iteration count to ignore branch effects when unrolling.
+
+@item unroll-outer-loop-branch-budget
+Maximum number of branches allowed in hot outer loop region after unroll.
+
@item lim-expensive
The minimum cost of an expensive expression in the loop invariant motion.
Index: loop-unroll.c
===
--- loop-unroll.c (revision 187013)
+++ loop-unroll.c (working copy)
@@ -152,6 +152,99 @@ static void combine_var_copies_in_loop_exit (struc
basic_block);
static rtx get_expansion (struct var_to_expand *);
+/* Compute the maximum number of times LOOP can be unrolled without exceeding
+ a branch budget, which can increase branch mispredictions. The number of
+ branches is computed by weighting each branch with its expected execution
+ probability through the loop based on profile data. If no profile feedback
+ data exists, simply return the current NUNROLL factor. */
+
+static unsigned
+max_unroll_with_branches(struct loop *loop, unsigned nunroll)
+{
+ struct loop *outer;
+ struct niter_desc *outer_desc = 0;
+ int outer_niters = 1;
+ int frequent_iteration_threshold;
+ unsigned branch_budget;
+ struct niter_desc *desc = get_simple_loop_desc (loop);
+
+ /* Ignore loops with FP computation as these tend to benefit much more
+ consistently from unrolling. */
+ if (desc->has_fp)
+return nunroll;
+
+ frequent_iteration_threshold = PARAM_VALUE
(PARAM_MIN_ITER_UNROLL_WITH_BRANCHES);
+ if (expected_loop_iterations (loop) >= (unsigned)
frequent_iteration_threshold)
+return nunroll;
+
+ /* If there was no profile feedback data, av_num_branches will be 0
+ and we won't limit unrolling. If the av_num_branches is at most 1,
+ also don't limit unrolling as the back-edge branch will not be
duplicated. */
Re: [C Patch]: pr52543
Mike Stump schrieb: On May 3, 2012, at 12:50 PM, Georg-Johann Lay wrote: It's hardly possible to write proper rtx_costs for SET: 1) What should be the cost of (const_int 1) if you don't see the machine mode? Is it QI, is it HI, is it SI or whatever? You can choose to see the complete expression in its entirety and form the cost for it. (set (reg:DI 1) (const_int 1)) Sorry, for the dumb question, but I still don't get it. TARGET_RTX_COSTS gets called with x = (const_int 1) and outer = SET for example. How do I get SET_DEST from that information? I don't now if lower-subreg.s ever emits such cost requests, but several passes definitely do. is a DI mode set of register 1, with the value 1. This can have a different cost, from the perspective of the cost function from: (set (reg:SI 1 (const_int 1)) or even (set (reg:SI 1 (const_int 2)) or (set (reg:SI 2 (const_int 1)) though, little else in the compiler would help mitigate such differences. Just return a non-zero value from the cost function. If you return zero, you are saying that you don't care, or that the costs compose in a simplistic manner. Try returning 1, and figuring out the total cost of the entire expression yourself, if the simplistic answer is wrong. TARGET_RTX_COSTS does not mention the "0 = don't know" in any way. There are post-reload passes that mention it in source comments, yes. 2) If the target will be a REG, what is the register class for the assignment? Hard register has a class associated with it that can be found with REGNO_REG_CLASS (REGNO (x)). If the register [...] Thanks for the detailed explanation. But again the question was for the case when TARGET_RTX_COSTS is called with outer = SET and x = (const_int ?). then, no class has been chosen for it yet, and the register allocator can choose any valid class it wants for the mode. I suspect you're better off explaining the average cost (or maybe even the best case cost), when multiple choices exist. In general, someone else should check later the true cost of the operation based upon the class and should be willing to influence the code generated (the class picked), so what you return for rtx_costs shouldn't matter too much. rtx_costs are called after reload, so it would be good to know. It would be good to know if it is a pseudo or hard reg. HARD_REGISTER_NUM_P (REGNO (x)) will tell you if it is hard. The inverse of this will tell you if is isn't hard, aka, a pseudo. And in many places the backend does not know where it is standing. Is it upon expanding? Prior or after combine? Or split1? I think the idea is to give the cost of the rtl it asks for. (set (reg:SI 1) (const_int 0)) should have the same cost, before reload, after reload, during optimization, just before final, or at expand time. Just grepped a log from avr-gcc -mlog=rtx_costs but there was not a single line with outer=pattern. In some cases like insn-combine where there is a complete pattern, synthesized from several insns, but just SET_DEST is passed to rtx_costs hiding informations from the backends in an unnecessary way; presumably for historical reasons. There are machines with complex instructions sets like 4-operand OR and combined OR and SHIFT or similar. Instead of writing hundreds or thousands of lines in rtx_costs and XEXP TARGET_RTX_COSTS (effectively rewriting insn-recog.c) it would be straight forward to attach costs to insns and use recog + insn_attr to get the costs. I tried that approach (write costs as insn attribute and use recog to get the costs) in a backend and it works smooth and really helped to keep the backend clean and maintainable. 3) Likewise, the costs of MEM are peeled of MEM and pass just the address without any information on the MEM like it's address space. Cost might highly depend on the address space involved. Yes, that is why on my machine: (set (mem) (reg)) has one set of costs, and (set (reg) (mem)) What hook are we talking about? TARGET_RTX_COSTS? (not called with outer=PATTERN) TARGET_MEMORY_MOVE_COST? (uses register classes) TARGET_ADDRESS_COST (no address space available as MEM was peeled) has a completely different set of costs. The address space, if it influences the cost, can be had with: MEM_ADDR_SPACE (XEXP (x, 0)) or a store, and: MEM_ADDR_SPACE (XEXP (x, 1)) for a load. The original PR is because split of mem:HI is fine -- if it reads from generic. And splitting mem:HI is complete bloat for other address spaces. Likewise for wider modes like PSI, SI, ... The you will want to check the mode of the MEM, and address space involved. The issue is to *get* the MEM. To hack around PR52543 for PSImode (there is just one AS that uses PSImode addresses), I used TARGET_MODE_DEPENDENT_ADDRESS_P, see respective FIXME in avr.c. But again no avail to get the AS. AS is attached to the MEM, not to the address, i.e. XEXP (mem, 0) which is passed to that hook. Thanks for all your detailed descriptions. Johann
