From: Eli Friedman Date: Fri, 24 May 2019 01:03:51 +0000 (+0000) Subject: Revert r361460 X-Git-Tag: llvmorg-10-init~4711 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=052f87ae36163cbd2033617eba655af7f1438733;p=platform%2Fupstream%2Fllvm.git Revert r361460 It regresses https://bugs.llvm.org/show_bug.cgi?id=38309 (represented by the testcase test/Transforms/GlobalOpt/globalsra-multigep.ll). llvm-svn: 361581 --- diff --git a/llvm/lib/Transforms/IPO/GlobalOpt.cpp b/llvm/lib/Transforms/IPO/GlobalOpt.cpp index c4f268a..c4fb3ce 100644 --- a/llvm/lib/Transforms/IPO/GlobalOpt.cpp +++ b/llvm/lib/Transforms/IPO/GlobalOpt.cpp @@ -184,7 +184,7 @@ static bool IsSafeComputationToRemove(Value *V, const TargetLibraryInfo *TLI) { /// This GV is a pointer root. Loop over all users of the global and clean up /// any that obviously don't assign the global a value that isn't dynamically /// allocated. -static bool CleanupPointerRootUsers(Value *V, +static bool CleanupPointerRootUsers(GlobalVariable *GV, const TargetLibraryInfo *TLI) { // A brief explanation of leak checkers. The goal is to find bugs where // pointers are forgotten, causing an accumulating growth in memory @@ -202,7 +202,7 @@ static bool CleanupPointerRootUsers(Value *V, SmallVector, 32> Dead; // Constants can't be pointers to dynamically allocated memory. - for (Value::user_iterator UI = V->user_begin(), E = V->user_end(); + for (Value::user_iterator UI = GV->user_begin(), E = GV->user_end(); UI != E;) { User *U = *UI++; if (StoreInst *SI = dyn_cast(U)) { @@ -232,9 +232,6 @@ static bool CleanupPointerRootUsers(Value *V, Dead.push_back(std::make_pair(I, MTI)); } } else if (ConstantExpr *CE = dyn_cast(U)) { - if (CE->getOpcode() == Instruction::GetElementPtr) { - Changed |= CleanupPointerRootUsers(CE, TLI); - } if (CE->use_empty()) { CE->destroyConstant(); Changed = true; @@ -244,7 +241,7 @@ static bool CleanupPointerRootUsers(Value *V, C->destroyConstant(); // This could have invalidated UI, start over from scratch. Dead.clear(); - CleanupPointerRootUsers(V, TLI); + CleanupPointerRootUsers(GV, TLI); return true; } } @@ -394,22 +391,6 @@ static bool isSafeSROAGEP(User *U) { [](User *UU) { return isSafeSROAElementUse(UU); }); } -/// Return true if the specified GEP is a safe user of a derived -/// expression from a global that we want to SROA. -static bool isSafeSubSROAGEP(User *U) { - - // Check to see if this ConstantExpr GEP is SRA'able. In particular, we - // don't like < 3 operand CE's, and we don't like non-constant integer - // indices. This enforces that all uses are 'gep GV, 0, C, ...' for some - // value of C. - if (U->getNumOperands() < 3 || !isa(U->getOperand(1)) || - !cast(U->getOperand(1))->isNullValue()) - return false; - - return llvm::all_of(U->users(), - [](User *UU) { return isSafeSROAElementUse(UU); }); -} - /// Return true if the specified instruction is a safe user of a derived /// expression from a global that we want to SROA. static bool isSafeSROAElementUse(Value *V) { @@ -428,7 +409,7 @@ static bool isSafeSROAElementUse(Value *V) { return SI->getOperand(0) != V; // Otherwise, it must be a GEP. Check it and its users are safe to SRA. - return isa(I) && isSafeSubSROAGEP(I); + return isa(I) && isSafeSROAGEP(I); } /// Look at all uses of the global and decide whether it is safe for us to diff --git a/llvm/test/Transforms/GlobalOpt/globalsra-multigep.ll b/llvm/test/Transforms/GlobalOpt/globalsra-multigep.ll index c32a620..87a8486 100644 --- a/llvm/test/Transforms/GlobalOpt/globalsra-multigep.ll +++ b/llvm/test/Transforms/GlobalOpt/globalsra-multigep.ll @@ -4,20 +4,13 @@ target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" target triple = "x86_64-unknown-linux-gnu" @g_data = internal unnamed_addr global <{ [8 x i16], [8 x i16] }> <{ [8 x i16] [i16 16, i16 16, i16 16, i16 16, i16 16, i16 16, i16 16, i16 16], [8 x i16] zeroinitializer }>, align 16 -; We normally cannot SRA here due to the second gep meaning the access to g_data may be to either element, -; unless the value is always zero. -; CHECK: @g_data.0 = internal unnamed_addr constant [8 x i16] [i16 16, i16 16, i16 16, i16 16, i16 16, i16 16, i16 16, i16 16], align 16 +; We cannot SRA here due to the second gep meaning the access to g_data may be to either element +; CHECK: @g_data = internal unnamed_addr constant <{ [8 x i16], [8 x i16] }> define i16 @test(i64 %a1) { entry: %g1 = getelementptr inbounds <{ [8 x i16], [8 x i16] }>, <{ [8 x i16], [8 x i16] }>* @g_data, i64 0, i32 0 %arrayidx.i = getelementptr inbounds [8 x i16], [8 x i16]* %g1, i64 0, i64 %a1 %r = load i16, i16* %arrayidx.i, align 2 - -; CHECK-NOT: getelementptr inbounds <{ [8 x i16], [8 x i16] }>, <{ [8 x i16], [8 x i16] }>* @g_data, i64 0, i32 0 -; CHECK: %arrayidx.i = getelementptr inbounds [8 x i16], [8 x i16]* @g_data.0, i64 0, i64 %a1 - ret i16 %r - - } diff --git a/llvm/test/Transforms/GlobalOpt/globalsra-struct.ll b/llvm/test/Transforms/GlobalOpt/globalsra-struct.ll deleted file mode 100644 index 957fba8..0000000 --- a/llvm/test/Transforms/GlobalOpt/globalsra-struct.ll +++ /dev/null @@ -1,18 +0,0 @@ -; RUN: opt < %s -globalopt -S | FileCheck %s - -%struct.Expr = type { [1 x i32], i32 } - -@e = internal global %struct.Expr zeroinitializer, align 4 -; CHECK-NOT: @e = internal global %struct.Expr zeroinitializer, align 4 - -define dso_local i32 @foo(i32 %i) { -entry: - %i.addr = alloca i32, align 4 - store i32 %i, i32* %i.addr, align 4 - %0 = load i32, i32* %i.addr, align 4 - %arrayidx = getelementptr inbounds [1 x i32], [1 x i32]* getelementptr inbounds (%struct.Expr, %struct.Expr* @e, i32 0, i32 0), i32 0, i32 %0 - store i32 57005, i32* %arrayidx, align 4 - %1 = load i32, i32* getelementptr inbounds (%struct.Expr, %struct.Expr* @e, i32 0, i32 1), align 4 - ret i32 %1 -; CHECK: ret i32 0 -}