[llvm-dev] Why does a DISubprogram need to be distinct?

[Adrian Prantl via llvm-dev]

> On Sep 25, 2020, at 4:36 PM, David Blaikie <dblaikie at gmail.com> wrote:
> 
> On Fri, Sep 25, 2020 at 4:08 PM Adrian Prantl <aprantl at apple.com> wrote:
>> 
>> First — thanks for fixing the test for me!
> 
> I'm a bit curious about the test - any idea how it came to be, if it's
> invalid? Did we produce such bitcode in the past and don't anymore -
> what's the rule about backwards compatibility here, then? It seems
> like any time we regenerate a bitcode compat test that's questionable
> because it means we won't be compatible with that bitcode we said we
> should be compatible with?

I think I mentioned in the commit message — we had a short period in clang with a bug

>>> Clang right before https://reviews.llvm.org/D79967 <https://reviews.llvm.org/D79967> would generate this kind of broken IR.

and the swift-5.3 branch happened to be cut in that unlucky window. To trigger it you need to markup a function with NoDebug (the bug was introduced  maybe a month earlier).

> 
>> We have two kinds of DISubprograms:
>> - uniqued DISubprograms are part of the type system and describe function *types*. They don't have a unit: field, because they want to be uniqued across compile units.
>> - distinct DISubprograms describe one specific function definition. They belong to a compile unit and thus have a unit: field.
>> 
>> You are only supposed to attach distinct DISubprograms to function definitions. Several places in CodeGen will unconditionally dereference the unit: field and crash if it isn't there.
> 
> But do they have to be distinct? They have different fields (as you
> say, declaration DISubprograms don't have a "unit:" field, definition
> DISubprograms do have a unit: field that refers to a (itself distinct)
> DICompileUnit)

IIRC, DIBuilder lets you either create a distinct one with a unit or a uniqued one without. I don't know if the backend would survive the same uniqued DISubprogram node being attached to multiple llvm::Functions or if that would invalidate some assumptions... In any case distinct seems to be a reasonable proxy for what we mean.

-- adrian

> 
>> We actually had a weaker form of the check I added to Verifier already in place (https://github.com/llvm/llvm-project/blob/51cad041e0cb26597c7ccc0fbfaa349b8fffbcda/llvm/lib/IR/Verifier.cpp#L1186), but it didn't cover the specific testcase I added in https://reviews.llvm.org/rGe17f52d623cc146b7d9bf5a2e02965043508b4c4.
>> 
>> -- adrian

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200925/1e3e1c58/attachment.html>