Skip to content

S
swiftshader
Switch branch/tag
  1. 22 May, 2015 2 commits
  3. 19 May, 2015 2 commits
    • Lower a few basic ARM binops for i{8,16,32,64}. · 2971997a
      Jan Voung authored 
      Do basic lowering for add, sub, and, or, xor, mul.
We don't yet take advantage of commuting immediate operands
(e.g., use rsb to reverse subtract instead of sub) or
inverting immediate operands (use bic to bit clear instead
of using and).

The binary operations can set the flags register (e.g., to
have the carry bit for use with a subsequent adc
instruction). That is optional for the "data processing"
instructions.

I'm not yet able to compile 8bit.pnacl.ll and
64bit.pnacl.ll so 8-bit and 64-bit are not well tested yet.
Only tests are in the arith.ll file (like arith-opt.ll, but
assembled instead of testing the "verbose inst" output).

Not doing divide yet. ARM divide by 0 does not trap, but
PNaCl requires uniform behavior for such bad code. Thus,
in LLVM we insert a 0 check and would have to do the same.

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

Review URL: https://codereview.chromium.org/1127003003
    • Subzero: Use cmov to improve lowering for the select instruction. · 537b5ba0
      Jim Stichnoth authored 
      This is instead of explicit control flow which may interfere with branch prediction.  However, explicit control flow is still needed for types other than i16 and i32, due to cmov limitations.

The assembler for cmov is extended to allow the non-dest operand to be a memory operand.

The select lowering is getting large enough that it was in our best interest to combine the default lowering with the bool-folding optimization.

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

Review URL: https://codereview.chromium.org/1125323004
  5. 18 May, 2015 1 commit
  7. 17 May, 2015 1 commit
    • Subzero: Fold icmp into br/select lowering. · a59ae6ff
      Jim Stichnoth authored 
      Originally there was a peephole-style optimization in lowerIcmp() that looks ahead to see if the next instruction is a conditional branch with the right properties, and if so, folds the icmp and br into a single lowering sequence.

However, sometimes extra instructions come between the icmp and br instructions, disabling the folding even though it would still be possible.

One thought is to do the folding inside lowerBr() instead of lowerIcmp(), by looking backward for a suitable icmp instruction.  The problem here is that the icmp lowering code may leave lowered instructions that can't easily be dead-code eliminated, e.g. instructions lacking a dest variable.

Instead, before lowering a basic block, we do a prepass on the block to identify folding candidates.  For the icmp/br example, the prepass would tentatively delete the icmp instruction and then the br lowering would fold in the icmp.

This folding can also be extended to several producers:
  icmp (i32 operands), icmp (i64 operands), fcmp, trunc .. to i1
and several consumers:
  br, select, sext, zext

This CL starts with 2 combinations: icmp32 paired with br & select.  Other combinations will be added in later CLs.

BUG= https://code.google.com/p/nativeclient/issues/detail?id=4162
BUG= https://code.google.com/p/nativeclient/issues/detail?id=4095
R=jvoung@chromium.org

Review URL: https://codereview.chromium.org/1141213004
  9. 16 May, 2015 1 commit
  11. 14 May, 2015 1 commit
    • Convert Constant->emit() definitions to allow multiple targets to define them. · 76bb0bec
      Jan Voung authored 
      Wasn't sure how to allow TargetX8632 and TargetARM32
to both define "ConstantInteger32::emit(GlobalContext *)",
and define them differently if both targets happen to be
ifdef'ed into the code. Rearranged things so that it's now
"TargetFoo::emit(ConstantInteger32 *)", so that each
TargetFoo can have a separate definition.

Some targets may allow emitting some types of constants
while other targets do not (64-bit int for x86-64?).
Also they emit constants with a different style.
E.g., the prefix for x86 is "$" while the prefix for ARM
is "#" and there isn't a prefix for mips(?).
Renamed emitWithoutDollar to emitWithoutPrefix.

Did this sort of multi-method dispatch via a visitor
pattern, which is a bit verbose though.

We may be able to remove the emitWithoutDollar/Prefix for
ConstantPrimitive by just inlining that into the few places
that need it (only needed for ConstantInteger32). This
undoes the unreachable methods added by: https://codereview.chromium.org/1017373002/diff/60001/src/IceTargetLoweringX8632.cpp
The only place extra was for emitting calls to constants.
There was already an inlined instance for OperandX8632Mem.

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

