[RFA 0/9] Address space support for x86

[RFA 0/9] Address space support for x86

[Richard Henderson]

I started with Armin Rigo's patch, from back in July,

  https://gcc.gnu.org/ml/gcc/2015-07/msg00125.html

but then wound up changing all of it.  To wit:

(1) If we use more sensible defaults for address spaces, we no longer
require 8 backend functions and 1 new hook.  Indeed, basic support for
x86 segmentation via address spaces then requires no overriding of any
hooks.

(2) In order to do anything useful with x86 segmentation, we need to
acknowledge that the most common usage puts a valid object at %seg:0.
Which means that we do need a new hook to disable several optimizations.

(3) I found that usage within glibc really requires a more structured
address space, one with a known mapping to the generic address space.
Thus I introduce __seg_tls, which knows that the segment base is also
stored at %seg:0.

(4) I attempted to convert the entire tls implementation to use __seg_tls.
I believe this is ultimately the best long-term solution, but I ran into
a number of problems both generically and in the backend that prevented
this from being simple.  I've dropped these patches for now.

(5) However, in the process of attempting (4), I found two problems in
the backend that could affect any use of address spaces: the movabs
patterns and string expansions.


r~


Richard Henderson (9):
  Change default of non-overlapping address space conversion
  Relax ADDR_SPACE_GENERIC_P checks for default address space hooks
  i386: Handle address spaces in movabs patterns
  i386: Disallow address spaces with string insns
  i386: Add address spaces for fs/gs segments
  i386: Replace ix86_address_seg with addr_space_t
  i386: Add address space for tls
  Add TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID
  Fix PR 66768

 gcc/config/i386/i386-c.c                     |   8 +
 gcc/config/i386/i386-protos.h                |   8 +-
 gcc/config/i386/i386.c                       | 296 +++++++++++++++++++--------
 gcc/config/i386/i386.h                       |   3 +-
 gcc/config/i386/i386.md                      |  91 +++++---
 gcc/config/i386/predicates.md                |   8 +-
 gcc/config/i386/rdos.h                       |   2 +-
 gcc/doc/tm.texi                              |   5 +
 gcc/doc/tm.texi.in                           |   2 +
 gcc/expr.c                                   |  30 +--
 gcc/fold-const.c                             |   6 +-
 gcc/gimple.c                                 |  12 +-
 gcc/target.def                               |   9 +
 gcc/targhooks.c                              |  48 ++---
 gcc/targhooks.h                              |   1 +
 gcc/testsuite/gcc.target/i386/addr-space-1.c |  11 +
 gcc/tree-ssa-address.c                       |   2 +-
 17 files changed, 385 insertions(+), 157 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/addr-space-1.c

-- 
2.4.3

[PATCH 4/9] i386: Disallow address spaces with string insns

[Richard Henderson]

While cmps and movs allow a segment override of the ds:esi
source, the es:edi source/destination cannot be overriden.
Simplify things in the backend for now by disallowing
segments for string insns entirely.


	* config/i386/i386.c (ix86_check_no_addr_space): New.
	(decide_alg): Add have_as parameter.
	(alg_usable_p): Likewise; disable rep algorithms if set.
	(ix86_expand_set_or_movmem): Notice if either MEM has a
	non-default address space.
	(ix86_expand_strlen): Likewise.
	* config/i386/i386.md (strmov, strset): Likewise.
	(*strmovdi_rex_1): Use ix86_check_no_addr_space.
	(*strmovsi_1, *strmovqi_1, *rep_movdi_rex64, *rep_movsi, *rep_movqi,
	*strsetdi_rex_1, *strsetsi_1, *strsethi_1, *strsetqi_1,
	*rep_stosdi_rex64, *rep_stossi, *rep_stosqi, *cmpstrnqi_nz_1,
	*cmpstrnqi_1, *strlenqi_1): Likewise.
---
 gcc/config/i386/i386-protos.h |  1 +
 gcc/config/i386/i386.c        | 61 ++++++++++++++++++++++++++++++++-----------
 gcc/config/i386/i386.md       | 59 +++++++++++++++++++++++++++++------------
 3 files changed, 89 insertions(+), 32 deletions(-)

diff --git a/gcc/config/i386/i386-protos.h b/gcc/config/i386/i386-protos.h
index 6a17ef4..5e46833 100644
--- a/gcc/config/i386/i386-protos.h
+++ b/gcc/config/i386/i386-protos.h
@@ -141,6 +141,7 @@ extern void ix86_split_ashr (rtx *, rtx, machine_mode);
 extern void ix86_split_lshr (rtx *, rtx, machine_mode);
 extern rtx ix86_find_base_term (rtx);
 extern bool ix86_check_movabs (rtx, int);
+extern bool ix86_check_no_addr_space (rtx);
 extern void ix86_split_idivmod (machine_mode, rtx[], bool);
 
 extern rtx assign_386_stack_local (machine_mode, enum ix86_stack_slot);
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 38953dd..b12fb2d 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -10552,6 +10552,20 @@ ix86_check_movabs (rtx insn, int opnum)
   gcc_assert (MEM_P (mem));
   return volatile_ok || !MEM_VOLATILE_P (mem);
 }
+
+/* Return false if INSN contains a MEM with a non-default address space.  */
+bool
+ix86_check_no_addr_space (rtx insn)
+{
+  subrtx_var_iterator::array_type array;
+  FOR_EACH_SUBRTX_VAR (iter, array, PATTERN (insn), ALL)
+    {
+      rtx x = *iter;
+      if (MEM_P (x) && !ADDR_SPACE_GENERIC_P (MEM_ADDR_SPACE (x)))
+	return false;
+    }
+  return true;
+}
 \f
 /* Initialize the table of extra 80387 mathematical constants.  */
 
@@ -25621,7 +25635,7 @@ expand_set_or_movmem_constant_prologue (rtx dst, rtx *srcp, rtx destreg,
 /* Return true if ALG can be used in current context.  
    Assume we expand memset if MEMSET is true.  */
 static bool
-alg_usable_p (enum stringop_alg alg, bool memset)
+alg_usable_p (enum stringop_alg alg, bool memset, bool have_as)
 {
   if (alg == no_stringop)
     return false;
@@ -25630,12 +25644,19 @@ alg_usable_p (enum stringop_alg alg, bool memset)
   /* Algorithms using the rep prefix want at least edi and ecx;
      additionally, memset wants eax and memcpy wants esi.  Don't
      consider such algorithms if the user has appropriated those
-     registers for their own purposes.	*/
+     registers for their own purposes, or if we have a non-default
+     address space, since some string insns cannot override the segment.  */
   if (alg == rep_prefix_1_byte
       || alg == rep_prefix_4_byte
       || alg == rep_prefix_8_byte)
-    return !(fixed_regs[CX_REG] || fixed_regs[DI_REG]
-             || (memset ? fixed_regs[AX_REG] : fixed_regs[SI_REG]));
+    {
+      if (have_as)
+	return false;
+      if (fixed_regs[CX_REG]
+	  || fixed_regs[DI_REG]
+	  || (memset ? fixed_regs[AX_REG] : fixed_regs[SI_REG]))
+	return false;
+    }
   return true;
 }
 
@@ -25643,7 +25664,8 @@ alg_usable_p (enum stringop_alg alg, bool memset)
 static enum stringop_alg
 decide_alg (HOST_WIDE_INT count, HOST_WIDE_INT expected_size,
 	    unsigned HOST_WIDE_INT min_size, unsigned HOST_WIDE_INT max_size,
-	    bool memset, bool zero_memset, int *dynamic_check, bool *noalign)
+	    bool memset, bool zero_memset, bool have_as,
+	    int *dynamic_check, bool *noalign)
 {
   const struct stringop_algs * algs;
   bool optimize_for_speed;
@@ -25675,7 +25697,7 @@ decide_alg (HOST_WIDE_INT count, HOST_WIDE_INT expected_size,
   for (i = 0; i < MAX_STRINGOP_ALGS; i++)
     {
       enum stringop_alg candidate = algs->size[i].alg;
-      bool usable = alg_usable_p (candidate, memset);
+      bool usable = alg_usable_p (candidate, memset, have_as);
       any_alg_usable_p |= usable;
 
       if (candidate != libcall && candidate && usable)
@@ -25691,17 +25713,17 @@ decide_alg (HOST_WIDE_INT count, HOST_WIDE_INT expected_size,
 
   /* If user specified the algorithm, honnor it if possible.  */
   if (ix86_stringop_alg != no_stringop
-      && alg_usable_p (ix86_stringop_alg, memset))
+      && alg_usable_p (ix86_stringop_alg, memset, have_as))
     return ix86_stringop_alg;
   /* rep; movq or rep; movl is the smallest variant.  */
   else if (!optimize_for_speed)
     {
       *noalign = true;
       if (!count || (count & 3) || (memset && !zero_memset))
-	return alg_usable_p (rep_prefix_1_byte, memset)
+	return alg_usable_p (rep_prefix_1_byte, memset, have_as)
 	       ? rep_prefix_1_byte : loop_1_byte;
       else
-	return alg_usable_p (rep_prefix_4_byte, memset)
+	return alg_usable_p (rep_prefix_4_byte, memset, have_as)
 	       ? rep_prefix_4_byte : loop;
     }
   /* Very tiny blocks are best handled via the loop, REP is expensive to
@@ -25724,7 +25746,8 @@ decide_alg (HOST_WIDE_INT count, HOST_WIDE_INT expected_size,
 	    {
 	      enum stringop_alg candidate = algs->size[i].alg;
 
-	      if (candidate != libcall && alg_usable_p (candidate, memset))
+	      if (candidate != libcall
+		  && alg_usable_p (candidate, memset, have_as))
 		{
 		  alg = candidate;
 		  alg_noalign = algs->size[i].noalign;
@@ -25744,7 +25767,7 @@ decide_alg (HOST_WIDE_INT count, HOST_WIDE_INT expected_size,
 		  else if (!any_alg_usable_p)
 		    break;
 		}
-	      else if (alg_usable_p (candidate, memset))
+	      else if (alg_usable_p (candidate, memset, have_as))
 		{
 		  *noalign = algs->size[i].noalign;
 		  return candidate;
@@ -25761,7 +25784,7 @@ decide_alg (HOST_WIDE_INT count, HOST_WIDE_INT expected_size,
      choice in ix86_costs.  */
   if ((TARGET_INLINE_ALL_STRINGOPS || TARGET_INLINE_STRINGOPS_DYNAMICALLY)
       && (algs->unknown_size == libcall
-	  || !alg_usable_p (algs->unknown_size, memset)))
+	  || !alg_usable_p (algs->unknown_size, memset, have_as)))
     {
       enum stringop_alg alg;
 
@@ -25778,7 +25801,7 @@ decide_alg (HOST_WIDE_INT count, HOST_WIDE_INT expected_size,
       if (max <= 0)
 	max = 4096;
       alg = decide_alg (count, max / 2, min_size, max_size, memset,
-			zero_memset, dynamic_check, noalign);
+			zero_memset, have_as, dynamic_check, noalign);
       gcc_assert (*dynamic_check == -1);
       if (TARGET_INLINE_STRINGOPS_DYNAMICALLY)
 	*dynamic_check = max;
@@ -25786,7 +25809,7 @@ decide_alg (HOST_WIDE_INT count, HOST_WIDE_INT expected_size,
 	gcc_assert (alg != libcall);
       return alg;
     }
-  return (alg_usable_p (algs->unknown_size, memset)
+  return (alg_usable_p (algs->unknown_size, memset, have_as)
 	  ? algs->unknown_size : libcall);
 }
 
@@ -25992,6 +26015,7 @@ ix86_expand_set_or_movmem (rtx dst, rtx src, rtx count_exp, rtx val_exp,
   unsigned HOST_WIDE_INT max_size = -1;
   unsigned HOST_WIDE_INT probable_max_size = -1;
   bool misaligned_prologue_used = false;
+  bool have_as;
 
   if (CONST_INT_P (align_exp))
     align = INTVAL (align_exp);
@@ -26029,11 +26053,15 @@ ix86_expand_set_or_movmem (rtx dst, rtx src, rtx count_exp, rtx val_exp,
   if (count > (HOST_WIDE_INT_1U << 30))
     return false;
 
+  have_as = !ADDR_SPACE_GENERIC_P (MEM_ADDR_SPACE (dst));
+  if (!issetmem)
+    have_as |= !ADDR_SPACE_GENERIC_P (MEM_ADDR_SPACE (src));
+
   /* Step 0: Decide on preferred algorithm, desired alignment and
      size of chunks to be copied by main loop.  */
   alg = decide_alg (count, expected_size, min_size, probable_max_size,
 		    issetmem,
-		    issetmem && val_exp == const0_rtx,
+		    issetmem && val_exp == const0_rtx, have_as,
 		    &dynamic_check, &noalign);
   if (alg == libcall)
     return false;
@@ -26647,6 +26675,9 @@ ix86_expand_strlen (rtx out, rtx src, rtx eoschar, rtx align)
       /* Can't use this if the user has appropriated eax, ecx, or edi.  */
       if (fixed_regs[AX_REG] || fixed_regs[CX_REG] || fixed_regs[DI_REG])
         return false;
+      /* Can't use this for non-default address spaces.  */
+      if (!ADDR_SPACE_GENERIC_P (MEM_ADDR_SPACE (src)))
+	return false;
 
       scratch2 = gen_reg_rtx (Pmode);
       scratch3 = gen_reg_rtx (Pmode);
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index ccb672d..2cb94fe 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -16159,6 +16159,10 @@
 	      (clobber (reg:CC FLAGS_REG))])]
   ""
 {
+  /* Can't use this for non-default address spaces.  */
+  if (!ADDR_SPACE_GENERIC_P (MEM_ADDR_SPACE (operands[3])))
+    FAIL;
+
   rtx adjust = GEN_INT (GET_MODE_SIZE (GET_MODE (operands[1])));
 
   /* If .md ever supports :P for Pmode, these can be directly
@@ -16199,7 +16203,8 @@
 	(plus:P (match_dup 3)
 		(const_int 8)))]
   "TARGET_64BIT
-   && !(fixed_regs[SI_REG] || fixed_regs[DI_REG])"
+   && !(fixed_regs[SI_REG] || fixed_regs[DI_REG])
+   && ix86_check_no_addr_space (insn)"
   "%^movsq"
   [(set_attr "type" "str")
    (set_attr "memory" "both")
@@ -16214,7 +16219,8 @@
    (set (match_operand:P 1 "register_operand" "=S")
 	(plus:P (match_dup 3)
 		(const_int 4)))]
-  "!(fixed_regs[SI_REG] || fixed_regs[DI_REG])"
+  "!(fixed_regs[SI_REG] || fixed_regs[DI_REG])
+   && ix86_check_no_addr_space (insn)"
   "%^movs{l|d}"
   [(set_attr "type" "str")
    (set_attr "memory" "both")
@@ -16229,7 +16235,8 @@
    (set (match_operand:P 1 "register_operand" "=S")
 	(plus:P (match_dup 3)
 		(const_int 2)))]
-  "!(fixed_regs[SI_REG] || fixed_regs[DI_REG])"
+  "!(fixed_regs[SI_REG] || fixed_regs[DI_REG])
+   && ix86_check_no_addr_space (insn)"
   "%^movsw"
   [(set_attr "type" "str")
    (set_attr "memory" "both")
@@ -16245,7 +16252,8 @@
 	(plus:P (match_dup 3)
 		(const_int 1)))]
   "!(fixed_regs[SI_REG] || fixed_regs[DI_REG])"
-  "%^movsb"
+  "%^movsb
+   && ix86_check_no_addr_space (insn)"
   [(set_attr "type" "str")
    (set_attr "memory" "both")
    (set (attr "prefix_rex")
@@ -16280,7 +16288,8 @@
 	(mem:BLK (match_dup 4)))
    (use (match_dup 5))]
   "TARGET_64BIT
-   && !(fixed_regs[CX_REG] || fixed_regs[SI_REG] || fixed_regs[DI_REG])"
+   && !(fixed_regs[CX_REG] || fixed_regs[SI_REG] || fixed_regs[DI_REG])
+   && ix86_check_no_addr_space (insn)"
   "%^rep{%;} movsq"
   [(set_attr "type" "str")
    (set_attr "prefix_rep" "1")
@@ -16299,7 +16308,8 @@
    (set (mem:BLK (match_dup 3))
 	(mem:BLK (match_dup 4)))
    (use (match_dup 5))]
-  "!(fixed_regs[CX_REG] || fixed_regs[SI_REG] || fixed_regs[DI_REG])"
+  "!(fixed_regs[CX_REG] || fixed_regs[SI_REG] || fixed_regs[DI_REG])
+   && ix86_check_no_addr_space (insn)"
   "%^rep{%;} movs{l|d}"
   [(set_attr "type" "str")
    (set_attr "prefix_rep" "1")
@@ -16316,7 +16326,8 @@
    (set (mem:BLK (match_dup 3))
 	(mem:BLK (match_dup 4)))
    (use (match_dup 5))]
-  "!(fixed_regs[CX_REG] || fixed_regs[SI_REG] || fixed_regs[DI_REG])"
+  "!(fixed_regs[CX_REG] || fixed_regs[SI_REG] || fixed_regs[DI_REG])
+   && ix86_check_no_addr_space (insn)"
   "%^rep{%;} movsb"
   [(set_attr "type" "str")
    (set_attr "prefix_rep" "1")
@@ -16356,6 +16367,10 @@
 	      (clobber (reg:CC FLAGS_REG))])]
   ""
 {
+  /* Can't use this for non-default address spaces.  */
+  if (!ADDR_SPACE_GENERIC_P (MEM_ADDR_SPACE (operands[1])))
+    FAIL;
+
   if (GET_MODE (operands[1]) != GET_MODE (operands[2]))
     operands[1] = adjust_address_nv (operands[1], GET_MODE (operands[2]), 0);
 
@@ -16391,7 +16406,8 @@
 		(const_int 8)))
    (unspec [(const_int 0)] UNSPEC_STOS)]
   "TARGET_64BIT
