Also fixes (#131488) Unreachable case is triggering `Callees.empty()` assert. Since this was [originally ](https://github.com/llvm/llvm-project/pull/87939/commits/02bd5a7013c558f1e5220fc89bafa68f40276549#diff-06aba0dac2a263dc14297a15655291d5506b760f54a736385bcf3208f83df843R2524) a `continue` anyway, have applied that as a fix and added a test case. Please let me know if there's a better way. Not sure who/how to get folks to review, tagging a few people (apologies if you're not the right person/this is the wrong way to do it, please let me know what to do in future if so) @labrinea @dtcxzyw @nikic @fhahn
Specifically in the context of the once-stored transformation, GlobalOpt would strip all pointer casts unconditionally, even though addrspacecasts might be runtime operations. This manifested particularly on CHERI targets. This patch was inspired by an existing change in CHERI LLVM (https://github.com/CHERIoT-Platform/llvm-project/commit/91afa60f17ea1b91e5cdba21a5c90400024b2b6a), but has been reimplemented with updated conventions, and a testcase constructed from scratch.
FMV priority is the returned value of a polymorphic function. On RISC-V and X86 targets a 32-bit value is enough. On AArch64 we currently need 64 bits and we will soon exceed that. APInt seems to be a suitable replacement for uint64_t, presumably with minimal compile time overhead. It allows bit manipulation, comparison and variable bit width.
There are no longer debug-info instructions, thus we don't need this skipping. Horray!
Partially reverts e37d736def5b95a2710f92881b5fc8b0494d8a05. The transform has a number of correctness and code quality issues, and will benefit from a from-scratch re-review more than incremental fixes. The correctness issues are hinted at in https://github.com/llvm/llvm-project/pull/144641, but I think it needs a larger rework to stop working on ArrayTypes and the implementation could use some other improvements (like callInstIsMemcpy should just be `dyn_cast<MemCpyInst>`). I can comment in more detail on a resubmission of the patch.
Co-authored-by: Matt Arsenault <arsenm2@gmail.com>
The dyn_cast was not checked for null, and the cast is guaranteed to succeed by an earlier check.
Following the work in PR #107279, this patch applies the annotative DebugLocs, which indicate that a particular instruction is intentionally missing a location for a given reason, to existing sites in the compiler where their conditions apply. This is NFC in ordinary LLVM builds (each function `DebugLoc::getFoo()` is inlined as `DebugLoc()`), but marks the instruction in coverage-tracking builds so that it will be ignored by Debugify, allowing only real errors to be reported. From a developer standpoint, it also communicates the intentionality and reason for a missing DebugLoc. Some notes for reviewers: - The difference between `I->dropLocation()` and `I->setDebugLoc(DebugLoc::getDropped())` is that the former _may_ decide to keep some debug info alive, while the latter will always be empty; in this patch, I always used the latter (even if the former could technically be correct), because the former could result in some (barely) different output, and I'd prefer to keep this patch purely NFC. - I've generally documented the uses of `DebugLoc::getUnknown()`, with the exception of the vectorizers - in summary, they are a huge cause of dropped source locations, and I don't have the time or the domain knowledge currently to solve that, so I've plastered it all over them as a form of "fixme".
Currently BlockAddresses store both the Function and the BasicBlock they reference, and the BlockAddress is part of the use list of both the Function and BasicBlock. This is quite awkward, because this is not really a use of the function itself (and walks of function uses generally skip block addresses for that reason). This also has weird implications on function RAUW (as that will replace the function in block addresses in a way that generally doesn't make sense), and causes other peculiar issues, like the ability to have multiple block addresses for one block (with different functions). Instead, I believe it makes more sense to specify only the basic block and let the function be implied by the BB parent. This does mean that we may have block addresses without a function (if the BB is not inserted), but this should only happen during IR construction.
When converting a malloc stored to a global into a global, we will introduce an i1 flag to track whether the global has been initialized. In case of atomic loads/stores, this will result in verifier failures, because atomic ops on i1 are illegal. Even if we changed this to i8, I don't think it is a good idea to change atomic types in that way. Instead, bail out of the transform is we encounter any atomic loads/stores of the global. Fixes https://github.com/llvm/llvm-project/issues/137152.