The implementation is just a generalization of the Select handler.
We're no trying to be smart and compute any kind of fixed point.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/Analysis/MemoryBuiltins.cpp | ||
---|---|---|
815–816 | Comment can be dropped. | |
838 | Support for Exact mode? It might be worthwhile to phrase this in terms of combine(SizeOffsetType, SizeOffsetType, EvalMode) -> SizeOffsetType and reuse that between the Select and the PHINode handling. | |
llvm/test/Transforms/InstCombine/builtin-object-size-phi.ll | ||
1 ↗ | (On Diff #416120) | This should run -instcombine only. |
LGTM
llvm/lib/Analysis/MemoryBuiltins.cpp | ||
---|---|---|
823 | Just as a note, because this is pre-existing behavior of the select code: I find it somewhat odd that we just check for the smaller/larger Size-Offset here, but return the full (Size, Offset) pair. So the Exact/Min/Max only applies to the Size-Offset value, while the Size or Offset values individual might not be Exact/Min/Max. Looking at how this is used, it probably doesn't matter in the end, but it looks a bit suspicious. | |
828 | Needs a default llvm_unreachable to suppress warning (at least with GCC). | |
llvm/test/Transforms/InstCombine/builtin-object-size-phi.ll | ||
1 ↗ | (On Diff #416120) | Okay, this was seriously confusing... The fold did not happen without -instcombine, even though -instcombine had no effect on the IR. The reason is that InstCombine required TLI, and then LowerConstantIntrinsics had an optional TLI dependency, so the no-op InstCombine had an effect. I've fixed this in https://github.com/llvm/llvm-project/commit/ab2284a6437bff8ba14d21cd6f9da927351dc17a by making TLI non-optional. You should be able to just use -lower-constant-intrinsics now, and move this to the LowerConstantIntrinsics directory. |
Comment can be dropped.