-   && !(fixed_regs[AX_REG] || fixed_regs[DI_REG])"
+   && !(fixed_regs[AX_REG] || fixed_regs[DI_REG])
+   && ix86_check_no_addr_space (insn)"
   "%^stosq"
   [(set_attr "type" "str")
    (set_attr "memory" "store")
@@ -16404,7 +16420,8 @@
 	(plus:P (match_dup 1)
 		(const_int 4)))
    (unspec [(const_int 0)] UNSPEC_STOS)]
-  "!(fixed_regs[AX_REG] || fixed_regs[DI_REG])"
+  "!(fixed_regs[AX_REG] || fixed_regs[DI_REG])
+   && ix86_check_no_addr_space (insn)"
   "%^stos{l|d}"
   [(set_attr "type" "str")
    (set_attr "memory" "store")
@@ -16417,7 +16434,8 @@
 	(plus:P (match_dup 1)
 		(const_int 2)))
    (unspec [(const_int 0)] UNSPEC_STOS)]
-  "!(fixed_regs[AX_REG] || fixed_regs[DI_REG])"
+  "!(fixed_regs[AX_REG] || fixed_regs[DI_REG])
+   && ix86_check_no_addr_space (insn)"
   "%^stosw"
   [(set_attr "type" "str")
    (set_attr "memory" "store")
@@ -16430,7 +16448,8 @@
 	(plus:P (match_dup 1)
 		(const_int 1)))
    (unspec [(const_int 0)] UNSPEC_STOS)]
-  "!(fixed_regs[AX_REG] || fixed_regs[DI_REG])"
+  "!(fixed_regs[AX_REG] || fixed_regs[DI_REG])
+   && ix86_check_no_addr_space (insn)"
   "%^stosb"
   [(set_attr "type" "str")
    (set_attr "memory" "store")
@@ -16462,7 +16481,8 @@
    (use (match_operand:DI 2 "register_operand" "a"))
    (use (match_dup 4))]
   "TARGET_64BIT
-   && !(fixed_regs[AX_REG] || fixed_regs[CX_REG] || fixed_regs[DI_REG])"
+   && !(fixed_regs[AX_REG] || fixed_regs[CX_REG] || fixed_regs[DI_REG])
+   && ix86_check_no_addr_space (insn)"
   "%^rep{%;} stosq"
   [(set_attr "type" "str")
    (set_attr "prefix_rep" "1")
@@ -16479,7 +16499,8 @@
 	(const_int 0))
    (use (match_operand:SI 2 "register_operand" "a"))
    (use (match_dup 4))]
-  "!(fixed_regs[AX_REG] || fixed_regs[CX_REG] || fixed_regs[DI_REG])"
+  "!(fixed_regs[AX_REG] || fixed_regs[CX_REG] || fixed_regs[DI_REG])
+   && ix86_check_no_addr_space (insn)"
   "%^rep{%;} stos{l|d}"
   [(set_attr "type" "str")
    (set_attr "prefix_rep" "1")
@@ -16495,7 +16516,8 @@
 	(const_int 0))
    (use (match_operand:QI 2 "register_operand" "a"))
    (use (match_dup 4))]
-  "!(fixed_regs[AX_REG] || fixed_regs[CX_REG] || fixed_regs[DI_REG])"
+  "!(fixed_regs[AX_REG] || fixed_regs[CX_REG] || fixed_regs[DI_REG])
+   && ix86_check_no_addr_space (insn)"
   "%^rep{%;} stosb"
   [(set_attr "type" "str")
    (set_attr "prefix_rep" "1")
@@ -16616,7 +16638,8 @@
    (clobber (match_operand:P 0 "register_operand" "=S"))
    (clobber (match_operand:P 1 "register_operand" "=D"))
    (clobber (match_operand:P 2 "register_operand" "=c"))]
-  "!(fixed_regs[CX_REG] || fixed_regs[SI_REG] || fixed_regs[DI_REG])"
+  "!(fixed_regs[CX_REG] || fixed_regs[SI_REG] || fixed_regs[DI_REG])
+   && ix86_check_no_addr_space (insn)"
   "%^repz{%;} cmpsb"
   [(set_attr "type" "str")
    (set_attr "mode" "QI")
@@ -16656,7 +16679,8 @@
    (clobber (match_operand:P 0 "register_operand" "=S"))
    (clobber (match_operand:P 1 "register_operand" "=D"))
    (clobber (match_operand:P 2 "register_operand" "=c"))]
-  "!(fixed_regs[CX_REG] || fixed_regs[SI_REG] || fixed_regs[DI_REG])"
+  "!(fixed_regs[CX_REG] || fixed_regs[SI_REG] || fixed_regs[DI_REG])
+   && ix86_check_no_addr_space (insn)"
   "%^repz{%;} cmpsb"
   [(set_attr "type" "str")
    (set_attr "mode" "QI")
@@ -16697,7 +16721,8 @@
 		   (match_operand:P 4 "register_operand" "0")] UNSPEC_SCAS))
    (clobber (match_operand:P 1 "register_operand" "=D"))
    (clobber (reg:CC FLAGS_REG))]
-  "!(fixed_regs[AX_REG] || fixed_regs[CX_REG] || fixed_regs[DI_REG])"
+  "!(fixed_regs[AX_REG] || fixed_regs[CX_REG] || fixed_regs[DI_REG])
+   && ix86_check_no_addr_space (insn)"
   "%^repnz{%;} scasb"
   [(set_attr "type" "str")
    (set_attr "mode" "QI")
-- 
2.4.3

[PATCH 5/9] i386: Add address spaces for fs/gs segments

[Richard Henderson]

	* config/i386/i386-protos.h (ADDR_SPACE_SEG_FS): New.
	(ADDR_SPACE_SEG_GS): New.
	* config/i386/i386-c.c (ix86_target_macros): Define __SEG_FS
	and __SEG_GS.
	(ix86_register_pragmas): Register address spaces.
	* config/i386/i386.c (ix86_print_operand_address_as): Rename from
	ix86_print_operand_address; add an addr_space_t parameter.
	(ix86_print_operand_address): New.
	(ix86_print_operand): Use ix86_print_operand_address_as.
	(ix86_attr_length_address_default): Account for segment prefix.

testsuite/
	* gcc.target/i386/addr-space-1.c: New test.
---
 gcc/config/i386/i386-c.c                     |   6 +
 gcc/config/i386/i386-protos.h                |   3 +
 gcc/config/i386/i386.c                       | 176 +++++++++++++++++----------
 gcc/testsuite/gcc.target/i386/addr-space-1.c |  11 ++
 4 files changed, 131 insertions(+), 65 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/addr-space-1.c

diff --git a/gcc/config/i386/i386-c.c b/gcc/config/i386/i386-c.c
index 9bc063e..e21b947 100644
--- a/gcc/config/i386/i386-c.c
+++ b/gcc/config/i386/i386-c.c
@@ -592,6 +592,9 @@ ix86_target_macros (void)
 			       ix86_tune,
 			       ix86_fpmath,
 			       cpp_define);
+
+  cpp_define (parse_in, "__SEG_FS");
+  cpp_define (parse_in, "__SEG_GS");
 }
 
 \f
@@ -606,6 +609,9 @@ ix86_register_pragmas (void)
   /* Update pragma hook to allow parsing #pragma GCC target.  */
   targetm.target_option.pragma_parse = ix86_pragma_target_parse;
 
+  c_register_addr_space ("__seg_fs", ADDR_SPACE_SEG_FS);
+  c_register_addr_space ("__seg_gs", ADDR_SPACE_SEG_GS);
+
 #ifdef REGISTER_SUBTARGET_PRAGMAS
   REGISTER_SUBTARGET_PRAGMAS ();
 #endif
diff --git a/gcc/config/i386/i386-protos.h b/gcc/config/i386/i386-protos.h
index 5e46833..f942ef5 100644
--- a/gcc/config/i386/i386-protos.h
+++ b/gcc/config/i386/i386-protos.h
@@ -326,3 +326,6 @@ struct ix86_first_cycle_multipass_data_
 # define TARGET_SCHED_FIRST_CYCLE_MULTIPASS_DATA_T	\
   struct ix86_first_cycle_multipass_data_
 #endif /* RTX_CODE */
+
+const addr_space_t ADDR_SPACE_SEG_FS = 1;
+const addr_space_t ADDR_SPACE_SEG_GS = 2;
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index b12fb2d..8a41f20 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -95,6 +95,7 @@ along with GCC; see the file COPYING3.  If not see
 static rtx legitimize_dllimport_symbol (rtx, bool);
 static rtx legitimize_pe_coff_extern_decl (rtx, bool);
 static rtx legitimize_pe_coff_symbol (rtx, bool);
