This is an archive of the discontinued LLVM Phabricator instance.

Differential D111520

[TypeSwitch/Compiler.h] Provide a LLVM_NODEBUG macro and use it in TypeSwitch.h
ClosedPublic

Authored by lattner on Oct 10 2021, 9:21 PM.

Details

Reviewers
dblaikie
abdulras
rriddle
mehdi_amini
aeubanks
Commits
rGe0582e69f517: [TypeSwitch/Compiler.h] Provide a LLVM_NODEBUG macro and use it in TypeSwitch.h
Summary

TypeSwitch.h is used pervasively in MLIR and often has dozens of types switched
over. It uses "zero cost" variadic templates to implement the dispatching
mechanism... which isn't zero cost in debug builds, and which causes a massive
problem for actually debugging things that use it - you get dozens of nonsense
frames in the debugger for simple things like a visitor.

Fix this by marking the key method in TypeSwitch as nodebug + alwaysinline.
This resolves LLVM PR49301

Diff Detail

Event Timeline

lattner created this revision.Oct 10 2021, 9:21 PM
Herald added subscribers: Chia-hungDuan, dexonsmith, rriddle. · View Herald TranscriptOct 10 2021, 9:21 PM
lattner requested review of this revision.Oct 10 2021, 9:21 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 10 2021, 9:21 PM
Herald added subscribers: llvm-commits, stephenneuendorffer. · View Herald Transcript
lattner added reviewers: dblaikie, abdulras, rriddle, mehdi_amini.Oct 10 2021, 9:22 PM

FWIW, the useless frames end up looking like this:

frame #10: 0x00000001002bec1a circt-opt`llvm::TypeSwitch<mlir::Operation*, mlir::LogicalResult>& llvm::TypeSwitch<mlir::Operation*, mlir::LogicalResult>::Case<circt::sv::AssertOp, circt::sv::Visitor<(anonymous namespace)::StmtEmitter, mlir::LogicalResult>::dispatchSVVisitor(this=0x00007000128653c0, caseFn=0x00007000128653b8)::'lambda'(auto)&>(mlir::LogicalResult&&) at TypeSwitch.h:116:16
frame #11: 0x00000001002b3e81 circt-opt`circt::sv::Visitor<(anonymous namespace)::StmtEmitter, mlir::LogicalResult>::dispatchSVVisitor(mlir::Operation*) [inlined] llvm::TypeSwitch<mlir::Operation*, mlir::LogicalResult>& llvm::detail::TypeSwitchBase<llvm::TypeSwitch<mlir::Operation*, mlir::LogicalResult>, mlir::Operation*>::Case<circt::sv::AssertOp, circt::sv::AssumeOp, circt::sv::CoverOp, circt::sv::AssertConcurrentOp, circt::sv::AssumeConcurrentOp, circt::sv::CoverConcurrentOp, circt::sv::BindOp, circt::sv::Visitor<(anonymous namespace)::StmtEmitter, mlir::LogicalResult>::dispatchSVVisitor(this=0x00007000128653c0, caseFn=0x00007000128653b8)::'lambda'(auto)&>(circt::sv::Visitor<(anonymous namespace)::StmtEmitter, mlir::LogicalResult>::dispatchSVVisitor(mlir::Operation*)::'lambda'(auto)&) at TypeSwitch.h:40:29
frame #12: 0x00000001002b3e67 circt-opt`circt::sv::Visitor<(anonymous namespace)::StmtEmitter, mlir::LogicalResult>::dispatchSVVisitor(mlir::Operation*) [inlined] llvm::TypeSwitch<mlir::Operation*, mlir::LogicalResult>& llvm::detail::TypeSwitchBase<llvm::TypeSwitch<mlir::Operation*, mlir::LogicalResult>, mlir::Operation*>::Case<circt::sv::ReadInterfaceSignalOp, circt::sv::AssertOp, circt::sv::AssumeOp, circt::sv::CoverOp, circt::sv::AssertConcurrentOp, circt::sv::AssumeConcurrentOp, circt::sv::CoverConcurrentOp, circt::sv::BindOp, circt::sv::Visitor<(anonymous namespace)::StmtEmitter, mlir::LogicalResult>::dispatchSVVisitor(this=0x00007000128653c0, caseFn=0x00007000128653b8)::'lambda'(auto)&>(circt::sv::Visitor<(anonymous namespace)::StmtEmitter, mlir::LogicalResult>::dispatchSVVisitor(mlir::Operation*)::'lambda'(auto)&) at TypeSwitch.h:41:19
frame #13: 0x00000001002b3e34 circt-opt`circt::sv::Visitor<(anonymous namespace)::StmtEmitter, mlir::LogicalResult>::dispatchSVVisitor(mlir::Operation*) [inlined] llvm::TypeSwitch<mlir::Operation*, mlir::LogicalResult>& llvm::detail::TypeSwitchBase<llvm::TypeSwitch<mlir::Operation*, mlir::LogicalResult>, mlir::Operation*>::Case<circt::sv::AssignInterfaceSignalOp, circt::sv::ReadInterfaceSignalOp, circt::sv::AssertOp, circt::sv::AssumeOp, circt::sv::CoverOp, circt::sv::AssertConcurrentOp, circt::sv::AssumeConcurrentOp, circt::sv::CoverConcurrentOp, circt::sv::BindOp, circt::sv::Visitor<(anonymous namespace)::StmtEmitter, mlir::LogicalResult>::dispatchSVVisitor(this=0x00007000128653c0, caseFn=0x00007000128653b8)::'lambda'(auto)&>(circt::sv::Visitor<(anonymous namespace)::StmtEmitter, mlir::LogicalResult>::dispatchSVVisitor(mlir::Operation*)::'lambda'(auto)&) at TypeSwitch.h:41:19
frame #14: 0x00000001002b3dfb circt-opt`circt::sv::Visitor<(anonymous namespace)::StmtEmitter, mlir::LogicalResult>::dispatchSVVisitor(mlir::Operation*) [inlined] llvm::TypeSwitch<mlir::Operation*, mlir::LogicalResult>& llvm::detail::TypeSwitchBase<llvm::TypeSwitch<mlir::Operation*, mlir::LogicalResult>, mlir::Operation*>::Case<circt::sv::GetModportOp, circt::sv::AssignInterfaceSignalOp, circt::sv::ReadInterfaceSignalOp, circt::sv::AssertOp, circt::sv::AssumeOp, circt::sv::CoverOp, circt::sv::AssertConcurrentOp, circt::sv::AssumeConcurrentOp, circt::sv::CoverConcurrentOp, circt::sv::BindOp, circt::sv::Visitor<(anonymous namespace)::StmtEmitter, mlir::LogicalResult>::dispatchSVVisitor(this=0x00007000128653c0, caseFn=0x00007000128653b8)::'lambda'(auto)&>(circt::sv::Visitor<(anonymous namespace)::StmtEmitter, mlir::LogicalResult>::dispatchSVVisitor(mlir::Operation*)::'lambda'(auto)&) at TypeSwitch.h:41:19
frame #15: 0x00000001002b3dc2 circt-opt`circt::sv::Visitor<(anonymous namespace)::StmtEmitter, mlir::LogicalResult>::dispatchSVVisitor(mlir::Operation*) [inlined] llvm::TypeSwitch<mlir::Operation*, mlir::LogicalResult>& llvm::detail::TypeSwitchBase<llvm::TypeSwitch<mlir::Operation*, mlir::LogicalResult>, mlir::Operation*>::Case<circt::sv::InterfaceInstanceOp, circt::sv::GetModportOp, circt::sv::AssignInterfaceSignalOp, circt::sv::ReadInterfaceSignalOp, circt::sv::AssertOp, circt::sv::AssumeOp, circt::sv::CoverOp, circt::sv::AssertConcurrentOp, circt::sv::AssumeConcurrentOp, circt::sv::CoverConcurrentOp, circt::sv::BindOp, circt::sv::Visitor<(anonymous namespace)::StmtEmitter, mlir::LogicalResult>::dispatchSVVisitor(this=0x00007000128653c0, caseFn=0x00007000128653b8)::'lambda'(auto)&>(circt::sv::Visitor<(anonymous namespace)::StmtEmitter, mlir::LogicalResult>::dispatchSVVisitor(mlir::Operation*)::'lambda'(auto)&) at TypeSwitch.h:41:19
frame #16: 0x00000001002b3d89 circt-opt`circt::sv::Visitor<(anonymous namespace)::StmtEmitter, mlir::LogicalResult>::dispatchSVVisitor(mlir::Operation*) [inlined] llvm::TypeSwitch<mlir::Operation*, mlir::LogicalResult>& llvm::detail::TypeSwitchBase<llvm::TypeSwitch<mlir::Operation*, mlir::LogicalResult>, mlir::Operation*>::Case<circt::sv::InterfaceModportOp, circt::sv::InterfaceInstanceOp, circt::sv::GetModportOp, circt::sv::AssignInterfaceSignalOp, circt::sv::ReadInterfaceSignalOp, circt::sv::AssertOp, circt::sv::AssumeOp, circt::sv::CoverOp, circt::sv::AssertConcurrentOp, circt::sv::AssumeConcurrentOp, circt::sv::CoverConcurrentOp, circt::sv::BindOp, circt::sv::Visitor<(anonymous namespace)::StmtEmitter, mlir::LogicalResult>::dispatchSVVisitor(this=0x00007000128653c0, caseFn=0x00007000128653b8)::'lambda'(auto)&>(circt::sv::Visitor<(anonymous namespace)::StmtEmitter, mlir::LogicalResult>::dispatchSVVisitor(mlir::Operation*)::'lambda'(auto)&) at TypeSwitch.h:41:19
frame #17: 0x00000001002b3d50 circt-opt`circt::sv::Visitor<(anonymous namespace)::StmtEmitter, mlir::LogicalResult>::dispatchSVVisitor(mlir::Operation*) [inlined] llvm::TypeSwitch<mlir::Operation*, mlir::LogicalResult>& llvm::detail::TypeSwitchBase<llvm::TypeSwitch<mlir::Operation*, mlir::LogicalResult>, mlir::Operation*>::Case<circt::sv::InterfaceSignalOp, circt::sv::InterfaceModportOp, circt::sv::InterfaceInstanceOp, circt::sv::GetModportOp, circt::sv::AssignInterfaceSignalOp, circt::sv::ReadInterfaceSignalOp, circt::sv::AssertOp, circt::sv::AssumeOp, circt::sv::CoverOp, circt::sv::AssertConcurrentOp, circt::sv::AssumeConcurrentOp, circt::sv::CoverConcurrentOp, circt::sv::BindOp, circt::sv::Visitor<(anonymous namespace)::StmtEmitter, mlir::LogicalResult>::dispatchSVVisitor(this=0x00007000128653c0, caseFn=0x00007000128653b8)::'lambda'(auto)&>(circt::sv::Visitor<(anonymous namespace)::StmtEmitter, mlir::LogicalResult>::dispatchSVVisitor(mlir::Operation*)::'lambda'(auto)&) at TypeSwitch.h:41:19
frame #18: 0x00000001002b3d17 circt-opt`circt::sv::Visitor<(anonymous namespace)::StmtEmitter, mlir::LogicalResult>::dispatchSVVisitor(mlir::Operation*) [inlined] llvm::TypeSwitch<mlir::Operation*, mlir::LogicalResult>& llvm::detail::TypeSwitchBase<llvm::TypeSwitch<mlir::Operation*, mlir::LogicalResult>, mlir::Operation*>::Case<circt::sv::InterfaceOp, circt::sv::InterfaceSignalOp, circt::sv::InterfaceModportOp, circt::sv::InterfaceInstanceOp, circt::sv::GetModportOp, circt::sv::AssignInterfaceSignalOp, circt::sv::ReadInterfaceSignalOp, circt::sv::AssertOp, circt::sv::AssumeOp, circt::sv::CoverOp, circt::sv::AssertConcurrentOp, circt::sv::AssumeConcurrentOp, circt::sv::CoverConcurrentOp, circt::sv::BindOp, circt::sv::Visitor<(anonymous namespace)::StmtEmitter, mlir::LogicalResult>::dispatchSVVisitor(this=0x00007000128653c0, caseFn=0x00007000128653b8)::'lambda'(auto)&>(circt::sv::Visitor<(anonymous namespace)::StmtEmitter, mlir::LogicalResult>::dispatchSVVisitor(mlir::Operation*)::'lambda'(auto)&) at TypeSwitch.h:41:19
frame #19: 0x00000001002b3cde circt-opt`circt::sv::Visitor<(anonymous namespace)::StmtEmitter, mlir::LogicalResult>::dispatchSVVisitor(mlir::Operation*) [inlined] llvm::TypeSwitch<mlir::Operation*, mlir::LogicalResult>& llvm::detail::TypeSwitchBase<llvm::TypeSwitch<mlir::Operation*, mlir::LogicalResult>
Harbormaster completed remote builds in B128033: Diff 378563.Oct 10 2021, 11:54 PM
mehdi_amini accepted this revision.Oct 11 2021, 9:27 AM

Nice!

This revision is now accepted and ready to land.Oct 11 2021, 9:27 AM
aeubanks added a subscriber: aeubanks.Oct 11 2021, 9:47 AM
aeubanks added inline comments.
llvm/include/llvm/Support/Compiler.h
255

LLVM_ATTRIBUTE_NODEBUG? When I saw LLVM_NODEBUG the first thing I thought of was the opposite of LLVM_DEBUG, which is very different.

lattner updated this revision to Diff 378765.Oct 11 2021, 12:54 PM

Switch to LLVM_ATTRIBUTE_NODEBUG as Mehdi suggests, it is much more clear.

rriddle added a comment.Oct 11 2021, 12:59 PM

Nice!

aeubanks accepted this revision.Oct 11 2021, 12:59 PM
This revision was landed with ongoing or failed builds.Oct 11 2021, 1:10 PM
Closed by commit rGe0582e69f517: [TypeSwitch/Compiler.h] Provide a LLVM_NODEBUG macro and use it in TypeSwitch.h (authored by lattner). · Explain Why
This revision was automatically updated to reflect the committed changes.
lattner added a commit: rGe0582e69f517: [TypeSwitch/Compiler.h] Provide a LLVM_NODEBUG macro and use it in TypeSwitch.h.
Harbormaster completed remote builds in B128178: Diff 378765.Oct 11 2021, 1:55 PM

