This is an archive of the discontinued LLVM Phabricator instance.

[Bitcode] Limit bits used for CallingConv::ID in Function, update tests
ClosedPublic

Authored by vsk on Oct 16 2015, 12:56 PM.

Details

Summary
  • Use 10 bits to represent calling convention ID's instead of 13.
  • Update the bitcode compatibility tests to reflect this change.

We currently implement about 80 calling conventions. This change lowers the cap to 1023 CC's (from 8191). This gives us back reserve bits in Function's subclass data field, which I plan on using soon.

As a follow-up, I will fix syntax errors in the compatibility tests caused by accidental 3.6 -> 3.8 IR conversion.

Diff Detail

Event Timeline

vsk updated this revision to Diff 37625.Oct 16 2015, 12:56 PM
vsk retitled this revision from to [Bitcode] Limit bits used for CallingConv::ID in Function, update tests.
vsk updated this object.
vsk added reviewers: dblaikie, dexonsmith.
vsk added a subscriber: llvm-commits.
vsk updated this revision to Diff 37767.Oct 19 2015, 10:08 AM
vsk updated this object.
rnk added a subscriber: rnk.Oct 26 2015, 9:31 AM

Technically this is a bitcode backwards compatibility break, but I don't think anyone has ever used a calling convention ID greater than 1023 except in these test cases. I think we could get away with rejecting large calling convention IDs in the bitcode reader, though. After the reader, we can assert that truncation should not occur in setCallingConv for Function, InvokeInst, and CallInst.

include/llvm/IR/Function.h
167

Should this assert that CC < 1024?

vsk updated this revision to Diff 38456.Oct 26 2015, 1:25 PM
  • Assert when we try to set invalid calling conventions (addresses rnk's comment).
  • Error out in the bitcode reader when we see a bad calling conv (this is the compatibility break).
rnk accepted this revision.Oct 27 2015, 1:26 PM
rnk added a reviewer: rnk.

Can we hoist 0xCFF / 1023 into the CallingConv.h header, and call it something like CallingConv::MaxID? Otherwise, this looks good.

This revision is now accepted and ready to land.Oct 27 2015, 1:26 PM
This revision was automatically updated to reflect the committed changes.
vsk marked an inline comment as done.Oct 27 2015, 2:20 PM

Thanks Reid, I placed the constant in CallingConv::MaxID as suggested. r251452.