[PATCH] [mips] [IAS] Restore STI.FeatureBits in .set pop.

[Toma Tabacu]

Hi dsanders,

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 restore STI.FeatureBits, 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 statemement in setFeatureBits() and clearFeatureBits(), as there is no reason to update if nothing changes.

http://reviews.llvm.org/D9156

Files:
  lib/Target/Mips/AsmParser/MipsAsmParser.cpp
  test/MC/Mips/set-push-pop-directives.s

Index: lib/Target/Mips/AsmParser/MipsAsmParser.cpp
===================================================================
--- lib/Target/Mips/AsmParser/MipsAsmParser.cpp
+++ lib/Target/Mips/AsmParser/MipsAsmParser.cpp
@@ -314,23 +314,23 @@
     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:
@@ -356,11 +356,11 @@
     
     // 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);
 
@@ -3552,7 +3552,9 @@
     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;
@@ -3600,7 +3602,9 @@
     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();
Index: test/MC/Mips/set-push-pop-directives.s
===================================================================
--- test/MC/Mips/set-push-pop-directives.s
+++ test/MC/Mips/set-push-pop-directives.s
@@ -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

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D9156.24139.patch
Type: text/x-patch
Size: 3495 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150421/8b623583/attachment.bin>