avr-llvm / llvm

[MERGED UPSTREAM] AVR backend for the LLVM compiler library
220 stars 21 forks source link

Refactor AVRAsmParser #96

Closed agnat closed 9 years ago

agnat commented 9 years ago

Here is one possible solution to #91 plus a few minor clean-ups.

What do you think?

dylanmckay commented 9 years ago

Thanks!

Does this allow sbiw Z, 1 as well as sbiw r30, 1? It looks like it does with a quick review of the code. If so, this is great!

Looks very nice :)

agnat commented 9 years ago

Does this allow sbiw Z, 1

Now it does... ;)

I also implemented the register pair colon syntax, as in r25:r24.

Sh4rK commented 9 years ago

Shouldn't the X, Y and Z register names be capital letters?

Sh4rK commented 9 years ago

Ok, I see that this would complicate parsing, as you use .lower() before matching to "standardize" the reg names, so it is probably acceptable this way.

agnat commented 9 years ago

Shouldn't the X, Y and Z register names be capital letters?

The current parser implementation is case insensitive (it canonicalizes to lower case). So, to get a match the register name must be lower case too. Fixing that is not that big a deal. But for now I just left it that way...

[...] so it is probably acceptable this way

That's what I thought. One step at a time ;)

dylanmckay commented 9 years ago

Shouldn't the X, Y and Z register names be capital letters?

Ideally, but GCC still works with the lower case variants so this isn't really a problem, and as @agnat said, this is an easy future fix.

This is good progress, thanks for the PR :)