[PATCH] D120000: [1/2] TLS loads opimization (hoist)

[Nuno Lopes via Phabricator via llvm-commits]

nlopes added a comment.

Thank you for the explanation!
My confusion I guess stemmed from all the assembly in the comments and tests, which is quite confusing given that the transformation works at IR level.
Anyway, this transformation works by creating some artificial bitcasts that you expect to be carried over to the backend so the lowering can share the calls to the TLS function.

This strategy seems very brittle to me. If some later optimization decides to remove the bitcast, your optimization will stop working. It's very likely that will be the case once opaque pointers take over.

I guess in the end you want a single fn call per variable, which could be introduced always after the allocas? Unless the price for the call is too high, and then you want to restrict it to the paths where it existed already.

I second Roman's question: why is this not enabled by default?
Also, you cannot introduce a new attribute in LLVM IR without documenting it first in LangRef. Also, we don't use attributes to toggle optimizations on/off. Please remove it and use a cmd flag if the optimization can't be enabled by default for some reason.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120000/new/

https://reviews.llvm.org/D120000