[PATCH] Check address space when optimizing gep+cast

[Jingyue Wu]

ping

Matt, does it look good to you?

Jingyue


On Fri, Mar 28, 2014 at 3:53 PM, Jingyue Wu <jingyue at google.com> wrote:

> fixed a typo
>
>
> On Fri, Mar 28, 2014 at 3:46 PM, Jingyue Wu <jingyue at google.com> wrote:
>
>>
>>
>> On Fri, Mar 28, 2014 at 3:05 PM, Eli Bendersky <eliben at google.com> wrote:
>>
>>>
>>>
>>>
>>> On Fri, Mar 28, 2014 at 2:25 PM, Jingyue Wu <jingyue at google.com> wrote:
>>>
>>>> Hi Matt,
>>>>
>>>> That makes sense, and here's a more constructive patch.
>>>>
>>>>
>>> +          if (StrippedPtrTy->getAddressSpace() == GEP.getAddressSpace())
>>> +            return Res;
>>> +          // Insert Res, and create an addrspacecast.
>>> +          return new AddrSpaceCastInst(Builder->Insert(Res),
>>> GEP.getType());
>>>
>>> Can you provide a bit more details in the comment? The surrounding code
>>> has examples like:
>>>
>>> // -> GEP ... stuff
>>>
>>> Maybe something like this to clarify the sequence it turns to.
>>>
>>
>> Done
>>
>>
>>>
>>>
>>> + at array = internal addrspace(3) global [256 x float] zeroinitializer,
>>> align 4
>>> +
>>> +define float* @keep_necessary_addrspacecast(i64 %i) {
>>> +entry:
>>> +; CHECK-LABEL: @keep_necessary_addrspacecast
>>> +; CHECK: addrspacecast float addrspace(3)* %{{[0-9]+}} to float*
>>> +  %0 = getelementptr [256 x float]* addrspacecast ([256 x float]
>>> addrspace(3)* @array to [256 x float]*), i64 0, i64 %i
>>> +  ret float* %0
>>> +}
>>>
>>>
>>> Please create test cases that cover both paths in the patch (array /
>>> not-array).
>>>
>>
>> Done
>>
>>
>>>
>>> Eli
>>>
>>>
>>>>  Jingyue
>>>>
>>>>
>>>> On Fri, Mar 28, 2014 at 12:07 PM, Matt Arsenault <
>>>> Matthew.Arsenault at amd.com> wrote:
>>>>
>>>>>  On 03/28/2014 12:01 PM, Jingyue Wu wrote:
>>>>>
>>>>>  Fixing PR19270. This issue is blocking a waiting patch of mine that
>>>>> implements the optimization we discussed in
>>>>> http://lists.cs.uiuc.edu/pipermail/llvmdev/2014-March/071440.html.
>>>>>
>>>>>  visitGetElementPtr in InstructionCombining.cpp gets rid of
>>>>> unnecessary pointer casts in gep (cast X). However, this optimization may
>>>>> change the address space of the result pointer type, and cause type
>>>>> mismatch.
>>>>>
>>>>>  e.g.,
>>>>> getelementptr [256 x float]* addrspacecast ([256 x float]
>>>>> addrspace(3)* @array to [256 x float]*), i64 0, i64 %i
>>>>> returns a float*, but the optimized instruction
>>>>> getelementptr [256 x float] addrspace(3)* @array, i64 0, i64 %i
>>>>>  returns a float addrspace(3)*
>>>>>
>>>>> The attached patch disables this optimization when the address space
>>>>> of the source is different from that of the destination.
>>>>>
>>>>>  Jingyue
>>>>>
>>>>>
>>>>> If we're doing what I suggested and trying to do operations in the
>>>>> original address space, then instead of disabling this, why don't you make
>>>>> this instead just move the addrspacecast to after the GEP? Do the GEP in
>>>>> addrspace(3), and then addrspacecast that.
>>>>>
>>>>> -Matt
>>>>>
>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140331/e3e9d6bc/attachment.html>