[PATCH] SelectionDAG: fold (fp_to_u/sint (u/sint_to_fp val)) when possible

[Mehdi Amini]

> On Feb 9, 2015, at 11:48 AM, Fiona Glaser <fglaser at apple.com> wrote:
> 
> 
>> On Feb 9, 2015, at 11:31 AM, Chandler Carruth <chandlerc at google.com <mailto:chandlerc at google.com>> wrote:
>> 
>> 
>> On Mon, Feb 9, 2015 at 11:30 AM, Fiona Glaser <fglaser at apple.com <mailto:fglaser at apple.com>> wrote:
>> I get the message ;-) Already redoing it in instcombine; turns out there’s actually already code for it, but it only triggers when TypeA == TypeB. Will have a patch in a few moments.
>> 
>> Oh, sorry, I just totally missed Duncan's email. Sorry for the noise!
> 
> No problem, thanks for the suggestion. I will eventually figure out the intuition for whether a fold belongs in Instcombine or DAG!
> 
> Here’s a new patch that works in instcombine instead.



+      /* extra bit for sign */
+      (int)SrcTy->getScalarSizeInBits() < OpITy->getFPMantissaWidth()) {

It is present in the existing code, but I’m not sure I understand.
First, I don’t understand the strict < instead of <=.
Then I understand that SIToFPInst has one less bit to be considered because the sign bit cannot contribute to a value here (the comment explains why), and it wouldn’t be part of the mantissa anyway.
I would have written something like:

   bool isSigned = isa<SIToFPInst>(OpI);
   if ((isa<UIToFPInst>(OpI) || isSigned) &&
       (int)FI.getType()->getScalarSizeInBits() -  isSigned <= /*extra bit for sign */
                    OpI->getType()->getFPMantissaWidth())

 It would be nice to also have the test that cover this edge case.

+    else if (FITy->getScalarSizeInBits() > SrcTy->getScalarSizeInBits())
+      return new ZExtInst(OpI->getOperand(0), FITy);

No need for ReplaceInstUsesWith() in the return statement?

Mehdi

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150209/6753a71d/attachment.html>