<br><br><div><span class="gmail_quote">2007/8/17, Evan Cheng <<a href="mailto:evan.cheng@apple.com">evan.cheng@apple.com</a>>:</span><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div style="">
<div></div><div>Very good progress. Thanks!</div><div><br></div><div>Comments inline.</div><div><br></div><div>Evan</div><div><br>...<br><div><br></div>Please use abort() instead so it does what's expected in non-debug build.
</div></div></blockquote><div>OK<br> </div><br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div style=""><div>...<br><div><br></div>i s -> is :-)
</div></div></blockquote><div><br>OK <br></div><br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div style=""><div>Also, why the name "ARMCompilationCallbackC"? Is it language specific?
</div></div></blockquote><div><br>I used the same naming convention of PPCJITInfo.cpp ("PPCCompilationCallbackC"). I can also use naming convention of X86ITInfo.cpp, which uses "X86CompilationCallback2").
</div><br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div style=""><div>...<br><div><br></div>Does a similar assertion makes sense here?
</div></div></blockquote><div><br>I dont think so. Such code is only called for branch and link instruction. In fact, the stub calls a function with MOV and LDR, instead of BL/B (because that problem of 24-bits field).<br>
</div><br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div style=""><div>...<br><div><br></div>This is ok.... But I would rather see you refactor getBinaryCodeForInstr() so you can "manufacture" the value by passing it ARM::LDR, ARM::PC, etc.? Do you think that's possible?
</div></div></blockquote><div><br>I don't think it is possible. If I "manufacture" the value by passing the opcode (ARM::LDR, ARM::PC...), I'll have to implement a big switch table as I did before (you have already comment this solution before). Generating the instructions using its classes it's better.
<br></div><br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div style=""><div>Also, in Emitter::getBinaryCodeForInstr():</div><div><br></div>
<blockquote style="border: medium none ; margin: 0pt 0pt 0pt 40px; padding: 0px;">unsigned Emitter::getBinaryCodeForInstr(const MachineInstr &MI) {<br> const TargetInstrDescriptor *Desc = MI.getInstrDescriptor();<br>
const unsigned opcode = MI.getOpcode();<br> unsigned Value = 0xE0000000;</blockquote><div><br></div><div>Comments? What is 0xe000000?</div></div></blockquote><div><br>Ok. I'll comment. It is an initial instruction mask.
<br><br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div style=""><br><div><br></div>Can 12 (and all the magic shift amounts in this function) be defined in ARMII enum? So you add comments there rather than in this code.
</div></blockquote><div><br>You're rigth. I'll define them in ARMII enum.<br></div><br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div style=""><div><div><blockquote type="cite"><div>+ MCE.emitWordLE(addr);</div><div> } else {</div><div>- MCE.startFunctionStub(5, 2);</div><div>- MCE.emitByte(0xEB); // branch and link to the corresponding function addr
</div><div>+ // branch and link to the corresponding function addr</div><div>+ MCE.startFunctionStub(20, 4);</div><div>+ MCE.emitWordLE(0xE92D4800); // STMFD SP!, [R11, LR]</div><div>+ MCE.emitWordLE(0xE28FE004); // ADD LR, PC, #4
</div><div>+ MCE.emitWordLE(0xE51FF004); // LDR PC, [PC,#-4]</div><div>+ MCE.emitWordLE(addr);</div><div>+ MCE.emitWordLE(0xE8BD8800); // LDMFD SP!, [R11, PC]</div></blockquote>Ditto.</div></div></div></blockquote>
<div> <br>There are comments for the hexa numbers emitted (MCE.emitWordLE). In such code, I'd better comment instructionsMCE.startFunctionStub().<br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div style=""><div><div><blockquote type="cite"><div> switch ((ARM::RelocationType)MR->getRelocationType()) {</div><div> case ARM::reloc_arm_relative: {</div><div> // PC relative relocation</div><div>- *((unsigned*)RelocPos) += (unsigned)ResultPtr;
</div><div>+ ResultPtr = ResultPtr-(intptr_t)RelocPos-8;</div><div>+ if (ResultPtr >= 0)</div><div>+ *((unsigned*)RelocPos) |= 1 << 23;</div><div>+ else {</div><div>+ ResultPtr *= -1;
</div><div>+ *((unsigned*)RelocPos) &= 0xFF7FFFFF;</div></blockquote><div><br></div>Please explain what's going on here? :-)</div></div></div></blockquote><div><br>:-). OK. Evan, sorry for not making comments. In certain functions of files
PPCJITInfo.cpp and X86JITInfo.cpp , I did not see any comments so I though that I could ignore them. I'll explain it.<br></div><br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div style=""><div><div></div></div></div></blockquote><div> </div><br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div style=""><div><div>
Instead of special casing it for LDRD, perhaps add a LB (L bit) class and attach to the other instructions that need it?</div></div></div></blockquote><div>LdFrm/StFrm is used to set bit L. However, the instruction LDRD is an "Enhanced DSP Extension" instruction (page A10-8) and it doesn't have an L bit to be set (neither STRD).
<br> </div><br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div style=""><div><div> I'd like to see PUWLSH bits modeled more clearly.
</div><div><br></div></div></div></blockquote></div>In order to model bits PUWLSH, I'll have to create more classes. However, some information about such bits can be retrieved from some other information:<br>bit P: there are three classes of addr. I check if it is IndexModePost to set it to 1.
<br>bit U: I have to check immed value (U=1 is possitive, U=0 it is possitive)<br>bit L: cI've created classes for it (ARMII::StFrm, ARMII::LdFrm).<br>bit S: I've created classes for it (ARMII::DPRImS,ARMII::DPRRegS,ARMII::DPRSoRegS).
<br>bit W: is set according to addr mode.<br clear="all"><br>-- <br>Raul Fernandes Herbster<br>Embedded and Pervasive Computing Laboratory - <a href="http://embedded.dee.ufcg.edu.br">embedded.dee.ufcg.edu.br</a><br>Electrical Engineering Department - DEE -
<a href="http://www.ee.ufcg.edu.br">www.ee.ufcg.edu.br</a><br>Electrical Engineering and Informatics Center - CEEI<br>Federal University of Campina Grande - UFCG - <a href="http://www.ufcg.edu.br">www.ufcg.edu.br</a><br>Caixa Postal 10105
<br>58109-970 Campina Grande - PB - Brasil