On Wed, Nov 28, 2018 at 11:37:42AM +1030, Alan Modra wrote: > 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.
Right. I grepped if there were other parallels without predicate, not finding any, but I did not notice that half of the predicates used are any_parallel_operand. > > 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)". And the "unspec_tls". Right. > 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. Well, but do we gain anything if it is eventually deleted? Simpler code? > > Please test with -mtls-markers, too, if you can, and test on AIX. > > Presumably you mean -mno-tls-markers. -mtls-markers is the default. Yeah, the ! case. > > 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. Sounds good. Segher
