llvm dev - Aug 2011 - [LLVMdev] ARMCodeEmitter.cpp JIT support very broken (2.9 and svn)

On Aug 30, 2011, at 3:12 PM, Owen Anderson wrote:
> The non-MC-based ARM JIT path is known not to work, and nobody is working
on fixing it.  The MC-based instruction encoder is rapidly maturing is generally
passable for static encoding, but the MCJIT is still in its infancy.
I was relying on this support in LLVM 2.8, and while it is definitely
incomplete, it does work if you don't depend on certain features.  I found
it worked adequately as long as you set the -thumb2,-t2xtpk attributes.

It would be nice if at least it didn't get further broken until the MC
Emitter was fully ready to take its place.   The first FIXME comment in
ARMCodeEmitter seems to indicate this is a goal.: "… They are placeholders
to allow this encoder to continue to function until the MC encoder is
sufficiently far along that this one can be eliminated entirely."
>> 1) VFP/Neon instructions don't encode correctly at al, because the
encoding methods generated by tablegen for them clobber the constructed binary
value when they try to implement 'PostEncoderMethod' support , for
example, from ARMGenCodeEmitter.inc:
>> 
>> 
>> The bug here is that in utils/TableGen/CodeEmitterGen.cpp, line 196:
>> 	Case += "      Value = " + PostEmitter + "(MI,
Value);\n";
>> should be
>> 	Case += "      Value |= " + PostEmitter + "(MI,
Value);\n";
>> 
> 
> This is the intended behavior.  The some post-encoder hooks need to clear
bits as well as set them.  If you're seeing incorrect output from the
post-encoder hook, it's because the hook itself has a bug.
Ah, ok.  So it looks like the stubs in ARMCodeEmitter.cpp such as 
unsigned VFPThumb2PostEncoder(const MachineInstr&MI, unsigned Val) const {
return 0; }
should instead be 
unsigned VFPThumb2PostEncoder(const MachineInstr&MI, unsigned Val) const {
return Val; }

>> This looks like it would affect all targets, except apparently only ARM
uses this feature.
>> 
>> 2) ARM BR_JTm and BR_JTadd do not emit because they were changed to
PseudoInstructions but the ARMCodeEmitter wasn't updated to compensate.  
emitPseudoInstruction() asserts (llvm_unreachable).
> 
> This is another symptom of the non-MC ARM JIT being unmaintained.  It is
correct for emitPseudoInstruction() to assert.  All pseudo instructions should
be expanded before they reach the encoder, and they are properly expanded in the
MC-based path.
But there are several PseudoInstruciton still around and handled here in the
non-MC based path.   Again, it'd be nice to not obsolete old code until
it's replacement is actually ready.

Hi Craig,


The problem with this is that the ARM JIT was never gotten to
"supported" status at any point, so regressions were not monitored.
The code path is essentially dead, at the moment, with noone willing to invest
time in flogging a dead horse as it'll all have to be rewritten when MC
lands properly and someone has the time/inclination to architect it.

I understand your frustration at it being allowed to code rot, but if it was
never complete with proper tests and noone cares enough, that is naturally going
to happen.

Cheers,

James
________________________________________
From: llvmdev-bounces at cs.uiuc.edu [llvmdev-bounces at cs.uiuc.edu] On Behalf
Of Craig Smith [craig at ni.com]
Sent: 30 August 2011 22:01
To: LLVM Developers Mailing List
Cc: Owen Anderson
Subject: Re: [LLVMdev] ARMCodeEmitter.cpp JIT support very broken (2.9 and     
svn)

On Aug 30, 2011, at 3:12 PM, Owen Anderson wrote:
> The non-MC-based ARM JIT path is known not to work, and nobody is working
on fixing it.  The MC-based instruction encoder is rapidly maturing is generally
passable for static encoding, but the MCJIT is still in its infancy.
I was relying on this support in LLVM 2.8, and while it is definitely
incomplete, it does work if you don't depend on certain features.  I found
it worked adequately as long as you set the -thumb2,-t2xtpk attributes.

It would be nice if at least it didn't get further broken until the MC
Emitter was fully ready to take its place.   The first FIXME comment in
ARMCodeEmitter seems to indicate this is a goal.: "… They are placeholders
to allow this encoder to continue to function until the MC encoder is
sufficiently far along that this one can be eliminated entirely."
>> 1) VFP/Neon instructions don't encode correctly at al, because the
encoding methods generated by tablegen for them clobber the constructed binary
value when they try to implement 'PostEncoderMethod' support , for
example, from ARMGenCodeEmitter.inc:
>>
>>
>> The bug here is that in utils/TableGen/CodeEmitterGen.cpp, line 196:
>>      Case += "      Value = " + PostEmitter + "(MI,
Value);\n";
>> should be
>>      Case += "      Value |= " + PostEmitter + "(MI,
Value);\n";
>>
>
> This is the intended behavior.  The some post-encoder hooks need to clear
bits as well as set them.  If you're seeing incorrect output from the
post-encoder hook, it's because the hook itself has a bug.
Ah, ok.  So it looks like the stubs in ARMCodeEmitter.cpp such as
unsigned VFPThumb2PostEncoder(const MachineInstr&MI, unsigned Val) const {
return 0; }
should instead be
unsigned VFPThumb2PostEncoder(const MachineInstr&MI, unsigned Val) const {
return Val; }

