This is an archive of the discontinued LLVM Phabricator instance.

[Bitcode][Asm] Teach LLVM to read and write operand bundles.
ClosedPublic

Authored by sanjoy on Aug 28 2015, 4:25 PM.

Details

Summary

This also adds the first set of tests for operand bundles.

The optimizer has not been audited to ensure that it does the right
thing with operand bundles.

Note:
This change adds a stub to LangRef, but documentation is still a TODO.
Given that this is a large change set, I figured it would be wise to get
this out for review and get some feedback earlier rather than later.
I'll update this patch with more comprehensive documentation soon.

Depends on D12456.

Diff Detail

Repository
rL LLVM

Event Timeline

sanjoy updated this revision to Diff 33489.Aug 28 2015, 4:25 PM
sanjoy retitled this revision from to [Bitcode][Asm] Teach LLVM to read and write operand bundles..
sanjoy updated this object.
sanjoy added a subscriber: llvm-commits.
lib/AsmParser/LLParser.cpp
1954 ↗(On Diff #33489)

Why use a lambda here as opposed to just inlining it?

test/Bitcode/operand-bundles.ll
19 ↗(On Diff #33489)

Do we really want two ways to represent that? I think reporting an error for [ ] would be appropriate.

sanjoy added inline comments.Aug 31 2015, 7:53 PM
lib/AsmParser/LLParser.cpp
1954 ↗(On Diff #33489)

I started out thinking that the early returns would simplify the code, but after re-reading it I don't think they're helping at all. I'll inline the logic and get rid of the lambda.

test/Bitcode/operand-bundles.ll
19 ↗(On Diff #33489)

Will fix.

sanjoy updated this revision to Diff 33750.Sep 1 2015, 4:06 PM

Address review + more docs.

Updates look good, thanks.

docs/LangRef.rst
1447 ↗(On Diff #33750)

typo: should be 'Operand bundles are tagged sets...'

sanjoy updated this revision to Diff 33877.Sep 2 2015, 4:13 PM
sanjoy edited edge metadata.
sanjoy updated this revision to Diff 34299.Sep 8 2015, 9:55 PM
  • add some tests to compatibility.ll

    Note: these do not yet pass but as far as I can tell, the issue is unrelated to operand bundles -- see PR24755.
maksfb added a subscriber: maksfb.Sep 9 2015, 11:34 PM
sanjoy updated this revision to Diff 34598.Sep 11 2015, 3:49 PM
  • address Duncan's review about bitcode reading and writing
    • use a prefixed list sequence of operand bundles that are consumed when parsing calls and invokes
    • use a common "bundle tag" area in a module, and refer to tags by their indices elsewhere
sanjoy updated this revision to Diff 34599.Sep 11 2015, 3:57 PM
  • address two other comments by Duncan:
    • always use 0 abbrev in WriteOperandBundles
    • hyphenation in the comment
sanjoy updated this revision to Diff 34753.Sep 14 2015, 3:56 PM
  • address Duncan's review w.r.t not duplicating code
  • some more docs
  • use ArrayRef<> correctly to avoid breakage when building with gcc
docs/LangRef.rst
1486 ↗(On Diff #34753)

typo: s/taken/take/

sanjoy updated this revision to Diff 34915.Sep 16 2015, 12:36 PM
  • fix typo pointed out by Joseph
This revision was automatically updated to reflect the committed changes.

Just to make it clear: this was LGTM'ed by Duncan via email (for some reason these emails did not show up here).