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/heads/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. > + > 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 >
