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