[llvm] r303133 - [InstCombine] add tests for PR32791; NFC

[Davide Italiano via llvm-commits]

On Mon, May 15, 2017 at 5:48 PM, Philip Reames
<listmail at philipreames.com> wrote:
> On 05/15/2017 05:37 PM, Davide Italiano via llvm-commits wrote:
>>
>> On Mon, May 15, 2017 at 5:32 PM, Sanjay Patel <spatel at rotateright.com>
>> wrote:
>>>
>>> This is just documenting the current status. If we choose not to
>>> transform
>>> these in InstCombine, then we should have a record of that limitation
>>> that
>>> we acknowledge is a good thing. This way someone won't come along in the
>>> future and wonder if the transform doesn't exist intentionally or it's
>>> just
>>> an oversight. I can add some test comments once we have a conclusion in
>>> D33172.
>>>
>> Putting these tests in `InstCombine` if we decide to do the
>> transformation somewhere else is just ... odd.
>> I don't think we should document all the transformations we don't do
>> otherwise we will end up with a bunch of useless and/or redundant
>> tests in tree. Please revert this change until we have reached a
>> consensus.
>
> Davide, to me, Sanjay's actions here seem reasonable.  Given his prior work,
> I'm more than wiling to believe that he'll update/remove/move the tests as
> needed based on the conclusion of the review.  I'd vote for leaving these in
> tree given that it makes the process of review easier as the changes being
> made become easy to see.
>>

I'm fairly sure Sanjay is a diligent developer but this is not my point.
The standard practice in LLVM is that of not committing code when
there are pending design discussions. At times, after code has been
committed if people raise objections we generally try to revert sooner
rather than later, then start a discussion and iterate from there.
There's an ongoing discussion on whether this transformation should
live and I've raised objections on having it in InstCombine, see
https://reviews.llvm.org/D33172
So, committing these tests anyway seems a little weird to me.

--
Davide

"There are no solved problems; there are only problems that are more
or less solved" -- Henri Poincare