+static void ix86_print_operand_address_as (FILE *file, rtx addr, addr_space_t);
 
 #ifndef CHECK_STACK_LIMIT
 #define CHECK_STACK_LIMIT (-1)
@@ -17090,32 +17091,22 @@ ix86_print_operand (FILE *file, rtx x, int code)
 
   else if (MEM_P (x))
     {
-      /* No `byte ptr' prefix for call instructions or BLKmode operands.  */
-      if (ASSEMBLER_DIALECT == ASM_INTEL && code != 'X' && code != 'P'
-	  && GET_MODE (x) != BLKmode)
+      rtx addr = XEXP (x, 0);
+
+      /* Avoid (%rip) for call operands.  */
+      if (code == 'P' && CONSTANT_ADDRESS_P (x) && !CONST_INT_P (x))
 	{
-	  const char * size;
-	  switch (GET_MODE_SIZE (GET_MODE (x)))
-	    {
-	    case 1: size = "BYTE"; break;
-	    case 2: size = "WORD"; break;
-	    case 4: size = "DWORD"; break;
-	    case 8: size = "QWORD"; break;
-	    case 12: size = "TBYTE"; break;
-	    case 16:
-	      if (GET_MODE (x) == XFmode)
-		size = "TBYTE";
-              else
-		size = "XMMWORD";
-              break;
-	    case 32: size = "YMMWORD"; break;
-	    case 64: size = "ZMMWORD"; break;
-	    default:
-	      gcc_unreachable ();
-	    }
+	  output_addr_const (file, addr);
+	  return;
+	}
+
+      /* No `byte ptr' prefix for call instructions ... */
+      if (ASSEMBLER_DIALECT == ASM_INTEL && code != 'X' && code != 'P')
+	{
+	  machine_mode mode = GET_MODE (x);
+	  const char *size;
 
-	  /* Check for explicit size override (codes 'b', 'w', 'k',
-	     'q' and 'x')  */
+	  /* Check for explicit size override codes.  */
 	  if (code == 'b')
 	    size = "BYTE";
 	  else if (code == 'w')
@@ -17126,20 +17117,39 @@ ix86_print_operand (FILE *file, rtx x, int code)
 	    size = "QWORD";
 	  else if (code == 'x')
 	    size = "XMMWORD";
-
-	  fputs (size, file);
-	  fputs (" PTR ", file);
+	  else if (mode == BLKmode)
+	    /* ... or BLKmode operands, when not overridden.  */
+	    size = NULL;
+	  else
+	    switch (GET_MODE_SIZE (mode))
+	      {
+	      case 1: size = "BYTE"; break;
+	      case 2: size = "WORD"; break;
+	      case 4: size = "DWORD"; break;
+	      case 8: size = "QWORD"; break;
+	      case 12: size = "TBYTE"; break;
+	      case 16:
+		if (mode == XFmode)
+		  size = "TBYTE";
+		else
+		  size = "XMMWORD";
+		break;
+	      case 32: size = "YMMWORD"; break;
+	      case 64: size = "ZMMWORD"; break;
+	      default:
+		gcc_unreachable ();
+	      }
+	  if (size)
+	    {
+	      fputs (size, file);
+	      fputs (" PTR ", file);
+	    }
 	}
 
-      x = XEXP (x, 0);
-      /* Avoid (%rip) for call operands.  */
-      if (CONSTANT_ADDRESS_P (x) && code == 'P'
-	  && !CONST_INT_P (x))
-	output_addr_const (file, x);
-      else if (this_is_asm_operands && ! address_operand (x, VOIDmode))
+      if (this_is_asm_operands && ! address_operand (addr, VOIDmode))
 	output_operand_lossage ("invalid constraints for operand");
       else
-	output_address (x);
+	ix86_print_operand_address_as (file, addr, MEM_ADDR_SPACE (x));
     }
 
   else if (CONST_DOUBLE_P (x) && GET_MODE (x) == SFmode)
@@ -17224,7 +17234,7 @@ ix86_print_operand_punct_valid_p (unsigned char code)
 /* Print a memory operand whose address is ADDR.  */
 
 static void
-ix86_print_operand_address (FILE *file, rtx addr)
+ix86_print_operand_address_as (FILE *file, rtx addr, addr_space_t as)
 {
   struct ix86_address parts;
   rtx base, index, disp;
@@ -17277,18 +17287,22 @@ ix86_print_operand_address (FILE *file, rtx addr)
   disp = parts.disp;
   scale = parts.scale;
 
-  switch (parts.seg)
+  if (ADDR_SPACE_GENERIC_P (as))
+    as = parts.seg;
+  else
+    gcc_assert (ADDR_SPACE_GENERIC_P (parts.seg));
+
+  if (!ADDR_SPACE_GENERIC_P (as))
     {
-    case SEG_DEFAULT:
-      break;
-    case SEG_FS:
-    case SEG_GS:
-      if (ASSEMBLER_DIALECT == ASM_ATT)
-	putc ('%', file);
-      fputs ((parts.seg == SEG_FS ? "fs:" : "gs:"), file);
-      break;
-    default:
-      gcc_unreachable ();
+      const char *string;
+
+      if (as == ADDR_SPACE_SEG_FS)
+	string = (ASSEMBLER_DIALECT == ASM_ATT ? "%fs:" : "fs:");
+      else if (as == ADDR_SPACE_SEG_GS)
+	string = (ASSEMBLER_DIALECT == ASM_ATT ? "%gs:" : "gs:");
+      else
+	gcc_unreachable ();
+      fputs (string, file);
     }
 
   /* Use one byte shorter RIP relative addressing for 64bit mode.  */
@@ -17447,6 +17461,12 @@ ix86_print_operand_address (FILE *file, rtx addr)
     }
 }
 
+static void
+ix86_print_operand_address (FILE *file, rtx addr)
+{
+  ix86_print_operand_address_as (file, addr, ADDR_SPACE_GENERIC);
+}
+
 /* Implementation of TARGET_ASM_OUTPUT_ADDR_CONST_EXTRA.  */
 
 static bool
@@ -27214,25 +27234,35 @@ ix86_attr_length_address_default (rtx_insn *insn)
 
   extract_insn_cached (insn);
   for (i = recog_data.n_operands - 1; i >= 0; --i)
-    if (MEM_P (recog_data.operand[i]))
-      {
-        constrain_operands_cached (insn, reload_completed);
-        if (which_alternative != -1)
-	  {
-	    const char *constraints = recog_data.constraints[i];
-	    int alt = which_alternative;
-
-	    while (*constraints == '=' || *constraints == '+')
-	      constraints++;
-	    while (alt-- > 0)
-	      while (*constraints++ != ',')
-		;
-	    /* Skip ignored operands.  */
-	    if (*constraints == 'X')
-	      continue;
-	  }
-	return memory_address_length (XEXP (recog_data.operand[i], 0), false);
-      }
+    {
+      rtx op = recog_data.operand[i];
+      if (MEM_P (op))
+	{
+	  constrain_operands_cached (insn, reload_completed);
+	  if (which_alternative != -1)
+	    {
+	      const char *constraints = recog_data.constraints[i];
+	      int alt = which_alternative;
+
+	      while (*constraints == '=' || *constraints == '+')
+		constraints++;
+	      while (alt-- > 0)
+	        while (*constraints++ != ',')
+		  ;
+	      /* Skip ignored operands.  */
+	      if (*constraints == 'X')
+		continue;
+	    }
+
+	  int len = memory_address_length (XEXP (op, 0), false);
+
+	  /* Account for segment prefix for non-default addr spaces.  */
+	  if (!ADDR_SPACE_GENERIC_P (MEM_ADDR_SPACE (op)))
+	    len++;
+
+	  return len;
+	}
+    }
   return 0;
 }
 
@@ -53586,6 +53616,22 @@ ix86_operands_ok_for_move_multiple (rtx *operands, bool load,
   return true;
 }
 
+/* Address space support.
+
+   This is not "far pointers" in the 16-bit sense, but an easy way
+   to use %fs and %gs segment prefixes.  Therefore:
+
+    (a) All address spaces have the same modes,
+    (b) All address spaces have the same addresss forms,
+    (c) While %fs and %gs are technically subsets of the generic
+        address space, they are probably not subsets of each other.
+    (d) Since we have no access to the segment base register values
+        without resorting to a system call, we cannot convert a
+        non-default address space to a default address space.
+        Therefore we do not claim %fs or %gs are subsets of generic.
+
+   Therefore, we need not override any of the address space hooks.  */
+
 /* Initialize the GCC target structure.  */
 #undef TARGET_RETURN_IN_MEMORY
 #define TARGET_RETURN_IN_MEMORY ix86_return_in_memory
diff --git a/gcc/testsuite/gcc.target/i386/addr-space-1.c b/gcc/testsuite/gcc.target/i386/addr-space-1.c
new file mode 100644
index 0000000..1e13147
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/addr-space-1.c
@@ -0,0 +1,11 @@
+/* { dg-do compile */
+/* { dg-options "-O2" } */
+/* { dg-final { scan-assembler "movl\[ \t\]%gs:\\((%eax|%rax)\\), %eax" } } */
+
+extern __seg_gs int *call_me (void);
+
+int
+read_seg_gs (void)
+{
+  return *call_me();
+}
-- 
2.4.3

Re: [PATCH 5/9] i386: Add address spaces for fs/gs segments

[Paolo Bonzini]

On 08/10/2015 06:59, Richard Henderson wrote:
> +/* Address space support.
> +
> +   This is not "far pointers" in the 16-bit sense, but an easy way
> +   to use %fs and %gs segment prefixes.  Therefore:
> +
> +    (a) All address spaces have the same modes,
> +    (b) All address spaces have the same addresss forms,
> +    (c) While %fs and %gs are technically subsets of the generic
> +        address space, they are probably not subsets of each other.
> +    (d) Since we have no access to the segment base register values
> +        without resorting to a system call, we cannot convert a
> +        non-default address space to a default address space.
> +        Therefore we do not claim %fs or %gs are subsets of generic.

rdfsbase and rdgsbase are potentially accessible to userspace too, so I
think %fs or %gs should be considered subsets of generic.

Paolo

> +   Therefore, we need not override any of the address space hooks.  */

Re: [PATCH 5/9] i386: Add address spaces for fs/gs segments

[Richard Henderson]

