This is an archive of the discontinued LLVM Phabricator instance.

[LV] Use Legal::isUniform to detect uniform pointers.
ClosedPublic

Authored by fhahn on May 19 2023, 1:04 PM.

Details

Summary

Update collectLoopUniforms to identify uniform pointers using
Legal::isUniform. This is more powerful and brings pointer
classification here in sync with setCostBasedWideningDecision
which uses isUniformMemOp. The existing mis-match in reasoning
can causes crashes due to D134460, which is fixed by this patch.

Fixes https://github.com/llvm/llvm-project/issues/60831.

Diff Detail

Event Timeline

fhahn created this revision.May 19 2023, 1:04 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 19 2023, 1:04 PM
fhahn requested review of this revision.May 19 2023, 1:04 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 19 2023, 1:04 PM
Ayal added a comment.May 23 2023, 11:33 AM

Good catch!

Thinking if this may call for an alternative fix within isVectorizedMemAccessUse(), which admittedly does not affect AArch64/sve-inv-store.ll(?)

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
4703–4704

How about the above alternative change? Seems more inline with isVectorizedMemAccessUse's documentation.
While we're here, an early exit seems simpler:

if (isa<StoreInst>(I) && I->getOperand(0) == Ptr)
  return false;
4753

This assert is currently redundant, and needs to be dropped (along with {}) in the alternative fix above.

llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
173–178

note: store to invariant address: insert-element - extract-element redundancy elimination

llvm/test/Transforms/LoopVectorize/skeleton-lcssa-crash.ll
54–55

note: store to invariant address - store-after-store redundancy elimination.

fhahn updated this revision to Diff 526573.May 30 2023, 4:15 AM

Updated to use alternative fix, thanks!

fhahn marked 2 inline comments as done.May 30 2023, 4:16 AM
fhahn added inline comments.
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
4703–4704

Thanks, updated to use the alternative fix.

Early exit added in b75086210774

4753

Updated, thanks!

Ayal accepted this revision.May 30 2023, 4:27 AM

This looks good to me, thanks!

This revision is now accepted and ready to land.May 30 2023, 4:27 AM
fhahn updated this revision to Diff 526622.May 30 2023, 7:56 AM
fhahn marked 2 inline comments as done.

Thanks, rebase before landing.

This revision was landed with ongoing or failed builds.May 30 2023, 8:46 AM
This revision was automatically updated to reflect the committed changes.