[Mlir-commits] [mlir] [mlir][emitc][NFC] Clean up EmitC (PR #152327)

[Marius Brehler]

https://github.com/marbre requested changes to this pull request.

Thanks for taking a shot at cleaning up.

I don't think this a NFC with +1,492 −1,476 lines changed as it isn't easy to review and furthermore makes it harder to find the original commits and their commit message which often provide additional details.

The following is
* > mlir/test/mlir-translate/emitc_classops.mlir was moved to mlir/test/Target/Cpp/class.mlir.
* > Test for emitc.class, emitc.field and emitc.get_field was added to mlir/test/Dialect/EmitC/ops.mlir.
* > Unnecessary -verify-diagnostics flags were removed from tests.

is not controversial from my point of view and can be included in a single commit.

To follow https://llvm.org/docs/Contributing.html#how-to-submit-a-patch more closely
> * not contain any unrelated changes
> * be an isolated change. Independent changes should be submitted as separate patches as this makes reviewing easier.

the change
* > Case 16 in bool mlir::emitc::isSupportedFloatType(Type type) in EmitC.cpp was refactored to:

could be submitted on its own even though a one line change.


I don't have a strong opinion on:
* > mlir/test/Dialect/EmitC/wrap_emitc_func_in_class.mlir and mlir/test/Dialect/EmitC/wrap_emitc_func_in_class_noAttr.mlir were moved to mlir/test/Dialect/EmitC/transforms.mlir.

but I think
> * Alphabetical order was restored in mlir/include/mlir/Dialect/EmitC/IR/EmitC.td and mlir/lib/Dialect/EmitC/IR/EmitC.cpp.

should be dropped as there is not much benefit compared to the disadvantage.

https://github.com/llvm/llvm-project/pull/152327