[SimplifyCFG] Avoid threading for loop headers #151142

ArneStenkrona2 · 2025-07-29T13:00:29Z

Updates SimplifyCFG to avoid jump threading through loop headers if -keep-loops is requested. Canonical loop form requires a loop header that dominates all blocks in the loop. If we thread through a header, we risk breaking its domination of the loop. This change avoids this issue by conservatively avoiding threading through headers entirely.

Fixes: #151144

github-actions · 2025-07-29T13:00:51Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2025-07-29T13:01:21Z

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-backend-arm

Author: Arne Stenkrona (ArneStenkrona2)

Changes

Updates SimplifyCFG to avoid jump threading through loop headers if -keep-loops is requested. Canonical loop form requires a loop header that dominates all blocks in the loop. If we thread through a header, we risk breaking its domination of the loop. This change avoids this issue by conservatively avoiding threading through headers entirely.

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

7 Files Affected:

(modified) llvm/lib/Transforms/Utils/SimplifyCFG.cpp (+8-2)
(modified) llvm/test/CodeGen/ARM/2013-05-05-IfConvertBug.ll (+3-3)
(modified) llvm/test/Transforms/SimplifyCFG/2008-07-13-InfLoopMiscompile.ll (+1-2)
(added) llvm/test/Transforms/SimplifyCFG/2025-07-29-non-canoncial-loop.ll (+37)
(modified) llvm/test/Transforms/SimplifyCFG/branch-phi-thread.ll (+1-1)
(modified) llvm/test/Transforms/SimplifyCFG/jump-threading.ll (+1-1)
(modified) llvm/test/Transforms/SimplifyCFG/two-entry-phi-return.ll (+1-1)

diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 94b0ab892f2dd..d8385beb349d6 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -8030,8 +8030,14 @@ bool SimplifyCFGOpt::simplifyCondBranch(BranchInst *BI, IRBuilder<> &Builder) {
   // If this is a branch on something for which we know the constant value in
   // predecessors (e.g. a phi node in the current block), thread control
   // through this block.
-  if (foldCondBranchOnValueKnownInPredecessor(BI, DTU, DL, Options.AC))
-    return requestResimplify();
+  // Note: If BB is a loop header then there is a risk that threading introduces
+  // a non-canonical loop by moving a back edge. So we avoid this optimization
+  // for loop headers if NeedCanonicalLoop is set.
+  bool InHeader = !LoopHeaders.empty() && is_contained(LoopHeaders, BB);
+  bool AvoidThreading = Options.NeedCanonicalLoop && InHeader;
+  if (!AvoidThreading)
+    if (foldCondBranchOnValueKnownInPredecessor(BI, DTU, DL, Options.AC))
+      return requestResimplify();
 
   // Scan predecessor blocks for conditional branches.
   for (BasicBlock *Pred : predecessors(BB))
diff --git a/llvm/test/CodeGen/ARM/2013-05-05-IfConvertBug.ll b/llvm/test/CodeGen/ARM/2013-05-05-IfConvertBug.ll
index 344bb15d2a8b8..8f798fac06f54 100644
--- a/llvm/test/CodeGen/ARM/2013-05-05-IfConvertBug.ll
+++ b/llvm/test/CodeGen/ARM/2013-05-05-IfConvertBug.ll
@@ -1,7 +1,7 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc < %s -mtriple=thumbv7-apple-ios -mcpu=cortex-a8 | FileCheck %s
-; RUN: llc < %s -mtriple=thumbv8 | FileCheck -check-prefix=CHECK-V8 %s
-; RUN: llc < %s -mtriple=thumbv7 -arm-restrict-it | FileCheck -check-prefix=CHECK-RESTRICT-IT %s
+; RUN: llc -keep-loops="false" < %s -mtriple=thumbv7-apple-ios -mcpu=cortex-a8 | FileCheck %s
+; RUN: llc -keep-loops="false" < %s -mtriple=thumbv8 | FileCheck -check-prefix=CHECK-V8 %s
+; RUN: llc -keep-loops="false" < %s -mtriple=thumbv7 -arm-restrict-it | FileCheck -check-prefix=CHECK-RESTRICT-IT %s
 
 define i32 @t1(i32 %a, i32 %b, ptr %retaddr) {
 ; CHECK-LABEL: t1:
diff --git a/llvm/test/Transforms/SimplifyCFG/2008-07-13-InfLoopMiscompile.ll b/llvm/test/Transforms/SimplifyCFG/2008-07-13-InfLoopMiscompile.ll
index 2e9e7b19c73e2..44d92e1a1c210 100644
--- a/llvm/test/Transforms/SimplifyCFG/2008-07-13-InfLoopMiscompile.ll
+++ b/llvm/test/Transforms/SimplifyCFG/2008-07-13-InfLoopMiscompile.ll
@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt < %s -passes=simplifycfg -simplifycfg-require-and-preserve-domtree=1 -S | FileCheck %s
+; RUN: opt < %s -passes=simplifycfg -simplifycfg-require-and-preserve-domtree=1 -keep-loops="false" -S | FileCheck %s
 ; PR2540
 ; Outval should end up with a select from 0/2, not all constants.
 
@@ -52,4 +52,3 @@ func_1.exit:		; preds = %cowblock, %entry
 }
 
 declare i32 @printf(ptr, ...) nounwind
-
diff --git a/llvm/test/Transforms/SimplifyCFG/2025-07-29-non-canoncial-loop.ll b/llvm/test/Transforms/SimplifyCFG/2025-07-29-non-canoncial-loop.ll
new file mode 100644
index 0000000000000..a64108f6346b9
--- /dev/null
+++ b/llvm/test/Transforms/SimplifyCFG/2025-07-29-non-canoncial-loop.ll
@@ -0,0 +1,37 @@
+; RUN: opt < %s -passes=simplifycfg -simplifycfg-require-and-preserve-domtree=1 -S | FileCheck --check-prefix=NO-THREADING %s
+; Checks that we do not thread the control flow through the loop header bb1 as
+; that will introduce a non-canonical loop
+
+; NO-THREADING-LABEL: define void @__start
+; NO-THREADING: bb3:
+; NO-THREADING-NEXT: br i1 %cond, label %bb1, label %bb5
+
+; RUN: opt < %s -passes=simplifycfg -simplifycfg-require-and-preserve-domtree=1 --keep-loops="false" -S | FileCheck --check-prefix=THREADING %s
+; Checks that we thread the control flow through the loop header bb1 since we
+; do not request --keep-loops
+
+; THREADING-LABEL: define void @__start
+; THREADING: bb3:
+; THREADING-NEXT: br i1 %cond, label %bb4, label %bb5
+
+define void @__start(i1 %cond) {
+entry:
+  br label %bb1
+
+bb1:                                            ; preds = %bb3, %entry
+  br i1 %cond, label %bb4, label %bb2
+
+bb2:                                            ; preds = %bb1
+  %_0_ = add i16 0, 0
+  br label %bb3
+
+bb3:                                            ; preds = %bb4, %bb2
+  br i1 %cond, label %bb1, label %bb5
+
+bb4:                                            ; preds = %bb1
+  %_1_ = add i32 0, 1
+  br label %bb3
+
+bb5:                                            ; preds = %bb3
+  ret void
+}
diff --git a/llvm/test/Transforms/SimplifyCFG/branch-phi-thread.ll b/llvm/test/Transforms/SimplifyCFG/branch-phi-thread.ll
index 0afec05ecbd6a..ec9423bd81675 100644
--- a/llvm/test/Transforms/SimplifyCFG/branch-phi-thread.ll
+++ b/llvm/test/Transforms/SimplifyCFG/branch-phi-thread.ll
@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt < %s -passes=simplifycfg,adce -simplifycfg-require-and-preserve-domtree=1 -S | FileCheck %s
+; RUN: opt < %s -passes=simplifycfg,adce -simplifycfg-require-and-preserve-domtree=1 -keep-loops="false" -S | FileCheck %s
 
 declare void @f1()
 
diff --git a/llvm/test/Transforms/SimplifyCFG/jump-threading.ll b/llvm/test/Transforms/SimplifyCFG/jump-threading.ll
index 50a32413a0551..a4073ae6eb0b4 100644
--- a/llvm/test/Transforms/SimplifyCFG/jump-threading.ll
+++ b/llvm/test/Transforms/SimplifyCFG/jump-threading.ll
@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt -S -passes=simplifycfg < %s | FileCheck %s
+; RUN: opt -S -passes=simplifycfg -keep-loops="false" < %s | FileCheck %s
 
 declare void @foo()
 declare void @bar()
diff --git a/llvm/test/Transforms/SimplifyCFG/two-entry-phi-return.ll b/llvm/test/Transforms/SimplifyCFG/two-entry-phi-return.ll
index 57930c91b9796..f6d71ddda74fe 100644
--- a/llvm/test/Transforms/SimplifyCFG/two-entry-phi-return.ll
+++ b/llvm/test/Transforms/SimplifyCFG/two-entry-phi-return.ll
@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt < %s -passes=simplifycfg -simplifycfg-require-and-preserve-domtree=1 -S | FileCheck %s
+; RUN: opt < %s -passes=simplifycfg -simplifycfg-require-and-preserve-domtree=1 -keep-loops="false" -S | FileCheck %s
 
 define i1 @qux(ptr %m, ptr %n, ptr %o, ptr %p) nounwind  {
 ; CHECK-LABEL: @qux(

ArneStenkrona2 · 2025-07-29T13:10:02Z

@nikic I didn't see a specific secction for SimplifyCFG in Maintainers.md. Perhaps either you, or someone you think is appropriate, can take a look at this.
I've opened a corresponding issue as well here #151144
Thanks :)

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

llvm/test/Transforms/SimplifyCFG/2025-07-29-non-canoncial-loop.ll

nikic

This generally sounds like a sensible thing to do. The dedicated JumpThreading pass checks this as well.

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

github-actions · 2025-07-29T15:27:07Z

✅ With the latest revision this PR passed the C/C++ code formatter.

ArneStenkrona2 · 2025-08-04T08:33:39Z

@nikic Let me know if there is anything else you'd like to see changed with the PR.

ArneStenkrona2 · 2025-08-12T09:21:47Z

ping

dtcxzyw

LGTM.

nikic

I was concerned about the number of regressions in memory optimizations on llvm-opt-benchmark. However, I suspect now that this is related to the non-standard configuration it uses. For one case I looked at, SimplifyCFG was able to unroll a two-iteration loop (phi with true + false used as exit condition), which enabled optimization. However, if llvm-opt-benchmark didn't use -disable-loop-unrolling, I'd expect LoopUnrollFull to handle it. So probably the regressions are over-represented.

Possibly we could still allow threading for cases where it unrolls the loop, but I guess this is ok for now.

Updates SimplifyCFG to avoid jump threading through loop headers if -keep-loops is requested. Canonical loop form requires a loop header that dominates all blocks in the loop. If we thread through a header, we risk breaking its domination of the loop. This change avoids this issue by conservatively avoiding threading through headers entirely.

ArneStenkrona2 · 2025-08-18T09:21:37Z

Thank you both for taking the time to review this, much appreciated.

Possibly we could still allow threading for cases where it unrolls the loop

That sounds sensible. Another idea I had was to collect a limited number of BBs with e.g. a DFS and see if the whole loop could be identified, which could allow us to check if the threading kept structured control flow. I thought I'd keep the patch as simple as possible though.

github-actions · 2025-08-18T09:47:13Z

@ArneStenkrona2 Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

ArneStenkrona2 · 2025-08-18T13:10:09Z

I took a look at the checks ea2f539
2 out of 67 show failures.
In clang-ppc64-aix they all seem to be this assertion:
Assertion failed: V.getBitWidth() == C.getIntWidth(type) && "Integer type is not the correct size for constant."
I don't see how my patch could cause type mismatches since it only deals with BBs.

In mlir-rocm-mi200 they all seem to be an ld error:
ld.lld: error: unknown abi version:
To my eyes this is certainly unrelated.

To summarize, this doesn't look like things the patch could cause so I will assume them to not be regressions. Not sure how to validate that best. When trying to run this locally llvm-lit reports the failing tests I've tried so far as unsupported. I also looked at the post-checks for the parent commit but seems like different checks are run.

I will say for now that this is unrelated to this patch. If anyone here disagrees feel free to ping me and I'll do my best to take a deeper look.

alexfh · 2025-08-26T17:10:08Z

Hi, we're seeing Clang crashes in the vector-combine pass after this commit. I'm reducing the test case now.

alexfh · 2025-08-26T18:59:17Z

@ArneStenkrona2 the test case is here: https://gcc.godbolt.org/z/fdfaEK5bx

target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

%struct.barney = type { %struct.spam }
%struct.spam = type { [1 x %struct.blam] }
%struct.blam = type { %struct.eggs }
%struct.eggs = type { %struct.barney.0 }
%struct.barney.0 = type { <8 x double> }

; Function Attrs: nocallback nofree nounwind willreturn memory(argmem: readwrite)
declare void @llvm.memcpy.p0.p0.i64(ptr noalias writeonly captures(none), ptr noalias readonly captures(none), i64, i1 immarg) #0

define ptr @wombat(ptr %arg) {
bb:
  %alloca = alloca %struct.barney, align 64
  %alloca1 = alloca %struct.barney, align 64
  %alloca2 = alloca %struct.barney, align 64
  store volatile i32 0, ptr %alloca2, align 4
  %call = call ptr @ham(ptr %alloca1, ptr %alloca2)
  call void @llvm.memcpy.p0.p0.i64(ptr %alloca, ptr %alloca1, i64 64, i1 false)
  %call3 = call ptr @baz(ptr %arg, ptr %alloca)
  ret ptr %call3
}

define ptr @ham(ptr %arg, ptr %arg1) {
bb:
  call void @wibble(ptr %arg, ptr %arg1)
  ret ptr null
}

define ptr @baz(ptr %arg, ptr %arg1) {
bb:
  call void @foo(ptr %arg, ptr %arg1)
  ret ptr null
}

define void @wibble(ptr %arg, ptr %arg1) {
bb:
  call void @bar(ptr %arg, ptr %arg1)
  ret void
}

define void @bar(ptr %arg, ptr %arg1) {
bb:
  %load = load <4 x double>, ptr %arg1, align 32
  %fadd = fadd <4 x double> %load, zeroinitializer
  %getelementptr = getelementptr i8, ptr %arg1, i64 32
  store <4 x double> %fadd, ptr %getelementptr, align 32
  call void @llvm.memcpy.p0.p0.i64(ptr %arg, ptr %arg1, i64 64, i1 false)
  ret void
}

define <4 x double> @eggs(<4 x double> %arg) {
bb:
  %fadd = fadd <4 x double> %arg, %arg
  ret <4 x double> %fadd
}

define void @foo(ptr %arg, ptr %arg1) {
bb:
  call void @widget(ptr %arg, ptr byval(%struct.eggs) %arg1) #1
  ret void
}

define void @widget(ptr %arg, ptr %arg1) {
bb:
  call void @hoge(ptr %arg, ptr byval(%struct.eggs) %arg1)
  ret void
}

define void @hoge(ptr %arg, ptr %arg1) {
bb:
  %load = load <4 x double>, ptr %arg1, align 64
  %call = call <4 x double> @eggs(<4 x double> %load)
  store <4 x double> %call, ptr %arg1, align 64
  call void @llvm.memcpy.p0.p0.i64(ptr %arg, ptr %arg1, i64 64, i1 false)
  ret void
}

attributes #0 = { nocallback nofree nounwind willreturn memory(argmem: readwrite) }
attributes #1 = { alwaysinline }

Please fix or revert soon! Thanks!

This reverts commit ea2f539.

alexfh · 2025-08-27T01:18:43Z

@ArneStenkrona2 I've sent #155533 to revert this commit, so that there's no rush to resolve the problem, after which this PR can be merged again along with the fix. However, if there's a trivial and low-risk fix-forward, this may be an option.

ArneStenkrona2 · 2025-08-27T14:35:11Z

@alexfh Thank you for bringing this to my attention.
The crash you are talking about is #155543, correct? It appears that PR #155621 resolved it without the need for a revert. Does that mean that the crash turned out to be unrelated to this patch (i.e. #151142)? If so, I assume there is no action for me to take at the moment. Do let me know if I've misunderstood the situation.

alexfh · 2025-08-27T16:32:04Z

@alexfh Thank you for bringing this to my attention. The crash you are talking about is #155543, correct? It appears that PR #155621 resolved it without the need for a revert. Does that mean that the crash turned out to be unrelated to this patch (i.e. #151142)? If so, I assume there is no action for me to take at the moment. Do let me know if I've misunderstood the situation.

Yes, you're right. Thanks @dtcxzyw for the fix!

ArneStenkrona2 · 2025-08-27T16:34:52Z

Nice :) Glad that you managed to find a quick resolution.

llvmbot added backend:ARM llvm:transforms labels Jul 29, 2025

ArneStenkrona2 mentioned this pull request Jul 29, 2025

SimplifyCFG breaks canonical loops even if --keep-loops is requested #151144

Closed

nikic reviewed Jul 29, 2025

View reviewed changes

llvm/lib/Transforms/Utils/SimplifyCFG.cpp Outdated Show resolved Hide resolved

llvm/lib/Transforms/Utils/SimplifyCFG.cpp Outdated Show resolved Hide resolved

llvm/test/Transforms/SimplifyCFG/2025-07-29-non-canoncial-loop.ll Outdated Show resolved Hide resolved

nikic mentioned this pull request Jul 29, 2025

Task submission dtcxzyw/llvm-opt-benchmark#1312

Open

zyw-bot mentioned this pull request Jul 29, 2025

pre-commit: PR151142 dtcxzyw/llvm-opt-benchmark#2618

Closed

nikic reviewed Jul 29, 2025

View reviewed changes

llvm/lib/Transforms/Utils/SimplifyCFG.cpp Outdated Show resolved Hide resolved

llvm/lib/Transforms/Utils/SimplifyCFG.cpp Outdated Show resolved Hide resolved

llvm/lib/Transforms/Utils/SimplifyCFG.cpp Outdated Show resolved Hide resolved

ArneStenkrona2 requested a review from nikic August 11, 2025 12:04

dtcxzyw approved these changes Aug 15, 2025

View reviewed changes

nikic approved these changes Aug 16, 2025

View reviewed changes

ArneStenkrona2 force-pushed the fix-irreducible-cfg-after-threading branch from b082c08 to cfa707d Compare August 18, 2025 09:11

nikic enabled auto-merge (squash) August 18, 2025 09:17

nikic merged commit ea2f539 into llvm:main Aug 18, 2025
9 checks passed

ArneStenkrona2 deleted the fix-irreducible-cfg-after-threading branch August 18, 2025 13:58

alexfh added a commit that referenced this pull request Aug 27, 2025

Revert "[SimplifyCFG] Avoid threading for loop headers (#151142)"

1518865

This reverts commit ea2f539.

alexfh mentioned this pull request Aug 27, 2025

Revert "[SimplifyCFG] Avoid threading for loop headers" #155533

Closed

dtcxzyw mentioned this pull request Aug 27, 2025

[VectorCombine] SIGSEGV at O3 #155543

Closed

[SimplifyCFG] Avoid threading for loop headers #151142

[SimplifyCFG] Avoid threading for loop headers #151142

Uh oh!

Conversation

ArneStenkrona2 commented Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jul 29, 2025

Uh oh!

llvmbot commented Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ArneStenkrona2 commented Jul 29, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ArneStenkrona2 commented Aug 4, 2025

Uh oh!

ArneStenkrona2 commented Aug 12, 2025

Uh oh!

dtcxzyw left a comment

Choose a reason for hiding this comment

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

ArneStenkrona2 commented Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Aug 18, 2025

Uh oh!

ArneStenkrona2 commented Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexfh commented Aug 26, 2025

Uh oh!

alexfh commented Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexfh commented Aug 27, 2025

Uh oh!

ArneStenkrona2 commented Aug 27, 2025

Uh oh!

alexfh commented Aug 27, 2025

Uh oh!

ArneStenkrona2 commented Aug 27, 2025

Uh oh!

Uh oh!

ArneStenkrona2 commented Jul 29, 2025 •

edited

Loading

llvmbot commented Jul 29, 2025 •

edited

Loading

github-actions bot commented Jul 29, 2025 •

edited

Loading

ArneStenkrona2 commented Aug 18, 2025 •

edited

Loading

ArneStenkrona2 commented Aug 18, 2025 •

edited

Loading

alexfh commented Aug 26, 2025 •

edited

Loading