public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 3/3] rs6000: Remove TM regs
  2019-05-06 21:56 [PATCH 1/3] rs6000: rs6000_dbx_register_number for fp/ap/mq Segher Boessenkool
  2019-05-06 21:56 ` [PATCH 2/3] rs6000: Delete PRE_GCC3_DWARF_FRAME_REGISTERS Segher Boessenkool
@ 2019-05-06 21:56 ` Segher Boessenkool
  1 sibling, 0 replies; 6+ messages in thread
From: Segher Boessenkool @ 2019-05-06 21:56 UTC (permalink / raw)
  To: gcc-patches; +Cc: dje.gcc, Segher Boessenkool

We do not need to expose the TM registers in debug info.  It isn't
actually useful there, because none of the theing that can modify
these register (other than explicit moves) are marked.

We also do not need the registers for GCC itself internally.  This
patch deletes them.

Tested on powerpc64-linux {-m32,-m64} and on powerpc64le-linux;
installing on trunk.


Segher


2019-05-06  Segher Boessenkool  <segher@kernel.crashing.org>

	* config/rs6000/rs6000.md (TFHAR_REGNO, TFIAR_REGNO, TEXASR_REGNO):
	Delete.
	* config/rs6000/rs6000.h (FIRST_PSEUDO_REGISTER): Adjust.
	(DWARF_FRAME_REGISTERS): Adjust.
	(FIXED_REGISTERS, CALL_USED_REGISTERS, CALL_REALLY_USED_REGISTERS):
	Adjust.
	(REG_ALLOC_ORDER): Adjust.
	(enum reg_class): Delete SPR_REGS.
	(REG_CLASS_NAMES): Delete SPR_REGS.
	(REG_CLASS_CONTENTS): Delete SPR_REGS.  Adjust for deleted TM regs.
	(REGISTER_NAMES): Adjust.
	(ADDITIONAL_REGISTER_NAMES): Adjust.
	* config/rs6000/darwin.h (REGISTER_NAMES): Adjust.
	* config/rs6000/htm.md (htm_mfspr_<mode>, htm_mtspr_<mode>): Adjust.
	* config/rs6000/predicates.md (htm_spr_reg_operand): Delete.
	* config/rs6000/rs6000.c (rs6000_init_hard_regno_mode_ok): Adjust.
	(htm_spr_regno): Delete.
	(htm_expand_builtin): Adjust: the HTM builtins now have one fewer
	argument.
	(rs6000_dbx_register_number): Adjust.

---
 gcc/config/rs6000/darwin.h      |  3 +--
 gcc/config/rs6000/htm.md        | 10 ++++------
 gcc/config/rs6000/predicates.md | 27 ---------------------------
 gcc/config/rs6000/rs6000.c      | 35 +----------------------------------
 gcc/config/rs6000/rs6000.h      | 31 +++++++++----------------------
 gcc/config/rs6000/rs6000.md     |  3 ---
 6 files changed, 15 insertions(+), 94 deletions(-)

diff --git a/gcc/config/rs6000/darwin.h b/gcc/config/rs6000/darwin.h
index 9fb36e4..d86333d 100644
--- a/gcc/config/rs6000/darwin.h
+++ b/gcc/config/rs6000/darwin.h
@@ -231,8 +231,7 @@ extern int darwin_emit_branch_islands;
     "v16", "v17", "v18", "v19", "v20", "v21", "v22", "v23",             \
     "v24", "v25", "v26", "v27", "v28", "v29", "v30", "v31",             \
     "vrsave", "vscr",							\
-    "sfp",								\
-    "tfhar", "tfiar", "texasr"						\
+    "sfp"								\
 }
 
 /* This outputs NAME to FILE.  */
