Skip to content

[PROTON] Remove incorrect conversions between nanoseconds and cycles.#9660

Open
ray-kast wants to merge 1 commit intotriton-lang:mainfrom
ray-kast:main
Open

[PROTON] Remove incorrect conversions between nanoseconds and cycles.#9660
ray-kast wants to merge 1 commit intotriton-lang:mainfrom
ray-kast:main

Conversation

@ray-kast
Copy link

@ray-kast ray-kast commented Mar 6, 2026

The addition of TargetInfoBase::globalTime updates TraceWriter to convert recorded cycle counts to globally-consistent nanosecond timestamps. However, the conversion is done using the hard-coded assumption of a 1GHz clock, which results in events being stretched or squashed to incorrect durations should that assumption not hold. Short of performing complex clock synchronization logic, removing calls to TargetInfoBase::clock and replacing them with TargetInfoBase::globalTime to nullify the need for this math is the cleanest fix for it.

New contributor declaration

  • I am not making a trivial change, such as fixing a typo in a comment.

  • I have written a PR description following these
    rules.

  • I have run pre-commit run --from-ref origin/main --to-ref HEAD.

  • Select one of the following.

    • I have added tests.
      • /test for lit tests
      • /unittest for C++ tests
      • /python/test for end-to-end tests
    • This PR does not need a test because globalTime is not deterministic.
  • Select one of the following.

    • I have not added any lit tests.
    • The lit tests I have added follow these best practices,
      including the "tests should be minimal" section. (Usually running Python code
      and using the instructions it generates is not minimal.)

Copy link
Contributor

@fywkevin fywkevin left a comment

Choose a reason for hiding this comment

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

Using global_time has much lower timing resolution than cycle (see the Q&A here: https://forums.developer.nvidia.com/t/questions-about-globaltimer-functionality-accessing-and-configuring/304268), and it sometimes has uncertain large inaccuracy. Here we are targeting cycle-level events so need a high-resolution clock32/clock64.

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.

2 participants