This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix PR37053: Move tweaks of commutative precedence to target hook
Richard Sandiford <rdsandiford@googlemail.com> writes:
> 2009/7/14 Paolo Bonzini <bonzini@gnu.org>:
>> On 07/10/2009 03:14 PM, Peter Bergner wrote:
>>>
>>> On Fri, 2009-07-10 at 00:58 +0200, Paolo Bonzini wrote:
>>>>
>>>> Note that it is already being done if reload_completed, just not if
>>>> reload_in_progress. ?No matter how pleasant, it would be a surprise if
>>>> the attached patch worked without this change it includes:
>>>>
>>>> - ? ? ?if (! constrain_operands (1))
>>>> + ? ? ?if (! constrain_operands (reload_completed))
>>>>
>>>> (Note: I haven't tested it with the change, either). ?On the other hand,
>>>> with the change there is a subtle difference in reload_as_needed now
>>>> (I'm not sure it matters).
>>>>
>>>> Maxim, can you try it on m68k?
>>>
>>> FYI, this bootstrapped and regtested with no errors on powerpc64-linux.
>>
>> [patch at http://permalink.gmane.org/gmane.comp.gcc.patches/189115]
>>
>> It also fixes the testcase. ?Ok for mainline and branches?
>
> Sorry to butt in again, but like you say, I'm worried about changing
> the contrain_operands argument from 1 to 0. The reload1.c code
> that we've been talking about runs after the main reload processing,
> so we shouldn't accept anything that doesn't strictly match its constraints.
> We might also be pessimising things by changing the behaviour of
> eliminate_regs_in_insn, although I haven't really thought about that much.
>
> I know it's ugly, but how about either: (a) temporarily setting
> reload_completed around the autoinc validate_change or
> (b) adding a new interface to select what we want, with the
> default being the same as now? E.g. (b) could take a callback
> that does the validation, which would actually make the routines
> a bit more flexible.
Still a bit rough, but how about this? In the end I just added an
enum rather than a hook. I suppose the enum could also be retrofitted
elsewhere, such as the parameter to constrain_operands, but I wanted
to keep it simple.
Not tested beyond the PR testcase.
Richard
gcc/
* recog.h (recog_strictness): New enum.
* recog.c (validation_strictness): New variable.
(insn_invalid_p): Check it.
(push_validation_strictness): New function.
(pop_validation_strictness): Likewise.
* reload1.c (reload_as_needed): Use RS_STRICT_CONSTRAINTS.
Index: gcc/recog.h
===================================================================
--- gcc/recog.h 2009-07-14 21:02:54.000000000 +0100
+++ gcc/recog.h 2009-07-14 21:11:52.000000000 +0100
@@ -68,9 +68,30 @@ struct operand_alternative
unsigned int anything_ok:1;
};
+/* Enumerates how strict we can be when matching instructions. */
+enum recog_strictness {
+ /* The default behavior: RS_STRICT_CONSTRAINTS after reload and
+ RS_NO_CONSTRAINTS otherwise. */
+ RS_DEFAULT,
+
+ /* Only check whether a pattern is an asm or matches a define_insn;
+ don't care about whether the constraints are satisfied. */
+ RS_NO_CONSTRAINTS,
+
+ /* As for RS_NO_CONSTRAINTS, but also require the constraints to
+ be satisfied in a non-strict sense. (See constrain_operands for
+ details about strictness.) */
+ RS_LOOSE_CONSTRAINTS,
+
+ /* As for RS_NO_CONSTRAINTS, but also require the constraints
+ to be satisfied in a strict sense. */
+ RS_STRICT_CONSTRAINTS
+};
extern void init_recog (void);
extern void init_recog_no_volatile (void);
+extern enum recog_strictness push_validation_strictness (enum recog_strictness);
+extern void pop_validation_strictness (enum recog_strictness);
extern int check_asm_operands (rtx);
extern int asm_operand_ok (rtx, const char *, const char **);
extern bool validate_change (rtx, rtx *, rtx, bool);
Index: gcc/recog.c
===================================================================
--- gcc/recog.c 2009-07-14 21:02:54.000000000 +0100
+++ gcc/recog.c 2009-07-14 21:05:11.000000000 +0100
@@ -179,6 +179,9 @@ typedef struct change_t
static int num_changes = 0;
+/* How strict the matching should be. */
+static enum recog_strictness validation_strictness;
+
/* Validate a proposed change to OBJECT. LOC is the location in the rtl
at which NEW_RTX will be placed. If OBJECT is zero, no validation is done,
the change is simply made.
@@ -303,7 +306,7 @@ insn_invalid_p (rtx insn)
&& ! reload_completed && ! reload_in_progress)
? &num_clobbers : 0);
int is_asm = icode < 0 && asm_noperands (PATTERN (insn)) >= 0;
-
+ enum recog_strictness strictness;
/* If this is an asm and the operand aren't legal, then fail. Likewise if
this is not an asm and the insn wasn't recognized. */
@@ -327,12 +330,16 @@ insn_invalid_p (rtx insn)
PATTERN (insn) = pat = newpat;
}
- /* After reload, verify that all constraints are satisfied. */
- if (reload_completed)
+ /* Optionally verify that all constraints are satisfied. */
+ strictness = validation_strictness;
+ if (strictness == RS_DEFAULT)
+ strictness = reload_completed ? RS_STRICT_CONSTRAINTS : RS_NO_CONSTRAINTS;
+
+ if (strictness != RS_NO_CONSTRAINTS)
{
extract_insn (insn);
- if (! constrain_operands (1))
+ if (! constrain_operands (strictness == RS_STRICT_CONSTRAINTS))
return 1;
}
@@ -347,6 +354,32 @@ num_changes_pending (void)
return num_changes;
}
+/* Switch to a new validation strictness for a period of time.
+ This controls the strictness of insn_invalid_p, the validate_*
+ routines, and apply_change_group.
+
+ Pass the returned value to pop_validation_strictness to return
+ to the current status. */
+
+enum recog_strictness
+push_validation_strictness (enum recog_strictness new_strictness)
+{
+ enum recog_strictness old_strictness;
+
+ old_strictness = validation_strictness;
+ validation_strictness = new_strictness;
+ return old_strictness;
+}
+
+/* Return to the status before the last push_validation_strictness.
+ OLD_STRICTNESS is the value returned by that function. */
+
+void
+pop_validation_strictness (enum recog_strictness old_strictness)
+{
+ validation_strictness = old_strictness;
+}
+
/* Tentatively apply the changes numbered NUM and up.
Return 1 if all changes are valid, zero otherwise. */
Index: gcc/reload1.c
===================================================================
--- gcc/reload1.c 2009-07-14 21:02:54.000000000 +0100
+++ gcc/reload1.c 2009-07-14 21:05:11.000000000 +0100
@@ -4304,31 +4304,16 @@ reload_as_needed (int live_known)
continue;
if (n == 1)
{
+ enum recog_strictness os;
+
+ os = (push_validation_strictness
+ (RS_STRICT_CONSTRAINTS));
n = validate_replace_rtx (reload_reg,
gen_rtx_fmt_e (code,
mode,
reload_reg),
p);
-
- /* We must also verify that the constraints
- are met after the replacement. */
- extract_insn (p);
- if (n)
- n = constrain_operands (1);
- else
- break;
-
- /* If the constraints were not met, then
- undo the replacement. */
- if (!n)
- {
- validate_replace_rtx (gen_rtx_fmt_e (code,
- mode,
- reload_reg),
- reload_reg, p);
- break;
- }
-
+ pop_validation_strictness (os);
}
break;
}