[PATCH 2/2] gcc: Avoid non-aligned access for ARM5e

[PATCH 2/2] gcc: Avoid non-aligned access for ARM5e

[Jens Rehsack]

[Icedtee Ticket #2153] -- see http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=2153

This patch prevents gcc generate code on ARM5e accessing 64 bit values, because gcc doesn't ensure they're 64 bit aligned.
Accessing a 64 bit value from a 32 bit alignment causes segmentation faults.

Patch is taken from https://github.com/archlinuxarm/PKGBUILDs/blob/master/core/gcc/0001-ARMv5-disable-LDRD-STRD.patch

Signed-off-by: Jens Rehsack <sno@netbsd.org>
---
 meta/recipes-devtools/gcc/gcc-4.9.inc              |  1 +
 .../gcc/gcc-4.9/0065-ARMv5-disable-LDRD-STRD.patch | 24 ++++++++++++++++++++++
 meta/recipes-devtools/gcc/gcc-5.2.inc              |  1 +
 .../gcc/gcc-5.2/0042-ARMv5-disable-LDRD-STRD.patch | 24 ++++++++++++++++++++++
 4 files changed, 50 insertions(+)
 create mode 100644 meta/recipes-devtools/gcc/gcc-4.9/0065-ARMv5-disable-LDRD-STRD.patch
 create mode 100644 meta/recipes-devtools/gcc/gcc-5.2/0042-ARMv5-disable-LDRD-STRD.patch

diff --git a/meta/recipes-devtools/gcc/gcc-4.9.inc b/meta/recipes-devtools/gcc/gcc-4.9.inc
index 691ba5f..9a1b44e 100644
--- a/meta/recipes-devtools/gcc/gcc-4.9.inc
+++ b/meta/recipes-devtools/gcc/gcc-4.9.inc
@@ -80,6 +80,7 @@ SRC_URI = "\
     file://0062-Use-SYSTEMLIBS_DIR-replacement-instead-of-hardcoding.patch \
     file://0063-nativesdk-gcc-support.patch \
     file://0064-handle-target-sysroot-multilib.patch \
+    file://0065-ARMv5-disable-LDRD-STRD.patch \
 "
 SRC_URI[md5sum] = "6f831b4d251872736e8e9cc09746f327"
 SRC_URI[sha256sum] = "2332b2a5a321b57508b9031354a8503af6fdfb868b8c1748d33028d100a8b67e"
diff --git a/meta/recipes-devtools/gcc/gcc-4.9/0065-ARMv5-disable-LDRD-STRD.patch b/meta/recipes-devtools/gcc/gcc-4.9/0065-ARMv5-disable-LDRD-STRD.patch
new file mode 100644
index 0000000..68fe3b9
--- /dev/null
+++ b/meta/recipes-devtools/gcc/gcc-4.9/0065-ARMv5-disable-LDRD-STRD.patch
@@ -0,0 +1,24 @@
+From b1884b6d1950422a32f9ced0be6d5492e4315547 Mon Sep 17 00:00:00 2001
+From: Kevin Mihelich <kevin@archlinuxarm.org>
+Date: Wed, 7 Jan 2015 18:43:57 -0700
+Subject: [PATCH] ARMv5: disable LDRD/STRD
+
+---
+ gcc/config/arm/arm.h | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
+index c60d9a4..65c60b9 100644
+--- a/gcc/config/arm/arm.h
++++ b/gcc/config/arm/arm.h
+@@ -282,7 +282,7 @@ extern void (*arm_lang_output_object_attributes_hook)(void);
+ /* Thumb-1 only.  */
+ #define TARGET_THUMB1_ONLY		(TARGET_THUMB1 && !arm_arch_notm)
+ 
+-#define TARGET_LDRD			(arm_arch5e && ARM_DOUBLEWORD_ALIGN \
++#define TARGET_LDRD			(arm_arch6 && ARM_DOUBLEWORD_ALIGN \
+                                          && !TARGET_THUMB1)
+ 
+ #define TARGET_CRC32			(arm_arch_crc)
+-- 
+2.2.1
diff --git a/meta/recipes-devtools/gcc/gcc-5.2.inc b/meta/recipes-devtools/gcc/gcc-5.2.inc
index f6673e6..044e8ae 100644
--- a/meta/recipes-devtools/gcc/gcc-5.2.inc
+++ b/meta/recipes-devtools/gcc/gcc-5.2.inc
@@ -73,6 +73,7 @@ SRC_URI = "\
            file://0039-libcc1-fix-libcc1-s-install-path-and-rpath.patch \
            file://0040-nativesdk-gcc-support.patch \
            file://0041-handle-target-sysroot-multilib.patch \
+           file://0042-ARMv5-disable-LDRD-STRD.patch \
           "
 
 BACKPORTS = ""
