This is an archive of the discontinued LLVM Phabricator instance.

[InstCombine] Fold freeze into phi if one operand is not undef
ClosedPublic

Authored by aqjune on Jul 26 2020, 10:30 AM.

Details

Summary

This patch adds folding freeze into phi if it has only one operand to target.

Diff Detail

Event Timeline

aqjune created this revision.Jul 26 2020, 10:30 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 26 2020, 10:30 AM
nikic added inline comments.Jul 26 2020, 2:37 PM
llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
1053

Won't this let through something like a constant vector with one undef element? Maybe just use the usual guaranteed-not-undef query here?

llvm/test/Transforms/InstCombine/freeze-phi.ll
216

Can you also add a test with an all-constant phi, in which case the freeze will get dropped completely? I believe the code also handles that case.

aqjune updated this revision to Diff 280765.Jul 26 2020, 5:47 PM

Use isGuaranteedNotToBeUndefOrPoison to correctly deal with constant vectors

aqjune marked 3 inline comments as done.Jul 26 2020, 5:49 PM
aqjune added inline comments.
llvm/test/Transforms/InstCombine/freeze-phi.ll
216

Seems like InstSimplify already deals with the case: function const in this file has a phi with constants.

aqjune retitled this revision from [InstCombine] Fold freeze into phi if beneficial to [InstCombine] Fold freeze into phi if one operand is not undef.Jul 26 2020, 7:23 PM
nikic accepted this revision.Jul 27 2020, 12:35 AM

LGTM

As a possible followup: This code could be generalized a bit by using "constant && !constexpr" for non-freeze and "guaranteed-not-undef" (without any constant checks) for freeze, with some appropriate renames. This would basically change the transform from splitting into "constant" and "non-constant" inputs, into "foldable" and "non-foldable" inputs (where "foldable = constant" for most cases).

llvm/test/Transforms/InstCombine/freeze-phi.ll
216

Oh right, that makes sense. In that case the whole phi is "guaranteed not undef", so we don't ever get here.

This revision is now accepted and ready to land.Jul 27 2020, 12:35 AM
This revision was automatically updated to reflect the committed changes.
aqjune marked an inline comment as done.

It makes sense; freeze is simply no-op for a non-undef/poison constexpr, so it should work.