[PATCH] D143229: [FunctionImporter] Add flag to disable upgrading debug info

Tue Feb 14 08:09:56 PST 2023

tejohnson added a comment.

In D143229#4113874 <https://reviews.llvm.org/D143229#4113874>, @aprantl wrote:

> In D143229#4107958 <https://reviews.llvm.org/D143229#4107958>, @mehdi_amini wrote:
>
>> Looking at the implementation:
>>
>> 1. Contrary to the name of the API, this does not seem to "upgrade" anything: it merely drops outdated debug info. (Am I missing something?)
>> 2. This runs the verifier but does not fail verification on debug info related issue, instead it'll drop metadata when something is wrong there.
>>
>> So are we re-running the IR verifier independently? And if we don't what kind of inconsistency would we be setting up?
>
> The name "upgrade" comes from a time when regularly bumped `DEBUG_METADATA_VERSION`. Back then it "upgraded" older debug info formats by dropping the outdated metadata. In the more recent past we are trying to make debug info upgradeable in the bit code reader. Unfortunately, because debug info metadata is a cyclic graph we keep finding bugs where the metadata produced by older versions of LLVM and/or 3rd party frontends is malformed in a way that causes problems (crashes, infinite loops, ...) with newer version of LLVM. Whenever we find such a problem we improve the IR Verifier to detect the malformed IR.

It seems reasonable to me to provide a way to skip this for situations where the build system guarantees that the bitcode is always generated by the same compiler, e.g. in a distributed ThinLTO build where any change in compiler will force a rebuild anyway, which is the case @aeubanks is looking at.

@aeubanks are we also invoking UpgradeDebugInfo, or the Verifier separately, on the main input bitcode in the distributed ThinLTO backend clang invocation (e.g. the clang -x ir input file)? For the same reason, we should be able to skip that. So I wonder if it makes sense then for the disabling mechanism (e.g. flag or whatever) to be in UpgradeDebugInfo or the Verifier itself?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143229