[PATCH] Allow isDereferencablePointer to look through BitCast ConstantExprs

[Nick Lewycky]

By the way, this isn't related to your patch per se but since you're 
working in the area I thought I should mention it. There's a small 
problem with isDereferenceablePointer and the dereferenceable attribute.

   define void @test(i8* dereferenceable %p) {
     call void @free(i8* %p)
     load i8* %p  ;; isDeferenceablePointer returns true.
     ret void
   }

The langref doesn't specify whether dereferenceable means that it's 
deref'able at the moment of function entry (what I intended) or 
deref'able for the lifetime of the function (what we're currently 
doing). We lower C++ reference types to dereferenceable, and I'm not 
sure that's correct with the second interpretation. I expect this code 
is legal C++:

   void t2(int &i) { free(&i); }
   void t1() { int i = malloc(sizeof(int)); t2(*i); }

Just something to be aware of. If you try to write another patch to 
improve isDereferenceablePointer and code starts miscompiling, this 
mismatch may be why.

Nick

Michael Kuperstein wrote:
> Hi hfinkel,
>
> isDereferencablePointer looks through BitCast instructions, but not constant expressions.
> This fixes PR22460.
>
> I could have both sides of the if use the same code - but it would have to be the one for the ConstantExpr side.
> Is that better?
>
> http://reviews.llvm.org/D7411
>
> Files:
>    lib/IR/Value.cpp
>    test/Transforms/LICM/constexpr.ll
>
> Index: lib/IR/Value.cpp
> ===================================================================
> --- lib/IR/Value.cpp
> +++ lib/IR/Value.cpp
> @@ -495,17 +495,29 @@
>     // to a type of smaller size (or the same size), and the alignment
>     // is at least as large as for the resulting pointer type, then
>     // we can look through the bitcast.
> -  if (DL)
> -    if (const BitCastInst* BC = dyn_cast<BitCastInst>(V)) {
> -      Type *STy = BC->getSrcTy()->getPointerElementType(),
> -           *DTy = BC->getDestTy()->getPointerElementType();
> -      if (STy->isSized()&&  DTy->isSized()&&
> +  if (DL) {
> +    Type *STy = nullptr, *DTy = nullptr;
> +    Value *SrcVal = nullptr;
> +
> +    if (const BitCastInst *BC = dyn_cast<BitCastInst>(V)) {
> +      SrcVal = BC->getOperand(0);
> +      STy = BC->getSrcTy()->getPointerElementType();
> +      DTy = BC->getDestTy()->getPointerElementType();
> +    } else if (const ConstantExpr *CE = dyn_cast<ConstantExpr>(V)) {
> +      if (CE->getOpcode() == Instruction::BitCast) {
> +        SrcVal = CE->getOperand(0);
> +        STy = SrcVal->getType()->getPointerElementType();
> +        DTy = CE->getType()->getPointerElementType();
> +      }
> +    }
> +
> +    if (STy&&  STy->isSized()&&  DTy->isSized()&&
>             (DL->getTypeStoreSize(STy)>=
>              DL->getTypeStoreSize(DTy))&&
>             (DL->getABITypeAlignment(STy)>=
>              DL->getABITypeAlignment(DTy)))
> -        return isDereferenceablePointer(BC->getOperand(0), DL, Visited);
> -    }
> +      return isDereferenceablePointer(SrcVal, DL, Visited);
> +  }
>
>     // Global variables which can't collapse to null are ok.
>     if (const GlobalVariable *GV = dyn_cast<GlobalVariable>(V))
> Index: test/Transforms/LICM/constexpr.ll
> ===================================================================
> --- test/Transforms/LICM/constexpr.ll
> +++ test/Transforms/LICM/constexpr.ll
> @@ -0,0 +1,46 @@
> +; RUN: opt<  %s -S -basicaa -licm | FileCheck %s
> +; This fixes PR22460
> +
> +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
> +target triple = "x86_64-pc-windows-msvc"
> +
> + at in = internal unnamed_addr global i32* null, align 8
> + at out = internal unnamed_addr global i32* null, align 8
> +
> +; CHECK-LABEL: @bar
> +; CHECK: entry:
> +; CHECK: load i64* bitcast (i32** @in to i64*)
> +; CHECK: do.body:
> +; CHECK-NOT: load
> +
> +define i64 @bar(i32 %N) {
> +entry:
> +  br label %do.body
> +
> +do.body:                                          ; preds = %l2, %entry
> +  %i.0 = phi i32 [ 0, %entry ], [ %inc, %l2 ]
> +  %total = phi i64 [ 0, %entry ], [ %next, %l2 ]
> +  %c = icmp eq i32 %N, 6
> +  br i1 %c, label %l1, label %do.body.l2_crit_edge
> +
> +do.body.l2_crit_edge:                             ; preds = %do.body
> +  %inval.pre = load i32** @in, align 8
> +  br label %l2
> +
> +l1:                                               ; preds = %do.body
> +  %v1 = load i64* bitcast (i32** @in to i64*), align 8
> +  store i64 %v1, i64* bitcast (i32** @out to i64*), align 8
> +  %0 = inttoptr i64 %v1 to i32*
> +  br label %l2
> +
> +l2:                                               ; preds = %do.body.l2_crit_edge, %l1
> +  %inval = phi i32* [ %inval.pre, %do.body.l2_crit_edge ], [ %0, %l1 ]
> +  %int = ptrtoint i32* %inval to i64
> +  %next = add i64 %total, %int
> +  %inc = add nsw i32 %i.0, 1
> +  %cmp = icmp slt i32 %inc, %N
> +  br i1 %cmp, label %do.body, label %do.end
> +
> +do.end:                                           ; preds = %l2
> +  ret i64 %total
> +}
>
> EMAIL PREFERENCES
>    http://reviews.llvm.org/settings/panel/emailpreferences/
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits