On Fri, Sep 7, 2012 at 10:35 AM, Richard Earnshaw <rearn...@arm.com> wrote: > On 20/08/12 12:36, Richard Guenther wrote: >> On Fri, Aug 17, 2012 at 7:05 PM, Richard Earnshaw <rearn...@arm.com> wrote: >>> On 17/08/12 16:20, Richard Earnshaw wrote: >>>> Ok, in which case we have to give is_widening_mult_rhs_p enough smarts >>>> to not strip >>>> >>>> (s32)u32 >>>> >>>> and return u32. >>>> >>>> I'll have another think about it. >>> >>> Take two. >>> >>> This version should address your concerns about handling >>> >>> (u32)u16 * (u32)u16 -> u64 >>> >>> We now look at each operand directly, but when doing so we check whether >>> the operand is the same size as the result or not. When it is, we can >>> strip any conversion; when it isn't the conversion must preserve >>> signedness of the inner operand and mustn't be a narrowing conversion. >>> >>> * tree-ssa-math-opts.c (widening_mult_conversion_strippable_p): >>> New function. >>> (is_widening_mult_rhs_p): Use it. >>> >>> Testing underway (again) >>> >>> OK? >> >> Ok. >> >> Thanks, >> Richard. >> >>> R. >>> >> > > > It turns out that this patch caused a few tests in gcc.target/arm to > start failing. Investigating has shown that there are a number of > reasons for this. > > One test (gcc.target/arm/pr50318-1.c) is just bogus. It's scanning for > an unsigned widening multiply when the correct operation is a signed > widening multiply. Fixing the compiler has just exposed the broken test. > > Two tests (gcc.target/arm/smlaltb-1.c and gcc.target/arm/smlaltt-1.c) > fail because the compiler now generates a subreg in the middle of the > widening multiply expression. Our patterns don't recognize this form > and I'm really not keen on the compiler doing this sort of thing anyway, > subreg operations of this nature are endian dependent and dealing with > this is messy. For now I'm going to xfail these two tests. > > The final two failures (gcc.target/arm/wmul-5.c and > gcc.target/arm/wmul-6.c) really should pass. They don't because my > first iteration of the patch failed to spot that (sign_extend:DI > (zero_extend:SI (reg:HI))) can be simplified legitimately to > (zero_extend:DI (reg:HI)). The patch below to > widening_mult_conversion_strippable_p fixes this. > > So this is a clean-up patch to fix these issues. > > * tree-ssa-math-opts.c (widening_mult_conversion_strippable_p): > Sign-extension of a zero-extended value can be simplified to > just zero-extension. > > testsuite: > > * gcc.target/arm/pr50318-1.c: Scan for smlal. > * gcc.target/arm/smlaltb-1.c: XFAIL test. > * gcc.target/arm/smlaltt-1.c: Likewise. > > OK?
Ok. Thanks, Richard.
