public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [RS6000] PR88614, output_operand: invalid %z value
@ 2019-01-06 22:59 Alan Modra
  2019-01-18 22:02 ` Segher Boessenkool
  0 siblings, 1 reply; 7+ messages in thread
From: Alan Modra @ 2019-01-06 22:59 UTC (permalink / raw)
  To: gcc-patches; +Cc: Segher Boessenkool

The direct cause of this PR is the fact that tls_gdld_nomark didn't
handle indirect calls.  Adding the missing support revealed that most
indirect calls were being optimised back to direct calls anyway, due
to tls_gdld_nomark not checking any of the parallel elements except
the first (plus the extra element that distinguishes this call from
normal calls).  Just checking the number of elements is enough to
separate the indirect calls from direct for ABI_ELFv2 and ABI_AIX,
while checking for the LONG_CALL bit in the cookie works for ABI_V4.
Direct calls being substituted for indirect calls is not the only
unwanted substitution.  See the tls_nomark_call comment.  I also saw a
_GLOBAL_OFFSET_TABLE_ symbol_ref being substituted for the GOT reg,
hence the unspec_tls change.

Bootstrap and regression testing on powerpc64le-linux and
powerpc64-linux in progress.  Note that the patch requires
https://gcc.gnu.org/ml/gcc-patches/2019-01/msg00252.html or the
earlier version for the attribute support.

	PR 88614
	* config/rs6000/predicates.md (unspec_tls): Ensure GOT reg
	stays a reg.
	(tls_nomark_call): New.
	* config/rs6000/rs6000.c (rs6000_call_sysv): Generate sysv4 secure
	plt call pattern here..
	* config/rs6000/rs6000.md (call_nonlocal_sysv): ..rather than here,
	delete split..
	(call_value_nonlocal_sysv): ..or here, delete split.
	(tls_gdld_nomark): Use tls_nomark_call predicate.  Set up operands
	for indirect calls and correct length attr.

diff --git a/gcc/config/rs6000/predicates.md b/gcc/config/rs6000/predicates.md
index 21791c51f2f..246452879a8 100644
--- a/gcc/config/rs6000/predicates.md
+++ b/gcc/config/rs6000/predicates.md
@@ -988,7 +988,12 @@ (define_predicate "rs6000_tls_symbol_ref"
 (define_predicate "unspec_tls"
   (match_code "unspec")
 {
-  return XINT (op, 1) == UNSPEC_TLSGD || XINT (op, 1) == UNSPEC_TLSLD;
+  if (XINT (op, 1) == UNSPEC_TLSGD)
+    return REG_P (XVECEXP (op, 0, 1));
+  else if (XINT (op, 1) == UNSPEC_TLSLD)
+    return REG_P (XVECEXP (op, 0, 0));
+  else
+    return 0;
 })
 
 ;; Return 1 if the operand, used inside a MEM, is a valid first argument
@@ -1018,6 +1023,86 @@ (define_predicate "indirect_call_operand"
   return false;
 })
 
+;; Verify that elements of the tls_gdld_nomark call insn parallel past the
+;; second element (added to distinguish this call from normal calls) match
+;; the normal contours of a call insn.  This is necessary to prevent
+;; substitutions we don't want, for example, an indirect call being
+;; optimised to a direct call, or (set (reg:r2) (unspec [] UNSPEC_TOCSLOT))
+;; being cleverly optimised to (set (reg:r2) (reg:r2)) because gcc
+;; "knows" that r2 hasn't changed from a previous call.
+(define_predicate "tls_nomark_call"
+  (match_code "parallel")
+{
+  int n = XVECLEN (op, 0);
+  rtvec v = XVEC (op, 0);
+  rtx set = RTVEC_ELT (v, 0);
+  if (GET_CODE (set) != SET)
+    return 0;
+  rtx call = XEXP (set, 1);
+  if (GET_CODE (call) != CALL)
+    return 0;
+  rtx mem = XEXP (call, 0);
+  if (GET_CODE (mem) != MEM)
+    return 0;
+  rtx addr = XEXP (mem, 0);
+  if (GET_CODE (addr) == SYMBOL_REF)
+    {
+      if (DEFAULT_ABI == ABI_ELFv2 || DEFAULT_ABI == ABI_AIX)
+	return (n == 3 && GET_CODE (RTVEC_ELT (v, 2)) == CLOBBER
+		&& REG_P (XEXP (RTVEC_ELT (v, 2), 0))
+		&& REGNO (XEXP (RTVEC_ELT (v, 2), 0)) == LR_REGNO);
+      else if (DEFAULT_ABI == ABI_V4)
+	return (n >= 4 && n <= 5 && GET_CODE (RTVEC_ELT (v, 2)) == USE
+		&& CONST_INT_P (XEXP (RTVEC_ELT (v, 2), 0))
+		&& (INTVAL (XEXP (RTVEC_ELT (v, 2), 0)) & CALL_LONG) == 0
+		&& (n == 4
+		    || (GET_CODE (RTVEC_ELT (v, 3)) == USE
+			&& REG_P (XEXP (RTVEC_ELT (v, 3), 0))))
+		&& GET_CODE (RTVEC_ELT (v, n - 1)) == CLOBBER
+		&& REG_P (XEXP (RTVEC_ELT (v, n - 1), 0))
+		&& REGNO (XEXP (RTVEC_ELT (v, n - 1), 0)) == LR_REGNO);
+      else
+	gcc_unreachable ();
+    }
+  else if (indirect_call_operand (addr, mode))
+    {
+      if (DEFAULT_ABI == ABI_ELFv2)
+	return (n == 4 && GET_CODE (RTVEC_ELT (v, 2)) == SET
+		&& REG_P (XEXP (RTVEC_ELT (v, 2), 0))
+		&& REGNO (XEXP (RTVEC_ELT (v, 2), 0)) == TOC_REGNUM
+		&& GET_CODE (XEXP (RTVEC_ELT (v, 2), 1)) == UNSPEC
+		&& XINT (XEXP (RTVEC_ELT (v, 2), 1), 1) == UNSPEC_TOCSLOT
+		&& XVECLEN (XEXP (RTVEC_ELT (v, 2), 1), 0) == 1
+		&& CONST_INT_P (XVECEXP (XEXP (RTVEC_ELT (v, 2), 1), 0, 0))
+		&& GET_CODE (RTVEC_ELT (v, 3)) == CLOBBER
+		&& REG_P (XEXP (RTVEC_ELT (v, 3), 0))
+		&& REGNO (XEXP (RTVEC_ELT (v, 3), 0)) == LR_REGNO);
+      else if (DEFAULT_ABI == ABI_AIX)
+	return (n == 5 && GET_CODE (RTVEC_ELT (v, 2)) == USE
+		&& GET_CODE (XEXP (RTVEC_ELT (v, 2), 0)) == MEM
+		&& GET_CODE (RTVEC_ELT (v, 3)) == SET
+		&& REG_P (XEXP (RTVEC_ELT (v, 3), 0))
+		&& REGNO (XEXP (RTVEC_ELT (v, 3), 0)) == TOC_REGNUM
+		&& GET_CODE (XEXP (RTVEC_ELT (v, 3), 1)) == UNSPEC
+		&& XINT (XEXP (RTVEC_ELT (v, 3), 1), 1) == UNSPEC_TOCSLOT
+		&& XVECLEN (XEXP (RTVEC_ELT (v, 3), 1), 0) == 1
+		&& CONST_INT_P (XVECEXP (XEXP (RTVEC_ELT (v, 3), 1), 0, 0))
+		&& GET_CODE (RTVEC_ELT (v, 4)) == CLOBBER
+		&& REG_P (XEXP (RTVEC_ELT (v, 4), 0))
+		&& REGNO (XEXP (RTVEC_ELT (v, 4), 0)) == LR_REGNO);
+      else if (DEFAULT_ABI == ABI_V4)
+	return (n == 4 && GET_CODE (RTVEC_ELT (v, 2)) == USE
+		&& CONST_INT_P (XEXP (RTVEC_ELT (v, 2), 0))
+		&& GET_CODE (RTVEC_ELT (v, 3)) == CLOBBER
+		&& REG_P (XEXP (RTVEC_ELT (v, 3), 0))
+		&& REGNO (XEXP (RTVEC_ELT (v, 3), 0)) == LR_REGNO);
+      else
+	gcc_unreachable ();
+    }
+  else
+    return 0;
+})
+
 ;; Return 1 if the operand is a SYMBOL_REF for a function known to be in
 ;; this file.
 (define_predicate "current_file_function_operand"
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index a25755418ea..4e3c5fc135f 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -37918,9 +37918,10 @@ rs6000_call_sysv (rtx value, rtx func_desc, rtx tlsarg, rtx cookie)
 {
   rtx func = func_desc;
   rtx func_addr;
-  rtx call[3];
+  rtx call[4];
   rtx insn;
   rtx abi_reg = NULL_RTX;
+  int n;
 
   if (global_tlsarg)
     tlsarg = global_tlsarg;
@@ -37968,9 +37969,16 @@ rs6000_call_sysv (rtx value, rtx func_desc, rtx tlsarg, rtx cookie)
     call[0] = gen_rtx_SET (value, call[0]);
 
   call[1] = gen_rtx_USE (VOIDmode, cookie);
-  call[2] = gen_hard_reg_clobber (Pmode, LR_REGNO);
+  n = 2;
+  if (TARGET_SECURE_PLT
+      && flag_pic
+      && GET_CODE (func_addr) == SYMBOL_REF
+      && !SYMBOL_REF_LOCAL_P (func_addr))
+    call[n++] = gen_rtx_USE (VOIDmode, pic_offset_table_rtx);
 
-  insn = gen_rtx_PARALLEL (VOIDmode, gen_rtvec_v (3, call));
+  call[n++] = gen_hard_reg_clobber (Pmode, LR_REGNO);
+
+  insn = gen_rtx_PARALLEL (VOIDmode, gen_rtvec_v (n, call));
   insn = emit_call_insn (insn);
   if (abi_reg)
     use_reg (&CALL_INSN_FUNCTION_USAGE (insn), abi_reg);
diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index 56364e0e43b..0d5ef31f9f2 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -9443,7 +9443,7 @@ (define_peephole2
 ;; TLS support.
 
 (define_insn "*tls_gdld_nomark<bits>"
-  [(match_parallel 3 ""
+  [(match_parallel 3 "tls_nomark_call"
     [(set (match_operand:P 0 "gpc_reg_operand" "=b")
 	  (call (mem:SI (match_operand:P 1))
 		(match_operand:P 2 "unspec_tls")))
@@ -9470,15 +9470,43 @@ (define_insn "*tls_gdld_nomark<bits>"
       else
 	output_asm_insn ("addi %0,%1,%&@got@tlsld", op);
     }
-  return rs6000_call_template (operands, 1);
+  if (GET_CODE (operands[1]) == SYMBOL_REF)
+    return rs6000_call_template (operands, 1);
+
+  /* Indirect calls need to recog a few more operands.  See the various
+     call_value_indirect patterns, and note that edit_tls_call_insn
+     added an extra element to the parallel.  */
+  rtx par = operands[3];
+  rtvec v = XVEC (par, 0);
+  if (DEFAULT_ABI == ABI_ELFv2)
+    operands[3] = XVECEXP (XEXP (RTVEC_ELT (v, 2), 1), 0, 0);
+  else if (DEFAULT_ABI == ABI_AIX)
+    {
+      operands[3] = XEXP (RTVEC_ELT (v, 2), 0);
+      operands[4] = XVECEXP (XEXP (RTVEC_ELT (v, 3), 1), 0, 0);
+    }
+  else if (DEFAULT_ABI == ABI_V4)
+    operands[3] = XEXP (RTVEC_ELT (v, 2), 0);
+  else
+    gcc_unreachable ();
+  return rs6000_indirect_call_template (operands, 1);
 }
   [(set_attr "type" "two")
    (set (attr "length")
-     (cond [(match_test "TARGET_CMODEL != CMODEL_SMALL")
-		(const_int 16)
-	    (match_test "DEFAULT_ABI != ABI_V4")
-		(const_int 12)]
-	(const_int 8)))])
+     (plus (if_then_else (match_test "TARGET_CMODEL != CMODEL_SMALL")
+			 (const_int 8)
+			 (const_int 4))
+	   (plus (if_then_else (match_test "GET_CODE (operands[1]) != SYMBOL_REF")
+			       (plus (if_then_else (match_test "!rs6000_speculate_indirect_jumps")
+						   (const_int 4)
+						   (const_int 0))
+				     (if_then_else (match_test "DEFAULT_ABI == ABI_AIX")
+						   (const_int 4)
+						   (const_int 0)))
+			       (const_int 0))
+		 (if_then_else (match_test "DEFAULT_ABI != ABI_V4")
+			       (const_int 8)
+			       (const_int 4)))))])
 
 (define_insn_and_split "*tls_gd<bits>"
   [(set (match_operand:P 0 "gpc_reg_operand" "=b")
@@ -10440,7 +10468,7 @@ (define_insn "*call_indirect_nonlocal_sysv<mode>"
 		  (const_string "8")]
 	      (const_string "4")))])
 
-(define_insn_and_split "*call_nonlocal_sysv<mode>"
+(define_insn "*call_nonlocal_sysv<mode>"
   [(call (mem:SI (match_operand:P 0 "symbol_ref_operand" "s,s"))
 	 (match_operand 1))
    (use (match_operand:SI 2 "immediate_operand" "O,n"))
@@ -10456,17 +10484,6 @@ (define_insn_and_split "*call_nonlocal_sysv<mode>"
     output_asm_insn ("creqv 6,6,6", operands);
 
   return rs6000_call_template (operands, 0);
-}
-  "DEFAULT_ABI == ABI_V4
-   && TARGET_SECURE_PLT && flag_pic && !SYMBOL_REF_LOCAL_P (operands[0])
-   && (INTVAL (operands[2]) & CALL_LONG) == 0"
-  [(parallel [(call (mem:SI (match_dup 0))
-		    (match_dup 1))
-	      (use (match_dup 2))
-	      (use (match_dup 3))
-	      (clobber (reg:SI LR_REGNO))])]
-{
-  operands[3] = pic_offset_table_rtx;
 }
   [(set_attr "type" "branch,branch")
    (set_attr "length" "4,8")])
@@ -10521,7 +10538,7 @@ (define_insn "*call_value_indirect_nonlocal_sysv<mode>"
 		  (const_string "8")]
 	      (const_string "4")))])
 
-(define_insn_and_split "*call_value_nonlocal_sysv<mode>"
+(define_insn "*call_value_nonlocal_sysv<mode>"
   [(set (match_operand 0 "" "")
 	(call (mem:SI (match_operand:P 1 "symbol_ref_operand" "s,s"))
 	      (match_operand 2)))
@@ -10538,18 +10555,6 @@ (define_insn_and_split "*call_value_nonlocal_sysv<mode>"
     output_asm_insn ("creqv 6,6,6", operands);
 
   return rs6000_call_template (operands, 1);
-}
-  "DEFAULT_ABI == ABI_V4
-   && TARGET_SECURE_PLT && flag_pic && !SYMBOL_REF_LOCAL_P (operands[1])
-   && (INTVAL (operands[3]) & CALL_LONG) == 0"
-  [(parallel [(set (match_dup 0)
-		   (call (mem:SI (match_dup 1))
-			 (match_dup 2)))
-	      (use (match_dup 3))
-	      (use (match_dup 4))
-	      (clobber (reg:SI LR_REGNO))])]
-{
-  operands[4] = pic_offset_table_rtx;
 }
   [(set_attr "type" "branch,branch")
    (set_attr "length" "4,8")])

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: [RS6000] PR88614, output_operand: invalid %z value
  2019-01-06 22:59 [RS6000] PR88614, output_operand: invalid %z value Alan Modra
@ 2019-01-18 22:02 ` Segher Boessenkool
  2019-01-20 13:38   ` Alan Modra
  0 siblings, 1 reply; 7+ messages in thread
From: Segher Boessenkool @ 2019-01-18 22:02 UTC (permalink / raw)
  To: Alan Modra; +Cc: gcc-patches

Hi Alan,

On Mon, Jan 07, 2019 at 09:29:18AM +1030, Alan Modra wrote:
> The direct cause of this PR is the fact that tls_gdld_nomark didn't
> handle indirect calls.  Adding the missing support revealed that most
> indirect calls were being optimised back to direct calls anyway, due
> to tls_gdld_nomark not checking any of the parallel elements except
> the first (plus the extra element that distinguishes this call from
> normal calls).  Just checking the number of elements is enough to
> separate the indirect calls from direct for ABI_ELFv2 and ABI_AIX,
> while checking for the LONG_CALL bit in the cookie works for ABI_V4.
> Direct calls being substituted for indirect calls is not the only
> unwanted substitution.  See the tls_nomark_call comment.  I also saw a
> _GLOBAL_OFFSET_TABLE_ symbol_ref being substituted for the GOT reg,
> hence the unspec_tls change.

> Bootstrap and regression testing on powerpc64le-linux and
> powerpc64-linux in progress.  Note that the patch requires
> https://gcc.gnu.org/ml/gcc-patches/2019-01/msg00252.html or the
> earlier version for the attribute support.

(Did you commit that yet?)

> +;; Verify that elements of the tls_gdld_nomark call insn parallel past the
> +;; second element (added to distinguish this call from normal calls) match
> +;; the normal contours of a call insn.  This is necessary to prevent
> +;; substitutions we don't want, for example, an indirect call being
> +;; optimised to a direct call, or (set (reg:r2) (unspec [] UNSPEC_TOCSLOT))
> +;; being cleverly optimised to (set (reg:r2) (reg:r2)) because gcc
> +;; "knows" that r2 hasn't changed from a previous call.
> +(define_predicate "tls_nomark_call"
> +  (match_code "parallel")
> +{
> +  int n = XVECLEN (op, 0);
> +  rtvec v = XVEC (op, 0);
> +  rtx set = RTVEC_ELT (v, 0);
> +  if (GET_CODE (set) != SET)
> +    return 0;
> +  rtx call = XEXP (set, 1);
> +  if (GET_CODE (call) != CALL)
> +    return 0;
> +  rtx mem = XEXP (call, 0);
> +  if (GET_CODE (mem) != MEM)
> +    return 0;
> +  rtx addr = XEXP (mem, 0);
> +  if (GET_CODE (addr) == SYMBOL_REF)
> +    {
> +      if (DEFAULT_ABI == ABI_ELFv2 || DEFAULT_ABI == ABI_AIX)
> +	return (n == 3 && GET_CODE (RTVEC_ELT (v, 2)) == CLOBBER
> +		&& REG_P (XEXP (RTVEC_ELT (v, 2), 0))
> +		&& REGNO (XEXP (RTVEC_ELT (v, 2), 0)) == LR_REGNO);
> +      else if (DEFAULT_ABI == ABI_V4)
> +	return (n >= 4 && n <= 5 && GET_CODE (RTVEC_ELT (v, 2)) == USE
> +		&& CONST_INT_P (XEXP (RTVEC_ELT (v, 2), 0))
> +		&& (INTVAL (XEXP (RTVEC_ELT (v, 2), 0)) & CALL_LONG) == 0
> +		&& (n == 4
> +		    || (GET_CODE (RTVEC_ELT (v, 3)) == USE
> +			&& REG_P (XEXP (RTVEC_ELT (v, 3), 0))))
> +		&& GET_CODE (RTVEC_ELT (v, n - 1)) == CLOBBER
> +		&& REG_P (XEXP (RTVEC_ELT (v, n - 1), 0))
> +		&& REGNO (XEXP (RTVEC_ELT (v, n - 1), 0)) == LR_REGNO);
> +      else
> +	gcc_unreachable ();
> +    }
> +  else if (indirect_call_operand (addr, mode))
> +    {
> +      if (DEFAULT_ABI == ABI_ELFv2)
> +	return (n == 4 && GET_CODE (RTVEC_ELT (v, 2)) == SET
> +		&& REG_P (XEXP (RTVEC_ELT (v, 2), 0))
> +		&& REGNO (XEXP (RTVEC_ELT (v, 2), 0)) == TOC_REGNUM
> +		&& GET_CODE (XEXP (RTVEC_ELT (v, 2), 1)) == UNSPEC
> +		&& XINT (XEXP (RTVEC_ELT (v, 2), 1), 1) == UNSPEC_TOCSLOT
> +		&& XVECLEN (XEXP (RTVEC_ELT (v, 2), 1), 0) == 1
> +		&& CONST_INT_P (XVECEXP (XEXP (RTVEC_ELT (v, 2), 1), 0, 0))
> +		&& GET_CODE (RTVEC_ELT (v, 3)) == CLOBBER
> +		&& REG_P (XEXP (RTVEC_ELT (v, 3), 0))
> +		&& REGNO (XEXP (RTVEC_ELT (v, 3), 0)) == LR_REGNO);
> +      else if (DEFAULT_ABI == ABI_AIX)
> +	return (n == 5 && GET_CODE (RTVEC_ELT (v, 2)) == USE
> +		&& GET_CODE (XEXP (RTVEC_ELT (v, 2), 0)) == MEM
> +		&& GET_CODE (RTVEC_ELT (v, 3)) == SET
> +		&& REG_P (XEXP (RTVEC_ELT (v, 3), 0))
> +		&& REGNO (XEXP (RTVEC_ELT (v, 3), 0)) == TOC_REGNUM
> +		&& GET_CODE (XEXP (RTVEC_ELT (v, 3), 1)) == UNSPEC
> +		&& XINT (XEXP (RTVEC_ELT (v, 3), 1), 1) == UNSPEC_TOCSLOT
> +		&& XVECLEN (XEXP (RTVEC_ELT (v, 3), 1), 0) == 1
> +		&& CONST_INT_P (XVECEXP (XEXP (RTVEC_ELT (v, 3), 1), 0, 0))
> +		&& GET_CODE (RTVEC_ELT (v, 4)) == CLOBBER
> +		&& REG_P (XEXP (RTVEC_ELT (v, 4), 0))
> +		&& REGNO (XEXP (RTVEC_ELT (v, 4), 0)) == LR_REGNO);
> +      else if (DEFAULT_ABI == ABI_V4)
> +	return (n == 4 && GET_CODE (RTVEC_ELT (v, 2)) == USE
> +		&& CONST_INT_P (XEXP (RTVEC_ELT (v, 2), 0))
> +		&& GET_CODE (RTVEC_ELT (v, 3)) == CLOBBER
> +		&& REG_P (XEXP (RTVEC_ELT (v, 3), 0))
> +		&& REGNO (XEXP (RTVEC_ELT (v, 3), 0)) == LR_REGNO);
> +      else
> +	gcc_unreachable ();
> +    }
> +  else
> +    return 0;
> +})

I find things like this almost impossible to read (and to verify for
correctness).  Maybe you can factor it a bit more?  Does it need to be a
predicate at all, or can you handle it by having various patterns?

> +     (plus (if_then_else (match_test "TARGET_CMODEL != CMODEL_SMALL")
> +			 (const_int 8)
> +			 (const_int 4))
> +	   (plus (if_then_else (match_test "GET_CODE (operands[1]) != SYMBOL_REF")
> +			       (plus (if_then_else (match_test "!rs6000_speculate_indirect_jumps")
> +						   (const_int 4)
> +						   (const_int 0))
> +				     (if_then_else (match_test "DEFAULT_ABI == ABI_AIX")
> +						   (const_int 4)
> +						   (const_int 0)))
> +			       (const_int 0))
> +		 (if_then_else (match_test "DEFAULT_ABI != ABI_V4")
> +			       (const_int 8)
> +			       (const_int 4)))))])

This part is way too wide.  Maybe it is would be readable if not :-(


Segher

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

* Re: [RS6000] PR88614, output_operand: invalid %z value
  2019-01-18 22:02 ` Segher Boessenkool
@ 2019-01-20 13:38   ` Alan Modra
  2019-01-21 12:19     ` Alan Modra
  0 siblings, 1 reply; 7+ messages in thread
From: Alan Modra @ 2019-01-20 13:38 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: gcc-patches

On Fri, Jan 18, 2019 at 04:02:14PM -0600, Segher Boessenkool wrote:
> Hi Alan,
> 
> On Mon, Jan 07, 2019 at 09:29:18AM +1030, Alan Modra wrote:
> > The direct cause of this PR is the fact that tls_gdld_nomark didn't
> > handle indirect calls.  Adding the missing support revealed that most
> > indirect calls were being optimised back to direct calls anyway, due
> > to tls_gdld_nomark not checking any of the parallel elements except
> > the first (plus the extra element that distinguishes this call from
> > normal calls).  Just checking the number of elements is enough to
> > separate the indirect calls from direct for ABI_ELFv2 and ABI_AIX,
> > while checking for the LONG_CALL bit in the cookie works for ABI_V4.
> > Direct calls being substituted for indirect calls is not the only
> > unwanted substitution.  See the tls_nomark_call comment.  I also saw a
> > _GLOBAL_OFFSET_TABLE_ symbol_ref being substituted for the GOT reg,
> > hence the unspec_tls change.
> 
> > Bootstrap and regression testing on powerpc64le-linux and
> > powerpc64-linux in progress.  Note that the patch requires
> > https://gcc.gnu.org/ml/gcc-patches/2019-01/msg00252.html or the
> > earlier version for the attribute support.
> 
> (Did you commit that yet?)

Yes, the prerequisite patches have been committed.

> > +;; Verify that elements of the tls_gdld_nomark call insn parallel past the
> > +;; second element (added to distinguish this call from normal calls) match
> > +;; the normal contours of a call insn.  This is necessary to prevent
> > +;; substitutions we don't want, for example, an indirect call being
> > +;; optimised to a direct call, or (set (reg:r2) (unspec [] UNSPEC_TOCSLOT))
> > +;; being cleverly optimised to (set (reg:r2) (reg:r2)) because gcc
> > +;; "knows" that r2 hasn't changed from a previous call.
> > +(define_predicate "tls_nomark_call"
> > +  (match_code "parallel")
> > +{
> > +  int n = XVECLEN (op, 0);
> > +  rtvec v = XVEC (op, 0);
> > +  rtx set = RTVEC_ELT (v, 0);
> > +  if (GET_CODE (set) != SET)
> > +    return 0;
> > +  rtx call = XEXP (set, 1);
> > +  if (GET_CODE (call) != CALL)
> > +    return 0;
> > +  rtx mem = XEXP (call, 0);
> > +  if (GET_CODE (mem) != MEM)
> > +    return 0;
> > +  rtx addr = XEXP (mem, 0);
> > +  if (GET_CODE (addr) == SYMBOL_REF)
> > +    {
> > +      if (DEFAULT_ABI == ABI_ELFv2 || DEFAULT_ABI == ABI_AIX)
> > +	return (n == 3 && GET_CODE (RTVEC_ELT (v, 2)) == CLOBBER
> > +		&& REG_P (XEXP (RTVEC_ELT (v, 2), 0))
> > +		&& REGNO (XEXP (RTVEC_ELT (v, 2), 0)) == LR_REGNO);
> > +      else if (DEFAULT_ABI == ABI_V4)
> > +	return (n >= 4 && n <= 5 && GET_CODE (RTVEC_ELT (v, 2)) == USE
> > +		&& CONST_INT_P (XEXP (RTVEC_ELT (v, 2), 0))
> > +		&& (INTVAL (XEXP (RTVEC_ELT (v, 2), 0)) & CALL_LONG) == 0
> > +		&& (n == 4
> > +		    || (GET_CODE (RTVEC_ELT (v, 3)) == USE
> > +			&& REG_P (XEXP (RTVEC_ELT (v, 3), 0))))
> > +		&& GET_CODE (RTVEC_ELT (v, n - 1)) == CLOBBER
> > +		&& REG_P (XEXP (RTVEC_ELT (v, n - 1), 0))
> > +		&& REGNO (XEXP (RTVEC_ELT (v, n - 1), 0)) == LR_REGNO);
> > +      else
> > +	gcc_unreachable ();
> > +    }
> > +  else if (indirect_call_operand (addr, mode))
> > +    {
> > +      if (DEFAULT_ABI == ABI_ELFv2)
> > +	return (n == 4 && GET_CODE (RTVEC_ELT (v, 2)) == SET
> > +		&& REG_P (XEXP (RTVEC_ELT (v, 2), 0))
> > +		&& REGNO (XEXP (RTVEC_ELT (v, 2), 0)) == TOC_REGNUM
> > +		&& GET_CODE (XEXP (RTVEC_ELT (v, 2), 1)) == UNSPEC
> > +		&& XINT (XEXP (RTVEC_ELT (v, 2), 1), 1) == UNSPEC_TOCSLOT
> > +		&& XVECLEN (XEXP (RTVEC_ELT (v, 2), 1), 0) == 1
> > +		&& CONST_INT_P (XVECEXP (XEXP (RTVEC_ELT (v, 2), 1), 0, 0))
> > +		&& GET_CODE (RTVEC_ELT (v, 3)) == CLOBBER
> > +		&& REG_P (XEXP (RTVEC_ELT (v, 3), 0))
> > +		&& REGNO (XEXP (RTVEC_ELT (v, 3), 0)) == LR_REGNO);
> > +      else if (DEFAULT_ABI == ABI_AIX)
> > +	return (n == 5 && GET_CODE (RTVEC_ELT (v, 2)) == USE
> > +		&& GET_CODE (XEXP (RTVEC_ELT (v, 2), 0)) == MEM
> > +		&& GET_CODE (RTVEC_ELT (v, 3)) == SET
> > +		&& REG_P (XEXP (RTVEC_ELT (v, 3), 0))
> > +		&& REGNO (XEXP (RTVEC_ELT (v, 3), 0)) == TOC_REGNUM
> > +		&& GET_CODE (XEXP (RTVEC_ELT (v, 3), 1)) == UNSPEC
> > +		&& XINT (XEXP (RTVEC_ELT (v, 3), 1), 1) == UNSPEC_TOCSLOT
> > +		&& XVECLEN (XEXP (RTVEC_ELT (v, 3), 1), 0) == 1
> > +		&& CONST_INT_P (XVECEXP (XEXP (RTVEC_ELT (v, 3), 1), 0, 0))
> > +		&& GET_CODE (RTVEC_ELT (v, 4)) == CLOBBER
> > +		&& REG_P (XEXP (RTVEC_ELT (v, 4), 0))
> > +		&& REGNO (XEXP (RTVEC_ELT (v, 4), 0)) == LR_REGNO);
> > +      else if (DEFAULT_ABI == ABI_V4)
> > +	return (n == 4 && GET_CODE (RTVEC_ELT (v, 2)) == USE
> > +		&& CONST_INT_P (XEXP (RTVEC_ELT (v, 2), 0))
> > +		&& GET_CODE (RTVEC_ELT (v, 3)) == CLOBBER
> > +		&& REG_P (XEXP (RTVEC_ELT (v, 3), 0))
> > +		&& REGNO (XEXP (RTVEC_ELT (v, 3), 0)) == LR_REGNO);
> > +      else
> > +	gcc_unreachable ();
> > +    }
> > +  else
> > +    return 0;
> > +})
> 
> I find things like this almost impossible to read (and to verify for
> correctness).  Maybe you can factor it a bit more?  Does it need to be a
> predicate at all, or can you handle it by having various patterns?

Hmm, if I invent a couple of new unspecs, UNSPEC_TLSGD_NOMARK and
UNSPEC_TLSLD_NOMARK, using them in place of UNSPEC_TLSGD and
UNSPEC_TLSLD when !TARGET_TLS_MARKERS then that should be enough to
tell when we have a -mno-tls-markers __tls_get_addr call.  So I guess
I could kill off edit_tls_call_insn and tls_gdld_nomark.  The
call_value_local and call_value_indirect insns would then need to
detect the special call and emit the arg setup insns.

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: [RS6000] PR88614, output_operand: invalid %z value
  2019-01-20 13:38   ` Alan Modra
@ 2019-01-21 12:19     ` Alan Modra
  2019-01-21 14:23       ` Segher Boessenkool
  0 siblings, 1 reply; 7+ messages in thread
From: Alan Modra @ 2019-01-21 12:19 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: gcc-patches

On Mon, Jan 21, 2019 at 12:08:33AM +1030, Alan Modra wrote:
> Hmm, if I invent a couple of new unspecs, UNSPEC_TLSGD_NOMARK and
> UNSPEC_TLSLD_NOMARK, using them in place of UNSPEC_TLSGD and
> UNSPEC_TLSLD when !TARGET_TLS_MARKERS then that should be enough to
> tell when we have a -mno-tls-markers __tls_get_addr call.  So I guess
> I could kill off edit_tls_call_insn and tls_gdld_nomark.  The
> call_value_local and call_value_indirect insns would then need to
> detect the special call and emit the arg setup insns.

Here's what the revised approach looks like, but without using new
unspecs.  Bootstrap and regression test on powerpc64le-linux and
powerpc64-linux biarch completed, and testing on powerpc64le-linux
with -mno-tls-markers.  powerpc64-linux -mno-tls-markers testing still
in progress.  OK?

----
The direct cause of this PR is the fact that tls_gdld_nomark didn't
handle indirect calls.  Also, most indirect calls were being optimised
back to direct calls anyway, due to tls_gdld_nomark not checking any
of the parallel elements except the first (plus the extra element that
distinguishes this call from normal calls).  There were other unwanted
substitutions too.

So this patch attacks the problem of handling special calls in a
different way.  Rather than adding another element to the call insn
parallel to distinguish -mno-tls-markers __tls_get_addr calls from any
other calls, we now inspect the second CALL arg.  Each
call_value_nonlocal and call_value_indirect insn now checks for the
tlsgd/ld unspecs when !TARGET_TLS_MARKERS and emits the arg setup
insns.  I disallow the local call patterns since we'll only see local
calls to __tls_get_addr in testcases, and it doesn't seem a good idea
to complicate the patterns just for a minor optimisation.  Sibling
call insns aren't used for libcalls, so none of these insns need to
change.

The patch also fixes a minor problem with -mno-tls-markers
__tls_get_addr calls causing a "li 3,0" instruction to be emitted
prior to the arg setup instructions, due to using a libcall with one
arg.  That isn't correct when the call insn itself sets up its arg.
Also, I've tidied the V4 secure-plt calls, generating them in
rs6000_call_sysv rather than by splitting in rs6000.md.  The
CALL_INSN_FUNCTION_USAGE added in edit_tls_call_insn is no longer
needed (since git commit 0a4b5c66df9).

On the subject of unwanted substitutions, I also saw a
_GLOBAL_OFFSET_TABLE_ symbol_ref being substituted for the GOT reg,
resulting in code like "addi 3,_GLOBAL_OFFSET_TABLE_,tls_ld@got@tlsld".
Fixed by the unspec_tls change.

	PR 88614
	* config/rs6000/predicates.md (unspec_tls): Ensure GOT reg
	stays a reg.  Allow a const_int.
	* config/rs6000/rs6000-protos.h (rs6000_output_tlsargs): Declare.
	* config/rs6000/rs6000.h (IS_V4_FP_ARGS): Define.
	(IS_NOMARK_TLSGETADDR): Define.
	* config/rs6000/rs6000.c (edit_tls_call_insn): Delete.
	(rs6000_output_tlsargs): New function.
	(rs6000_legitimize_tls_address): Don't say a !TARGET_TLS_MARKERS
	__tls_get_addr call takes an arg.
	(rs6000_call_sysv): Generate sysv4 secure plt call pattern here..
	* config/rs6000/rs6000.md (call_nonlocal_sysv): ..rather than here,
	delete split..
	(call_value_nonlocal_sysv): ..or here, delete split.
	(tls_gdld_nomark): Delete.
	(call_value_indirect_nonlocal_sysv): Use unspec_tls as operand2
	predicate.  Call rs6000_output_tlsargs.  Adjust length to suit.
	(call_value_nonlocal_sysv): Likewise.
	(call_value_nonlocal_sysv_secure): Likewise.
	(call_value_nonlocal_aix): Likewise.
	(call_value_indirect_aix): Likewise.
	(call_value_indirect_elfv2): Likewise.
	(call_value_local32, call_value_local64): Disable for no-mark tls.
	(call_value_local_aix): Likewise.

diff --git a/gcc/config/rs6000/predicates.md b/gcc/config/rs6000/predicates.md
index 21791c51f2f..540f45887e6 100644
--- a/gcc/config/rs6000/predicates.md
+++ b/gcc/config/rs6000/predicates.md
@@ -984,11 +984,18 @@ (define_predicate "rs6000_tls_symbol_ref"
   (and (match_code "symbol_ref")
        (match_test "RS6000_SYMBOL_REF_TLS_P (op)")))
 
-;; Return 1 for the UNSPEC used in TLS call operands
+;; Return 1 for the CONST_INT or UNSPEC second CALL operand.
+;; Prevents unwanted substitution of the unspec got_reg arg.
 (define_predicate "unspec_tls"
-  (match_code "unspec")
+  (match_code "const_int,unspec")
 {
-  return XINT (op, 1) == UNSPEC_TLSGD || XINT (op, 1) == UNSPEC_TLSLD;
+  if (CONST_INT_P (op))
+    return 1;
+  if (XINT (op, 1) == UNSPEC_TLSGD)
+    return REG_P (XVECEXP (op, 0, 1));
+  if (XINT (op, 1) == UNSPEC_TLSLD)
+    return REG_P (XVECEXP (op, 0, 0));
+  return 0;
 })
 
 ;; Return 1 if the operand, used inside a MEM, is a valid first argument
diff --git a/gcc/config/rs6000/rs6000-protos.h b/gcc/config/rs6000/rs6000-protos.h
index ea5c94b3ec7..9af619808ac 100644
--- a/gcc/config/rs6000/rs6000-protos.h
+++ b/gcc/config/rs6000/rs6000-protos.h
@@ -141,6 +141,7 @@ extern rtx (*rs6000_legitimize_reload_address_ptr) (rtx, machine_mode,
 						    int, int, int, int *);
 extern bool rs6000_legitimate_offset_address_p (machine_mode, rtx,
 						bool, bool);
+extern void rs6000_output_tlsargs (rtx *);
 extern rtx rs6000_find_base_term (rtx);
 extern rtx rs6000_return_addr (int, rtx);
 extern void rs6000_output_symbol_ref (FILE*, rtx);
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 3330b680d61..d7b7977a3b1 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -8626,35 +8626,35 @@ rs6000_legitimize_tls_address_aix (rtx addr, enum tls_model model)
   return dest;
 }
 
-/* Mess with a call, to make it look like the tls_gdld insns when
-   !TARGET_TLS_MARKERS.  These insns have an extra unspec to
-   differentiate them from standard calls, because they need to emit
-   the arg setup insns as well as the actual call.  That keeps the
-   arg setup insns immediately adjacent to the branch and link.  */
+/* Output arg setup instructions for a !TARGET_TLS_MARKERS
+   __tls_get_addr call.  */
 
-static void
-edit_tls_call_insn (rtx arg)
-{
-  rtx call_insn = last_call_insn ();
-  if (!TARGET_TLS_MARKERS)
-    {
-      rtx patt = PATTERN (call_insn);
-      gcc_assert (GET_CODE (patt) == PARALLEL);
-      rtvec orig = XVEC (patt, 0);
-      rtvec v = rtvec_alloc (GET_NUM_ELEM (orig) + 1);
-      gcc_assert (GET_NUM_ELEM (orig) > 0);
-      /* The (set (..) (call (mem ..))).  */
-      RTVEC_ELT (v, 0) = RTVEC_ELT (orig, 0);
-      /* The extra unspec.  */
-      RTVEC_ELT (v, 1) = arg;
-      /* All other assorted call pattern pieces.  */
-      for (int i = 1; i < GET_NUM_ELEM (orig); i++)
-	RTVEC_ELT (v, i + 1) = RTVEC_ELT (orig, i);
-      XVEC (patt, 0) = v;
-    }
-  if (DEFAULT_ABI == ABI_V4 && TARGET_SECURE_PLT && flag_pic)
-    use_reg (&CALL_INSN_FUNCTION_USAGE (call_insn),
-	     pic_offset_table_rtx);
+void
+rs6000_output_tlsargs (rtx *operands)
+{
+  rtx op[3];
+
+  op[0] = operands[0];
+  op[1] = XVECEXP (operands[2], 0, 0);
+  if (XINT (operands[2], 1) == UNSPEC_TLSGD)
+    {
+      op[2] = XVECEXP (operands[2], 0, 1);
+      if (TARGET_CMODEL != CMODEL_SMALL)
+	output_asm_insn ("addis %0,%2,%1@got@tlsgd@ha\n\t"
+			 "addi %0,%0,%1@got@tlsgd@l", op);
+      else
+	output_asm_insn ("addi %0,%2,%1@got@tlsgd", op);
+    }
+  else if (XINT (operands[2], 1) == UNSPEC_TLSLD)
+    {
+      if (TARGET_CMODEL != CMODEL_SMALL)
+	output_asm_insn ("addis %0,%1,%&@got@tlsld@ha\n\t"
+			 "addi %0,%0,%&@got@tlsld@l", op);
+      else
+	output_asm_insn ("addi %0,%1,%&@got@tlsld", op);
+    }
+  else
+    gcc_unreachable ();
 }
 
 /* Passes the tls arg value for global dynamic and local dynamic
@@ -8757,41 +8757,36 @@ rs6000_legitimize_tls_address (rtx addr, enum tls_model model)
 	{
 	  rtx arg = gen_rtx_UNSPEC (Pmode, gen_rtvec (2, addr, got),
 				    UNSPEC_TLSGD);
+	  tga = rs6000_tls_get_addr ();
 	  global_tlsarg = arg;
-	  rtx argreg = const0_rtx;
 	  if (TARGET_TLS_MARKERS)
 	    {
-	      argreg = gen_rtx_REG (Pmode, 3);
+	      rtx argreg = gen_rtx_REG (Pmode, 3);
 	      emit_insn (gen_rtx_SET (argreg, arg));
+	      emit_library_call_value (tga, dest, LCT_CONST, Pmode,
+				       argreg, Pmode);
 	    }
-
-	  tga = rs6000_tls_get_addr ();
-	  emit_library_call_value (tga, dest, LCT_CONST, Pmode,
-				   argreg, Pmode);
+	  else
+	    emit_library_call_value (tga, dest, LCT_CONST, Pmode);
 	  global_tlsarg = NULL_RTX;
-
-	  edit_tls_call_insn (arg);
 	}
       else if (model == TLS_MODEL_LOCAL_DYNAMIC)
 	{
-	  rtx arg = gen_rtx_UNSPEC (Pmode, gen_rtvec (1, got),
-				    UNSPEC_TLSLD);
+	  rtx arg = gen_rtx_UNSPEC (Pmode, gen_rtvec (1, got), UNSPEC_TLSLD);
+	  tga = rs6000_tls_get_addr ();
+	  tmp1 = gen_reg_rtx (Pmode);
 	  global_tlsarg = arg;
-	  rtx argreg = const0_rtx;
 	  if (TARGET_TLS_MARKERS)
 	    {
-	      argreg = gen_rtx_REG (Pmode, 3);
+	      rtx argreg = gen_rtx_REG (Pmode, 3);
 	      emit_insn (gen_rtx_SET (argreg, arg));
+	      emit_library_call_value (tga, tmp1, LCT_CONST, Pmode,
+				       argreg, Pmode);
 	    }
-
-	  tga = rs6000_tls_get_addr ();
-	  tmp1 = gen_reg_rtx (Pmode);
-	  emit_library_call_value (tga, tmp1, LCT_CONST, Pmode,
-				   argreg, Pmode);
+	  else
+	    emit_library_call_value (tga, tmp1, LCT_CONST, Pmode);
 	  global_tlsarg = NULL_RTX;
 
-	  edit_tls_call_insn (arg);
-
 	  if (rs6000_tls_size == 16)
 	    {
 	      if (TARGET_64BIT)
@@ -37920,9 +37915,10 @@ rs6000_call_sysv (rtx value, rtx func_desc, rtx tlsarg, rtx cookie)
 {
   rtx func = func_desc;
   rtx func_addr;
-  rtx call[3];
+  rtx call[4];
   rtx insn;
   rtx abi_reg = NULL_RTX;
+  int n;
 
   if (global_tlsarg)
     tlsarg = global_tlsarg;
@@ -37970,9 +37966,16 @@ rs6000_call_sysv (rtx value, rtx func_desc, rtx tlsarg, rtx cookie)
     call[0] = gen_rtx_SET (value, call[0]);
 
   call[1] = gen_rtx_USE (VOIDmode, cookie);
-  call[2] = gen_hard_reg_clobber (Pmode, LR_REGNO);
+  n = 2;
+  if (TARGET_SECURE_PLT
+      && flag_pic
+      && GET_CODE (func_addr) == SYMBOL_REF
+      && !SYMBOL_REF_LOCAL_P (func_addr))
+    call[n++] = gen_rtx_USE (VOIDmode, pic_offset_table_rtx);
 
-  insn = gen_rtx_PARALLEL (VOIDmode, gen_rtvec_v (3, call));
+  call[n++] = gen_hard_reg_clobber (Pmode, LR_REGNO);
+
+  insn = gen_rtx_PARALLEL (VOIDmode, gen_rtvec_v (n, call));
   insn = emit_call_insn (insn);
   if (abi_reg)
     use_reg (&CALL_INSN_FUNCTION_USAGE (insn), abi_reg);
diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h
index 9c0cc8de2b6..0281000e5e2 100644
--- a/gcc/config/rs6000/rs6000.h
+++ b/gcc/config/rs6000/rs6000.h
@@ -1489,6 +1489,16 @@ extern enum reg_class rs6000_constraints[RS6000_CONSTRAINT_MAX];
 #define CALL_LONG		0x00000008	/* always call indirect */
 #define CALL_LIBCALL		0x00000010	/* libcall */
 
+#define IS_V4_FP_ARGS(OP) \
+  ((INTVAL (OP) & (CALL_V4_CLEAR_FP_ARGS | CALL_V4_SET_FP_ARGS)) != 0)
+
+/* Whether OP is an UNSPEC used in !TARGET_TLS_MARKER calls.  */
+#define IS_NOMARK_TLSGETADDR(OP)		\
+  (!TARGET_TLS_MARKERS				\
+   && GET_CODE (OP) == UNSPEC			\
+   && (XINT (OP, 1) == UNSPEC_TLSGD		\
+       || XINT (OP, 1) == UNSPEC_TLSLD))
+
 /* We don't have prologue and epilogue functions to save/restore
    everything for most ABIs.  */
 #define WORLD_SAVE_P(INFO) 0
diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index 344cfa77887..cbb6bf3a57d 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -9437,44 +9437,6 @@ (define_peephole2
 \f
 ;; TLS support.
 
-(define_insn "*tls_gdld_nomark<bits>"
-  [(match_parallel 3 ""
-    [(set (match_operand:P 0 "gpc_reg_operand" "=b")
-	  (call (mem:SI (match_operand:P 1))
-		(match_operand:P 2 "unspec_tls")))
-     (match_dup 2)])]
-  "HAVE_AS_TLS && !TARGET_TLS_MARKERS && DEFAULT_ABI != ABI_DARWIN"
-{
-  rtx op[3];
-  op[0] = operands[0];
-  op[1] = XVECEXP (operands[2], 0, 0);
-  if (XINT (operands[2], 1) == UNSPEC_TLSGD)
-    {
-      op[2] = XVECEXP (operands[2], 0, 1);
-      if (TARGET_CMODEL != CMODEL_SMALL)
-	output_asm_insn ("addis %0,%2,%1@got@tlsgd@ha\;"
-			 "addi %0,%0,%1@got@tlsgd@l", op);
-      else
-	output_asm_insn ("addi %0,%2,%1@got@tlsgd", op);
-    }
-  else
-    {
-      if (TARGET_CMODEL != CMODEL_SMALL)
-	output_asm_insn ("addis %0,%1,%&@got@tlsld@ha\;"
-			 "addi %0,%0,%&@got@tlsld@l", op);
-      else
-	output_asm_insn ("addi %0,%1,%&@got@tlsld", op);
-    }
-  return rs6000_call_template (operands, 1);
-}
-  [(set_attr "type" "two")
-   (set (attr "length")
-     (cond [(match_test "TARGET_CMODEL != CMODEL_SMALL")
-		(const_int 16)
-	    (match_test "DEFAULT_ABI != ABI_V4")
-		(const_int 12)]
-	(const_int 8)))])
-
 (define_insn_and_split "*tls_gd<bits>"
   [(set (match_operand:P 0 "gpc_reg_operand" "=b")
 	(unspec:P [(match_operand:P 1 "rs6000_tls_symbol_ref" "")
@@ -10367,7 +10329,8 @@ (define_insn "*call_value_local32"
 	      (match_operand 2)))
    (use (match_operand:SI 3 "immediate_operand" "O,n"))
    (clobber (reg:SI LR_REGNO))]
-  "(INTVAL (operands[3]) & CALL_LONG) == 0"
+  "(INTVAL (operands[3]) & CALL_LONG) == 0
+   && !IS_NOMARK_TLSGETADDR (operands[2])"
 {
   if (INTVAL (operands[3]) & CALL_V4_SET_FP_ARGS)
     output_asm_insn ("crxor 6,6,6", operands);
@@ -10387,7 +10350,8 @@ (define_insn "*call_value_local64"
 	      (match_operand 2)))
    (use (match_operand:SI 3 "immediate_operand" "O,n"))
    (clobber (reg:DI LR_REGNO))]
-  "TARGET_64BIT && (INTVAL (operands[3]) & CALL_LONG) == 0"
+  "TARGET_64BIT && (INTVAL (operands[3]) & CALL_LONG) == 0
+   && !IS_NOMARK_TLSGETADDR (operands[2])"
 {
   if (INTVAL (operands[3]) & CALL_V4_SET_FP_ARGS)
     output_asm_insn ("crxor 6,6,6", operands);
@@ -10435,7 +10399,7 @@ (define_insn "*call_indirect_nonlocal_sysv<mode>"
 		  (const_string "8")]
 	      (const_string "4")))])
 
-(define_insn_and_split "*call_nonlocal_sysv<mode>"
+(define_insn "*call_nonlocal_sysv<mode>"
   [(call (mem:SI (match_operand:P 0 "symbol_ref_operand" "s,s"))
 	 (match_operand 1))
    (use (match_operand:SI 2 "immediate_operand" "O,n"))
@@ -10451,17 +10415,6 @@ (define_insn_and_split "*call_nonlocal_sysv<mode>"
     output_asm_insn ("creqv 6,6,6", operands);
 
   return rs6000_call_template (operands, 0);
-}
-  "DEFAULT_ABI == ABI_V4
-   && TARGET_SECURE_PLT && flag_pic && !SYMBOL_REF_LOCAL_P (operands[0])
-   && (INTVAL (operands[2]) & CALL_LONG) == 0"
-  [(parallel [(call (mem:SI (match_dup 0))
-		    (match_dup 1))
-	      (use (match_dup 2))
-	      (use (match_dup 3))
-	      (clobber (reg:SI LR_REGNO))])]
-{
-  operands[3] = pic_offset_table_rtx;
 }
   [(set_attr "type" "branch,branch")
    (set_attr "length" "4,8")])
@@ -10490,13 +10443,16 @@ (define_insn "*call_nonlocal_sysv_secure<mode>"
 (define_insn "*call_value_indirect_nonlocal_sysv<mode>"
   [(set (match_operand 0 "" "")
 	(call (mem:SI (match_operand:P 1 "indirect_call_operand" "c,*l,X"))
-	      (match_operand 2)))
+	      (match_operand:P 2 "unspec_tls" "")))
    (use (match_operand:SI 3 "immediate_operand" "n,n,n"))
    (clobber (reg:P LR_REGNO))]
   "DEFAULT_ABI == ABI_V4
    || DEFAULT_ABI == ABI_DARWIN"
 {
-  if (INTVAL (operands[3]) & CALL_V4_SET_FP_ARGS)
+  if (IS_NOMARK_TLSGETADDR (operands[2]))
+    rs6000_output_tlsargs (operands);
+
+  else if (INTVAL (operands[3]) & CALL_V4_SET_FP_ARGS)
     output_asm_insn ("crxor 6,6,6", operands);
 
   else if (INTVAL (operands[3]) & CALL_V4_CLEAR_FP_ARGS)
@@ -10506,27 +10462,30 @@ (define_insn "*call_value_indirect_nonlocal_sysv<mode>"
 }
   [(set_attr "type" "jmpreg")
    (set (attr "length")
-	(cond [(and (and (match_test "!rs6000_speculate_indirect_jumps")
-			 (match_test "which_alternative != 1"))
-		    (match_test "(INTVAL (operands[3]) & (CALL_V4_SET_FP_ARGS | CALL_V4_CLEAR_FP_ARGS))"))
-		  (const_string "12")
-	       (ior (and (match_test "!rs6000_speculate_indirect_jumps")
-			 (match_test "which_alternative != 1"))
-		   (match_test "(INTVAL (operands[3]) & (CALL_V4_SET_FP_ARGS | CALL_V4_CLEAR_FP_ARGS))"))
-		  (const_string "8")]
-	      (const_string "4")))])
-
-(define_insn_and_split "*call_value_nonlocal_sysv<mode>"
+	(plus
+	  (if_then_else (ior (match_test "IS_NOMARK_TLSGETADDR (operands[2])")
+			     (match_test "IS_V4_FP_ARGS (operands[3])"))
+	    (const_int 4)
+	    (const_int 0))
+	  (if_then_else (and (match_test "!rs6000_speculate_indirect_jumps")
+			     (match_test "which_alternative != 1"))
+	    (const_int 8)
+	    (const_int 4))))])
+
+(define_insn "*call_value_nonlocal_sysv<mode>"
   [(set (match_operand 0 "" "")
-	(call (mem:SI (match_operand:P 1 "symbol_ref_operand" "s,s"))
-	      (match_operand 2)))
-   (use (match_operand:SI 3 "immediate_operand" "O,n"))
+	(call (mem:SI (match_operand:P 1 "symbol_ref_operand" "s"))
+	      (match_operand:P 2 "unspec_tls" "")))
+   (use (match_operand:SI 3 "immediate_operand" "n"))
    (clobber (reg:P LR_REGNO))]
   "(DEFAULT_ABI == ABI_DARWIN
-   || (DEFAULT_ABI == ABI_V4
-       && (INTVAL (operands[3]) & CALL_LONG) == 0))"
+    || (DEFAULT_ABI == ABI_V4
+	&& (INTVAL (operands[3]) & CALL_LONG) == 0))"
 {
-  if (INTVAL (operands[3]) & CALL_V4_SET_FP_ARGS)
+  if (IS_NOMARK_TLSGETADDR (operands[2]))
+    rs6000_output_tlsargs (operands);
+
+  else if (INTVAL (operands[3]) & CALL_V4_SET_FP_ARGS)
     output_asm_insn ("crxor 6,6,6", operands);
 
   else if (INTVAL (operands[3]) & CALL_V4_CLEAR_FP_ARGS)
@@ -10534,33 +10493,28 @@ (define_insn_and_split "*call_value_nonlocal_sysv<mode>"
 
   return rs6000_call_template (operands, 1);
 }
-  "DEFAULT_ABI == ABI_V4
-   && TARGET_SECURE_PLT && flag_pic && !SYMBOL_REF_LOCAL_P (operands[1])
-   && (INTVAL (operands[3]) & CALL_LONG) == 0"
-  [(parallel [(set (match_dup 0)
-		   (call (mem:SI (match_dup 1))
-			 (match_dup 2)))
-	      (use (match_dup 3))
-	      (use (match_dup 4))
-	      (clobber (reg:SI LR_REGNO))])]
-{
-  operands[4] = pic_offset_table_rtx;
-}
-  [(set_attr "type" "branch,branch")
-   (set_attr "length" "4,8")])
+  [(set_attr "type" "branch")
+   (set (attr "length")
+	(if_then_else (ior (match_test "IS_NOMARK_TLSGETADDR (operands[2])")
+			   (match_test "IS_V4_FP_ARGS (operands[3])"))
+	  (const_int 8)
+	  (const_int 4)))])
 
 (define_insn "*call_value_nonlocal_sysv_secure<mode>"
   [(set (match_operand 0 "" "")
-	(call (mem:SI (match_operand:P 1 "symbol_ref_operand" "s,s"))
-	      (match_operand 2)))
-   (use (match_operand:SI 3 "immediate_operand" "O,n"))
-   (use (match_operand:SI 4 "register_operand" "r,r"))
+	(call (mem:SI (match_operand:P 1 "symbol_ref_operand" "s"))
+	      (match_operand:P 2 "unspec_tls" "")))
+   (use (match_operand:SI 3 "immediate_operand" "n"))
+   (use (match_operand:SI 4 "register_operand" "r"))
    (clobber (reg:P LR_REGNO))]
   "(DEFAULT_ABI == ABI_V4
     && TARGET_SECURE_PLT && flag_pic && !SYMBOL_REF_LOCAL_P (operands[1])
     && (INTVAL (operands[3]) & CALL_LONG) == 0)"
 {
-  if (INTVAL (operands[3]) & CALL_V4_SET_FP_ARGS)
+  if (IS_NOMARK_TLSGETADDR (operands[2]))
+    rs6000_output_tlsargs (operands);
+
+  else if (INTVAL (operands[3]) & CALL_V4_SET_FP_ARGS)
     output_asm_insn ("crxor 6,6,6", operands);
 
   else if (INTVAL (operands[3]) & CALL_V4_CLEAR_FP_ARGS)
@@ -10568,9 +10522,12 @@ (define_insn "*call_value_nonlocal_sysv_secure<mode>"
 
   return rs6000_call_template (operands, 1);
 }
-  [(set_attr "type" "branch,branch")
-   (set_attr "length" "4,8")])
-
+  [(set_attr "type" "branch")
+   (set (attr "length")
+	(if_then_else (ior (match_test "IS_NOMARK_TLSGETADDR (operands[2])")
+			   (match_test "IS_V4_FP_ARGS (operands[3])"))
+	  (const_int 8)
+	  (const_int 4)))])
 
 ;; Call to AIX abi function in the same module.
 
@@ -10587,7 +10544,8 @@ (define_insn "*call_value_local_aix<mode>"
 	(call (mem:SI (match_operand:P 1 "current_file_function_operand" "s"))
 	      (match_operand 2)))
    (clobber (reg:P LR_REGNO))]
-  "DEFAULT_ABI == ABI_AIX || DEFAULT_ABI == ABI_ELFv2"
+  "(DEFAULT_ABI == ABI_AIX || DEFAULT_ABI == ABI_ELFv2)
+   && !IS_NOMARK_TLSGETADDR (operands[2])"
   "bl %z1"
   [(set_attr "type" "branch")])
 
@@ -10608,14 +10566,22 @@ (define_insn "*call_nonlocal_aix<mode>"
 (define_insn "*call_value_nonlocal_aix<mode>"
   [(set (match_operand 0 "" "")
 	(call (mem:SI (match_operand:P 1 "symbol_ref_operand" "s"))
-	      (match_operand 2)))
+	      (match_operand:P 2 "unspec_tls" "")))
    (clobber (reg:P LR_REGNO))]
   "DEFAULT_ABI == ABI_AIX || DEFAULT_ABI == ABI_ELFv2"
 {
+  if (IS_NOMARK_TLSGETADDR (operands[2]))
+    rs6000_output_tlsargs (operands);
+
   return rs6000_call_template (operands, 1);
 }
   [(set_attr "type" "branch")
-   (set_attr "length" "8")])
+   (set (attr "length")
+	(if_then_else (match_test "IS_NOMARK_TLSGETADDR (operands[2])")
+	  (if_then_else (match_test "TARGET_CMODEL != CMODEL_SMALL")
+	    (const_int 16)
+	    (const_int 12))
+	  (const_int 8)))])
 
 ;; Call to indirect functions with the AIX abi using a 3 word descriptor.
 ;; Operand0 is the addresss of the function to call
@@ -10642,20 +10608,31 @@ (define_insn "*call_indirect_aix<mode>"
 (define_insn "*call_value_indirect_aix<mode>"
   [(set (match_operand 0 "" "")
 	(call (mem:SI (match_operand:P 1 "indirect_call_operand" "c,*l,X"))
-	      (match_operand 2)))
+	      (match_operand:P 2 "unspec_tls" "")))
    (use (match_operand:P 3 "memory_operand" "<ptrm>,<ptrm>,<ptrm>"))
-   (set (reg:P TOC_REGNUM) (unspec:P [(match_operand:P 4 "const_int_operand" "n,n,n")] UNSPEC_TOCSLOT))
+   (set (reg:P TOC_REGNUM)
+	(unspec:P [(match_operand:P 4 "const_int_operand" "n,n,n")]
+		  UNSPEC_TOCSLOT))
    (clobber (reg:P LR_REGNO))]
   "DEFAULT_ABI == ABI_AIX"
 {
+  if (IS_NOMARK_TLSGETADDR (operands[2]))
+    rs6000_output_tlsargs (operands);
+
   return rs6000_indirect_call_template (operands, 1);
 }
   [(set_attr "type" "jmpreg")
    (set (attr "length")
-	(if_then_else (and (match_test "!rs6000_speculate_indirect_jumps")
-			   (match_test "which_alternative != 1"))
-		      (const_string "16")
-		      (const_string "12")))])
+	(plus
+	  (if_then_else (match_test "IS_NOMARK_TLSGETADDR (operands[2])")
+	    (if_then_else (match_test "TARGET_CMODEL != CMODEL_SMALL")
+	      (const_int 8)
+	      (const_int 4))
+	    (const_int 0))
+	  (if_then_else (and (match_test "!rs6000_speculate_indirect_jumps")
+			     (match_test "which_alternative != 1"))
+	    (const_string "16")
+	    (const_string "12"))))])
 
 ;; Call to indirect functions with the ELFv2 ABI.
 ;; Operand0 is the addresss of the function to call
@@ -10680,19 +10657,30 @@ (define_insn "*call_indirect_elfv2<mode>"
 (define_insn "*call_value_indirect_elfv2<mode>"
   [(set (match_operand 0 "" "")
 	(call (mem:SI (match_operand:P 1 "indirect_call_operand" "c,*l,X"))
-	      (match_operand 2)))
-   (set (reg:P TOC_REGNUM) (unspec:P [(match_operand:P 3 "const_int_operand" "n,n,n")] UNSPEC_TOCSLOT))
+	      (match_operand:P 2 "unspec_tls" "")))
+   (set (reg:P TOC_REGNUM)
+	(unspec:P [(match_operand:P 3 "const_int_operand" "n,n,n")]
+		  UNSPEC_TOCSLOT))
    (clobber (reg:P LR_REGNO))]
   "DEFAULT_ABI == ABI_ELFv2"
 {
+  if (IS_NOMARK_TLSGETADDR (operands[2]))
+    rs6000_output_tlsargs (operands);
+
   return rs6000_indirect_call_template (operands, 1);
 }
   [(set_attr "type" "jmpreg")
    (set (attr "length")
-	(if_then_else (and (match_test "!rs6000_speculate_indirect_jumps")
-			   (match_test "which_alternative != 1"))
-		      (const_string "12")
-		      (const_string "8")))])
+	(plus
+	  (if_then_else (match_test "IS_NOMARK_TLSGETADDR (operands[2])")
+	    (if_then_else (match_test "TARGET_CMODEL != CMODEL_SMALL")
+	      (const_int 8)
+	      (const_int 4))
+	    (const_int 0))
+	  (if_then_else (and (match_test "!rs6000_speculate_indirect_jumps")
+			     (match_test "which_alternative != 1"))
+	    (const_string "12")
+	    (const_string "8"))))])
 
 ;; Call subroutine returning any type.
 (define_expand "untyped_call"

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: [RS6000] PR88614, output_operand: invalid %z value
  2019-01-21 12:19     ` Alan Modra
@ 2019-01-21 14:23       ` Segher Boessenkool
  2019-01-22  0:30         ` Alan Modra
  0 siblings, 1 reply; 7+ messages in thread
From: Segher Boessenkool @ 2019-01-21 14:23 UTC (permalink / raw)
  To: Alan Modra; +Cc: gcc-patches

On Mon, Jan 21, 2019 at 10:48:57PM +1030, Alan Modra wrote:
> On Mon, Jan 21, 2019 at 12:08:33AM +1030, Alan Modra wrote:
> > Hmm, if I invent a couple of new unspecs, UNSPEC_TLSGD_NOMARK and
> > UNSPEC_TLSLD_NOMARK, using them in place of UNSPEC_TLSGD and
> > UNSPEC_TLSLD when !TARGET_TLS_MARKERS then that should be enough to
> > tell when we have a -mno-tls-markers __tls_get_addr call.  So I guess
> > I could kill off edit_tls_call_insn and tls_gdld_nomark.  The
> > call_value_local and call_value_indirect insns would then need to
> > detect the special call and emit the arg setup insns.
> 
> Here's what the revised approach looks like, but without using new
> unspecs.  Bootstrap and regression test on powerpc64le-linux and
> powerpc64-linux biarch completed, and testing on powerpc64le-linux
> with -mno-tls-markers.  powerpc64-linux -mno-tls-markers testing still
> in progress.  OK?

This is easier to grok, thanks.

I think this would be nicer if you still used insn alternatives here.
What is needed for that?

> +void
> +rs6000_output_tlsargs (rtx *operands)
> +{
> +  rtx op[3];

Maybe comment what this temporary is for?

The patch is okay for trunk (if it survives on at least all three linux
targets).  Thanks!


Segher

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

* Re: [RS6000] PR88614, output_operand: invalid %z value
  2019-01-21 14:23       ` Segher Boessenkool
@ 2019-01-22  0:30         ` Alan Modra
  2019-01-22  0:35           ` Segher Boessenkool
  0 siblings, 1 reply; 7+ messages in thread
From: Alan Modra @ 2019-01-22  0:30 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: gcc-patches

On Mon, Jan 21, 2019 at 08:22:58AM -0600, Segher Boessenkool wrote:
> On Mon, Jan 21, 2019 at 10:48:57PM +1030, Alan Modra wrote:
> > Here's what the revised approach looks like, but without using new
> > unspecs.  Bootstrap and regression test on powerpc64le-linux and
> > powerpc64-linux biarch completed, and testing on powerpc64le-linux
> > with -mno-tls-markers.  powerpc64-linux -mno-tls-markers testing still
> > in progress.  OK?
> 
> This is easier to grok, thanks.

Yes, and not having to edit __tls_get_addr call patterns is good too.

> I think this would be nicer if you still used insn alternatives here.
> What is needed for that?

A new symbol constraint especially for __tls_get_addr?  Actually two
new constraints for ppc64, one for small model, the other for
medium/large!  No way.

> The patch is okay for trunk (if it survives on at least all three linux
> targets).  Thanks!

Happily it did, even with -mno-tls-markers.

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: [RS6000] PR88614, output_operand: invalid %z value
  2019-01-22  0:30         ` Alan Modra
@ 2019-01-22  0:35           ` Segher Boessenkool
  0 siblings, 0 replies; 7+ messages in thread
From: Segher Boessenkool @ 2019-01-22  0:35 UTC (permalink / raw)
  To: Alan Modra; +Cc: gcc-patches

On Tue, Jan 22, 2019 at 10:59:57AM +1030, Alan Modra wrote:
> > I think this would be nicer if you still used insn alternatives here.
> > What is needed for that?
> 
> A new symbol constraint especially for __tls_get_addr?  Actually two
> new constraints for ppc64, one for small model, the other for
> medium/large!  No way.

You don't need new constraints I think; you can use the "enabled" attribute
for it.  Not a stage4 thing I guess.

> > The patch is okay for trunk (if it survives on at least all three linux
> > targets).  Thanks!
> 
> Happily it did, even with -mno-tls-markers.

:-)


Segher

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

end of thread, other threads:[~2019-01-22  0:35 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-06 22:59 [RS6000] PR88614, output_operand: invalid %z value Alan Modra
2019-01-18 22:02 ` Segher Boessenkool
2019-01-20 13:38   ` Alan Modra
2019-01-21 12:19     ` Alan Modra
2019-01-21 14:23       ` Segher Boessenkool
2019-01-22  0:30         ` Alan Modra
2019-01-22  0:35           ` 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).