This is an archive of the discontinued LLVM Phabricator instance.

Differential D95818

[OpenMP] Fix sign comparison warnings from GCC
ClosedPublic

Authored by jlpeyton on Feb 1 2021, 1:55 PM.

Details

Reviewers
AndreyChurbanov
tlwilmar
jdoerfert
Commits
rGffca74b8b82f: [OpenMP] Fix sign comparison warnings from GCC
Summary

New affinity patch introduced legitimate sign-compare warnings that clang doesn't report but GCC-10 does. This removes the warnings by changing two variables types to unsigned.

Diff Detail

Event Timeline

jlpeyton created this revision.Feb 1 2021, 1:55 PM
Herald added subscribers: guansong, yaxunl. · View Herald TranscriptFeb 1 2021, 1:55 PM
jlpeyton requested review of this revision.Feb 1 2021, 1:55 PM
Herald added a reviewer: jdoerfert. · View Herald TranscriptFeb 1 2021, 1:55 PM
Herald added a subscriber: sstefan1. · View Herald Transcript
Harbormaster completed remote builds in B87420: Diff 320591.Feb 1 2021, 3:16 PM
AndreyChurbanov accepted this revision.Feb 2 2021, 2:55 AM

LGTM

This revision is now accepted and ready to land.Feb 2 2021, 2:55 AM
This revision was landed with ongoing or failed builds.Feb 2 2021, 8:53 AM
Closed by commit rGffca74b8b82f: [OpenMP] Fix sign comparison warnings from GCC (authored by jlpeyton). · Explain Why
This revision was automatically updated to reflect the committed changes.
jlpeyton added a commit: rGffca74b8b82f: [OpenMP] Fix sign comparison warnings from GCC.

Revision Contents

PathSize
openmp/
runtime/
src/
4 lines

Diff 320591

openmp/runtime/src/kmp_affinity.cpp

Show First 20 Lines • Show All 103 Lines • ▼ Show 20 Lines while (top_index1 < depth - 1 && top_index2 < depth) {
kmp_hw_t type1 = types[top_index1]; kmp_hw_t type1 = types[top_index1];
kmp_hw_t type2 = types[top_index2]; kmp_hw_t type2 = types[top_index2];
if (type1 == KMP_HW_SOCKET && type2 == KMP_HW_CORE) { if (type1 == KMP_HW_SOCKET && type2 == KMP_HW_CORE) {
top_index1 = top_index2++; top_index1 = top_index2++;
continue; continue;
} }
bool radix1 = true; bool radix1 = true;
bool all_same = true; bool all_same = true;
int id1 = addrP[0].first.labels[top_index1]; unsigned id1 = addrP[0].first.labels[top_index1];
Lint: Pre-merge checks
Inline Actions

clang-tidy: warning: invalid case style for variable 'id1' [readability-identifier-naming]
not useful

Lint: Pre-merge checks: clang-tidy: warning: invalid case style for variable 'id1' [readability-identifier-naming]…
int id2 = addrP[0].first.labels[top_index2]; unsigned id2 = addrP[0].first.labels[top_index2];
Lint: Pre-merge checks
Inline Actions

clang-tidy: warning: invalid case style for variable 'id2' [readability-identifier-naming]
not useful

Lint: Pre-merge checks: clang-tidy: warning: invalid case style for variable 'id2' [readability-identifier-naming]…
int pref1 = preference[type1]; int pref1 = preference[type1];
int pref2 = preference[type2]; int pref2 = preference[type2];
for (int hwidx = 1; hwidx < nTh; ++hwidx) { for (int hwidx = 1; hwidx < nTh; ++hwidx) {
if (addrP[hwidx].first.labels[top_index1] == id1 && if (addrP[hwidx].first.labels[top_index1] == id1 &&
addrP[hwidx].first.labels[top_index2] != id2) { addrP[hwidx].first.labels[top_index2] != id2) {
radix1 = false; radix1 = false;
break; break;
} }
▲ Show 20 Lines • Show All 5,335 Lines • Show Last 20 Lines