avr-llvm / llvm

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

Support local labels #53

Closed dylanmckay closed 9 years ago

dylanmckay commented 9 years ago

See lib/MC/MCParser/AsmParser.cpp:870.

For some reason, AsmParser::parsePrimaryExpr is not handling b and f suffixes on local labels.

dylanmckay commented 9 years ago

The problem lies in AVRAsmParser.cpp:466.

Whenever we try and parse an operand, we first check if it is a valid register. If it is, then we treat it as such. For situations like sbiw X, 24, X is parsed into a MCOperand of type register, and then we get to 24 (which is a valid register, our code currently allows 31 instead of r31 currently -- perhaps this is in error). 24 is parsed into an MCOperand of type register, even though it is not a register. We still get the correct output, however.

In the case of local labels, they are automatically taken to be registers. Because of this, the correct branch (in the else block) is not executed, and MCParser.parseExpression() doesn't handle it. MCParser.parseExpression() has code to handle a trailing b or f.

dylanmckay commented 9 years ago

There is an ambiguity when parsing integer operands without context, because AVR-GCC supports specifiying register names (r12) and register numbers (12) interchangeably. Without context, we can't tell the difference.

There are two solutions:

RandomInsano commented 9 years ago

My opinion is to not support GCC numbered registers until someone comes along who needs it. It sounds like such a confusing idea.

dylanmckay commented 9 years ago

My opinion is to not support GCC numbered registers until someone comes along who needs it. It sounds like such a confusing idea.

I concur.

dylanmckay commented 9 years ago

I have removed support for numbered registers in d9c85f193f0bf40f776b7a6581c79419dd1bbe7a; local labels are now correctly handled.