This is an archive of the discontinued LLVM Phabricator instance.

[ConstantFolding] Always return something from ConstantFoldConstant
ClosedPublic

Authored by nikic on Mar 3 2020, 10:22 AM.

Details

Summary

Spin-off from D75407. As described there, ConstantFoldConstant() currently returns null for non-ConstantExpr/ConstantVector inputs, but otherwise always returns non-null, independently of whether any folding has happened or not.

This is confusing and makes consumer code more complicated. I would expect either that ConstantFoldConstant() returns only if it actually folded something, or that it always returns non-null. I'm going to the latter possibility here, which appears to be more useful considering existing usage.

Diff Detail

Event Timeline

nikic created this revision.Mar 3 2020, 10:22 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 3 2020, 10:22 AM
efriedma accepted this revision.Mar 3 2020, 5:02 PM

Nice cleanup! LGTM

lib/Transforms/InstCombine/InstCombineShifts.cpp
619 ↗(On Diff #247950)

Maybe this code was written before we were doing folding in IRBuilder?

This revision is now accepted and ready to land.Mar 3 2020, 5:02 PM
This revision was automatically updated to reflect the committed changes.
nikic marked an inline comment as done.Mar 4 2020, 9:39 AM
nikic added inline comments.
lib/Transforms/InstCombine/InstCombineShifts.cpp
619 ↗(On Diff #247950)

That seems likely. I've double checked that this really does get folded without the call and dropped it in rG293d813020d7.