[llvm-commits] [llvm] r160711 - in /llvm/trunk: lib/Analysis/ConstantFolding.cpp test/Transforms/InstCombine/2012-07-25-LoadPart.ll

[Eli Friedman]

On Wed, Jul 25, 2012 at 2:14 AM, Duncan Sands <baldrick at free.fr> wrote:
> Author: baldrick
> Date: Wed Jul 25 04:14:54 2012
> New Revision: 160711
>
> URL: http://llvm.org/viewvc/llvm-project?rev=160711&view=rev
> Log:
> When folding a load from a global constant, if the load started in the middle
> of an array element (rather than at the beginning of the element) and extended
> into the next element, then the load from the second element was being handled
> wrong due to incorrect updating of the notion of which byte to load next.  This
> fixes PR13442.  Thanks to Chris Smowton for reporting the problem, analyzing it
> and providing a fix.
>
> Added:
>     llvm/trunk/test/Transforms/InstCombine/2012-07-25-LoadPart.ll
> Modified:
>     llvm/trunk/lib/Analysis/ConstantFolding.cpp
>
> Modified: llvm/trunk/lib/Analysis/ConstantFolding.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ConstantFolding.cpp?rev=160711&r1=160710&r2=160711&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Analysis/ConstantFolding.cpp (original)
> +++ llvm/trunk/lib/Analysis/ConstantFolding.cpp Wed Jul 25 04:14:54 2012
> @@ -358,17 +358,20 @@
>        NumElts = AT->getNumElements();
>      else
>        NumElts = cast<VectorType>(C->getType())->getNumElements();
> -
> +
>      for (; Index != NumElts; ++Index) {
>        if (!ReadDataFromGlobal(C->getAggregateElement(Index), Offset, CurPtr,
>                                BytesLeft, TD))
>          return false;
> -      if (EltSize >= BytesLeft)
> +
> +      uint64_t BytesWritten = EltSize - Offset;
> +      assert(BytesWritten <= EltSize && "Not indexing into this element?");
> +      if (BytesWritten >= BytesLeft)
>          return true;
> -
> +
>        Offset = 0;
> -      BytesLeft -= EltSize;
> -      CurPtr += EltSize;
> +      BytesLeft -= BytesWritten;
> +      CurPtr += BytesWritten;
>      }
>      return true;
>    }
>
> Added: llvm/trunk/test/Transforms/InstCombine/2012-07-25-LoadPart.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/2012-07-25-LoadPart.ll?rev=160711&view=auto
> ==============================================================================
> --- llvm/trunk/test/Transforms/InstCombine/2012-07-25-LoadPart.ll (added)
> +++ llvm/trunk/test/Transforms/InstCombine/2012-07-25-LoadPart.ll Wed Jul 25 04:14:54 2012
> @@ -0,0 +1,12 @@
> +; RUN: opt < %s -instcombine -S | FileCheck %s
> +; PR13442
> +
> +target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:64:64-v128:128:128-a0:0:64-f80:32:32-n8:16:32-S128"
> +
> + at test = constant [4 x i32] [i32 1, i32 2, i32 3, i32 4]
> +
> +define i64 @foo() {
> +  %ret = load i64* bitcast (i8* getelementptr (i8* bitcast ([4 x i32]* @test to i8*), i64 2) to i64*)
> +  ret i64 %ret
> +  ; CHECK: ret i64 844424930263040
> +}

Can you replace this testcase with one which doesn't have undefined behavior?

-Eli