[ImplicitNullChecks] Do not not handle call MachineInstrs

We don't track callee clobbered registers correctly, so avoid hoisting across calls. Note: for this bug to trigger we need a `readonly` call target, since we already have logic to not hoist across potentially storing instructions either. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@287159 91177308-0d34-0410-b5e6-96231b3b80d8
liuchyi · Nov 16, 2016 · 99e15d0 · 99e15d0
1 parent 34c84f4
commit 99e15d0
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 1 deletion.
diff --git a/lib/CodeGen/ImplicitNullChecks.cpp b/lib/CodeGen/ImplicitNullChecks.cpp
@@ -187,7 +187,10 @@ void HazardDetector::rememberInstruction(MachineInstr *MI) {
   assert(!isClobbered() &&
          "Don't add instructions to a clobbered hazard detector");
 
-  if (MI->mayStore() || MI->hasUnmodeledSideEffects()) {
+  // There may be readonly calls that we can handle in theory, but for
+  // now we don't bother since we don't handle callee clobbered
+  // registers.
+  if (MI->isCall() || MI->mayStore() || MI->hasUnmodeledSideEffects()) {
     hasSeenClobber = true;
     return;
   }

diff --git a/test/CodeGen/X86/implicit-null-checks.mir b/test/CodeGen/X86/implicit-null-checks.mir
@@ -79,6 +79,22 @@
     ret i32 200
   }
 
+  declare void @f() readonly
+
+  define i32 @no_hoist_across_call(i32* %ptr) {
+  entry:
+    %is_null = icmp eq i32* %ptr, null
+    br i1 %is_null, label %leave, label %stay, !make.implicit !0
+
+  stay:
+    call void @f()
+    %val = load i32, i32* %ptr
+    ret i32 %val
+
+  leave:
+    ret i32 0
+  }
+
   !0 = !{}
 ...
 ---
@@ -255,3 +271,44 @@ body:             |
     RET 0, %eax
 
 ...
+---
+name:            no_hoist_across_call
+# CHECK-LABEL: name:            no_hoist_across_call
+alignment:       4
+tracksRegLiveness: true
+liveins:
+  - { reg: '%rdi' }
+calleeSavedRegisters: [ '%bh', '%bl', '%bp', '%bpl', '%bx', '%ebp', '%ebx',
+                        '%rbp', '%rbx', '%r12', '%r13', '%r14', '%r15',
+                        '%r12b', '%r13b', '%r14b', '%r15b', '%r12d', '%r13d',
+                        '%r14d', '%r15d', '%r12w', '%r13w', '%r14w', '%r15w' ]
+# CHECK: body:
+# CHECK-NOT: FAULTING_LOAD_OP
+# CHECK: bb.1.stay:
+# CHECK: CALL64pcrel32
+body:             |
+  bb.0.entry:
+    successors: %bb.2.leave, %bb.1.stay
+    liveins: %rdi, %rbx
+
+    frame-setup PUSH64r killed %rbx, implicit-def %rsp, implicit %rsp
+    CFI_INSTRUCTION def_cfa_offset 16
+    CFI_INSTRUCTION offset %rbx, -16
+    %rbx = MOV64rr %rdi
+    TEST64rr %rbx, %rbx, implicit-def %eflags
+    JE_1 %bb.2.leave, implicit killed %eflags
+
+  bb.1.stay:
+    liveins: %rbx
+
+    CALL64pcrel32 @f, csr_64, implicit %rsp, implicit-def %rsp
+    %eax = MOV32rm killed %rbx, 1, _, 0, _ :: (load 4 from %ir.ptr)
+    %rbx = POP64r implicit-def %rsp, implicit %rsp
+    RETQ %eax
+
+  bb.2.leave:
+    %eax = XOR32rr undef %eax, undef %eax, implicit-def dead %eflags
+    %rbx = POP64r implicit-def %rsp, implicit %rsp
+    RETQ %eax
+
+...