search

avr-llvm / llvm

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

1-bit types consume one byte of space, not zero #180

Closed shepmaster closed 8 years ago

shepmaster commented 8 years ago

Integer division was rounding off i1 to take 0 bytes.

Closes #173

shepmaster commented 8 years ago

Note that there's at least one other example of this type of division which may want to be fixed. Seemed out of scope for this PR though.

dylanmckay commented 8 years ago

Very nice catch!

If this fixes the test, then test/CodeGen/AVR/lower-formal-arguments-assertion.ll will succeed, which will cause the test suite to fail (it is marked XFAIL:, "supposed to fail"). You will need to remove this line from the test.

dylanmckay commented 8 years ago

@shepmaster Looks like the test is still failing (as it was previously), by looking at the travis CI output.

shepmaster commented 8 years ago

the test is still failing

Hmm. This is what I ran locally:

./x86_64-apple-darwin/llvm/Debug+Asserts/bin/llc -march=avr -mcpu=atmega328p -filetype=obj ../src/llvm/test/CodeGen/AVR/lower-formal-arguments-assertion.ll

And that produced an object file. Any pointers on how to execute that test locally in the normal way?

shepmaster commented 8 years ago

execute that test locally in the normal way?

If I run this:

llc < src/llvm/test/CodeGen/AVR/lower-formal-arguments-assertion.ll -march=avr | FileCheck src/llvm/test/CodeGen/AVR/lower-formal-arguments-assertion.ll

I get this error:

error: no check strings found with prefix 'CHECK:'

Perhaps the test is (now incorrectly) failing because of this?

shepmaster commented 8 years ago

I changed up the test a bit; let's see how that flows.

shepmaster commented 8 years ago

is (now incorrectly) failing because of this?

Perhaps we should be using CHECK-NOT: in XFAIL tests?

dylanmckay commented 8 years ago

You're right - we need to have at least one CHECK line.

It would suffice to have CHECK-LABEL: foo next to the define @foo.

dylanmckay commented 8 years ago

Also, you can manually run the LLVM integrated tester yourself, which will invoke the ; RUN: command at the top of the test.

bin/llvm-lit ~/projects/llvm/avr-llvm/test/CodeGen/AVR/add.ll

It will execute the test. Also pass -v to print the command output if it fails.

shepmaster commented 8 years ago

we need to have at least one CHECK line.

Yup, I did that and pushed the code again and it's passed (PASS: LLVM :: CodeGen/AVR/lower-formal-arguments-assertion.ll (19 of 122))

you can manually run the LLVM integrated tester yourself

I was having issues with that, getting errors like No site specific configuration available!. And I tried running it in all sorts of different locations, changing PATH, adding a flag to point to lit.cfg directories... nothing :-)