[PATCH] [CodeGen] Emit Constants, not ScalarExprs, for "immediate"-constrained inlineasm arguments.
daniel.sanders at imgtec.com
Wed Jun 10 09:26:12 PDT 2015
> > 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?");
> 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
> 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.
More information about the cfe-commits