> On April 6, 2012, 12:33 p.m., Gabe Black wrote:
> > SConstruct, line 510
> > <http://reviews.gem5.org/r/1119/diff/2/?file=25598#file25598line510>
> >
> >     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.

It applies to 4.6 as well.


> On April 6, 2012, 12:33 p.m., Gabe Black wrote:
> > src/arch/sparc/isa/decoder.isa, line 686
> > <http://reviews.gem5.org/r/1119/diff/2/?file=25604#file25604line686>
> >
> >     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.

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. 


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

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


> On April 6, 2012, 12:33 p.m., Gabe Black wrote:
> > src/base/hashmap.hh, line 57
> > <http://reviews.gem5.org/r/1119/diff/2/?file=25609#file25609line57>
> >
> >     >= or >? gcc 4.6 was working the last I checked.

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 :)


> On April 6, 2012, 12:33 p.m., Gabe Black wrote:
> > src/mem/ruby/common/Address.hh, line 34
> > <http://reviews.gem5.org/r/1119/diff/2/?file=25613#file25613line34>
> >
> >     Leftovers from debugging?

I think gcc complained...but I'll double check


> On April 6, 2012, 12:33 p.m., Gabe Black wrote:
> > src/sim/init.hh, line 49
> > <http://reviews.gem5.org/r/1119/diff/2/?file=25620#file25620line49>
> >
> >     uint8_t?

Sounds good to me. Will change


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

See earlier comment about the ambiguous isnan.


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

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.


> On April 6, 2012, 12:33 p.m., Gabe Black wrote:
> > src/SConscript, line 855
> > <http://reviews.gem5.org/r/1119/diff/2/?file=25600#file25600line855>
> >
> >     Same >= vs. > question here.

...and same answer...it applies to gcc 4.6 as well.


- Andreas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/1119/#review2485
-----------------------------------------------------------


On April 6, 2012, 9:01 a.m., Andreas Hansson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/1119/
> -----------------------------------------------------------
> 
> (Updated April 6, 2012, 9:01 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 f51b4b4f0d5e 
>   ext/libelf/SConscript f51b4b4f0d5e 
>   src/SConscript f51b4b4f0d5e 
>   src/arch/alpha/mt.hh f51b4b4f0d5e 
>   src/arch/arm/isa/includes.isa f51b4b4f0d5e 
>   src/arch/power/isa/includes.isa f51b4b4f0d5e 
>   src/arch/sparc/isa/decoder.isa f51b4b4f0d5e 
>   src/arch/sparc/mt.hh f51b4b4f0d5e 
>   src/arch/sparc/tlb_map.hh f51b4b4f0d5e 
>   src/arch/x86/isa/microops/fpop.isa f51b4b4f0d5e 
>   src/arch/x86/isa/microops/mediaop.isa f51b4b4f0d5e 
>   src/base/hashmap.hh f51b4b4f0d5e 
>   src/base/inifile.cc f51b4b4f0d5e 
>   src/base/range.hh f51b4b4f0d5e 
>   src/base/stats/text.cc f51b4b4f0d5e 
>   src/mem/ruby/common/Address.hh f51b4b4f0d5e 
>   src/mem/ruby/network/fault_model/FaultModel.hh f51b4b4f0d5e 
>   src/mem/ruby/network/fault_model/FaultModel.cc f51b4b4f0d5e 
>   src/mem/ruby/network/garnet/BaseGarnetNetwork.cc f51b4b4f0d5e 
>   src/mem/ruby/network/orion/OrionConfig.hh f51b4b4f0d5e 
>   src/mem/ruby/network/orion/OrionRouter.cc f51b4b4f0d5e 
>   src/mem/ruby/network/orion/TechParameter.hh f51b4b4f0d5e 
>   src/sim/init.hh f51b4b4f0d5e 
>   src/sim/init.cc f51b4b4f0d5e 
> 
> 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

Reply via email to