| Message ID | 000001ceb34b$ed24bcd0$c76e3670$@arm.com |
|---|---|
| State | New |
| Headers | show |
On 17/09/13 03:16, bin.cheng wrote: > > >> -----Original Message----- >> From: Richard Earnshaw >> Sent: Thursday, September 12, 2013 11:24 PM >> To: Bin Cheng >> Cc: gcc-patches@gcc.gnu.org >> Subject: Re: [PATCH ARM]Extend thumb1_reorg to save more comparison >> instructions >> >> On 18/04/13 06:34, Bin Cheng wrote: >> >> Sorry for the delay, I've been trying to get my head around this one. >> >>> thumb1_reorg-20130417.txt >>> >>> >>> Index: gcc/config/arm/arm.c >>> >> ========================================================== >> ========= >>> --- gcc/config/arm/arm.c (revision 197562) >>> +++ gcc/config/arm/arm.c (working copy) >>> @@ -14026,6 +14026,7 @@ thumb1_reorg (void) >>> rtx set, dest, src; >>> rtx pat, op0; >>> rtx prev, insn = BB_END (bb); >>> + bool insn_clobbered = false; >>> >>> while (insn != BB_HEAD (bb) && DEBUG_INSN_P (insn)) >>> insn = PREV_INSN (insn); >>> @@ -14034,12 +14035,29 @@ thumb1_reorg (void) >>> if (INSN_CODE (insn) != CODE_FOR_cbranchsi4_insn) >>> continue; >>> >>> - /* Find the first non-note insn before INSN in basic block BB. > */ >>> + /* Get the register with which we are comparing. */ >>> + pat = PATTERN (insn); >>> + op0 = XEXP (XEXP (SET_SRC (pat), 0), 0); >>> + >>> + /* Find the first flag setting insn before INSN in basic block >>> + BB. */ >>> gcc_assert (insn != BB_HEAD (bb)); >>> - prev = PREV_INSN (insn); >>> - while (prev != BB_HEAD (bb) && (NOTE_P (prev) || DEBUG_INSN_P >> (prev))) >>> - prev = PREV_INSN (prev); >>> + for (prev = PREV_INSN (insn); >>> + (!insn_clobbered >>> + && prev != BB_HEAD (bb) >>> + && (NOTE_P (prev) >>> + || DEBUG_INSN_P (prev) >>> + || (GET_CODE (prev) == SET >> >> This can't be right. prev is an insn of some form, so the test that it is > a SET will >> always fail. >> >> What you need to do here is to initialize 'set' to null before the loop > and then >> have something like >> >> || ((set = single_set (prev)) != NULL >> >>> + && get_attr_conds (prev) == CONDS_NOCOND))); >>> + prev = PREV_INSN (prev)) >>> + { >>> + if (reg_set_p (op0, prev)) >>> + insn_clobbered = true; >>> + } >>> >>> + /* Skip if op0 is clobbered by insn other than prev. */ >>> + if (insn_clobbered) >>> + continue; >>> + >>> set = single_set (prev); >> >> This now becomes redundant and ... >> >>> if (!set) >>> continue; >> >> This will be based on the set you extracted above. >> > > Hi Richard, here is the updated patch according to your comments. Tested on > thumb1, please review. > OK. R.
Index: gcc/config/arm/arm.c =================================================================== --- gcc/config/arm/arm.c (revision 202599) +++ gcc/config/arm/arm.c (working copy) @@ -14265,9 +14265,10 @@ thumb1_reorg (void) FOR_EACH_BB (bb) { - rtx set, dest, src; - rtx pat, op0; + rtx dest, src; + rtx pat, op0, set = NULL; rtx prev, insn = BB_END (bb); + bool insn_clobbered = false; while (insn != BB_HEAD (bb) && DEBUG_INSN_P (insn)) insn = PREV_INSN (insn); @@ -14276,13 +14277,29 @@ thumb1_reorg (void) if (INSN_CODE (insn) != CODE_FOR_cbranchsi4_insn) continue; - /* Find the first non-note insn before INSN in basic block BB. */ + /* Get the register with which we are comparing. */ + pat = PATTERN (insn); + op0 = XEXP (XEXP (SET_SRC (pat), 0), 0); + + /* Find the first flag setting insn before INSN in basic block BB. */ gcc_assert (insn != BB_HEAD (bb)); - prev = PREV_INSN (insn); - while (prev != BB_HEAD (bb) && (NOTE_P (prev) || DEBUG_INSN_P (prev))) - prev = PREV_INSN (prev); + for (prev = PREV_INSN (insn); + (!insn_clobbered + && prev != BB_HEAD (bb) + && (NOTE_P (prev) + || DEBUG_INSN_P (prev) + || ((set = single_set (prev)) != NULL + && get_attr_conds (prev) == CONDS_NOCOND))); + prev = PREV_INSN (prev)) + { + if (reg_set_p (op0, prev)) + insn_clobbered = true; + } - set = single_set (prev); + /* Skip if op0 is clobbered by insn other than prev. */ + if (insn_clobbered) + continue; + if (!set) continue; @@ -14292,12 +14309,9 @@ thumb1_reorg (void) || !low_register_operand (src, SImode)) continue; - pat = PATTERN (insn); - op0 = XEXP (XEXP (SET_SRC (pat), 0), 0); /* Rewrite move into subtract of 0 if its operand is compared with ZERO - in INSN. Don't need to check dest since cprop_hardreg pass propagates - src into INSN. */ - if (REGNO (op0) == REGNO (src)) + in INSN. Both src and dest of the move insn are checked. */ + if (REGNO (op0) == REGNO (src) || REGNO (op0) == REGNO (dest)) { dest = copy_rtx (dest); src = copy_rtx (src);