x86: Set optimize to INT_MAX for -Os

Message ID 20190316235033.26752-1-hjl.tools@gmail.com
State New
Headers show
Series
  • x86: Set optimize to INT_MAX for -Os
Related show

Commit Message

H.J. Lu March 16, 2019, 11:50 p.m.
Set optimize to INT_MAX, instead of -1, for -Os so that -Os will include
-O2 optimization.

	PR gas/24353
	* config/tc-i386.c (md_parse_option): Set optimize to INT_MAX
	for -Os.
	* testsuite/gas/i386/optimize-2.s: Add a test.
	* testsuite/gas/i386/x86-64-optimize-3.s: Likewise.
	* testsuite/gas/i386/optimize-2.d: Updated.
	* testsuite/gas/i386/x86-64-optimize-3.d: Likewise.
---
 gas/ChangeLog                              | 12 ++++++++++++
 gas/config/tc-i386.c                       | 13 ++++++++++++-
 gas/testsuite/gas/i386/optimize-2.d        |  1 +
 gas/testsuite/gas/i386/optimize-2.s        |  2 ++
 gas/testsuite/gas/i386/x86-64-optimize-3.d |  1 +
 gas/testsuite/gas/i386/x86-64-optimize-3.s |  2 ++
 6 files changed, 30 insertions(+), 1 deletion(-)

-- 
2.20.1

Comments

Jan Beulich March 18, 2019, 11:35 a.m. | #1
>>> On 17.03.19 at 00:50, <hjl.tools@gmail.com> wrote:

> Set optimize to INT_MAX, instead of -1, for -Os so that -Os will include

> -O2 optimization.


I didn't think -Os should be a superset of -O2. Iirc it's not for e.g.
gcc. Why would this be a good idea? Judging by the PR it should
rather be the doc to get changed imo.

Jan
H.J. Lu March 19, 2019, 1:43 a.m. | #2
On Mon, Mar 18, 2019 at 7:35 PM Jan Beulich <JBeulich@suse.com> wrote:
>

> >>> On 17.03.19 at 00:50, <hjl.tools@gmail.com> wrote:

> > Set optimize to INT_MAX, instead of -1, for -Os so that -Os will include

> > -O2 optimization.

>

> I didn't think -Os should be a superset of -O2. Iirc it's not for e.g.

> gcc. Why would this be a good idea? Judging by the PR it should

> rather be the doc to get changed imo.


-Os was designed to include -O2.

-- 
H.J.
Jan Beulich March 19, 2019, 8:07 a.m. | #3
>>> On 19.03.19 at 02:43, <hjl.tools@gmail.com> wrote:

> On Mon, Mar 18, 2019 at 7:35 PM Jan Beulich <JBeulich@suse.com> wrote:

>>

>> >>> On 17.03.19 at 00:50, <hjl.tools@gmail.com> wrote:

>> > Set optimize to INT_MAX, instead of -1, for -Os so that -Os will include

>> > -O2 optimization.

>>

>> I didn't think -Os should be a superset of -O2. Iirc it's not for e.g.

>> gcc. Why would this be a good idea? Judging by the PR it should

>> rather be the doc to get changed imo.

> 

> -Os was designed to include -O2.


Well, taking compilers for comparison, -O<number> typically implies
optimization for performance. This often is requiring different
adjustments to generated code than size optimizations. I understand
(from the doc fragment) that it was meant to be that way; I wonder
though whether this shouldn't be re-considered (and to be honest,
back when you had introduced it, I merely looked at code and hence
didn't realize that code - matching my expectations - wasn't in line
with the doc).

Jan
Paul Koning March 19, 2019, 1:32 p.m. | #4
> On Mar 19, 2019, at 4:07 AM, Jan Beulich <JBeulich@suse.com> wrote:

> 

>>>> On 19.03.19 at 02:43, <hjl.tools@gmail.com> wrote:

>> On Mon, Mar 18, 2019 at 7:35 PM Jan Beulich <JBeulich@suse.com> wrote:

>>> 

>>>>>> On 17.03.19 at 00:50, <hjl.tools@gmail.com> wrote:

>>>> Set optimize to INT_MAX, instead of -1, for -Os so that -Os will include

>>>> -O2 optimization.

>>> 

>>> I didn't think -Os should be a superset of -O2. Iirc it's not for e.g.

>>> gcc. Why would this be a good idea? Judging by the PR it should

>>> rather be the doc to get changed imo.

>> 

>> -Os was designed to include -O2.

> 

> Well, taking compilers for comparison, -O<number> typically implies

> optimization for performance. This often is requiring different

> adjustments to generated code than size optimizations. I understand

> (from the doc fragment) that it was meant to be that way; I wonder

> though whether this shouldn't be re-considered (and to be honest,

> back when you had introduced it, I merely looked at code and hence

> didn't realize that code - matching my expectations - wasn't in line

> with the doc).


Agreed.  It's certainly not the case that -Os is a superset of -O2, and if it were that would be a serious bug.  For example (in gcc) -O2 does loop unrolling, which in general -Os should not do.  Also, quite apart from specific detail optimizations, -Os also affects internals like the cost calculation in the back end.  The documentation states that the intent is for the cost function to return a number measuring execution time for -O<number>, but a number relating to code size for -Os.  And indeed at least some of the back ends implement this.

	paul

Patch

diff --git a/gas/ChangeLog b/gas/ChangeLog
index e3cba86a0d..e5907d1b5a 100644
--- a/gas/ChangeLog
+++ b/gas/ChangeLog
@@ -1,3 +1,15 @@ 
+2019-03-17  H.J. Lu  <hongjiu.lu@intel.com>
+
+	PR gas/24353
+	* config/tc-i386.c: Include <limits.h> if it exists and try
+	including <sys/param.h> if we have it.
+	(INT_MAX): Define if not defined.
+	(md_parse_option): Set optimize to INT_MAX for -Os.
+	* testsuite/gas/i386/optimize-2.s: Add a test.
+	* testsuite/gas/i386/x86-64-optimize-3.s: Likewise.
+	* testsuite/gas/i386/optimize-2.d: Updated.
+	* testsuite/gas/i386/x86-64-optimize-3.d: Likewise.
+
 2019-03-17  H.J. Lu  <hongjiu.lu@intel.com>
 
 	PR gas/24352
diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
index 959fda2b84..8047ddf8b1 100644
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -33,6 +33,17 @@ 
 #include "elf/x86-64.h"
 #include "opcodes/i386-init.h"
 
+#ifdef HAVE_LIMITS_H
+#include <limits.h>
+#else
+#ifdef HAVE_SYS_PARAM_H
+#include <sys/param.h>
+#endif
+#ifndef INT_MAX
+#define INT_MAX (int) (((unsigned) (-1)) >> 1)
+#endif
+#endif
+
 #ifndef REGISTER_WARNINGS
 #define REGISTER_WARNINGS 1
 #endif
@@ -11350,7 +11361,7 @@  md_parse_option (int c, const char *arg)
 	{
 	  optimize_for_space = 1;
 	  /* Turn on all encoding optimizations.  */
-	  optimize = -1;
+	  optimize = INT_MAX;
 	}
       else
 	{
diff --git a/gas/testsuite/gas/i386/optimize-2.d b/gas/testsuite/gas/i386/optimize-2.d
index ec989b0e13..e8a516997a 100644
--- a/gas/testsuite/gas/i386/optimize-2.d
+++ b/gas/testsuite/gas/i386/optimize-2.d
@@ -16,4 +16,5 @@  Disassembly of section .text:
  +[a-f0-9]+:	f6 c3 7f             	test   \$0x7f,%bl
  +[a-f0-9]+:	f7 c7 7f 00 00 00    	test   \$0x7f,%edi
  +[a-f0-9]+:	66 f7 c7 7f 00       	test   \$0x7f,%di
+ +[a-f0-9]+:	c5 f1 55 e9          	vandnpd %xmm1,%xmm1,%xmm5
 #pass
diff --git a/gas/testsuite/gas/i386/optimize-2.s b/gas/testsuite/gas/i386/optimize-2.s
index b427a741b9..c9b57a8dd1 100644
--- a/gas/testsuite/gas/i386/optimize-2.s
+++ b/gas/testsuite/gas/i386/optimize-2.s
@@ -11,3 +11,5 @@  _start:
 	test	$0x7f, %bl
 	test	$0x7f, %edi
 	test	$0x7f, %di
+
+	vandnpd	%zmm1, %zmm1, %zmm5
diff --git a/gas/testsuite/gas/i386/x86-64-optimize-3.d b/gas/testsuite/gas/i386/x86-64-optimize-3.d
index b46f728dd8..f85c0af05e 100644
--- a/gas/testsuite/gas/i386/x86-64-optimize-3.d
+++ b/gas/testsuite/gas/i386/x86-64-optimize-3.d
@@ -24,4 +24,5 @@  Disassembly of section .text:
  +[a-f0-9]+:	41 f6 c1 7f          	test   \$0x7f,%r9b
  +[a-f0-9]+:	41 f6 c1 7f          	test   \$0x7f,%r9b
  +[a-f0-9]+:	41 f6 c1 7f          	test   \$0x7f,%r9b
+ +[a-f0-9]+:	c5 f1 55 e9          	vandnpd %xmm1,%xmm1,%xmm5
 #pass
diff --git a/gas/testsuite/gas/i386/x86-64-optimize-3.s b/gas/testsuite/gas/i386/x86-64-optimize-3.s
index 61c150a87c..4a52a25ddd 100644
--- a/gas/testsuite/gas/i386/x86-64-optimize-3.s
+++ b/gas/testsuite/gas/i386/x86-64-optimize-3.s
@@ -19,3 +19,5 @@  _start:
 	test	$0x7f, %r9d
 	test	$0x7f, %r9w
 	test	$0x7f, %r9b
+
+	vandnpd	%zmm1, %zmm1, %zmm5