[PATCH] D21127: Remove redundant FMUL in Newton-Raphson SQRT code

[Nikolai Bozhenov via llvm-commits]

n.bozhenov updated this revision to Diff 60392.
n.bozhenov added a comment.

Thanks for your remarks, Sanjay!
I've fixed formatting issues and uploaded a new version of the patch.

As for tests, I can easily add to `x86/sqrt-fastmath.ll` another RUN
with `-mattr=fma -recip=all:2` to check both FMA and two steps
refinement, but I'm not sure if it is a good idea. I believe
`sqrt-fastmath.ll` is overly fragile now. It can be easily broken by
perfectly valid variations in register allocation, code ordering or code
reassociation. And I'm afraid that additional tests for 2 steps
refinement will make the test even more fragile.

So, I wonder if it would be better to split SQRT testing logically in
two parts:

1. check that NR is applied iff necessary (lit-tests)
2. check that NR code calculates roots with expected accuracy (execution test)

In other words, my suggestion is to convert the accuracy testers from
PR21385 into new tests for LLVM test-suite instead of adding more
fragile lit-tests here. What do you think?


http://reviews.llvm.org/D21127

Files:
  lib/CodeGen/SelectionDAG/DAGCombiner.cpp
  test/CodeGen/X86/sqrt-fastmath.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D21127.60392.patch
Type: text/x-patch
Size: 13148 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160610/dc542b37/attachment.bin>