[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)
----------------
xiangzhangllvm wrote:
> craig.topper wrote:
> > 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.
> Eh ..., Yes, Seems make sense, just a question, in which case there will be an empty BB in IR level ? (even the last BB I see is always append with ret instruction) .
>  I check the BasicBlock::getTerminator() , it is possible return nullptr.
> 
> ```
> const Instruction *BasicBlock::getTerminator() const {
>   if (InstList.empty() || !InstList.back().isTerminator())
>     return nullptr;
>   return &InstList.back();
> }
> ```
It can happen when the basic block is first created and not connected to the CFG. But if it's connected, it has a terminator. The successor list of a basic block is stored directly in the terminator instruction. The predecessor list is found by iterating all of the users of the BasicBlock* and looking at which uses are terminator instructions.


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

https://reviews.llvm.org/D120000