This is the mail archive of the 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 <> 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
>insn, see:
>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 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);
>    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?



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