From: Borja F. <bor...@gm...> - 2012-05-01 22:10:19
|
They look good, very nice :) Some small details and nitpicks: 1) I think it's not necessary to add the reg_reg_imm variant because you covered it with the other two tests. 2) Do the tests work if you remove the target triple line? I've seen other backends don't include it. 3) Move the CHECK lines after the function prototype like other backends do. 4) In every CHECK line, can you remove the tabs between the instr mnemonic and the first operand and add a single space? (I'm unsure about this because i dont know if CHECK eats spaces). Also for the future, the llvm coding standards says to config your editor to replace tabs with spaces, so it's a good moment time to do it. 5) Oh and the most important one, please add this in only one file, add.ll or something like that, so we can keep this convention in the future, otherwise we'll end having too many test files. Something that should be covered is, when doing 16 bit additions we can use adiw or add depending on the imm value, can you cover this aswell? 2012/5/1 John Myers <ato...@gm...> > > > On Sun, Apr 29, 2012 at 10:46 AM, Nicklas Bo Jensen <nbj...@gm...>wrote: > >> Hi, >> >> I have successfully been able to compile your testcases >> (/avr-llvm/testcases/*.ll) to something looking like valid avr assembler. >> >> How should I test/simulate the assembler? I get errors when trying to >> simulate the generated assembler in AVRStudio. Perhaps they use a different >> assembler? >> >> avr-llvm produces GNU assembler syntax, which is different then the Atmel > assembler syntax. > > Eventually we could support multiple asm syntax's like the X86 target does. > |