• the metadata for the loads in the entry block don't need to be dropped? hi @aeubanks, I try to address above metadata issue on D146702.

close as commit e2f6ebe8ca15

Address comment
1、delete the repeat ICMP_NE check, as isEquality() already include it
2、use "thwart complexity-based canonicalization" for commuted case

Add "thwart complexity-based canonicalization"

address commet
1、Add more cases (also update on D146268)
2、update with getScalarSizeInBits and add related vector case
3、use ICmpInst::isEquality(Pred) to replace Pred == ICmpInst::ICMP_EQ
4、I.getPredicate() --> Pred

Add more tests according comment

ping ?

In D145846#4200141, @goldstein.w.n wrote:

The new IR with and removes a use of the input variable, which is better for analysis.

"and" -> And (otherwise it looks like the conjugation, not the operator.

1、Precommit the test
2、Add m_OneUse

Agree with @spatel , should be redesign in IR level

In D141188#4182953, @ayzhao wrote:

I'm continuing to see the same unit test failure as https://crbug.com/1420824#c2 after including the latest commit

Thank for your carefully examine.

In D141188#4180343, @glandium wrote:

The crash comes from D143225.

Thanks @glandium and @ayzhao, I'm trying to reproduce the above issue with chromium's code

In D141188#4166296, @glandium wrote:

FYI this patch also caused some of Firefox's unit tests to fail, e.g. https://treeherder.mozilla.org/logviewer?job_id=407511605&repo=toolchains&lineNumber=32470

Fix the runtime issue related to test unittests/ProfileData/ProfileDataTests.

For the runtime fails of unittests/ProfileData/ProfileDataTests, there is 2 points important

• a) build without -g fall as it transform code (From function _ZN12_GLOBAL__N_152CoverageMappingTest_test_line_coverage_iterator_Test8TestBodyEv)

_ZNK4llvm14iterator_rangeINS_8coverage20LineCoverageIteratorEE3endEv.exit: ; preds = %_ZNK4llvm14iterator_rangeINS_8coverage20LineCoverageIteratorEE5beginEv.exit, %if.then.i.i.i294
  %Stats.i.i295 = getelementptr inbounds %"class.llvm::coverage::LineCoverageIterator", ptr %__end1, i64 0, i32 6
  %Stats4.i.i296 = getelementptr inbounds %"class.llvm::iterator_range", ptr %ref.tmp32, i64 0, i32 1, i32 6
  call void @llvm.memcpy.p0.p0.i64(ptr noundef nonnull align 8 dereferenceable(40) %Stats.i.i295, ptr noundef nonnull align 8 dereferenceable(40) %Stats4.i.i296, i64 40, i1 false), !tbaa.struct !339
  %Next.i.i299 = getelementptr inbounds %"class.llvm::coverage::LineCoverageIterator", ptr %__begin1, i64 0, i32 2
  %Next3.i.i = getelementptr inbounds %"class.llvm::coverage::LineCoverageIterator", ptr %__end1, i64 0, i32 2
  %Ended.i.i301 = getelementptr inbounds %"class.llvm::coverage::LineCoverageIterator", ptr %__begin1, i64 0, i32 3
  %Ended4.i.i = getelementptr inbounds %"class.llvm::coverage::LineCoverageIterator", ptr %__end1, i64 0, i32 3
  %Line.i = getelementptr inbounds %"class.llvm::coverage::LineCoverageIterator", ptr %__begin1, i64 0, i32 6, i32 3
  %message_.i320 = getelementptr inbounds %"class.testing::AssertionResult", ptr %gtest_ar35, i64 0, i32 1
  %message_.i386 = getelementptr inbounds %"class.testing::AssertionResult", ptr %gtest_ar55, i64 0, i32 1
  br label %for.cond

In D141188#4161731, @bgraur wrote:

@Allen test unittests/ProfileData/ProfileDataTests fails when built with optimisations level -O1 with clang including this patch.

When building the test with -O3 (Release) or -O1 -g it passes.

Not sure if this patch triggers some other bug in clang or the test has UB or the patch itself is to blame.
Please revert this patch and investigate the issue without any pressure.

1. Rebase to update test after precommit
2. modify some cases's condition to avoid being their BBs optimizing away through the domination

rebase to top and fix the missing match

I will precommit to update the expired check on D144771

hi @arsenm, as the CHECK of these case are not updated with update_llc_test_checks.py, so it is a bit hard to update, do you mind I Precommit the CHECK regenerated with update_llc_test_checks.py for these 4 cases ?

Fix 4 AMDGPU backend failed cases

LLVM :: CodeGen/AMDGPU/branch-relaxation.ll
LLVM :: CodeGen/AMDGPU/collapse-endcf.ll
LLVM :: CodeGen/AMDGPU/infinite-loop.ll
LLVM :: CodeGen/AMDGPU/uniform-cfg.ll

Address comments

1. Add return false and its test case urem_with_dominating_condition_false
2. Delete some calls in function simplifyBinaryIntrinsic and foldICmpWithDominatingICmp, and update its case affected
3. update tile

In D144248#4140821, @nikic wrote:

Why was this abandoned? Just to be clear, I did not commit anything, just testing behavior.

In D144248#4140758, @nikic wrote:

I see. It looks like this much is safe to do without causing any regressions: https://github.com/nikic/llvm-project/commit/37366dc6bc6d168f081510a603982802a6964221 The ones in simplifyBinaryIntrinsic() and foldICmpWithDominatingICmp() are completely redundant.

The one in simplifySelectInst() is not redundant, because it uses a different context instruction: We would miss cases where we have br i1 %c followed by select i1 %c, because the comparison is not in the same block as the select. This is unfortunate...

In D144248#4139622, @nikic wrote:

Compile-time: https://llvm-compile-time-tracker.com/compare.php?from=45a0b812fa13ec255cae91f974540a4d805a8d79&to=787c3e2333e8d655a61ef013f7006d7675c03574&stat=instructions%3Au Some impact, but less than I expected, so I think we can do this.

This patch should also remove the isImpliedByDomCondition() calls in simplifySelectInst() and simplifyBinaryIntrinsic(), because they are subsumed by this one.

Yes, Agree your idea, thanks @spatel. I also realize that the cmn directive may not exist on all backends? so this is not appropriate.

hi @vitalybuka, as the issue can't be repoduced locally, do you object to retry again ?

update as precommit test in D144372 according review.

update comment

Address comments:
a) Drop all the attribute noundef on the argument and return value
b) Change all the value type to i8 as Alive2 can run quickly on all tests to verify correctness
c) Update case urem_without_assume

update upstream test case

Add a new case urem_with_dominating_condition

Address comments:
a) Adding a new case urem_without_assume
b) Update the code comment to match the code
c) Rename the FreezeX to FreezeOp0 as the name suggests

Address comments

• use match(Val, m_One()) to replace CVal->isOneValue()
• Drop the attribute noundef of function urem_assume_without_nuw, then it will see freeze
• Add a new negative case urem_assume_with_unexpected_const

use SQ.getWithInstruction(&I) to to change the context instruction

update to use dyn_cast_or_null according comment

Address comments:
a) check the assume with more generate API simplifyICmpInst
b) Add 3 test cases, including some Negative tests
c) Fix the checking and only do this when the return value is isOneValue()
d) Add the logic of frozen

Thanks @mkazantsev for correcting

I add a MR, try to remove the loop guard in loop guard D143705.

Thanks very much for detail idea.

sorry, missing the above review, and I'll fix that.

• Related link https://reviews.llvm.org/D143459