[PATCH] D72384: [mlir] mlir-cpu-runner test's cblas_interface should export functions on Windows

Alex Zinenko via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 8 05:12:05 PST 2020


ftynse requested changes to this revision.
ftynse added inline comments.
This revision now requires changes to proceed.


================
Comment at: mlir/test/mlir-cpu-runner/include/cblas_interface.h:1
+//===- cblas_interface.h - Simple Blas subset interface -------------------===//
+//
----------------
I'm not certain `cblas_interface` is the right name here, it doesn't seem to contain anything specific to BLAS. Maybe `linalg_interface` would have been a better name. May be a separate clean up commit after syncing with @nicolasvasilache 


================
Comment at: mlir/test/mlir-cpu-runner/include/cblas_interface.h:13-25
+#ifdef _WIN32
+#ifndef MLIR_CBLAS_INTERFACE_EXPORT
+#ifdef cblas_interface_EXPORTS
+/* We are building this library */
+#define MLIR_CBLAS_INTERFACE_EXPORT __declspec(dllexport)
+#else
+/* We are using this library */
----------------
This is a third occurrence of a similar macro definition. Would it be possible to generalize it somehow and have only one? 


================
Comment at: mlir/test/mlir-cpu-runner/include/cblas_interface.h:21
+#define MLIR_CBLAS_INTERFACE_EXPORT __declspec(dllimport)
+#endif
+#endif
----------------
Please add a comment that indicates which `#if` this is closing


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72384





More information about the llvm-commits mailing list