[PATCH] D104901: [SimplifyLibCalls] Fix memchr opt to use CreateLogicalAnd

[Juneyoung Lee via Phabricator via llvm-commits]

aqjune created this revision.
aqjune added reviewers: bjope, nikic, efriedma, xbolva00.
Herald added a subscriber: hiraditya.
aqjune requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104901

Files:
  llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
  llvm/test/Transforms/InstCombine/memchr.ll
  llvm/test/Transforms/InstCombine/strchr-1.ll


Index: llvm/test/Transforms/InstCombine/strchr-1.ll
===================================================================
--- llvm/test/Transforms/InstCombine/strchr-1.ll
+++ llvm/test/Transforms/InstCombine/strchr-1.ll
@@ -95,7 +95,7 @@
 ; CHECK-NEXT:    [[TMP3:%.*]] = shl i16 1, [[TMP2]]
 ; CHECK-NEXT:    [[TMP4:%.*]] = and i16 [[TMP3]], 9217
 ; CHECK-NEXT:    [[MEMCHR_BITS:%.*]] = icmp ne i16 [[TMP4]], 0
-; CHECK-NEXT:    [[MEMCHR1:%.*]] = and i1 [[MEMCHR_BOUNDS]], [[MEMCHR_BITS]]
+; CHECK-NEXT:    [[MEMCHR1:%.*]] = select i1 [[MEMCHR_BOUNDS]], i1 [[MEMCHR_BITS]], i1 false
 ; CHECK-NEXT:    ret i1 [[MEMCHR1]]
 ;
 
Index: llvm/test/Transforms/InstCombine/memchr.ll
===================================================================
--- llvm/test/Transforms/InstCombine/memchr.ll
+++ llvm/test/Transforms/InstCombine/memchr.ll
@@ -137,7 +137,7 @@
 ; CHECK-NEXT:    [[TMP3:%.*]] = shl i16 1, [[TMP2]]
 ; CHECK-NEXT:    [[TMP4:%.*]] = and i16 [[TMP3]], 9216
 ; CHECK-NEXT:    [[MEMCHR_BITS:%.*]] = icmp ne i16 [[TMP4]], 0
-; CHECK-NEXT:    [[MEMCHR:%.*]] = and i1 [[MEMCHR_BOUNDS]], [[MEMCHR_BITS]]
+; CHECK-NEXT:    [[MEMCHR:%.*]] = select i1 [[MEMCHR_BOUNDS]], i1 [[MEMCHR_BITS]], i1 false
 ; CHECK-NEXT:    ret i1 [[MEMCHR]]
 ;
   %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 @@
 ; CHECK-NEXT:    [[TMP2:%.*]] = shl i32 1, [[TMP1]]
 ; CHECK-NEXT:    [[TMP3:%.*]] = and i32 [[TMP2]], -2147483647
 ; CHECK-NEXT:    [[MEMCHR_BITS:%.*]] = icmp ne i32 [[TMP3]], 0
-; CHECK-NEXT:    [[MEMCHR:%.*]] = and i1 [[MEMCHR_BOUNDS]], [[MEMCHR_BITS]]
+; CHECK-NEXT:    [[MEMCHR:%.*]] = select i1 [[MEMCHR_BOUNDS]], i1 [[MEMCHR_BITS]], i1 false
 ; CHECK-NEXT:    ret i1 [[MEMCHR]]
 ;
   %dst = call i8* @memchr(i8* getelementptr inbounds ([2 x i8], [2 x i8]* @single, i64 0, i64 0), i32 %C, i32 2)
Index: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
===================================================================
--- llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
+++ llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
@@ -933,7 +933,7 @@
 
     // Finally merge both checks and cast to pointer type. The inttoptr
     // implicitly zexts the i1 to intptr type.
-    return B.CreateIntToPtr(B.CreateAnd(Bounds, Bits, "memchr"), CI->getType());
+    return B.CreateIntToPtr(B.CreateLogicalAnd(Bounds, Bits, "memchr"), CI->getType());
   }
 
   // Check if all arguments are constants.  If so, we can constant fold.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D104901.354449.patch
Type: text/x-patch
Size: 2495 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210625/ae599e98/attachment-0001.bin>