This is an archive of the discontinued LLVM Phabricator instance.

[LoopVec] Support global addresses as argument to uniform mem ops
ClosedPublic

Authored by reames on Nov 24 2020, 12:34 PM.

Details

Summary

The initial step of the uniform-after-vectorization (lane-0 demanded only) analysis was very awkwardly written. It would revisit use list of each pointer operand of a widened load/store. As a result, it was in the worst case O(N^2) where N was the number of instructions in a loop, and had restricted operand Value types to reduce the size of use lists.

This patch replaces the original algorithm with one which is at most O(2N) in the number of instructions in the loop. (The key observation is that each use of a potentially interesting pointer is visited at most twice, once on first scan, once in the use list of *it's* operand. Only instructions within the loop have their uses scanned.)

Diff Detail

Event Timeline

reames created this revision.Nov 24 2020, 12:34 PM
reames requested review of this revision.Nov 24 2020, 12:34 PM
anna added a comment.Dec 2 2020, 1:21 PM

Nice catch! LGTM from me. One comment inline. Pls see if @fhahn has any comments as well?

llvm/test/Transforms/LoopVectorize/X86/cost-model-assert.ll
52

Any idea why these extra no-ops get added? That seems like an unfortunate side effect (especially given the algorithm works to reduce compile time).

fhahn accepted this revision.Dec 3 2020, 1:52 PM

LGTM, thanks! It looks like the patch needs re-formatting before submitting.

llvm/test/Transforms/LoopVectorize/X86/cost-model-assert.ll
52

IIUC this is related to the fact that we are now considering %1 = load i8, i8* undef, align 1, as uniform and probably chose a higher interleave count because of that?

If the pointer argument would be an instruction or argument, we should get the same result even without the patch. It might be worth changing the load to use a 'real' pointer.

llvm/test/Transforms/LoopVectorize/X86/uniform_mem_op.ll
600

With the patch, constant expressions should also be supported, right? Could you add a test case for that as well?

This revision is now accepted and ready to land.Dec 3 2020, 1:52 PM
This revision was landed with ongoing or failed builds.Dec 3 2020, 2:56 PM
This revision was automatically updated to reflect the committed changes.