Page MenuHomePhabricator

[ConstantFolding] Fold undef for integer intrinsics
ClosedPublic

Authored by nikic on Dec 20 2018, 12:39 PM.

Details

Summary

This fixes https://bugs.llvm.org/show_bug.cgi?id=40110.

This implements handling of undef operands for integer intrinsics in ConstantFolding. In particular the bitcounting intrinsics (ctpop, cttz, ctlz), the with overflow intrinsics, the saturating math intrinsics and the funnel shift intrinsics.

The undef behavior follows what InstSimplify does for the general case of non-constant operands. For the bitcount intrinsics (where InstSimplify doesn't do undef handling -- there cannot be a combination of an undef + non-constant operand) I'm using a 0 result if the intrinsic is defined for zero and undef otherwise.

Diff Detail

Repository
rL LLVM

Event Timeline

nikic created this revision.Dec 20 2018, 12:39 PM
nikic marked an inline comment as done.Dec 20 2018, 12:46 PM
nikic added inline comments.
test/Transforms/InstCombine/saturating-add-sub-vector.ll
22 ↗(On Diff #179129)

I think this test could be dropped in favor of test/Analysis/ConstantFolding/saturating-add-sub.ll. It should already cover everything, just with not quite as wide vectors...

RKSimon added inline comments.Dec 21 2018, 6:49 AM
lib/Analysis/ConstantFolding.cpp
1658 ↗(On Diff #179129)

add ctpop comment

nikic updated this revision to Diff 179283.Dec 21 2018, 7:18 AM

Add ctpop comment.

nikic marked an inline comment as done and an inline comment as not done.Dec 28 2018, 7:53 AM

Ping

@spatel Any comments?

lib/Analysis/ConstantFolding.cpp
2035 ↗(On Diff #179283)

What about moving the mul cases to the top, handle this there and then fallthrough otherwise?

nikic updated this revision to Diff 179922.Jan 2 2019, 12:20 PM

Handle overflowing mul with a fallthrough case.

spatel added inline comments.Jan 10 2019, 10:18 AM
lib/Analysis/ConstantFolding.cpp
1632 ↗(On Diff #179922)

clang-format prefers "APInt *&C"?
(no space between * and &)

test/Transforms/InstCombine/saturating-add-sub-vector.ll
22 ↗(On Diff #179129)

Do you mean drop this whole file? Either way, I agree. Any tests that are just testing constant folding are better located in the ConstProp folder. If there's anything worth salvaging here, please move it. If not, let's just remove this file along with this commit.

nikic updated this revision to Diff 181145.Jan 10 2019, 1:31 PM

Fix formatting, remove InstCombine/saturating-add-sub-vector.ll test in favor of ConstantFolding/saturating-add-sub.ll.

spatel accepted this revision.Jan 10 2019, 2:32 PM

LGTM

This revision is now accepted and ready to land.Jan 10 2019, 2:32 PM
This revision was automatically updated to reflect the committed changes.