[llvm-commits] Patch: Fast-math flags for intrinsics

[Michael Ilseman]

On Dec 18, 2012, at 2:27 PM, Chandler Carruth <chandlerc at google.com> wrote:

> On Tue, Dec 18, 2012 at 2:07 PM, Michael Ilseman <milseman at apple.com> wrote:
> Attached are patches for fast-math support for IntrinsicInst.
> 
> I have one specific concern:
> 
> This includes bitcode changes to separate out IntrinsicInsts from CallInsts.
> 
> <snip>
> 
> Previously, IntrinsicInsts were handled as CallInsts. However, intrinsics can now have fast-math flags, and thus merit a separate bitcode encoding. This change adds FUNC_CODE_INST_INTRINSIC for intrinsic calls, separates them out from general calls, and adds encoding and decoding of fast-math flags. Backwards compatibility is retained. Adds parsing fast-math flags for IntrinsicInsts, and more tests.
> 
> This seems like a terrible consequence of using flags instead of metadata. Why is this a good thing to do?
> 

What is this terrible consequence? Why is it terrible to have another bitcode for intrinsics? They already have their own class at the API level. I can certainly just keep reusing the bitcode for CallInst, find some place to stick the bits in, and throw an error if the callee doesn't resolve to an intrinsic, but is that preferable? If anything, that approach might leave to a SWITCH_INST_MAGIC-eque versioning.

As far as using metadata, that discussion has existed in multiple incarnations, the most recent of which is:
http://lists.cs.uiuc.edu/pipermail/llvmdev/2012-November/055764.html


> I think we either need to either not have fast math markers on intrinsics or provide metadata alternatives to the flags so we can use that pre-existing general mechanism for extending the set of information we have about a particular group of instructions.
> 

We are reusing the pre-existing mechanism for extending the set of information we have about a particular group of instructions. There is nothing in the code before these patches that stops IntrinsicInst from having fast-math flags at the API level or internal representation, there just was no way to encode/decode them (or parse them from text). 

This still uses FPMathOperator as the internal representation, and nothing meaningful is introduced to change that. If you're referring to the "|| isa<IntrinsicInst>(I)" change from Operator.h, that diff can just be completely dropped from these patches without consequence (in fact, I'll do so as it's confusing). 

> Making calls to intrinsic functions less like any other call adds complexity to the entire IR just to support a few intrinsics and a few special-purpose optimizations.

Calls to intrinsic functions are already different from normal calls. Users already switch on the IntrinsicID in order to attempt optimization based on their known semantics. The internal representation is already a valid FPMathOperator, and intrinsics are good matches for fast-math flags as the optimizer knows their definition and can preserved semantic definedness.


Here's some patches that remove the confusing changes to Operator.h. The only changes are to the encoder/decoder, and the text parser.



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121218/2708a252/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Helper-functions-and-minor-refactoring.patch
Type: application/octet-stream
Size: 8959 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121218/2708a252/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121218/2708a252/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Intrinsic-support-for-fast-math-flags.patch
Type: application/octet-stream
Size: 8299 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121218/2708a252/attachment-0001.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121218/2708a252/attachment-0002.html>