On Wed, Jun 18, 2025 at 3:11 PM Cui, Lili <lili....@intel.com> wrote:
>
> From: Lili Cui <lili....@intel.com>
>
> Hi Uros,
>
> An assertion I added in shrink wrap separate V2 reports ICE when
> -fstack-clash-protection is enabled. The assertion should not be added here.
>
> I created a patch to remove 3 assertions and their associated code.
>
> 1. Reproduced PR120697 issue and solved the issue with this patch, also added
> a new test case for -fstack-clash-protection.
> 2. Recollected performance data with -fstack-clash-protection, there is no
> ICE and regressions.
> 3. Use this patch to build the latest Linux kernel and boot successfully.
> 4. Bootstrapped & regtested on x86-64-pc-linux-gnu.
>
> Ok for master?
>
> Thanks,
> Lili.
>
>
> gcc/ChangeLog:
>
> PR target/120697
> * config/i386/i386.cc (ix86_expand_prologue):
> Remove 3 assertions and associated code.
>
> gcc/testsuite/ChangeLog:
>
> PR target/120697
> * gcc.target/i386/stack-clash-protection.c: New test.
LGTM, but only in the sense that you are reverting parts of the original patch.
Thanks,
Uros.
> ---
> gcc/config/i386/i386.cc | 14 +-------------
> .../gcc.target/i386/stack-clash-protection.c | 19 +++++++++++++++++++
> 2 files changed, 20 insertions(+), 13 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/i386/stack-clash-protection.c
>
> diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc
> index 3824b533989..6dce7cdfdcb 100644
> --- a/gcc/config/i386/i386.cc
> +++ b/gcc/config/i386/i386.cc
> @@ -9234,10 +9234,9 @@ ix86_expand_prologue (void)
> the stack frame saving one cycle of the prologue. However, avoid
> doing this if we have to probe the stack; at least on x86_64 the
> stack probe can turn into a call that clobbers a red zone location.
> */
> - else if ((ix86_using_red_zone ()
> + else if (ix86_using_red_zone ()
> && (! TARGET_STACK_PROBE
> || frame.stack_pointer_offset < CHECK_STACK_LIMIT))
> - || crtl->shrink_wrapped_separate)
> {
> HOST_WIDE_INT allocate_offset;
> if (crtl->shrink_wrapped_separate)
> @@ -9253,11 +9252,6 @@ ix86_expand_prologue (void)
>
> ix86_emit_save_regs_using_mov (frame.reg_save_offset);
> int_registers_saved = true;
> -
> - if (ix86_using_red_zone ()
> - && (! TARGET_STACK_PROBE
> - || frame.stack_pointer_offset < CHECK_STACK_LIMIT))
> - cfun->machine->red_zone_used = true;
> }
> }
>
> @@ -9377,8 +9371,6 @@ ix86_expand_prologue (void)
> && flag_stack_clash_protection
> && !ix86_target_stack_probe ())
> {
> - gcc_assert (!crtl->shrink_wrapped_separate);
> -
> ix86_adjust_stack_and_probe (allocate, int_registers_saved, false);
> allocate = 0;
> }
> @@ -9389,8 +9381,6 @@ ix86_expand_prologue (void)
> {
> const HOST_WIDE_INT probe_interval = get_probe_interval ();
>
> - gcc_assert (!crtl->shrink_wrapped_separate);
> -
> if (STACK_CHECK_MOVING_SP)
> {
> if (crtl->is_leaf
> @@ -9447,8 +9437,6 @@ ix86_expand_prologue (void)
> else if (!ix86_target_stack_probe ()
> || frame.stack_pointer_offset < CHECK_STACK_LIMIT)
> {
> - gcc_assert (!crtl->shrink_wrapped_separate);
> -
> pro_epilogue_adjust_stack (stack_pointer_rtx, stack_pointer_rtx,
> GEN_INT (-allocate), -1,
> m->fs.cfa_reg == stack_pointer_rtx);
> diff --git a/gcc/testsuite/gcc.target/i386/stack-clash-protection.c
> b/gcc/testsuite/gcc.target/i386/stack-clash-protection.c
> new file mode 100644
> index 00000000000..5be28cb3ac7
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/stack-clash-protection.c
> @@ -0,0 +1,19 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fstack-clash-protection" } */
> +
> +int flag;
> +void open();
> +int getChar();
> +typedef enum { QUOTE } CharType;
> +typedef enum { UNQ } State;
> +CharType getCharType();
> +void expand() {
> + open();
> + if (flag)
> + return;
> + int ch = getChar();
> + State nextState = getCharType();
> + if (nextState)
> + while (ch)
> + ;
> +}
> --
> 2.34.1
>
