On 01/09/2018 08:23 AM, Richard Sandiford wrote:
> Richard Biener <richard.guent...@gmail.com> writes:
>> On Mon, Nov 20, 2017 at 12:31 PM, Bin.Cheng <amker.ch...@gmail.com> wrote:
>>> On Fri, Nov 17, 2017 at 3:03 PM, Richard Sandiford
>>> <richard.sandif...@linaro.org> wrote:
>>>> ivopts previously treated pointer arguments to internal functions
>>>> like IFN_MASK_LOAD and IFN_MASK_STORE as normal gimple values.
>>>> This patch makes it treat them as addresses instead. This makes
>>>> a significant difference to the code quality for SVE loops,
>>>> since we can then use loads and stores with scaled indices.
>>> Thanks for working on this. This can be extended to other internal
>>> functions which eventually
>>> are expanded into memory references. I believe (at least) both x86
>>> and AArch64 has such
>>> requirement.
>>
>> In addition to Bins comments I only have a single one (the rest of the
>> middle-end
>> changes look OK). The alias type of MEM_REFs and TARGET_MEM_REFs
>> in ADDR_EXPR context is meaningless so you don't need to jump through hoops
>> to get at it or preserve it in any way, likewise for CLIQUE/BASE if it
>> were present.
>
> Ah, OK.
>
>> Maybe you can simplify code with this.
>
> In the end it didn't really simplify the code, since internal-fn.c
> uses the address to build a (TARGET_)MEM_REF, and the alias information
> of that ref needs to be correct, since it gets carried across to the
> MEM rtx. But it does mean that the alias_ptr_type check in the previous:
>
> if (TREE_CODE (mem) == TARGET_MEM_REF
> && types_compatible_p (TREE_TYPE (mem), type)
> && alias_ptr_type == TREE_TYPE (TMR_OFFSET (mem))
> && integer_zerop (TMR_OFFSET (mem)))
> return mem;
>
> made no sense: we should simply replace the TMR_OFFSET if it has
> the wrong type.
>
>> As you're introducing &TARGET_MEM_REF as a valid construct (it weren't
>> before) you'll run into missing / misguided foldings eventually. So
>> be prepared to fix up fallout.
>
> OK :-) I haven't hit any new places yet, but like you say, I'll be on
> the lookout.
>
> Is the version below OK? Tested on aarch64-linux-gnu, x86_64-linux-gnu
> and powerpc64le-linux-gnu.
>
> Richard
>
>
> 2018-01-09 Richard Sandiford <richard.sandif...@linaro.org>
> Alan Hayward <alan.hayw...@arm.com>
> David Sherwood <david.sherw...@arm.com>
>
> gcc/
> * expr.c (expand_expr_addr_expr_1): Handle ADDR_EXPRs of
> TARGET_MEM_REFs.
> * gimple-expr.h (is_gimple_addressable: Likewise.
> * gimple-expr.c (is_gimple_address): Likewise.
> * internal-fn.c (expand_call_mem_ref): New function.
> (expand_mask_load_optab_fn): Use it.
> (expand_mask_store_optab_fn): Likewise.
OK.
jeff