>> This looks like it would affect all targets, except apparently only ARM
uses this feature.
>>
>> 2) ARM BR_JTm and BR_JTadd do not emit because they were changed to
PseudoInstructions but the ARMCodeEmitter wasn't updated to compensate.  
emitPseudoInstruction() asserts (llvm_unreachable).
>
> This is another symptom of the non-MC ARM JIT being unmaintained.  It is
correct for emitPseudoInstruction() to assert.  All pseudo instructions should
be expanded before they reach the encoder, and they are properly expanded in the
MC-based path.
But there are several PseudoInstruciton still around and handled here in the
non-MC based path.   Again, it'd be nice to not obsolete old code until
it's replacement is actually ready.







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


-- IMPORTANT NOTICE: The contents of this email and any attachments are
confidential and may also be privileged. If you are not the intended recipient,
please notify the sender immediately and do not disclose the contents to any
other person, use it for any purpose, or store or copy the information in any
medium.  Thank you.

Is MC JIT support expected in 3.0, and if not, what does the timeline look like?

Would I be better off trying to get the supported but incomplete MC JITter
working than spending effort preserving the dead branch?

Thanks
Craig

On Aug 30, 2011, at 4:44 PM, James Molloy wrote:
> Hi Craig,
> 
> 
> The problem with this is that the ARM JIT was never gotten to
"supported" status at any point, so regressions were not monitored.
The code path is essentially dead, at the moment, with noone willing to invest
time in flogging a dead horse as it'll all have to be rewritten when MC
lands properly and someone has the time/inclination to architect it.
> 
> I understand your frustration at it being allowed to code rot, but if it
was never complete with proper tests and noone cares enough, that is naturally
going to happen.
> 
> Cheers,
> 
> James
> ________________________________________
> From: llvmdev-bounces at cs.uiuc.edu [llvmdev-bounces at cs.uiuc.edu] On
Behalf Of Craig Smith [craig at ni.com]
> Sent: 30 August 2011 22:01
> To: LLVM Developers Mailing List
> Cc: Owen Anderson
> Subject: Re: [LLVMdev] ARMCodeEmitter.cpp JIT support very broken (2.9 and 
svn)
> 
> On Aug 30, 2011, at 3:12 PM, Owen Anderson wrote:
>> The non-MC-based ARM JIT path is known not to work, and nobody is
working on fixing it.  The MC-based instruction encoder is rapidly maturing is
generally passable for static encoding, but the MCJIT is still in its infancy.
> 
> I was relying on this support in LLVM 2.8, and while it is definitely
incomplete, it does work if you don't depend on certain features.  I found
it worked adequately as long as you set the -thumb2,-t2xtpk attributes.
> 
> It would be nice if at least it didn't get further broken until the MC
Emitter was fully ready to take its place.   The first FIXME comment in
ARMCodeEmitter seems to indicate this is a goal.: "… They are placeholders
to allow this encoder to continue to function until the MC encoder is
sufficiently far along that this one can be eliminated entirely."
> 
>>> 1) VFP/Neon instructions don't encode correctly at al, because
the encoding methods generated by tablegen for them clobber the constructed
binary value when they try to implement 'PostEncoderMethod' support ,
for example, from ARMGenCodeEmitter.inc:
>>> 
>>> 
>>> The bug here is that in utils/TableGen/CodeEmitterGen.cpp, line
196:
>>>     Case += "      Value = " + PostEmitter + "(MI,
Value);\n";
>>> should be
>>>     Case += "      Value |= " + PostEmitter + "(MI,
Value);\n";
>>> 
>> 
>> This is the intended behavior.  The some post-encoder hooks need to
clear bits as well as set them.  If you're seeing incorrect output from the
post-encoder hook, it's because the hook itself has a bug.
> 
> Ah, ok.  So it looks like the stubs in ARMCodeEmitter.cpp such as
> unsigned VFPThumb2PostEncoder(const MachineInstr&MI, unsigned Val)
const { return 0; }
> should instead be
> unsigned VFPThumb2PostEncoder(const MachineInstr&MI, unsigned Val)
const { return Val; }
> 
> 
>>> This looks like it would affect all targets, except apparently only
ARM uses this feature.
>>> 
>>> 2) ARM BR_JTm and BR_JTadd do not emit because they were changed to
PseudoInstructions but the ARMCodeEmitter wasn't updated to compensate.  
emitPseudoInstruction() asserts (llvm_unreachable).
>> 
>> This is another symptom of the non-MC ARM JIT being unmaintained.  It
is correct for emitPseudoInstruction() to assert.  All pseudo instructions
should be expanded before they reach the encoder, and they are properly expanded
in the MC-based path.
> 
> But there are several PseudoInstruciton still around and handled here in
the non-MC based path.   Again, it'd be nice to not obsolete old code until
it's replacement is actually ready.
> 
> 
> 
> 
> 
> 
> 
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
> 
> 
> -- IMPORTANT NOTICE: The contents of this email and any attachments are
confidential and may also be privileged. If you are not the intended recipient,
please notify the sender immediately and do not disclose the contents to any
other person, use it for any purpose, or store or copy the information in any
medium.  Thank you.
>