Review URL: https://codereview.chromium.org/1129263005
  13. 12 May, 2015 2 commits
  15. 07 May, 2015 1 commit
  17. 04 May, 2015 1 commit
    • Subzero: Use a setcc sequence for better icmp lowering. · f48b320c
      Jim Stichnoth authored 
      For an example like:
  %a = icmp eq i32 %b, %c

The original icmp lowering sequence for i8/i16/i32 was something like:

  cmpl b, c
  movb 1, a
  je label
  movb 0, a
label:

The improved sequence is:
  cmpl b, c
  sete a

In O2 mode, this doesn't help when successive compare/branch instructions are fused, but it does help when the boolean result needs to be saved and later used.

BUG= none
R=jvoung@chromium.org

Review URL: https://codereview.chromium.org/1118353005
  19. 30 Apr, 2015 3 commits
  21. 29 Apr, 2015 1 commit
    • Subzero: Produce actually correct code in --asm-verbose mode. · 76dcf1a8
      Jim Stichnoth authored 
      The "pnacl-sz --asm-verbose=1" mode annotates the asm output with physical register liveness information, including which registers are live at the beginning and end of each basic block, and which registers' live ranges end at each instruction.  Computing this information requires a final liveness analysis pass.  One of the side effects of liveness analysis is to remove dead instructions, which happens when the instruction's dest variable is not live and the instruction lacks important side effects.

In some cases, direct manipulation of physical registers was missing extra fakedef/fakeuse/etc., and as as result these instructions could be eliminated, leading to incorrect code.  Without --asm-verbose, these instructions were being created after the last run of liveness analysis, so they had no chance of being eliminated and everything was fine.  But with --asm-verbose, some instructions would be eliminated.

This CL fixes the omissions so that the resulting code is runnable.

An alternative would be to add a flag to liveness analysis directing it not to dead-code eliminate any more instructions.  However, it's better to get the liveness right in case future late-stage optimizations rely on it.

BUG= https://code.google.com/p/nativeclient/issues/detail?id=4135
TEST= pydir/szbuild_spec2k.py --filetype=asm -v --sz=--asm-verbose=1 --force
R=jvoung@chromium.org, kschimpf@google.com

Review URL: https://codereview.chromium.org/1113683002
  23. 28 Apr, 2015 1 commit
    • Subzero: Fix asm (non-ELF) output files. · 620ad732
      Jim Stichnoth authored 
      In an earlier version of Subzero, the text output stream object was
stack-allocated within main.  A later refactoring moved its allocation
into a helper function, but it was still being stack-allocated, which
was bad when the helper function returned.

This change allocates the object via "new", which fixes that problem,
but reveals another problem: the raw_ostream object for some reason
doesn't finish writing everything to disk and yielding a truncated
output file.  This is solved in the style of the ELF streamer, by
using raw_fd_ostream instead.

BUG= none
R=kschimpf@google.com

Review URL: https://codereview.chromium.org/1111603003
  25. 22 Apr, 2015 2 commits
  27. 21 Apr, 2015 2 commits
    • Subzero: Improve "make check-unit" execution. · e7e9b024
      Jim Stichnoth authored 
      If you switch between "cmake" and "autoconf" toolchain builds, and
neglect to clean out pnacl_newlib_raw/ in between, the wrong libgtest
and libgtest_main may get pulled in for the autoconf build, leading to
an assertion failure in "make check-unit".

This tweak fixes that problem by rejiggering the lib search path.

BUG= none
R=jvoung@chromium.org

Review URL: https://codereview.chromium.org/1099093005
    • Subzero: Auto-detect cmake versus autoconf LLVM build. · 0a9e1261
      Jim Stichnoth authored 
      The CMAKE=1 option is no longer needed.

Pretty much all the tools we need are now in pnacl_newlib_raw/bin, so use PNACL_BIN_PATH set to that instead of using LLVM_BIN_PATH and BINUTILS_BIN_PATH.

However, for the autoconf build, libgtest and libtest_main and clang-format are only under the llvm_x86_64_linux_work directory, so they need special casing.  This also means that you have to actually do an LLVM build and not blow away the work directory in order to "make check-unit" or "make format".

BUG= none
R=jvoung@chromium.org

