[PATCH] D137295: WebAssembly: Remove MachineFunction reference from MFI

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyMachineFunctionInfo.cpp:31
     const {
-  WebAssemblyFunctionInfo *Clone =
-      DestMF.cloneInfo<WebAssemblyFunctionInfo>(*this);
-  Clone->MF = &DestMF;
-  return Clone;
+  // FIXME: This is broken if WasmEHFuncInfo from the block references.
+  return DestMF.cloneInfo<WebAssemblyFunctionInfo>(*this);
----------------
aheejin wrote:
> arsenm wrote:
> > aheejin wrote:
> > > What does this mean by "if WasmEHFuncInfo from the block reference"?
> > Ultimately WasmEHFuncInfo has a bunch of MachineBasicBlock pointers in it. When cloned to a new machine function, those need to be updated to point to the blocks in the new function which isn't handled
> Apparently I'm not familiar with the cloning functionality you added a while ago. What is that for? Is that creating an in-memory copy of a while function, including BBs and instructions? Then what's src BB and what's dst BB? 
> 
> And how does `WinEHFuncInfo` handle it currently?
It's for llvm-reduce; it's just unhandled now. If you try to reduce a function with EH info it won't faithfully reproduce 


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyMachineFunctionInfo.cpp:161
+
+  // FIXME: This link should be removed
+  if (WasmEHFuncInfo *WasmEHInfo = MF.getWasmEHFuncInfo()) {
----------------
aheejin wrote:
> arsenm wrote:
> > aheejin wrote:
> > > Why? Which link?
> > This is the layering violation where this field is stored in the MachineFunction, but all the handling is done here in target code. Either everything should be done here, or done in the generic MIR handling 
> As I said, `WinEHFuncInfo` is also within `MachineFunction`. Also this `WasmEHFuncInfo` is not only for MIR handling. It is referenced throughout our compilation passes.
Right, it's a well spread out layering violation. The handling should be consistently in webassembly only, or consistently in the generic MachineFunction handling


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137295/new/

https://reviews.llvm.org/D137295