On Mon, Jul 4, 2011 at 4:23 PM, Andrew Stubbs <a...@codesourcery.com> wrote:
> On 01/07/11 13:25, Richard Guenther wrote:
>>
>> Well - some operations work the same on both signedness if you
>> just care about the twos-complement result. This includes
>> multiplication (but not for example division). For this special
>> case I suggest to not bother trying to invent a generic predicate
>> but do something local in tree-ssa-math-opts.c.
>
> OK, here's my updated patch.
>
> I've taken the view that we *know* what size and signedness the result of
> the multiplication is, and we know what size the input to the addition must
> be, so all the check has to do is make sure it does that same conversion,
> even if by a roundabout means.
>
> What I hadn't grasped before is that when extending a value it's the source
> type that is significant, not the destination, so the checks are not as
> complex as I had thought.
>
> So, this patch adds a test to ensure that:
>
> 1. the type is not truncated so far that we lose any information; and
>
> 2. the type is only ever extended in the proper signedness.
>
> Also, just to be absolutely sure, I've also added a little bit of logic to
> permit extends that are then undone by a truncate. I'm really not sure what
> guarantees there are about what sort of cast sequences can exist? Is this
> necessary? I haven't managed to coax it to generated any examples of extends
> followed by truncates myself, but in any case, it's hardly any code and
> it'll make sure it's future proofed.
>
> OK?
I think you should assume that series of widenings, (int)(short)char_variable
are already combined. Thus I believe you only need to consider a single
conversion in valid_types_for_madd_p.
+/* Check the input types, TYPE1 and TYPE2 to a widening multiply,
what are those types? Is TYPE1 the result type and TYPE2 the
operand type? If so why
+ initial_bitsize = TYPE_PRECISION (type1) + TYPE_PRECISION (type2);
this?!
+ initial_unsigned = TYPE_UNSIGNED (type1) && TYPE_UNSIGNED (type2);
that also looks odd. So probably TYPE1 isn't the result type. If they
are the types of the operands, then what operand is EXPR for?
I didn't look at the actual implementation of the function because of the
lack of understanding of the inputs.
- if (TREE_CODE (rhs1) == SSA_NAME)
+ for (tmp = rhs1, rhs1_code = ERROR_MARK;
+ TREE_CODE (tmp) == SSA_NAME
+ && (CONVERT_EXPR_CODE_P (rhs1_code) || rhs1_code == ERROR_MARK);
+ tmp = gimple_assign_rhs1 (rhs1_stmt))
{
- rhs1_stmt = SSA_NAME_DEF_STMT (rhs1);
- if (is_gimple_assign (rhs1_stmt))
- rhs1_code = gimple_assign_rhs_code (rhs1_stmt);
+ rhs1_stmt = SSA_NAME_DEF_STMT (tmp);
+ if (!is_gimple_assign (rhs1_stmt))
+ break;
+ rhs1_code = gimple_assign_rhs_code (rhs1_stmt);
}
the result looks a bit like spaghetti code ... and lacks a comment
on what it is trying to do. It looks like it sees through an arbitrary
number of conversions - possibly ones that will make the
macc invalid, as for (short)int-var * short-var + int-var. So you'll
be pessimizing code by doing that unconditionally. As I said
above you should at most consider one intermediate conversion.
I believe the code should be arranged such that only valid
conversions are looked through in the first place. Valid, in
that the resulting types should still match the macc constraints.
Richard.
> Andrew
>
