lgtm, looks like we already do this elsewhere in the file.

I don't know if anyone cares about the msan+zlib case, I just think it's important that FORCE_ON either enables zlib or raises an error. Looks like this patch ensures that.

In D85122#2200616, @sorear wrote:

For completeness I just filed https://bugs.llvm.org/show_bug.cgi?id=47021 for this issue. I missed the "contact the release manager directly" as a viable alternative on my first reading, so that was probably unnecessary, apologies if this creates a problem.

Looks great, thanks! I put a suggestion for different wording of the note, but I'll leave it up to you to decide.

Brian: I think this is good to go, right?

In D84825#2201313, @MaskRay wrote:

@hans Pushed to trunk. One test does not exist in release/11.x so this needs to resolve a conflict. I can push to release/11.x if you authorize it:)

In D85122#2198934, @Petar.Avramovic wrote:

@hans, could you please commit this patch for 11.x release?

In D85242#2198707, @mstorsjo wrote:

@hans - WDYT about backporting this one? It's a little bit risky if the check would turn out to be too strict, but there's been no false positives in my test build corpus (which includes a fair bit of assembly source); the upside is for anybody working on porting (mainly assembly) code that isn't originally written for arm64/windows - as it previously wouldn't error out at build time on constructs that don't exist on windows and won't work as intended at runtime.

Thanks! This looks promising.

In D85183#2195426, @mstorsjo wrote:

@hans - ok with this for the branch as well? This fixes a recently reported bug by a user.

In D84757#2187544, @hans wrote:

In D84757#2185837, @mstorsjo wrote:

@hans - @SquallATF wants this one backported to the 11.x release branch, which I think is reasonable - but maybe after waiting for a day or two in case there's any surprise.

Thanks! I've put it on my list.

In D85210#2194427, @mstorsjo wrote:

@hans - I think this one might be cherrypick-worthy

In D81776#2194429, @mstorsjo wrote:

@hans - I think this one might be cherrypick-worthy as well

In general when -ffoo is supported by clang-cl, -fno-foo should be too, so this makes sense.

In D84782#2191429, @yamauchi wrote:

In D84782#2191243, @MaskRay wrote:

@yamauchi Do we need cd890944ad344b1b8cac58332ab11c9eec6b61e9 and 3d6f53018f845e893ad34f64ff2851a2e5c3ba1d in https://github.com/llvm/llvm-project/tree/release/11.x ?

I think it'd be good to have them, if possible, though it's a latent, non-recent bug.

Reverted in 3ab01550b632dad46f9595d74855749557ffd25c

In D79219#2153585, @phosek wrote:

In D79219#2152747, @labath wrote:

I wouldn't mind separate (internal) cache variable, though I am somewhat surprised by this problem. A (non-cache) variable set in one of the parent scopes should still take precedence over a cache variable with the same name. And since config-ix.cmake is included from the top CMakeLists.txt, the value it defines should be available everywhere. Was this a problem for the regular build, or only for some of the more exotic build setups that don't start with llvm/CMakeLists.txt ?

Never mind, it's working as expected. The problem is that we disable zlib detection on Windows which I missed before. I'm not sure why that's the case, I tested this on Windows and it seems to be working fine, but for now I've kept the existing behavior, we can consider enabling zlib on Windows in a follow up change.

From my point of view, I think it would be fine to only support rpmalloc in this patch. One aspect that Russell touched on is that if we support a bunch of options, it's not clear how well that support will be tested. Supporting only rpmalloc would also simplify the patch a little, and small incremental change is always good I think.

In D84756#2185834, @mstorsjo wrote:

@hans - @SquallATF wants this one backported to the 11.x release branch, which I think is reasonable.

In D84757#2185837, @mstorsjo wrote:

@hans - @SquallATF wants this one backported to the 11.x release branch, which I think is reasonable - but maybe after waiting for a day or two in case there's any surprise.

Thanks! This fixes the link error I was hitting.

lhames: Ping?

In D84701#2177968, @hans wrote:

In D84701#2177048, @mstorsjo wrote:

Actually, this does fix a regression in one sense. Prior to LLD 9, the IMAGE_DLL_CHARACTERISTICS_NO_SEH flag did end up set even in mingw configurations, so ignoring the --no-seh option was just fine. But for LLD 9, the safeSEH mechanism implementation was improved, and this flag no longer was set in mingw configurations.

Thanks, will merge once it lands.

Thanks! Can you add a note in docs/ReleaseNotes.rst explaining that this is expected to change in 12, what that means, what flags one should use to try it, and what problems to look out for?

In D84701#2177048, @mstorsjo wrote:

Actually, this does fix a regression in one sense. Prior to LLD 9, the IMAGE_DLL_CHARACTERISTICS_NO_SEH flag did end up set even in mingw configurations, so ignoring the --no-seh option was just fine. But for LLD 9, the safeSEH mechanism implementation was improved, and this flag no longer was set in mingw configurations.

Pushed to 11.x as 00ed5355e45b4d7e84366619dbbbb9df9b4aa816

Pushed to 11.x as 9dea95b78082a1d9739a60be8a40d721788e4447
Please shout if any problems come up.

In rG7d076e19e31a2a32e357cbdcf0183f88fe1fb0fb#937325, @hans wrote:

Nemanja: should we cherry-pick this to the 11.x release branch, since 1fed131660b2 landed before? Are there other fixes that should also be cherry-picked?

Nemanja: should we cherry-pick this to the 11.x release branch, since 1fed131660b2 landed before? Are there other fixes that should also be cherry-picked?

Thanks!

Pushed to 11.x as 833f8c958601bb640ba6a25d627c1dc58dad14d2. Please let me know if there are any follow-ups.

In that case, I've enabled it again using add_compile_options instead of add_definitions. I've got my finger hovering over the button to revert.

lgtm, thanks!

In D84244#2167602, @logan-5 wrote:

Thanks for reverting--I agree that that's the right move at this point.

Pretty much totally out my depth here, and I don't have a way of debugging the Windows issue, so I'm not sure how to proceed. I'm tempted to just let this whole thing rest for now and maybe try again sometime in the future.

This caused asserts in Chromium's coverage builds: https://bugs.chromium.org/p/chromium/issues/detail?id=1108352
I've reverted it in the meantime (238bbd48c5a5f84deca36e5df980241578f7c1df).