| Message ID | 37378DC5BCD0EE48BA4B082E0B55DFAA429ED1AB@XAP-PVEXMBX02.xlnx.xilinx.com |
|---|---|
| State | New |
| Headers | show |
On 01/29/2016 02:31 AM, Ajit Kumar Agarwal wrote: > > This patch improves the allocation of registers in the given function. The allocation > is optimized for the conditional branches. The temporary register used in the > conditional branches to store the comparison results and use of temporary in the > conditional branch is optimized. Such temporary registers are allocated with a fixed > register r18. > > Currently such temporaries are allocated with a free registers in the given > function. Due to this one of the free register is reserved for the temporaries and > given function is left with a few registers. This is unoptimized with respect to > microblaze. In Microblaze r18 is marked as fixed and cannot be allocated to pseudos' > in the given function. Instead r18 can be used as a temporary for the conditional > branches with compare and branch. Use of r18 as a temporary for conditional branches > will save one of the free registers to be allocated. The free registers can be used > for other pseudos' and hence the better register allocation. > > The usage of r18 as above reduces the spill and fetch because of the availability of > one of the free registers to other pseudos instead of being used for conditional > temporaries. > > The advantage of the above is that the scope of the temporaries is limited to the > conditional branches and hence the usage of r18 as temporary for such conditional > branches is optimized and preserve the functionality of the function. > > Regtested for Microblaze target. > > Performance runs are done with Mibench/EEMBC benchmarks. > > Following gains are achieved. > > Benchmarks Gains > > automotive_qsort1 1.630730524% > network_dijkstra 1.527506256% > office_stringsearch 1 1.81356288% > security_rijndael_d 3.26129357% > basefp01_lite 4.465120185% > a2time01_lite 1.893862857% > cjpeg_lite 3.286496675% > djpeg_lite 3.120150612% > qos_lite 2.63964381% > office_ispell 1.531340405% > > Code Size improvements: > > Reduction in number of instructions for Mibench : 12927. > Reduction in number of instructions for EEMBC : 212. > > ChangeLog: > 2016-01-29 Ajit Agarwal <ajitkum@xilinx.com> > > * config/microblaze/microblaze.c > (microblaze_expand_conditional_branch): Use of MB_ABI_ASM_TEMP_REGNUM > for temporary conditional branch. > (microblaze_expand_conditional_branch_reg): Use of MB_ABI_ASM_TEMP_REGNUM > for temporary conditional branch. > (microblaze_expand_conditional_branch_sf): Use of MB_ABI_ASM_TEMP_REGNUM > for temporary conditional branch. You can combine these ChangeLog entries: * config/microblaze/microblaze.c (microblaze_expand_conditional_branch, microblaze_expand_conditional_branch_reg, microblaze_expand_conditional_branch_sf): Use MB_ABI_ASM_TEMP_REGNUM for temp reg. Otherwise, OK. > > Signed-off-by:Ajit Agarwal ajitkum@xilinx.com. > --- > gcc/config/microblaze/microblaze.c | 6 +++--- > 1 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/gcc/config/microblaze/microblaze.c b/gcc/config/microblaze/microblaze.c > index baff67a..b4277ad 100644 > --- a/gcc/config/microblaze/microblaze.c > +++ b/gcc/config/microblaze/microblaze.c > @@ -3402,7 +3402,7 @@ microblaze_expand_conditional_branch (machine_mode mode, rtx operands[]) > rtx cmp_op0 = operands[1]; > rtx cmp_op1 = operands[2]; > rtx label1 = operands[3]; > - rtx comp_reg = gen_reg_rtx (SImode); > + rtx comp_reg = gen_rtx_REG (SImode, MB_ABI_ASM_TEMP_REGNUM); > rtx condition; > > gcc_assert ((GET_CODE (cmp_op0) == REG) || (GET_CODE (cmp_op0) == SUBREG)); > @@ -3439,7 +3439,7 @@ microblaze_expand_conditional_branch_reg (enum machine_mode mode, > rtx cmp_op0 = operands[1]; > rtx cmp_op1 = operands[2]; > rtx label1 = operands[3]; > - rtx comp_reg = gen_reg_rtx (SImode); > + rtx comp_reg = gen_rtx_REG (SImode, MB_ABI_ASM_TEMP_REGNUM); > rtx condition; > > gcc_assert ((GET_CODE (cmp_op0) == REG) > @@ -3483,7 +3483,7 @@ microblaze_expand_conditional_branch_sf (rtx operands[]) > rtx condition; > rtx cmp_op0 = XEXP (operands[0], 0); > rtx cmp_op1 = XEXP (operands[0], 1); > - rtx comp_reg = gen_reg_rtx (SImode); > + rtx comp_reg = gen_rtx_REG (SImode, MB_ABI_ASM_TEMP_REGNUM); > > emit_insn (gen_cstoresf4 (comp_reg, operands[0], cmp_op0, cmp_op1)); > condition = gen_rtx_NE (SImode, comp_reg, const0_rtx); >
-----Original Message----- From: Michael Eager [mailto:eager@eagerm.com] Sent: Friday, January 29, 2016 11:33 PM To: Ajit Kumar Agarwal; GCC Patches Cc: Vinod Kathail; Shail Aditya Gupta; Vidhumouli Hunsigida; Nagaraju Mekala Subject: Re: [Patch,microblaze]: Better register allocation to minimize the spill and fetch. On 01/29/2016 02:31 AM, Ajit Kumar Agarwal wrote: > > This patch improves the allocation of registers in the given function. > The allocation is optimized for the conditional branches. The > temporary register used in the conditional branches to store the > comparison results and use of temporary in the conditional branch is > optimized. Such temporary registers are allocated with a fixed register r18. > > Currently such temporaries are allocated with a free registers in the > given function. Due to this one of the free register is reserved for > the temporaries and given function is left with a few registers. This > is unoptimized with respect to microblaze. In Microblaze r18 is marked as fixed and cannot be allocated to pseudos' > in the given function. Instead r18 can be used as a temporary for the > conditional branches with compare and branch. Use of r18 as a > temporary for conditional branches will save one of the free registers > to be allocated. The free registers can be used for other pseudos' and hence the better register allocation. > > The usage of r18 as above reduces the spill and fetch because of the > availability of one of the free registers to other pseudos instead of > being used for conditional temporaries. > > The advantage of the above is that the scope of the temporaries is > limited to the conditional branches and hence the usage of r18 as > temporary for such conditional branches is optimized and preserve the functionality of the function. > > Regtested for Microblaze target. > > Performance runs are done with Mibench/EEMBC benchmarks. > > Following gains are achieved. > > Benchmarks Gains > > automotive_qsort1 1.630730524% > network_dijkstra 1.527506256% > office_stringsearch 1 1.81356288% > security_rijndael_d 3.26129357% > basefp01_lite 4.465120185% > a2time01_lite 1.893862857% > cjpeg_lite 3.286496675% > djpeg_lite 3.120150612% > qos_lite 2.63964381% > office_ispell 1.531340405% > > Code Size improvements: > > Reduction in number of instructions for Mibench : 12927. > Reduction in number of instructions for EEMBC : 212. > > ChangeLog: > 2016-01-29 Ajit Agarwal <ajitkum@xilinx.com> > > * config/microblaze/microblaze.c > (microblaze_expand_conditional_branch): Use of MB_ABI_ASM_TEMP_REGNUM > for temporary conditional branch. > (microblaze_expand_conditional_branch_reg): Use of MB_ABI_ASM_TEMP_REGNUM > for temporary conditional branch. > (microblaze_expand_conditional_branch_sf): Use of MB_ABI_ASM_TEMP_REGNUM > for temporary conditional branch. >>You can combine these ChangeLog entries: >> * config/microblaze/microblaze.c >> (microblaze_expand_conditional_branch, microblaze_expand_conditional_branch_reg, >>microblaze_expand_conditional_branch_sf): Use MB_ABI_ASM_TEMP_REGNUM for temp reg. Attached is the patch with update in the changeLog. Modified changeLog is given below. ChangeLog: 2016-01-29 Ajit Agarwal <ajitkum@xilinx.com> * config/microblaze/microblaze.c (microblaze_expand_conditional_branch, microblaze_expand_conditional_branch_reg, microblaze_expand_conditional_branch_sf): Use MB_ABI_ASM_TEMP_REGNUM for temp reg. Thanks & Regards Ajit >>Otherwise, OK. > > Signed-off-by:Ajit Agarwal ajitkum@xilinx.com. > --- > gcc/config/microblaze/microblaze.c | 6 +++--- > 1 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/gcc/config/microblaze/microblaze.c > b/gcc/config/microblaze/microblaze.c > index baff67a..b4277ad 100644 > --- a/gcc/config/microblaze/microblaze.c > +++ b/gcc/config/microblaze/microblaze.c > @@ -3402,7 +3402,7 @@ microblaze_expand_conditional_branch (machine_mode mode, rtx operands[]) > rtx cmp_op0 = operands[1]; > rtx cmp_op1 = operands[2]; > rtx label1 = operands[3]; > - rtx comp_reg = gen_reg_rtx (SImode); > + rtx comp_reg = gen_rtx_REG (SImode, MB_ABI_ASM_TEMP_REGNUM); > rtx condition; > > gcc_assert ((GET_CODE (cmp_op0) == REG) || (GET_CODE (cmp_op0) == > SUBREG)); @@ -3439,7 +3439,7 @@ microblaze_expand_conditional_branch_reg (enum machine_mode mode, > rtx cmp_op0 = operands[1]; > rtx cmp_op1 = operands[2]; > rtx label1 = operands[3]; > - rtx comp_reg = gen_reg_rtx (SImode); > + rtx comp_reg = gen_rtx_REG (SImode, MB_ABI_ASM_TEMP_REGNUM); > rtx condition; > > gcc_assert ((GET_CODE (cmp_op0) == REG) @@ -3483,7 +3483,7 @@ > microblaze_expand_conditional_branch_sf (rtx operands[]) > rtx condition; > rtx cmp_op0 = XEXP (operands[0], 0); > rtx cmp_op1 = XEXP (operands[0], 1); > - rtx comp_reg = gen_reg_rtx (SImode); > + rtx comp_reg = gen_rtx_REG (SImode, MB_ABI_ASM_TEMP_REGNUM); > > emit_insn (gen_cstoresf4 (comp_reg, operands[0], cmp_op0, cmp_op1)); > condition = gen_rtx_NE (SImode, comp_reg, const0_rtx); > -- Michael Eager eager@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
On Jan 29, 2016, at 2:31 AM, Ajit Kumar Agarwal <ajit.kumar.agarwal@xilinx.com> wrote: > > This patch improves the allocation of registers in the given function. Is it just me, or, would it be even better to change the abi and make MB_ABI_ASM_TEMP_REGNUM be allocated by the register allocator?
-----Original Message----- From: Mike Stump [mailto:mikestump@comcast.net] Sent: Tuesday, February 02, 2016 12:12 AM To: Ajit Kumar Agarwal Cc: GCC Patches; Vinod Kathail; Shail Aditya Gupta; Vidhumouli Hunsigida; Nagaraju Mekala Subject: Re: [Patch,microblaze]: Better register allocation to minimize the spill and fetch. On Jan 29, 2016, at 2:31 AM, Ajit Kumar Agarwal <ajit.kumar.agarwal@xilinx.com> wrote: > > This patch improves the allocation of registers in the given function. >>Is it just me, or, would it be even better to change the abi and make MB_ABI_ASM_TEMP_REGNUM be allocated by the register allocator? Yes, it would be even better to make r18 (MB_ABI_ASM_TEMP_REGNUM) to be allocated by the register allocator for the given function. Currently r18 is marked as FIXED_REGISTERS and cannot be allocated to the given function only used for temporaries where the liveness is limited. R18 is used in some of the shift patterns and also for the conditional branches temporaries. I have made some of the ABI changes making r18 as not FIXED_REGISTERS in my local branch and the given function can be allocated with r18.This will reduce the spill and fetch to a great extent and I am seeing good amount of gains with this change for Mibench/EEMBC benchmarks. Since the ABI specifies r18 is reserved for assembler temporaries, lot of kernel driven code and glibc code uses r18 as a temporaries for inline assembly. Changing the ABI requires lot of changes in the kernel specific code for Microblaze where r18 is used as a temporary without storing and fetching after the scope is gone. Currently we don't have plans to change the ABI that might break the existing Kernel and GLIBC code. Changing the ABI require a good amount of work to be done in Kernel and GLIBC code for Microblaze. Thanks & Regards Ajit
diff --git a/gcc/config/microblaze/microblaze.c b/gcc/config/microblaze/microblaze.c index baff67a..b4277ad 100644 --- a/gcc/config/microblaze/microblaze.c +++ b/gcc/config/microblaze/microblaze.c @@ -3402,7 +3402,7 @@ microblaze_expand_conditional_branch (machine_mode mode, rtx operands[]) rtx cmp_op0 = operands[1]; rtx cmp_op1 = operands[2]; rtx label1 = operands[3]; - rtx comp_reg = gen_reg_rtx (SImode); + rtx comp_reg = gen_rtx_REG (SImode, MB_ABI_ASM_TEMP_REGNUM); rtx condition; gcc_assert ((GET_CODE (cmp_op0) == REG) || (GET_CODE (cmp_op0) == SUBREG)); @@ -3439,7 +3439,7 @@ microblaze_expand_conditional_branch_reg (enum machine_mode mode, rtx cmp_op0 = operands[1]; rtx cmp_op1 = operands[2]; rtx label1 = operands[3]; - rtx comp_reg = gen_reg_rtx (SImode); + rtx comp_reg = gen_rtx_REG (SImode, MB_ABI_ASM_TEMP_REGNUM); rtx condition; gcc_assert ((GET_CODE (cmp_op0) == REG) @@ -3483,7 +3483,7 @@ microblaze_expand_conditional_branch_sf (rtx operands[]) rtx condition; rtx cmp_op0 = XEXP (operands[0], 0); rtx cmp_op1 = XEXP (operands[0], 1); - rtx comp_reg = gen_reg_rtx (SImode); + rtx comp_reg = gen_rtx_REG (SImode, MB_ABI_ASM_TEMP_REGNUM); emit_insn (gen_cstoresf4 (comp_reg, operands[0], cmp_op0, cmp_op1)); condition = gen_rtx_NE (SImode, comp_reg, const0_rtx);