Re: [PATCH] D13978: [X86] Support MCU psABI when marking inregs
DavidKreitzer added inline comments. Comment at: lib/CodeGen/TargetInfo.cpp:1239 @@ +1238,3 @@ +// The MCU psABI allows passing parameters in-reg even if there are +// earlier, parameters that are passed on the stack. Also, +// it does not allow passing >8-byte structs in-register, Minor nit - you don't need the comma after "earlier". Comment at: test/CodeGen/x86_32-arguments-iamcu.c:1 @@ +1,2 @@ +// RUN: %clang_cc1 -w -triple i386-pc-elfiamcu -mfloat-abi soft -emit-llvm -o - %s | FileCheck %s + Good test! I think it would be a good idea to add a varargs function & verify that the args do not get marked inreg. http://reviews.llvm.org/D13978 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22045: [X86] Support of no_caller_saved_registers attribute (Clang part)
DavidKreitzer added a comment. The example Aaron sent in email is a good one: void func(int, int, int, int) __attribute__((no_caller_saved_registers, cdecl)); int main() { void (*fp)(int, int, int, int) __attribute__((cdecl)) = func; func(1, 2, 3, 4); fp(1, 2, 3, 4); // Not the same as the above call to func()? } Clang should at least be giving a warning for this just like it does if you try to mix cdecl and stdcall on IA-32 or if you mix regparm(2) and regparm(3). The current patch silently accepts the mismatch. https://reviews.llvm.org/D22045 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16808: [MCU] PR26438: Fix assertion failure on function returning an empty struct or union
DavidKreitzer added a comment. Denis, can you please explain your rationale for choosing to return 0-sized aggregates in memory for MCU? It doesn't match gcc behavior. For example, int g; union U {} u; union U f(int a, int b, int c) { g = a + b + c; return u; } void f1() { f(1, 2, 3); } Produces this, where all three incoming arguments are in registers. With your patch, %eax will be used for the return value pointer. f: addl%eax, %edx subl$4, %esp addl%edx, %ecx movl%ecx, g popl%eax ret (Using i586-intel-elfiamcu-gcc 5.2.1 20150820) HJ, even though a strict reading of the ABI suggests that a 0-size aggregate should be returned in register, it would be a good idea to make this more explicit in the ABI here, by adding the underlined text, for example. > **Returning Values** > Table 2.4 lists the location used to return a value for each fundamental > data type. > Aggregate types (structs and unions) are returned as follows: > • Short aggregate types no larger than 8 bytes, __including 0-length > aggregate types,__ are returned in %edx:%eax. > The most significant 32 bits are returned in %edx. The least significant 32 > bits are returned in %eax. > • Other aggregate types are returned in memory. http://reviews.llvm.org/D16808 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16808: [MCU] PR26438: Fix assertion failure on function returning an empty struct or union
DavidKreitzer added a comment. From an MCU ABI perspective, yes, returning an empty struct is equivalent to returning void. http://reviews.llvm.org/D16808 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20468: [X86][AVX] Ensure zero-extension of _mm256_extract_epi8 and _mm256_extract_epi16
DavidKreitzer added a subscriber: DavidKreitzer. DavidKreitzer added a comment. Hi Michael, I think the Intel Intrinsics reference and the Intel Compiler are in error and that this is the right fix for the LLVM headers. (I'll follow up to get the Intel Intrinsics reference & Intel Compiler fixed.) The _mm256_extract_epiN "convenience" intrinsics were first introduced by gcc and used an "int" return type. They were added to the Intel Compiler about 2 years ago, but for some reason were defined to use the smaller signed types. I'm double checking with the developer that added them, but I think it was just a mistake. -Dave Repository: rL LLVM http://reviews.llvm.org/D20468 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14864: [X86] Support for C calling convention only for MCU target.
DavidKreitzer added a comment. Hi Alexey, Did you consider adding a new class for the MCU target info, e.g. X86MCUTargetInfo? The more MCU-specific stuff that gets added here, the more it makes sense to pull it out into its own class. We already have line 3398-3401 & 3650-3653. We are also going to need to change the data layout string at 3644 for MCU. (Anton Nadolski is working on that.) - Dave http://reviews.llvm.org/D14864 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14864: [X86] Support for C calling convention only for MCU target.
DavidKreitzer added a comment. Thanks, Alexey! I think this is an improvement. I don't know if anyone else has an opinion on the name of the new class, but I would prefer X86 over I386, possibly even MCUX86_32TargetInfo. Thanks, - Dave http://reviews.llvm.org/D14864 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14864: [X86] Support for C calling convention only for MCU target.
DavidKreitzer added a comment. Looks good, Alexey! I have no further comments. - Dave http://reviews.llvm.org/D14864 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits