lukel97 wrote: > > I just got the results back, unfortunately the SLP regression on imagick_r > > still exists on RISC-V, is it ok if we hold off on landing this until that > > is fixed? > > It would be great to land this change as early as possible so that we have > enough time to identify and fix any new or unseen issues. As we get closer to > the release branch in mid-January, landing a change like this becomes more > risky. Also, with developers likely to be on holiday over parts of December > and early January, landing the PR this month would give us roughly a month > and a half to address any fallout. > > Separately, the SLP codebase is moving quite quickly, and there may be other > issues that we are not seeing yet because this is not the default path. > @lukel97 If you could do a quick triage of the imagick_r regression and see > whether there is a way to unblock this PR, that would give us the best chance > of enabling the feature for LLVM 24. Of course, if the RISC-V regression > turns out to be something that needs to be fixed before this can safely land, > then we should take that into account.
I don't think we can land this with a known significant performance regression, regardless of where we are in the release cycle. I agree with the point that having it in main gives more time to surface issues, but there's not much point in doing so when we've already identified an issue on RISC-V that would probably mean this patch would need to be reverted anyway. I'm also wary of the fact that we tried to fix the regression in https://github.com/llvm/llvm-project/pull/213104 but it didn't seem to work, so it's not clear yet how much work this is going to involve to fix the regression. I do think the ball is in RISC-V's court though to figure out where the cost modelling is going wrong with SLP. I'll try to see if we can find someone to look into this and bring this up in the next RISC-V sync up meeting. https://github.com/llvm/llvm-project/pull/195886 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
