Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add diagnostic help for inline asm operand constraint 'H' #86910

Closed
wants to merge 5 commits into from

Conversation

mahesh-attarde
Copy link
Contributor

Using inline asm operand constraint 'H' with constants does not reflect helpful error.
With addition of this message operands those are not offsetable, will report exact root cause.
Originally https://reviews.llvm.org/D35890

@llvmbot
Copy link
Collaborator

llvmbot commented Mar 28, 2024

@llvm/pr-subscribers-backend-webassembly
@llvm/pr-subscribers-backend-msp430
@llvm/pr-subscribers-backend-x86
@llvm/pr-subscribers-backend-loongarch
@llvm/pr-subscribers-backend-aarch64
@llvm/pr-subscribers-backend-m68k
@llvm/pr-subscribers-backend-amdgpu

@llvm/pr-subscribers-backend-risc-v

Author: None (mahesh-attarde)

Changes

Using inline asm operand constraint 'H' with constants does not reflect helpful error.
With addition of this message operands those are not offsetable, will report exact root cause.
Originally https://reviews.llvm.org/D35890


Patch is 40.56 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/86910.diff

34 Files Affected:

  • (modified) llvm/include/llvm/CodeGen/AsmPrinter.h (+4-2)
  • (modified) llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp (+6-3)
  • (modified) llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp (+5-3)
  • (modified) llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp (+3-2)
  • (modified) llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.h (+2-1)
  • (modified) llvm/lib/Target/ARM/ARMAsmPrinter.cpp (+3-2)
  • (modified) llvm/lib/Target/ARM/ARMAsmPrinter.h (+2-1)
  • (modified) llvm/lib/Target/AVR/AVRAsmPrinter.cpp (+5-3)
  • (modified) llvm/lib/Target/BPF/BPFAsmPrinter.cpp (+5-3)
  • (modified) llvm/lib/Target/CSKY/CSKYAsmPrinter.cpp (+3-2)
  • (modified) llvm/lib/Target/Hexagon/HexagonAsmPrinter.cpp (+2-2)
  • (modified) llvm/lib/Target/Hexagon/HexagonAsmPrinter.h (+2-1)
  • (modified) llvm/lib/Target/Lanai/LanaiAsmPrinter.cpp (+5-3)
  • (modified) llvm/lib/Target/LoongArch/LoongArchAsmPrinter.cpp (+3-2)
  • (modified) llvm/lib/Target/LoongArch/LoongArchAsmPrinter.h (+2-1)
  • (modified) llvm/lib/Target/M68k/M68kAsmPrinter.cpp (+3-2)
  • (modified) llvm/lib/Target/MSP430/MSP430AsmPrinter.cpp (+5-3)
  • (modified) llvm/lib/Target/Mips/MipsAsmPrinter.cpp (+3-2)
  • (modified) llvm/lib/Target/Mips/MipsAsmPrinter.h (+2-1)
  • (modified) llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp (+3-2)
  • (modified) llvm/lib/Target/NVPTX/NVPTXAsmPrinter.h (+2-1)
  • (modified) llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp (+5-3)
  • (modified) llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp (+5-3)
  • (modified) llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp (+4-2)
  • (modified) llvm/lib/Target/Sparc/SparcAsmPrinter.cpp (+4-3)
  • (modified) llvm/lib/Target/SystemZ/SystemZAsmPrinter.cpp (+2-2)
  • (modified) llvm/lib/Target/SystemZ/SystemZAsmPrinter.h (+2-1)
  • (modified) llvm/lib/Target/VE/VEAsmPrinter.cpp (+5-3)
  • (modified) llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp (+3-2)
  • (modified) llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.h (+2-1)
  • (modified) llvm/lib/Target/X86/X86AsmPrinter.cpp (+8-2)
  • (modified) llvm/lib/Target/X86/X86AsmPrinter.h (+2-1)
  • (modified) llvm/lib/Target/XCore/XCoreAsmPrinter.cpp (+5-3)
  • (added) llvm/test/CodeGen/X86/inline-asm-modifier-error.ll (+9)
