[llvm-commits] [PATCH] TableGen CodeEmitter Backend for JITCodeEmitter

[Jim Grosbach]

Hi Logan,

Where do you see the call to getBinaryCodeForInstr() coming from? The pass begins in runOnMachineFunction(), which calls emitInstruction() for each instruction in each basic block. emitInstruction() proceeds like:

void ARMCodeEmitter::emitInstruction(const MachineInstr &MI) {
  DEBUG(errs() << "JIT: " << (void*)MCE.getCurrentPCValue() << ":\t" << MI);

  MCE.processDebugLoc(MI.getDebugLoc(), true);

  ++NumEmitted;  // Keep track of the # of mi's emitted
  switch (MI.getDesc().TSFlags & ARMII::FormMask) {
...
  case ARMII::Pseudo:
    emitPseudoInstruction(MI);
    break;
…

Note that there is at no point a call to getBinaryCodeForInstr() before the call to emitPseudoInstruction(). You're probably seeing calls to getBinaryCodeForInstr() due to pseudo-instructions that are being handled incorrectly in the CodeEmitter during emitPseudoInstruction(). Those are bugs in the CodeEmitter, not a failure of how pseudos are defined. Patches to fix those bugs would be very welcome.

-Jim

On Jul 22, 2011, at 5:12 PM, Logan Chien wrote:

> Hi,
> 
>   I agree with you that pseudos are handled (and encoded) in ARMCodeEmitter::emitPseudoInstruction.
> But unfortunately, emitPseudoInstruction will try to call CodeEmitter::getBinaryCodeForInstr first, then
> getBinaryCodeForInstr will report a fatal error.  That's why I tried to add new 'case' (though we are doing
> nothing in getBinaryCodeForInstr.)  You may trace the control flow while emitting LDMIA_RET or B,
> and you will see why this patch is essential.  Thanks.
> 
> Sincerely,
> Logan
> 
> On Fri, Jul 22, 2011 at 11:40 PM, Jim Grosbach <grosbach at apple.com> wrote:
> This isn't what you want to do. The reason pseudos are excluded is that they don't have any binary information to emit. Adding them to the getBinaryCodeForInstr() won't help, as it won't return any bits of the instruction anyway.
> 
> Pseudos are handled in ARMCodeEmitter::emitPseudoInstruction(), and that function is required to fill in all required bits for the instruction(s) the pseudo expands to.
> 
> -Jim
> 
> On Jul 21, 2011, at 7:27 PM, Logan Chien wrote:
> 
>> Dear all,
>> 
>>   With commit r134539, the CodeEmitter backend of TableGen will not generate case for
>> pseudo instruction.  It seems that r134539 assumed that the pseudo instruction will
>> be lowered before calling CodeEmitter::getBinaryCodeForInstr.  Thus, getBinaryCodeForInstr
>> will report a fatal error if it gets a pseudo instruction.
>> 
>>   However, ARMCodeEmitter does not lower the pseudo instruction before calling
>> getBinaryCodeForInstr, and ARMCodeEmitter will no longer work after r134539.
>> With this patch, we will generate case for pseudo instruction for CodeEmitter (but not
>> MCCodeEmitter) just like what was done before.
>> 
>>   Please review.  Thanks!
>> 
>> Sincerely,
>> Logan
>> <0001-Fix-ARMCodeEmitter-pseudo-instruction-encoder.patch>_______________________________________________
>> llvm-commits mailing list
>> 
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110722/7fa05d90/attachment.html>