[PATCH] D61314: [SCCP] Remove forcedconstant, use InstSimplify to fold undef values.
Eli Friedman via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 7 13:39:51 PST 2020
efriedma added a comment.
I'd prefer to get rid of ResolveUndefsIn completely, if possible. If we don't special-case UndefValue anywhere in SCCP, that should just work, I think? SCCP should still be correct even though UndefValue isn't a "normal" constant; it doesn't try to make assumptions based on the pointer equivalence of two "constant" values, except for joins like PHI nodes where it shouldn't matter. Am I missing something? How much optimization power do we lose that way, versus this patch's approach?
The approach here is a little different from that. I'm understanding it correctly, the idea is that in cases where we can't prove a forced constant is actually constant, we instead go straight to overdefined? So we sort of keep our "undef=>undefined" extension of the SCCP model, but get rid of the shakiest part of it? That should work, I think... but I'm not sure the whole ResolvedUndefsIn is giving you any significant benefit at that point. At least, I can't think of a case off the top of my head where it helps. Maybe I'm forgetting something.
I'm not completely confident calling Simplify* with operands that aren't Constants does the right thing here; that's sort of an extension to the algorithm. I guess it should work, though?
Does this patch in its current form still depend on any of the other patches?
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