[PATCH] CodeGen: implement __emit intrinsic

Reid Kleckner rnk at google.com
Tue Dec 16 10:43:38 PST 2014


Almost there.


================
Comment at: include/clang/Basic/BuiltinsARM.def:88
@@ -87,1 +87,3 @@
 // MSVC
+LANGBUILTIN(__emit, "vIUiC", "", MS_COMPAT)
+
----------------
Honestly, I think __emit is in "conforming extensions" territory, aka -fms-extensions. So I think we can drop the -fms-compatibility part.

================
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 &&
----------------
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.

================
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");
+
----------------
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).

================
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();
----------------
Maybe llvm_unreachable, since this is an internal error if sema did the right checks?

================
Comment at: lib/CodeGen/CGBuiltin.cpp:3185
@@ +3184,3 @@
+    llvm::InlineAsm *Emit =
+        InlineAsm::get(FTy, ".inst.n 0x" + utohexstr(ZExtValue), "",
+                       /*SideEffects=*/true);
----------------
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.

================
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
----------------
This can be a -verify test now, and you can add a positive test for __emit(0xdeadbeef).

http://reviews.llvm.org/D3489

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






More information about the cfe-commits mailing list