This is the mail archive of the
mailing list for the GCC project.
Re: [patch][RFC] make lra.c:check_rtl set maybe_hot_insn_p
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: Steven Bosscher <stevenb dot gcc at gmail dot com>,GCC Patches <gcc-patches at gcc dot gnu dot org>
- Cc: Vladimir Makarov <vmakarov at redhat dot com>,Jeff Law <law at redhat dot com>,Richard Earnshaw <rearnsha at arm dot com>,Yvan Roux <yvan dot roux at linaro dot org>,Jan Hubicka <hubicka at ucw dot cz>
- Date: Sun, 10 Nov 2013 14:17:45 +0100
- Subject: Re: [patch][RFC] make lra.c:check_rtl set maybe_hot_insn_p
- Authentication-results: sourceware.org; auth=none
- References: <CABu31nPu+1QxCQDv71C=3V_qS61B3GkAd37Eh7WTZMqBTann_w at mail dot gmail dot com>
Steven Bosscher <email@example.com> wrote:
>This patch is necessary to make ARM pass the test suite with LRA
>enabled. The symptom is recog failing to recognize a store_minmaxsi
>But I am not sure if that's also the root cause of the problem, or
>whether the ARM back end should not let recognition of insn patterns
>be dependent on the state of the profile flags.
>The pattern for store_minmaxsi (in arm.md) is:
> [(set (match_operand:SI 0 "memory_operand" "=m")
> (match_operator:SI 3 "minmax_operator"
> [(match_operand:SI 1 "s_register_operand" "r")
> (match_operand:SI 2 "s_register_operand" "r")]))
> (clobber (reg:CC CC_REGNUM))]
> "TARGET_32BIT && optimize_insn_for_size_p()"
> operands = gen_rtx_fmt_ee (minmax_code (operands), SImode,
> operands, operands);
> output_asm_insn (\"cmp\\t%1, %2\", operands);
> if (TARGET_THUMB2)
> output_asm_insn (\"ite\t%d3\", operands);
> output_asm_insn (\"str%d3\\t%1, %0\", operands);
> output_asm_insn (\"str%D3\\t%2, %0\", operands);
> return \"\";
> [(set_attr "conds" "clob")
> (set (attr "length")
> (if_then_else (eq_attr "is_thumb" "yes")
> (const_int 14)
> (const_int 12)))
> (set_attr "type" "store1")]
>Note the insn condition uses optimize_insn_for_size_p(). This means
>the pattern can be valid or invalid dependent on the context where the
>insn appears: in hot or cold code. IMHO this behavior should not be
>allowed. The back end cannot expect the middle end to know at all
>times the context that the insn appears in, and more importantly
>whether a pattern is valid or not is independent of where the insn
>appears: That is a *cost* issue!
>It seems to me that the ARM back end should be fixed here, not LRA's
The intent is to allow this also to control combine and split, but certainly making insns invalid after the fact is bad. think of sinking a previously hot insn into a cold path...
Honza, how was this supposed to work?