Skip to content

[passes] Consider multiple DQ case#169

Merged
jinevening merged 2 commits into
Samsung:mainfrom
mhs4670go:fix
Jun 24, 2025
Merged

[passes] Consider multiple DQ case#169
jinevening merged 2 commits into
Samsung:mainfrom
mhs4670go:fix

Conversation

@mhs4670go

Copy link
Copy Markdown
Contributor

This commit considers multiple DQ case in SimpleAdd.

The case happens with pt2 quantization.
TICO-DCO-1.0-Signed-off-by: seongwoo mhs4670go@naver.com

This commit considers multiple DQ case in SimpleAdd.

TICO-DCO-1.0-Signed-off-by: seongwoo <mhs4670go@naver.com>
@mhs4670go mhs4670go requested a review from a team June 20, 2025 07:57
Comment on lines +53 to +55
op ─ Q ─┬─ DQ0 op(meta[QPARAM])
├─ DQ1 (each DQ* folded, Q dropped when orphaned)
└─ DQ2

@dayo09 dayo09 Jun 20, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you elaborate this case? How those three different DQs be folded to one op?

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.

All DQs have same qparam. This pattern is from pt2e quantization of torch.

# ───────────────────────────────────────────
else:
op_qparam: QuantParam = op.meta[QPARAM_KEY]
existing_qp: QuantParam = op.meta[QPARAM_KEY]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(optional) To me, op_qparam looks clearer. It's aligned with the comment.

jinevening
jinevening previously approved these changes Jun 23, 2025

@jinevening jinevening left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@dayo09 dayo09 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@jinevening jinevening left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@jinevening jinevening merged commit e9e7c23 into Samsung:main Jun 24, 2025
5 checks passed
@mhs4670go mhs4670go deleted the fix branch June 25, 2025 08:25
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.

3 participants