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

via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 30 09:40:12 PST 2018



> -----Original Message-----
> From: llvm-commits [mailto:llvm-commits-bounces at lists.llvm.org] On Behalf
> Of Duncan P. N. Exon Smith via llvm-commits
> Sent: Friday, November 30, 2018 12:26 PM
> To: reviews+D54755+public+5d150b7c4452e0eb at reviews.llvm.org
> Cc: sontuan.vu119 at gmail.com; anastasis.gramm2 at gmail.com;
> hiraditya at msn.com; djordje.todorovic at rt-rk.com; llvm-
> commits at lists.llvm.org; nikola.prica at rt-rk.com; ewan at codeplay.com;
> asowda at cisco.com
> Subject: Re: [PATCH] D54755: [DebugInfo] IR/Bitcode changes for
> DISubprogram flags
> 
> 
> 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.

If we thought we would be migrating flags one at a time, that would be one
thing.  I'd rather do a proper audit and reorg them all at once, which will
minimize the pain (until one of the flag words runs out of bits again).

FTR, I would not be able to pick up a task like this until at least February.
But at least the Fortran flags have a more reasonable home now.
--paulr

> 
> > 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
> >
> >
> >
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


More information about the llvm-commits mailing list