diff --git a/gcc/config/rs6000/htm.md b/gcc/config/rs6000/htm.md
index 66b583d..9e99afa 100644
--- a/gcc/config/rs6000/htm.md
+++ b/gcc/config/rs6000/htm.md
@@ -267,18 +267,16 @@ (define_insn "*ttest"
 
 (define_insn "htm_mfspr_<mode>"
   [(set (match_operand:GPR 0 "gpc_reg_operand" "=r")
-        (unspec_volatile:GPR [(match_operand 1 "u10bit_cint_operand" "n")
-			      (match_operand:GPR 2 "htm_spr_reg_operand" "")]
+        (unspec_volatile:GPR [(match_operand 1 "u10bit_cint_operand" "n")]
 			     UNSPECV_HTM_MFSPR))]
   "TARGET_HTM"
   "mfspr %0,%1";
   [(set_attr "type" "htm")])
 
 (define_insn "htm_mtspr_<mode>"
-  [(set (match_operand:GPR 2 "htm_spr_reg_operand" "")
-        (unspec_volatile:GPR [(match_operand:GPR 0 "gpc_reg_operand" "r")
-			      (match_operand 1 "u10bit_cint_operand" "n")]
-			     UNSPECV_HTM_MTSPR))]
+  [(unspec_volatile [(match_operand:GPR 0 "gpc_reg_operand" "r")
+		     (match_operand 1 "u10bit_cint_operand" "n")]
+		    UNSPECV_HTM_MTSPR)]
   "TARGET_HTM"
   "mtspr %1,%0";
   [(set_attr "type" "htm")])
diff --git a/gcc/config/rs6000/predicates.md b/gcc/config/rs6000/predicates.md
index 5cc80de..2643f1a 100644
--- a/gcc/config/rs6000/predicates.md
+++ b/gcc/config/rs6000/predicates.md
@@ -406,33 +406,6 @@ (define_predicate "fpr_reg_operand"
   return FP_REGNO_P (r);
 })
 
-;; Return 1 if op is a HTM specific SPR register.
-(define_predicate "htm_spr_reg_operand"
-  (match_operand 0 "register_operand")
-{
-  if (!TARGET_HTM)
-    return 0;
-
-  if (SUBREG_P (op))
-    op = SUBREG_REG (op);
-
-  if (!REG_P (op))
-    return 0;
-
-  switch (REGNO (op))
-    {
-      case TFHAR_REGNO:
-      case TFIAR_REGNO:
-      case TEXASR_REGNO:
-	return 1;
-      default:
-	break;
-    }
-  
-  /* Unknown SPR.  */
-  return 0;
-})
-
 ;; Return 1 if op is a general purpose register that is an even register
 ;; which suitable for a load/store quad operation
 ;; Subregs are not allowed here because when they are combine can
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 6f46fdd..af0ce16 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -3011,9 +3011,6 @@ rs6000_init_hard_regno_mode_ok (bool global_init_p)
   rs6000_regno_regclass[CA_REGNO] = NO_REGS;
   rs6000_regno_regclass[VRSAVE_REGNO] = VRSAVE_REGS;
   rs6000_regno_regclass[VSCR_REGNO] = VRSAVE_REGS;
-  rs6000_regno_regclass[TFHAR_REGNO] = SPR_REGS;
-  rs6000_regno_regclass[TFIAR_REGNO] = SPR_REGS;
-  rs6000_regno_regclass[TEXASR_REGNO] = SPR_REGS;
   rs6000_regno_regclass[ARG_POINTER_REGNUM] = BASE_REGS;
   rs6000_regno_regclass[FRAME_POINTER_REGNUM] = BASE_REGS;
 
@@ -14094,23 +14091,6 @@ htm_spr_num (enum rs6000_builtins code)
   return TEXASRU_SPR;
 }
 
-/* Return the appropriate SPR regno associated with the given builtin.  */
-static inline HOST_WIDE_INT
-htm_spr_regno (enum rs6000_builtins code)
-{
-  if (code == HTM_BUILTIN_GET_TFHAR
-      || code == HTM_BUILTIN_SET_TFHAR)
-    return TFHAR_REGNO;
-  else if (code == HTM_BUILTIN_GET_TFIAR
-	   || code == HTM_BUILTIN_SET_TFIAR)
-    return TFIAR_REGNO;
-  gcc_assert (code == HTM_BUILTIN_GET_TEXASR
-	      || code == HTM_BUILTIN_SET_TEXASR
-	      || code == HTM_BUILTIN_GET_TEXASRU
-	      || code == HTM_BUILTIN_SET_TEXASRU);
-  return TEXASR_REGNO;
-}
-
 /* Return the correct ICODE value depending on whether we are
    setting or reading the HTM SPRs.  */
 static inline enum insn_code
