<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">Comments inline. I tried to reply to
particular points of discussion downthread, so this is probably
best read last. <br>
<br>
On 07/09/2015 02:04 PM, Juergen Ributzka wrote:<br>
</div>
<blockquote
cite="mid:BDBBE029-F849-4816-B35D-A975B845B978@apple.com"
type="cite">
<meta http-equiv="Content-Type" content="text/html;
charset=windows-1252">
Hi @ll,
<div class=""><br class="">
</div>
<div class="">over the past year we gained more experience with
the patchpoint/stackmap/statepoint intrinsics and it exposed
limitations in the stackmap format.</div>
<div class="">The following proposal includes feedback and request
from several interested parties and I would like to hear your
feedback.</div>
<div class=""><br class="">
</div>
<div class="">Missing correlation between functions and stackmap
records:</div>
<div class="">Originally the client had to keep track of the ID to
know which stackmap record belongs to which function, but this
would stop working once functions are inlined.</div>
<div class="">The new format fixes that by adding a direct
reference from the function to the stackmap records.</div>
</blockquote>
+1<br>
<blockquote
cite="mid:BDBBE029-F849-4816-B35D-A975B845B978@apple.com"
type="cite">
<div class=""><br class="">
</div>
<div class="">Call Size and Function Size:</div>
<div class="">These are additional information that are of
interest and have been added to the format.</div>
<div class="">@Swaroop: Could you please provide a little more
detailed explanation on the "Call Size" field and what exactly
is there recorded. Is it just the call instruction</div>
<div class="">or also the materialization code for the address?
For what is this used for?</div>
</blockquote>
+1 subject to details being settled<br>
<blockquote
cite="mid:BDBBE029-F849-4816-B35D-A975B845B978@apple.com"
type="cite">
<div class=""><br class="">
</div>
<div class="">Flat format:</div>
<div class="">We think moving to a flat form will make parsing
easier, because every record has a fixed size and offsets can be
calculated easily. </div>
</blockquote>
I'm still not convinced this is actually a good idea, but I have no
strong objection either.
<blockquote
cite="mid:BDBBE029-F849-4816-B35D-A975B845B978@apple.com"
type="cite">
<div class="">The plan is to also</div>
<div class="">provide parsers for both stackmap versions (there is
already one for the first format in tree) and a corresponding
C-API to make it easier for clients to</div>
<div class="">adopt the new format. There is no plan to drop the
original format and we will continue to support both formats. I
will ask for feedback on the C API in a</div>
<div class="">separate RFC. <br>
</div>
</blockquote>
I strongly feel that trying to support multiple versions of the file
format in tree is a bad idea. It's a distraction, support burden,
and provides extremely little value. What's the value in supporting
a parser for a format that ToT will no longer be able to emit?
Anyone who is actually consuming the old format would have to be
using the old version in process anyways. <br>
<br>
(Possible exception: Is this intended to simplify the object caching
model? If so, have we ever given any guarantees about object
caching in the JIT across versions? That seems risky at best..)<br>
<br>
I'm also opposed to the complexity of C API, but I'll defer that
until the separate RFC is actually posted. <br>
<blockquote
cite="mid:BDBBE029-F849-4816-B35D-A975B845B978@apple.com"
type="cite">
<div class=""><br class="">
</div>
<div class="">Another benefit we hope to achieve from this format
is to optimize for size by uniquing entries - but that is
independent optimization and not required.</div>
<div class=""><br class="">
</div>
<div class="">More detailed frame record:</div>
<div class="">Clients require more information about the function
frame, such as spilled registers, etc. The frame base register
i.e. might change when dynamic stack</div>
<div class="">realignment is performed on X86.</div>
<div class=""><br class="">
</div>
<div class=""><br class="">
</div>
<div class="">If there is anything missing please let me know.</div>
<div class=""><br class="">
</div>
<div class="">Thanks</div>
<div class=""><br class="">
</div>
<div class="">Cheers,</div>
<div class="">Juergen</div>
<div class=""><br class="">
</div>
<div class=""><br class="">
</div>
</blockquote>
Is the format <font face="Courier New">Little endian, bit endian?
native endian?<br>
</font>
<blockquote
cite="mid:BDBBE029-F849-4816-B35D-A975B845B978@apple.com"
type="cite">
<div class="">
<div class=""><font class="" face="Courier New">Header v2 {<br
class="">
uint8 : Stack Map Version (2)<br class="">
uint8 : Reserved [3] (0)<br class="">
uint32 : Constants Offset (bytes)<br class="">
uint32 : Frame Records Offset (bytes)<br class="">
uint32 : Frame Registers Offset (bytes)<br class="">
uint32 : StackMap Records Offset (bytes)<br class="">
uint32 : Locations Offset (bytes)<br class="">
uint32 : LiveOuts Offset (bytes)</font></div>
</div>
</blockquote>
<font face="Courier New">Let's reserve at least two uint32s for
future field groups so that we can extend in a backwards
compatible way. Another option would be to steal one of the
reserved bytes and require parsers to ignore sections they don't
understand. </font><br>
<br>
Actually, so long as we document that there might be unknown bytes
between Header.end and Constants.begin we should be fine. It just
needs to be *clearly* documented.<br>
<br>
As an example, if we need to add the SymbolConstants section
mentioned down thread, that shouldn't require an version change. <br>
<blockquote
cite="mid:BDBBE029-F849-4816-B35D-A975B845B978@apple.com"
type="cite">
<div class="">
<div class=""><font class="" face="Courier New">}<br class="">
<br class="">
align to 8 bytes<br class="">
Constants[] {<br class="">
uint64 : LargeConstant<br class="">
}<br class="">
<br class="">
align to 8 bytes<br class="">
FrameRecord[] {<br class="">
uint64 : Function Address<br class="">
uint32 : Function Size<br class="">
uint32 : Stack Size<br class="">
uint16 : Flags {<br class="">
bool : HasFrame<br class="">
</font></div>
</div>
</blockquote>
<font face="Courier New">What does this mean? </font><br>
<blockquote
cite="mid:BDBBE029-F849-4816-B35D-A975B845B978@apple.com"
type="cite">
<div class="">
<div class=""><font class="" face="Courier New"> bool :
HasVariableSizeAlloca<br class="">
bool : HasStackRealignment<br class="">
bool : HasLiveOutInfo<br class="">
bool : Reserved [12]<br class="">
</font></div>
</div>
</blockquote>
<br>
<blockquote
cite="mid:BDBBE029-F849-4816-B35D-A975B845B978@apple.com"
type="cite">
<div class="">
<div class=""><font class="" face="Courier New"> }<br class="">
uint16 : Frame Base Register Dwarf RegNum<br class="">
</font></div>
</div>
</blockquote>
<font face="Courier New">I don't think this is actually sufficient
for dynamic stack realignment. Don't you end up with some things
indexed off RSP and others indexed off RBP?</font> Actually,
wait.. that's entirely handled within the record format. What does
tracking the Frame Base Register give us? I think I'm missing a use
case here.<br>
<blockquote
cite="mid:BDBBE029-F849-4816-B35D-A975B845B978@apple.com"
type="cite">
<div class="">
<div class=""><font class="" face="Courier New"> uint16 : Num
Frame Registers<br class="">
uint16 : Frame Register Index<br class="">
</font></div>
</div>
</blockquote>
<font face="Courier New">Are these (begin, begin+length) offsets
into the Frame Registers table? If so, should we swap them?</font><br>
<blockquote
cite="mid:BDBBE029-F849-4816-B35D-A975B845B978@apple.com"
type="cite">
<div class="">
<div class=""><font class="" face="Courier New"> uint16 : Num
StackMap Records<br class="">
uint16 : StackMap Record Index</font></div>
<div class=""><font class="" face="Courier New">}<br class="">
<br class="">
align to 4 bytes<br class="">
FrameRegister[] {<br class="">
uint16 : Dwarf RegNum<br class="">
int16 : Offset<br class="">
uint8 : Size in Bytes<br class="">
uint8 : Flags {<br class="">
bool : IsSpilled<br class="">
bool : Reserved [7]<br class="">
}<br class="">
}<br class="">
</font></div>
</div>
</blockquote>
<font face="Courier New">This seems tied specifically to the
assumption that registers are spilled on entry. Given the recent
work towards shrink wrapping, is that something we want to do?<br>
<br>
p.s. What's with the IsSpilled bit? Why would you have a record
for an unspilled register?<br>
</font>
<blockquote
cite="mid:BDBBE029-F849-4816-B35D-A975B845B978@apple.com"
type="cite">
<div class="">
<div class=""><font class="" face="Courier New"><br class="">
align to 8 bytes<br class="">
StackMapRecord[] {<br class="">
uint64 : PatchPoint ID<br class="">
</font></div>
</div>
</blockquote>
<font face="Courier New">"ID" (remove the patchpoint since
statepoint now has one too)</font><br>
<blockquote
cite="mid:BDBBE029-F849-4816-B35D-A975B845B978@apple.com"
type="cite">
<div class="">
<div class=""><font class="" face="Courier New"> uint32 :
Instruction Offset<br class="">
uint8 : Call size (bytes)<br class="">
</font></div>
</div>
</blockquote>
<font face="Courier New">"Patchable size" per down thread discussion</font><br>
<blockquote
cite="mid:BDBBE029-F849-4816-B35D-A975B845B978@apple.com"
type="cite">
<div class="">
<div class=""><font class="" face="Courier New"> uint8 : Flags
{<br class="">
bool : HasLiveOutInfo<br class="">
bool : Reserved [7]<br class="">
}<br class="">
</font></div>
</div>
</blockquote>
<font face="Courier New">Why do we need these flags? Isn't this Num
LiveOuts == 0?</font><br>
<blockquote
cite="mid:BDBBE029-F849-4816-B35D-A975B845B978@apple.com"
type="cite">
<div class="">
<div class=""><font class="" face="Courier New"> uint16 : Num
Locations<br class="">
uint16 : Location Index<br class="">
uint16 : Num LiveOuts<br class="">
uint16 : LiveOut Index<br class="">
}</font></div>
<div class=""><span style="font-family: 'Courier New';" class=""><br
class="">
</span></div>
<div class=""><span style="font-family: 'Courier New';" class="">align
to 4 bytes</span></div>
<div class=""><font class="" face="Courier New">Location[] {<br
class="">
uint8 : Register | Direct | Indirect | Constant |
ConstantIndex<br class="">
uint8 : Reserved (location flags)<br class="">
uint16 : Dwarf RegNum<br class="">
int32 : Offset or SmallConstant<br class="">
}<br class="">
<br class="">
align to 2 bytes<br class="">
LiveOuts[] {<br class="">
uint16 : Dwarf RegNum<br class="">
uint8 : Reserved<br class="">
uint8 : Size in Bytes<br class="">
}</font></div>
<div class=""><br class="">
</div>
</div>
</blockquote>
<br>
</body>
</html>