[PATCH] D54649: [FPEnv] Rough out constrained FCmp intrinsics

Ulrich Weigand via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 4 08:36:49 PST 2018


uweigand added a comment.

The one problem with your "new" code is that it now **forces** back-ends to implement something, or else code involving constrained intrinsics will trigger internal compiler errors.  It might be preferable to avoid those ...

But I also agree that your "old" code is a bit awkward.  Ideally, I'd have preferred to see the default handling of strict opcodes in the absence of target support to work just via regular legalization, i.e. remove all the special-cased  getStrictFPOperationAction and mutateStrictFPToFP stuff.  Instead, the strict opcodes would be treated like everything else, except they all have a default action of Expand, and the associated expand action (in the same place where all the other expand actions are) would simply replace the strict opcode by the corresponding normal opcode.

If that normal opcode then has an operation action of Custom, that should just automatically work I think (since the result of an Expand rule is just normally run through legalization again).

And any target that wants to actually properly handle strict opcodes would simply overwrite the default action by either Legal (if it can directly handle the strict opcode) or Custom (if it wants LowerOperation to be called with the strict opcode to do anything special).

But that's not where we are today, and I'm not sure if it is really worthwhile to rework that code at this point ...


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54649





More information about the llvm-commits mailing list