Re: [PATCH, ARM] PR47855 Compute attr length for some thumb2 insns, 2/3
On Thu, 2011-04-14 at 21:19 +0800, Carrot Wei wrote:
On Fri, Apr 8, 2011 at 6:51 PM, Ramana Radhakrishnan
ramana.radhakrish...@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.
Re: [PATCH, ARM] PR47855 Compute attr length for some thumb2 insns, 2/3
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 rearn...@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.radhakrish...@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.
