[llvm] [LoopSink] Do not sink instructions with side effects (PR #214988)

[via llvm-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-transforms

Author: Matthias Görgens (matthiasgoergens)

<details>
<summary>Changes</summary>

`sinkLoopInvariantInstructions` accepts any preheader instruction that `canSinkOrHoistInst` approves, but that helper only proves mechanical legality — aliasing, throw-safety — not that the execution count is preserved. A call that writes memory passes those checks whenever the loop contains no conflicting accesses, and `sinkInstruction` then moves it from the preheader, where it runs exactly once, into a cold block inside the loop, where it may run zero or many times: wrong code under profile-guided builds.

Skip instructions with observable side effects before consulting `canSinkOrHoistInst`. Sinking exists to shrink hot-path code size; an instruction with observable side effects was never a profitable or sound candidate. The regression test covers a writing call and a call that may not return, and pins that a read-only `willreturn` call stays sinkable.

Fixes #<!-- -->213025.

## Tool use

Per the [LLVM AI Tool Use Policy](https://llvm.org/docs/AIToolPolicy.html): AI
tools were involved throughout the preparation of this change. I am the author
and accountable for the contribution.

Assisted-by: OpenAI Codex
Assisted-by: Claude Code
Assisted-by: Kimi
Assisted-by: DeepSeek


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


2 Files Affected:

- (modified) llvm/lib/Transforms/Scalar/LoopSink.cpp (+5) 
- (added) llvm/test/Transforms/LICM/loopsink-writeonly-call.ll (+82) 


``````````diff
diff --git a/llvm/lib/Transforms/Scalar/LoopSink.cpp b/llvm/lib/Transforms/Scalar/LoopSink.cpp
index b9fde4c6a3b76..33578ea81573d 100644
--- a/llvm/lib/Transforms/Scalar/LoopSink.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopSink.cpp
@@ -341,6 +341,11 @@ static bool sinkLoopInvariantInstructions(Loop &L, AAResults &AA, LoopInfo &LI,
     // No need to check for instruction's operands are loop invariant.
     assert(L.hasLoopInvariantOperands(&I) &&
            "Insts in a loop's preheader should have loop invariant operands!");
+    // Sinking into a loop may change how many times an instruction executes.
+    // Do not move instructions with observable side effects from the
+    // preheader, where they execute exactly once, into the loop.
+    if (I.mayHaveSideEffects())
+      continue;
     if (!canSinkOrHoistInst(I, &AA, &DT, &L, MSSAU, false, LICMFlags))
       continue;
     if (sinkInstruction(L, I, ColdLoopBBs, LoopBlockNumber, LI, DT, BFI,
diff --git a/llvm/test/Transforms/LICM/loopsink-writeonly-call.ll b/llvm/test/Transforms/LICM/loopsink-writeonly-call.ll
new file mode 100644
index 0000000000000..4a8829344f8ac
--- /dev/null
+++ b/llvm/test/Transforms/LICM/loopsink-writeonly-call.ll
@@ -0,0 +1,82 @@
+; RUN: opt -S -verify-memoryssa -passes=loop-sink < %s | FileCheck %s
+
+; A call that writes memory is observable even when its return value is only
+; used on a cold path through the loop. It must not be sunk from the preheader.
+;
+; CHECK-LABEL: define void @writeonly_call(
+; CHECK:       entry:
+; CHECK-NEXT:    [[VALUE:%.*]] = call i32 @write()
+; CHECK-NEXT:    br label %loop
+; CHECK:       cold:
+; CHECK-NOT:     call i32 @write()
+define void @writeonly_call(i1 %condition) !prof !0 {
+entry:
+  %value = call i32 @write()
+  br label %loop
+
+loop:
+  br i1 %condition, label %cold, label %exit, !prof !1
+
+cold:
+  %unused = icmp eq i32 %value, 0
+  br label %loop
+
+exit:
+  ret void
+}
+
+declare i32 @write() nounwind willreturn memory(write)
+
+; A call without willreturn cannot move to a path that may not execute.
+;
+; CHECK-LABEL: define void @call_without_willreturn(
+; CHECK:       entry:
+; CHECK-NEXT:    [[VALUE:%.*]] = call i32 @may_not_return()
+; CHECK-NEXT:    br label %loop
+; CHECK:       cold:
+; CHECK-NOT:     call i32 @may_not_return()
+define void @call_without_willreturn(i1 %condition) !prof !0 {
+entry:
+  %value = call i32 @may_not_return()
+  br label %loop
+
+loop:
+  br i1 %condition, label %cold, label %exit, !prof !1
+
+cold:
+  %unused = icmp eq i32 %value, 0
+  br label %loop
+
+exit:
+  ret void
+}
+
+declare i32 @may_not_return() nounwind memory(none)
+
+; A side-effect-free call that is guaranteed to return remains sinkable.
+;
+; CHECK-LABEL: define void @readonly_willreturn_call(
+; CHECK:       entry:
+; CHECK-NEXT:    br label %loop
+; CHECK:       cold:
+; CHECK-NEXT:    [[VALUE:%.*]] = call i32 @read()
+define void @readonly_willreturn_call(i1 %condition) !prof !0 {
+entry:
+  %value = call i32 @read()
+  br label %loop
+
+loop:
+  br i1 %condition, label %cold, label %exit, !prof !1
+
+cold:
+  %unused = icmp eq i32 %value, 0
+  br label %loop
+
+exit:
+  ret void
+}
+
+declare i32 @read() nounwind willreturn memory(read)
+
+!0 = !{!"function_entry_count", i64 1}
+!1 = !{!"branch_weights", i32 1, i32 2000}

``````````

</details>


https://github.com/llvm/llvm-project/pull/214988