[PATCH] D77118: [PowerPC] Ignore implicit register operands for MCInst

Zhang Kang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 31 02:42:52 PDT 2020


ZhangKang marked 4 inline comments as done.
ZhangKang added a comment.

In D77118#1951919 <https://reviews.llvm.org/D77118#1951919>, @steven.zhang wrote:

> I think this is a good catch. InstAlias match the MI strictly including the implicit operands. We can either try to loose that check or remove the implicit operand when lowering the MI to MCInst, as it is not needed anymore.  And please test this patch completely as  this patch expose much more alias opportunities that didn't test before.


Lit test and Lnt test has passed, and those modified assembly I have checked.



================
Comment at: llvm/test/CodeGen/PowerPC/atomics-regression.ll:1234
 ; PPC64LE-NEXT:    sync
-; PPC64LE-NEXT:  .LBB79_1:
+; PPC64LE-NEXT:  .LBB79_1: #
 ; PPC64LE-NEXT:    ldarx 6, 0, 3
----------------
steven.zhang wrote:
> Could you please explain more on the extra "#" here ?
This case is updated by update_llc_test_checks.py, last update is in Aug 30, 2019. After last update, someone modify the assembly output to add extra `#`.


================
Comment at: llvm/test/CodeGen/PowerPC/xray-conditional-return.ll:42
 ; CHECK-LABEL: @Foo2
-; CHECK:   cmpw [[CR:[0-9]+]]
-; CHECK-NEXT:   bge [[CR]], [[LABEL:\.[a-zA-Z0-9]+]]
+; CHECK:   cmpw
+; CHECK-NEXT:   bge 0, [[LABEL:\.[a-zA-Z0-9]+]]
----------------
steven.zhang wrote:
> I don't see this change relative with alias.
In fact, the old assembly is:
```
cmpw 0, R1, R2
beq 0, Target
```
The new assembly is:
```
cmpw R1, R2
beq 0, Target
```

Because `cmpw R1, R2` has omit `CR0`, I must specify beq use the CR0.

In fact, the `beq 0, Target` should also omit `CR0` to `beq Target`, but it's not this patch should do.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77118/new/

https://reviews.llvm.org/D77118





More information about the llvm-commits mailing list