Page MenuHomePhabricator

[mlir] Convert MemRefReinterpretCastOp to LLVM.

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.

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


Nit: elide trivial braces


Nit: cache this in a variable to get better formatting


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.


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


Nit: mlir uses camelBack for variable names


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


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!


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.


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.