Seems reasonable to me. I'm surprised CMake doesn't have a general facility for escaping paths used in cflags. I suppose users generally don't add paths, just flags.

• assert

That's old code... I guess nobody has tried building LLVM with asan on Windows in a while.

There's another use case worth considering here, which is -fmodules-debuginfo. In that situation, a standalone object file is emitted that contains nothing but DWARF .debug_info sections, and the object is fed to the linker. If the linker drops it with --gc-sections, it's not going to work. Here's a transcript of me trying to use this feature:
https://reviews.llvm.org/P8150

lgtm with some nits

This morning I didn't know why this was interesting, but then @inglorion told me about the new .init_array + reloc-none GC sections strategy. I still have concerns about dropping debug info as LLD is doing now, but I'll pass them along separately.

In D61698#1502875, @ruiu wrote:

By the way, the other idea I had in mind for this problem is to define a new integer-ish class that can hold only a 2^n value, by overriding cast from/to an integer. I don't know if that is a good idea or a disgusting idea, though.

Sorry for the broken patch, I forgot to be extra careful on the codepaths I can't test, clearly. Does this work?

• various fixes to RTEMS / Myriad codepath

Ping, seems like a good simplification.

I was going to ask for a test case that makes local constants, but I see clang generates pretty weird metadata for this test case:

int f(int x) {
  static const int MyConst = 42;
  return x + MyConst;
}

That's supposed to generate a local S_CONSTANT, but we don't populate the DIExpression properly. Hm.

I think there's an issue with the whole idea of dropping .debug_info from objects without live sections. Consider:

// a.cpp
int main() {
  return f();
}
// b.cpp
struct Foo {
  int x, y;
};
int f() {
  volatile Foo var;
  var.x = 13;
  var.y = 42;
  return var.x + var.y;
}

When compiled and linked with thinlto, f is imported into the a.cpp TU, but the full definition of the Foo type remains in the b.cpp TU, because importing the full type would be expensive and wasteful.

In D61960#1503681, @rnk wrote:

Normally I would go ahead and land this, but I think I should let somebody who has more ownership over the ARM64 backend take a look first.

Normally I would go ahead and land this, but I think I should let somebody who has more ownership over the ARM64 backend take a look first.

lgtm

Windows parts look good! Sorry for the delay.

lgtm, thanks for adding the diagnostic!

In rC354839#649303, @martong wrote:

This is a destructive revert commit which does not simply revert the mentioned r354832 commit. Besides reverting that commit, it does remove a bunch of unrelated test files! E.g. cfe/trunk/test/ASTMerge/var/Inputs/var1.c. Could you please investigate and restore the unrelated test files?

lgtm

ptal

@echristo mentioned that this patch was also causing issues in another freestanding environment where _MSC_VER was defined, but no corecrt.h header existed on the system. So, I think it's likely that we don't want to do this in the long run. The compiler-provided headers and system headers are already too entangled, and this is one more dependency between them.

• remove [gs]etLog2Alignment

In the long term, I think we want to hoist section characteristics up into chunk, and that's going to create the 8192 max alignment constraint. This is just a first step to refactor the code to use the accessors.

Thanks, I would like to do this for the sanitizers as well, since this is a constant pain point for users, who have to come up with an architecture-dependent filename (clang_rt.asan-dynamic-$arch.lib or something).

I noticed you can see these warnings by compiling with /Wall, by the way. It's MSVC's equivalent of Clang's -Weverything. So I guess that's the new next level after /W4.

I confirmed that this hasn't been in the default warning set for 2017, which these days I guess is the oldest we support. Feel free to remove other off-by-default warnings, I don't think we need to proactively defend against users adding them to their own cflags, or using old msvc versions.

In D61621#1496978, @vsapsai wrote:

Didn't use -fms-compatibility in the test and it seems to be working fine.
Don't know if it is accidental and if I should add the flag.

In D61646#1495778, @phosek wrote:

This change broke our build which is using Clang and the x86_64-windows-msvc target to cross-compile our EFI bootloader. Now the compilation fails because it cannot find Windows headers (as expected). We're probably going to work around the issue by setting -U_MSC_VER, but I'm going to start a discussion whether we should maybe define a new target for EFI.

Nice.

Well, I'm curious what meaning GCC ascribes to a medium code model for COFF. Do they generate code to allow PE images larger than 2GB, or is it more like the ELF small code model, where they assume everything can be reached with RIP relative addressing?

In D61646#1495517, @thakis wrote:

Did I get the bug right that this adds almost 400kB to every file that includes stddef.h?

I'm a little concerned about overloading the code model this way. Currently we have the levels tiny, small, medium, large. Default is the same as small in every way so far as I'm aware. On COFF, the code models are currently untested, so far as I know, and I've been assuming that any usage of the non-small code model more or less implies ELF, and that it would be reasonable to emit wrong code or a fatal error on such codepaths during LLVM codegen. Maybe we have some use cases for the other code models in MCJIT.

lgtm, too

Presumably test/Bitcode/upgrade-global-ctors.ll.bc should remain unchanged, i.e. not be regenerated? We want it to use the pre-upgraded, two field form.

lgtm, thanks

I see one more instance of unprotected asm usage, but it's under #ifdef _MSC_VER, it's the HLE extensions. Can you protect that, and then add another RUN line with a windows triple and -fms-extensions to test it?