[PATCH] D146595: [clang] Add "debug_trampoline" attribute

Thu Apr 20 10:48:26 PDT 2023

dblaikie added a comment.

In D146595#4281710 <https://reviews.llvm.org/D146595#4281710>, @aprantl wrote:

> In D146595#4278361 <https://reviews.llvm.org/D146595#4278361>, @dblaikie wrote:
>
>> In D146595#4235340 <https://reviews.llvm.org/D146595#4235340>, @augusto2112 wrote:
>>
>>> In D146595#4235333 <https://reviews.llvm.org/D146595#4235333>, @aprantl wrote:
>>>
>>>> I hope I'm not kicking off a long bike-shedding thread, but I would propose to either call the attribute `transparent` to match the DWARF attribute name, or if we want to be more descriptive, `debug_transparent`, or `transparentdebug` to fit better with other attributes such as `nodebug`. My vote is on `transparent`.
>>>
>>> The dwarf attribute is actually called `trampoline`, not `transparent`. `trampoline` is a pretty generic name which could be used for other things other than debugging, so maybe `trampolinedebug`?
>>
>> FWIW, trampoline seems weird to me (both for the DWARF attribute, and the source level attribute) - my understanding is that trampolines are small functions that just immediately jump to some other function. In this case the intermediate function can be arbitrarily complicated, but just not /interesting/ to the average developer. Doesn't seem like a trampoline to me. (though maybe I'm thinking of a "thunk" specifically - at least wikipedia's deefinition of a programming trampoline is fairly broad/involves some fairly higher level "do some non-trivial things" operations, I guess)
>>
>> I guess an example of something that might be transparent but not trampoline-like would be std::sort - the ability to step into the comparison function, but skip over the actual sorting logic (or for_each, etc - you might want to step into your functor, but skip the intermediate logic that's handling the looping, etc). Those don't feel like trampolines, I guess?
>
> I think I agree. If we go with some kind of `transparent` we have the options of
>
> 1. `transparent` (which is quite broad)
> 2. `transparentdebug` (which follows the schema started by `nodebug`) I think this is ugly
> 3. `transparent_debug` (which is oddly inconsistent with `nodebug` but more readable)
> 4. `debug_transparent` (which starts a new nomenclature)
>
> My preferential vote is 4 (best), 2, 3, 1.

We do already have https://clang.llvm.org/docs/AttributeReference.html#standalone-debug - so maybe (3) is the way to go? but worst comes to worst we could rename standalone_debug to debug_standalone and deprecate the old spelling if it's worth it.

>>>> & there was some tangent about DWARF v COFF too, which I wouldn't mind weighing in on, but feel like it's all a bit fragmented, so not sure where all the discussions are/how to keep track of them.
>>>
>>> That was also in this review; @aaron.ballman pointed out that it would be best if new Clang attributes weren't targeting only DWARF, though I believe this request may run into some hard limitations of what CodeView/PDB can support.
>>
>> Yeah, that seems like a bit of a big ask, since there's a specific DWARF feature we're lowering to here - feels out of scope to go trying to figure out how it might be implemented in CodeView. For features that are format-agnostic (like simple template names, no-standalone-debug (various type homing strategies), etc) yeah, makes total sense for them to be format-agnostic. This isn't one of those, though.
>>
>> but, I guess, worth at least asking the question if there's an equivalent feature in CV already that this should be mapped to
>
> I already tried to research this — see my comment above "just my code" above. My layman understanding is that they use an external database of uninteresting symbols.
>
> Now if we are going to say this explicitly DWARF only feature, that could be an argument to stick with the `trampoline` name. But I'd rather say this is a general feature that — at this point in time — is only implement on DWARF targets.

Yeah, I'd be inclined to think of this as not a DWARF-specific feature, but one only implemented in DWARF at the moment. If CV adds some way to inject names into the database of uninteresting symbols, they could use this too.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146595/new/

https://reviews.llvm.org/D146595