[Inductor] Fallback scatter when src dtype is bf16 #113204

oulgen · 2023-11-07T21:15:08Z

Stack from ghstack (oldest at bottom):

-> [Inductor] Fallback scatter when src dtype is bf16 #113204

basic_gnn_gcn, basic_gnn_gin, basic_gnn_sage now pass

cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @peterbell10 @ipiszy @yf225 @chenyang78 @kadeng @muchulee8 @aakhundov @ColinPeppler

basic_gnn_gcn, basic_gnn_gin, basic_gnn_sage now pass [ghstack-poisoned]

pytorch-bot · 2023-11-07T21:15:11Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/113204

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit e5b540f with merge base ee777a7 ():
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

basic_gnn_gcn, basic_gnn_gin, basic_gnn_sage now pass ghstack-source-id: 884b08008526cdd9038b77fb11e0f52af487dcb9 Pull Request resolved: #113204

oulgen · 2023-11-07T21:16:15Z

torch/_inductor/lowering.py

@@ -3079,6 +3079,8 @@ def scatter_fallback(
    reduce_ty = "add" if fn == "aten.scatter_" else "sum"
    if (
        reduce not in {None, reduce_ty}
+        # tl.atomic_add does not support bf16
+        or src.get_dtype() in {torch.bfloat16}


leaving this as a set since i assume there will need to be more things here

use tuple? for single elem set seems like overkill

eellison

Can you add a fallback for index_put from this issue while we're at it:

see: #97016

add test ?

eellison · 2023-11-07T21:48:20Z

torch/_inductor/lowering.py

@@ -3079,6 +3079,8 @@ def scatter_fallback(
    reduce_ty = "add" if fn == "aten.scatter_" else "sum"
    if (
        reduce not in {None, reduce_ty}
+        # tl.atomic_add does not support bf16
+        or src.get_dtype() in {torch.bfloat16}


use tuple? for single elem set seems like overkill

Chillee

I think we need to guard this on what GPU we're running on? I think tl.atomic_add only doesn't work on A100 GPUs and below.

oulgen · 2023-11-07T22:01:39Z

@Chillee How can i check which gpu i am currently on? or rather how to say A100 or below?

oulgen · 2023-11-07T22:02:37Z

@eellison do you want me to combine both of those tl.atomic_add does not work checks?

eellison · 2023-11-07T22:04:19Z

@Chillee as far as I can tell it's not supported in triton regardless of the device: triton-lang/triton#1387.

@oulgen - yea, I think that makes sense.

basic_gnn_gcn, basic_gnn_gin, basic_gnn_sage now pass cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 aakhundov ColinPeppler [ghstack-poisoned]

basic_gnn_gcn, basic_gnn_gin, basic_gnn_sage now pass ghstack-source-id: e7ff37d21d8bcc0e696641e24522528a67f2fd8a Pull Request resolved: #113204

basic_gnn_gcn, basic_gnn_gin, basic_gnn_sage now pass cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 aakhundov ColinPeppler [ghstack-poisoned]

basic_gnn_gcn, basic_gnn_gin, basic_gnn_sage now pass ghstack-source-id: 46879f885f8cfe15988aeda2abfe246498b30486 Pull Request resolved: #113204

torch/_inductor/lowering.py

basic_gnn_gcn, basic_gnn_gin, basic_gnn_sage now pass cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 aakhundov ColinPeppler [ghstack-poisoned]

basic_gnn_gcn, basic_gnn_gin, basic_gnn_sage now pass ghstack-source-id: 68621a60fc2c0e9918c7be789b91b710268dbf48 Pull Request resolved: #113204

eellison

looks good, thanks!

oulgen · 2023-11-08T20:22:32Z

@pytorchbot merge

peterbell10 · 2023-11-08T20:23:38Z

torch/_inductor/lowering.py

@@ -2926,6 +2926,11 @@ def _unsafe_index_put_(self, indices, values, accumulate=False):
    return index_put_impl_(self, indices, values, accumulate, check=False)


+def needs_fallback_due_to_atomic_add_limitations(dtype):
+    # tl.atomic_add does NOT support the following types
+    return dtype in {torch.int64, torch.bool, torch.bfloat16}


Shouldn't this check the device as well?

Yeah, i guess we should only do the pessimization in cuda mode and not cpu mode.

pytorchmergebot · 2023-11-08T20:25:33Z

Merge failed

Reason: This PR needs a release notes: label
If your changes are user facing and intended to be a part of release notes, please use a label starting with release notes:.

If not, please add the topic: not user facing label.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "topic: not user facing"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

Details for Dev Infra team

Raised by workflow job

jansel · 2023-11-08T21:39:21Z

cc @htyu

basic_gnn_gcn, basic_gnn_gin, basic_gnn_sage now pass cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 aakhundov ColinPeppler [ghstack-poisoned]

basic_gnn_gcn, basic_gnn_gin, basic_gnn_sage now pass ghstack-source-id: ef37bea53dbeadd12241f96b776faa216df79f01 Pull Request resolved: #113204

basic_gnn_gcn, basic_gnn_gin, basic_gnn_sage now pass cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 aakhundov ColinPeppler [ghstack-poisoned]

basic_gnn_gcn, basic_gnn_gin, basic_gnn_sage now pass ghstack-source-id: ca07bd6e82a81bc327e0c26216e6841c8a96987b Pull Request resolved: #113204

oulgen · 2023-11-09T02:37:16Z

@pytorchbot merge

pytorchmergebot · 2023-11-09T02:39:06Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

…#113204)" Summary: Revert due to Llama 7b performance regression on Mi250x (83tok/s -> 79.5tok/s, ~4% regression) Test Plan: CI Differential Revision: D51287379

…#113204)" (pytorch#113599) Summary: Revert due to Llama 7b performance regression on Mi250x (83tok/s -> 79.5tok/s, ~4% regression) Test Plan: CI Differential Revision: D51287379

…#113204)" (pytorch#113599) Summary: Revert due to Llama 7b performance regression on Mi250x (83tok/s -> 79.5tok/s, ~4% regression) Test Plan: CI Reviewed By: xw285cornell Differential Revision: D51287379

basic_gnn_gcn, basic_gnn_gin, basic_gnn_sage now pass Pull Request resolved: pytorch#113204 Approved by: https://github.com/eellison

[Inductor] Fallback scatter when src dtype is bf16

326b195

basic_gnn_gcn, basic_gnn_gin, basic_gnn_sage now pass [ghstack-poisoned]

oulgen added a commit that referenced this pull request Nov 7, 2023

[Inductor] Fallback scatter when src dtype is bf16

3d2ec50

basic_gnn_gcn, basic_gnn_gin, basic_gnn_sage now pass ghstack-source-id: 884b08008526cdd9038b77fb11e0f52af487dcb9 Pull Request resolved: #113204

github-actions bot added module: inductor ciflow/inductor labels Nov 7, 2023

oulgen requested review from jansel, Chillee and eellison November 7, 2023 21:15

oulgen added ciflow/trunk Trigger trunk jobs on your pull request topic: bug fixes topic category and removed module: inductor ciflow/inductor labels Nov 7, 2023

oulgen commented Nov 7, 2023

View reviewed changes

oulgen mentioned this pull request Nov 7, 2023

Torchbench inference failures #112883

Open

9 tasks

eellison reviewed Nov 7, 2023

View reviewed changes

Chillee requested changes Nov 7, 2023

View reviewed changes

oulgen added a commit that referenced this pull request Nov 7, 2023

[Inductor] Fallback scatter when src dtype is bf16

7d3b294

basic_gnn_gcn, basic_gnn_gin, basic_gnn_sage now pass ghstack-source-id: e7ff37d21d8bcc0e696641e24522528a67f2fd8a Pull Request resolved: #113204

github-actions bot added module: inductor ciflow/inductor labels Nov 7, 2023

oulgen added a commit that referenced this pull request Nov 7, 2023

[Inductor] Fallback scatter when src dtype is bf16

b77be3e

basic_gnn_gcn, basic_gnn_gin, basic_gnn_sage now pass ghstack-source-id: 46879f885f8cfe15988aeda2abfe246498b30486 Pull Request resolved: #113204

oulgen requested review from Chillee and eellison November 7, 2023 23:04

eellison reviewed Nov 7, 2023

View reviewed changes

torch/_inductor/lowering.py Show resolved Hide resolved

oulgen added a commit that referenced this pull request Nov 8, 2023

[Inductor] Fallback scatter when src dtype is bf16

71c3c10

basic_gnn_gcn, basic_gnn_gin, basic_gnn_sage now pass ghstack-source-id: 68621a60fc2c0e9918c7be789b91b710268dbf48 Pull Request resolved: #113204

oulgen requested a review from eellison November 8, 2023 19:26

eellison approved these changes Nov 8, 2023

View reviewed changes

peterbell10 reviewed Nov 8, 2023

View reviewed changes

pytorchmergebot added the merging label Nov 8, 2023

pytorchmergebot removed the merging label Nov 8, 2023

oulgen added a commit that referenced this pull request Nov 8, 2023

[Inductor] Fallback scatter when src dtype is bf16

73d1c87

basic_gnn_gcn, basic_gnn_gin, basic_gnn_sage now pass ghstack-source-id: ef37bea53dbeadd12241f96b776faa216df79f01 Pull Request resolved: #113204

oulgen added the topic: not user facing topic category label Nov 8, 2023

oulgen added a commit that referenced this pull request Nov 9, 2023

[Inductor] Fallback scatter when src dtype is bf16

e4ac291

basic_gnn_gcn, basic_gnn_gin, basic_gnn_sage now pass ghstack-source-id: ca07bd6e82a81bc327e0c26216e6841c8a96987b Pull Request resolved: #113204

pytorchmergebot added the merging label Nov 9, 2023

pytorchmergebot added Merged and removed merging labels Nov 9, 2023

pytorchmergebot closed this in fbf7866 Nov 9, 2023

facebook-github-bot deleted the gh/oulgen/30/head branch November 12, 2023 15:24

nmacchioni mentioned this pull request Nov 14, 2023

Back out "[Inductor] Fallback scatter when src dtype is bf16 (#113204)" #113599

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Inductor] Fallback scatter when src dtype is bf16 #113204

[Inductor] Fallback scatter when src dtype is bf16 #113204

oulgen commented Nov 7, 2023 •

edited

pytorch-bot bot commented Nov 7, 2023 •

edited

oulgen Nov 7, 2023

eellison Nov 7, 2023

eellison left a comment •

edited

eellison Nov 7, 2023

Chillee left a comment

oulgen commented Nov 7, 2023

oulgen commented Nov 7, 2023

eellison commented Nov 7, 2023 •

edited

eellison left a comment

oulgen commented Nov 8, 2023

peterbell10 Nov 8, 2023

oulgen Nov 8, 2023

pytorchmergebot commented Nov 8, 2023

jansel commented Nov 8, 2023

oulgen commented Nov 9, 2023

pytorchmergebot commented Nov 9, 2023

[Inductor] Fallback scatter when src dtype is bf16 #113204

[Inductor] Fallback scatter when src dtype is bf16 #113204

Conversation

oulgen commented Nov 7, 2023 • edited

pytorch-bot bot commented Nov 7, 2023 • edited

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/113204

✅ No Failures

oulgen Nov 7, 2023

Choose a reason for hiding this comment

eellison Nov 7, 2023

Choose a reason for hiding this comment

eellison left a comment • edited

Choose a reason for hiding this comment

eellison Nov 7, 2023

Choose a reason for hiding this comment

Chillee left a comment

Choose a reason for hiding this comment

oulgen commented Nov 7, 2023

oulgen commented Nov 7, 2023

eellison commented Nov 7, 2023 • edited

eellison left a comment

Choose a reason for hiding this comment

oulgen commented Nov 8, 2023

peterbell10 Nov 8, 2023

Choose a reason for hiding this comment

oulgen Nov 8, 2023

Choose a reason for hiding this comment

pytorchmergebot commented Nov 8, 2023

Merge failed

jansel commented Nov 8, 2023

oulgen commented Nov 9, 2023

pytorchmergebot commented Nov 9, 2023

Merge started

oulgen commented Nov 7, 2023 •

edited

pytorch-bot bot commented Nov 7, 2023 •

edited

eellison left a comment •

edited

eellison commented Nov 7, 2023 •

edited