It seems unnecessary to have separate flags for assembly vs bitcode.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Event Timeline
Separate flags allows vendors (and the default!) to have use-list order on-by-default in bitcode and off-by-default in textual IR.
IMO, the best path to simplification here:
- Make it illegal or impossible to observe the use-lists of ConstantData (most importantly, ConstantInt and ConstantFloat). E.g., change replaceAllUsesWith() to crash for those types and require people to check first.
- Stop serializing the use-lists of ConstantData (since we know there are no observers). This removes 99% of (a) the space overhead in bitcode and (b) the noise in textual assembly, since we're no longer tracking the use-list of i1 0, i1 1, i8 0, etc.
- Consider merging the two options and flipping them both on by default since the noise in the textual IR is no longer terrible?
- Consider removing -preserve-bc-use-list-order and just turn it on all the time since it no longer has major overhead?
- Consider removing -preserve-ll-use-list-order and just turn it on all the time since it no longer adds undue noise to the IR?
Note that walking the uses of i1 0 (or similar) is dangerous since it's operating on all functions/modules/etc. in the LLVMContext. Any code that's doing this should probably be rewritten for other soundness reasons.
I used to have a series of patches to the above effect; there were only a few more test failures to fix before I could land it. Unfortunately I dropped the work since I got busy with something else. Here were some of the fixes I landed:
- r263853 / c3fa1eded258aa38a30911af6602892846efd6d1 (this one was pretty bad)
- r282320 / c82c11428e0ab2ff58a342ca3a615315929a2633
- r282333 / b4798739127cd7748177ac506e6d45cc8cf5e4f8
- r282334 / 4fd9b7e16f98173d730ac1a347f7eae96a9fd2db
- r282337 / b1b208a1f514ccb90b5afba0af49d6aa39dee21b
- r282338 / 11c06ea55a060d5460f716b00ca593e5dd829b17
llvm/tools/opt/opt.cpp | ||
---|---|---|
217 | Notice that bitcode defaults to on... | |
222 | ... but textual assembly defaults to off. |
Here's a link to a related RFC:
https://discourse.llvm.org/t/rfc-constantdata-should-not-have-use-lists/42606
Patch 0003 removed use-lists from ConstantData; likely no longer applies, but possibly not too hard to rebase.
At the time, @mehdi_amini and @chandlerc both favoured a subsequent follow up to change the type hierarchy. I agree! But it's not necessary for the initial step.
Notice that bitcode defaults to on...