[PATCH] D93229: [VectorCombine] allow peeking through GEPs when creating a vector load

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 16 05:37:22 PST 2020


spatel added a comment.

In D93229#2456676 <https://reviews.llvm.org/D93229#2456676>, @aqjune wrote:

> In D93229#2454698 <https://reviews.llvm.org/D93229#2454698>, @spatel wrote:
>
>> In D93229#2453538 <https://reviews.llvm.org/D93229#2453538>, @aqjune wrote:
>>
>>> Yes, we might want to know who's generating insertvalue undef and replace it with insertvalue poison.
>>> The shufflevector pattern works, but using poison as a placeholder will remove latent bugs.
>>> Maybe it is time to use poison?
>>
>> If we switch to initialize with poison elements, I think we would have to swap undef with poison in regression tests and pattern matching in instsimplify/instcombine at the same time. Otherwise, we will be testing/folding the wrong patterns?
>
> Yes, I think so. What do you think about having three patches
> (1) Poison placeholder patch (2) InstSimpify/InstCombine patch (3) The regression tests update patch.
> and landing those consecutively when all of those are accepted? Patch (1) and (2) should still pass `ninja check`. It will show which tests are related to what.
> I think I can prepare (2) and (3). The changes in regression tests (3) can be syntactically done with a script.
> I tried this and it worked: `sed -i.backup 's/insertelement <\(.*\)> undef,/insertelement <\1> poison,/g' (filename)`

We can use an alternate plan to make incremental progress, but it will cause redundancy while we transition:

1. Replicate all of the regression tests with the poison constant instead of undef. If we add some unique TODO text marker on all of those new tests, we can then grep to make sure everything that we expect to get updated is actually updated in later steps.
2. Update instcombine/simplify/vectorizer folds to match poison patterns (create a `m_UndefOrPoison()` pattern matcher?)
3. Run Alive2 through all of the updated regression tests to verify.
4. Update codegen to deal with poison constant?
5. Update folds from step 2 to create poison rather than undef (if that's what they were doing)?
6. Change IRBuilder or other instruction creators to create poison from the start.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93229/new/

https://reviews.llvm.org/D93229



More information about the llvm-commits mailing list