Skip to content

Add INT32 support to SUB#3037

Merged
mergify[bot] merged 6 commits intotensorflow:mainfrom
HemanthSai7:sub_int32
Jun 4, 2025
Merged

Add INT32 support to SUB#3037
mergify[bot] merged 6 commits intotensorflow:mainfrom
HemanthSai7:sub_int32

Conversation

@HemanthSai7
Copy link
Copy Markdown
Contributor

  • 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
@ddavis-2015 ddavis-2015 added the ci:full Triggers the comprehensive cross-platform test suite. label Mar 4, 2025
@TFLM-bot TFLM-bot removed the ci:full Triggers the comprehensive cross-platform test suite. label Mar 4, 2025
@ddavis-2015 ddavis-2015 self-requested a review March 4, 2025 08:33
Copy link
Copy Markdown
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 (LRTM) 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
Copy Markdown
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.

Comment thread tensorflow/lite/micro/kernels/sub.cc Outdated

if (output->type == kTfLiteFloat32) {
if (output->type == kTfLiteFloat32 || output->type == kTfLiteInt32) {
EvalSub(context, node, params, &data, input1, input2, output);
Copy link
Copy Markdown
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
Copy Markdown
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.

@HemanthSai7
Copy link
Copy Markdown
Contributor Author

Thank you. I'll make the changes you suggested soon.

Followed the CONTRIBUTING.md to first fetch upstream and then merge into branch
@HemanthSai7
Copy link
Copy Markdown
Contributor Author

@ddavis-2015 I have made the changes and also updated the licensing year.

@ddavis-2015 ddavis-2015 added the ci:full Triggers the comprehensive cross-platform test suite. label Mar 7, 2025
@TFLM-bot TFLM-bot removed the ci:full Triggers the comprehensive cross-platform test suite. label Mar 7, 2025
ElementCount(*output_dims), activation);
}

void TestSubInt32(int* input1_dims_data, const int32_t* input1_data,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You will need #if !defined(XTENSA) around this method also to prevent the unused function warning (which is promoted to an error).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done @ddavis-2015 .

@ddavis-2015 ddavis-2015 added the ci:full Triggers the comprehensive cross-platform test suite. label Mar 8, 2025
@TFLM-bot TFLM-bot removed the ci:full Triggers the comprehensive cross-platform test suite. label Mar 8, 2025
Copy link
Copy Markdown
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.

@suleshahid looks good here

@suleshahid
Copy link
Copy Markdown
Collaborator

Thanks for the PR! Sorry for the late review.

@mergify mergify Bot merged commit 686df2d into tensorflow:main Jun 4, 2025
33 checks passed
unmeshna017 pushed a commit to unmeshna017/tflite-micro that referenced this pull request Feb 10, 2026
- Add INT32 support in sub
- Add Tflite tests in sub_test.cc

bug=fixes tensorflow#2720
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