Skip to content

Add support for quantized LeakyReLU - #1

Closed
digantdesai wants to merge 1 commit into
pytorch:mainfrom
digantdesai:export-D47043207
Closed

Add support for quantized LeakyReLU#1
digantdesai wants to merge 1 commit into
pytorch:mainfrom
digantdesai:export-D47043207

Conversation

@digantdesai

Copy link
Copy Markdown
Contributor

Summary: Also adds support for backend_config

Reviewed By: mcr229

Differential Revision: D47043207

Summary: Also adds support for backend_config

Reviewed By: mcr229

Differential Revision: D47043207

fbshipit-source-id: 509bd4c02eb7ff5d3d47762522debd827bee7240
@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported labels Jun 28, 2023
@facebook-github-bot

Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D47043207

@facebook-github-bot

Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D47043207

@facebook-github-bot

Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D47043207

digantdesai added a commit to digantdesai/pytorch that referenced this pull request Jun 30, 2023
Summary:
Pull Request resolved: pytorch#104309

X-link: pytorch/executorch#1

Also adds support for backend_config

Test Plan: `buck test fbcode//mode/dev-nosan fbcode//executorch/backends/xnnpack/test:`

Reviewed By: mcr229

Differential Revision: D47043207

fbshipit-source-id: 3e2f7b614713ae5c3fba6ea3056376f15826de17
facebook-github-bot pushed a commit that referenced this pull request Jun 30, 2023
Summary:
X-link: pytorch/pytorch#104309

Pull Request resolved: #1

Also adds support for backend_config

Reviewed By: mcr229

Differential Revision: D47043207

fbshipit-source-id: 51abd266bba7441c28578f6c58686a3d021d9d2a
junpi3 pushed a commit that referenced this pull request Mar 11, 2024
Before: Each node contains a `UniformParamsBuffer`.
After: Each node contains a `std::vector<std::shared_ptr<UniformParamsBuffer>>`.

In follow up changes, we will break up parameters to be passed via multiple UniformParamsBuffer, since
1. some are tensor-specific (e.g. image extents) and
2. others are operator-specific (e.g. alpha for binary ops).

Hence, we need **`std::vector`**.

We are adding the methods for #1 in #2340. Since #1 and #2 will be owned by different objects, we need **pointers**. Since #1 is owned by `vTensor` which is non-copyable, we can't use unique_ptr so we need **`std::shared_ptr`**.

