[PATCH] D86243: [InstCombine] canonicalize 'not' ops before logical shifts

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D86243#2227038 <https://reviews.llvm.org/D86243#2227038>, @xbolva00 wrote:

> Some SCEV/pipeline tests?
>
> Maybe from https://reviews.llvm.org/D85969#change-8DkHYel6Y1JK ?

I think if we are going to add tests to show that instcombine and SCEV are cooperating as expected, that would go in PhaseOrdering. But we need a larger example than what we have in D85969 <https://reviews.llvm.org/D85969> to show that the IR for some loop is better with this change?

In D86243#2228399 <https://reviews.llvm.org/D86243#2228399>, @bjope wrote:

> In D86243#2227038 <https://reviews.llvm.org/D86243#2227038>, @xbolva00 wrote:
>
>> Some SCEV/pipeline tests?
>>
>> Maybe from https://reviews.llvm.org/D85969#change-8DkHYel6Y1JK ?
>
> I figure we want to move forward with D85969 <https://reviews.llvm.org/D85969> and land that as well. Right?
> I mean, can't ScalarEvolution analysis be used by some pass before instcombine in the pass pipeline. Then I assume it is nice if SCEV treats the two different forms of the equivalent expressions the same way.

I don't know the answer. I agree that making SCEV handle more patterns makes it less brittle, but how much redundancy is needed?


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

https://reviews.llvm.org/D86243