Subzero: Clean up some uses of *_cast<>.

Subzero: Clean up some uses of *_cast<>.

Some casts to size_t for use as array indexes are simply unnecessary.

Some explicit declaration types are changed to "auto" to avoid redundancy with the static_cast type.

A few llvm::dyn_cast<> operations are changed to llvm::cast<>, and vice versa.

A few explicit declaration types are changed to "auto" when used with llvm::cast<> and llvm::dynamic_cast<>.  Some of these were missed during an earlier cleansing because of multi-line issues.

There are still a few opportunities related to Variable register numbers, but they are ignored for now because they are being addressed in another CL.

BUG= none
R=jpp@chromium.org

Committed: https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-subzero.git;a=commit;h=2d6c82679e9786a924b76023aa30e1f44d0f8cbb

[Jim Stichnoth, 2016-02-07 15:28:19]

Description was changed from

==========
Subzero: Clean up some uses of static_cast<>.

BUG= none
==========

to

==========
Subzero: Clean up some uses of static_cast<>.

Some casts to size_t for use as array indexes are simply unnecessary.

Some explicit declaration types are changed to "auto" to avoid redundancy with
the static_cast type.

BUG= none
==========

[Jim Stichnoth, 2016-02-07 15:49:39]

Description was changed from

==========
Subzero: Clean up some uses of static_cast<>.

Some casts to size_t for use as array indexes are simply unnecessary.

Some explicit declaration types are changed to "auto" to avoid redundancy with
the static_cast type.

BUG= none
==========

to

==========
Subzero: Clean up some uses of *_cast<>.

Some casts to size_t for use as array indexes are simply unnecessary.

Some explicit declaration types are changed to "auto" to avoid redundancy with
the static_cast type.

A few llvm::dyn_cast<> operations are changed to llvm::cast<>.

A few explicit declaration types are changed to "auto" when used with
llvm::cast<> and llvm::dynamic_cast<>.  These were missed during an earlier
cleansing because of multi-line issues.

BUG= none
==========

[Jim Stichnoth, 2016-02-07 15:51:08]

Description was changed from

==========
Subzero: Clean up some uses of *_cast<>.

Some casts to size_t for use as array indexes are simply unnecessary.

Some explicit declaration types are changed to "auto" to avoid redundancy with
the static_cast type.

A few llvm::dyn_cast<> operations are changed to llvm::cast<>.

A few explicit declaration types are changed to "auto" when used with
llvm::cast<> and llvm::dynamic_cast<>.  These were missed during an earlier
cleansing because of multi-line issues.

BUG= none
==========

to

==========
Subzero: Clean up some uses of *_cast<>.

Some casts to size_t for use as array indexes are simply unnecessary.

Some explicit declaration types are changed to "auto" to avoid redundancy with
the static_cast type.

A few llvm::dyn_cast<> operations are changed to llvm::cast<>.

A few explicit declaration types are changed to "auto" when used with
llvm::cast<> and llvm::dynamic_cast<>.  Some of these were missed during an
earlier cleansing because of multi-line issues.

BUG= none
==========

[Jim Stichnoth, 2016-02-07 15:53:45]

Description was changed from

==========
Subzero: Clean up some uses of *_cast<>.

Some casts to size_t for use as array indexes are simply unnecessary.

Some explicit declaration types are changed to "auto" to avoid redundancy with
the static_cast type.

A few llvm::dyn_cast<> operations are changed to llvm::cast<>.

A few explicit declaration types are changed to "auto" when used with
llvm::cast<> and llvm::dynamic_cast<>.  Some of these were missed during an
earlier cleansing because of multi-line issues.

BUG= none
==========

to

==========
Subzero: Clean up some uses of *_cast<>.

Some casts to size_t for use as array indexes are simply unnecessary.

Some explicit declaration types are changed to "auto" to avoid redundancy with
the static_cast type.

A few llvm::dyn_cast<> operations are changed to llvm::cast<>.

A few explicit declaration types are changed to "auto" when used with
llvm::cast<> and llvm::dynamic_cast<>.  Some of these were missed during an
earlier cleansing because of multi-line issues.

There are still a few opportunities related to Variable register numbers, but
they are ignored for now because they are being addressed in another CL.

BUG= none
==========

[Jim Stichnoth, 2016-02-07 15:55:59]

stichnot@chromium.org changed reviewers:
+ eholk@chromium.org, jpp@chromium.org, kschimpf@google.com, sehr@chromium.org

[Jim Stichnoth, 2016-02-07 15:56:24]

Description was changed from

==========
Subzero: Clean up some uses of *_cast<>.

Some casts to size_t for use as array indexes are simply unnecessary.

Some explicit declaration types are changed to "auto" to avoid redundancy with
the static_cast type.

A few llvm::dyn_cast<> operations are changed to llvm::cast<>.

A few explicit declaration types are changed to "auto" when used with
llvm::cast<> and llvm::dynamic_cast<>.  Some of these were missed during an
earlier cleansing because of multi-line issues.

There are still a few opportunities related to Variable register numbers, but
they are ignored for now because they are being addressed in another CL.

BUG= none
==========

to

==========
Subzero: Clean up some uses of *_cast<>.

Some casts to size_t for use as array indexes are simply unnecessary.

Some explicit declaration types are changed to "auto" to avoid redundancy with
the static_cast type.

A few llvm::dyn_cast<> operations are changed to llvm::cast<>, and vice versa.

A few explicit declaration types are changed to "auto" when used with
llvm::cast<> and llvm::dynamic_cast<>.  Some of these were missed during an
earlier cleansing because of multi-line issues.

There are still a few opportunities related to Variable register numbers, but
they are ignored for now because they are being addressed in another CL.

BUG= none
==========

[John, 2016-02-07 16:18:33]

lgtm

https://codereview.chromium.org/1674033002/diff/20001/src/IceAssemblerARM32.cpp
File src/IceAssemblerARM32.cpp (right):

https://codereview.chromium.org/1674033002/diff/20001/src/IceAssemblerARM32.c...
src/IceAssemblerARM32.cpp:709: auto Offset = static_cast<IOffsetT>((Inst &
kBranchOffsetMask) << 8);
I'd prefer

IOffsetT Offset = ...;

https://codereview.chromium.org/1674033002/diff/20001/src/IceInstARM32.h
File src/IceInstARM32.h (right):

https://codereview.chromium.org/1674033002/diff/20001/src/IceInstARM32.h#newc...
src/IceInstARM32.h:352: static_cast<OperandKind>(kVariable_Target);
While at this, constexpr.

https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringMI...
File src/IceTargetLoweringMIPS32.cpp (right):

https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringMI...
src/IceTargetLoweringMIPS32.cpp:1248: auto Value =
static_cast<uint32_t>(C32->getValue());
uint32_t Value = ...;

While at this, const?

https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringX8...
File src/IceTargetLoweringX8632Traits.h (right):

https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringX8...
src/IceTargetLoweringX8632Traits.h:959: const static auto SpillVariableKind =
While at this, constexpr?

https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringX8...
File src/IceTargetLoweringX8664Traits.h (right):

https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringX8...
src/IceTargetLoweringX8664Traits.h:1004: const static auto SpillVariableKind =
While at this, constexpr?

https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringX8...
File src/IceTargetLoweringX86BaseImpl.h (right):

https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringX8...
src/IceTargetLoweringX86BaseImpl.h:2232: auto UDivisor =
static_cast<uint32_t>(Divisor);
uint32_t UDivisor = ...;

Can this be const?

https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringX8...
src/IceTargetLoweringX86BaseImpl.h:2321: int32_t Divisor = C->getValue();
Ditto.

https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringX8...
src/IceTargetLoweringX86BaseImpl.h:3223: bool HasC1 =
(Traits::TableFcmp[Condition].C1 != Traits::Cond::Br_None);
While at this, const?

