aemerson added a comment. In D143624#4118257 <https://reviews.llvm.org/D143624#4118257>, @dmgreen wrote:
> Hello - I had to revert this because of some large regressions we got from > routines in CMSIS-DSP. > > The llvm/test/Transforms/PhaseOrdering/ARM/arm_mult_q15.ll test shows the > problem - that's why that test exists to ensure that any pipeline changes > don't negatively affect these routines. Unfortunately you just changed the > test as opposed to showing the problems that this causes. They might be > fixable with some other tweaks elsewhere, but the ordering of inlining seems > important for getting the correct code that can be vectorized nicely. > > There are some other cases around inlining this thing on v6m cores: > https://github.com/ARM-software/CMSIS-DSP/blob/809202bf185280a322efc2e2c850a544747f9d79/Include/arm_math_memory.h#L76, > but I'm not sure about the details yet. The mult examples were the really > large regressions. It’s not clear from the original commit message why the test is related to inlining order? It seems entirely testing vectorization cost model which should be insensitive to these kind of changes, right? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143624/new/ https://reviews.llvm.org/D143624 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
