[InstCombine] Transform !range metadata to !nonnull when combining loads

[arielb1]

Summary

When combining an integer load with !range metadata that does not include 0 to a pointer load, make sure emit !nonnull metadata on the newly-created pointer load. This prevents the !nonnull metadata from being dropped during a ptrtoint/inttoptr pair.

This fixes PR30597

[hfinkel]

lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp

386

What is the:

N->getNumOperands() == 2

supposed to be checking? The range metadata can have any even number of operands; and we already have an assert for this in llvm::getConstantRangeFromMetadata.

387

Indentation looks odd here. Tabs?

test/Transforms/InstCombine/PR30597.ll

24

This is a nice test case, and I'd keep that, but can you also add a more-direct test case involving the range metadata directly?

[arielb1]

Address nits and add test.

[arielb1]

Fix submission errors.

[hfinkel]

Please upload your patches with full context; see http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface for instructions.

lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp

1290

Don't commit this white-space change.

test/Transforms/InstCombine/PR30597-2.ll

1

I don't see any need to have both of these tests in different files. Please put them in the same file.

test/Transforms/InstCombine/PR30597.ll

1

I understand why you want to run -simplifycfg here to cleanup the output of instcombine for the check, but we generally prefer to use the minimal set of passes. Please just run instcombine and check that the branch condition in the output is the appropriate constant.

[majnemer]

lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp

387

Generally, we just say unsigned.

test/Transforms/InstCombine/PR30597-2.ll

7

You should probably pattern match whatever %1 happens to be. We should also match the return.

7

What does !0 correspond to? Please match it.

test/Transforms/InstCombine/PR30597.ll

1

I'd go even further and suggest that the test be amended to return an i1.

[arielb1]

address review comments.

[arielb1]

unsigned int -> unsigned.

lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp

386

Cargo cult copypasta. Will remove

387

Sure.

[majnemer]

LGTM

[majnemer]

Committing to https://llvm.org/svn/llvm-project/llvm/trunk ...
A test/Transforms/InstCombine/PR30597.ll
M lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
Committed r283836

lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp

       break;
     case LLVMContext::MD_range:
       // FIXME: It would be nice to propagate this in some way, but the type
-      // conversions make it hard. If the new type is a pointer, we could
-      // translate it to !nonnull metadata.
+      // conversions make it hard.
+
+      // If it's a pointer now and the range does not contain 0, make it !nonnull.
+      if (NewTy->isPointerTy()) {
+        unsigned int BitWidth = IC.getDataLayout().getTypeSizeInBits(NewTy);
+        if (!getConstantRangeFromMetadata(*N).contains(APInt(BitWidth, 0))) {
+          MDNode *NN = MDNode::get(LI.getContext(), None);
+          NewLoad->setMetadata(LLVMContext::MD_nonnull, NN);
+        }
+      }
       break;
     }
   }

 bool InstCombiner::SimplifyStoreAtEndOfBlock(StoreInst &SI) {
   assert(SI.isUnordered() &&
          "this code has not been auditted for volatile or ordered store case");
-  
   BasicBlock *StoreBB = SI.getParent();
 
   // Check to see if the successor block has exactly two incoming edges.  If

test/Transforms/InstCombine/PR30597-2.ll

+; RUN: opt < %s -instcombine -S | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK-LABEL: @function
+; CHECK-NEXT: %loaded1 = load i64*, i64** %1, align 8, !nonnull !0
+
+; Function Attrs: readonly uwtable
+define i64* @function(i64* noalias nocapture readonly dereferenceable(8)) {
+entry-block:
+  %loaded = load i64, i64* %0, align 8, !range !0
+  %inttoptr = inttoptr i64 %loaded to i64*
+  ret i64* %inttoptr
+}
+
+!0 = !{i64 1, i64 140737488355327}

test/Transforms/InstCombine/PR30597.ll

+; RUN: opt < %s -instcombine -simplifycfg -S | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK-LABEL: @dot_ref_s
+; CHECK-NEXT: ret 42
+
+; Function Attrs: readonly uwtable
+define i32 @dot_ref_s(i32** noalias nocapture readonly dereferenceable(8)) {
+entry-block:
+  %loadedptr = load i32*, i32** %0, align 8, !nonnull !0
+  %ptrtoint = ptrtoint i32* %loadedptr to i64
+  %inttoptr = inttoptr i64 %ptrtoint to i32*
+  %switchtmp = icmp eq i32* %inttoptr, null
+  br i1 %switchtmp, label %bb6, label %bb7
+
+bb6:
+  ret i32 0
+
+bb7:
+  ret i32 42
+}
+
+!0 = !{}