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

[Richard Earnshaw <rearnsha at arm dot com>]

On Sat, 2011-04-16 at 12:34 +0800, Carrot Wei wrote:
> 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.

There are also some comparison operations that are also known to be 2
bytes (because they are known to set the condition codes).  But yes, the
scope here is quite limited.

R.

> 
> 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.
> >
> >
> >
>