lgtm, thanks!
https://codereview.chromium.org/1217673003/diff/20001/src/arm/assembler-arm.cc
File src/arm/assembler-arm.cc (right):
https://codereview.chromium.org/1217673003/diff/20001/src/arm/assembler-arm.cc#newcode3801
src/arm/assembler-arm.cc:3801: entry.set_merged_index(j);
On 2015/07/03 09:07:33, Yang wrote:
On 2015/07/03 08:58:04, rmcilroy wrote:
> I think this the wrong way round, you should be setting
> ...constants[j].set_merged_index[i]. Otherwise the delta for entry
will get
> larger (since the j is larger than i) which means it might be out of
ldr
range.
> I'm not sure if this is actually a problem though (I can't rememeber
how the
> assembler checks when it should emit the pool). Also if there was
another
> duplicate later then wouldnt entry j will get merged with that other
dup while
i
> is still duped with j?
I haven't changed the merge loop at all. j is never larger than i as
indicated
by the for-loop condition. For every entry i we look at prior entries
(0 .. i-1)
for duplicates. If we found an entry j, we will use the found prior
entry, by
setting the merge_index of i to j.
You're right, oops. I didn't look at the for loop header properly and
assumed that you were doing j=i, j++ :). This looks fine then, thanks.
https://codereview.chromium.org/1217673003/
--
--
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/d/optout.
