[PATCH] D90345: [DebugInfo] Fix ICE in DwarfCompileUnit::constructSubprogramScopeDIE

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D90345#2888538 <https://reviews.llvm.org/D90345#2888538>, @scott.linder wrote:

> In D90345#2888514 <https://reviews.llvm.org/D90345#2888514>, @dblaikie wrote:
>
>> In D90345#2887719 <https://reviews.llvm.org/D90345#2887719>, @scott.linder wrote:
>>
>>> Update tests.
>>>
>>> I went the route of "upgrading" bitcode which lacks the `type:` field on `DISubprogram`s. It seems like there is currently no difference from LLVM's perspective between `!DISubroutineType(types: !{})` and `!DISubroutineType(types: !{null})`, or at least the difference (or whether the first form should even be valid) is not clear to me.
>>>
>>> As an example, DebugTypeInfoRemoval uses `!DISubroutineType(types: !{})` to represent the C type `(void)()`, but the langref would lead one to believe this should be `!DISubroutineType(types: !{null})`.
>>>
>>> Does anyone have any thoughts?
>>
>> I'd lean slightly towards `!{null}` being the representation for a known signature of `void()` - for consistency.
>
> Would it be worth preceding this patch with another that enforces this? It seems like a verifier check, doc update, and bitcode reader upgrade path from `!{}` to `!{null}` for `DISubroutineType` would clear things up. If so, I can do that and then update this patch.

Maybe? Though might be more effort than its worth - it's probably pretty harmless for !{null} and !{} to be treated the same - especially if they are already supported? But if we're trying to pick a representation to use in new cases where it comes up I'd lean towards !{null}.

Open to other opinions - @aprantl and co have more context/care more about the verifier .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90345