probinson added a comment. I put in a lot of comments about spelling for the new parameter (constExpr, isConstexpr, isConstExpr) which should be named consistently throughout. Please do not use Constant or any variant, as that tends to mean something else.
But, what I would rather see instead of a new parameter everywhere: define a new DIFlag bit. Most of the churn goes away, and there's no bitcode compatibility issue. It's true that there are not many DIFlag bits left, but as this attribute can apply to both variables and functions, it seems appropriate to put it there. ================ Comment at: llvm/include/llvm/IR/DIBuilder.h:585 /// specified) + /// \isConstExpr Boolean flag indicating wheather this variable is + /// constexpr or not ---------------- `\param isConstExpr` also wheather -> whether ================ Comment at: llvm/include/llvm/IR/DIBuilder.h:666 /// \param SPFlags Additional flags specific to subprograms. /// \param TParams Function template parameters. /// \param ThrownTypes Exception types this function may throw. ---------------- Add `\param constExpr` here. ================ Comment at: llvm/include/llvm/IR/DIBuilder.h:702 /// This flags are used to emit dwarf attributes. /// \param SPFlags Additional flags specific to subprograms. /// \param TParams Function template parameters. ---------------- Add `\param isConstExpr` here ================ Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:1694 getThisAdjustment(), getFlags(), getSPFlags(), + false, /*todo*/ getUnit(), getTemplateParams(), getDeclaration(), ---------------- todo? ================ Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:2293 Metadata *getRawType() const { return getOperand(3); } + bool isConstant() const { return ConstExpr; } ---------------- `isConstExpr` ================ Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:2635 bool IsLocalToUnit, bool IsDefinition, uint32_t AlignInBits, - ArrayRef<Metadata *> Ops) - : DIVariable(C, DIGlobalVariableKind, Storage, Line, Ops, AlignInBits), + bool IsConstant, ArrayRef<Metadata *> Ops) + : DIVariable(C, DIGlobalVariableKind, Storage, Line, Ops, AlignInBits, ---------------- `IsConstExpr` as this flag is specifically tied to the const_expr attribute; "constant" means something different. ================ Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:2666 isDefinition(), getStaticDataMemberDeclaration(), - getTemplateParams(), getAlignInBits()); + getTemplateParams(), getAlignInBits(), false /*TODO*/); } ---------------- TODO? ================ Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:2675 + DIDerivedType *StaticDataMemberDeclaration, MDTuple *TemplateParams, + uint32_t AlignInBits, bool IsConstant), + (Scope, Name, LinkageName, File, Line, Type, IsLocalToUnit, IsDefinition, ---------------- `IsConstExpr` ================ Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:2683 + Metadata *StaticDataMemberDeclaration, Metadata *TemplateParams, + uint32_t AlignInBits, bool IsConstant), + (Scope, Name, LinkageName, File, Line, Type, IsLocalToUnit, IsDefinition, ---------------- `IsConstExpr` ================ Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:2778 unsigned Arg, DIFlags Flags, uint32_t AlignInBits, - ArrayRef<Metadata *> Ops) - : DIVariable(C, DILocalVariableKind, Storage, Line, Ops, AlignInBits), + bool IsConstant, ArrayRef<Metadata *> Ops) + : DIVariable(C, DILocalVariableKind, Storage, Line, Ops, AlignInBits, ---------------- `IsConstExpr` ================ Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:2789 DIType *Type, unsigned Arg, DIFlags Flags, - uint32_t AlignInBits, StorageType Storage, + uint32_t AlignInBits, bool IsConstant, + StorageType Storage, ---------------- `IsConstExpr` ================ Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:2799 Metadata *Type, unsigned Arg, DIFlags Flags, - uint32_t AlignInBits, StorageType Storage, + uint32_t AlignInBits, bool IsConstant, + StorageType Storage, ---------------- `IsConstExpr` ================ Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:2813 unsigned Line, DIType *Type, unsigned Arg, DIFlags Flags, - uint32_t AlignInBits), - (Scope, Name, File, Line, Type, Arg, Flags, AlignInBits)) + uint32_t AlignInBits, bool IsConstant), + (Scope, Name, File, Line, Type, Arg, Flags, AlignInBits, ---------------- `IsConstExpr` ================ Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:2819 + unsigned Line, Metadata *Type, unsigned Arg, DIFlags Flags, + uint32_t AlignInBits, bool IsConstant), + (Scope, Name, File, Line, Type, Arg, Flags, AlignInBits, ---------------- `IsConstExpr` ================ Comment at: llvm/lib/AsmParser/LLParser.cpp:4680 /// virtualIndex: 10, thisAdjustment: 4, flags: 11, /// spFlags: 10, isOptimized: false, templateParams: !4, /// declaration: !5, retainedNodes: !6, thrownTypes: !7) ---------------- Document the new parameter. ================ Comment at: llvm/lib/AsmParser/LLParser.cpp:4700 OPTIONAL(spFlags, DISPFlagField, ); \ + OPTIONAL(const_expr, MDBoolField, ); \ OPTIONAL(isOptimized, MDBoolField, ); \ ---------------- Naming convention wants this to be `constExpr` ================ Comment at: llvm/lib/AsmParser/LLParser.cpp:4877 /// isDefinition: true, templateParams: !3, /// declaration: !4, align: 8) bool LLParser::ParseDIGlobalVariable(MDNode *&Result, bool IsDistinct) { ---------------- Document the new parameter. ================ Comment at: llvm/lib/AsmParser/LLParser.cpp:4891 + OPTIONAL(align, MDUnsignedField, (0, UINT32_MAX)); \ + OPTIONAL(const_expr, MDBoolField, ); PARSE_MD_FIELDS(); ---------------- `constExpr` ================ Comment at: llvm/lib/AsmParser/LLParser.cpp:4909 /// file: !1, line: 7, type: !2, arg: 2, flags: 7, /// align: 8) bool LLParser::ParseDILocalVariable(MDNode *&Result, bool IsDistinct) { ---------------- Document the new parameter. ================ Comment at: llvm/lib/AsmParser/LLParser.cpp:4920 + OPTIONAL(align, MDUnsignedField, (0, UINT32_MAX)); \ + OPTIONAL(const_expr, MDBoolField, ); PARSE_MD_FIELDS(); ---------------- `constExpr` ================ Comment at: llvm/lib/IR/DIBuilder.cpp:646 + DIExpression *Expr, MDNode *Decl, MDTuple *TemplateParams, + uint32_t AlignInBits, bool isConstant) { checkGlobalVariableScope(Context); ---------------- `isConstExpr` (throughout this module) ================ Comment at: llvm/lib/IR/DIBuilder.cpp:718 File, LineNo, Ty, AlwaysPreserve, Flags, - /* AlignInBits */0); + /* AlignInBits */ 0, /* constexpr */ false); } ---------------- The comment should match the parameter name. ================ Comment at: llvm/lib/IR/DebugInfo.cpp:468 ContainingType, MDS->getVirtualIndex(), MDS->getThisAdjustment(), - MDS->getFlags(), MDS->getSPFlags(), Unit, TemplateParams, Declaration, - Variables); + MDS->getFlags(), MDS->getSPFlags(), false, /*todo*/ Unit, + TemplateParams, Declaration, Variables); ---------------- todo? ================ Comment at: llvm/lib/IR/DebugInfo.cpp:479 MDS->getVirtualIndex(), MDS->getThisAdjustment(), MDS->getFlags(), - MDS->getSPFlags(), Unit, TemplateParams, Declaration, Variables); + MDS->getSPFlags(), false /*todo*/, Unit, TemplateParams, Declaration, + Variables); ---------------- todo? ================ Comment at: llvm/lib/IR/DebugInfo.cpp:837 + pack_into_DISPFlags(IsLocalToUnit, IsDefinition, IsOptimized), + false /*todo*/, nullptr, nullptr, nullptr)); } ---------------- todo? ================ Comment at: llvm/lib/IR/DebugInfoMetadata.cpp:634 unsigned ScopeLine, Metadata *ContainingType, unsigned VirtualIndex, - int ThisAdjustment, DIFlags Flags, DISPFlags SPFlags, Metadata *Unit, - Metadata *TemplateParams, Metadata *Declaration, Metadata *RetainedNodes, - Metadata *ThrownTypes, StorageType Storage, bool ShouldCreate) { + int ThisAdjustment, DIFlags Flags, DISPFlags SPFlags, bool isConstexpr, + Metadata *Unit, Metadata *TemplateParams, Metadata *Declaration, ---------------- `isConstExpr` throughout this module. ================ Comment at: llvm/lib/IR/LLVMContextImpl.h:892 uint32_t AlignInBits; + bool isConstant; ---------------- `isConstExpr` ================ Comment at: llvm/lib/IR/LLVMContextImpl.h:925 + isConstant == RHS->isConstant()*/ + ; /*TODO*/ } ---------------- TODO? ================ Comment at: llvm/lib/IR/LLVMContextImpl.h:970 + isConstant == RHS->isConstant()*/ + ; } ---------------- commented-out bits of code? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73261/new/ https://reviews.llvm.org/D73261 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits