[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

vit9696 added a comment. Hello, Thank you for working this. I tried the change and have a couple of suggestions: 1. -mspe option in GCC works like -mspe=yes or -mspe=no. While it does make sense to have it the way you did (-mno-spe and -mspe) it would be nice to have at least have an alias for

[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

vit9696 added a comment. @jhibbits sorry for not answering earlier, I was occupied with NY holidays, and then had a lot of stuff on the road. Trying to sync to your latest changes, I merged the updated https://reviews.llvm.org/D54583 in my local copy with your fixes to libcall expansion. From

[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

vit9696 added a comment. Actually I am not sure about Linux, since this is bare metal, and I just used what fited. However, it does not look like 128-bit or 64-bit long doubles are related. I retried to compile the test case with powerpc-gnu-freebsd target (and even made a compile-time assertio

[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

vit9696 added a comment. You are right, had to modify it like this to get the crash with FreeBSD triple: void f1(long double v, void *a) { } void f2(void* v, __builtin_va_list arg) { f1(__builtin_va_arg(arg, long double), 0); } Repository: rC Clang CHANGES SINCE LAST ACTIO

[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

vit9696 added a comment. Justin, I am currently testing the latest patches, yet so far it looks very good. Thanks. I rechecked GCC, and it seems that it forces 64-bit long double for SPE regardless of the target. Could you please force that in LLVM as well? While imperfect, I currently changed

[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

vit9696 added a comment. @jhibbits it appears that va_list is not functional with SPE (va_arg returns garbage for double and stuff like printf is not functional). Is this expected or I miss a patch? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D49754/new/ https

[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

vit9696 added a comment. Right, I noticed the same thing yesterday. There is an override calling LowerVAARG for 64-bit integers, yet that is not a thing that does lowering for all the rest. I believe it is derived somewhere from td calling conventions. I will check it out later this evening and

[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

vit9696 added a comment. Thanks for pointing it out. You could use hasFeature from there during construction: return SetCGInfo( new PPC32TargetCodeGenInfo(Types, CodeGenOpts.FloatABI == "soft" || getTarget().hasFeature("spe"))); It works for me, but probably a separate argument is

[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

vit9696 added a subscriber: hfinkel. vit9696 added a comment. That's pretty good. Do you think it is possible to land it in 8.0 release? @hfinkel? @jhibbits could you please include this change too: https://reviews.llvm.org/D49754#1364865? We would prefer to continue using Linux target to be a

[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

vit9696 added a comment. Thanks for the fix. I made a quick check of the mentioned patch, and it looks like it does solve the issue. However, besides the previous crash, which remains unfixed as of 7.0.1rc2, there is another instruction selection failure crash that may be caused by the change.

[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

vit9696 added a comment. In D49754#1321078 , @glaubitz wrote: > I have applied this patch to the llvm-toolchain-7 package in Debian and did > not see any regressions on x86_64 or 32-Bit PowerPC. Additionally, I have > included the patches from https://re

[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

vit9696 added a comment. @jhibbits, @kthomsen, it appears that current patchset has issues when handling && on me. I have it applied over llvm 8.0.0 rc2, and the following code returns 0 to me with -O2 and below: #include #define FEQUAL(x,y) (((x) - (y)) < 0.01) // could put fabs if

[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

vit9696 added a comment. This is a series of patches, which I believe should merged altogether. Currently the following patches are relevant: - https://reviews.llvm.org/D49754 - https://reviews.llvm.org/D54409 - https://reviews.llvm.org/D54583 - https://reviews.llvm.org/D56703 The patches are i

[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

vit9696 added a comment. @nemanjai, sorry, under merging I meant committing into llvm upstream. Comment at: lib/Basic/Targets/PPC.cpp:318 + Features["spe"] = llvm::StringSwitch(CPU) +.Case("e500", true) +.Case("8548", true) -

[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

vit9696 added a comment. Right, ok. I have been testing this for quite some time now, including maths, and so far had no issues. Can this get merged into 9.0? I do not believe there are enough obstacles to postpone it any further. Thanks! Repository: rC Clang CHANGES SINCE LAST ACTION htt

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

vit9696 added a comment. Could somebody commit it into 9.0? Quite a lot of people depend on this option including us. Since it got accepted I do not see a reason for this to get postponed any further. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D28462/new/ https://reviews.llv

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

vit9696 added a comment. @ruiu, we have some hardware, which bootloader does not support PT_GNU_STACK segments, and would therefore benefit quite a bit from this option. As this does not really depend on NetBSD support, could you please merge this patch in time for 9.0? Thanks! CHANGES SINCE

[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

vit9696 added a comment. Not sure whether I understood you, do not you already have the common switch by feature, named spe, in Features["spe"] line for that? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D49754/new/ https://reviews.llvm.org/D49754 ___

[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

vit9696 added a comment. Ok, I found the fix for the first crash that landed in 8.0 trunk. It works fine for me if backported to 7.0.1: https://reviews.llvm.org/D50461 Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D49754/new/ https://reviews.llvm.org/D49754 __

[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

vit9696 added a comment. @jhibbits, thank you for merging. Will we have this in LLVM 9.0? Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D49754/new/ https://reviews.llvm.org/D49754 ___ cfe-commits mailing list cfe-commi

[PATCH] D44927: Enable msan unconditionally on Linux

vit9696 created this revision. vit9696 added a reviewer: eugenis. Herald added a subscriber: cfe-commits. Memeory sanitizer compatibility are already done in MemorySanitizer::doInitialization. It verifies whether the necessary offsets exist and bails out if not. For this reason it is no good to

[PATCH] D44927: Enable msan unconditionally on Linux

vit9696 added a comment. Now that https://reviews.llvm.org/D44926 was merged upstream, may I request this to be merged as well? As stated above it is pretty much pointless to have the check doubled, and after more than a week nobody thought of any issue. Thanks :) Repository: rC Clang https