This simplifies the case if we already have access to the raw data that we need to store in a ConstantDataArray.
The new factor method can also be reused for implementing the factory method that gets the data as ArrayRef.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
getImpl is private because it's a confusing API; it's very easy to mess up endianness, and ConstantDataArray doesn't work with arbitrary LLVM types (only i8/i16/i32/i64/float/double). This wrapper isn't really much better.
What use case isn't covered by the existing get() APIs?
We want to use it for XLA, simplifying the ConvertLiteralToIrConstant method:
https://github.com/tensorflow/tensorflow/blob/master/tensorflow/compiler/xla/service/llvm_ir/llvm_util.cc#L374
Essentially we already have a buffer that we want to store as-is, and we can compute the type we want to store based on the XLA PrimitiveType. Then we could get rid of the switch, and it would make our code easier.
Oh, you're translating from your Constant abstraction to LLVM's Constant abstraction. Makes sense.
Please rename this to getRaw or something, so it's clear it's not type-safe. And please fix the doc comment to specify what element types are allowed, and note how it deals with endianness.
Addressed review comments:
- renamed method to getRaw
- added more comments regarding supported types and endianness
I think this is fine to go in. It's not the most pretty API, but not really more unsafe than what's already there.