[PATCH] D127854: [InstCombine] Use known bits to determine exact int->fp cast

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp:1945
     auto *FPCast = cast<CastInst>(Src);
-    if (isKnownExactCastIntToFP(*FPCast))
+    if (isKnownExactCastIntToFP(*FPCast, *this))
       return CastInst::Create(FPCast->getOpcode(), FPCast->getOperand(0), Ty);
----------------
Allen wrote:
> spatel wrote:
> > We should have at least one test providing coverage for this path (and fpext too?). 
> > 
> > IIUC, this patch will improve a case like this:
> > 
> > ```
> > define half @masked_int_to_fp_trunc(i32 %A) {
> >   %m = and i32 %A, 16777215
> >   %B = sitofp i32 %m to float
> >   %C = fptrunc float %B to half
> >   ret half %C
> > }
> > 
> > ```
> > But we don't actually need a mask:
> > https://alive2.llvm.org/ce/z/iaEX2i
> > ...so the fold conditions still aren't quite right.
> hi @spatel, I think the case in your link https://alive2.llvm.org/ce/z/iaEX2i is related to another top, maybe make use of the fptrunc?
> 
> * In this patch, we hope fold the case
> ```
>   %m = and i25 %A, smallMaskValue
>   %B = uitofp i25 %m to float
>   %C = fptoui float %B to i25
> ```
> 
> * base on the bt we can see, it has both FPToUI and FPtoI.
> ```
> #0  isKnownExactCastIntToFP (I=..., IC=...)
>     at /home/zhongyunde/llvm-project-init-dev_12x/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp:1762
> #1  0x0000aaaaae7ee18c in llvm::InstCombinerImpl::foldItoFPtoI (this=0xffffffffbce0, FI=...)
>     at /home/zhongyunde/llvm-project-init-dev_12x/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp:1986
> #2  0x0000aaaaae7ee398 in llvm::InstCombinerImpl::visitFPToUI (this=0xffffffffbce0, FI=...)
>     at /home/zhongyunde/llvm-project-init-dev_12x/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp:2011
> ```
I understand that we are not optimizing ideally for this pattern, but that's not my point. 
These kinds of transforms are hard to get right (see for example D124692). 
This patch affects 3 caller code paths, so I'd like to have tests that provide some coverage for those paths.
Please rebase with test diffs after:
a5040860412f


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127854/new/

https://reviews.llvm.org/D127854