[llvm-dev] Visitation of declarations in FunctionAtrs with new and old pass managers?

[Philip Reames via llvm-dev]

On 4/12/21 12:00 PM, Philip Reames wrote:
>
> On 4/11/21 8:54 AM, Johannes Doerfert wrote:
>> Passes had to deal with Decl. vs Def. before and they managed.
>> That said, there is only that much you can do with declarations
>> and I would suggest we do it early as part of a module pass.
> This seems to align well with Arthur's suggestion, and seems 
> reasonable to me as well.
Posted for review here: https://reviews.llvm.org/D100400.
>>
>> Attributor run early as module pass will right now *not* annotate
>> the declaration but still derive the information for the call sites:
>> https://godbolt.org/z/b9a8Gheqb
>>
>> We could make it annotate declarations as well with little effort.
> Is Attributor a module pass?  If not, you would seem to have the same 
> problem as the existing code.
>>
>> ~ Johannes
>>
>>
>> On 4/11/21 1:39 AM, Arthur Eubanks wrote:
>>> I think it makes more sense to do something like that in a pass
>>> like InferFunctionAttrsPass. We should decide to visit declarations
>>> consistently with function pass managers, which currently don't visit
>>> declarations. Adding declarations to the new PM CGSCC infra would add
>>> complexity and passes would have to check if they're working with a
>>> declaration vs definition.
>>>
>>> On Fri, Apr 9, 2021 at 1:41 PM Philip Reames 
>>> <listmail at philipreames.com>
>>> wrote:
>>>
>>>> I ran across a dependency in the way the new and old pass managers
>>>> interact with function-attrs.  I'm not sure whether this is expected
>>>> behavior or not, but with some digging, I couldn't find a clear
>>>> motivation.  Anyone have context on this?
>>>>
>>>> Essentially, under the old pass manager, FunctionAttrs appears to 
>>>> visit
>>>> declarations, and under the new one, it doesn't.   Here's an example
>>>> that shows how this can change the output of function-attrs:
>>>>
>>>> $ cat decl.ll
>>>>
>>>> declare void @readnone() readnone
>>>>
>>>> $ ./opt -S -function-attrs decl.ll -enable-new-pm=1
>>>> ; ModuleID = 'decl.ll'
>>>> source_filename = "decl.ll"
>>>>
>>>> ; Function Attrs: readnone
>>>> declare void @readnone() #0
>>>>
>>>> attributes #0 = { readnone }
>>>>
>>>> $ ./opt -S -function-attrs decl.ll -enable-new-pm=0
>>>> ; ModuleID = 'decl.ll'
>>>> source_filename = "decl.ll"
>>>>
>>>> ; Function Attrs: nofree nosync readnone
>>>> declare void @readnone() #0
>>>>
>>>> attributes #0 = { nofree nosync readnone }
>>>>
>>>> (The example uses nofree and nosync, but please don't focus on the
>>>> semantics of those attributes.  That's a separate discussion.)
>>>>
>>>> To me, it seems odd to not have declarations be SCCs of their own, and
>>>> thus passed to function-attrs.  Does anyone have a good explanation 
>>>> for
>>>> why we made this change?  And in particular, what the "right" way of
>>>> inferring attributes for a partially annotated declaration might be in
>>>> our new world?
>>>>
>>>> Philip
>>>>
>>>>