Skip to content
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.

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

Add INT32 support to SUB #3037

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

HemanthSai7
Copy link

  • Add INT32 support in sub
  • Add Tflite tests in sub_test.cc

bug=fixes #2720

@HemanthSai7 HemanthSai7 requested a review from a team as a code owner January 17, 2025 04:13
Copy link
Member

@ddavis-2015 ddavis-2015 left a comment

Choose a reason for hiding this comment

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

@HemanthSai7 Thank you for your interest in TFLM (RTLM) and for submitting this PR!

Please add the following to SubPrepare in the sub_common.cc file:

  if (output->type == kTfLiteInt32) {
    // Only support INT32 unquantized SUB for now.
    TF_LITE_ENSURE_EQ(context, input1->quantization.type,
                      kTfLiteNoQuantization);
    TF_LITE_ENSURE_EQ(context, input2->quantization.type,
                      kTfLiteNoQuantization);
    TF_LITE_ENSURE_EQ(context, output->quantization.type,
                      kTfLiteNoQuantization);
  }

Comment on lines +243 to +254
TF_LITE_MICRO_TEST(Int32SubNoActivation) {
int inout_shape[] = {4, 1, 2, 2, 1};
const int32_t input1_values[] = {-2, 2147483646, -1, 1146622854};
const int32_t input2_values[] = {3, 1, -2147483647, -726978367};
const int32_t golden_values[] = {-5, 2147483645, 2147483646, 1873601221};
const int kOutputDimsCount = 4;
int32_t output_data[kOutputDimsCount];
tflite::testing::TestSubInt32(inout_shape, input1_values, inout_shape,
input2_values, inout_shape, golden_values,
kTfLiteActNone, output_data);
}

Copy link
Member

Choose a reason for hiding this comment

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

This code will need #if !defined(XTENSA) around it in order to pass the CI tests.

@@ -147,7 +184,7 @@ TfLiteStatus SubEval(TfLiteContext* context, TfLiteNode* node) {
TFLITE_DCHECK(node->user_data != nullptr);
const OpDataSub& data = *(static_cast<const OpDataSub*>(node->user_data));

if (output->type == kTfLiteFloat32) {
if (output->type == kTfLiteFloat32 || output->type == kTfLiteInt32) {
EvalSub(context, node, params, &data, input1, input2, output);
Copy link
Member

@ddavis-2015 ddavis-2015 Mar 4, 2025

Choose a reason for hiding this comment

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

Could you add a TF_LITE_ENSURE_OK check here (line 188). It will make the code more consistent.

@ddavis-2015
Copy link
Member

@HemanthSai7

Also, another Google procedural requirement: All changed files should have the copyright year set the to current year. Only the current year should appear in the copyright.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add INT32 Support to Sub
4 participants