On Wed, Jan 27, 2016 at 04:01:23PM -0500, Vladimir Makarov wrote:
> The following patch fixes PR69299.
>
> The details of the problem is described on
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69299
>
> The patch was successfully bootstrapped and tested on x86/x86-64.
>
> The patch introduces a new type of constraints
> define_special_memory_constraint for memory constraints whose address reload
> can not make memory to satisfy the constraint. It is useful when
> specifically aligned memory is necessary or desirable. I don't know what is
> the best name for this constraint. I use special_memory_constraint but it
> could be more specific, e.g. aligned_memory_constraint. Please let me know
> what is the best name for you.
>
> Is the patch ok to commit?
I support the general idea and for naming will defer to Jeff, just have some
nits below. But it is your code and area of expertise, so I don't claim to
be an expert here...
> --- ira-costs.c (revision 232571)
> +++ ira-costs.c (working copy)
> @@ -777,6 +777,7 @@ record_reg_classes (int n_alts, int n_op
> break;
>
> case CT_MEMORY:
> + case CT_SPECIAL_MEMORY:
> /* Every MEM can be reloaded to fit. */
> insn_allows_mem[i] = allows_mem[i] = 1;
> if (MEM_P (op))
The comment is true only for CT_MEMORY. Wonder if it wouldn't be better to
handle CT_SPECIAL_MEMORY separately, perhaps as:
case CT_SPECIAL_MEMORY:
if (MEM_P (op) && constraint_satisfied_p (op, cn))
{
insn_allows_mem[i] = allows_mem[i] = 1;
win = 1;
}
break;
? I.e. if the constraint is already satisfied, treat it like memory
constraint, otherwise treat like (unsatisfied) fixed form constraint.
Or, if you want to account for the possibility that it doesn't satisfy the
constraint yet due to address that if reloaded would make it satisfy,
consider !memory_operand (op, ...) case as unknown, with no need to
check the constraint. Because if op satisfies already memory_operand,
but doesn't constraint_satisfied_p, it means it will never satisfy.
> --- ira.c (revision 232571)
> +++ ira.c (working copy)
> @@ -1868,6 +1868,7 @@ ira_setup_alts (rtx_insn *insn, HARD_REG
>
> case CT_ADDRESS:
> case CT_MEMORY:
> + case CT_SPECIAL_MEMORY:
> goto op_success;
Perhaps treat it like CT_FIXED_FORM here instead? I.e. op_success only
if the constraint is satisfied? Or perhaps treat it as possible op_success
if !memory_operand, as mentioned above.
> --- recog.c (revision 232571)
> +++ recog.c (working copy)
> @@ -1791,6 +1791,7 @@ asm_operand_ok (rtx op, const char *cons
> break;
>
> case CT_MEMORY:
> + case CT_SPECIAL_MEMORY:
> /* Every memory operand can be reloaded to fit. */
> result = result || memory_operand (op, VOIDmode);
> break;
Again, I'd treat it like CT_FIXED_FORM here, or at least if
memory_operand check also constraint_satisfied_p.
Jakub
