This is an archive of the discontinued LLVM Phabricator instance.

[x86][ms-inline-asm] use of "jmp short" in asm is not supported
ClosedPublic

Authored by zizhar on Sep 27 2016, 3:51 AM.

Details

Summary

The following patch is for compatability with Microsoft.
Microsoft ignores the keyword "short" when used after a jmp, for example:

__asm {
    jmp short label
  label:
}

Currently llvm fails on this code.
In order to be Microsoft compatible, we should allow the previous example.

A test for that patch will be added in another ticket, since it's located in clang's codegen tests. Link will be added shortly.

Edit: link to test: https://reviews.llvm.org/D24958

Diff Detail

Event Timeline

zizhar updated this revision to Diff 72615.Sep 27 2016, 3:51 AM
zizhar retitled this revision from to [x86][ms-inline-asm] use of "jmp short" in asm is not supported.
zizhar updated this object.
zizhar added a reviewer: rnk.
zizhar set the repository for this revision to rL LLVM.
zizhar updated this object.Sep 27 2016, 3:58 AM
zizhar added a subscriber: llvm-commits.
rnk accepted this revision.Oct 5 2016, 10:10 AM
rnk edited edge metadata.

lgtm

lib/Target/X86/AsmParser/X86AsmParser.cpp
2142

I wish we had an explicit "short jump" assembly instruction, though. :( Anyway, for now we should do this for compatibility.

This revision is now accepted and ready to land.Oct 5 2016, 10:10 AM
rnk requested changes to this revision.Oct 5 2016, 10:12 AM
rnk edited edge metadata.

Actually, there are some minor style issues, but otherwise this is OK.

lib/Target/X86/AsmParser/X86AsmParser.cpp
2136

Follow the LLVM naming convention, "NextTok"

2139

format

2145

formatting

This revision now requires changes to proceed.Oct 5 2016, 10:12 AM
zizhar updated this revision to Diff 73762.Oct 6 2016, 4:30 AM
zizhar edited edge metadata.
zizhar removed rL LLVM as the repository for this revision.

fixed :>

rnk accepted this revision.Oct 12 2016, 4:52 PM
rnk edited edge metadata.

lgtm, sorry for the delay, vacation strikes

This revision is now accepted and ready to land.Oct 12 2016, 4:52 PM
This revision was automatically updated to reflect the committed changes.