Re: [PING][PATCH][AArch64] Implement ALU_BRANCH fusion

2017-06-14 Thread James Greenhalgh
On Thu, Apr 27, 2017 at 05:07:26AM +, Hurugalawadi, Naveen wrote: > Hi Wilco, > > >> You should only return true if there is a match, not if there is > >> not a match. > > Done. > > Bootstrapped and Regression tested on AArch64 and X86_64. > Please review the patch and let us know if its oka

Re: [PING 3] [PATCH] [AArch64] Implement ALU_BRANCH fusion

2017-06-14 Thread Hurugalawadi, Naveen
Hi Wilco, >> That looks good to me now. Thanks for the review and your okay for the patch. Please consider this as a personal reminder to review the patch at following link and let me know if its okay to commit? https://gcc.gnu.org/ml/gcc-patches/2017-04/msg01333.html Thanks, Naveen

Re: [PING 2] [PATCH] [AArch64] Implement ALU_BRANCH fusion

2017-05-26 Thread Wilco Dijkstra
Hurugalawadi, Naveen wrote: > > Please consider this as a personal reminder to review the patch > at following link and let me know your comments on the same.  > > https://gcc.gnu.org/ml/gcc-patches/2017-04/msg01333.html Looks good to me. Wilco   

Re: [PING 2] [PATCH] [AArch64] Implement ALU_BRANCH fusion

2017-05-25 Thread Hurugalawadi, Naveen
Hi,  Please consider this as a personal reminder to review the patch at following link and let me know your comments on the same.  https://gcc.gnu.org/ml/gcc-patches/2017-04/msg01333.html Thanks, Naveen   

Re: [PING] [PATCH] [AArch64] Implement ALU_BRANCH fusion

2017-05-10 Thread Hurugalawadi, Naveen
Hi, Please consider this as a personal reminder to review the patch at following link and let me know your comments on the same. https://gcc.gnu.org/ml/gcc-patches/2017-04/msg01333.html Thanks, Naveen   

Re: [PING][PATCH][AArch64] Implement ALU_BRANCH fusion

2017-04-26 Thread Hurugalawadi, Naveen
Hi Wilco, >> You should only return true if there is a match, not if there is >> not a match. Done. Bootstrapped and Regression tested on AArch64 and X86_64. Please review the patch and let us know if its okay? Thanks, Naveen    diff --git a/gcc/config/aarch64/aarch64-fusion-pairs.d

Re: [PING][PATCH][AArch64] Implement ALU_BRANCH fusion

2017-04-26 Thread Wilco Dijkstra
Hi Naveen, This version has the same issue of claiming that all instructions should be fused except for the cases that can be fused. You should only return true if there is a match, not if there is not a match. Cheers, Wilco   

Re: [PING][PATCH][AArch64] Implement ALU_BRANCH fusion

2017-04-26 Thread Hurugalawadi, Naveen
Hi Wilco, >> Same comment for this part, we want to return true if we match: Thanks for pointing out about the confusion. >> Note writing these complex conditions using positive logic makes them much >> more readable - if you have to negate use !(X && Y && Z) rather than >> !X || !Y || !Z. Modi

Re: [PING][PATCH][AArch64] Implement ALU_BRANCH fusion

2017-04-25 Thread Wilco Dijkstra
Hi Naveen, > https://gcc.gnu.org/ml/gcc-patches/2017-03/msg01369.html Same comment for this part, we want to return true if we match: + if (SET_DEST (curr_set) != (pc_rtx) + || GET_CODE (SET_SRC (curr_set)) != IF_THEN_ELSE + || ! REG_P (XEXP (XEXP (SET_SRC (curr_set), 0), 0)

Re: [PING][PATCH][AArch64] Implement ALU_BRANCH fusion

2017-04-25 Thread Hurugalawadi, Naveen
Hi, Please consider this as a personal reminder to review the patch at following link and let me know your comments on the same. https://gcc.gnu.org/ml/gcc-patches/2017-03/msg01369.html Thanks, Naveen

Re: [PATCH][AArch64] Implement ALU_BRANCH fusion

