[PATCH] D124369: [InstCombine] C0 >>{ashr, exact} (X - C1) --> (C0 << C1) >>{ashr} X

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 27 07:06:28 PDT 2022


spatel added a comment.

In D124369#3475837 <https://reviews.llvm.org/D124369#3475837>, @nico-abram wrote:

> To be sure I understand correctly: You mean adding just the tests, and have the same kind of CHECK-NEXT FileCheck directives, but with the expected input IR i.e the same IR the tests contain, to verify the inputs are not being affected by unrelated optimizations?

That is correct - we want to have all of the tests in place before this patch with the current output tested. That way if this patch has to be reverted we will still have test coverage in place for the transform. It also makes it easier to see exactly what is changing with this patch (and what is intentionally not changing).

> I'm attaching baseline.patch F22911457: baseline.patch <https://reviews.llvm.org/F22911457> with that changeset. If needed I can open a new revision for the baseline test cases, or just update the diff of this one. I ran the tests locally on the baseline and they passed.
>
> I do not have commit access, so it would be great if you could push for me. Please use "Nicolas Abram Lujan abramlujan at gmail.com" if a name/email is needed.

I made several changes/additions to that set of tests and pushed that patch here:
fd9026131e6c <https://reviews.llvm.org/rGfd9026131e6c5de6f4e582d5c85c5b5588672dfb>

Please have a look, then update this patch by running update_test_checks.py on those tests and re-uploading.

We should not need to set `NewShiftOp->setHasNoSignedWrap(false);`. If you are seeing a test difference with that line in place, we need to investigate further.


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

https://reviews.llvm.org/D124369



More information about the llvm-commits mailing list