This is an archive of the discontinued LLVM Phabricator instance.

ms inline asm: recognize case-insensitive JMP and CALL as TargetLowering::C_Address
ClosedPublic

Authored by MaskRay on May 4 2023, 6:18 PM.

Details

Summary

In a __asm block, a symbol reference is usually a memory constraint
(indirect TargetLowering::C_Memory) [LOOP]. CALL and JUMP instructions are special
that __asm call k can be an address constraint, if k is a function.

Clang always gives us indirect TargetLowering::C_Memory and need to convert it
to direct TargetLowering::C_Address. D133914 implements this conversion, but
does not consider JMP or case-insensitive CALL. This patch implements the missing
cases, so that __asm jmp k (jmp ${0:P}) will correctly lower to jmp _k
instead of jmp dword ptr [_k].

(__asm call k lowered to call dword ptr ${0:P} and is fixed by D149695 to
lower to call ${0:P} instead.)

[LOOP]: Some instructions like LOOP{,E,NE} and Jcc always use an address
constraint (loop _k instead of loop dword ptr [_k]).

After this patch and D149579, all the following cases will be correct.

int k(int);
int (*kptr)(int);
...
__asm call k; // correct without this patch
__asm CALL k; // correct, but needs this patch to be compatible with D149579
__asm jmp k;  // correct, but needs this patch to be compatible with D149579
__asm call kptr; // will be fixed by D149579.  "Broken case" in clang/test/CodeGen/ms-inline-asm-functions.c
__asm jmp kptr;  // will be fixed by this patch and D149579

Diff Detail

Event Timeline

MaskRay created this revision.May 4 2023, 6:18 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 4 2023, 6:18 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
MaskRay requested review of this revision.May 4 2023, 6:18 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 4 2023, 6:18 PM
MaskRay edited the summary of this revision. (Show Details)May 4 2023, 6:23 PM
MaskRay edited the summary of this revision. (Show Details)May 4 2023, 6:33 PM
MaskRay updated this revision to Diff 519734.May 4 2023, 8:40 PM
MaskRay retitled this revision from ms inline asm: recognize "jmp" as TargetLowering::C_Address to ms inline asm: recognize case-insensitive JMP and CALL as TargetLowering::C_Address.
MaskRay edited the summary of this revision. (Show Details)

better description

Herald added a project: Restricted Project. · View Herald TranscriptMay 4 2023, 8:41 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
pengfei added inline comments.May 4 2023, 10:32 PM
llvm/lib/Target/X86/X86ISelLowering.cpp
33955–33961

JMP?

MaskRay updated this revision to Diff 519744.May 4 2023, 10:35 PM
MaskRay marked an inline comment as done.

fix a typo in a comment

pengfei accepted this revision.May 4 2023, 11:06 PM

LGTM.

This revision is now accepted and ready to land.May 4 2023, 11:06 PM