* [PATCH] Introduce MODE_SIZE mode attribute
@ 2014-01-03 8:48 Jakub Jelinek
2014-01-03 9:23 ` Uros Bizjak
2014-01-03 15:39 ` Joseph S. Myers
0 siblings, 2 replies; 8+ messages in thread
From: Jakub Jelinek @ 2014-01-03 8:48 UTC (permalink / raw)
To: Uros Bizjak, Richard Henderson, Kirill Yukhin; +Cc: gcc-patches
Hi!
I've noticed that especially with the AVX512F introduction we use
GET_MODE_SIZE (<MODE>mode) quite heavily in the i386 *.md files, and
the problem with that is GET_MODE_SIZE isn't a compile time constant,
needs to read mode_size array (non-const) at runtime.
This patch attempts to decrease the enormous .text/.rodata growth from AVX512F
introduction a little bit and improve generated code, by making
GET_MODE_SIZE (<MODE>mode) compile time constants, thus all the
GET_MODE_SIZE (<MODE>mode) == 64 and similar tests can fold into
false/true. The patch saves ~ 110KB of .text (both x86_64 and i686)
and ~ 50KB of .rodata (x86_64) or ~ 27KB of .rodata (i686).
The disadvantage is that the mode attribute duplicates insn-modes.c,
but I guess the listed modes aren't going to change their sizes ever
on this target, and if some mode isn't listed and used in some mode
iterator, one would get syntax errors from insn-*.[ch] and so it wouldn't be
hard to add it.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2014-01-03 Jakub Jelinek <jakub@redhat.com>
* config/i386/i386.md (MODE_SIZE): New mode attribute.
(push splitter): Use <P:MODE_SIZE> instead of
GET_MODE_SIZE (<P:MODE>mode).
(lea splitter): Use <MODE_SIZE> instead of GET_MODE_SIZE (<MODE>mode).
(mov -1, reg peephole2): Likewise.
* config/i386/sse.md (*mov<mode>_internal,
<sse>_storeu<ssemodesuffix><avxsizesuffix>,
<sse2_avx_avx512f>_storedqu<mode>, <sse>_andnot<mode>3,
*<code><mode>3, *andnot<mode>3<mask_name>,
<mask_codefor><code><mode>3<mask_name>): Likewise.
* config/i386/subst.md (mask_mode512bit_condition,
sd_mask_mode512bit_condition): Likewise.
--- gcc/config/i386/i386.md.jj 2013-12-27 19:24:33.000000000 +0100
+++ gcc/config/i386/i386.md 2014-01-02 20:08:16.911851795 +0100
@@ -914,6 +914,20 @@ (define_mode_iterator SWIM248 [(HI "TARG
(define_mode_iterator DWI [(DI "!TARGET_64BIT")
(TI "TARGET_64BIT")])
+;; GET_MODE_SIZE for selected modes. As GET_MODE_SIZE is not
+;; compile time constant, it is faster to use <MODE_SIZE> than
+;; GET_MODE_SIZE (<MODE>mode). For XFmode which depends on
+;; command line options just use GET_MODE_SIZE macro.
+(define_mode_attr MODE_SIZE [(QI "1") (HI "2") (SI "4") (DI "8") (TI "16")
+ (SF "4") (DF "8") (XF "GET_MODE_SIZE (XFmode)")
+ (V16QI "16") (V32QI "32") (V64QI "64")
+ (V8HI "16") (V16HI "32") (V32HI "64")
+ (V4SI "16") (V8SI "32") (V16SI "64")
+ (V2DI "16") (V4DI "32") (V8DI "64")
+ (V1TI "16") (V2TI "32") (V4TI "64")
+ (V2DF "16") (V4DF "32") (V8DF "64")
+ (V4SF "16") (V8SF "32") (V16SF "64")])
+
;; Double word integer modes as mode attribute.
(define_mode_attr DWI [(QI "HI") (HI "SI") (SI "DI") (DI "TI")])
(define_mode_attr dwi [(QI "hi") (HI "si") (SI "di") (DI "ti")])
@@ -2734,7 +2748,7 @@ (define_split
"reload_completed"
[(set (reg:P SP_REG) (plus:P (reg:P SP_REG) (match_dup 2)))
(set (mem:SF (reg:P SP_REG)) (match_dup 1))]
- "operands[2] = GEN_INT (-GET_MODE_SIZE (<P:MODE>mode));")
+ "operands[2] = GEN_INT (-<P:MODE_SIZE>);")
(define_split
[(set (match_operand:SF 0 "push_operand")
@@ -5770,7 +5784,7 @@ (define_split
enum machine_mode mode = <MODE>mode;
rtx pat;
- if (GET_MODE_SIZE (mode) < GET_MODE_SIZE (SImode))
+ if (<MODE_SIZE> < GET_MODE_SIZE (SImode))
{
mode = SImode;
operands[0] = gen_lowpart (mode, operands[0]);
@@ -17403,7 +17417,7 @@ (define_peephole2
[(parallel [(set (match_dup 0) (const_int -1))
(clobber (reg:CC FLAGS_REG))])]
{
- if (GET_MODE_SIZE (<MODE>mode) < GET_MODE_SIZE (SImode))
+ if (<MODE_SIZE> < GET_MODE_SIZE (SImode))
operands[0] = gen_lowpart (SImode, operands[0]);
})
--- gcc/config/i386/sse.md.jj 2014-01-01 00:52:56.000000000 +0100
+++ gcc/config/i386/sse.md 2014-01-02 20:11:49.723916127 +0100
@@ -669,7 +669,7 @@ (define_insn "*mov<mode>_internal"
/* There is no evex-encoded vmov* for sizes smaller than 64-bytes
in avx512f, so we need to use workarounds, to access sse registers
16-31, which are evex-only. */
- if (TARGET_AVX512F && GET_MODE_SIZE (<MODE>mode) < 64
+ if (TARGET_AVX512F && <MODE_SIZE> < 64
&& ((REG_P (operands[0])
&& EXT_REX_SSE_REGNO_P (REGNO (operands[0])))
|| (REG_P (operands[1])
@@ -677,18 +677,18 @@ (define_insn "*mov<mode>_internal"
{
if (memory_operand (operands[0], <MODE>mode))
{
- if (GET_MODE_SIZE (<MODE>mode) == 32)
+ if (<MODE_SIZE> == 32)
return "vextract<shuffletype>64x4\t{$0x0, %g1, %0|%0, %g1, 0x0}";
- else if (GET_MODE_SIZE (<MODE>mode) == 16)
+ else if (<MODE_SIZE> == 16)
return "vextract<shuffletype>32x4\t{$0x0, %g1, %0|%0, %g1, 0x0}";
else
gcc_unreachable ();
}
else if (memory_operand (operands[1], <MODE>mode))
{
- if (GET_MODE_SIZE (<MODE>mode) == 32)
+ if (<MODE_SIZE> == 32)
return "vbroadcast<shuffletype>64x4\t{%1, %g0|%g0, %1}";
- else if (GET_MODE_SIZE (<MODE>mode) == 16)
+ else if (<MODE_SIZE> == 16)
return "vbroadcast<shuffletype>32x4\t{%1, %g0|%g0, %1}";
else
gcc_unreachable ();
@@ -759,7 +759,7 @@ (define_insn "*mov<mode>_internal"
(set (attr "mode")
(cond [(match_test "TARGET_SSE_PACKED_SINGLE_INSN_OPTIMAL")
(const_string "<ssePSmode>")
- (and (match_test "GET_MODE_SIZE (<MODE>mode) == 16")
+ (and (match_test "<MODE_SIZE> == 16")
(and (eq_attr "alternative" "2")
(match_test "TARGET_SSE_TYPELESS_STORES")))
(const_string "<ssePSmode>")
@@ -998,7 +998,7 @@ (define_insn "<sse>_storeu<ssemodesuffix
(set_attr "ssememalign" "8")
(set_attr "prefix" "maybe_vex")
(set (attr "mode")
- (cond [(and (match_test "GET_MODE_SIZE (<MODE>mode) == 16")
+ (cond [(and (match_test "<MODE_SIZE> == 16")
(ior (match_test "TARGET_SSE_PACKED_SINGLE_INSN_OPTIMAL")
(match_test "TARGET_SSE_TYPELESS_STORES")))
(const_string "<ssePSmode>")
@@ -1127,7 +1127,7 @@ (define_insn "<sse2_avx_avx512f>_storedq
(const_string "1")))
(set_attr "prefix" "maybe_vex")
(set (attr "mode")
- (cond [(and (match_test "GET_MODE_SIZE (<MODE>mode) == 16")
+ (cond [(and (match_test "<MODE_SIZE> == 16")
(ior (match_test "TARGET_SSE_PACKED_SINGLE_INSN_OPTIMAL")
(match_test "TARGET_SSE_TYPELESS_STORES")))
(const_string "<ssePSmode>")
@@ -2363,7 +2363,7 @@ (define_insn "<sse>_andnot<mode>3"
}
/* There is no vandnp[sd]. Use vpandnq. */
- if (GET_MODE_SIZE (<MODE>mode) == 64)
+ if (<MODE_SIZE> == 64)
{
suffix = "q";
ops = "vpandn%s\t{%%2, %%1, %%0|%%0, %%1, %%2}";
@@ -2435,7 +2435,7 @@ (define_insn "*<code><mode>3"
}
/* There is no v<logic>p[sd]. Use vp<logic>q. */
- if (GET_MODE_SIZE (<MODE>mode) == 64)
+ if (<MODE_SIZE> == 64)
{
suffix = "q";
ops = "vp<logic>%s\t{%%2, %%1, %%0|%%0, %%1, %%2}";
@@ -8940,7 +8940,7 @@ (define_insn "*andnot<mode>3<mask_name>"
(const_string "<sseinsnmode>")
(match_test "TARGET_AVX")
(if_then_else
- (match_test "GET_MODE_SIZE (<MODE>mode) > 16")
+ (match_test "<MODE_SIZE> > 16")
(const_string "V8SF")
(const_string "<sseinsnmode>"))
(ior (not (match_test "TARGET_SSE2"))
@@ -9032,7 +9032,7 @@ (define_insn "<mask_codefor><code><mode>
(const_string "<sseinsnmode>")
(match_test "TARGET_AVX")
(if_then_else
- (match_test "GET_MODE_SIZE (<MODE>mode) > 16")
+ (match_test "<MODE_SIZE> > 16")
(const_string "V8SF")
(const_string "<sseinsnmode>"))
(ior (not (match_test "TARGET_SSE2"))
--- gcc/config/i386/subst.md.jj 2014-01-01 00:45:15.000000000 +0100
+++ gcc/config/i386/subst.md 2014-01-02 20:12:17.285791572 +0100
@@ -51,7 +51,7 @@ (define_subst_attr "mask_operand11" "mas
(define_subst_attr "mask_operand18" "mask" "" "%{%19%}%N18")
(define_subst_attr "mask_operand19" "mask" "" "%{%20%}%N19")
(define_subst_attr "mask_codefor" "mask" "*" "")
-(define_subst_attr "mask_mode512bit_condition" "mask" "1" "(GET_MODE_SIZE (<MODE>mode) == 64)")
+(define_subst_attr "mask_mode512bit_condition" "mask" "1" "(<MODE_SIZE> == 64)")
(define_subst_attr "store_mask_constraint" "mask" "vm" "v")
(define_subst_attr "store_mask_predicate" "mask" "nonimmediate_operand" "register_operand")
(define_subst_attr "mask_prefix" "mask" "vex" "evex")
@@ -85,7 +85,7 @@ (define_subst_attr "sd_maskz_name" "sd"
(define_subst_attr "sd_mask_op4" "sd" "" "%{%5%}%N4")
(define_subst_attr "sd_mask_op5" "sd" "" "%{%6%}%N5")
(define_subst_attr "sd_mask_codefor" "sd" "*" "")
-(define_subst_attr "sd_mask_mode512bit_condition" "sd" "1" "(GET_MODE_SIZE (<MODE>mode) == 64)")
+(define_subst_attr "sd_mask_mode512bit_condition" "sd" "1" "(<MODE_SIZE> == 64)")
(define_subst "sd"
[(set (match_operand:SUBST_V 0)
Jakub
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Introduce MODE_SIZE mode attribute
2014-01-03 8:48 [PATCH] Introduce MODE_SIZE mode attribute Jakub Jelinek
@ 2014-01-03 9:23 ` Uros Bizjak
2014-01-03 15:39 ` Joseph S. Myers
1 sibling, 0 replies; 8+ messages in thread
From: Uros Bizjak @ 2014-01-03 9:23 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Richard Henderson, Kirill Yukhin, gcc-patches
On Fri, Jan 3, 2014 at 9:48 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> I've noticed that especially with the AVX512F introduction we use
> GET_MODE_SIZE (<MODE>mode) quite heavily in the i386 *.md files, and
> the problem with that is GET_MODE_SIZE isn't a compile time constant,
> needs to read mode_size array (non-const) at runtime.
>
> This patch attempts to decrease the enormous .text/.rodata growth from AVX512F
> introduction a little bit and improve generated code, by making
> GET_MODE_SIZE (<MODE>mode) compile time constants, thus all the
> GET_MODE_SIZE (<MODE>mode) == 64 and similar tests can fold into
> false/true. The patch saves ~ 110KB of .text (both x86_64 and i686)
> and ~ 50KB of .rodata (x86_64) or ~ 27KB of .rodata (i686).
>
> The disadvantage is that the mode attribute duplicates insn-modes.c,
> but I guess the listed modes aren't going to change their sizes ever
> on this target, and if some mode isn't listed and used in some mode
> iterator, one would get syntax errors from insn-*.[ch] and so it wouldn't be
> hard to add it.
This should be tolerable, at least for gains, shown above. OTOH, we
already have some attributes (e.g. ssescalarnum, ssescalarsize) that
use the same approach and the sky didn't fall down.
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2014-01-03 Jakub Jelinek <jakub@redhat.com>
>
> * config/i386/i386.md (MODE_SIZE): New mode attribute.
> (push splitter): Use <P:MODE_SIZE> instead of
> GET_MODE_SIZE (<P:MODE>mode).
> (lea splitter): Use <MODE_SIZE> instead of GET_MODE_SIZE (<MODE>mode).
> (mov -1, reg peephole2): Likewise.
> * config/i386/sse.md (*mov<mode>_internal,
> <sse>_storeu<ssemodesuffix><avxsizesuffix>,
> <sse2_avx_avx512f>_storedqu<mode>, <sse>_andnot<mode>3,
> *<code><mode>3, *andnot<mode>3<mask_name>,
> <mask_codefor><code><mode>3<mask_name>): Likewise.
> * config/i386/subst.md (mask_mode512bit_condition,
> sd_mask_mode512bit_condition): Likewise.
OK for mainline.
Thanks,
Uros.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Introduce MODE_SIZE mode attribute
2014-01-03 8:48 [PATCH] Introduce MODE_SIZE mode attribute Jakub Jelinek
2014-01-03 9:23 ` Uros Bizjak
@ 2014-01-03 15:39 ` Joseph S. Myers
2014-01-03 23:38 ` Jakub Jelinek
1 sibling, 1 reply; 8+ messages in thread
From: Joseph S. Myers @ 2014-01-03 15:39 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Uros Bizjak, Richard Henderson, Kirill Yukhin, gcc-patches
On Fri, 3 Jan 2014, Jakub Jelinek wrote:
> I've noticed that especially with the AVX512F introduction we use
> GET_MODE_SIZE (<MODE>mode) quite heavily in the i386 *.md files, and
> the problem with that is GET_MODE_SIZE isn't a compile time constant,
> needs to read mode_size array (non-const) at runtime.
It would seem better to me for genmodes to generate appropriate macro /
inline function definitions that can be used by GET_MODE_SIZE (along the
lines of (__builtin_constant_p (MODE) && !mode_size_variable (MODE) ?
get_mode_size_constant (MODE) : (unsigned short) mode_size[MODE]), where
mode_size_variable and get_mode_size_constant are always_inline functions
generated by genmodes) - that way all targets are covered automatically,
without needing such duplication of mode sizes. (Of course such
optimizations wouldn't apply for a first-stage compiler bootstrapped by a
compiler not supporting __builtin_constant_p, but lack of optimization in
the first stage of a bootstrap is not a particular concern.)
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Introduce MODE_SIZE mode attribute
2014-01-03 15:39 ` Joseph S. Myers
@ 2014-01-03 23:38 ` Jakub Jelinek
2014-01-04 2:27 ` Andrew Pinski
2014-01-06 15:21 ` Jakub Jelinek
0 siblings, 2 replies; 8+ messages in thread
From: Jakub Jelinek @ 2014-01-03 23:38 UTC (permalink / raw)
To: Joseph S. Myers
Cc: Uros Bizjak, Richard Henderson, Kirill Yukhin, gcc-patches
On Fri, Jan 03, 2014 at 03:39:11PM +0000, Joseph S. Myers wrote:
> On Fri, 3 Jan 2014, Jakub Jelinek wrote:
>
> > I've noticed that especially with the AVX512F introduction we use
> > GET_MODE_SIZE (<MODE>mode) quite heavily in the i386 *.md files, and
> > the problem with that is GET_MODE_SIZE isn't a compile time constant,
> > needs to read mode_size array (non-const) at runtime.
>
> It would seem better to me for genmodes to generate appropriate macro /
> inline function definitions that can be used by GET_MODE_SIZE (along the
> lines of (__builtin_constant_p (MODE) && !mode_size_variable (MODE) ?
> get_mode_size_constant (MODE) : (unsigned short) mode_size[MODE]), where
> mode_size_variable and get_mode_size_constant are always_inline functions
> generated by genmodes) - that way all targets are covered automatically,
> without needing such duplication of mode sizes. (Of course such
> optimizations wouldn't apply for a first-stage compiler bootstrapped by a
> compiler not supporting __builtin_constant_p, but lack of optimization in
> the first stage of a bootstrap is not a particular concern.)
That is certainly doable (as attached), but strangely if the patch (that I've
already committed) is reverted and this one applied, the .text savings are
much smaller.
Here are .text and .rodata readelf -WS lines from x86_64 (first 4 pairs) and
i686 (last 4 pairs) builds, always vanilla trunk before r206312, that plus
r206312 patch, without r206312 but with attached patch, with both r206312
and attached patch. So, for .text size the best is both patches, but
for .rodata patches just r206312. I'll try to look at details why this is so
next week.
[12] .text PROGBITS 00000000004f4b00 0f4b00 1131704 00 AX 0 0 16
[14] .rodata PROGBITS 0000000001626240 1226240 4093b4 00 A 0 0 64
[12] .text PROGBITS 00000000004f20a0 0f20a0 11156e4 00 AX 0 0 16
[14] .rodata PROGBITS 00000000016077c0 12077c0 3fcbb4 00 A 0 0 64
[12] .text PROGBITS 00000000004f4c60 0f4c60 112b8b4 00 AX 0 0 16
[14] .rodata PROGBITS 0000000001620540 1220540 40b134 00 A 0 0 64
[12] .text PROGBITS 00000000004f2200 0f2200 1113eb4 00 AX 0 0 16
[14] .rodata PROGBITS 00000000016060c0 12060c0 3fea74 00 A 0 0 64
[12] .text PROGBITS 0811d750 0d5750 12b4464 00 AX 0 0 16
[14] .rodata PROGBITS 093d1c00 1389c00 2d8094 00 A 0 0 64
[12] .text PROGBITS 0811b150 0d3150 12996a4 00 AX 0 0 16
[14] .rodata PROGBITS 093b4840 136c840 2d1354 00 A 0 0 64
[12] .text PROGBITS 0811d840 0d5840 12aa1e4 00 AX 0 0 16
[14] .rodata PROGBITS 093c7a40 137fa40 2d8b94 00 A 0 0 64
[12] .text PROGBITS 0811b240 0d3240 1292914 00 AX 0 0 16
[14] .rodata PROGBITS 093adb80 1365b80 2d1f94 00 A 0 0 64
2014-01-04 Jakub Jelinek <jakub@redhat.com>
* genmodes.c (struct mode_data): Add need_bytesize_adj field.
(blank_mdoe): Initialize it.
(emit_mode_size_inline, emit_mode_nunits_inline,
emit_mode_inner_inline): New functions.
(emit_insn_modes_h): Call them and surround their output with
#if GCC_VERSION >= 4001 ... #endif.
* machmode.h (GET_MODE_SIZE, GET_MODE_NUNITS, GET_MODE_INNER):
For GCC_VERSION >= 4001 use mode_*_inline routines instead of
mode_* arrays if the argument is __builtin_constant_p.
* lower-subreg.c (dump_choices): Make sure GET_MODE_SIZE argument
is enum machine_mode.
fortran/
* trans-types.c (gfc_init_kinds): Make sure GET_MODE_BITSIZE
argument is enum machine_mode.
--- gcc/lower-subreg.c.jj 2013-12-10 18:18:39.077943292 +0100
+++ gcc/lower-subreg.c 2014-01-03 18:35:00.510418999 +0100
@@ -1371,7 +1371,7 @@ dump_choices (bool speed_p, const char *
fprintf (dump_file, "Choices when optimizing for %s:\n", description);
for (i = 0; i < MAX_MACHINE_MODE; i++)
- if (GET_MODE_SIZE (i) > UNITS_PER_WORD)
+ if (GET_MODE_SIZE ((enum machine_mode) i) > UNITS_PER_WORD)
fprintf (dump_file, " %s mode %s for copy lowering.\n",
choices[speed_p].move_modes_to_split[i]
? "Splitting"
--- gcc/fortran/trans-types.c.jj 2013-11-21 22:24:18.790939654 +0100
+++ gcc/fortran/trans-types.c 2014-01-03 18:35:00.534418997 +0100
@@ -373,7 +373,7 @@ gfc_init_kinds (void)
/* The middle end doesn't support constants larger than 2*HWI.
Perhaps the target hook shouldn't have accepted these either,
but just to be safe... */
- bitsize = GET_MODE_BITSIZE (mode);
+ bitsize = GET_MODE_BITSIZE ((enum machine_mode) mode);
if (bitsize > 2*HOST_BITS_PER_WIDE_INT)
continue;
--- gcc/machmode.h.jj 2013-11-29 18:22:15.671594951 +0100
+++ gcc/machmode.h 2014-01-03 18:47:30.514374282 +0100
@@ -177,7 +177,13 @@ extern const unsigned char mode_class[NU
/* Get the size in bytes and bits of an object of mode MODE. */
extern CONST_MODE_SIZE unsigned char mode_size[NUM_MACHINE_MODES];
+#if GCC_VERSION >= 4001
+#define GET_MODE_SIZE(MODE) \
+ ((unsigned short) (__builtin_constant_p (MODE) \
+ ? mode_size_inline (MODE) : mode_size[MODE]))
+#else
#define GET_MODE_SIZE(MODE) ((unsigned short) mode_size[MODE])
+#endif
#define GET_MODE_BITSIZE(MODE) \
((unsigned short) (GET_MODE_SIZE (MODE) * BITS_PER_UNIT))
@@ -203,7 +209,13 @@ extern const unsigned HOST_WIDE_INT mode
/* Return the mode of the inner elements in a vector. */
extern const unsigned char mode_inner[NUM_MACHINE_MODES];
+#if GCC_VERSION >= 4001
+#define GET_MODE_INNER(MODE) \
+ ((enum machine_mode) (__builtin_constant_p (MODE) \
+ ? mode_inner_inline (MODE) : mode_inner[MODE]))
+#else
#define GET_MODE_INNER(MODE) ((enum machine_mode) mode_inner[MODE])
+#endif
/* Get the size in bytes or bites of the basic parts of an
object of mode MODE. */
@@ -224,7 +236,13 @@ extern const unsigned char mode_inner[NU
/* Get the number of units in the object. */
extern const unsigned char mode_nunits[NUM_MACHINE_MODES];
+#if GCC_VERSION >= 4001
+#define GET_MODE_NUNITS(MODE) \
+ ((unsigned char) (__builtin_constant_p (MODE) \
+ ? mode_nunits_inline (MODE) : mode_nunits[MODE]))
+#else
#define GET_MODE_NUNITS(MODE) mode_nunits[MODE]
+#endif
/* Get the next wider natural mode (eg, QI -> HI -> SI -> DI -> TI). */
--- gcc/genmodes.c.jj 2013-12-16 23:11:04.982989225 +0100
+++ gcc/genmodes.c 2014-01-03 18:47:16.153375138 +0100
@@ -72,6 +72,8 @@ struct mode_data
unsigned int counter; /* Rank ordering of modes */
unsigned int ibit; /* the number of integral bits */
unsigned int fbit; /* the number of fractional bits */
+ bool need_bytesize_adj; /* true if this mode need dynamic size
+ adjustment */
};
static struct mode_data *modes[MAX_MODE_CLASS];
@@ -82,7 +84,7 @@ static const struct mode_data blank_mode
0, "<unknown>", MAX_MODE_CLASS,
-1U, -1U, -1U, -1U,
0, 0, 0, 0, 0,
- "<unknown>", 0, 0, 0, 0
+ "<unknown>", 0, 0, 0, 0, false
};
static htab_t modes_by_name;
@@ -904,6 +906,105 @@ emit_max_int (void)
printf ("#define MAX_BITSIZE_MODE_ANY_MODE (%d*BITS_PER_UNIT)\n", mmax);
}
+/* Emit mode_size_inline routine into insn-modes.h header. */
+static void
+emit_mode_size_inline (void)
+{
+ int c;
+ struct mode_adjust *a;
+ struct mode_data *m;
+
+ /* Size adjustments must be propagated to all containing modes. */
+ for (a = adj_bytesize; a; a = a->next)
+ {
+ a->mode->need_bytesize_adj = true;
+ for (m = a->mode->contained; m; m = m->next_cont)
+ m->need_bytesize_adj = true;
+ }
+
+ printf ("\
+#ifdef __cplusplus\n\
+inline __attribute__((__always_inline__))\n\
+#else\n\
+extern __inline__ __attribute__((__always_inline__, __gnu_inline__))\n\
+#endif\n\
+unsigned char\n\
+mode_size_inline (enum machine_mode mode)\n\
+{\n\
+ extern %sunsigned char mode_size[NUM_MACHINE_MODES];\n\
+ switch (mode)\n\
+ {\n", adj_bytesize ? "" : "const ");
+
+ for_all_modes (c, m)
+ if (!m->need_bytesize_adj)
+ printf (" case %smode: return %u;\n", m->name, m->bytesize);
+
+ puts ("\
+ default: return mode_size[mode];\n\
+ }\n\
+}\n");
+}
+
+/* Emit mode_nunits_inline routine into insn-modes.h header. */
+static void
+emit_mode_nunits_inline (void)
+{
+ int c;
+ struct mode_data *m;
+
+ puts ("\
+#ifdef __cplusplus\n\
+inline __attribute__((__always_inline__))\n\
+#else\n\
+extern __inline__ __attribute__((__always_inline__, __gnu_inline__))\n\
+#endif\n\
+unsigned char\n\
+mode_nunits_inline (enum machine_mode mode)\n\
+{\n\
+ extern const unsigned char mode_nunits[NUM_MACHINE_MODES];\n\
+ switch (mode)\n\
+ {");
+
+ for_all_modes (c, m)
+ printf (" case %smode: return %u;\n", m->name, m->ncomponents);
+
+ puts ("\
+ default: return mode_nunits[mode];\n\
+ }\n\
+}\n");
+}
+
+/* Emit mode_inner_inline routine into insn-modes.h header. */
+static void
+emit_mode_inner_inline (void)
+{
+ int c;
+ struct mode_data *m;
+
+ puts ("\
+#ifdef __cplusplus\n\
+inline __attribute__((__always_inline__))\n\
+#else\n\
+extern __inline__ __attribute__((__always_inline__, __gnu_inline__))\n\
+#endif\n\
+unsigned char\n\
+mode_inner_inline (enum machine_mode mode)\n\
+{\n\
+ extern const unsigned char mode_inner[NUM_MACHINE_MODES];\n\
+ switch (mode)\n\
+ {");
+
+ for_all_modes (c, m)
+ printf (" case %smode: return %smode;\n", m->name,
+ c != MODE_PARTIAL_INT && m->component
+ ? m->component->name : void_mode->name);
+
+ puts ("\
+ default: return mode_inner[mode];\n\
+ }\n\
+}\n");
+}
+
static void
emit_insn_modes_h (void)
{
@@ -969,6 +1070,12 @@ enum machine_mode\n{");
printf ("#define CONST_MODE_IBIT%s\n", adj_ibit ? "" : " const");
printf ("#define CONST_MODE_FBIT%s\n", adj_fbit ? "" : " const");
emit_max_int ();
+ puts ("\n#if GCC_VERSION >= 4001\n");
+ emit_mode_size_inline ();
+ emit_mode_nunits_inline ();
+ emit_mode_inner_inline ();
+ puts ("#endif /* GCC_VERSION >= 4001 */");
+
puts ("\
\n\
#endif /* insn-modes.h */");
Jakub
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Introduce MODE_SIZE mode attribute
2014-01-03 23:38 ` Jakub Jelinek
@ 2014-01-04 2:27 ` Andrew Pinski
2014-01-04 2:30 ` Andrew Pinski
2014-01-06 15:21 ` Jakub Jelinek
1 sibling, 1 reply; 8+ messages in thread
From: Andrew Pinski @ 2014-01-04 2:27 UTC (permalink / raw)
To: Jakub Jelinek
Cc: Joseph S. Myers, Uros Bizjak, Richard Henderson, Kirill Yukhin,
GCC Patches
On Fri, Jan 3, 2014 at 3:37 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Fri, Jan 03, 2014 at 03:39:11PM +0000, Joseph S. Myers wrote:
>> On Fri, 3 Jan 2014, Jakub Jelinek wrote:
>>
>> > I've noticed that especially with the AVX512F introduction we use
>> > GET_MODE_SIZE (<MODE>mode) quite heavily in the i386 *.md files, and
>> > the problem with that is GET_MODE_SIZE isn't a compile time constant,
>> > needs to read mode_size array (non-const) at runtime.
>>
>> It would seem better to me for genmodes to generate appropriate macro /
>> inline function definitions that can be used by GET_MODE_SIZE (along the
>> lines of (__builtin_constant_p (MODE) && !mode_size_variable (MODE) ?
>> get_mode_size_constant (MODE) : (unsigned short) mode_size[MODE]), where
>> mode_size_variable and get_mode_size_constant are always_inline functions
>> generated by genmodes) - that way all targets are covered automatically,
>> without needing such duplication of mode sizes. (Of course such
>> optimizations wouldn't apply for a first-stage compiler bootstrapped by a
>> compiler not supporting __builtin_constant_p, but lack of optimization in
>> the first stage of a bootstrap is not a particular concern.)
>
> That is certainly doable (as attached), but strangely if the patch (that I've
> already committed) is reverted and this one applied, the .text savings are
> much smaller.
>
> Here are .text and .rodata readelf -WS lines from x86_64 (first 4 pairs) and
> i686 (last 4 pairs) builds, always vanilla trunk before r206312, that plus
> r206312 patch, without r206312 but with attached patch, with both r206312
> and attached patch. So, for .text size the best is both patches, but
> for .rodata patches just r206312. I'll try to look at details why this is so
> next week.
>
> [12] .text PROGBITS 00000000004f4b00 0f4b00 1131704 00 AX 0 0 16
> [14] .rodata PROGBITS 0000000001626240 1226240 4093b4 00 A 0 0 64
> [12] .text PROGBITS 00000000004f20a0 0f20a0 11156e4 00 AX 0 0 16
> [14] .rodata PROGBITS 00000000016077c0 12077c0 3fcbb4 00 A 0 0 64
> [12] .text PROGBITS 00000000004f4c60 0f4c60 112b8b4 00 AX 0 0 16
> [14] .rodata PROGBITS 0000000001620540 1220540 40b134 00 A 0 0 64
> [12] .text PROGBITS 00000000004f2200 0f2200 1113eb4 00 AX 0 0 16
> [14] .rodata PROGBITS 00000000016060c0 12060c0 3fea74 00 A 0 0 64
> [12] .text PROGBITS 0811d750 0d5750 12b4464 00 AX 0 0 16
> [14] .rodata PROGBITS 093d1c00 1389c00 2d8094 00 A 0 0 64
> [12] .text PROGBITS 0811b150 0d3150 12996a4 00 AX 0 0 16
> [14] .rodata PROGBITS 093b4840 136c840 2d1354 00 A 0 0 64
> [12] .text PROGBITS 0811d840 0d5840 12aa1e4 00 AX 0 0 16
> [14] .rodata PROGBITS 093c7a40 137fa40 2d8b94 00 A 0 0 64
> [12] .text PROGBITS 0811b240 0d3240 1292914 00 AX 0 0 16
> [14] .rodata PROGBITS 093adb80 1365b80 2d1f94 00 A 0 0 64
>
> 2014-01-04 Jakub Jelinek <jakub@redhat.com>
>
> * genmodes.c (struct mode_data): Add need_bytesize_adj field.
> (blank_mdoe): Initialize it.
> (emit_mode_size_inline, emit_mode_nunits_inline,
> emit_mode_inner_inline): New functions.
> (emit_insn_modes_h): Call them and surround their output with
> #if GCC_VERSION >= 4001 ... #endif.
> * machmode.h (GET_MODE_SIZE, GET_MODE_NUNITS, GET_MODE_INNER):
> For GCC_VERSION >= 4001 use mode_*_inline routines instead of
> mode_* arrays if the argument is __builtin_constant_p.
> * lower-subreg.c (dump_choices): Make sure GET_MODE_SIZE argument
> is enum machine_mode.
> fortran/
> * trans-types.c (gfc_init_kinds): Make sure GET_MODE_BITSIZE
> argument is enum machine_mode.
It turns out Jorn filed a bug about this exact issue (back in 2008).
See bug 36109.
Thanks,
Andrew Pinski
>
> --- gcc/lower-subreg.c.jj 2013-12-10 18:18:39.077943292 +0100
> +++ gcc/lower-subreg.c 2014-01-03 18:35:00.510418999 +0100
> @@ -1371,7 +1371,7 @@ dump_choices (bool speed_p, const char *
> fprintf (dump_file, "Choices when optimizing for %s:\n", description);
>
> for (i = 0; i < MAX_MACHINE_MODE; i++)
> - if (GET_MODE_SIZE (i) > UNITS_PER_WORD)
> + if (GET_MODE_SIZE ((enum machine_mode) i) > UNITS_PER_WORD)
> fprintf (dump_file, " %s mode %s for copy lowering.\n",
> choices[speed_p].move_modes_to_split[i]
> ? "Splitting"
> --- gcc/fortran/trans-types.c.jj 2013-11-21 22:24:18.790939654 +0100
> +++ gcc/fortran/trans-types.c 2014-01-03 18:35:00.534418997 +0100
> @@ -373,7 +373,7 @@ gfc_init_kinds (void)
> /* The middle end doesn't support constants larger than 2*HWI.
> Perhaps the target hook shouldn't have accepted these either,
> but just to be safe... */
> - bitsize = GET_MODE_BITSIZE (mode);
> + bitsize = GET_MODE_BITSIZE ((enum machine_mode) mode);
> if (bitsize > 2*HOST_BITS_PER_WIDE_INT)
> continue;
>
> --- gcc/machmode.h.jj 2013-11-29 18:22:15.671594951 +0100
> +++ gcc/machmode.h 2014-01-03 18:47:30.514374282 +0100
> @@ -177,7 +177,13 @@ extern const unsigned char mode_class[NU
> /* Get the size in bytes and bits of an object of mode MODE. */
>
> extern CONST_MODE_SIZE unsigned char mode_size[NUM_MACHINE_MODES];
> +#if GCC_VERSION >= 4001
> +#define GET_MODE_SIZE(MODE) \
> + ((unsigned short) (__builtin_constant_p (MODE) \
> + ? mode_size_inline (MODE) : mode_size[MODE]))
> +#else
> #define GET_MODE_SIZE(MODE) ((unsigned short) mode_size[MODE])
> +#endif
> #define GET_MODE_BITSIZE(MODE) \
> ((unsigned short) (GET_MODE_SIZE (MODE) * BITS_PER_UNIT))
>
> @@ -203,7 +209,13 @@ extern const unsigned HOST_WIDE_INT mode
> /* Return the mode of the inner elements in a vector. */
>
> extern const unsigned char mode_inner[NUM_MACHINE_MODES];
> +#if GCC_VERSION >= 4001
> +#define GET_MODE_INNER(MODE) \
> + ((enum machine_mode) (__builtin_constant_p (MODE) \
> + ? mode_inner_inline (MODE) : mode_inner[MODE]))
> +#else
> #define GET_MODE_INNER(MODE) ((enum machine_mode) mode_inner[MODE])
> +#endif
>
> /* Get the size in bytes or bites of the basic parts of an
> object of mode MODE. */
> @@ -224,7 +236,13 @@ extern const unsigned char mode_inner[NU
> /* Get the number of units in the object. */
>
> extern const unsigned char mode_nunits[NUM_MACHINE_MODES];
> +#if GCC_VERSION >= 4001
> +#define GET_MODE_NUNITS(MODE) \
> + ((unsigned char) (__builtin_constant_p (MODE) \
> + ? mode_nunits_inline (MODE) : mode_nunits[MODE]))
> +#else
> #define GET_MODE_NUNITS(MODE) mode_nunits[MODE]
> +#endif
>
> /* Get the next wider natural mode (eg, QI -> HI -> SI -> DI -> TI). */
>
> --- gcc/genmodes.c.jj 2013-12-16 23:11:04.982989225 +0100
> +++ gcc/genmodes.c 2014-01-03 18:47:16.153375138 +0100
> @@ -72,6 +72,8 @@ struct mode_data
> unsigned int counter; /* Rank ordering of modes */
> unsigned int ibit; /* the number of integral bits */
> unsigned int fbit; /* the number of fractional bits */
> + bool need_bytesize_adj; /* true if this mode need dynamic size
> + adjustment */
> };
>
> static struct mode_data *modes[MAX_MODE_CLASS];
> @@ -82,7 +84,7 @@ static const struct mode_data blank_mode
> 0, "<unknown>", MAX_MODE_CLASS,
> -1U, -1U, -1U, -1U,
> 0, 0, 0, 0, 0,
> - "<unknown>", 0, 0, 0, 0
> + "<unknown>", 0, 0, 0, 0, false
> };
>
> static htab_t modes_by_name;
> @@ -904,6 +906,105 @@ emit_max_int (void)
> printf ("#define MAX_BITSIZE_MODE_ANY_MODE (%d*BITS_PER_UNIT)\n", mmax);
> }
>
> +/* Emit mode_size_inline routine into insn-modes.h header. */
> +static void
> +emit_mode_size_inline (void)
> +{
> + int c;
> + struct mode_adjust *a;
> + struct mode_data *m;
> +
> + /* Size adjustments must be propagated to all containing modes. */
> + for (a = adj_bytesize; a; a = a->next)
> + {
> + a->mode->need_bytesize_adj = true;
> + for (m = a->mode->contained; m; m = m->next_cont)
> + m->need_bytesize_adj = true;
> + }
> +
> + printf ("\
> +#ifdef __cplusplus\n\
> +inline __attribute__((__always_inline__))\n\
> +#else\n\
> +extern __inline__ __attribute__((__always_inline__, __gnu_inline__))\n\
> +#endif\n\
> +unsigned char\n\
> +mode_size_inline (enum machine_mode mode)\n\
> +{\n\
> + extern %sunsigned char mode_size[NUM_MACHINE_MODES];\n\
> + switch (mode)\n\
> + {\n", adj_bytesize ? "" : "const ");
> +
> + for_all_modes (c, m)
> + if (!m->need_bytesize_adj)
> + printf (" case %smode: return %u;\n", m->name, m->bytesize);
> +
> + puts ("\
> + default: return mode_size[mode];\n\
> + }\n\
> +}\n");
> +}
> +
> +/* Emit mode_nunits_inline routine into insn-modes.h header. */
> +static void
> +emit_mode_nunits_inline (void)
> +{
> + int c;
> + struct mode_data *m;
> +
> + puts ("\
> +#ifdef __cplusplus\n\
> +inline __attribute__((__always_inline__))\n\
> +#else\n\
> +extern __inline__ __attribute__((__always_inline__, __gnu_inline__))\n\
> +#endif\n\
> +unsigned char\n\
> +mode_nunits_inline (enum machine_mode mode)\n\
> +{\n\
> + extern const unsigned char mode_nunits[NUM_MACHINE_MODES];\n\
> + switch (mode)\n\
> + {");
> +
> + for_all_modes (c, m)
> + printf (" case %smode: return %u;\n", m->name, m->ncomponents);
> +
> + puts ("\
> + default: return mode_nunits[mode];\n\
> + }\n\
> +}\n");
> +}
> +
> +/* Emit mode_inner_inline routine into insn-modes.h header. */
> +static void
> +emit_mode_inner_inline (void)
> +{
> + int c;
> + struct mode_data *m;
> +
> + puts ("\
> +#ifdef __cplusplus\n\
> +inline __attribute__((__always_inline__))\n\
> +#else\n\
> +extern __inline__ __attribute__((__always_inline__, __gnu_inline__))\n\
> +#endif\n\
> +unsigned char\n\
> +mode_inner_inline (enum machine_mode mode)\n\
> +{\n\
> + extern const unsigned char mode_inner[NUM_MACHINE_MODES];\n\
> + switch (mode)\n\
> + {");
> +
> + for_all_modes (c, m)
> + printf (" case %smode: return %smode;\n", m->name,
> + c != MODE_PARTIAL_INT && m->component
> + ? m->component->name : void_mode->name);
> +
> + puts ("\
> + default: return mode_inner[mode];\n\
> + }\n\
> +}\n");
> +}
> +
> static void
> emit_insn_modes_h (void)
> {
> @@ -969,6 +1070,12 @@ enum machine_mode\n{");
> printf ("#define CONST_MODE_IBIT%s\n", adj_ibit ? "" : " const");
> printf ("#define CONST_MODE_FBIT%s\n", adj_fbit ? "" : " const");
> emit_max_int ();
> + puts ("\n#if GCC_VERSION >= 4001\n");
> + emit_mode_size_inline ();
> + emit_mode_nunits_inline ();
> + emit_mode_inner_inline ();
> + puts ("#endif /* GCC_VERSION >= 4001 */");
> +
> puts ("\
> \n\
> #endif /* insn-modes.h */");
>
>
> Jakub
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Introduce MODE_SIZE mode attribute
2014-01-04 2:27 ` Andrew Pinski
@ 2014-01-04 2:30 ` Andrew Pinski
0 siblings, 0 replies; 8+ messages in thread
From: Andrew Pinski @ 2014-01-04 2:30 UTC (permalink / raw)
To: Jakub Jelinek
Cc: Joseph S. Myers, Uros Bizjak, Richard Henderson, Kirill Yukhin,
GCC Patches
On Fri, Jan 3, 2014 at 6:27 PM, Andrew Pinski <pinskia@gmail.com> wrote:
> On Fri, Jan 3, 2014 at 3:37 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>> On Fri, Jan 03, 2014 at 03:39:11PM +0000, Joseph S. Myers wrote:
>>> On Fri, 3 Jan 2014, Jakub Jelinek wrote:
>>>
>>> > I've noticed that especially with the AVX512F introduction we use
>>> > GET_MODE_SIZE (<MODE>mode) quite heavily in the i386 *.md files, and
>>> > the problem with that is GET_MODE_SIZE isn't a compile time constant,
>>> > needs to read mode_size array (non-const) at runtime.
>>>
>>> It would seem better to me for genmodes to generate appropriate macro /
>>> inline function definitions that can be used by GET_MODE_SIZE (along the
>>> lines of (__builtin_constant_p (MODE) && !mode_size_variable (MODE) ?
>>> get_mode_size_constant (MODE) : (unsigned short) mode_size[MODE]), where
>>> mode_size_variable and get_mode_size_constant are always_inline functions
>>> generated by genmodes) - that way all targets are covered automatically,
>>> without needing such duplication of mode sizes. (Of course such
>>> optimizations wouldn't apply for a first-stage compiler bootstrapped by a
>>> compiler not supporting __builtin_constant_p, but lack of optimization in
>>> the first stage of a bootstrap is not a particular concern.)
>>
>> That is certainly doable (as attached), but strangely if the patch (that I've
>> already committed) is reverted and this one applied, the .text savings are
>> much smaller.
>>
>> Here are .text and .rodata readelf -WS lines from x86_64 (first 4 pairs) and
>> i686 (last 4 pairs) builds, always vanilla trunk before r206312, that plus
>> r206312 patch, without r206312 but with attached patch, with both r206312
>> and attached patch. So, for .text size the best is both patches, but
>> for .rodata patches just r206312. I'll try to look at details why this is so
>> next week.
>>
>> [12] .text PROGBITS 00000000004f4b00 0f4b00 1131704 00 AX 0 0 16
>> [14] .rodata PROGBITS 0000000001626240 1226240 4093b4 00 A 0 0 64
>> [12] .text PROGBITS 00000000004f20a0 0f20a0 11156e4 00 AX 0 0 16
>> [14] .rodata PROGBITS 00000000016077c0 12077c0 3fcbb4 00 A 0 0 64
>> [12] .text PROGBITS 00000000004f4c60 0f4c60 112b8b4 00 AX 0 0 16
>> [14] .rodata PROGBITS 0000000001620540 1220540 40b134 00 A 0 0 64
>> [12] .text PROGBITS 00000000004f2200 0f2200 1113eb4 00 AX 0 0 16
>> [14] .rodata PROGBITS 00000000016060c0 12060c0 3fea74 00 A 0 0 64
>> [12] .text PROGBITS 0811d750 0d5750 12b4464 00 AX 0 0 16
>> [14] .rodata PROGBITS 093d1c00 1389c00 2d8094 00 A 0 0 64
>> [12] .text PROGBITS 0811b150 0d3150 12996a4 00 AX 0 0 16
>> [14] .rodata PROGBITS 093b4840 136c840 2d1354 00 A 0 0 64
>> [12] .text PROGBITS 0811d840 0d5840 12aa1e4 00 AX 0 0 16
>> [14] .rodata PROGBITS 093c7a40 137fa40 2d8b94 00 A 0 0 64
>> [12] .text PROGBITS 0811b240 0d3240 1292914 00 AX 0 0 16
>> [14] .rodata PROGBITS 093adb80 1365b80 2d1f94 00 A 0 0 64
>>
>> 2014-01-04 Jakub Jelinek <jakub@redhat.com>
>>
>> * genmodes.c (struct mode_data): Add need_bytesize_adj field.
>> (blank_mdoe): Initialize it.
>> (emit_mode_size_inline, emit_mode_nunits_inline,
>> emit_mode_inner_inline): New functions.
>> (emit_insn_modes_h): Call them and surround their output with
>> #if GCC_VERSION >= 4001 ... #endif.
>> * machmode.h (GET_MODE_SIZE, GET_MODE_NUNITS, GET_MODE_INNER):
>> For GCC_VERSION >= 4001 use mode_*_inline routines instead of
>> mode_* arrays if the argument is __builtin_constant_p.
>> * lower-subreg.c (dump_choices): Make sure GET_MODE_SIZE argument
>> is enum machine_mode.
>> fortran/
>> * trans-types.c (gfc_init_kinds): Make sure GET_MODE_BITSIZE
>> argument is enum machine_mode.
>
> It turns out Jorn filed a bug about this exact issue (back in 2008).
> See bug 36109.
Also Kazu filed a similar thing about TREE_CODE_LENGTH and
TREE_CODE_CLASS, see bug 14840; I attached a patch but I never got
around to seeing if it improves compile time speed either. It
definitely showed up in fold-const.c code at one point.
Thanks,
Andrew Pinski
>
>
> Thanks,
> Andrew Pinski
>
>
>>
>> --- gcc/lower-subreg.c.jj 2013-12-10 18:18:39.077943292 +0100
>> +++ gcc/lower-subreg.c 2014-01-03 18:35:00.510418999 +0100
>> @@ -1371,7 +1371,7 @@ dump_choices (bool speed_p, const char *
>> fprintf (dump_file, "Choices when optimizing for %s:\n", description);
>>
>> for (i = 0; i < MAX_MACHINE_MODE; i++)
>> - if (GET_MODE_SIZE (i) > UNITS_PER_WORD)
>> + if (GET_MODE_SIZE ((enum machine_mode) i) > UNITS_PER_WORD)
>> fprintf (dump_file, " %s mode %s for copy lowering.\n",
>> choices[speed_p].move_modes_to_split[i]
>> ? "Splitting"
>> --- gcc/fortran/trans-types.c.jj 2013-11-21 22:24:18.790939654 +0100
>> +++ gcc/fortran/trans-types.c 2014-01-03 18:35:00.534418997 +0100
>> @@ -373,7 +373,7 @@ gfc_init_kinds (void)
>> /* The middle end doesn't support constants larger than 2*HWI.
>> Perhaps the target hook shouldn't have accepted these either,
>> but just to be safe... */
>> - bitsize = GET_MODE_BITSIZE (mode);
>> + bitsize = GET_MODE_BITSIZE ((enum machine_mode) mode);
>> if (bitsize > 2*HOST_BITS_PER_WIDE_INT)
>> continue;
>>
>> --- gcc/machmode.h.jj 2013-11-29 18:22:15.671594951 +0100
>> +++ gcc/machmode.h 2014-01-03 18:47:30.514374282 +0100
>> @@ -177,7 +177,13 @@ extern const unsigned char mode_class[NU
>> /* Get the size in bytes and bits of an object of mode MODE. */
>>
>> extern CONST_MODE_SIZE unsigned char mode_size[NUM_MACHINE_MODES];
>> +#if GCC_VERSION >= 4001
>> +#define GET_MODE_SIZE(MODE) \
>> + ((unsigned short) (__builtin_constant_p (MODE) \
>> + ? mode_size_inline (MODE) : mode_size[MODE]))
>> +#else
>> #define GET_MODE_SIZE(MODE) ((unsigned short) mode_size[MODE])
>> +#endif
>> #define GET_MODE_BITSIZE(MODE) \
>> ((unsigned short) (GET_MODE_SIZE (MODE) * BITS_PER_UNIT))
>>
>> @@ -203,7 +209,13 @@ extern const unsigned HOST_WIDE_INT mode
>> /* Return the mode of the inner elements in a vector. */
>>
>> extern const unsigned char mode_inner[NUM_MACHINE_MODES];
>> +#if GCC_VERSION >= 4001
>> +#define GET_MODE_INNER(MODE) \
>> + ((enum machine_mode) (__builtin_constant_p (MODE) \
>> + ? mode_inner_inline (MODE) : mode_inner[MODE]))
>> +#else
>> #define GET_MODE_INNER(MODE) ((enum machine_mode) mode_inner[MODE])
>> +#endif
>>
>> /* Get the size in bytes or bites of the basic parts of an
>> object of mode MODE. */
>> @@ -224,7 +236,13 @@ extern const unsigned char mode_inner[NU
>> /* Get the number of units in the object. */
>>
>> extern const unsigned char mode_nunits[NUM_MACHINE_MODES];
>> +#if GCC_VERSION >= 4001
>> +#define GET_MODE_NUNITS(MODE) \
>> + ((unsigned char) (__builtin_constant_p (MODE) \
>> + ? mode_nunits_inline (MODE) : mode_nunits[MODE]))
>> +#else
>> #define GET_MODE_NUNITS(MODE) mode_nunits[MODE]
>> +#endif
>>
>> /* Get the next wider natural mode (eg, QI -> HI -> SI -> DI -> TI). */
>>
>> --- gcc/genmodes.c.jj 2013-12-16 23:11:04.982989225 +0100
>> +++ gcc/genmodes.c 2014-01-03 18:47:16.153375138 +0100
>> @@ -72,6 +72,8 @@ struct mode_data
>> unsigned int counter; /* Rank ordering of modes */
>> unsigned int ibit; /* the number of integral bits */
>> unsigned int fbit; /* the number of fractional bits */
>> + bool need_bytesize_adj; /* true if this mode need dynamic size
>> + adjustment */
>> };
>>
>> static struct mode_data *modes[MAX_MODE_CLASS];
>> @@ -82,7 +84,7 @@ static const struct mode_data blank_mode
>> 0, "<unknown>", MAX_MODE_CLASS,
>> -1U, -1U, -1U, -1U,
>> 0, 0, 0, 0, 0,
>> - "<unknown>", 0, 0, 0, 0
>> + "<unknown>", 0, 0, 0, 0, false
>> };
>>
>> static htab_t modes_by_name;
>> @@ -904,6 +906,105 @@ emit_max_int (void)
>> printf ("#define MAX_BITSIZE_MODE_ANY_MODE (%d*BITS_PER_UNIT)\n", mmax);
>> }
>>
>> +/* Emit mode_size_inline routine into insn-modes.h header. */
>> +static void
>> +emit_mode_size_inline (void)
>> +{
>> + int c;
>> + struct mode_adjust *a;
>> + struct mode_data *m;
>> +
>> + /* Size adjustments must be propagated to all containing modes. */
>> + for (a = adj_bytesize; a; a = a->next)
>> + {
>> + a->mode->need_bytesize_adj = true;
>> + for (m = a->mode->contained; m; m = m->next_cont)
>> + m->need_bytesize_adj = true;
>> + }
>> +
>> + printf ("\
>> +#ifdef __cplusplus\n\
>> +inline __attribute__((__always_inline__))\n\
>> +#else\n\
>> +extern __inline__ __attribute__((__always_inline__, __gnu_inline__))\n\
>> +#endif\n\
>> +unsigned char\n\
>> +mode_size_inline (enum machine_mode mode)\n\
>> +{\n\
>> + extern %sunsigned char mode_size[NUM_MACHINE_MODES];\n\
>> + switch (mode)\n\
>> + {\n", adj_bytesize ? "" : "const ");
>> +
>> + for_all_modes (c, m)
>> + if (!m->need_bytesize_adj)
>> + printf (" case %smode: return %u;\n", m->name, m->bytesize);
>> +
>> + puts ("\
>> + default: return mode_size[mode];\n\
>> + }\n\
>> +}\n");
>> +}
>> +
>> +/* Emit mode_nunits_inline routine into insn-modes.h header. */
>> +static void
>> +emit_mode_nunits_inline (void)
>> +{
>> + int c;
>> + struct mode_data *m;
>> +
>> + puts ("\
>> +#ifdef __cplusplus\n\
>> +inline __attribute__((__always_inline__))\n\
>> +#else\n\
>> +extern __inline__ __attribute__((__always_inline__, __gnu_inline__))\n\
>> +#endif\n\
>> +unsigned char\n\
>> +mode_nunits_inline (enum machine_mode mode)\n\
>> +{\n\
>> + extern const unsigned char mode_nunits[NUM_MACHINE_MODES];\n\
>> + switch (mode)\n\
>> + {");
>> +
>> + for_all_modes (c, m)
>> + printf (" case %smode: return %u;\n", m->name, m->ncomponents);
>> +
>> + puts ("\
>> + default: return mode_nunits[mode];\n\
>> + }\n\
>> +}\n");
>> +}
>> +
>> +/* Emit mode_inner_inline routine into insn-modes.h header. */
>> +static void
>> +emit_mode_inner_inline (void)
>> +{
>> + int c;
>> + struct mode_data *m;
>> +
>> + puts ("\
>> +#ifdef __cplusplus\n\
>> +inline __attribute__((__always_inline__))\n\
>> +#else\n\
>> +extern __inline__ __attribute__((__always_inline__, __gnu_inline__))\n\
>> +#endif\n\
>> +unsigned char\n\
>> +mode_inner_inline (enum machine_mode mode)\n\
>> +{\n\
>> + extern const unsigned char mode_inner[NUM_MACHINE_MODES];\n\
>> + switch (mode)\n\
>> + {");
>> +
>> + for_all_modes (c, m)
>> + printf (" case %smode: return %smode;\n", m->name,
>> + c != MODE_PARTIAL_INT && m->component
>> + ? m->component->name : void_mode->name);
>> +
>> + puts ("\
>> + default: return mode_inner[mode];\n\
>> + }\n\
>> +}\n");
>> +}
>> +
>> static void
>> emit_insn_modes_h (void)
>> {
>> @@ -969,6 +1070,12 @@ enum machine_mode\n{");
>> printf ("#define CONST_MODE_IBIT%s\n", adj_ibit ? "" : " const");
>> printf ("#define CONST_MODE_FBIT%s\n", adj_fbit ? "" : " const");
>> emit_max_int ();
>> + puts ("\n#if GCC_VERSION >= 4001\n");
>> + emit_mode_size_inline ();
>> + emit_mode_nunits_inline ();
>> + emit_mode_inner_inline ();
>> + puts ("#endif /* GCC_VERSION >= 4001 */");
>> +
>> puts ("\
>> \n\
>> #endif /* insn-modes.h */");
>>
>>
>> Jakub
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Introduce MODE_SIZE mode attribute
2014-01-03 23:38 ` Jakub Jelinek
2014-01-04 2:27 ` Andrew Pinski
@ 2014-01-06 15:21 ` Jakub Jelinek
2014-04-18 15:20 ` H.J. Lu
1 sibling, 1 reply; 8+ messages in thread
From: Jakub Jelinek @ 2014-01-06 15:21 UTC (permalink / raw)
To: Joseph S. Myers
Cc: Uros Bizjak, Richard Henderson, Kirill Yukhin, gcc-patches
On Sat, Jan 04, 2014 at 12:37:57AM +0100, Jakub Jelinek wrote:
> That is certainly doable (as attached), but strangely if the patch (that I've
> already committed) is reverted and this one applied, the .text savings are
> much smaller.
>
> Here are .text and .rodata readelf -WS lines from x86_64 (first 4 pairs) and
> i686 (last 4 pairs) builds, always vanilla trunk before r206312, that plus
> r206312 patch, without r206312 but with attached patch, with both r206312
> and attached patch. So, for .text size the best is both patches, but
> for .rodata patches just r206312. I'll try to look at details why this is so
> next week.
The difference is I think caused by the way gencondition.c works.
As the array with the conditions is a toplevel array, __builtin_constant_p
is folded there already during the parsing, after folding the conditions.
If I (manually for now) move the build/gencondmd.c insn_conditions array
instead of main and remove static const keywords, I get undefined references
when linking build/gencondmd, on x86_64 about the:
`default_target_flag_state'
`flag_unsafe_math_optimizations'
`insn'
`ix86_arch_features'
`ix86_fpmath'
`ix86_isa_flags'
`ix86_stack_protector_guard'
`operands'
`reload_completed'
`reload_in_progress'
`target_flags'
symbols.
Jakub
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Introduce MODE_SIZE mode attribute
2014-01-06 15:21 ` Jakub Jelinek
@ 2014-04-18 15:20 ` H.J. Lu
0 siblings, 0 replies; 8+ messages in thread
From: H.J. Lu @ 2014-04-18 15:20 UTC (permalink / raw)
To: Jakub Jelinek
Cc: Joseph S. Myers, Uros Bizjak, Richard Henderson, Kirill Yukhin,
GCC Patches
On Mon, Jan 6, 2014 at 7:20 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Sat, Jan 04, 2014 at 12:37:57AM +0100, Jakub Jelinek wrote:
>> That is certainly doable (as attached), but strangely if the patch (that I've
>> already committed) is reverted and this one applied, the .text savings are
>> much smaller.
>>
>> Here are .text and .rodata readelf -WS lines from x86_64 (first 4 pairs) and
>> i686 (last 4 pairs) builds, always vanilla trunk before r206312, that plus
>> r206312 patch, without r206312 but with attached patch, with both r206312
>> and attached patch. So, for .text size the best is both patches, but
>> for .rodata patches just r206312. I'll try to look at details why this is so
>> next week.
>
> The difference is I think caused by the way gencondition.c works.
> As the array with the conditions is a toplevel array, __builtin_constant_p
> is folded there already during the parsing, after folding the conditions.
>
For some reason, it triggered:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60887
--
H.J.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-04-18 15:00 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-03 8:48 [PATCH] Introduce MODE_SIZE mode attribute Jakub Jelinek
2014-01-03 9:23 ` Uros Bizjak
2014-01-03 15:39 ` Joseph S. Myers
2014-01-03 23:38 ` Jakub Jelinek
2014-01-04 2:27 ` Andrew Pinski
2014-01-04 2:30 ` Andrew Pinski
2014-01-06 15:21 ` Jakub Jelinek
2014-04-18 15:20 ` H.J. Lu
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).