[PATCH] D88995: Support vectors in CastInst::isBitOrNoopPointerCastable

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

In D88995#2323613 <https://reviews.llvm.org/D88995#2323613>, @reames wrote:

> Roman,
>
> I think you're bringing in a concern to this review which does not belong here.

I personally think it's a question of overall optimization pipeline sanity in light of recent discussions in related patches.
Sure, this isn't broken per-wording, but it is likely mis-directed, and it might be good to evaluate that before making things more entrenched.

In D88995#2323613 <https://reviews.llvm.org/D88995#2323613>, @reames wrote:

> Simple load forwarding is a transformation we implement in multiple locations (GVN, EarlyCSE, InstCombine).  This patch doesn't even change how we handle inttoptrs.  Existing code today will forward an integer load to a pointer load and insert an inttoptr.  All this patch does is be consistent about handling the same cases for vectors.

Yes.

In D88995#2323613 <https://reviews.llvm.org/D88995#2323613>, @reames wrote:

> I believe we should separate the concerns, and not commingle them.



In D88995#2323613 <https://reviews.llvm.org/D88995#2323613>, @reames wrote:

> To your actual question - which again, I believe is off topic for this review - we could consider extending the load transform to select the "better" of the two types, and potentially insert a cast for the former set of uses instead of the later.

No, that was not my question.
My question was:

In D88995#2318600 <https://reviews.llvm.org/D88995#2318600>, @lebedev.ri wrote:

> In the most original unoptimized IR, were there actually two different loads, one of a pointer and one of an integer?



In D88995#2323613 <https://reviews.llvm.org/D88995#2323613>, @reames wrote:

> If you want to avoid inttoptrs, we could canonicalize this case (consistently across the optimizer, not just here) to loading pointers and casting to ints.  I have to admit I don't fully understand the reasoning behind the desire to avoid inttoptr, so I'm not sure if this actually helps you or not.

Unfortunately, that's the caveat, the whole reason i'm asking this is because i've tried that already in D88842 <https://reviews.llvm.org/D88842>.



================
Comment at: llvm/test/Transforms/InstCombine/call.ll:238
 ; CHECK-LABEL: @test13(
-; CHECK: call void bitcast
+; CHECK: call void @test13a
   call void bitcast (void (<2 x i64>)* @test13a to void (<2 x i32*>)*)(<2 x i32*> %A)
----------------
Please can you autogenerate the checklines here and precommit?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88995/new/

https://reviews.llvm.org/D88995