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 >