public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH v2] i386 PIE: accept @GOTOFF in load/store multi base address
@ 2023-11-08 16:37 Alexandre Oliva
  2023-11-09  9:30 ` Uros Bizjak
  0 siblings, 1 reply; 2+ messages in thread
From: Alexandre Oliva @ 2023-11-08 16:37 UTC (permalink / raw)
  To: gcc-patches; +Cc: Rainer Orth, Mike Stump, Jan Hubicka, Uros Bizjak

Ping?
https://gcc.gnu.org/pipermail/gcc-patches/2022-July/598872.html

Looking at the code generated for sse2-{load,store}-multi.c with PIE,
I realized we could use UNSPEC_GOTOFF as a base address, and that this
would enable the test to use the vector insns expected by the tests
even with PIC, so I extended the base + offset logic used by the SSE2
multi-load/store peepholes to accept reg + symbolic base + offset too,
so that the test generated the expected insns even with PIE.

Regstrapped on x86_64-linux-gnu, also tested with gcc-13 on i686- and
x86_64-.  Ok to install?


for  gcc/ChangeLog

	* config/i386/i386.cc (symbolic_base_address_p,
	base_address_p): New, factored out from...
	(extract_base_offset_in_addr): ... here and extended to
	recognize REG+GOTOFF, as in gcc.target/i386/sse2-load-multi.c
	and sse2-store-multi.c with PIE enabled by default.
---
 gcc/config/i386/i386.cc |   89 ++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 75 insertions(+), 14 deletions(-)

diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc
index c2bd07fced7b1..eec9b42396e0a 100644
--- a/gcc/config/i386/i386.cc
+++ b/gcc/config/i386/i386.cc
@@ -25198,11 +25198,40 @@ ix86_reloc_rw_mask (void)
 }
 #endif
 
-/* If MEM is in the form of [base+offset], extract the two parts
-   of address and set to BASE and OFFSET, otherwise return false.  */
+/* Return true iff ADDR can be used as a symbolic base address.  */
 
 static bool
-extract_base_offset_in_addr (rtx mem, rtx *base, rtx *offset)
+symbolic_base_address_p (rtx addr)
+{
+  if (GET_CODE (addr) == SYMBOL_REF)
+    return true;
+
+  if (GET_CODE (addr) == UNSPEC && XINT (addr, 1) == UNSPEC_GOTOFF)
+    return true;
+
+  return false;
+}
+
+/* Return true iff ADDR can be used as a base address.  */
+
+static bool
+base_address_p (rtx addr)
+{
+  if (REG_P (addr))
+    return true;
+
+  if (symbolic_base_address_p (addr))
+    return true;
+
+  return false;
+}
+
+/* If MEM is in the form of [(base+symbase)+offset], extract the three
+   parts of address and set to BASE, SYMBASE and OFFSET, otherwise
+   return false.  */
+
+static bool
+extract_base_offset_in_addr (rtx mem, rtx *base, rtx *symbase, rtx *offset)
 {
   rtx addr;
 
@@ -25213,21 +25242,52 @@ extract_base_offset_in_addr (rtx mem, rtx *base, rtx *offset)
   if (GET_CODE (addr) == CONST)
     addr = XEXP (addr, 0);
 
-  if (REG_P (addr) || GET_CODE (addr) == SYMBOL_REF)
+  if (base_address_p (addr))
     {
       *base = addr;
+      *symbase = const0_rtx;
       *offset = const0_rtx;
       return true;
     }
 
   if (GET_CODE (addr) == PLUS
-      && (REG_P (XEXP (addr, 0))
-	  || GET_CODE (XEXP (addr, 0)) == SYMBOL_REF)
-      && CONST_INT_P (XEXP (addr, 1)))
+      && base_address_p (XEXP (addr, 0)))
     {
-      *base = XEXP (addr, 0);
-      *offset = XEXP (addr, 1);
-      return true;
+      rtx addend = XEXP (addr, 1);
+
+      if (GET_CODE (addend) == CONST)
+	addend = XEXP (addend, 0);
+
+      if (CONST_INT_P (addend))
+	{
+	  *base = XEXP (addr, 0);
+	  *symbase = const0_rtx;
+	  *offset = addend;
+	  return true;
+	}
+
+      /* Also accept REG + symbolic ref, with or without a CONST_INT
+	 offset.  */
+      if (REG_P (XEXP (addr, 0)))
+	{
+	  if (symbolic_base_address_p (addend))
+	    {
+	      *base = XEXP (addr, 0);
+	      *symbase = addend;
+	      *offset = const0_rtx;
+	      return true;
+	    }
+
+	  if (GET_CODE (addend) == PLUS
+	      && symbolic_base_address_p (XEXP (addend, 0))
+	      && CONST_INT_P (XEXP (addend, 1)))
+	    {
+	      *base = XEXP (addr, 0);
+	      *symbase = XEXP (addend, 0);
+	      *offset = XEXP (addend, 1);
+	      return true;
+	    }
+	}
     }
 
   return false;
@@ -25242,7 +25302,8 @@ ix86_operands_ok_for_move_multiple (rtx *operands, bool load,
 				    machine_mode mode)
 {
   HOST_WIDE_INT offval_1, offval_2, msize;
-  rtx mem_1, mem_2, reg_1, reg_2, base_1, base_2, offset_1, offset_2;
+  rtx mem_1, mem_2, reg_1, reg_2, base_1, base_2,
+    symbase_1, symbase_2, offset_1, offset_2;
 
   if (load)
     {
@@ -25265,13 +25326,13 @@ ix86_operands_ok_for_move_multiple (rtx *operands, bool load,
     return false;
 
   /* Check if the addresses are in the form of [base+offset].  */
-  if (!extract_base_offset_in_addr (mem_1, &base_1, &offset_1))
+  if (!extract_base_offset_in_addr (mem_1, &base_1, &symbase_1, &offset_1))
     return false;
-  if (!extract_base_offset_in_addr (mem_2, &base_2, &offset_2))
+  if (!extract_base_offset_in_addr (mem_2, &base_2, &symbase_2, &offset_2))
     return false;
 
   /* Check if the bases are the same.  */
-  if (!rtx_equal_p (base_1, base_2))
+  if (!rtx_equal_p (base_1, base_2) || !rtx_equal_p (symbase_1, symbase_2))
     return false;
 
   offval_1 = INTVAL (offset_1);


-- 
Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
   Free Software Activist                   GNU Toolchain Engineer
More tolerance and less prejudice are key for inclusion and diversity
Excluding neuro-others for not behaving ""normal"" is *not* inclusive

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] i386 PIE: accept @GOTOFF in load/store multi base address
  2023-11-08 16:37 [PATCH v2] i386 PIE: accept @GOTOFF in load/store multi base address Alexandre Oliva
@ 2023-11-09  9:30 ` Uros Bizjak
  0 siblings, 0 replies; 2+ messages in thread
