This is an archive of the discontinued LLVM Phabricator instance.

[mlir] Convert MemRefReinterpretCastOp to LLVM.
ClosedPublic

Authored by pifon2a on Oct 23 2020, 5:26 AM.

Diff Detail

Event Timeline

pifon2a created this revision.Oct 23 2020, 5:26 AM
pifon2a requested review of this revision.Oct 23 2020, 5:26 AM
pifon2a edited the summary of this revision. (Show Details)Oct 23 2020, 5:58 AM
ftynse requested changes to this revision.Oct 23 2020, 7:04 AM
ftynse added inline comments.
mlir/lib/Conversion/StandardToLLVM/StandardToLLVM.cpp
2440

Nit: mlir uses camelBack (not capitalizing first letter) for function names.

2463

Nit: elide trivial braces

2504

Nit: cache this in a variable to get better formatting

2506–2507

Hmm, I am not sure that the memory space is the same for both pointers. I know we can't specify more than one, but I seem to recall that the descriptor used to live in the default namespace.

2520–2521

This is not zero unless the memory space is also zero.

2522

Nit: mlir uses camelBack for variable names

2523

We shouldn't need a GEP if the argument is actually always zero.

mlir/test/mlir-cpu-runner/memref_reinterpret_cast.mlir
2

I would prefer a unit test for lowering to and integration test.

This revision now requires changes to proceed.Oct 23 2020, 7:04 AM
pifon2a updated this revision to Diff 300645.Oct 26 2020, 5:16 AM
pifon2a marked 7 inline comments as done.

Address most of the comments

@ftynse Thanks!

mlir/test/mlir-cpu-runner/memref_reinterpret_cast.mlir
2

then it should live together with memref_cast lowering test, right? Do you know where it is? I did not find it.

frgossen accepted this revision.Oct 26 2020, 5:26 AM
pifon2a marked an inline comment as done.Oct 26 2020, 9:02 AM
pifon2a added inline comments.
mlir/test/mlir-cpu-runner/memref_reinterpret_cast.mlir
2

done

pifon2a updated this revision to Diff 300692.Oct 26 2020, 9:02 AM

address the comments

ftynse accepted this revision.Oct 26 2020, 10:18 AM
This revision is now accepted and ready to land.Oct 26 2020, 10:18 AM
This revision was landed with ongoing or failed builds.Oct 26 2020, 12:13 PM
This revision was automatically updated to reflect the committed changes.