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.

Details

Summary

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.

Thanks;

arsenm added inline comments.Apr 23 2018, 11:43 AM
lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp
330

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

340–341

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.

348

Typo othereise

354

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

357

Same as with the load

433–436

Use dyn_cast instead of separate isa and cast

452

Same as the load case

test/CodeGen/AMDGPU/vector-alloca.ll
154

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
lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp
340–341

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.
lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp
330

Done in a separate patch under review.

354

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.