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

[Heejin Ahn via Phabricator via llvm-commits]

aheejin 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);
----------------
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?


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyMachineFunctionInfo.cpp:161
+
+  // FIXME: This link should be removed
+  if (WasmEHFuncInfo *WasmEHInfo = MF.getWasmEHFuncInfo()) {
----------------
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.


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

https://reviews.llvm.org/D137295