Skip to content

Conversation

@kparzysz-quic
Copy link
Contributor

@kparzysz-quic kparzysz-quic commented May 12, 2022

We no longer run simulator automatically, so this is not necessary. Also, the only way to pass options to the simulator was by setting an environment variable. That variable (HEXAGON_SIM_ARGS) should be set independently by the user from now on.

cc @mehrdadh

@mehrdadh
Copy link
Member

What do you mean by running simulator automatically here?

@github-actions github-actions bot requested a review from mehrdadh May 12, 2022 18:42
@kparzysz-quic
Copy link
Contributor Author

In offload mode the runtime started the simulator without user's intervention.

We no longer run simulator automatically, so this is not necessary.
Also, the only way to pass options to the simulator was by setting
an environment variable. That variable (HEXAGON_SIM_ARGS) should
be set independently by the user from now on.
Copy link
Member

@mehrdadh mehrdadh left a comment

Choose a reason for hiding this comment

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

LGTM!
cc @Lunderberg to take a look as well.

Copy link
Contributor

@Lunderberg Lunderberg left a comment

Choose a reason for hiding this comment

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

LGTM!

The previous use of HEXAGON_SIM_ARGS to pass information between two parts of the same python process was a bit more mutable global state than I prefer, so I like the removal. As I understand it, the functionality that it added was to launch a simulation with matched parameters to the Target object. In the future, if we want to parametrizing the unit tests across multiple simulated hardware versions, this may be useful functionality to maintain, but probably by constructing the arguments in HexagonLauncherSimulator based on the target.

@mehrdadh mehrdadh merged commit 6c339ea into apache:main May 13, 2022
@kparzysz-quic kparzysz-quic deleted the hexagon-sim-args branch May 13, 2022 17:16
mehrdadh pushed a commit to mehrdadh/tvm that referenced this pull request May 16, 2022
We no longer run simulator automatically, so this is not necessary.
Also, the only way to pass options to the simulator was by setting
an environment variable. That variable (HEXAGON_SIM_ARGS) should
be set independently by the user from now on.
shtinsa pushed a commit to Deelvin/tvm that referenced this pull request May 17, 2022
We no longer run simulator automatically, so this is not necessary.
Also, the only way to pass options to the simulator was by setting
an environment variable. That variable (HEXAGON_SIM_ARGS) should
be set independently by the user from now on.
shingjan pushed a commit to shingjan/tvm that referenced this pull request May 17, 2022
We no longer run simulator automatically, so this is not necessary.
Also, the only way to pass options to the simulator was by setting
an environment variable. That variable (HEXAGON_SIM_ARGS) should
be set independently by the user from now on.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants