Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mark cuarray as noalias #2395

Merged
merged 2 commits into from
May 28, 2024
Merged

Mark cuarray as noalias #2395

merged 2 commits into from
May 28, 2024

Conversation

wsmoses
Copy link
Contributor

@wsmoses wsmoses commented May 24, 2024

@vchuravy @maleadt I don't suppose you know a better way to get this info from julia effects a priori.

Has corresponding EnzymeCore update.

Note this is part of an ongoing effort for Enzyme to successfully differentiate (in fwd mode to start with) Cuda broadcasts/etc

@maleadt
Copy link
Member

maleadt commented May 25, 2024

I don't suppose you know a better way to get this info from julia effects a priori.

I have no idea what Enzyme.noalias means or what you're asking. If you're asserting here that CuArrays can't alias, that's wrong, because they can (views/reshapes/etc are materialized as CuArrays).

@wsmoses
Copy link
Contributor Author

wsmoses commented May 25, 2024

Partially, so I'm hoping to make something which generates an llvm noalias on the result of a call (which permits optimizations -- which are particularly helpul for broadcasting).

Specifically, here I'd like to provide the semantics a fresh allocation of a CuArray doesn't alias any other memory in the system (but of course a view taken of that variable may alias the original variable).

@wsmoses
Copy link
Contributor Author

wsmoses commented May 25, 2024

And yeah I'm looking to add it (EnzymeAD/Enzyme.jl#1467) as a companion to this PR to be able to propagate this information.

In contrast jl_alloc_array_1d we are able to add LLVM-level noalias this ourselves, but to add noalias for codes out of Julia base requires an interface function was my thought.

I'd love to alternatively read this in thorugh Julia effects, but I don't know if that has something helpful here.

@wsmoses wsmoses marked this pull request as ready for review May 27, 2024 11:50
@wsmoses
Copy link
Contributor Author

wsmoses commented May 27, 2024

@vchuravy with this passing, mind approving/merging?

@maleadt maleadt merged commit beff592 into JuliaGPU:master May 28, 2024
1 check passed
@maleadt
Copy link
Member

maleadt commented May 28, 2024

with this passing

It did not:

extensions/enzyme: Error During Test at /var/lib/buildkite-agent/builds/gpuci-4/julialang/cuda-dot-jl/test/extensions/enzyme.jl:74
  Got exception outside of a @test
  UndefVarError: `f` not defined

Changing f to firstsum triggers a scalar iteration warning. I'm reverting the PR.


Can you please verify the Enzyme tests pass locally before marking a PR ready for review? We don't have the capacity for CI-driven development. Also please include [skip julia] [skip cuda] etc tags to avoid running unnecessary tests.

maleadt added a commit that referenced this pull request May 28, 2024
@wsmoses
Copy link
Contributor Author

wsmoses commented May 28, 2024

Oh rip sorry I saw the green on CI and assumed it was fine, I guess it just didn't run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies About things we use.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants