Page MenuHomePhabricator

[NewPM] Port MachineModuleInfo to the new pass manager.
ClosedPublic

Authored by czhang on Jul 3 2019, 6:35 PM.

Details

Summary

Existing clients are converted to use MachineModuleInfoWrapperPass. The new interface is for defining a new pass manager API in CodeGen (D64179).

Event Timeline

czhang created this revision.Jul 3 2019, 6:35 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 3 2019, 6:35 PM
czhang updated this revision to Diff 208556.Jul 8 2019, 5:47 PM

rebase & clang-format

czhang updated this revision to Diff 208719.Jul 9 2019, 9:52 AM

Remove whitespace noise

arsenm added inline comments.Jul 15 2019, 10:30 AM
llvm/lib/CodeGen/IfConversion.cpp
361

Won't this crash if MachineModuleInfoWrapperPass is not available?

502

Ditto

czhang updated this revision to Diff 209912.EditedJul 15 2019, 10:50 AM

Fix potential null pointer dereferences.

czhang marked 2 inline comments as done.Jul 15 2019, 10:50 AM
arsenm added inline comments.Jul 15 2019, 10:53 AM
llvm/lib/CodeGen/BranchFolding.cpp
133–134

Another one her

llvm/lib/CodeGen/MachineBlockPlacement.cpp
3043

Another here

czhang updated this revision to Diff 209926.Jul 15 2019, 11:54 AM

More potential null pointer dereferences. That should be all of them.

czhang marked 2 inline comments as done.Jul 15 2019, 11:57 AM
arsenm accepted this revision.Jul 15 2019, 11:59 AM

LGTM with nits

llvm/lib/CodeGen/MachineModuleInfo.cpp
363

I don't think you need the llvm::

llvm/tools/llc/llc.cpp
582–583

Indentation broken

This revision is now accepted and ready to land.Jul 15 2019, 11:59 AM
czhang updated this revision to Diff 212447.Jul 30 2019, 3:02 PM

Fixes for nits.

czhang marked 2 inline comments as done.Jul 30 2019, 3:02 PM

Please, add a meaningful commit message that describes in high-level terms what/why you are doing here.

ychen edited the summary of this revision. (Show Details)Sep 9 2019, 10:27 PM
ychen added a subscriber: ychen.
ychen added a comment.Sep 9 2019, 10:29 PM

Hello, Charlie went back to school after his internship, I'll commit this on his behalf. Please let me know if there are any concerns.

ychen edited the summary of this revision. (Show Details)Sep 9 2019, 10:41 PM
MaskRay added inline comments.
llvm/include/llvm/CodeGen/MachineModuleInfo.h
291

{}; -> {}

ychen updated this revision to Diff 219578.Sep 10 2019, 11:34 AM

rebase & address comments.

Thank you for review. Could I another LGTM after rebase?

ychen updated this revision to Diff 220597.Sep 17 2019, 6:57 PM
  • Missing assignment of TheModule in move constructor.
arsenm accepted this revision.Sep 27 2019, 11:33 AM

LGTM

This revision was automatically updated to reflect the committed changes.
thakis added a subscriber: thakis.Sep 30 2019, 11:11 AM
thakis added inline comments.
llvm/trunk/include/llvm/CodeGen/MachineModuleInfo.h
151 ↗(On Diff #222461)

This causes

../../llvm/include/llvm/CodeGen/MachineModuleInfo.h:151:22: warning: explicitly defaulted move assignment operator is implicitly deleted [-Wdefaulted-function-deleted]
  MachineModuleInfo &operator=(MachineModuleInfo &&MMII) = default;
                     ^
../../llvm/include/llvm/CodeGen/MachineModuleInfo.h:82:28: note: move assignment operator of 'MachineModuleInfo' is implicitly deleted because field 'TM' is of reference type 'const llvm::LLVMTargetMachine &'
  const LLVMTargetMachine &TM;
                           ^

Fixed in r373244. Thanks!

rupprecht added inline comments.
llvm/trunk/include/llvm/CodeGen/MachineModuleInfo.h
268 ↗(On Diff #222461)

(reposting from the phab commit, since it looks like phab didn't send to any mailing lists...)

Is this method actually defined anywhere? I tried migrating an out of tree use of MachineModuleInfo to wrap this but I got a link error, and I don't think this is defined.

At any rate, I think I can just use the constructor above instead.

ychen added a comment.Sep 30 2019, 7:09 PM

Removed in r373297. Thanks.