Mousius commented on PR #12550: URL: https://github.com/apache/tvm/pull/12550#issuecomment-1243910186
> The reason that particular test has been added is because it's one behaviour of `ExprAllocator` we can't test through the interface of `AOTLowerMain`. This is because `AOTLowerMain` will throw an exception if it encounters a nested function. However, `AOTLowerMain` will throw _after_ `ExprAllocator` runs, so the code is still in a strict sense reachable. Ah, good point, I guess it's expedited the error 😸 > However, now we also have `AOTLowerMain` written, to test `ExprAllocator` entirely separately would require a lot of duplicated tests (the `AOTLowerMain` tests would be looking at the `Allocate`s produced whereas the `ExprAllocator` tests would look at the `StorageInfo`s). If we want to spend this effort, my personal view is it should be done in the context of promoting `ExprAllocator` to be a proper analysis pass rather than as part of this refactor. I think testing the behaviour which can't be tested higher up is good here, we don't have to duplicate the complete test suite at every layer - agree that if it was actually its own pass we can test it in isolation fully. > I can't write a test against `ExprAllocator` directly without exposing it through the header because otherwise I can't include it in the test case. That's why I added `CreateStorage`, to provide an exposed function through which to probe `ExprAllocator` (like how we have `AOTLowerMain` and `AOTMainLowerer`). To me this is slightly problematic because it was never the intent to expose `ExprAllocator`, but I have no choice if we require tests are written against it directly. Sorry, I misunderstood this originally (thought you meant you couldn't make it `static` rather than excluding it from the interface header), you can just put the prototype in the C++ test file, similar to how it's done for grabbing the Target hooks here: https://github.com/apache/tvm/blob/main/src/relay/backend/contrib/cmsisnn/target.cc#L33-L34 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
