This is an archive of the discontinued LLVM Phabricator instance.

[mlir][StandardToSPIRV] Add support for lowering index_cast to SPIR-V.
ClosedPublic

Authored by hanchung on May 8 2020, 11:53 AM.

Diff Detail

Event Timeline

hanchung created this revision.May 8 2020, 11:53 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 8 2020, 11:54 AM
hanchung updated this revision to Diff 262937.May 8 2020, 1:09 PM

Rebases to master.

hanchung updated this revision to Diff 262963.May 8 2020, 2:35 PM

I think indices could be negative? so switch to using SConvert.

antiagainst requested changes to this revision.May 8 2020, 5:58 PM
antiagainst added inline comments.
mlir/lib/Conversion/StandardToSPIRV/ConvertStandardToSPIRV.cpp
870

I think we need to special treat the case where the src/target bitwidth == SPIRVTypeConverter::getIndexType() by just removing this op? OpSConvert requires the source and target bitwidth to be different. Casting from index to i32 will result in an invalid OpSConvert.

This revision now requires changes to proceed.May 8 2020, 5:58 PM

I think indices could be negative? so switch to using SConvert.

Yeah the semantics of std.index_cast requires sign extending : https://mlir.llvm.org/docs/Dialects/Standard/#stdindex_cast-indexcastop

hanchung updated this revision to Diff 263279.May 11 2020, 2:33 PM
hanchung marked 2 inline comments as done.

Add test cases for casting between i32 and index.

mlir/lib/Conversion/StandardToSPIRV/ConvertStandardToSPIRV.cpp
870

I think this case is already handled in TypeCastingOpPattern?

auto dstType =
    this->typeConverter.convertType(operation.getResult().getType());
if (dstType == operands.front().getType()) {
  // Due to type conversion, we are seeing the same source and target type.
  // Then we can just erase this operation by forwarding its operand.
  rewriter.replaceOp(operation, operands.front());
} else {
  rewriter.template replaceOpWithNewOp<SPIRVOp>(
      operation, dstType, operands, ArrayRef<NamedAttribute>());
}

The first condition will replace it with operand. Let me add more tests for this case.

antiagainst accepted this revision.May 11 2020, 2:58 PM

Add test cases for casting between i32 and index.

Aha, right. Sorry forgot about the detailed impl there. :)

This revision is now accepted and ready to land.May 11 2020, 2:58 PM
This revision was automatically updated to reflect the committed changes.