[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