[PATCH] D73571: [MLIR] Add the sqrt operation to mlir.

Wed Jan 29 14:35:42 PST 2020

ftynse accepted this revision.
ftynse added inline comments.
This revision is now accepted and ready to land.

================
Comment at: mlir/test/Conversion/StandardToLLVM/convert-to-llvmir.mlir:445
+  %21 = std.sqrt %arg0 : f32
+// CHECK-NEXT: %21 = llvm.mlir.constant(7.900000e-01 : f64) : !llvm.double
+  %22 = constant 7.9e-01 : f64
----------------
llitchev wrote:
> ftynse wrote:
> > Can we rather take an `f64` argument to this function than add another op (constant) irrelevant to the test?
> > 
> > Orthogonally, we should not pattern-match SSA value names (`%21`). I see that the rest of this file does and that should be fixed independently (patches in that direction appreciated), but let's not introduce any new such matches. See https://mlir.llvm.org/getting_started/TestingGuide/ for more details.
> Yeah, my thought was to stick with the general file style - the tanh test above uses const and yes, the file uses the ordinal names for the vars...
I must have missed that when reviewing that commit. Thanks!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73571