On 02/07/14 13:05, Charles Baylis wrote:
> On 30 June 2014 14:26, Richard Earnshaw <rearn...@arm.com> wrote:
>> On 30/06/14 13:53, Charles Baylis wrote:
>>> I see two options to fix it - one is to teach the back-end to
>>> successfully generate code for this insn, and the other is to teach
>>> the back-end that such an insn is not valid. My proposed patch does
>>> the former. The latter can presumably be achieved by providing a
>>> different kind of memory constraint which disallows constant pool
>>> references for these insns although I haven't tried this yet.
>>
>> I think we should be doing the latter (not permitting these operations).
>> If we wanted to do the former, we could just add an offset range for
>> the insn.
>>
>> The reason we don't want the former is that the offset ranges are too
>> small and overly constrain literal pool placement.
>
> The attached patch adds a 'Uh' constraint, which means the same as
> 'm', except that literal pool references are not allowed. Patterns
> which generate ldr[s]b or ldr[s]h have been updated to use it, and the
> pool_range attributes have been removed from those patterns.
>
> Bootstrapped and make-checked with no regressions on qemu for
> arm-unknown-linux-gnueabihf.
>
>
> <date> Charles Baylis <charles.bay...@linaro.org>
>
> PR target/49423
> * config/arm/arm-protos.h (arm_legitimate_address_p,
> arm_is_constant_pool_ref): Add prototypes.
> * config/arm/arm.c (arm_legitimate_address_p): Remove static.
> (arm_is_constant_pool_ref) New function.
> * config/arm/arm.md (unaligned_loadhis, arm_zero_extendhisi2_v6,
> arm_zero_extendqisi2_v6): Use Uh constraint for memory operand.
> (arm_extendhisi2, arm_extendhisi2_v6): Use Uh constraint for memory
> operand and remove pool_range and neg_pool_range attributes.
^^^^^
Better to start a new sentence here.
> (arm_extendqihi_insn, arm_extendqisi, arm_extendqisi_v6): Remove
> pool_range and neg_pool_range attributes.
> * config/arm/constraints.md (Uh): New constraint. (Uq): Don't allow
> constant pool references.
"(Uq)" should be on a new line.
>
>
> OK for trunk?
>
I certainly think this is the right approach.
My only worry is whether we also need new predicates that have similar
restrictions (see, for example how Uq is matched with
arm_extendqisi_mem_op). On balance, I think we're OK without that
change, since I don't expect that we'll see MEM (CONST_POOL_ADDR) unless
reload has already decided to re-materialize a constant register.
So OK, but if you're considering back-ports, I suggest you let it bake a
while on trunk first.
R.
