Address first round of comments

Author: RiverDave

clang/include/clang/CIR/Dialect/IR/CIRAttrs.td

@@ -606,6 +606,38 @@ def CIR_VTableAttr : CIR_Attr<"VTable", "vtable", [TypedAttrInterface]> {
 //===----------------------------------------------------------------------===//
 
 def CIR_AddressSpaceAttr :  CIR_EnumAttr<CIR_AddressSpace, "address_space"> {
+  let summary = "Attribute representing memory address spaces";
+  let description = [{
+    Represents different memory address spaces for pointer types.
+    Address spaces distinguish between different types of memory regions,
+    such as global, local, or constant memory.
+
+    The `value` parameter is an extensible enum, which encodes target address
+    space as an offset to the last language address space. For that reason, the
+    attribute is implemented as custom AddressSpaceAttr, which provides custom
+    printer and parser for the `value` parameter.
+
+    CIR supports two categories:
+    - Language address spaces: Predefined spaces like `offload_global`,
+      `offload_constant`, `offload_private` for offload programming models
+    - Target address spaces: Numeric spaces `target<N>` for target-specific
+      memory regions, where N is the address space number
+
+    Examples:
+    ```mlir
+    // Default address space (implicit)
+    !cir.ptr<!s32i>
+    
+    // Language-defined offload address spaces  
+    !cir.ptr<!s32i, addrspace(offload_global)>
+    !cir.ptr<!s32i, addrspace(offload_constant)>
+    !cir.ptr<!s32i, addrspace(offload_private)>
+    
+    // Target-specific numeric address spaces
+    !cir.ptr<!s32i, addrspace(target<1>)>
+    !cir.ptr<!s32i, addrspace(target<10>)>
+    ```
+  }];
   let builders = [
     AttrBuilder<(ins "clang::LangAS":$langAS), [{
       return $_get($_ctxt, cir::toCIRAddressSpace(langAS));

clang/include/clang/CIR/Dialect/IR/CIREnumAttr.td

@@ -49,11 +49,6 @@ def CIR_AddressSpace : CIR_I32EnumAttr<
     The `address_space` attribute is used to represent address spaces for
     pointer types in CIR. It provides a unified model on top of `clang::LangAS`
     and simplifies the representation of address spaces.
-
-    The `value` parameter is an extensible enum, which encodes target address
-    space as an offset to the last language address space. For that reason, the
-    attribute is implemented as custom AddressSpaceAttr, which provides custom
-    printer and parser for the `value` parameter.
   }];
 
   let genSpecializedAttr = 0;

clang/lib/CIR/CodeGen/CIRGenExpr.cpp

@@ -2052,7 +2052,6 @@ mlir::Value CIRGenFunction::emitAlloca(StringRef name, mlir::Type ty,
   // CIR uses its own alloca address space rather than follow the target data
   // layout like original CodeGen. The data layout awareness should be done in
   // the lowering pass instead.
-  assert(!cir::MissingFeatures::addressSpace());
   cir::PointerType localVarPtrTy =
       builder.getPointerTo(ty, getCIRAllocaAddressSpace());
   mlir::IntegerAttr alignIntAttr = cgm.getSize(alignment);

clang/lib/CIR/CodeGen/CIRGenModule.cpp

@@ -76,6 +76,7 @@ CIRGenModule::CIRGenModule(mlir::MLIRContext &mlirContext,
   SInt128Ty = cir::IntType::get(&getMLIRContext(), 128, /*isSigned=*/true);
   UInt8Ty = cir::IntType::get(&getMLIRContext(), 8, /*isSigned=*/false);
   UInt8PtrTy = cir::PointerType::get(UInt8Ty);
+  cirAllocaAddressSpace = getTargetCIRGenInfo().getCIRAllocaAddressSpace();
   UInt16Ty = cir::IntType::get(&getMLIRContext(), 16, /*isSigned=*/false);
   UInt32Ty = cir::IntType::get(&getMLIRContext(), 32, /*isSigned=*/false);
   UInt64Ty = cir::IntType::get(&getMLIRContext(), 64, /*isSigned=*/false);

clang/lib/CIR/CodeGen/TargetInfo.h

@@ -32,8 +32,6 @@ bool isEmptyFieldForLayout(const ASTContext &context, const FieldDecl *fd);
 /// if the [[no_unique_address]] attribute would have made them empty.
 bool isEmptyRecordForLayout(const ASTContext &context, QualType t);
 
-class CIRGenFunction;
-
 class TargetCIRGenInfo {
   std::unique_ptr<ABIInfo> info;
 
@@ -45,6 +43,11 @@ class TargetCIRGenInfo {
   /// Returns ABI info helper for the target.
   const ABIInfo &getABIInfo() const { return *info; }
 
+  /// Get the CIR address space for alloca.
+  virtual cir::AddressSpace getCIRAllocaAddressSpace() const {
+    return cir::AddressSpace::Default;
+  }
+
   /// Determine whether a call to an unprototyped functions under
   /// the given calling convention should use the variadic
   /// convention or the non-variadic convention.

clang/lib/CIR/Dialect/IR/CIRTypes.cpp

@@ -852,13 +852,13 @@ mlir::ParseResult parseAddressSpaceValue(mlir::AsmParser &p,
   if (mlir::failed(result))
     return p.emitError(loc, "expected address space keyword");
 
-  // Address space is either a target address space or a regular one.
+  // Address space is either a target address space or a language-specific one.
   // - If it is a target address space, we expect a value to follow in the form
   // of `<value>`, where value is an integer that represents the target address
   // space value. This value is kept in the address space enum as an offset
   // from the maximum address space value, which is defined in
   // `cir::getMaxEnumValForAddressSpace()`. This allows us to use
-  // the same enum for both regular and target address spaces.
+  // the same enum for both language-specific and target address spaces.
   // - Otherwise, we just use the parsed value.
   if (cir::isTargetAddressSpace(result.value())) {
     if (p.parseLess())

clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp

@@ -2313,7 +2313,7 @@ getTargetAddrSpaceFromCIRAddrSpace(cir::AddressSpace addrSpace) {
   if (cir::isTargetAddressSpace(addrSpace))
     return cir::getTargetAddressSpaceValue(addrSpace);
 
-  llvm_unreachable("CIR AS map is not available");
+  llvm_unreachable("CIR target lowering is NYI");
 }
 
 static void prepareTypeConverter(mlir::LLVMTypeConverter &converter,

clang/test/CIR/address-space.c renamed to clang/test/CIR/CodeGen/address-space.c

@@ -2,21 +2,29 @@
 // RUN: FileCheck --input-file=%t.cir %s -check-prefix=CIR
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-llvm %s -o %t.ll
 // RUN: FileCheck --input-file=%t.ll %s -check-prefix=LLVM
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o %t.ll
+// RUN: FileCheck --input-file=%t.ll %s -check-prefix=OGCG
 
+// Test address space 1
 // CIR: cir.func dso_local {{@.*foo.*}}(%arg0: !cir.ptr<!s32i, addrspace(target<1>)>
 // LLVM: define dso_local void @foo(ptr addrspace(1) %0)
+// OGCG: define dso_local void @foo(ptr addrspace(1) noundef %arg)
 void foo(int __attribute__((address_space(1))) *arg) {
   return;
 }
 
+// Test explicit address space 0 (should be same as default)
 // CIR: cir.func dso_local {{@.*bar.*}}(%arg0: !cir.ptr<!s32i, addrspace(target<0>)>
 // LLVM: define dso_local void @bar(ptr %0)
+// OGCG: define dso_local void @bar(ptr noundef %arg)
 void bar(int __attribute__((address_space(0))) *arg) {
   return;
 }
 
+// Test default address space (no attribute)
 // CIR: cir.func dso_local {{@.*baz.*}}(%arg0: !cir.ptr<!s32i>
 // LLVM: define dso_local void @baz(ptr %0)
+// OGCG: define dso_local void @baz(ptr noundef %arg)
 void baz(int *arg) {
   return;
 }