On 02/27/2013 04:36 PM, David Holsgrove wrote:
-----Original Message-----
From: Michael Eager [mailto:ea...@eagercon.com]
Sent: Thursday, 28 February 2013 3:06 am
To: David Holsgrove
Cc: Michael Eager; gcc-patches@gcc.gnu.org; John Williams; Edgar E. Iglesias
(edgar.igles...@gmail.com); Vinod Kathail; Vidhumouli Hunsigida; Nagaraju
Mekala; Tom Shui
Subject: Re: [Patch, microblaze]: Add support for swap instructions and reorder
option
The purpose is to avoid issuing a warning for processors before 8.30.a
unless the user explicitly specifies -mxl-reorder.
Warning the user who explicitly specifies -mxl-reorder with cpu before v8.30.a
is the first goal, but we also need to prevent the usage of swap instructions by
default if they are not possible to use.
I think that the code can be reordered to make it clearer.
Replace this
+ /* TARGET_REORDER initialised as 2 in microblaze.opt,
+ passing -mxl-reorder sets TARGET_REORDER to 1,
+ and passing -mno-xl-reorder sets TARGET_REORDER to 0. */
+ ver = MICROBLAZE_VERSION_COMPARE (microblaze_select_cpu, "v8.30.a");
+ if (ver < 0)
+ {
+ /* MicroBlaze prior to 8.30a didn't have swapb or swaph insns,
+ so if -mxl-reorder passed, warn and clear TARGET_REORDER. */
+ if (TARGET_REORDER == 1)
+ warning (0,
+ "-mxl-reorder can be used only with -mcpu=v8.30.a or greater");
+ TARGET_REORDER = 0;
+ }
+ else if (ver == 0)
+ {
+ /* MicroBlaze v8.30a requires pattern compare for
+ swapb / swaph insns. */
+ if (!TARGET_PATTERN_COMPARE)
+ TARGET_REORDER = 0;
+ }
With this:
/* TARGET_REORDER defaults to 2 if -mxl-reorder not specified. */
if (TARGET_REORDER == 1)
{
ver = MICROBLAZE_VERSION_COMPARE (microblaze_select_cpu, "v8.30.a");
if (ver < 0)
{
warning (0, "-mxl-reorder can be used only with -mcpu=v8.30.a or
greater");
TARGET_REORDER = 0;
}
else if ((ver == 0) && !TARGET_PATTERN_COMPARE)
{
warning (0, "-mxl-reorder requires -mxl-pattern-compare for
-mcpu=v8.30.a");
TARGET_REORDER = 0;
}
}
So if we switch to your alternative, the default case (TARGET_REORDER=2) will
not
be checked for cpu version, or use of TARGET_PATTERN_COMPARE, meaning we
emit swap instructions which aren’t valid for these situations.
Adjusting your if statement to be;
if (TARGET_REORDER)
would catch the default case along with explicit use of -mxl-reorder, but then
we
would also be issuing the warnings for every compilation where cpu is less than
v8.30.a, or the user hasn’t passed -mxl-pattern-compare.
My attached patch will warn the user if they've explicitly used -mxl-reorder and
don’t meet the requirements, and will adjust the default case silently if
required.
+mxl-reorder
+Target Var(TARGET_REORDER) Init(2)
+Use reorder instructions (default)
Change to
+Use reorder instructions (swap and byte reversed load/store) (default)
Yes thanks, this is a clearer definition of the intent behind the option.
I'll check in the patch with these changes unless you have objections.
thanks again for the review, please let me know if this patch is acceptable.
David
Committed revision 196415.
Please submit a patch to update gcc/doc/invoke.texi with -mxl-reorder
description.
--
Michael Eager ea...@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
