[PATCH] D91716: AMDGPU/GlobalISel: Calculate isKnownNeverNaN for fminnum and fmaxnum

[Petar Avramovic via Phabricator via llvm-commits]

Petar.Avramovic added inline comments.


================
Comment at: llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-fminnum.mir:771-773
-    ; GFX9: [[FCANONICALIZE2:%[0-9]+]]:_(s32) = G_FCANONICALIZE [[FMAXNUM_IEEE]]
-    ; GFX9: [[FCANONICALIZE3:%[0-9]+]]:_(s32) = G_FCANONICALIZE [[COPY2]]
-    ; GFX9: [[FMINNUM_IEEE:%[0-9]+]]:_(s32) = G_FMINNUM_IEEE [[FCANONICALIZE2]], [[FCANONICALIZE3]]
----------------
Using only that formula this will not change and we are left with unnecessary G_FCANONICALIZE(G_FMAXNUM_IEEE) which stops us from performing further combines both from td file and combiner. Do you have other suggestion how to avoid making G_FCANONICALIZE? 


================
Comment at: llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-fminnum.mir:773
     %3:_(s32) = COPY $vgpr2
     %4:_(s32) = G_FMINNUM %2, %3
     $vgpr0 = COPY %4
----------------
arsenm wrote:
> Petar.Avramovic wrote:
> > 
> > 
> > 
> > ```
> > return isKnownNeverNaN(Op.getOperand(0), SNaN, Depth + 1) ||
> >        isKnownNeverNaN(Op.getOperand(1), SNaN, Depth + 1);
> > }
> > ```
> > This is legalized first. `%2` is isKnownNeverSNaN with IEEE = true, but since inputs for `%2:_(s32) = G_FMAXNUM %0, %1` are not yet canonicalized isKnownNeverSNaN will fail and canonicalize is inserted. However since we know what will happen with `'%2:_(s32) = G_FMAXNUM %0, %1` we declare it  isKnownNeverSNaN to not depend on order of legalization.
> > 
> > 
> The legalization order doesn't matter. These operations have their own independent semantics. isKnownNeverNaN needs to understand both pairs
How can this be done?
>The legalization order doesn't matter.
Formula above gives different result depending if input was legalized already or not. It would work if we would legalize uses first. 


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

https://reviews.llvm.org/D91716