[llvm-commits] [llvm] r93448 - /llvm/trunk/lib/Analysis/InlineCost.cpp

[Duncan Sands]

Hi Eric,

>> Notice how it checks I.onlyReadsMemory to see if sqrt writes errno or not?
>> Also, your code (and that in SDag) suffers from not being turn-offable.
>> How about factoring the checks out of SelectionDAGBuilder.cpp and reusing
>> them here?
> 
> Yes, I saw that.  Largely I wasn't as concerned with that since, in general,I don't think it's still much of an issue even still since most of those functions are still going to be lowered or optimized.  That said, I'm not wedded to not doing it either. :) The errno checks are probably a good idea, but the sanity checks really aren't necessary.

you are probably right that it doesn't matter much if sqrt is wrongly considered
one instruction long by the inliner.  "Wrongly" refers to those cases where the
code generators turn it into a normal call rather than a single instruction.
However for the sake of argument, let's pretend it does matter.  Then it is
important not to do this unless the SDAG lowering fires, thus the sanity checks
are needed (in case I define my own function I happen to call "sqrt" and which
has nothing to do with the sqrt you have in mind).

> I don't think the code should be able to be turned off - I can see doing it in SelectionDAGBuilder since it's an "optimization", but we're already in an opt pass here.

Likewise, in a free-standing environment a function called "sqrt" may have
nothing to do with the sqrt function you are thinking of, even if it has the
same prototype.

Ciao,

Duncan.