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.
The reason for removing some_operand is, that it is effectively exactly equal to general_operand: from gensupport.c: {"general_operand", false, true, {SUBREG, REG, MEM}}, the second "true" stands for "bool allows_const_p", where: if (std_preds[i].allows_const_p) for (j = 0; j < NUM_RTX_CODE; j++) if (GET_RTX_CLASS (j) == RTX_CONST_OBJ) add_predicate_code (pred, (enum rtx_code) j); and from rtl.def, these are all RTX_CONST_OBJs: rtl.def:DEF_RTL_EXPR(CONST_INT, "const_int", "w", RTX_CONST_OBJ) rtl.def:DEF_RTL_EXPR(CONST_WIDE_INT, "const_wide_int", "", RTX_CONST_OBJ) rtl.def:DEF_RTL_EXPR(CONST_FIXED, "const_fixed", "www", RTX_CONST_OBJ) rtl.def:DEF_RTL_EXPR(CONST_DOUBLE, "const_double", CONST_DOUBLE_FORMAT, RTX_CONST_OBJ) rtl.def:DEF_RTL_EXPR(CONST_VECTOR, "const_vector", "E", RTX_CONST_OBJ) rtl.def:DEF_RTL_EXPR(CONST, "const", "e", RTX_CONST_OBJ) rtl.def:DEF_RTL_EXPR(LABEL_REF, "label_ref", "u", RTX_CONST_OBJ) rtl.def:DEF_RTL_EXPR(SYMBOL_REF, "symbol_ref", "s0", RTX_CONST_OBJ) rtl.def:DEF_RTL_EXPR(HIGH, "high", "e", RTX_CONST_OBJ) This is exactly the list of constant operands (minus const_fixed) from some_operand predicate: (define_predicate "some_operand" (ior (match_code "reg,mem,const_int,const_wide_int,const_double,const_vector, label_ref,symbol_ref,const,high") (and (match_code "subreg") (match_test "some_operand (SUBREG_REG (op), VOIDmode)")))) So, the only question is about SUBREG processing here, but I don't think it matters here. Uros.