Page MenuHomePhabricator

[MemorySSA] Don't use template when the clone is a simplified instruction.
ClosedPublic

Authored by asbirlea on Jun 14 2019, 12:14 PM.

Details

Summary

LoopRotate doesn't create a faithful clone of an instruction, it may
simplify it beforehand. Hence the clone of an instruction that has a
MemoryDef associated may not be a definition, but a use or not a memory
alternig instruction.
Don't rely on the template when the clone may be simplified.

Diff Detail

Repository
rL LLVM

Event Timeline

asbirlea created this revision.Jun 14 2019, 12:14 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 14 2019, 12:14 PM
Herald added subscribers: Prazek, jlebar. · View Herald Transcript

just two stylistic nits; lgtm afterward. thanks for this!

include/llvm/Analysis/MemorySSAUpdater.h
292 ↗(On Diff #204828)

I think that we need to spell out very clearly in the comment what this new parameter allows. I assume it's something along the lines of "If true, the clone was exact. Otherwise, assume that the clone involved simplifications that may have:

  • turned a MemoryUse into an instruction that MemorySSA has no representation for, or
  • turned a MemoryDef into a MemoryUse or an instruction that MemorySSA has no representation for

No other cases are supported."

In which case, I'd probably spell this bool something like CloneWasSimplified or simialar; useTemplateMUD is inscrutable for those who don't know how this is implemented IMO

lib/Analysis/MemorySSAUpdater.cpp
659 ↗(On Diff #204828)

please use /*paramName=*/false

This revision is now accepted and ready to land.Jun 14 2019, 8:36 PM
asbirlea updated this revision to Diff 205135.Jun 17 2019, 11:40 AM
asbirlea marked 2 inline comments as done.

Address comments.

This revision was automatically updated to reflect the committed changes.