2017-03-26 Thread Hurugalawadi, Naveen
Hi, Thanks for the review and suggestions. > I think the patch isn't quite complete yet. You will also need changes in > generic code. Currently sched_macro_fuse_insns() does: Modified the sched_macro_fuse_insns() as required. > Basically the idea is to push the check for CC usage into target m

Re: [PATCH][AArch64] Implement ALU_BRANCH fusion

2017-03-20 Thread Andrew Pinski
On Wed, Mar 15, 2017 at 8:20 AM, Wilco Dijkstra wrote: > Hi, > > I think the patch isn't quite complete yet. You will also need changes in > generic code. Currently sched_macro_fuse_insns() does: > > if (any_condjump_p (insn)) > { > unsigned int condreg1, condreg2; > rtx cc_reg_1

Re: [PATCH][AArch64] Implement ALU_BRANCH fusion

2017-03-15 Thread Wilco Dijkstra
Hi, I think the patch isn't quite complete yet. You will also need changes in generic code. Currently sched_macro_fuse_insns() does: if (any_condjump_p (insn)) { unsigned int condreg1, condreg2; rtx cc_reg_1; targetm.fixed_condition_code_regs (&condreg1, &condreg2);

Re: [PATCH][AArch64] Implement ALU_BRANCH fusion

2017-03-15 Thread Hurugalawadi, Naveen
Hi Kyrill, >> I suggest you reword the whole comment and not talk about particular CPUs >> but rather about the kinds of instructions you want to fuse Modified as per the comments. Had modified the earlier version of patch which had the vulcan reservation before James comments. Please find attac

Re: [PATCH][AArch64] Implement ALU_BRANCH fusion

2017-03-15 Thread Kyrill Tkachov
Hi Naveen, On 15/03/17 05:32, Hurugalawadi, Naveen wrote: Hi James, My reason for asking is that the instruction fusion implemented in LLVM ( lib/Target/AArch64/AArch64MacroFusion.cpp::shouldScheduleAdjacent ) Sorry. There seems to be some confusion in the branch instructions. The branch shou

Re: [PATCH][AArch64] Implement ALU_BRANCH fusion

2017-03-14 Thread Hurugalawadi, Naveen
Hi James, >> My reason for asking is that the instruction fusion implemented in LLVM >> ( lib/Target/AArch64/AArch64MacroFusion.cpp::shouldScheduleAdjacent ) Sorry. There seems to be some confusion in the branch instructions. The branch should be conditional for ALU_BRANCH fusion. Please find at

Re: [PATCH][AArch64] Implement ALU_BRANCH fusion

2017-03-09 Thread James Greenhalgh
On Thu, Mar 09, 2017 at 06:22:33AM +, Hurugalawadi, Naveen wrote: > Hi James, > > Thanks for the review and your comments. > > >> I'd need more detail on what types of instruction pairs you > >> are trying to fuse. > > The documentation mentions it as follows:- > Single uop ALU instruction

Re: [PATCH][AArch64] Implement ALU_BRANCH fusion

2017-03-08 Thread Hurugalawadi, Naveen
Hi James, Thanks for the review and your comments. >> I'd need more detail on what types of instruction pairs you >> are trying to fuse. The documentation mentions it as follows:- Single uop ALU instruction may fuse with adjacent branch instruction in the same bundle >> This comment looks inc

Re: [PATCH][AArch64] Implement ALU_BRANCH fusion

2017-03-08 Thread James Greenhalgh
On Mon, Mar 06, 2017 at 05:10:10AM +, Hurugalawadi, Naveen wrote: > Hi, > > Please find attached the patch that implements alu_branch fusion > for AArch64. > The patch doesn't change spec but improve other benchmarks. > > Bootstrapped and Regression tested on aarch64-thunder-linux. > Please r

[PATCH][AArch64] Implement ALU_BRANCH fusion

2017-03-05 Thread Hurugalawadi, Naveen
Hi, Please find attached the patch that implements alu_branch fusion for AArch64. The patch doesn't change spec but improve other benchmarks. Bootstrapped and Regression tested on aarch64-thunder-linux. Please review the patch and let us know if its okay for Stage-1? Thanks, Naveen 2017-03-06