On Fri, 7 Feb 2020 at 17:55, Richard Earnshaw (lists)
<richard.earns...@arm.com> wrote:
>
> On 07/02/2020 16:43, Christophe Lyon wrote:
> > On Fri, 7 Feb 2020 at 14:49, Richard Earnshaw (lists)
> > <richard.earns...@arm.com> wrote:
> >>
> >> On 07/02/2020 13:19, Christophe Lyon wrote:
> >>> When running the testsuite with -fdisable-rtl-fwprop2 and -mpure-code
> >>> for cortex-m0, I noticed that some testcases were failing because we
> >>> still generate "ldr rX, .LCY", which is what we want to avoid with
> >>> -mpure-code. This is latent since a recent improvement in fwprop
> >>> (PR88833).
> >>>
> >>> In this patch I change the thumb1_movsi_insn pattern so that it emits
> >>> the desired instruction sequence when arm_disable_literal_pool is set.
> >>>
> >>> I tried to add a define_split instead, but couldn't make it work: the
> >>> compiler then complains it cannot split the instruction, while my new
> >>> define_split accepts the same operand types as thumb1_movsi_insn:
> >>>
> >>> c-c++-common/torture/complex-sign-mixed-add.c:41:1: error: could not
> >>> split insn
> >>> (insn 2989 425 4844 (set (reg/f:SI 3 r3 [1342])
> >>> (symbol_ref/u:SI ("*.LC6") [flags 0x2])) 836
> >>> {*thumb1_movsi_insn}
> >>> (expr_list:REG_EQUIV (symbol_ref/u:SI ("*.LC6") [flags 0x2])
> >>> (nil)))
> >>> during RTL pass: final
> >>>
> >>> (define_split
> >>> [(set (match_operand:SI 0 "register_operand" "")
> >>> (match_operand:SI 1 "general_operand" ""))]
> >>> "TARGET_THUMB1
> >>> && arm_disable_literal_pool
> >>> && GET_CODE (operands[1]) == SYMBOL_REF"
> >>> [(clobber (const_int 0))]
> >>> "
> >>> gen_thumb1_movsi_symbol_ref(operands[0], operands[1]);
> >>> DONE;
> >>> "
> >>> )
> >>> and I put this in thumb1_movsi_insn:
> >>> if (GET_CODE (operands[1]) == SYMBOL_REF && arm_disable_literal_pool)
> >>> {
> >>> return \"#\";
> >>> }
> >>> return \"ldr\\t%0, %1\";
> >>>
> >>> 2020-02-07 Christophe Lyon <christophe.l...@linaro.org>
> >>>
> >>> * config/arm/thumb1.md (thumb1_movsi_insn): Fix ldr alternative
> >>> to
> >>> work with -mpure-code.
> >>>
> >>
> >> + case 0:
> >> + case 1:
> >> + return \"movs %0, %1\";
> >> + case 2:
> >> + return \"movw %0, %1\";
> >>
> >> This is OK, but please replace the hard tab in the strings for MOVS/MOVW
> >> with \\t.
> >>
> >
> > OK that was merely a cut & paste from the existing code.
> >
> > I'm concerned that the length attribute is becoming wrong with my
> > patch, isn't this a problem?
> >
>
> Potentially yes. The branch range code needs this to handle overly long
> jumps correctly.
>
Do you mean that the probability of problems due to that shortcoming
is low enough that I can commit my patch?
Thanks,
Christophe
> R.
>
> > Thanks,
> >
> > Christophe
> >
> >> R.
>
