[PATCH] D73672: [MLIR][Standard] Implement constant folding for IndexCast

River Riddle via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 29 17:09:09 PST 2020


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


================
Comment at: mlir/lib/Dialect/StandardOps/Ops.cpp:1641
+OpFoldResult IndexCastOp::fold(ArrayRef<Attribute> operands) {
+  auto a = operands[0].dyn_cast_or_null<IntegerAttr>();
+  if (a) {
----------------
Few nits:
* Can you use a more descriptive name than 'a'? e.g. 'value'?
* Merge the variable into the if condition.



================
Comment at: mlir/lib/Dialect/StandardOps/Ops.cpp:1647
+  }
+  return IntegerAttr();
+}
----------------
nit: Why not just Attribute?


================
Comment at: mlir/test/Transforms/canonicalize.mlir:896
+  %2 = index_cast %c4_i16 : i16 to index
+  // CHECK: return %c4_i16, %c4 : i16, index
+  return %1, %2 : i16, index
----------------
Please follow the testing guidelines(e.g. don't capture SSA values directly):

https://mlir.llvm.org/getting_started/TestingGuide/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73672





More information about the llvm-commits mailing list