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 overhead of
>> saving those registers in the interrupt handler?
>> What about the case where performance is priority, and the developer knows
>> that
>> the interrupt handlers don't use the multiply registers?
>> Also, your code doesn't properly handle the case where the interrupts are
>> alread
>> disabled when those functions are called. It would re-enable interrupts
>> before
>> the main code was prepared for it.
Yes, I agree the patch does not handle the case where interrupts are disabled.
Also, the code performance would suffer when the 'di/ei' instructions are
placed
inline with the multiplication code.
I have worked out an updated patch, which would save the MDUC specific registers
in the interrupt routine when the option '-msave-mduc-in-interrupts' is passed.
This gets active only for the G13 targets.
This patch will save and restore the MDUC specific registers:
mduc,mdal/h,mdbl/h and mdcl/h
This option does add about 56 bytes of code to the interrupt service routine 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
(either in current state or with modifications)
The other option/solution would be for the end user to disable/enable
interrupts 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 <kaushik.pha...@kpit.com>
* 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.
Index: gcc/config/rl78/rl78-real.md
===================================================================
--- gcc/config/rl78/rl78-real.md (revision 227024)
+++ gcc/config/rl78/rl78-real.md (working copy)
@@ -37,6 +37,55 @@
;;---------- Moving ------------------------
+(define_insn "movqi_from_mduc"
+ [(set (match_operand:QI 0 "register_operand" "=a")
+ (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" "=A")
+ (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" "=A")
+ (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" "=A")
+ (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" "=A")
+ (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" "=A")
+ (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" "=A")
+ (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"
)
+(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 )
+ (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
===================================================================
--- 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
+#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");
+ 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 = gen_rtx_REG (HImode, STACK_POINTER_REGNUM);
rtx ax = gen_rtx_REG (HImode, AX_REG);
+ rtx operand1;
int rb = 0;
if (rl78_is_naked_func ())
@@ -1330,6 +1338,39 @@
F (emit_insn (gen_push (ax)));
}
+ /* 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 = gen_rtx_REG (HImode, STACK_POINTER_REGNUM);
rtx ax = gen_rtx_REG (HImode, AX_REG);
+ rtx operand1;
int rb = 0;
if (rl78_is_naked_func ())
@@ -1403,6 +1445,39 @@
}
}
+ /* 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 @@
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.md
===================================================================
--- 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)
])
Index: gcc/config/rl78/rl78.opt
===================================================================
--- 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
register.
Index: gcc/doc/invoke.texi
===================================================================
--- 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}.
+@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
@node RS/6000 and PowerPC Options
-----Original Message-----
From: DJ Delorie [mailto:d...@redhat.com]
Sent: Friday, June 05, 2015 12:15 PM
To: Kaushik Phatak <kaushik.pha...@kpit.com>
Cc: gcc-patches@gcc.gnu.org; ni...@redhat.com
Subject: Re: [PATCH : RL78] Disable interrupts during hardware multiplication
routines
