[llvm] de18fa9 - Revert "[InstSimplify] Bypass no-op `and`-mask, using known bits (PR49543)"

[Roman Lebedev via llvm-commits]

On Wed, Apr 21, 2021 at 7:54 PM Nikita Popov <nikita.ppv at gmail.com> wrote:
>
> On Wed, Apr 21, 2021 at 6:21 PM Roman Lebedev <lebedev.ri at gmail.com> wrote:
>>
>> 1. I don't recall getting a message "hey, this is causing xyz, any
>> thoughts, should revert?"
>> 2. I don't recall seeing a follow-up mail to the original commit that
>> it was reverted
>
>
> You replied to the revert email before I had a chance to message the original commit. From your reply to the revert I assumed you were already aware of the revert. Of course, now that you mention it, mailing the original commit may still be useful for others.

Note that i specifically said this wasn't the only instance.
Other examples:
https://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20210412/thread.html#903512
https://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20210315/thread.html#894082
https://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20210315/thread.html#893258
<...>

So these two points stand i think.

> As you are a very heavy user of post-commit review, please expect that reverts of your commits will be more common than if you go through pre-commit review.
I think this may be not spelled out enough. More common //why//?
Because that post-commit review approach potentially results in more
commits? Absolutely.

Please correct if i'm wrong, but the docs only mention that the
feedback post-commit is no less important than one pre-commit.
But i don't recall seeing anything that says pre-reviewed code/commits
is weaker/stronger for the purpose of post-commit review/reverts.

> If you had gone through review with this change, I would have left my concerns on the review. As you did not go through a review, I reverted to status quo, indicated my primary point of contention, and suggested that the patch go through review.

> In most cases I would simply leave comments on specific parts of the patch, with the expectation that they be addressed in due time. In this instance, I disagree with the patch in concept, so that wouldn't work.
Why wouldn't it work? I think this may be the main problem here.
And i disagree that llvm supports windows/provides clang-cl. Shall i
go ahead and revert their commits? Concretely, this comment suggests
that it wouldn't be addressable in a proper review, and it would be
blocked with no hope for forward progress. But is that really so?

> Possibly I misunderstood our review/revert policy, but I was under the impression that it's always possible to request that a post-commit review be converted into a pre-commit review, with the change being reverted in the meantime.

> Regards,
> Nikita
Roman

On Wed, Apr 21, 2021 at 7:54 PM Nikita Popov <nikita.ppv at gmail.com> wrote:
>
> On Wed, Apr 21, 2021 at 6:21 PM Roman Lebedev <lebedev.ri at gmail.com> wrote:
>>
>> 1. I don't recall getting a message "hey, this is causing xyz, any
>> thoughts, should revert?"
>> 2. I don't recall seeing a follow-up mail to the original commit that
>> it was reverted
>
>
> You replied to the revert email before I had a chance to message the original commit. From your reply to the revert I assumed you were already aware of the revert. Of course, now that you mention it, mailing the original commit may still be useful for others.
>
> As you are a very heavy user of post-commit review, please expect that reverts of your commits will be more common than if you go through pre-commit review. If you had gone through review with this change, I would have left my concerns on the review. As you did not go through a review, I reverted to status quo, indicated my primary point of contention, and suggested that the patch go through review.
>
> In most cases I would simply leave comments on specific parts of the patch, with the expectation that they be addressed in due time. In this instance, I disagree with the patch in concept, so that wouldn't work.
>
> Possibly I misunderstood our review/revert policy, but I was under the impression that it's always possible to request that a post-commit review be converted into a pre-commit review, with the change being reverted in the meantime.
>
> Regards,
> Nikita