[llvm-commits] [llvm] r58405 - r58409

[Evan Cheng]

On Oct 30, 2008, at 10:30 AM, Jim Grosbach wrote:

> On Oct 29, 2008, at 4:54 PM, Evan Cheng wrote:
>>
>> void JITEmitter::emitConstantPool(MachineConstantPool *MCP) {
>> -  if (TheJIT->getJITInfo().hasCustomConstantPool()) {
>> -    DOUT << "JIT: Target has custom constant pool handling.
>> Omitting standard "
>> -            "constant pool\n";
>> -    return;
>> -  }
>
> If this is removed, the JIT will still emit the generic constant pool
> at the start of the function. The ARM backend doesn't use that pool
> (all the constants are put into the constant islands), so the generic
> pool is a waste of space and completely unreferenced. There needs to
> be a way for the target to tell the generic bits that the target is
> handling all of the constant pool entries.

You're right. I am not crazy about this solution but I don't see a  
better one right now. We can restore this part.

>
>
>> +      MCE.emitWordLE(0);
>> +    } else {
>> +      abort(); // FIXME: Is this right?
>> +      const ConstantInt *CI = dyn_cast<ConstantInt>(CV);
>> +      uint32_t Val = *(uint32_t*)CI->getValue().getRawData();
>> +      MCE.emitWordLE(Val);
>> +    }
>
>
> This breaks simple cases like the following:
> #include <stdio.h>
>
> int main(int argc, char *argv[])
> {
>   unsigned a;
>
>   a = 0xaaaaaaaa;
> }
>
> These were working before and hit this abort now.

Right. It's intended. I want a better solution for all the cases. I'll  
deal with this shortly.

>
>
>> --- llvm/trunk/include/llvm/CodeGen/MachineRelocation.h (original)
>> +++ llvm/trunk/include/llvm/CodeGen/MachineRelocation.h Wed Oct 29
>> 18:53:42 2008
>> @@ -63,10 +63,11 @@
>>    unsigned GOTIndex;      // Index in the GOT of this symbol/global
>>  } Target;
>>
>> -  unsigned TargetReloType : 6; // The target relocation ID.
>> -  AddressType AddrType    : 4; // The field of Target to use.
>> -  bool NeedStub           : 1; // True if this relocation requires
>> a stub.
>> +  unsigned TargetReloType : 6; // The target relocation ID
>> +  AddressType AddrType    : 4; // The field of Target to use
>> +  bool NeedStub           : 1; // True if this relocation requires
>> a stub
>>  bool GOTRelative        : 1; // Should this relocation be relative
>> to the GOT?
>> +  bool TargetResolve      : 1; // True if target should resolve the
>> address
>
> I thought we were avoiding adding entries to the MachineRelocation
> class?

We have bits available. This should not cause any bloat.

>
>
> I have mixed feeling about this. On the one hand, it makes sense to
> have relocation related bits (pun intended) be part of the relocation
> class. On the other hand, this implies that the target will be mixing
> and matching whether a constant pool entry will be resolved by the
> target or by the generic code on a per-constant basis, which is not
> true. The decision to put the constants elsewhere is made on an all- 
> or-
> nothing basis, and it seems that this should mirror that decision. On

That's true for ARM. But it may not always be true. The main reason  
for the change is the old scheme cannot deal with machine constantpool  
entries. Also, this is a clearer separation. The CONSTPOOL_ENTRY is an  
instruction, so the target code emitter should populate it.

>
> the gripping hand, I like that this allows for target-specific stuff
> that's not just the constant pool entries.
>
> Perhaps the implication is that the non-target specific JIT shouldn't
> be doing the constant pool layout, and should be delegating that, and
> the associated relocation address resolution, to the target always?
>
> As a sidenote, shouldn't these all have been a single commit rather
> than one per file? The changes are related and require one another for
> a successful build. Having related changes be checked in as an atomic
> commit is very useful when doing forensics later.

*Shrug*. Some parts of these check-ins are not tied to the others.

Evan

>
>
>
> -Jim
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits