This is an archive of the discontinued LLVM Phabricator instance.

[MLIR] Support lowering n-D arith.index_cast to LLVM
ClosedPublic

Authored by pzread on Jul 15 2022, 3:08 PM.

Details

Summary

Previously we can only lower arith.index_cast with 1-D vectors to LLVM. This change added the support for n-D vectors.

Diff Detail

Event Timeline

pzread created this revision.Jul 15 2022, 3:08 PM
Herald added a reviewer: ftynse. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
pzread requested review of this revision.Jul 15 2022, 3:08 PM
pzread retitled this revision from Support lowering n-D arith.index_cast to LLVM. to [MLIR] Support lowering n-D arith.index_cast to LLVM.Jul 15 2022, 3:08 PM
pzread edited the summary of this revision. (Show Details)Jul 18 2022, 9:28 AM
pzread added a reviewer: hanchung.
ftynse accepted this revision.Jul 20 2022, 1:26 AM
ftynse added inline comments.
mlir/lib/Conversion/ArithmeticToLLVM/ArithmeticToLLVM.cpp
143–149

Can't this just be typeConverter->convertType(getElementTypeOrSelf(op.getIn().getType()) ?

This revision is now accepted and ready to land.Jul 20 2022, 1:26 AM
pzread added inline comments.Jul 20 2022, 8:38 AM
mlir/lib/Conversion/ArithmeticToLLVM/ArithmeticToLLVM.cpp
143–149

getElementTypeOrSelf can only extracts one level of a ShapedType.

ftynse added inline comments.Jul 21 2022, 1:07 AM
mlir/lib/Conversion/ArithmeticToLLVM/ArithmeticToLLVM.cpp
143–149

I know, and the original type op.getIn().getType() is a single-level vector type because we can't nest vectors. The code does literally the same for targetElementType in line 135.

hanchung accepted this revision.Jul 21 2022, 3:04 AM
hanchung added inline comments.
mlir/lib/Conversion/ArithmeticToLLVM/ArithmeticToLLVM.cpp
143–149

The op.getIn().getType() is vector types in this case. It's not the converted type. (The method you use is the converted type -- adaptor.getIn().getType(). As ftynse's suggestion, I think the typeConverter can handle the case?

151–152

I think we can use getIntOrFloatBitWidth(). It looks cleaner to me.

... = targetElementType.getIntOrFloatBitWidth();
...
pzread updated this revision to Diff 446478.Jul 21 2022, 6:56 AM

Use getIntOrFloatBitWidth to get bit width.

pzread marked an inline comment as done.Jul 21 2022, 7:02 AM
pzread added inline comments.
mlir/lib/Conversion/ArithmeticToLLVM/ArithmeticToLLVM.cpp
143–149

Oh, I'm sorry, I misread the op.getIn() as adaptor.getIn().

I'm not familiar with rewrite, but will it be a case where the operand of arith.index_cast has been rewritten into another type with a different element type (e.g., extension)? Therefore the element type from op.getIn() and adaptor.getIn() are different.

pzread updated this revision to Diff 449703.Aug 3 2022, 10:35 AM

Simplify the method to derive source type.

hanchung accepted this revision.Aug 9 2022, 11:07 AM
This revision was automatically updated to reflect the committed changes.