-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[mlir][LLVM] Add LLVMAddrSpaceAttrInterface and NVVMMemorySpaceAttr #157339
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -451,16 +451,14 @@ void mlir::configureGpuToNVVMTypeConverter(LLVMTypeConverter &converter) { | |
converter, [](gpu::AddressSpace space) -> unsigned { | ||
switch (space) { | ||
case gpu::AddressSpace::Global: | ||
return static_cast<unsigned>( | ||
NVVM::NVVMMemorySpace::kGlobalMemorySpace); | ||
return static_cast<unsigned>(NVVM::NVVMMemorySpace::Global); | ||
case gpu::AddressSpace::Workgroup: | ||
return static_cast<unsigned>( | ||
NVVM::NVVMMemorySpace::kSharedMemorySpace); | ||
return static_cast<unsigned>(NVVM::NVVMMemorySpace::Shared); | ||
case gpu::AddressSpace::Private: | ||
return 0; | ||
} | ||
llvm_unreachable("unknown address space enum value"); | ||
return 0; | ||
return static_cast<unsigned>(NVVM::NVVMMemorySpace::Generic); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change modifies the control flow by adding a return statement in the default case. This could affect performance profiling data by changing the execution path from an unreachable code path to a valid return. Copilot generated this review using guidance from repository custom instructions. Positive FeedbackNegative Feedback |
||
}); | ||
// Lowering for MMAMatrixType. | ||
converter.addConversion([&](gpu::MMAMatrixType type) -> Type { | ||
|
@@ -648,7 +646,7 @@ void mlir::populateGpuToNVVMConversionPatterns( | |
GPUFuncOpLoweringOptions{ | ||
/*allocaAddrSpace=*/0, | ||
/*workgroupAddrSpace=*/ | ||
static_cast<unsigned>(NVVM::NVVMMemorySpace::kSharedMemorySpace), | ||
static_cast<unsigned>(NVVM::NVVMMemorySpace::Shared), | ||
StringAttr::get(&converter.getContext(), | ||
NVVM::NVVMDialect::getKernelFuncAttrName()), | ||
StringAttr::get(&converter.getContext(), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -405,16 +405,14 @@ struct ConvertNVGPUToNVVMPass | |
converter, [](gpu::AddressSpace space) -> unsigned { | ||
switch (space) { | ||
case gpu::AddressSpace::Global: | ||
return static_cast<unsigned>( | ||
NVVM::NVVMMemorySpace::kGlobalMemorySpace); | ||
return static_cast<unsigned>(NVVM::NVVMMemorySpace::Global); | ||
case gpu::AddressSpace::Workgroup: | ||
return static_cast<unsigned>( | ||
NVVM::NVVMMemorySpace::kSharedMemorySpace); | ||
return static_cast<unsigned>(NVVM::NVVMMemorySpace::Shared); | ||
case gpu::AddressSpace::Private: | ||
return 0; | ||
} | ||
llvm_unreachable("unknown address space enum value"); | ||
return 0; | ||
return static_cast<unsigned>(NVVM::NVVMMemorySpace::Generic); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change modifies the control flow by adding a return statement in the default case. This could affect performance profiling data by changing the execution path from an unreachable code path to a valid return. Copilot generated this review using guidance from repository custom instructions. Positive FeedbackNegative Feedback |
||
}); | ||
/// device-side async tokens cannot be materialized in nvvm. We just | ||
/// convert them to a dummy i32 type in order to easily drop them during | ||
|
@@ -677,7 +675,7 @@ struct NVGPUAsyncCopyLowering | |
adaptor.getSrcIndices()); | ||
// Intrinsics takes a global pointer so we need an address space cast. | ||
auto srcPointerGlobalType = LLVM::LLVMPointerType::get( | ||
op->getContext(), NVVM::NVVMMemorySpace::kGlobalMemorySpace); | ||
op->getContext(), static_cast<unsigned>(NVVM::NVVMMemorySpace::Global)); | ||
scrPtr = LLVM::AddrSpaceCastOp::create(b, srcPointerGlobalType, scrPtr); | ||
int64_t dstElements = adaptor.getDstElements().getZExtValue(); | ||
int64_t sizeInBytes = | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -71,16 +71,14 @@ void transform::ApplyGPUToNVVMConversionPatternsOp::populatePatterns( | |
llvmTypeConverter, [](AddressSpace space) -> unsigned { | ||
switch (space) { | ||
case AddressSpace::Global: | ||
return static_cast<unsigned>( | ||
NVVM::NVVMMemorySpace::kGlobalMemorySpace); | ||
return static_cast<unsigned>(NVVM::NVVMMemorySpace::Global); | ||
case AddressSpace::Workgroup: | ||
return static_cast<unsigned>( | ||
NVVM::NVVMMemorySpace::kSharedMemorySpace); | ||
return static_cast<unsigned>(NVVM::NVVMMemorySpace::Shared); | ||
case AddressSpace::Private: | ||
return 0; | ||
} | ||
llvm_unreachable("unknown address space enum value"); | ||
return 0; | ||
return static_cast<unsigned>(NVVM::NVVMMemorySpace::Generic); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change modifies the control flow by adding a return statement in the default case. This could affect performance profiling data by changing the execution path from an unreachable code path to a valid return. Copilot generated this review using guidance from repository custom instructions. Positive FeedbackNegative Feedback |
||
}); | ||
// Used in GPUToNVVM/WmmaOpsToNvvm.cpp so attaching here for now. | ||
// TODO: We should have a single to_nvvm_type_converter. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fabianmcg is this supposed to be a
static_cast<unsigned>
? If not, why?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you're right, that should be an unsigned cast, line 3628 also should be changed to unsigned.