From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30673 invoked by alias); 27 Aug 2015 13:14:08 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 30660 invoked by uid 89); 27 Aug 2015 13:14:07 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.8 required=5.0 tests=AWL,BAYES_40,KAM_ASCII_DIVIDERS,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=no version=3.3.2 X-HELO: APC01-SG2-obe.outbound.protection.outlook.com Received: from mail-sg2apc01on0041.outbound.protection.outlook.com (HELO APC01-SG2-obe.outbound.protection.outlook.com) (104.47.125.41) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA256 encrypted) ESMTPS; Thu, 27 Aug 2015 13:14:05 +0000 Received: from HK2PR03MB1380.apcprd03.prod.outlook.com (10.165.56.142) by HK2PR03MB1378.apcprd03.prod.outlook.com (10.165.56.140) with Microsoft SMTP Server (TLS) id 15.1.243.23; Thu, 27 Aug 2015 13:13:57 +0000 Received: from HK2PR03MB1380.apcprd03.prod.outlook.com ([10.165.56.142]) by HK2PR03MB1380.apcprd03.prod.outlook.com ([10.165.56.142]) with mapi id 15.01.0243.024; Thu, 27 Aug 2015 13:13:57 +0000 From: Kaushik Phatak To: DJ Delorie CC: "'gcc-patches@gcc.gnu.org'" , "nick clifton (nickc@redhat.com)" Subject: RE: [PATCH : RL78] Disable interrupts during hardware multiplication routines Date: Thu, 27 Aug 2015 13:14:00 -0000 Message-ID: References: <201506050644.t556igDi027743@greed.delorie.com> In-Reply-To: <201506050644.t556igDi027743@greed.delorie.com> authentication-results: spf=none (sender IP is ) smtp.mailfrom=Kaushik.Phatak@kpit.com; x-microsoft-exchange-diagnostics: 1;HK2PR03MB1378;5:c6Mpf5X7IbtLQNJG+Ffmr3kIYTBlVy2zB6Rmdh8QTA6EzQSrO8mZYOfrLIaWwATKC7kEtInkBsK+YtF6K+JeT0+qRlCgCjDJ+dbj+uvKrfGNEKXBWk1GpG5fS51SiYbg1nQkFpgAtnLpQCl6QN/ljg==;24:f5DsJmhbzSFQLbjPdG6smZZRG0hsFBm5pUgfNsb9503hH8SObevlD4Bkd2P2L0IOp64ZyxL1+X4ZhKPGqOZ2/FPSuxmqXVLW5tZqqfJMFVc=;20:aTp6wu2+6R+Cui+UEtJughpd8GeshMX2/Dq1O+9M3rOmZP30h5X1wGF0lpTFSwfQD09lCAaXfFZOen1VW37GrQ== x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:HK2PR03MB1378; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:; x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(601004)(8121501046)(5005006)(3002001);SRVR:HK2PR03MB1378;BCL:0;PCL:0;RULEID:;SRVR:HK2PR03MB1378; x-forefront-prvs: 06818431B9 x-forefront-antispam-report: SFV:NSPM;SFS:(10009020)(6009001)(54534003)(13464003)(199003)(377454003)(377424004)(189002)(87936001)(19580395003)(19580405001)(2656002)(86362001)(74316001)(5007970100001)(68736005)(10400500002)(450100001)(77156002)(2950100001)(2900100001)(106356001)(102836002)(77096005)(105586002)(62966003)(33656002)(40100003)(122556002)(4001450100002)(92566002)(46102003)(76176999)(5003600100002)(54356999)(50986999)(5002640100001)(101416001)(76576001)(66066001)(110136002)(64706001)(107886002)(4001540100001)(5001960100002)(5001830100001)(5001860100001)(81156007)(97736004)(5004730100002)(5001920100001)(189998001)(4001430100001);DIR:OUT;SFP:1101;SCL:1;SRVR:HK2PR03MB1378;H:HK2PR03MB1380.apcprd03.prod.outlook.com;FPR:;SPF:None;PTR:InfoNoRecords;A:1;MX:1;LANG:en; received-spf: None (protection.outlook.com: kpit.com does not designate permitted sender hosts) Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: kpit.com X-MS-Exchange-CrossTenant-originalarrivaltime: 27 Aug 2015 13:13:57.0302 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 3539451e-b46e-4a26-a242-ff61502855c7 X-MS-Exchange-Transport-CrossTenantHeadersStamped: HK2PR03MB1378 X-SW-Source: 2015-08/txt/msg01694.txt.bz2 Hi DJ, Thanks for your feedback comments. Sorry it took me a while to get back on = this. >> Have you compared the latency of the multiply instructions to the overhe= ad of >> saving those registers in the interrupt handler? >> What about the case where performance is priority, and the developer kno= ws that >> the interrupt handlers don't use the multiply registers? >> Also, your code doesn't properly handle the case where the interrupts ar= e alread >> disabled when those functions are called. It would re-enable interrupts= before=20 >> the main code was prepared for it. Yes, I agree the patch does not handle the case where interrupts are disabl= ed.=20 Also, the code performance would suffer when the 'di/ei' instructions are p= laced=20 inline with the multiplication code. I have worked out an updated patch, which would save the MDUC specific regi= sters in the interrupt routine when the option '-msave-mduc-in-interrupts' is pas= sed. This gets active only for the G13 targets. This patch will save and restore the MDUC specific registers: mduc,mdal/h,m= dbl/h and mdcl/h This option does add about 56 bytes of code to the interrupt service routin= e due to the saves/restores via push/pop. Kindly review this patch and let me know if it would be useful for the RL78= port=20 (either in current state or with modifications) The other option/solution would be for the end user to disable/enable inter= rupts during the mul/div routines in project as per their requirement. This has been regression tested for ""-mg13 -msim -msave-mduc-in-interrupts" Best Regards, Kaushik gcc/ChangeLog 2015-08-27 Kaushik Phatak * config/rl78/rl78-real.md (movqi_from_mduc,movhi_from_mdal, movhi_from_mdah,movhi_from_mdbl,movhi_from_mdbh,movhi_from_mdcl, movhi_from_mdch,movqi_to_mduc,movhi_to_mdal,movhi_to_mdah, movhi_to_mdbl,movhi_to_mdbh,movhi_to_mdcl,movhi_to_mdch): New patterns. * config/rl78/rl78.c (rl78_expand_prologue): Save the MDUC related register in all interrupt handlers if necessary. (rl78_option_override): Add warning. (MUST_SAVE_MDUC_REGISTER): New macro. (rl78_expand_epilogue): Restore the MDUC registers if necessary. * config/rl78/rl78.opt (msave-mduc-in-interrupts): New option. * doc/invoke.texi (@item -msave-mduc-in-interrupts): New item. =09 Index: gcc/config/rl78/rl78-real.md =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- gcc/config/rl78/rl78-real.md (revision 227024) +++ gcc/config/rl78/rl78-real.md (working copy) @@ -37,6 +37,55 @@ =20 ;;---------- Moving ------------------------ =20 +(define_insn "movqi_from_mduc" + [(set (match_operand:QI 0 "register_operand" "=3Da") + (unspec_volatile:QI [(match_operand:QI 1 "" "")] UNS_BUILTIN_MDUC))] + "" + "mov\t%0, !0xf00e8" +) + +(define_insn "movhi_from_mdal" + [(set (match_operand:HI 0 "register_operand" "=3DA") + (unspec_volatile:HI [(match_operand:HI 1 "" "")] UNS_BUILTIN_MDAL))] + "" + "movw\t%0, !0xffff0" +) + +(define_insn "movhi_from_mdah" + [(set (match_operand:HI 0 "register_operand" "=3DA") + (unspec_volatile:HI [(match_operand:HI 1 "" "")] UNS_BUILTIN_MDAH))] + "" + "movw\t%0, !0xffff2" +) + +(define_insn "movhi_from_mdbl" + [(set (match_operand:HI 0 "register_operand" "=3DA") + (unspec_volatile:HI [(match_operand 1)] UNS_BUILTIN_MDBL))] + "" + "movw\t%0, !0xffff4" +) + +(define_insn "movhi_from_mdbh" + [(set (match_operand:HI 0 "register_operand" "=3DA") + (unspec_volatile:HI [(match_operand 1)] UNS_BUILTIN_MDBH))] + "" + "movw\t%0, !0xffff6" +) + +(define_insn "movhi_from_mdcl" + [(set (match_operand:HI 0 "register_operand" "=3DA") + (unspec_volatile:HI [(match_operand 1)] UNS_BUILTIN_MDCL))] + "" + "movw\t%0, !0xf00e0" +) + +(define_insn "movhi_from_mdch" + [(set (match_operand:HI 0 "register_operand" "=3DA") + (unspec_volatile:HI [(match_operand 1)] UNS_BUILTIN_MDCH))] + "" + "movw\t%0, !0xf00e2" +) + (define_insn "movqi_to_es" [(set (reg:QI ES_REG) (match_operand:QI 0 "register_operand" "a"))] @@ -51,6 +100,62 @@ "mov\t%0, es" ) =20 +(define_insn "movqi_to_mduc" + [(set (reg:QI 0 ) + (unspec_volatile:QI [(match_operand:QI 0)] UNS_BUILTIN_MDUC)) + (match_operand:QI 1 "register_operand" "A")] + "" + "mov\t!0xf00e8, %1" +) + +(define_insn "movhi_to_mdal" + [(set (reg:HI 0 ) + (unspec_volatile:HI [(match_operand:HI 0)] UNS_BUILTIN_MDAL)) + (match_operand:HI 1 "register_operand" "A")] + "" + "movw\t!0xffff0, %1" +) + +(define_insn "movhi_to_mdah" + [(set (reg:HI 0 ) + (unspec_volatile:HI [(match_operand:HI 0)] UNS_BUILTIN_MDAH)) + (match_operand:HI 1 "register_operand" "A")] + "" + "movw\t!0xffff2, %1" +) + +(define_insn "movhi_to_mdbl" + [(set (reg:HI 0 ) + (unspec_volatile:HI [(match_operand:HI 0)] UNS_BUILTIN_MDBL)) + (match_operand:HI 1 "register_operand" "A")] + "" + "movw\t!0xffff4, %1" +) + +(define_insn "movhi_to_mdbh" + [(set (reg:HI 0 ) + (unspec_volatile:HI [(match_operand:HI 0)] UNS_BUILTIN_MDBH)) + (match_operand:HI 1 "register_operand" "A")] + "" + "movw\t!0xffff6, %1" +) + +(define_insn "movhi_to_mdcl" + [(set (reg:HI 0 ) + (unspec_volatile:HI [(match_operand:HI 0)] UNS_BUILTIN_MDCL)) + (match_operand:HI 1 "register_operand" "A")] + "" + "movw\t!0xf00e0, %1" +) + +(define_insn "movhi_to_mdch" + [(set (reg:HI 0 )=20 + (unspec_volatile:HI [(match_operand:HI 0)] UNS_BUILTIN_MDCH)) + (match_operand:HI 1 "register_operand" "A")] + "" + "movw\t!0xf00e2, %1" +) + (define_insn "movqi_cs" [(set (reg:QI CS_REG) (match_operand:QI 0 "register_operand" "a"))] Index: gcc/config/rl78/rl78.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- gcc/config/rl78/rl78.c (revision 227024) +++ gcc/config/rl78/rl78.c (working copy) @@ -338,6 +338,10 @@ #undef TARGET_OPTION_OVERRIDE #define TARGET_OPTION_OVERRIDE rl78_option_override =20 +#define MUST_SAVE_MDUC_REGISTER \ + (TARGET_SAVE_MDUC_REGISTER \ + && (is_interrupt_func (NULL_TREE)) && RL78_MUL_G13) + static void rl78_option_override (void) { @@ -365,6 +369,9 @@ /* Address spaces are currently only supported by C. */ error ("-mes0 can only be used with C"); =20 + if (TARGET_SAVE_MDUC_REGISTER && !(TARGET_G13 || RL78_MUL_G13)) + warning (0, "mduc registers only saved for G13 target"); + switch (rl78_cpu_type) { case CPU_UNINIT: @@ -1273,6 +1280,7 @@ int i, fs; rtx sp =3D gen_rtx_REG (HImode, STACK_POINTER_REGNUM); rtx ax =3D gen_rtx_REG (HImode, AX_REG); + rtx operand1; int rb =3D 0; =20 if (rl78_is_naked_func ()) @@ -1330,6 +1338,39 @@ F (emit_insn (gen_push (ax))); } =20 + /* Save MDUC register from interrupt routine. */ + if (MUST_SAVE_MDUC_REGISTER) + { + emit_insn (gen_movqi_from_mduc (gen_rtx_REG (QImode, A_REG), + gen_rtx_UNSPEC_VOLATILE (QImode, gen_rtvec (1, operand1), + UNS_BUILTIN_MDUC))); + F (emit_insn (gen_push (ax))); + emit_insn (gen_movhi_from_mdal (gen_rtx_REG (HImode, AX_REG), + gen_rtx_UNSPEC_VOLATILE (HImode, gen_rtvec (1, operand1), + UNS_BUILTIN_MDAL))); + F (emit_insn (gen_push (ax))); + emit_insn (gen_movhi_from_mdah (gen_rtx_REG (HImode, AX_REG), + gen_rtx_UNSPEC_VOLATILE (HImode, gen_rtvec (1, operand1), + UNS_BUILTIN_MDAH))); + F (emit_insn (gen_push (ax))); + emit_insn (gen_movhi_from_mdbl (gen_rtx_REG (HImode, AX_REG), + gen_rtx_UNSPEC_VOLATILE (HImode, gen_rtvec (1, operand1), + UNS_BUILTIN_MDBL))); + F (emit_insn (gen_push (ax))); + emit_insn (gen_movhi_from_mdbh (gen_rtx_REG (HImode, AX_REG), + gen_rtx_UNSPEC_VOLATILE (HImode, gen_rtvec (1, operand1), + UNS_BUILTIN_MDBH))); + F (emit_insn (gen_push (ax))); + emit_insn (gen_movhi_from_mdcl (gen_rtx_REG (HImode, AX_REG), + gen_rtx_UNSPEC_VOLATILE (HImode, gen_rtvec (1, operand1), + UNS_BUILTIN_MDCL))); + F (emit_insn (gen_push (ax))); + emit_insn (gen_movhi_from_mdch (gen_rtx_REG (HImode, AX_REG), + gen_rtx_UNSPEC_VOLATILE (HImode, gen_rtvec (1, operand1), + UNS_BUILTIN_MDCH))); + F (emit_insn (gen_push (ax))); + } + if (frame_pointer_needed) { F (emit_move_insn (ax, sp)); @@ -1372,6 +1413,7 @@ int i, fs; rtx sp =3D gen_rtx_REG (HImode, STACK_POINTER_REGNUM); rtx ax =3D gen_rtx_REG (HImode, AX_REG); + rtx operand1; int rb =3D 0; =20 if (rl78_is_naked_func ()) @@ -1403,6 +1445,39 @@ } } =20 + /* Restore MDUC register from interrupt routine */ + if (MUST_SAVE_MDUC_REGISTER) + { + emit_insn (gen_pop (gen_rtx_REG (HImode, AX_REG))); + emit_insn (gen_movhi_to_mdch (gen_rtx_UNSPEC_VOLATILE (HImode, + gen_rtvec (1, operand1),UNS_BUILTIN_MDCH), + gen_rtx_REG (HImode, AX_REG))); + emit_insn (gen_pop (gen_rtx_REG (HImode, AX_REG))); + emit_insn (gen_movhi_to_mdcl (gen_rtx_UNSPEC_VOLATILE (HImode, + gen_rtvec (1, operand1),UNS_BUILTIN_MDCL), + gen_rtx_REG (HImode, AX_REG))); + emit_insn (gen_pop (gen_rtx_REG (HImode, AX_REG))); + emit_insn (gen_movhi_to_mdbh (gen_rtx_UNSPEC_VOLATILE (HImode, + gen_rtvec (1, operand1),UNS_BUILTIN_MDBH), + gen_rtx_REG (HImode, AX_REG))); + emit_insn (gen_pop (gen_rtx_REG (HImode, AX_REG))); + emit_insn (gen_movhi_to_mdbl (gen_rtx_UNSPEC_VOLATILE (HImode, + gen_rtvec (1, operand1),UNS_BUILTIN_MDBL), + gen_rtx_REG (HImode, AX_REG))); + emit_insn (gen_pop (gen_rtx_REG (HImode, AX_REG))); + emit_insn (gen_movhi_to_mdah (gen_rtx_UNSPEC_VOLATILE (HImode, + gen_rtvec (1, operand1),UNS_BUILTIN_MDAH), + gen_rtx_REG (HImode, AX_REG))); + emit_insn (gen_pop (gen_rtx_REG (HImode, AX_REG))); + emit_insn (gen_movhi_to_mdal (gen_rtx_UNSPEC_VOLATILE (HImode, + gen_rtvec (1, operand1),UNS_BUILTIN_MDAL), + gen_rtx_REG (HImode, AX_REG))); + emit_insn (gen_pop (gen_rtx_REG (HImode, AX_REG))); + emit_insn (gen_movqi_to_mduc (gen_rtx_UNSPEC_VOLATILE (QImode, + gen_rtvec (1, operand1),UNS_BUILTIN_MDUC), + gen_rtx_REG (QImode, A_REG))); + } + if (is_interrupt_func (cfun->decl) && cfun->machine->uses_es) { emit_insn (gen_pop (gen_rtx_REG (HImode, AX_REG))); @@ -1498,6 +1573,9 @@ =20 if (cfun->machine->uses_es) fprintf (file, "\t; uses ES register\n"); + + if (MUST_SAVE_MDUC_REGISTER) + fprintf (file, "\t; uses MDUC register\n"); } =20 /* Return an RTL describing where a function return value of type RET_TYPE Index: gcc/config/rl78/rl78.md =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- gcc/config/rl78/rl78.md (revision 227024) +++ gcc/config/rl78/rl78.md (working copy) @@ -50,6 +50,13 @@ (UNS_TRAMPOLINE_INIT 20) (UNS_TRAMPOLINE_UNINIT 21) (UNS_NONLOCAL_GOTO 22) + (UNS_BUILTIN_MDUC 35) + (UNS_BUILTIN_MDAL 36) + (UNS_BUILTIN_MDAH 37) + (UNS_BUILTIN_MDBL 38) + (UNS_BUILTIN_MDBH 39) + (UNS_BUILTIN_MDCL 40) + (UNS_BUILTIN_MDCH 41) =20 ]) =20 Index: gcc/config/rl78/rl78.opt =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- gcc/config/rl78/rl78.opt (revision 227024) +++ gcc/config/rl78/rl78.opt (working copy) @@ -91,3 +91,7 @@ mes0 Target Mask(ES0) Assume ES is zero throughout program execution, use ES: for read-only data. + +msave-mduc-in-interrupts +Target Mask(SAVE_MDUC_REGISTER) +Specifies whether interrupt functions should save and restore the MDUC reg= ister. Index: gcc/doc/invoke.texi =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- gcc/doc/invoke.texi (revision 227024) +++ gcc/doc/invoke.texi (working copy) @@ -19026,6 +19026,14 @@ or 32 bits (@option{-m32bit-doubles}) in size. The default is @option{-m32bit-doubles}. =20 +@item -msave-mduc-in-interrupts +@opindex msave-mduc-in-interrupts +Specifies that interrupt handler functions should preserve the +MDUC registers. This is only necessary if normal code might use +the MDUC register, for example because it performs multiplication +and division operations. The default is to ignore the MDUC registers +as this makes the interrupt handlers faster. The target option -mg13 + @end table =20 @node RS/6000 and PowerPC Options -----Original Message----- From: DJ Delorie [mailto:dj@redhat.com]=20 Sent: Friday, June 05, 2015 12:15 PM To: Kaushik Phatak Cc: gcc-patches@gcc.gnu.org; nickc@redhat.com Subject: Re: [PATCH : RL78] Disable interrupts during hardware multiplicati= on routines