Page MenuHomePhabricator

[ValueTracking] Improve isKnownNonNaN() to recognize zero splats.

Authored by jonpa on Feb 7 2020, 3:56 PM.




I noticed that this function:

define <2 x double> @f1(<2 x double> %val) {
  %cmp = fcmp ogt <2 x double> %val,  zeroinitializer
  %ret = select <2 x i1> %cmp, <2 x double> %val, <2 x double> zeroinitializer
  ret <2 x double> %ret

, which I expected to be implemented with a systemZ vfmaxdb (vector fp maximum), was not represented on the DAG with an fmaxnum node, and therefore not in the assembly output either.

I found this to be due to the fact that ValueTracking.cpp:isKnownNonNaN() is not recognizing the zero splat. This is because it is a ConstantAggregateZero which is a ConstantData and not a ConstantDataVector that the function is handling.

I wonder if my patch here is the right approach, or if it might be even better to make a utility function for this type of query to make this mistake less likely in other places? It is surprising to me to have a zero splat not recognized as a ConstantDataVector...

Diff Detail

Event Timeline

jonpa created this revision.Feb 7 2020, 3:56 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 7 2020, 3:56 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
tlively accepted this revision.Feb 13 2020, 1:48 PM

Looks good to me! I don't have enough context to know of any other locations where this could be missed, so I'm not sure whether a helper function would be useful. It could always be added later, anyways.

This revision is now accepted and ready to land.Feb 13 2020, 1:48 PM
arsenm added a subscriber: arsenm.Feb 13 2020, 1:59 PM
arsenm added inline comments.

I don't think you need the FP type check

jonpa marked 2 inline comments as done.Feb 19 2020, 9:37 AM
jonpa added inline comments.


This revision was automatically updated to reflect the committed changes.
jonpa marked an inline comment as done.