In D104194#2815762, @MaskRay wrote:

More concretely, branch instruction tests in llvm/test/MC/X86/x86-32.s can be moved outside, then you can add a test for jecxz. jexcz seems untested.

Add test case

Dropping unnecessary implicit-check-not guard

Directly check for the warning, rather than converting it to an error

This required a few too many changes; I'll re-upload a working variant on top of a prerequisite (D103993).

Fix handling of the second (and later) field after an ORG directive

While I was working on the comments above, I discovered a problem - after improving the tests and fixing various errors, the way we were dealing with determining field offsets caused a failure on the second field after an ORG offset change. I've been working on and off to adjust the code to handle this; it requires a nontrivial rework, as Structure has to track both its Size and the offset of the next field. I'll upload that revision shortly, and I'm afraid it will require a re-review. Sorry, thakis - and I'll try to find someone else if more appropriate.

In D92507#2448819, @thakis wrote:

In D92507#2445774, @epastor wrote:

In D92507#2431130, @thakis wrote:

Does it wrap around 32-bit in 64-bit output too?

Oddly enough: yes.

Is this relied on? Or could we error out on negative values instead?

I don't think we can prove that it isn't relied on, especially when ORG explicitly allows putting things "outside bounds", so I'd rather maintain compatibility.

Well, we don't need to prove it if we make it an error: We'll know if the diag fires. So if we make this an error first and then build lots of files with it, we'll learn if we run into it. At that point we'll know we need to make it compatible, but maybe we won't have to. Doesn't it seem better to default to less surprising behavior and only put in the compat warts once there's evidence that we need them?

Fix lint

Marking as Request Changes to get it off of my dashboard until the other comments get a response.

Add test (confirmed to fail before this patch)

Updating use of peekTok to account for macros & included files

Fix a copy/paste error

In D92505#2694696, @thakis wrote:

(this didn't have me as a reviewer, so I only saw it by coincidence)

We should also add the /WX switch :)

Rebase to HEAD

Rebase to HEAD

Can you add tests for this?

Rebase on parent

Revised to include fatal warning support, and test both cases

Restore the explicit cast to make it easier to switch to a Warning later

Emit an error on negative ORG offsets, per feedback.

Rebase on parent

Rebase on parent

Rebase on HEAD

Improve comments to clarify the change

In D93211#2452405, @tambre wrote:

CC'ing @epastor as a FYI for the assembly parser bug.

In D90061#2431150, @thakis wrote:

Still no test, as far as I can tell. I think the suggestion below turns this into a 3-line diff (+ test).

Rebase on parent

Fix errors in Alias/AliasArgs use, and add tests

Use AliasArgs to simplify

In D92507#2431130, @thakis wrote:

Does it wrap around 32-bit in 64-bit output too?

The test here doesn't currently warn - because test5 is already defined, and as such the line under t5: is lexed as:

def textequ <redef>

Rebase on parent

Rebase on HEAD

In D90058#2426388, @thakis wrote:

(Just to be sure: This is ready to go and not waiting on me, right?)

Update for new test files