diff mbox

PATCH: PR target/70738: Add -mgeneral-regs-only option

Message ID CAMe9rOpaY608q+VZqakMdXnE5dHtgj9dueUigkWNuBKp=CsOsw@mail.gmail.com
State New
Headers show

Commit Message

H.J. Lu May 11, 2016, 5:02 p.m. UTC 
On Tue, May 10, 2016 at 1:02 PM, Sandra Loosemore
<sandra@codesourcery.com> wrote:
> On 04/20/2016 07:42 AM, Koval, Julia wrote:
>>
>> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
>> index a5a8b23..82de5bf 100644
>> --- a/gcc/doc/extend.texi
>> +++ b/gcc/doc/extend.texi
>> @@ -5263,6 +5263,83 @@ On x86-32 targets, the @code{stdcall} attribute
>> causes the compiler to
>>  assume that the called function pops off the stack space used to
>>  pass arguments, unless it takes a variable number of arguments.
>>
>> +@item no_caller_saved_registers
>> +@cindex @code{no_caller_saved_registers} function attribute, x86
>> +Use this attribute to indicate that the specified function has no
>> +caller-saved registers.  That is, all registers are callee-saved.
>> +The compiler generates proper function entry and exit sequences to
>> +save and restore any modified registers, except for the EFLAGS
>> +register.  If the compiler generates MPX, SSE, MMX or x87 instructions
>> +in a function with @code{no_caller_saved_registers} attribute or
>> +functions called from a function with @code{no_caller_saved_registers}
>> +attribute may contain MPX, SSE, MMX or x87 instructions, the compiler
>> +must save and restore the corresponding state.
>
>
> I cannot parse the last sentence in this paragraph.  How can the compiler
> know whether called functions may contain those instructions? Plus, talking
> about what the compiler must do seems too implementor-speaky for user
> documentation.  Maybe you mean something like "The compiler also saves and
> restores state associated with MPX, SSE, MMX, and x87 instructions." ?
>
> I also think the documentation needs to give some hint about why a user
> would want to put this attribute on a function.
>
>> +
>> +@item interrupt
>> +@cindex @code{interrupt} function attribute, x86
>> +Use this attribute to indicate that the specified function is an
>> +interrupt handler.  The compiler generates function
>> +entry and exit sequences suitable for use in an interrupt handler when
>> +this attribute is present.  The @code{IRET} instruction, instead of the
>> +@code{RET} instruction, is used to return from interrupt handlers.  All
>> +registers, except for the EFLAGS register which is restored by the
>> +@code{IRET} instruction, are preserved by the compiler.  If the
>> +compiler generates MPX, SSE, MMX or x87 instructions in an interrupt
>> +handler, or functions called from an interrupt handler may contain MPX,
>> +SSE, MMX or x87 instructions, the compiler must save and restore the
>> +corresponding state.
>
>
> Similar problems here.
>
> From the further discussion that follows, it appears that you can use the
> "interrupt" attribute on exception handlers as well, but the paragraph above
> only mentions interrupt handlers.
>
>> +
>> +Since the direction flag in the FLAGS register in interrupt handlers
>> +is undetermined, cld instruction must be emitted in function prologue
>> +if rep string instructions are used in interrupt handler or interrupt
>> +handler isn't a leaf function.
>
>
> Again, this sounds like implementor-speak, and there are grammatical errors
> (noun/verb disagreement, missing articles).  Do users of this attribute need
> to know what instructions the compiler is emitting?  We already say above
> that it causes GCC to generate suitable entry and exit sequences.
>

It was done on purpose since this document is also served as
the spec for compiler implementers.  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?

Comments

Sandra Loosemore May 12, 2016, 3:46 p.m. UTC | #1 
On 05/11/2016 11:02 AM, H.J. Lu wrote:
> On Tue, May 10, 2016 at 1:02 PM, Sandra Loosemore
> <sandra@codesourcery.com> wrote:
>>
>> Again, this sounds like implementor-speak, and there are grammatical errors
>> (noun/verb disagreement, missing articles).  Do users of this attribute need
>> to know what instructions the compiler is emitting?  We already say above
>> that it causes GCC to generate suitable entry and exit sequences.
>>
>
> It was done on purpose since this document is also served as
> the spec for compiler implementers.

But readers of the user documentation are users, not compiler 
implementors, so the patch for the manual needs to have a different focus.

> 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.

> +
>  @end table
>
>  These @samp{-m} switches are supported in addition to the above

-Sandra
H.J. Lu May 12, 2016, 5:54 p.m. UTC | #2 
On Thu, May 12, 2016 at 8:46 AM, Sandra Loosemore
<sandra@codesourcery.com> wrote:
> On 05/11/2016 11:02 AM, H.J. Lu wrote:
>>
>> On Tue, May 10, 2016 at 1:02 PM, Sandra Loosemore
>> <sandra@codesourcery.com> wrote:
>>>
>>>
>>> Again, this sounds like implementor-speak, and there are grammatical
>>> errors
>>> (noun/verb disagreement, missing articles).  Do users of this attribute
>>> need
>>> to know what instructions the compiler is emitting?  We already say above
>>> that it causes GCC to generate suitable entry and exit sequences.
>>>
>>
>> It was done on purpose since this document is also served as
>> the spec for compiler implementers.
>
>
> But readers of the user documentation are users, not compiler implementors,
> so the patch for the manual needs to have a different focus.

That is why I suggested to add a note for compiler implementers
instead.

>> 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.
---

Thanks.
diff mbox

Patch

From d6f72979f077064c59ac83c8fe5e738ade732e96 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Wed, 11 May 2016 09:49:33 -0700
Subject: [PATCH] Add -mgeneral-regs-only option

X86 Linux kernel is compiled only with integer instructions.  Currently,

-mno-sse -mno-mmx -mno-sse2 -mno-3dnow -mno-avx -mno-80387
-mno-fp-ret-in-387  -mskip-rax-setup

is used to compile kernel.  If we add another non-integer feature, it
has to be turned off.  We can add a -mgeneral-regs-only option, similar
to AArch64, to disable all non-integer features so that kernel doesn't
need a long list and the same option will work for future compilers.
It can also be used to compile interrupt handler.

gcc/

	PR target/70738
	* common/config/i386/i386-common.c
	(OPTION_MASK_ISA_GENERAL_REGS_ONLY_UNSET): New.
	(ix86_handle_option): Disable MPX, MMX, SSE and x87 instructions
	for -mgeneral-regs-only.
	* config/i386/i386.c (ix86_option_override_internal): Don't
	enable x87 instructions if only the general registers are
	allowed.
	* config/i386/i386.opt: Add -mgeneral-regs-only.
	* doc/invoke.texi: Document -mgeneral-regs-only.

gcc/testsuite/

	PR target/70738
	* gcc.target/i386/pr70738-1.c: Likewise.
	* gcc.target/i386/pr70738-2.c: Likewise.
	* gcc.target/i386/pr70738-3.c: Likewise.
	* gcc.target/i386/pr70738-4.c: Likewise.
	* gcc.target/i386/pr70738-5.c: Likewise.
	* gcc.target/i386/pr70738-6.c: Likewise.
	* gcc.target/i386/pr70738-7.c: Likewise.
	* gcc.target/i386/pr70738-8.c: Likewise.
	* gcc.target/i386/pr70738-9.c: Likewise.
---
 gcc/common/config/i386/i386-common.c      | 20 ++++++++++++++++++++
 gcc/config/i386/i386.c                    |  5 ++++-
 gcc/config/i386/i386.opt                  |  4 ++++
 gcc/doc/invoke.texi                       |  8 +++++++-
 gcc/testsuite/gcc.target/i386/pr70738-1.c |  9 +++++++++
 gcc/testsuite/gcc.target/i386/pr70738-2.c | 10 ++++++++++
 gcc/testsuite/gcc.target/i386/pr70738-3.c | 11 +++++++++++
 gcc/testsuite/gcc.target/i386/pr70738-4.c | 10 ++++++++++
 gcc/testsuite/gcc.target/i386/pr70738-5.c | 16 ++++++++++++++++
 gcc/testsuite/gcc.target/i386/pr70738-6.c | 10 ++++++++++
 gcc/testsuite/gcc.target/i386/pr70738-7.c | 13 +++++++++++++
 gcc/testsuite/gcc.target/i386/pr70738-8.c | 30 ++++++++++++++++++++++++++++++
 gcc/testsuite/gcc.target/i386/pr70738-9.c | 23 +++++++++++++++++++++++
 13 files changed, 167 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr70738-1.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr70738-2.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr70738-3.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr70738-4.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr70738-5.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr70738-6.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr70738-7.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr70738-8.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr70738-9.c

diff --git a/gcc/common/config/i386/i386-common.c b/gcc/common/config/i386/i386-common.c
index cc65c8c..b150c9e 100644
--- a/gcc/common/config/i386/i386-common.c
+++ b/gcc/common/config/i386/i386-common.c
@@ -223,6 +223,11 @@  along with GCC; see the file COPYING3.  If not see
 #define OPTION_MASK_ISA_RDRND_UNSET OPTION_MASK_ISA_RDRND
 #define OPTION_MASK_ISA_F16C_UNSET OPTION_MASK_ISA_F16C
 
+#define OPTION_MASK_ISA_GENERAL_REGS_ONLY_UNSET \
+  (OPTION_MASK_ISA_MMX_UNSET \
+   | OPTION_MASK_ISA_SSE_UNSET \
+   | OPTION_MASK_ISA_MPX)
+
 /* Implement TARGET_HANDLE_OPTION.  */
 
 bool
@@ -236,6 +241,21 @@  ix86_handle_option (struct gcc_options *opts,
 
   switch (code)
     {
+    case OPT_mgeneral_regs_only:
+      if (value)
+	{
+	  /* Disable MPX, MMX, SSE and x87 instructions if only the
+	     general registers are allowed..  */
+	  opts->x_ix86_isa_flags
+	    &= ~OPTION_MASK_ISA_GENERAL_REGS_ONLY_UNSET;
+	  opts->x_ix86_isa_flags_explicit
+	    |= OPTION_MASK_ISA_GENERAL_REGS_ONLY_UNSET;
+	  opts->x_target_flags &= ~MASK_80387;
+	}
+      else
+	gcc_unreachable ();
+      return true;
+
     case OPT_mmmx:
       if (value)
 	{
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 9f62089..4d515d2 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -5331,7 +5331,10 @@  ix86_option_override_internal (bool main_args_p,
 	    && !(opts->x_ix86_isa_flags_explicit & OPTION_MASK_ISA_PKU))
 	  opts->x_ix86_isa_flags |= OPTION_MASK_ISA_PKU;
 
-	if (!(opts_set->x_target_flags & MASK_80387))
+	/* Don't enable x87 instructions if only the general registers
+	   are allowed.  */
+	if (!(opts_set->x_target_flags & MASK_GENERAL_REGS_ONLY)
+	    && !(opts_set->x_target_flags & MASK_80387))
 	  {
 	    if (processor_alias_table[i].flags & PTA_NO_80387)
 	      opts->x_target_flags &= ~MASK_80387;
diff --git a/gcc/config/i386/i386.opt b/gcc/config/i386/i386.opt
index 36dd4bd..d12b29a 100644
--- a/gcc/config/i386/i386.opt
+++ b/gcc/config/i386/i386.opt
@@ -897,3 +897,7 @@  Enum(stack_protector_guard) String(global) Value(SSP_GLOBAL)
 mmitigate-rop
 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
+Generate code which uses only the general registers.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index a54a0af..bbb00fd 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -1171,7 +1171,7 @@  See RS/6000 and PowerPC Options.
 -msse2avx -mfentry -mrecord-mcount -mnop-mcount -m8bit-idiv @gol
 -mavx256-split-unaligned-load -mavx256-split-unaligned-store @gol
 -malign-data=@var{type} -mstack-protector-guard=@var{guard} @gol
--mmitigate-rop}
+-mmitigate-rop -mgeneral-regs-only}
 
 @emph{x86 Windows Options}
 @gccoptlist{-mconsole -mcygwin -mno-cygwin -mdll @gol
@@ -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
+prevent the compiler from using floating-point, vector, mask and bound
+registers, but will not impose any restrictions on the assembler.
+
 @end table
 
 These @samp{-m} switches are supported in addition to the above
diff --git a/gcc/testsuite/gcc.target/i386/pr70738-1.c b/gcc/testsuite/gcc.target/i386/pr70738-1.c
new file mode 100644
index 0000000..19381c2
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr70738-1.c
@@ -0,0 +1,9 @@ 
+/* { dg-do compile { target { ! ia32 } } } */
+/* { dg-options "-msse2 -mgeneral-regs-only" } */
+
+typedef int int32x2_t __attribute__ ((__vector_size__ ((8))));
+
+int32x2_t test (int32x2_t a, int32x2_t b)
+{ /* { dg-error "SSE register return with SSE disabled" } */
+  return a + b;
+}
diff --git a/gcc/testsuite/gcc.target/i386/pr70738-2.c b/gcc/testsuite/gcc.target/i386/pr70738-2.c
new file mode 100644
index 0000000..8b90904
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr70738-2.c
@@ -0,0 +1,10 @@ 
+/* { dg-do compile { target ia32 } } */
+/* { dg-options "-mmmx -mgeneral-regs-only" } */
+
+typedef int int32x2_t __attribute__ ((__vector_size__ ((8))));
+
+int32x2_t
+test (int32x2_t a, int32x2_t b) /* { dg-warning "MMX vector argument without MMX enabled" } */
+{ /* { dg-warning "MMX vector return without MMX enabled" } */
+  return a + b;
+}
diff --git a/gcc/testsuite/gcc.target/i386/pr70738-3.c b/gcc/testsuite/gcc.target/i386/pr70738-3.c
new file mode 100644
index 0000000..1ac3adb
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr70738-3.c
@@ -0,0 +1,11 @@ 
+/* { dg-do compile } */
+/* { dg-options "-msse2 -mgeneral-regs-only" } */
+
+typedef int int32x4_t __attribute__ ((__vector_size__ ((16))));
+extern int32x4_t c;
+
+void
+test (int32x4_t a, int32x4_t b) /* { dg-warning "SSE vector argument without SSE enabled" } */
+{
+  c = a + b;
+}
diff --git a/gcc/testsuite/gcc.target/i386/pr70738-4.c b/gcc/testsuite/gcc.target/i386/pr70738-4.c
new file mode 100644
index 0000000..c6d20f2
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr70738-4.c
@@ -0,0 +1,10 @@ 
+/* { dg-do compile { target ia32 } } */
+/* { dg-options "-msse2 -mgeneral-regs-only" } */
+
+typedef int int32x4_t __attribute__ ((__vector_size__ ((16))));
+
+int32x4_t
+test (int32x4_t a, int32x4_t b) /* { dg-warning "SSE vector argument without SSE enabled" } */
+{ /* { dg-warning "SSE vector return without SSE enabled" } */
+  return a + b;
+}
diff --git a/gcc/testsuite/gcc.target/i386/pr70738-5.c b/gcc/testsuite/gcc.target/i386/pr70738-5.c
new file mode 100644
index 0000000..8b43809
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr70738-5.c
@@ -0,0 +1,16 @@ 
+/* { dg-do compile { target { ! ia32 } } } */
+/* { dg-options "-msse2 -mgeneral-regs-only" } */
+
+#include <stdarg.h>
+
+typedef int int32x2_t __attribute__ ((__vector_size__ ((8))));
+
+int
+test (int i, ...)
+{
+  va_list argp;
+  va_start (argp, i);
+  int32x2_t x = (int32x2_t) {0, 1};
+  x += va_arg (argp, int32x2_t); /* { dg-error "SSE register argument with SSE disabled" } */
+  return x[0] + x[1];
+}
diff --git a/gcc/testsuite/gcc.target/i386/pr70738-6.c b/gcc/testsuite/gcc.target/i386/pr70738-6.c
new file mode 100644
index 0000000..3bccabb
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr70738-6.c
@@ -0,0 +1,10 @@ 
+/* { dg-do compile { target { ! ia32 } } } */
+/* { dg-options "-msse2 -mgeneral-regs-only" } */
+
+extern float a, b, c;
+
+void
+foo (void)
+{
+  c = a * b; /* { dg-error "SSE register return with SSE disabled" } */
+}
diff --git a/gcc/testsuite/gcc.target/i386/pr70738-7.c b/gcc/testsuite/gcc.target/i386/pr70738-7.c
new file mode 100644
index 0000000..2e5b49f
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr70738-7.c
@@ -0,0 +1,13 @@ 
+/* { dg-do compile { target ia32 } } */
+/* { dg-options "-msse2 -mgeneral-regs-only" } */
+
+extern float a, b, c;
+
+void
+foo (void)
+{
+  c = a * b;
+}
+
+/* { dg-final { scan-assembler-not "mulss" } } */
+/* { dg-final { scan-assembler "call\[ \t\]__mulsf3" } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr70738-8.c b/gcc/testsuite/gcc.target/i386/pr70738-8.c
new file mode 100644
index 0000000..0740460
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr70738-8.c
@@ -0,0 +1,30 @@ 
+/* { dg-do run } */
+/* { dg-options "-O2 -mgeneral-regs-only" } */
+
+extern void abort ();
+
+int
+dec (int a, int b)
+{
+  return a + b;
+}
+
+int
+cal (int a, int b)
+{
+  int sum1 = a * b;
+  int sum2 = a / b;
+  int sum = dec (sum1, sum2);
+  return a + b + sum + sum1 + sum2;
+}
+
+int
+main (int argc, char **argv)
+{
+  int ret = cal (2, 1);
+
+  if (ret != 11)
+    abort ();
+
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.target/i386/pr70738-9.c b/gcc/testsuite/gcc.target/i386/pr70738-9.c
new file mode 100644
index 0000000..c71f0b0
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr70738-9.c
@@ -0,0 +1,23 @@ 
+/* { dg-do run } */
+/* { dg-options "-O2 -mgeneral-regs-only" } */
+
+extern void abort ();
+
+int
+cal (int a, int b)
+{
+  int sum = a + b;
+  int sum1 = a * b;
+  return (a + b + sum + sum1);
+}
+
+int
+main (int argc, char **argv)
+{
+  int ret = cal (1, 2);
+
+  if (ret != 8)
+    abort ();
+
+  return 0;
+}
-- 
2.5.5