Re: [PATCH] genemit: Handle `const_double_zero' rtx

2021-01-07 Thread Maciej W. Rozycki
On Wed, 6 Jan 2021, Richard Sandiford wrote:

> VOIDmode const_doubles should only be used for integers that cannot
> be expressed as a sign-extended HOST_WIDE_INT.  So (const_double 0 0)
> is an invalid rtx in both integer and FP contexts.

 The updated change makes the VAX and PDP-11 backends stop producing 
those, but as I noted this is IMHO tangential to the choice between 
CONST0_RTX and CONST_DOUBLE_ATOF for code generators produced from named 
insns.  And the latter does not interpret the mode, i.e. does not enforce 
the policy (which itself I don't argue against), leaving it up to people 
writing machine descriptions to get it right, which I think is the right 
place.

> (FTR, the constant should also be the second operand to the plus,
> but that's obviously tangential.)

 Also FTR, I made it the first one deliberately, as I made my experimental 
change across all the four basic arithmetic operations, so for obvious 
reasons I wanted to prevent a divisor from being 0 lest the middle end get 
anxious about it.  Also contrary to documentation the middle end does 
present constants as the first operand in some cases where it is not 
supposed to (and does insist on doing so even if the backend says it's 
more expensive), as it was previously discussed in the context of COMPARE.

 Actually the VAX machine operand syntax is generic enough you can have an 
immediate as any operand, and in particular the first input to binary 
operations -- e.g. the minuend, and obviously also the first addend -- in 
their three-operand encodings; all the four basic arithmetic operations 
have them.

 I have posted the updated change as a patch series now, including you in 
the list of the recipients in case you wanted to chime in.  I appreciate 
your feedback and experience in this area, thank you!

  Maciej


Re: [PATCH] genemit: Handle `const_double_zero' rtx

2021-01-06 Thread Richard Sandiford via Gcc-patches
"Maciej W. Rozycki"  writes:
> On Wed, 16 Dec 2020, Maciej W. Rozycki wrote:
>
>> > CONST_DOUBLE_ATOF ("0", VOIDmode) seems malformed though, and I'd expect
>> > it to assert in REAL_MODE_FORMAT (via the format_helper constructor).
>> > I'm not sure the patch is strictly safer than the status quo.
>> 
>>  I may have missed that, though I did follow the chain of calls involved 
>> here to see if there is anything problematic.  As I say I have a limited 
>> way to verify this in practice as the PDP-11 code involved appears to me 
>> to be dead, and the situation does not apply to the VAX backend.  Maybe I 
>> could simulate it somehow artificially to see what happens.
>
>  I have made an experiment and arranged for a couple of builtins to refer
> to CONST_DOUBLE_ATOF ("0", VOIDmode) via expanders and it works just fine 
> except for failing to match an RTL insn, like:
>
> builtin.c: In function 't':
> builtin.c:18:1: error: unrecognizable insn:
>18 | }
>   | ^
> (insn 6 3 7 2 (set (reg/v:SF 23 [ f ])
> (plus:SF (const_double 0 [0] 0 [0] 0 [0] 0 [0])
> (reg/v:SF 32 [ f ]))) "builtin.c":5:6 -1
>  (nil))
> during RTL pass: vregs
> builtin.c:18:1: internal compiler error: in extract_insn, at recog.c:2315
>
> so it does work in principle and would produce something if there was a 
> matching insn.

Ah, yeah, I'd missed that there was a special case for VOIDmode
in format_helper::format_helper.  Still…