@@ -14227,7 +14207,6 @@ htm_expand_builtin (tree exp, rtx target, bool * expandedp)
 	  {
 	    machine_mode mode = (TARGET_POWERPC64) ? DImode : SImode;
 	    op[nopnds++] = gen_rtx_CONST_INT (mode, htm_spr_num (fcode));
-	    op[nopnds++] = gen_rtx_REG (mode, htm_spr_regno (fcode));
 	  }
 	/* If this builtin accesses a CR, then pass in a scratch
 	   CR as the last operand.  */
@@ -14248,7 +14227,7 @@ htm_expand_builtin (tree exp, rtx target, bool * expandedp)
 	    if (!(attr & RS6000_BTC_VOID))
 	      expected_nopnds += 1;
 	    if (uses_spr)
-	      expected_nopnds += 2;
+	      expected_nopnds += 1;
 
 	    gcc_assert (nopnds == expected_nopnds
 			&& nopnds <= MAX_HTM_OPERANDS);
@@ -36298,12 +36277,6 @@ rs6000_dbx_register_number (unsigned int regno, unsigned int format)
 	return 356;
       if (regno == VSCR_REGNO)
 	return 67;
-      if (regno == TFHAR_REGNO)
-	return 228;
-      if (regno == TFIAR_REGNO)
-	return 229;
-      if (regno == TEXASR_REGNO)
-	return 230;
 
       /* These do not make much sense.  */
       if (regno == FRAME_POINTER_REGNUM)
@@ -36338,12 +36311,6 @@ rs6000_dbx_register_number (unsigned int regno, unsigned int format)
     return 109;
   if (regno == VSCR_REGNO)
     return 110;
-  if (regno == TFHAR_REGNO)
-    return 114;
-  if (regno == TFIAR_REGNO)
-    return 115;
-  if (regno == TEXASR_REGNO)
-    return 116;
 
   if (regno == FRAME_POINTER_REGNUM)
     return 111;
diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h
index 3829e8f..3a32c11 100644
--- a/gcc/config/rs6000/rs6000.h
+++ b/gcc/config/rs6000/rs6000.h
@@ -815,11 +815,10 @@ enum data_align { align_abi, align_opt, align_both };
 
    The 3 HTM registers aren't also included in DWARF_FRAME_REGISTERS.  */
 
-#define FIRST_PSEUDO_REGISTER 115
+#define FIRST_PSEUDO_REGISTER 112
 
-/* The sfp register and 3 HTM registers
-   aren't included in DWARF_FRAME_REGISTERS.  */
-#define DWARF_FRAME_REGISTERS (FIRST_PSEUDO_REGISTER - 4)
+/* The sfp register isn't included in DWARF_FRAME_REGISTERS.  */
+#define DWARF_FRAME_REGISTERS (FIRST_PSEUDO_REGISTER - 1)
 
 /* Use standard DWARF numbering for DWARF debugging information.  */
 #define DBX_REGISTER_NUMBER(REGNO) rs6000_dbx_register_number ((REGNO), 0)
@@ -851,7 +850,7 @@ enum data_align { align_abi, align_opt, align_both };
    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
    1, 1						   \
-   , 1, 1, 1, 1					   \
+   , 1						   \
 }
 
 /* 1 for registers not available across function calls.
@@ -871,7 +870,7 @@ enum data_align { align_abi, align_opt, align_both };
    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
    1, 1						   \
-   , 1, 1, 1, 1					   \
+   , 1						   \
 }
 
 /* Like `CALL_USED_REGISTERS' except this macro doesn't require that
@@ -890,7 +889,7 @@ enum data_align { align_abi, align_opt, align_both };
    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
    0, 0						   \
-   , 0, 0, 0, 0					   \
+   , 0						   \
 }
 
 #define TOTAL_ALTIVEC_REGS	(LAST_ALTIVEC_REGNO - FIRST_ALTIVEC_REGNO + 1)
@@ -929,9 +928,6 @@ enum data_align { align_abi, align_opt, align_both };
 	v31 - v20	(saved; order given to save least number)
 	vrsave, vscr	(fixed)
 	sfp		(fixed)
-	tfhar		(fixed)
-	tfiar		(fixed)
-	texasr		(fixed)
 */
 
 #if FIXED_R2 == 1
@@ -973,7 +969,7 @@ enum data_align { align_abi, align_opt, align_both };
    96, 95, 94, 93, 92, 91,					\
    108, 107, 106, 105, 104, 103, 102, 101, 100, 99, 98, 97,	\
    109, 110,							\
