https://codereview.chromium.org/17568015/diff/21001/src/hydrogen-bounds-check-removal.cc
File src/hydrogen-bounds-check-removal.cc (right):
https://codereview.chromium.org/17568015/diff/21001/src/hydrogen-bounds-check-removal.cc#newcode101
src/hydrogen-bounds-check-removal.cc:101: */
On 2013/07/10 16:34:18, titzer wrote:
It is still not clear to me what order the code visits the blocks, and
how
exactly that is accomplished. Will need in-person discussion for this.
Done.
https://codereview.chromium.org/17568015/diff/21001/src/hydrogen-bounds-check-removal.cc#newcode180
src/hydrogen-bounds-check-removal.cc:180: &failure, unsafe);
On 2013/07/10 16:34:18, titzer wrote:
It might actually be clearer to inline this PerformStep method and
then refactor
its pieces into different configurations.
Done.
https://codereview.chromium.org/17568015/diff/21001/src/hydrogen-bounds-check-removal.cc#newcode293
src/hydrogen-bounds-check-removal.cc:293: void
CollectInductionVariableData(HBasicBlock* bb);
On 2013/07/10 16:34:18, titzer wrote:
It's OK IMO to go ahead and pull up these other methods into the class
definition.
Done.
https://codereview.chromium.org/17568015/diff/21001/src/hydrogen-instructions.cc
File src/hydrogen-instructions.cc (right):
https://codereview.chromium.org/17568015/diff/21001/src/hydrogen-instructions.cc#newcode1986
src/hydrogen-instructions.cc:1986: result->base = value;
On 2013/07/10 16:34:18, titzer wrote:
Thanks for the comment; now that I understand the patterns you would
like to
match, I don't think you even need a recursive method. You can first
just try to
match the outer |/& operation, and if it succeeds, then try to match
the inner
+/- on the non-constant side, and if it fails, try to match the +/- on
the whole
thing. It would be far more clear than a loop that does not loop.
Done.
https://codereview.chromium.org/17568015/diff/21001/src/hydrogen-instructions.cc#newcode2241
src/hydrogen-instructions.cc:2241: bool
InductionVariableData::TokenCanRepresentLoopGuard(Token::Value token) {
On 2013/07/10 16:34:18, titzer wrote:
With the name change I see now more clearly what you meant. I actually
think you
want to compute whether a given {InductionVariable, Token, limit} is
false
leading out of a loop, since that is what is interesting below?
In other words, I am not sure if breaking this operation out into its
own method
is worthwhile.
Inlined it into CheckIfBranchIsLoopGuard.
https://codereview.chromium.org/17568015/diff/21001/src/hydrogen-instructions.cc#newcode2256
src/hydrogen-instructions.cc:2256: HBasicBlock* out_branch) {
On 2013/07/10 16:34:18, titzer wrote:
This is kind of a strange name for this method. I think it should be
inlined
(with a comment) into the second of the two methods which I suggest
making the
below method into.
After the off line discussion I ended up not inlining it (but it now
contains the code of CheckIfBranchIsLoopGuard).
https://codereview.chromium.org/17568015/diff/21001/src/hydrogen-instructions.cc#newcode2269
src/hydrogen-instructions.cc:2269: HInstruction* end =
predecessor->last();
On 2013/07/10 16:34:18, titzer wrote:
This method could benefit from being split up into one that matches
the
branching pattern from which we derive information (namely the given
block is
entered from a predecessor that has a branch on an induction variable)
and a
method that consumes that branching pattern and updates either the
induction
variable data or the additional limit.
You could have a little datastructure that represents basically the
triple
{InductionVariableData induction_variable, Token token, Instruction
limit).
Done.
https://codereview.chromium.org/17568015/diff/21001/src/hydrogen-instructions.cc#newcode2288
src/hydrogen-instructions.cc:2288: if (is_else_branch) {
On 2013/07/10 16:34:18, titzer wrote:
Please just negate the token above, and don't use an intermediate
is_else_branch
boolean variable.
Done.
https://codereview.chromium.org/17568015/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
