Skip to content

Commit

Permalink
[mlir][LLVM] Use int32_t to indirectly construct GEPArg (llvm#79562)
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
andrey-golubev and nikita-kud authored Jan 26, 2024
1 parent 5cf9f2c commit 89cd345
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 9 deletions.
3 changes: 2 additions & 1 deletion mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
9 changes: 5 additions & 4 deletions mlir/lib/Conversion/GPUCommon/GPUToLLVMConversion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
9 changes: 5 additions & 4 deletions mlir/lib/Dialect/LLVMIR/Transforms/TypeConsistency.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 89cd345

Please sign in to comment.