| Message ID | 1377345338-9695-1-git-send-email-jeroen@myspectrum.nl |
|---|---|
| State | Accepted |
| Delegated to: | Albert ARIBAUD |
| Headers | show |
Hi, experts: >-# check that only R_ARM_RELATIVE relocations are generated > ifneq ($(CONFIG_SPL_BUILD),y) >-ALL-y += checkarmreloc >+# Check that only R_ARM_RELATIVE relocations are generated. >+ALL-y += checkarmreloc >+# The movt / movw can hardcode 16 bit parts of the addresses in the >+# instruction. Relocation is not supported for that case, so disable >+# such usage by requiring word relocations. >+PLATFORM_CPPFLAGS += $(call cc-option, -mword-relocations) > endif Jeroen's patch is very simple. So, is there any side-effect? If not, why not add it into 2013.10 release version? :) Best wishes,
On 09/17/2013 12:44 PM, TigerLiu@viatech.com.cn wrote: > > Jeroen's patch is very simple. > So, is there any side-effect? Not that I am aware of. > If not, why not add it into 2013.10 release version? :) That is up to Albert and Tom. Regards, Jeroen
On Sat, Aug 24, 2013 at 01:55:38PM +0200, Jeroen Hofstee wrote: > The movt/movw instruction can be used to hardcode an > memory location in the instruction itself. The linker > starts complaining about this if the compiler decides > to do so: "relocation R_ARM_MOVW_ABS_NC against `a local > symbol' can not be used" and it is not support by U-boot > as well. Prevent their use by requiring word relocations. > This allows u-boot to be build at other optimalization > levels then -Os. > > Signed-off-by: Jeroen Hofstee <jeroen@myspectrum.nl> > Cc: TigerLiu@viatech.com.cn > Cc: Albert ARIBAUD <albert.u.boot@aribaud.net> > --- > arch/arm/config.mk | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) Is this also something we need for llvm? I am hesitant here because as Wolfgang points out, -O0 is usually the wrong way to debug a problem and I'll add we're well into the age where debuggers work just fine with optimized code. If there's some -O2 enabled gcc flag we want because of a measurable performance win, we should add it specifically to -Os.
Hello Tom, On 09/19/2013 11:16 PM, Tom Rini wrote: > On Sat, Aug 24, 2013 at 01:55:38PM +0200, Jeroen Hofstee wrote: > >> The movt/movw instruction can be used to hardcode an >> memory location in the instruction itself. The linker >> starts complaining about this if the compiler decides >> to do so: "relocation R_ARM_MOVW_ABS_NC against `a local >> symbol' can not be used" and it is not support by U-boot >> as well. Prevent their use by requiring word relocations. >> This allows u-boot to be build at other optimalization >> levels then -Os. >> >> Signed-off-by: Jeroen Hofstee <jeroen@myspectrum.nl> >> Cc: TigerLiu@viatech.com.cn >> Cc: Albert ARIBAUD <albert.u.boot@aribaud.net> >> --- >> arch/arm/config.mk | 8 ++++++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) > Is this also something we need for llvm? You guessed that right, for clang actually (llvm has already been taught to not emit movw/movt pairs, when requested not to do so). So with the -mword-relocations || present I can teach clang to tell llvm not to do it. I am not aware of any reason why gcc could not decide to do the same in future releases. A pointer comparison e.g. is of exactly the same size (afaik). In this case U-boot will no longer compile without mentioned flag. > I am hesitant here because as > Wolfgang points out, -O0 is usually the wrong way to debug a problem and > I'll add we're well into the age where debuggers work just fine with > optimized code. mmm, I don't share your concern here. Not that I disagree with what Wolfgang said, but since it is unrelated to the patch itself. What I read was that Wolfgang tried to explain to a ML poster without a proper name that it might be even harder at times to find a bug at -O0, since it is a different binary and that it is not considered a bug. I assume the fast majority of U-boot developers know these to debug things.. If you really have that little trust in U-boot developers a more proper way would be to actually create a make rule checking cflags and point them to a nice debugging document. And I really hope you don't do that ;) One thing I can think of in favour of -O0 is for educational purposes. You can run u-boot in qemu then without the, at times weird optimized jumps, to get an idea about basic code flow. > If there's some -O2 enabled gcc flag we want because of > a measurable performance win, we should add it specifically to -Os. > First of all the default -Os is unchanged and I have no intention to change it. -O2 won't build without the patch last time I checked ;) Anyway, I like the flag since it helps to not special case clang and it guarantees builds with gcc at all optimisation levels, now and in the future. I don't care if it goes in this release or the next one. Regards, Jeroen
On Fri, Sep 20, 2013 at 07:15:29PM +0200, Jeroen Hofstee wrote: > Hello Tom, > > On 09/19/2013 11:16 PM, Tom Rini wrote: > >On Sat, Aug 24, 2013 at 01:55:38PM +0200, Jeroen Hofstee wrote: > > > >>The movt/movw instruction can be used to hardcode an > >>memory location in the instruction itself. The linker > >>starts complaining about this if the compiler decides > >>to do so: "relocation R_ARM_MOVW_ABS_NC against `a local > >>symbol' can not be used" and it is not support by U-boot > >>as well. Prevent their use by requiring word relocations. > >>This allows u-boot to be build at other optimalization > >>levels then -Os. > >> > >>Signed-off-by: Jeroen Hofstee <jeroen@myspectrum.nl> > >>Cc: TigerLiu@viatech.com.cn > >>Cc: Albert ARIBAUD <albert.u.boot@aribaud.net> > >>--- > >> arch/arm/config.mk | 8 ++++++-- > >> 1 file changed, 6 insertions(+), 2 deletions(-) > >Is this also something we need for llvm? > > You guessed that right, for clang actually (llvm has > already been taught to not emit movw/movt pairs, > when requested not to do so). So with the > -mword-relocations || present I can teach clang to tell llvm > not to do it. > > I am not aware of any reason why gcc could not > decide to do the same in future releases. A pointer > comparison e.g. is of exactly the same size (afaik). > In this case U-boot will no longer compile without > mentioned flag. OK. [snip] > >If there's some -O2 enabled gcc flag we want because of > >a measurable performance win, we should add it specifically to -Os. > > > First of all the default -Os is unchanged and I have no > intention to change it. -O2 won't build without the patch > last time I checked ;) > > Anyway, I like the flag since it helps to not special case > clang and it guarantees builds with gcc at all optimisation > levels, now and in the future. I don't care if it goes in this > release or the next one. Right, I'm OK picking this patch up then, on the grounds of making clang/llvm work now, and potentially keeping a future gcc happy.
On Sat, Aug 24, 2013 at 5:55 AM, Jeroen Hofstee <jeroen@myspectrum.nl>wrote: > The movt/movw instruction can be used to hardcode an > memory location in the instruction itself. The linker > starts complaining about this if the compiler decides > to do so: "relocation R_ARM_MOVW_ABS_NC against `a local > symbol' can not be used" and it is not support by U-boot > as well. Prevent their use by requiring word relocations. > This allows u-boot to be build at other optimalization > levels then -Os. > > Signed-off-by: Jeroen Hofstee <jeroen@myspectrum.nl> > Cc: TigerLiu@viatech.com.cn > Cc: Albert ARIBAUD <albert.u.boot@aribaud.net> > This is useful I think. I'm not sure that -O0 works very well anymore (at least I need to make a few tweaks to use it), but -O1 is useful in some cases to provide better debugging. Acked-by: Simon Glass <sjg@chromium.org>
Hi Jeroen, On Sat, 24 Aug 2013 13:55:38 +0200, Jeroen Hofstee <jeroen@myspectrum.nl> wrote: > The movt/movw instruction can be used to hardcode an > memory location in the instruction itself. The linker > starts complaining about this if the compiler decides > to do so: "relocation R_ARM_MOVW_ABS_NC against `a local > symbol' can not be used" and it is not support by U-boot > as well. Prevent their use by requiring word relocations. > This allows u-boot to be build at other optimalization > levels then -Os. > > Signed-off-by: Jeroen Hofstee <jeroen@myspectrum.nl> > Cc: TigerLiu@viatech.com.cn > Cc: Albert ARIBAUD <albert.u.boot@aribaud.net> > --- > arch/arm/config.mk | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/config.mk b/arch/arm/config.mk > index 540a119..2277c82 100644 > --- a/arch/arm/config.mk > +++ b/arch/arm/config.mk > @@ -94,7 +94,11 @@ PLATFORM_RELFLAGS += -fno-optimize-sibling-calls > endif > endif > > -# check that only R_ARM_RELATIVE relocations are generated > ifneq ($(CONFIG_SPL_BUILD),y) > -ALL-y += checkarmreloc > +# Check that only R_ARM_RELATIVE relocations are generated. > +ALL-y += checkarmreloc > +# The movt / movw can hardcode 16 bit parts of the addresses in the > +# instruction. Relocation is not supported for that case, so disable > +# such usage by requiring word relocations. > +PLATFORM_CPPFLAGS += $(call cc-option, -mword-relocations) > endif Applied as a bugfix to u-boot-arm/master, thanks! Amicalement,
diff --git a/arch/arm/config.mk b/arch/arm/config.mk index 540a119..2277c82 100644 --- a/arch/arm/config.mk +++ b/arch/arm/config.mk @@ -94,7 +94,11 @@ PLATFORM_RELFLAGS += -fno-optimize-sibling-calls endif endif -# check that only R_ARM_RELATIVE relocations are generated ifneq ($(CONFIG_SPL_BUILD),y) -ALL-y += checkarmreloc +# Check that only R_ARM_RELATIVE relocations are generated. +ALL-y += checkarmreloc +# The movt / movw can hardcode 16 bit parts of the addresses in the +# instruction. Relocation is not supported for that case, so disable +# such usage by requiring word relocations. +PLATFORM_CPPFLAGS += $(call cc-option, -mword-relocations) endif
The movt/movw instruction can be used to hardcode an memory location in the instruction itself. The linker starts complaining about this if the compiler decides to do so: "relocation R_ARM_MOVW_ABS_NC against `a local symbol' can not be used" and it is not support by U-boot as well. Prevent their use by requiring word relocations. This allows u-boot to be build at other optimalization levels then -Os. Signed-off-by: Jeroen Hofstee <jeroen@myspectrum.nl> Cc: TigerLiu@viatech.com.cn Cc: Albert ARIBAUD <albert.u.boot@aribaud.net> --- arch/arm/config.mk | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)