diff --git a/llvm/include/llvm/CodeGen/AsmPrinter.h b/llvm/include/llvm/CodeGen/AsmPrinter.h
index a7fbf4aeb74494..e0373322ef8c4f 100644
--- a/llvm/include/llvm/CodeGen/AsmPrinter.h
+++ b/llvm/include/llvm/CodeGen/AsmPrinter.h
@@ -816,9 +816,11 @@ class AsmPrinter : public MachineFunctionPass {
 
   /// Print the specified operand of MI, an INLINEASM instruction, using the
   /// specified assembler variant.  Targets should override this to format as
-  /// appropriate.  This method can return true if the operand is erroneous.
+  /// appropriate.  This method can return true if the operand is erroneous,
+  /// method can provide helpful diagnostic message in some case.
   virtual bool PrintAsmOperand(const MachineInstr *MI, unsigned OpNo,
-                               const char *ExtraCode, raw_ostream &OS);
+                               const char *ExtraCode, raw_ostream &OS,
+                               std::string &ErrorMsg);
 
   /// Print the specified operand of MI, an INLINEASM instruction, using the
   /// specified assembler variant as an address. Targets should override this to
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
index d0ef3e5a19391c..aa060736ffe264 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
@@ -285,6 +285,7 @@ static void EmitInlineAsmStr(const char *AsmStr, const MachineInstr *MI,
         // We may have a location metadata attached to the end of the
         // instruction, and at no point should see metadata at any
         // other point while processing. It's an error if so.
+        std::string ErrorMsg;
         if (OpNo >= MI->getNumOperands() || MI->getOperand(OpNo).isMetadata()) {
           Error = true;
         } else {
@@ -307,13 +308,14 @@ static void EmitInlineAsmStr(const char *AsmStr, const MachineInstr *MI,
                 MI, OpNo, Modifier[0] ? Modifier : nullptr, OS);
           } else {
             Error = AP->PrintAsmOperand(MI, OpNo,
-                                        Modifier[0] ? Modifier : nullptr, OS);
+                                        Modifier[0] ? Modifier : nullptr, OS,
+                                        ErrorMsg);
           }
         }
         if (Error) {
           std::string msg;
           raw_string_ostream Msg(msg);
-          Msg << "invalid operand in inline asm: '" << AsmStr << "'";
+          Msg << "invalid operand in inline asm: '" << AsmStr << "'" <<ErrorMsg;
           MMI->getModule()->getContext().emitError(LocCookie, Msg.str());
         }
       }
