Page MenuHomePhabricator

[MergeICmps] Do not perform the transformation if GEP is used outside of block
ClosedPublic

Authored by twoh on Nov 4 2018, 9:36 PM.

Details

Summary

This patch prevents MergeICmps to performn the transformation if the address operand GEP of the load instruction has a use outside of the load's parent block. Without this patch, compiler crashes with the given test case because the use of %first.i is still around when the basic block is erased from https://github.com/llvm-mirror/llvm/blob/master/lib/Transforms/Scalar/MergeICmps.cpp#L620. I think checking isUsedOutsideOfBlock with GEP is the original intention of the code, as the checking for LoadI is already performed in the same function.

This patch is incomplete though, as this makes the pass overly conservative and fails the test tuple-four-int8.ll. I believe what needs to be done is checking if GEP has a use outside of block that is not the part of "Comparisons" chain. Submit the patch as of now to prevent compiler crash.

Diff Detail

Repository
rL LLVM

Event Timeline

twoh created this revision.Nov 4 2018, 9:36 PM
courbet accepted this revision.Nov 5 2018, 1:39 AM
I think checking isUsedOutsideOfBlock with GEP is the original intention of the code, as the checking for LoadI is already performed in the same function.

Indeed, this is a typo.

Thanks a lot.

test/Transforms/MergeICmps/X86/gep-used-outside.ll
6 ↗(On Diff #172544)

please update the comment.

This revision is now accepted and ready to land.Nov 5 2018, 1:39 AM
twoh updated this revision to Diff 172612.Nov 5 2018, 10:13 AM

Comments for the test updated. Thanks for the comment!

This revision was automatically updated to reflect the committed changes.