llvm dev - Feb 2009 - [LLVMdev] Removal of GVStub methods from MachineCodeEmitter, ELFWriter, and MachOWriter

I have done a possible cleanup patch for the MachineCodeEmitter, ELFWriter,
and MachOWriter classes. It removes the two startGVStub(), and
finishGVStub() JIT specific methods.

You may remember the following comments :-

    /// JIT SPECIFIC FUNCTIONS - DO NOT IMPLEMENT THESE HERE!

To get rid of these easily turned out to be a semicomplex modification
because of the JITInfo classes dependance on MachineCodeEmitter and the
GVStub methods. Also the fact that JITEmitter is hidden and quite a large
class in a .cpp file.

At first I did this by introducing a JITCodeEmitter class between
MachineCodeEmitter and the (un)hidden JITEmitter. But this lead to more
dependancies and they lead on to more.

I have now made class header for JITEmitter putting it in the llvm
namespace. There were several other header dependancies as a result and
lib/ExecutionEngine/JIT/JIT.h and lib/ExecutionEngine/JIT/JITDwarfEmitter.h
required moving to the include/llvm/ExecutionEngine/JIT directory as a
result to make them visible to JIT.h and dependants.

    * create JITEmitter.h and made class JITEmitter visible in llvm
namespace
    * moved lib/ExecutionEngine/JIT/JIT.h to
include/llvm/ExecutionEngine/JIT
    * moved lib/ExecutionEngine/JIT/JITDwarfEmitter.h to
include/llvm/ExecutionEngine/JIT
    * removed JIT specific startGVStub()'s functions and finishGVStub() from
MachineCodeEmitter, ELFWriter, and  MachOWriter classes.
     * modified include/llvm/Target/TargetJITInfo.h to use JITEmitter rather
than MachineCodeEmitter.
    * modified :-
        - lib/Target/PowerPC/PPCJITInfo.h
        - lib/Target/PowerPC/PPCJITInfo.cpp
        - lib/Target/ARM/ARMJITInfo.cpp
        - lib/Target/ARM/ARMJITInfo.h
        - lib/Target/Alpha/AlphaJITInfo.cpp
        - lib/Target/Alpha/AlphaJITInfo.h
        - lib/Target/X86/X86JITInfo.h
        - lib/Target/X86/X86JITInfo.cpp
        to use JITEmitter rather than MachineCodeEmitter.
    * renamed multiple MCE's to JE's
    * removed unneed getCodeEmitter()'s in JITEmitter.cpp class JIT.
    * modified lib/ExecutionEngine/JIT/TargetSelect.cpp to use
include/llvm/ExecutionEngine/JIT/JIT.h include file
     * modified lib/ExecutionEngine/JIT/Intercept.cpp to use
include/llvm/ExecutionEngine/JIT/JIT.h include file

I was wondering whether it would be better to call the JITEmitter
class JITCodeEmitter and have JCE's instead of JE's.

I dont think there are any other issues I missed or need tidying up either.
I removed some getCodeEmitter and accompanying redundant assertions from
JITEmitter.cpp JIT class methods, AFAICT this is only cruft.

Anyway here's the patch :-

    http://www.aarongray.org/LLVM/patches/llvm-JITEmitter.patch

There was another way I considered that was having a second MCE for writting
global data and stubs to. I believe this would not require making JITEmitter
and friend visible, but may introduce more runtime overhead.

If you could have a look at this patch and see what you think.

If things go well I will be working on patches for the ELFWriter and an
accompanying COFFWriter. I know I promised this before but unfortunate
circumstances stopped me working on it back then.

Hopefully I can do some ground work and incremental patches to get some code
generated then look at the ABI issues on Windows.

Cheers,

Aaron
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20090228/c0247cd1/attachment.html>

I'll look at these. First scan looks good. Are you able to run some  
tests?

Evan

