This is an archive of the discontinued LLVM Phabricator instance.

Reland r341269: [Constant Hoisting] Hoisting Constant GEP Expressions
ClosedPublic

Authored by zzheng on Sep 4 2018, 1:20 PM.

Details

Summary
Reland r341269. Previous commit has no deterministic order between two GEPs
that share the same base GV and have the same offset but are of different types.
Now we use std::stable_sort() for sorting constant candidates.

Reverting commit, r341365:

  Revert r341269: [Constant Hoisting] Hoisting Constant GEP Expressions

  One of the tests is failing 50% of the time when expensive checks are
  enabled. Not sure how deep the problem is so just reverting while the
  author can investigate so that the bots stop repeatedly failing and
  blaming things incorrectly. Will respond with details on the original
  commit.

Original commit, r341269:

  [Constant Hoisting] Hoisting Constant GEP Expressions

  Leverage existing logic in constant hoisting pass to transform constant GEP
  expressions sharing the same base global variable. Multi-dimensional GEPs are
  rewritten into single-dimensional GEPs.

  Differential Revision: https://reviews.llvm.org/D51396

Diff Detail

Repository
rL LLVM

Event Timeline

zzheng created this revision.Sep 4 2018, 1:20 PM
efriedma added inline comments.Sep 4 2018, 1:48 PM
lib/Transforms/Scalar/ConstantHoisting.cpp
649 ↗(On Diff #163892)

This is unfortunately not true; it's possible to have an array of length 0 as a member of a struct.

Can you just use stable_sort here, instead of trying to impose an ordering on GEPs with the same offset?

zzheng updated this revision to Diff 163927.Sep 4 2018, 2:48 PM

Use std::stable_sort() as Eli suggested.

zzheng edited the summary of this revision. (Show Details)Sep 4 2018, 2:48 PM
zzheng marked an inline comment as done.
efriedma accepted this revision.Sep 4 2018, 2:50 PM

LGTM

This revision is now accepted and ready to land.Sep 4 2018, 2:50 PM
This revision was automatically updated to reflect the committed changes.