- User Since
- Nov 4 2014, 3:46 PM (438 w, 2 d)
Thu, Mar 16
Feb 28 2023
Feb 7 2023
Feb 4 2023
I will let @davide to decide. I will talk to him offline if he didn't reply here. Looking at the libLTO shipped by Apple, it actually have more symbols than listed here so it could be reasonable just keep the minimal list of symbols intended here.
Feb 3 2023
I don't think this restriction works for Apple. Compatibility is important for users that needs external dependencies that they do not control. If the debug info upgrader is expensive for you, we can probably add a fast check to skip that, or just give user a flag to control that behavior.
Feb 2 2023
@MaskRay Those disassembler symbols are actually used by otool, see: https://opensource.apple.com/source/cctools/cctools-973.0.1/otool/arm_disasm.c.auto.html
I don't think it is feasible with current tool to write a test that check semantically the order of decls in clang module. (Let me know if that was wrong). The current test already unfortunately relies on the module layout assuming op13 is the field for anonymous declaration number. Adding more dependency on the exact layout of the clang module will make the test really fragile to any clang AST change.
Feb 1 2023
Jan 31 2023
Sorry, I'm still not really following - OK, sounds like you're saying this test does fail at HEAD/without this patch in reverse iteration mode, and is a bit larger than would be minimally necessary to achieve that, but also might fail at HEAD without reverse iteration, providing somewhat more testing than if it were fully minimized/only caught in reverse.
Fair enough -I don't think it's the right tradeoff, but I'm glad it's stable/provides that coverage.
Try fix pre-merge build failure
Rebase to main.
Jan 26 2023
I think this violates a core tenet of being a modular, reusable IR. It shouldn't be wrong to run a pass twice. It certainly shouldn't error by way of verifier error
I am not the expert to answer that question. I will think this is an assertion-like behavior. It tells you as a compiler developer that your pipeline configuration is wrong in a sense that you should add all the functions in before running post link passes. You should delay the post link pass when you hit this error, rather than disable the check without a compelling reason.
Jan 25 2023
My understanding of the flag is preventing you from:
- run the devirtual pass
- add some new function
- run the devirtual pass again
Jan 23 2023
I agree that this probably doesn't work for CAS since the use of CAS is not bounded to any context like a ThreadPoolExecutor, for example, it is currently a legal use case to have multiple thread pool to insert into CAS at the same time. It is not feasible to put such a restriction on CAS since CAS should be safe to read/write concurrently from different process.
Jan 20 2023
Ping. Rebase the file on main
This is not an apple platform problem. This is a cross compile problem where you have a SDK that is installed for cross compile target. In those cases, you just can't hard code the path to isysroot, and figuring out the sysroot is generally considered a problem for the build system, not the problem for compiler. If you have a config file, you can definitely setup SDKROOT in your configuration system after running xcrun. A similar but not identical problem is if you just install clang package but didn't install dev packages on a linux platform where you just can't find any headers. Same error, just different causes.
Jan 19 2023
No, reverse iteration will not break diff test for a small number of decls. Everything will be in reverse order so it is the same. Current test will fail early in reverse iteration and will fail in the end statistically for forward iteration. Will pass in all configurations. I think paying a bit more decls to make it fail in forward iteration is not a bad trade off.
This breaks macOS bot too: https://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/33839/
Update tests according to feedback
@dblaikie Do you have any objection for committing the patch as it is? I don't think reverse iteration test is a proper way to test for this specific bug.
Jan 17 2023
Update after makeArrayRef depreciation.
Formatting and minor update.
Address review feedback
Update after makeArrayRef deprecation
@dblaikie Do we have any bots running reverse iteration?
Jan 16 2023
Actually, sorting in numberAnonymousDeclsWithin doesn't work for some reasons.
Minor touch up on the test case. Also add some comments in test to explain what is being tested.
Jan 13 2023
@akyrtzi has the good idea. It is really hard to control Decl* to get values
to get an unstable iteration order from the small tests, going beyond 32 decls
to get out of SmallPtrSet's small model is much consistent.
Jan 12 2023
Jan 10 2023
Jan 9 2023
Address review feedback. Remove NFC from title.
Update patch after HashMap rename
Rename to TrieRawHashMap.
Jan 6 2023
Maybe RawHashTrieMap? It reads better when Raw is in the front, and it contains hash-trie and trie-map, which are both terms describing data structures similar to this but this is much simpler, thus raw.
@dexonsmith @benlangmuir @akyrtzi
Does TrieHashMap sound good to you?
Minor format and test update
Update unittest to wait and check.
@dexonsmith Move the discussion here.
Jan 5 2023
Add docs in the reference page that address CAS in different prespective.
The ability to dump the data structure I'm all for - like DenseMap, etc, but I think that dumping should be the user-visible state (it's a map, the trie part is an implementation detail that ideally shouldn't be exposed in the dump developers use regularly - presumably it's not necessary except when debugging the data structure itself, which should be rare (we don't usually need to look at DenseMap's internal bucketing, for instance)).
Fix non-assert build test
Address review feedback
Ping. All feedbacks are addressed.
Allow checking prefix of the sub-trie
Jan 4 2023
Fix typo in the comments
Update tests to avoid string comparsion.
Jan 3 2023
Add some documentation about HashMappedTrie
Rebase for std::optional
Address review feedback and rebase the patch using std::optional
Thanks for doing the comparison with https://reviews.llvm.org/D133715.
Dec 22 2022
Dec 21 2022
Dec 20 2022
Also needed a follow up fix to completely fix module 9cd6fbee7ed8
Dec 19 2022
Dec 16 2022
Dec 15 2022
Dec 14 2022
This breaks llvm/example:
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm-project/llvm/examples/ModuleMaker/ModuleMaker.cpp:58:7: error: 'getInstList' is a private member of 'llvm::BasicBlock'
The workaround in https://reviews.llvm.org/D139956 can get llvm-project bootstrap correctly, doesn't fix the underlying issue.
Dec 13 2022