Page MenuHomePhabricator

[Inline asm] Correct the constrain for function call in inline asm
AbandonedPublic

Authored by xiangzhangllvm on Aug 4 2021, 7:19 PM.

Details

Summary

(This patch will cause some inline_asm test check failed, let first discuss the corretness of this patch, then I'll decide modify the related tests or not. )

Currently Clang generate "function call in inline asm" with indirect address access "*m"

but the call instruction directly use the address of the function not the context in the address.

So I think we should change the address constrain from "*m" to 'm'

For example : (use Clang -fams-blocks -fpic -S -emit-llvm inline_asm_blocks_call.c to reproduce)

__asm{    call  sincos_asm  \n   ret };

without this patch, the clang will generate follow IR:

call void asm sideeffect inteldialect "call qword ptr ${0:P}\0A\09ret", "*m,~{dirflag},~{fpsr},~{flags}"(void (...)* @sincos_asm)
                                                                         ^
                                                                         indirectly access the call op of "void (...)* @sincos_asm"
                                                               but for call, it directly read the point "void (...)* @sincos_asm"

So then the back end generate follow wrong asm for it:

movq    sincos_asm@GOTPCREL(%rip), %rax
         #APP
         callq   *(%rax)    #  we should directly read the function address in register %rax not mem (%rax)        -- should be --> callq *%rax
          retq 
         #NO_APP

Diff Detail

Event Timeline

xiangzhangllvm created this revision.Aug 4 2021, 7:19 PM
xiangzhangllvm requested review of this revision.Aug 4 2021, 7:19 PM
xiangzhangllvm edited the summary of this revision. (Show Details)Aug 4 2021, 7:20 PM
xiangzhangllvm edited the summary of this revision. (Show Details)Aug 4 2021, 7:23 PM
xiangzhangllvm edited the summary of this revision. (Show Details)Aug 4 2021, 8:14 PM

(use Clang -fams-blocks -fpic -S -emit-llvm inline_asm_blocks_call.c to reproduce)

Two questions:

  1. Does it work without -fpic?
  2. This is a MS flavour inline asm, ATT should be correct and not be affect?
clang/lib/CodeGen/CGStmt.cpp
2094

Can we only handle for MS inline asm? Maybe need an bool type which passed from EmitAsmStmt:

bool IsMSAsm = isa<MSAsmStmt>(&S);
llvm::Value *Arg = EmitAsmInput(Info, InputExpr, Constraints, IsMSAsm);
...
llvm::Value* CodeGenFunction::EmitAsmInput(..., bool IsMSAsm) {
...
return EmitAsmInputLValue(..., IsMSAsm);

(use Clang -fams-blocks -fpic -S -emit-llvm inline_asm_blocks_call.c to reproduce)

Two questions:

  1. Does it work without -fpic?
  2. This is a MS flavour inline asm, ATT should be correct and not be affect?

The constrain should not depend on fpic or MS style.
non-MS style or non-fpic generate indirect "*m" for call function label is also incorrect.

clang/lib/CodeGen/CGStmt.cpp
2111

We'd better also check the InputValue is for call instruction in inline_asm. But seems hard to do it here.
Forgive me that I am not familiar with Frond End code.

LuoYuanke added inline comments.Aug 10 2021, 12:01 AM
clang/test/CodeGen/X86/inline_asm_blocks_call.c
8

This should changed to $0?

Can we change the MS inline asm lowering to use an "r" constraint here, instead of trying to mess with the meaning of "m"?

Can we change the MS inline asm lowering to use an "r" constraint here, instead of trying to mess with the meaning of "m"?

But the inline asm instruction string has append call with mem flag "qword ptr". We don't handle the instructions string until print it.

xiangzhangllvm abandoned this revision.Aug 19 2021, 7:20 PM