[PATCH] D16295: Change of UserLabelPrefix default value from "_" to ""

[Rafael Espíndola via cfe-commits]

I am pretty sure the cases in init.c are wrong as the assembly itself
doesn't use a '_'.

Having said that, it is probably a good thing to do this in two steps.
So this patch LGTM on the condition that you also open a bug to audit
the cases where we define __USER_LABEL_PREFIX__ to _ in init.c and CC
whoever added those.

Thanks,
Rafael


On 19 January 2016 at 05:58, Andrey Bokhanko <andreybokhanko at gmail.com> wrote:
> andreybokhanko added a comment.
>
> @rafael, all these changes are driven by tests.
>
> It seems you mean OS targeting, which is handled in other TargetInfo classes (LinuxTargetInfo in Linux case).
>
>
> ================
> Comment at: lib/Basic/Targets.cpp:801
> @@ -818,2 +800,3 @@
>      LongDoubleFormat = &llvm::APFloat::PPCDoubleDouble;
> +    UserLabelPrefix = "_";
>    }
> ----------------
> rafael wrote:
>> This looks wrong, we produce a "f:" not an "_f:" when targeting powerpc-linux-gnu.
>>
> Is this commented out, tools/clang/test/Preprocessor/init.c:5216 fails. As can be seen in the test, PPC603E target expects UserLabelPrefix to be equal to "_" in freestanding mode.
>
> As for powerpc-linux-gnu target, UserLabelPrefix is set to "" at lib/Basic/Target.cpp:416 (LinuxTargetInfo constructor).
>
> ================
> Comment at: lib/Basic/Targets.cpp:1617
> @@ -1633,2 +1616,3 @@
>      GPU = GK_SM20;
> +    UserLabelPrefix = "_";
>    }
> ----------------
> rafael wrote:
>> This also looks wrong.
> Same as above -- NVPTX target expects UserLabelPrefix to be "_" in freestanding mode (tools/clang/test/Preprocessor/init.c:4853). Linux target is covered in LinuxTargetInfo constructor.
>
>
> http://reviews.llvm.org/D16295
>
>
>