diff mbox series

[v3,2/2] Correct x86: Call ix86_access_stack_p only for larger alignment

Message ID CAMe9rOpz_daLJoXU=VD_J83JTkshCidaiGZ+A7OeP=FXcM=0uw@mail.gmail.com
State New
Headers
Series [v3,1/2] Update x86: Call ix86_access_stack_p only for larger alignment |

Commit Message

H.J. Lu April 7, 2026, 12:02 p.m. UTC 
  commit f511bf93f947199a9f9099fee87b7052e5515fb9
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Sun Mar 29 14:30:28 2026 -0700

    x86: Call ix86_access_stack_p only for larger alignment

has 2 issues:

1. It didn't exclude memory with SYMBOLIC_CONST when checking for stack
access.
2. It replaced:

  HARD_REG_SET hard_stack_slot_access = stack_slot_access;
  ...
      bool symbolic_const_load_p = false;

      if (!TEST_HARD_REG_BIT (hard_stack_slot_access, regno))
...
        symbolic_const_load_p = true;
      ...

      if (!symbolic_const_load_p
          || ix86_access_stack_p...)
call ix86_update_stack_alignment.

with

      if (ix86_need_alignment_p...
          && ix86_access_stack_p...)
call ix86_update_stack_alignment.

Since ix86_access_stack_p excludes registers on hard_stack_slot_access,
stack alignment isn't updated for registers on hard_stack_slot_access.
Instead, we should do

      if (ix86_need_alignment_p...
          && (TEST_HARD_REG_BIT (hard_stack_slot_access, regno)
              || ix86_access_stack_p...))
        call ix86_update_stack_alignment.

The compile times of PR target/124165 and PR target/124684 test are
unchanged.

      if (ix86_need_alignment_p...
          && (TEST_HARD_REG_BIT (hard_stack_slot_access, regno)
      || ix86_access_stack_p...))
call ix86_update_stack_alignment.

gcc/

PR target/124759
PR target/124789
* config/i386/i386.cc (ix86_need_alignment_p_2): New function.
Exclude memory with SYMBOLIC_CONST.
(ix86_need_alignment_p_1): Call ix86_need_alignment_p_2.
(ix86_find_max_used_stack_alignment): Restore
hard_stack_slot_access handling.

gcc/testsuite/

PR target/124759
PR target/124789
* gcc.target/i386/pr124759.c: New test.

Comments

Richard Biener April 7, 2026, 12:04 p.m. UTC | #1 
On Tue, 7 Apr 2026, H.J. Lu wrote:

> commit f511bf93f947199a9f9099fee87b7052e5515fb9
> Author: H.J. Lu <hjl.tools@gmail.com>
> Date:   Sun Mar 29 14:30:28 2026 -0700
> 
>     x86: Call ix86_access_stack_p only for larger alignment

This also LGTM.

Thanks,
Richard.

> has 2 issues:
> 
> 1. It didn't exclude memory with SYMBOLIC_CONST when checking for stack
> access.
> 2. It replaced:
> 
>   HARD_REG_SET hard_stack_slot_access = stack_slot_access;
>   ...
>       bool symbolic_const_load_p = false;
> 
>       if (!TEST_HARD_REG_BIT (hard_stack_slot_access, regno))
> ...
>         symbolic_const_load_p = true;
>       ...
> 
>       if (!symbolic_const_load_p
>           || ix86_access_stack_p...)
> call ix86_update_stack_alignment.
> 
> with
> 
>       if (ix86_need_alignment_p...
>           && ix86_access_stack_p...)
> call ix86_update_stack_alignment.
> 
> Since ix86_access_stack_p excludes registers on hard_stack_slot_access,
> stack alignment isn't updated for registers on hard_stack_slot_access.
> Instead, we should do
> 
>       if (ix86_need_alignment_p...
>           && (TEST_HARD_REG_BIT (hard_stack_slot_access, regno)
>               || ix86_access_stack_p...))
>         call ix86_update_stack_alignment.
> 
> The compile times of PR target/124165 and PR target/124684 test are
> unchanged.
> 
>       if (ix86_need_alignment_p...
>           && (TEST_HARD_REG_BIT (hard_stack_slot_access, regno)
>       || ix86_access_stack_p...))
> call ix86_update_stack_alignment.
> 
> gcc/
> 
> PR target/124759
> PR target/124789
> * config/i386/i386.cc (ix86_need_alignment_p_2): New function.
> Exclude memory with SYMBOLIC_CONST.
> (ix86_need_alignment_p_1): Call ix86_need_alignment_p_2.
> (ix86_find_max_used_stack_alignment): Restore
> hard_stack_slot_access handling.
> 
> gcc/testsuite/
> 
> PR target/124759
> PR target/124789
> * gcc.target/i386/pr124759.c: New test.
> 
>
diff mbox series

Patch

From eeb8b623766f5bbc3a6801445f1b48734e6ee899 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Fri, 3 Apr 2026 15:55:17 +0800
Subject: [PATCH v3 2/2] Correct x86: Call ix86_access_stack_p only for larger
 alignment

commit f511bf93f947199a9f9099fee87b7052e5515fb9
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Sun Mar 29 14:30:28 2026 -0700

    x86: Call ix86_access_stack_p only for larger alignment

has 2 issues:

1. It didn't exclude memory with SYMBOLIC_CONST when checking for stack
access.
2. It replaced:

  HARD_REG_SET hard_stack_slot_access = stack_slot_access;
  ...
      bool symbolic_const_load_p = false;

      if (!TEST_HARD_REG_BIT (hard_stack_slot_access, regno))
	...
        symbolic_const_load_p = true;
      ...

      if (!symbolic_const_load_p
          || ix86_access_stack_p...)
	call ix86_update_stack_alignment.

with

      if (ix86_need_alignment_p...
          && ix86_access_stack_p...)
	call ix86_update_stack_alignment.

Since ix86_access_stack_p excludes registers on hard_stack_slot_access,
stack alignment isn't updated for registers on hard_stack_slot_access.
Instead, we should do

      if (ix86_need_alignment_p...
          && (TEST_HARD_REG_BIT (hard_stack_slot_access, regno)
              || ix86_access_stack_p...))
        call ix86_update_stack_alignment.

The compile times of PR target/124165 and PR target/124684 test are
unchanged.

      if (ix86_need_alignment_p...
          && (TEST_HARD_REG_BIT (hard_stack_slot_access, regno)
	      || ix86_access_stack_p...))
	call ix86_update_stack_alignment.

gcc/

	PR target/124759
	PR target/124789
	* config/i386/i386.cc (ix86_need_alignment_p_2): New function.
	Exclude memory with SYMBOLIC_CONST.
	(ix86_need_alignment_p_1): Call ix86_need_alignment_p_2.
	(ix86_find_max_used_stack_alignment): Restore
	hard_stack_slot_access handling.

gcc/testsuite/

	PR target/124759
	PR target/124789
	* gcc.target/i386/pr124759.c: New test.

Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
---
 gcc/config/i386/i386.cc                  | 40 ++++++++++++++++++++----
 gcc/testsuite/gcc.target/i386/pr124759.c | 29 +++++++++++++++++
 2 files changed, 63 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr124759.c

diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc
index f4d0f623943..9d1a2af7064 100644
--- a/gcc/config/i386/i386.cc
+++ b/gcc/config/i386/i386.cc
@@ -8781,6 +8781,29 @@  ix86_access_stack_p (unsigned int regno, basic_block bb,
   return false;
 }
 
+/* Return true if OP isn't a memory operand with SYMBOLIC_CONST and
+   needs alignment > ALIGNMENT.  */
+
+static bool
+ix86_need_alignment_p_2 (const_rtx op, unsigned int alignment)
+{
+  bool need_alignment = MEM_ALIGN (op) > alignment;
+  tree mem_expr = MEM_EXPR (op);
+  if (!mem_expr)
+    return need_alignment;
+
+  tree var = get_base_address (mem_expr);
+  if (!VAR_P (var) || !DECL_RTL_SET_P (var))
+    return need_alignment;
+
+  rtx x = DECL_RTL (var);
+  if (!MEM_P (x))
+    return need_alignment;
+
+  x = XEXP (x, 0);
+  return !SYMBOLIC_CONST (x) && need_alignment;
+}
+
 /* Return true if SET needs alignment > ALIGNMENT.  */
 
 static bool
@@ -8789,7 +8812,7 @@  ix86_need_alignment_p_1 (rtx set, unsigned int alignment)
   rtx dest = SET_DEST (set);
 
   if (MEM_P (dest))
-    return MEM_ALIGN (dest) > alignment;
+    return ix86_need_alignment_p_2 (dest, alignment);
 
   const_rtx src = SET_SRC (set);
 
@@ -8799,7 +8822,7 @@  ix86_need_alignment_p_1 (rtx set, unsigned int alignment)
       auto op = *iter;
 
       if (MEM_P (op))
-	return MEM_ALIGN (op) > alignment;
+	return ix86_need_alignment_p_2 (op, alignment);
     }
 
   return false;
@@ -8890,6 +8913,9 @@  ix86_find_max_used_stack_alignment (unsigned int &stack_alignment,
       bitmap_set_bit (worklist, HARD_FRAME_POINTER_REGNUM);
     }
 
+  /* Registers on HARD_STACK_SLOT_ACCESS always access stack.  */
+  HARD_REG_SET hard_stack_slot_access = stack_slot_access;
+
   calculate_dominance_info (CDI_DOMINATORS);
 
   unsigned int regno;
@@ -8926,10 +8952,12 @@  ix86_find_max_used_stack_alignment (unsigned int &stack_alignment,
 	  /* Call ix86_access_stack_p only if INSN needs alignment >
 	     STACK_ALIGNMENT.  */
 	  if (ix86_need_alignment_p (insn, stack_alignment)
-	      && ix86_access_stack_p (regno, BLOCK_FOR_INSN (insn),
-				      set_up_by_prologue, prologue_used,
-				      reg_dominate_bbs_known,
-				      reg_dominate_bbs))
+	      && (TEST_HARD_REG_BIT (hard_stack_slot_access, regno)
+		  || ix86_access_stack_p (regno, BLOCK_FOR_INSN (insn),
+					  set_up_by_prologue,
+					  prologue_used,
+					  reg_dominate_bbs_known,
+					  reg_dominate_bbs)))
 	    {
 	      /* Update stack alignment if REGNO is used for stack
 		 access.  */
diff --git a/gcc/testsuite/gcc.target/i386/pr124759.c b/gcc/testsuite/gcc.target/i386/pr124759.c
new file mode 100644
index 00000000000..b5a86b7b9be
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr124759.c
@@ -0,0 +1,29 @@ 
+/* { dg-do run { target avx_runtime } } */
+/* { dg-options "-fhardened -O3 -mavx" } */
+
+struct a {
+  unsigned long long b;
+  unsigned long long c;
+  int d;
+  char e;
+  int f;
+} g;
+int h = 1, i, j;
+long long k, l, n;
+int m[1]={};
+short o;
+static inline short p(short q, short r) { return q + r; }
+int main() {
+  for (; h < 3; h = p(h, 1))
+    if (h) {
+      k |= 4;
+      struct a s = {8, 2, 2, 4, 15};
+      for (int n = 0; n != 54; n++)
+        if (m[l])
+          if (i)
+            o = 0;
+      g = s;
+      j &= 4073709551610LL;
+    }
+  return 0;
+}
-- 
2.53.0