[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

4 files changed, 97 insertions, 7 deletions

diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index 8c72e3255ab5..389c90937bb0 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 fb7c423e54e2..9ce37db9ea6c 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -551,11 +551,12 @@ private:
   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 e092e0f54c93..22bfe720c748 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 000000000000..4be6f50b9ef5
--- /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
+}