[llvm] 8484bab - [LangRef] Require elementtype attribute for indirect inline asm operands
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 4 01:02:15 PST 2022
Author: Nikita Popov
Date: 2022-01-04T10:02:06+01:00
New Revision: 8484bab9cd5e5af11acf64e68c2f82e250e08dbe
URL: https://github.com/llvm/llvm-project/commit/8484bab9cd5e5af11acf64e68c2f82e250e08dbe
DIFF: https://github.com/llvm/llvm-project/commit/8484bab9cd5e5af11acf64e68c2f82e250e08dbe.diff
LOG: [LangRef] Require elementtype attribute for indirect inline asm operands
Indirect inline asm operands may require the materialization of a
memory access according to the pointer element type. As this will
no longer be available with opaque pointers, we require it to be
explicitly annotated using the elementtype attribute, for example:
define void @test(i32* %p, i32 %x) {
call void asm "addl $1, $0", "=*rm,r"(i32* elementtype(i32) %p, i32 %x)
ret void
}
This patch only includes the LangRef change and Verifier updates to
allow adding the elementtype attribute in this position. It does not
yet enforce this, as this will require changes on the clang side
(and test updates) first.
Something I'm a bit unsure about is whether we really need the
elementtype for all indirect constraints, rather than only indirect
register constraints. I think indirect memory constraints might not
strictly need it (though the backend code is written in a way that
does require it). I think it's okay to just make this a general
requirement though, as this means we don't need to carefully deal
with multiple or alternative constraints. In addition, I believe
that MemorySanitizer benefits from having the element type even in
cases where it may not be strictly necessary for normal lowering
(https://github.com/llvm/llvm-project/blob/cd2b050fa4995b75b9c36fae16c0d9f105b67585/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp#L4066).
Differential Revision: https://reviews.llvm.org/D116531
Added:
llvm/test/Verifier/inline-asm-indirect-operand.ll
Modified:
llvm/docs/LangRef.rst
llvm/lib/IR/Verifier.cpp
llvm/test/Verifier/elementtype.ll
Removed:
################################################################################
diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index 8c72e3255ab51..389c90937bb00 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -4568,6 +4568,9 @@ functionality provides, compared to writing the store explicitly after the asm
statement, and it can only produce worse code, since it bypasses many
optimization passes. I would recommend not using it.)
+Call arguments for indirect constraints must have pointer type and must specify
+the :ref:`elementtype <attr_elementtype>` attribute to indicate the pointer
+element type.
Clobber constraints
"""""""""""""""""""
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index fb7c423e54e2c..9ce37db9ea6c4 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -551,11 +551,12 @@ class Verifier : public InstVisitor<Verifier>, VerifierSupport {
void checkUnsignedBaseTenFuncAttr(AttributeList Attrs, StringRef Attr,
const Value *V);
void verifyFunctionAttrs(FunctionType *FT, AttributeList Attrs,
- const Value *V, bool IsIntrinsic);
+ const Value *V, bool IsIntrinsic, bool IsInlineAsm);
void verifyFunctionMetadata(ArrayRef<std::pair<unsigned, MDNode *>> MDs);
void visitConstantExprsRecursively(const Constant *EntryC);
void visitConstantExpr(const ConstantExpr *CE);
+ void verifyInlineAsmCall(const CallBase &Call);
void verifyStatepoint(const CallBase &Call);
void verifyFrameRecoverIndices();
void verifySiblingFuncletUnwinds();
@@ -1870,7 +1871,8 @@ void Verifier::checkUnsignedBaseTenFuncAttr(AttributeList Attrs, StringRef Attr,
// Check parameter attributes against a function type.
// The value V is printed in error messages.
void Verifier::verifyFunctionAttrs(FunctionType *FT, AttributeList Attrs,
- const Value *V, bool IsIntrinsic) {
+ const Value *V, bool IsIntrinsic,
+ bool IsInlineAsm) {
if (Attrs.isEmpty())
return;
@@ -1913,8 +1915,10 @@ void Verifier::verifyFunctionAttrs(FunctionType *FT, AttributeList Attrs,
if (!IsIntrinsic) {
Assert(!ArgAttrs.hasAttribute(Attribute::ImmArg),
"immarg attribute only applies to intrinsics",V);
- Assert(!ArgAttrs.hasAttribute(Attribute::ElementType),
- "Attribute 'elementtype' can only be applied to intrinsics.", V);
+ if (!IsInlineAsm)
+ Assert(!ArgAttrs.hasAttribute(Attribute::ElementType),
+ "Attribute 'elementtype' can only be applied to intrinsics"
+ " and inline asm.", V);
}
verifyParameterAttrs(ArgAttrs, Ty, V);
@@ -2141,6 +2145,33 @@ bool Verifier::verifyAttributeCount(AttributeList Attrs, unsigned Params) {
return Attrs.getNumAttrSets() <= Params + 2;
}
+void Verifier::verifyInlineAsmCall(const CallBase &Call) {
+ const InlineAsm *IA = cast<InlineAsm>(Call.getCalledOperand());
+ unsigned ArgNo = 0;
+ for (const InlineAsm::ConstraintInfo &CI : IA->ParseConstraints()) {
+ // Only deal with constraints that correspond to call arguments.
+ bool HasArg = CI.Type == InlineAsm::isInput ||
+ (CI.Type == InlineAsm::isOutput && CI.isIndirect);
+ if (!HasArg)
+ continue;
+
+ if (CI.isIndirect) {
+ const Value *Arg = Call.getArgOperand(ArgNo);
+ Assert(Arg->getType()->isPointerTy(),
+ "Operand for indirect constraint must have pointer type",
+ &Call);
+
+ // TODO: Require elementtype attribute here.
+ } else {
+ Assert(!Call.paramHasAttr(ArgNo, Attribute::ElementType),
+ "Elementtype attribute can only be applied for indirect "
+ "constraints", &Call);
+ }
+
+ ArgNo++;
+ }
+}
+
/// Verify that statepoint intrinsic is well formed.
void Verifier::verifyStatepoint(const CallBase &Call) {
assert(Call.getCalledFunction() &&
@@ -2364,7 +2395,7 @@ void Verifier::visitFunction(const Function &F) {
bool IsIntrinsic = F.isIntrinsic();
// Check function attributes.
- verifyFunctionAttrs(FT, Attrs, &F, IsIntrinsic);
+ verifyFunctionAttrs(FT, Attrs, &F, IsIntrinsic, /* IsInlineAsm */ false);
// On function declarations/definitions, we do not support the builtin
// attribute. We do not check this in VerifyFunctionAttrs since that is
@@ -2779,6 +2810,7 @@ void Verifier::visitCallBrInst(CallBrInst &CBI) {
Assert(ArgBBs.count(BB), "Indirect label missing from arglist.", &CBI);
}
+ verifyInlineAsmCall(CBI);
visitTerminator(CBI);
}
@@ -3123,7 +3155,7 @@ void Verifier::visitCallBase(CallBase &Call) {
}
// Verify call attributes.
- verifyFunctionAttrs(FTy, Attrs, &Call, IsIntrinsic);
+ verifyFunctionAttrs(FTy, Attrs, &Call, IsIntrinsic, Call.isInlineAsm());
// Conservatively check the inalloca argument.
// We have a bug if we can find that there is an underlying alloca without
@@ -3316,6 +3348,9 @@ void Verifier::visitCallBase(CallBase &Call) {
"debug info must have a !dbg location",
Call);
+ if (Call.isInlineAsm())
+ verifyInlineAsmCall(Call);
+
visitInstruction(Call);
}
diff --git a/llvm/test/Verifier/elementtype.ll b/llvm/test/Verifier/elementtype.ll
index e092e0f54c93e..22bfe720c7482 100644
--- a/llvm/test/Verifier/elementtype.ll
+++ b/llvm/test/Verifier/elementtype.ll
@@ -14,7 +14,7 @@ define void @type_mismatch2() {
ret void
}
-; CHECK: Attribute 'elementtype' can only be applied to intrinsics.
+; CHECK: Attribute 'elementtype' can only be applied to intrinsics and inline asm.
define void @not_intrinsic() {
call void @some_function(i32* elementtype(i32) null)
ret void
diff --git a/llvm/test/Verifier/inline-asm-indirect-operand.ll b/llvm/test/Verifier/inline-asm-indirect-operand.ll
new file mode 100644
index 0000000000000..4be6f50b9ef54
--- /dev/null
+++ b/llvm/test/Verifier/inline-asm-indirect-operand.ll
@@ -0,0 +1,52 @@
+; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s
+
+define void @okay(i32* %p, i32 %x) {
+ call void asm "addl $1, $0", "=*rm,r"(i32* elementtype(i32) %p, i32 %x)
+ ret void
+}
+
+; CHECK: Attribute 'elementtype' type does not match parameter!
+; CHECK-NEXT: call void asm "addl $1, $0", "=*rm,r"(i32* elementtype(i64) %p, i32 %x)
+define void @wrong_element_type(i32* %p, i32 %x) {
+ call void asm "addl $1, $0", "=*rm,r"(i32* elementtype(i64) %p, i32 %x)
+ ret void
+}
+
+; CHECK: Operand for indirect constraint must have pointer type
+; CHECK-NEXT: call void asm "addl $1, $0", "=*rm,r"(i32 %p, i32 %x)
+define void @not_pointer_arg(i32 %p, i32 %x) {
+ call void asm "addl $1, $0", "=*rm,r"(i32 %p, i32 %x)
+ ret void
+}
+
+; CHECK: Elementtype attribute can only be applied for indirect constraints
+; CHECK-NEXT: call void asm "addl $1, $0", "=*rm,r"(i32* %p, i32* elementtype(i32) %x)
+define void @not_indirect(i32* %p, i32* %x) {
+ call void asm "addl $1, $0", "=*rm,r"(i32* %p, i32* elementtype(i32) %x)
+ ret void
+}
+
+; CHECK: Operand for indirect constraint must have pointer type
+; CHECK-NEXT: invoke void asm "addl $1, $0", "=*rm,r"(i32 %p, i32 %x)
+define void @not_pointer_arg_invoke(i32 %p, i32 %x) personality i8* null {
+ invoke void asm "addl $1, $0", "=*rm,r"(i32 %p, i32 %x)
+ to label %cont unwind label %lpad
+
+lpad:
+ %lp = landingpad i32
+ cleanup
+ ret void
+
+cont:
+ ret void
+}
+
+; CHECK: Operand for indirect constraint must have pointer type
+; CHECK-NEXT: callbr void asm "addl $1, $0", "=*rm,r"(i32 %p, i32 %x)
+define void @not_pointer_arg_callbr(i32 %p, i32 %x) {
+ callbr void asm "addl $1, $0", "=*rm,r"(i32 %p, i32 %x)
+ to label %cont []
+
+cont:
+ ret void
+}
More information about the llvm-commits
mailing list