This patch refactors the Mem2Reg infrastructure. It decouples
analysis from promotion, allowing for more control over the execution of
the logic. It also adjusts the interfaces to be less coupled to mem2reg
and more general. This will be useful for an upcoming revision
introducing generic SROA.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
mlir/include/mlir/Transforms/Mem2Reg.h | ||
---|---|---|
25 | nit: Can you put a comma after the first must eliminate or reformulate a bit to simplify the sentence? | |
40 | Optional is deprecated and should be replaced by std::optional AFAIK. | |
mlir/lib/Dialect/LLVMIR/IR/LLVMMemorySlot.cpp | ||
20 | Are you sure all this additional includes are needed? I have the feeling some of them can be eliminated? | |
mlir/lib/Transforms/Mem2Reg.cpp | ||
298 | nit: I would go for return info;. I would expect the compiler can do copy elision here (even if you use std::optional). The main argument is that it is more readable though. | |
441 | nit: Once you use std::optional please do not use has_value() and value(). Instead just use the optional directly in the condition and use * to get its value. |
mlir/include/mlir/Transforms/Mem2Reg.h | ||
---|---|---|
25 | I reformulated using words that require less context so it should be clearer. |
mlir/include/mlir/Interfaces/CMakeLists.txt | ||
---|---|---|
19 | What's the reason to make a hand-rolled tablegen target here? |
mlir/include/mlir/Interfaces/CMakeLists.txt | ||
---|---|---|
19 | This is because it was easier for me to extract this change out of my SROA cchange, and I forgot to manually revert it. The quickly upcoming SROA revision will make use of this by generating type interfaces, so I can also just leave it as-is as it will be used shortly. Please tell me what you prefer. |
mlir/include/mlir/Interfaces/CMakeLists.txt | ||
---|---|---|
19 | Technically, this change is not necessary. On the other hand, this will change anyway soonish, so don't bother with reverting this part. |
What's the reason to make a hand-rolled tablegen target here?