Add the ARM32 FP register table entries, simple arith, and args.

Add the ARM32 FP register table entries, simple arith, and args.

Lower some instructions, without much guarantee of
correctness. *Running* generated code will be risky
because the register allocator isn't aware of register
aliasing.

Fill in v{add,div,mul,sub}.f{32,64}, vmov, vldr
and vsqrt.f{32,64}. I tried to make the nacl-other-intrinsics
test not explode, so added vsqrt too. That was pretty
easy for sqrt, but then fabs tests also exploded. Those are not
truly fixed but are currently "fixed" by adding a FakeDef to
satisfy liveness.

Propagate float/double arguments to the right register
in lowerArguments, lowerCall, and propagate to s0/d0/q0
for lowerReturn. May need to double check the calling convention.
Currently can't test call-ret because vpush/vpop for prologues
and epilogues isn't done.

Legalize FP immediates to make the nacl-other-intrinsics sqrt
test happy. Use the correct type of load (vldr (.32 and .64 are
optional) instead of ldr{b,h,,d}).

Whether or not the float/vector instructions can be
predicated is a bit interesting. The float/double ones
can, but the SIMD versions cannot. E.g.

vadd<cond>.f32 s0, s0, s1 is okay
vadd<cond>.f32 q0, q0, q1 is not okay.

For now, just omit conditions from instructions that may
end up being reused for SIMD.

Split up the fp.pnacl.ll test into multiple ones so that
parts of lowering can be tested incrementally.

BUG= https://code.google.com/p/nativeclient/issues/detail?id=4076
R=stichnot@chromium.org

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

[jvoung (off chromium), 2015-08-07 00:34:35]

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

[Jim Stichnoth, 2015-08-07 15:11:36]

This otherwise looks gtm, but I'd like to hear your thoughts on the register
allocation issues.

https://codereview.chromium.org/1266263003/diff/170001/src/IceInstARM32.def
File src/IceInstARM32.def (right):

https://codereview.chromium.org/1266263003/diff/170001/src/IceInstARM32.def#n...
src/IceInstARM32.def:29: #define REGARM32_GPR_TABLE                             
                \
Fix all the backslash alignment.  Maybe put them all in column 80 like
clang-format likes to do?

https://codereview.chromium.org/1266263003/diff/170001/src/IceInstARM32.def#n...
src/IceInstARM32.def:71: X(Reg_s0 , 0 , "s0", 1, 0, 0, 0, 0, 1, 0, 0)     \
FWIW, I had some early thoughts on the register overlap/alias problem.  My idea
was to combine "s", "d", and "q" registers into the same table, where e.g.
s12/d6/q3 are in the same table row, similar to how eax/ax/al are in the same
table row for x86-32.

The register allocator would need to look at sequences of registers instead of
individual registers.  I think there are 3 instances of find_first in
IceRegAlloc.cpp where changes would be needed:

- In addFillSpill(), and later when Free.find_first() is called.  Change to find
an available aligned sequence.

- The code starting with "MinWeightIndex = RegMask.find_first();".  Calculate a
weight for the whole aligned sequence instead of just one register.

- Most of the llvm::SmallVector and some of the llvm::SmallBitVector variables
and the code that use them will need extra thought/bookkeeping.

- I haven't thought much about implications for register randomization.

I'm hoping that this approach would be simpler than a general solution for
register aliasing.

[jvoung (off chromium), 2015-08-08 00:32:44]

https://codereview.chromium.org/1266263003/diff/170001/src/IceInstARM32.def
File src/IceInstARM32.def (right):

https://codereview.chromium.org/1266263003/diff/170001/src/IceInstARM32.def#n...
src/IceInstARM32.def:29: #define REGARM32_GPR_TABLE                             
                \
On 2015/08/07 15:11:36, stichnot wrote:
> Fix all the backslash alignment.  Maybe put them all in column 80 like
> clang-format likes to do?

Done.

Maybe the in-comments python snippets can be improved to handle alignment and
the trailing slash better, and move them into a real .py file in pydir so
they're easier to run / get cleanly formatted automatically.

https://codereview.chromium.org/1266263003/diff/170001/src/IceInstARM32.def#n...
src/IceInstARM32.def:71: X(Reg_s0 , 0 , "s0", 1, 0, 0, 0, 0, 1, 0, 0)     \
On 2015/08/07 15:11:36, stichnot wrote:
> FWIW, I had some early thoughts on the register overlap/alias problem.  My
idea
> was to combine "s", "d", and "q" registers into the same table, where e.g.
> s12/d6/q3 are in the same table row, similar to how eax/ax/al are in the same
> table row for x86-32.


Do you mean something like

s0, d0, q0
s1, d0, q0
s2, d1, q0
s3, d1, q0
s4, d2, q1
//...
s60(fake), d30, q15
//...
s63(fake), d31, q15

or just omit the option of selecting s1,2,3 and work with s0,s4,...s28 for fp32,
but make s32(fake)...s60(fake) available for fp64 and simd?

s0, d0, q0
s4, d2, q1
//...
s60(fake), d30, q15

Over lunch John suggested something which sounds similar to what you're
suggesting, IIUC.  Have option 1 of the individual rows. Then when allocating
f32, look for 1 free reg, for f64 look for a sequence of 2 free regs, for
128-bit look for a sequence of 4 free regs. Similar for marking registers as
used, etc. For filling in the Variable RegNum field, probably just use the first
value (e.g., s0 instead of s1, for d0, so that there's more of a normal form for
checking isRedundantMove?).

=====

My initial thought was to add another column for "representative number"
(union-find-ish), where s0-3, d0-1, and q0 would be given the same
representative number. Then checks for overlap/the same group would be pretty
fast (are the rep #s the same). I didn't know if we needed a mapping the other
way though. E.g., from q0 to a list of [s0, s1, s2, s3] or a list of [d0, d1].
That sounds more confusing than using "s" units though... and may be looking at
the wrong part of regalloc (conflicts from what appear to be distinct elements,
vs finding free units). Keeping s/d/q with separate rows probably bloats the
bitvectors way more too.

=====

Would you want to take a look at this regalloc problem and take a stab at this
instead of me?


> 
> The register allocator would need to look at sequences of registers instead of
> individual registers.  I think there are 3 instances of find_first in
> IceRegAlloc.cpp where changes would be needed:
> 
> - In addFillSpill(), and later when Free.find_first() is called.  Change to
find
> an available aligned sequence.
> 
> - The code starting with "MinWeightIndex = RegMask.find_first();".  Calculate
a
> weight for the whole aligned sequence instead of just one register.
> 
> - Most of the llvm::SmallVector and some of the llvm::SmallBitVector variables
> and the code that use them will need extra thought/bookkeeping.
> 
> - I haven't thought much about implications for register randomization.
> 
> I'm hoping that this approach would be simpler than a general solution for
> register aliasing.

[Jim Stichnoth, 2015-08-09 00:10:05]

lgtm

https://codereview.chromium.org/1266263003/diff/170001/src/IceInstARM32.def
File src/IceInstARM32.def (right):

https://codereview.chromium.org/1266263003/diff/170001/src/IceInstARM32.def#n...
src/IceInstARM32.def:71: X(Reg_s0 , 0 , "s0", 1, 0, 0, 0, 0, 1, 0, 0)     \
On 2015/08/08 00:32:44, jvoung wrote:
> On 2015/08/07 15:11:36, stichnot wrote:
> > FWIW, I had some early thoughts on the register overlap/alias problem.  My
> idea
> > was to combine "s", "d", and "q" registers into the same table, where e.g.
> > s12/d6/q3 are in the same table row, similar to how eax/ax/al are in the
same
> > table row for x86-32.
> 
> 
> Do you mean something like
> 
> s0, d0, q0
> s1, d0, q0
> s2, d1, q0
> s3, d1, q0
> s4, d2, q1
> //...
> s60(fake), d30, q15
> //...
> s63(fake), d31, q15
> 
> or just omit the option of selecting s1,2,3 and work with s0,s4,...s28 for
fp32,
> but make s32(fake)...s60(fake) available for fp64 and simd?
> 
> s0, d0, q0
> s4, d2, q1
> //...
> s60(fake), d30, q15
> 
> Over lunch John suggested something which sounds similar to what you're
> suggesting, IIUC.  Have option 1 of the individual rows. Then when allocating
> f32, look for 1 free reg, for f64 look for a sequence of 2 free regs, for
> 128-bit look for a sequence of 4 free regs. Similar for marking registers as
> used, etc. For filling in the Variable RegNum field, probably just use the
first
> value (e.g., s0 instead of s1, for d0, so that there's more of a normal form
for
> checking isRedundantMove?).
> 
> =====
> 
> My initial thought was to add another column for "representative number"
> (union-find-ish), where s0-3, d0-1, and q0 would be given the same
> representative number. Then checks for overlap/the same group would be pretty
> fast (are the rep #s the same). I didn't know if we needed a mapping the other
> way though. E.g., from q0 to a list of [s0, s1, s2, s3] or a list of [d0, d1].
> That sounds more confusing than using "s" units though... and may be looking
at
> the wrong part of regalloc (conflicts from what appear to be distinct
elements,
> vs finding free units). Keeping s/d/q with separate rows probably bloats the
> bitvectors way more too.
> 
> =====
> 
> Would you want to take a look at this regalloc problem and take a stab at this
> instead of me?
> 
> 
> > 
> > The register allocator would need to look at sequences of registers instead
of
> > individual registers.  I think there are 3 instances of find_first in
> > IceRegAlloc.cpp where changes would be needed:
> > 
> > - In addFillSpill(), and later when Free.find_first() is called.  Change to
> find
> > an available aligned sequence.
> > 
> > - The code starting with "MinWeightIndex = RegMask.find_first();". 
Calculate
> a
> > weight for the whole aligned sequence instead of just one register.
> > 
> > - Most of the llvm::SmallVector and some of the llvm::SmallBitVector
variables
> > and the code that use them will need extra thought/bookkeeping.
> > 
> > - I haven't thought much about implications for register randomization.
> > 
> > I'm hoping that this approach would be simpler than a general solution for
> > register aliasing.
> 

I was thinking of a representation along these lines:

/* val,      encode, name32, name64, name128, nreg32, nreg64, nreg128, ... */
  X(Reg_s0,   0,     "s0",   "d0",   "q0",    1,      2,      4,       ...)
  X(Reg_s1,   1,     "s1",   "??",   "??",    1,      0,      0,       ...)
  X(Reg_s2,   2,     "s2",   "d1",   "??",    1,      2,      0,       ...)
  X(Reg_s3,   3,     "s3",   "??",   "??",    1,      0,      0,       ...)
  X(Reg_s4,   4,     "s4",   "d2",   "q1",    1,      2,      4,       ...)
  X(Reg_s5,   5,     "s5",   "??",   "??",    1,      0,      0,       ...)
  X(Reg_s6,   6,     "s6",   "d3",   "??",    1,      2,      0,       ...)
  X(Reg_s7,   7,     "s7",   "??",   "??",    1,      0,      0,       ...)
  X(Reg_s8,   8,     "s8",   "d4",   "q4",    1,      2,      4,       ...)
  X(Reg_s9,   9,     "s9",   "??",   "??",    1,      0,      0,       ...)
  X(Reg_s10, 10,     "s10",  "d5",   "??",    1,      2,      0,       ...)
  X(Reg_s11, 11,     "s11",  "??",   "??",    1,      0,      0,       ...)
etc.

Here, the Reg_s<x> column is the representative number.  The nreg<w> column
tells you if the register is available for that width (value>0), and how many
consecutive unit registers it takes.

This representation isn't quite sufficient because it's hard to map in both
directions, but presumably it can continue along those lines.

One could test it on x86-32 and maybe x86-64 by using it to represent
eax/ax/al/ah and friends, though for translator performance reasons, it might be
better to use the existing approach.

And yeah, I can take a crack at modifying the register allocator that way, and
in the meantime it should be fine to land the CL the current way.

[jvoung (off chromium), 2015-08-09 14:58:40]

Committed patchset #12 (id:210001) manually as
86ebec12680b5268fcb7082f770a26394e8b8080 (presubmit successful).