Message ID | alpine.LFD.2.21.2011200302090.656242@eddie.linux-mips.org |
---|---|
State | Accepted |
Headers | show |
Series | VAX: Bring the port up to date (yes, MODE_CC conversion is included) | expand |
On 11/19/20 8:36 PM, Maciej W. Rozycki wrote: > We do not define a comparison operation between floating-point and > integer data, including integer zero constant. Consequently the RTL > instruction stream presented to the post-reload comparison elimination > pass will include, where applicable, floating-point comparison insns > against `const_double:DF 0.0 [0x0.0p+0]' rather than `const_int 0 [0]', > meaning that the latter expression will not match when used in machine > description. > > Use `const_double_zero' then for the relevant patterns to match the > intended RTL instructions. > > gcc/ > * config/pdp11/pdp11.md (fcc_cc, fcc_ccnz): Use > `const_double_zero' to express double zero constant. OK jeff
On 11/20/20 4:36 AM, Maciej W. Rozycki wrote: > We do not define a comparison operation between floating-point and > integer data, including integer zero constant. Consequently the RTL > instruction stream presented to the post-reload comparison elimination > pass will include, where applicable, floating-point comparison insns > against `const_double:DF 0.0 [0x0.0p+0]' rather than `const_int 0 [0]', > meaning that the latter expression will not match when used in machine > description. > > Use `const_double_zero' then for the relevant patterns to match the > intended RTL instructions. > > gcc/ > * config/pdp11/pdp11.md (fcc_cc, fcc_ccnz): Use > `const_double_zero' to express double zero constant. > --- > gcc/config/pdp11/pdp11.md | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/gcc/config/pdp11/pdp11.md b/gcc/config/pdp11/pdp11.md > index 7a4d50fdba9..cdef49f3979 100644 > --- a/gcc/config/pdp11/pdp11.md > +++ b/gcc/config/pdp11/pdp11.md > @@ -105,7 +105,7 @@ (define_subst "fcc_cc" > (clobber (reg FCC_REGNUM))] > "" > [(set (reg:CC FCC_REGNUM) > - (compare:CC (match_dup 1) (const_int 0))) > + (compare:CC (match_dup 1) (const_double_zero))) > (set (match_dup 0) (match_dup 1))]) > > (define_subst "fcc_ccnz" > @@ -113,7 +113,7 @@ (define_subst "fcc_ccnz" > (clobber (reg FCC_REGNUM))] > "" > [(set (reg:CCNZ FCC_REGNUM) > - (compare:CCNZ (match_dup 1) (const_int 0))) > + (compare:CCNZ (match_dup 1) (const_double_zero))) > (set (match_dup 0) (match_dup 1))]) > > (define_subst_attr "cc_cc" "cc_cc" "_nocc" "_cc") > Hello. If I see correctly, starting from this revision I can't compile a cross compiler of x86_64-linux-gnu: ../configure --target=pdp11-aout --disable-bootstrap --enable-languages=c,c++ --disable-multilib --enable-obsolete $ make build/genemit ../../gcc/common.md ../../gcc/config/pdp11/pdp11.md \ insn-conditions.md > tmp-emit.c genemit: Internal error: abort in gen_exp, at genemit.c:202 make: *** [Makefile:2427: s-emit] Error 1 $ Breakpoint 1, fancy_abort (file=0x435238 "../../gcc/genemit.c", line=202, func=0x435230 "gen_exp") at ../../gcc/errors.c:133 133 internal_error ("abort in %s, at %s:%d", func, trim_filename (file), line); (gdb) bt #0 fancy_abort (file=0x435238 "../../gcc/genemit.c", line=202, func=0x435230 "gen_exp") at ../../gcc/errors.c:133 #1 0x0000000000402e4b in gen_exp (x=0x470de0, subroutine_type=DEFINE_INSN, used=0x4aa4b0 "", info=0x7fffffffdea0) at ../../gcc/genemit.c:202 #2 0x0000000000402f68 in gen_exp (x=0x4d6c70, subroutine_type=DEFINE_INSN, used=0x4aa4b0 "", info=0x7fffffffdea0) at ../../gcc/genemit.c:227 #3 0x0000000000402f68 in gen_exp (x=0x4d6c50, subroutine_type=DEFINE_INSN, used=0x4aa4b0 "", info=0x7fffffffdea0) at ../../gcc/genemit.c:227 #4 0x000000000040307d in gen_exp (x=0x4aa490, subroutine_type=DEFINE_INSN, used=0x4aa4b0 "", info=0x7fffffffdea0) at ../../gcc/genemit.c:255 #5 0x00000000004036c1 in gen_insn (info=0x7fffffffdea0) at ../../gcc/genemit.c:435 #6 0x0000000000404930 in main (argc=3, argv=0x7fffffffdfc8) at ../../gcc/genemit.c:911 Can you please take a look? Thanks, Martin
On Tue, 15 Dec 2020, Martin Liška wrote: > If I see correctly, starting from this revision I can't compile a cross > compiler of x86_64-linux-gnu: > > ../configure --target=pdp11-aout --disable-bootstrap --enable-languages=c,c++ > --disable-multilib --enable-obsolete Thanks for the heads-up and the details of the configuration, and sorry for the breakage. > $ make > build/genemit ../../gcc/common.md ../../gcc/config/pdp11/pdp11.md \ > insn-conditions.md > tmp-emit.c > genemit: Internal error: abort in gen_exp, at genemit.c:202 > make: *** [Makefile:2427: s-emit] Error 1 > > $ Breakpoint 1, fancy_abort (file=0x435238 "../../gcc/genemit.c", line=202, > func=0x435230 "gen_exp") at ../../gcc/errors.c:133 > 133 internal_error ("abort in %s, at %s:%d", func, trim_filename (file), > line); > (gdb) bt > #0 fancy_abort (file=0x435238 "../../gcc/genemit.c", line=202, func=0x435230 > "gen_exp") at ../../gcc/errors.c:133 > #1 0x0000000000402e4b in gen_exp (x=0x470de0, subroutine_type=DEFINE_INSN, > used=0x4aa4b0 "", info=0x7fffffffdea0) at ../../gcc/genemit.c:202 > #2 0x0000000000402f68 in gen_exp (x=0x4d6c70, subroutine_type=DEFINE_INSN, > used=0x4aa4b0 "", info=0x7fffffffdea0) at ../../gcc/genemit.c:227 > #3 0x0000000000402f68 in gen_exp (x=0x4d6c50, subroutine_type=DEFINE_INSN, > used=0x4aa4b0 "", info=0x7fffffffdea0) at ../../gcc/genemit.c:227 > #4 0x000000000040307d in gen_exp (x=0x4aa490, subroutine_type=DEFINE_INSN, > used=0x4aa4b0 "", info=0x7fffffffdea0) at ../../gcc/genemit.c:255 > #5 0x00000000004036c1 in gen_insn (info=0x7fffffffdea0) at > ../../gcc/genemit.c:435 > #6 0x0000000000404930 in main (argc=3, argv=0x7fffffffdfc8) at > ../../gcc/genemit.c:911 I don't have the target environment available and consequently the target does not complete building for me: The directory that should contain system headers does not exist: .../usr/sysroot/usr/include make[2]: *** [Makefile:3218: stmp-fixinc] Error 1 or otherwise I would have at least tried to compile it before submitting the change. The configuration does build far enough to trigger this issue though and I can confirm the change is indeed the culprit. > Can you please take a look? I'm fairly sure this is due to the difference in TARGET_SUPPORTS_WIDE_INT with the VAX backend vs the PDP-11 one. I have an idea how this should be addressed and will be implementing it shortly. NB the backend fails `-Werror' compilation: In file included from ./tm.h:18, from .../gcc/backend.h:28, from .../gcc/varasm.c:31: .../gcc/varasm.c: In function 'void assemble_zeros(long unsigned int)': .../gcc/config/pdp11/pdp11.h:622:20: error: format '%o' expects argument of type 'unsigned int', but argument 3 has type 'long unsigned int' [-Werror=format=] 622 | fprintf (FILE, "\t.blkb\t%o\n", (SIZE) & 0xffff); \ | ^~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~ | | | long unsigned int etc., so that has to be disabled. This may be with 64-bit hosts only, I'm not sure, however the way varargs are handled means the issue will likely cause a broken compiler to be built if the warning is ignored, at least with some hosts. Paul, can you please look into it sometime? Maciej
> On Dec 15, 2020, at 9:06 AM, Maciej W. Rozycki <macro@linux-mips.org> wrote: > > On Tue, 15 Dec 2020, Martin Liška wrote: > >> If I see correctly, starting from this revision I can't compile a cross >> compiler of x86_64-linux-gnu: >> >> ../configure --target=pdp11-aout --disable-bootstrap --enable-languages=c,c++ >> --disable-multilib --enable-obsolete PDP11 doesn't do C++, there isn't any way to do that with aout that I have seen (no named section support). There's been some experimental work on ELF for pdp11 (!) which should make this possible but I haven't tried that yet. > > Thanks for the heads-up and the details of the configuration, and sorry > for the breakage. > ... > >> Can you please take a look? > > I'm fairly sure this is due to the difference in TARGET_SUPPORTS_WIDE_INT > with the VAX backend vs the PDP-11 one. I have an idea how this should be > addressed and will be implementing it shortly. What's the difference? pdp11 does support wide int. > NB the backend fails `-Werror' compilation: > > In file included from ./tm.h:18, > from .../gcc/backend.h:28, > from .../gcc/varasm.c:31: > .../gcc/varasm.c: In function 'void assemble_zeros(long unsigned int)': > .../gcc/config/pdp11/pdp11.h:622:20: error: format '%o' expects argument of type 'unsigned int', but argument 3 has type 'long unsigned int' [-Werror=format=] > 622 | fprintf (FILE, "\t.blkb\t%o\n", (SIZE) & 0xffff); \ > | ^~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~ > | | > | long unsigned int > > etc., so that has to be disabled. This may be with 64-bit hosts only, I'm > not sure, however the way varargs are handled means the issue will likely > cause a broken compiler to be built if the warning is ignored, at least > with some hosts. > > Paul, can you please look into it sometime? Will do, thanks. paul
On Tue, 15 Dec 2020, Paul Koning wrote: > > I'm fairly sure this is due to the difference in TARGET_SUPPORTS_WIDE_INT > > with the VAX backend vs the PDP-11 one. I have an idea how this should be > > addressed and will be implementing it shortly. > > What's the difference? pdp11 does support wide int. The VAX backend, being old-fashioned, does not and therefore overloads the CONST_DOUBLE rtx for that purpose, making it possibly relevant here (it has turned out not after all: the cause is the use of named patterns for the CC-setting insns referred to by splitters, which the VAX backend does not have). NB there's been no rush to update the VAX backend for proper wide integer support, which will require some caution AFAICT. Anyway I have a fix now I am satisfied with and will be posting it shortly. I'll appreciate it if you verify it as I have no means beyond making sure that it builds. I'll cc you on the upcoming submission; a general maintainer will have to approve it though. Maciej
diff --git a/gcc/config/pdp11/pdp11.md b/gcc/config/pdp11/pdp11.md index 7a4d50fdba9..cdef49f3979 100644 --- a/gcc/config/pdp11/pdp11.md +++ b/gcc/config/pdp11/pdp11.md @@ -105,7 +105,7 @@ (define_subst "fcc_cc" (clobber (reg FCC_REGNUM))] "" [(set (reg:CC FCC_REGNUM) - (compare:CC (match_dup 1) (const_int 0))) + (compare:CC (match_dup 1) (const_double_zero))) (set (match_dup 0) (match_dup 1))]) (define_subst "fcc_ccnz" @@ -113,7 +113,7 @@ (define_subst "fcc_ccnz" (clobber (reg FCC_REGNUM))] "" [(set (reg:CCNZ FCC_REGNUM) - (compare:CCNZ (match_dup 1) (const_int 0))) + (compare:CCNZ (match_dup 1) (const_double_zero))) (set (match_dup 0) (match_dup 1))]) (define_subst_attr "cc_cc" "cc_cc" "_nocc" "_cc")