From: Carrot Wei <carrot@google.com>
To: Richard Earnshaw <rearnsha@arm.com>,
Ian Lance Taylor <iant@google.com>,
gcc-patches@gcc.gnu.org
Subject: Re: [PATCH: PR target/41705] Enable if conversion for thumb1 by new HAVE_conditional_execution definition
Date: Sun, 25 Oct 2009 14:22:00 -0000 [thread overview]
Message-ID: <7587b290910250710x719d7fe2w43b709b34a7ea67e@mail.gmail.com> (raw)
In-Reply-To: <1256315468.1606.20.camel@e200601-lin.cambridge.arm.com>
Changes done.
X86 is such a target that doesn't define the macro HAVE_conditional_execution.
I did the following testing without new failure:
Bootstrap on x86_64.
Gcc regression test on x86_64.
Gcc regression test on arm emulator with newlib.
ChangeLog:
2009-10-25 Wei Guozhi <carrot@google.com>
PR target/41705
* target.h (have_conditional_execution): Add a new target hook function.
* target-def.h (TARGET_HAVE_CONDITIONAL_EXECUTION): Likewise.
* targhooks.h (default_have_conditional_execution): Likewise.
* targhooks.c (default_have_conditional_execution): Likewise.
* doc/tm.texi (TARGET_HAVE_CONDITIONAL_EXECUTION): Document it.
* config/arm/arm.c (TARGET_HAVE_CONDITIONAL_EXECUTION): Define it.
(arm_have_conditional_execution): New function.
* ifcvt.c (noce_process_if_block, find_if_header,
cond_exec_find_if_block, dead_or_predicable): Change the usage of macro
HAVE_conditional_execution to a target hook call.
* recog.c (peephole2_optimize): Likewise.
* sched-rgn.c (add_branch_dependences): Likewise.
* final.c (asm_insn_count, final_scan_insn): Likewise.
* bb-reorder.c (HAVE_conditional_execution): Remove it.
thanks
Wei Guozhi
Index: target.h
===================================================================
--- target.h (revision 152888)
+++ target.h (working copy)
@@ -615,6 +615,9 @@ struct gcc_target
already been generated. */
bool (* branch_target_register_callee_saved) (bool after_pe_gen);
+ /* Return true if the target supports conditional execution. */
+ bool (* have_conditional_execution) (void);
+
/* True if the constant X cannot be placed in the constant pool. */
bool (* cannot_force_const_mem) (rtx);
Index: target-def.h
===================================================================
--- target-def.h (revision 152888)
+++ target-def.h (working copy)
@@ -492,6 +492,7 @@
#define TARGET_BRANCH_TARGET_REGISTER_CLASS \
default_branch_target_register_class
#define TARGET_BRANCH_TARGET_REGISTER_CALLEE_SAVED hook_bool_bool_false
+#define TARGET_HAVE_CONDITIONAL_EXECUTION default_have_conditional_execution
#define TARGET_CANNOT_FORCE_CONST_MEM hook_bool_rtx_false
#define TARGET_CANNOT_COPY_INSN_P NULL
#define TARGET_COMMUTATIVE_P hook_bool_const_rtx_commutative_p
@@ -892,6 +893,7 @@
TARGET_CANNOT_MODIFY_JUMPS_P, \
TARGET_BRANCH_TARGET_REGISTER_CLASS, \
TARGET_BRANCH_TARGET_REGISTER_CALLEE_SAVED, \
+ TARGET_HAVE_CONDITIONAL_EXECUTION, \
TARGET_CANNOT_FORCE_CONST_MEM, \
TARGET_CANNOT_COPY_INSN_P, \
TARGET_COMMUTATIVE_P, \
Index: targhooks.h
===================================================================
--- targhooks.h (revision 152888)
+++ targhooks.h (working copy)
@@ -120,3 +120,4 @@ extern bool default_target_option_valid_
extern bool default_target_option_pragma_parse (tree, tree);
extern bool default_target_can_inline_p (tree, tree);
extern unsigned int default_case_values_threshold (void);
+extern bool default_have_conditional_execution (void);
Index: targhooks.c
===================================================================
--- targhooks.c (revision 152888)
+++ targhooks.c (working copy)
@@ -892,4 +892,14 @@ unsigned int default_case_values_thresho
return (HAVE_casesi ? 4 : 5);
}
+bool
+default_have_conditional_execution (void)
+{
+#ifdef HAVE_conditional_execution
+ return HAVE_conditional_execution;
+#else
+ return false;
+#endif
+}
+
#include "gt-targhooks.h"
Index: tm.texi
===================================================================
--- tm.texi (revision 152888)
+++ tm.texi (working copy)
@@ -10733,6 +10733,12 @@ to have to make special provisions in @c
to reserve space for caller-saved target registers.
@end deftypefn
+@deftypefn {Target Hook} bool TARGET_HAVE_CONDITIONAL_EXECUTION (void)
+This target hook returns true if the target supports conditional execution.
+This target hook is required only when the target has several different
+modes and they have different conditional execution capability, such as ARM.
+@end deftypefn
+
@defmac POWI_MAX_MULTS
If defined, this macro is interpreted as a signed integer C expression
that specifies the maximum number of floating point multiplications
Index: arm.c
===================================================================
--- arm.c (revision 152888)
+++ arm.c (working copy)
@@ -138,6 +138,7 @@ static rtx arm_libcall_value (enum machi
static void arm_internal_label (FILE *, const char *, unsigned long);
static void arm_output_mi_thunk (FILE *, tree, HOST_WIDE_INT, HOST_WIDE_INT,
tree);
+static bool arm_have_conditional_execution (void);
static bool arm_rtx_costs_1 (rtx, enum rtx_code, int*, bool);
static bool arm_size_rtx_costs (rtx, enum rtx_code, enum rtx_code, int *);
static bool arm_slowmul_rtx_costs (rtx, enum rtx_code, enum rtx_code,
int *, bool);
@@ -445,6 +446,9 @@ static const struct attribute_spec arm_a
#define TARGET_HAVE_TLS true
#endif
+#undef TARGET_HAVE_CONDITIONAL_EXECUTION
+#define TARGET_HAVE_CONDITIONAL_EXECUTION arm_have_conditional_execution
+
#undef TARGET_CANNOT_FORCE_CONST_MEM
#define TARGET_CANNOT_FORCE_CONST_MEM arm_cannot_force_const_mem
@@ -21173,4 +21177,12 @@ arm_frame_pointer_required (void)
|| (TARGET_ARM && TARGET_APCS_FRAME && ! leaf_function_p ()));
}
+/* Only thumb1 can't support conditional execution, so return true if
+ the target is not thumb1. */
+static bool
+arm_have_conditional_execution (void)
+{
+ return !TARGET_THUMB1;
+}
+
#include "gt-arm.h"
Index: ifcvt.c
===================================================================
--- ifcvt.c (revision 152888)
+++ ifcvt.c (working copy)
@@ -47,9 +47,6 @@
#include "vecprim.h"
#include "dbgcnt.h"
-#ifndef HAVE_conditional_execution
-#define HAVE_conditional_execution 0
-#endif
#ifndef HAVE_conditional_move
#define HAVE_conditional_move 0
#endif
@@ -2419,7 +2416,7 @@ noce_process_if_block (struct noce_if_in
if (HAVE_conditional_move
&& noce_try_cmove (if_info))
goto success;
- if (! HAVE_conditional_execution)
+ if (! targetm.have_conditional_execution ())
{
if (noce_try_store_flag_constants (if_info))
goto success;
@@ -3063,7 +3060,7 @@ find_if_header (basic_block test_bb, int
&& noce_find_if_block (test_bb, then_edge, else_edge, pass))
goto success;
- if (HAVE_conditional_execution && reload_completed
+ if (targetm.have_conditional_execution () && reload_completed
&& cond_exec_find_if_block (&ce_info))
goto success;
@@ -3073,7 +3070,7 @@ find_if_header (basic_block test_bb, int
goto success;
if (dom_info_state (CDI_POST_DOMINATORS) >= DOM_NO_FAST_QUERY
- && (! HAVE_conditional_execution || reload_completed))
+ && (! targetm.have_conditional_execution () || reload_completed))
{
if (find_if_case_1 (test_bb, then_edge, else_edge))
goto success;
@@ -3180,7 +3177,7 @@ cond_exec_find_if_block (struct ce_if_bl
/* We only ever should get here after reload,
and only if we have conditional execution. */
- gcc_assert (HAVE_conditional_execution && reload_completed);
+ gcc_assert (targetm.have_conditional_execution () && reload_completed);
/* Discover if any fall through predecessors of the current test basic block
were && tests (which jump to the else block) or || tests (which jump to
@@ -3858,7 +3855,7 @@ dead_or_predicable (basic_block test_bb,
/* Disable handling dead code by conditional execution if the machine needs
to do anything funny with the tests, etc. */
#ifndef IFCVT_MODIFY_TESTS
- if (HAVE_conditional_execution)
+ if (targetm.have_conditional_execution ())
{
/* In the conditional execution case, we have things easy. We know
the condition is reversible. We don't have to check life info
Index: recog.c
===================================================================
--- recog.c (revision 152888)
+++ recog.c (working copy)
@@ -3267,40 +3267,44 @@ peephole2_optimize (void)
do_cleanup_cfg |= purge_dead_edges (bb);
}
-#ifdef HAVE_conditional_execution
- for (i = 0; i < MAX_INSNS_PER_PEEP2 + 1; ++i)
- peep2_insn_data[i].insn = NULL_RTX;
- peep2_insn_data[peep2_current].insn = PEEP2_EOB;
- peep2_current_count = 0;
-#else
- /* Back up lifetime information past the end of the
- newly created sequence. */
- if (++i >= MAX_INSNS_PER_PEEP2 + 1)
- i = 0;
- bitmap_copy (live, peep2_insn_data[i].live_before);
-
- /* Update life information for the new sequence. */
- x = attempt;
- do
- {
- if (INSN_P (x))
- {
- if (--i < 0)
- i = MAX_INSNS_PER_PEEP2;
- if (peep2_current_count < MAX_INSNS_PER_PEEP2
- && peep2_insn_data[i].insn == NULL_RTX)
- peep2_current_count++;
- peep2_insn_data[i].insn = x;
- df_insn_rescan (x);
- df_simulate_one_insn_backwards (bb, x, live);
- bitmap_copy (peep2_insn_data[i].live_before, live);
- }
- x = PREV_INSN (x);
- }
- while (x != prev);
+ if (targetm.have_conditional_execution ())
+ {
+ for (i = 0; i < MAX_INSNS_PER_PEEP2 + 1; ++i)
+ peep2_insn_data[i].insn = NULL_RTX;
+ peep2_insn_data[peep2_current].insn = PEEP2_EOB;
+ peep2_current_count = 0;
+ }
+ else
+ {
+ /* Back up lifetime information past the end of the
+ newly created sequence. */
+ if (++i >= MAX_INSNS_PER_PEEP2 + 1)
+ i = 0;
+ bitmap_copy (live, peep2_insn_data[i].live_before);
+
+ /* Update life information for the new sequence. */
+ x = attempt;
+ do
+ {
+ if (INSN_P (x))
+ {
+ if (--i < 0)
+ i = MAX_INSNS_PER_PEEP2;
+ if (peep2_current_count < MAX_INSNS_PER_PEEP2
+ && peep2_insn_data[i].insn == NULL_RTX)
+ peep2_current_count++;
+ peep2_insn_data[i].insn = x;
+ df_insn_rescan (x);
+ df_simulate_one_insn_backwards (bb, x, live);
+ bitmap_copy (peep2_insn_data[i].live_before,
+ live);
+ }
+ x = PREV_INSN (x);
+ }
+ while (x != prev);
- peep2_current = i;
-#endif
+ peep2_current = i;
+ }
/* If we generated a jump instruction, it won't have
JUMP_LABEL set. Recompute after we're done. */
Index: sched-rgn.c
===================================================================
--- sched-rgn.c (revision 152888)
+++ sched-rgn.c (working copy)
@@ -2508,7 +2508,9 @@ add_branch_dependences (rtx head, rtx ta
add_dependence (last, insn, REG_DEP_ANTI);
}
-#ifdef HAVE_conditional_execution
+ if (!targetm.have_conditional_execution ())
+ return;
+
/* Finally, if the block ends in a jump, and we are doing intra-block
scheduling, make sure that the branch depends on any COND_EXEC insns
inside the block to avoid moving the COND_EXECs past the branch insn.
@@ -2557,7 +2559,6 @@ add_branch_dependences (rtx head, rtx ta
if (INSN_P (insn) && GET_CODE (PATTERN (insn)) == COND_EXEC)
add_dependence (tail, insn, REG_DEP_ANTI);
}
-#endif
}
Index: final.c
===================================================================
--- final.c (revision 152888)
+++ final.c (working copy)
@@ -204,10 +204,8 @@ rtx final_sequence;
static int dialect_number;
#endif
-#ifdef HAVE_conditional_execution
/* Nonnull if the insn currently being emitted was a COND_EXEC pattern. */
rtx current_insn_predicate;
-#endif
#ifdef HAVE_ATTR_length
static int asm_insn_count (rtx);
@@ -2102,10 +2100,9 @@ final_scan_insn (rtx insn, FILE *file, i
const char *templ;
bool is_stmt;
-#ifdef HAVE_conditional_execution
/* Reset this early so it is correct for ASM statements. */
current_insn_predicate = NULL_RTX;
-#endif
+
/* An INSN, JUMP_INSN or CALL_INSN.
First check for special kinds that recog doesn't recognize. */
@@ -2590,10 +2587,9 @@ final_scan_insn (rtx insn, FILE *file, i
FINAL_PRESCAN_INSN (insn, recog_data.operand, recog_data.n_operands);
#endif
-#ifdef HAVE_conditional_execution
- if (GET_CODE (PATTERN (insn)) == COND_EXEC)
+ if (targetm.have_conditional_execution ()
+ && GET_CODE (PATTERN (insn)) == COND_EXEC)
current_insn_predicate = COND_EXEC_TEST (PATTERN (insn));
-#endif
#ifdef HAVE_cc0
cc_prev_status = cc_status;
Index: bb-reorder.c
===================================================================
--- bb-reorder.c (revision 152888)
+++ bb-reorder.c (working copy)
@@ -86,10 +86,6 @@
#include "tree-pass.h"
#include "df.h"
-#ifndef HAVE_conditional_execution
-#define HAVE_conditional_execution 0
-#endif
-
/* The number of rounds. In most cases there will only be 4 rounds, but
when partitioning hot and cold basic blocks into separate sections of
the .o file there will be an extra round.*/
On Sat, Oct 24, 2009 at 12:31 AM, Richard Earnshaw <rearnsha@arm.com> wrote:
>
> On Wed, 2009-10-21 at 20:52 -0700, Ian Lance Taylor wrote:
>> Carrot Wei <carrot@google.com> writes:
>>
>> > +bool default_have_conditional_execution (void)
>> > +{
>>
>> Line break after "bool".
>>
>>
>> > +/* Only thumb1 can't support conditional execution, so return true if
>> > + the target is not thumb1. */
>> > +bool
>> > +arm_have_conditional_execution (void)
>> > +{
>> > + return !TARGET_THUMB1;
>> > +}
>> > +
>>
>> Make this function static. Add a static declaration at the top of the
>> file near the other static declarations. Define is higher in the
>> file, perhaps around the cost functions.
>>
>>
>> > Index: config/arm/arm-protos.h
>> > ===================================================================
>> > --- config/arm/arm-protos.h (revision 152888)
>> > +++ config/arm/arm-protos.h (working copy)
>> > @@ -41,7 +41,8 @@ extern HOST_WIDE_INT thumb_compute_initi
>> > unsigned int);
>> > extern unsigned int arm_dbx_register_number (unsigned int);
>> > extern void arm_output_fn_unwind (FILE *, bool);
>> > -
>> > +extern bool arm_have_conditional_execution (void);
>>
>> Do not declare this here.
>>
>>
>> > @@ -146,7 +147,8 @@ extern const char *vfp_output_fstmd (rtx
>> > extern void arm_set_return_address (rtx, rtx);
>> > extern int arm_eliminable_register (rtx);
>> > extern const char *arm_output_shift(rtx *, int);
>> > -
>> > +extern bool removable_cmp_0 (rtx cmp_op);
>> > +extern const char *emit_branch_after_movs (rtx *operands);
>> > extern bool arm_output_addr_const_extra (FILE *, rtx);
>>
>> This seems like an unrelated change which should not be in this patch.
>>
>>
>> Please send a new patch. Please also test the new patch on x86 or
>> x86_64 GNU/Linux.
>>
>> I would like to hear if the ARM maintainers have any comments.
>>
>
> This looks OK to me with the changes Ian has requested. However, it
> causes code that was previously conditionalized on
> HAVE_conditional_execution to be unconditionally compiled, so please
> also make sure that the code builds cleanly for at least one target that
> does not define that macro.
>
> R.
>
>
>
next prev parent reply other threads:[~2009-10-25 14:10 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-10-16 13:28 Carrot Wei
2009-10-17 1:40 ` Ian Lance Taylor
2009-10-17 6:01 ` Carrot Wei
2009-10-17 6:01 ` Ian Lance Taylor
2009-10-17 6:18 ` Carrot Wei
2009-10-19 14:35 ` Carrot Wei
2009-10-22 5:29 ` Ian Lance Taylor
2009-10-23 17:11 ` Richard Earnshaw
2009-10-25 14:22 ` Carrot Wei [this message]
2009-10-25 15:55 ` Ian Lance Taylor
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7587b290910250710x719d7fe2w43b709b34a7ea67e@mail.gmail.com \
--to=carrot@google.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=iant@google.com \
--cc=rearnsha@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).