-   111, 112, 113, 114						\
+   111								\
 }
 
 /* True if register is floating-point.  */
@@ -1134,7 +1130,6 @@ enum reg_class
   VSX_REGS,
   VRSAVE_REGS,
   VSCR_REGS,
-  SPR_REGS,
   GEN_OR_FLOAT_REGS,
   LINK_REGS,
   CTR_REGS,
@@ -1163,7 +1158,6 @@ enum reg_class
   "VSX_REGS",								\
   "VRSAVE_REGS",							\
   "VSCR_REGS",								\
-  "SPR_REGS",								\
   "GEN_OR_FLOAT_REGS",							\
   "LINK_REGS",								\
   "CTR_REGS",								\
@@ -1199,8 +1193,6 @@ enum reg_class
   { 0x00000000, 0x00000000, 0x00000000, 0x00002000 },			\
   /* VSCR_REGS.  */							\
   { 0x00000000, 0x00000000, 0x00000000, 0x00004000 },			\
-  /* SPR_REGS.  */							\
-  { 0x00000000, 0x00000000, 0x00000000, 0x00010000 },			\
   /* GEN_OR_FLOAT_REGS.  */						\
   { 0xffffffff, 0xffffffff, 0x00000008, 0x00008000 },			\
   /* LINK_REGS.  */							\
@@ -1222,7 +1214,7 @@ enum reg_class
   /* CA_REGS.  */							\
   { 0x00000000, 0x00000000, 0x00001000, 0x00000000 },			\
   /* ALL_REGS.  */							\
-  { 0xffffffff, 0xffffffff, 0xfffffffe, 0x0001ffff }			\
+  { 0xffffffff, 0xffffffff, 0xfffffffe, 0x0000ffff }			\
 }
 
 /* The same information, inverted:
@@ -2144,10 +2136,7 @@ extern char rs6000_reg_names[][8];	/* register names (0 vs. %r0).  */
   &rs6000_reg_names[108][0],	/* v31  */				\
   &rs6000_reg_names[109][0],	/* vrsave  */				\
   &rs6000_reg_names[110][0],	/* vscr  */				\
-  &rs6000_reg_names[111][0],	/* sfp  */				\
-  &rs6000_reg_names[112][0],	/* tfhar  */				\
-  &rs6000_reg_names[113][0],	/* tfiar  */				\
-  &rs6000_reg_names[114][0],	/* texasr  */				\
+  &rs6000_reg_names[111][0]	/* sfp  */				\
 }
 
 /* Table of additional register names to use in user input.  */
@@ -2201,8 +2190,6 @@ extern char rs6000_reg_names[][8];	/* register names (0 vs. %r0).  */
   {"vs52", 97}, {"vs53", 98}, {"vs54", 99}, {"vs55", 100},	\
   {"vs56", 101},{"vs57", 102},{"vs58", 103},{"vs59", 104},      \
   {"vs60", 105},{"vs61", 106},{"vs62", 107},{"vs63", 108},	\
-  /* Transactional Memory Facility (HTM) Registers.  */		\
-  {"tfhar",  112}, {"tfiar",  113}, {"texasr",  114},		\
 }
 
 /* This is how to output an element of a case-vector that is relative.  */
diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index ad80592..9c1a645 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -51,9 +51,6 @@ (define_constants
    (VRSAVE_REGNO		109)
    (VSCR_REGNO			110)
    (FRAME_POINTER_REGNUM	111)
-   (TFHAR_REGNO			112)
-   (TFIAR_REGNO			113)
-   (TEXASR_REGNO		114)
   ])
 
 ;;
-- 
1.8.3.1

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

* [PATCH 1/3] rs6000: rs6000_dbx_register_number for fp/ap/mq
@ 2019-05-06 21:56 Segher Boessenkool
  2019-05-06 21:56 ` [PATCH 2/3] rs6000: Delete PRE_GCC3_DWARF_FRAME_REGISTERS Segher Boessenkool
  2019-05-06 21:56 ` [PATCH 3/3] rs6000: Remove TM regs Segher Boessenkool
  0 siblings, 2 replies; 6+ messages in thread
From: Segher Boessenkool @ 2019-05-06 21:56 UTC (permalink / raw)
  To: gcc-patches; +Cc: dje.gcc, Segher Boessenkool

