[windows] Don't inline fieldFromInstruction on Windows

[stella.stamenova]

Summary

The VS compiler (on Windows) has a bug which results in fieldFromInstruction being optimized out in some circumstances. This only happens in *release no debug info* builds that have assertions *turned off* - in all other situations the function is not inlined, so the functionality is correct. All of the bots have assertions turned on, so this path is not regularly tested. The workaround is to not inline the function on Windows - if the bug is fixed in a later release of the VS compiler, the noinline specification can be removed.

The test that consistently reproduces this is Lanai v11.txt test.

[zturner]

Does this currently affect all shipping versions of msvc? How come I’ve
never encountered it? Am i not running this test?

[stella.stamenova]

I don't know if it impacts all shipping versions since we only test with the latest versions. I would suspect that you either build with assertions set to ON or you include debug information when you build. Here's the CMAKE command line that reproduces this:

-DCMAKE_BUILD_TYPE=Release -G "Visual Studio 15 2017 Win64" -Thost=x64 -DLLVM_ENABLE_ASSERTIONS=OFF

[zturner]

I don’t use the vs generator and I don’t explicitly pass enable assertions

off (isn’t that the default for release builds)? I do build release

builds with no debug info though.

Note that clang-cl probably doesn’t have this bug, so the check should
probably be _MSC_VER && !clang

[stella.stamenova]

Do you build all targets? You could try explicitly running the test to see if it passes. It's a consistent repro for us.

[zturner]

Ahh I always pass LLVM_TARGETS_TO_BUILD=X86, that must be it (btw, that
cuts build time down a lot)

[stella.stamenova]

Follow Zachary's advise and do not force no inline in the case of clang.

[zturner]

Can you add a comment explaining why this is necessary? Other than that,
lgtm. Feel free to submit after adding the comment

[stella.stamenova]

I also filed a bug to reference.

utils/TableGen/FixedLenDecoderEmitter.cpp

LLVM_DEBUG({
     });
 #endif
 
   return true;
 }
 
 // emitFieldFromInstruction - Emit the templated helper function
 // fieldFromInstruction().
+// On Windows we make sure that this function is not inlined when
+// using the VS compiler. It has a bug which causes the function
+// to be optimized out in some circustances. See llvm.org/pr38292
 static void emitFieldFromInstruction(formatted_raw_ostream &OS) {
   OS << "// Helper function for extracting fields from encoded instructions.\n"
      << "template<typename InsnType>\n"
+     << "#if defined(_MSC_VER) && !defined(__clang__)\n"
+     << "__declspec(noinline)\n"
+     << "#endif\n"
      << "static InsnType fieldFromInstruction(InsnType insn, unsigned startBit,\n"
      << "                                     unsigned numBits) {\n"
      << "    assert(startBit + numBits <= (sizeof(InsnType)*8) &&\n"
      << "           \"Instruction field out of bounds!\");\n"
      << "    InsnType fieldMask;\n"
      << "    if (numBits == sizeof(InsnType)*8)\n"
      << "      fieldMask = (InsnType)(-1LL);\n"
      << "    else\n"
      << "      fieldMask = (((InsnType)1 << numBits) - 1) << startBit;\n"