RFR: 8312213: Remove unnecessary TEST instructions on x86 when flags reg will already be set [v4]

Tobias Hotz duke at openjdk.org
Mon Aug 7 11:45:31 UTC 2023


On Mon, 7 Aug 2023 06:15:16 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

>> Tobias Hotz has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Exchange and for or in the tests
>>   
>>   and will get matched to a test_reg_reg, so it was pointless
>
> src/hotspot/share/adlc/output_c.cpp line 4019:
> 
>> 4017:   if (inst->_flag != nullptr) {
>> 4018:     Flag* node = inst->_flag;
>> 4019:     const char* prefix = "Node::";
> 
> You could potentially make the prefix here `Node::PD::`, then the extra `PD::` could be removed from the .ad file (I don't think it really adds much?).

Well I thought that it could also be used to add some non-arch specific flags in the future, and with this the keyword would be more generic and allow for this as well.

> src/hotspot/share/adlc/output_c.cpp line 4023:
> 
>> 4021:     do {
>> 4022:       if (!node_flags_set) {
>> 4023:         fprintf(fp_cpp, "%s node->add_flag(%s%s", indent, strncmp(node->_name, prefix, strlen(prefix)) != 0 ? prefix : "", node->_name);
> 
> This seems to be guarding against a case where the flag is declared with the prefix already in the .ad file. Is this required for something?
> 
> (Otherwise I suggest just using `node->_name` here, as it forces the flag declarations in the .ad file to be consistent).

No, this is just a leftover from an earlier design, I'll remove the check and always add the prefix

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/14172#discussion_r1285750656
PR Review Comment: https://git.openjdk.org/jdk/pull/14172#discussion_r1285749667


More information about the hotspot-compiler-dev mailing list