On Fri, Aug 17, 2018 at 2:54 PM Alexander Monakov <amona...@ispras.ru> wrote: > > Hello, > > We currently have an unfortunate situation where, on the one hand, scalar > MULT_HIGHPART_EXPR is usable only if the backend provides the corresponding > pattern (otherwise ICEs in expand), but on the other hand, the BRIG frontend > wants to make use of it. > > I think BRIG FE is making assumptions on which types are going to work (based > on behavior of i386 backend), and this recently broke when the backend stopped > exposing SImode mult-highpart in 64-bit mode, causing PR 86948. > > For scalar integer modes it is easy enough to implement generic expansion via > widening multiply (although it still won't work for the widest mode). > > Do we want such functionality on trunk?
I think that all tree codes should expand correctly always and nowadays we should use IFNs for codes that restrict themselves to cases the backend can handle. Not sure how many we have left though ... So - how difficult is it to fix BRIG to not use MULT_HIGHPART_EXPR if not supported? Richard. > * expr.c (expand_expr_real_2) <case MULT_HIGHPART_EXPR>: Add generic > expansion via widening multiply and shift for integer modes. > * optabs.c (expand_mult_highpart): Receive 'method' from caller. > * optabs.h (expand_mult_highpart): Adjust prototype. > > diff --git a/gcc/expr.c b/gcc/expr.c > index de6709defd6..b3e33077b3d 100644 > --- a/gcc/expr.c > +++ b/gcc/expr.c > @@ -8913,10 +8913,23 @@ expand_expr_real_2 (sepops ops, rtx target, > machine_mode tmode, > goto binop; > > case MULT_HIGHPART_EXPR: > - expand_operands (treeop0, treeop1, subtarget, &op0, &op1, > EXPAND_NORMAL); > - temp = expand_mult_highpart (mode, op0, op1, target, unsignedp); > - gcc_assert (temp); > - return temp; > + { > + int method = can_mult_highpart_p (mode, unsignedp); > + if (!method) > + { > + gcc_assert (GET_MODE_CLASS (mode) == MODE_INT); > + machine_mode wmode = GET_MODE_2XWIDER_MODE (mode).require (); > + int prec = GET_MODE_UNIT_BITSIZE (mode); > + ops->code = WIDEN_MULT_EXPR; > + ops->type = build_nonstandard_integer_type (prec * 2, unsignedp); > + temp = expand_expr_real_2 (ops, NULL_RTX, wmode, EXPAND_NORMAL); > + temp = expand_shift (RSHIFT_EXPR, wmode, temp, prec, target, > + unsignedp); > + return convert_modes (mode, wmode, temp, unsignedp); > + } > + expand_operands (treeop0, treeop1, subtarget, &op0, &op1, > EXPAND_NORMAL); > + return expand_mult_highpart (mode, op0, op1, target, unsignedp, > method); > + } > > case FIXED_CONVERT_EXPR: > op0 = expand_normal (treeop0); > diff --git a/gcc/optabs.c b/gcc/optabs.c > index cadf4676c98..616d6f86720 100644 > --- a/gcc/optabs.c > +++ b/gcc/optabs.c > @@ -5871,20 +5871,17 @@ expand_vec_cmp_expr (tree type, tree exp, rtx target) > > rtx > expand_mult_highpart (machine_mode mode, rtx op0, rtx op1, > - rtx target, bool uns_p) > + rtx target, bool uns_p, int method) > { > struct expand_operand eops[3]; > enum insn_code icode; > - int method, i; > + int i; > machine_mode wmode; > rtx m1, m2; > optab tab1, tab2; > > - method = can_mult_highpart_p (mode, uns_p); > switch (method) > { > - case 0: > - return NULL_RTX; > case 1: > tab1 = uns_p ? umul_highpart_optab : smul_highpart_optab; > return expand_binop (mode, tab1, op0, op1, target, uns_p, > diff --git a/gcc/optabs.h b/gcc/optabs.h > index f9a8169daf8..ad78c4cbe76 100644 > --- a/gcc/optabs.h > +++ b/gcc/optabs.h > @@ -316,7 +316,7 @@ extern rtx expand_vec_cond_expr (tree, tree, tree, tree, > rtx); > extern rtx expand_vec_series_expr (machine_mode, rtx, rtx, rtx); > > /* Generate code for MULT_HIGHPART_EXPR. */ > -extern rtx expand_mult_highpart (machine_mode, rtx, rtx, rtx, bool); > +extern rtx expand_mult_highpart (machine_mode, rtx, rtx, rtx, bool, int); > > extern rtx expand_sync_lock_test_and_set (rtx, rtx, rtx); > extern rtx expand_atomic_test_and_set (rtx, rtx, enum memmodel);