Assert on polymorphic pointer intrinsic param

[thopre]

Summary

Opaque pointers cannot be polymorphic on the pointed type given their
lack thereof. However they are currently accepted by tablegen but the
intrinsic signature verifier trips when verifying any further
polymorphic type because the opaque pointer codepath for pointers will
not push the pointed type in ArgTys.

This commit adds an assert to easily catch such cases instead of having
the generic signature match failure.

[thopre]

Genuine test failure on CodeGenOpenCL/builtins-fp-atomics-gfx1030.cl

[thopre]

Exclude match arguments

[nikic]

Could you please share an example of a problematic intrinsic signature?

[thopre]

In D125764#3519318, @nikic wrote:

Could you please share an example of a problematic intrinsic signature?

def int_colossus_ststep :
  Intrinsic<
    [LLVMPointerType<llvm_any_ty>],
    [llvm_any_ty,
     LLVMPointerType<LLVMMatchType<0>>,
     llvm_i32_ty],
    [IntrWriteMem, IntrArgMemOnly]>;

[nikic]

In D125764#3519427, @thopre wrote:

In D125764#3519318, @nikic wrote:

Could you please share an example of a problematic intrinsic signature?

def int_colossus_ststep :
  Intrinsic<
    [LLVMPointerType<llvm_any_ty>],
    [llvm_any_ty,
     LLVMPointerType<LLVMMatchType<0>>,
     llvm_i32_ty],
    [IntrWriteMem, IntrArgMemOnly]>;

Okay, I don't think I really understand the issue you're fixing then. This kind of intrinsic declaration should work with opaque pointers. The LLVMMatchType<0> predicate inside LLVMPointerType<> should just be ignored, which is what the current code is doing (right?)

It looks like we also have some in-tree intrinsics that are using this pattern, such as int_riscv_vlm.

[thopre]

In D125764#3521604, @nikic wrote:

In D125764#3519427, @thopre wrote:

In D125764#3519318, @nikic wrote:

Could you please share an example of a problematic intrinsic signature?

def int_colossus_ststep :
  Intrinsic<
    [LLVMPointerType<llvm_any_ty>],
    [llvm_any_ty,
     LLVMPointerType<LLVMMatchType<0>>,
     llvm_i32_ty],
    [IntrWriteMem, IntrArgMemOnly]>;

Okay, I don't think I really understand the issue you're fixing then. This kind of intrinsic declaration should work with opaque pointers. The LLVMMatchType<0> predicate inside LLVMPointerType<> should just be ignored, which is what the current code is doing (right?)

It looks like we also have some in-tree intrinsics that are using this pattern, such as int_riscv_vlm.

int_riscv_vlm is different because it matches a non-pointer polymorphic type. (the return value). In our case, the polymorphic type is on the pointed type. When that happens, matchIntrinsicType will *not* recurse for the pointed type since there is no pointed type for opaque pointers and thus will *not* push the pointed type into ArgTys. When processing the first parameter (the llvm_any_ty), the function exits with a match failure during deferred checks because of:

      if (D.getArgumentNumber() > ArgTys.size() ||
          D.getArgumentKind() == IITDescriptor::AK_MatchType)
        return IsDeferredCheck || DeferCheck(Ty);

This is because the argument number is 1 but argTys.size() is 0 since the pointed to type was not pushed. Any polymorphic type after a polymorphic pointer type will fail because of that problem.

[nikic]

Thanks for the explanation, I get it now. It would probably be better to enforce this directly in tablegen, but this seems like a reasonable starting point.

[thopre]

In D125764#3521872, @nikic wrote:

Thanks for the explanation, I get it now. It would probably be better to enforce this directly in tablegen, but this seems like a reasonable starting point.

Agreed, I'll try to do a patch for that too.

llvm/lib/IR/Function.cpp

case IITDescriptor::Pointer: {
       }
       // Consume IIT descriptors relating to the pointer element type.
       // FIXME: Intrinsic type matching of nested single value types or even
       // aggregates doesn't work properly with opaque pointers but hopefully
       // doesn't happen in practice.
       while (Infos.front().Kind == IITDescriptor::Pointer ||
              Infos.front().Kind == IITDescriptor::Vector)
         Infos = Infos.slice(1);
+      assert((Infos.front().Kind != IITDescriptor::Argument ||
+              Infos.front().getArgumentKind() == IITDescriptor::AK_MatchType) &&
+             "Unsupported polymorphic pointer type with opaque pointer");
       Infos = Infos.slice(1);
       return false;
     }
 
     case IITDescriptor::Struct: {
       StructType *ST = dyn_cast<StructType>(Ty);
       if (!ST || !ST->isLiteral() || ST->isPacked() ||
           ST->getNumElements() != D.Struct_NumElements)