[llvm-commits] [llvm] r92934 - /llvm/trunk/lib/Target/X86/X86FastISel.cpp

[Eric Christopher]

On Jan 7, 2010, at 4:59 PM, Dan Gohman wrote:

> 
> On Jan 7, 2010, at 2:00 PM, Eric Christopher wrote:
> 
>> 
>> On Jan 7, 2010, at 1:10 PM, Dan Gohman wrote:
>> 
>>> 
>>> On Jan 7, 2010, at 11:45 AM, Eric Christopher wrote:
>>>> 
>>>> +  // If we've gotten here we need to make sure we don't have a constant
>>>> +  // that needs a relocation, because then we shouldn't put it into the
>>>> +  // constant pool.
>>>> +  if (C->getRelocationInfo() != Constant::NoRelocation)
>>>> +    return 0;
>>> 
>>> Would Constant::LocalRelocation be ok here?
>> 
>> It could be, but I'm not sure what the advantage would be to allowing anything with a relocation into the constant pool?
> 
> It lets fast-isel keep running. If it fails, SelectionDAG will take
> over for the rest of the block. At -O0, code quality is not a top concern.

Largely this is an optimization after I fixed which section we're going to place one of these in - it could really go away for fast-iself if that's as much of an issue. I.e. it's not needed for correctness here.

Thoughts?

-eric