Message ID | 5BFC2F2B.3050102@foss.arm.com |
---|---|
State | New |
Headers | show |
Series | [arm,1/3] Rename mul64 attr to widen_mul64 | expand |
On Mon, Nov 26, 2018 at 11:36:43AM -0600, Kyrill Tkachov wrote: > Hi all, > > In the AAarch64 ISA the MUL and MNEG instructions are actually aliases of MADD and MSUB. > Therefore they should have the type attribute mla, rather than mul, which should only be used > for AArch32 32-bit multiplication instructions. > > This will ensure more consistent scheduling decisions. > > Bootstrapped and tested on aarch64-none-linux-gnu. > > Ok for trunk? OK in principle. Did you audit the pipeline models to check this doesn't change scheduling class in an undesirable way for any of our supported targets? OK if so, if not can you run that audit and figure out the right thing to do to resolve it. Thanks, James
On 28/11/18 16:37, James Greenhalgh wrote: > On Mon, Nov 26, 2018 at 11:36:43AM -0600, Kyrill Tkachov wrote: >> Hi all, >> >> In the AAarch64 ISA the MUL and MNEG instructions are actually aliases of MADD and MSUB. >> Therefore they should have the type attribute mla, rather than mul, which should only be used >> for AArch32 32-bit multiplication instructions. >> >> This will ensure more consistent scheduling decisions. >> >> Bootstrapped and tested on aarch64-none-linux-gnu. >> >> Ok for trunk? > OK in principle. Did you audit the pipeline models to check this doesn't > change scheduling class in an undesirable way for any of our supported > targets? OK if so, if not can you run that audit and figure out the right > thing to do to resolve it. This has a change in 3 models. In saphira.md this changes the reservation of these instructions from saphira_muldiv_4_x_mul to saphira_muldiv_4_x_mla. Both have the same characteristics (latency etc) but they differ in their bypass behaviour: saphira_muldiv_4_x_mla can receive a bypass from saphira_muldiv_4_x_mul, but not the other way around. So with this change the MUL and MNEG will now be able to receive an early forwarded result from other multiply/multiply-accumulate operations, whereas before this was only done on MADD instructions. There is a similar situation in falkor.md. Sameera, can you share whether this new behaviour is appropriate for Falkor and Saphira? I note that if the forwarding only occurs on the accumulator operand, the bypass should be guarded on aarch_accumulator_forwarding In thunderx2t99.md the reservation changes from thunderx2t99_mul to thunderx2t99_madd. Steve, can you share whether the AArch64 MUL and MNEG instructions really do have different latencies and reservations from MADD and MSUB on Thunderx2? If so, then this change is wrong :( and we'll want to model these forms differently indeed. Thanks, Kyrill > Thanks, > James >
On Fri, 2018-11-30 at 15:37 +0000, Kyrill Tkachov wrote: > > In thunderx2t99.md the reservation changes from thunderx2t99_mul to > thunderx2t99_madd. > > Steve, can you share whether the AArch64 MUL and MNEG instructions > really do have different latencies and reservations from MADD and > MSUB > on Thunderx2? If so, then this change is wrong :( and we'll want to > model these forms differently indeed. > > Thanks, > Kyrill According to the thunderx2 documents I looked at, the mul/mneg instructions do have the same latencies as madd/msub. So this patch is OK from that standpoint and fixes an existing problem on Thunderx2. Steve Ellcey
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md index a3ccbbe791f94ff08d114af81e2b870919242458..73559b52ac24c58a8e23a297eac6d9a58b37b8fe 100644 --- a/gcc/config/aarch64/aarch64.md +++ b/gcc/config/aarch64/aarch64.md @@ -3385,7 +3385,7 @@ (define_insn "mul<mode>3" (match_operand:GPI 2 "register_operand" "r")))] "" "mul\\t%<w>0, %<w>1, %<w>2" - [(set_attr "type" "mul")] + [(set_attr "type" "mla")] ) ;; zero_extend version of above @@ -3396,7 +3396,7 @@ (define_insn "*mulsi3_uxtw" (match_operand:SI 2 "register_operand" "r"))))] "" "mul\\t%w0, %w1, %w2" - [(set_attr "type" "mul")] + [(set_attr "type" "mla")] ) (define_insn "madd<mode>" @@ -3452,7 +3452,7 @@ (define_insn "*mul<mode>_neg" "" "mneg\\t%<w>0, %<w>1, %<w>2" - [(set_attr "type" "mul")] + [(set_attr "type" "mla")] ) ;; zero_extend version of above @@ -3464,7 +3464,7 @@ (define_insn "*mulsi_neg_uxtw" "" "mneg\\t%w0, %w1, %w2" - [(set_attr "type" "mul")] + [(set_attr "type" "mla")] ) (define_insn "<su_optab>mulsidi3"