| Message ID | CAMe9rOpjomeEMO+=8=XDyW+Q4SRVE1jPHL7xsvm5Ckqk3ySQcg@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Fri, May 20, 2016 at 7:49 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Fri, May 20, 2016 at 10:15 AM, Rainer Orth > <ro@cebitec.uni-bielefeld.de> wrote: >> "H.J. Lu" <hjl.tools@gmail.com> writes: >> >>> On Thu, May 12, 2016 at 10:54 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >>>>>> Here is a patch to add >>>>>> -mgeneral-regs-only option to x86 backend. We can update >>>>>> spec for interrupt handle to recommend compiling interrupt handler >>>>>> with -mgeneral-regs-only option and add a note for compiler >>>>>> implementers. >>>>>> >>>>>> OK for trunk if there is no regression? >>>>> >>>>> >>>>> I can't comment on the code patch, but for the documentation part: >>>>> >>>>>> @@ -24242,6 +24242,12 @@ opcodes, to mitigate against certain forms of >>>>>> attack. At the moment, >>>>>> this option is limited in what it can do and should not be relied >>>>>> on to provide serious protection. >>>>>> >>>>>> +@item -mgeneral-regs-only >>>>>> +@opindex mgeneral-regs-only >>>>>> +Generate code which uses only the general-purpose registers. This will >>>>> >>>>> >>>>> s/which/that/ >>>>> >>>>>> +prevent the compiler from using floating-point, vector, mask and bound >>>>> >>>>> >>>>> s/will prevent/prevents/ >>>>> >>>>>> +registers, but will not impose any restrictions on the assembler. >>>>> >>>>> >>>>> Maybe you mean to say "does not restrict use of those registers in inline >>>>> assembly code"? In any case, please get rid of the future tense here, too. >>>> >>>> I changed it to >>>> >>>> --- >>>> @item -mgeneral-regs-only >>>> @opindex mgeneral-regs-only >>>> Generate code that uses only the general-purpose registers. This >>>> prevents the compiler from using floating-point, vector, mask and bound >>>> registers. >>>> --- >>>> >>> >>> Here is the updated patch. Tested on x86-64. OK for trunk? >> >> This patch broke {i386,x86_64}-apple-darwin15.5.0 bootstrap: >> >> In file included from ./tm.h:16:0, >> from /vol/gcc/src/hg/trunk/local/gcc/genattrtab.c:108: >> ./options.h:5443:2: error: #error too many target masks >> #error too many target masks >> ^ >> Makefile:2497: recipe for target 'build/genattrtab.o' failed >> make[3]: *** [build/genattrtab.o] Error 1 >> >> options.h has >> >> #define OPTION_MASK_ISA_XSAVES (HOST_WIDE_INT_1 << 62) >> #error too many target masks >> >> The tree bootstraps just fine at the previous revision. >> > > Tested on x86-64. OK for trunk? No, this is a flag, not a variable. Let's figure out how to extend target flags to more than 63 flags first. Please revert the original patch in the mean time. Thanks, Uros.
On Sat, May 21, 2016 at 9:48 AM, Uros Bizjak <ubizjak@gmail.com> wrote: > On Fri, May 20, 2016 at 7:49 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Fri, May 20, 2016 at 10:15 AM, Rainer Orth >> <ro@cebitec.uni-bielefeld.de> wrote: >>> "H.J. Lu" <hjl.tools@gmail.com> writes: >>> >>>> On Thu, May 12, 2016 at 10:54 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >>>>>>> Here is a patch to add >>>>>>> -mgeneral-regs-only option to x86 backend. We can update >>>>>>> spec for interrupt handle to recommend compiling interrupt handler >>>>>>> with -mgeneral-regs-only option and add a note for compiler >>>>>>> implementers. >>>>>>> >>>>>>> OK for trunk if there is no regression? >>>>>> >>>>>> >>>>>> I can't comment on the code patch, but for the documentation part: >>>>>> >>>>>>> @@ -24242,6 +24242,12 @@ opcodes, to mitigate against certain forms of >>>>>>> attack. At the moment, >>>>>>> this option is limited in what it can do and should not be relied >>>>>>> on to provide serious protection. >>>>>>> >>>>>>> +@item -mgeneral-regs-only >>>>>>> +@opindex mgeneral-regs-only >>>>>>> +Generate code which uses only the general-purpose registers. This will >>>>>> >>>>>> >>>>>> s/which/that/ >>>>>> >>>>>>> +prevent the compiler from using floating-point, vector, mask and bound >>>>>> >>>>>> >>>>>> s/will prevent/prevents/ >>>>>> >>>>>>> +registers, but will not impose any restrictions on the assembler. >>>>>> >>>>>> >>>>>> Maybe you mean to say "does not restrict use of those registers in inline >>>>>> assembly code"? In any case, please get rid of the future tense here, too. >>>>> >>>>> I changed it to >>>>> >>>>> --- >>>>> @item -mgeneral-regs-only >>>>> @opindex mgeneral-regs-only >>>>> Generate code that uses only the general-purpose registers. This >>>>> prevents the compiler from using floating-point, vector, mask and bound >>>>> registers. >>>>> --- >>>>> >>>> >>>> Here is the updated patch. Tested on x86-64. OK for trunk? >>> >>> This patch broke {i386,x86_64}-apple-darwin15.5.0 bootstrap: >>> >>> In file included from ./tm.h:16:0, >>> from /vol/gcc/src/hg/trunk/local/gcc/genattrtab.c:108: >>> ./options.h:5443:2: error: #error too many target masks >>> #error too many target masks >>> ^ >>> Makefile:2497: recipe for target 'build/genattrtab.o' failed >>> make[3]: *** [build/genattrtab.o] Error 1 >>> >>> options.h has >>> >>> #define OPTION_MASK_ISA_XSAVES (HOST_WIDE_INT_1 << 62) >>> #error too many target masks >>> >>> The tree bootstraps just fine at the previous revision. >>> >> >> Tested on x86-64. OK for trunk? > > No, this is a flag, not a variable. Let's figure out how to extend > target flags to more than 63 flags first. > > Please revert the original patch in the mean time. HJ is away from keyboard for a couple of days, I have reverted the patch on his request. Uros.
On Sat, May 21, 2016 at 12:48 AM, Uros Bizjak <ubizjak@gmail.com> wrote: > On Fri, May 20, 2016 at 7:49 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Fri, May 20, 2016 at 10:15 AM, Rainer Orth >> <ro@cebitec.uni-bielefeld.de> wrote: >>> "H.J. Lu" <hjl.tools@gmail.com> writes: >>> >>>> On Thu, May 12, 2016 at 10:54 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >>>>>>> Here is a patch to add >>>>>>> -mgeneral-regs-only option to x86 backend. We can update >>>>>>> spec for interrupt handle to recommend compiling interrupt handler >>>>>>> with -mgeneral-regs-only option and add a note for compiler >>>>>>> implementers. >>>>>>> >>>>>>> OK for trunk if there is no regression? >>>>>> >>>>>> >>>>>> I can't comment on the code patch, but for the documentation part: >>>>>> >>>>>>> @@ -24242,6 +24242,12 @@ opcodes, to mitigate against certain forms of >>>>>>> attack. At the moment, >>>>>>> this option is limited in what it can do and should not be relied >>>>>>> on to provide serious protection. >>>>>>> >>>>>>> +@item -mgeneral-regs-only >>>>>>> +@opindex mgeneral-regs-only >>>>>>> +Generate code which uses only the general-purpose registers. This will >>>>>> >>>>>> >>>>>> s/which/that/ >>>>>> >>>>>>> +prevent the compiler from using floating-point, vector, mask and bound >>>>>> >>>>>> >>>>>> s/will prevent/prevents/ >>>>>> >>>>>>> +registers, but will not impose any restrictions on the assembler. >>>>>> >>>>>> >>>>>> Maybe you mean to say "does not restrict use of those registers in inline >>>>>> assembly code"? In any case, please get rid of the future tense here, too. >>>>> >>>>> I changed it to >>>>> >>>>> --- >>>>> @item -mgeneral-regs-only >>>>> @opindex mgeneral-regs-only >>>>> Generate code that uses only the general-purpose registers. This >>>>> prevents the compiler from using floating-point, vector, mask and bound >>>>> registers. >>>>> --- >>>>> >>>> >>>> Here is the updated patch. Tested on x86-64. OK for trunk? >>> >>> This patch broke {i386,x86_64}-apple-darwin15.5.0 bootstrap: >>> >>> In file included from ./tm.h:16:0, >>> from /vol/gcc/src/hg/trunk/local/gcc/genattrtab.c:108: >>> ./options.h:5443:2: error: #error too many target masks >>> #error too many target masks >>> ^ >>> Makefile:2497: recipe for target 'build/genattrtab.o' failed >>> make[3]: *** [build/genattrtab.o] Error 1 >>> >>> options.h has >>> >>> #define OPTION_MASK_ISA_XSAVES (HOST_WIDE_INT_1 << 62) >>> #error too many target masks >>> >>> The tree bootstraps just fine at the previous revision. >>> >> >> Tested on x86-64. OK for trunk? > > No, this is a flag, not a variable. Let's figure out how to extend > target flags to more than 63 flags first. Extending target flags to more than 63 bits requires replacing HOST_WIDE_INT with a bit vector. Since target flags is used in TARGET_SUBTARGET_DEFAULT, change it to a bit vector is a non-trivial change. On the other hand, -mgeneral-regs-only is a command-line option which doesn't require support for TARGET_SUBTARGET_DEFAULT, similar to other -m options like -mmitigate-rop. Using flag_general_regs_only is an option.
On Tue, May 24, 2016 at 5:40 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> No, this is a flag, not a variable. Let's figure out how to extend >> target flags to more than 63 flags first. > > Extending target flags to more than 63 bits requires replacing > HOST_WIDE_INT with a bit vector. Since target flags is used in > TARGET_SUBTARGET_DEFAULT, change it to a bit vector is a > non-trivial change. On the other hand, -mgeneral-regs-only is a > command-line option which doesn't require support for > TARGET_SUBTARGET_DEFAULT, similar to other -m options like > -mmitigate-rop. Using flag_general_regs_only is an option. I have been informed that Intel people are looking into how to extend target flags to accommodate additional ISA flags. There is no point to hurry with an unoptimal solution. Perhaps you can coordinate your patch with their efforts? Uros.
On Tue, May 24, 2016 at 8:52 AM, Uros Bizjak <ubizjak@gmail.com> wrote: > On Tue, May 24, 2016 at 5:40 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > >>> No, this is a flag, not a variable. Let's figure out how to extend >>> target flags to more than 63 flags first. >> >> Extending target flags to more than 63 bits requires replacing >> HOST_WIDE_INT with a bit vector. Since target flags is used in >> TARGET_SUBTARGET_DEFAULT, change it to a bit vector is a >> non-trivial change. On the other hand, -mgeneral-regs-only is a >> command-line option which doesn't require support for >> TARGET_SUBTARGET_DEFAULT, similar to other -m options like >> -mmitigate-rop. Using flag_general_regs_only is an option. > > I have been informed that Intel people are looking into how to extend > target flags to accommodate additional ISA flags. There is no point to > hurry with an unoptimal solution. Perhaps you can coordinate your > patch with their efforts? iISA flags use x86_isa_flags, not target_flags. -mgeneral-regs-only shouldn't use x86_isa_flags. It was my oversight to use target_flags with -mgeneral-regs-only to begin with. I don't think using flag_general_regs_only is not an optimal solution, which I should have used in the first place. The x86 change for interrupt handler depends on -mgeneral-regs-only.
On Tue, May 24, 2016 at 6:22 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Tue, May 24, 2016 at 8:52 AM, Uros Bizjak <ubizjak@gmail.com> wrote: >> On Tue, May 24, 2016 at 5:40 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> >>>> No, this is a flag, not a variable. Let's figure out how to extend >>>> target flags to more than 63 flags first. >>> >>> Extending target flags to more than 63 bits requires replacing >>> HOST_WIDE_INT with a bit vector. Since target flags is used in >>> TARGET_SUBTARGET_DEFAULT, change it to a bit vector is a >>> non-trivial change. On the other hand, -mgeneral-regs-only is a >>> command-line option which doesn't require support for >>> TARGET_SUBTARGET_DEFAULT, similar to other -m options like >>> -mmitigate-rop. Using flag_general_regs_only is an option. >> >> I have been informed that Intel people are looking into how to extend >> target flags to accommodate additional ISA flags. There is no point to >> hurry with an unoptimal solution. Perhaps you can coordinate your >> patch with their efforts? > > iISA flags use x86_isa_flags, not target_flags. -mgeneral-regs-only > shouldn't use x86_isa_flags. It was my oversight to use target_flags > with -mgeneral-regs-only to begin with. I don't think using > flag_general_regs_only is not an optimal solution, which I should have > used in the first place. The x86 change for interrupt handler depends > on -mgeneral-regs-only. Oh, target_flags is only a 32bit integer :(. Is there a reason it can't be extended to HOST_WIDE_INT, as is the case with ix86_isa_flags? Uros.
On Tue, May 24, 2016 at 9:53 AM, Uros Bizjak <ubizjak@gmail.com> wrote: > On Tue, May 24, 2016 at 6:22 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Tue, May 24, 2016 at 8:52 AM, Uros Bizjak <ubizjak@gmail.com> wrote: >>> On Tue, May 24, 2016 at 5:40 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >>> >>>>> No, this is a flag, not a variable. Let's figure out how to extend >>>>> target flags to more than 63 flags first. >>>> >>>> Extending target flags to more than 63 bits requires replacing >>>> HOST_WIDE_INT with a bit vector. Since target flags is used in >>>> TARGET_SUBTARGET_DEFAULT, change it to a bit vector is a >>>> non-trivial change. On the other hand, -mgeneral-regs-only is a >>>> command-line option which doesn't require support for >>>> TARGET_SUBTARGET_DEFAULT, similar to other -m options like >>>> -mmitigate-rop. Using flag_general_regs_only is an option. >>> >>> I have been informed that Intel people are looking into how to extend >>> target flags to accommodate additional ISA flags. There is no point to >>> hurry with an unoptimal solution. Perhaps you can coordinate your >>> patch with their efforts? >> >> iISA flags use x86_isa_flags, not target_flags. -mgeneral-regs-only >> shouldn't use x86_isa_flags. It was my oversight to use target_flags >> with -mgeneral-regs-only to begin with. I don't think using >> flag_general_regs_only is not an optimal solution, which I should have >> used in the first place. The x86 change for interrupt handler depends >> on -mgeneral-regs-only. > > Oh, target_flags is only a 32bit integer :(. Is there a reason it > can't be extended to HOST_WIDE_INT, as is the case with > ix86_isa_flags? target_flags is generic, not target specific. I want to limit my change to x86 backend and -mgeneral-regs-only doesn't need to use target_flags .
From 174ef61afa610d7721d290a8a61136f86fa3987f Mon Sep 17 00:00:00 2001 From: "H.J. Lu" <hjl.tools@gmail.com> Date: Fri, 20 May 2016 10:45:12 -0700 Subject: [PATCH] Use flag_general_regs_only with -mgeneral-regs-only Since we run out of bits in target_flags for x86 Darwin targets, use flag_general_regs_only instead of target_flags. * config/i386/i386.c (ix86_option_override_internal): Check x_flag_general_regs_only instead of MASK_GENERAL_REGS_ONLY. * config/i386/i386.opt (-mgeneral-regs-only): Replace GENERAL_REGS_ONLY with flag_general_regs_only. --- gcc/config/i386/i386.c | 2 +- gcc/config/i386/i386.opt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 29a7941..e9b5182 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -5339,7 +5339,7 @@ ix86_option_override_internal (bool main_args_p, /* Don't enable x87 instructions if only the general registers are allowed. */ - if (!(opts_set->x_target_flags & MASK_GENERAL_REGS_ONLY) + if (!opts->x_flag_general_regs_only && !(opts_set->x_target_flags & MASK_80387)) { if (processor_alias_table[i].flags & PTA_NO_80387) diff --git a/gcc/config/i386/i386.opt b/gcc/config/i386/i386.opt index d12b29a..0fc7cf0 100644 --- a/gcc/config/i386/i386.opt +++ b/gcc/config/i386/i386.opt @@ -899,5 +899,5 @@ Target Var(flag_mitigate_rop) Init(0) Attempt to avoid generating instruction sequences containing ret bytes. mgeneral-regs-only -Target Report RejectNegative Mask(GENERAL_REGS_ONLY) Save +Target Report RejectNegative Var(flag_general_regs_only) Init(0) Generate code which uses only the general registers. -- 2.5.5