-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix ConstantInt::get for LLVM main change #8918
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
base: main
Are you sure you want to change the base?
Conversation
Summary of Changes The issue was caused by LLVM commit a83c89495ba6 (LLVM 22+) which disabled implicit truncation in ConstantInt::get() by default. The fix is straightforward: Changes in src/CodeGen_Hexagon.cpp: 1. Line 1279: Changed ConstantInt::get(i32_t, -element_bytes) to ConstantInt::getSigned(i32_t, -element_bytes) - This properly handles the negative value 2. Line 1596-1598: Added an assertion before creating vdelta switch values: internal_assert(switches[i] >= 0 && switches[i] <= 255) << "vdelta switch value " << switches[i] << " doesn't fit in 8 bits\n"; - This ensures values fit in 8 bits rather than silently truncating - For HVX vectors (max 128 elements), switches should only use bits 0-6, so this should never fail in practice - If it does fail, it indicates a bug that needs fixing 3. Line 1615: Changed ConstantInt::get(scalar_ty, val) to ConstantInt::getSigned(scalar_ty, val) in create_vector() - The val parameter can be negative (e.g., -1 for creating all-ones vectors) Co-authored-by: Claude Code
|
This was in fact entirely authored by claude code, with a fair amount of babysitting. The real value was not the lines of code that it changed, it was that it successfully diagnosed the exact nature of the problem by reading the llvm commit log and hunting through our codegen classes for problems. |
|
For consistency when committing, please adjust the |
alexreinking
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a pleasingly straightforward fix.
|
For the record I should say what babysitting was required: At first it came up with nearly the above, except instead of the assert it wanted to explicitly enable truncation by passing extra args to ConstantInt::get. This required using a new flag though in the LLVM API, so I told it to add helper functions gated on LLVM_VERSION, which it did. Then I thought better of it and told it to add an assert instead of allowing truncation, which meant we could roll back the helper functions and land on this simpler fix. |
|
Seems like there's another issue in |
Summary of Changes
The issue was caused by LLVM commit a83c89495ba6 (LLVM 22+) which disabled implicit truncation in
ConstantInt::get() by default. The fix is straightforward:
Changes in src/CodeGen_Hexagon.cpp:
-element_bytes)
internal_assert(switches[i] >= 0 && switches[i] <= 255) << "vdelta switch value " << switches[i] << " doesn't fit in 8 bits\n";
in practice
create_vector()
Co-authored-by: Claude Code noreply@anthropic.com