From 0c0ee9fbcadabf58c682b500199299aa7f245bba Mon Sep 17 00:00:00 2001
From: Serguei Katkov <serguei.katkov@azul.com>
Date: Wed, 9 Aug 2017 05:17:02 +0000
Subject: [PATCH] [ImplicitNullCheck] Fix the bug when dependent instruction
 accesses memory

It is possible that dependent instruction may access memory.
In this case we must reject optimization because the memory change will
be visible in null handler basic block. So we will execute an instruction which
we must not execute if check fails.

Reviewers: sanjoy, reames
Reviewed By: sanjoy
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D36392


git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@310443 91177308-0d34-0410-b5e6-96231b3b80d8
---
 lib/CodeGen/ImplicitNullChecks.cpp        |  4 +-
 test/CodeGen/X86/implicit-null-checks.mir | 50 +++++++++++++++++++++++
 2 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/lib/CodeGen/ImplicitNullChecks.cpp b/lib/CodeGen/ImplicitNullChecks.cpp
index e308f49ec4e..e3dcffe6bae 100644
--- a/lib/CodeGen/ImplicitNullChecks.cpp
+++ b/lib/CodeGen/ImplicitNullChecks.cpp
@@ -390,8 +390,10 @@ bool ImplicitNullChecks::canHoistInst(MachineInstr *FaultingMI,
   // We don't want to reason about speculating loads.  Note -- at this point
   // we should have already filtered out all of the other non-speculatable
   // things, like calls and stores.
+  // We also do not want to hoist stores because it might change the memory
+  // while the FaultingMI may result in faulting.
   assert(canHandle(DependenceMI) && "Should never have reached here!");
-  if (DependenceMI->mayLoad())
+  if (DependenceMI->mayLoadOrStore())
     return false;
 
   for (auto &DependenceMO : DependenceMI->operands()) {
diff --git a/test/CodeGen/X86/implicit-null-checks.mir b/test/CodeGen/X86/implicit-null-checks.mir
index 6efc965a694..df89e4f6bfa 100644
--- a/test/CodeGen/X86/implicit-null-checks.mir
+++ b/test/CodeGen/X86/implicit-null-checks.mir
@@ -365,6 +365,18 @@
     ret i32 undef
   }
 
+  define i32 @inc_spill_dep(i32* %ptr, i32 %val) {
+  entry:
+    %ptr_is_null = icmp eq i32* %ptr, null
+    br i1 %ptr_is_null, label %is_null, label %not_null, !make.implicit !0
+
+  not_null:
+    ret i32 undef
+
+  is_null:
+    ret i32 undef
+  }
+
   attributes #0 = { "target-features"="+bmi,+bmi2" }
 
   !0 = !{}
@@ -1265,3 +1277,41 @@ body:             |
     RETQ %eax
 
 ...
+---
+name:            inc_spill_dep
+# CHECK-LABEL: inc_spill_dep
+# CHECK: bb.0.entry:
+# CHECK:    TEST64rr %rdi, %rdi, implicit-def %eflags
+# CHECK-NEXT:    JE_1 %bb.2.is_null, implicit killed %eflags
+# CHECK: bb.1.not_null
+
+alignment:       4
+tracksRegLiveness: true
+stack:
+  - { id: 0, type: spill-slot, offset: -8, size: 8, alignment: 8}
+liveins:
+  - { reg: '%rdi' }
+  - { reg: '%rsi' }
+body:             |
+  bb.0.entry:
+    liveins: %rdi, %rsi
+
+    %rsp = frame-setup SUB64ri8 %rsp, 8, implicit-def dead %eflags
+    MOV32mr %rsp, 1, %noreg, 0, %noreg, %esi :: (store 4 into %stack.0)
+    TEST64rr %rdi, %rdi, implicit-def %eflags
+    JE_1 %bb.2.is_null, implicit killed %eflags
+
+  bb.1.not_null:
+    liveins: %rdi, %rsi
+
+    %r14d = MOV32rm %rsp, 1, %noreg, 0, %noreg :: (load 4 from %stack.0)
+    MOV64mr %rsp, 1, %noreg, 0, %noreg, %rdi :: (store 8 into %stack.0)
+    %edi = MOV32rm %rdi, 1, %noreg, 8, %noreg :: (load 4 from %ir.ptr)
+    %eax = MOV32rr %edi
+    RETQ %eax
+
+  bb.2.is_null:
+    %eax = XOR32rr undef %eax, undef %eax, implicit-def dead %eflags
+    RETQ %eax
+
+...
-- 
2.50.1