Skip to content

[Clang][CodeGen] Preserve alignment information for pointer arithmetics #152575

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

[Clang][CodeGen] Preserve alignment information for pointer arithmetics #152575

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
16f7036
[Clang][CodeGen] Add pre-commit tests. NFC.
dtcxzyw Aug 7, 2025
e58f835
[Clang][CodeGen] Preserve alignment information for pointer arithmetics
dtcxzyw Aug 8, 2025
c23075d
[Clang][CodeGen] Fix comments. NFC.
dtcxzyw Aug 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 64 additions & 15 deletions clang/lib/CodeGen/CGExpr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1314,6 +1314,57 @@ void CodeGenModule::EmitExplicitCastExprType(const ExplicitCastExpr *E,
// LValue Expression Emission
//===----------------------------------------------------------------------===//

static CharUnits getArrayElementAlign(CharUnits arrayAlign, llvm::Value *idx,
CharUnits eltSize) {
// If we have a constant index, we can use the exact offset of the
// element we're accessing.
if (auto *constantIdx = dyn_cast<llvm::ConstantInt>(idx)) {
CharUnits offset = constantIdx->getZExtValue() * eltSize;
return arrayAlign.alignmentAtOffset(offset);
}

// Otherwise, use the worst-case alignment for any element.
return arrayAlign.alignmentOfArrayElement(eltSize);
}

/// Emit pointer + index arithmetic.
static Address emitPointerArithmetic(CodeGenFunction &CGF,
const BinaryOperator *BO,
LValueBaseInfo *BaseInfo,
TBAAAccessInfo *TBAAInfo,
KnownNonNull_t IsKnownNonNull) {
assert(BO->isAdditiveOp() && "Expect an addition or subtraction.");
Expr *pointerOperand = BO->getLHS();
Expr *indexOperand = BO->getRHS();
bool isSubtraction = BO->getOpcode() == BO_Sub;

Address BaseAddr = Address::invalid();
llvm::Value *index = nullptr;
// In a subtraction, the LHS is always the pointer.
// Note: do not change the evaluation order.
if (!isSubtraction && !pointerOperand->getType()->isAnyPointerType()) {
std::swap(pointerOperand, indexOperand);
index = CGF.EmitScalarExpr(indexOperand);
BaseAddr = CGF.EmitPointerWithAlignment(pointerOperand, BaseInfo, TBAAInfo,
NotKnownNonNull);
} else {
BaseAddr = CGF.EmitPointerWithAlignment(pointerOperand, BaseInfo, TBAAInfo,
NotKnownNonNull);
index = CGF.EmitScalarExpr(indexOperand);
}

llvm::Value *pointer = BaseAddr.getBasePointer();
llvm::Value *Res = CGF.EmitPointerArithmetic(
BO, pointerOperand, pointer, indexOperand, index, isSubtraction);
QualType PointeeTy = BO->getType()->getPointeeType();
CharUnits Align =
getArrayElementAlign(BaseAddr.getAlignment(), index,
CGF.getContext().getTypeSizeInChars(PointeeTy));
return Address(Res, CGF.ConvertTypeForMem(PointeeTy), Align,
CGF.CGM.getPointerAuthInfoForPointeeType(PointeeTy),
/*Offset=*/nullptr, IsKnownNonNull);
}

static Address EmitPointerWithAlignment(const Expr *E, LValueBaseInfo *BaseInfo,
TBAAAccessInfo *TBAAInfo,
KnownNonNull_t IsKnownNonNull,
Expand Down Expand Up @@ -1376,6 +1427,13 @@ static Address EmitPointerWithAlignment(const Expr *E, LValueBaseInfo *BaseInfo,
if (CE->getCastKind() == CK_AddressSpaceConversion)
Addr = CGF.Builder.CreateAddrSpaceCast(
Addr, CGF.ConvertType(E->getType()), ElemTy);
// Note: Workaround for PR114062. See also the special handling in
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inserting random casts is how we get into trouble like this in the first place...

// ScalarExprEmitter::VisitCastExpr.
if (auto *A = dyn_cast<llvm::Argument>(Addr.getBasePointer());
A && A->hasStructRetAttr())
Addr = CGF.Builder.CreateAddrSpaceCast(
Addr, CGF.ConvertType(E->getType()), ElemTy);

return CGF.authPointerToPointerCast(Addr, CE->getSubExpr()->getType(),
CE->getType());
}
Expand Down Expand Up @@ -1436,6 +1494,12 @@ static Address EmitPointerWithAlignment(const Expr *E, LValueBaseInfo *BaseInfo,
}
}

// Pointer arithmetic: pointer +/- index.
if (auto *BO = dyn_cast<BinaryOperator>(E)) {
if (BO->isAdditiveOp())
return emitPointerArithmetic(CGF, BO, BaseInfo, TBAAInfo, IsKnownNonNull);
}

// TODO: conditional operators, comma.

// Otherwise, use the alignment of the type.
Expand Down Expand Up @@ -4222,21 +4286,6 @@ static Address emitArraySubscriptGEP(CodeGenFunction &CGF, Address addr,
}
}

static CharUnits getArrayElementAlign(CharUnits arrayAlign,
llvm::Value *idx,
CharUnits eltSize) {
// If we have a constant index, we can use the exact offset of the
// element we're accessing.
if (auto constantIdx = dyn_cast<llvm::ConstantInt>(idx)) {
CharUnits offset = constantIdx->getZExtValue() * eltSize;
return arrayAlign.alignmentAtOffset(offset);

// Otherwise, use the worst-case alignment for any element.
} else {
return arrayAlign.alignmentOfArrayElement(eltSize);
}
}

static QualType getFixedSizeElementType(const ASTContext &ctx,
const VariableArrayType *vla) {
QualType eltType;
Expand Down
4 changes: 3 additions & 1 deletion clang/lib/CodeGen/CGExprScalar.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4183,7 +4183,9 @@ Value *ScalarExprEmitter::EmitOverflowCheckedBinOp(const BinOpInfo &Ops) {
return phi;
}

/// This function is used for BO_Add/BO_Sub/BO_AddAssign/BO_SubAssign.
/// BO_Add/BO_Sub are handled by EmitPointerWithAlignment to preserve alignment
/// information.
/// This function is used for BO_AddAssign/BO_SubAssign.
static Value *emitPointerArithmetic(CodeGenFunction &CGF, const BinOpInfo &op,
bool isSubtraction) {
// Must have binary (not unary) expr here. Unary pointer
Expand Down
6 changes: 3 additions & 3 deletions clang/test/CodeGen/packed-arrays.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ int align3_x0 = __alignof(((struct s3*) 0)->x[0]);
// CHECK: load i32, ptr %{{.*}}, align 1
// CHECK: }
// CHECK-LABEL: define{{.*}} i32 @f0_b
// CHECK: load i32, ptr %{{.*}}, align 4
// CHECK: load i32, ptr %{{.*}}, align 1
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It matches GCC's behavior: https://godbolt.org/z/TeGqTY1qc

// CHECK: }
int f0_a(struct s0 *a) {
return a->x[1];
Expand Down Expand Up @@ -100,7 +100,7 @@ int f1_d(struct s1 *a) {
// CHECK: load i32, ptr %{{.*}}, align 1
// CHECK: }
// CHECK-LABEL: define{{.*}} i32 @f2_b
// CHECK: load i32, ptr %{{.*}}, align 4
// CHECK: load i32, ptr %{{.*}}, align 1
// CHECK: }
// CHECK-LABEL: define{{.*}} i32 @f2_c
// CHECK: load i32, ptr %{{.*}}, align 1
Expand All @@ -125,7 +125,7 @@ int f2_d(struct s2 *a) {
// CHECK: load i32, ptr %{{.*}}, align 1
// CHECK: }
// CHECK-LABEL: define{{.*}} i32 @f3_b
// CHECK: load i32, ptr %{{.*}}, align 4
// CHECK: load i32, ptr %{{.*}}, align 1
// CHECK: }
// CHECK-LABEL: define{{.*}} i32 @f3_c
// CHECK: load i32, ptr %{{.*}}, align 1
Expand Down
83 changes: 83 additions & 0 deletions clang/test/CodeGen/pointer-arithmetic-align.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --version 5
// RUN: %clang_cc1 -O1 -triple=x86_64-unknown-linux %s -emit-llvm -o - | FileCheck %s

typedef unsigned char uint8_t;
typedef unsigned long long uint64_t;

struct a {
uint64_t b;
uint8_t block[16];
};

// CHECK-LABEL: define dso_local void @ptradd_0(
// CHECK-SAME: ptr noundef writeonly captures(none) initializes((8, 9)) [[CTX:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
// CHECK-NEXT: [[ENTRY:.*:]]
// CHECK-NEXT: [[BLOCK:%.*]] = getelementptr inbounds nuw i8, ptr [[CTX]], i64 8
// CHECK-NEXT: store i8 0, ptr [[BLOCK]], align 8, !tbaa [[TBAA2:![0-9]+]]
// CHECK-NEXT: ret void
//
void ptradd_0(struct a *ctx) {
*(ctx->block + 0) = 0;
}

// CHECK-LABEL: define dso_local void @ptradd_4(
// CHECK-SAME: ptr noundef writeonly captures(none) initializes((12, 13)) [[CTX:%.*]]) local_unnamed_addr #[[ATTR0]] {
// CHECK-NEXT: [[ENTRY:.*:]]
// CHECK-NEXT: [[ADD_PTR:%.*]] = getelementptr inbounds nuw i8, ptr [[CTX]], i64 12
// CHECK-NEXT: store i8 0, ptr [[ADD_PTR]], align 4, !tbaa [[TBAA2]]
// CHECK-NEXT: ret void
//
void ptradd_4(struct a *ctx) {
*(ctx->block + 4) = 0;
}

// CHECK-LABEL: define dso_local void @ptradd_8(
// CHECK-SAME: ptr noundef writeonly captures(none) initializes((16, 17)) [[CTX:%.*]]) local_unnamed_addr #[[ATTR0]] {
// CHECK-NEXT: [[ENTRY:.*:]]
// CHECK-NEXT: [[ADD_PTR:%.*]] = getelementptr inbounds nuw i8, ptr [[CTX]], i64 16
// CHECK-NEXT: store i8 0, ptr [[ADD_PTR]], align 8, !tbaa [[TBAA2]]
// CHECK-NEXT: ret void
//
void ptradd_8(struct a *ctx) {
*(ctx->block + 8) = 0;
}

// CHECK-LABEL: define dso_local void @ptradd_8_commuted(
// CHECK-SAME: ptr noundef writeonly captures(none) initializes((16, 17)) [[CTX:%.*]]) local_unnamed_addr #[[ATTR0]] {
// CHECK-NEXT: [[ENTRY:.*:]]
// CHECK-NEXT: [[ADD_PTR:%.*]] = getelementptr inbounds nuw i8, ptr [[CTX]], i64 16
// CHECK-NEXT: store i8 0, ptr [[ADD_PTR]], align 8, !tbaa [[TBAA2]]
// CHECK-NEXT: ret void
//
void ptradd_8_commuted(struct a *ctx) {
*(8 + ctx->block) = 0;
}

// CHECK-LABEL: define dso_local void @ptrsub_4(
// CHECK-SAME: ptr noundef writeonly captures(none) initializes((8, 9)) [[CTX:%.*]]) local_unnamed_addr #[[ATTR0]] {
// CHECK-NEXT: [[ENTRY:.*:]]
// CHECK-NEXT: [[ADD_PTR:%.*]] = getelementptr inbounds nuw i8, ptr [[CTX]], i64 8
// CHECK-NEXT: store i8 0, ptr [[ADD_PTR]], align 4, !tbaa [[TBAA2]]
// CHECK-NEXT: ret void
//
void ptrsub_4(struct a *ctx) {
*(&ctx->block[4] - 4) = 0;
}

// CHECK-LABEL: define dso_local void @neg_ptradd_var_index(
// CHECK-SAME: ptr noundef writeonly captures(none) [[CTX:%.*]], i8 noundef zeroext [[IDX:%.*]]) local_unnamed_addr #[[ATTR0]] {
// CHECK-NEXT: [[ENTRY:.*:]]
// CHECK-NEXT: [[BLOCK:%.*]] = getelementptr inbounds nuw i8, ptr [[CTX]], i64 8
// CHECK-NEXT: [[IDX_EXT:%.*]] = zext i8 [[IDX]] to i64
// CHECK-NEXT: [[ADD_PTR:%.*]] = getelementptr inbounds nuw i8, ptr [[BLOCK]], i64 [[IDX_EXT]]
// CHECK-NEXT: store i8 0, ptr [[ADD_PTR]], align 1, !tbaa [[TBAA2]]
// CHECK-NEXT: ret void
//
void neg_ptradd_var_index(struct a *ctx, uint8_t idx) {
*(ctx->block + idx) = 0;
}
//.
// CHECK: [[TBAA2]] = !{[[META3:![0-9]+]], [[META3]], i64 0}
// CHECK: [[META3]] = !{!"omnipotent char", [[META4:![0-9]+]], i64 0}
// CHECK: [[META4]] = !{!"Simple C/C++ TBAA"}
//.
2 changes: 1 addition & 1 deletion clang/test/CodeGenCXX/sret_cast_with_nonzero_alloca_as.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ struct X { int z[17]; };
// CHECK-NEXT: [[TMP1:%.*]] = load i8, ptr [[Y_ADDR_ASCAST]], align 1
// CHECK-NEXT: [[AGG_RESULT_ASCAST1:%.*]] = addrspacecast ptr addrspace(5) [[AGG_RESULT]] to ptr
// CHECK-NEXT: [[ADD_PTR2:%.*]] = getelementptr inbounds i8, ptr [[AGG_RESULT_ASCAST1]], i64 2
// CHECK-NEXT: store i8 [[TMP1]], ptr [[ADD_PTR2]], align 1
// CHECK-NEXT: store i8 [[TMP1]], ptr [[ADD_PTR2]], align 2
// CHECK-NEXT: ret void
//
X foo(char x, char y) {
Expand Down