clang/gcc: Fix compilation issues with clang 3.0 and gcc 4.6

Information
Submitter:	Andreas Hansson
Repository:	gem5
Branch:	default
Bugs:
Depends On:
Reviewers
Groups:	Default
People:

Description

clang/gcc: Fix compilation issues with clang 3.0 and gcc 4.6

This patch addresses a number of minor issues that cause problems when
compiling with clang >= 3.0 and gcc >= 4.6. Most importantly, it
avoids using the deprecated ext/hash_map and instead uses
unordered_map (and similarly so for the hash_set). To make use of the
new STL containers, g++ and clang has to be invoked with "-std=c++0x",
and this is now added for all gcc versions >= 4.6, and for clang >=
3.0. For gcc >= 4.3 and <= 4.5 and clang <= 3.0 we use the tr1
unordered_map to avoid the deprecation warning.

The addition of c++0x in turn causes a few problems, as the
compiler is more stringent and adds a number of new warnings. Below,
the most important issues are enumerated:

1) the use of namespaces is more strict, e.g. for isnan, and all
   headers opening the entire namespace std are now fixed.

2) another other issue caused by the more stringent compiler is the
   narrowing of the embedded python, which used to be a char array,
   and is now unsigned char since there were values larger than 128.

3) a particularly odd issue that arose with the new c++0x behaviour is
   found in range.hh, where the operator< causes gcc to complain about
   the template type parsing (the "<" is interpreted as the beginning
   of a template argument), and the problem seems to be related to the
   begin/end members introduced for the range-type iteration, which is
   a new feature in c++11.

As a minor update, this patch also fixes the build flags for the clang
debug target that used to be shared with gcc and incorrectly use
"-ggdb".

Testing Done

util/regress all passing (disregarding t1000 and eio) with gcc 4.6.2, 
and compiling with clang 2.9 and 3.0 on Ubuntu 12.04 and MacOSX 10.7.3

Issue Summary

Description	From	Last Updated	Status
space after >=. Isn't this a problem with versions after 4.6? I don't remember the semantics of compareVersions, but I ...	Gabe Black	April 6, 2012, 8:52 p.m.	Resolved
space after >=	Gabe Black	April 6, 2012, 8:52 p.m.	Resolved
You might want to even use uint8_t here since there's technically no requirement that a char is one byte.	Gabe Black	April 6, 2012, 8:52 p.m.	Resolved
Same >= vs. > question here.	Gabe Black	April 6, 2012, 8:52 p.m.	Resolved
Yes, thank you.	Gabe Black	April 6, 2012, 8:52 p.m.	Resolved
You can inject a "using std::isnan;" into the decoder .cc and avoid all these individual changes. Please let me know ...	Gabe Black	April 6, 2012, 8:52 p.m.	Dropped
Space after ,? I'm not sure it's explicitly in the style guide, but to me it is in spirit at ...	Gabe Black	April 6, 2012, 8:52 p.m.	Resolved
Likewise for these microops, although since it only crops up on a couple lines it may not be that big ...	Gabe Black	April 6, 2012, 8:52 p.m.	Dropped
>= or >? gcc 4.6 was working the last I checked.	Gabe Black	April 6, 2012, 8:52 p.m.	Dropped
#defines are bad. They change the code behind the programmers back and can accidentally apply places they didn't intend. Can ...	Gabe Black	April 7, 2012, 10:07 p.m.	Dropped
"using std::isnan;" since this shows up a bunch in this file.	Gabe Black	April 7, 2012, 10:07 p.m.	Dropped
Leftovers from debugging?	Gabe Black	April 6, 2012, 8:55 p.m.	Dropped
uint8_t?	Gabe Black	April 7, 2012, 10:07 p.m.	Resolved

it's fine with the caveat that things which are deprecated today are normally gone tomorrow. Putting the no-deprecated warning is a bit dangerous. Is there perhaps a hash_map replacement that we could grab with version ifdefs?

Andreas Hansson April 2, 2012, 7:31 a.m. (April 2, 2012, 7:31 a.m.)

The new data structure that replaces ext/hash_map is unordered_map. However, ifdefs won't do the trick, as it seems we also have to add -std=c++0x on the command line for this to work, but I'll do some more digging around.

Nathan Binkert April 2, 2012, 7:34 a.m. (April 2, 2012, 7:34 a.m.)

Seems like we should have a conf.CheckHeader for the unordered map and do the proper #ifdefs off that in base/hashmap.hh

Andreas Hansson April 2, 2012, 7:49 a.m. (April 2, 2012, 7:49 a.m.)

Sorry if it is covered by CheckHeader, but how does that solve the inclusion of -std=c++0x in the compiler flags, or do you suggest to do that unconditionally?

Nathan Binkert April 2, 2012, 10:14 a.m. (April 2, 2012, 10:14 a.m.)

The Configure() object (where CheckHeader comes from) takes an environment.  If you want to make it entirely programmatic, you can clone the main environment.  Add -std=c++0x, create a configure object and then use CheckHeader.  If all that passes, you can add the -std=c++0x to the main environment itself, and then you can add a flag to the export_vars which will give you a #include that you can grab a #define from.

(It may be entirelly possible t add the -std=c++0x flag to CheckHeader itself.  I'm just not sure.  I'd probably figure that out first.)

Nathan Binkert April 2, 2012, 10:15 a.m. (April 2, 2012, 10:15 a.m.)

I'm also not opposed to -std=c++0x for all clang if that works.  (Or just checking the version of clang and doing it for certain versions.)

Andreas Hansson April 2, 2012, 10:29 a.m. (April 2, 2012, 10:29 a.m.)

I should add that the same problem arises with gcc >= 4.6 and that there currently is a corresponding "-Wno-deprecated" for these cases. Thus, what ever we choose for clang we should also do for gcc >= 4.6.

Ali Saidi April 2, 2012, 11:08 a.m. (April 2, 2012, 11:08 a.m.)

Ok. Well feel free to not bother for clang then, however I imagine we're going to need a solution when 4.7 is released, so if you're half way through it might be good to just finish the job now.

Summary:

-	clang: Fix compilation problems with clang 3.0 on Ubuntu 12.04
+	clang/gcc: Fix compilation issues with clang 3.0 and gcc 4.6

Description:

~		clang: Fix compilation problems with clang 3.0 on Ubuntu 12.04
	~	clang/gcc: Fix compilation issues with clang 3.0 and gcc 4.6

~		This patch disables the warning for deprecated headers since the
~		ext/hash_map is considered deprecated with clang 3.0 on Ubuntu 12.04
~		(compiled against gcc 4.6 headers). It also fixes the build flags for
~		the debug target that used to be shared with gcc.
	~	This patch addresses a number of minor issues that cause problems when
	~	compiling with clang >= 3.0 and gcc >= 4.6. Most importantly, it
	~	avoids using the deprecated ext/hash_map and instead uses
	~	unordered_map (and similarly so for the hash_set). To make use of the
	+	new STL containers, g++ and clang has to be invoked with "-std=c++0x",
	+	and this is now added for all gcc versions >= 4.6, and always for
	+	clang since even 2.9 supports this. The addition of c++0x in turn
	+	causes a few problems, as the compiler is more stringent and adds a
	+	number of new warnings. Below, the most important issues are
	+	enumerated:
	+
	+	1) the use of namespaces is more strict, e.g. for isnan, and all
	+	headers opening the entire namespace std are now fixed.
	+
	+	2) another other issue caused by the more stringent compiler is the
	+	narrowing of the embedded python, which used to be a char array,
	+	and is now unsigned char since there were values larger than 128.
	+
	+	3) a particularly odd issue that arose with the new c++0x behaviour is
	+	found in range.hh, where the operator< causes gcc to complain about
	+	the template type parsing (the "<" is interpreted as the beginning
	+	of a template argument), and the problem seems to be related to the
	+	begin/end members introduced for the range-type iteration, which is
	+	a new feature in c++11.
	+
	+	As a minor update, this patch also fixes the build flags for the clang
	+	debug target that used to be shared with gcc and incorrectly use
	+	"-ggdb".

Diff:

Revision 2 (+109 -59)

Show changes

	SConstruct
	ext/libelf/SConscript
	src/SConscript
	src/arch/alpha/mt.hh
	src/arch/arm/isa/includes.isa
	src/arch/power/isa/includes.isa
	src/arch/sparc/mt.hh
	src/arch/sparc/tlb_map.hh
	16 more

SConstruct (Diff revision 2)

Nice to know that the code is 0x compliant (when are they going to change the flag to c++11?

Andreas Hansson April 6, 2012, 2:46 a.m. (April 6, 2012, 2:46 a.m.)
```
gcc 4.7
```

SConstruct (Diff revision 2)

space after >=. Isn't this a problem with versions *after* 4.6? I don't remember the semantics of compareVersions, but I would expect >= would return true on 4.6 as well.

Show all issues

Andreas Hansson April 6, 2012, 6:03 p.m. (April 6, 2012, 6:03 p.m.)
```
It applies to 4.6 as well.
```

ext/libelf/SConscript (Diff revision 2)

space after >=

Show all issues

src/SConscript (Diff revision 2)

You might want to even use uint8_t here since there's technically no requirement that a char is one byte.

Show all issues

src/SConscript (Diff revision 2)

Same >= vs. > question here.

Show all issues

Andreas Hansson April 6, 2012, 6:03 p.m. (April 6, 2012, 6:03 p.m.)
```
...and same answer...it applies to gcc 4.6 as well.
```

src/arch/alpha/mt.hh (Diff revision 2)

Yes, thank you.

Show all issues

src/arch/sparc/isa/decoder.isa (Diff revision 2)

You can inject a "using std::isnan;" into the decoder .cc and avoid all these individual changes. Please let me know if you need help doing that.

Show all issues

Andreas Hansson April 6, 2012, 6:03 p.m. (April 6, 2012, 6:03 p.m.)

That was actually causing the problem since apparently there is a whole plethora of isnan (in the :: namespace) and it then becomes ambiguous which one to use.

src/arch/sparc/tlb_map.hh (Diff revision 2)

Space after ,? I'm not sure it's explicitly in the style guide, but to me it is in spirit at least.

Show all issues

Andreas Hansson April 6, 2012, 6:03 p.m. (April 6, 2012, 6:03 p.m.)

Am I missing something? There is a space after the comma in make_pair(r, d).

Gabe Black April 7, 2012, 8:30 a.m. (April 7, 2012, 8:30 a.m.)
```
In the template types.
```

src/arch/x86/isa/microops/fpop.isa (Diff revision 2)

Likewise for these microops, although since it only crops up on a couple lines it may not be that big of a problem.

Show all issues

src/base/hashmap.hh (Diff revision 2)

>= or >? gcc 4.6 was working the last I checked.

Show all issues

Andreas Hansson April 6, 2012, 6:03 p.m. (April 6, 2012, 6:03 p.m.)

gcc 4.6 was working because we stuck -Wno-deprecated in the global CCFLAGS. We could keep on doing that and ignoring that things are considered deprecated, but this patch tries to do some future proofing :)

src/base/hashmap.hh (Diff revision 2)

#defines are bad. They change the code behind the programmers back and can accidentally apply places they didn't intend. Can you use typedefs with templates? Or create new templates which wrap the old ones.

Show all issues

Andreas Hansson April 6, 2012, 6:03 p.m. (April 6, 2012, 6:03 p.m.)

I'll give it a bash. I merely tried to keep it as similar as possible to what was already there, but your proposal makes sense...and defines are definitely not my favourites either.

Andreas Hansson April 6, 2012, 10:24 p.m. (April 6, 2012, 10:24 p.m.)

I don't think there is a good non-invasive solution here that does not require a lot of code changes (besides the existing defines). With gcc 4.7, there will be support for template aliases and then we could do something like:

template<class Key, class T, class Hash = hash<Key> >
using hash_map = std::unordered_map<Key, T, Hash>

For now, I'd suggest we stick with the defines. Acceptable?

Gabe Black April 7, 2012, 8:30 a.m. (April 7, 2012, 8:30 a.m.)

I would really like to see them changed, but if there's no possible way to not use #defines then I don't think we have a choice.

src/base/stats/text.cc (Diff revision 2)

"using std::isnan;" since this shows up a bunch in this file.

Show all issues

Andreas Hansson April 6, 2012, 6:03 p.m. (April 6, 2012, 6:03 p.m.)
```
See earlier comment about the ambiguous isnan.
```

Andreas Hansson April 6, 2012, 8:36 p.m. (April 6, 2012, 8:36 p.m.)

build/ARM/base/stats/text.cc:216:43: error: call of overloaded 'isnan(const Result&)' is ambiguous
build/ARM/base/stats/text.cc:216:43: note: candidates are:
/usr/include/x86_64-linux-gnu/bits/mathcalls.h:236:1: note: int isnan(double)
/usr/include/c++/4.6/cmath:552:3: note: bool std::isnan(long double)
/usr/include/c++/4.6/cmath:544:3: note: bool std::isnan(float)
/usr/include/c++/4.6/cmath:548:3: note: bool std::isnan(double)


I'll see if I can track down why on earth mathcalls.h is included (probably somewhere we use math.h)

Andreas Hansson April 6, 2012, 8:47 p.m. (April 6, 2012, 8:47 p.m.)

Similar story: https://lists.webkit.org/pipermail/webkit-unassigned/2011-April/319904.html

To summarise, gcc cannot tell if we refer to the C99 version, int isnan(double), or the version in std; We can either chase down every inclusion path to math.h (which I an not sure we could even do if we tried, or change all uses of the unprefixed isnan to std::isnan as I did. Suggestions?

Gabe Black April 7, 2012, 8:30 a.m. (April 7, 2012, 8:30 a.m.)

I guess we're stuck with the std:: then. Welcome to the future! (grumble, grumble)

src/mem/ruby/common/Address.hh (Diff revision 2)

Leftovers from debugging?

Show all issues

Andreas Hansson April 6, 2012, 6:03 p.m. (April 6, 2012, 6:03 p.m.)
```
I think gcc complained...but I'll double check
```

Andreas Hansson April 6, 2012, 8:56 p.m. (April 6, 2012, 8:56 p.m.)

build/ARM/mem/ruby/common/Address.hh: In function 'std::ostream& operator<<(std::ostream&, const Address&)':
build/ARM/mem/ruby/common/Address.hh:112:12: error: 'flush' is not a member of 'std'
scons: *** [build/ARM/mem/ruby/common/Address.o] Error 1

src/sim/init.hh (Diff revision 2)

uint8_t?

Show all issues

Andreas Hansson April 6, 2012, 6:03 p.m. (April 6, 2012, 6:03 p.m.)
```
Sounds good to me. Will change
```

Diff:

Revision 3 (+115 -63)

Show changes

	SConstruct
	ext/libelf/SConscript
	src/SConscript
	src/arch/alpha/mt.hh
	src/arch/alpha/isa/main.isa
	src/arch/arm/isa/includes.isa
	src/arch/mips/isa/includes.isa
	src/arch/power/isa/includes.isa
	18 more

Ship It!

Andreas Hansson April 8, 2012, 12:08 a.m. (April 8, 2012, 12:08 a.m.)

...and just when I thought this was done, it turns out that the removal of "-Wno-deprecated" causes problems with gcc >= 4.3 and <= 4.5. I have added the necessary bits to cover these cases as well and tested it with clang 2.9, 3.0, and 3.1; and gcc 4.2.2, 4.3.3, 4.5.2 and 4.6.3 and it all works fine.

The biggest change with respect to the previous revision is that it now has to rely on two new macros "__hash_namespace_begin" and "__hash_namespace_end" to open and close the hash namespace, this is to cover the case with gcc >=4.3 and <= 4.5 where the unordered_map is in the nested namespace std::tr1.

I hope it is still fine with everyone to ship it, but I'll wait a day or two just to be sure.

Gabe Black April 8, 2012, 4:23 a.m. (April 8, 2012, 4:23 a.m.)

What about using a namespace alias instead of another #define?

Andreas Hansson April 8, 2012, 11:38 p.m. (April 8, 2012, 11:38 p.m.)

The C++ standard does not allow namespace aliases to be used for namespace extensions, as we do with the hash function.

The result: error: namespace alias '__hash_namespace' not allowed here, assuming 'std'

In conclusion, we're stuck with the #defines. I hope that answers the question.

Diff:

Revision 4 (+151 -74)

Show changes

	SConstruct
	ext/libelf/SConscript
	src/SConscript
	src/arch/alpha/mt.hh
	src/arch/alpha/isa/main.isa
	src/arch/arm/types.hh
	src/arch/arm/isa/includes.isa
	src/arch/mips/isa/includes.isa
	22 more

Description:

		clang/gcc: Fix compilation issues with clang 3.0 and gcc 4.6

		This patch addresses a number of minor issues that cause problems when
		compiling with clang >= 3.0 and gcc >= 4.6. Most importantly, it
		avoids using the deprecated ext/hash_map and instead uses
		unordered_map (and similarly so for the hash_set). To make use of the
		new STL containers, g++ and clang has to be invoked with "-std=c++0x",
~		and this is now added for all gcc versions >= 4.6, and always for
~		clang since even 2.9 supports this. The addition of c++0x in turn
~		causes a few problems, as the compiler is more stringent and adds a
~		number of new warnings. Below, the most important issues are
~		enumerated:
	~	and this is now added for all gcc versions >= 4.6, and for clang >=
	~	3.0. For gcc >= 4.3 and <= 4.5 and clang <= 3.0 we use the tr1
	~	unordered_map to avoid the deprecation warning.
	~
	~	The addition of c++0x in turn causes a few problems, as the
	+	compiler is more stringent and adds a number of new warnings. Below,
	+	the most important issues are enumerated:

		1) the use of namespaces is more strict, e.g. for isnan, and all
		headers opening the entire namespace std are now fixed.

		2) another other issue caused by the more stringent compiler is the
		narrowing of the embedded python, which used to be a char array,
		and is now unsigned char since there were values larger than 128.

		3) a particularly odd issue that arose with the new c++0x behaviour is
		found in range.hh, where the operator< causes gcc to complain about
		the template type parsing (the "<" is interpreted as the beginning
		of a template argument), and the problem seems to be related to the
		begin/end members introduced for the range-type iteration, which is
		a new feature in c++11.

		As a minor update, this patch also fixes the build flags for the clang
		debug target that used to be shared with gcc and incorrectly use
		"-ggdb".

Diff:

Revision 5 (+199 -77)

Show changes

	SConstruct
	ext/libelf/SConscript
	src/SConscript
	src/arch/alpha/mt.hh
	src/arch/alpha/isa/main.isa
	src/arch/arm/types.hh
	src/arch/arm/isa/includes.isa
	src/arch/mips/isa/includes.isa
	26 more

You have a pending review.

Review Board 2.0.15

This change has been marked as submitted.

Screenshots

Files

Issue Summary

Summary:

Description:

Diff:

Diff:

Diff:

Description:

Diff:

Status: Closed (submitted)