On 2020-10-20 1:33 a.m., Hongtao Liu wrote:
On Mon, Oct 19, 2020 at 11:38 PM Vladimir Makarov <vmaka...@redhat.com> wrote:
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).
Yes.
+ 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;
Assume you want to assign res with x.
Also in the iteration, x would first be op which would be false for
MEM_P, then op would be returned.
That's not what you mean, so i changed to
/* 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))
continue;
/* Return op when there're multiple memory operands. */
if (res != op)
return op;
else
res = x;
}
Actually I wanted to have constraint satisfying rtx with memory covered
by **only unary** operator(s). Your code satisfies memory covered by
non-unary operators (e.g. binary ones).
Why do I prefer less general constraint? Because other operands of
operator containing the memory might need reloads too and the more
general constraint will ignore this. If this situation is impossible
now, it might be possible in the future.
My proposed code is wrong as I forgot that FOR_EACH_SUBRTX_VAR processes
sub-rtx recursively. Thank you for starting the discussion. Now 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)
{
for (rtx x = op;; x = XEXP (x, 0)) {
if (MEM_P (x))
return x;
if (GET_RTX_LENGTH (GET_CODE (x)) != 1 || GET_RTX_FORMAT (GET_CODE
(x))[0] != 'e')
break;
}
return op;
}
Let me know what do you think.
