Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Switch Instead of If, main branch (2024.12.04.) #796

Merged
merged 2 commits into from
Dec 6, 2024
Merged

Use Switch Instead of If, main branch (2024.12.04.) #796

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1d8e4db
Replace some if(...) blocks with switch(...) ones.
krasznaa Dec 4, 2024
c7d26ee
Switched to using __builtin_unreachable() in the switch statements.
krasznaa Dec 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 42 additions & 47 deletions core/include/traccc/edm/track_state.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,28 +79,28 @@ struct track_state {
TRACCC_HOST_DEVICE inline matrix_type<D, 1> measurement_local() const {
static_assert(((D == 1u) || (D == 2u)),
"The measurement dimension should be 1 or 2");
assert((m_measurement.subs.get_indices()[0] == e_bound_loc0) ||
(m_measurement.subs.get_indices()[0] == e_bound_loc1));

matrix_type<D, 1> ret;
if (m_measurement.subs.get_indices()[0] == e_bound_loc0) {
matrix_operator().element(ret, 0, 0) = m_measurement.local[0];
if constexpr (D == 2u) {
matrix_operator().element(ret, 1, 0) = m_measurement.local[1];
}
} else if (m_measurement.subs.get_indices()[0] == e_bound_loc1) {
matrix_operator().element(ret, 0, 0) = m_measurement.local[1];
if constexpr (D == 2u) {
matrix_operator().element(ret, 1, 0) = m_measurement.local[0];
}
} else {
assert(
"The measurement index out of e_bound_loc0 and e_bound_loc1 "
"should not happen.");
matrix_operator().element(ret, 0, 0) = m_measurement.local[0];
if constexpr (D == 2u) {
matrix_operator().element(ret, 1, 0) = m_measurement.local[1];
}
switch (m_measurement.subs.get_indices()[0]) {
case e_bound_loc0:
matrix_operator().element(ret, 0, 0) = m_measurement.local[0];
if constexpr (D == 2u) {
matrix_operator().element(ret, 1, 0) =
m_measurement.local[1];
}
break;
case e_bound_loc1:
matrix_operator().element(ret, 0, 0) = m_measurement.local[1];
if constexpr (D == 2u) {
matrix_operator().element(ret, 1, 0) =
m_measurement.local[0];
}
break;
default:
__builtin_unreachable();
}

return ret;
}

Expand All @@ -109,38 +109,33 @@ struct track_state {
TRACCC_HOST_DEVICE inline matrix_type<D, D> measurement_covariance() const {
static_assert(((D == 1u) || (D == 2u)),
"The measurement dimension should be 1 or 2");
assert((m_measurement.subs.get_indices()[0] == e_bound_loc0) ||
(m_measurement.subs.get_indices()[0] == e_bound_loc1));

matrix_type<D, D> ret;
if (m_measurement.subs.get_indices()[0] == e_bound_loc0) {

matrix_operator().element(ret, 0, 0) = m_measurement.variance[0];
if constexpr (D == 2u) {
matrix_operator().element(ret, 0, 1) = 0.f;
matrix_operator().element(ret, 1, 0) = 0.f;
matrix_operator().element(ret, 1, 1) =
m_measurement.variance[1];
}

} else if (m_measurement.subs.get_indices()[0] == e_bound_loc1) {

matrix_operator().element(ret, 0, 0) = m_measurement.variance[1];
if constexpr (D == 2u) {
matrix_operator().element(ret, 0, 1) = 0.f;
matrix_operator().element(ret, 1, 0) = 0.f;
matrix_operator().element(ret, 1, 1) =
switch (m_measurement.subs.get_indices()[0]) {
case e_bound_loc0:
matrix_operator().element(ret, 0, 0) =
m_measurement.variance[0];
}
} else {
assert(
"The measurement index out of e_bound_loc0 and e_bound_loc1 "
"should not happen.");
matrix_operator().element(ret, 0, 0) = m_measurement.variance[0];
if constexpr (D == 2u) {
matrix_operator().element(ret, 0, 1) = 0.f;
matrix_operator().element(ret, 1, 0) = 0.f;
matrix_operator().element(ret, 1, 1) =
if constexpr (D == 2u) {
matrix_operator().element(ret, 0, 1) = 0.f;
matrix_operator().element(ret, 1, 0) = 0.f;
matrix_operator().element(ret, 1, 1) =
m_measurement.variance[1];
}
break;
case e_bound_loc1:
matrix_operator().element(ret, 0, 0) =
m_measurement.variance[1];
}
if constexpr (D == 2u) {
matrix_operator().element(ret, 0, 1) = 0.f;
matrix_operator().element(ret, 1, 0) = 0.f;
matrix_operator().element(ret, 1, 1) =
m_measurement.variance[0];
}
break;
default:
__builtin_unreachable();
}
return ret;
}
Expand Down
17 changes: 11 additions & 6 deletions core/include/traccc/fitting/kalman_filter/gain_matrix_updater.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,18 @@ struct gain_matrix_updater {

const auto D = trk_state.get_measurement().meas_dim;
assert(D == 1u || D == 2u);
if (D == 1u) {
return update<1u, shape_type>(trk_state, bound_params);
} else if (D == 2u) {
return update<2u, shape_type>(trk_state, bound_params);
bool result = false;
switch (D) {
case 1u:
result = update<1u, shape_type>(trk_state, bound_params);
break;
case 2u:
result = update<2u, shape_type>(trk_state, bound_params);
break;
Comment on lines +50 to +55
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
case 1u:
result = update<1u, shape_type>(trk_state, bound_params);
break;
case 2u:
result = update<2u, shape_type>(trk_state, bound_params);
break;
case 1u:
return update<1u, shape_type>(trk_state, bound_params);
case 2u:
return update<2u, shape_type>(trk_state, bound_params);

Copy link
Member Author

Choose a reason for hiding this comment

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

The reason I went with that setup was two-fold:

  • I'm not 100% sure what happens with __builtin_unreachable() on all the platforms that this code will run on.

I was considering putting

default:
    __builtin_unreachable();
    return false;

there, "just to be safe". But this formalism seemed better.

  • GPU functions can sometimes have hangups about returning at different points. Many coding rules even forbid multiple return points from functions. So I thought this may actually help.

But admittedly, I'm not absolutely sure about either of these points. 🤔

default:
__builtin_unreachable();
}

return false;
return result;
}

template <size_type D, typename shape_t>
Expand Down
Loading