[AggressiveInstCombine] Fix strncmp inlining

[FLZ101]

Fix the issue that char constants are converted to uint64_t in the wrong way when doing the inlining.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Franklin Zhang (FLZ101)

Changes

• fix the issue that char constants are converted to uint64_t in the wrong way when doing the inlining

---

Full diff: https://github.com/llvm/llvm-project/pull/91204.diff

2 Files Affected:

• (modified) llvm/lib/Transforms/AggressiveInstCombine/AggressiveInstCombine.cpp (+2-1)
• (modified) llvm/test/Transforms/AggressiveInstCombine/strncmp-1.ll (+38)

diff --git a/llvm/lib/Transforms/AggressiveInstCombine/AggressiveInstCombine.cpp b/llvm/lib/Transforms/AggressiveInstCombine/AggressiveInstCombine.cpp
index 39eca4f41ec57f..c7e84a009221f6 100644
--- a/llvm/lib/Transforms/AggressiveInstCombine/AggressiveInstCombine.cpp
+++ b/llvm/lib/Transforms/AggressiveInstCombine/AggressiveInstCombine.cpp
@@ -1073,7 +1073,8 @@ void StrNCmpInliner::inlineCompare(Value *LHS, StringRef RHS, uint64_t N,
         B.CreateZExt(B.CreateLoad(B.getInt8Ty(),
                                   B.CreateInBoundsPtrAdd(Base, B.getInt64(i))),
                      CI->getType());
-    Value *VR = ConstantInt::get(CI->getType(), RHS[i]);
+    Value *VR =
+        ConstantInt::get(CI->getType(), static_cast<unsigned char>(RHS[i]));
     Value *Sub = Swapped ? B.CreateSub(VR, VL) : B.CreateSub(VL, VR);
     if (i < N - 1)
       B.CreateCondBr(B.CreateICmpNE(Sub, ConstantInt::get(CI->getType(), 0)),
diff --git a/llvm/test/Transforms/AggressiveInstCombine/strncmp-1.ll b/llvm/test/Transforms/AggressiveInstCombine/strncmp-1.ll
index f3f88663672fe2..21c82d82451da0 100644
--- a/llvm/test/Transforms/AggressiveInstCombine/strncmp-1.ll
+++ b/llvm/test/Transforms/AggressiveInstCombine/strncmp-1.ll
@@ -8,6 +8,7 @@ declare i32 @strcmp(ptr nocapture, ptr nocapture)
 
 @s2 = constant [2 x i8] c"a\00"
 @s3 = constant [3 x i8] c"ab\00"
+@s3ff = constant [3 x i8] c"\FE\FF\00"
 
 define i1 @test_strncmp_1(ptr %s) {
 ; CHECK-LABEL: define i1 @test_strncmp_1(
@@ -214,3 +215,40 @@ entry:
   %cmp = icmp sle i32 %call, 0
   ret i1 %cmp
 }
+
+define i1 @test_strcmp_4(ptr %s) {
+; CHECK-LABEL: define i1 @test_strcmp_4(
+; CHECK-SAME: ptr [[S:%.*]]) {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[SUB_0:%.*]]
+; CHECK:       sub_0:
+; CHECK-NEXT:    [[TMP0:%.*]] = load i8, ptr [[S]], align 1
+; CHECK-NEXT:    [[TMP1:%.*]] = zext i8 [[TMP0]] to i32
+; CHECK-NEXT:    [[TMP2:%.*]] = sub i32 254, [[TMP1]]
+; CHECK-NEXT:    [[TMP3:%.*]] = icmp ne i32 [[TMP2]], 0
+; CHECK-NEXT:    br i1 [[TMP3]], label [[NE:%.*]], label [[SUB_1:%.*]]
+; CHECK:       sub_1:
+; CHECK-NEXT:    [[TMP4:%.*]] = getelementptr inbounds i8, ptr [[S]], i64 1
+; CHECK-NEXT:    [[TMP5:%.*]] = load i8, ptr [[TMP4]], align 1
+; CHECK-NEXT:    [[TMP6:%.*]] = zext i8 [[TMP5]] to i32
+; CHECK-NEXT:    [[TMP7:%.*]] = sub i32 255, [[TMP6]]
+; CHECK-NEXT:    [[TMP8:%.*]] = icmp ne i32 [[TMP7]], 0
+; CHECK-NEXT:    br i1 [[TMP8]], label [[NE]], label [[SUB_2:%.*]]
+; CHECK:       sub_2:
+; CHECK-NEXT:    [[TMP9:%.*]] = getelementptr inbounds i8, ptr [[S]], i64 2
+; CHECK-NEXT:    [[TMP10:%.*]] = load i8, ptr [[TMP9]], align 1
+; CHECK-NEXT:    [[TMP11:%.*]] = zext i8 [[TMP10]] to i32
+; CHECK-NEXT:    [[TMP12:%.*]] = sub i32 0, [[TMP11]]
+; CHECK-NEXT:    br label [[NE]]
+; CHECK:       ne:
+; CHECK-NEXT:    [[TMP13:%.*]] = phi i32 [ [[TMP2]], [[SUB_0]] ], [ [[TMP7]], [[SUB_1]] ], [ [[TMP12]], [[SUB_2]] ]
+; CHECK-NEXT:    br label [[ENTRY_TAIL:%.*]]
+; CHECK:       entry.tail:
+; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i32 [[TMP13]], 0
+; CHECK-NEXT:    ret i1 [[CMP]]
+;
+entry:
+  %call = tail call i32 @strcmp(ptr nonnull dereferenceable(3) @s3ff, ptr nonnull dereferenceable(1) %s)
+  %cmp = icmp eq i32 %call, 0
+  ret i1 %cmp
+}

[nikic]

LGTM

[dtcxzyw]

LGTM. It should be interpreted as unsigned char.
See https://en.cppreference.com/w/c/string/byte/strcmp