Message ID | alpine.LFD.2.21.2011200258180.656242@eddie.linux-mips.org |
---|---|
State | Accepted |
Headers | show |
Series | VAX: Bring the port up to date (yes, MODE_CC conversion is included) | expand |
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
diff --git a/gcc/config/vax/vax.md b/gcc/config/vax/vax.md index 34fdf67bb6d..2f6643abe5c 100644 --- a/gcc/config/vax/vax.md +++ b/gcc/config/vax/vax.md @@ -445,35 +445,32 @@ (define_insn "mul<mode>3" (define_insn "mulsidi3" [(set (match_operand:DI 0 "nonimmediate_operand" "=g") - (mult:DI (sign_extend:DI - (match_operand:SI 1 "nonimmediate_operand" "nrmT")) - (sign_extend:DI - (match_operand:SI 2 "nonimmediate_operand" "nrmT"))))] + (mult:DI + (sign_extend:DI (match_operand:SI 1 "general_operand" "nrmT")) + (sign_extend:DI (match_operand:SI 2 "general_operand" "nrmT"))))] "" "emul %1,%2,$0,%0") -(define_insn "" +(define_insn "*maddsidi4" [(set (match_operand:DI 0 "nonimmediate_operand" "=g") (plus:DI - (mult:DI (sign_extend:DI - (match_operand:SI 1 "nonimmediate_operand" "nrmT")) - (sign_extend:DI - (match_operand:SI 2 "nonimmediate_operand" "nrmT"))) - (sign_extend:DI (match_operand:SI 3 "nonimmediate_operand" "g"))))] + (mult:DI + (sign_extend:DI (match_operand:SI 1 "general_operand" "nrmT")) + (sign_extend:DI (match_operand:SI 2 "general_operand" "nrmT"))) + (sign_extend:DI (match_operand:SI 3 "general_operand" "g"))))] "" "emul %1,%2,%3,%0") ;; 'F' constraint means type CONST_DOUBLE -(define_insn "" +(define_insn "*maddsidi4_const" [(set (match_operand:DI 0 "nonimmediate_operand" "=g") (plus:DI - (mult:DI (sign_extend:DI - (match_operand:SI 1 "nonimmediate_operand" "nrmT")) - (sign_extend:DI - (match_operand:SI 2 "nonimmediate_operand" "nrmT"))) - (match_operand:DI 3 "immediate_operand" "F")))] + (mult:DI + (sign_extend:DI (match_operand:SI 1 "general_operand" "nrmT")) + (sign_extend:DI (match_operand:SI 2 "general_operand" "nrmT"))) + (match_operand:DI 3 "immediate_operand" "F")))] "GET_CODE (operands[3]) == CONST_DOUBLE - && CONST_DOUBLE_HIGH (operands[3]) == (CONST_DOUBLE_LOW (operands[3]) >> 31)" + && CONST_DOUBLE_HIGH (operands[3]) == (CONST_DOUBLE_LOW (operands[3]) >> 31)" "* { if (CONST_DOUBLE_HIGH (operands[3]))