[PATCH][1/2] Move mult synthesis definitions into a separate file
Hi all,
There are other places besides expand where we might want to synthesize an
integer
multiplication by a constant. Thankfully the algorithm selection code in
expmed.c
is already quite well separated from the RTL implementation, so if we can just
factor
out the prototype of choose_mult_variant and some enums and structs that it
needs into
a separate header file we can reuse them from other parts of the compiler.
I need this for patch 2/2 which hooks up the vectorizer to synthesize vector
multiplications using sequences of shifts and other arithmetic ops when
appropriate.
The new header is called mult-synthesis.h. Should I add it to some makefile?
grepping around for a bit I'm not sure what to do about it.
Bootstrapped and tested on arm, aarch64, x86_64.
Thanks,
Kyrill
2016-06-13 Kyrylo Tkachov
* mult-synthesis.h: New file. Add choose_mult_variant prototype.
* expmed.h: Include mult-synthesis.h
(enum alg_code): Move to mult-synthesis.h
(struct mult_cost): Likewise.
(struct algorithm): Likewise.
* expmed.c (enum mult_variant): Move to mult-synthesis.h
(choose_mult_variant): Delete prototype. Remove static qualifier.
diff --git a/gcc/expmed.h b/gcc/expmed.h
index 1a32e9f1b664f250c5092022eb965237ed0342fc..304ce02d78a9e3e024c13caee7869d67dfdab65c 100644
--- a/gcc/expmed.h
+++ b/gcc/expmed.h
@@ -21,35 +21,7 @@ along with GCC; see the file COPYING3. If not see
#define EXPMED_H 1
#include "insn-codes.h"
-
-enum alg_code {
- alg_unknown,
- alg_zero,
- alg_m, alg_shift,
- alg_add_t_m2,
- alg_sub_t_m2,
- alg_add_factor,
- alg_sub_factor,
- alg_add_t2_m,
- alg_sub_t2_m,
- alg_impossible
-};
-
-/* This structure holds the "cost" of a multiply sequence. The
- "cost" field holds the total rtx_cost of every operator in the
- synthetic multiplication sequence, hence cost(a op b) is defined
- as rtx_cost(op) + cost(a) + cost(b), where cost(leaf) is zero.
- The "latency" field holds the minimum possible latency of the
- synthetic multiply, on a hypothetical infinitely parallel CPU.
- This is the critical path, or the maximum height, of the expression
- tree which is the sum of rtx_costs on the most expensive path from
- any leaf to the root. Hence latency(a op b) is defined as zero for
- leaves and rtx_cost(op) + max(latency(a), latency(b)) otherwise. */
-
-struct mult_cost {
- short cost; /* Total rtx_cost of the multiplication sequence. */
- short latency; /* The latency of the multiplication sequence. */
-};
+#include "mult-synthesis.h"
/* This macro is used to compare a pointer to a mult_cost against an
single integer "rtx_cost" value. This is equivalent to the macro
@@ -65,38 +37,6 @@ struct mult_cost {
|| ((X)->cost == (Y)->cost \
&& (X)->latency < (Y)->latency))
-/* This structure records a sequence of operations.
- `ops' is the number of operations recorded.
- `cost' is their total cost.
- The operations are stored in `op' and the corresponding
- logarithms of the integer coefficients in `log'.
-
- These are the operations:
- alg_zero total := 0;
- alg_m total := multiplicand;
- alg_shift total := total * coeff
- alg_add_t_m2 total := total + multiplicand * coeff;
- alg_sub_t_m2 total := total - multiplicand * coeff;
- alg_add_factor total := total * coeff + total;
- alg_sub_factor total := total * coeff - total;
- alg_add_t2_m total := total * coeff + multiplicand;
- alg_sub_t2_m total := total * coeff - multiplicand;
-
- The first operand must be either alg_zero or alg_m. */
-
-struct algorithm
-{
- struct mult_cost cost;
- short ops;
- /* The size of the OP and LOG fields are not directly related to the
- word size, but the worst-case algorithms will be if we have few
- consecutive ones or zeros, i.e., a multiplicand like 10101010101...
- In that case we will generate shift-by-2, add, shift-by-2, add,...,
- in total wordsize operations. */
- enum alg_code op[MAX_BITS_PER_WORD];
- char log[MAX_BITS_PER_WORD];
-};
-
/* The entry for our multiplication cache/hash table. */
struct alg_hash_entry {
/* The number we are multiplying by. */
diff --git a/gcc/expmed.c b/gcc/expmed.c
index 6645a535b3eef9624e6f3ce61d2fcf864d1cf574..22564fa423aec52febef6220d3f59a82e09b118a 100644
--- a/gcc/expmed.c
+++ b/gcc/expmed.c
@@ -2482,16 +2482,9 @@ expand_variable_shift (enum tree_code code, machine_mode mode, rtx shifted,
}
-/* Indicates the type of fixup needed after a constant multiplication.
- BASIC_VARIANT means no fixup is needed, NEGATE_VARIANT means that
- the result should be negated, and ADD_VARIANT means that the
- multiplicand should be added to the result. */
-enum mult_variant {basic_variant, negate_variant, add_variant};
static void synth_mult (struct algorithm *, unsigned HOST_WIDE_INT,
const struct mult_cost *, machine_mode mode);
-static bool choose_mult_variant (machine_mode, HOST_WIDE_INT,
- struct algorit
Re: [PATCH][1/2] Move mult synthesis definitions into a separate file
On Mon, Jun 13, 2016 at 2:23 PM, Kyrill Tkachov wrote: > Hi all, > > There are other places besides expand where we might want to synthesize an > integer > multiplication by a constant. Thankfully the algorithm selection code in > expmed.c > is already quite well separated from the RTL implementation, so if we can > just factor > out the prototype of choose_mult_variant and some enums and structs that it > needs into > a separate header file we can reuse them from other parts of the compiler. > > I need this for patch 2/2 which hooks up the vectorizer to synthesize vector > multiplications using sequences of shifts and other arithmetic ops when > appropriate. > > The new header is called mult-synthesis.h. Should I add it to some makefile? > grepping around for a bit I'm not sure what to do about it. Possibly PLUGIN_HEADERS. You could have included expmed.h from the vectorizer, no? After all this patch now breaks that things declared in A.h are defined in A.c as you didn't move choose_mult_variant itself. Thanks, Richard. > > Bootstrapped and tested on arm, aarch64, x86_64. > > Thanks, > Kyrill > > 2016-06-13 Kyrylo Tkachov > > * mult-synthesis.h: New file. Add choose_mult_variant prototype. > * expmed.h: Include mult-synthesis.h > (enum alg_code): Move to mult-synthesis.h > (struct mult_cost): Likewise. > (struct algorithm): Likewise. > * expmed.c (enum mult_variant): Move to mult-synthesis.h > (choose_mult_variant): Delete prototype. Remove static qualifier.
Re: [PATCH][1/2] Move mult synthesis definitions into a separate file
Hi Richard, On 13/06/16 15:07, Richard Biener wrote: On Mon, Jun 13, 2016 at 2:23 PM, Kyrill Tkachov wrote: Hi all, There are other places besides expand where we might want to synthesize an integer multiplication by a constant. Thankfully the algorithm selection code in expmed.c is already quite well separated from the RTL implementation, so if we can just factor out the prototype of choose_mult_variant and some enums and structs that it needs into a separate header file we can reuse them from other parts of the compiler. I need this for patch 2/2 which hooks up the vectorizer to synthesize vector multiplications using sequences of shifts and other arithmetic ops when appropriate. The new header is called mult-synthesis.h. Should I add it to some makefile? grepping around for a bit I'm not sure what to do about it. Possibly PLUGIN_HEADERS. Ok. You could have included expmed.h from the vectorizer, no? After all this patch now breaks that things declared in A.h are defined in A.c as you didn't move choose_mult_variant itself. I think including expmed.h would work. I thought it defined too many irrelevant RTL-specific things that you wouldn't want in the vectoriser. If you don't mind I'm happy to just include expmed.h. Do we have a rule for defining things delcared in A.h in A.c? I notice we declare various extern things in rtl.h that aren't defined in rtl.c, though I suppose that would be an exception... Thanks, Kyrill Thanks, Richard. Bootstrapped and tested on arm, aarch64, x86_64. Thanks, Kyrill 2016-06-13 Kyrylo Tkachov * mult-synthesis.h: New file. Add choose_mult_variant prototype. * expmed.h: Include mult-synthesis.h (enum alg_code): Move to mult-synthesis.h (struct mult_cost): Likewise. (struct algorithm): Likewise. * expmed.c (enum mult_variant): Move to mult-synthesis.h (choose_mult_variant): Delete prototype. Remove static qualifier.
Re: [PATCH][1/2] Move mult synthesis definitions into a separate file
On Mon, Jun 13, 2016 at 4:17 PM, Kyrill Tkachov wrote: > Hi Richard, > > On 13/06/16 15:07, Richard Biener wrote: >> >> On Mon, Jun 13, 2016 at 2:23 PM, Kyrill Tkachov >> wrote: >>> >>> Hi all, >>> >>> There are other places besides expand where we might want to synthesize >>> an >>> integer >>> multiplication by a constant. Thankfully the algorithm selection code in >>> expmed.c >>> is already quite well separated from the RTL implementation, so if we can >>> just factor >>> out the prototype of choose_mult_variant and some enums and structs that >>> it >>> needs into >>> a separate header file we can reuse them from other parts of the >>> compiler. >>> >>> I need this for patch 2/2 which hooks up the vectorizer to synthesize >>> vector >>> multiplications using sequences of shifts and other arithmetic ops when >>> appropriate. >>> >>> The new header is called mult-synthesis.h. Should I add it to some >>> makefile? >>> grepping around for a bit I'm not sure what to do about it. >> >> Possibly PLUGIN_HEADERS. > > > Ok. > >> You could have included expmed.h from the vectorizer, no? After all this >> patch now breaks that things declared in A.h are defined in A.c as you >> didn't move choose_mult_variant itself. > > > I think including expmed.h would work. I thought it defined too many > irrelevant RTL-specific things that you wouldn't want in the vectoriser. > If you don't mind I'm happy to just include expmed.h. Yes, please do so. > Do we have a rule for defining things delcared in A.h in A.c? We worked hard toward that with the header file cleanup and I think it is a natural thing to do. > I notice we declare various extern things in rtl.h that aren't defined in > rtl.c, though I suppose that would be an exception... Similar for tree.h - but those are really just not fully cleaned up yet. Richard. > Thanks, > Kyrill > > >> Thanks, >> Richard. >> >>> Bootstrapped and tested on arm, aarch64, x86_64. >>> >>> Thanks, >>> Kyrill >>> >>> 2016-06-13 Kyrylo Tkachov >>> >>> * mult-synthesis.h: New file. Add choose_mult_variant prototype. >>> * expmed.h: Include mult-synthesis.h >>> (enum alg_code): Move to mult-synthesis.h >>> (struct mult_cost): Likewise. >>> (struct algorithm): Likewise. >>> * expmed.c (enum mult_variant): Move to mult-synthesis.h >>> (choose_mult_variant): Delete prototype. Remove static qualifier. > >
