[PATCH] [PowerPC] Replace foul hackery with real calls to __tls_get_addr

[Bill Schmidt]

>>! In D6209#14, @jhibbits wrote:
>>>! In D6209#13, @wschmidt wrote:
>> 
>>> I don't follow.  VK_PLT is a decorator for @PLT, and I already have tests for ensuring @PLT becomes R_PPC_PLTREL24 (ppc-reloc.s).  Isn't a sufficient test for this simply to ensure that @PLT is generated when that code path is followed?  Do you instead expect that the inline assembler will recognize a call to __tls_get_addr() and itself add VK_PLT?  The other relocations for PLT are identical to those of powerpc64 in this case.  Okay, maybe a test to generate R_PPC_GOT_TLSLD/R_PPC_GOT_TLSGD instead of R_PPC_GOT_TLSLD_LO/R_PPC_GOT_TLSGD_LO might be prudent, and I can readily add, but that's not what you requested.
>> 
>> Normally, @PLT is how you get R_PPC_PLTREL24, but that is apparently not the case here.  You added some special-case code in PPCAsmPrinter.cpp that specifically places the VK_PLT on the __tls_get_addr symbol ONLY, which is never emitted with @plt.  (It's in the deleted chunks in this patch.)  This appears to be a way of forcing the relocation that you want, and this path is not tested in your test cases.
>> 
>> From what I have been able to piece together, I believe that placing MO_PLT_OR_STUB on the TGA operand early will eventually cause VK_PLT to be used, and hence generate the correct relocation.  I can't tell whether this regresses your code or not, though, because there wasn't a test.
>> 
>> The way I know there isn't a test is that I only discovered your code when I was inspecting the patch.  I deleted your VK_PLT workaround with impunity and none of the tests failed.  When I spotted that I added the MO_PLT_OR_STUB code.
> 
> There are two reasons for this:  I took the path of least code change, where you were already using VK_None, on the MCSymbolExpr, and the call was already lowered, we were constructing the branch by hand.  The VK_PLT normally gets added in GetSymbolRef() in PPCMCInstLower.cpp.  An alternative route, in hindsight, could have been to lower it after constructing the call, but again that's hindsight, and with your change becomes irrelevant.  Without explicitly adding the VK_PLT, it was not generating the @PLT you see in tls-pic.ll (I checked).  So, really, it was only a case of where in the path it was added, and in this case the flag was added late, so the kind was needed.
> 
> Since you already did a code inspection, you may have seen that in PrepareCall() PPCII::MO_PLT_OR_STUB gets added automatically, making your addition unnecessary, so I think you can drop that from the patch (if it already passes the tests, then it works).

Mea culpa -- I missed the @PLT examples in the tls-pic.ll test.  I misunderstood what was going on with the extra code there, and agree that I can remove the extra code (which makes things much cleaner, yay).  Thanks for educating me, and putting up with the nonsense. ;)  This stuff is so dispersed throughout the code that it's hard to keep all the threads together.

http://reviews.llvm.org/D6209