Review URL: https://codereview.chromium.org/1085733002
  29. 16 Apr, 2015 5 commits
  31. 10 Apr, 2015 1 commit
  33. 09 Apr, 2015 2 commits
  35. 07 Apr, 2015 1 commit
  37. 06 Apr, 2015 1 commit
  39. 31 Mar, 2015 1 commit
    • Add argv[0] before parsing commandline flags. · 9c1d3869
      Jan Voung authored 
      The \0 delimited string array that the browser sends doesn't have
the program name and the IRT only tokenizes that and forwards
it along. We need argv[0] to make the llvm CL parser happy
(used for -help message, etc).

Alternatively, we could have the IRT fill in a program name
so that the argv is a real argv. That will involve less copying since
the argv will be the right size to begin with, but prevents each app
from customizing its argv[0] =/

BUG= https://code.google.com/p/nativeclient/issues/detail?id=4091
TEST= manual for now (construct the sel_universal script to only pass
the "--build-atts" flag and see it exits without being swallowed,
or pass "-Ofoo" and see an error + exit)

R=mtrofin@chromium.org, stichnot@chromium.org

Review URL: https://codereview.chromium.org/1041843003
  41. 30 Mar, 2015 1 commit
  43. 29 Mar, 2015 1 commit
    • Subzero: Fix dependency checking to avoid unnecessary rebuilds. · 8c7b0a2c
      Jim Stichnoth authored 
      When trying to do bisection debugging, the pnacl-llc translation was happening every time even if the pexe didn't change.  This is because it was checking for a binary called 'llc' in the current directory, instead of an absolute path the pnacl-llc.  (This check is done so that updating pnacl-llc triggers a rebuild of the bisection binary, similar to the check for an update of pnacl-sz.)

BUG= none
R=jvoung@chromium.org

Review URL: https://codereview.chromium.org/1044623003
  45. 27 Mar, 2015 1 commit
    • Refactor Subzero initialization and add a browser callback handler. · 44c3a804
      Jan Voung authored 
      Handlers are represented as a "compile server" even though
right now it can really only handle a single
compile request.

Then there can be a commandline-based server and a
browser-based server. This server takes over the main
thread. In the browser-based case the server can block,
waiting on bytes to be pushed. This becomes a producer of
bitcode bytes.

The original main thread which did bitcode reading is now
shifted to yet another worker thread, which is then the
consumer of bitcode bytes.

This uses an IRT interface for listening to messages
from the browser:
https://codereview.chromium.org/984713003/

TEST=Build the IRT core nexe w/ the above patch and compile w/ something like:

echo """
readwrite_file objfile /tmp/temp.nexe---gcc.opt.stripped.pexe---.o
rpc StreamInitWithSplit i(4) h(objfile) h(invalid) h(invalid) h(invalid) h(invalid) h(invalid) h(invalid) h(invalid) h(invalid) h(invalid) h(invalid) h(invalid) h(invalid) h(invalid) h(invalid) h(invalid) C(4,-O2\x00) * s()
stream_file /usr/local/google/home/jvoung/pexe_tests/gcc.opt.stripped.pexe 65536 1000000000
rpc StreamEnd * i() s() s() s()
echo "pnacl-sz complete"
""" | scons-out/opt-linux-x86-32/staging/sel_universal \
    -a -B scons-out/nacl_irt-x86-32/staging/irt_core.nexe \
    --abort_on_error \
    -- toolchain/linux_x86/pnacl_translator/translator/x86-32/bin/pnacl-sz.nexe

echo """
readwrite_file nexefile /tmp/temp.nexe.tmp
readonly_file objfile0 /tmp/temp.nexe---gcc.opt.stripped.pexe---.o
rpc RunWithSplit i(1) h(objfile0) h(invalid) h(invalid) h(invalid) h(invalid) h(invalid) h(invalid) h(invalid) h(invalid) h(invalid) h(invalid) h(invalid) h(invalid) h(invalid) h(invalid) h(invalid) h(nexefile) *
echo "ld complete"
""" | /usr/local/google/home/nacl3/native_client/scons-out/opt-linux-x86-32/staging/sel_universal \
    --abort_on_error \
    -a -B \
    scons-out/nacl_irt-x86-32/staging/irt_core.nexe \
    -E NACL_IRT_OPEN_RESOURCE_BASE=toolchain/linux_x86/pnacl_translator/translator/x86-32/lib/ \
    -E NACL_IRT_OPEN_RESOURCE_REMAP=libpnacl_irt_shim.a:libpnacl_irt_shim_dummy.a \
    -- toolchain/linux_x86/pnacl_translator/translator/x86-32/bin/ld.nexe

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

Review URL: https://codereview.chromium.org/997773002
  47. 24 Mar, 2015 2 commits
    • Subzero: Fix a lowering bug involving xchg and xadd instructions. · 0e432ac4
      Jim Stichnoth authored 
      The x86-32 xchg and xadd instructions are modeled using two source operands, one of which is a memory operand and the other ultimately a physical register.  These instructions have a side effect of modifying both operands.

During lowering, we need to specially express that the instruction modifies the Variable operand (since it doesn't appear as the instruction's Dest variable).  This makes the register allocator aware of the Variable being multi-def, and prevents it from sharing a register with an overlapping live range.

This was being partially expressed by adding a FakeDef instruction.  However, FakeDef instructions are still allowed to be dead-code eliminated, and if this happens, the Variable may appear to be single-def, triggering the unsafe register sharing.

The solution is to prevent the FakeDef instruction from being eliminated, via a FakeUse instruction.

It turns out that the current register allocator isn't aggressive enough to manifest the bug with cmpxchg instructions, but the fix and tests are there just in case.

BUG= none
R=jvoung@chromium.org

Review URL: https://codereview.chromium.org/1020853011
    • Make compile without ICE_THREAD_LOCAL_HACK (avoid "Type *TLS = TLS;") · 3e5009f6
      Jan Voung authored 
      Otherwise you get:

In file included from src/IceGlobalContext.cpp:21:
In file included from src/IceCfg.h:21:
src/IceGlobalContext.h:257:44: error: variable 'TLS' is uninitialized when used within its own initialization [-Werror,-Wuninitialized]
    ThreadContext *TLS = ICE_TLS_GET_FIELD(TLS);
                   ~~~                     ^~~
src/IceTLS.h:95:39: note: expanded from macro 'ICE_TLS_GET_FIELD'
                                      ^
So rename the local var to Tls.

BUG=none
R=stichnot@chromium.org

Review URL: https://codereview.chromium.org/1030793002
  49. 23 Mar, 2015 3 commits
    • Subzero: Don't use key SSE instructions on potentially unaligned loads. · f79d2cb6
      Jim Stichnoth authored 
      The non-mov-like SSE instructions generally require 16-byte aligned memory operands.  The PNaCl bitcode ABI only guarantees 4-byte alignment or less on vector loads and stores.  Subzero maintains stack alignment so stack memory operands are fine.

We handle this by legalizing memory operands into a register wherever there is doubt.

This bug was first discovered on the vector_align scons test.

BUG= https://code.google.com/p/nativeclient/issues/detail?id=4083
BUG= https://code.google.com/p/nativeclient/issues/detail?id=4133
R=jvoung@chromium.org

Review URL: https://codereview.chromium.org/1024253003
    • Subzero: Prune unreachable nodes after constructing the Cfg. · 69d3f9c6
      Jim Stichnoth authored 
      The gcc torture test suite has examples where there is a function call (to a routine that throws an exception or aborts or something), followed by an "unreachable" instruction, followed by more code that may e.g. return a value to the caller.  In these examples, the code following the unreachable is itself unreachable.

Problems arise when the unreachable code references a variable defined in the reachable code.  This triggers a liveness consistency error because the use of the variable has no reaching definition.

It's a bit surprising that LLVM actually allows this, but it does so we need to deal with it.

The solution is, after initial CFG construction, do a traversal starting from the entry node and then delete any undiscovered nodes.

There is code in Subzero that assumes Cfg::Nodes[i]->Number == i, so the nodes need to be renumbered after pruning.  The alternative was to set Nodes[i]=nullptr and not change the node number, but that would mean peppering the code base with CfgNode null checks.

BUG= none
R=jvoung@chromium.org

Review URL: https://codereview.chromium.org/1027933002
    • Subzero: Fix inappropriate use of nullptr. · 27c56bf6
      Jim Stichnoth authored 
      When lowering of a couple of atomic intrinsics down to a loop structure, a FakeUse on the memory address's base variable is created.  However, if the memory address is a global constant, there is no base variable.  So check for that and don't create a FakeUse if there is none.

BUG= none
TEST=synchronization_sync (scons test)
R=jvoung@chromium.org

Review URL: https://codereview.chromium.org/1023673007