Page MenuHomePhabricator

[CUDA][HIP] Fix typo in `BestViableFunction`
ClosedPublic

Authored by hliao on Sep 18 2019, 1:13 PM.

Details

Summary
  • Should consider viable ones only when checking SameSide candidates.
  • Replace erasing with clearing viable flag to reduce data moving/copying.
  • Add one and revise another one as the diagnostic message are more relevant compared to previous one.

Diff Detail

Repository
rL LLVM

Event Timeline

hliao created this revision.Sep 18 2019, 1:13 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 18 2019, 1:13 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
tra added a reviewer: jlebar.Sep 18 2019, 3:35 PM
tra added a comment.Sep 18 2019, 3:38 PM

LGTM. You may want to wait a bit for Justin's feedback, in case he has some concerns.

clang/test/SemaCUDA/function-overload.cu
406 ↗(On Diff #220738)

I'd remove irrelevant.

407–411 ↗(On Diff #220738)

We don't need definitions for struct and operator-;

This should do:

struct C1;
struct C2;
__device__ int operator-(const C1 &x, const C1 &y);
int operator-(const C2 &x, const C2 &y);
...
int test_constexpr_overload(C2 &x, C2 &y) {
    return constexpr_overload(x, y);
}
414 ↗(On Diff #220738)

Is constexpr necessary here? If not, then we would not need -std=c++11 either.
I think you really want __host__ __device__ here instead. constexpr is just applying the attributes implicitly.

tra accepted this revision.Sep 18 2019, 3:38 PM
This revision is now accepted and ready to land.Sep 18 2019, 3:38 PM
hliao marked 3 inline comments as done.Sep 19 2019, 6:12 AM

r372318 with test case revised following suggestion.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptSep 19 2019, 6:13 AM