On 11/25/20 9:47 PM, Hongtao Liu wrote:
> On Wed, Nov 25, 2020 at 7:37 PM Jakub Jelinek <ja...@redhat.com> wrote:
>> On Wed, Nov 25, 2020 at 07:32:44PM +0800, Hongtao Liu wrote:
>>> Update patch:
>>> 1. ix86_expand_special_args_builtin is used for expanding mask load
>>> intrinsics, this function will always convert the constant mask
>>> operands into reg. So for the situation of all-ones mask, keep this
>>> constant, and also change the mask operand predicate(of corresponding
>>> expander) to register_or_constm1_operand.
>>> 2. Delete last_arg_constant which is not used in
>>> ix86_expand_special_args_builtin(maybe should be in a separate patch?)
>> Yes, please make it a separate patch, it should go in first and
>> should just drop last_arg_constant from that function plus the
>> reindentation.
>>
>> Then post the PR97642 incremental to that.
>>
> Updated.
>
>> Thanks.
>>
>> Jakub
>>
>
> 0002-Fix-incorrect-replacement-of-vmovdqu32-with-vpblendd.patch
>
> From b1256b6ef8f877244f4955b9205d53797424fc27 Mon Sep 17 00:00:00 2001
> From: liuhongt <hongtao....@intel.com>
> Date: Tue, 3 Nov 2020 17:26:43 +0800
> Subject: [PATCH 2/2] Fix incorrect replacement of vmovdqu32 with vpblendd
> which can cause fault.
>
> gcc/ChangeLog:
>
> PR target/97642
> * config/i386/i386-expand.c
> (ix86_expand_special_args_builtin): Don't move all-ones mask
> operands into register.
> * config/i386/sse.md (UNSPEC_MASKLOAD): New unspec.
> (*<avx512>_load<mode>_mask): New define_insns for masked load
> instructions.
> (<avx512>_load<mode>_mask): Changed to define_expands which
> specifically handle memory or all-ones mask operands.
> (<avx512>_blendm<mode>): Changed to define_insns which are same
> as original <avx512>_load<mode>_mask with adjustment of
> operands order.
> (*<avx512>_load<mode>): New define_insn_and_split which is
> used to optimize for masked load with all one mask.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/i386/avx512bw-vmovdqu16-1.c: Adjust testcase to
> make sure only masked load instruction is generated.
> * gcc.target/i386/avx512bw-vmovdqu8-1.c: Ditto.
> * gcc.target/i386/avx512f-vmovapd-1.c: Ditto.
> * gcc.target/i386/avx512f-vmovaps-1.c: Ditto.
> * gcc.target/i386/avx512f-vmovdqa32-1.c: Ditto.
> * gcc.target/i386/avx512f-vmovdqa64-1.c: Ditto.
> * gcc.target/i386/avx512vl-vmovapd-1.c: Ditto.
> * gcc.target/i386/avx512vl-vmovaps-1.c: Ditto.
> * gcc.target/i386/avx512vl-vmovdqa32-1.c: Ditto.
> * gcc.target/i386/avx512vl-vmovdqa64-1.c: Ditto.
> * gcc.target/i386/pr97642-1.c: New test.
> * gcc.target/i386/pr97642-2.c: New test.
> ---
> [ ... ]
> diff --git a/gcc/testsuite/gcc.target/i386/pr97642-2.c
> b/gcc/testsuite/gcc.target/i386/pr97642-2.c
> new file mode 100644
> index 00000000000..eb06a2739b4
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr97642-2.c
> @@ -0,0 +1,77 @@
> +/* PR target/97642 */
> +/* { dg-do run } */
> +/* { dg-options "-O2 -mavx512dq -mavx512vl -mavx512bw" } */
> +/* { dg-require-effective-target avx512vl } */
> +/* { dg-require-effective-target avx512dq } */
> +/* { dg-require-effective-target avx512bw } */
Given the uses of mprotect in this test, don't we need the test to be
limited to systems where that's supported. Even just limiting to linux
targets is probably sufficient.
With that change, I think this is OK for the trunk.
jeff
