Revert r315899 in the 6.0 branch

[DimitryAndric]

Bugzilla Link	35913
Resolution	MOVED
Resolved on	Jan 16, 2018 14:35
Version	6.0
OS	All
Blocks	#35152
CC	@emaste,@zmodem,@RKSimon

Extended Description

As describe in bug 35741, bug 35749, and bug 35831, https://reviews.llvm.org/rL315899 has caused serious regressions in assembly parsing of x86 lock and rep prefixes.

Though bug 35741 has been fixed, there has been no movement nor reviews on the others, so I would like to request this revision to be reverted in the 6.0 branch. Post 6.0, the regressions can then be fixed.

[llvmbot]

Sorry, but it seems I don't understand something. This patch does not have any relation to bug 35741, bug 35749, and bug 35831. What's the problem?

[DimitryAndric]

Sorry, but it seems I don't understand something. This patch does not have
any relation to bug 35741, bug 35749, and bug 35831.

Yes it does, it causes regressions, as described in those bugs:

• comments after lock/rep prefixes were no longer accepted (fixed now)
• .byte directives after lock/rep prefixes are no longer accepted (still open)
• labels after lock/rep prefixes are no longer accepted (still open)

and I think it is very likely there are other regressions in processing lock/rep prefixes. For example, having a .s file with just:

    lock

assembles fine with GNU as, or with clang before r315899, but after that change it gives:

justlock.s:2:1: error: unknown token in expression

^

There are probably some other edge cases that I haven't thought of, but which are sure to be used in some software out there...

[llvmbot]

Or I got the issue: I fixed one issue but I missed 2 others. What should I do now to resolve the problem? It was realy complex change in prefix support and we don't have proper test base for all possible cases that's why we can expect simmilar issues (and you wrote about this possibility).

As I understand I should simply it in trunk, right? And how we can discover other possible problems?

[llvmbot]

Misprint: I should simply fix it in trunk, right?

[DimitryAndric]

Misprint: I should simply fix it in trunk, right?

Yes, please. If the fixes are in, we can re-title this PR or close it and create a new one for merging the fixes to the release_60 branch.

[zmodem]

Dimitry: I'm not keen on reverting on the branch only, especially since the problematic commit landed a while back.

I'd prefer to either get the problematic commit reverted on trunk, and merge the revert, or merge fixes.

[llvmbot]

I already fixed the issue and published the fix for review: D42102 [X86] Allow usage of prefixes as a separate instr. It's really simple fix and I hope it will LGTM soon. Let's keep it in trunk and promote to the branch.

[DimitryAndric]

Closing this in favor of bug 35976.