Skip to content

Conversation

@vargaz
Copy link
Contributor

@vargaz vargaz commented Sep 14, 2023

…xception() in AOT-ed code.

Part of the fix for #90692.

@ghost ghost assigned vargaz Sep 14, 2023
@vargaz vargaz requested a review from kg September 14, 2023 03:32
@vargaz vargaz force-pushed the wasm-catch branch 2 times, most recently from 024d620 to 60cca14 Compare September 14, 2023 03:43
@kg
Copy link
Member

kg commented Sep 14, 2023

When combined with #91364, this PR fixes #90692 for me.

Copy link
Member

@kg kg left a comment

Choose a reason for hiding this comment

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

LGTM but I'm not an LLVM expert

@vargaz
Copy link
Contributor Author

vargaz commented Sep 14, 2023

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).


LLVMValueRef args [] = { catchpad };
LLVMValueRef call = call_intrins (ctx, INTRINS_WASM_GET_EXCEPTION, args, "");
LLVMSetTailCall (call, TRUE);
Copy link
Contributor

Choose a reason for hiding this comment

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

I am pretty sure there is no need for the tail annotation and Clang just adds it because that's what it does for most calls.

@vargaz vargaz merged commit 9aeed30 into dotnet:main Sep 14, 2023
@vargaz vargaz deleted the wasm-catch branch September 14, 2023 12:06
@ghost ghost locked as resolved and limited conversation to collaborators Oct 14, 2023
@kg
Copy link
Member

kg commented Feb 6, 2024

We may want to backport this to 8.0 staging. (I thought this fix was already in 8, but it looks like it's not?)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants