[PATCH] D39304: [IR] redefine 'reassoc' fast-math-flag and add 'trans' fast-math-flag

[Sanjay Patel via Phabricator via llvm-commits]

spatel created this revision.
Herald added subscribers: mcrosier, arsenm.

As discussed on llvm-dev:
http://lists.llvm.org/pipermail/llvm-dev/2016-November/107104.html
and again more recently:
http://lists.llvm.org/pipermail/llvm-dev/2017-October/118118.html

...this is a step in cleaning up our fast-math-flags implementation in IR.

As proposed in the above threads, I've replaced the 'UnsafeAlgebra' bit (which had the 'umbrella' meaning that all flags are set) with a new bit that only applies to algebraic reassociation - 'AllowReassoc'.

I've also added a bit to allow relaxed precision for transcendental functions called 'AllowTrans' (this was initially proposed as 'libm' or similar).

...and we're out of bits. 7 bits ought to be enough for anyone, right? :) FWIW, I did look at getting this out of SubclassOptionalData via SubclassData (spacious 16-bits), but that's apparently already used for other purposes. Also, I don't think we can just add a field to FPMathOperator because Operator is not intended to be instantiated. We'll defer movement of FMF to another day.

I'm not sure, but I may be diverging from the last proposal by keeping the 'fast' keyword. I thought about removing that, but seeing IR like this:
%f.fast = fadd reassoc nnan ninf nsz arcp contract trans float %op1, %op2
...made me think we want to keep the shortcut synonym.

I've also gone ahead and renamed the getter/setters, and mechanically added 'TODO' comments where we need to review how the old setUnsafeAlgebra() was used. In some cases, it's obvious that should be translated to setAllowReassoc(), but others may need to be discussed.

Finally, this change is binary incompatible with existing IR as seen in the compatibility tests. I'm hoping this:
"Newer releases can ignore features from older releases, but they cannot miscompile them. For example, if nsw is ever replaced with something else, dropping it would be a valid way to upgrade the IR." ( http://llvm.org/docs/DeveloperPolicy.html#ir-backwards-compatibility )
...provides the flexibility we want to make this change without requiring a new IR version. Ie, I don't think we're ever loosening the FP strictness of existing IR. At worst, we will fail to optimize some previously 'fast' code because it's no longer recognized as 'fast'. This should get fixed as we squash all of the TODO comments.


https://reviews.llvm.org/D39304

Files:
  docs/LangRef.rst
  include/llvm/IR/Instruction.h
  include/llvm/IR/Operator.h
  include/llvm/Transforms/Utils/LoopUtils.h
  lib/AsmParser/LLLexer.cpp
  lib/AsmParser/LLParser.h
  lib/AsmParser/LLToken.h
  lib/Bitcode/Reader/BitcodeReader.cpp
  lib/Bitcode/Writer/BitcodeWriter.cpp
  lib/CodeGen/ExpandReductions.cpp
  lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  lib/IR/AsmWriter.cpp
  lib/IR/Instruction.cpp
  lib/Target/AMDGPU/AMDGPUCodeGenPrepare.cpp
  lib/Target/AMDGPU/AMDGPULibCalls.cpp
  lib/Transforms/InstCombine/InstCombineAddSub.cpp
  lib/Transforms/InstCombine/InstCombineCalls.cpp
  lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
  lib/Transforms/Scalar/Reassociate.cpp
  lib/Transforms/Utils/LoopUtils.cpp
  lib/Transforms/Utils/SimplifyLibCalls.cpp
  lib/Transforms/Vectorize/LoopVectorize.cpp
  lib/Transforms/Vectorize/SLPVectorizer.cpp
  test/Bitcode/compatibility-3.6.ll
  test/Bitcode/compatibility-3.7.ll
  test/Bitcode/compatibility-3.8.ll
  test/Bitcode/compatibility-3.9.ll
  test/Bitcode/compatibility-4.0.ll
  test/Bitcode/compatibility-5.0.ll
  test/Bitcode/compatibility.ll
  unittests/IR/IRBuilderTest.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D39304.120299.patch
Type: text/x-patch
Size: 46842 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171025/bb64cae0/attachment-0001.bin>