This is an archive of the discontinued LLVM Phabricator instance.

[instcombine] propagate freeze through single use poison producing flag instruction
ClosedPublic

Authored by reames on Oct 12 2021, 1:25 PM.

Details

Summary

If we have an instruction which produces poison only when flags are specified on the instruction, then we know that freezing the operands and dropping flags is equivalent to freezing the result. If we know those flags don't result in any undefined behavior being executed, then there's no point in preserving the flags as we gain no knowledge by having them.

This patch extends the existing propagation logic which sinks freeze to single potential non-poison operands to allow dropping of flags when we know the freeze is the sole use of the instruction with poison flags.

The main value is that we tend to sink freezes towards the phi in IV cycles where the incoming value to the phi is the freeze of an IV increment. This will in turn (in a future patch), let us fold the freeze through the phi into the loop preheader. Motivated by eliminating need for CanonicalizeFreezeInLoops for the clearly profitable cases from onephi.ll test case in the test directory.

Diff Detail

Event Timeline

reames created this revision.Oct 12 2021, 1:25 PM
reames requested review of this revision.Oct 12 2021, 1:25 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 12 2021, 1:25 PM
nikic accepted this revision.Oct 12 2021, 1:32 PM

LGTM apart from the requested test.

llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
3574

I'm missing a test for this case (i.e. freeze eliminated completely after dropping flags).

This revision is now accepted and ready to land.Oct 12 2021, 1:32 PM
This revision was landed with ongoing or failed builds.Oct 12 2021, 1:53 PM
This revision was automatically updated to reflect the committed changes.
nikic added a comment.Oct 12 2021, 2:14 PM

Ooops, something I missed and that wasn't covered by tests: dropPoisonGeneratingFlags() currently doesn't drop poison FMF, so there's a mismatch between the analysis and the dropping code.

Ooops, something I missed and that wasn't covered by tests: dropPoisonGeneratingFlags() currently doesn't drop poison FMF, so there's a mismatch between the analysis and the dropping code.

Fixed in 4c5702c