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

[Philip Reames via llvm-dev]

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.
>
> 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
>>>
>>>