Skip to content

[Sink][NFC] Move all checks for unsafe instructions into one function #137398

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 26, 2025

Conversation

LU-JOHN
Copy link
Contributor

@LU-JOHN LU-JOHN commented Apr 25, 2025

Move check for instruction that is unsafe to sink into isSafeToMove function.

@llvmbot
Copy link
Member

llvmbot commented Apr 25, 2025

@llvm/pr-subscribers-llvm-transforms

Author: None (LU-JOHN)

Changes

Move check for instruction that is unsafe to sink into isSafeToMove function.


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

1 Files Affected:

  • (modified) llvm/lib/Transforms/Scalar/Sink.cpp (+6-6)
diff --git a/llvm/lib/Transforms/Scalar/Sink.cpp b/llvm/lib/Transforms/Scalar/Sink.cpp
index 1a48a59c4189e..0865d20f9d990 100644
--- a/llvm/lib/Transforms/Scalar/Sink.cpp
+++ b/llvm/lib/Transforms/Scalar/Sink.cpp
@@ -35,6 +35,12 @@ static bool isSafeToMove(Instruction *Inst, AliasAnalysis &AA,
     return false;
   }
 
+  // Don't sink static alloca instructions.  CodeGen assumes allocas outside the
+  // entry block are dynamically sized stack objects.
+  if (AllocaInst *AI = dyn_cast<AllocaInst>(Inst))
+    if (AI->isStaticAlloca())
+      return false;
+
   if (LoadInst *L = dyn_cast<LoadInst>(Inst)) {
     MemoryLocation Loc = MemoryLocation::get(L);
     for (Instruction *S : Stores)
@@ -103,12 +109,6 @@ static bool SinkInstruction(Instruction *Inst,
                             SmallPtrSetImpl<Instruction *> &Stores,
                             DominatorTree &DT, LoopInfo &LI, AAResults &AA) {
 
-  // Don't sink static alloca instructions.  CodeGen assumes allocas outside the
-  // entry block are dynamically sized stack objects.
-  if (AllocaInst *AI = dyn_cast<AllocaInst>(Inst))
-    if (AI->isStaticAlloca())
-      return false;
-
   // Check if it's safe to move the instruction.
   if (!isSafeToMove(Inst, AA, Stores))
     return false;

@nikic nikic changed the title [NFC] Move all checks for unsafe instructions into one function [Sink][NFC] Move all checks for unsafe instructions into one function Apr 25, 2025
@nikic nikic merged commit 571e024 into llvm:main Apr 26, 2025
13 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Apr 26, 2025

LLVM Buildbot has detected a new failure on builder llvm-clang-x86_64-gcc-ubuntu running on sie-linux-worker3 while building llvm at step 6 "test-build-unified-tree-check-all".

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

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 :: tools/llvm-exegesis/RISCV/rvv/filter.test' FAILED ********************
Exit Code: 2

Command Output (stderr):
--
/home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/build/bin/llvm-exegesis -mtriple=riscv64 -mcpu=sifive-x280 -benchmark-phase=assemble-measured-code --mode=inverse_throughput --opcode-name=PseudoVNCLIPU_WX_M1_MASK     --riscv-filter-config='vtype = {VXRM: rod, AVL: VLMAX, SEW: e(8|16), Policy: ta/mu}' --max-configs-per-opcode=1000 --min-instructions=10 | /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/build/bin/FileCheck /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/llvm-project/llvm/test/tools/llvm-exegesis/RISCV/rvv/filter.test # RUN: at line 1
+ /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/build/bin/FileCheck /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/llvm-project/llvm/test/tools/llvm-exegesis/RISCV/rvv/filter.test
+ /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/build/bin/llvm-exegesis -mtriple=riscv64 -mcpu=sifive-x280 -benchmark-phase=assemble-measured-code --mode=inverse_throughput --opcode-name=PseudoVNCLIPU_WX_M1_MASK '--riscv-filter-config=vtype = {VXRM: rod, AVL: VLMAX, SEW: e(8|16), Policy: ta/mu}' --max-configs-per-opcode=1000 --min-instructions=10
PseudoVNCLIPU_WX_M1_MASK: Failed to produce any snippet via: instruction has tied variables, avoiding Read-After-Write issue, picking random def and use registers not aliasing each other, for uses, one unique register for each position
FileCheck error: '<stdin>' is empty.
FileCheck command line:  /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/build/bin/FileCheck /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/llvm-project/llvm/test/tools/llvm-exegesis/RISCV/rvv/filter.test

--

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


jyli0116 pushed a commit to jyli0116/llvm-project that referenced this pull request Apr 28, 2025
…llvm#137398)

Move check for instruction that is unsafe to sink into isSafeToMove
function.

Signed-off-by: John Lu <[email protected]>
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…llvm#137398)

Move check for instruction that is unsafe to sink into isSafeToMove
function.

Signed-off-by: John Lu <[email protected]>
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…llvm#137398)

Move check for instruction that is unsafe to sink into isSafeToMove
function.

Signed-off-by: John Lu <[email protected]>
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…llvm#137398)

Move check for instruction that is unsafe to sink into isSafeToMove
function.

Signed-off-by: John Lu <[email protected]>
Ankur-0429 pushed a commit to Ankur-0429/llvm-project that referenced this pull request May 9, 2025
…llvm#137398)

Move check for instruction that is unsafe to sink into isSafeToMove
function.

Signed-off-by: John Lu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants