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