* [RFC] Request for comments on ivopts patch
@ 2015-12-08 18:56 Steve Ellcey
2015-12-09 10:24 ` Richard Biener
0 siblings, 1 reply; 6+ messages in thread
From: Steve Ellcey @ 2015-12-08 18:56 UTC (permalink / raw)
To: gcc-patches; +Cc: amker.cheng
I have an ivopts optimization question/proposal. When compiling the
attached program the ivopts pass prefers the original ivs over new ivs
and that causes us to generate less efficient code on MIPS. It may
affect other platforms too.
The Source code is a C strcmp:
int strcmp (const char *p1, const char *p2)
{
const unsigned char *s1 = (const unsigned char *) p1;
const unsigned char *s2 = (const unsigned char *) p2;
unsigned char c1, c2;
do {
c1 = (unsigned char) *s1++;
c2 = (unsigned char) *s2++;
if (c1 == '\0') return c1 - c2;
} while (c1 == c2);
return c1 - c2;
}
Currently the code prefers the original ivs and so it generates
code that increments s1 and s2 before doing the loads (and uses
a -1 offset):
<bb 3>:
# s1_1 = PHI <p1_4(D)(2), s1_6(6)>
# s2_2 = PHI <p2_5(D)(2), s2_9(6)>
s1_6 = s1_1 + 1;
c1_8 = MEM[base: s1_6, offset: 4294967295B];
s2_9 = s2_2 + 1;
c2_10 = MEM[base: s2_9, offset: 4294967295B];
if (c1_8 == 0)
goto <bb 4>;
else
goto <bb 5>;
If I remove the cost increment for non-original ivs then GCC
does the loads before the increments:
<bb 3>:
# ivtmp.6_17 = PHI <ivtmp.6_24(2), ivtmp.6_14(6)>
# ivtmp.7_21 = PHI <ivtmp.7_22(2), ivtmp.7_23(6)>
_25 = (void *) ivtmp.6_17;
c1_8 = MEM[base: _25, offset: 0B];
_26 = (void *) ivtmp.7_21;
c2_10 = MEM[base: _26, offset: 0B];
if (c1_8 == 0)
goto <bb 4>;
else
goto <bb 5>;
.
.
<bb 5>:
ivtmp.6_14 = ivtmp.6_17 + 1;
ivtmp.7_23 = ivtmp.7_21 + 1;
if (c1_8 == c2_10)
goto <bb 6>;
else
goto <bb 7>;
This second case (without the preference for the original IV)
generates better code on MIPS because the final assembly
has the increment instructions between the loads and the tests
of the values being loaded and so there is no delay (or less delay)
between the load and use. It seems like this could easily be
the case for other platforms too so I was wondering what people
thought of this patch:
2015-12-08 Steve Ellcey <sellcey@imgtec.com>
* tree-ssa-loop-ivopts.c (determine_iv_cost): Remove preference for
original ivs.
diff --git a/gcc/tree-ssa-loop-ivopts.c b/gcc/tree-ssa-loop-ivopts.c
index 98dc451..26daabc 100644
--- a/gcc/tree-ssa-loop-ivopts.c
+++ b/gcc/tree-ssa-loop-ivopts.c
@@ -5818,14 +5818,6 @@ determine_iv_cost (struct ivopts_data *data, struct iv_cand *cand)
cost = cost_step + adjust_setup_cost (data, cost_base.cost);
- /* Prefer the original ivs unless we may gain something by replacing it.
- The reason is to make debugging simpler; so this is not relevant for
- artificial ivs created by other optimization passes. */
- if (cand->pos != IP_ORIGINAL
- || !SSA_NAME_VAR (cand->var_before)
- || DECL_ARTIFICIAL (SSA_NAME_VAR (cand->var_before)))
- cost++;
-
/* Prefer not to insert statements into latch unless there are some
already (so that we do not create unnecessary jumps). */
if (cand->pos == IP_END
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] Request for comments on ivopts patch
2015-12-08 18:56 [RFC] Request for comments on ivopts patch Steve Ellcey
@ 2015-12-09 10:24 ` Richard Biener
2015-12-11 23:48 ` Steve Ellcey
0 siblings, 1 reply; 6+ messages in thread
From: Richard Biener @ 2015-12-09 10:24 UTC (permalink / raw)
To: Steve Ellcey; +Cc: GCC Patches, amker cheng
On Tue, Dec 8, 2015 at 7:56 PM, Steve Ellcey <sellcey@imgtec.com> wrote:
> I have an ivopts optimization question/proposal. When compiling the
> attached program the ivopts pass prefers the original ivs over new ivs
> and that causes us to generate less efficient code on MIPS. It may
> affect other platforms too.
>
> The Source code is a C strcmp:
>
> int strcmp (const char *p1, const char *p2)
> {
> const unsigned char *s1 = (const unsigned char *) p1;
> const unsigned char *s2 = (const unsigned char *) p2;
> unsigned char c1, c2;
> do {
> c1 = (unsigned char) *s1++;
> c2 = (unsigned char) *s2++;
> if (c1 == '\0') return c1 - c2;
> } while (c1 == c2);
> return c1 - c2;
> }
>
> Currently the code prefers the original ivs and so it generates
> code that increments s1 and s2 before doing the loads (and uses
> a -1 offset):
>
> <bb 3>:
> # s1_1 = PHI <p1_4(D)(2), s1_6(6)>
> # s2_2 = PHI <p2_5(D)(2), s2_9(6)>
> s1_6 = s1_1 + 1;
> c1_8 = MEM[base: s1_6, offset: 4294967295B];
> s2_9 = s2_2 + 1;
> c2_10 = MEM[base: s2_9, offset: 4294967295B];
> if (c1_8 == 0)
> goto <bb 4>;
> else
> goto <bb 5>;
>
> If I remove the cost increment for non-original ivs then GCC
> does the loads before the increments:
>
> <bb 3>:
> # ivtmp.6_17 = PHI <ivtmp.6_24(2), ivtmp.6_14(6)>
> # ivtmp.7_21 = PHI <ivtmp.7_22(2), ivtmp.7_23(6)>
> _25 = (void *) ivtmp.6_17;
> c1_8 = MEM[base: _25, offset: 0B];
> _26 = (void *) ivtmp.7_21;
> c2_10 = MEM[base: _26, offset: 0B];
> if (c1_8 == 0)
> goto <bb 4>;
> else
> goto <bb 5>;
> .
> .
> <bb 5>:
> ivtmp.6_14 = ivtmp.6_17 + 1;
> ivtmp.7_23 = ivtmp.7_21 + 1;
> if (c1_8 == c2_10)
> goto <bb 6>;
> else
> goto <bb 7>;
>
>
> This second case (without the preference for the original IV)
> generates better code on MIPS because the final assembly
> has the increment instructions between the loads and the tests
> of the values being loaded and so there is no delay (or less delay)
> between the load and use. It seems like this could easily be
> the case for other platforms too so I was wondering what people
> thought of this patch:
You don't comment on the comment you remove ... debugging
programs is also important!
So if then the cost of both cases should be distinguished
somewhere else, like granting a bonus for increment before
exit test or so.
Richard.
> 2015-12-08 Steve Ellcey <sellcey@imgtec.com>
>
> * tree-ssa-loop-ivopts.c (determine_iv_cost): Remove preference for
> original ivs.
>
>
> diff --git a/gcc/tree-ssa-loop-ivopts.c b/gcc/tree-ssa-loop-ivopts.c
> index 98dc451..26daabc 100644
> --- a/gcc/tree-ssa-loop-ivopts.c
> +++ b/gcc/tree-ssa-loop-ivopts.c
> @@ -5818,14 +5818,6 @@ determine_iv_cost (struct ivopts_data *data, struct iv_cand *cand)
>
> cost = cost_step + adjust_setup_cost (data, cost_base.cost);
>
> - /* Prefer the original ivs unless we may gain something by replacing it.
> - The reason is to make debugging simpler; so this is not relevant for
> - artificial ivs created by other optimization passes. */
> - if (cand->pos != IP_ORIGINAL
> - || !SSA_NAME_VAR (cand->var_before)
> - || DECL_ARTIFICIAL (SSA_NAME_VAR (cand->var_before)))
> - cost++;
> -
> /* Prefer not to insert statements into latch unless there are some
> already (so that we do not create unnecessary jumps). */
> if (cand->pos == IP_END
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] Request for comments on ivopts patch
2015-12-09 10:24 ` Richard Biener
@ 2015-12-11 23:48 ` Steve Ellcey
2015-12-14 8:57 ` Richard Biener
0 siblings, 1 reply; 6+ messages in thread
From: Steve Ellcey @ 2015-12-11 23:48 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches, amker cheng
On Wed, 2015-12-09 at 11:24 +0100, Richard Biener wrote:
> > This second case (without the preference for the original IV)
> > generates better code on MIPS because the final assembly
> > has the increment instructions between the loads and the tests
> > of the values being loaded and so there is no delay (or less delay)
> > between the load and use. It seems like this could easily be
> > the case for other platforms too so I was wondering what people
> > thought of this patch:
>
> You don't comment on the comment you remove ... debugging
> programs is also important!
>
> So if then the cost of both cases should be distinguished
> somewhere else, like granting a bonus for increment before
> exit test or so.
>
> Richard.
Here is new patch that tries to do that. It accomplishes the same thing
as my original patch but by checking different features. Basically, for
machines with no autoinc/autodec it has a preference for IVs that don't
change during loop (i.e. var_before == var_after).
What do you think about this approach?
Steve Ellcey
sellcey@imgtec.com
2015-12-11 Steve Ellcey <sellcey@imgtec.com>
* tree-ssa-loop-ivopts.c (determine_iv_cost): Add cost to ivs that
need to be updated during loop.
diff --git a/gcc/tree-ssa-loop-ivopts.c b/gcc/tree-ssa-loop-ivopts.c
index 98dc451..ecf9737 100644
--- a/gcc/tree-ssa-loop-ivopts.c
+++ b/gcc/tree-ssa-loop-ivopts.c
@@ -5826,6 +5826,14 @@ determine_iv_cost (struct ivopts_data *data, struct iv_cand *cand)
|| DECL_ARTIFICIAL (SSA_NAME_VAR (cand->var_before)))
cost++;
+ /* If we are not using autoincrement or autodecrement, prefer ivs that
+ do not have to be incremented/decremented during the loop. This can
+ move loads ahead of the instructions that update the address. */
+ if (cand->pos != IP_BEFORE_USE
+ && cand->pos != IP_AFTER_USE
+ && cand->var_before != cand->var_after)
+ cost++;
+
/* Prefer not to insert statements into latch unless there are some
already (so that we do not create unnecessary jumps). */
if (cand->pos == IP_END
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] Request for comments on ivopts patch
2015-12-11 23:48 ` Steve Ellcey
@ 2015-12-14 8:57 ` Richard Biener
2015-12-14 23:06 ` Steve Ellcey
0 siblings, 1 reply; 6+ messages in thread
From: Richard Biener @ 2015-12-14 8:57 UTC (permalink / raw)
To: sellcey; +Cc: GCC Patches, amker cheng
On Sat, Dec 12, 2015 at 12:48 AM, Steve Ellcey <sellcey@imgtec.com> wrote:
> On Wed, 2015-12-09 at 11:24 +0100, Richard Biener wrote:
>
>> > This second case (without the preference for the original IV)
>> > generates better code on MIPS because the final assembly
>> > has the increment instructions between the loads and the tests
>> > of the values being loaded and so there is no delay (or less delay)
>> > between the load and use. It seems like this could easily be
>> > the case for other platforms too so I was wondering what people
>> > thought of this patch:
>>
>> You don't comment on the comment you remove ... debugging
>> programs is also important!
>>
>> So if then the cost of both cases should be distinguished
>> somewhere else, like granting a bonus for increment before
>> exit test or so.
>>
>> Richard.
>
> Here is new patch that tries to do that. It accomplishes the same thing
> as my original patch but by checking different features. Basically, for
> machines with no autoinc/autodec it has a preference for IVs that don't
> change during loop (i.e. var_before == var_after).
>
> What do you think about this approach?
>
> Steve Ellcey
> sellcey@imgtec.com
>
>
> 2015-12-11 Steve Ellcey <sellcey@imgtec.com>
>
> * tree-ssa-loop-ivopts.c (determine_iv_cost): Add cost to ivs that
> need to be updated during loop.
>
>
> diff --git a/gcc/tree-ssa-loop-ivopts.c b/gcc/tree-ssa-loop-ivopts.c
> index 98dc451..ecf9737 100644
> --- a/gcc/tree-ssa-loop-ivopts.c
> +++ b/gcc/tree-ssa-loop-ivopts.c
> @@ -5826,6 +5826,14 @@ determine_iv_cost (struct ivopts_data *data, struct iv_cand *cand)
> || DECL_ARTIFICIAL (SSA_NAME_VAR (cand->var_before)))
> cost++;
>
> + /* If we are not using autoincrement or autodecrement, prefer ivs that
> + do not have to be incremented/decremented during the loop. This can
> + move loads ahead of the instructions that update the address. */
> + if (cand->pos != IP_BEFORE_USE
> + && cand->pos != IP_AFTER_USE
> + && cand->var_before != cand->var_after)
> + cost++;
> +
I don't know enough to assess the effect of this but
1) not all archs can do auto-incdec so either the comment is misleading
or the test should probably be amended
2) I wonder why with the comment ("during the loop") you exclude IP_NORMAL/END
that said, the comment needs to explain the situation better.
Of course all such patches need some code-gen effect investigation
on more than one arch.
[I wonder if a IV cost adjust target hook makes sense at some point]
Richard.
> /* Prefer not to insert statements into latch unless there are some
> already (so that we do not create unnecessary jumps). */
> if (cand->pos == IP_END
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] Request for comments on ivopts patch
2015-12-14 8:57 ` Richard Biener
@ 2015-12-14 23:06 ` Steve Ellcey
2015-12-16 13:43 ` Richard Biener
0 siblings, 1 reply; 6+ messages in thread
From: Steve Ellcey @ 2015-12-14 23:06 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches, amker cheng
On Mon, 2015-12-14 at 09:57 +0100, Richard Biener wrote:
> I don't know enough to assess the effect of this but
>
> 1) not all archs can do auto-incdec so either the comment is misleading
> or the test should probably be amended
> 2) I wonder why with the comment ("during the loop") you exclude IP_NORMAL/END
>
> that said, the comment needs to explain the situation better.
>
> Of course all such patches need some code-gen effect investigation
> on more than one arch.
>
> [I wonder if a IV cost adjust target hook makes sense at some point]
>
> Richard.
I like the idea of a target hook to modify IV costs. What do you think
about this? I had to move some structures from tree-ssa-loop-ivopts.c
to tree-ssa-loop-ivopts.h in order to give a target hooks access to
information on the IV candidates.
Steve Ellcey
sellcey@imgtec.com
2015-12-14 Steve Ellcey <sellcey@imgtec.com>
* doc/tm.texi.in (TARGET_ADJUST_IV_CAND_COST): New target function.
* target.def (adjust_iv_cand_cost): New target function.
* target.h (struct iv_cand): New forward declaration.
* targhooks.c (default_adjust_iv_cand_cost): New default function.
* targhooks.h (default_adjust_iv_cand_cost): Ditto.
* tree-ssa-loop-ivopts.c (struct iv, enum iv_position, struct iv_cand)
Moved to tree-ssa-loop-ivopts.h.
(determine_iv_cost): Add call to targetm.adjust_iv_cand_cost.
* tree-ssa-loop-ivopts.h (struct iv, enum iv_position, struct iv_cand)
Copied here from tree-ssa-loop-ivopts.h.
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index a0a0a81..1ad4c2d 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -8221,6 +8221,8 @@ and the associated definitions of those functions.
@hook TARGET_OFFLOAD_OPTIONS
+@hook TARGET_ADJUST_IV_CAND_COST
+
@defmac TARGET_SUPPORTS_WIDE_INT
On older ports, large integers are stored in @code{CONST_DOUBLE} rtl
diff --git a/gcc/target.def b/gcc/target.def
index d754337..6bdcfcc 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -5846,6 +5846,12 @@ DEFHOOK
void, (tree *hold, tree *clear, tree *update),
default_atomic_assign_expand_fenv)
+DEFHOOK
+(adjust_iv_cand_cost,
+"Allow target to modify the cost of a possible induction variable.",
+void, (struct iv_cand *cand),
+ default_adjust_iv_cand_cost)
+
/* Leave the boolean fields at the end. */
/* True if we can create zeroed data by switching to a BSS section
diff --git a/gcc/target.h b/gcc/target.h
index ffc4d6a..6f55575 100644
--- a/gcc/target.h
+++ b/gcc/target.h
@@ -139,6 +139,9 @@ struct ao_ref;
/* This is defined in tree-vectorizer.h. */
struct _stmt_vec_info;
+/* This is defined in tree-ivopts.h. */
+struct iv_cand;
+
/* These are defined in tree-vect-stmts.c. */
extern tree stmt_vectype (struct _stmt_vec_info *);
extern bool stmt_in_inner_loop_p (struct _stmt_vec_info *);
diff --git a/gcc/targhooks.c b/gcc/targhooks.c
index dcf0863..0d0bbfc 100644
--- a/gcc/targhooks.c
+++ b/gcc/targhooks.c
@@ -1961,4 +1961,10 @@ default_optab_supported_p (int, machine_mode, machine_mode, optimization_type)
return true;
}
+/* Default implementation of TARGET_ADJUST_IV_CAND_COST. */
+void
+default_adjust_iv_cand_cost (struct iv_cand *)
+{
+}
+
#include "gt-targhooks.h"
diff --git a/gcc/targhooks.h b/gcc/targhooks.h
index 47b5cfc..dd0481d 100644
--- a/gcc/targhooks.h
+++ b/gcc/targhooks.h
@@ -253,4 +253,6 @@ extern void default_setup_incoming_vararg_bounds (cumulative_args_t ca ATTRIBUTE
extern bool default_optab_supported_p (int, machine_mode, machine_mode,
optimization_type);
+extern void default_adjust_iv_cand_cost (struct iv_cand *);
+
#endif /* GCC_TARGHOOKS_H */
diff --git a/gcc/tree-ssa-loop-ivopts.c b/gcc/tree-ssa-loop-ivopts.c
index 98dc451..5bfd232 100644
--- a/gcc/tree-ssa-loop-ivopts.c
+++ b/gcc/tree-ssa-loop-ivopts.c
@@ -126,21 +126,6 @@ avg_loop_niter (struct loop *loop)
return niter;
}
-/* Representation of the induction variable. */
-struct iv
-{
- tree base; /* Initial value of the iv. */
- tree base_object; /* A memory object to that the induction variable points. */
- tree step; /* Step of the iv (constant only). */
- tree ssa_name; /* The ssa name with the value. */
- unsigned use_id; /* The identifier in the use if it is the case. */
- bool biv_p; /* Is it a biv? */
- bool have_use_for; /* Do we already have a use for it? */
- bool no_overflow; /* True if the iv doesn't overflow. */
- bool have_address_use;/* For biv, indicate if it's used in any address
- type use. */
-};
-
/* Per-ssa version information (induction variable descriptions, etc.). */
struct version_info
{
@@ -212,41 +197,6 @@ struct iv_use
/* Const offset stripped from base address. */
};
-/* The position where the iv is computed. */
-enum iv_position
-{
- IP_NORMAL, /* At the end, just before the exit condition. */
- IP_END, /* At the end of the latch block. */
- IP_BEFORE_USE, /* Immediately before a specific use. */
- IP_AFTER_USE, /* Immediately after a specific use. */
- IP_ORIGINAL /* The original biv. */
-};
-
-/* The induction variable candidate. */
-struct iv_cand
-{
- unsigned id; /* The number of the candidate. */
- bool important; /* Whether this is an "important" candidate, i.e. such
- that it should be considered by all uses. */
- ENUM_BITFIELD(iv_position) pos : 8; /* Where it is computed. */
- gimple *incremented_at;/* For original biv, the statement where it is
- incremented. */
- tree var_before; /* The variable used for it before increment. */
- tree var_after; /* The variable used for it after increment. */
- struct iv *iv; /* The value of the candidate. NULL for
- "pseudocandidate" used to indicate the possibility
- to replace the final value of an iv by direct
- computation of the value. */
- unsigned cost; /* Cost of the candidate. */
- unsigned cost_step; /* Cost of the candidate's increment operation. */
- struct iv_use *ainc_use; /* For IP_{BEFORE,AFTER}_USE candidates, the place
- where it is incremented. */
- bitmap depends_on; /* The list of invariants that are used in step of the
- biv. */
- struct iv *orig_iv; /* The original iv if this cand is added from biv with
- smaller type. */
-};
-
/* Hashtable entry for common candidate derived from iv uses. */
struct iv_common_cand
{
@@ -5834,6 +5784,8 @@ determine_iv_cost (struct ivopts_data *data, struct iv_cand *cand)
cand->cost = cost;
cand->cost_step = cost_step;
+
+ targetm.adjust_iv_cand_cost (cand);
}
/* Determines costs of computation of the candidates. */
diff --git a/gcc/tree-ssa-loop-ivopts.h b/gcc/tree-ssa-loop-ivopts.h
index 4495504..a3bc69e 100644
--- a/gcc/tree-ssa-loop-ivopts.h
+++ b/gcc/tree-ssa-loop-ivopts.h
@@ -20,6 +20,57 @@ along with GCC; see the file COPYING3. If not see
#ifndef GCC_TREE_SSA_LOOP_IVOPTS_H
#define GCC_TREE_SSA_LOOP_IVOPTS_H
+
+/* The position where the iv is computed. */
+enum iv_position
+{
+ IP_NORMAL, /* At the end, just before the exit condition. */
+ IP_END, /* At the end of the latch block. */
+ IP_BEFORE_USE, /* Immediately before a specific use. */
+ IP_AFTER_USE, /* Immediately after a specific use. */
+ IP_ORIGINAL /* The original biv. */
+};
+
+/* Representation of the induction variable. */
+struct iv
+{
+ tree base; /* Initial value of the iv. */
+ tree base_object; /* A memory object to that the induction variable points. */
+ tree step; /* Step of the iv (constant only). */
+ tree ssa_name; /* The ssa name with the value. */
+ unsigned use_id; /* The identifier in the use if it is the case. */
+ bool biv_p; /* Is it a biv? */
+ bool have_use_for; /* Do we already have a use for it? */
+ bool no_overflow; /* True if the iv doesn't overflow. */
+ bool have_address_use;/* For biv, indicate if it's used in any address
+ type use. */
+};
+
+/* The induction variable candidate. */
+struct iv_cand
+{
+ unsigned id; /* The number of the candidate. */
+ bool important; /* Whether this is an "important" candidate, i.e. such
+ that it should be considered by all uses. */
+ ENUM_BITFIELD(iv_position) pos : 8; /* Where it is computed. */
+ gimple *incremented_at;/* For original biv, the statement where it is
+ incremented. */
+ tree var_before; /* The variable used for it before increment. */
+ tree var_after; /* The variable used for it after increment. */
+ struct iv *iv; /* The value of the candidate. NULL for
+ "pseudocandidate" used to indicate the possibility
+ to replace the final value of an iv by direct
+ computation of the value. */
+ unsigned cost; /* Cost of the candidate. */
+ unsigned cost_step; /* Cost of the candidate's increment operation. */
+ struct iv_use *ainc_use; /* For IP_{BEFORE,AFTER}_USE candidates, the place
+ where it is incremented. */
+ bitmap depends_on; /* The list of invariants that are used in step of the
+ biv. */
+ struct iv *orig_iv; /* The original iv if this cand is added from biv with
+ smaller type. */
+};
+
extern edge single_dom_exit (struct loop *);
extern void dump_iv (FILE *, struct iv *);
extern void dump_use (FILE *, struct iv_use *);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] Request for comments on ivopts patch
2015-12-14 23:06 ` Steve Ellcey
@ 2015-12-16 13:43 ` Richard Biener
0 siblings, 0 replies; 6+ messages in thread
From: Richard Biener @ 2015-12-16 13:43 UTC (permalink / raw)
To: sellcey; +Cc: GCC Patches, amker cheng
On Tue, Dec 15, 2015 at 12:06 AM, Steve Ellcey <sellcey@imgtec.com> wrote:
> On Mon, 2015-12-14 at 09:57 +0100, Richard Biener wrote:
>
>> I don't know enough to assess the effect of this but
>>
>> 1) not all archs can do auto-incdec so either the comment is misleading
>> or the test should probably be amended
>> 2) I wonder why with the comment ("during the loop") you exclude IP_NORMAL/END
>>
>> that said, the comment needs to explain the situation better.
>>
>> Of course all such patches need some code-gen effect investigation
>> on more than one arch.
>>
>> [I wonder if a IV cost adjust target hook makes sense at some point]
>>
>> Richard.
>
> I like the idea of a target hook to modify IV costs. What do you think
> about this? I had to move some structures from tree-ssa-loop-ivopts.c
> to tree-ssa-loop-ivopts.h in order to give a target hooks access to
> information on the IV candidates.
IMHO it's better to not have empty default implementations but do
if (targetm.adjust_iv_cand_cost)
targetm.adjust_iv_cand_cost (...);
which saves an unconditional indirect call on most targets.
Generally I think we should prefer target independent heuristics if possible
or heuristics derived from target cost queries. So this kind of hook
says we've given up (and it makes it too easy to change things just for
a single target).
Anyway, this is for next stage1 of course so there's some time to come up
with good ideas and do testing on more than one target.
Richard.
> Steve Ellcey
> sellcey@imgtec.com
>
>
> 2015-12-14 Steve Ellcey <sellcey@imgtec.com>
>
> * doc/tm.texi.in (TARGET_ADJUST_IV_CAND_COST): New target function.
> * target.def (adjust_iv_cand_cost): New target function.
> * target.h (struct iv_cand): New forward declaration.
> * targhooks.c (default_adjust_iv_cand_cost): New default function.
> * targhooks.h (default_adjust_iv_cand_cost): Ditto.
> * tree-ssa-loop-ivopts.c (struct iv, enum iv_position, struct iv_cand)
> Moved to tree-ssa-loop-ivopts.h.
> (determine_iv_cost): Add call to targetm.adjust_iv_cand_cost.
> * tree-ssa-loop-ivopts.h (struct iv, enum iv_position, struct iv_cand)
> Copied here from tree-ssa-loop-ivopts.h.
>
> diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
> index a0a0a81..1ad4c2d 100644
> --- a/gcc/doc/tm.texi.in
> +++ b/gcc/doc/tm.texi.in
> @@ -8221,6 +8221,8 @@ and the associated definitions of those functions.
>
> @hook TARGET_OFFLOAD_OPTIONS
>
> +@hook TARGET_ADJUST_IV_CAND_COST
> +
> @defmac TARGET_SUPPORTS_WIDE_INT
>
> On older ports, large integers are stored in @code{CONST_DOUBLE} rtl
> diff --git a/gcc/target.def b/gcc/target.def
> index d754337..6bdcfcc 100644
> --- a/gcc/target.def
> +++ b/gcc/target.def
> @@ -5846,6 +5846,12 @@ DEFHOOK
> void, (tree *hold, tree *clear, tree *update),
> default_atomic_assign_expand_fenv)
>
> +DEFHOOK
> +(adjust_iv_cand_cost,
> +"Allow target to modify the cost of a possible induction variable.",
> +void, (struct iv_cand *cand),
> + default_adjust_iv_cand_cost)
> +
> /* Leave the boolean fields at the end. */
>
> /* True if we can create zeroed data by switching to a BSS section
> diff --git a/gcc/target.h b/gcc/target.h
> index ffc4d6a..6f55575 100644
> --- a/gcc/target.h
> +++ b/gcc/target.h
> @@ -139,6 +139,9 @@ struct ao_ref;
> /* This is defined in tree-vectorizer.h. */
> struct _stmt_vec_info;
>
> +/* This is defined in tree-ivopts.h. */
> +struct iv_cand;
> +
> /* These are defined in tree-vect-stmts.c. */
> extern tree stmt_vectype (struct _stmt_vec_info *);
> extern bool stmt_in_inner_loop_p (struct _stmt_vec_info *);
> diff --git a/gcc/targhooks.c b/gcc/targhooks.c
> index dcf0863..0d0bbfc 100644
> --- a/gcc/targhooks.c
> +++ b/gcc/targhooks.c
> @@ -1961,4 +1961,10 @@ default_optab_supported_p (int, machine_mode, machine_mode, optimization_type)
> return true;
> }
>
> +/* Default implementation of TARGET_ADJUST_IV_CAND_COST. */
> +void
> +default_adjust_iv_cand_cost (struct iv_cand *)
> +{
> +}
> +
> #include "gt-targhooks.h"
> diff --git a/gcc/targhooks.h b/gcc/targhooks.h
> index 47b5cfc..dd0481d 100644
> --- a/gcc/targhooks.h
> +++ b/gcc/targhooks.h
> @@ -253,4 +253,6 @@ extern void default_setup_incoming_vararg_bounds (cumulative_args_t ca ATTRIBUTE
> extern bool default_optab_supported_p (int, machine_mode, machine_mode,
> optimization_type);
>
> +extern void default_adjust_iv_cand_cost (struct iv_cand *);
> +
> #endif /* GCC_TARGHOOKS_H */
> diff --git a/gcc/tree-ssa-loop-ivopts.c b/gcc/tree-ssa-loop-ivopts.c
> index 98dc451..5bfd232 100644
> --- a/gcc/tree-ssa-loop-ivopts.c
> +++ b/gcc/tree-ssa-loop-ivopts.c
> @@ -126,21 +126,6 @@ avg_loop_niter (struct loop *loop)
> return niter;
> }
>
> -/* Representation of the induction variable. */
> -struct iv
> -{
> - tree base; /* Initial value of the iv. */
> - tree base_object; /* A memory object to that the induction variable points. */
> - tree step; /* Step of the iv (constant only). */
> - tree ssa_name; /* The ssa name with the value. */
> - unsigned use_id; /* The identifier in the use if it is the case. */
> - bool biv_p; /* Is it a biv? */
> - bool have_use_for; /* Do we already have a use for it? */
> - bool no_overflow; /* True if the iv doesn't overflow. */
> - bool have_address_use;/* For biv, indicate if it's used in any address
> - type use. */
> -};
> -
> /* Per-ssa version information (induction variable descriptions, etc.). */
> struct version_info
> {
> @@ -212,41 +197,6 @@ struct iv_use
> /* Const offset stripped from base address. */
> };
>
> -/* The position where the iv is computed. */
> -enum iv_position
> -{
> - IP_NORMAL, /* At the end, just before the exit condition. */
> - IP_END, /* At the end of the latch block. */
> - IP_BEFORE_USE, /* Immediately before a specific use. */
> - IP_AFTER_USE, /* Immediately after a specific use. */
> - IP_ORIGINAL /* The original biv. */
> -};
> -
> -/* The induction variable candidate. */
> -struct iv_cand
> -{
> - unsigned id; /* The number of the candidate. */
> - bool important; /* Whether this is an "important" candidate, i.e. such
> - that it should be considered by all uses. */
> - ENUM_BITFIELD(iv_position) pos : 8; /* Where it is computed. */
> - gimple *incremented_at;/* For original biv, the statement where it is
> - incremented. */
> - tree var_before; /* The variable used for it before increment. */
> - tree var_after; /* The variable used for it after increment. */
> - struct iv *iv; /* The value of the candidate. NULL for
> - "pseudocandidate" used to indicate the possibility
> - to replace the final value of an iv by direct
> - computation of the value. */
> - unsigned cost; /* Cost of the candidate. */
> - unsigned cost_step; /* Cost of the candidate's increment operation. */
> - struct iv_use *ainc_use; /* For IP_{BEFORE,AFTER}_USE candidates, the place
> - where it is incremented. */
> - bitmap depends_on; /* The list of invariants that are used in step of the
> - biv. */
> - struct iv *orig_iv; /* The original iv if this cand is added from biv with
> - smaller type. */
> -};
> -
> /* Hashtable entry for common candidate derived from iv uses. */
> struct iv_common_cand
> {
> @@ -5834,6 +5784,8 @@ determine_iv_cost (struct ivopts_data *data, struct iv_cand *cand)
>
> cand->cost = cost;
> cand->cost_step = cost_step;
> +
> + targetm.adjust_iv_cand_cost (cand);
> }
>
> /* Determines costs of computation of the candidates. */
> diff --git a/gcc/tree-ssa-loop-ivopts.h b/gcc/tree-ssa-loop-ivopts.h
> index 4495504..a3bc69e 100644
> --- a/gcc/tree-ssa-loop-ivopts.h
> +++ b/gcc/tree-ssa-loop-ivopts.h
> @@ -20,6 +20,57 @@ along with GCC; see the file COPYING3. If not see
> #ifndef GCC_TREE_SSA_LOOP_IVOPTS_H
> #define GCC_TREE_SSA_LOOP_IVOPTS_H
>
> +
> +/* The position where the iv is computed. */
> +enum iv_position
> +{
> + IP_NORMAL, /* At the end, just before the exit condition. */
> + IP_END, /* At the end of the latch block. */
> + IP_BEFORE_USE, /* Immediately before a specific use. */
> + IP_AFTER_USE, /* Immediately after a specific use. */
> + IP_ORIGINAL /* The original biv. */
> +};
> +
> +/* Representation of the induction variable. */
> +struct iv
> +{
> + tree base; /* Initial value of the iv. */
> + tree base_object; /* A memory object to that the induction variable points. */
> + tree step; /* Step of the iv (constant only). */
> + tree ssa_name; /* The ssa name with the value. */
> + unsigned use_id; /* The identifier in the use if it is the case. */
> + bool biv_p; /* Is it a biv? */
> + bool have_use_for; /* Do we already have a use for it? */
> + bool no_overflow; /* True if the iv doesn't overflow. */
> + bool have_address_use;/* For biv, indicate if it's used in any address
> + type use. */
> +};
> +
> +/* The induction variable candidate. */
> +struct iv_cand
> +{
> + unsigned id; /* The number of the candidate. */
> + bool important; /* Whether this is an "important" candidate, i.e. such
> + that it should be considered by all uses. */
> + ENUM_BITFIELD(iv_position) pos : 8; /* Where it is computed. */
> + gimple *incremented_at;/* For original biv, the statement where it is
> + incremented. */
> + tree var_before; /* The variable used for it before increment. */
> + tree var_after; /* The variable used for it after increment. */
> + struct iv *iv; /* The value of the candidate. NULL for
> + "pseudocandidate" used to indicate the possibility
> + to replace the final value of an iv by direct
> + computation of the value. */
> + unsigned cost; /* Cost of the candidate. */
> + unsigned cost_step; /* Cost of the candidate's increment operation. */
> + struct iv_use *ainc_use; /* For IP_{BEFORE,AFTER}_USE candidates, the place
> + where it is incremented. */
> + bitmap depends_on; /* The list of invariants that are used in step of the
> + biv. */
> + struct iv *orig_iv; /* The original iv if this cand is added from biv with
> + smaller type. */
> +};
> +
> extern edge single_dom_exit (struct loop *);
> extern void dump_iv (FILE *, struct iv *);
> extern void dump_use (FILE *, struct iv_use *);
>
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-12-16 13:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-08 18:56 [RFC] Request for comments on ivopts patch Steve Ellcey
2015-12-09 10:24 ` Richard Biener
2015-12-11 23:48 ` Steve Ellcey
2015-12-14 8:57 ` Richard Biener
2015-12-14 23:06 ` Steve Ellcey
2015-12-16 13:43 ` Richard Biener
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).