[Peephole-Opt][X86] Enhance peephole opt to see through SUBREG_TO_REG (following AND) and eliminates redundant TEST instruction.

[mingmingl]

Summary

The goal of this patch is to optimize away the test instruction in the following pattern

%reg = and ... EFLAGS set
... EFLAGS not changed.
%extended_reg = subreg_to_reg 0, %reg, %subreg.sub_index. EFLAGS not changed
testrr %extended_reg, %extended_reg, implicit-def $eflags. This could be optimized away.

Before this patch, the optimization wasn't carried out, since extended_reg is defined by SUBREG_TO_REG, which is not handled by isDefConvertible in X86InstrInfo.cpp.

The implementation optimize away the test if the bits used by TEST readers are already set by AND.

[davidxl]

I suspect there is something else going on.

The following case works fine without the patch.

define dso_local noundef i64 @_Z3fooPlll(ptr nocapture noundef %0, i64 noundef %1, i64 noundef %2) local_unnamed_addr #0 {

%4 = load i64, ptr %0, align 8
%5 = and i64 %4, 3
%6 = icmp eq i64 %5, 0
%7 = select i1 %6, i64 %1, i64 %2
store i64 %7, ptr %0, align 8
ret i64 %5

}

However a simple change below to replace the operand %2 in the select with %5 (output of and operation), then it stops working.

define dso_local noundef i64 @_Z3fooPlll(ptr nocapture noundef %0, i64 noundef %1, i64 noundef %2) local_unnamed_addr #0 {

%4 = load i64, ptr %0, align 8
%5 = and i64 %4, 3
%6 = icmp eq i64 %5, 0
%7 = select i1 %6, i64 %1, i64 %5
store i64 %7, ptr %0, align 8
ret i64 %5

}

Can you investigate the difference? The behavior difference on ARM is also worth comparing with.

[craig.topper]

llvm/test/CodeGen/X86/peephole-test-after-add.mir

29 ↗

