[PATCH] D75352: Add rsqrt op to Standard dialect and lower it to LLVM dialect.

Mon Mar 2 05:02:59 PST 2020

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

================
Comment at: mlir/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp:1686
+        loc, typeConverter.convertType(operandType),
+        rewriter.getF32FloatAttr(1.0));
+    auto sqrt = rewriter.create<LLVM::SqrtOp>(loc, transformed.operand());
----------------
akuegel wrote:
> ftynse wrote:
> > This may not necessarily work if the operand type is not float. Maybe you could dispatch to different attributes based on type?
> I tried to fix this by using getFloatAttr(operandType, 1.0) instead. Not sure whether operandType is the actual type it expects here though. Now I get llvm.float / llvm.double as types for the constants, before it was f32.
Ah, and if I run the test without -c opt, I also get an assert failure. So as I feared, this is not the right fix yet. Will try to figure out how to make it use the F32 / F64 type as required.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75352