[llvm-commits] [llvm] r48130 - in /llvm/trunk: include/llvm/CodeGen/SelectionDAGNodes.h lib/CodeGen/LowerSubregs.cpp lib/CodeGen/SelectionDAG/ScheduleDAG.cpp lib/Target/Target.td lib/Target/X86/X86ISelDAGToDAG.cpp lib/Target/X86/X86Instr64bit.td lib/Target/X86/X86InstrInfo.cpp lib/Target/X86/X86InstrInfo.h lib/Target/X86/X86InstrInfo.td lib/Target/X86/X86RegisterInfo.h lib/Target/X86/X86RegisterInfo.td

[Chris Lattner]

On Mar 10, 2008, at 10:51 AM, Evan Cheng wrote:

>>
>> +namespace X86 {
>> +  /// SubregIndex - The index of various sized subregister classes.
>> Note that
>> +  /// these indices must be kept in sync with the class indices in
>> the
>> +  /// X86RegisterInfo.td file.
>> +  enum SubregIndex {
>> +    SUBREG_8BIT = 1, SUBREG_16BIT = 2, SUBREG_32BIT = 3
>> +  };
>> +}
>> +
>
> Nice. Using enum is an improvement to magic constants. But how about
> making these target independent? However, that means adding a bunch of
> them which are not currently used: SUBREG_SECOND_8BIT, etc. Instead,
> perhaps you can add a helper function to map to the magic indicies,
> e.g. SubRegIndex(MVT::ValueType SuperRegVT, MVT::ValueType SubRegVt,
> unsigned WhichChunk)? What do you think?

I think it is better to keep these as target-specific values.  Even if  
we had a few target-independent ones, we'd have to allow targets to  
define their own, meaning the end solution is more complex than what  
we have now.

-Chris