Re: Make ivopts handle calls to internal functions

[Christophe Lyon <christophe dot lyon at linaro dot org>]

Hi,


On 13 January 2018 at 16:34, Jeff Law <law@redhat.com> wrote:
> 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


I've reported that the updated tests fail on aarch64-none-elf -mabi=ilp32:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83848

Christophe