In one of my pending region compiler diffs, I added code to forget the
values/types of locals after some FCalls (not doing so was wrong but we were
mostly getting away with it). This exposed an issue where the known type of a
local in the IR was less specific that what Translator::analyze was using. The
more specific type came from metadata on instructions after the FCall, so this
diff applies metadat to the IR before translating each instruction, using the
same code I wrote for the region compiler.
The changes to apply the metadata are pretty small; most of the
changes in this diffs are real bugs I found while attempting an
alternate solution.
Some bytecode ops, such as CGetL3, would munch the
types of the things on the stack above the new cell that was
pushed because getStackValue assumes all stack outputs from
interpOne are on top of the stack. The result is that the top
of the stack would contain the output type, and the two things
below it would not get any type (unknown).
This usually isn't a problem, but if we fail enough in type
prediction we might get around to InterpOneing these ops.
This diff removes the Marker opcode, replacing it with a BCMarker
struct in each IRInstruction. This gives us fewer redundant lines in IRTrace
dumps and allows for more straightforward control of which IRInstructions are
associated with which bytecodes. I took this opportunity to do some more
cleanup of ir dumps as well, and it's now possible to interpOne every codegen
punt.
This was going to be easy, but then it wasn't. The existing
interpOne relied heavily on the outputs of NormalizedInstruction, and
those are only populated by code that we want to kill soon. It also
didn't properly deal with bytecodes that wrote to more than one local,
or more than one stack cell. So the bulk of this diff is rewriting
interpOne to not rely on the NI outputs, then it was simple to hook it
into the region translator. I also cleaned things up so we don't
attempt to translate instructions that have no hope of succeeding; we
just interp them on the first try now.
Introducing `ZendParamMode` to as a idl flag. We are not consistent with zend on how they do their params for builtins. We cast to the expected data type. They do some checks, and if the checks don't pass they issue a warning and return (usually) `null`. This diff starts us down that path.
I'm introducing the param and using it in the places where we were emulating the calling convention in the `f_foo` functions. I'm going to follow up with converting as many as I can and then eventually this becomes the default. I also want this to be applied to php files in systemlib.
Many of the conversions are from https://github.com/php/php-src/blob/master/Zend/zend_API.c#L305
More changes for HPHP to help make it clang friendly
~~~hphp/compiler/expression/constant_expression.h
~~~hphp/compiler/expression/function_call.h
rfind returns a size_t/unsigned int
~~~hphp/runtime/base/server/http_protocol.cpp
Switched to std::to_string. Assuming [] was not intended here
~~~hphp/runtime/base/ref_data.h
These fields were accessed in a public manner, assuming public was intended
instead of private
~~~hphp/runtime/base/variable_serializer.cpp
Switched to using [] and & to make clang happy. Assuming this was to either
take or drop the first char.
~~~hphp/runtime/ext/asio/asio_external_thread_event_queue.h
~~~hphp/runtime/ext/asio/asio_external_thread_event_queue.cpp
Cast which performs the conversions of a reinterpret_cast is not
allowed in a constant expression. This is been moved to a macro as a temporary
fix.
+++hphp/runtime/ext/ext_misc.cpp
Added std::atomic to supress warnings
~~~hphp/runtime/vm/jit/simplifier.cpp
Chosen constructor is explicit in copy-initialization
~~~hphp/runtime/vm/jit/translator-asm-helpers.S
Ambiguous instructions require an explicit suffix
Changed cmp to cmpl
~~~hphp/runtime/vm/jit/translator-x64-helpers.cpp
Clang does not support global register variables
+++hphp/runtime/vm/unwind.cpp
describeFault was only used when DEBUG or USE_TRACE was defined
~~~hphp/runtime/vm/verifier/check_unit.cpp
Made fmt pointer const to avoid string format issues/warnings
~~~hphp/util/stack_trace.cpp
Clang does not support variable-length arrays.
Uniqe_ptr is used instead to take advantage runtime-sized arrays, a
restriced form of variable-length arrays
~~~hphp/util/thread_local.h
Clang seems to be supporting the __thread attribute, or at the very least
it is not complaining about it.
~~~hphp/util/tiny_vector.h
Clang does not like the flexible array here, since T is not always POD.
I have reimplemented the array here by just sticking one value in the struct
and calculating the offset from its address manually.
Alterinatively, we could change the the non-POD types to be pointers, or
we could edit their implemenations.
+++hphp/util/util.h
Created a template for the union,
A function declared with the constexpr specifier cannot contain
type declarations that do not define classes or enumerations
+++hphp/runtime/vm/jit/x64-util.h
Added a TODO
The way hphp/runtime/vm/jit/x64-util.h is currently implemented, it only
works if USE_GCC_FAST_TLS is defined
This converter enables much more thorough testing of the
region translator. It currently passes all tests, though it does punt
on one or more Tracelets in roughly 1/6th of them. The two big
unimplemented features for the whole pipeline are interpOne (should be
pretty easy) and parameter reffiness checks (probably
nontrivial). I'll attack those in separate diffs next.
This diff adds a very, very basic region translator. Together with
-vEval.JitRegionSelector=method, it is capable of translating very simple
methods without using a Tracelet. It takes in a RegionDesc struct and creates
NormalizedInstructions on the stack to drive the translate* methods in
irtranslator.cpp. I started trying to get all tests passing with
JitRegionSelector=method (by replacing lots of asserts with punts) but decided
it's not worth it right now.
I'm planning on writing a Tracelet -> RegionDesc converter next and expanding
translateRegion to handle everything that throws at it.
We were being too conservative in determining whether to use
the static property cache when accessing a property from outside the
class. In fact we'd never use it in that case, because we required
that the class not need initialization (and any class with static
properties needs initialization!).
We can relax this restriction to just enforce that the class not
define sinit or pinit methods -- scalar properties are fine.
- smart::make_unique, returns a unique pointer to smart allocated
data.
- Support forwarding in our allocator's construct, so
smart::vector<foo::unique_ptr<>> works.
- Move smart containers to their own header.
Paul Bissonnette