[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



REPOSITORY
  rL LLVM

http://reviews.llvm.org/D7411

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/