On Tue, Jul 12, 2011 at 11:50 AM, Andrew Stubbs <a...@codesourcery.com> wrote:
> On 23/06/11 15:39, Andrew Stubbs wrote:
>>
>> This patch has two effects:
>>
>> 1. It permits the use of widening multiply instructions that widen by
>> more than one mode. E.g. HImode -> DImode.
>>
>> 2. It enables the use of widening multiply instructions for (extended)
>> inputs of narrower mode than the instruction takes. E.g. QImode ->
>> DImode where only HI->DI or SI->DI is available.
>>
>> Hopefully, most of the patch is self-explanatory, but here are few notes:
>>
>> The code introduces a temporary FIXME comment; this will be removed
>> later in the patch series. In fact, this is not a new restriction;
>> previously "type1" and "type2" were implicitly identical because they
>> were required to be one mode smaller than "type".
>>
>> I regard the ARM portion of this patch as obvious, so I don't think I
>> need an ARM maintainer to read this.
>>
>> Is the patch OK?
>
> I found a bug in this patch. It seems I do need to add casts for the inputs
> to widening multiplies (even though I know the registers are already fine),
> because otherwise something is insisting on truncating the values to the
> minimum width, which isn't helpful when it's actually an instruction with
> wider inputs.
>
> The mode changing bits from patch 4 have therefore been moved here. I've
> made the changes Richard Guenther requested there, I think.
>
> Otherwise, the patch is the same as before.
I wonder if we want to restrict the WIDEN_* operations to operate
on types that have matching type/mode precision(**). Consider
struct {
int a : 7;
int b : 7;
} x;
short c = x.a * x.b;
which will be represented as (short)((int)<7-bit-type-with-QImode> *
(int)<7-bit-type-with-QImode>).
I wonder if you can do some experiments with bitfield types and see
if your patch series handles them correctly.
As for the patch, please update tree.def with the new requirements
for the WIDEN_* codes.
As for the bitfield precisions, we probably want to reject types that
do not have TYPE_PRECISION (type) == GET_MODE_PRECISION (TYPE_MODE
(type)). Or maybe we can allow them if we generate
correct and good code for them?
+ tmp2 = TYPE_UNSIGNED (type1) == TYPE_UNSIGNED (type2)
+ ? tmp1
+ : create_tmp_var (
+ build_nonstandard_integer_type (
+ GET_MODE_PRECISION (from_mode), TYPE_UNSIGNED (type1)),
+ NULL);
please use an if () stmt to avoid gross formatting.
+ if (TYPE_MODE (type1) != from_mode)
these kind of checks are unsafe if type1 does not have a TYPE_PRECISION
equal to its mode precision.
Thanks,
Richard.
> Andrew
>
>
