[PATCH] D158589: [MIPS] Add handing of forbidden slot when IR with inline asm

Ying Huang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 14 01:12:16 PST 2023


yingopq added a comment.

@craig.topper Could you kindly help review this issue patch at your convenience,  thanks!



================
Comment at: llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h:95
+
+  /// MIp - Pointer to the machine instruction of the current inlineAsm.
+  const MachineInstr *MIp;
----------------
MaskRay wrote:
> Don't repeat the variable/function name in new code.
> Don't repeat the variable/function name in new code.
You mean we should use another name, not MIp?


================
Comment at: llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h:96
+  /// MIp - Pointer to the machine instruction of the current inlineAsm.
+  const MachineInstr *MIp;
 };
----------------
MaskRay wrote:
> Do we have to have this variable?
> Do we have to have this variable?
In function MatchAndEmitInstruction, we use this structure Info.ParsedOperands to pass MI to target backend MIPS which need use MI to get next instruction of MBB.


================
Comment at: llvm/test/MC/Mips/forbidden-slot.s:1
+# RUN: clang --target=mipsel-linux-gnu -march=mips32r6 -c %s -o tmp.o
+# RUN: llvm-objdump -d tmp.o | FileCheck %s --check-prefix=MIPSELR6
----------------
MaskRay wrote:
> we cannot use clang in llvm tests due to layering reasons https://maskray.me/blog/2021-08-08-toolchain-testing#the-test-checks-at-the-wrong-layer. also, llvm-project may be built with just llvm, not clang, and this would fail.
OK, thanks, I would use llvm-mc, like this llvm-mc -assemble -mcpu=mips64r6 -arch=mips64el -filetype=obj %s -o tmp.o.

$ ./build/bin/llvm-lit -v llvm/test/MC/Mips/forbidden-slot.s
-- Testing: 1 tests, 1 workers --
PASS: LLVM :: MC/Mips/forbidden-slot.s (1 of 1)

Testing Time: 0.07s
  Passed: 1



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

https://reviews.llvm.org/D158589



More information about the llvm-commits mailing list