On 10/16/2015 05:28 AM, Paolo Bonzini wrote:
>
>
> On 08/10/2015 06:59, Richard Henderson wrote:
>> +/* Address space support.
>> +
>> +   This is not "far pointers" in the 16-bit sense, but an easy way
>> +   to use %fs and %gs segment prefixes.  Therefore:
>> +
>> +    (a) All address spaces have the same modes,
>> +    (b) All address spaces have the same addresss forms,
>> +    (c) While %fs and %gs are technically subsets of the generic
>> +        address space, they are probably not subsets of each other.
>> +    (d) Since we have no access to the segment base register values
>> +        without resorting to a system call, we cannot convert a
>> +        non-default address space to a default address space.
>> +        Therefore we do not claim %fs or %gs are subsets of generic.
>
> rdfsbase and rdgsbase are potentially accessible to userspace too, so I
> think %fs or %gs should be considered subsets of generic.

Potentially, yes.  Though in practice not enabled for any current operating 
system that I can find.  Thus at present we still require some sort of syscall.

I suppose we could add a libgcc interface for it, but it certainly wouldn't be 
quick.

r~

[PATCH 9/9] Fix PR 66768

[Richard Henderson]

This is the patch that richi includes in the PR.  There will need to
be an additional patch to solve an ICE for the AVR backend, as noted
in the PR, but this is good enough to solve the bad-code generation
problem for the i386 backend.


	* tree-ssa-address.c (create_mem_ref_raw): Retain the correct
	type for the address base.
---
 gcc/tree-ssa-address.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/tree-ssa-address.c b/gcc/tree-ssa-address.c
index 042f9c9..bd10ae7 100644
--- a/gcc/tree-ssa-address.c
+++ b/gcc/tree-ssa-address.c
@@ -388,7 +388,7 @@ create_mem_ref_raw (tree type, tree alias_ptr_type, struct mem_address *addr,
     }
   else
     {
-      base = build_int_cst (ptr_type_node, 0);
+      base = build_int_cst (build_pointer_type (type), 0);
       index2 = addr->base;
     }
 
-- 
2.4.3

Re: [PATCH 9/9] Fix PR 66768

[Bin.Cheng]

On Thu, Oct 8, 2015 at 12:59 PM, Richard Henderson <rth@redhat.com> wrote:
> This is the patch that richi includes in the PR.  There will need to
> be an additional patch to solve an ICE for the AVR backend, as noted
> in the PR, but this is good enough to solve the bad-code generation
> problem for the i386 backend.
Hi Richard,
For the record, the root cause is in IVO because it fails to preserve
base object.  This patch can only paper over the issue for address
spaces where PTR type and sizetype have the same length, otherwise IVO
generates wrong code which can't be walked around by this patch.  I
will take PR66768.

Thanks,
bin
>
>
>         * tree-ssa-address.c (create_mem_ref_raw): Retain the correct
>         type for the address base.
> ---
>  gcc/tree-ssa-address.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gcc/tree-ssa-address.c b/gcc/tree-ssa-address.c
> index 042f9c9..bd10ae7 100644
> --- a/gcc/tree-ssa-address.c
> +++ b/gcc/tree-ssa-address.c
> @@ -388,7 +388,7 @@ create_mem_ref_raw (tree type, tree alias_ptr_type, struct mem_address *addr,
>      }
>    else
>      {
> -      base = build_int_cst (ptr_type_node, 0);
> +      base = build_int_cst (build_pointer_type (type), 0);
>        index2 = addr->base;
>      }
>
> --
> 2.4.3
>

Re: [PATCH 9/9] Fix PR 66768

[Bernd Schmidt]

On 10/08/2015 07:17 AM, Bin.Cheng wrote:
> On Thu, Oct 8, 2015 at 12:59 PM, Richard Henderson <rth@redhat.com> wrote:
>> This is the patch that richi includes in the PR.  There will need to
>> be an additional patch to solve an ICE for the AVR backend, as noted
>> in the PR, but this is good enough to solve the bad-code generation
>> problem for the i386 backend.
> Hi Richard,
> For the record, the root cause is in IVO because it fails to preserve
> base object.  This patch can only paper over the issue for address
> spaces where PTR type and sizetype have the same length, otherwise IVO
> generates wrong code which can't be walked around by this patch.  I
> will take PR66768.

Hmm. In 2012 I submitted a patch "Preserve pointer types in ivopts", 
which got lost in review. It was for a different problem than address 
spaces, but it might be worth taking a look whether that approach could 
help solve this issue.


Bernd

Re: [PATCH 9/9] Fix PR 66768

[Bin.Cheng]

On Thu, Oct 8, 2015 at 5:55 PM, Bernd Schmidt <bschmidt@redhat.com> wrote:
> On 10/08/2015 07:17 AM, Bin.Cheng wrote:
>>
>> On Thu, Oct 8, 2015 at 12:59 PM, Richard Henderson <rth@redhat.com> wrote:
>>>
>>> This is the patch that richi includes in the PR.  There will need to
>>> be an additional patch to solve an ICE for the AVR backend, as noted
>>> in the PR, but this is good enough to solve the bad-code generation
>>> problem for the i386 backend.
>>
>> Hi Richard,
>> For the record, the root cause is in IVO because it fails to preserve
>> base object.  This patch can only paper over the issue for address
>> spaces where PTR type and sizetype have the same length, otherwise IVO
>> generates wrong code which can't be walked around by this patch.  I
>> will take PR66768.
>
>
> Hmm. In 2012 I submitted a patch "Preserve pointer types in ivopts", which
> got lost in review. It was for a different problem than address spaces, but
> it might be worth taking a look whether that approach could help solve this
> issue.
Hi Bernd,
Thanks for your suggestion, I will search for that patch.

Thanks,
bin
>
>
> Bernd

Re: [PATCH 9/9] Fix PR 66768

[Richard Biener]

On Thu, Oct 8, 2015 at 6:59 AM, Richard Henderson <rth@redhat.com> wrote:
> This is the patch that richi includes in the PR.  There will need to
> be an additional patch to solve an ICE for the AVR backend, as noted
> in the PR, but this is good enough to solve the bad-code generation
> problem for the i386 backend.

For the record, it's obvious.

Thanks,
Richard.

>
>         * tree-ssa-address.c (create_mem_ref_raw): Retain the correct
>         type for the address base.
> ---
>  gcc/tree-ssa-address.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gcc/tree-ssa-address.c b/gcc/tree-ssa-address.c
> index 042f9c9..bd10ae7 100644
> --- a/gcc/tree-ssa-address.c
> +++ b/gcc/tree-ssa-address.c
> @@ -388,7 +388,7 @@ create_mem_ref_raw (tree type, tree alias_ptr_type, struct mem_address *addr,
>      }
>    else
>      {
> -      base = build_int_cst (ptr_type_node, 0);
> +      base = build_int_cst (build_pointer_type (type), 0);
>        index2 = addr->base;
>      }
>
> --
> 2.4.3
>

[PATCH 1/9] Change default of non-overlapping address space conversion

[Richard Henderson]

The current default of making all undefined coversions being
set to null is not useful.  It has caused all users to lie
and say that spaces are subsets when they are not, just so
that they can override the conversion.


	* expr.c (expand_expr_real_2): Use convert_modes on disjoint
	address spaces.
---
 gcc/expr.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/gcc/expr.c b/gcc/expr.c
index 6498a63..fda2ae0 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -8155,34 +8155,40 @@ expand_expr_real_2 (sepops ops, rtx target, machine_mode tmode,
     case ADDR_SPACE_CONVERT_EXPR:
       {
 	tree treeop0_type = TREE_TYPE (treeop0);
-	addr_space_t as_to;
-	addr_space_t as_from;
 
 	gcc_assert (POINTER_TYPE_P (type));
 	gcc_assert (POINTER_TYPE_P (treeop0_type));
 
-	as_to = TYPE_ADDR_SPACE (TREE_TYPE (type));
-	as_from = TYPE_ADDR_SPACE (TREE_TYPE (treeop0_type));
+	addr_space_t as_to = TYPE_ADDR_SPACE (TREE_TYPE (type));
+	addr_space_t as_from = TYPE_ADDR_SPACE (TREE_TYPE (treeop0_type));
 
         /* Conversions between pointers to the same address space should
 	   have been implemented via CONVERT_EXPR / NOP_EXPR.  */
 	gcc_assert (as_to != as_from);
 
+	op0 = expand_expr (treeop0, NULL_RTX, VOIDmode, modifier);
+
         /* Ask target code to handle conversion between pointers
 	   to overlapping address spaces.  */
 	if (targetm.addr_space.subset_p (as_to, as_from)
 	    || targetm.addr_space.subset_p (as_from, as_to))
 	  {
-	    op0 = expand_expr (treeop0, NULL_RTX, VOIDmode, modifier);
 	    op0 = targetm.addr_space.convert (op0, treeop0_type, type);
-	    gcc_assert (op0);
-	    return op0;
 	  }
-
-	/* For disjoint address spaces, converting anything but
-	   a null pointer invokes undefined behaviour.  We simply
-	   always return a null pointer here.  */
-	return CONST0_RTX (mode);
+        else
+          {
+	    /* For disjoint address spaces, converting anything but a null
+	       pointer invokes undefined behaviour.  We truncate or extend the
+	       value as if we'd converted via integers, which handles 0 as
+	       required, and all others as the programmer likely expects.  */
+#ifndef POINTERS_EXTEND_UNSIGNED
+	    const int POINTERS_EXTEND_UNSIGNED = 1;
+#endif
+	    op0 = convert_modes (mode, TYPE_MODE (treeop0_type),
+				 op0, POINTERS_EXTEND_UNSIGNED);
+	  }
+	gcc_assert (op0);
+	return op0;
       }
 
     case POINTER_PLUS_EXPR:
-- 
2.4.3

Re: [PATCH 1/9] Change default of non-overlapping address space conversion

[Bernd Schmidt]

On 10/08/2015 06:59 AM, Richard Henderson wrote:
> The current default of making all undefined coversions being
> set to null is not useful.  It has caused all users to lie
> and say that spaces are subsets when they are not, just so
> that they can override the conversion.
>
>
> 	* expr.c (expand_expr_real_2): Use convert_modes on disjoint
> 	address spaces.

That looks ok.

If you really want to use address spaces, there are two open issues with 
them that I'm aware of: TYPE_ADDR_SPACE on arrays does not hold the 
address space, which sometimes confuses the expanders (at least it did 
in the past), and I seem to recall gcc and gdb don't quite agree on the 
debug information. I'm Cc'ing Luis to see if he still has the gcc and 
gdb patches we worked out.


Bernd

[PATCH 3/9] i386: Handle address spaces in movabs patterns

[Richard Henderson]

Do not discard the address space that might be within the MEM.


	* config/i386/i386.md (*movabs<mode>_1): Print the full memory rtx.
	(*movabs<mode>_2): Likewise.
---
 gcc/config/i386/i386.md | 32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index d0c0d23..ccb672d 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -2595,9 +2595,19 @@
   [(set (mem:SWI1248x (match_operand:DI 0 "x86_64_movabs_operand" "i,r"))
 	(match_operand:SWI1248x 1 "nonmemory_operand" "a,r<i>"))]
   "TARGET_LP64 && ix86_check_movabs (insn, 0)"
-  "@
-   movabs{<imodesuffix>}\t{%1, %P0|[%P0], %1}
-   mov{<imodesuffix>}\t{%1, %a0|<iptrsize> PTR %a0, %1}"
+{
+  /* Recover the full memory rtx.  */
+  operands[0] = SET_DEST (PATTERN (insn));
+  switch (which_alternative)
+    {
+    case 0:
+      return "movabs{<imodesuffix>}\t{%1, %0|%0, %1}";
+    case 1:
+      return "mov{<imodesuffix>}\t{%1, %0|%0, %1}";
+    default:
+      gcc_unreachable ();
+    }
+}
   [(set_attr "type" "imov")
    (set_attr "modrm" "0,*")
    (set_attr "length_address" "8,0")
@@ -2609,9 +2619,19 @@
   [(set (match_operand:SWI1248x 0 "register_operand" "=a,r")
         (mem:SWI1248x (match_operand:DI 1 "x86_64_movabs_operand" "i,r")))]
   "TARGET_LP64 && ix86_check_movabs (insn, 1)"
-  "@
-   movabs{<imodesuffix>}\t{%P1, %0|%0, [%P1]}
-   mov{<imodesuffix>}\t{%a1, %0|%0, <iptrsize> PTR %a1}"
+{
+  /* Recover the full memory rtx.  */
+  operands[1] = SET_SRC (PATTERN (insn));
+  switch (which_alternative)
+    {
+    case 0:
+      return "movabs{<imodesuffix>}\t{%1, %0|%0, %1}";
+    case 1:
+      return "mov{<imodesuffix>}\t{%1, %0|%0, %1}";
+    default:
+      gcc_unreachable ();
+    }
+}
   [(set_attr "type" "imov")
    (set_attr "modrm" "0,*")
    (set_attr "length_address" "8,0")
-- 
2.4.3

[PATCH 8/9] Add TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID

[Richard Henderson]

	* target.def (TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID): New.
	* targhooks.h (default_addr_space_zero_address_valid): Declare.
	* targhooks.c (default_addr_space_zero_address_valid): New.
	* doc/tm.texi, doc/tm.texi.in: Update.
	* config/i386/i386.c (ix86_addr_space_zero_address_valid): New.
	(TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID): New.
	* fold-const.c (const_unop) [ADDR_SPACE_CONVERT_EXPR]: Disable
	folding of 0 when it is a valid address for the source space.
	* gimple.c (check_loadstore): Disable noticing dereference when
	0 is a valid address for the space.
---
 gcc/config/i386/i386.c | 10 ++++++++++
 gcc/doc/tm.texi        |  5 +++++
 gcc/doc/tm.texi.in     |  2 ++
 gcc/fold-const.c       |  6 +++++-
 gcc/gimple.c           | 12 +++++++++---
 gcc/target.def         |  9 +++++++++
 gcc/targhooks.c        |  9 +++++++++
 gcc/targhooks.h        |  1 +
 8 files changed, 50 insertions(+), 4 deletions(-)

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index f7abb00..e2dec2a 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -53669,6 +53669,16 @@ ix86_addr_space_convert (rtx op, tree from_type, tree to_type)
 #undef TARGET_ADDR_SPACE_CONVERT
 #define TARGET_ADDR_SPACE_CONVERT ix86_addr_space_convert
 
+/* All use of segmentation is assumed to make address 0 valid.  */
+
+static bool
+ix86_addr_space_zero_address_valid (addr_space_t as)
+{
+  return as != ADDR_SPACE_GENERIC;
+}
+#undef TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID
+#define TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID ix86_addr_space_zero_address_valid
+
 /* Initialize the GCC target structure.  */
 #undef TARGET_RETURN_IN_MEMORY
 #define TARGET_RETURN_IN_MEMORY ix86_return_in_memory
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 72366b9..238f5f5 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -10312,6 +10312,11 @@ arithmetic operations.  Pointers to a superset address space can be
 converted to pointers to a subset address space via explicit casts.
 @end deftypefn
 
+@deftypefn {Target Hook} bool TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID (addr_space_t @var{as})
+Define this to modify the default handling of address 0 for the
+address space.  Return true if 0 should be considered a valid address.
+@end deftypefn
+
 @deftypefn {Target Hook} rtx TARGET_ADDR_SPACE_CONVERT (rtx @var{op}, tree @var{from_type}, tree @var{to_type})
 Define this to convert the pointer expression represented by the RTL
 @var{op} with type @var{from_type} that points to a named address
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index d8d0087..9230fb2 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -7432,6 +7432,8 @@ c_register_addr_space ("__ea", ADDR_SPACE_EA);
 
 @hook TARGET_ADDR_SPACE_SUBSET_P
 
+@hook TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID
+
 @hook TARGET_ADDR_SPACE_CONVERT
 
 @node Misc
diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 2851a29..2e06217 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -1564,7 +1564,11 @@ const_unop (enum tree_code code, tree type, tree arg0)
       return fold_convert_const (code, type, arg0);
 
     case ADDR_SPACE_CONVERT_EXPR:
-      if (integer_zerop (arg0))
+      /* If the source address is 0, and the source address space
+	 cannot have a valid object at 0, fold to dest type null.  */
+      if (integer_zerop (arg0)
+	  && !(targetm.addr_space.zero_address_valid
+	       (TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg0))))))
 	return fold_convert_const (code, type, arg0);
       break;
 
