[SILOptimizer] Add ‘llvm.cttz.*’ Intrinsic support to Constant Folding #23720
Conversation
rdar://problem/29522851
| @swift-ci Please test and merge |
| // Undefined | ||
| return nullptr; | ||
| } | ||
| LZ = LHSI.getBitWidth(); |
There was a problem hiding this comment.
Can you do an early exit here?
gottesmm left a comment
gottesmm left a comment There was a problem hiding this comment.
Post commit review. Please fix.
| } | ||
| LZ = LHSI.getBitWidth(); | ||
| } else { | ||
| switch (ID) { |
There was a problem hiding this comment.
My suggestion: Use an inline constructor:
unsigned LZ = [&] {
...
}();
Also. Do you really think we are going to support more types of intrinsics here? Why not just put in an assert or something like that.
| ResultsInError); | ||
| } | ||
| | ||
| static SILValue countZeros(BuiltinInst *BI, llvm::Intrinsic::ID ID) { |
There was a problem hiding this comment.
I find this name means nothing and this needs a comment. My suggestion:
/// Constant fold a cttz or ctlz builtin inst of an integer literal. If \p countLeadingZeros is set to true, then we /// assume \p bi must be ctlz. If false, \p bi must be cttz. /// /// NOTE: We assert that \p bi is either cttz or ctlz. static SILValue constantFoldCountLeadingOrTrialingZeroIntrinsic(BuiltinInst *bi, bool countLeadingZeros) { ... } /// Count either the leading or trailing zeros.
Also, do you really need the intrinsic id here? Couldn't you just pass in a bool stating which of the two you are handling?
| } | ||
| | ||
| static SILValue countZeros(BuiltinInst *BI, llvm::Intrinsic::ID ID) { | ||
| assert(BI->getArguments().size() == 2 && "Ctlz should have 2 args."); |
There was a problem hiding this comment.
This is a bad assert message. You are handling both ctlz and cttz.
| Another thing. You can make all of this much simpler by using PatternMatch.h. |
3 participants
rdar://problem/29522851