Skip to content

[CIR][CodeGen][Lowering] Better handling of alloca address space with unified AS #682

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Aug 1, 2024
Merged

[CIR][CodeGen][Lowering] Better handling of alloca address space with unified AS #682

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
024d4da
unified alloca as
seven-mile Jul 16, 2024
ae7071e
add comment
seven-mile Jul 27, 2024
811276e
fix typo
seven-mile Jul 29, 2024
fe5a5bf
also fix the alloca op lowering
seven-mile Jul 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1100,7 +1100,7 @@ RValue CIRGenFunction::buildBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
// the AST level this is handled within CreateTempAlloca et al., but for the
// builtin / dynamic alloca we have to handle it here.
assert(!MissingFeatures::addressSpace());
auto AAS = builder.getAddrSpaceAttr(getASTAllocaAddressSpace());
auto AAS = getCIRAllocaAddressSpace();
auto EAS = builder.getAddrSpaceAttr(
E->getType()->getPointeeType().getAddressSpace());
if (EAS != AAS) {
Expand Down
9 changes: 7 additions & 2 deletions clang/lib/CIR/CodeGen/CIRGenExpr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2752,7 +2752,10 @@ mlir::Value CIRGenFunction::buildAlloca(StringRef name, mlir::Type ty,
mlir::Location loc, CharUnits alignment,
mlir::OpBuilder::InsertPoint ip,
mlir::Value arraySize) {
auto localVarPtrTy = mlir::cir::PointerType::get(builder.getContext(), ty);
// CIR uses its own alloca AS rather than follow the target data layout like
// original CodeGen. The data layout awareness should be done in the lowering
// pass instead.
auto localVarPtrTy = builder.getPointerTo(ty, getCIRAllocaAddressSpace());
auto alignIntAttr = CGM.getSize(alignment);

mlir::Value addr;
Expand Down Expand Up @@ -2977,7 +2980,9 @@ Address CIRGenFunction::CreateTempAlloca(mlir::Type Ty, CharUnits Align,
// be different from the type defined by the language. For example,
// in C++ the auto variables are in the default address space. Therefore
// cast alloca to the default address space when necessary.
if (getASTAllocaAddressSpace() != LangAS::Default) {
if (auto ASTAS =
builder.getAddrSpaceAttr(CGM.getLangTempAllocaAddressSpace());
getCIRAllocaAddressSpace() != ASTAS) {
llvm_unreachable("Requires address space cast which is NYI");
}
return Address(V, Ty, Align);
Expand Down
12 changes: 11 additions & 1 deletion clang/lib/CIR/CodeGen/CIRGenModule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ CIRGenModule::CIRGenModule(mlir::MLIRContext &context,
AllocaInt8PtrTy = UInt8PtrTy;
// TODO: GlobalsInt8PtrTy
// TODO: ConstGlobalsPtrTy
ASTAllocaAddressSpace = getTargetCIRGenInfo().getASTAllocaAddressSpace();
CIRAllocaAddressSpace = getTargetCIRGenInfo().getCIRAllocaAddressSpace();

PtrDiffTy = ::mlir::cir::IntType::get(
builder.getContext(), astCtx.getTargetInfo().getMaxPointerWidth(),
Expand Down Expand Up @@ -1403,6 +1403,16 @@ LangAS CIRGenModule::getGlobalConstantAddressSpace() const {
return LangAS::Default;
}

// TODO(cir): this could be a common AST helper for both CIR and LLVM codegen.
LangAS CIRGenModule::getLangTempAllocaAddressSpace() const {
if (getLangOpts().OpenCL)
return LangAS::opencl_private;
if (getLangOpts().SYCLIsDevice || getLangOpts().CUDAIsDevice ||
(getLangOpts().OpenMP && getLangOpts().OpenMPIsTargetDevice))
llvm_unreachable("NYI");
return LangAS::Default;
}

static mlir::cir::GlobalOp
generateStringLiteral(mlir::Location loc, mlir::TypedAttr C,
mlir::cir::GlobalLinkageKind LT, CIRGenModule &CGM,
Expand Down
5 changes: 5 additions & 0 deletions clang/lib/CIR/CodeGen/CIRGenModule.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,11 @@ class CIRGenModule : public CIRGenTypeCache {
/// in AST is always in default address space.
LangAS getGlobalConstantAddressSpace() const;

/// Returns the address space for temporary alloca in the language. This makes
/// the allocated variable's address matches the expectation of AST, rather
/// than keep the address space of the alloca in CIR.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • "allocated variable's address matches" -> "allocated variable's address space match"
  • do we really "keep" the AS; isn't it rather something like "defer mapping" or so?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch, thanks~

do we really "keep" the AS; isn't it rather something like "defer mapping" or so?

Sorry for my dumbness, could you please elaborate the "defer mapping" approach?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, just a minor nit, I meant that by using the language's AS in CIR, we defer the mapping to the target-specific AS that is used for the alloca instruction later in the pipeline. We don't keep it because that alloca has not been created yet.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I got it. I've forgotten what I wrote in the comment😂Surely will fix it.

LangAS getLangTempAllocaAddressSpace() const;

/// Set attributes which are common to any form of a global definition (alias,
/// Objective-C method, function, global variable).
///
Expand Down
12 changes: 4 additions & 8 deletions clang/lib/CIR/CodeGen/CIRGenTypeCache.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "clang/AST/CharUnits.h"
#include "clang/Basic/AddressSpaces.h"
#include "clang/CIR/Dialect/IR/CIRTypes.h"
#include "clang/CIR/Dialect/IR/CIRAttrs.h"
#include "clang/CIR/MissingFeatures.h"

namespace cir {
Expand Down Expand Up @@ -108,7 +109,7 @@ struct CIRGenTypeCache {
// unsigned char SizeAlignInBytes;
// };

clang::LangAS ASTAllocaAddressSpace;
mlir::cir::AddressSpaceAttr CIRAllocaAddressSpace;

// clang::CharUnits getSizeSize() const {
// return clang::CharUnits::fromQuantity(SizeSizeInBytes);
Expand All @@ -123,13 +124,8 @@ struct CIRGenTypeCache {
return clang::CharUnits::fromQuantity(PointerAlignInBytes);
}

clang::LangAS getASTAllocaAddressSpace() const {
// Address spaces are not yet fully supported, but the usage of the default
// alloca address space can be used for now only for comparison with the
// default address space.
assert(!MissingFeatures::addressSpace());
assert(ASTAllocaAddressSpace == clang::LangAS::Default);
return ASTAllocaAddressSpace;
mlir::cir::AddressSpaceAttr getCIRAllocaAddressSpace() const {
return CIRAllocaAddressSpace;
}
};

Expand Down
6 changes: 6 additions & 0 deletions clang/lib/CIR/CodeGen/TargetInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,12 @@ class CommonSPIRTargetCIRGenInfo : public TargetCIRGenInfo {
CommonSPIRTargetCIRGenInfo(std::unique_ptr<ABIInfo> ABIInfo)
: TargetCIRGenInfo(std::move(ABIInfo)) {}

mlir::cir::AddressSpaceAttr getCIRAllocaAddressSpace() const override {
return mlir::cir::AddressSpaceAttr::get(
&getABIInfo().CGT.getMLIRContext(),
mlir::cir::AddressSpaceAttr::Kind::offload_private);
}

unsigned getOpenCLKernelCallingConv() const override {
return llvm::CallingConv::SPIR_KERNEL;
}
Expand Down
8 changes: 5 additions & 3 deletions clang/lib/CIR/CodeGen/TargetInfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,11 @@ class TargetCIRGenInfo {
std::vector<LValue> &ResultRegDests,
std::string &AsmString, unsigned NumOutputs) const {}

/// Get the AST address space for alloca.
virtual clang::LangAS getASTAllocaAddressSpace() const {
return clang::LangAS::Default;
/// Get the CIR address space for alloca.
virtual mlir::cir::AddressSpaceAttr getCIRAllocaAddressSpace() const {
// Return the null attribute, which means the target does not care about the
// alloca address space.
return {};
}

/// Perform address space cast of an expression of pointer type.
Expand Down
16 changes: 12 additions & 4 deletions clang/test/CIR/CodeGen/OpenCL/addrspace-alloca.cl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,28 @@
// CIR: cir.func @func(%arg0: !cir.ptr<!s32i, addrspace(offload_local)>
// LLVM: @func(ptr addrspace(3)
kernel void func(local int *p) {
// CIR-NEXT: %[[#ALLOCA_P:]] = cir.alloca !cir.ptr<!s32i, addrspace(offload_local)>, !cir.ptr<!cir.ptr<!s32i, addrspace(offload_local)>>, ["p", init] {alignment = 8 : i64}
// CIR-NEXT: %[[#ALLOCA_P:]] = cir.alloca !cir.ptr<!s32i, addrspace(offload_local)>, !cir.ptr<!cir.ptr<!s32i, addrspace(offload_local)>, addrspace(offload_private)>, ["p", init] {alignment = 8 : i64}
// LLVM-NEXT: %[[#ALLOCA_P:]] = alloca ptr addrspace(3), i64 1, align 8

int x;
// CIR-NEXT: %[[#ALLOCA_X:]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["x"] {alignment = 4 : i64}
// CIR-NEXT: %[[#ALLOCA_X:]] = cir.alloca !s32i, !cir.ptr<!s32i, addrspace(offload_private)>, ["x"] {alignment = 4 : i64}
// LLVM-NEXT: %[[#ALLOCA_X:]] = alloca i32, i64 1, align 4

global char *b;
// CIR-NEXT: %[[#ALLOCA_B:]] = cir.alloca !cir.ptr<!s8i, addrspace(offload_global)>, !cir.ptr<!cir.ptr<!s8i, addrspace(offload_global)>>, ["b"] {alignment = 8 : i64}
// CIR-NEXT: %[[#ALLOCA_B:]] = cir.alloca !cir.ptr<!s8i, addrspace(offload_global)>, !cir.ptr<!cir.ptr<!s8i, addrspace(offload_global)>, addrspace(offload_private)>, ["b"] {alignment = 8 : i64}
// LLVM-NEXT: %[[#ALLOCA_B:]] = alloca ptr addrspace(1), i64 1, align 8

private int *ptr;
// CIR-NEXT: %[[#ALLOCA_PTR:]] = cir.alloca !cir.ptr<!s32i, addrspace(offload_private)>, !cir.ptr<!cir.ptr<!s32i, addrspace(offload_private)>, addrspace(offload_private)>, ["ptr"] {alignment = 8 : i64}
// LLVM-NEXT: %[[#ALLOCA_PTR:]] = alloca ptr, i64 1, align 8

// Store of the argument `p`
// CIR-NEXT: cir.store %arg0, %[[#ALLOCA_P]] : !cir.ptr<!s32i, addrspace(offload_local)>, !cir.ptr<!cir.ptr<!s32i, addrspace(offload_local)>>
// CIR-NEXT: cir.store %arg0, %[[#ALLOCA_P]] : !cir.ptr<!s32i, addrspace(offload_local)>, !cir.ptr<!cir.ptr<!s32i, addrspace(offload_local)>, addrspace(offload_private)>
// LLVM-NEXT: store ptr addrspace(3) %{{[0-9]+}}, ptr %[[#ALLOCA_P]], align 8

ptr = &x;
// CIR-NEXT: cir.store %[[#ALLOCA_X]], %[[#ALLOCA_PTR]] : !cir.ptr<!s32i, addrspace(offload_private)>, !cir.ptr<!cir.ptr<!s32i, addrspace(offload_private)>, addrspace(offload_private)>
// LLVM-NEXT: store ptr %[[#ALLOCA_X]], ptr %[[#ALLOCA_PTR]]

return;
}
Loading