llvm dev - Oct 2017 - Is every intrinsic norecurse?

> 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

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

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
>