| Message ID | AM5PR0802MB26106A94BD2269F13E58F6C883DC0@AM5PR0802MB2610.eurprd08.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
On 27/06/17 14:39, Wilco Dijkstra wrote: > This patch fixes a failure in gcc.target/aarch64/reload-valid-spoff.c > triggered by https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01367.html - > it supersedes https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01907.html > as this fixes the root cause of the failure. > > In ILP32 all memory accesses must have Pmode as the base address, but > aarch64_expand_mov_immediate wasn't emitting a conversion in one case. > Besides fixing this add an assert that flags any MEM operands that are > not Pmode. > > Passes regress (with/without ilp32). OK for commit? > > ChangeLog: > 2017-06-27 Wilco Dijkstra <wdijkstr@arm.com> > > * config/aarch64/aarch64 (aarch64_expand_mov_immediate): > Convert memory address to Pmode. Missing ChangeLog entry for the new assert. > -- > diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c > index 329d244e9cf16dbdf849e5dd02b3999caf0cd5a7..9038748ba049ba589f067f3f04c31704fe673d2c 100644 > --- a/gcc/config/aarch64/aarch64.c > +++ b/gcc/config/aarch64/aarch64.c > @@ -1958,6 +1958,8 @@ aarch64_expand_mov_immediate (rtx dest, rtx imm) > gcc_assert (can_create_pseudo_p ()); > base = gen_reg_rtx (ptr_mode); > aarch64_expand_mov_immediate (base, XEXP (mem, 0)); > + if (ptr_mode != Pmode) > + base = convert_memory_address (Pmode, base); > mem = gen_rtx_MEM (ptr_mode, base); > } > > @@ -5207,6 +5209,7 @@ aarch64_print_operand (FILE *f, rtx x, int code) > > case MEM: > output_address (GET_MODE (x), XEXP (x, 0)); > + gcc_assert (GET_MODE (XEXP (x, 0)) == Pmode); > break; This is worthy of a comment. Something like "All memory references must be in Pmode, which is the natural mode of the machine. This remains the case even if ptr_mode is different, as for ILP32." Ok with those changes. R. > > case CONST: >
On Jun 27 2017, Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote: > @@ -5207,6 +5209,7 @@ aarch64_print_operand (FILE *f, rtx x, int code) > > case MEM: > output_address (GET_MODE (x), XEXP (x, 0)); > + gcc_assert (GET_MODE (XEXP (x, 0)) == Pmode); > break; > > case CONST: That breaks a lot of gnat tests in ilp32 mode: spawn -ignore SIGHUP /opt/gcc/gcc-20170630/Build/gcc/gnatmake --GCC=/opt/gcc/gcc-20170630/Build/gcc/xgcc --GNATBIND=/opt/gcc/gcc-20170630/Build/gcc/gnatbind --GNATLINK=/opt/gcc/gcc-20170630/Build/gcc/gnatlink -cargs -B/opt/gcc/gcc-20170630/Build/gcc -largs --GCC=/opt/gcc/gcc-20170630/Build/gcc/xgcc -B/opt/gcc/gcc-20170630/Build/gcc -mabi=ilp32 -margs --RTS=/opt/gcc/gcc-20170630/Build/aarch64-suse-linux/ilp32/libada -q -f /opt/gcc/gcc-20170630/gcc/testsuite/gnat.dg/abstract_with_anonymous_result.adb -mabi=ilp32 -fno-diagnostics-show-caret -fdiagnostics-color=never -lm -o ./abstract_with_anonymous_result.exe +===========================GNAT BUG DETECTED==============================+ | 8.0.0 20170630 (experimental) [trunk revision 249826] (aarch64-suse-linux) GCC error:| | in aarch64_print_operand, at config/aarch64/aarch64.c:5271 | | Error detected around /opt/gcc/gcc-20170630/gcc/testsuite/gnat.dg/abstract_with_anonymous_result.adb:37:5| | Please submit a bug report; see https://gcc.gnu.org/bugs/ . | | Use a subject line meaningful to you and us to track the bug. | | Include the entire contents of this bug box in the report. | | Include the exact command that you entered. | | Also include sources listed below. | +==========================================================================+ Andreas.
Andreas Schwab wrote: > @@ -5207,6 +5209,7 @@ aarch64_print_operand (FILE *f, rtx x, int code) > > case MEM: > output_address (GET_MODE (x), XEXP (x, 0)); > + gcc_assert (GET_MODE (XEXP (x, 0)) == Pmode); > break; > > case CONST: > That breaks a lot of gnat tests in ilp32 mode: That's interesting since it works fine in C and C++, and unless there are some ADA specific MD patterns, it seems unlikely it could only affect ADA. So how do you build the ADA backend? GCC can't even get past the configure as it doesn't appear to understand GNAT is installed after finding it... Is there some more magic setup required for ADA? Wilco checking for gnatbind... gnatbind checking for gnatmake... gnatmake checking whether compiler driver understands Ada... no checking how to compare bootstrapped objects... cmp --ignore-initial=16 $$f1 $$f2 checking for objdir... .libs configure: WARNING: using in-tree isl, disabling version check configure: error: GNAT is required to build ada >gnat -v GNAT 7.1.0 Copyright 1996-2017, Free Software Foundation, Inc. List of available commands ...
On Jul 04 2017, Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
> checking whether compiler driver understands Ada... no
You need to fix that first.
Andreas.
On Tue, Jul 04, 2017 at 12:19:35PM +0000, Wilco Dijkstra wrote: > Andreas Schwab wrote: > > @@ -5207,6 +5209,7 @@ aarch64_print_operand (FILE *f, rtx x, int code) > > > > case MEM: > > output_address (GET_MODE (x), XEXP (x, 0)); > > + gcc_assert (GET_MODE (XEXP (x, 0)) == Pmode); > > break; > > > > case CONST: > > > That breaks a lot of gnat tests in ilp32 mode: > > That's interesting since it works fine in C and C++, and unless there are some > ADA specific MD patterns, it seems unlikely it could only affect ADA. > > So how do you build the ADA backend? GCC can't even get past the configure as > it doesn't appear to understand GNAT is installed after finding it... > Is there some more magic setup required for ADA? > > Wilco > > checking for gnatbind... gnatbind > checking for gnatmake... gnatmake > checking whether compiler driver understands Ada... no The line above means that using $CC -c dummy_ada_file.adb generates an error. $CC is "gcc" by default, some installs use gnatgcc instead of gcc to provide an Ada compiler. In other words, try to do: gcc -c hello.adb manually and you'll see the error for yourself, or check your config.log file. hello.adb can be as simple as the following single line: procedure hello is begin null; end;
On Tue, Jul 4, 2017 at 1:56 PM, Arnaud Charlet <charlet@adacore.com> wrote: > On Tue, Jul 04, 2017 at 12:19:35PM +0000, Wilco Dijkstra wrote: >> Andreas Schwab wrote: >> > @@ -5207,6 +5209,7 @@ aarch64_print_operand (FILE *f, rtx x, int code) >> > >> > case MEM: >> > output_address (GET_MODE (x), XEXP (x, 0)); >> > + gcc_assert (GET_MODE (XEXP (x, 0)) == Pmode); >> > break; >> > >> > case CONST: >> >> > That breaks a lot of gnat tests in ilp32 mode: >> >> That's interesting since it works fine in C and C++, and unless there are some >> ADA specific MD patterns, it seems unlikely it could only affect ADA. >> >> So how do you build the ADA backend? GCC can't even get past the configure as >> it doesn't appear to understand GNAT is installed after finding it... >> Is there some more magic setup required for ADA? >> >> Wilco >> >> checking for gnatbind... gnatbind >> checking for gnatmake... gnatmake >> checking whether compiler driver understands Ada... no > > The line above means that using $CC -c dummy_ada_file.adb > generates an error. > > $CC is "gcc" by default, some installs use gnatgcc instead of gcc to > provide an Ada compiler. > > In other words, try to do: > > gcc -c hello.adb > > manually and you'll see the error for yourself, or check your config.log > file. > > hello.adb can be as simple as the following single line: > > procedure hello is begin null; end; Yeah it turns out that on the machine Wilco was using, we are running 14.04 which has a gcc 4.8 base compiler that didn't have Ada on for AArch64. I think we can work around by installing a gcc-5 package and then setting CC to gcc-5. Ramana
Hi, On Tue, 4 Jul 2017, Ramana Radhakrishnan wrote: > Yeah it turns out that on the machine Wilco was using, we are running > 14.04 which has a gcc 4.8 base compiler that didn't have Ada on for > AArch64. > > I think we can work around by installing a gcc-5 package and then > setting CC to gcc-5. You'll probably also have to set GNATBIND and GNATMAKE to the appropriately suffixed variants. Just saying, because that's what I'm usually forgetting and end up with strange errors :) Ciao, Michael.
Michael Matz wrote: > > You'll probably also have to set GNATBIND and GNATMAKE to the > appropriately suffixed variants. Just saying, because that's what I'm > usually forgetting and end up with strange errors :) Configure seems to be able to find gnatbind/gnatmake as they are in /usr/bin. Compiling a simple .adb file as suggested gives: > /usr/bin/gcc-5 tmp.adb -S gcc-5: error trying to exec 'gnat1': execvp: No such file or directory Is this part of the GCC driver, GNAT or something else that needs to be installed first? Wilco
Hi, On Tue, 4 Jul 2017, Wilco Dijkstra wrote: > > You'll probably also have to set GNATBIND and GNATMAKE to the > > appropriately suffixed variants. Just saying, because that's what I'm > > usually forgetting and end up with strange errors :) > > Configure seems to be able to find gnatbind/gnatmake as they are in /usr/bin. Sure, but if you use gcc-5 as compiler you'll want to use gnatbind-5 as well, not gnatbind (which presumably is from the default 4.8 compiler you have). > Compiling a simple .adb file as suggested gives: > > > /usr/bin/gcc-5 tmp.adb -S > gcc-5: error trying to exec 'gnat1': execvp: No such file or directory > > Is this part of the GCC driver, GNAT or something else that needs to be > installed first? Don't know the package naming convention on Ubuntu (?). For us it's e.g. gcc5-ada that you'd need to install in addition. google tells me gnat-5 should be it. Ciao, Michael.
On Jul 04 2017, Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
> Configure seems to be able to find gnatbind/gnatmake as they are in /usr/bin.
Strange there are ada tools, but no ada compiler. Are you sure you
installed all relevant ada packages?
Andreas.
On Tue, Jul 4, 2017 at 2:53 PM, Michael Matz <matz@suse.de> wrote: > Hi, > > On Tue, 4 Jul 2017, Wilco Dijkstra wrote: > >> > You'll probably also have to set GNATBIND and GNATMAKE to the >> > appropriately suffixed variants. Just saying, because that's what I'm >> > usually forgetting and end up with strange errors :) >> >> Configure seems to be able to find gnatbind/gnatmake as they are in /usr/bin. > > Sure, but if you use gcc-5 as compiler you'll want to use gnatbind-5 as > well, not gnatbind (which presumably is from the default 4.8 compiler you > have). > >> Compiling a simple .adb file as suggested gives: >> >> > /usr/bin/gcc-5 tmp.adb -S >> gcc-5: error trying to exec 'gnat1': execvp: No such file or directory >> >> Is this part of the GCC driver, GNAT or something else that needs to be >> installed first? > > Don't know the package naming convention on Ubuntu (?). For us it's e.g. > gcc5-ada that you'd need to install in addition. google tells me gnat-5 > should be it. gnat-7 was on the machine instead of gnat-5 and that was probably the cause of the fun and games. Ramana > > > Ciao, > Michael.
On Tue, Jun 27, 2017 at 6:39 AM, Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote: > This patch fixes a failure in gcc.target/aarch64/reload-valid-spoff.c > triggered by https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01367.html - > it supersedes https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01907.html > as this fixes the root cause of the failure. > > In ILP32 all memory accesses must have Pmode as the base address, but > aarch64_expand_mov_immediate wasn't emitting a conversion in one case. > Besides fixing this add an assert that flags any MEM operands that are > not Pmode. > > Passes regress (with/without ilp32). OK for commit? This looks related to PR 80266 in that one was crashing due to the store pair instruction like what was reported. Thanks, Andrew > > ChangeLog: > 2017-06-27 Wilco Dijkstra <wdijkstr@arm.com> > > * config/aarch64/aarch64 (aarch64_expand_mov_immediate): > Convert memory address to Pmode. > -- > diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c > index 329d244e9cf16dbdf849e5dd02b3999caf0cd5a7..9038748ba049ba589f067f3f04c31704fe673d2c 100644 > --- a/gcc/config/aarch64/aarch64.c > +++ b/gcc/config/aarch64/aarch64.c > @@ -1958,6 +1958,8 @@ aarch64_expand_mov_immediate (rtx dest, rtx imm) > gcc_assert (can_create_pseudo_p ()); > base = gen_reg_rtx (ptr_mode); > aarch64_expand_mov_immediate (base, XEXP (mem, 0)); > + if (ptr_mode != Pmode) > + base = convert_memory_address (Pmode, base); > mem = gen_rtx_MEM (ptr_mode, base); > } > > @@ -5207,6 +5209,7 @@ aarch64_print_operand (FILE *f, rtx x, int code) > > case MEM: > output_address (GET_MODE (x), XEXP (x, 0)); > + gcc_assert (GET_MODE (XEXP (x, 0)) == Pmode); > break; > > case CONST:
Andrew Pinski wrote: > > This looks related to PR 80266 in that one was crashing due to the > store pair instruction like what was reported. Yes it's the same bug. I've now finally reproduced it, it seems many stack addresses in Ada are SImode which is incorrect (and ultimately can trigger the LDP assertion like it did with -mcmodel=large). I don't see any checks in the backend that enforce Pmode for addresses, for example aarch64_classify_address doesn't reject SImode addresses... Wilco
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index 329d244e9cf16dbdf849e5dd02b3999caf0cd5a7..9038748ba049ba589f067f3f04c31704fe673d2c 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -1958,6 +1958,8 @@ aarch64_expand_mov_immediate (rtx dest, rtx imm) gcc_assert (can_create_pseudo_p ()); base = gen_reg_rtx (ptr_mode); aarch64_expand_mov_immediate (base, XEXP (mem, 0)); + if (ptr_mode != Pmode) + base = convert_memory_address (Pmode, base); mem = gen_rtx_MEM (ptr_mode, base); } @@ -5207,6 +5209,7 @@ aarch64_print_operand (FILE *f, rtx x, int code) case MEM: output_address (GET_MODE (x), XEXP (x, 0)); + gcc_assert (GET_MODE (XEXP (x, 0)) == Pmode); break; case CONST: