PR for llvm/llvm-project#79600 #79603
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
@andrey-golubev What do you think about merging this PR to the release branch? |
|
Here we have to wait for the build bots, as they are mandatory. |
GEPArg can only be constructed from int32_t and mlir::Value. Explicitly
cast other types (e.g. unsigned, size_t) to int32_t to avoid narrowing
conversion warnings on MSVC. Some recent examples of such are:
```
mlir\lib\Dialect\LLVMIR\Transforms\TypeConsistency.cpp: error C2398:
Element '1': conversion from 'size_t' to 'T' requires a narrowing
conversion
with
[
T=mlir::LLVM::GEPArg
]
mlir\lib\Dialect\LLVMIR\Transforms\TypeConsistency.cpp: error C2398:
Element '1': conversion from 'unsigned int' to 'T' requires a narrowing
conversion
with
[
T=mlir::LLVM::GEPArg
]
```
Co-authored-by: Nikita Kudriavtsev <[email protected]>
(cherry picked from commit 89cd345)
|
@llvm/pr-subscribers-mlir @llvm/pr-subscribers-mlir-llvm Author: None (github-actions[bot]) Changesresolves llvm/llvm-project#79600 Full diff: https://github.com/llvm/llvm-project/pull/79603.diff 3 Files Affected:
diff --git a/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp b/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp
index ae2bd8e5b5405d9..73d418cb8413276 100644
--- a/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp
+++ b/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp
@@ -529,7 +529,8 @@ LogicalResult GPUPrintfOpToVPrintfLowering::matchAndRewrite(
/*alignment=*/0);
for (auto [index, arg] : llvm::enumerate(args)) {
Value ptr = rewriter.create<LLVM::GEPOp>(
- loc, ptrType, structType, tempAlloc, ArrayRef<LLVM::GEPArg>{0, index});
+ loc, ptrType, structType, tempAlloc,
+ ArrayRef<LLVM::GEPArg>{0, static_cast<int32_t>(index)});
rewriter.create<LLVM::StoreOp>(loc, arg, ptr);
}
std::array<Value, 2> printfArgs = {stringStart, tempAlloc};
diff --git a/mlir/lib/Conversion/GPUCommon/GPUToLLVMConversion.cpp b/mlir/lib/Conversion/GPUCommon/GPUToLLVMConversion.cpp
index f853d5c47b623cf..78d4e8062468720 100644
--- a/mlir/lib/Conversion/GPUCommon/GPUToLLVMConversion.cpp
+++ b/mlir/lib/Conversion/GPUCommon/GPUToLLVMConversion.cpp
@@ -1041,13 +1041,14 @@ Value ConvertLaunchFuncOpToGpuRuntimeCallPattern::generateParamsArray(
auto arrayPtr = builder.create<LLVM::AllocaOp>(
loc, llvmPointerType, llvmPointerType, arraySize, /*alignment=*/0);
for (const auto &en : llvm::enumerate(arguments)) {
+ const auto index = static_cast<int32_t>(en.index());
Value fieldPtr =
builder.create<LLVM::GEPOp>(loc, llvmPointerType, structType, structPtr,
- ArrayRef<LLVM::GEPArg>{0, en.index()});
+ ArrayRef<LLVM::GEPArg>{0, index});
builder.create<LLVM::StoreOp>(loc, en.value(), fieldPtr);
- auto elementPtr = builder.create<LLVM::GEPOp>(
- loc, llvmPointerType, llvmPointerType, arrayPtr,
- ArrayRef<LLVM::GEPArg>{en.index()});
+ auto elementPtr =
+ builder.create<LLVM::GEPOp>(loc, llvmPointerType, llvmPointerType,
+ arrayPtr, ArrayRef<LLVM::GEPArg>{index});
builder.create<LLVM::StoreOp>(loc, fieldPtr, elementPtr);
}
return arrayPtr;
diff --git a/mlir/lib/Dialect/LLVMIR/Transforms/TypeConsistency.cpp b/mlir/lib/Dialect/LLVMIR/Transforms/TypeConsistency.cpp
index 72f9295749a66ba..b25c831bc7172a3 100644
--- a/mlir/lib/Dialect/LLVMIR/Transforms/TypeConsistency.cpp
+++ b/mlir/lib/Dialect/LLVMIR/Transforms/TypeConsistency.cpp
@@ -488,7 +488,8 @@ static void splitVectorStore(const DataLayout &dataLayout, Location loc,
// Other patterns will turn this into a type-consistent GEP.
auto gepOp = rewriter.create<GEPOp>(
loc, address.getType(), rewriter.getI8Type(), address,
- ArrayRef<GEPArg>{storeOffset + index * elementSize});
+ ArrayRef<GEPArg>{
+ static_cast<int32_t>(storeOffset + index * elementSize)});
rewriter.create<StoreOp>(loc, extractOp, gepOp);
}
@@ -524,9 +525,9 @@ static void splitIntegerStore(const DataLayout &dataLayout, Location loc,
// We create an `i8` indexed GEP here as that is the easiest (offset is
// already known). Other patterns turn this into a type-consistent GEP.
- auto gepOp =
- rewriter.create<GEPOp>(loc, address.getType(), rewriter.getI8Type(),
- address, ArrayRef<GEPArg>{currentOffset});
+ auto gepOp = rewriter.create<GEPOp>(
+ loc, address.getType(), rewriter.getI8Type(), address,
+ ArrayRef<GEPArg>{static_cast<int32_t>(currentOffset)});
rewriter.create<StoreOp>(loc, valueToStore, gepOp);
// No need to care about padding here since we already checked previously
|
|
@llvm/pr-subscribers-mlir-gpu Author: None (github-actions[bot]) Changesresolves llvm/llvm-project#79600 Full diff: https://github.com/llvm/llvm-project/pull/79603.diff 3 Files Affected:
diff --git a/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp b/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp
index ae2bd8e5b5405d..73d418cb841327 100644
--- a/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp
+++ b/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp
@@ -529,7 +529,8 @@ LogicalResult GPUPrintfOpToVPrintfLowering::matchAndRewrite(
/*alignment=*/0);
for (auto [index, arg] : llvm::enumerate(args)) {
Value ptr = rewriter.create<LLVM::GEPOp>(
- loc, ptrType, structType, tempAlloc, ArrayRef<LLVM::GEPArg>{0, index});
+ loc, ptrType, structType, tempAlloc,
+ ArrayRef<LLVM::GEPArg>{0, static_cast<int32_t>(index)});
rewriter.create<LLVM::StoreOp>(loc, arg, ptr);
}
std::array<Value, 2> printfArgs = {stringStart, tempAlloc};
diff --git a/mlir/lib/Conversion/GPUCommon/GPUToLLVMConversion.cpp b/mlir/lib/Conversion/GPUCommon/GPUToLLVMConversion.cpp
index f853d5c47b623c..78d4e806246872 100644
--- a/mlir/lib/Conversion/GPUCommon/GPUToLLVMConversion.cpp
+++ b/mlir/lib/Conversion/GPUCommon/GPUToLLVMConversion.cpp
@@ -1041,13 +1041,14 @@ Value ConvertLaunchFuncOpToGpuRuntimeCallPattern::generateParamsArray(
auto arrayPtr = builder.create<LLVM::AllocaOp>(
loc, llvmPointerType, llvmPointerType, arraySize, /*alignment=*/0);
for (const auto &en : llvm::enumerate(arguments)) {
+ const auto index = static_cast<int32_t>(en.index());
Value fieldPtr =
builder.create<LLVM::GEPOp>(loc, llvmPointerType, structType, structPtr,
- ArrayRef<LLVM::GEPArg>{0, en.index()});
+ ArrayRef<LLVM::GEPArg>{0, index});
builder.create<LLVM::StoreOp>(loc, en.value(), fieldPtr);
- auto elementPtr = builder.create<LLVM::GEPOp>(
- loc, llvmPointerType, llvmPointerType, arrayPtr,
- ArrayRef<LLVM::GEPArg>{en.index()});
+ auto elementPtr =
+ builder.create<LLVM::GEPOp>(loc, llvmPointerType, llvmPointerType,
+ arrayPtr, ArrayRef<LLVM::GEPArg>{index});
builder.create<LLVM::StoreOp>(loc, fieldPtr, elementPtr);
}
return arrayPtr;
diff --git a/mlir/lib/Dialect/LLVMIR/Transforms/TypeConsistency.cpp b/mlir/lib/Dialect/LLVMIR/Transforms/TypeConsistency.cpp
index 72f9295749a66b..b25c831bc7172a 100644
--- a/mlir/lib/Dialect/LLVMIR/Transforms/TypeConsistency.cpp
+++ b/mlir/lib/Dialect/LLVMIR/Transforms/TypeConsistency.cpp
@@ -488,7 +488,8 @@ static void splitVectorStore(const DataLayout &dataLayout, Location loc,
// Other patterns will turn this into a type-consistent GEP.
auto gepOp = rewriter.create<GEPOp>(
loc, address.getType(), rewriter.getI8Type(), address,
- ArrayRef<GEPArg>{storeOffset + index * elementSize});
+ ArrayRef<GEPArg>{
+ static_cast<int32_t>(storeOffset + index * elementSize)});
rewriter.create<StoreOp>(loc, extractOp, gepOp);
}
@@ -524,9 +525,9 @@ static void splitIntegerStore(const DataLayout &dataLayout, Location loc,
// We create an `i8` indexed GEP here as that is the easiest (offset is
// already known). Other patterns turn this into a type-consistent GEP.
- auto gepOp =
- rewriter.create<GEPOp>(loc, address.getType(), rewriter.getI8Type(),
- address, ArrayRef<GEPArg>{currentOffset});
+ auto gepOp = rewriter.create<GEPOp>(
+ loc, address.getType(), rewriter.getI8Type(), address,
+ ArrayRef<GEPArg>{static_cast<int32_t>(currentOffset)});
rewriter.create<StoreOp>(loc, valueToStore, gepOp);
// No need to care about padding here since we already checked previously
|
Finished, so asking for someone to press the merge button. Thanks in advance! |
zero9178
approved these changes
Jan 29, 2024
|
Merged: 3df71e5 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
resolves #79600