On 01/28/2016 09:07 AM, Vladimir Makarov wrote:
On 01/28/2016 08:05 AM, Jakub Jelinek wrote:
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,
I actually like the name special_memory_constraint. I'm sure it will
eventually be use for more than just aligned memories. Off the top of my head
I can imagine constraints for specific address spaces.
--- 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.
The difference in code most probably does not affect generated code correctness
but may change code quality. After some thinking, I decided to change it to
case CT_SPECIAL_MEMORY:
if (MEM_P (op) && constraint_satisfied_p (op, cn))
win = 1;
allows_mem[i] = insn_allows_mem[i] = 1;
break;
...
But taking complexity of RA we can do unaligned memory -> pseudo first
assigning a hard reg to the pseudo and on later subpasses to spill the
pseudo for some reasons. Removing such RA freedom might result in having
LRA stuck.
I believe that I follow your reasoning here, and the possible paths through LRA
that would make this happen, and I agree that we should allow LRA the freedom
to spill the pseudo that we just allocated.
The patch with that change looks good to me.
r~
