On 25 November 2015 at 11:36, Kyrill Tkachov <kyrylo.tkac...@arm.com> wrote: > Hi Yvan, > > > On 24/11/15 15:05, Yvan Roux wrote: >> >> Hi Kyrill, >> >> On 24 November 2015 at 14:31, Kyrill Tkachov <kyrylo.tkac...@arm.com> >> wrote: >>> >>> Ping. >>> >>> Thanks, >>> Kyrill >>> >>> >>> >>> On 13/11/15 11:50, Kyrill Tkachov wrote: >>>> >>>> Ping. >>>> https://gcc.gnu.org/ml/gcc-patches/2015-11/msg00686.html >>>> >>>> Thanks, >>>> Kyrill >>>> >>>> On 06/11/15 17:05, Kyrill Tkachov wrote: >>>>> >>>>> Hi all, >>>>> >>>>> This patch introduces support for the smmul, smmla and smmls >>>>> instructions >>>>> that appear in armv6 architecture levels >>>>> and higher. To quote the SMMUL description from the ARMARM: >>>>> "Signed Most Significant Word Multiply multiplies two signed 32-bit >>>>> values, extracts the most significant 32 bits of >>>>> the result, and writes those bits to the destination register." >>>>> >>>>> The smmla and smmls are the multiply-accumulate and multiply-subtract >>>>> extensions of that multiply. >>>>> There also exists an smmulr variant that rounds the result rather than >>>>> truncating it. >>>>> However, when I tried adding patterns for those forms I got an LRA ICE >>>>> that I was not able to figure out. >>>>> I'll try to find a testcase for that, but in the meantime there's no >>>>> reason to not have patterns for the >>>>> non-rounding variants. >> >> I agree, but would be interested to give a look at the LRA issue when >> you have a testcase. > > > I've filed PR 68536 for the ICE I've been seeing. Any insight is, of course, > welcome.
Ok great, I'll give a look. >>>>> Bootstrapped and tested on arm-none-linux-gnueabihf. >>>>> I've seen this trigger in quite a few places in SPEC2006 where it >>>>> always >>>>> made the code better. >>>>> >>>>> Ok for trunk? >> >> Patch looks good to me (but can't approved it), I just wonder if >> adding a subreg_highpart_p predicate in emit-rtl.c would be useful, >> looking at other usage of subreg_highpart_offset it doesn't seem to be >> the case, but maybe for consistency with the existing >> subreg_lowpart_p. > > > Perhaps, we can put it there if there's much scope for its reuse in other > parts of the compiler. I didn't put it there in the first place so as not to > extend the review scope of the patch to the midend... Yes it's what I thought, and I don't see other usage. Now on AArch64 I see that the pattern used to generate smulh and umulh (<su>muldi3_highpart), which are doing the same kind of operation in the 64bit world, is doing it in a different manner (lshift and truncate) and I'm not sure which approach is the best here. Cheers, Yvan > Thanks for looking at this, > Kyrill > > >> >> Cheers, >> Yvan >> >>>>> Thanks, >>>>> Kyrill >>>>> >>>>> 2015-11-06 Kyrylo Tkachov <kyrylo.tkac...@arm.com> >>>>> >>>>> PR target/49526 >>>>> * config/arm/arm.md (*mulsidi3si_v6): New pattern. >>>>> (*mulsidi3siaddsi_v6): Likewise. >>>>> (*mulsidi3sisubsi_v6): Likewise. >>>>> * config/arm/predicates.md (subreg_highpart_operator): >>>>> New predicate. >>>>> >>>>> 2015-11-06 Kyrylo Tkachov <kyrylo.tkac...@arm.com> >>>>> >>>>> PR target/49526 >>>>> * gcc.target/arm/pr49526_1.c: New test. >>>>> * gcc.target/arm/pr49526_2.c: Likewise. >>>>> * gcc.target/arm/pr49526_3.c: Likewise. >>>> >>>> >
