Re: [PATCH, alpha]: Remove some_operand and some_ni_operand

[Uros Bizjak <ubizjak at gmail dot com>]

On Thu, May 14, 2015 at 1:31 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Wed, May 13, 2015 at 8:53 PM, Richard Henderson <rth@redhat.com> 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  <ubizjak@gmail.com>
>>>
>>>     * config/alpha/alpha.md (extendqidi2): Use general_operand
>>>     instead of some_operand for operand[1] predicate.
>>>     (extendhidi2): Ditto.
>>>     (cbranchdi4): Use general_operand instead of some_operand
>>>     for operand[1] and operands[2] 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
>>
>>   https://gcc.gnu.org/ml/gcc-patches/2004-02/msg02436.html
>>
>> 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
>      (nil))
>
> (insn 6 5 7 (set (reg:DI 71)
>         (const_int 16384 [0x4000])) 20040123-1.c:10 -1
>      (nil))
>
> (insn 7 6 8 (set (reg:DI 72)
>         (eq:DI (reg/f:DI 70)
>             (reg:DI 71))) 20040123-1.c:10 -1
>      (nil))
>
> (jump_insn 8 7 0 (set (pc)
>         (if_then_else (eq (reg:DI 72)
>                 (const_int 0 [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
/home/uros/gcc-svn/trunk/gcc/config/alpha/alpha.c:2508
2508    alpha_emit_conditional_branch (rtx operands[], machine_mode cmp_mode)
(gdb) p debug_rtx (operands[0])
(ne (reg/f:DI 70)
    (const_int 16384 [0x4000]))
$1 = void
(gdb) p debug_rtx (operands[1])
(reg/f:DI 70)
$2 = void
(gdb) p debug_rtx (operands[2])
(const_int 16384 [0x4000])
$3 = void

Uros.