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.

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

Reland "[CUDA][HIP] Fix overloading resolution in global var init" #65606

Merged
merged 1 commit into from
Sep 8, 2023

Conversation

yxsamliu
Copy link
Collaborator

@yxsamliu yxsamliu commented Sep 7, 2023

https://reviews.llvm.org/D158247 caused regressions for HIP on Windows and was reverted.

A reduced test case is:

typedef void (__stdcall* funcTy)();
void invoke(funcTy f);

static void __stdcall callee() noexcept {
}

void foo() {
   invoke(callee);
}

It is due to clang missing handling host/device attributes for calling convention at a few places

This patch fixes that.

@yxsamliu yxsamliu requested a review from a team as a code owner September 7, 2023 13:26
@yxsamliu yxsamliu self-assigned this Sep 7, 2023
@yxsamliu yxsamliu requested a review from a team as a code owner September 7, 2023 13:26
@github-actions github-actions bot added the clang:frontend Language frontend issues, e.g. anything involving "Sema" label Sep 7, 2023
Copy link
Member

@Artem-B Artem-B left a comment

Choose a reason for hiding this comment

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

LGTM.

I'm still figuring out the github-based workflow. One thing that may be useful in the future
would be to start the pull request branch with the original/reverted commit and put the updates into separate commits, so one could see incremental changes, similar to what phabricator's "history" was providing.

@joker-eph
Copy link
Collaborator

Please reflow the title to avoid the .... and line wrap.

@yxsamliu yxsamliu changed the title Reland "[CUDA][HIP] Fix overloading resolution in global variable ini… Reland "[CUDA][HIP] Fix overloading resolution in global var init" Sep 8, 2023
https://reviews.llvm.org/D158247 caused regressions for HIP on Windows
and was reverted.

A reduced test case is:

```
typedef void (__stdcall* funcTy)();
void invoke(funcTy f);

static void __stdcall callee() noexcept {
}

void foo() {
   invoke(callee);
}
```

It is due to clang missing handling host/device attributes for calling convention
at a few places

This patch fixes that.
@yxsamliu yxsamliu merged commit 9b77638 into llvm:main Sep 8, 2023
@@ -70,3 +70,4 @@ pythonenv*
/clang/utils/analyzer/projects/*/RefScanBuildResults
# automodapi puts generated documentation files here.
/lldb/docs/python_api/
/Debug/
Copy link
Member

Choose a reason for hiding this comment

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

This seems accidental? I removed it in 994bdce

(Personally I place build directories in /tmp to prevent mistakes..)

searlmc1 pushed a commit to ROCm/llvm-project that referenced this pull request Oct 13, 2023
…lvm#65606)

Cherry-pick 9b77638

https://reviews.llvm.org/D158247 caused regressions for HIP on Windows
and was reverted.

A reduced test case is:

```
typedef void (__stdcall* funcTy)();
void invoke(funcTy f);

static void __stdcall callee() noexcept {
}

void foo() {
   invoke(callee);
}
```

It is due to clang missing handling host/device attributes for calling
convention at a few places

This patch fixes that.

Change-Id: Ibbc5cbe232d73ddaeb91f13f6afbff3151c9bf0b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema"
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants