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
