[PATCH] D31910: [LV] Fix logic to extract correct element in first order recurrences

[Matthew Simpson via Phabricator via llvm-commits]

mssimpso added a comment.

Hi Anna,

I think we can probably solve this without putting a limitation on the trip count. But I'm wondering if it would be better for now to just avoid vectorizing this case, at least until we can work out a more robust code generation strategy for the first-order recurrences. Why not change RecurrenceDescriptor::isFirstOrderRecurrence to ensure the phi has no users outside the loop? I think that's the underlying issue, right? We're currently assuming that only the phi update can be used externally. But in your test case, it's the phi that's used externally. Just to make sure I understand things correctly, after vectorization we end up with something like the following for the external use:

  %r.lcssa = phi i32 [ %scalar.recur, %for.body ], [ %vector.recur.extract, %middle.block ]

So if the scalar loop is executed, %r.lcssa is the phi, but if the trip count is evenly divided and the scalar loop is not executed, %r.lcssa is the phi's last update. And we currently assume we'll only encounter something like:

  %r.lcssa = phi i32 [ %update, %for.body ], [ %vector.recur.extract, %middle.block ]

where it's the update that's used externally. I think the solution would eventually be something like:

  middle.block:
    %vector.recur.extract = extractelement <4 x i32> %vector.update, i32 3
    %new.extract = extractelement <4 x i32> %vector.recur, i32 3
  scalar.ph:
    %scalar.recur.init = phi i32 [ %vector.recur.extract, %middle.block ], ...
  for.end:
    %r.lcssa = phi i32 [ %scalar.recur, %for.body ], [ %new.extract, %middle.block ]

where we add an additional extract for the last element of the vector phi, if the phi is used externally. What do you think? We could disable the external use for now to fix the correctness issue and then re-enable it once we get the code generation right.


https://reviews.llvm.org/D31910