Ping: [PATCH][Arm] Enable MVE SIMD modes for vectorization

[Dennis Zhang]

Hi Christophe,

On 12/10/2020 12:40, Christophe Lyon wrote:
> Hi,
> 
> 
> On Thu, 8 Oct 2020 at 16:22, Christophe Lyon <christophe.lyon@linaro.org> wrote:
>>
>> On Thu, 8 Oct 2020 at 16:08, Dennis Zhang <dennis.zhang@arm.com> wrote:
>>>
>>> Hi Christophe,
>>>
>>> On 08/10/2020 14:14, Christophe Lyon wrote:
>>>> Hi,
>>>>
>>>>
>>>> On Tue, 6 Oct 2020 at 15:37, Dennis Zhang via Gcc-patches
>>>> <gcc-patches@gcc.gnu.org> wrote:
>>>>>
>>>>> On 9/16/20 4:00 PM, Dennis Zhang wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> This patch enables SIMD modes for MVE auto-vectorization.
>>>>>> In this patch, the integer and float MVE SIMD modes are returned by
>>>>>> arm_preferred_simd_mode (TARGET_VECTORIZE_PREFERRED_SIMD_MODE hook) when
>>>>>> MVE or MVE_FLOAT is enabled.
>>>>>> Then the expanders for auto-vectorization can be used for generating MVE
>>>>>> SIMD code.
>>>>>>
>>>>>> This patch also fixes bugs in MVE vreiterpretq_*.c tests which are
>>>>>> revealed by the enabled MVE SIMD modes.
>>>>>> The tests are for checking the MVE reinterpret intrinsics.
>>>>>> There are two functions in each of the tests. The two functions contain
>>>>>> the pattern of identical code so that they are folded in icf pass.
>>>>>> Because of icf, the instruction count only checks one function which is 8.
>>>>>> However when the SIMD modes are enabled, the estimation of the code size
>>>>>> becomes smaller so that inlining is applied after icf, then the
>>>>>> instruction count becomes 16 which causes failure of the tests.
>>>>>> Because the icf is not the expected pattern to be tested but causes
>>>>>> above issues, -fno-ipa-icf is applied to the tests to avoid unstable
>>>>>> instruction count.
>>>>>>
>>>>>> This patch is separated from
>>>>>> https://gcc.gnu.org/pipermail/gcc-patches/2020-August/552104.html
>>>>>> because this part is not strongly connected to the aim of that one so
>>>>>> that causing confusion.
>>>>>>
>>>>>> Regtested and bootstraped.
>>>>>>
>>>>>> Is it OK for trunk please?
>>>>>>
>>>>>> Thanks
>>>>>> Dennis
>>>>>>
>>>>>> gcc/ChangeLog:
>>>>>>
>>>>>> 2020-09-15  Dennis Zhang  <dennis.zhang@arm.com>
>>>>>>
>>>>>>         * config/arm/arm.c (arm_preferred_simd_mode): Enable MVE SIMD modes.
>>>>>>
>>>>
>>>> Since toolchain builds work again after Jakub's divmod fix, I'm now
>>>> facing another build error likely caused by this patch:
>>>> In file included from
>>>> /tmp/2601185_2.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/coretypes.h:449:0,
>>>>                    from
>>>> /tmp/2601185_2.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/config/arm/arm.c:28:
>>>> /tmp/2601185_2.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/config/arm/arm.c:
>>>> In function 'machine_mode arm_preferred_simd_mode(scalar_mode)':
>>>> ./insn-modes.h:196:71: error: temporary of non-literal type
>>>> 'scalar_int_mode' in a constant expression
>>>>    #define QImode (scalar_int_mode ((scalar_int_mode::from_int) E_QImode))
>>>>                                                                          ^
>>>> /tmp/2601185_2.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/config/arm/arm.c:28970:12:
>>>> note: in expansion of macro 'QImode'
>>>>          case QImode:
>>>>
>>>> and similarly for the other cases.
>>>>
>>>> Does the build work for you?
>>>>
>>>> Thanks,
>>>>
>>>> Christophe
>>>>
>>>
>>> Thanks for the report. Sorry to see the error.
>>> I tested it for arm-none-eabi and arm-none-linux-gnueabi targets. I
>>> didn't get this error.
>>> Could you please help to show the configuration you use for your build?
>>> I will test and fix at once.
>>>
>>
>> It fails on all of them for me. Does it work for you with current
>> master? (r11-3720-gf18eeb6b958acd5e1590ca4a73231486b749be9b)
>>
> 
> So... I guess you are using a host with GCC more recent than 4.8.5? :-)
> When I build manually on ubuntu-16.04 with gcc-5.4, the build succeeds,
> and after manually building with the same environment in the compute
> farm I use for validation (RHEL 7, gcc-4.8.5), I managed to reproduce the
> build failure.
> It's a matter of replacing
> case QImode:
> with
> case E_QImode:
> 
> Is the attached patch OK? Or do we instead want to revisit the minimum
> gcc version required to build gcc?
> 
> Thanks,
> 
> Christophe
> 

I've tested your patch and it works with my other patches depending on 
this one. So I agree this patch is OK. Thanks for the fix.

Bests
Dennis