Re: [PATCH, ARM] PR47855 Compute attr "length" for some thumb2 insns, 2/3

[Carrot Wei <carrot@google.com>]

Hi Richard

Thank you for the detailed explanation. It sounds like an inherent
difficulty of rtl passes. Then the only opportunity is ldrb/strb
instructions because they never affect cc registers.

thanks
Carrot

On Fri, Apr 15, 2011 at 9:34 PM, Richard Earnshaw <rearnsha@arm.com> wrote:
>
> On Thu, 2011-04-14 at 21:19 +0800, Carrot Wei wrote:
>> On Fri, Apr 8, 2011 at 6:51 PM, Ramana Radhakrishnan
>> <ramana.radhakrishnan@linaro.org> wrote:
>> > On 08/04/11 10:57, Carrot Wei wrote:
>> >>
>> >> Hi
>> >>
>> >> This is the second part of the fixing for
>> >>
>> >> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47855
>> >>
>> >> This patch contains the length computation for insn patterns
>> >> "*arm_movqi_insn"
>> >> and "*arm_addsi3". Since the alternatives and encodings are much more
>> >> complex,
>> >> the attribute length is computed in separate C functions.
>>
>> > Sorry, no. This is potentially a maintenance pain. It hardcodes alternatives
>> > from a pattern elsewhere in the C file. I don't like doing this unless we
>> > have to with the sync primitives or with push_multi. In this case I'm not
>> > convinced we need such functions in the .c file.
>> >
>> > Why can't we use the "enabled" attribute here with appropriate constraints
>> > for everything other than the memory cases (even there we might be able to
>> > invent some new constraints) ?
>> >
>> > Also a note about programming style. There are the helper macros like REG_P,
>> > CONST_INT_P and MEM_P which remove the necessity for checks like
>> >
>> > GET_CODE (x) == y where y E { REG, CONST_INT, MEM}
>>
>> Hi Ramana
>>
>> As you suggested I created several new constraints, and use the
>> "enabled" attribute to split the current alternatives in this new
>> patch. It has been tested on arm qemu without regression.
>>
>> thanks
>> Carrot
>
>
> Sorry, I don't think this approach can work.  Certainly not with the way
> the compiler currently works, and especially for mov and add insns.
>
> These instructions are only 2 bytes long if either:
> 1) They clobber the condition code register or
> 2) They occur inside an IT block.
>
> We can't tell either of these from the pattern, so you're
> underestimating the length of the instruction in some circumstances by
> claiming that they are only 2 bytes long.  That /will/ lead to broken
> code someday.
>
> We can't add potential clobbers to mov and add patterns because that
> will break reload which relies on these patterns being simple-set insns
> with no added baggage.  It *might* be possible to add clobbers to other
> operations, but that will then most-likely upset instruction scheduling
> (I think the scheduler treats two insns that clobber the same hard reg
> as being strongly ordered).  Putting in the clobber too early will
> certainly affect cond-exec generation.
>
> In short, I'm not aware of a simple way to address this problem so that
> we get accurate length information, but minimal impact on other passes
> in the compiler.
>
> R.
>
>
>