On 05/14/2015 04:40 AM, Uros Bizjak wrote: > 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)
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. r~