[FaultMaps] Let the frontend pre-select implicit null check candidates.
authorSanjoy Das <sanjoy@playingwithpointers.com>
Tue, 30 Jun 2015 21:22:32 +0000 (21:22 +0000)
committerSanjoy Das <sanjoy@playingwithpointers.com>
Tue, 30 Jun 2015 21:22:32 +0000 (21:22 +0000)
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

llvm-svn: 241116

llvm/docs/FaultMaps.rst
llvm/lib/CodeGen/ImplicitNullChecks.cpp
llvm/test/CodeGen/X86/implicit-null-check-negative.ll
llvm/test/CodeGen/X86/implicit-null-check.ll

index c69c7bd..4ecdd86 100644 (file)
@@ -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.)
index d7644a6..a02cd67 100644 (file)
@@ -124,6 +124,13 @@ bool ImplicitNullChecks::analyzeBlockForNullChecks(
     MachineBasicBlock &MBB, SmallVectorImpl<NullCheck> &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))
index 02c3d6e..8fbed9f 100644 (file)
@@ -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 = !{}
index defd48a..1d1b36b 100644 (file)
@@ -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: