> On April 6, 2012, 12:33 p.m., Gabe Black wrote: > > src/arch/sparc/tlb_map.hh, line 101 > > <http://reviews.gem5.org/r/1119/diff/2/?file=25606#file25606line101> > > > > Space after ,? I'm not sure it's explicitly in the style guide, but to > > me it is in spirit at least. > > Andreas Hansson wrote: > Am I missing something? There is a space after the comma in make_pair(r, > d).
In the template types. > On April 6, 2012, 12:33 p.m., Gabe Black wrote: > > src/base/hashmap.hh, line 63 > > <http://reviews.gem5.org/r/1119/diff/2/?file=25609#file25609line63> > > > > #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. > > Andreas Hansson wrote: > 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 wrote: > 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? 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. > On April 6, 2012, 12:33 p.m., Gabe Black wrote: > > src/base/stats/text.cc, line 169 > > <http://reviews.gem5.org/r/1119/diff/2/?file=25612#file25612line169> > > > > "using std::isnan;" since this shows up a bunch in this file. > > Andreas Hansson wrote: > See earlier comment about the ambiguous isnan. > > Andreas Hansson wrote: > 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 wrote: > 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? I guess we're stuck with the std:: then. Welcome to the future! (grumble, grumble) - Gabe ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1119/#review2485 ----------------------------------------------------------- On April 7, 2012, 6:14 a.m., Andreas Hansson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1119/ > ----------------------------------------------------------- > > (Updated April 7, 2012, 6:14 a.m.) > > > Review request for Default. > > > 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: > > 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". > > > Diffs > ----- > > SConstruct a47fd7c2d44e > ext/libelf/SConscript a47fd7c2d44e > src/SConscript a47fd7c2d44e > src/arch/alpha/isa/main.isa a47fd7c2d44e > src/arch/alpha/mt.hh a47fd7c2d44e > src/arch/arm/isa/includes.isa a47fd7c2d44e > src/arch/mips/isa/includes.isa a47fd7c2d44e > src/arch/power/isa/includes.isa a47fd7c2d44e > src/arch/sparc/isa/decoder.isa a47fd7c2d44e > src/arch/sparc/mt.hh a47fd7c2d44e > src/arch/sparc/tlb_map.hh a47fd7c2d44e > src/arch/x86/isa/microops/fpop.isa a47fd7c2d44e > src/arch/x86/isa/microops/mediaop.isa a47fd7c2d44e > src/base/hashmap.hh a47fd7c2d44e > src/base/inifile.cc a47fd7c2d44e > src/base/range.hh a47fd7c2d44e > src/base/stats/text.cc a47fd7c2d44e > src/mem/ruby/common/Address.hh a47fd7c2d44e > src/mem/ruby/network/fault_model/FaultModel.hh a47fd7c2d44e > src/mem/ruby/network/fault_model/FaultModel.cc a47fd7c2d44e > src/mem/ruby/network/garnet/BaseGarnetNetwork.cc a47fd7c2d44e > src/mem/ruby/network/orion/OrionConfig.hh a47fd7c2d44e > src/mem/ruby/network/orion/OrionRouter.cc a47fd7c2d44e > src/mem/ruby/network/orion/TechParameter.hh a47fd7c2d44e > src/sim/init.hh a47fd7c2d44e > src/sim/init.cc a47fd7c2d44e > > Diff: http://reviews.gem5.org/r/1119/diff/ > > > Testing > ------- > > 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 > > > Thanks, > > Andreas Hansson > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
