[PATCH] D73261: [dwarf5] Support DebugInfo for constexpr for C++ variables and functions
Paul Robinson via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Jan 23 12:47:58 PST 2020
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
More information about the cfe-commits
mailing list