[llvm-commits] [Review Request] lib/Target/X86/X86FastISel.cpp: [PR6275] Fix for Win32's dllimport function.

[Chris Lattner]

On Feb 17, 2011, at 11:46 PM, NAKAMURA Takumi wrote:

> Function must not be GlobalVariable.
> 
> FIXME: Are latter conditions needed to check?
> ---
> lib/Target/X86/X86FastISel.cpp |    5 +++++
> 1 files changed, 5 insertions(+), 0 deletions(-)
> ---
> 
> I am dubious in one point;
> 
>    // Can't handle TLS or DLLImport.
>    // FIXME: Are they needed?
>    if (const GlobalVariable *GVar = dyn_cast<GlobalVariable>(GV))
>      if (GVar->isThreadLocal() || GVar->hasDLLImportLinkage())
>        return false;
> 
> Let me know if this clause could be eliminated, thank you.

I think that they can. http://llvm.org/docs/LangRef.html#linkage_dllimport says "or variable". 

The patch looks good to me.  Please remove the fixme and the "|| GVar->hasDLLImportLinkage()", since the new check subsumes it.

-Chris