Attempt to fix AMD 1.10 conj

[kshyatt]

checked that 1.11 works with this. Happy for stylistic fixes -- it seems on 1.10 and AMD compilation of conj doesn't work but creating a custom f_conj does, mysterious but ok.

[github-actions]

Your PR no longer requires formatting changes. Thank you for your contribution!

[codecov]

Codecov Report

❌ Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
ext/StridedAMDGPUExt.jl	66.66%	1 Missing ⚠️

Files with missing lines	Coverage Δ
ext/StridedGPUArraysExt.jl	49.25% <100.00%> (ø)
src/mapreduce.jl	80.50% <100.00%> (+0.05%)	⬆️
ext/StridedAMDGPUExt.jl	16.66% <66.66%> (+16.66%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[lkdvos]

Is this something we should report to AMDGPU.jl though? It might be reasonable to just give them a MWE with a kernel that uses conj?

[kshyatt]

I already opened an issue there JuliaGPU/AMDGPU.jl#931

[lkdvos]

Should we just wait and see what the verdict is there? In a perfect world this is just fixed upstream and we don't even have to include this :)

[kshyatt]

Yeah, I don't know how long it's going to take for someone who knows the AMD compiler better than I to take a look, though.

[kshyatt]

In the meantime people can Pkg.dev this branch and keep making progress I guess

[lkdvos]

Or use Julia 1.11+?

[kshyatt]

IDK if anyone is stuck on 1.10 for whatever reason 🤷

[Jutho]

This PR is the start of descending into Enzyme style code 🛩️ .

[kshyatt]

This PR is the start of descending into Enzyme style code 🛩️ .

😭 😭 😭

i just want to help people conjugate their arrays

[Jutho]

This probably creates a new f_conj on every substitute_ops call? Is it not better to have an external _conj(x) = real(x) - imag(x) * im definition, and substitute that?

[Jutho]

Does normal conjugation of an array work on AMD? It is just ridiculous that this solution works and normal conj does not. Anyway, since this is restricted to the extension (which I didn't appreciate before), I would be willing to accept it if this helps people.

[kshyatt]

Does normal conjugation of an array work on AMD?

Yes, which is crazy.

It is just ridiculous that this solution works and normal conj does not.

I'm not happy about it either 🙀

[kshyatt]

It helps me mostly by causing CI to pass :/

[lkdvos]

I think I would be okay with merging something like this if we could restrict it to be only for AMD, it seems silly to have to deal with this for CUDA?

[kshyatt]

Yeah I can move it to the AMD extension and we could define a function _conj in the main package that normally points to just good old conj, and leave a comment there reminding us to remove it ASAP?

[kshyatt]

Cleaned this up quite a bit and left comments reminding us to get rid of it as soon as we can

[lkdvos]

As a small suggestion, how about instead of the type and substitute_op, we define _get_op(x::ROCStridedView) = x.op === conj ? _conj : x.op with a default _get_op(x::StridedView) = x.op? This can then be broadcast in the kernel, which might be easier to read there anyways.

[kshyatt]

Aren't we passing the raw ROCArrays to the kernel, not the StridedViews? That's why I put this in _mapreduce_block initially

[kshyatt]

Anyway, look ok now? Tests seem happy