Serialize TypeTree offsets as signed constants - #3070
Merged
Conversation
TypeTree offsets may be negative -- -1 denotes "any offset" -- but
TypeTree::toMD emitted them via ConstantInt::get without IsSigned, so -1
became uint64_t 0xFFFFFFFFFFFFFFFF for an i32.
LLVM <= 21 silently truncated this (ConstantInt::get passed
implicitTrunc=true). As of LLVM 23 the default is implicitTrunc=false,
so the same call now trips
APInt.h: Assertion `llvm::isUIntN(BitWidth, val) &&
"Value is not an N-bit unsigned value"' failed.
This became reachable on LLVM >= 23 in e4afe54, which caches constant
global type analysis via GV->setMetadata("enzyme_type", ...). Rust's
autodiff support hits it on essentially every kernel, since its panic
location globals get a TypeTree containing -1 offsets.
Sign-extension is bit-identical to the previous truncating behavior
(-1 -> 0xFFFFFFFF) and matches insertFromMD, which already reads the
offsets back with getSExtValue.
Also fix two other ConstantInt::get(Ty, -1) sites found by audit that
would assert the same way on LLVM >= 23 for sub-64-bit types.
Fixes #3060
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P65yn8LELKWU1dq6AQvA5f
The test uses opaque pointers, but %newLoadEnzyme appends -opaque-pointers=0 on LLVM 16, so `ptr` failed to parse. Use the %OPnewLoadEnzyme substitution that exists for exactly this case, rather than an explicit -opaque-pointers flag (which LLVM >= 17 rejects). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P65yn8LELKWU1dq6AQvA5f
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3060 —
Assertion \llvm::isUIntN(BitWidth, val) && "Value is not an N-bit unsigned value"'` on LLVM 23.Root cause
TypeTree::toMDserializes offsets into!enzyme_typemetadata:pair.firstis anintoffset, and TypeTree uses-1to mean "any offset". That converts touint64_t0xFFFFFFFFFFFFFFFF, which is not a 32-bit unsigned value.LLVM changed the behavior under us:
ConstantInt::get(IntegerType*, uint64_t V, bool IsSigned)APInt(BitWidth, V, isSigned, /*implicitTrunc=*/true)— silently truncatesAPInt(Ty->getBitWidth(), V, IsSigned, ImplicitTrunc=false)— assertsWhy it started firing now
This is a latent bug that only became reachable on this path in e4afe54 ("Speed up large constant ta"), which caches constant-global type analysis:
Rust hits it on essentially every autodiff kernel, since its panic-location globals get a TypeTree containing
-1offsets (!{!"Unknown", i32 -1, ...}).Backtrace from a local LLVM 23 build:
Note this is not LLVM-23-only — LLVM 24 asserts identically. The opt21 link in the issue passes only because of the implicit truncation.
Fix
Pass
IsSigned=true. This is bit-identical to the old truncating behavior (-1→0xFFFFFFFF) and matches the reader side,insertFromMD, which already usesgetSExtValue().Two other
ConstantInt::get(Ty, -1)sites found by audit are fixed the same way (MustExitScalarEvolution.cpp,Utils.cpp:nextPowerOfTwo). Both would assert identically on LLVM ≥ 23 for sub-64-bit types; I could not construct an input that reaches either, so they carry no dedicated test.Testing
New test
test/Enzyme/ForwardMode/globalconsttypemd.llis reduced from the issue reproducer and pins the emitted!enzyme_typeoffsets. Verified it fails (asserts) without the fix and passes with it, on LLVM 21, 23, and 24.check-enzymeon LLVM 24, before vs. after:isUIntNabortsThe 30 aborts were spread across existing tests (
ForwardMode/globalfn.ll,ReverseMode/storeconstexpr.ll, theProbProgandomp*tests, …). The remaining failures are pre-existing CHECK-line drift for LLVM 23/24 IR printing, unrelated to this change.🤖 Generated with Claude Code
https://claude.ai/code/session_01P65yn8LELKWU1dq6AQvA5f