On Tue, Nov 27, 2018 at 10:29:29AM -0600, Segher Boessenkool wrote:
> Hi!
Thanks for the review!
> > +(define_insn "*tls_gdld_aix<P:bits>"
> > + [(match_parallel 3 ""
>
> A match_parallel without predicate... Does this work?!
Yes. In fact, rs6000/predicates.md any_parallel_operand is useless
except as documentation. The only thing it checks is that its operand
is a parallel, but that has already been checked.
> Does this not
> accidentally pick up the wrong things?
No. The purpose of the predicate is to match anything beyond the
vector of expressions. So tls_gdld_aix* matches insns that look like:
(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)
...
This is sufficiently different from other calls, by virtue of the
"(match_dup 2)".
Incidentally, I think tls_gdld_aix and tls_gdld_sysv could be merged
at the expense of complicating the length attribute expression.
> Do you think we should to deprecate -mtls-markers in GCC 9?
Support for the TLS marker relocs was added to binutils in 2009 (git
commit 727fc41e077), so yes, the option is not likely to be useful
nowadays.
> Please test with -mtls-markers, too, if you can, and test on AIX.
Presumably you mean -mno-tls-markers. -mtls-markers is the default.
> Looks fine. Thank you for the cleanup! Okay for trunk, but please do the
> extra testing.
Huh, local testing of -mno-tls-markers showed a lack of a
TARGET_TLS_MARKERS check in rs6000_call_template_1. Likely this would
blow up on AIX. I'll test with the following delta.
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 56ca117a0a0..5f4fcee3b33 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -8622,7 +8622,7 @@ edit_tls_call_insn (rtx arg)
}
/* Passes the tls arg value for global dynamic and local dynamic
- emit_library_call_value in rs6000_legitimize_Tls_address to
+ emit_library_call_value in rs6000_legitimize_tls_address to
rs6000_call_aix and rs6000_call_sysv. This is used to emit the
marker relocs put on __tls_get_addr calls. */
static rtx global_tlsarg;
@@ -21429,7 +21429,7 @@ rs6000_call_template_1 (rtx *operands, unsigned int
funop, bool sibcall)
char arg[12];
arg[0] = 0;
- if (GET_CODE (operands[funop + 1]) == UNSPEC)
+ if (TARGET_TLS_MARKERS && GET_CODE (operands[funop + 1]) == UNSPEC)
{
if (XINT (operands[funop + 1], 1) == UNSPEC_TLSGD)
sprintf (arg, "(%%%u@tlsgd)", funop + 1);
--
Alan Modra
Australia Development Lab, IBM
