Hi, Also, I think there is a bigger problem lurking than just with norecurse. I think that in general, functionattrs is not very good with attributes when intrinsics are present. E.g. https://bugs.llvm.org/show_bug.cgi?id=34696 Here dbg.value prevents both norecurse and readnone from being deduced. So, it would be nice to fix this for norecurse, but it would be even nicer to fix it for intrinsics and attributes in general. Regards, Mikael On 10/18/2017 07:24 PM, Sanjoy Das via llvm-dev wrote:> That list is not complete - pretty much every intrinsic that can throw > can also recurse (not for any fundamental reason -- just that > intrinsics that throw do that by calling some other function, and that > callee function can also be recursive). You may also want to ask Gor > about the coro_ intrinsics. > > The way you're going about the change is risky though -- have you > considered adding a NoRecurse property instead? That way you can > start marking most of the "obviously" non-recursive intrinsics right > away and slowly expand the set. > > This is asymmetric with "Throws", but for good reason - before Throws > every intrinsics was implicitly assumed to be no-throw, which is the > opposite situation from norecurse, IIUC. > > -- Sanjoy > > On Wed, Oct 18, 2017 at 8:53 AM, David Stenberg via llvm-dev > <llvm-dev at lists.llvm.org> wrote: >>> From: piotrekpad at gmail.com [mailto:piotrekpad at gmail.com] On Behalf Of Piotr Padlewski >>> Sent: den 16 oktober 2017 17:33 >>> >>> Hi David, >>> The patch didn't get a lot of attention, so probably others does not >>> consider it a huge deal. Anyway, if someone is willing to review this, I >>> can pursue rebasing it. >> >> Okay. We are interested in getting something akin to your patch delivered; at >> least so that the dbg intrinsics gets marked as norecurse. >> >> Unfortunately I'm not very familiar with more "esoteric" intrinsics, so I can't >> bring anything special to the table when it comes to reviewing that the set of >> exempted intrinsics is correct. Other than that I can gladly help in whatever >> way to help this patch land. >> >> (I rebased your patch, and it seems like there where only a few, minor updates >> required in three files. Other than that, there are four tests in clang that >> need some smaller fixes.) >> >>> Piotr >> >> Best regards, >> David >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >
On Thu, Oct 19, 2017 at 10:23 PM, Mikael Holmén via llvm-dev < llvm-dev at lists.llvm.org> wrote:> Hi, > > Also, I think there is a bigger problem lurking than just with norecurse.Yes> I think that in general, functionattrs is not very good with attributes > when intrinsics are present. >> > E.g. > https://bugs.llvm.org/show_bug.cgi?id=34696 > > Here dbg.value prevents both norecurse and readnone from being deduced. >A lot of these are the more general issue of intrinsics not being marked with proper memory attributes as a form of attempted control dependence/optimization blocking/etc Intrinsics.td even says this.> So, it would be nice to fix this for norecurse, but it would be even nicer > to fix it for intrinsics and attributes in general. >Nobody as of yet has signed up to fix this, because it often requires significant thinking about each intrinsic and what really should be happening to it, modeling that, and teaching optimizers to deal with it. Instead the large hammer is chosen. Eventually it'll matter enough to performance for someone to do the work :) -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20171019/0d8cc010/attachment.html>
On 10/20/2017 12:36 AM, Daniel Berlin via llvm-dev wrote:> > > On Thu, Oct 19, 2017 at 10:23 PM, Mikael Holmén via llvm-dev > <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote: > > Hi, > > Also, I think there is a bigger problem lurking than just with > norecurse. > > Yes > > I think that in general, functionattrs is not very good with > attributes when intrinsics are present. > > > E.g. > https://bugs.llvm.org/show_bug.cgi?id=34696 > <https://bugs.llvm.org/show_bug.cgi?id=34696> > > Here dbg.value prevents both norecurse and readnone from being > deduced. > > > A lot of these are the more general issue of intrinsics not being > marked with proper memory attributes as a form of attempted control > dependence/optimization blocking/etc > > Intrinsics.td even says this. > > > So, it would be nice to fix this for norecurse, but it would be > even nicer to fix it for intrinsics and attributes in general. > > > Nobody as of yet has signed up to fix this, because it often requires > significant thinking about each intrinsic and what really should be > happening to it, modeling that, and teaching optimizers to deal with it. > > Instead the large hammer is chosen. > > Eventually it'll matter enough to performance for someone to do the > work :)I agree with this, but I also agree with Sanjoy: We should add a NoRecurse property and mark intrinsics with it. This fits our general scheme for intrinsics (i.e., "if we say nothing, we assume the worst"). The fact that we model all sorts of dependencies as memory dependencies is also a problem, but a somewhat independent one. -Hal> > > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev-- Hal Finkel Lead, Compiler Technology and Programming Languages Leadership Computing Facility Argonne National Laboratory -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20171025/c9627a0c/attachment-0001.html>