[patch] Auto-set ARM -mimplicit-it option

[patch] Auto-set ARM -mimplicit-it option

[Andrew Stubbs]

[-- Attachment #1: Type: text/plain, Size: 337 bytes --]

This patch causes the compiler to pass -mimplicit-it=thumb to the 
assembler, when the running in -mthumb mode.

The default, if the compiler passes nothing, is -mimplicit-it=arm, which 
is inappropriate when a program is targeted at thumb mode.

I've built and tested an ARM Linux cross compiler, and found no regressions.

OK?

Andrew

[-- Attachment #2: arm-default-implicit-it.patch --]
[-- Type: text/x-diff, Size: 681 bytes --]

2010-07-12  Andrew Stubbs  <ams@codesourcery.com>

	* config/arm/elf.h (ASM_SPEC): Pass -mimplicit-it=thumb if -mthumb.

---
 src/gcc-mainline/gcc/config/arm/elf.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/src/gcc-mainline/gcc/config/arm/elf.h b/src/gcc-mainline/gcc/config/arm/elf.h
index 8840088..c8a3572 100644
--- a/src/gcc-mainline/gcc/config/arm/elf.h
+++ b/src/gcc-mainline/gcc/config/arm/elf.h
@@ -63,6 +63,7 @@
 %{mthumb-interwork:-mthumb-interwork} \
 %{msoft-float:-mfloat-abi=soft} %{mhard-float:-mfloat-abi=hard} \
 %{mfloat-abi=*} %{mfpu=*} \
+%{mthumb:%{!-mimplicit-it=*:-mimplicit-it=thumb}} \
 %(subtarget_extra_asm_spec)"
 #endif
 

Re: [patch] Auto-set ARM -mimplicit-it option

[Richard Earnshaw]

On Fri, 2010-07-16 at 11:03 +0100, Andrew Stubbs wrote:
> This patch causes the compiler to pass -mimplicit-it=thumb to the 
> assembler, when the running in -mthumb mode.
> 
> The default, if the compiler passes nothing, is -mimplicit-it=arm, which 
> is inappropriate when a program is targeted at thumb mode.
> 
> I've built and tested an ARM Linux cross compiler, and found no regressions.
> 
> OK?
> 
> Andrew

Are you sure?  The gas docs say:

        The -mimplicit-it option controls the behavior of the assembler
        when conditional instructions are not enclosed in IT blocks.
        There are four possible behaviors. If never is specified, such
        constructs cause a warning in ARM code and an error in Thumb-2
        code. If always is specified, such constructs are accepted in
        both ARM and Thumb-2 code, where the IT instruction is added
        implicitly. If arm is specified, such constructs are accepted in
        ARM code and cause an error in Thumb-2 code. If thumb is
        specified, such constructs cause a warning in ARM code and are
        accepted in Thumb-2 code. If you omit this option, the behavior
        is equivalent to -mimplicit-it=arm.
        
Based on this, I think:

        If arm is specified, such constructs [ie. implicit insertion of
        IT instructions] are accepted in ARM code and cause an error in
        Thumb-2 code.
        
Is the behaviour that we *do* want.  Ie, accept implicit IT blocks in
ARM code, but cause an error if they're implicit in Thumb code.  The
compiler should never generate conditional Thumb-2 instructions without
emitting an IT instruction first.

I must admit that the docs are not particularly clearly written, but
based on what I think they say, I think the default is the correct
choice for the compiler.

R.

Re: [patch] Auto-set ARM -mimplicit-it option

[Andrew Stubbs]

On 16/07/10 13:46, Richard Earnshaw wrote:
> Are you sure?  The gas docs say:
>
>          The -mimplicit-it option controls the behavior of the assembler
>          when conditional instructions are not enclosed in IT blocks.
>          There are four possible behaviors. If never is specified, such
>          constructs cause a warning in ARM code and an error in Thumb-2
>          code. If always is specified, such constructs are accepted in
>          both ARM and Thumb-2 code, where the IT instruction is added
>          implicitly. If arm is specified, such constructs are accepted in
>          ARM code and cause an error in Thumb-2 code. If thumb is
>          specified, such constructs cause a warning in ARM code and are
>          accepted in Thumb-2 code. If you omit this option, the behavior
>          is equivalent to -mimplicit-it=arm.

Right, "never" and "always" mods are irrelevant here - I'm not proposing 
that the compiler ever pass either automatically.

So, the choice is between -mimplicit-it=arm (the gas default) or 
-mimplicit-it=thumb. I'm proposing the latter, but only in the case that 
the compiler is running in thumb mode.

> Based on this, I think:
>
>          If arm is specified, such constructs [ie. implicit insertion of
>          IT instructions] are accepted in ARM code and cause an error in
>          Thumb-2 code.
>
> Is the behaviour that we *do* want.  Ie, accept implicit IT blocks in
> ARM code, but cause an error if they're implicit in Thumb code.  The
> compiler should never generate conditional Thumb-2 instructions without
> emitting an IT instruction first.

The problem is not generated code. I believe the problem is inline 
assembler.

> I must admit that the docs are not particularly clearly written, but
> based on what I think they say, I think the default is the correct
> choice for the compiler.

The default is the right choice in ARM mode, but the complaint is that 
in thumb mode you suddenly need to be explicit.

I'm not sure exactly where this issue comes from, but ARM Ubuntu (which 
is almost exclusively thumb, I think) has had a patch like this for some 
time, and found it solves some problems.

Whatever, it seems to me quite reasonable that if the assembler has 
smarts in one mode, then probably it should do in the other, but then, I 
don't claim to be an ARM expert.

Andrew

Re: [patch] Auto-set ARM -mimplicit-it option

[Richard Earnshaw]

On Fri, 2010-07-16 at 14:04 +0100, Andrew Stubbs wrote:
> On 16/07/10 13:46, Richard Earnshaw wrote:
> > Are you sure?  The gas docs say:
> >
> >          The -mimplicit-it option controls the behavior of the assembler
> >          when conditional instructions are not enclosed in IT blocks.
> >          There are four possible behaviors. If never is specified, such
> >          constructs cause a warning in ARM code and an error in Thumb-2
> >          code. If always is specified, such constructs are accepted in
> >          both ARM and Thumb-2 code, where the IT instruction is added
> >          implicitly. If arm is specified, such constructs are accepted in
> >          ARM code and cause an error in Thumb-2 code. If thumb is
> >          specified, such constructs cause a warning in ARM code and are
> >          accepted in Thumb-2 code. If you omit this option, the behavior
> >          is equivalent to -mimplicit-it=arm.
> 
> Right, "never" and "always" mods are irrelevant here - I'm not proposing 
> that the compiler ever pass either automatically.
> 
> So, the choice is between -mimplicit-it=arm (the gas default) or 
> -mimplicit-it=thumb. I'm proposing the latter, but only in the case that 
> the compiler is running in thumb mode.
> 
> > Based on this, I think:
> >
> >          If arm is specified, such constructs [ie. implicit insertion of
> >          IT instructions] are accepted in ARM code and cause an error in
> >          Thumb-2 code.
> >
> > Is the behaviour that we *do* want.  Ie, accept implicit IT blocks in
> > ARM code, but cause an error if they're implicit in Thumb code.  The
> > compiler should never generate conditional Thumb-2 instructions without
> > emitting an IT instruction first.
> 
> The problem is not generated code. I believe the problem is inline 
> assembler.
> 
> > I must admit that the docs are not particularly clearly written, but
> > based on what I think they say, I think the default is the correct
> > choice for the compiler.
> 
> The default is the right choice in ARM mode, but the complaint is that 
> in thumb mode you suddenly need to be explicit.
> 
> I'm not sure exactly where this issue comes from, but ARM Ubuntu (which 
> is almost exclusively thumb, I think) has had a patch like this for some 
> time, and found it solves some problems.
> 
> Whatever, it seems to me quite reasonable that if the assembler has 
> smarts in one mode, then probably it should do in the other, but then, I 
> don't claim to be an ARM expert.
> 
> Andrew

