Page MenuHomePhabricator

[AArch64] Make target intrinsics DefaultAttrIntrinsics.
ClosedPublic

Authored by fhahn on Thu, Jan 14, 7:09 AM.

Details

Summary

DefaultAttrIntrinsics was introduced to add very common attributes to a
large set of intrinsics.

Currently the added attributes include:

nofree nosync nounwind willreturn

I think those should hold for most AArch64 target intrinsics, but
there are too many to check manually. This patch makes most AArch64 target
intrinsics DefaultAttrsIntrinsics.

Some notable exceptions I think are exclusive loads and stores as well
as the memory barrier intrinsics, for which nosync does not apply I
think.

Diff Detail

Event Timeline

fhahn created this revision.Thu, Jan 14, 7:09 AM
fhahn requested review of this revision.Thu, Jan 14, 7:09 AM
Herald added a project: Restricted Project. · View Herald TranscriptThu, Jan 14, 7:09 AM

Some notable exceptions I think are exclusive loads and stores as well as the memory barrier intrinsics, for which nosync does not apply I think.

FWIW, that sounds about right.

Really happy people start to use this for target intrinsics as well.

Some notable exceptions I think are exclusive loads and stores as well as the memory barrier intrinsics, for which nosync does not apply I think.

FWIW, that sounds about right.

Really happy people start to use this for target intrinsics as well.

Totally agree! :)

(Some ARM-aware ppl should review/accept)

SjoerdMeijer accepted this revision.Fri, Jan 15, 7:35 AM

Looks like we get a lot of happiness from using these attributes. ;-)
I had a look at it before and the general direction also looked okay to me too, which I was happy to see confirmed. Therefore I am happy to accept this, even though I haven't checked all intrinsics because this is a lot of changes. So perhaps wait a day with committing in case anyone wants to have one more look.

This revision is now accepted and ready to land.Fri, Jan 15, 7:35 AM
This revision was landed with ongoing or failed builds.Mon, Jan 18, 9:37 AM
This revision was automatically updated to reflect the committed changes.
fhahn added a comment.Mon, Jan 18, 9:39 AM

Looks like we get a lot of happiness from using these attributes. ;-)
I had a look at it before and the general direction also looked okay to me too, which I was happy to see confirmed. Therefore I am happy to accept this, even though I haven't checked all intrinsics because this is a lot of changes. So perhaps wait a day with committing in case anyone wants to have one more look.

Thanks! The changes will at least ensure that there shouldn't be regression after D94106.

I also excluded the instructions in the transactional memory extension: 291ac7e622d5