[PATCH] [InstCombine] Canonicalize addrspacecast between different element types

[Jingyue Wu]

Hi Phillip,

Thanks for your comments! They do make a lot of sense.

I wasn't here when the addrspacecast instruction was added. Maybe Matt can
provide more context on why addrspacecast is a superset of bitcast.
However, I don't see too much value of including the semantics of bitcast
besides making bitcode more compact.

With that, I would be more than happy to work on having clang emitting both
of them. But even with changes in clang, I think instcombine would still
need this canonicalization to handle IR not directly generated by clang.

Best,
Jingyue


On Wed, Jun 4, 2014 at 10:59 AM, Philip Reames <listmail at philipreames.com>
wrote:

> I'm reading this commit out of context, so this question might not make a
> lot of sense.  If that's the case, please ignore.
>
> If we are canonicalizing addrspacecasts to only convert between address
> spaces and not types, is there any value in having addrspacecast be a
> superset of bitcast to begin with?  Why not have the frontend emit both the
> addrspacecast and the bitcast itself?
>
> Philip
>
>
>
> On 06/03/2014 06:48 PM, Matt Arsenault wrote:
>
>> LGTM
>>
>> ================
>> Comment at: lib/Transforms/InstCombine/InstCombineCasts.cpp:1925-1926
>> @@ +1924,4 @@
>> +
>> +    // Putting bitcast before addrspacecast helps
>> NVPTXFavorNonGenericAddrSpaces
>> +    // to fold the addrspacecast into load/store.
>> +    Value *NewBitCast = Builder->CreateBitCast(Src, MidTy);
>> ----------------
>> I would remove  the comment about a specific target pass
>>
>> http://reviews.llvm.org/D4013
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140605/9354502b/attachment.html>