Skip to content

[SILOptimizer] Add ‘llvm.cttz.*’ Intrinsic support to Constant Folding #23720

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 2, 2019
Merged

[SILOptimizer] Add ‘llvm.cttz.*’ Intrinsic support to Constant Folding #23720

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 41 additions & 25 deletions lib/SILOptimizer/Utils/ConstantFolding.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,44 @@ constantFoldBinaryWithOverflow(BuiltinInst *BI, BuiltinValueKind ID,
ResultsInError);
}

static SILValue countZeros(BuiltinInst *BI, llvm::Intrinsic::ID ID) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

assert(BI->getArguments().size() == 2 && "Ctlz should have 2 args.");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bad assert message. You are handling both ctlz and cttz.

OperandValueArrayRef Args = BI->getArguments();

// Fold for integer constant arguments.
auto *LHS = dyn_cast<IntegerLiteralInst>(Args[0]);
if (!LHS) {
return nullptr;
}
APInt LHSI = LHS->getValue();
unsigned LZ = 0;
// Check corner-case of source == zero
if (LHSI == 0) {
auto *RHS = dyn_cast<IntegerLiteralInst>(Args[1]);
if (!RHS || RHS->getValue() != 0) {
// Undefined
return nullptr;
}
LZ = LHSI.getBitWidth();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you do an early exit here?

} else {
switch (ID) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

default:
return nullptr;
case llvm::Intrinsic::ctlz: {
LZ = LHSI.countLeadingZeros();
break;
}
case llvm::Intrinsic::cttz: {
LZ = LHSI.countTrailingZeros();
break;
}
}
}
APInt LZAsAPInt = APInt(LHSI.getBitWidth(), LZ);
SILBuilderWithScope B(BI);
return B.createIntegerLiteral(BI->getLoc(), LHS->getType(), LZAsAPInt);
}

static SILValue constantFoldIntrinsic(BuiltinInst *BI, llvm::Intrinsic::ID ID,
Optional<bool> &ResultsInError) {
switch (ID) {
Expand All @@ -291,31 +329,9 @@ static SILValue constantFoldIntrinsic(BuiltinInst *BI, llvm::Intrinsic::ID ID,
return Op1;
}

case llvm::Intrinsic::ctlz: {
assert(BI->getArguments().size() == 2 && "Ctlz should have 2 args.");
OperandValueArrayRef Args = BI->getArguments();

// Fold for integer constant arguments.
auto *LHS = dyn_cast<IntegerLiteralInst>(Args[0]);
if (!LHS) {
return nullptr;
}
APInt LHSI = LHS->getValue();
unsigned LZ = 0;
// Check corner-case of source == zero
if (LHSI == 0) {
auto *RHS = dyn_cast<IntegerLiteralInst>(Args[1]);
if (!RHS || RHS->getValue() != 0) {
// Undefined
return nullptr;
}
LZ = LHSI.getBitWidth();
} else {
LZ = LHSI.countLeadingZeros();
}
APInt LZAsAPInt = APInt(LHSI.getBitWidth(), LZ);
SILBuilderWithScope B(BI);
return B.createIntegerLiteral(BI->getLoc(), LHS->getType(), LZAsAPInt);
case llvm::Intrinsic::ctlz:
case llvm::Intrinsic::cttz: {
return countZeros(BI, ID);
}

case llvm::Intrinsic::sadd_with_overflow:
Expand Down
30 changes: 30 additions & 0 deletions test/SILOptimizer/constant_propagation.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,36 @@ bb0:
// CHECK-NEXT: return [[RES]] : $Builtin.Int64
}

sil @count_trailing_zeros_corner_case : $@convention(thin) () -> Builtin.Int64 {
bb0:
%zero64 = integer_literal $Builtin.Int64, 0
%zero1 = integer_literal $Builtin.Int1, 0
%cttz = builtin "int_cttz_Int64"(%zero64 : $Builtin.Int64, %zero1 : $Builtin.Int1) : $Builtin.Int64
return %cttz : $Builtin.Int64

// CHECK-LABEL: sil @count_trailing_zeros_corner_case
// CHECK-NOT: integer_literal $Builtin.Int64, 0
// CHECK-NOT: integer_literal $Builtin.Int1, 0
// CHECK-NOT: builtin
// CHECK: [[RES:%.*]] = integer_literal $Builtin.Int64, 64
// CHECK-NEXT: return [[RES]] : $Builtin.Int64
}

sil @count_trailing_zeros : $@convention(thin) () -> Builtin.Int64 {
bb0:
%zero64 = integer_literal $Builtin.Int64, 2
%zero1 = integer_literal $Builtin.Int1, 0
%cttz = builtin "int_cttz_Int64"(%zero64 : $Builtin.Int64, %zero1 : $Builtin.Int1) : $Builtin.Int64
return %cttz : $Builtin.Int64

// CHECK-LABEL: sil @count_trailing_zeros
// CHECK-NOT: integer_literal $Builtin.Int64, 2
// CHECK-NOT: integer_literal $Builtin.Int1, 0
// CHECK-NOT: builtin
// CHECK: [[RES:%.*]] = integer_literal $Builtin.Int64, 1
// CHECK-NEXT: return [[RES]] : $Builtin.Int64
}

// Compute an expression using a chain of arithmetic with overflow instructions: 2 * (2 + 3) - 3
sil @fold_arithmetic_with_overflow : $@convention(thin) () -> Builtin.Int64 {
bb0:
Expand Down