Skip to content

[vm/ffi] Pointer optimizations broke Arm32 AoT #38737

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
dcharkes opened this issue Oct 7, 2019 · 2 comments
Closed

[vm/ffi] Pointer optimizations broke Arm32 AoT #38737

dcharkes opened this issue Oct 7, 2019 · 2 comments
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. crash Process exits with SIGSEGV, SIGABRT, etc. An unhandled exception is not a crash. library-ffi

Comments

@dcharkes
Copy link
Contributor

dcharkes commented Oct 7, 2019

The pointer optimizations broke the FFI in arm32 precompiled.

Quick investigation: We're trying to convert kUnboxedUint32 and kUnboxedInt64 to kUntagged. But as this is 32 bit, we should only convert kUnboxedUint32 to untagged.

I'm unsure why the representation is different in precompiled.

(Note we do not have ia32 precompiled, so this can only be debugged on the phone.)

tools/test.py -n dartkp-android-debug-arm ffi/data_test
Test configuration:
    dartkp-android-debug-arm(architecture: arm, compiler: dartkp, mode: debug, runtime: dart_precompiled, system: android)
Suites tested: ffi
Found 1 Android devices.

FAILED: dartkp-dart_precompiled debug_arm ffi/data_test
Expected: Pass Slow
Actual: Crash
Unexpected compile error.

--- Command "vm_compile_to_kernel []" (took 04.000661s):
DART_CONFIGURATION=DebugAndroidARM /usr/local/google/home/dacoharkes/dart-sdk/sdk/pkg/vm/tool/gen_kernel --aot --platform=out/DebugAndroidARM/vm_platform_strong.dill -o /usr/local/google/home/dacoharkes/dart-sdk/sdk/out/DebugAndroidARM/generated_compilations/dartkp/tests_ffi_data_test/out.dill /usr/local/google/home/dacoharkes/dart-sdk/sdk/tests/ffi/data_test.dart --packages=/usr/local/google/home/dacoharkes/dart-sdk/sdk/.packages -Ddart.developer.causal_async_stacks=true

exit code:
0

--- Command "precompiler" (took 03.000972s):
DART_CONFIGURATION=DebugAndroidARM out/DebugAndroidARM/clang_x86/gen_snapshot --snapshot-kind=app-aot-elf --elf=/usr/local/google/home/dacoharkes/dart-sdk/sdk/out/DebugAndroidARM/generated_compilations/dartkp/tests_ffi_data_test/out.aotsnapshot --no-sim-use-hardfp --ignore-unrecognized-flags --packages=/usr/local/google/home/dacoharkes/dart-sdk/sdk/.packages /usr/local/google/home/dacoharkes/dart-sdk/sdk/out/DebugAndroidARM/generated_compilations/dartkp/tests_ffi_data_test/out.dill

exit code:
-6

stderr:
../../runtime/vm/compiler/backend/il.h: 7996: error: expected: to != kUntagged || from == kUnboxedIntPtr
version=2.6.0-edge.db352304ecd7823a69491fa2677d963c6fbee922 (Mon Oct 7 10:35:45 2019 +0000) on "linux_simarm"
thread=77719, isolate=isolate(0x2048e20)
  pc 0x00ab31ac fp 0xffe09b48 dart::Profiler::DumpStackTrace(void*)
  pc 0x01000114 fp 0xffe09b68 Dart_DumpNativeStackTrace
  pc 0x0083e67b fp 0xffe09b98 dart::Assert::Fail(char const*, ...)
  pc 0x00beaf83 fp 0xffe09bd8 out/DebugAndroidARM/clang_x86/gen_snapshot+0x7eaf83
  pc 0x00c11a81 fp 0xffe09c38 dart::IntConverterInstr::Canonicalize(dart::FlowGraph*)
  pc 0x00bec7fd fp 0xffe09c88 dart::FlowGraph::Canonicalize()
  pc 0x00cccdd8 fp 0xffe09ca8 out/DebugAndroidARM/clang_x86/gen_snapshot+0x8ccdd8
  pc 0x00ccc385 fp 0xffe09d38 dart::CompilerPass::Run(dart::CompilerPassState*) const
  pc 0x00ccc2e4 fp 0xffe09dc8 dart::CompilerPass::Run(dart::CompilerPassState*) const
  pc 0x00ccc926 fp 0xffe09de8 dart::CompilerPass::RunPipeline(dart::CompilerPass::PipelineMode, dart::CompilerPassState*)
  pc 0x00b94650 fp 0xffe0a178 dart::PrecompileParsedFunctionHelper::Compile(dart::CompilationPipeline*)
  pc 0x00b968f7 fp 0xffe0a2f8 out/DebugAndroidARM/clang_x86/gen_snapshot+0x7968f7
  pc 0x00b9165d fp 0xffe0a378 dart::Precompiler::CompileFunction(dart::Precompiler*, dart::Thread*, dart::Zone*, dart::Function const&)
  pc 0x00b8ffb6 fp 0xffe0a3c8 dart::Precompiler::ProcessFunction(dart::Function const&)
  pc 0x00b8bd52 fp 0xffe0a3e8 dart::Precompiler::Iterate()
  pc 0x00b89060 fp 0xffe0a4f8 dart::Precompiler::DoCompileAll()
  pc 0x00b88ae3 fp 0xffe0a748 dart::Precompiler::CompileAll()
  pc 0x00ffe58c fp 0xffe0a7c8 Dart_Precompile
  pc 0x0081368c fp 0xffe0a8e8 dart::bin::main(int, char**)
  pc 0x008144f4 fp 0xffe0a918 main