>> > FWIW, I agree with Jeff that this ought to be CONST0_RTX (mode).
>> 
>>  I'll have to update several places then and push the changes through full 
>> regression testing, so it'll probably take until the next week.
>
>  FWIW, CONST_DOUBLE_ATOF ("0", VOIDmode) is of course not equivalent to 
> CONST0_RTX (VOIDmode), as the latter produces a CONST_INT rather than a 
> CONST_DOUBLE rtx:
>
> builtin.c: In function 't':
> builtin.c:18:1: error: unrecognizable insn:
>18 | }
>   | ^
> (insn 6 3 7 2 (set (reg/v:SF 23 [ f ])
> (plus:SF (const_int 0 [0])
> (reg/v:SF 32 [ f ]))) "builtin1.c":5:6 -1
>  (nil))
> during RTL pass: vregs
> builtin.c:18:1: internal compiler error: in extract_insn, at recog.c:2315
>
> I suppose we do not have to support VOIDmode here, but I feel a bit uneasy 
> about the lack of identity mapping between machine description (where in 
> principle we could already use `const_double' with any mode, not only ones 
> CONST0_RTX expands to a CONST_DOUBLE for) and RTL produced if CONST0_RTX 
> was used rather than CONST_DOUBLE_ATOF, as CONST0_RTX does not always 
> return a CONST_DOUBLE rtx.

Both of the insns above are malformed though.  Floating-point
constants have to have floating-point modes.  const_ints and
VOIDmode const_doubles are always integers instead.

So even if CONST_DOUBLE_ATOF (…, VOIDmode) fails to ICE, I think it's
a constraint violation in that it's using a floating-point routine to
construct an integer rtx representation.

VOIDmode const_doubles should only be used for integers that cannot
be expressed as a sign-extended HOST_WIDE_INT.  So (const_double 0 0)
is an invalid rtx in both integer and FP contexts.

>  For the sake of the experiment I modified machine description further so 
> as to actually let the rtx produced by CONST_DOUBLE_ATOF ("0", VOIDmode) 
> through by providing suitable insns, and here's an excerpt from annotated 
> artificial assembly produced:
>
> #(insn 6 21 7 (set (reg/v:SF 0 %r0 [orig:23 f ] [23])
> #(plus:SF (const_double 0 [0] 0 [0] 0 [0] 0 [0])
> #(mem/c:SF (plus:SI (reg/f:SI 12 %ap)
> #(const_int 4 [0x4])) [1 f+0 S4 A32]))) "builtin.c":5:6 
> 199 {*addzsf3}
> # (expr_list:REG_DEAD (reg/f:SI 12 %ap)
> #(nil)))
>   #addf3 $0,4(%ap),%r0# 6 [c=40]  *addzsf3
>
> The CONST_DOUBLE rtx yielded the `$0' operand, so the expression made it 
> through the backend down to generated assembly (the leading comment 
> character comes from the artificial `*addzsf3' insn in modified MD).

Yeah, unfortunately RTL lacks the sanitary checks that gimple has,
so it's not too surprising that the pattern makes it through to final.
It's still malformed though.

(FTR, the constant should also be the second operand to the plus,
but that's obviously tangential.)

Thanks,
Richard


Re: [PATCH] genemit: Handle `const_double_zero' rtx

2021-01-05 Thread Maciej W. Rozycki
On Wed, 16 Dec 2020, Maciej W. Rozycki wrote:

> > CONST_DOUBLE_ATOF ("0", VOIDmode) seems malformed though, and I'd expect
> > it to assert in REAL_MODE_FORMAT (via the format_helper constructor).
> > I'm not sure the patch is strictly safer than the status quo.
> 
>  I may have missed that, though I did follow the chain of calls involved 
> here to see if there is anything problematic.  As I say I have a limited 
> way to verify this in practice as the PDP-11 code involved appears to me 
> to be dead, and the situation does not apply to the VAX backend.  Maybe I 
> could simulate it somehow artificially to see what happens.

 I have made an experiment and arranged for a couple of builtins to refer
to CONST_DOUBLE_ATOF ("0", VOIDmode) via expanders and it works just fine 
except for failing to match an RTL insn, like:

builtin.c: In function 't':
builtin.c:18:1: error: unrecognizable insn:
   18 | }
  | ^
(insn 6 3 7 2 (set (reg/v:SF 23 [ f ])
(plus:SF (const_double 0 [0] 0 [0] 0 [0] 0 [0])
(reg/v:SF 32 [ f ]))) "builtin.c":5:6 -1
 (nil))
during RTL pass: vregs
builtin.c:18:1: internal compiler error: in extract_insn, at recog.c:2315

so it does work in principle and would produce something if there was a 
matching insn.

> > FWIW, I agree with Jeff that this ought to be CONST0_RTX (mode).
> 
>  I'll have to update several places then and push the changes through full 
> regression testing, so it'll probably take until the next week.

 FWIW, CONST_DOUBLE_ATOF ("0", VOIDmode) is of course not equivalent to 
CONST0_RTX (VOIDmode), as the latter produces a CONST_INT rather than a 
CONST_DOUBLE rtx:

builtin.c: In function 't':
builtin.c:18:1: error: unrecognizable insn:
   18 | }
  | ^
