diff mbox

[1/2] Move mult synthesis definitions into a separate file

Message ID 575EA5B6.3040707@foss.arm.com
State New
Headers show

Commit Message

Kyrill Tkachov June 13, 2016, 12:23 p.m. UTC
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  <kyrylo.tkachov@arm.com>

     * 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.

Comments

Richard Biener June 13, 2016, 2:07 p.m. UTC | #1
On Mon, Jun 13, 2016 at 2:23 PM, Kyrill Tkachov
<kyrylo.tkachov@foss.arm.com> 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  <kyrylo.tkachov@arm.com>
>
>     * 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.
Kyrill Tkachov June 13, 2016, 2:17 p.m. UTC | #2
Hi Richard,

On 13/06/16 15:07, Richard Biener wrote:
> On Mon, Jun 13, 2016 at 2:23 PM, Kyrill Tkachov
> <kyrylo.tkachov@foss.arm.com> 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  <kyrylo.tkachov@arm.com>
>>
>>      * 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.
Richard Biener June 14, 2016, 7:19 a.m. UTC | #3
On Mon, Jun 13, 2016 at 4:17 PM, Kyrill Tkachov
<kyrylo.tkachov@foss.arm.com> wrote:
> Hi Richard,
>
> On 13/06/16 15:07, Richard Biener wrote:
>>
>> On Mon, Jun 13, 2016 at 2:23 PM, Kyrill Tkachov
>> <kyrylo.tkachov@foss.arm.com> 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  <kyrylo.tkachov@arm.com>
>>>
>>>      * 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 mbox

Patch

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 algorithm *, enum mult_variant *, int);
 static rtx expand_mult_const (machine_mode, rtx, HOST_WIDE_INT, rtx,
 			      const struct algorithm *, enum mult_variant);
 static unsigned HOST_WIDE_INT invert_mod2n (unsigned HOST_WIDE_INT, int);
@@ -2981,7 +2974,7 @@  synth_mult (struct algorithm *alg_out, unsigned HOST_WIDE_INT t,
    Return true if the cheapest of these cost less than MULT_COST,
    describing the algorithm in *ALG and final fixup in *VARIANT.  */
 
-static bool
+bool
 choose_mult_variant (machine_mode mode, HOST_WIDE_INT val,
 		     struct algorithm *alg, enum mult_variant *variant,
 		     int mult_cost)
diff --git a/gcc/mult-synthesis.h b/gcc/mult-synthesis.h
new file mode 100644
index 0000000000000000000000000000000000000000..cd0e46176805dabeac0c71c5a16f9bfb8ee1356e
--- /dev/null
+++ b/gcc/mult-synthesis.h
@@ -0,0 +1,94 @@ 
+/* Multiplication synthesis utilities.
+   Copyright (C) 1987-2016 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+#ifndef GCC_MULT_SYNTHESIS_H
+#define GCC_MULT_SYNTHESIS_H 1
+
+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
+};
+
+/* 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};
+
+/* 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.  */
+};
+
+/* 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];
+};
+
+/* Defined in expmed.c.  */
+bool choose_mult_variant (machine_mode, HOST_WIDE_INT, struct algorithm *,
+			  mult_variant *, int);
+
+#endif /* ifdef GCC_MULT_SYNTHESIS_H.  */