I think implicit generation of instructions in compiler generated code
is just wrong, even in inline assembler.  If the assembler can't work
out how much code an assembler block will generate (which it can only do
by counting statements in the block) then it can't do literal pool
placement accurately; in some cases it might even get it wrong and put a
literal pool out of range.

Inline assembly code needs to be fixed.  Papering over this problem
isn't going to help get that done.

R.

Re: [patch] Auto-set ARM -mimplicit-it option

[Richard Earnshaw]

On Fri, 2010-07-16 at 14:11 +0100, Richard Earnshaw wrote:
> On Fri, 2010-07-16 at 14:04 +0100, Andrew Stubbs wrote:
> > On 16/07/10 13:46, Richard Earnshaw wrote:
> > > Are you sure?  The gas docs say:
> > >
> > >          The -mimplicit-it option controls the behavior of the assembler
> > >          when conditional instructions are not enclosed in IT blocks.
> > >          There are four possible behaviors. If never is specified, such
> > >          constructs cause a warning in ARM code and an error in Thumb-2
> > >          code. If always is specified, such constructs are accepted in
> > >          both ARM and Thumb-2 code, where the IT instruction is added
> > >          implicitly. If arm is specified, such constructs are accepted in
> > >          ARM code and cause an error in Thumb-2 code. If thumb is
> > >          specified, such constructs cause a warning in ARM code and are
> > >          accepted in Thumb-2 code. If you omit this option, the behavior
> > >          is equivalent to -mimplicit-it=arm.
> > 
> > Right, "never" and "always" mods are irrelevant here - I'm not proposing 
> > that the compiler ever pass either automatically.
> > 
> > So, the choice is between -mimplicit-it=arm (the gas default) or 
> > -mimplicit-it=thumb. I'm proposing the latter, but only in the case that 
> > the compiler is running in thumb mode.
> > 
> > > Based on this, I think:
> > >
> > >          If arm is specified, such constructs [ie. implicit insertion of
> > >          IT instructions] are accepted in ARM code and cause an error in
> > >          Thumb-2 code.
> > >
> > > Is the behaviour that we *do* want.  Ie, accept implicit IT blocks in
> > > ARM code, but cause an error if they're implicit in Thumb code.  The
> > > compiler should never generate conditional Thumb-2 instructions without
> > > emitting an IT instruction first.
> > 
> > The problem is not generated code. I believe the problem is inline 
> > assembler.
> > 
> > > I must admit that the docs are not particularly clearly written, but
> > > based on what I think they say, I think the default is the correct
> > > choice for the compiler.
> > 
> > The default is the right choice in ARM mode, but the complaint is that 
> > in thumb mode you suddenly need to be explicit.
> > 
> > I'm not sure exactly where this issue comes from, but ARM Ubuntu (which 
> > is almost exclusively thumb, I think) has had a patch like this for some 
> > time, and found it solves some problems.
> > 
> > Whatever, it seems to me quite reasonable that if the assembler has 
> > smarts in one mode, then probably it should do in the other, but then, I 
> > don't claim to be an ARM expert.
> > 
> > Andrew
> 
> I think implicit generation of instructions in compiler generated code
> is just wrong, even in inline assembler.  If the assembler can't work
                                                   ^^^^^^^^^
                                                   compiler

