Re: [RS6000] PR88614, output_operand: invalid %z value

[Alan Modra <amodra@gmail.com>]

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