Page MenuHomePhabricator

[mlir] Refactor ElementsAttr's value access API
ClosedPublic

Authored by rriddle on Nov 4 2021, 4:57 PM.

Details

Summary

There are several aspects of the API that either aren't easy to use, or are
deceptively easy to do the wrong thing. The main change of this commit
is to remove all of the getValue<T>/getFlatValue<T> from ElementsAttr
and instead provide operator[] methods on the ranges returned by
getValues<T>. This provides a much more convenient API for the value
ranges. It also removes the easy-to-be-inefficient nature of
getValue/getFlatValue, which under the hood would construct a new range for
the type T. Constructing a range is not necessarily cheap in all cases, and
could lead to very poor performance if used within a loop; i.e. if you were to
naively write something like:

DenseElementsAttr attr = ...;
for (int i = 0; i < size; ++i) {
  // We are internally rebuilding the APFloat value range on each iteration!!
  APFloat it = attr.getFlatValue<APFloat>(i);
}

Diff Detail

Event Timeline

rriddle created this revision.Nov 4 2021, 4:57 PM
rriddle requested review of this revision.Nov 4 2021, 4:57 PM
mehdi_amini accepted this revision.Nov 4 2021, 5:34 PM
This revision is now accepted and ready to land.Nov 4 2021, 5:34 PM
This revision was automatically updated to reflect the committed changes.