diff --git a/gcc/gimple.c b/gcc/gimple.c
index c3762e1..e551dac 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -2618,9 +2618,15 @@ nonfreeing_call_p (gimple *call)
 static bool
 check_loadstore (gimple *, tree op, tree, void *data)
 {
-  if ((TREE_CODE (op) == MEM_REF || TREE_CODE (op) == TARGET_MEM_REF)
-      && operand_equal_p (TREE_OPERAND (op, 0), (tree)data, 0))
-    return true;
+  if (TREE_CODE (op) == MEM_REF || TREE_CODE (op) == TARGET_MEM_REF)
+    {
+      /* Some address spaces may legitimately dereference zero.  */
+      addr_space_t as = TYPE_ADDR_SPACE (TREE_TYPE (op));
+      if (targetm.addr_space.zero_address_valid (as))
+	return false;
+
+      return operand_equal_p (TREE_OPERAND (op, 0), (tree)data, 0);
+    }
   return false;
 }
 
diff --git a/gcc/target.def b/gcc/target.def
index d29aad5..f75ce97 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -3185,6 +3185,15 @@ converted to pointers to a subset address space via explicit casts.",
  bool, (addr_space_t subset, addr_space_t superset),
  default_addr_space_subset_p)
 
+/* True if 0 is a valid address in the address space, or false if
+   0 is a NULL in the address space.  */
+DEFHOOK
+(zero_address_valid,
+ "Define this to modify the default handling of address 0 for the\n\
+address space.  Return true if 0 should be considered a valid address.",
+ bool, (addr_space_t as),
+ default_addr_space_zero_address_valid)
+
 /* Function to convert an rtl expression from one address space to another.  */
 DEFHOOK
 (convert,
diff --git a/gcc/targhooks.c b/gcc/targhooks.c
index a8a243c..c78a540 100644
--- a/gcc/targhooks.c
+++ b/gcc/targhooks.c
@@ -1256,6 +1256,15 @@ default_addr_space_subset_p (addr_space_t subset, addr_space_t superset)
   return (subset == superset);
 }
 
+/* The default hook for determining if 0 within a named address
+   space is a valid address.  */
+
+bool
+default_addr_space_zero_address_valid (addr_space_t as ATTRIBUTE_UNUSED)
+{
+  return false;
+}
+
 /* The default hook for TARGET_ADDR_SPACE_CONVERT. This hook should never be
    called for targets with only a generic address space.  */
 
diff --git a/gcc/targhooks.h b/gcc/targhooks.h
index 77c284a..ade3327 100644
--- a/gcc/targhooks.h
+++ b/gcc/targhooks.h
@@ -173,6 +173,7 @@ extern bool default_addr_space_legitimate_address_p (machine_mode, rtx,
 extern rtx default_addr_space_legitimize_address (rtx, rtx, machine_mode,
 						  addr_space_t);
 extern bool default_addr_space_subset_p (addr_space_t, addr_space_t);
+extern bool default_addr_space_zero_address_valid (addr_space_t);
 extern rtx default_addr_space_convert (rtx, tree, tree);
 extern unsigned int default_case_values_threshold (void);
 extern bool default_have_conditional_execution (void);
-- 
2.4.3

Re: [PATCH 8/9] Add TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID

[Richard Biener]

On Thu, Oct 8, 2015 at 6:59 AM, Richard Henderson <rth@redhat.com> wrote:
>         * target.def (TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID): New.
>         * targhooks.h (default_addr_space_zero_address_valid): Declare.
>         * targhooks.c (default_addr_space_zero_address_valid): New.
>         * doc/tm.texi, doc/tm.texi.in: Update.
>         * config/i386/i386.c (ix86_addr_space_zero_address_valid): New.
>         (TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID): New.
>         * fold-const.c (const_unop) [ADDR_SPACE_CONVERT_EXPR]: Disable
>         folding of 0 when it is a valid address for the source space.
>         * gimple.c (check_loadstore): Disable noticing dereference when
>         0 is a valid address for the space.

I think this is incomplete and you need to look at all places that check for
flag_delete_null_pointer_checks (ick, the ubsan abuse looks interesting...).
We'd best abstract that flag check somewhere, only doing the address-space
check for ! ADDR_SPACE_GENERIC.

I also wonder about the const_unop change - if the target address-space
has a valid 0 but the source has not then you create a valid object address
from an invalid one?

The check_loadstore change should instead have adjusted the
flag_delete_null_pointer_checks guard in infer_nonnull_range_by_dereference.

Yes, that flag is used to say whether 0 is a valid address ...

Richard.

> ---
>  gcc/config/i386/i386.c | 10 ++++++++++
>  gcc/doc/tm.texi        |  5 +++++
>  gcc/doc/tm.texi.in     |  2 ++
>  gcc/fold-const.c       |  6 +++++-
>  gcc/gimple.c           | 12 +++++++++---
>  gcc/target.def         |  9 +++++++++
>  gcc/targhooks.c        |  9 +++++++++
>  gcc/targhooks.h        |  1 +
>  8 files changed, 50 insertions(+), 4 deletions(-)
>
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index f7abb00..e2dec2a 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -53669,6 +53669,16 @@ ix86_addr_space_convert (rtx op, tree from_type, tree to_type)
>  #undef TARGET_ADDR_SPACE_CONVERT
>  #define TARGET_ADDR_SPACE_CONVERT ix86_addr_space_convert
>
> +/* All use of segmentation is assumed to make address 0 valid.  */
> +
> +static bool
> +ix86_addr_space_zero_address_valid (addr_space_t as)
> +{
> +  return as != ADDR_SPACE_GENERIC;
> +}
> +#undef TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID
> +#define TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID ix86_addr_space_zero_address_valid
> +
>  /* Initialize the GCC target structure.  */
>  #undef TARGET_RETURN_IN_MEMORY
>  #define TARGET_RETURN_IN_MEMORY ix86_return_in_memory
> diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
> index 72366b9..238f5f5 100644
> --- a/gcc/doc/tm.texi
> +++ b/gcc/doc/tm.texi
> @@ -10312,6 +10312,11 @@ arithmetic operations.  Pointers to a superset address space can be
>  converted to pointers to a subset address space via explicit casts.
>  @end deftypefn
>
> +@deftypefn {Target Hook} bool TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID (addr_space_t @var{as})
> +Define this to modify the default handling of address 0 for the
> +address space.  Return true if 0 should be considered a valid address.
> +@end deftypefn
> +
>  @deftypefn {Target Hook} rtx TARGET_ADDR_SPACE_CONVERT (rtx @var{op}, tree @var{from_type}, tree @var{to_type})
>  Define this to convert the pointer expression represented by the RTL
>  @var{op} with type @var{from_type} that points to a named address
> diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
> index d8d0087..9230fb2 100644
> --- a/gcc/doc/tm.texi.in
> +++ b/gcc/doc/tm.texi.in
> @@ -7432,6 +7432,8 @@ c_register_addr_space ("__ea", ADDR_SPACE_EA);
>
>  @hook TARGET_ADDR_SPACE_SUBSET_P
>
> +@hook TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID
> +
>  @hook TARGET_ADDR_SPACE_CONVERT
>
>  @node Misc
> diff --git a/gcc/fold-const.c b/gcc/fold-const.c
> index 2851a29..2e06217 100644
> --- a/gcc/fold-const.c
> +++ b/gcc/fold-const.c
> @@ -1564,7 +1564,11 @@ const_unop (enum tree_code code, tree type, tree arg0)
>        return fold_convert_const (code, type, arg0);
>
>      case ADDR_SPACE_CONVERT_EXPR:
> -      if (integer_zerop (arg0))
> +      /* If the source address is 0, and the source address space
> +        cannot have a valid object at 0, fold to dest type null.  */
> +      if (integer_zerop (arg0)
> +         && !(targetm.addr_space.zero_address_valid
> +              (TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg0))))))
>         return fold_convert_const (code, type, arg0);
>        break;
>
> diff --git a/gcc/gimple.c b/gcc/gimple.c
> index c3762e1..e551dac 100644
> --- a/gcc/gimple.c
> +++ b/gcc/gimple.c
> @@ -2618,9 +2618,15 @@ nonfreeing_call_p (gimple *call)
>  static bool
>  check_loadstore (gimple *, tree op, tree, void *data)
>  {
> -  if ((TREE_CODE (op) == MEM_REF || TREE_CODE (op) == TARGET_MEM_REF)
> -      && operand_equal_p (TREE_OPERAND (op, 0), (tree)data, 0))
> -    return true;
> +  if (TREE_CODE (op) == MEM_REF || TREE_CODE (op) == TARGET_MEM_REF)
> +    {
> +      /* Some address spaces may legitimately dereference zero.  */
> +      addr_space_t as = TYPE_ADDR_SPACE (TREE_TYPE (op));
> +      if (targetm.addr_space.zero_address_valid (as))
> +       return false;
> +
> +      return operand_equal_p (TREE_OPERAND (op, 0), (tree)data, 0);
> +    }
>    return false;
>  }
>
> diff --git a/gcc/target.def b/gcc/target.def
> index d29aad5..f75ce97 100644
> --- a/gcc/target.def
> +++ b/gcc/target.def
> @@ -3185,6 +3185,15 @@ converted to pointers to a subset address space via explicit casts.",
>   bool, (addr_space_t subset, addr_space_t superset),
>   default_addr_space_subset_p)
>
> +/* True if 0 is a valid address in the address space, or false if
> +   0 is a NULL in the address space.  */
> +DEFHOOK
> +(zero_address_valid,
> + "Define this to modify the default handling of address 0 for the\n\
> +address space.  Return true if 0 should be considered a valid address.",
> + bool, (addr_space_t as),
> + default_addr_space_zero_address_valid)
> +
>  /* Function to convert an rtl expression from one address space to another.  */
>  DEFHOOK
>  (convert,
> diff --git a/gcc/targhooks.c b/gcc/targhooks.c
> index a8a243c..c78a540 100644
> --- a/gcc/targhooks.c
> +++ b/gcc/targhooks.c
> @@ -1256,6 +1256,15 @@ default_addr_space_subset_p (addr_space_t subset, addr_space_t superset)
>    return (subset == superset);
>  }
>
> +/* The default hook for determining if 0 within a named address
> +   space is a valid address.  */
> +
> +bool
> +default_addr_space_zero_address_valid (addr_space_t as ATTRIBUTE_UNUSED)
> +{
> +  return false;
> +}
> +
>  /* The default hook for TARGET_ADDR_SPACE_CONVERT. This hook should never be
>     called for targets with only a generic address space.  */
>
> diff --git a/gcc/targhooks.h b/gcc/targhooks.h
> index 77c284a..ade3327 100644
> --- a/gcc/targhooks.h
> +++ b/gcc/targhooks.h
> @@ -173,6 +173,7 @@ extern bool default_addr_space_legitimate_address_p (machine_mode, rtx,
>  extern rtx default_addr_space_legitimize_address (rtx, rtx, machine_mode,
>                                                   addr_space_t);
>  extern bool default_addr_space_subset_p (addr_space_t, addr_space_t);
> +extern bool default_addr_space_zero_address_valid (addr_space_t);
>  extern rtx default_addr_space_convert (rtx, tree, tree);
>  extern unsigned int default_case_values_threshold (void);
>  extern bool default_have_conditional_execution (void);
> --
> 2.4.3
>

Re: [PATCH 8/9] Add TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID

[Richard Henderson]

On 10/08/2015 09:20 PM, Richard Biener wrote:
> On Thu, Oct 8, 2015 at 6:59 AM, Richard Henderson <rth@redhat.com> wrote:
>>          * target.def (TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID): New.
>>          * targhooks.h (default_addr_space_zero_address_valid): Declare.
>>          * targhooks.c (default_addr_space_zero_address_valid): New.
>>          * doc/tm.texi, doc/tm.texi.in: Update.
>>          * config/i386/i386.c (ix86_addr_space_zero_address_valid): New.
>>          (TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID): New.
>>          * fold-const.c (const_unop) [ADDR_SPACE_CONVERT_EXPR]: Disable
>>          folding of 0 when it is a valid address for the source space.
>>          * gimple.c (check_loadstore): Disable noticing dereference when
>>          0 is a valid address for the space.
>
> I think this is incomplete and you need to look at all places that check for
> flag_delete_null_pointer_checks (ick, the ubsan abuse looks interesting...).
> We'd best abstract that flag check somewhere, only doing the address-space
> check for ! ADDR_SPACE_GENERIC.

I did a fair survey of the uses of f_d_n_p_c.  Most of them are tests for 
explicit symbols that are weak, etc.  I suppose we should probably then check 
to see if the symbol is placed in a non-default address space, but in the 
context of the code I was working on, that never comes up.

> I also wonder about the const_unop change - if the target address-space
> has a valid 0 but the source has not then you create a valid object address
> from an invalid one?

I guess we would, but ... what else can you do when there's no invalid address?

> The check_loadstore change should instead have adjusted the
> flag_delete_null_pointer_checks guard in infer_nonnull_range_by_dereference.

Nope, that doesn't work.  You have to wait until you see the actual MEM being 
dereferenced before you can look at it's address space.


r~

Re: [PATCH 8/9] Add TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID

[Richard Biener]

On Thu, Oct 8, 2015 at 11:10 PM, Richard Henderson <rth@redhat.com> wrote:
> On 10/08/2015 09:20 PM, Richard Biener wrote:
>>
>> On Thu, Oct 8, 2015 at 6:59 AM, Richard Henderson <rth@redhat.com> wrote:
>>>
>>>          * target.def (TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID): New.
>>>          * targhooks.h (default_addr_space_zero_address_valid): Declare.
>>>          * targhooks.c (default_addr_space_zero_address_valid): New.
>>>          * doc/tm.texi, doc/tm.texi.in: Update.
>>>          * config/i386/i386.c (ix86_addr_space_zero_address_valid): New.
>>>          (TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID): New.
>>>          * fold-const.c (const_unop) [ADDR_SPACE_CONVERT_EXPR]: Disable
>>>          folding of 0 when it is a valid address for the source space.
>>>          * gimple.c (check_loadstore): Disable noticing dereference when
>>>          0 is a valid address for the space.
>>
>>
>> I think this is incomplete and you need to look at all places that check
>> for
>> flag_delete_null_pointer_checks (ick, the ubsan abuse looks
>> interesting...).
>> We'd best abstract that flag check somewhere, only doing the address-space
>> check for ! ADDR_SPACE_GENERIC.
>
>
> I did a fair survey of the uses of f_d_n_p_c.  Most of them are tests for
> explicit symbols that are weak, etc.  I suppose we should probably then
> check to see if the symbol is placed in a non-default address space, but in
> the context of the code I was working on, that never comes up.

One I know about is tree-ssa-structalias.c:get_constraint_for_1 which treats
zero address specially.  A zero_address_valid_p predicate taking a
pointer (type)
would be nice to have to abstract those flag checks appropriately.

>> I also wonder about the const_unop change - if the target address-space
>> has a valid 0 but the source has not then you create a valid object
>> address
>> from an invalid one?
>
>
> I guess we would, but ... what else can you do when there's no invalid
> address?

True ... maybe a less likely valid one?  (0xfff...ff?)

>> The check_loadstore change should instead have adjusted the
>> flag_delete_null_pointer_checks guard in
>> infer_nonnull_range_by_dereference.
>
>
> Nope, that doesn't work.  You have to wait until you see the actual MEM
> being dereferenced before you can look at it's address space.

Well, as we are explicitely looking for the pointer 'op' we know the
address-space
beforehand, no?  TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (op)))?

Richard.

>
> r~

Re: [PATCH 8/9] Add TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID

[Richard Henderson]

On 10/12/2015 09:10 PM, Richard Biener wrote:
>>> The check_loadstore change should instead have adjusted the
>>> flag_delete_null_pointer_checks guard in
>>> infer_nonnull_range_by_dereference.
>>
>>
>> Nope, that doesn't work.  You have to wait until you see the actual MEM
>> being dereferenced before you can look at it's address space.
>
> Well, as we are explicitely looking for the pointer 'op' we know the
> address-space
> beforehand, no?  TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (op)))?

No.  We don't even know what type we're looking for; we're merely looking for 
any use of NULL within any memory reference within STMT.

Specifically, when we're not looking for a specific SSA_NAME (which would be 
properly typed), we always pass in a plain (void *)0:

           bool by_dereference
             = infer_nonnull_range_by_dereference (stmt, null_pointer_node);



r~

Re: [PATCH 8/9] Add TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID

[Richard Biener]

On Tue, Oct 13, 2015 at 1:27 AM, Richard Henderson <rth@redhat.com> wrote:
> On 10/12/2015 09:10 PM, Richard Biener wrote:
>>>>
>>>> The check_loadstore change should instead have adjusted the
>>>> flag_delete_null_pointer_checks guard in
>>>> infer_nonnull_range_by_dereference.
>>>
>>>
>>>
>>> Nope, that doesn't work.  You have to wait until you see the actual MEM
>>> being dereferenced before you can look at it's address space.
>>
>>
>> Well, as we are explicitely looking for the pointer 'op' we know the
>> address-space
>> beforehand, no?  TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (op)))?
>
>
> No.  We don't even know what type we're looking for; we're merely looking
> for any use of NULL within any memory reference within STMT.
>
> Specifically, when we're not looking for a specific SSA_NAME (which would be
> properly typed), we always pass in a plain (void *)0:
>
>           bool by_dereference
>             = infer_nonnull_range_by_dereference (stmt, null_pointer_node);

Ick.

Richard.

>
>
> r~

Re: [PATCH 8/9] Add TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID

[Jeff Law]

On 10/13/2015 04:13 AM, Richard Biener wrote:
> On Tue, Oct 13, 2015 at 1:27 AM, Richard Henderson <rth@redhat.com> wrote:
>> On 10/12/2015 09:10 PM, Richard Biener wrote:
>>>>>
>>>>> The check_loadstore change should instead have adjusted the
>>>>> flag_delete_null_pointer_checks guard in
>>>>> infer_nonnull_range_by_dereference.
>>>>
>>>>
>>>>
>>>> Nope, that doesn't work.  You have to wait until you see the actual MEM
>>>> being dereferenced before you can look at it's address space.
>>>
>>>
>>> Well, as we are explicitely looking for the pointer 'op' we know the
>>> address-space
>>> beforehand, no?  TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (op)))?
>>
>>
>> No.  We don't even know what type we're looking for; we're merely looking
>> for any use of NULL within any memory reference within STMT.
>>
>> Specifically, when we're not looking for a specific SSA_NAME (which would be
>> properly typed), we always pass in a plain (void *)0:
>>
>>            bool by_dereference
>>              = infer_nonnull_range_by_dereference (stmt, null_pointer_node);
>
> Ick.
It's just looking to see if there's an explicit *0 in stmt.  That can 
occur due to cprop & friends obviously.  It was an easy way to avoid 
having to write a special walker.

The problem here is we don't know what address space the *0 is going to 
hit, right?   Isn't that also an issue for code generation as well?

Jeff

Re: [PATCH 8/9] Add TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID

[Richard Henderson]

On 10/14/2015 02:49 AM, Jeff Law wrote:
> The problem here is we don't know what address space the *0 is going to hit,
> right?

Correct, not before we do the walk of stmt to see what's present.

> Isn't that also an issue for code generation as well?

What sort of problem are you thinking of?  I haven't seen one yet.


r~

Re: [PATCH 8/9] Add TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID

[Richard Biener]

On Tue, Oct 13, 2015 at 10:59 PM, Richard Henderson <rth@redhat.com> wrote:
> On 10/14/2015 02:49 AM, Jeff Law wrote:
>>
>> The problem here is we don't know what address space the *0 is going to
>> hit,
>> right?
>
>
> Correct, not before we do the walk of stmt to see what's present.
>
>> Isn't that also an issue for code generation as well?
>
>
> What sort of problem are you thinking of?  I haven't seen one yet.

The actual dereference of course has a properly address-space qualified zero.

Only your walking depends on operand_equal_p to treat different address-space
zero addresses as equal (which they are of course not ...):


int
operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags)
{
...
  /* Check equality of integer constants before bailing out due to
     precision differences.  */
  if (TREE_CODE (arg0) == INTEGER_CST && TREE_CODE (arg1) == INTEGER_CST)
    {
      /* Address of INTEGER_CST is not defined; check that we did not forget
         to drop the OEP_ADDRESS_OF/OEP_CONSTANT_ADDRESS_OF flags.  */
      gcc_checking_assert (!(flags
                             & (OEP_ADDRESS_OF | OEP_CONSTANT_ADDRESS_OF)));
      return tree_int_cst_equal (arg0, arg1);
    }

but only later we do

      /* We cannot consider pointers to different address space equal.  */
      if (POINTER_TYPE_P (TREE_TYPE (arg0))
                          && POINTER_TYPE_P (TREE_TYPE (arg1))
          && (TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg0)))
              != TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg1)))))
        return 0;

So "fixing" that would make the walker only look for default
address-space zero dereferences.

I think we need to fix operand_equal_p anyway because 0 is clearly not
equal to 0 (only if
they convert to the same literal)

Richard.


>
> r~

Re: [PATCH 8/9] Add TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID

[Richard Biener]

On Wed, Oct 14, 2015 at 11:19 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Tue, Oct 13, 2015 at 10:59 PM, Richard Henderson <rth@redhat.com> wrote:
>> On 10/14/2015 02:49 AM, Jeff Law wrote:
>>>
>>> The problem here is we don't know what address space the *0 is going to
>>> hit,
>>> right?
>>
>>
>> Correct, not before we do the walk of stmt to see what's present.
>>
>>> Isn't that also an issue for code generation as well?
>>
>>
>> What sort of problem are you thinking of?  I haven't seen one yet.
>
> The actual dereference of course has a properly address-space qualified zero.
>
> Only your walking depends on operand_equal_p to treat different address-space
> zero addresses as equal (which they are of course not ...):
>
>
> int
> operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags)
> {
> ...
>   /* Check equality of integer constants before bailing out due to
>      precision differences.  */
>   if (TREE_CODE (arg0) == INTEGER_CST && TREE_CODE (arg1) == INTEGER_CST)
>     {
>       /* Address of INTEGER_CST is not defined; check that we did not forget
>          to drop the OEP_ADDRESS_OF/OEP_CONSTANT_ADDRESS_OF flags.  */
>       gcc_checking_assert (!(flags
>                              & (OEP_ADDRESS_OF | OEP_CONSTANT_ADDRESS_OF)));
>       return tree_int_cst_equal (arg0, arg1);
>     }
>
> but only later we do
>
>       /* We cannot consider pointers to different address space equal.  */
>       if (POINTER_TYPE_P (TREE_TYPE (arg0))
>                           && POINTER_TYPE_P (TREE_TYPE (arg1))
>           && (TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg0)))
>               != TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg1)))))
>         return 0;
>
> So "fixing" that would make the walker only look for default
> address-space zero dereferences.
>
> I think we need to fix operand_equal_p anyway because 0 is clearly not
> equal to 0 (only if
> they convert to the same literal)

I think you could trigger bogus CSE of dereferences of literal addresses
from different address-spaces.

Richard.

> Richard.
>
>
>>
>> r~

Re: [PATCH 8/9] Add TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID

[Richard Henderson]

On 10/14/2015 08:22 PM, Richard Biener wrote:
> I think you could trigger bogus CSE of dereferences of literal addresses
> from different address-spaces.

Good catch.  You're spot on with that.


r~


int test(void)
{
   int __seg_fs *f = (int __seg_fs *)16;
   int __seg_gs *g = (int __seg_gs *)16;
   return *f + *g;
}

test:
	movl	%fs:16, %eax
	addl	%eax, %eax
	ret

Re: [PATCH 8/9] Add TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID

[Jeff Law]

On 10/14/2015 03:19 AM, Richard Biener wrote:
> On Tue, Oct 13, 2015 at 10:59 PM, Richard Henderson <rth@redhat.com> wrote:
>> On 10/14/2015 02:49 AM, Jeff Law wrote:
>>>
>>> The problem here is we don't know what address space the *0 is going to
>>> hit,
>>> right?
>>
>>
>> Correct, not before we do the walk of stmt to see what's present.
>>
>>> Isn't that also an issue for code generation as well?
>>
>>
>> What sort of problem are you thinking of?  I haven't seen one yet.
>
> The actual dereference of course has a properly address-space qualified zero.
OK.  That's the bit I was missing and hinted out in the message I just 
sent -- the address-space information is carried outside the address. 
It seems that we're carrying it in the type, which is probably sensible 
at some level.


>
> Only your walking depends on operand_equal_p to treat different address-space
> zero addresses as equal (which they are of course not ...):
And that's the key problem with carrying the address-space information 
outside the address.   We have to look at more than just the raw address 
to determine if we've got a faulting *0 vs an address space qualified *0.

>
>
> int
> operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags)
> {
> ...
>    /* Check equality of integer constants before bailing out due to
>       precision differences.  */
>    if (TREE_CODE (arg0) == INTEGER_CST && TREE_CODE (arg1) == INTEGER_CST)
>      {
>        /* Address of INTEGER_CST is not defined; check that we did not forget
>           to drop the OEP_ADDRESS_OF/OEP_CONSTANT_ADDRESS_OF flags.  */
>        gcc_checking_assert (!(flags
>                               & (OEP_ADDRESS_OF | OEP_CONSTANT_ADDRESS_OF)));
>        return tree_int_cst_equal (arg0, arg1);
>      }
>
> but only later we do
>
>        /* We cannot consider pointers to different address space equal.  */
>        if (POINTER_TYPE_P (TREE_TYPE (arg0))
>                            && POINTER_TYPE_P (TREE_TYPE (arg1))
>            && (TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg0)))
>                != TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg1)))))
>          return 0;
>
> So "fixing" that would make the walker only look for default
> address-space zero dereferences.
Agreed.

>
> I think we need to fix operand_equal_p anyway because 0 is clearly not
> equal to 0 (only if they convert to the same literal)
My worry here is we'd be getting onto a slippery slope.  But it may be 
unavoidable.

jeff

