On 9/3/19 4:00 PM, Jeff Law wrote:
> On 9/3/19 8:01 AM, Kyrill Tkachov wrote:
>> Hi all,
>>
>> On 9/5/18 12:48 PM, a...@codesourcery.com wrote:
>>> At present, pointers passed to builtin functions, including atomic
>>> operators,
>>> are stripped of their address space properties. This doesn't seem
to be
>>> deliberate, it just omits to copy them.
>>>
>>> Not only that, but it forces pointer sizes to Pmode, which isn't
>>> appropriate
>>> for all address spaces.
>>>
>>> This patch attempts to correct both issues. It works for GCN
atomics and
>>> GCN OpenACC gang-private variables.
>>>
>>> 2018-09-05 Andrew Stubbs <a...@codesourcery.com>
>>> Julian Brown <jul...@codesourcery.com>
>>>
>>> gcc/
>>> * builtins.c (get_builtin_sync_mem): Handle address spaces.
>>
>> Sorry for responding to this so late. I'm testing a rebased version of
>> Richard's OOL atomic patches [1] and am hitting an ICE building the
>> -mabi=ilp32 libgfortran multilib for aarch64-none-elf:
>>
>> 0x7284db emit_library_call_value_1(int, rtx_def*, rtx_def*,
>> libcall_type, machine_mode, int, std::pair<rtx_def*, machine_mode>*)
>> $SRC/gcc/calls.c:4915
>> 0x1037817 emit_library_call_value(rtx_def*, rtx_def*, libcall_type,
>> machine_mode, rtx_def*, machine_mode, rtx_def*, machine_mode, rtx_def*,
>> machine_mode)
>> $SRC/gcc/rtl.h:4240
>> 0x1037817 aarch64_expand_compare_and_swap(rtx_def**)
>> $SRC/gcc/config/aarch64/aarch64.c:16981
>> 0x1353a43 gen_atomic_compare_and_swapsi(rtx_def*, rtx_def*, rtx_def*,
>> rtx_def*, rtx_def*, rtx_def*, rtx_def*, rtx_def*)
>> $SRC/gcc/config/aarch64/atomics.md:34
>> 0xb1f9f1 insn_gen_fn::operator()(rtx_def*, rtx_def*, rtx_def*,
rtx_def*,
>> rtx_def*, rtx_def*, rtx_def*, rtx_def*) const
>> $SRC/gcc/recog.h:324
>> 0xb1f9f1 maybe_gen_insn(insn_code, unsigned int, expand_operand*)
>> $SRC/gcc/optabs.c:7443
>> 0xb1fa78 maybe_expand_insn(insn_code, unsigned int, expand_operand*)
>> $SRC/gcc/optabs.c:7459
>> 0xb21024 expand_atomic_compare_and_swap(rtx_def**, rtx_def**, rtx_def*,
>> rtx_def*, rtx_def*, bool, memmodel, memmodel)
>> $SRC/gcc/optabs.c:6448
>> 0x709bd3 expand_builtin_atomic_compare_exchange
>> $SRC/gcc/builtins.c:6379
>> 0x71a4e9 expand_builtin(tree_node*, rtx_def*, rtx_def*,
machine_mode, int)
>> $SRC/gcc/builtins.c:8147
>> 0x88b746 expand_expr_real_1(tree_node*, rtx_def*, machine_mode,
>> expand_modifier, rtx_def**, bool)
>> $SRC/gcc/expr.c:11052
>> 0x88cce6 expand_expr_real(tree_node*, rtx_def*, machine_mode,
>> expand_modifier, rtx_def**, bool)
>> $SRC/gcc/expr.c:8289
>> 0x74cb47 expand_expr
>> $SRC/gcc/expr.h:281
>> 0x74cb47 expand_call_stmt
>> $SRC/gcc/cfgexpand.c:2731
>> 0x74cb47 expand_gimple_stmt_1
>> $SRC/gcc/cfgexpand.c:3710
>> 0x74cb47 expand_gimple_stmt
>> $SRC/gcc/cfgexpand.c:3875
>> 0x75439b expand_gimple_basic_block
>> $SRC/gcc/cfgexpand.c:5915
>> 0x7563ab execute
>> $SRC/gcc/cfgexpand.c:6538
>> Please submit a full bug report,
>> with preprocessed source if appropriate.
>> Please include the complete backtrace with any bug report.
>> See <https://gcc.gnu.org/bugs/> for instructions.
>>
>> A MEM rtx now uses a DImode address where for ILP32 we expect SImode.
>>
>> This looks to be because....
>>
>> [1] https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00062.html
>>
>>
>>> ---
>>> gcc/builtins.c | 13 ++++++++++---
>>> 1 file changed, 10 insertions(+), 3 deletions(-)
>>>
>> 0002-Propagate-address-spaces-to-builtins.patch
>>
>> diff --git a/gcc/builtins.c b/gcc/builtins.c
>> index 58ea747..361361c 100644
>> --- a/gcc/builtins.c
>> +++ b/gcc/builtins.c
>> @@ -5781,14 +5781,21 @@ static rtx
>> get_builtin_sync_mem (tree loc, machine_mode mode)
>> {
>> rtx addr, mem;
>> + int addr_space = TYPE_ADDR_SPACE (POINTER_TYPE_P (TREE_TYPE (loc))
>> + ? TREE_TYPE (TREE_TYPE (loc))
>> + : TREE_TYPE (loc));
>> + scalar_int_mode addr_mode = targetm.addr_space.address_mode
>> (addr_space);
>>
>> ... This now returns Pmode (the default for the hook) for aarch64
ILP32,
>> which is always DImode.
>>
>> - addr = expand_expr (loc, NULL_RTX, ptr_mode, EXPAND_SUM);
>>
>> Before this patch we used ptr_mode, which does the right thing for
>> AArch64 ILP32.
>> Do you think we should just be implementing
>> targetm.addr_space.address_mode for AArch64 to return SImode for ILP32?
> Possibly. Is there any fallout from making that change?
Unfortunately some ICEs when building libgcc with POST_INC arguments
output :(
I'll need to dig further.
