Skip to content

Commit 5569bf2

Browse files
dtcxzywdtcxzyw
authored
[Clang][CodeGen] Preserve alignment information for pointer arithmetics (#152575)
Previously, the alignment of pointer arithmetics was inferred from the pointee type, losing the alignment information from its operands: https://github.com/llvm/llvm-project/blob/503c0908c3450d228debd64baecf41df8f58476e/clang/lib/CodeGen/CGExpr.cpp#L1446-L1449 This patch preserves alignment information for pointer arithmetics `P +/- C`, to match the behavior of identical array subscript `&P[C]`: https://godbolt.org/z/xx1hfTrx4. Closes #152330. Although the motivating case can be fixed by #145733, the alignment cannot be recovered without a dominating memory access with larger alignment.
1 parent 41aba9e commit 5569bf2

File tree

5 files changed

+148
-20
lines changed

5 files changed

+148
-20
lines changed

‎clang/lib/CodeGen/CGExpr.cpp

Lines changed: 58 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1325,6 +1325,57 @@ void CodeGenModule::EmitExplicitCastExprType(const ExplicitCastExpr *E,
13251325
// LValue Expression Emission
13261326
//===----------------------------------------------------------------------===//
13271327

1328+
static CharUnits getArrayElementAlign(CharUnits arrayAlign, llvm::Value *idx,
1329+
CharUnits eltSize) {
1330+
// If we have a constant index, we can use the exact offset of the
1331+
// element we're accessing.
1332+
if (auto *constantIdx = dyn_cast<llvm::ConstantInt>(idx)) {
1333+
CharUnits offset = constantIdx->getZExtValue() * eltSize;
1334+
return arrayAlign.alignmentAtOffset(offset);
1335+
}
1336+
1337+
// Otherwise, use the worst-case alignment for any element.
1338+
return arrayAlign.alignmentOfArrayElement(eltSize);
1339+
}
1340+
1341+
/// Emit pointer + index arithmetic.
1342+
static Address emitPointerArithmetic(CodeGenFunction &CGF,
1343+
const BinaryOperator *BO,
1344+
LValueBaseInfo *BaseInfo,
1345+
TBAAAccessInfo *TBAAInfo,
1346+
KnownNonNull_t IsKnownNonNull) {
1347+
assert(BO->isAdditiveOp() && "Expect an addition or subtraction.");
1348+
Expr *pointerOperand = BO->getLHS();
1349+
Expr *indexOperand = BO->getRHS();
1350+
bool isSubtraction = BO->getOpcode() == BO_Sub;
1351+
1352+
Address BaseAddr = Address::invalid();
1353+
llvm::Value *index = nullptr;
1354+
// In a subtraction, the LHS is always the pointer.
1355+
// Note: do not change the evaluation order.
1356+
if (!isSubtraction && !pointerOperand->getType()->isAnyPointerType()) {
1357+
std::swap(pointerOperand, indexOperand);
1358+
index = CGF.EmitScalarExpr(indexOperand);
1359+
BaseAddr = CGF.EmitPointerWithAlignment(pointerOperand, BaseInfo, TBAAInfo,
1360+
NotKnownNonNull);
1361+
} else {
1362+
BaseAddr = CGF.EmitPointerWithAlignment(pointerOperand, BaseInfo, TBAAInfo,
1363+
NotKnownNonNull);
1364+
index = CGF.EmitScalarExpr(indexOperand);
1365+
}
1366+
1367+
llvm::Value *pointer = BaseAddr.getBasePointer();
1368+
llvm::Value *Res = CGF.EmitPointerArithmetic(
1369+
BO, pointerOperand, pointer, indexOperand, index, isSubtraction);
1370+
QualType PointeeTy = BO->getType()->getPointeeType();
1371+
CharUnits Align =
1372+
getArrayElementAlign(BaseAddr.getAlignment(), index,
1373+
CGF.getContext().getTypeSizeInChars(PointeeTy));
1374+
return Address(Res, CGF.ConvertTypeForMem(PointeeTy), Align,
1375+
CGF.CGM.getPointerAuthInfoForPointeeType(PointeeTy),
1376+
/*Offset=*/nullptr, IsKnownNonNull);
1377+
}
1378+
13281379
static Address EmitPointerWithAlignment(const Expr *E, LValueBaseInfo *BaseInfo,
13291380
TBAAAccessInfo *TBAAInfo,
13301381
KnownNonNull_t IsKnownNonNull,
@@ -1387,6 +1438,7 @@ static Address EmitPointerWithAlignment(const Expr *E, LValueBaseInfo *BaseInfo,
13871438
if (CE->getCastKind() == CK_AddressSpaceConversion)
13881439
Addr = CGF.Builder.CreateAddrSpaceCast(
13891440
Addr, CGF.ConvertType(E->getType()), ElemTy);
1441+
13901442
return CGF.authPointerToPointerCast(Addr, CE->getSubExpr()->getType(),
13911443
CE->getType());
13921444
}
@@ -1447,6 +1499,12 @@ static Address EmitPointerWithAlignment(const Expr *E, LValueBaseInfo *BaseInfo,
14471499
}
14481500
}
14491501

1502+
// Pointer arithmetic: pointer +/- index.
1503+
if (auto *BO = dyn_cast<BinaryOperator>(E)) {
1504+
if (BO->isAdditiveOp())
1505+
return emitPointerArithmetic(CGF, BO, BaseInfo, TBAAInfo, IsKnownNonNull);
1506+
}
1507+
14501508
// TODO: conditional operators, comma.
14511509

14521510
// Otherwise, use the alignment of the type.
@@ -4236,21 +4294,6 @@ static Address emitArraySubscriptGEP(CodeGenFunction &CGF, Address addr,
42364294
}
42374295
}
42384296

4239-
static CharUnits getArrayElementAlign(CharUnits arrayAlign,
4240-
llvm::Value *idx,
4241-
CharUnits eltSize) {
4242-
// If we have a constant index, we can use the exact offset of the
4243-
// element we're accessing.
4244-
if (auto constantIdx = dyn_cast<llvm::ConstantInt>(idx)) {
4245-
CharUnits offset = constantIdx->getZExtValue() * eltSize;
4246-
return arrayAlign.alignmentAtOffset(offset);
4247-
4248-
// Otherwise, use the worst-case alignment for any element.
4249-
} else {
4250-
return arrayAlign.alignmentOfArrayElement(eltSize);
4251-
}
4252-
}
4253-
42544297
static QualType getFixedSizeElementType(const ASTContext &ctx,
42554298
const VariableArrayType *vla) {
42564299
QualType eltType;

‎clang/lib/CodeGen/CGExprScalar.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4186,7 +4186,9 @@ Value *ScalarExprEmitter::EmitOverflowCheckedBinOp(const BinOpInfo &Ops) {
41864186
return phi;
41874187
}
41884188

4189-
/// This function is used for BO_Add/BO_Sub/BO_AddAssign/BO_SubAssign.
4189+
/// BO_Add/BO_Sub are handled by EmitPointerWithAlignment to preserve alignment
4190+
/// information.
4191+
/// This function is used for BO_AddAssign/BO_SubAssign.
41904192
static Value *emitPointerArithmetic(CodeGenFunction &CGF, const BinOpInfo &op,
41914193
bool isSubtraction) {
41924194
// Must have binary (not unary) expr here. Unary pointer

‎clang/test/CodeGen/packed-arrays.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ int align3_x0 = __alignof(((struct s3*) 0)->x[0]);
5555
// CHECK: load i32, ptr %{{.*}}, align 1
5656
// CHECK: }
5757
// CHECK-LABEL: define{{.*}} i32 @f0_b
58-
// CHECK: load i32, ptr %{{.*}}, align 4
58+
// CHECK: load i32, ptr %{{.*}}, align 1
5959
// CHECK: }
6060
int f0_a(struct s0 *a) {
6161
return a->x[1];
@@ -100,7 +100,7 @@ int f1_d(struct s1 *a) {
100100
// CHECK: load i32, ptr %{{.*}}, align 1
101101
// CHECK: }
102102
// CHECK-LABEL: define{{.*}} i32 @f2_b
103-
// CHECK: load i32, ptr %{{.*}}, align 4
103+
// CHECK: load i32, ptr %{{.*}}, align 1
104104
// CHECK: }
105105
// CHECK-LABEL: define{{.*}} i32 @f2_c
106106
// CHECK: load i32, ptr %{{.*}}, align 1
@@ -125,7 +125,7 @@ int f2_d(struct s2 *a) {
125125
// CHECK: load i32, ptr %{{.*}}, align 1
126126
// CHECK: }
127127
// CHECK-LABEL: define{{.*}} i32 @f3_b
128-
// CHECK: load i32, ptr %{{.*}}, align 4
128+
// CHECK: load i32, ptr %{{.*}}, align 1
129129
// CHECK: }
130130
// CHECK-LABEL: define{{.*}} i32 @f3_c
131131
// CHECK: load i32, ptr %{{.*}}, align 1

‎clang/test/CodeGen/pointer-arithmetic-align.c

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --version 5
2+
// RUN: %clang_cc1 -O1 -triple=x86_64-unknown-linux %s -emit-llvm -o - | FileCheck %s
3+
4+
typedef unsigned char uint8_t;
5+
typedef unsigned long long uint64_t;
6+
7+
struct a {
8+
uint64_t b;
9+
uint8_t block[16];
10+
};
11+
12+
// CHECK-LABEL: define dso_local void @ptradd_0(
13+
// CHECK-SAME: ptr noundef writeonly captures(none) initializes((8, 9)) [[CTX:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
14+
// CHECK-NEXT: [[ENTRY:.*:]]
15+
// CHECK-NEXT: [[BLOCK:%.*]] = getelementptr inbounds nuw i8, ptr [[CTX]], i64 8
16+
// CHECK-NEXT: store i8 0, ptr [[BLOCK]], align 8, !tbaa [[TBAA2:![0-9]+]]
17+
// CHECK-NEXT: ret void
18+
//
19+
void ptradd_0(struct a *ctx) {
20+
*(ctx->block + 0) = 0;
21+
}
22+
23+
// CHECK-LABEL: define dso_local void @ptradd_4(
24+
// CHECK-SAME: ptr noundef writeonly captures(none) initializes((12, 13)) [[CTX:%.*]]) local_unnamed_addr #[[ATTR0]] {
25+
// CHECK-NEXT: [[ENTRY:.*:]]
26+
// CHECK-NEXT: [[ADD_PTR:%.*]] = getelementptr inbounds nuw i8, ptr [[CTX]], i64 12
27+
// CHECK-NEXT: store i8 0, ptr [[ADD_PTR]], align 4, !tbaa [[TBAA2]]
28+
// CHECK-NEXT: ret void
29+
//
30+
void ptradd_4(struct a *ctx) {
31+
*(ctx->block + 4) = 0;
32+
}
33+
34+
// CHECK-LABEL: define dso_local void @ptradd_8(
35+
// CHECK-SAME: ptr noundef writeonly captures(none) initializes((16, 17)) [[CTX:%.*]]) local_unnamed_addr #[[ATTR0]] {
36+
// CHECK-NEXT: [[ENTRY:.*:]]
37+
// CHECK-NEXT: [[ADD_PTR:%.*]] = getelementptr inbounds nuw i8, ptr [[CTX]], i64 16
38+
// CHECK-NEXT: store i8 0, ptr [[ADD_PTR]], align 8, !tbaa [[TBAA2]]
39+
// CHECK-NEXT: ret void
40+
//
41+
void ptradd_8(struct a *ctx) {
42+
*(ctx->block + 8) = 0;
43+
}
44+
45+
// CHECK-LABEL: define dso_local void @ptradd_8_commuted(
46+
// CHECK-SAME: ptr noundef writeonly captures(none) initializes((16, 17)) [[CTX:%.*]]) local_unnamed_addr #[[ATTR0]] {
47+
// CHECK-NEXT: [[ENTRY:.*:]]
48+
// CHECK-NEXT: [[ADD_PTR:%.*]] = getelementptr inbounds nuw i8, ptr [[CTX]], i64 16
49+
// CHECK-NEXT: store i8 0, ptr [[ADD_PTR]], align 8, !tbaa [[TBAA2]]
50+
// CHECK-NEXT: ret void
51+
//
52+
void ptradd_8_commuted(struct a *ctx) {
53+
*(8 + ctx->block) = 0;
54+
}
55+
56+
// CHECK-LABEL: define dso_local void @ptrsub_4(
57+
// CHECK-SAME: ptr noundef writeonly captures(none) initializes((8, 9)) [[CTX:%.*]]) local_unnamed_addr #[[ATTR0]] {
58+
// CHECK-NEXT: [[ENTRY:.*:]]
59+
// CHECK-NEXT: [[ADD_PTR:%.*]] = getelementptr inbounds nuw i8, ptr [[CTX]], i64 8
60+
// CHECK-NEXT: store i8 0, ptr [[ADD_PTR]], align 4, !tbaa [[TBAA2]]
61+
// CHECK-NEXT: ret void
62+
//
63+
void ptrsub_4(struct a *ctx) {
64+
*(&ctx->block[4] - 4) = 0;
65+
}
66+
67+
// CHECK-LABEL: define dso_local void @neg_ptradd_var_index(
68+
// CHECK-SAME: ptr noundef writeonly captures(none) [[CTX:%.*]], i8 noundef zeroext [[IDX:%.*]]) local_unnamed_addr #[[ATTR0]] {
69+
// CHECK-NEXT: [[ENTRY:.*:]]
70+
// CHECK-NEXT: [[BLOCK:%.*]] = getelementptr inbounds nuw i8, ptr [[CTX]], i64 8
71+
// CHECK-NEXT: [[IDX_EXT:%.*]] = zext i8 [[IDX]] to i64
72+
// CHECK-NEXT: [[ADD_PTR:%.*]] = getelementptr inbounds nuw i8, ptr [[BLOCK]], i64 [[IDX_EXT]]
73+
// CHECK-NEXT: store i8 0, ptr [[ADD_PTR]], align 1, !tbaa [[TBAA2]]
74+
// CHECK-NEXT: ret void
75+
//
76+
void neg_ptradd_var_index(struct a *ctx, uint8_t idx) {
77+
*(ctx->block + idx) = 0;
78+
}
79+
//.
80+
// CHECK: [[TBAA2]] = !{[[META3:![0-9]+]], [[META3]], i64 0}
81+
// CHECK: [[META3]] = !{!"omnipotent char", [[META4:![0-9]+]], i64 0}
82+
// CHECK: [[META4]] = !{!"Simple C/C++ TBAA"}
83+
//.

‎clang/test/CodeGenCXX/sret_cast_with_nonzero_alloca_as.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ struct X { int z[17]; };
1818
// CHECK-NEXT: store i8 [[TMP0]], ptr [[ADD_PTR]], align 1
1919
// CHECK-NEXT: [[TMP1:%.*]] = load i8, ptr [[Y_ADDR_ASCAST]], align 1
2020
// CHECK-NEXT: [[ADD_PTR1:%.*]] = getelementptr inbounds i8, ptr [[AGG_RESULT_ASCAST]], i64 2
21-
// CHECK-NEXT: store i8 [[TMP1]], ptr [[ADD_PTR1]], align 1
21+
// CHECK-NEXT: store i8 [[TMP1]], ptr [[ADD_PTR1]], align 2
2222
// CHECK-NEXT: ret void
2323
//
2424
X foo(char x, char y) {

0 commit comments

Comments
 (0)