[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

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@241116 91177308-0d34-0410-b5e6-96231b3b80d8

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

index c69c7bd01101b51edd379d6bd3d3ad833c8a530c..4ecdd86d7693c4bb31b38630176200c6cd757389 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 d7644a6676c6a1cd4af37fb2be3322fa584f69d2..a02cd67ac649e242ba188599b3b5b65069787125 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 02c3d6e57d19432b7721e60289f4606b30304b76..8fbed9f7bee85c785fb217a7a5f7fa194f250e63 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 defd48a781a878e931b55b8b5e506f2fd6dc0d2c..1d1b36bbd5d060402c715593c8a5419842769090 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: