diff --git a/llvm/include/llvm/CodeGen/TargetInstrInfo.h b/llvm/include/llvm/CodeGen/TargetInstrInfo.h --- a/llvm/include/llvm/CodeGen/TargetInstrInfo.h +++ b/llvm/include/llvm/CodeGen/TargetInstrInfo.h @@ -1270,6 +1270,15 @@ return false; } + /// Returns true if MI's Def is NullValueReg, and the MI + /// does not change the NullValue. i.e. cases such as rax = shr rax, X where + /// NullValueReg = rax. + virtual bool isNullBehaviourUnchanged(const MachineInstr *MI, + const Register NullValueReg, + const TargetRegisterInfo *TRI) const { + return false; + } + /// If the instruction is an increment of a constant value, return the amount. virtual bool getIncrementValue(const MachineInstr &MI, int &Value) const { return false; diff --git a/llvm/lib/CodeGen/ImplicitNullChecks.cpp b/llvm/lib/CodeGen/ImplicitNullChecks.cpp --- a/llvm/lib/CodeGen/ImplicitNullChecks.cpp +++ b/llvm/lib/CodeGen/ImplicitNullChecks.cpp @@ -435,12 +435,6 @@ if (AnyAliasLiveIn(TRI, NullSucc, DependenceMO.getReg())) return true; - // 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(!(DependenceMO.isDef() && - TRI->regsOverlap(DependenceMO.getReg(), PointerReg)) && - "Should have been checked before!"); } // The dependence does not clobber live-ins in NullSucc block. @@ -628,11 +622,9 @@ return true; } - // If MI re-defines the PointerReg then we cannot move further. - if (llvm::any_of(MI.operands(), [&](MachineOperand &MO) { - return MO.isReg() && MO.getReg() && MO.isDef() && - TRI->regsOverlap(MO.getReg(), PointerReg); - })) + // If MI re-defines the PointerReg in a way that changes the value of + // PointerReg if it was null, then we cannot move further. + if (!TII->isNullBehaviourUnchanged(&MI, PointerReg, TRI)) return false; InstsSeenSoFar.push_back(&MI); } diff --git a/llvm/lib/Target/X86/X86InstrInfo.h b/llvm/lib/Target/X86/X86InstrInfo.h --- a/llvm/lib/Target/X86/X86InstrInfo.h +++ b/llvm/lib/Target/X86/X86InstrInfo.h @@ -317,6 +317,10 @@ SmallVectorImpl &Cond, bool AllowModify) const override; + bool isNullBehaviourUnchanged(const MachineInstr *MI, + const Register NullValueReg, + const TargetRegisterInfo *TRI) const override; + bool getMemOperandsWithOffsetWidth( const MachineInstr &LdSt, SmallVectorImpl &BaseOps, int64_t &Offset, diff --git a/llvm/lib/Target/X86/X86InstrInfo.cpp b/llvm/lib/Target/X86/X86InstrInfo.cpp --- a/llvm/lib/Target/X86/X86InstrInfo.cpp +++ b/llvm/lib/Target/X86/X86InstrInfo.cpp @@ -3663,6 +3663,29 @@ } } +bool X86InstrInfo::isNullBehaviourUnchanged( + const MachineInstr *MI, const Register NullValueReg, + const TargetRegisterInfo *TRI) const { + if (!MI->modifiesRegister(NullValueReg, TRI)) + return true; + // Shift right/left of a null is still null. + if (MI->getOpcode() == X86::SHR64ri || MI->getOpcode() == X86::SHR32ri || + MI->getOpcode() == X86::SHL64ri || MI->getOpcode() == X86::SHR64ri) { + auto &MO = MI->getOperand(1); + if (MO.isReg() && MO.getReg() == NullValueReg && MO.isTied()) + return true; + } + // Zero extend of a sub-reg of NullValueReg into itself does not change the + // null value. + if (MI->getOpcode() == X86::MOV32rr) + if (llvm::all_of(MI->operands(), [&](const MachineOperand &MO) { + return TRI->isSubRegisterEq(NullValueReg, MO.getReg()); + })) + return true; + + return false; +} + bool X86InstrInfo::getMemOperandsWithOffsetWidth( const MachineInstr &MemOp, SmallVectorImpl &BaseOps, int64_t &Offset, bool &OffsetIsScalable, unsigned &Width, diff --git a/llvm/test/CodeGen/X86/implicit-null-check.ll b/llvm/test/CodeGen/X86/implicit-null-check.ll --- a/llvm/test/CodeGen/X86/implicit-null-check.ll +++ b/llvm/test/CodeGen/X86/implicit-null-check.ll @@ -48,16 +48,80 @@ ret i32 %t } +; This redefines the null check reg by doing a zero-extend and a shift on +; itself. +; Converted into implicit null check since both of these operations do not +; change the nullness of %x (i.e. if it is null, it remains null). +define i64 @imp_null_check_load_shift_addr(i64* %x) { +; CHECK-LABEL: imp_null_check_load_shift_addr: +; CHECK: ## %bb.0: ## %entry +; CHECK-NEXT: shlq $6, %rdi +; CHECK-NEXT: Ltmp2: +; CHECK-NEXT: movq 8(%rdi), %rax ## on-fault: LBB2_1 +; CHECK-NEXT: ## %bb.2: ## %not_null +; CHECK-NEXT: retq +; CHECK-NEXT: LBB2_1: ## %is_null +; CHECK-NEXT: movl $42, %eax +; CHECK-NEXT: retq + + entry: + %c = icmp eq i64* %x, null + br i1 %c, label %is_null, label %not_null, !make.implicit !0 + + is_null: + ret i64 42 + + not_null: + %y = ptrtoint i64* %x to i64 + %shry = shl i64 %y, 6 + %y.ptr = inttoptr i64 %shry to i64* + %x.loc = getelementptr i64, i64* %y.ptr, i64 1 + %t = load i64, i64* %x.loc + ret i64 %t +} + +; Same as imp_null_check_load_shift_addr but shift is by 3 and this is now +; converted into an LEA. +; TODO: Can be converted into implicit null check +define i64 @imp_null_check_load_shift_by_3_addr(i64* %x) { +; CHECK-LABEL: imp_null_check_load_shift_by_3_addr: +; CHECK: ## %bb.0: ## %entry +; CHECK-NEXT: testq %rdi, %rdi +; CHECK-NEXT: je LBB3_1 +; CHECK-NEXT: ## %bb.2: ## %not_null +; CHECK-NEXT: movq 8(,%rdi,8), %rax +; CHECK-NEXT: retq +; CHECK-NEXT: LBB3_1: ## %is_null +; CHECK-NEXT: movl $42, %eax +; CHECK-NEXT: retq + + entry: + %c = icmp eq i64* %x, null + br i1 %c, label %is_null, label %not_null, !make.implicit !0 + + is_null: + ret i64 42 + + not_null: + %y = ptrtoint i64* %x to i64 + %shry = shl i64 %y, 3 + %y.ptr = inttoptr i64 %shry to i64* + %x.loc = getelementptr i64, i64* %y.ptr, i64 1 + %t = load i64, i64* %x.loc + ret i64 %t +} + +; TODO: Can be converted into implicit check. ;; Probably could be implicit, but we're conservative for now define i32 @imp_null_check_seq_cst_load(i32* %x) { ; CHECK-LABEL: imp_null_check_seq_cst_load: ; CHECK: ## %bb.0: ## %entry ; CHECK-NEXT: testq %rdi, %rdi -; CHECK-NEXT: je LBB2_1 +; CHECK-NEXT: je LBB4_1 ; CHECK-NEXT: ## %bb.2: ## %not_null ; CHECK-NEXT: movl (%rdi), %eax ; CHECK-NEXT: retq -; CHECK-NEXT: LBB2_1: ## %is_null +; CHECK-NEXT: LBB4_1: ## %is_null ; CHECK-NEXT: movl $42, %eax ; CHECK-NEXT: retq @@ -78,11 +142,11 @@ ; CHECK-LABEL: imp_null_check_volatile_load: ; CHECK: ## %bb.0: ## %entry ; CHECK-NEXT: testq %rdi, %rdi -; CHECK-NEXT: je LBB3_1 +; CHECK-NEXT: je LBB5_1 ; CHECK-NEXT: ## %bb.2: ## %not_null ; CHECK-NEXT: movl (%rdi), %eax ; CHECK-NEXT: retq -; CHECK-NEXT: LBB3_1: ## %is_null +; CHECK-NEXT: LBB5_1: ## %is_null ; CHECK-NEXT: movl $42, %eax ; CHECK-NEXT: retq @@ -102,11 +166,11 @@ define i8 @imp_null_check_load_i8(i8* %x) { ; CHECK-LABEL: imp_null_check_load_i8: ; CHECK: ## %bb.0: ## %entry -; CHECK-NEXT: Ltmp2: -; CHECK-NEXT: movb (%rdi), %al ## on-fault: LBB4_1 +; CHECK-NEXT: Ltmp3: +; CHECK-NEXT: movb (%rdi), %al ## on-fault: LBB6_1 ; CHECK-NEXT: ## %bb.2: ## %not_null ; CHECK-NEXT: retq -; CHECK-NEXT: LBB4_1: ## %is_null +; CHECK-NEXT: LBB6_1: ## %is_null ; CHECK-NEXT: movb $42, %al ; CHECK-NEXT: retq @@ -126,8 +190,8 @@ ; CHECK-LABEL: imp_null_check_load_i256: ; CHECK: ## %bb.0: ## %entry ; CHECK-NEXT: movq %rdi, %rax -; CHECK-NEXT: Ltmp3: -; CHECK-NEXT: movq (%rsi), %rcx ## on-fault: LBB5_1 +; CHECK-NEXT: Ltmp4: +; CHECK-NEXT: movq (%rsi), %rcx ## on-fault: LBB7_1 ; CHECK-NEXT: ## %bb.2: ## %not_null ; CHECK-NEXT: movq 8(%rsi), %rdx ; CHECK-NEXT: movq 16(%rsi), %rdi @@ -137,7 +201,7 @@ ; CHECK-NEXT: movq %rdx, 8(%rax) ; CHECK-NEXT: movq %rcx, (%rax) ; CHECK-NEXT: retq -; CHECK-NEXT: LBB5_1: ## %is_null +; CHECK-NEXT: LBB7_1: ## %is_null ; CHECK-NEXT: movq $0, 24(%rax) ; CHECK-NEXT: movq $0, 16(%rax) ; CHECK-NEXT: movq $0, 8(%rax) @@ -161,11 +225,11 @@ define i32 @imp_null_check_gep_load(i32* %x) { ; CHECK-LABEL: imp_null_check_gep_load: ; CHECK: ## %bb.0: ## %entry -; CHECK-NEXT: Ltmp4: -; CHECK-NEXT: movl 128(%rdi), %eax ## on-fault: LBB6_1 +; CHECK-NEXT: Ltmp5: +; CHECK-NEXT: movl 128(%rdi), %eax ## on-fault: LBB8_1 ; CHECK-NEXT: ## %bb.2: ## %not_null ; CHECK-NEXT: retq -; CHECK-NEXT: LBB6_1: ## %is_null +; CHECK-NEXT: LBB8_1: ## %is_null ; CHECK-NEXT: movl $42, %eax ; CHECK-NEXT: retq @@ -185,12 +249,12 @@ define i32 @imp_null_check_add_result(i32* %x, i32 %p) { ; CHECK-LABEL: imp_null_check_add_result: ; CHECK: ## %bb.0: ## %entry -; CHECK-NEXT: Ltmp5: -; CHECK-NEXT: addl (%rdi), %esi ## on-fault: LBB7_1 +; CHECK-NEXT: Ltmp6: +; CHECK-NEXT: addl (%rdi), %esi ## on-fault: LBB9_1 ; CHECK-NEXT: ## %bb.2: ## %not_null ; CHECK-NEXT: movl %esi, %eax ; CHECK-NEXT: retq -; CHECK-NEXT: LBB7_1: ## %is_null +; CHECK-NEXT: LBB9_1: ## %is_null ; CHECK-NEXT: movl $42, %eax ; CHECK-NEXT: retq @@ -210,12 +274,12 @@ define i32 @imp_null_check_sub_result(i32* %x, i32 %p) { ; CHECK-LABEL: imp_null_check_sub_result: ; CHECK: ## %bb.0: ## %entry -; CHECK-NEXT: Ltmp6: -; CHECK-NEXT: movl (%rdi), %eax ## on-fault: LBB8_1 +; CHECK-NEXT: Ltmp7: +; CHECK-NEXT: movl (%rdi), %eax ## on-fault: LBB10_1 ; CHECK-NEXT: ## %bb.2: ## %not_null ; CHECK-NEXT: subl %esi, %eax ; CHECK-NEXT: retq -; CHECK-NEXT: LBB8_1: ## %is_null +; CHECK-NEXT: LBB10_1: ## %is_null ; CHECK-NEXT: movl $42, %eax ; CHECK-NEXT: retq @@ -235,12 +299,12 @@ define i32 @imp_null_check_mul_result(i32* %x, i32 %p) { ; CHECK-LABEL: imp_null_check_mul_result: ; CHECK: ## %bb.0: ## %entry -; CHECK-NEXT: Ltmp7: -; CHECK-NEXT: imull (%rdi), %esi ## on-fault: LBB9_1 +; CHECK-NEXT: Ltmp8: +; CHECK-NEXT: imull (%rdi), %esi ## on-fault: LBB11_1 ; CHECK-NEXT: ## %bb.2: ## %not_null ; CHECK-NEXT: movl %esi, %eax ; CHECK-NEXT: retq -; CHECK-NEXT: LBB9_1: ## %is_null +; CHECK-NEXT: LBB11_1: ## %is_null ; CHECK-NEXT: movl $42, %eax ; CHECK-NEXT: retq @@ -260,13 +324,13 @@ define i32 @imp_null_check_udiv_result(i32* %x, i32 %p) { ; CHECK-LABEL: imp_null_check_udiv_result: ; CHECK: ## %bb.0: ## %entry -; CHECK-NEXT: Ltmp8: -; CHECK-NEXT: movl (%rdi), %eax ## on-fault: LBB10_1 +; CHECK-NEXT: Ltmp9: +; CHECK-NEXT: movl (%rdi), %eax ## on-fault: LBB12_1 ; CHECK-NEXT: ## %bb.2: ## %not_null ; CHECK-NEXT: xorl %edx, %edx ; CHECK-NEXT: divl %esi ; CHECK-NEXT: retq -; CHECK-NEXT: LBB10_1: ## %is_null +; CHECK-NEXT: LBB12_1: ## %is_null ; CHECK-NEXT: movl $42, %eax ; CHECK-NEXT: retq @@ -286,13 +350,13 @@ define i32 @imp_null_check_shl_result(i32* %x, i32 %p) { ; CHECK-LABEL: imp_null_check_shl_result: ; CHECK: ## %bb.0: ## %entry -; CHECK-NEXT: Ltmp9: -; CHECK-NEXT: movl (%rdi), %eax ## on-fault: LBB11_1 +; CHECK-NEXT: Ltmp10: +; CHECK-NEXT: movl (%rdi), %eax ## on-fault: LBB13_1 ; CHECK-NEXT: ## %bb.2: ## %not_null ; CHECK-NEXT: movl %esi, %ecx ; CHECK-NEXT: shll %cl, %eax ; CHECK-NEXT: retq -; CHECK-NEXT: LBB11_1: ## %is_null +; CHECK-NEXT: LBB13_1: ## %is_null ; CHECK-NEXT: movl $42, %eax ; CHECK-NEXT: retq @@ -312,13 +376,13 @@ define i32 @imp_null_check_lshr_result(i32* %x, i32 %p) { ; CHECK-LABEL: imp_null_check_lshr_result: ; CHECK: ## %bb.0: ## %entry -; CHECK-NEXT: Ltmp10: -; CHECK-NEXT: movl (%rdi), %eax ## on-fault: LBB12_1 +; CHECK-NEXT: Ltmp11: +; CHECK-NEXT: movl (%rdi), %eax ## on-fault: LBB14_1 ; CHECK-NEXT: ## %bb.2: ## %not_null ; CHECK-NEXT: movl %esi, %ecx ; CHECK-NEXT: shrl %cl, %eax ; CHECK-NEXT: retq -; CHECK-NEXT: LBB12_1: ## %is_null +; CHECK-NEXT: LBB14_1: ## %is_null ; CHECK-NEXT: movl $42, %eax ; CHECK-NEXT: retq @@ -341,13 +405,13 @@ 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: ## %bb.0: ## %entry -; CHECK-NEXT: Ltmp11: -; CHECK-NEXT: movl (%rdi), %eax ## on-fault: LBB13_1 +; CHECK-NEXT: Ltmp12: +; CHECK-NEXT: movl (%rdi), %eax ## on-fault: LBB15_1 ; CHECK-NEXT: ## %bb.2: ## %not_null ; CHECK-NEXT: movl (%rsi), %ecx ; CHECK-NEXT: movl %ecx, (%rdx) ; CHECK-NEXT: retq -; CHECK-NEXT: LBB13_1: ## %is_null +; CHECK-NEXT: LBB15_1: ## %is_null ; CHECK-NEXT: movl $42, %eax ; CHECK-NEXT: retq @@ -368,17 +432,17 @@ define i32 @imp_null_check_via_mem_comparision(i32* %x, i32 %val) { ; CHECK-LABEL: imp_null_check_via_mem_comparision: ; CHECK: ## %bb.0: ## %entry -; CHECK-NEXT: Ltmp12: -; CHECK-NEXT: cmpl %esi, 4(%rdi) ## on-fault: LBB14_3 +; CHECK-NEXT: Ltmp13: +; CHECK-NEXT: cmpl %esi, 4(%rdi) ## on-fault: LBB16_3 ; CHECK-NEXT: ## %bb.1: ## %not_null -; CHECK-NEXT: jge LBB14_2 +; CHECK-NEXT: jge LBB16_2 ; CHECK-NEXT: ## %bb.4: ## %ret_100 ; CHECK-NEXT: movl $100, %eax ; CHECK-NEXT: retq -; CHECK-NEXT: LBB14_3: ## %is_null +; CHECK-NEXT: LBB16_3: ## %is_null ; CHECK-NEXT: movl $42, %eax ; CHECK-NEXT: retq -; CHECK-NEXT: LBB14_2: ## %ret_200 +; CHECK-NEXT: LBB16_2: ## %ret_200 ; CHECK-NEXT: movl $200, %eax ; CHECK-NEXT: retq @@ -406,13 +470,13 @@ ; CHECK-LABEL: imp_null_check_gep_load_with_use_dep: ; CHECK: ## %bb.0: ## %entry ; CHECK-NEXT: ## kill: def $esi killed $esi def $rsi -; CHECK-NEXT: Ltmp13: -; CHECK-NEXT: movl (%rdi), %eax ## on-fault: LBB15_1 +; CHECK-NEXT: Ltmp14: +; CHECK-NEXT: movl (%rdi), %eax ## on-fault: LBB17_1 ; CHECK-NEXT: ## %bb.2: ## %not_null ; CHECK-NEXT: addl %edi, %esi ; CHECK-NEXT: leal 4(%rax,%rsi), %eax ; CHECK-NEXT: retq -; CHECK-NEXT: LBB15_1: ## %is_null +; CHECK-NEXT: LBB17_1: ## %is_null ; CHECK-NEXT: movl $42, %eax ; CHECK-NEXT: retq @@ -438,12 +502,12 @@ ; CHECK-LABEL: imp_null_check_load_fence1: ; CHECK: ## %bb.0: ## %entry ; CHECK-NEXT: testq %rdi, %rdi -; CHECK-NEXT: je LBB16_1 +; CHECK-NEXT: je LBB18_1 ; CHECK-NEXT: ## %bb.2: ## %not_null ; CHECK-NEXT: ##MEMBARRIER ; CHECK-NEXT: movl (%rdi), %eax ; CHECK-NEXT: retq -; CHECK-NEXT: LBB16_1: ## %is_null +; CHECK-NEXT: LBB18_1: ## %is_null ; CHECK-NEXT: movl $42, %eax ; CHECK-NEXT: retq @@ -466,12 +530,12 @@ ; CHECK-LABEL: imp_null_check_load_fence2: ; CHECK: ## %bb.0: ## %entry ; CHECK-NEXT: testq %rdi, %rdi -; CHECK-NEXT: je LBB17_1 +; CHECK-NEXT: je LBB19_1 ; CHECK-NEXT: ## %bb.2: ## %not_null ; CHECK-NEXT: mfence ; CHECK-NEXT: movl (%rdi), %eax ; CHECK-NEXT: retq -; CHECK-NEXT: LBB17_1: ## %is_null +; CHECK-NEXT: LBB19_1: ## %is_null ; CHECK-NEXT: movl $42, %eax ; CHECK-NEXT: retq @@ -491,11 +555,11 @@ define void @imp_null_check_store(i32* %x) { ; CHECK-LABEL: imp_null_check_store: ; CHECK: ## %bb.0: ## %entry -; CHECK-NEXT: Ltmp14: -; CHECK-NEXT: movl $1, (%rdi) ## on-fault: LBB18_1 +; CHECK-NEXT: Ltmp15: +; CHECK-NEXT: movl $1, (%rdi) ## on-fault: LBB20_1 ; CHECK-NEXT: ## %bb.2: ## %not_null ; CHECK-NEXT: retq -; CHECK-NEXT: LBB18_1: ## %is_null +; CHECK-NEXT: LBB20_1: ## %is_null ; CHECK-NEXT: retq entry: @@ -514,11 +578,11 @@ define void @imp_null_check_unordered_store(i32* %x) { ; CHECK-LABEL: imp_null_check_unordered_store: ; CHECK: ## %bb.0: ## %entry -; CHECK-NEXT: Ltmp15: -; CHECK-NEXT: movl $1, (%rdi) ## on-fault: LBB19_1 +; CHECK-NEXT: Ltmp16: +; CHECK-NEXT: movl $1, (%rdi) ## on-fault: LBB21_1 ; CHECK-NEXT: ## %bb.2: ## %not_null ; CHECK-NEXT: retq -; CHECK-NEXT: LBB19_1: ## %is_null +; CHECK-NEXT: LBB21_1: ## %is_null ; CHECK-NEXT: retq entry: @@ -536,11 +600,11 @@ define i32 @imp_null_check_neg_gep_load(i32* %x) { ; CHECK-LABEL: imp_null_check_neg_gep_load: ; CHECK: ## %bb.0: ## %entry -; CHECK-NEXT: Ltmp16: -; CHECK-NEXT: movl -128(%rdi), %eax ## on-fault: LBB20_1 +; CHECK-NEXT: Ltmp17: +; CHECK-NEXT: movl -128(%rdi), %eax ## on-fault: LBB22_1 ; CHECK-NEXT: ## %bb.2: ## %not_null ; CHECK-NEXT: retq -; CHECK-NEXT: LBB20_1: ## %is_null +; CHECK-NEXT: LBB22_1: ## %is_null ; CHECK-NEXT: movl $42, %eax ; CHECK-NEXT: retq