Skip to content

[JIT] add type refinements for isinstance checks#26271

Closed
zdevito wants to merge 12 commits intogh/zdevito/111/basefrom
gh/zdevito/111/head
Closed

[JIT] add type refinements for isinstance checks#26271
zdevito wants to merge 12 commits intogh/zdevito/111/basefrom
gh/zdevito/111/head

Conversation

@zdevito
Copy link
Contributor

@zdevito zdevito commented Sep 16, 2019

Stack from ghstack:

This replaces unchecked_unwrap_optional with unchecked_cast. This
enables the generalization of type refinement so that it works for
isinstance checks as well. This also removes unchecked_unwrap_optional from
code we generate, which is good because it is a hard op to serialize well
since it doesn't directly encode the Optional[T] being unwrapped. In contrast,
unchecked_cast always explicitly lists the type.

Differential Revision: D17412856

[JIT] add type refinements for isinstance checks

This replaces unchecked_unwrap_optional with unchecked_cast. This
enables the generalization of type refinement so that it works for
isinstance checks as well. This also removes unchecked_unwrap_optional from
code we generate, which is good because it is a hard op to serialize well
since it doesn't directly encode the Optional[T] being unwrapped. In contrast,
unchecked_cast always explicitly lists the type.

gh-metadata: pytorch pytorch 26271 gh/zdevito/111/head
[JIT] add type refinements for isinstance checks

This replaces unchecked_unwrap_optional with unchecked_cast. This
enables the generalization of type refinement so that it works for
isinstance checks as well. This also removes unchecked_unwrap_optional from
code we generate, which is good because it is a hard op to serialize well
since it doesn't directly encode the Optional[T] being unwrapped. In contrast,
unchecked_cast always explicitly lists the type.

gh-metadata: pytorch pytorch 26271 gh/zdevito/111/head
[JIT] add type refinements for isinstance checks

This replaces unchecked_unwrap_optional with unchecked_cast. This
enables the generalization of type refinement so that it works for
isinstance checks as well. This also removes unchecked_unwrap_optional from
code we generate, which is good because it is a hard op to serialize well
since it doesn't directly encode the Optional[T] being unwrapped. In contrast,
unchecked_cast always explicitly lists the type.

gh-metadata: pytorch pytorch 26271 gh/zdevito/111/head
[JIT] add type refinements for isinstance checks

This replaces unchecked_unwrap_optional with unchecked_cast. This
enables the generalization of type refinement so that it works for
isinstance checks as well. This also removes unchecked_unwrap_optional from
code we generate, which is good because it is a hard op to serialize well
since it doesn't directly encode the Optional[T] being unwrapped. In contrast,
unchecked_cast always explicitly lists the type.

gh-metadata: pytorch pytorch 26271 gh/zdevito/111/head
[JIT] add type refinements for isinstance checks

This replaces unchecked_unwrap_optional with unchecked_cast. This
enables the generalization of type refinement so that it works for
isinstance checks as well. This also removes unchecked_unwrap_optional from
code we generate, which is good because it is a hard op to serialize well
since it doesn't directly encode the Optional[T] being unwrapped. In contrast,
unchecked_cast always explicitly lists the type.

gh-metadata: pytorch pytorch 26271 gh/zdevito/111/head
[JIT] add type refinements for isinstance checks

This replaces unchecked_unwrap_optional with unchecked_cast. This
enables the generalization of type refinement so that it works for
isinstance checks as well. This also removes unchecked_unwrap_optional from
code we generate, which is good because it is a hard op to serialize well
since it doesn't directly encode the Optional[T] being unwrapped. In contrast,
unchecked_cast always explicitly lists the type.

gh-metadata: pytorch pytorch 26271 gh/zdevito/111/head
[JIT] add type refinements for isinstance checks

This replaces unchecked_unwrap_optional with unchecked_cast. This
enables the generalization of type refinement so that it works for
isinstance checks as well. This also removes unchecked_unwrap_optional from
code we generate, which is good because it is a hard op to serialize well
since it doesn't directly encode the Optional[T] being unwrapped. In contrast,
unchecked_cast always explicitly lists the type.

gh-metadata: pytorch pytorch 26271 gh/zdevito/111/head
@zdevito zdevito requested a review from eellison October 1, 2019 23:48
test/test_jit.py Outdated
self.assertEqual(foo2(None, 4), 0)
self.assertEqual(foo2(4, None), 0)


Copy link
Member

Choose a reason for hiding this comment

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

nit, extra spaces

// isinstance appearing in an if expression
// causes type refinement to occur, we dont
// want to since it has already been handled
if (v->node()->kind() == prim::isinstance)
Copy link
Member

Choose a reason for hiding this comment

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

