* RFA: Small tweak to ira-lives.c:single_reg_class
@ 2014-06-05 20:12 Richard Sandiford
2014-06-06 3:43 ` Vladimir Makarov
0 siblings, 1 reply; 2+ messages in thread
From: Richard Sandiford @ 2014-06-05 20:12 UTC (permalink / raw)
To: gcc-patches; +Cc: vmakarov
I'm about to post a series of patches that reworks the handling of
standard constraints. As part of that I needed to make single_reg_class
handle "extra" constraints in a similar way to the standard ones.
It's not a particularly worthwhile change in itself -- not enough to justify
this long essay -- but I split it out because it's the only part of the
series that changes codegen.
The function looks like this:
case 'i':
if (CONSTANT_P (op)
|| (equiv_const != NULL_RTX && CONSTANT_P (equiv_const)))
return NO_REGS;
break;
case 'n':
if (CONST_SCALAR_INT_P (op)
|| (equiv_const != NULL_RTX && CONST_SCALAR_INT_P (equiv_const)))
return NO_REGS;
break;
case 's':
if ((CONSTANT_P (op) && !CONST_SCALAR_INT_P (op))
|| (equiv_const != NULL_RTX
&& CONSTANT_P (equiv_const)
&& !CONST_SCALAR_INT_P (equiv_const)))
return NO_REGS;
break;
case 'I':
case 'J':
case 'K':
case 'L':
case 'M':
case 'N':
case 'O':
case 'P':
if ((CONST_INT_P (op)
&& CONST_OK_FOR_CONSTRAINT_P (INTVAL (op), c, constraints))
|| (equiv_const != NULL_RTX
&& CONST_INT_P (equiv_const)
&& CONST_OK_FOR_CONSTRAINT_P (INTVAL (equiv_const),
c, constraints)))
return NO_REGS;
break;
case 'E':
case 'F':
if (CONST_DOUBLE_AS_FLOAT_P (op)
|| (GET_CODE (op) == CONST_VECTOR
&& GET_MODE_CLASS (GET_MODE (op)) == MODE_VECTOR_FLOAT)
|| (equiv_const != NULL_RTX
&& (CONST_DOUBLE_AS_FLOAT_P (equiv_const)
|| (GET_CODE (equiv_const) == CONST_VECTOR
&& (GET_MODE_CLASS (GET_MODE (equiv_const))
== MODE_VECTOR_FLOAT)))))
return NO_REGS;
break;
case 'G':
case 'H':
if ((CONST_DOUBLE_AS_FLOAT_P (op)
&& CONST_DOUBLE_OK_FOR_CONSTRAINT_P (op, c, constraints))
|| (equiv_const != NULL_RTX
&& CONST_DOUBLE_AS_FLOAT_P (equiv_const)
&& CONST_DOUBLE_OK_FOR_CONSTRAINT_P (equiv_const,
c, constraints)))
return NO_REGS;
/* ??? what about memory */
case 'r':
case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
case 'h': case 'j': case 'k': case 'l':
case 'q': case 't': case 'u':
case 'v': case 'w': case 'x': case 'y': case 'z':
case 'A': case 'B': case 'C': case 'D':
case 'Q': case 'R': case 'S': case 'T': case 'U':
case 'W': case 'Y': case 'Z':
next_cl = (c == 'r'
? GENERAL_REGS
: REG_CLASS_FROM_CONSTRAINT (c, constraints));
if (cl == NO_REGS
? ira_class_singleton[next_cl][GET_MODE (op)] < 0
: (ira_class_singleton[cl][GET_MODE (op)]
!= ira_class_singleton[next_cl][GET_MODE (op)]))
return NO_REGS;
cl = next_cl;
break;
[...]
default:
return NO_REGS;
So for known constant contraints we check whether OP or its equivalent
constant satisfies the constraint and return NO_REGS if so. I'd like to
extend this behaviour to the extra constraints, since some targets match
constants (often symbolic or unspec-based constants) there too.
The code that checks next_cl effectively assumes that the constraint is
always a register constraint. If it's something else, next_cl will be
NO_REGS, which isn't a singleton class, so we'll return NO_REGS regardless
of what type of constraint we're matching or what OP is. (In principle
this includes register constraints that are disabled on the current subtarget,
since they'll have a next_cl of NO_REGS too.)
In order to handle extra constant constraints as described above,
we'd need to ignore cases where next_cl itself is NO_REGS. This brings
me on to memory and address constraints...
The comment says:
/* ??? what about memory */
At the moment we return NO_REGS for target-independent memory constraints
like "m", "o" and "g", because of the default case. The handling of
next_cl means that we effectively do the same for extra memory constraints,
since next_cl is always NO_REGS for them. That seems reasonable to me
and I'm not trying to change it here. The patch just makes the current
choice explicit by checking for extra memory constraints.
Likewise we return NO_REGS for 'p' and (indirectly) for extra address
constraints. This too makes sense, since I don't think we support
a singleton BASE_REG_CLASS. Again the patch makes that explicit.
So all in all, the patch only really affects two cases: extra constant
constraints and the (probably rare) situation that there is a singleton
register constraint alongside disabled/NO_REGS register constraints.
The former does occur on Alpha and MIPS. E.g.:
(define_insn "*call_osf_1"
[(call (mem:DI (match_operand:DI 0 "call_operand" "c,R,s"))
(match_operand 1))
(use (reg:DI 29))
(clobber (reg:DI 26))]
"! TARGET_EXPLICIT_RELOCS && TARGET_ABI_OSF"
"@
jsr $26,($27),0\;ldgp $29,0($26)
bsr $26,$%0..ng
jsr $26,%0\;ldgp $29,0($26)"
[(set_attr "type" "jsr")
(set_attr "length" "12,*,16")])
Here "c" is a singleton register class (the ABI-defined indirect
call register). "s" is the standard constraint for symbolic addresses
and "R" is an extra constant constraint. If the call address is a
register, single_reg_class currently returns NO_REGS because of the
way that "R" is handled via next_cl==NO_REGS. After the patch it
returns the singleton register class for "c" instead. I think this
is the right behaviour, since indirect calls really do require a
single register; the other constraints are for direct calls instead.
It looks like there's a missing break after the 'G' and 'H' handling.
Tested on x86_64-linux-gnu. I also did an assembly comparison for
a range of targets and Alpha and MIPS seemed to be the only ones
affected. OK to install?
Sorry for the long write-up...
Thanks,
Richard
gcc/
* ira-lives.c (single_reg_class): Add missing break. Return NO_REGS
for extra address and memory constraints, not just 'p' and
TARGET_MEM_CONSTRAINT. Likewise if the operand is a constant or
equivalent to a constant and if it matches an extra constraint.
Ignore other non-register operands.
Index: gcc/ira-lives.c
===================================================================
--- gcc/ira-lives.c 2014-06-04 22:15:23.527995920 +0100
+++ gcc/ira-lives.c 2014-06-04 22:15:50.765243138 +0100
@@ -839,7 +839,8 @@ single_reg_class (const char *constraint
&& CONST_DOUBLE_OK_FOR_CONSTRAINT_P (equiv_const,
c, constraints)))
return NO_REGS;
- /* ??? what about memory */
+ break;
+
case 'r':
case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
case 'h': case 'j': case 'k': case 'l':
@@ -848,9 +849,22 @@ single_reg_class (const char *constraint
case 'A': case 'B': case 'C': case 'D':
case 'Q': case 'R': case 'S': case 'T': case 'U':
case 'W': case 'Y': case 'Z':
+#ifdef EXTRA_CONSTRAINT_STR
+ /* ??? Is rejecting memory the best thing to do? */
+ if (EXTRA_MEMORY_CONSTRAINT (c, constraints)
+ || EXTRA_ADDRESS_CONSTRAINT (c, constraints))
+ return NO_REGS;
+ if (EXTRA_CONSTRAINT_STR (op, c, constraints)
+ || (equiv_const != NULL_RTX
+ && CONSTANT_P (equiv_const)
+ && EXTRA_CONSTRAINT_STR (equiv_const, c, constraints)))
+ return NO_REGS;
+#endif
next_cl = (c == 'r'
? GENERAL_REGS
: REG_CLASS_FROM_CONSTRAINT (c, constraints));
+ if (next_cl == NO_REGS)
+ break;
if (cl == NO_REGS
? ira_class_singleton[next_cl][GET_MODE (op)] < 0
: (ira_class_singleton[cl][GET_MODE (op)]
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: RFA: Small tweak to ira-lives.c:single_reg_class
2014-06-05 20:12 RFA: Small tweak to ira-lives.c:single_reg_class Richard Sandiford
@ 2014-06-06 3:43 ` Vladimir Makarov
0 siblings, 0 replies; 2+ messages in thread
From: Vladimir Makarov @ 2014-06-06 3:43 UTC (permalink / raw)
To: gcc-patches, rdsandiford
On 2014-06-05, 4:12 PM, Richard Sandiford wrote:
>
> It looks like there's a missing break after the 'G' and 'H' handling.
>
> Tested on x86_64-linux-gnu. I also did an assembly comparison for
> a range of targets and Alpha and MIPS seemed to be the only ones
> affected. OK to install?
>
> Sorry for the long write-up...
>
It is long but make me to know new things about Alpha and MIPS insn cases.
By the way, there are a lot of controversy about the changing cost for
particular hard reg. Sometimes it creates a lot of mess in allocation,
when one pseudo involves several hard regs.
Also a lot of alternatives can gives different hard reg preferencing or
wrong ones as we don't know what alternative will be used. Therefore I
think about earlier alternative selections and reg class cost
calculations based on the selected alternatives. But it will be a long
project probably involving code selection too. I don't know how will it
work until it is implemented which will not happen soon.
As I remember I added the code as I see small improvements on x86/x86-64
on benchmarks.
As for the patch, it is ok for me.
Thanks, Richard.
>
> gcc/
> * ira-lives.c (single_reg_class): Add missing break. Return NO_REGS
> for extra address and memory constraints, not just 'p' and
> TARGET_MEM_CONSTRAINT. Likewise if the operand is a constant or
> equivalent to a constant and if it matches an extra constraint.
> Ignore other non-register operands.
>
> Index: gcc/ira-lives.c
> ===================================================================
> --- gcc/ira-lives.c 2014-06-04 22:15:23.527995920 +0100
> +++ gcc/ira-lives.c 2014-06-04 22:15:50.765243138 +0100
> @@ -839,7 +839,8 @@ single_reg_class (const char *constraint
> && CONST_DOUBLE_OK_FOR_CONSTRAINT_P (equiv_const,
> c, constraints)))
> return NO_REGS;
> - /* ??? what about memory */
> + break;
> +
> case 'r':
> case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
> case 'h': case 'j': case 'k': case 'l':
> @@ -848,9 +849,22 @@ single_reg_class (const char *constraint
> case 'A': case 'B': case 'C': case 'D':
> case 'Q': case 'R': case 'S': case 'T': case 'U':
> case 'W': case 'Y': case 'Z':
> +#ifdef EXTRA_CONSTRAINT_STR
> + /* ??? Is rejecting memory the best thing to do? */
> + if (EXTRA_MEMORY_CONSTRAINT (c, constraints)
> + || EXTRA_ADDRESS_CONSTRAINT (c, constraints))
> + return NO_REGS;
> + if (EXTRA_CONSTRAINT_STR (op, c, constraints)
> + || (equiv_const != NULL_RTX
> + && CONSTANT_P (equiv_const)
> + && EXTRA_CONSTRAINT_STR (equiv_const, c, constraints)))
> + return NO_REGS;
> +#endif
> next_cl = (c == 'r'
> ? GENERAL_REGS
> : REG_CLASS_FROM_CONSTRAINT (c, constraints));
> + if (next_cl == NO_REGS)
> + break;
> if (cl == NO_REGS
> ? ira_class_singleton[next_cl][GET_MODE (op)] < 0
> : (ira_class_singleton[cl][GET_MODE (op)]
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2014-06-06 3:43 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-05 20:12 RFA: Small tweak to ira-lives.c:single_reg_class Richard Sandiford
2014-06-06 3:43 ` Vladimir Makarov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).