Page MenuHomePhabricator

Add alignment attribute to memref.global
AcceptedPublic

Authored by dpotop on May 13 2021, 12:46 PM.

Details

Reviewers
ftynse
Summary

I have added an alignment attribute to memref.global.
Naturally, I have also:

  • changed the documentation of memref.global to reflect this
  • added verification code that checks that the attribute value (if present) is a positive power of 2
  • added code to StandardToLLVM.cpp ensuring that the alignment attribute is correctly passed during lowering.
  • added one test case for the lowering to LLVM and one test case checking that the verification code functions.

Diff Detail

Event Timeline

dpotop created this revision.May 13 2021, 12:46 PM
dpotop requested review of this revision.May 13 2021, 12:46 PM
rriddle added inline comments.May 13 2021, 12:52 PM
mlir/lib/Conversion/StandardToLLVM/StandardToLLVM.cpp
2303–2308
mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
1117–1118
1119–1120
1121

Print what the alignment is here.

dpotop updated this revision to Diff 345357.May 14 2021, 12:10 AM
dpotop marked 2 inline comments as done.

Add alignment attribute to memref.global

Applying code simplifications suggested by rriddle.

dpotop added inline comments.May 14 2021, 12:50 AM
mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
1119–1120

Since I have to also print it, I preferred first extracting it to avoid calling the * operator twice.

dpotop updated this revision to Diff 345612.May 15 2021, 12:11 AM

Executing git pull --rebase to correct a testing error.

dpotop updated this revision to Diff 345615.May 15 2021, 1:57 AM

Minor formatting based on git-clang-format.

ftynse accepted this revision.May 17 2021, 6:58 AM
ftynse added inline comments.
mlir/include/mlir/Dialect/MemRef/IR/MemRefOps.td
508–509

Nit: I wouldn't expect the documentation of an op to describe how it is lowered to a specific dialect.

mlir/test/Conversion/StandardToLLVM/standard-to-llvm.mlir
243

Nit: we use "translation" when we create a different IR, e.g. LLVM IR, and "conversion" when we stay within MLIR.

This revision is now accepted and ready to land.May 17 2021, 6:58 AM