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

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/TLSVariableHoist.cpp:200
+  // There is unique predecessor outside the loop.
+  // Note the terminator maybe nullptr, because the PreHeader maybe an empty BB.
+  if (PreHeader)
----------------
If the Preheader exists, it isn't empty. It will always have a branch to the loop. There are no fallthroughs in IR. So the terminator will not be nullptr.


================
Comment at: llvm/lib/Transforms/Scalar/TLSVariableHoist.cpp:239
+  tlshoist::TLSCandidate &Cand = TLSCandMap[GV];
+  if (!DT)
+    return findInsertPosInEntry(Fn, Cand);
----------------
Why would DT be null? The pass has DominatorTree as a requirement.


================
Comment at: llvm/lib/Transforms/Scalar/TLSVariableHoist.cpp:248
+    Instruction *Pos = User.Inst;
+    if (LI)
+      if (Loop *L = LI->getLoopFor(BB)) {
----------------
Why would LI be null? The pass has it as a requirement


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

https://reviews.llvm.org/D120000