From: Uros Bizjak @ 2023-11-09  9:30 UTC (permalink / raw)
  To: Alexandre Oliva; +Cc: gcc-patches, Rainer Orth, Mike Stump, Jan Hubicka

On Wed, Nov 8, 2023 at 5:37 PM Alexandre Oliva <oliva@adacore.com> wrote:
>
> Ping?
> https://gcc.gnu.org/pipermail/gcc-patches/2022-July/598872.html
>
> Looking at the code generated for sse2-{load,store}-multi.c with PIE,
> I realized we could use UNSPEC_GOTOFF as a base address, and that this
> would enable the test to use the vector insns expected by the tests
> even with PIC, so I extended the base + offset logic used by the SSE2
> multi-load/store peepholes to accept reg + symbolic base + offset too,
> so that the test generated the expected insns even with PIE.
>
> Regstrapped on x86_64-linux-gnu, also tested with gcc-13 on i686- and
> x86_64-.  Ok to install?
>
>
> for  gcc/ChangeLog
>
>         * config/i386/i386.cc (symbolic_base_address_p,
>         base_address_p): New, factored out from...
>         (extract_base_offset_in_addr): ... here and extended to
>         recognize REG+GOTOFF, as in gcc.target/i386/sse2-load-multi.c
>         and sse2-store-multi.c with PIE enabled by default.

LGTM.

Thanks,
Uros.

