Update title.

Use most generic range if K don't have a !noundef

In D146629#4213401, @jdoerfert wrote:

This is valid, I think.
That said, I went to try what could go wrong and it was easy.
At least early-cse is buggy so this will cause issues until those bugs are fixed:
https://godbolt.org/z/Tz98hYa9E

FWIW, instcombine gets it right regardless of the order. Might be luck or logic to properly merge the attributes.

Rebase and add test for load with !noundef

Keep the metadata if J has !noundef

Rename some variables.

Grammar fixing.

Address comments and add back test file.

Move function into a variable and add sub test.

Support BinOp Identity.

This patch caused a code size regression in our ci tests https://github.com/dtcxzyw/llvm-ci/issues/8. And part of the extra codes it generates is as follows:

Address comments.

In D144861#4175471, @nikic wrote:

I've posted https://reviews.llvm.org/D145510 as an alternative patch, which hopefully resolves all the issues with the current implementation.

Match binutils and spec.

Change base commit.

Rebase and address comments

In D145223#4166576, @nikic wrote:

In D145223#4166573, @xbolva00 wrote:

This optimization can be more general, why only add?

Right. This can be generalized at least to ConstantExpr::getBinOpIdentity(), but more generally to performing a simplification on all operands -- the latter might be expensive in terms of compile-time though.

In D145223#4166573, @xbolva00 wrote:

This optimization can be more general, why only add?

Rerun with --check-globals

Add back !range metadata

Replace with combinemetadataForCSE and make test more robust.

In D123515#4158758, @luismarques wrote:

@StephenFan Can you please rebase this?

In D144861#4160432, @StephenFan wrote:

! In D144861#4160179, @efriedma wrote:
From my analysis of the testcase, we called "getAddExpr(SCEV::FlagNUW)" while analyzing the first loop, and that broke the analysis of the second loop. The actual simplification isn't relevant.

Sorry, I don't know what you mean by "The actual simplification isn't relevant". Would you mind explaining a little more?

The getAddExpr() with NUW flag sets this flag globally, for all uses of this add. The icmp simplification itself doesn't make a difference, just the side effect that the (not globally correct) nuw flag is set.

In D144861#4160179, @efriedma wrote:

From my analysis of the testcase, we called "getAddExpr(SCEV::FlagNUW)" while analyzing the first loop, and that broke the analysis of the second loop. The actual simplification isn't relevant.

Fix

Add test files.

In D142801#4140817, @nikic wrote:

Yes. Do I need to do it all at once? My original thought was to do it step by step.

Do you plan to work on the other combineMetadata() users?

Is it correct if we load with the same type as the const array but the load's pointer operand is with the i8* type?

define i32 @load_gep_const_alleq_array(i64 %idx) {
  %gep = getelementptr inbounds i8, ptr @constalleqarray, i64 %idx
  %load = load i32, ptr %gep
  ret i32 %load
}

It seems the change in patchReplacementInstruction is not tested. I think it's better to make it tested.

I have a question what behavior the compiler raises if !invariant.group 's semantic is violated? For this case:

define i32 @foo(ptr nocapture %p, ptr nocapture %q) {
entry:
  %0 = load i32, ptr %p, align 4, !invariant.load !0
  store i8 2, ptr %p, align 1
  %1 = load i32, ptr %p, align 4, !invariant.load !0
  %add = add nsw i32 %1, %0
  ret i32 %add
}

https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp#L1283 may also be a potential user if a new PointerStripKind added.

In D143641#4125313, @nikic wrote:

I'm curious where this came up in practice. Constant GEP based on constant GEP gets merged by InstCombine.

TBH, The test was constructed by hand. So it may not make sense in practice since as you said it always gets merged by InstCombine. But that reminds me if it makes sense to also check whether the index operands are invariant?

Remove TODO.

Replace recursion as bounded iteration.

Add test of K dominates J.

In D142801#4087875, @nikic wrote:

Shouldn't MD_noundef be added to some more known metadata lists, and in particular also combineMetadataForCSE?

Fix typo.

Rebase and handle i1 vectors.

Replace attribute to noinline.

Add an assertion.

In D140974#4025888, @nikic wrote:

Shouldn't this be handled by adding this fold to InstCombine? https://alive2.llvm.org/ce/z/EXY5JH