2007/8/17, Evan Cheng <[EMAIL PROTECTED]>:
>
> Very good progress. Thanks!
>
> Comments inline.
>
> Evan
>
> ...
>
> Please use abort() instead so it does what's expected in non-debug build.
>
OK
...
>
> i s -> is :-)
>
OK
Also, why the name "ARMCompilationCallbackC"? Is it language specific?
>
I used the same naming convention of
PPCJITInfo.cpp("PPCCompilationCallbackC"). I can also use naming
convention of
X86ITInfo.cpp, which uses "X86CompilationCallback2").
...
>
> Does a similar assertion makes sense here?
>
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).
...
>
> 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?
>
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.
Also, in Emitter::getBinaryCodeForInstr():
>
> unsigned Emitter::getBinaryCodeForInstr(const MachineInstr &MI) {
> const TargetInstrDescriptor *Desc = MI.getInstrDescriptor();
> const unsigned opcode = MI.getOpcode();
> unsigned Value = 0xE0000000;
>
>
> Comments? What is 0xe000000?
>
Ok. I'll comment. It is an initial instruction mask.
>
> 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.
>
You're rigth. I'll define them in ARMII enum.
+ MCE.emitWordLE(addr);
> } else {
> - MCE.startFunctionStub(5, 2);
> - MCE.emitByte(0xEB); // branch and link to the corresponding function
> addr
> + // branch and link to the corresponding function addr
> + MCE.startFunctionStub(20, 4);
> + MCE.emitWordLE(0xE92D4800); // STMFD SP!, [R11, LR]
> + MCE.emitWordLE(0xE28FE004); // ADD LR, PC, #4
> + MCE.emitWordLE(0xE51FF004); // LDR PC, [PC,#-4]
> + MCE.emitWordLE(addr);
> + MCE.emitWordLE(0xE8BD8800); // LDMFD SP!, [R11, PC]
>
> Ditto.
>
There are comments for the hexa numbers emitted (MCE.emitWordLE). In such
code, I'd better comment instructionsMCE.startFunctionStub().
> switch ((ARM::RelocationType)MR->getRelocationType()) {
> case ARM::reloc_arm_relative: {
> // PC relative relocation
> - *((unsigned*)RelocPos) += (unsigned)ResultPtr;
> + ResultPtr = ResultPtr-(intptr_t)RelocPos-8;
> + if (ResultPtr >= 0)
> + *((unsigned*)RelocPos) |= 1 << 23;
> + else {
> + ResultPtr *= -1;
> + *((unsigned*)RelocPos) &= 0xFF7FFFFF;
>
>
> Please explain what's going on here? :-)
>
:-). 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.
Instead of special casing it for LDRD, perhaps add a LB (L bit) class and
> attach to the other instructions that need it?
>
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).
I'd like to see PUWLSH bits modeled more clearly.
>
> 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:
bit P: there are three classes of addr. I check if it is IndexModePost to
set it to 1.
bit U: I have to check immed value (U=1 is possitive, U=0 it is possitive)
bit L: cI've created classes for it (ARMII::StFrm, ARMII::LdFrm).
bit S: I've created classes for it
(ARMII::DPRImS,ARMII::DPRRegS,ARMII::DPRSoRegS).
bit W: is set according to addr mode.
--
Raul Fernandes Herbster
Embedded and Pervasive Computing Laboratory - embedded.dee.ufcg.edu.br
Electrical Engineering Department - DEE - www.ee.ufcg.edu.br
Electrical Engineering and Informatics Center - CEEI
Federal University of Campina Grande - UFCG - www.ufcg.edu.br
Caixa Postal 10105
58109-970 Campina Grande - PB - Brasil
_______________________________________________
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
