Richard Sandiford <[email protected]> writes:
> Andrew Stubbs <[email protected]> writes:
>> [...]
>> diff --git a/gcc/simplify-rtx.cc b/gcc/simplify-rtx.cc
>> index 38c8abfe3e7..67f3307d2b0 100644
>> --- a/gcc/simplify-rtx.cc
>> +++ b/gcc/simplify-rtx.cc
>> @@ -8777,9 +8777,43 @@ simplify_context::simplify_subreg (machine_mode
>> outermode, rtx op,
>> && (! MEM_VOLATILE_P (op)
>> || ! have_insn_for (SET, innermode))
>> && !(STRICT_ALIGNMENT && MEM_ALIGN (op) < GET_MODE_ALIGNMENT
>> (outermode))
>> - && known_le (outersize, innersize))
>> + && known_le (outersize, innersize)
>> + /* You can't get the Nth element of a vector of addresses (i.e.
>> + gather/scatter) by adding an offset to the base. */
>> + && !VECTOR_MODE_P (GET_MODE (XEXP (op, 0))))
>> return adjust_address_nv (op, outermode, byte);
>>
>> + /* Simplify (subreg:<m> (mem:<vec_m> (reg:<vec_p> addr)) x)
>> + to (mem:<m> (subreg:<p> (reg:<vec_p>)) y) */
>> + if (MEM_P (op)
>> + && VECTOR_MODE_P (innermode)
>> + && VECTOR_MODE_P (GET_MODE (XEXP (op, 0)))
>> + && GET_MODE_INNER (innermode) == outermode)
>> + {
>> + unsigned int index;
>> + if (constant_multiple_p (byte, GET_MODE_SIZE (outermode), &index))
>> + {
>
> I think this should instead be:
>
> poly_uint64 index;
> if (multiple_p (byte, GET_MODE_SIZE (outermode), &index))
> ...
>
> with:
>
>> + rtx addr = XEXP (op, 0);
>> + machine_mode addrmode = GET_MODE (addr);
>> + machine_mode addr_elt_mode = GET_MODE_INNER (addrmode);
>
> ...this using "auto" or "scalar_mode" instead of "machine_mode". Then:
>
>> + poly_uint64 addr_byte = index * GET_MODE_SIZE (addr_elt_mode);
>
> ...this will still be a poly_uint64, but with the non-constant portion
> coming from the index rather than the mode.
>
> LGTM with that change, thanks, and sorry for the slow review.
Sorry, a couple of other things:
>> +
>> + rtx new_addr
>> + = (REG_P (addr)
>> + ? simplify_gen_subreg (addr_elt_mode, addr, addrmode,
>> + addr_byte)
>> + : simplify_subreg (addr_elt_mode, addr, addrmode, addr_byte));
I think we should use simplify_gen_subreg unconditionally unless there's
a specific reason not to. This is a simplification regardless of what
the address is.
>> + if (new_addr)
>> + {
>> + rtx new_mem = gen_rtx_MEM (outermode, new_addr);
>> + set_mem_addr_space (new_mem, MEM_ADDR_SPACE (op));
>> + MEM_VOLATILE_P (new_mem) = MEM_VOLATILE_P (op);
>> + MEM_NOTRAP_P (new_mem) = MEM_NOTRAP_P (op);
>> + return new_mem;
I think this should use change_address instead. AIUI, that is suitable
for any change that preserves the alias set.
Still LGTM with those changes.
Thanks,
Richard
>> + }
>> + }
>> + }
>> +
>> /* Handle complex or vector values represented as CONCAT or VEC_CONCAT
>> of two parts. */
>> if (GET_CODE (op) == CONCAT
>> diff --git a/gcc/target.def b/gcc/target.def
>> index 11334f9d443..4c4f91827b2 100644
>> --- a/gcc/target.def
>> +++ b/gcc/target.def
>> @@ -3451,7 +3451,10 @@ HOOK_VECTOR (TARGET_ADDR_SPACE_HOOKS, addr_space)
>> DEFHOOK
>> (pointer_mode,
>> "Define this to return the machine mode to use for pointers to\n\
>> -@var{address_space} if the target supports named address spaces.\n\
>> +@var{address_space} if the target supports named address spaces.\n\n\
>> +Targets that support vectors of pointers should return only the scalar
>> base\n\
>> +mode here, but permit the vector modes in\n\
>> +@code{TARGET_ADDR_SPACE_VALID_POINTER_MODE}.\n\n\
>> The default version of this hook returns @code{ptr_mode}.",
>> scalar_int_mode, (addr_space_t address_space),
>> default_addr_space_pointer_mode)
>> @@ -3460,7 +3463,11 @@ The default version of this hook returns
>> @code{ptr_mode}.",
>> DEFHOOK
>> (address_mode,
>> "Define this to return the machine mode to use for addresses in\n\
>> -@var{address_space} if the target supports named address spaces.\n\
>> +@var{address_space} if the target supports named address spaces.\n\n\
>> +Targets that support vectors of addresses should return only the scalar
>> base\n\
>> +mode here, but permit the vector modes in\n\
>> +@code{TARGET_ADDR_SPACE_LEGITIMATE_ADDRESS_P} and\n\
>> +@code{TARGET_ADDR_SPACE_LEGITIMIZE_ADDRESS}.\n\n\
>> The default version of this hook returns @code{Pmode}.",
>> scalar_int_mode, (addr_space_t address_space),
>> default_addr_space_address_mode)
>> @@ -3470,13 +3477,16 @@ The default version of this hook returns
>> @code{Pmode}.",
>> DEFHOOK
>> (valid_pointer_mode,
>> "Define this to return nonzero if the port can handle pointers\n\
>> -with machine mode @var{mode} to address space @var{as}. This target\n\
>> -hook is the same as the @code{TARGET_VALID_POINTER_MODE} target hook,\n\
>> -except that it includes explicit named address space support. The
>> default\n\
>> -version of this hook returns true for the modes returned by either the\n\
>> -@code{TARGET_ADDR_SPACE_POINTER_MODE} or
>> @code{TARGET_ADDR_SPACE_ADDRESS_MODE}\n\
>> -target hooks for the given address space.",
>> - bool, (scalar_int_mode mode, addr_space_t as),
>> +with machine mode @var{mode} to address space @var{as}. This hook can\n\
>> +also be used to allow pointers to use vector modes (e.g. to permit\n\
>> +@samp{(mem:V4SI (reg:V4DI))}). This target hook is the same as the\n\
>> +@code{TARGET_VALID_POINTER_MODE} target hook, except that it includes\n\
>> +explicit named address space support, and does not mandate\n\
>> +@code{scalar_int_mode}. The default version of this hook returns true
>> for\n\
>> +the modes returned by either the @code{TARGET_ADDR_SPACE_POINTER_MODE}\n\
>> +or @code{TARGET_ADDR_SPACE_ADDRESS_MODE} target hooks for the given
>> address\n\
>> +space.",
>> + bool, (machine_mode mode, addr_space_t as),
>> default_addr_space_valid_pointer_mode)
>>
>> /* True if an address is a valid memory address to a given named address
>> diff --git a/gcc/targhooks.cc b/gcc/targhooks.cc
>> index 7123827cfd7..01defd7dbb3 100644
>> --- a/gcc/targhooks.cc
>> +++ b/gcc/targhooks.cc
>> @@ -1728,10 +1728,13 @@ default_addr_space_address_mode (addr_space_t
>> addrspace ATTRIBUTE_UNUSED)
>> To match the above, the same modes apply to all address spaces. */
>>
>> bool
>> -default_addr_space_valid_pointer_mode (scalar_int_mode mode,
>> +default_addr_space_valid_pointer_mode (machine_mode mode,
>> addr_space_t as ATTRIBUTE_UNUSED)
>> {
>> - return targetm.valid_pointer_mode (mode);
>> + if (! SCALAR_INT_MODE_P (mode))
>> + return false;
>> +
>> + return targetm.valid_pointer_mode (as_a <scalar_int_mode> (mode));
>> }
>>
>> /* Some places still assume that all pointer or address modes are the
>> diff --git a/gcc/targhooks.h b/gcc/targhooks.h
>> index ffeaeb2a772..2a429e7f4a4 100644
>> --- a/gcc/targhooks.h
>> +++ b/gcc/targhooks.h
>> @@ -206,8 +206,7 @@ extern bool default_valid_pointer_mode (scalar_int_mode);
>> extern bool default_ref_may_alias_errno (class ao_ref *);
>> extern scalar_int_mode default_addr_space_pointer_mode (addr_space_t);
>> extern scalar_int_mode default_addr_space_address_mode (addr_space_t);
>> -extern bool default_addr_space_valid_pointer_mode (scalar_int_mode,
>> - addr_space_t);
>> +extern bool default_addr_space_valid_pointer_mode (machine_mode,
>> addr_space_t);
>> extern bool default_addr_space_legitimate_address_p (machine_mode, rtx,
>> bool,
>> addr_space_t, code_helper);
>> extern rtx default_addr_space_legitimize_address (rtx, rtx, machine_mode,
