On 4/22/19 6:39 PM, Kugan Vivekanandarajah wrote:
> Hi Jeff,
>
> [...]
>
> + "#"
> + "&& 1"
> + [(const_int 0)]
> + "{
> + /* If we do not have an RMW operand, then copy the input
> + to the output before this insn. Also modify the existing
> + insn in-place so we can have make_field_assignment actually
> + generate a suitable extraction. */
> + if (!rtx_equal_p (operands[0], operands[1]))
> + {
> + emit_move_insn (operands[0], operands[1]);
> + XEXP (XEXP (SET_SRC (PATTERN (curr_insn)), 0), 0) = copy_rtx (operands[0]);
> + }
> +
> + rtx make_field_assignment (rtx);
> + rtx newpat = make_field_assignment (PATTERN (curr_insn));
> + gcc_assert (newpat);
> + emit_insn (newpat);
> + DONE;
>
> It seems that make_field_assignment returns a new pattern only if it
> succeeds and returns the same pattern otherwise. So I am wondering if
> it is worth simplifying the above. Like removing the assert and
> checking/inserting move only when new pattern is returned?
Thanks, this is something I meant to clean up and just totally forgot.
If the assert ever triggers it means something has gone horribly wrong.
Essentially the insn's condition wasn't sufficiently tight.
Catching it with the assert at this point is much easier to debug. The
failure modes when you don't catch the failure to create a field
assignment here occur much later in the pipeline and it won't be obvious
why things went wrong (I know that from experience :-).
So please consider the assert to be
gcc_assert (newpat != PATTERN (curr_insn);
Jeff