(insn 6 3 7 2 (set (reg/v:SF 23 [ f ])
(plus:SF (const_int 0 [0])
(reg/v:SF 32 [ f ]))) "builtin1.c":5:6 -1
 (nil))
during RTL pass: vregs
builtin.c:18:1: internal compiler error: in extract_insn, at recog.c:2315

I suppose we do not have to support VOIDmode here, but I feel a bit uneasy 
about the lack of identity mapping between machine description (where in 
principle we could already use `const_double' with any mode, not only ones 
CONST0_RTX expands to a CONST_DOUBLE for) and RTL produced if CONST0_RTX 
was used rather than CONST_DOUBLE_ATOF, as CONST0_RTX does not always 
return a CONST_DOUBLE rtx.

 For the sake of the experiment I modified machine description further so 
as to actually let the rtx produced by CONST_DOUBLE_ATOF ("0", VOIDmode) 
through by providing suitable insns, and here's an excerpt from annotated 
artificial assembly produced:

#(insn 6 21 7 (set (reg/v:SF 0 %r0 [orig:23 f ] [23])
#(plus:SF (const_double 0 [0] 0 [0] 0 [0] 0 [0])
#(mem/c:SF (plus:SI (reg/f:SI 12 %ap)
#(const_int 4 [0x4])) [1 f+0 S4 A32]))) "builtin.c":5:6 199 
{*addzsf3}
# (expr_list:REG_DEAD (reg/f:SI 12 %ap)
#(nil)))
#addf3 $0,4(%ap),%r0# 6 [c=40]  *addzsf3

The CONST_DOUBLE rtx yielded the `$0' operand, so the expression made it 
through the backend down to generated assembly (the leading comment 
character comes from the artificial `*addzsf3' insn in modified MD).

 So with the CONST_DOUBLE_ATOF approach we have a coherent generic model 
where we can express arbitrary modes with CONST_DOUBLE rtxes without the 
need to analyse in the parser (genemit.c) whether the expression requested 
makes sense or not.  Whereas with the CONST0_RTX approach we'll either 
have to diagnose odd `const_double_zero' usage (making it inconsistent 
with `const_zero') or leave it to the undefined (and again inconsistent).

 What I think we want to do though is to make CONST_DOUBLE_ATOF ("0", 
mode) effectively alias to CONST0_RTX (mode) in the cases where the rtx 
produced is of the CONST_DOUBLE type.  By the look of `init_emit_once' I 
infer we do that already, so I must conclude that the choice between:

  printf ("CONST_DOUBLE_ATOF (\"0\", %smode)",
  GET_MODE_NAME (GET_MODE (x)));

and:

  printf ("CONST0_RTX (%smode)",
  GET_MODE_NAME (GET_MODE (x)));

for genemit.c is for those cases merely syntactic.  So I think I made the 
correct choice here and I'd still rather go with CONST_DOUBLE_ATOF, in 
which case we can simply ignore uninteresting modes.

 Have I expressed myself clearly enough?  I can post the patch I made the 
experiments with and builtin.c for the context.

 NB the festive season and the turn of events beforehand has delayed me a 
bit, but I now have a proper fix for the issue considered here, which 
actually removes the current use of VOIDmode with CONST_DOUBLE, and it's 
now only a matter of CONST0_RTX vs CONST_DOUBLE_ATOF to be used there.  
I'll post the patch shortly and we can continue the discussion in that 
context then.

 Thank you both for your input.

  Maciej


Re: [PATCH] genemit: Handle `const_double_zero' rtx

2020-12-16 Thread Maciej W. Rozycki
On Wed, 16 Dec 2020, Paul Koning wrote:

> > NB the PDP-11 pieces affected here and tripping this assertion are I 
> > believe dead code, as these insns are only ever produced by splitters and 
> > do not appear to be referred by their names.  
> 
> Right; I gave them names for documentation purposes, after all insns are 
> allowed to have names whether or not the name is referenced or is a 
> required name for RTL generation.

 You can use `*foo' however to have it printed in dumps and be able to 
refer to it otherwise, while not producing callable `gen_foo' entrypoints.  
It is not a feature GCC has always had, but it's been around for a little 
while already.

  Maciej


Re: [PATCH] genemit: Handle `const_double_zero' rtx

2020-12-16 Thread Paul Koning via Gcc-patches



> On Dec 16, 2020, at 6:35 AM, Maciej W. Rozycki  wrote:
> 
> ...
> NB the PDP-11 pieces affected here and tripping this assertion are I 
> believe dead code, as these insns are only ever produced by splitters and 
> do not appear to be referred by their names.  

Right; I gave them names for documentation purposes, after all insns are 
allowed to have names whether or not the name is referenced or is a required 
name for RTL generation.

paul




Re: [PATCH] genemit: Handle `const_double_zero' rtx

2020-12-16 Thread Maciej W. Rozycki
On Wed, 16 Dec 2020, Richard Sandiford wrote:

> >> Presumably you can't use CONST0_RTX (mode) here?  Assuming that's the
> >> case, then this is fine.
> >
> >  Well, `mode' is VOID for simplicity and to match what the middle end 
> > presents as an operand to the COMPARE operation, as also shown with the 
> > diff above, so with CONST0_RTX (mode) we'd be back to what amounts to 
> > `const0_rtx' aka `const_int 0', which is exactly what we want to get away 
> > from.
> >
> >  Alternatively we could possibly give `const_double_zero' a proper FP 
> > mode, but it seems to me like an unnecessary complication, as it would 
> > then have to be individually requested and iterated over all the relevant 
> > modes.
> 
> Generated FP const_double rtxes have to have a proper (non-VOID) mode
> though.  VOIDmode CONST_DOUBLEs are always (legacy) integers instead
> of FP values.
> 
> So yeah, giving const_double_zero a proper mode seems like the way to go.
> It's technically possible to drop it in a pure matching context, but I'm
> not sure how valuable that is in practice, given that other parts of the
> matched pattern would usually need to refer to the same mode anyway.

 Fair enough.  For some reason however VOIDmode CONST_DOUBLEs seem to work 
with the FP operations in the VAX backend for the purpose of post-reload 
comparison elimination while CONST_INTs do not.

> >  Have I missed anything here?
> >
> >  NB the PDP-11 pieces affected here and tripping this assertion are I 
> > believe dead code, as these insns are only ever produced by splitters and 
> > do not appear to be referred by their names.  We need this change however 
> > for completeness so that `const_double_zero' can be used in contexts where 
> > an insn actually will be referred by its name.
> >
> >  However the PDP-11 code being dead makes it a bit more difficult to 
> > examine actual consequences of such a change like I have proposed than it 
> > would otherwise be.  In these circumstances I think my proposal is safe if 
> > a bit overly cautious.
> 
> CONST_DOUBLE_ATOF ("0", VOIDmode) seems malformed though, and I'd expect
> it to assert in REAL_MODE_FORMAT (via the format_helper constructor).
> I'm not sure the patch is strictly safer than the status quo.

 I may have missed that, though I did follow the chain of calls involved 
here to see if there is anything problematic.  As I say I have a limited 
way to verify this in practice as the PDP-11 code involved appears to me 
to be dead, and the situation does not apply to the VAX backend.  Maybe I 
could simulate it somehow artificially to see what happens.

> FWIW, I agree with Jeff that this ought to be CONST0_RTX (mode).

 I'll have to update several places then and push the changes through full 
regression testing, so it'll probably take until the next week.

 Thanks for your input!

  Maciej


Re: [PATCH] genemit: Handle `const_double_zero' rtx

2020-12-16 Thread Richard Sandiford via Gcc-patches
"Maciej W. Rozycki"  writes:
> On Tue, 15 Dec 2020, Jeff Law wrote:
>
>> > @@ -1942,7 +1942,7 @@ gen_divdf3_cc (rtx operand0 ATTRIBUTE_UN
>> >gen_rtx_DIV (DFmode,
>> >operand1,
>> >operand2),
>> > -  const0_rtx)),
>> > +  CONST_DOUBLE_ATOF ("0", VOIDmode))),
>> >gen_rtx_SET (operand0,
>> >gen_rtx_DIV (DFmode,
>> >copy_rtx (operand1),
>> >
>> >gcc/
>> >* genemit.c (gen_exp) : Handle `const_double_zero' 
>> >rtx.
>> > ---
>> > Hi,
>> >
>> >  I have verified this with a bootstrap of a `powerpc64le-linux-gnu' native 
>> > compiler, and that it builds a `pdp11-aout' cross-compiler up to the point 
>> > of failing to find target headers.  I have no means to verify it further, 
>> > the target configuration is too exotic to me, so I will appreciate help.
>> >
>> >  NB this only triggers with insn-emit.c code produced from named RTL insns 
>> > for the purpose of calling them directly, which does not apply for the VAX 
>> > backend, as it does not give names to any insns using `const_double_zero'. 
>> >  
>> > Which is why I didn't spot this issue with all my VAX verification.
>> >
>> >  Thanks to Martin for bringing my attention to this regression, and sorry 
>> > to miss this in testing.
>> >
>> >  OK to apply?
>> Presumably you can't use CONST0_RTX (mode) here?  Assuming that's the
>> case, then this is fine.
>
>  Well, `mode' is VOID for simplicity and to match what the middle end 
> presents as an operand to the COMPARE operation, as also shown with the 
> diff above, so with CONST0_RTX (mode) we'd be back to what amounts to 
> `const0_rtx' aka `const_int 0', which is exactly what we want to get away 
> from.
>
>  Alternatively we could possibly give `const_double_zero' a proper FP 
> mode, but it seems to me like an unnecessary complication, as it would 
> then have to be individually requested and iterated over all the relevant 
> modes.

Generated FP const_double rtxes have to have a proper (non-VOID) mode
though.  VOIDmode CONST_DOUBLEs are always (legacy) integers instead
of FP values.

So yeah, giving const_double_zero a proper mode seems like the way to go.
It's technically possible to drop it in a pure matching context, but I'm
not sure how valuable that is in practice, given that other parts of the
matched pattern would usually need to refer to the same mode anyway.

>  Have I missed anything here?
>
>  NB the PDP-11 pieces affected here and tripping this assertion are I 
> believe dead code, as these insns are only ever produced by splitters and 
> do not appear to be referred by their names.  We need this change however 
> for completeness so that `const_double_zero' can be used in contexts where 
> an insn actually will be referred by its name.
>
>  However the PDP-11 code being dead makes it a bit more difficult to 
> examine actual consequences of such a change like I have proposed than it 
> would otherwise be.  In these circumstances I think my proposal is safe if 
> a bit overly cautious.

CONST_DOUBLE_ATOF ("0", VOIDmode) seems malformed though, and I'd expect
it to assert in REAL_MODE_FORMAT (via the format_helper constructor).
I'm not sure the patch is strictly safer than the status quo.

FWIW, I agree with Jeff that this ought to be CONST0_RTX (mode).

Thanks,
Richard


Re: [PATCH] genemit: Handle `const_double_zero' rtx

2020-12-16 Thread Maciej W. Rozycki
On Tue, 15 Dec 2020, Jeff Law wrote:

> > @@ -1942,7 +1942,7 @@ gen_divdf3_cc (rtx operand0 ATTRIBUTE_UN
> > gen_rtx_DIV (DFmode,
> > operand1,
> > operand2),
> > -   const0_rtx)),
> > +   CONST_DOUBLE_ATOF ("0", VOIDmode))),
> > gen_rtx_SET (operand0,
> > gen_rtx_DIV (DFmode,
> > copy_rtx (operand1),
> >
> > gcc/
> > * genemit.c (gen_exp) : Handle `const_double_zero' 
> > rtx.
> > ---
> > Hi,
> >
> >  I have verified this with a bootstrap of a `powerpc64le-linux-gnu' native 
> > compiler, and that it builds a `pdp11-aout' cross-compiler up to the point 
> > of failing to find target headers.  I have no means to verify it further, 
> > the target configuration is too exotic to me, so I will appreciate help.
> >
> >  NB this only triggers with insn-emit.c code produced from named RTL insns 
> > for the purpose of calling them directly, which does not apply for the VAX 
> > backend, as it does not give names to any insns using `const_double_zero'.  
> > Which is why I didn't spot this issue with all my VAX verification.
> >
> >  Thanks to Martin for bringing my attention to this regression, and sorry 
> > to miss this in testing.
> >
> >  OK to apply?
> Presumably you can't use CONST0_RTX (mode) here?  Assuming that's the
> case, then this is fine.

 Well, `mode' is VOID for simplicity and to match what the middle end 
presents as an operand to the COMPARE operation, as also shown with the 
diff above, so with CONST0_RTX (mode) we'd be back to what amounts to 
`const0_rtx' aka `const_int 0', which is exactly what we want to get away 
from.

 Alternatively we could possibly give `const_double_zero' a proper FP 
mode, but it seems to me like an unnecessary complication, as it would 
then have to be individually requested and iterated over all the relevant 
modes.

 Have I missed anything here?

 NB the PDP-11 pieces affected here and tripping this assertion are I 
believe dead code, as these insns are only ever produced by splitters and 
do not appear to be referred by their names.  We need this change however 
for completeness so that `const_double_zero' can be used in contexts where 
an insn actually will be referred by its name.

 However the PDP-11 code being dead makes it a bit more difficult to 
examine actual consequences of such a change like I have proposed than it 
would otherwise be.  In these circumstances I think my proposal is safe if 
a bit overly cautious.

  Maciej


Re: [PATCH] genemit: Handle `const_double_zero' rtx

2020-12-15 Thread Jeff Law via Gcc-patches



On 12/15/20 1:38 PM, Maciej W. Rozycki wrote:
> Complement commit 20ab43b5cad6 ("RTL: Add `const_double_zero' syntactic 
> rtx") and remove a commit c60d0736dff7 ("PDP11: Use `const_double_zero' 
> to express double zero constant") build regression observed with the 
> `pdp11-aout' target:
>
> genemit: Internal error: abort in gen_exp, at genemit.c:202
> make[2]: *** [Makefile:2427: s-emit] Error 1
>
> where a:
>
> (const_double 0 [0] 0 [0] 0 [0] 0 [0])
>
> rtx coming from:
>
> (parallel [
> (set (reg:CC 16)
> (compare:CC (abs:DF (match_operand:DF 1 ("general_operand") 
> ("0,0")))
> (const_double 0 [0] 0 [0] 0 [0] 0 [0])))
> (set (match_operand:DF 0 ("nonimmediate_operand") ("=fR,Q"))
> (abs:DF (match_dup 1)))
> ])
>
> and ultimately `(const_double_zero)' referred in a named RTL insn cannot 
> be interpreted.  Handle the rtx then by supplying the constant 0 double 
> operand requested, resulting in the following update to insn-emit.c code 
> produced for the `pdp11-aout' target, relative to before the triggering 
> commit:
>
> @@ -1514,7 +1514,7 @@ gen_absdf2_cc (rtx operand0 ATTRIBUTE_UN
>   gen_rtx_COMPARE (CCmode,
>   gen_rtx_ABS (DFmode,
>   operand1),
> - const0_rtx)),
> + CONST_DOUBLE_ATOF ("0", VOIDmode))),
>   gen_rtx_SET (operand0,
>   gen_rtx_ABS (DFmode,
>   copy_rtx (operand1);
> @@ -1555,7 +1555,7 @@ gen_negdf2_cc (rtx operand0 ATTRIBUTE_UN
>   gen_rtx_COMPARE (CCmode,
>   gen_rtx_NEG (DFmode,
>   operand1),
> - const0_rtx)),
> + CONST_DOUBLE_ATOF ("0", VOIDmode))),
>   gen_rtx_SET (operand0,
>   gen_rtx_NEG (DFmode,
>   copy_rtx (operand1);
> @@ -1790,7 +1790,7 @@ gen_muldf3_cc (rtx operand0 ATTRIBUTE_UN
>   gen_rtx_MULT (DFmode,
>   operand1,
>   operand2),
> - const0_rtx)),
> + CONST_DOUBLE_ATOF ("0", VOIDmode))),
>   gen_rtx_SET (operand0,
>   gen_rtx_MULT (DFmode,
>   copy_rtx (operand1),
> @@ -1942,7 +1942,7 @@ gen_divdf3_cc (rtx operand0 ATTRIBUTE_UN
>   gen_rtx_DIV (DFmode,
>   operand1,
>   operand2),
> - const0_rtx)),
> + CONST_DOUBLE_ATOF ("0", VOIDmode))),
>   gen_rtx_SET (operand0,
>   gen_rtx_DIV (DFmode,
>   copy_rtx (operand1),
>
>   gcc/
>   * genemit.c (gen_exp) : Handle `const_double_zero' 
>   rtx.
> ---
> Hi,
>
>  I have verified this with a bootstrap of a `powerpc64le-linux-gnu' native 
> compiler, and that it builds a `pdp11-aout' cross-compiler up to the point 
> of failing to find target headers.  I have no means to verify it further, 
> the target configuration is too exotic to me, so I will appreciate help.
>
>  NB this only triggers with insn-emit.c code produced from named RTL insns 
> for the purpose of calling them directly, which does not apply for the VAX 
> backend, as it does not give names to any insns using `const_double_zero'.  
> Which is why I didn't spot this issue with all my VAX verification.
>
>  Thanks to Martin for bringing my attention to this regression, and sorry 
> to miss this in testing.
>
>  OK to apply?
Presumably you can't use CONST0_RTX (mode) here?  Assuming that's the
case, then this is fine.

jeff



[PATCH] genemit: Handle `const_double_zero' rtx

2020-12-15 Thread Maciej W. Rozycki
Complement commit 20ab43b5cad6 ("RTL: Add `const_double_zero' syntactic 
rtx") and remove a commit c60d0736dff7 ("PDP11: Use `const_double_zero' 
to express double zero constant") build regression observed with the 
`pdp11-aout' target:

genemit: Internal error: abort in gen_exp, at genemit.c:202
make[2]: *** [Makefile:2427: s-emit] Error 1

where a:

(const_double 0 [0] 0 [0] 0 [0] 0 [0])

rtx coming from:

(parallel [
(set (reg:CC 16)
(compare:CC (abs:DF (match_operand:DF 1 ("general_operand") 
("0,0")))
(const_double 0 [0] 0 [0] 0 [0] 0 [0])))
(set (match_operand:DF 0 ("nonimmediate_operand") ("=fR,Q"))
(abs:DF (match_dup 1)))
])

and ultimately `(const_double_zero)' referred in a named RTL insn cannot 
be interpreted.  Handle the rtx then by supplying the constant 0 double 
operand requested, resulting in the following update to insn-emit.c code 
produced for the `pdp11-aout' target, relative to before the triggering 
commit:

@@ -1514,7 +1514,7 @@ gen_absdf2_cc (rtx operand0 ATTRIBUTE_UN
gen_rtx_COMPARE (CCmode,
gen_rtx_ABS (DFmode,
operand1),
-   const0_rtx)),
+   CONST_DOUBLE_ATOF ("0", VOIDmode))),
gen_rtx_SET (operand0,
gen_rtx_ABS (DFmode,
copy_rtx (operand1);
@@ -1555,7 +1555,7 @@ gen_negdf2_cc (rtx operand0 ATTRIBUTE_UN
gen_rtx_COMPARE (CCmode,
gen_rtx_NEG (DFmode,
operand1),
-   const0_rtx)),
+   CONST_DOUBLE_ATOF ("0", VOIDmode))),
gen_rtx_SET (operand0,
gen_rtx_NEG (DFmode,
copy_rtx (operand1);
@@ -1790,7 +1790,7 @@ gen_muldf3_cc (rtx operand0 ATTRIBUTE_UN
gen_rtx_MULT (DFmode,
operand1,
operand2),
-   const0_rtx)),
+   CONST_DOUBLE_ATOF ("0", VOIDmode))),
gen_rtx_SET (operand0,
gen_rtx_MULT (DFmode,
copy_rtx (operand1),
@@ -1942,7 +1942,7 @@ gen_divdf3_cc (rtx operand0 ATTRIBUTE_UN
gen_rtx_DIV (DFmode,
operand1,
operand2),
-   const0_rtx)),
+   CONST_DOUBLE_ATOF ("0", VOIDmode))),
gen_rtx_SET (operand0,
gen_rtx_DIV (DFmode,
copy_rtx (operand1),

gcc/
* genemit.c (gen_exp) : Handle `const_double_zero' 
rtx.
---
Hi,

 I have verified this with a bootstrap of a `powerpc64le-linux-gnu' native 
compiler, and that it builds a `pdp11-aout' cross-compiler up to the point 
of failing to find target headers.  I have no means to verify it further, 
the target configuration is too exotic to me, so I will appreciate help.

 NB this only triggers with insn-emit.c code produced from named RTL insns 
for the purpose of calling them directly, which does not apply for the VAX 
backend, as it does not give names to any insns using `const_double_zero'.  
Which is why I didn't spot this issue with all my VAX verification.

 Thanks to Martin for bringing my attention to this regression, and sorry 
to miss this in testing.

 OK to apply?

  Maciej
---
 gcc/genemit.c |8 
 1 file changed, 8 insertions(+)

gcc-genemit-const-double-zero.diff
Index: gcc/gcc/genemit.c
===
--- gcc.orig/gcc/genemit.c
+++ gcc/gcc/genemit.c
@@ -195,6 +195,14 @@ gen_exp (rtx x, enum rtx_code subroutine
   return;
 
 case CONST_DOUBLE:
+  /* Handle `const_double_zero' rtx.  */
+  if (CONST_DOUBLE_REAL_VALUE (x)->cl == rvc_zero)
+   {
+ printf ("CONST_DOUBLE_ATOF (\"0\", %smode)",
+ GET_MODE_NAME (GET_MODE (x)));
+ return;
+   }
+  /* Fall through.  */
 case CONST_FIXED:
 case CONST_WIDE_INT:
   /* These shouldn't be written in MD files.  Instead, the appropriate