This is an archive of the discontinued LLVM Phabricator instance.

[mlir] Generalize OpFoldResult usage in ops with offsets, sizes and operands.
ClosedPublic

Authored by nicolasvasilache on Jan 24 2021, 7:55 AM.

Details

Summary

This revision starts evolving the APIs to manipulate ops with offsets, sizes and operands towards a ValueOrAttr abstraction that is already used in folding under the name OpFoldResult.

The objective, in the future, is to allow such manipulations all the way to the level of ODS to avoid all the genuflexions involved in distinguishing between values and attributes for generic constant foldings.

Once this evolution is accepted, the next step will be a mechanical OpFoldResult -> ValueOrAttr.

Diff Detail

Event Timeline

nicolasvasilache requested review of this revision.Jan 24 2021, 7:55 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 24 2021, 7:55 AM
jpienaar added inline comments.
mlir/include/mlir/IR/OpDefinition.h
309

But: reduce lifeness scope:

if (Value ...)

Keep a ValueRange based builder around.

pifon2a accepted this revision.Jan 25 2021, 3:03 AM
pifon2a added inline comments.
mlir/lib/Dialect/StandardOps/IR/Ops.cpp
45–52

nit:

if (auto v =  ofr.dyn_cast<Value>()) {
  dynamicVec.push_back(v);
  staticVec.push_back(sentinel);
  return;
}
APInt apInt = ofr.dyn_cast<Attribute>().cast<IntegerAttr>().getValue();
 taticVec.push_back(apInt.getSExtValue());
}
2108–2120

can it be initialized from (sizes.begin(), sizes.end())?

This revision is now accepted and ready to land.Jan 25 2021, 3:03 AM
nicolasvasilache marked 3 inline comments as done.Jan 25 2021, 4:49 AM
nicolasvasilache added inline comments.
mlir/lib/Dialect/StandardOps/IR/Ops.cpp
2108–2120

Unfortunately no, I get:

third_party/crosstool/v18/stable/toolchain/bin/../include/c++/v1/memory:2193:51: error: no matching constructor for initialization of 'value_type' (aka 'mlir::OpFoldResult')
            ::new ((void*)_VSTD::addressof(*__r)) value_type(*__f);
                                                  ^          ~~~~
third_party/llvm/llvm-project/llvm/include/llvm/ADT/SmallVector.h:490:10: note: in instantiation of function template specialization 'std::uninitialized_copy<const long *, mlir::OpFoldResult *>' requested here
    std::uninitialized_copy(I, E, Dest);
         ^
third_party/llvm/llvm-project/llvm/include/llvm/ADT/SmallVector.h:652:11: note: in instantiation of function template specialization 'llvm::SmallVectorTemplateBase<mlir::OpFoldResult, true>::uninitialized_copy<const long *, mlir::OpFoldResult *>' requested here
    this->uninitialized_copy(in_start, in_end, this->end());
          ^
third_party/llvm/llvm-project/llvm/include/llvm/ADT/SmallVector.h:1184:11: note: in instantiation of function template specialization 'llvm::SmallVectorImpl<mlir::OpFoldResult>::append<const long *, void>' requested here
    this->append(S, E);
          ^
third_party/llvm/llvm-project/mlir/lib/Dialect/StandardOps/IR/Ops.cpp:2127:29: note: in instantiation of function template specialization 'llvm::SmallVector<mlir::OpFoldResult, 6>::SmallVector<const long *, void>' requested here
  SmallVector<OpFoldResult> sizeValues(sizes.begin(), sizes.end());
                            ^
third_party/llvm/llvm-project/llvm/include/llvm/ADT/PointerUnion.h:104:5: note: candidate inherited constructor not viable: no known conversion from 'const long' to 'llvm::PointerIntPair<void *, 1, int, llvm::pointer_union_detail::PointerUnionUIntTraits<mlir::Attribute, mlir::Value>>' for 1st argument
    PointerUnionMembers(ValTy Val) : Val(Val) {}
    ^
third_party/llvm/llvm-project/mlir/include/mlir/IR/OpDefinition.h:220:41: note: constructor from base class 'PointerUnion<mlir::Attribute, mlir::Value>' inherited here
  using PointerUnion<Attribute, Value>::PointerUnion;
                                        ^
third_party/llvm/llvm-project/llvm/include/llvm/ADT/PointerUnion.h:117:5: note: candidate inherited constructor not viable: no known conversion from 'const long' to 'mlir::Value' for 1st argument
    PointerUnionMembers(Type V)
    ^
third_party/llvm/llvm-project/mlir/include/mlir/IR/OpDefinition.h:220:41: note: constructor from base class 'PointerUnion<mlir::Attribute, mlir::Value>' inherited here
  using PointerUnion<Attribute, Value>::PointerUnion;
                                        ^
third_party/llvm/llvm-project/llvm/include/llvm/ADT/PointerUnion.h:117:5: note: candidate inherited constructor not viable: no known conversion from 'const long' to 'mlir::Attribute' for 1st argument
    PointerUnionMembers(Type V)
    ^
third_party/llvm/llvm-project/mlir/include/mlir/IR/OpDefinition.h:220:41: note: constructor from base class 'PointerUnion<mlir::Attribute, mlir::Value>' inherited here
  using PointerUnion<Attribute, Value>::PointerUnion;
                                        ^
third_party/llvm/llvm-project/llvm/include/llvm/ADT/PointerUnion.h:167:3: note: candidate inherited constructor not viable: no known conversion from 'const long' to 'std::nullptr_t' (aka 'nullptr_t') for 1st argument
  PointerUnion(std::nullptr_t) : PointerUnion() {}
  ^
third_party/llvm/llvm-project/mlir/include/mlir/IR/OpDefinition.h:220:41: note: constructor from base class 'PointerUnion<mlir::Attribute, mlir::Value>' inherited here
  using PointerUnion<Attribute, Value>::PointerUnion;

We'll prob want a ValueOrAttrRange in the future, I'm not too hung up on the current impl.

nicolasvasilache marked an inline comment as done.

Address and rebase.

This revision was landed with ongoing or failed builds.Jan 25 2021, 6:21 AM
This revision was automatically updated to reflect the committed changes.