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

[Allen zhong via Phabricator via llvm-commits]

Allen 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);
----------------
spatel wrote:
> 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
Done, thanks!


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

https://reviews.llvm.org/D127854