The patches for the RemoveDIs textual IR representation are in the process of landing, but as part of the verifier changes, some differences between debug intrinsics and debug records have been raised for handling invalid code; specifically, some inputs that are currently verifier failures will become parser failures. The difference between parser and verifier errors are important primarily because a verifier error can be recovered from by dropping debug info for the module, while a parser error is always unrecoverable.
The points of failure for debug intrinsics and records can be broadly categorized as:
- Syntax
- Arity
- Types
- Semantics
Debug intrinsics are generally quite permissive in the parsing phase, willing to accept any number of parameters of any valid type, and the verifier will later enforce the correctness of the number, type, and semantic expectations of those parameters; only bad syntax is a parser error.
Debug records are stricter; the parser has strict expectations for the parameter count of each type of record, and all parameters must be MDNodes, with the exception of value/address parameters which can be any type of metadata. Initially these checks were even stricter, as DIExpressions were forced to be inline (since they never need to be forward-referenced), but this requirement has been relaxed. All other semantic errors, including ensuring that the parameters are the correct type of MDNode/metadata, are still verifier errors.
This means that there is existing IR that currently parses correctly and will have debug info dropped by the verifier, that if directly converted to the new debug info format will fail in the parsing stage. I don’t believe this should be an issue - we continue to support parsing old IR, and no compiler that produces RemoveDI-IR will produce output that fails in the parser; the only point of friction is in automatically-updated tests that intentionally contain invalid debug info; as the intent of these is likely to specifically test verifier behaviour, it should be acceptable to either drop these tests or change them to expect a parser error.
Hopefully this makes the changes clear - if anyone has any thoughts or complaints about this change then I’d appreciate hearing them!