nit, braces here :{

return false;

// isinstance appearing in an if expression
// causes type refinement to occur, we dont
Copy link
Member

Choose a reason for hiding this comment

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

I find this comment hard to parse. "we dont want to" what? (I think the answer is we don't want to inline). "it has already been handled", what is "it" in this case? Maybe worth expanding this.

Copy link
Contributor

@eellison eellison left a comment

Choose a reason for hiding this comment

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

it would be good as a follow up to implement a peephole optimization for this op

[JIT] add type refinements for isinstance checks

This replaces unchecked_unwrap_optional with unchecked_cast. This
enables the generalization of type refinement so that it works for
isinstance checks as well. This also removes unchecked_unwrap_optional from
code we generate, which is good because it is a hard op to serialize well
since it doesn't directly encode the Optional[T] being unwrapped. In contrast,
unchecked_cast always explicitly lists the type.

gh-metadata: pytorch pytorch 26271 gh/zdevito/111/head
[JIT] add type refinements for isinstance checks

This replaces unchecked_unwrap_optional with unchecked_cast. This
enables the generalization of type refinement so that it works for
isinstance checks as well. This also removes unchecked_unwrap_optional from
code we generate, which is good because it is a hard op to serialize well
since it doesn't directly encode the Optional[T] being unwrapped. In contrast,
unchecked_cast always explicitly lists the type.

gh-metadata: pytorch pytorch 26271 gh/zdevito/111/head
[JIT] add type refinements for isinstance checks

This replaces unchecked_unwrap_optional with unchecked_cast. This
enables the generalization of type refinement so that it works for
isinstance checks as well. This also removes unchecked_unwrap_optional from
code we generate, which is good because it is a hard op to serialize well
since it doesn't directly encode the Optional[T] being unwrapped. In contrast,
unchecked_cast always explicitly lists the type.

gh-metadata: pytorch pytorch 26271 gh/zdevito/111/head
[JIT] add type refinements for isinstance checks

This replaces unchecked_unwrap_optional with unchecked_cast. This
enables the generalization of type refinement so that it works for
isinstance checks as well. This also removes unchecked_unwrap_optional from
code we generate, which is good because it is a hard op to serialize well
since it doesn't directly encode the Optional[T] being unwrapped. In contrast,
unchecked_cast always explicitly lists the type.

gh-metadata: pytorch pytorch 26271 gh/zdevito/111/head
[JIT] add type refinements for isinstance checks

This replaces unchecked_unwrap_optional with unchecked_cast. This
enables the generalization of type refinement so that it works for
isinstance checks as well. This also removes unchecked_unwrap_optional from
code we generate, which is good because it is a hard op to serialize well
since it doesn't directly encode the Optional[T] being unwrapped. In contrast,
unchecked_cast always explicitly lists the type.

gh-metadata: pytorch pytorch 26271 gh/zdevito/111/head
zdevito added a commit to zdevito/ATen that referenced this pull request Oct 10, 2019
Summary:
Pull Request resolved: pytorch/pytorch#26271

This replaces unchecked_unwrap_optional with unchecked_cast. This
enables the generalization of type refinement so that it works for
isinstance checks as well. This also removes unchecked_unwrap_optional from
code we generate, which is good because it is a hard op to serialize well
since it doesn't directly encode the Optional[T] being unwrapped. In contrast,
unchecked_cast always explicitly lists the type.

Test Plan: Imported from OSS

Differential Revision: D17412856

Pulled By: zdevito

fbshipit-source-id: ded47eb086c4610998ad92bb1174225af00220f7
@facebook-github-bot
Copy link
Contributor

@zdevito merged this pull request in d44b9cd.

@facebook-github-bot facebook-github-bot deleted the gh/zdevito/111/head branch October 28, 2019 22:23
thiagocrepaldi pushed a commit to thiagocrepaldi/pytorch that referenced this pull request Feb 4, 2020
Summary:
Pull Request resolved: pytorch#26271

This replaces unchecked_unwrap_optional with unchecked_cast. This
enables the generalization of type refinement so that it works for
isinstance checks as well. This also removes unchecked_unwrap_optional from
code we generate, which is good because it is a hard op to serialize well
since it doesn't directly encode the Optional[T] being unwrapped. In contrast,
unchecked_cast always explicitly lists the type.

Test Plan: Imported from OSS

Differential Revision: D17412856

Pulled By: zdevito

fbshipit-source-id: ded47eb086c4610998ad92bb1174225af00220f7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Merged module: internals Related to internal abstractions in c10 and ATen module: pybind Related to our Python bindings / interactions with other Python libraries oncall: jit Add this issue/PR to JIT oncall triage queue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants