[PATCH] D11438: Part 2 to fix x86_64 fp128 calling convention.

[Chih-Hung Hsieh via llvm-commits]

chh added inline comments.

================
Comment at: test/CodeGen/X86/fp128-cast.ll:136
@@ +135,3 @@
+define void @TestUIToFPU32_F128() #2 {
+entry:
+  %0 = load i32, i32* @vu32, align 4
----------------
davidxl wrote:
> Why do we care what transformations have been done to get the IR? The IR code should by  itself readable -- so while the C example is useful, I still prefer the naming in IR simplified.
The comment now seems to be showing at wrong place in this code diff.
The biggest confusing name is u.sroa.0.4.extract.shift  in TestBits128.
So I will shorten long names in this function for now.

The C code is important for anyone in the future to test, if not having time to rebuild and test the whole AOSP with libm.
Any simplification of the IR might work, but clang could generate different bit operators for those long double union types and trigger problems with any ad-hoc f128 optimization.

 

================
Comment at: test/CodeGen/X86/fp128-compare.ll:34
@@ +33,3 @@
+  ret i32 %conv
+; CHECK-LABEL: TestComp128LT:
+; CHECK:       callq __lttf2
----------------
davidxl wrote:
> ok. But it is possible with test + sets, right?  may be adding a comment so that people know how to fix the test if it breaks in the future?
Sure, added comment.
If it is broken in the future, maybe it would be easier to continue using this trick. :-)


================
Comment at: test/CodeGen/X86/fp128-i128.ll:7
@@ +6,3 @@
+; union IEEEl2bits {
+;   long double e;
+;   struct {
----------------
davidxl wrote:
> __float128
Unfortunately, clang does not accept __float128 keyword, although it can emit f128 for llvm.


================
Comment at: test/CodeGen/X86/fp128-i128.ll:23
@@ +22,3 @@
+; void foo(long double x);
+; void TestUnionLD1(long double s, unsigned long n) {
+;      union IEEEl2bits u;
----------------
davidxl wrote:
> long double --> __float128?
no __float128 in clang.


================
Comment at: test/CodeGen/X86/fp128-i128.ll:44
@@ +43,3 @@
+; CHECK:       movaps %xmm0, -24(%rsp)
+; CHECK-NEXT:  movq -24(%rsp), %rax
+; CHECK-NEXT:  movabsq $281474976710655, %rcx
----------------
davidxl wrote:
> The pattern checked is pretty long -- I worry it may break in the future. Is it possible to relax it some how?
I tried to reduce the C code, but any reduction won't trigger the complicated IL that reached a point that my partial fix core dumped. Maybe we can take out a few CHECK-NEXT requirements.

On the other hard, I was terrified by so many ad-hoc optimizations of floating points for the usage patterns in libm.
I guess llvm tried to match or do better then gcc and libm tried to use every possible bit operations.
So maybe it is better for Android or anyone depends on f128 type to have more check rules here.
Whoever changes float optimization in the future has better fully understand and update these tests.
 



http://reviews.llvm.org/D11438