This refactors load folding to happen in two cleanly separated steps: ConstantFoldLoadFromConstPtr() takes a pointer to load from and decomposes it into a constant initializer base and an offset. Then ConstantFoldLoadFromConst() loads from that initializer at the given offset. This makes the core logic independent of having actual GEP expressions (and those GEP expressions having certain structure) and will allow exposing ConstantFoldLoadFromConst() as an independent API in the future.
Details
Diff Detail
Event Timeline
llvm/lib/Analysis/ConstantFolding.cpp | ||
---|---|---|
694 | Note that this already looks through aliases, so we don't need to handle that separately anymore. | |
735 | I dropped this separate string handling code, because I believe it is redundant with the "reinterpret load" logic. | |
llvm/test/Transforms/InstSimplify/ConstProp/loads.ll | ||
250 | This is a read from a zero-size global. Both results are valid, this is an artifact of different code ordering. | |
270 | A side benefit of not relying on the original GEP structure. |
very nice!
llvm/lib/Analysis/ConstantFolding.cpp | ||
---|---|---|
66–69 | I think this declaration is not necessary anymore |
llvm/test/Transforms/InstSimplify/ConstProp/loads.ll | ||
---|---|---|
34–39 | Nikita, Artur, Philip, // NOTE: Looking through a addrspacecast is not legal here since it would // change address spaces (and thus possibly pointer sizes) I found that the LangRef specifies that the memory location is preserved through addrspacecasts. But nothing is said about sizes. What if we have big-endian in addrspace(0) and little-endian in addrspace(1)? If you agree, I can upstream the one line change in Value.cpp that stops offset calculation at addrspacecasts. |
llvm/test/Transforms/InstSimplify/ConstProp/loads.ll | ||
---|---|---|
34–39 | I don't know whether it's okay to look through addrspacecasts in that method -- the exact semantics of addrspacecasts are not well-specified. The implementation at least is quite careful about dealing with different-width address spaces, so someone clearly thought it was okay at some point. Note that we also accumulate offsets across different address spaces in BasicAA. (I would actually be very happy if that is incorrect and we can drop it, because it causes a large amount of complexity. Just don't know whether that's the case or not.)
This is impossible, because LLVM does not support specifying endianness per address space. |
FYI, there's a miscompilation apparently triggered by this change. Not sure yet whether it's the source of the problem or just exposed it.
https://godbolt.org/z/PvsE4Ybqr
NVCC and clang-11 correctly reload x after llvm.nvvm.barrier, but clang @head does not.
Good news this pass does not appear to be the culprit. The miscompilation happens with earlier builds.
a9bceb2b059dc24870882a71baece895fe430107 from before this patch landed already has the issue.
I think this declaration is not necessary anymore