Change indirect-globals to use a dedicated allocIndirectGV

This lets us remove start/finishGVStub and the BufferState helper class from the MachineCodeEmitter interface.  It has the side-effect of not setting the indirect global writable and then executable on ARM, but that shouldn't be necessary.

[Jeffrey Yasskin, 2009-11-24 19:28:10]

Hi Evan, could you take a look?
http://codereview.appspot.com/download/issue157145_7.diff
I think this is an ok change for arm (and it passes tests on nlewycky's box)
because even though the indirect global is generated as part of a call stub,
it's not actually executed.

[Jeffrey Yasskin (google), 2009-12-07 21:34:36]

Hi Evan, any word on this?

On Tue, Nov 24, 2009 at 11:28 AM,  <jyasskin@gmail.com> wrote:
> Reviewers: evan.cheng_apple.com,
>
> Message:
> Hi Evan, could you take a look?
> http://codereview.appspot.com/download/issue157145_7.diff
> I think this is an ok change for arm (and it passes tests on nlewycky's
> box) because even though the indirect global is generated as part of a
> call stub, it's not actually executed.
>
> Description:
> This lets us remove start/finishGVStub and the BufferState helper class
> from the MachineCodeEmitter interface.  It has the side-effect of not
> setting the indirect global writable and then executable on ARM, but
> that shouldn't be necessary.
>
> Please review this at http://codereview.appspot.com/157145
>
> Affected files:
>   M     include/llvm/CodeGen/JITCodeEmitter.h
>   M     include/llvm/CodeGen/MachineCodeEmitter.h
>   M     lib/ExecutionEngine/JIT/JITEmitter.cpp
>   M     lib/Target/ARM/ARMJITInfo.cpp
>   M     lib/Target/Alpha/AlphaJITInfo.cpp
>   M     lib/Target/PowerPC/PPCJITInfo.cpp
>   M     lib/Target/X86/X86JITInfo.cpp
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits@cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>

[evan.cheng_apple.com, 2009-12-07 22:23:54]

Sorry, completely missed it.

+  // Size == alignment on ARM.
+  void *PtrAddr = JCE.allocIndirectGV(
+      GV, Buffer, sizeof(Buffer), sizeof(Buffer));
The comment is somewhat misleading. The alignment and size of the buffer happen
to be both 4 in this case. Your code makes it seem like they have to be the
same.

Apart from that, it seems fine. Eric, could you test it on x86 and x86_64
Darwin?

Thanks,

Evan

On Dec 7, 2009, at 1:34 PM, Jeffrey Yasskin wrote:

> Hi Evan, any word on this?
> 
> On Tue, Nov 24, 2009 at 11:28 AM,  <jyasskin@gmail.com> wrote:
>> Reviewers: evan.cheng_apple.com,
>> 
>> Message:
>> Hi Evan, could you take a look?
>> http://codereview.appspot.com/download/issue157145_7.diff
>> I think this is an ok change for arm (and it passes tests on nlewycky's
>> box) because even though the indirect global is generated as part of a
>> call stub, it's not actually executed.
>> 
>> Description:
>> This lets us remove start/finishGVStub and the BufferState helper class
>> from the MachineCodeEmitter interface.  It has the side-effect of not
>> setting the indirect global writable and then executable on ARM, but
>> that shouldn't be necessary.
>> 
>> Please review this at http://codereview.appspot.com/157145
>> 
>> Affected files:
>>   M     include/llvm/CodeGen/JITCodeEmitter.h
>>   M     include/llvm/CodeGen/MachineCodeEmitter.h
>>   M     lib/ExecutionEngine/JIT/JITEmitter.cpp
>>   M     lib/Target/ARM/ARMJITInfo.cpp
>>   M     lib/Target/Alpha/AlphaJITInfo.cpp
>>   M     lib/Target/PowerPC/PPCJITInfo.cpp
>>   M     lib/Target/X86/X86JITInfo.cpp
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits@cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> 

[Jeffrey Yasskin (google), 2009-12-07 22:59:45]

On Mon, Dec 7, 2009 at 2:23 PM, Evan Cheng <evan.cheng@apple.com> wrote:
> Sorry, completely missed it.
>
> +  // Size == alignment on ARM.
> +  void *PtrAddr = JCE.allocIndirectGV(
> +      GV, Buffer, sizeof(Buffer), sizeof(Buffer));
>
> The comment is somewhat misleading. The alignment and size of the buffer
> happen to be both 4 in this case. Your code makes it seem like they have to
> be the same.

Good point. I've removed that comment and specified the alignment
explicitly on both ARM and X86.
http://codereview.appspot.com/download/issue157145_2003.diff

> Apart from that, it seems fine. Eric, could you test it on x86 and x86_64
> Darwin?

I'll wait for Eric's ok to check this in.

> Thanks,
> Evan
> On Dec 7, 2009, at 1:34 PM, Jeffrey Yasskin wrote:
>
> Hi Evan, any word on this?
>
> On Tue, Nov 24, 2009 at 11:28 AM,  <jyasskin@gmail.com> wrote:
>
> Reviewers: evan.cheng_apple.com,
>
> Message:
>
> Hi Evan, could you take a look?
>
> http://codereview.appspot.com/download/issue157145_7.diff
>
> I think this is an ok change for arm (and it passes tests on nlewycky's
>
> box) because even though the indirect global is generated as part of a
>
> call stub, it's not actually executed.
>
> Description:
>
> This lets us remove start/finishGVStub and the BufferState helper class
>
> from the MachineCodeEmitter interface.  It has the side-effect of not
>
> setting the indirect global writable and then executable on ARM, but
>
> that shouldn't be necessary.
>
> Please review this at http://codereview.appspot.com/157145
>
> Affected files:
>
>   M     include/llvm/CodeGen/JITCodeEmitter.h
>
>   M     include/llvm/CodeGen/MachineCodeEmitter.h
>
>   M     lib/ExecutionEngine/JIT/JITEmitter.cpp
>
>   M     lib/Target/ARM/ARMJITInfo.cpp
>
>   M     lib/Target/Alpha/AlphaJITInfo.cpp
>
>   M     lib/Target/PowerPC/PPCJITInfo.cpp
>
>   M     lib/Target/X86/X86JITInfo.cpp
>
>
> _______________________________________________
>
> llvm-commits mailing list
>
> llvm-commits@cs.uiuc.edu
>
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>

[echristo_apple.com, 2009-12-08 01:29:27]

> 
>> Apart from that, it seems fine. Eric, could you test it on x86 and x86_64
>> Darwin?
> 
> I'll wait for Eric's ok to check this in.

Testing now.

-eric

[echristo_apple.com, 2009-12-08 02:22:57]

On Dec 7, 2009, at 5:29 PM, Eric Christopher wrote:

>> 
>>> Apart from that, it seems fine. Eric, could you test it on x86 and x86_64
>>> Darwin?
>> 
>> I'll wait for Eric's ok to check this in.
> 
> Testing now.

The resultant llvm-gcc failed to bootstrap on 32-bit leopard (Died with an
UNREACHABLE).

I haven't debugged it more and it could be transient on ToT.

-eric