diff mbox series

[29/31] PDP11: Use `const_double_zero' to express double zero constant

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

Commit Message

Maciej W. Rozycki Nov. 20, 2020, 3:36 a.m. UTC
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(-)

Comments

Jeff Law Nov. 21, 2020, 4:07 a.m. UTC | #1
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
Martin Liška Dec. 15, 2020, 8:26 a.m. UTC | #2
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
Maciej W. Rozycki Dec. 15, 2020, 2:06 p.m. UTC | #3
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
Paul Koning Dec. 15, 2020, 6:02 p.m. UTC | #4
> 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
Maciej W. Rozycki Dec. 15, 2020, 6:38 p.m. UTC | #5
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 mbox series

Patch

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")