@@ -466,7 +468,8 @@ void AsmPrinter::PrintSymbolOperand(const MachineOperand &MO, raw_ostream &OS) {
 /// override this to format as appropriate for machine specific ExtraCodes
 /// or when the arch-independent handling would be too complex otherwise.
 bool AsmPrinter::PrintAsmOperand(const MachineInstr *MI, unsigned OpNo,
-                                 const char *ExtraCode, raw_ostream &O) {
+                                 const char *ExtraCode, raw_ostream &O,
+                                 std::string &ErrorMsg) {
   // Does this asm operand have a single letter operand modifier?
   if (ExtraCode && ExtraCode[0]) {
     if (ExtraCode[1] != 0) return true; // Unknown modifier.
diff --git a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
index 4fa719ad67cf33..4e52f2d01d764a 100644
--- a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
+++ b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
@@ -177,7 +177,8 @@ class AArch64AsmPrinter : public AsmPrinter {
                           raw_ostream &O);
 
   bool PrintAsmOperand(const MachineInstr *MI, unsigned OpNum,
-                       const char *ExtraCode, raw_ostream &O) override;
+                       const char *ExtraCode, raw_ostream &O, 
+                       std::string &ErrorMsg) override;
   bool PrintAsmMemoryOperand(const MachineInstr *MI, unsigned OpNum,
                              const char *ExtraCode, raw_ostream &O) override;
 
@@ -910,11 +911,12 @@ bool AArch64AsmPrinter::printAsmRegInClass(const MachineOperand &MO,
 }
 
 bool AArch64AsmPrinter::PrintAsmOperand(const MachineInstr *MI, unsigned OpNum,
-                                        const char *ExtraCode, raw_ostream &O) {
+                                        const char *ExtraCode, raw_ostream &O,
+                                        std::string &ErrorMsg) {
   const MachineOperand &MO = MI->getOperand(OpNum);
 
   // First try the generic code, which knows about modifiers like 'c' and 'n'.
-  if (!AsmPrinter::PrintAsmOperand(MI, OpNum, ExtraCode, O))
+  if (!AsmPrinter::PrintAsmOperand(MI, OpNum, ExtraCode, O, ErrorMsg))
     return false;
 
   // Does this asm operand have a single letter operand modifier?
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp b/llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp
index 72e8b59e0a4096..8f174a2a32bbe9 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp
@@ -1235,9 +1235,10 @@ void AMDGPUAsmPrinter::getAmdKernelCode(amd_kernel_code_t &Out,
 }
 
 bool AMDGPUAsmPrinter::PrintAsmOperand(const MachineInstr *MI, unsigned OpNo,
-                                       const char *ExtraCode, raw_ostream &O) {
+                                       const char *ExtraCode, raw_ostream &O,
+                                       std::string &ErrorMsg) {
   // First try the generic code, which knows about modifiers like 'c' and 'n'.
-  if (!AsmPrinter::PrintAsmOperand(MI, OpNo, ExtraCode, O))
+  if (!AsmPrinter::PrintAsmOperand(MI, OpNo, ExtraCode, O, ErrorMsg))
     return false;
 
   if (ExtraCode && ExtraCode[0]) {
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.h b/llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.h
index 79326cd3d3289f..d9d3ec4f7d1cea 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.h
+++ b/llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.h
@@ -129,7 +129,8 @@ class AMDGPUAsmPrinter final : public AsmPrinter {
   void emitEndOfAsmFile(Module &M) override;
 
   bool PrintAsmOperand(const MachineInstr *MI, unsigned OpNo,
-                       const char *ExtraCode, raw_ostream &O) override;
+                       const char *ExtraCode, raw_ostream &O,
+                       std::string &ErrorMsg) override;
 
 protected:
   void getAnalysisUsage(AnalysisUsage &AU) const override;
diff --git a/llvm/lib/Target/ARM/ARMAsmPrinter.cpp b/llvm/lib/Target/ARM/ARMAsmPrinter.cpp
index 642739a29d6b06..51377adc7fc495 100644
--- a/llvm/lib/Target/ARM/ARMAsmPrinter.cpp
+++ b/llvm/lib/Target/ARM/ARMAsmPrinter.cpp
@@ -284,7 +284,8 @@ GetARMJTIPICJumpTableLabel(unsigned uid) const {
 }
 
 bool ARMAsmPrinter::PrintAsmOperand(const MachineInstr *MI, unsigned OpNum,
-                                    const char *ExtraCode, raw_ostream &O) {
+                                    const char *ExtraCode, raw_ostream &O,
+                                    std::string &ErrorMsg) {
   // Does this asm operand have a single letter operand modifier?
   if (ExtraCode && ExtraCode[0]) {
     if (ExtraCode[1] != 0) return true; // Unknown modifier.
@@ -292,7 +293,7 @@ bool ARMAsmPrinter::PrintAsmOperand(const MachineInstr *MI, unsigned OpNum,
     switch (ExtraCode[0]) {
     default:
       // See if this is a generic print operand
-      return AsmPrinter::PrintAsmOperand(MI, OpNum, ExtraCode, O);
+      return AsmPrinter::PrintAsmOperand(MI, OpNum, ExtraCode, O, ErrorMsg);
     case 'P': // Print a VFP double precision register.
     case 'q': // Print a NEON quad precision register.
       printOperand(MI, OpNum, O);
diff --git a/llvm/lib/Target/ARM/ARMAsmPrinter.h b/llvm/lib/Target/ARM/ARMAsmPrinter.h
index 33b4417aa9b809..bcb591703679ec 100644
--- a/llvm/lib/Target/ARM/ARMAsmPrinter.h
+++ b/llvm/lib/Target/ARM/ARMAsmPrinter.h
@@ -77,7 +77,8 @@ class LLVM_LIBRARY_VISIBILITY ARMAsmPrinter : public AsmPrinter {
 
   void PrintSymbolOperand(const MachineOperand &MO, raw_ostream &O) override;
   bool PrintAsmOperand(const MachineInstr *MI, unsigned OpNum,
-                       const char *ExtraCode, raw_ostream &O) override;
+                       const char *ExtraCode, raw_ostream &O,
+                       std::string &ErrorMsg) override;
   bool PrintAsmMemoryOperand(const MachineInstr *MI, unsigned OpNum,
                              const char *ExtraCode, raw_ostream &O) override;
 
diff --git a/llvm/lib/Target/AVR/AVRAsmPrinter.cpp b/llvm/lib/Target/AVR/AVRAsmPrinter.cpp
index 1c8213b668f71a..5a77f97f5eee64 100644
--- a/llvm/lib/Target/AVR/AVRAsmPrinter.cpp
+++ b/llvm/lib/Target/AVR/AVRAsmPrinter.cpp
@@ -52,7 +52,8 @@ class AVRAsmPrinter : public AsmPrinter {
   void printOperand(const MachineInstr *MI, unsigned OpNo, raw_ostream &O);
 
   bool PrintAsmOperand(const MachineInstr *MI, unsigned OpNum,
-                       const char *ExtraCode, raw_ostream &O) override;
+                       const char *ExtraCode, raw_ostream &O,
+                       std::string &ErrorMsg) override;
 
   bool PrintAsmMemoryOperand(const MachineInstr *MI, unsigned OpNum,
                              const char *ExtraCode, raw_ostream &O) override;
@@ -98,10 +99,11 @@ void AVRAsmPrinter::printOperand(const MachineInstr *MI, unsigned OpNo,
 }
 
 bool AVRAsmPrinter::PrintAsmOperand(const MachineInstr *MI, unsigned OpNum,
-                                    const char *ExtraCode, raw_ostream &O) {
+                                    const char *ExtraCode, raw_ostream &O,
+                                    std::string &ErrorMsg) {
   // Default asm printer can only deal with some extra codes,
   // so try it first.
-  if (!AsmPrinter::PrintAsmOperand(MI, OpNum, ExtraCode, O))
+  if (!AsmPrinter::PrintAsmOperand(MI, OpNum, ExtraCode, O, ErrorMsg))
     return false;
 
   const MachineOperand &MO = MI->getOperand(OpNum);
diff --git a/llvm/lib/Target/BPF/BPFAsmPrinter.cpp b/llvm/lib/Target/BPF/BPFAsmPrinter.cpp
index c8849bd50464c7..5420d8727a4e91 100644
--- a/llvm/lib/Target/BPF/BPFAsmPrinter.cpp
+++ b/llvm/lib/Target/BPF/BPFAsmPrinter.cpp
@@ -44,7 +44,8 @@ class BPFAsmPrinter : public AsmPrinter {
   bool doInitialization(Module &M) override;
   void printOperand(const MachineInstr *MI, int OpNum, raw_ostream &O);
   bool PrintAsmOperand(const MachineInstr *MI, unsigned OpNo,
-                       const char *ExtraCode, raw_ostream &O) override;
+                       const char *ExtraCode, raw_ostream &O, 
+                       std::string &ErrorMsg) override;
   bool PrintAsmMemoryOperand(const MachineInstr *MI, unsigned OpNum,
                              const char *ExtraCode, raw_ostream &O) override;
 
@@ -108,9 +109,10 @@ void BPFAsmPrinter::printOperand(const MachineInstr *MI, int OpNum,
 }
 
 bool BPFAsmPrinter::PrintAsmOperand(const MachineInstr *MI, unsigned OpNo,
-                                    const char *ExtraCode, raw_ostream &O) {
+                                    const char *ExtraCode, raw_ostream &O,
+                                    std::string &ErrorMsg) {
   if (ExtraCode && ExtraCode[0])
-    return AsmPrinter::PrintAsmOperand(MI, OpNo, ExtraCode, O);
+    return AsmPrinter::PrintAsmOperand(MI, OpNo, ExtraCode, O, ErrorMsg);
 
   printOperand(MI, OpNo, O);
   return false;
diff --git a/llvm/lib/Target/CSKY/CSKYAsmPrinter.cpp b/llvm/lib/Target/CSKY/CSKYAsmPrinter.cpp
index 7d121b8d24f059..85c4a0232617ae 100644
--- a/llvm/lib/Target/CSKY/CSKYAsmPrinter.cpp
+++ b/llvm/lib/Target/CSKY/CSKYAsmPrinter.cpp
@@ -261,9 +261,10 @@ void CSKYAsmPrinter::emitAttributes() {
 }
 
 bool CSKYAsmPrinter::PrintAsmOperand(const MachineInstr *MI, unsigned OpNo,
-                                     const char *ExtraCode, raw_ostream &OS) {
+                                     const char *ExtraCode, raw_ostream &OS,
+                                     std::string &ErrorMsg) {
   // First try the generic code, which knows about modifiers like 'c' and 'n'.
-  if (!AsmPrinter::PrintAsmOperand(MI, OpNo, ExtraCode, OS))
+  if (!AsmPrinter::PrintAsmOperand(MI, OpNo, ExtraCode, OS, ErrorMsg))
     return false;
 
   const MachineOperand &MO = MI->getOperand(OpNo);
diff --git a/llvm/lib/Target/Hexagon/HexagonAsmPrinter.cpp b/llvm/lib/Target/Hexagon/HexagonAsmPrinter.cpp
index d2f64ac9e90b0b..63797977f2ac77 100644
--- a/llvm/lib/Target/Hexagon/HexagonAsmPrinter.cpp
+++ b/llvm/lib/Target/Hexagon/HexagonAsmPrinter.cpp
@@ -114,7 +114,7 @@ bool HexagonAsmPrinter::isBlockOnlyReachableByFallthrough(
 /// PrintAsmOperand - Print out an operand for an inline asm expression.
 bool HexagonAsmPrinter::PrintAsmOperand(const MachineInstr *MI, unsigned OpNo,
                                         const char *ExtraCode,
-                                        raw_ostream &OS) {
+                                        raw_ostream &OS, std::string &ErrorMsg) {
   // Does this asm operand have a single letter operand modifier?
   if (ExtraCode && ExtraCode[0]) {
     if (ExtraCode[1] != 0)
@@ -123,7 +123,7 @@ bool HexagonAsmPrinter::PrintAsmOperand(const MachineInstr *MI, unsigned OpNo,
     switch (ExtraCode[0]) {
     default:
       // See if this is a generic print operand
-      return AsmPrinter::PrintAsmOperand(MI, OpNo, ExtraCode, OS);
+      return AsmPrinter::PrintAsmOperand(MI, OpNo, ExtraCode, OS, ErrorMsg);
     case 'L':
     case 'H': { // The highest-numbered register of a pair.
       const MachineOperand &MO = MI->getOperand(OpNo);
diff --git a/llvm/lib/Target/Hexagon/HexagonAsmPrinter.h b/llvm/lib/Target/Hexagon/HexagonAsmPrinter.h
index b555c885965036..71104a13aae9c2 100644
--- a/llvm/lib/Target/Hexagon/HexagonAsmPrinter.h
+++ b/llvm/lib/Target/Hexagon/HexagonAsmPrinter.h
@@ -67,7 +67,8 @@ class TargetMachine;
 
     void printOperand(const MachineInstr *MI, unsigned OpNo, raw_ostream &O);
     bool PrintAsmOperand(const MachineInstr *MI, unsigned OpNo,
-                         const char *ExtraCode, raw_ostream &OS) override;
+                         const char *ExtraCode, raw_ostream &OS,
+                         std::string &ErrorMsg) override;
     bool PrintAsmMemoryOperand(const MachineInstr *MI, unsigned OpNo,
                                const char *ExtraCode, raw_ostream &OS) override;
     void emitStartOfAsmFile(Module &M) override;
diff --git a/llvm/lib/Target/Lanai/LanaiAsmPrinter.cpp b/llvm/lib/Target/Lanai/LanaiAsmPrinter.cpp
index c66d9166828c10..7af0268a54f4c9 100644
--- a/llvm/lib/Target/Lanai/LanaiAsmPrinter.cpp
+++ b/llvm/lib/Target/Lanai/LanaiAsmPrinter.cpp
@@ -50,7 +50,8 @@ class LanaiAsmPrinter : public AsmPrinter {
 
   void printOperand(const MachineInstr *MI, int OpNum, raw_ostream &O);
   bool PrintAsmOperand(const MachineInstr *MI, unsigned OpNo,
-                       const char *ExtraCode, raw_ostream &O) override;
+                       const char *ExtraCode, raw_ostream &O,
+                       std::string &ErrorMsg) override;
   void emitInstruction(const MachineInstr *MI) override;
   bool isBlockOnlyReachableByFallthrough(
       const MachineBasicBlock *MBB) const override;
@@ -109,7 +110,8 @@ void LanaiAsmPrinter::printOperand(const MachineInstr *MI, int OpNum,
 
 // PrintAsmOperand - Print out an operand for an inline asm expression.
 bool LanaiAsmPrinter::PrintAsmOperand(const MachineInstr *MI, unsigned OpNo,
-                                      const char *ExtraCode, raw_ostream &O) {
+                                      const char *ExtraCode, raw_ostream &O,
+                                      std::string &ErrorMsg) {
   // Does this asm operand have a single letter operand modifier?
   if (ExtraCode && ExtraCode[0]) {
     if (ExtraCode[1])
@@ -138,7 +140,7 @@ bool LanaiAsmPrinter::PrintAsmOperand(const MachineInstr *MI, unsigned OpNo,
       return false;
     }
     default:
-      return AsmPrinter::PrintAsmOperand(MI, OpNo, ExtraCode, O);
+      return AsmPrinter::PrintAsmOperand(MI, OpNo, ExtraCode, O, ErrorMsg);
     }
   }
   printOperand(MI, OpNo, O);
diff --git a/llvm/lib/Target/LoongArch/LoongArchAsmPrinter.cpp b/llvm/lib/Target/LoongArch/LoongArchAsmPrinter.cpp
index 27979a830b10e6..843361dda61b0f 100644
--- a/llvm/lib/Target/LoongArch/LoongArchAsmPrinter.cpp
+++ b/llvm/lib/Target/LoongArch/LoongArchAsmPrinter.cpp
@@ -56,9 +56,10 @@ void LoongArchAsmPrinter::emitInstruction(const MachineInstr *MI) {
 
 bool LoongArchAsmPrinter::PrintAsmOperand(const MachineInstr *MI, unsigned OpNo,
                                           const char *ExtraCode,
-                                          raw_ostream &OS) {
+                                          raw_ostream &OS, 
+                                          std::string &ErrorMsg) {
   // First try the generic code, which knows about modifiers like 'c' and 'n'.
-  if (!AsmPrinter::PrintAsmOperand(MI, OpNo, ExtraCode, OS))
+  if (!AsmPrinter::PrintAsmOperand(MI, OpNo, ExtraCode, OS, ErrorMsg))
     return false;
 
   const MachineOperand &MO = MI->getOperand(OpNo);
diff --git a/llvm/lib/Target/LoongArch/LoongArchAsmPrinter.h b/llvm/lib/Target/LoongArch/LoongArchAsmPrinter.h
index 693456443c7a48..03f9febf8490eb 100644
--- a/llvm/lib/Target/LoongArch/LoongArchAsmPrinter.h
+++ b/llvm/lib/Target/LoongArch/LoongArchAsmPrinter.h
@@ -37,7 +37,8 @@ class LLVM_LIBRARY_VISIBILITY LoongArchAsmPrinter : public AsmPrinter {
   void emitInstruction(const MachineInstr *MI) override;
 
   bool PrintAsmOperand(const MachineInstr *MI, unsigned OpNo,
-                       const char *ExtraCode, raw_ostream &OS) override;
+                       const char *ExtraCode, raw_ostream &OS,
+                       std::string &ErrorMsg) override;
   bool PrintAsmMemoryOperand(const MachineInstr *MI, unsigned OpNo,
                              const char *ExtraCode, raw_ostream &OS) override;
 
diff --git a/llvm/lib/Target/M68k/M68kAsmPrinter.cpp b/llvm/lib/Target/M68k/M68kAsmPrinter.cpp
index f748450c170aa6..e8a5e8020b618b 100644
--- a/llvm/lib/Target/M68k/M68kAsmPrinter.cpp
+++ b/llvm/lib/Target/M68k/M68kAsmPrinter.cpp
@@ -65,7 +65,8 @@ void M68kAsmPrinter::printOperand(const MachineInstr *MI, int OpNum,
 }
 
 bool M68kAsmPrinter::PrintAsmOperand(const MachineInstr *MI, unsigned OpNo,
-                                     const char *ExtraCode, raw_ostream &OS) {
+                                     const char *ExtraCode, raw_ostream &OS,
+                                     std::string &ErrorMsg) {
   // Print the operand if there is no operand modifier.
   if (!ExtraCode || !ExtraCode[0]) {
     printOperand(MI, OpNo, OS);
@@ -73,7 +74,7 @@ bool M68kAsmPrinter::PrintAsmOperand(const MachineInstr *MI, unsigned OpNo,
   }
 
   // Fallback to the default implementation.
-  return AsmPrinter::PrintAsmOperand(MI, OpNo, ExtraCode, OS);
+  return AsmPrinter::PrintAsmOperand(MI, OpNo, ExtraCode, OS, ErrorMsg);
 }
 
 void M68kAsmPrinter::printDisp(const MachineInstr *MI, unsigned opNum,
diff --git a/llvm/lib/Target/MSP430/MSP430AsmPrinter.cpp b/llvm/lib/Target/MSP430/MSP430AsmPrinter.cpp
index 9cd2cbe89e461f..351b014ae4f2c5 100644
--- a/llvm/lib/Target/MSP430/MSP430AsmPrinter.cpp
+++ b/llvm/lib/Target/MSP430/MSP430AsmPrinter.cpp
@@ -54,7 +54,8 @@ namespace {
     void printSrcMemOperand(const MachineInstr *MI, int OpNum,
                             raw_ostream &O);
     bool PrintAsmOperand(const MachineInstr *MI, unsigned OpNo,
-                         const char *ExtraCode, raw_ostream &O) override;
+                         const char *ExtraCode, raw_ostream &O, 
+                         std::string &ErrorMsg) override;
     bool PrintAsmMemoryOperand(const MachineInstr *MI, unsigned OpNo,
                                const char *ExtraCode, raw_ostream &O) override;
     void emitInstruction(const MachineInstr *MI) override;
@@ -127,10 +128,11 @@ void MSP430AsmPrinter::printSrcMemOperand(const MachineInstr *MI, int OpNum,
 /// PrintAsmOperand - Print out an operand for an inline asm expression.
 ///
 bool MSP430AsmPrinter::PrintAsmOperand(const MachineInstr *MI, unsigned OpNo,
-                                       const char *ExtraCode, raw_ostream &O) {
+                                       const char *ExtraCode, raw_ostream &O,
+                                       std::string &ErrorMsg) {
   // Does this asm operand have a single letter operand modifier?
   if (ExtraCode && ExtraCode[0])
-    return AsmPrin...
[truncated]

Copy link

github-actions bot commented Mar 28, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@mahesh-attarde
Copy link
Contributor Author

CodeGen/SPIRV/pointers/struct-opaque-pointers.ll Test failure is unrelated here.

Copy link
Contributor

@arsenm arsenm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be a better API to migrate the print from return bool to Error instead of adding a string out argument

llvm/test/CodeGen/X86/inline-asm-modifier-error.ll Outdated Show resolved Hide resolved
llvm/test/CodeGen/X86/inline-asm-modifier-error.ll Outdated Show resolved Hide resolved
@mahesh-attarde
Copy link
Contributor Author

I think it would be a better API to migrate the print from return bool to Error instead of adding a string out argument

This is tricky. AsmPrinter::PrintAsmOperand is overridden in each target. 'H' can be slightly different for each target. So when we return to caller of PrintAsmOperand ( generalization) we may need separate special message as per Target. To support it, i added std::string parameter. Is that okay?

mahesh-attarde and others added 2 commits March 28, 2024 16:55
update testcase

Co-authored-by: Matt Arsenault <arsenm2@gmail.com>
update testcase

Co-authored-by: Matt Arsenault <arsenm2@gmail.com>
@arsenm
Copy link
Contributor

arsenm commented Mar 28, 2024

I think it would be a better API to migrate the print from return bool to Error instead of adding a string out argument

This is tricky. AsmPrinter::PrintAsmOperand is overridden in each target. 'H' can be slightly different for each target. So when we return to caller of PrintAsmOperand ( generalization) we may need separate special message as per Target. To support it, i added std::string parameter. Is that okay?

Error carries along with it any error string you want

mahesh-attarde referenced this pull request Apr 8, 2024
This adds support for using the L and H argument modifiers for twinword
operands in inline asm code, such as in:

```
%1 = tail call i64 asm sideeffect "rd %pc, ${0:L} ; srlx ${0:L}, 32, ${0:H}", "={o4}"()
```

This is needed by the Linux kernel.
@mahesh-attarde
Copy link
Contributor Author

#88248
working patch from other pull request. something went wrong with git files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants