Skip to content

Commit

Permalink
[NFC] Fix getReferencedModule and add test locking in correct symbol …
Browse files Browse the repository at this point in the history
…resolution behavior
  • Loading branch information
darthscsi committed Oct 16, 2023
1 parent 475d4e9 commit fa69518
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 2 deletions.
3 changes: 1 addition & 2 deletions lib/Dialect/HW/InstanceImplementation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@ instance_like_impl::getReferencedModule(const HWSymbolCache *cache,
if (auto *result = cache->getDefinition(moduleName))
return result;

auto topLevelModuleOp = instanceOp->getParentOfType<ModuleOp>();
return topLevelModuleOp.lookupSymbol(moduleName.getValue());
return SymbolTable::lookupNearestSymbolFrom(instanceOp, moduleName);
}

LogicalResult instance_like_impl::verifyReferencedModule(
Expand Down
14 changes: 14 additions & 0 deletions test/Dialect/HW/errors.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -393,3 +393,17 @@ hw.module @struct(in %a: !hw.struct<foo: i8, bar: i8, foo: i8, baz: i8, bar: i8>
// expected-error @+2 {{duplicate field name 'foo' in hw.union type}}
// expected-error @+1 {{duplicate field name 'bar' in hw.union type}}
hw.module @union(in %a: !hw.union<foo: i8, bar: i8, foo: i8, baz: i8, bar: i8>) {}

// -----

// Test that nested symbol tables fail in the correct way when trying to use an
// instance to escape its containing table.

hw.module @Foo () {}

builtin.module @Nested {
hw.module @Bar () {
// expected-error @+1 {{Cannot find module definition 'Foo'}}
hw.instance "inst" @Foo () -> ()
}
}

2 comments on commit fa69518

@teqdruid
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't this result in theoretically different behavior when one calls with a cache?

@darthscsi
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the cache doesn't fail on this, that is a bug. Incidentally, this test should testing a code path with a cache.

Please sign in to comment.