"Maciej W. Rozycki" <ma...@linux-mips.org> 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) <CONST_DOUBLE>: 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
