https://codereview.chromium.org/803993002/diff/1/src/compiler/loop-analysis.cc
File src/compiler/loop-analysis.cc (right):

https://codereview.chromium.org/803993002/diff/1/src/compiler/loop-analysis.cc#newcode255
src/compiler/loop-analysis.cc:255: int count = 0;
On 2014/12/16 06:10:25, Benedikt Meurer wrote:
s/int/size_t/

Done.

https://codereview.chromium.org/803993002/diff/1/src/compiler/loop-analysis.cc#newcode298
src/compiler/loop-analysis.cc:298: int count = 0;
On 2014/12/16 06:10:25, Benedikt Meurer wrote:
s/int/size_t/

Done.

https://codereview.chromium.org/803993002/diff/1/src/compiler/loop-analysis.cc#newcode328
src/compiler/loop-analysis.cc:328: static_cast<uint8_t>(loop_num);
On 2014/12/16 06:10:25, Benedikt Meurer wrote:
Please add a TODO here to ensure that this static_cast is not
overlooked when
the restriction on loop_num is gone.

Done.

https://codereview.chromium.org/803993002/diff/1/src/compiler/loop-analysis.cc#newcode336
src/compiler/loop-analysis.cc:336: static_cast<uint8_t>(loop_num);
On 2014/12/16 06:10:25, Benedikt Meurer wrote:
Please add a TODO here to ensure that this static_cast is not
overlooked when
the restriction on loop_num is gone.

Done.

https://codereview.chromium.org/803993002/diff/1/src/compiler/loop-analysis.h
File src/compiler/loop-analysis.h (right):

https://codereview.chromium.org/803993002/diff/1/src/compiler/loop-analysis.h#newcode32
src/compiler/loop-analysis.h:32: Loop* parent() { return parent_; }
On 2014/12/16 06:10:25, Benedikt Meurer wrote:
Nit: const.

Done.

https://codereview.chromium.org/803993002/diff/1/src/compiler/loop-analysis.h#newcode34
src/compiler/loop-analysis.h:34: size_t HeaderSize() { return
body_start_ - header_start_; }
On 2014/12/16 06:10:25, Benedikt Meurer wrote:
Nit: const.

Done.

https://codereview.chromium.org/803993002/diff/1/src/compiler/loop-analysis.h#newcode35
src/compiler/loop-analysis.h:35: size_t BodySize() { return body_end_ -
body_start_; }
On 2014/12/16 06:10:25, Benedikt Meurer wrote:
Nit: const.

Done.

https://codereview.chromium.org/803993002/diff/1/src/compiler/loop-analysis.h#newcode36
src/compiler/loop-analysis.h:36: size_t TotalSize() { return body_end_ -
header_start_; }
On 2014/12/16 06:10:25, Benedikt Meurer wrote:
Nit: const.

Done.

https://codereview.chromium.org/803993002/diff/1/src/compiler/loop-analysis.h#newcode43
src/compiler/loop-analysis.h:43: : parent_(NULL),
On 2014/12/16 06:10:25, Benedikt Meurer wrote:
Please use type-safe nullptr instead of NULL for newly written code,
in line
with the Chromium/Google style guides.

Done.

https://codereview.chromium.org/803993002/diff/1/src/compiler/loop-analysis.h#newcode58
src/compiler/loop-analysis.h:58: Loop* ContainingLoop(Node* node) {
On 2014/12/16 06:10:25, Benedikt Meurer wrote:
Nit: const.

Const starts to spread like a virus from this site, so I'll skip this
one if you don't mind.

https://codereview.chromium.org/803993002/diff/1/src/compiler/loop-analysis.h#newcode66
src/compiler/loop-analysis.h:66: bool Contains(Loop* loop, Node* node) {
On 2014/12/16 06:10:25, Benedikt Meurer wrote:
Nit: const.

Same as above.

https://codereview.chromium.org/803993002/diff/1/src/compiler/loop-analysis.h#newcode77
src/compiler/loop-analysis.h:77: int LoopNum(Loop* loop) {
On 2014/12/16 06:10:25, Benedikt Meurer wrote:
Nit: const.

Done.

https://codereview.chromium.org/803993002/diff/1/src/compiler/loop-analysis.h#newcode77
src/compiler/loop-analysis.h:77: int LoopNum(Loop* loop) {
On 2014/12/16 06:10:25, Benedikt Meurer wrote:
Nit: const.

Done.

https://codereview.chromium.org/803993002/diff/1/src/zone-containers.h
File src/zone-containers.h (right):

https://codereview.chromium.org/803993002/diff/1/src/zone-containers.h#newcode92
src/zone-containers.h:92: struct PtrRange {
On 2014/12/16 06:10:25, Benedikt Meurer wrote:
Ok, I see the reason to have this class in here, but it should be (a)
a real STL
compatible class and (b) not in zone-containers.h as it has nothing to
with
Zone. I.e.

template <typename Iterator>
class iterator_range {
  public:
   typedef Iterator iterator;
   typedef Iterator const_iterator;
   typedef std::iterator_traits<Iterator>::difference_type
difference_type;

   template <typename Iterator2>
   iterator_range(Iterator2 begin, Iterator2 end) : begin_(begin),
end_(end) {}

   const_iterator begin() const { return begin_; }
   const_iterator end() const { return end_; }

  private:
   const_iterator begin_ const;
   const_iterator end_ const;
};

Not sure where to put it tho. Maybe introduce some stl-utils or
stl-support
header.

Using base::iterator_range now.

https://codereview.chromium.org/803993002/diff/1/test/cctest/compiler/test-loop-analysis.cc
File test/cctest/compiler/test-loop-analysis.cc (right):

https://codereview.chromium.org/803993002/diff/1/test/cctest/compiler/test-loop-analysis.cc#newcode156
test/cctest/compiler/test-loop-analysis.cc:156: void
CheckRangeContains(PtrRange<Node*> range, Node* node) {
On 2014/12/16 06:10:25, Benedikt Meurer wrote:
As Sven mentioned, this is just std::find().

Done.

https://codereview.chromium.org/803993002/

--
--
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.

Reply via email to