From 68b7cba2b04a036dfb2e216cfafe70a83fbc7530 Mon Sep 17 00:00:00 2001 From: Henry Jiang Date: Fri, 11 Apr 2025 12:55:45 -0400 Subject: [PATCH] [LoopIdiom] Update strlen idiom body loop condition to be clean up by LoopDeletion (#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 https://github.com/llvm/llvm-project/issues/134736 --- .../Transforms/Scalar/LoopIdiomRecognize.cpp | 15 +++++++ .../Transforms/LoopIdiom/strlen-cleanup.ll | 43 +++++++++++++++++++ llvm/test/Transforms/LoopIdiom/strlen.ll | 12 +++--- llvm/test/Transforms/LoopIdiom/wcslen16.ll | 2 +- llvm/test/Transforms/LoopIdiom/wcslen32.ll | 2 +- 5 files changed, 66 insertions(+), 8 deletions(-) create mode 100644 llvm/test/Transforms/LoopIdiom/strlen-cleanup.ll diff --git a/llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp b/llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp index 4940dd8c47dd..eacc67cd5a47 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(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,17 @@ bool LoopIdiomRecognize::recognizeAndInsertStrLen() { // up by later passes for (PHINode *PN : Cleanup) RecursivelyDeleteDeadPHINode(PN); + + // LoopDeletion only delete invariant loops with known trip-count. We can + // update the condition so it will reliablely delete the invariant loop + assert(LoopTerm->getNumSuccessors() == 2 && + (LoopTerm->getSuccessor(0) == LoopBody || + LoopTerm->getSuccessor(1) == LoopBody) && + "loop body must have a successor that is it self"); + 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 000000000000..60b5ac6ce1d7 --- /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 c1141177c659..3b9d33ebdc74 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 8be51869f1f2..cd8a31f4af3d 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 2c7ceb4d187e..f1cddf9424ff 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