This is an archive of the discontinued LLVM Phabricator instance.

[NFC] Separate bitcode reading for FUNC_CODE_INST_CMPXCHG(_OLD)
ClosedPublic

Authored by gchatelet on Jul 8 2020, 2:35 AM.

Details

Summary

This is preparatory work to unable storing alignment for AtomicCmpXchgInst.
See D83136 for context and bug: https://bugs.llvm.org/show_bug.cgi?id=27168

Diff Detail

Event Timeline

gchatelet created this revision.Jul 8 2020, 2:35 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 8 2020, 2:35 AM
gchatelet marked 2 inline comments as done.Jul 8 2020, 2:40 AM

This patch splits the two implementations, moves a few lines and simplifies the code by inferring constant values.
The semantic is unchanged.

llvm/include/llvm/Bitcode/LLVMBitCodes.h
539

The documentation here was wrong.
alignment was never stored for FUNC_CODE_INST_CMPXCHG_OLD and failure_ordering and weak were optional.

llvm/lib/Bitcode/Reader/BitcodeReader.cpp
4989

Each function taking Slot (previously OpNum) will increase it if successful.
This allows to replace OpNum + X by its value.

jfb added inline comments.Jul 9 2020, 8:15 AM
llvm/include/llvm/Bitcode/LLVMBitCodes.h
539

It used to only have "ordering", and didn't separate success / failure (so it wasn't optional as much as not there).

jfb accepted this revision.Jul 9 2020, 8:15 AM
This revision is now accepted and ready to land.Jul 9 2020, 8:15 AM
gchatelet marked 2 inline comments as done.Jul 9 2020, 9:18 PM
gchatelet added inline comments.
llvm/include/llvm/Bitcode/LLVMBitCodes.h
539

Thx for the explanation :)

This revision was automatically updated to reflect the committed changes.
gchatelet marked an inline comment as done.
dmajor added a subscriber: dmajor.Jul 23 2020, 8:48 AM

To connect the dots for anyone following along: this was reverted in cc28058c13e89ecc85dac7e1bd5d13a2ce1bb620.

To connect the dots for anyone following along: this was reverted in cc28058c13e89ecc85dac7e1bd5d13a2ce1bb620.

Yes thx, I haven't been able to reproduce the error for now but working on it.

courbet added inline comments.Sep 9 2020, 6:51 AM
llvm/lib/Bitcode/Reader/BitcodeReader.cpp
4989

I don't think there is any guarantee that they increase it by one. This could break silently in the future...

courbet added inline comments.Sep 9 2020, 7:01 AM
llvm/lib/Bitcode/Reader/BitcodeReader.cpp
4989

Nevermind, I'm commenting on the old patch, I though I was looking at the new one...