shafik marked an inline comment as done.
shafik added inline comments.
================
Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:3659
// Use llvm function name.
- Name = Fn->getName();
+ if (Fn->getName().startswith("___Z"))
+ LinkageName = Fn->getName();
----------------
dblaikie wrote:
> shafik wrote:
> > dblaikie wrote:
> > > shafik wrote:
> > > > dblaikie wrote:
> > > > > shafik wrote:
> > > > > > dblaikie wrote:
> > > > > > > aprantl wrote:
> > > > > > > > aprantl wrote:
> > > > > > > > > Could you please add a comment that Clang Blocks are
> > > > > > > > > generated as raw llvm::Functions but do have a mangled name
> > > > > > > > > and that is handling this case? Otherwise this would look
> > > > > > > > > suspicious.
> > > > > > > > Should *all* raw LLVM functions have their name as the linkage
> > > > > > > > name? Perhaps a raw LLVM function should only have a linkage
> > > > > > > > name and no human-readable name?
> > > > > > > Seems plausible to me - do we have any data on other types of
> > > > > > > functions that hit this codepath?
> > > > > > So it was not obvious to me what other cases would this branch so I
> > > > > > added an assert and ran `check-clang` and from that I saw four
> > > > > > cases that ended up here:
> > > > > >
> > > > > > `GenerateCapturedStmtFunction`
> > > > > > `GenerateOpenMPCapturedStmtFunction`
> > > > > > `GenerateBlockFunction`
> > > > > > `generateDestroyHelper`
> > > > > >
> > > > > > It is not obvious to me we want to alter the behavior of any of the
> > > > > > other cases.
> > > > > Could you show any small source examples & their corresponding DWARF
> > > > > & how that DWARF would change? (what names are we using, what names
> > > > > would we end up using/what sort of things are they naming)
> > > > Ok, I understand the objections to special casing like this. We ended
> > > > up setting both the `Name` and `LinkageName` unconditionally in this
> > > > branch because not setting the name for subroutines end up with us
> > > > generating `DW_TAG_subprogram` without a `DW_AT_name` which is not
> > > > valid. We discovered this when running the LLDB test suite.
> > > If we need to have a name, and these are mangled names - were they
> > > mangled /from/ something & have an unmangled name we should be using,
> > > then?
> > >
> > > llvm-cxxfilt demangles the example/test as "invocation function for block
> > > in f(void (int) block_pointer)" - perhaps we should name this "invocation
> > > function"? & let the scope of the DIE communicate the rest of the
> > > information about this thing (like the "operator()" for a lambda is just
> > > "operator()")?
> > If we take the example from the test I added it demangles to:
> >
> > ```
> > invocation function for block in f(void (int) block_pointer)
> > ```
> >
> > There is no usable "short" name there, it is a complex description of the
> > block and what wraps it. In general from the LLDB perspective we would want
> > a name to set a breakpoint or display it during a back trace. In this case
> > we care about displaying this properly in a back trace and it would not be
> > reasonable to use such a name to set a breakpoint.
> >
> How's that compare to lambdas, for example?
I updated the PR to used a more targeted approach based on
`CodeGenFunction::GenerateBlockFunction` also generating a mangled name for
blocks when it creates the `llvm::function`.
Lambdas are interesting, they are RecordDecl with a callable `operator()` so we
have both a `DW_AT_name` for them and a `DW_AT_linkage_name` for the callable.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73282/new/
https://reviews.llvm.org/D73282
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
