LGTM > > In http://reviews.llvm.org/D10255#184893, @dsanders wrote:
> > 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. > > I'm not too familiar with inline-asm handling, but could it be because it > supports symbols, and symbols aren't really immediates (e.g. aren't usually > supported in non-addressing mode > contexts) ? Possibly, but it could also be unintentional and just happened to turn out ok in the end. I noticed it while removing a hardcoded 'm' that caused all memory constraints to behave like 'm'. ================ 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?"); + } ---------------- ab wrote: > dsanders wrote: > > Should this cause an error to be emitted on release builds too? > I'm not sure, my understanding of clang is, by the time we get to IR gen, > source errors have already been caught and reported. I've just tried it on X86 with 'I' and it does emit an error so something else must be handling it. ================ 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 ---------------- ab wrote: > dsanders wrote: > > Sanitizers aren't always available. It's possible we need a 'REQUIRES:' > > line to disable the test for some buildbots. > That makes sense for runtime tests, but is it necessary for codegen tests? I > thought all of IR gen was always available, much like targets. I seem to remember it reporting errors early for some reason. I've just tried it and codegen is working without working sanitizers being available. 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
