LGTM.

Bot works.

Can you add more info about compilation errors and compiler version?

LGTM. Given some verbosity can really help us to understand code quickly.

LGTM with one nit.

I would suggest to retitle it to be more descriptive about code behavior, like 'Add check' blah blah. Describe what bugs it can fix in summary field.

Add indentation in RUN lines.

In D71391#1781219, @Jim wrote:

Hi @ZhangKang
You may follow @lkail 's suggestion is fine.
I am not a PowerPC guy. Just give some suggestions.

Hi @Jim we currently prefer to set this flag at a fine grained granularity, it's the purpose of sending a series of patches to perform such cleanups.

LGTM.

Can you provide tests to demonstrate how ISD::SIGN_EXTEND_INREG works on PowerPC?

In rG884351547da27e76e14bef5fe20c3e3cb0e89acd#857181, @jsji wrote:

@lkail When committing code for others, please follow the format described in Developer Policy. Thanks.
https://llvm.org/docs/DeveloperPolicy.html#attribution-of-changes

https://llvm.org/docs/DeveloperPolicy.html#commit-messages

Attribution of Changes should be in a separate line, after the end of the body, as simple as “Patch by John Doe.”. This is how we officially handle attribution, and there are automated processes that rely on this format.

@vddvss Landed with https://reviews.llvm.org/rG884351547da27e76e14bef5fe20c3e3cb0e89acd, thanks for fixing it.

Hi @vddvss do you need help landing this patch?

clang-10: /home/buildslave/buildslave/clang-cmake-armv7-selfhost-neon/llvm/llvm/lib/CodeGen/MachineOperand.cpp:118: bool llvm::MachineOperand::isRenamable() const: Assertion `Register::isPhysicalRegister(getReg()) && "isRenamable should only be checked on physical registers"' failed.

It misses check if the register is zero before invoking isRenamable.

It breaks http://lab.llvm.org:8011/builders/clang-cmake-armv7-selfhost/builds/2747, reverted. I'll investigate it.

As a newbie of PowerPC backend, I think what @jsji proposed will ease my mind burden when faced with o ended instructions. I have read ISA-3.0 and know

A period (.) as the last character of an instruction
mnemonic means that the instruction *records* sta-
tus information in certain fields of the Condition
Register as a side effect of execution.

I can directly react to the effect of record-form instruction if it ends with _record. If _record is too long, how about _rec, just like X86's _alt ended instructions?

Rebased and add comment for invalidateRegister.

LGTM. Thanks for fixing it. Personally, I would recommend Alive to verify peephole optimizations.

In D67794#1747301, @lenary wrote:

The RISC-V test changes look good. (I do not intend to review the optimisation implementation or other backend test changes).

Rebased and fixed broken tests. Ready to be reviewed.

Rebased and get broken tests, I'll investigate it and change this patch.

What it broke seems to be tests on Power8.

LGTM considering patterns added for SIGN_EXTENDED_INREG also follow its atomic semantics.

I plan to move the two changed swap-le tests into another NFC patch, since they're not related to the logic.

I think all test cases change should be reflected so that reviewers can have a look if they are regressions.

Add test case involving RegMask.

My concern is store and store volatile are so different in semantics that it might break original test intention.

Since d27a16eb392f39f9ee04ff5194b1eff3e189e6f8 reverts previous changes, LLVMDIBuilderCreateTypedef's signature becomes what it was. I'll revert this commit.

Introduce CopyTracker::invalidateRegister, original markRegUnavailable and clobberRegister are not appropriate for backward copy propagation.

Address @qcolombet's comment. @qcolombet I have no idea which is preferable to pass EnableBackwardCP option, target hook or cl::opt?

LGTM.

LGTM. I'll wait for @nemanjai if he has additional comments.

Hi @hiraditya , thanks for your work. I want to test your patch on PowerPC, however current patch seems unable to be applied to current master branch.

ping

ping

ping

Gentle ping

Hi @jgorbe , indeed we still get some enhance opportunity for this patch, such as considering register pressure. I'm wondering if the regression was caused by spilling.

I think the test can be simplified more, kinda like

# RUN: llc -O3 -mtriple=powerpc64le-unknown-linux-gnu -stop-after=ppc-pre-emit-peephole -verify-machineinstrs %s -o - | FileCheck %s
---                                                                                                                                                                           
name: foo                                                                                                                                                                   
alignment: 4                                                                                                                                                                  
tracksRegLiveness: true                                                                                                                                                       
body: |                                                                                                                                                                       
  bb.0.entry:                                                                                                                                                                 
    liveins: $r3                                                                                                                                                              
    renamable $r4 = LI 0                                                                                                                                                      
    renamable $r5 = SRW renamable $r3, renamable killed $r4                                                                                                                   
    $r3 = COPY renamable killed $r5                                                                                                                                           
    BLR implicit $lr, implicit $rm, implicit $r3                                                                                                                              
...

And a NFC patch to pre-commit the test would also be preferable.