[clang] [Clang] Improve generation of GEP and RecordDecl loop (PR #101434)
Bill Wendling via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 1 12:46:01 PDT 2024
https://github.com/bwendling updated https://github.com/llvm/llvm-project/pull/101434
>From 580fb57202daa5fcfba3e0fdcc50bb3786dc9798 Mon Sep 17 00:00:00 2001
From: Bill Wendling <morbo at google.com>
Date: Wed, 31 Jul 2024 16:53:21 -0700
Subject: [PATCH 1/2] [Clang] Improve generation of GEP and RecordDecl loop
As with other loops, we need only look at a RecordDecl's FieldDecls.
Convert to using them. In the meantime, we can improve the generation of
the 'counted_by' FieldDecl's GEP: create one GEP instead of a series of
GEPs.
---
clang/lib/CodeGen/CGBuiltin.cpp | 2 +-
clang/lib/CodeGen/CGExpr.cpp | 41 ++++++++++++++--------------
clang/lib/CodeGen/CodeGenFunction.h | 6 ++--
clang/test/CodeGen/attr-counted-by.c | 8 +++---
4 files changed, 28 insertions(+), 29 deletions(-)
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 0c2ee446aa303..b2cab812985af 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -996,7 +996,7 @@ CodeGenFunction::emitFlexibleArrayMemberSize(const Expr *E, unsigned Type,
// Build a load of the counted_by field.
bool IsSigned = CountedByFD->getType()->isSignedIntegerType();
- Value *CountedByInst = EmitCountedByFieldExpr(Base, FAMDecl, CountedByFD);
+ Value *CountedByInst = EmitLoadOfCountedByField(Base, FAMDecl, CountedByFD);
if (!CountedByInst)
return getDefaultBuiltinObjectSizeResult(Type, ResType);
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 5f58a64d8386c..cf22ff43c35f7 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -1067,8 +1067,7 @@ class StructAccessBase
} // end anonymous namespace
-using RecIndicesTy =
- SmallVector<std::pair<const RecordDecl *, llvm::Value *>, 8>;
+using RecIndicesTy = SmallVector<llvm::Value *, 8>;
static bool getGEPIndicesToField(CodeGenFunction &CGF, const RecordDecl *RD,
const FieldDecl *Field,
@@ -1083,7 +1082,7 @@ static bool getGEPIndicesToField(CodeGenFunction &CGF, const RecordDecl *RD,
FieldNo = Layout.getLLVMFieldNo(FD);
if (FD == Field) {
- Indices.emplace_back(std::make_pair(RD, CGF.Builder.getInt32(FieldNo)));
+ Indices.emplace_back(CGF.Builder.getInt32(FieldNo));
return true;
}
@@ -1092,7 +1091,7 @@ static bool getGEPIndicesToField(CodeGenFunction &CGF, const RecordDecl *RD,
if (getGEPIndicesToField(CGF, Ty->getAsRecordDecl(), Field, Indices)) {
if (RD->isUnion())
FieldNo = 0;
- Indices.emplace_back(std::make_pair(RD, CGF.Builder.getInt32(FieldNo)));
+ Indices.emplace_back(CGF.Builder.getInt32(FieldNo));
return true;
}
}
@@ -1109,7 +1108,7 @@ static bool getGEPIndicesToField(CodeGenFunction &CGF, const RecordDecl *RD,
/// - \p FAMDecl: the \p Decl for the flexible array member. It may not be
/// within the top-level struct.
/// - \p CountDecl: must be within the same non-anonymous struct as \p FAMDecl.
-llvm::Value *CodeGenFunction::EmitCountedByFieldExpr(
+llvm::Value *CodeGenFunction::EmitLoadOfCountedByField(
const Expr *Base, const FieldDecl *FAMDecl, const FieldDecl *CountDecl) {
const RecordDecl *RD = CountDecl->getParent()->getOuterLexicalRecordContext();
@@ -1136,15 +1135,15 @@ llvm::Value *CodeGenFunction::EmitCountedByFieldExpr(
return nullptr;
}
- llvm::Value *Zero = Builder.getInt32(0);
RecIndicesTy Indices;
-
getGEPIndicesToField(*this, RD, CountDecl, Indices);
+ if (Indices.empty())
+ return nullptr;
- for (auto I = Indices.rbegin(), E = Indices.rend(); I != E; ++I)
- Res = Builder.CreateInBoundsGEP(
- ConvertType(QualType(I->first->getTypeForDecl(), 0)), Res,
- {Zero, I->second}, "..counted_by.gep");
+ Indices.emplace_back(Builder.getInt32(0));
+ Res = Builder.CreateInBoundsGEP(
+ ConvertType(QualType(RD->getTypeForDecl(), 0)), Res,
+ RecIndicesTy(llvm::reverse(Indices)), "..counted_by.gep");
return Builder.CreateAlignedLoad(ConvertType(CountDecl->getType()), Res,
getIntAlign(), "..counted_by.load");
@@ -4108,25 +4107,25 @@ static Address emitArraySubscriptGEP(CodeGenFunction &CGF, Address addr,
/// The offset of a field from the beginning of the record.
static bool getFieldOffsetInBits(CodeGenFunction &CGF, const RecordDecl *RD,
- const FieldDecl *FD, int64_t &Offset) {
+ const FieldDecl *Field, int64_t &Offset) {
ASTContext &Ctx = CGF.getContext();
const ASTRecordLayout &Layout = Ctx.getASTRecordLayout(RD);
unsigned FieldNo = 0;
- for (const Decl *D : RD->decls()) {
- if (const auto *Record = dyn_cast<RecordDecl>(D))
- if (getFieldOffsetInBits(CGF, Record, FD, Offset)) {
- Offset += Layout.getFieldOffset(FieldNo);
- return true;
- }
+ for (const FieldDecl *FD : RD->fields()) {
+ if (FD == Field) {
+ Offset += Layout.getFieldOffset(FieldNo);
+ return true;
+ }
- if (const auto *Field = dyn_cast<FieldDecl>(D))
- if (FD == Field) {
+ QualType Ty = FD->getType();
+ if (Ty->isRecordType())
+ if (getFieldOffsetInBits(CGF, Ty->getAsRecordDecl(), Field, Offset)) {
Offset += Layout.getFieldOffset(FieldNo);
return true;
}
- if (isa<FieldDecl>(D))
+ if (!RD->isUnion())
++FieldNo;
}
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index 1911fbac100c5..60885f7f9df41 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -3310,9 +3310,9 @@ class CodeGenFunction : public CodeGenTypeCache {
const FieldDecl *FindCountedByField(const FieldDecl *FD);
/// Build an expression accessing the "counted_by" field.
- llvm::Value *EmitCountedByFieldExpr(const Expr *Base,
- const FieldDecl *FAMDecl,
- const FieldDecl *CountDecl);
+ llvm::Value *EmitLoadOfCountedByField(const Expr *Base,
+ const FieldDecl *FAMDecl,
+ const FieldDecl *CountDecl);
llvm::Value *EmitScalarPrePostIncDec(const UnaryOperator *E, LValue LV,
bool isInc, bool isPre);
diff --git a/clang/test/CodeGen/attr-counted-by.c b/clang/test/CodeGen/attr-counted-by.c
index 46a6c2b492dbe..f5032ded971a8 100644
--- a/clang/test/CodeGen/attr-counted-by.c
+++ b/clang/test/CodeGen/attr-counted-by.c
@@ -754,11 +754,11 @@ size_t test7_bdos(struct union_of_fams *p) {
// SANITIZE-WITH-ATTR-NEXT: [[IDXPROM:%.*]] = sext i32 [[INDEX]] to i64
// SANITIZE-WITH-ATTR-NEXT: [[TMP0:%.*]] = zext i8 [[DOT_COUNTED_BY_LOAD]] to i64, !nosanitize [[META2]]
// SANITIZE-WITH-ATTR-NEXT: [[TMP1:%.*]] = icmp ult i64 [[IDXPROM]], [[TMP0]], !nosanitize [[META2]]
-// SANITIZE-WITH-ATTR-NEXT: br i1 [[TMP1]], label [[CONT9:%.*]], label [[HANDLER_OUT_OF_BOUNDS:%.*]], !prof [[PROF3]], !nosanitize [[META2]]
+// SANITIZE-WITH-ATTR-NEXT: br i1 [[TMP1]], label [[CONT7:%.*]], label [[HANDLER_OUT_OF_BOUNDS:%.*]], !prof [[PROF3]], !nosanitize [[META2]]
// SANITIZE-WITH-ATTR: handler.out_of_bounds:
// SANITIZE-WITH-ATTR-NEXT: tail call void @__ubsan_handle_out_of_bounds_abort(ptr nonnull @[[GLOB12:[0-9]+]], i64 [[IDXPROM]]) #[[ATTR10]], !nosanitize [[META2]]
// SANITIZE-WITH-ATTR-NEXT: unreachable, !nosanitize [[META2]]
-// SANITIZE-WITH-ATTR: cont9:
+// SANITIZE-WITH-ATTR: cont7:
// SANITIZE-WITH-ATTR-NEXT: [[INTS:%.*]] = getelementptr inbounds i8, ptr [[P]], i64 9
// SANITIZE-WITH-ATTR-NEXT: [[ARRAYIDX:%.*]] = getelementptr inbounds [0 x i8], ptr [[INTS]], i64 0, i64 [[IDXPROM]]
// SANITIZE-WITH-ATTR-NEXT: store i8 [[DOT_COUNTED_BY_LOAD]], ptr [[ARRAYIDX]], align 1, !tbaa [[TBAA8]]
@@ -908,11 +908,11 @@ size_t test9_bdos(struct union_of_fams *p) {
// SANITIZE-WITH-ATTR-NEXT: [[IDXPROM:%.*]] = sext i32 [[INDEX]] to i64
// SANITIZE-WITH-ATTR-NEXT: [[TMP0:%.*]] = zext i32 [[DOT_COUNTED_BY_LOAD]] to i64, !nosanitize [[META2]]
// SANITIZE-WITH-ATTR-NEXT: [[TMP1:%.*]] = icmp ult i64 [[IDXPROM]], [[TMP0]], !nosanitize [[META2]]
-// SANITIZE-WITH-ATTR-NEXT: br i1 [[TMP1]], label [[CONT9:%.*]], label [[HANDLER_OUT_OF_BOUNDS:%.*]], !prof [[PROF3]], !nosanitize [[META2]]
+// SANITIZE-WITH-ATTR-NEXT: br i1 [[TMP1]], label [[CONT7:%.*]], label [[HANDLER_OUT_OF_BOUNDS:%.*]], !prof [[PROF3]], !nosanitize [[META2]]
// SANITIZE-WITH-ATTR: handler.out_of_bounds:
// SANITIZE-WITH-ATTR-NEXT: tail call void @__ubsan_handle_out_of_bounds_abort(ptr nonnull @[[GLOB15:[0-9]+]], i64 [[IDXPROM]]) #[[ATTR10]], !nosanitize [[META2]]
// SANITIZE-WITH-ATTR-NEXT: unreachable, !nosanitize [[META2]]
-// SANITIZE-WITH-ATTR: cont9:
+// SANITIZE-WITH-ATTR: cont7:
// SANITIZE-WITH-ATTR-NEXT: [[BYTES:%.*]] = getelementptr inbounds i8, ptr [[P]], i64 12
// SANITIZE-WITH-ATTR-NEXT: [[ARRAYIDX:%.*]] = getelementptr inbounds [0 x i8], ptr [[BYTES]], i64 0, i64 [[IDXPROM]]
// SANITIZE-WITH-ATTR-NEXT: [[NARROW:%.*]] = tail call i32 @llvm.smax.i32(i32 [[DOT_COUNTED_BY_LOAD]], i32 0)
>From 406f7de9b3b75d4dcd1c1d6d23d609a5c7c84131 Mon Sep 17 00:00:00 2001
From: Bill Wendling <morbo at google.com>
Date: Thu, 1 Aug 2024 12:45:50 -0700
Subject: [PATCH 2/2] Use push_back
---
clang/lib/CodeGen/CGExpr.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index cf22ff43c35f7..bb671c5ab5813 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -1140,7 +1140,7 @@ llvm::Value *CodeGenFunction::EmitLoadOfCountedByField(
if (Indices.empty())
return nullptr;
- Indices.emplace_back(Builder.getInt32(0));
+ Indices.push_back(Builder.getInt32(0));
Res = Builder.CreateInBoundsGEP(
ConvertType(QualType(RD->getTypeForDecl(), 0)), Res,
RecIndicesTy(llvm::reverse(Indices)), "..counted_by.gep");
More information about the cfe-commits
mailing list