[PATCH] GVN: Use proper members for accessing into GEPs

[Alp Toker]

For reference, the test case in PR17732 (wrong code at -Os and above)
has been failing since LLVM r168629:

Author: Eli Friedman <eli.friedman at gmail.com>
Date:   Mon Nov 26 23:04:53 2012 +0000

    Get rid of the getPointeeAlignment helper function from
    InstCombineLoadStoreAlloca.cpp, which had many issues.
    (At least two bugs were noted on llvm-commits, and it was overly
conservative.)
    Instead, use getOrEnforceKnownAlignment.

Regards,
Alp.


On 30/10/2013 11:17, David Majnemer wrote:
> Hi nicholas,
>
> GVN wants to take the following global value:
> @main.c = private unnamed_addr constant { [2 x i8], i32, i8, [3 x i8] } { [2 x i8] zeroinitializer, i32 0, i8 1, [3 x i8] undef }, align 4
>
> and index into offset '8' to extract the i8 holding the value 1.
>
> in GetMemInstValueForLoad, we construct the following ConstantExpr:
> i8* getelementptr ({ [2 x i8], i32, i8, [3 x i8] }* @main.c, i32 0, i32 0, i64 8)
>
> This GEP was formed by way of a ConstantExpr::getBitCast which wanted to turn @main.c into an i8*
> The constant folding logic in FoldBitCast decided to reuse the first
> field (which was already an i8*) and form a GEP instead of a bitcast
> instruction.
>
> GetMemInstValueForLoad proceeds to call
> ConstantFoldLoadThroughGEPConstantExpr which walks thru the GEP and
> tries to access the third element of a ConstantAggregateZero which will
> always give you zero.
>
> http://llvm-reviews.chandlerc.com/D2060
>
> Files:
>   lib/Transforms/Scalar/GVN.cpp
>   test/Transforms/GVN/pr17732.ll
>
> Index: lib/Transforms/Scalar/GVN.cpp
> ===================================================================
> --- lib/Transforms/Scalar/GVN.cpp
> +++ lib/Transforms/Scalar/GVN.cpp
> @@ -1256,6 +1256,11 @@
>    ConstantInt::get(Type::getInt64Ty(Src->getContext()), (unsigned)Offset);
>    Src = ConstantExpr::getGetElementPtr(Src, OffsetCst);
>    Src = ConstantExpr::getBitCast(Src, PointerType::getUnqual(LoadTy));
> +  if (ConstantExpr *SrcCE = dyn_cast<ConstantExpr>(Src)) {
> +    Src = ConstantFoldConstantExpression(SrcCE, &TD);
> +    if (!Src)
> +      return 0;
> +  }
>    return ConstantFoldLoadFromConstPtr(Src, &TD);
>  }
>  
> Index: test/Transforms/GVN/pr17732.ll
> ===================================================================
> --- /dev/null
> +++ test/Transforms/GVN/pr17732.ll
> @@ -0,0 +1,21 @@
> +; RUN: opt -gvn -S -o - < %s | FileCheck %s
> +
> +target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
> +target triple = "x86_64-unknown-linux-gnu"
> +
> +%struct.S2 = type { [2 x i8], i32, i8 }
> +
> + at main.c = private unnamed_addr constant { [2 x i8], i32, i8, [3 x i8] } { [2 x i8] zeroinitializer, i32 0, i8 1, [3 x i8] undef }, align 4
> + at b = common global %struct.S2 zeroinitializer, align 4
> +
> +define i32 @main() {
> +entry:
> +  tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* getelementptr inbounds (%struct.S2* @b, i64 0, i32 0, i64 0), i8* getelementptr inbounds ({ [2 x i8], i32, i8, [3 x i8] }* @main.c, i64 0, i32 0, i64 0), i64 12, i32 4, i1 false)
> +  %0 = load i8* getelementptr inbounds (%struct.S2* @b, i64 0, i32 2), align 4
> +  %conv = sext i8 %0 to i32
> +  ret i32 %conv
> +; CHECK-LABEL: define i32 @main(
> +; CHECK: ret i32 1
> +}
> +
> +declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture, i8* nocapture readonly, i64, i32, i1)
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-- 
http://www.nuanti.com
the browser experts