clang: Fix recently introduced clang compilation errors

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

Description

clang: Fix recently introduced clang compilation errors

This patch makes the code compile with clang 2.9 and 3.0 again by
making two very minor changes. Firt, it maintains a strict typing in
the forward declaration of the BaseCPUParams. Second, it adds a
FullSystemInt flag of the type unsigned int next to the boolean
FullSystem flag. The FullSystemInt variable can be used in
decode-statements (expands to switch statements) in the instruction
decoder.

Testing Done

compile with clang 2.9/3.0 and gcc 4.5/4.6 and util/regress all passing (disregarding t1000 and eio)

Ship It!

Did you consider my idea of changing the decoder to use "FullSystem ? 1 : 0" in the switch?  It's not pretty but it is a more localized solution.

If you considered it and rejected it, that's fine, just wanted to know...

Gabe Black March 9, 2012, 3:55 a.m. (March 9, 2012, 3:55 a.m.)

I would really prefer FullSystem ? 1 : 0. FullSystem is really a bool, and the decoder using it as an int is the thing causing the problem, not that it's declared as a bool.

Andreas Hansson March 9, 2012, 6:52 a.m. (March 9, 2012, 6:52 a.m.)

You guys will have to help me please isa_parser.py in how to format that in the *.isa files. Where it now says "decode FullSystem", what must it say? I've tried:

decode FullSystem ? 0 : 1 

decode (FullSystem ? 0 : 1)

decode {FullSystem ? 0 : 1}

Suggestions are welcome. I'll try and reverse engineer the parser in the meanwhile.

Gabe Black March 9, 2012, 8:52 a.m. (March 9, 2012, 8:52 a.m.)

I'd like to be able to put more free form expressions in the decode blocks, but right now you're limited to just single identifiers. You could define a new value called, for instance, FullSystemInt, or add a macro which expands to the expression you want. If you go that way, it would make sense I think to put FullSystemInt in the same file as FullSystem

I have a half written change which disconnects the global notion of FullSystem from the decoders, and if/when I get that done the types for the local and global version can (and it seems should) be different.

Description:

		clang: Fix recently introduced clang compilation errors

		This patch makes the code compile with clang 2.9 and 3.0 again by
		making two very minor changes. Firt, it maintains a strict typing in
~		the forward declaration of the BaseCPUParams. Second, it makes the
~		FullSystem flag an unsigned int rather than a bool since the variable
~		is used in switch-statements in the instruction decoder. The
~		alternative is to introduce if/else in the decoder specification, but
~		this change would be more invasive.
	~	the forward declaration of the BaseCPUParams. Second, it adds a
	~	FullSystemInt flag of the type unsigned int next to the boolean
	~	FullSystem flag. The FullSystemInt variable can be used in
	~	decode-statements (expands to switch statements) in the instruction
	~	decoder.

Diff:

Revision 2 (+23 -9)

Show changes

	src/arch/alpha/isa/decoder.isa
	src/arch/mips/isa/decoder.isa
	src/arch/x86/isa/decoder/one_byte_opcodes.isa
	src/arch/x86/isa/decoder/two_byte_opcodes.isa
	src/cpu/base.hh
	src/sim/full_system.hh
	src/sim/root.cc

Diff:

Revision 3 (+23 -9)

Show changes

	src/arch/alpha/isa/decoder.isa
	src/arch/mips/isa/decoder.isa
	src/arch/x86/isa/decoder/one_byte_opcodes.isa
	src/arch/x86/isa/decoder/two_byte_opcodes.isa
	src/cpu/base.hh
	src/sim/full_system.hh
	src/sim/root.cc

Thanks, Andreas.  Peronally I'd prefer to use a macro, since then you don't have two independent variables that really reflect the same thing.  However, it sounds like Gabe has further changes in the pipeline, and if this is just a stopgap anyway then it's not worth quibbling about.

You have a pending review.

Review Board 2.0.15

This change has been marked as submitted.

Screenshots

Files

Description:

Diff:

Diff:

Status: Closed (submitted)