Skip to content

Commit 1605593

Browse files
committed
[SimplifyLibCalls] Fix memchr opt to use CreateLogicalAnd
This fixes a bug at LibCallSimplifier::optimizeMemChr which does the following transformation: ``` // memchr("\r\n", C, 2) != nullptr -> (1 << C & ((1 << '\r') | (1 << '\n'))) // != 0 // after bounds check. ``` As written above, a bounds check on C (whether it is less than integer bitwidth) is done before doing `1 << C` otherwise 1 << C will overflow. If the bounds check is false, the result of (1 << C & ...) must not be used at all, otherwise the result of shift (which is poison) will contaminate the whole results. A correct way to encode this is `select i1 (bounds check), (1 << C & ...), false` because select does not allow the unused operand to contaminate the result. However, this optimization was introducing `and (bounds check), (1 << C & ...)` which cannot do that. The bug was found from compilation of this C++ code: https://reviews.llvm.org/rG2fd3037ac615#1007197 Reviewed By: nikic Differential Revision: https://reviews.llvm.org/D104901
1 parent 5ccb742 commit 1605593

File tree

3 files changed

+5
-4
lines changed

3 files changed

+5
-4
lines changed

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -933,7 +933,8 @@ Value *LibCallSimplifier::optimizeMemChr(CallInst *CI, IRBuilderBase &B) {
933933

934934
// Finally merge both checks and cast to pointer type. The inttoptr
935935
// implicitly zexts the i1 to intptr type.
936-
return B.CreateIntToPtr(B.CreateAnd(Bounds, Bits, "memchr"), CI->getType());
936+
return B.CreateIntToPtr(B.CreateLogicalAnd(Bounds, Bits, "memchr"),
937+
CI->getType());
937938
}
938939

939940
// Check if all arguments are constants. If so, we can constant fold.

llvm/test/Transforms/InstCombine/memchr.ll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ define i1 @test11(i32 %C) {
137137
; CHECK-NEXT: [[TMP3:%.*]] = shl i16 1, [[TMP2]]
138138
; CHECK-NEXT: [[TMP4:%.*]] = and i16 [[TMP3]], 9216
139139
; CHECK-NEXT: [[MEMCHR_BITS:%.*]] = icmp ne i16 [[TMP4]], 0
140-
; CHECK-NEXT: [[MEMCHR:%.*]] = and i1 [[MEMCHR_BOUNDS]], [[MEMCHR_BITS]]
140+
; CHECK-NEXT: [[MEMCHR:%.*]] = select i1 [[MEMCHR_BOUNDS]], i1 [[MEMCHR_BITS]], i1 false
141141
; CHECK-NEXT: ret i1 [[MEMCHR]]
142142
;
143143
%dst = call i8* @memchr(i8* getelementptr inbounds ([3 x i8], [3 x i8]* @newlines, i64 0, i64 0), i32 %C, i32 2)
@@ -164,7 +164,7 @@ define i1 @test13(i32 %C) {
164164
; CHECK-NEXT: [[TMP2:%.*]] = shl i32 1, [[TMP1]]
165165
; CHECK-NEXT: [[TMP3:%.*]] = and i32 [[TMP2]], -2147483647
166166
; CHECK-NEXT: [[MEMCHR_BITS:%.*]] = icmp ne i32 [[TMP3]], 0
167-
; CHECK-NEXT: [[MEMCHR:%.*]] = and i1 [[MEMCHR_BOUNDS]], [[MEMCHR_BITS]]
167+
; CHECK-NEXT: [[MEMCHR:%.*]] = select i1 [[MEMCHR_BOUNDS]], i1 [[MEMCHR_BITS]], i1 false
168168
; CHECK-NEXT: ret i1 [[MEMCHR]]
169169
;
170170
%dst = call i8* @memchr(i8* getelementptr inbounds ([2 x i8], [2 x i8]* @single, i64 0, i64 0), i32 %C, i32 2)

llvm/test/Transforms/InstCombine/strchr-1.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ define i1 @test_simplify7(i32 %C) {
9595
; CHECK-NEXT: [[TMP3:%.*]] = shl i16 1, [[TMP2]]
9696
; CHECK-NEXT: [[TMP4:%.*]] = and i16 [[TMP3]], 9217
9797
; CHECK-NEXT: [[MEMCHR_BITS:%.*]] = icmp ne i16 [[TMP4]], 0
98-
; CHECK-NEXT: [[MEMCHR1:%.*]] = and i1 [[MEMCHR_BOUNDS]], [[MEMCHR_BITS]]
98+
; CHECK-NEXT: [[MEMCHR1:%.*]] = select i1 [[MEMCHR_BOUNDS]], i1 [[MEMCHR_BITS]], i1 false
9999
; CHECK-NEXT: ret i1 [[MEMCHR1]]
100100
;
101101

0 commit comments

Comments
 (0)