[25/31] VAX: Fix predicates for widening multiply and multiply-add insns

It makes no sense for insn operand predicates, as long as they accept a
register operand, to be more restrictive than the set of the associated
constraints, because expand will choose the insn based on the relevant
operand being a pseudo register then and reload will keep it happily as
an immediate if a constraint permits it.  So the restriction posed by
such a predicate will be happily ignored, and moreover if a splitter is
added, such as required for MODE_CC support, the new instructions will
reject the original operands supplied, causing an ICE like below:

.../gcc/testsuite/gfortran.dg/graphite/PR67518.f90:44:0: Error: could not split insn
(insn 90 662 663 (set (reg:DI 10 %r10 [orig:97 _235 ] [97])
        (mult:DI (sign_extend:DI (mem/c:SI (plus:SI (reg/f:SI 13 %fp)
                        (const_int -800 [0xfffffffffffffce0])) [14 %sfp+-800 S4 A32]))
            (sign_extend:DI (const_int -51 [0xffffffffffffffcd])))) 299 {mulsidi3}
     (expr_list:REG_EQUAL (mult:DI (sign_extend:DI (subreg:SI (mem/c:DI (plus:SI (reg/f:SI 13 %fp)
                            (const_int -800 [0xfffffffffffffce0])) [14 %sfp+-800 S8 A32]) 0))
            (const_int -51 [0xffffffffffffffcd]))
        (nil)))
during RTL pass: final
.../gcc/testsuite/gfortran.dg/graphite/PR67518.f90:44:0: internal compiler error: in final_scan_insn_1, at final.c:3073
Please submit a full bug report,
with preprocessed source if appropriate.
See <https://gcc.gnu.org/bugs/> for instructions.

Change the predicates used with the widening multiply and multiply-add
insns to allow immediates then, just as the constraints and the machine
instructions produced permit.

Also give the insns names, for easier reference here and elsewhere.

	gcc/
	* config/vax/vax.md (mulsidi3): Fix the multiplicand predicates.
	(*maddsidi4, *maddsidi4_const): Likewise.  Name insns.
---
 gcc/config/vax/vax.md | 31 ++++++++++++++-----------------
 1 file changed, 14 insertions(+), 17 deletions(-)

On 11/19/20 8:36 PM, Maciej W. Rozycki wrote:
> It makes no sense for insn operand predicates, as long as they accept a
> register operand, to be more restrictive than the set of the associated
> constraints, because expand will choose the insn based on the relevant
> operand being a pseudo register then and reload will keep it happily as
> an immediate if a constraint permits it.  So the restriction posed by
> such a predicate will be happily ignored, and moreover if a splitter is
> added, such as required for MODE_CC support, the new instructions will
> reject the original operands supplied, causing an ICE like below:
>
> .../gcc/testsuite/gfortran.dg/graphite/PR67518.f90:44:0: Error: could not split insn
> (insn 90 662 663 (set (reg:DI 10 %r10 [orig:97 _235 ] [97])
>         (mult:DI (sign_extend:DI (mem/c:SI (plus:SI (reg/f:SI 13 %fp)
>                         (const_int -800 [0xfffffffffffffce0])) [14 %sfp+-800 S4 A32]))
>             (sign_extend:DI (const_int -51 [0xffffffffffffffcd])))) 299 {mulsidi3}
>      (expr_list:REG_EQUAL (mult:DI (sign_extend:DI (subreg:SI (mem/c:DI (plus:SI (reg/f:SI 13 %fp)
>                             (const_int -800 [0xfffffffffffffce0])) [14 %sfp+-800 S8 A32]) 0))
>             (const_int -51 [0xffffffffffffffcd]))
>         (nil)))
> during RTL pass: final
> .../gcc/testsuite/gfortran.dg/graphite/PR67518.f90:44:0: internal compiler error: in final_scan_insn_1, at final.c:3073
> Please submit a full bug report,
> with preprocessed source if appropriate.
> See <https://gcc.gnu.org/bugs/> for instructions.
>
> Change the predicates used with the widening multiply and multiply-add
> insns to allow immediates then, just as the constraints and the machine
> instructions produced permit.
>
> Also give the insns names, for easier reference here and elsewhere.
>
> 	gcc/
> 	* config/vax/vax.md (mulsidi3): Fix the multiplicand predicates.
> 	(*maddsidi4, *maddsidi4_const): Likewise.  Name insns.
OK
jeff

ps.  Yes, I skipped the insv/extv changes.  They're usually a rats nest
of special cases.  We'll come back to them.

On Fri, 20 Nov 2020, Jeff Law wrote:

> ps.  Yes, I skipped the insv/extv changes.  They're usually a rats nest
> of special cases.  We'll come back to them.

 I've thought of actually reducing the number of patterns to the minimum 
possible by folding the existing ones together, and then getting the 
alternatives sorted by fine-grained constraints at reload.

 There is that complication caused by INSV machine instruction preserving 
condition codes (understandably), so keeping it together with alternative 
code sequences that do set the codes in a single RTL insn would cause 
trouble with getting a matching insn for the comparison elimination pass.  
Or so I think.

 As you say, we can sort it later.  It's an optimisation only anyway, the 
VAX EXTV/EXTZV/INSV machine instructions are flexible enough to handle 
just about anything, except that reportedly at a slow pace, at least with 
some implementations (I'd have to review that NVAX manual for the details, 
but it's hefty 1022 pages).  So at worst we could use those instructions 
in all cases.

  Maciej

On 11/30/20 9:02 AM, Maciej W. Rozycki wrote:
> On Fri, 20 Nov 2020, Jeff Law wrote:
>
>> ps.  Yes, I skipped the insv/extv changes.  They're usually a rats nest
>> of special cases.  We'll come back to them.
>  I've thought of actually reducing the number of patterns to the minimum 
> possible by folding the existing ones together, and then getting the 
> alternatives sorted by fine-grained constraints at reload.
That might work, but I've generally found insv/extv to be fairly painful
through the years, more because of limitations on the generic bits
rather than the target bits.

>
>  There is that complication caused by INSV machine instruction preserving 
> condition codes (understandably), so keeping it together with alternative 
> code sequences that do set the codes in a single RTL insn would cause 
> trouble with getting a matching insn for the comparison elimination pass.  
> Or so I think.
Yea.  I think that is (in general) not a well solved problem.  We have
similar issues on the H8 where we have multiple ways to implement
certain operations -- some of which set/clobber flags while others don't
touch them.  Depending on the context one form may be preferable to the
other.  I've punted this problem so far as I strongly suspect the gains
in handling this scenario well are marginal at best.

Jeff