> ---
>  gcc/config/i386/i386.cc |   89 ++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 75 insertions(+), 14 deletions(-)
>
> diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc
> index c2bd07fced7b1..eec9b42396e0a 100644
> --- a/gcc/config/i386/i386.cc
> +++ b/gcc/config/i386/i386.cc
> @@ -25198,11 +25198,40 @@ ix86_reloc_rw_mask (void)
>  }
>  #endif
>
> -/* If MEM is in the form of [base+offset], extract the two parts
> -   of address and set to BASE and OFFSET, otherwise return false.  */
> +/* Return true iff ADDR can be used as a symbolic base address.  */
>
>  static bool
> -extract_base_offset_in_addr (rtx mem, rtx *base, rtx *offset)
> +symbolic_base_address_p (rtx addr)
> +{
> +  if (GET_CODE (addr) == SYMBOL_REF)
> +    return true;
> +
> +  if (GET_CODE (addr) == UNSPEC && XINT (addr, 1) == UNSPEC_GOTOFF)
> +    return true;
> +
> +  return false;
> +}
> +
> +/* Return true iff ADDR can be used as a base address.  */
> +
> +static bool
> +base_address_p (rtx addr)
> +{
> +  if (REG_P (addr))
> +    return true;
> +
> +  if (symbolic_base_address_p (addr))
> +    return true;
> +
> +  return false;
> +}
> +
> +/* If MEM is in the form of [(base+symbase)+offset], extract the three
> +   parts of address and set to BASE, SYMBASE and OFFSET, otherwise
> +   return false.  */
> +
> +static bool
> +extract_base_offset_in_addr (rtx mem, rtx *base, rtx *symbase, rtx *offset)
>  {
>    rtx addr;
>
> @@ -25213,21 +25242,52 @@ extract_base_offset_in_addr (rtx mem, rtx *base, rtx *offset)
>    if (GET_CODE (addr) == CONST)
>      addr = XEXP (addr, 0);
>
> -  if (REG_P (addr) || GET_CODE (addr) == SYMBOL_REF)
> +  if (base_address_p (addr))
>      {
>        *base = addr;
> +      *symbase = const0_rtx;
>        *offset = const0_rtx;
>        return true;
>      }
>
>    if (GET_CODE (addr) == PLUS
> -      && (REG_P (XEXP (addr, 0))
> -         || GET_CODE (XEXP (addr, 0)) == SYMBOL_REF)
> -      && CONST_INT_P (XEXP (addr, 1)))
> +      && base_address_p (XEXP (addr, 0)))
>      {
> -      *base = XEXP (addr, 0);
> -      *offset = XEXP (addr, 1);
> -      return true;
> +      rtx addend = XEXP (addr, 1);
> +
> +      if (GET_CODE (addend) == CONST)
> +       addend = XEXP (addend, 0);
> +
> +      if (CONST_INT_P (addend))
> +       {
> +         *base = XEXP (addr, 0);
> +         *symbase = const0_rtx;
> +         *offset = addend;
> +         return true;
> +       }
> +
> +      /* Also accept REG + symbolic ref, with or without a CONST_INT
> +        offset.  */
> +      if (REG_P (XEXP (addr, 0)))
> +       {
> +         if (symbolic_base_address_p (addend))
> +           {
> +             *base = XEXP (addr, 0);
> +             *symbase = addend;
> +             *offset = const0_rtx;
> +             return true;
> +           }
> +
> +         if (GET_CODE (addend) == PLUS
> +             && symbolic_base_address_p (XEXP (addend, 0))
> +             && CONST_INT_P (XEXP (addend, 1)))
> +           {
> +             *base = XEXP (addr, 0);
> +             *symbase = XEXP (addend, 0);
> +             *offset = XEXP (addend, 1);
> +             return true;
> +           }
> +       }
>      }
>
>    return false;
> @@ -25242,7 +25302,8 @@ ix86_operands_ok_for_move_multiple (rtx *operands, bool load,
>                                     machine_mode mode)
>  {
>    HOST_WIDE_INT offval_1, offval_2, msize;
> -  rtx mem_1, mem_2, reg_1, reg_2, base_1, base_2, offset_1, offset_2;
> +  rtx mem_1, mem_2, reg_1, reg_2, base_1, base_2,
> +    symbase_1, symbase_2, offset_1, offset_2;
>
>    if (load)
>      {
> @@ -25265,13 +25326,13 @@ ix86_operands_ok_for_move_multiple (rtx *operands, bool load,
>      return false;
>
>    /* Check if the addresses are in the form of [base+offset].  */
> -  if (!extract_base_offset_in_addr (mem_1, &base_1, &offset_1))
> +  if (!extract_base_offset_in_addr (mem_1, &base_1, &symbase_1, &offset_1))
>      return false;
> -  if (!extract_base_offset_in_addr (mem_2, &base_2, &offset_2))
> +  if (!extract_base_offset_in_addr (mem_2, &base_2, &symbase_2, &offset_2))
>      return false;
>
>    /* Check if the bases are the same.  */
> -  if (!rtx_equal_p (base_1, base_2))
> +  if (!rtx_equal_p (base_1, base_2) || !rtx_equal_p (symbase_1, symbase_2))
>      return false;
>
>    offval_1 = INTVAL (offset_1);
>
>
> --
> Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
>    Free Software Activist                   GNU Toolchain Engineer
> More tolerance and less prejudice are key for inclusion and diversity
> Excluding neuro-others for not behaving ""normal"" is *not* inclusive

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-11-09  9:30 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-08 16:37 [PATCH v2] i386 PIE: accept @GOTOFF in load/store multi base address Alexandre Oliva
2023-11-09  9:30 ` Uros Bizjak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).