Skip to content
This repository was archived by the owner on Jul 1, 2023. It is now read-only.

Make PrintX10Metrics available #917

Merged
merged 1 commit into from
Apr 30, 2020
Merged

Conversation

asuhan
Copy link
Contributor

@asuhan asuhan commented Apr 30, 2020

No description provided.

@asuhan asuhan requested a review from compnerd April 30, 2020 21:49
Copy link
Contributor

@saeta saeta left a comment

Choose a reason for hiding this comment

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

Could you add a quick test to ensure we don't accidentally break this in the future (i.e. ensure that the symbol compiles and also that executing it doesn't crash*)?

  • Ideally, we might be able to make certain assertions about its functionality, but I think it's okay to not go that far right now.

compnerd
compnerd previously approved these changes Apr 30, 2020
@compnerd compnerd dismissed their stale review April 30, 2020 21:55

Additional testing requested

@compnerd
Copy link
Contributor

Oh, and did you ensure that PrintMetrics is properly annotated for export on Windows from the C++ side?

@asuhan asuhan force-pushed the asuhan/export_print_metrics branch from d77c5e7 to c01eaf3 Compare April 30, 2020 21:57
@asuhan
Copy link
Contributor Author

asuhan commented Apr 30, 2020

@compnerd It has XLA_API already, do we need anything else?
@saeta I don't know if we can test this effectively from this repository, the main concern is visibility. The ultimate test would be to call it from swift-models, I think.

@compnerd
Copy link
Contributor

XLA_API is exactly what is needed, if it has it already, then nothing to do here, move along :)

@asuhan asuhan merged commit 0b169ac into master Apr 30, 2020
@asuhan asuhan deleted the asuhan/export_print_metrics branch April 30, 2020 22:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants