[PATCH] D35335: [AMDGPU] fcaninicalize min/max optimization for GFX9+

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

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


================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:4680
 
+    if (ST->supportsMinMaxDenormModes() &&
+        isKnownNeverSNan(DAG, Op.getOperand(0)) &&
----------------
arsenm wrote:
> Doesn't this need to check whether deforms are enabled or not
Such check is not required but could extend the optimization for f16/f64 (where denorms are always enabled and not flushed) and for f32 on pre-GFX9 if denorms are enabled (which we do not do). Consider:

1. Denorms are supported -> no need to use use canonicalize to care about denorms at all, only care about sNaNs.
2. Denorms are flushed -> use the above check supportsMinMaxDenormModes() to see if they are flushed correctly.

So the actual condition would be (if_denorms_supported(VT) || ST->supportsMinMaxDenormModes()).

Actually I think it can be a part of bigger optimization, because if denorms are always enabled for a VT we can just ignore the actual opcode of fcanonicalize's operand and just optimize it out completely if signaling NaNs are not supported.


https://reviews.llvm.org/D35335