Re: Make ivopts handle calls to internal functions

[Jeff Law <law at redhat dot com>]

On 01/09/2018 08:23 AM, Richard Sandiford wrote:
> Richard Biener <richard.guenther@gmail.com> writes:
>> On Mon, Nov 20, 2017 at 12:31 PM, Bin.Cheng <amker.cheng@gmail.com> wrote:
>>> On Fri, Nov 17, 2017 at 3:03 PM, Richard Sandiford
>>> <richard.sandiford@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.sandiford@linaro.org>
> 	    Alan Hayward  <alan.hayward@arm.com>
> 	    David Sherwood  <david.sherwood@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