Support for HiPE-compatible code emission

Nadav Rotem nrotem at apple.com
Thu Feb 14 09:16:13 PST 2013


Yiannis, 

Thanks for following Eli's comments. 

+
+  if (Is64Bit) {
+    if (CallingConvention == CallingConv::HiPE)
+      return Primary ? X86::R14 : X86::R13;
+    else
+      return Primary ? X86::R11 : X86::R12;
+  }
+
+  if (CallingConvention == CallingConv::HiPE)
+    return Primary ? X86::EBX : X86::EDI;
+

Please put all of the "HiPE" logic in one place. Most people don't know what HiPE is, and would prefer to skip it when they read the code. Write something like:

// Erlang stuff. 
if (CallingConvention == CallingConv::HiPE) {
 ...
} 

 if (Is64Bit)  
   return Primary ? X86::R11 : X86::R12;
 ...

+/// GetScratchRegister - Get a temp register for performing work in the
+/// segmented stack and/or HiPE stack prologue. Depending on platform and the
+/// properties of the function either one or two registers will be needed. Set
+/// primary to true for the first register, false for the second.

Please remove " and/or HiPE" from the comment. This information does not help most people understand what this function does.  

In the test case "X86/hipe-prologue.ll"  Please add comment that describes what HIPE is. 

Thanks,
Nadav 


On Feb 14, 2013, at 4:43 AM, Yiannis Tsiouris <gtsiour at softlab.ntua.gr> wrote:

> On 02/13/2013 09:32 PM, Eli Bendersky wrote:
>>>> Tests indicate that you care about the x32 ABI, where sometimes you want
>>>> to
>>>> know the data model and not the platform bitness. This applies to stack
>>>> slot
>>>> size and to registers used for addresses. How does this affect your code?
>>> I'm not sure I get your question here. Let me clarify what I actually care
>>> about as this misunderstanding might be a result of my abuse of the LLVM
>>> API:
>>>> +  const unsigned Guaranteed = HipeLeafWords * SlotSize;
>>>> +  const unsigned CallerStkArity =
>>>> +    std::max<int>(0, MF.getFunction()->arg_size() - CCRegisteredArgs);
>>>> +  unsigned MaxStack =
>>>> +    MFI->getStackSize() + CallerStkArity * SlotSize + SlotSize;
>>> I 'd like to compute some metrics (such as the above) based on the number of
>>> *bytes* that are occupied on the stack for some values (e.g., the number of
>>> bytes that are used on the stack for the arguments of a function). One way
>>> to do that (that seemed reasonable to me :-)) was to use the pointer-size
>>> (or word-size or slot-size). So, what I only care about is the platform
>>> bitness. Is there a better way to write that? :-)
>> Intel 64-bit CPUs can have a x32 mode in which pointers are 4 bytes
>> long. Therefore, when you want the stack slot size, there's another
>> getter for that now and you should not rely on the identity "stack
>> size == pointer size". See some of the others tests that trigger the
>> x32 ABI in upstream trunk.
>>   
> Oh, now I see what you mean! :-) I think what I want is 'TargetRegisterInfo::getSlotSize()' then. I'm re-attaching the patch with that fixed.
> 
>>> A couple of small fixes here, too; they shouldn't be a problem.
>>> 
>>> Is this OK to commit?
>> Thanks for the fixes. I'm not the authority for final approvals in
>> this region though. Nadav or someone else should give you the green
>> light if they're OK with the patch.
>>   
> If there is no other objection, can someone please commit it for me?
> 
> Thanks Eli! ;-)
> 
> -- 
> Yiannis Tsiouris
> Ph.D. student,
> Software Engineering Laboratory,
> National Technical University of Athens
> WWW: http://www.softlab.ntua.gr/~gtsiour
> 
> <hipe-prologue-v3.patch>




More information about the llvm-commits mailing list