[PATCH] D141720: [mlgo] Remove the protobuf dependency

Mircea Trofin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 13 13:53:48 PST 2023


mtrofin added a comment.

In D141720#4052669 <https://reviews.llvm.org/D141720#4052669>, @aidengrossman wrote:

> LGTM
> One really minor nit.
>
> I'm assuming the performance difference is pretty minimal between this and the TF Protobuf implementation within the overall training process (although most of the overhead would probably by in parsing on the Python side I would think)?

Right, the main value is in removing a dependency, enable the compiler stream out observations without needing to cache them, which also means we can reuse this code to implement the interactive model evaluator for gym environments. The last 2 will happen in subsequent patches, this just achieves the first bit.



================
Comment at: llvm/test/CodeGen/MLRegalloc/dev-mode-extra-features-logging.ll:25
 ; CHECK-NOT: nan
-; CHECK-LABEL: key: \"instructions\"
 ; Check the first five opcodes in the first eviction problem
+; Also, the first eviction problem is significantly less than 300 instructions. Check
----------------
aidengrossman wrote:
> Maybe note here explicitly we're regexing most of the opcodes due to exact values causing test failures over minor differences? Otherwise the regex might be somewhat hard to parse without additional context.
done


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141720



More information about the llvm-commits mailing list