Page MenuHomePhabricator

[mlir][VectorOps] Redo the scalar loop emission in VectoToSCF to pad instead of clipping
ClosedPublic

Authored by bkramer on Sep 7 2020, 8:45 AM.

Details

Summary

This replaces the select chain for edge-padding with an scf.if that
performs the memory operation when the index is in bounds and uses the
pad value when it's not. For transfer_write the same mechanism is used,
skipping the store when the index is out of bounds.

The integration test has a bunch of cases of how I believe this should
work.

Diff Detail

Event Timeline

bkramer created this revision.Sep 7 2020, 8:45 AM
Herald added a project: Restricted Project. · View Herald Transcript
bkramer requested review of this revision.Sep 7 2020, 8:45 AM
nicolasvasilache accepted this revision.Sep 8 2020, 12:53 AM

Thanks much for updating this older piece!

mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp
201

Return a Value of type i1 ?

237

would it be simpler to cast at call site and get rid of this template altogether ?

464

Could we rename to inBoundsFun/outOfBoundsFun or s/Fun/Lambda or something along those lines ?
When I looked at the use at the end of the function I had trouble figuring out what was what.

503

The artificial split was due to C++ comma operator undefined ordering.
Now that you have simplified this we could just write:

scalarAccessExprs[memRefDim] = (loopIndex < 0) ? i : i + ivs[loopIndex];
508

Any way to make this look a little more like the actual if-then-else that it builds?
Maybe with some clang-format ?

607

Better, thanks!

This revision is now accepted and ready to land.Sep 8 2020, 12:53 AM
rriddle added inline comments.Sep 8 2020, 2:29 AM
mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp
516

nit: Static functions should be placed in the top-level namespace.