On Mon, Oct 19, 2026 at 1:24 PM Tamar Christina <[email protected]> wrote: > > > -----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.
is_packed should be set only when the original scalar access is not naturally aligned (alignment == size). It's intended to allow easy distinction between element aligned vs. arbitrary misaligned accesses. > In the Arm backend this means that > > if (is_packed) > return align == 1; > > can now pass with a known, notzero misalignment. this is an odd check anyway. > 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. All targets allow 'integer mode' vectorization aka "generic vectorization". The integer mode then merely acts as container and we emulate operations. > 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. Because it's not a vector mode and we don't even invoke that hook. We simply assume a DImode load is OK when !SLOW_UNALIGNED_ACCESS or a movmisalign handler is there. IIRC we don't (or we should) not invoke the vector_misalignment hook with such mode. Do we? > 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. So if we do, and arm is strict-alignment wrt scalar modes then the above looks OK unless there's fast misaligned access support. But I think that should be checked in vect_supportable_dr_alignment directly, querying SLOW_UNALIGNED_ACCESS and for a movmisalign optab. We'll emit proper misaligned scalar MEM_REFs and those should then be expanded w/o going the bitfield route. Richard. > > 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 > > >
