Ocaml binding update for fast math flags optimisation

[ZuymantO]

Summary

This update permits to the LLVM OCaml binding to easily add FM flags to FMOperators like fdiv, fadd, fsub etc.

[ZuymantO]

Update patch, here is the C++ API part.

[whitequark]

To me it seems natural to use a bitfield for the flags here. In particular this would get rid of a very weird looping pattern in LLVMGetFastMathFlags. Is there a reason you didn't use a bitfield?

[dblaikie]

This isn't actually being sent to the llvm mailing list. Please remove "llvm" from the subscribers and add "llvm-commits" instead. That'll ensure it gets the appropriate coverage.

[ZuymantO]

In D4423#12, @whitequark wrote:

To me it seems natural to use a bitfield for the flags here. In particular this would get rid of a very weird looping pattern in LLVMGetFastMathFlags. Is there a reason you didn't use a bitfield?

For readability before all things.
But since these flags are used (added and removed) in FastMathFlags class by masks usage it seems really natural to use the bitfields instead of the enumerations. But I am also a little bit wary about the OCaml C Bindings type constructor transformation into C++'s world.

[whitequark]

I really think this should be a bitmask. OCaml has facilities for transforming bitmasks to lists of variants and vice versa. See the convert_flag_list routine as used in e.g. https://github.com/ocaml/ocaml/blob/trunk/otherlibs/unix/open.c#L65.

[ZuymantO]

Ok I managed to remove the enumeration part from Core.h and to define the associates macro in the binding part (llvm_ocaml.c).
But something does not work as well. I want to use the convert_flag_list but that function cause a segmentation fault (core dump).

int fmf_mask_of_flag_list(value flag_list){

// can be ... = {(1 << 0), (1 << 1), (1 << 2), (1 << 3), (1 << 4)} but unreadable ...
int *flag_tab = {FMF_FAST, FMF_NNAN, FMF_NINF, FMF_NSZ, FMF_ARCP};
int converted_flags = 0;
converted_flags = convert_flag_list(flag_list, flag_tab); // this line cause the segfault
return converted_flags;

}

and convert_flag_list definition should looks like

int convert_flag_list(value list, int *flags)
{

int res;
res = 0;
while (list != Val_int(0)) {
  res |= flags[Int_val(Field(list, 0))];
  list = Field(list, 1);
}
return res;

}
I'am looking for a designed solution before update this. But if you have an idea about the seg fault, let me know.

Thanks.

[ZuymantO]

Update of the ocaml bindings patch.
Using bit mask instead of enumerations.

[ZuymantO]

After the last patch I found that, parts code in my project where I suppressed flags from an instruction did not work as expected. This is the patch to resolve that.

[ZuymantO]

Fully combined updates.

lib/IR/Core.cpp

 }
 
 void LLVMSetFastMathFlags(LLVMValueRef Inst, int Flags){
+  if (0 == Flags){
+    FastMathFlags fmf =
+      (unwrap<Instruction>(Inst)->getFastMathFlags());
+    fmf.clear();
+    unwrap<Instruction>(Inst)->setFastMathFlags(fmf);
+    unwrap<Instruction>(Inst)->setHasUnsafeAlgebra(false);
+    unwrap<Instruction>(Inst)->setHasNoNaNs(false);
+    unwrap<Instruction>(Inst)->setHasNoInfs(false);
+    unwrap<Instruction>(Inst)->setHasNoSignedZeros(false);
+    unwrap<Instruction>(Inst)->setHasAllowReciprocal(false);
+    return;
+  }
+
   if (Flags & FastMathFlags::UnsafeAlgebra)
     unwrap<Instruction>(Inst)->setHasUnsafeAlgebra(true);
   if (Flags & FastMathFlags::NoNaNs)

     unwrap<Instruction>(Inst)->setHasNoSignedZeros(true);
   if (Flags & FastMathFlags::AllowReciprocal)
     unwrap<Instruction>(Inst)->setHasAllowReciprocal(true);
-  if (0 == Flags){
-    FastMathFlags fmf =
-      (unwrap<Instruction>(Inst)->getFastMathFlags());
-    fmf.clear();
-    unwrap<Instruction>(Inst)->setFastMathFlags(fmf);
-  }
 }
 
 int LLVMCountFastMathFlags(LLVMValueRef Inst){