(On Diff #423997)

Are the other basic blocks important?

107 ↗

(On Diff #423997)

Please use update_mir_test_checks.py

[craig.topper]

llvm/lib/Target/X86/X86InstrInfo.cpp

986 ↗

(On Diff #423997)

SUBREG_TO_REG is only used for i32->i64 conversions. So the only TEST it could ever be is TEST64rr.

997 ↗

(On Diff #423997)

This is making a copy of the a MachineOperand. Can you use a const reference?

999 ↗

(On Diff #423997)

It's not necessary.

1010 ↗

(On Diff #423997)

This is making a copy of the MachineOperand

1015 ↗

(On Diff #423997)

Drop curly braces

1020 ↗

(On Diff #423997)

Variable names should be capitalized

1058 ↗

(On Diff #423997)

Another copy

4318 ↗

(On Diff #423997)

AND/OR/XOR set the OF flag to 0.

4319 ↗

(On Diff #423997)

If you look through SUBREG_TO_REG then the width changed. The TEST64rr would have calculated the sign flag from bit 63(which is 0 due to the SUBREG_TO_REG), but the AND32 calculated it from bit 31 which is unknown.

[craig.topper]

In D124118#3463205, @davidxl wrote:

I suspect there is something else going on.

The following case works fine without the patch.

define dso_local noundef i64 @_Z3fooPlll(ptr nocapture noundef %0, i64 noundef %1, i64 noundef %2) local_unnamed_addr #0 {

%4 = load i64, ptr %0, align 8
%5 = and i64 %4, 3
%6 = icmp eq i64 %5, 0
%7 = select i1 %6, i64 %1, i64 %2
store i64 %7, ptr %0, align 8
ret i64 %5

}

However a simple change below to replace the operand %2 in the select with %5 (output of and operation), then it stops working.

define dso_local noundef i64 @_Z3fooPlll(ptr nocapture noundef %0, i64 noundef %1, i64 noundef %2) local_unnamed_addr #0 {

%4 = load i64, ptr %0, align 8
%5 = and i64 %4, 3
%6 = icmp eq i64 %5, 0
%7 = select i1 %6, i64 %1, i64 %5
store i64 %7, ptr %0, align 8
ret i64 %5

}

Can you investigate the difference? The behavior difference on ARM is also worth comparing with.

There's a peephole in X86DAGToDAGISel::Select and PostProcessISelDAG that both fail if the AND has users other than the TEST.

[davidxl]

In D124118#3463462, @craig.topper wrote:

In D124118#3463205, @davidxl wrote:

I suspect there is something else going on.

The following case works fine without the patch.

define dso_local noundef i64 @_Z3fooPlll(ptr nocapture noundef %0, i64 noundef %1, i64 noundef %2) local_unnamed_addr #0 {

%4 = load i64, ptr %0, align 8
%5 = and i64 %4, 3
%6 = icmp eq i64 %5, 0
%7 = select i1 %6, i64 %1, i64 %2
store i64 %7, ptr %0, align 8
ret i64 %5

}

However a simple change below to replace the operand %2 in the select with %5 (output of and operation), then it stops working.

define dso_local noundef i64 @_Z3fooPlll(ptr nocapture noundef %0, i64 noundef %1, i64 noundef %2) local_unnamed_addr #0 {

%4 = load i64, ptr %0, align 8
%5 = and i64 %4, 3
%6 = icmp eq i64 %5, 0
%7 = select i1 %6, i64 %1, i64 %5
store i64 %7, ptr %0, align 8
ret i64 %5

}

Can you investigate the difference? The behavior difference on ARM is also worth comparing with.

There's a peephole in X86DAGToDAGISel::Select and PostProcessISelDAG that both fail if the AND has users other than the TEST.

thanks. That explains it.

For this case, the subreg_to_reg is indeed the reason blocking the peephole-opt from cleaning it up (as can be seen in the case if the AND operand is changed to 0x1ffffffff where subreg_to_reg is not generated)

llvm/lib/Target/X86/X86InstrInfo.cpp

1043 ↗

(On Diff #423997)

Is there need to handle OR and XOR here? It seems we won't see code patterns with OR followed by subreg_to_reg.

[mingmingl]

Resolve the comments.

[mingmingl]

llvm/lib/Target/X86/X86InstrInfo.cpp

986 ↗

(On Diff #423997)

It looks like when SUBREG_TO_REG is used in SSE, the result type is not 64 bit [1]. So revised this helper function to

1. only handle the case when CmpInstr is TEST64rr and SUBREG_TO_REG is using sub_32bit as subregister index
2. returns false early for the rest of cases.

[1] https://github.com/llvm/llvm-project/blob/334522ca58aa6d6703405e479de8f5bf310ddbe2/llvm/lib/Target/X86/X86InstrSSE.td#L289

1043 ↗

(On Diff #423997)

(After reading this, it occurred to me existing definitions in td files in llvm/lib/Target/X86 are useful to find out how operands of SUBREG_TO-REG are generated).

Revised to handle AND only, considering

1. SUBREG_TO_REG almost always use 0 as immediate number.
2. it's hard to know the SF bit for OR, or XOR given 0 is the immediate number.

p.s. It seems [1] is where pattern (add64 -> and32 + subreg_to_reg) is given a higher priority, and I assume removing this from td and looking at the result could verify.

[1] https://github.com/llvm/llvm-project/blob/334522ca58aa6d6703405e479de8f5bf310ddbe2/llvm/lib/Target/X86/X86InstrCompiler.td#L1619

4318 ↗

(On Diff #423997)

ah.. thanks for the catch! corrected it.

4319 ↗

(On Diff #423997)

thanks for the catch!

Fix it by adding helper function isSFZero, and only optimize away the TEST64rr when SF is known to be zero.

llvm/test/CodeGen/X86/peephole-test-after-add.mir

29 ↗

(On Diff #423997)

Merge two IR functions into one file and by using the example from David.

[mingmingl]

In D124118#3463778, @davidxl wrote:

In D124118#3463462, @craig.topper wrote:

In D124118#3463205, @davidxl wrote:

I suspect there is something else going on.

The following case works fine without the patch.

define dso_local noundef i64 @_Z3fooPlll(ptr nocapture noundef %0, i64 noundef %1, i64 noundef %2) local_unnamed_addr #0 {

%4 = load i64, ptr %0, align 8
%5 = and i64 %4, 3
%6 = icmp eq i64 %5, 0
%7 = select i1 %6, i64 %1, i64 %2
store i64 %7, ptr %0, align 8
ret i64 %5

}

However a simple change below to replace the operand %2 in the select with %5 (output of and operation), then it stops working.

define dso_local noundef i64 @_Z3fooPlll(ptr nocapture noundef %0, i64 noundef %1, i64 noundef %2) local_unnamed_addr #0 {

%4 = load i64, ptr %0, align 8
%5 = and i64 %4, 3
%6 = icmp eq i64 %5, 0
%7 = select i1 %6, i64 %1, i64 %5
store i64 %7, ptr %0, align 8
ret i64 %5

}

Can you investigate the difference? The behavior difference on ARM is also worth comparing with.

There's a peephole in X86DAGToDAGISel::Select and PostProcessISelDAG that both fail if the AND has users other than the TEST.

thanks. That explains it.

For this case, the subreg_to_reg is indeed the reason blocking the peephole-opt from cleaning it up (as can be seen in the case if the AND operand is changed to 0x1ffffffff where subreg_to_reg is not generated)

Besides peephole-opt in X86ISelDAGToDAG that looks for (X86cmp (and $op, $imm), 0)[1], another significance is in legalization

Using llc -O3 -debug -debug-only=isel,x86-isel,selectiondag,legalizedag,peephole-opt,x86-instr-info <file.ll> gives the following log respectively

• https://gist.github.com/minglotus-6/39fb318886ae8deaf34a6f2f0d03fceb for the good case (TEST64rr erased)
• https://gist.github.com/minglotus-6/fcd07bc00d9bc362b7bd5675ba131740 for the bad case (TEST64rr kept)

To elaborate,

• for the good case
  • https://gist.github.com/minglotus-6/39fb318886ae8deaf34a6f2f0d03fceb#file-first-log-L96-L104 show legalization doesn't generate CMP.
• for the bad case
  • https://gist.github.com/minglotus-6/fcd07bc00d9bc362b7bd5675ba131740#file-second-log-L93-L101 shows legalization generates CMP

Tracking the implementation, X86TargetLowering::emitFlagsForSetcc has different paths

• when AND has one use (around line 24366), no cmp is emit
• for the bad case, will call EmitCmp around line 24426

(X86ISelLowering.cpp is too large to display in Github, or generate a permanent link, so add the line number)

[1] https://github.com/llvm/llvm-project/blob/360d44e86defea94fb5608765fbdbfdb2a36f4c6/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp#L5608

[mingmingl]

resolve a build failure due to conflicting function name (same return type, different argument list)

[craig.topper]

llvm/lib/Target/X86/X86InstrInfo.cpp

980 ↗

(On Diff #424124)

iz->is

[mingmingl]

Resolve the comment.

[mingmingl]

resolve comments

[mingmingl]

Remove new lines added by editor.

[mingmingl]

Mix two local branches.

Corrected it.

[mingmingl]

a gentle ping!~

[craig.topper]

llvm/lib/Target/X86/X86InstrInfo.cpp

970 ↗

(On Diff #424981)

I don't know if this is necessary. If the SUBREG_TO_REG was used, the sign bit of the full 64 bit value was provably zero during SelectionDAG. I hope any branch, setcc, cmov would have been folded away based on it.

Can't we just set NoSignFlag=true?

[mingmingl]

thanks for reviews!

llvm/lib/Target/X86/X86InstrInfo.cpp

970 ↗

(On Diff #424981)

isSFZero is used to tell the difference of following two scenarios, and (if I read correctly) NoSignFlag (as well as ClearsOverflowFlag) indicate whether the EFLAG bit is set (as opposed to the value of bit), so for AND operation, we want to set NoSignFlag to false.

AND32ri8 reg, 0x9  # signed bit is 1, so after sign extension, the SF bit could be 1 or 0 depending on reg value
SUBREG_TO_REG 0, extended_reg, reg , sub_32bit
TEST64rr extended_reg, extended_reg,  # SF bit is 0 since higher 32 bit of extended_reg is 0

AND32ri8 reg, 0x7  # signed bit is 0, so after sign extension, the SF bit is known to be 0
SUBREG_TO_REG 0, extended_reg, reg , sub_32bit
TEST64rr extended_reg, extended_reg,  # SF bit is 0 since higher 32 bit of extended_reg is 0

We want to optimize away test for 2) but keep test for 1).

Let me know if I miss anything!

[craig.topper]

llvm/lib/Target/X86/X86InstrInfo.cpp

970 ↗

(On Diff #424981)

If the instructions that use the flags from the TEST don't look at the SF flag then it doesn't matter. Setting NoSignFlag = true will disable the optimization if the SF flag is used. Your motivating test case doesn't use the sign flag so this should be sufficient.

[mingmingl]

Set NoSignFlag to false (to poison SF bit for readers), and get rid of isSFZero helper function.

[mingmingl]

llvm/lib/Target/X86/X86InstrInfo.cpp

970 ↗

(On Diff #424981)

I see your point now. Updated. PTAL.

[mingmingl]

Correct the auto-format a little bit

Before

// The reason to poison SF bit is that SF bit value could be different
// in the `AND` and `TEST` operation
//
// dest_reg = AND32ri8 src_reg, #imm  # signed bit is not known without
// peeking at #imm. extended_dest_reg = SUBREG_TO_REG 0, dest_reg, sub_32bit

After

// The reason to poison SF bit is that SF bit value could be different
// in the `AND` and `TEST` operation; signed bit is not known for `AND`
// without peeking at #imm, and is known to be 0 as a result of
// `TEST64rr`.
// dest_reg = AND32ri8 src_reg, #imm
// extended_dest_reg = SUBREG_TO_REG 0, dest_reg, sub_32bit
// TEST64rr extended_dest_reg, extended_dest_reg,

[mingmingl]

A gentle ping!~

[skan]

As to the desciption, maybe it should not be called a heuristic optimization?

llvm/lib/Target/X86/X86InstrInfo.cpp

970 ↗

(On Diff #425906)

Remove inline?

988–993 ↗

(On Diff #425906)

Could we remove these assert? I think they're not useful.

1004 ↗

(On Diff #425906)

Useless assert

1026 ↗

(On Diff #425906)

And32->and32

1027–1031 ↗

(On Diff #425906)

The format of comments here seems weird?

1032 ↗

(On Diff #425906)

And32->AND32

1048–1050 ↗

(On Diff #425906)

"The optimizatin will be disasbled"
I think this is not covered by your test yet?

1054 ↗

(On Diff #425906)

Why don't we peek at imm here? It's possible to set NoSignFlag to false.

1056–1058 ↗

(On Diff #425906)

Could you uniform the instruction name in your comments? You use SUBREG_TO_REG here while subreg_to_reg somewhere.

[skan]

llvm/test/CodeGen/X86/peephole-test-after-add.mir

2 ↗

(On Diff #425906)

Minor suggestion -o - %s -> < %s

[skan]

llvm/lib/Target/X86/X86InstrInfo.cpp

1054 ↗

(On Diff #425906)

Never mind. I missed craig's comments

[skan]

llvm/test/CodeGen/X86/peephole-test-after-add.mir

2 ↗

(On Diff #425906)

Never mind. I didn't realize that "< %s" does not work for "--run-pass".

[craig.topper]

llvm/test/CodeGen/X86/peephole-test-after-add.mir

2 ↗

(On Diff #425906)

I think it doesn't work for .mir input because llc uses the extension of the file name is used to determine file type. If you use re-direction llc can't see the extension so just assumes .ll.

[mingmingl]

Address the style comments, and add a regression test to gate the behavior that TEST is retained if SF bit is read.

llvm/lib/Target/X86/X86InstrInfo.cpp

970 ↗

(On Diff #425906)

Removed inline.

For my understanding, is it because of lengthy function body and long argument list that inline hint doesn't make much sense here?

988–993 ↗

(On Diff #425906)

Could we remove these assert? I think they're not useful.

Removed the second assert(CmpValDefInstr.getNumOperands() == 4).

I wonder if the 1st assert (assert(MRI->getVRegDef(CmpInstr.getOperand(0).getReg()) == &CmpValDefInstr)) helps in the sense that

1. the information asserted (def-use between this two instructions) are not explicitly mentioned in any comment
2. this information is important for the correctness

Nevertheless kept the 1st assert and added comment. Let me know what I miss.

1004 ↗

(On Diff #425906)

Since the function body getImm() or getReg() has asserts, it makes sense to just rely on that to give enough debugging information.

So removed three asserts (on MO3, MO2, MO1)

1027–1031 ↗

(On Diff #425906)

thanks for the catch! Fixed it.

1048–1050 ↗

(On Diff #425906)

it makes sense to add a test.

Added test_not_erased_when_sf_used in llvm/test/CodeGen/X86/peephole-test-after-add.mir, to cover the behavior that TEST64rr is retained if SF bit is used.

1054 ↗

(On Diff #425906)

Yeah I was initially thinking of peeking at imm, and later (upon Craig's comment) realized there is another way to do this.

1056–1058 ↗

(On Diff #425906)

Unified the instruction name in this patch.

p.s. I realized this sequence of instructions should be better placed around line 1024, so merge it with the sequence of instructions right after if (X86::isAND(VregDefInstr->getOpcode())) {

llvm/test/CodeGen/X86/peephole-test-after-add.mir

2 ↗

(On Diff #425906)

thanks for the discussion! I'll keep it the current way and reply first, before getting a better idea of the difference (between -o - %s and %s in this command) myself.

[mingmingl]

In D124118#3498831, @skan wrote:

As to the desciption, maybe it should not be called a heuristic optimization?

modified summary with diff

1. removed heurstic
2. optimize test away if EFLAG bits used as a result of test are already set by AND.

[mingmingl]

[Only two minor style changes]
Rename the .ll file in llvm/test/CodeGen/X86/peephole-test-after-add.mir, and remove braces on simple single statement bodies of if-else statements.

[skan]

llvm/lib/Target/X86/X86InstrInfo.cpp

995 ↗

(On Diff #428251)

What does the "Caller guranteed" mean here? Does it say that TEST64rr is the user of SUBREG_TO_REG?

1031 ↗

(On Diff #428251)

const MachineInstr &

970 ↗

(On Diff #425906)

Yes. In fact, I think it's useless to add inline to functions w/ internal linkage for non-peformance critical code. Compiler can decide whether it should be inlined.

[mingmingl]

Replied the comments.

Besides, in findRedundantFlagInstr, change from

  const MachineOperand& MO1 = CmpValDefInstr.getOperand(1);
// As seen in X86 td files, MO1 is typically
  // 0.
  if (.MO1.getImm() != 0)
    return false;

to

// As seen in X86 td files, CmpValDefInstr.getOperand(1).getImm() is typically
  // 0.
  if (CmpValDefInstr.getOperand(1).getImm() != 0)
    return false;

Same for MO2 and MO3.

[skan]

LGTM!

[mingmingl]

(I forgot to click "reply" to send out the inline comments... but going to do it now)

Thanks for all reviews and inputs!

Going to hold it for a while since other reviewers might have comments as well.

llvm/lib/Target/X86/X86InstrInfo.cpp

995 ↗

(On Diff #428251)

Does it say that TEST64rr is the user of SUBREG_TO_REG?

Yes this is correct. I think this assertion might prevent inadvertent misuse of findRedundantFlagInstr, yet I agree it's probably premature at this point.

Let me know if it's more idiomatic to just remove this assertion.

970 ↗

(On Diff #425906)

Compiler can decide whether it should be inlined.

This is true.

[davidxl]

llvm/test/CodeGen/X86/peephole-test-after-add.mir

23 ↗

(On Diff #428279)

The and instruction clears SF and upper bits of the value which means the test instruction later will also produce SF == 0. In other words, in this case, not only the test instruction, but also the CMOVErr can be optimized into a copy.

(this can be looked at in a follow up patch if it is the case).

[craig.topper]

llvm/test/CodeGen/X86/peephole-test-after-add.mir

23 ↗

(On Diff #428279)

Do we not figure that out in SelectionDAG?

[mingmingl]

llvm/test/CodeGen/X86/peephole-test-after-add.mir

23 ↗

(On Diff #428279)

The and instruction clears SF and upper bits of the value which means the test instruction later will also produce SF == 0. In other words, in this case, not only the test instruction, but also the CMOVErr can be optimized into a copy.

(this can be looked at in a follow up patch if it is the case).

Added a FIXME around line 1052 (inside findRedundantFlagInstr) to follow up on this.

Do we not figure that out in SelectionDAG?

Not sure if we could figure it out in SelectionDAG step. There is TargetInstrInfo::optimizeSelect virtual method for peephole-opt, and an implementation (ARMBaseInstrInfo::optimizeSelect) [1]. Will look into which pass to put this optimization later.

[1] https://github.com/llvm/llvm-project/blob/0353c2c996c5863463c356de97c9852f9330ed11/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp#L2321

[mingmingl]

Add FIXME for a follow-up optimization.

[craig.topper]

llvm/test/CodeGen/X86/peephole-test-after-add.mir

23 ↗

(On Diff #428279)

I thought DAGCombiner for ISD::SETCC should be able to figure out that an AND with 3 guarantees the sign bit is 0, but it appears it does not. InstCombine does optimize it though.

[mingmingl]

llvm/test/CodeGen/X86/peephole-test-after-add.mir

23 ↗

(On Diff #428279)

Got it. DAGCombiner sounds a right place to look at a sequence of instructions and optimize.

[mingmingl]

Reopen so as to re-commit (upon request https://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20220509/1036289.html)

[mingmingl]

Reapply with a descriptive commit message.

[craig.topper]

Please put [X86] in the title.

[dim]

FYI https://github.com/llvm/llvm-project/issues/56103

llvm/lib/Passes/PassBuilderPipelines.cpp

 static InlineParams getInlineParamsFromOptLevel(OptimizationLevel Level) {
   return getInlineParams(Level.getSpeedupLevel(), Level.getSizeLevel());
 }
 
 ModuleInlinerWrapperPass
 PassBuilder::buildInlinerPipeline(OptimizationLevel Level,
                                   ThinOrFullLTOPhase Phase) {
   InlineParams IP = getInlineParamsFromOptLevel(Level);
+  // For PreLinkThinLTO + SamplePGO, set hot-caller threshold to 0 to
+  // disable hot callsite inline (as much as possible [1]) because it makes
+  // profile annotation in the backend inaccurate.
+  //
+  // [1] Note the cost of a function could be below zero due to erased
+  // prologue / epilogue.
   if (Phase == ThinOrFullLTOPhase::ThinLTOPreLink && PGOOpt &&
       PGOOpt->Action == PGOOptions::SampleUse)
     IP.HotCallSiteThreshold = 0;
 
   if (PGOOpt)
     IP.EnableDeferral = EnablePGOInlineDeferral;
 
   ModuleInlinerWrapperPass MIWP(IP, PerformMandatoryInliningsFirst,

 }
 
 ModulePassManager
 PassBuilder::buildModuleInlinerPipeline(OptimizationLevel Level,
                                         ThinOrFullLTOPhase Phase) {
   ModulePassManager MPM;
 
   InlineParams IP = getInlineParamsFromOptLevel(Level);
+  // For PreLinkThinLTO + SamplePGO, set hot-caller threshold to 0 to
+  // disable hot callsite inline (as much as possible [1]) because it makes
+  // profile annotation in the backend inaccurate.
+  //
+  // [1] Note the cost of a function could be below zero due to erased
+  // prologue / epilogue.
   if (Phase == ThinOrFullLTOPhase::ThinLTOPreLink && PGOOpt &&
       PGOOpt->Action == PGOOptions::SampleUse)
     IP.HotCallSiteThreshold = 0;
 
   if (PGOOpt)
     IP.EnableDeferral = EnablePGOInlineDeferral;
 
   // The inline deferral logic is used to avoid losing some

llvm/test/Other/Inputs/new-pm-thinlto-prelink-samplepgo-inline-threshold.prof

+_Z3sumii:105580:5279
+ 0: 5279
+ 1: 5279
+ 2: 5279
+_Z4sum1ii:105580:5279
+ 0: 5279
+ 1: 5279
+ 2: 5279
+main:225715:0
+ 2.1: 5553
+ 3: 4522
+ # This indicates that at line 3 of this function, the 'then' branch
+ # of the conditional is taken (discriminator '1').
+ 3.1: 5752 _Z3sumii:5860
+ 4: 20 _Z4sum1ii:20
+ 4.1: 11504 _Z4sum1ii:11720

llvm/test/Other/new-pm-thinlto-prelink-samplepgo-inline-threshold.ll

+; Tests that hot callsite threshold is set to 0 artifically for thinlto-prelink pipeline.
+;
+; Function `sum` is annotated with inline cost -1 and function `sum1` is
+; annotated with inline cost 0, by function attribute `function-inline-cost`.
+;
+; `if.then` basic block is hot so the callsite threshold is set to 0.
+; `while.body.split` basic block is cold so threshold is calculated using
+; the rest of default heuristics, which should be sufficient to inline a
+; function with 0 cost.
+
+; RUN: opt < %s -pass-remarks=inline -pass-remarks-missed=inline -passes='thinlto-pre-link<O2>' -pgo-kind=pgo-sample-use-pipeline -sample-profile-file=%S/Inputs/new-pm-thinlto-prelink-samplepgo-inline-threshold.prof -S | FileCheck %s
+
+; RUN: opt < %s -pass-remarks=inline -pass-remarks-missed=inline -passes='thinlto-pre-link<O2>' -pgo-kind=pgo-sample-use-pipeline -sample-profile-file=%S/Inputs/new-pm-thinlto-prelink-samplepgo-inline-threshold.prof -S 2>&1 | FileCheck %s -check-prefix=REMARK
+
+; Original C++ test case
+;
+; #include <stdio.h>
+
+; int sum(int x, int y) { return x + y; }
+
+; int sum1(int x, int y) { return x + y; }
+
+; int main() {
+;   int s, i = 0;
+;   while (i++ < 20000 * 20000) {
+;     if (i != 100) s = sum(i, s);
+;     s = sum1(i, s);
+;   }
+;   printf("sum is %d\n", s);
+;   return 0;
+; }
+;
+
+; REMARK: test.cc:14:9: '_Z4sum1ii' inlined into 'main' with (cost=0, threshold=45) at callsite main:4:9;
+; REMARK: test.cc:13:23: '_Z3sumii' inlined into 'main' with (cost=-1, threshold=0) at callsite main:3:23.2;
+; REMARK: test.cc:14:9: '_Z4sum1ii' not inlined into 'main' because too costly to inline (cost=0, threshold=0)
+
+; ModuleID = 'test.cc'
+source_filename = "test.cc"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@.str = constant [11 x i8] c"sum is %d\0A\00", align 1
+
+define i32 @_Z3sumii(i32 %x, i32 %y) "function-inline-cost"="-1" #0 !dbg !7 {
+entry:
+  %add = add nsw i32 %y, %x, !dbg !10
+  ret i32 %add, !dbg !11
+}
+
+define i32 @_Z4sum1ii(i32 %x, i32 %y) "function-inline-cost"="0" #0 !dbg !12 {
+entry:
+  %add = add nsw i32 %y, %x, !dbg !13
+  ret i32 %add, !dbg !14
+}
+
+define i32 @main() #0 !dbg !15 {
+entry:
+  br label %while.body, !dbg !16
+
+while.body:                                       ; preds = %entry, %if.end
+  %inc14 = phi i32 [ 1, %entry ], [ %inc, %if.end ]
+  %s.013 = phi i32 [ undef, %entry ], [ %phi.call, %if.end ]
+  %cmp1.not = icmp eq i32 %inc14, 100, !dbg !17
+  br i1 %cmp1.not, label %while.body.split, label %if.then, !dbg !18
+
+while.body.split:                                 ; preds = %while.body
+; CHECK-NOT: call noundef i32 @_Z4sum1ii
+  %call211 = call noundef i32 @_Z4sum1ii(i32 100, i32 %s.013), !dbg !19
+  br label %if.end, !dbg !18
+
+if.then:                                          ; preds = %while.body
+; CHECK-NOT: call i32 @_Z3sumii
+; CHECK: call i32 @_Z4sum1ii
+  %call = call i32 @_Z3sumii(i32 %inc14, i32 %s.013), !dbg !20
+  %call212 = call i32 @_Z4sum1ii(i32 %inc14, i32 %call), !dbg !19
+  br label %if.end, !dbg !21
+
+if.end:                                           ; preds = %while.body.split, %if.then
+  %phi.call = phi i32 [ %call211, %while.body.split ], [ %call212, %if.then ], !dbg !19
+  %inc = add i32 %inc14, 1, !dbg !22
+  %exitcond.not = icmp eq i32 %inc, 400000001, !dbg !23
+  br i1 %exitcond.not, label %while.end, label %while.body, !dbg !16, !llvm.loop !24
+
+while.end:                                        ; preds = %if.end
+  %call3 = tail call i32 (i8*, ...) @printf(i8* dereferenceable(1) getelementptr inbounds ([11 x i8], [11 x i8]* @.str, i64 0, i64 0), i32 %phi.call), !dbg !27
+  ret i32 0, !dbg !28
+}
+
+declare i32 @printf(i8* nocapture noundef readonly, ...)
+
+attributes #0 = {"use-sample-profile"}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 14.0.0 (https://github.com/llvm/llvm-project.git 329fda39c507e8740978d10458451dcdb21563be)", isOptimized: true, runtimeVersion: 0, emissionKind: LineTablesOnly, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "test.cc", directory: "", checksumkind: CSK_MD5, checksum: "e8c44edbdcc2c41f9f891ac2b2ddd591")
+!2 = !{i32 7, !"Dwarf Version", i32 5}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!7 = distinct !DISubprogram(name: "sum", scope: !1, file: !1, line: 6, type: !8, scopeLine: 6, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !9)
+!8 = !DISubroutineType(types: !9)
+!9 = !{}
+!10 = !DILocation(line: 6, column: 60, scope: !7)
+!11 = !DILocation(line: 6, column: 51, scope: !7)
+!12 = distinct !DISubprogram(name: "sum1", scope: !1, file: !1, line: 8, type: !8, scopeLine: 8, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !9)
+!13 = !DILocation(line: 8, column: 61, scope: !12)
+!14 = !DILocation(line: 8, column: 52, scope: !12)
+!15 = distinct !DISubprogram(name: "main", scope: !1, file: !1, line: 10, type: !8, scopeLine: 10, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !9)
+!16 = !DILocation(line: 12, column: 3, scope: !15)
+!17 = !DILocation(line: 13, column: 11, scope: !15)
+!18 = !DILocation(line: 13, column: 9, scope: !15)
+!19 = !DILocation(line: 14, column: 9, scope: !15)
+!20 = !DILocation(line: 13, column: 23, scope: !15)
+!21 = !DILocation(line: 13, column: 19, scope: !15)
+!22 = !DILocation(line: 12, column: 11, scope: !15)
+!23 = !DILocation(line: 12, column: 14, scope: !15)
+!24 = distinct !{!24, !16, !25, !26}
+!25 = !DILocation(line: 15, column: 3, scope: !15)
+!26 = !{!"llvm.loop.mustprogress"}
+!27 = !DILocation(line: 16, column: 3, scope: !15)
+!28 = !DILocation(line: 17, column: 3, scope: !15)