| Message ID | 20250521115945.3629052-1-sundeep.kokkonda@windriver.com |
|---|---|
| State | Under Review |
| Delegated to: | Steve Sakoman |
| Headers | show |
| Series | [kirkstone,v2] gcc: AArch64 - Fix strict-align cpymem/setmem | expand |
On 2025-05-21 7:59 a.m., sundeep.kokkonda@windriver.com wrote: It's good practice to CC the person who asked for a v2. I've added Gyorgy to this thread. > 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) > > This patch includes only the alignment-related changes in the aarch64.c file. > Changes to aarch64.md were skipped due to non-essential impact and potential incompatibility. That's better than not mentioned what parts of the patch you dropped but it's doesn't *explain* why that change isn't needed or I'm missing the point! ;-) Please dig a bit deeper and explain: 1. what TARGET_MOPS means and does and maybe what the .md file's purpose is, 2. why was it removed in the gcc-12 commit, 3. why it's safe to ignore that change for gcc-11, 4. Since you are not backporting the MOPS part, it might be nice to point that out in each part of the patch that mentions MOPS. Maybe that's overkill and you just need to explain in the commit log but think about someone who is just looking at the patch and do what makes sense to you. I can almost understand things better now but I'll let you present it, step by step, in v3. > All changes and outputs have been verified by the author. Is there a test case that covers all of this that starts passing with this change? Please explain how are you testing? ../Randy I looked at this a bit and found gcc/ChangeLog-2021 that talks about the introduction of TARGET_MOPS. This was done in gcc-12 and later as you have hinted at. ❯ git blame gcc/ChangeLog-2021 | rg TARGET_MOPS d04ae83244d3 (Jakub Jelinek 2022-01-03 10:18:16 +0100 1214) (aarch64_expand_setmem): Adjust for TARGET_MOPS. d04ae83244d3 (Jakub Jelinek 2022-01-03 10:18:16 +0100 1215) * config/aarch64/aarch64.h (CLEAR_RATIO): Adjust for TARGET_MOPS. d04ae83244d3 (Jakub Jelinek 2022-01-03 10:18:16 +0100 1219) (setmemdi): Adjust for TARGET_MOPS. d04ae83244d3 (Jakub Jelinek 2022-01-03 10:18:16 +0100 1238) (TARGET_MOPS): Define. d04ae83244d3 (Jakub Jelinek 2022-01-03 10:18:16 +0100 1239) (MOVE_RATIO): Adjust for TARGET_MOPS. d04ae83244d3 (Jakub Jelinek 2022-01-03 10:18:16 +0100 1242) (cpymemdi): Adjust for TARGET_MOPS. 2021-12-13 Kyrylo Tkachov <kyrylo.tkachov@arm.com> * config/aarch64/aarch64.c (aarch64_expand_setmem_mops): Define. (aarch64_expand_setmem): Adjust for TARGET_MOPS. * config/aarch64/aarch64.h (CLEAR_RATIO): Adjust for TARGET_MOPS. (SET_RATIO): Likewise. * config/aarch64/aarch64.md ("unspec"): Add UNSPEC_SETMEM. (aarch64_setmemdi): Define. (setmemdi): Adjust for TARGET_MOPS. * config/aarch64/aarch64.opt (aarch64-mops-memset-size-threshold): New param. ... 2021-12-13 Kyrylo Tkachov <kyrylo.tkachov@arm.com> * config/aarch64/aarch64-option-extensions.def (mops): Define. * config/aarch64/aarch64.c (aarch64_expand_cpymem_mops): Define. (aarch64_expand_cpymem): Define. * config/aarch64/aarch64.h (AARCH64_FL_MOPS): Define. (AARCH64_ISA_MOPS): Define. (TARGET_MOPS): Define. (MOVE_RATIO): Adjust for TARGET_MOPS. * config/aarch64/aarch64.md ("unspec"): Add UNSPEC_CPYMEM. (aarch64_cpymemdi): New pattern. (cpymemdi): Adjust for TARGET_MOPS. * config/aarch64/aarch64.opt (aarch64-mops-memcpy-size-threshol): New param. * doc/invoke.texi (AArch64 Options): Document +mops. > > 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 > > 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);
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);