Repository navigation
C front-end: resolve vector right shifts to lshr or ashr - #9234
traffaillac wants to merge 3 commits into
Conversation
For scalars, typecheck_expr_shifts turns the generic ID_shr into ID_lshr or ID_ashr depending on the signedness of the left operand. For vector shifts (vector >> scalar and vector >> vector), it returned before doing so, leaving an ID_shr that neither remove_vector nor the back-ends support. The back-end then printed "warning: ignoring shr" and left the result unconstrained, so assertions about it failed spuriously. Resolve ID_shr from the element type of the vector in both vector cases, using a new helper resolve_vector_shr. The new regression test gcc_vector_shift_right covers both cases, with unsigned (logical shift) and signed (arithmetic shift) elements. Without this change it reports "warning: ignoring shr" twice and both assertions fail.
|
Many thanks for your contribution! The test will need appropriate I am wondering what happens if you try to shift a vector whose elements are _Bool? An enum, possibly with an underlying type? Some floating-point or fixed-point type? |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #9234 +/- ##
========================================
Coverage 80.91% 80.92%
========================================
Files 1718 1718
Lines 190292 190335 +43
Branches 73 73
========================================
+ Hits 153974 154024 +50
+ Misses 36318 36311 -7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The test uses __attribute__((vector_size(...))), which other compilers such as Visual Studio do not support. Guard it with #ifdef __GNUC__, as the existing gcc_vector1 and gcc_vector3 tests do, so that the test trivially succeeds on such platforms instead of failing to parse. The gcc-only tag is no longer needed.
Vectors can only have signed, unsigned, floating-point or fixed-point elements (typecheck_vector_type rejects _Bool and enum elements). The shift type checker accepted any of these, but the back-ends only support shifts of integers: a right shift of a vector of floats or fixed-point values was ignored with "warning: ignoring shr", leaving the result unconstrained, and a left shift of a vector of floats failed an invariant in numeric_cast_v. As GCC and Clang do, report an error for shifts whose left operand is a vector with non-integer elements. resolve_vector_shr can then assume signed or unsigned elements. The new float.desc and fixedbv.desc tests in gcc_vector_shift_right check the error for a right shift of floats and a left shift of fixed-point values.
|
#ifdef added, is this the right way to do so? (vs. moving the test earlier so that is is not even compiled on Windows) On your second question, below is the answer from Claude. TLDR : probably some issues left, and to complement AI answer I can make it fix them too, either on this PR or another one. I don't use vectors of _Bool, enum or float, so I won't be able to report more bugs on them in the future. Cheers, On element types: _Bool and enum elements (with or without an underlying type) are already rejected by typecheck_vector_type ("cannot make a vector of subtype"), matching Clang. Floating-point and fixed-point elements were accepted, though: >> was then ignored by the back-end ("warning: ignoring shr"), and << on a float vector even failed an invariant in numeric_cast_v. I've added a commit that rejects shifts of vectors with non-integer elements, as GCC and Clang do, with tests for both cases. For the cast PR, I extended the test to signed, float and fixed-point elements, which the fix already handles. |
|
Yes, looks good. Only missing a run of |
Hi,
Thanks for this amazing project! I am beginning to use it to validate a video codec making heavy use of GCC vector extensions (https://github.com/tvlabs/edge264).
The code to reproduce this bug and the fix were generated by Claude Opus 5.5, as I am not familiar with this codebase. I insctructed it to follow the points below. Please tell me if they need manual refining in any direction.
Cheers,
Thibault Raffaillac