Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 33 additions & 2 deletions backends/cadence/hifi/operators/op_quantized_linear_out.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <executorch/backends/cadence/hifi/kernels/kernels.h>
#include <executorch/backends/cadence/hifi/operators/operators.h>
#include <executorch/runtime/kernel/kernel_includes.h>
#include <on_device_ai/Assistant/Jarvis/min_runtime/operators/generic/op_quantized_linear.h>
#include <xa_nnlib_kernels_api.h>
#include <xtensa/tie/xt_datacache.h>
#include <algorithm>
Expand Down Expand Up @@ -218,7 +219,22 @@ void quantized_linear_out(
int64_t out_zero_point,
__ET_UNUSED const optional<Tensor>& offset,
Tensor& out) {
if (out.scalar_type() == executorch::aten::ScalarType::Byte) {
if (out.scalar_type() == ::executorch::aten::ScalarType::Short &&
in.scalar_type() == ::executorch::aten::ScalarType::Short &&
weight.scalar_type() == ::executorch::aten::ScalarType::Char) {
::impl::generic::native::quantized_linear_out(
ctx,
in,
weight,
bias,
in_zero_point,
weight_zero_point,
out_multiplier,
out_shift,
out_zero_point,
offset,
out);
} else if (out.scalar_type() == executorch::aten::ScalarType::Byte) {
Comment on lines +222 to +237
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent namespace qualification. Lines 222-224 use fully qualified ::executorch::aten::ScalarType:: while line 237 uses just executorch::aten::ScalarType:: (without leading ::). For consistency and clarity, use the same namespace qualification pattern throughout the function. The fully qualified form (with leading ::) is preferred to avoid potential ambiguity.

Copilot uses AI. Check for mistakes.
_quantized_linear_asym8u(
in,
weight,
Expand Down Expand Up @@ -260,7 +276,22 @@ void quantized_linear_per_tensor_out(
int64_t out_zero_point,
__ET_UNUSED const optional<Tensor>& offset,
Tensor& out) {
if (out.scalar_type() == executorch::aten::ScalarType::Byte) {
if (out.scalar_type() == ::executorch::aten::ScalarType::Short &&
in.scalar_type() == ::executorch::aten::ScalarType::Short &&
weight.scalar_type() == ::executorch::aten::ScalarType::Char) {
::impl::generic::native::quantized_linear_per_tensor_out(
ctx,
in,
weight,
bias,
in_zero_point,
weight_zero_point,
out_multiplier,
out_shift,
out_zero_point,
offset,
out);
} else if (out.scalar_type() == executorch::aten::ScalarType::Byte) {
Comment on lines +279 to +294
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent namespace qualification. Lines 279-281 use fully qualified ::executorch::aten::ScalarType:: while line 294 uses just executorch::aten::ScalarType:: (without leading ::). For consistency and clarity, use the same namespace qualification pattern throughout the function. The fully qualified form (with leading ::) is preferred to avoid potential ambiguity.

Copilot uses AI. Check for mistakes.
_quantized_linear_per_tensor_asym8u(
in,
weight,
Expand Down
5 changes: 4 additions & 1 deletion backends/cadence/hifi/operators/targets.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ OPERATORS = [
"quantized_fully_connected_asym8sxasym8s_asym8s_per_tensor_out",
"quantized_fully_connected_asym8uxasym8u_asym8u_per_tensor_out",
"quantized_layer_norm",
"quantized_linear_out",
"quantized_linear_asym8sxasym8s_asym8s_per_tensor_out",
"quantized_linear_asym8uxasym8u_asym8u_per_tensor_out",
"quantized_matmul_out",
Expand Down Expand Up @@ -122,3 +121,7 @@ def define_common_targets():
# Define build targets for all operators registered in the tables above.
for op in OPERATORS:
define_operator(op)

# quantized_linear_out and quantized_linear_per_tensor_out needs additional dependency for int16 support
define_operator("quantized_linear_out", deps=["fbcode//on_device_ai/Assistant/Jarvis/min_runtime/operators/generic:op_quantized_linear"])
define_operator("quantized_linear_per_tensor_out", deps=["fbcode//on_device_ai/Assistant/Jarvis/min_runtime/operators/generic:op_quantized_linear"])
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The define_operator("quantized_linear_per_tensor_out", ...) call will look for a source file named op_quantized_linear_per_tensor_out.cpp (based on line 27 which uses op_name + ".cpp"), but this file doesn't exist. The quantized_linear_per_tensor_out function is defined in op_quantized_linear_out.cpp along with quantized_linear_out. This will cause a build failure. Consider either:

  1. Removing this line if quantized_linear_per_tensor_out is only exported as part of the quantized_linear_out target, or
  2. Creating a separate op_quantized_linear_per_tensor_out.cpp file if it should be a separate build target.
Suggested change
define_operator("quantized_linear_per_tensor_out", deps=["fbcode//on_device_ai/Assistant/Jarvis/min_runtime/operators/generic:op_quantized_linear"])

Copilot uses AI. Check for mistakes.
132 changes: 132 additions & 0 deletions backends/cadence/hifi/operators/tests/test_op_quantized_linear_out.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
/*
* Copyright (c) Meta Platforms, Inc. and affiliates.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree.
*/

#include <gtest/gtest.h>
#include <sys/times.h>

#include <executorch/kernels/test/TestUtil.h>
#include <executorch/runtime/core/error.h>
#include <executorch/runtime/core/exec_aten/exec_aten.h>
#include <executorch/runtime/core/exec_aten/testing_util/tensor_factory.h>
#include <executorch/runtime/core/exec_aten/testing_util/tensor_util.h>
#include <executorch/runtime/platform/runtime.h>

#include <executorch/backends/cadence/hifi/operators/operators.h>

namespace impl {
namespace HiFi {
namespace native {
namespace {

using ::executorch::aten::Scalar;
using ::executorch::aten::ScalarType;
using ::executorch::aten::Tensor;
using ::executorch::aten::TensorImpl;
using ::executorch::runtime::Error;
using ::executorch::runtime::KernelRuntimeContext;
using ::executorch::runtime::runtime_init;
using ::executorch::runtime::testing::TensorFactory;
using std::optional;
using std::string_view;

class HiFiQuantizedLinearTest : public OperatorTest {
public:
protected:
void quantized_linear_out(
const Tensor& input,
const Tensor& weight,
const Tensor& bias,
int64_t in_zero_point,
const Tensor& weight_zero_point,
const Tensor& out_multiplier,
const Tensor& out_shift,
int64_t out_zero_point,
const optional<Tensor>& offset,
Tensor& output) {
return ::impl::HiFi::native::quantized_linear_out(
context_,
input,
weight,
bias,
in_zero_point,
weight_zero_point,
out_multiplier,
out_shift,
out_zero_point,
offset,
output);
}

void quantized_linear_per_tensor_out(
const Tensor& input,
const Tensor& weight,
const Tensor& bias,
int64_t in_zero_point,
int64_t weight_zero_point,
int64_t out_multiplier,
int64_t out_shift,
int64_t out_zero_point,
const optional<Tensor>& offset,
Tensor& output) {
return ::impl::HiFi::native::quantized_linear_per_tensor_out(
context_,
input,
weight,
bias,
in_zero_point,
weight_zero_point,
out_multiplier,
out_shift,
out_zero_point,
offset,
output);
}
};

// Test quantized_linear_out with int16 activations (asym8s)
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment "Test quantized_linear_out with int16 activations (asym8s)" is confusing. The term "asym8s" typically refers to asymmetric 8-bit signed quantization, but this test is for 16-bit activations with 8-bit weights. Consider updating the comment to clarify the actual quantization scheme, e.g., "Test quantized_linear_out with int16 activations and int8 weights".

Suggested change
// Test quantized_linear_out with int16 activations (asym8s)
// Test quantized_linear_out with int16 activations and int8 weights

Copilot uses AI. Check for mistakes.
TEST_F(HiFiQuantizedLinearTest, QuantizedLinearInt16Test) {
TensorFactory<ScalarType::Short> tf_int16;
TensorFactory<ScalarType::Int> tf_int32;
TensorFactory<ScalarType::Char> tf_int8;

// Simple 2D case: input [2, 3] x weight [4, 3] = output [2, 4]
// Values captured from e2e test with
// CadenceWith16BitLinearActivationsQuantizer
Tensor input =
tf_int16.make({2, 3}, {-28170, -26389, -32768, -31474, -32266, -29076});
Tensor weight = tf_int8.make(
{4, 3}, {1, 87, -128, -114, -59, 44, -1, 127, -12, 44, -46, -29});
Tensor bias = tf_int32.zeros({4});
Tensor output = tf_int16.zeros({2, 4});

int64_t in_zero_point = -29822;
Tensor weight_zero_point = tf_int32.make({1}, {2});
Tensor out_multiplier = tf_int32.make({1}, {2011373824});
Tensor out_shift = tf_int32.make({1}, {-8});
int64_t out_zero_point = -30847;
quantized_linear_out(
input,
weight,
bias,
in_zero_point,
weight_zero_point,
out_multiplier,
out_shift,
out_zero_point,
std::nullopt,
output);
// Expected output from e2e test
Tensor expected_output = tf_int16.make(
{2, 4}, {-28384, -32767, -29144, -30862, -31956, -29486, -31985, -30756});
EXPECT_TENSOR_CLOSE(output, expected_output);
}

} // namespace
} // namespace native
} // namespace HiFi
} // namespace impl
Loading