[PATCH] Remove const_cast for STI when parsing inline asm

David Peixotto dpeixott at codeaurora.org
Wed Feb 5 15:25:33 PST 2014


Hi garious, rengolin, grosbach,

In a previous commit (r199818) we added a const_cast to an existing
subtarget info instead of creating a new one so that we could reuse
it when creating the TargetAsmParser for parsing inline assembly.
This cast was necessary because we needed to reuse the existing STI
to avoid generating incorrect code when the inline asm contained
mode-switching directives (e.g. .code 16).

The root cause of the failure was that there was an implicit sharing
of the STI between the parser and the MCCodeEmitter. To fix a
different but related issue, we now explicitly pass the STI to the
MCCodeEmitter (see commits r200345-r200351).

The const_cast is no longer necessary and we can now create a fresh
STI for the inline asm parser to use.

http://llvm-reviews.chandlerc.com/D2709

Files:
  include/llvm/CodeGen/AsmPrinter.h
  lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
  lib/Target/ARM/ARMAsmPrinter.cpp
  lib/Target/ARM/ARMAsmPrinter.h

Index: include/llvm/CodeGen/AsmPrinter.h
===================================================================
--- include/llvm/CodeGen/AsmPrinter.h
+++ include/llvm/CodeGen/AsmPrinter.h
@@ -475,7 +475,7 @@
     /// \p EndInfo   - the final subtarget info after parsing the inline asm,
     ///                or NULL if the value is unknown.
     virtual void emitInlineAsmEnd(const MCSubtargetInfo &StartInfo,
-                                  MCSubtargetInfo *EndInfo) const;
+                                  const MCSubtargetInfo *EndInfo) const;
 
   private:
     /// Private state for PrintSpecial()
Index: lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
===================================================================
--- lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
+++ lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
@@ -117,14 +117,15 @@
                                                   OutContext, OutStreamer,
                                                   *MAI));
 
-  // Reuse the existing Subtarget, because the AsmParser may need to
-  // modify it.  For example, when switching between ARM and
-  // Thumb mode.
-  MCSubtargetInfo* STI =
-    const_cast<MCSubtargetInfo*>(&TM.getSubtarget<MCSubtargetInfo>());
-
-  // Preserve a copy of the original STI because the parser may modify it.
-  // The target can restore the original state in emitInlineAsmEnd().
+  // Initialize the parser with a fresh subtarget info. We do not need to reuse
+  // the existing STI in the TM because any sharing of the STI with backend
+  // code emitters is now explicit.
+  OwningPtr<MCSubtargetInfo> STI(TM.getTarget().createMCSubtargetInfo(
+      TM.getTargetTriple(), TM.getTargetCPU(), TM.getTargetFeatureString()));
+
+  // Preserve a copy of the original STI because the parser may modify it.  For
+  // example, when switching between arm and thumb mode. The target can emit
+  // code to restore the original state in emitInlineAsmEnd().
   MCSubtargetInfo STIOrig = *STI;
 
   OwningPtr<MCTargetAsmParser>
@@ -138,7 +139,7 @@
   // Don't implicitly switch to the text section before the asm.
   int Res = Parser->Run(/*NoInitialTextSection*/ true,
                         /*NoFinalize*/ true);
-  emitInlineAsmEnd(STIOrig, STI);
+  emitInlineAsmEnd(STIOrig, STI.get());
   if (Res && !HasDiagHandler)
     report_fatal_error("Error parsing inline asm\n");
 }
@@ -550,4 +551,4 @@
 }
 
 void AsmPrinter::emitInlineAsmEnd(const MCSubtargetInfo &StartInfo,
-                                  MCSubtargetInfo *EndInfo) const {}
+                                  const MCSubtargetInfo *EndInfo) const {}
Index: lib/Target/ARM/ARMAsmPrinter.cpp
===================================================================
--- lib/Target/ARM/ARMAsmPrinter.cpp
+++ lib/Target/ARM/ARMAsmPrinter.cpp
@@ -443,14 +443,12 @@
 }
 
 void ARMAsmPrinter::emitInlineAsmEnd(const MCSubtargetInfo &StartInfo,
-                                     MCSubtargetInfo *EndInfo) const {
+                                     const MCSubtargetInfo *EndInfo) const {
   // If either end mode is unknown (EndInfo == NULL) or different than
   // the start mode, then restore the start mode.
   const bool WasThumb = isThumb(StartInfo);
   if (EndInfo == NULL || WasThumb != isThumb(*EndInfo)) {
     OutStreamer.EmitAssemblerFlag(WasThumb ? MCAF_Code16 : MCAF_Code32);
-    if (EndInfo)
-      EndInfo->ToggleFeature(ARM::ModeThumb);
   }
 }
 
Index: lib/Target/ARM/ARMAsmPrinter.h
===================================================================
--- lib/Target/ARM/ARMAsmPrinter.h
+++ lib/Target/ARM/ARMAsmPrinter.h
@@ -64,7 +64,8 @@
                                      raw_ostream &O) LLVM_OVERRIDE;
 
   virtual void emitInlineAsmEnd(const MCSubtargetInfo &StartInfo,
-                                MCSubtargetInfo *EndInfo) const LLVM_OVERRIDE;
+                                const MCSubtargetInfo *EndInfo) const
+      LLVM_OVERRIDE;
 
   void EmitJumpTable(const MachineInstr *MI);
   void EmitJump2Table(const MachineInstr *MI);
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D2709.1.patch
Type: text/x-patch
Size: 4031 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140205/5904968c/attachment.bin>


More information about the llvm-commits mailing list