Page MenuHomePhabricator

Ocaml binding update for fast math flags optimisation
Needs ReviewPublic

Authored by ZuymantO on Jul 8 2014, 10:44 AM.

Details

Reviewers
wanders
Summary

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

Diff Detail

Event Timeline

ZuymantO updated this revision to Diff 11165.Jul 8 2014, 10:44 AM
ZuymantO retitled this revision from to Ocaml binding update for fast math flags optimisation.
ZuymantO updated this object.
ZuymantO edited the test plan for this revision. (Show Details)
ZuymantO added a reviewer: deleted.
ZuymantO added a project: deleted.
ZuymantO changed the edit policy from "All Users" to "No One".
ZuymantO added subscribers: Unknown Object (MLST), deleted.
ZuymantO edited reviewers, added: wanders; removed: deleted.Jul 8 2014, 10:57 AM
ZuymantO removed a subscriber: Unknown Object (MLST).
ZuymantO changed the edit policy from "No One" to "Administrators".Jul 8 2014, 12:14 PM
ZuymantO updated this revision to Diff 11169.Jul 8 2014, 12:49 PM

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

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?

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.

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.

ZuymantO edited subscribers, added: Unknown Object (MLST); removed: deleted.Jul 8 2014, 1:21 PM

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.

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 updated this revision to Diff 11369.Jul 14 2014, 5:09 AM

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

ZuymantO updated this revision to Diff 11402.Jul 14 2014, 12:57 PM

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 updated this revision to Diff 11793.Jul 22 2014, 3:59 PM

Fully combined updates.

ZuymantO changed the edit policy from "Administrators" to "No One".