The frame pointer and the argument pointer aren't real registers.  MQ
was a register on old POWER.  All three are still used as arguments to
rs6000_dbx_register_number during initialisation.  If we handle them
explicitly we can do a gcc_unreachable to catch other unexpected
registers.

Tested on powerpc64-linux {-m32,-m64} and on powerpc64le-linux;
installing on trunk.


Segher


2019-05-06  Segher Boessenkool  <segher@kernel.crashing.org>

	* config/rs6000/rs6000.c (rs6000_dbx_register_number): Handle
	FRAME_POINTER_REGNUM, ARG_POINTER_REGNUM, and 64 (which was MQ).

--
 gcc/config/rs6000/rs6000.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index c75fd86..6f46fdd 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -36305,7 +36305,15 @@ rs6000_dbx_register_number (unsigned int regno, unsigned int format)
       if (regno == TEXASR_REGNO)
 	return 230;
 
-      return regno;
+      /* These do not make much sense.  */
+      if (regno == FRAME_POINTER_REGNUM)
+	return 111;
+      if (regno == ARG_POINTER_REGNUM)
+	return 67;
+      if (regno == 64)
+	return 100;
+
+      gcc_unreachable ();
 #endif
     }
 
@@ -36337,7 +36345,14 @@ rs6000_dbx_register_number (unsigned int regno, unsigned int format)
   if (regno == TEXASR_REGNO)
     return 116;
 
-  return regno;
+  if (regno == FRAME_POINTER_REGNUM)
+    return 111;
+  if (regno == ARG_POINTER_REGNUM)
+    return 67;
+  if (regno == 64)
+    return 64;
+
+  gcc_unreachable ();
 }
 
 /* target hook eh_return_filter_mode */
-- 
1.8.3.1

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

* [PATCH 2/3] rs6000: Delete PRE_GCC3_DWARF_FRAME_REGISTERS
  2019-05-06 21:56 [PATCH 1/3] rs6000: rs6000_dbx_register_number for fp/ap/mq Segher Boessenkool
@ 2019-05-06 21:56 ` Segher Boessenkool
  2023-01-13 18:05   ` Jakub Jelinek
  2019-05-06 21:56 ` [PATCH 3/3] rs6000: Remove TM regs Segher Boessenkool
  1 sibling, 1 reply; 6+ messages in thread
From: Segher Boessenkool @ 2019-05-06 21:56 UTC (permalink / raw)
  To: gcc-patches; +Cc: dje.gcc, Segher Boessenkool

We don't need this.


Segher


2019-05-06  Segher Boessenkool  <segher@kernel.crashing.org>

	* config/rs6000/rs6000.h (PRE_GCC3_DWARF_FRAME_REGISTERS): Delete.

---
 gcc/config/rs6000/rs6000.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h
index ff9449c..3829e8f 100644
--- a/gcc/config/rs6000/rs6000.h
+++ b/gcc/config/rs6000/rs6000.h
@@ -817,9 +817,6 @@ enum data_align { align_abi, align_opt, align_both };
 
 #define FIRST_PSEUDO_REGISTER 115
 
-/* This must be included for pre gcc 3.0 glibc compatibility.  */
-#define PRE_GCC3_DWARF_FRAME_REGISTERS 77
-
 /* The sfp register and 3 HTM registers
    aren't included in DWARF_FRAME_REGISTERS.  */
 #define DWARF_FRAME_REGISTERS (FIRST_PSEUDO_REGISTER - 4)
-- 
1.8.3.1

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

* Re: [PATCH 2/3] rs6000: Delete PRE_GCC3_DWARF_FRAME_REGISTERS
  2019-05-06 21:56 ` [PATCH 2/3] rs6000: Delete PRE_GCC3_DWARF_FRAME_REGISTERS Segher Boessenkool
@ 2023-01-13 18:05   ` Jakub Jelinek
  2023-01-14 13:38     ` Segher Boessenkool
  0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2023-01-13 18:05 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: gcc-patches, dje.gcc

On Mon, May 06, 2019 at 09:55:50PM +0000, Segher Boessenkool wrote:
> We don't need this.
> 
> 
> Segher
> 
> 
> 2019-05-06  Segher Boessenkool  <segher@kernel.crashing.org>
> 
> 	* config/rs6000/rs6000.h (PRE_GCC3_DWARF_FRAME_REGISTERS): Delete.

Why do you think so?

This seems to be a clear ABI break to me in the __frame_state_for
API.
So, if a __frame_state_for caller calls the function, it will overflow
the buffer passed by the caller.

> diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h
> index ff9449c..3829e8f 100644
> --- a/gcc/config/rs6000/rs6000.h
> +++ b/gcc/config/rs6000/rs6000.h
> @@ -817,9 +817,6 @@ enum data_align { align_abi, align_opt, align_both };
>  
>  #define FIRST_PSEUDO_REGISTER 115
>  
> -/* This must be included for pre gcc 3.0 glibc compatibility.  */
> -#define PRE_GCC3_DWARF_FRAME_REGISTERS 77
> -
>  /* The sfp register and 3 HTM registers
>     aren't included in DWARF_FRAME_REGISTERS.  */
>  #define DWARF_FRAME_REGISTERS (FIRST_PSEUDO_REGISTER - 4)
> -- 
> 1.8.3.1

	Jakub


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

* Re: [PATCH 2/3] rs6000: Delete PRE_GCC3_DWARF_FRAME_REGISTERS
  2023-01-13 18:05   ` Jakub Jelinek
@ 2023-01-14 13:38     ` Segher Boessenkool
  2023-01-14 14:10       ` Jakub Jelinek
  0 siblings, 1 reply; 6+ messages in thread
From: Segher Boessenkool @ 2023-01-14 13:38 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches, dje.gcc

Hi!

On Fri, Jan 13, 2023 at 07:05:34PM +0100, Jakub Jelinek wrote:
> On Mon, May 06, 2019 at 09:55:50PM +0000, Segher Boessenkool wrote:
> > We don't need this.
> > 
> > 
> > Segher
> > 
> > 
> > 2019-05-06  Segher Boessenkool  <segher@kernel.crashing.org>
> > 
> > 	* config/rs6000/rs6000.h (PRE_GCC3_DWARF_FRAME_REGISTERS): Delete.
> 
> Why do you think so?

First off, this was three years ago, and no problems have shown up.

> This seems to be a clear ABI break to me in the __frame_state_for
> API.
> So, if a __frame_state_for caller calls the function, it will overflow
> the buffer passed by the caller.

77 <= 111 so how can this overflow where it didn't before?

> > -/* This must be included for pre gcc 3.0 glibc compatibility.  */
> > -#define PRE_GCC3_DWARF_FRAME_REGISTERS 77

=== unwind-dw2.c
/* Dwarf frame registers used for pre gcc 3.0 compiled glibc.  */
#ifndef PRE_GCC3_DWARF_FRAME_REGISTERS
#define PRE_GCC3_DWARF_FRAME_REGISTERS __LIBGCC_DWARF_FRAME_REGISTERS__
#endif

=== c-family/c-cppbuiltin.cc
      builtin_define_with_int_value ("__LIBGCC_DWARF_FRAME_REGISTERS__",
                                     DWARF_FRAME_REGISTERS);


(Btw, we used many dwarf reg numbers > 1000 for a long time.)


Segher

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

* Re: [PATCH 2/3] rs6000: Delete PRE_GCC3_DWARF_FRAME_REGISTERS
  2023-01-14 13:38     ` Segher Boessenkool
@ 2023-01-14 14:10       ` Jakub Jelinek
  0 siblings, 0 replies; 6+ messages in thread
From: Jakub Jelinek @ 2023-01-14 14:10 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: gcc-patches, dje.gcc

On Sat, Jan 14, 2023 at 07:38:37AM -0600, Segher Boessenkool wrote:
> Hi!
> 
> On Fri, Jan 13, 2023 at 07:05:34PM +0100, Jakub Jelinek wrote:
> > On Mon, May 06, 2019 at 09:55:50PM +0000, Segher Boessenkool wrote:
> > > We don't need this.
> > > 
> > > 
> > > Segher
> > > 
> > > 
> > > 2019-05-06  Segher Boessenkool  <segher@kernel.crashing.org>
> > > 
> > > 	* config/rs6000/rs6000.h (PRE_GCC3_DWARF_FRAME_REGISTERS): Delete.
> > 
> > Why do you think so?
> 
> First off, this was three years ago, and no problems have shown up.

It is true that there aren't that many users of GCC 2.x compiled binaries
around.

> > This seems to be a clear ABI break to me in the __frame_state_for
> > API.
> > So, if a __frame_state_for caller calls the function, it will overflow
> > the buffer passed by the caller.
> 
> 77 <= 111 so how can this overflow where it didn't before?

typedef struct frame_state
{
  void *cfa;
  void *eh_ptr;
  long cfa_offset;
  long args_size;
  long reg_or_offset[PRE_GCC3_DWARF_FRAME_REGISTERS+1];
  unsigned short cfa_reg;
  unsigned short retaddr_column;
  char saved[PRE_GCC3_DWARF_FRAME_REGISTERS+1];
} frame_state;

struct frame_state * __frame_state_for (void *, struct frame_state *);

Unlike the new unwinder stuff, __frame_state_for can be called from other
objects, it is exported from both libgcc (on some architectures, including
ppc, ppc64 and ppc64le) and from glibc (on some architectures, including ppc
and i386, but not ppc64 nor ppc64le).
PRE_GCC3_DWARF_FRAME_REGISTERS defaults to __LIBGCC_DWARF_FRAME_REGISTERS__,
which is initialized to DWARF_FRAME_REGISTERS (if not defined, it is
FIRST_PSEUDO_REGISTER).
So, if PRE_GCC3_DWARF_FRAME_REGISTERS was previously defined to 77 and
now is not, the layout of the above structure changed, and if something
calls __frame_state_for with the layout of the structure with 77
registers, then it will not work properly if you get 111 instead.

In the GCC 2.x code, __frame_state_for was defined in gcc/frame.c
(libgcc.a(frame.o), while it was used in libgcc2.c if L_eh was defined
(so libgcc.a(_eh.o)) and the libgcc.a vs. libgcc_eh.a split wasn't present
yet, so parts of the unwinder could be exported from one shared library and
consumed by another one etc.  Depending on what is picked for
__frame_state_for, this can lead to ABI issues (in particular if the
libgcc_s.so.1 version is).
New code doesn't call __frame_state_for, it is there solely for
backwards compatibility.  So, even when GCC 10/11/12 had it broken,
there is no reason not to fix it in 13 and perhaps backport to all release
branches.

glibc sources have:
./sysdeps/sh/gccframe.h:#define DWARF_FRAME_REGISTERS 49
./sysdeps/i386/gccframe.h:#define DWARF_FRAME_REGISTERS 17
./sysdeps/powerpc/gccframe.h:#define DWARF_FRAME_REGISTERS 77
./sysdeps/hppa/gccframe.h:#define DWARF_FRAME_REGISTERS 89
On the GCC side compared to the above:
gcc/config/sh/sh.h:#define DWARF_FRAME_REGISTERS (153)
gcc/config/i386/i386.h:#define DWARF_FRAME_REGISTERS 17
gcc/config/rs6000/rs6000.h:#define FIRST_PSEUDO_REGISTER 111
gcc/config/pa/pa.h:#define DWARF_FRAME_REGISTERS (FIRST_PSEUDO_REGISTER - 1)
gcc/config/pa/pa32-regs.h:#define FIRST_PSEUDO_REGISTER 90  /* 32 general regs + 56 fp regs +
gcc/config/pa/pa64-regs.h:#define FIRST_PSEUDO_REGISTER 62  /* 32 general regs + 28 fp regs +

So, when PRE_GCC3_DWARF_FRAME_REGISTERS was defined on rs6000, it matched
glibc on all arches but sh (I have no idea about sh and don't really care).
I assume for 64-bit pa __frame_state_for isn't exported from glibc.

	Jakub


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

end of thread, other threads:[~2023-01-14 14:10 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-06 21:56 [PATCH 1/3] rs6000: rs6000_dbx_register_number for fp/ap/mq Segher Boessenkool
2019-05-06 21:56 ` [PATCH 2/3] rs6000: Delete PRE_GCC3_DWARF_FRAME_REGISTERS Segher Boessenkool
2023-01-13 18:05   ` Jakub Jelinek
2023-01-14 13:38     ` Segher Boessenkool
2023-01-14 14:10       ` Jakub Jelinek
2019-05-06 21:56 ` [PATCH 3/3] rs6000: Remove TM regs Segher Boessenkool

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