[ImplicitNullChecks] Be smarter in picking the memory op.

Summary:
Before this change ImplicitNullChecks would only pick loads of the form:

```
   test Reg, Reg
   jz elsewhere
 fallthrough:
   movl 32(Reg), Reg2
```

but not (say)

```
   test Reg, Reg
   jz elsewhere
 fallthrough:
   inc Reg3
   movl 32(Reg), Reg2
```

This change teaches ImplicitNullChecks to look through "unrelated"
instructions like `inc Reg3` when searching for a load instruction
to convert to a trapping load.

Reviewers: atrick, JosephTremoulet, reames

Subscribers: llvm-commits

Differential Revision: http://reviews.llvm.org/D11044

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@241850 91177308-0d34-0410-b5e6-96231b3b80d8
This commit is contained in:
Sanjoy Das 2015-07-09 20:13:25 +00:00
parent 377796abdf
commit a5a3e2d58a
3 changed files with 159 additions and 10 deletions

View File

@ -25,10 +25,12 @@
// //
//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//
#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/SmallVector.h" #include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/Statistic.h" #include "llvm/ADT/Statistic.h"
#include "llvm/CodeGen/Passes.h" #include "llvm/CodeGen/Passes.h"
#include "llvm/CodeGen/MachineFunction.h" #include "llvm/CodeGen/MachineFunction.h"
#include "llvm/CodeGen/MachineMemOperand.h"
#include "llvm/CodeGen/MachineOperand.h" #include "llvm/CodeGen/MachineOperand.h"
#include "llvm/CodeGen/MachineFunctionPass.h" #include "llvm/CodeGen/MachineFunctionPass.h"
#include "llvm/CodeGen/MachineInstrBuilder.h" #include "llvm/CodeGen/MachineInstrBuilder.h"
@ -177,6 +179,9 @@ bool ImplicitNullChecks::analyzeBlockForNullChecks(
// callq throw_NullPointerException // callq throw_NullPointerException
// //
// LblNotNull: // LblNotNull:
// Inst0
// Inst1
// ...
// Def = Load (%RAX + <offset>) // Def = Load (%RAX + <offset>)
// ... // ...
// //
@ -187,6 +192,8 @@ bool ImplicitNullChecks::analyzeBlockForNullChecks(
// jmp LblNotNull ;; explicit or fallthrough // jmp LblNotNull ;; explicit or fallthrough
// //
// LblNotNull: // LblNotNull:
// Inst0
// Inst1
// ... // ...
// //
// LblNull: // LblNull:
@ -194,16 +201,76 @@ bool ImplicitNullChecks::analyzeBlockForNullChecks(
// //
unsigned PointerReg = MBP.LHS.getReg(); unsigned PointerReg = MBP.LHS.getReg();
MachineInstr *MemOp = &*NotNullSucc->begin();
// As we scan NotNullSucc for a suitable load instruction, we keep track of
// the registers defined and used by the instructions we scan past. This bit
// of information lets us decide if it is legal to hoist the load instruction
// we find (if we do find such an instruction) to before NotNullSucc.
DenseSet<unsigned> RegDefs, RegUses;
// Returns true if it is safe to reorder MI to before NotNullSucc.
auto IsSafeToHoist = [&](MachineInstr *MI) {
// Right now we don't want to worry about LLVM's memory model. This can be
// made more precise later.
for (auto *MMO : MI->memoperands())
if (!MMO->isUnordered())
return false;
for (auto &MO : MI->operands()) {
if (MO.isReg() && MO.getReg()) {
for (unsigned Reg : RegDefs)
if (TRI->regsOverlap(Reg, MO.getReg()))
return false; // We found a write-after-write or read-after-write
if (MO.isDef())
for (unsigned Reg : RegUses)
if (TRI->regsOverlap(Reg, MO.getReg()))
return false; // We found a write-after-read
}
}
return true;
};
for (auto MII = NotNullSucc->begin(), MIE = NotNullSucc->end(); MII != MIE;
++MII) {
MachineInstr *MI = &*MII;
unsigned BaseReg, Offset; unsigned BaseReg, Offset;
if (TII->getMemOpBaseRegImmOfs(MemOp, BaseReg, Offset, TRI)) if (TII->getMemOpBaseRegImmOfs(MI, BaseReg, Offset, TRI))
if (MemOp->mayLoad() && !MemOp->isPredicable() && BaseReg == PointerReg && if (MI->mayLoad() && !MI->isPredicable() && BaseReg == PointerReg &&
Offset < PageSize && MemOp->getDesc().getNumDefs() == 1) { Offset < PageSize && MI->getDesc().getNumDefs() == 1 &&
NullCheckList.emplace_back(MemOp, MBP.ConditionDef, &MBB, NotNullSucc, IsSafeToHoist(MI)) {
NullCheckList.emplace_back(MI, MBP.ConditionDef, &MBB, NotNullSucc,
NullSucc); NullSucc);
return true; return true;
} }
// MI did not match our criteria for conversion to a trapping load. Check
// if we can continue looking.
if (MI->mayStore() || MI->hasUnmodeledSideEffects())
return false;
for (auto *MMO : MI->memoperands())
// Right now we don't want to worry about LLVM's memory model.
if (!MMO->isUnordered())
return false;
// It _may_ be okay to reorder a later load instruction across MI. Make a
// note of its operands so that we can make the legality check if we find a
// suitable load instruction:
for (auto &MO : MI->operands()) {
if (!MO.isReg() || !MO.getReg())
continue;
if (MO.isDef())
RegDefs.insert(MO.getReg());
else
RegUses.insert(MO.getReg());
}
}
return false; return false;
} }

View File

@ -51,4 +51,46 @@ define i32 @imp_null_check_load_no_md(i32* %x) {
ret i32 %t ret i32 %t
} }
define i32 @imp_null_check_no_hoist_over_acquire_load(i32* %x, i32* %y) {
; We cannot hoist %t1 over %t0 since %t0 is an acquire load
entry:
%c = icmp eq i32* %x, null
br i1 %c, label %is_null, label %not_null, !make.implicit !0
is_null:
ret i32 42
not_null:
%t0 = load atomic i32, i32* %y acquire, align 4
%t1 = load i32, i32* %x
%p = add i32 %t0, %t1
ret i32 %p
}
define i32 @imp_null_check_add_result(i32* %x, i32* %y) {
; This will codegen to:
;
; movl (%rsi), %eax
; addl (%rdi), %eax
;
; The load instruction we wish to hoist is the addl, but there is a
; write-after-write hazard preventing that from happening. We could
; get fancy here and exploit the commutativity of addition, but right
; now -implicit-null-checks isn't that smart.
;
entry:
%c = icmp eq i32* %x, null
br i1 %c, label %is_null, label %not_null, !make.implicit !0
is_null:
ret i32 42
not_null:
%t0 = load i32, i32* %y
%t1 = load i32, i32* %x
%p = add i32 %t0, %t1
ret i32 %p
}
!0 = !{} !0 = !{}

View File

@ -76,6 +76,31 @@ define i32 @imp_null_check_add_result(i32* %x, i32 %p) {
ret i32 %p1 ret i32 %p1
} }
define i32 @imp_null_check_hoist_over_unrelated_load(i32* %x, i32* %y, i32* %z) {
; CHECK-LABEL: _imp_null_check_hoist_over_unrelated_load:
; CHECK: Ltmp7:
; CHECK: movl (%rdi), %eax
; CHECK: movl (%rsi), %ecx
; CHECK: movl %ecx, (%rdx)
; CHECK: retq
; CHECK: Ltmp6:
; CHECK: movl $42, %eax
; CHECK: retq
entry:
%c = icmp eq i32* %x, null
br i1 %c, label %is_null, label %not_null, !make.implicit !0
is_null:
ret i32 42
not_null:
%t0 = load i32, i32* %y
%t1 = load i32, i32* %x
store i32 %t0, i32* %z
ret i32 %t1
}
!0 = !{} !0 = !{}
; CHECK-LABEL: __LLVM_FaultMaps: ; CHECK-LABEL: __LLVM_FaultMaps:
@ -88,7 +113,7 @@ define i32 @imp_null_check_add_result(i32* %x, i32 %p) {
; CHECK-NEXT: .short 0 ; CHECK-NEXT: .short 0
; # functions: ; # functions:
; CHECK-NEXT: .long 3 ; CHECK-NEXT: .long 4
; FunctionAddr: ; FunctionAddr:
; CHECK-NEXT: .quad _imp_null_check_add_result ; CHECK-NEXT: .quad _imp_null_check_add_result
@ -116,6 +141,19 @@ define i32 @imp_null_check_add_result(i32* %x, i32 %p) {
; Fault[0].HandlerOffset: ; Fault[0].HandlerOffset:
; CHECK-NEXT: .long Ltmp2-_imp_null_check_gep_load ; CHECK-NEXT: .long Ltmp2-_imp_null_check_gep_load
; FunctionAddr:
; CHECK-NEXT: .quad _imp_null_check_hoist_over_unrelated_load
; NumFaultingPCs
; CHECK-NEXT: .long 1
; Reserved:
; CHECK-NEXT: .long 0
; Fault[0].Type:
; CHECK-NEXT: .long 1
; Fault[0].FaultOffset:
; CHECK-NEXT: .long Ltmp7-_imp_null_check_hoist_over_unrelated_load
; Fault[0].HandlerOffset:
; CHECK-NEXT: .long Ltmp6-_imp_null_check_hoist_over_unrelated_load
; FunctionAddr: ; FunctionAddr:
; CHECK-NEXT: .quad _imp_null_check_load ; CHECK-NEXT: .quad _imp_null_check_load
; NumFaultingPCs ; NumFaultingPCs
@ -131,10 +169,12 @@ define i32 @imp_null_check_add_result(i32* %x, i32 %p) {
; OBJDUMP: FaultMap table: ; OBJDUMP: FaultMap table:
; OBJDUMP-NEXT: Version: 0x1 ; OBJDUMP-NEXT: Version: 0x1
; OBJDUMP-NEXT: NumFunctions: 3 ; OBJDUMP-NEXT: NumFunctions: 4
; OBJDUMP-NEXT: FunctionAddress: 0x000000, NumFaultingPCs: 1 ; OBJDUMP-NEXT: FunctionAddress: 0x000000, NumFaultingPCs: 1
; OBJDUMP-NEXT: Fault kind: FaultingLoad, faulting PC offset: 0, handling PC offset: 5 ; OBJDUMP-NEXT: Fault kind: FaultingLoad, faulting PC offset: 0, handling PC offset: 5
; OBJDUMP-NEXT: FunctionAddress: 0x000000, NumFaultingPCs: 1 ; OBJDUMP-NEXT: FunctionAddress: 0x000000, NumFaultingPCs: 1
; OBJDUMP-NEXT: Fault kind: FaultingLoad, faulting PC offset: 0, handling PC offset: 7 ; OBJDUMP-NEXT: Fault kind: FaultingLoad, faulting PC offset: 0, handling PC offset: 7
; OBJDUMP-NEXT: FunctionAddress: 0x000000, NumFaultingPCs: 1 ; OBJDUMP-NEXT: FunctionAddress: 0x000000, NumFaultingPCs: 1
; OBJDUMP-NEXT: Fault kind: FaultingLoad, faulting PC offset: 0, handling PC offset: 7
; OBJDUMP-NEXT: FunctionAddress: 0x000000, NumFaultingPCs: 1
; OBJDUMP-NEXT: Fault kind: FaultingLoad, faulting PC offset: 0, handling PC offset: 3 ; OBJDUMP-NEXT: Fault kind: FaultingLoad, faulting PC offset: 0, handling PC offset: 3