[PATCH] D80248: Added pow intrinsic to LLVMIR dialect

[Alex Zinenko via Phabricator via llvm-commits]

ftynse requested changes to this revision.
ftynse added a comment.
This revision now requires changes to proceed.

Could we also test the conversion to actual LLVM IR?



================
Comment at: mlir/test/Dialect/LLVMIR/roundtrip.mlir:104
+// CHECK:       %31 = llvm.mlir.constant(4.200000e+00 : f32) : !llvm.float
+// CHECK-NEXT:  %32 = "llvm.intr.pow"(%arg1, %31) : (!llvm.float, !llvm.float) -> !llvm.float
+  %31 = llvm.mlir.constant(4.200000e+00 : f32) : !llvm.float
----------------
Please don't pattern-match SSA names, https://mlir.llvm.org/getting_started/TestingGuide/#filecheck-best-practices


================
Comment at: mlir/test/Dialect/LLVMIR/roundtrip.mlir:106
+  %31 = llvm.mlir.constant(4.200000e+00 : f32) : !llvm.float
+  %32 = "llvm.intr.pow"(%arg1, %31) : (!llvm.float, !llvm.float) -> !llvm.float
+
----------------
Could you just use %arg1 as both arguments? Constants can be moved around or simplified, so let's not rely on the constant operation being immediately before the one ew test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80248