[Buildroot] [PATCH] ruby: fix 'pcrel too far' build problem on SuperH architectures

[Buildroot] [PATCH] ruby: fix 'pcrel too far' build problem on SuperH architectures

[Thomas De Schampheleire]

The 'pcrel too far' problem detected in the autobuild on SuperH
architectures, seems to be caused by the -Os optimization flag. Using
standard optimization fixes the problem.

Fixes http://autobuild.buildroot.net/results/bc36e051e06f596c2fafdd3cc3745bb34b73ace3/

Investigated-by: Lionel Orry <lionel.orry@gmail.com>
Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

---
 package/ruby/ruby.mk |  7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/package/ruby/ruby.mk b/package/ruby/ruby.mk
--- a/package/ruby/ruby.mk
+++ b/package/ruby/ruby.mk
@@ -15,6 +15,13 @@ HOST_RUBY_CONF_OPT = --disable-install-d
 RUBY_LICENSE = Ruby or BSD-2c, BSD-3c, others
 RUBY_LICENSE_FILES = LEGAL COPYING BSDL
 
+# With some SuperH toolchains (like Sourcery CodeBench 2012.09), ruby fails to
+# build with 'pcrel too far'. This seems to be caused by the -Os option we pass
+# by default. To fix the problem, use standard -O2 optimization instead.
+ifeq ($(BR2_sh)$(BR2_sh64),y)
+TARGET_CFLAGS += -O2
+endif
+
 # Force optionals to build before we do
 ifeq ($(BR2_PACKAGE_BERKELEYDB),y)
 	RUBY_DEPENDENCIES += berkeleydb

[Buildroot] [PATCH] ruby: fix 'pcrel too far' build problem on SuperH architectures

[Thomas Petazzoni]

Dear Thomas De Schampheleire,

On Sun, 13 Oct 2013 09:44:36 +0200, Thomas De Schampheleire wrote:

> +# With some SuperH toolchains (like Sourcery CodeBench 2012.09), ruby fails to
> +# build with 'pcrel too far'. This seems to be caused by the -Os option we pass
> +# by default. To fix the problem, use standard -O2 optimization instead.
> +ifeq ($(BR2_sh)$(BR2_sh64),y)
> +TARGET_CFLAGS += -O2
> +endif

I don't think it's the right way of fixing the problem. This is going to
affect the CFLAGS of *all* packages. You shouldn't change
TARGET_CFLAGS, but instead do:

RUBY_CONF_ENV += CFLAGS="$(TARGET_CFLAGS) -O2"

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH] ruby: fix 'pcrel too far' build problem on SuperH architectures

[Thomas De Schampheleire]

Hi Thomas,

On Sun, Oct 13, 2013 at 9:51 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Thomas De Schampheleire,
>
> On Sun, 13 Oct 2013 09:44:36 +0200, Thomas De Schampheleire wrote:
>
>> +# With some SuperH toolchains (like Sourcery CodeBench 2012.09), ruby fails to
>> +# build with 'pcrel too far'. This seems to be caused by the -Os option we pass
>> +# by default. To fix the problem, use standard -O2 optimization instead.
>> +ifeq ($(BR2_sh)$(BR2_sh64),y)
>> +TARGET_CFLAGS += -O2
>> +endif
>
> I don't think it's the right way of fixing the problem. This is going to
> affect the CFLAGS of *all* packages. You shouldn't change
> TARGET_CFLAGS, but instead do:
>
> RUBY_CONF_ENV += CFLAGS="$(TARGET_CFLAGS) -O2"

Ah yes, you're right. The above was what I proposed initially, but I
tried to be too smart.
I was looking for a way to avoid duplicating TARGET_CFLAGS, because if
someone needs to add a new cflag for ruby specifically, they now need
to explicitly take the superh case into account.

I'll send a new version.

Thanks,
Thomas

[Buildroot] [PATCH] ruby: fix 'pcrel too far' build problem on SuperH architectures

[Thomas Petazzoni]

Dear Thomas De Schampheleire,

On Sun, 13 Oct 2013 09:58:05 +0200, Thomas De Schampheleire wrote:

> > I don't think it's the right way of fixing the problem. This is going to
> > affect the CFLAGS of *all* packages. You shouldn't change
> > TARGET_CFLAGS, but instead do:
> >
> > RUBY_CONF_ENV += CFLAGS="$(TARGET_CFLAGS) -O2"
> 
> Ah yes, you're right. The above was what I proposed initially, but I
> tried to be too smart.
> I was looking for a way to avoid duplicating TARGET_CFLAGS, because if
> someone needs to add a new cflag for ruby specifically, they now need
> to explicitly take the superh case into account.
> 
> I'll send a new version.

The way we usually do this is:

RUBY_CFLAGS = $(TARGET_CFLAGS)

ifeq ($(BR2_sh)$(BR2_sh64),y)
RUBY_CFLAGS += -O2
endif

RUBY_CONF_ENV += CFLAGS="$(RUBY_CFLAGS)"

This way, someone else can easily add another RUBY_CFLAGS +=, and not
worry about the SuperH thing.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH] ruby: fix 'pcrel too far' build problem on SuperH architectures

[Thomas De Schampheleire]

On Sun, Oct 13, 2013 at 10:25 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Thomas De Schampheleire,
>
> On Sun, 13 Oct 2013 09:58:05 +0200, Thomas De Schampheleire wrote:
>
>> > I don't think it's the right way of fixing the problem. This is going to
>> > affect the CFLAGS of *all* packages. You shouldn't change
>> > TARGET_CFLAGS, but instead do:
>> >
>> > RUBY_CONF_ENV += CFLAGS="$(TARGET_CFLAGS) -O2"
>>
>> Ah yes, you're right. The above was what I proposed initially, but I
>> tried to be too smart.
>> I was looking for a way to avoid duplicating TARGET_CFLAGS, because if
>> someone needs to add a new cflag for ruby specifically, they now need
>> to explicitly take the superh case into account.
>>
>> I'll send a new version.
>
> The way we usually do this is:
>
> RUBY_CFLAGS = $(TARGET_CFLAGS)
>
> ifeq ($(BR2_sh)$(BR2_sh64),y)
> RUBY_CFLAGS += -O2
> endif
>
> RUBY_CONF_ENV += CFLAGS="$(RUBY_CFLAGS)"
>
> This way, someone else can easily add another RUBY_CFLAGS +=, and not
> worry about the SuperH thing.


Ok, that indeed looks better. I'll send a third version later.

Thanks for your feedback,
Thomas