> On Jun 30, 2015, at 6:54 AM, Kugan <kugan.vivekanandara...@linaro.org> wrote:
>
>
> On 29/06/15 21:56, Maxim Kuvyrkov wrote:
>>> On Jun 28, 2015, at 2:28 PM, Kugan <kugan.vivekanandara...@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.vivekanandara...@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 <kug...@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...@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
