[llvm-commits] [llvm] r127809 - in /llvm/trunk: include/llvm/Target/TargetLowering.h lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp lib/Target/X86/X86ISelLowering.cpp lib/Target/X86/X86ISelLowering.h

[Ken Dyck]

On Thu, Mar 17, 2011 at 10:53 AM, Cameron Zwarich <zwarich at apple.com> wrote:
> --- llvm/trunk/include/llvm/Target/TargetLowering.h (original)
> +++ llvm/trunk/include/llvm/Target/TargetLowering.h Thu Mar 17 09:53:37 2011
> @@ -1293,9 +1293,10 @@
>   /// but this is not true all the time, e.g. i1 on x86-64. It is also not
>   /// necessary for non-C calling conventions. The frontend should handle this
>   /// and include all of the necessary information.
> -  virtual MVT
> -  getTypeForExtArgOrReturn(EVT VT, ISD::NodeType ExtendKind) const {
> -    return MVT::i32;
> +  virtual EVT getTypeForExtArgOrReturn(LLVMContext &Context, EVT VT,
> +                                       ISD::NodeType ExtendKind) const {
> +    EVT MinVT = getRegisterType(Context, MVT::i32);
> +    return VT.bitsLT(MinVT) ? MinVT : VT;
>   }

Is this really an improvement? From where I sit, it seems to tightly
couple the method to its call site, reducing its potential to be
called from elsewhere[1]. And it requires every implementation to
duplicate the size-checking logic, which multiplies the number of
places that need to be updated should there ever be a need to change
it.

-Ken

[1] For instance,
http://lists.cs.uiuc.edu/pipermail/llvmdev/2011-March/038815.html