[OpenCL] Allow targets to select address space per type

[svenvh]

Summary

Generalize getOpenCLImageAddrSpace into getOpenCLTypeAddrSpace, such
that targets can select the address space per type.

No functional changes intended.

Initial patch by Simon Perretta.

[yaxunl]

Can you add missing types to test/CodeGenOpenCL/opencl_types.cl ?

include/clang/Basic/TargetInfo.h

1034 ↗

(On Diff #101718)

I think it is a good idea to make this function a central place for all OpenCL types.

Can you also add sampler and pipe type then update CGOpenCLRuntime::getPipeType and CGOpenCLRuntime::getSamplerType to use this function to get address space for sampler and pipe?

1041 ↗

(On Diff #101718)

I think the default (including even_t, clk_event_t, queue_t, reserved_id_t) should be global since these opaque OpenCL objects are pointers to some shared resources. These pointers may be an automatic variable themselves but the memory they point to should be global. Since these objects are dynamically allocated, assuming them in private address space implies runtime needs to maintain a private memory pool for each work item and allocate objects from there. Considering the huge number of work items in typical OpenCL applications, it would be very inefficient to implement these objects in private memory pool. On the other hand, a global memory pool seems much reasonable.

Anastasia/Alexey, any comments on this? Thanks.

lib/AST/ASTContext.cpp

1775 ↗

(On Diff #101718)

All OpenCL types may be unified as

AS = getOpenCLTypeAddrSpace(TK);
Width = Target->getPointerWidth(AS);
Align = Target->getPointerAlign(AS);

where TK = cast<BuiltinType>(T)->getKind();

lib/Basic/Targets.cpp

2376 ↗

(On Diff #101718)

should return TargetInfo::getOpenCLTypeAddrSpace

[bader]

include/clang/Basic/TargetInfo.h

1041 ↗

(On Diff #101718)

I remember we discussed this a couple of time in the past.
The address space for variables of these types is not clearly stated in the spec, so I think the right way to treat it - it's implementation defined.
On the other hand your reasoning on using global address space as default AS makes sense in general - so can we put additional clarification to the spec to align it with the proposed implementation?

lib/Basic/Targets.cpp

2367 ↗

(On Diff #101718)

I think the rule LLVM applies is: use virtual in base classes and override w/o virtual in derived classes.
'virtual' is not necessary here.

[yaxunl]

include/clang/Basic/TargetInfo.h

1041 ↗

(On Diff #101718)

I think it is unnecessary to specify the implementation details in the OpenCL spec.

It is also unnecessary for SPIR-V spec since the pointee address space of OpenCL opaque struct is not encoded in SPIR-V.

We can use the commonly accepted address space definition in the TargetInfo base class. If a target chooses to use different address space for specific opaque objects, it can override it in its own virtual function.

[bader]

include/clang/Basic/TargetInfo.h

1041 ↗

(On Diff #101718)

I think it is unnecessary to specify the implementation details in the OpenCL spec.

Agree, but my point was about specifying the intention in the specification.
For instance, OpenCL spec says that image objects are located in global memory.
It says nothing about objects like events, queues, etc., so I assumed that if it says nothing an implementation is allowed to choose the memory region, but it might be false assumption.

[Anastasia]

include/clang/Basic/TargetInfo.h

1041 ↗

(On Diff #101718)

We could create a bug to Khronos OpenCL C spec and discuss it on the next call just to make sure... but otherwise this change seems reasonable enough!

[yaxunl]

include/clang/Basic/TargetInfo.h

1041 ↗

(On Diff #101718)

Can you or Alexey bring this issue to the WG? Thanks.

[svenvh]

Apologies for the late followup! Rebased the patch and addressed your comments.

[svenvh]

include/clang/Basic/TargetInfo.h

1041 ↗

(On Diff #101718)

It's probably better to change the default to global in a separate commit, once we agree that that is the best solution. So I have preserved the address spaces of the old code in this updated patch.

[yaxunl]

LGTM. Thanks.

[chapuni]

cfe/trunk/lib/Basic/TargetInfo.cpp

351

Excuse me for old commit, I think it might be layering violation.
Could we avoid using AST/Type here?

[Anastasia]

cfe/trunk/lib/Basic/TargetInfo.cpp

351

One of the problems is that even if we could write another layer of enums to map OpenCL specific AST types into the TargetInfo representation it doesn't really help creating a layered structure between AST and Basic libs. They seem to have a large logical dependence despite not including the library headers physically. So modification in AST would require modifications in Basic anyway resulting in overhead of changing 2 places instead of 1. So I am wondering if there is any better approach here e.g. revisiting the library dependencies or what classes they should contain?

[rsmith]

cfe/trunk/lib/Basic/TargetInfo.cpp

351

I don't know what large logical dependencies you're talking about here. Basic is supposed to be answering questions about lowering that are independent of our choice of AST model. It should never look at a clang::Type for any reason to do this, and should never need to do so. If you find it does need to do so, you're asking the TargetInfo questions at the wrong level of abstraction.

As far as I can see, there's only one question you actually need to ask the TargetInfo here, and that's whether builtin types are in the opencl_global or opencl_constant address space.

[yaxunl]

cfe/trunk/lib/Basic/TargetInfo.cpp

351

This function has been expanded to include more types and the returned address space is not limited to global/constant.

I think we need some enum definition in TargetInfo.h for OpenCL types with target-dependent address spaces, as Anastasia suggested before, e.g.

enum OpenCLTypeKindWithTargetDependentAddrSpace {
Default,
Image,
Sampler,
//...
};

Then define this function with the enum as parameter. Then add a function to ASTContext to map AST types to the enum.

[Anastasia]

Perhaps, I don't understand the concept of layered design in this particular case. But I just find it annoying that we need to re-implement the entire OpenCL AST Type structure in Basic. And even if we don't have dependencies on the files physically we still logically bound to the AST representation in Basics and just entirely mirror it there.

[svenvh]

Following up on the layering violation in https://reviews.llvm.org/D40838

cfe/trunk/include/clang/Basic/TargetInfo.h

 namespace clang {
 class DiagnosticsEngine;
 class LangOptions;
 class CodeGenOptions;
 class MacroBuilder;
 class QualType;
 class SourceLocation;
 class SourceManager;
+class Type;
 
 namespace Builtin { struct Info; }
 
 /// \brief Exposes information about the current target.
 ///
 class TargetInfo : public RefCountedBase<TargetInfo> {
   std::shared_ptr<TargetOptions> TargetOpts;
   llvm::Triple Triple;
OpenCLOptions &getSupportedOpenCLOpts() {
     return getTargetOpts().SupportedOpenCLOptions;
   }
 
   /// \brief Get const supported OpenCL extensions and optional core features.
   const OpenCLOptions &getSupportedOpenCLOpts() const {
       return getTargetOpts().SupportedOpenCLOptions;
   }
 
-  /// \brief Get OpenCL image type address space.
-  virtual LangAS::ID getOpenCLImageAddrSpace() const {
-    return LangAS::opencl_global;
-  }
+  /// \brief Get address space for OpenCL type.
+  virtual LangAS::ID getOpenCLTypeAddrSpace(const Type *T) const;
 
   /// \returns Target specific vtbl ptr address space.
   virtual unsigned getVtblPtrAddressSpace() const {
     return 0;
   }
 
   /// \returns If a target requires an address within a target specific address
   /// space \p AddressSpace to be converted in order to be used, then return the

cfe/trunk/lib/AST/ASTContext.cpp

 ///
 /// FIXME: Pointers into different addr spaces could have different sizes and
 /// alignment requirements: getPointerInfo should take an AddrSpace, this
 /// should take a QualType, &c.
 TypeInfo ASTContext::getTypeInfoImpl(const Type *T) const {
   uint64_t Width = 0;
   unsigned Align = 8;
   bool AlignIsRequired = false;
+  unsigned AS = 0;
   switch (T->getTypeClass()) {
 #define TYPE(Class, Base)
 #define ABSTRACT_TYPE(Class, Base)
 #define NON_CANONICAL_TYPE(Class, Base)
 #define DEPENDENT_TYPE(Class, Base) case Type::Class:
 #define NON_CANONICAL_UNLESS_DEPENDENT_TYPE(Class, Base)                       \
   case Type::Class:                                                            \
   assert(!T->isDependentType() && "should not see dependent types here");      \
case BuiltinType::NullPtr:
       Align = Target->getPointerAlign(0); //   == sizeof(void*)
       break;
     case BuiltinType::ObjCId:
     case BuiltinType::ObjCClass:
     case BuiltinType::ObjCSel:
       Width = Target->getPointerWidth(0); 
       Align = Target->getPointerAlign(0);
       break;
-    case BuiltinType::OCLSampler: {
-      auto AS = getTargetAddressSpace(LangAS::opencl_constant);
-      Width = Target->getPointerWidth(AS);
-      Align = Target->getPointerAlign(AS);
-      break;
-    }
+    case BuiltinType::OCLSampler:
     case BuiltinType::OCLEvent:
     case BuiltinType::OCLClkEvent:
     case BuiltinType::OCLQueue:
     case BuiltinType::OCLReserveID:
-      // Currently these types are pointers to opaque types.
-      Width = Target->getPointerWidth(0);
-      Align = Target->getPointerAlign(0);
-      break;
 #define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) \
     case BuiltinType::Id:
 #include "clang/Basic/OpenCLImageTypes.def"
-      {
-        auto AS = getTargetAddressSpace(Target->getOpenCLImageAddrSpace());
-        Width = Target->getPointerWidth(AS);
-        Align = Target->getPointerAlign(AS);
-      }
+      AS = getTargetAddressSpace(Target->getOpenCLTypeAddrSpace(T));
+      break;
     }
     break;
   case Type::ObjCObjectPointer:
     Width = Target->getPointerWidth(0);
     Align = Target->getPointerAlign(0);
     break;
   case Type::BlockPointer: {
-    unsigned AS = getTargetAddressSpace(
-        cast<BlockPointerType>(T)->getPointeeType());
+    AS = getTargetAddressSpace(cast<BlockPointerType>(T)->getPointeeType());
     Width = Target->getPointerWidth(AS);
     Align = Target->getPointerAlign(AS);
     break;
   }
   case Type::LValueReference:
   case Type::RValueReference: {
     // alignof and sizeof should never enter this code path here, so we go
     // the pointer route.
-    unsigned AS = getTargetAddressSpace(
-        cast<ReferenceType>(T)->getPointeeType());
+    AS = getTargetAddressSpace(cast<ReferenceType>(T)->getPointeeType());
     Width = Target->getPointerWidth(AS);
     Align = Target->getPointerAlign(AS);
     break;
   }
   case Type::Pointer: {
-    unsigned AS = getTargetAddressSpace(cast<PointerType>(T)->getPointeeType());
+    AS = getTargetAddressSpace(cast<PointerType>(T)->getPointeeType());
     Width = Target->getPointerWidth(AS);
     Align = Target->getPointerAlign(AS);
     break;
   }
   case Type::MemberPointer: {
     const MemberPointerType *MPT = cast<MemberPointerType>(T);
     std::tie(Width, Align) = ABI->getMemberPointerWidthAndAlign(MPT);
     break;

cfe/trunk/lib/Basic/TargetInfo.cpp

 //===--- TargetInfo.cpp - Information about Target machine ----------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 //  This file implements the TargetInfo and TargetInfoImpl interfaces.
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/Basic/TargetInfo.h"
+#include "clang/AST/Type.h"
 #include "clang/Basic/AddressSpaces.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/LangOptions.h"
 #include "llvm/ADT/APFloat.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <cstdlib>
 using namespace clang;
for (const auto &F : FeatureVec) {
     StringRef Name = F;
     // Apply the feature via the target.
     bool Enabled = Name[0] == '+';
     setFeatureEnabled(Features, Name.substr(1), Enabled);
   }
   return true;
 }
 
+LangAS::ID TargetInfo::getOpenCLTypeAddrSpace(const Type *T) const {
+  auto BT = dyn_cast<BuiltinType>(T);
+
+  if (!BT) {
+    if (isa<PipeType>(T))
+      return LangAS::opencl_global;
+
+    return LangAS::Default;
+  }
+
+  switch (BT->getKind()) {
+#define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix)                   \
+  case BuiltinType::Id:                                                        \
+    return LangAS::opencl_global;
+#include "clang/Basic/OpenCLImageTypes.def"
+
+  case BuiltinType::OCLSampler:
+    return LangAS::opencl_constant;
+
+  default:
+    return LangAS::Default;
+  }
+}
+
 //===----------------------------------------------------------------------===//
 
 
 static StringRef removeGCCRegisterPrefix(StringRef Name) {
   if (Name[0] == '%' || Name[0] == '#')
     Name = Name.substr(1);
 
   return Name;

cfe/trunk/lib/Basic/Targets/AMDGPU.h

 //===--- AMDGPU.h - Declare AMDGPU target feature support -------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file declares AMDGPU TargetInfo objects.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_LIB_BASIC_TARGETS_AMDGPU_H
 #define LLVM_CLANG_LIB_BASIC_TARGETS_AMDGPU_H
 
+#include "clang/AST/Type.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/TargetOptions.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Support/Compiler.h"
 
 namespace clang {
 namespace targets {
 
if (GPU >= GK_GFX6) {
       Opts.support("cl_khr_mipmap_image");
       Opts.support("cl_khr_subgroups");
       Opts.support("cl_khr_3d_image_writes");
       Opts.support("cl_amd_media_ops");
       Opts.support("cl_amd_media_ops2");
     }
   }
 
-  LangAS::ID getOpenCLImageAddrSpace() const override {
+  LangAS::ID getOpenCLTypeAddrSpace(const Type *T) const override {
+    auto BT = dyn_cast<BuiltinType>(T);
+
+    if (!BT)
+      return TargetInfo::getOpenCLTypeAddrSpace(T);
+
+    switch (BT->getKind()) {
+#define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix)                   \
+  case BuiltinType::Id:                                                        \
     return LangAS::opencl_constant;
+#include "clang/Basic/OpenCLImageTypes.def"
+
+    default:
+      return TargetInfo::getOpenCLTypeAddrSpace(T);
+    }
   }
 
   llvm::Optional<unsigned> getConstantAddressSpace() const override {
     return LangAS::FirstTargetAddressSpace + AS.Constant;
   }
 
   /// \returns Target specific vtbl ptr address space.
   unsigned getVtblPtrAddressSpace() const override { return AS.Constant; }

cfe/trunk/lib/CodeGen/CGOpenCLRuntime.h

public:
   /// Emit the IR required for a work-group-local variable declaration, and add
   /// an entry to CGF's LocalDeclMap for D.  The base class does this using
   /// CodeGenFunction::EmitStaticVarDecl to emit an internal global for D.
   virtual void EmitWorkGroupLocalVarDecl(CodeGenFunction &CGF,
                                          const VarDecl &D);
 
   virtual llvm::Type *convertOpenCLSpecificType(const Type *T);
 
-  virtual llvm::Type *getPipeType();
+  virtual llvm::Type *getPipeType(const PipeType *T);
 
-  llvm::PointerType *getSamplerType();
+  llvm::PointerType *getSamplerType(const Type *T);
 
   // \brief Returnes a value which indicates the size in bytes of the pipe
   // element.
   virtual llvm::Value *getPipeElemSize(const Expr *PipeArg);
 
   // \brief Returnes a value which indicates the alignment in bytes of the pipe
   // element.
   virtual llvm::Value *getPipeElemAlign(const Expr *PipeArg);
 };
 
 }
 }
 
 #endif

cfe/trunk/lib/CodeGen/CGOpenCLRuntime.cpp

void CGOpenCLRuntime::EmitWorkGroupLocalVarDecl(CodeGenFunction &CGF,
   return CGF.EmitStaticVarDecl(D, llvm::GlobalValue::InternalLinkage);
 }
 
 llvm::Type *CGOpenCLRuntime::convertOpenCLSpecificType(const Type *T) {
   assert(T->isOpenCLSpecificType() &&
          "Not an OpenCL specific type!");
 
   llvm::LLVMContext& Ctx = CGM.getLLVMContext();
-  uint32_t ImgAddrSpc = CGM.getContext().getTargetAddressSpace(
-    CGM.getTarget().getOpenCLImageAddrSpace());
+  uint32_t AddrSpc = CGM.getContext().getTargetAddressSpace(
+      CGM.getTarget().getOpenCLTypeAddrSpace(T));
   switch (cast<BuiltinType>(T)->getKind()) {
   default:
     llvm_unreachable("Unexpected opencl builtin type!");
     return nullptr;
 #define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) \
   case BuiltinType::Id: \
     return llvm::PointerType::get( \
         llvm::StructType::create(Ctx, "opencl." #ImgType "_" #Suffix "_t"), \
-        ImgAddrSpc);
+        AddrSpc);
 #include "clang/Basic/OpenCLImageTypes.def"
   case BuiltinType::OCLSampler:
-    return getSamplerType();
+    return getSamplerType(T);
   case BuiltinType::OCLEvent:
-    return llvm::PointerType::get(llvm::StructType::create(
-                           Ctx, "opencl.event_t"), 0);
+    return llvm::PointerType::get(
+        llvm::StructType::create(Ctx, "opencl.event_t"), AddrSpc);
   case BuiltinType::OCLClkEvent:
     return llvm::PointerType::get(
-        llvm::StructType::create(Ctx, "opencl.clk_event_t"), 0);
+        llvm::StructType::create(Ctx, "opencl.clk_event_t"), AddrSpc);
   case BuiltinType::OCLQueue:
     return llvm::PointerType::get(
-        llvm::StructType::create(Ctx, "opencl.queue_t"), 0);
+        llvm::StructType::create(Ctx, "opencl.queue_t"), AddrSpc);
   case BuiltinType::OCLReserveID:
     return llvm::PointerType::get(
-        llvm::StructType::create(Ctx, "opencl.reserve_id_t"), 0);
+        llvm::StructType::create(Ctx, "opencl.reserve_id_t"), AddrSpc);
   }
 }
 
-llvm::Type *CGOpenCLRuntime::getPipeType() {
+llvm::Type *CGOpenCLRuntime::getPipeType(const PipeType *T) {
   if (!PipeTy){
-    uint32_t PipeAddrSpc =
-      CGM.getContext().getTargetAddressSpace(LangAS::opencl_global);
+    uint32_t PipeAddrSpc = CGM.getContext().getTargetAddressSpace(
+        CGM.getTarget().getOpenCLTypeAddrSpace(T));
     PipeTy = llvm::PointerType::get(llvm::StructType::create(
       CGM.getLLVMContext(), "opencl.pipe_t"), PipeAddrSpc);
   }
 
   return PipeTy;
 }
 
-llvm::PointerType *CGOpenCLRuntime::getSamplerType() {
+llvm::PointerType *CGOpenCLRuntime::getSamplerType(const Type *T) {
   if (!SamplerTy)
     SamplerTy = llvm::PointerType::get(llvm::StructType::create(
       CGM.getLLVMContext(), "opencl.sampler_t"),
       CGM.getContext().getTargetAddressSpace(
-      LangAS::opencl_constant));
+          CGM.getTarget().getOpenCLTypeAddrSpace(T)));
   return SamplerTy;
 }
 
 llvm::Value *CGOpenCLRuntime::getPipeElemSize(const Expr *PipeArg) {
   const PipeType *PipeTy = PipeArg->getType()->getAs<PipeType>();
   // The type of the last (implicit) argument to be passed.
   llvm::Type *Int32Ty = llvm::IntegerType::getInt32Ty(CGM.getLLVMContext());
   unsigned TypeSize = CGM.getContext()

cfe/trunk/lib/CodeGen/CodeGenModule.cpp

if (!SanStats)
     SanStats = llvm::make_unique<llvm::SanitizerStatReport>(&getModule());
 
   return *SanStats;
 }
 llvm::Value *
 CodeGenModule::createOpenCLIntToSamplerConversion(const Expr *E,
                                                   CodeGenFunction &CGF) {
   llvm::Constant *C = EmitConstantExpr(E, E->getType(), &CGF);
-  auto SamplerT = getOpenCLRuntime().getSamplerType();
+  auto SamplerT = getOpenCLRuntime().getSamplerType(E->getType().getTypePtr());
   auto FTy = llvm::FunctionType::get(SamplerT, {C->getType()}, false);
   return CGF.Builder.CreateCall(CreateRuntimeFunction(FTy,
                                 "__translate_sampler_initializer"),
                                 {C});
 }

cfe/trunk/lib/CodeGen/CodeGenTypes.cpp

if (valueSize != atomicSize) {
         llvm::ArrayType::get(CGM.Int8Ty, (atomicSize - valueSize) / 8)
       };
       ResultType = llvm::StructType::get(getLLVMContext(),
                                          llvm::makeArrayRef(elts));
     }
     break;
   }
   case Type::Pipe: {
-    ResultType = CGM.getOpenCLRuntime().getPipeType();
+    ResultType = CGM.getOpenCLRuntime().getPipeType(cast<PipeType>(Ty));
     break;
   }
   }
   
   assert(ResultType && "Didn't convert a type?");
   
   TypeCache[Ty] = ResultType;
   return ResultType;

cfe/trunk/test/CodeGenOpenCL/opencl_types.cl

-// RUN: %clang_cc1 %s -triple "spir-unknown-unknown" -emit-llvm -o - -O0 | FileCheck %s --check-prefix=CHECK-SPIR
-// RUN: %clang_cc1 %s -triple "amdgcn--amdhsa" -emit-llvm -o - -O0 | FileCheck %s --check-prefix=CHECK-AMDGCN
+// RUN: %clang_cc1 -cl-std=CL2.0 %s -triple "spir-unknown-unknown" -emit-llvm -o - -O0 | FileCheck %s --check-prefix=CHECK-SPIR
+// RUN: %clang_cc1 -cl-std=CL2.0 %s -triple "amdgcn--amdhsa" -emit-llvm -o - -O0 | FileCheck %s --check-prefix=CHECK-AMDGCN
 
 #define CLK_ADDRESS_CLAMP_TO_EDGE       2
 #define CLK_NORMALIZED_COORDS_TRUE      1
 #define CLK_FILTER_NEAREST              0x10
 #define CLK_FILTER_LINEAR               0x20
 
 constant sampler_t glb_smp = CLK_ADDRESS_CLAMP_TO_EDGE|CLK_NORMALIZED_COORDS_TRUE|CLK_FILTER_NEAREST;
 // CHECK-SPIR-NOT: constant i32

 // CHECK-SPIR-LABEL: define {{.*}}void @fnc4smp(%opencl.sampler_t addrspace(2)*
 // CHECK-AMDGCN-LABEL: define {{.*}}void @fnc4smp(%opencl.sampler_t addrspace(2)*
 
 kernel void foo(image1d_t img) {
   sampler_t smp = CLK_ADDRESS_CLAMP_TO_EDGE|CLK_NORMALIZED_COORDS_TRUE|CLK_FILTER_LINEAR;
   // CHECK-SPIR: alloca %opencl.sampler_t addrspace(2)*
   event_t evt;
   // CHECK-SPIR: alloca %opencl.event_t*
+  clk_event_t clk_evt;
+  // CHECK-SPIR: alloca %opencl.clk_event_t*
+  queue_t queue;
+  // CHECK-SPIR: alloca %opencl.queue_t*
+  reserve_id_t rid;
+  // CHECK-SPIR: alloca %opencl.reserve_id_t*
   // CHECK-SPIR: store %opencl.sampler_t addrspace(2)*
   fnc4smp(smp);
   // CHECK-SPIR: call {{.*}}void @fnc4smp(%opencl.sampler_t addrspace(2)*
   fnc4smp(glb_smp);
   // CHECK-SPIR: call {{.*}}void @fnc4smp(%opencl.sampler_t addrspace(2)*
 }
 
+kernel void foo_pipe(read_only pipe int p) {}
+// CHECK-SPIR: @foo_pipe(%opencl.pipe_t addrspace(1)* %p)
+// CHECK_AMDGCN: @foo_pipe(%opencl.pipe_t addrspace(1)* %p)
+
 void __attribute__((overloadable)) bad1(image1d_t b, image2d_t c, image2d_t d) {}
 // CHECK-SPIR-LABEL: @{{_Z4bad114ocl_image1d_ro14ocl_image2d_roS0_|"\\01\?bad1@@\$\$J0YAXPAUocl_image1d_ro@@PAUocl_image2d_ro@@1@Z"}}
 // CHECK-AMDGCN-LABEL: @{{_Z4bad114ocl_image1d_ro14ocl_image2d_roS0_|"\\01\?bad1@@\$\$J0YAXPAUocl_image1d_ro@@PAUocl_image2d_ro@@1@Z"}}(%opencl.image1d_ro_t addrspace(2)*{{.*}}%opencl.image2d_ro_t addrspace(2)*{{.*}}%opencl.image2d_ro_t addrspace(2)*{{.*}})