search

EnzymeAD / Enzyme.jl

Julia bindings for the Enzyme automatic differentiator
https://enzyme.mit.edu
MIT License
422 stars 58 forks source link

Workaround breakage in llvm.jl #1497

Closed wsmoses closed 3 weeks ago

vchuravy commented 3 weeks ago

x-ref: https://github.com/maleadt/LLVM.jl/pull/414

wsmoses commented 3 weeks ago

@vchuravy apparenrlt we need to backported to 11 too see slack

maleadt commented 3 weeks ago

I pushed & tagged a deprecation, so this should be fixed for users.

I would recommend pirating internals like that though. It's also not clear to me why you're overriding the function in the first place, https://github.com/EnzymeAD/Enzyme/blob/79e829910a63618e1bb5843b8564031a8345305c/enzyme/Enzyme/CApi.cpp#L913-L919 seems wrong (metadata can be attached to function values, not only instructions).

wsmoses commented 3 weeks ago

Yeah but we only needed it to attach to instructions. But it’s awesome that we won’t need to vendor that

On Thu, Jun 6, 2024 at 7:04 PM Tim Besard @.***> wrote:

I pushed & tagged a deprecation, so this should be fixed for users.

I would recommend pirating internals like that though. It's also not clear to me why you're overriding the function in the first place, https://github.com/EnzymeAD/Enzyme/blob/79e829910a63618e1bb5843b8564031a8345305c/enzyme/Enzyme/CApi.cpp#L913-L919 seems wrong (metadata can be attached to function values, not only instructions).

— Reply to this email directly, view it on GitHub https://github.com/EnzymeAD/Enzyme.jl/pull/1497#issuecomment-2153001905, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJTUXCVJWRUASHABW553R3ZGCI77AVCNFSM6AAAAABI5AFCZGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNJTGAYDCOJQGU . You are receiving this because you authored the thread.Message ID: @.***>

vchuravy commented 3 weeks ago

apparenrlt we need to backported to 11 too see slack

Enzyme.jl 0.11 only ever supported LLVM.jl v6 so no?

wsmoses commented 3 weeks ago

Oh awesome then. And per the llvm.jl patch should we change this now to if not defined valuemetadatadict

On Thu, Jun 6, 2024 at 7:12 PM Valentin Churavy @.***> wrote:

apparenrlt we need to backported to 11 too see slack

Enzyme.jl 0.11 only ever supported LLVM.jl v6 so no?

— Reply to this email directly, view it on GitHub https://github.com/EnzymeAD/Enzyme.jl/pull/1497#issuecomment-2153023183, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJTUXEUSGSVXDA6XDFKP53ZGCJ7NAVCNFSM6AAAAABI5AFCZGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNJTGAZDGMJYGM . You are receiving this because you authored the thread.Message ID: @.***>