This is an archive of the discontinued LLVM Phabricator instance.

[InstSimplify] teach SimplifySelectInst() to fold more vector selects
ClosedPublic

Authored by haicheng on Sep 28 2017, 9:55 AM.

Details

Summary

Call ConstantExpr::getSelect() to fold cases like below

select <2 x i1><i1 true, i1 false>, <2 x i8> <i8 0, i8 1>, <2 x i8> <i8 2, i8 3>

All operands are constants and the condition has mixed true and false conditions

Diff Detail

Repository
rL LLVM

Event Timeline

haicheng created this revision.Sep 28 2017, 9:55 AM
spatel added inline comments.
lib/Analysis/InstructionSimplify.cpp
3585 ↗(On Diff #117012)

I think we should use ConstantFoldSelectInstruction() here (to be more efficient?). That's the interface for other opcodes from what I can see in other parts of the file.

haicheng added inline comments.Sep 29 2017, 12:34 PM
lib/Analysis/InstructionSimplify.cpp
3585 ↗(On Diff #117012)

I think I cannot use ConstantFoldSelectInstruction() which is declared in IR/ConstantFold.h rather than llvm/Analysis/ConstantFolding.h. From the comments in IR/ConstantFold.h, ConstantFoldSelectInstruction() is used internal to ConstantExpr::get* methods.

Many ConstantFoldxxx methods used in this file are declared in llvm/Analysis/ConstantFolding.h. I also find several places in this file calling ConstantExpr::get* methods.

spatel added inline comments.Sep 29 2017, 1:54 PM
lib/Analysis/InstructionSimplify.cpp
3585 ↗(On Diff #117012)

Why not just add a line to ConstantFolding.h with the definition of ConstantFoldSelectInstruction()?

I'm looking at:
SimplifyExtractValueInst
SimplifyInsertValueInst
SimplifyExtractElementInst
SimplifyCastInst
etc.

They are all using this pattern, so why should SimplifySelectInst() be different? I think your way is fine too, but if we do that, then we should change everything else to be consistent.

haicheng updated this revision to Diff 117396.Oct 2 2017, 11:17 AM

Switch to use ConstantFoldSelectInstruction().

haicheng marked an inline comment as done.Oct 2 2017, 11:17 AM
spatel accepted this revision.Oct 2 2017, 12:45 PM

LGTM.

This revision is now accepted and ready to land.Oct 2 2017, 12:45 PM
This revision was automatically updated to reflect the committed changes.