llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.03k stars 11.58k forks source link

Adding hot-patch compiler flag when compiling UE5 from source fails on multiple files. #56234

Closed Hisamera closed 5 months ago

Hisamera commented 2 years ago

Hello

I would like to get this out first I read contributing guideline, but because of Epic EULA I cannot just plop here it's entire source code, some assignee or contributor to LLVM can quite easliy become an Epic's developer here on Github and get the source code for self.

tl;dr UE5 with hotpatch flag fails to compile, please help or advise. For more information see attached files.

With very little changes (and if you disable few warnings, with none) you can compile UE5 source with Clang on Windows, to which I'm quite amazed, because when I tried it before (long time ago) it produced so many errors (because Epic used it's... "flavour" of C++ which was non-standard), that I quickly gave up.

Problem start's when you add to a "/hotpatch" flag to a clang-cl compiler in VCToolChain.cs file (I attached a dif of my source to a Epics commit d11782b9046e9d0b130309591e4efc57f4b8b037 (HEAD -> release, tag: 5.0.2-release, origin/release, origin/HEAD); Merge: 46544fa5e0aa 471880283c09; Author: UnrealBot unrealbot@noreply.users.github.com;Date: Thu May 26 15:05:56 2022 +0000; 5.0.2 release), after this dozens of files throw errors, but with literally thousands compiling just okay I think there may be some bug in frontend. I looked into source files that throw function compilation problems, but I am unable to find a pattern and fix it.

We can go by for now with re-launching editor every time our code changes, as LLVM toolset, and LLVM-compatible programs save us much more time, and we have do-it-in-Blueprints-first approach that we don't need it as much, but it certainly comes in handy.

We very much would like to see it's working as it allows to be OS-agnostic, as Clang is on Windows, natively on Linux, and Apple has it's own "custom" version of Clang.

I'm providing all .sh files, as you can see on them, whenever it is a unity or not build doesn't matter, and frontend exits on the same functions. Git diff of changes that I've performed, and both unity and non-unity compilation log, so you can precisely see how it compiled (or to be exact failed to compile).

If you are short on hands, I could maybe chip in something (This would be my first time dabbling in compiler code, So I just assume that even if I started doing something, someone more in-the-know would finish it faster than me) but to do it, I'd need to get some jumpstart, and by this I mean some links to documentation of how to read those errors, and what to keep in mind while doing this, but If I have to read C++ official documentation things could go rough, as I've never read it Clang-issue.zip .

sylvain-audi commented 1 year ago

Hello, FYI, I also encountered issues when trying to compile our project with /hotpatch flag. There were some clang crashes, and some invalid generated code.

See the issue 59039 and the comments in patch review https://reviews.llvm.org/D137642

Endilll commented 11 months ago

@Hisamera @sylvain-audi Do you still experience issues with Clang 17 or trunk?

Endilll commented 11 months ago

If you can't share preprocessed source, you can reduce it yourself with creduce, and submit here a Compiler Explorer link with minimal reproducer. creduce also renames things by default, so names are not going to leak as well.

sylvain-audi commented 11 months ago

Hello, yes I see the issue is still present. I took the repro from the comment in the issue 59039 -> https://godbolt.org/z/39v9hjx61 : the generated missing_tail_call is invalid as it's missing the expected tailcall and doesn,t even have a ret instruction.

It's just an example showing that the transformation pass for fms-hotpatch may generate bad code, there are certainly other cases.

On our side, we made a workaround in our LLVM fork, where we simply force the patching to systematically insert a 2-byte nop before the first instruction instead of trying to patch it, but this solution is probably not acceptable for upstream.

sylvain-audi commented 11 months ago

My comments on this issue were about the bug we encountered with /hotpatch, but not in the context of building UE.

Also, the problems we had were at runtime: the compilation was successful but the generated executable had invalid code.

aganea commented 5 months ago

I've managed to successfully compile and run UE 5.3.2 with clang-cl 18 and the PR referenced above. Also, integration with latest Live++ 2.4.1 works fine, assuming that you also use https://github.com/brickadia/LivePP2/tree/main and not Epic's LiveCoding (which hasn't been updated for more than 3 years).

aganea commented 5 months ago

@Hisamera The commit https://github.com/llvm/llvm-project/commit/ec1af63dde58c735fe60d6f2aafdb10fa93f410d should fix your usage. If not, feel free to re-open this issue.

llvmbot commented 5 months ago

@llvm/issue-subscribers-backend-x86

Author: Dorian (Hisamera)

Hello I would like to get this out first I read contributing guideline, but because of Epic EULA I cannot just plop here it's entire source code, some assignee or contributor to LLVM can quite easliy become an Epic's developer here on Github and get the source code for self. tl;dr UE5 with hotpatch flag fails to compile, please help or advise. For more information see attached files. With very little changes (and if you disable few warnings, with none) you can compile UE5 source with Clang on Windows, to which I'm quite amazed, because when I tried it before (long time ago) it produced so many errors (because Epic used it's... "flavour" of C++ which was non-standard), that I quickly gave up. Problem start's when you add to a "/hotpatch" flag to a clang-cl compiler in VCToolChain.cs file (I attached a dif of my source to a Epics commit d11782b9046e9d0b130309591e4efc57f4b8b037 (HEAD -> release, tag: 5.0.2-release, origin/release, origin/HEAD); Merge: 46544fa5e0aa 471880283c09; Author: UnrealBot <unrealbot@noreply.users.github.com>;Date: Thu May 26 15:05:56 2022 +0000; 5.0.2 release), after this dozens of files throw errors, but with literally thousands compiling just okay I think there may be some bug in frontend. I looked into source files that throw function compilation problems, but I am unable to find a pattern and fix it. We can go by for now with re-launching editor every time our code changes, as LLVM toolset, and LLVM-compatible programs save us much more time, and we have do-it-in-Blueprints-first approach that we don't need it as much, but it certainly comes in handy. We very much would like to see it's working as it allows to be OS-agnostic, as Clang is on Windows, natively on Linux, and Apple has it's own "custom" version of Clang. I'm providing all .sh files, as you can see on them, whenever it is a unity or not build doesn't matter, and frontend exits on the same functions. Git diff of changes that I've performed, and both unity and non-unity compilation log, so you can precisely see how it compiled (or to be exact failed to compile). If you are short on hands, I could maybe chip in something (This would be my first time dabbling in compiler code, So I just assume that even if I started doing something, someone more in-the-know would finish it faster than me) but to do it, I'd need to get some jumpstart, and by this I mean some links to documentation of how to read those errors, and what to keep in mind while doing this, but If I have to read C++ official documentation things could go rough, as I've never read it [Clang-issue.zip](https://github.com/llvm/llvm-project/files/8987316/Clang-issue.zip) .