Request for reviews (M): 7069452: Cleanup NodeFlags (original) (raw)

Vladimir Kozlov vladimir.kozlov at oracle.com
Tue Jul 26 17:05:55 PDT 2011

Tom Rodriguez wrote:

So isGoto returns false for GotoNode? That doesn't seem right.

It is used only for Mach nodes in block.cpp.

+bool Node::isGoto() const { return isBranch() && isMach() && (asMach()->idealOpcode() == OpGoto); } why does this have an isBranch test? Isn't testing the idealOpcode sufficient for the mach case?

is_Branch() is bits test and ideal_Opcode() is virtual call. So it is a little speedup. I can set is_Branch flag in ideal GotoNode and change is_Goto() to:

bool Node::is_Goto() const { if (!is_Branch()) return false; if (is_Mach()) return (as_Mach()->ideal_Opcode() == Op_Goto); else return (Opcode() == Op_Goto); }

I will look on Flag_is_block_start and Flag_is_safepoint_node.

Thanks, Vladimir

Flagisblockstart seems to be minimally useful since it's only used to mark StartNode, at least as far as I can see. Flagissafepointnode could probably be eliminated without too much work too. It's set for MachSafePointNode and cleared for MachCallLeaf. The adlc tries to clear it for FastUnlock but those inherit from MachNode, not MachSafePointNode so it seems slightly useless. tom On Jul 22, 2011, at 11:06 AM, Vladimir Kozlov wrote:

http://cr.openjdk.java.net/~kvn/7069452/webrev

Fixed 7069452: Cleanup NodeFlags We are almost out of bits (16) for Node::NodeFlags. I removed flags which duplicate information in Node::NodeClasses. isCall() uses classid check and is now valid only for ideal CallNode. isGoto() checks idealopcode(). isVector() is replaced with check for Vector,VectorLoad,VectorStore classes. MachProjNode was added to classid check to avoid calling Opcode() in many places. MulNode was removed from classid since it was used only in one place. I removed all logic associated with ispcrelative flag. It was only checked in one place during long to short branch replacement. I replaced it with check in adlc parser - short branches should be defined only for a branch to a label, which means branch with PC relative offset. To relay on developer to set inspcrelative() was mistake since it could be used in wrong places. For example, it was specified for table jumps, calls and FastLock/FastUnlock.

More information about the hotspot-compiler-dev mailing list