[PATCH] D61547: [IR] Disallow llvm.global_ctors and llvm.global_dtors of the 2-field form in textual format

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith added a comment.

In D61547#1497555 <https://reviews.llvm.org/D61547#1497555>, @MaskRay wrote:

> In D61547#1497030 <https://reviews.llvm.org/D61547#1497030>, @dexonsmith wrote:
>
> > In D61547#1495855 <https://reviews.llvm.org/D61547#1495855>, @MaskRay wrote:
> >
> > > In D61547#1495339 <https://reviews.llvm.org/D61547#1495339>, @rnk wrote:
> > >
> > > > Presumably `test/Bitcode/upgrade-global-ctors.ll.bc` should remain unchanged, i.e. not be regenerated? We want it to use the pre-upgraded, two field form.
> > >
> > >
> > > I used llvm-as without this patch to generate it. I added anither test to check the upgrader correctly adds i8* null
> >
> >
> > What's the problem with using the pre-existing file?
>
>
> test/Bitcode/upgrade-global-ctors.ll.bc uses zeroinitializer, it doesn't check if AuoUpgrader adds i8* null as the third field. So I improved the test a bit and made it clear both globals_ctors and global_dtors are upgraded.


It makes sense to check `global_dtors`, but changing binary files is expensive in Git, and it's important to use a known version of `llvm-as` to generate the bitcode file.  Here's what I suggest.

- Leave test/Bitcode/upgrade-global-ctors.ll.bc alone (revert to the previous binary).
  - Update test/Bitcode/upgrade-global-ctors.ll's CHECK line appropriately.
  - Put the `verify-uselistorder` checks back into the test.
- Add a new test for the destructor, probably test/Bitcode/upgrade-global-dtors.ll{,.bc}.
  - Generate the .bc using `llvm-as` from the most recent LLVM release (8.0, right?) and document in the .ll how it was generated and with what version.
  - Add `verify-uselistorder` checks.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61547/new/

https://reviews.llvm.org/D61547