[CodeGen] Allow mixed scalar type constraints for inline asm #65465
Conversation
cor3ntin
added
llvm:SelectionDAG
|
Are there existing bug issues associated with this? |
I don't know. Found this issue in our downstream target. |
| @@ -0,0 +1,8 @@ | |||
| // RUN: %clang_cc1 %s -emit-llvm -o /dev/null | |||
|
|
|||
| unsigned test(float f) | |||
There was a problem hiding this comment.
What IR does this generate?
There was a problem hiding this comment.
This test is using -emit-llvm, but the patch is in SelectionDAG. SelectionDAG doesn't run with -emit-llvm
There was a problem hiding this comment.
Thanks for the review. You are right, indeed the test was pointless, the problem is with CodeGen. Updated the issue accordingly.
dfszabo
force-pushed
the
main-inline-asm-fix-fp-int-move
branch
from
5d403b7 to
22c95d7
Compare
dfszabo
force-pushed
the
main-inline-asm-fix-fp-int-move
branch
from
22c95d7 to
04f13a6
Compare
dfszabo
changed the title
dfszabo
force-pushed
the
main-inline-asm-fix-fp-int-move
branch
from
04f13a6 to
ea6ac87
Compare
|
Ping |
1 similar comment
|
Ping |
| %i = alloca i32, align 4 | ||
| store float %f, ptr %f.addr, align 4 | ||
| %0 = load float, ptr %f.addr, align 4 | ||
| %1 = call i32 asm sideeffect "", "=r,0,~{dirflag},~{fpsr},~{flags}"(float %0) |
| store i32 %1, ptr %i, align 4 | ||
| %2 = load i32, ptr %i, align 4 | ||
| ret i32 %2 | ||
| } |
There was a problem hiding this comment.
Can you also try the same, except with a mixed pointer and int/float? Also some vector cases?
dfszabo
force-pushed
the
main-inline-asm-fix-fp-int-move
branch
from
eb8afd2 to
0a3516b
Compare
| %f.addr = alloca float*, align 4 | ||
| %i = alloca i32, align 4 | ||
| store float* %f, ptr %f.addr, align 4 | ||
| %load_f = load float*, ptr %f.addr, align 4 |
There was a problem hiding this comment.
Test needs to be updated to use opaque pointers. Also you don't need all of this intermediate alloca stuff, you can simplify the incoming values and uses
| ; return i; | ||
| ; } | ||
|
|
||
|
|
There was a problem hiding this comment.
Add some comments explaining what this is showing
GCC supports code like "asm volatile ("" : "=r" (i) : "0" (f))" where i is integer type and f is floating point type. Currently this code produces an error with Clang. The change allows mixed scalar types between input and output constraints.
dfszabo
force-pushed
the
main-inline-asm-fix-fp-int-move
branch
from
52423a1 to
52e62c6
Compare
| ; CHECK-NEXT: # kill: def $eax killed $eax killed $rax | ||
| ; CHECK-NEXT: retq | ||
| entry: | ||
| %asm_call = call i32 asm sideeffect "", "=r,0,~{dirflag},~{fpsr},~{flags}"(ptr %f) |
There was a problem hiding this comment.
Is it really allowed to have the mismatched sizes, pointer 64 with i32? Should that be an x86-only thing?
There was a problem hiding this comment.
What about double + i32? Or i16 + float?
There was a problem hiding this comment.
Ugh. GCC seems to accept whatever garbage you give it.
| ; CHECK-NEXT: # kill: def $eax killed $eax killed $rax | ||
| ; CHECK-NEXT: retq | ||
| entry: | ||
| %asm_call = call i32 asm sideeffect "", "=r,0,~{dirflag},~{fpsr},~{flags}"(ptr %f) |
There was a problem hiding this comment.
Ugh. GCC seems to accept whatever garbage you give it.
|
Well if approved, then can someone merge it? |
GCC supports code like "asm volatile ("" : "=r" (i) : "0" (f))" where i is integer type and f is floating point type. Currently this code produces an error with Clang. The change allows mixed scalar types between input and output constraints.