On Wed, 1 Mar 2017, Jakub Jelinek wrote:
> On Wed, Mar 01, 2017 at 03:03:29PM +0100, Richard Biener wrote:
> > The patch adds better limiting to that interface and fixes one
> > false positive in fixed-value.c. Two other false positives are
> > fixed by the wide-int.h patch posted a few hours ago and a patch
> > to genemit from Jakub.
>
> Here is that patch. Right now insn-emit.c for match_scratch
> operands in expanders emits
> rtx operand7 ATTRIBUTE_UNUSED;
> ...
> rtx operands[8];
> operands[0] = operand0;
> ...
> // but no operands[7] = something here;
> ...
> // C code from *.md file (which typically doesn't touch operands[7])
> ...
> operand7 = operands[7];
> (void) operand7;
> ...
> // generated code that doesn't use operand7
> This triggers -Wuninitialized warning with Richard's patch and is really UB,
> even when we actually don't use it and so hopefully optimize away.
> The following patch just removes all operand7 and operands[7] references
> from the generated code (i.e. operands that are for match_scratch).
>
> Usually match_scratch numbers come after match_operand/match_dup etc.
> numbers, but as can be seen, there are few spots where that is not the case.
> The patch adds verification of this requirement in genemit and then fixes
> the issues it has diagnosed.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, plus tested with
> make s-emit in a cross to
> {powerpc64,aarch64,armv7hl,sparc64,s390x,cris,sh,ia64,hppa,mips}-linux, ok
> for trunk?
This is ok.
Thanks for fixing this,
Richard.
> 2017-03-01 Jakub Jelinek <ja...@redhat.com>
>
> PR tree-optimization/79345
> * gensupport.h (struct pattern_stats): Add min_scratch_opno field.
> * gensupport.c (get_pattern_stats_1) <case MATCH_SCRATCH>: Update it.
> (get_pattern_stats): Initialize it.
> * genemit.c (gen_expand): Verify match_scratch numbers come after
> match_operand/match_dup numbers.
> * config/i386/i386.md (<s>mul<mode>3_highpart): Swap match_dup and
> match_scratch numbers.
> * config/i386/sse.md (avx2_gathersi<mode>, avx2_gatherdi<mode>):
> Likewise.
> * config/s390/s390.md (trunctdsd2): Likewise.
>
> --- gcc/gensupport.h.jj 2017-01-01 12:45:38.000000000 +0100
> +++ gcc/gensupport.h 2017-03-01 12:06:21.816440102 +0100
> @@ -199,7 +199,8 @@ struct pattern_stats
> /* The largest match_dup, match_op_dup or match_par_dup number found. */
> int max_dup_opno;
>
> - /* The largest match_scratch number found. */
> + /* The smallest and largest match_scratch number found. */
> + int min_scratch_opno;
> int max_scratch_opno;
>
> /* The number of times match_dup, match_op_dup or match_par_dup appears
> --- gcc/gensupport.c.jj 2017-01-05 22:10:31.000000000 +0100
> +++ gcc/gensupport.c 2017-03-01 12:21:24.830327207 +0100
> @@ -3000,6 +3000,10 @@ get_pattern_stats_1 (struct pattern_stat
> break;
>
> case MATCH_SCRATCH:
> + if (stats->min_scratch_opno == -1)
> + stats->min_scratch_opno = XINT (x, 0);
> + else
> + stats->min_scratch_opno = MIN (stats->min_scratch_opno, XINT (x, 0));
> stats->max_scratch_opno = MAX (stats->max_scratch_opno, XINT (x, 0));
> break;
>
> @@ -3032,6 +3036,7 @@ get_pattern_stats (struct pattern_stats
>
> stats->max_opno = -1;
> stats->max_dup_opno = -1;
> + stats->min_scratch_opno = -1;
> stats->max_scratch_opno = -1;
> stats->num_dups = 0;
>
> --- gcc/genemit.c.jj 2017-01-01 12:45:35.000000000 +0100
> +++ gcc/genemit.c 2017-03-01 12:16:28.391343302 +0100
> @@ -448,6 +448,10 @@ gen_expand (md_rtx_info *info)
>
> /* Find out how many operands this function has. */
> get_pattern_stats (&stats, XVEC (expand, 1));
> + if (stats.min_scratch_opno != -1
> + && stats.min_scratch_opno <= MAX (stats.max_opno, stats.max_dup_opno))
> + fatal_at (info->loc, "define_expand for %s needs to have match_scratch "
> + "numbers above all other operands", XSTR (expand, 0));
>
> /* Output the function name and argument declarations. */
> printf ("rtx\ngen_%s (", XSTR (expand, 0));
> @@ -479,8 +483,6 @@ gen_expand (md_rtx_info *info)
> make a local variable. */
> for (i = stats.num_generator_args; i <= stats.max_dup_opno; i++)
> printf (" rtx operand%d;\n", i);
> - for (; i <= stats.max_scratch_opno; i++)
> - printf (" rtx operand%d ATTRIBUTE_UNUSED;\n", i);
> printf (" rtx_insn *_val = 0;\n");
> printf (" start_sequence ();\n");
>
> @@ -516,7 +518,7 @@ gen_expand (md_rtx_info *info)
> (unless we aren't going to use them at all). */
> if (XVEC (expand, 1) != 0)
> {
> - for (i = 0; i < stats.num_operand_vars; i++)
> + for (i = 0; i <= MAX (stats.max_opno, stats.max_dup_opno); i++)
> {
> printf (" operand%d = operands[%d];\n", i, i);
> printf (" (void) operand%d;\n", i);
> --- gcc/config/i386/i386.md.jj 2017-02-22 18:15:48.000000000 +0100
> +++ gcc/config/i386/i386.md 2017-03-01 12:23:22.882736837 +0100
> @@ -7364,11 +7364,11 @@ (define_expand "<s>mul<mode>3_highpart"
> (match_operand:SWI48 1 "nonimmediate_operand"))
> (any_extend:<DWI>
> (match_operand:SWI48 2 "register_operand")))
> - (match_dup 4))))
> - (clobber (match_scratch:SWI48 3))
> + (match_dup 3))))
> + (clobber (match_scratch:SWI48 4))
> (clobber (reg:CC FLAGS_REG))])]
> ""
> - "operands[4] = GEN_INT (GET_MODE_BITSIZE (<MODE>mode));")
> + "operands[3] = GEN_INT (GET_MODE_BITSIZE (<MODE>mode));")
>
> (define_insn "*<s>muldi3_highpart_1"
> [(set (match_operand:DI 0 "register_operand" "=d")
> --- gcc/config/i386/sse.md.jj 2017-02-07 16:41:32.000000000 +0100
> +++ gcc/config/i386/sse.md 2017-03-01 12:24:57.193471018 +0100
> @@ -18873,7 +18873,7 @@ (define_expand "avx2_gathersi<mode>"
> (unspec:VEC_GATHER_MODE
> [(match_operand:VEC_GATHER_MODE 1 "register_operand")
> (mem:<ssescalarmode>
> - (match_par_dup 7
> + (match_par_dup 6
> [(match_operand 2 "vsib_address_operand")
> (match_operand:<VEC_GATHER_IDXSI>
> 3 "register_operand")
> @@ -18881,10 +18881,10 @@ (define_expand "avx2_gathersi<mode>"
> (mem:BLK (scratch))
> (match_operand:VEC_GATHER_MODE 4 "register_operand")]
> UNSPEC_GATHER))
> - (clobber (match_scratch:VEC_GATHER_MODE 6))])]
> + (clobber (match_scratch:VEC_GATHER_MODE 7))])]
> "TARGET_AVX2"
> {
> - operands[7]
> + operands[6]
> = gen_rtx_UNSPEC (Pmode, gen_rtvec (3, operands[2], operands[3],
> operands[5]), UNSPEC_VSIBADDR);
> })
> @@ -18934,7 +18934,7 @@ (define_expand "avx2_gatherdi<mode>"
> (unspec:VEC_GATHER_MODE
> [(match_operand:<VEC_GATHER_SRCDI> 1 "register_operand")
> (mem:<ssescalarmode>
> - (match_par_dup 7
> + (match_par_dup 6
> [(match_operand 2 "vsib_address_operand")
> (match_operand:<VEC_GATHER_IDXDI>
> 3 "register_operand")
> @@ -18942,10 +18942,10 @@ (define_expand "avx2_gatherdi<mode>"
> (mem:BLK (scratch))
> (match_operand:<VEC_GATHER_SRCDI> 4 "register_operand")]
> UNSPEC_GATHER))
> - (clobber (match_scratch:VEC_GATHER_MODE 6))])]
> + (clobber (match_scratch:VEC_GATHER_MODE 7))])]
> "TARGET_AVX2"
> {
> - operands[7]
> + operands[6]
> = gen_rtx_UNSPEC (Pmode, gen_rtvec (3, operands[2], operands[3],
> operands[5]), UNSPEC_VSIBADDR);
> })
> --- gcc/config/s390/s390.md.jj 2017-02-06 13:32:14.000000000 +0100
> +++ gcc/config/s390/s390.md 2017-03-01 12:45:07.184184616 +0100
> @@ -5074,15 +5074,15 @@ (define_insn "truncddsd2"
>
> (define_expand "trunctdsd2"
> [(parallel
> - [(set (match_dup 3)
> + [(set (match_dup 2)
> (float_truncate:DD (match_operand:TD 1 "register_operand" "")))
> (unspec:DI [(const_int DFP_RND_PREP_FOR_SHORT_PREC)] UNSPEC_ROUND)
> - (clobber (match_scratch:TD 2 ""))])
> + (clobber (match_scratch:TD 3 ""))])
> (set (match_operand:SD 0 "register_operand" "")
> - (float_truncate:SD (match_dup 3)))]
> + (float_truncate:SD (match_dup 2)))]
> "TARGET_HARD_DFP"
> {
> - operands[3] = gen_reg_rtx (DDmode);
> + operands[2] = gen_reg_rtx (DDmode);
> })
>
> ;
>
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
