<div dir="ltr">Hi David,<div><br></div><div>Comments inline:</div><div>--</div><div><div>> diff --git a/include/llvm/MC/MCMachObjectWriter.h b/include/llvm/MC/MCMachObjectWriter.h</div><div>> index 3c9a588..1630049 100644</div>
<div>> --- a/include/llvm/MC/MCMachObjectWriter.h</div><div>> +++ b/include/llvm/MC/MCMachObjectWriter.h</div><div>> @@ -17,6 +17,16 @@</div><div>> #include "llvm/MC/MCObjectWriter.h"</div><div>> #include "llvm/Object/MachOFormat.h"</div>
<div>> #include "llvm/Support/DataTypes.h"</div><div>> +</div><div>> +/**</div><div>> +<span class="" style="white-space:pre"> </span>Mach-O needs indirect symbols grouped by section.</div><div>> +<span class="" style="white-space:pre"> </span>Goal: 1</div>
<div>> + */</div><div>> +#define ORDER_INDIRECT_SYMBOLS_BY_SECTION 1</div><div>> +#if ORDER_INDIRECT_SYMBOLS_BY_SECTION</div><div>> +#include "llvm/ADT/SetVector.h"</div><div>> +#endif</div>
<div>> +</div><div><br></div><div>I would prefer to review if the patch was just applied to the existing file, not</div><div>conditionalized in this fashion, it just makes review easier (or at least closer</div><div>to what I am used to).</div>
<div><br></div><div>> #include <vector></div><div>> </div><div>> namespace llvm {</div><div>> @@ -112,6 +122,22 @@ class MachObjectWriter : public MCObjectWriter {</div><div>> </div><div>> /// @}</div>
<div>> </div><div>> +#if ORDER_INDIRECT_SYMBOLS_BY_SECTION</div><div>> + /// @name Indirect Symbol Table Data</div><div>> + /// @{</div><div>> +</div><div>> + typedef std::vector<const MCSymbol*> IndirectSymbol_list_type;</div>
<div>> + typedef DenseMap<const MCSectionData*, IndirectSymbol_list_type></div><div>> +<span class="" style="white-space:pre"> </span>IndirectSymbol_map_type;</div><div>> + // keep sections in order of appearance</div>
<div><br></div><div>Please use proper capitalization, etc. in comments</div><div> <a href="http://llvm.org/docs/CodingStandards.html#commenting">http://llvm.org/docs/CodingStandards.html#commenting</a></div><div>this applies in a couple other places as well.</div>
<div><br></div><div>> + typedef SetVector<const MCSectionData*><span class="" style="white-space:pre"> </span>IndirectSymbolSection_set_type;</div><div>> +</div><div>> + IndirectSymbol_map_type<span class="" style="white-space:pre"> </span>IndirectSymbolMap;</div>
<div>> + IndirectSymbolSection_set_type<span class="" style="white-space:pre"> </span>IndirectSymbolSections;</div><div>> +</div><div><br></div><div>There isn't a good reason to have both two separate densemaps from MCSectionData</div>
<div>to record the indirect symbol information (IndirectSymbolMap and</div><div>IndirectSymBase). Instead, can you introduce an extra structure that records the</div><div>per-section indirect symbol information and has both the list of indirect</div>
<div>symbols in that section and the offset (once computed) of those symbols).</div><div><br></div><div>Also, please don't introduce typedefs for types that are not commonly used</div><div>(e.g. IndirectSymbol_map_type), just write the densemap declaration inline.</div>
<div><br></div><div>> + /// @}</div><div>> +#endif</div><div>> +</div><div>> public:</div><div>> MachObjectWriter(MCMachObjectTargetWriter *MOTW, raw_ostream &_OS,</div><div>> bool _IsLittleEndian)</div>
<div>> diff --git a/lib/MC/MachObjectWriter.cpp b/lib/MC/MachObjectWriter.cpp</div><div>> index a5ba3c3..d3eaf19 100644</div><div>> --- a/lib/MC/MachObjectWriter.cpp</div><div>> +++ b/lib/MC/MachObjectWriter.cpp</div>
<div>> @@ -428,6 +428,7 @@ void MachObjectWriter::BindIndirectSymbols(MCAssembler &Asm) {</div><div>> //</div><div>> // FIXME: Revisit this when the dust settles.</div><div>> </div><div>> +#if !ORDER_INDIRECT_SYMBOLS_BY_SECTION</div>
<div>> // Bind non lazy symbol pointers first.</div><div>> unsigned IndirectIndex = 0;</div><div>> for (MCAssembler::indirect_symbol_iterator it = Asm.indirect_symbol_begin(),</div><div>> @@ -466,6 +467,46 @@ void MachObjectWriter::BindIndirectSymbols(MCAssembler &Asm) {</div>
<div>> if (Created)</div><div>> Entry.setFlags(Entry.getFlags() | 0x0001);</div><div>> }</div><div>> +#else // ORDER_INDIRECT_SYMBOLS_BY_SECTION</div><div>> + // sort indirect symbols by section</div>
<div>> + for (MCAssembler::indirect_symbol_iterator it = Asm.indirect_symbol_begin(),</div><div>> + ie = Asm.indirect_symbol_end(); it != ie; ++it) {</div><div>> + // track their sections by order of appearance</div>
<div>> + IndirectSymbolSections.insert(it->SectionData);</div><div>> + IndirectSymbolMap[it->SectionData].push_back(it->Symbol);</div><div>> + }</div><div><br></div><div>Can this use the same order of traversal as the existing code? That is, bind the</div>
<div>non lazy symbol pointers first and then make a second pass? You should be able</div><div>to do the creation of the symbol data in those same loops, and I believe that</div><div>will keep binary compatibility on some of the existing test cases.</div>
<div><br></div><div>> + // process indirect symbols by section</div><div>> + unsigned offset = 0;<span class="" style="white-space:pre"> </span>// running total of indirect symbol index offset</div><div>> + IndirectSymbolSection_set_type::const_iterator</div>
<div>> +<span class="" style="white-space:pre"> </span>i(IndirectSymbolSections.begin()), e(IndirectSymbolSections.end());</div><div>> + for ( ; i!=e; ++i) {</div><div>> + const IndirectSymbol_list_type& b(IndirectSymbolMap.find(*i)->second);</div>
<div>> + IndirectSymbol_list_type::const_iterator bi(b.begin()), be(b.end());</div><div>> + const MCSectionMachO& Section(cast<MCSectionMachO>((*i)->getSection()));</div><div>> + switch (Section.getType()) {</div>
<div>> + default: break;</div><div>> + case MCSectionMachO::S_NON_LAZY_SYMBOL_POINTERS: {</div><div>> + for ( ; bi!=be; ++bi)</div><div>> + Asm.getOrCreateSymbolData(**bi);</div><div>> + break;</div>
<div>> + }</div><div>> + case MCSectionMachO::S_LAZY_SYMBOL_POINTERS: // fall-through</div><div>> + case MCSectionMachO::S_SYMBOL_STUBS: {</div><div>> + for ( ; bi!=be; ++bi) {</div><div>> + // Set the symbol type to undefined lazy, but only on construction.</div>
<div>> + // FIXME: Do not hardcode.</div><div>> + bool Created;</div><div>> + MCSymbolData &Entry(Asm.getOrCreateSymbolData(**bi, &Created));</div><div>> + if (Created)</div><div>
> + Entry.setFlags(Entry.getFlags() | 0x0001);</div><div>> + }<span class="" style="white-space:pre"> </span>// end for</div><div>> + break;</div><div>> + }</div><div>> + }<span class="" style="white-space:pre"> </span>// end switch(sectionType)</div>
<div>> + IndirectSymBase.insert(std::make_pair(*i, offset));</div><div>> + offset += b.size();</div><div>> + }</div><div><br></div><div>Once the code in here to construct the symbol data moves into the above loop,</div>
<div>this one should just be a trivial pass to assign the offsets.</div><div><br></div><div>> +#endif<span class="" style="white-space:pre"> </span>// ORDER_INDIRECT_SYMBOLS_BY_SECTION</div><div>> }</div><div>> </div>
<div>> /// ComputeSymbolTable - Compute the symbol table data</div><div>> @@ -895,26 +936,44 @@ void MachObjectWriter::WriteObject(MCAssembler &Asm,</div><div>> // Write the symbol table data, if used.</div>
<div>> if (NumSymbols) {</div><div>> // Write the indirect symbol entries.</div><div>> +#if ORDER_INDIRECT_SYMBOLS_BY_SECTION</div><div>> + for (IndirectSymbolSection_set_type::const_iterator</div><div>
> + si(IndirectSymbolSections.begin()), se(IndirectSymbolSections.end());</div><div>> + si != se; ++si) {</div><div>> + const IndirectSymbol_list_type& l(IndirectSymbolMap.find(*si)->second);</div>
<div>> + for (IndirectSymbol_list_type::const_iterator</div><div>> + it(l.begin()), ie(l.end()); it != ie; ++it)</div><div>> +#else</div><div>> for (MCAssembler::const_indirect_symbol_iterator</div>
<div>> it = Asm.indirect_symbol_begin(),</div><div>> - ie = Asm.indirect_symbol_end(); it != ie; ++it) {</div><div>> + ie = Asm.indirect_symbol_end(); it != ie; ++it)</div><div>> +#endif</div>
<div>> + {</div><div>> // Indirect symbols in the non lazy symbol pointer section have some</div><div>> // special handling.</div><div>> const MCSectionMachO &Section =</div><div>> +#if ORDER_INDIRECT_SYMBOLS_BY_SECTION</div>
<div>> + static_cast<const MCSectionMachO&>((*si)->getSection());</div><div>> + const MCSymbol* Sym = *it;</div><div>> +#else</div><div>> static_cast<const MCSectionMachO&>(it->SectionData->getSection());</div>
<div>> + const MCSymbol* Sym = it->Symbol;</div><div>> +#endif</div><div>> if (Section.getType() == MCSectionMachO::S_NON_LAZY_SYMBOL_POINTERS) {</div><div>> // If this symbol is defined and internal, mark it as such.</div>
<div>> - if (it->Symbol->isDefined() &&</div><div>> - !Asm.getSymbolData(*it->Symbol).isExternal()) {</div><div>> + if (Sym->isDefined() &&</div><div>> + !Asm.getSymbolData(*Sym).isExternal()) {</div>
<div>> uint32_t Flags = macho::ISF_Local;</div><div>> - if (it->Symbol->isAbsolute())</div><div>> + if (Sym->isAbsolute())</div><div>> Flags |= macho::ISF_Absolute;</div>
<div>> Write32(Flags);</div><div>> continue;</div><div>> }</div><div>> }</div><div>> -</div><div>> - Write32(Asm.getSymbolData(*it->Symbol).getIndex());</div>
<div>> + Write32(Asm.getSymbolData(*Sym).getIndex());</div><div>> +#if ORDER_INDIRECT_SYMBOLS_BY_SECTION</div><div>> + }</div><div>> +#endif</div><div>> }</div><div>> </div></div><div><br>
</div><div>--</div><div><br></div><div style> - Daniel</div><div style><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Mar 27, 2013 at 5:33 PM, David Fang <span dir="ltr"><<a href="mailto:fang@csl.cornell.edu" target="_blank">fang@csl.cornell.edu</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Daniel,<br>
ping. Have any time to review patch.2?<br>
<br>
Fang<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5">
Daniel and all,<br>
Attached is take 2 of the patch, just moved the edits from MCAssembler to MachObjectWriter. The two indirect symbol tests still get their indirect symbols re-ordered, this patch includes those diffs. Is this closer to what you have in mind?<br>
On my handful of test cases, this still produces ordering that is consistent with /usr/bin/as (vs. integrated-as).<br>
<br>
Fang<br>
<br>
</div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5">
On Tue, Mar 19, 2013 at 2:27 AM, David Fang <<a href="mailto:fang@csl.cornell.edu" target="_blank">fang@csl.cornell.edu</a>> wrote:<br>
<br>
> Hi Daniel,<br>
> Thanks for reviewing the patch. I thought I tried suggestion #1<br>
> before this patch in > <a href="http://llvm.org/bugs/show_bug.**cgi?id=14636#c33" target="_blank">http://llvm.org/bugs/show_bug.<u></u>**cgi?id=14636#c33</a><<a href="http://llvm.org/bugs/show_bug.cgi?id=14636#c33" target="_blank">http://<u></u>llvm.org/bugs/show_bug.cgi?id=<u></u>14636#c33</a>>,<br>
> which only reordeed IndirectSymBase without reordering WriteObject, and<br>
> didn't quite produce the desired result. I may revisit it again.<br>
> If I reorder IndirectSymBase, I need to also reorder in<br>
> WriteObject, so am I allowed to move the IndirectSymbol (mapped by > section)<br>
> and IndirectSymbolSection (order of first appearance) structures out of<br>
> MCAssembler into MachObjectWriter? I need the ordering information from<br>
> the first pass of BindIndirectSymbols to persist until WriteObject, > without<br>
> having to sort twice. This transformation is intended for X86,PPC<br>
> (probably ARM as well) +mach-o back-ends after all.<br>
> <br>
I don't understand what the issue is here, you can of course add extra<br>
data<br>
structures into the MachObjectWriter class and compute them during<br>
BindIndirectSymbols() and reuse them during WriteObject(). You shouldn't<br>
need to change anything in the MCAssembler.<br>
<br>
Basically I am just suggesting moving the building up of the Mach-O<br>
specific data structures into BindIndirectSymbols() and the<br>
MachObjectWriter class.<br>
<br>
- Daniel<br>
<br>
<br>
> > Fang<br>
> > Hi David,<br>
> > > > Ok, here are my main high level comments on the patch. I have some<br>
> > specific<br>
> > nits too but I'll save them for later:<br>
> > > > #1. I would rather this patch not require modifying the MCAssembler > > level<br>
> > interface. Instead, what I would like is for the new data structures<br>
> > (IndirectSymbols and IndirectSymbolSections) to get built up inside<br>
> > BindIndirectSymbols().<br>
> > > > #2. I don't think this patch should actually cause test cases to > > change.<br>
> > Most of those test cases were generated based on matching existing > > 'as'<br>
> > output for x86, and the way I envisioned this patch working I wouldn't<br>
> > have<br>
> > expected it to change the order of symbols except in the case where > > the<br>
> > indirect symbols would be out of order.<br>
> > > > I believe that if you fix #1 then #2 will fall out, because the > > indirect<br>
> > symbol lists will get built up in the same order as they are now > > (based on<br>
> > iterating the indirect symbol list twice).<br>
> > > > - Daniel<br>
> > > > > > On Thu, Mar 14, 2013 at 10:26 AM, David Fang <<a href="mailto:fang@csl.cornell.edu" target="_blank">fang@csl.cornell.edu</a>><br>
> > wrote:<br>
> > > > Sure, Daniel, you seem pretty active, I can ping again in a week.<br>
> > > > > > patch notes: The patch is written so that changing a single #if in<br>
> > > MCAssembler.h at the top will effectively revert it (for ease of<br>
> > > testing).<br>
> > > A few typedefs for the new structures in MCAssembler are made public > > > just<br>
> > > for convenience. It looks like I'm deep in FIXME territory, judging > > > by<br>
> > > the<br>
> > > comments in the touched files. Feel free to pass a revised patch > > > back to<br>
> > > me as you see fit.<br>
> > > > > > Fang<br>
> > > > > > Hi David,<br>
> > > > > > > > > > > Thanks for working on a patch, I'll take a look but it may take me > > > > a<br>
> > > > couple<br>
> > > > days to get to it.<br>
> > > > > > > > - Daniel<br>
> > > > > > > > > > > > On Wed, Mar 13, 2013 at 1:48 PM, David Fang <<a href="mailto:fang@csl.cornell.edu" target="_blank">fang@csl.cornell.edu</a>><br>
> > > > wrote:<br>
> > > > > > > > Daniel,<br>
> > > > > > > > > The resulting LLVM test diffs with my patch are shown > > > > > here:<br>
> > > > > <a href="http://llvm.org/bugs/show_bug.******cgi?id=14636#c38" target="_blank">http://llvm.org/bugs/show_bug.<u></u>******cgi?id=14636#c38</a><<a href="http://llvm.org/bugs/show_bug.****cgi?id=14636#c38" target="_blank">http://<u></u>llvm.org/bugs/show_bug.****<u></u>cgi?id=14636#c38</a>><br>
> > > > > <http://**<a href="http://llvm.org/bugs/show_bug.**cgi?**id=14636#c38" target="_blank">llvm.org/bugs/show_<u></u>bug.**cgi?**id=14636#c38</a><<a href="http://llvm.org/bugs/show_bug.**cgi?id=14636#c38" target="_blank">http:<u></u>//llvm.org/bugs/show_bug.**<u></u>cgi?id=14636#c38</a>><br>
> > > > > > > > > > > <http://**<a href="http://llvm.org/bugs/show_**bug.cgi?id=**14636#c38" target="_blank">llvm.org/bugs/show_*<u></u>*bug.cgi?id=**14636#c38</a><<a href="http://llvm.org/bugs/show_bug.cgi?id=**14636#c38" target="_blank">http:/<u></u>/llvm.org/bugs/show_bug.cgi?<u></u>id=**14636#c38</a>><br>
> > > > > <http://**<a href="http://llvm.org/bugs/show_bug.cgi?id=**14636#c38" target="_blank">llvm.org/bugs/show_<u></u>bug.cgi?id=**14636#c38</a><<a href="http://llvm.org/bugs/show_bug.cgi?id=14636#c38" target="_blank">http://<u></u>llvm.org/bugs/show_bug.cgi?id=<u></u>14636#c38</a>><br>
> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 2 tests "failed" in MC/MachO: indirect-symbols.s and > > > > > symbol-indirect.s<br>
> > > > > No other LLVM/clang tests regressed (Targets: PPC,X86,ARM).<br>
> > > > > > > > > > The differences are that symbols are re-ordered, which > > > > > accomplishes the<br>
> > > > > objective of the patch. There are FIXME comments that require a<br>
> > > > > maintainer's scrutiny.<br>
> > > > > Could you review these differences that result from my patch?<br>
> > > > > The patch as-is now is in proof-of-concept form. I'd be happy > > > > > to<br>
> > > > > revise<br>
> > > > > as you suggest.<br>
> > > > > If this patch benefits all Mach-O architectures (getting closer > > > > > to<br>
> > > > > usable<br>
> > > > > integrated assembly), I'd like to get this into trunk.<br>
> > > > > > > > > > Fang<br>
> > > > > > > > > > > > > > > Michael, Daniel,<br>
> > > > > > > > > > The attached patch I wrote works for me so far on my > > > > > small<br>
> > > > > > test<br>
> > > > > > cases: llvm/clang's object file matches the indirect symbol > > > > > > ordering<br>
> > > > > > of<br>
> > > > > > the<br>
> > > > > > /usr/bin/as-assembled object's (tested on my powerpc-darwin8 > > > > > > branch).<br>
> > > > > > Michael, could you try it out on your own test cases?<br>
> > > > > > Daniel, could you kindly comment and review? Just the > > > > > > technical<br>
> > > > > > merits,<br>
> > > > > > not style.<br>
> > > > > > I've also attached the same patch to bug 14636.<br>
> > > > > > > > > > > > Thanks for taking a look.<br>
> > > > > > > > > > > > Fang<br>
> > > > > > > > > > > > Daniel,<br>
> > > > > > > > > > > > Some quick questions about your proposed recipe:<br>
> > > > > > > > > > > > > > Ok, here is what I think the right fix is. Instead of > > > > > > > creating the<br>
> > > > > > > > > > > > > > > IndirectSymBase mapping we use to associate sections > > > > > > > > with their<br>
> > > > > > > > > indirect offset start, in BindIndirectSymbols() we > > > > > > > > > should:<br>
> > > > > > > > > > > > > > > > > > 1. Add a simple container struct (lets say > ><br>
> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > MachSectionIndirectSymbols)<br>
> > > > > > > > > > > > > > > > > > > > > > > > > > for tracking the per-section list of indirect symbols. It > > > > > > > will<br>
> > > > > > > > keep<br>
> > > > > > > > > > > > > > > > > the list of symbols in the section and the index of > > > > > > > > > the first > ><br>
> > > > > > > > > > > > > > > > > > indirect<br>
> > > > > > > > > > > > > > > > > > > > > > symbol in the section.<br>
> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Like a map<section*, vector<indirect_symbol> >?<br>
> > > > > > > > > > > > > > Instead of keeping track of the first indirect symbol index, > > > > > > > could<br>
> > > > > > > we<br>
> > > > > > > just<br>
> > > > > > > as easily compute the index offsets later during the second > > > > > > > pass:<br>
> > > > > > > > > > > > > > offset = 0;<br>
> > > > > > > for each section S (in order)<br>
> > > > > > > record offset for section S<br>
> > > > > > > [optional] write vector of indirect symbols for S<br>
> > > > > > > offset += S.num_indirect_symbols();<br>
> > > > > > > end for<br>
> > > > > > > > > > > > > > I was able to fix the reserve1 field by doing something like > > > > > > > this<br>
> > > > > > > (bug<br>
> > > > > > > 14636, comments 30-33), but I only later discovered that the<br>
> > > > > > > indirect<br>
> > > > > > > symbols needed to be reordered.<br>
> > > > > > > > > > > > > > 2. Keep a mapping from sections to the above type.<br>
> > > > > > > > > > > > > > > 3. Add a SetVector to record the order of the sections > > > > > > > > that have<br>
> > > > > > > > > indirect symbols.<br>
> > > > > > > > > > > > > > > > > > > > > > > > > > This I understand, I found SetVector and SmallSetVector > > > > > > > > in ADT.<br>
> > > > > > > > > > > > > > 4. During BindIndirectSymbols() we maintain the above > > > > > > > information<br>
> > > > > > > > > > > > > > > (populating the MachSectionIndirectSymbols per-section > > > > > > > > symbol > ><br>
> > > > > > > > > > > > > > > > > > arrays).<br>
> > > > > > > > > > > > > > > > > > > > > > BindIndirectSymbols looks like it need not change, because > > > > > > > it's<br>
> > > > > > > already<br>
> > > > > > > operating on the assumption of ordered indirect symbols, > > > > > > > right?<br>
> > > > > > > > > > > > > > 5. Once we have scanned all the symbols we make another > > > > > > > pass over<br>
> > > > > > > > > > > > > > > > > > > > > > > > the<br>
> > > > > > > > > > > > > > > > > > > > > > sections (in the order seen via indirect symbols) and > > > > > > > assign the ><br>
> > > > > > > > > > > > > > > > > > > > > > > > > > start<br>
> > > > > > > > > > > > > > > > indices.<br>
> > > > > > > > > > > > > > > > > > > > > > > > > > as I outlined above (offset = start indices).<br>
> > > > > > > > > > > > > > 6. Update writing of the indirect symbol table to write > > > > > > > in the<br>
> > > > > > > same<br>
> > > > > > > > > > > > > > > order as traversed in #5.<br>
> > > > > > > > > > > > > > > > > > Does that make sense? It's more work than your patch > > > > > > > > > but it<br>
> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > (a) > > should<br>
> > > > > > > > > > > > > > > > > > > > > > > > > > preserve binary compatibility with 'as' in situations > > > > > > > where the<br>
> > > > > > > > > > > > > > > > > indirect symbols don't appear out of order w.r.t. the > > > > > > > > > sections,<br>
> > > > > > > > > > > > > > > > > > (b) > > it<br>
> > > > > > > > > > > > > > > > > > > > > > makes somewhere more explicit the relationship between > > > > > > > sections<br>
> > > > > > > > and<br>
> > > > > > > > > > > > > > > > > their list of symbols being in contiguous order.<br>
> > > > > > > > > > > > > > > > > > - Daniel<br>
> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Will this re-ordering be acceptable to all Mach-O > > > > > > > > > > back-ends?<br>
> > > > > > > x86,<br>
> > > > > > > ARM,<br>
> > > > > > > PPC?<br>
> > > > > > > > > > > > > > Fang<br>
> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > --<br>
> > > > > > > > > > > David Fang<br>
> > > > > <a href="http://www.csl.cornell.edu/~******fang/" target="_blank">http://www.csl.cornell.edu/~**<u></u>****fang/</a><<a href="http://www.csl.cornell.edu/~****fang/" target="_blank">http://www.csl.<u></u>cornell.edu/~****fang/</a>><br>
> > > > > <<a href="http://www.csl." target="_blank">http://www.csl.</a>**<a href="http://cornell.edu/~**fang/" target="_blank">cornell.edu/<u></u>~**fang/</a><<a href="http://www.csl.cornell.edu/~**fang/" target="_blank">http://www.csl.<u></u>cornell.edu/~**fang/</a>><br>
> > > > > > <<br>
> > > > > http: //<a href="http://www.csl.cornell.edu/~****fang/" target="_blank">www.csl.cornell.edu/~****<u></u>fang/</a><<a href="http://www.csl.cornell.edu/~**fang/" target="_blank">http://www.csl.cornell.<u></u>edu/~**fang/</a>><<br>
> > > > > http: //<a href="http://www.csl.cornell.edu/~**fang/" target="_blank">www.csl.cornell.edu/~**fang/</a><u></u><<a href="http://www.csl.cornell.edu/~fang/" target="_blank">http://www.csl.cornell.edu/~<u></u>fang/</a>><br>
> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > --<br>
> > > David Fang<br>
> > > http: //<a href="http://www.csl.cornell.edu/~****fang/" target="_blank">www.csl.cornell.edu/~****<u></u>fang/</a><<a href="http://www.csl.cornell.edu/~**fang/" target="_blank">http://www.csl.cornell.<u></u>edu/~**fang/</a>><<br>
</div></div>
> > > http: //<a href="http://www.csl.cornell.edu/~**fang/" target="_blank">www.csl.cornell.edu/~**fang/</a> > > > http: <<a href="http://www.csl.cornell.edu/~fang/" target="_blank">http://www.csl.cornell.edu/~<u></u>fang/</a>>><div class="im">
<br>
> > > > > > > > > > > > --<br>
> David Fang<br>
> <a href="http://www.csl.cornell.edu/~**fang/" target="_blank">http://www.csl.cornell.edu/~**<u></u>fang/</a> <<a href="http://www.csl.cornell.edu/~fang/" target="_blank">http://www.csl.cornell.edu/~<u></u>fang/</a>><br>
> > <br>
</div></blockquote>
<br>
<br>
</blockquote><div class="HOEnZb"><div class="h5">
<br>
-- <br>
David Fang<br>
<a href="http://www.csl.cornell.edu/~fang/" target="_blank">http://www.csl.cornell.edu/~<u></u>fang/</a><br>
<br>
</div></div></blockquote></div><br></div>