[PATCH] D138470: [bazel] Restore libpfm as a conditional dependency for exegesis.

[Jordan Rupprecht via Phabricator via llvm-commits]

rupprecht added inline comments.


================
Comment at: utils/bazel/llvm-project-overlay/llvm/BUILD.bazel:2764
+    deps = select({
+        ":pfm_external": ["@pfm//:pfm_external"],
+        ":pfm_system": ["@pfm//:pfm_system"],
----------------
chapuni wrote:
> I think the naming of `@pfm//:pfm_external` would be different from other external libraries like `@llvm_foobar//:foobar`
I don't have a strong preference for this name, I just lifted the naming scheme from D119547, e.g. which uses "@mpfr//:mpfr_external" and others for different ways to depend on mpfr. I'm not well versed in the style guide/naming scheme for these kinds of files so I just went with consistency with something else.

For the repo name, I think "@llvm_pfm" would be fine. Is the idea that we name it "@llvm_pfm" and not "@pfm" to clarify that the configuration for setting up pfm is local to the llvm-project repository, i.e. we're not pulling a bzl configuration from pfm's repo?

For the target name, I personally don't think just ":pfm" would be ideal, because it isn't as explicit about which pfm is being used. But if we wanted to use a different name instead of "external", that's totally fine with me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138470