[LoopIdiom] Update strlen idiom body loop condition to be clean up by LoopDeletion #134906

mustartt · 2025-04-08T18:54:57Z

Fixes the case where subsequent passes were unable to find and delete the invariant loop left over by the strlen idiom conversion. Since loop-deletion only operate on computable loops, we can update the loop condition to something more easily picked up by loop-deletion

As pointed out in #134736

llvmbot · 2025-04-08T18:55:31Z

@llvm/pr-subscribers-llvm-transforms

Author: Henry Jiang (mustartt)

Changes

Fixes the case where subsequent passes were unable to find and delete the invariant loop left over by the strlen idiom conversion. Since loop-deletion only operate on computable loops, we can update the loop condition to something more easily picked up by loop-deletion

As pointed out in #134736

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

5 Files Affected:

(modified) llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp (+9)
(added) llvm/test/Transforms/LoopIdiom/strlen-cleanup.ll (+43)
(modified) llvm/test/Transforms/LoopIdiom/strlen.ll (+6-6)
(modified) llvm/test/Transforms/LoopIdiom/wcslen16.ll (+1-1)
(modified) llvm/test/Transforms/LoopIdiom/wcslen32.ll (+1-1)

diff --git a/llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp b/llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
index 4940dd8c47dde..d78a02381c68b 100644
--- a/llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
@@ -1745,7 +1745,11 @@ bool LoopIdiomRecognize::recognizeAndInsertStrLen() {
     return false;
 
   BasicBlock *Preheader = CurLoop->getLoopPreheader();
+  BasicBlock *LoopBody = *CurLoop->block_begin();
   BasicBlock *LoopExitBB = CurLoop->getExitBlock();
+  BranchInst *LoopTerm = dyn_cast<BranchInst>(LoopBody->getTerminator());
+  assert(Preheader && LoopBody && LoopExitBB && LoopTerm &&
+         "Should be verified to be valid by StrlenVerifier");
 
   if (Verifier.OpWidth == 8) {
     if (DisableLIRP::Strlen)
@@ -1804,6 +1808,11 @@ bool LoopIdiomRecognize::recognizeAndInsertStrLen() {
   // up by later passes
   for (PHINode *PN : Cleanup)
     RecursivelyDeleteDeadPHINode(PN);
+
+  ConstantInt *NewLoopCond = LoopTerm->getSuccessor(0) == LoopBody
+                                 ? Builder.getFalse()
+                                 : Builder.getTrue();
+  LoopTerm->setCondition(NewLoopCond);
   SE->forgetLoop(CurLoop);
 
   ++NumStrLen;
diff --git a/llvm/test/Transforms/LoopIdiom/strlen-cleanup.ll b/llvm/test/Transforms/LoopIdiom/strlen-cleanup.ll
new file mode 100644
index 0000000000000..60b5ac6ce1d78
--- /dev/null
+++ b/llvm/test/Transforms/LoopIdiom/strlen-cleanup.ll
@@ -0,0 +1,43 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt < %s -S -passes=loop-idiom,loop-deletion | FileCheck %s
+
+define [2 x i64] @convert_null_terminated_to_bounded_ptr(ptr noundef %p) {
+; CHECK-LABEL: define [2 x i64] @convert_null_terminated_to_bounded_ptr(
+; CHECK-SAME: ptr noundef [[P:%.*]]) {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    [[WCSLEN:%.*]] = call i64 @wcslen(ptr [[P]])
+; CHECK-NEXT:    br label %[[TERMINATED_BY_LOOP_END:.*]]
+; CHECK:       [[TERMINATED_BY_LOOP_END]]:
+; CHECK-NEXT:    [[TMP0:%.*]] = getelementptr inbounds i32, ptr [[P]], i64 [[WCSLEN]]
+; CHECK-NEXT:    [[TERMINATED_BY_UPPER:%.*]] = getelementptr i8, ptr [[TMP0]], i64 4
+; CHECK-NEXT:    [[TMP1:%.*]] = ptrtoint ptr [[P]] to i64
+; CHECK-NEXT:    [[DOTFCA_0_INSERT:%.*]] = insertvalue [2 x i64] poison, i64 [[TMP1]], 0
+; CHECK-NEXT:    [[TMP2:%.*]] = ptrtoint ptr [[TERMINATED_BY_UPPER]] to i64
+; CHECK-NEXT:    [[DOTFCA_1_INSERT:%.*]] = insertvalue [2 x i64] [[DOTFCA_0_INSERT]], i64 [[TMP2]], 1
+; CHECK-NEXT:    ret [2 x i64] [[DOTFCA_1_INSERT]]
+;
+entry:
+  br label %terminated_by.loop_cond
+
+terminated_by.loop_cond:                          ; preds = %terminated_by.loop_cond, %entry
+  %terminated_by.len.0 = phi i64 [ 0, %entry ], [ %terminated_by.new_len, %terminated_by.loop_cond ]
+  %0 = getelementptr inbounds i32, ptr %p, i64 %terminated_by.len.0
+  %terminated_by.elem = load i32, ptr %0, align 4
+  %terminted_by.check_terminator = icmp eq i32 %terminated_by.elem, 0
+  %terminated_by.new_len = add i64 %terminated_by.len.0, 1
+  br i1 %terminted_by.check_terminator, label %terminated_by.loop_end, label %terminated_by.loop_cond
+
+terminated_by.loop_end:                           ; preds = %terminated_by.loop_cond
+  %1 = getelementptr inbounds i32, ptr %p, i64 %terminated_by.len.0
+  %terminated_by.upper = getelementptr i8, ptr %1, i64 4
+  %2 = ptrtoint ptr %p to i64
+  %.fca.0.insert = insertvalue [2 x i64] poison, i64 %2, 0
+  %3 = ptrtoint ptr %terminated_by.upper to i64
+  %.fca.1.insert = insertvalue [2 x i64] %.fca.0.insert, i64 %3, 1
+  ret [2 x i64] %.fca.1.insert
+}
+
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 1, !"wchar_size", i32 4}
+
diff --git a/llvm/test/Transforms/LoopIdiom/strlen.ll b/llvm/test/Transforms/LoopIdiom/strlen.ll
index c1141177c659f..3b9d33ebdc74e 100644
--- a/llvm/test/Transforms/LoopIdiom/strlen.ll
+++ b/llvm/test/Transforms/LoopIdiom/strlen.ll
@@ -24,7 +24,7 @@ define i64 @valid_basic_strlen(ptr %str) {
 ; CHECK-NEXT:    [[TMP0:%.*]] = load i8, ptr [[STR_ADDR_0]], align 1
 ; CHECK-NEXT:    [[CMP_NOT:%.*]] = icmp eq i8 [[TMP0]], 0
 ; CHECK-NEXT:    [[INCDEC_PTR]] = getelementptr i8, ptr [[STR_ADDR_0]], i64 1
-; CHECK-NEXT:    br i1 [[CMP_NOT]], label %[[WHILE_END:.*]], label %[[WHILE_COND]]
+; CHECK-NEXT:    br i1 true, label %[[WHILE_END:.*]], label %[[WHILE_COND]]
 ; CHECK:       [[WHILE_END]]:
 ; CHECK-NEXT:    [[SUB_PTR_LHS_CAST:%.*]] = ptrtoint ptr [[SCEVGEP]] to i64
 ; CHECK-NEXT:    [[SUB_PTR_RHS_CAST:%.*]] = ptrtoint ptr [[STR]] to i64
@@ -74,7 +74,7 @@ define i32 @valid_basic_strlen_rotated(ptr %str) {
 ; CHECK-NEXT:    [[INCDEC_PTR]] = getelementptr inbounds nuw i8, ptr [[STR_ADDR_0]], i64 1
 ; CHECK-NEXT:    [[TMP2:%.*]] = load i8, ptr [[INCDEC_PTR]], align 1
 ; CHECK-NEXT:    [[TOBOOL1_NOT:%.*]] = icmp eq i8 [[TMP2]], 0
-; CHECK-NEXT:    br i1 [[TOBOOL1_NOT]], label %[[DO_END:.*]], label %[[DO_BODY]]
+; CHECK-NEXT:    br i1 true, label %[[DO_END:.*]], label %[[DO_BODY]]
 ; CHECK:       [[DO_END]]:
 ; CHECK-NEXT:    [[SUB_PTR_LHS_CAST:%.*]] = ptrtoint ptr [[SCEVGEP1]] to i64
 ; CHECK-NEXT:    [[SUB_PTR_RHS_CAST:%.*]] = ptrtoint ptr [[STR]] to i64
@@ -163,7 +163,7 @@ define dso_local void @valid_strlen_with_aux_indvar(ptr noundef %str, ptr nounde
 ; CHECK-NEXT:    [[INCDEC_PTR2]] = getelementptr inbounds nuw i8, ptr [[FOO_ADDR_011]], i64 1
 ; CHECK-NEXT:    [[TMP10:%.*]] = load i8, ptr [[INCDEC_PTR]], align 1
 ; CHECK-NEXT:    [[TOBOOL_NOT:%.*]] = icmp eq i8 [[TMP10]], 0
-; CHECK-NEXT:    br i1 [[TOBOOL_NOT]], label %[[WHILE_END_LOOPEXIT:.*]], label %[[WHILE_BODY]]
+; CHECK-NEXT:    br i1 true, label %[[WHILE_END_LOOPEXIT:.*]], label %[[WHILE_BODY]]
 ; CHECK:       [[WHILE_END_LOOPEXIT]]:
 ; CHECK-NEXT:    br label %[[WHILE_END]]
 ; CHECK:       [[WHILE_END]]:
@@ -232,7 +232,7 @@ define i32 @valid_strlen_index(ptr %str) {
 ; CHECK-NEXT:    [[TMP0:%.*]] = load i8, ptr [[ARRAYIDX]], align 1
 ; CHECK-NEXT:    [[TOBOOL_NOT:%.*]] = icmp eq i8 [[TMP0]], 0
 ; CHECK-NEXT:    [[INDVARS_IV_NEXT]] = add nuw nsw i64 [[INDVARS_IV]], 1
-; CHECK-NEXT:    br i1 [[TOBOOL_NOT]], label %[[WHILE_END:.*]], label %[[WHILE_COND]]
+; CHECK-NEXT:    br i1 true, label %[[WHILE_END:.*]], label %[[WHILE_COND]]
 ; CHECK:       [[WHILE_END]]:
 ; CHECK-NEXT:    [[TMP1:%.*]] = trunc nuw nsw i64 [[STRLEN]] to i32
 ; CHECK-NEXT:    ret i32 [[TMP1]]
@@ -290,7 +290,7 @@ define dso_local void @valid_strlen_offset(ptr noundef %str) local_unnamed_addr
 ; CHECK-NEXT:    [[TMP4:%.*]] = load i8, ptr [[STR_ADDR_0]], align 1
 ; CHECK-NEXT:    [[TOBOOL_NOT:%.*]] = icmp eq i8 [[TMP4]], 0
 ; CHECK-NEXT:    [[INCDEC_PTR14]] = getelementptr inbounds nuw i8, ptr [[STR_ADDR_0]], i64 1
-; CHECK-NEXT:    br i1 [[TOBOOL_NOT]], label %[[WHILE_END:.*]], label %[[WHILE_COND]]
+; CHECK-NEXT:    br i1 true, label %[[WHILE_END:.*]], label %[[WHILE_COND]]
 ; CHECK:       [[WHILE_END]]:
 ; CHECK-NEXT:    tail call void @use(ptr noundef nonnull [[SCEVGEP]])
 ; CHECK-NEXT:    br label %[[RETURN]]
@@ -377,7 +377,7 @@ define void @valid_nested_idiom(ptr %strs, i32 %n) {
 ; CHECK-NEXT:    [[INC]] = add nuw nsw i32 [[COUNT_08]], 1
 ; CHECK-NEXT:    [[TMP4:%.*]] = load i8, ptr [[INCDEC_PTR]], align 1
 ; CHECK-NEXT:    [[TOBOOL_NOT:%.*]] = icmp eq i8 [[TMP4]], 0
-; CHECK-NEXT:    br i1 [[TOBOOL_NOT]], label %[[WHILE_END_LOOPEXIT:.*]], label %[[WHILE_BODY]]
+; CHECK-NEXT:    br i1 true, label %[[WHILE_END_LOOPEXIT:.*]], label %[[WHILE_BODY]]
 ; CHECK:       [[WHILE_END_LOOPEXIT]]:
 ; CHECK-NEXT:    br label %[[WHILE_END]]
 ; CHECK:       [[WHILE_END]]:
diff --git a/llvm/test/Transforms/LoopIdiom/wcslen16.ll b/llvm/test/Transforms/LoopIdiom/wcslen16.ll
index 8be51869f1f2e..cd8a31f4af3dd 100644
--- a/llvm/test/Transforms/LoopIdiom/wcslen16.ll
+++ b/llvm/test/Transforms/LoopIdiom/wcslen16.ll
@@ -26,7 +26,7 @@ define i64 @valid_strlen16(ptr %src) {
 ; CHECK-NEXT:    [[CURR_0]] = getelementptr inbounds i8, ptr [[SRC_PN]], i64 2
 ; CHECK-NEXT:    [[TMP3:%.*]] = load i16, ptr [[CURR_0]], align 2
 ; CHECK-NEXT:    [[TOBOOL_NOT:%.*]] = icmp eq i16 [[TMP3]], 0
-; CHECK-NEXT:    br i1 [[TOBOOL_NOT]], label %[[WHILE_END:.*]], label %[[WHILE_COND]]
+; CHECK-NEXT:    br i1 true, label %[[WHILE_END:.*]], label %[[WHILE_COND]]
 ; CHECK:       [[WHILE_END]]:
 ; CHECK-NEXT:    [[SUB_PTR_LHS_CAST:%.*]] = ptrtoint ptr [[END]] to i64
 ; CHECK-NEXT:    [[SUB_PTR_RHS_CAST:%.*]] = ptrtoint ptr [[SRC]] to i64
diff --git a/llvm/test/Transforms/LoopIdiom/wcslen32.ll b/llvm/test/Transforms/LoopIdiom/wcslen32.ll
index 2c7ceb4d187e6..f1cddf9424ffd 100644
--- a/llvm/test/Transforms/LoopIdiom/wcslen32.ll
+++ b/llvm/test/Transforms/LoopIdiom/wcslen32.ll
@@ -26,7 +26,7 @@ define i64 @valid_wcslen32(ptr %src) {
 ; CHECK-NEXT:    [[CURR_0]] = getelementptr inbounds i8, ptr [[SRC_PN]], i64 4
 ; CHECK-NEXT:    [[TMP3:%.*]] = load i32, ptr [[CURR_0]], align 4
 ; CHECK-NEXT:    [[TOBOOL_NOT:%.*]] = icmp eq i32 [[TMP3]], 0
-; CHECK-NEXT:    br i1 [[TOBOOL_NOT]], label %[[WHILE_END:.*]], label %[[WHILE_COND]]
+; CHECK-NEXT:    br i1 true, label %[[WHILE_END:.*]], label %[[WHILE_COND]]
 ; CHECK:       [[WHILE_END]]:
 ; CHECK-NEXT:    [[SUB_PTR_LHS_CAST:%.*]] = ptrtoint ptr [[END]] to i64
 ; CHECK-NEXT:    [[SUB_PTR_RHS_CAST:%.*]] = ptrtoint ptr [[SRC]] to i64

Meinersbur

LGTM

Meinersbur · 2025-04-09T12:01:03Z

llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp

@@ -1804,6 +1808,11 @@ bool LoopIdiomRecognize::recognizeAndInsertStrLen() {
  // up by later passes
  for (PHINode *PN : Cleanup)
    RecursivelyDeleteDeadPHINode(PN);
+
+  ConstantInt *NewLoopCond = LoopTerm->getSuccessor(0) == LoopBody


Consider adding a comment explaining that this intends to make the old code unreachable/dead.

Add assert that at least one of the successors is LoopBody?

Thanks for taking a look. I added the asserts and comments.

I will take a look to see if there's any further testing required.

llvm-ci · 2025-04-11T17:29:15Z

LLVM Buildbot has detected a new failure on builder clang-ppc64le-linux-test-suite running on ppc64le-clang-test-suite while building llvm at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/95/builds/11892

Here is the relevant piece of the build log for the reference

Step 6 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'LLVM-Unit :: Support/./SupportTests/943/1509' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-test-suite/clang-ppc64le-test-suite/build/unittests/Support/./SupportTests-LLVM-Unit-3440064-943-1509.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=1509 GTEST_SHARD_INDEX=943 /home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-test-suite/clang-ppc64le-test-suite/build/unittests/Support/./SupportTests
--

Script:
--
/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-test-suite/clang-ppc64le-test-suite/build/unittests/Support/./SupportTests --gtest_filter=ProgramEnvTest.TestLockFile
--
/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-test-suite/clang-ppc64le-test-suite/llvm-project/llvm/unittests/Support/ProgramTest.cpp:570: Failure
Value of: Error.empty()
  Actual: false
Expected: true


/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-test-suite/clang-ppc64le-test-suite/llvm-project/llvm/unittests/Support/ProgramTest.cpp:570
Value of: Error.empty()
  Actual: false
Expected: true



********************

… LoopDeletion (llvm#134906) Fixes the case where subsequent passes were unable to find and delete the invariant loop left over by the strlen idiom conversion. Since `loop-deletion` only operate on computable loops, we can update the loop condition to something more easily picked up by `loop-deletion` As pointed out in llvm#134736

Update loop cond for cleanup by loop-delete

c4746a9

mustartt linked an issue Apr 8, 2025 that may be closed by this pull request

#108985 optimization doesn't remove strlen like loop body in some cases #134736

Closed

llvmbot added the llvm:transforms label Apr 8, 2025

delcypher requested a review from fhahn April 8, 2025 21:56

mustartt requested review from Meinersbur and maryammo April 8, 2025 21:57

Meinersbur approved these changes Apr 9, 2025

View reviewed changes

Address review comments

6918d2c

Meinersbur approved these changes Apr 11, 2025

View reviewed changes

mustartt merged commit 68b7cba into llvm:main Apr 11, 2025
11 checks passed

mustartt deleted the strlen-simplfy-delete branch April 11, 2025 16:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[LoopIdiom] Update strlen idiom body loop condition to be clean up by LoopDeletion #134906

[LoopIdiom] Update strlen idiom body loop condition to be clean up by LoopDeletion #134906

Uh oh!

mustartt commented Apr 8, 2025

Uh oh!

llvmbot commented Apr 8, 2025

Uh oh!

Meinersbur left a comment

Uh oh!

Meinersbur Apr 9, 2025

Uh oh!

mustartt Apr 9, 2025

Uh oh!

Uh oh!

llvm-ci commented Apr 11, 2025

Uh oh!

Uh oh!

[LoopIdiom] Update strlen idiom body loop condition to be clean up by LoopDeletion #134906

[LoopIdiom] Update strlen idiom body loop condition to be clean up by LoopDeletion #134906

Uh oh!

Conversation

mustartt commented Apr 8, 2025

Uh oh!

llvmbot commented Apr 8, 2025

Uh oh!

Meinersbur left a comment

Choose a reason for hiding this comment

Uh oh!

Meinersbur Apr 9, 2025

Choose a reason for hiding this comment

Uh oh!

mustartt Apr 9, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

llvm-ci commented Apr 11, 2025

Uh oh!

Uh oh!