Of course the assembler knows, but the compiler doesn't.

> out how much code an assembler block will generate (which it can only do
> by counting statements in the block) then it can't do literal pool
> placement accurately; in some cases it might even get it wrong and put a
> literal pool out of range.
> 
> Inline assembly code needs to be fixed.  Papering over this problem
> isn't going to help get that done.
> 
> R.

Re: [patch] Auto-set ARM -mimplicit-it option

[Andrew Stubbs]

On 16/07/10 14:11, Richard Earnshaw wrote:
> Inline assembly code needs to be fixed.  Papering over this problem
> isn't going to help get that done.

OK, thanks for the review, Richard.

Can you tell me why it's OK to do this for ARM mode?

Andrew

Re: [patch] Auto-set ARM -mimplicit-it option

[Richard Earnshaw]

On Fri, 2010-07-16 at 14:57 +0100, Andrew Stubbs wrote:
> On 16/07/10 14:11, Richard Earnshaw wrote:
> > Inline assembly code needs to be fixed.  Papering over this problem
> > isn't going to help get that done.
> 
> OK, thanks for the review, Richard.
> 
> Can you tell me why it's OK to do this for ARM mode?
> 
> Andrew

Because there isn't an IT instruction in ARM mode, it's a phantom that
doesn't expand to any object code (it's just permitted to permit writing
assembly code that can be assembled either as ARM or Thumb-2 code).

R.