Revision Contents

PathSize
llvm/
include/
llvm/
ADT/
7 lines
Support/
9 lines

Diff 378772

llvm/include/llvm/ADT/TypeSwitch.h

Show All 29 Linespublic:
/// TypeSwitchBase is not copyable. /// TypeSwitchBase is not copyable.
TypeSwitchBase(const TypeSwitchBase &) = delete; TypeSwitchBase(const TypeSwitchBase &) = delete;
void operator=(const TypeSwitchBase &) = delete; void operator=(const TypeSwitchBase &) = delete;
void operator=(TypeSwitchBase &&other) = delete; void operator=(TypeSwitchBase &&other) = delete;
/// Invoke a case on the derived class with multiple case types. /// Invoke a case on the derived class with multiple case types.
template <typename CaseT, typename CaseT2, typename... CaseTs, template <typename CaseT, typename CaseT2, typename... CaseTs,
typename CallableT> typename CallableT>
DerivedT &Case(CallableT &&caseFn) { // This is marked always_inline and nodebug so it doesn't show up in stack
// traces at -O0 (or other optimization levels). Large TypeSwitch's are
// common, are equivalent to a switch, and don't add any value to stack
// traces.
LLVM_ATTRIBUTE_ALWAYS_INLINE LLVM_ATTRIBUTE_NODEBUG DerivedT &
Case(CallableT &&caseFn) {
DerivedT &derived = static_cast<DerivedT &>(*this); DerivedT &derived = static_cast<DerivedT &>(*this);
return derived.template Case<CaseT>(caseFn) return derived.template Case<CaseT>(caseFn)
.template Case<CaseT2, CaseTs...>(caseFn); .template Case<CaseT2, CaseTs...>(caseFn);
} }
/// Invoke a case on the derived class, inferring the type of the Case from /// Invoke a case on the derived class, inferring the type of the Case from
/// the first input of the given callable. /// the first input of the given callable.
/// Note: This inference rules for this overload are very simple: strip /// Note: This inference rules for this overload are very simple: strip
▲ Show 20 Lines • Show All 136 Lines • Show Last 20 Lines

