From 8e641889eb63e95abc996dc3f2432f61fbbcb443 Mon Sep 17 00:00:00 2001 From: Toma Tabacu Date: Fri, 5 Jun 2015 11:48:54 +0000 Subject: [PATCH] [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 llvm-svn: 239144 --- include/llvm/MC/MCSubtargetInfo.h | 4 +++- lib/Target/Mips/AsmParser/MipsAsmParser.cpp | 26 ++++++++++++--------- test/MC/Mips/set-push-pop-directives-bad.s | 9 +++++++ test/MC/Mips/set-push-pop-directives.s | 17 ++++++++++++++ 4 files changed, 44 insertions(+), 12 deletions(-) diff --git a/include/llvm/MC/MCSubtargetInfo.h b/include/llvm/MC/MCSubtargetInfo.h index 1778a6d13fb..d25f5d92a8c 100644 --- a/include/llvm/MC/MCSubtargetInfo.h +++ b/include/llvm/MC/MCSubtargetInfo.h @@ -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. diff --git a/lib/Target/Mips/AsmParser/MipsAsmParser.cpp b/lib/Target/Mips/AsmParser/MipsAsmParser.cpp index 0d08138f8a9..98d413ca4fe 100644 --- a/lib/Target/Mips/AsmParser/MipsAsmParser.cpp +++ b/lib/Target/Mips/AsmParser/MipsAsmParser.cpp @@ -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 MCTargetAsmParser { 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(getAvailableFeatures())); + make_unique(STI.getFeatureBits())); // Create an assembler options environment for the user to modify. AssemblerOptions.push_back( - make_unique(getAvailableFeatures())); + make_unique(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::parseSetMips0Directive() { 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(); diff --git a/test/MC/Mips/set-push-pop-directives-bad.s b/test/MC/Mips/set-push-pop-directives-bad.s index 53d8b230815..8994eea1c8b 100644 --- a/test/MC/Mips/set-push-pop-directives-bad.s +++ b/test/MC/Mips/set-push-pop-directives-bad.s @@ -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 diff --git a/test/MC/Mips/set-push-pop-directives.s b/test/MC/Mips/set-push-pop-directives.s index 5f55b7c7e4d..3a0b2aecc58 100644 --- a/test/MC/Mips/set-push-pop-directives.s +++ b/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