[PATCH] D53892: [CodeGen] Support custom format of stack maps

[Cherry Zhang via Phabricator via llvm-commits]

cherry added inline comments.


================
Comment at: include/llvm/CodeGen/GCMetadataPrinter.h:68
+  /// returns false and the default format will be used.
+  virtual bool emitStackMaps(StackMaps &SM, AsmPrinter &AP) { return false; }
 };
----------------
reames wrote:
> This new call is arguably duplicating the existing finishModule callback.  It'd be nice to find a way to merge the two honestly.  
I agree that this would be nice. However, given the signature of finishAssembly, assuming we don't want to change that, there are some problems:

- we need to pass a StackMaps object. One possibility is to link it to the GCModuleInfo object.

- finishAssembly doesn't return anything, but we need a return value to decide whether to use the default format.

Any idea of how to solve these?



================
Comment at: lib/CodeGen/AsmPrinter/AsmPrinter.cpp:3005-3007
+      if (MP->emitStackMaps(SM, *this))
+        // We've emitted a custom format, done.
+        return;
----------------
apilipenko wrote:
> reames wrote:
> > cherry wrote:
> > > apilipenko wrote:
> > > > I'm not sure that early termination is the right thing to do. There might be more than one GC strategy in use in the current module, and all of them might want to emit stack maps.
> > > In theory there could be more than one GC strategies, although I couldn't see how it can be used.
> > > 
> > > What about changing it to running all the GCMetadataPrinters, then return if any of the emitStackMaps returns true?
> > > 
> > The structure of the existing code is that a single model can contain functions compiled with two different gc strategies.  I agree it's a bit odd, but it should continue to work.
> > 
> > I'd also like to move towards being able to emit *multiple* formats for a single GC.  (Primarily for testing purposes.)  That can be out of scope for your change if you desire.  
> You can make the value returned from emitStackMaps indicate whether the default stackmap is needed. You would print the default format (`SM.serializeToStackMapSection `) if at least one of the MPs requested default format serialization.
> 
> If a strategy doesn't have a MP than by default it requests default serialization printing. 
Done as @apilipenko suggested.

For emitting multiple formats, the hook GCMetadataPrinter::emitStackMaps itself can do it. It can also call SM.serializeToStackMapSection to emit the default format (along with a custom format).



Repository:
  rL LLVM

https://reviews.llvm.org/D53892