This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [Patch, microblaze]: Add support for swap instructions and reorder option


On 02/27/2013 04:36 PM, David Holsgrove wrote:


-----Original Message-----
From: Michael Eager [mailto:eager@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.iglesias@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 eager@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]