[llvm-commits] [llvm] r111850 - /llvm/trunk/lib/Target/ARM/ARMFastISel.cpp

[Jim Grosbach]

On Aug 23, 2010, at 3:56 PM, Eric Christopher wrote:

>> 
>> I was going to ask, "Is there a check somewhere to make sure we don't try to get here in Thumb or Thumb2 mode?" but you've already checked in exactly that. So moving on... 
>> 
> 
> :)
> 
>> getFrameIndexReference() doesn't check whether the reg/offset will be in range for an instruction, but rather just returns the appropriate base register and the offset from it for a frame index reference. The LDR instruction generated here may thus have an out-of-range offset in the case of large (> 4k) stack frames. At least for now, it may be reasonable to just punt for offsets that don't fit in the immediate.
>> 
> 
> Current in development patch :)
> 
>> Similarly, is there are check above this to make sure we're only dealing with GPR register class loads? Even in ARM mode, there's a number of others (see loadRegFromStackSlot() in ARMBaseInstrInfo.cpp for examples) that may need to be handled. Like the above, though, it's not unreasonable to just detect them and punt for now.
> 
> Not yet.  I think that's Now+1 in my plans.
> 

Coolness. Sounds good.

>> 
>> You're resolving frame index references directly. Is this happening after register allocation? I don't see how it can be, but could be wrong. If it's pre-regalloc, those offsets will be incorrect, as the code won't know how many callee-saved register slots are allocated for the function, nor how many spill slots. Perhaps you can use loadRegFromStackSlot() and storeRegToStackSlot() directly and the use the normal PEI code?
> 
> I've worried about this, but it seems to work for X86 and it would have the same problems of spill code so I'm not worrying about it overly at the moment.  I don't promise any of this will work at the moment past not segfaulting :)

I suspect x86 is doing things like "always push all callee saved regs whether we used them or not" and always referencing from the frame pointer. That way it can always have a fixed offset from EBP and not worry about how many spills there are. I.e., it's doing custom PEI that allows much stronger assumptions about frame layout much earlier in the compilation. ARM could do similar things if the full-force PEI is too much of a performance hit. In which case, you'd want to use a different "get me the reg+offset info" for frame index references, as the current one looks at the frame info populated by PEI to determine that stuff.

My naive thought is to give the "real" PEI a shot to get things up and running, and then revisit that if it ends up being too much of a compile-time hit.

-Jim