| Message ID | 4C402E89.5060403@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
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.
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
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.
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.
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
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.
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