[llvm] 19dc02e - [MergeICmps] Allow sinking past non-load/store

[Nikita Popov via llvm-commits]

Author: Nikita Popov
Date: 2021-08-23T22:03:49+02:00
New Revision: 19dc02e99f802922a3af69e802465bee0723b57a

URL: https://github.com/llvm/llvm-project/commit/19dc02e99f802922a3af69e802465bee0723b57a
DIFF: https://github.com/llvm/llvm-project/commit/19dc02e99f802922a3af69e802465bee0723b57a.diff

LOG: [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

Added: 
    

Modified: 
    llvm/lib/Transforms/Scalar/MergeICmps.cpp
    llvm/test/Transforms/MergeICmps/X86/split-block-does-work.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Scalar/MergeICmps.cpp b/llvm/lib/Transforms/Scalar/MergeICmps.cpp
index f13f24ad2027b..34465c76dd3db 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<LoadInst>(I))
-    return LI->isSimple();
-  if (const StoreInst *SI = dyn_cast<StoreInst>(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 0b9663f449803..1e341b92918d9 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(