[mlir] Floating constants for import-llvm

[shraiysh]

Summary

mlir-translate -import-llvm test.ll was going into segmentation fault if test.ll had float or double constants.
For example,

%3 = fadd double 3.030000e+01, %0

Now, it is handled in Importer::getConstantAsAttr (similar behaviour as normal integers)
Added tests for FP arithmetic

[shraiysh]

The order of instructions in the test for FP arithmetic wasn't right. Fixed it.

[shraiysh]

Also, @mehdi_amini I do not have permissions to commit. It would be great if you could commit this.

[ftynse]

Thanks!
I would also add an error message in cases where constant conversion failed and made sure the callers of getConstantAsAttr are aware that it can return null (there may be x86_fp80 and fp128 values, that are not supported in MLIR at all). Fine for a separate commit.

[bondhugula]

The author on the landed commit has somehow changed to @ftynse. Was this intended?

[ftynse]

That's the side effect of Phabricator/Arcanist: it replaces the original author with whoever commits the patch. Apparently, it cannot show me the email of the original author due to some privacy concerns, so I cannot manually amend the commit unless the email is available somewhere else, e.g. in the signed-of-by field.

[shraiysh]

I would also add an error message in cases where constant conversion failed and made sure the callers of getConstantAsAttr are aware that it can return null (there may be x86_fp80 and fp128 values, that are not supported in MLIR at all). Fine for a separate commit.

Sure!

That's the side effect of Phabricator/Arcanist: it replaces the original author with whoever commits the patch. Apparently, it cannot show me the email of the original author due to some privacy concerns, so I cannot manually amend the commit unless the email is available somewhere else, e.g. in the signed-of-by field.

For future reference, is there anything I can do to make sure my name is shown in the author field?

Thanks.

[ftynse]

If there's a name and an email available in the revision description, I'll take that. Maybe mehdi_amini has a better idea.

[mehdi_amini]

It is the responsibility of the person landing the patch to properly include the author in the patch, if necessary asking the author in the reviews for how they want it to appear.

[mehdi_amini]

By the way: arc patch honors the commit author when the patch is created with arcanist. This revision was uploaded in the web interface this is why you didn't get it automatically @ftynse

@shraiysh you may want to try to use arc diff to upload your patch in the future.

[shraiysh]

Thanks for the clarification. I will use arc in my future patches.

mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp

 Attribute Importer::getConstantAsAttr(llvm::Constant *value) {
   if (auto *ci = dyn_cast<llvm::ConstantInt>(value))
     return b.getIntegerAttr(
         IntegerType::get(ci->getType()->getBitWidth(), context),
         ci->getValue());
   if (auto *c = dyn_cast<llvm::ConstantDataArray>(value))
     if (c->isString())
       return b.getStringAttr(c->getAsString());
+  if (auto *c = dyn_cast<llvm::ConstantFP>(value)) {
+    if (c->getType()->isDoubleTy())
+      return b.getFloatAttr(FloatType::getF64(context), c->getValueAPF());
+    else if (c->getType()->isFloatingPointTy())
+      return b.getFloatAttr(FloatType::getF32(context), c->getValueAPF());
+  }
   return Attribute();
 }
 
 /// Converts LLVM global variable linkage type into the LLVM dialect predicate.
 static LLVM::Linkage
 processLinkage(llvm::GlobalVariable::LinkageTypes linkage) {
   switch (linkage) {
   case llvm::GlobalValue::PrivateLinkage:

mlir/test/Target/import.ll

 
 ; CHECK-LABEL: llvm.func @f6(%arg0: !llvm<"void (i16)*">)
 define void @f6(void (i16) *%fn) {
 ; CHECK: %[[c:[0-9]+]] = llvm.mlir.constant(0 : i16) : !llvm.i16
 ; CHECK: llvm.call %arg0(%[[c]])
   call void %fn(i16 0)
   ret void
 }
+
+; CHECK-LABEL: llvm.func @FPArithmetic(%arg0: !llvm.float, %arg1: !llvm.float, %arg2: !llvm.double, %arg3: !llvm.double)
+define void @FPArithmetic(float %a, float %b, double %c, double %d) {
+  ; CHECK: %[[a1:[0-9]+]] = llvm.mlir.constant(3.030000e+01 : f64) : !llvm.double
+  ; CHECK: %[[a2:[0-9]+]] = llvm.mlir.constant(3.030000e+01 : f32) : !llvm.float
+  ; CHECK: %[[a3:[0-9]+]] = llvm.fadd %[[a2]], %arg0 : !llvm.float
+  %1 = fadd float 0x403E4CCCC0000000, %a
+  ; CHECK: %[[a4:[0-9]+]] = llvm.fadd %arg0, %arg1 : !llvm.float
+  %2 = fadd float %a, %b
+  ; CHECK: %[[a5:[0-9]+]] = llvm.fadd %[[a1]], %arg2 : !llvm.double
+  %3 = fadd double 3.030000e+01, %c
+  ; CHECK: %[[a6:[0-9]+]] = llvm.fsub %arg0, %arg1 : !llvm.float
+  %4 = fsub float %a, %b
+  ; CHECK: %[[a7:[0-9]+]] = llvm.fsub %arg2, %arg3 : !llvm.double
+  %5 = fsub double %c, %d
+  ; CHECK: %[[a8:[0-9]+]] = llvm.fmul %arg0, %arg1 : !llvm.float
+  %6 = fmul float %a, %b
+  ; CHECK: %[[a9:[0-9]+]] = llvm.fmul %arg2, %arg3 : !llvm.double
+  %7 = fmul double %c, %d
+  ; CHECK: %[[a10:[0-9]+]] = llvm.fdiv %arg0, %arg1 : !llvm.float
+  %8 = fdiv float %a, %b
+  ; CHECK: %[[a12:[0-9]+]] = llvm.fdiv %arg2, %arg3 : !llvm.double
+  %9 = fdiv double %c, %d
+  ; CHECK: %[[a11:[0-9]+]] = llvm.frem %arg0, %arg1 : !llvm.float
+  %10 = frem float %a, %b
+  ; CHECK: %[[a13:[0-9]+]] = llvm.frem %arg2, %arg3 : !llvm.double
+  %11 = frem double %c, %d
+  ret void
+}
 No newline at end of file