[PATCH] D28468: [X86] Fix for bugzilla 31576 - add support for "data32" instruction prefix
Nirav Dave via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 17 11:17:27 PST 2017
niravd accepted this revision.
niravd added a comment.
This revision is now accepted and ready to land.
> Regarding the disassembly, when data16/data32 is adjacent to an instruction like lgdt, then the encoding and the decoding is correct and the asm printer prints the right instruction. I've added disassembly tests for these cases.
> When data16/data32 stands on its own the 0x66 byte by is disassembled correctly, but the asm printer always prints "data16" and not "data32" for the 16 bit mode.
> I've found somebody already added an exception for CALLpcrel32 instruction in X86ATTInstPrinter because there's seems to be some lack of support of the "Requires" attribute.
> I can add an exception there as well, but I'm not sure if this is the right thing to do because data16/data32 shouldn't really stand on it's own, it's a prefix to the instruction that follows.
Oof. That's unfortunate. As ugly as it is, I think you should add the special case to like CALLpcrel32 just so we're certain we always generate code that doesn't trigger warnings/errors.
I still don't see the additional tests so make sure they're added, but otherwise LGTM.
More information about the llvm-commits