[llvm] [TargetInstrInfo] enable foldMemoryOperand for InlineAsm (PR #70743)

Nick Desaulniers via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 14 12:51:21 PST 2023


================
@@ -565,6 +565,64 @@ static MachineInstr *foldPatchpoint(MachineFunction &MF, MachineInstr &MI,
   return NewMI;
 }
 
+static void foldInlineAsmMemOperand(MachineInstr *MI, unsigned OpNo, int FI,
+                                    const TargetInstrInfo &TII) {
+  MachineOperand &MO = MI->getOperand(OpNo);
+  const VirtRegInfo &RI = AnalyzeVirtRegInBundle(*MI, MO.getReg());
+
+  // If the machine operand is tied, untie it first.
+  if (MO.isTied()) {
+    unsigned TiedTo = MI->findTiedOperandIdx(OpNo);
+    MI->untieRegOperand(OpNo);
----------------
nickdesaulniers wrote:

> The resulting code will be fine as long as regalloc spills at most one operand. If it spills two or more, we'll have more than one memory location on someop. Assuming someop is a single x86 operation, I believe this will produce something invalid.

Correct, but for that case, I would say that the inline asm is invalid and should only use `m` on one or two of the three operands:

```diff
     asm (
         "someop %0, %1, %2"
-         : "=rm"(dst) : "rm"(a1), "rm"(a2));
+         : "=rm"(dst) : "r"(a1), "r"(a2));
```

That's just another instance of "the constraints were wrongly specified, but you didn't notice until there was register pressure (and the compiler was forced to choose m rather than r)." And that's not specific to clang or GCC.

---

> With tied operands, I believe we just make this more likely to happen

A lot of the cases you're thinking of seem to be related to any given single x86 instruction within inline asm likely not supporting more than 1 (or is it 2) memory operands.  But for tied operands, you likely have more than one instruction in your inline asm, so that's less likely to be problematic.

```c
asm (
  "mov %1, %r8\n\t"
  "mov %r8, %0"
  : "=rm"(output) : "0"(input));
```
so if we can't allocate `output` to a register, and `input` is tied, the single stack slot chosen for the both of them is not an issue, since they are used in different instructions (which happen to support register or memory for those operands).

My point is that the use of tied operands is likely to be with inline asm w/ more than one instruction, so the limitations of most modern CISC ISAs (is x86 _modern_...well, relative to other CISC) around how many memory operands they can have I think is not a concern.  (If it is, because only one instruction is being used, then it's probably a mistake for the programmer to specify "rm" rather than "r").

https://github.com/llvm/llvm-project/pull/70743


More information about the llvm-commits mailing list