Hi Dan,<div><br></div><div>I'm not sure what you mean by vtable construction cost. Do you mean the additional memory to store the vptr? I can't deny that a switch-based implementation uses a little less memory and is probably slightly faster (one less memory read & a closer jump on dispatch). However, I think the resulting code could be significantly cleaner and more extensible.</div>
<div><br></div><div>As you can probably tell, I've been working on adding native-client instruction bundling to the MC assembler. The modifications are almost finished. Unfortunately, it has been quite an unpleasant experience.</div>
<div><meta http-equiv="content-type" content="text/html; charset=utf-8"><div><br></div></div><div>One of the things I did was to add a new fragment type which can store multiple instructions (usually about 3), each with its own fixups and (possibly) each needing relaxation separately. The existing relaxation and lowering mechanism assumes that only MCInstFragment needs relaxation and lowering. I had to copy & paste & modify most of the relaxation code in order to make it relax my new fragment type. If there were instead virtual methods NeedRelaxation() and PerformRelaxation() and LowerFragment() on fragments, this would not have been necessary.</div>
<div><br></div><div>I also needed to control the creation of fragments. With bundling enabled, the creation of a fragment sometimes needs to trigger the substitution for a different type of fragment. Unfortunately, the creation of fragments (e.g. "new MCAlignFragment(...)") is sprinkled all over the MCObjectStreamer subclasses instead of being private to MCAssembler. This made the required changes messy and spread out.</div>
<div><br></div><div>I'm curious as to why the the assembler doesn't keep fragments as private data. Instead, they get passed and manipulated all over the place. Fragments even get passed directly to the TargetAsmBackend! The number of places in the code that "knows" about a fragments is alarming.</div>
<div><br></div><div>I think that ideally, fragment creation could have been hidden behind functions like:</div><div><meta http-equiv="content-type" content="text/html; charset=utf-8"><div><br></div><div><font class="Apple-style-span" face="'courier new', monospace">// Emits into MCInstFragment or into the current MCDataFragment</font></div>
</div><div><font class="Apple-style-span" face="'courier new', monospace">MCAssembler::EmitInstruction(MCSectionData *Parent, const MCInst &Inst);</font></div><div><font class="Apple-style-span" face="'courier new', monospace"><br>
</font></div><div><font class="Apple-style-span" face="'courier new', monospace">MCAssembler::EmitValueToAlignment(...);</font></div><div><font class="Apple-style-span" face="'courier new', monospace"><br>
</font></div><div><font class="Apple-style-span" face="'courier new', monospace">MCAssember::EmitCodeAlignment(...);</font></div><div><font class="Apple-style-span" face="'courier new', monospace"><br></font></div>
<div><font class="Apple-style-span" face="'courier new', monospace">MCAssembler::EmitOrg(...);</font></div><div><font class="Apple-style-span" face="'courier new', monospace"><br></font></div><div><font class="Apple-style-span" face="'courier new', monospace">MCAssembler::EmitDwarf(...);</font></div>
<div><br></div><div>I'd like to talk more about this. I'd like to get the bundling feature clean enough to be added to mainstream LLVM MC. The changes are mostly innocuous, but little things are getting in the way of making the implementation compact. Unfortunately, I'm under intense pressure to get this stuff working a week ago (in our local branch). I can't focus on refactoring at the moment.</div>
<div><br></div><div>When I'm finished, I'll send you a ping.</div><div><br></div><div>Thanks,</div><div>  David M</div><div><br></div><div><br><div class="gmail_quote">On Fri, Nov 5, 2010 at 11:09 AM, Daniel Dunbar <span dir="ltr"><<a href="mailto:daniel@zuster.org">daniel@zuster.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">Hi David,<br>
<br>
MCFragment actually used to be virtual.<br>
<br>
I eventually killed this because in practice I felt like the number of<br>
interesting fragments was likely to be small, and the vtable<br>
construction cost wasn't worth it.<br>
<br>
I would propose making an extensible MCFragment subclass to cover the<br>
case of wanting to add plugable MCFragments, instead of imposing a<br>
vtable construction cost on the core classes (which was construct many<br>
hundreds of, and don't actually dispatch on much).<br>
<font color="#888888"><br>
 - Daniel<br>
</font><div><div></div><div class="h5"><br></div></div></blockquote></div></div>