Page MenuHomePhabricator

[mlir] Optimize operand storage such that all operations can have resizable operand lists
ClosedPublic

Authored by rriddle on Apr 26 2020, 2:12 AM.

Details

Summary

This revision refactors the structure of the operand storage such that there is no additional memory cost for resizable operand lists until it is required. This is done by using two different internal representations for the operand storage:

  • One using trailing operands
  • One using a dynamically allocated std::vector<OpOperand>

This allows for removing the resizable operand list bit, and will free up APIs from needing to workaround non-resizable operand lists.

Depends On D78854

Diff Detail

Unit TestsFailed

TimeTest
140 msMLIR.Dialect/Affine::Unknown Unit Message ("")
Script: -- : 'RUN: at line 1'; c:\ws\workspace\amd64_windows_vs2017\llvm-project\build\bin\mlir-opt.exe C:\ws\workspace\amd64_windows_vs2017\llvm-project\mlir\test\Dialect\Affine\loop-tiling.mlir -split-input-file -affine-loop-tile="tile-size=32" | c:\ws\workspace\amd64_windows_vs2017\llvm-project\build\bin\filecheck.exe C:\ws\workspace\amd64_windows_vs2017\llvm-project\mlir\test\Dialect\Affine\loop-tiling.mlir
70 msMLIR.Dialect/Affine::Unknown Unit Message ("")
Script: -- : 'RUN: at line 1'; c:\ws\workspace\amd64_windows_vs2017\llvm-project\build\bin\mlir-opt.exe -allow-unregistered-dialect C:\ws\workspace\amd64_windows_vs2017\llvm-project\mlir\test\Dialect\Affine\unroll-jam.mlir -affine-loop-unroll-jam="unroll-jam-factor=2" | c:\ws\workspace\amd64_windows_vs2017\llvm-project\build\bin\filecheck.exe C:\ws\workspace\amd64_windows_vs2017\llvm-project\mlir\test\Dialect\Affine\unroll-jam.mlir
230 msMLIR.Dialect/Affine::Unknown Unit Message ("")
Script: -- : 'RUN: at line 1'; c:\ws\workspace\amd64_windows_vs2017\llvm-project\build\bin\mlir-opt.exe -allow-unregistered-dialect C:\ws\workspace\amd64_windows_vs2017\llvm-project\mlir\test\Dialect\Affine\unroll.mlir -affine-loop-unroll="unroll-full" | c:\ws\workspace\amd64_windows_vs2017\llvm-project\build\bin\filecheck.exe C:\ws\workspace\amd64_windows_vs2017\llvm-project\mlir\test\Dialect\Affine\unroll.mlir --check-prefix UNROLL-FULL
120 msMLIR.Transforms::Unknown Unit Message ("")
Script: -- : 'RUN: at line 1'; c:\ws\workspace\amd64_windows_vs2017\llvm-project\build\bin\mlir-opt.exe C:\ws\workspace\amd64_windows_vs2017\llvm-project\mlir\test\Transforms\loop-fusion-transformation.mlir -test-loop-fusion -test-loop-fusion-transformation -split-input-file -canonicalize | c:\ws\workspace\amd64_windows_vs2017\llvm-project\build\bin\filecheck.exe C:\ws\workspace\amd64_windows_vs2017\llvm-project\mlir\test\Transforms\loop-fusion-transformation.mlir
150 msMLIR.Transforms::Unknown Unit Message ("")
Script: -- : 'RUN: at line 1'; c:\ws\workspace\amd64_windows_vs2017\llvm-project\build\bin\mlir-opt.exe -allow-unregistered-dialect C:\ws\workspace\amd64_windows_vs2017\llvm-project\mlir\test\Transforms\loop-fusion.mlir -affine-loop-fusion -split-input-file | c:\ws\workspace\amd64_windows_vs2017\llvm-project\build\bin\filecheck.exe C:\ws\workspace\amd64_windows_vs2017\llvm-project\mlir\test\Transforms\loop-fusion.mlir

Event Timeline

rriddle created this revision.Apr 26 2020, 2:12 AM
rriddle updated this revision to Diff 260154.Apr 26 2020, 2:14 AM

Fix a few typos

The reported MLIR test failures seem legit?

mehdi_amini added inline comments.Apr 26 2020, 12:44 PM
mlir/lib/IR/OperationSupport.cpp
134

Using a std::vector adds an extra indirection, you didn't think it was worth having a vector with inlined storage?

rriddle updated this revision to Diff 260179.Apr 26 2020, 1:11 PM

Remove the need for OpOperand copy constructor.

rriddle marked 2 inline comments as done.Apr 26 2020, 1:15 PM
rriddle added inline comments.
mlir/lib/IR/OperationSupport.cpp
134

I think it would be worth it, but I don't have an idea of a size that would be good to use in general. I'm deferring that until I can get a few benchmarks that can give me a good indicator here.

mehdi_amini added inline comments.Apr 26 2020, 4:39 PM
mlir/lib/IR/OperationSupport.cpp
134

Why don't we just allocate inline always the number of operands initially and then grow to the next power of two when we run out of capacity?

rriddle updated this revision to Diff 260192.Apr 26 2020, 5:46 PM
rriddle marked an inline comment as done.

Resolve comments

rriddle marked an inline comment as done.Apr 26 2020, 5:47 PM
rriddle updated this revision to Diff 260193.Apr 26 2020, 6:00 PM

Formatting

mehdi_amini accepted this revision.Apr 26 2020, 6:43 PM

Awesome!

This revision is now accepted and ready to land.Apr 26 2020, 6:43 PM
Harbormaster failed remote builds in B54725: Diff 260192!
This revision was automatically updated to reflect the committed changes.