From 2e079eaf6b847ef1ffbd39704e7a1f027eacea2d Mon Sep 17 00:00:00 2001 From: Sanjoy Das Date: Wed, 1 Feb 2017 16:04:21 +0000 Subject: [PATCH] [ImplicitNullCheck] Extend canReorder scope Summary: This change allows a re-order of two intructions if their uses are overlapped. Patch by Serguei Katkov! Reviewers: reames, sanjoy Reviewed By: sanjoy Subscribers: llvm-commits Differential Revision: https://reviews.llvm.org/D29120 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@293775 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/ImplicitNullChecks.cpp | 7 +- test/CodeGen/X86/implicit-null-check.ll | 46 ++++++++++- test/CodeGen/X86/implicit-null-checks.mir | 96 ++++++++++++++++++++++- 3 files changed, 142 insertions(+), 7 deletions(-) diff --git a/lib/CodeGen/ImplicitNullChecks.cpp b/lib/CodeGen/ImplicitNullChecks.cpp index 3a0551ed034..0a9b9a91fe9 100644 --- a/lib/CodeGen/ImplicitNullChecks.cpp +++ b/lib/CodeGen/ImplicitNullChecks.cpp @@ -253,7 +253,7 @@ bool ImplicitNullChecks::canReorder(const MachineInstr *A, unsigned RegB = MOB.getReg(); - if (TRI->regsOverlap(RegA, RegB)) + if (TRI->regsOverlap(RegA, RegB) && (MOA.isDef() || MOB.isDef())) return false; } } @@ -310,7 +310,7 @@ ImplicitNullChecks::isSuitableMemoryOp(MachineInstr &MI, unsigned PointerReg, // lookup due to this condition will fail for any further instruction. for (auto *PrevMI : PrevInsts) for (auto &PrevMO : PrevMI->operands()) - if (PrevMO.isReg() && PrevMO.getReg() && + if (PrevMO.isReg() && PrevMO.getReg() && PrevMO.isDef() && TRI->regsOverlap(PrevMO.getReg(), PointerReg)) return SR_Impossible; @@ -367,7 +367,8 @@ bool ImplicitNullChecks::canHoistLoadInst( // The Dependency can't be re-defining the base register -- then we won't // get the memory operation on the address we want. This is already // checked in \c IsSuitableMemoryOp. - assert(!TRI->regsOverlap(DependenceMO.getReg(), PointerReg) && + assert(!(DependenceMO.isDef() && + TRI->regsOverlap(DependenceMO.getReg(), PointerReg)) && "Should have been checked before!"); } diff --git a/test/CodeGen/X86/implicit-null-check.ll b/test/CodeGen/X86/implicit-null-check.ll index 9a8a3a4369d..286b4fa7a81 100644 --- a/test/CodeGen/X86/implicit-null-check.ll +++ b/test/CodeGen/X86/implicit-null-check.ll @@ -135,6 +135,33 @@ define i32 @imp_null_check_via_mem_comparision(i32* %x, i32 %val) { ret i32 200 } +define i32 @imp_null_check_gep_load_with_use_dep(i32* %x, i32 %a) { +; CHECK-LABEL: imp_null_check_gep_load_with_use_dep: +; CHECK: [[BB0_imp_null_check_gep_load_with_use_dep:L[^:]+]]: +; CHECK: movl (%rdi), %eax +; CHECK: addl %edi, %esi +; CHECK: leal 4(%rax,%rsi), %eax +; CHECK: retq +; CHECK: [[BB1_imp_null_check_gep_load_with_use_dep:LBB5_[0-9]+]]: +; 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: + %x.loc = getelementptr i32, i32* %x, i32 1 + %y = ptrtoint i32* %x.loc to i32 + %b = add i32 %a, %y + %t = load i32, i32* %x + %z = add i32 %t, %b + ret i32 %z +} + !0 = !{} ; CHECK-LABEL: __LLVM_FaultMaps: @@ -147,7 +174,7 @@ define i32 @imp_null_check_via_mem_comparision(i32* %x, i32 %val) { ; CHECK-NEXT: .short 0 ; # functions: -; CHECK-NEXT: .long 5 +; CHECK-NEXT: .long 6 ; FunctionAddr: ; CHECK-NEXT: .quad _imp_null_check_add_result @@ -175,6 +202,19 @@ define i32 @imp_null_check_via_mem_comparision(i32* %x, i32 %val) { ; Fault[0].HandlerOffset: ; CHECK-NEXT: .long [[BB1_imp_null_check_gep_load]]-_imp_null_check_gep_load +; FunctionAddr: +; CHECK-NEXT: .quad _imp_null_check_gep_load_with_use_dep +; NumFaultingPCs +; CHECK-NEXT: .long 1 +; Reserved: +; CHECK-NEXT: .long 0 +; Fault[0].Type: +; CHECK-NEXT: .long 1 +; Fault[0].FaultOffset: +; CHECK-NEXT: .long [[BB0_imp_null_check_gep_load_with_use_dep]]-_imp_null_check_gep_load_with_use_dep +; Fault[0].HandlerOffset: +; CHECK-NEXT: .long [[BB1_imp_null_check_gep_load_with_use_dep]]-_imp_null_check_gep_load_with_use_dep + ; FunctionAddr: ; CHECK-NEXT: .quad _imp_null_check_hoist_over_unrelated_load ; NumFaultingPCs @@ -216,12 +256,14 @@ define i32 @imp_null_check_via_mem_comparision(i32* %x, i32 %val) { ; OBJDUMP: FaultMap table: ; OBJDUMP-NEXT: Version: 0x1 -; OBJDUMP-NEXT: NumFunctions: 5 +; OBJDUMP-NEXT: NumFunctions: 6 ; OBJDUMP-NEXT: FunctionAddress: 0x000000, NumFaultingPCs: 1 ; OBJDUMP-NEXT: Fault kind: FaultingLoad, faulting PC offset: 0, handling PC offset: 5 ; 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: 9 +; 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 diff --git a/test/CodeGen/X86/implicit-null-checks.mir b/test/CodeGen/X86/implicit-null-checks.mir index d2a9e5e50a2..af9758fc8ad 100644 --- a/test/CodeGen/X86/implicit-null-checks.mir +++ b/test/CodeGen/X86/implicit-null-checks.mir @@ -145,6 +145,35 @@ attributes #0 = { "target-features"="+bmi,+bmi2" } + define i32 @imp_null_check_gep_load_with_use_dep(i32* %x, i32 %a) { + entry: + %c = icmp eq i32* %x, null + br i1 %c, label %is_null, label %not_null, !make.implicit !0 + + is_null: ; preds = %entry + ret i32 42 + + not_null: ; preds = %entry + %x.loc = getelementptr i32, i32* %x, i32 1 + %y = ptrtoint i32* %x.loc to i32 + %b = add i32 %a, %y + %t = load i32, i32* %x + %z = add i32 %t, %b + ret i32 %z + } + + define i32 @imp_null_check_load_with_base_sep(i32* %x, i32 %a) { + entry: + %c = icmp eq i32* %x, null + br i1 %c, label %is_null, label %not_null, !make.implicit !0 + + is_null: ; preds = %entry + ret i32 42 + + not_null: ; preds = %entry + ret i32 undef + } + !0 = !{} ... --- @@ -447,8 +476,8 @@ body: | name: use_alternate_load_op # CHECK-LABEL: use_alternate_load_op # CHECK: bb.0.entry: -# CHECK: TEST64rr %rdi, %rdi, implicit-def %eflags -# CHECK-NEXT: JE_1 %bb.2.is_null, implicit killed %eflags +# CHECK: %r10 = FAULTING_LOAD_OP %bb.2.is_null, {{[0-9]+}}, killed %rdi, 1, _, 0, _ +# CHECK-NEXT: JMP_1 %bb.1.not_null # CHECK: bb.1.not_null alignment: 4 @@ -477,3 +506,66 @@ body: | RETQ %eax ... +--- +name: imp_null_check_gep_load_with_use_dep +# CHECK: bb.0.entry: +# CHECK: %eax = FAULTING_LOAD_OP %bb.2.is_null, {{[0-9]+}}, killed %rdi, 1, _, 0, _, implicit-def %rax :: (load 4 from %ir.x) +# CHECK-NEXT: JMP_1 %bb.1.not_null +alignment: 4 +tracksRegLiveness: true +liveins: + - { reg: '%rdi' } + - { reg: '%rsi' } +body: | + bb.0.entry: + successors: %bb.1.is_null(0x30000000), %bb.2.not_null(0x50000000) + liveins: %rsi, %rdi + + TEST64rr %rdi, %rdi, implicit-def %eflags + JE_1 %bb.1.is_null, implicit %eflags + + bb.2.not_null: + liveins: %rdi, %rsi + + %rsi = ADD64rr %rsi, %rdi, implicit-def dead %eflags + %eax = MOV32rm killed %rdi, 1, _, 0, _, implicit-def %rax :: (load 4 from %ir.x) + %eax = LEA64_32r killed %rax, 1, killed %rsi, 4, _ + RETQ %eax + + bb.1.is_null: + %eax = MOV32ri 42 + RETQ %eax + +... +--- +name: imp_null_check_load_with_base_sep +# CHECK: bb.0.entry: +# CHECK: %rsi = ADD64rr %rsi, %rdi, implicit-def dead %eflags +# CHECK-NEXT: %esi = FAULTING_LOAD_OP %bb.2.is_null, {{[0-9]+}}, killed %esi, %rdi, 1, _, 0, _, implicit-def dead %eflags +# CHECK-NEXT: JMP_1 %bb.1.not_null +alignment: 4 +tracksRegLiveness: true +liveins: + - { reg: '%rdi' } + - { reg: '%rsi' } +body: | + bb.0.entry: + successors: %bb.1.is_null(0x30000000), %bb.2.not_null(0x50000000) + liveins: %rsi, %rdi + + TEST64rr %rdi, %rdi, implicit-def %eflags + JE_1 %bb.1.is_null, implicit %eflags + + bb.2.not_null: + liveins: %rdi, %rsi + + %rsi = ADD64rr %rsi, %rdi, implicit-def dead %eflags + %esi = AND32rm killed %esi, %rdi, 1, _, 0, _, implicit-def dead %eflags + %eax = MOV32rr %esi + RETQ %eax + + bb.1.is_null: + %eax = MOV32ri 42 + RETQ %eax + +... -- 2.50.1