[llvm] r296440 - [ImplicitNullCheck] Add alias analysis usage
Sanjoy Das via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 27 23:04:50 PST 2017
Author: sanjoy
Date: Tue Feb 28 01:04:49 2017
New Revision: 296440
URL: http://llvm.org/viewvc/llvm-project?rev=296440&view=rev
Log:
[ImplicitNullCheck] Add alias analysis usage
Summary:
With this change ImplicitNullCheck optimization uses alias analysis
and can use load/store memory access for implicit null check if there
are other load/store before but memory accesses do not alias.
Patch by Serguei Katkov!
Reviewers: sanjoy
Reviewed By: sanjoy
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D30331
Modified:
llvm/trunk/lib/CodeGen/ImplicitNullChecks.cpp
llvm/trunk/test/CodeGen/X86/implicit-null-checks.mir
Modified: llvm/trunk/lib/CodeGen/ImplicitNullChecks.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/ImplicitNullChecks.cpp?rev=296440&r1=296439&r2=296440&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/ImplicitNullChecks.cpp (original)
+++ llvm/trunk/lib/CodeGen/ImplicitNullChecks.cpp Tue Feb 28 01:04:49 2017
@@ -153,6 +153,7 @@ class ImplicitNullChecks : public Machin
const TargetRegisterInfo *TRI = nullptr;
AliasAnalysis *AA = nullptr;
MachineModuleInfo *MMI = nullptr;
+ MachineFrameInfo *MFI = nullptr;
bool analyzeBlockForNullChecks(MachineBasicBlock &MBB,
SmallVectorImpl<NullCheck> &NullCheckList);
@@ -160,18 +161,29 @@ class ImplicitNullChecks : public Machin
MachineBasicBlock *HandlerMBB);
void rewriteNullChecks(ArrayRef<NullCheck> NullCheckList);
- enum SuitabilityResult { SR_Suitable, SR_Unsuitable, SR_Impossible };
-
+ enum AliasResult {
+ AR_NoAlias,
+ AR_MayAlias,
+ AR_WillAliasEverything
+ };
+ /// Returns AR_NoAlias if \p MI memory operation does not alias with
+ /// \p PrevMI, AR_MayAlias if they may alias and AR_WillAliasEverything if
+ /// they may alias and any further memory operation may alias with \p PrevMI.
+ AliasResult areMemoryOpsAliased(MachineInstr &MI, MachineInstr *PrevMI);
+
+ enum SuitabilityResult {
+ SR_Suitable,
+ SR_Unsuitable,
+ SR_Impossible
+ };
/// Return SR_Suitable if \p MI a memory operation that can be used to
/// implicitly null check the value in \p PointerReg, SR_Unsuitable if
/// \p MI cannot be used to null check and SR_Impossible if there is
/// no sense to continue lookup due to any other instruction will not be able
/// to be used. \p PrevInsts is the set of instruction seen since
- /// the explicit null check on \p PointerReg. \p SeenLoad means that load
- /// instruction has been observed in \PrevInsts set.
+ /// the explicit null check on \p PointerReg.
SuitabilityResult isSuitableMemoryOp(MachineInstr &MI, unsigned PointerReg,
- ArrayRef<MachineInstr *> PrevInsts,
- bool &SeenLoad);
+ ArrayRef<MachineInstr *> PrevInsts);
/// Return true if \p FaultingMI can be hoisted from after the the
/// instructions in \p InstsSeenSoFar to before them. Set \p Dependence to a
@@ -269,6 +281,7 @@ bool ImplicitNullChecks::runOnMachineFun
TII = MF.getSubtarget().getInstrInfo();
TRI = MF.getRegInfo().getTargetRegisterInfo();
MMI = &MF.getMMI();
+ MFI = &MF.getFrameInfo();
AA = &getAnalysis<AAResultsWrapperPass>().getAAResults();
SmallVector<NullCheck, 16> NullCheckList;
@@ -292,47 +305,84 @@ static bool AnyAliasLiveIn(const TargetR
return false;
}
+ImplicitNullChecks::AliasResult
+ImplicitNullChecks::areMemoryOpsAliased(MachineInstr &MI,
+ MachineInstr *PrevMI) {
+ // If it is not memory access, skip the check.
+ if (!(PrevMI->mayStore() || PrevMI->mayLoad()))
+ return AR_NoAlias;
+ // Load-Load may alias
+ if (!(MI.mayStore() || PrevMI->mayStore()))
+ return AR_NoAlias;
+ // We lost info, conservatively alias. If it was store then no sense to
+ // continue because we won't be able to check against it further.
+ if (MI.memoperands_empty())
+ return MI.mayStore() ? AR_WillAliasEverything : AR_MayAlias;
+ if (PrevMI->memoperands_empty())
+ return PrevMI->mayStore() ? AR_WillAliasEverything : AR_MayAlias;
+
+ for (MachineMemOperand *MMO1 : MI.memoperands()) {
+ // MMO1 should have a value due it comes from operation we'd like to use
+ // as implicit null check.
+ assert(MMO1->getValue() && "MMO1 should have a Value!");
+ for (MachineMemOperand *MMO2 : PrevMI->memoperands()) {
+ if (const PseudoSourceValue *PSV = MMO2->getPseudoValue()) {
+ if (PSV->mayAlias(MFI))
+ return AR_MayAlias;
+ continue;
+ }
+ llvm::AliasResult AAResult = AA->alias(
+ MemoryLocation(MMO1->getValue(), MemoryLocation::UnknownSize,
+ MMO1->getAAInfo()),
+ MemoryLocation(MMO2->getValue(), MemoryLocation::UnknownSize,
+ MMO2->getAAInfo()));
+ if (AAResult != NoAlias)
+ return AR_MayAlias;
+ }
+ }
+ return AR_NoAlias;
+}
+
ImplicitNullChecks::SuitabilityResult
ImplicitNullChecks::isSuitableMemoryOp(MachineInstr &MI, unsigned PointerReg,
- ArrayRef<MachineInstr *> PrevInsts,
- bool &SeenLoad) {
+ ArrayRef<MachineInstr *> PrevInsts) {
int64_t Offset;
unsigned BaseReg;
- // First, if it is a store and we saw load before we bail out
- // because we will not be able to re-order load-store without
- // using alias analysis.
- if (SeenLoad && MI.mayStore())
- return SR_Impossible;
-
- SeenLoad = SeenLoad || MI.mayLoad();
-
- // Without alias analysis we cannot re-order store with anything.
- // so if this instruction is not a candidate we should stop.
- SuitabilityResult Unsuitable = MI.mayStore() ? SR_Impossible : SR_Unsuitable;
-
if (!TII->getMemOpBaseRegImmOfs(MI, BaseReg, Offset, TRI) ||
BaseReg != PointerReg)
- return Unsuitable;
+ return SR_Unsuitable;
// We want the mem access to be issued at a sane offset from PointerReg,
// so that if PointerReg is null then the access reliably page faults.
if (!((MI.mayLoad() || MI.mayStore()) && !MI.isPredicable() &&
Offset < PageSize))
- return Unsuitable;
+ return SR_Unsuitable;
// Finally, we need to make sure that the access instruction actually is
// accessing from PointerReg, and there isn't some re-definition of PointerReg
// between the compare and the memory access.
// If PointerReg has been redefined before then there is no sense to continue
// lookup due to this condition will fail for any further instruction.
+ SuitabilityResult Suitable = SR_Suitable;
for (auto *PrevMI : PrevInsts)
- for (auto &PrevMO : PrevMI->operands())
+ for (auto &PrevMO : PrevMI->operands()) {
if (PrevMO.isReg() && PrevMO.getReg() && PrevMO.isDef() &&
TRI->regsOverlap(PrevMO.getReg(), PointerReg))
return SR_Impossible;
- return SR_Suitable;
+ // Check whether the current memory access aliases with previous one.
+ // If we already found that it aliases then no need to continue.
+ // But we continue base pointer check as it can result in SR_Impossible.
+ if (Suitable == SR_Suitable) {
+ AliasResult AR = areMemoryOpsAliased(MI, PrevMI);
+ if (AR == AR_WillAliasEverything)
+ return SR_Impossible;
+ if (AR == AR_MayAlias)
+ Suitable = SR_Unsuitable;
+ }
+ }
+ return Suitable;
}
bool ImplicitNullChecks::canHoistInst(MachineInstr *FaultingMI,
@@ -503,15 +553,13 @@ bool ImplicitNullChecks::analyzeBlockFor
const unsigned PointerReg = MBP.LHS.getReg();
SmallVector<MachineInstr *, 8> InstsSeenSoFar;
- bool SeenLoad = false;
for (auto &MI : *NotNullSucc) {
if (!canHandle(&MI) || InstsSeenSoFar.size() >= MaxInstsToConsider)
return false;
MachineInstr *Dependence;
- SuitabilityResult SR =
- isSuitableMemoryOp(MI, PointerReg, InstsSeenSoFar, SeenLoad);
+ SuitabilityResult SR = isSuitableMemoryOp(MI, PointerReg, InstsSeenSoFar);
if (SR == SR_Impossible)
return false;
if (SR == SR_Suitable &&
Modified: llvm/trunk/test/CodeGen/X86/implicit-null-checks.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/implicit-null-checks.mir?rev=296440&r1=296439&r2=296440&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/X86/implicit-null-checks.mir (original)
+++ llvm/trunk/test/CodeGen/X86/implicit-null-checks.mir Tue Feb 28 01:04:49 2017
@@ -341,6 +341,30 @@
ret void
}
+ define i32 @inc_store_and_load_no_alias(i32* noalias %ptr, i32* noalias %ptr2) {
+ 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
+ }
+
+ define i32 @inc_store_and_load_alias(i32* %ptr, i32* %ptr2) {
+ 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 = !{}
@@ -645,7 +669,7 @@ body: |
name: use_alternate_load_op
# CHECK-LABEL: name: use_alternate_load_op
# CHECK: bb.0.entry:
-# CHECK: %r10 = FAULTING_OP 1, %bb.2.is_null, {{[0-9]+}}, killed %rdi, 1, _, 0, _
+# CHECK: %rax = FAULTING_OP 1, %bb.2.is_null, {{[0-9]+}}, killed %rdi, 1, _, 0, _
# CHECK-NEXT: JMP_1 %bb.1.not_null
# CHECK: bb.1.not_null
@@ -666,9 +690,9 @@ body: |
liveins: %rdi, %rsi
%rcx = MOV64rm killed %rsi, 1, _, 0, _
- %rdx = AND64rm killed %rcx, %rdi, 1, _, 0, _, implicit-def dead %eflags
- %r10 = MOV64rm killed %rdi, 1, _, 0, _
- RETQ %r10d
+ %rcx = AND64rm killed %rcx, %rdi, 1, _, 0, _, implicit-def dead %eflags
+ %rax = MOV64rm killed %rdi, 1, _, 0, _
+ RETQ %eax
bb.2.is_null:
%eax = XOR32rr undef %eax, undef %eax, implicit-def dead %eflags
@@ -1207,3 +1231,69 @@ body: |
RETQ
...
+---
+name: inc_store_and_load_no_alias
+# CHECK-LABEL: inc_store_and_load_no_alias
+# CHECK: bb.0.entry:
+# CHECK: %eax = FAULTING_OP 1, %bb.2.is_null, {{[0-9]+}}, killed %rdi, 1, _, 0, _ :: (load 4 from %ir.ptr)
+# CHECK-NEXT: JMP_1 %bb.1.not_null
+# CHECK: bb.1.not_null
+
+alignment: 4
+tracksRegLiveness: true
+liveins:
+ - { reg: '%rdi' }
+ - { reg: '%rsi' }
+body: |
+ bb.0.entry:
+ successors: %bb.2.is_null, %bb.1.not_null
+ liveins: %rdi, %rsi
+
+ TEST64rr %rdi, %rdi, implicit-def %eflags
+ JE_1 %bb.2.is_null, implicit killed %eflags
+
+ bb.1.not_null:
+ liveins: %rdi, %rsi
+
+ MOV32mi killed %rsi, 1, _, 0, _, 3 :: (store 4 into %ir.ptr2)
+ %eax = MOV32rm killed %rdi, 1, _, 0, _ :: (load 4 from %ir.ptr)
+ RETQ %eax
+
+ bb.2.is_null:
+ %eax = XOR32rr undef %eax, undef %eax, implicit-def dead %eflags
+ RETQ %eax
+
+...
+---
+name: inc_store_and_load_alias
+# CHECK-LABEL: inc_store_and_load_alias
+# 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
+liveins:
+ - { reg: '%rdi' }
+ - { reg: '%rsi' }
+body: |
+ bb.0.entry:
+ successors: %bb.2.is_null, %bb.1.not_null
+ liveins: %rdi, %rsi
+
+ TEST64rr %rdi, %rdi, implicit-def %eflags
+ JE_1 %bb.2.is_null, implicit killed %eflags
+
+ bb.1.not_null:
+ liveins: %rdi, %rsi
+
+ MOV32mi killed %rsi, 1, _, 0, _, 3 :: (store 4 into %ir.ptr2)
+ %eax = MOV32rm killed %rdi, 1, _, 0, _ :: (load 4 from %ir.ptr)
+ RETQ %eax
+
+ bb.2.is_null:
+ %eax = XOR32rr undef %eax, undef %eax, implicit-def dead %eflags
+ RETQ %eax
+
+...
More information about the llvm-commits
mailing list