[PATCH] D148150: [InstrProf][Temporal] Add weight field to traces

[Ellis Hoag via Phabricator via llvm-commits]

ellis added inline comments.


================
Comment at: llvm/lib/ProfileData/InstrProfWriter.cpp:314
 void InstrProfWriter::addTemporalProfileTraces(
-    SmallVector<TemporalProfTraceTy> SrcTraces, uint64_t SrcStreamSize) {
+    SmallVectorImpl<TemporalProfTraceTy> &SrcTraces, uint64_t SrcStreamSize) {
   // Assume that the source has the same reservoir size as the destination to
----------------
snehasish wrote:
> If you use llvm::ArrayRef instead of SmallVectorImpl& then in the unit tests you can avoid having to create SmallVector objects which are then passed to this function. You can replace them with initializer lists and simplify the code a little bit.
> 
> ```
> SmallVector Traces1({FooTrace}), Traces2({BarTrace});
> Writer.addTemporalProfileTraces(Traces1, 1);
> Writer2.addTemporalProfileTraces(Traces2, 1);
> ```
> becomes
> ```
> Writer.addTemporalProfileTraces({FooTrace}, 1);
> Writer2.addTemporalProfileTraces({BarTrace}, 1);
> ```
> 
I can't use `ArrayRef` because I'm modifying `SrcTraces` by moving traces from it to the destination writer. `MutableArrayRef` causes subtle bugs in the unit tests when I make the above change because it clears `FooTrace` and `BarTrace`. I think I could use `OwningArrayRef` but I think this would lead to more copying than is necessary.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148150/new/

https://reviews.llvm.org/D148150