[PATCH] D35218: [AMDGPU] fcanonicalize elimination optimization

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:4638
+
+  case ISD::FCANONICALIZE:
+    return true;
----------------
rampitec wrote:
> arsenm wrote:
> > rampitec wrote:
> > > rampitec wrote:
> > > > arsenm wrote:
> > > > > fcanonicalize should have the idempotent optimization applied so there shouldn't be a need to handle it
> > > > It is not:
> > > > ```
> > > >         v_mul_f32_e32 v2, 1.0, v2
> > > >         v_mul_f32_e32 v2, 1.0, v2
> > > > ```
> > > And I do not think there are idempotent SDNodes.
> > I don't think it's done now, but fcanonicalize (fcanonicalize x) should fold to just one canonicalize
> ...and that is what I am doing here.
In fact even if a general folding is implemented in DAGCombiner, I believe it shall stay here as well. It is the same idea as with constants, something else which also normalizes can be combined with it.


https://reviews.llvm.org/D35218