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

[Shimin Cui via Phabricator via llvm-commits]

scui added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/GlobalOpt.cpp:979
       case ICmpInst::ICMP_ULT:
       case ICmpInst::ICMP_SLT:   // X < null -> always false
         LV = ConstantInt::getFalse(GV->getContext());
----------------
efriedma wrote:
> scui wrote:
> > efriedma wrote:
> > > Can we explicitly note here, for the signed compares, why it doesn't matter if the high bit of the global is set? The logic here isn't obvious.
> > I'm not sure what comments I can add,  as I actually don't know the logic here for the folding of the signed compares.  The code has been like this from 2009 (the earliest I can found). Do you have any suggestions on how to handle all these signed compares?
> We could probably just reject icmps with a signed predicate; frontends won't normally generate signed pointer comparisons anyway.
> 
> Alternatively, you could use some tortured logic here: the program doesn't actually capture the pointer, and you can't do anything useful just knowing the sign of the pointer. So just pretend the sign bit isn't set.
> We could probably just reject icmps with a signed predicate; frontends won't normally generate signed pointer comparisons anyway.
> 

Thank you very much for the suggestions. I'll take the safe approach - rejecting the signed comparisons. Hopefully it won't have much impact on real programs.




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

https://reviews.llvm.org/D102711