[PATCH] D142971: [SystemZ] Implement isGuaranteedNotToBeUndefOrPoisonForTargetNode()

Ulrich Weigand via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 31 11:43:12 PST 2023


uweigand accepted this revision.
uweigand added a comment.
This revision is now accepted and ready to land.

> There are probably other SystemZISD nodes that could go in here, but I am not quite sure how this is supposed to work: It seems to me that some kind of undef/poison value is recognized and at some point a freeze instruction is inserted in the IR. Then the DAGCombiner optimizes it away because there is a target node that now says that it is never undef..? This is contradictory but probably still worthwhile somehow, but I can't quite figure out how..?

Here's the documentation of the `FREEZE` node:

  // FREEZE - FREEZE(VAL) returns an arbitrary value if VAL is UNDEF (or
  // is evaluated to UNDEF), or returns VAL otherwise. Note that each
  // read of UNDEF can yield different value, but FREEZE(UNDEF) cannot.

So the way I understand that is that various code transformations are only correct if multiple evaluations of a value always yield the same result.  Now, since this property does not hold for undef values, you'd have to prove the value cannot be undef in order to perform the transformation.  That may be difficult or impossible in general.  Therefore, you can use a `FREEZE` node instead, which prevents this from happening.

Now, it is on the code generator to handle `FREEZE` efficiently.  But one obvious observation is that *if* the inner value of `FREEZE` *actually* can be proven to never be undef, then the `FREEZE` is a no-op and can just be removed.  This is what common code already does, but sometimes it fails with target nodes, because it doesn't understand what those nodes do and therefore cannot perform this optimization.  This target hook helps with that.

So in general, every target node where we unconditionally know it never returns undef should go in `isGuaranteedNotToBeUndefOrPoisonForTargetNode`, and every target node where we at least conditionally know that, in the case where none of the *arguments* to the target node are undef, the result is never undef either, should go in `canCreateUndefOrPoisonForTargetNode`.

These require reasoning about the properties of our various target nodes, so I think it's fine if this goes in in stages over time.  But this patch at least is obviously correct, and if it fixes some failures, it should go it now.


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

https://reviews.llvm.org/D142971



More information about the llvm-commits mailing list