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

[David Majnemer via llvm-commits]

majnemer added inline comments.


> InstCombineLoadStoreAlloca.cpp:387
> +      if (NewTy->isPointerTy()) {
> +        unsigned int BitWidth = IC.getDataLayout().getTypeSizeInBits(NewTy);
> +        if (!getConstantRangeFromMetadata(*N).contains(APInt(BitWidth, 0))) {

Generally, we just say `unsigned`.

> PR30597-2.ll:7
> +; CHECK-LABEL: @function
> +; CHECK-NEXT: %loaded1 = load i64*, i64** %1, align 8, !nonnull !0
> +

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

> PR30597-2.ll:7
> +; CHECK-LABEL: @function
> +; CHECK-NEXT: %loaded1 = load i64*, i64** %1, align 8, !nonnull !0
> +

What does !0 correspond to? Please match it.

> hfinkel wrote in 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.

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

Repository:
  rL LLVM

https://reviews.llvm.org/D25215