> -----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
>
> --
> Michael Eager ea...@eagercon.com
> 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
0001-microblaze-add-support-for-swap-instructions-and-reo.patch
Description: 0001-microblaze-add-support-for-swap-instructions-and-reo.patch
