[PATCH] D114216: [asm] Remove explicit branch for modifier 'l'

[Nico Weber via Phabricator via llvm-commits]

thakis created this revision.
thakis added a reviewer: hans.
Herald added subscribers: pengfei, Jim, hiraditya, dylanmckay.
thakis requested review of this revision.
Herald added a project: LLVM.

No intended behavior change.

EmitGCCInlineAsmStr() used to explicitly check for modifier 'l'
after handling block address and machine basic block operands.
This prevented passing a MachineOperand with 'l' modifier to
PrintAsmMemoryOperand(). Conceptually that seems kind of nice,
but in practice the overrides of PrintAsmMemoryOperand() in all (*)
AsmPrinter subclasses already reject modifiers they don't know about,
and none of them don't know about 'l'. So removing this doesn't have
a behavior difference, is less code, and it makes EmitGCCInlineAsmStr()
and EmitMSInlineAsmStr() more similar, to prepare for merging them later.

(Why not _add_ the branch to EmitMSInlineAsmStr() instead? Because that
always works with X86AsmPrinter I think, and
X86AsmPrinter::PrintAsmMemoryOperand() very decisively rejects the 'l'
modifier, so it's hard to motivate adding that branch.)

*: The one exception was AVRAsmPrinter, which had an llvm_unreachable
instead of returning true. So this commit changes that, so that
the AVR target keeps not crashing when passing a mem operand with a
:l modifier to it. All the other targets already don't crash on this.


https://reviews.llvm.org/D114216

Files:
  llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
  llvm/lib/Target/AVR/AVRAsmPrinter.cpp
  llvm/test/CodeGen/AVR/inline-asm/inline-asm-invalid.ll


Index: llvm/test/CodeGen/AVR/inline-asm/inline-asm-invalid.ll
===================================================================
--- /dev/null
+++ llvm/test/CodeGen/AVR/inline-asm/inline-asm-invalid.ll
@@ -0,0 +1,10 @@
+; RUN: not llc < %s -march=avr -no-integrated-as 2>&1 | FileCheck %s
+
+define void @foo(i16 %a) {
+  ; CHECK: error: invalid operand in inline asm: 'jl ${0:l}'
+  %i.addr = alloca i32, align 4
+  call void asm sideeffect "jl ${0:l}", "*m"(i32* %i.addr)
+
+  ret void
+}
+
Index: llvm/lib/Target/AVR/AVRAsmPrinter.cpp
===================================================================
--- llvm/lib/Target/AVR/AVRAsmPrinter.cpp
+++ llvm/lib/Target/AVR/AVRAsmPrinter.cpp
@@ -144,9 +144,8 @@
 bool AVRAsmPrinter::PrintAsmMemoryOperand(const MachineInstr *MI,
                                           unsigned OpNum, const char *ExtraCode,
                                           raw_ostream &O) {
-  if (ExtraCode && ExtraCode[0]) {
-    llvm_unreachable("This branch is not implemented yet");
-  }
+  if (ExtraCode && ExtraCode[0])
+    return true; // Unknown modifier
 
   const MachineOperand &MO = MI->getOperand(OpNum);
   (void)MO;
Index: llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
===================================================================
--- llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
+++ llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
@@ -453,8 +453,6 @@
           } else if (MI->getOperand(OpNo).isMBB()) {
             const MCSymbol *Sym = MI->getOperand(OpNo).getMBB()->getSymbol();
             Sym->print(OS, AP->MAI);
-          } else if (Modifier[0] == 'l') {
-            Error = true;
           } else if (InlineAsm::isMemKind(OpFlags)) {
             Error = AP->PrintAsmMemoryOperand(
                 MI, OpNo, Modifier[0] ? Modifier : nullptr, OS);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D114216.388365.patch
Type: text/x-patch
Size: 1833 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211119/50711d9e/attachment.bin>