[PATCH] D56289: [AMDGPU] Fixed cvt_f32_ubyte combine

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec marked an inline comment as done.
rampitec added a comment.

In D56289#1345663 <https://reviews.llvm.org/D56289#1345663>, @arsenm wrote:

> Probably should try fixing ShrinkDemandedConstant first


I do not see that a bug in the first place. You are creating mask and ask ShrinkDemandedConstant to do the job. My take it is assumed you did the checks needed and know what are you doing. I will just prevent legal optimizations elsewhere this way.



================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:8589-8590
   const TargetLowering &TLI = DAG.getTargetLoweringInfo();
-  if (TLI.ShrinkDemandedConstant(Src, Demanded, TLO) ||
+  if ((Src->hasOneUse() && TLI.ShrinkDemandedConstant(Src, Demanded, TLO)) ||
       TLI.SimplifyDemandedBits(Src, Demanded, Known, TLO)) {
     DCI.CommitTargetLoweringOpt(TLO);
----------------
arsenm wrote:
> Should skip both if !hasOneUse. I thought SimplifyDemandedBits does this check already, so I don't know why ShrinkDemandedConstant doesn't also
SimplifyDemandedBits does that check, but it does more, so in some cases it is able to work even with multiple uses.


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

https://reviews.llvm.org/D56289