| Message ID | 559212EF.7080301@linaro.org |
|---|---|
| State | New |
| Headers | show |
> On Jun 30, 2015, at 6:54 AM, Kugan <kugan.vivekanandarajah@linaro.org> wrote: > > > On 29/06/15 21:56, Maxim Kuvyrkov wrote: >>> On Jun 28, 2015, at 2:28 PM, Kugan <kugan.vivekanandarajah@linaro.org> wrote: >>> >>> This patch allows setting REG_EQUAL for ZERO_EXTRACT and handle that in >>> cse (where the src for the ZERO_EXTRACT needs to be calculated) >>> >>> Thanks, >>> Kugan >> >>> From 75e746e559ffd21b25542b3db627e3b318118569 Mon Sep 17 00:00:00 2001 >>> From: Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org> >>> Date: Fri, 26 Jun 2015 17:12:07 +1000 >>> Subject: [PATCH 1/2] Allow adding REG_EQUAL for ZERO_EXTRACT >>> >>> --- >>> gcc/ChangeLog | 6 ++++++ >>> gcc/cse.c | 41 ++++++++++++++++++++++++++++++++++++++++- >>> gcc/emit-rtl.c | 3 ++- >>> 3 files changed, 48 insertions(+), 2 deletions(-) >>> >>> diff --git a/gcc/ChangeLog b/gcc/ChangeLog >>> index 080aa39..d4a73d6 100644 >>> --- a/gcc/ChangeLog >>> +++ b/gcc/ChangeLog >>> @@ -1,3 +1,9 @@ >>> +2015-06-26 Kugan Vivekanandarajah <kuganv@linaro.org> >>> + >>> + * cse.c (cse_insn): Calculate src_eqv for ZERO_EXTRACT. >>> + * emit-rtl.c (set_for_reg_notes): Allow ZERO_EXTRACT to set >>> + REG_EQUAL note. >>> + >>> 2015-06-25 H.J. Lu <hongjiu.lu@intel.com> >>> >>> * gentarget-def.c (def_target_insn): Cast return of strtol to >>> diff --git a/gcc/cse.c b/gcc/cse.c >>> index 100c9c8..8add651 100644 >>> --- a/gcc/cse.c >>> +++ b/gcc/cse.c >>> @@ -4531,8 +4531,47 @@ cse_insn (rtx_insn *insn) >>> if (n_sets == 1 && REG_NOTES (insn) != 0 >>> && (tem = find_reg_note (insn, REG_EQUAL, NULL_RTX)) != 0 >>> && (! rtx_equal_p (XEXP (tem, 0), SET_SRC (sets[0].rtl)) >>> + || GET_CODE (SET_DEST (sets[0].rtl)) == ZERO_EXTRACT >>> || GET_CODE (SET_DEST (sets[0].rtl)) == STRICT_LOW_PART)) >>> - src_eqv = copy_rtx (XEXP (tem, 0)); >>> + { >>> + src_eqv = copy_rtx (XEXP (tem, 0)); >>> + >>> + /* If DEST is of the form ZERO_EXTACT, as in: >>> + (set (zero_extract:SI (reg:SI 119) >>> + (const_int 16 [0x10]) >>> + (const_int 16 [0x10])) >>> + (const_int 51154 [0xc7d2])) >>> + REG_EQUAL note will specify the value of register (reg:SI 119) at this >>> + point. Note that this is different from SRC_EQV. We can however >>> + calculate SRC_EQV with the position and width of ZERO_EXTRACT. */ >>> + if (GET_CODE (SET_DEST (sets[0].rtl)) == ZERO_EXTRACT) >> >> Consider changing >> >> if (something >> && (!rtx_equal_p) >> || ZERO_EXTRACT >> || STRICT_LOW_PART) >> >> to >> >> if (something >> && !rtx_equal_p) >> { >> if (ZERO_EXTRACT) >> { >> } >> else if (STRICT_LOW_PART) >> { >> } >> } >> >> Otherwise looks good to me, but you still need another approval. > > Thanks Maxim for the review. How about the attached patch? Looks good, with a couple of indentation nit-picks below. No need to repost the patch on their account. Wait for another a maintainer's review. > --- a/gcc/cse.c > +++ b/gcc/cse.c > @@ -4525,14 +4525,49 @@ cse_insn (rtx_insn *insn) > canonicalize_insn (insn, &sets, n_sets); > > /* If this insn has a REG_EQUAL note, store the equivalent value in SRC_EQV, > - if different, or if the DEST is a STRICT_LOW_PART. The latter condition > - is necessary because SRC_EQV is handled specially for this case, and if > - it isn't set, then there will be no equivalence for the destination. */ > + if different, or if the DEST is a STRICT_LOW_PART/ZERO_EXTRACT. The > + latter condition is necessary because SRC_EQV is handled specially for > + this case, and if it isn't set, then there will be no equivalence > + for the destination. */ > if (n_sets == 1 && REG_NOTES (insn) != 0 > - && (tem = find_reg_note (insn, REG_EQUAL, NULL_RTX)) != 0 > - && (! rtx_equal_p (XEXP (tem, 0), SET_SRC (sets[0].rtl)) > - || GET_CODE (SET_DEST (sets[0].rtl)) == STRICT_LOW_PART)) > - src_eqv = copy_rtx (XEXP (tem, 0)); > + && (tem = find_reg_note (insn, REG_EQUAL, NULL_RTX)) != 0) > + { > + if ((! rtx_equal_p (XEXP (tem, 0), SET_SRC (sets[0].rtl))) > + || GET_CODE (SET_DEST (sets[0].rtl)) == STRICT_LOW_PART) > + src_eqv = copy_rtx (XEXP (tem, 0)); Please double check indentation here. > + > + /* If DEST is of the form ZERO_EXTACT, as in: > + (set (zero_extract:SI (reg:SI 119) > + (const_int 16 [0x10]) > + (const_int 16 [0x10])) > + (const_int 51154 [0xc7d2])) > + REG_EQUAL note will specify the value of register (reg:SI 119) at this > + point. Note that this is different from SRC_EQV. We can however > + calculate SRC_EQV with the position and width of ZERO_EXTRACT. */ > + else if (GET_CODE (SET_DEST (sets[0].rtl)) == ZERO_EXTRACT > + &&CONST_INT_P (src_eqv) Add a space between && and CONST_INT_P. > + && CONST_INT_P (XEXP (SET_DEST (sets[0].rtl), 1)) > + && CONST_INT_P (XEXP (SET_DEST (sets[0].rtl), 2))) > + { > + rtx dest_reg = XEXP (SET_DEST (sets[0].rtl), 0); > + rtx width = XEXP (SET_DEST (sets[0].rtl), 1); > + rtx pos = XEXP (SET_DEST (sets[0].rtl), 2); > + HOST_WIDE_INT val = INTVAL (src_eqv); > + HOST_WIDE_INT mask; > + unsigned int shift; > + if (BITS_BIG_ENDIAN) > + shift = GET_MODE_PRECISION (GET_MODE (dest_reg)) > + - INTVAL (pos) - INTVAL (width); The usual practice is to brace the calculation that spans multiple lines, so that second and subsequent lines are aligned to the right of "=", e.g., shift = (GET_MODE_PRECISION (GET_MODE (dest_reg)) - INTVAL (pos) - INTVAL (width)); > + else > + shift = INTVAL (pos); > + if (INTVAL (width) == HOST_BITS_PER_WIDE_INT) > + mask = ~(HOST_WIDE_INT) 0; > + else > + mask = ((HOST_WIDE_INT) 1 << INTVAL (width)) - 1; > + val = (val >> shift) & mask; > + src_eqv = GEN_INT (val); > + } > + } > -- Maxim Kuvyrkov www.linaro.org
From 3754bcd11fb732e03a39d6625a9eb14934b36643 Mon Sep 17 00:00:00 2001 From: Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org> Date: Fri, 26 Jun 2015 17:12:07 +1000 Subject: [PATCH 1/2] Allow adding REG_EQUAL for ZERO_EXTRACT --- gcc/ChangeLog | 6 ++++++ gcc/cse.c | 49 ++++++++++++++++++++++++++++++++++++++++++------- gcc/emit-rtl.c | 3 ++- 3 files changed, 50 insertions(+), 8 deletions(-) diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 080aa39..d4a73d6 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,9 @@ +2015-06-26 Kugan Vivekanandarajah <kuganv@linaro.org> + + * cse.c (cse_insn): Calculate src_eqv for ZERO_EXTRACT. + * emit-rtl.c (set_for_reg_notes): Allow ZERO_EXTRACT to set + REG_EQUAL note. + 2015-06-25 H.J. Lu <hongjiu.lu@intel.com> * gentarget-def.c (def_target_insn): Cast return of strtol to diff --git a/gcc/cse.c b/gcc/cse.c index 100c9c8..ea9e989 100644 --- a/gcc/cse.c +++ b/gcc/cse.c @@ -4525,14 +4525,49 @@ cse_insn (rtx_insn *insn) canonicalize_insn (insn, &sets, n_sets); /* If this insn has a REG_EQUAL note, store the equivalent value in SRC_EQV, - if different, or if the DEST is a STRICT_LOW_PART. The latter condition - is necessary because SRC_EQV is handled specially for this case, and if - it isn't set, then there will be no equivalence for the destination. */ + if different, or if the DEST is a STRICT_LOW_PART/ZERO_EXTRACT. The + latter condition is necessary because SRC_EQV is handled specially for + this case, and if it isn't set, then there will be no equivalence + for the destination. */ if (n_sets == 1 && REG_NOTES (insn) != 0 - && (tem = find_reg_note (insn, REG_EQUAL, NULL_RTX)) != 0 - && (! rtx_equal_p (XEXP (tem, 0), SET_SRC (sets[0].rtl)) - || GET_CODE (SET_DEST (sets[0].rtl)) == STRICT_LOW_PART)) - src_eqv = copy_rtx (XEXP (tem, 0)); + && (tem = find_reg_note (insn, REG_EQUAL, NULL_RTX)) != 0) + { + if ((! rtx_equal_p (XEXP (tem, 0), SET_SRC (sets[0].rtl))) + || GET_CODE (SET_DEST (sets[0].rtl)) == STRICT_LOW_PART) + src_eqv = copy_rtx (XEXP (tem, 0)); + + /* If DEST is of the form ZERO_EXTACT, as in: + (set (zero_extract:SI (reg:SI 119) + (const_int 16 [0x10]) + (const_int 16 [0x10])) + (const_int 51154 [0xc7d2])) + REG_EQUAL note will specify the value of register (reg:SI 119) at this + point. Note that this is different from SRC_EQV. We can however + calculate SRC_EQV with the position and width of ZERO_EXTRACT. */ + else if (GET_CODE (SET_DEST (sets[0].rtl)) == ZERO_EXTRACT + &&CONST_INT_P (src_eqv) + && CONST_INT_P (XEXP (SET_DEST (sets[0].rtl), 1)) + && CONST_INT_P (XEXP (SET_DEST (sets[0].rtl), 2))) + { + rtx dest_reg = XEXP (SET_DEST (sets[0].rtl), 0); + rtx width = XEXP (SET_DEST (sets[0].rtl), 1); + rtx pos = XEXP (SET_DEST (sets[0].rtl), 2); + HOST_WIDE_INT val = INTVAL (src_eqv); + HOST_WIDE_INT mask; + unsigned int shift; + if (BITS_BIG_ENDIAN) + shift = GET_MODE_PRECISION (GET_MODE (dest_reg)) + - INTVAL (pos) - INTVAL (width); + else + shift = INTVAL (pos); + if (INTVAL (width) == HOST_BITS_PER_WIDE_INT) + mask = ~(HOST_WIDE_INT) 0; + else + mask = ((HOST_WIDE_INT) 1 << INTVAL (width)) - 1; + val = (val >> shift) & mask; + src_eqv = GEN_INT (val); + } + } /* Set sets[i].src_elt to the class each source belongs to. Detect assignments from or to volatile things diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c index e7f7eab..cb891b1 100644 --- a/gcc/emit-rtl.c +++ b/gcc/emit-rtl.c @@ -5228,7 +5228,8 @@ set_for_reg_notes (rtx insn) reg = SET_DEST (pat); /* Notes apply to the contents of a STRICT_LOW_PART. */ - if (GET_CODE (reg) == STRICT_LOW_PART) + if (GET_CODE (reg) == STRICT_LOW_PART + || GET_CODE (reg) == ZERO_EXTRACT) reg = XEXP (reg, 0); /* Check that we have a register. */ -- 1.9.1