llvm/include/llvm/Support/Compiler.h

Show First 20 Lines • Show All 242 Lines • ▼ Show 20 Lines
#if __has_attribute(always_inline) || LLVM_GNUC_PREREQ(4, 0, 0) #if __has_attribute(always_inline) || LLVM_GNUC_PREREQ(4, 0, 0)
#define LLVM_ATTRIBUTE_ALWAYS_INLINE inline __attribute__((always_inline)) #define LLVM_ATTRIBUTE_ALWAYS_INLINE inline __attribute__((always_inline))
#elif defined(_MSC_VER) #elif defined(_MSC_VER)
#define LLVM_ATTRIBUTE_ALWAYS_INLINE __forceinline #define LLVM_ATTRIBUTE_ALWAYS_INLINE __forceinline
#else #else
#define LLVM_ATTRIBUTE_ALWAYS_INLINE inline #define LLVM_ATTRIBUTE_ALWAYS_INLINE inline
#endif #endif
/// LLVM_ATTRIBUTE_NO_DEBUG - On compilers where we have a directive to do
/// so, mark a method "no debug" because debug info makes the debugger
/// experience worse. GCC introduced this in GCC 4.0
#if __has_attribute(nodebug) || LLVM_GNUC_PREREQ(4, 0, 0)
#define LLVM_ATTRIBUTE_NODEBUG __attribute__((nodebug))
aeubanksUnsubmitted
Not Done
ReplyInline Actions

LLVM_ATTRIBUTE_NODEBUG? When I saw LLVM_NODEBUG the first thing I thought of was the opposite of LLVM_DEBUG, which is very different.

aeubanks: `LLVM_ATTRIBUTE_NODEBUG`? When I saw `LLVM_NODEBUG` the first thing I thought of was the…
#else
#define LLVM_ATTRIBUTE_NODEBUG
#endif
#if __has_attribute(returns_nonnull) || LLVM_GNUC_PREREQ(4, 9, 0) #if __has_attribute(returns_nonnull) || LLVM_GNUC_PREREQ(4, 9, 0)
#define LLVM_ATTRIBUTE_RETURNS_NONNULL __attribute__((returns_nonnull)) #define LLVM_ATTRIBUTE_RETURNS_NONNULL __attribute__((returns_nonnull))
#elif defined(_MSC_VER) #elif defined(_MSC_VER)
#define LLVM_ATTRIBUTE_RETURNS_NONNULL _Ret_notnull_ #define LLVM_ATTRIBUTE_RETURNS_NONNULL _Ret_notnull_
#else #else
#define LLVM_ATTRIBUTE_RETURNS_NONNULL #define LLVM_ATTRIBUTE_RETURNS_NONNULL
#endif #endif
▲ Show 20 Lines • Show All 292 Lines • Show Last 20 Lines