<div dir="ltr">Good news! (maybe)<div><br></div><div>I was focusing too much on the instruction definition and not enough on looking at how the instruction was getting created...</div><div>While I inspected the ISel lowering code creating the TLS access pseudo (in X86ISelLowering.cpp) to find out how to do something similar to what AArch64 does, I found a comment in the GetTLSADDR() function basically saying that the code there was responsible for informing the MachineFrameInfo that the function had calls... however it was only setting the "adjustsStack" flag and not the "hasCalls" flag.</div><div><br></div><div>This means that I don't strictly need to change the definition of the TLS access pseudo, and I can fix the PR with just a one-line change in X86ISelLowering.cpp.</div><div><br></div><div>Do you think it's acceptable?</div><div>I attach the new patch. I also changed the comment in the test to explain the situation.</div><div><br></div><div>Cheers,</div><div> Dario Domizioli</div><div> SN Systems - Sony Computer Entertainment Group</div><div><div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div></div></div></div><div class="gmail_extra"><br><div class="gmail_quote">On 11 November 2014 12:16, Dario Domizioli <span dir="ltr"><<a href="mailto:dario.domizioli@gmail.com" target="_blank">dario.domizioli@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Thanks Rafael!<div><br></div><div>I'll have a look at the AArch64 version (and the PPC patch) and I'll try to implement something similar.</div><div>I'll post a new patch soon.</div><span class=""><div><br></div><div>Cheers,</div><div> Dario Domizioli</div><div> SN Systems - Sony Computer Entertainment Group</div><div><br></div><div><br></div><div><br></div><div><br></div></span></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On 11 November 2014 05:45, Rafael Espíndola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">For a related issue, see <a href="http://reviews.llvm.org/D6209" target="_blank">http://reviews.llvm.org/D6209</a><br>
<div><div><br>
On 10 November 2014 16:02, Rafael Espíndola <<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>> wrote:<br>
> On 10 November 2014 10:48, Dario Domizioli <<a href="mailto:dario.domizioli@gmail.com" target="_blank">dario.domizioli@gmail.com</a>> wrote:<br>
>> Hi Rafael.<br>
>><br>
>> I have been looking at this for a while, but I cannot seem to find a way of<br>
>> preserving an LLVM intrinsic after ISel and expanding it later.<br>
>> Basically, if I add an LLVM intrinsic for TLS access in IR/IntrinsicsX86.td,<br>
>> then the backend expects me to provide an instruction selection rule for<br>
>> lowering it to MachineInstructions; however as you said we cannot do it at<br>
>> ISel stage because we must ensure the structure is preserved so that the<br>
>> linker can pattern-match it.<br>
>><br>
>> With "intrinsic", do you instead mean an actual function that we manufacture<br>
>> on the fly in Clang (or provide a declaration for in a header), and then<br>
>> eliminate / patch up in MC later on?<br>
>> That might work but it seems a bit convoluted to me. Also... if we<br>
>> manufacture the function, we end up with a declaration in the IR that<br>
>> doesn't correspond to anything in the source; if we instead have the<br>
>> function in a header, doesn't that require the user to include the header?<br>
>> Is that what you're suggesting, or am I getting confused?<br>
><br>
> Sorry, you were right. This has to be an instruction, it is too late<br>
> to have an intrinsic.<br>
><br>
> Your change to add a isCall is probably also correct. The problem with<br>
> TLS_addr64 is that it has not been upgraded to use the new call<br>
> representation with register masks.<br>
><br>
> Probably the best example of how this should work is what AArch64 does:<br>
><br>
> -----------------------------------------------------------------------<br>
> let isCall = 1, Defs = [LR] in<br>
> def TLSDESC_BLR<br>
> : Pseudo<(outs), (ins GPR64:$dest, i64imm:$sym),<br>
> [(AArch64tlsdesc_call GPR64:$dest, tglobaltlsaddr:$sym)]>;<br>
> ------------------------------------------------------------------------<br>
><br>
> so it has isCall, but not the explicit Defs list. It is created with<br>
><br>
> -------------------------------------<br>
> const uint32_t *Mask = ARI->getTLSCallPreservedMask();<br>
> ....<br>
><br>
> SmallVector<SDValue, 6> Ops;<br>
> Ops.push_back(Chain);<br>
> Ops.push_back(Func);<br>
> Ops.push_back(SymAddr);<br>
> Ops.push_back(DAG.getRegister(AArch64::X0, PtrVT));<br>
> Ops.push_back(DAG.getRegisterMask(Mask));<br>
> Ops.push_back(Glue);<br>
><br>
> SDVTList NodeTys = DAG.getVTList(MVT::Other, MVT::Glue);<br>
> Chain = DAG.getNode(AArch64ISD::TLSDESC_CALL, DL, NodeTys, Ops);<br>
> --------------------------------------<br>
><br>
> With this X86FloatingPoint.cpp should not get confused about<br>
> __tls_get_addr returning a value in the fp stack.<br>
><br>
> Cheers,<br>
> Rafael<br>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>