On 2015/06/02 16:55:31, mtbrandyberry wrote:
https://codereview.chromium.org/1131783003/diff/40001/src/ppc/lithium-codegen-ppc.cc
File src/ppc/lithium-codegen-ppc.cc (right):
https://codereview.chromium.org/1131783003/diff/40001/src/ppc/lithium-codegen-ppc.cc#newcode57
src/ppc/lithium-codegen-ppc.cc:57: // Avoid DCHECK(!is_linked()) failure
in
~Label()
On 2015/06/02 16:40:12, rmcilroy wrote:
> On 2015/06/02 15:36:10, mtbrandyberry wrote:
> > On 2015/06/02 13:57:21, rmcilroy wrote:
> > > On 2015/06/01 21:01:30, mtbrandyberry wrote:
> > > > On 2015/06/01 09:52:09, rmcilroy wrote:
> > > > > Won't this cause issues in non-debug code if the label is
getting
> deleted
> > > > before
> > > > > the constant pool is emitted?
> > > >
> > > > Not quite. The label gets deleted upon destruction of the
assembler
> object.
> >
> > > > The issue here is that the embedded constant pool label may be
linked
to
> in
> > > the
> > > > prologue, but never bound since this path aborts the compilation
prior
to
> > > > reaching GetCode (where the constant pool would have been
emitted).
> > > >
> > > > If there is a better way to work around this, let me know. It
seems
like
> > the
> > > > labels used for deferred code generation could also run into this.
> Perhaps
> > > the
> > > > tests just don't exercise that scenario?
> > >
> > > The more I think about it the less keen I am to be storing this as a
label
> in
> > > the constant pool builder object. Could you just do the same as on
Arm
> instead
> > > (getting the offset from the code object and adding that to the code
target
> > > address)?
> >
> > Loading the offset from the code object is problematic on PPC due to
the
lack
> of
> > pc-relative addressing.
>
> OK.
>
> > I disagree that this scenario or the builder's use of a label is
inherently
> bad.
> > As far as I can tell, it's no different than any other use of a label
prior
> to
> > binding it. If anything, the problem is that the Label destructor's
DCHECK
is
> > invalid for cases such as this where the compilation was aborted and
the
code
> > will be discarded.
>
> Ok. How about adding a masm()->AbortConstantPoolBuilding() (just for
PPC)
which
> explicitly binds the label to make it obvious why this is happening (and
avoid
> emitting the constant pool when assembly failed, which seems like it
might
go
> wrong).
sgtm
I think we're ready to commit with your OK.
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.
