llvm dev - Apr 2008 - [LLVMdev] Being able to know the jitted code-size before emitting

>
> How about a  default GetInstSize() as well? Return 1 for every  
> instruction except for some special TargetInstrInfo instructions, e.g.  
> PHI, IMPLICIT_DEF, LABEL. I don't know if it's useful or not. But  
> perhaps we can default most targets to it?
>
>   
I prefer not giving a default implementation and aborting with a message 
that says the target did not implement it. This way we know why it's not 
working. Is it OK?
>> //
>> -unsigned X86RegisterInfo::getX86RegNum(unsigned RegNo) const {
>> +unsigned X86RegisterInfo::getX86RegNum(unsigned RegNo) {
>>     
>
> What happened to "const"?
>   
I made the function static.
>> +  unsigned Size = GetInstSizeWithDesc(*MI, &Desc, IsPIC,  
>> Is64BitMode);
>> +  if (Desc.getOpcode() == X86::MOVPC32r) {
>> +      Size += GetInstSizeWithDesc(*MI, &get(X86::POP32r), IsPIC,  
>> Is64BitMode);
>> +  }
>>     
>
> I would prefer this special case is handled in GetInstSizeWithDesc().
>
>   
I can not do that in GetInstSizeWithDesc because the test is on the 
MachineInstruction, which will always say that Desc.getOpcode == MOVPC32 
in GetInstSizeWithDesc (hence the function will endlessly call itself).
X86CodeEmitter is doing the same.
>
> Looks great! Thanks.
>   
Thx to you for reviewing the patch! I'll modify it with your suggestions 
and wait for your response to this email before committing it.

Nicolas
> Evan
>
>   
>> _______________________________________________
>> LLVM Developers mailing list
>> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>>     
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>

Nicolas Geoffray wrote:
>
> I can not do that in GetInstSizeWithDesc because the test is on the 
> MachineInstruction, which will always say that Desc.getOpcode == MOVPC32 
> in GetInstSizeWithDesc (hence the function will endlessly call itself).
> X86CodeEmitter is doing the same.
>
>   
OK, forget this comment, it was wrong. The reason is that the get 
function requires an TargetInstrInfo object and that GetInstSizeWithDesc 
does not have and GetInstSizeInBytes has.

Nicolas

On Apr 16, 2008, at 1:46 AM, Nicolas Geoffray wrote:
>
>>
>> How about a  default GetInstSize() as well? Return 1 for every
>> instruction except for some special TargetInstrInfo instructions,  
>> e.g.
>> PHI, IMPLICIT_DEF, LABEL. I don't know if it's useful or not.
But
>> perhaps we can default most targets to it?
>>
>>
>
> I prefer not giving a default implementation and aborting with a  
> message
> that says the target did not implement it. This way we know why it's  
> not
> working. Is it OK?
That's ok.
>
>
>>> //
>>> -unsigned X86RegisterInfo::getX86RegNum(unsigned RegNo) const {
>>> +unsigned X86RegisterInfo::getX86RegNum(unsigned RegNo) {
>>>
>>
>> What happened to "const"?
>>
>
> I made the function static.
Ok.
>
>
>>> +  unsigned Size = GetInstSizeWithDesc(*MI, &Desc, IsPIC,
>>> Is64BitMode);
>>> +  if (Desc.getOpcode() == X86::MOVPC32r) {
>>> +      Size += GetInstSizeWithDesc(*MI, &get(X86::POP32r),
IsPIC,
>>> Is64BitMode);
>>> +  }
>>>
>>
>> I would prefer this special case is handled in GetInstSizeWithDesc().
>>
>>
>
> I can not do that in GetInstSizeWithDesc because the test is on the
> MachineInstruction, which will always say that Desc.getOpcode ==  
> MOVPC32
> in GetInstSizeWithDesc (hence the function will endlessly call  
> itself).
> X86CodeEmitter is doing the same.
Ok.
>
>
>>
>> Looks great! Thanks.
>>
>
> Thx to you for reviewing the patch! I'll modify it with your  
> suggestions
> and wait for your response to this email before committing it.
>
Thanks,

Evan
> Nicolas
>
>> Evan
>>
>>
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>>>
>>
>> _______________________________________________
>> LLVM Developers mailing list
>> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

Thx again Evan for the review. Here's a new patch for the JIT in itself. 
The major changes are:
1) A JITMemoryManager now has a flag saying "I require to know the size 
of what you want to emit"
2) DwarfJITEmitter is augmented with GetSize* functions
3) JITEmitter::startFunction checks if the JITMemoryManager requires to 
know the size. If so, it computes it and gives it through the ActualPtr 
argument.

I suppose it's OK to commit, but if anyone wants to complain, I'm 
listening :)

Thanks,
Nicolas


Evan Cheng wrote:
> On Apr 16, 2008, at 1:46 AM, Nicolas Geoffray wrote:
>
>   
>>> How about a  default GetInstSize() as well? Return 1 for every
>>> instruction except for some special TargetInstrInfo instructions,  
>>> e.g.
>>> PHI, IMPLICIT_DEF, LABEL. I don't know if it's useful or
not. But
>>> perhaps we can default most targets to it?
>>>
>>>
>>>       
>> I prefer not giving a default implementation and aborting with a  
>> message
>> that says the target did not implement it. This way we know why
it's
>> not
>> working. Is it OK?
>>     
>
> That's ok.
>
>   
>>     
>>>> //
>>>> -unsigned X86RegisterInfo::getX86RegNum(unsigned RegNo) const {
>>>> +unsigned X86RegisterInfo::getX86RegNum(unsigned RegNo) {
>>>>
>>>>         
>>> What happened to "const"?
>>>
>>>       
>> I made the function static.
>>     
>
> Ok.
>
>   
>>     
>>>> +  unsigned Size = GetInstSizeWithDesc(*MI, &Desc, IsPIC,
>>>> Is64BitMode);
>>>> +  if (Desc.getOpcode() == X86::MOVPC32r) {
>>>> +      Size += GetInstSizeWithDesc(*MI, &get(X86::POP32r),
IsPIC,
>>>> Is64BitMode);
>>>> +  }
>>>>
>>>>         
>>> I would prefer this special case is handled in
GetInstSizeWithDesc().
>>>
>>>
>>>       
>> I can not do that in GetInstSizeWithDesc because the test is on the
>> MachineInstruction, which will always say that Desc.getOpcode ==  
>> MOVPC32
>> in GetInstSizeWithDesc (hence the function will endlessly call  
>> itself).
>> X86CodeEmitter is doing the same.
>>     
>
> Ok.
>
>   
>>     
>>> Looks great! Thanks.
>>>
>>>       
>> Thx to you for reviewing the patch! I'll modify it with your  
>> suggestions
>> and wait for your response to this email before committing it.
>>
>>     
>
> Thanks,
>
> Evan
>
>   
>> Nicolas
>>
>>     
>>> Evan
>>>
>>>
>>>       
>>>> _______________________________________________
>>>> LLVM Developers mailing list
>>>> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>>>>
>>>>         
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>>>
>>>       
>> _______________________________________________
>> LLVM Developers mailing list
>> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>>     
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>   
-------------- next part --------------
A non-text attachment was scrubbed...
Name: jit-size.patch
Type: text/x-patch
Size: 22557 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20080417/e2106922/attachment.bin>