| Message ID | c33c97c491ef8718f23f3282ae5caafbe141df10.1536144068.git.ams@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
| Series | AMD GCN Port | expand |
On Wed, Sep 5, 2018 at 1:50 PM <ams@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. OK. Richard. > 2018-09-05 Andrew Stubbs <ams@codesourcery.com> > Julian Brown <julian@codesourcery.com> > > gcc/ > * builtins.c (get_builtin_sync_mem): Handle address spaces. > --- > gcc/builtins.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) >
On Sep 05 2018, <ams@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 <ams@codesourcery.com> > Julian Brown <julian@codesourcery.com> > > gcc/ > * builtins.c (get_builtin_sync_mem): Handle address spaces. That breaks aarch64 ILP32. ../../../../libgomp/ordered.c: In function 'GOMP_doacross_wait': ../../../../libgomp/ordered.c:486:1: internal compiler error: output_operand: invalid address mode 486 | } | ^ 0xf7219f aarch64_print_address_internal ../../gcc/config/aarch64/aarch64.c:7163 0xf7269b aarch64_print_operand_address ../../gcc/config/aarch64/aarch64.c:7267 0x8871ef output_address(machine_mode, rtx_def*) ../../gcc/final.c:4069 0xf7302b aarch64_print_operand ../../gcc/config/aarch64/aarch64.c:6952 0x887133 output_operand(rtx_def*, int) ../../gcc/final.c:4053 0x887b5b output_asm_insn(char const*, rtx_def**) ../../gcc/final.c:3965 0x889157 output_asm_insn(char const*, rtx_def**) ../../gcc/final.c:3842 0x889157 final_scan_insn_1 ../../gcc/final.c:3103 0x88984b final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*) ../../gcc/final.c:3149 0x889b1f final_1 ../../gcc/final.c:2019 0x88aaff rest_of_handle_final ../../gcc/final.c:4660 0x88aaff execute ../../gcc/final.c:4734 Andreas.
On 22/09/18 19:51, Andreas Schwab wrote:
> That breaks aarch64 ILP32.
I'm struggling to reproduce this because apparently I don't know how to
build aarch64 ILP32.
Presumably, in order to be building libgomp this must be an
aarch64-linux-gnu toolchain, but when I set --with-abi=ilp32 I can't
build glibc:
In file included from ../nptl/descr.h:24,
from ../sysdeps/aarch64/nptl/tls.h:44,
from ../sysdeps/unix/sysv/linux/aarch64/sysdep.h:29,
from <stdin>:1:
../include/setjmp.h:50:3: error: static assertion failed: "offset of
__saved_mask field of struct __jmp_buf_tag != 184
What should I have done?
Thanks
Andrew
On Sep 24 2018, Andrew Stubbs <ams@codesourcery.com> wrote:
> What should I have done?
Make sure you have the ILP32 patches for glibc and kernel. You can get
them from the arm/ilp32 branch on sourceware.org
<http://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/arm/ilp32>
and the staging/ilp32-4.17 branch of the arm64 kernel tree
<https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/log/?h=staging/ilp32-4.17>.
You can also get pre-built packages from
<https://download.opensuse.org/repositories/devel:/ARM:/Factory:/Contrib:/ILP32/standard/>
Andreas.
Hi all, On 9/5/18 12:48 PM, ams@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 <ams@codesourcery.com> > Julian Brown <julian@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? Thanks, Kyrill - addr = convert_memory_address (Pmode, addr); + addr = expand_expr (loc, NULL_RTX, addr_mode, EXPAND_SUM); /* Note that we explicitly do not want any alias information for this memory, so that we kill all other live memories. Otherwise we don't satisfy the full barrier semantics of the intrinsic. */ - mem = validize_mem (gen_rtx_MEM (mode, addr)); + mem = gen_rtx_MEM (mode, addr); + + set_mem_addr_space (mem, addr_space); + + mem = validize_mem (mem); /* The alignment needs to be at least according to that of the mode. */ set_mem_align (mem, MAX (GET_MODE_ALIGNMENT (mode),
On 9/3/19 8:01 AM, Kyrill Tkachov wrote: > Hi all, > > On 9/5/18 12:48 PM, ams@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 <ams@codesourcery.com> >> Julian Brown <julian@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? Jeff
On 03/09/2019 15:01, Kyrill Tkachov wrote: > 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: I thought Andreas already fixed ILP32. https://gcc.gnu.org/ml/gcc-patches/2018-09/msg01439.html Andrew
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, ams@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 <ams@codesourcery.com> >>> Julian Brown <julian@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. Thanks, Kyrill > > Jeff
On 9/4/19 3:21 PM, Kyrill Tkachov wrote: > > 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, ams@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 <ams@codesourcery.com> > >>> Julian Brown <julian@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. Adding a convert_memory_address to ptr_mode before each call to emit_library_call_value in the OOL atomics code to convert the addresses does fix it the ICEs. Let's see what testing shows. Kyrill > > Thanks, > > Kyrill > > > > > > Jeff
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); - addr = expand_expr (loc, NULL_RTX, ptr_mode, EXPAND_SUM); - addr = convert_memory_address (Pmode, addr); + addr = expand_expr (loc, NULL_RTX, addr_mode, EXPAND_SUM); /* Note that we explicitly do not want any alias information for this memory, so that we kill all other live memories. Otherwise we don't satisfy the full barrier semantics of the intrinsic. */ - mem = validize_mem (gen_rtx_MEM (mode, addr)); + mem = gen_rtx_MEM (mode, addr); + + set_mem_addr_space (mem, addr_space); + + mem = validize_mem (mem); /* The alignment needs to be at least according to that of the mode. */ set_mem_align (mem, MAX (GET_MODE_ALIGNMENT (mode),