This is an archive of the discontinued LLVM Phabricator instance.

[InstCombine] Allow undef vectors when foldSelectToCopysign
ClosedPublic

Authored by Chenbing.Zheng on May 16 2022, 1:32 AM.

Details

Summary

In function foldSelectToCopysign, we can allow undefs when
matching vectors.

Diff Detail

Event Timeline

Chenbing.Zheng created this revision.May 16 2022, 1:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 16 2022, 1:32 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
Chenbing.Zheng requested review of this revision.May 16 2022, 1:32 AM

This does not look correct.

In the example test, if both elements of X are negative, we should return <-42.0, -42.0>. But after this transform, we will return <-42.0, undef>?

This does not look correct.

In the example test, if both elements of X are negative, we should return <-42.0, -42.0>. But after this transform, we will return <-42.0, undef>?

Sorry - I missed the invert / fneg. That should be:

In the example test, if both elements of X are positive, we should return <-42.0, -42.0>. But after this transform, we will return <-42.0, undef>?

address comment

spatel added inline comments.May 17 2022, 1:19 PM
llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
2307–2308

This might be more efficient and easier to read:

Value *MagArg = ConstantFP::get(SelType, abs(*TC));
llvm/test/Transforms/InstCombine/select.ll
1703–1704

Test coverage would be better if you vary the predicate (for example ule). Also, swap the positive and negative constants on the next test?

address comments

spatel accepted this revision.May 18 2022, 6:47 AM

LGTM - it might also be possible to allow undefs in the compare constant, but we would have to test that carefully with Alive2 to be sure.

This revision is now accepted and ready to land.May 18 2022, 6:47 AM
This revision was landed with ongoing or failed builds.May 18 2022, 8:15 PM
This revision was automatically updated to reflect the committed changes.