[PATCH] D77528: [MLIR] Add support to use aligned_alloc to lower AllocOp from std to llvm

Uday Bondhugula via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 11 01:50:46 PDT 2020


bondhugula marked an inline comment as done.
bondhugula added inline comments.


================
Comment at: mlir/test/Conversion/StandardToLLVM/convert-dynamic-memref-ops.mlir:186
+  return %0 : memref<32x18xf32>
+}
+
----------------
mehdi_amini wrote:
> Can you split this in a new test file? 
> 
> We will process the entire file twice while the tests between the two invocations are actually entirely disjoint.
Sure - this make sense. Given a similar approach and a tendency to put everything in one file at other places, we missed that. The only benefit of having it here is that the generated ocde follows the pattern of the test case right above and so it was easy to add it here and update both together when needed. But thinking about this: what about the additional proofing this provides given that it is also running (without crashing, etc.) on the other cases even if we don't have a FileCheck for the rest? I think there is some benefit there without having to copy over that stuff to another file? How do you weigh these?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77528





More information about the llvm-commits mailing list