[PATCH] D113173: [AsmPrinter][ORE] use correct opcode name

[ChenZheng via Phabricator via llvm-commits]

shchenz created this revision.
shchenz added reviewers: jsji, fhahn, arsenm, paquette, PowerPC.
Herald added subscribers: hiraditya, nemanjai.
shchenz requested review of this revision.
Herald added subscribers: llvm-commits, wdng.
Herald added a project: LLVM.

This is found on AIX when using opt-viewer.py.

On AIX, we have a pseudo instruction:

  def BCTRL_LWZinto_toc:
    XLForm_2_ext_and_DForm_1<19, 528, 20, 0, 1, 32, (outs),
     (ins memri:$src), "bctrl\n\tlwz 2, $src", IIC_BrB,
     [(PPCbctrl_load_toc iaddr:$src)]>, Requires<[In32BitMode]>;

This pseudo instruction consists of two instructions: `bctrl` without any explicit operands and `lwz` with two operands `2` and `$src`.

`getMnemonic` introduced in D90040 <https://reviews.llvm.org/D90040> returns a very strange opcode name for the above instruction, it is `bctrl\n\tlwz 2,` because `AsmWriterInst::AsmWriterInst()` treats `$` as separator between opcode and operands.

So in the output YAML when generating remarks for asm-printer pass, we get:

  - String:          "\n"
  - String:          "bctrl\n\tlwz 2, "
  - String:          ': '
  - INST_bctrl
        lwz 2,: '1' 
  - String:          "\n"

The invalid YAML will cause opt-viewer.py to work abnormally.

Seems `AsmWriterInst::AsmWriterInst()` treating `$` as the only separator for opcode name and operands is right, because `\n` or `\t` are both treated as a normal character in opcode name or operands name, and `\n\t` exists very common in instruction mnemonic on some targets.  so we can not fix the issue there.

And using `bctrl` as the opcode name for the pseudo instruction `BCTRL_LWZinto_toc` also follows the current design, the base class for the instruction form indicates:

  // Two joined instructions; used to emit two adjacent instructions as one.
  // The itinerary from the first instruction is used for scheduling and
  // classification.
  class I2 {
  }


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113173

Files:
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/test/CodeGen/PowerPC/instruction-mix-remarks-BCTRL_LWZinto_toc.ll


Index: llvm/test/CodeGen/PowerPC/instruction-mix-remarks-BCTRL_LWZinto_toc.ll
===================================================================
--- llvm/test/CodeGen/PowerPC/instruction-mix-remarks-BCTRL_LWZinto_toc.ll
+++ llvm/test/CodeGen/PowerPC/instruction-mix-remarks-BCTRL_LWZinto_toc.ll
@@ -6,8 +6,7 @@
 ; CHECK:  - String:          "\n"
 ; CHECK:  - String:          "bctrl\n\tld 2, "
 ; CHECK:  - String:          ': '
-; CHECK:  - INST_bctrl
-; CHECK:	ld 2,: '1'
+; CHECK:  - INST_bctrl:      '1'
 ; CHECK:  - String:          "\n"
 
 
Index: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
===================================================================
--- llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -1415,7 +1415,10 @@
       });
       R << "BasicBlock: " << ore::NV("BasicBlock", MBB.getName()) << "\n";
       for (auto &KV : MnemonicVec) {
-        auto Name = (Twine("INST_") + KV.first.trim()).str();
+        auto Name = (Twine("INST_") + KV.first.trim().take_until([](char c) {
+                      return c == ' ' || c == '\t' || c == '\n' || c == '\v' ||
+                             c == '\f' || c == '\r';
+                    })).str();
         R << KV.first << ": " << ore::NV(Name, KV.second) << "\n";
       }
       ORE->emit(R);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D113173.384674.patch
Type: text/x-patch
Size: 1315 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211104/bd255f36/attachment.bin>