llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-codegen Author: Bill Wendling (bwendling) <details> <summary>Changes</summary> 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. --- Full diff: https://github.com/llvm/llvm-project/pull/101434.diff 4 Files Affected: - (modified) clang/lib/CodeGen/CGBuiltin.cpp (+1-1) - (modified) clang/lib/CodeGen/CGExpr.cpp (+20-21) - (modified) clang/lib/CodeGen/CodeGenFunction.h (+3-3) - (modified) clang/test/CodeGen/attr-counted-by.c (+4-4) ``````````diff 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) `````````` </details> https://github.com/llvm/llvm-project/pull/101434 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits