In D107822#2956860, @lebedev.ri wrote:

The code here looks very weird.
Please can you at least split this up into a preparatory NFC changes, the deref-based fold, and the domination-based fold?
Also, please precommit the tests, even if just locally for you, so that the patches show how the check lines change.

Updated diff

ping

updated.

updated

Modified code and Added new tests

Updating the same diff to commit this patch.

In D106872#2922157, @lebedev.ri wrote:

In D106872#2922101, @Krishnakariya wrote:

I committed this patch by mistake. I am extremely sorry about this.
I've reverted the commit.

FWIW reading through the disscussion i would think this has been basically accepted, if not stamped yet for formal reasons of wanting the tests to be precommitted.

I committed this patch by mistake. I am extremely sorry about this.
I've reverted the commit.

Simplified code.

Updated diff after pre-committing tests.

Added new tests

Yes, it handles this case. I have added a new test for the same.

Added a test for different input pointer types (func_pointer_different_types).

In D106872#2907220, @lebedev.ri wrote:

It would be good to mention in the description *why* this is being done, why this is okay (alive2 link would suffice)

Sure, I have updated the description.

Yes, we may remove FMF checks as below. I will make a new patch for this.
FSub logic: Remove check for nnan flag presence in fsub instruction. Example: https://alive2.llvm.org/ce/z/tYeZub (fsub).
FNeg logic: Remove check for the presence of nnan and nsz flag in fneg instruction. Example: https://alive2.llvm.org/ce/z/TbuLnk (fneg), https://alive2.llvm.org/ce/z/yBDCZ_ (fsub).

Could you please commit this patch for me? I do not have commit access yet.

Hi @spatel,
I am sorry for my delayed responses to this patch. I wanted to ask for some clarification on your previous comments.
I have created few new tests similar to the original ones, with the only change that the FMF flag present in the fsub/fneg is now present on the select instruction. Original tests are left untouched.
Is this what you were expecting in your last comment?

Simplified code.

simplified code for function "foldICmpWithCastOp".

Ping @spatel, a gentle reminder. Could you please review the updates on this patch?

Created a new function (simplifyIntToPtrRoundTripCast) which simplifies the inttoptr/ptrtoint cast.

In D105771#2871729, @nikic wrote:

@Krishnakariya Could you please share Your Name <your@email.com> to use for the commit?

In D105771#2871413, @nikic wrote:

LGTM

Thanks.
I do not have commit access. Please commit this patch for me.

Modified test3 and updated the names of unit tests.

Added an command-line option disable-i2p-p2i-opt to opt to disable inttoptr/ptrtoint optimization.
I have removed the const keyword from the declaration of the CastResults array.
This change is temporary, and the array would be constified again.

Updated the check.

Updates:

1. Added check for nnan (earlier removed) and added new tests.
2. Check for nsz in select for fneg logic is required. So, I didn't remove this check. Please see this example: https://alive2.llvm.org/ce/z/sEfTYx.

Added the check and test case for the same.

In D101727#2832971, @spatel wrote:

I'm confused. This patch started by just switching the FMF propagation source from the fneg/fsub to the select. Now, we are back to taking flags from fneg and then intersecting some flags from the select.

But this approach has known bugs, as you have shown in the new Alive2 example. Why is this version of the patch better than just propagating all flags from the select?

This patch is complete and is ready for review.

I have made changes in FMF propagation logic.
FMF like nnan and ninf should be propagated only if present in select instruction, while for other flags like arcp, reassoc, it doesn't really depend on select instruction.

Added description for the test case.