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] [ARM] [RFC] Fix longstanding push_minipool_fix ICE (PR49423, lp1296601)


On 02/07/14 13:05, Charles Baylis wrote:
> On 30 June 2014 14:26, Richard Earnshaw <rearnsha@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.baylis@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.


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