[llvm-commits] [llvm] r72631 - in /llvm/trunk: include/llvm/CodeGen/ include/llvm/Target/ lib/CodeGen/ lib/ExecutionEngine/JIT/ lib/Target/ARM/ lib/Target/Alpha/ lib/Target/PowerPC/ lib/Target/X86/

[Anton Korobeynikov]

Hello, Bruno

> First patch in the direction of splitting MachineCodeEmitter in two subclasses:
> JITCodeEmitter and ObjectCodeEmitter. No functional changes yet. Patch by Aaron Gray
Sorry, I was not able to look into it before

> +  void emitWordLE(unsigned W) {
> +    if (4 <= BufferEnd-CurBufferPtr) {
> +      *CurBufferPtr++ = (unsigned char)(W >>  0);
> +      *CurBufferPtr++ = (unsigned char)(W >>  8);
> +      *CurBufferPtr++ = (unsigned char)(W >> 16);
> +      *CurBufferPtr++ = (unsigned char)(W >> 24);
> +    } else {
> +      CurBufferPtr = BufferEnd;
> +    }
> +  }
Please use uint8_t here and everywhere instead of unsigned char.

> +  /// emitInt32 - Emit a int32 directive.
> +  void emitInt32(int Value) {
Please use int32_t here. Same for other places.


> +
> +FunctionPass *createARMCodeEmitterPass(
> +    ARMTargetMachine &TM, MachineCodeEmitter &MCE);
> +/*
> +template< class machineCodeEmitter>
> +FunctionPass *createARMCodeEmitterPass(
> +    ARMTargetMachine &TM, machineCodeEmitter &MCE);
> +*/
Huh? Also, please follow LLVM Coding Style convention.

> +
> +  template< class machineCodeEmitter>
No space before "class". Maybe it's better to select another name for
template? MachineCodeEmitter vs machineCodeEmitter looks ambiguous.

> +  class VISIBILITY_HIDDEN Emitter : public MachineFunctionPass, 
> +    	public ARMCodeEmitter
> +  {
Here also. "{" is normally put on the same line as class def.

> +
> +namespace llvm {
> +	
> +FunctionPass *createARMCodeEmitterPass(
> +    ARMTargetMachine &TM, MachineCodeEmitter &MCE)
> +{
> +  return new Emitter<MachineCodeEmitter>(TM, MCE);
> +}
> +FunctionPass *createARMJITCodeEmitterPass(
> +    ARMTargetMachine &TM, JITCodeEmitter &JCE)
> +{
> +  return new Emitter<JITCodeEmitter>(TM, JCE);
>  }
See above about coding style.

> +  template <class machineCodeEmitter>
> +  class VISIBILITY_HIDDEN Emitter : public MachineFunctionPass,
> +      public AlphaCodeEmitter
> +  {
Same here

> +
> +FunctionPass *llvm::createAlphaCodeEmitterPass( AlphaTargetMachine &TM,
>                                                 MachineCodeEmitter &MCE) {
> -  return new AlphaCodeEmitter(TM, MCE);
> +  return new Emitter<MachineCodeEmitter>(TM, MCE);
> +}
And here...

> +
> +FunctionPass *llvm::createAlphaJITCodeEmitterPass( AlphaTargetMachine &TM,
> +                                               JITCodeEmitter &JCE) {
> +  return new Emitter<JITCodeEmitter>(TM, JCE);
>  }
And here.

> +    /// getBinaryCodeForInstr - This function, generated by the
> +    /// CodeEmitterGenerator using TableGen, produces the binary encoding for
> +    /// machine instructions.
> +
> +    unsigned getBinaryCodeForInstr(const MachineInstr &MI);
Do not add blank lines after comment blocks.

<same comments applies for stuff later>

-- 
With best regards, Anton Korobeynikov.

Faculty of Mathematics & Mechanics, Saint Petersburg State University.