[PATCH] D102711: [GlobalOpt] Handle null check with global pointer variables

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

I'd like to see a little more test coverage, given all the different cases where the behavior of the transform changed.



================
Comment at: llvm/lib/Transforms/IPO/GlobalOpt.cpp:932
+      auto *InitBoolValue = ConstantInt::getBool(
+          GV->getContext(), !GV->hasInitializer() ||
+                                SI->getValueOperand() != GV->getInitializer());
----------------
scui wrote:
> efriedma wrote:
> > I think `GV->hasInitializer()` must be true here?
> > 
> > The equality comparison here makes me a little nervous, in case the way we analyze bitcasts changes in the future.  `SI->getValueOperand()->isNullValue()` would probably be more clear anyway.
> That's right,  GV->hasInitializer() is true here. Modify the code as you suggested. 
Would `!isa<Constant>(SI->getValueOperand()))` work?


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

https://reviews.llvm.org/D102711