https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringX8...
src/IceTargetLoweringX86BaseImpl.h:3224: bool HasC2 =
(Traits::TableFcmp[Condition].C2 != Traits::Cond::Br_None);
Ditto

https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringX8...
src/IceTargetLoweringX86BaseImpl.h:3322: assert(Predicate !=
Traits::Cond::Cmpps_Invalid);
While at this, const?

https://codereview.chromium.org/1674033002/diff/20001/src/PNaClTranslator.cpp
File src/PNaClTranslator.cpp (right):

https://codereview.chromium.org/1674033002/diff/20001/src/PNaClTranslator.cpp...
src/PNaClTranslator.cpp:2740: auto IsTailCall = static_cast<bool>(CCInfo & 1);
bool IsTailCall = ...;

While at this, const?

[John, 2016-02-07 16:19:33]

On 2016/02/07 16:18:33, John wrote:
> lgtm
> 
>
https://codereview.chromium.org/1674033002/diff/20001/src/IceAssemblerARM32.cpp
> File src/IceAssemblerARM32.cpp (right):
> 
>
https://codereview.chromium.org/1674033002/diff/20001/src/IceAssemblerARM32.c...
> src/IceAssemblerARM32.cpp:709: auto Offset = static_cast<IOffsetT>((Inst &
> kBranchOffsetMask) << 8);
> I'd prefer
> 
> IOffsetT Offset = ...;
> 
> https://codereview.chromium.org/1674033002/diff/20001/src/IceInstARM32.h
> File src/IceInstARM32.h (right):
> 
>
https://codereview.chromium.org/1674033002/diff/20001/src/IceInstARM32.h#newc...
> src/IceInstARM32.h:352: static_cast<OperandKind>(kVariable_Target);
> While at this, constexpr.
> 
>
https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringMI...
> File src/IceTargetLoweringMIPS32.cpp (right):
> 
>
https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringMI...
> src/IceTargetLoweringMIPS32.cpp:1248: auto Value =
> static_cast<uint32_t>(C32->getValue());
> uint32_t Value = ...;
> 
> While at this, const?
> 
>
https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringX8...
> File src/IceTargetLoweringX8632Traits.h (right):
> 
>
https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringX8...
> src/IceTargetLoweringX8632Traits.h:959: const static auto SpillVariableKind =
> While at this, constexpr?
> 
>
https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringX8...
> File src/IceTargetLoweringX8664Traits.h (right):
> 
>
https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringX8...
> src/IceTargetLoweringX8664Traits.h:1004: const static auto SpillVariableKind =
> While at this, constexpr?
> 
>
https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringX8...
> File src/IceTargetLoweringX86BaseImpl.h (right):
> 
>
https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringX8...
> src/IceTargetLoweringX86BaseImpl.h:2232: auto UDivisor =
> static_cast<uint32_t>(Divisor);
> uint32_t UDivisor = ...;
> 
> Can this be const?
> 
>
https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringX8...
> src/IceTargetLoweringX86BaseImpl.h:2321: int32_t Divisor = C->getValue();
> Ditto.
> 
>
https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringX8...
> src/IceTargetLoweringX86BaseImpl.h:3223: bool HasC1 =
> (Traits::TableFcmp[Condition].C1 != Traits::Cond::Br_None);
> While at this, const?
> 
>
https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringX8...
> src/IceTargetLoweringX86BaseImpl.h:3224: bool HasC2 =
> (Traits::TableFcmp[Condition].C2 != Traits::Cond::Br_None);
> Ditto
> 
>
https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringX8...
> src/IceTargetLoweringX86BaseImpl.h:3322: assert(Predicate !=
> Traits::Cond::Cmpps_Invalid);
> While at this, const?
> 
> https://codereview.chromium.org/1674033002/diff/20001/src/PNaClTranslator.cpp
> File src/PNaClTranslator.cpp (right):
> 
>
https://codereview.chromium.org/1674033002/diff/20001/src/PNaClTranslator.cpp...
> src/PNaClTranslator.cpp:2740: auto IsTailCall = static_cast<bool>(CCInfo & 1);
> bool IsTailCall = ...;
> 
> While at this, const?

I forgot to say, but everything is optional

[Jim Stichnoth, 2016-02-07 16:42:42]

https://codereview.chromium.org/1674033002/diff/20001/src/IceAssemblerARM32.cpp
File src/IceAssemblerARM32.cpp (right):

https://codereview.chromium.org/1674033002/diff/20001/src/IceAssemblerARM32.c...
src/IceAssemblerARM32.cpp:709: auto Offset = static_cast<IOffsetT>((Inst &
kBranchOffsetMask) << 8);
On 2016/02/07 16:18:32, John wrote:
> I'd prefer
> 
> IOffsetT Offset = ...;

You mean the original way?  I'm curious why, since this seems to be a poster
child for use of "auto".

https://codereview.chromium.org/1674033002/diff/20001/src/IceInstARM32.h
File src/IceInstARM32.h (right):

https://codereview.chromium.org/1674033002/diff/20001/src/IceInstARM32.h#newc...
src/IceInstARM32.h:352: static_cast<OperandKind>(kVariable_Target);
On 2016/02/07 16:18:32, John wrote:
> While at this, constexpr.

Done.

https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringX8...
File src/IceTargetLoweringX8632Traits.h (right):

https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringX8...
src/IceTargetLoweringX8632Traits.h:959: const static auto SpillVariableKind =
On 2016/02/07 16:18:33, John wrote:
> While at this, constexpr?

Done.

https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringX8...
File src/IceTargetLoweringX8664Traits.h (right):

https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringX8...
src/IceTargetLoweringX8664Traits.h:1004: const static auto SpillVariableKind =
On 2016/02/07 16:18:33, John wrote:
> While at this, constexpr?

Done.

https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringX8...
File src/IceTargetLoweringX86BaseImpl.h (right):

https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringX8...
src/IceTargetLoweringX86BaseImpl.h:3223: bool HasC1 =
(Traits::TableFcmp[Condition].C1 != Traits::Cond::Br_None);
On 2016/02/07 16:18:33, John wrote:
> While at this, const?

Done.

https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringX8...
src/IceTargetLoweringX86BaseImpl.h:3224: bool HasC2 =
(Traits::TableFcmp[Condition].C2 != Traits::Cond::Br_None);
On 2016/02/07 16:18:33, John wrote:
> Ditto

Done.

https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringX8...
src/IceTargetLoweringX86BaseImpl.h:3322: assert(Predicate !=
Traits::Cond::Cmpps_Invalid);
On 2016/02/07 16:18:33, John wrote:
> While at this, const?

Done.

[John, 2016-02-07 16:59:43]

lgtm

Still LGTM

https://codereview.chromium.org/1674033002/diff/20001/src/IceAssemblerARM32.cpp
File src/IceAssemblerARM32.cpp (right):

https://codereview.chromium.org/1674033002/diff/20001/src/IceAssemblerARM32.c...
src/IceAssemblerARM32.cpp:709: auto Offset = static_cast<IOffsetT>((Inst &
kBranchOffsetMask) << 8);
On 2016/02/07 16:42:42, stichnot wrote:
> On 2016/02/07 16:18:32, John wrote:
> > I'd prefer
> > 
> > IOffsetT Offset = ...;
> 
> You mean the original way?  I'm curious why, since this seems to be a poster
> child for use of "auto".

I meant

const IOffsetT Offset = (Inst & kBranchOffsetMask) << 8;

In other words, I'd let the declaration cast the rhs. Not sure if this works,
though.

[Jim Stichnoth, 2016-02-07 17:44:41]

https://codereview.chromium.org/1674033002/diff/20001/src/IceAssemblerARM32.cpp
File src/IceAssemblerARM32.cpp (right):

