Openembedded Core Discussions
 help / color / mirror / Atom feed
From: Gyorgy Sarvari <skandigraun@gmail.com>
To: Sundeep KOKKONDA <Sundeep.Kokkonda@windriver.com>,
	openembedded-core@lists.openembedded.org
Cc: randy.macleod@windriver.com, steve@sakoman.com
Subject: Re: [OE-core] [kirkstone][PATCH] gcc: AArch64 - Fix strict-align cpymem/setmem
Date: Wed, 21 May 2025 12:38:15 +0200	[thread overview]
Message-ID: <d50b735b-d894-4c1b-b902-5d6ffca36edb@gmail.com> (raw)
In-Reply-To: <5e1efec5-8840-42ee-b745-a817a70678df@windriver.com>

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]
>>> -=-=-=-=-=-=-=-=-=-=-=-
>>>



      reply	other threads:[~2025-05-21 10:38 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-21  9:09 [kirkstone][PATCH] gcc: AArch64 - Fix strict-align cpymem/setmem sundeep.kokkonda
2025-05-21  9:53 ` [OE-core] " Gyorgy Sarvari
2025-05-21 10:29   ` Sundeep KOKKONDA
2025-05-21 10:38     ` Gyorgy Sarvari [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d50b735b-d894-4c1b-b902-5d6ffca36edb@gmail.com \
    --to=skandigraun@gmail.com \
    --cc=Sundeep.Kokkonda@windriver.com \
    --cc=openembedded-core@lists.openembedded.org \
    --cc=randy.macleod@windriver.com \
    --cc=steve@sakoman.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox