llvm dev - Feb 2020 - [LLVM][DISubprogram][LL format updation query] Question regarding moving DISubprogram DIFlags to DISPFlag.

Hello,

In regard to the review request https://reviews.llvm.org/D74470,
I am trying to move five of the DIFlags to DISPFlag for the moment namely
DIFlagExplicit, DIFlagPrototyped, DIFlagNoReturn, DIFlagThunk,
DIFlagAllCallsDescribed.
The llvm ir format for DISubprogram currently has backword compatibility where
the isLocal, isDefinition, virtuality, isOptimized and SPFlags are mutually
exclusive.
My question is,
is it a good idea to remove the booleans support'(isLocal, isDefinition) and
move most of it to spflags and flags in llvm ir? The llvm ir backward
compatibility does not list the clear requirements on documentations page. This
change affects more then 750 ll files.

Regards,
Chirag Partel.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200220/0ffe3376/attachment.html>

Hi Chirag,

On 20.2.20. 07:51, Chirag Patel via llvm-dev wrote:
> Hello,
> 
>  
> 
> In regard to the review request https://reviews.llvm.org/D74470,
> 
> I am trying to move five of the DIFlags to DISPFlag for the moment namely
DIFlagExplicit, DIFlagPrototyped, DIFlagNoReturn, DIFlagThunk,
DIFlagAllCallsDescribed.
> 
> The llvm ir format for DISubprogram currently has backword compatibility
where the isLocal, isDefinition, virtuality, isOptimized and SPFlags are
mutually exclusive.
> 
> My question is,
> 
> is it a good idea to remove the booleans support’(isLocal, isDefinition)
and move most of it to spflags and flags in llvm ir?
But it was already "done", we currently have the
'DISPFlagLocalToUnit' and 'DISPFlagDefinition' (please take a
look into the https://reviews.llvm.org/D54755 and
https://reviews.llvm.org/D59288).

The llvm ir backward compatibility does not list the clear requirements on
documentations page. This change affects more then 750 ll
files.
> 
I am not sure what change will take 750 ll files? Removing the 'isLocal'
and 'isDefinition'? I think the role of LLVM IR backward compatibility
is to support interpretation of the old metadata in terms of the newest one.
Therefore, if there is an 'isDefinition' metadata field, that should be
interpreted as 'DISPFlagDefinition'.

> 
> Regards,
> 
> Chirag Partel.
>

Yes, removing the support for isLocal, isDefinition fields completely from ll
files, currently the LLParser still parses it. I want to remove it and update
the all the ll files which still uses it.
Also the metadata read will support old format, no changes in that.

so if ll file has isLocal and isDefinition it will result in parser error. But
the bitcode read will work as usual.

- Chirag.

-----Original Message-----
From: Djordje Todorovic <djordje.todorovic at rt-rk.com> 
Sent: 20 February 2020 14:16
To: Chirag Patel <Chirag at raincode.com>; llvm-dev at lists.llvm.org
Subject: Re: [llvm-dev] [LLVM][DISubprogram][LL format updation query] Question
regarding moving DISubprogram DIFlags to DISPFlag.

Hi Chirag,

On 20.2.20. 07:51, Chirag Patel via llvm-dev wrote:
> Hello,
> 
>  
> 
> In regard to the review request https://reviews.llvm.org/D74470,
> 
> I am trying to move five of the DIFlags to DISPFlag for the moment namely
DIFlagExplicit, DIFlagPrototyped, DIFlagNoReturn, DIFlagThunk,
DIFlagAllCallsDescribed.
> 
> The llvm ir format for DISubprogram currently has backword compatibility
where the isLocal, isDefinition, virtuality, isOptimized and SPFlags are
mutually exclusive.
> 
> My question is,
> 
> is it a good idea to remove the booleans support'(isLocal,
isDefinition) and move most of it to spflags and flags in llvm ir?
But it was already "done", we currently have the
'DISPFlagLocalToUnit' and 'DISPFlagDefinition' (please take a
look into the https://reviews.llvm.org/D54755 and
https://reviews.llvm.org/D59288).

The llvm ir backward compatibility does not list the clear requirements on
documentations page. This change affects more then 750 ll
files.
> 
I am not sure what change will take 750 ll files? Removing the 'isLocal'
and 'isDefinition'? I think the role of LLVM IR backward compatibility
is to support interpretation of the old metadata in terms of the newest one.
Therefore, if there is an 'isDefinition' metadata field, that should be
interpreted as 'DISPFlagDefinition'.

> 
> Regards,
> 
> Chirag Partel.
>

(+ the usual debug info folks just to highlight it/bring it to their
attention (I see Paul's already chimed in, but didn't want to leave him
out
;) ))

On Wed, Feb 19, 2020 at 10:51 PM Chirag Patel via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> Hello,
>
>
>
> In regard to the review request https://reviews.llvm.org/D74470,
>
> I am trying to move five of the DIFlags to DISPFlag for the moment namely
> DIFlagExplicit, DIFlagPrototyped, DIFlagNoReturn, DIFlagThunk,
> DIFlagAllCallsDescribed.
>
> The llvm ir format for DISubprogram currently has backword compatibility
> where the isLocal, isDefinition, virtuality, isOptimized and SPFlags are
> mutually exclusive.
>
> My question is,
>
> is it a good idea to remove the booleans support’(isLocal, isDefinition)
> and move most of it to spflags and flags in llvm ir? The llvm ir backward
> compatibility does not list the clear requirements on documentations page.
> This change affects more then 750 ll files.
>
>
>
> Regards,
>
> Chirag Partel.
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200220/9502e2b4/attachment.html>