r258504 - Change of UserLabelPrefix default value from "_" to ""
James Y Knight via cfe-commits
cfe-commits at lists.llvm.org
Wed Jan 27 05:42:48 PST 2016
Imagine there's a 2d table of values for UserLabelPrefix, each row for CPU
and column for OS. The value of many if those cells was changed by this
commit, because you stopped painting columns as "".
That is, originally, the default entry was "_", then cpu rows were filled,
and then the os columns were painted on top of that.
Now, the Sparc row gets painted "_", but the rtems column isn't painted on
top anymore. So, Sparc+rtems gets the wrong value.
As to test cases, yes, I agree, I will add some more preprocessor tests for
other OS/CPU combinations.
On Jan 27, 2016 3:12 AM, "Andrey Bokhanko" <andreybokhanko at gmail.com> wrote:
> 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 <jyknight at 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 at 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 at lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160127/ed748fa5/attachment-0001.html>
More information about the cfe-commits
mailing list