Reduce superfluous move instructions #32
Conversation
This commit reduces superfluous move instructions in the object code (at least for x86_64) by changing internal representation of HSA registers in gccbrig. Instead representing HSA's untyped registers as unsigned int the gccbrig analyzes brig code and builds the register variables as a type used in tree expressions at most. This gives better chance to optimize CONVERT_VIEW_EXPRs away.
| else | ||
| element = convert (operand_type, element); | ||
| } | ||
| element = build_reinterpret_cast (operand_type, element); |
There was a problem hiding this comment.
Is this correct? Don't we need proper casting (via the convert call) when the integer sizes differ to ensure no garbage is left to the upper bits?
There was a problem hiding this comment.
I changed build_reinterpret_cast to do unsigned convert (zero extend or clip out upper bits when sizes differ). Signedness does not seem to matter here in the tests I have run but I check couple more cases for this.
|
|
||
| tree_stl_vec | ||
| brig_code_entry_handler::build_operands (const BrigInstBase &brig_inst) | ||
| { |
There was a problem hiding this comment.
Please drop the comment, why it's there? Also below?
| build_or_analyze_operands (brig_inst, /* analyze = */ true); | ||
| } | ||
|
|
||
| /* Implements both the build_operands () and analyze_operands () call |
There was a problem hiding this comment.
Also briefly explain what is being analyzed.
| } | ||
|
|
||
| /* Implements both the build_operands () and analyze_operands () call | ||
| so changes goes in tandem. Performs build_operands () when ANALYZE |
There was a problem hiding this comment.
-> "only analyze the operands and return an empty list."
| operand = build_reinterpret_cast (operand_type, operand); | ||
| else if (reg_width > instr_width) | ||
| { | ||
| /* Clip the operand because the instruction's bitwidth |
There was a problem hiding this comment.
Again, is this really safe to replace with an interpret cast?
| for (regs_use_index::const_iterator it = begin_it; it != end_it; it++) | ||
| { | ||
| std::string hsa_reg = gccbrig_hsa_reg_name_from_hash (it->first); | ||
| printf ("%s:\n", hsa_reg.c_str ()); |
There was a problem hiding this comment.
Is it correct to always output to 'stdout' with the verbose output, or is there some stream variable for it in GCC?
gcc/brig/brigfrontend/brig-util.h
Outdated
|
|
||
| struct reg_use_info | ||
| { | ||
| /* The generic tree types as the registers is interpreted and their |
There was a problem hiding this comment.
This comment is a bit unclear. Also GCC style requires two spaces after dots.
Something like this would be clearer:
"This vector keeps count of the times an HSAIL register is used for storing data of a given type..."
| /* Test for casting from/to representation of HSA registers. */ | ||
|
|
||
| /* HSA registers are untyped but in gccbrig they are presented as */ | ||
| /* variables with a type selected by analysis. Currently, each */ |
| /* HSA registers are untyped but in gccbrig they are presented as */ | ||
| /* variables with a type selected by analysis. Currently, each */ | ||
| /* register variable, per function, has a type as it is used at */ | ||
| /* most. Therefore, register variable can be nearly any type. The */ |
| { | ||
| private_u64 %foo; | ||
| private_u64 %bar; | ||
| private_b128 %baz; |
* Style fixes. * Remove reg use info printing. * Rename build_reinterpret_cast to build_resize_convert_view to reflect the new semantics better.
2 participants
This commit reduces superfluous move instructions in the object code
(at least for x86_64) by changing internal representation of HSA
registers in gccbrig. Instead representing HSA's untyped registers as
unsigned int the gccbrig analyzes brig code and builds the register
variables as a type used in tree expressions at most. This gives
better chance to optimize CONVERT_VIEW_EXPRs away.