Skip to content
This repository was archived by the owner on Sep 2, 2018. It is now read-only.

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

Merged
merged 1 commit into from
Jan 20, 2016

Conversation

shepmaster
Copy link

Integer division was rounding off i1 to take 0 bytes.

Closes #173

@shepmaster
Copy link
Author

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
Copy link
Member

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
Copy link
Member

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

@shepmaster
Copy link
Author

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
Copy link
Author

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?

Integer division was rounding off `i1` to take 0 bytes.

Closes avr-llvm#173
@shepmaster shepmaster force-pushed the i8-truncated-to-zero branch from 73671bd to 29f9ff0 Compare January 20, 2016 00:41
@shepmaster
Copy link
Author

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

@shepmaster
Copy link
Author

is (now incorrectly) failing because of this?

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

@dylanmckay
Copy link
Member

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
Copy link
Member

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
Copy link
Author

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 :-)

dylanmckay pushed a commit that referenced this pull request Jan 20, 2016
1-bit types consume one byte of space, not zero
@dylanmckay dylanmckay merged commit 8351752 into avr-llvm:avr-support Jan 20, 2016
@shepmaster shepmaster deleted the i8-truncated-to-zero branch January 20, 2016 03:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants