[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