On Feb 28, 2009, at 9:36 AM, Aaron Gray wrote:
> I have done a possible cleanup patch for the MachineCodeEmitter,  
> ELFWriter, and MachOWriter classes. It removes the two  
> startGVStub(), and finishGVStub() JIT specific methods.
>
> You may remember the following comments :-
>
>     /// JIT SPECIFIC FUNCTIONS - DO NOT IMPLEMENT THESE HERE!
>
> To get rid of these easily turned out to be a semicomplex  
> modification because of the JITInfo classes dependance on  
> MachineCodeEmitter and the GVStub methods. Also the fact that  
> JITEmitter is hidden and quite a large class in a .cpp file.
>
> At first I did this by introducing a JITCodeEmitter class between  
> MachineCodeEmitter and the (un)hidden JITEmitter. But this lead to  
> more dependancies and they lead on to more.
>
> I have now made class header for JITEmitter putting it in the llvm  
> namespace. There were several other header dependancies as a result  
> and lib/ExecutionEngine/JIT/JIT.h and lib/ExecutionEngine/JIT/ 
> JITDwarfEmitter.h required moving to the include/llvm/ 
> ExecutionEngine/JIT directory as a result to make them visible to  
> JIT.h and dependants.
>
>     * create JITEmitter.h and made class JITEmitter visible in llvm  
> namespace
>     * moved lib/ExecutionEngine/JIT/JIT.h to include/llvm/ 
> ExecutionEngine/JIT
>     * moved lib/ExecutionEngine/JIT/JITDwarfEmitter.h to include/ 
> llvm/ExecutionEngine/JIT
>     * removed JIT specific startGVStub()'s functions and  
> finishGVStub() from MachineCodeEmitter, ELFWriter, and  MachOWriter  
> classes.
>     * modified include/llvm/Target/TargetJITInfo.h to use JITEmitter  
> rather than MachineCodeEmitter.
>     * modified :-
>         - lib/Target/PowerPC/PPCJITInfo.h
>         - lib/Target/PowerPC/PPCJITInfo.cpp
>         - lib/Target/ARM/ARMJITInfo.cpp
>         - lib/Target/ARM/ARMJITInfo.h
>         - lib/Target/Alpha/AlphaJITInfo.cpp
>         - lib/Target/Alpha/AlphaJITInfo.h
>         - lib/Target/X86/X86JITInfo.h
>         - lib/Target/X86/X86JITInfo.cpp
>         to use JITEmitter rather than MachineCodeEmitter.
>     * renamed multiple MCE's to JE's
>     * removed unneed getCodeEmitter()'s in JITEmitter.cpp class JIT.
>     * modified lib/ExecutionEngine/JIT/TargetSelect.cpp to use  
> include/llvm/ExecutionEngine/JIT/JIT.h include file
>     * modified lib/ExecutionEngine/JIT/Intercept.cpp to use include/ 
> llvm/ExecutionEngine/JIT/JIT.h include file
>
> I was wondering whether it would be better to call the JITEmitter  
> class JITCodeEmitter and have JCE's instead of JE's.
>
> I dont think there are any other issues I missed or need tidying up  
> either. I removed some getCodeEmitter and accompanying redundant  
> assertions from JITEmitter.cpp JIT class methods, AFAICT this is  
> only cruft.
>
> Anyway here's the patch :-
>
>     http://www.aarongray.org/LLVM/patches/llvm-JITEmitter.patch
>
> There was another way I considered that was having a second MCE for  
> writting global data and stubs to. I believe this would not require  
> making JITEmitter and friend visible, but may introduce more runtime  
> overhead.
>
> If you could have a look at this patch and see what you think.
>
> If things go well I will be working on patches for the ELFWriter and  
> an accompanying COFFWriter. I know I promised this before but  
> unfortunate circumstances stopped me working on it back then.
>
> Hopefully I can do some ground work and incremental patches to get  
> some code generated then look at the ABI issues on Windows.
>
> Cheers,
>
> Aaron
>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20090302/db4df688/attachment.html>

Hi Evan,

Providing Nick's Cygwin patch works then I am upto doing Cygwin testing.

I ran the GVStub removal patch on Linux X86 and it passes 'make check'
fine.

The only thing I was not sure of was JITEmitter.h's #includes, they may need
some attention.

Also the other thing was should this all be done by using a second
MachineCodeEmitter in class JIT to provide the GV functionality. If you think
that would be a better method I could provide a patch to do that.

Anyway see what you think.

Cheers,

Aaron



  ----- Original Message ----- 
  From: Evan Cheng 
  To: LLVM Developers Mailing List 
  Sent: Monday, March 02, 2009 5:19 PM
  Subject: Re: [LLVMdev] Removal of GVStub methods from
MachineCodeEmitter,ELFWriter, and MachOWriter


  I'll look at these. First scan looks good. Are you able to run some tests?


  Evan


  On Feb 28, 2009, at 9:36 AM, Aaron Gray wrote:


    I have done a possible cleanup patch for the MachineCodeEmitter, ELFWriter,
and MachOWriter classes. It removes the two startGVStub(), and finishGVStub()
JIT specific methods.

    You may remember the following comments :-

        /// JIT SPECIFIC FUNCTIONS - DO NOT IMPLEMENT THESE HERE!

    To get rid of these easily turned out to be a semicomplex modification
because of the JITInfo classes dependance on MachineCodeEmitter and the GVStub
methods. Also the fact that JITEmitter is hidden and quite a large class in a
.cpp file.

    At first I did this by introducing a JITCodeEmitter class between
MachineCodeEmitter and the (un)hidden JITEmitter. But this lead to more
dependancies and they lead on to more.

    I have now made class header for JITEmitter putting it in the llvm
namespace. There were several other header dependancies as a result and
lib/ExecutionEngine/JIT/JIT.h and lib/ExecutionEngine/JIT/JITDwarfEmitter.h
required moving to the include/llvm/ExecutionEngine/JIT directory as a result to
make them visible to JIT.h and dependants.

        * create JITEmitter.h and made class JITEmitter visible in llvm
namespace
        * moved lib/ExecutionEngine/JIT/JIT.h to
include/llvm/ExecutionEngine/JIT
        * moved lib/ExecutionEngine/JIT/JITDwarfEmitter.h to
include/llvm/ExecutionEngine/JIT
        * removed JIT specific startGVStub()'s functions and finishGVStub()
from MachineCodeEmitter, ELFWriter, and  MachOWriter classes.
        * modified include/llvm/Target/TargetJITInfo.h to use JITEmitter rather
than MachineCodeEmitter.
        * modified :-
            - lib/Target/PowerPC/PPCJITInfo.h
            - lib/Target/PowerPC/PPCJITInfo.cpp
            - lib/Target/ARM/ARMJITInfo.cpp
            - lib/Target/ARM/ARMJITInfo.h
            - lib/Target/Alpha/AlphaJITInfo.cpp
            - lib/Target/Alpha/AlphaJITInfo.h
            - lib/Target/X86/X86JITInfo.h
            - lib/Target/X86/X86JITInfo.cpp 
            to use JITEmitter rather than MachineCodeEmitter.
        * renamed multiple MCE's to JE's
        * removed unneed getCodeEmitter()'s in JITEmitter.cpp class JIT.
        * modified lib/ExecutionEngine/JIT/TargetSelect.cpp to use
include/llvm/ExecutionEngine/JIT/JIT.h include file
        * modified lib/ExecutionEngine/JIT/Intercept.cpp to use
include/llvm/ExecutionEngine/JIT/JIT.h include file

    I was wondering whether it would be better to call the JITEmitter class
JITCodeEmitter and have JCE's instead of JE's.

    I dont think there are any other issues I missed or need tidying up either.
I removed some getCodeEmitter and accompanying redundant assertions from
JITEmitter.cpp JIT class methods, AFAICT this is only cruft.

    Anyway here's the patch :-

        http://www.aarongray.org/LLVM/patches/llvm-JITEmitter.patch

    There was another way I considered that was having a second MCE for writting
global data and stubs to. I believe this would not require making JITEmitter and
friend visible, but may introduce more runtime overhead.

    If you could have a look at this patch and see what you think.

    If things go well I will be working on patches for the ELFWriter and an
accompanying COFFWriter. I know I promised this before but unfortunate
circumstances stopped me working on it back then.

    Hopefully I can do some ground work and incremental patches to get some code
generated then look at the ABI issues on Windows.

    Cheers,

    Aaron


    _______________________________________________
    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 --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20090302/658f015d/attachment.html>