- 02 Oct, 2015 3 commits
-
-
Karl Schimpf authored
The pnacl linux x86_64 buildbot doesn't understand ::stdout (it uses a macro to define stdout). Fix by removing :: prefix. Also redirects the error messages to stderr instead of stdout. BUG=None R=stichnot@chromium.org Review URL: https://codereview.chromium.org/1383053002 .
-
Karl Schimpf authored
Fixes bug in function reportFatalErrorThenExitSuccess by using fwrite instead of write (a unix posix include file not supported by MSC). BUG=None R=stichnot@chromium.org Review URL: https://codereview.chromium.org/1370323005 .
-
Jim Stichnoth authored
The problem is that given code like this: a = b + c d = a + e ... ... (use of a) ... Lowering may produce code like this, at least on x86: T1 = b T1 += c a = T1 T2 = a T2 += e d = T2 ... ... (use of a) ... If "a" has a long live range, it may not get a register, resulting in clumsy code in the middle of the sequence like "a=reg; reg=a". Normally one might expect store forwarding to make the clumsy code fast, but it does presumably add an extra instruction-retirement cycle to the critical path in a pointer-chasing loop, and makes a big difference on some benchmarks. The simple fix here is, at the end of lowering "a=b+c", keep track of the final "a=T1" assignment. Then, when lowering "d=a+e" and we look up "a", we can substitute "T1". This slightly increases the live range of T1, but it does a great job of avoiding the redundant reload of the register from the stack location. A more general fix (in the future) might be to do live range splitting and let the register allocator handle it. BUG= https://code.google.com/p/nativeclient/issues/detail?id=4095 R=kschimpf@google.com Review URL: https://codereview.chromium.org/1385433002 .
-
- 01 Oct, 2015 7 commits
-
-
John Porto authored
This is in preparation for llvm.nacl.atomic.* lowerings. atomic i64 loads and stores require their operands to be consecutive registers starting at an even register that is not r14. BUG= https://code.google.com/p/nativeclient/issues/detail?id=4076 R=kschimpf@google.com Review URL: https://codereview.chromium.org/1382063002 .
-
John Porto authored
This bug was uncovered While implementing the llvm.nacl.atomic.cmpxchg lowering for i64 for ARM32. For reference, the lowering is retry: ldrexd tmp_i, tmp_i+1 [addr] cmp tmp_i+1, expected_i+1 cmpeq tmp_i, expected_i strexdeq success, new_i, new_i+1, [addr] movne expected_i+1, tmp_i+1 movne expected_i, tmp_i cmpeq success, #0 bne retry mov dest_i+1, tmp_i+1 mov dest_i, tmp_i The register allocator would allocate r4 to both success and new_i, which is clearly wrong (expected_i is alive thought the cmpxchg loop.) Adding a fake-use(new_i) after the loop caused the register allocator to fail due to the impossibility to allocate a register for an infinite weight register. The problem was being caused for not evicting live ranges that were assigned registers that alias the selected register. BUG= R=kschimpf@google.com, stichnot@chromium.org Review URL: https://codereview.chromium.org/1373823006 . -
John Porto authored
These instructions are used to load/store data atomically, and to notify the processor about a data memory barrier. They are used for implementing the llvm.nacl.atomic.* lowerings. BUG= https://code.google.com/p/nativeclient/issues/detail?id=4076 R=stichnot@chromium.org Review URL: https://codereview.chromium.org/1378303003 .
-
Karl Schimpf authored
A recent change to IceCompilerServer.cpp was added to allow fatal errors to return exit status zero. However, this code called ::write (a C function) that is not defined when compiling with MSC. This CL adds includes to fix this problem. BUG=None R=stichnot@chromium.org Review URL: https://codereview.chromium.org/1379613005 .
-
Jim Stichnoth authored
When the register allocator decides whether to allow the candidate's live range to overlap its preferred variable's live range (and share their register), it needs to consider whether any redefinitions in one variable occur within the live range of the other variable, in which case overlap should not be allowed. There was a bug in the API for iterating over the defining instructions for a variable, in which the earliest definition might be ignored in some cases. This came from the fact that the first definition and latter definitions are split apart for translation speed reasons, and a particular API is needed for finding an unambiguous first definition, which is possible when all definitions are within a single block but not so possible when definitions cross block boundaries. (This only happens for the simple phi lowering.) Since both semantics are needed, a separate API is added to support both. For spec2k, the asm output is identical to before, so this changes nothing. When translating spec2k with "-O2 -phi-edge-split=0", there is a single minor difference in ammp that actually looks legit in both cases. However, when testing an upcoming CL, -phi-edge-split=0 triggered the bug, causing gcc and crafty to fail with incorrect output. This CL also fixes some minor issues, and adds dump output of the instruction definition list when available. BUG= none R=jpp@chromium.org Review URL: https://codereview.chromium.org/1381563004 .
-
Jim Stichnoth authored
The std::list<> implementation used by g++ needs some extra stuff defined in the custom allocator. This can be smoke-tested with: make -f Makefile.standalone CXX=g++ LLVM_EXTRA_WARNINGS="-Wno-unknown-pragmas -Wno-unused-parameter -Wno-comment -Wno-enum-compare -Wno-strict-aliasing" STDLIB_FLAGS= until the link fails for unrelated reasons. BUG= https://code.google.com/p/nativeclient/issues/detail?id=4325 R=kschimpf@google.com Review URL: https://codereview.chromium.org/1367403004 .
-
Jim Stichnoth authored
Frame offsets for variables are emitted using a symbolic name based on the variable's name. This makes it a bit easier to digest the asm code. For example, if variable Foo gets an esp offset 24, asm like this: ... 24(%esp) ... will instead be emitted like this: lv$Foo = 24 ... ... lv$Foo(%esp) ... Predecessor labels are printed for each basic block. Loop nest depth is printed for each basic block. (Would be nice if we had loop header info as well.) BUG= none R=jpp@chromium.org Review URL: https://codereview.chromium.org/1377323002 .
-
- 30 Sep, 2015 2 commits
-
-
Karl Schimpf authored
Moves the alignment check method from the function block parser, to the top-level parser, and then uses it to also check alignment on global variables. BUG= https://code.google.com/p/nativeclient/issues/detail?id=4329 R=stichnot@chromium.org Review URL: https://codereview.chromium.org/1378163002 .
-
Karl Schimpf authored
Fixes pnacl-sz to return with exit status 0 in report_fatal_error, if command line flag --exit-status is specified. The importance of this is that it allows afl-fuzz to not report the mutation as a crash. In addition, afl-fuzz doesn't record crash paths in its search history. By returning success, afl-fuzz can continue to apply additional mutations to the bad input. This allows afl-fuzz to add errors that require multiple changes to occur on the input. BUG=None R=stichnot@chromium.org Review URL: https://codereview.chromium.org/1382653002 .
-
- 28 Sep, 2015 2 commits
-
-
Karl Schimpf authored
Fixes bug where code did not check that the address of an indirect call must be i32. BUG= https://code.google.com/p/nativeclient/issues/detail?id=4321 R=stichnot@chromium.org Review URL: https://codereview.chromium.org/1363983002 .
-
John Porto authored
This is in preparation for adding atomic support to the ARM backend. Moreover, the code is becoming increasingly complicated due to the use of Variable64On32 as instruction operands. BUG= https://code.google.com/p/nativeclient/issues/detail?id=4076 R=kschimpf@google.com Review URL: https://codereview.chromium.org/1372083002 .
-
- 26 Sep, 2015 1 commit
-
-
Jim Stichnoth authored
1. Rename all identifiers containing "nonkillable" to use the more understandable "redefined". 2. Change inferTwoAddress() to be called inferRedefinition(), and to check *all* instruction source variables (instead of just the first source operand) against the Dest variable. This eliminates the need for several instances of _set_dest_redefined(). The performance impact on translation time is something like 0.1%, which is dwarfed by the usability gain. 3. Change a cryptic assert in (O2) live range construction to print detailed information on the liveness errors. 4. Change a cryptic assert in (Om1) live range construction to do the same. BUG= none R=jpp@chromium.org Review URL: https://codereview.chromium.org/1368993004 .
-
- 25 Sep, 2015 1 commit
-
-
John Porto authored
This patch enables many crosstests for ARM32. Very limited vector support is implemented (essentially, whatever it takes to compile the .ll files contain vector operations.) Atomics as well as vector crosstests are still disabled. BUG= https://code.google.com/p/nativeclient/issues/detail?id=4076 R=stichnot@chromium.org Review URL: https://codereview.chromium.org/1359193003 .
-
- 24 Sep, 2015 1 commit
-
-
David Sehr authored
Move the three shift processing blocks into one function, refactor for some removal of commonality. BUG=none R=stichnot@chromium.org Review URL: https://codereview.chromium.org/1364603002 .
-
- 23 Sep, 2015 2 commits
-
-
Jim Stichnoth authored
PNaCl simplifies varargs calls by creating a known-size argument array with an alloca instruction, and passing the address of that argument array. These alloca instructions don't necessarily require use of a frame pointer, freeing up the frame pointer register for normal register allocation. These varargs calls sometimes show up in cold paths of hot functions, so increasing the number of registers available to the register allocator can produce tangible gains. This patch does a simple recognition of these alloca patterns, and on x86 doesn't force a frame pointer if all alloca instructions are suitable. Future work is to avoid saving the alloca result as a local variable, and instead rematerialize the address as needed with respect to the stack or frame pointer. BUG= none R=jpp@chromium.org Review URL: https://codereview.chromium.org/1361803002 .
-
Jim Stichnoth authored
BUG= none R=jpp@chromium.org Review URL: https://codereview.chromium.org/1356953003 .
-
- 22 Sep, 2015 4 commits
-
-
Karl Schimpf authored
Fixes instrinsic function "validateCall" to properly define which parameter type doesn't match the expected signature for that intrinsic. Previous code did not take into account that the first element of the intrinsic signature was the return type. Also fixes error messages to print the name of the intrinsic function. BUG= https://code.google.com/p/nativeclient/issues/detail?id=4326 R=stichnot@chromium.org Review URL: https://codereview.chromium.org/1359993002 .
-
Karl Schimpf authored
The bitcode parser assumes that the module symbol table, if it exists, must appear before the first function block (i.e. defininition). However, the parser did not check this constraint. This patch fixes that omission. BUG= https://code.google.com/p/nativeclient/issues/detail?id=4320 R=stichnot@chromium.org Review URL: https://codereview.chromium.org/1359923002 .
-
Karl Schimpf authored
Previous code checked, after the fact, that there weren't multiple modules in the block. However, the code for TopLevelParser assumes that there is only on module block, allowing it to more efficiently test internal state. To fix the problem, modified TopLevelParser::ParseBlock to fail, and not attempt to parse the second block if the file contains more than one block. BUG= https://code.google.com/p/nativeclient/issues/detail?id=4260 R=stichnot@chromium.org Review URL: https://codereview.chromium.org/1362643002 .
-
David Sehr authored
Hopefully improves perf in fpclassifyd in ammp spec test. BUG=none R=stichnot@chromium.org Review URL: https://codereview.chromium.org/1351133003 .
-
- 21 Sep, 2015 1 commit
-
-
Karl Schimpf authored
Fixes bug where pnacl-sz was not checking if the inserted type matched the element type of the corresponding vector. Also fixes operand look up (within a function) to error recover rather than always die with a fatal error. BUG= https://code.google.com/p/nativeclient/issues/detail?id=4313 R=stichnot@chromium.org Review URL: https://codereview.chromium.org/1357273004 .
-
- 18 Sep, 2015 6 commits
-
-
John Porto authored
BUG= https://code.google.com/p/nativeclient/issues/detail?id=4076 R=stichnot@chromium.org Review URL: https://codereview.chromium.org/1356763004 .
-
Jim Stichnoth authored
Defer hi/lo i64 var splitting until after the block profiling IR is inserted, since block profiling operates on i64 counters. BUG= none R=jpp@chromium.org Review URL: https://codereview.chromium.org/1357733004 .
-
Karl Schimpf authored
Checks each parameter of a call against the corresponding function signature. It also checks that parameters are valid PNaCl parameter types, unless the call is to an intrinsic. BUG= https://code.google.com/p/nativeclient/issues/detail?id=4309 R=stichnot@chromium.org Review URL: https://codereview.chromium.org/1354673002 .
-
John Porto authored
BUG= https://code.google.com/p/nativeclient/issues/detail?id=4076 R=stichnot@chromium.org Review URL: https://codereview.chromium.org/1347223003 .
-
Jim Stichnoth authored
1. Regalloc dump output was displaying status updates for the wrong variable in some cases. 2. getPhysicalRegister() creates a variable for referring to a specific physical register for low-level purposes, such as the stack pointer, or the frame pointer, or a pushed/popped callee-save register. We change its behavior so that all such physical registers do not have their liveness tracked/validated, not just the stack pointer. For #2, the original behavior was causing a liveness validation failure if a function had a single basic block and used callee-save registers, and the -asm-verbose flag was used. This is because -asm-verbose runs a final liveness pass after the prolog/epilog are generated, and the initial callee-save register pushes would make it look like single-basic-block variables are live coming into a basic block, which is a hallmark of a liveness problem. BUG= none R=jpp@chromium.org Review URL: https://codereview.chromium.org/1353923004 .
-
Andrew Scull authored
BUG= R=stichnot@chromium.org Review URL: https://codereview.chromium.org/1326013002 .
-
- 17 Sep, 2015 2 commits
-
-
Karl Schimpf authored
Fix handling of module value symbol tables. Also deals with case where the module doesn't have a global variables block. BUG= https://code.google.com/p/nativeclient/issues/detail?id=4320 R=stichnot@chromium.org Review URL: https://codereview.chromium.org/1345293002 .
-
Andrew Scull authored
This doesn't make a big difference but does reduce the proportion of time spent in malloc and free. BUG= R=stichnot@chromium.org Review URL: https://codereview.chromium.org/1349833005 .
-
- 16 Sep, 2015 7 commits
-
-
John Porto authored
Packs VFP arguments as tight as the ABI wants, and adds tests for float and double arguments. vector argument tests will come soon. BUG= https://code.google.com/p/nativeclient/issues/detail?id=4076 R=stichnot@chromium.org Review URL: https://codereview.chromium.org/1348393002 .
-
Karl Schimpf authored
Makes sure that names within a symbol table are unique. Also cleans up error reporting to be more consistent between symbol tables. BUG= https://code.google.com/p/nativeclient/issues/detail?id=4301 R=stichnot@chromium.org Review URL: https://codereview.chromium.org/1347683003 .
-
Karl Schimpf authored
BUG= https://code.google.com/p/nativeclient/issues/detail?id=4302 R=stichnot@chromium.org Review URL: https://codereview.chromium.org/1346723002 .
-
Andrew Scull authored
BUG= R=stichnot@chromium.org Review URL: https://codereview.chromium.org/1341423002 .
-
Jim Stichnoth authored
At the end, the ELF emitter code marks all the external symbols that are undefined in this object file, but it filters intrinsics out of this set. Previously, it was asserting on a "bad intrinsic", meaning a function whose name starts with "llvm." but doesn't match a known intrinsic. The assert doesn't really make sense here. Either it should be more strongly validated early on in the translation, or it should just pass through (and the link will probably fail, or maybe not). BUG= https://code.google.com/p/nativeclient/issues/detail?id=4319 R=kschimpf@google.com Review URL: https://codereview.chromium.org/1343283002 .
-
Jim Stichnoth authored
It checks that each phi label corresponds to an incoming edge, and that each incoming edge has a corresponding phi label. It does not check that duplicate incoming edges get the same phi value. Performance impact is minimal (~0.2%) despite the O(N^2) implementation. BUG= https://code.google.com/p/nativeclient/issues/detail?id=4304 R=jpp@chromium.org Review URL: https://codereview.chromium.org/1351433002 .
-
Jim Stichnoth authored
X86 requires an immediate shift value to fit within 8 bits. It's undefined LLVM behavior if it doesn't (or in fact if the value exceeds the number of bits in the type width), but at least we can produce valid x86 instructions. BUG= https://code.google.com/p/nativeclient/issues/detail?id=4316 R=ascull@google.com Review URL: https://codereview.chromium.org/1339603006 .
-
- 15 Sep, 2015 1 commit
-
-
Jim Stichnoth authored
The idea is that, before each load or store operation, we add a couple of compares/branches against the load/store address, one for the lower bound and one for the upper bound. The conditional branches would be to an error throwing routine, and would never be taken in practice. The compares might be against an immediate or a global location. So a load of [reg] will mock-expand to this: cmp reg, 0 je label cmp reg, 1 je label label: mov xxx, [reg] We also make address mode inference less aggressive, because for a load of e.g. [eax+4*ecx], we can't compare that address expression against anything in any instruction, so we would have to reconstruct the address and undo at least part of the address mode inference. The bounds-check mock is added for loads, stores, and rmw operations (with an exclusion for stores to the stack for out-arg pushes). There are probably a small handful of other cases that are missing the bounds check, but if we add the transformation inside legalize(), which is the most obvious place, we may add extra bounds checks because sometimes legalize() is called twice on the same operand. BUG= none R=ascull@google.com Review URL: https://codereview.chromium.org/1338633005 .
-