This is an archive of the discontinued LLVM Phabricator instance.

[FPEnv] New document for adding new constrained FP intrinsics
ClosedPublic

Authored by kpn on Mar 26 2019, 11:58 AM.

Details

Summary

Just like it it says on the tin: This document describes how to add a new constrained floating point intrinsics. It is intended to be a starting point for this topic.

It was split out of D55897, which itself was split out of D43515.

Diff Detail

Repository
rL LLVM

Event Timeline

kpn created this revision.Mar 26 2019, 11:58 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 26 2019, 11:58 AM
docs/AddingConstrainedIntrinsics.rst
62 ↗(On Diff #192310)

I don't understand this last sentence. Our strict nodes do use the chain and the non-strict versions don't, right?

BTW, Craig tells me that we're probably going to have to rethink the who mutateStrictFPToFP think to keep instruction ordering correct. I'm not suggesting that you change that in this document now. Just mentioning it as something to think about.

cameron.mcinally marked an inline comment as done.Mar 29 2019, 2:18 PM
cameron.mcinally added inline comments.
docs/AddingConstrainedIntrinsics.rst
62 ↗(On Diff #192310)

BTW, Craig tells me that we're probably going to have to rethink the who mutateStrictFPToFP think to keep instruction ordering correct. I'm not suggesting that you change that in this document now. Just mentioning it as something to think about.

Digressing a bit...

I also ran into problems with mutateStrictFPToFP and Custom lowerings in D54649.

Ideally mutateStrictFPToFP would go away once we have proper backend support. E.g. D55506. Although, I suppose it will take some time for all the backends to get up to speed, so mutateStrictFPToFP will have to live on for some time... :/

kpn marked an inline comment as done.Apr 10 2019, 10:54 AM
kpn added inline comments.
docs/AddingConstrainedIntrinsics.rst
62 ↗(On Diff #192310)

I think I wrote that back when I was trying to avoid use of the chain. Plus, some of the code got moved to the legalizer since then. I've reworded this to be more current and reflect use of the chain.

kpn updated this revision to Diff 194547.Apr 10 2019, 10:56 AM
This revision is now accepted and ready to land.Apr 10 2019, 12:58 PM
This revision was automatically updated to reflect the committed changes.