Page MenuHomePhabricator

AMDGPU/SI: Handle BitCast of GEP in promoting alloca to vector
Needs ReviewPublic

Authored by cfang on Apr 3 2018, 1:50 PM.



This patch handles the case that the pointer is a BITCAST of GEP to change the data type. Before this patch, the check stops
at the BITCAST (pointer) and the load/store are not actually handled at all.

We also remove an invalid LIT test because we haven't handled the case yet, and check always pass with or without the promotion because
the index is an immediate and the load/store could be easily optimized away in other optimizations.

Diff Detail

Event Timeline

cfang created this revision.Apr 3 2018, 1:50 PM

Add reviewers and ping.


arsenm added inline comments.Apr 23 2018, 11:43 AM

I think there's another bug here (which should be fixed in a separate patch). This probably also needs to check if it's atomic


This shouldn't need to special case GEP users. You really want something like stripPointerCasts, but not one that looks through addrspacecast.

Could use a test with 0 index GEPs which should show the same problem.


Typo othereise


This looks broken. addrspacecast should forbid the vector promotion. It should only be allowed for the LDS promotion. Separate patch?


Same as with the load


Use dyn_cast instead of separate isa and cast


Same as the load case


needs some more tests with multiple uses of the bitcast source, with accesses through different types as well as non-bitcast users

cfang added inline comments.Apr 26 2018, 2:08 PM

I could not understand this comment.

Here we are handling the case of load(bitcast(gep))!
We are checking the bitcase and collect the load. We will transfer to vector load later, which depends on the gep for the index.
So we have to have the gep, and collect the load/store.

cfang marked 8 inline comments as done.May 9 2018, 2:50 PM
cfang added inline comments.

Done in a separate patch under review.


Done in a separate patch under review.

cfang updated this revision to Diff 146011.May 9 2018, 2:52 PM
cfang marked 2 inline comments as done.

Update based on Matt's comments.