https://codereview.chromium.org/1674033002/diff/20001/src/IceAssemblerARM32.c...
src/IceAssemblerARM32.cpp:709: auto Offset = static_cast<IOffsetT>((Inst &
kBranchOffsetMask) << 8);
On 2016/02/07 16:59:43, John wrote:
> On 2016/02/07 16:42:42, stichnot wrote:
> > On 2016/02/07 16:18:32, John wrote:
> > > I'd prefer
> > > 
> > > IOffsetT Offset = ...;
> > 
> > You mean the original way?  I'm curious why, since this seems to be a poster
> > child for use of "auto".
> 
> I meant
> 
> const IOffsetT Offset = (Inst & kBranchOffsetMask) << 8;
> 
> In other words, I'd let the declaration cast the rhs. Not sure if this works,
> though.

Done.

https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringMI...
File src/IceTargetLoweringMIPS32.cpp (right):

https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringMI...
src/IceTargetLoweringMIPS32.cpp:1248: auto Value =
static_cast<uint32_t>(C32->getValue());
On 2016/02/07 16:18:33, John wrote:
> uint32_t Value = ...;
> 
> While at this, const?

Done.

https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringX8...
File src/IceTargetLoweringX86BaseImpl.h (right):

https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringX8...
src/IceTargetLoweringX86BaseImpl.h:2232: auto UDivisor =
static_cast<uint32_t>(Divisor);
On 2016/02/07 16:18:33, John wrote:
> uint32_t UDivisor = ...;
> 
> Can this be const?

Done.

https://codereview.chromium.org/1674033002/diff/20001/src/IceTargetLoweringX8...
src/IceTargetLoweringX86BaseImpl.h:2321: int32_t Divisor = C->getValue();
On 2016/02/07 16:18:33, John wrote:
> Ditto.

Done.

https://codereview.chromium.org/1674033002/diff/20001/src/PNaClTranslator.cpp
File src/PNaClTranslator.cpp (right):

https://codereview.chromium.org/1674033002/diff/20001/src/PNaClTranslator.cpp...
src/PNaClTranslator.cpp:2740: auto IsTailCall = static_cast<bool>(CCInfo & 1);
On 2016/02/07 16:18:33, John wrote:
> bool IsTailCall = ...;
> 
> While at this, const?

Done.

[Jim Stichnoth, 2016-02-07 17:50:31]

Description was changed from

==========
Subzero: Clean up some uses of *_cast<>.

Some casts to size_t for use as array indexes are simply unnecessary.

Some explicit declaration types are changed to "auto" to avoid redundancy with
the static_cast type.

A few llvm::dyn_cast<> operations are changed to llvm::cast<>, and vice versa.

A few explicit declaration types are changed to "auto" when used with
llvm::cast<> and llvm::dynamic_cast<>.  Some of these were missed during an
earlier cleansing because of multi-line issues.

There are still a few opportunities related to Variable register numbers, but
they are ignored for now because they are being addressed in another CL.

BUG= none
==========

to

==========
Subzero: Clean up some uses of *_cast<>.

Some casts to size_t for use as array indexes are simply unnecessary.

Some explicit declaration types are changed to "auto" to avoid redundancy with
the static_cast type.

A few llvm::dyn_cast<> operations are changed to llvm::cast<>, and vice versa.

A few explicit declaration types are changed to "auto" when used with
llvm::cast<> and llvm::dynamic_cast<>.  Some of these were missed during an
earlier cleansing because of multi-line issues.

There are still a few opportunities related to Variable register numbers, but
they are ignored for now because they are being addressed in another CL.

BUG= none
R=jpp@chromium.org

Committed:
https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-subzero.git;a...
==========

[Jim Stichnoth, 2016-02-07 17:50:32]

Committed patchset #4 (id:60001) manually as
2d6c82679e9786a924b76023aa30e1f44d0f8cbb (presubmit successful).