On Thu, May 14, 2015 at 1:31 PM, Uros Bizjak <ubiz...@gmail.com> wrote: > On Wed, May 13, 2015 at 8:53 PM, Richard Henderson <r...@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 <ubiz...@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.