From ff819cc8f0ac4331135136d7763ec9275250ac8e Mon Sep 17 00:00:00 2001 From: Animesh Jain Date: Sat, 3 May 2025 22:01:36 -0700 Subject: [PATCH] [dynamo][super variable] Fix bug to use correct source --- .../aot_eager_huggingface_training.csv | 4 +- .../dynamic_inductor_huggingface_training.csv | 4 +- .../cu124/inductor_huggingface_training.csv | 4 +- ...dynamic_aot_eager_huggingface_training.csv | 4 +- .../dynamic_inductor_huggingface_training.csv | 4 +- .../dynamo_eager_huggingface_training.csv | 4 +- .../inductor_huggingface_training.csv | 4 +- .../rocm/aot_eager_huggingface_training.csv | 4 +- ...dynamic_aot_eager_huggingface_training.csv | 4 +- .../dynamic_inductor_huggingface_training.csv | 4 +- .../dynamo_eager_huggingface_training.csv | 4 +- .../rocm/inductor_huggingface_training.csv | 4 +- test/dynamo/test_misc.py | 9 +- torch/_dynamo/guards.py | 9 ++ torch/_dynamo/source.py | 24 +++++ torch/_dynamo/variables/misc.py | 18 +++- torch/csrc/dynamo/guards.cpp | 95 +++++++++++++++++++ 17 files changed, 174 insertions(+), 29 deletions(-) diff --git a/benchmarks/dynamo/ci_expected_accuracy/aot_eager_huggingface_training.csv b/benchmarks/dynamo/ci_expected_accuracy/aot_eager_huggingface_training.csv index 8202281ed9bc5..66e088f334071 100644 --- a/benchmarks/dynamo/ci_expected_accuracy/aot_eager_huggingface_training.csv +++ b/benchmarks/dynamo/ci_expected_accuracy/aot_eager_huggingface_training.csv @@ -82,7 +82,7 @@ ElectraForQuestionAnswering,pass,5 -GPT2ForSequenceClassification,pass,5 +GPT2ForSequenceClassification,pass,6 @@ -94,7 +94,7 @@ LayoutLMForMaskedLM,pass,5 -LayoutLMForSequenceClassification,pass,5 +LayoutLMForSequenceClassification,pass,6 diff --git a/benchmarks/dynamo/ci_expected_accuracy/cu124/dynamic_inductor_huggingface_training.csv b/benchmarks/dynamo/ci_expected_accuracy/cu124/dynamic_inductor_huggingface_training.csv index 8202281ed9bc5..66e088f334071 100644 --- a/benchmarks/dynamo/ci_expected_accuracy/cu124/dynamic_inductor_huggingface_training.csv +++ b/benchmarks/dynamo/ci_expected_accuracy/cu124/dynamic_inductor_huggingface_training.csv @@ -82,7 +82,7 @@ ElectraForQuestionAnswering,pass,5 -GPT2ForSequenceClassification,pass,5 +GPT2ForSequenceClassification,pass,6 @@ -94,7 +94,7 @@ LayoutLMForMaskedLM,pass,5 -LayoutLMForSequenceClassification,pass,5 +LayoutLMForSequenceClassification,pass,6 diff --git a/benchmarks/dynamo/ci_expected_accuracy/cu124/inductor_huggingface_training.csv b/benchmarks/dynamo/ci_expected_accuracy/cu124/inductor_huggingface_training.csv index 8202281ed9bc5..66e088f334071 100644 --- a/benchmarks/dynamo/ci_expected_accuracy/cu124/inductor_huggingface_training.csv +++ b/benchmarks/dynamo/ci_expected_accuracy/cu124/inductor_huggingface_training.csv @@ -82,7 +82,7 @@ ElectraForQuestionAnswering,pass,5 -GPT2ForSequenceClassification,pass,5 +GPT2ForSequenceClassification,pass,6 @@ -94,7 +94,7 @@ LayoutLMForMaskedLM,pass,5 -LayoutLMForSequenceClassification,pass,5 +LayoutLMForSequenceClassification,pass,6 diff --git a/benchmarks/dynamo/ci_expected_accuracy/dynamic_aot_eager_huggingface_training.csv b/benchmarks/dynamo/ci_expected_accuracy/dynamic_aot_eager_huggingface_training.csv index 8202281ed9bc5..66e088f334071 100644 --- a/benchmarks/dynamo/ci_expected_accuracy/dynamic_aot_eager_huggingface_training.csv +++ b/benchmarks/dynamo/ci_expected_accuracy/dynamic_aot_eager_huggingface_training.csv @@ -82,7 +82,7 @@ ElectraForQuestionAnswering,pass,5 -GPT2ForSequenceClassification,pass,5 +GPT2ForSequenceClassification,pass,6 @@ -94,7 +94,7 @@ LayoutLMForMaskedLM,pass,5 -LayoutLMForSequenceClassification,pass,5 +LayoutLMForSequenceClassification,pass,6 diff --git a/benchmarks/dynamo/ci_expected_accuracy/dynamic_inductor_huggingface_training.csv b/benchmarks/dynamo/ci_expected_accuracy/dynamic_inductor_huggingface_training.csv index 8202281ed9bc5..66e088f334071 100644 --- a/benchmarks/dynamo/ci_expected_accuracy/dynamic_inductor_huggingface_training.csv +++ b/benchmarks/dynamo/ci_expected_accuracy/dynamic_inductor_huggingface_training.csv @@ -82,7 +82,7 @@ ElectraForQuestionAnswering,pass,5 -GPT2ForSequenceClassification,pass,5 +GPT2ForSequenceClassification,pass,6 @@ -94,7 +94,7 @@ LayoutLMForMaskedLM,pass,5 -LayoutLMForSequenceClassification,pass,5 +LayoutLMForSequenceClassification,pass,6 diff --git a/benchmarks/dynamo/ci_expected_accuracy/dynamo_eager_huggingface_training.csv b/benchmarks/dynamo/ci_expected_accuracy/dynamo_eager_huggingface_training.csv index 8202281ed9bc5..66e088f334071 100644 --- a/benchmarks/dynamo/ci_expected_accuracy/dynamo_eager_huggingface_training.csv +++ b/benchmarks/dynamo/ci_expected_accuracy/dynamo_eager_huggingface_training.csv @@ -82,7 +82,7 @@ ElectraForQuestionAnswering,pass,5 -GPT2ForSequenceClassification,pass,5 +GPT2ForSequenceClassification,pass,6 @@ -94,7 +94,7 @@ LayoutLMForMaskedLM,pass,5 -LayoutLMForSequenceClassification,pass,5 +LayoutLMForSequenceClassification,pass,6 diff --git a/benchmarks/dynamo/ci_expected_accuracy/inductor_huggingface_training.csv b/benchmarks/dynamo/ci_expected_accuracy/inductor_huggingface_training.csv index 8202281ed9bc5..66e088f334071 100644 --- a/benchmarks/dynamo/ci_expected_accuracy/inductor_huggingface_training.csv +++ b/benchmarks/dynamo/ci_expected_accuracy/inductor_huggingface_training.csv @@ -82,7 +82,7 @@ ElectraForQuestionAnswering,pass,5 -GPT2ForSequenceClassification,pass,5 +GPT2ForSequenceClassification,pass,6 @@ -94,7 +94,7 @@ LayoutLMForMaskedLM,pass,5 -LayoutLMForSequenceClassification,pass,5 +LayoutLMForSequenceClassification,pass,6 diff --git a/benchmarks/dynamo/ci_expected_accuracy/rocm/aot_eager_huggingface_training.csv b/benchmarks/dynamo/ci_expected_accuracy/rocm/aot_eager_huggingface_training.csv index b54c6a84bc26e..9fdb41506e3b2 100644 --- a/benchmarks/dynamo/ci_expected_accuracy/rocm/aot_eager_huggingface_training.csv +++ b/benchmarks/dynamo/ci_expected_accuracy/rocm/aot_eager_huggingface_training.csv @@ -82,7 +82,7 @@ ElectraForQuestionAnswering,pass,5 -GPT2ForSequenceClassification,pass,5 +GPT2ForSequenceClassification,pass,6 @@ -94,7 +94,7 @@ LayoutLMForMaskedLM,pass,5 -LayoutLMForSequenceClassification,pass,5 +LayoutLMForSequenceClassification,pass,6 diff --git a/benchmarks/dynamo/ci_expected_accuracy/rocm/dynamic_aot_eager_huggingface_training.csv b/benchmarks/dynamo/ci_expected_accuracy/rocm/dynamic_aot_eager_huggingface_training.csv index b54c6a84bc26e..9fdb41506e3b2 100644 --- a/benchmarks/dynamo/ci_expected_accuracy/rocm/dynamic_aot_eager_huggingface_training.csv +++ b/benchmarks/dynamo/ci_expected_accuracy/rocm/dynamic_aot_eager_huggingface_training.csv @@ -82,7 +82,7 @@ ElectraForQuestionAnswering,pass,5 -GPT2ForSequenceClassification,pass,5 +GPT2ForSequenceClassification,pass,6 @@ -94,7 +94,7 @@ LayoutLMForMaskedLM,pass,5 -LayoutLMForSequenceClassification,pass,5 +LayoutLMForSequenceClassification,pass,6 diff --git a/benchmarks/dynamo/ci_expected_accuracy/rocm/dynamic_inductor_huggingface_training.csv b/benchmarks/dynamo/ci_expected_accuracy/rocm/dynamic_inductor_huggingface_training.csv index 8202281ed9bc5..66e088f334071 100644 --- a/benchmarks/dynamo/ci_expected_accuracy/rocm/dynamic_inductor_huggingface_training.csv +++ b/benchmarks/dynamo/ci_expected_accuracy/rocm/dynamic_inductor_huggingface_training.csv @@ -82,7 +82,7 @@ ElectraForQuestionAnswering,pass,5 -GPT2ForSequenceClassification,pass,5 +GPT2ForSequenceClassification,pass,6 @@ -94,7 +94,7 @@ LayoutLMForMaskedLM,pass,5 -LayoutLMForSequenceClassification,pass,5 +LayoutLMForSequenceClassification,pass,6 diff --git a/benchmarks/dynamo/ci_expected_accuracy/rocm/dynamo_eager_huggingface_training.csv b/benchmarks/dynamo/ci_expected_accuracy/rocm/dynamo_eager_huggingface_training.csv index b54c6a84bc26e..9fdb41506e3b2 100644 --- a/benchmarks/dynamo/ci_expected_accuracy/rocm/dynamo_eager_huggingface_training.csv +++ b/benchmarks/dynamo/ci_expected_accuracy/rocm/dynamo_eager_huggingface_training.csv @@ -82,7 +82,7 @@ ElectraForQuestionAnswering,pass,5 -GPT2ForSequenceClassification,pass,5 +GPT2ForSequenceClassification,pass,6 @@ -94,7 +94,7 @@ LayoutLMForMaskedLM,pass,5 -LayoutLMForSequenceClassification,pass,5 +LayoutLMForSequenceClassification,pass,6 diff --git a/benchmarks/dynamo/ci_expected_accuracy/rocm/inductor_huggingface_training.csv b/benchmarks/dynamo/ci_expected_accuracy/rocm/inductor_huggingface_training.csv index 8202281ed9bc5..66e088f334071 100644 --- a/benchmarks/dynamo/ci_expected_accuracy/rocm/inductor_huggingface_training.csv +++ b/benchmarks/dynamo/ci_expected_accuracy/rocm/inductor_huggingface_training.csv @@ -82,7 +82,7 @@ ElectraForQuestionAnswering,pass,5 -GPT2ForSequenceClassification,pass,5 +GPT2ForSequenceClassification,pass,6 @@ -94,7 +94,7 @@ LayoutLMForMaskedLM,pass,5 -LayoutLMForSequenceClassification,pass,5 +LayoutLMForSequenceClassification,pass,6 diff --git a/test/dynamo/test_misc.py b/test/dynamo/test_misc.py index 9a4a1bcca719e..7541bd3b9d88c 100644 --- a/test/dynamo/test_misc.py +++ b/test/dynamo/test_misc.py @@ -11981,6 +11981,10 @@ def fn(t): self.assertEqual(y, t.sin()) def test_overridden_getattribute(self): + class Bar: + def __init__(self, v): + self.v = v + class Foo: attribute_map = {} @@ -11988,6 +11992,9 @@ def __init__(self): self.attribute_map = { "a_premap": "a", } + # `bar` attribute requires propagating sources correctly through + # object.__getattribute__ + self.bar = Bar(5) def __setattr__(self, key, value): if key in super().__getattribute__("attribute_map"): @@ -12015,7 +12022,7 @@ def get_foo(): return f def fn(x, f): - return x * f.a_premap * f.a * f.b * f.sentinel + return x * f.a_premap * f.a * f.b * f.sentinel * f.bar.v x = torch.randn(4) diff --git a/torch/_dynamo/guards.py b/torch/_dynamo/guards.py index c02bda4568c9f..e92ebd5219663 100644 --- a/torch/_dynamo/guards.py +++ b/torch/_dynamo/guards.py @@ -102,6 +102,7 @@ FlattenScriptObjectSource, FloatTensorSource, FSDPNNModuleSource, + GenericAttrSource, GetItemSource, GlobalSource, GlobalStateSource, @@ -1046,6 +1047,14 @@ def get_guard_manager_from_source(self, source): example_value=example_value, guard_manager_enum=guard_manager_enum, ) + elif istype(source, GenericAttrSource): + assert base_guard_manager # to make mypy happy + out = base_guard_manager.generic_getattr_manager( + attr=source.member, + source=source_name, + example_value=example_value, + guard_manager_enum=guard_manager_enum, + ) elif istype(source, (AttrSource, UnspecializedParamBufferSource)): assert base_guard_manager # to make mypy happy diff --git a/torch/_dynamo/source.py b/torch/_dynamo/source.py index e01c166c97d21..55600277b280e 100644 --- a/torch/_dynamo/source.py +++ b/torch/_dynamo/source.py @@ -240,6 +240,30 @@ def name(self): return f"{self.base.name()}.{self.member}" +@dataclasses.dataclass(frozen=True) +class GenericAttrSource(ChainedSource): + member: str + + def __post_init__(self): + assert self.base, "Can't construct an AttrSource without a valid base source" + if "." in self.member: + member_parts = self.member.split(".") + object.__setattr__( + self, "base", AttrSource(self.base, ".".join(member_parts[:-1])) + ) + object.__setattr__(self, "member", member_parts[-1]) + + def reconstruct(self, codegen): + codegen(self.base) + codegen.extend_output(codegen.create_load_attrs(self.member)) + + def guard_source(self): + return self.base.guard_source() + + def name(self): + return f"object.__getattribute__({self.base.name()}, {self.member!r})" + + @dataclasses.dataclass(frozen=True) class LocalCellSource(Source): """ diff --git a/torch/_dynamo/variables/misc.py b/torch/_dynamo/variables/misc.py index 241bfb2c808b2..186454a34e808 100644 --- a/torch/_dynamo/variables/misc.py +++ b/torch/_dynamo/variables/misc.py @@ -38,7 +38,13 @@ from ..exc import raise_observed_exception, unimplemented from ..guards import GuardBuilder, install_guard from ..mutation_guard import unpatched_nn_module_init -from ..source import AttrSource, GetItemSource, TypeSource, WeakRefCallSource +from ..source import ( + AttrSource, + GenericAttrSource, + GetItemSource, + TypeSource, + WeakRefCallSource, +) from ..utils import ( check_unspec_or_constant_args, cmp_name_to_op_mapping, @@ -260,12 +266,16 @@ def call_method( return result try: - attr_value = self.objvar.value.__getattribute__(attr_name) + # NB - use object.__getattribute__ to prevent running any user code + attr_value = object.__getattribute__(self.objvar.value, attr_name) except AttributeError: raise_observed_exception(AttributeError, tx) - source = self.source and AttrSource(self.source, attr_name) - return VariableTracker.build(tx, attr_value, source) + attr_source = None + if self.objvar.source is not None: + # setup a object.__getattribute__(self.objvar, name) source + attr_source = GenericAttrSource(self.objvar.source, attr_name) + return VariableTracker.build(tx, attr_value, attr_source) unimplemented(f"non-function or method super: {inner_fn}") diff --git a/torch/csrc/dynamo/guards.cpp b/torch/csrc/dynamo/guards.cpp index 60fb339d1dbd9..37ed08cf1203a 100644 --- a/torch/csrc/dynamo/guards.cpp +++ b/torch/csrc/dynamo/guards.cpp @@ -3488,6 +3488,85 @@ class GetAttrGuardAccessor : public GuardAccessor { PyObject* _attr_name; }; +/** + * Represents object.__getattribute__(obj, attr_name) acccessor. + */ +class GenericGetAttrGuardAccessor : public GuardAccessor { + public: + GenericGetAttrGuardAccessor( + RootGuardManager* root, + py::str name, + std::string source, + py::handle example_value, + py::handle guard_manager_enum) + : GuardAccessor( + root, + name, + std::move(source), + example_value, + guard_manager_enum), + _attr_name(name.ptr()) {} + + // NB: Intentional duplication between check_nopybind and + // check_verbose_nopybind. + bool check_nopybind(PyObject* obj, bool matches_dict_tag = false) + override { // borrowed ref + PyObject* x = PyObject_GenericGetAttr(obj, _attr_name); // new ref + if (x == nullptr) { + // Attribute absent, clear the exception and return false. + PyErr_Clear(); + return false; + } + bool result = _guard_manager->check_nopybind(x); + Py_DECREF(x); + return result; + } + + GuardDebugInfo check_verbose_nopybind( + PyObject* obj) override { // borrowed ref + PyObject* x = PyObject_GenericGetAttr(obj, _attr_name); // new ref + if (x == nullptr) { + // Attribute absent, clear the exception and return false. + PyErr_Clear(); + return GuardDebugInfo( + false, "getattr failed on source " + get_source(), 0); + } + GuardDebugInfo result = _guard_manager->check_verbose_nopybind(x); + Py_DECREF(x); + return result; + } + + std::string repr() const override { + // Helpful when priting GuardManager tree structure. + return "GenericGetAttrGuardAccessor(" + + py::str(_attr_name).cast() + ")"; + } + + public: // cloning functions + GenericGetAttrGuardAccessor( + GuardManager* guard_manager, + GenericGetAttrGuardAccessor* from) + : GuardAccessor(guard_manager, from) { + from->clone_visitor(this); + } + + GuardAccessor* clone( + RootGuardManager* cloned_root, + const py::function& clone_filter_fn) override { + return clone_common( + cloned_root, clone_filter_fn); + } + + void clone_visitor(GenericGetAttrGuardAccessor* to) { + to->_attr_name = _attr_name; + } + + private: + // no need of py::object here because the attr_name is already passed on to + // the base class as accessor_key which is a py::object. + PyObject* _attr_name{nullptr}; +}; + /** * Represents x.__dict__ acccessor. */ @@ -5349,6 +5428,12 @@ PyObject* torch_c_dynamo_guards_init() { GuardAccessor, std::unique_ptr>(py_m, "GetAttrGuardAccessor"); // NOLINTNEXTLINE(bugprone-unused-raii) + py::class_< + GenericGetAttrGuardAccessor, + GuardAccessor, + std::unique_ptr>( + py_m, "GenericGetAttrGuardAccessor"); + // NOLINTNEXTLINE(bugprone-unused-raii) py::class_< GetGenericDictGuardAccessor, GuardAccessor, @@ -5917,6 +6002,16 @@ PyObject* torch_c_dynamo_guards_init() { py::return_value_policy::reference) // return by reference because C++ GuardManager has the ownership of // accessors and guard managers + .def( + "generic_getattr_manager", + &GuardManager::get_child_manager, + py::arg("attr"), + py::arg("source"), + py::arg("example_value"), + py::arg("guard_manager_enum"), + py::return_value_policy::reference) + // return by reference because C++ GuardManager has the ownership of + // accessors and guard managers .def( "getattr_manager", &GuardManager::get_child_manager,