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

[Xiang Zhang via Phabricator via llvm-commits]

xiangzhangllvm marked 5 inline comments as done.
xiangzhangllvm 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)
----------------
craig.topper wrote:
> 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.
Thanks for your explain!


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

https://reviews.llvm.org/D120000