From 19dc02e99f802922a3af69e802465bee0723b57a Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Sun, 22 Aug 2021 18:15:55 +0200 Subject: [PATCH] [MergeICmps] Allow sinking past non-load/store This is a followup to D106591. MergeICmps currently only allows sinking the loads past either instructions that don't write to memory at all, or simple loads/stores that don't modify the memory the loads access. The "simple loads/stores" part of this check doesn't seem necessary to me -- AA isModRef() already accurately models any operation that may clobber the memory. For example, in the adjusted test case the transform is still fine if the call to @foo() isn't readonly, but inaccessiblememonly -- in both cases, the call cannot modify the loaded memory. Differential Revision: https://reviews.llvm.org/D108517 --- llvm/lib/Transforms/Scalar/MergeICmps.cpp | 14 +------------- .../Transforms/MergeICmps/X86/split-block-does-work.ll | 2 +- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/llvm/lib/Transforms/Scalar/MergeICmps.cpp b/llvm/lib/Transforms/Scalar/MergeICmps.cpp index f13f24a..34465c7 100644 --- a/llvm/lib/Transforms/Scalar/MergeICmps.cpp +++ b/llvm/lib/Transforms/Scalar/MergeICmps.cpp @@ -66,15 +66,6 @@ namespace { #define DEBUG_TYPE "mergeicmps" -// Returns true if the instruction is a simple load or a simple store -static bool isSimpleLoadOrStore(const Instruction *I) { - if (const LoadInst *LI = dyn_cast(I)) - return LI->isSimple(); - if (const StoreInst *SI = dyn_cast(I)) - return SI->isSimple(); - return false; -} - // A BCE atom "Binary Compare Expression Atom" represents an integer load // that is a constant offset from a base value, e.g. `a` or `o.c` in the example // at the top. @@ -244,10 +235,7 @@ bool BCECmpBlock::canSinkBCECmpInst(const Instruction *Inst, // If this instruction may clobber the loads and is in middle of the BCE cmp // block instructions, then bail for now. if (Inst->mayWriteToMemory()) { - // Bail if this is not a simple load or store - if (!isSimpleLoadOrStore(Inst)) - return false; - // Disallow stores that might alias the BCE operands + // Disallow instructions that might modify the BCE operands MemoryLocation LLoc = MemoryLocation::get(Cmp.Lhs.LoadI); MemoryLocation RLoc = MemoryLocation::get(Cmp.Rhs.LoadI); if (isModSet(AA.getModRefInfo(Inst, LLoc)) || diff --git a/llvm/test/Transforms/MergeICmps/X86/split-block-does-work.ll b/llvm/test/Transforms/MergeICmps/X86/split-block-does-work.ll index 0b9663f..1e341b9 100644 --- a/llvm/test/Transforms/MergeICmps/X86/split-block-does-work.ll +++ b/llvm/test/Transforms/MergeICmps/X86/split-block-does-work.ll @@ -3,7 +3,7 @@ %S = type { i32, i32, i32, i32 } -declare void @foo(...) readonly +declare void @foo(...) inaccessiblememonly ; We can split %entry and create a memcmp(16 bytes). define zeroext i1 @opeq1( -- 2.7.4