On 10/02/2020 09:27, Christophe Lyon wrote:
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?
It's hard to say that. The number of instructions you generate for this
is significant, so it likely will throw off the calculations and
somebody will get unlucky. On the other hand, I don't think we should
pessimize code for the non-pure-code variants by inflating the size for
this unconditionally.
It seems there are two ways to fix this.
1) create a new alternative for the pure-code variant with its own
length attribute, then only enable that for the case you need. That
would also allow you to go back to the templated asm format.
2) use a level of indirection to calculate the length - I haven't tried
this, but it would be something like:
- create a new attribute - lets say default_length
- rename length for this pattern to default_length
- create another new attribute - lets say purecode_length, add lengths
for each alternative but adjusted for the purecode variant.
- make the length attribute for this pattern select based on the
disable_literal_pool variable between the default_length and
purecode_length attributes.
R.
Thanks,
Christophe
R.
Thanks,
Christophe
R.