Re: [PATCH 8/9] Add TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID

[Jeff Law]

On 10/13/2015 02:59 PM, Richard Henderson wrote:
> On 10/14/2015 02:49 AM, Jeff Law wrote:
>> The problem here is we don't know what address space the *0 is going
>> to hit,
>> right?
>
> Correct, not before we do the walk of stmt to see what's present.
So the address space information isn't part of the address?  I must 
admit I haven't looked at how that stuff is being implemented.

>
>> Isn't that also an issue for code generation as well?
>
> What sort of problem are you thinking of?  I haven't seen one yet.
If the address space information was supposed to be carried in the 
address itself, then we'd need the address to be distinct from 
NULL_POINTER_NODE.

It sounds to me like you're carrying address space information outside 
the address itself, which avoid those issues.  However, it does mean 
that the path isolation code needs some kind of adjustment to 
distinguish between *0 that will fault and *0 which hits a different 
address space and may not fault.

jeff

[PATCH 6/9] i386: Replace ix86_address_seg with addr_space_t

[Richard Henderson]

	* config/i386/i386-protos.h (enum ix86_address_seg): Remove.
	(struct ix86_address): Use addr_space_t for seg.
	* config/i386/i386.c (ix86_decompose_address): Use ADDR_SPACE_* names.
	(ix86_legitimate_address_p): Likewise.
	(ix86_print_operand_address_as): Likewise.
	(memory_address_length): Likewise.
	* config/i386/i386.h (DEFAULT_TLS_SEG_REG): Likewise.
	* config/i386/rdos.h (DEFAULT_TLS_SEG_REG): Likewise.
	* config/i386/predicates.md (address_no_seg_operand): Likewise.
	(vsib_address_operand): Likewise.
	(address_mpx_no_base_operand, address_mpx_no_index_operand): Likewise.
---
 gcc/config/i386/i386-protos.h |  3 +--
 gcc/config/i386/i386.c        | 12 ++++++------
 gcc/config/i386/i386.h        |  3 ++-
 gcc/config/i386/predicates.md |  8 ++++----
 gcc/config/i386/rdos.h        |  2 +-
 5 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/gcc/config/i386/i386-protos.h b/gcc/config/i386/i386-protos.h
index f942ef5..4e6d9ad 100644
--- a/gcc/config/i386/i386-protos.h
+++ b/gcc/config/i386/i386-protos.h
@@ -279,12 +279,11 @@ extern rtx maybe_get_pool_constant (rtx);
 extern char internal_label_prefix[16];
 extern int internal_label_prefix_len;
 
-enum ix86_address_seg { SEG_DEFAULT, SEG_FS, SEG_GS };
 struct ix86_address
 {
   rtx base, index, disp;
   HOST_WIDE_INT scale;
-  enum ix86_address_seg seg;
+  addr_space_t seg;
 };
 
 extern int ix86_decompose_address (rtx, struct ix86_address *);
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 8a41f20..f5a0b8f 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -13951,7 +13951,7 @@ ix86_decompose_address (rtx addr, struct ix86_address *out)
   rtx scale_rtx = NULL_RTX;
   rtx tmp;
   int retval = 1;
-  enum ix86_address_seg seg = SEG_DEFAULT;
+  addr_space_t seg = ADDR_SPACE_GENERIC;
 
   /* Allow zero-extended SImode addresses,
      they will be emitted with addr32 prefix.  */
@@ -14050,7 +14050,7 @@ ix86_decompose_address (rtx addr, struct ix86_address *out)
 	    case UNSPEC:
 	      if (XINT (op, 1) == UNSPEC_TP
 	          && TARGET_TLS_DIRECT_SEG_REFS
-	          && seg == SEG_DEFAULT)
+	          && seg == ADDR_SPACE_GENERIC)
 		seg = DEFAULT_TLS_SEG_REG;
 	      else
 		return 0;
@@ -14638,7 +14638,7 @@ ix86_legitimate_address_p (machine_mode, rtx addr, bool strict)
   struct ix86_address parts;
   rtx base, index, disp;
   HOST_WIDE_INT scale;
-  enum ix86_address_seg seg;
+  addr_space_t seg;
 
   if (ix86_decompose_address (addr, &parts) <= 0)
     /* Decomposition failed.  */
@@ -14684,7 +14684,7 @@ ix86_legitimate_address_p (machine_mode, rtx addr, bool strict)
     return false;
 
   /* Address override works only on the (%reg) part of %fs:(%reg).  */
-  if (seg != SEG_DEFAULT
+  if (seg != ADDR_SPACE_GENERIC
       && ((base && GET_MODE (base) != word_mode)
 	  || (index && GET_MODE (index) != word_mode)))
     return false;
@@ -17326,7 +17326,7 @@ ix86_print_operand_address_as (FILE *file, rtx addr, addr_space_t as)
 
       if (CONST_INT_P (disp))
 	{
-	  if (ASSEMBLER_DIALECT == ASM_INTEL && parts.seg == SEG_DEFAULT)
+	  if (ASSEMBLER_DIALECT == ASM_INTEL && parts.seg == ADDR_SPACE_GENERIC)
 	    fputs ("ds:", file);
 	  fprintf (file, HOST_WIDE_INT_PRINT_DEC, INTVAL (disp));
 	}
@@ -27073,7 +27073,7 @@ memory_address_length (rtx addr, bool lea)
   ok = ix86_decompose_address (addr, &parts);
   gcc_assert (ok);
 
-  len = (parts.seg == SEG_DEFAULT) ? 0 : 1;
+  len = (parts.seg == ADDR_SPACE_GENERIC) ? 0 : 1;
 
   /*  If this is not LEA instruction, add the length of addr32 prefix.  */
   if (TARGET_64BIT && !lea
diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h
index a24dea5..d9a78df 100644
--- a/gcc/config/i386/i386.h
+++ b/gcc/config/i386/i386.h
@@ -598,7 +598,8 @@ extern tree x86_mfence;
 #define DEFAULT_ABI SYSV_ABI
 
 /* The default TLS segment register used by target.  */
-#define DEFAULT_TLS_SEG_REG (TARGET_64BIT ? SEG_FS : SEG_GS)
+#define DEFAULT_TLS_SEG_REG \
+  (TARGET_64BIT ? ADDR_SPACE_SEG_FS : ADDR_SPACE_SEG_GS)
 
 /* Subtargets may reset this to 1 in order to enable 96-bit long double
    with the rounding mode forced to 53 bits.  */
diff --git a/gcc/config/i386/predicates.md b/gcc/config/i386/predicates.md
index 042b949..7b2584c 100644
--- a/gcc/config/i386/predicates.md
+++ b/gcc/config/i386/predicates.md
@@ -969,7 +969,7 @@
 
   ok = ix86_decompose_address (op, &parts);
   gcc_assert (ok);
-  return parts.seg == SEG_DEFAULT;
+  return parts.seg == ADDR_SPACE_GENERIC;
 })
 
 ;; Return true if op if a valid base register, displacement or
@@ -983,7 +983,7 @@
 
   ok = ix86_decompose_address (op, &parts);
   gcc_assert (ok);
-  if (parts.index || parts.seg != SEG_DEFAULT)
+  if (parts.index || parts.seg != ADDR_SPACE_GENERIC)
     return false;
 
   /* VSIB addressing doesn't support (%rip).  */
@@ -1027,7 +1027,7 @@
   if (parts.index && parts.base)
     return false;
 
-  if (parts.seg != SEG_DEFAULT)
+  if (parts.seg != ADDR_SPACE_GENERIC)
     return false;
 
   /* Do not support (%rip).  */
@@ -1059,7 +1059,7 @@
   if (parts.index)
     return false;
 
-  if (parts.seg != SEG_DEFAULT)
+  if (parts.seg != ADDR_SPACE_GENERIC)
     return false;
 
   /* Do not support (%rip).  */
diff --git a/gcc/config/i386/rdos.h b/gcc/config/i386/rdos.h
index f9bfe6d..ccf6b78 100644
--- a/gcc/config/i386/rdos.h
+++ b/gcc/config/i386/rdos.h
@@ -25,7 +25,7 @@ along with GCC; see the file COPYING3.  If not see
 #define TARGET_TLS_DIRECT_SEG_REFS_DEFAULT MASK_TLS_DIRECT_SEG_REFS
 
 #undef DEFAULT_TLS_SEG_REG
-#define DEFAULT_TLS_SEG_REG SEG_GS 
+#define DEFAULT_TLS_SEG_REG ADDR_SPACE_SEG_GS
 
 #undef TARGET_RDOS
 #define TARGET_RDOS 1
-- 
2.4.3

[PATCH 7/9] i386: Add address space for tls

[Richard Henderson]

	* config/i386/i386-protos.h (ADDR_SPACE_SEG_TLS): New.
	* config/i386/i386-c.c (ix86_target_macros): Define __SEG_TLS.
	(ix86_register_pragmas): Register __seg_tls.
	* config/i386/i386.c (ix86_print_operand_address_as): Handle
	ADDR_SPACE_SEG_TLS.
	(ix86_addr_space_subset_p): New.
	(TARGET_ADDR_SPACE_SUBSET_P): New.
	(ix86_addr_space_convert): New.
	(TARGET_ADDR_SPACE_CONVERT): New.
---
 gcc/config/i386/i386-c.c      |  2 ++
 gcc/config/i386/i386-protos.h |  1 +
 gcc/config/i386/i386.c        | 39 ++++++++++++++++++++++++++++++++++++++-
 3 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/gcc/config/i386/i386-c.c b/gcc/config/i386/i386-c.c
index e21b947..71c12de 100644
--- a/gcc/config/i386/i386-c.c
+++ b/gcc/config/i386/i386-c.c
@@ -595,6 +595,7 @@ ix86_target_macros (void)
 
   cpp_define (parse_in, "__SEG_FS");
   cpp_define (parse_in, "__SEG_GS");
+  cpp_define (parse_in, "__SEG_TLS");
 }
 
 \f
@@ -611,6 +612,7 @@ ix86_register_pragmas (void)
 
   c_register_addr_space ("__seg_fs", ADDR_SPACE_SEG_FS);
   c_register_addr_space ("__seg_gs", ADDR_SPACE_SEG_GS);
+  c_register_addr_space ("__seg_tls", ADDR_SPACE_SEG_TLS);
 
 #ifdef REGISTER_SUBTARGET_PRAGMAS
   REGISTER_SUBTARGET_PRAGMAS ();
diff --git a/gcc/config/i386/i386-protos.h b/gcc/config/i386/i386-protos.h
index 4e6d9ad..026b778 100644
--- a/gcc/config/i386/i386-protos.h
+++ b/gcc/config/i386/i386-protos.h
@@ -328,3 +328,4 @@ struct ix86_first_cycle_multipass_data_
 
 const addr_space_t ADDR_SPACE_SEG_FS = 1;
 const addr_space_t ADDR_SPACE_SEG_GS = 2;
