treapster created this revision. treapster added a reviewer: rafaelauler. Herald added subscribers: jsji, ayermolo, pengfei, rupprecht, hiraditya, kristof.beyls. Herald added a reviewer: jhenderson. Herald added a reviewer: MaskRay. Herald added a reviewer: rafauler. Herald added a reviewer: Amir. Herald added a reviewer: maksfb. Herald added a project: All. treapster requested review of this revision. Herald added subscribers: llvm-commits, lldb-commits, yota9, StephenFan. Herald added projects: LLDB, LLVM.
It think it's reasonable to enable all supported extensions when we are disassembling something because we don't want to [hardcode](hhtps://reviews.llvm.org/D121999) it in gdb, objdump, bolt and whatever other places use disassembler in llvm. The only reason not to do it I can think of is if there are conflicting instructions with the same encoding in different extensions, but it doesn't seem possible within one archeticture. We probably should use +all on x86 and other platforms? I'm not sure how test for this should look like, because disassembler is used by at least three tools I know of(gdb, objdump, BOLT), so it's not clear where should we put it, and how to make sure that all new instructions are added to it. Waiting for your feedback. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D127741 Files: bolt/lib/Core/BinaryContext.cpp lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp llvm/lib/MC/MCSubtargetInfo.cpp llvm/tools/llvm-objdump/llvm-objdump.cpp Index: llvm/tools/llvm-objdump/llvm-objdump.cpp =================================================================== --- llvm/tools/llvm-objdump/llvm-objdump.cpp +++ llvm/tools/llvm-objdump/llvm-objdump.cpp @@ -1693,6 +1693,10 @@ if (!MAttrs.empty()) for (unsigned I = 0; I != MAttrs.size(); ++I) Features.AddFeature(MAttrs[I]); + else { + if (Obj->getArch() == llvm::Triple::aarch64) + Features.AddFeature("+all"); + } std::unique_ptr<const MCRegisterInfo> MRI( TheTarget->createMCRegInfo(TripleName)); Index: llvm/lib/MC/MCSubtargetInfo.cpp =================================================================== --- llvm/lib/MC/MCSubtargetInfo.cpp +++ llvm/lib/MC/MCSubtargetInfo.cpp @@ -198,6 +198,8 @@ Help(ProcDesc, ProcFeatures); else if (Feature == "+cpuhelp") cpuHelp(ProcDesc); + else if (Feature == "+all") + Bits.set(); else ApplyFeatureFlag(Bits, Feature, ProcFeatures); } Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp =================================================================== --- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp +++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp @@ -1179,16 +1179,9 @@ features_str += "+dspr2,"; } - // If any AArch64 variant, enable latest ISA with all extensions. + // If any AArch64 variant, enable all features. if (triple.isAArch64()) { - features_str += "+v9.3a,"; - std::vector<llvm::StringRef> features; - // Get all possible features - llvm::AArch64::getExtensionFeatures(-1, features); - features_str += llvm::join(features, ","); - - if (triple.getVendor() == llvm::Triple::Apple) - cpu = "apple-latest"; + features_str += "+all"; } if (triple.isRISCV()) { Index: bolt/lib/Core/BinaryContext.cpp =================================================================== --- bolt/lib/Core/BinaryContext.cpp +++ bolt/lib/Core/BinaryContext.cpp @@ -124,8 +124,7 @@ break; case llvm::Triple::aarch64: ArchName = "aarch64"; - FeaturesStr = "+fp-armv8,+neon,+crypto,+dotprod,+crc,+lse,+ras,+rdm," - "+fullfp16,+spe,+fuse-aes,+rcpc"; + FeaturesStr = "+all"; break; default: return createStringError(std::errc::not_supported,
Index: llvm/tools/llvm-objdump/llvm-objdump.cpp =================================================================== --- llvm/tools/llvm-objdump/llvm-objdump.cpp +++ llvm/tools/llvm-objdump/llvm-objdump.cpp @@ -1693,6 +1693,10 @@ if (!MAttrs.empty()) for (unsigned I = 0; I != MAttrs.size(); ++I) Features.AddFeature(MAttrs[I]); + else { + if (Obj->getArch() == llvm::Triple::aarch64) + Features.AddFeature("+all"); + } std::unique_ptr<const MCRegisterInfo> MRI( TheTarget->createMCRegInfo(TripleName)); Index: llvm/lib/MC/MCSubtargetInfo.cpp =================================================================== --- llvm/lib/MC/MCSubtargetInfo.cpp +++ llvm/lib/MC/MCSubtargetInfo.cpp @@ -198,6 +198,8 @@ Help(ProcDesc, ProcFeatures); else if (Feature == "+cpuhelp") cpuHelp(ProcDesc); + else if (Feature == "+all") + Bits.set(); else ApplyFeatureFlag(Bits, Feature, ProcFeatures); } Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp =================================================================== --- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp +++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp @@ -1179,16 +1179,9 @@ features_str += "+dspr2,"; } - // If any AArch64 variant, enable latest ISA with all extensions. + // If any AArch64 variant, enable all features. if (triple.isAArch64()) { - features_str += "+v9.3a,"; - std::vector<llvm::StringRef> features; - // Get all possible features - llvm::AArch64::getExtensionFeatures(-1, features); - features_str += llvm::join(features, ","); - - if (triple.getVendor() == llvm::Triple::Apple) - cpu = "apple-latest"; + features_str += "+all"; } if (triple.isRISCV()) { Index: bolt/lib/Core/BinaryContext.cpp =================================================================== --- bolt/lib/Core/BinaryContext.cpp +++ bolt/lib/Core/BinaryContext.cpp @@ -124,8 +124,7 @@ break; case llvm::Triple::aarch64: ArchName = "aarch64"; - FeaturesStr = "+fp-armv8,+neon,+crypto,+dotprod,+crc,+lse,+ras,+rdm," - "+fullfp16,+spe,+fuse-aes,+rcpc"; + FeaturesStr = "+all"; break; default: return createStringError(std::errc::not_supported,
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits