This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch][RFC] make lra.c:check_rtl set maybe_hot_insn_p


Steven Bosscher <stevenb.gcc@gmail.com> wrote:
>Hello,
>
>This patch is necessary to make ARM pass the test suite with LRA
>enabled. The symptom is recog failing to recognize a store_minmaxsi
>insn, see:
> http://gcc.gnu.org/ml/gcc-patches/2013-11/msg00725.html
>
>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:
>
>(define_insn "*store_minmaxsi"
>  [(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[3] = gen_rtx_fmt_ee (minmax_code (operands[3]), SImode,
>                                operands[1], operands[2]);
>  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
>check_rtl.
>
>Comments&thoughts?

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?

Richard.

>Ciao!
>Steven



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]