+const addr_space_t ADDR_SPACE_SEG_TLS = 3;
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index f5a0b8f..f7abb00 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -17296,6 +17296,8 @@ ix86_print_operand_address_as (FILE *file, rtx addr, addr_space_t as)
     {
       const char *string;
 
+      if (as == ADDR_SPACE_SEG_TLS)
+	as = DEFAULT_TLS_SEG_REG;
       if (as == ADDR_SPACE_SEG_FS)
 	string = (ASSEMBLER_DIALECT == ASM_ATT ? "%fs:" : "fs:");
       else if (as == ADDR_SPACE_SEG_GS)
@@ -53629,8 +53631,43 @@ ix86_operands_ok_for_move_multiple (rtx *operands, bool load,
         without resorting to a system call, we cannot convert a
         non-default address space to a default address space.
         Therefore we do not claim %fs or %gs are subsets of generic.
+    (e) However, __seg_tls uses UNSPEC_TP as the base (which itself is
+	stored at __seg_tls:0) so we can map between tls and generic.  */
 
-   Therefore, we need not override any of the address space hooks.  */
+static bool
+ix86_addr_space_subset_p (addr_space_t subset, addr_space_t superset)
+{
+    return (subset == superset
+	    || (superset == ADDR_SPACE_GENERIC
+		&& subset == ADDR_SPACE_SEG_TLS));
+}
+#undef TARGET_ADDR_SPACE_SUBSET_P
+#define TARGET_ADDR_SPACE_SUBSET_P ix86_addr_space_subset_p
+
+static rtx
+ix86_addr_space_convert (rtx op, tree from_type, tree to_type)
+{
+  addr_space_t from_as = TYPE_ADDR_SPACE (TREE_TYPE (from_type));
+  addr_space_t to_as = TYPE_ADDR_SPACE (TREE_TYPE (to_type));
+
+  /* Conversion between SEG_TLS and GENERIC is handled by adding or
+     subtracting the thread pointer.  */
+  if ((from_as == ADDR_SPACE_GENERIC && to_as == ADDR_SPACE_SEG_TLS)
+      || (from_as == ADDR_SPACE_SEG_TLS && to_as == ADDR_SPACE_GENERIC))
+    {
+      machine_mode mode = GET_MODE (op);
+      if (mode == VOIDmode)
+	mode = ptr_mode;
+      rtx tp = get_thread_pointer (mode, optimize || mode != ptr_mode);
+      return expand_binop (mode, (to_as == ADDR_SPACE_GENERIC
+				  ? add_optab : sub_optab),
+			   op, tp, NULL, 1, OPTAB_WIDEN);
+    }
+
+  return op;
+}
+#undef TARGET_ADDR_SPACE_CONVERT
+#define TARGET_ADDR_SPACE_CONVERT ix86_addr_space_convert
 
 /* Initialize the GCC target structure.  */
 #undef TARGET_RETURN_IN_MEMORY
-- 
2.4.3

Re: [PATCH 7/9] i386: Add address space for tls

[Florian Weimer]

On 10/08/2015 06:59 AM, Richard Henderson wrote:
>    cpp_define (parse_in, "__SEG_FS");
>    cpp_define (parse_in, "__SEG_GS");
> +  cpp_define (parse_in, "__SEG_TLS");

Is there a good place to document these #defines?  I looked at your
glibc patch and wonder where __SEG_TLS was coming from.

Florian

[PATCH 2/9] Relax ADDR_SPACE_GENERIC_P checks for default address space hooks

[Richard Henderson]

If all address spaces use the same modes and forms, we would
be forced to replicate these hooks in the backend.  Which would
then require the creation of a new hook to replace
target_default_pointer_address_modes_p.


	* targhooks.c (default_addr_space_pointer_mode): Remove check
	for generic address space.
	(default_addr_space_address_mode): Likewise.
	(default_addr_space_valid_pointer_mode): Likewise.
	(default_addr_space_legitimate_address_p): Likewise.
	(default_addr_space_legitimize_address): Likewise.
---
 gcc/targhooks.c | 39 ++++++++++++++++-----------------------
 1 file changed, 16 insertions(+), 23 deletions(-)

diff --git a/gcc/targhooks.c b/gcc/targhooks.c
index 7238c8f..a8a243c 100644
--- a/gcc/targhooks.c
+++ b/gcc/targhooks.c
@@ -1181,35 +1181,31 @@ default_ref_may_alias_errno (ao_ref *ref)
   return false;
 }
 
-/* Return the mode for a pointer to a given ADDRSPACE, defaulting to ptr_mode
-   for the generic address space only.  */
+/* Return the mode for a pointer to a given ADDRSPACE,
+   defaulting to ptr_mode for all address spaces.  */
 
 machine_mode
 default_addr_space_pointer_mode (addr_space_t addrspace ATTRIBUTE_UNUSED)
 {
-  gcc_assert (ADDR_SPACE_GENERIC_P (addrspace));
   return ptr_mode;
 }
 
-/* Return the mode for an address in a given ADDRSPACE, defaulting to Pmode
-   for the generic address space only.  */
+/* Return the mode for an address in a given ADDRSPACE,
+   defaulting to Pmode for all address spaces.  */
 
 machine_mode
 default_addr_space_address_mode (addr_space_t addrspace ATTRIBUTE_UNUSED)
 {
-  gcc_assert (ADDR_SPACE_GENERIC_P (addrspace));
   return Pmode;
 }
 
-/* Named address space version of valid_pointer_mode.  */
+/* Named address space version of valid_pointer_mode.
+   To match the above, the same modes apply to all address spaces.  */
 
 bool
-default_addr_space_valid_pointer_mode (machine_mode mode, addr_space_t as)
+default_addr_space_valid_pointer_mode (machine_mode mode,
+				       addr_space_t as ATTRIBUTE_UNUSED)
 {
-  if (!ADDR_SPACE_GENERIC_P (as))
-    return (mode == targetm.addr_space.pointer_mode (as)
-	    || mode == targetm.addr_space.address_mode (as));
-
   return targetm.valid_pointer_mode (mode);
 }
 
@@ -1229,27 +1225,24 @@ target_default_pointer_address_modes_p (void)
   return true;
 }
 
-/* Named address space version of legitimate_address_p.  */
+/* Named address space version of legitimate_address_p.
+   By default, all address spaces have the same form.  */
 
 bool
 default_addr_space_legitimate_address_p (machine_mode mode, rtx mem,
-					 bool strict, addr_space_t as)
+					 bool strict,
+					 addr_space_t as ATTRIBUTE_UNUSED)
 {
-  if (!ADDR_SPACE_GENERIC_P (as))
-    gcc_unreachable ();
-
   return targetm.legitimate_address_p (mode, mem, strict);
 }
 
-/* Named address space version of LEGITIMIZE_ADDRESS.  */
+/* Named address space version of LEGITIMIZE_ADDRESS.
+   By default, all address spaces have the same form.  */
 
 rtx
-default_addr_space_legitimize_address (rtx x, rtx oldx,
-				       machine_mode mode, addr_space_t as)
+default_addr_space_legitimize_address (rtx x, rtx oldx, machine_mode mode,
+				       addr_space_t as ATTRIBUTE_UNUSED)
 {
-  if (!ADDR_SPACE_GENERIC_P (as))
-    return x;
-
   return targetm.legitimize_address (x, oldx, mode);
 }
 
-- 
2.4.3

Re: [PATCH 2/9] Relax ADDR_SPACE_GENERIC_P checks for default address space hooks

[Richard Biener]

On Thu, Oct 8, 2015 at 6:59 AM, Richard Henderson <rth@redhat.com> wrote:
> If all address spaces use the same modes and forms, we would
> be forced to replicate these hooks in the backend.  Which would
> then require the creation of a new hook to replace
> target_default_pointer_address_modes_p.

Looks good to me.

Richard.

>
>         * targhooks.c (default_addr_space_pointer_mode): Remove check
>         for generic address space.
>         (default_addr_space_address_mode): Likewise.
>         (default_addr_space_valid_pointer_mode): Likewise.
>         (default_addr_space_legitimate_address_p): Likewise.
>         (default_addr_space_legitimize_address): Likewise.
> ---
>  gcc/targhooks.c | 39 ++++++++++++++++-----------------------
>  1 file changed, 16 insertions(+), 23 deletions(-)
>
> diff --git a/gcc/targhooks.c b/gcc/targhooks.c
> index 7238c8f..a8a243c 100644
> --- a/gcc/targhooks.c
> +++ b/gcc/targhooks.c
> @@ -1181,35 +1181,31 @@ default_ref_may_alias_errno (ao_ref *ref)
>    return false;
>  }
>
> -/* Return the mode for a pointer to a given ADDRSPACE, defaulting to ptr_mode
> -   for the generic address space only.  */
> +/* Return the mode for a pointer to a given ADDRSPACE,
> +   defaulting to ptr_mode for all address spaces.  */
>
>  machine_mode
>  default_addr_space_pointer_mode (addr_space_t addrspace ATTRIBUTE_UNUSED)
>  {
> -  gcc_assert (ADDR_SPACE_GENERIC_P (addrspace));
>    return ptr_mode;
>  }
>
> -/* Return the mode for an address in a given ADDRSPACE, defaulting to Pmode
> -   for the generic address space only.  */
> +/* Return the mode for an address in a given ADDRSPACE,
> +   defaulting to Pmode for all address spaces.  */
>
>  machine_mode
>  default_addr_space_address_mode (addr_space_t addrspace ATTRIBUTE_UNUSED)
>  {
> -  gcc_assert (ADDR_SPACE_GENERIC_P (addrspace));
>    return Pmode;
>  }
>
> -/* Named address space version of valid_pointer_mode.  */
> +/* Named address space version of valid_pointer_mode.
> +   To match the above, the same modes apply to all address spaces.  */
>
>  bool
> -default_addr_space_valid_pointer_mode (machine_mode mode, addr_space_t as)
> +default_addr_space_valid_pointer_mode (machine_mode mode,
> +                                      addr_space_t as ATTRIBUTE_UNUSED)
>  {
> -  if (!ADDR_SPACE_GENERIC_P (as))
> -    return (mode == targetm.addr_space.pointer_mode (as)
> -           || mode == targetm.addr_space.address_mode (as));
> -
>    return targetm.valid_pointer_mode (mode);
>  }
>
> @@ -1229,27 +1225,24 @@ target_default_pointer_address_modes_p (void)
>    return true;
>  }
>
> -/* Named address space version of legitimate_address_p.  */
> +/* Named address space version of legitimate_address_p.
> +   By default, all address spaces have the same form.  */
>
>  bool
>  default_addr_space_legitimate_address_p (machine_mode mode, rtx mem,
> -                                        bool strict, addr_space_t as)
> +                                        bool strict,
> +                                        addr_space_t as ATTRIBUTE_UNUSED)
>  {
> -  if (!ADDR_SPACE_GENERIC_P (as))
> -    gcc_unreachable ();
> -
>    return targetm.legitimate_address_p (mode, mem, strict);
>  }
>
> -/* Named address space version of LEGITIMIZE_ADDRESS.  */
> +/* Named address space version of LEGITIMIZE_ADDRESS.
> +   By default, all address spaces have the same form.  */
>
>  rtx
> -default_addr_space_legitimize_address (rtx x, rtx oldx,
> -                                      machine_mode mode, addr_space_t as)
> +default_addr_space_legitimize_address (rtx x, rtx oldx, machine_mode mode,
> +                                      addr_space_t as ATTRIBUTE_UNUSED)
>  {
> -  if (!ADDR_SPACE_GENERIC_P (as))
> -    return x;
> -
>    return targetm.legitimize_address (x, oldx, mode);
>  }
>
> --
> 2.4.3
>

Re: [RFA 0/9] Address space support for x86

[Richard Biener]

On Thu, Oct 8, 2015 at 6:59 AM, Richard Henderson <rth@redhat.com> wrote:
> I started with Armin Rigo's patch, from back in July,
>
>   https://gcc.gnu.org/ml/gcc/2015-07/msg00125.html
>
> but then wound up changing all of it.  To wit:
>
> (1) If we use more sensible defaults for address spaces, we no longer
> require 8 backend functions and 1 new hook.  Indeed, basic support for
> x86 segmentation via address spaces then requires no overriding of any
> hooks.
>
> (2) In order to do anything useful with x86 segmentation, we need to
> acknowledge that the most common usage puts a valid object at %seg:0.
> Which means that we do need a new hook to disable several optimizations.

We could of course simply make all but the default address-space fall
under -fno-delete-null-pointer-check rules...

> (3) I found that usage within glibc really requires a more structured
> address space, one with a known mapping to the generic address space.
> Thus I introduce __seg_tls, which knows that the segment base is also
> stored at %seg:0.
>
> (4) I attempted to convert the entire tls implementation to use __seg_tls.
> I believe this is ultimately the best long-term solution, but I ran into
> a number of problems both generically and in the backend that prevented
> this from being simple.  I've dropped these patches for now.
>
> (5) However, in the process of attempting (4), I found two problems in
> the backend that could affect any use of address spaces: the movabs
> patterns and string expansions.
>
>
> r~
>
>
> Richard Henderson (9):
>   Change default of non-overlapping address space conversion
>   Relax ADDR_SPACE_GENERIC_P checks for default address space hooks
>   i386: Handle address spaces in movabs patterns
>   i386: Disallow address spaces with string insns
>   i386: Add address spaces for fs/gs segments
>   i386: Replace ix86_address_seg with addr_space_t
>   i386: Add address space for tls
>   Add TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID
>   Fix PR 66768
>
>  gcc/config/i386/i386-c.c                     |   8 +
>  gcc/config/i386/i386-protos.h                |   8 +-
>  gcc/config/i386/i386.c                       | 296 +++++++++++++++++++--------
>  gcc/config/i386/i386.h                       |   3 +-
>  gcc/config/i386/i386.md                      |  91 +++++---
>  gcc/config/i386/predicates.md                |   8 +-
>  gcc/config/i386/rdos.h                       |   2 +-
>  gcc/doc/tm.texi                              |   5 +
>  gcc/doc/tm.texi.in                           |   2 +
>  gcc/expr.c                                   |  30 +--
>  gcc/fold-const.c                             |   6 +-
>  gcc/gimple.c                                 |  12 +-
>  gcc/target.def                               |   9 +
>  gcc/targhooks.c                              |  48 ++---
>  gcc/targhooks.h                              |   1 +
>  gcc/testsuite/gcc.target/i386/addr-space-1.c |  11 +
>  gcc/tree-ssa-address.c                       |   2 +-
>  17 files changed, 385 insertions(+), 157 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/i386/addr-space-1.c
>
> --
> 2.4.3
>

Re: [RFA 0/9] Address space support for x86

[Richard Henderson]

On 10/08/2015 09:06 PM, Richard Biener wrote:
> On Thu, Oct 8, 2015 at 6:59 AM, Richard Henderson <rth@redhat.com> wrote:
>> I started with Armin Rigo's patch, from back in July,
>>
>>    https://gcc.gnu.org/ml/gcc/2015-07/msg00125.html
>>
>> but then wound up changing all of it.  To wit:
>>
>> (1) If we use more sensible defaults for address spaces, we no longer
>> require 8 backend functions and 1 new hook.  Indeed, basic support for
>> x86 segmentation via address spaces then requires no overriding of any
>> hooks.
>>
>> (2) In order to do anything useful with x86 segmentation, we need to
>> acknowledge that the most common usage puts a valid object at %seg:0.
>> Which means that we do need a new hook to disable several optimizations.
>
> We could of course simply make all but the default address-space fall
> under -fno-delete-null-pointer-check rules...

Yes, but that's probably less than optimal for targets whose non-default 
address space is simply "far", using a wider pointer mode.


r~