Make forced eager fallback optional in codegen #75274
Conversation
- default to generating forced fallback for TS backend (where it is used for tests/debugging, but false otherwise [ghstack-poisoned]
🔗 Helpful links
💊 CI failures summary and remediationsAs of commit 850d45d (more details on the Dr. CI page):
🕵️ 1 new failure recognized by patternsThe following CI failures do not appear to be due to upstream breakages
|
| Job | Step | Action |
|---|---|---|
 pull / linux-bionic-rocm5.0-py3.7 / test (default, 2, 2, linux.rocm.gpu) |
Set up job | 🔁 rerun |
This comment was automatically generated by Dr. CI (expand for details).
Please report bugs/suggestions to the (internal) Dr. CI Users group.
| per_operator_headers: bool = False, | ||
| backend_name: str = default_args.backend_name) -> None: | ||
| backend_name: str = default_args.backend_name, | ||
| gen_forced_fallback_code: bool = False) -> None: |
There was a problem hiding this comment.
mostly just to make sure i'm following along - doesn't this path only get taken at runtime if an environment variable is set? I'm wondering if there's any reason to make it conditional in the codegen instead of just being conditional on the env variable.
There was a problem hiding this comment.
I agree with you, but Jack was wanting cleaner generated code; I think we could convince him to accept it, but there was one particular issue:
the header ts_eager_fallback.h needed to be included if that code is generated, so using this flag in the codegen lets us skip including that header. If in the future we make the fallback part of the backend-api, we could consider always generating this codepath.
There was a problem hiding this comment.
ok, sounds reasonable. The delta is super small anyway.
|
@wconstab has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
- default to generating forced fallback for TS backend (where it is used for tests/debugging, but false otherwise Differential Revision: [D35411211](https://our.internmc.facebook.com/intern/diff/D35411211) [ghstack-poisoned]
|
@wconstab has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
- default to generating forced fallback for TS backend (where it is used for tests/debugging, but false otherwise Differential Revision: [D35411211](https://our.internmc.facebook.com/intern/diff/D35411211) [ghstack-poisoned]
|
@wconstab has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
Summary: Pull Request resolved: #75274 - default to generating forced fallback for TS backend (where it is used for tests/debugging, but false otherwise Test Plan: Imported from OSS Reviewed By: bdhirsh Differential Revision: D35411211 Pulled By: wconstab fbshipit-source-id: ccff2f65aa5d8e1aa670d210ce51805985df55ce
|
Hey @wconstab. |
wconstab
added
topic: not user facing
Stack from ghstack (oldest at bottom):
for tests/debugging, but false otherwise
Differential Revision: D35411211