Skip to content

Commit 4a7da98

Browse files
author
Christian Bruel
committed
[GlobalOpt] recognize dead struct fields and propagate values
Summary: Allow struct fields SRA and dead stores. This works by considering fields accesses from getElementPtr to be considered as a possible pointer root that can be cleaned up. We check that the variable can be SRA by recursively checking the sub expressions with the new isSafeSubSROAGEP function. basically this allows the array in following C code to be optimized out struct Expr { int a[2]; int b; }; static struct Expr e; int foo (int i) { e.b = 2; e.a[i] = 1; return e.b; } Reviewers: greened, bkramer, nicholas, jmolloy Reviewed By: jmolloy Subscribers: llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D61911 llvm-svn: 361460
1 parent 202dc12 commit 4a7da98

File tree

3 files changed

+50
-6
lines changed

3 files changed

+50
-6
lines changed

llvm/lib/Transforms/IPO/GlobalOpt.cpp

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ static bool IsSafeComputationToRemove(Value *V, const TargetLibraryInfo *TLI) {
184184
/// This GV is a pointer root. Loop over all users of the global and clean up
185185
/// any that obviously don't assign the global a value that isn't dynamically
186186
/// allocated.
187-
static bool CleanupPointerRootUsers(GlobalVariable *GV,
187+
static bool CleanupPointerRootUsers(Value *V,
188188
const TargetLibraryInfo *TLI) {
189189
// A brief explanation of leak checkers. The goal is to find bugs where
190190
// pointers are forgotten, causing an accumulating growth in memory
@@ -202,7 +202,7 @@ static bool CleanupPointerRootUsers(GlobalVariable *GV,
202202
SmallVector<std::pair<Instruction *, Instruction *>, 32> Dead;
203203

204204
// Constants can't be pointers to dynamically allocated memory.
205-
for (Value::user_iterator UI = GV->user_begin(), E = GV->user_end();
205+
for (Value::user_iterator UI = V->user_begin(), E = V->user_end();
206206
UI != E;) {
207207
User *U = *UI++;
208208
if (StoreInst *SI = dyn_cast<StoreInst>(U)) {
@@ -232,6 +232,9 @@ static bool CleanupPointerRootUsers(GlobalVariable *GV,
232232
Dead.push_back(std::make_pair(I, MTI));
233233
}
234234
} else if (ConstantExpr *CE = dyn_cast<ConstantExpr>(U)) {
235+
if (CE->getOpcode() == Instruction::GetElementPtr) {
236+
Changed |= CleanupPointerRootUsers(CE, TLI);
237+
}
235238
if (CE->use_empty()) {
236239
CE->destroyConstant();
237240
Changed = true;
@@ -241,7 +244,7 @@ static bool CleanupPointerRootUsers(GlobalVariable *GV,
241244
C->destroyConstant();
242245
// This could have invalidated UI, start over from scratch.
243246
Dead.clear();
244-
CleanupPointerRootUsers(GV, TLI);
247+
CleanupPointerRootUsers(V, TLI);
245248
return true;
246249
}
247250
}
@@ -391,6 +394,22 @@ static bool isSafeSROAGEP(User *U) {
391394
[](User *UU) { return isSafeSROAElementUse(UU); });
392395
}
393396

397+
/// Return true if the specified GEP is a safe user of a derived
398+
/// expression from a global that we want to SROA.
399+
static bool isSafeSubSROAGEP(User *U) {
400+
401+
// Check to see if this ConstantExpr GEP is SRA'able. In particular, we
402+
// don't like < 3 operand CE's, and we don't like non-constant integer
403+
// indices. This enforces that all uses are 'gep GV, 0, C, ...' for some
404+
// value of C.
405+
if (U->getNumOperands() < 3 || !isa<Constant>(U->getOperand(1)) ||
406+
!cast<Constant>(U->getOperand(1))->isNullValue())
407+
return false;
408+
409+
return llvm::all_of(U->users(),
410+
[](User *UU) { return isSafeSROAElementUse(UU); });
411+
}
412+
394413
/// Return true if the specified instruction is a safe user of a derived
395414
/// expression from a global that we want to SROA.
396415
static bool isSafeSROAElementUse(Value *V) {
@@ -409,7 +428,7 @@ static bool isSafeSROAElementUse(Value *V) {
409428
return SI->getOperand(0) != V;
410429

411430
// Otherwise, it must be a GEP. Check it and its users are safe to SRA.
412-
return isa<GetElementPtrInst>(I) && isSafeSROAGEP(I);
431+
return isa<GetElementPtrInst>(I) && isSafeSubSROAGEP(I);
413432
}
414433

415434
/// Look at all uses of the global and decide whether it is safe for us to

llvm/test/Transforms/GlobalOpt/globalsra-multigep.ll

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,20 @@ target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
44
target triple = "x86_64-unknown-linux-gnu"
55

66
@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
7-
; We cannot SRA here due to the second gep meaning the access to g_data may be to either element
8-
; CHECK: @g_data = internal unnamed_addr constant <{ [8 x i16], [8 x i16] }>
7+
; We normally cannot SRA here due to the second gep meaning the access to g_data may be to either element,
8+
; unless the value is always zero.
9+
; 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
910

1011
define i16 @test(i64 %a1) {
1112
entry:
1213
%g1 = getelementptr inbounds <{ [8 x i16], [8 x i16] }>, <{ [8 x i16], [8 x i16] }>* @g_data, i64 0, i32 0
1314
%arrayidx.i = getelementptr inbounds [8 x i16], [8 x i16]* %g1, i64 0, i64 %a1
1415
%r = load i16, i16* %arrayidx.i, align 2
16+
17+
; CHECK-NOT: getelementptr inbounds <{ [8 x i16], [8 x i16] }>, <{ [8 x i16], [8 x i16] }>* @g_data, i64 0, i32 0
18+
; CHECK: %arrayidx.i = getelementptr inbounds [8 x i16], [8 x i16]* @g_data.0, i64 0, i64 %a1
19+
1520
ret i16 %r
21+
22+
1623
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
; RUN: opt < %s -globalopt -S | FileCheck %s
2+
3+
%struct.Expr = type { [1 x i32], i32 }
4+
5+
@e = internal global %struct.Expr zeroinitializer, align 4
6+
; CHECK-NOT: @e = internal global %struct.Expr zeroinitializer, align 4
7+
8+
define dso_local i32 @foo(i32 %i) {
9+
entry:
10+
%i.addr = alloca i32, align 4
11+
store i32 %i, i32* %i.addr, align 4
12+
%0 = load i32, i32* %i.addr, align 4
13+
%arrayidx = getelementptr inbounds [1 x i32], [1 x i32]* getelementptr inbounds (%struct.Expr, %struct.Expr* @e, i32 0, i32 0), i32 0, i32 %0
14+
store i32 57005, i32* %arrayidx, align 4
15+
%1 = load i32, i32* getelementptr inbounds (%struct.Expr, %struct.Expr* @e, i32 0, i32 1), align 4
16+
ret i32 %1
17+
; CHECK: ret i32 0
18+
}

0 commit comments

Comments
 (0)