> -----Original Message----- > From: Richard Biener <[email protected]> > Sent: 01 October 2026 07:40 > To: Torbjorn SVENSSON <[email protected]> > Cc: gcc-patches mailing list <[email protected]>; Richard Earnshaw > <[email protected]> > Subject: Re: [PATCH v2] testsuite: Split out known-misaligned V2QI store > target [PR124342] [PR124619] > > On Wed, Sep 30, 2026 at 8:06 PM Torbjorn SVENSSON > <[email protected]> wrote: > > > > > > > > On 2026-09-30 10:01, Richard Biener wrote: > > > On Tue, Sep 29, 2026 at 3:32 PM Torbjorn SVENSSON > > > <[email protected]> wrote: > > >> > > >> > > >> > > >> On 2026-09-29 14:57, Richard Biener wrote: > > >>> On Tue, Sep 29, 2026 at 9:52 AM Torbjorn SVENSSON > > >>> <[email protected]> wrote: > > >>>> > > >>>> > > >>>> > > >>>> On 2026-09-24 15:56, Richard Biener wrote: > > >>>>> On Thu, Sep 24, 2026 at 3:34 PM Torbjorn SVENSSON > > >>>>> <[email protected]> wrote: > > >>>>>> > > >>>>>> > > >>>>>> > > >>>>>> On 2026-09-24 15:25, Richard Biener wrote: > > >>>>>>> On Thu, Sep 24, 2026 at 2:24 PM Torbjorn SVENSSON > > >>>>>>> <[email protected]> wrote: > > >>>>>>>> > > >>>>>>>> > > >>>>>>>> > > >>>>>>>> On 2026-09-23 15:40, Richard Biener wrote: > > >>>>>>>>> On Wed, Sep 2, 2026 at 1:18 PM Torbjörn Svensson via > Sourceware Forge > > >>>>>>>>> <[email protected]> wrote: > > >>>>>>>>>> > > >>>>>>>>>> From: Torbjörn SVENSSON <[email protected]> > > >>>>>>>>>> > > >>>>>>>>>> vect_slp_v2qi_store_unalign tests whether the vectorizer > handles a V2QI > > >>>>>>>>>> store through a byte-aligned base, but some -Wstringop- > overflow tests > > >>>>>>>>>> instead depend on a V2QI store at a known one-byte > misalignment from a > > >>>>>>>>>> defined object. > > >>>>>>>>>> > > >>>>>>>>>> These two variants are not the same on armv7ve with -mfloat- > abi=hard. > > >>>>>>>>>> Add a separate effective target for the latter case. > > >>>>>>>>> > > >>>>>>>>> Can you again explain why they are not the same? I'm guessing > it's > > >>>>>>>>> about knowing the misalignment vs. unknown misalignment but > then > > >>>>>>>>> vect_slp_v2qi_store_misalign vs. _unalign is not obvious. Maybe > > >>>>>>>>> _unknown_misalign vs. _known_misalign? But I'm still curious > > >>>>>>>>> why a target choses to allow one but not the other (for stores > there's > > >>>>>>>>> no dr_explicit_realign[_optimized])? > > >>>>>>>> > > >>>>>>>> Unless I've completly missunderstood the sequence, it should > stem from > > >>>>>>>> > https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/config/arm/arm.cc;hb=refs/h > eads/trunk#l30984 > > >>>>>>>> > > >>>>>>>> After another bisect, I identified that the first failing commit is > > >>>>>>>> r16-4314-g5e9eecc6686 (for r16-4315 and later, r16-4355- > g53085a46e7f162 > > >>>>>>>> is also needed to build). > > >>>>>>>> > > >>>>>>>> I think the reason for the regression is that the value of "align" > > >>>>>>>> is > > >>>>>>>> not the same for TYPE_ALIGN_UNIT(type) and > GET_MODE_UNIT_SIZE(mode). > > >>>>>>>> > > >>>>>>>> In the v2qi tests, mode would match HImode, thus: > > >>>>>>>> > > >>>>>>>> - GET_MODE_UNIT_SIZE(HImode) would return 2. > > >>>>>>>> - TYPE_ALIGN_UNIT(type) would return 1 for char. > > >>>>>>>> > > >>>>>>>> So, maybe my patch is not correct and instead the function should > > >>>>>>>> simply be like this since NEON apparently accepts unknown > alignment: > > >>>>>>>> > > >>>>>>>> static bool > > >>>>>>>> arm_builtin_support_vector_misalignment (machine_mode mode, > > >>>>>>>> int misalignment, > > >>>>>>>> bool is_packed, > > >>>>>>>> bool > > >>>>>>>> is_gather_scatter) > > >>>>>>>> { > > >>>>>>>> if (TARGET_NEON && !BYTES_BIG_ENDIAN && > unaligned_access) > > >>>>>>>> { > > >>>>>>>> HOST_WIDE_INT align = GET_MODE_UNIT_SIZE (mode); > > >>>>>>>> > > >>>>>>>> if (is_gather_scatter) > > >>>>>>>> return true; > > >>>>>>>> > > >>>>>>>> if (is_packed) > > >>>>>>>> return align == 1; > > >>>>>>>> > > >>>>>>>> /* NEON supports access regardless of the alignment. */ > > >>>>>>>> return true; > > >>>>>>>> } > > >>>>>>>> > > >>>>>>>> return default_builtin_support_vector_misalignment (mode, > misalignment, > > >>>>>>>> > > >>>>>>>> is_packed, > > >>>>>>>> > > >>>>>>>> is_gather_scatter); > > >>>>>>>> } > > >>>>>>>> > > >>>>>>>> > > >>>>>>>> WDYT? > > >>>>>>> > > >>>>>>> Yeah, the current code is > > >>>>>>> > > >>>>>>> /* If the misalignment is unknown, we should be able to > handle the access > > >>>>>>> so long as it is not to a member of a packed data > > >>>>>>> structure. */ > > >>>>>>> if (misalignment == -1) > > >>>>>>> return true; > > >>>>>>> > > >>>>>>> /* Return true if the misalignment is a multiple of the > > >>>>>>> natural > alignment > > >>>>>>> of the vector's element type. This is probably always > > >>>>>>> going to > be > > >>>>>>> true in practice, since we've already established that > > >>>>>>> this isn't > a > > >>>>>>> packed access. */ > > >>>>>>> return ((misalignment % align) == 0); > > >>>>>>> > > >>>>>>> which says it accepts any misalignment (misalignment == -1) but not > > >>>>>>> a known misalignment, of say 3. That's just odd. > > >>>>>>> > > >>>>>>>> Also, the check "align == 1" in the is_packed branch is probably > > >>>>>>>> not > > >>>>>>>> doing the right thing since the value in "align" is the size(?) > > >>>>>>>> and not > > >>>>>>>> the alignment. > > >>>>>>> > > >>>>>>> yeah, a vector mode of size 1 probably does not exist. Iff we > > >>>>>>> accept misalignment == -1 there's no reason to reject "is_packed". > > >>>>>>> is_packed just means we cannot reach alignment by peeling. > > >>>>>>> There's no good reason to check is_packed. > > >>>>>> > > >>>>>> So, based on this, maybe the function body could be reduced to: > > >>>>>> > > >>>>>> { > > >>>>>> if (TARGET_NEON && !BYTES_BIG_ENDIAN && unaligned_access) > > >>>>>> return true; > > >>>>>> > > >>>>>> return default_builtin_support_vector_misalignment (mode, > misalignment, > > >>>>>> is_packed, > > >>>>>> > > >>>>>> is_gather_scatter); > > >>>>>> } > > >>>>>> > > >>>>>> I can do this change and do a regression test run on it to see if it > causes > > >>>>>> any regressions if you think this is better. > > >>>>> > > >>>>> Looks reasonable. Note the aarch64 variant does > > >>>>> > > >>>>> if (TARGET_SIMD && STRICT_ALIGNMENT) > > >>>>> { > > >>>>> if (is_gather_scatter) > > >>>>> return true; > > >>>>> > > >>>>> /* Return if movmisalign pattern is not supported for this > > >>>>> mode. > */ > > >>>>> if (optab_handler (movmisalign_optab, mode) == > CODE_FOR_nothing) > > >>>>> return false; > > >>>>> > > >>>>> /* Misalignment factor is unknown at compile time. */ > > >>>>> if (misalignment == -1) > > >>>>> return false; > > >>>>> } > > >>>>> return default_builtin_support_vector_misalignment (mode, > misalignment, > > >>>>> is_packed, > > >>>>> > > >>>>> is_gather_scatter); > > >>>>> > > >>>>> so basically defaults for !STRICT_ALIGNMENT. I suppose the > equivalent > > >>>>> is unaligned_access > > >>>>> on the arm side. Compared to your variant with 'return true' it falls > > >>>>> through to the > > >>>>> default (which tests for movmisalign support of the mode). So if arm > > >>>>> can do that for all > > >>>>> vector modes a return true sounds good, if not, fallthru to the > > >>>>> default hook looks safer. > > >>>> > > >>>> I tried to use the aarch64 approach, but replaced TARGET_SIMD with > TARGET_NEON. > > >>>> This works for the test in question, but it causes other regressions > > >>>> for > the > > >>>> vect-early-break. > > >>>> Then I tried to simply return true for any TARGET_NEON, same result. > > >>>> > > >>>> After having tried all these ways, I got lazy and simply asked AI if > > >>>> it was > > >>>> possible to solve this without having the "type" parameter and the > conclusion > > >>>> it gave me was: > > >>>> > > >>>> There is no generic way for this hook to exactly reproduce > > >>>> `TYPE_ALIGN_UNIT(type)` from only `mode`, `misalignment`, and the > flags: two > > >>>> accesses can have the same hook arguments but different element-type > alignment. > > >>>> The broad NEON `return true` passes `Wstringop-overflow-2.c` but > adds four > > >>>> `vect-early-break` failures. The mode-unit rule with an HImode > adjustment still > > >>>> adds those failures; the full comparison is also affected by different > > >>>> non- > GCC > > >>>> components in the prebuilt baseline versus this build. > > >>>> > > >>>> > > >>>> I think it's safe to say that this might be an unsolvable problem. > > >>> > > >>> But what matters for the target is the mode, not the type. So IMO the > > >>> AI is misleading > > >>> here or the vectorizer asks for a non-natural aligned vector type > > >>> which it should not. > > >> > > >> I'm not so worried about the diagnostic part of the failure, but I > > >> presume > that this > > >> function also controls what code that might be generated. Thus, this > could potentially > > >> lead to wrong code or suboptimal code. > > >> Obvious, I would like the failure to go away since it's about 160 > > >> fail/xpass > for these > > >> 2 tests. > > >> > > >> What would you say is the next step to figure out what is going wrong > here? > > >> > > >> In r16-4314-g5e9eecc6686, I see that riscv might also have a similar > problem based on > > >> this change: > > >> > > >> { > > >> /* If we know that misalignment is a multiple of the element > > >> size, > we're > > >> good. */ > > >> - if (misalignment % TYPE_ALIGN_UNIT (type) == 0) > > >> + if (misalignment % (GET_MODE_UNIT_SIZE (mode)) == 0) > > >> return true; > > >> } > > >> > > >> Have you heard anyone in the riscv gang that noticed a problem here? > > > > > > Oh, but that is changing misalignment % alignment for misalignment % size > > > (which is more conservative). I'd expect TYPE_ALIGN and > GET_MODE_ALIGNMENT > > > to agree (also size!) and if you put an assert to that near the > > > single(?) caller of the > > > hook that should never fire. > > > > > > Does it for the testcases you point out regress? > > > > Sorry, but it can't even get that far... > > > > I used r16-4313-gb52a90e7a03fef as a baseline and added this change on > top: > > > > diff --git a/gcc/config/arm/arm.cc b/gcc/config/arm/arm.cc > > index 8b951f3d4a6..0ec5eca0497 100644 > > --- a/gcc/config/arm/arm.cc > > +++ b/gcc/config/arm/arm.cc > > @@ -30670,6 +30670,9 @@ arm_builtin_support_vector_misalignment > (machine_mode mode, > > { > > HOST_WIDE_INT align = TYPE_ALIGN_UNIT (type); > > > > + HOST_WIDE_INT mode_unit_size = GET_MODE_UNIT_SIZE (mode); > > + gcc_assert (align == mode_unit_size); > > That's not what I suggested. You were suggesting to re-instantiate > the 'type' argument > which was removed as redundant, so I suggested to assert type and mode size > and > alignment agree, not that alignment and size agree.
It's not the type that caused the test output to change, I think it's in how Is_packed is determed. The change in https://gcc.gnu.org/cgit/gcc/commit/?id=5e9eecc66867ba changes the value of is_packed which was only set when the misalignment was unknown, into always being set. In the Arm backend this means that if (is_packed) return align == 1; can now pass with a known, notzero misalignment. Which seems to be happening here and allowing the code to vectorize using a scalar integer mode. It seems like the Arm backend allows scalar modes like DImode to be legitimate vector modes. Using the hook from AArch64 doesn't work here because Arm is strict aligned by default but provides relaxations on loads and stores in certain cases. There's definitely a bug in that the implicitly relies on movmisalign_optab but never calls it, though that wouldn't stop this because DImode has an implementation. But still the contract should be upheld. I couldn't figure out why these test say that if we vectorize the warning shouldn't be shown. As far as I can tell it seems to be always there? Just not checked in other targets? But in similar question, is DImode and other integer modes an actual valid vector mode to use here? The generated code seems correct to me (and I can see this being useful for SLP, particularly of memory) But the target's arm_vector_mode_supported_p doesn't allow it. At least for consistency the following fix works here diff --git a/gcc/config/arm/arm.cc b/gcc/config/arm/arm.cc index 33b64995442..b7285cae864 100644 --- a/gcc/config/arm/arm.cc +++ b/gcc/config/arm/arm.cc @@ -30979,6 +30979,9 @@ arm_builtin_support_vector_misalignment (machine_mode mode, if (is_packed) return align == 1; + if (!VECTOR_MODE_P (mode)) + return false; + /* If the misalignment is unknown, we should be able to handle the access so long as it is not to a member of a packed data structure. */ if (misalignment == -1) But I don't know what the Arm maintainers think here. Thanks, Tamar > > > + > > if (is_gather_scatter) > > return true; > > > > > > When I try to build newlib with this (just building a full arm-none-eabi > toolchain), I get the following ICE: > > > > CC libc/stdlib/libc_a-arc4random.o > > during GIMPLE pass: slp > > /build/r16-4313- > gb52a90e7a03fef/src/newlib/newlib/libc/stdlib/arc4random.c: In function > '_rs_stir': > > /build/r16-4313- > gb52a90e7a03fef/src/newlib/newlib/libc/stdlib/arc4random.c:94:1: internal > compiler error: in arm_builtin_support_vector_misalignment, at > config/arm/arm.cc:30674 > > 94 | _rs_stir(void) > > | ^~~~~~~~ > > 0x1b8dcb9 internal_error(char const*, ...) > > /build/r16-4313-gb52a90e7a03fef/src/gcc/gcc/diagnostic-global- > context.cc:786 > > 0x6f5939 fancy_abort(char const*, int, char const*) > > /build/r16-4313- > gb52a90e7a03fef/src/gcc/gcc/diagnostics/context.cc:1803 > > 0x113ebfc arm_builtin_support_vector_misalignment > > /build/r16-4313- > gb52a90e7a03fef/src/gcc/gcc/config/arm/arm.cc:30674 > > 0x1ac6e9e vect_supportable_dr_alignment(vec_info*, dr_vec_info*, > tree_node*, int, bool) > > /build/r16-4313-gb52a90e7a03fef/src/gcc/gcc/tree-vect-data- > refs.cc:6643 > > 0x10187a6 get_load_store_type > > /build/r16-4313-gb52a90e7a03fef/src/gcc/gcc/tree-vect- > stmts.cc:2343 > > 0x102c1f3 vectorizable_load > > /build/r16-4313-gb52a90e7a03fef/src/gcc/gcc/tree-vect- > stmts.cc:9448 > > 0x103b23e vect_analyze_stmt(vec_info*, _slp_tree*, _slp_instance*, > vec<stmt_info_for_cost, va_heap, vl_ptr>*) > > /build/r16-4313-gb52a90e7a03fef/src/gcc/gcc/tree-vect- > stmts.cc:12665 > > 0x108d8e7 vect_slp_analyze_node_operations_1 > > /build/r16-4313-gb52a90e7a03fef/src/gcc/gcc/tree-vect-slp.cc:8065 > > 0x108d8e7 vect_slp_analyze_node_operations > > /build/r16-4313-gb52a90e7a03fef/src/gcc/gcc/tree-vect-slp.cc:8334 > > 0x108d857 vect_slp_analyze_node_operations > > /build/r16-4313-gb52a90e7a03fef/src/gcc/gcc/tree-vect-slp.cc:8311 > > 0x108f6eb vect_slp_analyze_operations(vec_info*) > > /build/r16-4313-gb52a90e7a03fef/src/gcc/gcc/tree-vect-slp.cc:8738 > > 0x1094e35 vect_slp_analyze_bb_1 > > /build/r16-4313-gb52a90e7a03fef/src/gcc/gcc/tree-vect-slp.cc:9785 > > 0x1094e35 vect_slp_region > > /build/r16-4313-gb52a90e7a03fef/src/gcc/gcc/tree-vect-slp.cc:9832 > > 0x1095e6d vect_slp_bbs > > /build/r16-4313-gb52a90e7a03fef/src/gcc/gcc/tree-vect-slp.cc:10055 > > 0x10964eb vect_slp_function(function*) > > /build/r16-4313-gb52a90e7a03fef/src/gcc/gcc/tree-vect-slp.cc:10177 > > 0x10a04fe execute > > /build/r16-4313-gb52a90e7a03fef/src/gcc/gcc/tree-vectorizer.cc:1568 > > Please submit a full bug report, with preprocessed source (by using > > -freport- > bug). > > Please include the complete backtrace with any bug report. > > See <https://gcc.gnu.org/bugs/> for instructions. > > Makefile:22030: recipe for target 'libc/stdlib/libc_a-arc4random.o' failed > > > > > > Does this give you any ideas? > > > > Would it help to provide the scalar_mode along with the machine_mode to > arm_builtin_support_vector_misalignment in any way? > > > > Kind regards, > > Torbjörn > >
