[PATCH] D61314: [SCCP] Remove forcedconstant and replace some uses with constant ranges.

Eli Friedman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 20 18:07:13 PST 2020


efriedma added a comment.

You aren't really removing forcedconstant, here; instead, you're basically making every constant behave like a forcedconstant (it can be merged with another constant).  I'm not very confident our extension of the SCCP algorithm to include forcedconstants is truly sound. And I think constructing ConstantRanges out of forcedconstants makes the whole foundation shakier; I'm afraid you could end up constructing a contradiction because we don't ever force the value of the forcedconstant.

I'd be much more comfortable with pretending that undef is an opaque constant, like a GlobalValue.  That makes the whole algorithm much closer to a textbook algorithm, and it should be essentially sound, I think.  If we don't manipulate UndefValues except through ConstantExpr folding, it should be sound as long as ConstantExpr folding is itself sound.  And we don't have to do any extra work as undef gets replaced with poison.  (Once we have poison constants, I think we can treat them as Unknown without any special logic.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61314





More information about the llvm-commits mailing list