This is an archive of the discontinued LLVM Phabricator instance.

[mlir] Fix invalid handling of AllocOp symbolOperands by SimplifyAllocConst.
ClosedPublic

Authored by Hardcode84 on Jun 14 2021, 12:44 PM.

Details

Summary

symbolOperands were completely ignored by SimplifyAllocConst. Also, I dindn't find any tests for this pattern. Also, slightly improved diagnostic message for verifyAllocLikeOp.

Diff Detail

Event Timeline

Hardcode84 created this revision.Jun 14 2021, 12:44 PM
Hardcode84 requested review of this revision.Jun 14 2021, 12:44 PM

Do not use getOperand

Not sure who is the right person to review this

mehdi_amini edited reviewers, added: bondhugula; removed: mehdi_amini.Jun 21 2021, 4:40 PM
bondhugula accepted this revision.Jun 21 2021, 5:42 PM

Thanks for fixing this. Some minor comments.

mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
161

This is incorrectly named - should be dynamicSizes.

171

operand -> dynamicSize

mlir/test/Dialect/MemRef/canonicalize.mlir
226–230

There are tests for this pattern but they are in test/Transforms/canonicalize.mlir - @alloc_const_fold. Can you bring that test case here and keep either that or this?

234

Nit: ignore_symbols -> with_symbols ?

This revision is now accepted and ready to land.Jun 21 2021, 5:42 PM

rebase, move some tests from transforms/canonicalize, renames operand -> dynamicSize

bondhugula accepted this revision.Jun 22 2021, 3:01 AM