Failing to run benchmark #1
Comments
|
The stack trace is the following: #0 0x000000001067b9c4 in HPHP::string_number_format(double, int, HPHP::String const&, HPHP::String const&) () |
|
Hi @leitao! We have identified the problem and we just sent the fix. The problem happens on a function cast for different type of data, at hphp/runtime/vm/native-func-caller.h:839. On that line there is a function call cast objects to int64_t, what is not working for PPC64. We are still analysing this issue, but for now, the last couple of commits solves the problem. Fell free to check it. Thanks! |
|
Nice, let me try! On Mon, Jun 29, 2015 at 4:24 PM, lbianc notifications@github.com wrote:
|
…imezone to UTC
Summary: Without this change, the following code:
c_dt = HPHP::Unit::lookupClass(HPHP::s_DateTime.get());
assert(c_dt);
HPHP::ObjectData* obj = HPHP::ObjectData::newInstance(c_dt);
DateTimeData* data = Native::data<DateTimeData>(obj);
data->m_dt = makeSmartPtr<DateTime>(0, false);
data->m_dt->fromTimeStamp(milliseconds / 1000, true);
Would cause issues when var_dumping the returned object. var_dump would call
debugInfo on the object, which in turns tries to use zone_type_to_string, which
has an assertion if it can't find the type. With the above example, the zone
type would be false, and we'd get this assertion:
Program received signal SIGABRT, Aborted.
0x00007fffeeae5107 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
56 ../nptl/sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0 0x00007fffeeae5107 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
#1 0x00007fffeeae64e
Closes facebook#5547
Reviewed By: @sgolemon
Differential Revision: D2197285
Summary: In a switch statement with a fallthrough we would use the same environemnt
for the next case we encounter. However this is incorrect. Consider the
following case.
```
$x = 0; // int
switch (0) {
case 1:
$x = '';
// FALLTHROUGH
case 2:
$x; // <-- #1
}
$x; // <-- #2
```
Currently at position #1 we will think $x is a string, since that is what we
assigned $x to be in the above case. However we could have switched directly
into case 2 without a fallthrough at which point $x would be an int.
This bug also impacts the result outside the switch statement. Because we
believe $x is of type string in both branches we believe $x is a string at
position #2 as well.
The solution is to intersect the local environment before and after type
checking the block in case 1. This intersected local environment is what should
be passed as the environment for case 2.
Reviewed By: @jwatzman, @int3
Differential Revision: D2230208
Summary: 1. Avoid putting logic into the entry point; it should just be a thin wrapper around the underlying function 2. Once #1 is done, there's no need to pass the entry points around as arguments 3. Also avoid passing around log_mode as a separate arg; it can be computed easily from the options record Reviewed By: achow Differential Revision: D2722256 fb-gh-sync-id: 4655f7049c828a605c1a50c0026e5fd42aca971f
Summary:
This diff models typing for optional shape fields.
= Model
These fields are modeled as follows:
```name=typing_defs.ml,lang=ocaml
'phase shape_field_type = {
sft_optional : bool;
sft_ty : 'phase ty;
}
```
And integrated into the existing type structure as follows:
```name=typing_defs.ml,lang=diff
ty_ = (* ...existing types... *)
| Tshape :
shape_fields_known *
- ('phase ty Nast.ShapeMap.t)
+ ('phase shape_field_type Nast.ShapeMap.t)
-> 'phase ty_
```
= Justification
There were two reasonable ways of modeling optional shape fields:
# `Tshape` remains unchanged, and a new toplevel type `Toptional_shape_field` is introduced.
# `Tshape` is changed to hold `shape_field_type`s that explicitly denote whether they are optional or not.
I have opted for #2.
Choice #1 has these trade-offs:
# (good) The shape handling code remains unchanged.
# (ok/good) `Tshape` is symmetric in a sense, and does not need additional handling logic. Handling logic can be placed closer to where toplevel processing for `ty` appears.
# (bad) **Everywhere** that a `ty` is processed, a new case has to be added to handle the optional shape field. In most of these cases, optional shape fields are not even logically possible!
# (extremely bad) There is special casing logic that needs to be written for optional shape fields, and this logic must be done **in the context of a `Tshape`**. This design would break the abstraction, forcing us to push through `Tshape` information to the toplevel processing for `ty`.
Choice #2 has these trade-offs:
# (good) An `shape_field_type` may //only// appear in the context of a `Tshape`. We will never process an `shape_field_type` in a place where it's not logically possible. Consequently, the processing always has visibility to the `Tshape` of which the `shape_field_type` is a part of.
# (good) This design forces the developer to handle processing for `shape_field_type`s when processing a `Tshape`. It will be very hard to "forget" to handle this case, since it will be enforced by the type system.
# (bad) It turns out there are lots of places that don't care about whether a type is optional or not. These points in the code now have to be filled with boilerplate to unwrap the `ty` from the `shape_field_type`.
= Conclusion
Whereas #1 has serious logic and maintainability concerns, #2 really just has concerns related to code cleanliness. Additionally, to handle those code cleanliness concerns, I created the `typing_helpers` library to handle patterns that occurred repeatedly in this diff.
= Other notes
There are lots of locations in this diff where I'm not completely sure what's going on. Please let me know if there are any areas that look suspect. If things do in fact look off, it might be useful for us to go through some parts of the diff in person. Since the existing typecheck tests and the new ones for the optional shape fields all pass, I'm reasonably confident that there's not a regression, but I can't be sure.
Reviewed By: dlreeves
Differential Revision: D4563246
fbshipit-source-id: da8d446429351bf804c0485335c29ab83fd049da
Summary: If a branching instruction has the same block as the next and taken target, load-elim will always choose the next state to merge into the successor. This can cause problems if the instruction alters the tracked state and we later simplify away the branch instruction. Here's a somewhat constrived example to illustrate the problem: ``` B1: (1) t1:Str|InitNull = LdLoc<Str|InitNull,1> (2) t2:Str = CheckType<Str> t1:Str|InitNull B2, B2 B2: (3) t3:Str|InitNull = LdLoc<Str|InitNull,1> (4) DecRef t3:Str|InitNull ``` Load-elim will deduce at (2) that a load from local slot #1 can be substituted with t2 (along the next branch). When it processes (3), it will choose the next branch's state to merge. So, it will rewrite (3) to t2 and copy-propagate. Obtaining: ``` B1: (1) t1:Str|InitNull = LdLoc<Str|InitNull,1> (2) t2:Str = CheckType<Str> t1:Str|InitNull B2, B2 B2: (4) DecRef t2:Str ``` However, the simplifier and DCE now kicks in and optimizes away (2) because both of its branches jump to the same block, leaving us with just: ``` B1: (4) DecRef t2:Str ``` IE, we're dec-reffing a tmp that no longer has a definition. The fix for all this is use the taken branch's state when an instruction has the same block for next and taken. In the above example, the taken branch's state won't include the subsitution rule, so we won't copy propagate t2 before eliminating it. Reviewed By: markw65 Differential Revision: D4870758 fbshipit-source-id: 12f3311b63c1be58cb3ba36dd62228c95446ef76
Summary: Reported by UBSAN: ```lang=bash 001+ hphp/runtime/debugger/debugger_client.cpp:2515:7: runtime error: load of value 190, which is not a valid value for type 'bool' 002+ #0 0xa04d7ed in HPHP::Eval::DebuggerClient::saveConfig() hphp/runtime/debugger/debugger_client.cpp:2515 003+ #1 0xa02b046 in HPHP::Eval::DebuggerClient::setDebuggerClientSmallStep(bool const&) hphp/runtime/debugger/debugger_client.h:352 004+ #2 0xa02b046 in HPHP::Eval::DebuggerClient::loadConfig() hphp/runtime/debugger/debugger_client.cpp:2391 005+ #3 0xa02995f in HPHP::Eval::DebuggerClient::init(HPHP::Eval::DebuggerClientOptions const&) hphp/runtime/debugger/debugger_client.cpp:750 006+ #4 0xa015d01 in HPHP::Eval::DebuggerClient::start(HPHP::Eval::DebuggerClientOptions const&) hphp/runtime/debugger/debugger_client.cpp:783 007+ #5 0xa015d01 in HPHP::Eval::DebuggerClient::Start(HPHP::Eval::DebuggerClientOptions const&) hphp/runtime/debugger/debugger_client.cpp:399 008+ #6 0x9fe833b in HPHP::Eval::Debugger::StartClient(HPHP::Eval::DebuggerClientOptions const&) hphp/runtime/debugger/debugger.cpp:56 009+ #7 0x884b41f in HPHP::execute_program_impl(int, char**) hphp/runtime/base/program-functions.cpp:1929 010+ #8 0x883d586 in HPHP::execute_program(int, char**) hphp/runtime/base/program-functions.cpp:1200 011+ #9 0x72782e in main hphp/hhvm/main.cpp:85 012+ #10 0x7ffbeed1c857 in __libc_start_main /home/engshare/third-party2/glibc/2.23/src/glibc-2.23/csu/../csu/libc-start.c:289 013+ #11 0x724e28 in _start /home/engshare/third-party2/glibc/2.23/src/glibc-2.23/csu/../sysdeps/x86_64/start.S:118 015+ SUMMARY: UndefinedBehaviorSanitizer: invalid-bool-load hphp/runtime/debugger/debugger_client.cpp:2515:7 in ``` Reviewed By: markw65 Differential Revision: D5898634 fbshipit-source-id: 102f90ead9766b381a4dc722800ad6e7fb8a76be
Currently hhvm for ppc64le (interpreted mode) is segfaulting to run the following benchmark:
http://www.php-benchmark-script.com/bench.zip
With the following error:
The text was updated successfully, but these errors were encountered: