Page MenuHomePhabricator

[GVN] Make handling of zeros more consistent
Needs RevisionPublic

Authored by vtjnash on Nov 3 2020, 11:49 AM.

Details

Reviewers
huihuiz
reames
Summary

This is an attempt to be slightly more consistent about what checks each function does, by deferring most of them to using canCoerceMustAliasedTypeToLoad. The expected benefit of this approach is demonstrated by the new optimizations exposed represented by the additional test examples.

This tries to handle zeroinitializer consistently, by preferring using
helper functions instead of re-implementing some of the same checks.
This lets us forward zeros successfully in a few more cases.

And since casting a zero should always be possible (without needing a
bitcast), make sure the canCoerce check reflects that, and that all
relevant users are handling that.

(After adding tests to show the goals, I took pieces of https://reviews.llvm.org/D59661 and https://reviews.llvm.org/D69910 to get those improvements, and so then closed those)

Diff Detail

Event Timeline

vtjnash created this revision.Nov 3 2020, 11:49 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 3 2020, 11:49 AM
vtjnash requested review of this revision.Nov 3 2020, 11:49 AM
vtjnash edited the summary of this revision. (Show Details)Nov 3 2020, 12:39 PM
vtjnash added reviewers: huihuiz, reames.
vtjnash updated this revision to Diff 392847.Dec 8 2021, 11:30 AM

rebase onto LLVM main, allowing it to handle null values everywhere, and not just for non-integral pointers

reames requested changes to this revision.Dec 10 2021, 9:10 AM

This patch appears to be doing at least two things:

  1. Consistent handling on zero constants for bitcastable types.
  2. Changing scalable vector handling.

I strongly recommend you split this into two patches. My reasoning is that while I can probably review 1 for you, I do not have the context to usefully review 2. I suspect other reviewers will have the same problem, possibly in the other direction.

llvm/lib/Transforms/Utils/VNCoercion.cpp
12

Please add this rename/invert separately. No review needed.

This revision now requires changes to proceed.Dec 10 2021, 9:10 AM