[2/3] TLS loads optimization (hoist)

[xiangzhangllvm]

Summary

Refine tls-load-hoist llvm option from string value to bool

[pengfei]

This can be [2/3] ?

[xiangzhangllvm]

In D122890#3421890, @pengfei wrote:

This can be [2/3] ?

done

[LuoYuanke]

LGTM.

[lebedev.ri]

I'm not really sure that attribute should be there, especially after the optimization is enabled by default and cl::opt is inverted.

[LuoYuanke]

In D122890#3421936, @lebedev.ri wrote:

I'm not really sure that attribute should be there, especially after the optimization is enabled by default and cl::opt is inverted.

I think we can leave end user a chance to enable it or not. Maybe for debug purpose.

[lebedev.ri]

In D122890#3421946, @LuoYuanke wrote:

In D122890#3421936, @lebedev.ri wrote:

I'm not really sure that attribute should be there, especially after the optimization is enabled by default and cl::opt is inverted.

I think we can leave end user a chance to enable it or not. Maybe for debug purpose.

I don't believe that is consistent with the current situation, i.e. is there any prior art for this, an attribute that specifically tells to perform some optimization?
How should it play with inliner, should it stop the inlining if they mismatch on the caller-callee, or should it be propagated to callee? Etc.

[xiangzhangllvm]

In D122890#3421936, @lebedev.ri wrote:

I'm not really sure that attribute should be there, especially after the optimization is enabled by default and cl::opt is inverted.

Yes, In fact I saw it before, due to, D122866, I prefer keep it here.
And if customer strongly require the option, I may need to re-active the D122558

[LuoYuanke]

In D122890#3421961, @lebedev.ri wrote:

In D122890#3421946, @LuoYuanke wrote:

In D122890#3421936, @lebedev.ri wrote:

I'm not really sure that attribute should be there, especially after the optimization is enabled by default and cl::opt is inverted.

I think we can leave end user a chance to enable it or not. Maybe for debug purpose.

I don't believe that is consistent with the current situation, i.e. is there any prior art for this, an attribute that specifically tells to perform some optimization?
How should it play with inliner, should it stop the inlining if they mismatch on the caller-callee, or should it be propagated to callee? Etc.

I think this pass runs after inlining.

llvm/docs/LangRef.rst

 ``"thunk"``
     This attribute indicates that the function will delegate to some other
     function with a tail call. The prototype of a thunk should not be used for
     optimization purposes. The caller is expected to cast the thunk prototype to
     match the thunk target prototype.
 
 ``"tls-load-hoist"``
     This attribute indicates that the function will try to reduce redundant
-    tls address caculation by hoisting tls variable.
+    tls address calculation by hoisting tls variable.
 
 ``uwtable[(sync|async)]``
     This attribute indicates that the ABI being targeted requires that
     an unwind table entry be produced for this function even if we can
     show that no exceptions passes by it. This is normally the case for
     the ELF x86-64 abi, but it can be disabled for some compilation
     units. The optional parameter describes what kind of unwind tables
     to generate: ``sync`` for normal unwind tables, ``async`` for asynchronous

llvm/lib/Transforms/Scalar/TLSVariableHoist.cpp

 #include <tuple>
 #include <utility>
 
 using namespace llvm;
 using namespace tlshoist;
 
 #define DEBUG_TYPE "tlshoist"
 
-// TODO: Support "strict" model if we need to strictly load TLS address,
-// because "non-optimize" may also do some optimization in other passes.
-static cl::opt<std::string> TLSLoadHoist(
-    "tls-load-hoist",
-    cl::desc(
-        "hoist the TLS loads in PIC model: "
-        "tls-load-hoist=optimize: Eleminate redundant TLS load(s)."
-        "tls-load-hoist=strict: Strictly load TLS address before every use."
-        "tls-load-hoist=non-optimize: Generally load TLS before use(s)."),
-    cl::init("non-optimize"), cl::Hidden);
+static cl::opt<bool> TLSLoadHoist(
+    "tls-load-hoist", cl::init(false), cl::Hidden,
+    cl::desc("hoist the TLS loads in PIC model to eleminate redundant "
+             "TLS address calculation."));
 
 namespace {
 
 /// The TLS Variable hoist pass.
 class TLSVariableHoistLegacyPass : public FunctionPass {
 public:
   static char ID; // Pass identification, replacement for typeid
 

 }
 
 /// Optimize expensive TLS variables in the given function.
 bool TLSVariableHoistPass::runImpl(Function &Fn, DominatorTree &DT,
                                    LoopInfo &LI) {
   if (Fn.hasOptNone())
     return false;
 
-  if (TLSLoadHoist != "optimize" &&
-      !Fn.getAttributes().hasFnAttr("tls-load-hoist"))
+  if (!TLSLoadHoist && !Fn.getAttributes().hasFnAttr("tls-load-hoist"))
     return false;
 
   this->LI = &LI;
   this->DT = &DT;
   assert(this->LI && this->DT && "Unexcepted requirement!");
 
   // Collect all TLS variable candidates.
   collectTLSCandidates(Fn);

llvm/test/CodeGen/X86/tls-loads-control.ll

-; RUN: llc -mtriple=x86_64-unknown-unknown -O2 --relocation-model=pic --tls-load-hoist=optimize --stop-after=tlshoist -o - %s | FileCheck %s
+; RUN: llc -mtriple=x86_64-unknown-unknown -O2 --relocation-model=pic --tls-load-hoist=true --stop-after=tlshoist -o - %s | FileCheck %s
 ; RUN: llc -mtriple=x86_64-unknown-unknown -O2 --relocation-model=pic --stop-after=tlshoist -o - %s | FileCheck %s
 
 ; This test come from compiling clang/test/CodeGen/intel/tls_loads.cpp with:
 ; (clang tls_loads.cpp -fPIC -ftls-model=global-dynamic -O2 -S -emit-llvm)
 
 ; // Variable declaration and definition:
 ; thread_local int thl_x;
 ; thread_local int thl_x2;

llvm/test/CodeGen/X86/tls-loads-control2.ll

-; RUN: opt -S -mtriple=x86_64-unknown-unknown -tlshoist --relocation-model=pic --tls-load-hoist=optimize -o - %s | FileCheck %s --check-prefix=HOIST0
-; RUN: opt -S -mtriple=x86_64-unknown-unknown -tlshoist --relocation-model=pic --tls-load-hoist=non-optimize -o - %s | FileCheck %s --check-prefix=HOIST2
+; RUN: opt -S -mtriple=x86_64-unknown-unknown -tlshoist --relocation-model=pic --tls-load-hoist=true -o - %s | FileCheck %s --check-prefix=HOIST0
 ; RUN: opt -S -mtriple=x86_64-unknown-unknown -tlshoist --relocation-model=pic -o - %s | FileCheck %s --check-prefix=HOIST2
 
 $_ZTW5thl_x = comdat any
 
 @thl_x = thread_local global i32 0, align 4
 
 ; Function Attrs: mustprogress uwtable
 define i32 @_Z2f1i(i32 %c) local_unnamed_addr #0 {

llvm/test/CodeGen/X86/tls-loads-control3.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc -mtriple=x86_64-unknown-unknown -O2 --relocation-model=pic --tls-load-hoist=optimize -o - %s | FileCheck %s --check-prefix=HOIST0
-; RUN: llc -mtriple=x86_64-unknown-unknown -O2 --relocation-model=pic --tls-load-hoist=non-optimize -o - %s | FileCheck %s --check-prefix=HOIST2
+; RUN: llc -mtriple=x86_64-unknown-unknown -O2 --relocation-model=pic --tls-load-hoist=true -o - %s | FileCheck %s --check-prefix=HOIST0
 ; RUN: llc -mtriple=x86_64-unknown-unknown -O2 --relocation-model=pic -o - %s | FileCheck %s --check-prefix=HOIST2
 
 ; This test has no module flag {"tls-load-hoist", i32 0}, so use --tls-load-hoist=x
 ; to choose the way of loading thread_local address.
 
 ; This test come from compiling clang/test/CodeGen/intel/tls_loads.cpp with:
 ; (clang tls_loads.cpp -fPIC -ftls-model=global-dynamic -O2 -S -emit-llvm)