Hi James, > I reverted this change with r258894, as it breaks (at least) sparc-rtems.
Can you send me a reproducer, please? Right now I'm at a dead end -- no tests are broken, and no buildbots are reported any fails. Yet you claim that my patch breaks one target -- and there is zero info how to verify this. > Clearly this area of the code was not sufficiently covered by the testsuite. Not sure I understand what you mean -- it's actually covered quite extensively in test/Preprocessor/init.c. There is a line for generic SPARC as well: // SPARC:#define __USER_LABEL_PREFIX__ _ and I added initialization of UserLabelPrefix to "_" in SparcTargetInfo constructor specifically to accommodate this test. Perhaps you might want to add tests for sparc-rterms target as well? Yours, Andrey On Wed, Jan 27, 2016 at 4:10 AM, James Y Knight <jykni...@google.com> wrote: > I reverted this change with r258894, as it breaks (at least) sparc-rtems. > Clearly this area of the code was not sufficiently covered by the testsuite. > > On Fri, Jan 22, 2016 at 10:24 AM, Andrey Bokhanko via cfe-commits > <cfe-commits@lists.llvm.org> wrote: >> >> Author: asbokhan >> Date: Fri Jan 22 09:24:34 2016 >> New Revision: 258504 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=258504&view=rev >> Log: >> Change of UserLabelPrefix default value from "_" to "" >> >> Differential Revision: http://reviews.llvm.org/D16295 >> >> Modified: >> cfe/trunk/lib/Basic/TargetInfo.cpp >> cfe/trunk/lib/Basic/Targets.cpp >> >> Modified: cfe/trunk/lib/Basic/TargetInfo.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/TargetInfo.cpp?rev=258504&r1=258503&r2=258504&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/Basic/TargetInfo.cpp (original) >> +++ cfe/trunk/lib/Basic/TargetInfo.cpp Fri Jan 22 09:24:34 2016 >> @@ -72,7 +72,7 @@ TargetInfo::TargetInfo(const llvm::Tripl >> DoubleFormat = &llvm::APFloat::IEEEdouble; >> LongDoubleFormat = &llvm::APFloat::IEEEdouble; >> DataLayoutString = nullptr; >> - UserLabelPrefix = "_"; >> + UserLabelPrefix = ""; >> MCountName = "mcount"; >> RegParmMax = 0; >> SSERegParmMax = 0; >> >> Modified: cfe/trunk/lib/Basic/Targets.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=258504&r1=258503&r2=258504&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/Basic/Targets.cpp (original) >> +++ cfe/trunk/lib/Basic/Targets.cpp Fri Jan 22 09:24:34 2016 >> @@ -102,9 +102,7 @@ protected: >> >> public: >> CloudABITargetInfo(const llvm::Triple &Triple) >> - : OSTargetInfo<Target>(Triple) { >> - this->UserLabelPrefix = ""; >> - } >> + : OSTargetInfo<Target>(Triple) {} >> }; >> >> static void getDarwinDefines(MacroBuilder &Builder, const LangOptions >> &Opts, >> @@ -242,6 +240,7 @@ public: >> this->TLSSupported = !Triple.isOSVersionLT(2); >> >> this->MCountName = "\01mcount"; >> + this->UserLabelPrefix = "_"; >> } >> >> std::string isValidSectionSpecifier(StringRef SR) const override { >> @@ -284,8 +283,6 @@ protected: >> public: >> DragonFlyBSDTargetInfo(const llvm::Triple &Triple) >> : OSTargetInfo<Target>(Triple) { >> - this->UserLabelPrefix = ""; >> - >> switch (Triple.getArch()) { >> default: >> case llvm::Triple::x86: >> @@ -327,8 +324,6 @@ protected: >> } >> public: >> FreeBSDTargetInfo(const llvm::Triple &Triple) : >> OSTargetInfo<Target>(Triple) { >> - this->UserLabelPrefix = ""; >> - >> switch (Triple.getArch()) { >> default: >> case llvm::Triple::x86: >> @@ -368,9 +363,7 @@ protected: >> } >> public: >> KFreeBSDTargetInfo(const llvm::Triple &Triple) >> - : OSTargetInfo<Target>(Triple) { >> - this->UserLabelPrefix = ""; >> - } >> + : OSTargetInfo<Target>(Triple) {} >> }; >> >> // Minix Target >> @@ -392,9 +385,7 @@ protected: >> DefineStd(Builder, "unix", Opts); >> } >> public: >> - MinixTargetInfo(const llvm::Triple &Triple) : >> OSTargetInfo<Target>(Triple) { >> - this->UserLabelPrefix = ""; >> - } >> + MinixTargetInfo(const llvm::Triple &Triple) : >> OSTargetInfo<Target>(Triple) {} >> }; >> >> // Linux target >> @@ -467,7 +458,6 @@ protected: >> } >> public: >> NetBSDTargetInfo(const llvm::Triple &Triple) : >> OSTargetInfo<Target>(Triple) { >> - this->UserLabelPrefix = ""; >> this->MCountName = "_mcount"; >> } >> }; >> @@ -488,7 +478,6 @@ protected: >> } >> public: >> OpenBSDTargetInfo(const llvm::Triple &Triple) : >> OSTargetInfo<Target>(Triple) { >> - this->UserLabelPrefix = ""; >> this->TLSSupported = false; >> >> switch (Triple.getArch()) { >> @@ -536,7 +525,6 @@ protected: >> } >> public: >> BitrigTargetInfo(const llvm::Triple &Triple) : >> OSTargetInfo<Target>(Triple) { >> - this->UserLabelPrefix = ""; >> this->MCountName = "__mcount"; >> } >> }; >> @@ -554,9 +542,7 @@ protected: >> Builder.defineMacro("__ELF__"); >> } >> public: >> - PSPTargetInfo(const llvm::Triple &Triple) : >> OSTargetInfo<Target>(Triple) { >> - this->UserLabelPrefix = ""; >> - } >> + PSPTargetInfo(const llvm::Triple &Triple) : >> OSTargetInfo<Target>(Triple) {} >> }; >> >> // PS3 PPU Target >> @@ -576,7 +562,6 @@ protected: >> } >> public: >> PS3PPUTargetInfo(const llvm::Triple &Triple) : >> OSTargetInfo<Target>(Triple) { >> - this->UserLabelPrefix = ""; >> this->LongWidth = this->LongAlign = 32; >> this->PointerWidth = this->PointerAlign = 32; >> this->IntMaxType = TargetInfo::SignedLongLong; >> @@ -604,7 +589,6 @@ public: >> >> // On PS4, TLS variable cannot be aligned to more than 32 bytes (256 >> bits). >> this->MaxTLSAlign = 256; >> - this->UserLabelPrefix = ""; >> >> switch (Triple.getArch()) { >> default: >> @@ -724,7 +708,6 @@ protected: >> >> public: >> NaClTargetInfo(const llvm::Triple &Triple) : >> OSTargetInfo<Target>(Triple) { >> - this->UserLabelPrefix = ""; >> this->LongAlign = 32; >> this->LongWidth = 32; >> this->PointerAlign = 32; >> @@ -778,7 +761,6 @@ public: >> explicit WebAssemblyOSTargetInfo(const llvm::Triple &Triple) >> : OSTargetInfo<Target>(Triple) { >> this->MCountName = "__mcount"; >> - this->UserLabelPrefix = ""; >> this->TheCXXABI.set(TargetCXXABI::WebAssembly); >> } >> }; >> @@ -816,6 +798,7 @@ public: >> SimdDefaultAlign = 128; >> LongDoubleWidth = LongDoubleAlign = 128; >> LongDoubleFormat = &llvm::APFloat::PPCDoubleDouble; >> + UserLabelPrefix = "_"; >> } >> >> /// \brief Flags for architecture specific defines. >> @@ -1631,6 +1614,7 @@ public: >> NoAsmVariants = true; >> // Set the default GPU to sm20 >> GPU = GK_SM20; >> + UserLabelPrefix = "_"; >> } >> void getTargetDefines(const LangOptions &Opts, >> MacroBuilder &Builder) const override { >> @@ -3671,6 +3655,8 @@ public: >> // FIXME: Check that we actually have cmpxchg8b before setting >> // MaxAtomicInlineWidth. (cmpxchg8b is an i586 instruction.) >> MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64; >> + >> + UserLabelPrefix = "_"; >> } >> BuiltinVaListKind getBuiltinVaListKind() const override { >> return TargetInfo::CharPtrBuiltinVaList; >> @@ -3882,7 +3868,6 @@ public: >> IntPtrType = SignedLong; >> PtrDiffType = SignedLong; >> ProcessIDType = SignedLong; >> - this->UserLabelPrefix = ""; >> this->TLSSupported = false; >> } >> void getTargetDefines(const LangOptions &Opts, >> @@ -3929,8 +3914,6 @@ protected: >> >> public: >> RTEMSTargetInfo(const llvm::Triple &Triple) : >> OSTargetInfo<Target>(Triple) { >> - this->UserLabelPrefix = ""; >> - >> switch (Triple.getArch()) { >> default: >> case llvm::Triple::x86: >> @@ -3957,7 +3940,6 @@ public: >> SizeType = UnsignedLong; >> IntPtrType = SignedLong; >> PtrDiffType = SignedLong; >> - this->UserLabelPrefix = ""; >> } >> void getTargetDefines(const LangOptions &Opts, >> MacroBuilder &Builder) const override { >> @@ -4005,6 +3987,8 @@ public: >> // x86-64 has atomics up to 16 bytes. >> MaxAtomicPromoteWidth = 128; >> MaxAtomicInlineWidth = 128; >> + >> + UserLabelPrefix = "_"; >> } >> BuiltinVaListKind getBuiltinVaListKind() const override { >> return TargetInfo::X86_64ABIBuiltinVaList; >> @@ -4060,7 +4044,6 @@ public: >> SizeType = UnsignedLongLong; >> PtrDiffType = SignedLongLong; >> IntPtrType = SignedLongLong; >> - this->UserLabelPrefix = ""; >> } >> >> void getTargetDefines(const LangOptions &Opts, >> @@ -4543,6 +4526,8 @@ public: >> // that follows it, `bar', `bar' will be aligned as the type of the >> // zero length bitfield. >> UseZeroLengthBitfieldAlignment = true; >> + >> + UserLabelPrefix = "_"; >> } >> >> StringRef getABI() const override { return ABI; } >> @@ -5120,7 +5105,6 @@ public: >> TLSSupported = false; >> WCharType = UnsignedShort; >> SizeType = UnsignedInt; >> - UserLabelPrefix = ""; >> } >> void getVisualStudioDefines(const LangOptions &Opts, >> MacroBuilder &Builder) const { >> @@ -5320,6 +5304,8 @@ public: >> >> // AArch64 targets default to using the ARM C++ ABI. >> TheCXXABI.set(TargetCXXABI::GenericAArch64); >> + >> + UserLabelPrefix = "_"; >> } >> >> StringRef getABI() const override { return ABI; } >> @@ -5844,7 +5830,9 @@ class SparcTargetInfo : public TargetInf >> bool SoftFloat; >> public: >> SparcTargetInfo(const llvm::Triple &Triple) >> - : TargetInfo(Triple), SoftFloat(false) {} >> + : TargetInfo(Triple), SoftFloat(false) { >> + UserLabelPrefix = "_"; >> + } >> >> bool handleTargetFeatures(std::vector<std::string> &Features, >> DiagnosticsEngine &Diags) override { >> @@ -6145,6 +6133,7 @@ public: >> MinGlobalAlign = 16; >> DataLayoutString = >> "E-m:e-i1:8:16-i8:8:16-i64:64-f128:64-a:8:16-n32:64"; >> MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64; >> + UserLabelPrefix = "_"; >> } >> void getTargetDefines(const LangOptions &Opts, >> MacroBuilder &Builder) const override { >> @@ -6304,6 +6293,7 @@ public: >> PtrDiffType = SignedInt; >> SigAtomicType = SignedLong; >> DataLayoutString = "e-m:e-p:16:16-i32:16:32-a:16-n8:16"; >> + UserLabelPrefix = "_"; >> } >> void getTargetDefines(const LangOptions &Opts, >> MacroBuilder &Builder) const override { >> @@ -6400,6 +6390,7 @@ public: >> "-f64:32-v64:32-v128:32-a:0:32-n32"; >> AddrSpaceMap = &TCEOpenCLAddrSpaceMap; >> UseAddrSpaceMapMangling = true; >> + UserLabelPrefix = "_"; >> } >> >> void getTargetDefines(const LangOptions &Opts, >> @@ -6502,6 +6493,7 @@ public: >> IsNan2008(false), IsSingleFloat(false), FloatABI(HardFloat), >> DspRev(NoDSP), HasMSA(false), HasFP64(false), ABI(ABIStr) { >> TheCXXABI.set(TargetCXXABI::GenericMIPS); >> + UserLabelPrefix = "_"; >> } >> >> bool isNaN2008Default() const { >> @@ -7078,7 +7070,6 @@ class PNaClTargetInfo : public TargetInf >> public: >> PNaClTargetInfo(const llvm::Triple &Triple) : TargetInfo(Triple) { >> BigEndian = false; >> - this->UserLabelPrefix = ""; >> this->LongAlign = 32; >> this->LongWidth = 32; >> this->PointerAlign = 32; >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > > _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
