subject:"\[PATCH\] D59744\: Fix i386 ABI \"_

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-19 Thread Craig Topper via Phabricator via cfe-commits

craig.topper added a comment.

-O0 always inline isn't working because the frontend is emitting a store of 
vector type to memory then a load of x86_mmx to do the type coercion. The 
caller does the opposite to coerce back from mmx. This -O0 pipeline isn't 
capable of getting rid of these redundant store/load pairs. We might have a 
better chance if we just emitted bitcasts.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-19 Thread Hans Wennborg via Phabricator via cfe-commits

hans added a comment.

> $ bin/clang -m32 -O0 /tmp/a.c && ./a.out
> -nan
> 
>   Before your change, it prints 3.14.

I looked through the Intel manual to understand what's happening in detail:

When we return from f() with the new ABI, we write to the %mm0 register, and as 
a side effect:

(9.5.1) After each MMX instruction, the entire x87 FPU tag word is set to valid 
(00B).

What does that mean?

(8.1.7) "The x87 FPU uses the tag values to detect stack overflow and underflow 
conditions (see Section 8.5.1.1)"

(8.5.1.1) "Stack overflow — An instruction attempts to load a non-empty x87 FPU 
register from memory. A non-empty
register is defined as a register containing a zero (tag value of 01), a valid 
value (tag value of 00), or a special
value (tag value of 10).

When the x87 FPU detects stack overflow or underflow, it sets the IE flag (bit 
0) and the SF flag (bit 6) in the x87
FPU status word to 1. It then sets condition-code flag C1 (bit 9) in the x87 
FPU status word to 1 if stack overflow
occurred or to 0 if stack underflow occurred.
If the invalid-operation exception is masked, the x87 FPU returns the floating 
point, integer, or packed decimal
integer indefinite value to the destination operand, depending on the 
instruction being executed. This value over-
writes the destination register or memory location specified by the 
instruction."

Okay, so essentially any MMX instruction marks the x87 register stack as full, 
and when we try to store into it in d() with "fldl" we get a stack overflow, 
and because the exception is masked, it stores "the floating point indefinite 
value" into the register, which is what we end up printing.

At least I finally think I understand what's going on :-)


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-19 Thread Hans Wennborg via Phabricator via cfe-commits

hans added a comment.

I've reverted in r363790 until a solution can be found.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-19 Thread Hans Wennborg via Phabricator via cfe-commits

hans added a comment.

In D59744#1549746 , @hans wrote:

> In D59744#1549675 , @wxiao3 wrote:
>
> > Can anyone provide me some small reproducers code for the issue tripped 
> > over by Chromium / Skia?
>
>
> Sorry, I don't have a small repro yet. I'm still working on finding out 
> exactly what's happening in Chromium, but it's a large test. It's easy to 
> find where the x87 state gets clobbered after your change, but I haven't 
> found what code was depending on that state yet.


Oh, I thought the problem was just that the registers alias, not that the whole 
x87 state gets messed up by mmx instructions. Here's a simple repro:

  $ cat /tmp/a.c
  #include 
  #include 
  
  #ifdef __clang__
  typedef uint16_t __attribute__((ext_vector_type(4))) V;
  #else
  typedef uint16_t V __attribute__ ((vector_size (4*sizeof(uint16_t;
  #endif
  
  V f() {
V v = { 1,2,3,4 };
return v;
  }
  
  double d() { return 3.14; }
  
  int main() {
f();
printf("%lf\n", d());
return 0;
  }
  
  $ bin/clang -m32 -O0 /tmp/a.c && ./a.out
  -nan

Before your change, it prints 3.14.

Chromium was previously working around this problem in gcc by force-inlining 
f() into main(). That doesn't work with Clang because it touches %mm0 even 
after inlining.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-19 Thread Simon Pilgrim via Phabricator via cfe-commits

RKSimon added a comment.

I've raised https://bugs.llvm.org/show_bug.cgi?id=42319 which suggests the 
creation of a EMMS insertion pass.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-19 Thread Hans Wennborg via Phabricator via cfe-commits

hans added a comment.

In D59744#1549675 , @wxiao3 wrote:

> Can anyone provide me some small reproducers code for the issue tripped over 
> by Chromium / Skia?

Sorry, I don't have a small repro yet. I'm still working on finding out exactly 
what's happening in Chromium, but it's a large test. It's easy to find where 
the x87 state gets clobbered after your change, but I haven't found what code 
was depending on that state yet.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-18 Thread Wei Xiao via Phabricator via cfe-commits

wxiao3 added a comment.

Can anyone provide me some small reproducers code for the issue tripped over by 
Chromium / Skia?


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-18 Thread Eli Friedman via Phabricator via cfe-commits

efriedma added a comment.

> I'm just laying out the basic requirements for getting this patch back in, 
> because the current patch is invalid given LLVM's current requirements.

Yes, I'm on the same page.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-18 Thread John McCall via Phabricator via cfe-commits

rjmccall added a comment.

In D59744#1549229 , @efriedma wrote:

> If we're going to insert emms instructions automatically, it doesn't really 
> make sense to do it in the frontend; the backend could figure out the most 
> efficient placement itself.  (See lib/Target/X86/X86VZeroUpper.cpp, which 
> implements similar logic for AVX.)  The part I'd be worried about is the 
> potential performance hit from calling emms in places where other compilers 
> wouldn't, for code using MMX intrinsics.

It would certainly be simpler for the frontend if the backend did this — in 
fact, even if the "frontend" was going to do it, I would have suggested doing 
it as a pass over the emitted IR rather than a special case in IRGen.  Anyway, 
I'm open to any reasonable option; at this point, I'm just laying out the basic 
requirements for getting this patch back in, because the current patch is 
invalid given LLVM's current requirements.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-18 Thread Eli Friedman via Phabricator via cfe-commits

efriedma added a comment.

If we're going to insert emms instructions automatically, it doesn't really 
make sense to do it in the frontend; the backend could figure out the most 
efficient placement itself.  (See lib/Target/X86/X86VZeroUpper.cpp, which 
implements similar logic for AVX.)  The part I'd be worried about is the 
potential performance hit from calling emms in places where other compilers 
wouldn't, for code using MMX intrinsics.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-18 Thread John McCall via Phabricator via cfe-commits

rjmccall added a comment.

Thank you.  So it sounds like this patch needs to be reverted, and the correct 
version of it will have to insert these intrinsic calls in four places:

- before translating vector arguments to MMX type before calls that pass 
`__m64` arguments,
- after translating MMX parameters to vector type in functions that receive 
`__m64` parameters,
- before translating vector results to MMX type in functions that return 
`__m64`, and
- after translating MMX results to vector type after calls that return `__m64`.

Will that be sufficient to satisfy LLVM?


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-18 Thread Mike Klein via Phabricator via cfe-commits

mtklein added a comment.

Ah, thank you for that explanation.  That's got to be exactly what we're 
tripping over in Chromium / Skia.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-18 Thread Eli Friedman via Phabricator via cfe-commits

efriedma added a comment.

> Are you saying that using MMX in LLVM requires source-level workarounds in 
> some way, and so we can't lower portable code to use MMX because that code 
> will (reasonably) lack those workarounds?

Yes.

The x86 architecture requires that a program executes an "emms" instruction 
between any MMX instructions, and any x87 instructions.  Otherwise, the x87 
instructions will produce nonsense results.  LLVM, and other compilers, never 
insert emms automatically; this is partially historical, but also because emms 
can be expensive on Intel chips. Instead, the user is expected to call 
_mm_empty() in appropriate places.

To allow users to generate arbitrary vector IR without tripping over this, LLVM 
does not lower vector IR to MMX instructions; instead, it only generates MMX 
instructions for operations using the special type x86_mmx. If any instruction 
or argument has a result or operand of type x86_mmx in LLVM IR, the user must 
explicitly execute emms (@llvm.x86.mmx.emms() in IR, _mm_empty() in C) between 
that instruction, and any code that might use x87 registers.   "Between" isn't 
really sound because emms intrinsic doesn't reliably prevent code motion of 
floating-point operations, but it works well enough in practice.  (See also 
https://bugs.llvm.org/show_bug.cgi?id=35982 .)

On the clang side, without this patch, we only generate code using the x86_mmx 
type in a couple places: _mm_* calls, and inline asm with an MMX operand.  If 
the user does not use either of those, there will never be any values of type 
x86_mmx, so there will never be any MMX instructions, and we avoid the whole 
mess.  64-bit vector operations get lowered to SSE2 instructions instead (or 
scalarized).

This patch introduces a new place where clang will generate the type x86_mmx: 
for call arguments and return values.  This means more places where the user is 
required to write _mm_empty() to get correct behavior.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-18 Thread John McCall via Phabricator via cfe-commits

rjmccall added a comment.

In D59744#1548919 , @efriedma wrote:

> > Now, we could theoretically use a different ABI rule for vectors defined 
> > with Clang-specific extensions, but that seems like it would cause quite a 
> > few problems of its own.
>
> I think we can't reasonably impose this ABI rule on vectors defined with 
> ext_vector_type: that makes it impossible to build portable OpenCL code for 
> 32-bit x86, given the side-effects of introducing any use of the x86_mmx type.

Sorry, I've remained somewhat intentionally ignorant of the issues here.  Are 
you saying that using MMX in LLVM requires source-level workarounds in some 
way, and so we can't lower portable code to use MMX because that code will 
(reasonably) lack those workarounds?  If that's true, then fixing that seems 
like a blocker to landing this patch; it is better to be ABI-non-compliant than 
to produce broken code.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-18 Thread Eli Friedman via Phabricator via cfe-commits

efriedma added a comment.

> Now, we could theoretically use a different ABI rule for vectors defined with 
> Clang-specific extensions, but that seems like it would cause quite a few 
> problems of its own.

I think we can't reasonably impose this ABI rule on vectors defined with 
ext_vector_type: that makes it impossible to build portable OpenCL code for 
32-bit x86, given the side-effects of introducing any use of the x86_mmx type.  
So that leaves us with two options: make vector_size and ext_vector_type 
incompatible, or revert this patch and intentionally remain ABI-incompatible 
with gcc. (I guess we could theoretically try to separate out a special case 
for OpenCL instead, but that seems even more fragile.)

Being ABI-incompatible is obviously inconvenient if you're writing code using 
MMX types/intrinsics, but using MMX intrinsics is sort of "at your own risk" 
anyway, given neither LLVM nor gcc properly manages the state of the MMX/x87 
register file.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-18 Thread John McCall via Phabricator via cfe-commits

rjmccall added a comment.

In D59744#1548540 , @mtklein wrote:

> Hey folks, I'm the Skia point of contact on this, and "luckily" the person 
> who wrote all the code that got us into this mess.  Let me cross post a 
> couple questions I've had from the Chromium bug over here where folks might 
> know the answer...
>
> Now that Clang's decided to match GCC's behavior of using mm0 to pass around 
> 8-byte vectors on x86-32, is there any way to use 8-byte vector types safely 
> any more?  I don't really have the full context of this Clang change, but is 
> it maybe a good idea applied to too many types?  I notice the change mentions 
> __m64, but here I'm using uint16_t ext_vector_type(4) exclusively, never 
> __m64 or even an 8x8 vector... can we just squint and say u16x4 and __m64 
> aren't the same, passing __m64 according to the ABI but vector extensions 
> however we were doing it before?

`__m64` is of course defined using the compiler's vector extensions.  More 
importantly, GCC also has those vector extensions (or at least some of them), 
and my understanding is that GCC is interpreting the ABI's  `__m64` to mean 
"all 8-byte vectors" (which seems quite reasonable to me), and that's what 
Clang needs to stay compatible with on systems where GCC is the system compiler.

Now, we could theoretically use a different ABI rule for vectors defined with 
Clang-specific extensions, but that seems like it would cause quite a few 
problems of its own.

> In short, should working with 4x u16 be safe on x86-32 and there's a bug / 
> undefined behavior in my code leading to this  mm0/st0 clobber, or is this 
> just actually not really spec'd to work?

It's always possible that there's a bug in the compiler, but the most likely 
thing is that you have assembly code that's not obeying the ABI in some way.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-18 Thread Mike Klein via Phabricator via cfe-commits

mtklein added a comment.

Hey folks, I'm the Skia point of contact on this, and "luckily" the person who 
wrote all the code that got us into this mess.  Let me cross post a couple 
questions I've had from the Chromium bug over here where folks might know the 
answer...

Now that Clang's decided to match GCC's behavior of using mm0 to pass around 
8-byte vectors on x86-32, is there any way to use 8-byte vector types safely 
any more?  I don't really have the full context of this Clang change, but is it 
maybe a good idea applied to too many types?  I notice the change mentions 
__m64, but here I'm using uint16_t ext_vector_type(4) exclusively, never __m64 
or even an 8x8 vector... can we just squint and say u16x4 and __m64 aren't the 
same, passing __m64 according to the ABI but vector extensions however we were 
doing it before?  Or can we work out some sort of ABI that preserves st0/mm0?  
I think we're finding that even with forced-inlining, at -O0 we still end up 
getting u16x4 values stored in mm0 briefly (kind of roundabout through xmm 
registers and the stack once or twice too).

In short, should working with 4x u16 be safe on x86-32 and there's a bug / 
undefined behavior in my code leading to this  mm0/st0 clobber, or is this just 
actually not really spec'd to work?


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-18 Thread Hans Wennborg via Phabricator via cfe-commits

hans added a comment.

In D59744#1547445 , @wxiao3 wrote:

> @hans
>
> Please make sure all Chromium for 32-bit Linux libraries are following System 
> V ABI (i.e., m64 is passed on mmx register). I suspect that there are some 
> hand written assembly code in your libraries which is not following the ABI.

We still don't have the root cause, but the library in question (Skia) doesn't 
have much assembly code. After your patch, %st0 (which aliases with %mm0) gets 
clobbered if a function returns a 4 x u16 vector. Skia tries to work around 
this by force-inlining such functions, but we're still seeing functions where 
%mm0 gets used. We believe this is the cause, but I'm still trying to figure 
out where the remaining %mm0 uses come from.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-17 Thread Wei Xiao via Phabricator via cfe-commits

wxiao3 added a comment.

I have created a patch for you: https://reviews.llvm.org/D63473
Is it ok?


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-17 Thread Nico Weber via Phabricator via cfe-commits

thakis added a comment.

In D59744#1547445 , @wxiao3 wrote:

> @hans
>
> Please make sure all Chromium for 32-bit Linux libraries are following System 
> V ABI (i.e., m64 is passed on mmx register). I suspect that there are some 
> hand written assembly code in your libraries which is not following the ABI.

That's likely true, but also not very helpful since the ABI implications here 
are pretty big (see comments on the chromium bug). It's also currently 
impossible to write an assembly function that works with both trunk clang and 
clang 8.0.0, which makes it difficult to update compilers independent of 
changing the code. clang generally tries to be abi-compatible with itself. This 
should probably support the existing fclang-abi-compat= flag at least (and have 
a release notes entry); possibly there should be a dedicated flag for this.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-17 Thread Wei Xiao via Phabricator via cfe-commits

wxiao3 added a comment.

@hans

Please make sure all Chromium for 32-bit Linux libraries are following System V 
ABI (i.e., m64 is passed on mmx register). I suspect that there are some hand 
written assembly code in your libraries which is not following the ABI.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-17 Thread Hans Wennborg via Phabricator via cfe-commits

hans added a comment.

I tried replying on the cfe-commits email, but both Pengfei and Wei's email 
addresses seem to bounce, so replying here instead:

This broke Chromium for 32-bit Linux:
https://bugs.chromium.org/p/chromium/issues/detail?id=974542#c5

It's not clear what's happening yet, bet just to give a heads up.

Since this changes ABI behaviour, it would probably we worth
mentioning in docs/ReleaseNotes.rst too.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-11 Thread Pengfei Wang via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL363116: [X86] [ABI] Fix i386 ABI __m64 type bug 
(authored by pengfei, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59744?vs=204197=204203#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744

Files:
  cfe/trunk/lib/CodeGen/TargetInfo.cpp
  cfe/trunk/test/CodeGen/x86_32-arguments-linux.c
  cfe/trunk/test/CodeGen/x86_32-m64.c

Index: cfe/trunk/test/CodeGen/x86_32-arguments-linux.c
===
--- cfe/trunk/test/CodeGen/x86_32-arguments-linux.c
+++ cfe/trunk/test/CodeGen/x86_32-arguments-linux.c
@@ -3,7 +3,7 @@
 
 // CHECK-LABEL: define void @f56(
 // CHECK: i8 signext %a0, %struct.s56_0* byval(%struct.s56_0) align 4 %a1,
-// CHECK: i64 %a2.coerce, %struct.s56_1* byval(%struct.s56_1) align 4,
+// CHECK: x86_mmx %a2.coerce, %struct.s56_1* byval(%struct.s56_1) align 4,
 // CHECK: <1 x double> %a4, %struct.s56_2* byval(%struct.s56_2) align 4,
 // CHECK: <4 x i32> %a6, %struct.s56_3* byval(%struct.s56_3) align 4,
 // CHECK: <2 x double> %a8, %struct.s56_4* byval(%struct.s56_4) align 4,
@@ -12,7 +12,7 @@
 
 // CHECK: call void (i32, ...) @f56_0(i32 1,
 // CHECK: i32 %{{.*}}, %struct.s56_0* byval(%struct.s56_0) align 4 %{{[^ ]*}},
-// CHECK: i64 %{{[^ ]*}}, %struct.s56_1* byval(%struct.s56_1) align 4 %{{[^ ]*}},
+// CHECK: x86_mmx %{{[^ ]*}}, %struct.s56_1* byval(%struct.s56_1) align 4 %{{[^ ]*}},
 // CHECK: <1 x double> %{{[^ ]*}}, %struct.s56_2* byval(%struct.s56_2) align 4 %{{[^ ]*}},
 // CHECK: <4 x i32> %{{[^ ]*}}, %struct.s56_3* byval(%struct.s56_3) align 4 %{{[^ ]*}},
 // CHECK: <2 x double> %{{[^ ]*}}, %struct.s56_4* byval(%struct.s56_4) align 4 %{{[^ ]*}},
Index: cfe/trunk/test/CodeGen/x86_32-m64.c
===
--- cfe/trunk/test/CodeGen/x86_32-m64.c
+++ cfe/trunk/test/CodeGen/x86_32-m64.c
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-linux-gnu -target-cpu pentium4 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,LINUX
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-netbsd -target-cpu pentium4 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,NETBSD
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-apple-darwin9 -target-cpu yonah -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,DARWIN
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-elfiamcu -mfloat-abi soft -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,IAMCU
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-win32 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,WIN32
+
+#include 
+__m64 m64;
+void callee(__m64 __m1, __m64 __m2);
+__m64 caller(__m64 __m1, __m64 __m2)
+{
+// LINUX-LABEL: define x86_mmx @caller(x86_mmx %__m1.coerce, x86_mmx %__m2.coerce)
+// LINUX: tail call void @callee(x86_mmx %__m2.coerce, x86_mmx %__m1.coerce)
+// LINUX: ret x86_mmx
+// NETBSD-LABEL: define x86_mmx @caller(x86_mmx %__m1.coerce, x86_mmx %__m2.coerce)
+// NETBSD: tail call void @callee(x86_mmx %__m2.coerce, x86_mmx %__m1.coerce)
+// NETBSD: ret x86_mmx
+// DARWIN-LABEL: define i64 @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// DARWIN: tail call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// DARWIN: ret i64
+// IAMCU-LABEL: define <1 x i64> @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// IAMCU: tail call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// IAMCU: ret <1 x i64>
+// WIN32-LABEL: define dso_local <1 x i64> @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// WIN32: call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// WIN32: ret <1 x i64>
+  callee(__m2, __m1);
+  return m64;
+}
Index: cfe/trunk/lib/CodeGen/TargetInfo.cpp
===
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp
@@ -915,14 +915,6 @@
: ABIArgInfo::getDirect());
 }
 
-/// IsX86_MMXType - Return true if this is an MMX type.
-bool IsX86_MMXType(llvm::Type *IRType) {
-  // Return true if the type is an MMX type <2 x i32>, <4 x i16>, or <8 x i8>.
-  return IRType->isVectorTy() && IRType->getPrimitiveSizeInBits() == 64 &&
-cast(IRType)->getElementType()->isIntegerTy() &&
-IRType->getScalarSizeInBits() != 64;
-}
-
 static llvm::Type* X86AdjustInlineAsmType(CodeGen::CodeGenFunction ,
   StringRef Constraint,
   llvm::Type* Ty) {
@@ -1011,6 +1003,7 @@
   bool IsSoftFloatABI;
   bool IsMCUABI;
   unsigned DefaultNumRegisterParameters;
+  bool IsMMXEnabled;
 
   static bool isRegisterSize(unsigned Size) {
 return (Size == 8 || Size == 16 || Size == 32 || Size == 64);
@@ -1070,13 +1063,15 @@
 
   X86_32ABIInfo(CodeGen::CodeGenTypes , bool

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-11 Thread Wei Xiao via Phabricator via cfe-commits

wxiao3 added a comment.

Thanks for the comments!
Updated for landing.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-11 Thread Wei Xiao via Phabricator via cfe-commits

wxiao3 updated this revision to Diff 204197.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744

Files:
  lib/CodeGen/TargetInfo.cpp
  test/CodeGen/x86_32-arguments-linux.c
  test/CodeGen/x86_32-m64.c

Index: test/CodeGen/x86_32-m64.c
===
--- /dev/null
+++ test/CodeGen/x86_32-m64.c
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-linux-gnu -target-cpu pentium4 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,LINUX
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-netbsd -target-cpu pentium4 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,NETBSD
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-apple-darwin9 -target-cpu yonah -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,DARWIN
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-elfiamcu -mfloat-abi soft -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,IAMCU
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-win32 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,WIN32
+
+#include 
+__m64 m64;
+void callee(__m64 __m1, __m64 __m2);
+__m64 caller(__m64 __m1, __m64 __m2)
+{
+// LINUX-LABEL: define x86_mmx @caller(x86_mmx %__m1.coerce, x86_mmx %__m2.coerce)
+// LINUX: tail call void @callee(x86_mmx %__m2.coerce, x86_mmx %__m1.coerce)
+// LINUX: ret x86_mmx
+// NETBSD-LABEL: define x86_mmx @caller(x86_mmx %__m1.coerce, x86_mmx %__m2.coerce)
+// NETBSD: tail call void @callee(x86_mmx %__m2.coerce, x86_mmx %__m1.coerce)
+// NETBSD: ret x86_mmx
+// DARWIN-LABEL: define i64 @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// DARWIN: tail call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// DARWIN: ret i64
+// IAMCU-LABEL: define <1 x i64> @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// IAMCU: tail call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// IAMCU: ret <1 x i64>
+// WIN32-LABEL: define dso_local <1 x i64> @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// WIN32: call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// WIN32: ret <1 x i64>
+  callee(__m2, __m1);
+  return m64;
+}
Index: test/CodeGen/x86_32-arguments-linux.c
===
--- test/CodeGen/x86_32-arguments-linux.c
+++ test/CodeGen/x86_32-arguments-linux.c
@@ -3,7 +3,7 @@
 
 // CHECK-LABEL: define void @f56(
 // CHECK: i8 signext %a0, %struct.s56_0* byval(%struct.s56_0) align 4 %a1,
-// CHECK: i64 %a2.coerce, %struct.s56_1* byval(%struct.s56_1) align 4,
+// CHECK: x86_mmx %a2.coerce, %struct.s56_1* byval(%struct.s56_1) align 4,
 // CHECK: <1 x double> %a4, %struct.s56_2* byval(%struct.s56_2) align 4,
 // CHECK: <4 x i32> %a6, %struct.s56_3* byval(%struct.s56_3) align 4,
 // CHECK: <2 x double> %a8, %struct.s56_4* byval(%struct.s56_4) align 4,
@@ -12,7 +12,7 @@
 
 // CHECK: call void (i32, ...) @f56_0(i32 1,
 // CHECK: i32 %{{.*}}, %struct.s56_0* byval(%struct.s56_0) align 4 %{{[^ ]*}},
-// CHECK: i64 %{{[^ ]*}}, %struct.s56_1* byval(%struct.s56_1) align 4 %{{[^ ]*}},
+// CHECK: x86_mmx %{{[^ ]*}}, %struct.s56_1* byval(%struct.s56_1) align 4 %{{[^ ]*}},
 // CHECK: <1 x double> %{{[^ ]*}}, %struct.s56_2* byval(%struct.s56_2) align 4 %{{[^ ]*}},
 // CHECK: <4 x i32> %{{[^ ]*}}, %struct.s56_3* byval(%struct.s56_3) align 4 %{{[^ ]*}},
 // CHECK: <2 x double> %{{[^ ]*}}, %struct.s56_4* byval(%struct.s56_4) align 4 %{{[^ ]*}},
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -915,14 +915,6 @@
: ABIArgInfo::getDirect());
 }
 
-/// IsX86_MMXType - Return true if this is an MMX type.
-bool IsX86_MMXType(llvm::Type *IRType) {
-  // Return true if the type is an MMX type <2 x i32>, <4 x i16>, or <8 x i8>.
-  return IRType->isVectorTy() && IRType->getPrimitiveSizeInBits() == 64 &&
-cast(IRType)->getElementType()->isIntegerTy() &&
-IRType->getScalarSizeInBits() != 64;
-}
-
 static llvm::Type* X86AdjustInlineAsmType(CodeGen::CodeGenFunction ,
   StringRef Constraint,
   llvm::Type* Ty) {
@@ -1011,6 +1003,7 @@
   bool IsSoftFloatABI;
   bool IsMCUABI;
   unsigned DefaultNumRegisterParameters;
+  bool IsMMXEnabled;
 
   static bool isRegisterSize(unsigned Size) {
 return (Size == 8 || Size == 16 || Size == 32 || Size == 64);
@@ -1070,13 +1063,15 @@
 
   X86_32ABIInfo(CodeGen::CodeGenTypes , bool DarwinVectorABI,
 bool RetSmallStructInRegABI, bool Win32StructABI,
-unsigned NumRegisterParameters, bool SoftFloatABI)
+unsigned NumRegisterParameters, bool SoftFloatABI,
+bool MMXEnabled)
 : SwiftABIInfo(CGT), IsDarwinVectorABI(DarwinVectorABI),
   IsRetSmallStructInRegABI(RetSmallStructInRegABI),
   IsWin32StructABI(Win32StructABI),

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-11 Thread John McCall via Phabricator via cfe-commits

rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

Minor comments, then LGTM.




Comment at: lib/CodeGen/TargetInfo.cpp:1094
+// FreeBSD) don't want to spend any effort dealing with the ramifications
+// of ABI breaks at present.
+const llvm::Triple  = getTarget().getTriple();

"The System V i386 psABI requires __m64 to be passed in MMX registers.
Clang historically had a bug where it failed to apply this rule, and some 
platforms
(e.g. Darwin, PS4, and FreeBSD) have opted to maintain compatibility with the 
old
Clang behavior, so we only apply it on platforms that have specifically 
requested it
(currently just Linux and NetBSD)."



Comment at: lib/CodeGen/TargetInfo.cpp:1417
+if (VT->getElementType()->isIntegerType() && Size == 64 &&
+  isPassInMMXRegABI())
+  return ABIArgInfo::getDirect(llvm::Type::getX86_MMXTy(getVMContext()));

Indentation on the continuation line.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-11 Thread Wei Xiao via Phabricator via cfe-commits

wxiao3 added a comment.

Thanks for the suggestions!
I have updated it.

Ok now?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-11 Thread Wei Xiao via Phabricator via cfe-commits

wxiao3 updated this revision to Diff 204057.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744

Files:
  lib/CodeGen/TargetInfo.cpp
  test/CodeGen/x86_32-arguments-linux.c
  test/CodeGen/x86_32-m64.c

Index: test/CodeGen/x86_32-m64.c
===
--- /dev/null
+++ test/CodeGen/x86_32-m64.c
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-linux-gnu -target-cpu pentium4 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,LINUX
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-netbsd -target-cpu pentium4 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,NETBSD
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-apple-darwin9 -target-cpu yonah -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,DARWIN
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-elfiamcu -mfloat-abi soft -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,IAMCU
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-win32 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,WIN32
+
+#include 
+__m64 m64;
+void callee(__m64 __m1, __m64 __m2);
+__m64 caller(__m64 __m1, __m64 __m2)
+{
+// LINUX-LABEL: define x86_mmx @caller(x86_mmx %__m1.coerce, x86_mmx %__m2.coerce)
+// LINUX: tail call void @callee(x86_mmx %__m2.coerce, x86_mmx %__m1.coerce)
+// LINUX: ret x86_mmx
+// NETBSD-LABEL: define x86_mmx @caller(x86_mmx %__m1.coerce, x86_mmx %__m2.coerce)
+// NETBSD: tail call void @callee(x86_mmx %__m2.coerce, x86_mmx %__m1.coerce)
+// NETBSD: ret x86_mmx
+// DARWIN-LABEL: define i64 @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// DARWIN: tail call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// DARWIN: ret i64
+// IAMCU-LABEL: define <1 x i64> @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// IAMCU: tail call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// IAMCU: ret <1 x i64>
+// WIN32-LABEL: define dso_local <1 x i64> @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// WIN32: call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// WIN32: ret <1 x i64>
+  callee(__m2, __m1);
+  return m64;
+}
Index: test/CodeGen/x86_32-arguments-linux.c
===
--- test/CodeGen/x86_32-arguments-linux.c
+++ test/CodeGen/x86_32-arguments-linux.c
@@ -3,7 +3,7 @@
 
 // CHECK-LABEL: define void @f56(
 // CHECK: i8 signext %a0, %struct.s56_0* byval(%struct.s56_0) align 4 %a1,
-// CHECK: i64 %a2.coerce, %struct.s56_1* byval(%struct.s56_1) align 4,
+// CHECK: x86_mmx %a2.coerce, %struct.s56_1* byval(%struct.s56_1) align 4,
 // CHECK: <1 x double> %a4, %struct.s56_2* byval(%struct.s56_2) align 4,
 // CHECK: <4 x i32> %a6, %struct.s56_3* byval(%struct.s56_3) align 4,
 // CHECK: <2 x double> %a8, %struct.s56_4* byval(%struct.s56_4) align 4,
@@ -12,7 +12,7 @@
 
 // CHECK: call void (i32, ...) @f56_0(i32 1,
 // CHECK: i32 %{{.*}}, %struct.s56_0* byval(%struct.s56_0) align 4 %{{[^ ]*}},
-// CHECK: i64 %{{[^ ]*}}, %struct.s56_1* byval(%struct.s56_1) align 4 %{{[^ ]*}},
+// CHECK: x86_mmx %{{[^ ]*}}, %struct.s56_1* byval(%struct.s56_1) align 4 %{{[^ ]*}},
 // CHECK: <1 x double> %{{[^ ]*}}, %struct.s56_2* byval(%struct.s56_2) align 4 %{{[^ ]*}},
 // CHECK: <4 x i32> %{{[^ ]*}}, %struct.s56_3* byval(%struct.s56_3) align 4 %{{[^ ]*}},
 // CHECK: <2 x double> %{{[^ ]*}}, %struct.s56_4* byval(%struct.s56_4) align 4 %{{[^ ]*}},
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -915,14 +915,6 @@
: ABIArgInfo::getDirect());
 }
 
-/// IsX86_MMXType - Return true if this is an MMX type.
-bool IsX86_MMXType(llvm::Type *IRType) {
-  // Return true if the type is an MMX type <2 x i32>, <4 x i16>, or <8 x i8>.
-  return IRType->isVectorTy() && IRType->getPrimitiveSizeInBits() == 64 &&
-cast(IRType)->getElementType()->isIntegerTy() &&
-IRType->getScalarSizeInBits() != 64;
-}
-
 static llvm::Type* X86AdjustInlineAsmType(CodeGen::CodeGenFunction ,
   StringRef Constraint,
   llvm::Type* Ty) {
@@ -1011,6 +1003,7 @@
   bool IsSoftFloatABI;
   bool IsMCUABI;
   unsigned DefaultNumRegisterParameters;
+  bool IsMMXEnabled;
 
   static bool isRegisterSize(unsigned Size) {
 return (Size == 8 || Size == 16 || Size == 32 || Size == 64);
@@ -1070,13 +1063,15 @@
 
   X86_32ABIInfo(CodeGen::CodeGenTypes , bool DarwinVectorABI,
 bool RetSmallStructInRegABI, bool Win32StructABI,
-unsigned NumRegisterParameters, bool SoftFloatABI)
+unsigned NumRegisterParameters, bool SoftFloatABI,
+bool MMXEnabled)
 : SwiftABIInfo(CGT), IsDarwinVectorABI(DarwinVectorABI),
   IsRetSmallStructInRegABI(RetSmallStructInRegABI),
   IsWin32StructABI(Win32StructABI),

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-10 Thread John McCall via Phabricator via cfe-commits

rjmccall added inline comments.



Comment at: lib/CodeGen/TargetInfo.cpp:1005
   bool IsMCUABI;
+  bool IsLinuxABI;
   unsigned DefaultNumRegisterParameters;

mgorny wrote:
> Maybe replace the two booleans with something alike `IsPassInMMXRegABI`? And 
> while at it, include NetBSD there, please.
`CGT` is a member variable, so you can just query the target fresh in your 
`isPassInMMXRegABI` method.  The check upfront for a 64-bit vector type should 
keep this well out of the fast path.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-06 Thread Wei Xiao via Phabricator via cfe-commits

wxiao3 added a comment.

Thanks for the suggestions!
I have updated the patch accordingly.

Ok for merge now?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-06 Thread Wei Xiao via Phabricator via cfe-commits

wxiao3 updated this revision to Diff 203492.
wxiao3 edited the summary of this revision.
wxiao3 added a reviewer: mgorny.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744

Files:
  lib/CodeGen/TargetInfo.cpp
  test/CodeGen/x86_32-arguments-linux.c
  test/CodeGen/x86_32-m64.c

Index: test/CodeGen/x86_32-m64.c
===
--- /dev/null
+++ test/CodeGen/x86_32-m64.c
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-linux-gnu -target-cpu pentium4 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,LINUX
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-netbsd -target-cpu pentium4 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,NETBSD
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-apple-darwin9 -target-cpu yonah -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,DARWIN
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-elfiamcu -mfloat-abi soft -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,IAMCU
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-win32 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,WIN32
+
+#include 
+__m64 m64;
+void callee(__m64 __m1, __m64 __m2);
+__m64 caller(__m64 __m1, __m64 __m2)
+{
+// LINUX-LABEL: define x86_mmx @caller(x86_mmx %__m1.coerce, x86_mmx %__m2.coerce)
+// LINUX: tail call void @callee(x86_mmx %__m2.coerce, x86_mmx %__m1.coerce)
+// LINUX: ret x86_mmx
+// NETBSD-LABEL: define x86_mmx @caller(x86_mmx %__m1.coerce, x86_mmx %__m2.coerce)
+// NETBSD: tail call void @callee(x86_mmx %__m2.coerce, x86_mmx %__m1.coerce)
+// NETBSD: ret x86_mmx
+// DARWIN-LABEL: define i64 @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// DARWIN: tail call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// DARWIN: ret i64
+// IAMCU-LABEL: define <1 x i64> @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// IAMCU: tail call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// IAMCU: ret <1 x i64>
+// WIN32-LABEL: define dso_local <1 x i64> @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// WIN32: call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// WIN32: ret <1 x i64>
+  callee(__m2, __m1);
+  return m64;
+}
Index: test/CodeGen/x86_32-arguments-linux.c
===
--- test/CodeGen/x86_32-arguments-linux.c
+++ test/CodeGen/x86_32-arguments-linux.c
@@ -3,7 +3,7 @@
 
 // CHECK-LABEL: define void @f56(
 // CHECK: i8 signext %a0, %struct.s56_0* byval(%struct.s56_0) align 4 %a1,
-// CHECK: i64 %a2.coerce, %struct.s56_1* byval(%struct.s56_1) align 4,
+// CHECK: x86_mmx %a2.coerce, %struct.s56_1* byval(%struct.s56_1) align 4,
 // CHECK: <1 x double> %a4, %struct.s56_2* byval(%struct.s56_2) align 4,
 // CHECK: <4 x i32> %a6, %struct.s56_3* byval(%struct.s56_3) align 4,
 // CHECK: <2 x double> %a8, %struct.s56_4* byval(%struct.s56_4) align 4,
@@ -12,7 +12,7 @@
 
 // CHECK: call void (i32, ...) @f56_0(i32 1,
 // CHECK: i32 %{{.*}}, %struct.s56_0* byval(%struct.s56_0) align 4 %{{[^ ]*}},
-// CHECK: i64 %{{[^ ]*}}, %struct.s56_1* byval(%struct.s56_1) align 4 %{{[^ ]*}},
+// CHECK: x86_mmx %{{[^ ]*}}, %struct.s56_1* byval(%struct.s56_1) align 4 %{{[^ ]*}},
 // CHECK: <1 x double> %{{[^ ]*}}, %struct.s56_2* byval(%struct.s56_2) align 4 %{{[^ ]*}},
 // CHECK: <4 x i32> %{{[^ ]*}}, %struct.s56_3* byval(%struct.s56_3) align 4 %{{[^ ]*}},
 // CHECK: <2 x double> %{{[^ ]*}}, %struct.s56_4* byval(%struct.s56_4) align 4 %{{[^ ]*}},
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -915,14 +915,6 @@
: ABIArgInfo::getDirect());
 }
 
-/// IsX86_MMXType - Return true if this is an MMX type.
-bool IsX86_MMXType(llvm::Type *IRType) {
-  // Return true if the type is an MMX type <2 x i32>, <4 x i16>, or <8 x i8>.
-  return IRType->isVectorTy() && IRType->getPrimitiveSizeInBits() == 64 &&
-cast(IRType)->getElementType()->isIntegerTy() &&
-IRType->getScalarSizeInBits() != 64;
-}
-
 static llvm::Type* X86AdjustInlineAsmType(CodeGen::CodeGenFunction ,
   StringRef Constraint,
   llvm::Type* Ty) {
@@ -1010,7 +1002,10 @@
   bool IsWin32StructABI;
   bool IsSoftFloatABI;
   bool IsMCUABI;
+  bool IsLinuxABI;
+  bool IsNetBSDABI;
   unsigned DefaultNumRegisterParameters;
+  bool IsMMXEnabled;
 
   static bool isRegisterSize(unsigned Size) {
 return (Size == 8 || Size == 16 || Size == 32 || Size == 64);
@@ -1070,13 +1065,17 @@
 
   X86_32ABIInfo(CodeGen::CodeGenTypes , bool DarwinVectorABI,
 bool RetSmallStructInRegABI, bool Win32StructABI,
-unsigned NumRegisterParameters, bool SoftFloatABI)
+unsigned NumRegisterParameters, bool SoftFloatABI,
+bool MMXEnabled)
 : SwiftABIInfo(CGT),

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-06 Thread Michał Górny via Phabricator via cfe-commits

mgorny added inline comments.



Comment at: lib/CodeGen/TargetInfo.cpp:1005
   bool IsMCUABI;
+  bool IsLinuxABI;
   unsigned DefaultNumRegisterParameters;

Maybe replace the two booleans with something alike `IsPassInMMXRegABI`? And 
while at it, include NetBSD there, please.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-05 Thread Joerg Sonnenberger via Phabricator via cfe-commits

joerg added a comment.

I think MMX code is obscure enough at this point that it doesn't matter much 
either way. Even less across DSO boundaries. That's why I don't really care 
either way.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-04 Thread Kamil Rytarowski via Phabricator via cfe-commits

krytarowski added a comment.

In D59744#1529218 , @mgorny wrote:

> In D59744#1529182 , @krytarowski 
> wrote:
>
> > In D59744#1527412 , @wxiao3 wrote:
> >
> > > Consider other Systems (e.g Darwin, PS4 and FreeBSD) don't want to spend 
> > > any effort dealing with the ramifications of ABI breaks (as discussed in 
> > > https://reviews.llvm.org/D60748) at present, I only fix the bug for 
> > > Linux. If other system wants the fix, the only thing needed is to add a 
> > > flag (like "IsLinuxABI" ) to enable it.
> >
> >
> > CC @mgorny and @joerg - do we want this for NetBSD?
>
>
> Probably yes. FWICS, gcc uses `%mm0` and `%mm1` on NetBSD while clang doesn't.


Unless Joerg will protest, @wxiao3 please enable it on NetBSD as well.. but 
personally I would enable it unconditionally for all sysv ABIs.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-04 Thread Michał Górny via Phabricator via cfe-commits

mgorny added a comment.

In D59744#1529182 , @krytarowski wrote:

> In D59744#1527412 , @wxiao3 wrote:
>
> > Consider other Systems (e.g Darwin, PS4 and FreeBSD) don't want to spend 
> > any effort dealing with the ramifications of ABI breaks (as discussed in 
> > https://reviews.llvm.org/D60748) at present, I only fix the bug for Linux. 
> > If other system wants the fix, the only thing needed is to add a flag (like 
> > "IsLinuxABI" ) to enable it.
>
>
> CC @mgorny and @joerg - do we want this for NetBSD?


Probably yes. FWICS, gcc uses `%mm0` and `%mm1` on NetBSD while clang doesn't.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-04 Thread Kamil Rytarowski via Phabricator via cfe-commits

krytarowski added subscribers: mgorny, joerg.
krytarowski added a comment.

In D59744#1527412 , @wxiao3 wrote:

> Consider other Systems (e.g Darwin, PS4 and FreeBSD) don't want to spend any 
> effort dealing with the ramifications of ABI breaks (as discussed in 
> https://reviews.llvm.org/D60748) at present, I only fix the bug for Linux. If 
> other system wants the fix, the only thing needed is to add a flag (like 
> "IsLinuxABI" ) to enable it.


CC @mgorny and @joerg - do we want this for NetBSD?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-03 Thread Wei Xiao via Phabricator via cfe-commits

wxiao3 added a comment.

Consider other Systems (e.g Darwin, PS4 and FreeBSD) don't want to spend any 
effort dealing with the ramifications of ABI breaks (as discussed in 
https://reviews.llvm.org/D60748) at present, I only fix the bug for Linux. If 
other system wants the fix, the only thing needed is to add a flag (like 
"IsLinuxABI" ) to enable it.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-02 Thread Kamil Rytarowski via Phabricator via cfe-commits

krytarowski added a comment.

sysv abi is not only for UNIX but most non-Windows ones (BSDs, HAIKU, ...).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-01 Thread Wei Xiao via Phabricator via cfe-commits

wxiao3 added a comment.

Hi all,

With the latest version, I have made below changes according to all your 
comments:

1. Only apply the fix to Linux where many libraries are built by GCC.
2. Avoid converting the QualType to llvm::Type and then checking if the 
llvm::Type is a 64-bit vector, which is unnecessary and inefficient. 
Furthermore, I remove the utility function: IsX86_MMXType since It's very 
simple to check _m64 type based on QualType.

Ok for merge now?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-01 Thread Wei Xiao via Phabricator via cfe-commits

wxiao3 updated this revision to Diff 202579.
wxiao3 edited the summary of this revision.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744

Files:
  lib/CodeGen/TargetInfo.cpp
  test/CodeGen/x86_32-arguments-linux.c
  test/CodeGen/x86_32-m64.c

Index: test/CodeGen/x86_32-m64.c
===
--- /dev/null
+++ test/CodeGen/x86_32-m64.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-linux-gnu -target-cpu pentium4 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,LINUX
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-apple-darwin9 -target-cpu yonah -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,DARWIN
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-elfiamcu -mfloat-abi soft -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,IAMCU
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-win32 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,WIN32
+
+#include 
+__m64 m64;
+void callee(__m64 __m1, __m64 __m2);
+__m64 caller(__m64 __m1, __m64 __m2)
+{
+// LINUX-LABEL: define x86_mmx @caller(x86_mmx %__m1.coerce, x86_mmx %__m2.coerce)
+// LINUX: tail call void @callee(x86_mmx %__m2.coerce, x86_mmx %__m1.coerce)
+// LINUX: ret x86_mmx
+// DARWIN-LABEL: define i64 @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// DARWIN: tail call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// DARWIN: ret i64
+// IAMCU-LABEL: define <1 x i64> @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// IAMCU: tail call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// IAMCU: ret <1 x i64>
+// WIN32-LABEL: define dso_local <1 x i64> @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// WIN32: call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// WIN32: ret <1 x i64>
+  callee(__m2, __m1);
+  return m64;
+}
Index: test/CodeGen/x86_32-arguments-linux.c
===
--- test/CodeGen/x86_32-arguments-linux.c
+++ test/CodeGen/x86_32-arguments-linux.c
@@ -3,7 +3,7 @@
 
 // CHECK-LABEL: define void @f56(
 // CHECK: i8 signext %a0, %struct.s56_0* byval align 4 %a1,
-// CHECK: i64 %a2.coerce, %struct.s56_1* byval align 4,
+// CHECK: x86_mmx %a2.coerce, %struct.s56_1* byval align 4,
 // CHECK: <1 x double> %a4, %struct.s56_2* byval align 4,
 // CHECK: <4 x i32> %a6, %struct.s56_3* byval align 4,
 // CHECK: <2 x double> %a8, %struct.s56_4* byval align 4,
@@ -12,7 +12,7 @@
 
 // CHECK: call void (i32, ...) @f56_0(i32 1,
 // CHECK: i32 %{{.*}}, %struct.s56_0* byval align 4 %{{[^ ]*}},
-// CHECK: i64 %{{[^ ]*}}, %struct.s56_1* byval align 4 %{{[^ ]*}},
+// CHECK: x86_mmx %{{[^ ]*}}, %struct.s56_1* byval align 4 %{{[^ ]*}},
 // CHECK: <1 x double> %{{[^ ]*}}, %struct.s56_2* byval align 4 %{{[^ ]*}},
 // CHECK: <4 x i32> %{{[^ ]*}}, %struct.s56_3* byval align 4 %{{[^ ]*}},
 // CHECK: <2 x double> %{{[^ ]*}}, %struct.s56_4* byval align 4 %{{[^ ]*}},
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -915,14 +915,6 @@
: ABIArgInfo::getDirect());
 }
 
-/// IsX86_MMXType - Return true if this is an MMX type.
-bool IsX86_MMXType(llvm::Type *IRType) {
-  // Return true if the type is an MMX type <2 x i32>, <4 x i16>, or <8 x i8>.
-  return IRType->isVectorTy() && IRType->getPrimitiveSizeInBits() == 64 &&
-cast(IRType)->getElementType()->isIntegerTy() &&
-IRType->getScalarSizeInBits() != 64;
-}
-
 static llvm::Type* X86AdjustInlineAsmType(CodeGen::CodeGenFunction ,
   StringRef Constraint,
   llvm::Type* Ty) {
@@ -1010,7 +1002,9 @@
   bool IsWin32StructABI;
   bool IsSoftFloatABI;
   bool IsMCUABI;
+  bool IsLinuxABI;
   unsigned DefaultNumRegisterParameters;
+  bool IsMMXEnabled;
 
   static bool isRegisterSize(unsigned Size) {
 return (Size == 8 || Size == 16 || Size == 32 || Size == 64);
@@ -1070,13 +1064,16 @@
 
   X86_32ABIInfo(CodeGen::CodeGenTypes , bool DarwinVectorABI,
 bool RetSmallStructInRegABI, bool Win32StructABI,
-unsigned NumRegisterParameters, bool SoftFloatABI)
+unsigned NumRegisterParameters, bool SoftFloatABI,
+bool MMXEnabled)
 : SwiftABIInfo(CGT), IsDarwinVectorABI(DarwinVectorABI),
   IsRetSmallStructInRegABI(RetSmallStructInRegABI),
   IsWin32StructABI(Win32StructABI),
   IsSoftFloatABI(SoftFloatABI),
   IsMCUABI(CGT.getTarget().getTriple().isOSIAMCU()),
-  DefaultNumRegisterParameters(NumRegisterParameters) {}
+  IsLinuxABI(CGT.getTarget().getTriple().isOSLinux()),
+  DefaultNumRegisterParameters(NumRegisterParameters),
+  IsMMXEnabled(MMXEnabled) {}
 
   bool shouldPassIndirectlyForSwift(ArrayRef scalars,
 bool asReturnValue) const override {
@@

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-05-13 Thread Wei Xiao via Phabricator via cfe-commits

wxiao3 marked an inline comment as done.
wxiao3 added inline comments.



Comment at: lib/CodeGen/TargetInfo.cpp:919
 /// IsX86_MMXType - Return true if this is an MMX type.
 bool IsX86_MMXType(llvm::Type *IRType) {
-  // Return true if the type is an MMX type <2 x i32>, <4 x i16>, or <8 x i8>.

rnk wrote:
> wxiao3 wrote:
> > rnk wrote:
> > > I think looking at the LLVM type to decide how something should be passed 
> > > is a bad pattern to follow. We should look at the clang AST to decide how 
> > > things will be passed, not LLVM types. Would that be complicated? Are 
> > > there aggregate types that end up getting passed directly in MMX 
> > > registers?
> > For x86 32 bit target, no aggregate types end up getting passed in MMX 
> > register.
> > The only type passed by MMX is 
> > 
> > > __m64
> > 
> >  which is defined in header file (mmintrin.h):
> > 
> > 
> > ```
> > typedef long long __m64 __attribute__((__vector_size__(8), __aligned__(8)));
> > ```
> > 
> > Yes, it would be good if we define _m64 as a builtin type and handle it in 
> > AST level. But I'm afraid that it won't be a trivial work. Since GCC also 
> > handles __m64 in the same way as Clang currently does, can we just keep 
> > current implementation as it is?
> > 
> That's not quite what I'm suggesting. I'm saying that IsX86_MMXType should 
> take a QualType parameter, and it should check if that qualtype looks like 
> the __m64 vector type, instead of converting the QualType to llvm::Type and 
> then checking if the llvm::Type is a 64-bit vector. Does that seem 
> reasonable? See the code near the call site conditionalized on 
> IsDarwinVectorABI which already has similar logic.
Yes, it's unnecessary to convert QualType to llvm::Type just for the _m64 
vector type checking.
Since It's very simple to check _m64 type based on QualType with 
pre-conditioned type assertion 
```
if (const VectorType *VT = RetTy->getAs())
```
I just remove the utility function: IsX86_MMXType.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-05-13 Thread Wei Xiao via Phabricator via cfe-commits

wxiao3 updated this revision to Diff 199233.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744

Files:
  lib/CodeGen/TargetInfo.cpp
  test/CodeGen/vector.c
  test/CodeGen/x86_32-arguments-darwin.c
  test/CodeGen/x86_32-arguments-linux.c
  test/CodeGen/x86_32-m64.c

Index: test/CodeGen/x86_32-m64.c
===
--- /dev/null
+++ test/CodeGen/x86_32-m64.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-linux-gnu -target-cpu pentium4 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,LINUX
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-apple-darwin9 -target-cpu yonah -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,DARWIN
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-elfiamcu -mfloat-abi soft -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,IAMCU
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-win32 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,WIN32
+
+#include 
+__m64 m64;
+void callee(__m64 __m1, __m64 __m2);
+__m64 caller(__m64 __m1, __m64 __m2)
+{
+// LINUX-LABEL: define x86_mmx @caller(x86_mmx %__m1.coerce, x86_mmx %__m2.coerce)
+// LINUX: tail call void @callee(x86_mmx %__m2.coerce, x86_mmx %__m1.coerce)
+// LINUX: ret x86_mmx
+// DARWIN-LABEL: define i64 @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// DARWIN: tail call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// DARWIN: ret i64
+// IAMCU-LABEL: define <1 x i64> @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// IAMCU: tail call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// IAMCU: ret <1 x i64>
+// WIN32-LABEL: define dso_local <1 x i64> @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// WIN32: call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// WIN32: ret <1 x i64>
+  callee(__m2, __m1);
+  return m64;
+}
Index: test/CodeGen/x86_32-arguments-linux.c
===
--- test/CodeGen/x86_32-arguments-linux.c
+++ test/CodeGen/x86_32-arguments-linux.c
@@ -3,7 +3,7 @@
 
 // CHECK-LABEL: define void @f56(
 // CHECK: i8 signext %a0, %struct.s56_0* byval align 4 %a1,
-// CHECK: i64 %a2.coerce, %struct.s56_1* byval align 4,
+// CHECK: x86_mmx %a2.coerce, %struct.s56_1* byval align 4,
 // CHECK: <1 x double> %a4, %struct.s56_2* byval align 4,
 // CHECK: <4 x i32> %a6, %struct.s56_3* byval align 4,
 // CHECK: <2 x double> %a8, %struct.s56_4* byval align 4,
@@ -12,7 +12,7 @@
 
 // CHECK: call void (i32, ...) @f56_0(i32 1,
 // CHECK: i32 %{{.*}}, %struct.s56_0* byval align 4 %{{[^ ]*}},
-// CHECK: i64 %{{[^ ]*}}, %struct.s56_1* byval align 4 %{{[^ ]*}},
+// CHECK: x86_mmx %{{[^ ]*}}, %struct.s56_1* byval align 4 %{{[^ ]*}},
 // CHECK: <1 x double> %{{[^ ]*}}, %struct.s56_2* byval align 4 %{{[^ ]*}},
 // CHECK: <4 x i32> %{{[^ ]*}}, %struct.s56_3* byval align 4 %{{[^ ]*}},
 // CHECK: <2 x double> %{{[^ ]*}}, %struct.s56_4* byval align 4 %{{[^ ]*}},
Index: test/CodeGen/x86_32-arguments-darwin.c
===
--- test/CodeGen/x86_32-arguments-darwin.c
+++ test/CodeGen/x86_32-arguments-darwin.c
@@ -229,7 +229,7 @@
 
 // CHECK-LABEL: define void @f56(
 // CHECK: i8 signext %a0, %struct.s56_0* byval align 4 %a1,
-// CHECK: i64 %a2.coerce, %struct.s56_1* byval align 4,
+// CHECK: x86_mmx %a2.coerce, %struct.s56_1* byval align 4,
 // CHECK: i64 %a4.coerce, %struct.s56_2* byval align 4,
 // CHECK: <4 x i32> %a6, %struct.s56_3* byval align 16 %a7,
 // CHECK: <2 x double> %a8, %struct.s56_4* byval align 16 %a9,
@@ -238,7 +238,7 @@
 
 // CHECK:   call void (i32, ...) @f56_0(i32 1,
 // CHECK: i32 %{{[^ ]*}}, %struct.s56_0* byval align 4 %{{[^ ]*}},
-// CHECK: i64 %{{[^ ]*}}, %struct.s56_1* byval align 4 %{{[^ ]*}},
+// CHECK: x86_mmx %{{[^ ]*}}, %struct.s56_1* byval align 4 %{{[^ ]*}},
 // CHECK: i64 %{{[^ ]*}}, %struct.s56_2* byval align 4 %{{[^ ]*}},
 // CHECK: <4 x i32> %{{[^ ]*}}, %struct.s56_3* byval align 16 %{{[^ ]*}},
 // CHECK: <2 x double> %{{[^ ]*}}, %struct.s56_4* byval align 16 %{{[^ ]*}},
Index: test/CodeGen/vector.c
===
--- test/CodeGen/vector.c
+++ test/CodeGen/vector.c
@@ -78,5 +78,5 @@
   return y;
 }
 
-// CHECK: define void @lax_vector_compare2(<2 x i32>* {{.*sret.*}}, i64 {{.*}}, i64 {{.*}})
+// CHECK: define void @lax_vector_compare2(<2 x i32>* {{.*sret.*}}, i64 {{.*}}, x86_mmx {{.*}})
 // CHECK: icmp eq <2 x i32>
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -915,14 +915,6 @@
: ABIArgInfo::getDirect());
 }
 
-/// IsX86_MMXType - Return true if this is an MMX type.
-bool IsX86_MMXType(llvm::Type *IRType) {
-  // Return true if the type is an MMX type <2 x i32>, <4 x i16>, or <8 x i8>.
-  return IRType->isVectorTy() &&

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-05-01 Thread Reid Kleckner via Phabricator via cfe-commits

rnk added inline comments.



Comment at: lib/CodeGen/TargetInfo.cpp:919
 /// IsX86_MMXType - Return true if this is an MMX type.
 bool IsX86_MMXType(llvm::Type *IRType) {
-  // Return true if the type is an MMX type <2 x i32>, <4 x i16>, or <8 x i8>.

wxiao3 wrote:
> rnk wrote:
> > I think looking at the LLVM type to decide how something should be passed 
> > is a bad pattern to follow. We should look at the clang AST to decide how 
> > things will be passed, not LLVM types. Would that be complicated? Are there 
> > aggregate types that end up getting passed directly in MMX registers?
> For x86 32 bit target, no aggregate types end up getting passed in MMX 
> register.
> The only type passed by MMX is 
> 
> > __m64
> 
>  which is defined in header file (mmintrin.h):
> 
> 
> ```
> typedef long long __m64 __attribute__((__vector_size__(8), __aligned__(8)));
> ```
> 
> Yes, it would be good if we define _m64 as a builtin type and handle it in 
> AST level. But I'm afraid that it won't be a trivial work. Since GCC also 
> handles __m64 in the same way as Clang currently does, can we just keep 
> current implementation as it is?
> 
That's not quite what I'm suggesting. I'm saying that IsX86_MMXType should take 
a QualType parameter, and it should check if that qualtype looks like the __m64 
vector type, instead of converting the QualType to llvm::Type and then checking 
if the llvm::Type is a 64-bit vector. Does that seem reasonable? See the code 
near the call site conditionalized on IsDarwinVectorABI which already has 
similar logic.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-04-27 Thread Wei Xiao via Phabricator via cfe-commits

wxiao3 marked 2 inline comments as done.
wxiao3 added inline comments.



Comment at: lib/CodeGen/TargetInfo.cpp:919
 /// IsX86_MMXType - Return true if this is an MMX type.
 bool IsX86_MMXType(llvm::Type *IRType) {
-  // Return true if the type is an MMX type <2 x i32>, <4 x i16>, or <8 x i8>.

rnk wrote:
> I think looking at the LLVM type to decide how something should be passed is 
> a bad pattern to follow. We should look at the clang AST to decide how things 
> will be passed, not LLVM types. Would that be complicated? Are there 
> aggregate types that end up getting passed directly in MMX registers?
For x86 32 bit target, no aggregate types end up getting passed in MMX register.
The only type passed by MMX is 

> __m64

 which is defined in header file (mmintrin.h):


```
typedef long long __m64 __attribute__((__vector_size__(8), __aligned__(8)));
```

Yes, it would be good if we define _m64 as a builtin type and handle it in AST 
level. But I'm afraid that it won't be a trivial work. Since GCC also handles 
__m64 in the same way as Clang currently does, can we just keep current 
implementation as it is?




Comment at: lib/CodeGen/TargetInfo.cpp:9489
+  // System V i386 ABI requires __m64 value passing by MMX registers.
+  bool EnableMMX = getContext().getTargetInfo().getABI() != "no-mmx";
   return SetCGInfo(new X86_32TargetCodeGenInfo(

rnk wrote:
> I think this needs to preserve existing behavior for Darwin and PS4 based on 
> comments from @rjmccall and @dexonsmith in D60748.
ok, I will follow it.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-04-22 Thread Reid Kleckner via Phabricator via cfe-commits

rnk added subscribers: dexonsmith, rjmccall.
rnk added inline comments.



Comment at: lib/CodeGen/TargetInfo.cpp:919
 /// IsX86_MMXType - Return true if this is an MMX type.
 bool IsX86_MMXType(llvm::Type *IRType) {
-  // Return true if the type is an MMX type <2 x i32>, <4 x i16>, or <8 x i8>.

I think looking at the LLVM type to decide how something should be passed is a 
bad pattern to follow. We should look at the clang AST to decide how things 
will be passed, not LLVM types. Would that be complicated? Are there aggregate 
types that end up getting passed directly in MMX registers?



Comment at: lib/CodeGen/TargetInfo.cpp:9489
+  // System V i386 ABI requires __m64 value passing by MMX registers.
+  bool EnableMMX = getContext().getTargetInfo().getABI() != "no-mmx";
   return SetCGInfo(new X86_32TargetCodeGenInfo(

I think this needs to preserve existing behavior for Darwin and PS4 based on 
comments from @rjmccall and @dexonsmith in D60748.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-04-17 Thread Wei Xiao via Phabricator via cfe-commits

wxiao3 updated this revision to Diff 195660.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744

Files:
  lib/CodeGen/TargetInfo.cpp
  test/CodeGen/vector.c
  test/CodeGen/x86_32-arguments-darwin.c
  test/CodeGen/x86_32-arguments-linux.c
  test/CodeGen/x86_32-m64.c

Index: test/CodeGen/x86_32-m64.c
===
--- /dev/null
+++ test/CodeGen/x86_32-m64.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-linux-gnu -target-cpu pentium4 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,LINUX
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-apple-darwin9 -target-cpu yonah -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,DARWIN
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-elfiamcu -mfloat-abi soft -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,IAMCU
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-win32 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,WIN32
+
+#include 
+__m64 m64;
+void callee(__m64 __m1, __m64 __m2);
+__m64 caller(__m64 __m1, __m64 __m2)
+{
+// LINUX-LABEL: define x86_mmx @caller(x86_mmx %__m1.coerce, x86_mmx %__m2.coerce)
+// LINUX: tail call void @callee(x86_mmx %__m2.coerce, x86_mmx %__m1.coerce)
+// LINUX: ret x86_mmx
+// DARWIN-LABEL: define i64 @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// DARWIN: tail call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// DARWIN: ret i64
+// IAMCU-LABEL: define <1 x i64> @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// IAMCU: tail call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// IAMCU: ret <1 x i64>
+// WIN32-LABEL: define dso_local <1 x i64> @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// WIN32: call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// WIN32: ret <1 x i64>
+  callee(__m2, __m1);
+  return m64;
+}
Index: test/CodeGen/x86_32-arguments-linux.c
===
--- test/CodeGen/x86_32-arguments-linux.c
+++ test/CodeGen/x86_32-arguments-linux.c
@@ -3,7 +3,7 @@
 
 // CHECK-LABEL: define void @f56(
 // CHECK: i8 signext %a0, %struct.s56_0* byval align 4 %a1,
-// CHECK: i64 %a2.coerce, %struct.s56_1* byval align 4,
+// CHECK: x86_mmx %a2.coerce, %struct.s56_1* byval align 4,
 // CHECK: <1 x double> %a4, %struct.s56_2* byval align 4,
 // CHECK: <4 x i32> %a6, %struct.s56_3* byval align 4,
 // CHECK: <2 x double> %a8, %struct.s56_4* byval align 4,
@@ -12,7 +12,7 @@
 
 // CHECK: call void (i32, ...) @f56_0(i32 1,
 // CHECK: i32 %{{.*}}, %struct.s56_0* byval align 4 %{{[^ ]*}},
-// CHECK: i64 %{{[^ ]*}}, %struct.s56_1* byval align 4 %{{[^ ]*}},
+// CHECK: x86_mmx %{{[^ ]*}}, %struct.s56_1* byval align 4 %{{[^ ]*}},
 // CHECK: <1 x double> %{{[^ ]*}}, %struct.s56_2* byval align 4 %{{[^ ]*}},
 // CHECK: <4 x i32> %{{[^ ]*}}, %struct.s56_3* byval align 4 %{{[^ ]*}},
 // CHECK: <2 x double> %{{[^ ]*}}, %struct.s56_4* byval align 4 %{{[^ ]*}},
Index: test/CodeGen/x86_32-arguments-darwin.c
===
--- test/CodeGen/x86_32-arguments-darwin.c
+++ test/CodeGen/x86_32-arguments-darwin.c
@@ -229,7 +229,7 @@
 
 // CHECK-LABEL: define void @f56(
 // CHECK: i8 signext %a0, %struct.s56_0* byval align 4 %a1,
-// CHECK: i64 %a2.coerce, %struct.s56_1* byval align 4,
+// CHECK: x86_mmx %a2.coerce, %struct.s56_1* byval align 4,
 // CHECK: i64 %a4.coerce, %struct.s56_2* byval align 4,
 // CHECK: <4 x i32> %a6, %struct.s56_3* byval align 16 %a7,
 // CHECK: <2 x double> %a8, %struct.s56_4* byval align 16 %a9,
@@ -238,7 +238,7 @@
 
 // CHECK:   call void (i32, ...) @f56_0(i32 1,
 // CHECK: i32 %{{[^ ]*}}, %struct.s56_0* byval align 4 %{{[^ ]*}},
-// CHECK: i64 %{{[^ ]*}}, %struct.s56_1* byval align 4 %{{[^ ]*}},
+// CHECK: x86_mmx %{{[^ ]*}}, %struct.s56_1* byval align 4 %{{[^ ]*}},
 // CHECK: i64 %{{[^ ]*}}, %struct.s56_2* byval align 4 %{{[^ ]*}},
 // CHECK: <4 x i32> %{{[^ ]*}}, %struct.s56_3* byval align 16 %{{[^ ]*}},
 // CHECK: <2 x double> %{{[^ ]*}}, %struct.s56_4* byval align 16 %{{[^ ]*}},
Index: test/CodeGen/vector.c
===
--- test/CodeGen/vector.c
+++ test/CodeGen/vector.c
@@ -78,5 +78,5 @@
   return y;
 }
 
-// CHECK: define void @lax_vector_compare2(<2 x i32>* {{.*sret.*}}, i64 {{.*}}, i64 {{.*}})
+// CHECK: define void @lax_vector_compare2(<2 x i32>* {{.*sret.*}}, i64 {{.*}}, x86_mmx {{.*}})
 // CHECK: icmp eq <2 x i32>
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -917,10 +917,10 @@
 
 /// IsX86_MMXType - Return true if this is an MMX type.
 bool IsX86_MMXType(llvm::Type *IRType) {
-  // Return true if the type is an MMX type <2 x i32>, <4 x i16>, or <8 x i8>.
+  // Return true if the type is an MMX type <1 x i64>, <2 x i32>, <4 x i16>,
+  // or <8 x i8>.
   return IRType->isVectorTy() &&

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-04-17 Thread Wei Xiao via Phabricator via cfe-commits

wxiao3 updated this revision to Diff 195659.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744

Files:
  lib/CodeGen/TargetInfo.cpp
  test/CodeGen/x86_32-arguments-linux.c
  test/CodeGen/x86_32-mmx-linux.c

Index: test/CodeGen/x86_32-mmx-linux.c
===
--- /dev/null
+++ test/CodeGen/x86_32-mmx-linux.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-linux-gnu -target-cpu pentium4 -emit-llvm -o %t %s
+// RUN: FileCheck < %t %s
+
+// CHECK-LABEL: define x86_mmx @caller(x86_mmx %__m1.coerce, x86_mmx %__m2.coerce)
+// CHECK: tail call void @callee(x86_mmx %__m2.coerce, x86_mmx %__m1.coerce)
+// CHECK: ret x86_mmx
+
+#include 
+__m64 m64;
+void callee(__m64 __m1, __m64 __m2);
+__m64 caller(__m64 __m1, __m64 __m2)
+{
+  callee(__m2, __m1);
+  return m64;
+}
Index: test/CodeGen/x86_32-arguments-linux.c
===
--- test/CodeGen/x86_32-arguments-linux.c
+++ test/CodeGen/x86_32-arguments-linux.c
@@ -3,7 +3,7 @@
 
 // CHECK-LABEL: define void @f56(
 // CHECK: i8 signext %a0, %struct.s56_0* byval align 4 %a1,
-// CHECK: i64 %a2.coerce, %struct.s56_1* byval align 4,
+// CHECK: x86_mmx %a2.coerce, %struct.s56_1* byval align 4,
 // CHECK: <1 x double> %a4, %struct.s56_2* byval align 4,
 // CHECK: <4 x i32> %a6, %struct.s56_3* byval align 4,
 // CHECK: <2 x double> %a8, %struct.s56_4* byval align 4,
@@ -12,7 +12,7 @@
 
 // CHECK: call void (i32, ...) @f56_0(i32 1,
 // CHECK: i32 %{{.*}}, %struct.s56_0* byval align 4 %{{[^ ]*}},
-// CHECK: i64 %{{[^ ]*}}, %struct.s56_1* byval align 4 %{{[^ ]*}},
+// CHECK: x86_mmx %{{[^ ]*}}, %struct.s56_1* byval align 4 %{{[^ ]*}},
 // CHECK: <1 x double> %{{[^ ]*}}, %struct.s56_2* byval align 4 %{{[^ ]*}},
 // CHECK: <4 x i32> %{{[^ ]*}}, %struct.s56_3* byval align 4 %{{[^ ]*}},
 // CHECK: <2 x double> %{{[^ ]*}}, %struct.s56_4* byval align 4 %{{[^ ]*}},
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -917,10 +917,10 @@
 
 /// IsX86_MMXType - Return true if this is an MMX type.
 bool IsX86_MMXType(llvm::Type *IRType) {
-  // Return true if the type is an MMX type <2 x i32>, <4 x i16>, or <8 x i8>.
+  // Return true if the type is an MMX type <1 x i64>, <2 x i32>, <4 x i16>,
+  // or <8 x i8>.
   return IRType->isVectorTy() && IRType->getPrimitiveSizeInBits() == 64 &&
-cast(IRType)->getElementType()->isIntegerTy() &&
-IRType->getScalarSizeInBits() != 64;
+cast(IRType)->getElementType()->isIntegerTy();
 }
 
 static llvm::Type* X86AdjustInlineAsmType(CodeGen::CodeGenFunction ,
@@ -1011,6 +1011,7 @@
   bool IsSoftFloatABI;
   bool IsMCUABI;
   unsigned DefaultNumRegisterParameters;
+  bool IsMMXEnabled;
 
   static bool isRegisterSize(unsigned Size) {
 return (Size == 8 || Size == 16 || Size == 32 || Size == 64);
@@ -1070,13 +1071,15 @@
 
   X86_32ABIInfo(CodeGen::CodeGenTypes , bool DarwinVectorABI,
 bool RetSmallStructInRegABI, bool Win32StructABI,
-unsigned NumRegisterParameters, bool SoftFloatABI)
+unsigned NumRegisterParameters, bool SoftFloatABI,
+bool MMXEnabled)
 : SwiftABIInfo(CGT), IsDarwinVectorABI(DarwinVectorABI),
   IsRetSmallStructInRegABI(RetSmallStructInRegABI),
   IsWin32StructABI(Win32StructABI),
   IsSoftFloatABI(SoftFloatABI),
   IsMCUABI(CGT.getTarget().getTriple().isOSIAMCU()),
-  DefaultNumRegisterParameters(NumRegisterParameters) {}
+  DefaultNumRegisterParameters(NumRegisterParameters),
+  IsMMXEnabled(MMXEnabled) {}
 
   bool shouldPassIndirectlyForSwift(ArrayRef scalars,
 bool asReturnValue) const override {
@@ -1097,10 +1100,11 @@
 public:
   X86_32TargetCodeGenInfo(CodeGen::CodeGenTypes , bool DarwinVectorABI,
   bool RetSmallStructInRegABI, bool Win32StructABI,
-  unsigned NumRegisterParameters, bool SoftFloatABI)
+  unsigned NumRegisterParameters, bool SoftFloatABI,
+  bool MMXEnabled = false)
   : TargetCodeGenInfo(new X86_32ABIInfo(
 CGT, DarwinVectorABI, RetSmallStructInRegABI, Win32StructABI,
-NumRegisterParameters, SoftFloatABI)) {}
+NumRegisterParameters, SoftFloatABI, MMXEnabled)) {}
 
   static bool isStructReturnInRegABI(
   const llvm::Triple , const CodeGenOptions );
@@ -1407,6 +1411,9 @@
   return getIndirectReturnResult(RetTy, State);
 }
 
+if (IsMMXEnabled && IsX86_MMXType(CGT.ConvertType(RetTy)))
+  return ABIArgInfo::getDirect(llvm::Type::getX86_MMXTy(getVMContext()));
+
 return ABIArgInfo::getDirect();
   }
 
@@ -1711,8 +1718,11 @@
 Size));
 }
 
-if

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-04-17 Thread Simon Pilgrim via Phabricator via cfe-commits

RKSimon added a comment.

One last style comment from me but we need somebody better with the different 
ABIs to finally approve this.




Comment at: lib/CodeGen/TargetInfo.cpp:1416
+  return ABIArgInfo::getDirect(llvm::Type::getX86_MMXTy(getVMContext()));
+}
+

superfluous braces?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-04-15 Thread Wei Xiao via Phabricator via cfe-commits

wxiao3 updated this revision to Diff 195291.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744

Files:
  lib/CodeGen/TargetInfo.cpp
  test/CodeGen/vector.c
  test/CodeGen/x86_32-arguments-darwin.c
  test/CodeGen/x86_32-arguments-linux.c
  test/CodeGen/x86_32-m64.c

Index: test/CodeGen/x86_32-m64.c
===
--- /dev/null
+++ test/CodeGen/x86_32-m64.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-linux-gnu -target-cpu pentium4 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,LINUX
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-apple-darwin9 -target-cpu yonah -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,DARWIN
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-elfiamcu -mfloat-abi soft -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,IAMCU
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-win32 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,WIN32
+
+#include 
+__m64 m64;
+void callee(__m64 __m1, __m64 __m2);
+__m64 caller(__m64 __m1, __m64 __m2)
+{
+// LINUX-LABEL: define x86_mmx @caller(x86_mmx %__m1.coerce, x86_mmx %__m2.coerce)
+// LINUX: tail call void @callee(x86_mmx %__m2.coerce, x86_mmx %__m1.coerce)
+// LINUX: ret x86_mmx
+// DARWIN-LABEL: define i64 @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// DARWIN: tail call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// DARWIN: ret i64
+// IAMCU-LABEL: define <1 x i64> @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// IAMCU: tail call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// IAMCU: ret <1 x i64>
+// WIN32-LABEL: define dso_local <1 x i64> @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// WIN32: call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// WIN32: ret <1 x i64>
+  callee(__m2, __m1);
+  return m64;
+}
Index: test/CodeGen/x86_32-arguments-linux.c
===
--- test/CodeGen/x86_32-arguments-linux.c
+++ test/CodeGen/x86_32-arguments-linux.c
@@ -3,7 +3,7 @@
 
 // CHECK-LABEL: define void @f56(
 // CHECK: i8 signext %a0, %struct.s56_0* byval align 4 %a1,
-// CHECK: i64 %a2.coerce, %struct.s56_1* byval align 4,
+// CHECK: x86_mmx %a2.coerce, %struct.s56_1* byval align 4,
 // CHECK: <1 x double> %a4, %struct.s56_2* byval align 4,
 // CHECK: <4 x i32> %a6, %struct.s56_3* byval align 4,
 // CHECK: <2 x double> %a8, %struct.s56_4* byval align 4,
@@ -12,7 +12,7 @@
 
 // CHECK: call void (i32, ...) @f56_0(i32 1,
 // CHECK: i32 %{{.*}}, %struct.s56_0* byval align 4 %{{[^ ]*}},
-// CHECK: i64 %{{[^ ]*}}, %struct.s56_1* byval align 4 %{{[^ ]*}},
+// CHECK: x86_mmx %{{[^ ]*}}, %struct.s56_1* byval align 4 %{{[^ ]*}},
 // CHECK: <1 x double> %{{[^ ]*}}, %struct.s56_2* byval align 4 %{{[^ ]*}},
 // CHECK: <4 x i32> %{{[^ ]*}}, %struct.s56_3* byval align 4 %{{[^ ]*}},
 // CHECK: <2 x double> %{{[^ ]*}}, %struct.s56_4* byval align 4 %{{[^ ]*}},
Index: test/CodeGen/x86_32-arguments-darwin.c
===
--- test/CodeGen/x86_32-arguments-darwin.c
+++ test/CodeGen/x86_32-arguments-darwin.c
@@ -229,7 +229,7 @@
 
 // CHECK-LABEL: define void @f56(
 // CHECK: i8 signext %a0, %struct.s56_0* byval align 4 %a1,
-// CHECK: i64 %a2.coerce, %struct.s56_1* byval align 4,
+// CHECK: x86_mmx %a2.coerce, %struct.s56_1* byval align 4,
 // CHECK: i64 %a4.coerce, %struct.s56_2* byval align 4,
 // CHECK: <4 x i32> %a6, %struct.s56_3* byval align 16 %a7,
 // CHECK: <2 x double> %a8, %struct.s56_4* byval align 16 %a9,
@@ -238,7 +238,7 @@
 
 // CHECK:   call void (i32, ...) @f56_0(i32 1,
 // CHECK: i32 %{{[^ ]*}}, %struct.s56_0* byval align 4 %{{[^ ]*}},
-// CHECK: i64 %{{[^ ]*}}, %struct.s56_1* byval align 4 %{{[^ ]*}},
+// CHECK: x86_mmx %{{[^ ]*}}, %struct.s56_1* byval align 4 %{{[^ ]*}},
 // CHECK: i64 %{{[^ ]*}}, %struct.s56_2* byval align 4 %{{[^ ]*}},
 // CHECK: <4 x i32> %{{[^ ]*}}, %struct.s56_3* byval align 16 %{{[^ ]*}},
 // CHECK: <2 x double> %{{[^ ]*}}, %struct.s56_4* byval align 16 %{{[^ ]*}},
Index: test/CodeGen/vector.c
===
--- test/CodeGen/vector.c
+++ test/CodeGen/vector.c
@@ -78,5 +78,5 @@
   return y;
 }
 
-// CHECK: define void @lax_vector_compare2(<2 x i32>* {{.*sret.*}}, i64 {{.*}}, i64 {{.*}})
+// CHECK: define void @lax_vector_compare2(<2 x i32>* {{.*sret.*}}, i64 {{.*}}, x86_mmx {{.*}})
 // CHECK: icmp eq <2 x i32>
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -917,10 +917,10 @@
 
 /// IsX86_MMXType - Return true if this is an MMX type.
 bool IsX86_MMXType(llvm::Type *IRType) {
-  // Return true if the type is an MMX type <2 x i32>, <4 x i16>, or <8 x i8>.
+  // Return true if the type is an MMX type <1 x i64>, <2 x i32>, <4 x i16>,
+  // or <8 x i8>.
   return IRType->isVectorTy() &&

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-04-15 Thread Simon Pilgrim via Phabricator via cfe-commits

RKSimon added inline comments.



Comment at: test/CodeGen/x86_32-m64-darwin.c:1
+// RUN: %clang_cc1 -w -fblocks -triple i386-apple-darwin9 -target-cpu yonah 
-target-feature +mmx -emit-llvm -O2 -o - %s | FileCheck %s
+

You should be able to merge all of these triples into the same test file, each 
with their own RUN: line, you will need to add a FileCheck prefix, something 
like:
```
| FileCheck %s --check-prefixes=CHECK,DARWIN
| FileCheck %s --check-prefixes=CHECK,IAMCU
| FileCheck %s --check-prefixes=CHECK,LINUX
| FileCheck %s --check-prefixes=CHECK,WIN32
```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-04-15 Thread Wei Xiao via Phabricator via cfe-commits

wxiao3 updated this revision to Diff 195096.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744

Files:
  lib/CodeGen/TargetInfo.cpp
  test/CodeGen/vector.c
  test/CodeGen/x86_32-arguments-darwin.c
  test/CodeGen/x86_32-arguments-linux.c
  test/CodeGen/x86_32-m64-darwin.c
  test/CodeGen/x86_32-m64-iamcu.c
  test/CodeGen/x86_32-m64-linux.c
  test/CodeGen/x86_32-m64-win32.c

Index: test/CodeGen/x86_32-m64-win32.c
===
--- /dev/null
+++ test/CodeGen/x86_32-m64-win32.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -w -triple i386-pc-win32 -emit-llvm -O2 -o - %s | FileCheck %s
+
+// CHECK-LABEL: define dso_local <1 x i64> @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// CHECK: call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// CHECK: ret <1 x i64>
+
+#include 
+__m64 m64;
+void callee(__m64 __m1, __m64 __m2);
+__m64 caller(__m64 __m1, __m64 __m2)
+{
+  callee(__m2, __m1);
+  return m64;
+}
Index: test/CodeGen/x86_32-m64-linux.c
===
--- /dev/null
+++ test/CodeGen/x86_32-m64-linux.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-linux-gnu -target-cpu pentium4 -emit-llvm -o %t %s
+// RUN: FileCheck < %t %s
+
+// CHECK-LABEL: define x86_mmx @caller(x86_mmx %__m1.coerce, x86_mmx %__m2.coerce)
+// CHECK: tail call void @callee(x86_mmx %__m2.coerce, x86_mmx %__m1.coerce)
+// CHECK: ret x86_mmx
+
+#include 
+__m64 m64;
+void callee(__m64 __m1, __m64 __m2);
+__m64 caller(__m64 __m1, __m64 __m2)
+{
+  callee(__m2, __m1);
+  return m64;
+}
Index: test/CodeGen/x86_32-m64-iamcu.c
===
--- /dev/null
+++ test/CodeGen/x86_32-m64-iamcu.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -w -triple i386-pc-elfiamcu -mfloat-abi soft -emit-llvm -O2 -o - %s | FileCheck %s
+
+// CHECK-LABEL: define <1 x i64> @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// CHECK: tail call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// CHECK: ret <1 x i64>
+
+#include 
+__m64 m64;
+void callee(__m64 __m1, __m64 __m2);
+__m64 caller(__m64 __m1, __m64 __m2)
+{
+  callee(__m2, __m1);
+  return m64;
+}
Index: test/CodeGen/x86_32-m64-darwin.c
===
--- /dev/null
+++ test/CodeGen/x86_32-m64-darwin.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -w -fblocks -triple i386-apple-darwin9 -target-cpu yonah -target-feature +mmx -emit-llvm -O2 -o - %s | FileCheck %s
+
+// CHECK-LABEL: define i64 @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// CHECK: tail call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// CHECK: ret i64
+
+#include 
+__m64 m64;
+void callee(__m64 __m1, __m64 __m2);
+__m64 caller(__m64 __m1, __m64 __m2)
+{
+  callee(__m2, __m1);
+  return m64;
+}
Index: test/CodeGen/x86_32-arguments-linux.c
===
--- test/CodeGen/x86_32-arguments-linux.c
+++ test/CodeGen/x86_32-arguments-linux.c
@@ -3,7 +3,7 @@
 
 // CHECK-LABEL: define void @f56(
 // CHECK: i8 signext %a0, %struct.s56_0* byval align 4 %a1,
-// CHECK: i64 %a2.coerce, %struct.s56_1* byval align 4,
+// CHECK: x86_mmx %a2.coerce, %struct.s56_1* byval align 4,
 // CHECK: <1 x double> %a4, %struct.s56_2* byval align 4,
 // CHECK: <4 x i32> %a6, %struct.s56_3* byval align 4,
 // CHECK: <2 x double> %a8, %struct.s56_4* byval align 4,
@@ -12,7 +12,7 @@
 
 // CHECK: call void (i32, ...) @f56_0(i32 1,
 // CHECK: i32 %{{.*}}, %struct.s56_0* byval align 4 %{{[^ ]*}},
-// CHECK: i64 %{{[^ ]*}}, %struct.s56_1* byval align 4 %{{[^ ]*}},
+// CHECK: x86_mmx %{{[^ ]*}}, %struct.s56_1* byval align 4 %{{[^ ]*}},
 // CHECK: <1 x double> %{{[^ ]*}}, %struct.s56_2* byval align 4 %{{[^ ]*}},
 // CHECK: <4 x i32> %{{[^ ]*}}, %struct.s56_3* byval align 4 %{{[^ ]*}},
 // CHECK: <2 x double> %{{[^ ]*}}, %struct.s56_4* byval align 4 %{{[^ ]*}},
Index: test/CodeGen/x86_32-arguments-darwin.c
===
--- test/CodeGen/x86_32-arguments-darwin.c
+++ test/CodeGen/x86_32-arguments-darwin.c
@@ -229,7 +229,7 @@
 
 // CHECK-LABEL: define void @f56(
 // CHECK: i8 signext %a0, %struct.s56_0* byval align 4 %a1,
-// CHECK: i64 %a2.coerce, %struct.s56_1* byval align 4,
+// CHECK: x86_mmx %a2.coerce, %struct.s56_1* byval align 4,
 // CHECK: i64 %a4.coerce, %struct.s56_2* byval align 4,
 // CHECK: <4 x i32> %a6, %struct.s56_3* byval align 16 %a7,
 // CHECK: <2 x double> %a8, %struct.s56_4* byval align 16 %a9,
@@ -238,7 +238,7 @@
 
 // CHECK:   call void (i32, ...) @f56_0(i32 1,
 // CHECK: i32 %{{[^ ]*}}, %struct.s56_0* byval align 4 %{{[^ ]*}},
-// CHECK: i64 %{{[^ ]*}}, %struct.s56_1* byval align 4 %{{[^ ]*}},
+// CHECK: x86_mmx %{{[^ ]*}}, %struct.s56_1* byval align 4 %{{[^ ]*}},
 // CHECK: i64 %{{[^ ]*}}, %struct.s56_2* byval align 4 %{{[^ ]*}},
 // CHECK: <4 x i32> %{{[^ ]*}}, %struct.s56_3* byval align 16 %{{[^

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-04-08 Thread Simon Pilgrim via Phabricator via cfe-commits

RKSimon added inline comments.



Comment at: test/CodeGen/x86_32-mmx-linux.c:2
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-linux-gnu -target-cpu 
pentium4 -emit-llvm -o %t %s
+// RUN: FileCheck < %t %s
+

Test on more triples and add the test file to trunk with current codegen so 
this patch shows the delta


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-04-04 Thread Reid Kleckner via Phabricator via cfe-commits

rnk added inline comments.



Comment at: lib/CodeGen/TargetInfo.cpp:9473-9474
   IsWin32FloatStructABI, CodeGenOpts.NumRegisterParameters));
+} else if (Triple.getOS() == llvm::Triple::Linux) {
+  // System V i386 ABI requires __m64 value passing by MMX registers.
+  bool EnableMMX = getContext().getTargetInfo().getABI() != "no-mmx";

The Sys V rules apply to every non-Windows OS, not just Linux. I think you 
should add the parameter regardless of the OS


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-04-03 Thread Wei Xiao via Phabricator via cfe-commits

wxiao3 added a comment.

Dear reviewers, any comments?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-03-23 Thread Wei Xiao via Phabricator via cfe-commits

wxiao3 created this revision.
wxiao3 added reviewers: annita.zhang, LuoYuanke, smaslov, craig.topper, 
hjl.tools.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

According to System V i386 ABI: the "__m64" type paramater and return va
passed by MMX registers. But current implementation treats "__m64" as "i
which results in parameter passing by stack and returning by EDX and EAX

This patch fixes the bug (https://bugs.llvm.org/show_bug.cgi?id=41029).


Repository:
  rC Clang

https://reviews.llvm.org/D59744

Files:
  lib/CodeGen/TargetInfo.cpp
  test/CodeGen/x86_32-arguments-linux.c
  test/CodeGen/x86_32-mmx-linux.c

Index: test/CodeGen/x86_32-mmx-linux.c
===
--- /dev/null
+++ test/CodeGen/x86_32-mmx-linux.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-linux-gnu -target-cpu pentium4 -emit-llvm -o %t %s
+// RUN: FileCheck < %t %s
+
+// CHECK-LABEL: define x86_mmx @caller(x86_mmx %__m1.coerce, x86_mmx %__m2.coerce)
+// CHECK: tail call void @callee(x86_mmx %__m2.coerce, x86_mmx %__m1.coerce)
+// CHECK: ret x86_mmx
+
+#include 
+__m64 m64;
+void callee(__m64 __m1, __m64 __m2);
+__m64 caller(__m64 __m1, __m64 __m2)
+{
+  callee(__m2, __m1);
+  return m64;
+}
Index: test/CodeGen/x86_32-arguments-linux.c
===
--- test/CodeGen/x86_32-arguments-linux.c
+++ test/CodeGen/x86_32-arguments-linux.c
@@ -3,7 +3,7 @@
 
 // CHECK-LABEL: define void @f56(
 // CHECK: i8 signext %a0, %struct.s56_0* byval align 4 %a1,
-// CHECK: i64 %a2.coerce, %struct.s56_1* byval align 4,
+// CHECK: x86_mmx %a2.coerce, %struct.s56_1* byval align 4,
 // CHECK: <1 x double> %a4, %struct.s56_2* byval align 4,
 // CHECK: <4 x i32> %a6, %struct.s56_3* byval align 4,
 // CHECK: <2 x double> %a8, %struct.s56_4* byval align 4,
@@ -12,7 +12,7 @@
 
 // CHECK: call void (i32, ...) @f56_0(i32 1,
 // CHECK: i32 %{{.*}}, %struct.s56_0* byval align 4 %{{[^ ]*}},
-// CHECK: i64 %{{[^ ]*}}, %struct.s56_1* byval align 4 %{{[^ ]*}},
+// CHECK: x86_mmx %{{[^ ]*}}, %struct.s56_1* byval align 4 %{{[^ ]*}},
 // CHECK: <1 x double> %{{[^ ]*}}, %struct.s56_2* byval align 4 %{{[^ ]*}},
 // CHECK: <4 x i32> %{{[^ ]*}}, %struct.s56_3* byval align 4 %{{[^ ]*}},
 // CHECK: <2 x double> %{{[^ ]*}}, %struct.s56_4* byval align 4 %{{[^ ]*}},
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -914,10 +914,10 @@
 
 /// IsX86_MMXType - Return true if this is an MMX type.
 bool IsX86_MMXType(llvm::Type *IRType) {
-  // Return true if the type is an MMX type <2 x i32>, <4 x i16>, or <8 x i8>.
+  // Return true if the type is an MMX type <1 x i64>, <2 x i32>, <4 x i16>,
+  // or <8 x i8>.
   return IRType->isVectorTy() && IRType->getPrimitiveSizeInBits() == 64 &&
-cast(IRType)->getElementType()->isIntegerTy() &&
-IRType->getScalarSizeInBits() != 64;
+cast(IRType)->getElementType()->isIntegerTy();
 }
 
 static llvm::Type* X86AdjustInlineAsmType(CodeGen::CodeGenFunction ,
@@ -1008,6 +1008,7 @@
   bool IsSoftFloatABI;
   bool IsMCUABI;
   unsigned DefaultNumRegisterParameters;
+  bool IsMMXEnabled;
 
   static bool isRegisterSize(unsigned Size) {
 return (Size == 8 || Size == 16 || Size == 32 || Size == 64);
@@ -1067,13 +1068,15 @@
 
   X86_32ABIInfo(CodeGen::CodeGenTypes , bool DarwinVectorABI,
 bool RetSmallStructInRegABI, bool Win32StructABI,
-unsigned NumRegisterParameters, bool SoftFloatABI)
+unsigned NumRegisterParameters, bool SoftFloatABI,
+bool MMXEnabled)
 : SwiftABIInfo(CGT), IsDarwinVectorABI(DarwinVectorABI),
   IsRetSmallStructInRegABI(RetSmallStructInRegABI),
   IsWin32StructABI(Win32StructABI),
   IsSoftFloatABI(SoftFloatABI),
   IsMCUABI(CGT.getTarget().getTriple().isOSIAMCU()),
-  DefaultNumRegisterParameters(NumRegisterParameters) {}
+  DefaultNumRegisterParameters(NumRegisterParameters),
+  IsMMXEnabled(MMXEnabled) {}
 
   bool shouldPassIndirectlyForSwift(ArrayRef scalars,
 bool asReturnValue) const override {
@@ -1094,10 +1097,11 @@
 public:
   X86_32TargetCodeGenInfo(CodeGen::CodeGenTypes , bool DarwinVectorABI,
   bool RetSmallStructInRegABI, bool Win32StructABI,
-  unsigned NumRegisterParameters, bool SoftFloatABI)
+  unsigned NumRegisterParameters, bool SoftFloatABI,
+  bool MMXEnabled = false)
   : TargetCodeGenInfo(new X86_32ABIInfo(
 CGT, DarwinVectorABI, RetSmallStructInRegABI, Win32StructABI,
-NumRegisterParameters, SoftFloatABI)) {}
+NumRegisterParameters, SoftFloatABI, MMXEnabled)) {}
 
   static bool isStructReturnInRegABI(
   const llvm::Triple , const CodeGenOptions );
@@ -1404,6

55 matches

Mail list logo