From: Kaushik Phatak <Kaushik.Phatak@kpit.com>
To: "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>
Subject: RE: [PATCH : RL78] Disable interrupts during hardware multiplication routines
Date: Mon, 07 Dec 2015 12:36:00 -0000 [thread overview]
Message-ID: <SG2PR03MB139162D32BF30504AF24B5AEFC090@SG2PR03MB1391.apcprd03.prod.outlook.com> (raw)
In-Reply-To: <201508271711.t7RHBSgM026604@greed.delorie.com>
[-- Attachment #1: Type: text/plain, Size: 9337 bytes --]
Hi DJ,
Please find attached an updated patch which tries to address the points raised by you in my earlier attempt,
https://gcc.gnu.org/ml/gcc-patches/2015-08/msg01729.html
1. Added an option for -msave.. and -mno-save..
The default will be to save the MDUC registers for the g13 target in the ISR.
2. As an optimization, it will check for usage of mul and divmod routines before saving/restoring.
3. I have eliminated the special insns used earlier and directly generating movhi/movqi for the mem reference, also
setting them as volatile.
4. Updated and fixed the issue in invoke.texi.
This has been regression tested for -mg13 -msim.
The only glitch I observed was the list file printed out the address of the MDUC
registers in decimal and not in HEX, for example,
mov a, !983272 is displayed instead of,
mov a, !0xF00E8
However, the objectdump generates these addresses correctly in hex along with their register name
references (<MDUC>,<MDAL> etc.)
Please let me know if this updated patch is OK.
Best Regards,
Kaushik
gcc/ChangeLog
2015-12-07 Kaushik Phatak <kaushik.phatak@kpit.com>
* config/rl78/rl78.c (rl78_expand_prologue): Save the MDUC related
registers 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.c (check_mduc_usage): New function.
* config/rl78/rl78.opt (msave-mduc-in-interrupts): New option.
(mno-save-mduc-in-interrupts): New option.
* doc/invoke.texi (@item -msave-mduc-in-interrupts): New item.
(@item -mno-save-mduc-in-interrupts): New item
Index: gcc/config/rl78/rl78.c
===================================================================
--- gcc/config/rl78/rl78.c (revision 2871)
+++ gcc/config/rl78/rl78.c (working copy)
@@ -342,6 +342,10 @@
#undef TARGET_OPTION_OVERRIDE
#define TARGET_OPTION_OVERRIDE rl78_option_override
+#define MUST_SAVE_MDUC_REGISTER \
+ (!TARGET_NO_SAVE_MDUC_REGISTER \
+ && (is_interrupt_func (NULL_TREE)) && RL78_MUL_G13)
+
static void
rl78_option_override (void)
{
@@ -366,6 +370,9 @@
/* Address spaces are currently only supported by C. */
error ("-mes0 can only be used with C");
+ 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:
@@ -1307,6 +1314,27 @@
return (lookup_attribute ("naked", DECL_ATTRIBUTES (current_function_decl)) != NULL_TREE);
}
+/* Check if the block uses mul/div insns. */
+int
+check_mduc_usage ()
+{
+ rtx insn;
+ basic_block bb;
+ FOR_EACH_BB_FN (bb, cfun)
+ {
+ FOR_BB_INSNS (bb, insn)
+ {
+ if (recog_memoized (insn) == CODE_FOR_udivmodsi4_g13
+ || recog_memoized (insn) == CODE_FOR_mulhi3_g13
+ || recog_memoized (insn) == CODE_FOR_mulsi3_g13)
+ {
+ return 1;
+ }
+ }
+ }
+ return 0;
+}
+
/* Expand the function prologue (from the prologue pattern). */
void
rl78_expand_prologue (void)
@@ -1318,7 +1346,7 @@
if (rl78_is_naked_func ())
return;
-
+
/* Always re-compute the frame info - the register usage may have changed. */
rl78_compute_frame_info ();
@@ -1371,6 +1399,46 @@
F (emit_insn (gen_push (ax)));
}
+ /* Save MDUC register inside interrupt routine. */
+ if (MUST_SAVE_MDUC_REGISTER && (!crtl->is_leaf || check_mduc_usage ()))
+ {
+ rtx mem_mduc;
+
+ mem_mduc = gen_rtx_MEM (QImode, GEN_INT (0xf00e8));
+ MEM_VOLATILE_P (mem_mduc) = 1;
+ emit_insn (gen_movqi (gen_rtx_REG (QImode, A_REG), mem_mduc));
+ emit_insn (gen_push (gen_rtx_REG (HImode, AX_REG)));
+
+ mem_mduc = gen_rtx_MEM (HImode, GEN_INT (0xffff0));
+ MEM_VOLATILE_P (mem_mduc) = 1;
+ emit_insn (gen_movhi (gen_rtx_REG (HImode, AX_REG), mem_mduc));
+ emit_insn (gen_push (gen_rtx_REG (HImode, AX_REG)));
+
+ mem_mduc = gen_rtx_MEM (HImode, GEN_INT (0xffff2));
+ MEM_VOLATILE_P (mem_mduc) = 1;
+ emit_insn (gen_movhi (gen_rtx_REG (HImode, AX_REG), mem_mduc));
+ emit_insn (gen_push (gen_rtx_REG (HImode, AX_REG)));
+
+ mem_mduc = gen_rtx_MEM (HImode, GEN_INT (0xffff4));
+ MEM_VOLATILE_P (mem_mduc) = 1;
+ emit_insn (gen_movhi (gen_rtx_REG (HImode, AX_REG), mem_mduc));
+ emit_insn (gen_push (gen_rtx_REG (HImode, AX_REG)));
+
+ mem_mduc = gen_rtx_MEM (HImode, GEN_INT (0xffff6));
+ MEM_VOLATILE_P (mem_mduc) = 1;
+ emit_insn (gen_movhi (gen_rtx_REG (HImode, AX_REG), mem_mduc));
+ emit_insn (gen_push (gen_rtx_REG (HImode, AX_REG)));
+
+ mem_mduc = gen_rtx_MEM (HImode, GEN_INT (0xf00e0));
+ MEM_VOLATILE_P (mem_mduc) = 1;
+ emit_insn (gen_movhi (gen_rtx_REG (HImode, AX_REG), mem_mduc));
+ emit_insn (gen_push (gen_rtx_REG (HImode, AX_REG)));
+
+ mem_mduc = gen_rtx_MEM (HImode, GEN_INT (0xf00e2));
+ MEM_VOLATILE_P (mem_mduc) = 1;
+ emit_insn (gen_movhi (gen_rtx_REG (HImode, AX_REG), mem_mduc));
+ emit_insn (gen_push (gen_rtx_REG (HImode, AX_REG)));
+ }
if (frame_pointer_needed)
{
F (emit_move_insn (ax, sp));
@@ -1444,6 +1512,47 @@
}
}
+ /* Restore MDUC register from interrupt routine. */
+ if (MUST_SAVE_MDUC_REGISTER && (!crtl->is_leaf || check_mduc_usage ()))
+ {
+ rtx mem_mduc;
+
+ mem_mduc = gen_rtx_MEM (HImode, GEN_INT (0xf00e2));
+ MEM_VOLATILE_P (mem_mduc) = 1;
+ emit_insn (gen_pop (gen_rtx_REG (HImode, AX_REG)));
+ emit_insn (gen_movhi (mem_mduc, gen_rtx_REG (HImode, AX_REG)));
+
+ mem_mduc = gen_rtx_MEM (HImode, GEN_INT (0xf00e0));
+ MEM_VOLATILE_P (mem_mduc) = 1;
+ emit_insn (gen_pop (gen_rtx_REG (HImode, AX_REG)));
+ emit_insn (gen_movhi (mem_mduc, gen_rtx_REG (HImode, AX_REG)));
+
+ mem_mduc = gen_rtx_MEM (HImode, GEN_INT (0xffff6));
+ MEM_VOLATILE_P (mem_mduc) = 1;
+ emit_insn (gen_pop (gen_rtx_REG (HImode, AX_REG)));
+ emit_insn (gen_movhi (mem_mduc, gen_rtx_REG (HImode, AX_REG)));
+
+ mem_mduc = gen_rtx_MEM (HImode, GEN_INT (0xffff4));
+ MEM_VOLATILE_P (mem_mduc) = 1;
+ emit_insn (gen_pop (gen_rtx_REG (HImode, AX_REG)));
+ emit_insn (gen_movhi (mem_mduc, gen_rtx_REG (HImode, AX_REG)));
+
+ mem_mduc = gen_rtx_MEM (HImode, GEN_INT (0xffff2));
+ MEM_VOLATILE_P (mem_mduc) = 1;
+ emit_insn (gen_pop (gen_rtx_REG (HImode, AX_REG)));
+ emit_insn (gen_movhi (mem_mduc, gen_rtx_REG (HImode, AX_REG)));
+
+ mem_mduc = gen_rtx_MEM (HImode, GEN_INT (0xffff0));
+ MEM_VOLATILE_P (mem_mduc) = 1;
+ emit_insn (gen_pop (gen_rtx_REG (HImode, AX_REG)));
+ emit_insn (gen_movhi (mem_mduc, gen_rtx_REG (HImode, AX_REG)));
+
+ mem_mduc = gen_rtx_MEM (QImode, GEN_INT (0xf00e8));
+ MEM_VOLATILE_P (mem_mduc) = 1;
+ emit_insn (gen_pop (gen_rtx_REG (HImode, AX_REG)));
+ emit_insn (gen_movqi (mem_mduc, gen_rtx_REG (QImode, A_REG)));
+ }
+
/* Save ES register inside interrupt functions. */
if (is_interrupt_func (cfun->decl) && cfun->machine->uses_es)
{
@@ -1599,6 +1708,9 @@
if (cfun->machine->uses_es)
fprintf (file, "\t; uses ES register\n");
+
+ if (MUST_SAVE_MDUC_REGISTER)
+ fprintf (file, "\t; uses MDUC register\n");
}
/* Return an RTL describing where a function return value of type RET_TYPE
Index: gcc/config/rl78/rl78.opt
===================================================================
--- gcc/config/rl78/rl78.opt (revision 2871)
+++ gcc/config/rl78/rl78.opt (working copy)
@@ -103,4 +103,11 @@
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)
+Stores the MDUC registers for interrupt handlers for G13 target.
+This is the default.
+mno-save-mduc-in-interrupts
+Target RejectNegative Mask(NO_SAVE_MDUC_REGISTER)
+Does not save the MDUC registers for interrupt handlers for G13 target.
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi (revision 2871)
+++ gcc/doc/invoke.texi (working copy)
@@ -18901,6 +18901,18 @@
registers @code{r24..r31} are reserved for use in interrupt handlers.
With this option enabled these registers can be used in ordinary
functions as well.
+
+@item -msave-mduc-in-interrupts
+@item -mno-save-mduc-in-interrupts
+@opindex msave-mduc-in-interrupts
+@opindex mno-save-mduc-in-interrupts
+Specifies that interrupt handler functions should preserve the
+MDUC registers. This is only necessary if normal code might use
+the MDUC registers, 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
+needs to be passed for this to work as this feature is only available
+on the G13 target (S2 core).
@end table
@node RS/6000 and PowerPC Options
[-- Attachment #2: rl78_mduc_patch.diff --]
[-- Type: application/octet-stream, Size: 7243 bytes --]
Index: gcc/config/rl78/rl78.c
===================================================================
--- gcc/config/rl78/rl78.c (revision 2871)
+++ gcc/config/rl78/rl78.c (working copy)
@@ -342,6 +342,10 @@
#undef TARGET_OPTION_OVERRIDE
#define TARGET_OPTION_OVERRIDE rl78_option_override
+#define MUST_SAVE_MDUC_REGISTER \
+ (!TARGET_NO_SAVE_MDUC_REGISTER \
+ && (is_interrupt_func (NULL_TREE)) && RL78_MUL_G13)
+
static void
rl78_option_override (void)
{
@@ -366,6 +370,9 @@
/* Address spaces are currently only supported by C. */
error ("-mes0 can only be used with C");
+ 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:
@@ -1307,6 +1314,27 @@
return (lookup_attribute ("naked", DECL_ATTRIBUTES (current_function_decl)) != NULL_TREE);
}
+/* Check if the block uses mul/div insns. */
+int
+check_mduc_usage ()
+{
+ rtx insn;
+ basic_block bb;
+ FOR_EACH_BB_FN (bb, cfun)
+ {
+ FOR_BB_INSNS (bb, insn)
+ {
+ if (recog_memoized (insn) == CODE_FOR_udivmodsi4_g13
+ || recog_memoized (insn) == CODE_FOR_mulhi3_g13
+ || recog_memoized (insn) == CODE_FOR_mulsi3_g13)
+ {
+ return 1;
+ }
+ }
+ }
+ return 0;
+}
+
/* Expand the function prologue (from the prologue pattern). */
void
rl78_expand_prologue (void)
@@ -1371,6 +1399,46 @@
F (emit_insn (gen_push (ax)));
}
+ /* Save MDUC register inside interrupt routine. */
+ if (MUST_SAVE_MDUC_REGISTER && (!crtl->is_leaf || check_mduc_usage ()))
+ {
+ rtx mem_mduc;
+
+ mem_mduc = gen_rtx_MEM (QImode, GEN_INT (0xf00e8));
+ MEM_VOLATILE_P (mem_mduc) = 1;
+ emit_insn (gen_movqi (gen_rtx_REG (QImode, A_REG), mem_mduc));
+ emit_insn (gen_push (gen_rtx_REG (HImode, AX_REG)));
+
+ mem_mduc = gen_rtx_MEM (HImode, GEN_INT (0xffff0));
+ MEM_VOLATILE_P (mem_mduc) = 1;
+ emit_insn (gen_movhi (gen_rtx_REG (HImode, AX_REG), mem_mduc));
+ emit_insn (gen_push (gen_rtx_REG (HImode, AX_REG)));
+
+ mem_mduc = gen_rtx_MEM (HImode, GEN_INT (0xffff2));
+ MEM_VOLATILE_P (mem_mduc) = 1;
+ emit_insn (gen_movhi (gen_rtx_REG (HImode, AX_REG), mem_mduc));
+ emit_insn (gen_push (gen_rtx_REG (HImode, AX_REG)));
+
+ mem_mduc = gen_rtx_MEM (HImode, GEN_INT (0xffff4));
+ MEM_VOLATILE_P (mem_mduc) = 1;
+ emit_insn (gen_movhi (gen_rtx_REG (HImode, AX_REG), mem_mduc));
+ emit_insn (gen_push (gen_rtx_REG (HImode, AX_REG)));
+
+ mem_mduc = gen_rtx_MEM (HImode, GEN_INT (0xffff6));
+ MEM_VOLATILE_P (mem_mduc) = 1;
+ emit_insn (gen_movhi (gen_rtx_REG (HImode, AX_REG), mem_mduc));
+ emit_insn (gen_push (gen_rtx_REG (HImode, AX_REG)));
+
+ mem_mduc = gen_rtx_MEM (HImode, GEN_INT (0xf00e0));
+ MEM_VOLATILE_P (mem_mduc) = 1;
+ emit_insn (gen_movhi (gen_rtx_REG (HImode, AX_REG), mem_mduc));
+ emit_insn (gen_push (gen_rtx_REG (HImode, AX_REG)));
+
+ mem_mduc = gen_rtx_MEM (HImode, GEN_INT (0xf00e2));
+ MEM_VOLATILE_P (mem_mduc) = 1;
+ emit_insn (gen_movhi (gen_rtx_REG (HImode, AX_REG), mem_mduc));
+ emit_insn (gen_push (gen_rtx_REG (HImode, AX_REG)));
+ }
if (frame_pointer_needed)
{
F (emit_move_insn (ax, sp));
@@ -1444,6 +1512,47 @@
}
}
+ /* Restore MDUC register from interrupt routine. */
+ if (MUST_SAVE_MDUC_REGISTER && (!crtl->is_leaf || check_mduc_usage ()))
+ {
+ rtx mem_mduc;
+
+ mem_mduc = gen_rtx_MEM (HImode, GEN_INT (0xf00e2));
+ MEM_VOLATILE_P (mem_mduc) = 1;
+ emit_insn (gen_pop (gen_rtx_REG (HImode, AX_REG)));
+ emit_insn (gen_movhi (mem_mduc, gen_rtx_REG (HImode, AX_REG)));
+
+ mem_mduc = gen_rtx_MEM (HImode, GEN_INT (0xf00e0));
+ MEM_VOLATILE_P (mem_mduc) = 1;
+ emit_insn (gen_pop (gen_rtx_REG (HImode, AX_REG)));
+ emit_insn (gen_movhi (mem_mduc, gen_rtx_REG (HImode, AX_REG)));
+
+ mem_mduc = gen_rtx_MEM (HImode, GEN_INT (0xffff6));
+ MEM_VOLATILE_P (mem_mduc) = 1;
+ emit_insn (gen_pop (gen_rtx_REG (HImode, AX_REG)));
+ emit_insn (gen_movhi (mem_mduc, gen_rtx_REG (HImode, AX_REG)));
+
+ mem_mduc = gen_rtx_MEM (HImode, GEN_INT (0xffff4));
+ MEM_VOLATILE_P (mem_mduc) = 1;
+ emit_insn (gen_pop (gen_rtx_REG (HImode, AX_REG)));
+ emit_insn (gen_movhi (mem_mduc, gen_rtx_REG (HImode, AX_REG)));
+
+ mem_mduc = gen_rtx_MEM (HImode, GEN_INT (0xffff2));
+ MEM_VOLATILE_P (mem_mduc) = 1;
+ emit_insn (gen_pop (gen_rtx_REG (HImode, AX_REG)));
+ emit_insn (gen_movhi (mem_mduc, gen_rtx_REG (HImode, AX_REG)));
+
+ mem_mduc = gen_rtx_MEM (HImode, GEN_INT (0xffff0));
+ MEM_VOLATILE_P (mem_mduc) = 1;
+ emit_insn (gen_pop (gen_rtx_REG (HImode, AX_REG)));
+ emit_insn (gen_movhi (mem_mduc, gen_rtx_REG (HImode, AX_REG)));
+
+ mem_mduc = gen_rtx_MEM (QImode, GEN_INT (0xf00e8));
+ MEM_VOLATILE_P (mem_mduc) = 1;
+ emit_insn (gen_pop (gen_rtx_REG (HImode, AX_REG)));
+ emit_insn (gen_movqi (mem_mduc, gen_rtx_REG (QImode, A_REG)));
+ }
+
/* Save ES register inside interrupt functions. */
if (is_interrupt_func (cfun->decl) && cfun->machine->uses_es)
{
@@ -1599,6 +1708,9 @@
if (cfun->machine->uses_es)
fprintf (file, "\t; uses ES register\n");
+
+ if (MUST_SAVE_MDUC_REGISTER)
+ fprintf (file, "\t; uses MDUC register\n");
}
/* Return an RTL describing where a function return value of type RET_TYPE
Index: gcc/config/rl78/rl78.opt
===================================================================
--- gcc/config/rl78/rl78.opt (revision 2871)
+++ gcc/config/rl78/rl78.opt (working copy)
@@ -103,4 +103,11 @@
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)
+Stores the MDUC registers for interrupt handlers for G13 target.
+This is the default.
+mno-save-mduc-in-interrupts
+Target RejectNegative Mask(NO_SAVE_MDUC_REGISTER)
+Does not save the MDUC registers for interrupt handlers for G13 target.
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi (revision 2871)
+++ gcc/doc/invoke.texi (working copy)
@@ -18901,6 +18901,18 @@
registers @code{r24..r31} are reserved for use in interrupt handlers.
With this option enabled these registers can be used in ordinary
functions as well.
+
+@item -msave-mduc-in-interrupts
+@item -mno-save-mduc-in-interrupts
+@opindex msave-mduc-in-interrupts
+@opindex mno-save-mduc-in-interrupts
+Specifies that interrupt handler functions should preserve the
+MDUC registers. This is only necessary if normal code might use
+the MDUC registers, 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
+needs to be passed for this to work as this feature is only available
+on the G13 target (S2 core).
@end table
@node RS/6000 and PowerPC Options
next prev parent reply other threads:[~2015-12-07 12:36 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-05 6:44 Kaushik Phatak
2015-06-05 8:02 ` DJ Delorie
2015-08-27 13:14 ` Kaushik Phatak
2015-08-27 17:11 ` Mike Stump
2015-08-27 17:25 ` DJ Delorie
2015-12-07 12:36 ` Kaushik Phatak [this message]
[not found] <CAJA2CV1y0Mk20KqgFbdFs19tyPCH7HvR_XGAwy=OyhYcgNLuWQ@mail.gmail.com>
[not found] ` <C6735D72-E033-470D-A8B2-1949D7A04AEE@comcast.net>
2016-01-07 11:27 ` Kaushik M Phatak
2016-01-07 12:03 ` Nick Clifton
2016-01-12 7:21 Kaushik M Phatak
2016-01-12 12:20 ` Nick Clifton
2016-01-12 15:54 ` Mike Stump
2016-05-04 11:47 ` Kaushik Phatak
2016-05-09 11:46 ` Nick Clifton
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=SG2PR03MB139162D32BF30504AF24B5AEFC090@SG2PR03MB1391.apcprd03.prod.outlook.com \
--to=kaushik.phatak@kpit.com \
--cc=gcc-patches@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).