From e1e95c11b8f07686651369b25522557e5ebb6931 Mon Sep 17 00:00:00 2001 From: Sanjoy Das Date: Tue, 30 Jun 2015 21:22:32 +0000 Subject: [PATCH] [FaultMaps] Let the frontend pre-select implicit null check candidates. Summary: This change introduces a !make.implicit metadata that allows the frontend to pre-select the set of explicit null checks that will be considered for transformation into implicit null checks. The reason for not using profiling data instead of !make.implicit is explained in the change to `FaultMaps.rst`. Reviewers: atrick, reames, pgavlin, JosephTremoulet Subscribers: llvm-commits Differential Revision: http://reviews.llvm.org/D10824 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@241116 91177308-0d34-0410-b5e6-96231b3b80d8 --- docs/FaultMaps.rst | 33 ++++++++++++++++++- lib/CodeGen/ImplicitNullChecks.cpp | 7 ++++ .../X86/implicit-null-check-negative.ll | 20 +++++++++-- test/CodeGen/X86/implicit-null-check.ll | 8 +++-- 4 files changed, 62 insertions(+), 6 deletions(-) diff --git a/docs/FaultMaps.rst b/docs/FaultMaps.rst index c69c7bd0110..4ecdd86d769 100644 --- a/docs/FaultMaps.rst +++ b/docs/FaultMaps.rst @@ -64,7 +64,7 @@ checking if a pointer is ``null``, like: %ptr = call i32* @get_ptr() %ptr_is_null = icmp i32* %ptr, null - br i1 %ptr_is_null, label %is_null, label %not_null + br i1 %ptr_is_null, label %is_null, label %not_null, !make.implicit !0 not_null: %t = load i32, i32* %ptr @@ -73,6 +73,8 @@ checking if a pointer is ``null``, like: is_null: call void @HFC() unreachable + + !0 = !{} to control flow implicit in the instruction loading or storing through the pointer being null checked: @@ -94,3 +96,32 @@ level (so the above example is only representative, not literal). The The ``ImplicitNullChecks`` pass adds entries to the ``__llvm_faultmaps`` section described above as needed. + +``make.implicit`` metadata +-------------------------- + +Making null checks implicit is an aggressive optimization, and it can +be a net performance pessimization if too many memory operations end +up faulting because of it. A language runtime typically needs to +ensure that only a negligible number of implicit null checks actually +fault once the application has reached a steady state. A standard way +of doing this is by healing failed implicit null checks into explicit +null checks via code patching or recompilation. It follows that there +are two requirements an explicit null check needs to satisfy for it to +be profitable to convert it to an implicit null check: + + 1. The case where the pointer is actually null (i.e. the "failing" + case) is extremely rare. + + 2. The failing path heals the implicit null check into an explicit + null check so that the application does not repeatedly page + fault. + +The frontend is expected to mark branches that satisfy (1) and (2) +using a ``!make.implicit`` metadata node (the actual content of the +metadata node is ignored). Only branches that are marked with +``!make.implicit`` metadata are considered as candidates for +conversion into implicit null checks. + +(Note that while we could deal with (1) using profiling data, dealing +with (2) requires some information not present in branch profiles.) diff --git a/lib/CodeGen/ImplicitNullChecks.cpp b/lib/CodeGen/ImplicitNullChecks.cpp index d7644a6676c..a02cd67ac64 100644 --- a/lib/CodeGen/ImplicitNullChecks.cpp +++ b/lib/CodeGen/ImplicitNullChecks.cpp @@ -124,6 +124,13 @@ bool ImplicitNullChecks::analyzeBlockForNullChecks( MachineBasicBlock &MBB, SmallVectorImpl &NullCheckList) { typedef TargetInstrInfo::MachineBranchPredicate MachineBranchPredicate; + MDNode *BranchMD = + MBB.getBasicBlock() + ? MBB.getBasicBlock()->getTerminator()->getMetadata("make.implicit") + : nullptr; + if (!BranchMD) + return false; + MachineBranchPredicate MBP; if (TII->AnalyzeBranchPredicate(MBB, MBP, true)) diff --git a/test/CodeGen/X86/implicit-null-check-negative.ll b/test/CodeGen/X86/implicit-null-check-negative.ll index 02c3d6e57d1..8fbed9f7bee 100644 --- a/test/CodeGen/X86/implicit-null-check-negative.ll +++ b/test/CodeGen/X86/implicit-null-check-negative.ll @@ -10,7 +10,7 @@ define i32 @imp_null_check_load(i32* %x, i32* %y) { %c = icmp eq i32* %x, null ; It isn't legal to move the load from %x from "not_null" to here -- ; the store to %y could be aliasing it. - br i1 %c, label %is_null, label %not_null + br i1 %c, label %is_null, label %not_null, !make.implicit !0 is_null: ret i32 42 @@ -24,7 +24,7 @@ define i32 @imp_null_check_load(i32* %x, i32* %y) { define i32 @imp_null_check_gep_load(i32* %x) { entry: %c = icmp eq i32* %x, null - br i1 %c, label %is_null, label %not_null + br i1 %c, label %is_null, label %not_null, !make.implicit !0 is_null: ret i32 42 @@ -36,3 +36,19 @@ define i32 @imp_null_check_gep_load(i32* %x) { %t = load i32, i32* %x.gep ret i32 %t } + +define i32 @imp_null_check_load_no_md(i32* %x) { +; This is fine, except it is missing the !make.implicit metadata. + entry: + %c = icmp eq i32* %x, null + br i1 %c, label %is_null, label %not_null + + is_null: + ret i32 42 + + not_null: + %t = load i32, i32* %x + ret i32 %t +} + +!0 = !{} diff --git a/test/CodeGen/X86/implicit-null-check.ll b/test/CodeGen/X86/implicit-null-check.ll index defd48a781a..1d1b36bbd5d 100644 --- a/test/CodeGen/X86/implicit-null-check.ll +++ b/test/CodeGen/X86/implicit-null-check.ll @@ -21,7 +21,7 @@ define i32 @imp_null_check_load(i32* %x) { entry: %c = icmp eq i32* %x, null - br i1 %c, label %is_null, label %not_null + br i1 %c, label %is_null, label %not_null, !make.implicit !0 is_null: ret i32 42 @@ -42,7 +42,7 @@ define i32 @imp_null_check_gep_load(i32* %x) { entry: %c = icmp eq i32* %x, null - br i1 %c, label %is_null, label %not_null + br i1 %c, label %is_null, label %not_null, !make.implicit !0 is_null: ret i32 42 @@ -65,7 +65,7 @@ define i32 @imp_null_check_add_result(i32* %x, i32 %p) { entry: %c = icmp eq i32* %x, null - br i1 %c, label %is_null, label %not_null + br i1 %c, label %is_null, label %not_null, !make.implicit !0 is_null: ret i32 42 @@ -76,6 +76,8 @@ define i32 @imp_null_check_add_result(i32* %x, i32 %p) { ret i32 %p1 } +!0 = !{} + ; CHECK-LABEL: __LLVM_FaultMaps: ; Version: -- 2.34.1