Differential Revision: [D54691831](https://our.internmc.facebook.com/intern/diff/D54691831/)

[ghstack-poisoned]
junpi3 pushed a commit that referenced this pull request Mar 11, 2024
Before: Each node contains a `UniformParamsBuffer`.
After: Each node contains a `std::vector<std::shared_ptr<UniformParamsBuffer>>`.

In follow up changes, we will break up parameters to be passed via multiple UniformParamsBuffer, since
1. some are tensor-specific (e.g. image extents) and
2. others are operator-specific (e.g. alpha for binary ops).

Hence, we need **`std::vector`**.

We are adding the methods for #1 in #2340. Since #1 and #2 will be owned by different objects, we need **pointers**. Since #1 is owned by `vTensor` which is non-copyable, we can't use unique_ptr so we need **`std::shared_ptr`**.

Differential Revision: [D54691831](https://our.internmc.facebook.com/intern/diff/D54691831/)

ghstack-source-id: 218195447
Pull Request resolved: #2348
facebook-github-bot pushed a commit that referenced this pull request Mar 11, 2024
Summary:
bypass-github-export-checks

Pull Request resolved: #2348

Before: Each node contains a `UniformParamsBuffer`.
After: Each node contains a `std::vector<std::shared_ptr<UniformParamsBuffer>>`.

In follow up changes, we will break up parameters to be passed via multiple UniformParamsBuffer, since
1. some are tensor-specific (e.g. image extents) and
2. others are operator-specific (e.g. alpha for binary ops).

Hence, we need **`std::vector`**.

We are adding the methods for #1 in #2340. Since #1 and #2 will be owned by different objects, we need **pointers**. Since #1 is owned by `vTensor` which is non-copyable, we can't use unique_ptr so we need **`std::shared_ptr`**.
ghstack-source-id: 218195447
exported-using-ghexport

Reviewed By: SS-JIA

Differential Revision: D54691831

fbshipit-source-id: 84ab9f777e247fd56234290ed7f7343b9701c73f
junpi3 pushed a commit that referenced this pull request Mar 13, 2024
In #2271, we already added
- IntList
- DoubleList
- BoolList
- ValueList

to the schema and the runtime's Value class. Their serialization was incomplete missing two components:
1. Receiving a list in `torch.fx.Node.args`.
2. Receiving a non-tensor in `torch.fx.Node`.

This change completes #1.


Also, this change fixes a bug where values type `bool` matches both types `bool` and `int` and hence were being added twice.

If our type support grows more complex, we can consider using our own types similar to the core Executorch runtime: https://github.com/pytorch/executorch/blob/689796499024fc4a133318d707f4c10db73da967/exir/emit/_emitter.py#L158-L166

Differential Revision: [D54708353](https://our.internmc.facebook.com/intern/diff/D54708353/)

[ghstack-poisoned]
facebook-github-bot pushed a commit that referenced this pull request Mar 13, 2024
Summary:
bypass-github-export-checks

Pull Request resolved: #2404

In #2271, we already added
- IntList
- DoubleList
- BoolList
- ValueList

to the schema and the runtime's Value class. Their serialization was incomplete missing two components:
1. Receiving a list in `torch.fx.Node.args`.
2. Receiving a non-tensor in `torch.fx.Node`.

This change completes #1.

Also, this change fixes a bug where values type `bool` matches both types `bool` and `int` and hence were being added twice.

If our type support grows more complex, we can consider using our own types similar to the core Executorch runtime: https://github.com/pytorch/executorch/blob/689796499024fc4a133318d707f4c10db73da967/exir/emit/_emitter.py#L158-L166
ghstack-source-id: 218539049
exported-using-ghexport

Reviewed By: SS-JIA

Differential Revision: D54708353

fbshipit-source-id: 8641647b515e201ea63db67115c01c1532ad6566
8Keep added a commit to 8Keep/executorch that referenced this pull request May 29, 2024
Reviewed By: itamaro

Differential Revision: D51566750
8Keep added a commit to 8Keep/executorch that referenced this pull request May 29, 2024
Summary: Pull Request resolved: pytorch#3763

Reviewed By: itamaro

Differential Revision: D51566750
facebook-github-bot pushed a commit that referenced this pull request May 29, 2024
Summary: Pull Request resolved: #3763

Reviewed By: itamaro, tarun292

Differential Revision: D51566750

fbshipit-source-id: 654c426d479833867e93083e9b55786abfc24a32
haowhsu-quic referenced this pull request in CodeLinaro/executorch Jul 16, 2024
haowhsu-quic referenced this pull request in CodeLinaro/executorch Jul 17, 2024
shoumikhin added a commit that referenced this pull request Aug 24, 2026
ETDump records an intermediate tensor by handing the tensor's data pointer to
a data sink, and every sink reads those bytes with a plain host read:

  memcpy(cur_data_begin, ptr, length);

When the tensor lives on an accelerator that pointer is not host memory, so
the read segfaults. A program placed on CUDA crashes as soon as tracing is
turned on, which is exactly when someone is trying to debug it.

Returning an error instead would not help. All four callers wrap the result in
ET_CHECK_MSG, so an error aborts the process rather than skipping the tensor.

This change brings the data back to host memory first. When the tensor is not
on CPU, ETDump looks up the allocator registered for that device type, stages
the bytes into a temporary host buffer with copy_device_to_host, writes that
buffer to the sink and frees it. A tensor on CPU keeps the old path and copies
nothing extra.

If no allocator is registered for the device, ETDump now reports NotFound and
logs the device type instead of reading the pointer anyway.

Test plan:

Two new test files, each with a CMake target and a Buck target.

devtools/etdump/tests/etdump_device_test.cpp registers the existing
MockCudaAllocator, which backs its device memory with host memory, and has two
tests. One logs a tensor tagged as CUDA and checks both that ETDump went
through the allocator and that the bytes reached the debug buffer. The other
logs a tensor on CPU and checks that the allocator was not used at all, so the
CPU path is unchanged.

devtools/etdump/tests/etdump_device_no_allocator_test.cpp covers the case
where nothing is registered for the device. The registry is a process wide
static with no way to remove an entry, so that case needs a binary that never
registers anything, which is why it is a second file.

devtools/etdump/tests/CMakeLists.txt was not referenced by any parent
CMakeLists, so nothing in that directory was built by CMake. This adds
add_subdirectory(tests) to devtools/etdump/CMakeLists.txt under BUILD_TESTING,
so both new tests are picked up by ctest, which is how the C++ tests run.

The pre-existing sdk_etdump_tests target stays out of the CMake build. It
compiles etdump_test.cpp, which includes etdump_filter.h, which needs re2, and
the devtools build does not pull re2 in. It is now guarded on re2 being
available rather than being silently unreachable.

With this change both new binaries pass under ctest:

  1/2 Test #1: etdump_device_test ................   Passed
  2/2 Test #2: etdump_device_no_allocator_test ...   Passed

With etdump_flatcc.cpp reverted to the old code and everything rebuilt, both
fail:

  Expected equality of these values:
    g_mock_cuda.d2h_count_
      Which is: 0
    1

  Death test: etdump_gen.log_evalue(EValue(tensor))
      Result: failed to die.

Also reproduced the real crash on one NVIDIA H100, with a small program that
allocates through cudaMalloc, tags a tensor as CUDA and logs it:

  before: Segmentation fault (core dumped)
  after:  debug buffer holds 1.5 2.5 3.5 4.5

Checked that etdump_flatcc.cpp still compiles with -DUSE_ATEN_LIB.

clang-format reports no changes needed on the four touched C++ files.

Not covered:

The ATen mode branch of the device type conversion only compiles. There is no
ATen mode CMake build to run it in, and an ATen mode tensor in ExecuTorch
carries no device metadata today, so the branch has no caller that can reach
it.
shoumikhin added a commit that referenced this pull request Aug 24, 2026
ETDump records an intermediate tensor by handing the tensor's data pointer to
a data sink, and every sink reads those bytes with a plain host read:

  memcpy(cur_data_begin, ptr, length);

When the tensor lives on an accelerator that pointer is not host memory, so
the read segfaults. A program placed on CUDA crashes as soon as tracing is
turned on, which is exactly when someone is trying to debug it.

Returning an error instead would not help. All four callers wrap the result in
ET_CHECK_MSG, so an error aborts the process rather than skipping the tensor.

This change brings the data back to host memory first. When the tensor is not
on CPU, ETDump looks up the allocator registered for that device type, stages
the bytes into a temporary host buffer with copy_device_to_host, writes that
buffer to the sink and frees it. A tensor on CPU keeps the old path and copies
nothing extra.

If no allocator is registered for the device, ETDump now reports NotFound and
logs the device type instead of reading the pointer anyway.

Test plan:

Two new test files, each with a CMake target and a Buck target.

devtools/etdump/tests/etdump_device_test.cpp registers the existing
MockCudaAllocator, which backs its device memory with host memory, and has two
tests. One logs a tensor tagged as CUDA and checks both that ETDump went
through the allocator and that the bytes reached the debug buffer. The other
logs a tensor on CPU and checks that the allocator was not used at all, so the
CPU path is unchanged.

devtools/etdump/tests/etdump_device_no_allocator_test.cpp covers the case
where nothing is registered for the device. The registry is a process wide
static with no way to remove an entry, so that case needs a binary that never
registers anything, which is why it is a second file.

devtools/etdump/tests/CMakeLists.txt was not referenced by any parent
CMakeLists, so nothing in that directory was built by CMake. This adds
add_subdirectory(tests) to devtools/etdump/CMakeLists.txt under BUILD_TESTING,
so both new tests are picked up by ctest, which is how the C++ tests run.

The pre-existing sdk_etdump_tests target stays out of the CMake build. It
compiles etdump_test.cpp, which includes etdump_filter.h, which needs re2, and
the devtools build does not pull re2 in. It is now guarded on re2 being
available rather than being silently unreachable.

With this change both new binaries pass under ctest:

  1/2 Test #1: etdump_device_test ................   Passed
  2/2 Test #2: etdump_device_no_allocator_test ...   Passed

With etdump_flatcc.cpp reverted to the old code and everything rebuilt, both
fail:

  Expected equality of these values:
    g_mock_cuda.d2h_count_
      Which is: 0
    1

  Death test: etdump_gen.log_evalue(EValue(tensor))
      Result: failed to die.

Also reproduced the real crash on one NVIDIA H100, with a small program that
allocates through cudaMalloc, tags a tensor as CUDA and logs it:

  before: Segmentation fault (core dumped)
  after:  debug buffer holds 1.5 2.5 3.5 4.5

Checked that etdump_flatcc.cpp still compiles with -DUSE_ATEN_LIB.

clang-format reports no changes needed on the four touched C++ files.

Not covered:

The ATen mode branch of the device type conversion only compiles. There is no
ATen mode CMake build to run it in, and an ATen mode tensor in ExecuTorch
carries no device metadata today, so the branch has no caller that can reach
it.
shoumikhin added a commit that referenced this pull request Aug 25, 2026
### Summary

ETDump records an intermediate tensor by handing the tensor's data
pointer to a
data sink, and every sink reads those bytes with a plain host read:

```cpp
memcpy(cur_data_begin, ptr, length);
```

When the tensor lives on an accelerator that pointer is not host memory,
so the
read segfaults. A program placed on CUDA crashes as soon as tracing is
turned
on, which is exactly when someone is trying to debug it.

Returning an error instead would not help. All four callers wrap the
result in
`ET_CHECK_MSG`, so an error aborts the process rather than skipping the
tensor.

This change brings the data back to host memory first. When the tensor
is not on
CPU, ETDump looks up the allocator registered for that device type,
stages the
bytes into a temporary host buffer with `copy_device_to_host`, writes
that buffer
to the sink and frees it. A tensor on CPU keeps the old path and copies
nothing
extra.

If no allocator is registered for the device, ETDump now reports
`NotFound` and
logs the device type instead of reading the pointer anyway.

### Test plan

Two new test files, each with a CMake target and a Buck target.

`devtools/etdump/tests/etdump_device_test.cpp` registers the existing
`MockCudaAllocator`, which backs its device memory with host memory, and
has two
tests. One logs a tensor tagged as CUDA and checks both that ETDump went
through
the allocator and that the bytes reached the debug buffer. The other
logs a
tensor on CPU and checks that the allocator was not used at all, so the
CPU path
is unchanged.

`devtools/etdump/tests/etdump_device_no_allocator_test.cpp` covers the
case where
nothing is registered for the device. The registry is a process wide
static with
no way to remove an entry, so that case needs a binary that never
registers
anything, which is why it is a second file.

`devtools/etdump/tests/CMakeLists.txt` was not referenced by any parent
`CMakeLists.txt`, so nothing in that directory was built by CMake. This
adds
`add_subdirectory(tests)` to `devtools/etdump/CMakeLists.txt` under
`BUILD_TESTING`, so both new tests are picked up by `ctest`, which is
how the C++
tests run.

The pre-existing `sdk_etdump_tests` target stays out of the CMake build.
It
compiles `etdump_test.cpp`, which includes `etdump_filter.h`, which
needs re2,
and the devtools build does not pull re2 in. It is now guarded on re2
being
available rather than being silently unreachable.

With this change both new binaries pass under `ctest`:

```
1/2 Test #1: etdump_device_test ................   Passed
2/2 Test #2: etdump_device_no_allocator_test ...   Passed
```

With `etdump_flatcc.cpp` reverted to the old code and everything
rebuilt, both
fail:

```
Expected equality of these values:
  g_mock_cuda.d2h_count_
    Which is: 0
  1

Death test: etdump_gen.log_evalue(EValue(tensor))
    Result: failed to die.
```

Also reproduced the real crash on one NVIDIA H100, with a small program
that
allocates through `cudaMalloc`, tags a tensor as CUDA and logs it:

```
before: Segmentation fault (core dumped)
after:  debug buffer holds 1.5 2.5 3.5 4.5
```

Checked that `etdump_flatcc.cpp` still compiles with `-DUSE_ATEN_LIB`.

`clang-format` reports no changes needed on the four touched C++ files.

### Landing order

#22058 adds device-planned arenas to the Python bindings that ETDump can
be handed
pointers into, and without this change `BufferDataSink::write` would
`memcpy` device
memory from the host. This should land before or together with it.

### Not covered

The ATen mode branch of the device type conversion only compiles. There
is no
ATen mode CMake build to run it in, so nothing here executes it. It is
reachable
in principle: `runtime/executor/tensor_parser_aten.cpp` reads the
serialized
device type and index, including CUDA, and builds the ATen tensor on
that device.
An earlier version of this description said ATen tensors carry no device
metadata,
which is wrong.

`LogTensorOnCpuDoesNotStageThroughTheAllocator` passes with the
production change
reverted, since CPU tensors already went straight to the data sink. It
documents
the CPU path rather than locking the fix; the other two tests are the
ones that
require the new branch.

---------

Co-authored-by: r <r@e>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants