https://codereview.chromium.org/1131783003/diff/1/src/arm/full-codegen-arm.cc
File src/arm/full-codegen-arm.cc (right):
https://codereview.chromium.org/1131783003/diff/1/src/arm/full-codegen-arm.cc#newcode362
src/arm/full-codegen-arm.cc:362: masm()->EmitConstantPool();
On 2015/05/22 12:21:21, rmcilroy wrote:
On 2015/05/20 22:28:21, mtbrandyberry wrote:
> On 2015/05/20 14:32:10, rmcilroy wrote:
> > Should we be emitting this here if we are also emitting it in
> > Assembler::GetCode?
>
> Currently, the embedded constant pool is emitted immediately
following the
> instructions but prior to any other tables (e.g. safepoint,
back-edge). The
> intent was to keep the constant pool related logic together and have
the
> constant pool area immediately follow code in the disassembly. I'll
concede
> that there is no strict reason for this ordering. (Note, however,
that the
> constant pool builder is smart enough to emit the table only on the
first
> invocation).
>
> Let me know if this is a significant issue and I will modify it to
emit at the
> end.
I think this would be better right at the end just before the
relocinfo (which
is what describes the constant pool anyway). I don't think this should
make the
disassembly any more complex, but if it does let me know and I'll
re-evaluate.
Acknowledged.
https://codereview.chromium.org/1131783003/diff/1/src/assembler.h
File src/assembler.h (right):
https://codereview.chromium.org/1131783003/diff/1/src/assembler.h#newcode1174
src/assembler.h:1174: return ((kPointerSize == kDoubleSize || type ==
INTPTR) ? kPointerSize
On 2015/05/22 12:21:22, rmcilroy wrote:
On 2015/05/20 22:28:22, mtbrandyberry wrote:
> On 2015/05/20 14:32:11, rmcilroy wrote:
> > I think all you need here is:
> > return (type == INTPTR) ? kPointerSize : kDoubleSize
>
> My thinking was that the compiler could eliminate the condition all
together
on
> 64-bit architectures.
If the compiler is smart enough to do that, then it is also probably
smart
enough to do constant propigation and work out kPointerSize and
kDoubleSize are
the same and also eliminate the condition. In anycase, I don't think
this
microoptimisation is worth the reduced readability unless you have
specific
numbers to back up it being worthwhile.
Acknowledged.
https://codereview.chromium.org/1131783003/diff/1/src/frames.h
File src/frames.h (right):
https://codereview.chromium.org/1131783003/diff/1/src/frames.h#newcode128
src/frames.h:128: static const int kConstantPoolOffset = kCPSlotSize ?
-1 * kPointerSize : 0;
On 2015/05/22 12:21:22, rmcilroy wrote:
On 2015/05/20 22:28:22, mtbrandyberry wrote:
> On 2015/05/20 14:32:11, rmcilroy wrote:
> > nit - align '='
>
> git cl format did this. Should I override?
Ahh right, in that case could you touch all the constants in this
block so git
cl format lines makes them all two-space instead of lined up.
Acknowledged.
https://codereview.chromium.org/1131783003/
--
--
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.
