Prevent building with MSVC 14.24

[thakis]

Summary

MSVC 14.24 miscompiles some of LLVM's code, which makes at least these tests fail:

LLVM :: MC/MachO/gen-dwarf-cpp.s
LLVM :: MC/MachO/gen-dwarf-macro-cpp.s
LLVM :: MC/MachO/gen-dwarf-producer.s
LLVM :: MC/MachO/gen-dwarf.s

It seems better to diagnose that at build time. Since both the previous and the next version have a fix, this might be good enough and we might not need a real workaround. (We ran into this at https://crbug.com/1045948)

[hans]

Seems reasonable to me. Should we comment on the upstream bug that this breaks LLVM? I see there's a reduced repro, but I don't see LLVM mentioned.

On second thought, while it's not strictly our problem, what is a user who runs into this supposed to do? AFAIK, Microsoft doesn't provide access to old compiler versions within the same family (2019), so unless you have an old install lying around or want to try the next preview, what to do? Would it be better to selectively disable optimization of the miscompiled function?

llvm/include/llvm/Support/Compiler.h

31

Maybe link to http://lists.llvm.org/pipermail/llvm-dev/2020-January/138496.html ?

[thakis]

In D73550#1844539, @hans wrote:

Seems reasonable to me. Should we comment on the upstream bug that this breaks LLVM? I see there's a reduced repro, but I don't see LLVM mentioned.

Sounds like it's already fixed upstream, so if the motivation is to help prioritize the bug then that's not necessary.

On second thought, while it's not strictly our problem, what is a user who runs into this supposed to do? AFAIK, Microsoft doesn't provide access to old compiler versions within the same family (2019), so unless you have an old install lying around or want to try the next preview, what to do?

You can get a newer MSVC, or you can use clang-cl as host compiler :)

Would it be better to selectively disable optimization of the miscompiled function?

I think this change here is better than not changing anything, and I don't want to spend more time on this than I already have.

[hans]

In D73550#1844563, @thakis wrote:

In D73550#1844539, @hans wrote:

Seems reasonable to me. Should we comment on the upstream bug that this breaks LLVM? I see there's a reduced repro, but I don't see LLVM mentioned.

Sounds like it's already fixed upstream, so if the motivation is to help prioritize the bug then that's not necessary.

The idea was to highlight that this breaks a popular open-source project and that maybe they should include it in their testing. I'll add a note on the bug.

On second thought, while it's not strictly our problem, what is a user who runs into this supposed to do? AFAIK, Microsoft doesn't provide access to old compiler versions within the same family (2019), so unless you have an old install lying around or want to try the next preview, what to do?

You can get a newer MSVC, or you can use clang-cl as host compiler :)

The newer MSVC hasn't been released yet though.

Would it be better to selectively disable optimization of the miscompiled function?

I think this change here is better than not changing anything, and I don't want to spend more time on this than I already have.

Agreed this is better than nothing. lgtm.

[aganea]

llvm/include/llvm/Support/Compiler.h

32

Maybe better to indicate which version of VS this corresponds to.

#error "MSVC 19.24 (Visual Studio 2019 version 16.4) is known to miscompile LLVM. Please upgrade."

[aganea]

In D73550#1844570, @hans wrote:

The newer MSVC hasn't been released yet though.

Shouldn't be too long, it's already available on the preview program: https://docs.microsoft.com/en-us/visualstudio/releases/2019/release-notes-preview

[mehdi_amini]

I have concern with this patch: this is a big hammer that blocks building *anything* using LLVM even if it does not involve any of the code path that is problematic.

I would rather see this implemented as a CMake check and a flag that can override/disable the check.

[aganea]

In D73550#1848010, @mehdi_amini wrote:

I have concern with this patch: this is a big hammer that blocks building *anything* using LLVM even if it does not involve any of the code path that is problematic.

How can you tell which parts of the code are affected by this miscompilation? I've seen occasionally crashes in opt which only occur for this version of MSVC. I fear that people would lose time investigating indirect bugs caused by this.

I would rather see this implemented as a CMake check and a flag that can override/disable the check.

Do you see this as an issue for end-users/consumers of LLVM? Or just for LLVM developers? One can simply disable this check locally, given that 16.5 will be released soon; or use clang-cl 9.0.1.

[mehdi_amini]

In D73550#1848076, @aganea wrote:

In D73550#1848010, @mehdi_amini wrote:

I have concern with this patch: this is a big hammer that blocks building *anything* using LLVM even if it does not involve any of the code path that is problematic.

How can you tell which parts of the code are affected by this miscompilation? I've seen occasionally crashes in opt which only occur for this version of MSVC. I fear that people would lose time investigating indirect bugs caused by this.

My project tests aren't broken :)

People would have to explicitly disable the CMake check so they would take it upon themselves to investigate bugs.

I would rather see this implemented as a CMake check and a flag that can override/disable the check.

Do you see this as an issue for end-users/consumers of LLVM? Or just for LLVM developers? One can simply disable this check locally, given that 16.5 will be released soon; or use clang-cl 9.0.1.

I know one current issue for a consumer of LLVM (but this is just one example): the TensorFlow CI is broken on Windows (it is using MLIR for its TensorFlow lite converter which is pulling the LLVM Support library (and nothing else I suspect).

Of course this affects *every* consumer of MLIR, but I haven't seen an evidence of a problem there.

[mehdi_amini]

Sent: https://reviews.llvm.org/D73677

llvm/include/llvm/Support/Compiler.h

 
 #ifdef __cplusplus
 #include <new>
 #endif
 #include <stddef.h>
 
 #if defined(_MSC_VER)
 #include <sal.h>
+
+#if _MSC_VER == 1924
+// See https://developercommunity.visualstudio.com/content/problem/845933/miscompile-boolean-condition-deduced-to-be-always.html
+// and thread "[llvm-dev] Longstanding failing tests - clang-tidy, MachO, Polly"
+// on llvm-dev Jan 21-23 2020.
+#error "MSVC 19.24 version of MSVC is known to miscompile LLVM."
+#endif
 #endif
 
 #ifndef __has_feature
 # define __has_feature(x) 0
 #endif
 
 #ifndef __has_extension
 # define __has_extension(x) 0