Thanks! LGTM.

Will land this now, because it's a weekend and the previous patch is breaking the buildbots.

iIsEHPadFunctionsSetup -> IsEHPadFunctionsSetUp

• Change the static variable init to a class member variable IsEHFunctionsSetup (Not sure whether I should AreEHFunctionsSetup instead, but I think usually variable names start with Is..? Let me know if this sounds weird.)
• Move class member variable ExecutedLSDA to an auto variable in prepareEHPads. This variable is used only within that function.

• Add more comment

• Comment fix

• clang-format

There seem to be several other places in which we use Function::Create in this file.. Should we mark all of them as imported as well?

Thanks!

Yeah then it should be fine to add them in Emscripten side.

(Virtually) talked in person and I have better context for this change now. It'd be better we can add a test for this.

• Is this for static linking or dynamic linking? How did things work before this?
• Is there any tests?

• Test fix

• Add comment

Hmm, come to think of it, I can add a test change that cannot run without this change after all. Thanks for suggestions. Done in D76823.

V8 validation fails without this. I'm not sure if there's a way test v8 validation w/ LLVM unit tests?

Move inline comments to their own lines

• clang-format

I think the full diff still has not been uploaded yet; everywhere says "Context not available". Could you use arc diff to upload the diff?

Nice, looks a lot simpler!

This part has kind of complicated history and I hope this can be simplified someday 😂... Btw can the test case be simplified? Do we need all that code to generate the pattern?

There seem to be multiple places that call Graph.getLoopEntries(). I guess we should sort all of them..? How about sort LoopEntries at the end of calculate() once and for all?

Yay! 🎉🎉🎉 It's very simpler and nicer this way.

Nice!

Nice work!

It looks this assumes D71484 and D71496 have not been reverted, right?

• Have you had a chance to see my previous round of comments?
• Do we need to reapply D71484 and D71496?

I also did a quick search for what other backends are doing, and they are all kind of different. Many targets seem to implement some function called isEligibleForTailCallOptimization, even though the superclass TargetLowering does not have that method. Anyway, I think it would be fine if we add things we need on an as-needed basis.

I still don't have 100% understanding of what the def issue was, but I can catch up later!

rebase + test fix

Fix test comments

rebase

• Remove a stray attribute number

• Test target feature section

I think their implemention is mostly complete, but please let me know if not.

Nice work! Sorry for the delayed reply. I like the test generation script and the autogenerated test case, and given the nature of the tests, I don't think there will be many changes to the generated results. With larger number of limits we can maybe use the script like a fuzzer or something too.

LGTM. Please rebase this onto https://reviews.llvm.org/D71484; this currently contains diff of that CL too.