I'm wondering if the assert ought to be a normal error message and I think we may need a REQUIRES for buildbots lacking sanitizer support but apart from that this patch LGTM.
Just to mention it, there's something odd about the 'i' constraint in the backends but I haven't had chance to investigate it yet. For some reason, it has to be handled in *DAGToDAGISel::SelectInlineAsmMemoryOperand() despite not being a memory operand. ================ Comment at: lib/CodeGen/CGStmt.cpp:1759-1760 @@ +1758,4 @@ + return llvm::ConstantInt::get(getLLVMContext(), Result); + assert(!Info.requiresImmediateConstant() && + "Required-immediate inlineasm arg isn't constant?"); + } ---------------- Should this cause an error to be emitted on release builds too? ================ Comment at: test/CodeGen/inline-asm-immediate-ubsan.c:2 @@ +1,3 @@ +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s \ +// RUN: -fsanitize=signed-integer-overflow \ +// RUN: | FileCheck %s --check-prefix=CHECK ---------------- Sanitizers aren't always available. It's possible we need a 'REQUIRES:' line to disable the test for some buildbots. http://reviews.llvm.org/D10255 EMAIL PREFERENCES http://reviews.llvm.org/settings/panel/emailpreferences/ _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
