On 2014/08/13 09:26:41, Benedikt Meurer wrote:
On 2014/08/13 at 09:19:45, titzer wrote:
> On 2014/08/13 09:14:13, Benedikt Meurer wrote:
> >
https://codereview.chromium.org/460593003/diff/20001/src/compiler/change-lowering.cc
> > File src/compiler/change-lowering.cc (right):
> >
> >
https://codereview.chromium.org/460593003/diff/20001/src/compiler/change-lowering.cc#newcode138
> > src/compiler/change-lowering.cc:138: Reduction
> > ChangeLowering<4>::ChangeBoolToBit(Node* val) {
> > That's another option, sure. But I decided to do it this way.
>
> This is not necessary complexity. You undid a simple branch and
replaced it
with template specialization on the pointer size. This an abuse of
templates
with clear disadvantages and little or no benefit.
>
> Can we please go back to the simpler code?
It was "a simple branch" because it was using the global state implicitly
(i.e.
WordEqual and friends). Of course it's simple if you solve only a subset
of
the
actual problem. Doing it right will require the "simple branch" in many
places,
which is probably less readable than just having specialized templates.
Note that this class is not finished yet, and because we can now actually
test
it, it'll be easy to refactor and remove the duplication.
As I mentioned, a simple field on the class would suffice to get rid of the
dependency on the kPointerSize. A template is still IMHO overkill.
https://codereview.chromium.org/460593003/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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 v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
