On Tue, May 10, 2016 at 5:41 PM, Ilya Enkovich <enkovich....@gmail.com> wrote:
> On 02 May 10:50, Uros Bizjak wrote:
>> On Fri, Apr 29, 2016 at 5:42 PM, Ilya Enkovich <enkovich....@gmail.com>
>> wrote:
>> > Hi,
>> >
>> > As the first part of PR70799 fix I'd like to add constants support for
>> > DI-STV pass (which is also related to PR70763). This patch adds CONST_INT
>> > support as insn operands and extends cost model accordingly.
>> >
>> > Bootstrapped and regtested on x86_64-unknown-linux-gnu{-m32}. OK for
>> > trunk?
>> >
>> > Thanks,
>> > Ilya
>> > --
>> > gcc/
>> >
>> > 2016-04-29 Ilya Enkovich <ilya.enkov...@intel.com>
>> >
>> > PR target/70799
>> > PR target/70763
>> > * config/i386/i386.c (dimode_scalar_to_vector_candidate_p): Allow
>> > integer constants.
>> > (dimode_scalar_chain::vector_const_cost): New.
>> > (dimode_scalar_chain::compute_convert_gain): Handle constants.
>> > (dimode_scalar_chain::convert_op): Likewise.
>> > (dimode_scalar_chain::convert_insn): Likewise.
>> >
>> > gcc/testsuite/
>> >
>> > 2016-04-29 Ilya Enkovich <ilya.enkov...@intel.com>
>> >
>> > * gcc.target/i386/pr70799-1.c: New test.
>> >> @@ -3639,6 +3675,22 @@ dimode_scalar_chain::convert_op (rtx *op, rtx_insn
>> >> *insn)
>> > }
>> > *op = gen_rtx_SUBREG (V2DImode, *op, 0);
>> > }
>> > + else if (CONST_INT_P (*op))
>> > + {
>> > + rtx cst = const0_rtx;
>> > + rtx tmp = gen_rtx_SUBREG (V2DImode, gen_reg_rtx (DImode), 0);
>> > +
>> > + /* Prefer all ones vector in case of -1. */
>> > + if (constm1_operand (*op, GET_MODE (*op)))
>> > + cst = *op;
>> > + cst = gen_rtx_CONST_VECTOR (V2DImode, gen_rtvec (2, *op, cst));
>>
>> It took me a while to decipher the above functionality ;)
>>
>> Why not just:
>>
>> else if (CONST_INT_P (*op))
>> {
>> rtx tmp = gen_rtx_SUBREG (V2DImode, gen_reg_rtx (DImode), 0);
>> rtx vec;
>>
>> /* Prefer all ones vector in case of -1. */
>> if (constm1_operand (*op, GET_MODE (*op)))
>> vec = CONSTM1_RTX (V2DImode);
>> else
>> vec = gen_rtx_CONST_VECTOR (V2DImode,
>> gen_rtvec (2, *op, const0_rtx));
>>
>> if (!standard_sse_constant_p (vec, V2DImode))
>> vec = validize_mem (force_const_mem (V2DImode, vec));
>>
>> emit_insn_before (gen_move_insn (tmp, vec), insn);
>> *op = tmp;
>> }
>
> Agree. This is more readable.
>
>>
>> Comparing this part to timode_scalar_chain::convert_insn, there is a
>> NONDEBUG_INSN_P check. Do you need one in the above code? Also, TImode
>> pass handles REG_EQUAL and REG_EQUIV notes. Does dimode pass also need
>> this functionality?
>
> timode_scalar_chain conversion code is just simpler because of restricted
> external dependencies for a chain and everything is processed in convert_insn.
> For dimode_scalar_chain NONDEBUG_INSN_P is checked in convert_reg code.
> Also I don't think I need anything to do with notes because I don't make
> PUT_MODE for register. Register mode and register value are unchanged and
> therefore both debug insns and notes may be skipped.
>
>>
>> Uros.
>
> Here is an updated version. Bootstrap and regression testing is in progress.
> OK for trunk if testing pass?
>
> Thanks,
> Ilya
> --
> gcc/
>
> 2016-05-05 Ilya Enkovich <ilya.enkov...@intel.com>
>
> * config/i386/i386.c (dimode_scalar_to_vector_candidate_p): Allow
> integer constants.
> (dimode_scalar_chain::vector_const_cost): New.
> (dimode_scalar_chain::compute_convert_gain): Handle constants.
> (dimode_scalar_chain::convert_op): Likewise.
> (dimode_scalar_chain::convert_insn): Likewise.
>
> gcc/testsuite/
>
> 2016-05-05 Ilya Enkovich <ilya.enkov...@intel.com>
>
> * gcc.target/i386/pr70799-1.c: New test.
OK for mainline.
Thanks,
Uros.
