[PATCH] CodeGen: implement __emit intrinsic

[Saleem Abdulrasool]

================
Comment at: include/clang/Basic/BuiltinsARM.def:88
@@ -87,1 +87,3 @@
 // MSVC
+LANGBUILTIN(__emit, "vIUiC", "", MS_COMPAT)
+
----------------
rnk wrote:
> Honestly, I think __emit is in "conforming extensions" territory, aka -fms-extensions. So I think we can drop the -fms-compatibility part.
Ugh, I *really* hate the naming.  It gets me every time.  Yes, you are right, this is a conforming extensions, and thus -fms-extensions not -fms-compatibility.  One of these days, Ill get it right on the first try.  This is not that day :-(.

================
Comment at: lib/CodeGen/CGBuiltin.cpp:3171-3172
@@ +3170,4 @@
+  if (BuiltinID == ARM::BI__emit) {
+    assert(getTarget().getTriple().isOSWindows() &&
+           "__emit is only supported on Windows on ARM");
+    assert(getTarget().getTriple().getArch() == llvm::Triple::thumb &&
----------------
rnk wrote:
> There are evil users out there using -fms-compatibility -fms-extensions on non-Windows platforms. We shouldn't assert. I think removing the assert would be fine.
Ick; okay.

================
Comment at: lib/CodeGen/CGBuiltin.cpp:3173-3174
@@ +3172,4 @@
+           "__emit is only supported on Windows on ARM");
+    assert(getTarget().getTriple().getArch() == llvm::Triple::thumb &&
+           "Windows on ARM only supports thumb");
+
----------------
rnk wrote:
> Likewise, we shouldn't assert here. If we want to reject it, we should emit a diagnostic in sema. Since that's probably more work than just implementing ARM support, I vote for that. Just check the triple and pick between 32-bit ARM instrs and 16-bit thumb instrs (.inst vs .inst.n).
Sure; that can be useful if we were to ever support WinCE.

================
Comment at: lib/CodeGen/CGBuiltin.cpp:3180-3181
@@ +3179,4 @@
+    APSInt Value;
+    if (!E->getArg(0)->EvaluateAsInt(Value, CGM.getContext()))
+      report_fatal_error("constant value required!");
+    uint64_t ZExtValue = Builder.getInt(Value)->getZExtValue();
----------------
rnk wrote:
> Maybe llvm_unreachable, since this is an internal error if sema did the right checks?
Yeah, that sounds reasonable.

================
Comment at: lib/CodeGen/CGBuiltin.cpp:3185
@@ +3184,3 @@
+    llvm::InlineAsm *Emit =
+        InlineAsm::get(FTy, ".inst.n 0x" + utohexstr(ZExtValue), "",
+                       /*SideEffects=*/true);
----------------
rnk wrote:
> What happens here if the user passes a constant larger than the instruction size, such as 0xdeadbeef for thumb? MSDN says we should truncate, but it looks like LLVM will emit an error in the backend. I guess that's OK.
Thats a good point; fixed and added a test.

================
Comment at: test/CodeGen/builtins-arm-msvc-compat-error.c:1
@@ +1,2 @@
+// RUN: not %clang_cc1 -triple thumbv7-windows -fms-compatibility -emit-llvm -o /dev/null %s 2>&1 \
+// RUN:     | FileCheck %s -check-prefix CHECK
----------------
rnk wrote:
> This can be a -verify test now, and you can add a positive test for __emit(0xdeadbeef).
Ah, nice.  Done.

http://reviews.llvm.org/D3489

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/