================
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/
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
