Skip to content

Commit 187e02f

Browse files
authored
[CodeGenPrepare] Check types when unmerging GEPs across indirect branches (#68587)
The optimization in CodeGenPrepare, where GEPs are unmerged across indirect branches must respect the types of both GEPs and their sizes when adjusting the indices. The sample here shows the bug: https://godbolt.org/z/8e9o5sYPP The value `%elementValuePtr` addresses the second field of the `%struct.Blub`. It is therefore a GEP with index 1 and type i8. The value `%nextArrayElement` addresses the next array element. It is therefore a GEP with index 1 and type `%struct.Blub`. Both values point to completely different addresses, even if the indices are the same, due to the types being different. However, after CodeGenPrepare has run, `%nextArrayElement` is a bitcast from `%elementValuePtr`, meaning both were treated as equal. The cause for this is that the unmerging optimization does not take types into consideration. It sees both GEPs have `%currentArrayElement` as source operand and therefore tries to rewrite `%nextArrayElement` in terms of `%elementValuePtr`. It changes the index to the difference of the two GEPs. As both indices are `1`, the difference is `0`. As the indices are `0` the GEP is later replaced with a simple bitcast in CodeGenPrepare. Before adjusting the indices, the types of the GEPs would have to be aligned and the indices scaled accordingly for the optimization to be correct. Due to the size of the struct being `16` and the `%elementValuePtr` pointing to offset `1`, the correct index for the unmerged `%nextArrayElement` would be 15. I assume this bug emerged from the opaque pointer change as GEPs like `%elementValuePtr` that access the struct field based of type i8 did not naturally occur before. In light of future migration to ptradd, simply not performing the optimization if the types mismatch should be sufficient.
1 parent 06cd648 commit 187e02f

File tree

2 files changed

+53
-0
lines changed

2 files changed

+53
-0
lines changed

llvm/lib/CodeGen/CodeGenPrepare.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7999,6 +7999,8 @@ static bool tryUnmergingGEPsAcrossIndirectBr(GetElementPtrInst *GEPI,
79997999
return false;
80008000
if (UGEPI->getOperand(0) != GEPIOp)
80018001
return false;
8002+
if (UGEPI->getSourceElementType() != GEPI->getSourceElementType())
8003+
return false;
80028004
if (GEPIIdx->getType() !=
80038005
cast<ConstantInt>(UGEPI->getOperand(1))->getType())
80048006
return false;
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 3
2+
; RUN: opt -S -codegenprepare %s -o - | FileCheck %s
3+
4+
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
5+
target triple = "x86_64-unknown-linux-gnu"
6+
7+
%struct.Blub = type { i8, i8, ptr }
8+
9+
@indirectBrPtr = external hidden global ptr
10+
11+
define ptr @testFunc(ptr readonly %array, i1 %skip) {
12+
; CHECK-LABEL: define ptr @testFunc(
13+
; CHECK-SAME: ptr readonly [[ARRAY:%.*]], i1 [[SKIP:%.*]]) {
14+
; CHECK-NEXT: entry:
15+
; CHECK-NEXT: br i1 [[SKIP]], label [[LOOPHEADER:%.*]], label [[ENDBLOCK_CLONE:%.*]]
16+
; CHECK: loopHeader:
17+
; CHECK-NEXT: [[CURRENTARRAYELEMENT:%.*]] = phi ptr [ [[ARRAY]], [[ENTRY:%.*]] ], [ [[NEXTARRAYELEMENT:%.*]], [[LOOPFOOTER:%.*]] ]
18+
; CHECK-NEXT: [[ELEMENTVALUEPTR:%.*]] = getelementptr inbounds i8, ptr [[CURRENTARRAYELEMENT]], i64 1
19+
; CHECK-NEXT: [[ELEMENTVALUE:%.*]] = load i8, ptr [[ELEMENTVALUEPTR]], align 1
20+
; CHECK-NEXT: indirectbr ptr @indirectBrPtr, [label [[LOOPFOOTER]], label %endBlock]
21+
; CHECK: loopFooter:
22+
; CHECK-NEXT: [[ISGOODVALUE:%.*]] = icmp eq i8 [[ELEMENTVALUE]], 0
23+
; CHECK-NEXT: [[NEXTARRAYELEMENT]] = getelementptr inbounds [[STRUCT_BLUB:%.*]], ptr [[CURRENTARRAYELEMENT]], i64 1
24+
; CHECK-NEXT: br i1 [[ISGOODVALUE]], label [[LOOPHEADER]], label [[ENDBLOCK_CLONE]]
25+
; CHECK: endBlock:
26+
; CHECK-NEXT: br label [[DOTSPLIT:%.*]]
27+
; CHECK: .split:
28+
; CHECK-NEXT: [[MERGE:%.*]] = phi ptr [ [[ELEMENTVALUEPTR]], [[ENDBLOCK:%.*]] ], [ [[RETVAL_CLONE:%.*]], [[ENDBLOCK_CLONE]] ]
29+
; CHECK-NEXT: ret ptr [[MERGE]]
30+
; CHECK: endBlock.clone:
31+
; CHECK-NEXT: [[RETVAL_CLONE]] = phi ptr [ [[ARRAY]], [[ENTRY]] ], [ [[ELEMENTVALUEPTR]], [[LOOPFOOTER]] ]
32+
; CHECK-NEXT: br label [[DOTSPLIT]]
33+
;
34+
entry:
35+
br i1 %skip, label %loopHeader, label %endBlock
36+
37+
loopHeader:
38+
%currentArrayElement = phi ptr [ %array, %entry ], [ %nextArrayElement, %loopFooter ]
39+
%elementValuePtr = getelementptr inbounds i8, ptr %currentArrayElement, i64 1
40+
%elementValue = load i8, ptr %elementValuePtr, align 1
41+
indirectbr ptr @indirectBrPtr, [label %loopFooter, label %endBlock]
42+
43+
loopFooter:
44+
%isGoodValue = icmp eq i8 %elementValue, 0
45+
%nextArrayElement = getelementptr inbounds %struct.Blub, ptr %currentArrayElement, i64 1
46+
br i1 %isGoodValue, label %loopHeader, label %endBlock
47+
48+
endBlock:
49+
%retVal = phi ptr [ %array, %entry ], [ %elementValuePtr, %loopFooter ], [ %elementValuePtr, %loopHeader ]
50+
ret ptr %retVal
51+
}

0 commit comments

Comments
 (0)