Hi!
On Fri, Oct 30, 2020 at 09:36:13AM -0700, Carl Love wrote:
> On Wed, 2020-10-28 at 20:43 -0400, David Edelsohn wrote:
> > Better, but please use
> >
> > /* { dg-require-effective-target int128 } */
> >
> > not "target int128" in the selector. Segher and I both agree that
> > it's cleaner and more readable. The selector (the target part on the
> > dg-do line) should not be used for this type of requirement.
Yes.
> +(define_insn "*bcd<bcd_add_sub>_test_<mode>"
> [(set (reg:CCFP CR6_REGNO)
> (compare:CCFP
> - (unspec:V2DF [(match_operand:V1TI 1 "register_operand" "v")
> - (match_operand:V1TI 2 "register_operand" "v")
> + (unspec:V2DF [(match_operand:VBCD 1 "register_operand" "v")
> + (match_operand:VBCD 2 "register_operand" "v")
> (match_operand:QI 3 "const_0_to_1_operand" "i")]
This should be "n" instead of "i". This is existing code of course, but
please do that in the new code at least? (And changing "i" to "n" in
existing code wherever an assembly-time literal constant is needed is
pre-approved -- "i" allows relocations, "n" does not, essentially.)
> +(define_insn "dfp_denbcd_v16qi_inst"
> + [(set (match_operand:TD 0 "gpc_reg_operand" "=d")
> + (unspec:TD [(match_operand:QI 1 "const_0_to_1_operand" "i")
(like here)
Because the predicate here only allows actual numbers (const_ints), it
is quite hard to ever make the "i" go wrong, but it isn't impossible in
principle.
Other than that nit, yes this looks good. So okay for trunk, thanks!
Segher
