Page MenuHomePhabricator

[WIP] [Polly] [GPUNodeBuilder] Correct usage of llvm::Value with getLatestValue.
ClosedPublic

Authored by bollu on Jul 28 2017, 9:01 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

bollu created this revision.Jul 28 2017, 9:01 AM
bollu added a comment.Aug 1 2017, 4:57 AM

Crafting a testcase for this is hard, I'm actually unable to reduce this. @grosser, @Meinersbur - Any ideas on what to do?

bollu updated this revision to Diff 109101.Aug 1 2017, 6:51 AM
  • Update test case to be *much* clearer now. Spent some time reducing and commenting.
bollu updated this revision to Diff 109102.Aug 1 2017, 6:58 AM
  • [NFC] whitespace nit.
grosser accepted this revision.Aug 1 2017, 7:03 AM

The test case looks good, but it seems you merged in your earlier commit. Assuming these will be committed separately, this LGTM.

This revision is now accepted and ready to land.Aug 1 2017, 7:03 AM
This revision was automatically updated to reflect the committed changes.
singam-sanjay edited edge metadata.Aug 3 2017, 3:12 AM

@bollu what is this patch about ?

bollu added a comment.Aug 3 2017, 4:15 AM

@singam-sanjay : During invariant load hoisting, we setup "replacement" values for old values. However, we don't pickup the "replacement" values in all places we should be doing so. This patch caught one such instance and fixed it. I believe there are possibly more occurrences of this bug that I've missed :)

bollu added a comment.Aug 3 2017, 4:16 AM

@singam-sanjay Specifically, the getLatestValue returns either the replacement value if it exists, or the original if there is no replacement.