As far as the general approach, I feel very strongly that introducing a
"high-level" incrementalstats instruction is not the right way to go. We are
spending a lot of time and effort trying to lower the hydrogen IR and get
rid
of a lot of the ad-hoc instructions that were introduced in the original
Crankshaft implementation, because in general it is very difficult to apply
optimizations and analysis algorithms over such a large set of specialized
instructions.
This patch looks like it's going in the right direction, I'm not sure why
you
are concerned about efficiency and maintainability. You should never be
counting
stats in performance critical code, and as you see from Michael comments,
there
are plenty of other of applications of ConstantE. It think it is a _really_
good
sign that it's roughly the same amount of code as before, but now there are
actually parts of the code that can be reused. For example, the ConstantE
mechanism can be used for directly loading values from the Isolate (I will
need
this soon for getting the hash seed from the isolate in generated code).
As for the details, I'll take a look today and give you feedback. I am not
so
excited about introducing HLoadExternalReference. It seems you want much
(all?)
of the machinery that we already have in place for HConstants in general,
such
as GVN and special live range handling/emit at uses by the register
allocator.
It seems like the wrong thing to create a new instruction that requires the
same
special handling as HConstant throughout the compiler.
I will take a more detailed look at the code today and provide further
comments.
+tizter for a second opinion.
https://codereview.chromium.org/19562003/diff/4001/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):
https://codereview.chromium.org/19562003/diff/4001/src/hydrogen-instructions.h#newcode3289
src/hydrogen-instructions.h:3289: Representation r =
Representation::None());
Why None? It seems to make much more sense to use "External", that's
exactly what it's for.
https://codereview.chromium.org/19562003/
--
--
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.