-- End of DumpStackTrace

--- Re-run this test:
python tools/test.py -n dartkp-android-debug-arm ffi/data_test
@dcharkes dcharkes added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. crash Process exits with SIGSEGV, SIGABRT, etc. An unhandled exception is not a crash. library-ffi labels Oct 7, 2019
@dcharkes
Copy link
Contributor Author

dcharkes commented Oct 7, 2019 •
edited
Loading

Edit: this seemed to be a problem specifically to one device.

dart-bot pushed a commit that referenced this issue Oct 7, 2019
@dcharkes
Revert "[vm/ffi] Optimize Pointer operations for statically known types"
0eff6b3 
This reverts commit 3712ed2.

Reason for revert: Breaks Arm32 precompiled.
Issue: #38737

Original change's description:
> [vm/ffi] Optimize Pointer operations for statically known types
> 
> This CL optimizes Pointer operations in hot loops for Pointer<NativeInteger/NativeDouble/Pointer> (not for structs).
> 
> Design: go/dart-ffi-pointers-il
> 
> It provides roughly a 100x speedup for the FfiMemory benchmark. The next 5x speedup is to get rid of allocations due to `load` and `store` not being inlined.
> 
> FFI API is changed to enable optimizations:
> 
> * Disable dynamic invocations of Pointer.load / Pointer.store.
> * Disallow implicit downcast of argument passed to Pointer.store.
> * Stop zeroing out Pointer.address on Pointer.free().
> 
> Issue: #38172
> 
> Related issues:
> Closes: #35902 (Disallowing dynamic invocations of Pointer ops.)
> Closes: #37385 (Function variance checking.)
> 
> Change-Id: I96058d8b5b49052eb6999f084372e6f08b4f6f17
> Cq-Include-Trybots: luci.dart.try:vm-ffi-android-debug-arm-try,vm-ffi-android-debug-arm64-try,app-kernel-linux-debug-x64-try,vm-kernel-linux-debug-ia32-try,vm-dartkb-linux-debug-simarm64-try,vm-kernel-win-debug-x64-try,vm-kernel-win-debug-ia32-try,vm-dartkb-linux-debug-x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-dartkb-linux-release-x64-abi-try,vm-kernel-precomp-android-release-arm64-try
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/117547
> Commit-Queue: Daco Harkes <[email protected]>
> Reviewed-by: Martin Kustermann <[email protected]>

[email protected],[email protected],[email protected]

Change-Id: I3b7923ace45beaa9f99119e9ea20c1e52b429ad8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Issue: #38172
Cq-Include-Trybots: luci.dart.try:vm-ffi-android-debug-arm-try, vm-ffi-android-debug-arm64-try, app-kernel-linux-debug-x64-try, vm-kernel-linux-debug-ia32-try, vm-dartkb-linux-debug-simarm64-try, vm-kernel-win-debug-x64-try, vm-kernel-win-debug-ia32-try, vm-dartkb-linux-debug-x64-try, vm-kernel-precomp-linux-debug-x64-try, vm-dartkb-linux-release-x64-abi-try, vm-kernel-precomp-android-release-arm64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/120582
Reviewed-by: Daco Harkes <[email protected]>
Commit-Queue: Daco Harkes <[email protected]>
@dcharkes
Copy link
Contributor Author

dcharkes commented Oct 9, 2019

Fixed in 15e8c12 and d23c824.

@dcharkes dcharkes closed this as completed Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. crash Process exits with SIGSEGV, SIGABRT, etc. An unhandled exception is not a crash. library-ffi
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@dcharkes