This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Patch: Consistently generate widening multiplies


On Wed, Apr 7, 2010 at 2:40 PM, Bernd Schmidt <bernds@codesourcery.com> wrote:
>> As I said earlier, please add a hunk to tree-cfg.c
>> verify_gimple_assign_binary, the verifier is essentially documentation
>> of gimple.
>
> Will do.
>
>>
>> Further comments inline ...
>
> Thanks for reviewing.
>
>>> + ? ? ? ? if (rhs1_stmt != NULL && gimple_bb (rhs1_stmt) != gimple_bb (stmt))
>>> + ? ? ? ? ? {
>>> + ? ? ? ? ? ? gimple_stmt_iterator other_gsi = gsi_for_stmt (rhs1_stmt);
>>> + ? ? ? ? ? ? gimple tmp_stmt;
>>> + ? ? ? ? ? ? tree tmpvar1;
>>> +
>>> + ? ? ? ? ? ? tmpvar1 = create_tmp_var (TREE_TYPE (rhs1_convop), "wmulsrc");
>>> + ? ? ? ? ? ? add_referenced_var (tmpvar1);
>>> + ? ? ? ? ? ? tmpvar1 = make_ssa_name (tmpvar1, NULL);
>>> +
>>> + ? ? ? ? ? ? tmp_stmt = gimple_build_assign (tmpvar1, rhs1_convop);
>>> + ? ? ? ? ? ? gsi_insert_before (&other_gsi, tmp_stmt, GSI_SAME_STMT);
>>> + ? ? ? ? ? ? rhs1_convop = tmpvar1;
>>> + ? ? ? ? ? }
>>
>> What's this code for?
>
> I was slightly worried that if I just used the argument of the
> CONVERT_EXPR, using it in the multiply might give me a different value
> in the presence of loops and conditions. ?So I insert a copy in the same
> place as the conversion which I know will give me the correct value.
> Something like the following is what I'm worried about:
>
> ?int a = 5, b = 3;
> ?short xyz;
> ?for (i = 0; i < 5; i++)
> ? ?{
> ? ? ? xyz = foo ();
> ? ? ? if (i == 3)
> ? ? ? ? a = (int)xyz;
> ? ?}
> ?...
> ?c = a * b;
>
> That turns into
>
> ?int a = 5, b = 3;
> ?short xyz, copy;
> ?for (i = 0; i < 5; i++)
> ? ?{
> ? ? ? xyz = foo ();
> ? ? ? if (i == 3)
> ? ? ? ? {
> ? ? ? ? ? copy = xyz;
> ? ? ? ? ? a = (int)xyz;
> ? ? ? ? }
> ? ?}
> ?...
> ?c = (int)copy * b;
>
> rather than using xyz in the multiply.

Ah, but you are in SSA form, so there's nothing bad that can happen.
Thus you can simply delete the two blocks.

>> I think that with multiplies by constants you are creating bogously
>> typed WIDEN_MULT_EXPRs as you are not adjusting the type
>> of the constant operands.
>
> There is a test for int_fits_type_p to ensure the constants can fit in
> the type of the non-constant operand. ?That's verified in the expander
> later. ?Possibly I'm too used to untyped RTL constants - should I just
> create a new integer with fold_convert?

Yes.

>> Can you rename the pass to something more generic? ?I'd like
>> to amend it to also detect widening addition (add with carry)
>> and fused multiply-add. ?So it should be sort-of a tree pre-expand
>> pass, or a tree pattern matcher. ?No, I don't have a suitable name ...
>
> tree-combine? ;-) ?Or maybe "math-propagate"? ?"arith-prop"?

"math-fuse"?  Well, I guess as you didn't add any user-visible
flags we can rename it when more functionality gets in.

Thanks,
Richard.

>
> Bernd
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]