On 2/16/19, Jakub Jelinek <ja...@redhat.com> wrote:
> Hi!
>
> This is something I've noticed in a s390 change I'll post soon (where it
> was
> even completely unnecessary), but it applies to i386 backend too.
> Seems we have lots of .bss global state, 66x 64-byte and 61x 128-byte long
> static buffers. Instead of doing
> static char buf[128];
> ...
> s{,n}printf (buf, ...);
> ...
> return buf;
> in the insn templates we can do:
> char buf[128];
> ...
> s{,n}printf (buf, ...);
> ...
> output_asm_insn (buf, operands);
> return "";
> and avoid that way the global state. The only problem with that is
> that final.c does something in between:
> 1) if return from the template is NULL, not this case
> 2) if return from the template is "#", not this case
> 3) if (targetm.asm_out.unwind_emit_before_insn
> && targetm.asm_out.unwind_emit)
> targetm.asm_out.unwind_emit (asm_out_file, insn);
> while cygming.h has
> #define TARGET_ASM_UNWIND_EMIT i386_pe_seh_unwind_emit
> #define TARGET_ASM_UNWIND_EMIT_BEFORE_INSN false
> it is ok too (and other i386 subtargets don't do either,
> so unwind_emit_before_insn is true (the default) and unwind_emit
> NULL
> 4) rtx_call_insn *call_insn = dyn_cast <rtx_call_insn *> (insn);
> if (call_insn != NULL)
> that is for calls only, the patch doesn't change any calls
> Those 4 spots are in between get_insn_template and
> output_asm_insn (templ, recog_data.operand);
> which starts with:
> /* An insn may return a null string template
> in a case where no assembler code is needed. */
> if (*templ == 0)
> return;
> so I think the patch doesn't make it more costly, there is just
> one output_asm_insn extra call and the old one will return immediately.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2019-02-16 Jakub Jelinek <ja...@redhat.com>
>
> * config/i386/i386.md (*movqi_internal): Remove static from
> buf variable. Use output_asm_insn (buf, operands); return "";
> instead of return buf;.
> * config/i386/sse.md (<sse>_andnot<mode>3<mask_name>,
> *<code><mode>3<mask_name>, *andnot<mode>3, *andnottf3, *<code><mode>3,
> *<code>tf3, <mask_codefor><code><mode>3<mask_name>): Likewise.
Looks like cargo cult programming to me (some of the copies are even mine ;) .
OK.
Thanks,
Uros.
>
> --- gcc/config/i386/i386.md.jj 2019-02-12 21:48:53.183072497 +0100
> +++ gcc/config/i386/i386.md 2019-02-15 23:25:36.198589133 +0100
> @@ -2531,7 +2531,7 @@ (define_insn "*movqi_internal"
> "Q ,R,r,n,m,q,rn, m,qn,r,k,k,k,m,C,BC"))]
> "!(MEM_P (operands[0]) && MEM_P (operands[1]))"
> {
> - static char buf[128];
> + char buf[128];
> const char *ops;
> const char *suffix;
>
> @@ -2564,7 +2564,8 @@ (define_insn "*movqi_internal"
> suffix = (get_attr_mode (insn) == MODE_HI) ? "w" : "b";
>
> snprintf (buf, sizeof (buf), ops, suffix);
> - return buf;
> + output_asm_insn (buf, operands);
> + return "";
>
> case TYPE_MSKLOG:
> if (operands[1] == const0_rtx)
> --- gcc/config/i386/sse.md.jj 2019-02-14 08:06:39.446519415 +0100
> +++ gcc/config/i386/sse.md 2019-02-15 23:28:54.305366640 +0100
> @@ -3198,7 +3198,7 @@ (define_insn "<sse>_andnot<mode>3<mask_n
> (match_operand:VF_128_256 2 "vector_operand" "xBm,xm,vm,vm")))]
> "TARGET_SSE && <mask_avx512vl_condition>"
> {
> - static char buf[128];
> + char buf[128];
> const char *ops;
> const char *suffix;
>
> @@ -3233,7 +3233,8 @@ (define_insn "<sse>_andnot<mode>3<mask_n
> }
>
> snprintf (buf, sizeof (buf), ops, suffix);
> - return buf;
> + output_asm_insn (buf, operands);
> + return "";
> }
> [(set_attr "isa" "noavx,avx,avx512dq,avx512f")
> (set_attr "type" "sselog")
> @@ -3264,7 +3265,7 @@ (define_insn "<sse>_andnot<mode>3<mask_n
> (match_operand:VF_512 2 "nonimmediate_operand" "vm")))]
> "TARGET_AVX512F"
> {
> - static char buf[128];
> + char buf[128];
> const char *ops;
> const char *suffix;
>
> @@ -3281,7 +3282,8 @@ (define_insn "<sse>_andnot<mode>3<mask_n
> snprintf (buf, sizeof (buf),
> "v%sandn%s\t{%%2, %%1, %%0<mask_operand3_1>|%%0<mask_operand3_1>,
> %%1,
> %%2}",
> ops, suffix);
> - return buf;
> + output_asm_insn (buf, operands);
> + return "";
> }
> [(set_attr "type" "sselog")
> (set_attr "prefix" "evex")
> @@ -3314,7 +3316,7 @@ (define_insn "*<code><mode>3<mask_name>"
> "TARGET_SSE && <mask_avx512vl_condition>
> && !(MEM_P (operands[1]) && MEM_P (operands[2]))"
> {
> - static char buf[128];
> + char buf[128];
> const char *ops;
> const char *suffix;
>
> @@ -3349,7 +3351,8 @@ (define_insn "*<code><mode>3<mask_name>"
> }
>
> snprintf (buf, sizeof (buf), ops, suffix);
> - return buf;
> + output_asm_insn (buf, operands);
> + return "";
> }
> [(set_attr "isa" "noavx,avx,avx512dq,avx512f")
> (set_attr "type" "sselog")
> @@ -3378,7 +3381,7 @@ (define_insn "*<code><mode>3<mask_name>"
> (match_operand:VF_512 2 "nonimmediate_operand" "vm")))]
> "TARGET_AVX512F && !(MEM_P (operands[1]) && MEM_P (operands[2]))"
> {
> - static char buf[128];
> + char buf[128];
> const char *ops;
> const char *suffix;
>
> @@ -3395,7 +3398,8 @@ (define_insn "*<code><mode>3<mask_name>"
> snprintf (buf, sizeof (buf),
> "v%s<logic>%s\t{%%2, %%1, %%0<mask_operand3_1>|%%0<mask_operand3_1>,
> %%1, %%2}",
> ops, suffix);
> - return buf;
> + output_asm_insn (buf, operands);
> + return "";
> }
> [(set_attr "type" "sselog")
> (set_attr "prefix" "evex")
> @@ -3449,7 +3453,7 @@ (define_insn "*andnot<mode>3"
> (match_operand:MODEF 2 "register_operand" "x,x,v,v")))]
> "SSE_FLOAT_MODE_P (<MODE>mode)"
> {
> - static char buf[128];
> + char buf[128];
> const char *ops;
> const char *suffix
> = (get_attr_mode (insn) == MODE_V4SF) ? "ps" : "<ssevecmodesuffix>";
> @@ -3485,7 +3489,8 @@ (define_insn "*andnot<mode>3"
> }
>
> snprintf (buf, sizeof (buf), ops, suffix);
> - return buf;
> + output_asm_insn (buf, operands);
> + return "";
> }
> [(set_attr "isa" "noavx,avx,avx512vl,avx512f")
> (set_attr "type" "sselog")
> @@ -3516,7 +3521,7 @@ (define_insn "*andnottf3"
> (match_operand:TF 2 "vector_operand" "xBm,xm,vm,v")))]
> "TARGET_SSE"
> {
> - static char buf[128];
> + char buf[128];
> const char *ops;
> const char *tmp
> = (which_alternative >= 2 ? "pandnq"
> @@ -3539,7 +3544,8 @@ (define_insn "*andnottf3"
> }
>
> snprintf (buf, sizeof (buf), ops, tmp);
> - return buf;
> + output_asm_insn (buf, operands);
> + return "";
> }
> [(set_attr "isa" "noavx,avx,avx512vl,avx512f")
> (set_attr "type" "sselog")
> @@ -3572,7 +3578,7 @@ (define_insn "*<code><mode>3"
> (match_operand:MODEF 2 "register_operand" "x,x,v,v")))]
> "SSE_FLOAT_MODE_P (<MODE>mode)"
> {
> - static char buf[128];
> + char buf[128];
> const char *ops;
> const char *suffix
> = (get_attr_mode (insn) == MODE_V4SF) ? "ps" : "<ssevecmodesuffix>";
> @@ -3607,7 +3613,8 @@ (define_insn "*<code><mode>3"
> }
>
> snprintf (buf, sizeof (buf), ops, suffix);
> - return buf;
> + output_asm_insn (buf, operands);
> + return "";
> }
> [(set_attr "isa" "noavx,avx,avx512vl,avx512f")
> (set_attr "type" "sselog")
> @@ -3646,7 +3653,7 @@ (define_insn "*<code>tf3"
> (match_operand:TF 2 "vector_operand" "xBm,xm,vm,v")))]
> "TARGET_SSE && !(MEM_P (operands[1]) && MEM_P (operands[2]))"
> {
> - static char buf[128];
> + char buf[128];
> const char *ops;
> const char *tmp
> = (which_alternative >= 2 ? "p<logic>q"
> @@ -3669,7 +3676,8 @@ (define_insn "*<code>tf3"
> }
>
> snprintf (buf, sizeof (buf), ops, tmp);
> - return buf;
> + output_asm_insn (buf, operands);
> + return "";
> }
> [(set_attr "isa" "noavx,avx,avx512vl,avx512f")
> (set_attr "type" "sselog")
> @@ -12066,7 +12074,7 @@ (define_insn "*andnot<mode>3"
> (match_operand:VI 2 "vector_operand" "xBm,xm,vm")))]
> "TARGET_SSE"
> {
> - static char buf[64];
> + char buf[64];
> const char *ops;
> const char *tmp;
> const char *ssesuffix;
> @@ -12136,7 +12144,8 @@ (define_insn "*andnot<mode>3"
> }
>
> snprintf (buf, sizeof (buf), ops, tmp, ssesuffix);
> - return buf;
> + output_asm_insn (buf, operands);
> + return "";
> }
> [(set_attr "isa" "noavx,avx,avx")
> (set_attr "type" "sselog")
> @@ -12211,7 +12220,7 @@ (define_insn "<mask_codefor><code><mode>
> "TARGET_SSE && <mask_mode512bit_condition>
> && !(MEM_P (operands[1]) && MEM_P (operands[2]))"
> {
> - static char buf[64];
> + char buf[64];
> const char *ops;
> const char *tmp;
> const char *ssesuffix;
> @@ -12276,7 +12285,8 @@ (define_insn "<mask_codefor><code><mode>
> }
>
> snprintf (buf, sizeof (buf), ops, tmp, ssesuffix);
> - return buf;
> + output_asm_insn (buf, operands);
> + return "";
> }
> [(set_attr "isa" "noavx,avx,avx")
> (set_attr "type" "sselog")
> @@ -12311,7 +12321,7 @@ (define_insn "*<code><mode>3"
> (match_operand:VI12_AVX_AVX512F 2 "vector_operand" "xBm,xm,vm")))]
> "TARGET_SSE && !(MEM_P (operands[1]) && MEM_P (operands[2]))"
> {
> - static char buf[64];
> + char buf[64];
> const char *ops;
> const char *tmp;
> const char *ssesuffix;
> @@ -12371,7 +12381,8 @@ (define_insn "*<code><mode>3"
> }
>
> snprintf (buf, sizeof (buf), ops, tmp, ssesuffix);
> - return buf;
> + output_asm_insn (buf, operands);
> + return "";
> }
> [(set_attr "isa" "noavx,avx,avx")
> (set_attr "type" "sselog")
>
> Jakub
>
