This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH, alpha]: Remove some_operand and some_ni_operand
- From: Richard Henderson <rth at redhat dot com>
- To: Uros Bizjak <ubizjak at gmail dot com>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Thu, 14 May 2015 11:06:31 -0700
- Subject: Re: [PATCH, alpha]: Remove some_operand and some_ni_operand
- Authentication-results: sourceware.org; auth=none
- References: <CAFULd4a1apauTA_Ug=xacQAVbz2KNGX2_STtx9WEHcx0PTOe3g at mail dot gmail dot com> <55539D8E dot 8020202 at redhat dot com> <CAFULd4bsxE_cEZgEOoZAH3Cqby_oAgqfktu+iGwt9cWVDDdyuQ at mail dot gmail dot com> <CAFULd4aHEeCqku--Zavsc1L5W9cQjToiEQOF-YR7HycfKfvpnw at mail dot gmail dot com>
On 05/14/2015 04:40 AM, Uros Bizjak wrote:
> On Thu, May 14, 2015 at 1:31 PM, Uros Bizjak <email@example.com> wrote:
>> On Wed, May 13, 2015 at 8:53 PM, Richard Henderson <firstname.lastname@example.org> wrote:
>>> On 05/13/2015 11:11 AM, Uros Bizjak wrote:
>>>> We can use general_operand instead of some_operand.
>>>> 2015-05-13 Uros Bizjak <email@example.com>
>>>> * config/alpha/alpha.md (extendqidi2): Use general_operand
>>>> instead of some_operand for operand predicate.
>>>> (extendhidi2): Ditto.
>>>> (cbranchdi4): Use general_operand instead of some_operand
>>>> for operand and operands predicates.
>>>> (cstoredi4): Ditto.
>>>> * config/alpha/predicates.md (some_operand): Remove unused predicate.
>>>> (some_ni_operand): Ditto.
>>>> Tested on alpha-linux-gnu.
>>>> Richard, does this look OK to you, or is there any other reason that
>>>> general_operand predicates were not used here?
>>> For the extensions, it was put in by Kenner in 1997 (90f6b60d), to improve code
>>> for unaligned memories. That code was removed in 2011 by me (8b2983a3), so I
>>> think dropping some_operand there is fine.
>>> For the conditionals, it was added in 2004 by me (62350d6c), and that code is
>>> still there. Specifically,
>>> @@ -3177,11 +3177,17 @@ alpha_emit_conditional_branch (enum rtx_code code)
>>> cmp_code = NIL, branch_code = code;
>>> /* If the constants doesn't fit into an immediate, but can
>>> be generated by lda/ldah, we adjust the argument and
>>> compare against zero, so we can use beq/bne directly. */
>>> - else if (GET_CODE (op1) == CONST_INT && (code == EQ || code == NE))
>>> + /* ??? Don't do this when comparing against symbols, otherwise
>>> + we'll reduce (&x == 0x1234) to (&x-0x1234 == 0), which will
>>> + be declared false out of hand (at least for non-weak). */
>>> + else if (GET_CODE (op1) == CONST_INT
>>> + && (code == EQ || code == NE)
>>> + && !(symbolic_operand (op0, VOIDmode)
>>> + || (GET_CODE (op0) == REG && REG_POINTER (op0))))
>>> If I didn't use some_operand, the SYMBOL_REF would be lowered and we'll only
>>> see a REG here. Searching the mail archive I find
>>> pointing to the test case gcc.dg/20040123-1.c
>>> Perhaps debugging that testcase to see what's reaching a_e_c_b in these modern
>>> times will tell you what's most appropriate.
>> Both, patched and unpatched compiler generate:
>> ;; Generating RTL for gimple basic block 2
>> ;; if (&a == 16384B)
>> (insn 5 4 6 (set (reg/f:DI 70)
>> (symbol_ref:DI ("a") [flags 0x40] <var_decl 0x200006f8360
>> a>)) 20040123-1.c:10 -1
>> (insn 6 5 7 (set (reg:DI 71)
>> (const_int 16384 [0x4000])) 20040123-1.c:10 -1
>> (insn 7 6 8 (set (reg:DI 72)
>> (eq:DI (reg/f:DI 70)
>> (reg:DI 71))) 20040123-1.c:10 -1
>> (jump_insn 8 7 0 (set (pc)
>> (if_then_else (eq (reg:DI 72)
>> (const_int 0 ))
>> (label_ref 0)
>> (pc))) 20040123-1.c:10 -1
>> (int_list:REG_BR_PROB 9996 (nil)))
>> and gcc.dg/20040123-1.c passes for as long as I remember...
> Bah, pushed send too fast.
> This is what is received by a_e_c_b in both, patched and unpatched case:
> Breakpoint 1, alpha_emit_conditional_branch (operands=0x7fffffffd6e0,
> cmp_mode=DImode) at
> 2508 alpha_emit_conditional_branch (rtx operands, machine_mode cmp_mode)
> (gdb) p debug_rtx (operands)
> (ne (reg/f:DI 70)
> (const_int 16384 [0x4000]))
> $1 = void
> (gdb) p debug_rtx (operands)
> (reg/f:DI 70)
Ok, gotcha -- the REG_POINTER_P section of the test is still triggering. I
suspect that the symbolic_operand test can no longer trigger.
I think the whole patch is ok.