[llvm] r239144 - [mips] [IAS] Restore STI.FeatureBits in .set pop.

Toma Tabacu toma.tabacu at imgtec.com
Fri Jun 5 04:48:55 PDT 2015


Author: tomatabacu
Date: Fri Jun  5 06:48:54 2015
New Revision: 239144

URL: http://llvm.org/viewvc/llvm-project?rev=239144&view=rev
Log:
[mips] [IAS] Restore STI.FeatureBits in .set pop.

Summary:
Only restoring AvailableFeatures is not enough and will lead to buggy behaviour.
For example, if we have a feature enabled and we ".set pop", the next time we try
to ".set" that feature nothing will happen because the "!(STI.getFeatureBits()[Feature])"
check will be false, because we didn't restore STI.FeatureBits.

In order to fix this, we need to make MipsAssemblerOptions remember the STI.FeatureBits
instead of the AvailableFeatures and then regenerate AvailableFeatures each time we ".set pop".
This is because, AFAIK, there is no way to convert from AvailableFeatures back to STI.FeatureBits,
but the reverse is possible by using ComputeAvailableFeatures(STI.FeatureBits).

I also moved the updating of AssemblerOptions inside the "if" statement in
setFeatureBits() and clearFeatureBits(), as there is no reason to update if
nothing changes.

Reviewers: dsanders, mkuper

Reviewed By: dsanders

Subscribers: llvm-commits

Differential Revision: http://reviews.llvm.org/D9156

Modified:
    llvm/trunk/include/llvm/MC/MCSubtargetInfo.h
    llvm/trunk/lib/Target/Mips/AsmParser/MipsAsmParser.cpp
    llvm/trunk/test/MC/Mips/set-push-pop-directives-bad.s
    llvm/trunk/test/MC/Mips/set-push-pop-directives.s

Modified: llvm/trunk/include/llvm/MC/MCSubtargetInfo.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCSubtargetInfo.h?rev=239144&r1=239143&r2=239144&view=diff
==============================================================================
--- llvm/trunk/include/llvm/MC/MCSubtargetInfo.h (original)
+++ llvm/trunk/include/llvm/MC/MCSubtargetInfo.h Fri Jun  5 06:48:54 2015
@@ -73,7 +73,9 @@ public:
 
   /// setFeatureBits - Set the feature bits.
   ///
-  void setFeatureBits(FeatureBitset& FeatureBits_) { FeatureBits = FeatureBits_; }
+  void setFeatureBits(const FeatureBitset &FeatureBits_) {
+    FeatureBits = FeatureBits_;
+  }
 
   /// InitMCProcessorInfo - Set or change the CPU (optionally supplemented with
   /// feature string). Recompute feature bits and scheduling model.

Modified: llvm/trunk/lib/Target/Mips/AsmParser/MipsAsmParser.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mips/AsmParser/MipsAsmParser.cpp?rev=239144&r1=239143&r2=239144&view=diff
==============================================================================
--- llvm/trunk/lib/Target/Mips/AsmParser/MipsAsmParser.cpp (original)
+++ llvm/trunk/lib/Target/Mips/AsmParser/MipsAsmParser.cpp Fri Jun  5 06:48:54 2015
@@ -43,7 +43,7 @@ class MCInstrInfo;
 namespace {
 class MipsAssemblerOptions {
 public:
-  MipsAssemblerOptions(uint64_t Features_) : 
+  MipsAssemblerOptions(const FeatureBitset &Features_) :
     ATReg(1), Reorder(true), Macro(true), Features(Features_) {}
 
   MipsAssemblerOptions(const MipsAssemblerOptions *Opts) {
@@ -70,8 +70,8 @@ public:
   void setMacro() { Macro = true; }
   void setNoMacro() { Macro = false; }
 
-  uint64_t getFeatures() const { return Features; }
-  void setFeatures(uint64_t Features_) { Features = Features_; }
+  const FeatureBitset &getFeatures() const { return Features; }
+  void setFeatures(const FeatureBitset &Features_) { Features = Features_; }
 
   // Set of features that are either architecture features or referenced
   // by them (e.g.: FeatureNaN2008 implied by FeatureMips32r6).
@@ -84,7 +84,7 @@ private:
   unsigned ATReg;
   bool Reorder;
   bool Macro;
-  uint64_t Features;
+  FeatureBitset Features;
 };
 }
 
@@ -327,23 +327,23 @@ class MipsAsmParser : public MCTargetAsm
     STI.setFeatureBits(FeatureBits);
     setAvailableFeatures(
         ComputeAvailableFeatures(STI.ToggleFeature(ArchFeature)));
-    AssemblerOptions.back()->setFeatures(getAvailableFeatures());
+    AssemblerOptions.back()->setFeatures(STI.getFeatureBits());
   }
 
   void setFeatureBits(uint64_t Feature, StringRef FeatureString) {
     if (!(STI.getFeatureBits()[Feature])) {
       setAvailableFeatures(
           ComputeAvailableFeatures(STI.ToggleFeature(FeatureString)));
+      AssemblerOptions.back()->setFeatures(STI.getFeatureBits());
     }
-    AssemblerOptions.back()->setFeatures(getAvailableFeatures());
   }
 
   void clearFeatureBits(uint64_t Feature, StringRef FeatureString) {
     if (STI.getFeatureBits()[Feature]) {
       setAvailableFeatures(
           ComputeAvailableFeatures(STI.ToggleFeature(FeatureString)));
+      AssemblerOptions.back()->setFeatures(STI.getFeatureBits());
     }
-    AssemblerOptions.back()->setFeatures(getAvailableFeatures());
   }
 
 public:
@@ -369,11 +369,11 @@ public:
     
     // Remember the initial assembler options. The user can not modify these.
     AssemblerOptions.push_back(
-                     make_unique<MipsAssemblerOptions>(getAvailableFeatures()));
+                     make_unique<MipsAssemblerOptions>(STI.getFeatureBits()));
     
     // Create an assembler options environment for the user to modify.
     AssemblerOptions.push_back(
-                     make_unique<MipsAssemblerOptions>(getAvailableFeatures()));
+                     make_unique<MipsAssemblerOptions>(STI.getFeatureBits()));
 
     getTargetStreamer().updateABIInfo(*this);
 
@@ -3603,7 +3603,9 @@ bool MipsAsmParser::parseSetPopDirective
     return reportParseError(Loc, ".set pop with no .set push");
 
   AssemblerOptions.pop_back();
-  setAvailableFeatures(AssemblerOptions.back()->getFeatures());
+  setAvailableFeatures(
+      ComputeAvailableFeatures(AssemblerOptions.back()->getFeatures()));
+  STI.setFeatureBits(AssemblerOptions.back()->getFeatures());
 
   getTargetStreamer().emitDirectiveSetPop();
   return false;
@@ -3673,7 +3675,9 @@ bool MipsAsmParser::parseSetMips0Directi
     return reportParseError("unexpected token, expected end of statement");
 
   // Reset assembler options to their initial values.
-  setAvailableFeatures(AssemblerOptions.front()->getFeatures());
+  setAvailableFeatures(
+      ComputeAvailableFeatures(AssemblerOptions.front()->getFeatures()));
+  STI.setFeatureBits(AssemblerOptions.front()->getFeatures());
   AssemblerOptions.back()->setFeatures(AssemblerOptions.front()->getFeatures());
 
   getTargetStreamer().emitDirectiveSetMips0();

Modified: llvm/trunk/test/MC/Mips/set-push-pop-directives-bad.s
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/Mips/set-push-pop-directives-bad.s?rev=239144&r1=239143&r2=239144&view=diff
==============================================================================
--- llvm/trunk/test/MC/Mips/set-push-pop-directives-bad.s (original)
+++ llvm/trunk/test/MC/Mips/set-push-pop-directives-bad.s Fri Jun  5 06:48:54 2015
@@ -12,3 +12,12 @@
 # CHECK: :[[@LINE-1]]:19: error: unexpected token, expected end of statement
         .set pop bar
 # CHECK: :[[@LINE-1]]:18: error: unexpected token, expected end of statement
+
+        .set hardfloat
+        .set push
+        .set softfloat
+        add.s $f2, $f2, $f2
+# CHECK: :[[@LINE-1]]:9: error: instruction requires a CPU feature not currently enabled
+        .set pop
+        add.s $f2, $f2, $f2
+# CHECK-NOT: :[[@LINE-1]]:9: error: instruction requires a CPU feature not currently enabled

Modified: llvm/trunk/test/MC/Mips/set-push-pop-directives.s
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/Mips/set-push-pop-directives.s?rev=239144&r1=239143&r2=239144&view=diff
==============================================================================
--- llvm/trunk/test/MC/Mips/set-push-pop-directives.s (original)
+++ llvm/trunk/test/MC/Mips/set-push-pop-directives.s Fri Jun  5 06:48:54 2015
@@ -51,3 +51,20 @@
 # CHECK:  b        1336
 # CHECK:  nop
 # CHECK:  addvi.b  $w15, $w13, 18
+
+    .set push
+    .set dsp
+    lbux    $7, $10($11)
+    .set pop
+
+    .set push
+    .set dsp
+    lbux    $7, $10($11)
+# CHECK-NOT: :[[@LINE-1]]:5: error: instruction requires a CPU feature not currently enabled
+    .set pop
+
+    .set push
+    .set dsp
+    lbux    $7, $10($11)
+# CHECK-NOT: :[[@LINE-1]]:5: error: instruction requires a CPU feature not currently enabled
+    .set pop





More information about the llvm-commits mailing list