[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. |
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? |
6 participants
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.