[PATCH] Check address space when optimizing gep+cast

[Eli Bendersky]

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.


+ 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).

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/20140328/4b8fb8b6/attachment.html>