| Message ID | 350403c10d5a64a57fb5ef9ab7efb0ab@agner.ch |
|---|---|
| State | Not Applicable |
| Headers | show |
Hello Stefan, On Tue, 18 Nov 2014 19:37:18 +0100, Stefan Agner <stefan@agner.ch> wrote: > diff --git a/arch/arm/config.mk b/arch/arm/config.mk > index f0eafd6..ddbc8dc 100644 > --- a/arch/arm/config.mk > +++ b/arch/arm/config.mk > @@ -30,6 +30,8 @@ PF_CPPFLAGS_ARM := $(call cc-option, -mthumb > -mthumb-interwork,\ > $(call cc-option,-marm,)\ > $(call cc-option,-mno-thumb-interwork,)\ > ) > +AFLAGS_AUTOIT :=$(call > as-option,-Wa$(comma)-mimplicit-it=always,-Wa$(comma)-mauto-it) > +PF_CPPFLAGS_ARM += $(AFLAGS_AUTOIT) > else > PF_CPPFLAGS_ARM := $(call cc-option,-marm,) \ > $(call cc-option,-mno-thumb-interwork,) > The main change here is the implicit-it/auto-it functionality. For me it > works when enabling that globally. Is it harmful to enable that > globally? The other changes need a proper ifdef, but should be ok I > guess. -mimplicit-it=always will have no effect on ARM builds as it defaults to 'arm', meaning that IT instructions are already optional in ARM mode, and switching to 'always' changes the behavior only for Thumb mode. For -mauto-it, it is not documented in the gas documentation online or in my current as' --target-help. I'll dig this deeper today, but barring any scream from me, the change above is fine globally in U-Boot. > -- > Stefan Amicalement,
Dear Albert, In message <20141119074214.3d414ce6@lilith> you wrote: > > For -mauto-it, it is not documented in the gas documentation online or > in my current as' --target-help. I'll dig this deeper today, but barring > any scream from me, the change above is fine globally in U-Boot. Apparently this [1] is where it is coming from; no further documentation there, though. [1] https://sourceware.org/ml/binutils/2009-05/msg00132.html Best regards, Wolfgang Denk
On 19 Nov 2014, wd@denx.de wrote: > Dear Albert, > > In message <20141119074214.3d414ce6@lilith> you wrote: >> >> For -mauto-it, it is not documented in the gas documentation online >> or in my current as' --target-help. I'll dig this deeper today, but >> barring any scream from me, the change above is fine globally in >> U-Boot. > Apparently this [1] is where it is coming from; no further > documentation there, though. > [1] https://sourceware.org/ml/binutils/2009-05/msg00132.html I would think that if this worked they would make it automatic and not an option. Probably this is only in certain binutils/as. With 4.6.3 and 4.9.1 I do not have this option, [foo.S] .syntax unified .thumb foo: cmp r0, #5 movne r1,#6 moveq r1,#2 bx lr bar: cmp r0, #10 movhi r1,#3 movls r1,#7 moveq r1,#11 bx lr $ arm-none-linux-gnueabi.gcc4.6.3/bin/arm-none-linux-gnueabi-as -mcpu=cortex-a5 -mimplicit-it=always foo.S -o foo.o $ arm-none-linux-gnueabi-vybrid-4.9.1/bin/arm-none-linux-gnueabi-as -mcpu=cortex-a5 -mimplicit-it=always foo.S -o foo.o Both give 'objdump -S foo.o', foo.o: file format elf32-littlearm Disassembly of section .text: 00000000 <foo>: 0: 2805 cmp r0, #5 2: bf14 ite ne 4: 2106 movne r1, #6 6: 2102 moveq r1, #2 8: 4770 bx lr 0000000a <bar>: a: 280a cmp r0, #10 c: bf8c ite hi e: 2103 movhi r1, #3 10: 2107 movls r1, #7 12: bf08 it eq 14: 210b moveq r1, #11 16: 4770 bx lr I think before the patch there would be 'it' values before each and every condition. In fact, if you change 'bar' to, bar: cmp r0, #10 movhi r1,#3 movlo r1,#7 moveq r1,#11 bx lr You get three 'IT' conditions as 'HI' and 'LO' are not opposite. The patch seem to detect things that are the exact opposite. The 'bar' above ends up with '11' in r1 if the value is zero, but it temporarily '7'. The 2nd bar will only place 11 in r1. Fwiw, Bill Pringlemeir. Ref: https://wiki.ubuntu.com/ARM/Thumb2PortingHowto#Conditional_Execution
Hello Bill, On Wed, 19 Nov 2014 11:31:05 -0500, Bill Pringlemeir <bpringlemeir@nbsps.com> wrote: > On 19 Nov 2014, wd@denx.de wrote: > > > Dear Albert, > > > > In message <20141119074214.3d414ce6@lilith> you wrote: > >> > >> For -mauto-it, it is not documented in the gas documentation online > >> or in my current as' --target-help. I'll dig this deeper today, but > >> barring any scream from me, the change above is fine globally in > >> U-Boot. > > > Apparently this [1] is where it is coming from; no further > > documentation there, though. > > > [1] https://sourceware.org/ml/binutils/2009-05/msg00132.html > > I would think that if this worked they would make it automatic and not > an option. Probably this is only in certain binutils/as. > > With 4.6.3 and 4.9.1 I do not have this option, Which option do you mean? -mimplicit-it or -mauto-it? > [foo.S] > .syntax unified > .thumb > foo: > cmp r0, #5 > movne r1,#6 > moveq r1,#2 > bx lr > bar: > cmp r0, #10 > movhi r1,#3 > movls r1,#7 > moveq r1,#11 > bx lr > > $ arm-none-linux-gnueabi.gcc4.6.3/bin/arm-none-linux-gnueabi-as > -mcpu=cortex-a5 -mimplicit-it=always foo.S -o foo.o > $ arm-none-linux-gnueabi-vybrid-4.9.1/bin/arm-none-linux-gnueabi-as > -mcpu=cortex-a5 -mimplicit-it=always foo.S -o foo.o > > Both give 'objdump -S foo.o', > foo.o: file format elf32-littlearm > > Disassembly of section .text: > > 00000000 <foo>: > 0: 2805 cmp r0, #5 > 2: bf14 ite ne > 4: 2106 movne r1, #6 > 6: 2102 moveq r1, #2 > 8: 4770 bx lr > > 0000000a <bar>: > a: 280a cmp r0, #10 > c: bf8c ite hi > e: 2103 movhi r1, #3 > 10: 2107 movls r1, #7 > 12: bf08 it eq > 14: 210b moveq r1, #11 > 16: 4770 bx lr > > I think before the patch there would be 'it' values before each and > every condition. In fact, if you change 'bar' to, > > bar: > cmp r0, #10 > movhi r1,#3 > movlo r1,#7 > moveq r1,#11 > bx lr > > You get three 'IT' conditions as 'HI' and 'LO' are not opposite. The > patch seem to detect things that are the exact opposite. The 'bar' > above ends up with '11' in r1 if the value is zero, but it temporarily > '7'. The 2nd bar will only place 11 in r1. > > Fwiw, > Bill Pringlemeir. > > Ref: https://wiki.ubuntu.com/ARM/Thumb2PortingHowto#Conditional_Execution All this is about -mimplicit-it, which *is* documented, *is* consistently present in common toolchains AFAICT and is and (as) logcical (as it can be when dealing with backward compatibility of command lines etc). However, my problem is not with -mimplicit-it; it is with -mauto-it. My gut feeling is that -mauto-it is a predecessor of -mimplicit-it. Amicalement,
>>> In message <20141119074214.3d414ce6@lilith> you^H^H^H Stefan wrote: >>>> For -mauto-it, it is not documented in the gas documentation online >>>> or in my current as' --target-help. I'll dig this deeper today, but >>>> barring any scream from me, the change above is fine globally in >>>> U-Boot. >> On 19 Nov 2014, wd@denx.de wrote: >>> Apparently this [1] is where it is coming from; no further >>> documentation there, though. >> >>> [1] https://sourceware.org/ml/binutils/2009-05/msg00132.html > On Wed, 19 Nov 2014 11:31:05 -0500, Bill Pringlemeir >> I would think that if this worked they would make it automatic and >> not an option. Probably this is only in certain binutils/as. >> >> With 4.6.3 and 4.9.1 I do not have this option, On 19 Nov 2014, albert.u.boot@aribaud.net wrote: > Which option do you mean? -mimplicit-it or -mauto-it? '-mauto-it' , which I think if it is working correctly would be rolled into '-mimplicit-it' as it generates better code (for an assembler :). I followed the thread above and the patch originator says he needs to fix section issues and the 'command line options' and he would follow up the proposed patch. I guess at some version each and every '<inst>xx' was converted to 'it xx\n<inst>' where <inst> is some conditional instruction. For the patch above, '-mauto-it' teaches the assembler to glob them together into 'itet...' type conditions. The Thumb2 supports up to four conditions (and negated condition) instructions. On my version of the tools (I think it is gcc; but maybe binutils), if I use '-mauto-it' it gives an unknown option error. I think that Linux does a probe of this feature and passes it (-mauto-it) if the assembler accepts it. So, if we add to u-boot we should probably take care that the ARM 'as' can take the option. I also see posts on the web of people complaining of this option in other code bases. Fwiw, Bill.
Hello Bill, On Wed, 19 Nov 2014 13:34:34 -0500, Bill Pringlemeir <bpringlemeir@nbsps.com> wrote: > > > >>> In message <20141119074214.3d414ce6@lilith> you^H^H^H Stefan wrote: > > >>>> For -mauto-it, it is not documented in the gas documentation online > >>>> or in my current as' --target-help. I'll dig this deeper today, but > >>>> barring any scream from me, the change above is fine globally in > >>>> U-Boot. > > >> On 19 Nov 2014, wd@denx.de wrote: > > >>> Apparently this [1] is where it is coming from; no further > >>> documentation there, though. > >> > >>> [1] https://sourceware.org/ml/binutils/2009-05/msg00132.html > > > On Wed, 19 Nov 2014 11:31:05 -0500, Bill Pringlemeir > > >> I would think that if this worked they would make it automatic and > >> not an option. Probably this is only in certain binutils/as. > >> > >> With 4.6.3 and 4.9.1 I do not have this option, > > > On 19 Nov 2014, albert.u.boot@aribaud.net wrote: > > > Which option do you mean? -mimplicit-it or -mauto-it? > > '-mauto-it' , which I think if it is working correctly would be rolled > into '-mimplicit-it' as it generates better code (for an assembler :). > I followed the thread above and the patch originator says he needs to > fix section issues and the 'command line options' and he would follow up > the proposed patch. I am getting lost, even when reading (quickly, I admit) the patch that adds it; I don't see what -mauto-it does exactly. Can you summarize and clarify the effects of -mimplicit-it (I guess I know this one but it's never a bad thing to get a second opinion), -mauto-it and their interaction? > I guess at some version each and every '<inst>xx' was converted to 'it > xx\n<inst>' where <inst> is some conditional instruction. For the patch > above, '-mauto-it' teaches the assembler to glob them together into > 'itet...' type conditions. The Thumb2 supports up to four conditions > (and negated condition) instructions. > > On my version of the tools (I think it is gcc; but maybe binutils), if I > use '-mauto-it' it gives an unknown option error. I think that Linux > does a probe of this feature and passes it (-mauto-it) if the assembler > accepts it. So, if we add to u-boot we should probably take care that > the ARM 'as' can take the option. I also see posts on the web of people > complaining of this option in other code bases. Are they complaining that the option is passed on to an as which does not know it, or that the option is known but does something wrong? (I'm still having a hard time understanding whether -mauto-it is an accepted new feature that is slow to get integrated or the remnant of an old proposal which did not make it.) > Fwiw, > Bill. Amicalement,
> On Wed, 19 Nov 2014 13:34:34 -0500, Bill Pringlemeir > <bpringlemeir@nbsps.com> wrote: >>>>> In message <20141119074214.3d414ce6@lilith> Albert wrote: >>>>>> For -mauto-it, it is not documented in the gas documentation >>>>>> online or in my current as' --target-help. I'll dig this deeper >>>>>> today, but barring any scream from me, the change above is fine >>>>>> globally in U-Boot. >>>> On 19 Nov 2014, wd@denx.de wrote: >> >>>>> Apparently this [1] is where it is coming from; no further >>>>> documentation there, though. >>>> >>>>> [1] https://sourceware.org/ml/binutils/2009-05/msg00132.html >> >>> On Wed, 19 Nov 2014 11:31:05 -0500, Bill Pringlemeir >> >>>> I would think that if this worked they would make it automatic and >>>> not an option. Probably this is only in certain binutils/as. >>>> With 4.6.3 and 4.9.1 I do not have this option, >> On 19 Nov 2014, albert.u.boot@aribaud.net wrote: >>> Which option do you mean? -mimplicit-it or -mauto-it? >> '-mauto-it' , which I think if it is working correctly would be >> rolled into '-mimplicit-it' as it generates better code (for an >> assembler :). I followed the thread above and the patch originator >> says he needs to fix section issues and the 'command line options' >> and he would follow up the proposed patch. On 20 Nov 2014, albert.u.boot@aribaud.net wrote: > I am getting lost, even when reading (quickly, I admit) the patch that > adds it; I don't see what -mauto-it does exactly. Can you summarize > and clarify the effects of -mimplicit-it (I guess I know this one but > it's never a bad thing to get a second opinion), -mauto-it and their > interaction? I guess you know how the 'IT' works. The Ubuntu/Debian people give a good explanation in a few paragraphs, https://wiki.ubuntu.com/ARM/Thumb2PortingHowto#Conditional_Execution My trying to explain this may have confused thing... Here is Wolfgang's reference, https://sourceware.org/ml/binutils/2009-05/msg00132.html Here is a 2nd reference, https://sourceware.org/ml/binutils/2009-06/msg00162.html Originally Daniel Gutson used '-mauto-it' and then it was converted to '-mimplicit-it'. I am not sure if '-mauto-it' exists in the wild. I have never heard of that option before seeing this email thread. Also my assembler says, Assembler messages: Error: unrecognized option -mauto-it I have built with the most recent binutils, gcc4.9.1 using crosstool-ng. Maybe only some non-mainline tools picked up this '-mauto-it' patch. I don't think it hurts to support '-mauto-it', but an assembler test should be done to see if it accepts the option. hth, Bill Pringlemeir.
Hello Bill, On Thu, 20 Nov 2014 11:34:45 -0500, Bill Pringlemeir <bpringlemeir@nbsps.com> wrote: > Originally Daniel Gutson used '-mauto-it' and then it was converted to > '-mimplicit-it'. Ok, so I was right in my gut(son)[1] feeling that -mauto-it was a predecessor of -mimplicit-it -- and actually only in the first iteration(s) of the patch that would have introduced it. > I am not sure if '-mauto-it' exists in the wild. I have never heard of > that option before seeing this email thread. Also my assembler says, > > Assembler messages: > Error: unrecognized option -mauto-it > > I have built with the most recent binutils, gcc4.9.1 using crosstool-ng. > Maybe only some non-mainline tools picked up this '-mauto-it' patch. I > don't think it hurts to support '-mauto-it', but an assembler test > should be done to see if it accepts the option. I've gone through the binutils git tree, and -mauto-it is mentioned only in a patch that fixes the gas /docs/ which erroneously mentioned -mauto-it where it should have mentioned -mimplicit-it. Hence, I think we should not test for -mauto-it at all, and not mention it even. Stefan, can you resubmit without the -mauto-it part, and renaming AFLAGS_AUTOIT into AFLAGS_IMPLICIT_IT? > hth, > Bill Pringlemeir. Amicalement,
diff --git a/arch/arm/config.mk b/arch/arm/config.mk index f0eafd6..ddbc8dc 100644 --- a/arch/arm/config.mk +++ b/arch/arm/config.mk @@ -30,6 +30,8 @@ PF_CPPFLAGS_ARM := $(call cc-option, -mthumb -mthumb-interwork,\ $(call cc-option,-marm,)\ $(call cc-option,-mno-thumb-interwork,)\ ) +AFLAGS_AUTOIT :=$(call as-option,-Wa$(comma)-mimplicit-it=always,-Wa$(comma)-mauto-it) +PF_CPPFLAGS_ARM += $(AFLAGS_AUTOIT) else PF_CPPFLAGS_ARM := $(call cc-option,-marm,) \ $(call cc-option,-mno-thumb-interwork,) diff --git a/arch/arm/lib/memcpy.S b/arch/arm/lib/memcpy.S index f655256..fcf028c 100644 --- a/arch/arm/lib/memcpy.S +++ b/arch/arm/lib/memcpy.S @@ -12,11 +12,14 @@ #include <asm/assembler.h> -#define W(instr) instr +#define W(instr) instr.w #define LDR1W_SHIFT 0 #define STR1W_SHIFT 0 +#define CALGN(code...) + + .macro ldr1w ptr reg abort W(ldr) \reg, [\ptr], #4 .endm @@ -57,12 +60,15 @@ /* Prototype: void *memcpy(void *dest, const void *src, size_t n); */ + .syntax unified + .thumb + .thumb_func .globl memcpy memcpy: - +/* cmp r0, r1 moveq pc, lr - +*/ enter r4, lr subs r2, r2, #4 diff --git a/include/configs/colibri_vf.h b/include/configs/colibri_vf.h index 76564ac..41a0dac 100644 --- a/include/configs/colibri_vf.h +++ b/include/configs/colibri_vf.h @@ -53,7 +53,10 @@ #define CONFIG_CMD_NAND #define CONFIG_NAND_VF610_NFC #define CONFIG_SYS_NAND_SELF_INIT + +#define CONFIG_SYS_THUMB_BUILD #define CONFIG_USE_ARCH_MEMCPY + #define CONFIG_SYS_MAX_NAND_DEVICE 1 #define CONFIG_SYS_NAND_BASE NFC_BASE_ADDR