Arthur Eubanks via llvm-dev
2021-Apr-12 23:49 UTC
[llvm-dev] Visitation of declarations in FunctionAtrs with new and old pass managers?
> > We should decide to visit declarations consistently with function passmanagers, which currently don't visit declarations.> I couldn't parse this statement, can you rephrase?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. 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). I suppose it's possible to introduce a call to an arbitrary function in some module pass, but that seems out of the ordinary. On Mon, Apr 12, 2021 at 12:00 PM Philip Reames <listmail at philipreames.com> 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. > > > > 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 > >>> > >>> >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210412/32cd25da/attachment.html>
Philip Reames via llvm-dev
2021-Apr-13 02:00 UTC
[llvm-dev] Visitation of declarations in FunctionAtrs with new and old pass managers?
On 4/12/21 4:49 PM, Arthur Eubanks wrote:> > > We should decide to visit declarations consistently with function > pass managers, which currently don't visit declarations. > > I couldn't parse this statement, can you rephrase? > 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.This makes sense. It's important to emphasize this is about the new pass manager. I don't believe this matches the behavior of the old PM for either pass type.> > 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). > I suppose it's possible to introduce a call to an arbitrary function > in some module pass, but that seems out of the ordinary.How does this connect? Everything you said parses; I just don't see how this is connected to the topic of this thread. Philip> > On Mon, Apr 12, 2021 at 12:00 PM Philip Reames > <listmail at philipreames.com <mailto:listmail at philipreames.com>> 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. > > > > 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 <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 <mailto: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 > >>> > >>> >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210412/2ad7e798/attachment.html>