diff --git a/meta/recipes-devtools/gcc/gcc-5.2/0042-ARMv5-disable-LDRD-STRD.patch b/meta/recipes-devtools/gcc/gcc-5.2/0042-ARMv5-disable-LDRD-STRD.patch
new file mode 100644
index 0000000..68fe3b9
--- /dev/null
+++ b/meta/recipes-devtools/gcc/gcc-5.2/0042-ARMv5-disable-LDRD-STRD.patch
@@ -0,0 +1,24 @@
+From b1884b6d1950422a32f9ced0be6d5492e4315547 Mon Sep 17 00:00:00 2001
+From: Kevin Mihelich <kevin@archlinuxarm.org>
+Date: Wed, 7 Jan 2015 18:43:57 -0700
+Subject: [PATCH] ARMv5: disable LDRD/STRD
+
+---
+ gcc/config/arm/arm.h | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
+index c60d9a4..65c60b9 100644
+--- a/gcc/config/arm/arm.h
++++ b/gcc/config/arm/arm.h
+@@ -282,7 +282,7 @@ extern void (*arm_lang_output_object_attributes_hook)(void);
+ /* Thumb-1 only.  */
+ #define TARGET_THUMB1_ONLY		(TARGET_THUMB1 && !arm_arch_notm)
+ 
+-#define TARGET_LDRD			(arm_arch5e && ARM_DOUBLEWORD_ALIGN \
++#define TARGET_LDRD			(arm_arch6 && ARM_DOUBLEWORD_ALIGN \
+                                          && !TARGET_THUMB1)
+ 
+ #define TARGET_CRC32			(arm_arch_crc)
+-- 
+2.2.1
-- 
2.4.3

-- 
Jens Rehsack - rehsack@gmail.com

Re: [PATCH 2/2] gcc: Avoid non-aligned access for ARM5e

[Khem Raj]

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


> On Oct 8, 2015, at 7:33 AM, Jens Rehsack <rehsack@gmail.com> wrote:
> 
> 
> [Icedtee Ticket #2153] -- see http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=2153
> 
> This patch prevents gcc generate code on ARM5e accessing 64 bit values, because gcc doesn't ensure they're 64 bit aligned.
> Accessing a 64 bit value from a 32 bit alignment causes segmentation faults.
> 
> Patch is taken from https://github.com/archlinuxarm/PKGBUILDs/blob/master/core/gcc/0001-ARMv5-disable-LDRD-STRD.patch

Can you try using -mfix-cortex-m3-ldrd, does that help ?
The patch is a workaround to gcc, as it seems real problem is in defining instruction constraints
please open a gcc bug for this.

> 
> Signed-off-by: Jens Rehsack <sno@netbsd.org>
> ---
> meta/recipes-devtools/gcc/gcc-4.9.inc              |  1 +
> .../gcc/gcc-4.9/0065-ARMv5-disable-LDRD-STRD.patch | 24 ++++++++++++++++++++++
> meta/recipes-devtools/gcc/gcc-5.2.inc              |  1 +
> .../gcc/gcc-5.2/0042-ARMv5-disable-LDRD-STRD.patch | 24 ++++++++++++++++++++++
> 4 files changed, 50 insertions(+)
> create mode 100644 meta/recipes-devtools/gcc/gcc-4.9/0065-ARMv5-disable-LDRD-STRD.patch
> create mode 100644 meta/recipes-devtools/gcc/gcc-5.2/0042-ARMv5-disable-LDRD-STRD.patch
> 
> diff --git a/meta/recipes-devtools/gcc/gcc-4.9.inc b/meta/recipes-devtools/gcc/gcc-4.9.inc
> index 691ba5f..9a1b44e 100644
> --- a/meta/recipes-devtools/gcc/gcc-4.9.inc
> +++ b/meta/recipes-devtools/gcc/gcc-4.9.inc
> @@ -80,6 +80,7 @@ SRC_URI = "\
>     file://0062-Use-SYSTEMLIBS_DIR-replacement-instead-of-hardcoding.patch \
>     file://0063-nativesdk-gcc-support.patch \
>     file://0064-handle-target-sysroot-multilib.patch \
> +    file://0065-ARMv5-disable-LDRD-STRD.patch \
> "
> SRC_URI[md5sum] = "6f831b4d251872736e8e9cc09746f327"
> SRC_URI[sha256sum] = "2332b2a5a321b57508b9031354a8503af6fdfb868b8c1748d33028d100a8b67e"
> diff --git a/meta/recipes-devtools/gcc/gcc-4.9/0065-ARMv5-disable-LDRD-STRD.patch b/meta/recipes-devtools/gcc/gcc-4.9/0065-ARMv5-disable-LDRD-STRD.patch
> new file mode 100644
> index 0000000..68fe3b9
> --- /dev/null
> +++ b/meta/recipes-devtools/gcc/gcc-4.9/0065-ARMv5-disable-LDRD-STRD.patch
> @@ -0,0 +1,24 @@
> +From b1884b6d1950422a32f9ced0be6d5492e4315547 Mon Sep 17 00:00:00 2001
> +From: Kevin Mihelich <kevin@archlinuxarm.org>
> +Date: Wed, 7 Jan 2015 18:43:57 -0700
> +Subject: [PATCH] ARMv5: disable LDRD/STRD
> +
> +---
> + gcc/config/arm/arm.h | 2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
> +index c60d9a4..65c60b9 100644
> +--- a/gcc/config/arm/arm.h
> ++++ b/gcc/config/arm/arm.h
> +@@ -282,7 +282,7 @@ extern void (*arm_lang_output_object_attributes_hook)(void);
> + /* Thumb-1 only.  */
> + #define TARGET_THUMB1_ONLY		(TARGET_THUMB1 && !arm_arch_notm)
> +
> +-#define TARGET_LDRD			(arm_arch5e && ARM_DOUBLEWORD_ALIGN \
> ++#define TARGET_LDRD			(arm_arch6 && ARM_DOUBLEWORD_ALIGN \
> +                                          && !TARGET_THUMB1)
> +
> + #define TARGET_CRC32			(arm_arch_crc)
> +--
> +2.2.1
> diff --git a/meta/recipes-devtools/gcc/gcc-5.2.inc b/meta/recipes-devtools/gcc/gcc-5.2.inc
> index f6673e6..044e8ae 100644
> --- a/meta/recipes-devtools/gcc/gcc-5.2.inc
> +++ b/meta/recipes-devtools/gcc/gcc-5.2.inc
> @@ -73,6 +73,7 @@ SRC_URI = "\
>            file://0039-libcc1-fix-libcc1-s-install-path-and-rpath.patch \
>            file://0040-nativesdk-gcc-support.patch \
>            file://0041-handle-target-sysroot-multilib.patch \
> +           file://0042-ARMv5-disable-LDRD-STRD.patch \
>           "
> 
> BACKPORTS = ""
> diff --git a/meta/recipes-devtools/gcc/gcc-5.2/0042-ARMv5-disable-LDRD-STRD.patch b/meta/recipes-devtools/gcc/gcc-5.2/0042-ARMv5-disable-LDRD-STRD.patch
> new file mode 100644
> index 0000000..68fe3b9
> --- /dev/null
> +++ b/meta/recipes-devtools/gcc/gcc-5.2/0042-ARMv5-disable-LDRD-STRD.patch
> @@ -0,0 +1,24 @@
> +From b1884b6d1950422a32f9ced0be6d5492e4315547 Mon Sep 17 00:00:00 2001
> +From: Kevin Mihelich <kevin@archlinuxarm.org>
> +Date: Wed, 7 Jan 2015 18:43:57 -0700
> +Subject: [PATCH] ARMv5: disable LDRD/STRD
> +
> +---
> + gcc/config/arm/arm.h | 2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
> +index c60d9a4..65c60b9 100644
> +--- a/gcc/config/arm/arm.h
> ++++ b/gcc/config/arm/arm.h
> +@@ -282,7 +282,7 @@ extern void (*arm_lang_output_object_attributes_hook)(void);
> + /* Thumb-1 only.  */
> + #define TARGET_THUMB1_ONLY		(TARGET_THUMB1 && !arm_arch_notm)
> +
> +-#define TARGET_LDRD			(arm_arch5e && ARM_DOUBLEWORD_ALIGN \
> ++#define TARGET_LDRD			(arm_arch6 && ARM_DOUBLEWORD_ALIGN \
> +                                          && !TARGET_THUMB1)
> +
> + #define TARGET_CRC32			(arm_arch_crc)
> +--
> +2.2.1
> --
> 2.4.3
> 
> --
> Jens Rehsack - rehsack@gmail.com
> 


[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 211 bytes --]

Re: [PATCH 2/2] gcc: Avoid non-aligned access for ARM5e

[Jens Rehsack]

> Am 08.10.2015 um 16:46 schrieb Khem Raj <raj.khem@gmail.com>:
> 
> 
>> On Oct 8, 2015, at 7:33 AM, Jens Rehsack <rehsack@gmail.com> wrote:
>> 
>> 
>> [Icedtee Ticket #2153] -- see http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=2153
>> 
>> This patch prevents gcc generate code on ARM5e accessing 64 bit values, because gcc doesn't ensure they're 64 bit aligned.
>> Accessing a 64 bit value from a 32 bit alignment causes segmentation faults.
>> 
>> Patch is taken from https://github.com/archlinuxarm/PKGBUILDs/blob/master/core/gcc/0001-ARMv5-disable-LDRD-STRD.patch
> 
> Can you try using -mfix-cortex-m3-ldrd, does that help ?

Well, I have a marvel-kirkwood - I'll give it a shot but don't know whether it works at all.

> The patch is a workaround to gcc, as it seems real problem is in defining instruction constraints
> please open a gcc bug for this.

Dunno whether I want to have all that stress. When neither ARCH linux guys did that nor Icedtea
maintainers, how could I succeed there?

Best I can do is (if -mfix-cortex-m3-ldrd doesn't help), keep it private. When -mfix-cortex-m3-ldrd
helps, is adding it to ./meta/conf/machine/include/tune-arm926ejs.inc a sane approach?

Cheers
-- 
Jens Rehsack - rehsack@gmail.com

Re: [PATCH 2/2] gcc: Avoid non-aligned access for ARM5e

[Khem Raj]

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


> On Oct 8, 2015, at 9:07 AM, Jens Rehsack <rehsack@gmail.com> wrote:
> 
> 
>> Am 08.10.2015 um 16:46 schrieb Khem Raj <raj.khem@gmail.com>:
>> 
>> 
>>> On Oct 8, 2015, at 7:33 AM, Jens Rehsack <rehsack@gmail.com> wrote:
>>> 
>>> 
>>> [Icedtee Ticket #2153] -- see http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=2153
>>> 
>>> This patch prevents gcc generate code on ARM5e accessing 64 bit values, because gcc doesn't ensure they're 64 bit aligned.
>>> Accessing a 64 bit value from a 32 bit alignment causes segmentation faults.
>>> 
>>> Patch is taken from https://github.com/archlinuxarm/PKGBUILDs/blob/master/core/gcc/0001-ARMv5-disable-LDRD-STRD.patch
>> 
>> Can you try using -mfix-cortex-m3-ldrd, does that help ?
> 
> Well, I have a marvel-kirkwood - I'll give it a shot but don't know whether it works at all.
> 
>> The patch is a workaround to gcc, as it seems real problem is in defining instruction constraints
>> please open a gcc bug for this.
> 
> Dunno whether I want to have all that stress. When neither ARCH linux guys did that nor Icedtea
> maintainers, how could I succeed there?
> 
> Best I can do is (if -mfix-cortex-m3-ldrd doesn't help), keep it private. When -mfix-cortex-m3-ldrd
> helps, is adding it to ./meta/conf/machine/include/tune-arm926ejs.inc a sane approach?

This might be OK. Although I would like to see it fixed in right way in gcc, we don’t want to carry patches
that are not accepted upstream. We already have 60 odd patches and we should reduce them if possible.

[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 211 bytes --]

Re: [PATCH 2/2] gcc: Avoid non-aligned access for ARM5e

[Jens Rehsack]

> Am 08.10.2015 um 18:40 schrieb Khem Raj <raj.khem@gmail.com>:
> 
>> 
>> On Oct 8, 2015, at 9:07 AM, Jens Rehsack <rehsack@gmail.com> wrote:
>> 
>> 
>>> Am 08.10.2015 um 16:46 schrieb Khem Raj <raj.khem@gmail.com>:
>>> 
>>> 
>>>> On Oct 8, 2015, at 7:33 AM, Jens Rehsack <rehsack@gmail.com> wrote:
>>>> 
>>>> 
>>>> [Icedtee Ticket #2153] -- see http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=2153
>>>> 
>>>> This patch prevents gcc generate code on ARM5e accessing 64 bit values, because gcc doesn't ensure they're 64 bit aligned.
>>>> Accessing a 64 bit value from a 32 bit alignment causes segmentation faults.
>>>> 
>>>> Patch is taken from https://github.com/archlinuxarm/PKGBUILDs/blob/master/core/gcc/0001-ARMv5-disable-LDRD-STRD.patch
>>> 
>>> Can you try using -mfix-cortex-m3-ldrd, does that help ?
>> 
>> Well, I have a marvel-kirkwood - I'll give it a shot but don't know whether it works at all.
>> 
>>> The patch is a workaround to gcc, as it seems real problem is in defining instruction constraints
>>> please open a gcc bug for this.
>> 
>> Dunno whether I want to have all that stress. When neither ARCH linux guys did that nor Icedtea
>> maintainers, how could I succeed there?
>> 
>> Best I can do is (if -mfix-cortex-m3-ldrd doesn't help), keep it private. When -mfix-cortex-m3-ldrd
>> helps, is adding it to ./meta/conf/machine/include/tune-arm926ejs.inc a sane approach?
> 
> This might be OK. Although I would like to see it fixed in right way in gcc, we don’t want to carry patches
> that are not accepted upstream. We already have 60 odd patches and we should reduce them if possible.

I tried the approach applying -mfix-cortex-m3-ldrd to TUNE_CCARGS for armv5+dsp - and it generates
ldrd instructions anyway. Accepted upstream or not, I have no time to get the right upstream fix.

So I keep it private.

During hacking on the fix in meta/conf/machine/ I recognized that the expected thump suffix in meta/conf/machine/include/tune-arm926ejs.inc
isn't there and modified the line as

diff --git a/meta/conf/machine/include/tune-arm926ejs.inc b/meta/conf/machine/include/tune-arm926ejs.inc
index 335e4e0..a310c9d 100644
--- a/meta/conf/machine/include/tune-arm926ejs.inc
+++ b/meta/conf/machine/include/tune-arm926ejs.inc
@@ -8,4 +8,4 @@ TUNE_CCARGS .= "${@bb.utils.contains("TUNE_FEATURES", "arm926ejs", " -mtune=arm9
 AVAILTUNES += "arm926ejs"
 ARMPKGARCH_tune-arm926ejs = "arm926ejs"
 TUNE_FEATURES_tune-arm926ejs = "${TUNE_FEATURES_tune-armv5te} arm926ejs"
-PACKAGE_EXTRA_ARCHS_tune-arm926ejs = "${PACKAGE_EXTRA_ARCHS_tune-armv5te} arm926ejste"
+PACKAGE_EXTRA_ARCHS_tune-arm926ejs = "${PACKAGE_EXTRA_ARCHS_tune-armv5te} arm926ejste arm926ejse"

Want to have a patch for that or shall I keep it private, too?

Cheers
-- 
Jens Rehsack - rehsack@gmail.com

Re: [PATCH 2/2] gcc: Avoid non-aligned access for ARM5e

[Khem Raj]

On Mon, Oct 12, 2015 at 9:01 AM, Jens Rehsack <rehsack@gmail.com> wrote:
>
>> Am 08.10.2015 um 18:40 schrieb Khem Raj <raj.khem@gmail.com>:
>>
>>>
>>> On Oct 8, 2015, at 9:07 AM, Jens Rehsack <rehsack@gmail.com> wrote:
>>>
>>>
>>>> Am 08.10.2015 um 16:46 schrieb Khem Raj <raj.khem@gmail.com>:
>>>>
>>>>
>>>>> On Oct 8, 2015, at 7:33 AM, Jens Rehsack <rehsack@gmail.com> wrote:
>>>>>
>>>>>
>>>>> [Icedtee Ticket #2153] -- see http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=2153
>>>>>
>>>>> This patch prevents gcc generate code on ARM5e accessing 64 bit values, because gcc doesn't ensure they're 64 bit aligned.
>>>>> Accessing a 64 bit value from a 32 bit alignment causes segmentation faults.
>>>>>
>>>>> Patch is taken from https://github.com/archlinuxarm/PKGBUILDs/blob/master/core/gcc/0001-ARMv5-disable-LDRD-STRD.patch
>>>>
>>>> Can you try using -mfix-cortex-m3-ldrd, does that help ?
>>>
>>> Well, I have a marvel-kirkwood - I'll give it a shot but don't know whether it works at all.
>>>
>>>> The patch is a workaround to gcc, as it seems real problem is in defining instruction constraints
>>>> please open a gcc bug for this.
>>>
>>> Dunno whether I want to have all that stress. When neither ARCH linux guys did that nor Icedtea
>>> maintainers, how could I succeed there?
>>>
>>> Best I can do is (if -mfix-cortex-m3-ldrd doesn't help), keep it private. When -mfix-cortex-m3-ldrd
>>> helps, is adding it to ./meta/conf/machine/include/tune-arm926ejs.inc a sane approach?
>>
>> This might be OK. Although I would like to see it fixed in right way in gcc, we don’t want to carry patches
>> that are not accepted upstream. We already have 60 odd patches and we should reduce them if possible.
>
> I tried the approach applying -mfix-cortex-m3-ldrd to TUNE_CCARGS for armv5+dsp - and it generates
> ldrd instructions anyway. Accepted upstream or not, I have no time to get the right upstream fix.
>
> So I keep it private.
>
> During hacking on the fix in meta/conf/machine/ I recognized that the expected thump suffix in meta/conf/machine/include/tune-arm926ejs.inc
> isn't there and modified the line as
>
> diff --git a/meta/conf/machine/include/tune-arm926ejs.inc b/meta/conf/machine/include/tune-arm926ejs.inc
> index 335e4e0..a310c9d 100644
> --- a/meta/conf/machine/include/tune-arm926ejs.inc
> +++ b/meta/conf/machine/include/tune-arm926ejs.inc
> @@ -8,4 +8,4 @@ TUNE_CCARGS .= "${@bb.utils.contains("TUNE_FEATURES", "arm926ejs", " -mtune=arm9
>  AVAILTUNES += "arm926ejs"
>  ARMPKGARCH_tune-arm926ejs = "arm926ejs"
>  TUNE_FEATURES_tune-arm926ejs = "${TUNE_FEATURES_tune-armv5te} arm926ejs"
> -PACKAGE_EXTRA_ARCHS_tune-arm926ejs = "${PACKAGE_EXTRA_ARCHS_tune-armv5te} arm926ejste"
> +PACKAGE_EXTRA_ARCHS_tune-arm926ejs = "${PACKAGE_EXTRA_ARCHS_tune-armv5te} arm926ejste arm926ejse"
>

this is ok ?

> Want to have a patch for that or shall I keep it private, too?
>
> Cheers
> --
> Jens Rehsack - rehsack@gmail.com
>

Re: [PATCH 2/2] gcc: Avoid non-aligned access for ARM5e

[Jens Rehsack]

> Am 12.10.2015 um 18:32 schrieb Khem Raj <raj.khem@gmail.com>:
> 
> On Mon, Oct 12, 2015 at 9:01 AM, Jens Rehsack <rehsack@gmail.com> wrote:
>> 
>>> Am 08.10.2015 um 18:40 schrieb Khem Raj <raj.khem@gmail.com>:
>> [...]
>> During hacking on the fix in meta/conf/machine/ I recognized that the expected thump suffix in meta/conf/machine/include/tune-arm926ejs.inc
>> isn't there and modified the line as
>> 
>> diff --git a/meta/conf/machine/include/tune-arm926ejs.inc b/meta/conf/machine/include/tune-arm926ejs.inc
>> index 335e4e0..a310c9d 100644
>> --- a/meta/conf/machine/include/tune-arm926ejs.inc
>> +++ b/meta/conf/machine/include/tune-arm926ejs.inc
>> @@ -8,4 +8,4 @@ TUNE_CCARGS .= "${@bb.utils.contains("TUNE_FEATURES", "arm926ejs", " -mtune=arm9
>> AVAILTUNES += "arm926ejs"
>> ARMPKGARCH_tune-arm926ejs = "arm926ejs"
>> TUNE_FEATURES_tune-arm926ejs = "${TUNE_FEATURES_tune-armv5te} arm926ejs"
>> -PACKAGE_EXTRA_ARCHS_tune-arm926ejs = "${PACKAGE_EXTRA_ARCHS_tune-armv5te} arm926ejste"
>> +PACKAGE_EXTRA_ARCHS_tune-arm926ejs = "${PACKAGE_EXTRA_ARCHS_tune-armv5te} arm926ejste arm926ejse"
>> 
> 
> this is ok ?

From 

TUNE_FEATURES="${TUNE_FEATURES_tune-${DEFAULTTUNE}}"

is ok probably the wrong question ;)

Setting DEFAULTTUNE to arm926ejs results in expanding PACKAGE_ARCH from TUNE_PKGARCH which is combined via
"${ARMPKGARCH}${ARMPKGSFX_THUMB}${ARMPKGSFX_DSP}${ARMPKGSFX_EABI}${ARMPKGSFX_ENDIAN}${ARMPKGSFX_FPU}" in
meta/conf/machine/include/arm/arch-arm.inc:12

Since ARMPKGSFX_THUMB is set to "t" (or "t2" on ARMv6+) only when thumb code should be generated and
ARMPKGSFX_DSP is set to "e" when DSP is featured, both have to be recognized.

The entire construct doesn't feel "ok'ish" - but it works and is more or less reliable ...

Cheers
-- 
Jens Rehsack - rehsack@gmail.com

Re: [PATCH 2/2] gcc: Avoid non-aligned access for ARM5e

[Otavio Salvador]

Hello Jens and Khem,

On Mon, Oct 12, 2015 at 2:54 PM, Jens Rehsack <rehsack@gmail.com> wrote:
>> Am 12.10.2015 um 18:32 schrieb Khem Raj <raj.khem@gmail.com>:
>> On Mon, Oct 12, 2015 at 9:01 AM, Jens Rehsack <rehsack@gmail.com> wrote:
>>>> Am 08.10.2015 um 18:40 schrieb Khem Raj <raj.khem@gmail.com>:
>>> [...]
>>> During hacking on the fix in meta/conf/machine/ I recognized that the expected thump suffix in meta/conf/machine/include/tune-arm926ejs.inc
>>> isn't there and modified the line as
>>>
>>> diff --git a/meta/conf/machine/include/tune-arm926ejs.inc b/meta/conf/machine/include/tune-arm926ejs.inc
>>> index 335e4e0..a310c9d 100644
>>> --- a/meta/conf/machine/include/tune-arm926ejs.inc
>>> +++ b/meta/conf/machine/include/tune-arm926ejs.inc
>>> @@ -8,4 +8,4 @@ TUNE_CCARGS .= "${@bb.utils.contains("TUNE_FEATURES", "arm926ejs", " -mtune=arm9
>>> AVAILTUNES += "arm926ejs"
>>> ARMPKGARCH_tune-arm926ejs = "arm926ejs"
>>> TUNE_FEATURES_tune-arm926ejs = "${TUNE_FEATURES_tune-armv5te} arm926ejs"
>>> -PACKAGE_EXTRA_ARCHS_tune-arm926ejs = "${PACKAGE_EXTRA_ARCHS_tune-armv5te} arm926ejste"
>>> +PACKAGE_EXTRA_ARCHS_tune-arm926ejs = "${PACKAGE_EXTRA_ARCHS_tune-armv5te} arm926ejste arm926ejse"
>>>
>>
>> this is ok ?
>
> From
>
> TUNE_FEATURES="${TUNE_FEATURES_tune-${DEFAULTTUNE}}"
>
> is ok probably the wrong question ;)
>
> Setting DEFAULTTUNE to arm926ejs results in expanding PACKAGE_ARCH from TUNE_PKGARCH which is combined via
> "${ARMPKGARCH}${ARMPKGSFX_THUMB}${ARMPKGSFX_DSP}${ARMPKGSFX_EABI}${ARMPKGSFX_ENDIAN}${ARMPKGSFX_FPU}" in
> meta/conf/machine/include/arm/arch-arm.inc:12
>
> Since ARMPKGSFX_THUMB is set to "t" (or "t2" on ARMv6+) only when thumb code should be generated and
> ARMPKGSFX_DSP is set to "e" when DSP is featured, both have to be recognized.
>
> The entire construct doesn't feel "ok'ish" - but it works and is more or less reliable ...

I understand Khem is excitant to include more non-upstreamed patches
into GCC but I would prefer to have it included and Java working fine
than a local patch applied forever. :-(

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

Re: [PATCH 2/2] gcc: Avoid non-aligned access for ARM5e

[Phil Blundell]

On Tue, 2015-10-13 at 08:45 -0300, Otavio Salvador wrote:
> I understand Khem is excitant to include more non-upstreamed patches
> into GCC but I would prefer to have it included and Java working fine
> than a local patch applied forever. :-(

Maybe you'd like to file a bug against upstream GCC in that case.  I
think Khem already suggested this to Jens but he said he had "no time"
to do that.

p.

Re: [PATCH 2/2] gcc: Avoid non-aligned access for ARM5e

[Otavio Salvador]

On Tue, Oct 13, 2015 at 9:07 AM, Phil Blundell <pb@pbcl.net> wrote:
> On Tue, 2015-10-13 at 08:45 -0300, Otavio Salvador wrote:
>> I understand Khem is excitant to include more non-upstreamed patches
>> into GCC but I would prefer to have it included and Java working fine
>> than a local patch applied forever. :-(
>
> Maybe you'd like to file a bug against upstream GCC in that case.  I
> think Khem already suggested this to Jens but he said he had "no time"
> to do that.

We don't require upstream bugzilla items for patches in all recipes so
GCC should not be an exception. Sorry but I don't buy this argument
and I think the patch is clear and short enough to go in.

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

Re: [PATCH 2/2] gcc: Avoid non-aligned access for ARM5e

[Phil Blundell]

On Tue, 2015-10-13 at 09:18 -0300, Otavio Salvador wrote:
> On Tue, Oct 13, 2015 at 9:07 AM, Phil Blundell <pb@pbcl.net> wrote:
> > On Tue, 2015-10-13 at 08:45 -0300, Otavio Salvador wrote:
> > > I understand Khem is excitant to include more non-upstreamed
> > > patches
> > > into GCC but I would prefer to have it included and Java working
> > > fine
> > > than a local patch applied forever. :-(
> > 
> > Maybe you'd like to file a bug against upstream GCC in that case. 
> >  I
> > think Khem already suggested this to Jens but he said he had "no
> > time"
> > to do that.
> 
> We don't require upstream bugzilla items for patches in all recipes 
> so GCC should not be an exception. Sorry but I don't buy this 
> argument and I think the patch is clear and short enough to go in.

GCC is not necessarily an exception to that rule.  If the patch was a
correct and proper fix for the bug then I would be happy to see it
installed even without an upstream bugzilla entry.  But that is not the
case here: the patch is, at best, a hack which happens to work around
the problem in some circumstances.  It is not clear what the underlying
bug is, or whether the patch is sufficient to avoid the bug in all
cases (i.e. whether it might in fact happen for ARMv6 as well).  

Without knowing more about the underlying GCC bug it will be very diffi
cult to determine when the actual bug has been fixed and it is safe to
remove the patch, in which case we might end up carrying the patch
around forever.

Nor is the patch entirely harmless: turning off LDRD/STRD instructions
across the board will cause code size and performance to get slightly
worse for all users of the compiler.

If none of the proponents of the patch are prepared to make the small
amount of effort required to file an upstream bug then it's not at all
obvious to me that oe-core as a project should accept these various
burdens associated with installing this patch.

p.

Re: [PATCH 2/2] gcc: Avoid non-aligned access for ARM5e

[Khem Raj]

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


> On Oct 13, 2015, at 4:45 AM, Otavio Salvador <otavio.salvador@ossystems.com.br> wrote:
> 
> Hello Jens and Khem,
> 
> On Mon, Oct 12, 2015 at 2:54 PM, Jens Rehsack <rehsack@gmail.com> wrote:
>>> Am 12.10.2015 um 18:32 schrieb Khem Raj <raj.khem@gmail.com>:
>>> On Mon, Oct 12, 2015 at 9:01 AM, Jens Rehsack <rehsack@gmail.com> wrote:
>>>>> Am 08.10.2015 um 18:40 schrieb Khem Raj <raj.khem@gmail.com>:
>>>> [...]
>>>> During hacking on the fix in meta/conf/machine/ I recognized that the expected thump suffix in meta/conf/machine/include/tune-arm926ejs.inc
>>>> isn't there and modified the line as
>>>> 
>>>> diff --git a/meta/conf/machine/include/tune-arm926ejs.inc b/meta/conf/machine/include/tune-arm926ejs.inc
>>>> index 335e4e0..a310c9d 100644
>>>> --- a/meta/conf/machine/include/tune-arm926ejs.inc
>>>> +++ b/meta/conf/machine/include/tune-arm926ejs.inc
>>>> @@ -8,4 +8,4 @@ TUNE_CCARGS .= "${@bb.utils.contains("TUNE_FEATURES", "arm926ejs", " -mtune=arm9
>>>> AVAILTUNES += "arm926ejs"
>>>> ARMPKGARCH_tune-arm926ejs = "arm926ejs"
>>>> TUNE_FEATURES_tune-arm926ejs = "${TUNE_FEATURES_tune-armv5te} arm926ejs"
>>>> -PACKAGE_EXTRA_ARCHS_tune-arm926ejs = "${PACKAGE_EXTRA_ARCHS_tune-armv5te} arm926ejste"
>>>> +PACKAGE_EXTRA_ARCHS_tune-arm926ejs = "${PACKAGE_EXTRA_ARCHS_tune-armv5te} arm926ejste arm926ejse"
>>>> 
>>> 
>>> this is ok ?
>> 
>> From
>> 
>> TUNE_FEATURES="${TUNE_FEATURES_tune-${DEFAULTTUNE}}"
>> 
>> is ok probably the wrong question ;)
>> 
>> Setting DEFAULTTUNE to arm926ejs results in expanding PACKAGE_ARCH from TUNE_PKGARCH which is combined via
>> "${ARMPKGARCH}${ARMPKGSFX_THUMB}${ARMPKGSFX_DSP}${ARMPKGSFX_EABI}${ARMPKGSFX_ENDIAN}${ARMPKGSFX_FPU}" in
>> meta/conf/machine/include/arm/arch-arm.inc:12
>> 
>> Since ARMPKGSFX_THUMB is set to "t" (or "t2" on ARMv6+) only when thumb code should be generated and
>> ARMPKGSFX_DSP is set to "e" when DSP is featured, both have to be recognized.
>> 
>> The entire construct doesn't feel "ok'ish" - but it works and is more or less reliable ...
> 
> I understand Khem is excitant to include more non-upstreamed patches
> into GCC but I would prefer to have it included and Java working fine
> than a local patch applied forever. :-(

All I am asking is to open a bug with gcc. So to get some opinion on impact of patch, workaround of this nature can
be degarading code generation, we do not have any report on that. So that my concern was



[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 211 bytes --]

Re: [PATCH 2/2] gcc: Avoid non-aligned access for ARM5e

[Jens Rehsack]

> Am 13.10.2015 um 18:49 schrieb Khem Raj <raj.khem@gmail.com>:
> 
>> 
>> On Oct 13, 2015, at 4:45 AM, Otavio Salvador <otavio.salvador@ossystems.com.br> wrote:
>> 
>> Hello Jens and Khem,
>> 
>> On Mon, Oct 12, 2015 at 2:54 PM, Jens Rehsack <rehsack@gmail.com> wrote:
>>>> Am 12.10.2015 um 18:32 schrieb Khem Raj <raj.khem@gmail.com>:
>>>> On Mon, Oct 12, 2015 at 9:01 AM, Jens Rehsack <rehsack@gmail.com> wrote:
>>>>>> Am 08.10.2015 um 18:40 schrieb Khem Raj <raj.khem@gmail.com>:
>>>>> [...]
>>>>> During hacking on the fix in meta/conf/machine/ I recognized that the expected thump suffix in meta/conf/machine/include/tune-arm926ejs.inc
>>>>> isn't there and modified the line as
>>>>> 
>>>>> diff --git a/meta/conf/machine/include/tune-arm926ejs.inc b/meta/conf/machine/include/tune-arm926ejs.inc
>>>>> index 335e4e0..a310c9d 100644
>>>>> --- a/meta/conf/machine/include/tune-arm926ejs.inc
>>>>> +++ b/meta/conf/machine/include/tune-arm926ejs.inc
>>>>> @@ -8,4 +8,4 @@ TUNE_CCARGS .= "${@bb.utils.contains("TUNE_FEATURES", "arm926ejs", " -mtune=arm9
>>>>> AVAILTUNES += "arm926ejs"
>>>>> ARMPKGARCH_tune-arm926ejs = "arm926ejs"
>>>>> TUNE_FEATURES_tune-arm926ejs = "${TUNE_FEATURES_tune-armv5te} arm926ejs"
>>>>> -PACKAGE_EXTRA_ARCHS_tune-arm926ejs = "${PACKAGE_EXTRA_ARCHS_tune-armv5te} arm926ejste"
>>>>> +PACKAGE_EXTRA_ARCHS_tune-arm926ejs = "${PACKAGE_EXTRA_ARCHS_tune-armv5te} arm926ejste arm926ejse"
>>>>> 
>>>> 
>>>> this is ok ?
>>> 
>>> From
>>> 
>>> TUNE_FEATURES="${TUNE_FEATURES_tune-${DEFAULTTUNE}}"
>>> 
>>> is ok probably the wrong question ;)
>>> 
>>> Setting DEFAULTTUNE to arm926ejs results in expanding PACKAGE_ARCH from TUNE_PKGARCH which is combined via
>>> "${ARMPKGARCH}${ARMPKGSFX_THUMB}${ARMPKGSFX_DSP}${ARMPKGSFX_EABI}${ARMPKGSFX_ENDIAN}${ARMPKGSFX_FPU}" in
>>> meta/conf/machine/include/arm/arch-arm.inc:12
>>> 
>>> Since ARMPKGSFX_THUMB is set to "t" (or "t2" on ARMv6+) only when thumb code should be generated and
>>> ARMPKGSFX_DSP is set to "e" when DSP is featured, both have to be recognized.
>>> 
>>> The entire construct doesn't feel "ok'ish" - but it works and is more or less reliable ...
>> 
>> I understand Khem is excitant to include more non-upstreamed patches
>> into GCC but I would prefer to have it included and Java working fine
>> than a local patch applied forever. :-(
> 
> All I am asking is to open a bug with gcc. So to get some opinion on impact of patch, workaround of this nature can
> be degarading code generation, we do not have any report on that. So that my concern was

I will check how difficult it is and report if it's a low hangin' fruit ;)

Cheers
-- 
Jens Rehsack - rehsack@gmail.com