This is an archive of the discontinued LLVM Phabricator instance.

[InstCombine] Fix bug in `FoldOpIntoSelect` where we would incorrectly fold `undef` as constant
ClosedPublic

Authored by goldstein.w.n on May 1 2023, 8:30 AM.

Details

Summary

D146349 Introduced the ability to use the information from the
select condition to deduce constants as we folded a binop into
select. I.e if the select cond was icmp eq %A, 10, then in the
true-arm of select, we would be able to replace usage of A with
10.

This is broken for vectors that contain undef elements. I.e with
icmp eq %A, <10, undef>, subsituting <10, undef> for A can
result in creating a more undefined result than we otherwise would
have.

We fix the issue with simply checking if the candidate constant for
substituting may contain undef elements and don't do it in that
case.

Diff Detail

Event Timeline

goldstein.w.n created this revision.May 1 2023, 8:30 AM
goldstein.w.n requested review of this revision.May 1 2023, 8:30 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 1 2023, 8:30 AM
nikic added inline comments.May 1 2023, 8:56 AM
llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
1044

I think this needs to use isGuaranteedNotToBeUndefOrPoison to handle constant expressions correctly.

Use isGuaranteedNotToBeUndefOrPoison

goldstein.w.n marked an inline comment as done.May 1 2023, 9:32 AM
nikic added inline comments.May 1 2023, 11:20 AM
llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
1044

For constants, it's enough to just pass C, you can skip all the other arguments.

Remove extra args

goldstein.w.n marked an inline comment as done.May 1 2023, 11:39 AM
nikic accepted this revision.May 1 2023, 11:40 AM

LGTM

This revision is now accepted and ready to land.May 1 2023, 11:40 AM
This revision was landed with ongoing or failed builds.May 1 2023, 3:24 PM
This revision was automatically updated to reflect the committed changes.