This is an archive of the discontinued LLVM Phabricator instance.

Add InstCombine/InstructionSimplify support for Freeze Instruction
ClosedPublic

Authored by aqjune on Jan 23 2017, 3:52 AM.

Details

Summary
  • Add llvm::SimplifyFreezeInst
  • Add InstCombiner::visitFreeze
  • Add llvm tests

Diff Detail

Event Timeline

aqjune created this revision.Jan 23 2017, 3:52 AM
regehr added a subscriber: regehr.Jan 28 2017, 10:03 PM
aqjune updated this revision to Diff 86251.Jan 30 2017, 12:12 AM
aqjune updated this revision to Diff 106913.Jul 17 2017, 11:53 AM

Rebased to svn commit 308173

filcab added a subscriber: filcab.Jul 19 2017, 10:27 PM

LGTM

include/llvm/Analysis/InstructionSimplify.h
294 ↗(On Diff #86251)

No need for the name in the comment.

aqjune updated this revision to Diff 145378.May 5 2018, 11:16 AM

Rebased to svn commit 331586

aqjune marked an inline comment as done.May 5 2018, 11:16 AM
lebedev.ri added inline comments.
test/Transforms/InstCombine/freeze.ll
1 ↗(On Diff #145378)

Please consider using utils/update_test_checks.py

aqjune updated this revision to Diff 217189.Aug 26 2019, 10:19 AM

Rebase to trunk@369887 (August 26th, 2019)

aqjune marked an inline comment as done.Aug 26 2019, 10:19 AM
reames accepted this revision.Oct 28 2019, 10:25 AM
reames added a subscriber: reames.

LGTM.

p.s. Do you have a list of items for this work somewhere? We clearly need support in a number of passes, is there a master list so that we can split work once the IR def pass lands?

This revision is now accepted and ready to land.Oct 28 2019, 10:25 AM
lebedev.ri accepted this revision.Oct 28 2019, 10:31 AM
lebedev.ri added a reviewer: spatel.

This needs both an instsimplify test and an instcombine test.
Other than that this seems conservatively correct.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptNov 11 2019, 7:21 PM
Herald added a subscriber: hiraditya. · View Herald Transcript

LGTM.

p.s. Do you have a list of items for this work somewhere? We clearly need support in a number of passes, is there a master list so that we can split work once the IR def pass lands?

Hi, sorry for the delay. I don't have master list, but possible places where freeze can be considered is as follows:

  • The loopunswitch bug can be fixed with freeze. https://reviews.llvm.org/D29015 , https://reviews.llvm.org/D29016 , I'll update the patch today or tomorrow.
  • select(c, bop(x,y), bop(x,z)) -> bop(x,(select(c,y,z)) is buggy if c is poison and bop is division because it generates division by poison. This can be resolved by freezing c. It is at InstCombiner::foldSelectOpOp of InstCombineSelect.cpp
  • Check whether freeze was regarded as a zero-cost instruction when inlining
  • Division hoisting
divisor = y | 1 // divisor is either non-zero or poison
while (cond) {
  x = 100 / divisor // Hoisting this x may introduce UB, so y should be freezed in advance.
  use(x)
}

Similar case is having if (divisor != 0) check surrounding the while loop. When divisor is undef (and if branching on undef is nondeterministic jump), hoisting the division is not explained. You can freeze the divisor.

  • SelectionDAGBuilder::visitBr() tries to lower br i1 (and (icmp ...), (icmp ...)) into two separated branches, but it is blocked if one of the icmps is freezed. Adding transformations like freeze (icmp a, const) -> icmp (freeze a), const is helpful for this.