[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [microblaze-uclinux] [patch] msrset/msrclr support

To: microblaze-uclinux@xxxxxxxxxxxxxx
Subject: Re: [microblaze-uclinux] [patch] msrset/msrclr support
From: Goran Bilski <Goran.Bilski@xxxxxxxxxx>
Date: Tue, 18 May 2004 17:27:40 +0200
In-reply-to: <1084891709.7714.5.camel@pac>
Organization: Xilinx, Inc.
References: <40A97558.2090607@itee.uq.edu.au> <1084858359.6214.17.camel@pac><40A9A4E7.8070902@xilinx.com> <1084891709.7714.5.camel@pac>
Reply-to: microblaze-uclinux@xxxxxxxxxxxxxx
Sender: owner-microblaze-uclinux@xxxxxxxxxxxxxx
User-agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4)Gecko/20030624 Netscape/7.1 (ax)
An instruction can only have 2 registers as inputs and 1 register as the 
output.
This limits the sequence of instruction that can be converted into a new 
instructions.

With FSL the HW function can have unlimited number of inputs and outputs 
which allows for much large functions to be converted the HW.
Ex. IDCT in jpeg has 64 inputs and 64 outputs.

Göran

Matthew Rubenstein wrote:

>	Hi - I see how the FSL saves development time, but how does it execute
>faster than an instruction, when both are made of HW logic on the same
>chip?
>
>
>On Tue, 2004-05-18 at 01:53, Goran Bilski wrote:
>  
>
>>Hi,
>>
>>It's much better to find the C-function that consumes most time in the 
>>kernel and put the whole function on a FSL.
>>The FSL interface exists today and you can build your own function and 
>>attach to it.
>>It will give more you more speed up than user defined instructions.
>>
>>Göran
>>
>>Matthew Rubenstein wrote:
>>
>>    
>>
>>>	Has anyone profiled the code to find the most common instruction
>>>sequences, which might be similarly replaced by atomic instructions? Is
>>>there anything preventing rolling our own instruction patches, rather
>>>than waiting for Xilinx? Another win for the platform over hardcoded
>>>CPU. Perhaps a new tool might be added to the toolchain which would
>>>profile the code, report candidates for porting to instructions, and
>>>recompile once the new instruction dependencies were satisfied in
>>>Verilog.
>>>
>>>
>>>On Mon, 2004-05-17 at 22:30, John Williams wrote:
>>> 
>>>
>>>      
>>>
>>>>Hi folks,
>>>>
>>>>The latest version of the microblaze core in EDK6.2 SP1 
>>>>(microblaze_v2_10_a) has optional support for a new pair of instructions:
>>>>
>>>>msrset rd, IMM  and   msrclr rd, IMM
>>>>
>>>>this pair of instructions are basically an atomic test-and-set on the 
>>>>microblaze status register.  We lobbied Xilinx to add these on the 
>>>>grounds of the code size reduction:
>>>>
>>>>mfs	rd, rmsr
>>>>andi	rd, rd, ~2
>>>>mts	rmsr, rd
>>>>
>>>>becomes
>>>>
>>>>msrclr	r0, ~2
>>>>
>>>>to clear interrupts, and similarly for enable interrupts (and caches 
>>>>too).  Also you no longer need a scratch register when doing these 
>>>>manipulations on the MSR.
>>>>
>>>>Even better (well, in fact by design!), the msrclr instruction maps 
>>>>directly onto the very common save_flags_cli() macro, that is sprinkled 
>>>>liberally throughout the kernel and drivers.
>>>>
>>>>The attached patch (David or Greg would you please apply?) adds optional 
>>>>support for these new instructions.  It is selectable from the 
>>>>configuration menu (processor options, in kernel config).
>>>>
>>>>Using this option saves roughly 15kbytes of code in a typical kernel 
>>>>image, or about 1.5 percent.  I expect there's a small performance 
>>>>improvement as well, but haven't tried to measure it yet.
>>>>
>>>>The only catch is that a gremlin in EDK6.2 (and also SP1 alas) means 
>>>>that you have to twiddle your microblaze.mpd file to allow you to set 
>>>>the C_USE_MSR_INSTR parameter.  It's trivial:
>>>>
>>>>edit
>>>>/edk6.2/hw/XilinxProcessorIPLib/pcores/microblaze_v2_10_a/data/microblaze_v2_1_0.mpd
>>>>
>>>>add a new line (somewhere around line 50, among the other PARAMETER 
>>>>declarations, doesn't matter):
>>>>
>>>>PARAMETER C_USE_MSR_INSTR = 0, DT = integer
>>>>
>>>>Finally, edit the system.mhs file of your favourite mbvanilla target, 
>>>>and add the following in the microblaze_0 block:
>>>>
>>>>PARAMETER C_USE_MSR_INSTR = 1
>>>>
>>>>Enable support in the kernel as described above, make dep etc, and off 
>>>>you go!
>>>>
>>>>Congratulations if you are still reading!  Any feedback welcome.
>>>>
>>>>Cheers,
>>>>
>>>>John
>>>>
>>>>______________________________________________________________________
>>>>Index: include/asm-microblaze/system.h
>>>>===================================================================
>>>>RCS file: /var/cvs/uClinux-2.4.x/include/asm-microblaze/system.h,v
>>>>retrieving revision 1.5
>>>>diff -u -b -B -w -p -r1.5 system.h
>>>>--- include/asm-microblaze/system.h	10 Dec 2003 02:39:03 -0000	1.5
>>>>+++ include/asm-microblaze/system.h	18 May 2004 02:14:38 -0000
>>>>@@ -37,10 +37,46 @@ extern void *switch_thread (struct threa
>>>>	}								      \
>>>>  } while (0)
>>>>
>>>>+#ifdef CONFIG_MICROBLAZE_MSRSETCLR
>>>>+/* the msrset/msrclr instructions are available, let's use them! */
>>>>
>>>>/* Enable/disable interrupts.  */
>>>>#define __sti() \
>>>>{								\
>>>>+  __asm__ __volatile__ ("msrset	r0, 0x2;"			\
>>>>+			: 					\
>>>>+			:					\
>>>>+			: "memory");				\
>>>>+}
>>>>+
>>>>+#define __cli() 						\
>>>>+{								\
>>>>+  __asm__ __volatile__ ("msrclr r0, 0x2;"			\
>>>>+			: 					\
>>>>+			:					\
>>>>+			: "memory");				\
>>>>+}
>>>>+
>>>>+#define	__save_flags_cli(flags) 				\
>>>>+{								\
>>>>+  	__asm__ __volatile__ ("msrclr %0, 0x2;"			\
>>>>+			: "=r" (flags)				\
>>>>+			: 					\
>>>>+			: "memory");				\
>>>>+}
>>>>+
>>>>+#define	__save_flags_sti(flags) 				\
>>>>+{								\
>>>>+  	__asm__ __volatile__ ("msrset %0, 0x2;"			\
>>>>+			: "=r" (flags)				\
>>>>+			:					\
>>>>+			: "memory");				\
>>>>+}
>>>>+
>>>>+#else /* MICROBLAZE_CONFIG_MSRSETCLR not set */
>>>>+/* Enable/disable interrupts.  */
>>>>+#define __sti() \
>>>>+{								\
>>>>	register unsigned tmp;					\
>>>>  __asm__ __volatile__ ("					\
>>>>			mfs 	%0, rmsr;			\
>>>>@@ -63,11 +99,6 @@ extern void *switch_thread (struct threa
>>>>			: "memory");				\
>>>>}
>>>>
>>>>-#define __save_flags(flags) \
>>>>-  __asm__ __volatile__ ("mfs	%0, rmsr" : "=r" (flags))
>>>>-#define __restore_flags(flags) \
>>>>-  __asm__ __volatile__ ("mts	rmsr, %0" :: "r" (flags))
>>>>-
>>>>#define	__save_flags_cli(flags) \
>>>>{								\
>>>>	register unsigned tmp;					\
>>>>@@ -91,6 +122,13 @@ extern void *switch_thread (struct threa
>>>>			:					\
>>>>			: "memory");				\
>>>>}
>>>>+
>>>>+#endif
>>>>+
>>>>+#define __save_flags(flags) \
>>>>+  __asm__ __volatile__ ("mfs	%0, rmsr" : "=r" (flags))
>>>>+#define __restore_flags(flags) \
>>>>+  __asm__ __volatile__ ("mts	rmsr, %0" :: "r" (flags))
>>>>
>>>>/* For spinlocks etc */
>>>>#define local_irq_save(flags)	__save_flags_cli (flags)
>>>>Index: include/asm-microblaze/cache.h
>>>>===================================================================
>>>>RCS file: /var/cvs/uClinux-2.4.x/include/asm-microblaze/cache.h,v
>>>>retrieving revision 1.3
>>>>diff -u -b -B -w -p -r1.3 cache.h
>>>>--- include/asm-microblaze/cache.h	22 Sep 2003 04:34:25 -0000	1.3
>>>>+++ include/asm-microblaze/cache.h	18 May 2004 02:14:41 -0000
>>>>@@ -24,7 +24,23 @@
>>>>#define ICACHE_MSR_BIT (1 << 5)
>>>>#define DCACHE_MSR_BIT (1 << 7)
>>>>
>>>>-#ifdef CONFIG_MICROBLAZE_ICACHE
>>>>+#ifdef CONFIG_MICROBLAZE_ICACHE		/* Cache support? */
>>>>+
>>>>+#ifdef CONFIG_MICROBLAZE_MSRSETCLR
>>>>+#define __enable_icache()						\
>>>>+	__asm__ __volatile__ ("	msrset	r0, %0;"			\
>>>>+				: 					\
>>>>+				: "i" (ICACHE_MSR_BIT)			\
>>>>+				: "memory")
>>>>+
>>>>+#define __disable_icache()						\
>>>>+	__asm__ __volatile__ ("	msrclr r0, %0;"				\
>>>>+				: 					\
>>>>+				: "i" (ICACHE_MSR_BIT)			\
>>>>+				: "memory")
>>>>+
>>>>+
>>>>+#else /* !CONFIG_MICROBLAZE_MSRSETCLR */
>>>>#define __enable_icache()						\
>>>>	__asm__ __volatile__ ("						\
>>>>				mfs	r12, rmsr;			\
>>>>@@ -44,6 +60,8 @@
>>>>				: "memory", "r12")
>>>>
>>>> 
>>>>+#endif /* CONFIG_MICROBLAZE_MSRSETCLR */
>>>>+
>>>>#define __invalidate_icache(addr)					\
>>>>	__asm__ __volatile__ ("						\
>>>>				wic	%0, r0"				\
>>>>@@ -56,6 +74,21 @@
>>>>#endif
>>>>
>>>>#ifdef CONFIG_MICROBLAZE_DCACHE
>>>>+
>>>>+#ifdef CONFIG_MICROBLAZE_MSRSETCLR
>>>>+#define __enable_dcache()						\
>>>>+	__asm__ __volatile__ (" msrset	r0, %0;"			\
>>>>+				: 					\
>>>>+				: "i" (DCACHE_MSR_BIT)			\
>>>>+				: "memory")
>>>>+
>>>>+#define __disable_dcache()						\
>>>>+	__asm__ __volatile__ (" msrclr	r0, %0;"			\
>>>>+				: 					\
>>>>+				: "i" (DCACHE_MSR_BIT)			\
>>>>+				: "memory")
>>>>+
>>>>+#else /* !CONFIG_MICROBLAZE_MSRSETCLR */
>>>>#define __enable_dcache()						\
>>>>	__asm__ __volatile__ ("						\
>>>>				mfs	r12, rmsr;			\
>>>>@@ -73,6 +106,8 @@
>>>>				: 					\
>>>>				: "i" (DCACHE_MSR_BIT)			\
>>>>				: "memory", "r12")
>>>>+
>>>>+#endif /* CONFIG_MICROBLAZE_MSRSETCLR */
>>>>
>>>>#define __invalidate_dcache(addr)					\
>>>>	__asm__ __volatile__ ("						\
>>>>Index: arch/microblaze/kernel/entry.S
>>>>===================================================================
>>>>RCS file: /var/cvs/uClinux-2.4.x/arch/microblaze/kernel/entry.S,v
>>>>retrieving revision 1.7
>>>>diff -u -b -B -w -p -r1.7 entry.S
>>>>--- arch/microblaze/kernel/entry.S	4 Mar 2004 03:47:35 -0000	1.7
>>>>+++ arch/microblaze/kernel/entry.S	18 May 2004 02:14:51 -0000
>>>>@@ -31,6 +31,25 @@
>>>>/* The offset of the struct pt_regs in a `state save frame' on the stack.  */
>>>>#define PTO	STATE_SAVE_PT_OFFSET
>>>>
>>>>+/* Standard way of enabling and disabling interrupts in entry.S */
>>>>+#ifdef CONFIG_MICROBLAZE_MSRSETCLR
>>>>+#define ENTRY_CLI(scratch_reg)                                                \
>>>>+	msrclr scratch_reg, 0x2;
>>>>+
>>>>+#define ENTRY_EI(scratch_reg)                                                 \
>>>>+	msrset scratch_reg, 0x2;
>>>>+#else
>>>>+#define ENTRY_CLI(scratch_reg)                                                \
>>>>+        mfs     scratch_reg, rmsr;                                            \
>>>>+        andi    scratch_reg, scratch_reg, ~2;                                 \
>>>>+        mts     rmsr, scratch_reg;
>>>>+
>>>>+#define ENTRY_EI(scratch_reg)                                                 \
>>>>+	mfs     scratch_reg, rmsr;                                            \
>>>>+	ori     scratch_reg, scratch_reg, 2;                                  \
>>>>+	mts     rmsr, scratch_reg;
>>>>+#endif
>>>>+
>>>>/* Standard macros to save and restore regs in the pt_regs struct pointed
>>>>   to by the stack pointer (r1) */
>>>>#define SAVE_REG(reg_num) \
>>>>@@ -469,9 +488,7 @@
>>>>   the C compiler are restored (that is, R1(sp), R2(gp), R15(lp), and
>>>>   anything restored by EXTRA_STATE_RESTORER).  */
>>>>#define RETURN(type)							      \
>>>>-	mfs	r11, rmsr;		/* Disable interrupts */	      \
>>>>-	andi	r11, r11, ~2;						      \
>>>>-	mts	rmsr, r11;						      \
>>>>+	ENTRY_CLI(r11);							      \
>>>>        lwi	r11, r1, PTO+PT_KERNEL_MODE;				      \
>>>>	bnei	r11, 2f;		/* See if returning to kernel mode, */\
>>>>					/* ... if so, skip resched &c.  */    \
>>>>@@ -511,9 +528,7 @@
>>>>3:	SAVE_EXTRA_STATE_FOR_FUNCALL(type); /* Prepare for funcall. */	      \
>>>>	bralid	r15, CSYM(schedule);	/* Call scheduler */		      \
>>>>	nop;				/* delay slot */		      \
>>>>-	mfs	r11, rmsr;	/* The scheduler enables interrupts */	      \
>>>>-	andi	r11, r11, ~2;						      \
>>>>-	mts	rmsr, r11;						      \
>>>>+	ENTRY_CLI(r11);		/* The scheduler enables interrupts */	      \
>>>>	RESTORE_EXTRA_STATE_FOR_FUNCALL(type);				      \
>>>>	brid	5b;		/* Back to continue return processing */      \
>>>>	nop;								      \
>>>>@@ -532,9 +547,7 @@
>>>>	add	r6, r0, r0;		/* Arg 2: sigset_t *oldset */	      \
>>>>	bralid	r15, CSYM(do_signal);	/* Handle any signals */	      \
>>>>	nop;								      \
>>>>-	mfs	r11, rmsr;	/* Sig handling enables interrupts */	      \
>>>>-	andi	r11, r11, ~2;						      \
>>>>-	mts	rmsr, r11;						      \
>>>>+	ENTRY_CLI(r11);		/* Sig handling enables interrupts */	      \
>>>>        RESTORE_EXTRA_STATE(type);	/* Restore extra regs.  */	      \
>>>>	brid	1b;							      \
>>>>	nop;	
>>>>@@ -584,16 +597,12 @@ CSYM(_shift_temp_loc):
>>>> *   Return value in r3
>>>> */
>>>>G_ENTRY(trap):
>>>>-	mfs	r11, rmsr;		// disable interrupts 
>>>>-	andi	r11, r11, ~2;
>>>>-	mts	rmsr, r11;		
>>>>+	ENTRY_CLI(r11);
>>>>	swi	r1, r0, ENTRY_SP;	// save stack (emulate v850)
>>>>	SAVE_STATE (TRAP, r12, ENTRY_SP) // Save registers. 
>>>>	// No need to enable intrs here because microblaze
>>>>	// traps just implemented as a branch, not a hardware trap
>>>>-	mfs	r11, rmsr;		// Enable interrupts
>>>>-	ori	r11, r11, 2;
>>>>-	mts	rmsr, r11;
>>>>+	ENTRY_EI(r11);
>>>>	la	r15, r0, ret_from_trap-8// where the trap should return
>>>>					// need -8 to adjust for rtsd r15, 8
>>>>
>>>>@@ -813,10 +822,7 @@ END(nmi0)
>>>>G_ENTRY(illegal_instruction):
>>>>	swi	r1, r0, ENTRY_SP;	// Save stack (emulate v850)
>>>>	SAVE_STATE (IRQ, r0, ENTRY_SP)	// Save registers. 
>>>>-	mfs	r5, rmsr;		// Enable interrupts
>>>>-	ori	r5, r5, 2;
>>>>-	mts	rmsr, r5;
>>>>-
>>>>+	ENTRY_EI(r11);
>>>>	addi	r5, r0, SIGILL;		// Arg 0: signal number
>>>>	RETRIEVE_CURRENT_TASK(r6);	// Arg 1: task
>>>>	la	r15, r0, ret_from_irq-8	// where the handler should return
>>>>@@ -836,14 +842,10 @@ G_ENTRY(dbtrap):
>>>>	rtbd	r16, dbtrap_o;
>>>>	nop;
>>>>dbtrap_o:
>>>>-	mfs	r11, rmsr;		// disable interrupts 
>>>>-	andi	r11, r11, ~2;
>>>>-	mts	rmsr, r11;		
>>>>+	ENTRY_CLI(r11);
>>>>	swi	r1, r0, ENTRY_SP;	// Save stack (emulate v850)
>>>>	SAVE_STATE (DBTRAP, r0, ENTRY_SP)// Save registers. 
>>>>-	mfs	r11, rmsr;		// Enable interrupts
>>>>-	ori	r11, r11, 2;
>>>>-	mts	rmsr, r11;
>>>>+	ENTRY_EI(r11);
>>>>
>>>>	// Must insert code to detect illegal traps etc
>>>>
>>>>Index: arch/microblaze/config.in
>>>>===================================================================
>>>>RCS file: /var/cvs/uClinux-2.4.x/arch/microblaze/config.in,v
>>>>retrieving revision 1.10
>>>>diff -u -b -B -w -p -r1.10 config.in
>>>>--- arch/microblaze/config.in	12 Mar 2004 05:27:26 -0000	1.10
>>>>+++ arch/microblaze/config.in	18 May 2004 02:14:52 -0000
>>>>@@ -51,6 +51,7 @@ mainmenu_option next_comment
>>>>  #### Microblaze processor-specific config
>>>>
>>>>  #bool 'Reset Guard' 		CONFIG_RESET_GUARD
>>>>+  bool 'Use msrset/msrclr insns' CONFIG_MICROBLAZE_MSRSETCLR
>>>>  bool 'Hardware Multiplier'    CONFIG_MICROBLAZE_HARD_MULT
>>>>  bool 'Hardware Divider'       CONFIG_MICROBLAZE_HARD_DIV
>>>>  bool 'Hardware Barrel Shift'  CONFIG_MICROBLAZE_HARD_BARREL
>>>>   
>>>>
>>>>        
>>>>
>>___________________________
>>microblaze-uclinux mailing list
>>microblaze-uclinux@itee.uq.edu.au
>>Project Home Page : http://www.itee.uq.edu.au/~jwilliams/mblaze-uclinux
>>Mailing List Archive : http://www.itee.uq.edu.au/~listarch/microblaze-uclinux/
>>    
>>


___________________________
microblaze-uclinux mailing list
microblaze-uclinux@itee.uq.edu.au
Project Home Page : http://www.itee.uq.edu.au/~jwilliams/mblaze-uclinux
Mailing List Archive : http://www.itee.uq.edu.au/~listarch/microblaze-uclinux/
References:
- [microblaze-uclinux] [patch] msrset/msrclr support
  - From: John Williams
- Re: [microblaze-uclinux] [patch] msrset/msrclr support
  - From: Matthew Rubenstein
- Re: [microblaze-uclinux] [patch] msrset/msrclr support
  - From: Goran Bilski
- Re: [microblaze-uclinux] [patch] msrset/msrclr support
  - From: Matthew Rubenstein
Prev by Date: Re: [microblaze-uclinux] [patch] msrset/msrclr support
Next by Date: [microblaze-uclinux] FSL coprocessors vs custom instructions
Previous by thread: Re: [microblaze-uclinux] [patch] msrset/msrclr support
Next by thread: [microblaze-uclinux] FSL coprocessors vs custom instructions
Index(es):
- Date
- Thread