Skip to content

Commit

Permalink
Fix VariableBuilder._wrap on frozenset and enforce invariants on `C…
Browse files Browse the repository at this point in the history
…onstantVariable` (#141504)

Summary:
Prior to this patch, we are using `ConstantVariable.create` to create VT
for frozenset objects, and intended yet failed to predicate that on all
itmes being literals (see pytorch/pytorch#140984 (comment)).

The code was from pytorch/torchdynamo@7c03434 and
the original goal was to help DBR quantization, but as the new test in
this patch shows, it could lead to silent incorrectness.

Upon a closer look, this exposes some subtleties in how Dynamo handles
`ConstantVariable` and `LOAD_CONST`, so this patch both fixes the
aforementioned issue and documents, enforces, and makes explicit the
invariants around `ConstantVariable` and `LOAD_CONST` -- only immutable
objects are supported.

Specifically, this patch:
1. refine the checks for wrapping a `frozenset` object, document why we
   can't just wrap its items directly due to lack of `Sourcec` for set
   items, and use a safe workaround (`SourcelessBuilder`) to ensure
   soundness while keeping the DBR quantization support.
2. Adds more types to `common_constant_types`, thereby making
   `ConstantVariable.is_base_literal` more lenient, and strictly checks
   this property in the constructor of `ConstantVariable`.
3. Change relevant uses of `create_instruction("LOAD_CONST", ...)` to
   `create_load_const` which checks `is_safe_constant`, and makes
   developer overrides explicit by using `create_load_const_unchecked`
   when needed.
4. In a few places, use more specific `VariableTracker`, e.g.,
   `TypingVariable` rather than `ConstantVariable`, and
   `FrozensetVariable` rather than `SetVariable`.

(2) and (3) are mainly to future-proof Dynamo against bugs like (1).

X-link: pytorch/pytorch#141504
Approved by: https://github.com/jansel

Reviewed By: atalman

Differential Revision: D66559499

Pulled By: StrongerXi

fbshipit-source-id: 15d11fc3e6b4d3107eefdcee3cb3ff8688a08691
  • Loading branch information
StrongerXi authored and facebook-github-bot committed Dec 2, 2024
1 parent 5c7b91e commit 7217626
Showing 1 changed file with 6 additions and 0 deletions.
6 changes: 6 additions & 0 deletions userbenchmark/dynamo/dynamobench/_dynamo/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -1692,11 +1692,17 @@ def rot_n_helper(n):
bytes,
type(None),
Ellipsis.__class__,
NotImplemented.__class__,
types.CodeType,
# Commonly used immutable types from torch.
torch.device,
torch.dtype,
torch.memory_format,
torch.layout,
torch.finfo,
torch.iinfo,
torch.nn.attention.SDPBackend,
torch.cuda._CudaDeviceProperties,
}

if has_triton_package():
Expand Down

0 comments on commit 7217626

Please sign in to comment.