[clang] [llvm] [KeyInstr] Add DISubprogram::keyInstructions bit (PR #144107)

[Orlando Cazalet-Hyams via cfe-commits]

OCHyams wrote:

> LGTM, I think this raises a few questions about whether we're going to support non-key-instructions code forever more. AFAIUI the premise is that the debugging behaviour is worse if you've got groupless instructions in a key-instructions program?

Worse is an understatement, as you won't step on any of the instructions (except calls).

> There might be various routes that introduce these things:
> 
> * Old bitcode being autoupgraded,
> * Developers who turn the option off,
> * LLVM frontends that don't support / implement key instructions,
> 
> Which over time will hopefully reduce / change / be implemented. Presumably in some future date we'll be able to make key instructions non-optional. Can you forsee us doing anything differently in anticipation of that?

The path to making key instructions non-optional looks long, and more difficult than, for example, making debug records non-optional (as that's NFC (-ish, since it changed textual format)). As well as different front ends (not) supporting it (which could be a large ask, unless we also supply the "just tag every store" pass) there's the valid objections to running it at O0 as it exists now. So I think that any such future is sufficiently far away that introducing dedicated is-it-on bits is reasonable. In short, I think the answer is no.

> One wonders whether DebugKeyInstructions should be a mandatory non-default argument, to let people know that something significant is changing, and meaning that we don't have to bake the default into all the callsites. On the other hand, it's not like this is a common or frequently used API call.

I thought about this and decided to see whether anyone on review had strong opinions. Do you have a preference?

The same could be said for the textual DISubprogram field, which isn't printed if it's false (and is loaded as false if it's not present). We could make it mandatory. That could come as a separate PR, if you think it sounds like a good idea.

> arg to DISubprograms isn't defaultable, so we shouldn't expand everything?

Not sure I follow this question - can you elaborate?

https://github.com/llvm/llvm-project/pull/144107