diff mbox series

[kirkstone] gcc: AArch64 - Fix strict-align cpymem/setmem

Message ID 20250521090916.974418-1-sundeep.kokkonda@windriver.com
State Superseded
Delegated to: Steve Sakoman
Headers show
Series [kirkstone] gcc: AArch64 - Fix strict-align cpymem/setmem | expand

Commit Message

Sundeep KOKKONDA May 21, 2025, 9:09 a.m. UTC
From: Sundeep KOKKONDA <sundeep.kokkonda@windriver.com>

The cpymemdi/setmemdi implementation doesn't fully support strict alignment.
Block the expansion if the alignment is less than 16 with STRICT_ALIGNMENT.
Clean up the condition when to use MOPS. (PR103100)

Upstream-Status: Backport [https://gcc.gnu.org/cgit/gcc/commit/?id=b9d16d8361a9e3a82a2f21e759e760d235d43322]

Signed-off-by: Sundeep KOKKONDA <sundeep.kokkonda@windriver.com>
---
 meta/recipes-devtools/gcc/gcc-11.5.inc        |  1 +
 ...rch64-fix-strict-align-cpymem-setmem.patch | 45 +++++++++++++++++++
 2 files changed, 46 insertions(+)
 create mode 100644 meta/recipes-devtools/gcc/gcc/0032-gcc-aarch64-fix-strict-align-cpymem-setmem.patch

Comments

Gyorgy Sarvari May 21, 2025, 9:53 a.m. UTC | #1
On 5/21/25 11:09, Sundeep KOKKONDA via lists.openembedded.org wrote:
> From: Sundeep KOKKONDA <sundeep.kokkonda@windriver.com>
>
> The cpymemdi/setmemdi implementation doesn't fully support strict alignment.
> Block the expansion if the alignment is less than 16 with STRICT_ALIGNMENT.
> Clean up the condition when to use MOPS. (PR103100)
>
> Upstream-Status: Backport [https://gcc.gnu.org/cgit/gcc/commit/?id=b9d16d8361a9e3a82a2f21e759e760d235d43322]
Is this a partial backport of the commit?
I wouldn't expect the backported patch to match the original 1-to-1, but
the original commit also contains some changes of the machine
description, which is missing from this patch. Is that not required?
>
> Signed-off-by: Sundeep KOKKONDA <sundeep.kokkonda@windriver.com>
> ---
>  meta/recipes-devtools/gcc/gcc-11.5.inc        |  1 +
>  ...rch64-fix-strict-align-cpymem-setmem.patch | 45 +++++++++++++++++++
>  2 files changed, 46 insertions(+)
>  create mode 100644 meta/recipes-devtools/gcc/gcc/0032-gcc-aarch64-fix-strict-align-cpymem-setmem.patch
>
> diff --git a/meta/recipes-devtools/gcc/gcc-11.5.inc b/meta/recipes-devtools/gcc/gcc-11.5.inc
> index f17ec9da5c..1e8371b2bd 100644
> --- a/meta/recipes-devtools/gcc/gcc-11.5.inc
> +++ b/meta/recipes-devtools/gcc/gcc-11.5.inc
> @@ -60,6 +60,7 @@ SRC_URI = "\
>             file://0029-Fix-install-path-of-linux64.h.patch \
>             file://0030-rust-recursion-limit.patch \
>             file://0031-gcc-sanitizers-fix.patch \
> +           file://0032-gcc-aarch64-fix-strict-align-cpymem-setmem.patch \
>             file://0001-CVE-2021-42574.patch \
>             file://0002-CVE-2021-42574.patch \
>             file://0003-CVE-2021-42574.patch \
> diff --git a/meta/recipes-devtools/gcc/gcc/0032-gcc-aarch64-fix-strict-align-cpymem-setmem.patch b/meta/recipes-devtools/gcc/gcc/0032-gcc-aarch64-fix-strict-align-cpymem-setmem.patch
> new file mode 100644
> index 0000000000..4c2d827799
> --- /dev/null
> +++ b/meta/recipes-devtools/gcc/gcc/0032-gcc-aarch64-fix-strict-align-cpymem-setmem.patch
> @@ -0,0 +1,45 @@
> +gcc: AArch64 - Fix strict-align cpymem/setmem
> +
> +The cpymemdi/setmemdi implementation doesn't fully support strict alignment.
> +Block the expansion if the alignment is less than 16 with STRICT_ALIGNMENT.
> +Clean up the condition when to use MOPS.
> +
> +Upstream-Status: Backport [https://gcc.gnu.org/cgit/gcc/commit/?id=b9d16d8361a9e3a82a2f21e759e760d235d43322]
> +
> +Signed-off-by: Wilco Dijkstra <wilco.dijkstra@arm.com>
> +Signed-off-by: Sundeep KOKKONDA <sundeep.kokkonda@windriver.com>
> +---
> +--- a/gcc/config/aarch64/aarch64.c	2025-05-08 20:40:10.969865898 -0700
> ++++ b/gcc/config/aarch64/aarch64.c	2025-05-13 23:11:07.006796627 -0700
> +@@ -23621,14 +23621,15 @@
> +   int mode_bits;
> +   rtx dst = operands[0];
> +   rtx src = operands[1];
> ++  unsigned align = UINTVAL (operands[3]);
> +   rtx base;
> +   machine_mode cur_mode = BLKmode;
> + 
> +   /* Only expand fixed-size copies.  */
> +-  if (!CONST_INT_P (operands[2]))
> ++  if (!CONST_INT_P (operands[2]) || (STRICT_ALIGNMENT && align < 16))
> +     return false;
> + 
> +-  unsigned HOST_WIDE_INT size = INTVAL (operands[2]);
> ++  unsigned HOST_WIDE_INT size = UINTVAL (operands[2]);
> + 
> +   /* Inline up to 256 bytes when optimizing for speed.  */
> +   unsigned HOST_WIDE_INT max_copy_size = 256;
> +@@ -23750,11 +23751,12 @@
> +   unsigned HOST_WIDE_INT len;
> +   rtx dst = operands[0];
> +   rtx val = operands[2], src;
> ++  unsigned align = UINTVAL (operands[3]);
> +   rtx base;
> +   machine_mode cur_mode = BLKmode, next_mode;
> + 
> +   /* We can't do anything smart if the amount to copy is not constant.  */
> +-  if (!CONST_INT_P (operands[1]))
> ++  if (!CONST_INT_P (operands[1]) || (STRICT_ALIGNMENT && align < 16))
> +     return false;
> + 
> +   bool speed_p = !optimize_function_for_size_p (cfun);
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#217009): https://lists.openembedded.org/g/openembedded-core/message/217009
> Mute This Topic: https://lists.openembedded.org/mt/113226173/6084445
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [skandigraun@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Sundeep KOKKONDA May 21, 2025, 10:29 a.m. UTC | #2
On 21-May-25 15:23, Gyorgy Sarvari wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
> On 5/21/25 11:09, Sundeep KOKKONDA via lists.openembedded.org wrote:
>> From: Sundeep KOKKONDA <sundeep.kokkonda@windriver.com>
>>
>> The cpymemdi/setmemdi implementation doesn't fully support strict alignment.
>> Block the expansion if the alignment is less than 16 with STRICT_ALIGNMENT.
>> Clean up the condition when to use MOPS. (PR103100)
>>
>> Upstream-Status: Backport [https://gcc.gnu.org/cgit/gcc/commit/?id=b9d16d8361a9e3a82a2f21e759e760d235d43322]
> Is this a partial backport of the commit?
> I wouldn't expect the backported patch to match the original 1-to-1, but
> the original commit also contains some changes of the machine
> description, which is missing from this patch. Is that not required?

The original commit was for gcc-12 and above and the gcc community is 
not going to do downward backport of the patch for gcc-11.

The original patch contains 'alignment part in .c' & .md file changes. 
The changes in machine description files (which are not mandatory as per 
the author) are causing some failures on other modules while building 
the image due to missing RTL instructions (gcc compiles successfully). 
So, I took only the alignment part of the patch. This cherrypicked patch 
& it's output correctness I get it verified with the patch author.


Thanks,

Sundeep K.

>> Signed-off-by: Sundeep KOKKONDA <sundeep.kokkonda@windriver.com>
>> ---
>>   meta/recipes-devtools/gcc/gcc-11.5.inc        |  1 +
>>   ...rch64-fix-strict-align-cpymem-setmem.patch | 45 +++++++++++++++++++
>>   2 files changed, 46 insertions(+)
>>   create mode 100644 meta/recipes-devtools/gcc/gcc/0032-gcc-aarch64-fix-strict-align-cpymem-setmem.patch
>>
>> diff --git a/meta/recipes-devtools/gcc/gcc-11.5.inc b/meta/recipes-devtools/gcc/gcc-11.5.inc
>> index f17ec9da5c..1e8371b2bd 100644
>> --- a/meta/recipes-devtools/gcc/gcc-11.5.inc
>> +++ b/meta/recipes-devtools/gcc/gcc-11.5.inc
>> @@ -60,6 +60,7 @@ SRC_URI = "\
>>              file://0029-Fix-install-path-of-linux64.h.patch \
>>              file://0030-rust-recursion-limit.patch \
>>              file://0031-gcc-sanitizers-fix.patch \
>> +           file://0032-gcc-aarch64-fix-strict-align-cpymem-setmem.patch \
>>              file://0001-CVE-2021-42574.patch \
>>              file://0002-CVE-2021-42574.patch \
>>              file://0003-CVE-2021-42574.patch \
>> diff --git a/meta/recipes-devtools/gcc/gcc/0032-gcc-aarch64-fix-strict-align-cpymem-setmem.patch b/meta/recipes-devtools/gcc/gcc/0032-gcc-aarch64-fix-strict-align-cpymem-setmem.patch
>> new file mode 100644
>> index 0000000000..4c2d827799
>> --- /dev/null
>> +++ b/meta/recipes-devtools/gcc/gcc/0032-gcc-aarch64-fix-strict-align-cpymem-setmem.patch
>> @@ -0,0 +1,45 @@
>> +gcc: AArch64 - Fix strict-align cpymem/setmem
>> +
>> +The cpymemdi/setmemdi implementation doesn't fully support strict alignment.
>> +Block the expansion if the alignment is less than 16 with STRICT_ALIGNMENT.
>> +Clean up the condition when to use MOPS.
>> +
>> +Upstream-Status: Backport [https://gcc.gnu.org/cgit/gcc/commit/?id=b9d16d8361a9e3a82a2f21e759e760d235d43322]
>> +
>> +Signed-off-by: Wilco Dijkstra <wilco.dijkstra@arm.com>
>> +Signed-off-by: Sundeep KOKKONDA <sundeep.kokkonda@windriver.com>
>> +---
>> +--- a/gcc/config/aarch64/aarch64.c   2025-05-08 20:40:10.969865898 -0700
>> ++++ b/gcc/config/aarch64/aarch64.c   2025-05-13 23:11:07.006796627 -0700
>> +@@ -23621,14 +23621,15 @@
>> +   int mode_bits;
>> +   rtx dst = operands[0];
>> +   rtx src = operands[1];
>> ++  unsigned align = UINTVAL (operands[3]);
>> +   rtx base;
>> +   machine_mode cur_mode = BLKmode;
>> +
>> +   /* Only expand fixed-size copies.  */
>> +-  if (!CONST_INT_P (operands[2]))
>> ++  if (!CONST_INT_P (operands[2]) || (STRICT_ALIGNMENT && align < 16))
>> +     return false;
>> +
>> +-  unsigned HOST_WIDE_INT size = INTVAL (operands[2]);
>> ++  unsigned HOST_WIDE_INT size = UINTVAL (operands[2]);
>> +
>> +   /* Inline up to 256 bytes when optimizing for speed.  */
>> +   unsigned HOST_WIDE_INT max_copy_size = 256;
>> +@@ -23750,11 +23751,12 @@
>> +   unsigned HOST_WIDE_INT len;
>> +   rtx dst = operands[0];
>> +   rtx val = operands[2], src;
>> ++  unsigned align = UINTVAL (operands[3]);
>> +   rtx base;
>> +   machine_mode cur_mode = BLKmode, next_mode;
>> +
>> +   /* We can't do anything smart if the amount to copy is not constant.  */
>> +-  if (!CONST_INT_P (operands[1]))
>> ++  if (!CONST_INT_P (operands[1]) || (STRICT_ALIGNMENT && align < 16))
>> +     return false;
>> +
>> +   bool speed_p = !optimize_function_for_size_p (cfun);
>>
>> -=-=-=-=-=-=-=-=-=-=-=-
>> Links: You receive all messages sent to this group.
>> View/Reply Online (#217009): https://lists.openembedded.org/g/openembedded-core/message/217009
>> Mute This Topic: https://lists.openembedded.org/mt/113226173/6084445
>> Group Owner: openembedded-core+owner@lists.openembedded.org
>> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [skandigraun@gmail.com]
>> -=-=-=-=-=-=-=-=-=-=-=-
>>
Gyorgy Sarvari May 21, 2025, 10:38 a.m. UTC | #3
On 5/21/25 12:29, Sundeep KOKKONDA wrote:
> On 21-May-25 15:23, Gyorgy Sarvari wrote:
>> CAUTION: This email comes from a non Wind River email account!
>> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>>
>> On 5/21/25 11:09, Sundeep KOKKONDA via lists.openembedded.org wrote:
>>> From: Sundeep KOKKONDA <sundeep.kokkonda@windriver.com>
>>>
>>> The cpymemdi/setmemdi implementation doesn't fully support strict alignment.
>>> Block the expansion if the alignment is less than 16 with STRICT_ALIGNMENT.
>>> Clean up the condition when to use MOPS. (PR103100)
>>>
>>> Upstream-Status: Backport [https://gcc.gnu.org/cgit/gcc/commit/?id=b9d16d8361a9e3a82a2f21e759e760d235d43322]
>> Is this a partial backport of the commit?
>> I wouldn't expect the backported patch to match the original 1-to-1, but
>> the original commit also contains some changes of the machine
>> description, which is missing from this patch. Is that not required?
> The original commit was for gcc-12 and above and the gcc community is 
> not going to do downward backport of the patch for gcc-11.
>
> The original patch contains 'alignment part in .c' & .md file changes. 
> The changes in machine description files (which are not mandatory as per 
> the author) are causing some failures on other modules while building 
> the image due to missing RTL instructions (gcc compiles successfully). 
> So, I took only the alignment part of the patch. This cherrypicked patch 
> & it's output correctness I get it verified with the patch author.
>
Got it, thank you for the clarification. Do you think you could add
something along these lines to the commit message also?

> Thanks,
>
> Sundeep K.
>
>>> Signed-off-by: Sundeep KOKKONDA <sundeep.kokkonda@windriver.com>
>>> ---
>>>   meta/recipes-devtools/gcc/gcc-11.5.inc        |  1 +
>>>   ...rch64-fix-strict-align-cpymem-setmem.patch | 45 +++++++++++++++++++
>>>   2 files changed, 46 insertions(+)
>>>   create mode 100644 meta/recipes-devtools/gcc/gcc/0032-gcc-aarch64-fix-strict-align-cpymem-setmem.patch
>>>
>>> diff --git a/meta/recipes-devtools/gcc/gcc-11.5.inc b/meta/recipes-devtools/gcc/gcc-11.5.inc
>>> index f17ec9da5c..1e8371b2bd 100644
>>> --- a/meta/recipes-devtools/gcc/gcc-11.5.inc
>>> +++ b/meta/recipes-devtools/gcc/gcc-11.5.inc
>>> @@ -60,6 +60,7 @@ SRC_URI = "\
>>>              file://0029-Fix-install-path-of-linux64.h.patch \
>>>              file://0030-rust-recursion-limit.patch \
>>>              file://0031-gcc-sanitizers-fix.patch \
>>> +           file://0032-gcc-aarch64-fix-strict-align-cpymem-setmem.patch \
>>>              file://0001-CVE-2021-42574.patch \
>>>              file://0002-CVE-2021-42574.patch \
>>>              file://0003-CVE-2021-42574.patch \
>>> diff --git a/meta/recipes-devtools/gcc/gcc/0032-gcc-aarch64-fix-strict-align-cpymem-setmem.patch b/meta/recipes-devtools/gcc/gcc/0032-gcc-aarch64-fix-strict-align-cpymem-setmem.patch
>>> new file mode 100644
>>> index 0000000000..4c2d827799
>>> --- /dev/null
>>> +++ b/meta/recipes-devtools/gcc/gcc/0032-gcc-aarch64-fix-strict-align-cpymem-setmem.patch
>>> @@ -0,0 +1,45 @@
>>> +gcc: AArch64 - Fix strict-align cpymem/setmem
>>> +
>>> +The cpymemdi/setmemdi implementation doesn't fully support strict alignment.
>>> +Block the expansion if the alignment is less than 16 with STRICT_ALIGNMENT.
>>> +Clean up the condition when to use MOPS.
>>> +
>>> +Upstream-Status: Backport [https://gcc.gnu.org/cgit/gcc/commit/?id=b9d16d8361a9e3a82a2f21e759e760d235d43322]
>>> +
>>> +Signed-off-by: Wilco Dijkstra <wilco.dijkstra@arm.com>
>>> +Signed-off-by: Sundeep KOKKONDA <sundeep.kokkonda@windriver.com>
>>> +---
>>> +--- a/gcc/config/aarch64/aarch64.c   2025-05-08 20:40:10.969865898 -0700
>>> ++++ b/gcc/config/aarch64/aarch64.c   2025-05-13 23:11:07.006796627 -0700
>>> +@@ -23621,14 +23621,15 @@
>>> +   int mode_bits;
>>> +   rtx dst = operands[0];
>>> +   rtx src = operands[1];
>>> ++  unsigned align = UINTVAL (operands[3]);
>>> +   rtx base;
>>> +   machine_mode cur_mode = BLKmode;
>>> +
>>> +   /* Only expand fixed-size copies.  */
>>> +-  if (!CONST_INT_P (operands[2]))
>>> ++  if (!CONST_INT_P (operands[2]) || (STRICT_ALIGNMENT && align < 16))
>>> +     return false;
>>> +
>>> +-  unsigned HOST_WIDE_INT size = INTVAL (operands[2]);
>>> ++  unsigned HOST_WIDE_INT size = UINTVAL (operands[2]);
>>> +
>>> +   /* Inline up to 256 bytes when optimizing for speed.  */
>>> +   unsigned HOST_WIDE_INT max_copy_size = 256;
>>> +@@ -23750,11 +23751,12 @@
>>> +   unsigned HOST_WIDE_INT len;
>>> +   rtx dst = operands[0];
>>> +   rtx val = operands[2], src;
>>> ++  unsigned align = UINTVAL (operands[3]);
>>> +   rtx base;
>>> +   machine_mode cur_mode = BLKmode, next_mode;
>>> +
>>> +   /* We can't do anything smart if the amount to copy is not constant.  */
>>> +-  if (!CONST_INT_P (operands[1]))
>>> ++  if (!CONST_INT_P (operands[1]) || (STRICT_ALIGNMENT && align < 16))
>>> +     return false;
>>> +
>>> +   bool speed_p = !optimize_function_for_size_p (cfun);
>>>
>>> -=-=-=-=-=-=-=-=-=-=-=-
>>> Links: You receive all messages sent to this group.
>>> View/Reply Online (#217009): https://lists.openembedded.org/g/openembedded-core/message/217009
>>> Mute This Topic: https://lists.openembedded.org/mt/113226173/6084445
>>> Group Owner: openembedded-core+owner@lists.openembedded.org
>>> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [skandigraun@gmail.com]
>>> -=-=-=-=-=-=-=-=-=-=-=-
>>>
diff mbox series

Patch

diff --git a/meta/recipes-devtools/gcc/gcc-11.5.inc b/meta/recipes-devtools/gcc/gcc-11.5.inc
index f17ec9da5c..1e8371b2bd 100644
--- a/meta/recipes-devtools/gcc/gcc-11.5.inc
+++ b/meta/recipes-devtools/gcc/gcc-11.5.inc
@@ -60,6 +60,7 @@  SRC_URI = "\
            file://0029-Fix-install-path-of-linux64.h.patch \
            file://0030-rust-recursion-limit.patch \
            file://0031-gcc-sanitizers-fix.patch \
+           file://0032-gcc-aarch64-fix-strict-align-cpymem-setmem.patch \
            file://0001-CVE-2021-42574.patch \
            file://0002-CVE-2021-42574.patch \
            file://0003-CVE-2021-42574.patch \
diff --git a/meta/recipes-devtools/gcc/gcc/0032-gcc-aarch64-fix-strict-align-cpymem-setmem.patch b/meta/recipes-devtools/gcc/gcc/0032-gcc-aarch64-fix-strict-align-cpymem-setmem.patch
new file mode 100644
index 0000000000..4c2d827799
--- /dev/null
+++ b/meta/recipes-devtools/gcc/gcc/0032-gcc-aarch64-fix-strict-align-cpymem-setmem.patch
@@ -0,0 +1,45 @@ 
+gcc: AArch64 - Fix strict-align cpymem/setmem
+
+The cpymemdi/setmemdi implementation doesn't fully support strict alignment.
+Block the expansion if the alignment is less than 16 with STRICT_ALIGNMENT.
+Clean up the condition when to use MOPS.
+
+Upstream-Status: Backport [https://gcc.gnu.org/cgit/gcc/commit/?id=b9d16d8361a9e3a82a2f21e759e760d235d43322]
+
+Signed-off-by: Wilco Dijkstra <wilco.dijkstra@arm.com>
+Signed-off-by: Sundeep KOKKONDA <sundeep.kokkonda@windriver.com>
+---
+--- a/gcc/config/aarch64/aarch64.c	2025-05-08 20:40:10.969865898 -0700
++++ b/gcc/config/aarch64/aarch64.c	2025-05-13 23:11:07.006796627 -0700
+@@ -23621,14 +23621,15 @@
+   int mode_bits;
+   rtx dst = operands[0];
+   rtx src = operands[1];
++  unsigned align = UINTVAL (operands[3]);
+   rtx base;
+   machine_mode cur_mode = BLKmode;
+ 
+   /* Only expand fixed-size copies.  */
+-  if (!CONST_INT_P (operands[2]))
++  if (!CONST_INT_P (operands[2]) || (STRICT_ALIGNMENT && align < 16))
+     return false;
+ 
+-  unsigned HOST_WIDE_INT size = INTVAL (operands[2]);
++  unsigned HOST_WIDE_INT size = UINTVAL (operands[2]);
+ 
+   /* Inline up to 256 bytes when optimizing for speed.  */
+   unsigned HOST_WIDE_INT max_copy_size = 256;
+@@ -23750,11 +23751,12 @@
+   unsigned HOST_WIDE_INT len;
+   rtx dst = operands[0];
+   rtx val = operands[2], src;
++  unsigned align = UINTVAL (operands[3]);
+   rtx base;
+   machine_mode cur_mode = BLKmode, next_mode;
+ 
+   /* We can't do anything smart if the amount to copy is not constant.  */
+-  if (!CONST_INT_P (operands[1]))
++  if (!CONST_INT_P (operands[1]) || (STRICT_ALIGNMENT && align < 16))
+     return false;
+ 
+   bool speed_p = !optimize_function_for_size_p (cfun);