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