Bug 107714 - MVE: Invalid addressing mode generated for VLD2
Summary: MVE: Invalid addressing mode generated for VLD2
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 13.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2022-11-16 12:56 UTC by Kevin Bracey
Modified: 2023-01-16 13:22 UTC (History)
1 user (show)

See Also:
Host:
Target: arm
Build:
Known to work:
Known to fail: 11.3.1, 12.2.1
Last reconfirmed: 2022-12-09 00:00:00


Attachments
Stripped-down reproducer source (337 bytes, text/x-csrc)
2022-11-16 12:56 UTC, Kevin Bracey
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Kevin Bracey 2022-11-16 12:56:04 UTC
Created attachment 53909 [details]
Stripped-down reproducer source

While I was working on some Helium intrinsics, GCC produced some invalid code, meaning my optimisation can only be enabled in our armclang builds. Problem seems to be still present on GCC trunk.

Posted at https://godbolt.org/z/h3EhMvxao

Compilation options -O2 -mcpu=cortex-m55 -mfloat-abi=hard

Error: instruction does not accept this addressing mode -- `vld21.8 {q4,q5},[r3],r2'

Compiler Explorer output for trunk shows the same invalid addressing mode.

(It also shows non-existent registers q8 and up in use - I don't know why. Not a problem in my local GCC, obtained from Arm's embedded distribution).
Comment 1 Kevin Bracey 2022-11-16 15:52:24 UTC
Looking at that assembly output from Compiler Explorer, I'm also at a loss as to what happened to the "6" for the VMUL. Maybe something else to look at?
Comment 2 Kevin Bracey 2022-11-16 16:00:41 UTC
Ah, the vmulq is falling foul of some sort of generic selection problem. Substituting with vmulq_n_u8() gets me the actual 6.

Something in the same area as my bug 107515, perhaps - I've been making liberal use of the generic functions.
Comment 3 Stam Markianos-Wright 2022-11-17 16:43:20 UTC
Thanks for finding this! Confirmed the `vld2` bug on latest trunk -- my guess would be either a GCC backend or a gas bug (I'm not familiar with these instructions).

For the `vmulq` issue I'll reply under the other thread, but I believe that very soon we'll be able to put that down as "already fixed", so we can keep this thread for `vld2` only.
Comment 4 Kevin Bracey 2022-11-21 12:12:56 UTC
The assembler's rejection of the vld2 is valid - the only permitted post-indexed form is to use "!" for increment by 32 (the amount read).

Experimenting by changing "inStep" you can see the compiler backend knows that 32 is the only valid constant offset - it generates the "!" form for that correctly - but it apparently hasn't been told not to use register offsets.
Comment 5 Kevin Bracey 2022-11-21 12:49:55 UTC
I had a look at the GCC source. The vld2/vst2/vld4/vst4 instructions in mve.md have reused the "Um" constraint used for vld<N>/vst<N> in Neon, which permits both "!" and register offset.

This needs to be tightened up - can't see an existing equivalent constraint. Perhaps "Um" can be given variant MVE/Neon behaviour, like "Uj".
Comment 6 GCC Commits 2022-12-30 11:25:42 UTC
The master branch has been updated by Stam Markianos-Wright <stammark@gcc.gnu.org>:

https://gcc.gnu.org/g:4269a6567eb991e6838f40bda5be9e3a7972530c

commit r13-4935-g4269a6567eb991e6838f40bda5be9e3a7972530c
Author: Stam Markianos-Wright <stam.markianos-wright@arm.com>
Date:   Fri Dec 30 11:25:22 2022 +0000

    Fix memory constraint on MVE v[ld/st][2/4] instructions [PR107714]
    
    In the M-Class Arm-ARM:
    
    https://developer.arm.com/documentation/ddi0553/bu/?lang=en
    
    these MVE instructions only have '!' writeback variant and at:
    
    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107714
    
    we found that the Um constraint would also allow through a
    register offset writeback, resulting in an assembler error.
    
    Here I have added a new constraint and predicate for these
    instructions, which (uniquely, AFAICT), only support a `!` writeback
    increment by the data size (inside the compiler this is a POST_INC).
    
    No regressions in arm-none-eabi with MVE and MVE.FP.
    
    gcc/ChangeLog:
            PR target/107714
            * config/arm/arm-protos.h (mve_struct_mem_operand): New protoype.
            * config/arm/arm.cc (mve_struct_mem_operand): New function.
            * config/arm/constraints.md (Ug): New constraint.
            * config/arm/mve.md (mve_vst4q<mode>): Change constraint.
            (mve_vst2q<mode>): Likewise.
            (mve_vld4q<mode>): Likewise.
            (mve_vld2q<mode>): Likewise.
            * config/arm/predicates.md (mve_struct_operand): New predicate.
    
    gcc/testsuite/ChangeLog:
            PR target/107714
            * gcc.target/arm/mve/intrinsics/vldst24q_reg_offset.c: New test.
Comment 7 GCC Commits 2023-01-10 13:38:20 UTC
The releases/gcc-11 branch has been updated by Stam Markianos-Wright <stammark@gcc.gnu.org>:

https://gcc.gnu.org/g:08842ad274f5e2630994f7c6e70b2d31768107ea

commit r11-10461-g08842ad274f5e2630994f7c6e70b2d31768107ea
Author: Stam Markianos-Wright <stam.markianos-wright@arm.com>
Date:   Fri Dec 30 11:25:22 2022 +0000

    Fix memory constraint on MVE v[ld/st][2/4] instructions [PR107714]
    
    In the M-Class Arm-ARM:
    
    https://developer.arm.com/documentation/ddi0553/bu/?lang=en
    
    these MVE instructions only have '!' writeback variant and at:
    
    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107714
    
    we found that the Um constraint would also allow through a
    register offset writeback, resulting in an assembler error.
    
    Here I have added a new constraint and predicate for these
    instructions, which (uniquely, AFAICT), only support a `!` writeback
    increment by the data size (inside the compiler this is a POST_INC).
    
    No regressions in arm-none-eabi with MVE and MVE.FP.
    
    gcc/ChangeLog:
            PR target/107714
            * config/arm/arm-protos.h (mve_struct_mem_operand): New protoype.
            * config/arm/arm.c (mve_struct_mem_operand): New function.
            * config/arm/constraints.md (Ug): New constraint.
            * config/arm/mve.md (mve_vst4q<mode>): Change constraint.
            (mve_vst2q<mode>): Likewise.
            (mve_vld4q<mode>): Likewise.
            (mve_vld2q<mode>): Likewise.
            * config/arm/predicates.md (mve_struct_operand): New predicate.
    
    gcc/testsuite/ChangeLog:
            PR target/107714
            * gcc.target/arm/mve/intrinsics/vldst24q_reg_offset.c: New test.
    
    (cherry picked from commit 4269a6567eb991e6838f40bda5be9e3a7972530c)
Comment 8 GCC Commits 2023-01-10 13:41:01 UTC
The releases/gcc-12 branch has been updated by Stam Markianos-Wright <stammark@gcc.gnu.org>:

https://gcc.gnu.org/g:25edc76f2afba0b4eaf22174d42de042a6969dbe

commit r12-9038-g25edc76f2afba0b4eaf22174d42de042a6969dbe
Author: Stam Markianos-Wright <stam.markianos-wright@arm.com>
Date:   Fri Dec 30 11:25:22 2022 +0000

    Fix memory constraint on MVE v[ld/st][2/4] instructions [PR107714]
    
    In the M-Class Arm-ARM:
    
    https://developer.arm.com/documentation/ddi0553/bu/?lang=en
    
    these MVE instructions only have '!' writeback variant and at:
    
    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107714
    
    we found that the Um constraint would also allow through a
    register offset writeback, resulting in an assembler error.
    
    Here I have added a new constraint and predicate for these
    instructions, which (uniquely, AFAICT), only support a `!` writeback
    increment by the data size (inside the compiler this is a POST_INC).
    
    No regressions in arm-none-eabi with MVE and MVE.FP.
    
    gcc/ChangeLog:
            PR target/107714
            * config/arm/arm-protos.h (mve_struct_mem_operand): New protoype.
            * config/arm/arm.cc (mve_struct_mem_operand): New function.
            * config/arm/constraints.md (Ug): New constraint.
            * config/arm/mve.md (mve_vst4q<mode>): Change constraint.
            (mve_vst2q<mode>): Likewise.
            (mve_vld4q<mode>): Likewise.
            (mve_vld2q<mode>): Likewise.
            * config/arm/predicates.md (mve_struct_operand): New predicate.
    
    gcc/testsuite/ChangeLog:
            PR target/107714
            * gcc.target/arm/mve/intrinsics/vldst24q_reg_offset.c: New test.
    
    (cherry picked from commit 4269a6567eb991e6838f40bda5be9e3a7972530c)
Comment 9 Stam Markianos-Wright 2023-01-16 13:22:21 UTC
This should now be resolved on all active branches (11, 12, 13)