This generally looks like it's going in the right direction, although I
haven't
expected everything in detail yet.
However, one thing that doesn't feel right is the removal of the SSI nodes
after
optimization is complete. I think a better approach is to add a method to
HValue
called something like GetBaseValue() which returns "this" for HValue and
for SSI
values follows the chain up to what you need. All subsequent passes that
need
the "base" value (like the register allocator) should use the GetBaseValue()
method and the graph doesn't need to be re-written to remove SSI.
Also, I don't know if I'm quite happy with the name SsiDefinition. How about
something that is more consistent with our other value name, e.g.
HSSIValue. A
name like this would also makes it clearer what its role is as opposed to a
"plain vanilla" HValue.
https://chromiumcodereview.appspot.com/11678007/diff/1/src/hydrogen-instructions.cc
File src/hydrogen-instructions.cc (right):
https://chromiumcodereview.appspot.com/11678007/diff/1/src/hydrogen-instructions.cc#newcode767
src/hydrogen-instructions.cc:767: length(), current,
HSsiDefinition::IF_TAGGED_IS_SMI, length());
I don't think we want to handle "SMI-ness" here with SSI here yet. We
currently track SMI-ness using the HType of a HValue, and I don't think
it makes sense to add a new mechanism to do so. At the very least, SSI
nodes that have SMIs values should return HType::Smi().
https://chromiumcodereview.appspot.com/11678007/diff/1/src/hydrogen-instructions.cc#newcode2269
src/hydrogen-instructions.cc:2269: ValueInfoRelation relation,
I think I would pre just a bit more abstraction here. I think what you
want is a SSIInformation type that abstracts the type of information you
can annotate a value with. You already have two different ones, so I
think it makes sense, i.e. one for comparison relation and one for a
type change (i.e. ensuring the "SMIness" of a value).
https://chromiumcodereview.appspot.com/11678007/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
