Updated the commit message

Simplified expression handling code, and cleaned up other things

Rebased

ping !

@efriedma I have fixed the SCEV part, it mostly seems correct. Also added the testcase with large add constant. Please take a look once ! thanks !

added empty line at end of file in a test case

updated with clang-format

In D152278#4590846, @efriedma wrote:

I'm not sure why you're having so much trouble with the unequal shift math. The case of unequal shift amount is really just treating the shl as two shifts: one shift by an arbitrary amount, followed by one shift with shift amount equal to the ashr.

Update the SCEV code and add a new testcase

ping ping !!

Hey @efriedma Thanks for spotting the bug, I updated the SCEV for case when m > n, it seems to be correct now. Can you please review the patch, thanks !

Fixed issue with generated SCEV for ShlAmt > AshrAmt

Hey @nikic and @efriedma Sorry for the ping again, but I have addressed all the comments (added the testcases as well), can you please review once again, it's been stuck since a long time. Thanks for your time.

Updated LSR testcase and added new testcases for AShr SCEV model

Rebase

ping !! @nikic can you take a look at this, thanks !

In D156304#4545880, @nikic wrote:

LGTM, please add NFC to the patch title.

ping @fhahn , @sdesmalen can you please review this once ? Thanks !

ping !

In D155275#4500754, @ABataev wrote:

Hi, thanks for the patch, but it is not quite correct. I going to commit a fix in a few minutes with your test (just need to fix assertion check, actually)

Updated patch after running through clang-format

In D152278#4420657, @vedant-amd wrote:

updated code to shift right the AddConstant by ShlAmt and flip add, trunc exprs

updated code to shift right the AddConstant by ShlAmt and flip add, trunc exprs

removed restriction on shift amount and the corresponding test case

updated failing testcase in LSR

In D152278#4417345, @nikic wrote:

In D152278#4416394, @vedant-amd wrote:

In D152278#4416367, @xbolva00 wrote:

You have to update that file anyway. Pre-commit builds are clearly failing.

Right, is it fine if I mark the test case as XFAIL for now ?

No, you need to regenerate the test by running the update_test_checks.py script.

In D152278#4416367, @xbolva00 wrote:

You have to update that file anyway. Pre-commit builds are clearly failing.

Ping ! I didn't add the LSR failing testcase fix here, because I honestly don't know if it uncovered a bug, or it's a valid transformation in LSR.

In D152278#4411959, @xbolva00 wrote:

You should update checks in loopstrengthreduce::scaling-factor-incompat-type.ll

In D152278#4408118, @vedant-amd wrote:

In D152278#4407841, @nikic wrote:

I assume the motivation for this is the following InstCombine transform? https://llvm.godbolt.org/z/zd63TWKbW I don't think we can change that canonicalization, so undoing it in SCEV is fine.

Hey @nikic, I did refactor the change, please take a look, all the SCEV test cases do pass with it. It look correct functionally.

In D152278#4407841, @nikic wrote:

I assume the motivation for this is the following InstCombine transform? https://llvm.godbolt.org/z/zd63TWKbW I don't think we can change that canonicalization, so undoing it in SCEV is fine.

Refactored code to reuse ashr + shl handling code

In D152278#4407841, @nikic wrote:

I assume the motivation for this is the following InstCombine transform? https://llvm.godbolt.org/z/zd63TWKbW I don't think we can change that canonicalization, so undoing it in SCEV is fine.

updated using clang-format

Hey @efriedma , I have made the requested changes (and replied to the questions). Please let me know if anything more needs to be done. Thanks !

Added check to make sure AddConstant isn't nullptr and few other changes.

Updated unit test to remove ret undefs

In D152278#4399751, @vedant-amd wrote:

Hey @fhahn @nikic @mkazantsev can you please review this patch, I stumbled upon this while working on a bug in LSR, this patch does fail one of the unit test-case (scaling-factor-incompat-type.ll) in LSR. I believe that might be an effect of SCEV of this AShr pattern being computed, as LSR generally does do some optimizations based on the Shl + Add + AShr instructions.

Hey @fhahn @nikic @mkazantsev can you please review this patch, I stumbled upon this while working on a bug in LSR, this patch does fail one of the unit test-case (scaling-factor-incompat-type.ll) in LSR. I believe that might be an effect of SCEV of this AShr pattern being computed, as LSR generally does do some optimizations based on the Shl + Add + AShr instructions.

In D149130#4329307, @fhahn wrote:

LGTM, thanks!

In D149130#4327201, @fhahn wrote:

It looks like clang-format failed in the pre-commit tests. Could you please make sure the diff is properly formatted by running clang-format-diff?

Run clang-format-diff on the code

Hey @fhahn @jdoerfert Sorry for the delay, but I have updated the patch to reflect your requested changes. Please let me know if it looks okay ?

Removed pass wide unreachable block elimination added a depth_first approach to iterate over basic blocks

In D149130#4296535, @fhahn wrote:

If the block is unreachable, how come it is even considered for outlining?

In D149130#4296268, @jdoerfert wrote:

Generally, I don't believe we should run elimination in all passes.
Instead, use the dominance tree to walk the blocks of a function, or walk it by manually following the control edges.

Hey @fhahn I don't have rights to commit this myself, can you commit it on my behalf ? (or give me commit access? edit: I have mailed chris, as per the docs) Thanks !

In D149130#4296535, @fhahn wrote:

If the block is unreachable, how come it is even considered for outlining?

Removed whitespaces

run clang-format

minor fix to commit message