This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: expansion of vector shifts...
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: David Miller <davem at davemloft dot net>
- Cc: rdsandiford at googlemail dot com, gcc at gcc dot gnu dot org, jakub at redhat dot com, ebotcazou at adacore dot com, gcc-patches at gcc dot gnu dot org
- Date: Wed, 13 Feb 2013 12:15:13 +0100
- Subject: Re: expansion of vector shifts...
- References: <20121027.052901.128093230202064296.davem@davemloft.net> <87fw4xk2jm.fsf@talisman.home> <20121116.003305.2180726158176000063.davem@redhat.com> <20130212.173146.281298392845155966.davem@davemloft.net>
On Tue, Feb 12, 2013 at 11:31 PM, David Miller <davem@davemloft.net> wrote:
> From: David Miller <davem@redhat.com>
> Date: Fri, 16 Nov 2012 00:33:05 -0500 (EST)
>
>> From: Richard Sandiford <rdsandiford@googlemail.com>
>> Date: Mon, 29 Oct 2012 10:14:53 +0000
>>
>>> ...given that the code is like you say written:
>>>
>>> if (SHIFT_COUNT_TRUNCATED)
>>> {
>>> if (CONST_INT_P (op1)
>>> ...
>>> else if (GET_CODE (op1) == SUBREG
>>> && subreg_lowpart_p (op1)
>>> && INTEGRAL_MODE_P (GET_MODE (SUBREG_REG (op1))))
>>> op1 = SUBREG_REG (op1);
>>> }
>>>
>>> INTEGRAL_MODE_P (GET_MODE (op1)) might be better than an explicit
>>> VECTOR_MODE_P check. The code really doesn't make sense for anything
>>> other than integers.
>>>
>>> (It amounts to the same thing in practice, of course...)
>>
>> Agreed, I've just committed the following. Thanks!
>>
>> ====================
>> Fix gcc.c-torture/compile/pr53410-2.c on sparc.
>>
>> * expmed.c (expand_shift_1): Don't strip non-integral SUBREGs.
>
> This is broken on sparc again, although I'm confused about how this
> has happened.
>
> The suggestion was to use INTEGRAL_MODE_P as the test, so what's there
> in expand_shift_1() is:
>
> else if (GET_CODE (op1) == SUBREG
> && subreg_lowpart_p (op1)
> && INTEGRAL_MODE_P (GET_MODE (SUBREG_REG (op1)))
> && INTEGRAL_MODE_P (GET_MODE (op1)))
> op1 = SUBREG_REG (op1);
>
> but INTEGRAL_MODE_P accepts vectors. This is really confusing because
> I was absolutely sure I re-ran the test case with the fix I committed
> and it didn't crash any more.
>
> Maybe what we really mean to do here is check both op1 and SUBREG_REG
> (op1) against SCALAR_INT_MODE_P instead of INTEGRAL_MODE_P?
Yes.
> Something like this:
>
> gcc/
>
> 2013-02-12 David S. Miller <davem@davemloft.net>
>
> * expmed.c (expand_shift_1): Only strip scalar integer subregs.
>
> diff --git a/gcc/expmed.c b/gcc/expmed.c
> index 4a6ddb0..954a360 100644
> --- a/gcc/expmed.c
> +++ b/gcc/expmed.c
> @@ -2116,8 +2116,8 @@ expand_shift_1 (enum tree_code code, enum machine_mode mode, rtx shifted,
> % GET_MODE_BITSIZE (mode));
> else if (GET_CODE (op1) == SUBREG
> && subreg_lowpart_p (op1)
> - && INTEGRAL_MODE_P (GET_MODE (SUBREG_REG (op1)))
> - && INTEGRAL_MODE_P (GET_MODE (op1)))
> + && SCALAR_INT_MODE_P (GET_MODE (SUBREG_REG (op1)))
> + && SCALAR_INT_MODE_P (GET_MODE (op1)))
> op1 = SUBREG_REG (op1);
> }
>
>
>