This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, alpha]: Some further HWI == 64 improvements
- From: Uros Bizjak <ubizjak at gmail dot com>
- To: Richard Henderson <rth at redhat dot com>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 13 May 2015 19:53:04 +0200
- Subject: Re: [PATCH, alpha]: Some further HWI == 64 improvements
- Authentication-results: sourceware.org; auth=none
- References: <CAFULd4Z6x7GbBc6-yDWEccdWt+A-1iAFFK7GVvS3gJhVLnUobA at mail dot gmail dot com> <55537FE9 dot 2060104 at redhat dot com>
On Wed, May 13, 2015 at 6:46 PM, Richard Henderson <rth@redhat.com> wrote:
> On 05/13/2015 08:04 AM, Uros Bizjak wrote:
>> Hello!
>>
>> 2015-05-13 Uros Bizjak <ubizjak@gmail.com>
>>
>> * config/alpha/alpha.c (alpha_emit_set_long_const): Remove c1 argument.
>> (alpha_extract_integer): Redeclare as static HOST_WIDE_INT.
>> Remove *p0 and *p1 arguments. Rewrite function.
>> (alpha_legitimate_constant_p): Update call to alpha_extract_integer.
>> (alpha_split_const_mov): Update calls to alpha_extract_integer and
>> alpha_emit_set_long_const.
>> (alpha_expand_epilogue): Update calls to alpha_emit_set_long_const.
>> (alpha_output_mi_thunk_osf): Ditto.
>> * config/alpha/alpha.md (movti): Do not check operands[1]
>> for CONST_DOUBLE.
>>
>> Tested on alpha-linux-gnu and committed to mainline SVN.
>> + switch (GET_CODE (x))
>> + {
>> + case CONST_INT:
>> + return INTVAL (x);
>> + case CONST_WIDE_INT:
>> + return CONST_WIDE_INT_ELT (x, 0);
>> + case CONST_DOUBLE:
>> + return CONST_DOUBLE_LOW (x);
>> + default:
>> + gcc_unreachable ();
>> + }
>
> Surely we don't actually need to test for CONST_DOUBLE anymore?
Indeed. I will look into it in a follow-up patch. There are probably
some more checks for CONST_DOUBLE, I'll look into this later.
>> /* Implement TARGET_LEGITIMATE_CONSTANT_P. This is all constants for which
>> @@ -2138,7 +2135,7 @@ static rtx
>> bool
>> alpha_legitimate_constant_p (machine_mode mode, rtx x)
>> {
>> - HOST_WIDE_INT i0, i1;
>> + HOST_WIDE_INT i0;
>>
>> switch (GET_CODE (x))
>> {
>> @@ -2185,7 +2182,7 @@ alpha_legitimate_constant_p (machine_mode mode, rt
>> do_integer:
>> if (TARGET_BUILD_CONSTANTS)
>> return true;
>> - alpha_extract_integer (x, &i0, &i1);
>> + i0 = alpha_extract_integer (x);
>> return alpha_emit_set_const_1 (x, mode, i0, 3, true) != NULL;
>
> Doesn't this now allow CONST_WIDE_INT that are in fact wider than 64 bits?
> Which I can't imagine being legitimate...
I did check this part with:
--cut here--
__int128 a;
void test (void)
{
a = ((__int128) 7 << 100) + ((__int128) 1 << 30);
}
--cut here--
and generated code was identical to unpatched compiler:
ldah $1,16384($31)
lda $2,a
stq $1,0($2)
lda $1,7($31)
sll $1,36,$1
stq $1,8($2)
ret $31,($26),1
However, this works only for TARGET_BUILD_CONSTANTS. This is a
preexisting bug, it looks that we want:
--cut here--
Index: alpha.c
===================================================================
--- alpha.c (revision 223166)
+++ alpha.c (working copy)
@@ -2152,7 +2152,6 @@ alpha_legitimate_constant_p (machine_mode mode, rt
if (GET_CODE (x) != SYMBOL_REF)
return true;
-
/* FALLTHRU */
case SYMBOL_REF:
@@ -2162,7 +2161,7 @@ alpha_legitimate_constant_p (machine_mode mode, rt
case CONST_WIDE_INT:
if (x == CONST0_RTX (mode))
return true;
- goto do_integer;
+ return TARGET_BUILD_CONSTANTS != 0;
case CONST_DOUBLE:
if (x == CONST0_RTX (mode))
@@ -2176,10 +2175,9 @@ alpha_legitimate_constant_p (machine_mode mode, rt
return false;
if (GET_MODE_SIZE (mode) != 8)
return false;
- goto do_integer;
+ /* FALLTHRU */
case CONST_INT:
- do_integer:
if (TARGET_BUILD_CONSTANTS)
return true;
i0 = alpha_extract_integer (x);
--cut here--
The only part that actually processes CONST_WIDE_INT is
alpha_legitimate_constant and movti expander. The latter immediately
splits any TImode CONST_WIDE_INTs.
Uros.