This is an archive of the discontinued LLVM Phabricator instance.

[X86][MC] Fix the bug of -output-asm-variant=1 for intel syntax
ClosedPublic

Authored by skan on Feb 16 2023, 5:28 AM.

Details

Summary

Before this patch

$ echo "leal    (,%r15), %eax" | llvm-mc --show-encoding --output-asm-variant=1

        lea     eax, [r15]                      # encoding: [0x42,0x8d,0x04,0x3d,0x00,0x00,0x00,0x00]

$ echo "lea     eax, [r15]" | llvm-mc --show-encoding -x86-asm-syntax=intel --output-asm-variant=1

        lea     eax, [r15]                      # encoding: [0x41,0x8d,0x07]

MC printed the register r15 as a base in intel syntax even when it's an index.
Then we got a different encoding by using the assembly from the output of the
first command.

I believe the behavior is too weird to be called a feature.

After this patch, we get

$ echo "leal    (,%r15), %eax" | llvm-mc --show-encoding --output-asm-variant=1

        lea     eax, [1*r15]                    # encoding: [0x42,0x8d,0x04,0x3d,0x00,0x00,0x00,0x00]

Diff Detail

Event Timeline

skan created this revision.Feb 16 2023, 5:28 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 16 2023, 5:28 AM
skan requested review of this revision.Feb 16 2023, 5:28 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 16 2023, 5:28 AM
pengfei accepted this revision.Feb 16 2023, 6:02 AM

LG.

llvm/test/MC/Disassembler/X86/intel-syntax.txt
176–181

Too many spaces.

This revision is now accepted and ready to land.Feb 16 2023, 6:02 AM
skan updated this revision to Diff 497998.Feb 16 2023, 6:27 AM
skan marked an inline comment as done.

Address review comments: remove some space

RKSimon accepted this revision.Feb 16 2023, 9:07 AM

LGTM - cheers

MaskRay accepted this revision.Feb 16 2023, 9:45 AM
MaskRay added inline comments.Feb 16 2023, 10:04 AM
llvm/test/MC/Disassembler/X86/intel-syntax.txt
177

Shall we add a test with a non-zero displacement like lea eax, [1*r15+4]?

MaskRay added a comment.EditedFeb 16 2023, 10:16 AM

I believe the behavior is too weird to be called a feature.

More importantly, the behavior does not match llvm-mc -filetype=obj. The new behavior matches llvm-mc -filetype=obj and as -msyntax=intel -mnaked-reg.

skan updated this revision to Diff 498210.Feb 16 2023, 5:45 PM
skan marked an inline comment as done.

Address review comments: add 2 more tests

skan added a comment.Feb 16 2023, 5:52 PM

I believe the behavior is too weird to be called a feature.

More importantly, the behavior does not match llvm-mc -filetype=obj. The new behavior matches llvm-mc -filetype=obj and as -msyntax=intel -mnaked-reg.

The encoding is right, w/o this patch, we can get still get a correct encoding for "lea eax, [1*r15] " with llvm-mc. The issue here is that the output assembly is incorrect.

llvm/test/MC/Disassembler/X86/intel-syntax.txt
177

Good suggestion. Done.

This revision was landed with ongoing or failed builds.Feb 16 2023, 5:54 PM
This revision was automatically updated to reflect the committed changes.