Consolidate shift processing into utility function

Consolidate shift processing into utility function

Move the three shift processing blocks into one function, refactor for some
removal of commonality.
BUG=none
R=stichnot@chromium.org

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

[sehr, 2015-09-22 23:20:06]

sehr@chromium.org changed reviewers:
+ jpp@chromium.org, stichnot@chromium.org

[sehr, 2015-09-22 23:20:19]

Some consolidation.  PTAL.

[Jim Stichnoth, 2015-09-22 23:49:28]

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

https://codereview.chromium.org/1364603002/diff/20001/src/IceTargetLoweringX8...
src/IceTargetLoweringX86Base.h:716: void lowerShift64(InstArithmetic::OpKind op,
Operand *Src0Lo, Operand *Src0Hi,
Name the arg "Op" per LLVM coding style.

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

https://codereview.chromium.org/1364603002/diff/20001/src/IceTargetLoweringX8...
src/IceTargetLoweringX86BaseImpl.h:1051: void
TargetX86Base<Machine>::lowerShift64(InstArithmetic::OpKind op,
s/op/Op/

https://codereview.chromium.org/1364603002/diff/20001/src/IceTargetLoweringX8...
src/IceTargetLoweringX86BaseImpl.h:1065: if (op == InstArithmetic::Shl) {
Maybe use a switch instead of if/then/else chain?

And then maybe the above assert() could become
  default: assert(0 && "non-shift op");

[sehr, 2015-09-23 16:02:45]

PTAL.

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

https://codereview.chromium.org/1364603002/diff/20001/src/IceTargetLoweringX8...
src/IceTargetLoweringX86Base.h:716: void lowerShift64(InstArithmetic::OpKind op,
Operand *Src0Lo, Operand *Src0Hi,
On 2015/09/22 23:49:27, stichnot wrote:
> Name the arg "Op" per LLVM coding style.

Done.

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

https://codereview.chromium.org/1364603002/diff/20001/src/IceTargetLoweringX8...
src/IceTargetLoweringX86BaseImpl.h:1051: void
TargetX86Base<Machine>::lowerShift64(InstArithmetic::OpKind op,
On 2015/09/22 23:49:28, stichnot wrote:
> s/op/Op/

Done.

https://codereview.chromium.org/1364603002/diff/20001/src/IceTargetLoweringX8...
src/IceTargetLoweringX86BaseImpl.h:1065: if (op == InstArithmetic::Shl) {
On 2015/09/22 23:49:28, stichnot wrote:
> Maybe use a switch instead of if/then/else chain?
> 
> And then maybe the above assert() could become
>   default: assert(0 && "non-shift op");

Done.

[Jim Stichnoth, 2015-09-23 20:19:47]

lgtm

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

https://codereview.chromium.org/1364603002/diff/40001/src/IceTargetLoweringX8...
src/IceTargetLoweringX86BaseImpl.h:1065: assert(0 && "non-shift op");
Here and below, I would add an explicit "break;" after the assert.

[sehr, 2015-09-24 18:43:00]

Committed patchset #4 (id:60001) manually as
188eae5c51cd6d8b4fd53dafe65d4ef7a1b5d867 (presubmit successful).