On 2020-10-11 8:58 p.m., Hongtao Liu wrote:
Hi: This is done in 2 steps: 1. Extend special memory constraint to handle non MEM_P cases, i.e. (vec_duplicate:V4SF (mem:SF (addr))) 2. Refactor implementation of *_bcst{_1,_2,_3} patterns. Add new predicate bcst_mem_operand and corresponding constraint "Br" to merge "$(pattern)_bcst{_1,_2,_3}" into "$(pattern)", also delete those separate "*_bcst{_1,_2,_3}" patterns. Bootstrap is ok, regression test on i386 backend is ok. gcc/ChangeLog: PR target/87767 * ira-costs.c (record_operand_costs): Extract memory operand from recog_data.operand[i] for record_address_regs. (record_reg_classes): Extract memory operand from OP for conditional judgement MEM_P. * ira.c (ira_setup_alts): Ditto. * lra-constraints.c (extract_mem_from_operand): New function. (satisfies_memory_constraint_p): Extract memory operand from OP for decompose_mem_address, return false when there's no memory operand inside OP. (process_alt_operands): Remove MEM_P (op) since it would be judged in satisfies_memory_constraint_p. * recog.c (asm_operand_ok): Extract memory operand from OP for judgement of memory_operand (OP, VOIDmode). (constrain_operands): Don't unwrapper unary operator when there's memory operand inside. * rtl.h (extract_mem_from_operand): New decl.
Thank you for working on the PR. In general patch is ok for me. The only thing is
+/* For special_memory_operand, it could be false for MEM_P (op), + i.e. bcst_mem_operand in i386 backend. + Extract and return real memory operand or op. */ +rtx +extract_mem_from_operand (rtx op) +{ + if (MEM_P (op)) + return op; + /* Only allow one memory_operand inside special memory operand. */ The comment contradicts to the below code which returns the first memory operand (not the only one). + subrtx_var_iterator::array_type array; + FOR_EACH_SUBRTX_VAR (iter, array, op, ALL) + { + rtx x = *iter; + if (MEM_P (x)) + return x; + } + + return op; +} + I think the code should look like /* For special_memory_operand, it could be false for MEM_P (op), i.e. bcst_mem_operand in i386 backend. Extract and return real memory operand or op. */ rtx extract_mem_from_operand (rtx op) { if (MEM_P (op)) return op; /* Only allow one memory_operand inside special memory operand. */ subrtx_var_iterator::array_type array; rtx res = op; FOR_EACH_SUBRTX_VAR (iter, array, op, ALL) { rtx x = *iter; if (!MEM_P (x) || res != op) return op; res = op; } return res; } With this change, the patch is ok for me and can be committed into the trunk.