[NFC] Add a test case to make sure !prof is preserved when one instruction CSE'ed another.

[mingmingl]

Summary

• This should have been be a part of D148877. Before that patch, !prof is not added to known-id-set, and turns out unknown types of metadata are dropped in the implementation.
  • This test is mainly added to make sure there won't be regressions for this kind of pattern. The pattern is observed it in application code; looks like the result of indirect call is used as function arguments initially; after the function is inlined load-after-store CSE opportunity is exposed.

[tejohnson]

Can you expand the summary to explain why it is being dropped, and conceptually what the fix is doing? I'm not familiar with DoesKMove and what the 2 cases are here.

llvm/test/Transforms/EarlyCSE/metadata.ll

2

Add a comment at the top about what this test is testing.

22

What was being CSE'd here that caused it to get dropped?

[tejohnson]

Confused - there was a code change here previously - is it no longer needed?

[davidxl]

looks like the wrong test case is uploaded? Should it be CSE of direct calls with "branch_weight" profile instead of indirect calls (indirect calls are unlikely to be CSEed without points to information).

[mingmingl]

add a comment about what's being tested.

[mingmingl]

In D149396#4303857, @tejohnson wrote:

Confused - there was a code change here previously - is it no longer needed?

Yes the code change is no longer needed. I initially thought K's metadata needs to be explicitly reset to be preserved in combineMetadata when KDominatesJ is false (or DoesKMove is false), but realized it shouldn't be the case, so reverted the code change (to explicitly reset metadata again)

In D149396#4304082, @davidxl wrote:

looks like the wrong test case is uploaded? Should it be CSE of direct calls with "branch_weight" profile instead of indirect calls (indirect calls are unlikely to be CSEed without points to information).

Here the indirect call result CSE'ed a load-after-store instruction, rather than two indirect call instructions are CSE'ed to one

llvm/test/Transforms/EarlyCSE/metadata.ll

22

Here the optimizer finds %foo should have the same value as %call1 so it CSE'ed the load operation, and combines metadata of %call1 = tail call i32 %0(ptr %call, i32 %a, i32 %b), !prof !0 and %foo = load i32, ptr %x, align 4.

Before D148877, !prof is not added to known-id-set, and turns out unknown types of metadata are dropped in the implementation.

This test is mainly added to make sure there won't be regressions for this kind of pattern. The pattern is observed it in application code; looks like the result of indirect call is used as function arguments initially; after the function is inlined load-after-store is exposed (I didn't try to get a c++ repro since the IR is a little complex)

With 'opt -passes=early-cse -debug metadata.ll' debug log gives
EarlyCSE CSE LOAD: %foo = load i32, ptr %x, align 4 to: store i32 %call1, ptr %x, align 4 (related source code)

[mingmingl]

In D149396#4303857, @tejohnson wrote:

Confused - there was a code change here previously - is it no longer needed?

'llvm/lib/Transforms/Utils/Local.cpp' in https://reviews.llvm.org/D149387 is the original code change (that is not needed)

[davidxl]

makes sense.

[tejohnson]

lgtm

llvm/test/Transforms/EarlyCSE/metadata.ll

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 2
+; RUN: opt < %s -S -passes=early-cse | FileCheck %s
+
+; Test that when the load from %x is CSE'ed to %call1 by EarlyCSE, the !prof metadata
+; is not dropped.
+define i32 @indirect_call_value_profile_preserved(ptr %x, i32 %a, i32 %b, i32 %c)  {
+; CHECK-LABEL: define i32 @indirect_call_value_profile_preserved
+; CHECK-SAME: (ptr [[X:%.*]], i32 [[A:%.*]], i32 [[B:%.*]], i32 [[C:%.*]]) {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[CALL:%.*]] = tail call ptr @_Z10createTypei(i32 [[C]])
+; CHECK-NEXT:    [[VTABLE:%.*]] = load ptr, ptr [[CALL]], align 8
+; CHECK-NEXT:    [[VFN:%.*]] = getelementptr inbounds ptr, ptr [[VTABLE]], i64 1
+; CHECK-NEXT:    [[TMP0:%.*]] = load ptr, ptr [[VFN]], align 8
+; CHECK-NEXT:    [[CALL1:%.*]] = tail call i32 [[TMP0]](ptr [[CALL]], i32 [[A]], i32 [[B]]), !prof [[PROF0:![0-9]+]]
+; CHECK-NEXT:    store i32 [[CALL1]], ptr [[X]], align 4
+; CHECK-NEXT:    ret i32 [[CALL1]]
+;
+entry:
+  %call = tail call ptr @_Z10createTypei(i32 %c)
+  %vtable = load ptr, ptr %call, align 8
+  %vfn = getelementptr inbounds ptr, ptr %vtable, i64 1
+  %0 = load ptr, ptr %vfn, align 8
+  %call1 = tail call i32 %0(ptr %call, i32 %a, i32 %b), !prof !0
+  store i32 %call1, ptr %x
+  %foo = load i32, ptr %x
+  ret i32 %foo
+}
+
+declare ptr @_Z10createTypei(i32)
+
+!0 =!{!"VP", i32 0, i64 1600, i64 12345, i64 1030, i64 678, i64 410}