[PATCH] D54755: [DebugInfo] IR/Bitcode changes for DISubprogram flags

Fri Nov 30 09:26:00 PST 2018

Yes, it forces you to explicitly consider when adding/removing flags how to upgrade bitcode appropriately.  It also designs away bugs related to reorganizing the IR enum.  But maybe the maintenance overhead isn’t worth it. 

> On Nov 30, 2018, at 08:55, Paul Robinson via Phabricator <reviews at reviews.llvm.org> wrote:
> 
> probinson added a comment.
> 
> In D54755#1313679 <https://reviews.llvm.org/D54755#1313679>, @dexonsmith wrote:
> 
>> In D54755#1313098 <https://reviews.llvm.org/D54755#1313098>, @probinson wrote:
>> 
>>> In D54755#1313064 <https://reviews.llvm.org/D54755#1313064>, @probinson wrote:
>>> 
>>>> In D54755#1312971 <https://reviews.llvm.org/D54755#1312971>, @aprantl wrote:
>>>> 
>>>>> While I was updating some internal testcases I wondered: why is DIFlagPrototype not a DISPFlag? Does this flag make sense on something other than functions?
>>>> 
>>>> 
>>>> Because I was not interested in moving existing flags from the old word to the new word in the first round.  There are others that probably apply only to subprograms:
>>>> NoReturn, MainSubprogram; probably Thunk, All CallsDescribed; maybe Trivial, Explicit.
>>>> Really we should do some kind of audit, and do a bulk move in one go so we can minimize the bitcode-upgrade pain.
>>> 
>>> 
>>> One additional problem is that the existing flags are exposed as part of the public API, i.e. to front-ends, so we'll need to work out how to avoid Bad Stuff(tm) if we repurpose any existing flag bits.
>> 
>> 
>> Another option here is not to merge them at all in bitcode (I didn't consider this before).  You can add an abbreviation for a fixed-width field of 1 bit each:
>> https://llvm.org/docs/BitCodeFormat.html#fixed-width-value
> 
> 
> Wouldn't that cause every new flag to be a bitcode format change?  The value of having a flags word is that new flags in the same word don't affect the bitcode format.

> 
> 
> Repository:
>  rL LLVM
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D54755/new/
> 
> https://reviews.llvm.org/D54755
> 
> 
> 