Skip to content

Conversation

@kawashima-fj
Copy link
Member

Just code cleanup. See commit comment for meanings.

... in the case of `OPAL_GCC_INLINE_ASSEMBLY == 0`

In this case, `OPAL_HAVE_SYS_TIMER_GET_CYCLES` should be 0 because
the `opal_sys_timer_get_cycles` function is not defined.

The history:

1. Before 8d4175a, `OPAL_HAVE_SYS_TIMER_GET_CYCLES` was 0.
2. In 8d4175a, adf92d6, adf92d6, and c62ce15,
   `OPAL_HAVE_SYS_TIMER_GET_CYCLES` was changed to 1 by introducing
   `opal/asm/base/*.asm`.
3. In ebce88b, `opal/asm/base/*.asm` were removed.

Signed-off-by: KAWASHIMA Takahiro <[email protected]>
... to avoid using an architecture name macro in
`opal/mca/timer/linux/timer_linux_component.c`.

The function name `opal_sys_timer_freq` is also changed for
consistency with `opal_sys_timer_get_cycles`.

Signed-off-by: KAWASHIMA Takahiro <[email protected]>
@bosilca
Copy link
Member

bosilca commented Apr 4, 2019

I do not see what is the benefit of OPAL_HAVE_SYS_TIMER_GET_FREQ. The new code does not bring any benefit, it is exactly the same code except with one additional layer of indirection. Everything else looks like a good catch.

@kawashima-fj
Copy link
Member Author

@bosilca The benefit is removing OPAL_ASSEMBLY_ARCH == OPAL_ARM64 (architecture name) from opal/mca/timer/linux/timer_linux_component.c. It was a kind of abstraction violation.

@kawashima-fj kawashima-fj merged commit 163bbd4 into open-mpi:master Apr 8, 2019
@kawashima-fj kawashima-fj deleted the pr/sys-timer-cleanup branch April 8, 2019 00:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants