Skip to content
This repository has been archived by the owner on Nov 4, 2024. It is now read-only.

[Enzyme] Mark certain operations as Enzyme inactive #69

Merged
merged 4 commits into from
May 12, 2024
Merged

Conversation

avik-pal
Copy link
Member

No description provided.

@avik-pal avik-pal force-pushed the ap/mark_inactive branch 3 times, most recently from be96fce to 12f75f7 Compare May 12, 2024 19:12
@avik-pal avik-pal force-pushed the ap/mark_inactive branch 3 times, most recently from e044ebd to 5e4ef71 Compare May 12, 2024 20:22
@avik-pal avik-pal linked an issue May 12, 2024 that may be closed by this pull request
@@ -130,6 +130,7 @@ end
@inline _dropout_fptype(x) = float(real(eltype(x)))

CRC.@non_differentiable _dropout_fptype(::Any...)
EnzymeRules.inactive(::typeof(_dropout_fptype), ::Any...) = nothing

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you see if it still works if you change to inactive_noinl for all of these?

It will be performant if so

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think some of these are not needed at all. Is there a cost of defining these functions?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inactive_noinl is strictly beneficial. It adds inactive metadata to the call/body which can improve performance or do nothing in the worst case if Enzyme can already prove it inactive on own.

inactive itself will stop inlining so there's a potential performance penalty. It will, however, guarantee that the activity information is propagated (whereas the top is presently a best effort procedure).

@wsmoses
Copy link

wsmoses commented May 12, 2024

It's worthwhile still keeping the other ones as noinl FYI

@avik-pal avik-pal merged commit e829b63 into main May 12, 2024
19 of 22 checks passed
@avik-pal avik-pal deleted the ap/mark_inactive branch May 12, 2024 22:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fuse GroupNorm Activation into the main kernel
2 participants