aaron.ballman added inline comments.
================ Comment at: clang/include/clang/AST/Decl.h:1993 const DeclarationNameInfo &NameInfo, QualType T, - TypeSourceInfo *TInfo, StorageClass S, bool isInlineSpecified, - ConstexprSpecKind ConstexprKind, + TypeSourceInfo *TInfo, StorageClass S, bool UsesFPIntrin, + bool isInlineSpecified, ConstexprSpecKind ConstexprKind, ---------------- I have no idea if others agree, but I have a slight preference for putting `UsesFPIntrin` towards the end of the parameter list (here and in the Create methods) -- this flag seems less important than the constexpr kind or whether inline is specified (and, it helpfully avoids putting two `bool` parameters next to one another). If you agree, perhaps it could be moved to before `TrailingRequiresClause`? ================ Comment at: clang/include/clang/AST/Decl.h:2598 + /// Determine whether the function was declared in source context + /// that requires constrained FP intrinsics + bool UsesFPIntrin() const { return FunctionDeclBits.UsesFPIntrin; } ---------------- ================ Comment at: clang/include/clang/AST/Decl.h:2602 + /// Set whether the function was declared in source context + /// that requires constrained FP intrinsics + void setUsesFPIntrin(bool I) { FunctionDeclBits.UsesFPIntrin = I; } ---------------- ================ Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:939 + llvm::RoundingMode RM = getLangOpts().getFPRoundingMode(); + auto fpExceptionBehavior = + ToConstrainedExceptMD(getLangOpts().getFPExceptionMode()); ---------------- Please spell out the type rather than use `auto`, also, should probably be `FP` rather than `fp` per the usual naming conventions. ================ Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:700 CurCodeDecl = D; - if (const auto *FD = dyn_cast_or_null<FunctionDecl>(D)) - if (FD->usesSEHTry()) - CurSEHParent = FD; + const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(D); + if (FD && FD->usesSEHTry()) ---------------- mibintc wrote: > there's a whole bunch of places in this func that creates FD, I changed them > into a single place. hope that's OK, or i could pull it out as a pre-patch if > you prefer I think it'd be better to separate these changes into an NFC patch (feel free to land it without review; I've checked the changes here and they look correct and NFC). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102343/new/ https://reviews.llvm.org/D102343 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits